Re: [Intel-gfx] [PATCH v2 15/17] drm/i915: Clean up jsl/ehl buf trans functions

From: Jani Nikula <jani.nikula@linux.intel.com>
To: Ville Syrjala <ville.syrjala@linux.intel.com>,
	intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH v2 15/17] drm/i915: Clean up jsl/ehl buf trans functions
Date: Wed, 23 Jun 2021 17:13:53 +0300	[thread overview]
Message-ID: <87tulou0ri.fsf@intel.com> (raw)
In-Reply-To: <20210608073603.2408-16-ville.syrjala@linux.intel.com>

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 *

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx