On Fri, 2026-02-13 at 06:54 +0200, Ville Syrjälä wrote:
> On Thu, Feb 12, 2026 at 08:45:59PM +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]>
> > ---
> > drivers/gpu/drm/i915/display/intel_audio.c | 16 +++-------------
> > drivers/gpu/drm/i915/display/intel_display_wa.c | 4 ++++
> > drivers/gpu/drm/i915/display/intel_display_wa.h | 1 +
> > 3 files changed, 8 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_audio.c
> > b/drivers/gpu/drm/i915/display/intel_audio.c
> > index 5f3c175afdd2..be4b5dbd36fe 100644
> > --- a/drivers/gpu/drm/i915/display/intel_audio.c
> > +++ b/drivers/gpu/drm/i915/display/intel_audio.c
> > @@ -37,6 +37,7 @@
> > #include "intel_crtc.h"
> > #include "intel_de.h"
> > #include "intel_display_types.h"
> > +#include "intel_display_wa.h"
> > #include "intel_lpe_audio.h"
> >
> > /**
> > @@ -184,17 +185,6 @@ static const struct hdmi_aud_ncts
> > hdmi_aud_ncts_36bpp[] = {
> > { 192000, TMDS_445_5M, 20480, 371250 },
> > };
> >
> > -/*
> > - * WA_14020863754: Implement Audio Workaround
> > - * Corner case with Min Hblank Fix can cause audio hang
>
> We are now losing the description of the problem. Not great.
> Not that the description here is super clear, but at least it
> gives me some idea what this is about.
>
> Perhaps such descriptions should remain with the implementation?
Sorry, this was accidental. In all the remaining patches I just kept
the comments at the call site.
I'll add it back.
> > - */
> > -static bool needs_wa_14020863754(struct intel_display *display)
> > -{
> > - return DISPLAY_VERx100(display) == 3000 ||
> > - DISPLAY_VERx100(display) == 2000 ||
> > - DISPLAY_VERx100(display) == 1401;
> > -}
> > -
> > /* get AUD_CONFIG_PIXEL_CLOCK_HDMI_* value for mode */
> > static u32 audio_config_hdmi_pixel_clock(const struct intel_crtc_state
> > *crtc_state)
> > {
> > @@ -440,7 +430,7 @@ static void hsw_audio_codec_disable(struct
> > intel_encoder *encoder,
> > intel_de_rmw(display, HSW_AUD_PIN_ELD_CP_VLD,
> > AUDIO_OUTPUT_ENABLE(cpu_transcoder), 0);
> >
> > - if (needs_wa_14020863754(display))
> > + if (intel_display_wa(display, 14020863754))
>
> This thing is still a major disaster. There is no way to get from
> here to the list of affected platforms without jumping through crazy
> hoops.
I agree, this whole workarounds stuff is a mess. There are gazillions
of workarounds spread around the code, and most of them can't be
converted to using this "framework". Actually, anything that needs
data from elsewhere than struct intel_display doesn't fit in.
> At the very least the intel_display_wa() macro magic needs to die
> and this should just take the enum directly. Then I could at least
> jump to places where said enum value is used fairly quickly with
> cscope.
I totally agree and thought the same. The macro thing to convert this
number to INTEL_DISPLAY_WA_<number> is useless and just convolutes
things even more.
I'll add a patch to the beginning of this series to remove that.
OTOH, there's a lot to improve, expand and clean up in the way we
handle these. I'm pretty sure a lot of the workarounds are pretty much
dead-code too, because some of the devices the workarounds are for have
never been released to public. I bet the majority of the STEP_A0
devices handled in workarounds have never been released.
But this was not the intention of this patch-series. Here I'm only
moving the "simple ones", i.e. the ones that can be easily be handled
by this framework.
--
Cheers,
Luca.
> > intel_de_rmw(display, AUD_CHICKENBIT_REG3,
> > DACBE_DISABLE_MIN_HBLANK_FIX, 0);
> >
> > intel_audio_sdp_split_update(old_crtc_state, false);
> > @@ -572,7 +562,7 @@ static void hsw_audio_codec_enable(struct intel_encoder
> > *encoder,
> >
> > intel_audio_sdp_split_update(crtc_state, true);
> >
> > - if (needs_wa_14020863754(display))
> > + if (intel_display_wa(display, 14020863754))
> > intel_de_rmw(display, AUD_CHICKENBIT_REG3, 0,
> > DACBE_DISABLE_MIN_HBLANK_FIX);
> >
> > /* Enable audio presence detect */
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_wa.c
> > b/drivers/gpu/drm/i915/display/intel_display_wa.c
> > index c2ccdca2c2f3..99ccc383ee70 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_wa.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display_wa.c
> > @@ -74,6 +74,10 @@ bool __intel_display_wa(struct intel_display *display,
> > enum intel_display_wa wa,
> > DISPLAY_VERx100(display) == 3500;
> > case INTEL_DISPLAY_WA_14011503117:
> > return DISPLAY_VER(display) == 13;
> > + case INTEL_DISPLAY_WA_14020863754:
> > + return DISPLAY_VERx100(display) == 3000 ||
> > + DISPLAY_VERx100(display) == 2000 ||
> > + DISPLAY_VERx100(display) == 1401;
> > case INTEL_DISPLAY_WA_14025769978:
> > return DISPLAY_VER(display) == 35;
> > case INTEL_DISPLAY_WA_15018326506:
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_wa.h
> > b/drivers/gpu/drm/i915/display/intel_display_wa.h
> > index 767420d5f406..bb1382148b6e 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_wa.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_wa.h
> > @@ -29,6 +29,7 @@ bool intel_display_needs_wa_16023588340(struct
> > intel_display *display);
> > enum intel_display_wa {
> > INTEL_DISPLAY_WA_13012396614,
> > INTEL_DISPLAY_WA_14011503117,
> > + INTEL_DISPLAY_WA_14020863754,
> > INTEL_DISPLAY_WA_14025769978,
> > INTEL_DISPLAY_WA_15018326506,
> > INTEL_DISPLAY_WA_16023588340,
> > --
> > 2.51.0
