On Mon, Dec 31, 2018 at 11:03:22AM +0200, Jani Nikula wrote: > On Fri, 14 Dec 2018, Aditya Swarup <aditya.swarup@xxxxxxxxx> wrote: > > CNL macros for register groups CNL_PORT_TX_DW2_* / CNL_PORT_TX_DW5_* are > > configured incorrectly wrt definition of _CNL_PORT_TX_DW_GRP. > > What a mess. > > Please fix this by making CNL and ICL macros behave the same way. It's > silly to have the macro args reversed between them. I think dw first, > port second. > > BR, > Jani. I believe it should be the other way round for the macros to make sense. Port first, followed by dw and then ln. For example we can see in the following macro expansion: #define _ICL_PORT_CL_DW(dw, port) (_ICL_COMBOPHY(port) + \ 4 * (dw)) port is used before dw and we should follow that sequence. I have another patch ready which makes ICL macros consistent with CNL macros if you approve the sequence of (port, dw, ln). Let me know if you agree. Regards, Aditya Swarup > > > > > > Cc: Clint Taylor <clinton.a.taylor@xxxxxxxxx> > > Cc: Imre Deak <imre.deak@xxxxxxxxx> > > Signed-off-by: Aditya Swarup <aditya.swarup@xxxxxxxxx> > > --- > > drivers/gpu/drm/i915/i915_reg.h | 8 ++++---- > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > > index 0796526dc10f..db1332cd9dcd 100644 > > --- a/drivers/gpu/drm/i915/i915_reg.h > > +++ b/drivers/gpu/drm/i915/i915_reg.h > > @@ -1836,8 +1836,8 @@ enum i915_power_well_id { > > #define _ICL_PORT_TX_DW_LN(dw, ln, port) (_ICL_COMBOPHY(port) + \ > > _ICL_PORT_TX_LN(ln) + 4 * (dw)) > > > > -#define CNL_PORT_TX_DW2_GRP(port) _MMIO(_CNL_PORT_TX_DW_GRP(2, port)) > > -#define CNL_PORT_TX_DW2_LN0(port) _MMIO(_CNL_PORT_TX_DW_LN0(2, port)) > > +#define CNL_PORT_TX_DW2_GRP(port) _MMIO(_CNL_PORT_TX_DW_GRP(port, 2)) > > +#define CNL_PORT_TX_DW2_LN0(port) _MMIO(_CNL_PORT_TX_DW_LN0(port, 2)) > > #define ICL_PORT_TX_DW2_AUX(port) _MMIO(_ICL_PORT_TX_DW_AUX(2, port)) > > #define ICL_PORT_TX_DW2_GRP(port) _MMIO(_ICL_PORT_TX_DW_GRP(2, port)) > > #define ICL_PORT_TX_DW2_LN0(port) _MMIO(_ICL_PORT_TX_DW_LN(2, 0, port)) > > @@ -1869,8 +1869,8 @@ enum i915_power_well_id { > > #define CURSOR_COEFF(x) ((x) << 0) > > #define CURSOR_COEFF_MASK (0x3F << 0) > > > > -#define CNL_PORT_TX_DW5_GRP(port) _MMIO(_CNL_PORT_TX_DW_GRP(5, port)) > > -#define CNL_PORT_TX_DW5_LN0(port) _MMIO(_CNL_PORT_TX_DW_LN0(5, port)) > > +#define CNL_PORT_TX_DW5_GRP(port) _MMIO(_CNL_PORT_TX_DW_GRP(port, 5)) > > +#define CNL_PORT_TX_DW5_LN0(port) _MMIO(_CNL_PORT_TX_DW_LN0(port, 5)) > > #define ICL_PORT_TX_DW5_AUX(port) _MMIO(_ICL_PORT_TX_DW_AUX(5, port)) > > #define ICL_PORT_TX_DW5_GRP(port) _MMIO(_ICL_PORT_TX_DW_GRP(5, port)) > > #define ICL_PORT_TX_DW5_LN0(port) _MMIO(_ICL_PORT_TX_DW_LN(5, 0, port)) > > -- > Jani Nikula, Intel Open Source Graphics Center _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx