From: Rodrigo Vivi <rodrigo.vivi@intel.com>
To: Francois Dugast <francois.dugast@intel.com>
Cc: igt-dev@lists.freedesktop.org
Subject: Re: [igt-dev] [PATCH v5 10/21] drm-uapi/xe: Align on a common way to return arrays (memory regions)
Date: Thu, 30 Nov 2023 15:11:01 -0500 [thread overview]
Message-ID: <ZWjsVSigGYCCHFWB@intel.com> (raw)
In-Reply-To: <20231130184536.7-11-francois.dugast@intel.com>
On Thu, Nov 30, 2023 at 06:45:25PM +0000, Francois Dugast wrote:
> Align with commit ("drm/xe/uapi: Align on a common way to return
> arrays (memory regions)")
>
> Signed-off-by: Francois Dugast <francois.dugast@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
> include/drm-uapi/xe_drm.h | 12 +++++------
> lib/xe/xe_query.c | 32 ++++++++++++++---------------
> lib/xe/xe_query.h | 2 +-
> lib/xe/xe_util.c | 6 +++---
> tests/intel/xe_create.c | 2 +-
> tests/intel/xe_drm_fdinfo.c | 8 ++++----
> tests/intel/xe_pm.c | 8 ++++----
> tests/intel/xe_query.c | 40 ++++++++++++++++++-------------------
> tests/kms_plane.c | 2 +-
> 9 files changed, 56 insertions(+), 56 deletions(-)
>
> diff --git a/include/drm-uapi/xe_drm.h b/include/drm-uapi/xe_drm.h
> index 2a4e8743b..62e4d1c29 100644
> --- a/include/drm-uapi/xe_drm.h
> +++ b/include/drm-uapi/xe_drm.h
> @@ -182,10 +182,10 @@ enum drm_xe_memory_class {
> };
>
> /**
> - * struct drm_xe_query_mem_region - Describes some region as known to
> + * struct drm_xe_mem_region - Describes some region as known to
> * the driver.
> */
> -struct drm_xe_query_mem_region {
> +struct drm_xe_mem_region {
> /**
> * @mem_class: The memory class describing this region.
> *
> @@ -322,12 +322,12 @@ struct drm_xe_query_engine_cycles {
> * struct drm_xe_query_mem_regions in .data.
> */
> struct drm_xe_query_mem_regions {
> - /** @num_regions: number of memory regions returned in @regions */
> - __u32 num_regions;
> + /** @num_mem_regions: number of memory regions returned in @mem_regions */
> + __u32 num_mem_regions;
> /** @pad: MBZ */
> __u32 pad;
> - /** @regions: The returned regions for this device */
> - struct drm_xe_query_mem_region regions[];
> + /** @mem_regions: The returned memory regions for this device */
> + struct drm_xe_mem_region mem_regions[];
> };
>
> /**
> diff --git a/lib/xe/xe_query.c b/lib/xe/xe_query.c
> index f9dec1f7a..d00051bd9 100644
> --- a/lib/xe/xe_query.c
> +++ b/lib/xe/xe_query.c
> @@ -134,8 +134,8 @@ static uint64_t gt_vram_size(const struct drm_xe_query_mem_regions *mem_regions,
> {
> int region_idx = ffs(native_region_for_gt(gt_list, gt)) - 1;
>
> - if (XE_IS_CLASS_VRAM(&mem_regions->regions[region_idx]))
> - return mem_regions->regions[region_idx].total_size;
> + if (XE_IS_CLASS_VRAM(&mem_regions->mem_regions[region_idx]))
> + return mem_regions->mem_regions[region_idx].total_size;
>
> return 0;
> }
> @@ -145,16 +145,16 @@ static uint64_t gt_visible_vram_size(const struct drm_xe_query_mem_regions *mem_
> {
> int region_idx = ffs(native_region_for_gt(gt_list, gt)) - 1;
>
> - if (XE_IS_CLASS_VRAM(&mem_regions->regions[region_idx]))
> - return mem_regions->regions[region_idx].cpu_visible_size;
> + if (XE_IS_CLASS_VRAM(&mem_regions->mem_regions[region_idx]))
> + return mem_regions->mem_regions[region_idx].cpu_visible_size;
>
> return 0;
> }
>
> static bool __mem_has_vram(struct drm_xe_query_mem_regions *mem_regions)
> {
> - for (int i = 0; i < mem_regions->num_regions; i++)
> - if (XE_IS_CLASS_VRAM(&mem_regions->regions[i]))
> + for (int i = 0; i < mem_regions->num_mem_regions; i++)
> + if (XE_IS_CLASS_VRAM(&mem_regions->mem_regions[i]))
> return true;
>
> return false;
> @@ -164,9 +164,9 @@ static uint32_t __mem_default_alignment(struct drm_xe_query_mem_regions *mem_reg
> {
> uint32_t alignment = XE_DEFAULT_ALIGNMENT;
>
> - for (int i = 0; i < mem_regions->num_regions; i++)
> - if (alignment < mem_regions->regions[i].min_page_size)
> - alignment = mem_regions->regions[i].min_page_size;
> + for (int i = 0; i < mem_regions->num_mem_regions; i++)
> + if (alignment < mem_regions->mem_regions[i].min_page_size)
> + alignment = mem_regions->mem_regions[i].min_page_size;
>
> return alignment;
> }
> @@ -454,16 +454,16 @@ struct drm_xe_query_engine_info *xe_engine(int fd, int idx)
> *
> * Returns memory region structure for @region mask.
> */
> -struct drm_xe_query_mem_region *xe_mem_region(int fd, uint64_t region)
> +struct drm_xe_mem_region *xe_mem_region(int fd, uint64_t region)
> {
> struct xe_device *xe_dev;
> int region_idx = ffs(region) - 1;
>
> xe_dev = find_in_cache(fd);
> igt_assert(xe_dev);
> - igt_assert(xe_dev->mem_regions->num_regions > region_idx);
> + igt_assert(xe_dev->mem_regions->num_mem_regions > region_idx);
>
> - return &xe_dev->mem_regions->regions[region_idx];
> + return &xe_dev->mem_regions->mem_regions[region_idx];
> }
>
> /**
> @@ -501,7 +501,7 @@ const char *xe_region_name(uint64_t region)
> */
> uint16_t xe_region_class(int fd, uint64_t region)
> {
> - struct drm_xe_query_mem_region *memreg;
> + struct drm_xe_mem_region *memreg;
>
> memreg = xe_mem_region(fd, region);
>
> @@ -593,21 +593,21 @@ uint64_t xe_vram_available(int fd, int gt)
> {
> struct xe_device *xe_dev;
> int region_idx;
> - struct drm_xe_query_mem_region *mem_region;
> + struct drm_xe_mem_region *mem_region;
> struct drm_xe_query_mem_regions *mem_regions;
>
> xe_dev = find_in_cache(fd);
> igt_assert(xe_dev);
>
> region_idx = ffs(native_region_for_gt(xe_dev->gt_list, gt)) - 1;
> - mem_region = &xe_dev->mem_regions->regions[region_idx];
> + mem_region = &xe_dev->mem_regions->mem_regions[region_idx];
>
> if (XE_IS_CLASS_VRAM(mem_region)) {
> uint64_t available_vram;
>
> mem_regions = xe_query_mem_regions_new(fd);
> pthread_mutex_lock(&cache.cache_mutex);
> - mem_region->used = mem_regions->regions[region_idx].used;
> + mem_region->used = mem_regions->mem_regions[region_idx].used;
> available_vram = mem_region->total_size - mem_region->used;
> pthread_mutex_unlock(&cache.cache_mutex);
> free(mem_regions);
> diff --git a/lib/xe/xe_query.h b/lib/xe/xe_query.h
> index fede00036..5862ecba6 100644
> --- a/lib/xe/xe_query.h
> +++ b/lib/xe/xe_query.h
> @@ -83,7 +83,7 @@ uint64_t vram_memory(int fd, int gt);
> uint64_t vram_if_possible(int fd, int gt);
> struct drm_xe_query_engine_info *xe_engines(int fd);
> struct drm_xe_query_engine_info *xe_engine(int fd, int idx);
> -struct drm_xe_query_mem_region *xe_mem_region(int fd, uint64_t region);
> +struct drm_xe_mem_region *xe_mem_region(int fd, uint64_t region);
> const char *xe_region_name(uint64_t region);
> uint16_t xe_region_class(int fd, uint64_t region);
> uint32_t xe_min_page_size(int fd, uint64_t region);
> diff --git a/lib/xe/xe_util.c b/lib/xe/xe_util.c
> index fdbede8d0..53ae2099a 100644
> --- a/lib/xe/xe_util.c
> +++ b/lib/xe/xe_util.c
> @@ -11,7 +11,7 @@
> #include "xe/xe_query.h"
> #include "xe/xe_util.h"
>
> -static bool __region_belongs_to_regions_type(struct drm_xe_query_mem_region *region,
> +static bool __region_belongs_to_regions_type(struct drm_xe_mem_region *region,
> uint32_t *mem_regions_type,
> int num_regions)
> {
> @@ -24,7 +24,7 @@ static bool __region_belongs_to_regions_type(struct drm_xe_query_mem_region *reg
> struct igt_collection *
> __xe_get_memory_region_set(int xe, uint32_t *mem_regions_type, int num_regions)
> {
> - struct drm_xe_query_mem_region *memregion;
> + struct drm_xe_mem_region *memregion;
> struct igt_collection *set = NULL;
> uint64_t memreg = all_memory_regions(xe), region;
> int count = 0, pos = 0;
> @@ -79,7 +79,7 @@ char *xe_memregion_dynamic_subtest_name(int xe, struct igt_collection *set)
> igt_assert(name);
>
> for_each_collection_data(data, set) {
> - struct drm_xe_query_mem_region *memreg;
> + struct drm_xe_mem_region *memreg;
> int r;
>
> region = data->value;
> diff --git a/tests/intel/xe_create.c b/tests/intel/xe_create.c
> index a1ef8a725..773f1446b 100644
> --- a/tests/intel/xe_create.c
> +++ b/tests/intel/xe_create.c
> @@ -49,7 +49,7 @@ static int __create_bo(int fd, uint32_t vm, uint64_t size, uint32_t placement,
> */
> static void create_invalid_size(int fd)
> {
> - struct drm_xe_query_mem_region *memregion;
> + struct drm_xe_mem_region *memregion;
> uint64_t memreg = all_memory_regions(fd), region;
> uint32_t vm;
> uint32_t handle;
> diff --git a/tests/intel/xe_drm_fdinfo.c b/tests/intel/xe_drm_fdinfo.c
> index cec3e0825..fc39649ea 100644
> --- a/tests/intel/xe_drm_fdinfo.c
> +++ b/tests/intel/xe_drm_fdinfo.c
> @@ -42,7 +42,7 @@ IGT_TEST_DESCRIPTION("Read and verify drm client memory consumption using fdinfo
> /* Subtests */
> static void test_active(int fd, struct drm_xe_query_engine_info *engine)
> {
> - struct drm_xe_query_mem_region *memregion;
> + struct drm_xe_mem_region *memregion;
> uint64_t memreg = all_memory_regions(fd), region;
> struct drm_client_fdinfo info = { };
> uint32_t vm;
> @@ -169,7 +169,7 @@ static void test_active(int fd, struct drm_xe_query_engine_info *engine)
>
> static void test_shared(int xe)
> {
> - struct drm_xe_query_mem_region *memregion;
> + struct drm_xe_mem_region *memregion;
> uint64_t memreg = all_memory_regions(xe), region;
> struct drm_client_fdinfo info = { };
> struct drm_gem_flink flink;
> @@ -214,7 +214,7 @@ static void test_shared(int xe)
>
> static void test_total_resident(int xe)
> {
> - struct drm_xe_query_mem_region *memregion;
> + struct drm_xe_mem_region *memregion;
> uint64_t memreg = all_memory_regions(xe), region;
> struct drm_client_fdinfo info = { };
> uint32_t vm;
> @@ -262,7 +262,7 @@ static void test_total_resident(int xe)
>
> static void basic(int xe)
> {
> - struct drm_xe_query_mem_region *memregion;
> + struct drm_xe_mem_region *memregion;
> uint64_t memreg = all_memory_regions(xe), region;
> struct drm_client_fdinfo info = { };
> unsigned int ret;
> diff --git a/tests/intel/xe_pm.c b/tests/intel/xe_pm.c
> index d78ca31a8..a8fc56e4b 100644
> --- a/tests/intel/xe_pm.c
> +++ b/tests/intel/xe_pm.c
> @@ -400,10 +400,10 @@ static void test_vram_d3cold_threshold(device_t device, int sysfs_fd)
> query.data = to_user_pointer(mem_regions);
> igt_assert_eq(igt_ioctl(device.fd_xe, DRM_IOCTL_XE_DEVICE_QUERY, &query), 0);
>
> - for (i = 0; i < mem_regions->num_regions; i++) {
> - if (mem_regions->regions[i].mem_class == DRM_XE_MEM_REGION_CLASS_VRAM) {
> - vram_used_mb += (mem_regions->regions[i].used / (1024 * 1024));
> - vram_total_mb += (mem_regions->regions[i].total_size / (1024 * 1024));
> + for (i = 0; i < mem_regions->num_mem_regions; i++) {
> + if (mem_regions->mem_regions[i].mem_class == DRM_XE_MEM_REGION_CLASS_VRAM) {
> + vram_used_mb += (mem_regions->mem_regions[i].used / (1024 * 1024));
> + vram_total_mb += (mem_regions->mem_regions[i].total_size / (1024 * 1024));
> }
> }
>
> diff --git a/tests/intel/xe_query.c b/tests/intel/xe_query.c
> index 48042337a..207785a38 100644
> --- a/tests/intel/xe_query.c
> +++ b/tests/intel/xe_query.c
> @@ -218,34 +218,34 @@ test_query_mem_regions(int fd)
> query.data = to_user_pointer(mem_regions);
> igt_assert_eq(igt_ioctl(fd, DRM_IOCTL_XE_DEVICE_QUERY, &query), 0);
>
> - for (i = 0; i < mem_regions->num_regions; i++) {
> + for (i = 0; i < mem_regions->num_mem_regions; i++) {
> igt_info("mem region %d: %s\t%#llx / %#llx\n", i,
> - mem_regions->regions[i].mem_class ==
> + mem_regions->mem_regions[i].mem_class ==
> DRM_XE_MEM_REGION_CLASS_SYSMEM ? "SYSMEM"
> - :mem_regions->regions[i].mem_class ==
> + :mem_regions->mem_regions[i].mem_class ==
> DRM_XE_MEM_REGION_CLASS_VRAM ? "VRAM" : "?",
> - mem_regions->regions[i].used,
> - mem_regions->regions[i].total_size
> + mem_regions->mem_regions[i].used,
> + mem_regions->mem_regions[i].total_size
> );
> igt_info("min_page_size=0x%x\n",
> - mem_regions->regions[i].min_page_size);
> + mem_regions->mem_regions[i].min_page_size);
>
> igt_info("visible size=%lluMiB\n",
> - mem_regions->regions[i].cpu_visible_size >> 20);
> + mem_regions->mem_regions[i].cpu_visible_size >> 20);
> igt_info("visible used=%lluMiB\n",
> - mem_regions->regions[i].cpu_visible_used >> 20);
> -
> - igt_assert_lte_u64(mem_regions->regions[i].cpu_visible_size,
> - mem_regions->regions[i].total_size);
> - igt_assert_lte_u64(mem_regions->regions[i].cpu_visible_used,
> - mem_regions->regions[i].cpu_visible_size);
> - igt_assert_lte_u64(mem_regions->regions[i].cpu_visible_used,
> - mem_regions->regions[i].used);
> - igt_assert_lte_u64(mem_regions->regions[i].used,
> - mem_regions->regions[i].total_size);
> - igt_assert_lte_u64(mem_regions->regions[i].used -
> - mem_regions->regions[i].cpu_visible_used,
> - mem_regions->regions[i].total_size);
> + mem_regions->mem_regions[i].cpu_visible_used >> 20);
> +
> + igt_assert_lte_u64(mem_regions->mem_regions[i].cpu_visible_size,
> + mem_regions->mem_regions[i].total_size);
> + igt_assert_lte_u64(mem_regions->mem_regions[i].cpu_visible_used,
> + mem_regions->mem_regions[i].cpu_visible_size);
> + igt_assert_lte_u64(mem_regions->mem_regions[i].cpu_visible_used,
> + mem_regions->mem_regions[i].used);
> + igt_assert_lte_u64(mem_regions->mem_regions[i].used,
> + mem_regions->mem_regions[i].total_size);
> + igt_assert_lte_u64(mem_regions->mem_regions[i].used -
> + mem_regions->mem_regions[i].cpu_visible_used,
> + mem_regions->mem_regions[i].total_size);
> }
> dump_hex_debug(mem_regions, query.size);
> free(mem_regions);
> diff --git a/tests/kms_plane.c b/tests/kms_plane.c
> index e50a94578..51ca082ae 100644
> --- a/tests/kms_plane.c
> +++ b/tests/kms_plane.c
> @@ -467,7 +467,7 @@ test_plane_panning(data_t *data, enum pipe pipe)
> }
>
> if (is_xe_device(data->drm_fd)) {
> - struct drm_xe_query_mem_region *memregion;
> + struct drm_xe_mem_region *memregion;
> uint64_t memreg = all_memory_regions(data->drm_fd), region;
>
> xe_for_each_mem_region(data->drm_fd, memreg, region) {
> --
> 2.34.1
>
next prev parent reply other threads:[~2023-11-30 20:11 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-30 18:45 [igt-dev] [PATCH v5 00/21] uAPI Alignment - Cleanup and future proof Francois Dugast
2023-11-30 18:45 ` [igt-dev] [PATCH v5 01/21] drm-uapi/xe: Extend drm_xe_vm_bind_op Francois Dugast
2023-11-30 18:45 ` [igt-dev] [PATCH v5 02/21] xe_ioctl: Converge bo_create to the most used version Francois Dugast
2023-12-01 9:51 ` Kamil Konieczny
2023-11-30 18:45 ` [igt-dev] [PATCH v5 03/21] xe_ioctl: Rename *xe_bo_create_flags to simply xe_bo_create Francois Dugast
2023-12-01 10:04 ` Kamil Konieczny
2023-11-30 18:45 ` [igt-dev] [PATCH v5 04/21] xe_query: Add missing include Francois Dugast
2023-11-30 18:45 ` [igt-dev] [PATCH v5 05/21] xe_query: Kill visible_vram_if_possible Francois Dugast
2023-11-30 18:45 ` [igt-dev] [PATCH v5 06/21] drm-uapi/xe: Separate bo_create placement from flags Francois Dugast
2023-12-01 10:38 ` Kamil Konieczny
2023-11-30 18:45 ` [igt-dev] [PATCH v5 07/21] xe: s/hw_engine/engine Francois Dugast
2023-11-30 18:45 ` [igt-dev] [PATCH v5 08/21] drm-uapi/xe: Make DRM_XE_DEVICE_QUERY_ENGINES future proof Francois Dugast
2023-12-01 14:09 ` Souza, Jose
2023-11-30 18:45 ` [igt-dev] [PATCH v5 09/21] drm-uapi/xe: Reject bo creation of unaligned size Francois Dugast
2023-11-30 20:06 ` Rodrigo Vivi
2023-11-30 18:45 ` [igt-dev] [PATCH v5 10/21] drm-uapi/xe: Align on a common way to return arrays (memory regions) Francois Dugast
2023-11-30 20:11 ` Rodrigo Vivi [this message]
2023-11-30 18:45 ` [igt-dev] [PATCH v5 11/21] drm-uapi/xe: Align on a common way to return arrays (gt) Francois Dugast
2023-11-30 20:03 ` Rodrigo Vivi
2023-11-30 18:45 ` [igt-dev] [PATCH v5 12/21] drm-uapi/xe: Align on a common way to return arrays (engines) Francois Dugast
2023-11-30 20:04 ` Rodrigo Vivi
2023-11-30 18:45 ` [igt-dev] [PATCH v5 13/21] drm-uapi/xe: Split xe_sync types from flags Francois Dugast
2023-11-30 20:07 ` Rodrigo Vivi
2023-11-30 18:45 ` [igt-dev] [PATCH v5 14/21] drm-uapi/xe: Kill tile_mask Francois Dugast
2023-11-30 18:45 ` [igt-dev] [PATCH v5 15/21] drm-uapi/xe: Crystal Reference Clock updates Francois Dugast
2023-11-30 20:10 ` Rodrigo Vivi
2023-11-30 18:45 ` [igt-dev] [PATCH v5 16/21] drm-uapi/xe: Add Tile ID information to the GT info query Francois Dugast
2023-11-30 19:04 ` Rodrigo Vivi
2023-11-30 18:45 ` [igt-dev] [PATCH v5 17/21] drm-uapi/xe: Fix various struct padding for 64b alignment Francois Dugast
2023-11-30 20:07 ` Rodrigo Vivi
2023-11-30 18:45 ` [igt-dev] [PATCH v5 18/21] drm-uapi/xe: Move xe_exec after xe_exec_queue Francois Dugast
2023-11-30 19:04 ` Rodrigo Vivi
2023-11-30 18:45 ` [igt-dev] [PATCH v5 19/21] tests/intel/xe: Adjust to KMD uAPI changes for long-running VMs Francois Dugast
2023-12-01 10:00 ` Francois Dugast
2023-11-30 18:45 ` [igt-dev] [PATCH v5 20/21] drm-uapi/xe: Remove unused extension definition Francois Dugast
2023-11-30 19:04 ` Rodrigo Vivi
2023-11-30 18:45 ` [igt-dev] [PATCH v5 21/21] drm-uapi/xe: Kill exec_queue_set_property Francois Dugast
2023-11-30 19:05 ` Rodrigo Vivi
2023-11-30 20:35 ` [igt-dev] ✗ Fi.CI.BAT: failure for uAPI Alignment - Cleanup and future proof (rev5) Patchwork
2023-11-30 23:25 ` [igt-dev] ✗ CI.xeBAT: " Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZWjsVSigGYCCHFWB@intel.com \
--to=rodrigo.vivi@intel.com \
--cc=francois.dugast@intel.com \
--cc=igt-dev@lists.freedesktop.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox