* [PATCH 0/2] doc/rfc for small BAR support
@ 2022-02-18 11:22 ` Matthew Auld
0 siblings, 0 replies; 15+ messages in thread
From: Matthew Auld @ 2022-02-18 11:22 UTC (permalink / raw)
To: intel-gfx; +Cc: dri-devel
The new bits of proposed uAPI for the upcoming small BAR support.
--
2.34.1
^ permalink raw reply [flat|nested] 15+ messages in thread
* [Intel-gfx] [PATCH 0/2] doc/rfc for small BAR support
@ 2022-02-18 11:22 ` Matthew Auld
0 siblings, 0 replies; 15+ messages in thread
From: Matthew Auld @ 2022-02-18 11:22 UTC (permalink / raw)
To: intel-gfx; +Cc: dri-devel
The new bits of proposed uAPI for the upcoming small BAR support.
--
2.34.1
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/2] drm/doc: remove rfc section for dg1
2022-02-18 11:22 ` [Intel-gfx] " Matthew Auld
@ 2022-02-18 11:22 ` Matthew Auld
-1 siblings, 0 replies; 15+ messages in thread
From: Matthew Auld @ 2022-02-18 11:22 UTC (permalink / raw)
To: intel-gfx
Cc: Thomas Hellström, Jordan Justen, dri-devel, Kenneth Graunke,
Jon Bloomfield, Daniel Vetter, mesa-dev
We already completed the steps for this.
Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
Cc: Jon Bloomfield <jon.bloomfield@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Kenneth Graunke <kenneth@whitecape.org>
Cc: mesa-dev@lists.freedesktop.org
---
Documentation/gpu/rfc/i915_gem_lmem.rst | 22 ----------------------
Documentation/gpu/rfc/index.rst | 4 ----
2 files changed, 26 deletions(-)
delete mode 100644 Documentation/gpu/rfc/i915_gem_lmem.rst
diff --git a/Documentation/gpu/rfc/i915_gem_lmem.rst b/Documentation/gpu/rfc/i915_gem_lmem.rst
deleted file mode 100644
index b421a3c1806e..000000000000
--- a/Documentation/gpu/rfc/i915_gem_lmem.rst
+++ /dev/null
@@ -1,22 +0,0 @@
-=========================
-I915 DG1/LMEM RFC Section
-=========================
-
-Upstream plan
-=============
-For upstream the overall plan for landing all the DG1 stuff and turning it for
-real, with all the uAPI bits is:
-
-* Merge basic HW enabling of DG1(still without pciid)
-* Merge the uAPI bits behind special CONFIG_BROKEN(or so) flag
- * At this point we can still make changes, but importantly this lets us
- start running IGTs which can utilize local-memory in CI
-* Convert over to TTM, make sure it all keeps working. Some of the work items:
- * TTM shrinker for discrete
- * dma_resv_lockitem for full dma_resv_lock, i.e not just trylock
- * Use TTM CPU pagefault handler
- * Route shmem backend over to TTM SYSTEM for discrete
- * TTM purgeable object support
- * Move i915 buddy allocator over to TTM
-* Send RFC(with mesa-dev on cc) for final sign off on the uAPI
-* Add pciid for DG1 and turn on uAPI for real
diff --git a/Documentation/gpu/rfc/index.rst b/Documentation/gpu/rfc/index.rst
index 91e93a705230..018a8bf317a6 100644
--- a/Documentation/gpu/rfc/index.rst
+++ b/Documentation/gpu/rfc/index.rst
@@ -16,10 +16,6 @@ host such documentation:
* Once the code has landed move all the documentation to the right places in
the main core, helper or driver sections.
-.. toctree::
-
- i915_gem_lmem.rst
-
.. toctree::
i915_scheduler.rst
--
2.34.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Intel-gfx] [PATCH 1/2] drm/doc: remove rfc section for dg1
@ 2022-02-18 11:22 ` Matthew Auld
0 siblings, 0 replies; 15+ messages in thread
From: Matthew Auld @ 2022-02-18 11:22 UTC (permalink / raw)
To: intel-gfx
Cc: Thomas Hellström, dri-devel, Kenneth Graunke, Daniel Vetter,
mesa-dev
We already completed the steps for this.
Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
Cc: Jon Bloomfield <jon.bloomfield@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Kenneth Graunke <kenneth@whitecape.org>
Cc: mesa-dev@lists.freedesktop.org
---
Documentation/gpu/rfc/i915_gem_lmem.rst | 22 ----------------------
Documentation/gpu/rfc/index.rst | 4 ----
2 files changed, 26 deletions(-)
delete mode 100644 Documentation/gpu/rfc/i915_gem_lmem.rst
diff --git a/Documentation/gpu/rfc/i915_gem_lmem.rst b/Documentation/gpu/rfc/i915_gem_lmem.rst
deleted file mode 100644
index b421a3c1806e..000000000000
--- a/Documentation/gpu/rfc/i915_gem_lmem.rst
+++ /dev/null
@@ -1,22 +0,0 @@
-=========================
-I915 DG1/LMEM RFC Section
-=========================
-
-Upstream plan
-=============
-For upstream the overall plan for landing all the DG1 stuff and turning it for
-real, with all the uAPI bits is:
-
-* Merge basic HW enabling of DG1(still without pciid)
-* Merge the uAPI bits behind special CONFIG_BROKEN(or so) flag
- * At this point we can still make changes, but importantly this lets us
- start running IGTs which can utilize local-memory in CI
-* Convert over to TTM, make sure it all keeps working. Some of the work items:
- * TTM shrinker for discrete
- * dma_resv_lockitem for full dma_resv_lock, i.e not just trylock
- * Use TTM CPU pagefault handler
- * Route shmem backend over to TTM SYSTEM for discrete
- * TTM purgeable object support
- * Move i915 buddy allocator over to TTM
-* Send RFC(with mesa-dev on cc) for final sign off on the uAPI
-* Add pciid for DG1 and turn on uAPI for real
diff --git a/Documentation/gpu/rfc/index.rst b/Documentation/gpu/rfc/index.rst
index 91e93a705230..018a8bf317a6 100644
--- a/Documentation/gpu/rfc/index.rst
+++ b/Documentation/gpu/rfc/index.rst
@@ -16,10 +16,6 @@ host such documentation:
* Once the code has landed move all the documentation to the right places in
the main core, helper or driver sections.
-.. toctree::
-
- i915_gem_lmem.rst
-
.. toctree::
i915_scheduler.rst
--
2.34.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 2/2] drm/doc: add rfc section for small BAR uapi
2022-02-18 11:22 ` [Intel-gfx] " Matthew Auld
@ 2022-02-18 11:22 ` Matthew Auld
-1 siblings, 0 replies; 15+ messages in thread
From: Matthew Auld @ 2022-02-18 11:22 UTC (permalink / raw)
To: intel-gfx
Cc: Thomas Hellström, Jordan Justen, dri-devel, Kenneth Graunke,
Jon Bloomfield, Daniel Vetter, mesa-dev
Add an entry for the new uapi needed for small BAR on DG2+.
Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
Cc: Jon Bloomfield <jon.bloomfield@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Kenneth Graunke <kenneth@whitecape.org>
Cc: mesa-dev@lists.freedesktop.org
---
Documentation/gpu/rfc/i915_small_bar.h | 153 +++++++++++++++++++++++
Documentation/gpu/rfc/i915_small_bar.rst | 40 ++++++
Documentation/gpu/rfc/index.rst | 4 +
3 files changed, 197 insertions(+)
create mode 100644 Documentation/gpu/rfc/i915_small_bar.h
create mode 100644 Documentation/gpu/rfc/i915_small_bar.rst
diff --git a/Documentation/gpu/rfc/i915_small_bar.h b/Documentation/gpu/rfc/i915_small_bar.h
new file mode 100644
index 000000000000..fa65835fd608
--- /dev/null
+++ b/Documentation/gpu/rfc/i915_small_bar.h
@@ -0,0 +1,153 @@
+/**
+ * struct __drm_i915_gem_create_ext - Existing gem_create behaviour, with added
+ * extension support using struct i915_user_extension.
+ *
+ * Note that in the future we want to have our buffer flags here, at least for
+ * the stuff that is immutable. Previously we would have two ioctls, one to
+ * create the object with gem_create, and another to apply various parameters,
+ * however this creates some ambiguity for the params which are considered
+ * immutable. Also in general we're phasing out the various SET/GET ioctls.
+ */
+struct __drm_i915_gem_create_ext {
+ /**
+ * @size: Requested size for the object.
+ *
+ * The (page-aligned) allocated size for the object will be returned.
+ *
+ * Note that for some devices we have might have further minimum
+ * page-size restrictions(larger than 4K), like for device local-memory.
+ * However in general the final size here should always reflect any
+ * rounding up, if for example using the I915_GEM_CREATE_EXT_MEMORY_REGIONS
+ * extension to place the object in device local-memory.
+ */
+ __u64 size;
+ /**
+ * @handle: Returned handle for the object.
+ *
+ * Object handles are nonzero.
+ */
+ __u32 handle;
+ /**
+ * @flags: Optional flags.
+ *
+ * Supported values:
+ *
+ * I915_GEM_CREATE_EXT_FLAG_NEEDS_CPU_ACCESS - Signal to the kernel that
+ * the object will need to be accessed via the CPU.
+ *
+ * Only valid when placing objects in I915_MEMORY_CLASS_DEVICE, and
+ * only strictly required on platforms where only some of the device
+ * memory is directly visible or mappable through the CPU, like on DG2+.
+ *
+ * One of the placements MUST also be I915_MEMORY_CLASS_SYSTEM, to
+ * ensure we can always spill the allocation to system memory, if we
+ * can't place the object in the mappable part of
+ * I915_MEMORY_CLASS_DEVICE.
+ *
+ * Note that buffers that need to be captured with EXEC_OBJECT_CAPTURE,
+ * will need to enable this hint, if the object can also be placed in
+ * I915_MEMORY_CLASS_DEVICE, starting from DG2+. The execbuf call will
+ * throw an error otherwise. This also means that such objects will need
+ * I915_MEMORY_CLASS_SYSTEM set as a possible placement.
+ *
+ * Without this hint, the kernel will assume that non-mappable
+ * I915_MEMORY_CLASS_DEVICE is preferred for this object. Note that the
+ * kernel can still migrate the object to the mappable part, as a last
+ * resort, if userspace ever CPU faults this object, but this might be
+ * expensive, and so ideally should be avoided.
+ */
+#define I915_GEM_CREATE_EXT_FLAG_NEEDS_CPU_ACCESS (1 << 0)
+ __u32 flags;
+ /**
+ * @extensions: The chain of extensions to apply to this object.
+ *
+ * This will be useful in the future when we need to support several
+ * different extensions, and we need to apply more than one when
+ * creating the object. See struct i915_user_extension.
+ *
+ * If we don't supply any extensions then we get the same old gem_create
+ * behaviour.
+ *
+ * For I915_GEM_CREATE_EXT_MEMORY_REGIONS usage see
+ * struct drm_i915_gem_create_ext_memory_regions.
+ *
+ * For I915_GEM_CREATE_EXT_PROTECTED_CONTENT usage see
+ * struct drm_i915_gem_create_ext_protected_content.
+ */
+#define I915_GEM_CREATE_EXT_MEMORY_REGIONS 0
+#define I915_GEM_CREATE_EXT_PROTECTED_CONTENT 1
+ __u64 extensions;
+};
+
+#define DRM_I915_QUERY_VMA_INFO 5
+
+/**
+ * struct __drm_i915_query_vma_info
+ *
+ * Given a vm and GTT address, lookup the corresponding vma, returning its set
+ * of attributes.
+ *
+ * .. code-block:: C
+ *
+ * struct drm_i915_query_vma_info info = {};
+ * struct drm_i915_query_item item = {
+ * .data_ptr = (uintptr_t)&info,
+ * .query_id = DRM_I915_QUERY_VMA_INFO,
+ * };
+ * struct drm_i915_query query = {
+ * .num_items = 1,
+ * .items_ptr = (uintptr_t)&item,
+ * };
+ * int err;
+ *
+ * // Unlike some other types of queries, there is no need to first query
+ * // the size of the data_ptr blob here, since we already know ahead of
+ * // time how big this needs to be.
+ * item.length = sizeof(info);
+ *
+ * // Next we fill in the vm_id and ppGTT address of the vma we wish
+ * // to query, before then firing off the query.
+ * info.vm_id = vm_id;
+ * info.offset = gtt_address;
+ * err = ioctl(fd, DRM_IOCTL_I915_QUERY, &query);
+ * if (err || item.length < 0) ...
+ *
+ * // If all went well we can now inspect the returned attributes.
+ * if (info.attributes & DRM_I915_QUERY_VMA_INFO_CPU_VISIBLE) ...
+ */
+struct __drm_i915_query_vma_info {
+ /**
+ * @vm_id: The given vm id that contains the vma. The id is the value
+ * returned by the DRM_I915_GEM_VM_CREATE. See struct
+ * drm_i915_gem_vm_control.vm_id.
+ */
+ __u32 vm_id;
+ /** @pad: MBZ. */
+ __u32 pad;
+ /**
+ * @offset: The corresponding ppGTT address of the vma which the kernel
+ * will use to perform the lookup.
+ */
+ __u64 offset;
+ /**
+ * @attributes: The returned attributes for the given vma.
+ *
+ * Possible values:
+ *
+ * DRM_I915_QUERY_VMA_INFO_CPU_VISIBLE - Set if the pages backing the
+ * vma are currently CPU accessible. If this is not set then the vma is
+ * currently backed by I915_MEMORY_CLASS_DEVICE memory, which the CPU
+ * cannot directly access(this is only possible on discrete devices with
+ * a small BAR). Attempting to MMAP and fault such an object will
+ * require the kernel first synchronising any GPU work tied to the
+ * object, before then migrating the pages, either to the CPU accessible
+ * part of I915_MEMORY_CLASS_DEVICE, or I915_MEMORY_CLASS_SYSTEM, if the
+ * placements permit it. See I915_GEM_CREATE_EXT_FLAG_NEEDS_CPU_ACCESS.
+ *
+ * Note that this is inherently racy.
+ */
+#define DRM_I915_QUERY_VMA_INFO_CPU_VISIBLE (1<<0)
+ __u64 attributes;
+ /** @rsvd: MBZ */
+ __u32 rsvd[4];
+};
diff --git a/Documentation/gpu/rfc/i915_small_bar.rst b/Documentation/gpu/rfc/i915_small_bar.rst
new file mode 100644
index 000000000000..fea92d3d69ab
--- /dev/null
+++ b/Documentation/gpu/rfc/i915_small_bar.rst
@@ -0,0 +1,40 @@
+==========================
+I915 Small BAR RFC Section
+==========================
+Starting from DG2 we will have resizable BAR support for device local-memory,
+but in some cases the final BAR size might still be smaller than the total
+local-memory size. In such cases only part of local-memory will be CPU
+accessible, while the remainder is only accessible via the GPU.
+
+I915_GEM_CREATE_EXT_FLAG_NEEDS_CPU_ACCESS flag
+----------------------------------------------
+New gem_create_ext flag to tell the kernel that a BO will require CPU access.
+The becomes important when placing an object in LMEM, where underneath the
+device has a small BAR, meaning only part of it is CPU accessible. Without this
+flag the kernel will assume that CPU access is not required, and prioritize
+using the non-CPU visible portion of LMEM(if present on the device).
+
+Related to this, we now also reject any objects marked with
+EXEC_OBJECT_CAPTURE, which are also not tagged with NEEDS_CPU_ACCESS. This only
+impacts DG2+.
+
+XXX: One open here is whether we should extend the memory region query to return
+the CPU visible size of the region. For now the IGTs just use debugfs to query
+the size. However, if userspace sees a real need for this then extending the
+region query would be a lot nicer.
+
+.. kernel-doc:: Documentation/gpu/rfc/i915_small_bar.h
+ :functions: __drm_i915_gem_create_ext
+
+DRM_I915_QUERY_VMA_INFO query
+-----------------------------
+Query the attributes of some vma. Given a vm and GTT offset, find the
+respective vma, and return its set of attrubutes. For now we only support
+DRM_I915_QUERY_VMA_INFO_CPU_VISIBLE, which is set if the object/vma is
+currently placed in memory that is accessible by the CPU. This should always be
+set on devices where the CPU visible size of LMEM matches the probed size. If
+this is not set then CPU faulting the object will first require migrating the
+pages.
+
+.. kernel-doc:: Documentation/gpu/rfc/i915_small_bar.h
+ :functions: __drm_i915_query_vma_info
diff --git a/Documentation/gpu/rfc/index.rst b/Documentation/gpu/rfc/index.rst
index 018a8bf317a6..5b8495bdc1fd 100644
--- a/Documentation/gpu/rfc/index.rst
+++ b/Documentation/gpu/rfc/index.rst
@@ -19,3 +19,7 @@ host such documentation:
.. toctree::
i915_scheduler.rst
+
+.. toctree::
+
+ i915_small_bar.rst
--
2.34.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Intel-gfx] [PATCH 2/2] drm/doc: add rfc section for small BAR uapi
@ 2022-02-18 11:22 ` Matthew Auld
0 siblings, 0 replies; 15+ messages in thread
From: Matthew Auld @ 2022-02-18 11:22 UTC (permalink / raw)
To: intel-gfx
Cc: Thomas Hellström, dri-devel, Kenneth Graunke, Daniel Vetter,
mesa-dev
Add an entry for the new uapi needed for small BAR on DG2+.
Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
Cc: Jon Bloomfield <jon.bloomfield@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Kenneth Graunke <kenneth@whitecape.org>
Cc: mesa-dev@lists.freedesktop.org
---
Documentation/gpu/rfc/i915_small_bar.h | 153 +++++++++++++++++++++++
Documentation/gpu/rfc/i915_small_bar.rst | 40 ++++++
Documentation/gpu/rfc/index.rst | 4 +
3 files changed, 197 insertions(+)
create mode 100644 Documentation/gpu/rfc/i915_small_bar.h
create mode 100644 Documentation/gpu/rfc/i915_small_bar.rst
diff --git a/Documentation/gpu/rfc/i915_small_bar.h b/Documentation/gpu/rfc/i915_small_bar.h
new file mode 100644
index 000000000000..fa65835fd608
--- /dev/null
+++ b/Documentation/gpu/rfc/i915_small_bar.h
@@ -0,0 +1,153 @@
+/**
+ * struct __drm_i915_gem_create_ext - Existing gem_create behaviour, with added
+ * extension support using struct i915_user_extension.
+ *
+ * Note that in the future we want to have our buffer flags here, at least for
+ * the stuff that is immutable. Previously we would have two ioctls, one to
+ * create the object with gem_create, and another to apply various parameters,
+ * however this creates some ambiguity for the params which are considered
+ * immutable. Also in general we're phasing out the various SET/GET ioctls.
+ */
+struct __drm_i915_gem_create_ext {
+ /**
+ * @size: Requested size for the object.
+ *
+ * The (page-aligned) allocated size for the object will be returned.
+ *
+ * Note that for some devices we have might have further minimum
+ * page-size restrictions(larger than 4K), like for device local-memory.
+ * However in general the final size here should always reflect any
+ * rounding up, if for example using the I915_GEM_CREATE_EXT_MEMORY_REGIONS
+ * extension to place the object in device local-memory.
+ */
+ __u64 size;
+ /**
+ * @handle: Returned handle for the object.
+ *
+ * Object handles are nonzero.
+ */
+ __u32 handle;
+ /**
+ * @flags: Optional flags.
+ *
+ * Supported values:
+ *
+ * I915_GEM_CREATE_EXT_FLAG_NEEDS_CPU_ACCESS - Signal to the kernel that
+ * the object will need to be accessed via the CPU.
+ *
+ * Only valid when placing objects in I915_MEMORY_CLASS_DEVICE, and
+ * only strictly required on platforms where only some of the device
+ * memory is directly visible or mappable through the CPU, like on DG2+.
+ *
+ * One of the placements MUST also be I915_MEMORY_CLASS_SYSTEM, to
+ * ensure we can always spill the allocation to system memory, if we
+ * can't place the object in the mappable part of
+ * I915_MEMORY_CLASS_DEVICE.
+ *
+ * Note that buffers that need to be captured with EXEC_OBJECT_CAPTURE,
+ * will need to enable this hint, if the object can also be placed in
+ * I915_MEMORY_CLASS_DEVICE, starting from DG2+. The execbuf call will
+ * throw an error otherwise. This also means that such objects will need
+ * I915_MEMORY_CLASS_SYSTEM set as a possible placement.
+ *
+ * Without this hint, the kernel will assume that non-mappable
+ * I915_MEMORY_CLASS_DEVICE is preferred for this object. Note that the
+ * kernel can still migrate the object to the mappable part, as a last
+ * resort, if userspace ever CPU faults this object, but this might be
+ * expensive, and so ideally should be avoided.
+ */
+#define I915_GEM_CREATE_EXT_FLAG_NEEDS_CPU_ACCESS (1 << 0)
+ __u32 flags;
+ /**
+ * @extensions: The chain of extensions to apply to this object.
+ *
+ * This will be useful in the future when we need to support several
+ * different extensions, and we need to apply more than one when
+ * creating the object. See struct i915_user_extension.
+ *
+ * If we don't supply any extensions then we get the same old gem_create
+ * behaviour.
+ *
+ * For I915_GEM_CREATE_EXT_MEMORY_REGIONS usage see
+ * struct drm_i915_gem_create_ext_memory_regions.
+ *
+ * For I915_GEM_CREATE_EXT_PROTECTED_CONTENT usage see
+ * struct drm_i915_gem_create_ext_protected_content.
+ */
+#define I915_GEM_CREATE_EXT_MEMORY_REGIONS 0
+#define I915_GEM_CREATE_EXT_PROTECTED_CONTENT 1
+ __u64 extensions;
+};
+
+#define DRM_I915_QUERY_VMA_INFO 5
+
+/**
+ * struct __drm_i915_query_vma_info
+ *
+ * Given a vm and GTT address, lookup the corresponding vma, returning its set
+ * of attributes.
+ *
+ * .. code-block:: C
+ *
+ * struct drm_i915_query_vma_info info = {};
+ * struct drm_i915_query_item item = {
+ * .data_ptr = (uintptr_t)&info,
+ * .query_id = DRM_I915_QUERY_VMA_INFO,
+ * };
+ * struct drm_i915_query query = {
+ * .num_items = 1,
+ * .items_ptr = (uintptr_t)&item,
+ * };
+ * int err;
+ *
+ * // Unlike some other types of queries, there is no need to first query
+ * // the size of the data_ptr blob here, since we already know ahead of
+ * // time how big this needs to be.
+ * item.length = sizeof(info);
+ *
+ * // Next we fill in the vm_id and ppGTT address of the vma we wish
+ * // to query, before then firing off the query.
+ * info.vm_id = vm_id;
+ * info.offset = gtt_address;
+ * err = ioctl(fd, DRM_IOCTL_I915_QUERY, &query);
+ * if (err || item.length < 0) ...
+ *
+ * // If all went well we can now inspect the returned attributes.
+ * if (info.attributes & DRM_I915_QUERY_VMA_INFO_CPU_VISIBLE) ...
+ */
+struct __drm_i915_query_vma_info {
+ /**
+ * @vm_id: The given vm id that contains the vma. The id is the value
+ * returned by the DRM_I915_GEM_VM_CREATE. See struct
+ * drm_i915_gem_vm_control.vm_id.
+ */
+ __u32 vm_id;
+ /** @pad: MBZ. */
+ __u32 pad;
+ /**
+ * @offset: The corresponding ppGTT address of the vma which the kernel
+ * will use to perform the lookup.
+ */
+ __u64 offset;
+ /**
+ * @attributes: The returned attributes for the given vma.
+ *
+ * Possible values:
+ *
+ * DRM_I915_QUERY_VMA_INFO_CPU_VISIBLE - Set if the pages backing the
+ * vma are currently CPU accessible. If this is not set then the vma is
+ * currently backed by I915_MEMORY_CLASS_DEVICE memory, which the CPU
+ * cannot directly access(this is only possible on discrete devices with
+ * a small BAR). Attempting to MMAP and fault such an object will
+ * require the kernel first synchronising any GPU work tied to the
+ * object, before then migrating the pages, either to the CPU accessible
+ * part of I915_MEMORY_CLASS_DEVICE, or I915_MEMORY_CLASS_SYSTEM, if the
+ * placements permit it. See I915_GEM_CREATE_EXT_FLAG_NEEDS_CPU_ACCESS.
+ *
+ * Note that this is inherently racy.
+ */
+#define DRM_I915_QUERY_VMA_INFO_CPU_VISIBLE (1<<0)
+ __u64 attributes;
+ /** @rsvd: MBZ */
+ __u32 rsvd[4];
+};
diff --git a/Documentation/gpu/rfc/i915_small_bar.rst b/Documentation/gpu/rfc/i915_small_bar.rst
new file mode 100644
index 000000000000..fea92d3d69ab
--- /dev/null
+++ b/Documentation/gpu/rfc/i915_small_bar.rst
@@ -0,0 +1,40 @@
+==========================
+I915 Small BAR RFC Section
+==========================
+Starting from DG2 we will have resizable BAR support for device local-memory,
+but in some cases the final BAR size might still be smaller than the total
+local-memory size. In such cases only part of local-memory will be CPU
+accessible, while the remainder is only accessible via the GPU.
+
+I915_GEM_CREATE_EXT_FLAG_NEEDS_CPU_ACCESS flag
+----------------------------------------------
+New gem_create_ext flag to tell the kernel that a BO will require CPU access.
+The becomes important when placing an object in LMEM, where underneath the
+device has a small BAR, meaning only part of it is CPU accessible. Without this
+flag the kernel will assume that CPU access is not required, and prioritize
+using the non-CPU visible portion of LMEM(if present on the device).
+
+Related to this, we now also reject any objects marked with
+EXEC_OBJECT_CAPTURE, which are also not tagged with NEEDS_CPU_ACCESS. This only
+impacts DG2+.
+
+XXX: One open here is whether we should extend the memory region query to return
+the CPU visible size of the region. For now the IGTs just use debugfs to query
+the size. However, if userspace sees a real need for this then extending the
+region query would be a lot nicer.
+
+.. kernel-doc:: Documentation/gpu/rfc/i915_small_bar.h
+ :functions: __drm_i915_gem_create_ext
+
+DRM_I915_QUERY_VMA_INFO query
+-----------------------------
+Query the attributes of some vma. Given a vm and GTT offset, find the
+respective vma, and return its set of attrubutes. For now we only support
+DRM_I915_QUERY_VMA_INFO_CPU_VISIBLE, which is set if the object/vma is
+currently placed in memory that is accessible by the CPU. This should always be
+set on devices where the CPU visible size of LMEM matches the probed size. If
+this is not set then CPU faulting the object will first require migrating the
+pages.
+
+.. kernel-doc:: Documentation/gpu/rfc/i915_small_bar.h
+ :functions: __drm_i915_query_vma_info
diff --git a/Documentation/gpu/rfc/index.rst b/Documentation/gpu/rfc/index.rst
index 018a8bf317a6..5b8495bdc1fd 100644
--- a/Documentation/gpu/rfc/index.rst
+++ b/Documentation/gpu/rfc/index.rst
@@ -19,3 +19,7 @@ host such documentation:
.. toctree::
i915_scheduler.rst
+
+.. toctree::
+
+ i915_small_bar.rst
--
2.34.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for doc/rfc for small BAR support
2022-02-18 11:22 ` [Intel-gfx] " Matthew Auld
` (2 preceding siblings ...)
(?)
@ 2022-02-18 14:15 ` Patchwork
-1 siblings, 0 replies; 15+ messages in thread
From: Patchwork @ 2022-02-18 14:15 UTC (permalink / raw)
To: Matthew Auld; +Cc: intel-gfx
== Series Details ==
Series: doc/rfc for small BAR support
URL : https://patchwork.freedesktop.org/series/100399/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
b54660c8b559 drm/doc: remove rfc section for dg1
-:20: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating?
#20:
deleted file mode 100644
total: 0 errors, 1 warnings, 0 checks, 10 lines checked
5a0f497847ee drm/doc: add rfc section for small BAR uapi
-:20: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating?
#20:
new file mode 100644
-:25: WARNING:SPDX_LICENSE_TAG: Missing or malformed SPDX-License-Identifier tag in line 1
#25: FILE: Documentation/gpu/rfc/i915_small_bar.h:1:
+/**
-:173: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
#173: FILE: Documentation/gpu/rfc/i915_small_bar.h:149:
+#define DRM_I915_QUERY_VMA_INFO_CPU_VISIBLE (1<<0)
^
-:184: WARNING:SPDX_LICENSE_TAG: Missing or malformed SPDX-License-Identifier tag in line 1
#184: FILE: Documentation/gpu/rfc/i915_small_bar.rst:1:
+==========================
total: 0 errors, 3 warnings, 1 checks, 200 lines checked
^ permalink raw reply [flat|nested] 15+ messages in thread
* [Intel-gfx] ✓ Fi.CI.BAT: success for doc/rfc for small BAR support
2022-02-18 11:22 ` [Intel-gfx] " Matthew Auld
` (3 preceding siblings ...)
(?)
@ 2022-02-18 14:45 ` Patchwork
-1 siblings, 0 replies; 15+ messages in thread
From: Patchwork @ 2022-02-18 14:45 UTC (permalink / raw)
To: Matthew Auld; +Cc: intel-gfx
[-- Attachment #1: Type: text/plain, Size: 4153 bytes --]
== Series Details ==
Series: doc/rfc for small BAR support
URL : https://patchwork.freedesktop.org/series/100399/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_11248 -> Patchwork_22331
====================================================
Summary
-------
**SUCCESS**
No regressions found.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22331/index.html
Participating hosts (45 -> 43)
------------------------------
Missing (2): fi-bsw-cyan shard-tglu
Known issues
------------
Here are the changes found in Patchwork_22331 that come from known issues:
### CI changes ###
#### Issues hit ####
* boot:
- fi-ilk-650: [PASS][1] -> [FAIL][2] ([i915#4684])
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11248/fi-ilk-650/boot.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22331/fi-ilk-650/boot.html
### IGT changes ###
#### Issues hit ####
* igt@gem_huc_copy@huc-copy:
- fi-skl-6600u: NOTRUN -> [SKIP][3] ([fdo#109271] / [i915#2190])
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22331/fi-skl-6600u/igt@gem_huc_copy@huc-copy.html
* igt@gem_lmem_swapping@verify-random:
- fi-skl-6600u: NOTRUN -> [SKIP][4] ([fdo#109271] / [i915#4613]) +3 similar issues
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22331/fi-skl-6600u/igt@gem_lmem_swapping@verify-random.html
* igt@kms_chamelium@vga-edid-read:
- fi-skl-6600u: NOTRUN -> [SKIP][5] ([fdo#109271] / [fdo#111827]) +8 similar issues
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22331/fi-skl-6600u/igt@kms_chamelium@vga-edid-read.html
* igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy:
- fi-skl-6600u: NOTRUN -> [SKIP][6] ([fdo#109271]) +21 similar issues
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22331/fi-skl-6600u/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy.html
* igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d:
- fi-skl-6600u: NOTRUN -> [SKIP][7] ([fdo#109271] / [i915#533])
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22331/fi-skl-6600u/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d.html
#### Possible fixes ####
* igt@kms_frontbuffer_tracking@basic:
- fi-cml-u2: [DMESG-WARN][8] ([i915#4269]) -> [PASS][9]
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11248/fi-cml-u2/igt@kms_frontbuffer_tracking@basic.html
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22331/fi-cml-u2/igt@kms_frontbuffer_tracking@basic.html
* igt@kms_pipe_crc_basic@read-crc-pipe-b:
- fi-cfl-8109u: [DMESG-WARN][10] ([i915#295]) -> [PASS][11] +12 similar issues
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11248/fi-cfl-8109u/igt@kms_pipe_crc_basic@read-crc-pipe-b.html
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22331/fi-cfl-8109u/igt@kms_pipe_crc_basic@read-crc-pipe-b.html
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
[i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
[i915#295]: https://gitlab.freedesktop.org/drm/intel/issues/295
[i915#4269]: https://gitlab.freedesktop.org/drm/intel/issues/4269
[i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
[i915#4684]: https://gitlab.freedesktop.org/drm/intel/issues/4684
[i915#533]: https://gitlab.freedesktop.org/drm/intel/issues/533
Build changes
-------------
* Linux: CI_DRM_11248 -> Patchwork_22331
CI-20190529: 20190529
CI_DRM_11248: 8861c3684bdcd4c8cb8385fbc37a2d9033dff955 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_6347: 37ea4c86f97c0e05fcb6b04cff72ec927930536e @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
Patchwork_22331: 5a0f497847eebbd1790fddf08216a8f49e82741f @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
5a0f497847ee drm/doc: add rfc section for small BAR uapi
b54660c8b559 drm/doc: remove rfc section for dg1
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22331/index.html
[-- Attachment #2: Type: text/html, Size: 5214 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Intel-gfx] [PATCH 1/2] drm/doc: remove rfc section for dg1
2022-02-18 11:22 ` [Intel-gfx] " Matthew Auld
(?)
@ 2022-02-18 18:30 ` Lucas De Marchi
-1 siblings, 0 replies; 15+ messages in thread
From: Lucas De Marchi @ 2022-02-18 18:30 UTC (permalink / raw)
To: Matthew Auld
Cc: Thomas Hellström, Daniel Vetter, intel-gfx, dri-devel,
Kenneth Graunke, mesa-dev
On Fri, Feb 18, 2022 at 11:22:41AM +0000, Matthew Auld wrote:
>We already completed the steps for this.
>
>Signed-off-by: Matthew Auld <matthew.auld@intel.com>
>Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
>Cc: Jon Bloomfield <jon.bloomfield@intel.com>
>Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>Cc: Jordan Justen <jordan.l.justen@intel.com>
>Cc: Kenneth Graunke <kenneth@whitecape.org>
>Cc: mesa-dev@lists.freedesktop.org
I was indeed wondering why that was still there and why we were going a
similar route with DG2, but this time adding it after the plan completed.
Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>
thanks
Lucas De Marchi
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] drm/doc: add rfc section for small BAR uapi
2022-02-18 11:22 ` [Intel-gfx] " Matthew Auld
@ 2022-02-22 10:36 ` Thomas Hellström
-1 siblings, 0 replies; 15+ messages in thread
From: Thomas Hellström @ 2022-02-22 10:36 UTC (permalink / raw)
To: Matthew Auld, intel-gfx
Cc: Daniel Vetter, dri-devel, Kenneth Graunke, Jon Bloomfield,
Jordan Justen, mesa-dev
On 2/18/22 12:22, Matthew Auld wrote:
> Add an entry for the new uapi needed for small BAR on DG2+.
>
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> Cc: Jon Bloomfield <jon.bloomfield@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Kenneth Graunke <kenneth@whitecape.org>
> Cc: mesa-dev@lists.freedesktop.org
> ---
> Documentation/gpu/rfc/i915_small_bar.h | 153 +++++++++++++++++++++++
> Documentation/gpu/rfc/i915_small_bar.rst | 40 ++++++
> Documentation/gpu/rfc/index.rst | 4 +
> 3 files changed, 197 insertions(+)
> create mode 100644 Documentation/gpu/rfc/i915_small_bar.h
> create mode 100644 Documentation/gpu/rfc/i915_small_bar.rst
>
> diff --git a/Documentation/gpu/rfc/i915_small_bar.h b/Documentation/gpu/rfc/i915_small_bar.h
> new file mode 100644
> index 000000000000..fa65835fd608
> --- /dev/null
> +++ b/Documentation/gpu/rfc/i915_small_bar.h
> @@ -0,0 +1,153 @@
> +/**
> + * struct __drm_i915_gem_create_ext - Existing gem_create behaviour, with added
> + * extension support using struct i915_user_extension.
> + *
> + * Note that in the future we want to have our buffer flags here,
Does this sentence need updating, with the flags member?
> at least for
> + * the stuff that is immutable. Previously we would have two ioctls, one to
> + * create the object with gem_create, and another to apply various parameters,
> + * however this creates some ambiguity for the params which are considered
> + * immutable. Also in general we're phasing out the various SET/GET ioctls.
> + */
> +struct __drm_i915_gem_create_ext {
> + /**
> + * @size: Requested size for the object.
> + *
> + * The (page-aligned) allocated size for the object will be returned.
> + *
> + * Note that for some devices we have might have further minimum
> + * page-size restrictions(larger than 4K), like for device local-memory.
> + * However in general the final size here should always reflect any
> + * rounding up, if for example using the I915_GEM_CREATE_EXT_MEMORY_REGIONS
> + * extension to place the object in device local-memory.
> + */
> + __u64 size;
> + /**
> + * @handle: Returned handle for the object.
> + *
> + * Object handles are nonzero.
> + */
> + __u32 handle;
> + /**
> + * @flags: Optional flags.
> + *
> + * Supported values:
> + *
> + * I915_GEM_CREATE_EXT_FLAG_NEEDS_CPU_ACCESS - Signal to the kernel that
> + * the object will need to be accessed via the CPU.
> + *
> + * Only valid when placing objects in I915_MEMORY_CLASS_DEVICE, and
> + * only strictly required on platforms where only some of the device
> + * memory is directly visible or mappable through the CPU, like on DG2+.
> + *
> + * One of the placements MUST also be I915_MEMORY_CLASS_SYSTEM, to
> + * ensure we can always spill the allocation to system memory, if we
> + * can't place the object in the mappable part of
> + * I915_MEMORY_CLASS_DEVICE.
> + *
> + * Note that buffers that need to be captured with EXEC_OBJECT_CAPTURE,
> + * will need to enable this hint, if the object can also be placed in
> + * I915_MEMORY_CLASS_DEVICE, starting from DG2+. The execbuf call will
> + * throw an error otherwise. This also means that such objects will need
> + * I915_MEMORY_CLASS_SYSTEM set as a possible placement.
> + *
> + * Without this hint, the kernel will assume that non-mappable
> + * I915_MEMORY_CLASS_DEVICE is preferred for this object. Note that the
> + * kernel can still migrate the object to the mappable part, as a last
> + * resort, if userspace ever CPU faults this object, but this might be
> + * expensive, and so ideally should be avoided.
> + */
> +#define I915_GEM_CREATE_EXT_FLAG_NEEDS_CPU_ACCESS (1 << 0)
> + __u32 flags;
> + /**
> + * @extensions: The chain of extensions to apply to this object.
> + *
> + * This will be useful in the future when we need to support several
> + * different extensions, and we need to apply more than one when
> + * creating the object. See struct i915_user_extension.
> + *
> + * If we don't supply any extensions then we get the same old gem_create
> + * behaviour.
> + *
> + * For I915_GEM_CREATE_EXT_MEMORY_REGIONS usage see
> + * struct drm_i915_gem_create_ext_memory_regions.
> + *
> + * For I915_GEM_CREATE_EXT_PROTECTED_CONTENT usage see
> + * struct drm_i915_gem_create_ext_protected_content.
> + */
> +#define I915_GEM_CREATE_EXT_MEMORY_REGIONS 0
> +#define I915_GEM_CREATE_EXT_PROTECTED_CONTENT 1
> + __u64 extensions;
> +};
> +
> +#define DRM_I915_QUERY_VMA_INFO 5
> +
> +/**
> + * struct __drm_i915_query_vma_info
> + *
> + * Given a vm and GTT address, lookup the corresponding vma, returning its set
> + * of attributes.
> + *
> + * .. code-block:: C
> + *
> + * struct drm_i915_query_vma_info info = {};
> + * struct drm_i915_query_item item = {
> + * .data_ptr = (uintptr_t)&info,
> + * .query_id = DRM_I915_QUERY_VMA_INFO,
> + * };
> + * struct drm_i915_query query = {
> + * .num_items = 1,
> + * .items_ptr = (uintptr_t)&item,
> + * };
> + * int err;
> + *
> + * // Unlike some other types of queries, there is no need to first query
> + * // the size of the data_ptr blob here, since we already know ahead of
> + * // time how big this needs to be.
> + * item.length = sizeof(info);
> + *
> + * // Next we fill in the vm_id and ppGTT address of the vma we wish
> + * // to query, before then firing off the query.
> + * info.vm_id = vm_id;
> + * info.offset = gtt_address;
> + * err = ioctl(fd, DRM_IOCTL_I915_QUERY, &query);
> + * if (err || item.length < 0) ...
> + *
> + * // If all went well we can now inspect the returned attributes.
> + * if (info.attributes & DRM_I915_QUERY_VMA_INFO_CPU_VISIBLE) ...
> + */
> +struct __drm_i915_query_vma_info {
> + /**
> + * @vm_id: The given vm id that contains the vma. The id is the value
> + * returned by the DRM_I915_GEM_VM_CREATE. See struct
> + * drm_i915_gem_vm_control.vm_id.
> + */
> + __u32 vm_id;
> + /** @pad: MBZ. */
> + __u32 pad;
> + /**
> + * @offset: The corresponding ppGTT address of the vma which the kernel
> + * will use to perform the lookup.
> + */
> + __u64 offset;
> + /**
> + * @attributes: The returned attributes for the given vma.
> + *
> + * Possible values:
> + *
> + * DRM_I915_QUERY_VMA_INFO_CPU_VISIBLE - Set if the pages backing the
> + * vma are currently CPU accessible. If this is not set then the vma is
> + * currently backed by I915_MEMORY_CLASS_DEVICE memory, which the CPU
> + * cannot directly access(this is only possible on discrete devices with
> + * a small BAR). Attempting to MMAP and fault such an object will
> + * require the kernel first synchronising any GPU work tied to the
> + * object, before then migrating the pages, either to the CPU accessible
> + * part of I915_MEMORY_CLASS_DEVICE, or I915_MEMORY_CLASS_SYSTEM, if the
> + * placements permit it. See I915_GEM_CREATE_EXT_FLAG_NEEDS_CPU_ACCESS.
> + *
> + * Note that this is inherently racy.
> + */
> +#define DRM_I915_QUERY_VMA_INFO_CPU_VISIBLE (1<<0)
> + __u64 attributes;
> + /** @rsvd: MBZ */
> + __u32 rsvd[4];
> +};
> diff --git a/Documentation/gpu/rfc/i915_small_bar.rst b/Documentation/gpu/rfc/i915_small_bar.rst
> new file mode 100644
> index 000000000000..fea92d3d69ab
> --- /dev/null
> +++ b/Documentation/gpu/rfc/i915_small_bar.rst
> @@ -0,0 +1,40 @@
> +==========================
> +I915 Small BAR RFC Section
> +==========================
> +Starting from DG2 we will have resizable BAR support for device local-memory,
> +but in some cases the final BAR size might still be smaller than the total
> +local-memory size. In such cases only part of local-memory will be CPU
> +accessible, while the remainder is only accessible via the GPU.
> +
> +I915_GEM_CREATE_EXT_FLAG_NEEDS_CPU_ACCESS flag
> +----------------------------------------------
> +New gem_create_ext flag to tell the kernel that a BO will require CPU access.
> +The becomes important when placing an object in LMEM, where underneath the
> +device has a small BAR, meaning only part of it is CPU accessible. Without this
> +flag the kernel will assume that CPU access is not required, and prioritize
> +using the non-CPU visible portion of LMEM(if present on the device).
> +
> +Related to this, we now also reject any objects marked with
> +EXEC_OBJECT_CAPTURE, which are also not tagged with NEEDS_CPU_ACCESS. This only
> +impacts DG2+.
> +
> +XXX: One open here is whether we should extend the memory region query to return
> +the CPU visible size of the region. For now the IGTs just use debugfs to query
> +the size. However, if userspace sees a real need for this then extending the
> +region query would be a lot nicer.
I guess UMD folks need to comment on this. Although I think since there
might be a number of
clients utilizing the mappable part, and a number of buffers pinned in
there, I figure this might be of limited value outside of tests without
some kind of cgroups support.
Otherwise
Acked-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> +
> +.. kernel-doc:: Documentation/gpu/rfc/i915_small_bar.h
> + :functions: __drm_i915_gem_create_ext
> +
> +DRM_I915_QUERY_VMA_INFO query
> +-----------------------------
> +Query the attributes of some vma. Given a vm and GTT offset, find the
> +respective vma, and return its set of attrubutes. For now we only support
> +DRM_I915_QUERY_VMA_INFO_CPU_VISIBLE, which is set if the object/vma is
> +currently placed in memory that is accessible by the CPU. This should always be
> +set on devices where the CPU visible size of LMEM matches the probed size. If
> +this is not set then CPU faulting the object will first require migrating the
> +pages.
> +
> +.. kernel-doc:: Documentation/gpu/rfc/i915_small_bar.h
> + :functions: __drm_i915_query_vma_info
> diff --git a/Documentation/gpu/rfc/index.rst b/Documentation/gpu/rfc/index.rst
> index 018a8bf317a6..5b8495bdc1fd 100644
> --- a/Documentation/gpu/rfc/index.rst
> +++ b/Documentation/gpu/rfc/index.rst
> @@ -19,3 +19,7 @@ host such documentation:
> .. toctree::
>
> i915_scheduler.rst
> +
> +.. toctree::
> +
> + i915_small_bar.rst
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Intel-gfx] [PATCH 2/2] drm/doc: add rfc section for small BAR uapi
@ 2022-02-22 10:36 ` Thomas Hellström
0 siblings, 0 replies; 15+ messages in thread
From: Thomas Hellström @ 2022-02-22 10:36 UTC (permalink / raw)
To: Matthew Auld, intel-gfx
Cc: Daniel Vetter, dri-devel, Kenneth Graunke, mesa-dev
On 2/18/22 12:22, Matthew Auld wrote:
> Add an entry for the new uapi needed for small BAR on DG2+.
>
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> Cc: Jon Bloomfield <jon.bloomfield@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Kenneth Graunke <kenneth@whitecape.org>
> Cc: mesa-dev@lists.freedesktop.org
> ---
> Documentation/gpu/rfc/i915_small_bar.h | 153 +++++++++++++++++++++++
> Documentation/gpu/rfc/i915_small_bar.rst | 40 ++++++
> Documentation/gpu/rfc/index.rst | 4 +
> 3 files changed, 197 insertions(+)
> create mode 100644 Documentation/gpu/rfc/i915_small_bar.h
> create mode 100644 Documentation/gpu/rfc/i915_small_bar.rst
>
> diff --git a/Documentation/gpu/rfc/i915_small_bar.h b/Documentation/gpu/rfc/i915_small_bar.h
> new file mode 100644
> index 000000000000..fa65835fd608
> --- /dev/null
> +++ b/Documentation/gpu/rfc/i915_small_bar.h
> @@ -0,0 +1,153 @@
> +/**
> + * struct __drm_i915_gem_create_ext - Existing gem_create behaviour, with added
> + * extension support using struct i915_user_extension.
> + *
> + * Note that in the future we want to have our buffer flags here,
Does this sentence need updating, with the flags member?
> at least for
> + * the stuff that is immutable. Previously we would have two ioctls, one to
> + * create the object with gem_create, and another to apply various parameters,
> + * however this creates some ambiguity for the params which are considered
> + * immutable. Also in general we're phasing out the various SET/GET ioctls.
> + */
> +struct __drm_i915_gem_create_ext {
> + /**
> + * @size: Requested size for the object.
> + *
> + * The (page-aligned) allocated size for the object will be returned.
> + *
> + * Note that for some devices we have might have further minimum
> + * page-size restrictions(larger than 4K), like for device local-memory.
> + * However in general the final size here should always reflect any
> + * rounding up, if for example using the I915_GEM_CREATE_EXT_MEMORY_REGIONS
> + * extension to place the object in device local-memory.
> + */
> + __u64 size;
> + /**
> + * @handle: Returned handle for the object.
> + *
> + * Object handles are nonzero.
> + */
> + __u32 handle;
> + /**
> + * @flags: Optional flags.
> + *
> + * Supported values:
> + *
> + * I915_GEM_CREATE_EXT_FLAG_NEEDS_CPU_ACCESS - Signal to the kernel that
> + * the object will need to be accessed via the CPU.
> + *
> + * Only valid when placing objects in I915_MEMORY_CLASS_DEVICE, and
> + * only strictly required on platforms where only some of the device
> + * memory is directly visible or mappable through the CPU, like on DG2+.
> + *
> + * One of the placements MUST also be I915_MEMORY_CLASS_SYSTEM, to
> + * ensure we can always spill the allocation to system memory, if we
> + * can't place the object in the mappable part of
> + * I915_MEMORY_CLASS_DEVICE.
> + *
> + * Note that buffers that need to be captured with EXEC_OBJECT_CAPTURE,
> + * will need to enable this hint, if the object can also be placed in
> + * I915_MEMORY_CLASS_DEVICE, starting from DG2+. The execbuf call will
> + * throw an error otherwise. This also means that such objects will need
> + * I915_MEMORY_CLASS_SYSTEM set as a possible placement.
> + *
> + * Without this hint, the kernel will assume that non-mappable
> + * I915_MEMORY_CLASS_DEVICE is preferred for this object. Note that the
> + * kernel can still migrate the object to the mappable part, as a last
> + * resort, if userspace ever CPU faults this object, but this might be
> + * expensive, and so ideally should be avoided.
> + */
> +#define I915_GEM_CREATE_EXT_FLAG_NEEDS_CPU_ACCESS (1 << 0)
> + __u32 flags;
> + /**
> + * @extensions: The chain of extensions to apply to this object.
> + *
> + * This will be useful in the future when we need to support several
> + * different extensions, and we need to apply more than one when
> + * creating the object. See struct i915_user_extension.
> + *
> + * If we don't supply any extensions then we get the same old gem_create
> + * behaviour.
> + *
> + * For I915_GEM_CREATE_EXT_MEMORY_REGIONS usage see
> + * struct drm_i915_gem_create_ext_memory_regions.
> + *
> + * For I915_GEM_CREATE_EXT_PROTECTED_CONTENT usage see
> + * struct drm_i915_gem_create_ext_protected_content.
> + */
> +#define I915_GEM_CREATE_EXT_MEMORY_REGIONS 0
> +#define I915_GEM_CREATE_EXT_PROTECTED_CONTENT 1
> + __u64 extensions;
> +};
> +
> +#define DRM_I915_QUERY_VMA_INFO 5
> +
> +/**
> + * struct __drm_i915_query_vma_info
> + *
> + * Given a vm and GTT address, lookup the corresponding vma, returning its set
> + * of attributes.
> + *
> + * .. code-block:: C
> + *
> + * struct drm_i915_query_vma_info info = {};
> + * struct drm_i915_query_item item = {
> + * .data_ptr = (uintptr_t)&info,
> + * .query_id = DRM_I915_QUERY_VMA_INFO,
> + * };
> + * struct drm_i915_query query = {
> + * .num_items = 1,
> + * .items_ptr = (uintptr_t)&item,
> + * };
> + * int err;
> + *
> + * // Unlike some other types of queries, there is no need to first query
> + * // the size of the data_ptr blob here, since we already know ahead of
> + * // time how big this needs to be.
> + * item.length = sizeof(info);
> + *
> + * // Next we fill in the vm_id and ppGTT address of the vma we wish
> + * // to query, before then firing off the query.
> + * info.vm_id = vm_id;
> + * info.offset = gtt_address;
> + * err = ioctl(fd, DRM_IOCTL_I915_QUERY, &query);
> + * if (err || item.length < 0) ...
> + *
> + * // If all went well we can now inspect the returned attributes.
> + * if (info.attributes & DRM_I915_QUERY_VMA_INFO_CPU_VISIBLE) ...
> + */
> +struct __drm_i915_query_vma_info {
> + /**
> + * @vm_id: The given vm id that contains the vma. The id is the value
> + * returned by the DRM_I915_GEM_VM_CREATE. See struct
> + * drm_i915_gem_vm_control.vm_id.
> + */
> + __u32 vm_id;
> + /** @pad: MBZ. */
> + __u32 pad;
> + /**
> + * @offset: The corresponding ppGTT address of the vma which the kernel
> + * will use to perform the lookup.
> + */
> + __u64 offset;
> + /**
> + * @attributes: The returned attributes for the given vma.
> + *
> + * Possible values:
> + *
> + * DRM_I915_QUERY_VMA_INFO_CPU_VISIBLE - Set if the pages backing the
> + * vma are currently CPU accessible. If this is not set then the vma is
> + * currently backed by I915_MEMORY_CLASS_DEVICE memory, which the CPU
> + * cannot directly access(this is only possible on discrete devices with
> + * a small BAR). Attempting to MMAP and fault such an object will
> + * require the kernel first synchronising any GPU work tied to the
> + * object, before then migrating the pages, either to the CPU accessible
> + * part of I915_MEMORY_CLASS_DEVICE, or I915_MEMORY_CLASS_SYSTEM, if the
> + * placements permit it. See I915_GEM_CREATE_EXT_FLAG_NEEDS_CPU_ACCESS.
> + *
> + * Note that this is inherently racy.
> + */
> +#define DRM_I915_QUERY_VMA_INFO_CPU_VISIBLE (1<<0)
> + __u64 attributes;
> + /** @rsvd: MBZ */
> + __u32 rsvd[4];
> +};
> diff --git a/Documentation/gpu/rfc/i915_small_bar.rst b/Documentation/gpu/rfc/i915_small_bar.rst
> new file mode 100644
> index 000000000000..fea92d3d69ab
> --- /dev/null
> +++ b/Documentation/gpu/rfc/i915_small_bar.rst
> @@ -0,0 +1,40 @@
> +==========================
> +I915 Small BAR RFC Section
> +==========================
> +Starting from DG2 we will have resizable BAR support for device local-memory,
> +but in some cases the final BAR size might still be smaller than the total
> +local-memory size. In such cases only part of local-memory will be CPU
> +accessible, while the remainder is only accessible via the GPU.
> +
> +I915_GEM_CREATE_EXT_FLAG_NEEDS_CPU_ACCESS flag
> +----------------------------------------------
> +New gem_create_ext flag to tell the kernel that a BO will require CPU access.
> +The becomes important when placing an object in LMEM, where underneath the
> +device has a small BAR, meaning only part of it is CPU accessible. Without this
> +flag the kernel will assume that CPU access is not required, and prioritize
> +using the non-CPU visible portion of LMEM(if present on the device).
> +
> +Related to this, we now also reject any objects marked with
> +EXEC_OBJECT_CAPTURE, which are also not tagged with NEEDS_CPU_ACCESS. This only
> +impacts DG2+.
> +
> +XXX: One open here is whether we should extend the memory region query to return
> +the CPU visible size of the region. For now the IGTs just use debugfs to query
> +the size. However, if userspace sees a real need for this then extending the
> +region query would be a lot nicer.
I guess UMD folks need to comment on this. Although I think since there
might be a number of
clients utilizing the mappable part, and a number of buffers pinned in
there, I figure this might be of limited value outside of tests without
some kind of cgroups support.
Otherwise
Acked-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> +
> +.. kernel-doc:: Documentation/gpu/rfc/i915_small_bar.h
> + :functions: __drm_i915_gem_create_ext
> +
> +DRM_I915_QUERY_VMA_INFO query
> +-----------------------------
> +Query the attributes of some vma. Given a vm and GTT offset, find the
> +respective vma, and return its set of attrubutes. For now we only support
> +DRM_I915_QUERY_VMA_INFO_CPU_VISIBLE, which is set if the object/vma is
> +currently placed in memory that is accessible by the CPU. This should always be
> +set on devices where the CPU visible size of LMEM matches the probed size. If
> +this is not set then CPU faulting the object will first require migrating the
> +pages.
> +
> +.. kernel-doc:: Documentation/gpu/rfc/i915_small_bar.h
> + :functions: __drm_i915_query_vma_info
> diff --git a/Documentation/gpu/rfc/index.rst b/Documentation/gpu/rfc/index.rst
> index 018a8bf317a6..5b8495bdc1fd 100644
> --- a/Documentation/gpu/rfc/index.rst
> +++ b/Documentation/gpu/rfc/index.rst
> @@ -19,3 +19,7 @@ host such documentation:
> .. toctree::
>
> i915_scheduler.rst
> +
> +.. toctree::
> +
> + i915_small_bar.rst
^ permalink raw reply [flat|nested] 15+ messages in thread
* RE: [PATCH 2/2] drm/doc: add rfc section for small BAR uapi
2022-02-22 10:36 ` [Intel-gfx] " Thomas Hellström
@ 2022-02-22 17:39 ` Abodunrin, Akeem G
-1 siblings, 0 replies; 15+ messages in thread
From: Abodunrin, Akeem G @ 2022-02-22 17:39 UTC (permalink / raw)
To: Thomas Hellström, Auld, Matthew, intel-gfx
Cc: Daniel Vetter, dri-devel, Kenneth Graunke, Bloomfield, Jon,
Justen, Jordan L, mesa-dev
> -----Original Message-----
> From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of
> Thomas Hellström
> Sent: Tuesday, February 22, 2022 2:36 AM
> To: Auld, Matthew <matthew.auld@intel.com>; intel-gfx@lists.freedesktop.org
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>; dri-devel@lists.freedesktop.org;
> Kenneth Graunke <kenneth@whitecape.org>; Bloomfield, Jon
> <jon.bloomfield@intel.com>; Justen, Jordan L <jordan.l.justen@intel.com>;
> mesa-dev@lists.freedesktop.org
> Subject: Re: [PATCH 2/2] drm/doc: add rfc section for small BAR uapi
>
>
> On 2/18/22 12:22, Matthew Auld wrote:
> > Add an entry for the new uapi needed for small BAR on DG2+.
> >
> > Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> > Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> > Cc: Jon Bloomfield <jon.bloomfield@intel.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: Jordan Justen <jordan.l.justen@intel.com>
> > Cc: Kenneth Graunke <kenneth@whitecape.org>
> > Cc: mesa-dev@lists.freedesktop.org
> > ---
> > Documentation/gpu/rfc/i915_small_bar.h | 153
> +++++++++++++++++++++++
> > Documentation/gpu/rfc/i915_small_bar.rst | 40 ++++++
> > Documentation/gpu/rfc/index.rst | 4 +
> > 3 files changed, 197 insertions(+)
> > create mode 100644 Documentation/gpu/rfc/i915_small_bar.h
> > create mode 100644 Documentation/gpu/rfc/i915_small_bar.rst
> >
> > diff --git a/Documentation/gpu/rfc/i915_small_bar.h
> > b/Documentation/gpu/rfc/i915_small_bar.h
> > new file mode 100644
> > index 000000000000..fa65835fd608
> > --- /dev/null
> > +++ b/Documentation/gpu/rfc/i915_small_bar.h
> > @@ -0,0 +1,153 @@
> > +/**
> > + * struct __drm_i915_gem_create_ext - Existing gem_create behaviour,
> > +with added
> > + * extension support using struct i915_user_extension.
> > + *
> > + * Note that in the future we want to have our buffer flags here,
>
> Does this sentence need updating, with the flags member?
>
>
> > at least for
> > + * the stuff that is immutable. Previously we would have two ioctls,
> > +one to
> > + * create the object with gem_create, and another to apply various
> > +parameters,
> > + * however this creates some ambiguity for the params which are
> > +considered
> > + * immutable. Also in general we're phasing out the various SET/GET ioctls.
> > + */
> > +struct __drm_i915_gem_create_ext {
> > + /**
> > + * @size: Requested size for the object.
> > + *
> > + * The (page-aligned) allocated size for the object will be returned.
> > + *
> > + * Note that for some devices we have might have further minimum
> > + * page-size restrictions(larger than 4K), like for device local-memory.
> > + * However in general the final size here should always reflect any
> > + * rounding up, if for example using the
> I915_GEM_CREATE_EXT_MEMORY_REGIONS
> > + * extension to place the object in device local-memory.
> > + */
> > + __u64 size;
> > + /**
> > + * @handle: Returned handle for the object.
> > + *
> > + * Object handles are nonzero.
> > + */
> > + __u32 handle;
> > + /**
> > + * @flags: Optional flags.
> > + *
> > + * Supported values:
> > + *
> > + * I915_GEM_CREATE_EXT_FLAG_NEEDS_CPU_ACCESS - Signal to the
> kernel that
> > + * the object will need to be accessed via the CPU.
> > + *
> > + * Only valid when placing objects in I915_MEMORY_CLASS_DEVICE,
> and
> > + * only strictly required on platforms where only some of the device
> > + * memory is directly visible or mappable through the CPU, like on DG2+.
> > + *
> > + * One of the placements MUST also be I915_MEMORY_CLASS_SYSTEM,
> to
> > + * ensure we can always spill the allocation to system memory, if we
> > + * can't place the object in the mappable part of
> > + * I915_MEMORY_CLASS_DEVICE.
> > + *
> > + * Note that buffers that need to be captured with
> EXEC_OBJECT_CAPTURE,
> > + * will need to enable this hint, if the object can also be placed in
> > + * I915_MEMORY_CLASS_DEVICE, starting from DG2+. The execbuf call
> will
> > + * throw an error otherwise. This also means that such objects will need
> > + * I915_MEMORY_CLASS_SYSTEM set as a possible placement.
> > + *
> > + * Without this hint, the kernel will assume that non-mappable
> > + * I915_MEMORY_CLASS_DEVICE is preferred for this object. Note that
> the
> > + * kernel can still migrate the object to the mappable part, as a last
> > + * resort, if userspace ever CPU faults this object, but this might be
> > + * expensive, and so ideally should be avoided.
> > + */
> > +#define I915_GEM_CREATE_EXT_FLAG_NEEDS_CPU_ACCESS (1 << 0)
> > + __u32 flags;
> > + /**
> > + * @extensions: The chain of extensions to apply to this object.
> > + *
> > + * This will be useful in the future when we need to support several
> > + * different extensions, and we need to apply more than one when
> > + * creating the object. See struct i915_user_extension.
> > + *
> > + * If we don't supply any extensions then we get the same old
> gem_create
> > + * behaviour.
> > + *
> > + * For I915_GEM_CREATE_EXT_MEMORY_REGIONS usage see
> > + * struct drm_i915_gem_create_ext_memory_regions.
> > + *
> > + * For I915_GEM_CREATE_EXT_PROTECTED_CONTENT usage see
> > + * struct drm_i915_gem_create_ext_protected_content.
> > + */
> > +#define I915_GEM_CREATE_EXT_MEMORY_REGIONS 0 #define
> > +I915_GEM_CREATE_EXT_PROTECTED_CONTENT 1
> > + __u64 extensions;
> > +};
> > +
> > +#define DRM_I915_QUERY_VMA_INFO 5
> > +
> > +/**
> > + * struct __drm_i915_query_vma_info
> > + *
> > + * Given a vm and GTT address, lookup the corresponding vma,
> > +returning its set
> > + * of attributes.
> > + *
> > + * .. code-block:: C
> > + *
> > + * struct drm_i915_query_vma_info info = {};
> > + * struct drm_i915_query_item item = {
> > + * .data_ptr = (uintptr_t)&info,
> > + * .query_id = DRM_I915_QUERY_VMA_INFO,
> > + * };
> > + * struct drm_i915_query query = {
> > + * .num_items = 1,
> > + * .items_ptr = (uintptr_t)&item,
> > + * };
> > + * int err;
> > + *
> > + * // Unlike some other types of queries, there is no need to first query
> > + * // the size of the data_ptr blob here, since we already know ahead of
> > + * // time how big this needs to be.
> > + * item.length = sizeof(info);
> > + *
> > + * // Next we fill in the vm_id and ppGTT address of the vma we wish
> > + * // to query, before then firing off the query.
> > + * info.vm_id = vm_id;
> > + * info.offset = gtt_address;
> > + * err = ioctl(fd, DRM_IOCTL_I915_QUERY, &query);
> > + * if (err || item.length < 0) ...
> > + *
> > + * // If all went well we can now inspect the returned attributes.
> > + * if (info.attributes & DRM_I915_QUERY_VMA_INFO_CPU_VISIBLE) ...
> > + */
> > +struct __drm_i915_query_vma_info {
> > + /**
> > + * @vm_id: The given vm id that contains the vma. The id is the value
> > + * returned by the DRM_I915_GEM_VM_CREATE. See struct
> > + * drm_i915_gem_vm_control.vm_id.
> > + */
> > + __u32 vm_id;
> > + /** @pad: MBZ. */
> > + __u32 pad;
> > + /**
> > + * @offset: The corresponding ppGTT address of the vma which the
> kernel
> > + * will use to perform the lookup.
> > + */
> > + __u64 offset;
> > + /**
> > + * @attributes: The returned attributes for the given vma.
> > + *
> > + * Possible values:
> > + *
> > + * DRM_I915_QUERY_VMA_INFO_CPU_VISIBLE - Set if the pages
> backing the
> > + * vma are currently CPU accessible. If this is not set then the vma is
> > + * currently backed by I915_MEMORY_CLASS_DEVICE memory, which
> the CPU
> > + * cannot directly access(this is only possible on discrete devices with
> > + * a small BAR). Attempting to MMAP and fault such an object will
> > + * require the kernel first synchronising any GPU work tied to the
> > + * object, before then migrating the pages, either to the CPU accessible
> > + * part of I915_MEMORY_CLASS_DEVICE, or
> I915_MEMORY_CLASS_SYSTEM, if the
> > + * placements permit it. See
> I915_GEM_CREATE_EXT_FLAG_NEEDS_CPU_ACCESS.
> > + *
> > + * Note that this is inherently racy.
> > + */
> > +#define DRM_I915_QUERY_VMA_INFO_CPU_VISIBLE (1<<0)
> > + __u64 attributes;
> > + /** @rsvd: MBZ */
> > + __u32 rsvd[4];
> > +};
> > diff --git a/Documentation/gpu/rfc/i915_small_bar.rst
> > b/Documentation/gpu/rfc/i915_small_bar.rst
> > new file mode 100644
> > index 000000000000..fea92d3d69ab
> > --- /dev/null
> > +++ b/Documentation/gpu/rfc/i915_small_bar.rst
> > @@ -0,0 +1,40 @@
> > +==========================
> > +I915 Small BAR RFC Section
> > +==========================
> > +Starting from DG2 we will have resizable BAR support for device
> > +local-memory, but in some cases the final BAR size might still be
> > +smaller than the total local-memory size. In such cases only part of
> > +local-memory will be CPU accessible, while the remainder is only accessible
> via the GPU.
... In such cases only part of local-memory will be CPU accessible/ In such cases, only {small} part of local-memory will be CPU accessible?
> > +
> > +I915_GEM_CREATE_EXT_FLAG_NEEDS_CPU_ACCESS flag
> > +----------------------------------------------
> > +New gem_create_ext flag to tell the kernel that a BO will require CPU access.
> > +The becomes important when placing an object in LMEM, where
The/This?
> > +underneath the device has a small BAR, meaning only part of it is CPU
> > +accessible. Without this flag the kernel will assume that CPU access
> > +is not required, and prioritize using the non-CPU visible portion of LMEM(if
> present on the device).
> > +
> > +Related to this, we now also reject any objects marked with
> > +EXEC_OBJECT_CAPTURE, which are also not tagged with
> NEEDS_CPU_ACCESS.
> > +This only impacts DG2+.
> > +
> > +XXX: One open here is whether we should extend the memory region
> > +query to return the CPU visible size of the region. For now the IGTs
> > +just use debugfs to query the size. However, if userspace sees a real
> > +need for this then extending the region query would be a lot nicer.
>
> I guess UMD folks need to comment on this. Although I think since there might
> be a number of clients utilizing the mappable part, and a number of buffers
> pinned in there, I figure this might be of limited value outside of tests without
> some kind of cgroups support.
>
> Otherwise
>
> Acked-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
>
>
> > +
> > +.. kernel-doc:: Documentation/gpu/rfc/i915_small_bar.h
> > + :functions: __drm_i915_gem_create_ext
> > +
> > +DRM_I915_QUERY_VMA_INFO query
> > +-----------------------------
> > +Query the attributes of some vma. Given a vm and GTT offset, find the
> > +respective vma, and return its set of attrubutes. For now we only
Attrubutes/attributes?
You might want to read the document again, and fix punctuations in addition to those typos - at least to make it more legible...
Thanks,
~Akeem
> > +support DRM_I915_QUERY_VMA_INFO_CPU_VISIBLE, which is set if the
> > +object/vma is currently placed in memory that is accessible by the
> > +CPU. This should always be set on devices where the CPU visible size
> > +of LMEM matches the probed size. If this is not set then CPU faulting
> > +the object will first require migrating the pages.
> > +
> > +.. kernel-doc:: Documentation/gpu/rfc/i915_small_bar.h
> > + :functions: __drm_i915_query_vma_info
> > diff --git a/Documentation/gpu/rfc/index.rst
> > b/Documentation/gpu/rfc/index.rst index 018a8bf317a6..5b8495bdc1fd
> > 100644
> > --- a/Documentation/gpu/rfc/index.rst
> > +++ b/Documentation/gpu/rfc/index.rst
> > @@ -19,3 +19,7 @@ host such documentation:
> > .. toctree::
> >
> > i915_scheduler.rst
> > +
> > +.. toctree::
> > +
> > + i915_small_bar.rst
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Intel-gfx] [PATCH 2/2] drm/doc: add rfc section for small BAR uapi
@ 2022-02-22 17:39 ` Abodunrin, Akeem G
0 siblings, 0 replies; 15+ messages in thread
From: Abodunrin, Akeem G @ 2022-02-22 17:39 UTC (permalink / raw)
To: Thomas Hellström, Auld, Matthew, intel-gfx
Cc: Daniel Vetter, dri-devel, Kenneth Graunke, mesa-dev
> -----Original Message-----
> From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of
> Thomas Hellström
> Sent: Tuesday, February 22, 2022 2:36 AM
> To: Auld, Matthew <matthew.auld@intel.com>; intel-gfx@lists.freedesktop.org
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>; dri-devel@lists.freedesktop.org;
> Kenneth Graunke <kenneth@whitecape.org>; Bloomfield, Jon
> <jon.bloomfield@intel.com>; Justen, Jordan L <jordan.l.justen@intel.com>;
> mesa-dev@lists.freedesktop.org
> Subject: Re: [PATCH 2/2] drm/doc: add rfc section for small BAR uapi
>
>
> On 2/18/22 12:22, Matthew Auld wrote:
> > Add an entry for the new uapi needed for small BAR on DG2+.
> >
> > Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> > Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> > Cc: Jon Bloomfield <jon.bloomfield@intel.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: Jordan Justen <jordan.l.justen@intel.com>
> > Cc: Kenneth Graunke <kenneth@whitecape.org>
> > Cc: mesa-dev@lists.freedesktop.org
> > ---
> > Documentation/gpu/rfc/i915_small_bar.h | 153
> +++++++++++++++++++++++
> > Documentation/gpu/rfc/i915_small_bar.rst | 40 ++++++
> > Documentation/gpu/rfc/index.rst | 4 +
> > 3 files changed, 197 insertions(+)
> > create mode 100644 Documentation/gpu/rfc/i915_small_bar.h
> > create mode 100644 Documentation/gpu/rfc/i915_small_bar.rst
> >
> > diff --git a/Documentation/gpu/rfc/i915_small_bar.h
> > b/Documentation/gpu/rfc/i915_small_bar.h
> > new file mode 100644
> > index 000000000000..fa65835fd608
> > --- /dev/null
> > +++ b/Documentation/gpu/rfc/i915_small_bar.h
> > @@ -0,0 +1,153 @@
> > +/**
> > + * struct __drm_i915_gem_create_ext - Existing gem_create behaviour,
> > +with added
> > + * extension support using struct i915_user_extension.
> > + *
> > + * Note that in the future we want to have our buffer flags here,
>
> Does this sentence need updating, with the flags member?
>
>
> > at least for
> > + * the stuff that is immutable. Previously we would have two ioctls,
> > +one to
> > + * create the object with gem_create, and another to apply various
> > +parameters,
> > + * however this creates some ambiguity for the params which are
> > +considered
> > + * immutable. Also in general we're phasing out the various SET/GET ioctls.
> > + */
> > +struct __drm_i915_gem_create_ext {
> > + /**
> > + * @size: Requested size for the object.
> > + *
> > + * The (page-aligned) allocated size for the object will be returned.
> > + *
> > + * Note that for some devices we have might have further minimum
> > + * page-size restrictions(larger than 4K), like for device local-memory.
> > + * However in general the final size here should always reflect any
> > + * rounding up, if for example using the
> I915_GEM_CREATE_EXT_MEMORY_REGIONS
> > + * extension to place the object in device local-memory.
> > + */
> > + __u64 size;
> > + /**
> > + * @handle: Returned handle for the object.
> > + *
> > + * Object handles are nonzero.
> > + */
> > + __u32 handle;
> > + /**
> > + * @flags: Optional flags.
> > + *
> > + * Supported values:
> > + *
> > + * I915_GEM_CREATE_EXT_FLAG_NEEDS_CPU_ACCESS - Signal to the
> kernel that
> > + * the object will need to be accessed via the CPU.
> > + *
> > + * Only valid when placing objects in I915_MEMORY_CLASS_DEVICE,
> and
> > + * only strictly required on platforms where only some of the device
> > + * memory is directly visible or mappable through the CPU, like on DG2+.
> > + *
> > + * One of the placements MUST also be I915_MEMORY_CLASS_SYSTEM,
> to
> > + * ensure we can always spill the allocation to system memory, if we
> > + * can't place the object in the mappable part of
> > + * I915_MEMORY_CLASS_DEVICE.
> > + *
> > + * Note that buffers that need to be captured with
> EXEC_OBJECT_CAPTURE,
> > + * will need to enable this hint, if the object can also be placed in
> > + * I915_MEMORY_CLASS_DEVICE, starting from DG2+. The execbuf call
> will
> > + * throw an error otherwise. This also means that such objects will need
> > + * I915_MEMORY_CLASS_SYSTEM set as a possible placement.
> > + *
> > + * Without this hint, the kernel will assume that non-mappable
> > + * I915_MEMORY_CLASS_DEVICE is preferred for this object. Note that
> the
> > + * kernel can still migrate the object to the mappable part, as a last
> > + * resort, if userspace ever CPU faults this object, but this might be
> > + * expensive, and so ideally should be avoided.
> > + */
> > +#define I915_GEM_CREATE_EXT_FLAG_NEEDS_CPU_ACCESS (1 << 0)
> > + __u32 flags;
> > + /**
> > + * @extensions: The chain of extensions to apply to this object.
> > + *
> > + * This will be useful in the future when we need to support several
> > + * different extensions, and we need to apply more than one when
> > + * creating the object. See struct i915_user_extension.
> > + *
> > + * If we don't supply any extensions then we get the same old
> gem_create
> > + * behaviour.
> > + *
> > + * For I915_GEM_CREATE_EXT_MEMORY_REGIONS usage see
> > + * struct drm_i915_gem_create_ext_memory_regions.
> > + *
> > + * For I915_GEM_CREATE_EXT_PROTECTED_CONTENT usage see
> > + * struct drm_i915_gem_create_ext_protected_content.
> > + */
> > +#define I915_GEM_CREATE_EXT_MEMORY_REGIONS 0 #define
> > +I915_GEM_CREATE_EXT_PROTECTED_CONTENT 1
> > + __u64 extensions;
> > +};
> > +
> > +#define DRM_I915_QUERY_VMA_INFO 5
> > +
> > +/**
> > + * struct __drm_i915_query_vma_info
> > + *
> > + * Given a vm and GTT address, lookup the corresponding vma,
> > +returning its set
> > + * of attributes.
> > + *
> > + * .. code-block:: C
> > + *
> > + * struct drm_i915_query_vma_info info = {};
> > + * struct drm_i915_query_item item = {
> > + * .data_ptr = (uintptr_t)&info,
> > + * .query_id = DRM_I915_QUERY_VMA_INFO,
> > + * };
> > + * struct drm_i915_query query = {
> > + * .num_items = 1,
> > + * .items_ptr = (uintptr_t)&item,
> > + * };
> > + * int err;
> > + *
> > + * // Unlike some other types of queries, there is no need to first query
> > + * // the size of the data_ptr blob here, since we already know ahead of
> > + * // time how big this needs to be.
> > + * item.length = sizeof(info);
> > + *
> > + * // Next we fill in the vm_id and ppGTT address of the vma we wish
> > + * // to query, before then firing off the query.
> > + * info.vm_id = vm_id;
> > + * info.offset = gtt_address;
> > + * err = ioctl(fd, DRM_IOCTL_I915_QUERY, &query);
> > + * if (err || item.length < 0) ...
> > + *
> > + * // If all went well we can now inspect the returned attributes.
> > + * if (info.attributes & DRM_I915_QUERY_VMA_INFO_CPU_VISIBLE) ...
> > + */
> > +struct __drm_i915_query_vma_info {
> > + /**
> > + * @vm_id: The given vm id that contains the vma. The id is the value
> > + * returned by the DRM_I915_GEM_VM_CREATE. See struct
> > + * drm_i915_gem_vm_control.vm_id.
> > + */
> > + __u32 vm_id;
> > + /** @pad: MBZ. */
> > + __u32 pad;
> > + /**
> > + * @offset: The corresponding ppGTT address of the vma which the
> kernel
> > + * will use to perform the lookup.
> > + */
> > + __u64 offset;
> > + /**
> > + * @attributes: The returned attributes for the given vma.
> > + *
> > + * Possible values:
> > + *
> > + * DRM_I915_QUERY_VMA_INFO_CPU_VISIBLE - Set if the pages
> backing the
> > + * vma are currently CPU accessible. If this is not set then the vma is
> > + * currently backed by I915_MEMORY_CLASS_DEVICE memory, which
> the CPU
> > + * cannot directly access(this is only possible on discrete devices with
> > + * a small BAR). Attempting to MMAP and fault such an object will
> > + * require the kernel first synchronising any GPU work tied to the
> > + * object, before then migrating the pages, either to the CPU accessible
> > + * part of I915_MEMORY_CLASS_DEVICE, or
> I915_MEMORY_CLASS_SYSTEM, if the
> > + * placements permit it. See
> I915_GEM_CREATE_EXT_FLAG_NEEDS_CPU_ACCESS.
> > + *
> > + * Note that this is inherently racy.
> > + */
> > +#define DRM_I915_QUERY_VMA_INFO_CPU_VISIBLE (1<<0)
> > + __u64 attributes;
> > + /** @rsvd: MBZ */
> > + __u32 rsvd[4];
> > +};
> > diff --git a/Documentation/gpu/rfc/i915_small_bar.rst
> > b/Documentation/gpu/rfc/i915_small_bar.rst
> > new file mode 100644
> > index 000000000000..fea92d3d69ab
> > --- /dev/null
> > +++ b/Documentation/gpu/rfc/i915_small_bar.rst
> > @@ -0,0 +1,40 @@
> > +==========================
> > +I915 Small BAR RFC Section
> > +==========================
> > +Starting from DG2 we will have resizable BAR support for device
> > +local-memory, but in some cases the final BAR size might still be
> > +smaller than the total local-memory size. In such cases only part of
> > +local-memory will be CPU accessible, while the remainder is only accessible
> via the GPU.
... In such cases only part of local-memory will be CPU accessible/ In such cases, only {small} part of local-memory will be CPU accessible?
> > +
> > +I915_GEM_CREATE_EXT_FLAG_NEEDS_CPU_ACCESS flag
> > +----------------------------------------------
> > +New gem_create_ext flag to tell the kernel that a BO will require CPU access.
> > +The becomes important when placing an object in LMEM, where
The/This?
> > +underneath the device has a small BAR, meaning only part of it is CPU
> > +accessible. Without this flag the kernel will assume that CPU access
> > +is not required, and prioritize using the non-CPU visible portion of LMEM(if
> present on the device).
> > +
> > +Related to this, we now also reject any objects marked with
> > +EXEC_OBJECT_CAPTURE, which are also not tagged with
> NEEDS_CPU_ACCESS.
> > +This only impacts DG2+.
> > +
> > +XXX: One open here is whether we should extend the memory region
> > +query to return the CPU visible size of the region. For now the IGTs
> > +just use debugfs to query the size. However, if userspace sees a real
> > +need for this then extending the region query would be a lot nicer.
>
> I guess UMD folks need to comment on this. Although I think since there might
> be a number of clients utilizing the mappable part, and a number of buffers
> pinned in there, I figure this might be of limited value outside of tests without
> some kind of cgroups support.
>
> Otherwise
>
> Acked-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
>
>
> > +
> > +.. kernel-doc:: Documentation/gpu/rfc/i915_small_bar.h
> > + :functions: __drm_i915_gem_create_ext
> > +
> > +DRM_I915_QUERY_VMA_INFO query
> > +-----------------------------
> > +Query the attributes of some vma. Given a vm and GTT offset, find the
> > +respective vma, and return its set of attrubutes. For now we only
Attrubutes/attributes?
You might want to read the document again, and fix punctuations in addition to those typos - at least to make it more legible...
Thanks,
~Akeem
> > +support DRM_I915_QUERY_VMA_INFO_CPU_VISIBLE, which is set if the
> > +object/vma is currently placed in memory that is accessible by the
> > +CPU. This should always be set on devices where the CPU visible size
> > +of LMEM matches the probed size. If this is not set then CPU faulting
> > +the object will first require migrating the pages.
> > +
> > +.. kernel-doc:: Documentation/gpu/rfc/i915_small_bar.h
> > + :functions: __drm_i915_query_vma_info
> > diff --git a/Documentation/gpu/rfc/index.rst
> > b/Documentation/gpu/rfc/index.rst index 018a8bf317a6..5b8495bdc1fd
> > 100644
> > --- a/Documentation/gpu/rfc/index.rst
> > +++ b/Documentation/gpu/rfc/index.rst
> > @@ -19,3 +19,7 @@ host such documentation:
> > .. toctree::
> >
> > i915_scheduler.rst
> > +
> > +.. toctree::
> > +
> > + i915_small_bar.rst
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Intel-gfx] [PATCH 2/2] drm/doc: add rfc section for small BAR uapi
2022-02-18 11:22 ` [Intel-gfx] " Matthew Auld
(?)
(?)
@ 2022-03-18 9:38 ` Lionel Landwerlin
2022-03-18 10:21 ` Matthew Auld
-1 siblings, 1 reply; 15+ messages in thread
From: Lionel Landwerlin @ 2022-03-18 9:38 UTC (permalink / raw)
To: Matthew Auld, intel-gfx
Cc: Thomas Hellström, Kenneth Graunke, mesa-dev, dri-devel,
Daniel Vetter
Hey Matthew, all,
This sounds like a good thing to have.
There are a number of DG2 machines where we have a small BAR and this is
causing more apps to fail.
Anv currently reports 3 memory heaps to the app :
- local device only (not host visible) -> mapped to lmem
- device/cpu -> mapped to smem
- local device but also host visible -> mapped to lmem
So we could use this straight away, by just not putting the
I915_GEM_CREATE_EXT_FLAG_NEEDS_CPU_ACCESS flag on the allocation of the
first heap.
One thing I don't see in this proposal is how can we get the size of the
2 lmem heap : cpu visible, cpu not visible
We could use that to report the appropriate size to the app.
We probably want to report a new drm_i915_memory_region_info and either :
- put one of the reserve field to use to indicate : cpu visible
- or define a new enum value in drm_i915_gem_memory_class
Cheers,
-Lionel
On 18/02/2022 13:22, Matthew Auld wrote:
> Add an entry for the new uapi needed for small BAR on DG2+.
>
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> Cc: Jon Bloomfield <jon.bloomfield@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Kenneth Graunke <kenneth@whitecape.org>
> Cc: mesa-dev@lists.freedesktop.org
> ---
> Documentation/gpu/rfc/i915_small_bar.h | 153 +++++++++++++++++++++++
> Documentation/gpu/rfc/i915_small_bar.rst | 40 ++++++
> Documentation/gpu/rfc/index.rst | 4 +
> 3 files changed, 197 insertions(+)
> create mode 100644 Documentation/gpu/rfc/i915_small_bar.h
> create mode 100644 Documentation/gpu/rfc/i915_small_bar.rst
>
> diff --git a/Documentation/gpu/rfc/i915_small_bar.h b/Documentation/gpu/rfc/i915_small_bar.h
> new file mode 100644
> index 000000000000..fa65835fd608
> --- /dev/null
> +++ b/Documentation/gpu/rfc/i915_small_bar.h
> @@ -0,0 +1,153 @@
> +/**
> + * struct __drm_i915_gem_create_ext - Existing gem_create behaviour, with added
> + * extension support using struct i915_user_extension.
> + *
> + * Note that in the future we want to have our buffer flags here, at least for
> + * the stuff that is immutable. Previously we would have two ioctls, one to
> + * create the object with gem_create, and another to apply various parameters,
> + * however this creates some ambiguity for the params which are considered
> + * immutable. Also in general we're phasing out the various SET/GET ioctls.
> + */
> +struct __drm_i915_gem_create_ext {
> + /**
> + * @size: Requested size for the object.
> + *
> + * The (page-aligned) allocated size for the object will be returned.
> + *
> + * Note that for some devices we have might have further minimum
> + * page-size restrictions(larger than 4K), like for device local-memory.
> + * However in general the final size here should always reflect any
> + * rounding up, if for example using the I915_GEM_CREATE_EXT_MEMORY_REGIONS
> + * extension to place the object in device local-memory.
> + */
> + __u64 size;
> + /**
> + * @handle: Returned handle for the object.
> + *
> + * Object handles are nonzero.
> + */
> + __u32 handle;
> + /**
> + * @flags: Optional flags.
> + *
> + * Supported values:
> + *
> + * I915_GEM_CREATE_EXT_FLAG_NEEDS_CPU_ACCESS - Signal to the kernel that
> + * the object will need to be accessed via the CPU.
> + *
> + * Only valid when placing objects in I915_MEMORY_CLASS_DEVICE, and
> + * only strictly required on platforms where only some of the device
> + * memory is directly visible or mappable through the CPU, like on DG2+.
> + *
> + * One of the placements MUST also be I915_MEMORY_CLASS_SYSTEM, to
> + * ensure we can always spill the allocation to system memory, if we
> + * can't place the object in the mappable part of
> + * I915_MEMORY_CLASS_DEVICE.
> + *
> + * Note that buffers that need to be captured with EXEC_OBJECT_CAPTURE,
> + * will need to enable this hint, if the object can also be placed in
> + * I915_MEMORY_CLASS_DEVICE, starting from DG2+. The execbuf call will
> + * throw an error otherwise. This also means that such objects will need
> + * I915_MEMORY_CLASS_SYSTEM set as a possible placement.
> + *
> + * Without this hint, the kernel will assume that non-mappable
> + * I915_MEMORY_CLASS_DEVICE is preferred for this object. Note that the
> + * kernel can still migrate the object to the mappable part, as a last
> + * resort, if userspace ever CPU faults this object, but this might be
> + * expensive, and so ideally should be avoided.
> + */
> +#define I915_GEM_CREATE_EXT_FLAG_NEEDS_CPU_ACCESS (1 << 0)
> + __u32 flags;
> + /**
> + * @extensions: The chain of extensions to apply to this object.
> + *
> + * This will be useful in the future when we need to support several
> + * different extensions, and we need to apply more than one when
> + * creating the object. See struct i915_user_extension.
> + *
> + * If we don't supply any extensions then we get the same old gem_create
> + * behaviour.
> + *
> + * For I915_GEM_CREATE_EXT_MEMORY_REGIONS usage see
> + * struct drm_i915_gem_create_ext_memory_regions.
> + *
> + * For I915_GEM_CREATE_EXT_PROTECTED_CONTENT usage see
> + * struct drm_i915_gem_create_ext_protected_content.
> + */
> +#define I915_GEM_CREATE_EXT_MEMORY_REGIONS 0
> +#define I915_GEM_CREATE_EXT_PROTECTED_CONTENT 1
> + __u64 extensions;
> +};
> +
> +#define DRM_I915_QUERY_VMA_INFO 5
> +
> +/**
> + * struct __drm_i915_query_vma_info
> + *
> + * Given a vm and GTT address, lookup the corresponding vma, returning its set
> + * of attributes.
> + *
> + * .. code-block:: C
> + *
> + * struct drm_i915_query_vma_info info = {};
> + * struct drm_i915_query_item item = {
> + * .data_ptr = (uintptr_t)&info,
> + * .query_id = DRM_I915_QUERY_VMA_INFO,
> + * };
> + * struct drm_i915_query query = {
> + * .num_items = 1,
> + * .items_ptr = (uintptr_t)&item,
> + * };
> + * int err;
> + *
> + * // Unlike some other types of queries, there is no need to first query
> + * // the size of the data_ptr blob here, since we already know ahead of
> + * // time how big this needs to be.
> + * item.length = sizeof(info);
> + *
> + * // Next we fill in the vm_id and ppGTT address of the vma we wish
> + * // to query, before then firing off the query.
> + * info.vm_id = vm_id;
> + * info.offset = gtt_address;
> + * err = ioctl(fd, DRM_IOCTL_I915_QUERY, &query);
> + * if (err || item.length < 0) ...
> + *
> + * // If all went well we can now inspect the returned attributes.
> + * if (info.attributes & DRM_I915_QUERY_VMA_INFO_CPU_VISIBLE) ...
> + */
> +struct __drm_i915_query_vma_info {
> + /**
> + * @vm_id: The given vm id that contains the vma. The id is the value
> + * returned by the DRM_I915_GEM_VM_CREATE. See struct
> + * drm_i915_gem_vm_control.vm_id.
> + */
> + __u32 vm_id;
> + /** @pad: MBZ. */
> + __u32 pad;
> + /**
> + * @offset: The corresponding ppGTT address of the vma which the kernel
> + * will use to perform the lookup.
> + */
> + __u64 offset;
> + /**
> + * @attributes: The returned attributes for the given vma.
> + *
> + * Possible values:
> + *
> + * DRM_I915_QUERY_VMA_INFO_CPU_VISIBLE - Set if the pages backing the
> + * vma are currently CPU accessible. If this is not set then the vma is
> + * currently backed by I915_MEMORY_CLASS_DEVICE memory, which the CPU
> + * cannot directly access(this is only possible on discrete devices with
> + * a small BAR). Attempting to MMAP and fault such an object will
> + * require the kernel first synchronising any GPU work tied to the
> + * object, before then migrating the pages, either to the CPU accessible
> + * part of I915_MEMORY_CLASS_DEVICE, or I915_MEMORY_CLASS_SYSTEM, if the
> + * placements permit it. See I915_GEM_CREATE_EXT_FLAG_NEEDS_CPU_ACCESS.
> + *
> + * Note that this is inherently racy.
> + */
> +#define DRM_I915_QUERY_VMA_INFO_CPU_VISIBLE (1<<0)
> + __u64 attributes;
> + /** @rsvd: MBZ */
> + __u32 rsvd[4];
> +};
> diff --git a/Documentation/gpu/rfc/i915_small_bar.rst b/Documentation/gpu/rfc/i915_small_bar.rst
> new file mode 100644
> index 000000000000..fea92d3d69ab
> --- /dev/null
> +++ b/Documentation/gpu/rfc/i915_small_bar.rst
> @@ -0,0 +1,40 @@
> +==========================
> +I915 Small BAR RFC Section
> +==========================
> +Starting from DG2 we will have resizable BAR support for device local-memory,
> +but in some cases the final BAR size might still be smaller than the total
> +local-memory size. In such cases only part of local-memory will be CPU
> +accessible, while the remainder is only accessible via the GPU.
> +
> +I915_GEM_CREATE_EXT_FLAG_NEEDS_CPU_ACCESS flag
> +----------------------------------------------
> +New gem_create_ext flag to tell the kernel that a BO will require CPU access.
> +The becomes important when placing an object in LMEM, where underneath the
> +device has a small BAR, meaning only part of it is CPU accessible. Without this
> +flag the kernel will assume that CPU access is not required, and prioritize
> +using the non-CPU visible portion of LMEM(if present on the device).
> +
> +Related to this, we now also reject any objects marked with
> +EXEC_OBJECT_CAPTURE, which are also not tagged with NEEDS_CPU_ACCESS. This only
> +impacts DG2+.
> +
> +XXX: One open here is whether we should extend the memory region query to return
> +the CPU visible size of the region. For now the IGTs just use debugfs to query
> +the size. However, if userspace sees a real need for this then extending the
> +region query would be a lot nicer.
> +
> +.. kernel-doc:: Documentation/gpu/rfc/i915_small_bar.h
> + :functions: __drm_i915_gem_create_ext
> +
> +DRM_I915_QUERY_VMA_INFO query
> +-----------------------------
> +Query the attributes of some vma. Given a vm and GTT offset, find the
> +respective vma, and return its set of attrubutes. For now we only support
> +DRM_I915_QUERY_VMA_INFO_CPU_VISIBLE, which is set if the object/vma is
> +currently placed in memory that is accessible by the CPU. This should always be
> +set on devices where the CPU visible size of LMEM matches the probed size. If
> +this is not set then CPU faulting the object will first require migrating the
> +pages.
> +
> +.. kernel-doc:: Documentation/gpu/rfc/i915_small_bar.h
> + :functions: __drm_i915_query_vma_info
> diff --git a/Documentation/gpu/rfc/index.rst b/Documentation/gpu/rfc/index.rst
> index 018a8bf317a6..5b8495bdc1fd 100644
> --- a/Documentation/gpu/rfc/index.rst
> +++ b/Documentation/gpu/rfc/index.rst
> @@ -19,3 +19,7 @@ host such documentation:
> .. toctree::
>
> i915_scheduler.rst
> +
> +.. toctree::
> +
> + i915_small_bar.rst
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Intel-gfx] [PATCH 2/2] drm/doc: add rfc section for small BAR uapi
2022-03-18 9:38 ` Lionel Landwerlin
@ 2022-03-18 10:21 ` Matthew Auld
0 siblings, 0 replies; 15+ messages in thread
From: Matthew Auld @ 2022-03-18 10:21 UTC (permalink / raw)
To: Lionel Landwerlin, intel-gfx
Cc: Thomas Hellström, Kenneth Graunke, mesa-dev, dri-devel,
Daniel Vetter
On 18/03/2022 09:38, Lionel Landwerlin wrote:
> Hey Matthew, all,
>
> This sounds like a good thing to have.
> There are a number of DG2 machines where we have a small BAR and this is
> causing more apps to fail.
>
> Anv currently reports 3 memory heaps to the app :
>
> - local device only (not host visible) -> mapped to lmem
> - device/cpu -> mapped to smem
> - local device but also host visible -> mapped to lmem
>
> So we could use this straight away, by just not putting the
> I915_GEM_CREATE_EXT_FLAG_NEEDS_CPU_ACCESS flag on the allocation of the
> first heap.
>
> One thing I don't see in this proposal is how can we get the size of the
> 2 lmem heap : cpu visible, cpu not visible
> We could use that to report the appropriate size to the app.
> We probably want to report a new drm_i915_memory_region_info and either :
> - put one of the reserve field to use to indicate : cpu visible
> - or define a new enum value in drm_i915_gem_memory_class
Thanks for taking a look at this. Returning the probed CPU visible size
as part of the region query seems reasonable. Something like:
@@ -3074,8 +3074,18 @@ struct drm_i915_memory_region_info {
/** @unallocated_size: Estimate of memory remaining (-1 =
unknown) */
__u64 unallocated_size;
- /** @rsvd1: MBZ */
- __u64 rsvd1[8];
+ union {
+ /** @rsvd1: MBZ */
+ __u64 rsvd1[8];
+
+ struct {
+ /**
+ * @probed_cpu_visible_size: Memory probed by
the driver
+ * that is CPU accessible. (-1 = unknown)
+ */
+ __u64 probed_cpu_visible_size;
+ };
+ };
I will add this in the next version, if no objections.
>
> Cheers,
>
> -Lionel
>
>
> On 18/02/2022 13:22, Matthew Auld wrote:
>> Add an entry for the new uapi needed for small BAR on DG2+.
>>
>> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
>> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
>> Cc: Jon Bloomfield <jon.bloomfield@intel.com>
>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>> Cc: Kenneth Graunke <kenneth@whitecape.org>
>> Cc: mesa-dev@lists.freedesktop.org
>> ---
>> Documentation/gpu/rfc/i915_small_bar.h | 153 +++++++++++++++++++++++
>> Documentation/gpu/rfc/i915_small_bar.rst | 40 ++++++
>> Documentation/gpu/rfc/index.rst | 4 +
>> 3 files changed, 197 insertions(+)
>> create mode 100644 Documentation/gpu/rfc/i915_small_bar.h
>> create mode 100644 Documentation/gpu/rfc/i915_small_bar.rst
>>
>> diff --git a/Documentation/gpu/rfc/i915_small_bar.h
>> b/Documentation/gpu/rfc/i915_small_bar.h
>> new file mode 100644
>> index 000000000000..fa65835fd608
>> --- /dev/null
>> +++ b/Documentation/gpu/rfc/i915_small_bar.h
>> @@ -0,0 +1,153 @@
>> +/**
>> + * struct __drm_i915_gem_create_ext - Existing gem_create behaviour,
>> with added
>> + * extension support using struct i915_user_extension.
>> + *
>> + * Note that in the future we want to have our buffer flags here, at
>> least for
>> + * the stuff that is immutable. Previously we would have two ioctls,
>> one to
>> + * create the object with gem_create, and another to apply various
>> parameters,
>> + * however this creates some ambiguity for the params which are
>> considered
>> + * immutable. Also in general we're phasing out the various SET/GET
>> ioctls.
>> + */
>> +struct __drm_i915_gem_create_ext {
>> + /**
>> + * @size: Requested size for the object.
>> + *
>> + * The (page-aligned) allocated size for the object will be
>> returned.
>> + *
>> + * Note that for some devices we have might have further minimum
>> + * page-size restrictions(larger than 4K), like for device
>> local-memory.
>> + * However in general the final size here should always reflect any
>> + * rounding up, if for example using the
>> I915_GEM_CREATE_EXT_MEMORY_REGIONS
>> + * extension to place the object in device local-memory.
>> + */
>> + __u64 size;
>> + /**
>> + * @handle: Returned handle for the object.
>> + *
>> + * Object handles are nonzero.
>> + */
>> + __u32 handle;
>> + /**
>> + * @flags: Optional flags.
>> + *
>> + * Supported values:
>> + *
>> + * I915_GEM_CREATE_EXT_FLAG_NEEDS_CPU_ACCESS - Signal to the
>> kernel that
>> + * the object will need to be accessed via the CPU.
>> + *
>> + * Only valid when placing objects in I915_MEMORY_CLASS_DEVICE, and
>> + * only strictly required on platforms where only some of the device
>> + * memory is directly visible or mappable through the CPU, like
>> on DG2+.
>> + *
>> + * One of the placements MUST also be I915_MEMORY_CLASS_SYSTEM, to
>> + * ensure we can always spill the allocation to system memory, if we
>> + * can't place the object in the mappable part of
>> + * I915_MEMORY_CLASS_DEVICE.
>> + *
>> + * Note that buffers that need to be captured with
>> EXEC_OBJECT_CAPTURE,
>> + * will need to enable this hint, if the object can also be
>> placed in
>> + * I915_MEMORY_CLASS_DEVICE, starting from DG2+. The execbuf call
>> will
>> + * throw an error otherwise. This also means that such objects
>> will need
>> + * I915_MEMORY_CLASS_SYSTEM set as a possible placement.
>> + *
>> + * Without this hint, the kernel will assume that non-mappable
>> + * I915_MEMORY_CLASS_DEVICE is preferred for this object. Note
>> that the
>> + * kernel can still migrate the object to the mappable part, as a
>> last
>> + * resort, if userspace ever CPU faults this object, but this
>> might be
>> + * expensive, and so ideally should be avoided.
>> + */
>> +#define I915_GEM_CREATE_EXT_FLAG_NEEDS_CPU_ACCESS (1 << 0)
>> + __u32 flags;
>> + /**
>> + * @extensions: The chain of extensions to apply to this object.
>> + *
>> + * This will be useful in the future when we need to support several
>> + * different extensions, and we need to apply more than one when
>> + * creating the object. See struct i915_user_extension.
>> + *
>> + * If we don't supply any extensions then we get the same old
>> gem_create
>> + * behaviour.
>> + *
>> + * For I915_GEM_CREATE_EXT_MEMORY_REGIONS usage see
>> + * struct drm_i915_gem_create_ext_memory_regions.
>> + *
>> + * For I915_GEM_CREATE_EXT_PROTECTED_CONTENT usage see
>> + * struct drm_i915_gem_create_ext_protected_content.
>> + */
>> +#define I915_GEM_CREATE_EXT_MEMORY_REGIONS 0
>> +#define I915_GEM_CREATE_EXT_PROTECTED_CONTENT 1
>> + __u64 extensions;
>> +};
>> +
>> +#define DRM_I915_QUERY_VMA_INFO 5
>> +
>> +/**
>> + * struct __drm_i915_query_vma_info
>> + *
>> + * Given a vm and GTT address, lookup the corresponding vma,
>> returning its set
>> + * of attributes.
>> + *
>> + * .. code-block:: C
>> + *
>> + * struct drm_i915_query_vma_info info = {};
>> + * struct drm_i915_query_item item = {
>> + * .data_ptr = (uintptr_t)&info,
>> + * .query_id = DRM_I915_QUERY_VMA_INFO,
>> + * };
>> + * struct drm_i915_query query = {
>> + * .num_items = 1,
>> + * .items_ptr = (uintptr_t)&item,
>> + * };
>> + * int err;
>> + *
>> + * // Unlike some other types of queries, there is no need to
>> first query
>> + * // the size of the data_ptr blob here, since we already know
>> ahead of
>> + * // time how big this needs to be.
>> + * item.length = sizeof(info);
>> + *
>> + * // Next we fill in the vm_id and ppGTT address of the vma we wish
>> + * // to query, before then firing off the query.
>> + * info.vm_id = vm_id;
>> + * info.offset = gtt_address;
>> + * err = ioctl(fd, DRM_IOCTL_I915_QUERY, &query);
>> + * if (err || item.length < 0) ...
>> + *
>> + * // If all went well we can now inspect the returned attributes.
>> + * if (info.attributes & DRM_I915_QUERY_VMA_INFO_CPU_VISIBLE) ...
>> + */
>> +struct __drm_i915_query_vma_info {
>> + /**
>> + * @vm_id: The given vm id that contains the vma. The id is the
>> value
>> + * returned by the DRM_I915_GEM_VM_CREATE. See struct
>> + * drm_i915_gem_vm_control.vm_id.
>> + */
>> + __u32 vm_id;
>> + /** @pad: MBZ. */
>> + __u32 pad;
>> + /**
>> + * @offset: The corresponding ppGTT address of the vma which the
>> kernel
>> + * will use to perform the lookup.
>> + */
>> + __u64 offset;
>> + /**
>> + * @attributes: The returned attributes for the given vma.
>> + *
>> + * Possible values:
>> + *
>> + * DRM_I915_QUERY_VMA_INFO_CPU_VISIBLE - Set if the pages backing
>> the
>> + * vma are currently CPU accessible. If this is not set then the
>> vma is
>> + * currently backed by I915_MEMORY_CLASS_DEVICE memory, which the
>> CPU
>> + * cannot directly access(this is only possible on discrete
>> devices with
>> + * a small BAR). Attempting to MMAP and fault such an object will
>> + * require the kernel first synchronising any GPU work tied to the
>> + * object, before then migrating the pages, either to the CPU
>> accessible
>> + * part of I915_MEMORY_CLASS_DEVICE, or I915_MEMORY_CLASS_SYSTEM,
>> if the
>> + * placements permit it. See
>> I915_GEM_CREATE_EXT_FLAG_NEEDS_CPU_ACCESS.
>> + *
>> + * Note that this is inherently racy.
>> + */
>> +#define DRM_I915_QUERY_VMA_INFO_CPU_VISIBLE (1<<0)
>> + __u64 attributes;
>> + /** @rsvd: MBZ */
>> + __u32 rsvd[4];
>> +};
>> diff --git a/Documentation/gpu/rfc/i915_small_bar.rst
>> b/Documentation/gpu/rfc/i915_small_bar.rst
>> new file mode 100644
>> index 000000000000..fea92d3d69ab
>> --- /dev/null
>> +++ b/Documentation/gpu/rfc/i915_small_bar.rst
>> @@ -0,0 +1,40 @@
>> +==========================
>> +I915 Small BAR RFC Section
>> +==========================
>> +Starting from DG2 we will have resizable BAR support for device
>> local-memory,
>> +but in some cases the final BAR size might still be smaller than the
>> total
>> +local-memory size. In such cases only part of local-memory will be CPU
>> +accessible, while the remainder is only accessible via the GPU.
>> +
>> +I915_GEM_CREATE_EXT_FLAG_NEEDS_CPU_ACCESS flag
>> +----------------------------------------------
>> +New gem_create_ext flag to tell the kernel that a BO will require CPU
>> access.
>> +The becomes important when placing an object in LMEM, where
>> underneath the
>> +device has a small BAR, meaning only part of it is CPU accessible.
>> Without this
>> +flag the kernel will assume that CPU access is not required, and
>> prioritize
>> +using the non-CPU visible portion of LMEM(if present on the device).
>> +
>> +Related to this, we now also reject any objects marked with
>> +EXEC_OBJECT_CAPTURE, which are also not tagged with NEEDS_CPU_ACCESS.
>> This only
>> +impacts DG2+.
>> +
>> +XXX: One open here is whether we should extend the memory region
>> query to return
>> +the CPU visible size of the region. For now the IGTs just use debugfs
>> to query
>> +the size. However, if userspace sees a real need for this then
>> extending the
>> +region query would be a lot nicer.
>> +
>> +.. kernel-doc:: Documentation/gpu/rfc/i915_small_bar.h
>> + :functions: __drm_i915_gem_create_ext
>> +
>> +DRM_I915_QUERY_VMA_INFO query
>> +-----------------------------
>> +Query the attributes of some vma. Given a vm and GTT offset, find the
>> +respective vma, and return its set of attrubutes. For now we only
>> support
>> +DRM_I915_QUERY_VMA_INFO_CPU_VISIBLE, which is set if the object/vma is
>> +currently placed in memory that is accessible by the CPU. This should
>> always be
>> +set on devices where the CPU visible size of LMEM matches the probed
>> size. If
>> +this is not set then CPU faulting the object will first require
>> migrating the
>> +pages.
>> +
>> +.. kernel-doc:: Documentation/gpu/rfc/i915_small_bar.h
>> + :functions: __drm_i915_query_vma_info
>> diff --git a/Documentation/gpu/rfc/index.rst
>> b/Documentation/gpu/rfc/index.rst
>> index 018a8bf317a6..5b8495bdc1fd 100644
>> --- a/Documentation/gpu/rfc/index.rst
>> +++ b/Documentation/gpu/rfc/index.rst
>> @@ -19,3 +19,7 @@ host such documentation:
>> .. toctree::
>> i915_scheduler.rst
>> +
>> +.. toctree::
>> +
>> + i915_small_bar.rst
>
>
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2022-03-18 10:21 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-18 11:22 [PATCH 0/2] doc/rfc for small BAR support Matthew Auld
2022-02-18 11:22 ` [Intel-gfx] " Matthew Auld
2022-02-18 11:22 ` [PATCH 1/2] drm/doc: remove rfc section for dg1 Matthew Auld
2022-02-18 11:22 ` [Intel-gfx] " Matthew Auld
2022-02-18 18:30 ` Lucas De Marchi
2022-02-18 11:22 ` [PATCH 2/2] drm/doc: add rfc section for small BAR uapi Matthew Auld
2022-02-18 11:22 ` [Intel-gfx] " Matthew Auld
2022-02-22 10:36 ` Thomas Hellström
2022-02-22 10:36 ` [Intel-gfx] " Thomas Hellström
2022-02-22 17:39 ` Abodunrin, Akeem G
2022-02-22 17:39 ` [Intel-gfx] " Abodunrin, Akeem G
2022-03-18 9:38 ` Lionel Landwerlin
2022-03-18 10:21 ` Matthew Auld
2022-02-18 14:15 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for doc/rfc for small BAR support Patchwork
2022-02-18 14:45 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
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.