[PATCH v2] drm/i915/display: Adjust Added Wake Time with PKG_C_LATENCY

[Kandpal, Suraj]

> -----Original Message-----
> From: Manna, Animesh <animesh.manna at intel.com>
> Sent: Monday, December 9, 2024 1:17 PM
> To: intel-gfx at lists.freedesktop.org; intel-xe at lists.freedesktop.org
> Cc: Manna, Animesh <animesh.manna at intel.com>; Kandpal, Suraj
> <suraj.kandpal at intel.com>
> Subject: [PATCH v2] drm/i915/display: Adjust Added Wake Time with
> PKG_C_LATENCY
> 
> The PKG_C_LATENCY Added Wake Time field is not working.
> When added wake time is needed, such as for flip queue DSB execution,
> increase the PKG_C_LATENCY Pkg C Latency field by the added wake time.

No need to mention the issue when It comes to WA only what the patch is doing the
Rest of the info is present in the WA

> 
> WA: 22020432604

This needs to come just above the CC with no new line in between CC and WA no.

> 
> v1: Initial version.
> v2: Rebase and cosmetic changes.
> 
> Cc: Suraj Kandpal <suraj.kandpal at intel.com>
> Signed-off-by: Animesh Manna <animesh.manna at intel.com>
> ---
>  drivers/gpu/drm/i915/display/skl_watermark.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c
> b/drivers/gpu/drm/i915/display/skl_watermark.c
> index d93f6786db0e..f6f7205e06eb 100644
> --- a/drivers/gpu/drm/i915/display/skl_watermark.c
> +++ b/drivers/gpu/drm/i915/display/skl_watermark.c
> @@ -2894,6 +2894,12 @@ intel_program_dpkgc_latency(struct
> intel_atomic_state *state)
>  			display->sagv.block_time_us;
>  	}
> 
> +	/* Wa_22020432604 */
> +	if (DISPLAY_VER(i915) == 30) {
> +		latency += added_wake_time;

This wouldn't be the correct place to place it in since this would change the value in case the latency fetched is 0 
>From skl_watermark_max_latency and we actually want to write all 1's and want to disable the deep pkgc
The best place would be right after fetching  max_latency  so it plays nice with the other WA and makes sure that pkgc latency 
Is a multiple of max line time when latency is not 0
So something like

If (display_ver && !latency)
	latency += added_wake_time;

this may also require you to move around where added_wake_time is assigned so that's a different patch
	
	
> +		added_wake_time = 0;

Also lets not re assign 0 to added wake time variable let it just be written its not going to be used anyways and wil
Not have any extra writes from our side

Regards,
Suraj Kandpal
 
> +	}


> +
>  	clear = LNL_ADDED_WAKE_TIME_MASK |
> LNL_PKG_C_LATENCY_MASK;
>  	val = REG_FIELD_PREP(LNL_PKG_C_LATENCY_MASK, latency) |
>  		REG_FIELD_PREP(LNL_ADDED_WAKE_TIME_MASK,
> added_wake_time);
> --
> 2.29.0