From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 3/5] drm: Allow the driver to reject vblank requests only when it really has the vblank interrupts disabled Date: Wed, 5 Mar 2014 14:55:56 +0100 Message-ID: <20140305135556.GS17001@phenom.ffwll.local> References: <1393009415-27651-1-git-send-email-ville.syrjala@linux.intel.com> <1393009415-27651-4-git-send-email-ville.syrjala@linux.intel.com> <20140304092454.GS17001@phenom.ffwll.local> <20140305123825.GR3852@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mail-ea0-f181.google.com (mail-ea0-f181.google.com [209.85.215.181]) by gabe.freedesktop.org (Postfix) with ESMTP id 165E9F9EA4 for ; Wed, 5 Mar 2014 05:56:00 -0800 (PST) Received: by mail-ea0-f181.google.com with SMTP id k10so1040313eaj.26 for ; Wed, 05 Mar 2014 05:56:00 -0800 (PST) Content-Disposition: inline In-Reply-To: <20140305123825.GR3852@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces@lists.freedesktop.org Errors-To: intel-gfx-bounces@lists.freedesktop.org To: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Cc: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Wed, Mar 05, 2014 at 02:38:25PM +0200, Ville Syrj=E4l=E4 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=E4l=E4 > > > = > > > 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=E4l=E4 > > = > > 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 t= he > > 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_modese= t. > > 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 a= nd > > there's no userspace yet out there which races a modeset with vblank > > ioctl calls. Then only issue would be userspace which does vblank wai= ts > > on disabled ioctls which a) is buggy b) we can easily fix with a driv= er > > 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