| Message ID | 20210325093213.20794-1-anshuman.gupta@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Tweaked Wa_14010685332 for all PCHs | expand |
On Thu, Mar 25, 2021 at 03:02:13PM +0530, Anshuman Gupta wrote: > dispcnlunit1_cp_xosc_clkreq clock observed to be active on TGL-H platform > despite Wa_14010685332 original sequence thus blocks entry to deeper s0ix state. > > The Tweaked Wa_14010685332 sequence fixes this issue, therefore use tweaked > Wa_14010685332 sequence for every PCH since PCH_CNP. > > Fixes: b896898c7369 ("drm/i915: Tweaked Wa_14010685332 for PCHs used on gen11 platforms") > Cc: Matt Roper <matthew.d.roper@intel.com> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com> > --- > .../drm/i915/display/intel_display_power.c | 18 +++++++++------- > drivers/gpu/drm/i915/i915_irq.c | 21 ------------------- > 2 files changed, 10 insertions(+), 29 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c > index 7e0eaa872350..4e970be36487 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_power.c > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c > @@ -5910,13 +5910,14 @@ void intel_display_power_suspend_late(struct drm_i915_private *i915) > { > if (INTEL_GEN(i915) >= 11 || IS_GEN9_LP(i915)) { > bxt_enable_dc9(i915); > - /* Tweaked Wa_14010685332:icp,jsp,mcc */ > - if (INTEL_PCH_TYPE(i915) >= PCH_ICP && INTEL_PCH_TYPE(i915) <= PCH_MCC) > - intel_de_rmw(i915, SOUTH_CHICKEN1, > - SBCLK_RUN_REFCLK_DIS, SBCLK_RUN_REFCLK_DIS); > } else if (IS_HASWELL(i915) || IS_BROADWELL(i915)) { > hsw_enable_pc8(i915); > } > + > + /* Tweaked Wa_14010685332:cnp,icp,jsp,mcc,tgp,rkl,adp */ why are you adding "rkl"? I don't like mixing gpu with pch names... > + if (INTEL_PCH_TYPE(i915) == PCH_CNP || > + (INTEL_PCH_TYPE(i915) >= PCH_ICP && INTEL_PCH_TYPE(i915) < PCH_DG1)) why can't we simply use if (INTEL_PCH_TYPE(i915) >= PCH_CNP) ? > + intel_de_rmw(i915, SOUTH_CHICKEN1, SBCLK_RUN_REFCLK_DIS, SBCLK_RUN_REFCLK_DIS); > } > > void intel_display_power_resume_early(struct drm_i915_private *i915) > @@ -5924,13 +5925,14 @@ void intel_display_power_resume_early(struct drm_i915_private *i915) > if (INTEL_GEN(i915) >= 11 || IS_GEN9_LP(i915)) { > gen9_sanitize_dc_state(i915); > bxt_disable_dc9(i915); > - /* Tweaked Wa_14010685332:icp,jsp,mcc */ > - if (INTEL_PCH_TYPE(i915) >= PCH_ICP && INTEL_PCH_TYPE(i915) <= PCH_MCC) > - intel_de_rmw(i915, SOUTH_CHICKEN1, SBCLK_RUN_REFCLK_DIS, 0); > - > } else if (IS_HASWELL(i915) || IS_BROADWELL(i915)) { > hsw_disable_pc8(i915); > } > + > + /* Tweaked Wa_14010685332:cnp,icp,jsp,mcc,tgp,rkl,adp */ > + if (INTEL_PCH_TYPE(i915) == PCH_CNP || > + (INTEL_PCH_TYPE(i915) >= PCH_ICP && INTEL_PCH_TYPE(i915) < PCH_DG1)) > + intel_de_rmw(i915, SOUTH_CHICKEN1, SBCLK_RUN_REFCLK_DIS, 0); > } > > void intel_display_power_suspend(struct drm_i915_private *i915) > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index 44aed4cbf894..8abcd35df926 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -3040,24 +3040,6 @@ static void valleyview_irq_reset(struct drm_i915_private *dev_priv) > spin_unlock_irq(&dev_priv->irq_lock); > } > > -static void cnp_display_clock_wa(struct drm_i915_private *dev_priv) > -{ > - struct intel_uncore *uncore = &dev_priv->uncore; > - > - /* > - * Wa_14010685332:cnp/cmp,tgp,adp > - * TODO: Clarify which platforms this applies to > - * TODO: Figure out if this workaround can be applied in the s0ix suspend/resume handlers as > - * on earlier platforms and whether the workaround is also needed for runtime suspend/resume > - */ > - if (INTEL_PCH_TYPE(dev_priv) == PCH_CNP || > - (INTEL_PCH_TYPE(dev_priv) >= PCH_TGP && INTEL_PCH_TYPE(dev_priv) < PCH_DG1)) { > - intel_uncore_rmw(uncore, SOUTH_CHICKEN1, SBCLK_RUN_REFCLK_DIS, > - SBCLK_RUN_REFCLK_DIS); > - intel_uncore_rmw(uncore, SOUTH_CHICKEN1, SBCLK_RUN_REFCLK_DIS, 0); > - } > -} > - > static void gen8_irq_reset(struct drm_i915_private *dev_priv) > { > struct intel_uncore *uncore = &dev_priv->uncore; > @@ -3082,7 +3064,6 @@ static void gen8_irq_reset(struct drm_i915_private *dev_priv) > if (HAS_PCH_SPLIT(dev_priv)) > ibx_irq_reset(dev_priv); > > - cnp_display_clock_wa(dev_priv); > } > > static void gen11_display_irq_reset(struct drm_i915_private *dev_priv) > @@ -3123,8 +3104,6 @@ static void gen11_display_irq_reset(struct drm_i915_private *dev_priv) > > if (INTEL_PCH_TYPE(dev_priv) >= PCH_ICP) > GEN3_IRQ_RESET(uncore, SDE); > - > - cnp_display_clock_wa(dev_priv); > } > > static void gen11_irq_reset(struct drm_i915_private *dev_priv) > -- > 2.26.2 >
On Thu, Mar 25, 2021 at 05:39:47PM +0530, Anshuman Gupta wrote: > dispcnlunit1_cp_xosc_clkreq clock observed to be active on TGL-H platform > despite Wa_14010685332 original sequence, thus blocks entry to deeper s0ix state. > > The Tweaked Wa_14010685332 sequence fixes this issue, therefore use tweaked > Wa_14010685332 sequence for every PCH since PCH_CNP. > > v2: > - removed RKL from comment and simplified condition. [Rodrigo] Hi, I didn't see this patch shown on any trees yet. May I know the current state of this patch? Thanks. > > Fixes: b896898c7369 ("drm/i915: Tweaked Wa_14010685332 for PCHs used on gen11 platforms") > Cc: Matt Roper <matthew.d.roper@intel.com> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > Cc: Imre Deak <imre.deak@intel.com> > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com> > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > --- > .../drm/i915/display/intel_display_power.c | 16 +++++++------- > drivers/gpu/drm/i915/i915_irq.c | 21 ------------------- > 2 files changed, 8 insertions(+), 29 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c > index cef177208e68..b76cc4379d5c 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_power.c > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c > @@ -5910,13 +5910,13 @@ void intel_display_power_suspend_late(struct drm_i915_private *i915) > { > if (DISPLAY_VER(i915) >= 11 || IS_GEN9_LP(i915)) { > bxt_enable_dc9(i915); > - /* Tweaked Wa_14010685332:icp,jsp,mcc */ > - if (INTEL_PCH_TYPE(i915) >= PCH_ICP && INTEL_PCH_TYPE(i915) <= PCH_MCC) > - intel_de_rmw(i915, SOUTH_CHICKEN1, > - SBCLK_RUN_REFCLK_DIS, SBCLK_RUN_REFCLK_DIS); > } else if (IS_HASWELL(i915) || IS_BROADWELL(i915)) { > hsw_enable_pc8(i915); > } > + > + /* Tweaked Wa_14010685332:cnp,icp,jsp,mcc,tgp,adp */ > + if (INTEL_PCH_TYPE(i915) >= PCH_CNP && INTEL_PCH_TYPE(i915) < PCH_DG1) > + intel_de_rmw(i915, SOUTH_CHICKEN1, SBCLK_RUN_REFCLK_DIS, SBCLK_RUN_REFCLK_DIS); > } > > void intel_display_power_resume_early(struct drm_i915_private *i915) > @@ -5924,13 +5924,13 @@ void intel_display_power_resume_early(struct drm_i915_private *i915) > if (DISPLAY_VER(i915) >= 11 || IS_GEN9_LP(i915)) { > gen9_sanitize_dc_state(i915); > bxt_disable_dc9(i915); > - /* Tweaked Wa_14010685332:icp,jsp,mcc */ > - if (INTEL_PCH_TYPE(i915) >= PCH_ICP && INTEL_PCH_TYPE(i915) <= PCH_MCC) > - intel_de_rmw(i915, SOUTH_CHICKEN1, SBCLK_RUN_REFCLK_DIS, 0); > - > } else if (IS_HASWELL(i915) || IS_BROADWELL(i915)) { > hsw_disable_pc8(i915); > } > + > + /* Tweaked Wa_14010685332:cnp,icp,jsp,mcc,tgp,adp */ > + if (INTEL_PCH_TYPE(i915) >= PCH_CNP && INTEL_PCH_TYPE(i915) < PCH_DG1) > + intel_de_rmw(i915, SOUTH_CHICKEN1, SBCLK_RUN_REFCLK_DIS, 0); > } > > void intel_display_power_suspend(struct drm_i915_private *i915) > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index 7eefbdec25a2..4547ba2f19b2 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -3040,24 +3040,6 @@ static void valleyview_irq_reset(struct drm_i915_private *dev_priv) > spin_unlock_irq(&dev_priv->irq_lock); > } > > -static void cnp_display_clock_wa(struct drm_i915_private *dev_priv) > -{ > - struct intel_uncore *uncore = &dev_priv->uncore; > - > - /* > - * Wa_14010685332:cnp/cmp,tgp,adp > - * TODO: Clarify which platforms this applies to > - * TODO: Figure out if this workaround can be applied in the s0ix suspend/resume handlers as > - * on earlier platforms and whether the workaround is also needed for runtime suspend/resume > - */ > - if (INTEL_PCH_TYPE(dev_priv) == PCH_CNP || > - (INTEL_PCH_TYPE(dev_priv) >= PCH_TGP && INTEL_PCH_TYPE(dev_priv) < PCH_DG1)) { > - intel_uncore_rmw(uncore, SOUTH_CHICKEN1, SBCLK_RUN_REFCLK_DIS, > - SBCLK_RUN_REFCLK_DIS); > - intel_uncore_rmw(uncore, SOUTH_CHICKEN1, SBCLK_RUN_REFCLK_DIS, 0); > - } > -} > - > static void gen8_irq_reset(struct drm_i915_private *dev_priv) > { > struct intel_uncore *uncore = &dev_priv->uncore; > @@ -3082,7 +3064,6 @@ static void gen8_irq_reset(struct drm_i915_private *dev_priv) > if (HAS_PCH_SPLIT(dev_priv)) > ibx_irq_reset(dev_priv); > > - cnp_display_clock_wa(dev_priv); > } > > static void gen11_display_irq_reset(struct drm_i915_private *dev_priv) > @@ -3123,8 +3104,6 @@ static void gen11_display_irq_reset(struct drm_i915_private *dev_priv) > > if (INTEL_PCH_TYPE(dev_priv) >= PCH_ICP) > GEN3_IRQ_RESET(uncore, SDE); > - > - cnp_display_clock_wa(dev_priv); > } > > static void gen11_irq_reset(struct drm_i915_private *dev_priv)
diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c index 7e0eaa872350..4e970be36487 100644 --- a/drivers/gpu/drm/i915/display/intel_display_power.c +++ b/drivers/gpu/drm/i915/display/intel_display_power.c @@ -5910,13 +5910,14 @@ void intel_display_power_suspend_late(struct drm_i915_private *i915) { if (INTEL_GEN(i915) >= 11 || IS_GEN9_LP(i915)) { bxt_enable_dc9(i915); - /* Tweaked Wa_14010685332:icp,jsp,mcc */ - if (INTEL_PCH_TYPE(i915) >= PCH_ICP && INTEL_PCH_TYPE(i915) <= PCH_MCC) - intel_de_rmw(i915, SOUTH_CHICKEN1, - SBCLK_RUN_REFCLK_DIS, SBCLK_RUN_REFCLK_DIS); } else if (IS_HASWELL(i915) || IS_BROADWELL(i915)) { hsw_enable_pc8(i915); } + + /* Tweaked Wa_14010685332:cnp,icp,jsp,mcc,tgp,rkl,adp */ + if (INTEL_PCH_TYPE(i915) == PCH_CNP || + (INTEL_PCH_TYPE(i915) >= PCH_ICP && INTEL_PCH_TYPE(i915) < PCH_DG1)) + intel_de_rmw(i915, SOUTH_CHICKEN1, SBCLK_RUN_REFCLK_DIS, SBCLK_RUN_REFCLK_DIS); } void intel_display_power_resume_early(struct drm_i915_private *i915) @@ -5924,13 +5925,14 @@ void intel_display_power_resume_early(struct drm_i915_private *i915) if (INTEL_GEN(i915) >= 11 || IS_GEN9_LP(i915)) { gen9_sanitize_dc_state(i915); bxt_disable_dc9(i915); - /* Tweaked Wa_14010685332:icp,jsp,mcc */ - if (INTEL_PCH_TYPE(i915) >= PCH_ICP && INTEL_PCH_TYPE(i915) <= PCH_MCC) - intel_de_rmw(i915, SOUTH_CHICKEN1, SBCLK_RUN_REFCLK_DIS, 0); - } else if (IS_HASWELL(i915) || IS_BROADWELL(i915)) { hsw_disable_pc8(i915); } + + /* Tweaked Wa_14010685332:cnp,icp,jsp,mcc,tgp,rkl,adp */ + if (INTEL_PCH_TYPE(i915) == PCH_CNP || + (INTEL_PCH_TYPE(i915) >= PCH_ICP && INTEL_PCH_TYPE(i915) < PCH_DG1)) + intel_de_rmw(i915, SOUTH_CHICKEN1, SBCLK_RUN_REFCLK_DIS, 0); } void intel_display_power_suspend(struct drm_i915_private *i915) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 44aed4cbf894..8abcd35df926 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -3040,24 +3040,6 @@ static void valleyview_irq_reset(struct drm_i915_private *dev_priv) spin_unlock_irq(&dev_priv->irq_lock); } -static void cnp_display_clock_wa(struct drm_i915_private *dev_priv) -{ - struct intel_uncore *uncore = &dev_priv->uncore; - - /* - * Wa_14010685332:cnp/cmp,tgp,adp - * TODO: Clarify which platforms this applies to - * TODO: Figure out if this workaround can be applied in the s0ix suspend/resume handlers as - * on earlier platforms and whether the workaround is also needed for runtime suspend/resume - */ - if (INTEL_PCH_TYPE(dev_priv) == PCH_CNP || - (INTEL_PCH_TYPE(dev_priv) >= PCH_TGP && INTEL_PCH_TYPE(dev_priv) < PCH_DG1)) { - intel_uncore_rmw(uncore, SOUTH_CHICKEN1, SBCLK_RUN_REFCLK_DIS, - SBCLK_RUN_REFCLK_DIS); - intel_uncore_rmw(uncore, SOUTH_CHICKEN1, SBCLK_RUN_REFCLK_DIS, 0); - } -} - static void gen8_irq_reset(struct drm_i915_private *dev_priv) { struct intel_uncore *uncore = &dev_priv->uncore; @@ -3082,7 +3064,6 @@ static void gen8_irq_reset(struct drm_i915_private *dev_priv) if (HAS_PCH_SPLIT(dev_priv)) ibx_irq_reset(dev_priv); - cnp_display_clock_wa(dev_priv); } static void gen11_display_irq_reset(struct drm_i915_private *dev_priv) @@ -3123,8 +3104,6 @@ static void gen11_display_irq_reset(struct drm_i915_private *dev_priv) if (INTEL_PCH_TYPE(dev_priv) >= PCH_ICP) GEN3_IRQ_RESET(uncore, SDE); - - cnp_display_clock_wa(dev_priv); } static void gen11_irq_reset(struct drm_i915_private *dev_priv)
dispcnlunit1_cp_xosc_clkreq clock observed to be active on TGL-H platform despite Wa_14010685332 original sequence thus blocks entry to deeper s0ix state. The Tweaked Wa_14010685332 sequence fixes this issue, therefore use tweaked Wa_14010685332 sequence for every PCH since PCH_CNP. Fixes: b896898c7369 ("drm/i915: Tweaked Wa_14010685332 for PCHs used on gen11 platforms") Cc: Matt Roper <matthew.d.roper@intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com> --- .../drm/i915/display/intel_display_power.c | 18 +++++++++------- drivers/gpu/drm/i915/i915_irq.c | 21 ------------------- 2 files changed, 10 insertions(+), 29 deletions(-)