[Intel-gfx] [PATCH 13/15] drm/i915: Split alds/rkl from icl_ddi_combo_{enable, disable}_clock()

[Lucas De Marchi]

On Mon, Feb 01, 2021 at 08:33:41PM +0200, Ville Syrjälä wrote:
>From: Ville Syrjälä <ville.syrjala at linux.intel.com>
>
>Since .{enable,disable}_clock() are already vfuncs it's a bit silly to
>have if-ladders inside them. Just provide specialized version for adlp
>and rkl so we don't need any of that.

s/alds/adl-s/

s/adlp/adl-s/


>
>Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
>---
> drivers/gpu/drm/i915/display/intel_ddi.c | 93 ++++++++++++++++--------
> 1 file changed, 62 insertions(+), 31 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
>index 1bd2aa86183d..bafb754d1b66 100644
>--- a/drivers/gpu/drm/i915/display/intel_ddi.c
>+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
>@@ -3127,28 +3127,6 @@ static u32 icl_dpclka_cfgcr0_clk_off(struct drm_i915_private *dev_priv,
> 	return 0;
> }
>
>-static u32 icl_dpclka_cfgcr0_clk_sel(struct drm_i915_private *dev_priv,
>-				     enum intel_dpll_id id, enum phy phy)

ok, but why do we even add them in this series if we are going to
remove. Just makes it harder to review.

For this end state:


Reviewed-by: Lucas De Marchi <lucas.demarchi at intel.com>

Lucas De Marchi

>-{
>-	if (IS_ALDERLAKE_S(dev_priv))
>-		return id << ADLS_DPCLKA_CFGCR_DDI_SHIFT(phy);
>-	else if (IS_ROCKETLAKE(dev_priv))
>-		return RKL_DPCLKA_CFGCR0_DDI_CLK_SEL(id, phy);
>-	else
>-		return ICL_DPCLKA_CFGCR0_DDI_CLK_SEL(id, phy);
>-}
>-
>-static u32 icl_dpclka_cfgcr0_clk_sel_mask(struct drm_i915_private *dev_priv,
>-					  enum phy phy)
>-{
>-	if (IS_ALDERLAKE_S(dev_priv))
>-		return ADLS_DPCLKA_CFGCR_DDI_CLK_SEL_MASK(phy);
>-	else if (IS_ROCKETLAKE(dev_priv))
>-		return RKL_DPCLKA_CFGCR0_DDI_CLK_SEL_MASK(phy);
>-	else
>-		return ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_MASK(phy);
>-}
>-
> static i915_reg_t icl_dpclka_cfgcr0_reg(struct drm_i915_private *i915,
> 					enum phy phy)
> {
>@@ -3184,6 +3162,56 @@ static void _cnl_ddi_disable_clock(struct drm_i915_private *i915, i915_reg_t reg
> 	mutex_unlock(&i915->dpll.lock);
> }
>
>+static void adls_ddi_enable_clock(struct intel_encoder *encoder,
>+				  const struct intel_crtc_state *crtc_state)
>+{
>+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>+	const struct intel_shared_dpll *pll = crtc_state->shared_dpll;
>+	enum phy phy = intel_port_to_phy(dev_priv, encoder->port);
>+
>+	if (drm_WARN_ON(&dev_priv->drm, !pll))
>+		return;
>+
>+	_cnl_ddi_enable_clock(dev_priv, ADLS_DPCLKA_CFGCR(phy),
>+			      ADLS_DPCLKA_CFGCR_DDI_CLK_SEL_MASK(phy),
>+			      pll->info->id << ADLS_DPCLKA_CFGCR_DDI_SHIFT(phy),
>+			      ICL_DPCLKA_CFGCR0_DDI_CLK_OFF(phy));
>+}
>+
>+static void adls_ddi_disable_clock(struct intel_encoder *encoder)
>+{
>+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>+	enum phy phy = intel_port_to_phy(dev_priv, encoder->port);
>+
>+	_cnl_ddi_disable_clock(dev_priv, ADLS_DPCLKA_CFGCR(phy),
>+			       ICL_DPCLKA_CFGCR0_DDI_CLK_OFF(phy));
>+}
>+
>+static void rkl_ddi_enable_clock(struct intel_encoder *encoder,
>+				 const struct intel_crtc_state *crtc_state)
>+{
>+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>+	const struct intel_shared_dpll *pll = crtc_state->shared_dpll;
>+	enum phy phy = intel_port_to_phy(dev_priv, encoder->port);
>+
>+	if (drm_WARN_ON(&dev_priv->drm, !pll))
>+		return;
>+
>+	_cnl_ddi_enable_clock(dev_priv, ICL_DPCLKA_CFGCR0,
>+			      RKL_DPCLKA_CFGCR0_DDI_CLK_SEL_MASK(phy),
>+			      RKL_DPCLKA_CFGCR0_DDI_CLK_SEL(pll->info->id, phy),
>+			      RKL_DPCLKA_CFGCR0_DDI_CLK_OFF(phy));
>+}
>+
>+static void rkl_ddi_disable_clock(struct intel_encoder *encoder)
>+{
>+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>+	enum phy phy = intel_port_to_phy(dev_priv, encoder->port);
>+
>+	_cnl_ddi_disable_clock(dev_priv, ICL_DPCLKA_CFGCR0,
>+			       RKL_DPCLKA_CFGCR0_DDI_CLK_OFF(phy));
>+}
>+
> static void dg1_ddi_enable_clock(struct intel_encoder *encoder,
> 				 const struct intel_crtc_state *crtc_state)
> {
>@@ -3228,10 +3256,10 @@ static void icl_ddi_combo_enable_clock(struct intel_encoder *encoder,
> 	if (drm_WARN_ON(&dev_priv->drm, !pll))
> 		return;
>
>-	_cnl_ddi_enable_clock(dev_priv, icl_dpclka_cfgcr0_reg(dev_priv, phy),
>-			      icl_dpclka_cfgcr0_clk_sel_mask(dev_priv, phy),
>-			      icl_dpclka_cfgcr0_clk_sel(dev_priv, pll->info->id, phy),
>-			      icl_dpclka_cfgcr0_clk_off(dev_priv, phy));
>+	_cnl_ddi_enable_clock(dev_priv, ICL_DPCLKA_CFGCR0,
>+			      ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_MASK(phy),
>+			      ICL_DPCLKA_CFGCR0_DDI_CLK_SEL(pll->info->id, phy),
>+			      ICL_DPCLKA_CFGCR0_DDI_CLK_OFF(phy));
> }
>
> static void icl_ddi_combo_disable_clock(struct intel_encoder *encoder)
>@@ -3239,8 +3267,8 @@ static void icl_ddi_combo_disable_clock(struct intel_encoder *encoder)
> 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> 	enum phy phy = intel_port_to_phy(dev_priv, encoder->port);
>
>-	_cnl_ddi_disable_clock(dev_priv, icl_dpclka_cfgcr0_reg(dev_priv, phy),
>-			       icl_dpclka_cfgcr0_clk_off(dev_priv, phy));
>+	_cnl_ddi_disable_clock(dev_priv, ICL_DPCLKA_CFGCR0,
>+			       ICL_DPCLKA_CFGCR0_DDI_CLK_OFF(phy));
> }
>
> static void dg1_sanitize_port_clk_off(struct drm_i915_private *dev_priv,
>@@ -5654,9 +5682,12 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
> 	encoder->cloneable = 0;
> 	encoder->pipe_mask = ~0;
>
>-	if (IS_ALDERLAKE_S(dev_priv) || IS_ROCKETLAKE(dev_priv)) {
>-		encoder->enable_clock = icl_ddi_combo_enable_clock;
>-		encoder->disable_clock = icl_ddi_combo_disable_clock;
>+	if (IS_ALDERLAKE_S(dev_priv)) {
>+		encoder->enable_clock = adls_ddi_enable_clock;
>+		encoder->disable_clock = adls_ddi_disable_clock;
>+	} else if (IS_ROCKETLAKE(dev_priv)) {
>+		encoder->enable_clock = rkl_ddi_enable_clock;
>+		encoder->disable_clock = rkl_ddi_disable_clock;
> 	} else if (IS_DG1(dev_priv)) {
> 		encoder->enable_clock = dg1_ddi_enable_clock;
> 		encoder->disable_clock = dg1_ddi_disable_clock;
>-- 
>2.26.2
>
>_______________________________________________
>Intel-gfx mailing list
>Intel-gfx at lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/intel-gfx