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

[Kahola, Mika]

> -----Original Message-----
> From: Deak, Imre <imre.deak at intel.com>
> Sent: Monday, 12 May 2025 23.26
> To: Sousa, Gustavo <gustavo.sousa at intel.com>
> Cc: intel-gfx at lists.freedesktop.org; intel-xe at lists.freedesktop.org; Kahola, Mika
> <mika.kahola at intel.com>; stable at vger.kernel.org
> Subject: Re: [PATCH] drm/i915/ptl: Use everywhere the correct DDI port clock
> select mask
> 
> 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.

Bits 16-17 are not defined or marked as reserved, so Imre's approach seems safe.

Tested-by: Mika Kahola <mika.kahola at intel.com>
Reviewed-by: Mika Kahola <mika.kahola at intel.com>

> 
> > 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
> > >