On Tue, Oct 21, 2025 at 09:28:40PM -0300, Gustavo Sousa wrote:
> When reading memory latencies for watermark calculations, previous
> display releases instructed to apply an adjustment of adding a certain
> value (e.g. 6us) to all levels when the level 0's memory latency read
> from hardware was zero.
>
> For Xe3p_LPD, the instruction is to always use 6us for level 0 and to
> add that value to the other levels. Update adjust_wm_latency()
> accordingly.
>
> While previously the adjustment was considered a workaround by the
> driver, for Xe3p_LPD that is part of the formal specification. So,
> let's make sure that we differentiate those two in the driver code, even
> if there is a bit of redundancy with "inc += wm_read_latency(display)"
> appearing twice in the code.
>
> v2:
> - Rebased after addition of prep patch "drm/i915/wm: Reorder
> adjust_wm_latency() for Xe3_LPD".
>
> Bspec: 68986, 69126
> Cc: Matt Atwood <[email protected]>
> Cc: Ville Syrjälä <[email protected]>
> Signed-off-by: Gustavo Sousa <[email protected]>
> ---
> drivers/gpu/drm/i915/display/skl_watermark.c | 25 +++++++++++++++++--------
> 1 file changed, 17 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c
> b/drivers/gpu/drm/i915/display/skl_watermark.c
> index 57260a2a765a..5bb6cdc4ad2c 100644
> --- a/drivers/gpu/drm/i915/display/skl_watermark.c
> +++ b/drivers/gpu/drm/i915/display/skl_watermark.c
> @@ -3220,15 +3220,24 @@ adjust_wm_latency(struct intel_display *display)
>
> sanitize_wm_latency(display);
>
> - /*
> - * WaWmMemoryReadLatency
> - *
> - * punit doesn't take into account the read latency so we need
> - * to add proper adjustment to each valid level we retrieve
> - * from the punit when level 0 response data is 0us.
> - */
> - if (wm[0] == 0)
> + if (DISPLAY_VER(display) >= 35) {
> + /*
> + * Xe3p asks to ignore wm[0] read from the register and always
> + * use the adjustment that adds the read latency to all valid
> + * latency values.
> + */
> + wm[0] = 0;
> inc += wm_read_latency(display);
> + } else if (wm[0] == 0) {
> + /*
> + * WaWmMemoryReadLatency
> + *
> + * punit doesn't take into account the read latency so we need
> + * to add proper adjustment to each valid level we retrieve
> + * from the punit when level 0 response data is 0us.
> + */
> + inc += wm_read_latency(display);
> + }
Wouldn't it be simpler to just have a separate
/*
* Xe3p and beyond should ignore level 0's reported latency and
* always apply WaWmMemoryReadLatency logic.
*/
if (DISPLAY_VER(display) >= 35)
wm[0] = 0;
and leave the rest of the code unchanged?
Either way, matches the spec so
Reviewed-by: Matt Roper <[email protected]>
Matt
>
> /*
> * WA Level-0 adjustment for 16Gb+ DIMMs: SKL+
>
> --
> 2.51.0
>
--
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation
