[Intel-gfx] [RFC PATCH 092/162] drm/i915/uapi: introduce drm_i915_gem_create_ext
Thomas Hellström (Intel)
thomas_os at shipmail.org
Tue Dec 1 15:06:28 UTC 2020
On 11/27/20 2:25 PM, Chris Wilson wrote:
> 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 at intel.com>
>> Signed-off-by: CQ Tang <cq.tang at intel.com>
>> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
>> ---
>> 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.
Matthew, I have a patch series in the makings that moves this blit to
get_pages().
/Thomas
More information about the dri-devel
mailing list