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.ku...@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@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
