Quoting Ville Syrjälä (2025-10-16 18:03:45-03:00)
>On Wed, Oct 15, 2025 at 12:15:15AM -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.
>>
>> Bspec: 68986, 69126
>> Signed-off-by: Gustavo Sousa <[email protected]>
>> ---
>> drivers/gpu/drm/i915/display/skl_watermark.c | 7 +++++++
>> 1 file changed, 7 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c
>> b/drivers/gpu/drm/i915/display/skl_watermark.c
>> index 41f64e347436..88342d07727f 100644
>> --- a/drivers/gpu/drm/i915/display/skl_watermark.c
>> +++ b/drivers/gpu/drm/i915/display/skl_watermark.c
>> @@ -3249,6 +3249,13 @@ adjust_wm_latency(struct intel_display *display)
>>
>> make_wm_latency_monotonic(display);
>>
>> + /*
>> + * Xe3p asks to ignore wm[0] read from the register and always
>> + * use the adjustment done with read_latency.
>> + */
>> + if (DISPLAY_VER(display) >= 35)
>> + wm[0] = 0;
>
>make_wm_latency_monotonic() already used wm[0]. I think this
>needs to be the very first thing you do in adjust_wm_latency().
Right. Or as an alternative, maybe we could have
make_wm_latency_monotonic() be the last thing to be done?
I was thinking about having this as the end result:
|diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c
b/drivers/gpu/drm/i915/display/skl_watermark.c
|index 237af46c1974..b3f8cbadeb99 100644
|--- a/drivers/gpu/drm/i915/display/skl_watermark.c
|+++ b/drivers/gpu/drm/i915/display/skl_watermark.c
|@@ -3213,39 +3213,44 @@ static void
| adjust_wm_latency(struct intel_display *display)
| {
| u16 *wm = display->wm.skl_latency;
|+ int inc = 0;
|
| if (display->platform.dg2)
| multiply_wm_latency(display, 2);
|
| sanitize_wm_latency(display);
|
|- make_wm_latency_monotonic(display);
|-
| /*
| * Xe3p asks to ignore wm[0] read from the register and always
| * use the adjustment done with read_latency.
| */
|- if (DISPLAY_VER(display) >= 35)
|+ if (DISPLAY_VER(display) >= 35) {
| 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.
|- */
|- if (wm[0] == 0)
|- increase_wm_latency(display, wm_read_latency(display));
|+ 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);
|+ }
|
| /*
| * WA Level-0 adjustment for 16Gb+ DIMMs: SKL+
| * If we could not get dimm info enable this WA to prevent from
| * any underrun. If not able to get DIMM info assume 16Gb+ DIMM
| * to avoid any underrun.
| */
| if (need_16gb_dimm_wa(display))
|- increase_wm_latency(display, 1);
|+ inc += 1;
|+
|+ if (inc)
|+ increase_wm_latency(display, inc);
|+
|+ make_wm_latency_monotonic(display);
| }
|
| static void mtl_read_wm_latency(struct intel_display *display)
With that, we:
* make sure to differentiate between WaWmMemoryReadLatency
and what now is a "normal" Bspec instruction starting with
Xe3p_LD.
* have a single call to increase_wm_latency().
It could be split into 2 patches, if you prefer: first to use a single
call to increase_wm_latency() and then another for Xe3p_LPD (which would
include moving make_wm_latency_monotonic()).
What do you think?
--
Gustavo Sousa
>
>> +
>> /*
>> * WaWmMemoryReadLatency
>> *
>>
>> --
>> 2.51.0
>
>--
>Ville Syrjälä
>Intel
