[Intel-xe] [PATCH v5 10/20] drm/xe: Remove XE_EXEC_QUEUE_SET_PROPERTY_COMPUTE_MODE from uAPI

[Rodrigo Vivi]

On Tue, Oct 17, 2023 at 08:37:47AM -0700, Dixit, Ashutosh wrote:
> On Fri, 06 Oct 2023 02:59:33 -0700, Francois Dugast wrote:
> >
> 
> Hi Francois,
> 
> > diff --git a/include/uapi/drm/xe_drm.h b/include/uapi/drm/xe_drm.h
> > index ad21ba1d6e0b..2a9e04024723 100644
> > --- a/include/uapi/drm/xe_drm.h
> > +++ b/include/uapi/drm/xe_drm.h
> > @@ -781,21 +781,14 @@ struct drm_xe_exec_queue_set_property {
> >	/** @exec_queue_id: Exec queue ID */
> >	__u32 exec_queue_id;
> >
> > -#define XE_EXEC_QUEUE_SET_PROPERTY_PRIORITY			0
> > +#define XE_EXEC_QUEUE_SET_PROPERTY_PRIORITY		0
> >  #define XE_EXEC_QUEUE_SET_PROPERTY_TIMESLICE		1
> >  #define XE_EXEC_QUEUE_SET_PROPERTY_PREEMPTION_TIMEOUT	2
> > -	/*
> > -	 * Long running or ULLS engine mode. DMA fences not allowed in this
> > -	 * mode. Must match the value of DRM_XE_VM_CREATE_COMPUTE_MODE, serves
> > -	 * as a sanity check the UMD knows what it is doing. Can only be set at
> > -	 * engine create time.
> > -	 */
> > -#define XE_EXEC_QUEUE_SET_PROPERTY_COMPUTE_MODE		3
> > -#define XE_EXEC_QUEUE_SET_PROPERTY_PERSISTENCE		4
> > -#define XE_EXEC_QUEUE_SET_PROPERTY_JOB_TIMEOUT		5
> > -#define XE_EXEC_QUEUE_SET_PROPERTY_ACC_TRIGGER		6
> > -#define XE_EXEC_QUEUE_SET_PROPERTY_ACC_NOTIFY		7
> > -#define XE_EXEC_QUEUE_SET_PROPERTY_ACC_GRANULARITY		8
> > +#define XE_EXEC_QUEUE_SET_PROPERTY_PERSISTENCE		3
> > +#define XE_EXEC_QUEUE_SET_PROPERTY_JOB_TIMEOUT		4
> > +#define XE_EXEC_QUEUE_SET_PROPERTY_ACC_TRIGGER		5
> > +#define XE_EXEC_QUEUE_SET_PROPERTY_ACC_NOTIFY		6
> > +#define XE_EXEC_QUEUE_SET_PROPERTY_ACC_GRANULARITY	7
> >	/** @property: property to set */
> >	__u32 property;
> >
> >	/** @value: property value */
> >	__u64 value;
> 
> Mostly a nit, but I had a question about this because I am facing a similar
> decision elsewhere. Why do we have one property/value pair in this struct
> when all the rest of the property/value pairs are coming in via the
> extensions? Basically, how about removing this property/value pair from
> thus struct and let all the property/value pairs come in only via
> extensions? I think that would both simplify the code a bit and be more
> consistent. Thoughts?

or maybe the other way around and do not have extensions on the first version
of the api?

just thinking out loud, but no hard feelings for either way...

> 
> Thanks.
> --
> Ashutosh