From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 09/12] drm/i915: rip our vblank reset hacks for runtime PM Date: Tue, 20 May 2014 16:20:52 +0200 Message-ID: <20140520142052.GZ8790@phenom.ffwll.local> References: <1400093477-3217-1-git-send-email-daniel.vetter@ffwll.ch> <1400093477-3217-10-git-send-email-daniel.vetter@ffwll.ch> <20140520120341.GF27580@intel.com> <20140520133814.GX8790@phenom.ffwll.local> <20140520140332.GH27580@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mail-ee0-f49.google.com (mail-ee0-f49.google.com [74.125.83.49]) by gabe.freedesktop.org (Postfix) with ESMTP id A977B6E8F5 for ; Tue, 20 May 2014 07:20:56 -0700 (PDT) Received: by mail-ee0-f49.google.com with SMTP id e53so615691eek.8 for ; Tue, 20 May 2014 07:20:56 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20140520140332.GH27580@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Cc: Daniel Vetter , Intel Graphics Development , DRI Development List-Id: intel-gfx@lists.freedesktop.org On Tue, May 20, 2014 at 05:03:33PM +0300, Ville Syrj=E4l=E4 wrote: > On Tue, May 20, 2014 at 03:38:14PM +0200, Daniel Vetter wrote: > > On Tue, May 20, 2014 at 03:03:41PM +0300, Ville Syrj=E4l=E4 wrote: > > > On Wed, May 14, 2014 at 08:51:11PM +0200, Daniel Vetter wrote: > > > > Now that we unconditionally dtrt when disabling/enabling crtcs we > > > > don't need any hacks any longer to keep the vblank logic sane when > > > > all the registers go poof. So let's rip it all out. > > > = > > > Hmm. drm_update_vblank_count() will now see some kind of diff between > > > the last and current value when the registers got cloberred. So the > > > vblank counter reported to userspace will jump. But I guess that's fi= ne > > > as long as userspace realizes that the counter is not at all reliable > > > across modesets. > > = > > I've added checks for this (the rpm varianst) and for the similiar > > suspend/resume issues (the suspend variants) to kms_flip. It seems to w= ork > > and we don't actually jump to far. But maybe the tests are horribly > > broken. > = > Hmm. If we can force the power well off at the start of the test and then > set a mode, I'd expect the vblank counter to jump by almost max_vblank_co= unt > since the hw counter would appear to wrap. Oh, I think the tests are busted ... Need to look at this again. -Daniel > = > > = > > Can you please take a closer look? I've thought that the entire point of > > this series (well, one of them) was to finally fix this gag and avoid > > handing totally bogus frame counter values to userspace. Especially for > > system suspend/resume where userspace might get susprised ... > = > I was mostly interested in making vblank interrupts work during plane > enable/disable. Anything else is a bonus. > = > > -Daniel > > = > > > = > > > > = > > > > This essentially undoes > > > > = > > > > commit 9dbd8febb4dbc9199fcf340b882eb930e36b65b6 > > > > Author: Paulo Zanoni > > > > Date: Tue Jul 23 10:48:11 2013 -0300 > > > > = > > > > drm/i915: update last_vblank when disabling the power well > > > > = > > > > Apparently igt/kms_flip is already powerful enough to exercise this > > > > properly, yay! See the reference regression report for details. > > > > = > > > > References: https://bugs.freedesktop.org/show_bug.cgi?id=3D66808 > > > > Testcase: igt/kms_flip/*-vs-rpm > > > > Signed-off-by: Daniel Vetter > > > > --- > > > > drivers/gpu/drm/i915/intel_pm.c | 34 -----------------------------= ----- > > > > 1 file changed, 34 deletions(-) > > > > = > > > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915= /intel_pm.c > > > > index 75c1c766b507..45fa43f16bb3 100644 > > > > --- a/drivers/gpu/drm/i915/intel_pm.c > > > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > > > @@ -5423,33 +5423,6 @@ static void hsw_power_well_post_enable(struc= t drm_i915_private *dev_priv) > > > > } > > > > } > > > > = > > > > -static void reset_vblank_counter(struct drm_device *dev, enum pipe= pipe) > > > > -{ > > > > - assert_spin_locked(&dev->vbl_lock); > > > > - > > > > - dev->vblank[pipe].last =3D 0; > > > > -} > > > > - > > > > -static void hsw_power_well_post_disable(struct drm_i915_private *d= ev_priv) > > > > -{ > > > > - struct drm_device *dev =3D dev_priv->dev; > > > > - enum pipe pipe; > > > > - unsigned long irqflags; > > > > - > > > > - /* > > > > - * After this, the registers on the pipes that are part of the po= wer > > > > - * well will become zero, so we have to adjust our counters accor= ding to > > > > - * that. > > > > - * > > > > - * FIXME: Should we do this in general in drm_vblank_post_modeset? > > > > - */ > > > > - spin_lock_irqsave(&dev->vbl_lock, irqflags); > > > > - for_each_pipe(pipe) > > > > - if (pipe !=3D PIPE_A) > > > > - reset_vblank_counter(dev, pipe); > > > > - spin_unlock_irqrestore(&dev->vbl_lock, irqflags); > > > > -} > > > > - > > > > static void hsw_set_power_well(struct drm_i915_private *dev_priv, > > > > struct i915_power_well *power_well, bool enable) > > > > { > > > > @@ -5478,8 +5451,6 @@ static void hsw_set_power_well(struct drm_i91= 5_private *dev_priv, > > > > I915_WRITE(HSW_PWR_WELL_DRIVER, 0); > > > > POSTING_READ(HSW_PWR_WELL_DRIVER); > > > > DRM_DEBUG_KMS("Requesting to disable the power well\n"); > > > > - > > > > - hsw_power_well_post_disable(dev_priv); > > > > } > > > > } > > > > } > > > > @@ -5646,11 +5617,6 @@ static void vlv_display_power_well_disable(s= truct drm_i915_private *dev_priv, > > > > valleyview_disable_display_irqs(dev_priv); > > > > spin_unlock_irq(&dev_priv->irq_lock); > > > > = > > > > - spin_lock_irq(&dev->vbl_lock); > > > > - for_each_pipe(pipe) > > > > - reset_vblank_counter(dev, pipe); > > > > - spin_unlock_irq(&dev->vbl_lock); > > > > - > > > > vlv_set_power_well(dev_priv, power_well, false); > > > > } > > > > = > > > > -- = > > > > 1.8.3.1 > > > > = > > > > _______________________________________________ > > > > Intel-gfx mailing list > > > > Intel-gfx@lists.freedesktop.org > > > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > = > > > -- = > > > Ville Syrj=E4l=E4 > > > Intel OTC > > = > > -- = > > Daniel Vetter > > Software Engineer, Intel Corporation > > +41 (0) 79 365 57 48 - http://blog.ffwll.ch > = > -- = > Ville Syrj=E4l=E4 > Intel OTC -- = Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch