fix multiplanar framebuffers

src/egl_gbm_render_surface.c

@@ -60,6 +60,7 @@ struct egl_gbm_render_surface {
     struct locked_fb *locked_front_fb;
 #ifdef DEBUG
     atomic_int n_locked_fbs;
+    bool logged_format_and_modifier;
 #endif
 };
 
@@ -241,6 +242,10 @@ static int egl_gbm_render_surface_init(
         s->locked_fbs->is_locked = (atomic_flag) ATOMIC_FLAG_INIT;
     }
     s->locked_front_fb = NULL;
+#ifdef DEBUG
+    s->n_locked_fbs = 0;
+    s->logged_format_and_modifier = false;
+#endif
     return 0;
 
 fail_destroy_egl_surface:
@@ -403,16 +408,10 @@ static int egl_gbm_render_surface_present_kms(struct surface *s, const struct fl
         ASSERT_NOT_NULL(drmdev);
 
         TRACER_BEGIN(egl_surface->surface.tracer, "drmdev_add_fb (non-opaque)");
-        fb_id = drmdev_add_fb(
+        fb_id = drmdev_add_fb_from_gbm_bo(
             drmdev,
-            gbm_bo_get_width(bo),
-            gbm_bo_get_height(bo),
-            egl_surface->pixel_format,
-            gbm_bo_get_handle(bo).u32,
-            gbm_bo_get_stride(bo),
-            gbm_bo_get_offset(bo, 0),
-            gbm_bo_get_modifier(bo) != DRM_FORMAT_MOD_INVALID,
-            gbm_bo_get_modifier(bo)
+            bo,
+            /* cast_opaque */ false
         );
         TRACER_END(egl_surface->surface.tracer, "drmdev_add_fb (non-opaque)");
 
@@ -425,16 +424,10 @@ static int egl_gbm_render_surface_present_kms(struct surface *s, const struct fl
         // if this EGL surface is non-opaque and has an opaque equivalent
         if (!get_pixfmt_info(egl_surface->pixel_format)->is_opaque &&
             pixfmt_opaque(egl_surface->pixel_format) != egl_surface->pixel_format) {
-            opaque_fb_id = drmdev_add_fb(
+            opaque_fb_id = drmdev_add_fb_from_gbm_bo(
                 drmdev,
-                gbm_bo_get_width(bo),
-                gbm_bo_get_height(bo),
-                pixfmt_opaque(egl_surface->pixel_format),
-                gbm_bo_get_handle(bo).u32,
-                gbm_bo_get_stride(bo),
-                gbm_bo_get_offset(bo, 0),
-                gbm_bo_get_modifier(bo) != DRM_FORMAT_MOD_INVALID,
-                gbm_bo_get_modifier(bo)
+                bo,
+                /* cast_opaque */ true
             );
             if (opaque_fb_id == 0) {
                 ok = EIO;
@@ -610,6 +603,25 @@ static int egl_gbm_render_surface_queue_present(struct render_surface *s, const
     bo = gbm_surface_lock_front_buffer(egl_surface->gbm_surface);
     TRACER_END(s->surface.tracer, "gbm_surface_lock_front_buffer");
 
+#ifdef DEBUG
+    if (!egl_surface->logged_format_and_modifier) {
+        uint32_t fourcc = gbm_bo_get_format(bo);
+        uint64_t modifier = gbm_bo_get_modifier(bo);
+
+        bool has_format = has_pixfmt_for_gbm_format(fourcc);
+        enum pixfmt format = has_format ? get_pixfmt_for_gbm_format(fourcc) : PIXFMT_RGB565;
+
+        LOG_DEBUG(
+            "using fourcc %c%c%c%c (%s) with modifier 0x%"PRIx64"\n",
+            fourcc & 0xFF, (fourcc >> 8) & 0xFF, (fourcc >> 16) & 0xFF, (fourcc >> 24) & 0xFF,
+            has_format ? get_pixfmt_info(format)->name : "?",
+            modifier
+        );
+
+        egl_surface->logged_format_and_modifier = true;
+    }
+#endif
+
     if (bo == NULL) {
         ok = errno;
         LOG_ERROR("Couldn't lock GBM front buffer. gbm_surface_lock_front_buffer: %s\n", strerror(ok));

src/modesetting.c

@@ -1620,6 +1620,106 @@ uint32_t drmdev_add_fb_from_dmabuf_multiplanar(
     return fb;
 }
 
+uint32_t drmdev_add_fb_from_gbm_bo_locked(
+    struct drmdev *drmdev,
+    struct gbm_bo *bo,
+    bool cast_opaque
+) {
+    enum pixfmt format;
+    uint32_t fourcc;
+    int n_planes;
+
+    n_planes = gbm_bo_get_plane_count(bo);
+    ASSERT(0 <= n_planes && n_planes <= 4);
+
+    fourcc = gbm_bo_get_format(bo);
+
+    if (!has_pixfmt_for_gbm_format(fourcc)) {
+        LOG_ERROR("GBM pixel format is not supported.\n");
+        return 0;
+    }
+
+    format = get_pixfmt_for_gbm_format(fourcc);
+
+    if (cast_opaque) {
+        format = pixfmt_opaque(format);
+    }
+
+    uint32_t handles[4];
+    uint32_t pitches[4];
+
+    // Returns DRM_FORMAT_MOD_INVALID on failure, or DRM_FORMAT_MOD_LINEAR
+    // for dumb buffers.
+    uint64_t modifier = gbm_bo_get_modifier(bo);
+    bool has_modifiers = modifier != DRM_FORMAT_MOD_INVALID;
+
+    for (int i = 0; i < n_planes; i++) {
+        // gbm_bo_get_handle_for_plane will return -1 (in gbm_bo_handle.s32) and
+        // set errno on failure.
+        errno = 0;
+        union gbm_bo_handle handle = gbm_bo_get_handle_for_plane(bo, i);
+        if (handle.s32 == -1) {
+            LOG_ERROR("Could not get GEM handle for plane %d: %s\n", i, strerror(errno));
+            return 0;
+        }
+
+        handles[i] = handle.u32;
+
+        // gbm_bo_get_stride_for_plane will return 0 and set errno on failure.
+        errno = 0;
+        uint32_t pitch = gbm_bo_get_stride_for_plane(bo, i);
+        if (pitch == 0 && errno != 0) {
+            LOG_ERROR("Could not get framebuffer stride for plane %d: %s\n", i, strerror(errno));
+            return 0;
+        }
+
+        pitches[i] = pitch;
+    }
+
+    for (int i = n_planes; i < 4; i++) {
+        handles[i] = 0;
+        pitches[i] = 0;
+    }
+
+    return drmdev_add_fb_multiplanar_locked(
+        drmdev,
+        gbm_bo_get_width(bo),
+        gbm_bo_get_height(bo),
+        format,
+        handles,
+        pitches,
+        (uint32_t[4]) {
+            n_planes >= 1 ? gbm_bo_get_offset(bo, 0) : 0,
+            n_planes >= 2 ? gbm_bo_get_offset(bo, 1) : 0,
+            n_planes >= 3 ? gbm_bo_get_offset(bo, 2) : 0,
+            n_planes >= 4 ? gbm_bo_get_offset(bo, 3) : 0,
+        },
+        has_modifiers,
+        (uint64_t[4]) {
+            n_planes >= 1 ? modifier : 0,
+            n_planes >= 2 ? modifier : 0,
+            n_planes >= 3 ? modifier : 0,
+            n_planes >= 4 ? modifier : 0,
+        }
+    );
+}
+
+uint32_t drmdev_add_fb_from_gbm_bo(
+    struct drmdev *drmdev,
+    struct gbm_bo *bo,
+    bool cast_opaque
+) {
+    uint32_t fb;
+
+    drmdev_lock(drmdev);
+
+    fb = drmdev_add_fb_from_gbm_bo_locked(drmdev, bo, cast_opaque);
+
+    drmdev_unlock(drmdev);
+
+    return fb;
+}
+
 int drmdev_rm_fb_locked(struct drmdev *drmdev, uint32_t fb_id) {
     int ok;
 

src/modesetting.h

@@ -686,6 +686,12 @@ uint32_t drmdev_add_fb_from_dmabuf_multiplanar(
     const uint64_t modifiers[4]
 );
 
+uint32_t drmdev_add_fb_from_gbm_bo(
+    struct drmdev *drmdev,
+    struct gbm_bo *bo,
+    bool cast_opaque
+);
+
 int drmdev_rm_fb_locked(struct drmdev *drmdev, uint32_t fb_id);
 
 int drmdev_rm_fb(struct drmdev *drmdev, uint32_t fb_id);

src/pixel_format.h

@@ -395,6 +395,27 @@ ATTR_CONST static inline enum pixfmt get_pixfmt_for_drm_format(uint32_t fourcc)
     return PIXFMT_RGB565;
 }
 
+ATTR_CONST static inline bool has_pixfmt_for_gbm_format(uint32_t fourcc) {
+    for (int i = 0; i < n_pixfmt_infos; i++) {
+        if (get_pixfmt_info(i)->gbm_format == fourcc) {
+            return true;
+        }
+    }
+
+    return false;
+}
+
+ATTR_CONST static inline enum pixfmt get_pixfmt_for_gbm_format(uint32_t fourcc) {
+    for (int i = 0; i < n_pixfmt_infos; i++) {
+        if (get_pixfmt_info(i)->gbm_format == fourcc) {
+            return i;
+        }
+    }
+
+    ASSERT_MSG(false, "Check has_pixfmt_for_gbm_format if an enum pixfmt exists for a specific GBM fourcc.");
+    return PIXFMT_RGB565;
+}
+
 COMPILE_ASSERT(PIXFMT_RGB565 == 0);
 
 #define ASSERT_PIXFMT_VALID(format) ASSERT_MSG(format >= PIXFMT_RGB565 && format <= PIXFMT_MAX, "Invalid pixel format")