Re: [RFC PATCH 2/2] drm/xe/uapi: Expose wmtp as engine capability

[Francois Dugast <francois.dugast@intel.com>]

Hi,

On Mon, Feb 19, 2024 at 11:09:13AM +0100, Nirmoy Das wrote:
> 
> On 2/16/2024 8:52 PM, Matt Roper wrote:
> > On Sat, Feb 03, 2024 at 07:05:34AM +0100, Nirmoy Das wrote:
> > > Add capabilities to engine struct which can be used to
> > > expose various capabilities of each engines.
> > > 
> > > Add wmtp as engine capability which is retrieve from
> > > GT info.
> > > 
> > > Take a field from the reserved for that purpose.
> > > 
> > > Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
> > We'll need a userspace consumer for the uapi and some simple IGT tests
> > (e.g., making sure this flag never shows up on the wrong engine types or
> > platforms), but once those are available,
> 
> Thanks Matt, I wanted to get initial feedback.  I will now sync up with UMD
> friends to create a pull request for this, and also work on  IGT tests.
> 
> 
> > 
> >          Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
> > 
> > 
> > Matt
> > 
> > > ---
> > >   drivers/gpu/drm/xe/xe_query.c | 5 +++++
> > >   include/uapi/drm/xe_drm.h     | 6 ++++--
> > >   2 files changed, 9 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/xe/xe_query.c b/drivers/gpu/drm/xe/xe_query.c
> > > index 7e924faeeea0..10981b46c56f 100644
> > > --- a/drivers/gpu/drm/xe/xe_query.c
> > > +++ b/drivers/gpu/drm/xe/xe_query.c
> > > @@ -215,6 +215,11 @@ static int query_engines(struct xe_device *xe,
> > >   			engines->engines[i].instance.pad = 0;
> > >   			memset(engines->engines[i].reserved, 0,
> > >   			       sizeof(engines->engines[i].reserved));
> > > +			if (gt->info.has_wmtp &&
> > > +			    (hwe->class == XE_ENGINE_CLASS_COMPUTE ||
> > > +			     hwe->class == XE_ENGINE_CLASS_RENDER))
> > > +				engines->engines[i].capabilities |=
> > > +					DRM_XE_ENGINE_CAPABILITY_WMTP;
> > >   			i++;
> > >   		}
> > > diff --git a/include/uapi/drm/xe_drm.h b/include/uapi/drm/xe_drm.h
> > > index 50bbea0992d9..5dac079e64de 100644
> > > --- a/include/uapi/drm/xe_drm.h
> > > +++ b/include/uapi/drm/xe_drm.h
> > > @@ -247,9 +247,11 @@ struct drm_xe_engine_class_instance {
> > >   struct drm_xe_engine {
> > >   	/** @instance: The @drm_xe_engine_class_instance */
> > >   	struct drm_xe_engine_class_instance instance;
> > > -
> > > +#define DRM_XE_ENGINE_CAPABILITY_WMTP		BIT(0)

Where does user space get the BIT() macro definition from? We should
probably use an explicit value for this constant, as it is done in
the rest of this header.

> > > +	/** @capabilities: Capabilities of this engine. */
> > > +	__u64 capabilities;
> > >   	/** @reserved: Reserved */
> > > -	__u64 reserved[3];
> > > +	__u64 reserved[2];
> > >   };
> > >   /**
> > > -- 
> > > 2.42.0
> > >