On Tue, 19 May 2026, Luca Coelho <[email protected]> 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, Type-C/SNPS PHYs) > is 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]> > --- > drivers/gpu/drm/i915/display/intel_display_regs.h | 13 ++++++++++--- > 1 file changed, 10 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..bc90a21e8b46 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_regs.h > +++ b/drivers/gpu/drm/i915/display/intel_display_regs.h > @@ -2869,9 +2869,16 @@ 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) > + * is not valid here. Cast to unsigned so the shift is always > + * well-defined. > + */ > +#define ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(phy) ((unsigned int)(phy) * > 2) > +#define ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_MASK(phy) ((u32)3 << > ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(phy)) > +#define ICL_DPCLKA_CFGCR0_DDI_CLK_SEL(pll, phy) ((u32)(pll) << > ICL_DPCLKA_CFGCR0_DDI_CLK_SEL_SHIFT(phy)) > +
Here too, please avoid direct casts; we have the REG_* helpers exactly for this purpose. BR, Jani. > #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)) -- Jani Nikula, Intel
