On Thu, Aug 21, 2014 at 05:09:38PM -0300, Paulo Zanoni wrote: > From: Paulo Zanoni <[email protected]> > > 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: [email protected] > Testcase: igt/pm_rpm/drm-resources-equal > Testcase: igt/pm_rpm/i2c > Signed-off-by: Paulo Zanoni <[email protected]>
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 <[email protected]> -- 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); > + } > > DRM_DEBUG_KMS("Device suspended\n"); > return 0; > -- > 2.0.1 > > _______________________________________________ > Intel-gfx mailing list > [email protected] > http://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
