Re: [PATCH 2/2] drm/i915: Ignore stale wm register values on resume on ilk-bdw

[Matt Roper <matthew.d.roper@intel.com>]

On Fri, May 13, 2016 at 05:55:18PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> When we resume the watermark register may contain some BIOS leftovers,
> or just the hardware reset values. We should ignore those as the
> pipes will be off anyway, and so frobbing around with intermediate
> watermarks doesn't make much sense.
> 
> In fact I think we should just throw the skip_intermediate_wm flag
> out, and instead properly sanitize the "active" watermarks to match
> the current plane and pipe states.

That's what the flag was originally added for.  The easiest way to
sanitize the active watermarks is to run the current atomic state
through our whole atomic check process to calculate all derived state
plus the watermark values, and then just push the optimal values into
the hardware.  The flag tells us to just skip calculating the
intermediate values (which might be bogus and/or fail anyway) since all
we care about are the final/optimal values.

Anyway, this looks pretty similar to the fix I wrote right before I went
OOO, but never got a chance to send out; I set the skip flag when the
atomic state was created in the suspend function and you do it in the
resume function, but I think the result should be the same either way,
so overall lgtm.

For both patches:
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>


> The actual wm state readout might
> also need a bit of work. But for now, let's continue with the
> skip_intermediate_wm to keep the fix more minimal.
> 
> Fixes this sort of errors on resume
> [drm:ilk_validate_pipe_wm] LP0 watermark invalid
> [drm:intel_crtc_atomic_check] No valid intermediate pipe watermarks are possible
> [drm:intel_display_resume [i915]] *ERROR* Restoring old state failed with -22
> and a boatload of subsequent modeset BAT fails on my ILK.
> 
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Fixes: ed4a6a7ca853 ("drm/i915: Add two-stage ILK-style watermark programming (v11)")
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index a08c4a45b8d3..938653063f15 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -12003,6 +12003,9 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc,
>  			DRM_DEBUG_KMS("No valid intermediate pipe watermarks are possible\n");
>  			return ret;
>  		}
> +	} else if (dev_priv->display.compute_intermediate_wm) {
> +		if (HAS_PCH_SPLIT(dev_priv) && INTEL_GEN(dev_priv) < 9)
> +			pipe_config->wm.intermediate = pipe_config->wm.optimal.ilk;
>  	}
>  
>  	if (INTEL_INFO(dev)->gen >= 9) {
> @@ -15991,6 +15994,9 @@ retry:
>  
>  		state->acquire_ctx = &ctx;
>  
> +		/* ignore any reset values/BIOS leftovers in the WM registers */
> +		to_intel_atomic_state(state)->skip_intermediate_wm = true;
> +
>  		for_each_crtc_in_state(state, crtc, crtc_state, i) {
>  			/*
>  			 * Force recalculation even if we restore
> -- 
> 2.7.4
> 

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx