[Intel-gfx] [PATCH 3/8] drm/i915/gen10: Calculate and enable transition WM

[Maarten Lankhorst]

Op 18-07-17 om 14:49 schreef Mahesh Kumar:
> From: "Kumar, Mahesh" <mahesh1.kumar at intel.com>
>
> GEN > 9 require transition WM to be programmed if IPC is enabled.
> This patch calculates & enable transition WM for supported platforms.
> If transition WM is enabled, Plane read requests are sent at high
> priority until filling above the transition watermark, then the
> requests are sent at lower priority until dropping below the level-0 WM.
> The lower priority requests allow other memory clients to have better
> memory access.
>
> transition minimum is the minimum amount needed for trans_wm to work to
> ensure  the demote does not happen before enough data has been read to
> meet the level 0 watermark requirements.
>
> transition amount is configurable value. Higher values will
> tend to cause longer periods of high priority reads followed by longer
> periods of lower priority reads. Tuning to lower values will tend to
> cause shorter periods of high and lower priority reads.
>
> Keeping transition amount to 10 in this patch, as suggested by HW team.
>
> Signed-off-by: Mahesh Kumar <mahesh1.kumar at intel.com>
> Acked-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 51 +++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 49 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index b2bd65847d9b..9a2ed1b734d5 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4579,12 +4579,55 @@ skl_compute_linetime_wm(struct intel_crtc_state *cstate)
>  }
>  
>  static void skl_compute_transition_wm(struct intel_crtc_state *cstate,
> +				      struct skl_wm_params *wp,
> +				      struct skl_wm_level *wm_l0,
> +				      uint16_t ddb_allocation,
>  				      struct skl_wm_level *trans_wm /* out */)
>  {
> +	struct drm_device *dev = cstate->base.crtc->dev;
> +	const struct drm_i915_private *dev_priv = to_i915(dev);
> +	uint16_t trans_min, trans_y_tile_min;
> +	uint16_t trans_amount = 10; /* This is configurable amount */
should probably be a const since it's a tweakable.
> +	uint16_t trans_offset_b, res_blocks;
> +
>  	if (!cstate->base.active)
> +		goto exit;
> +
> +	/* Transition WM are not recommended by HW team for GEN9 */
> +	if (INTEL_GEN(dev_priv) <= 9)
> +		goto exit;
> +
> +	/* Transition WM don't make any sense if ipc is disabled */
> +	if (!dev_priv->ipc_enabled)
> +		goto exit;
> +
> +	if (INTEL_GEN(dev_priv) >= 10)
> +		trans_min = 4;
> +
> +	trans_offset_b = trans_min + trans_amount;
> +	trans_y_tile_min = (uint16_t) mul_round_up_u32_fixed16(2,
> +							wp->y_tile_minimum);
> +
> +	if (wp->y_tiled) {
> +		res_blocks = max(wm_l0->plane_res_b, trans_y_tile_min) +
> +				trans_offset_b;
Perhaps only calculate trans_y_tile_min if y-tiled?
> +	} else {
> +		res_blocks = wm_l0->plane_res_b + trans_offset_b;
> +	}
> +
> +	res_blocks += 1;
> +
> +	/* WA BUG:1938466 add one block for non y-tile planes */
> +	if (!wp->y_tiled && IS_CNL_REVID(dev_priv, CNL_REVID_A0, CNL_REVID_A0))
> +		res_blocks += 1;
Could be added to the !y_tiled branch?

Patch looks sane though, and seems to match bspec exactly. I couldn't find the value for trans_amount, but I'll take your word it's the right one. :)

With the minor fixes:

Reviewed-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>