[Intel-gfx] [PATCH] drm/i915: Wait until after wm optimization to drop runtime PM reference

[Ville Syrjälä]

On Fri, Mar 04, 2016 at 03:59:39PM -0800, Matt Roper wrote:
> At the end of an atomic commit, we currently wait for vblanks to
> complete, call put() on the various runtime PM references, and then try
> to optimize our watermarks (on platforms that need two-step watermark
> programming).  This can lead to watermark registers being programmed
> while the power well is powered down.  We need to wait until after
> watermark optimization is complete before dropping our runtime power
> references.
> 
> Note that in the future the watermark optimization is probably going to
> move to an asynchronous workqueue task that happens at some arbitrary
> point after vblank.  When we make that change, we'll no longer
> necessarily be operating under the power reference held here, so we'll
> need to wrap the watermark register programmin in a call to
> intel_runtime_pm_get_if_in_use() or similar.
> 
> Cc: arun.siluvery at linux.intel.com
> Cc: ville.syrjala at linux.intel.com
> Cc: maarten.lankhorst at linux.intel.com
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94349
> Fixes: ed4a6a7ca853 ("drm/i915: Add two-stage ILK-style watermark programming (v11)")
> Signed-off-by: Matt Roper <matthew.d.roper at intel.com>

Reviewed-by: Ville Syrjälä <ville.syrjala at linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_display.c | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 62d36a7..0af08d7 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13789,16 +13789,6 @@ static int intel_atomic_commit(struct drm_device *dev,
>  	if (!state->legacy_cursor_update)
>  		intel_atomic_wait_for_vblanks(dev, dev_priv, crtc_vblank_mask);
>  
> -	for_each_crtc_in_state(state, crtc, crtc_state, i) {
> -		intel_post_plane_update(to_intel_crtc(crtc));
> -
> -		if (put_domains[i])
> -			modeset_put_power_domains(dev_priv, put_domains[i]);
> -	}
> -
> -	if (intel_state->modeset)
> -		intel_display_power_put(dev_priv, POWER_DOMAIN_MODESET);
> -
>  	/*
>  	 * Now that the vblank has passed, we can go ahead and program the
>  	 * optimal watermarks on platforms that need two-step watermark
> @@ -13813,6 +13803,16 @@ static int intel_atomic_commit(struct drm_device *dev,
>  			dev_priv->display.optimize_watermarks(intel_cstate);
>  	}
>  
> +	for_each_crtc_in_state(state, crtc, crtc_state, i) {
> +		intel_post_plane_update(to_intel_crtc(crtc));
> +
> +		if (put_domains[i])
> +			modeset_put_power_domains(dev_priv, put_domains[i]);
> +	}
> +
> +	if (intel_state->modeset)
> +		intel_display_power_put(dev_priv, POWER_DOMAIN_MODESET);
> +
>  	mutex_lock(&dev->struct_mutex);
>  	drm_atomic_helper_cleanup_planes(dev, state);
>  	mutex_unlock(&dev->struct_mutex);
> -- 
> 2.1.4

-- 
Ville Syrjälä
Intel OTC