[Intel-gfx] [PATCH 01/20] drm/i915/icl: Define register for DSI PLL

[Jani Nikula]

On Fri, 29 Jun 2018, "Chauhan, Madhav" <madhav.chauhan at intel.com> wrote:
>> -----Original Message-----
>> From: Nikula, Jani
>> Sent: Friday, June 29, 2018 5:13 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 01/20] drm/i915/icl: Define register for DSI PLL
>> 
>> On Fri, 15 Jun 2018, Madhav Chauhan <madhav.chauhan at intel.com> wrote:
>> > This patch adds the new registers and corresponding bit definitions
>> > which will be used for programming/enable DSI PLL.
>> >
>> > Signed-off-by: Madhav Chauhan <madhav.chauhan at intel.com>
>> > ---
>> >  drivers/gpu/drm/i915/i915_reg.h | 12 ++++++++++++
>> >  1 file changed, 12 insertions(+)
>> >
>> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
>> > b/drivers/gpu/drm/i915/i915_reg.h index f0317bde..bf2d3e4 100644
>> > --- a/drivers/gpu/drm/i915/i915_reg.h
>> > +++ b/drivers/gpu/drm/i915/i915_reg.h
>> > @@ -9339,6 +9339,18 @@ enum skl_power_gate {
>> >  #define MIPIO_TXESC_CLK_DIV2			_MMIO(0x160008)
>> >  #define  GLK_TX_ESC_CLK_DIV2_MASK			0x3FF
>> >
>> > +#define _ICL_DSI_ESC_CLK_DIV0		0x6b090
>> > +#define _ICL_DSI_ESC_CLK_DIV1		0x6b890
>> > +#define ICL_DSI_ESC_CLK_DIV(port)	_MMIO_PORT((port),	\
>> > +
>> 	_ICL_DSI_ESC_CLK_DIV0, \
>> > +
>> 	_ICL_DSI_ESC_CLK_DIV1)
>> > +#define _ICL_DPHY_ESC_CLK_DIV0		0x162190
>> > +#define _ICL_DPHY_ESC_CLK_DIV1		0x6C190
>> > +#define ICL_DPHY_ESC_CLK_DIV(port)	_MMIO_PORT((port),	\
>> > +						_ICL_DPHY_ESC_CLK_DIV0, \
>> > +						_ICL_DPHY_ESC_CLK_DIV1)
>> > +#define ICL_ESC_CLK_DIV_MASK		0x1ff
>>           ^
>> 
>> Nitpick, 3 spaces there. With that fixed,
>
> You mean 2 spaces, rt??

The intention is to have three spaces (i.e. two extra spaces), but there
are both usages atm in the file. At least you're consistent with the
indentation, so up to you to change or not.

BR,
Jani.

>
> Thanks for starting the review of DSI patches :)
>
> Regards,
> Madhav
>
>> 
>> Reviewed-by: Jani Nikula <jani.nikula at intel.com>
>> 
>> Side note, you could define shifts and masks for both escape clock divider M
>> and byte clocks per escape clock while at it.
>> 
>> > +
>> >  /* Gen4+ Timestamp and Pipe Frame time stamp registers */
>> >  #define GEN4_TIMESTAMP		_MMIO(0x2358)
>> >  #define ILK_TIMESTAMP_HI	_MMIO(0x70070)
>> 
>> --
>> Jani Nikula, Intel Open Source Graphics Center

-- 
Jani Nikula, Intel Open Source Graphics Center