On Fri, 2026-02-13 at 07:09 +0200, Ville Syrjälä wrote:
> On Thu, Feb 12, 2026 at 08:46:11PM +0200, Luca Coelho wrote:
> > Convert the low-hanging fruits of workaround checks to the workaround
> > framework. Instead of having display structure checks for the
> > workarounds all over, concentrate the checks in intel_wa.c.
> >
> > Acked-by: Jani Nikula <[email protected]>
> > Signed-off-by: Luca Coelho <[email protected]>
> > ---
> > .../gpu/drm/i915/display/intel_display_wa.c | 15 ++++++++++++--
> > .../gpu/drm/i915/display/intel_display_wa.h | 4 ++++
> > drivers/gpu/drm/i915/display/intel_psr.c | 20 ++++++++-----------
> > 3 files changed, 25 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_wa.c
> > b/drivers/gpu/drm/i915/display/intel_display_wa.c
> > index 77ea2e5b8144..783e1383ff89 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_wa.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display_wa.c
> > @@ -112,6 +112,13 @@ bool __intel_display_wa(struct intel_display *display,
> > enum intel_display_wa wa,
> > return DISPLAY_VER(display) == 20;
> > case INTEL_DISPLAY_WA_15018326506:
> > return display->platform.battlemage;
> > + case INTEL_DISPLAY_WA_16011303918:
> > + case INTEL_DISPLAY_WA_22011320316:
> > + return display->platform.alderlake_p &&
> > + IS_DISPLAY_STEP(display, STEP_A0, STEP_B0);
> > + case INTEL_DISPLAY_WA_16011181250:
> > + return display->platform.rocketlake ||
> > display->platform.alderlake_s ||
> > + display->platform.dg2;
> > case INTEL_DISPLAY_WA_16011342517:
> > return display->platform.alderlake_p &&
> > IS_DISPLAY_STEP(display, STEP_A0, STEP_D0);
> > @@ -121,15 +128,19 @@ bool __intel_display_wa(struct intel_display
> > *display, enum intel_display_wa wa,
> > return intel_display_needs_wa_16023588340(display);
> > case INTEL_DISPLAY_WA_16025573575:
> > return intel_display_needs_wa_16025573575(display);
> > + case INTEL_DISPLAY_WA_16025596647:
> > + return DISPLAY_VER(display) != 20 &&
> > + !IS_DISPLAY_VERx100_STEP(display, 3000,
> > + STEP_A0, STEP_B0);
>
> This one is nuts. It declarates (incorrectly) which platforms don't
> need the w/a. I don't think this sort of thing should be allowed here
> ever.
Yeah, this is bad. I noticed the inversion (i.e. return who _doesn't_
need, as opposed to how all the other ones, which return who _does_
need the workaroun), but I admit I didn't give it much attention. I'll
invert it.
> Presumably the only reason it was OK in the old place is because
> those codepaths were only executed on some new platforms. But
> __intel_display_wa() is so generic that is is clearly meant to
> give correct answers regardless of where it gets called.
I don't think it was okay in the old place either. It's just confusing
(unless it had a clear comment on why it was like this).
--
Cheers,
Luca.
> > case INTEL_DISPLAY_WA_18034343758:
> > return DISPLAY_VER(display) == 20 ||
> > (display->platform.pantherlake &&
> > IS_DISPLAY_STEP(display, STEP_A0, STEP_B0));
> > case INTEL_DISPLAY_WA_22010178259:
> > return DISPLAY_VER(display) == 12;
> > - case INTEL_DISPLAY_WA_22011320316:
> > + case INTEL_DISPLAY_WA_22012278275:
> > return display->platform.alderlake_p &&
> > - IS_DISPLAY_STEP(display, STEP_A0, STEP_B0);
> > + IS_DISPLAY_STEP(display, STEP_A0, STEP_E0);
> > case INTEL_DISPLAY_WA_22014263786:
> > return IS_DISPLAY_VERx100(display, 1100, 1400);
> > case INTEL_DISPLAY_WA_22021048059:
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_wa.h
> > b/drivers/gpu/drm/i915/display/intel_display_wa.h
> > index 3d2cf05ffacc..35d8df4c75a2 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_wa.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_wa.h
> > @@ -44,13 +44,17 @@ enum intel_display_wa {
> > INTEL_DISPLAY_WA_14025769978,
> > INTEL_DISPLAY_WA_15013987218,
> > INTEL_DISPLAY_WA_15018326506,
> > + INTEL_DISPLAY_WA_16011181250,
> > + INTEL_DISPLAY_WA_16011303918,
> > INTEL_DISPLAY_WA_16011342517,
> > INTEL_DISPLAY_WA_16011863758,
> > INTEL_DISPLAY_WA_16023588340,
> > INTEL_DISPLAY_WA_16025573575,
> > + INTEL_DISPLAY_WA_16025596647,
> > INTEL_DISPLAY_WA_18034343758,
> > INTEL_DISPLAY_WA_22010178259,
> > INTEL_DISPLAY_WA_22011320316,
> > + INTEL_DISPLAY_WA_22012278275,
> > INTEL_DISPLAY_WA_22012358565,
> > INTEL_DISPLAY_WA_22014263786,
> > INTEL_DISPLAY_WA_22021048059,
> > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > b/drivers/gpu/drm/i915/display/intel_psr.c
> > index 5bea2eda744b..b21e52f0c461 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > @@ -40,6 +40,7 @@
> > #include "intel_display_rpm.h"
> > #include "intel_display_types.h"
> > #include "intel_display_utils.h"
> > +#include "intel_display_wa.h"
> > #include "intel_dmc.h"
> > #include "intel_dp.h"
> > #include "intel_dp_aux.h"
> > @@ -1082,7 +1083,7 @@ static void hsw_activate_psr2(struct intel_dp
> > *intel_dp)
> > }
> >
> > /* Wa_22012278275:adl-p */
> > - if (display->platform.alderlake_p && IS_DISPLAY_STEP(display, STEP_A0,
> > STEP_E0)) {
> > + if (intel_display_wa(display, 22012278275)) {
> > static const u8 map[] = {
> > 2, /* 5 lines */
> > 1, /* 6 lines */
> > @@ -1263,7 +1264,7 @@ tgl_dc3co_exitline_compute_config(struct intel_dp
> > *intel_dp,
> > return;
> >
> > /* Wa_16011303918:adl-p */
> > - if (display->platform.alderlake_p && IS_DISPLAY_STEP(display, STEP_A0,
> > STEP_B0))
> > + if (intel_display_wa(display, 16011303918))
> > return;
> >
> > /*
> > @@ -1540,8 +1541,7 @@ static bool intel_psr2_config_valid(struct intel_dp
> > *intel_dp,
> > }
> >
> > /* Wa_16011181250 */
> > - if (display->platform.rocketlake || display->platform.alderlake_s ||
> > - display->platform.dg2) {
> > + if (intel_display_wa(display, 16011181250)) {
> > drm_dbg_kms(display->drm,
> > "PSR2 is defeatured for this platform\n");
> > return false;
> > @@ -1823,8 +1823,7 @@ void intel_psr_set_non_psr_pipes(struct intel_dp
> > *intel_dp,
> > u8 active_pipes = 0;
> >
> > /* Wa_16025596647 */
> > - if (DISPLAY_VER(display) != 20 &&
> > - !IS_DISPLAY_VERx100_STEP(display, 3000, STEP_A0, STEP_B0))
> > + if (intel_display_wa(display, 16025596647))
> > return;
> >
> > /* Not needed by Panel Replay */
> > @@ -3973,8 +3972,7 @@ static void psr_dc5_dc6_wa_work(struct work_struct
> > *work)
> > */
> > void intel_psr_notify_dc5_dc6(struct intel_display *display)
> > {
> > - if (DISPLAY_VER(display) != 20 &&
> > - !IS_DISPLAY_VERx100_STEP(display, 3000, STEP_A0, STEP_B0))
> > + if (intel_display_wa(display, 16025596647))
> > return;
> >
> > schedule_work(&display->psr_dc5_dc6_wa_work);
> > @@ -3989,8 +3987,7 @@ void intel_psr_notify_dc5_dc6(struct intel_display
> > *display)
> > */
> > void intel_psr_dc5_dc6_wa_init(struct intel_display *display)
> > {
> > - if (DISPLAY_VER(display) != 20 &&
> > - !IS_DISPLAY_VERx100_STEP(display, 3000, STEP_A0, STEP_B0))
> > + if (intel_display_wa(display, 16025596647))
> > return;
> >
> > INIT_WORK(&display->psr_dc5_dc6_wa_work, psr_dc5_dc6_wa_work);
> > @@ -4011,8 +4008,7 @@ void intel_psr_notify_pipe_change(struct
> > intel_atomic_state *state,
> > struct intel_display *display = to_intel_display(state);
> > struct intel_encoder *encoder;
> >
> > - if (DISPLAY_VER(display) != 20 &&
> > - !IS_DISPLAY_VERx100_STEP(display, 3000, STEP_A0, STEP_B0))
> > + if (intel_display_wa(display, 16025596647))
> > return;
> >
> > for_each_intel_encoder_with_psr(display->drm, encoder) {
> > --
> > 2.51.0
