All of lore.kernel.org
 help / color / mirror / Atom feed
From: Matthew Auld <matthew.auld@intel.com>
To: intel-gfx@lists.freedesktop.org
Cc: Daniel Vetter <daniel.vetter@intel.com>, dri-devel@lists.freedesktop.org
Subject: [RFC PATCH] drm/doc/rfc: i915 DG1 uAPI
Date: Mon, 12 Apr 2021 13:18:02 +0100	[thread overview]
Message-ID: <20210412121802.57131-1-matthew.auld@intel.com> (raw)

Add an entry for the new uAPI needed for DG1.

Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@intel.com>
Cc: Dave Airlie <airlied@gmail.com>
---
 Documentation/gpu/rfc/i915_create_ext.c       | 48 ++++++++++
 .../gpu/rfc/i915_create_ext_placements.c      | 19 ++++
 Documentation/gpu/rfc/i915_region_query.c     | 54 +++++++++++
 Documentation/gpu/rfc/index.rst               | 91 +++++++++++++++++++
 4 files changed, 212 insertions(+)
 create mode 100644 Documentation/gpu/rfc/i915_create_ext.c
 create mode 100644 Documentation/gpu/rfc/i915_create_ext_placements.c
 create mode 100644 Documentation/gpu/rfc/i915_region_query.c

diff --git a/Documentation/gpu/rfc/i915_create_ext.c b/Documentation/gpu/rfc/i915_create_ext.c
new file mode 100644
index 000000000000..57c98717d14c
--- /dev/null
+++ b/Documentation/gpu/rfc/i915_create_ext.c
@@ -0,0 +1,48 @@
+struct drm_i915_gem_create_ext {
+	/*
+	 * Requested size for the object.
+	 *
+	 * The (page-aligned) allocated size for the object will be returned.
+	 */
+	__u64 size;
+	/*
+	 * Returned handle for the object.
+	 *
+	 * Object handles are nonzero.
+	 */
+	__u32 handle;
+	/* MBZ */
+	__u32 flags;
+	/*
+	 * For I915_GEM_CREATE_EXT_SETPARAM extension usage see both:
+	 *	struct drm_i915_gem_create_ext_setparam.
+	 *	struct drm_i915_gem_object_param for the possible parameters.
+	 */
+#define I915_GEM_CREATE_EXT_SETPARAM 0
+	__u64 extensions;
+};
+
+struct drm_i915_gem_object_param {
+	/* Object handle (0 for I915_GEM_CREATE_EXT_SETPARAM) */
+	__u32 handle;
+
+	/* Data pointer size */
+	__u32 size;
+
+/*
+ * I915_OBJECT_PARAM:
+ *
+ * Select object namespace for the param.
+ */
+#define I915_OBJECT_PARAM  (1ull<<32)
+
+	__u64 param;
+
+	/* Data value or pointer */
+	__u64 data;
+};
+
+struct drm_i915_gem_create_ext_setparam {
+	struct i915_user_extension base;
+	struct drm_i915_gem_object_param param;
+};
diff --git a/Documentation/gpu/rfc/i915_create_ext_placements.c b/Documentation/gpu/rfc/i915_create_ext_placements.c
new file mode 100644
index 000000000000..e2d14484d50a
--- /dev/null
+++ b/Documentation/gpu/rfc/i915_create_ext_placements.c
@@ -0,0 +1,19 @@
+#define I915_OBJECT_PARAM  (1ull<<32)
+
+/*
+ * I915_OBJECT_PARAM_MEMORY_REGIONS
+ *
+ * Set the data pointer with the desired set of placements in priority
+ * order(each entry must be unique and supported by the device), as an array of
+ * drm_i915_gem_memory_class_instance, or an equivalent layout of class:instance
+ * pair encodings. See DRM_I915_QUERY_MEMORY_REGIONS for how to query the
+ * supported regions.
+ *
+ * In this case the data pointer size should be the number of
+ * drm_i915_gem_memory_class_instance elements in the placements array.
+ *
+ */
+#define I915_PARAM_MEMORY_REGIONS 0
+#define I915_OBJECT_PARAM_MEMORY_REGIONS (I915_OBJECT_PARAM | \
+					  I915_PARAM_MEMORY_REGIONS)
+	__u64 param;
diff --git a/Documentation/gpu/rfc/i915_region_query.c b/Documentation/gpu/rfc/i915_region_query.c
new file mode 100644
index 000000000000..45e688726375
--- /dev/null
+++ b/Documentation/gpu/rfc/i915_region_query.c
@@ -0,0 +1,54 @@
+enum drm_i915_gem_memory_class {
+	I915_MEMORY_CLASS_SYSTEM = 0,
+	I915_MEMORY_CLASS_DEVICE,
+};
+
+struct drm_i915_gem_memory_class_instance {
+	__u16 memory_class; /* see enum drm_i915_gem_memory_class */
+	__u16 memory_instance;
+};
+
+/**
+ * struct drm_i915_memory_region_info
+ *
+ * Describes one region as known to the driver.
+ */
+struct drm_i915_memory_region_info {
+	/** class:instance pair encoding */
+	struct drm_i915_gem_memory_class_instance region;
+
+	/** MBZ */
+	__u32 rsvd0;
+
+	/** MBZ */
+	__u64 caps;
+
+	/** MBZ */
+	__u64 flags;
+
+	/** Memory probed by the driver (-1 = unknown) */
+	__u64 probed_size;
+
+	/** Estimate of memory remaining (-1 = unknown) */
+	__u64 unallocated_size;
+
+	/** MBZ */
+	__u64 rsvd1[8];
+};
+
+/**
+ * struct drm_i915_query_memory_regions
+ *
+ * Region info query enumerates all regions known to the driver by filling in
+ * an array of struct drm_i915_memory_region_info structures.
+ */
+struct drm_i915_query_memory_regions {
+	/** Number of supported regions */
+	__u32 num_regions;
+
+	/** MBZ */
+	__u32 rsvd[3];
+
+	/* Info about each supported region */
+	struct drm_i915_memory_region_info regions[];
+};
diff --git a/Documentation/gpu/rfc/index.rst b/Documentation/gpu/rfc/index.rst
index a8621f7dab8b..40572f514371 100644
--- a/Documentation/gpu/rfc/index.rst
+++ b/Documentation/gpu/rfc/index.rst
@@ -15,3 +15,94 @@ host such documentation:
 
 * Once the code has landed move all the documentation to the right places in
   the main core, helper or driver sections.
+
+I915 DG1/LMEM RFC Section
+=========================
+
+Object placement and region query
+=================================
+Starting from DG1 we need to give userspace the ability to allocate buffers from
+device local-memory. Currently the driver supports gem_create, which can place
+buffers in system memory via shmem, and the usual assortment of other
+interfaces, like dumb buffers and userptr.
+
+To support this new capability, while also providing a uAPI which will work
+beyond just DG1, we propose to offer three new bits of uAPI:
+
+DRM_I915_QUERY_MEMORY_REGIONS
+-----------------------------
+Query mechanism which allows userspace to discover the list of supported memory
+regions(like system-memory and local-memory) for a given device. We identify
+each region with a class and instance pair, which should be unique. The class
+here would be DEVICE or SYSTEM, and the instance would be zero, on platforms
+like DG1.
+
+Side note: The class/instance design is borrowed from our existing engine uAPI,
+where we describe every physical engine in terms of its class, and the
+particular instance, since we can have more than one per class.
+
+In the future we also want to expose more information which can further
+describe the capabilities of a region.
+
+.. literalinclude:: i915_region_query.c
+
+GEM_CREATE_EXT
+--------------
+New ioctl which is basically just gem_create but now allows userspace to
+provide a chain of possible extensions. Note that if we don't provide any
+extensions then we get the exact same behaviour as gem_create.
+
+.. literalinclude:: i915_create_ext.c
+
+Side note: We also need to support PXP[1] in the near future, which is also
+applicable to integrated platforms, and adds its own gem_create_ext extension,
+which basically lets userspace mark a buffer as "protected".
+
+I915_OBJECT_PARAM_MEMORY_REGIONS
+--------------------------------
+Implemented as an extension for gem_create_ext, we would now allow userspace to
+optionally provide an immutable list of preferred placements at creation time,
+in priority order, for a given buffer object.  For the placements we expect
+them each to use the class/instance encoding, as per the output of the regions
+query. Having the list in priority order will be useful in the future when
+placing an object, say during eviction.
+
+.. literalinclude:: i915_create_ext_placements.c
+
+As an example, on DG1 if we wish to set the placement as local-memory we can do
+something like:
+
+.. code-block::
+
+        struct drm_i915_gem_memory_class_instance region_param = {
+                .memory_class = I915_MEMORY_CLASS_DEVICE,
+                .memory_instance = 0,
+        };
+        struct drm_i915_gem_create_ext_setparam setparam_region = {
+                .base = { .name = I915_GEM_CREATE_EXT_SETPARAM },
+                .param = {
+                        .param = I915_OBJECT_PARAM_MEMORY_REGIONS,
+                        .data = (uintptr_t)&region_param,
+                        .size = 1,
+                },
+        };
+
+        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) ...
+
+One fair criticism here is that this seems a little over-engineered[2]. If we
+just consider DG1 then yes, a simple gem_create.flags or something is totally
+all that's needed to tell the kernel to allocate the buffer in local-memory or
+whatever. However looking to the future we need uAPI which can also support
+upcoming Xe HP multi-tile architecture in a sane way, where there can be
+multiple local-memory instances for a given device, and so using both class and
+instance in our uAPI to describe regions is desirable, although specifically
+for DG1 it's uninteresting, since we only have a single local-memory instance.
+
+[1] https://patchwork.freedesktop.org/series/86798/
+
+[2] https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/5599#note_553791
-- 
2.26.3

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

WARNING: multiple messages have this Message-ID (diff)
From: Matthew Auld <matthew.auld@intel.com>
To: intel-gfx@lists.freedesktop.org
Cc: Daniel Vetter <daniel.vetter@intel.com>, dri-devel@lists.freedesktop.org
Subject: [Intel-gfx] [RFC PATCH] drm/doc/rfc: i915 DG1 uAPI
Date: Mon, 12 Apr 2021 13:18:02 +0100	[thread overview]
Message-ID: <20210412121802.57131-1-matthew.auld@intel.com> (raw)

Add an entry for the new uAPI needed for DG1.

Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@intel.com>
Cc: Dave Airlie <airlied@gmail.com>
---
 Documentation/gpu/rfc/i915_create_ext.c       | 48 ++++++++++
 .../gpu/rfc/i915_create_ext_placements.c      | 19 ++++
 Documentation/gpu/rfc/i915_region_query.c     | 54 +++++++++++
 Documentation/gpu/rfc/index.rst               | 91 +++++++++++++++++++
 4 files changed, 212 insertions(+)
 create mode 100644 Documentation/gpu/rfc/i915_create_ext.c
 create mode 100644 Documentation/gpu/rfc/i915_create_ext_placements.c
 create mode 100644 Documentation/gpu/rfc/i915_region_query.c

diff --git a/Documentation/gpu/rfc/i915_create_ext.c b/Documentation/gpu/rfc/i915_create_ext.c
new file mode 100644
index 000000000000..57c98717d14c
--- /dev/null
+++ b/Documentation/gpu/rfc/i915_create_ext.c
@@ -0,0 +1,48 @@
+struct drm_i915_gem_create_ext {
+	/*
+	 * Requested size for the object.
+	 *
+	 * The (page-aligned) allocated size for the object will be returned.
+	 */
+	__u64 size;
+	/*
+	 * Returned handle for the object.
+	 *
+	 * Object handles are nonzero.
+	 */
+	__u32 handle;
+	/* MBZ */
+	__u32 flags;
+	/*
+	 * For I915_GEM_CREATE_EXT_SETPARAM extension usage see both:
+	 *	struct drm_i915_gem_create_ext_setparam.
+	 *	struct drm_i915_gem_object_param for the possible parameters.
+	 */
+#define I915_GEM_CREATE_EXT_SETPARAM 0
+	__u64 extensions;
+};
+
+struct drm_i915_gem_object_param {
+	/* Object handle (0 for I915_GEM_CREATE_EXT_SETPARAM) */
+	__u32 handle;
+
+	/* Data pointer size */
+	__u32 size;
+
+/*
+ * I915_OBJECT_PARAM:
+ *
+ * Select object namespace for the param.
+ */
+#define I915_OBJECT_PARAM  (1ull<<32)
+
+	__u64 param;
+
+	/* Data value or pointer */
+	__u64 data;
+};
+
+struct drm_i915_gem_create_ext_setparam {
+	struct i915_user_extension base;
+	struct drm_i915_gem_object_param param;
+};
diff --git a/Documentation/gpu/rfc/i915_create_ext_placements.c b/Documentation/gpu/rfc/i915_create_ext_placements.c
new file mode 100644
index 000000000000..e2d14484d50a
--- /dev/null
+++ b/Documentation/gpu/rfc/i915_create_ext_placements.c
@@ -0,0 +1,19 @@
+#define I915_OBJECT_PARAM  (1ull<<32)
+
+/*
+ * I915_OBJECT_PARAM_MEMORY_REGIONS
+ *
+ * Set the data pointer with the desired set of placements in priority
+ * order(each entry must be unique and supported by the device), as an array of
+ * drm_i915_gem_memory_class_instance, or an equivalent layout of class:instance
+ * pair encodings. See DRM_I915_QUERY_MEMORY_REGIONS for how to query the
+ * supported regions.
+ *
+ * In this case the data pointer size should be the number of
+ * drm_i915_gem_memory_class_instance elements in the placements array.
+ *
+ */
+#define I915_PARAM_MEMORY_REGIONS 0
+#define I915_OBJECT_PARAM_MEMORY_REGIONS (I915_OBJECT_PARAM | \
+					  I915_PARAM_MEMORY_REGIONS)
+	__u64 param;
diff --git a/Documentation/gpu/rfc/i915_region_query.c b/Documentation/gpu/rfc/i915_region_query.c
new file mode 100644
index 000000000000..45e688726375
--- /dev/null
+++ b/Documentation/gpu/rfc/i915_region_query.c
@@ -0,0 +1,54 @@
+enum drm_i915_gem_memory_class {
+	I915_MEMORY_CLASS_SYSTEM = 0,
+	I915_MEMORY_CLASS_DEVICE,
+};
+
+struct drm_i915_gem_memory_class_instance {
+	__u16 memory_class; /* see enum drm_i915_gem_memory_class */
+	__u16 memory_instance;
+};
+
+/**
+ * struct drm_i915_memory_region_info
+ *
+ * Describes one region as known to the driver.
+ */
+struct drm_i915_memory_region_info {
+	/** class:instance pair encoding */
+	struct drm_i915_gem_memory_class_instance region;
+
+	/** MBZ */
+	__u32 rsvd0;
+
+	/** MBZ */
+	__u64 caps;
+
+	/** MBZ */
+	__u64 flags;
+
+	/** Memory probed by the driver (-1 = unknown) */
+	__u64 probed_size;
+
+	/** Estimate of memory remaining (-1 = unknown) */
+	__u64 unallocated_size;
+
+	/** MBZ */
+	__u64 rsvd1[8];
+};
+
+/**
+ * struct drm_i915_query_memory_regions
+ *
+ * Region info query enumerates all regions known to the driver by filling in
+ * an array of struct drm_i915_memory_region_info structures.
+ */
+struct drm_i915_query_memory_regions {
+	/** Number of supported regions */
+	__u32 num_regions;
+
+	/** MBZ */
+	__u32 rsvd[3];
+
+	/* Info about each supported region */
+	struct drm_i915_memory_region_info regions[];
+};
diff --git a/Documentation/gpu/rfc/index.rst b/Documentation/gpu/rfc/index.rst
index a8621f7dab8b..40572f514371 100644
--- a/Documentation/gpu/rfc/index.rst
+++ b/Documentation/gpu/rfc/index.rst
@@ -15,3 +15,94 @@ host such documentation:
 
 * Once the code has landed move all the documentation to the right places in
   the main core, helper or driver sections.
+
+I915 DG1/LMEM RFC Section
+=========================
+
+Object placement and region query
+=================================
+Starting from DG1 we need to give userspace the ability to allocate buffers from
+device local-memory. Currently the driver supports gem_create, which can place
+buffers in system memory via shmem, and the usual assortment of other
+interfaces, like dumb buffers and userptr.
+
+To support this new capability, while also providing a uAPI which will work
+beyond just DG1, we propose to offer three new bits of uAPI:
+
+DRM_I915_QUERY_MEMORY_REGIONS
+-----------------------------
+Query mechanism which allows userspace to discover the list of supported memory
+regions(like system-memory and local-memory) for a given device. We identify
+each region with a class and instance pair, which should be unique. The class
+here would be DEVICE or SYSTEM, and the instance would be zero, on platforms
+like DG1.
+
+Side note: The class/instance design is borrowed from our existing engine uAPI,
+where we describe every physical engine in terms of its class, and the
+particular instance, since we can have more than one per class.
+
+In the future we also want to expose more information which can further
+describe the capabilities of a region.
+
+.. literalinclude:: i915_region_query.c
+
+GEM_CREATE_EXT
+--------------
+New ioctl which is basically just gem_create but now allows userspace to
+provide a chain of possible extensions. Note that if we don't provide any
+extensions then we get the exact same behaviour as gem_create.
+
+.. literalinclude:: i915_create_ext.c
+
+Side note: We also need to support PXP[1] in the near future, which is also
+applicable to integrated platforms, and adds its own gem_create_ext extension,
+which basically lets userspace mark a buffer as "protected".
+
+I915_OBJECT_PARAM_MEMORY_REGIONS
+--------------------------------
+Implemented as an extension for gem_create_ext, we would now allow userspace to
+optionally provide an immutable list of preferred placements at creation time,
+in priority order, for a given buffer object.  For the placements we expect
+them each to use the class/instance encoding, as per the output of the regions
+query. Having the list in priority order will be useful in the future when
+placing an object, say during eviction.
+
+.. literalinclude:: i915_create_ext_placements.c
+
+As an example, on DG1 if we wish to set the placement as local-memory we can do
+something like:
+
+.. code-block::
+
+        struct drm_i915_gem_memory_class_instance region_param = {
+                .memory_class = I915_MEMORY_CLASS_DEVICE,
+                .memory_instance = 0,
+        };
+        struct drm_i915_gem_create_ext_setparam setparam_region = {
+                .base = { .name = I915_GEM_CREATE_EXT_SETPARAM },
+                .param = {
+                        .param = I915_OBJECT_PARAM_MEMORY_REGIONS,
+                        .data = (uintptr_t)&region_param,
+                        .size = 1,
+                },
+        };
+
+        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) ...
+
+One fair criticism here is that this seems a little over-engineered[2]. If we
+just consider DG1 then yes, a simple gem_create.flags or something is totally
+all that's needed to tell the kernel to allocate the buffer in local-memory or
+whatever. However looking to the future we need uAPI which can also support
+upcoming Xe HP multi-tile architecture in a sane way, where there can be
+multiple local-memory instances for a given device, and so using both class and
+instance in our uAPI to describe regions is desirable, although specifically
+for DG1 it's uninteresting, since we only have a single local-memory instance.
+
+[1] https://patchwork.freedesktop.org/series/86798/
+
+[2] https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/5599#note_553791
-- 
2.26.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

             reply	other threads:[~2021-04-12 12:18 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-12 12:18 Matthew Auld [this message]
2021-04-12 12:18 ` [Intel-gfx] [RFC PATCH] drm/doc/rfc: i915 DG1 uAPI Matthew Auld
2021-04-12 12:29 ` Daniel Vetter
2021-04-12 12:29   ` [Intel-gfx] " Daniel Vetter
2021-04-12 15:39 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
2021-04-12 15:43 ` [Intel-gfx] ✗ Fi.CI.DOCS: " Patchwork
2021-04-12 16:07 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2021-04-12 19:24 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210412121802.57131-1-matthew.auld@intel.com \
    --to=matthew.auld@intel.com \
    --cc=daniel.vetter@intel.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.