From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com ([192.55.52.115]:34855 "EHLO mga14.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755940AbdLVT6w (ORCPT ); Fri, 22 Dec 2017 14:58:52 -0500 Date: Fri, 22 Dec 2017 21:58:48 +0200 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= To: Lucas De Marchi Cc: intel-gfx@lists.freedesktop.org, Arthur J Runyan , Rodrigo Vivi , stable@vger.kernel.org Subject: Re: [PATCH v4] drm/i915: Apply Display WA #1183 on skl, kbl, and cfl Message-ID: <20171222195848.GS10981@intel.com> References: <20171204232210.4958-1-lucas.demarchi@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20171204232210.4958-1-lucas.demarchi@intel.com> Sender: stable-owner@vger.kernel.org List-ID: On Mon, Dec 04, 2017 at 03:22:10PM -0800, Lucas De Marchi wrote: > Display WA #1183 was recently added to workaround > "Failures when enabling DPLL0 with eDP link rate 2.16 > or 4.32 GHz and CD clock frequency 308.57 or 617.14 MHz > (CDCLK_CTL CD Frequency Select 10b or 11b) used in this > enabling or in previous enabling." > > This workaround was designed to minimize the impact only > to save the bad case with that link rates. But HW engineers > indicated that it should be safe to apply broadly, although > they were expecting the DPLL0 link rate to be unchanged on > runtime. > > We need to cover 2 cases: when we are in fact enabling DPLL0 > and when we are just changing the frequency with small > differences. > > This is based on previous patch by Rodrigo Vivi with suggestions > from Ville Syrj�l�. > > Cc: Arthur J Runyan > Cc: Ville Syrj�l� > Cc: Rodrigo Vivi > Cc: stable@vger.kernel.org > Signed-off-by: Lucas De Marchi Hmm. This is v4, but it was posted after v5? You didn't include a changelog in the commit message so I have no idea what is the difference between these versions. They appear similar on a first glance. The changes here *look* correct to me, so Reviewed-by: Ville Syrj�l� But I'm not going to push this right now because I'm too confused about the version of the patch. I'll be out for a few weeks so if the confusion is lifted during that time and you want this in please poke at someone else to push it. > --- > drivers/gpu/drm/i915/i915_reg.h | 2 ++ > drivers/gpu/drm/i915/intel_cdclk.c | 35 ++++++++++++++++++++++++--------- > drivers/gpu/drm/i915/intel_runtime_pm.c | 10 ++++++++++ > 3 files changed, 38 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 09bf043c1c2e..73335e709ed6 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -7021,6 +7021,7 @@ enum { > #define RESET_PCH_HANDSHAKE_ENABLE (1<<4) > > #define GEN8_CHICKEN_DCPR_1 _MMIO(0x46430) > +#define SKL_SELECT_ALTERNATE_DC_EXIT (1<<30) > #define MASK_WAKEMEM (1<<13) > > #define SKL_DFSM _MMIO(0x51000) > @@ -8575,6 +8576,7 @@ enum skl_power_gate { > #define BXT_CDCLK_CD2X_DIV_SEL_2 (2<<22) > #define BXT_CDCLK_CD2X_DIV_SEL_4 (3<<22) > #define BXT_CDCLK_CD2X_PIPE(pipe) ((pipe)<<20) > +#define CDCLK_DIVMUX_CD_OVERRIDE (1<<19) > #define BXT_CDCLK_CD2X_PIPE_NONE BXT_CDCLK_CD2X_PIPE(3) > #define BXT_CDCLK_SSA_PRECHARGE_ENABLE (1<<16) > #define CDCLK_FREQ_DECIMAL_MASK (0x7ff) > diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c > index 9c5ceb98d48f..d77e2bec1e29 100644 > --- a/drivers/gpu/drm/i915/intel_cdclk.c > +++ b/drivers/gpu/drm/i915/intel_cdclk.c > @@ -931,16 +931,10 @@ static void skl_set_preferred_cdclk_vco(struct drm_i915_private *dev_priv, > > static void skl_dpll0_enable(struct drm_i915_private *dev_priv, int vco) > { > - int min_cdclk = skl_calc_cdclk(0, vco); > u32 val; > > WARN_ON(vco != 8100000 && vco != 8640000); > > - /* select the minimum CDCLK before enabling DPLL 0 */ > - val = CDCLK_FREQ_337_308 | skl_cdclk_decimal(min_cdclk); > - I915_WRITE(CDCLK_CTL, val); > - POSTING_READ(CDCLK_CTL); > - > /* > * We always enable DPLL0 with the lowest link rate possible, but still > * taking into account the VCO required to operate the eDP panel at the > @@ -994,7 +988,7 @@ static void skl_set_cdclk(struct drm_i915_private *dev_priv, > { > int cdclk = cdclk_state->cdclk; > int vco = cdclk_state->vco; > - u32 freq_select; > + u32 freq_select, cdclk_ctl; > int ret; > > mutex_lock(&dev_priv->pcu_lock); > @@ -1009,7 +1003,7 @@ static void skl_set_cdclk(struct drm_i915_private *dev_priv, > return; > } > > - /* set CDCLK_CTL */ > + /* Choose frequency for this cdclk */ > switch (cdclk) { > default: > WARN_ON(cdclk != dev_priv->cdclk.hw.ref); > @@ -1036,10 +1030,33 @@ static void skl_set_cdclk(struct drm_i915_private *dev_priv, > dev_priv->cdclk.hw.vco != vco) > skl_dpll0_disable(dev_priv); > > + cdclk_ctl = I915_READ(CDCLK_CTL); > + > + if (dev_priv->cdclk.hw.vco != vco) { > + /* Wa Display #1183: skl,kbl,cfl */ > + cdclk_ctl &= ~(CDCLK_FREQ_SEL_MASK | CDCLK_FREQ_DECIMAL_MASK); > + cdclk_ctl |= freq_select | skl_cdclk_decimal(cdclk); > + I915_WRITE(CDCLK_CTL, cdclk_ctl); > + } > + > + /* Wa Display #1183: skl,kbl,cfl */ > + cdclk_ctl |= CDCLK_DIVMUX_CD_OVERRIDE; > + I915_WRITE(CDCLK_CTL, cdclk_ctl); > + POSTING_READ(CDCLK_CTL); > + > if (dev_priv->cdclk.hw.vco != vco) > skl_dpll0_enable(dev_priv, vco); > > - I915_WRITE(CDCLK_CTL, freq_select | skl_cdclk_decimal(cdclk)); > + /* Wa Display #1183: skl,kbl,cfl */ > + cdclk_ctl &= ~(CDCLK_FREQ_SEL_MASK | CDCLK_FREQ_DECIMAL_MASK); > + I915_WRITE(CDCLK_CTL, cdclk_ctl); > + > + cdclk_ctl |= freq_select | skl_cdclk_decimal(cdclk); > + I915_WRITE(CDCLK_CTL, cdclk_ctl); > + > + /* Wa Display #1183: skl,kbl,cfl */ > + cdclk_ctl &= ~CDCLK_DIVMUX_CD_OVERRIDE; > + I915_WRITE(CDCLK_CTL, cdclk_ctl); > POSTING_READ(CDCLK_CTL); > > /* inform PCU of the change */ > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c > index 8315499452dc..35796fa8e6b4 100644 > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > @@ -598,6 +598,11 @@ void gen9_enable_dc5(struct drm_i915_private *dev_priv) > > DRM_DEBUG_KMS("Enabling DC5\n"); > > + /* Wa Display #1183: skl,kbl,cfl */ > + if (IS_GEN9_BC(dev_priv)) > + I915_WRITE(GEN8_CHICKEN_DCPR_1, I915_READ(GEN8_CHICKEN_DCPR_1) | > + SKL_SELECT_ALTERNATE_DC_EXIT); > + > gen9_set_dc_state(dev_priv, DC_STATE_EN_UPTO_DC5); > } > > @@ -625,6 +630,11 @@ void skl_disable_dc6(struct drm_i915_private *dev_priv) > { > DRM_DEBUG_KMS("Disabling DC6\n"); > > + /* Wa Display #1183: skl,kbl,cfl */ > + if (IS_GEN9_BC(dev_priv)) > + I915_WRITE(GEN8_CHICKEN_DCPR_1, I915_READ(GEN8_CHICKEN_DCPR_1) | > + SKL_SELECT_ALTERNATE_DC_EXIT); > + > gen9_set_dc_state(dev_priv, DC_STATE_DISABLE); > } > > -- > 2.14.3 -- Ville Syrj�l� Intel OTC