On Thu, Aug 21, 2014 at 05:09:38PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
>
> 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@xxxxxxxxx
> Testcase: igt/pm_rpm/drm-resources-equal
> Testcase: igt/pm_rpm/i2c
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
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@xxxxxxxxx>
--
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
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
