Revert "minigbm: introduce test allocation"

This reverts commit f0e607c7d4.

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>
This commit is contained in:
Ilja H. Friedel 2020-03-08 04:48:13 +00:00 committed by Commit Bot
parent c5352e6b36
commit 08d8dbf094
5 changed files with 58 additions and 119 deletions

89
drv.c
View file

@ -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)

6
drv.h
View file

@ -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);

View file

@ -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);

9
gbm.h
View file

@ -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

66
i915.c
View file

@ -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,