Re: [PATCH] drm/i915/fbc: Apply wa_15018326506

[Jani Nikula <jani.nikula@linux.intel.com>]

On Fri, 07 Nov 2025, "Govindapillai, Vinod" <vinod.govindapillai@intel.com> wrote:
> On Fri, 2025-11-07 at 12:16 +0200, Jani Nikula wrote:
>> On Fri, 07 Nov 2025, Vinod Govindapillai
>> <vinod.govindapillai@intel.com> wrote:
>> > Disable FBC as part for the wa for the bmg variant
>> > 
>> > Bspec: 74212
>> > Signed-off-by: Vinod Govindapillai <vinod.govindapillai@intel.com>
>> > ---
>> >  drivers/gpu/drm/i915/display/intel_display_wa.c |  4 ++++
>> >  drivers/gpu/drm/i915/display/intel_display_wa.h |  1 +
>> >  drivers/gpu/drm/i915/display/intel_fbc.c        | 10 ++++++++++
>> >  3 files changed, 15 insertions(+)
>> > 
>> > diff --git a/drivers/gpu/drm/i915/display/intel_display_wa.c
>> > b/drivers/gpu/drm/i915/display/intel_display_wa.c
>> > index c528aaa679ca..ba2272d85a04 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_display_wa.c
>> > +++ b/drivers/gpu/drm/i915/display/intel_display_wa.c
>> > @@ -10,6 +10,7 @@
>> >  #include "intel_display_core.h"
>> >  #include "intel_display_regs.h"
>> >  #include "intel_display_wa.h"
>> > +#include "intel_step.h"
>> >  
>> >  static void gen11_display_wa_apply(struct intel_display *display)
>> >  {
>> > @@ -69,6 +70,9 @@ bool __intel_display_wa(struct intel_display
>> > *display, enum intel_display_wa wa,
>> >  		return DISPLAY_VER(display) == 13;
>> >  	case INTEL_DISPLAY_WA_22014263786:
>> >  		return IS_DISPLAY_VERx100(display, 1100, 1400);
>> > +	case INTEL_DISPLAY_WA_15018326506:
>> > +		return DISPLAY_VERx100(display) == 1401 &&
>> > +		       INTEL_DISPLAY_STEP(display) == STEP_C0;
>> 
>> IS_DISPLAY_VERx100_STEP().
>
> I was looking into the comments for that macro! This wa apply only for
> step c0 (as of now)! I wasn't sure what the "until" part could be in
> that macro! So what do you suggest for the "until"  STEP_D0 or
> STEP_FOREVER in such cases!

If it's C0 only, then C1 is the next step, not D0.

Basically there's only one user of INTEL_DISPLAY_STEP() outside of the
macros themselves, and we should avoid adding more users.

BR,
Jani.



>
> BR
> Vinod
>
>
>> 
>> >  	default:
>> >  		drm_WARN(display->drm, 1, "Missing Wa number:
>> > %s\n", name);
>> >  		break;
>> > diff --git a/drivers/gpu/drm/i915/display/intel_display_wa.h
>> > b/drivers/gpu/drm/i915/display/intel_display_wa.h
>> > index 3644e8e2b724..f648b00cb97d 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_display_wa.h
>> > +++ b/drivers/gpu/drm/i915/display/intel_display_wa.h
>> > @@ -26,6 +26,7 @@ enum intel_display_wa {
>> >  	INTEL_DISPLAY_WA_16025573575,
>> >  	INTEL_DISPLAY_WA_14011503117,
>> >  	INTEL_DISPLAY_WA_22014263786,
>> > +	INTEL_DISPLAY_WA_15018326506,
>> >  };
>> >  
>> >  bool __intel_display_wa(struct intel_display *display, enum
>> > intel_display_wa wa, const char *name);
>> > diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c
>> > b/drivers/gpu/drm/i915/display/intel_fbc.c
>> > index a1e3083022ee..16cd99db2978 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_fbc.c
>> > +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
>> > @@ -1521,6 +1521,16 @@ static int intel_fbc_check_plane(struct
>> > intel_atomic_state *state,
>> >  		return 0;
>> >  	}
>> >  
>> > +	/*
>> > +	 * wa_15018326506:
>> > +	 * Fixes: Underrun during media decode
>> > +	 * Workaround: Do not enable FBC
>> > +	 */
>> > +	if (intel_display_wa(display, 15018326506)) {
>> > +		plane_state->no_fbc_reason = "Wa_15018326506";
>> > +		return 0;
>> > +	}
>> > +
>> >  	/* WaFbcTurnOffFbcWhenHyperVisorIsUsed:skl,bxt */
>> >  	if (intel_display_vtd_active(display) &&
>> >  	    (display->platform.skylake || display-
>> > >platform.broxton)) {
>> 
>

-- 
Jani Nikula, Intel