[Intel-gfx] [PATCH 4/6] drm/i915: transition WMs ask for Selected Result Blocks

[Matt Roper]

On Thu, Oct 04, 2018 at 04:15:58PM -0700, Paulo Zanoni wrote:
> The transition watermarks ask for Selected Result Blocks (the real
> value), not Result Blocks (the integer value). Given how ceilings are
> applied in both the non-transition and the transition watermarks
> calculations, we can get away with assuming that Selected Result
> Blocks is actually Result Blocks minus 1 without any rounding errors.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni at intel.com>

Matches current bspec.

Reviewed-by: Matt Roper <matthew.d.roper at intel.com>

I wonder if we should make trans_amount configurable via debugfs so that
we can experiment with the impact of transitional watermarks.


Matt

> ---
>  drivers/gpu/drm/i915/intel_pm.c | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 40ce99c455f3..14f13a371989 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4874,7 +4874,7 @@ static void skl_compute_transition_wm(struct intel_crtc_state *cstate,
>  	const struct drm_i915_private *dev_priv = to_i915(dev);
>  	uint16_t trans_min, trans_y_tile_min;
>  	const uint16_t trans_amount = 10; /* This is configurable amount */
> -	uint16_t trans_offset_b, res_blocks;
> +	uint16_t wm0_sel_res_b, trans_offset_b, res_blocks;
>  
>  	if (!cstate->base.active)
>  		goto exit;
> @@ -4893,13 +4893,25 @@ static void skl_compute_transition_wm(struct intel_crtc_state *cstate,
>  
>  	trans_offset_b = trans_min + trans_amount;
>  
> +	/*
> +	 * The spec asks for Selected Result Blocks for wm0 (the real value),
> +	 * not Result Blocks (the integer value). Pay attention to the capital
> +	 * letters. The value wm_l0->plane_res_b is actually Result Blocks, but
> +	 * since Result Blocks is the ceiling of Selected Result Blocks plus 1,
> +	 * and since we later will have to get the ceiling of the sum in the
> +	 * transition watermarks calculation, we can just pretend Selected
> +	 * Result Blocks is Result Blocks minus 1 and it should work for the
> +	 * current platforms.
> +	 */
> +	wm0_sel_res_b = wm_l0->plane_res_b - 1;
> +
>  	if (wp->y_tiled) {
>  		trans_y_tile_min = (uint16_t) mul_round_up_u32_fixed16(2,
>  							wp->y_tile_minimum);
> -		res_blocks = max(wm_l0->plane_res_b, trans_y_tile_min) +
> +		res_blocks = max(wm0_sel_res_b, trans_y_tile_min) +
>  				trans_offset_b;
>  	} else {
> -		res_blocks = wm_l0->plane_res_b + trans_offset_b;
> +		res_blocks = wm0_sel_res_b + trans_offset_b;
>  
>  		/* WA BUG:1938466 add one block for non y-tile planes */
>  		if (IS_CNL_REVID(dev_priv, CNL_REVID_A0, CNL_REVID_A0))
> -- 
> 2.14.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795