[Intel-gfx] [PATCH v2 1/2] drm/i915/icl: Add remaining registers and bitfields for MG PHY DDI

Manasi Navare manasi.d.navare at intel.com
Fri Jul 13 18:44:13 UTC 2018


Thanks for the review comments.

On Thu, Jul 12, 2018 at 05:00:42PM -0700, Paulo Zanoni wrote:
> Em Qui, 2018-06-28 às 15:35 -0700, Manasi Navare escreveu:
> > This patch adds the remaining register definitions and bit fields
> > required for MG PHy DDI buffer initializations and voltage
> > swing programming for MG PHy DDI ports.
> > 
> > While at it this patch also fixes the naming for previously defined
> > MG PHY registers in original commit id (c92f47b5ec977a "drm/i915/icl:
> > Add register defs for voltage swing sequences for MG PHY DDI").
> > Since the MG PHY registers are first defined in ICL platform, there
> > is no need for _ICL prefix.
> 
> IMHO drive-by "do this other trivial thing"s are fine when it's only 1
> or 2 small chunks affected. In this case it's half of the patch, so I
> would prefer having it in a separate patch. But let's leave it as-is
> here since at least the registers being touched are not used.
> 
> More below.
> 
> > 
> > v2:
> > * Change the MG_TX_DRVCTL registers names to match the spec (Anusha)
> > 
> > Signed-off-by: Manasi Navare <manasi.d.navare at intel.com>
> > Cc: Paulo Zanoni <paulo.r.zanoni at intel.com>
> > Cc: James Ausmus <james.ausmus at intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h | 246 +++++++++++++++++++++++-------
> > ----------
> >  1 file changed, 145 insertions(+), 101 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index c30cfcd..6119acc 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -1872,121 +1872,165 @@ enum i915_power_well_id {
> >  #define   N_SCALAR(x)			((x) << 24)
> >  #define   N_SCALAR_MASK			(0x7F << 24)
> >  
> > -#define _ICL_MG_PHY_PORT_LN(port, ln, ln0p1, ln0p2, ln1p1) \
> > +#define MG_PHY_PORT_LN(port, ln, ln0p1, ln0p2, ln1p1) \
> >  	_MMIO(_PORT((port) - PORT_C, ln0p1, ln0p2) + (ln) * ((ln1p1)
> > - (ln0p1)))
> >  
> > -#define _ICL_MG_TX_LINK_PARAMS_TX1LN0_PORT1		0x16812C
> > -#define _ICL_MG_TX_LINK_PARAMS_TX1LN1_PORT1		0x16852C
> > -#define _ICL_MG_TX_LINK_PARAMS_TX1LN0_PORT2		0x16912C
> > -#define _ICL_MG_TX_LINK_PARAMS_TX1LN1_PORT2		0x16952C
> > -#define _ICL_MG_TX_LINK_PARAMS_TX1LN0_PORT3		0x16A12C
> > -#define _ICL_MG_TX_LINK_PARAMS_TX1LN1_PORT3		0x16A52C
> > -#define _ICL_MG_TX_LINK_PARAMS_TX1LN0_PORT4		0x16B12C
> > -#define _ICL_MG_TX_LINK_PARAMS_TX1LN1_PORT4		0x16B52C
> > -#define ICL_PORT_MG_TX1_LINK_PARAMS(port, ln) \
> > -	_ICL_MG_PHY_PORT_LN(port, ln,
> > _ICL_MG_TX_LINK_PARAMS_TX1LN0_PORT1, \
> > -				      _ICL_MG_TX_LINK_PARAMS_TX1LN0_
> > PORT2, \
> > -				      _ICL_MG_TX_LINK_PARAMS_TX1LN1_
> > PORT1)
> > -
> > -#define _ICL_MG_TX_LINK_PARAMS_TX2LN0_PORT1		0x1680AC
> > -#define _ICL_MG_TX_LINK_PARAMS_TX2LN1_PORT1		0x1684AC
> > -#define _ICL_MG_TX_LINK_PARAMS_TX2LN0_PORT2		0x1690AC
> > -#define _ICL_MG_TX_LINK_PARAMS_TX2LN1_PORT2		0x1694AC
> > -#define _ICL_MG_TX_LINK_PARAMS_TX2LN0_PORT3		0x16A0AC
> > -#define _ICL_MG_TX_LINK_PARAMS_TX2LN1_PORT3		0x16A4AC
> > -#define _ICL_MG_TX_LINK_PARAMS_TX2LN0_PORT4		0x16B0AC
> > -#define _ICL_MG_TX_LINK_PARAMS_TX2LN1_PORT4		0x16B4AC
> > -#define ICL_PORT_MG_TX2_LINK_PARAMS(port, ln) \
> > -	_ICL_MG_PHY_PORT_LN(port, ln,
> > _ICL_MG_TX_LINK_PARAMS_TX2LN0_PORT1, \
> > -				      _ICL_MG_TX_LINK_PARAMS_TX2LN0_
> > PORT2, \
> > -				      _ICL_MG_TX_LINK_PARAMS_TX2LN1_
> > PORT1)
> > +#define MG_TX_LINK_PARAMS_TX1LN0_PORT1		0x16812C
> > +#define MG_TX_LINK_PARAMS_TX1LN1_PORT1		0x16852C
> > +#define MG_TX_LINK_PARAMS_TX1LN0_PORT2		0x16912C
> > +#define MG_TX_LINK_PARAMS_TX1LN1_PORT2		0x16952C
> > +#define MG_TX_LINK_PARAMS_TX1LN0_PORT3		0x16A12C
> > +#define MG_TX_LINK_PARAMS_TX1LN1_PORT3		0x16A52C
> > +#define MG_TX_LINK_PARAMS_TX1LN0_PORT4		0x16B12C
> > +#define _MG_TX_LINK_PARAMS_TX1LN1_PORT4		0x16B52C
> 
> You left an underline on the line above.

Will take care of this in the next revision.

> 
> 
> 
> > +#define MG_TX1_LINK_PARAMS(port, ln) \
> > +	MG_PHY_PORT_LN(port, ln, MG_TX_LINK_PARAMS_TX1LN0_PORT1, \
> > +				 MG_TX_LINK_PARAMS_TX1LN0_PORT2, \
> > +				 MG_TX_LINK_PARAMS_TX1LN1_PORT1)
> > +
> > +#define MG_TX_LINK_PARAMS_TX2LN0_PORT1		0x1680AC
> > +#define MG_TX_LINK_PARAMS_TX2LN1_PORT1		0x1684AC
> > +#define MG_TX_LINK_PARAMS_TX2LN0_PORT2		0x1690AC
> > +#define MG_TX_LINK_PARAMS_TX2LN1_PORT2		0x1694AC
> > +#define MG_TX_LINK_PARAMS_TX2LN0_PORT3		0x16A0AC
> > +#define MG_TX_LINK_PARAMS_TX2LN1_PORT3		0x16A4AC
> > +#define MG_TX_LINK_PARAMS_TX2LN0_PORT4		0x16B0AC
> > +#define MG_TX_LINK_PARAMS_TX2LN1_PORT4		0x16B4AC
> > +#define MG_TX2_LINK_PARAMS(port, ln) \
> > +	MG_PHY_PORT_LN(port, ln, MG_TX_LINK_PARAMS_TX2LN0_PORT1, \
> > +				 MG_TX_LINK_PARAMS_TX2LN0_PORT2, \
> > +				 MG_TX_LINK_PARAMS_TX2LN1_PORT1)
> >  #define CRI_USE_FS32			(1 << 5)
> >  
> > -#define _ICL_MG_TX_PISO_READLOAD_TX1LN0_PORT1		0x16814
> > C
> > -#define _ICL_MG_TX_PISO_READLOAD_TX1LN1_PORT1		0x16854
> > C
> > -#define _ICL_MG_TX_PISO_READLOAD_TX1LN0_PORT2		0x16914
> > C
> > -#define _ICL_MG_TX_PISO_READLOAD_TX1LN1_PORT2		0x16954
> > C
> > -#define _ICL_MG_TX_PISO_READLOAD_TX1LN0_PORT3		0x16A14
> > C
> > -#define _ICL_MG_TX_PISO_READLOAD_TX1LN1_PORT3		0x16A54
> > C
> > -#define _ICL_MG_TX_PISO_READLOAD_TX1LN0_PORT4		0x16B14
> > C
> > -#define _ICL_MG_TX_PISO_READLOAD_TX1LN1_PORT4		0x16B54
> > C
> > -#define ICL_PORT_MG_TX1_PISO_READLOAD(port, ln) \
> > -	_ICL_MG_PHY_PORT_LN(port, ln,
> > _ICL_MG_TX_PISO_READLOAD_TX1LN0_PORT1, \
> > -				      _ICL_MG_TX_PISO_READLOAD_TX1LN
> > 0_PORT2, \
> > -				      _ICL_MG_TX_PISO_READLOAD_TX1LN
> > 1_PORT1)
> > -
> > -#define _ICL_MG_TX_PISO_READLOAD_TX2LN0_PORT1		0x1680C
> > C
> > -#define _ICL_MG_TX_PISO_READLOAD_TX2LN1_PORT1		0x1684C
> > C
> > -#define _ICL_MG_TX_PISO_READLOAD_TX2LN0_PORT2		0x1690C
> > C
> > -#define _ICL_MG_TX_PISO_READLOAD_TX2LN1_PORT2		0x1694C
> > C
> > -#define _ICL_MG_TX_PISO_READLOAD_TX2LN0_PORT3		0x16A0C
> > C
> > -#define _ICL_MG_TX_PISO_READLOAD_TX2LN1_PORT3		0x16A4C
> > C
> > -#define _ICL_MG_TX_PISO_READLOAD_TX2LN0_PORT4		0x16B0C
> > C
> > -#define _ICL_MG_TX_PISO_READLOAD_TX2LN1_PORT4		0x16B4C
> > C
> > -#define ICL_PORT_MG_TX2_PISO_READLOAD(port, ln) \
> > -	_ICL_MG_PHY_PORT_LN(port, ln,
> > _ICL_MG_TX_PISO_READLOAD_TX2LN0_PORT1, \
> > -				      _ICL_MG_TX_PISO_READLOAD_TX2LN
> > 0_PORT2, \
> > -				      _ICL_MG_TX_PISO_READLOAD_TX2LN
> > 1_PORT1)
> > +#define MG_TX_PISO_READLOAD_TX1LN0_PORT1		0x16814C
> > +#define MG_TX_PISO_READLOAD_TX1LN1_PORT1		0x16854C
> > +#define MG_TX_PISO_READLOAD_TX1LN0_PORT2		0x16914C
> > +#define MG_TX_PISO_READLOAD_TX1LN1_PORT2		0x16954C
> > +#define MG_TX_PISO_READLOAD_TX1LN0_PORT3		0x16A14C
> > +#define MG_TX_PISO_READLOAD_TX1LN1_PORT3		0x16A54C
> > +#define MG_TX_PISO_READLOAD_TX1LN0_PORT4		0x16B14C
> > +#define MG_TX_PISO_READLOAD_TX1LN1_PORT4		0x16B54C
> > +#define MG_TX1_PISO_READLOAD(port, ln) \
> > +	MG_PHY_PORT_LN(port, ln, MG_TX_PISO_READLOAD_TX1LN0_PORT1, \
> > +				 MG_TX_PISO_READLOAD_TX1LN0_PORT2, \
> > +				 MG_TX_PISO_READLOAD_TX1LN1_PORT1)
> > +
> > +#define MG_TX_PISO_READLOAD_TX2LN0_PORT1		0x1680CC
> > +#define MG_TX_PISO_READLOAD_TX2LN1_PORT1		0x1684CC
> > +#define MG_TX_PISO_READLOAD_TX2LN0_PORT2		0x1690CC
> > +#define MG_TX_PISO_READLOAD_TX2LN1_PORT2		0x1694CC
> > +#define MG_TX_PISO_READLOAD_TX2LN0_PORT3		0x16A0CC
> > +#define MG_TX_PISO_READLOAD_TX2LN1_PORT3		0x16A4CC
> > +#define MG_TX_PISO_READLOAD_TX2LN0_PORT4		0x16B0CC
> > +#define MG_TX_PISO_READLOAD_TX2LN1_PORT4		0x16B4CC
> > +#define MG_TX2_PISO_READLOAD(port, ln) \
> > +	MG_PHY_PORT_LN(port, ln, MG_TX_PISO_READLOAD_TX2LN0_PORT1, \
> > +				 MG_TX_PISO_READLOAD_TX2LN0_PORT2, \
> > +				 MG_TX_PISO_READLOAD_TX2LN1_PORT1)
> >  #define CRI_CALCINIT					(1 << 1)
> >  
> > -#define _ICL_MG_TX_SWINGCTRL_TX1LN0_PORT1		0x168148
> > -#define _ICL_MG_TX_SWINGCTRL_TX1LN1_PORT1		0x168548
> > -#define _ICL_MG_TX_SWINGCTRL_TX1LN0_PORT2		0x169148
> > -#define _ICL_MG_TX_SWINGCTRL_TX1LN1_PORT2		0x169548
> > -#define _ICL_MG_TX_SWINGCTRL_TX1LN0_PORT3		0x16A148
> > -#define _ICL_MG_TX_SWINGCTRL_TX1LN1_PORT3		0x16A548
> > -#define _ICL_MG_TX_SWINGCTRL_TX1LN0_PORT4		0x16B148
> > -#define _ICL_MG_TX_SWINGCTRL_TX1LN1_PORT4		0x16B548
> > -#define ICL_PORT_MG_TX1_SWINGCTRL(port, ln) \
> > -	_ICL_MG_PHY_PORT_LN(port, ln,
> > _ICL_MG_TX_SWINGCTRL_TX1LN0_PORT1, \
> > -				      _ICL_MG_TX_SWINGCTRL_TX1LN0_PO
> > RT2, \
> > -				      _ICL_MG_TX_SWINGCTRL_TX1LN1_PO
> > RT1)
> > -
> > -#define _ICL_MG_TX_SWINGCTRL_TX2LN0_PORT1		0x1680C8
> > -#define _ICL_MG_TX_SWINGCTRL_TX2LN1_PORT1		0x1684C8
> > -#define _ICL_MG_TX_SWINGCTRL_TX2LN0_PORT2		0x1690C8
> > -#define _ICL_MG_TX_SWINGCTRL_TX2LN1_PORT2		0x1694C8
> > -#define _ICL_MG_TX_SWINGCTRL_TX2LN0_PORT3		0x16A0C8
> > -#define _ICL_MG_TX_SWINGCTRL_TX2LN1_PORT3		0x16A4C8
> > -#define _ICL_MG_TX_SWINGCTRL_TX2LN0_PORT4		0x16B0C8
> > -#define _ICL_MG_TX_SWINGCTRL_TX2LN1_PORT4		0x16B4C8
> > -#define ICL_PORT_MG_TX2_SWINGCTRL(port, ln) \
> > -	_ICL_MG_PHY_PORT_LN(port, ln,
> > _ICL_MG_TX_SWINGCTRL_TX2LN0_PORT1, \
> > -				      _ICL_MG_TX_SWINGCTRL_TX2LN0_PO
> > RT2, \
> > -				      _ICL_MG_TX_SWINGCTRL_TX2LN1_PO
> > RT1)
> > +#define MG_TX_SWINGCTRL_TX1LN0_PORT1		0x168148
> > +#define MG_TX_SWINGCTRL_TX1LN1_PORT1		0x168548
> > +#define MG_TX_SWINGCTRL_TX1LN0_PORT2		0x169148
> > +#define MG_TX_SWINGCTRL_TX1LN1_PORT2		0x169548
> > +#define MG_TX_SWINGCTRL_TX1LN0_PORT3		0x16A148
> > +#define MG_TX_SWINGCTRL_TX1LN1_PORT3		0x16A548
> > +#define MG_TX_SWINGCTRL_TX1LN0_PORT4		0x16B148
> > +#define MG_TX_SWINGCTRL_TX1LN1_PORT4		0x16B548
> > +#define MG_TX1_SWINGCTRL(port, ln) \
> > +	MG_PHY_PORT_LN(port, ln, MG_TX_SWINGCTRL_TX1LN0_PORT1, \
> > +				 MG_TX_SWINGCTRL_TX1LN0_PORT2, \
> > +				 MG_TX_SWINGCTRL_TX1LN1_PORT1)
> > +
> > +#define MG_TX_SWINGCTRL_TX2LN0_PORT1		0x1680C8
> > +#define MG_TX_SWINGCTRL_TX2LN1_PORT1		0x1684C8
> > +#define MG_TX_SWINGCTRL_TX2LN0_PORT2		0x1690C8
> > +#define MG_TX_SWINGCTRL_TX2LN1_PORT2		0x1694C8
> > +#define MG_TX_SWINGCTRL_TX2LN0_PORT3		0x16A0C8
> > +#define MG_TX_SWINGCTRL_TX2LN1_PORT3		0x16A4C8
> > +#define MG_TX_SWINGCTRL_TX2LN0_PORT4		0x16B0C8
> > +#define MG_TX_SWINGCTRL_TX2LN1_PORT4		0x16B4C8
> > +#define MG_TX2_SWINGCTRL(port, ln) \
> > +	MG_PHY_PORT_LN(port, ln, MG_TX_SWINGCTRL_TX2LN0_PORT1, \
> > +				 MG_TX_SWINGCTRL_TX2LN0_PORT2, \
> > +				 MG_TX_SWINGCTRL_TX2LN1_PORT1)
> >  #define CRI_TXDEEMPH_OVERRIDE_17_12(x)			((x)
> > << 0)
> >  #define CRI_TXDEEMPH_OVERRIDE_17_12_MASK		(0x3F << 0)
> >  
> > -#define _ICL_MG_TX_DRVCTRL_TX1LN0_PORT1			0x168
> > 144
> > -#define _ICL_MG_TX_DRVCTRL_TX1LN1_PORT1			0x168
> > 544
> > -#define _ICL_MG_TX_DRVCTRL_TX1LN0_PORT2			0x169
> > 144
> > -#define _ICL_MG_TX_DRVCTRL_TX1LN1_PORT2			0x169
> > 544
> > -#define _ICL_MG_TX_DRVCTRL_TX1LN0_PORT3			0x16A
> > 144
> > -#define _ICL_MG_TX_DRVCTRL_TX1LN1_PORT3			0x16A
> > 544
> > -#define _ICL_MG_TX_DRVCTRL_TX1LN0_PORT4			0x16B
> > 144
> > -#define _ICL_MG_TX_DRVCTRL_TX1LN1_PORT4			0x16B
> > 544
> > -#define ICL_PORT_MG_TX1_DRVCTRL(port, ln) \
> > -	_ICL_MG_PHY_PORT_LN(port, ln,
> > _ICL_MG_TX_DRVCTRL_TX1LN0_PORT1, \
> > -				      _ICL_MG_TX_DRVCTRL_TX1LN0_PORT
> > 2, \
> > -				      _ICL_MG_TX_DRVCTRL_TX1LN1_PORT
> > 1)
> > -
> > -#define _ICL_MG_TX_DRVCTRL_TX2LN0_PORT1			0x168
> > 0C4
> > -#define _ICL_MG_TX_DRVCTRL_TX2LN1_PORT1			0x168
> > 4C4
> > -#define _ICL_MG_TX_DRVCTRL_TX2LN0_PORT2			0x169
> > 0C4
> > -#define _ICL_MG_TX_DRVCTRL_TX2LN1_PORT2			0x169
> > 4C4
> > -#define _ICL_MG_TX_DRVCTRL_TX2LN0_PORT3			0x16A
> > 0C4
> > -#define _ICL_MG_TX_DRVCTRL_TX2LN1_PORT3			0x16A
> > 4C4
> > -#define _ICL_MG_TX_DRVCTRL_TX2LN0_PORT4			0x16B
> > 0C4
> > -#define _ICL_MG_TX_DRVCTRL_TX2LN1_PORT4			0x16B
> > 4C4
> > -#define ICL_PORT_MG_TX2_DRVCTRL(port, ln) \
> > -	_ICL_MG_PHY_PORT_LN(port, ln,
> > _ICL_MG_TX_DRVCTRL_TX2LN0_PORT1, \
> > -				      _ICL_MG_TX_DRVCTRL_TX2LN0_PORT
> > 2, \
> > -				      _ICL_MG_TX_DRVCTRL_TX2LN1_PORT
> > 1)
> > +#define MG_TX_DRVCTRL_TX1LN0_TXPORT1			0x168144
> > +#define MG_TX_DRVCTRL_TX1LN1_TXPORT1			0x168544
> > +#define MG_TX_DRVCTRL_TX1LN0_TXPORT2			0x169144
> > +#define MG_TX_DRVCTRL_TX1LN1_TXPORT2			0x169544
> > +#define MG_TX_DRVCTRL_TX1LN0_TXPORT3			0x16A144
> > +#define MG_TX_DRVCTRL_TX1LN1_TXPORT3			0x16A544
> > +#define MG_TX_DRVCTRL_TX1LN0_TXPORT4			0x16B144
> > +#define MG_TX_DRVCTRL_TX1LN1_TXPORT4			0x16B544
> > +#define MG_TX1_DRVCTRL(port, ln) \
> > +	MG_PHY_PORT_LN(port, ln, MG_TX_DRVCTRL_TX1LN0_TXPORT1, \
> > +				 MG_TX_DRVCTRL_TX1LN0_TXPORT2, \
> > +				 MG_TX_DRVCTRL_TX1LN1_TXPORT1)
> > +
> > +#define MG_TX_DRVCTRL_TX2LN0_PORT1			0x1680C4
> > +#define MG_TX_DRVCTRL_TX2LN1_PORT1			0x1684C4
> > +#define MG_TX_DRVCTRL_TX2LN0_PORT2			0x1690C4
> > +#define MG_TX_DRVCTRL_TX2LN1_PORT2			0x1694C4
> > +#define MG_TX_DRVCTRL_TX2LN0_PORT3			0x16A0C4
> > +#define MG_TX_DRVCTRL_TX2LN1_PORT3			0x16A4C4
> > +#define MG_TX_DRVCTRL_TX2LN0_PORT4			0x16B0C4
> > +#define MG_TX_DRVCTRL_TX2LN1_PORT4			0x16B4C4
> > +#define MG_TX2_DRVCTRL(port, ln) \
> > +	MG_PHY_PORT_LN(port, ln, MG_TX_DRVCTRL_TX2LN0_PORT1, \
> > +				 MG_TX_DRVCTRL_TX2LN0_PORT2, \
> > +				 MG_TX_DRVCTRL_TX2LN1_PORT1)
> >  #define CRI_TXDEEMPH_OVERRIDE_11_6(x)			((x) <<
> > 24)
> >  #define CRI_TXDEEMPH_OVERRIDE_11_6_MASK			(0x3F
> > << 24)
> >  #define CRI_TXDEEMPH_OVERRIDE_EN			(1 << 22)
> >  #define CRI_TXDEEMPH_OVERRIDE_5_0(x)			((x) <<
> > 16)
> >  #define CRI_TXDEEMPH_OVERRIDE_5_0_MASK			(0x3F
> > << 16)
> > +#define CRI_LOADGEN_SEL(x)				((x) <<
> > 12)
> > +#define CRI_LOADGEN_SEL_MASK				(0x3 <<
> > 12)
> 
> Since you're drive-by fixing things you may as well add the two
> additional spaces required for bit macros (between #define and the
> macro name), and then also add the spaces in the new registers defined
> later. This applies to the registers above too.
>

Yes I agree, will add the spaces for the MASK #defines.
 
> 
> > +
> > +#define MG_CLKHUB_LN0_PORT1			0x16839C
> > +#define MG_CLKHUB_LN1_PORT1			0x16879C
> > +#define MG_CLKHUB_LN0_PORT2			0x16939C
> > +#define MG_CLKHUB_LN1_PORT2			0x16979C
> > +#define MG_CLKHUB_LN0_PORT3			0x16A39C
> > +#define MG_CLKHUB_LN1_PORT3			0x16A79C
> > +#define MG_CLKHUB_LN0_PORT4			0x16B39C
> > +#define MG_CLKHUB_LN1_PORT4			0x16B79C
> > +#define MG_CLKHUB(port, ln) \
> > +	MG_PHY_PORT_LN(port, ln, MG_CLKHUB_LN0_PORT1, \
> > +				 MG_CLKHUB_LN0_PORT2, \
> > +				 MG_CLKHUB_LN1_PORT1)
> > +#define CFG_LOW_RATE_LKREN_EN				(1 <<
> > 11)
> > +
> > +#define MG_TX_DCC_TX1LN0_PORT1			0x168110
> > +#define MG_TX_DCC_TX1LN1_PORT1			0x168510
> > +#define MG_TX_DCC_TX1LNO_PORT2			0x169110
> 
> Here and in many cases below: s/O/0/

Thanks for pointing this out, will fix this.

> 
> 
> > +#define MG_TX_DCC_TX1LN1_PORT2			0x169510
> > +#define MG_TX_DCC_TX1LNO_PORT3			0x16A110
> > +#define MG_TX_DCC_TX1LN1_PORT3			0x16A510
> > +#define MG_TX_DCC_TX1LNO_PORT4			0x16B110
> > +#define MG_TX_DCC_TX1LN1_PORT4			0x16B510
> > +#define MG_TX1_DCC(port, ln) \
> > +	MG_PHY_PORT_LN(port, ln, MG_TX_DCC_TX1LN0_PORT1, \
> > +				 MG_TX_DCC_TX1LNO_PORT2, \
> > +				 MG_TX_DCC_TX1LN1_PORT1)
> > +#define MG_TX_DCC_TX2LN0_PORT1			0x168090
> > +#define MG_TX_DCC_TX2LN1_PORT1			0x168490
> > +#define MG_TX_DCC_TX2LNO_PORT2			0x169090
> > +#define MG_TX_DCC_TX2LN1_PORT2			0x169490
> > +#define MG_TX_DCC_TX2LNO_PORT3			0x16A090
> > +#define MG_TX_DCC_TX2LN1_PORT3			0x16A490
> > +#define MG_TX_DCC_TX2LNO_PORT4			0x16B090
> > +#define MG_TX_DCC_TX2LN1_PORT4			0x16B490
> > +#define MG_TX2_DCC(port, ln) \
> > +	MG_PHY_PORT_LN(port, ln, MG_TX_DCC_TX2LN0_PORT1, \
> > +				 MG_TX_DCC_TX2LNO_PORT2, \
> > +				 MG_TX_DCC_TX2LN1_PORT1)
> > +#define CFG_AMI_CK_DIV_OVERRIDE_EN		(1 << 24)
> 
> Definitions for bit 24 should go below definitions for bit 25. See the
> big comment at the top of the file.
>

Cool, will shuffle the order or bit macros.

Manasi
 
> Otherwise, looks good.
> 
> 
> > +#define CFG_AMI_CK_DIV_OVERRIDE_VAL(x)		((x) << 25)
> > +#define CFG_AMI_CK_DIV_OVERRIDE_VAL_MASK	(0x3 << 25)
> >  
> >  /* The spec defines this only for BXT PHY0, but lets assume that
> > this
> >   * would exist for PHY1 too if it had a second channel.


More information about the Intel-gfx mailing list