Re: [PATCH 3/3] drm/i915: send PCI_D3hot adapter opregion message on BDW RPM suspend

[Daniel Vetter <daniel@ffwll.ch>]

On Thu, Aug 21, 2014 at 10:47:12PM +0100, Damien Lespiau wrote:
> On Thu, Aug 21, 2014 at 05:09:38PM -0300, Paulo Zanoni wrote:
> > From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > 
> > On BDW we're seeing a problem that after we runtime resume, the
> > outputs connected to DDI C are not detected: they don't appear in the
> > SDEISR register and GMBUS transactions don't work. They stop working
> > at the moment we call intel_opregion_notify_adapter() during runtime
> > suspend, but they don't go back to work when we call the same function
> > during runtime resume. They only work after we do a modeset and call
> > intel_opregion_notify_encoder(), but this point is already too late.
> > 
> > While debugging, I tried to pass PCI_D3hot which is the value that
> > matches the spec, and it seems to have solved the problem. I couldn't
> > find any explanation of why this solves the problem, but there's also
> > no documented explanation - besides our code and git log - of why
> > Haswell should use PCI_D1, so keep this for now in order to keep BDW
> > runtime PM working.
> > 
> > Also add a comment to point the fact that there's no spec documenting
> > all the weirdness involved here.
> > 
> > Cc: kristen.c.accardi@intel.com
> > Testcase: igt/pm_rpm/drm-resources-equal
> > Testcase: igt/pm_rpm/i2c
> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> There's still investigation to understand what the firmware is doing
> behind our backs, but the change looks likely to be correct and fixes
> the problem at hand so:
> 
> Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
> 
> -- 
> Damien
> 
> > ---
> >  drivers/gpu/drm/i915/i915_drv.c | 28 ++++++++++++++++++++++------
> >  1 file changed, 22 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > index 2dcc0d8..6f0c95f 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -1455,13 +1455,29 @@ static int intel_runtime_suspend(struct device *device)
> >  	dev_priv->pm.suspended = true;
> >  
> >  	/*
> > -	 * current versions of firmware which depend on this opregion
> > -	 * notification have repurposed the D1 definition to mean
> > -	 * "runtime suspended" vs. what you would normally expect (D3)
> > -	 * to distinguish it from notifications that might be sent
> > -	 * via the suspend path.
> > +	 * FIXME: We really should find a document that references the arguments
> > +	 * used below!
> >  	 */
> > -	intel_opregion_notify_adapter(dev, PCI_D1);
> > +	if (IS_HASWELL(dev)) {
> > +		/*
> > +		 * current versions of firmware which depend on this opregion
> > +		 * notification have repurposed the D1 definition to mean
> > +		 * "runtime suspended" vs. what you would normally expect (D3)
> > +		 * to distinguish it from notifications that might be sent via
> > +		 * the suspend path.
> > +		 */
> > +		intel_opregion_notify_adapter(dev, PCI_D1);
> > +	} else {
> > +		/*
> > +		 * On Broadwell, if we use PCI_D1 the PCH DDI ports will stop
> > +		 * being detected, and the call we do at intel_runtime_resume()
> > +		 * won't be able to restore them. Since PCI_D3hot matches the
> > +		 * actual specification and appears to be working, use it. Let's
> > +		 * assume the other non-Haswell platforms will stay the same as
> > +		 * Broadwell.
> > +		 */
> > +		intel_opregion_notify_adapter(dev, PCI_D3hot);

I guess we could start to open betting pools whether this will grow until
we have all the pci states or not ;-)

Anyway, all 3 patches merged to dinq, thanks.
-Daniel

> > +	}
> >  
> >  	DRM_DEBUG_KMS("Device suspended\n");
> >  	return 0;
> > -- 
> > 2.0.1
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch