[Intel-gfx] [PATCH] drm/i915/cnl: Fix CNL macros for Voltage Swing programming

[Aditya Swarup]

On Mon, Dec 31, 2018 at 11:03:22AM +0200, Jani Nikula wrote:
> On Fri, 14 Dec 2018, Aditya Swarup <aditya.swarup at intel.com> 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 at intel.com>
> > Cc: Imre Deak <imre.deak at intel.com>
> > Signed-off-by: Aditya Swarup <aditya.swarup at intel.com>
> > ---
> >  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