Re: [Intel-gfx] [RFC PATCH 092/162] drm/i915/uapi: introduce drm_i915_gem_create_ext

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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

Matthew, I have a patch series in the makings that moves this blit to get_pages().

/Thomas


_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux