Development/picky gpus (#929)

* Compositor: Only set the modifiers of the used planes

* Compositor: Extend docs with how to enable debug in the gpu driver

* Compositor: tweaked the tracing a bit

* Compositor: replaced (f)printf by tracing

* Update GBM.cpp

---------

Co-authored-by: Pierre Wielders <pierre@wielders.net>

Author: bramoosterhuis

Compositor/lib/Mesa/README.md

@@ -283,8 +283,46 @@ grep "VSync.*called" /var/log/thunder.log | tail -10
 # CPU usage tracking
 top -p $(pidof Thunder)
 ```
+## DRM Debug Commands
 
-### Debug Commands
+### Enable Debug Logging
+
+#### Check and Mount debugfs
+
+```bash
+# Check if debugfs is mounted
+mount | grep debugfs
+
+# Mount debugfs (if not already mounted)
+sudo mount -t debugfs none /sys/kernel/debug
+
+# Make it persistent across reboots (optional)
+echo "debugfs /sys/kernel/debug debugfs defaults 0 0" | sudo tee -a /etc/fstab
+```
+
+#### Enable DRM Kernel Debug Output
+
+```bash
+# Enable DRM debug logging
+# Debug levels:
+# 0x01 - Core DRM
+# 0x02 - Driver-specific
+# 0x04 - KMS (mode setting)
+# 0x08 - Prime (buffer sharing)
+# 0x10 - Atomic
+# 0x1f - All common categories
+echo 0x1f | sudo tee /sys/module/drm/parameters/debug
+
+# Watch kernel logs in real-time
+sudo dmesg -wH
+
+# Disable debug logging when done
+echo 0 | sudo tee /sys/module/drm/parameters/debug
+```
+
+### DRM Information Commands
+
+#### Basic DRM Information
 
 ```bash
 # DRM connector info
@@ -293,9 +331,74 @@ modetest -M <gpu> -c
 # Display timing info  
 modetest -M <gpu> -w <connector>:<property>:<value>
 
+# Show all planes and their capabilities (including format/modifier support)
+modetest -M <gpu> -p
+
+# List available DRM devices
+ls /sys/kernel/debug/dri/
+```
+
+#### Detailed State Information
+
+```bash
 # Buffer info
 cat /sys/kernel/debug/dri/0/framebuffer
+
+# Full DRM state (connectors, CRTCs, planes, properties)
+sudo cat /sys/kernel/debug/dri/0/state
+
+# Check specific VC4 info (Broadcom/Raspberry Pi)
+sudo cat /sys/kernel/debug/dri/0/vc4_*
+```
+
+### Example: Debug Framebuffer Creation Issues
+
+```bash
+# 1. Enable DRM debugging
+echo 0x1f | sudo tee /sys/module/drm/parameters/debug
+
+# 2. Run your application in another terminal
+
+# 3. Monitor kernel logs for DRM-specific messages
+sudo dmesg | grep -i drm
+
+# 4. Look for specific errors
+sudo dmesg | grep -i "modifier\|addfb\|framebuffer"
+
+# 5. Disable debugging when done
+echo 0 | sudo tee /sys/module/drm/parameters/debug
+```
+
+### Common Issues and Their Debug Output
+
+#### Modifier-related Issues
+
+When you see errors like:
+```
+non-zero modifier for unused plane
+```
+
+This indicates that modifiers are being set for unused planes. Only active planes should have non-zero modifiers.
+
+#### Format Not Supported
+
 ```
+unsupported pixel format
+```
+
+Use `modetest -M <gpu> -p` to check which formats and modifiers are supported by each plane.
+
+#### Invalid Argument (EINVAL)
+
+```
+ret=-22
+```
+
+Common causes:
+- Incorrect modifier for unused planes
+- Format/modifier combination not supported by hardware
+- Invalid buffer dimensions or stride
+
 
 ## 🤝 Contributing
 

Compositor/lib/Mesa/backend/src/DRM/Atomic.h

@@ -94,7 +94,7 @@ namespace Compositor {
                     ASSERT(_request != nullptr);
 
                     if (propertyId == DRM::InvalidIdentifier) {
-                        TRACE(Trace::Error, ("ObjectId[%u] property not found.", objectId));
+                        TRACE(Trace::Warning, ("ObjectId[%u] property not found.", objectId));
                     } else {
                         int presult(0);
 

Compositor/lib/Mesa/buffer/src/GBM/GBM.cpp

@@ -57,14 +57,14 @@ namespace {
         }
 
         if (fmt == NULL) {
-            fprintf(stderr, "unsupported format %" PRIu32 "\n", formatIn);
+            TRACE_GLOBAL(Trace::Error, ("unsupported format %" PRIu32, formatIn));
             return;
         }
 
         FILE* filePointer = fopen(filename.c_str(), "wb");
 
         if (filePointer == NULL) {
-            fprintf(stderr, "failed to open output file\n");
+            TRACE_GLOBAL(Trace::Error, ("Failed to open output file"));
             return;
         }
 

Compositor/lib/Mesa/drm/DRM.cpp

@@ -419,14 +419,12 @@ namespace Compositor {
 
             uint16_t nPlanes(0);
 
-            // buffer->Lock();
-
             std::array<uint32_t, 4> handles = { 0, 0, 0, 0 };
             std::array<uint32_t, 4> pitches = { 0, 0, 0, 0 };
             std::array<uint32_t, 4> offsets = { 0, 0, 0, 0 };
-            std::array<uint64_t, 4> modifiers;
+            std::array<uint64_t, 4> modifiers = { 0, 0, 0, 0 };
 
-            modifiers.fill(buffer->Modifier());
+            uint64_t bufferModifier = buffer->Modifier();
 
             Exchange::IGraphicsBuffer::IIterator* planes = buffer->Acquire(Compositor::DefaultTimeoutMs);
             ASSERT(planes != nullptr);
@@ -435,50 +433,71 @@ namespace Compositor {
                 ASSERT(planes->IsValid() == true);
 
                 if (drmPrimeFDToHandle(cardFd, planes->Descriptor(), &handles[nPlanes]) != 0) {
-                    TRACE_GLOBAL(Trace::Error, ("Failed to acquirer drm handle from plane accessor"));
+                    TRACE_GLOBAL(Trace::Error, ("Failed to acquire drm handle from plane accessor"));
                     CloseDrmHandles(cardFd, handles);
-                    break;
+                    buffer->Relinquish();
+                    return 0;
                 }
 
                 pitches[nPlanes] = planes->Stride();
                 offsets[nPlanes] = planes->Offset();
+                modifiers[nPlanes] = bufferModifier;
 
                 ++nPlanes;
             }
 
             buffer->Relinquish();
 
-            if (modifierSupported && buffer->Modifier() != DRM_FORMAT_MOD_INVALID) {
+            TRACE_GLOBAL(Trace::Information, ("Attempting FB creation: %ux%u, format=0x%x, modifier=0x%" PRIX64 ", nPlanes=%u", 
+                                 buffer->Width(), buffer->Height(), buffer->Format(), buffer->Modifier(), nPlanes));
 
-                if (drmModeAddFB2WithModifiers(cardFd, buffer->Width(), buffer->Height(), buffer->Format(), handles.data(), pitches.data(), offsets.data(), modifiers.data(), &framebufferId, DRM_MODE_FB_MODIFIERS) != 0) {
-                    TRACE_GLOBAL(Trace::Error, ("Failed to allocate drm framebuffer with modifiers"));
-                }
-            } else {
-                if (buffer->Modifier() != DRM_FORMAT_MOD_INVALID && buffer->Modifier() != DRM_FORMAT_MOD_LINEAR) {
-                    TRACE_GLOBAL(Trace::Error, ("Cannot import drm framebuffer with explicit modifier 0x%" PRIX64, buffer->Modifier()));
-                    return 0;
+            if (modifierSupported && buffer->Modifier() != DRM_FORMAT_MOD_INVALID) {
+                int ret = drmModeAddFB2WithModifiers(cardFd, buffer->Width(), buffer->Height(),
+                    buffer->Format(), handles.data(), pitches.data(),
+                    offsets.data(), modifiers.data(), &framebufferId,
+                    DRM_MODE_FB_MODIFIERS);
+
+                if (ret != 0) {
+                    TRACE_GLOBAL(Trace::Warning, ("Failed to allocate drm framebuffer with modifiers (error: %d, %s), falling back to drmModeAddFB2", ret, strerror(errno)));
+                    framebufferId = 0; // Reset in case it was partially set
+                } else {
+                    TRACE_GLOBAL(Trace::Information, ("Successfully created framebuffer with modifiers"));
+                    CloseDrmHandles(cardFd, handles);
+                    return framebufferId;
                 }
+            }
+
+            if (buffer->Modifier() != DRM_FORMAT_MOD_INVALID && buffer->Modifier() != DRM_FORMAT_MOD_LINEAR) {
+                TRACE_GLOBAL(Trace::Warning, ("Cannot import drm framebuffer with explicit modifier 0x%" PRIX64 " without modifier support, trying anyway", buffer->Modifier()));
+            }
 
-                int ret = drmModeAddFB2(cardFd, buffer->Width(), buffer->Height(), buffer->Format(), handles.data(), pitches.data(), offsets.data(), &framebufferId, 0);
+            int ret = drmModeAddFB2(cardFd, buffer->Width(), buffer->Height(), buffer->Format(), handles.data(), pitches.data(), offsets.data(), &framebufferId, 0);
 
-                if (ret != 0 && buffer->Format() == DRM_FORMAT_ARGB8888 /*&& nPlanes == 1*/ && offsets[0] == 0) {
-                    TRACE_GLOBAL(Trace::Error, ("Failed to allocate drm framebuffer (%s), falling back to old school drmModeAddFB", strerror(-ret)));
+            if (ret != 0) {
+                TRACE_GLOBAL(Trace::Warning, ("Failed to allocate drm framebuffer with drmModeAddFB2 (%s), trying legacy drmModeAddFB", strerror(-ret)));
 
+                // Last resort: old-school drmModeAddFB (only works for single-plane ARGB8888)
+                if (buffer->Format() == DRM_FORMAT_ARGB8888 && nPlanes == 1 && offsets[0] == 0) {
                     uint32_t depth = 32;
                     uint32_t bpp = 32;
 
                     if (drmModeAddFB(cardFd, buffer->Width(), buffer->Height(), depth, bpp, pitches[0], handles[0], &framebufferId) != 0) {
-                        TRACE_GLOBAL(Trace::Error, ("Failed to allocate a drm framebuffer the old school way..."));
+                        TRACE_GLOBAL(Trace::Error, ("Failed to allocate a drm framebuffer with legacy method"));
+                    } else {
+                        TRACE_GLOBAL(Trace::Information, ("Successfully created framebuffer with legacy method"));
                     }
-
-                } else if (ret != 0) {
-                    TRACE_GLOBAL(Trace::Error, ("Failed to allocate a drm framebuffer..."));
+                } else {
+                    TRACE_GLOBAL(Trace::Error, ("Failed to allocate a drm framebuffer with all methods"));
                 }
+            } else {
+                TRACE_GLOBAL(Trace::Information, ("Successfully created framebuffer without modifiers"));
             }
 
             CloseDrmHandles(cardFd, handles);
 
-            TRACE_GLOBAL(Trace::Information, ("DRM framebuffer object %u allocated for buffer %p", framebufferId, buffer));
+            if (framebufferId > 0) {
+                TRACE_GLOBAL(Trace::Information, ("DRM framebuffer object %u allocated for buffer %p", framebufferId, buffer));
+            }
 
             return framebufferId;
         }

Compositor/lib/Mesa/renderer/test/BaseTest.h

@@ -22,7 +22,6 @@ class BaseTest {
 
         virtual void Presented(const Compositor::IOutput* output, const uint64_t sequence, const uint64_t time) override
         {
-            std::cerr << "Presented!" << std::endl;
             _parent.HandleVSync(output, sequence, time);
         }
 

Compositor/lib/Mesa/renderer/test/testclear.cpp

@@ -202,9 +202,9 @@ int main(int argc, char* argv[])
         const std::map<std::string, std::vector<std::string>> modules = {
             { "CompositorRenderTest", { "" } },
             { "CompositorBuffer", { "Error", "Information" } },
-            { "CompositorBackend", { "Error", "Information" } },
+            { "CompositorBackend", { "Error" } },
             { "CompositorRenderer", { "Error", "Warning", "Information" } },
-            { "DRMCommon", { "Error", "Information" } }
+            { "DRMCommon", { "Error", "Warning", "Information" } }
         };
 
         for (const auto& module_entry : modules) {

Compositor/lib/Mesa/renderer/test/testdmabuf.cpp

@@ -282,9 +282,9 @@ int main(int argc, char* argv[])
         const std::map<std::string, std::vector<std::string>> modules = {
             { "CompositorRenderTest", { "" } },
             { "CompositorBuffer", { "Error", "Information" } },
-            { "CompositorBackend", { "Error", "Information" } },
+            { "CompositorBackend", { "Error" } },
             { "CompositorRenderer", { "Error", "Warning", "Information" } },
-            { "DRMCommon", { "Error", "Information" } }
+            { "DRMCommon", { "Error", "Warning", "Information" } }
         };
 
         for (const auto& module_entry : modules) {

Compositor/lib/Mesa/renderer/test/testquads.cpp

@@ -191,9 +191,9 @@ int main(int argc, char* argv[])
         const std::map<std::string, std::vector<std::string>> modules = {
             { "CompositorRenderTest", { "" } },
             { "CompositorBuffer", { "Error", "Information" } },
-            { "CompositorBackend", { "Error", "Information" } },
+            { "CompositorBackend", { "Error" } },
             { "CompositorRenderer", { "Error", "Warning", "Information" } },
-            { "DRMCommon", { "Error", "Information" } }
+            { "DRMCommon", { "Error", "Warning", "Information" } }
         };
 
         for (const auto& module_entry : modules) {

Compositor/lib/Mesa/renderer/test/testtexture.cpp

@@ -209,9 +209,9 @@ int main(int argc, char* argv[])
         const std::map<std::string, std::vector<std::string>> modules = {
             { "CompositorRenderTest", { "" } },
             { "CompositorBuffer", { "Error", "Information" } },
-            { "CompositorBackend", { "Error", "Information" } },
+            { "CompositorBackend", { "Error" } },
             { "CompositorRenderer", { "Error", "Warning", "Information" } },
-            { "DRMCommon", { "Error", "Information" } }
+            { "DRMCommon", { "Error", "Warning", "Information" } }
         };
 
         for (const auto& module_entry : modules) {

Compositor/lib/Mesa/renderer/test/texturebuffer/DmaBuffer.h

@@ -133,7 +133,7 @@ namespace Compositor {
                 &_modifiers);
 
             if (!queried) {
-                printf("Failed to query EGL Image.\n");
+                TRACE(Trace::Error, ("Failed to query EGL Image"));
             }
 
             ASSERT(queried);
@@ -146,12 +146,12 @@ namespace Compositor {
                 &_offset);
 
             if (!exported) {
-                printf("Failed to export EGL Image to a DMA buffer.\n");
+                TRACE(Trace::Error, ("Failed to export EGL Image to a DMA buffer"));
             }
 
             ASSERT(exported);
 
-            printf("DMA buffer available on descriptor %d.\n", _id);
+            TRACE(Trace::Information, ("DMA buffer available on descriptor %d.", _id));
 
             glBindTexture(_target, 0);
         }