On Mon, 15 Apr 2024, Balasubramani Vivekanandan <balasubramani.vivekanandan@xxxxxxxxx> wrote: > 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 bdd0c8c4ef97..23a79e911972 100644 > --- a/drivers/gpu/drm/i915/display/intel_cx0_phy_regs.h > +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy_regs.h > @@ -254,24 +254,67 @@ > #define PHY_C20_VDR_CUSTOM_WIDTH 0xD02 > #define PHY_C20_CUSTOM_WIDTH_MASK REG_GENMASK(1, 0) > #define PHY_C20_CUSTOM_WIDTH(val) REG_FIELD_PREP8(PHY_C20_CUSTOM_WIDTH_MASK, val) > -#define PHY_C20_A_TX_CNTX_CFG(idx) (0xCF2E - (idx)) > -#define PHY_C20_B_TX_CNTX_CFG(idx) (0xCF2A - (idx)) The style absolutely everywhere is to define the values with prefix underscore right above the macro that uses them. Please just look around. Look at the big comment near the top of i915_reg.h. No need for the _ADDR suffix. Please also add a helper to choose which one to use instead of duplicating absolutely everywhere. Something like this: #define _IS_XE2HPD_C20(i915) (DISPLAY_VER_FULL(i915) == IP_VER(14, 1)) #define _MTL_C20_A_TX_CNTX_CFG 0xcf2e #define _XE2HPD_C20_A_TX_CNTX_CFG 0xcf5e #define PHY_C20_A_TX_CNTX_CFG(i915, idx) \ ((_IS_XE2HPD_C20(i915) ? _XE2HPD_C20_A_TX_CNTX_CFG : _MTL_C20_A_TX_CNTX_CFG) - (idx)) And that's enough wrapping, no need to split across four lines. BR, Jani. > + > +#define PHY_C20_A_TX_CNTX_CFG(i915, idx) \ > + (((DISPLAY_VER_FULL(i915) == IP_VER(14, 1)) ? \ > + XE2HPD_C20_A_TX_CNTX_CFG_ADDR : MTL_C20_A_TX_CNTX_CFG_ADDR) - \ > + (idx)) > +#define PHY_C20_B_TX_CNTX_CFG(i915, idx) \ > + (((DISPLAY_VER_FULL(i915) == IP_VER(14, 1)) ? \ > + XE2HPD_C20_B_TX_CNTX_CFG_ADDR : MTL_C20_B_TX_CNTX_CFG_ADDR) - \ > + (idx)) > #define C20_PHY_TX_RATE REG_GENMASK(2, 0) > -#define PHY_C20_A_CMN_CNTX_CFG(idx) (0xCDAA - (idx)) > -#define PHY_C20_B_CMN_CNTX_CFG(idx) (0xCDA5 - (idx)) > -#define PHY_C20_A_MPLLA_CNTX_CFG(idx) (0xCCF0 - (idx)) > -#define PHY_C20_B_MPLLA_CNTX_CFG(idx) (0xCCE5 - (idx)) > +#define PHY_C20_A_CMN_CNTX_CFG(i915, idx) \ > + (((DISPLAY_VER_FULL(i915) == IP_VER(14, 1)) ? \ > + XE2HPD_C20_A_CMN_CNTX_CFG_ADDR : MTL_C20_A_CMN_CNTX_CFG_ADDR) - \ > + (idx)) > +#define PHY_C20_B_CMN_CNTX_CFG(i915, idx) \ > + (((DISPLAY_VER_FULL(i915) == IP_VER(14, 1)) ? \ > + XE2HPD_C20_B_CMN_CNTX_CFG_ADDR : MTL_C20_B_CMN_CNTX_CFG_ADDR) - \ > + (idx)) > +#define PHY_C20_A_MPLLA_CNTX_CFG(i915, idx) \ > + (((DISPLAY_VER_FULL(i915) == IP_VER(14, 1)) ? \ > + XE2HPD_C20_A_MPLLA_CFG_ADDR : MTL_C20_A_MPLLA_CFG_ADDR) - \ > + (idx)) > +#define PHY_C20_B_MPLLA_CNTX_CFG(i915, idx) \ > + (((DISPLAY_VER_FULL(i915) == IP_VER(14, 1)) ? \ > + XE2HPD_C20_B_MPLLA_CFG_ADDR : MTL_C20_B_MPLLA_CFG_ADDR) - \ > + (idx)) > #define C20_MPLLA_FRACEN REG_BIT(14) > #define C20_FB_CLK_DIV4_EN REG_BIT(13) > #define C20_MPLLA_TX_CLK_DIV_MASK REG_GENMASK(10, 8) > -#define PHY_C20_A_MPLLB_CNTX_CFG(idx) (0xCB5A - (idx)) > -#define PHY_C20_B_MPLLB_CNTX_CFG(idx) (0xCB4E - (idx)) > +#define PHY_C20_A_MPLLB_CNTX_CFG(i915, idx) \ > + (((DISPLAY_VER_FULL(i915) == IP_VER(14, 1)) ? \ > + XE2HPD_C20_A_MPLLB_CFG_ADDR : MTL_C20_A_MPLLB_CFG_ADDR) - \ > + (idx)) > +#define PHY_C20_B_MPLLB_CNTX_CFG(i915, idx) \ > + (((DISPLAY_VER_FULL(i915) == IP_VER(14, 1)) ? \ > + XE2HPD_C20_B_MPLLB_CFG_ADDR : MTL_C20_B_MPLLB_CFG_ADDR) - \ > + (idx)) > #define C20_MPLLB_TX_CLK_DIV_MASK REG_GENMASK(15, 13) > #define C20_MPLLB_FRACEN REG_BIT(13) > #define C20_REF_CLK_MPLLB_DIV_MASK REG_GENMASK(12, 10) > #define C20_MULTIPLIER_MASK REG_GENMASK(11, 0) > #define C20_PHY_USE_MPLLB REG_BIT(7) > > +#define MTL_C20_A_TX_CNTX_CFG_ADDR 0xCF2E > +#define MTL_C20_B_TX_CNTX_CFG_ADDR 0xCF2A > +#define MTL_C20_A_CMN_CNTX_CFG_ADDR 0xCDAA > +#define MTL_C20_B_CMN_CNTX_CFG_ADDR 0xCDA5 > +#define MTL_C20_A_MPLLA_CFG_ADDR 0xCCF0 > +#define MTL_C20_B_MPLLA_CFG_ADDR 0xCCE5 > +#define MTL_C20_A_MPLLB_CFG_ADDR 0xCB5A > +#define MTL_C20_B_MPLLB_CFG_ADDR 0xCB4E > + > +#define XE2HPD_C20_A_TX_CNTX_CFG_ADDR 0xCF5E > +#define XE2HPD_C20_B_TX_CNTX_CFG_ADDR 0xCF5A > +#define XE2HPD_C20_A_CMN_CNTX_CFG_ADDR 0xCE8E > +#define XE2HPD_C20_B_CMN_CNTX_CFG_ADDR 0xCE89 > +#define XE2HPD_C20_A_MPLLA_CFG_ADDR 0xCE58 > +#define XE2HPD_C20_B_MPLLA_CFG_ADDR 0xCE4D > +#define XE2HPD_C20_A_MPLLB_CFG_ADDR 0xCCC2 > +#define XE2HPD_C20_B_MPLLB_CFG_ADDR 0xCCB6 > + > /* C20 Phy VSwing Masks */ > #define C20_PHY_VSWING_PREEMPH_MASK REG_GENMASK8(5, 0) > #define C20_PHY_VSWING_PREEMPH(val) REG_FIELD_PREP8(C20_PHY_VSWING_PREEMPH_MASK, val) -- Jani Nikula, Intel
