Re: [PATCH v3] drm/i915: Apply Wa Display #1183 on skl, kbl, and cfl

[Lucas De Marchi <lucas.de.marchi@gmail.com>]

On Thu, Nov 9, 2017 at 5:11 AM, Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
> On Thu, Nov 09, 2017 at 02:58:04AM -0800, Lucas De Marchi wrote:
>> Wa Display #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. The workaround
>> for those cases are similar but different enough to have them
>> done in different places.
>>
>> This is based on previous patch by Rodrigo Vivi with suggestions
>> from Ville Syrjälä.
>
> Still doesn't look like what I suggested.

I agree with your suggestion of moving stuff to skl_set_cdclk() to
cover the case in which
vco isn't changing. However see the paragraph I added above on why I
need to do it
differently. In short: the sequence on the WA for enabling and
updating cdclck are different,
with some code duplication unfortunately. I don't see you covering
that case in your
suggestion. Have I missed anything?


Lucas De Marchi

>
>>
>> Cc: Arthur J Runyan <arthur.j.runyan@intel.com>
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
>> ---
>>
>> I tried to test this but both on SKL and KBL that I have the bug that requires
>> the WA isn't triggered.
>>
>>  drivers/gpu/drm/i915/i915_reg.h         |  2 ++
>>  drivers/gpu/drm/i915/intel_cdclk.c      | 51 ++++++++++++++++++++++++++-------
>>  drivers/gpu/drm/i915/intel_runtime_pm.c | 10 +++++++
>>  3 files changed, 53 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index 6ef33422f762..a32d8200bb47 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -6981,6 +6981,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)
>> @@ -8535,6 +8536,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 e8884c2ade98..5e6fc2602711 100644
>> --- a/drivers/gpu/drm/i915/intel_cdclk.c
>> +++ b/drivers/gpu/drm/i915/intel_cdclk.c
>> @@ -929,17 +929,18 @@ static void skl_set_preferred_cdclk_vco(struct drm_i915_private *dev_priv,
>>               intel_update_max_cdclk(dev_priv);
>>  }
>>
>> -static void skl_dpll0_enable(struct drm_i915_private *dev_priv, int vco)
>> +static void skl_dpll0_enable(struct drm_i915_private *dev_priv, int vco,
>> +                          u32 freq_select, u32 cdclk)
>>  {
>> -     int min_cdclk = skl_calc_cdclk(0, vco);
>> -     u32 val;
>> +     u32 val, cdctl;
>>
>>       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);
>> +     /* Wa Display #1183: skl,kbl,cfl */
>> +     cdctl = I915_READ(CDCLK_CTL);
>> +     cdctl &= ~(CDCLK_FREQ_SEL_MASK | CDCLK_FREQ_DECIMAL_MASK);
>> +     cdctl |= freq_select | skl_cdclk_decimal(cdclk);
>> +     I915_WRITE(CDCLK_CTL, cdctl);
>>
>>       /*
>>        * We always enable DPLL0 with the lowest link rate possible, but still
>> @@ -965,6 +966,10 @@ static void skl_dpll0_enable(struct drm_i915_private *dev_priv, int vco)
>>       I915_WRITE(DPLL_CTRL1, val);
>>       POSTING_READ(DPLL_CTRL1);
>>
>> +     /* Wa Display #1183: skl,kbl,cfl */
>> +     cdctl |= CDCLK_DIVMUX_CD_OVERRIDE;
>> +     I915_WRITE(CDCLK_CTL, cdctl);
>> +
>>       I915_WRITE(LCPLL1_CTL, I915_READ(LCPLL1_CTL) | LCPLL_PLL_ENABLE);
>>
>>       if (intel_wait_for_register(dev_priv,
>> @@ -972,6 +977,17 @@ static void skl_dpll0_enable(struct drm_i915_private *dev_priv, int vco)
>>                                   5))
>>               DRM_ERROR("DPLL0 not locked\n");
>>
>> +     /* Wa Display #1183: skl,kbl,cfl */
>> +     cdctl &= ~CDCLK_FREQ_SEL_MASK;
>> +     I915_WRITE(CDCLK_CTL, cdctl);
>> +
>> +     cdctl |= freq_select;
>> +     I915_WRITE(CDCLK_CTL, cdctl);
>> +
>> +     cdctl &= ~CDCLK_DIVMUX_CD_OVERRIDE;
>> +     I915_WRITE(CDCLK_CTL, cdctl);
>> +     POSTING_READ(CDCLK_CTL);
>> +
>>       dev_priv->cdclk.hw.vco = vco;
>>
>>       /* We'll want to keep using the current vco from now on. */
>> @@ -1037,10 +1053,25 @@ static void skl_set_cdclk(struct drm_i915_private *dev_priv,
>>               skl_dpll0_disable(dev_priv);
>>
>>       if (dev_priv->cdclk.hw.vco != vco)
>> -             skl_dpll0_enable(dev_priv, vco);
>> +             skl_dpll0_enable(dev_priv, vco, freq_select, cdclk);
>> +     else {
>> +             u32 cdctl = I915_READ(CDCLK_CTL);
>>
>> -     I915_WRITE(CDCLK_CTL, freq_select | skl_cdclk_decimal(cdclk));
>> -     POSTING_READ(CDCLK_CTL);
>> +             /* Wa Display #1183: skl,kbl,cfl */
>> +             cdctl |= CDCLK_DIVMUX_CD_OVERRIDE;
>> +             I915_WRITE(CDCLK_CTL, cdctl);
>> +
>> +             cdctl &= ~CDCLK_FREQ_SEL_MASK;
>> +             I915_WRITE(CDCLK_CTL, cdctl);
>> +
>> +             cdctl &= ~CDCLK_FREQ_DECIMAL_MASK;
>> +             cdctl |= freq_select | skl_cdclk_decimal(cdclk);
>> +             I915_WRITE(CDCLK_CTL, cdctl);
>> +
>> +             cdctl &= ~CDCLK_DIVMUX_CD_OVERRIDE;
>> +             I915_WRITE(CDCLK_CTL, cdctl);
>> +             POSTING_READ(CDCLK_CTL);
>> +     }
>>
>>       /* inform PCU of the change */
>>       mutex_lock(&dev_priv->pcu_lock);
>> 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
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx