[Intel-gfx] [PATCH 01/11] drm/i915: don't apply Display WAs 1125 and 1126 to GLK/CNL+

[Ville Syrjälä]

On Tue, Oct 16, 2018 at 03:01:23PM -0700, Paulo Zanoni wrote:
> BSpec does not show these WAs as applicable to GLK, and for CNL it
> only shows them applicable for a super early pre-production stepping
> we shouldn't be caring about anymore. Remove these so we can avoid
> them on ICL too.
> 
> Cc: Matt Roper <matthew.d.roper at intel.com>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 43 ++++++++++++++++++++++-------------------
>  1 file changed, 23 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 67a4d0735291..18157c6ee126 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4696,28 +4696,31 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
>  	res_lines = div_round_up_fixed16(selected_result,
>  					 wp->plane_blocks_per_line);
>  
> -	/* Display WA #1125: skl,bxt,kbl,glk */
> -	if (level == 0 && wp->rc_surface)
> -		res_blocks += fixed16_to_u32_round_up(wp->y_tile_minimum);
> +	if (IS_GEN9(dev_priv) && !IS_GEMINILAKE(dev_priv)) {

IS_GEN9_BC || IS_BXT

would be a little easier to parse, me thinks.

> +		/* Display WA #1125: skl,bxt,kbl */
> +		if (level == 0 && wp->rc_surface)
> +			res_blocks +=
> +				fixed16_to_u32_round_up(wp->y_tile_minimum);
> +
> +		/* Display WA #1126: skl,bxt,kbl */
> +		if (level >= 1 && level <= 7) {
> +			if (wp->y_tiled) {
> +				res_blocks +=
> +				    fixed16_to_u32_round_up(wp->y_tile_minimum);
> +				res_lines += wp->y_min_scanlines;
> +			} else {
> +				res_blocks++;
> +			}
>  
> -	/* Display WA #1126: skl,bxt,kbl,glk */
> -	if (level >= 1 && level <= 7) {
> -		if (wp->y_tiled) {
> -			res_blocks += fixed16_to_u32_round_up(
> -							wp->y_tile_minimum);
> -			res_lines += wp->y_min_scanlines;
> -		} else {
> -			res_blocks++;
> +			/*
> +			 * Make sure result blocks for higher latency levels are
> +			 * atleast as high as level below the current level.
> +			 * Assumption in DDB algorithm optimization for special
> +			 * cases. Also covers Display WA #1125 for RC.
> +			 */
> +			if (result_prev->plane_res_b > res_blocks)
> +				res_blocks = result_prev->plane_res_b;
>  		}
> -
> -		/*
> -		 * Make sure result blocks for higher latency levels are atleast
> -		 * as high as level below the current level.
> -		 * Assumption in DDB algorithm optimization for special cases.
> -		 * Also covers Display WA #1125 for RC.
> -		 */
> -		if (result_prev->plane_res_b > res_blocks)
> -			res_blocks = result_prev->plane_res_b;

This last thing is part of the glk+ watermark formula as well. But
I'm not 100% convinced that it's needed. One might assume that the the
non-decrasing latency values guarantee that the resulting watermarks
are also non-decreasing. But I've not actually done the math to see if
that's true.

Hmm. It might not actually be true on account of the 'memory latency
microseconds >= line time microseconds' check when selecting which
method to use to calculate the watermark. Not quite sure which way
that would make things go.

We also seem to be missing the res_lines handling here. But given
that we only did this for compressed fbs before I don't think this
patch is making things much worse by limiting this to pre-glk only.
The glk+ handling and res_lines fix could be done as a followup.

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


>  	}
>  
>  	if (INTEL_GEN(dev_priv) >= 11) {
> -- 
> 2.14.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel