[Intel-gfx] [PATCH] drm/i915/vlv: assert and de-assert sideband reset on resume

[Jesse Barnes]

On Fri, 11 Apr 2014 21:10:21 +0300
Ville Syrjälä <ville.syrjala at linux.intel.com> wrote:

> On Fri, Apr 11, 2014 at 10:35:40AM -0700, Jesse Barnes wrote:
> > On Fri, 11 Apr 2014 20:26:24 +0300
> > Ville Syrjälä <ville.syrjala at linux.intel.com> wrote:
> > 
> > > On Fri, Apr 11, 2014 at 10:00:16AM -0700, Jesse Barnes wrote:
> > > > This is a bit like the CMN reset de-assert we do in DPIO_CTL, except
> > > > that it resets the whole common lane section of the PHY.  This is
> > > > required on machines where the BIOS doesn't do this for us on resume to
> > > > properly re-calibrate and get the PHY ready to transmit data.
> > > > 
> > > > Without this patch, such machines won't resume correctly much of the time,
> > > > with the symptom being a 'port ready' timeout and/or a link training
> > > > failure.
> > > > 
> > > > I'm open to better suggestions on how to do the power well toggle, with
> > > > the existing code it looks like I'd have to walk through a bunch of
> > > > power domains looking for a match, then call a generic function which
> > > > will warn.  I'd prefer to just expose the specific domains directly for
> > > > low level platform code like this.
> > > > 
> > > > Signed-off-by: Jesse Barnes <jbarnes at virtuousgeek.org>
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_pm.c     |    4 ++--
> > > >  drivers/gpu/drm/i915/intel_uncore.c |   19 +++++++++++++++++++
> > > >  2 files changed, 21 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > > > index fa00185..3afd0bc 100644
> > > > --- a/drivers/gpu/drm/i915/intel_pm.c
> > > > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > > > @@ -5454,8 +5454,8 @@ static bool i9xx_always_on_power_well_enabled(struct drm_i915_private *dev_priv,
> > > >  	return true;
> > > >  }
> > > >  
> > > > -static void vlv_set_power_well(struct drm_i915_private *dev_priv,
> > > > -			       struct i915_power_well *power_well, bool enable)
> > > > +void vlv_set_power_well(struct drm_i915_private *dev_priv,
> > > > +			struct i915_power_well *power_well, bool enable)
> > > >  {
> > > >  	enum punit_power_well power_well_id = power_well->data;
> > > >  	u32 mask;
> > > > diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> > > > index 2a72bab..f1abd2d 100644
> > > > --- a/drivers/gpu/drm/i915/intel_uncore.c
> > > > +++ b/drivers/gpu/drm/i915/intel_uncore.c
> > > > @@ -363,6 +363,9 @@ static void intel_uncore_forcewake_reset(struct drm_device *dev, bool restore)
> > > >  	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
> > > >  }
> > > >  
> > > > +void vlv_set_power_well(struct drm_i915_private *dev_priv,
> > > > +			struct i915_power_well *power_well, bool enable);
> > > > +
> > > >  void intel_uncore_early_sanitize(struct drm_device *dev)
> > > >  {
> > > >  	struct drm_i915_private *dev_priv = dev->dev_private;
> > > > @@ -381,6 +384,22 @@ void intel_uncore_early_sanitize(struct drm_device *dev)
> > > >  		DRM_INFO("Found %zuMB of eLLC\n", dev_priv->ellc_size);
> > > >  	}
> > > >  
> > > > +	/*
> > > > +	 * From VLV2A0_DP_eDP_HDMI_DPIO_driver_vbios_notes_11.docx:
> > > > +	 * Need to assert and de-assert PHY SB reset by gating the common
> > > > +	 * lane power, then un-gating it.
> > > > +	 * Simply ungating isn't enough to reset the PHY enough to get
> > > > +	 * ports and lanes running.
> > > > +	 */
> > > > +	if (IS_VALLEYVIEW(dev)) {
> > > > +		struct i915_power_well cmn_well = {
> > > > +			.data = PUNIT_POWER_WELL_DPIO_CMN_BC
> > > > +		};
> > > > +
> > > > +		vlv_set_power_well(dev_priv, &cmn_well, false);
> > > > +		vlv_set_power_well(dev_priv, &cmn_well, true);
> > > > +	}
> > > 
> > > Stick this into intel_reset_dpio() instead?
> > > 
> > > And what about fastboot and whatnot? Should we check if the display is
> > > already up and running somehow before we go and kill it with this?
> > 
> > reset_dpio is too late.
> 
> How come? We shouldn't touch the PHY before it. So either reset_dpio is
> in the wrong place for some reason, or something else gets called too
> soon.

Oh actually I haven't tested with just the common reset, it may be ok
to put that into the DPIO init function.  My earlier patch was toggling
all the wells, including display, which would obviously clobber things.

-- 
Jesse Barnes, Intel Open Source Technology Center