diff mbox series

[v2,15/17] drm/i915: Clean up jsl/ehl buf trans functions

Message ID 20210608073603.2408-16-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915: DDI buf trans cleaup and fixes | expand

Commit Message

Ville Syrjälä June 8, 2021, 7:36 a.m. UTC 
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The jsl/ehl buf trans functions are needlessly conplicated.
Simplify them.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 .../drm/i915/display/intel_ddi_buf_trans.c    | 87 +++++--------------
 1 file changed, 20 insertions(+), 67 deletions(-)

Comments

Jani Nikula June 23, 2021, 2:13 p.m. UTC | #1 
On Tue, 08 Jun 2021, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> The jsl/ehl buf trans functions are needlessly conplicated.
                                                   ^

My only disappointment here is that now some of the
*_get_combo_buf_trans_edp() functions handle low vswing inside, and some
expect to only be called for low vswing.

At least cnl could switch to same style as here, the rest get more
complicated.

Not a big issue, and the code is easy enough to follow for each
individual platform. And I like the reduction in call depth.

Reviewed-by: Jani Nikula <jani.nikula@intel.com>


> Simplify them.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  .../drm/i915/display/intel_ddi_buf_trans.c    | 87 +++++--------------
>  1 file changed, 20 insertions(+), 67 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi_buf_trans.c b/drivers/gpu/drm/i915/display/intel_ddi_buf_trans.c
> index 9398aa62585b..2bd51ce4aa2c 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi_buf_trans.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi_buf_trans.c
> @@ -1377,42 +1377,16 @@ icl_get_mg_buf_trans(struct intel_encoder *encoder,
>  		return icl_get_mg_buf_trans_dp(encoder, crtc_state, n_entries);
>  }
>  
> -static const struct intel_ddi_buf_trans *
> -ehl_get_combo_buf_trans_hdmi(struct intel_encoder *encoder,
> -			     const struct intel_crtc_state *crtc_state,
> -			     int *n_entries)
> -{
> -	return intel_get_buf_trans(&icl_combo_phy_ddi_translations_hdmi,
> -				   n_entries);
> -}
> -
> -static const struct intel_ddi_buf_trans *
> -ehl_get_combo_buf_trans_dp(struct intel_encoder *encoder,
> -			   const struct intel_crtc_state *crtc_state,
> -			   int *n_entries)
> -{
> -	return intel_get_buf_trans(&ehl_combo_phy_ddi_translations_dp,
> -				   n_entries);
> -}
>  
>  static const struct intel_ddi_buf_trans *
>  ehl_get_combo_buf_trans_edp(struct intel_encoder *encoder,
>  			    const struct intel_crtc_state *crtc_state,
>  			    int *n_entries)
>  {
> -	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> -
> -	if (dev_priv->vbt.edp.low_vswing) {
> -		if (crtc_state->port_clock > 270000) {
> -			return intel_get_buf_trans(&ehl_combo_phy_ddi_translations_edp_hbr2,
> -						   n_entries);
> -		} else {
> -			return intel_get_buf_trans(&icl_combo_phy_ddi_translations_edp_hbr2,
> -						   n_entries);
> -		}
> -	}
> -
> -	return ehl_get_combo_buf_trans_dp(encoder, crtc_state, n_entries);
> +	if (crtc_state->port_clock > 270000)
> +		return intel_get_buf_trans(&ehl_combo_phy_ddi_translations_edp_hbr2, n_entries);
> +	else
> +		return intel_get_buf_trans(&icl_combo_phy_ddi_translations_edp_hbr2, n_entries);
>  }
>  
>  static const struct intel_ddi_buf_trans *
> @@ -1420,30 +1394,15 @@ ehl_get_combo_buf_trans(struct intel_encoder *encoder,
>  			const struct intel_crtc_state *crtc_state,
>  			int *n_entries)
>  {
> +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> +
>  	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
> -		return ehl_get_combo_buf_trans_hdmi(encoder, crtc_state, n_entries);
> -	else if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_EDP))
> +		return intel_get_buf_trans(&icl_combo_phy_ddi_translations_hdmi, n_entries);
> +	else if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_EDP) &&
> +		 dev_priv->vbt.edp.low_vswing)
>  		return ehl_get_combo_buf_trans_edp(encoder, crtc_state, n_entries);
>  	else
> -		return ehl_get_combo_buf_trans_dp(encoder, crtc_state, n_entries);
> -}
> -
> -static const struct intel_ddi_buf_trans *
> -jsl_get_combo_buf_trans_hdmi(struct intel_encoder *encoder,
> -			     const struct intel_crtc_state *crtc_state,
> -			     int *n_entries)
> -{
> -	return intel_get_buf_trans(&icl_combo_phy_ddi_translations_hdmi,
> -				   n_entries);
> -}
> -
> -static const struct intel_ddi_buf_trans *
> -jsl_get_combo_buf_trans_dp(struct intel_encoder *encoder,
> -			   const struct intel_crtc_state *crtc_state,
> -			   int *n_entries)
> -{
> -	return intel_get_buf_trans(&icl_combo_phy_ddi_translations_dp_hbr2_edp_hbr3,
> -				   n_entries);
> +		return intel_get_buf_trans(&ehl_combo_phy_ddi_translations_dp, n_entries);
>  }
>  
>  static const struct intel_ddi_buf_trans *
> @@ -1451,19 +1410,10 @@ jsl_get_combo_buf_trans_edp(struct intel_encoder *encoder,
>  			    const struct intel_crtc_state *crtc_state,
>  			    int *n_entries)
>  {
> -	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> -
> -	if (dev_priv->vbt.edp.low_vswing) {
> -		if (crtc_state->port_clock > 270000) {
> -			return intel_get_buf_trans(&jsl_combo_phy_ddi_translations_edp_hbr2,
> -						   n_entries);
> -		} else {
> -			return intel_get_buf_trans(&jsl_combo_phy_ddi_translations_edp_hbr,
> -						   n_entries);
> -		}
> -	}
> -
> -	return jsl_get_combo_buf_trans_dp(encoder, crtc_state, n_entries);
> +	if (crtc_state->port_clock > 270000)
> +		return intel_get_buf_trans(&jsl_combo_phy_ddi_translations_edp_hbr2, n_entries);
> +	else
> +		return intel_get_buf_trans(&jsl_combo_phy_ddi_translations_edp_hbr, n_entries);
>  }
>  
>  static const struct intel_ddi_buf_trans *
> @@ -1471,12 +1421,15 @@ jsl_get_combo_buf_trans(struct intel_encoder *encoder,
>  			const struct intel_crtc_state *crtc_state,
>  			int *n_entries)
>  {
> +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> +
>  	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
> -		return jsl_get_combo_buf_trans_hdmi(encoder, crtc_state, n_entries);
> -	else if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_EDP))
> +		return intel_get_buf_trans(&icl_combo_phy_ddi_translations_hdmi, n_entries);
> +	else if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_EDP) &&
> +		 dev_priv->vbt.edp.low_vswing)
>  		return jsl_get_combo_buf_trans_edp(encoder, crtc_state, n_entries);
>  	else
> -		return jsl_get_combo_buf_trans_dp(encoder, crtc_state, n_entries);
> +		return intel_get_buf_trans(&icl_combo_phy_ddi_translations_dp_hbr2_edp_hbr3, n_entries);
>  }
>  
>  static const struct intel_ddi_buf_trans *
Ville Syrjälä June 24, 2021, 5:05 p.m. UTC | #2 
On Wed, Jun 23, 2021 at 05:13:53PM +0300, Jani Nikula wrote:
> On Tue, 08 Jun 2021, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > The jsl/ehl buf trans functions are needlessly conplicated.
>                                                    ^
> 
> My only disappointment here is that now some of the
> *_get_combo_buf_trans_edp() functions handle low vswing inside, and some
> expect to only be called for low vswing.

Yeah, that is a bit annoying. Not really sure what the best approach is
for everything :/

> 
> At least cnl could switch to same style as here, the rest get more
> complicated.
> 
> Not a big issue, and the code is easy enough to follow for each
> individual platform. And I like the reduction in call depth.
> 
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>

Ta.

> 
> 
> > Simplify them.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  .../drm/i915/display/intel_ddi_buf_trans.c    | 87 +++++--------------
> >  1 file changed, 20 insertions(+), 67 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi_buf_trans.c b/drivers/gpu/drm/i915/display/intel_ddi_buf_trans.c
> > index 9398aa62585b..2bd51ce4aa2c 100644
> > --- a/drivers/gpu/drm/i915/display/intel_ddi_buf_trans.c
> > +++ b/drivers/gpu/drm/i915/display/intel_ddi_buf_trans.c
> > @@ -1377,42 +1377,16 @@ icl_get_mg_buf_trans(struct intel_encoder *encoder,
> >  		return icl_get_mg_buf_trans_dp(encoder, crtc_state, n_entries);
> >  }
> >  
> > -static const struct intel_ddi_buf_trans *
> > -ehl_get_combo_buf_trans_hdmi(struct intel_encoder *encoder,
> > -			     const struct intel_crtc_state *crtc_state,
> > -			     int *n_entries)
> > -{
> > -	return intel_get_buf_trans(&icl_combo_phy_ddi_translations_hdmi,
> > -				   n_entries);
> > -}
> > -
> > -static const struct intel_ddi_buf_trans *
> > -ehl_get_combo_buf_trans_dp(struct intel_encoder *encoder,
> > -			   const struct intel_crtc_state *crtc_state,
> > -			   int *n_entries)
> > -{
> > -	return intel_get_buf_trans(&ehl_combo_phy_ddi_translations_dp,
> > -				   n_entries);
> > -}
> >  
> >  static const struct intel_ddi_buf_trans *
> >  ehl_get_combo_buf_trans_edp(struct intel_encoder *encoder,
> >  			    const struct intel_crtc_state *crtc_state,
> >  			    int *n_entries)
> >  {
> > -	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > -
> > -	if (dev_priv->vbt.edp.low_vswing) {
> > -		if (crtc_state->port_clock > 270000) {
> > -			return intel_get_buf_trans(&ehl_combo_phy_ddi_translations_edp_hbr2,
> > -						   n_entries);
> > -		} else {
> > -			return intel_get_buf_trans(&icl_combo_phy_ddi_translations_edp_hbr2,
> > -						   n_entries);
> > -		}
> > -	}
> > -
> > -	return ehl_get_combo_buf_trans_dp(encoder, crtc_state, n_entries);
> > +	if (crtc_state->port_clock > 270000)
> > +		return intel_get_buf_trans(&ehl_combo_phy_ddi_translations_edp_hbr2, n_entries);
> > +	else
> > +		return intel_get_buf_trans(&icl_combo_phy_ddi_translations_edp_hbr2, n_entries);
> >  }
> >  
> >  static const struct intel_ddi_buf_trans *
> > @@ -1420,30 +1394,15 @@ ehl_get_combo_buf_trans(struct intel_encoder *encoder,
> >  			const struct intel_crtc_state *crtc_state,
> >  			int *n_entries)
> >  {
> > +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > +
> >  	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
> > -		return ehl_get_combo_buf_trans_hdmi(encoder, crtc_state, n_entries);
> > -	else if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_EDP))
> > +		return intel_get_buf_trans(&icl_combo_phy_ddi_translations_hdmi, n_entries);
> > +	else if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_EDP) &&
> > +		 dev_priv->vbt.edp.low_vswing)
> >  		return ehl_get_combo_buf_trans_edp(encoder, crtc_state, n_entries);
> >  	else
> > -		return ehl_get_combo_buf_trans_dp(encoder, crtc_state, n_entries);
> > -}
> > -
> > -static const struct intel_ddi_buf_trans *
> > -jsl_get_combo_buf_trans_hdmi(struct intel_encoder *encoder,
> > -			     const struct intel_crtc_state *crtc_state,
> > -			     int *n_entries)
> > -{
> > -	return intel_get_buf_trans(&icl_combo_phy_ddi_translations_hdmi,
> > -				   n_entries);
> > -}
> > -
> > -static const struct intel_ddi_buf_trans *
> > -jsl_get_combo_buf_trans_dp(struct intel_encoder *encoder,
> > -			   const struct intel_crtc_state *crtc_state,
> > -			   int *n_entries)
> > -{
> > -	return intel_get_buf_trans(&icl_combo_phy_ddi_translations_dp_hbr2_edp_hbr3,
> > -				   n_entries);
> > +		return intel_get_buf_trans(&ehl_combo_phy_ddi_translations_dp, n_entries);
> >  }
> >  
> >  static const struct intel_ddi_buf_trans *
> > @@ -1451,19 +1410,10 @@ jsl_get_combo_buf_trans_edp(struct intel_encoder *encoder,
> >  			    const struct intel_crtc_state *crtc_state,
> >  			    int *n_entries)
> >  {
> > -	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > -
> > -	if (dev_priv->vbt.edp.low_vswing) {
> > -		if (crtc_state->port_clock > 270000) {
> > -			return intel_get_buf_trans(&jsl_combo_phy_ddi_translations_edp_hbr2,
> > -						   n_entries);
> > -		} else {
> > -			return intel_get_buf_trans(&jsl_combo_phy_ddi_translations_edp_hbr,
> > -						   n_entries);
> > -		}
> > -	}
> > -
> > -	return jsl_get_combo_buf_trans_dp(encoder, crtc_state, n_entries);
> > +	if (crtc_state->port_clock > 270000)
> > +		return intel_get_buf_trans(&jsl_combo_phy_ddi_translations_edp_hbr2, n_entries);
> > +	else
> > +		return intel_get_buf_trans(&jsl_combo_phy_ddi_translations_edp_hbr, n_entries);
> >  }
> >  
> >  static const struct intel_ddi_buf_trans *
> > @@ -1471,12 +1421,15 @@ jsl_get_combo_buf_trans(struct intel_encoder *encoder,
> >  			const struct intel_crtc_state *crtc_state,
> >  			int *n_entries)
> >  {
> > +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > +
> >  	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
> > -		return jsl_get_combo_buf_trans_hdmi(encoder, crtc_state, n_entries);
> > -	else if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_EDP))
> > +		return intel_get_buf_trans(&icl_combo_phy_ddi_translations_hdmi, n_entries);
> > +	else if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_EDP) &&
> > +		 dev_priv->vbt.edp.low_vswing)
> >  		return jsl_get_combo_buf_trans_edp(encoder, crtc_state, n_entries);
> >  	else
> > -		return jsl_get_combo_buf_trans_dp(encoder, crtc_state, n_entries);
> > +		return intel_get_buf_trans(&icl_combo_phy_ddi_translations_dp_hbr2_edp_hbr3, n_entries);
> >  }
> >  
> >  static const struct intel_ddi_buf_trans *
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_ddi_buf_trans.c b/drivers/gpu/drm/i915/display/intel_ddi_buf_trans.c
index 9398aa62585b..2bd51ce4aa2c 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi_buf_trans.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi_buf_trans.c
@@ -1377,42 +1377,16 @@  icl_get_mg_buf_trans(struct intel_encoder *encoder,
 		return icl_get_mg_buf_trans_dp(encoder, crtc_state, n_entries);
 }
 
-static const struct intel_ddi_buf_trans *
-ehl_get_combo_buf_trans_hdmi(struct intel_encoder *encoder,
-			     const struct intel_crtc_state *crtc_state,
-			     int *n_entries)
-{
-	return intel_get_buf_trans(&icl_combo_phy_ddi_translations_hdmi,
-				   n_entries);
-}
-
-static const struct intel_ddi_buf_trans *
-ehl_get_combo_buf_trans_dp(struct intel_encoder *encoder,
-			   const struct intel_crtc_state *crtc_state,
-			   int *n_entries)
-{
-	return intel_get_buf_trans(&ehl_combo_phy_ddi_translations_dp,
-				   n_entries);
-}
 
 static const struct intel_ddi_buf_trans *
 ehl_get_combo_buf_trans_edp(struct intel_encoder *encoder,
 			    const struct intel_crtc_state *crtc_state,
 			    int *n_entries)
 {
-	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
-
-	if (dev_priv->vbt.edp.low_vswing) {
-		if (crtc_state->port_clock > 270000) {
-			return intel_get_buf_trans(&ehl_combo_phy_ddi_translations_edp_hbr2,
-						   n_entries);
-		} else {
-			return intel_get_buf_trans(&icl_combo_phy_ddi_translations_edp_hbr2,
-						   n_entries);
-		}
-	}
-
-	return ehl_get_combo_buf_trans_dp(encoder, crtc_state, n_entries);
+	if (crtc_state->port_clock > 270000)
+		return intel_get_buf_trans(&ehl_combo_phy_ddi_translations_edp_hbr2, n_entries);
+	else
+		return intel_get_buf_trans(&icl_combo_phy_ddi_translations_edp_hbr2, n_entries);
 }
 
 static const struct intel_ddi_buf_trans *
@@ -1420,30 +1394,15 @@  ehl_get_combo_buf_trans(struct intel_encoder *encoder,
 			const struct intel_crtc_state *crtc_state,
 			int *n_entries)
 {
+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+
 	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
-		return ehl_get_combo_buf_trans_hdmi(encoder, crtc_state, n_entries);
-	else if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_EDP))
+		return intel_get_buf_trans(&icl_combo_phy_ddi_translations_hdmi, n_entries);
+	else if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_EDP) &&
+		 dev_priv->vbt.edp.low_vswing)
 		return ehl_get_combo_buf_trans_edp(encoder, crtc_state, n_entries);
 	else
-		return ehl_get_combo_buf_trans_dp(encoder, crtc_state, n_entries);
-}
-
-static const struct intel_ddi_buf_trans *
-jsl_get_combo_buf_trans_hdmi(struct intel_encoder *encoder,
-			     const struct intel_crtc_state *crtc_state,
-			     int *n_entries)
-{
-	return intel_get_buf_trans(&icl_combo_phy_ddi_translations_hdmi,
-				   n_entries);
-}
-
-static const struct intel_ddi_buf_trans *
-jsl_get_combo_buf_trans_dp(struct intel_encoder *encoder,
-			   const struct intel_crtc_state *crtc_state,
-			   int *n_entries)
-{
-	return intel_get_buf_trans(&icl_combo_phy_ddi_translations_dp_hbr2_edp_hbr3,
-				   n_entries);
+		return intel_get_buf_trans(&ehl_combo_phy_ddi_translations_dp, n_entries);
 }
 
 static const struct intel_ddi_buf_trans *
@@ -1451,19 +1410,10 @@  jsl_get_combo_buf_trans_edp(struct intel_encoder *encoder,
 			    const struct intel_crtc_state *crtc_state,
 			    int *n_entries)
 {
-	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
-
-	if (dev_priv->vbt.edp.low_vswing) {
-		if (crtc_state->port_clock > 270000) {
-			return intel_get_buf_trans(&jsl_combo_phy_ddi_translations_edp_hbr2,
-						   n_entries);
-		} else {
-			return intel_get_buf_trans(&jsl_combo_phy_ddi_translations_edp_hbr,
-						   n_entries);
-		}
-	}
-
-	return jsl_get_combo_buf_trans_dp(encoder, crtc_state, n_entries);
+	if (crtc_state->port_clock > 270000)
+		return intel_get_buf_trans(&jsl_combo_phy_ddi_translations_edp_hbr2, n_entries);
+	else
+		return intel_get_buf_trans(&jsl_combo_phy_ddi_translations_edp_hbr, n_entries);
 }
 
 static const struct intel_ddi_buf_trans *
@@ -1471,12 +1421,15 @@  jsl_get_combo_buf_trans(struct intel_encoder *encoder,
 			const struct intel_crtc_state *crtc_state,
 			int *n_entries)
 {
+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+
 	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
-		return jsl_get_combo_buf_trans_hdmi(encoder, crtc_state, n_entries);
-	else if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_EDP))
+		return intel_get_buf_trans(&icl_combo_phy_ddi_translations_hdmi, n_entries);
+	else if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_EDP) &&
+		 dev_priv->vbt.edp.low_vswing)
 		return jsl_get_combo_buf_trans_edp(encoder, crtc_state, n_entries);
 	else
-		return jsl_get_combo_buf_trans_dp(encoder, crtc_state, n_entries);
+		return intel_get_buf_trans(&icl_combo_phy_ddi_translations_dp_hbr2_edp_hbr3, n_entries);
 }
 
 static const struct intel_ddi_buf_trans *