On Fri, 2023-01-27 at 19:30 +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> Extract the skl+ wm latency determination into a small helper
> so that everyone has the same idea what the latency should be.
>
> This introduces a slight functional change in that
> skl_cursor_allocation() will now start to account for the
> extra 4 usec that the kbk/cfl/cml IPC w/a adds.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/skl_watermark.c | 40 +++++++++++++-----
> --
> 1 file changed, 26 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c
> b/drivers/gpu/drm/i915/display/skl_watermark.c
> index ae4e9e680c2e..65c746d018b5 100644
> --- a/drivers/gpu/drm/i915/display/skl_watermark.c
> +++ b/drivers/gpu/drm/i915/display/skl_watermark.c
> @@ -704,6 +704,28 @@ static void skl_compute_plane_wm(const struct
> intel_crtc_state *crtc_state,
> const struct skl_wm_level
> *result_prev,
> struct skl_wm_level *result /* out
> */);
>
> +static unsigned int skl_wm_latency(struct drm_i915_private *i915,
> int level,
> + const struct skl_wm_params *wp)
> +{
> + unsigned int latency = i915->display.wm.skl_latency[level];
> +
> + if (latency == 0)
> + return 0;
What does it mean if latency is 0 here? Why it is ok to skip checks for
ipc_enabled and memory bandwidth workaround?
> +
> + /*
> + * WaIncreaseLatencyIPCEnabled: kbl,cfl
> + * Display WA #1141: kbl,cfl
> + */
> + if ((IS_KABYLAKE(i915) || IS_COFFEELAKE(i915) ||
> IS_COMETLAKE(i915)) &&
> + skl_watermark_ipc_enabled(i915))
> + latency += 4;
> +
> + if (skl_needs_memory_bw_wa(i915) && wp->x_tiled)
> + latency += 15;
> +
> + return latency;
> +}
> +
> static unsigned int
> skl_cursor_allocation(const struct intel_crtc_state *crtc_state,
> int num_active)
> @@ -723,7 +745,7 @@ skl_cursor_allocation(const struct
> intel_crtc_state *crtc_state,
> drm_WARN_ON(&i915->drm, ret);
>
> for (level = 0; level <= max_level; level++) {
> - unsigned int latency = i915-
> >display.wm.skl_latency[level];
> + unsigned int latency = skl_wm_latency(i915, level,
> &wp);
>
> skl_compute_plane_wm(crtc_state, plane, level,
> latency, &wp, &wm, &wm);
> if (wm.min_ddb_alloc == U16_MAX)
> @@ -1834,17 +1856,6 @@ static void skl_compute_plane_wm(const struct
> intel_crtc_state *crtc_state,
> return;
> }
>
> - /*
> - * WaIncreaseLatencyIPCEnabled: kbl,cfl
> - * Display WA #1141: kbl,cfl
> - */
> - if ((IS_KABYLAKE(i915) || IS_COFFEELAKE(i915) ||
> IS_COMETLAKE(i915)) &&
> - skl_watermark_ipc_enabled(i915))
> - latency += 4;
> -
> - if (skl_needs_memory_bw_wa(i915) && wp->x_tiled)
> - latency += 15;
> -
> method1 = skl_wm_method1(i915, wp->plane_pixel_rate,
> wp->cpp, latency, wp-
> >dbuf_block_size);
> method2 = skl_wm_method2(wp->plane_pixel_rate,
> @@ -1971,7 +1982,7 @@ skl_compute_wm_levels(const struct
> intel_crtc_state *crtc_state,
>
> for (level = 0; level <= max_level; level++) {
> struct skl_wm_level *result = &levels[level];
> - unsigned int latency = i915-
> >display.wm.skl_latency[level];
> + unsigned int latency = skl_wm_latency(i915, level,
> wm_params);
>
> skl_compute_plane_wm(crtc_state, plane, level,
> latency,
> wm_params, result_prev, result);
> @@ -1991,7 +2002,8 @@ static void tgl_compute_sagv_wm(const struct
> intel_crtc_state *crtc_state,
> unsigned int latency = 0;
>
> if (i915->display.sagv.block_time_us)
> - latency = i915->display.sagv.block_time_us + i915-
> >display.wm.skl_latency[0];
> + latency = i915->display.sagv.block_time_us +
> + skl_wm_latency(i915, 0, wm_params);
>
> skl_compute_plane_wm(crtc_state, plane, 0, latency,
> wm_params, &levels[0],
