| Message ID | 20220525184337.491763-3-matthew.auld@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | small BAR uapi bits | expand |
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; > + }; > + }; > }; > > /**
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 --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; + }; + }; }; /**
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(-)