Re: [PATCH 16/37] drm/hdlcd|mali: Drop drm_vblank_cleanup

[Liviu Dudau <liviu.dudau@arm.com>]

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@arm.com> 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@arm.com> 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.

Best regards,
Liviu

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

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx