Intel-XE Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] drm/xe/uapi: restructure query config types in an enum
@ 2024-07-29 12:15 Ohad Sharabi
  2024-07-29 12:41 ` Gustavo Sousa
                   ` (9 more replies)
  0 siblings, 10 replies; 15+ messages in thread
From: Ohad Sharabi @ 2024-07-29 12:15 UTC (permalink / raw)
  To: intel-xe; +Cc: obitton, kelbaz, gustavo.sousa, Ohad Sharabi

The config query has entries per config type.

In query_config(), an array is allocated using the last element of
the query config to dictate the array length.

Instead, a more robust approach would be to use the standard "number of
elements" last element of an enum such that one wouldn't need to modify
the ioctl each time another entry is added.

v2:
 - modify `enum drm_xe_query_config_type` doc

Signed-off-by: Ohad Sharabi <osharabi@habana.ai>
---
 drivers/gpu/drm/xe/xe_query.c |  2 +-
 include/uapi/drm/xe_drm.h     | 58 ++++++++++++++++++++++-------------
 2 files changed, 38 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/xe/xe_query.c b/drivers/gpu/drm/xe/xe_query.c
index 73ef6e4c2dc9..aaa60ff764ee 100644
--- a/drivers/gpu/drm/xe/xe_query.c
+++ b/drivers/gpu/drm/xe/xe_query.c
@@ -313,7 +313,7 @@ static int query_mem_regions(struct xe_device *xe,
 
 static int query_config(struct xe_device *xe, struct drm_xe_device_query *query)
 {
-	const u32 num_params = DRM_XE_QUERY_CONFIG_MAX_EXEC_QUEUE_PRIORITY + 1;
+	const u32 num_params = DRM_XE_QUERY_CONFIG_NUM_CONFIGS;
 	size_t size =
 		sizeof(struct drm_xe_query_config) + num_params * sizeof(u64);
 	struct drm_xe_query_config __user *query_ptr =
diff --git a/include/uapi/drm/xe_drm.h b/include/uapi/drm/xe_drm.h
index 29425d7fdc77..9222563cddc3 100644
--- a/include/uapi/drm/xe_drm.h
+++ b/include/uapi/drm/xe_drm.h
@@ -378,6 +378,42 @@ struct drm_xe_query_mem_regions {
 	struct drm_xe_mem_region mem_regions[];
 };
 
+#define DRM_XE_QUERY_CONFIG_FLAG_HAS_VRAM	(1 << 0)
+
+/**
+ * enum drm_xe_query_config_type - Indices for the supported configs returned
+ * in &drm_xe_query_config.info array
+ */
+enum drm_xe_query_config_type {
+	/**
+	 * @DRM_XE_QUERY_CONFIG_REV_AND_DEVICE_ID: Device ID (lower 16 bits)
+	 * and the device revision (next 8 bits).
+	 */
+	DRM_XE_QUERY_CONFIG_REV_AND_DEVICE_ID,
+	/**
+	 * @DRM_XE_QUERY_CONFIG_FLAGS: Flags describing the device
+	 * configuration, see list below.
+	 *
+	 *    - %DRM_XE_QUERY_CONFIG_FLAG_HAS_VRAM - Flag is set if the device
+	 *      has usable VRAM
+	 */
+	DRM_XE_QUERY_CONFIG_FLAGS,
+	/**
+	 * @DRM_XE_QUERY_CONFIG_MIN_ALIGNMENT: Minimal memory alignment
+	 * required by this device, typically SZ_4K or SZ_64K.
+	 */
+	DRM_XE_QUERY_CONFIG_MIN_ALIGNMENT,
+	/** @DRM_XE_QUERY_CONFIG_VA_BITS: Maximum bits of a virtual address. */
+	DRM_XE_QUERY_CONFIG_VA_BITS,
+	/**
+	 * @DRM_XE_QUERY_CONFIG_MAX_EXEC_QUEUE_PRIORITY: Value of the highest
+	 * available exec queue priority.
+	 */
+	DRM_XE_QUERY_CONFIG_MAX_EXEC_QUEUE_PRIORITY,
+	/** @DRM_XE_QUERY_CONFIG_NUM_CONFIGS: must be last. Number of configs */
+	DRM_XE_QUERY_CONFIG_NUM_CONFIGS,
+};
+
 /**
  * struct drm_xe_query_config - describe the device configuration
  *
@@ -385,33 +421,13 @@ struct drm_xe_query_mem_regions {
  * is equal to DRM_XE_DEVICE_QUERY_CONFIG, then the reply uses
  * struct drm_xe_query_config in .data.
  *
- * The index in @info can be:
- *  - %DRM_XE_QUERY_CONFIG_REV_AND_DEVICE_ID - Device ID (lower 16 bits)
- *    and the device revision (next 8 bits)
- *  - %DRM_XE_QUERY_CONFIG_FLAGS - Flags describing the device
- *    configuration, see list below
- *
- *    - %DRM_XE_QUERY_CONFIG_FLAG_HAS_VRAM - Flag is set if the device
- *      has usable VRAM
- *  - %DRM_XE_QUERY_CONFIG_MIN_ALIGNMENT - Minimal memory alignment
- *    required by this device, typically SZ_4K or SZ_64K
- *  - %DRM_XE_QUERY_CONFIG_VA_BITS - Maximum bits of a virtual address
- *  - %DRM_XE_QUERY_CONFIG_MAX_EXEC_QUEUE_PRIORITY - Value of the highest
- *    available exec queue priority
+ * The index in @info are the entries in `enum drm_xe_query_config`
  */
 struct drm_xe_query_config {
 	/** @num_params: number of parameters returned in info */
 	__u32 num_params;
-
 	/** @pad: MBZ */
 	__u32 pad;
-
-#define DRM_XE_QUERY_CONFIG_REV_AND_DEVICE_ID	0
-#define DRM_XE_QUERY_CONFIG_FLAGS			1
-	#define DRM_XE_QUERY_CONFIG_FLAG_HAS_VRAM	(1 << 0)
-#define DRM_XE_QUERY_CONFIG_MIN_ALIGNMENT		2
-#define DRM_XE_QUERY_CONFIG_VA_BITS			3
-#define DRM_XE_QUERY_CONFIG_MAX_EXEC_QUEUE_PRIORITY	4
 	/** @info: array of elements containing the config info */
 	__u64 info[];
 };
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread
* [PATCH v2] drm/xe/uapi: restructure query config types in an enum
@ 2024-07-29 12:12 Ohad Sharabi
  0 siblings, 0 replies; 15+ messages in thread
From: Ohad Sharabi @ 2024-07-29 12:12 UTC (permalink / raw)
  To: intel-xe; +Cc: obitton, kelbaz, Ohad Sharabi

The config query has entries per config type.

In query_config(), an array is allocated using the last element of
the query config to dictate the array length.

Instead, a more robust approach would be to use the standard "number of
elements" last element of an enum such that one wouldn't need to modify
the ioctl each time another entry is added.

Signed-off-by: Ohad Sharabi <osharabi@habana.ai>
---
 drivers/gpu/drm/xe/xe_query.c |  2 +-
 include/uapi/drm/xe_drm.h     | 58 ++++++++++++++++++++++-------------
 2 files changed, 38 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/xe/xe_query.c b/drivers/gpu/drm/xe/xe_query.c
index 73ef6e4c2dc9..aaa60ff764ee 100644
--- a/drivers/gpu/drm/xe/xe_query.c
+++ b/drivers/gpu/drm/xe/xe_query.c
@@ -313,7 +313,7 @@ static int query_mem_regions(struct xe_device *xe,
 
 static int query_config(struct xe_device *xe, struct drm_xe_device_query *query)
 {
-	const u32 num_params = DRM_XE_QUERY_CONFIG_MAX_EXEC_QUEUE_PRIORITY + 1;
+	const u32 num_params = DRM_XE_QUERY_CONFIG_NUM_CONFIGS;
 	size_t size =
 		sizeof(struct drm_xe_query_config) + num_params * sizeof(u64);
 	struct drm_xe_query_config __user *query_ptr =
diff --git a/include/uapi/drm/xe_drm.h b/include/uapi/drm/xe_drm.h
index 29425d7fdc77..9222563cddc3 100644
--- a/include/uapi/drm/xe_drm.h
+++ b/include/uapi/drm/xe_drm.h
@@ -378,6 +378,42 @@ struct drm_xe_query_mem_regions {
 	struct drm_xe_mem_region mem_regions[];
 };
 
+#define DRM_XE_QUERY_CONFIG_FLAG_HAS_VRAM	(1 << 0)
+
+/**
+ * enum drm_xe_query_config_type - Indices for the supported configs returned
+ * in &drm_xe_query_config.info array
+ */
+enum drm_xe_query_config_type {
+	/**
+	 * @DRM_XE_QUERY_CONFIG_REV_AND_DEVICE_ID: Device ID (lower 16 bits)
+	 * and the device revision (next 8 bits).
+	 */
+	DRM_XE_QUERY_CONFIG_REV_AND_DEVICE_ID,
+	/**
+	 * @DRM_XE_QUERY_CONFIG_FLAGS: Flags describing the device
+	 * configuration, see list below.
+	 *
+	 *    - %DRM_XE_QUERY_CONFIG_FLAG_HAS_VRAM - Flag is set if the device
+	 *      has usable VRAM
+	 */
+	DRM_XE_QUERY_CONFIG_FLAGS,
+	/**
+	 * @DRM_XE_QUERY_CONFIG_MIN_ALIGNMENT: Minimal memory alignment
+	 * required by this device, typically SZ_4K or SZ_64K.
+	 */
+	DRM_XE_QUERY_CONFIG_MIN_ALIGNMENT,
+	/** @DRM_XE_QUERY_CONFIG_VA_BITS: Maximum bits of a virtual address. */
+	DRM_XE_QUERY_CONFIG_VA_BITS,
+	/**
+	 * @DRM_XE_QUERY_CONFIG_MAX_EXEC_QUEUE_PRIORITY: Value of the highest
+	 * available exec queue priority.
+	 */
+	DRM_XE_QUERY_CONFIG_MAX_EXEC_QUEUE_PRIORITY,
+	/** @DRM_XE_QUERY_CONFIG_NUM_CONFIGS: must be last. Number of configs */
+	DRM_XE_QUERY_CONFIG_NUM_CONFIGS,
+};
+
 /**
  * struct drm_xe_query_config - describe the device configuration
  *
@@ -385,33 +421,13 @@ struct drm_xe_query_mem_regions {
  * is equal to DRM_XE_DEVICE_QUERY_CONFIG, then the reply uses
  * struct drm_xe_query_config in .data.
  *
- * The index in @info can be:
- *  - %DRM_XE_QUERY_CONFIG_REV_AND_DEVICE_ID - Device ID (lower 16 bits)
- *    and the device revision (next 8 bits)
- *  - %DRM_XE_QUERY_CONFIG_FLAGS - Flags describing the device
- *    configuration, see list below
- *
- *    - %DRM_XE_QUERY_CONFIG_FLAG_HAS_VRAM - Flag is set if the device
- *      has usable VRAM
- *  - %DRM_XE_QUERY_CONFIG_MIN_ALIGNMENT - Minimal memory alignment
- *    required by this device, typically SZ_4K or SZ_64K
- *  - %DRM_XE_QUERY_CONFIG_VA_BITS - Maximum bits of a virtual address
- *  - %DRM_XE_QUERY_CONFIG_MAX_EXEC_QUEUE_PRIORITY - Value of the highest
- *    available exec queue priority
+ * The index in @info are the entries in `enum drm_xe_query_config`
  */
 struct drm_xe_query_config {
 	/** @num_params: number of parameters returned in info */
 	__u32 num_params;
-
 	/** @pad: MBZ */
 	__u32 pad;
-
-#define DRM_XE_QUERY_CONFIG_REV_AND_DEVICE_ID	0
-#define DRM_XE_QUERY_CONFIG_FLAGS			1
-	#define DRM_XE_QUERY_CONFIG_FLAG_HAS_VRAM	(1 << 0)
-#define DRM_XE_QUERY_CONFIG_MIN_ALIGNMENT		2
-#define DRM_XE_QUERY_CONFIG_VA_BITS			3
-#define DRM_XE_QUERY_CONFIG_MAX_EXEC_QUEUE_PRIORITY	4
 	/** @info: array of elements containing the config info */
 	__u64 info[];
 };
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2024-07-29 16:28 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-07-29 12:15 [PATCH v2] drm/xe/uapi: restructure query config types in an enum Ohad Sharabi
2024-07-29 12:41 ` Gustavo Sousa
2024-07-29 13:00 ` ✓ CI.Patch_applied: success for drm/xe/uapi: restructure query config types in an enum (rev3) Patchwork
2024-07-29 13:01 ` ✓ CI.checkpatch: " Patchwork
2024-07-29 13:01 ` [PATCH v2] drm/xe/uapi: restructure query config types in an enum Lucas De Marchi
2024-07-29 13:19   ` Jani Nikula
2024-07-29 13:41     ` Lucas De Marchi
2024-07-29 13:52     ` Gustavo Sousa
2024-07-29 13:02 ` ✓ CI.KUnit: success for drm/xe/uapi: restructure query config types in an enum (rev3) Patchwork
2024-07-29 13:14 ` ✓ CI.Build: " Patchwork
2024-07-29 13:16 ` ✓ CI.Hooks: " Patchwork
2024-07-29 13:17 ` ✓ CI.checksparse: " Patchwork
2024-07-29 13:37 ` ✓ CI.BAT: " Patchwork
2024-07-29 16:28 ` ✗ CI.FULL: failure " Patchwork
  -- strict thread matches above, loose matches on Subject: below --
2024-07-29 12:12 [PATCH v2] drm/xe/uapi: restructure query config types in an enum Ohad Sharabi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox