On Fri, 2026-02-13 at 08:24 +0000, Kandpal, Suraj wrote:
> > Subject: Re: [PATCH v2 11/15] drm/i915/display: convert W/As in
> > intel_modeset_setup.c to new framework
> > 
> > On Fri, 2026-02-13 at 07:04 +0200, Ville Syrjälä wrote:
> > > On Thu, Feb 12, 2026 at 08:46:09PM +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_display_wa.c    | 2 ++
> > > >  drivers/gpu/drm/i915/display/intel_display_wa.h    | 1 +
> > > >  drivers/gpu/drm/i915/display/intel_modeset_setup.c | 3 ++-
> > > >  3 files changed, 5 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_display_wa.c
> > > > b/drivers/gpu/drm/i915/display/intel_display_wa.c
> > > > index 3aa79e607bf8..72f645686efd 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_display_wa.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_display_wa.c
> > > > @@ -79,6 +79,8 @@ bool __intel_display_wa(struct intel_display *display,
> > enum intel_display_wa wa,
> > > >         case INTEL_DISPLAY_WA_13012396614:
> > > >                 return DISPLAY_VERx100(display) == 3000 ||
> > > >                         DISPLAY_VERx100(display) == 3500;
> > > > +       case INTEL_DISPLAY_WA_14010480278:
> > > > +               return (IS_DISPLAY_VER(display, 10, 12));
> > > 
> > > This is now quite confusing. That w/a number only means something for
> > > tgl+. I think if we want to start converting this kind of places
> > > tgl+someone
> > > needs to come up with an actual plan how to deal with older platforms.
> > 
> > If there are more numbers from other platforms, I think they should just 
> > have
> > their own cases, like in some later patches in this series,
> > eg.:
> > 
> >     case INTEL_DISPLAY_WA_<tgl+_wa_number>:
> >     case INTEL_DISPLAY_WA_<lnl_wa_number>:
> > 
> > ...but this was a problem before my series already, and it's not something 
> > I'm
> > trying to address here.
> > 
> > 
> > > Also I'm pretty sure that even among the new platforms some w/a's are
> > > listed with different numbers for different platforms. Has anyone
> > > thought what we should do about that?
> > 
> > Yeah, and there's also the case where more than workaround has the same
> > check, we'll end up with many fallthrough cases.
> > 
> 
> 
> My two cents here:
> Maybe we first get all the WA's here into this framework then work on how to 
> optimize it later.
> Perhaps documenting how and where WA's can be placed, their numbering and 
> version checking.

Yes, I agree with moving the "low-hanging fruits" to the new framework
and continue to improve it from there.  That was the intention of this
series, a first step that hopefully makes it easier to continue with
subsequent clean-ups.

--
Cheers,
Luca.



> Regards,
> Suraj Kandpal
> 
> > --
> > Cheers,
> > Luca.
> > 
> > 
> > > >         case INTEL_DISPLAY_WA_14010547955:
> > > >                 return display->platform.dg2;
> > > >         case INTEL_DISPLAY_WA_14010685332:
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_display_wa.h
> > > > b/drivers/gpu/drm/i915/display/intel_display_wa.h
> > > > index 57345d0abe1c..d8359f88de29 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_display_wa.h
> > > > +++ b/drivers/gpu/drm/i915/display/intel_display_wa.h
> > > > @@ -30,6 +30,7 @@ enum intel_display_wa {
> > > >         INTEL_DISPLAY_WA_1409120013,
> > > >         INTEL_DISPLAY_WA_1409767108,
> > > >         INTEL_DISPLAY_WA_13012396614,
> > > > +       INTEL_DISPLAY_WA_14010480278,
> > > >         INTEL_DISPLAY_WA_14010547955,
> > > >         INTEL_DISPLAY_WA_14010685332,
> > > >         INTEL_DISPLAY_WA_14011294188,
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_modeset_setup.c
> > > > b/drivers/gpu/drm/i915/display/intel_modeset_setup.c
> > > > index 9b0becee221c..7ee1494a67af 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_modeset_setup.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_modeset_setup.c
> > > > @@ -25,6 +25,7 @@
> > > >  #include "intel_display_power.h"
> > > >  #include "intel_display_regs.h"
> > > >  #include "intel_display_types.h"
> > > > +#include "intel_display_wa.h"
> > > >  #include "intel_dmc.h"
> > > >  #include "intel_fifo_underrun.h"
> > > >  #include "intel_modeset_setup.h"
> > > > @@ -913,7 +914,7 @@ static void intel_early_display_was(struct
> > intel_display *display)
> > > >          * Display WA #1185 WaDisableDARBFClkGating:glk,icl,ehl,tgl
> > > >          * Also known as Wa_14010480278.
> > > >          */
> > > > -       if (IS_DISPLAY_VER(display, 10, 12))
> > > > +       if (intel_display_wa(display, 14010480278))
> > > 
> > > >                 intel_de_rmw(display, GEN9_CLKGATE_DIS_0, 0,
> > DARBF_GATING_DIS);
> > > > 
> > > >         /*
> > > > --
> > > > 2.51.0

Reply via email to