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). -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
