Mesa (main): iris: Only use SET/GET_TILING when exporting/importing BOs
GitLab Mirror
gitlab-mirror at kemper.freedesktop.org
Mon Jun 7 18:49:17 UTC 2021
Module: Mesa
Branch: main
Commit: ab49063f449d7be71dbd0eebe4a7f967bbaede23
URL: http://cgit.freedesktop.org/mesa/mesa/commit/?id=ab49063f449d7be71dbd0eebe4a7f967bbaede23
Author: Kenneth Graunke <kenneth at whitecape.org>
Date: Wed Jun 2 15:21:35 2021 -0700
iris: Only use SET/GET_TILING when exporting/importing BOs
In the past, we tracked bo->tiling_mode and bo->stride, and used
GEM_{GET,SET}_TILING on all buffers we allocated. This made more sense
in the old days (long before iris even existed) when we used GTT maps to
detile resources. However, that support is now gone, and we never used
it in iris anyway. We don't need to do this in most cases anymore.
We are trying to deprecate these kernel APIs. They have many issues.
One is having a global tiling mode for a buffer when userspace may
want to suballocate multiple resources with different tiling modes
from the same object. Another is...what if processes want to interpret
the data differently, and hot-swap the tiling mode out from under
another process? Another is the fundamental race conditions. There
are many reasons not to use these APIs.
Unfortunately, there is still one case where it's used: when importing
and exporting DMABUFs, we have to communicate the tiling somehow. The
right way to do that is using modifiers, but those didn't always exist,
and aren't always enabled (maybe aren't even commonly enabled). So we
use GET/SET_TILING as a poor-man's IPC mechanism of sorts.
This patch stops calling those APIs in general but continues doing so
for imported/exported objects when we don't have modifiers.
We eliminate iris_bo_alloc_tiled entirely. There is now only one!
One small behavioral change snuck in: iris_memobj_create_from_handle
now aligns the virtual address to 64K rather than 1B when modifiers
aren't present. This should be harmless, and lets us delete a whole
bunch of code.
Reviewed-by: Tapani Pälli <tapani.palli at intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/11169>
---
src/gallium/drivers/iris/iris_bufmgr.c | 201 ++++++++-----------------------
src/gallium/drivers/iris/iris_bufmgr.h | 36 +-----
src/gallium/drivers/iris/iris_resource.c | 53 ++++----
3 files changed, 80 insertions(+), 210 deletions(-)
diff --git a/src/gallium/drivers/iris/iris_bufmgr.c b/src/gallium/drivers/iris/iris_bufmgr.c
index 65b2b4838ff..2e22dc8fd79 100644
--- a/src/gallium/drivers/iris/iris_bufmgr.c
+++ b/src/gallium/drivers/iris/iris_bufmgr.c
@@ -53,6 +53,7 @@
#include "dev/intel_debug.h"
#include "common/intel_gem.h"
#include "dev/intel_device_info.h"
+#include "isl/isl.h"
#include "main/macros.h"
#include "os/os_mman.h"
#include "util/debug.h"
@@ -199,9 +200,6 @@ static struct list_head global_bufmgr_list = {
.prev = &global_bufmgr_list,
};
-static int bo_set_tiling_internal(struct iris_bo *bo, uint32_t tiling_mode,
- uint32_t stride);
-
static void bo_free(struct iris_bo *bo);
static struct iris_bo *
@@ -501,8 +499,6 @@ alloc_fresh_bo(struct iris_bufmgr *bufmgr, uint64_t bo_size)
bo->bufmgr = bufmgr;
bo->size = bo_size;
bo->idle = true;
- bo->tiling_mode = I915_TILING_NONE;
- bo->stride = 0;
/* Calling set_domain() will allocate pages for the BO outside of the
* struct mutex lock in the kernel, which is more efficient than waiting
@@ -522,15 +518,13 @@ alloc_fresh_bo(struct iris_bufmgr *bufmgr, uint64_t bo_size)
return bo;
}
-static struct iris_bo *
-bo_alloc_internal(struct iris_bufmgr *bufmgr,
- const char *name,
- uint64_t size,
- uint32_t alignment,
- enum iris_memory_zone memzone,
- unsigned flags,
- uint32_t tiling_mode,
- uint32_t stride)
+struct iris_bo *
+iris_bo_alloc(struct iris_bufmgr *bufmgr,
+ const char *name,
+ uint64_t size,
+ uint32_t alignment,
+ enum iris_memory_zone memzone,
+ unsigned flags)
{
struct iris_bo *bo;
unsigned int page_size = getpagesize();
@@ -576,9 +570,6 @@ bo_alloc_internal(struct iris_bufmgr *bufmgr,
goto err_free;
}
- if (bo_set_tiling_internal(bo, tiling_mode, stride))
- goto err_free;
-
bo->name = name;
p_atomic_set(&bo->refcount, 1);
bo->reusable = bucket && bufmgr->bo_reuse;
@@ -618,28 +609,6 @@ err_free:
return NULL;
}
-struct iris_bo *
-iris_bo_alloc(struct iris_bufmgr *bufmgr,
- const char *name,
- uint64_t size,
- uint32_t alignment,
- enum iris_memory_zone memzone,
- unsigned flags)
-{
- return bo_alloc_internal(bufmgr, name, size, alignment, memzone,
- flags, I915_TILING_NONE, 0);
-}
-
-struct iris_bo *
-iris_bo_alloc_tiled(struct iris_bufmgr *bufmgr, const char *name,
- uint64_t size, uint32_t alignment,
- enum iris_memory_zone memzone,
- uint32_t tiling_mode, uint32_t pitch, unsigned flags)
-{
- return bo_alloc_internal(bufmgr, name, size, alignment, memzone,
- flags, tiling_mode, pitch);
-}
-
struct iris_bo *
iris_bo_create_userptr(struct iris_bufmgr *bufmgr, const char *name,
void *ptr, size_t size,
@@ -758,24 +727,11 @@ iris_bo_gem_create_from_name(struct iris_bufmgr *bufmgr,
_mesa_hash_table_insert(bufmgr->handle_table, &bo->gem_handle, bo);
_mesa_hash_table_insert(bufmgr->name_table, &bo->global_name, bo);
- struct drm_i915_gem_get_tiling get_tiling = { .handle = bo->gem_handle };
- ret = intel_ioctl(bufmgr->fd, DRM_IOCTL_I915_GEM_GET_TILING, &get_tiling);
- if (ret != 0)
- goto err_unref;
-
- bo->tiling_mode = get_tiling.tiling_mode;
-
- /* XXX stride is unknown */
DBG("bo_create_from_handle: %d (%s)\n", handle, bo->name);
out:
mtx_unlock(&bufmgr->lock);
return bo;
-
-err_unref:
- bo_free(bo);
- mtx_unlock(&bufmgr->lock);
- return NULL;
}
static void
@@ -1153,50 +1109,64 @@ iris_bufmgr_destroy(struct iris_bufmgr *bufmgr)
free(bufmgr);
}
-static int
-bo_set_tiling_internal(struct iris_bo *bo, uint32_t tiling_mode,
- uint32_t stride)
+int
+iris_gem_get_tiling(struct iris_bo *bo, uint32_t *tiling)
{
struct iris_bufmgr *bufmgr = bo->bufmgr;
- struct drm_i915_gem_set_tiling set_tiling;
- int ret;
- if (bo->global_name == 0 &&
- tiling_mode == bo->tiling_mode && stride == bo->stride)
+ if (!bufmgr->has_tiling_uapi) {
+ *tiling = I915_TILING_NONE;
return 0;
+ }
+
+ struct drm_i915_gem_get_tiling ti = { .handle = bo->gem_handle };
+ int ret = intel_ioctl(bufmgr->fd, DRM_IOCTL_I915_GEM_GET_TILING, &ti);
+
+ if (ret) {
+ DBG("gem_get_tiling failed for BO %u: %s\n",
+ bo->gem_handle, strerror(ret));
+ }
+
+ *tiling = ti.tiling_mode;
+
+ return ret;
+}
+
+int
+iris_gem_set_tiling(struct iris_bo *bo, const struct isl_surf *surf)
+{
+ struct iris_bufmgr *bufmgr = bo->bufmgr;
+ uint32_t tiling_mode = isl_tiling_to_i915_tiling(surf->tiling);
+ int ret;
/* If we can't do map_gtt, the set/get_tiling API isn't useful. And it's
* actually not supported by the kernel in those cases.
*/
- if (!bufmgr->has_tiling_uapi) {
- bo->tiling_mode = tiling_mode;
- bo->stride = stride;
+ if (!bufmgr->has_tiling_uapi)
return 0;
- }
- memset(&set_tiling, 0, sizeof(set_tiling));
+ /* GEM_SET_TILING is slightly broken and overwrites the input on the
+ * error path, so we have to open code intel_ioctl().
+ */
do {
- /* set_tiling is slightly broken and overwrites the
- * input on the error path, so we have to open code
- * drm_ioctl.
- */
- set_tiling.handle = bo->gem_handle;
- set_tiling.tiling_mode = tiling_mode;
- set_tiling.stride = stride;
-
+ struct drm_i915_gem_set_tiling set_tiling = {
+ .handle = bo->gem_handle,
+ .tiling_mode = tiling_mode,
+ .stride = surf->row_pitch_B,
+ };
ret = ioctl(bufmgr->fd, DRM_IOCTL_I915_GEM_SET_TILING, &set_tiling);
} while (ret == -1 && (errno == EINTR || errno == EAGAIN));
- if (ret == -1)
- return -errno;
- bo->tiling_mode = set_tiling.tiling_mode;
- bo->stride = set_tiling.stride;
- return 0;
+ if (ret) {
+ DBG("gem_set_tiling failed for BO %u: %s\n",
+ bo->gem_handle, strerror(ret));
+ }
+
+ return ret;
}
struct iris_bo *
-iris_bo_import_dmabuf(struct iris_bufmgr *bufmgr, int prime_fd,
- uint64_t modifier)
+iris_bo_import_dmabuf(struct iris_bufmgr *bufmgr, int prime_fd)
{
uint32_t handle;
struct iris_bo *bo;
@@ -1257,79 +1227,6 @@ iris_bo_import_dmabuf(struct iris_bufmgr *bufmgr, int prime_fd,
bo->gem_handle = handle;
_mesa_hash_table_insert(bufmgr->handle_table, &bo->gem_handle, bo);
- const struct isl_drm_modifier_info *mod_info =
- isl_drm_modifier_get_info(modifier);
- if (mod_info) {
- bo->tiling_mode = isl_tiling_to_i915_tiling(mod_info->tiling);
- } else if (bufmgr->has_tiling_uapi) {
- struct drm_i915_gem_get_tiling get_tiling = { .handle = bo->gem_handle };
- if (intel_ioctl(bufmgr->fd, DRM_IOCTL_I915_GEM_GET_TILING, &get_tiling))
- goto err;
- bo->tiling_mode = get_tiling.tiling_mode;
- } else {
- bo->tiling_mode = I915_TILING_NONE;
- }
-
-out:
- mtx_unlock(&bufmgr->lock);
- return bo;
-
-err:
- bo_free(bo);
- mtx_unlock(&bufmgr->lock);
- return NULL;
-}
-
-struct iris_bo *
-iris_bo_import_dmabuf_no_mods(struct iris_bufmgr *bufmgr,
- int prime_fd)
-{
- uint32_t handle;
- struct iris_bo *bo;
-
- mtx_lock(&bufmgr->lock);
- int ret = drmPrimeFDToHandle(bufmgr->fd, prime_fd, &handle);
- if (ret) {
- DBG("import_dmabuf: failed to obtain handle from fd: %s\n",
- strerror(errno));
- mtx_unlock(&bufmgr->lock);
- return NULL;
- }
-
- /*
- * See if the kernel has already returned this buffer to us. Just as
- * for named buffers, we must not create two bo's pointing at the same
- * kernel object
- */
- bo = find_and_ref_external_bo(bufmgr->handle_table, handle);
- if (bo)
- goto out;
-
- bo = bo_calloc();
- if (!bo)
- goto out;
-
- p_atomic_set(&bo->refcount, 1);
-
- /* Determine size of bo. The fd-to-handle ioctl really should
- * return the size, but it doesn't. If we have kernel 3.12 or
- * later, we can lseek on the prime fd to get the size. Older
- * kernels will just fail, in which case we fall back to the
- * provided (estimated or guess size). */
- ret = lseek(prime_fd, 0, SEEK_END);
- if (ret != -1)
- bo->size = ret;
-
- bo->bufmgr = bufmgr;
- bo->name = "prime";
- bo->reusable = false;
- bo->imported = true;
- bo->mmap_mode = IRIS_MMAP_WC;
- bo->kflags = EXEC_OBJECT_SUPPORTS_48B_ADDRESS | EXEC_OBJECT_PINNED;
- bo->gtt_offset = vma_alloc(bufmgr, IRIS_MEMZONE_OTHER, bo->size, 1);
- bo->gem_handle = handle;
- _mesa_hash_table_insert(bufmgr->handle_table, &bo->gem_handle, bo);
-
out:
mtx_unlock(&bufmgr->lock);
return bo;
diff --git a/src/gallium/drivers/iris/iris_bufmgr.h b/src/gallium/drivers/iris/iris_bufmgr.h
index 528712be602..9c0a243d7d2 100644
--- a/src/gallium/drivers/iris/iris_bufmgr.h
+++ b/src/gallium/drivers/iris/iris_bufmgr.h
@@ -37,6 +37,7 @@
struct iris_batch;
struct intel_device_info;
struct pipe_debug_callback;
+struct isl_surf;
/**
* Memory zones. When allocating a buffer, you can request that it is
@@ -182,12 +183,6 @@ struct iris_bo {
*/
unsigned global_name;
- /**
- * Current tiling mode
- */
- uint32_t tiling_mode;
- uint32_t stride;
-
time_t free_time;
/** Mapped address for the buffer, saved across map/unmap cycles */
@@ -262,26 +257,6 @@ struct iris_bo *iris_bo_alloc(struct iris_bufmgr *bufmgr,
enum iris_memory_zone memzone,
unsigned flags);
-/**
- * Allocate a tiled buffer object.
- *
- * Alignment for tiled objects is set automatically; the 'flags'
- * argument provides a hint about how the object will be used initially.
- *
- * Valid tiling formats are:
- * I915_TILING_NONE
- * I915_TILING_X
- * I915_TILING_Y
- */
-struct iris_bo *iris_bo_alloc_tiled(struct iris_bufmgr *bufmgr,
- const char *name,
- uint64_t size,
- uint32_t alignment,
- enum iris_memory_zone memzone,
- uint32_t tiling_mode,
- uint32_t pitch,
- unsigned flags);
-
struct iris_bo *
iris_bo_create_userptr(struct iris_bufmgr *bufmgr, const char *name,
void *ptr, size_t size,
@@ -409,9 +384,11 @@ int iris_hw_context_set_priority(struct iris_bufmgr *bufmgr,
void iris_destroy_hw_context(struct iris_bufmgr *bufmgr, uint32_t ctx_id);
+int iris_gem_get_tiling(struct iris_bo *bo, uint32_t *tiling);
+int iris_gem_set_tiling(struct iris_bo *bo, const struct isl_surf *surf);
+
int iris_bo_export_dmabuf(struct iris_bo *bo, int *prime_fd);
-struct iris_bo *iris_bo_import_dmabuf(struct iris_bufmgr *bufmgr, int prime_fd,
- uint64_t modifier);
+struct iris_bo *iris_bo_import_dmabuf(struct iris_bufmgr *bufmgr, int prime_fd);
/**
* Exports a bo as a GEM handle into a given DRM file descriptor
@@ -425,9 +402,6 @@ struct iris_bo *iris_bo_import_dmabuf(struct iris_bufmgr *bufmgr, int prime_fd,
int iris_bo_export_gem_handle_for_device(struct iris_bo *bo, int drm_fd,
uint32_t *out_handle);
-struct iris_bo *iris_bo_import_dmabuf_no_mods(struct iris_bufmgr *bufmgr,
- int prime_fd);
-
uint32_t iris_bo_export_gem_handle(struct iris_bo *bo);
int iris_reg_read(struct iris_bufmgr *bufmgr, uint32_t offset, uint64_t *out);
diff --git a/src/gallium/drivers/iris/iris_resource.c b/src/gallium/drivers/iris/iris_resource.c
index 18d04fb7cde..3d28c83c3ad 100644
--- a/src/gallium/drivers/iris/iris_resource.c
+++ b/src/gallium/drivers/iris/iris_resource.c
@@ -308,7 +308,6 @@ iris_memobj_create_from_handle(struct pipe_screen *pscreen,
struct iris_screen *screen = (struct iris_screen *)pscreen;
struct iris_memory_object *memobj = CALLOC_STRUCT(iris_memory_object);
struct iris_bo *bo;
- const struct isl_drm_modifier_info *mod_inf;
if (!memobj)
return NULL;
@@ -319,19 +318,7 @@ iris_memobj_create_from_handle(struct pipe_screen *pscreen,
whandle->handle);
break;
case WINSYS_HANDLE_TYPE_FD:
- mod_inf = isl_drm_modifier_get_info(whandle->modifier);
- if (mod_inf) {
- bo = iris_bo_import_dmabuf(screen->bufmgr, whandle->handle,
- whandle->modifier);
- } else {
- /* If we can't get information about the tiling from the
- * kernel we ignore it. We are going to set it when we
- * create the resource.
- */
- bo = iris_bo_import_dmabuf_no_mods(screen->bufmgr,
- whandle->handle);
- }
-
+ bo = iris_bo_import_dmabuf(screen->bufmgr, whandle->handle);
break;
default:
unreachable("invalid winsys handle type");
@@ -1048,10 +1035,8 @@ iris_resource_create_with_modifiers(struct pipe_screen *pscreen,
}
uint32_t alignment = MAX2(4096, res->surf.alignment_B);
- res->bo = iris_bo_alloc_tiled(screen->bufmgr, name, bo_size, alignment,
- memzone,
- isl_tiling_to_i915_tiling(res->surf.tiling),
- res->surf.row_pitch_B, flags);
+ res->bo =
+ iris_bo_alloc(screen->bufmgr, name, bo_size, alignment, memzone, flags);
if (!res->bo)
goto fail;
@@ -1164,8 +1149,7 @@ iris_resource_from_handle(struct pipe_screen *pscreen,
switch (whandle->type) {
case WINSYS_HANDLE_TYPE_FD:
- res->bo = iris_bo_import_dmabuf(bufmgr, whandle->handle,
- whandle->modifier);
+ res->bo = iris_bo_import_dmabuf(bufmgr, whandle->handle);
break;
case WINSYS_HANDLE_TYPE_SHARED:
res->bo = iris_bo_gem_create_from_name(bufmgr, "winsys image",
@@ -1182,17 +1166,19 @@ iris_resource_from_handle(struct pipe_screen *pscreen,
/* Create a surface for each plane specified by the external format. */
if (whandle->plane < util_format_get_num_planes(whandle->format)) {
+ uint64_t modifier = whandle->modifier;
- const uint64_t modifier =
- whandle->modifier != DRM_FORMAT_MOD_INVALID ?
- whandle->modifier : tiling_to_modifier(res->bo->tiling_mode);
+ if (whandle->modifier == DRM_FORMAT_MOD_INVALID) {
+ /* We don't have a modifier; match whatever GEM_GET_TILING says */
+ uint32_t tiling;
+ iris_gem_get_tiling(res->bo, &tiling);
+ modifier = tiling_to_modifier(tiling);
+ }
UNUSED const bool isl_surf_created_successfully =
iris_resource_configure_main(screen, res, templ, modifier,
whandle->stride);
assert(isl_surf_created_successfully);
- assert(res->bo->tiling_mode ==
- isl_tiling_to_i915_tiling(res->surf.tiling));
UNUSED const bool ok = iris_resource_configure_aux(screen, res, true);
assert(ok);
@@ -1344,14 +1330,20 @@ iris_resource_get_param(struct pipe_screen *pscreen,
return true;
case PIPE_RESOURCE_PARAM_MODIFIER:
*value = res->mod_info ? res->mod_info->modifier :
- tiling_to_modifier(res->bo->tiling_mode);
+ tiling_to_modifier(isl_tiling_to_i915_tiling(res->surf.tiling));
return true;
case PIPE_RESOURCE_PARAM_HANDLE_TYPE_SHARED:
+ if (!wants_aux)
+ iris_gem_set_tiling(bo, &res->surf);
+
result = iris_bo_flink(bo, &handle) == 0;
if (result)
*value = handle;
return result;
case PIPE_RESOURCE_PARAM_HANDLE_TYPE_KMS: {
+ if (!wants_aux)
+ iris_gem_set_tiling(bo, &res->surf);
+
/* Because we share the same drm file across multiple iris_screen, when
* we export a GEM handle we must make sure it is valid in the DRM file
* descriptor the caller is using (this is the FD given at screen
@@ -1365,6 +1357,9 @@ iris_resource_get_param(struct pipe_screen *pscreen,
}
case PIPE_RESOURCE_PARAM_HANDLE_TYPE_FD:
+ if (!wants_aux)
+ iris_gem_set_tiling(bo, &res->surf);
+
result = iris_bo_export_dmabuf(bo, (int *) &handle) == 0;
if (result)
*value = handle;
@@ -1408,7 +1403,7 @@ iris_resource_get_handle(struct pipe_screen *pscreen,
whandle->format = res->external_format;
whandle->modifier =
res->mod_info ? res->mod_info->modifier
- : tiling_to_modifier(res->bo->tiling_mode);
+ : tiling_to_modifier(isl_tiling_to_i915_tiling(res->surf.tiling));
#ifndef NDEBUG
enum isl_aux_usage allowed_usage =
@@ -1424,8 +1419,11 @@ iris_resource_get_handle(struct pipe_screen *pscreen,
switch (whandle->type) {
case WINSYS_HANDLE_TYPE_SHARED:
+ iris_gem_set_tiling(bo, &res->surf);
return iris_bo_flink(bo, &whandle->handle) == 0;
case WINSYS_HANDLE_TYPE_KMS: {
+ iris_gem_set_tiling(bo, &res->surf);
+
/* Because we share the same drm file across multiple iris_screen, when
* we export a GEM handle we must make sure it is valid in the DRM file
* descriptor the caller is using (this is the FD given at screen
@@ -1438,6 +1436,7 @@ iris_resource_get_handle(struct pipe_screen *pscreen,
return true;
}
case WINSYS_HANDLE_TYPE_FD:
+ iris_gem_set_tiling(bo, &res->surf);
return iris_bo_export_dmabuf(bo, (int *) &whandle->handle) == 0;
}
More information about the mesa-commit
mailing list