1 files changed, 170 insertions, 51 deletions

diff --git a/src/plugins/platforms/eglfs/deviceintegration/eglfs_kms/qeglfskmsgbmscreen.cpp b/src/plugins/platforms/eglfs/deviceintegration/eglfs_kms/qeglfskmsgbmscreen.cpp
index c7b0208188..8dcfed04db 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);
@@ -71,8 +73,9 @@ QEglFSKmsGbmScreen::FrameBuffer *QEglFSKmsGbmScreen::framebufferForBufferObject(
         return nullptr;
     }
 
-    gbm_bo_set_user_data(bo, fb.get(), bufferDestroyedHandler);
-    return fb.release();
+    auto res = fb.get();
+    gbm_bo_set_user_data(bo, fb.release(), bufferDestroyedHandler);
+    return res;
 }
 
 QEglFSKmsGbmScreen::QEglFSKmsGbmScreen(QEglFSKmsDevice *device, const QKmsOutput &output, bool headless)
@@ -140,11 +143,12 @@ gbm_surface *QEglFSKmsGbmScreen::createSurface(EGLConfig eglConfig)
             }
         }
 
+        const uint32_t gbmFormat = drmFormatToGbmFormat(m_output.drm_format);
+
         // Fallback for older drivers, and when "format" is explicitly specified
         // in the output config. (not guaranteed that the requested format works
         // of course, but do what we are told to)
         if (!m_gbm_surface) {
-            uint32_t gbmFormat = drmFormatToGbmFormat(m_output.drm_format);
             if (queryFromEgl)
                 qCDebug(qLcEglfsKmsDebug, "Could not create surface with EGL_NATIVE_VISUAL_ID, falling back to format %x", gbmFormat);
             m_gbm_surface = gbm_surface_create(gbmDevice,
@@ -153,13 +157,39 @@ gbm_surface *QEglFSKmsGbmScreen::createSurface(EGLConfig eglConfig)
                                            gbmFormat,
                                            gbmFlags());
         }
+
+        // Fallback for some drivers, its required to request with modifiers
+        if (!m_gbm_surface) {
+            uint64_t modifier = DRM_FORMAT_MOD_LINEAR;
+
+            m_gbm_surface = gbm_surface_create_with_modifiers(gbmDevice,
+                                    rawGeometry().width(),
+                                    rawGeometry().height(),
+                                    gbmFormat,
+                                    &modifier, 1);
+        }
+        // Fail here, as it would fail with the next usage of the GBM surface, which is very unexpected
+        if (!m_gbm_surface)
+            qFatal("Could not create GBM surface!");
     }
     return m_gbm_surface; // not owned, gets destroyed in QEglFSKmsGbmIntegration::destroyNativeWindow() via QEglFSKmsGbmWindow::invalidateSurface()
 }
 
 void QEglFSKmsGbmScreen::resetSurface()
 {
+    m_flipPending = false; // not necessarily true but enough to keep bo_next
+    m_gbm_bo_current = nullptr;
     m_gbm_surface = nullptr;
+
+    // Leave m_gbm_bo_next untouched. waitForFlip() should
+    // still do its work, when called. Otherwise we end up
+    // in device-is-busy errors if there is a new QWindow
+    // created afterwards. (QTBUG-122663)
+
+    // If not using atomic, will need a new drmModeSetCrtc if a new window
+    // gets created later on (and so there's a new fb).
+    if (!device()->hasAtomicSupport())
+        needsNewModeSetForNextFb = true;
 }
 
 void QEglFSKmsGbmScreen::initCloning(QPlatformScreen *screenThisScreenClones,
@@ -171,8 +201,10 @@ void QEglFSKmsGbmScreen::initCloning(QPlatformScreen *screenThisScreenClones,
         qWarning("QEglFSKmsGbmScreen %s cannot be clone source and destination at the same time", qPrintable(name()));
         return;
     }
-    if (clonesAnother)
+    if (clonesAnother) {
         m_cloneSource = static_cast<QEglFSKmsGbmScreen *>(screenThisScreenClones);
+        qCDebug(qLcEglfsKmsDebug, "Screen %s clones %s", qPrintable(name()), qPrintable(m_cloneSource->name()));
+    }
 
     // clone sources need to know their additional destinations
     for (QPlatformScreen *s : screensCloningThisScreen) {
@@ -187,8 +219,9 @@ void QEglFSKmsGbmScreen::ensureModeSet(uint32_t fb)
     QKmsOutput &op(output());
     const int fd = device()->fd();
 
-    if (!op.mode_set) {
+    if (!op.mode_set || needsNewModeSetForNextFb) {
         op.mode_set = true;
+        needsNewModeSetForNextFb = false;
 
         bool doModeSet = true;
         drmModeCrtcPtr currentMode = drmModeGetCrtc(fd, op.crtc_id);
@@ -227,6 +260,29 @@ 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)
+{
+    // note that with cloning involved this callback is called also for screens that clone another one
+    Q_UNUSED(fd);
+    QEglFSKmsGbmScreen *screen = static_cast<QEglFSKmsGbmScreen *>(user_data);
+    screen->flipFinished();
+    screen->pageFlipped(sequence, tv_sec, tv_usec);
+}
+
+void QEglFSKmsGbmScreen::waitForFlipWithEventReader(QEglFSKmsGbmScreen *screen)
+{
+    m_flipMutex.lock();
+    QEglFSKmsGbmDevice *dev = static_cast<QEglFSKmsGbmDevice *>(device());
+    dev->eventReader()->startWaitFlip(screen, &m_flipMutex, &m_flipCond);
+    m_flipCond.wait(&m_flipMutex);
+    m_flipMutex.unlock();
+    screen->flipFinished();
+}
+
 void QEglFSKmsGbmScreen::waitForFlip()
 {
     if (m_headless || m_cloneSource)
@@ -236,18 +292,69 @@ 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<QEglFSKmsGbmDevice *>(device());
+    if (dev->usesEventReader()) {
+        waitForFlipWithEventReader(this);
+        // Now, unlike on the other code path, we need to ensure the
+        // flips have completed for the screens that just scan out
+        // this one's content, because the eventReader's wait is
+        // per-output.
+        for (CloneDestination &d : m_cloneDests) {
+            if (d.screen != this)
+                waitForFlipWithEventReader(d.screen);
+        }
+    } 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();
 #endif
 }
 
+#if QT_CONFIG(drm_atomic)
+static void addAtomicFlip(drmModeAtomicReq *request, const QKmsOutput &output, uint32_t fb)
+{
+    drmModeAtomicAddProperty(request, output.eglfs_plane->id,
+                             output.eglfs_plane->framebufferPropertyId, fb);
+
+    drmModeAtomicAddProperty(request, output.eglfs_plane->id,
+                             output.eglfs_plane->crtcPropertyId, output.crtc_id);
+
+    drmModeAtomicAddProperty(request, output.eglfs_plane->id,
+                             output.eglfs_plane->srcwidthPropertyId, output.size.width() << 16);
+
+    drmModeAtomicAddProperty(request, output.eglfs_plane->id,
+                             output.eglfs_plane->srcXPropertyId, 0);
+
+    drmModeAtomicAddProperty(request, output.eglfs_plane->id,
+                             output.eglfs_plane->srcYPropertyId, 0);
+
+    drmModeAtomicAddProperty(request, output.eglfs_plane->id,
+                             output.eglfs_plane->srcheightPropertyId, output.size.height() << 16);
+
+    drmModeAtomicAddProperty(request, output.eglfs_plane->id,
+                             output.eglfs_plane->crtcXPropertyId, 0);
+
+    drmModeAtomicAddProperty(request, output.eglfs_plane->id,
+                             output.eglfs_plane->crtcYPropertyId, 0);
+
+    drmModeAtomicAddProperty(request, output.eglfs_plane->id,
+                             output.eglfs_plane->crtcwidthPropertyId, output.modes[output.mode].hdisplay);
+
+    drmModeAtomicAddProperty(request, output.eglfs_plane->id,
+                             output.eglfs_plane->crtcheightPropertyId, output.modes[output.mode].vdisplay);
+}
+#endif
+
 void QEglFSKmsGbmScreen::flip()
 {
     // For headless screen just return silently. It is not necessarily an error
@@ -267,14 +374,24 @@ void QEglFSKmsGbmScreen::flip()
 
     m_gbm_bo_next = gbm_surface_lock_front_buffer(m_gbm_surface);
     if (!m_gbm_bo_next) {
-        qWarning("Could not lock GBM surface front buffer!");
+        qWarning("Could not lock GBM surface front buffer for screen %s", qPrintable(name()));
         return;
     }
 
+    auto gbmRelease = qScopeGuard([this]{
+        m_flipPending = false;
+        gbm_surface_release_buffer(m_gbm_surface, m_gbm_bo_next);
+        m_gbm_bo_next = nullptr;
+    });
+
     FrameBuffer *fb = framebufferForBufferObject(m_gbm_bo_next);
+    if (!fb) {
+        qWarning("FrameBuffer not available. Cannot flip");
+        return;
+    }
     ensureModeSet(fb->fb);
 
-    QKmsOutput &op(output());
+    const QKmsOutput &thisOutput(output());
     const int fd = device()->fd();
     m_flipPending = true;
 
@@ -282,40 +399,27 @@ void QEglFSKmsGbmScreen::flip()
 #if QT_CONFIG(drm_atomic)
         drmModeAtomicReq *request = device()->threadLocalAtomicRequest();
         if (request) {
-            drmModeAtomicAddProperty(request, op.eglfs_plane->id, op.eglfs_plane->framebufferPropertyId, fb->fb);
-            drmModeAtomicAddProperty(request, op.eglfs_plane->id, op.eglfs_plane->crtcPropertyId, op.crtc_id);
-            drmModeAtomicAddProperty(request, op.eglfs_plane->id, op.eglfs_plane->srcwidthPropertyId,
-                                     op.size.width() << 16);
-            drmModeAtomicAddProperty(request, op.eglfs_plane->id, op.eglfs_plane->srcXPropertyId, 0);
-            drmModeAtomicAddProperty(request, op.eglfs_plane->id, op.eglfs_plane->srcYPropertyId, 0);
-            drmModeAtomicAddProperty(request, op.eglfs_plane->id, op.eglfs_plane->srcheightPropertyId,
-                                     op.size.height() << 16);
-            drmModeAtomicAddProperty(request, op.eglfs_plane->id, op.eglfs_plane->crtcXPropertyId, 0);
-            drmModeAtomicAddProperty(request, op.eglfs_plane->id, op.eglfs_plane->crtcYPropertyId, 0);
-            drmModeAtomicAddProperty(request, op.eglfs_plane->id, op.eglfs_plane->crtcwidthPropertyId,
-                                     m_output.modes[m_output.mode].hdisplay);
-            drmModeAtomicAddProperty(request, op.eglfs_plane->id, op.eglfs_plane->crtcheightPropertyId,
-                                     m_output.modes[m_output.mode].vdisplay);
-
+            addAtomicFlip(request, thisOutput, fb->fb);
             static int zpos = qEnvironmentVariableIntValue("QT_QPA_EGLFS_KMS_ZPOS");
-            if (zpos)
-                drmModeAtomicAddProperty(request, op.eglfs_plane->id, op.eglfs_plane->zposPropertyId, zpos);
+            if (zpos) {
+                drmModeAtomicAddProperty(request, thisOutput.eglfs_plane->id,
+                                         thisOutput.eglfs_plane->zposPropertyId, zpos);
+            }
             static uint blendOp = uint(qEnvironmentVariableIntValue("QT_QPA_EGLFS_KMS_BLEND_OP"));
-            if (blendOp)
-                drmModeAtomicAddProperty(request, op.eglfs_plane->id, op.eglfs_plane->blendOpPropertyId, blendOp);
+            if (blendOp) {
+                drmModeAtomicAddProperty(request, thisOutput.eglfs_plane->id,
+                                         thisOutput.eglfs_plane->blendOpPropertyId, blendOp);
+            }
         }
 #endif
     } else {
         int ret = drmModePageFlip(fd,
-                              op.crtc_id,
-                              fb->fb,
-                              DRM_MODE_PAGE_FLIP_EVENT,
-                              this);
+                                  thisOutput.crtc_id,
+                                  fb->fb,
+                                  DRM_MODE_PAGE_FLIP_EVENT,
+                                  this);
         if (ret) {
             qErrnoWarning("Could not queue DRM page flip on screen %s", qPrintable(name()));
-            m_flipPending = false;
-            gbm_surface_release_buffer(m_gbm_surface, m_gbm_bo_next);
-            m_gbm_bo_next = nullptr;
             return;
         }
     }
@@ -324,17 +428,20 @@ void QEglFSKmsGbmScreen::flip()
         if (d.screen != this) {
             d.screen->ensureModeSet(fb->fb);
             d.cloneFlipPending = true;
-            QKmsOutput &destOutput(d.screen->output());
+            const QKmsOutput &destOutput(d.screen->output());
 
             if (device()->hasAtomicSupport()) {
 #if QT_CONFIG(drm_atomic)
                 drmModeAtomicReq *request = device()->threadLocalAtomicRequest();
-                if (request) {
-                    drmModeAtomicAddProperty(request, destOutput.eglfs_plane->id,
-                                                      destOutput.eglfs_plane->framebufferPropertyId, fb->fb);
-                    drmModeAtomicAddProperty(request, destOutput.eglfs_plane->id,
-                                                      destOutput.eglfs_plane->crtcPropertyId, destOutput.crtc_id);
-                }
+                if (request)
+                    addAtomicFlip(request, destOutput, fb->fb);
+
+                // ### This path is broken. On the other branch we can easily
+                // pass in d.screen as the user_data for drmModePageFlip, but
+                // using one atomic request breaks down here since we get events
+                // with the same user_data passed to drmModeAtomicCommit.  Until
+                // this gets reworked (multiple requests?) screen cloning is not
+                // compatible with atomic.
 #endif
             } else {
                 int ret = drmModePageFlip(fd,
@@ -343,16 +450,24 @@ void QEglFSKmsGbmScreen::flip()
                                           DRM_MODE_PAGE_FLIP_EVENT,
                                           d.screen);
                 if (ret) {
-                    qErrnoWarning("Could not queue DRM page flip for clone screen %s", qPrintable(name()));
+                    qErrnoWarning("Could not queue DRM page flip for screen %s (clones screen %s)",
+                                  qPrintable(d.screen->name()),
+                                  qPrintable(name()));
                     d.cloneFlipPending = false;
                 }
             }
         }
     }
 
+    if (device()->hasAtomicSupport()) {
 #if QT_CONFIG(drm_atomic)
-    device()->threadLocalAtomicCommit(this);
+        if (!device()->threadLocalAtomicCommit(this)) {
+            return;
+        }
 #endif
+    }
+
+    gbmRelease.dismiss();
 }
 
 void QEglFSKmsGbmScreen::flipFinished()
@@ -379,19 +494,23 @@ void QEglFSKmsGbmScreen::cloneDestFlipFinished(QEglFSKmsGbmScreen *cloneDestScre
 
 void QEglFSKmsGbmScreen::updateFlipStatus()
 {
-    Q_ASSERT(!m_cloneSource);
+    // only for 'real' outputs that own the color buffer, i.e. that are not cloning another one
+    if (m_cloneSource)
+        return;
 
+    // proceed only if flips for both this and all others that clone this have finished
     if (m_flipPending)
         return;
 
-    for (const CloneDestination &d : qAsConst(m_cloneDests)) {
+    for (const CloneDestination &d : std::as_const(m_cloneDests)) {
         if (d.cloneFlipPending)
             return;
     }
 
-    if (m_gbm_bo_current)
+    if (m_gbm_bo_current) {
         gbm_surface_release_buffer(m_gbm_surface,
                                    m_gbm_bo_current);
+    }
 
     m_gbm_bo_current = m_gbm_bo_next;
     m_gbm_bo_next = nullptr;