[PATCH 0/2] Workaround for MST displays failing to come back after resume

[PATCH 0/2] Workaround for MST displays failing to come back after resume

[Lyude]

This is a follow-up to the original patches I sent to try to fix the issue of
drm_dp_mst_topology_mgr_resume() not working due to aux transactions failing
temporarily after resuming the machine. Unfortunately I haven't been able to
figure out the actual cause of this issue; there don't seem to be any IRQs
interrupting the DP aux transactions and everything seems to be normal except
for the amount of time this MST dock takes to become available again over the DP
aux channel. I have made a few discoveries though:

The reason why calling intel_dp_mst_resume() before calling
intel_runtime_pm_enable_interrupts() worked is due to how
intel_dp_aux_wait_done() works:

	static uint32_t
	intel_dp_aux_wait_done(struct intel_dp *intel_dp, bool has_aux_irq)
	{
		struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
		struct drm_device *dev = intel_dig_port->base.base.dev;
		struct drm_i915_private *dev_priv = dev->dev_private;
		i915_reg_t ch_ctl = intel_dp->aux_ch_ctl_reg;
		uint32_t status;
		bool done;

	#define C (((status = I915_READ_NOTRACE(ch_ctl)) & DP_AUX_CH_CTL_SEND_BUSY) == 0)
		if (has_aux_irq)
			done = wait_event_timeout(dev_priv->gmbus_wait_queue, C,
						  msecs_to_jiffies_timeout(10));
		else
			done = wait_for_atomic(C, 10) == 0;
		if (!done)
			DRM_ERROR("dp aux hw did not signal timeout (has irq: %i)!\n",
				  has_aux_irq);
	#undef C

		return status;
	}

When calling this function without interrupts enabled, wait_event_timeout() ends
up timing out after 10ms, manually checking the DP AUX status register, and
discovering that the aux transaction succeeded. This makes the aux transactions
take quite a while, but still manage to work. Because of this, there's always a
10ms delay each time we do a transaction, and we end up delaying things long
enough for the aux transactions to become functional again which results in
intel_dp_mst_resume() working. With interrupts enabled, we get notified of the
timeouts within a period of 3ms five times in a row, which doesn't give enough
time for the aux transactions to start working again. If we change the timeout
to something shorter like 3ms, calling intel_dp_mst_resume() before
intel_runtime_pm_enable_interrupts() stops working.

So, my only possible thought for why this issue occurs is that the MST dock
simply needs more time to respond. It's possible this issue has actually always
been here, but we just never managed to have a skl machine resume quickly enough
to notice it. The T560, the only machine I seem to be able to reproduce this
issue on, does happen to be the fastest model out of all of the Skylake
production machines I have available here.

It should be noted that for the second patch, I've considered a different
workaround: calling intel_dp_check_mst_status() before calling
drm_dp_mst_topology_mgr_resume(). This is another viable solution since it
causes us to try to read the ESI from the dock using intel_dp_dpcd_read_wake(),
which retries aux transactions enough times to give the dock time to resume. If
everyone would rather that solution, I'd be happy to post that version of the
patch instead.

Lyude (2):
  drm/i915: Call intel_dp_mst_resume() before resuming displays
  drm/i915: Retry after 30ms if we fail to resume DP MST

 drivers/gpu/drm/i915/i915_drv.c |  4 ++--
 drivers/gpu/drm/i915/intel_dp.c | 13 +++++++++++++
 2 files changed, 15 insertions(+), 2 deletions(-)

-- 
2.5.0

[PATCH 1/2] drm/i915: Call intel_dp_mst_resume() before resuming displays

[Lyude]

Since we need MST devices ready before we try to resume displays,
calling this after intel_display_resume() can result in some issues with
various laptop docks where the monitor won't turn back on after
suspending the system.

This order was originally changed in

	commit e7d6f7d70829 ("drm/i915: resume MST after reading back hw state")

In order to fix some unclaimed register errors, however the actual cause
of those has since been fixed.

CC: stable@vger.kernel.org
Signed-off-by: Lyude <cpaul@redhat.com>
---
 drivers/gpu/drm/i915/i915_drv.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index f357058..08854ae 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -761,12 +761,12 @@ static int i915_drm_resume(struct drm_device *dev)
 		dev_priv->display.hpd_irq_setup(dev);
 	spin_unlock_irq(&dev_priv->irq_lock);
 
+	intel_dp_mst_resume(dev);
+
 	drm_modeset_lock_all(dev);
 	intel_display_resume(dev);
 	drm_modeset_unlock_all(dev);
 
-	intel_dp_mst_resume(dev);
-
 	/*
 	 * ... but also need to make sure that hotplug processing
 	 * doesn't cause havoc. Like in the driver load code we don't
-- 
2.5.0

[PATCH 2/2] drm/i915: Retry after 30ms if we fail to resume DP MST

[Lyude]

For whatever reason, I've found that some laptops aren't immediately
capable of doing aux transactions with their docks when they come out of
standby. While I'm still not entirely sure what the cause of this is,
sleeping for 30ms and then retrying drm_dp_mst_topology_mgr_resume()
should be a sufficient enough workaround until we find a real fix.

CC: stable@vger.kernel.org
Signed-off-by: Lyude <cpaul@redhat.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 1d8de43..8cc5f6f 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -6114,6 +6114,19 @@ void intel_dp_mst_resume(struct drm_device *dev)
 
 			ret = drm_dp_mst_topology_mgr_resume(&intel_dig_port->dp.mst_mgr);
 			if (ret != 0) {
+				/*
+				 * For some reason, some laptops can't bring
+				 * their MST docks back up immediately after
+				 * resume and need to wait a short period of
+				 * time before aux transactions with the dock
+				 * become functional again. Until we find a
+				 * proper fix for this, this workaround should
+				 * suffice
+				 */
+				msleep(30);
+				ret = drm_dp_mst_topology_mgr_resume(&intel_dig_port->dp.mst_mgr);
+			}
+			if (ret != 0) {
 				intel_dp_check_mst_status(&intel_dig_port->dp);
 			}
 		}
-- 
2.5.0