[Intel-xe] [PATCH 3/5] drm/xe: Move defines after relevant fields

Lucas De Marchi lucas.demarchi at intel.com
Fri May 26 20:35:51 UTC 2023


On Fri, May 26, 2023 at 07:08:22PM +0000, Francois Dugast wrote:
>Align on same rule in the whoe file: defines come after the
>relevant fields.

is that a convention from somewhere? See below.

>
>Reported-by: Oded Gabbay <ogabbay at kernel.org>
>Link: https://lists.freedesktop.org/archives/intel-xe/2023-May/004704.html
>Signed-off-by: Francois Dugast <francois.dugast at intel.com>
>---
> include/uapi/drm/xe_drm.h | 40 +++++++++++++++++++--------------------
> 1 file changed, 20 insertions(+), 20 deletions(-)
>
>diff --git a/include/uapi/drm/xe_drm.h b/include/uapi/drm/xe_drm.h
>index 5d34b570a305..160b0aad4a96 100644
>--- a/include/uapi/drm/xe_drm.h
>+++ b/include/uapi/drm/xe_drm.h
>@@ -116,9 +116,6 @@ struct xe_user_extension {
> #define DRM_IOCTL_XE_WAIT_USER_FENCE		DRM_IOWR(DRM_COMMAND_BASE + DRM_XE_WAIT_USER_FENCE, struct drm_xe_wait_user_fence)
> #define DRM_IOCTL_XE_VM_MADVISE			DRM_IOW( DRM_COMMAND_BASE + DRM_XE_VM_MADVISE, struct drm_xe_vm_madvise)
>
>-#define XE_MEM_REGION_CLASS_SYSMEM	0
>-#define XE_MEM_REGION_CLASS_VRAM	1
>-
> struct drm_xe_query_mem_usage {
> 	__u32 num_regions;
> 	__u32 pad;
>@@ -135,9 +132,13 @@ struct drm_xe_query_mem_usage {
> 	} regions[];
> };
>
>+#define XE_MEM_REGION_CLASS_SYSMEM	0
>+#define XE_MEM_REGION_CLASS_VRAM	1
>+
> struct drm_xe_query_config {
> 	__u32 num_params;
> 	__u32 pad;
>+	__u64 info[];
> #define XE_QUERY_CONFIG_REV_AND_DEVICE_ID	0
> #define XE_QUERY_CONFIG_FLAGS			1
> 	#define XE_QUERY_CONFIG_FLAGS_HAS_VRAM		(0x1 << 0)

I thnk it would be much cleaner to go the opposite and ensure a newline
in place so it's not ambiguous.

struct drm_xe_query_config {
	__u32 num_params;
	__u32 pad;

#define XE_QUERY_CONFIG_REV_AND_DEVICE_ID	0
#define XE_QUERY_CONFIG_FLAGS			1
#define ...
	/** @info: .... */
	__u64 info[];
};

Lucas De Marchi

>@@ -148,7 +149,6 @@ struct drm_xe_query_config {
> #define XE_QUERY_CONFIG_MEM_REGION_COUNT	5
> #define XE_QUERY_CONFIG_MAX_ENGINE_PRIORITY	6
> #define XE_QUERY_CONFIG_NUM_PARAM		XE_QUERY_CONFIG_MAX_ENGINE_PRIORITY + 1
>-	__u64 info[];
> };
>
> struct drm_xe_query_gts {
>@@ -161,10 +161,10 @@ struct drm_xe_query_gts {
> 	 */
>
> 	struct drm_xe_query_gt {
>+		__u16 type;
> #define XE_QUERY_GT_TYPE_MAIN		0
> #define XE_QUERY_GT_TYPE_REMOTE		1
> #define XE_QUERY_GT_TYPE_MEDIA		2
>-		__u16 type;
> 		__u16 instance;
> 		__u32 clock_freq;
> 		__u64 features;
>@@ -231,9 +231,9 @@ struct drm_xe_gem_create {
> 	 * @flags: Flags, currently a mask of memory instances of where BO can
> 	 * be placed
> 	 */
>+	__u32 flags;
> #define XE_GEM_CREATE_FLAG_DEFER_BACKING	(0x1 << 24)
> #define XE_GEM_CREATE_FLAG_SCANOUT		(0x1 << 25)
>-	__u32 flags;
>
> 	/**
> 	 * @vm_id: Attached VM, if any
>@@ -294,8 +294,8 @@ struct drm_xe_ext_vm_set_property {
> 	struct xe_user_extension base;
>
> 	/** @property: property to set */
>-#define XE_VM_PROPERTY_BIND_OP_ERROR_CAPTURE_ADDRESS		0
> 	__u32 property;
>+#define XE_VM_PROPERTY_BIND_OP_ERROR_CAPTURE_ADDRESS		0
>
> 	/** @value: property value */
> 	__u64 value;
>@@ -306,8 +306,8 @@ struct drm_xe_ext_vm_set_property {
>
> struct drm_xe_vm_create {
> 	/** @extensions: Pointer to the first extension struct, if any */
>-#define XE_VM_EXTENSION_SET_PROPERTY	0
> 	__u64 extensions;
>+#define XE_VM_EXTENSION_SET_PROPERTY	0
>
> 	/** @flags: Flags */
> 	__u32 flags;
>@@ -368,9 +368,6 @@ struct drm_xe_vm_bind_op {
> 	/** @op: Operation to perform (lower 16 bits) and flags (upper 16 bits) */
> 	__u32 op;
>
>-	/** @mem_region: Memory region to prefetch VMA to, instance not a mask */
>-	__u32 region;
>-
> #define XE_VM_BIND_OP_MAP		0x0
> #define XE_VM_BIND_OP_UNMAP		0x1
> #define XE_VM_BIND_OP_MAP_USERPTR	0x2
>@@ -410,6 +407,9 @@ struct drm_xe_vm_bind_op {
> 	 */
> #define XE_VM_BIND_FLAG_IMMEDIATE	(0x1 << 18)
>
>+	/** @mem_region: Memory region to prefetch VMA to, instance not a mask */
>+	__u32 region;
>+
> 	/** @reserved: Reserved */
> 	__u64 reserved[2];
> };
>@@ -476,6 +476,7 @@ struct drm_xe_engine_set_property {
> 	__u32 engine_id;
>
> 	/** @property: property to set */
>+	__u32 property;
> #define XE_ENGINE_SET_PROPERTY_PRIORITY			0
> #define XE_ENGINE_SET_PROPERTY_TIMESLICE		1
> #define XE_ENGINE_SET_PROPERTY_PREEMPTION_TIMEOUT	2
>@@ -491,7 +492,6 @@ struct drm_xe_engine_set_property {
> #define XE_ENGINE_SET_PROPERTY_ACC_TRIGGER		6
> #define XE_ENGINE_SET_PROPERTY_ACC_NOTIFY		7
> #define XE_ENGINE_SET_PROPERTY_ACC_GRANULARITY		8
>-	__u32 property;
>
> 	/** @value: property value */
> 	__u64 value;
>@@ -520,8 +520,8 @@ struct drm_xe_engine_class_instance {
>
> struct drm_xe_engine_create {
> 	/** @extensions: Pointer to the first extension struct, if any */
>-#define XE_ENGINE_EXTENSION_SET_PROPERTY               0
> 	__u64 extensions;
>+#define XE_ENGINE_EXTENSION_SET_PROPERTY               0
>
> 	/** @width: submission width (number BB per exec) for this engine */
> 	__u16 width;
>@@ -559,8 +559,8 @@ struct drm_xe_engine_get_property {
> 	__u32 engine_id;
>
> 	/** @property: property to get */
>-#define XE_ENGINE_GET_PROPERTY_BAN			0
> 	__u32 property;
>+#define XE_ENGINE_GET_PROPERTY_BAN			0
>
> 	/** @value: property value */
> 	__u64 value;
>@@ -686,26 +686,26 @@ struct drm_xe_wait_user_fence {
> 		__u64 vm_id;
> 	};
> 	/** @op: wait operation (type of comparison) */
>+	__u16 op;
> #define DRM_XE_UFENCE_WAIT_EQ	0
> #define DRM_XE_UFENCE_WAIT_NEQ	1
> #define DRM_XE_UFENCE_WAIT_GT	2
> #define DRM_XE_UFENCE_WAIT_GTE	3
> #define DRM_XE_UFENCE_WAIT_LT	4
> #define DRM_XE_UFENCE_WAIT_LTE	5
>-	__u16 op;
> 	/** @flags: wait flags */
>+	__u16 flags;
> #define DRM_XE_UFENCE_WAIT_SOFT_OP	(1 << 0)	/* e.g. Wait on VM bind */
> #define DRM_XE_UFENCE_WAIT_ABSTIME	(1 << 1)
> #define DRM_XE_UFENCE_WAIT_VM_ERROR	(1 << 2)
>-	__u16 flags;
> 	/** @value: compare value */
> 	__u64 value;
> 	/** @mask: comparison mask */
>+	__u64 mask;
> #define DRM_XE_UFENCE_WAIT_U8		0xffu
> #define DRM_XE_UFENCE_WAIT_U16		0xffffu
> #define DRM_XE_UFENCE_WAIT_U32		0xffffffffu
> #define DRM_XE_UFENCE_WAIT_U64		0xffffffffffffffffu
>-	__u64 mask;
> 	/** @timeout: how long to wait before bailing, value in jiffies */
> 	__s64 timeout;
> 	/**
>@@ -736,6 +736,9 @@ struct drm_xe_vm_madvise {
> 	/** @addr: Address of the VMA to operation on */
> 	__u64 addr;
>
>+	/** @property: property to set */
>+	__u32 property;
>+
> 	/*
> 	 * Setting the preferred location will trigger a migrate of the VMA
> 	 * backing store to new location if the backing store is already
>@@ -771,9 +774,6 @@ struct drm_xe_vm_madvise {
> 	/* Pin the VMA in memory, must be elevated user */
> #define DRM_XE_VM_MADVISE_PIN			6
>
>-	/** @property: property to set */
>-	__u32 property;
>-
> 	/** @value: property value */
> 	__u64 value;
>
>-- 
>2.34.1
>


More information about the Intel-xe mailing list