On Tue, Oct 21, 2025 at 09:28:38PM -0300, Gustavo Sousa wrote: > From: Ravi Kumar Vodapalli <[email protected]> > > Some of the register fields of MBUS_CTL and DBUF_CTL register are > changed for Xe3p_LPD platforms. Update the changed fields in the driver. > Below are the changes: > > MBUS_CTL: > Translation Throttle Min > It changed from BIT[15:13] to BIT[16:13] > > DBUF_CTL: > Min Tracker State Service > It changed from BIT[18:16] to BIT[20:16] > Max Tracker State Service > It changed to from BIT[23:19] to BIT[14:10] > but using default value, so no need to define > in code.
In a lot of cases when a register field picks up extra high bit(s), the extra bits were previously reserved, so it's fine to just adjust the existing definition (since reserved bits are required to always read out of hardware as zeroes). However in these cases, the new bits these fields are extending into were actively used by the hardware for other purposes on previous platforms, which is why it's necessary to keep the existing pre-Xe3p definitions unchanged and create separate Xe3p ones that can be used only on the newer Xe3p platforms. You should make some mention of that in the commit message so it's clear why we're handling these a bit differently than a lot of other registers. > > v2: > - Keep definitions in the same line (i.e. without line continuation > breaks) for better readability. (Jani) > > Bspec: 68868, 68872 > Cc: Jani Nikula <[email protected]> > Signed-off-by: Ravi Kumar Vodapalli <[email protected]> > Signed-off-by: Gustavo Sousa <[email protected]> > --- > drivers/gpu/drm/i915/display/skl_watermark.c | 16 +++++-- > drivers/gpu/drm/i915/display/skl_watermark_regs.h | 52 > ++++++++++++----------- > 2 files changed, 40 insertions(+), 28 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c > b/drivers/gpu/drm/i915/display/skl_watermark.c > index 256162da9afc..c141d575009f 100644 > --- a/drivers/gpu/drm/i915/display/skl_watermark.c > +++ b/drivers/gpu/drm/i915/display/skl_watermark.c > @@ -3477,7 +3477,10 @@ void intel_dbuf_mdclk_cdclk_ratio_update(struct > intel_display *display, > if (!HAS_MBUS_JOINING(display)) > return; > > - if (DISPLAY_VER(display) >= 20) > + if (DISPLAY_VER(display) >= 35) > + intel_de_rmw(display, MBUS_CTL, > XE3P_MBUS_TRANSLATION_THROTTLE_MIN_MASK, > + XE3P_MBUS_TRANSLATION_THROTTLE_MIN(ratio - 1)); > + else if (DISPLAY_VER(display) >= 20) > intel_de_rmw(display, MBUS_CTL, > MBUS_TRANSLATION_THROTTLE_MIN_MASK, > MBUS_TRANSLATION_THROTTLE_MIN(ratio - 1)); > > @@ -3488,9 +3491,14 @@ void intel_dbuf_mdclk_cdclk_ratio_update(struct > intel_display *display, > ratio, str_yes_no(joined_mbus)); > > for_each_dbuf_slice(display, slice) > - intel_de_rmw(display, DBUF_CTL_S(slice), > - DBUF_MIN_TRACKER_STATE_SERVICE_MASK, > - DBUF_MIN_TRACKER_STATE_SERVICE(ratio - 1)); > + if (DISPLAY_VER(display) >= 35) > + intel_de_rmw(display, DBUF_CTL_S(slice), > + XE3P_DBUF_MIN_TRACKER_STATE_SERVICE_MASK, > + XE3P_DBUF_MIN_TRACKER_STATE_SERVICE(ratio > - 1)); > + else > + intel_de_rmw(display, DBUF_CTL_S(slice), > + DBUF_MIN_TRACKER_STATE_SERVICE_MASK, > + DBUF_MIN_TRACKER_STATE_SERVICE(ratio - 1)); > } > > static void intel_dbuf_mdclk_min_tracker_update(struct intel_atomic_state > *state) > diff --git a/drivers/gpu/drm/i915/display/skl_watermark_regs.h > b/drivers/gpu/drm/i915/display/skl_watermark_regs.h > index c5572fc0e847..94915afc6b0b 100644 > --- a/drivers/gpu/drm/i915/display/skl_watermark_regs.h > +++ b/drivers/gpu/drm/i915/display/skl_watermark_regs.h > @@ -32,16 +32,18 @@ > #define MBUS_BBOX_CTL_S1 _MMIO(0x45040) > #define MBUS_BBOX_CTL_S2 _MMIO(0x45044) > > -#define MBUS_CTL _MMIO(0x4438C) > -#define MBUS_JOIN REG_BIT(31) > -#define MBUS_HASHING_MODE_MASK REG_BIT(30) > -#define MBUS_HASHING_MODE_2x2 > REG_FIELD_PREP(MBUS_HASHING_MODE_MASK, 0) > -#define MBUS_HASHING_MODE_1x4 > REG_FIELD_PREP(MBUS_HASHING_MODE_MASK, 1) > -#define MBUS_JOIN_PIPE_SELECT_MASK REG_GENMASK(28, 26) > -#define MBUS_JOIN_PIPE_SELECT(pipe) > REG_FIELD_PREP(MBUS_JOIN_PIPE_SELECT_MASK, pipe) > -#define MBUS_JOIN_PIPE_SELECT_NONE MBUS_JOIN_PIPE_SELECT(7) > -#define MBUS_TRANSLATION_THROTTLE_MIN_MASK REG_GENMASK(15, 13) > -#define MBUS_TRANSLATION_THROTTLE_MIN(val) > REG_FIELD_PREP(MBUS_TRANSLATION_THROTTLE_MIN_MASK, val) > +#define MBUS_CTL _MMIO(0x4438C) > +#define MBUS_JOIN REG_BIT(31) > +#define MBUS_HASHING_MODE_MASK REG_BIT(30) > +#define MBUS_HASHING_MODE_2x2 > REG_FIELD_PREP(MBUS_HASHING_MODE_MASK, 0) > +#define MBUS_HASHING_MODE_1x4 > REG_FIELD_PREP(MBUS_HASHING_MODE_MASK, 1) > +#define MBUS_JOIN_PIPE_SELECT_MASK REG_GENMASK(28, 26) > +#define MBUS_JOIN_PIPE_SELECT(pipe) > REG_FIELD_PREP(MBUS_JOIN_PIPE_SELECT_MASK, pipe) > +#define MBUS_JOIN_PIPE_SELECT_NONE MBUS_JOIN_PIPE_SELECT(7) > +#define MBUS_TRANSLATION_THROTTLE_MIN_MASK REG_GENMASK(15, 13) > +#define MBUS_TRANSLATION_THROTTLE_MIN(val) > REG_FIELD_PREP(MBUS_TRANSLATION_THROTTLE_MIN_MASK, val) > +#define XE3P_MBUS_TRANSLATION_THROTTLE_MIN_MASK REG_GENMASK(16, 13) > +#define XE3P_MBUS_TRANSLATION_THROTTLE_MIN(val) > REG_FIELD_PREP(XE3P_MBUS_TRANSLATION_THROTTLE_MIN_MASK, val) Nitpick: I'm not sure if we're 100% consistent, but I feel like we usually sort bitfields based on their upper bound rather than their lower bound. So even though xe3p and pre-xe3p start at the same bit 13, the xe3p should probably be sorted first since it ends at a higher bit (16 vs 15). > > /* > * The below are numbered starting from "S1" on gen11/gen12, but starting > @@ -51,21 +53,23 @@ > * way things will be named by the hardware team going forward, plus it's > more > * consistent with how most of the rest of our registers are named. > */ > -#define _DBUF_CTL_S0 0x45008 > -#define _DBUF_CTL_S1 0x44FE8 > -#define _DBUF_CTL_S2 0x44300 > -#define _DBUF_CTL_S3 0x44304 > -#define DBUF_CTL_S(slice) _MMIO(_PICK(slice, \ > - _DBUF_CTL_S0, \ > - _DBUF_CTL_S1, \ > - _DBUF_CTL_S2, \ > - _DBUF_CTL_S3)) > -#define DBUF_POWER_REQUEST REG_BIT(31) > -#define DBUF_POWER_STATE REG_BIT(30) > -#define DBUF_TRACKER_STATE_SERVICE_MASK REG_GENMASK(23, 19) > -#define DBUF_TRACKER_STATE_SERVICE(x) > REG_FIELD_PREP(DBUF_TRACKER_STATE_SERVICE_MASK, x) > -#define DBUF_MIN_TRACKER_STATE_SERVICE_MASK REG_GENMASK(18, 16) /* ADL-P+ */ > +#define _DBUF_CTL_S0 0x45008 > +#define _DBUF_CTL_S1 0x44FE8 > +#define _DBUF_CTL_S2 0x44300 > +#define _DBUF_CTL_S3 0x44304 > +#define DBUF_CTL_S(slice) _MMIO(_PICK(slice, \ > + > _DBUF_CTL_S0, \ > + > _DBUF_CTL_S1, \ > + > _DBUF_CTL_S2, \ > + > _DBUF_CTL_S3)) > +#define DBUF_POWER_REQUEST REG_BIT(31) > +#define DBUF_POWER_STATE REG_BIT(30) > +#define DBUF_TRACKER_STATE_SERVICE_MASK REG_GENMASK(23, 19) > +#define DBUF_TRACKER_STATE_SERVICE(x) > REG_FIELD_PREP(DBUF_TRACKER_STATE_SERVICE_MASK, x) > +#define DBUF_MIN_TRACKER_STATE_SERVICE_MASK REG_GENMASK(18, 16) /* > ADL-P+ */ > #define DBUF_MIN_TRACKER_STATE_SERVICE(x) > REG_FIELD_PREP(DBUF_MIN_TRACKER_STATE_SERVICE_MASK, x) /* ADL-P+ */ > +#define XE3P_DBUF_MIN_TRACKER_STATE_SERVICE_MASK REG_GENMASK(20, 16) > +#define XE3P_DBUF_MIN_TRACKER_STATE_SERVICE(x) > REG_FIELD_PREP(XE3P_DBUF_MIN_TRACKER_STATE_SERVICE_MASK, x) Same here. Matt > > #define MTL_LATENCY_LP0_LP1 _MMIO(0x45780) > #define MTL_LATENCY_LP2_LP3 _MMIO(0x45784) > > -- > 2.51.0 > -- Matt Roper Graphics Software Engineer Linux GPU Platform Enablement Intel Corporation
