On Wed, 03 Apr 2024, Balasubramani Vivekanandan <balasubramani.vivekanan...@intel.com> wrote: > New platforms have different addresses for C20 PLL registers. This patch > prepares the driver to work with different register addresses. > New structure `struct intel_c20pll_reg` is created to hold the register > addresses for each platform with different register address.
Absolutely not a fan. We have so many ways to handle register offsets, and this adds another one, completely different from the rest. Most other places that have complex conditions for choosing a register have a function to pick the register offset. BR, Jani. > > CC: Clint Taylor <clinton.a.tay...@intel.com> > Signed-off-by: Balasubramani Vivekanandan > <balasubramani.vivekanan...@intel.com> > --- > drivers/gpu/drm/i915/display/intel_cx0_phy.c | 53 +++++++++++++------ > .../gpu/drm/i915/display/intel_cx0_phy_regs.h | 36 ++++++++++--- > 2 files changed, 65 insertions(+), 24 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c > b/drivers/gpu/drm/i915/display/intel_cx0_phy.c > index a2c4bf33155f..13a2e3db2812 100644 > --- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c > +++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c > @@ -759,6 +759,17 @@ static const struct intel_c10pll_state * const > mtl_c10_edp_tables[] = { > NULL, > }; > > +static struct intel_c20pll_reg mtl_c20_reg = { > + .tx_cnt_a = MTL_C20_A_TX_CNTX_CFG_ADDR, > + .tx_cnt_b = MTL_C20_B_TX_CNTX_CFG_ADDR, > + .cmn_cnt_a = MTL_C20_A_CMN_CNTX_CFG_ADDR, > + .cmn_cnt_b = MTL_C20_B_CMN_CNTX_CFG_ADDR, > + .mplla_a = MTL_C20_A_MPLLA_CFG_ADDR, > + .mplla_b = MTL_C20_B_MPLLA_CFG_ADDR, > + .mpllb_a = MTL_C20_A_MPLLB_CFG_ADDR, > + .mpllb_b = MTL_C20_B_MPLLB_CFG_ADDR > +}; > + > /* C20 basic DP 1.4 tables */ > static const struct intel_c20pll_state mtl_c20_dp_rbr = { > .clock = 162000, > @@ -2161,6 +2172,7 @@ static void intel_c20pll_readout_hw_state(struct > intel_encoder *encoder, > bool cntx; > intel_wakeref_t wakeref; > int i; > + struct intel_c20pll_reg *pll_reg = &mtl_c20_reg; > > wakeref = intel_cx0_phy_transaction_begin(encoder); > > @@ -2171,20 +2183,20 @@ static void intel_c20pll_readout_hw_state(struct > intel_encoder *encoder, > for (i = 0; i < ARRAY_SIZE(pll_state->tx); i++) { > if (cntx) > pll_state->tx[i] = intel_c20_sram_read(encoder, > INTEL_CX0_LANE0, > - > PHY_C20_B_TX_CNTX_CFG(i)); > + > PHY_C20_B_TX_CNTX_CFG(pll_reg, i)); > else > pll_state->tx[i] = intel_c20_sram_read(encoder, > INTEL_CX0_LANE0, > - > PHY_C20_A_TX_CNTX_CFG(i)); > + > PHY_C20_A_TX_CNTX_CFG(pll_reg, i)); > } > > /* Read common configuration */ > for (i = 0; i < ARRAY_SIZE(pll_state->cmn); i++) { > if (cntx) > pll_state->cmn[i] = intel_c20_sram_read(encoder, > INTEL_CX0_LANE0, > - > PHY_C20_B_CMN_CNTX_CFG(i)); > + > PHY_C20_B_CMN_CNTX_CFG(pll_reg, i)); > else > pll_state->cmn[i] = intel_c20_sram_read(encoder, > INTEL_CX0_LANE0, > - > PHY_C20_A_CMN_CNTX_CFG(i)); > + > PHY_C20_A_CMN_CNTX_CFG(pll_reg, i)); > } > > if (intel_c20phy_use_mpllb(pll_state)) { > @@ -2192,20 +2204,20 @@ static void intel_c20pll_readout_hw_state(struct > intel_encoder *encoder, > for (i = 0; i < ARRAY_SIZE(pll_state->mpllb); i++) { > if (cntx) > pll_state->mpllb[i] = > intel_c20_sram_read(encoder, INTEL_CX0_LANE0, > - > PHY_C20_B_MPLLB_CNTX_CFG(i)); > + > PHY_C20_B_MPLLB_CNTX_CFG(pll_reg, i)); > else > pll_state->mpllb[i] = > intel_c20_sram_read(encoder, INTEL_CX0_LANE0, > - > PHY_C20_A_MPLLB_CNTX_CFG(i)); > + > PHY_C20_A_MPLLB_CNTX_CFG(pll_reg, i)); > } > } else { > /* MPLLA configuration */ > for (i = 0; i < ARRAY_SIZE(pll_state->mplla); i++) { > if (cntx) > pll_state->mplla[i] = > intel_c20_sram_read(encoder, INTEL_CX0_LANE0, > - > PHY_C20_B_MPLLA_CNTX_CFG(i)); > + > PHY_C20_B_MPLLA_CNTX_CFG(pll_reg, i)); > else > pll_state->mplla[i] = > intel_c20_sram_read(encoder, INTEL_CX0_LANE0, > - > PHY_C20_A_MPLLA_CNTX_CFG(i)); > + > PHY_C20_A_MPLLA_CNTX_CFG(pll_reg, i)); > } > } > > @@ -2341,6 +2353,7 @@ static void intel_c20_pll_program(struct > drm_i915_private *i915, > u32 clock = crtc_state->port_clock; > bool cntx; > int i; > + const struct intel_c20pll_reg *pll_reg = &mtl_c20_reg; > > if (intel_crtc_has_dp_encoder(crtc_state)) > dp = true; > @@ -2363,17 +2376,25 @@ static void intel_c20_pll_program(struct > drm_i915_private *i915, > /* 3.1 Tx configuration */ > for (i = 0; i < ARRAY_SIZE(pll_state->tx); i++) { > if (cntx) > - intel_c20_sram_write(encoder, INTEL_CX0_LANE0, > PHY_C20_A_TX_CNTX_CFG(i), pll_state->tx[i]); > + intel_c20_sram_write(encoder, INTEL_CX0_LANE0, > + PHY_C20_A_TX_CNTX_CFG(pll_reg, i), > + pll_state->tx[i]); > else > - intel_c20_sram_write(encoder, INTEL_CX0_LANE0, > PHY_C20_B_TX_CNTX_CFG(i), pll_state->tx[i]); > + intel_c20_sram_write(encoder, INTEL_CX0_LANE0, > + PHY_C20_B_TX_CNTX_CFG(pll_reg, i), > + pll_state->tx[i]); > } > > /* 3.2 common configuration */ > for (i = 0; i < ARRAY_SIZE(pll_state->cmn); i++) { > if (cntx) > - intel_c20_sram_write(encoder, INTEL_CX0_LANE0, > PHY_C20_A_CMN_CNTX_CFG(i), pll_state->cmn[i]); > + intel_c20_sram_write(encoder, INTEL_CX0_LANE0, > + PHY_C20_A_CMN_CNTX_CFG(pll_reg, i), > + pll_state->cmn[i]); > else > - intel_c20_sram_write(encoder, INTEL_CX0_LANE0, > PHY_C20_B_CMN_CNTX_CFG(i), pll_state->cmn[i]); > + intel_c20_sram_write(encoder, INTEL_CX0_LANE0, > + PHY_C20_B_CMN_CNTX_CFG(pll_reg, i), > + pll_state->cmn[i]); > } > > /* 3.3 mpllb or mplla configuration */ > @@ -2381,22 +2402,22 @@ static void intel_c20_pll_program(struct > drm_i915_private *i915, > for (i = 0; i < ARRAY_SIZE(pll_state->mpllb); i++) { > if (cntx) > intel_c20_sram_write(encoder, INTEL_CX0_LANE0, > - > PHY_C20_A_MPLLB_CNTX_CFG(i), > + > PHY_C20_A_MPLLB_CNTX_CFG(pll_reg, i), > pll_state->mpllb[i]); > else > intel_c20_sram_write(encoder, INTEL_CX0_LANE0, > - > PHY_C20_B_MPLLB_CNTX_CFG(i), > + > PHY_C20_B_MPLLB_CNTX_CFG(pll_reg, i), > pll_state->mpllb[i]); > } > } else { > for (i = 0; i < ARRAY_SIZE(pll_state->mplla); i++) { > if (cntx) > intel_c20_sram_write(encoder, INTEL_CX0_LANE0, > - > PHY_C20_A_MPLLA_CNTX_CFG(i), > + > PHY_C20_A_MPLLA_CNTX_CFG(pll_reg, i), > pll_state->mplla[i]); > else > intel_c20_sram_write(encoder, INTEL_CX0_LANE0, > - > PHY_C20_B_MPLLA_CNTX_CFG(i), > + > PHY_C20_B_MPLLA_CNTX_CFG(pll_reg, i), > pll_state->mplla[i]); > } > } > 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..882b98dc347b 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,44 @@ > #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)) > +#define PHY_C20_A_TX_CNTX_CFG(reg, idx) ((reg)->tx_cnt_a - > (idx)) > +#define PHY_C20_B_TX_CNTX_CFG(reg, idx) ((reg)->tx_cnt_b - > (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(reg, idx) ((reg)->cmn_cnt_a - (idx)) > +#define PHY_C20_B_CMN_CNTX_CFG(reg, idx) ((reg)->cmn_cnt_b - (idx)) > +#define PHY_C20_A_MPLLA_CNTX_CFG(reg, idx) ((reg)->mplla_a - (idx)) > +#define PHY_C20_B_MPLLA_CNTX_CFG(reg, idx) ((reg)->mplla_b - (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(reg, idx) ((reg)->mpllb_a - (idx)) > +#define PHY_C20_B_MPLLB_CNTX_CFG(reg, idx) ((reg)->mpllb_b - (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) > > +struct intel_c20pll_reg { > + u16 tx_cnt_a; > + u16 tx_cnt_b; > + u16 cmn_cnt_a; > + u16 cmn_cnt_b; > + u16 mplla_a; > + u16 mplla_b; > + u16 mpllb_a; > + u16 mpllb_b; > +}; > + > +#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 > + > /* 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