Re: [PATCH] drm/i2c: tda998x: don't register the connector

[Brian Starkey]

On Fri, Sep 23, 2016 at 12:58:46PM +0200, Daniel Vetter wrote:
> On Fri, Sep 23, 2016 at 11:34 AM, Liviu Dudau <Liviu.Dudau@xxxxxxx> wrote:
> > rmmod-ing the hdlcd module generates a WARN() splat as the vsync is still
> > enabled, but we never got the call to turn off the CRTC. Brian is still
> > tracking through the fbdev emulation to figure out the cause for that.
>
> fbdev emulation doesn't do that for you. If you need/want to shut down
> all the crtcs on driver unload, you need to do that yourself. There's
> atomic helpers to do that for you that for you.

The problem is a sort-of circular dependency between ->lastclose (at
least the common implementation of it), unregister and disabling
fbdev.

I want to move drm_dev_unregister() to be the first thing we do at
rmmod-time. However we need to disable fbdev first, otherwise
->lastclose restores the fbdev mode, guaranteeing that vsync is turned
on for drm_vsync_cleanup() to then WARN on.

There's a slightly different (perceived) problem - the one that Liviu
mentions - that drm_fbdev_cma_fini() doesn't disable the CRTC anyway.
You say it's not the fbdev helpers' responsibility to teardown their
modeset, but regardless I have nowhere to disable the CRTC if I want
to do drm_dev_unregister() first; and if the CRTC isn't disabled
there's always a chance of hitting the same vsync WARN even without
fbdev.

We *could* add an ->unload and disable everything there, but as that's
deprecated I'm guessing there should be another way.
Perhaps we should track ->firstopen/->lastclose pairs so we can detect
that ->lastclose is being called from unregister and use it to
disable everything in that case.

drm_vblank_cleanup() seems to have been carried over to unregister
from drm_put_dev(), but drm_dev_register() doesn't call
drm_vblank_init() so it seems a little strange to have it there.
I can see other drivers I'd expect to hit the same WARN but I don't
have HW to test it on.

Thanks,
Brian

> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch