Re: drm/msm: DisplayPort hard-reset on hotplug regression in 6.8-rc1

From: Abhinav Kumar
Date: Fri Mar 08 2024 - 12:50:50 EST

Hi Johan

On 3/8/2024 4:43 AM, Johan Hovold wrote:
Hi Abhinav, Rob, Dmitry and Kuogee,

On Tue, Feb 27, 2024 at 02:33:48PM +0100, Johan Hovold wrote:

Since 6.8-rc1 I have seen (and received reports) of hard resets of the
Lenovo ThinkPad X13s after connecting and disconnecting an external
display.

I have triggered this on a simple disconnect while in a VT console, but
also when stopping Xorg after having repeatedly connected and
disconnected an external display and tried to enable it using xrandr.

In the former case, the last (custom debug) messages printed over an SSH
session were once:

[ 948.416358] usb 5-1: USB disconnect, device number 3
[ 948.443496] msm_dpu ae01000.display-controller: msm_fbdev_client_hotplug
[ 948.443723] msm-dp-display ae98000.displayport-controller: dp_power_clk_enable - type = 1, enable = 0
[ 948.443872] msm-dp-display ae98000.displayport-controller: dp_ctrl_phy_exit
[ 948.445117] msm-dp-display ae98000.displayport-controller: dp_ctrl_phy_exit - done
and then the hypervisor resets the machine.

Has there been any progress on tracking down the reset on disconnect
issue? I was expecting you to share your findings here so that we can
determine if the rest of the runtime PM series needs to be reverted or
not before 6.8 is released (possibly on Sunday).

I really did not want to spend more on this driver than I already have
this cycle, but the lack of (visible) progress has again forced me to do
so.

It's quite likely that the resets are indeed a regression caused by the
runtime PM series as the bus clocks were not disabled on disconnect
before that one was merged in 6.8-rc1.


For this last remaining reset with the stacktrace you have mentioned below, I do not think this was introduced due to PM runtime series. We have had this report earlier with the same stacktrace as well in earlier kernels which didnt have PM runtime support:

https://gitlab.freedesktop.org/drm/msm/-/issues/17

We had fixed the issue reported by the user that time with below patch which was confirmed by the user as fixed:

https://lore.kernel.org/all/1664408211-25314-1-git-send-email-quic_khsieh@xxxxxxxxxxx/

But, it seems like there is another race condition in this code resulting in this issue again.

I have posted my analysis with the patch here as a RFC y'day:

https://patchwork.freedesktop.org/patch/581758/

I missed CCing you and Bjorn on the RFC but when I post it as a patch either today/tomm, will CC you both.

Sorry, much of the discussion of this issue was happening on #freedreno last few days.

In a VT console, the device is now runtime suspended immediately on
disconnect, while in X, it currently remains active until X is killed,
which is consistent with what I reported above.

We now also have Bjorn's call trace from a different Qualcomm platform
triggering an unclocked access on disconnect, something which would
trigger a reset by the hypervisor on sc8280xp platforms like the X13s:

[ 2030.379417] SError Interrupt on CPU0, code 0x00000000be000000 -- SError
[ 2030.379425] CPU: 0 PID: 239 Comm: kworker/0:2 Not tainted 6.8.0-rc4-next-20240216-00015-gc937d3c43ffe-dirty #219
[ 2030.379430] Hardware name: Qualcomm Technologies, Inc. Robotics RB3gen2 (DT)
[ 2030.379435] Workqueue: events output_poll_execute [drm_kms_helper]
[ 2030.379495] pstate: 604000c5 (nZCv daIF +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[ 2030.379501] pc : el1_interrupt+0x28/0xc0
[ 2030.379514] lr : el1h_64_irq_handler+0x18/0x24
[ 2030.379520] sp : ffff80008129b700
[ 2030.379523] x29: ffff80008129b700 x28: ffff7a0a031aa200 x27: ffff7a0af768c3c0
[ 2030.379530] x26: 0000000000000000 x25: ffff7a0a024bb568 x24: ffff7a0a031aa200
[ 2030.379537] x23: 0000000060400005 x22: ffffd2a4a10c871c x21: ffff80008129b8a0
[ 2030.379544] x20: ffffd2a4a00002f0 x19: ffff80008129b750 x18: 0000000000000000
[ 2030.379549] x17: 0000000000000000 x16: ffffd2a4a10c21d4 x15: 0000000000000000
[ 2030.379555] x14: 0000000000000000 x13: 000000000001acf6 x12: 0000000000000000
[ 2030.379560] x11: 0000000000000001 x10: ffff7a0af623f680 x9 : 0000000100000001
[ 2030.379565] x8 : 00000000000000c0 x7 : 0000000000000000 x6 : 000000000000003f
[ 2030.379570] x5 : ffff7a0a003c6a70 x4 : 000000000000001f x3 : ffffd2a48d6722dc
[ 2030.379576] x2 : 0000000000000002 x1 : ffffd2a4a00002f0 x0 : ffff80008129b750
[ 2030.379583] Kernel panic - not syncing: Asynchronous SError Interrupt
[ 2030.379586] CPU: 0 PID: 239 Comm: kworker/0:2 Not tainted 6.8.0-rc4-next-20240216-00015-gc937d3c43ffe-dirty #219
[ 2030.379590] Hardware name: Qualcomm Technologies, Inc. Robotics RB3gen2 (DT)
[ 2030.379592] Workqueue: events output_poll_execute [drm_kms_helper]
[ 2030.379642] Call trace:
[ 2030.379644] dump_backtrace+0xec/0x108
[ 2030.379654] show_stack+0x18/0x24
[ 2030.379659] dump_stack_lvl+0x40/0x84
[ 2030.379664] dump_stack+0x18/0x24
[ 2030.379668] panic+0x130/0x34c
[ 2030.379673] nmi_panic+0x44/0x90
[ 2030.379679] arm64_serror_panic+0x68/0x74
[ 2030.379683] do_serror+0xc4/0xcc
[ 2030.379686] el1h_64_error_handler+0x34/0x4c
[ 2030.379692] el1h_64_error+0x64/0x68
[ 2030.379696] el1_interrupt+0x28/0xc0
[ 2030.379700] el1h_64_irq_handler+0x18/0x24
[ 2030.379706] el1h_64_irq+0x64/0x68
[ 2030.379710] _raw_spin_unlock_irq+0x20/0x48
[ 2030.379718] wait_for_common+0xb4/0x16c
[ 2030.379722] wait_for_completion_timeout+0x14/0x20
[ 2030.379727] dp_ctrl_push_idle+0x34/0x8c [msm]
[ 2030.379844] dp_bridge_atomic_disable+0x18/0x24 [msm]
[ 2030.379959] drm_atomic_bridge_chain_disable+0x6c/0xb4 [drm]
[ 2030.380150] drm_atomic_helper_commit_modeset_disables+0x174/0x57c [drm_kms_helper]
[ 2030.380200] msm_atomic_commit_tail+0x1b4/0x474 [msm]
[ 2030.380316] commit_tail+0xa4/0x158 [drm_kms_helper]
[ 2030.380369] drm_atomic_helper_commit+0x24c/0x26c [drm_kms_helper]
[ 2030.380418] drm_atomic_commit+0xa8/0xd4 [drm]
[ 2030.380529] drm_client_modeset_commit_atomic+0x16c/0x244 [drm]
[ 2030.380641] drm_client_modeset_commit_locked+0x50/0x168 [drm]
[ 2030.380753] drm_client_modeset_commit+0x2c/0x54 [drm]
[ 2030.380865] __drm_fb_helper_restore_fbdev_mode_unlocked+0x60/0xa4 [drm_kms_helper]
[ 2030.380915] drm_fb_helper_hotplug_event+0xe0/0xf4 [drm_kms_helper]
[ 2030.380965] msm_fbdev_client_hotplug+0x28/0xc8 [msm]
[ 2030.381081] drm_client_dev_hotplug+0x94/0x118 [drm]
[ 2030.381192] output_poll_execute+0x214/0x26c [drm_kms_helper]
[ 2030.381241] process_scheduled_works+0x19c/0x2cc
[ 2030.381249] worker_thread+0x290/0x3cc
[ 2030.381255] kthread+0xfc/0x184
[ 2030.381260] ret_from_fork+0x10/0x20

The above could happen if the convoluted hotplug state machine breaks
down so that the device is runtime suspended before
dp_bridge_atomic_disable() is called.


Yes, state machine got broken and I have explained how in the commit text of the RFC. But its not necessarily due to PM runtime but a sequence of events happening from userspace exposing this breakage.

For some reason, possibly due to unrelated changes in timing, possibly
after the hotplug revert, I am no longer able to reproduce the reset
with 6.8-rc7 on the X13s.


I do not know how the hotplug revert fixed this stack because I think this can still happen.

I am however able to get the hotplug state machine to leak
runtime PM reference counts which prevents it from ever being suspended
(e.g. by disconnecting slowly so that we get multiple connect and
disconnect events). This can manifest itself as a hotplug event which is
processed after disconnecting the display:

[drm:dp_panel_read_sink_caps [msm]] *ERROR* read dpcd failed -110


hmm ... this is another new report. I was not aware of this till this instance. We would like to debug this issue too as we have also not seen this one before.

But, I am suspicious about how or why a refcount leak leads to a DPCD read failure.

I would not be surprised at all if there's a sequence of events that
leads to an unbalanced put instead (and the stack trace and observations
above make this appear likely).


Stacktrace has a different analysis.

Were you able to determine what events lead to the premature disabling
of the bus clocks on the RB3?


Yes, please check my RFC . I am waiting for Kuogee to review it too and I shall post it formally as a patch.

Have you got any reason to believe that the revert of the hotplug
notification patch may in anyway prevent that? Or is it just papering
over the issue?


No, I do not think reverting the hotplug notification patch can fix that reset.

Hotplug in Xorg seems to work worse than it did with 6.7, which also had
some issues. Connecting a display once seems to work fine, but trying to
re-enable a reconnected display using xrandr sometimes does not work at
all, while with 6.7 it usually worked on the second xrandr execution.

xrandr reports the reconnected display as disconnected:

Running 'xrandr --output DP-2 --auto' 2-3 times makes xrandr report the
display as connected, but the display is still blank (unlike with 6.7).

As I mentioned elsewhere, the revert of e467e0bde881 ("drm/msm/dp: use
drm_bridge_hpd_notify() to report HPD status changes") in rc7 does seem
to help with the hotplug detect issues that I could reproduce (in VT
console, X and wayland).


Yes revert of e467e0bde881 ("drm/msm/dp: use
> drm_bridge_hpd_notify() to report HPD status changes") affecting hotplug status and associated connection issues makes sense. But I think the reset with the above mentioned stacktrace can still happen.

The question is whether we should revert the whole runtime PM series so
that the bus clock is left on, which should prevent any resets on
disconnect.


At this point, I would say if the RFC resolves the reset for you and Bjorn with the above stack, that should be the way to go and not full revert.

Unless you are facing other issues other than this last reset crash.

Without any analysis from you or reason to be believe the issue to have
been resolved, I'm inclined to just go ahead and revert it. It clearly
had not been tested enough before being merged and I'm quite frustrated
with how this has been handled.


I must admit that the number of issues reported on sc8280xp was surprising for us as well because the series was tested on both sc7180/sc7280 by us and other users but not on sc8280xp as we did not have access to this device that time.

sc7280 is the only other device we supported upstream which supported eDP natively before Qualcomm X1E80100 landed this cycle. Surprisingly enough, perhaps because it uses a chrome defconfig or chrome userspace, no issues were seen that time or even now for that matter.

To avoid the breakages on sc8280xp moving forward as it has more visibility, our team here has decided to setup sc8280xp devices to test out all msm display related changes especially eDP/DP as it shares the eDP/DP driver. We can also CC you on these, in case you want to test it out as well and give your Tested-by.

Johan