Revert "minigbm: introduce test allocation"

This reverts commit f0e607c7d436134b53fd45a4ead36d714451be8a.

Reason for revert: caused flaky regressions across the board.

BUG=b:150997559
Exempt-From-Owner-Approval: revert.

Original change's description:
> minigbm: introduce test allocation
>
> This change introduces a GBM_TEST_ALLOC flag to minigbm, which allows
> for the creation of fake buffers that can be used to determine buffer
> metadata without actually allocating a full buffer. The new flag is
> supported by the i915 backends. This flag also alleviates the need to
> cache buffers when virtio_gpu queries metadata properties.
>
> BUG=b:145994510
> TEST=play youtube with arcvm demo image plus this and virgl change
>
> Change-Id: I9c6819aa3b5b674e4bb33b0656f2a9f155b0884e
> Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/minigbm/+/1980688
> Tested-by: David Stevens <stevensd@chromium.org>
> Reviewed-by: Gurchetan Singh <gurchetansingh@chromium.org>
> Commit-Queue: David Stevens <stevensd@chromium.org>

Bug: b:145994510
Change-Id: I50079b7f0aabf38e1f373cac0f28c0e057eed760
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/minigbm/+/2093923
Commit-Queue: Ilja H. Friedel <ihf@chromium.org>
Tested-by: Ilja H. Friedel <ihf@chromium.org>
Reviewed-by: Ilja H. Friedel <ihf@chromium.org>

diff --git a/drv.c b/drv.c
index e5e0be6..dcca838 100644 (file)
--- a/drv.c
+++ b/drv.c
@@ -114,21 +114,11 @@ static const struct backend *drv_get_backend(int fd)
                &backend_vgem,     &backend_virtio_gpu,
        };
 
-       for (i = 0; i < ARRAY_SIZE(backend_list); i++) {
-               const struct backend *b = backend_list[i];
-               // Exactly one of the main create functions must be defined.
-               assert((b->bo_create != NULL) ^ (b->bo_create_from_metadata != NULL));
-               // Either both or neither must be implemented.
-               assert((b->bo_compute_metadata != NULL) == (b->bo_create_from_metadata != NULL));
-               // Both can't be defined, but it's okay for neither to be (i.e. only bo_create).
-               assert((b->bo_create_with_modifiers == NULL) ||
-                      (b->bo_create_from_metadata == NULL));
-
-               if (!strcmp(drm_version->name, b->name)) {
+       for (i = 0; i < ARRAY_SIZE(backend_list); i++)
+               if (!strcmp(drm_version->name, backend_list[i]->name)) {
                        drmFreeVersion(drm_version);
-                       return b;
+                       return backend_list[i];
                }
-       }
 
        drmFreeVersion(drm_version);
        return NULL;
@@ -233,7 +223,7 @@ struct combination *drv_get_combination(struct driver *drv, uint32_t format, uin
 }
 
 struct bo *drv_bo_new(struct driver *drv, uint32_t width, uint32_t height, uint32_t format,
-                     uint64_t use_flags, bool is_test_buffer)
+                     uint64_t use_flags)
 {
 
        struct bo *bo;
@@ -248,7 +238,6 @@ struct bo *drv_bo_new(struct driver *drv, uint32_t width, uint32_t height, uint3
        bo->meta.format = format;
        bo->meta.use_flags = use_flags;
        bo->meta.num_planes = drv_num_planes_from_format(format);
-       bo->is_test_buffer = is_test_buffer;
 
        if (!bo->meta.num_planes) {
                free(bo);
@@ -264,28 +253,13 @@ struct bo *drv_bo_create(struct driver *drv, uint32_t width, uint32_t height, ui
        int ret;
        size_t plane;
        struct bo *bo;
-       bool is_test_alloc;
-
-       is_test_alloc = use_flags & BO_USE_TEST_ALLOC;
-       use_flags &= ~BO_USE_TEST_ALLOC;
 
-       bo = drv_bo_new(drv, width, height, format, use_flags, is_test_alloc);
+       bo = drv_bo_new(drv, width, height, format, use_flags);
 
        if (!bo)
                return NULL;
 
-       ret = -EINVAL;
-       if (drv->backend->bo_compute_metadata) {
-               ret = drv->backend->bo_compute_metadata(bo, width, height, format, use_flags, NULL,
-                                                       0);
-               if (!is_test_alloc && ret == 0) {
-                       ret = drv->backend->bo_create_from_metadata(bo);
-                       if (ret == 0)
-                               return bo;
-               }
-       } else if (!is_test_alloc) {
-               ret = drv->backend->bo_create(bo, width, height, format, use_flags);
-       }
+       ret = drv->backend->bo_create(bo, width, height, format, use_flags);
 
        if (ret) {
                free(bo);
@@ -313,26 +287,17 @@ struct bo *drv_bo_create_with_modifiers(struct driver *drv, uint32_t width, uint
        size_t plane;
        struct bo *bo;
 
-       if (!drv->backend->bo_create_with_modifiers && !drv->backend->bo_compute_metadata) {
+       if (!drv->backend->bo_create_with_modifiers) {
                errno = ENOENT;
                return NULL;
        }
 
-       bo = drv_bo_new(drv, width, height, format, BO_USE_NONE, false);
+       bo = drv_bo_new(drv, width, height, format, BO_USE_NONE);
 
        if (!bo)
                return NULL;
 
-       ret = -EINVAL;
-       if (drv->backend->bo_compute_metadata) {
-               ret = drv->backend->bo_compute_metadata(bo, width, height, format, BO_USE_NONE,
-                                                       modifiers, count);
-               if (ret == 0)
-                       ret = drv->backend->bo_create_from_metadata(bo);
-       } else {
-               ret = drv->backend->bo_create_with_modifiers(bo, width, height, format, modifiers,
-                                                            count);
-       }
+       ret = drv->backend->bo_create_with_modifiers(bo, width, height, format, modifiers, count);
 
        if (ret) {
                free(bo);
@@ -360,22 +325,20 @@ void drv_bo_destroy(struct bo *bo)
        uintptr_t total = 0;
        struct driver *drv = bo->drv;
 
-       if (!bo->is_test_buffer) {
-               pthread_mutex_lock(&drv->driver_lock);
+       pthread_mutex_lock(&drv->driver_lock);
 
-               for (plane = 0; plane < bo->meta.num_planes; plane++)
-                       drv_decrement_reference_count(drv, bo, plane);
+       for (plane = 0; plane < bo->meta.num_planes; plane++)
+               drv_decrement_reference_count(drv, bo, plane);
 
-               for (plane = 0; plane < bo->meta.num_planes; plane++)
-                       total += drv_get_reference_count(drv, bo, plane);
+       for (plane = 0; plane < bo->meta.num_planes; plane++)
+               total += drv_get_reference_count(drv, bo, plane);
 
-               pthread_mutex_unlock(&drv->driver_lock);
+       pthread_mutex_unlock(&drv->driver_lock);
 
-               if (total == 0) {
-                       ret = drv_mapping_destroy(bo);
-                       assert(ret == 0);
-                       bo->drv->backend->bo_destroy(bo);
-               }
+       if (total == 0) {
+               ret = drv_mapping_destroy(bo);
+               assert(ret == 0);
+               bo->drv->backend->bo_destroy(bo);
        }
 
        free(bo);
@@ -388,7 +351,7 @@ struct bo *drv_bo_import(struct driver *drv, struct drv_import_fd_data *data)
        struct bo *bo;
        off_t seek_end;
 
-       bo = drv_bo_new(drv, data->width, data->height, data->format, data->use_flags, false);
+       bo = drv_bo_new(drv, data->width, data->height, data->format, data->use_flags);
 
        if (!bo)
                return NULL;
@@ -452,10 +415,6 @@ void *drv_bo_map(struct bo *bo, const struct rectangle *rect, uint32_t map_flags
        /* No CPU access for protected buffers. */
        assert(!(bo->meta.use_flags & BO_USE_PROTECTED));
 
-       if (bo->is_test_buffer) {
-               return MAP_FAILED;
-       }
-
        memset(&mapping, 0, sizeof(mapping));
        mapping.rect = *rect;
        mapping.refcount = 1;
@@ -603,10 +562,6 @@ int drv_bo_get_plane_fd(struct bo *bo, size_t plane)
        int ret, fd;
        assert(plane < bo->meta.num_planes);
 
-       if (bo->is_test_buffer) {
-               return -EINVAL;
-       }
-
        ret = drmPrimeHandleToFD(bo->drv->fd, bo->handles[plane].u32, DRM_CLOEXEC | DRM_RDWR, &fd);
 
        // Older DRM implementations blocked DRM_RDWR, but gave a read/write mapping anyways
@@ -658,10 +613,6 @@ uint32_t drv_num_buffers_per_bo(struct bo *bo)
        uint32_t count = 0;
        size_t plane, p;
 
-       if (bo->is_test_buffer) {
-               return 0;
-       }
-
        for (plane = 0; plane < bo->meta.num_planes; plane++) {
                for (p = 0; p < plane; p++)
                        if (bo->handles[p].u32 == bo->handles[plane].u32)

diff --git a/drv.h b/drv.h
index 937487b..39fd5dd 100644 (file)
--- a/drv.h
+++ b/drv.h
@@ -12,7 +12,6 @@ extern "C" {
 #endif
 
 #include <drm_fourcc.h>
-#include <stdbool.h>
 #include <stdint.h>
 
 #define DRV_MAX_PLANES 4
@@ -36,8 +35,7 @@ extern "C" {
 #define BO_USE_SW_WRITE_RARELY         (1ull << 12)
 #define BO_USE_HW_VIDEO_DECODER         (1ull << 13)
 #define BO_USE_HW_VIDEO_ENCODER         (1ull << 14)
-#define BO_USE_TEST_ALLOC              (1ull << 15)
-#define BO_USE_RENDERSCRIPT            (1ull << 16)
+#define BO_USE_RENDERSCRIPT            (1ull << 15)
 
 /* Quirks for allocating a buffer. */
 #define BO_QUIRK_NONE                  0
@@ -126,7 +124,7 @@ const char *drv_get_name(struct driver *drv);
 struct combination *drv_get_combination(struct driver *drv, uint32_t format, uint64_t use_flags);
 
 struct bo *drv_bo_new(struct driver *drv, uint32_t width, uint32_t height, uint32_t format,
-                     uint64_t use_flags, bool is_test_buffer);
+                     uint64_t use_flags);
 
 struct bo *drv_bo_create(struct driver *drv, uint32_t width, uint32_t height, uint32_t format,
                         uint64_t use_flags);

diff --git a/drv_priv.h b/drv_priv.h
index 31ab892..76fa443 100644 (file)
--- a/drv_priv.h
+++ b/drv_priv.h
@@ -8,7 +8,6 @@
 #define DRV_PRIV_H
 
 #include <pthread.h>
-#include <stdbool.h>
 #include <stdint.h>
 #include <stdlib.h>
 #include <sys/types.h>
@@ -32,7 +31,6 @@ struct bo_metadata {
 struct bo {
        struct driver *drv;
        struct bo_metadata meta;
-       bool is_test_buffer;
        union bo_handle handles[DRV_MAX_PLANES];
        void *priv;
 };
@@ -67,11 +65,6 @@ struct backend {
                         uint64_t use_flags);
        int (*bo_create_with_modifiers)(struct bo *bo, uint32_t width, uint32_t height,
                                        uint32_t format, const uint64_t *modifiers, uint32_t count);
-       // Either both or neither _metadata functions must be implemented.
-       // If the functions are implemented, bo_create and bo_create_with_modifiers must not be.
-       int (*bo_compute_metadata)(struct bo *bo, uint32_t width, uint32_t height, uint32_t format,
-                                  uint64_t use_flags, const uint64_t *modifiers, uint32_t count);
-       int (*bo_create_from_metadata)(struct bo *bo);
        int (*bo_destroy)(struct bo *bo);
        int (*bo_import)(struct bo *bo, struct drv_import_fd_data *data);
        void *(*bo_map)(struct bo *bo, struct vma *vma, size_t plane, uint32_t map_flags);

diff --git a/gbm.h b/gbm.h
index 2492728..4993b5c 100644 (file)
--- a/gbm.h
+++ b/gbm.h
@@ -271,15 +271,6 @@ enum gbm_bo_flags {
     * The buffer will be read by a video encode accelerator.
     */
    GBM_BO_USE_HW_VIDEO_ENCODER = (1 << 14),
-
-   /**
-    * If this flag is set, no backing memory will be allocated for the
-    * created buffer. The metadata of the buffer (e.g. size) can be
-    * queried, and the values will be equal to a buffer allocated with
-    * the same same arguments minus this flag. However, any methods
-    * which would otherwise access the underlying buffer will fail.
-    */
-   GBM_TEST_ALLOC = (1 << 15),
 };
 
 int

diff --git a/i915.c b/i915.c
index 05c8272..d0c9db1 100644 (file)
--- a/i915.c
+++ b/i915.c
@@ -268,26 +268,13 @@ static int i915_bo_from_format(struct bo *bo, uint32_t width, uint32_t height, u
        return 0;
 }
 
-static int i915_bo_compute_metadata(struct bo *bo, uint32_t width, uint32_t height, uint32_t format,
-                                   uint64_t use_flags, const uint64_t *modifiers, uint32_t count)
+static int i915_bo_create_for_modifier(struct bo *bo, uint32_t width, uint32_t height,
+                                      uint32_t format, uint64_t modifier)
 {
-       static const uint64_t modifier_order[] = {
-               I915_FORMAT_MOD_Y_TILED_CCS,
-               I915_FORMAT_MOD_Y_TILED,
-               I915_FORMAT_MOD_X_TILED,
-               DRM_FORMAT_MOD_LINEAR,
-       };
-       uint64_t modifier;
-
-       if (modifiers) {
-               modifier =
-                   drv_pick_modifier(modifiers, count, modifier_order, ARRAY_SIZE(modifier_order));
-       } else {
-               struct combination *combo = drv_get_combination(bo->drv, format, use_flags);
-               if (!combo)
-                       return -EINVAL;
-               modifier = combo->metadata.modifier;
-       }
+       int ret;
+       size_t plane;
+       struct drm_i915_gem_create gem_create;
+       struct drm_i915_gem_set_tiling gem_set_tiling;
 
        switch (modifier) {
        case DRM_FORMAT_MOD_LINEAR:
@@ -359,15 +346,6 @@ static int i915_bo_compute_metadata(struct bo *bo, uint32_t width, uint32_t heig
        } else {
                i915_bo_from_format(bo, width, height, format);
        }
-       return 0;
-}
-
-static int i915_bo_create_from_metadata(struct bo *bo)
-{
-       int ret;
-       size_t plane;
-       struct drm_i915_gem_create gem_create;
-       struct drm_i915_gem_set_tiling gem_set_tiling;
 
        memset(&gem_create, 0, sizeof(gem_create));
        gem_create.size = bo->meta.total_size;
@@ -400,6 +378,34 @@ static int i915_bo_create_from_metadata(struct bo *bo)
        return 0;
 }
 
+static int i915_bo_create(struct bo *bo, uint32_t width, uint32_t height, uint32_t format,
+                         uint64_t use_flags)
+{
+       struct combination *combo;
+
+       combo = drv_get_combination(bo->drv, format, use_flags);
+       if (!combo)
+               return -EINVAL;
+
+       return i915_bo_create_for_modifier(bo, width, height, format, combo->metadata.modifier);
+}
+
+static int i915_bo_create_with_modifiers(struct bo *bo, uint32_t width, uint32_t height,
+                                        uint32_t format, const uint64_t *modifiers, uint32_t count)
+{
+       static const uint64_t modifier_order[] = {
+               I915_FORMAT_MOD_Y_TILED_CCS,
+               I915_FORMAT_MOD_Y_TILED,
+               I915_FORMAT_MOD_X_TILED,
+               DRM_FORMAT_MOD_LINEAR,
+       };
+       uint64_t modifier;
+
+       modifier = drv_pick_modifier(modifiers, count, modifier_order, ARRAY_SIZE(modifier_order));
+
+       return i915_bo_create_for_modifier(bo, width, height, format, modifier);
+}
+
 static void i915_close(struct driver *drv)
 {
        free(drv->priv);
@@ -555,8 +561,8 @@ const struct backend backend_i915 = {
        .name = "i915",
        .init = i915_init,
        .close = i915_close,
-       .bo_compute_metadata = i915_bo_compute_metadata,
-       .bo_create_from_metadata = i915_bo_create_from_metadata,
+       .bo_create = i915_bo_create,
+       .bo_create_with_modifiers = i915_bo_create_with_modifiers,
        .bo_destroy = drv_gem_bo_destroy,
        .bo_import = i915_bo_import,
        .bo_map = i915_bo_map,