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

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

On Thu, 19 Jan 2017, "Chauhan, Madhav" <madhav.chauhan@intel.com> wrote:
>> -----Original Message-----
>> From: Nikula, Jani
>> Sent: Wednesday, January 18, 2017 9:00 PM
>> To: Chauhan, Madhav <madhav.chauhan@intel.com>; intel-
>> gfx@lists.freedesktop.org
>> Cc: Shankar, Uma <uma.shankar@intel.com>; Mukherjee, Indranil
>> <indranil.mukherjee@intel.com>; Kamath, Sunil <sunil.kamath@intel.com>;
>> Saarinen, Jani <jani.saarinen@intel.com>; Conselvan De Oliveira, Ander
>> <ander.conselvan.de.oliveira@intel.com>; Konduru, Chandra
>> <chandra.konduru@intel.com>; Kumar, Shobhit
>> <shobhit.kumar@intel.com>; Syrjala, Ville <ville.syrjala@intel.com>; Deepak
>> M <m.deepak@intel.com>; Chauhan, Madhav
>> <madhav.chauhan@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@intel.com>
>> wrote:
>> > From: Deepak M <m.deepak@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@intel.com>
>> > Signed-off-by: Madhav Chauhan <madhav.chauhan@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.

>
>> 
>> >
>> > +		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.

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