Date:   Fri, 25 Jan 2019 20:24:35 -0500
From:   Lyude Paul <lyude@...hat.com>
To:     intel-gfx@...ts.freedesktop.org
Cc:     Todd Previte <tprevite@...il.com>,
        Dave Airlie <airlied@...hat.com>,
        Jani Nikula <jani.nikula@...ux.intel.com>,
        Joonas Lahtinen <joonas.lahtinen@...ux.intel.com>,
        Rodrigo Vivi <rodrigo.vivi@...el.com>, stable@...r.kernel.org,
        David Airlie <airlied@...ux.ie>,
        Daniel Vetter <daniel@...ll.ch>,
        dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: [PATCH] drm/i915: Don't send MST hotplugs until after resume

Turns out we are sending a lot more hotplug events then we need, and
this is causing some pretty serious issues. Currently, we call
intel_dp_mst_resume() in i915_drm_resume() well before we have any sort
of hotplugging setup. This is a pretty big problem, because in practice
it will generally result in throwing the power domain refcounts out of
wack.

For instance: On my T480s, removing a previously connected topology
before the system finishes resuming causes
drm_kms_helper_hotplug_event() to be called before HPD is setup again,
which causes us to do a connector reprobe, which then causes
intel_dp_detect() to be called on all DP devices -including- the eDP
display. From there, intel_dp_detect() is run on the eDP display which
triggers DPCD transactions. Those DPCD transactions then cause us to
call edp_panel_vdd_on(), which then causes us to grab an additional
wakeref to the relevant power wells (PORT_DDI_A_IO on this machine).
>From there, this wakeref is never released which then causes the next
suspend/resume cycle to entirely fail due to the hardware not being
powered off correctly.

This sucks really badly, and I don't see any decent way to actually fix
this in intel_dp_detect() easily. Additionally, I don't even think it'd
be worth the time now since we're not expecting to handle any kind of
connector reprobing at the point in which we call intel_dp_mst_resume(),
but we also can't move intel_dp_mst_resume() any higher in the resume
process since MST topologies need to be resumed before
intel_display_resume() is called.

However, there's a light at the end of the tunnel! After reading through
a lot of code dozens of times, it occurred to me that we -never-
actually need to send hotplug events when calling
drm_dp_mst_topology_mgr_set_mst() since we send hotplug events in
drm_dp_destroy_connector_work(). Imagine that!

So, since we only seem to call intel_dp_mst_check_status() to disable
MST on the encoder in question and then send a hotplug, get rid of this
and instead just disable MST mode when a hub fails in
intel_dp_mst_resume(). From there, drm_dp_destroy_connector_work() will
eventually send the hotplug event.

Signed-off-by: Lyude Paul <lyude@...hat.com>
Fixes: 0e32b39ceed6 ("drm/i915: add DP 1.2 MST support (v0.7)")
Cc: Todd Previte <tprevite@...il.com>
Cc: Dave Airlie <airlied@...hat.com>
Cc: Jani Nikula <jani.nikula@...ux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@...ux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@...el.com>
Cc: intel-gfx@...ts.freedesktop.org
Cc: <stable@...r.kernel.org> # v3.17+
---
 drivers/gpu/drm/i915/intel_dp.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 681e88405ada..c2399acf177b 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -7096,7 +7096,10 @@ void intel_dp_mst_resume(struct drm_i915_private *dev_priv)
 			continue;
 
 		ret = drm_dp_mst_topology_mgr_resume(&intel_dp->mst_mgr);
-		if (ret)
-			intel_dp_check_mst_status(intel_dp);
+		if (ret) {
+			intel_dp->is_mst = false;
+			drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr,
+							false);
+		}
 	}
 }
-- 
2.20.1

Powered by blists - more mailing lists