[Intel-xe] [PATCH 3/5] drm/xe: Move defines after relevant fields
Francois Dugast
francois.dugast at intel.com
Tue May 30 09:28:26 UTC 2023
On Fri, May 26, 2023 at 01:35:51PM -0700, Lucas De Marchi wrote:
> 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.
Not that I know. It is one of the patterns previously used in this file but
I am fine changing it to what you described below, in a new revision.
Francois
>
> >
> > 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