On Wed, May 31, 2017 at 05:57:07PM +0100, Liviu Dudau wrote: > On Wed, May 31, 2017 at 06:41:05PM +0200, Daniel Vetter wrote: > > On Wed, May 31, 2017 at 1:22 PM, Liviu Dudau <liviu.dudau@xxxxxxx> wrote: > > > On Wed, May 31, 2017 at 01:03:34PM +0200, Daniel Vetter wrote: > > >> On Wed, May 31, 2017 at 12:57 PM, Liviu Dudau <liviu.dudau@xxxxxxx> wrote: > > >> > On Wed, May 24, 2017 at 04:51:51PM +0200, Daniel Vetter wrote: > > >> >> IRQs are properly shut down, so it almost works as race-free shutdown. > > >> >> Except the irq is stopped after the vblank stuff, so boom anyway. > > >> >> Proper way would be to call drm_atomic_helper_shutdown before any of > > >> >> the kms things gets stopped. So no harm in removing the > > >> >> drm_vblank_cleanup here really. > > >> > > > >> > Slightly confused on the implied message in the commit text: is "Proper way > > >> > would be to call drm_atomic_helper_shutdown" a hint? A promise of a future > > >> > patch? A message to the future us on how to fix things if they blow up? > > >> > > > >> > If calling drm_atomic_helper_shutdown() is the proper thing to do, why does the > > >> > patch (and the series) avoids doing that? Lack of understanding of the driver's > > >> > internal workings? Then I want to help, if I can understand the new direction. > > >> > > >> Yes, I wanted to not make things worse. If you look at the overall > > >> result (especially last patch) I'm also trying to better document > > >> stuff in the vblank area, but summarized, if you want to make sure > > >> that vblank processing has stopped, you need to call drm_vblank_off on > > >> each active crtc. The simplest way to get that is by using > > >> drm_atomic_helper_shutdown(). Calling drm_vblank_cleanup doesn't > > >> really do anything useful (see the last patch for the only valid > > >> usecase there ever was). > > > > > > OK, so there should be a follow up patch adding drm_atomic_helper_shutdown(). I guess > > > I need to call dibs on it? :) > > > > I googled what "to call dibs on smth" means and still can't figure out > > what you mean here. Can you pls explain? > > Means I put my name on it. Urban dictionary on "call dibs" gives it a very gender > specific usage, my intent was to convey the message that I will take it on my TODO list. Ah ok, thanks for explaining. I was indeed thrown off be the ub entry ... -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
