https://bugs.kde.org/show_bug.cgi?id=506359
Giovanni Bassi <[email protected]> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |[email protected] --- Comment #4 from Giovanni Bassi <[email protected]> --- I think I am hitting this same bug, and I have been running a local workaround for it for a while. I am not a KDE or kwin developer, I am a normal user who reads C++, so please treat the analysis below as a report of what I observed plus my best guess at the mechanism, not as an authoritative diagnosis. If it turns out my situation is a different root cause than the original report, tell me and I will open a separate bug. My setup: kwin 6.7.4 (Plasma 6.7.4) on Wayland, NixOS, kernel 7.1.5. Two AMD GPUs, both on amdgpu: a Navi 32 discrete card (1002:747E at 0000:03:00.0) that drives all the monitors, and the CPU's integrated GPU (1002:164E at 0000:6e:00.0). Three DisplayPort monitors, all on the discrete card: a Samsung U32J59x on DP-1 and two Samsung U28E590 on DP-2 and DP-3. The symptom: after the screens have been off for a while (DPMS off, typically overnight, with the session locked), turning the monitors back on leaves me with an unusable lock screen. The greeter is on screen but it is not bound to the real outputs anymore, and it never recovers on its own no matter how long I wait. Face authentication (using howdy) actually succeeds and the session still stays locked, which is how I know the authentication path is fine and the problem is on the output/screen side. What I believe is happening: when these monitors come back from DPMS off, they drop and re-assert HPD while the DisplayPort link is retrained. Each monitor does this independently and they are staggered by a few hundred milliseconds. DrmBackend::handleUdevEvent() in src/backends/drm/drm_backend.cpp reprobes synchronously on every single udev "change" event it receives, so during that storm there is a window where all three connectors have already reported themselves as gone but none of them has come back yet. In that window kwin sees zero connected outputs. Everything that depends on there being a screen then falls back to a placeholder, and the already-running kscreenlocker_greet never rebinds to the real screens once they do come back a moment later. So a transient, self-correcting hardware state gets turned into a permanent broken session, because kwin acted on an intermediate snapshot instead of waiting for the dust to settle. I want to be clear that the HPD drop itself may well be a monitor or amdgpu quirk rather than a kwin problem. My point is only that kwin currently has no protection against acting on that intermediate state, and that is what makes it unrecoverable rather than a brief flicker. For what it is worth, this machine also needs amdgpu.dcdebugmask=0x10 (PSR off) and amdgpu.sg_display=0 for a separate display-pipe failure on wake, and even with both of those in place the disconnect storm still happens, so I do not think more driver workarounds are the answer here. It also seems to me that kwin already accepts elsewhere that a temporary disconnect is a normal thing when a display is in standby: https://invent.kde.org/plasma/kwin/-/merge_requests/2940 deliberately keeps such an output off instead of waking it. That change handles what to do about the disconnect, but nothing stops kwin from acting on the disconnect in the first place. The workaround I have been running is below. It coalesces the udev change events instead of reprobing on each one: the first event starts a 1 second settle timer that each further event restarts, so the reprobe only happens once the events stop arriving, with a 3 second cap so a display that keeps generating events cannot postpone the update forever. When the timer fires it updates all GPUs rather than just the one the event came from, because the events can span more than one GPU and because the GPU the events came from may be gone by then. Please do not read this as a proposed fix. The timings are values that work on my hardware and nothing more, I have not thought about what this does to genuine hotplug latency (unplugging a cable is now up to a second slower to register), and I have not tested it on anything other than my own three-monitor setup. I am posting it because it has made the machine reliable for me across several Plasma releases, and because if the approach is directionally right then somebody who actually knows this code can do it properly. This patch is against 6.7.4; I originally wrote it against an earlier release and have had to rebase it a couple of times as the surrounding code moved. ```diff --- a/src/backends/drm/drm_backend.h +++ b/src/backends/drm/drm_backend.h @@ -9,10 +9,12 @@ #pragma once #include "core/outputbackend.h" +#include <QElapsedTimer> #include <QList> #include <QPointer> #include <QSize> #include <QSocketNotifier> +#include <QTimer> #include <memory> #include <sys/types.h> @@ -80,11 +82,14 @@ void addOutput(DrmAbstractOutput *output); void removeOutput(DrmAbstractOutput *output); void handleUdevEvent(); + void scheduleUpdateOutputs(); DrmGpu *addGpu(const QString &fileName); std::unique_ptr<Udev> m_udev; std::unique_ptr<UdevMonitor> m_udevMonitor; std::unique_ptr<QSocketNotifier> m_socketNotifier; + QTimer m_updateOutputsTimer; + QElapsedTimer m_hotplugTimestamp; Session *m_session; QList<DrmAbstractOutput *> m_outputs; --- a/src/backends/drm/drm_backend.cpp +++ b/src/backends/drm/drm_backend.cpp @@ -52,12 +52,28 @@ namespace KWin { +// Displays can report themselves as briefly disconnected while their link is being +// retrained, most notably when they come back from dpms off. With multiple displays +// those events arrive staggered, so acting on each one as it comes in can result in +// observing a state where no output is connected at all. Wait for the events to +// settle instead, but don't let a display that keeps generating them postpone the +// update indefinitely. +static constexpr auto s_hotplugSettleTime = 1s; +static constexpr auto s_hotplugMaxDelay = 3s; + DrmBackend::DrmBackend(Session *session, QObject *parent) : OutputBackend(parent) , m_udev(std::make_unique<Udev>()) , m_udevMonitor(m_udev->createMonitor()) , m_session(session) { + m_updateOutputsTimer.setSingleShot(true); + connect(&m_updateOutputsTimer, &QTimer::timeout, this, [this]() { + m_hotplugTimestamp.invalidate(); + // all gpus are updated, both because the events may span more than one of + // them and because the gpu the events came from might be gone by now + updateOutputs(); + }); } DrmBackend::~DrmBackend() = default; @@ -206,12 +222,25 @@ DrmGpu *gpu = findGpu(device->devNum()); if (gpu && gpu->isActive()) { qCDebug(KWIN_DRM) << "Received change event for monitored drm device" << gpu->drmDevice()->path(); - updateOutputs(gpu); + scheduleUpdateOutputs(); } } } } +void DrmBackend::scheduleUpdateOutputs() +{ + if (!m_hotplugTimestamp.isValid()) { + m_hotplugTimestamp.start(); + } else if (m_hotplugTimestamp.durationElapsed() >= s_hotplugMaxDelay) { + m_updateOutputsTimer.stop(); + m_hotplugTimestamp.invalidate(); + updateOutputs(); + return; + } + m_updateOutputsTimer.start(s_hotplugSettleTime); +} + DrmGpu *DrmBackend::addGpu(const QString &fileName) { std::expected<int, Session::Error> fd = m_session->openRestricted(fileName); ``` I am happy to gather whatever would help confirm this. I can revert the patch and capture kwin_wayland logs with the KWIN_DRM category enabled across a DPMS off and on cycle, so you can see the actual sequence and timing of the change events and the connector states kwin reads at each step. Just tell me what you want captured and how. I hope this helps. Cheers. -- You are receiving this mail because: You are watching all bug changes.
