Re: [Intel-gfx] [PATCH 09/12] drm/i915: rip our vblank reset hacks for runtime PM

From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Intel Graphics Development <intel-gfx@lists.freedesktop.org>,
	DRI Development <dri-devel@lists.freedesktop.org>
Subject: Re: [Intel-gfx] [PATCH 09/12] drm/i915: rip our vblank reset hacks for runtime PM
Date: Tue, 20 May 2014 15:03:41 +0300	[thread overview]
Message-ID: <20140520120341.GF27580@intel.com> (raw)
In-Reply-To: <1400093477-3217-10-git-send-email-daniel.vetter@ffwll.ch>

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 fine
as long as userspace realizes that the counter is not at all reliable
across modesets.

> 
> This essentially undoes
> 
> commit 9dbd8febb4dbc9199fcf340b882eb930e36b65b6
> Author: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 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=66808
> Testcase: igt/kms_flip/*-vs-rpm
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  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(struct 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 = 0;
> -}
> -
> -static void hsw_power_well_post_disable(struct drm_i915_private *dev_priv)
> -{
> -	struct drm_device *dev = dev_priv->dev;
> -	enum pipe pipe;
> -	unsigned long irqflags;
> -
> -	/*
> -	 * After this, the registers on the pipes that are part of the power
> -	 * well will become zero, so we have to adjust our counters according 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 != 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_i915_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(struct 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älä
Intel OTC