From 4a1d5ea53c9aa9f16cff937f3abf59c733ca1bfb Mon Sep 17 00:00:00 2001 From: Vlad Zahorodnii Date: Thu, 25 Nov 2021 09:44:54 +0200 Subject: [PATCH] backends/drm: Improve PauseDevice/ResumeDevice signal handling systemd takes care of setting and dropping master permissions when sending PauseDevice and ResumeDevice signals. When the ResumeDevice signal is received, the relevant drm device should already have master permissions set up. On the other hand, when the active property changes, there's still a chance that systemd haven't granted drm master permissions to us. --- src/backends/drm/drm_backend.cpp | 21 +++++++++++++++++++-- src/backends/drm/drm_backend.h | 3 +++ src/backends/drm/drm_gpu.cpp | 8 ++++---- src/session.h | 16 ++++++++++++++++ src/session_consolekit.cpp | 16 ++++++++++++++++ src/session_consolekit.h | 3 +++ src/session_logind.cpp | 16 ++++++++++++++++ src/session_logind.h | 3 +++ 8 files changed, 80 insertions(+), 6 deletions(-) diff --git a/src/backends/drm/drm_backend.cpp b/src/backends/drm/drm_backend.cpp index 4b130727d0..62a59938fb 100644 --- a/src/backends/drm/drm_backend.cpp +++ b/src/backends/drm/drm_backend.cpp @@ -75,6 +75,11 @@ DrmBackend::~DrmBackend() qDeleteAll(m_gpus); } +bool DrmBackend::isActive() const +{ + return m_active; +} + Session *DrmBackend::session() const { return m_session; @@ -154,6 +159,7 @@ void DrmBackend::reactivate() // removed, we need to re-scan outputs. updateOutputs(); updateCursor(); + Q_EMIT activeChanged(); } void DrmBackend::deactivate() @@ -167,11 +173,22 @@ void DrmBackend::deactivate() } m_active = false; + Q_EMIT activeChanged(); } bool DrmBackend::initialize() { - connect(session(), &Session::activeChanged, this, &DrmBackend::activate); + // TODO: Pause/Resume individual GPU devices instead. + connect(session(), &Session::devicePaused, this, [this](dev_t deviceId) { + if (primaryGpu()->deviceId() == deviceId) { + deactivate(); + } + }); + connect(session(), &Session::deviceResumed, this, [this](dev_t deviceId) { + if (primaryGpu()->deviceId() == deviceId) { + reactivate(); + } + }); connect(session(), &Session::awoke, this, &DrmBackend::turnOutputsOn); if (!m_explicitGpus.isEmpty()) { @@ -212,7 +229,7 @@ bool DrmBackend::initialize() void DrmBackend::handleUdevEvent() { while (auto device = m_udevMonitor->getDevice()) { - if (!session()->isActive()) { + if (!m_active) { continue; } if (!m_explicitGpus.isEmpty() && !m_explicitGpus.contains(device->devNode())) { diff --git a/src/backends/drm/drm_backend.h b/src/backends/drm/drm_backend.h index 097257265d..d10edd74a3 100644 --- a/src/backends/drm/drm_backend.h +++ b/src/backends/drm/drm_backend.h @@ -70,11 +70,14 @@ public: DrmGpu *findGpu(dev_t deviceId) const; DrmGpu *findGpuByFd(int fd) const; + bool isActive() const; + public Q_SLOTS: void turnOutputsOn(); void sceneInitialized() override; Q_SIGNALS: + void activeChanged(); void gpuRemoved(DrmGpu *gpu); void gpuAdded(DrmGpu *gpu); diff --git a/src/backends/drm/drm_gpu.cpp b/src/backends/drm/drm_gpu.cpp index bfd747469c..6ffcdad48d 100644 --- a/src/backends/drm/drm_gpu.cpp +++ b/src/backends/drm/drm_gpu.cpp @@ -107,10 +107,10 @@ DrmGpu::DrmGpu(DrmBackend *backend, const QString &devNode, int fd, dev_t device }); connect(m_leaseDevice, &KWaylandServer::DrmLeaseDeviceV1Interface::leaseRequested, this, &DrmGpu::handleLeaseRequest); connect(m_leaseDevice, &KWaylandServer::DrmLeaseDeviceV1Interface::leaseRevoked, this, &DrmGpu::handleLeaseRevoked); - connect(m_platform->session(), &Session::activeChanged, m_leaseDevice, [this](bool active){ - if (!active) { + connect(m_platform, &DrmBackend::activeChanged, m_leaseDevice, [this]() { + if (!m_platform->isActive()) { // when we gain drm master we want to update outputs first and only then notify the lease device - m_leaseDevice->setDrmMaster(active); + m_leaseDevice->setDrmMaster(false); } }); } @@ -518,7 +518,7 @@ void DrmGpu::pageFlipHandler(int fd, unsigned int sequence, unsigned int sec, un void DrmGpu::dispatchEvents() { - if (!m_platform->session()->isActive()) { + if (!m_platform->isActive()) { return; } drmEventContext context = {}; diff --git a/src/session.h b/src/session.h index db584bfaa1..22ddfc17c0 100644 --- a/src/session.h +++ b/src/session.h @@ -11,6 +11,8 @@ #include #include +#include + namespace KWin { @@ -93,6 +95,20 @@ Q_SIGNALS: */ void activeChanged(bool active); + /** + * This signal is emitted when the specified device can be used again. + */ + void deviceResumed(dev_t deviceId); + + /** + * This signal is emitted when the given device cannot be used by the compositor + * anymore. For example, this normally occurs when switching between VTs. + * + * Note that when this signal is emitted for a DRM device, master permissions can + * be already revoked. + */ + void devicePaused(dev_t deviceId); + protected: explicit Session(QObject *parent = nullptr); }; diff --git a/src/session_consolekit.cpp b/src/session_consolekit.cpp index 828ec90e1e..979d19a369 100644 --- a/src/session_consolekit.cpp +++ b/src/session_consolekit.cpp @@ -290,6 +290,11 @@ bool ConsoleKitSession::initialize() this, SLOT(handlePauseDevice(uint, uint, QString))); + QDBusConnection::systemBus().connect(s_serviceName, m_sessionPath, s_sessionInterface, + QStringLiteral("ResumeDevice"), + this, + SLOT(handleResumeDevice(uint,uint,QDBusUnixFileDescriptor))); + QDBusConnection::systemBus().connect(s_serviceName, m_sessionPath, s_propertiesInterface, QStringLiteral("PropertiesChanged"), this, @@ -308,6 +313,8 @@ void ConsoleKitSession::updateActive(bool active) void ConsoleKitSession::handlePauseDevice(uint major, uint minor, const QString &type) { + Q_EMIT devicePaused(makedev(major, minor)); + if (type == QLatin1String("pause")) { QDBusMessage message = QDBusMessage::createMethodCall(s_serviceName, m_sessionPath, s_sessionInterface, @@ -318,6 +325,15 @@ void ConsoleKitSession::handlePauseDevice(uint major, uint minor, const QString } } +void ConsoleKitSession::handleResumeDevice(uint major, uint minor, QDBusUnixFileDescriptor fileDescriptor) +{ + // We don't care about the file descriptor as the libinput backend will re-open input devices + // and the drm file descriptors remain valid after pausing gpus. + Q_UNUSED(fileDescriptor) + + Q_EMIT deviceResumed(makedev(major, minor)); +} + void ConsoleKitSession::handlePropertiesChanged(const QString &interfaceName, const QVariantMap &properties) { if (interfaceName == s_sessionInterface) { diff --git a/src/session_consolekit.h b/src/session_consolekit.h index e6cbdd29b2..6243f5b3fa 100644 --- a/src/session_consolekit.h +++ b/src/session_consolekit.h @@ -8,6 +8,8 @@ #include "session.h" +#include + namespace KWin { @@ -28,6 +30,7 @@ public: void switchTo(uint terminal) override; private Q_SLOTS: + void handleResumeDevice(uint major, uint minor, QDBusUnixFileDescriptor fileDescriptor); void handlePauseDevice(uint major, uint minor, const QString &type); void handlePropertiesChanged(const QString &interfaceName, const QVariantMap &properties); void handlePrepareForSleep(bool sleep); diff --git a/src/session_logind.cpp b/src/session_logind.cpp index 78a2a2d63a..714ff812ed 100644 --- a/src/session_logind.cpp +++ b/src/session_logind.cpp @@ -288,6 +288,11 @@ bool LogindSession::initialize() this, SLOT(handlePauseDevice(uint,uint,QString))); + QDBusConnection::systemBus().connect(s_serviceName, m_sessionPath, s_sessionInterface, + QStringLiteral("ResumeDevice"), + this, + SLOT(handleResumeDevice(uint,uint,QDBusUnixFileDescriptor))); + QDBusConnection::systemBus().connect(s_serviceName, m_sessionPath, s_propertiesInterface, QStringLiteral("PropertiesChanged"), this, @@ -306,6 +311,8 @@ void LogindSession::updateActive(bool active) void LogindSession::handlePauseDevice(uint major, uint minor, const QString &type) { + Q_EMIT devicePaused(makedev(major, minor)); + if (type == QLatin1String("pause")) { QDBusMessage message = QDBusMessage::createMethodCall(s_serviceName, m_sessionPath, s_sessionInterface, @@ -316,6 +323,15 @@ void LogindSession::handlePauseDevice(uint major, uint minor, const QString &typ } } +void LogindSession::handleResumeDevice(uint major, uint minor, QDBusUnixFileDescriptor fileDescriptor) +{ + // We don't care about the file descriptor as the libinput backend will re-open input devices + // and the drm file descriptors remain valid after pausing gpus. + Q_UNUSED(fileDescriptor) + + Q_EMIT deviceResumed(makedev(major, minor)); +} + void LogindSession::handlePropertiesChanged(const QString &interfaceName, const QVariantMap &properties) { if (interfaceName == s_sessionInterface) { diff --git a/src/session_logind.h b/src/session_logind.h index 3730498370..dc7a2d82ef 100644 --- a/src/session_logind.h +++ b/src/session_logind.h @@ -8,6 +8,8 @@ #include "session.h" +#include + namespace KWin { @@ -28,6 +30,7 @@ public: void switchTo(uint terminal) override; private Q_SLOTS: + void handleResumeDevice(uint major, uint minor, QDBusUnixFileDescriptor fileDescriptor); void handlePauseDevice(uint major, uint minor, const QString &type); void handlePropertiesChanged(const QString &interfaceName, const QVariantMap &properties); void handlePrepareForSleep(bool sleep);