Re: [PATCH 1/2] drm/i915: Send a DPIO cmnreset during driver load or system resume.

["Lee, Chon Ming" <chon.ming.lee@intel.com>]

On 09/24 18:18, Ville Syrjälä wrote:
> On Tue, Sep 24, 2013 at 03:14:27PM +0800, Chon Ming Lee wrote:
> > Without the DPIO cmnreset, the PLL fail to lock.  This should have
> > done by BIOS.
> > 
> > v2: Move this to intel_uncore_sanitize to allow it to get call during
> > resume path. (Daniel)
> > v3: Remove redundant write 0 to DPIO_CTL, and use DPIO_RESET instead of
> > just 0x1 (Ville)
> >     Without BIOS, DPIO/render well/media well may still power gated.
> > Turn it off.
> > 
> > Signed-off-by: Chon Ming Lee <chon.ming.lee@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h     |    9 +++++++++
> >  drivers/gpu/drm/i915/intel_uncore.c |   23 +++++++++++++++++++++++
> >  2 files changed, 32 insertions(+), 0 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index c4f9bef..c02f893 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -361,6 +361,15 @@
> >  #define PUNIT_OPCODE_REG_READ			6
> >  #define PUNIT_OPCODE_REG_WRITE			7
> >  
> > +#define PUNIT_REG_PWRGT_CTRL			0x60
> > +#define PUNIT_REG_PWRGT_STATUS			0x61
> > +#define	  PUNIT_CLK_GATE			1
> > +#define	  PUNIT_PWR_RESET			2
> > +#define	  PUNIT_PWR_GATE			3
> > +#define	  RENDER_PWRGT				(PUNIT_PWR_GATE << 0)
> > +#define	  MEDIA_PWRGT				(PUNIT_PWR_GATE << 2)
> > +#define	  DPIO_PWRGT				(PUNIT_PWR_GATE << 6)
> 
> Subsys 6 seems to be one of four TX lanes, and there's also the common
> lane subsys, and the disp2d is one as well. RX supposedly is not relevant
> for display PHY, not sure why it has subsys allocated too.
> 
> Anyways my point would be that shouldn't we check all subsys ie render + media +
> disp2d + common lane + all tx lanes?
> 
By default, the common lane + all tx lanes are not power gated during cold boot
or system resume.  Unless S0ix entry actually power gate it by driver, then,
this will need to add to turn off it.  

> And should we maybe power gate the RX lanes always as those shouldn't be needed
> for display?

Yes, you are correct.  I believe there should be another patch to do it, to
enable power gate the VLV correctly for SOix entry or exit.  
> 
> > +
> >  #define PUNIT_REG_GPU_LFM			0xd3
> >  #define PUNIT_REG_GPU_FREQ_REQ			0xd4
> >  #define PUNIT_REG_GPU_FREQ_STS			0xd8
> > diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> > index 8649f1c..6923b4d 100644
> > --- a/drivers/gpu/drm/i915/intel_uncore.c
> > +++ b/drivers/gpu/drm/i915/intel_uncore.c
> > @@ -276,10 +276,33 @@ static void intel_uncore_forcewake_reset(struct drm_device *dev)
> >  
> >  void intel_uncore_sanitize(struct drm_device *dev)
> >  {
> > +	struct drm_i915_private *dev_priv = dev->dev_private;
> > +	u32 reg_val;
> > +
> >  	intel_uncore_forcewake_reset(dev);
> >  
> >  	/* BIOS often leaves RC6 enabled, but disable it for hw init */
> >  	intel_disable_gt_powersave(dev);
> > +
> > +	/* Trigger DPIO CMN RESET and turn off power gate, require
> > +	 * especially in BIOS less system
> > +	 */
> > +	if (IS_VALLEYVIEW(dev)) {
> > +
> > +		mutex_lock(&dev_priv->rps.hw_lock);
> > +		reg_val = vlv_punit_read(dev_priv, PUNIT_REG_PWRGT_STATUS);
> > +
> > +		if (reg_val & (RENDER_PWRGT | MEDIA_PWRGT | DPIO_PWRGT))
> > +			vlv_punit_write(dev_priv, PUNIT_REG_PWRGT_CTRL, 0x0);
> > +
> > +		mutex_unlock(&dev_priv->rps.hw_lock);
> > +
> > +		reg_val = I915_READ(DPIO_CTL);
> > +		if (!(reg_val & DPIO_RESET)) {
> > +			I915_WRITE(DPIO_CTL, DPIO_RESET);
> > +			POSTING_READ(DPIO_CTL);
> > +		}
> > +	}
> >  }
> >  
> >  /*
> > -- 
> > 1.7.7.6
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel OTC