Re: [PATCH 3/5] drm: Allow the driver to reject vblank requests only when it really has the vblank interrupts disabled

[Daniel Vetter <daniel@ffwll.ch>]

On Wed, Mar 05, 2014 at 02:38:25PM +0200, Ville Syrjälä wrote:
> On Tue, Mar 04, 2014 at 10:24:54AM +0100, Daniel Vetter wrote:
> > On Fri, Feb 21, 2014 at 09:03:33PM +0200, ville.syrjala@linux.intel.com wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > Allow the driver to specify whether all new vblank requests after
> > > drm_vblank_off() should be rejected. And add a counterpart called
> > > drm_vblank_on() which will again allow vblank requests to come in.
> > > 
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Not really happy about this - drm_irq.c is already a giant mess, adding
> > more driver-specific hacks doesn't help. I think we need a few bits of
> > polish on top of your code:
> > 
> > - Add stern warnings to pre/post_modeset that they're inherently racy.
> > 
> > - Add calls to drm_vblank_off to every driver lacking them. Put it at the
> >   beginning of their crtc disable functions expect when there's a call to
> >   pre_modeset. Then it should be right after that.
> > 
> > - Sprinkle calls to drm_vblank_on over all drivers. Put them at the end of
> >   their crtc enable functions except when there's a call to post_modeset.
> >   Then put it right before that.
> > 
> > - Rip out the reject argument again and make it the default. If we have
> >   drm_vblank_off everywhere then all old vblank waits should complete and
> >   there's no userspace yet out there which races a modeset with vblank
> >   ioctl calls. Then only issue would be userspace which does vblank waits
> >   on disabled ioctls which a) is buggy b) we can easily fix with a driver
> >   quirk flag if _really_ needed.
> > 
> > This way the drm_irq.c mess will at least converge a bit and so should
> > help generic display servers (like Wayland) a lot.
> > 
> > I can volunteer for this if you want to punt on it yourself.
> 
> Much appreciated. I'll punt.
> 
> My hope was that other people would fix their own mess after seeing how
> i915 did it, and then we could rip out the crap, but if you're feeling
> the urge to do it all upfront I certainly won't object.

My experience tells me that the only way to fix a cross-driver mess is to
simple charge ahead. If driver maintainers are asleep they'll end up with
a broken driver, but that usually gets their attention. Pleas and praying
don't ;-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch