On Wed, Sep 11, 2019 at 04:31:27PM +0300, Ville Syrjala wrote: > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > We're forgetting to mask off all three pipe select bits from the > CDCLK_CTL value on icl+ which may lead to the extra bit being > left in. That will cause us to consider the current hardware > cdclk state as invalid, and we proceed to sanitize it even > though the hardware may have active pipes and whatnot. > > Fix up the mask so we get rid of all three pipe select bits > and thus hopefully no longer sanitize cdclk when it's already > correctly programmed. > > Cc: Matt Roper <matthew.d.roper@xxxxxxxxx> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111641 > Fixes: 0c1279b58fc7 ("drm/i915: Consolidate {bxt,cnl,icl}_init_cdclk") > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> Reviewed-by: Matt Roper <matthew.d.roper@xxxxxxxxx> I'm confused why pre-merge CI flagged the results as a success if TGL was hitting this? Matt > --- > drivers/gpu/drm/i915/display/intel_cdclk.c | 42 ++++++++++++---------- > 1 file changed, 23 insertions(+), 19 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c > index 6b75d2a91cd9..f59a6f775177 100644 > --- a/drivers/gpu/drm/i915/display/intel_cdclk.c > +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c > @@ -1464,6 +1464,26 @@ static void cnl_cdclk_pll_enable(struct drm_i915_private *dev_priv, int vco) > dev_priv->cdclk.hw.vco = vco; > } > > +static u32 bxt_cdclk_cd2x_pipe(struct drm_i915_private *dev_priv, enum pipe pipe) > +{ > + if (INTEL_GEN(dev_priv) >= 12) { > + if (pipe == INVALID_PIPE) > + return TGL_CDCLK_CD2X_PIPE_NONE; > + else > + return TGL_CDCLK_CD2X_PIPE(pipe); > + } else if (INTEL_GEN(dev_priv) >= 11) { > + if (pipe == INVALID_PIPE) > + return ICL_CDCLK_CD2X_PIPE_NONE; > + else > + return ICL_CDCLK_CD2X_PIPE(pipe); > + } else { > + if (pipe == INVALID_PIPE) > + return BXT_CDCLK_CD2X_PIPE_NONE; > + else > + return BXT_CDCLK_CD2X_PIPE(pipe); > + } > +} > + > static void bxt_set_cdclk(struct drm_i915_private *dev_priv, > const struct intel_cdclk_state *cdclk_state, > enum pipe pipe) > @@ -1534,24 +1554,8 @@ static void bxt_set_cdclk(struct drm_i915_private *dev_priv, > bxt_de_pll_enable(dev_priv, vco); > } > > - val = divider | skl_cdclk_decimal(cdclk); > - > - if (INTEL_GEN(dev_priv) >= 12) { > - if (pipe == INVALID_PIPE) > - val |= TGL_CDCLK_CD2X_PIPE_NONE; > - else > - val |= TGL_CDCLK_CD2X_PIPE(pipe); > - } else if (INTEL_GEN(dev_priv) >= 11) { > - if (pipe == INVALID_PIPE) > - val |= ICL_CDCLK_CD2X_PIPE_NONE; > - else > - val |= ICL_CDCLK_CD2X_PIPE(pipe); > - } else { > - if (pipe == INVALID_PIPE) > - val |= BXT_CDCLK_CD2X_PIPE_NONE; > - else > - val |= BXT_CDCLK_CD2X_PIPE(pipe); > - } > + val = divider | skl_cdclk_decimal(cdclk) | > + bxt_cdclk_cd2x_pipe(dev_priv, pipe); > > /* > * Disable SSA Precharge when CD clock frequency < 500 MHz, > @@ -1620,7 +1624,7 @@ static void bxt_sanitize_cdclk(struct drm_i915_private *dev_priv) > * dividers both synching to an active pipe, or asynchronously > * (PIPE_NONE). > */ > - cdctl &= ~BXT_CDCLK_CD2X_PIPE_NONE; > + cdctl &= ~bxt_cdclk_cd2x_pipe(dev_priv, INVALID_PIPE); > > /* Make sure this is a legal cdclk value for the platform */ > cdclk = bxt_calc_cdclk(dev_priv, dev_priv->cdclk.hw.cdclk); > -- > 2.21.0 > -- Matt Roper Graphics Software Engineer VTT-OSGC Platform Enablement Intel Corporation (916) 356-2795 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
