Quoting Jani Nikula (2025-10-15 11:58:37-03:00) >On Wed, 15 Oct 2025, Gustavo Sousa <[email protected]> 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. >> >> Bspec: 68868, 68872 >> 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 | 12 ++++++++++-- >> 2 files changed, 22 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c >> b/drivers/gpu/drm/i915/display/skl_watermark.c >> index 9df9ee137bf9..41f64e347436 100644 >> --- a/drivers/gpu/drm/i915/display/skl_watermark.c >> +++ b/drivers/gpu/drm/i915/display/skl_watermark.c >> @@ -3505,7 +3505,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)); >> >> @@ -3516,9 +3519,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..7e0877303e05 100644 >> --- a/drivers/gpu/drm/i915/display/skl_watermark_regs.h >> +++ b/drivers/gpu/drm/i915/display/skl_watermark_regs.h >> @@ -41,7 +41,11 @@ >> #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_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) >> >> /* >> * The below are numbered starting from "S1" on gen11/gen12, but starting >> @@ -65,7 +69,11 @@ >> #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 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) > >Please just keep the long lines in this file. In this case, I think it's >cleaner.
Alright, done. Because of the length of the new XE3P_* macros, the column alignment for the definition is off by 1 tab character w.r.t. to the items above them. I took a guess and aligned the already existing ones with one extra tab for each. Let me know if that's fine to you. These changes are in still my local tree as I incorporate the remaining of the review feedback (and also wait a bit more for more feedback). -- Gustavo Sousa > > >> >> #define MTL_LATENCY_LP0_LP1 _MMIO(0x45780) >> #define MTL_LATENCY_LP2_LP3 _MMIO(0x45784) > >-- >Jani Nikula, Intel
