Hi Suraj,
On Mon, 2024-02-05 at 13:31 +0530, Suraj Kandpal wrote:
> Program the PKGC_LATENCY register with the highest latency from
> level 1 and above LP registers else program with all 1's.
> This is used to improve package C residency by sending the highest
> latency tolerance requirement (LTR) when the planes are done with the
> frame until the next frame programming window (set context latency,
> window 2) starts.
> Bspec: 68986
>
> --v2
> -Fix indentation [Chaitanya]
>
> Signed-off-by: Suraj Kandpal <suraj.kandpal@xxxxxxxxx>
> Reviewed-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/skl_watermark.c | 31 ++++++++++++++++++++
> 1 file changed, 31 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c
> b/drivers/gpu/drm/i915/display/skl_watermark.c
> index 051a02ac01a4..1ce4b33a407a 100644
> --- a/drivers/gpu/drm/i915/display/skl_watermark.c
> +++ b/drivers/gpu/drm/i915/display/skl_watermark.c
> @@ -3394,6 +3394,34 @@ static void skl_read_wm_latency(struct drm_i915_private *i915, u16 wm[])
> adjust_wm_latency(i915, wm, num_levels, read_latency);
> }
>
> +/*
> + * Program PKG_C_LATENCY Pkg C with highest valid latency from
> + * watermark level1 and up and above. If watermark level 1 is
> + * invalid program it with all 1's.
> + * Program PKG_C_LATENCY Added Wake Time = 0.
> + */
Could you please confirm if the added wake time = 0 always? The Bspec says the otherway!
> +static void intel_program_pkgc_latency(struct drm_i915_private *i915,
> + u16 wm_latency[])
> +{
> + u16 max_value = 0;
> + u32 clear = 0, val = 0;
> + int max_level = i915->display.wm.num_levels, i;
> +
> + for (i = 1; i <= max_level; i++) {
> + if (wm_latency[i] == 0)
> + break;
> + else if (wm_latency[i] > max_value)
> + max_value = wm_latency[i];
> + }
May be efficient to iterate max to 1 (skl_max_wm_level_for_vblank())
Also better to call skl_wm_lateny() instead of accessing the lantency values directly as there are
some latency adjustments been done per platforms.
> +
> + if (max_value == 0)
> + max_value = ~0 & LNL_PKG_C_LATENCY_MASK;
> +
> + clear |= LNL_ADDED_WAKE_TIME_MASK | LNL_PKG_C_LATENCY_MASK;
As mentioned above the waketime is cleared here. Please double check if it is as expected.
> + val |= max_value;
> + intel_uncore_rmw(&i915->uncore, LNL_PKG_C_LATENCY, clear, val);
> +}
> +
> static void skl_setup_wm_latency(struct drm_i915_private *i915)
> {
> if (HAS_HW_SAGV_WM(i915))
> @@ -3407,6 +3435,9 @@ static void skl_setup_wm_latency(struct drm_i915_private *i915)
> skl_read_wm_latency(i915, i915->display.wm.skl_latency);
>
> intel_print_wm_latency(i915, "Gen9 Plane", i915->display.wm.skl_latency);
> +
> + if (DISPLAY_VER(i915) >= 20)
> + intel_program_pkgc_latency(i915, i915->display.wm.skl_latency);
skl_setup_wm_latency() gets called only at the init time. Though latency values dont change, but as
per bspec you need to disable this for VRR. I guess you would need to have provision to update this
based on that. So should this check be moved to intel_atomic_check()?
BR
Vinod
> }
>
> static const struct intel_wm_funcs skl_wm_funcs = {
