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 <[email protected]>
> Reviewed-by: Gurchetan Singh <[email protected]>
> Commit-Queue: David Stevens <[email protected]>
Bug: b:145994510
Change-Id: I50079b7f0aabf38e1f373cac0f28c0e057eed760
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/minigbm/+/2093923
Commit-Queue: Ilja H. Friedel <[email protected]>
Tested-by: Ilja H. Friedel <[email protected]>
Reviewed-by: Ilja H. Friedel <[email protected]>
diff --git a/drv.c b/drv.c
index e5e0be6..dcca838 100644
--- a/drv.c
+++ b/drv.c
@@ -114,21 +114,11 @@
&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 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 @@
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 @@
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 @@
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 @@
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 *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 @@
/* 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 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 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
--- a/drv.h
+++ b/drv.h
@@ -12,7 +12,6 @@
#endif
#include <drm_fourcc.h>
-#include <stdbool.h>
#include <stdint.h>
#define DRV_MAX_PLANES 4
@@ -36,8 +35,7 @@
#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 @@
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
--- 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 {
struct driver *drv;
struct bo_metadata meta;
- bool is_test_buffer;
union bo_handle handles[DRV_MAX_PLANES];
void *priv;
};
@@ -67,11 +65,6 @@
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
--- a/gbm.h
+++ b/gbm.h
@@ -271,15 +271,6 @@
* 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
--- a/i915.c
+++ b/i915.c
@@ -268,26 +268,13 @@
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 @@
} 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 @@
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 @@
.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,
