[PATCH v2] drm/i915/fbc: update the impacted platforms in wa_22014263786

[PATCH v2] drm/i915/fbc: update the impacted platforms in wa_22014263786

[Vinod Govindapillai]

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 <vinod.govindapillai@intel.com>
---
 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;
+}
+
 /*
  * Wa_14011503117:
  * Fixes: Before enabling the scaler DE fatal error is masked
@@ -67,6 +77,8 @@ bool __intel_display_wa(struct intel_display *display, enum intel_display_wa wa,
 		return intel_display_needs_wa_16025573575(display);
 	case INTEL_DISPLAY_WA_14011503117:
 		return DISPLAY_VER(display) == 13;
+	case INTEL_DISPLAY_WA_22014263786:
+		return intel_display_needs_wa_22014263786(display);
 	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 abc1df83f066..3644e8e2b724 100644
--- a/drivers/gpu/drm/i915/display/intel_display_wa.h
+++ b/drivers/gpu/drm/i915/display/intel_display_wa.h
@@ -25,6 +25,7 @@ enum intel_display_wa {
 	INTEL_DISPLAY_WA_16023588340,
 	INTEL_DISPLAY_WA_16025573575,
 	INTEL_DISPLAY_WA_14011503117,
+	INTEL_DISPLAY_WA_22014263786,
 };
 
 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 0d380c825791..9607fdcb0cc0 100644
--- a/drivers/gpu/drm/i915/display/intel_fbc.c
+++ b/drivers/gpu/drm/i915/display/intel_fbc.c
@@ -933,8 +933,7 @@ static void intel_fbc_program_workarounds(struct intel_fbc *fbc)
 		intel_de_rmw(display, ILK_DPFC_CHICKEN(fbc->id),
 			     0, DPFC_CHICKEN_COMP_DUMMY_PIXEL);
 
-	/* Wa_22014263786:icl,jsl,tgl,dg1,rkl,adls,adlp,mtl */
-	if (DISPLAY_VER(display) >= 11 && !display->platform.dg2)
+	if (intel_display_wa(display, 22014263786))
 		intel_de_rmw(display, ILK_DPFC_CHICKEN(fbc->id),
 			     0, DPFC_CHICKEN_FORCE_SLB_INVALIDATION);
 
-- 
2.43.0

Re: [PATCH v2] drm/i915/fbc: update the impacted platforms in wa_22014263786

[Jani Nikula]

On Wed, 01 Oct 2025, Vinod Govindapillai <vinod.govindapillai@intel.com> 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 <vinod.govindapillai@intel.com>
> ---
>  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*.

BR,
Jani.

> +}
> +
>  /*
>   * Wa_14011503117:
>   * Fixes: Before enabling the scaler DE fatal error is masked
> @@ -67,6 +77,8 @@ bool __intel_display_wa(struct intel_display *display, enum intel_display_wa wa,
>  		return intel_display_needs_wa_16025573575(display);
>  	case INTEL_DISPLAY_WA_14011503117:
>  		return DISPLAY_VER(display) == 13;
> +	case INTEL_DISPLAY_WA_22014263786:
> +		return intel_display_needs_wa_22014263786(display);
>  	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 abc1df83f066..3644e8e2b724 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_wa.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_wa.h
> @@ -25,6 +25,7 @@ enum intel_display_wa {
>  	INTEL_DISPLAY_WA_16023588340,
>  	INTEL_DISPLAY_WA_16025573575,
>  	INTEL_DISPLAY_WA_14011503117,
> +	INTEL_DISPLAY_WA_22014263786,
>  };
>  
>  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 0d380c825791..9607fdcb0cc0 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
> @@ -933,8 +933,7 @@ static void intel_fbc_program_workarounds(struct intel_fbc *fbc)
>  		intel_de_rmw(display, ILK_DPFC_CHICKEN(fbc->id),
>  			     0, DPFC_CHICKEN_COMP_DUMMY_PIXEL);
>  
> -	/* Wa_22014263786:icl,jsl,tgl,dg1,rkl,adls,adlp,mtl */
> -	if (DISPLAY_VER(display) >= 11 && !display->platform.dg2)
> +	if (intel_display_wa(display, 22014263786))
>  		intel_de_rmw(display, ILK_DPFC_CHICKEN(fbc->id),
>  			     0, DPFC_CHICKEN_FORCE_SLB_INVALIDATION);

-- 
Jani Nikula, Intel

✗ i915.CI.BAT: failure for drm/i915/fbc: update the impacted platforms in wa_22014263786 (rev2)

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 5477 bytes --]

== Series Details ==

Series: drm/i915/fbc: update the impacted platforms in wa_22014263786 (rev2)
URL   : https://patchwork.freedesktop.org/series/154951/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_17293 -> Patchwork_154951v2
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_154951v2 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_154951v2, please notify your bug team (I915-ci-infra@lists.freedesktop.org) to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_154951v2/index.html

Participating hosts (44 -> 43)
------------------------------

  Missing    (1): fi-snb-2520m 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_154951v2:

### IGT changes ###

#### Possible regressions ####

  * igt@core_hotunplug@unbind-rebind:
    - fi-kbl-8809g:       [PASS][1] -> [ABORT][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_17293/fi-kbl-8809g/igt@core_hotunplug@unbind-rebind.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_154951v2/fi-kbl-8809g/igt@core_hotunplug@unbind-rebind.html

  
Known issues
------------

  Here are the changes found in Patchwork_154951v2 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_module_load@load:
    - bat-mtlp-9:         [PASS][3] -> [ABORT][4] ([i915#13494])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_17293/bat-mtlp-9/igt@i915_module_load@load.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_154951v2/bat-mtlp-9/igt@i915_module_load@load.html

  * igt@i915_selftest@live:
    - bat-mtlp-8:         [PASS][5] -> [DMESG-FAIL][6] ([i915#12061]) +1 other test dmesg-fail
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_17293/bat-mtlp-8/igt@i915_selftest@live.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_154951v2/bat-mtlp-8/igt@i915_selftest@live.html
    - bat-adlp-9:         [PASS][7] -> [ABORT][8] ([i915#14365]) +1 other test abort
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_17293/bat-adlp-9/igt@i915_selftest@live.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_154951v2/bat-adlp-9/igt@i915_selftest@live.html

  * igt@i915_selftest@live@workarounds:
    - bat-dg2-9:          [PASS][9] -> [DMESG-FAIL][10] ([i915#12061]) +1 other test dmesg-fail
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_17293/bat-dg2-9/igt@i915_selftest@live@workarounds.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_154951v2/bat-dg2-9/igt@i915_selftest@live@workarounds.html
    - bat-arls-6:         [PASS][11] -> [DMESG-FAIL][12] ([i915#12061]) +1 other test dmesg-fail
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_17293/bat-arls-6/igt@i915_selftest@live@workarounds.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_154951v2/bat-arls-6/igt@i915_selftest@live@workarounds.html

  
#### Possible fixes ####

  * igt@i915_selftest@live:
    - bat-dg2-8:          [DMESG-FAIL][13] ([i915#12061]) -> [PASS][14] +1 other test pass
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_17293/bat-dg2-8/igt@i915_selftest@live.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_154951v2/bat-dg2-8/igt@i915_selftest@live.html

  * igt@i915_selftest@live@reset:
    - bat-atsm-1:         [ABORT][15] ([i915#14201]) -> [PASS][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_17293/bat-atsm-1/igt@i915_selftest@live@reset.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_154951v2/bat-atsm-1/igt@i915_selftest@live@reset.html

  
#### Warnings ####

  * igt@i915_selftest@live:
    - bat-atsm-1:         [ABORT][17] ([i915#14201]) -> [DMESG-FAIL][18] ([i915#12061] / [i915#13929])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_17293/bat-atsm-1/igt@i915_selftest@live.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_154951v2/bat-atsm-1/igt@i915_selftest@live.html

  * igt@i915_selftest@live@mman:
    - bat-atsm-1:         [DMESG-FAIL][19] ([i915#14204]) -> [DMESG-FAIL][20] ([i915#13929])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_17293/bat-atsm-1/igt@i915_selftest@live@mman.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_154951v2/bat-atsm-1/igt@i915_selftest@live@mman.html

  
  [i915#12061]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12061
  [i915#13494]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13494
  [i915#13929]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13929
  [i915#14201]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/14201
  [i915#14204]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/14204
  [i915#14365]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/14365


Build changes
-------------

  * Linux: CI_DRM_17293 -> Patchwork_154951v2

  CI-20190529: 20190529
  CI_DRM_17293: bb42887c28cd6ede2c83547a9afc852a6aed3569 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_8565: 8565
  Patchwork_154951v2: bb42887c28cd6ede2c83547a9afc852a6aed3569 @ git://anongit.freedesktop.org/gfx-ci/linux

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_154951v2/index.html

[-- Attachment #2: Type: text/html, Size: 6734 bytes --]

Re: [PATCH v2] drm/i915/fbc: update the impacted platforms in wa_22014263786

[Govindapillai, Vinod]

On Wed, 2025-10-01 at 13:03 +0300, Jani Nikula wrote:
> On Wed, 01 Oct 2025, Vinod Govindapillai <vinod.govindapillai@intel.com> 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 <vinod.govindapillai@intel.com>
> > ---
> >  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.

BR
Vinod
> 
> BR,
> Jani.
> 
> > +}
> > +
> >  /*
> >   * Wa_14011503117:
> >   * Fixes: Before enabling the scaler DE fatal error is masked
> > @@ -67,6 +77,8 @@ bool __intel_display_wa(struct intel_display *display, enum intel_display_wa
> > wa,
> >  		return intel_display_needs_wa_16025573575(display);
> >  	case INTEL_DISPLAY_WA_14011503117:
> >  		return DISPLAY_VER(display) == 13;
> > +	case INTEL_DISPLAY_WA_22014263786:
> > +		return intel_display_needs_wa_22014263786(display);
> >  	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 abc1df83f066..3644e8e2b724 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_wa.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_wa.h
> > @@ -25,6 +25,7 @@ enum intel_display_wa {
> >  	INTEL_DISPLAY_WA_16023588340,
> >  	INTEL_DISPLAY_WA_16025573575,
> >  	INTEL_DISPLAY_WA_14011503117,
> > +	INTEL_DISPLAY_WA_22014263786,
> >  };
> >  
> >  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 0d380c825791..9607fdcb0cc0 100644
> > --- a/drivers/gpu/drm/i915/display/intel_fbc.c
> > +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
> > @@ -933,8 +933,7 @@ static void intel_fbc_program_workarounds(struct intel_fbc *fbc)
> >  		intel_de_rmw(display, ILK_DPFC_CHICKEN(fbc->id),
> >  			     0, DPFC_CHICKEN_COMP_DUMMY_PIXEL);
> >  
> > -	/* Wa_22014263786:icl,jsl,tgl,dg1,rkl,adls,adlp,mtl */
> > -	if (DISPLAY_VER(display) >= 11 && !display->platform.dg2)
> > +	if (intel_display_wa(display, 22014263786))
> >  		intel_de_rmw(display, ILK_DPFC_CHICKEN(fbc->id),
> >  			     0, DPFC_CHICKEN_FORCE_SLB_INVALIDATION);
> 

Re: [PATCH v2] drm/i915/fbc: update the impacted platforms in wa_22014263786

[Jani Nikula]

On Wed, 01 Oct 2025, "Govindapillai, Vinod" <vinod.govindapillai@intel.com> wrote:
> On Wed, 2025-10-01 at 13:03 +0300, Jani Nikula wrote:
>> On Wed, 01 Oct 2025, Vinod Govindapillai <vinod.govindapillai@intel.com> 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 <vinod.govindapillai@intel.com>
>> > ---
>> >  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*.

CC: Ville who's had pretty strong opinions on this whole thing.


BR,
Jani.


>
> BR
> Vinod
>> 
>> BR,
>> Jani.
>> 
>> > +}
>> > +
>> >  /*
>> >   * Wa_14011503117:
>> >   * Fixes: Before enabling the scaler DE fatal error is masked
>> > @@ -67,6 +77,8 @@ bool __intel_display_wa(struct intel_display *display, enum intel_display_wa
>> > wa,
>> >  		return intel_display_needs_wa_16025573575(display);
>> >  	case INTEL_DISPLAY_WA_14011503117:
>> >  		return DISPLAY_VER(display) == 13;
>> > +	case INTEL_DISPLAY_WA_22014263786:
>> > +		return intel_display_needs_wa_22014263786(display);
>> >  	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 abc1df83f066..3644e8e2b724 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_display_wa.h
>> > +++ b/drivers/gpu/drm/i915/display/intel_display_wa.h
>> > @@ -25,6 +25,7 @@ enum intel_display_wa {
>> >  	INTEL_DISPLAY_WA_16023588340,
>> >  	INTEL_DISPLAY_WA_16025573575,
>> >  	INTEL_DISPLAY_WA_14011503117,
>> > +	INTEL_DISPLAY_WA_22014263786,
>> >  };
>> >  
>> >  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 0d380c825791..9607fdcb0cc0 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_fbc.c
>> > +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
>> > @@ -933,8 +933,7 @@ static void intel_fbc_program_workarounds(struct intel_fbc *fbc)
>> >  		intel_de_rmw(display, ILK_DPFC_CHICKEN(fbc->id),
>> >  			     0, DPFC_CHICKEN_COMP_DUMMY_PIXEL);
>> >  
>> > -	/* Wa_22014263786:icl,jsl,tgl,dg1,rkl,adls,adlp,mtl */
>> > -	if (DISPLAY_VER(display) >= 11 && !display->platform.dg2)
>> > +	if (intel_display_wa(display, 22014263786))
>> >  		intel_de_rmw(display, ILK_DPFC_CHICKEN(fbc->id),
>> >  			     0, DPFC_CHICKEN_FORCE_SLB_INVALIDATION);
>> 
>

-- 
Jani Nikula, Intel

Re: [PATCH v2] drm/i915/fbc: update the impacted platforms in wa_22014263786

[Govindapillai, Vinod]

+ 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" <vinod.govindapillai@intel.com> wrote:
> > On Wed, 2025-10-01 at 13:03 +0300, Jani Nikula wrote:
> > > On Wed, 01 Oct 2025, Vinod Govindapillai <vinod.govindapillai@intel.com> 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 <vinod.govindapillai@intel.com>
> > > > ---
> > > >  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

BR
Vinod
> 
> CC: Ville who's had pretty strong opinions on this whole thing.
> 
> 
> BR,
> Jani.
> 
> 
> > 
> > BR
> > Vinod
> > > 
> > > BR,
> > > Jani.
> > > 
> > > > +}
> > > > +
> > > >  /*
> > > >   * Wa_14011503117:
> > > >   * Fixes: Before enabling the scaler DE fatal error is masked
> > > > @@ -67,6 +77,8 @@ bool __intel_display_wa(struct intel_display *display, enum
> > > > intel_display_wa
> > > > wa,
> > > >  		return intel_display_needs_wa_16025573575(display);
> > > >  	case INTEL_DISPLAY_WA_14011503117:
> > > >  		return DISPLAY_VER(display) == 13;
> > > > +	case INTEL_DISPLAY_WA_22014263786:
> > > > +		return intel_display_needs_wa_22014263786(display);
> > > >  	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 abc1df83f066..3644e8e2b724 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_display_wa.h
> > > > +++ b/drivers/gpu/drm/i915/display/intel_display_wa.h
> > > > @@ -25,6 +25,7 @@ enum intel_display_wa {
> > > >  	INTEL_DISPLAY_WA_16023588340,
> > > >  	INTEL_DISPLAY_WA_16025573575,
> > > >  	INTEL_DISPLAY_WA_14011503117,
> > > > +	INTEL_DISPLAY_WA_22014263786,
> > > >  };
> > > >  
> > > >  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 0d380c825791..9607fdcb0cc0 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_fbc.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
> > > > @@ -933,8 +933,7 @@ static void intel_fbc_program_workarounds(struct intel_fbc *fbc)
> > > >  		intel_de_rmw(display, ILK_DPFC_CHICKEN(fbc->id),
> > > >  			     0, DPFC_CHICKEN_COMP_DUMMY_PIXEL);
> > > >  
> > > > -	/* Wa_22014263786:icl,jsl,tgl,dg1,rkl,adls,adlp,mtl */
> > > > -	if (DISPLAY_VER(display) >= 11 && !display->platform.dg2)
> > > > +	if (intel_display_wa(display, 22014263786))
> > > >  		intel_de_rmw(display, ILK_DPFC_CHICKEN(fbc->id),
> > > >  			     0, DPFC_CHICKEN_FORCE_SLB_INVALIDATION);
> > > 
> > 
> 

Re: [PATCH v2] drm/i915/fbc: update the impacted platforms in wa_22014263786

[Ville Syrjälä]

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" <vinod.govindapillai@intel.com> wrote:
> > > On Wed, 2025-10-01 at 13:03 +0300, Jani Nikula wrote:
> > > > On Wed, 01 Oct 2025, Vinod Govindapillai <vinod.govindapillai@intel.com> 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 <vinod.govindapillai@intel.com>
> > > > > ---
> > > > >  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?

-- 
Ville Syrjälä
Intel

Re: [PATCH v2] drm/i915/fbc: update the impacted platforms in wa_22014263786

[Ville Syrjälä]

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" <vinod.govindapillai@intel.com> wrote:
> > > > On Wed, 2025-10-01 at 13:03 +0300, Jani Nikula wrote:
> > > > > On Wed, 01 Oct 2025, Vinod Govindapillai <vinod.govindapillai@intel.com> 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 <vinod.govindapillai@intel.com>
> > > > > > ---
> > > > > >  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