From 7b8771d618d34fd117bf390e5a6639fbab7727b2 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 6d903a3f1e6..89479fc250b 100644 --- a/src/plugins/platforms/eglfs/deviceintegration/eglfs_kms/qeglfskmsgbmdevice.cpp +++ b/src/plugins/platforms/eglfs/deviceintegration/eglfs_kms/qeglfskmsgbmdevice.cpp @@ -47,7 +47,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; } @@ -56,7 +62,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); @@ -138,4 +145,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 832b3a28732..e00992ed291 100644 --- a/src/plugins/platforms/eglfs/deviceintegration/eglfs_kms/qeglfskmsgbmdevice_p.h +++ b/src/plugins/platforms/eglfs/deviceintegration/eglfs_kms/qeglfskmsgbmdevice_p.h @@ -19,6 +19,7 @@ #include "qeglfskmsgbmcursor_p.h" #include +#include #include @@ -51,11 +52,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 c7b0208188e..5ac28315ffd 100644 --- a/src/plugins/platforms/eglfs/deviceintegration/eglfs_kms/qeglfskmsgbmscreen.cpp +++ b/src/plugins/platforms/eglfs/deviceintegration/eglfs_kms/qeglfskmsgbmscreen.cpp @@ -20,6 +20,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); @@ -227,6 +229,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) @@ -236,12 +250,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 d2882038bdb..5a81b8004db 100644 --- a/src/plugins/platforms/eglfs/deviceintegration/eglfs_kms/qeglfskmsgbmscreen_p.h +++ b/src/plugins/platforms/eglfs/deviceintegration/eglfs_kms/qeglfskmsgbmscreen_p.h @@ -52,6 +52,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; @@ -61,6 +66,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 7dc5235d338..6e11953a699 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 @@ -17,7 +17,6 @@ // #include "private/qeglfsglobal_p.h" -#include "qeglfskmseventreader_p.h" #include QT_BEGIN_NAMESPACE @@ -31,11 +30,6 @@ public: bool isPrimary, const QPoint &virtualPos, const QList &virtualSiblings) override; - - QEglFSKmsEventReader *eventReader() { return &m_eventReader; } - -protected: - QEglFSKmsEventReader m_eventReader; }; QT_END_NAMESPACE