Bug 357543

Summary:	drmModeSetCrtc not called when presenting GBM buffer (Wayland)
Product:	[Plasma] kwin	Reporter:	Jay Cornwall <jay>
Component:	platform-drm	Assignee:	KWin default assignee <kwin-bugs-null>
Status:	RESOLVED FIXED
Severity:	grave	CC:	luis.davila.pc
Priority:	NOR
Version First Reported In:	5.5.2
Target Milestone:	---
Platform:	Compiled Sources
OS:	Linux
Latest Commit:	http://commits.kde.org/kwin/a18177cc24d20bca02f60a95b67f2dcbd1ee8afc	Version Fixed/Implemented In:	5.5.4
Sentry Crash Report:

Description Jay Cornwall 2016-01-04 20:02:24 UTC

DrmBuffer uses both DRM (for m_blackBuffer) and GBM to allocate framebuffers.

drmModeSetCrtc is only called when the buffer stride changes. However, the DRM and GBM buffers have different tiling configurations (with identical strides) on drm/radeon. It is invalid to call drmModePageFlip when switching between these two buffer types without first calling drmModeSetCrtc.

This currently results in corrupted visual output with KWin/Wayland. Adding a call to drmModeSetCrtc when first switching from the DRM buffer to the GBM buffer fixes the issue.

Reproducible: Always

Comment 1 Martin Flöser 2016-01-05 07:07:46 UTC

thanks for the info, didn't know that!

I'm surprised that I never noticed any corrupted visuals. Maybe it just matches on my system?

Comment 2 Martin Flöser 2016-01-05 15:24:33 UTC

possible patch (not tested):

diff --git a/backends/drm/drm_backend.cpp b/backends/drm/drm_backend.cpp
index 128bdc2..7b1b240 100644
--- a/backends/drm/drm_backend.cpp
+++ b/backends/drm/drm_backend.cpp
@@ -614,7 +614,7 @@ bool DrmOutput::present(DrmBuffer *buffer)
     if (m_currentBuffer) {
         return false;
     }
-    if (m_lastStride != buffer->stride()) {
+    if (m_lastStride != buffer->stride() || m_lastGbm != buffer->isGbm()) {
         // need to set a new mode first
         if (!setMode(buffer)) {
             return false;
@@ -811,6 +811,7 @@ bool DrmOutput::setMode(DrmBuffer *buffer)
 {
     if (drmModeSetCrtc(m_backend->fd(), m_crtcId, buffer->bufferId(), 0, 0, &m_connector, 1, &m_mode) == 0) {
         m_lastStride = buffer->stride();
+        m_lastGbm = buffer->isGbm();
         return true;
     } else {
         qCWarning(KWIN_DRM) << "Mode setting failed";
diff --git a/backends/drm/drm_backend.h b/backends/drm/drm_backend.h
index af22594..b247776 100644
--- a/backends/drm/drm_backend.h
+++ b/backends/drm/drm_backend.h
@@ -181,6 +181,7 @@ private:
     quint32 m_crtcId = 0;
     quint32 m_connector = 0;
     quint32 m_lastStride = 0;
+    bool m_lastGbm = false;
     drmModeModeInfo m_mode;
     DrmBuffer *m_currentBuffer = nullptr;
     DrmBuffer *m_blackBuffer = nullptr;
@@ -221,6 +222,9 @@ public:
     gbm_bo *gbm() const {
         return m_bo;
     }
+    bool isGbm() const {
+        return m_bo != nullptr;
+    }
     void releaseGbm();
 
 private:

Comment 3 Jay Cornwall 2016-01-05 17:47:28 UTC

Looks good on my Southern Islands AMD GPU.

I just noticed there is still a tiling issue on a Volcanic Islands AMD GPU but I think it's a different problem, as I can see the kernel picking up a non-zero tiling configuration with this change. I'll dig into that separately.

Comment 4 Martin Flöser 2016-01-11 11:55:03 UTC

Git commit a18177cc24d20bca02f60a95b67f2dcbd1ee8afc by Martin Gräßlin.
Committed on 11/01/2016 at 11:53.
Pushed by graesslin into branch 'Plasma/5.5'.

[backends/drm] Set mode when changing from/to a gbm buffer

Tiling configurations of DRM and GBM buffers differ, so we need to
explicitly set the mode when it changes.
FIXED-IN: 5.5.4

M  +2    -1    backends/drm/drm_backend.cpp
M  +4    -0    backends/drm/drm_backend.h

http://commits.kde.org/kwin/a18177cc24d20bca02f60a95b67f2dcbd1ee8afc