[PATCH 2/2] drm/i915/fbc: Move DPFC_CHICKEN programming into intel_fbc_program_workarounds()

Gustavo Sousa gustavo.sousa at intel.com
Tue Jan 30 19:31:49 UTC 2024


Quoting Ville Syrjälä (2024-01-26 07:26:52-03:00)
>On Tue, Jan 23, 2024 at 10:42:46AM -0300, Gustavo Sousa wrote:
>> Quoting Ville Syrjala (2024-01-23 06:00:51-03:00)
>> >From: Ville Syrjälä <ville.syrjala at linux.intel.com>
>> >
>> >Move all DPFC_CHICKEN programming into intel_fbc_program_workarounds().
>> >We already have one thing programmed there, whereas the rest is strewn
>> >about in intel_display_wa_apply() and init_clock_gating(). Since we have
>> >a single place doing all the programming (and it's serialized by the
>> >crtc commits) there should be no danger of rmw races.
>> >
>> >Other FBC related workarounds also exist, but those require fiddling
>> >with other registers that may also get programmed from other places,
>> >so we'll need to think harder what to do with those.
>> >
>> >Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
>> >---
>> > .../gpu/drm/i915/display/intel_display_wa.c   |  8 -----
>> > drivers/gpu/drm/i915/display/intel_fbc.c      | 32 ++++++++++++++++--
>> > drivers/gpu/drm/i915/intel_clock_gating.c     | 33 -------------------
>> > 3 files changed, 29 insertions(+), 44 deletions(-)
>> >
>> >diff --git a/drivers/gpu/drm/i915/display/intel_display_wa.c b/drivers/gpu/drm/i915/display/intel_display_wa.c
>> >index ac136fd992ba..e5a8022db664 100644
>> >--- a/drivers/gpu/drm/i915/display/intel_display_wa.c
>> >+++ b/drivers/gpu/drm/i915/display/intel_display_wa.c
>> >@@ -10,20 +10,12 @@
>> > 
>> > static void gen11_display_wa_apply(struct drm_i915_private *i915)
>> > {
>> >-        /* Wa_1409120013 */
>> >-        intel_de_write(i915, ILK_DPFC_CHICKEN(INTEL_FBC_A),
>> >-                       DPFC_CHICKEN_COMP_DUMMY_PIXEL);
>> >-
>> >         /* Wa_14010594013 */
>> >         intel_de_rmw(i915, GEN8_CHICKEN_DCPR_1, 0, ICL_DELAY_PMRSP);
>> > }
>> > 
>> > static void xe_d_display_wa_apply(struct drm_i915_private *i915)
>> > {
>> >-        /* Wa_1409120013 */
>> >-        intel_de_write(i915, ILK_DPFC_CHICKEN(INTEL_FBC_A),
>> >-                       DPFC_CHICKEN_COMP_DUMMY_PIXEL);
>> >-
>> >         /* Wa_14013723622 */
>> >         intel_de_rmw(i915, CLKREQ_POLICY, CLKREQ_POLICY_MEM_UP_OVRD, 0);
>> > }
>> >diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
>> >index f17a1afb4929..1a2d4d91a85f 100644
>> >--- a/drivers/gpu/drm/i915/display/intel_fbc.c
>> >+++ b/drivers/gpu/drm/i915/display/intel_fbc.c
>> >@@ -826,10 +826,36 @@ static void intel_fbc_program_cfb(struct intel_fbc *fbc)
>> > 
>> > static void intel_fbc_program_workarounds(struct intel_fbc *fbc)
>> > {
>> >+        struct drm_i915_private *i915 = fbc->i915;
>> >+
>> >+        if (IS_SKYLAKE(i915) || IS_BROXTON(i915)) {
>> >+                /*
>> >+                 * WaFbcHighMemBwCorruptionAvoidance:skl,bxt
>> >+                 * Display WA #0883: skl,bxt
>> >+                 */
>> >+                intel_de_rmw(i915, ILK_DPFC_CHICKEN(fbc->id),
>> >+                             0, DPFC_DISABLE_DUMMY0);
>> >+        }
>> >+
>> >+        if (IS_SKYLAKE(i915) || IS_KABYLAKE(i915) ||
>> >+            IS_COFFEELAKE(i915) || IS_COMETLAKE(i915)) {
>> >+                /*
>> >+                 * WaFbcNukeOnHostModify:skl,kbl,cfl
>> >+                 * Display WA #0873: skl,kbl,cfl
>> >+                 */
>> >+                intel_de_rmw(i915, ILK_DPFC_CHICKEN(fbc->id),
>> >+                             0, DPFC_NUKE_ON_ANY_MODIFICATION);
>> >+        }
>> >+
>> >+        /* Wa_1409120013:icl,jsl,tgl,dg1 */
>> >+        if (IS_DISPLAY_VER(i915, 11, 12))
>> >+                intel_de_rmw(i915, ILK_DPFC_CHICKEN(fbc->id),
>> >+                             0, DPFC_CHICKEN_COMP_DUMMY_PIXEL);
>> >+
>> >         /* Wa_22014263786:icl,jsl,tgl,dg1,rkl,adls,adlp,mtl */
>> >-        if (DISPLAY_VER(fbc->i915) >= 11 && !IS_DG2(fbc->i915))
>> >-                intel_de_rmw(fbc->i915, ILK_DPFC_CHICKEN(fbc->id), 0,
>> >-                             DPFC_CHICKEN_FORCE_SLB_INVALIDATION);
>> >+        if (DISPLAY_VER(i915) >= 11 && !IS_DG2(i915))
>> >+                intel_de_rmw(i915, ILK_DPFC_CHICKEN(fbc->id),
>> >+                             0, DPFC_CHICKEN_FORCE_SLB_INVALIDATION);
>> 
>> Since we are writing to the same register, maybe we could have a single read,
>> modify and write instead of multiple rmw calls?
>
>Perhaps. Although we do at most do two rmws here on any given system.
>So it's not particularly expensive to keep it simple like this.

Well, I think having a single rmw would not include too much complexity,
since we would only need keep track of a single set_bits variable and
use it at the end.

Anyways, the changes look correct here. So, with or without the
suggestion:

Reviewed-by: Gustavo Sousa <gustavo.sousa at intel.com>

>
>I was also pondering about splitting this into vfuncs, which would
>need multiple rmws anyway. But the whole thing is a bit of a mess
>in terms of which platforms need what, so not sure it's make it
>look any nicer.
>
>> 
>> --
>> Gustavo Sousa
>> 
>> > }
>> > 
>> > static void __intel_fbc_cleanup_cfb(struct intel_fbc *fbc)
>> >diff --git a/drivers/gpu/drm/i915/intel_clock_gating.c b/drivers/gpu/drm/i915/intel_clock_gating.c
>> >index 9c21ce69bd98..39f23288e8a8 100644
>> >--- a/drivers/gpu/drm/i915/intel_clock_gating.c
>> >+++ b/drivers/gpu/drm/i915/intel_clock_gating.c
>> >@@ -105,12 +105,6 @@ static void bxt_init_clock_gating(struct drm_i915_private *i915)
>> >          * Display WA #0562: bxt
>> >          */
>> >         intel_uncore_rmw(&i915->uncore, DISP_ARB_CTL, 0, DISP_FBC_WM_DIS);
>> >-
>> >-        /*
>> >-         * WaFbcHighMemBwCorruptionAvoidance:bxt
>> >-         * Display WA #0883: bxt
>> >-         */
>> >-        intel_uncore_rmw(&i915->uncore, ILK_DPFC_CHICKEN(INTEL_FBC_A), 0, DPFC_DISABLE_DUMMY0);
>> > }
>> > 
>> > static void glk_init_clock_gating(struct drm_i915_private *i915)
>> >@@ -396,13 +390,6 @@ static void cfl_init_clock_gating(struct drm_i915_private *i915)
>> >          * Display WA #0562: cfl
>> >          */
>> >         intel_uncore_rmw(&i915->uncore, DISP_ARB_CTL, 0, DISP_FBC_WM_DIS);
>> >-
>> >-        /*
>> >-         * WaFbcNukeOnHostModify:cfl
>> >-         * Display WA #0873: cfl
>> >-         */
>> >-        intel_uncore_rmw(&i915->uncore, ILK_DPFC_CHICKEN(INTEL_FBC_A),
>> >-                         0, DPFC_NUKE_ON_ANY_MODIFICATION);
>> > }
>> > 
>> > static void kbl_init_clock_gating(struct drm_i915_private *i915)
>> >@@ -427,13 +414,6 @@ static void kbl_init_clock_gating(struct drm_i915_private *i915)
>> >          * Display WA #0562: kbl
>> >          */
>> >         intel_uncore_rmw(&i915->uncore, DISP_ARB_CTL, 0, DISP_FBC_WM_DIS);
>> >-
>> >-        /*
>> >-         * WaFbcNukeOnHostModify:kbl
>> >-         * Display WA #0873: kbl
>> >-         */
>> >-        intel_uncore_rmw(&i915->uncore, ILK_DPFC_CHICKEN(INTEL_FBC_A),
>> >-                         0, DPFC_NUKE_ON_ANY_MODIFICATION);
>> > }
>> > 
>> > static void skl_init_clock_gating(struct drm_i915_private *i915)
>> >@@ -452,19 +432,6 @@ static void skl_init_clock_gating(struct drm_i915_private *i915)
>> >          * Display WA #0562: skl
>> >          */
>> >         intel_uncore_rmw(&i915->uncore, DISP_ARB_CTL, 0, DISP_FBC_WM_DIS);
>> >-
>> >-        /*
>> >-         * WaFbcNukeOnHostModify:skl
>> >-         * Display WA #0873: skl
>> >-         */
>> >-        intel_uncore_rmw(&i915->uncore, ILK_DPFC_CHICKEN(INTEL_FBC_A),
>> >-                         0, DPFC_NUKE_ON_ANY_MODIFICATION);
>> >-
>> >-        /*
>> >-         * WaFbcHighMemBwCorruptionAvoidance:skl
>> >-         * Display WA #0883: skl
>> >-         */
>> >-        intel_uncore_rmw(&i915->uncore, ILK_DPFC_CHICKEN(INTEL_FBC_A), 0, DPFC_DISABLE_DUMMY0);
>> > }
>> > 
>> > static void bdw_init_clock_gating(struct drm_i915_private *i915)
>> >-- 
>> >2.43.0
>> >
>
>-- 
>Ville Syrjälä
>Intel


More information about the Intel-gfx mailing list