Re: [PATCH 4/7] drm/i915: Use the default 600ns LDO programming sequence delay

From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Deepak S <deepak.s@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 4/7] drm/i915: Use the default 600ns LDO programming sequence delay
Date: Fri, 8 May 2015 16:22:48 +0300	[thread overview]
Message-ID: <20150508132248.GF18908@intel.com> (raw)
In-Reply-To: <554CB3A3.1000304@linux.intel.com>

On Fri, May 08, 2015 at 06:31:23PM +0530, Deepak S wrote:
> 
> 
> On Friday 10 April 2015 08:51 PM, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Not sure which LDO programming sequence delay should be used for the CHV
> > PHY, but the spec says that 600ns is "Used by default for initial
> > bringup", and the BIOS seems to use that, so let's do the same.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >   drivers/gpu/drm/i915/i915_reg.h         | 4 ++++
> >   drivers/gpu/drm/i915/intel_runtime_pm.c | 2 ++
> >   2 files changed, 6 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 98588d5..977bad6 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -1887,6 +1887,10 @@ enum skl_disp_power_wells {
> >   #define DPIO_PHY_STATUS			(VLV_DISPLAY_BASE + 0x6240)
> >   #define   DPLL_PORTD_READY_MASK		(0xf)
> >   #define DISPLAY_PHY_CONTROL (VLV_DISPLAY_BASE + 0x60100)
> > +#define   PHY_LDO_DELAY_0NS			0x0
> > +#define   PHY_LDO_DELAY_200NS			0x1
> 
> PHY_LDO_DELAY_0NS & PHY_LDO_DELAY_200NS not used right?
> Should we keep the definitions?

I generally like to keep a bit of extra for VLV/CHV due to the bad doc
situation.

> 
> > +#define   PHY_LDO_DELAY_600NS			0x2
> > +#define   PHY_LDO_SEQ_DELAY(delay, phy)		((delay) << (2*(phy)+23))
> >   #define   PHY_CH_SU_PSR				0x1
> >   #define   PHY_CH_DEEP_PSR			0x7
> >   #define   PHY_CH_POWER_MODE(mode, phy, ch)	((mode) << (6*(phy)+3*(ch)+2))
> > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > index 1f800f8..5cd8a51 100644
> > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > @@ -1406,6 +1406,8 @@ static void chv_phy_control_init(struct drm_i915_private *dev_priv)
> >   	 * value.
> >   	 */
> >   	dev_priv->chv_phy_control =
> > +		PHY_LDO_SEQ_DELAY(PHY_LDO_DELAY_600NS, DPIO_PHY0) |
> > +		PHY_LDO_SEQ_DELAY(PHY_LDO_DELAY_600NS, DPIO_PHY1) |
> >   		PHY_CH_POWER_MODE(PHY_CH_SU_PSR, DPIO_PHY0, DPIO_CH0) |
> >   		PHY_CH_POWER_MODE(PHY_CH_SU_PSR, DPIO_PHY0, DPIO_CH1) |
> >   		PHY_CH_POWER_MODE(PHY_CH_SU_PSR, DPIO_PHY1, DPIO_CH0);
> 
> I think we need to squash this patch to previous one?
> [Intel-gfx] [PATCH 1/7] drm/i915: Implement chv display PHY lane stagger setup
> http://www.spinics.net/lists/intel-gfx/msg64481.html

Well, IIRC I never saw any real issues with the 0ns delay either, with
or without the lane stagger setup. So not much point in squashing IMO.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx