gbm/dri: Check dri extension version before flush after unmap

[mesa.git] / src / gbm / backends / dri / gbm_dri.c

diff --git a/src/gbm/backends/dri/gbm_dri.c b/src/gbm/backends/dri/gbm_dri.c
index a7ac1493658f7067bcee55a656a4cf8646ecd9d9..8df6a6b64d0b44bf77da468a41b15e0bd0dc3e59 100644 (file)
--- a/src/gbm/backends/dri/gbm_dri.c
+++ b/src/gbm/backends/dri/gbm_dri.c
@@ -710,22 +710,23 @@ gbm_dri_bo_get_stride(struct gbm_bo *_bo, int plane)
    return (uint32_t)stride;
 }
 
-static int64_t
+static uint32_t
 gbm_dri_bo_get_offset(struct gbm_bo *_bo, int plane)
 {
    struct gbm_dri_device *dri = gbm_dri_device(_bo->gbm);
    struct gbm_dri_bo *bo = gbm_dri_bo(_bo);
    int offset = 0;
 
-   if (!dri->image || dri->image->base.version < 13 || !dri->image->fromPlanar) {
-      errno = ENOSYS;
-      return -1;
-   }
+   /* These error cases do not actually return an error code, as the user
+    * will also fail to obtain the handle/FD from the BO. In that case, the
+    * offset is irrelevant, as they have no buffer to offset into, so
+    * returning 0 is harmless.
+    */
+   if (!dri->image || dri->image->base.version < 13 || !dri->image->fromPlanar)
+      return 0;
 
-   if (plane >= get_number_planes(dri, bo->image)) {
-      errno = EINVAL;
-      return -2;
-   }
+   if (plane >= get_number_planes(dri, bo->image))
+      return 0;
 
     /* Dumb images have no offset */
    if (bo->image == NULL) {
@@ -1143,12 +1144,29 @@ gbm_dri_bo_create(struct gbm_device *gbm,
          goto failed;
       }
 
+      /* It's acceptable to create an image with INVALID modifier in the list,
+       * but it cannot be on the only modifier (since it will certainly fail
+       * later). While we could easily catch this after modifier creation, doing
+       * the check here is a convenient debug check likely pointing at whatever
+       * interface the client is using to build its modifier list.
+       */
+      if (count == 1 && modifiers[0] == DRM_FORMAT_MOD_INVALID) {
+         fprintf(stderr, "Only invalid modifier specified\n");
+         errno = EINVAL;
+         goto failed;
+      }
+
       bo->image =
          dri->image->createImageWithModifiers(dri->screen,
                                               width, height,
                                               dri_format,
                                               modifiers, count,
                                               bo);
+
+      if (bo->image) {
+         /* The client passed in a list of invalid modifiers */
+         assert(gbm_dri_bo_get_modifier(&bo->base.base) != DRM_FORMAT_MOD_INVALID);
+      }
    } else {
       bo->image = dri->image->createImage(dri->screen, width, height,
                                           dri_format, dri_use, bo);
@@ -1221,6 +1239,14 @@ gbm_dri_bo_unmap(struct gbm_bo *_bo, void *map_data)
       return;
 
    dri->image->unmapImage(dri->context, bo->image, map_data);
+
+   /*
+    * Not all DRI drivers use direct maps. They may queue up DMA operations
+    * on the mapping context. Since there is no explicit gbm flush
+    * mechanism, we need to flush here.
+    */
+   if (dri->flush->base.version >= 4)
+      dri->flush->flush_with_flags(dri->context, NULL, __DRI2_FLUSH_CONTEXT, 0);
 }
 
 
@@ -1240,6 +1266,17 @@ gbm_dri_surface_create(struct gbm_device *gbm,
       return NULL;
    }
 
+   /* It's acceptable to create an image with INVALID modifier in the list,
+    * but it cannot be on the only modifier (since it will certainly fail
+    * later). While we could easily catch this after modifier creation, doing
+    * the check here is a convenient debug check likely pointing at whatever
+    * interface the client is using to build its modifier list.
+    */
+   if (count == 1 && modifiers[0] == DRM_FORMAT_MOD_INVALID) {
+      fprintf(stderr, "Only invalid modifier specified\n");
+      errno = EINVAL;
+   }
+
    surf = calloc(1, sizeof *surf);
    if (surf == NULL) {
       errno = ENOMEM;
@@ -1263,6 +1300,10 @@ gbm_dri_surface_create(struct gbm_device *gbm,
       return NULL;
    }
 
+   /* TODO: We are deferring validation of modifiers until the image is actually
+    * created. This deferred creation can fail due to a modifier-format
+    * mismatch. The result is the client has a surface but no object to back it.
+    */
    surf->base.count = count;
    memcpy(surf->base.modifiers, modifiers, count * sizeof(*modifiers));