Re: [PATCH] drm/i915/ptl: Use everywhere the correct DDI port clock select mask

[Imre Deak <imre.deak@intel.com>]

On Mon, May 12, 2025 at 05:03:52PM -0300, Gustavo Sousa wrote:
> Quoting Imre Deak (2025-05-12 11:26:00-03:00)
>
> [...]
>
> >diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy_regs.h b/drivers/gpu/drm/i915/display/intel_cx0_phy_regs.h
> >index 960f7f778fb81..59c22beaf1de5 100644
> >--- a/drivers/gpu/drm/i915/display/intel_cx0_phy_regs.h
> >+++ b/drivers/gpu/drm/i915/display/intel_cx0_phy_regs.h
> >@@ -192,10 +192,17 @@
> > 
> > #define   XELPDP_TBT_CLOCK_REQUEST                        REG_BIT(19)
> > #define   XELPDP_TBT_CLOCK_ACK                                REG_BIT(18)
> >-#define   XELPDP_DDI_CLOCK_SELECT_MASK                        REG_GENMASK(15, 12)
> >-#define   XE3_DDI_CLOCK_SELECT_MASK                        REG_GENMASK(16, 12)
> >-#define   XELPDP_DDI_CLOCK_SELECT(val)                        REG_FIELD_PREP(XELPDP_DDI_CLOCK_SELECT_MASK, val)
> >-#define   XE3_DDI_CLOCK_SELECT(val)                        REG_FIELD_PREP(XE3_DDI_CLOCK_SELECT_MASK, val)
> >+#define   _XELPDP_DDI_CLOCK_SELECT_MASK                        REG_GENMASK(15, 12)
> >+#define   _XE3_DDI_CLOCK_SELECT_MASK                        REG_GENMASK(16, 12)
> 
> Since bit 16 is unused for pre-Xe3 display IPs, I wonder if we should
> simply redefine XELPDP_DDI_CLOCK_SELECT_MASK to be REG_GENMASK(16, 12)
> and add a comment noting changes by display IP? Are we aware of
> something that would be wired to bit 16 that would prevent us from doing
> that?

Not sure if a register bit is not used, unless it's defined as reserved.
For pre-Xe3 (pre-PTL) bits 16-17 are not defined as reserved.

> I remember something similar was done to other register fields in the
> past. Some examples I found:
> 
>     3fe856180c94 ("drm/i915/xe3lpd: Add new bit range of MAX swing setup")
>     247bdac958fc ("drm/i915/adl_p: Add ddb allocation support")
>     d7e449a858ec ("drm/i915: Just use icl+ definition for PLANE_WM blocks field")
> 
> --
> Gustavo Sousa
> 
> >+#define   XELPDP_DDI_CLOCK_SELECT_MASK(display)                (DISPLAY_VER(display) >= 30 ? \
> >+                                                         _XE3_DDI_CLOCK_SELECT_MASK : _XELPDP_DDI_CLOCK_SELECT_MASK)
> >+#define   XELPDP_DDI_CLOCK_SELECT_PREP(display, val)        (DISPLAY_VER(display) >= 30 ? \
> >+                                                         REG_FIELD_PREP(_XE3_DDI_CLOCK_SELECT_MASK, (val)) : \
> >+                                                         REG_FIELD_PREP(_XELPDP_DDI_CLOCK_SELECT_MASK, (val)))
> >+#define   XELPDP_DDI_CLOCK_SELECT_GET(display, val)        (DISPLAY_VER(display) >= 30 ? \
> >+                                                         REG_FIELD_GET(_XE3_DDI_CLOCK_SELECT_MASK, (val)) : \
> >+                                                         REG_FIELD_GET(_XELPDP_DDI_CLOCK_SELECT_MASK, (val)))
> >+
> > #define   XELPDP_DDI_CLOCK_SELECT_NONE                        0x0
> > #define   XELPDP_DDI_CLOCK_SELECT_MAXPCLK                0x8
> > #define   XELPDP_DDI_CLOCK_SELECT_DIV18CLK                0x9
> >-- 
> >2.44.2
> >