diff mbox series

[v3,5/7] drm/i915/xe3lpd: Add new bit range of MAX swing setup

Message ID 20241015231124.23982-6-matthew.s.atwood@intel.com (mailing list archive)
State New
Headers show
Series Add xe3lpd edp enabling | expand

Commit Message

Matt Atwood Oct. 15, 2024, 11:11 p.m. UTC 
From: Suraj Kandpal <suraj.kandpal@intel.com>

Add new bit range for Max PHY Swing Setup in PORT_ALPM_CTL
register for DISPLAY_VER >= 30.

v2: implement as two seperate macros instead of a single macro

Bspec: 70277
Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
---
 drivers/gpu/drm/i915/display/intel_alpm.c     |  9 ++++++--
 drivers/gpu/drm/i915/display/intel_psr_regs.h | 22 ++++++++++---------
 2 files changed, 19 insertions(+), 12 deletions(-)

Comments

Matt Roper Oct. 16, 2024, 4:42 p.m. UTC | #1 
On Tue, Oct 15, 2024 at 04:11:22PM -0700, Matt Atwood wrote:
> From: Suraj Kandpal <suraj.kandpal@intel.com>
> 
> Add new bit range for Max PHY Swing Setup in PORT_ALPM_CTL
> register for DISPLAY_VER >= 30.

So the only change here is that the register field got larger, right?
I.e., it's 25:20 now instead of 23:20?  In that case I don't think we
need this extra complexity; we can simply do a one-line change of
PORT_ALPM_CTL_MAX_PHY_SWING_SETUP_MASK with the larger range of bits.
Bits 25:24 were previously reserved so we were never writing anything
into them on older platforms.  Extending the mask won't change any
behavior on older platforms and will just allow us to stick larger
values in there on Xe3 and beyond.

We have lots of cases in the display driver where fields get slightly
wider to be able to hold larger values on newer platforms.  As long as
the low bit of the field doesn't move, and the upper bits were
previously reserved/unused, we simply extend the field without adding
conditional per-platform logic in those cases.


Matt

> 
> v2: implement as two seperate macros instead of a single macro
> 
> Bspec: 70277
> Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_alpm.c     |  9 ++++++--
>  drivers/gpu/drm/i915/display/intel_psr_regs.h | 22 ++++++++++---------
>  2 files changed, 19 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_alpm.c b/drivers/gpu/drm/i915/display/intel_alpm.c
> index 55f3ae1e68c9..847662930cb8 100644
> --- a/drivers/gpu/drm/i915/display/intel_alpm.c
> +++ b/drivers/gpu/drm/i915/display/intel_alpm.c
> @@ -314,7 +314,7 @@ static void lnl_alpm_configure(struct intel_dp *intel_dp,
>  	struct intel_display *display = to_intel_display(intel_dp);
>  	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
>  	enum port port = dp_to_dig_port(intel_dp)->base.port;
> -	u32 alpm_ctl;
> +	u32 alpm_ctl, alpm_swing_setup;
>  
>  	if (DISPLAY_VER(display) < 20 ||
>  	    (!intel_dp->psr.sel_update_enabled && !intel_dp_is_edp(intel_dp)))
> @@ -331,10 +331,15 @@ static void lnl_alpm_configure(struct intel_dp *intel_dp,
>  			ALPM_CTL_AUX_LESS_SLEEP_HOLD_TIME_50_SYMBOLS |
>  			ALPM_CTL_AUX_LESS_WAKE_TIME(intel_dp->alpm_parameters.aux_less_wake_lines);
>  
> +
> +		if (DISPLAY_VER(display) >= 30)
> +			alpm_swing_setup = XE3_PORT_ALPM_CTL_MAX_PHY_SWING_SETUP(15);
> +		else
> +			alpm_swing_setup = PORT_ALPM_CTL_MAX_PHY_SWING_SETUP(15);
>  		intel_de_write(display,
>  			       PORT_ALPM_CTL(port),
>  			       PORT_ALPM_CTL_ALPM_AUX_LESS_ENABLE |
> -			       PORT_ALPM_CTL_MAX_PHY_SWING_SETUP(15) |
> +			       alpm_swing_setup |
>  			       PORT_ALPM_CTL_MAX_PHY_SWING_HOLD(0) |
>  			       PORT_ALPM_CTL_SILENCE_PERIOD(
>  				       intel_dp->alpm_parameters.silence_period_sym_clocks));
> diff --git a/drivers/gpu/drm/i915/display/intel_psr_regs.h b/drivers/gpu/drm/i915/display/intel_psr_regs.h
> index 0841242543ca..3aeb2af1fbf9 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr_regs.h
> +++ b/drivers/gpu/drm/i915/display/intel_psr_regs.h
> @@ -294,16 +294,18 @@
>  #define  ALPM_CTL2_NUMBER_AUX_LESS_ML_PHY_SLEEP_SEQUENCES_MASK	REG_GENMASK(2, 0)
>  #define  ALPM_CTL2_NUMBER_AUX_LESS_ML_PHY_SLEEP_SEQUENCES(val)	REG_FIELD_PREP(ALPM_CTL2_NUMBER_AUX_LESS_ML_PHY_SLEEP_SEQUENCES_MASK, val)
>  
> -#define _PORT_ALPM_CTL_A			0x16fa2c
> -#define _PORT_ALPM_CTL_B			0x16fc2c
> -#define PORT_ALPM_CTL(port)			_MMIO_PORT(port, _PORT_ALPM_CTL_A, _PORT_ALPM_CTL_B)
> -#define  PORT_ALPM_CTL_ALPM_AUX_LESS_ENABLE	REG_BIT(31)
> -#define  PORT_ALPM_CTL_MAX_PHY_SWING_SETUP_MASK	REG_GENMASK(23, 20)
> -#define  PORT_ALPM_CTL_MAX_PHY_SWING_SETUP(val)	REG_FIELD_PREP(PORT_ALPM_CTL_MAX_PHY_SWING_SETUP_MASK, val)
> -#define  PORT_ALPM_CTL_MAX_PHY_SWING_HOLD_MASK	REG_GENMASK(19, 16)
> -#define  PORT_ALPM_CTL_MAX_PHY_SWING_HOLD(val)	REG_FIELD_PREP(PORT_ALPM_CTL_MAX_PHY_SWING_HOLD_MASK, val)
> -#define  PORT_ALPM_CTL_SILENCE_PERIOD_MASK	REG_GENMASK(7, 0)
> -#define  PORT_ALPM_CTL_SILENCE_PERIOD(val)	REG_FIELD_PREP(PORT_ALPM_CTL_SILENCE_PERIOD_MASK, val)
> +#define _PORT_ALPM_CTL_A				0x16fa2c
> +#define _PORT_ALPM_CTL_B				0x16fc2c
> +#define PORT_ALPM_CTL(port)				_MMIO_PORT(port, _PORT_ALPM_CTL_A, _PORT_ALPM_CTL_B)
> +#define  PORT_ALPM_CTL_ALPM_AUX_LESS_ENABLE		REG_BIT(31)
> +#define  PORT_ALPM_CTL_MAX_PHY_SWING_SETUP_MASK		REG_GENMASK(23, 20)
> +#define  XE3_PORT_ALPM_CTL_MAX_PHY_SWING_SETUP_MASK	REG_GENMASK(25, 20)
> +#define  PORT_ALPM_CTL_MAX_PHY_SWING_SETUP(val)		REG_FIELD_PREP(PORT_ALPM_CTL_MAX_PHY_SWING_SETUP_MASK, val)
> +#define  XE3_PORT_ALPM_CTL_MAX_PHY_SWING_SETUP(val)	REG_FIELD_PREP(XE3_PORT_ALPM_CTL_MAX_PHY_SWING_SETUP_MASK, val)
> +#define  PORT_ALPM_CTL_MAX_PHY_SWING_HOLD_MASK		REG_GENMASK(19, 16)
> +#define  PORT_ALPM_CTL_MAX_PHY_SWING_HOLD(val)		REG_FIELD_PREP(PORT_ALPM_CTL_MAX_PHY_SWING_HOLD_MASK, val)
> +#define  PORT_ALPM_CTL_SILENCE_PERIOD_MASK		REG_GENMASK(7, 0)
> +#define  PORT_ALPM_CTL_SILENCE_PERIOD(val)		REG_FIELD_PREP(PORT_ALPM_CTL_SILENCE_PERIOD_MASK, val)
>  
>  #define _PORT_ALPM_LFPS_CTL_A					0x16fa30
>  #define _PORT_ALPM_LFPS_CTL_B					0x16fc30
> -- 
> 2.45.0
>
Kandpal, Suraj Oct. 17, 2024, 4:39 a.m. UTC | #2 
> -----Original Message-----
> From: Roper, Matthew D <matthew.d.roper@intel.com>
> Sent: Wednesday, October 16, 2024 10:13 PM
> To: Atwood, Matthew S <matthew.s.atwood@intel.com>
> Cc: intel-xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; Kandpal,
> Suraj <suraj.kandpal@intel.com>
> Subject: Re: [PATCH v3 5/7] drm/i915/xe3lpd: Add new bit range of MAX
> swing setup
> 
> On Tue, Oct 15, 2024 at 04:11:22PM -0700, Matt Atwood wrote:
> > From: Suraj Kandpal <suraj.kandpal@intel.com>
> >
> > Add new bit range for Max PHY Swing Setup in PORT_ALPM_CTL register
> > for DISPLAY_VER >= 30.
> 
> So the only change here is that the register field got larger, right?
> I.e., it's 25:20 now instead of 23:20?  In that case I don't think we need this
> extra complexity; we can simply do a one-line change of
> PORT_ALPM_CTL_MAX_PHY_SWING_SETUP_MASK with the larger range of
> bits.
> Bits 25:24 were previously reserved so we were never writing anything into
> them on older platforms.  Extending the mask won't change any behavior on
> older platforms and will just allow us to stick larger values in there on Xe3
> and beyond.
> 
> We have lots of cases in the display driver where fields get slightly wider to
> be able to hold larger values on newer platforms.  As long as the low bit of
> the field doesn't move, and the upper bits were previously
> reserved/unused, we simply extend the field without adding conditional
> per-platform logic in those cases.
> 

Ohkay make sense

Regards,
Suraj Kandpal
> 
> Matt
> 
> >
> > v2: implement as two seperate macros instead of a single macro
> >
> > Bspec: 70277
> > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> > Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_alpm.c     |  9 ++++++--
> >  drivers/gpu/drm/i915/display/intel_psr_regs.h | 22
> > ++++++++++---------
> >  2 files changed, 19 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_alpm.c
> > b/drivers/gpu/drm/i915/display/intel_alpm.c
> > index 55f3ae1e68c9..847662930cb8 100644
> > --- a/drivers/gpu/drm/i915/display/intel_alpm.c
> > +++ b/drivers/gpu/drm/i915/display/intel_alpm.c
> > @@ -314,7 +314,7 @@ static void lnl_alpm_configure(struct intel_dp
> *intel_dp,
> >  	struct intel_display *display = to_intel_display(intel_dp);
> >  	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> >  	enum port port = dp_to_dig_port(intel_dp)->base.port;
> > -	u32 alpm_ctl;
> > +	u32 alpm_ctl, alpm_swing_setup;
> >
> >  	if (DISPLAY_VER(display) < 20 ||
> >  	    (!intel_dp->psr.sel_update_enabled &&
> > !intel_dp_is_edp(intel_dp))) @@ -331,10 +331,15 @@ static void
> lnl_alpm_configure(struct intel_dp *intel_dp,
> >
> 	ALPM_CTL_AUX_LESS_SLEEP_HOLD_TIME_50_SYMBOLS |
> >
> > ALPM_CTL_AUX_LESS_WAKE_TIME(intel_dp-
> >alpm_parameters.aux_less_wake_li
> > nes);
> >
> > +
> > +		if (DISPLAY_VER(display) >= 30)
> > +			alpm_swing_setup =
> XE3_PORT_ALPM_CTL_MAX_PHY_SWING_SETUP(15);
> > +		else
> > +			alpm_swing_setup =
> PORT_ALPM_CTL_MAX_PHY_SWING_SETUP(15);
> >  		intel_de_write(display,
> >  			       PORT_ALPM_CTL(port),
> >  			       PORT_ALPM_CTL_ALPM_AUX_LESS_ENABLE |
> > -			       PORT_ALPM_CTL_MAX_PHY_SWING_SETUP(15) |
> > +			       alpm_swing_setup |
> >  			       PORT_ALPM_CTL_MAX_PHY_SWING_HOLD(0) |
> >  			       PORT_ALPM_CTL_SILENCE_PERIOD(
> >  				       intel_dp-
> >alpm_parameters.silence_period_sym_clocks));
> > diff --git a/drivers/gpu/drm/i915/display/intel_psr_regs.h
> > b/drivers/gpu/drm/i915/display/intel_psr_regs.h
> > index 0841242543ca..3aeb2af1fbf9 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr_regs.h
> > +++ b/drivers/gpu/drm/i915/display/intel_psr_regs.h
> > @@ -294,16 +294,18 @@
> >  #define
> ALPM_CTL2_NUMBER_AUX_LESS_ML_PHY_SLEEP_SEQUENCES_MASK
> 	REG_GENMASK(2, 0)
> >  #define
> ALPM_CTL2_NUMBER_AUX_LESS_ML_PHY_SLEEP_SEQUENCES(val)
> 	REG_FIELD_PREP(ALPM_CTL2_NUMBER_AUX_LESS_ML_PHY_SLEEP_S
> EQUENCES_MASK, val)
> >
> > -#define _PORT_ALPM_CTL_A			0x16fa2c
> > -#define _PORT_ALPM_CTL_B			0x16fc2c
> > -#define PORT_ALPM_CTL(port)			_MMIO_PORT(port,
> _PORT_ALPM_CTL_A, _PORT_ALPM_CTL_B)
> > -#define  PORT_ALPM_CTL_ALPM_AUX_LESS_ENABLE	REG_BIT(31)
> > -#define  PORT_ALPM_CTL_MAX_PHY_SWING_SETUP_MASK
> 	REG_GENMASK(23, 20)
> > -#define  PORT_ALPM_CTL_MAX_PHY_SWING_SETUP(val)
> 	REG_FIELD_PREP(PORT_ALPM_CTL_MAX_PHY_SWING_SETUP_MASK,
> val)
> > -#define  PORT_ALPM_CTL_MAX_PHY_SWING_HOLD_MASK
> 	REG_GENMASK(19, 16)
> > -#define  PORT_ALPM_CTL_MAX_PHY_SWING_HOLD(val)
> 	REG_FIELD_PREP(PORT_ALPM_CTL_MAX_PHY_SWING_HOLD_MASK,
> val)
> > -#define  PORT_ALPM_CTL_SILENCE_PERIOD_MASK	REG_GENMASK(7, 0)
> > -#define  PORT_ALPM_CTL_SILENCE_PERIOD(val)
> 	REG_FIELD_PREP(PORT_ALPM_CTL_SILENCE_PERIOD_MASK, val)
> > +#define _PORT_ALPM_CTL_A				0x16fa2c
> > +#define _PORT_ALPM_CTL_B				0x16fc2c
> > +#define PORT_ALPM_CTL(port)
> 	_MMIO_PORT(port, _PORT_ALPM_CTL_A, _PORT_ALPM_CTL_B)
> > +#define  PORT_ALPM_CTL_ALPM_AUX_LESS_ENABLE
> 	REG_BIT(31)
> > +#define  PORT_ALPM_CTL_MAX_PHY_SWING_SETUP_MASK
> 	REG_GENMASK(23, 20)
> > +#define  XE3_PORT_ALPM_CTL_MAX_PHY_SWING_SETUP_MASK
> 	REG_GENMASK(25, 20)
> > +#define  PORT_ALPM_CTL_MAX_PHY_SWING_SETUP(val)
> 	REG_FIELD_PREP(PORT_ALPM_CTL_MAX_PHY_SWING_SETUP_MASK,
> val)
> > +#define  XE3_PORT_ALPM_CTL_MAX_PHY_SWING_SETUP(val)
> 	REG_FIELD_PREP(XE3_PORT_ALPM_CTL_MAX_PHY_SWING_SETUP_
> MASK, val)
> > +#define  PORT_ALPM_CTL_MAX_PHY_SWING_HOLD_MASK
> 	REG_GENMASK(19, 16)
> > +#define  PORT_ALPM_CTL_MAX_PHY_SWING_HOLD(val)
> 	REG_FIELD_PREP(PORT_ALPM_CTL_MAX_PHY_SWING_HOLD_MASK,
> val)
> > +#define  PORT_ALPM_CTL_SILENCE_PERIOD_MASK
> 	REG_GENMASK(7, 0)
> > +#define  PORT_ALPM_CTL_SILENCE_PERIOD(val)
> 	REG_FIELD_PREP(PORT_ALPM_CTL_SILENCE_PERIOD_MASK, val)
> >
> >  #define _PORT_ALPM_LFPS_CTL_A
> 	0x16fa30
> >  #define _PORT_ALPM_LFPS_CTL_B
> 	0x16fc30
> > --
> > 2.45.0
> >
> 
> --
> Matt Roper
> Graphics Software Engineer
> Linux GPU Platform Enablement
> Intel Corporation
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_alpm.c b/drivers/gpu/drm/i915/display/intel_alpm.c
index 55f3ae1e68c9..847662930cb8 100644
--- a/drivers/gpu/drm/i915/display/intel_alpm.c
+++ b/drivers/gpu/drm/i915/display/intel_alpm.c
@@ -314,7 +314,7 @@  static void lnl_alpm_configure(struct intel_dp *intel_dp,
 	struct intel_display *display = to_intel_display(intel_dp);
 	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
 	enum port port = dp_to_dig_port(intel_dp)->base.port;
-	u32 alpm_ctl;
+	u32 alpm_ctl, alpm_swing_setup;
 
 	if (DISPLAY_VER(display) < 20 ||
 	    (!intel_dp->psr.sel_update_enabled && !intel_dp_is_edp(intel_dp)))
@@ -331,10 +331,15 @@  static void lnl_alpm_configure(struct intel_dp *intel_dp,
 			ALPM_CTL_AUX_LESS_SLEEP_HOLD_TIME_50_SYMBOLS |
 			ALPM_CTL_AUX_LESS_WAKE_TIME(intel_dp->alpm_parameters.aux_less_wake_lines);
 
+
+		if (DISPLAY_VER(display) >= 30)
+			alpm_swing_setup = XE3_PORT_ALPM_CTL_MAX_PHY_SWING_SETUP(15);
+		else
+			alpm_swing_setup = PORT_ALPM_CTL_MAX_PHY_SWING_SETUP(15);
 		intel_de_write(display,
 			       PORT_ALPM_CTL(port),
 			       PORT_ALPM_CTL_ALPM_AUX_LESS_ENABLE |
-			       PORT_ALPM_CTL_MAX_PHY_SWING_SETUP(15) |
+			       alpm_swing_setup |
 			       PORT_ALPM_CTL_MAX_PHY_SWING_HOLD(0) |
 			       PORT_ALPM_CTL_SILENCE_PERIOD(
 				       intel_dp->alpm_parameters.silence_period_sym_clocks));
diff --git a/drivers/gpu/drm/i915/display/intel_psr_regs.h b/drivers/gpu/drm/i915/display/intel_psr_regs.h
index 0841242543ca..3aeb2af1fbf9 100644
--- a/drivers/gpu/drm/i915/display/intel_psr_regs.h
+++ b/drivers/gpu/drm/i915/display/intel_psr_regs.h
@@ -294,16 +294,18 @@ 
 #define  ALPM_CTL2_NUMBER_AUX_LESS_ML_PHY_SLEEP_SEQUENCES_MASK	REG_GENMASK(2, 0)
 #define  ALPM_CTL2_NUMBER_AUX_LESS_ML_PHY_SLEEP_SEQUENCES(val)	REG_FIELD_PREP(ALPM_CTL2_NUMBER_AUX_LESS_ML_PHY_SLEEP_SEQUENCES_MASK, val)
 
-#define _PORT_ALPM_CTL_A			0x16fa2c
-#define _PORT_ALPM_CTL_B			0x16fc2c
-#define PORT_ALPM_CTL(port)			_MMIO_PORT(port, _PORT_ALPM_CTL_A, _PORT_ALPM_CTL_B)
-#define  PORT_ALPM_CTL_ALPM_AUX_LESS_ENABLE	REG_BIT(31)
-#define  PORT_ALPM_CTL_MAX_PHY_SWING_SETUP_MASK	REG_GENMASK(23, 20)
-#define  PORT_ALPM_CTL_MAX_PHY_SWING_SETUP(val)	REG_FIELD_PREP(PORT_ALPM_CTL_MAX_PHY_SWING_SETUP_MASK, val)
-#define  PORT_ALPM_CTL_MAX_PHY_SWING_HOLD_MASK	REG_GENMASK(19, 16)
-#define  PORT_ALPM_CTL_MAX_PHY_SWING_HOLD(val)	REG_FIELD_PREP(PORT_ALPM_CTL_MAX_PHY_SWING_HOLD_MASK, val)
-#define  PORT_ALPM_CTL_SILENCE_PERIOD_MASK	REG_GENMASK(7, 0)
-#define  PORT_ALPM_CTL_SILENCE_PERIOD(val)	REG_FIELD_PREP(PORT_ALPM_CTL_SILENCE_PERIOD_MASK, val)
+#define _PORT_ALPM_CTL_A				0x16fa2c
+#define _PORT_ALPM_CTL_B				0x16fc2c
+#define PORT_ALPM_CTL(port)				_MMIO_PORT(port, _PORT_ALPM_CTL_A, _PORT_ALPM_CTL_B)
+#define  PORT_ALPM_CTL_ALPM_AUX_LESS_ENABLE		REG_BIT(31)
+#define  PORT_ALPM_CTL_MAX_PHY_SWING_SETUP_MASK		REG_GENMASK(23, 20)
+#define  XE3_PORT_ALPM_CTL_MAX_PHY_SWING_SETUP_MASK	REG_GENMASK(25, 20)
+#define  PORT_ALPM_CTL_MAX_PHY_SWING_SETUP(val)		REG_FIELD_PREP(PORT_ALPM_CTL_MAX_PHY_SWING_SETUP_MASK, val)
+#define  XE3_PORT_ALPM_CTL_MAX_PHY_SWING_SETUP(val)	REG_FIELD_PREP(XE3_PORT_ALPM_CTL_MAX_PHY_SWING_SETUP_MASK, val)
+#define  PORT_ALPM_CTL_MAX_PHY_SWING_HOLD_MASK		REG_GENMASK(19, 16)
+#define  PORT_ALPM_CTL_MAX_PHY_SWING_HOLD(val)		REG_FIELD_PREP(PORT_ALPM_CTL_MAX_PHY_SWING_HOLD_MASK, val)
+#define  PORT_ALPM_CTL_SILENCE_PERIOD_MASK		REG_GENMASK(7, 0)
+#define  PORT_ALPM_CTL_SILENCE_PERIOD(val)		REG_FIELD_PREP(PORT_ALPM_CTL_SILENCE_PERIOD_MASK, val)
 
 #define _PORT_ALPM_LFPS_CTL_A					0x16fa30
 #define _PORT_ALPM_LFPS_CTL_B					0x16fc30