diff mbox series

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

Message ID 20240123090051.29818-2-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series [1/2] drm/i915/fbc: Don't use a fence for a plane if FBC is not possible | expand

Commit Message

Ville Syrjälä Jan. 23, 2024, 9 a.m. UTC 
From: Ville Syrjälä <ville.syrjala@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@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(-)

Comments

Gustavo Sousa Jan. 23, 2024, 1:42 p.m. UTC | #1 
Quoting Ville Syrjala (2024-01-23 06:00:51-03:00)
>From: Ville Syrjälä <ville.syrjala@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@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?

--
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ä Jan. 26, 2024, 10:26 a.m. UTC | #2 
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@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@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.

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
> >
Gustavo Sousa Jan. 30, 2024, 7:31 p.m. UTC | #3 
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@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@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@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
diff mbox series

Patch

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);
 }
 
 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)