Re: Panic after S3 resume and modeset with MST

Takashi Iwai
2017-03-29T13:10:11Z

On Mon, 27 Mar 2017 18:02:13 +0200,
Takashi Iwai wrote:
>
> Hi,
>
> the upstream fix a16b7658f4e0d4aec9bc3e75a5f0cc3f7a3a0422
> drm/i915: Call intel_dp_mst_resume() before resuming displays
>
> seems to trigger a kernel panic when some modeset change happens after
> S3 resume. The details are found in openSUSE bugzilla,
> https://bugzilla.suse.com/show_bug.cgi?id=1029634
>
> In short, the following procedure causes a kernel panic (supposedly)
> almost 100% on Dell Latitude with Skylake with MST DP on dock:
>
> - Boot with a docking station, DP-1 connected.
> - Login on X
> - xrandr --output eDP-1 --primary --auto --output DP-1-1 --auto --left-of eDP-1
> ==> This changes the mode.
> - Suspend ("systemctl suspend" in my case), and close the lid.
> - Remove from the dock (keep the lid closed).
> - Open the lid, which resumes automatically. It works.
> - Suspend again.
> - Connect to the dock again (keep the lid closed).
> - Open the lid, which resumes automatically. It's still OK.
> - xrandr --output eDP-1 --primary --auto --output DP-1-1 --auto --left-of eDP-1
> ==> Now the kernel feezes.
>
> Reverting the commit mentioned above fixes the problem.
>
> The problem is present in all versions I tested. The reported kernel
> in the Bugzilla is 4.4.x-based one, but the issue is seen in 4.11-rc3,
> too. Note that the S3 resume itself works in 4.11-rc3; the kernel
> panic happens when invoking xrandr manually after that.
>
> Unfortunately, I couldn't get a kernel panic message, so far. kdump
> didn't work well in this case by some reason. There are some
> screenshots taken by the original reporter (could switch VT
> beforehand), but I don't know whether it helps.
>
> If you have any hints for further debugging, it'd be highly
> appreciated.

It seems that the patch below works around the problem.
Can anyone enlighten what's going on there?

thanks,

Takashi

-- 8< --
From: Takashi Iwai <tiwai@suse.de>
Subject: [PATCH] drm/i915: Fix crash after S3 resume with DP MST mode change

We've got a bug report showing that Skylake Dell machines with a
docking station causes a kernel panic after S3 resume and modeset.
The details are found in the openSUSE bugzilla entry below. The
typical test procedure is:

- Laptop is Dell Latitude with eDP (1366x768)
- Boot with docking station connected to a DP (1920x1080)
- Login, change the mode via
xrandr --output eDP-1 --auto --output DP-1-1 --auto --left-of eDP-1
- Suspend, and close the lid after the suspend
(or close the lid to trigger the suspend)
- Undock while keeping the lid closed.
- Open the lid, which triggers the resume;
the machine wakes up well, and X shows up. No problem, so far.
- Suspend again, close the lid.
- Dock again while keeping the lid closed.
- Open the lid, triggering the resume; this wakes up still fine.
- At this moment, run xrandr again to re-setup DP-1
xrandr --output eDP-1 --auto --output DP-1-1 --auto --left-of eDP-1
==> This triggers a hard crash.

I could bisect it, and this leaded to the commit a16b7658f4e0
("drm/i915: Call intel_dp_mst_resume() before resuming displays").

Basically the commit just shuffles the calls of intel_display_resume()
and intel_dp_mst_resume(). So as a workaround, I tried to split
intel_dp_mst_resume() call to postpone the suspected code (the
invocation of intel_dp_check_mst_status()), then bingo, this cured the
problem.

But don't ask me *why* this fixes. It's still in a cargo-cult state.

Fixes: a16b7658f4e0 ("drm/i915: Call intel_dp_mst_resume() before resuming displays")
Bugzilla: https://bugzilla.suse.com/show_bug.cgi?id=1029634
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
drivers/gpu/drm/i915/i915_drv.c | 5 ++++-
drivers/gpu/drm/i915/intel_dp.c | 20 +++++++++++++++++++-
drivers/gpu/drm/i915/intel_drv.h | 3 ++-
3 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 1c75402a59c1..62c40090ceed 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1559,6 +1559,7 @@ static int i915_suspend_switcheroo(struct drm_device *dev, pm_message_t state)
static int i915_drm_resume(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = to_i915(dev);
+ int mst_pending;
int ret;

disable_rpm_wakeref_asserts(dev_priv);
@@ -1608,10 +1609,12 @@ static int i915_drm_resume(struct drm_device *dev)
dev_priv->display.hpd_irq_setup(dev_priv);
spin_unlock_irq(&dev_priv->irq_lock);

- intel_dp_mst_resume(dev);
+ mst_pending = intel_dp_mst_resume(dev);

intel_display_resume(dev);

+ intel_dp_mst_resume_post(dev, mst_pending);
+
drm_kms_helper_poll_enable(dev);

/*
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index d1670b8afbf5..fc5ea900e6f3 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -6027,9 +6027,10 @@ void intel_dp_mst_suspend(struct drm_device *dev)
}
}

-void intel_dp_mst_resume(struct drm_device *dev)
+int intel_dp_mst_resume(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = to_i915(dev);
+ int pending = 0;
int i;

for (i = 0; i < I915_MAX_PORTS; i++) {
@@ -6041,6 +6042,23 @@ void intel_dp_mst_resume(struct drm_device *dev)

ret = drm_dp_mst_topology_mgr_resume(&intel_dig_port->dp.mst_mgr);
if (ret)
+ pending |= 1 << i;
+ }
+
+ return pending;
+}
+
+void intel_dp_mst_resume_post(struct drm_device *dev, int pending)
+{
+ struct drm_i915_private *dev_priv = to_i915(dev);
+ int i;
+
+ for (i = 0; i < I915_MAX_PORTS; i++) {
+ struct intel_digital_port *intel_dig_port =
+ dev_priv->hotplug.irq_port[i];
+ if (!intel_dig_port || !intel_dig_port->dp.can_mst)
+ continue;
+ if (pending & (1 << i))
intel_dp_check_mst_status(&intel_dig_port->dp);
}
}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 344f238b283f..2025aafdcd26 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1447,7 +1447,8 @@ void intel_edp_panel_on(struct intel_dp *intel_dp);
void intel_edp_panel_off(struct intel_dp *intel_dp);
void intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connector);
void intel_dp_mst_suspend(struct drm_device *dev);
-void intel_dp_mst_resume(struct drm_device *dev);
+int intel_dp_mst_resume(struct drm_device *dev);
+void intel_dp_mst_resume_post(struct drm_device *dev, int pending);
int intel_dp_max_link_rate(struct intel_dp *intel_dp);
int intel_dp_rate_select(struct intel_dp *intel_dp, int rate);
void intel_dp_hot_plug(struct intel_encoder *intel_encoder);
--
2.11.1