[Intel-gfx] [GLK MIPI DSI V1 1/9] drm/i915/glk: Add new bit fields in MIPI CTRL register

[Chauhan, Madhav]

> -----Original Message-----
> From: Nikula, Jani
> Sent: Tuesday, December 13, 2016 5:07 PM
> To: Chauhan, Madhav <madhav.chauhan at intel.com>; intel-
> gfx at lists.freedesktop.org
> Cc: Conselvan De Oliveira, Ander <ander.conselvan.de.oliveira at intel.com>;
> Saarinen, Jani <jani.saarinen at intel.com>; Konduru, Chandra
> <chandra.konduru at intel.com>; Shankar, Uma <uma.shankar at intel.com>;
> Mukherjee, Indranil <indranil.mukherjee at intel.com>; Kumar, Shobhit
> <shobhit.kumar at intel.com>; Deepak, M <m.deepak at intel.com>; Chauhan,
> Madhav <madhav.chauhan at intel.com>
> Subject: Re: [GLK MIPI DSI V1 1/9] drm/i915/glk: Add new bit fields in MIPI
> CTRL register
> 
> On Thu, 08 Dec 2016, Madhav Chauhan <madhav.chauhan at intel.com>
> wrote:
> > From: Deepak M <m.deepak at intel.com>
> >
> > Signed-off-by: Deepak M <m.deepak at intel.com>
> > Signed-off-by: Madhav Chauhan <madhav.chauhan at intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h | 15 +++++++++++++++
> >  1 file changed, 15 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h index 90685d2..6bd68bf 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -8658,6 +8658,21 @@ enum {
> >  #define _MIPIA_CTRL			(dev_priv->mipi_mmio_base
> + 0xb104)
> >  #define _MIPIC_CTRL			(dev_priv->mipi_mmio_base
> + 0xb904)
> >  #define MIPI_CTRL(port)			_MMIO_MIPI(port,
> _MIPIA_CTRL, _MIPIC_CTRL)
> 
> Ugh, the register has been completely changed between VLV/CHV and BXT.
> *All* of the new bits will need BXT_ or GLK_ prefix, depending on when they
> were introduced. Please also put them in high to low bit order around
> BXT_PIPE_SELECT stuff below, to group all BXT+ stuff separately from
> VLV/CHV.
Thanks for review Jani.
> 
> > +#define  PHY_STATUS					(1 << 31) /*
> RO */
> 
> GLK_. The name could indicate the purpose, even if that's not the way it's in
> the spec. It'll help us. GLK_PHY_STATUS_PORT_READY?
> 
> > +#define  ULPS_NOT_ACTIVE				(1 << 30) /* RO */
> 
> GLK_
> 
> > +#define  MIPIIO_RESET					(1 << 28)
> 
> GLK_MIPIIO_RESET_RELEASED?
> 
> > +#define  CLOCK_LANE_STOP_STATE				(1 << 27) /*
> RO */
> 
> GLK_CLOCK_LANES_STOP_STATE
> 
> > +#define  DATA_LANE_STOP_STATE				(1 << 26) /*
> RO */
> 
> GLK_DATA_LANES_STOP_STATE
> 
> > +#define  LP_WAKE					(1 << 22)
> 
> GLK_
> 
> > +#define  LP11_LOW_PWR_MODE				(1 << 21)
> 
> GLK_LP11_LOW_POWER_MODE
> 
> > +#define  LP00_LOW_PWR_MODE				(1 << 20)
> 
> GLK_LP00_LOW_POWER_MODE
> 
> > +#define  FIREWALL_ENABLE				(1 << 16)
> 
> GLK_
> 
> > +#define  BXT_PIXEL_OVERLAP_CNT_MASK			(0xf << 10)
> > +#define  BXT_PIXEL_OVERLAP_CNT_SHIFT			10
> > +#define  DSC_ENABLE					(1 << 3)
> 
> BXT_?
DSC_ENABLE common for GLK and BXT, shouldn't we add GEN9LP_??
> 
> > +#define  RGB_FLIP					(1 << 2)
> 
> BXT_?
This is also common for GLK and BXT, GEN9LP_ ??

Overall, any bit field/register (added in this series) specific to GLK should have GLK_ and common to GLK/BXT should have GEN9LP_ ??
> 
> > +#define  PWR_ACK					(1 << 1) /* RO */
> > +#define  MIPI_MODE					(1 << 0)
> 
> GLK_MIPIIO_ENABLE?
> 
> BR,
> Jani.
> 
> >  #define  ESCAPE_CLOCK_DIVIDER_SHIFT			5 /* A only */
> >  #define  ESCAPE_CLOCK_DIVIDER_MASK			(3 << 5)
> >  #define  ESCAPE_CLOCK_DIVIDER_1				(0 << 5)
> 
> 
> --
> Jani Nikula, Intel Open Source Technology Center