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

[Nirmoy Das <nirmoy.das@intel.com>]

On 7/12/2024 6:10 PM, Francois Dugast wrote:
> 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.

I was unaware of it. I will remove the BIT macro.


Thanks,

Nirmoy

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