Re: [PATCH 14/32] drm/i915/xe3p_lpd: Adapt to updates on MBUS_CTL/DBUF_CTL registers

[Gustavo Sousa <gustavo.sousa@intel.com>]

Quoting Jani Nikula (2025-10-15 11:58:37-03:00)
>On Wed, 15 Oct 2025, Gustavo Sousa <gustavo.sousa@intel.com> wrote:
>> From: Ravi Kumar Vodapalli <ravi.kumar.vodapalli@intel.com>
>>
>> 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 <ravi.kumar.vodapalli@intel.com>
>> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
>> ---
>>  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