[Intel-gfx] [PATCH 05/20] drm/i915/icl: Define PORT_CL_DW_10 register

[Chauhan, Madhav]

> -----Original Message-----
> From: Nikula, Jani
> Sent: Monday, July 2, 2018 3:43 PM
> To: Chauhan, Madhav <madhav.chauhan at intel.com>; intel-
> gfx at lists.freedesktop.org
> Cc: Zanoni, Paulo R <paulo.r.zanoni at intel.com>; Shankar, Uma
> <uma.shankar at intel.com>; Vivi, Rodrigo <rodrigo.vivi at intel.com>
> Subject: RE: [PATCH 05/20] drm/i915/icl: Define PORT_CL_DW_10 register
> 
> On Mon, 02 Jul 2018, "Chauhan, Madhav" <madhav.chauhan at intel.com>
> wrote:
> >> -----Original Message-----
> >> From: Nikula, Jani
> >> Sent: Friday, June 29, 2018 7:24 PM
> >> To: Chauhan, Madhav <madhav.chauhan at intel.com>; intel-
> >> gfx at lists.freedesktop.org
> >> Cc: Zanoni, Paulo R <paulo.r.zanoni at intel.com>; Shankar, Uma
> >> <uma.shankar at intel.com>; Vivi, Rodrigo <rodrigo.vivi at intel.com>;
> >> Chauhan, Madhav <madhav.chauhan at intel.com>
> >> Subject: Re: [PATCH 05/20] drm/i915/icl: Define PORT_CL_DW_10
> >> register
> >>
> >> On Fri, 15 Jun 2018, Madhav Chauhan <madhav.chauhan at intel.com>
> wrote:
> >> > This register used to power down individual lanes for DDI/DSI ports.
> >> > Bitfields to power up/down various combinations of lanes are also
> >> > added in this patch.
> >> >
> >> > Signed-off-by: Madhav Chauhan <madhav.chauhan at intel.com>
> >> > ---
> >> >  drivers/gpu/drm/i915/i915_reg.h | 19 +++++++++++++++++++
> >> >  1 file changed, 19 insertions(+)
> >> >
> >> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> >> > b/drivers/gpu/drm/i915/i915_reg.h index 0d268d1..1b91e73 100644
> >> > --- a/drivers/gpu/drm/i915/i915_reg.h
> >> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> >> > @@ -1666,6 +1666,25 @@ enum i915_power_well_id {
> >> >  #define ICL_PORT_CL_DW5(port)	_MMIO_PORT(port,
> >> _ICL_PORT_CL_DW5_A, \
> >> >  						 _ICL_PORT_CL_DW5_B)
> >> >
> >> > +#define _CNL_PORT_CL_DW10_A		0x162028
> >> > +#define _ICL_PORT_CL_DW10_B		0x6c028
> >> > +#define ICL_PORT_CL_DW10(port)		_MMIO_PORT(port,
> 	\
> >> > +						   _CNL_PORT_CL_DW10_A, \
> >> > +						   _ICL_PORT_CL_DW10_B)
> >> > +#define  PG_SEQ_DELAY_OVRRIDE		(3 << 25)
> >>
> >> _OVERRIDE_MASK?
> >
> > Ok. Will add it.
> >
> >>
> >> > +#define  PG_SEQ_DELAY_OVRRIDE_ENABLE	(1 << 24)
> >>
> >> OVERRIDE
> >
> > Ok.
> >
> >>
> >> > +#define  PWR_UP_ALL_LANES		0x0
> >> > +#define  PWR_DOWN_LN_3_2_1		0xe
> >> > +#define  PWR_DOWN_LN_3_2		0xc
> >> > +#define  PWR_DOWN_LN_3			0x8
> >> > +#define  PWR_DOWN_LN_2_1_0		0x7
> >> > +#define  PWR_DOWN_LN_1_0		0x3
> >> > +#define  PWR_DOWN_LN_1			0x2
> >> > +#define  PWR_DOWN_LN_3_1		0xa
> >> > +#define  PWR_DOWN_LN_3_1_0		0xb
> >>
> >> We define the values of fields shifted to position, so they can be
> >> directly OR'd in place.
> >>
> >> Effectively you could do this:
> >>
> >> #define   PWR_UP_LANES(x)		(GENMASK(3, x) << 4)
> >> #define   PWR_UP_LANES_REVERSE(x)	(GENMASK(3 - (x), 0) << 4)
> >>
> >> and use them like PWR_UP_LANES(intel_dsi->lane_count), saving tens of
> >> lines of code.
> >
> > IIUC, we can directly use these macros instead of switch case in code.
> > But using PWR_UP_LANES(2) will give 0xC but needed 0xA for DSI.
> > Or are you suggesting something else??
> 
> Aww damn. Yes, I meant you could use the macro directly without the big
> switch-case in code, but I was confused with the lane configuration for the 2
> lane case. Sorry.
> 
> I guess just please define the PWR_DOWN_LN_* macros shifted in place, i.e.
> 
> #define   PWR_DOWN_LN_3_2_1		(0xe << 4)
> 
> etc. and remove the shift in the code. This is just to be in line with the rest of
> i915_reg.h.

Make sense, will add changes in next version. Thanks!!

Regards,
Madhav

> 
> BR,
> Jani.
> 
> 
> 
> >
> > Regards,
> > Madhav
> >
> >>
> >> BR,
> >> Jani.
> >>
> >> > +#define  PWR_DOWN_LN_MASK		0xf0
> >> > +#define  PWR_DOWN_LN_SHIFT		4
> >> > +
> >> >  #define _PORT_CL1CM_DW9_A		0x162024
> >> >  #define _PORT_CL1CM_DW9_BC		0x6C024
> >> >  #define   IREF0RC_OFFSET_SHIFT		8
> >>
> >> --
> >> Jani Nikula, Intel Open Source Graphics Center
> 
> --
> Jani Nikula, Intel Open Source Graphics Center