| Message ID | 20190704010658.32170-1-matthew.d.roper@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | None | expand |
On Wed, Jul 03, 2019 at 06:06:58PM -0700, Matt Roper wrote: > Although the register name implies that it operates on DDI's, > DPCLKA_CFGCR0_ICL actually needs to be programmed according to the PHY > that's in use. I.e., when using EHL's DDI-D on combo PHY A, the bits > described as "port A" in the bspec are what we need to set. The bspec > clarifies: > > "[For EHL] DDID clock tied to DDIA clock, so DPCLKA_CFGCR0 DDIA > Clock Select chooses the PLL for both DDIA and DDID and drives > port A in all cases." > > Also, since the CNL DPCLKA_CFGCR0 bit defines are still port-based, we > create separate ICL-specific defines that accept the PHY rather than > trying to share the same bit definitions between CNL and ICL. > > v5: Make icl_dpclka_cfgcr0_clk_off() take phy rather than port. When > splitting the original patch the hunk to handle this wound up too > late in the series. (Sparse) > > Bspec: 33148 > Cc: José Roberto de Souza <jose.souza@intel.com> > Signed-off-by: Matt Roper <matthew.d.roper@intel.com> Looks correct to me. Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/i915/display/icl_dsi.c | 17 ++++++--- > drivers/gpu/drm/i915/display/intel_ddi.c | 47 +++++++++++++++--------- > drivers/gpu/drm/i915/i915_reg.h | 12 ++++-- > 3 files changed, 50 insertions(+), 26 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c > index b8673debf932..f574af62888c 100644 > --- a/drivers/gpu/drm/i915/display/icl_dsi.c > +++ b/drivers/gpu/drm/i915/display/icl_dsi.c > @@ -560,11 +560,13 @@ static void gen11_dsi_gate_clocks(struct intel_encoder *encoder) > struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base); > u32 tmp; > enum port port; > + enum phy phy; > > mutex_lock(&dev_priv->dpll_lock); > tmp = I915_READ(DPCLKA_CFGCR0_ICL); > for_each_dsi_port(port, intel_dsi->ports) { > - tmp |= DPCLKA_CFGCR0_DDI_CLK_OFF(port); > + phy = intel_port_to_phy(dev_priv, port); > + tmp |= ICL_DPCLKA_CFGCR0_DDI_CLK_OFF(phy); > } > > I915_WRITE(DPCLKA_CFGCR0_ICL, tmp); > @@ -577,11 +579,13 @@ static void gen11_dsi_ungate_clocks(struct intel_encoder *encoder) > struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base); > u32 tmp; > enum port port; > + enum phy phy; > > mutex_lock(&dev_priv->dpll_lock); > tmp = I915_READ(DPCLKA_CFGCR0_ICL); > for_each_dsi_port(port, intel_dsi->ports) { > - tmp &= ~DPCLKA_CFGCR0_DDI_CLK_OFF(port); > + phy = intel_port_to_phy(dev_priv, port); > + tmp &= ~ICL_DPCLKA_CFGCR0_DDI_CLK_OFF(phy); > } > > I915_WRITE(DPCLKA_CFGCR0_ICL, tmp); > @@ -595,19 +599,22 @@ static void gen11_dsi_map_pll(struct intel_encoder *encoder, > struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base); > struct intel_shared_dpll *pll = crtc_state->shared_dpll; > enum port port; > + enum phy phy; > u32 val; > > mutex_lock(&dev_priv->dpll_lock); > > val = I915_READ(DPCLKA_CFGCR0_ICL); > for_each_dsi_port(port, intel_dsi->ports) { > - val &= ~DPCLKA_CFGCR0_DDI_CLK_SEL_MASK(port); > - val |= DPCLKA_CFGCR0_DDI_CLK_SEL(pll->info->id, port); > + phy = intel_port_to_phy(dev_priv, port); > + val &= ~ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_MASK(phy); > + val |= ICL_DPCLKA_CFGCR0_DDI_CLK_SEL(pll->info->id, phy); > } > I915_WRITE(DPCLKA_CFGCR0_ICL, val); > > for_each_dsi_port(port, intel_dsi->ports) { > - val &= ~DPCLKA_CFGCR0_DDI_CLK_OFF(port); > + phy = intel_port_to_phy(dev_priv, port); > + val &= ~ICL_DPCLKA_CFGCR0_DDI_CLK_OFF(phy); > } > I915_WRITE(DPCLKA_CFGCR0_ICL, val); > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c > index a4172595c8d8..065feb917db4 100644 > --- a/drivers/gpu/drm/i915/display/intel_ddi.c > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c > @@ -2729,12 +2729,13 @@ u32 ddi_signal_levels(struct intel_dp *intel_dp) > > static inline > u32 icl_dpclka_cfgcr0_clk_off(struct drm_i915_private *dev_priv, > - enum port port) > + enum phy phy) > { > - if (intel_port_is_combophy(dev_priv, port)) { > - return ICL_DPCLKA_CFGCR0_DDI_CLK_OFF(port); > - } else if (intel_port_is_tc(dev_priv, port)) { > - enum tc_port tc_port = intel_port_to_tc(dev_priv, port); > + if (intel_phy_is_combo(dev_priv, phy)) { > + return ICL_DPCLKA_CFGCR0_DDI_CLK_OFF(phy); > + } else if (intel_phy_is_tc(dev_priv, phy)) { > + enum tc_port tc_port = intel_port_to_tc(dev_priv, > + (enum port)phy); > > return ICL_DPCLKA_CFGCR0_TC_CLK_OFF(tc_port); > } > @@ -2747,22 +2748,32 @@ static void icl_map_plls_to_ports(struct intel_encoder *encoder, > { > struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); > struct intel_shared_dpll *pll = crtc_state->shared_dpll; > - enum port port = encoder->port; > + enum phy phy = intel_port_to_phy(dev_priv, encoder->port); > u32 val; > > mutex_lock(&dev_priv->dpll_lock); > > val = I915_READ(DPCLKA_CFGCR0_ICL); > - WARN_ON((val & icl_dpclka_cfgcr0_clk_off(dev_priv, port)) == 0); > + WARN_ON((val & icl_dpclka_cfgcr0_clk_off(dev_priv, phy)) == 0); > > - if (intel_port_is_combophy(dev_priv, port)) { > - val &= ~DPCLKA_CFGCR0_DDI_CLK_SEL_MASK(port); > - val |= DPCLKA_CFGCR0_DDI_CLK_SEL(pll->info->id, port); > + if (intel_phy_is_combo(dev_priv, phy)) { > + /* > + * Even though this register references DDIs, note that we > + * want to pass the PHY rather than the port (DDI). For > + * ICL, port=phy in all cases so it doesn't matter, but for > + * EHL the bspec notes the following: > + * > + * "DDID clock tied to DDIA clock, so DPCLKA_CFGCR0 DDIA > + * Clock Select chooses the PLL for both DDIA and DDID and > + * drives port A in all cases." > + */ > + val &= ~ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_MASK(phy); > + val |= ICL_DPCLKA_CFGCR0_DDI_CLK_SEL(pll->info->id, phy); > I915_WRITE(DPCLKA_CFGCR0_ICL, val); > POSTING_READ(DPCLKA_CFGCR0_ICL); > } > > - val &= ~icl_dpclka_cfgcr0_clk_off(dev_priv, port); > + val &= ~icl_dpclka_cfgcr0_clk_off(dev_priv, phy); > I915_WRITE(DPCLKA_CFGCR0_ICL, val); > > mutex_unlock(&dev_priv->dpll_lock); > @@ -2771,13 +2782,13 @@ static void icl_map_plls_to_ports(struct intel_encoder *encoder, > static void icl_unmap_plls_to_ports(struct intel_encoder *encoder) > { > struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); > - enum port port = encoder->port; > + enum phy phy = intel_port_to_phy(dev_priv, encoder->port); > u32 val; > > mutex_lock(&dev_priv->dpll_lock); > > val = I915_READ(DPCLKA_CFGCR0_ICL); > - val |= icl_dpclka_cfgcr0_clk_off(dev_priv, port); > + val |= icl_dpclka_cfgcr0_clk_off(dev_priv, phy); > I915_WRITE(DPCLKA_CFGCR0_ICL, val); > > mutex_unlock(&dev_priv->dpll_lock); > @@ -2838,9 +2849,11 @@ void icl_sanitize_encoder_pll_mapping(struct intel_encoder *encoder) > > val = I915_READ(DPCLKA_CFGCR0_ICL); > for_each_port_masked(port, port_mask) { > + enum phy phy = intel_port_to_phy(dev_priv, port); > + > bool ddi_clk_ungated = !(val & > icl_dpclka_cfgcr0_clk_off(dev_priv, > - port)); > + phy)); > > if (ddi_clk_needed == ddi_clk_ungated) > continue; > @@ -2852,9 +2865,9 @@ void icl_sanitize_encoder_pll_mapping(struct intel_encoder *encoder) > if (WARN_ON(ddi_clk_needed)) > continue; > > - DRM_NOTE("Port %c is disabled/in DSI mode with an ungated DDI clock, gate it\n", > - port_name(port)); > - val |= icl_dpclka_cfgcr0_clk_off(dev_priv, port); > + DRM_NOTE("PHY %c is disabled/in DSI mode with an ungated DDI clock, gate it\n", > + phy_name(port)); > + val |= icl_dpclka_cfgcr0_clk_off(dev_priv, phy); > I915_WRITE(DPCLKA_CFGCR0_ICL, val); > } > } > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index c814cc1b3ae5..c9e2e09b6f01 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -9680,17 +9680,21 @@ enum skl_power_gate { > * CNL Clocks > */ > #define DPCLKA_CFGCR0 _MMIO(0x6C200) > -#define DPCLKA_CFGCR0_ICL _MMIO(0x164280) > #define DPCLKA_CFGCR0_DDI_CLK_OFF(port) (1 << ((port) == PORT_F ? 23 : \ > (p`:wqort) + 10)) > -#define ICL_DPCLKA_CFGCR0_DDI_CLK_OFF(port) (1 << ((port) + 10)) > -#define ICL_DPCLKA_CFGCR0_TC_CLK_OFF(tc_port) (1 << ((tc_port) == PORT_TC4 ? \ > - 21 : (tc_port) + 12)) > #define DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(port) ((port) == PORT_F ? 21 : \ > (port) * 2) > #define DPCLKA_CFGCR0_DDI_CLK_SEL_MASK(port) (3 << DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(port)) > #define DPCLKA_CFGCR0_DDI_CLK_SEL(pll, port) ((pll) << DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(port)) > > +#define DPCLKA_CFGCR0_ICL _MMIO(0x164280) > +#define ICL_DPCLKA_CFGCR0_DDI_CLK_OFF(phy) (1 << _PICK(phy, 10, 11, 24)) > +#define ICL_DPCLKA_CFGCR0_TC_CLK_OFF(tc_port) (1 << ((tc_port) == PORT_TC4 ? \ > + 21 : (tc_port) + 12)) > +#define ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(phy) ((phy) * 2) > +#define ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_MASK(phy) (3 << ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(phy)) > +#define ICL_DPCLKA_CFGCR0_DDI_CLK_SEL(pll, phy) ((pll) << ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(phy)) > + > /* CNL PLL */ > #define DPLL0_ENABLE 0x46010 > #define DPLL1_ENABLE 0x46014 > -- > 2.17.2 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Wed, 2019-07-03 at 18:06 -0700, Matt Roper wrote: > Although the register name implies that it operates on DDI's, > DPCLKA_CFGCR0_ICL actually needs to be programmed according to the > PHY > that's in use. I.e., when using EHL's DDI-D on combo PHY A, the bits > described as "port A" in the bspec are what we need to set. The > bspec > clarifies: > > "[For EHL] DDID clock tied to DDIA clock, so DPCLKA_CFGCR0 > DDIA > Clock Select chooses the PLL for both DDIA and DDID and > drives > port A in all cases." > > Also, since the CNL DPCLKA_CFGCR0 bit defines are still port-based, > we > create separate ICL-specific defines that accept the PHY rather than > trying to share the same bit definitions between CNL and ICL. > Nit: Why not already rename DPCLKA_CFGCR0_ICL to ICL_DPCLKA_CFGCR0? The bits have the new name, so you are already touching in everyplace that uses DPCLKA_CFGCR0_ICL. > v5: Make icl_dpclka_cfgcr0_clk_off() take phy rather than port. When > splitting the original patch the hunk to handle this wound up too > late in the series. (Sparse) Reviewed-by: José Roberto de Souza <jose.souza@intel.com> > > Bspec: 33148 > Cc: José Roberto de Souza <jose.souza@intel.com> > Signed-off-by: Matt Roper <matthew.d.roper@intel.com> > --- > drivers/gpu/drm/i915/display/icl_dsi.c | 17 ++++++--- > drivers/gpu/drm/i915/display/intel_ddi.c | 47 +++++++++++++++------- > -- > drivers/gpu/drm/i915/i915_reg.h | 12 ++++-- > 3 files changed, 50 insertions(+), 26 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c > b/drivers/gpu/drm/i915/display/icl_dsi.c > index b8673debf932..f574af62888c 100644 > --- a/drivers/gpu/drm/i915/display/icl_dsi.c > +++ b/drivers/gpu/drm/i915/display/icl_dsi.c > @@ -560,11 +560,13 @@ static void gen11_dsi_gate_clocks(struct > intel_encoder *encoder) > struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base); > u32 tmp; > enum port port; > + enum phy phy; > > mutex_lock(&dev_priv->dpll_lock); > tmp = I915_READ(DPCLKA_CFGCR0_ICL); > for_each_dsi_port(port, intel_dsi->ports) { > - tmp |= DPCLKA_CFGCR0_DDI_CLK_OFF(port); > + phy = intel_port_to_phy(dev_priv, port); > + tmp |= ICL_DPCLKA_CFGCR0_DDI_CLK_OFF(phy); > } > > I915_WRITE(DPCLKA_CFGCR0_ICL, tmp); > @@ -577,11 +579,13 @@ static void gen11_dsi_ungate_clocks(struct > intel_encoder *encoder) > struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base); > u32 tmp; > enum port port; > + enum phy phy; > > mutex_lock(&dev_priv->dpll_lock); > tmp = I915_READ(DPCLKA_CFGCR0_ICL); > for_each_dsi_port(port, intel_dsi->ports) { > - tmp &= ~DPCLKA_CFGCR0_DDI_CLK_OFF(port); > + phy = intel_port_to_phy(dev_priv, port); > + tmp &= ~ICL_DPCLKA_CFGCR0_DDI_CLK_OFF(phy); > } > > I915_WRITE(DPCLKA_CFGCR0_ICL, tmp); > @@ -595,19 +599,22 @@ static void gen11_dsi_map_pll(struct > intel_encoder *encoder, > struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base); > struct intel_shared_dpll *pll = crtc_state->shared_dpll; > enum port port; > + enum phy phy; > u32 val; > > mutex_lock(&dev_priv->dpll_lock); > > val = I915_READ(DPCLKA_CFGCR0_ICL); > for_each_dsi_port(port, intel_dsi->ports) { > - val &= ~DPCLKA_CFGCR0_DDI_CLK_SEL_MASK(port); > - val |= DPCLKA_CFGCR0_DDI_CLK_SEL(pll->info->id, port); > + phy = intel_port_to_phy(dev_priv, port); > + val &= ~ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_MASK(phy); > + val |= ICL_DPCLKA_CFGCR0_DDI_CLK_SEL(pll->info->id, > phy); > } > I915_WRITE(DPCLKA_CFGCR0_ICL, val); > > for_each_dsi_port(port, intel_dsi->ports) { > - val &= ~DPCLKA_CFGCR0_DDI_CLK_OFF(port); > + phy = intel_port_to_phy(dev_priv, port); > + val &= ~ICL_DPCLKA_CFGCR0_DDI_CLK_OFF(phy); > } > I915_WRITE(DPCLKA_CFGCR0_ICL, val); > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c > b/drivers/gpu/drm/i915/display/intel_ddi.c > index a4172595c8d8..065feb917db4 100644 > --- a/drivers/gpu/drm/i915/display/intel_ddi.c > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c > @@ -2729,12 +2729,13 @@ u32 ddi_signal_levels(struct intel_dp > *intel_dp) > > static inline > u32 icl_dpclka_cfgcr0_clk_off(struct drm_i915_private *dev_priv, > - enum port port) > + enum phy phy) > { > - if (intel_port_is_combophy(dev_priv, port)) { > - return ICL_DPCLKA_CFGCR0_DDI_CLK_OFF(port); > - } else if (intel_port_is_tc(dev_priv, port)) { > - enum tc_port tc_port = intel_port_to_tc(dev_priv, > port); > + if (intel_phy_is_combo(dev_priv, phy)) { > + return ICL_DPCLKA_CFGCR0_DDI_CLK_OFF(phy); > + } else if (intel_phy_is_tc(dev_priv, phy)) { > + enum tc_port tc_port = intel_port_to_tc(dev_priv, > + (enum > port)phy); > > return ICL_DPCLKA_CFGCR0_TC_CLK_OFF(tc_port); > } > @@ -2747,22 +2748,32 @@ static void icl_map_plls_to_ports(struct > intel_encoder *encoder, > { > struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); > struct intel_shared_dpll *pll = crtc_state->shared_dpll; > - enum port port = encoder->port; > + enum phy phy = intel_port_to_phy(dev_priv, encoder->port); > u32 val; > > mutex_lock(&dev_priv->dpll_lock); > > val = I915_READ(DPCLKA_CFGCR0_ICL); > - WARN_ON((val & icl_dpclka_cfgcr0_clk_off(dev_priv, port)) == > 0); > + WARN_ON((val & icl_dpclka_cfgcr0_clk_off(dev_priv, phy)) == 0); > > - if (intel_port_is_combophy(dev_priv, port)) { > - val &= ~DPCLKA_CFGCR0_DDI_CLK_SEL_MASK(port); > - val |= DPCLKA_CFGCR0_DDI_CLK_SEL(pll->info->id, port); > + if (intel_phy_is_combo(dev_priv, phy)) { > + /* > + * Even though this register references DDIs, note that > we > + * want to pass the PHY rather than the port > (DDI). For > + * ICL, port=phy in all cases so it doesn't matter, but > for > + * EHL the bspec notes the following: > + * > + * "DDID clock tied to DDIA clock, so DPCLKA_CFGCR0 > DDIA > + * Clock Select chooses the PLL for both DDIA and > DDID and > + * drives port A in all cases." > + */ > + val &= ~ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_MASK(phy); > + val |= ICL_DPCLKA_CFGCR0_DDI_CLK_SEL(pll->info->id, > phy); > I915_WRITE(DPCLKA_CFGCR0_ICL, val); > POSTING_READ(DPCLKA_CFGCR0_ICL); > } > > - val &= ~icl_dpclka_cfgcr0_clk_off(dev_priv, port); > + val &= ~icl_dpclka_cfgcr0_clk_off(dev_priv, phy); > I915_WRITE(DPCLKA_CFGCR0_ICL, val); > > mutex_unlock(&dev_priv->dpll_lock); > @@ -2771,13 +2782,13 @@ static void icl_map_plls_to_ports(struct > intel_encoder *encoder, > static void icl_unmap_plls_to_ports(struct intel_encoder *encoder) > { > struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); > - enum port port = encoder->port; > + enum phy phy = intel_port_to_phy(dev_priv, encoder->port); > u32 val; > > mutex_lock(&dev_priv->dpll_lock); > > val = I915_READ(DPCLKA_CFGCR0_ICL); > - val |= icl_dpclka_cfgcr0_clk_off(dev_priv, port); > + val |= icl_dpclka_cfgcr0_clk_off(dev_priv, phy); > I915_WRITE(DPCLKA_CFGCR0_ICL, val); > > mutex_unlock(&dev_priv->dpll_lock); > @@ -2838,9 +2849,11 @@ void icl_sanitize_encoder_pll_mapping(struct > intel_encoder *encoder) > > val = I915_READ(DPCLKA_CFGCR0_ICL); > for_each_port_masked(port, port_mask) { > + enum phy phy = intel_port_to_phy(dev_priv, port); > + > bool ddi_clk_ungated = !(val & > icl_dpclka_cfgcr0_clk_off(dev_ > priv, > - port > )); > + phy) > ); > > if (ddi_clk_needed == ddi_clk_ungated) > continue; > @@ -2852,9 +2865,9 @@ void icl_sanitize_encoder_pll_mapping(struct > intel_encoder *encoder) > if (WARN_ON(ddi_clk_needed)) > continue; > > - DRM_NOTE("Port %c is disabled/in DSI mode with an > ungated DDI clock, gate it\n", > - port_name(port)); > - val |= icl_dpclka_cfgcr0_clk_off(dev_priv, port); > + DRM_NOTE("PHY %c is disabled/in DSI mode with an > ungated DDI clock, gate it\n", > + phy_name(port)); > + val |= icl_dpclka_cfgcr0_clk_off(dev_priv, phy); > I915_WRITE(DPCLKA_CFGCR0_ICL, val); > } > } > diff --git a/drivers/gpu/drm/i915/i915_reg.h > b/drivers/gpu/drm/i915/i915_reg.h > index c814cc1b3ae5..c9e2e09b6f01 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -9680,17 +9680,21 @@ enum skl_power_gate { > * CNL Clocks > */ > #define DPCLKA_CFGCR0 _MMIO(0x6C200) > -#define DPCLKA_CFGCR0_ICL _MMIO(0x164280) > #define DPCLKA_CFGCR0_DDI_CLK_OFF(port) (1 << ((port) > == PORT_F ? 23 : \ > (port) + 10)) > -#define ICL_DPCLKA_CFGCR0_DDI_CLK_OFF(port) (1 << ((port) + 10)) > -#define ICL_DPCLKA_CFGCR0_TC_CLK_OFF(tc_port) (1 << ((tc_port) == > PORT_TC4 ? \ > - 21 : (tc_port) + > 12)) > #define DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(port) ((port) == > PORT_F ? 21 : \ > (port) * 2) > #define DPCLKA_CFGCR0_DDI_CLK_SEL_MASK(port) (3 << > DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(port)) > #define DPCLKA_CFGCR0_DDI_CLK_SEL(pll, port) ((pll) << > DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(port)) > > +#define DPCLKA_CFGCR0_ICL _MMIO(0x164280) > +#define ICL_DPCLKA_CFGCR0_DDI_CLK_OFF(phy) (1 << _PICK(phy, 10, > 11, 24)) > +#define ICL_DPCLKA_CFGCR0_TC_CLK_OFF(tc_port) (1 << ((tc_port) == > PORT_TC4 ? \ > + 21 : (tc_port) + > 12)) > +#define ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(phy) ((phy) * 2) > +#define ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_MASK(phy) (3 << > ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(phy)) > +#define ICL_DPCLKA_CFGCR0_DDI_CLK_SEL(pll, phy) ((pll) << > ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(phy)) > + > /* CNL PLL */ > #define DPLL0_ENABLE 0x46010 > #define DPLL1_ENABLE 0x46014
diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c index b8673debf932..f574af62888c 100644 --- a/drivers/gpu/drm/i915/display/icl_dsi.c +++ b/drivers/gpu/drm/i915/display/icl_dsi.c @@ -560,11 +560,13 @@ static void gen11_dsi_gate_clocks(struct intel_encoder *encoder) struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base); u32 tmp; enum port port; + enum phy phy; mutex_lock(&dev_priv->dpll_lock); tmp = I915_READ(DPCLKA_CFGCR0_ICL); for_each_dsi_port(port, intel_dsi->ports) { - tmp |= DPCLKA_CFGCR0_DDI_CLK_OFF(port); + phy = intel_port_to_phy(dev_priv, port); + tmp |= ICL_DPCLKA_CFGCR0_DDI_CLK_OFF(phy); } I915_WRITE(DPCLKA_CFGCR0_ICL, tmp); @@ -577,11 +579,13 @@ static void gen11_dsi_ungate_clocks(struct intel_encoder *encoder) struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base); u32 tmp; enum port port; + enum phy phy; mutex_lock(&dev_priv->dpll_lock); tmp = I915_READ(DPCLKA_CFGCR0_ICL); for_each_dsi_port(port, intel_dsi->ports) { - tmp &= ~DPCLKA_CFGCR0_DDI_CLK_OFF(port); + phy = intel_port_to_phy(dev_priv, port); + tmp &= ~ICL_DPCLKA_CFGCR0_DDI_CLK_OFF(phy); } I915_WRITE(DPCLKA_CFGCR0_ICL, tmp); @@ -595,19 +599,22 @@ static void gen11_dsi_map_pll(struct intel_encoder *encoder, struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base); struct intel_shared_dpll *pll = crtc_state->shared_dpll; enum port port; + enum phy phy; u32 val; mutex_lock(&dev_priv->dpll_lock); val = I915_READ(DPCLKA_CFGCR0_ICL); for_each_dsi_port(port, intel_dsi->ports) { - val &= ~DPCLKA_CFGCR0_DDI_CLK_SEL_MASK(port); - val |= DPCLKA_CFGCR0_DDI_CLK_SEL(pll->info->id, port); + phy = intel_port_to_phy(dev_priv, port); + val &= ~ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_MASK(phy); + val |= ICL_DPCLKA_CFGCR0_DDI_CLK_SEL(pll->info->id, phy); } I915_WRITE(DPCLKA_CFGCR0_ICL, val); for_each_dsi_port(port, intel_dsi->ports) { - val &= ~DPCLKA_CFGCR0_DDI_CLK_OFF(port); + phy = intel_port_to_phy(dev_priv, port); + val &= ~ICL_DPCLKA_CFGCR0_DDI_CLK_OFF(phy); } I915_WRITE(DPCLKA_CFGCR0_ICL, val); diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c index a4172595c8d8..065feb917db4 100644 --- a/drivers/gpu/drm/i915/display/intel_ddi.c +++ b/drivers/gpu/drm/i915/display/intel_ddi.c @@ -2729,12 +2729,13 @@ u32 ddi_signal_levels(struct intel_dp *intel_dp) static inline u32 icl_dpclka_cfgcr0_clk_off(struct drm_i915_private *dev_priv, - enum port port) + enum phy phy) { - if (intel_port_is_combophy(dev_priv, port)) { - return ICL_DPCLKA_CFGCR0_DDI_CLK_OFF(port); - } else if (intel_port_is_tc(dev_priv, port)) { - enum tc_port tc_port = intel_port_to_tc(dev_priv, port); + if (intel_phy_is_combo(dev_priv, phy)) { + return ICL_DPCLKA_CFGCR0_DDI_CLK_OFF(phy); + } else if (intel_phy_is_tc(dev_priv, phy)) { + enum tc_port tc_port = intel_port_to_tc(dev_priv, + (enum port)phy); return ICL_DPCLKA_CFGCR0_TC_CLK_OFF(tc_port); } @@ -2747,22 +2748,32 @@ static void icl_map_plls_to_ports(struct intel_encoder *encoder, { struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); struct intel_shared_dpll *pll = crtc_state->shared_dpll; - enum port port = encoder->port; + enum phy phy = intel_port_to_phy(dev_priv, encoder->port); u32 val; mutex_lock(&dev_priv->dpll_lock); val = I915_READ(DPCLKA_CFGCR0_ICL); - WARN_ON((val & icl_dpclka_cfgcr0_clk_off(dev_priv, port)) == 0); + WARN_ON((val & icl_dpclka_cfgcr0_clk_off(dev_priv, phy)) == 0); - if (intel_port_is_combophy(dev_priv, port)) { - val &= ~DPCLKA_CFGCR0_DDI_CLK_SEL_MASK(port); - val |= DPCLKA_CFGCR0_DDI_CLK_SEL(pll->info->id, port); + if (intel_phy_is_combo(dev_priv, phy)) { + /* + * Even though this register references DDIs, note that we + * want to pass the PHY rather than the port (DDI). For + * ICL, port=phy in all cases so it doesn't matter, but for + * EHL the bspec notes the following: + * + * "DDID clock tied to DDIA clock, so DPCLKA_CFGCR0 DDIA + * Clock Select chooses the PLL for both DDIA and DDID and + * drives port A in all cases." + */ + val &= ~ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_MASK(phy); + val |= ICL_DPCLKA_CFGCR0_DDI_CLK_SEL(pll->info->id, phy); I915_WRITE(DPCLKA_CFGCR0_ICL, val); POSTING_READ(DPCLKA_CFGCR0_ICL); } - val &= ~icl_dpclka_cfgcr0_clk_off(dev_priv, port); + val &= ~icl_dpclka_cfgcr0_clk_off(dev_priv, phy); I915_WRITE(DPCLKA_CFGCR0_ICL, val); mutex_unlock(&dev_priv->dpll_lock); @@ -2771,13 +2782,13 @@ static void icl_map_plls_to_ports(struct intel_encoder *encoder, static void icl_unmap_plls_to_ports(struct intel_encoder *encoder) { struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); - enum port port = encoder->port; + enum phy phy = intel_port_to_phy(dev_priv, encoder->port); u32 val; mutex_lock(&dev_priv->dpll_lock); val = I915_READ(DPCLKA_CFGCR0_ICL); - val |= icl_dpclka_cfgcr0_clk_off(dev_priv, port); + val |= icl_dpclka_cfgcr0_clk_off(dev_priv, phy); I915_WRITE(DPCLKA_CFGCR0_ICL, val); mutex_unlock(&dev_priv->dpll_lock); @@ -2838,9 +2849,11 @@ void icl_sanitize_encoder_pll_mapping(struct intel_encoder *encoder) val = I915_READ(DPCLKA_CFGCR0_ICL); for_each_port_masked(port, port_mask) { + enum phy phy = intel_port_to_phy(dev_priv, port); + bool ddi_clk_ungated = !(val & icl_dpclka_cfgcr0_clk_off(dev_priv, - port)); + phy)); if (ddi_clk_needed == ddi_clk_ungated) continue; @@ -2852,9 +2865,9 @@ void icl_sanitize_encoder_pll_mapping(struct intel_encoder *encoder) if (WARN_ON(ddi_clk_needed)) continue; - DRM_NOTE("Port %c is disabled/in DSI mode with an ungated DDI clock, gate it\n", - port_name(port)); - val |= icl_dpclka_cfgcr0_clk_off(dev_priv, port); + DRM_NOTE("PHY %c is disabled/in DSI mode with an ungated DDI clock, gate it\n", + phy_name(port)); + val |= icl_dpclka_cfgcr0_clk_off(dev_priv, phy); I915_WRITE(DPCLKA_CFGCR0_ICL, val); } } diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index c814cc1b3ae5..c9e2e09b6f01 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -9680,17 +9680,21 @@ enum skl_power_gate { * CNL Clocks */ #define DPCLKA_CFGCR0 _MMIO(0x6C200) -#define DPCLKA_CFGCR0_ICL _MMIO(0x164280) #define DPCLKA_CFGCR0_DDI_CLK_OFF(port) (1 << ((port) == PORT_F ? 23 : \ (port) + 10)) -#define ICL_DPCLKA_CFGCR0_DDI_CLK_OFF(port) (1 << ((port) + 10)) -#define ICL_DPCLKA_CFGCR0_TC_CLK_OFF(tc_port) (1 << ((tc_port) == PORT_TC4 ? \ - 21 : (tc_port) + 12)) #define DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(port) ((port) == PORT_F ? 21 : \ (port) * 2) #define DPCLKA_CFGCR0_DDI_CLK_SEL_MASK(port) (3 << DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(port)) #define DPCLKA_CFGCR0_DDI_CLK_SEL(pll, port) ((pll) << DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(port)) +#define DPCLKA_CFGCR0_ICL _MMIO(0x164280) +#define ICL_DPCLKA_CFGCR0_DDI_CLK_OFF(phy) (1 << _PICK(phy, 10, 11, 24)) +#define ICL_DPCLKA_CFGCR0_TC_CLK_OFF(tc_port) (1 << ((tc_port) == PORT_TC4 ? \ + 21 : (tc_port) + 12)) +#define ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(phy) ((phy) * 2) +#define ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_MASK(phy) (3 << ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(phy)) +#define ICL_DPCLKA_CFGCR0_DDI_CLK_SEL(pll, phy) ((pll) << ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(phy)) + /* CNL PLL */ #define DPLL0_ENABLE 0x46010 #define DPLL1_ENABLE 0x46014
Although the register name implies that it operates on DDI's, DPCLKA_CFGCR0_ICL actually needs to be programmed according to the PHY that's in use. I.e., when using EHL's DDI-D on combo PHY A, the bits described as "port A" in the bspec are what we need to set. The bspec clarifies: "[For EHL] DDID clock tied to DDIA clock, so DPCLKA_CFGCR0 DDIA Clock Select chooses the PLL for both DDIA and DDID and drives port A in all cases." Also, since the CNL DPCLKA_CFGCR0 bit defines are still port-based, we create separate ICL-specific defines that accept the PHY rather than trying to share the same bit definitions between CNL and ICL. v5: Make icl_dpclka_cfgcr0_clk_off() take phy rather than port. When splitting the original patch the hunk to handle this wound up too late in the series. (Sparse) Bspec: 33148 Cc: José Roberto de Souza <jose.souza@intel.com> Signed-off-by: Matt Roper <matthew.d.roper@intel.com> --- drivers/gpu/drm/i915/display/icl_dsi.c | 17 ++++++--- drivers/gpu/drm/i915/display/intel_ddi.c | 47 +++++++++++++++--------- drivers/gpu/drm/i915/i915_reg.h | 12 ++++-- 3 files changed, 50 insertions(+), 26 deletions(-)