Quoting Matthew Auld (2020-11-27 12:06:08)
> Same old gem_create but with now with extensions support. This is needed
> to support various upcoming usecases. For now we use the extensions
> mechanism to support setting an immutable-priority-list of potential
> placements, at creation time.
>
> If we wish to set the placements/regions we can simply do:
>
> struct drm_i915_gem_object_param region_param = { … }; /* Unchanged */
> struct drm_i915_gem_create_ext_setparam setparam_region = {
> .base = { .name = I915_GEM_CREATE_EXT_SETPARAM },
> .param = region_param,
> }
>
> struct drm_i915_gem_create_ext create_ext = {
> .size = 16 * PAGE_SIZE,
> .extensions = (uintptr_t)&setparam_region,
> };
> int err = ioctl(fd, DRM_IOCTL_I915_GEM_CREATE_EXT, &create_ext);
> if (err) ...
>
> If we use the normal gem_create or gem_create_ext without the
> extensions/placements then we still get the old behaviour with only
> placing the object in system memory.
>
> One important change here is the returned size will now be rounded up to
> the correct size, depending on the list of placements, where we might
> have minimum page-size restrictions on some platforms when dealing with
> device local-memory.
>
> Also, we still keep around the i915_gem_object_setparam ioctl, although
> that is now restricted by the placement list(i.e we are not allowed to
> add new placements), and longer term that will be going away wrt setting
> placements, since it was deemed that the kernel doesn't need to support
> a dynamic list of placements, which is now solidified by this uapi
> change.
>
> Testcase: igt/gem_create/create-ext-placement-sanity-check
> Testcase: igt/gem_create/create-ext-placement-each
> Testcase: igt/gem_create/create-ext-placement-all
> Signed-off-by: Matthew Auld <matthew.auld@xxxxxxxxx>
> Signed-off-by: CQ Tang <cq.tang@xxxxxxxxx>
> Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/i915/Makefile | 1 +
> drivers/gpu/drm/i915/gem/i915_gem_create.c | 398 ++++++++++++++++++
> drivers/gpu/drm/i915/gem/i915_gem_object.c | 2 +
> .../gpu/drm/i915/gem/i915_gem_object_types.h | 9 +
> drivers/gpu/drm/i915/gem/i915_gem_region.c | 4 +
> drivers/gpu/drm/i915/i915_drv.c | 2 +-
> drivers/gpu/drm/i915/i915_gem.c | 103 +----
> drivers/gpu/drm/i915/intel_memory_region.c | 20 +
> drivers/gpu/drm/i915/intel_memory_region.h | 4 +
> include/uapi/drm/i915_drm.h | 60 +++
> 10 files changed, 500 insertions(+), 103 deletions(-)
> create mode 100644 drivers/gpu/drm/i915/gem/i915_gem_create.c
>
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index ec361d61230b..3955134feca7 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -134,6 +134,7 @@ gem-y += \
> gem/i915_gem_clflush.o \
> gem/i915_gem_client_blt.o \
> gem/i915_gem_context.o \
> + gem/i915_gem_create.o \
> gem/i915_gem_dmabuf.o \
> gem/i915_gem_domain.o \
> gem/i915_gem_execbuffer.o \
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_create.c b/drivers/gpu/drm/i915/gem/i915_gem_create.c
> new file mode 100644
> index 000000000000..6f6dd4f1ce7e
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_create.c
> @@ -0,0 +1,398 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2020 Intel Corporation
> + */
> +
> +#include "gem/i915_gem_ioctls.h"
> +#include "gem/i915_gem_lmem.h"
> +#include "gem/i915_gem_object_blt.h"
> +#include "gem/i915_gem_region.h"
> +
> +#include "i915_drv.h"
> +#include "i915_user_extensions.h"
> +
> +static u32 max_page_size(struct intel_memory_region **placements,
> + int n_placements)
> +{
> + u32 max_page_size = 0;
> + int i;
> +
> + for (i = 0; i < n_placements; ++i) {
> + max_page_size = max_t(u32, max_page_size,
> + placements[i]->min_page_size);
> + }
> +
> + GEM_BUG_ON(!max_page_size);
> + return max_page_size;
> +}
> +
> +static int
> +i915_gem_create(struct drm_file *file,
> + struct intel_memory_region **placements,
> + int n_placements,
> + u64 *size_p,
> + u32 *handle_p)
> +{
> + struct drm_i915_gem_object *obj;
> + u32 handle;
> + u64 size;
> + int ret;
> +
> + size = round_up(*size_p, max_page_size(placements, n_placements));
> + if (size == 0)
> + return -EINVAL;
> +
> + /* For most of the ABI (e.g. mmap) we think in system pages */
> + GEM_BUG_ON(!IS_ALIGNED(size, PAGE_SIZE));
> +
> + /* Allocate the new object */
> + obj = i915_gem_object_create_region(placements[0], size, 0);
> + if (IS_ERR(obj))
> + return PTR_ERR(obj);
> +
> + if (i915_gem_object_is_lmem(obj)) {
> + struct intel_gt *gt = obj->mm.region->gt;
> + struct intel_context *ce = gt->engine[BCS0]->blitter_context;
> +
> + /*
> + * XXX: We really want to move this to get_pages(), but we
> + * require grabbing the BKL for the blitting operation which is
> + * annoying. In the pipeline is support for async get_pages()
> + * which should fit nicely for this. Also note that the actual
> + * clear should be done async(we currently do an object_wait
> + * which is pure garbage), we just need to take care if
> + * userspace opts of implicit sync for the execbuf, to avoid any
> + * potential info leak.
> + */
Not just XXX, but the design should be completed first.
-Chris
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
