All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jani Nikula <jani.nikula@linux.intel.com>
To: Imre Deak <imre.deak@intel.com>, intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 3/3] drm/i915/adlp: Fix AUX power well -> PHY mapping
Date: Wed, 09 Jun 2021 14:42:04 +0300	[thread overview]
Message-ID: <87y2bjz2lv.fsf@intel.com> (raw)
In-Reply-To: <20210526143729.2563672-3-imre.deak@intel.com>

On Wed, 26 May 2021, Imre Deak <imre.deak@intel.com> wrote:
> On ADL_P the power well->PHY mapping doesn't follow the mapping on previous
> platforms, fix this up.
>
> While at it remove the redundant dev_priv param from
> icl_tc_phy_aux_ch().
>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  .../drm/i915/display/intel_display_power.c    | 34 ++++++++++---------
>  1 file changed, 18 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
> index a95bbf23e6b7c..7ddd63114b36b 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> @@ -291,8 +291,7 @@ static void hsw_power_well_pre_disable(struct drm_i915_private *dev_priv,
>  #define ICL_TBT_AUX_PW_TO_CH(pw_idx)	\
>  	((pw_idx) - ICL_PW_CTL_IDX_AUX_TBT1 + AUX_CH_C)
>  
> -static enum aux_ch icl_tc_phy_aux_ch(struct drm_i915_private *dev_priv,
> -				     struct i915_power_well *power_well)
> +static enum aux_ch icl_aux_pw_to_ch(const struct i915_power_well *power_well)
>  {
>  	int pw_idx = power_well->desc->hsw.idx;
>  
> @@ -327,6 +326,15 @@ aux_ch_to_digital_port(struct drm_i915_private *dev_priv,
>  	return dig_port;
>  }
>  
> +static enum phy icl_aux_pw_to_phy(struct drm_i915_private *i915,
> +				  const struct i915_power_well *power_well)
> +{
> +	enum aux_ch aux_ch = icl_aux_pw_to_ch(power_well);
> +	struct intel_digital_port *dig_port = aux_ch_to_digital_port(i915, aux_ch);

Replying to an already merged patch...

aux_ch_to_digital_port() may return NULL but we don't really check this
anywhere.

Any thoughts how this should be handled?

BR,
Jani.


> +
> +	return intel_port_to_phy(i915, dig_port->base.port);
> +}
> +
>  static void hsw_wait_for_power_well_enable(struct drm_i915_private *dev_priv,
>  					   struct i915_power_well *power_well,
>  					   bool timeout_expected)
> @@ -468,15 +476,13 @@ static void hsw_power_well_disable(struct drm_i915_private *dev_priv,
>  	hsw_wait_for_power_well_disable(dev_priv, power_well);
>  }
>  
> -#define ICL_AUX_PW_TO_PHY(pw_idx)	((pw_idx) - ICL_PW_CTL_IDX_AUX_A)
> -
>  static void
>  icl_combo_phy_aux_power_well_enable(struct drm_i915_private *dev_priv,
>  				    struct i915_power_well *power_well)
>  {
>  	const struct i915_power_well_regs *regs = power_well->desc->hsw.regs;
>  	int pw_idx = power_well->desc->hsw.idx;
> -	enum phy phy = ICL_AUX_PW_TO_PHY(pw_idx);
> +	enum phy phy = icl_aux_pw_to_phy(dev_priv, power_well);
>  	u32 val;
>  
>  	drm_WARN_ON(&dev_priv->drm, !IS_ICELAKE(dev_priv));
> @@ -508,7 +514,7 @@ icl_combo_phy_aux_power_well_disable(struct drm_i915_private *dev_priv,
>  {
>  	const struct i915_power_well_regs *regs = power_well->desc->hsw.regs;
>  	int pw_idx = power_well->desc->hsw.idx;
> -	enum phy phy = ICL_AUX_PW_TO_PHY(pw_idx);
> +	enum phy phy = icl_aux_pw_to_phy(dev_priv, power_well);
>  	u32 val;
>  
>  	drm_WARN_ON(&dev_priv->drm, !IS_ICELAKE(dev_priv));
> @@ -595,7 +601,7 @@ static void
>  icl_tc_phy_aux_power_well_enable(struct drm_i915_private *dev_priv,
>  				 struct i915_power_well *power_well)
>  {
> -	enum aux_ch aux_ch = icl_tc_phy_aux_ch(dev_priv, power_well);
> +	enum aux_ch aux_ch = icl_aux_pw_to_ch(power_well);
>  	struct intel_digital_port *dig_port = aux_ch_to_digital_port(dev_priv, aux_ch);
>  	const struct i915_power_well_regs *regs = power_well->desc->hsw.regs;
>  	bool is_tbt = power_well->desc->hsw.is_tc_tbt;
> @@ -643,7 +649,7 @@ static void
>  icl_tc_phy_aux_power_well_disable(struct drm_i915_private *dev_priv,
>  				  struct i915_power_well *power_well)
>  {
> -	enum aux_ch aux_ch = icl_tc_phy_aux_ch(dev_priv, power_well);
> +	enum aux_ch aux_ch = icl_aux_pw_to_ch(power_well);
>  	struct intel_digital_port *dig_port = aux_ch_to_digital_port(dev_priv, aux_ch);
>  
>  	icl_tc_port_assert_ref_held(dev_priv, power_well, dig_port);
> @@ -655,11 +661,9 @@ static void
>  icl_aux_power_well_enable(struct drm_i915_private *dev_priv,
>  			  struct i915_power_well *power_well)
>  {
> -	int pw_idx = power_well->desc->hsw.idx;
> -	enum phy phy = ICL_AUX_PW_TO_PHY(pw_idx);  /* non-TBT only */
> -	bool is_tbt = power_well->desc->hsw.is_tc_tbt;
> +	enum phy phy = icl_aux_pw_to_phy(dev_priv, power_well);
>  
> -	if (is_tbt || intel_phy_is_tc(dev_priv, phy))
> +	if (intel_phy_is_tc(dev_priv, phy))
>  		return icl_tc_phy_aux_power_well_enable(dev_priv, power_well);
>  	else if (IS_ICELAKE(dev_priv))
>  		return icl_combo_phy_aux_power_well_enable(dev_priv,
> @@ -672,11 +676,9 @@ static void
>  icl_aux_power_well_disable(struct drm_i915_private *dev_priv,
>  			   struct i915_power_well *power_well)
>  {
> -	int pw_idx = power_well->desc->hsw.idx;
> -	enum phy phy = ICL_AUX_PW_TO_PHY(pw_idx);  /* non-TBT only */
> -	bool is_tbt = power_well->desc->hsw.is_tc_tbt;
> +	enum phy phy = icl_aux_pw_to_phy(dev_priv, power_well);
>  
> -	if (is_tbt || intel_phy_is_tc(dev_priv, phy))
> +	if (intel_phy_is_tc(dev_priv, phy))
>  		return icl_tc_phy_aux_power_well_disable(dev_priv, power_well);
>  	else if (IS_ICELAKE(dev_priv))
>  		return icl_combo_phy_aux_power_well_disable(dev_priv,

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

  parent reply	other threads:[~2021-06-09 11:42 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-26 14:37 [Intel-gfx] [PATCH 1/3] drm/i915/ddi: Flush encoder power domain ref puts during driver unload Imre Deak
2021-05-26 14:37 ` [Intel-gfx] [PATCH 2/3] drm/i915: Fix incorrect assert about pending power domain async-put work Imre Deak
2021-06-02  9:05   ` Anshuman Gupta
2021-06-02 14:01     ` Imre Deak
2021-06-03  6:22       ` Gupta, Anshuman
2021-05-26 14:37 ` [Intel-gfx] [PATCH 3/3] drm/i915/adlp: Fix AUX power well -> PHY mapping Imre Deak
2021-06-03  9:43   ` Anshuman Gupta
2021-06-09 11:42   ` Jani Nikula [this message]
2021-06-09 12:34     ` Imre Deak
2021-05-26 21:18 ` [Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915/ddi: Flush encoder power domain ref puts during driver unload Patchwork
2021-05-27 10:02 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
2021-05-27 18:04   ` Imre Deak
2021-05-27 19:41     ` Vudum, Lakshminarayana
2021-05-27 18:40 ` [Intel-gfx] ✓ Fi.CI.IGT: success " Patchwork
2021-06-03 16:34   ` Imre Deak
2021-06-02  8:06 ` [Intel-gfx] [PATCH 1/3] " Anshuman Gupta

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=87y2bjz2lv.fsf@intel.com \
    --to=jani.nikula@linux.intel.com \
    --cc=imre.deak@intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    /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.