All of lore.kernel.org
 help / color / mirror / Atom feed
From: Imre Deak <imre.deak@intel.com>
To: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 3/9] drm/i915: Nuke usless .set_signal_levels() wrappers
Date: Wed, 29 Sep 2021 22:43:39 +0300	[thread overview]
Message-ID: <20210929194339.GE2192289@ideak-desk.fi.intel.com> (raw)
In-Reply-To: <20210927182455.27119-4-ville.syrjala@linux.intel.com>

On Mon, Sep 27, 2021 at 09:24:49PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Now that .set_signal_levels() is used for HDMI as well, we can
> remove the extra level of indirection and just plug the correct
> stuff straight into .set_signal_levels().
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Imre Deak <imre.deak@intel.com>

> ---
>  drivers/gpu/drm/i915/display/intel_ddi.c      | 119 +++++-------------
>  drivers/gpu/drm/i915/display/intel_ddi.h      |   6 +-
>  drivers/gpu/drm/i915/display/intel_snps_phy.c |   7 +-
>  drivers/gpu/drm/i915/display/intel_snps_phy.h |   5 +-
>  4 files changed, 39 insertions(+), 98 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 4a22dcde66d9..62ab57c391ef 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -995,11 +995,11 @@ static void skl_ddi_set_iboost(struct intel_encoder *encoder,
>  		_skl_ddi_set_iboost(dev_priv, PORT_E, iboost);
>  }
>  
> -static void bxt_ddi_vswing_sequence(struct intel_encoder *encoder,
> -				    const struct intel_crtc_state *crtc_state,
> -				    int level)
> +static void bxt_set_signal_levels(struct intel_encoder *encoder,
> +				  const struct intel_crtc_state *crtc_state)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> +	int level = intel_ddi_level(encoder, crtc_state);
>  	const struct intel_ddi_buf_trans *trans;
>  	enum port port = encoder->port;
>  	int n_entries;
> @@ -1047,10 +1047,10 @@ static u8 intel_ddi_dp_preemph_max(struct intel_dp *intel_dp)
>  }
>  
>  static void icl_ddi_combo_vswing_program(struct intel_encoder *encoder,
> -					 const struct intel_crtc_state *crtc_state,
> -					 int level)
> +					 const struct intel_crtc_state *crtc_state)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> +	int level = intel_ddi_level(encoder, crtc_state);
>  	const struct intel_ddi_buf_trans *trans;
>  	enum phy phy = intel_port_to_phy(dev_priv, encoder->port);
>  	int n_entries, ln;
> @@ -1109,9 +1109,8 @@ static void icl_ddi_combo_vswing_program(struct intel_encoder *encoder,
>  	intel_de_write(dev_priv, ICL_PORT_TX_DW7_GRP(phy), val);
>  }
>  
> -static void icl_combo_phy_ddi_vswing_sequence(struct intel_encoder *encoder,
> -					      const struct intel_crtc_state *crtc_state,
> -					      int level)
> +static void icl_combo_phy_set_signal_levels(struct intel_encoder *encoder,
> +					    const struct intel_crtc_state *crtc_state)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>  	enum phy phy = intel_port_to_phy(dev_priv, encoder->port);
> @@ -1162,7 +1161,7 @@ static void icl_combo_phy_ddi_vswing_sequence(struct intel_encoder *encoder,
>  	intel_de_write(dev_priv, ICL_PORT_TX_DW5_GRP(phy), val);
>  
>  	/* 5. Program swing and de-emphasis */
> -	icl_ddi_combo_vswing_program(encoder, crtc_state, level);
> +	icl_ddi_combo_vswing_program(encoder, crtc_state);
>  
>  	/* 6. Set training enable to trigger update */
>  	val = intel_de_read(dev_priv, ICL_PORT_TX_DW5_LN0(phy));
> @@ -1170,12 +1169,12 @@ static void icl_combo_phy_ddi_vswing_sequence(struct intel_encoder *encoder,
>  	intel_de_write(dev_priv, ICL_PORT_TX_DW5_GRP(phy), val);
>  }
>  
> -static void icl_mg_phy_ddi_vswing_sequence(struct intel_encoder *encoder,
> -					   const struct intel_crtc_state *crtc_state,
> -					   int level)
> +static void icl_mg_phy_set_signal_levels(struct intel_encoder *encoder,
> +					 const struct intel_crtc_state *crtc_state)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>  	enum tc_port tc_port = intel_port_to_tc(dev_priv, encoder->port);
> +	int level = intel_ddi_level(encoder, crtc_state);
>  	const struct intel_ddi_buf_trans *trans;
>  	int n_entries, ln;
>  	u32 val;
> @@ -1293,26 +1292,12 @@ static void icl_mg_phy_ddi_vswing_sequence(struct intel_encoder *encoder,
>  	}
>  }
>  
> -static void icl_ddi_vswing_sequence(struct intel_encoder *encoder,
> -				    const struct intel_crtc_state *crtc_state,
> -				    int level)
> -{
> -	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> -	enum phy phy = intel_port_to_phy(dev_priv, encoder->port);
> -
> -	if (intel_phy_is_combo(dev_priv, phy))
> -		icl_combo_phy_ddi_vswing_sequence(encoder, crtc_state, level);
> -	else
> -		icl_mg_phy_ddi_vswing_sequence(encoder, crtc_state, level);
> -}
> -
> -static void
> -tgl_dkl_phy_ddi_vswing_sequence(struct intel_encoder *encoder,
> -				const struct intel_crtc_state *crtc_state,
> -				int level)
> +static void tgl_dkl_phy_set_signal_levels(struct intel_encoder *encoder,
> +					  const struct intel_crtc_state *crtc_state)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>  	enum tc_port tc_port = intel_port_to_tc(dev_priv, encoder->port);
> +	int level = intel_ddi_level(encoder, crtc_state);
>  	const struct intel_ddi_buf_trans *trans;
>  	u32 val, dpcnt_mask, dpcnt_val;
>  	int n_entries, ln;
> @@ -1364,19 +1349,6 @@ tgl_dkl_phy_ddi_vswing_sequence(struct intel_encoder *encoder,
>  	}
>  }
>  
> -static void tgl_ddi_vswing_sequence(struct intel_encoder *encoder,
> -				    const struct intel_crtc_state *crtc_state,
> -				    int level)
> -{
> -	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> -	enum phy phy = intel_port_to_phy(dev_priv, encoder->port);
> -
> -	if (intel_phy_is_combo(dev_priv, phy))
> -		icl_combo_phy_ddi_vswing_sequence(encoder, crtc_state, level);
> -	else
> -		tgl_dkl_phy_ddi_vswing_sequence(encoder, crtc_state, level);
> -}
> -
>  static int translate_signal_level(struct intel_dp *intel_dp,
>  				  u8 signal_levels)
>  {
> @@ -1404,8 +1376,8 @@ static int intel_ddi_dp_level(struct intel_dp *intel_dp)
>  	return translate_signal_level(intel_dp, signal_levels);
>  }
>  
> -static int intel_ddi_level(struct intel_encoder *encoder,
> -			   const struct intel_crtc_state *crtc_state)
> +int intel_ddi_level(struct intel_encoder *encoder,
> +		    const struct intel_crtc_state *crtc_state)
>  {
>  	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
>  		return intel_ddi_hdmi_level(encoder, crtc_state);
> @@ -1413,42 +1385,6 @@ static int intel_ddi_level(struct intel_encoder *encoder,
>  		return intel_ddi_dp_level(enc_to_intel_dp(encoder));
>  }
>  
> -static void
> -dg2_set_signal_levels(struct intel_encoder *encoder,
> -		      const struct intel_crtc_state *crtc_state)
> -{
> -	int level = intel_ddi_level(encoder, crtc_state);
> -
> -	intel_snps_phy_ddi_vswing_sequence(encoder, crtc_state, level);
> -}
> -
> -static void
> -tgl_set_signal_levels(struct intel_encoder *encoder,
> -		      const struct intel_crtc_state *crtc_state)
> -{
> -	int level = intel_ddi_level(encoder, crtc_state);
> -
> -	tgl_ddi_vswing_sequence(encoder, crtc_state, level);
> -}
> -
> -static void
> -icl_set_signal_levels(struct intel_encoder *encoder,
> -		      const struct intel_crtc_state *crtc_state)
> -{
> -	int level = intel_ddi_level(encoder, crtc_state);
> -
> -	icl_ddi_vswing_sequence(encoder, crtc_state, level);
> -}
> -
> -static void
> -bxt_set_signal_levels(struct intel_encoder *encoder,
> -		      const struct intel_crtc_state *crtc_state)
> -{
> -	int level = intel_ddi_level(encoder, crtc_state);
> -
> -	bxt_ddi_vswing_sequence(encoder, crtc_state, level);
> -}
> -
>  static void
>  hsw_set_signal_levels(struct intel_encoder *encoder,
>  		      const struct intel_crtc_state *crtc_state)
> @@ -4625,16 +4561,23 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
>  		encoder->get_config = hsw_ddi_get_config;
>  	}
>  
> -	if (IS_DG2(dev_priv))
> -		encoder->set_signal_levels = dg2_set_signal_levels;
> -	else if (DISPLAY_VER(dev_priv) >= 12)
> -		encoder->set_signal_levels = tgl_set_signal_levels;
> -	else if (DISPLAY_VER(dev_priv) >= 11)
> -		encoder->set_signal_levels = icl_set_signal_levels;
> -	else if (IS_GEMINILAKE(dev_priv) || IS_BROXTON(dev_priv))
> +	if (IS_DG2(dev_priv)) {
> +		encoder->set_signal_levels = intel_snps_phy_set_signal_levels;
> +	} else if (DISPLAY_VER(dev_priv) >= 12) {
> +		if (intel_phy_is_combo(dev_priv, phy))
> +			encoder->set_signal_levels = icl_combo_phy_set_signal_levels;
> +		else
> +			encoder->set_signal_levels = tgl_dkl_phy_set_signal_levels;
> +	} else if (DISPLAY_VER(dev_priv) >= 11) {
> +		if (intel_phy_is_combo(dev_priv, phy))
> +			encoder->set_signal_levels = icl_combo_phy_set_signal_levels;
> +		else
> +			encoder->set_signal_levels = icl_mg_phy_set_signal_levels;
> +	} else if (IS_GEMINILAKE(dev_priv) || IS_BROXTON(dev_priv)) {
>  		encoder->set_signal_levels = bxt_set_signal_levels;
> -	else
> +	} else {
>  		encoder->set_signal_levels = hsw_set_signal_levels;
> +	}
>  
>  	intel_ddi_buf_trans_init(encoder);
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.h b/drivers/gpu/drm/i915/display/intel_ddi.h
> index 7d448485d887..d6947c06a455 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.h
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.h
> @@ -59,13 +59,11 @@ void intel_ddi_set_vc_payload_alloc(const struct intel_crtc_state *crtc_state,
>  				    bool state);
>  void intel_ddi_compute_min_voltage_level(struct drm_i915_private *dev_priv,
>  					 struct intel_crtc_state *crtc_state);
> -u32 bxt_signal_levels(struct intel_dp *intel_dp,
> -		      const struct intel_crtc_state *crtc_state);
> -u32 ddi_signal_levels(struct intel_dp *intel_dp,
> -		      const struct intel_crtc_state *crtc_state);
>  int intel_ddi_toggle_hdcp_bits(struct intel_encoder *intel_encoder,
>  			       enum transcoder cpu_transcoder,
>  			       bool enable, u32 hdcp_mask);
>  void intel_ddi_sanitize_encoder_pll_mapping(struct intel_encoder *encoder);
> +int intel_ddi_level(struct intel_encoder *encoder,
> +		    const struct intel_crtc_state *crtc_state);
>  
>  #endif /* __INTEL_DDI_H__ */
> diff --git a/drivers/gpu/drm/i915/display/intel_snps_phy.c b/drivers/gpu/drm/i915/display/intel_snps_phy.c
> index 3734e349f91d..f59cc320ce9c 100644
> --- a/drivers/gpu/drm/i915/display/intel_snps_phy.c
> +++ b/drivers/gpu/drm/i915/display/intel_snps_phy.c
> @@ -5,6 +5,7 @@
>  
>  #include <linux/util_macros.h>
>  
> +#include "intel_ddi.h"
>  #include "intel_ddi_buf_trans.h"
>  #include "intel_de.h"
>  #include "intel_display_types.h"
> @@ -51,13 +52,13 @@ void intel_snps_phy_update_psr_power_state(struct drm_i915_private *dev_priv,
>  			 SNPS_PHY_TX_REQ_LN_DIS_PWR_STATE_PSR, val);
>  }
>  
> -void intel_snps_phy_ddi_vswing_sequence(struct intel_encoder *encoder,
> -					const struct intel_crtc_state *crtc_state,
> -					int level)
> +void intel_snps_phy_set_signal_levels(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_ddi_buf_trans *trans;
>  	enum phy phy = intel_port_to_phy(dev_priv, encoder->port);
> +	int level = intel_ddi_level(encoder, crtc_state);
>  	int n_entries, ln;
>  
>  	trans = encoder->get_buf_trans(encoder, crtc_state, &n_entries);
> diff --git a/drivers/gpu/drm/i915/display/intel_snps_phy.h b/drivers/gpu/drm/i915/display/intel_snps_phy.h
> index a68547a6fee5..11dcd6deb070 100644
> --- a/drivers/gpu/drm/i915/display/intel_snps_phy.h
> +++ b/drivers/gpu/drm/i915/display/intel_snps_phy.h
> @@ -29,8 +29,7 @@ int intel_mpllb_calc_port_clock(struct intel_encoder *encoder,
>  				const struct intel_mpllb_state *pll_state);
>  
>  int intel_snps_phy_check_hdmi_link_rate(int clock);
> -void intel_snps_phy_ddi_vswing_sequence(struct intel_encoder *encoder,
> -					const struct intel_crtc_state *crtc_state,
> -					int level);
> +void intel_snps_phy_set_signal_levels(struct intel_encoder *encoder,
> +				      const struct intel_crtc_state *crtc_state);
>  
>  #endif /* __INTEL_SNPS_PHY_H__ */
> -- 
> 2.32.0
> 

  reply	other threads:[~2021-09-29 19:43 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-27 18:24 [Intel-gfx] [PATCH 0/9] drm/i915: DP per-lane drive settings prep work Ville Syrjala
2021-09-27 18:24 ` [Intel-gfx] [PATCH 1/9] drm/i915: s/ddi_translations/trans/ Ville Syrjala
2021-09-29 16:59   ` Imre Deak
2021-09-27 18:24 ` [Intel-gfx] [PATCH 2/9] drm/i915: Generalize .set_signal_levels() Ville Syrjala
2021-09-29 19:17   ` Imre Deak
2021-09-30  7:33     ` Ville Syrjälä
2021-09-27 18:24 ` [Intel-gfx] [PATCH 3/9] drm/i915: Nuke usless .set_signal_levels() wrappers Ville Syrjala
2021-09-29 19:43   ` Imre Deak [this message]
2021-09-27 18:24 ` [Intel-gfx] [PATCH 4/9] drm/i915: De-wrapper bxt_ddi_phy_set_signal_levels() Ville Syrjala
2021-09-29 19:48   ` Imre Deak
2021-09-27 18:24 ` [Intel-gfx] [PATCH 5/9] drm/i915: Hoover the level>=n_entries WARN into intel_ddi_level() Ville Syrjala
2021-09-29 20:09   ` Imre Deak
2021-09-27 18:24 ` [Intel-gfx] [PATCH 6/9] drm/i915: Nuke intel_ddi_hdmi_num_entries() Ville Syrjala
2021-09-29 20:11   ` Imre Deak
2021-09-27 18:24 ` [Intel-gfx] [PATCH 7/9] drm/i915: Pass the lane to intel_ddi_level() Ville Syrjala
2021-09-29 20:14   ` Imre Deak
2021-09-27 18:24 ` [Intel-gfx] [PATCH 8/9] drm/i915: Prepare link training for per-lane drive settings Ville Syrjala
2021-09-28 21:22   ` [Intel-gfx] [PATCH v2 " Ville Syrjala
2021-09-29 16:54   ` [Intel-gfx] [PATCH v3 " Ville Syrjala
2021-09-29 20:26     ` Imre Deak
2021-09-30  7:07       ` Ville Syrjälä
2021-09-27 18:24 ` [Intel-gfx] [PATCH 9/9] drm/i915: Allow per-lane drive settings with LTTPRs Ville Syrjala
2021-09-29 16:55   ` [Intel-gfx] [PATCH v2 " Ville Syrjala
2021-09-29 20:27     ` Imre Deak
2021-09-27 20:15 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: DP per-lane drive settings prep work Patchwork
2021-09-27 20:16 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2021-09-27 20:44 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2021-09-28  1:25 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
2021-09-28 21:54 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: DP per-lane drive settings prep work (rev2) Patchwork
2021-09-28 21:55 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2021-09-28 22:23 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2021-09-29  0:54 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
2021-09-29 17:34 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: DP per-lane drive settings prep work (rev4) Patchwork
2021-09-29 17:35 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2021-09-29 18:05 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2021-09-29 21:12 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210929194339.GE2192289@ideak-desk.fi.intel.com \
    --to=imre.deak@intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=ville.syrjala@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.