> -----Original Message----- > From: Roper, Matthew D <matthew.d.roper@xxxxxxxxx> > Sent: Wednesday, October 16, 2024 10:13 PM > To: Atwood, Matthew S <matthew.s.atwood@xxxxxxxxx> > Cc: intel-xe@xxxxxxxxxxxxxxxxxxxxx; intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Kandpal, > Suraj <suraj.kandpal@xxxxxxxxx> > 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@xxxxxxxxx> > > > > 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@xxxxxxxxx> > > Signed-off-by: Matt Atwood <matthew.s.atwood@xxxxxxxxx> > > --- > > 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
