diff mbox series

[02/10] drm/i915/uapi: add probed_cpu_visible_size

Message ID 20220525184337.491763-3-matthew.auld@intel.com (mailing list archive)
State New, archived
Headers show
Series small BAR uapi bits | expand

Commit Message

Matthew Auld May 25, 2022, 6:43 p.m. UTC
Userspace wants to know the size of CPU visible portion of device
local-memory, and on small BAR devices the probed_size is no longer
enough. In Vulkan, for example, it would like to know the size in bytes
for CPU visible VkMemoryHeap. We already track the io_size for each
region, so it's just case of plumbing that through to the region query.

Testcase: igt@i915_query@query-regions-sanity-check
Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@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: Akeem G Abodunrin <akeem.g.abodunrin@intel.com>
---
 drivers/gpu/drm/i915/i915_query.c |  6 +++
 include/uapi/drm/i915_drm.h       | 74 +++++++++++++++++--------------
 2 files changed, 47 insertions(+), 33 deletions(-)

Comments

Nirmoy Das June 1, 2022, 12:36 p.m. UTC | #1
Acked-by: Nirmoy Das <nirmoy.das@intel.com>

On 5/25/2022 8:43 PM, Matthew Auld wrote:
> Userspace wants to know the size of CPU visible portion of device
> local-memory, and on small BAR devices the probed_size is no longer
> enough. In Vulkan, for example, it would like to know the size in bytes
> for CPU visible VkMemoryHeap. We already track the io_size for each
> region, so it's just case of plumbing that through to the region query.
>
> Testcase: igt@i915_query@query-regions-sanity-check
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@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: Akeem G Abodunrin <akeem.g.abodunrin@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_query.c |  6 +++
>   include/uapi/drm/i915_drm.h       | 74 +++++++++++++++++--------------
>   2 files changed, 47 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_query.c b/drivers/gpu/drm/i915/i915_query.c
> index 7584cec53d5d..9aa0b28aa6ee 100644
> --- a/drivers/gpu/drm/i915/i915_query.c
> +++ b/drivers/gpu/drm/i915/i915_query.c
> @@ -496,6 +496,12 @@ static int query_memregion_info(struct drm_i915_private *i915,
>   		info.region.memory_class = mr->type;
>   		info.region.memory_instance = mr->instance;
>   		info.probed_size = mr->total;
> +
> +		if (mr->type == INTEL_MEMORY_LOCAL)
> +			info.probed_cpu_visible_size = mr->io_size;
> +		else
> +			info.probed_cpu_visible_size = mr->total;
> +
>   		info.unallocated_size = mr->avail;
>   
>   		if (__copy_to_user(info_ptr, &info, sizeof(info)))
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index de49b68b4fc8..9df419a45244 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -3207,36 +3207,6 @@ struct drm_i915_gem_memory_class_instance {
>    * struct drm_i915_memory_region_info - Describes one region as known to the
>    * driver.
>    *
> - * Note that we reserve some stuff here for potential future work. As an example
> - * we might want expose the capabilities for a given region, which could include
> - * things like if the region is CPU mappable/accessible, what are the supported
> - * mapping types etc.
> - *
> - * Note that to extend struct drm_i915_memory_region_info and struct
> - * drm_i915_query_memory_regions in the future the plan is to do the following:
> - *
> - * .. code-block:: C
> - *
> - *	struct drm_i915_memory_region_info {
> - *		struct drm_i915_gem_memory_class_instance region;
> - *		union {
> - *			__u32 rsvd0;
> - *			__u32 new_thing1;
> - *		};
> - *		...
> - *		union {
> - *			__u64 rsvd1[8];
> - *			struct {
> - *				__u64 new_thing2;
> - *				__u64 new_thing3;
> - *				...
> - *			};
> - *		};
> - *	};
> - *
> - * With this things should remain source compatible between versions for
> - * userspace, even as we add new fields.
> - *
>    * Note this is using both struct drm_i915_query_item and struct drm_i915_query.
>    * For this new query we are adding the new query id DRM_I915_QUERY_MEMORY_REGIONS
>    * at &drm_i915_query_item.query_id.
> @@ -3248,14 +3218,52 @@ struct drm_i915_memory_region_info {
>   	/** @rsvd0: MBZ */
>   	__u32 rsvd0;
>   
> -	/** @probed_size: Memory probed by the driver (-1 = unknown) */
> +	/**
> +	 * @probed_size: Memory probed by the driver (-1 = unknown)
> +	 *
> +	 * Note that it should not be possible to ever encounter a zero value
> +	 * here, also note that no current region type will ever return -1 here.
> +	 * Although for future region types, this might be a possibility. The
> +	 * same applies to the other size fields.
> +	 */
>   	__u64 probed_size;
>   
>   	/** @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).
> +			 *
> +			 * This will be always be <= @probed_size, and the
> +			 * remainder (if there is any) will not be CPU
> +			 * accessible.
> +			 *
> +			 * On systems without small BAR, the @probed_size will
> +			 * always equal the @probed_cpu_visible_size, since all
> +			 * of it will be CPU accessible.
> +			 *
> +			 * Note this is only tracked for
> +			 * I915_MEMORY_CLASS_DEVICE regions (for other types the
> +			 * value here will always equal the @probed_size).
> +			 *
> +			 * Note that if the value returned here is zero, then
> +			 * this must be an old kernel which lacks the relevant
> +			 * small-bar uAPI support (including
> +			 * I915_GEM_CREATE_EXT_FLAG_NEEDS_CPU_ACCESS), but on
> +			 * such systems we should never actually end up with a
> +			 * small BAR configuration, assuming we are able to load
> +			 * the kernel module. Hence it should be safe to treat
> +			 * this the same as when @probed_cpu_visible_size ==
> +			 * @probed_size.
> +			 */
> +			__u64 probed_cpu_visible_size;
> +		};
> +	};
>   };
>   
>   /**
Thomas Hellström (Intel) June 16, 2022, 11:22 a.m. UTC | #2
On 5/25/22 20:43, Matthew Auld wrote:
> Userspace wants to know the size of CPU visible portion of device
> local-memory, and on small BAR devices the probed_size is no longer
> enough. In Vulkan, for example, it would like to know the size in bytes
> for CPU visible VkMemoryHeap. We already track the io_size for each
> region, so it's just case of plumbing that through to the region query.
Nit: so plumb that through (imperative)
>
> Testcase: igt@i915_query@query-regions-sanity-check
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@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: Akeem G Abodunrin <akeem.g.abodunrin@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_query.c |  6 +++
>   include/uapi/drm/i915_drm.h       | 74 +++++++++++++++++--------------
>   2 files changed, 47 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_query.c b/drivers/gpu/drm/i915/i915_query.c
> index 7584cec53d5d..9aa0b28aa6ee 100644
> --- a/drivers/gpu/drm/i915/i915_query.c
> +++ b/drivers/gpu/drm/i915/i915_query.c
> @@ -496,6 +496,12 @@ static int query_memregion_info(struct drm_i915_private *i915,
>   		info.region.memory_class = mr->type;
>   		info.region.memory_instance = mr->instance;
>   		info.probed_size = mr->total;
> +
> +		if (mr->type == INTEL_MEMORY_LOCAL)
> +			info.probed_cpu_visible_size = mr->io_size;
> +		else
> +			info.probed_cpu_visible_size = mr->total;
> +
>   		info.unallocated_size = mr->avail;
>   
>   		if (__copy_to_user(info_ptr, &info, sizeof(info)))
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index de49b68b4fc8..9df419a45244 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -3207,36 +3207,6 @@ struct drm_i915_gem_memory_class_instance {
>    * struct drm_i915_memory_region_info - Describes one region as known to the
>    * driver.
>    *
> - * Note that we reserve some stuff here for potential future work. As an example
> - * we might want expose the capabilities for a given region, which could include
> - * things like if the region is CPU mappable/accessible, what are the supported
> - * mapping types etc.
> - *
> - * Note that to extend struct drm_i915_memory_region_info and struct
> - * drm_i915_query_memory_regions in the future the plan is to do the following:
> - *
> - * .. code-block:: C
> - *
> - *	struct drm_i915_memory_region_info {
> - *		struct drm_i915_gem_memory_class_instance region;
> - *		union {
> - *			__u32 rsvd0;
> - *			__u32 new_thing1;
> - *		};
> - *		...
> - *		union {
> - *			__u64 rsvd1[8];
> - *			struct {
> - *				__u64 new_thing2;
> - *				__u64 new_thing3;
> - *				...
> - *			};
> - *		};
> - *	};
> - *
> - * With this things should remain source compatible between versions for
> - * userspace, even as we add new fields.
> - *
>    * Note this is using both struct drm_i915_query_item and struct drm_i915_query.
>    * For this new query we are adding the new query id DRM_I915_QUERY_MEMORY_REGIONS
>    * at &drm_i915_query_item.query_id.
> @@ -3248,14 +3218,52 @@ struct drm_i915_memory_region_info {
>   	/** @rsvd0: MBZ */
>   	__u32 rsvd0;
>   
> -	/** @probed_size: Memory probed by the driver (-1 = unknown) */
> +	/**
> +	 * @probed_size: Memory probed by the driver (-1 = unknown)
> +	 *
> +	 * Note that it should not be possible to ever encounter a zero value
> +	 * here, also note that no current region type will ever return -1 here.
> +	 * Although for future region types, this might be a possibility. The
> +	 * same applies to the other size fields.
> +	 */
>   	__u64 probed_size;
>   
>   	/** @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).
> +			 *
> +			 * This will be always be <= @probed_size, and the
> +			 * remainder (if there is any) will not be CPU
> +			 * accessible.
> +			 *
> +			 * On systems without small BAR, the @probed_size will
> +			 * always equal the @probed_cpu_visible_size, since all
> +			 * of it will be CPU accessible.
> +			 *
> +			 * Note this is only tracked for
> +			 * I915_MEMORY_CLASS_DEVICE regions (for other types the
> +			 * value here will always equal the @probed_size).
> +			 *
> +			 * Note that if the value returned here is zero, then
> +			 * this must be an old kernel which lacks the relevant
> +			 * small-bar uAPI support (including
> +			 * I915_GEM_CREATE_EXT_FLAG_NEEDS_CPU_ACCESS), but on
> +			 * such systems we should never actually end up with a
> +			 * small BAR configuration, assuming we are able to load
> +			 * the kernel module. Hence it should be safe to treat
> +			 * this the same as when @probed_cpu_visible_size ==
> +			 * @probed_size.
> +			 */
> +			__u64 probed_cpu_visible_size;
> +		};
> +	};
>   };
>   
>   /**

Reviewed-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/i915_query.c b/drivers/gpu/drm/i915/i915_query.c
index 7584cec53d5d..9aa0b28aa6ee 100644
--- a/drivers/gpu/drm/i915/i915_query.c
+++ b/drivers/gpu/drm/i915/i915_query.c
@@ -496,6 +496,12 @@  static int query_memregion_info(struct drm_i915_private *i915,
 		info.region.memory_class = mr->type;
 		info.region.memory_instance = mr->instance;
 		info.probed_size = mr->total;
+
+		if (mr->type == INTEL_MEMORY_LOCAL)
+			info.probed_cpu_visible_size = mr->io_size;
+		else
+			info.probed_cpu_visible_size = mr->total;
+
 		info.unallocated_size = mr->avail;
 
 		if (__copy_to_user(info_ptr, &info, sizeof(info)))
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index de49b68b4fc8..9df419a45244 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -3207,36 +3207,6 @@  struct drm_i915_gem_memory_class_instance {
  * struct drm_i915_memory_region_info - Describes one region as known to the
  * driver.
  *
- * Note that we reserve some stuff here for potential future work. As an example
- * we might want expose the capabilities for a given region, which could include
- * things like if the region is CPU mappable/accessible, what are the supported
- * mapping types etc.
- *
- * Note that to extend struct drm_i915_memory_region_info and struct
- * drm_i915_query_memory_regions in the future the plan is to do the following:
- *
- * .. code-block:: C
- *
- *	struct drm_i915_memory_region_info {
- *		struct drm_i915_gem_memory_class_instance region;
- *		union {
- *			__u32 rsvd0;
- *			__u32 new_thing1;
- *		};
- *		...
- *		union {
- *			__u64 rsvd1[8];
- *			struct {
- *				__u64 new_thing2;
- *				__u64 new_thing3;
- *				...
- *			};
- *		};
- *	};
- *
- * With this things should remain source compatible between versions for
- * userspace, even as we add new fields.
- *
  * Note this is using both struct drm_i915_query_item and struct drm_i915_query.
  * For this new query we are adding the new query id DRM_I915_QUERY_MEMORY_REGIONS
  * at &drm_i915_query_item.query_id.
@@ -3248,14 +3218,52 @@  struct drm_i915_memory_region_info {
 	/** @rsvd0: MBZ */
 	__u32 rsvd0;
 
-	/** @probed_size: Memory probed by the driver (-1 = unknown) */
+	/**
+	 * @probed_size: Memory probed by the driver (-1 = unknown)
+	 *
+	 * Note that it should not be possible to ever encounter a zero value
+	 * here, also note that no current region type will ever return -1 here.
+	 * Although for future region types, this might be a possibility. The
+	 * same applies to the other size fields.
+	 */
 	__u64 probed_size;
 
 	/** @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).
+			 *
+			 * This will be always be <= @probed_size, and the
+			 * remainder (if there is any) will not be CPU
+			 * accessible.
+			 *
+			 * On systems without small BAR, the @probed_size will
+			 * always equal the @probed_cpu_visible_size, since all
+			 * of it will be CPU accessible.
+			 *
+			 * Note this is only tracked for
+			 * I915_MEMORY_CLASS_DEVICE regions (for other types the
+			 * value here will always equal the @probed_size).
+			 *
+			 * Note that if the value returned here is zero, then
+			 * this must be an old kernel which lacks the relevant
+			 * small-bar uAPI support (including
+			 * I915_GEM_CREATE_EXT_FLAG_NEEDS_CPU_ACCESS), but on
+			 * such systems we should never actually end up with a
+			 * small BAR configuration, assuming we are able to load
+			 * the kernel module. Hence it should be safe to treat
+			 * this the same as when @probed_cpu_visible_size ==
+			 * @probed_size.
+			 */
+			__u64 probed_cpu_visible_size;
+		};
+	};
 };
 
 /**