All of lore.kernel.org
 help / color / mirror / Atom feed
From: Manasi Navare <manasi.d.navare@intel.com>
To: Aditya Swarup <aditya.swarup@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 1/2] drm/i915: Make macro definitions consistent for ICL and CNL
Date: Tue, 12 Feb 2019 15:58:09 -0800	[thread overview]
Message-ID: <20190212235809.GA6529@intel.com> (raw)
In-Reply-To: <20190128220012.13122-2-aditya.swarup@intel.com>

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@intel.com>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Signed-off-by: Aditya Swarup <aditya.swarup@intel.com>

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@intel.com>

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@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2019-02-12 23:56 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-28 22:00 [PATCH 0/2] Make macro definitions consistent in intel_reg.h Aditya Swarup
2019-01-28 22:00 ` [PATCH 1/2] drm/i915: Make macro definitions consistent for ICL and CNL Aditya Swarup
2019-02-12 23:58   ` Manasi Navare [this message]
2019-01-28 22:00 ` [PATCH 2/2] drm/i915: Make MG phy macros semantically consistent Aditya Swarup
2019-02-12 23:59   ` Manasi Navare
2019-02-14  7:47     ` Lucas De Marchi
2019-02-14 13:39       ` Jani Nikula
2019-01-28 23:00 ` ✗ Fi.CI.CHECKPATCH: warning for Make macro definitions consistent in intel_reg.h Patchwork
2019-01-28 23:21 ` ✓ Fi.CI.BAT: success " Patchwork
2019-01-29  7:24 ` ✓ Fi.CI.IGT: " 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=20190212235809.GA6529@intel.com \
    --to=manasi.d.navare@intel.com \
    --cc=aditya.swarup@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.