Re: [Intel-gfx] [PATCH 10/11] drm/i915/skl: Update watermarks before the crtc is disabled.

[Daniel Vetter <daniel@ffwll.ch>]

On Thu, Oct 22, 2015 at 01:56:35PM +0200, Maarten Lankhorst wrote:
> On skylake some of the registers are only writable when the correct
> power wells are enabled. Because of this watermarks have to be updated
> before the crtc turns off, or you get unclaimed register read and write
> warnings.
> 
> This patch needs to be modified slightly to apply to -fixes.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92181
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: stable@vger.kernel.org
> Cc: Matt Roper <matthew.d.roper@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 6f92ccf2461c..26931d8eb7ce 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4761,7 +4761,7 @@ static void intel_post_plane_update(struct intel_crtc_state *old_crtc_state)
>  
>  	crtc->wm.cxsr_allowed = true;
>  
> -	if (pipe_config->wm_changed)
> +	if (pipe_config->wm_changed && pipe_config->base.active)
>  		intel_update_watermarks(&crtc->base);

Calling post_plane_commit seems like a bug of the higher-level functions.
If we can't fix that quickly I think we should have at least an early exit
at the top, with a FIXME comment.

If the platform/feature-specific commit hooks have to care about
state->active, then the higher level functions imo haven't done their jobs
properly.

>  
>  	for_each_plane_in_state(old_state, plane, old_plane_state, i) {
> @@ -13205,6 +13205,9 @@ static int intel_atomic_commit(struct drm_device *dev,
>  			dev_priv->display.crtc_disable(crtc);
>  			intel_crtc->active = false;
>  			intel_disable_shared_dpll(intel_crtc);
> +
> +			if (!crtc->state->active)
> +				intel_update_watermarks(crtc);

Does this ever do anything at all? We just killed the crtc completely
above, those watermark writes will get cleansed as soon as the power well
goes down.
-Daniel

>  		}
>  	}
>  
> -- 
> 2.1.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch