Re: [PATCH 05/20] drm/i915/icl: Define PORT_CL_DW_10 register

[Jani Nikula <jani.nikula@intel.com>]

On Mon, 02 Jul 2018, "Chauhan, Madhav" <madhav.chauhan@intel.com> wrote:
>> -----Original Message-----
>> From: Nikula, Jani
>> Sent: Friday, June 29, 2018 7:24 PM
>> To: Chauhan, Madhav <madhav.chauhan@intel.com>; intel-
>> gfx@lists.freedesktop.org
>> Cc: Zanoni, Paulo R <paulo.r.zanoni@intel.com>; Shankar, Uma
>> <uma.shankar@intel.com>; Vivi, Rodrigo <rodrigo.vivi@intel.com>;
>> Chauhan, Madhav <madhav.chauhan@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@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@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.

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