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

Reply via email to