On Thu, Oct 30, 2025 at 12:22:01AM +0200, Ville Syrjälä wrote: > On Tue, Oct 21, 2025 at 09:28:39PM -0300, Gustavo Sousa wrote: > > In an upcoming change related to Xe3p_LPD, we will need to (i) update > > wm[0] in adjust_wm_latency() and (ii) do the same increase_wm_latency() > > that is currently done when (wm[0] == 0). > > > > Because make_wm_latency_monotonic() depends on wm[0], part (i) needs to > > be done before it gets called. In order to keep (i) and (ii) as a > > contiguous logical sequence, let's reorder adjust_wm_latency(), making > > make_wm_latency_monotonic() the last thing to be done. > > > > Also take this opportunity to simplify the code by doing the call to > > increase_wm_latency() only once. > > > > Cc: Ville Syrjälä <[email protected]> > > Signed-off-by: Gustavo Sousa <[email protected]> > > --- > > drivers/gpu/drm/i915/display/skl_watermark.c | 12 ++++++++---- > > 1 file changed, 8 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c > > b/drivers/gpu/drm/i915/display/skl_watermark.c > > index c141d575009f..57260a2a765a 100644 > > --- a/drivers/gpu/drm/i915/display/skl_watermark.c > > +++ b/drivers/gpu/drm/i915/display/skl_watermark.c > > @@ -3213,14 +3213,13 @@ 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); > > - > > I was thinking that by doing this early we avoid potentially papering > over our own bugs in the later adjustments. But Windows does appear to > do this after the read latency adjustment.
Hmm, I suppose it doesn't really matter whether it's before or after since the read latency adjustment applies to all wm levels. So I think I still prefer to keep it early to avoid papering over our own bugs. > > And it looks like Windows actually stopped doing this for xe3 and now > it rejects non-monotonic values. And it also does that after the read > latency adjustment. > > So I guess what we want to do is move this later, only call it for > pre-xe3, and add another step after it to validate that the latencies > are indeed monotonic. > > > /* > > * WaWmMemoryReadLatency > > * > > @@ -3229,7 +3228,7 @@ adjust_wm_latency(struct intel_display *display) > > * 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); > > > > /* > > * WA Level-0 adjustment for 16Gb+ DIMMs: SKL+ > > @@ -3238,7 +3237,12 @@ adjust_wm_latency(struct intel_display *display) > > * to avoid any underrun. > > */ > > if (need_16gb_dimm_wa(display)) > > - increase_wm_latency(display, 1); > > + inc += 1; > > + > > + if (inc) > > + increase_wm_latency(display, inc); > > I don't see that variable being helpful in any real way. > Just makes the function more complicated for no good reason. > It also has nothing to do with the rest of this patch. > > > + > > + make_wm_latency_monotonic(display); > > } > > > > static void mtl_read_wm_latency(struct intel_display *display) > > > > -- > > 2.51.0 > > -- > Ville Syrjälä > Intel -- Ville Syrjälä Intel
