From 33ae5eed132d00ffcb9b6a54841a8e7ec2522b2a Mon Sep 17 00:00:00 2001 From: Laszlo Agocs Date: Mon, 19 Sep 2022 12:58:05 +0200 Subject: [PATCH] eglfs: Add env.var. to disable the dedicated drmHandleEvent thread Setting QT_QPA_EGLFS_KMS_NO_EVENT_READER_THREAD=1 makes it operate like it did before 5.12.7: just calling drmhandleEvent (guarded by a mutex) on the current (main or render, depending on the QQ render loop) thread. This should not be needed and is discouraged (will certainly cause deadlocks in multiscreen setups + QQ threaded render loop on certain embedded systems), but it seems necessary to provide a way to revert back to the old way of functioning as there are reports about problems with screen cloning when the dedicated event reading thread is used. Task-number: QTBUG-91882 Change-Id: I4cddcd09149dcab9e135467b6ef0e047a2a0ecff Reviewed-by: Qt CI Bot Reviewed-by: Andy Nichols (cherry picked from commit 820775166132b073a941f2389fba81db49619688) Reviewed-by: Qt Cherry-pick Bot --- .../eglfs_kms/qeglfskmsgbmdevice.cpp | 17 ++++++++- .../eglfs_kms/qeglfskmsgbmdevice_p.h | 6 ++- .../eglfs_kms/qeglfskmsgbmscreen.cpp | 38 ++++++++++++++++--- .../eglfs_kms/qeglfskmsgbmscreen_p.h | 6 +++ .../eglfs_kms_support/qeglfskmsdevice_p.h | 6 --- 5 files changed, 58 insertions(+), 15 deletions(-) diff --git a/src/plugins/platforms/eglfs/deviceintegration/eglfs_kms/qeglfskmsgbmdevice.cpp b/src/plugins/platforms/eglfs/deviceintegration/eglfs_kms/qeglfskmsgbmdevice.cpp index b5b7811b796..a98ed0b8120 100644 --- a/src/plugins/platforms/eglfs/deviceintegration/eglfs_kms/qeglfskmsgbmdevice.cpp +++ b/src/plugins/platforms/eglfs/deviceintegration/eglfs_kms/qeglfskmsgbmdevice.cpp @@ -83,7 +83,13 @@ bool QEglFSKmsGbmDevice::open() setFd(fd); - m_eventReader.create(this); + if (usesEventReader()) { + qCDebug(qLcEglfsKmsDebug, "Using dedicated drm event reading thread"); + m_eventReader.create(this); + } else { + qCDebug(qLcEglfsKmsDebug, "Not using dedicated drm event reading thread; " + "threaded multi-screen setups may experience problems"); + } return true; } @@ -92,7 +98,8 @@ void QEglFSKmsGbmDevice::close() { // Note: screens are gone at this stage. - m_eventReader.destroy(); + if (usesEventReader()) + m_eventReader.destroy(); if (m_gbm_device) { gbm_device_destroy(m_gbm_device); @@ -174,4 +181,10 @@ void QEglFSKmsGbmDevice::registerScreen(QPlatformScreen *screen, m_globalCursor->reevaluateVisibilityForScreens(); } +bool QEglFSKmsGbmDevice::usesEventReader() const +{ + static const bool eventReaderThreadDisabled = qEnvironmentVariableIntValue("QT_QPA_EGLFS_KMS_NO_EVENT_READER_THREAD"); + return !eventReaderThreadDisabled; +} + QT_END_NAMESPACE diff --git a/src/plugins/platforms/eglfs/deviceintegration/eglfs_kms/qeglfskmsgbmdevice_p.h b/src/plugins/platforms/eglfs/deviceintegration/eglfs_kms/qeglfskmsgbmdevice_p.h index bb67d800ee9..d7bd1f613c4 100644 --- a/src/plugins/platforms/eglfs/deviceintegration/eglfs_kms/qeglfskmsgbmdevice_p.h +++ b/src/plugins/platforms/eglfs/deviceintegration/eglfs_kms/qeglfskmsgbmdevice_p.h @@ -55,6 +55,7 @@ #include "qeglfskmsgbmcursor_p.h" #include +#include #include @@ -87,11 +88,14 @@ public: const QPoint &virtualPos, const QList &virtualSiblings) override; + bool usesEventReader() const; + QEglFSKmsEventReader *eventReader() { return &m_eventReader; } + private: Q_DISABLE_COPY(QEglFSKmsGbmDevice) gbm_device *m_gbm_device; - + QEglFSKmsEventReader m_eventReader; QEglFSKmsGbmCursor *m_globalCursor; }; diff --git a/src/plugins/platforms/eglfs/deviceintegration/eglfs_kms/qeglfskmsgbmscreen.cpp b/src/plugins/platforms/eglfs/deviceintegration/eglfs_kms/qeglfskmsgbmscreen.cpp index 301aaf15bfe..6b9727844bc 100644 --- a/src/plugins/platforms/eglfs/deviceintegration/eglfs_kms/qeglfskmsgbmscreen.cpp +++ b/src/plugins/platforms/eglfs/deviceintegration/eglfs_kms/qeglfskmsgbmscreen.cpp @@ -56,6 +56,8 @@ QT_BEGIN_NAMESPACE Q_DECLARE_LOGGING_CATEGORY(qLcEglfsKmsDebug) +QMutex QEglFSKmsGbmScreen::m_nonThreadedFlipMutex; + static inline uint32_t drmFormatToGbmFormat(uint32_t drmFormat) { Q_ASSERT(DRM_FORMAT_XRGB8888 == GBM_FORMAT_XRGB8888); @@ -263,6 +265,18 @@ void QEglFSKmsGbmScreen::ensureModeSet(uint32_t fb) } } +void QEglFSKmsGbmScreen::nonThreadedPageFlipHandler(int fd, + unsigned int sequence, + unsigned int tv_sec, + unsigned int tv_usec, + void *user_data) +{ + Q_UNUSED(fd); + QEglFSKmsGbmScreen *screen = static_cast(user_data); + screen->flipFinished(); + screen->pageFlipped(sequence, tv_sec, tv_usec); +} + void QEglFSKmsGbmScreen::waitForFlip() { if (m_headless || m_cloneSource) @@ -272,12 +286,24 @@ void QEglFSKmsGbmScreen::waitForFlip() if (!m_gbm_bo_next) return; - m_flipMutex.lock(); - device()->eventReader()->startWaitFlip(this, &m_flipMutex, &m_flipCond); - m_flipCond.wait(&m_flipMutex); - m_flipMutex.unlock(); - - flipFinished(); + QEglFSKmsGbmDevice *dev = static_cast(device()); + if (dev->usesEventReader()) { + m_flipMutex.lock(); + dev->eventReader()->startWaitFlip(this, &m_flipMutex, &m_flipCond); + m_flipCond.wait(&m_flipMutex); + m_flipMutex.unlock(); + flipFinished(); + } else { + QMutexLocker lock(&m_nonThreadedFlipMutex); + while (m_gbm_bo_next) { + drmEventContext drmEvent; + memset(&drmEvent, 0, sizeof(drmEvent)); + drmEvent.version = 2; + drmEvent.vblank_handler = nullptr; + drmEvent.page_flip_handler = nonThreadedPageFlipHandler; + drmHandleEvent(device()->fd(), &drmEvent); + } + } #if QT_CONFIG(drm_atomic) device()->threadLocalAtomicReset(); diff --git a/src/plugins/platforms/eglfs/deviceintegration/eglfs_kms/qeglfskmsgbmscreen_p.h b/src/plugins/platforms/eglfs/deviceintegration/eglfs_kms/qeglfskmsgbmscreen_p.h index ae12e95e62d..66855b9bb76 100644 --- a/src/plugins/platforms/eglfs/deviceintegration/eglfs_kms/qeglfskmsgbmscreen_p.h +++ b/src/plugins/platforms/eglfs/deviceintegration/eglfs_kms/qeglfskmsgbmscreen_p.h @@ -88,6 +88,11 @@ protected: void flipFinished(); void ensureModeSet(uint32_t fb); void cloneDestFlipFinished(QEglFSKmsGbmScreen *cloneDestScreen); + static void nonThreadedPageFlipHandler(int fd, + unsigned int sequence, + unsigned int tv_sec, + unsigned int tv_usec, + void *user_data); gbm_surface *m_gbm_surface; @@ -97,6 +102,7 @@ protected: QMutex m_flipMutex; QWaitCondition m_flipCond; + static QMutex m_nonThreadedFlipMutex; QScopedPointer m_cursor; diff --git a/src/plugins/platforms/eglfs/deviceintegration/eglfs_kms_support/qeglfskmsdevice_p.h b/src/plugins/platforms/eglfs/deviceintegration/eglfs_kms_support/qeglfskmsdevice_p.h index 006f4a656e6..0a7679f765e 100644 --- a/src/plugins/platforms/eglfs/deviceintegration/eglfs_kms_support/qeglfskmsdevice_p.h +++ b/src/plugins/platforms/eglfs/deviceintegration/eglfs_kms_support/qeglfskmsdevice_p.h @@ -53,7 +53,6 @@ // #include "private/qeglfsglobal_p.h" -#include "qeglfskmseventreader_p.h" #include QT_BEGIN_NAMESPACE @@ -67,11 +66,6 @@ public: bool isPrimary, const QPoint &virtualPos, const QList &virtualSiblings) override; - - QEglFSKmsEventReader *eventReader() { return &m_eventReader; } - -protected: - QEglFSKmsEventReader m_eventReader; }; QT_END_NAMESPACE