[igt-dev] [PATCH v5 12/21] drm-uapi/xe: Align on a common way to return arrays (engines)
Rodrigo Vivi
rodrigo.vivi at intel.com
Thu Nov 30 20:04:26 UTC 2023
On Thu, Nov 30, 2023 at 06:45:27PM +0000, Francois Dugast wrote:
> Align with commit ("drm/xe/uapi: Align on a common way to return
> arrays (engines)")
>
> Signed-off-by: Francois Dugast <francois.dugast at intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> ---
> include/drm-uapi/xe_drm.h | 78 +++++++++++++++++++------------
> lib/xe/xe_query.c | 24 ++++------
> lib/xe/xe_query.h | 11 ++---
> tests/intel/xe_create.c | 2 +-
> tests/intel/xe_drm_fdinfo.c | 2 +-
> tests/intel/xe_exec_store.c | 2 +-
> tests/intel/xe_noexec_ping_pong.c | 2 +-
> tests/intel/xe_waitfence.c | 2 +-
> 8 files changed, 66 insertions(+), 57 deletions(-)
>
> diff --git a/include/drm-uapi/xe_drm.h b/include/drm-uapi/xe_drm.h
> index d37266072..55b3edc93 100644
> --- a/include/drm-uapi/xe_drm.h
> +++ b/include/drm-uapi/xe_drm.h
> @@ -127,9 +127,9 @@ struct xe_user_extension {
> /**
> * struct drm_xe_engine_class_instance - instance of an engine class
> *
> - * It is returned as part of the @drm_xe_query_engine_info, but it also is
> - * used as the input of engine selection for both @drm_xe_exec_queue_create
> - * and @drm_xe_query_engine_cycles
> + * It is returned as part of the @drm_xe_engine, but it also is used as
> + * the input of engine selection for both @drm_xe_exec_queue_create and
> + * @drm_xe_query_engine_cycles
> *
> */
> struct drm_xe_engine_class_instance {
> @@ -153,13 +153,9 @@ struct drm_xe_engine_class_instance {
> };
>
> /**
> - * struct drm_xe_query_engine_info - describe hardware engine
> - *
> - * If a query is made with a struct @drm_xe_device_query where .query
> - * is equal to %DRM_XE_DEVICE_QUERY_ENGINES, then the reply uses an array of
> - * struct @drm_xe_query_engine_info in .data.
> + * struct drm_xe_engine - describe hardware engine
> */
> -struct drm_xe_query_engine_info {
> +struct drm_xe_engine {
> /** @instance: The @drm_xe_engine_class_instance */
> struct drm_xe_engine_class_instance instance;
>
> @@ -167,6 +163,22 @@ struct drm_xe_query_engine_info {
> __u64 reserved[3];
> };
>
> +/**
> + * struct drm_xe_query_engines - describe engines
> + *
> + * If a query is made with a struct @drm_xe_device_query where .query
> + * is equal to %DRM_XE_DEVICE_QUERY_ENGINES, then the reply uses an array of
> + * struct @drm_xe_query_engines in .data.
> + */
> +struct drm_xe_query_engines {
> + /** @num_engines: number of engines returned in @engines */
> + __u32 num_engines;
> + /** @pad: MBZ */
> + __u32 pad;
> + /** @engines: The returned engines for this device */
> + struct drm_xe_engine engines[];
> +};
> +
> /**
> * enum drm_xe_memory_class - Supported memory classes.
> */
> @@ -466,28 +478,32 @@ struct drm_xe_query_topology_mask {
> *
> * .. code-block:: C
> *
> - * struct drm_xe_engine_class_instance *hwe;
> - * struct drm_xe_device_query query = {
> - * .extensions = 0,
> - * .query = DRM_XE_DEVICE_QUERY_ENGINES,
> - * .size = 0,
> - * .data = 0,
> - * };
> - * ioctl(fd, DRM_IOCTL_XE_DEVICE_QUERY, &query);
> - * hwe = malloc(query.size);
> - * query.data = (uintptr_t)hwe;
> - * ioctl(fd, DRM_IOCTL_XE_DEVICE_QUERY, &query);
> - * int num_engines = query.size / sizeof(*hwe);
> - * for (int i = 0; i < num_engines; i++) {
> - * printf("Engine %d: %s\n", i,
> - * hwe[i].engine_class == DRM_XE_ENGINE_CLASS_RENDER ? "RENDER":
> - * hwe[i].engine_class == DRM_XE_ENGINE_CLASS_COPY ? "COPY":
> - * hwe[i].engine_class == DRM_XE_ENGINE_CLASS_VIDEO_DECODE ? "VIDEO_DECODE":
> - * hwe[i].engine_class == DRM_XE_ENGINE_CLASS_VIDEO_ENHANCE ? "VIDEO_ENHANCE":
> - * hwe[i].engine_class == DRM_XE_ENGINE_CLASS_COMPUTE ? "COMPUTE":
> - * "UNKNOWN");
> - * }
> - * free(hwe);
> + * struct drm_xe_query_engines *engines;
> + * struct drm_xe_device_query query = {
> + * .extensions = 0,
> + * .query = DRM_XE_DEVICE_QUERY_ENGINES,
> + * .size = 0,
> + * .data = 0,
> + * };
> + * ioctl(fd, DRM_IOCTL_XE_DEVICE_QUERY, &query);
> + * engines = malloc(query.size);
> + * query.data = (uintptr_t)engines;
> + * ioctl(fd, DRM_IOCTL_XE_DEVICE_QUERY, &query);
> + * for (int i = 0; i < engines->num_engines; i++) {
> + * printf("Engine %d: %s\n", i,
> + * engines->engines[i].instance.engine_class ==
> + * DRM_XE_ENGINE_CLASS_RENDER ? "RENDER":
> + * engines->engines[i].instance.engine_class ==
> + * DRM_XE_ENGINE_CLASS_COPY ? "COPY":
> + * engines->engines[i].instance.engine_class ==
> + * DRM_XE_ENGINE_CLASS_VIDEO_DECODE ? "VIDEO_DECODE":
> + * engines->engines[i].instance.engine_class ==
> + * DRM_XE_ENGINE_CLASS_VIDEO_ENHANCE ? "VIDEO_ENHANCE":
> + * engines->engines[i].instance.engine_class ==
> + * DRM_XE_ENGINE_CLASS_COMPUTE ? "COMPUTE":
> + * "UNKNOWN");
> + * }
> + * free(engines);
> */
> struct drm_xe_device_query {
> /** @extensions: Pointer to the first extension struct, if any */
> diff --git a/lib/xe/xe_query.c b/lib/xe/xe_query.c
> index d00051bd9..fa2b49079 100644
> --- a/lib/xe/xe_query.c
> +++ b/lib/xe/xe_query.c
> @@ -72,10 +72,9 @@ static uint64_t __memory_regions(const struct drm_xe_query_gt_list *gt_list)
> return regions;
> }
>
> -static struct drm_xe_query_engine_info *
> -xe_query_engines(int fd, unsigned int *num_engines)
> +static struct drm_xe_query_engines *xe_query_engines(int fd)
> {
> - struct drm_xe_query_engine_info *engines;
> + struct drm_xe_query_engines *engines;
> struct drm_xe_device_query query = {
> .extensions = 0,
> .query = DRM_XE_DEVICE_QUERY_ENGINES,
> @@ -83,7 +82,6 @@ xe_query_engines(int fd, unsigned int *num_engines)
> .data = 0,
> };
>
> - igt_assert(num_engines);
> igt_assert_eq(igt_ioctl(fd, DRM_IOCTL_XE_DEVICE_QUERY, &query), 0);
>
> engines = malloc(query.size);
> @@ -92,8 +90,6 @@ xe_query_engines(int fd, unsigned int *num_engines)
> query.data = to_user_pointer(engines);
> igt_assert_eq(igt_ioctl(fd, DRM_IOCTL_XE_DEVICE_QUERY, &query), 0);
>
> - *num_engines = query.size / sizeof(*engines);
> -
> return engines;
> }
>
> @@ -253,7 +249,7 @@ struct xe_device *xe_device_get(int fd)
> xe_dev->dev_id = xe_dev->config->info[DRM_XE_QUERY_CONFIG_REV_AND_DEVICE_ID] & 0xffff;
> xe_dev->gt_list = xe_query_gt_list_new(fd);
> xe_dev->memory_regions = __memory_regions(xe_dev->gt_list);
> - xe_dev->engines = xe_query_engines(fd, &xe_dev->number_engines);
> + xe_dev->engines = xe_query_engines(fd);
> xe_dev->mem_regions = xe_query_mem_regions_new(fd);
> xe_dev->vram_size = calloc(xe_dev->gt_list->num_gt, sizeof(*xe_dev->vram_size));
> xe_dev->visible_vram_size = calloc(xe_dev->gt_list->num_gt, sizeof(*xe_dev->visible_vram_size));
> @@ -427,7 +423,7 @@ uint64_t vram_if_possible(int fd, int gt)
> *
> * Returns engines array of xe device @fd.
> */
> -xe_dev_FN(xe_engines, engines, struct drm_xe_query_engine_info *);
> +xe_dev_FN(xe_engines, engines->engines, struct drm_xe_engine *);
>
> /**
> * xe_engine:
> @@ -436,15 +432,15 @@ xe_dev_FN(xe_engines, engines, struct drm_xe_query_engine_info *);
> *
> * Returns engine info of xe device @fd and @idx.
> */
> -struct drm_xe_query_engine_info *xe_engine(int fd, int idx)
> +struct drm_xe_engine *xe_engine(int fd, int idx)
> {
> struct xe_device *xe_dev;
>
> xe_dev = find_in_cache(fd);
> igt_assert(xe_dev);
> - igt_assert(idx >= 0 && idx < xe_dev->number_engines);
> + igt_assert(idx >= 0 && idx < xe_dev->engines->num_engines);
>
> - return &xe_dev->engines[idx];
> + return &xe_dev->engines->engines[idx];
> }
>
> /**
> @@ -534,7 +530,7 @@ xe_dev_FN(xe_config, config, struct drm_xe_query_config *);
> *
> * Returns number of hw engines of xe device @fd.
> */
> -xe_dev_FN(xe_number_engines, number_engines, unsigned int);
> +xe_dev_FN(xe_number_engines, engines->num_engines, unsigned int);
>
> /**
> * xe_has_vram:
> @@ -657,8 +653,8 @@ bool xe_has_engine_class(int fd, uint16_t engine_class)
> xe_dev = find_in_cache(fd);
> igt_assert(xe_dev);
>
> - for (int i = 0; i < xe_dev->number_engines; i++)
> - if (xe_dev->engines[i].instance.engine_class == engine_class)
> + for (int i = 0; i < xe_dev->engines->num_engines; i++)
> + if (xe_dev->engines->engines[i].instance.engine_class == engine_class)
> return true;
>
> return false;
> diff --git a/lib/xe/xe_query.h b/lib/xe/xe_query.h
> index 5862ecba6..883cabb7d 100644
> --- a/lib/xe/xe_query.h
> +++ b/lib/xe/xe_query.h
> @@ -32,11 +32,8 @@ struct xe_device {
> /** @gt_list: bitmask of all memory regions */
> uint64_t memory_regions;
>
> - /** @engines: array of hardware engines */
> - struct drm_xe_query_engine_info *engines;
> -
> - /** @number_engines: length of hardware engines array */
> - unsigned int number_engines;
> + /** @engines: hardware engines */
> + struct drm_xe_query_engines *engines;
>
> /** @mem_regions: regions memory information and usage */
> struct drm_xe_query_mem_regions *mem_regions;
> @@ -81,8 +78,8 @@ uint64_t all_memory_regions(int fd);
> uint64_t system_memory(int fd);
> 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_engine *xe_engines(int fd);
> +struct drm_xe_engine *xe_engine(int fd, int idx);
> 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);
> diff --git a/tests/intel/xe_create.c b/tests/intel/xe_create.c
> index 773f1446b..bbdddc7c9 100644
> --- a/tests/intel/xe_create.c
> +++ b/tests/intel/xe_create.c
> @@ -149,7 +149,7 @@ static void create_execqueues(int fd, enum exec_queue_destroy ed)
> igt_nsec_elapsed(&tv);
>
> igt_fork(n, nproc) {
> - struct drm_xe_query_engine_info *engine;
> + struct drm_xe_engine *engine;
> uint32_t exec_queue, exec_queues[exec_queues_per_process];
> int idx, err, i;
>
> diff --git a/tests/intel/xe_drm_fdinfo.c b/tests/intel/xe_drm_fdinfo.c
> index fc39649ea..ec457b1c1 100644
> --- a/tests/intel/xe_drm_fdinfo.c
> +++ b/tests/intel/xe_drm_fdinfo.c
> @@ -40,7 +40,7 @@ IGT_TEST_DESCRIPTION("Read and verify drm client memory consumption using fdinfo
> #define BO_SIZE (65536)
>
> /* Subtests */
> -static void test_active(int fd, struct drm_xe_query_engine_info *engine)
> +static void test_active(int fd, struct drm_xe_engine *engine)
> {
> struct drm_xe_mem_region *memregion;
> uint64_t memreg = all_memory_regions(fd), region;
> diff --git a/tests/intel/xe_exec_store.c b/tests/intel/xe_exec_store.c
> index 48e843af5..2927214e3 100644
> --- a/tests/intel/xe_exec_store.c
> +++ b/tests/intel/xe_exec_store.c
> @@ -63,7 +63,7 @@ static void store(int fd)
> .syncs = to_user_pointer(&sync),
> };
> struct data *data;
> - struct drm_xe_query_engine_info *engine;
> + struct drm_xe_engine *engine;
> uint32_t vm;
> uint32_t exec_queue;
> uint32_t syncobj;
> diff --git a/tests/intel/xe_noexec_ping_pong.c b/tests/intel/xe_noexec_ping_pong.c
> index 585af413d..9659272b5 100644
> --- a/tests/intel/xe_noexec_ping_pong.c
> +++ b/tests/intel/xe_noexec_ping_pong.c
> @@ -43,7 +43,7 @@
> * there is worked queued on one of the VM's compute exec_queues.
> */
>
> -static void test_ping_pong(int fd, struct drm_xe_query_engine_info *engine)
> +static void test_ping_pong(int fd, struct drm_xe_engine *engine)
> {
> size_t vram_size = xe_vram_size(fd, 0);
> size_t align = xe_get_default_alignment(fd);
> diff --git a/tests/intel/xe_waitfence.c b/tests/intel/xe_waitfence.c
> index bab2bed42..a902ad408 100644
> --- a/tests/intel/xe_waitfence.c
> +++ b/tests/intel/xe_waitfence.c
> @@ -81,7 +81,7 @@ enum waittype {
> static void
> waitfence(int fd, enum waittype wt)
> {
> - struct drm_xe_query_engine_info *engine = NULL;
> + struct drm_xe_engine *engine = NULL;
> struct timespec ts;
> int64_t current, signalled;
> uint32_t bo_1;
> --
> 2.34.1
>
More information about the igt-dev
mailing list