On Tue, 9 Jun 2026, Luca Coelho wrote:
ICL_DPCLKA_CFGCR0 has a 2-bit DDI_CLK_SEL field per combo PHY, for
PHY_A..PHY_D only. Any other phy value (PHY_NONE, TypeC/SNPS PHYs) is
FWIW, double space: s/ / /
not valid here.
This is not a problem with the current implementation, because phy is
always valid when these macros are called, but it's more robust to
cast to unsigned so the shift is always well-defined.
Signed-off-by: Luca Coelho <[email protected]>
---
.../gpu/drm/i915/display/intel_display_regs.h | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display_regs.h
b/drivers/gpu/drm/i915/display/intel_display_regs.h
index 4321f8b529da..2dcbbad70174 100644
--- a/drivers/gpu/drm/i915/display/intel_display_regs.h
+++ b/drivers/gpu/drm/i915/display/intel_display_regs.h
@@ -2869,9 +2869,19 @@ enum skl_power_gate {
#define ICL_DPCLKA_CFGCR0_TC_CLK_OFF(tc_port) (1 << ((tc_port) < TC_PORT_4 ? \
(tc_port) + 12 : \
(tc_port) - TC_PORT_4 +
21))
-#define ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(phy) ((phy) * 2)
-#define ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_MASK(phy) (3 <<
ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(phy))
-#define ICL_DPCLKA_CFGCR0_DDI_CLK_SEL(pll, phy) ((pll) <<
ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(phy))
+
+/*
+ * ICL_DPCLKA_CFGCR0 has a 2-bit DDI_CLK_SEL field per combo PHY, for
+ * PHY_A..PHY_D only. Any other phy value (PHY_NONE, TypeC/SNPS PHYs)
double space: s/ / /
Reviewed-by: Michał Grzelak <[email protected]>
BR,
Michał
+ * is not valid here.
+ */
+#define ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(phy) _PICK(phy & 0x3, 0, 2,
4, 6)
+#define ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_MASK(phy) \
+ REG_GENMASK(ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(phy) + 1, \
+ ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(phy))
+#define ICL_DPCLKA_CFGCR0_DDI_CLK_SEL(pll, phy) \
+ ((pll) << ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(phy))
+
#define RKL_DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(phy) _PICK(phy, 0, 2, 4, 27)
#define RKL_DPCLKA_CFGCR0_DDI_CLK_SEL_MASK(phy) \
(3 << RKL_DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(phy))
