Re: [Intel-gfx] [PATCH 9/9] drm/i915: Address broxton phy registers based on phy and channel number

[Imre Deak]

On ke, 2016-10-05 at 15:09 +0300, Ander Conselvan de Oliveira wrote:
> The port registers related to the phys in broxton map to different
> channels and specific phys. Make that mapping explicit.
> 
> Signed-off-by: Ander Conselvan de Oliveira 
> ---
>  drivers/gpu/drm/i915/i915_drv.h   |   2 +
>  drivers/gpu/drm/i915/i915_reg.h   | 211 
> +-
>  drivers/gpu/drm/i915/intel_dpio_phy.c | 131 +++--
>  drivers/gpu/drm/i915/intel_dpll_mgr.c |  84 --
>  4 files changed, 222 insertions(+), 206 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 77f1374..c3fa29a 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3735,6 +3735,8 @@ u32 vlv_flisdsi_read(struct drm_i915_private *dev_priv, 
> u32 reg);
>  void vlv_flisdsi_write(struct drm_i915_private *dev_priv, u32 reg, u32 val);
>  
>  /* intel_dpio_phy.c */
> +void bxt_port_to_phy_channel(enum port port,
> +  u32 *phy, enum dpio_channel *ch);
>  void bxt_ddi_phy_set_signal_level(struct drm_i915_private *dev_priv,
>     enum port port, u32 margin, u32 scale,
>     u32 enable, u32 deemphasis);
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index d3802c6..416cbb1 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1187,6 +1187,9 @@ enum skl_disp_power_wells {
>  #define   DPIO_UPAR_SHIFT30
>  
>  /* BXT PHY registers */
> +#define BXT_PHY1_BASE0x162000
> +#define BXT_PHY0_BASE0x6C000
> +
>  #define _BXT_PHY(phy, a, b)  _MMIO_PIPE((phy), (a), (b))
>  
>  #define BXT_P_CR_GT_DISP_PWRON   _MMIO(0x138090)
> @@ -1216,31 +1219,26 @@ enum skl_disp_power_wells {
>  #define   PORT_PLL_REF_SEL   (1 << 27)
>  #define BXT_PORT_PLL_ENABLE(port)_MMIO_PORT(port, _PORT_PLL_A, 
> _PORT_PLL_B)
>  
> -#define _PORT_PLL_EBB_0_A0x162034
> -#define _PORT_PLL_EBB_0_B0x6C034
> -#define _PORT_PLL_EBB_0_C0x6C340
> +#define _PORT_PLL_EBB_0_CH0  0x34
> +#define _PORT_PLL_EBB_0_CH1  0x340
>  #define   PORT_PLL_P1_SHIFT  13
>  #define   PORT_PLL_P1_MASK   (0x07 << PORT_PLL_P1_SHIFT)
>  #define   PORT_PLL_P1(x) ((x)  << PORT_PLL_P1_SHIFT)
>  #define   PORT_PLL_P2_SHIFT  8
>  #define   PORT_PLL_P2_MASK   (0x1f << PORT_PLL_P2_SHIFT)
>  #define   PORT_PLL_P2(x) ((x)  << PORT_PLL_P2_SHIFT)
> -#define BXT_PORT_PLL_EBB_0(port) _MMIO_PORT3(port, _PORT_PLL_EBB_0_A, \
> - _PORT_PLL_EBB_0_B,  \
> - _PORT_PLL_EBB_0_C)
> +#define BXT_PORT_PLL_EBB_0(base, ch) \
> + _MMIO((base) + _PIPE((ch), _PORT_PLL_EBB_0_CH0, _PORT_PLL_EBB_0_CH1))

Passing the phy and calculating the base here would be more natural.
Also we should try to preserve bspec reg names and addresses to make
lookup easier. You could do that by using _PORT_PLL_EBB_0_B
and _PORT_PLL_EBB_0_C here and adjusting the base by -BXT_PHY0_BASE.
I'd also leave _PORT_PLL_EBB_0_A around for reference.

>  
> -#define _PORT_PLL_EBB_4_A0x162038
> -#define _PORT_PLL_EBB_4_B0x6C038
> -#define _PORT_PLL_EBB_4_C0x6C344
> +#define _PORT_PLL_EBB_4_CH0  0x38
> +#define _PORT_PLL_EBB_4_CH1  0x344
>  #define   PORT_PLL_10BIT_CLK_ENABLE  (1 << 13)
>  #define   PORT_PLL_RECALIBRATE   (1 << 14)
> -#define BXT_PORT_PLL_EBB_4(port) _MMIO_PORT3(port, _PORT_PLL_EBB_4_A, \
> - _PORT_PLL_EBB_4_B,  \
> - _PORT_PLL_EBB_4_C)
> +#define BXT_PORT_PLL_EBB_4(base, ch) \
> + _MMIO((base) + _PIPE((ch), _PORT_PLL_EBB_4_CH0, _PORT_PLL_EBB_4_CH1))
>  
> -#define _PORT_PLL_0_A0x162100
> -#define _PORT_PLL_0_B0x6C100
> -#define _PORT_PLL_0_C0x6C380
> +#define _PORT_PLL_0_CH0  0x100
> +#define _PORT_PLL_0_CH1  0x380
>  /* PORT_PLL_0_A */
>  #define   PORT_PLL_M2_MASK   0xFF
>  /* PORT_PLL_1_A */
> @@ -1267,65 +1265,43 @@ enum skl_disp_power_wells {
>  #define  PORT_PLL_DCO_AMP_DEFAULT15
>  #define  PORT_PLL_DCO_AMP_MASK   0x3c00
>  #define  PORT_PLL_DCO_AMP(x) ((x)<<10)
> -#define _PORT_PLL_BASE(port) _PORT3(port, _PORT_PLL_0_A, \
> - _PORT_PLL_0_B,  \
> - _PORT_PLL_0_C)
> -#define BXT_PORT_PLL(port, idx)  _MMIO(_PORT_PLL_BASE(port) + 
> (idx) * 4)
> +#define _PORT_PLL_BASE(base, ch) \
> + ((base) + _PIPE((ch), _PORT_PLL_0_CH0, _PORT_PLL_0_CH1))
> +#define 

[Intel-gfx] [PATCH 9/9] drm/i915: Address broxton phy registers based on phy and channel number

[Ander Conselvan de Oliveira]

The port registers related to the phys in broxton map to different
channels and specific phys. Make that mapping explicit.

Signed-off-by: Ander Conselvan de Oliveira 

---
 drivers/gpu/drm/i915/i915_drv.h   |   2 +
 drivers/gpu/drm/i915/i915_reg.h   | 211 +-
 drivers/gpu/drm/i915/intel_dpio_phy.c | 131 +++--
 drivers/gpu/drm/i915/intel_dpll_mgr.c |  84 --
 4 files changed, 222 insertions(+), 206 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 77f1374..c3fa29a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3735,6 +3735,8 @@ u32 vlv_flisdsi_read(struct drm_i915_private *dev_priv, 
u32 reg);
 void vlv_flisdsi_write(struct drm_i915_private *dev_priv, u32 reg, u32 val);
 
 /* intel_dpio_phy.c */
+void bxt_port_to_phy_channel(enum port port,
+u32 *phy, enum dpio_channel *ch);
 void bxt_ddi_phy_set_signal_level(struct drm_i915_private *dev_priv,
  enum port port, u32 margin, u32 scale,
  u32 enable, u32 deemphasis);
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index d3802c6..416cbb1 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1187,6 +1187,9 @@ enum skl_disp_power_wells {
 #define   DPIO_UPAR_SHIFT  30
 
 /* BXT PHY registers */
+#define BXT_PHY1_BASE  0x162000
+#define BXT_PHY0_BASE  0x6C000
+
 #define _BXT_PHY(phy, a, b)_MMIO_PIPE((phy), (a), (b))
 
 #define BXT_P_CR_GT_DISP_PWRON _MMIO(0x138090)
@@ -1216,31 +1219,26 @@ enum skl_disp_power_wells {
 #define   PORT_PLL_REF_SEL (1 << 27)
 #define BXT_PORT_PLL_ENABLE(port)  _MMIO_PORT(port, _PORT_PLL_A, 
_PORT_PLL_B)
 
-#define _PORT_PLL_EBB_0_A  0x162034
-#define _PORT_PLL_EBB_0_B  0x6C034
-#define _PORT_PLL_EBB_0_C  0x6C340
+#define _PORT_PLL_EBB_0_CH00x34
+#define _PORT_PLL_EBB_0_CH10x340
 #define   PORT_PLL_P1_SHIFT13
 #define   PORT_PLL_P1_MASK (0x07 << PORT_PLL_P1_SHIFT)
 #define   PORT_PLL_P1(x)   ((x)  << PORT_PLL_P1_SHIFT)
 #define   PORT_PLL_P2_SHIFT8
 #define   PORT_PLL_P2_MASK (0x1f << PORT_PLL_P2_SHIFT)
 #define   PORT_PLL_P2(x)   ((x)  << PORT_PLL_P2_SHIFT)
-#define BXT_PORT_PLL_EBB_0(port)   _MMIO_PORT3(port, _PORT_PLL_EBB_0_A, \
-   _PORT_PLL_EBB_0_B,  \
-   _PORT_PLL_EBB_0_C)
+#define BXT_PORT_PLL_EBB_0(base, ch)   \
+   _MMIO((base) + _PIPE((ch), _PORT_PLL_EBB_0_CH0, _PORT_PLL_EBB_0_CH1))
 
-#define _PORT_PLL_EBB_4_A  0x162038
-#define _PORT_PLL_EBB_4_B  0x6C038
-#define _PORT_PLL_EBB_4_C  0x6C344
+#define _PORT_PLL_EBB_4_CH00x38
+#define _PORT_PLL_EBB_4_CH10x344
 #define   PORT_PLL_10BIT_CLK_ENABLE(1 << 13)
 #define   PORT_PLL_RECALIBRATE (1 << 14)
-#define BXT_PORT_PLL_EBB_4(port)   _MMIO_PORT3(port, _PORT_PLL_EBB_4_A, \
-   _PORT_PLL_EBB_4_B,  \
-   _PORT_PLL_EBB_4_C)
+#define BXT_PORT_PLL_EBB_4(base, ch)   \
+   _MMIO((base) + _PIPE((ch), _PORT_PLL_EBB_4_CH0, _PORT_PLL_EBB_4_CH1))
 
-#define _PORT_PLL_0_A  0x162100
-#define _PORT_PLL_0_B  0x6C100
-#define _PORT_PLL_0_C  0x6C380
+#define _PORT_PLL_0_CH00x100
+#define _PORT_PLL_0_CH10x380
 /* PORT_PLL_0_A */
 #define   PORT_PLL_M2_MASK 0xFF
 /* PORT_PLL_1_A */
@@ -1267,65 +1265,43 @@ enum skl_disp_power_wells {
 #define  PORT_PLL_DCO_AMP_DEFAULT  15
 #define  PORT_PLL_DCO_AMP_MASK 0x3c00
 #define  PORT_PLL_DCO_AMP(x)   ((x)<<10)
-#define _PORT_PLL_BASE(port)   _PORT3(port, _PORT_PLL_0_A, \
-   _PORT_PLL_0_B,  \
-   _PORT_PLL_0_C)
-#define BXT_PORT_PLL(port, idx)_MMIO(_PORT_PLL_BASE(port) + 
(idx) * 4)
+#define _PORT_PLL_BASE(base, ch)   \
+   ((base) + _PIPE((ch), _PORT_PLL_0_CH0, _PORT_PLL_0_CH1))
+#define BXT_PORT_PLL(base, ch, idx)\
+   _MMIO(_PORT_PLL_BASE(base, ch) + (idx) * 4)
 
 /* BXT PHY common lane registers */
-#define _PORT_CL1CM_DW0_A  0x162000
-#define _PORT_CL1CM_DW0_BC 0x6C000
+#define BXT_PORT_CL1CM_DW0(base)   _MMIO((base) + 0x0)
 #define   PHY_POWER_GOOD   (1 << 16)
 #define   PHY_RESERVED (1 << 7)
-#define BXT_PORT_CL1CM_DW0(phy)_BXT_PHY((phy), 
_PORT_CL1CM_DW0_BC, \
-