From: Ville Syrjälä ville.syrjala@linux.intel.com
Looks like skl/bxt/derivatives also need the plane stride stretch w/a when using async flips and VT-d is enabled, or else we get corruption on screen. To my surprise this was even documented in bspec, but only as a note on the CHICHKEN_PIPESL register description rather than on the w/a list.
So very much the same thing as on HSW/BDW, except the bits moved yet again.
Cc: stable@vger.kernel.org Cc: Karthik B S karthik.b.s@intel.com Fixes: 55ea1cb178ef ("drm/i915: Enable async flips in i915") Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/i915/i915_reg.h | 5 +++++ drivers/gpu/drm/i915/intel_pm.c | 12 ++++++++++++ 2 files changed, 17 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 3a20a55d2512..29f6bfc2002d 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -8222,6 +8222,11 @@ enum { #define HSW_SPR_STRETCH_MAX_X1 REG_FIELD_PREP(HSW_SPR_STRETCH_MAX_MASK, 3) #define HSW_FBCQ_DIS (1 << 22) #define BDW_DPRS_MASK_VBLANK_SRD (1 << 0) +#define SKL_PLANE1_STRETCH_MAX_MASK REG_GENMASK(1, 0) +#define SKL_PLANE1_STRETCH_MAX_X8 REG_FIELD_PREP(SKL_PLANE1_STRETCH_MAX_MASK, 0) +#define SKL_PLANE1_STRETCH_MAX_X4 REG_FIELD_PREP(SKL_PLANE1_STRETCH_MAX_MASK, 1) +#define SKL_PLANE1_STRETCH_MAX_X2 REG_FIELD_PREP(SKL_PLANE1_STRETCH_MAX_MASK, 2) +#define SKL_PLANE1_STRETCH_MAX_X1 REG_FIELD_PREP(SKL_PLANE1_STRETCH_MAX_MASK, 3) #define CHICKEN_PIPESL_1(pipe) _MMIO_PIPE(pipe, _CHICKEN_PIPESL_1_A, _CHICKEN_PIPESL_1_B)
#define _CHICKEN_TRANS_A 0x420c0 diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index ef5f73934dab..74d4620a4999 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -76,6 +76,8 @@ struct intel_wm_config {
static void gen9_init_clock_gating(struct drm_i915_private *dev_priv) { + enum pipe pipe; + if (HAS_LLC(dev_priv)) { /* * WaCompressedResourceDisplayNewHashMode:skl,kbl @@ -89,6 +91,16 @@ static void gen9_init_clock_gating(struct drm_i915_private *dev_priv) SKL_DE_COMPRESSED_HASH_MODE); }
+ for_each_pipe(dev_priv, pipe) { + /* + * "Plane N strech max must be programmed to 11b (x1) + * when Async flips are enabled on that plane." + */ + if (!IS_GEMINILAKE(dev_priv) && intel_vtd_active()) + intel_uncore_rmw(&dev_priv->uncore, CHICKEN_PIPESL_1(pipe), + SKL_PLANE1_STRETCH_MAX_MASK, SKL_PLANE1_STRETCH_MAX_X1); + } + /* See Bspec note for PSR2_CTL bit 31, Wa#828:skl,bxt,kbl,cfl */ intel_uncore_write(&dev_priv->uncore, CHICKEN_PAR1_1, intel_uncore_read(&dev_priv->uncore, CHICKEN_PAR1_1) | SKL_EDP_PSR_FIX_RDWRAP);
On Thu, Sep 30, 2021 at 10:09:42PM +0300, Ville Syrjala wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
Looks like skl/bxt/derivatives also need the plane stride stretch w/a when using async flips and VT-d is enabled, or else we get corruption on screen. To my surprise this was even documented in bspec, but only as a note on the CHICHKEN_PIPESL register description rather than on the w/a list.
So very much the same thing as on HSW/BDW, except the bits moved yet again.
Bspec 7522 doesn't say anything about this requirement being tied to VT-d on these platforms. Should we drop the intel_vtd_active() condition to be safe?
Matt
Cc: stable@vger.kernel.org Cc: Karthik B S karthik.b.s@intel.com Fixes: 55ea1cb178ef ("drm/i915: Enable async flips in i915") Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
drivers/gpu/drm/i915/i915_reg.h | 5 +++++ drivers/gpu/drm/i915/intel_pm.c | 12 ++++++++++++ 2 files changed, 17 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 3a20a55d2512..29f6bfc2002d 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -8222,6 +8222,11 @@ enum { #define HSW_SPR_STRETCH_MAX_X1 REG_FIELD_PREP(HSW_SPR_STRETCH_MAX_MASK, 3) #define HSW_FBCQ_DIS (1 << 22) #define BDW_DPRS_MASK_VBLANK_SRD (1 << 0) +#define SKL_PLANE1_STRETCH_MAX_MASK REG_GENMASK(1, 0) +#define SKL_PLANE1_STRETCH_MAX_X8 REG_FIELD_PREP(SKL_PLANE1_STRETCH_MAX_MASK, 0) +#define SKL_PLANE1_STRETCH_MAX_X4 REG_FIELD_PREP(SKL_PLANE1_STRETCH_MAX_MASK, 1) +#define SKL_PLANE1_STRETCH_MAX_X2 REG_FIELD_PREP(SKL_PLANE1_STRETCH_MAX_MASK, 2) +#define SKL_PLANE1_STRETCH_MAX_X1 REG_FIELD_PREP(SKL_PLANE1_STRETCH_MAX_MASK, 3) #define CHICKEN_PIPESL_1(pipe) _MMIO_PIPE(pipe, _CHICKEN_PIPESL_1_A, _CHICKEN_PIPESL_1_B) #define _CHICKEN_TRANS_A 0x420c0 diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index ef5f73934dab..74d4620a4999 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -76,6 +76,8 @@ struct intel_wm_config { static void gen9_init_clock_gating(struct drm_i915_private *dev_priv) {
- enum pipe pipe;
- if (HAS_LLC(dev_priv)) { /*
- WaCompressedResourceDisplayNewHashMode:skl,kbl
@@ -89,6 +91,16 @@ static void gen9_init_clock_gating(struct drm_i915_private *dev_priv) SKL_DE_COMPRESSED_HASH_MODE); }
- for_each_pipe(dev_priv, pipe) {
/** "Plane N strech max must be programmed to 11b (x1)* when Async flips are enabled on that plane."*/if (!IS_GEMINILAKE(dev_priv) && intel_vtd_active())intel_uncore_rmw(&dev_priv->uncore, CHICKEN_PIPESL_1(pipe),SKL_PLANE1_STRETCH_MAX_MASK, SKL_PLANE1_STRETCH_MAX_X1);- }
- /* See Bspec note for PSR2_CTL bit 31, Wa#828:skl,bxt,kbl,cfl */ intel_uncore_write(&dev_priv->uncore, CHICKEN_PAR1_1, intel_uncore_read(&dev_priv->uncore, CHICKEN_PAR1_1) | SKL_EDP_PSR_FIX_RDWRAP);
-- 2.32.0
On Fri, Oct 01, 2021 at 02:08:15PM -0700, Matt Roper wrote:
On Thu, Sep 30, 2021 at 10:09:42PM +0300, Ville Syrjala wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
Looks like skl/bxt/derivatives also need the plane stride stretch w/a when using async flips and VT-d is enabled, or else we get corruption on screen. To my surprise this was even documented in bspec, but only as a note on the CHICHKEN_PIPESL register description rather than on the w/a list.
So very much the same thing as on HSW/BDW, except the bits moved yet again.
Bspec 7522 doesn't say anything about this requirement being tied to VT-d on these platforms. Should we drop the intel_vtd_active() condition to be safe?
I think it's just an oversight in bspec. I read through the hsd and IIRC it did specify that it's VT-d only. Also real life confirms it. No problems whatsoever when VT-d is disabled.
Matt
Cc: stable@vger.kernel.org Cc: Karthik B S karthik.b.s@intel.com Fixes: 55ea1cb178ef ("drm/i915: Enable async flips in i915") Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
drivers/gpu/drm/i915/i915_reg.h | 5 +++++ drivers/gpu/drm/i915/intel_pm.c | 12 ++++++++++++ 2 files changed, 17 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 3a20a55d2512..29f6bfc2002d 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -8222,6 +8222,11 @@ enum { #define HSW_SPR_STRETCH_MAX_X1 REG_FIELD_PREP(HSW_SPR_STRETCH_MAX_MASK, 3) #define HSW_FBCQ_DIS (1 << 22) #define BDW_DPRS_MASK_VBLANK_SRD (1 << 0) +#define SKL_PLANE1_STRETCH_MAX_MASK REG_GENMASK(1, 0) +#define SKL_PLANE1_STRETCH_MAX_X8 REG_FIELD_PREP(SKL_PLANE1_STRETCH_MAX_MASK, 0) +#define SKL_PLANE1_STRETCH_MAX_X4 REG_FIELD_PREP(SKL_PLANE1_STRETCH_MAX_MASK, 1) +#define SKL_PLANE1_STRETCH_MAX_X2 REG_FIELD_PREP(SKL_PLANE1_STRETCH_MAX_MASK, 2) +#define SKL_PLANE1_STRETCH_MAX_X1 REG_FIELD_PREP(SKL_PLANE1_STRETCH_MAX_MASK, 3) #define CHICKEN_PIPESL_1(pipe) _MMIO_PIPE(pipe, _CHICKEN_PIPESL_1_A, _CHICKEN_PIPESL_1_B) #define _CHICKEN_TRANS_A 0x420c0 diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index ef5f73934dab..74d4620a4999 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -76,6 +76,8 @@ struct intel_wm_config { static void gen9_init_clock_gating(struct drm_i915_private *dev_priv) {
- enum pipe pipe;
- if (HAS_LLC(dev_priv)) { /*
- WaCompressedResourceDisplayNewHashMode:skl,kbl
@@ -89,6 +91,16 @@ static void gen9_init_clock_gating(struct drm_i915_private *dev_priv) SKL_DE_COMPRESSED_HASH_MODE); }
- for_each_pipe(dev_priv, pipe) {
/** "Plane N strech max must be programmed to 11b (x1)* when Async flips are enabled on that plane."*/if (!IS_GEMINILAKE(dev_priv) && intel_vtd_active())intel_uncore_rmw(&dev_priv->uncore, CHICKEN_PIPESL_1(pipe),SKL_PLANE1_STRETCH_MAX_MASK, SKL_PLANE1_STRETCH_MAX_X1);- }
- /* See Bspec note for PSR2_CTL bit 31, Wa#828:skl,bxt,kbl,cfl */ intel_uncore_write(&dev_priv->uncore, CHICKEN_PAR1_1, intel_uncore_read(&dev_priv->uncore, CHICKEN_PAR1_1) | SKL_EDP_PSR_FIX_RDWRAP);
-- 2.32.0
-- Matt Roper Graphics Software Engineer VTT-OSGC Platform Enablement Intel Corporation (916) 356-2795
On Sat, Oct 02, 2021 at 01:01:31AM +0300, Ville Syrjälä wrote:
On Fri, Oct 01, 2021 at 02:08:15PM -0700, Matt Roper wrote:
On Thu, Sep 30, 2021 at 10:09:42PM +0300, Ville Syrjala wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
Looks like skl/bxt/derivatives also need the plane stride stretch w/a when using async flips and VT-d is enabled, or else we get corruption on screen. To my surprise this was even documented in bspec, but only as a note on the CHICHKEN_PIPESL register description rather than on the w/a list.
So very much the same thing as on HSW/BDW, except the bits moved yet again.
Bspec 7522 doesn't say anything about this requirement being tied to VT-d on these platforms. Should we drop the intel_vtd_active() condition to be safe?
I think it's just an oversight in bspec. I read through the hsd and IIRC it did specify that it's VT-d only. Also real life confirms it. No problems whatsoever when VT-d is disabled.
BTW I was hopeful this would fix shard-skl but no such luck. Well, in fact it does fix the crc error, indicating the patch does work. Unfortunately those systems have yet another undiagnosed async flip problem. From the ci report on this series I can see that the machine was only capable of ~1.2 async flips per frame during the crc test. I guess technically anything >1 counts as "some async flips did happen" but it really should not be that low (I put the arbitrary limit in the test at two flips per frame). My cfl can do IIRC 50-150 per frame, depending on the phase of the moon and whatnot.
On Sat, Oct 02, 2021 at 01:01:31AM +0300, Ville Syrjälä wrote:
On Fri, Oct 01, 2021 at 02:08:15PM -0700, Matt Roper wrote:
On Thu, Sep 30, 2021 at 10:09:42PM +0300, Ville Syrjala wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
Looks like skl/bxt/derivatives also need the plane stride stretch w/a when using async flips and VT-d is enabled, or else we get corruption on screen. To my surprise this was even documented in bspec, but only as a note on the CHICHKEN_PIPESL register description rather than on the w/a list.
So very much the same thing as on HSW/BDW, except the bits moved yet again.
Bspec 7522 doesn't say anything about this requirement being tied to VT-d on these platforms. Should we drop the intel_vtd_active() condition to be safe?
I think it's just an oversight in bspec. I read through the hsd and IIRC it did specify that it's VT-d only. Also real life confirms it. No problems whatsoever when VT-d is disabled.
I notice there are additional bits that we should set to apply this workaround to planes 2, 3, and 4, but since i915 still artificially limits async flips to just the primary plane, only programming bits 1:0 should be fine for now; we'll just need to remember to extend this workaround if we do start allowing async flips on other planes in the future.
Reviewed-by: Matt Roper matthew.d.roper@intel.com
Matt
Cc: stable@vger.kernel.org Cc: Karthik B S karthik.b.s@intel.com Fixes: 55ea1cb178ef ("drm/i915: Enable async flips in i915") Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com
drivers/gpu/drm/i915/i915_reg.h | 5 +++++ drivers/gpu/drm/i915/intel_pm.c | 12 ++++++++++++ 2 files changed, 17 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 3a20a55d2512..29f6bfc2002d 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -8222,6 +8222,11 @@ enum { #define HSW_SPR_STRETCH_MAX_X1 REG_FIELD_PREP(HSW_SPR_STRETCH_MAX_MASK, 3) #define HSW_FBCQ_DIS (1 << 22) #define BDW_DPRS_MASK_VBLANK_SRD (1 << 0) +#define SKL_PLANE1_STRETCH_MAX_MASK REG_GENMASK(1, 0) +#define SKL_PLANE1_STRETCH_MAX_X8 REG_FIELD_PREP(SKL_PLANE1_STRETCH_MAX_MASK, 0) +#define SKL_PLANE1_STRETCH_MAX_X4 REG_FIELD_PREP(SKL_PLANE1_STRETCH_MAX_MASK, 1) +#define SKL_PLANE1_STRETCH_MAX_X2 REG_FIELD_PREP(SKL_PLANE1_STRETCH_MAX_MASK, 2) +#define SKL_PLANE1_STRETCH_MAX_X1 REG_FIELD_PREP(SKL_PLANE1_STRETCH_MAX_MASK, 3) #define CHICKEN_PIPESL_1(pipe) _MMIO_PIPE(pipe, _CHICKEN_PIPESL_1_A, _CHICKEN_PIPESL_1_B) #define _CHICKEN_TRANS_A 0x420c0 diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index ef5f73934dab..74d4620a4999 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -76,6 +76,8 @@ struct intel_wm_config { static void gen9_init_clock_gating(struct drm_i915_private *dev_priv) {
- enum pipe pipe;
- if (HAS_LLC(dev_priv)) { /*
- WaCompressedResourceDisplayNewHashMode:skl,kbl
@@ -89,6 +91,16 @@ static void gen9_init_clock_gating(struct drm_i915_private *dev_priv) SKL_DE_COMPRESSED_HASH_MODE); }
- for_each_pipe(dev_priv, pipe) {
/** "Plane N strech max must be programmed to 11b (x1)* when Async flips are enabled on that plane."*/if (!IS_GEMINILAKE(dev_priv) && intel_vtd_active())intel_uncore_rmw(&dev_priv->uncore, CHICKEN_PIPESL_1(pipe),SKL_PLANE1_STRETCH_MAX_MASK, SKL_PLANE1_STRETCH_MAX_X1);- }
- /* See Bspec note for PSR2_CTL bit 31, Wa#828:skl,bxt,kbl,cfl */ intel_uncore_write(&dev_priv->uncore, CHICKEN_PAR1_1, intel_uncore_read(&dev_priv->uncore, CHICKEN_PAR1_1) | SKL_EDP_PSR_FIX_RDWRAP);
-- 2.32.0
-- Matt Roper Graphics Software Engineer VTT-OSGC Platform Enablement Intel Corporation (916) 356-2795
-- Ville Syrjälä Intel
On Mon, Oct 04, 2021 at 10:50:00AM -0700, Matt Roper wrote:
On Sat, Oct 02, 2021 at 01:01:31AM +0300, Ville Syrjälä wrote:
On Fri, Oct 01, 2021 at 02:08:15PM -0700, Matt Roper wrote:
On Thu, Sep 30, 2021 at 10:09:42PM +0300, Ville Syrjala wrote:
From: Ville Syrjälä ville.syrjala@linux.intel.com
Looks like skl/bxt/derivatives also need the plane stride stretch w/a when using async flips and VT-d is enabled, or else we get corruption on screen. To my surprise this was even documented in bspec, but only as a note on the CHICHKEN_PIPESL register description rather than on the w/a list.
So very much the same thing as on HSW/BDW, except the bits moved yet again.
Bspec 7522 doesn't say anything about this requirement being tied to VT-d on these platforms. Should we drop the intel_vtd_active() condition to be safe?
I think it's just an oversight in bspec. I read through the hsd and IIRC it did specify that it's VT-d only. Also real life confirms it. No problems whatsoever when VT-d is disabled.
I notice there are additional bits that we should set to apply this workaround to planes 2, 3, and 4, but since i915 still artificially limits async flips to just the primary plane, only programming bits 1:0 should be fine for now; we'll just need to remember to extend this workaround if we do start allowing async flips on other planes in the future.
Aye. gen8_de_pipe_flip_done_mask() is the other place where we still hardcode this for plane 1 only. I think the rest of the code I did end up making more or less plane agnostic already.
I was considering at least parametrizing the register defines but then I got a nagging feeling that I once ran into some issues while trying to stick non-constant numbers into REG_BIT & co. So I decided to hardcode plane 1 for the moment.
Reviewed-by: Matt Roper matthew.d.roper@intel.com
Thanks. Pushed.
linux-stable-mirror@lists.linaro.org
-
Matt Roper -
Ville Syrjala -
Ville Syrjälä