Re: [PATCH 1/2] drm/i915: Make macro definitions consistent for ICL and CNL

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Jan 28, 2019 at 02:00:11PM -0800, Aditya Swarup wrote:
> Macro definitions to be organized semantically based on dword, lane and
> port(in this order).
> 
> Cc: Clint Taylor <clinton.a.taylor@xxxxxxxxx>
> Cc: Imre Deak <imre.deak@xxxxxxxxx>
> Cc: Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx>
> Signed-off-by: Aditya Swarup <aditya.swarup@xxxxxxxxx>

I think you can change the commit message to say Combo PHY DDI programming
related macro definitions since the next patch mentiones MG PHY macro defs.
Also you should add Fixes tag with the SHA of the original patch that adds these
macros.
Everything else looks good to me w.r.t the changes suggested to use the arguments order
as dword, lane, port.
So with the above changes,

Reviewed-by: Manasi Navare <manasi.d.navare@xxxxxxxxx>

Manasi
> ---
>  drivers/gpu/drm/i915/i915_reg.h  |  6 +++---
>  drivers/gpu/drm/i915/icl_dsi.c   |  8 ++++----
>  drivers/gpu/drm/i915/intel_ddi.c | 16 ++++++++--------
>  3 files changed, 15 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 1eca166d95bb..b0535073c3f0 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1860,13 +1860,13 @@ enum i915_power_well_id {
>  #define _CNL_PORT_TX_DW4_LN1_AE		0x1624D0
>  #define CNL_PORT_TX_DW4_GRP(port)	_MMIO(_CNL_PORT_TX_DW_GRP(4, (port)))
>  #define CNL_PORT_TX_DW4_LN0(port)	_MMIO(_CNL_PORT_TX_DW_LN0(4, (port)))
> -#define CNL_PORT_TX_DW4_LN(port, ln)   _MMIO(_CNL_PORT_TX_DW_LN0(4, (port)) + \
> +#define CNL_PORT_TX_DW4_LN(ln, port)   _MMIO(_CNL_PORT_TX_DW_LN0(4, (port)) + \
>  					   ((ln) * (_CNL_PORT_TX_DW4_LN1_AE - \
>  						    _CNL_PORT_TX_DW4_LN0_AE)))
>  #define ICL_PORT_TX_DW4_AUX(port)	_MMIO(_ICL_PORT_TX_DW_AUX(4, port))
>  #define ICL_PORT_TX_DW4_GRP(port)	_MMIO(_ICL_PORT_TX_DW_GRP(4, port))
>  #define ICL_PORT_TX_DW4_LN0(port)	_MMIO(_ICL_PORT_TX_DW_LN(4, 0, port))
> -#define ICL_PORT_TX_DW4_LN(port, ln)	_MMIO(_ICL_PORT_TX_DW_LN(4, ln, port))
> +#define ICL_PORT_TX_DW4_LN(ln, port)	_MMIO(_ICL_PORT_TX_DW_LN(4, ln, port))
>  #define   LOADGEN_SELECT		(1 << 31)
>  #define   POST_CURSOR_1(x)		((x) << 12)
>  #define   POST_CURSOR_1_MASK		(0x3F << 12)
> @@ -1893,7 +1893,7 @@ enum i915_power_well_id {
>  #define ICL_PORT_TX_DW7_AUX(port)	_MMIO(_ICL_PORT_TX_DW_AUX(7, port))
>  #define ICL_PORT_TX_DW7_GRP(port)	_MMIO(_ICL_PORT_TX_DW_GRP(7, port))
>  #define ICL_PORT_TX_DW7_LN0(port)	_MMIO(_ICL_PORT_TX_DW_LN(7, 0, port))
> -#define ICL_PORT_TX_DW7_LN(port, ln)	_MMIO(_ICL_PORT_TX_DW_LN(7, ln, port))
> +#define ICL_PORT_TX_DW7_LN(ln, port)	_MMIO(_ICL_PORT_TX_DW_LN(7, ln, port))
>  #define   N_SCALAR(x)			((x) << 24)
>  #define   N_SCALAR_MASK			(0x7F << 24)
>  
> diff --git a/drivers/gpu/drm/i915/icl_dsi.c b/drivers/gpu/drm/i915/icl_dsi.c
> index 73a7bee24a66..beb30d9a855c 100644
> --- a/drivers/gpu/drm/i915/icl_dsi.c
> +++ b/drivers/gpu/drm/i915/icl_dsi.c
> @@ -246,13 +246,13 @@ static void dsi_program_swing_and_deemphasis(struct intel_encoder *encoder)
>  
>  		for (lane = 0; lane <= 3; lane++) {
>  			/* Bspec: must not use GRP register for write */
> -			tmp = I915_READ(ICL_PORT_TX_DW4_LN(port, lane));
> +			tmp = I915_READ(ICL_PORT_TX_DW4_LN(lane, port));
>  			tmp &= ~(POST_CURSOR_1_MASK | POST_CURSOR_2_MASK |
>  				 CURSOR_COEFF_MASK);
>  			tmp |= POST_CURSOR_1(0x0);
>  			tmp |= POST_CURSOR_2(0x0);
>  			tmp |= CURSOR_COEFF(0x3f);
> -			I915_WRITE(ICL_PORT_TX_DW4_LN(port, lane), tmp);
> +			I915_WRITE(ICL_PORT_TX_DW4_LN(lane, port), tmp);
>  		}
>  	}
>  }
> @@ -390,11 +390,11 @@ static void gen11_dsi_config_phy_lanes_sequence(struct intel_encoder *encoder)
>  		tmp &= ~LOADGEN_SELECT;
>  		I915_WRITE(ICL_PORT_TX_DW4_AUX(port), tmp);
>  		for (lane = 0; lane <= 3; lane++) {
> -			tmp = I915_READ(ICL_PORT_TX_DW4_LN(port, lane));
> +			tmp = I915_READ(ICL_PORT_TX_DW4_LN(lane, port));
>  			tmp &= ~LOADGEN_SELECT;
>  			if (lane != 2)
>  				tmp |= LOADGEN_SELECT;
> -			I915_WRITE(ICL_PORT_TX_DW4_LN(port, lane), tmp);
> +			I915_WRITE(ICL_PORT_TX_DW4_LN(lane, port), tmp);
>  		}
>  	}
>  
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index acd94354afc8..c6def69348a6 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -2315,13 +2315,13 @@ static void cnl_ddi_vswing_program(struct intel_encoder *encoder,
>  	/* Program PORT_TX_DW4 */
>  	/* We cannot write to GRP. It would overrite individual loadgen */
>  	for (ln = 0; ln < 4; ln++) {
> -		val = I915_READ(CNL_PORT_TX_DW4_LN(port, ln));
> +		val = I915_READ(CNL_PORT_TX_DW4_LN(ln, port));
>  		val &= ~(POST_CURSOR_1_MASK | POST_CURSOR_2_MASK |
>  			 CURSOR_COEFF_MASK);
>  		val |= POST_CURSOR_1(ddi_translations[level].dw4_post_cursor_1);
>  		val |= POST_CURSOR_2(ddi_translations[level].dw4_post_cursor_2);
>  		val |= CURSOR_COEFF(ddi_translations[level].dw4_cursor_coeff);
> -		I915_WRITE(CNL_PORT_TX_DW4_LN(port, ln), val);
> +		I915_WRITE(CNL_PORT_TX_DW4_LN(ln, port), val);
>  	}
>  
>  	/* Program PORT_TX_DW5 */
> @@ -2377,14 +2377,14 @@ static void cnl_ddi_vswing_sequence(struct intel_encoder *encoder,
>  	 * > 6 GHz (LN0=0, LN1=0, LN2=0, LN3=0)
>  	 */
>  	for (ln = 0; ln <= 3; ln++) {
> -		val = I915_READ(CNL_PORT_TX_DW4_LN(port, ln));
> +		val = I915_READ(CNL_PORT_TX_DW4_LN(ln, port));
>  		val &= ~LOADGEN_SELECT;
>  
>  		if ((rate <= 600000 && width == 4 && ln >= 1)  ||
>  		    (rate <= 600000 && width < 4 && (ln == 1 || ln == 2))) {
>  			val |= LOADGEN_SELECT;
>  		}
> -		I915_WRITE(CNL_PORT_TX_DW4_LN(port, ln), val);
> +		I915_WRITE(CNL_PORT_TX_DW4_LN(ln, port), val);
>  	}
>  
>  	/* 3. Set PORT_CL_DW5 SUS Clock Config to 11b */
> @@ -2446,13 +2446,13 @@ static void icl_ddi_combo_vswing_program(struct drm_i915_private *dev_priv,
>  	/* Program PORT_TX_DW4 */
>  	/* We cannot write to GRP. It would overwrite individual loadgen. */
>  	for (ln = 0; ln <= 3; ln++) {
> -		val = I915_READ(ICL_PORT_TX_DW4_LN(port, ln));
> +		val = I915_READ(ICL_PORT_TX_DW4_LN(ln, port));
>  		val &= ~(POST_CURSOR_1_MASK | POST_CURSOR_2_MASK |
>  			 CURSOR_COEFF_MASK);
>  		val |= POST_CURSOR_1(ddi_translations[level].dw4_post_cursor_1);
>  		val |= POST_CURSOR_2(ddi_translations[level].dw4_post_cursor_2);
>  		val |= CURSOR_COEFF(ddi_translations[level].dw4_cursor_coeff);
> -		I915_WRITE(ICL_PORT_TX_DW4_LN(port, ln), val);
> +		I915_WRITE(ICL_PORT_TX_DW4_LN(ln, port), val);
>  	}
>  
>  	/* Program PORT_TX_DW7 */
> @@ -2503,14 +2503,14 @@ static void icl_combo_phy_ddi_vswing_sequence(struct intel_encoder *encoder,
>  	 * > 6 GHz (LN0=0, LN1=0, LN2=0, LN3=0)
>  	 */
>  	for (ln = 0; ln <= 3; ln++) {
> -		val = I915_READ(ICL_PORT_TX_DW4_LN(port, ln));
> +		val = I915_READ(ICL_PORT_TX_DW4_LN(ln, port));
>  		val &= ~LOADGEN_SELECT;
>  
>  		if ((rate <= 600000 && width == 4 && ln >= 1) ||
>  		    (rate <= 600000 && width < 4 && (ln == 1 || ln == 2))) {
>  			val |= LOADGEN_SELECT;
>  		}
> -		I915_WRITE(ICL_PORT_TX_DW4_LN(port, ln), val);
> +		I915_WRITE(ICL_PORT_TX_DW4_LN(ln, port), val);
>  	}
>  
>  	/* 3. Set PORT_CL_DW5 SUS Clock Config to 11b */
> -- 
> 2.17.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux