On Wed, Oct 01, 2025 at 08:19:23PM +0300, Ville Syrjälä wrote: > On Wed, Oct 01, 2025 at 01:20:20PM +0000, Govindapillai, Vinod wrote: > > + I had Lucas' email id wrong. Fixed. > > > > On Wed, 2025-10-01 at 16:10 +0300, Jani Nikula wrote: > > > On Wed, 01 Oct 2025, "Govindapillai, Vinod" > > > <[email protected]> wrote: > > > > On Wed, 2025-10-01 at 13:03 +0300, Jani Nikula wrote: > > > > > On Wed, 01 Oct 2025, Vinod Govindapillai > > > > > <[email protected]> wrote: > > > > > > wa_22014263786 is not applicable to the BMG and hence exclude it > > > > > > from the wa. > > > > > > > > > > > > v2: Limit this wa to display verion 11 to 14, drop DG2 from the > > > > > > exclusion list, use intel_display_wa (Lucas) > > > > > > > > > > > > Bspec: 74212, 66624 > > > > > > Signed-off-by: Vinod Govindapillai <[email protected]> > > > > > > --- > > > > > > drivers/gpu/drm/i915/display/intel_display_wa.c | 12 ++++++++++++ > > > > > > drivers/gpu/drm/i915/display/intel_display_wa.h | 1 + > > > > > > drivers/gpu/drm/i915/display/intel_fbc.c | 3 +-- > > > > > > 3 files changed, 14 insertions(+), 2 deletions(-) > > > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display_wa.c > > > > > > b/drivers/gpu/drm/i915/display/intel_display_wa.c > > > > > > index 31cd2c9cd488..7ca238725e30 100644 > > > > > > --- a/drivers/gpu/drm/i915/display/intel_display_wa.c > > > > > > +++ b/drivers/gpu/drm/i915/display/intel_display_wa.c > > > > > > @@ -52,6 +52,16 @@ static bool > > > > > > intel_display_needs_wa_16025573575(struct intel_display > > > > > > *display) > > > > > > return DISPLAY_VERx100(display) == 3000 || > > > > > > DISPLAY_VERx100(display) == 3002; > > > > > > } > > > > > > > > > > > > +/* > > > > > > + * Wa_22014263786: > > > > > > + * Fixes: Screen flicker with FBC and Package C state enabled > > > > > > + * Workaround: Forced SLB invalidation before start of new frame. > > > > > > + */ > > > > > > +static bool intel_display_needs_wa_22014263786(struct > > > > > > intel_display *display) > > > > > > +{ > > > > > > + return DISPLAY_VERx100(display) >= 1100 && > > > > > > DISPLAY_VERx100(display) < 1401; > > > > > > > > > > IS_DISPLAY_VERx100(display, 1100, 1400) > > > > > > > > > > Also I'm not sure if we really need separate functions for one-liners > > > > > like this. The documentation could be in the switch case too? *shrug*. > > > > > > > > Thanks. I will update that. I will get rid of the comments. I dont > > > > think it adds any extra > > > > information other than what it can be found from wa details. > > > > > > But I did not say we should get rid of the comments! We don't want to > > > make people look up the wa details, because it's not publicly available > > > information. > > > > > > I'm just wondering if we need the separate *function*. > > > > I got that part. I thought I will remove the comments along with that > > change! Originally this wa > > also did not have much information as comments other than the impacted > > platforms. > > > > Okay. Will wait for Ville's feedback before floating another version > > I think production w/a should generally be included in the public PRMs. > But I don't enjoy looking them up in either internal or public source. > > What I would like to see described is the symptoms that are getting > fixed, and whther those symptoms occur under specific circumstancs > (esp. if we decided to apply the w/a as a bigger hammer without > actually checking for those specific circumstances). That sort of > information can be very useful when you're seeing some problem and > pondering whether you might be missing some existing w/a. > > Very low level details aren't probably particularly useful even to > us. To really understand those you probably have to trawl the hsd(s) > anyway. > > So after some pondering I'm thinking the explanations should perhaps > still stay where the w/a is implemented, and this w/a list would > _only_ contain the platform checks. And I would also want to be able > to immediately jump from the actual code to the relevant w/a platform > check (as in with cscope/etc). Though that's still a somewhat annoying > extra step or two, esp. since I can't directly jump to the defition of > the enum value but rather need to jump to one of its uses in > __intel_displa_wa() :/ I suppose if all of these were functions without > the enum step in the middle that problem would be solved. But dunno if > we want to have a function per w/a number?
The other annoyance is that soemtimes the same w/a lives under a different number for each platform. What should we do with those? Oh, and we do also have the unsolved problem of what to do with w/as that predate the hsd based numbering (either using the skl style w/a numbers or simply no numbers). And we also have some undocumented w/as as well (eg. issues we found long after the hw people had moved on). -- Ville Syrjälä Intel
