[Intel-gfx] [PATCH 1/3] drm/i915/cnl; Add macro to get PORT_TX register

[Lucas De Marchi]

On Fri, Mar 09, 2018 at 11:55:47AM -0800, Rodrigo Vivi wrote:
> On Fri, Mar 09, 2018 at 06:28:56PM +0530, Mahesh Kumar wrote:
> > This patch creates a new macro to get PORT_TX register for any given DW.
> > This will remove the need of defining register address for each port & DW.
> 
> please squash patches 1 and 2. I had to open both simultaneously to review it
> what indicates that they should be 1 patch.
> 
> > 
> > Signed-off-by: Mahesh Kumar <mahesh1.kumar at intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h | 28 ++++++++++++++++++++++++++++
> >  1 file changed, 28 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index e6a8c0ee7df1..30ef3513dc6f 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -1964,6 +1964,34 @@ enum i915_power_well_id {
> >  						    _CNL_PORT_PCS_DW1_LN0_F)
> >  #define   COMMON_KEEPER_EN		(1 << 26)
> >  
> > +/* CNL Port TX registers */
> > +#define _CNL_PORT_TX_AE_GRP_OFFSET		0x162340
> > +#define _CNL_PORT_TX_B_GRP_OFFSET		0x1623C0
> > +#define _CNL_PORT_TX_C_GRP_OFFSET		0x162B40
> > +#define _CNL_PORT_TX_D_GRP_OFFSET		0x162BC0
> > +#define _CNL_PORT_TX_F_GRP_OFFSET		0x162A40
> > +#define _CNL_PORT_TX_AE_LN0_OFFSET		0x162440
> > +#define _CNL_PORT_TX_B_LN0_OFFSET		0x162640
> > +#define _CNL_PORT_TX_C_LN0_OFFSET		0x162C40
> > +#define _CNL_PORT_TX_D_LN0_OFFSET		0x162E40
> > +#define _CNL_PORT_TX_F_LN0_OFFSET		0x162840
> > +#define CNL_PORT_TX_DW_GRP(port, dw)	(_PICK((port), \
> > +					       _CNL_PORT_TX_AE_GRP_OFFSET, \
> > +					       _CNL_PORT_TX_B_GRP_OFFSET, \
> > +					       _CNL_PORT_TX_B_GRP_OFFSET, \
> > +					       _CNL_PORT_TX_D_GRP_OFFSET, \
> > +					       _CNL_PORT_TX_AE_GRP_OFFSET, \
> > +					       _CNL_PORT_TX_F_GRP_OFFSET) + \
> > +					       4*(dw))
> 
> the math is right. I'm glad someone could see some logic on all these
> numbers. I with we had a basic offset and math for all the port registers, but...
> 
> > +#define CNL_PORT_TX_DW_LN0(port, dw)	(_PICK((port), \
> 
> who converts the offset to MMIO reg now?

I don't think this is supposed to be used outside the header is it?
I think it should have a underscore, because otherwise it's confusing
that CNL_PORT_TX_DW_LN0 returns an offsed and CNL_PORT_TX_DW[0-5]_LN0
return an mmio reg.

And if it's used elsewhere, maybe append _OFFSET to the macro?

Lucas De Marchi

> 
> > +					       _CNL_PORT_TX_AE_LN0_OFFSET, \
> > +					       _CNL_PORT_TX_B_LN0_OFFSET, \
> > +					       _CNL_PORT_TX_B_LN0_OFFSET, \
> > +					       _CNL_PORT_TX_D_LN0_OFFSET, \
> > +					       _CNL_PORT_TX_AE_LN0_OFFSET, \
> > +					       _CNL_PORT_TX_F_LN0_OFFSET) + \
> > +					       4*(dw))
> > +
> >  #define _CNL_PORT_TX_DW2_GRP_AE		0x162348
> >  #define _CNL_PORT_TX_DW2_GRP_B		0x1623C8
> >  #define _CNL_PORT_TX_DW2_GRP_C		0x162B48
> > -- 
> > 2.14.1
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx