[Intel-gfx] [GLK MIPI DSI V3 2/7] drm/i915/glk: Program new MIPI DSI PHY registers for GLK

[Chauhan, Madhav]

> -----Original Message-----
> From: Nikula, Jani
> Sent: Thursday, January 19, 2017 2:54 PM
> To: Chauhan, Madhav <madhav.chauhan at intel.com>; intel-
> gfx at lists.freedesktop.org
> Cc: Shankar, Uma <uma.shankar at intel.com>; Mukherjee, Indranil
> <indranil.mukherjee at intel.com>; Kamath, Sunil <sunil.kamath at intel.com>;
> Saarinen, Jani <jani.saarinen at intel.com>; Conselvan De Oliveira, Ander
> <ander.conselvan.de.oliveira at intel.com>; Konduru, Chandra
> <chandra.konduru at intel.com>; Kumar, Shobhit
> <shobhit.kumar at intel.com>; Syrjala, Ville <ville.syrjala at intel.com>
> Subject: RE: [GLK MIPI DSI V3 2/7] drm/i915/glk: Program new MIPI DSI PHY
> registers for GLK
> 
> On Thu, 19 Jan 2017, "Chauhan, Madhav" <madhav.chauhan at intel.com>
> wrote:
> >> -----Original Message-----
> >> From: Nikula, Jani
> >> Sent: Wednesday, January 18, 2017 9:00 PM
> >> To: Chauhan, Madhav <madhav.chauhan at intel.com>; intel-
> >> gfx at lists.freedesktop.org
> >> Cc: Shankar, Uma <uma.shankar at intel.com>; Mukherjee, Indranil
> >> <indranil.mukherjee at intel.com>; Kamath, Sunil
> >> <sunil.kamath at intel.com>; Saarinen, Jani <jani.saarinen at intel.com>;
> >> Conselvan De Oliveira, Ander <ander.conselvan.de.oliveira at intel.com>;
> >> Konduru, Chandra <chandra.konduru at intel.com>; Kumar, Shobhit
> >> <shobhit.kumar at intel.com>; Syrjala, Ville <ville.syrjala at intel.com>;
> >> Deepak M <m.deepak at intel.com>; Chauhan, Madhav
> >> <madhav.chauhan at intel.com>
> >> Subject: Re: [GLK MIPI DSI V3 2/7] drm/i915/glk: Program new MIPI DSI
> >> PHY registers for GLK
> >>
> >> On Mon, 02 Jan 2017, Madhav Chauhan <madhav.chauhan at intel.com>
> >> wrote:
> >> > From: Deepak M <m.deepak at intel.com>
> >> >
> >> > Program the clk lane and tlpx time count registers to configure DSI
> >> > PHY.
> >> >
> >> > v2: Addressed Jani's Review comments(renamed bit field macros)
> >> > v3: Program clk lane timing reg same as dphy param reg.
> >> >
> >> > 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  | 8 ++++++++
> >> > drivers/gpu/drm/i915/intel_dsi.c | 7 +++++++
> >> >  2 files changed, 15 insertions(+)
> >> >
> >> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> >> > b/drivers/gpu/drm/i915/i915_reg.h index 00970aa..f111c3f 100644
> >> > --- a/drivers/gpu/drm/i915/i915_reg.h
> >> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> >> > @@ -8554,6 +8554,14 @@ enum {
> >> >  #define  LP_BYTECLK_SHIFT				0
> >> >  #define  LP_BYTECLK_MASK				(0xffff << 0)
> >> >
> >> > +#define _MIPIA_TLPX_TIME_COUNT		(dev_priv-
> >mipi_mmio_base
> >> + 0xb0a4)
> >> > +#define _MIPIC_TLPX_TIME_COUNT		(dev_priv-
> >mipi_mmio_base
> >> + 0xb8a4)
> >> > +#define MIPI_TLPX_TIME_COUNT(port)	 _MMIO_MIPI(port,
> >> _MIPIA_TLPX_TIME_COUNT, _MIPIC_TLPX_TIME_COUNT)
> >> > +
> >> > +#define _MIPIA_CLK_LANE_TIMING		(dev_priv-
> >mipi_mmio_base
> >> + 0xb098)
> >> > +#define _MIPIC_CLK_LANE_TIMING		(dev_priv-
> >mipi_mmio_base
> >> + 0xb898)
> >> > +#define MIPI_CLK_LANE_TIMING(port)	 _MMIO_MIPI(port,
> >> _MIPIA_CLK_LANE_TIMING, _MIPIC_CLK_LANE_TIMING)
> >> > +
> >> >  /* bits 31:0 */
> >> >  #define _MIPIA_LP_GEN_DATA		(dev_priv->mipi_mmio_base
> >> + 0xb064)
> >> >  #define _MIPIC_LP_GEN_DATA		(dev_priv->mipi_mmio_base
> >> + 0xb864)
> >> > diff --git a/drivers/gpu/drm/i915/intel_dsi.c
> >> > b/drivers/gpu/drm/i915/intel_dsi.c
> >> > index 16732e7..be81283 100644
> >> > --- a/drivers/gpu/drm/i915/intel_dsi.c
> >> > +++ b/drivers/gpu/drm/i915/intel_dsi.c
> >> > @@ -1279,6 +1279,13 @@ static void intel_dsi_prepare(struct
> >> intel_encoder *intel_encoder,
> >> >  		 */
> >> >  		I915_WRITE(MIPI_LP_BYTECLK(port), intel_dsi->lp_byte_clk);
> >>
> >> IIUC you also need to write 1 to MIPI_LP_BYTECLK(port) on GLK. Or how
> >> do you read the spec on the register?
> >
> > Sorry if I missed, is it mentioned in spec that MIPI_LP_BYTECLK(port) will be
> programmed to '1'??
> > Checked spec, couldn't find this. Please specify.
> 
> The spec lists "valid values" for MIPI_LP_BYTECLK and there's only value
> 1 for GLK+. I don't know if they try to list that as just the default, but it looks
> odd.

Ahh..Got it. That should be the default value not the only correct value.
Might be some mistake to show it as valid value.

> 
> >
> >>
> >> >
> >> > +		if (IS_GEMINILAKE(dev_priv)) {
> >> > +			I915_WRITE(MIPI_TLPX_TIME_COUNT(port),
> >> > +					intel_dsi->lp_byte_clk);
> >> > +			/* Shadow of DPHY reg */
> >> > +			I915_WRITE(MIPI_CLK_LANE_TIMING(port), intel_dsi-
> >> >dphy_reg);
> >>
> >> The spec lists only specific valid values for the register. Is the spec right?
> >
> > Spec mentions DPHY parameters i.e. exit, trail, count, prep should be
> > programmed in terms of byte clock which is same as for DPHY param reg.
> > To confirm this, printed clk lane timing register value programmed by BIOS
> and its same as DPHY param reg value.
> 
> Same here. Perhaps they're trying to just list the defaults, but it shows up as
> "valid values"...
> 
> In any case, the DPHY register has changed starting in BXT, and the fields are
> now 8 bits each rather than fewer as for BYT/CHV.

Agree. 

> 
> BR,
> Jani.
> 
> 
> 
> >
> >>
> >> BR,
> >> Jani.
> >>
> >> > +		}
> >> > +
> >> >  		/* the bw essential for transmitting 16 long packets
> >> containing
> >> >  		 * 252 bytes meant for dcs write memory command is
> >> programmed in
> >> >  		 * this register in terms of byte clocks. based on dsi transfer
> >>
> >> --
> >> Jani Nikula, Intel Open Source Technology Center
> 
> --
> Jani Nikula, Intel Open Source Technology Center