Re: [PATCH v4 3/5] drm/xe: Introduce has_device_atomics_on_smem device info

["Welty, Brian" <brian.welty@intel.com>]

On 4/25/2024 3:23 PM, Nirmoy Das wrote:
> Add has_device_atomics_on_smem to specify that a device
> supports device atomics on system memory. Currently XE2
> supports this so set this for XE2.
> 
> v2: Set has_device_atomics_on_smem for all platform but
>      PVC.
> 
> Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
> Reviewed-by: Oak Zeng <oak.zeng@intel.com>
> Reviewed-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>   drivers/gpu/drm/xe/xe_device_types.h | 2 ++
>   drivers/gpu/drm/xe/xe_pci.c          | 2 ++
>   2 files changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
> index 7cddb00f9c35..0af739981ebf 100644
> --- a/drivers/gpu/drm/xe/xe_device_types.h
> +++ b/drivers/gpu/drm/xe/xe_device_types.h
> @@ -283,6 +283,8 @@ struct xe_device {
>   		u8 skip_guc_pc:1;
>   		/** @info.has_atomic_enable_pte_bit: Device has atomic enable PTE bit */
>   		u8 has_atomic_enable_pte_bit:1;
> +		/** @info.has_device_atomics_on_smem: Supports device atomics on SMEM */
> +		u8 has_device_atomics_on_smem:1;
>   
>   #if IS_ENABLED(CONFIG_DRM_XE_DISPLAY)
>   		struct {
> diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
> index 5618318e6f3a..c6e8a264348c 100644
> --- a/drivers/gpu/drm/xe/xe_pci.c
> +++ b/drivers/gpu/drm/xe/xe_pci.c
> @@ -632,6 +632,8 @@ static int xe_info_init(struct xe_device *xe,
>   	xe->info.vm_max_level = graphics_desc->vm_max_level;
>   	xe->info.has_asid = graphics_desc->has_asid;
>   	xe->info.has_atomic_enable_pte_bit = graphics_desc->has_atomic_enable_pte_bit;
> +	if (xe->info.platform != XE_PVC)
> +		xe->info.has_device_atomics_on_smem = 1;

Minor comment...
I guess no harm with how this is used later.

But seems best to not set unless has_atomic_enable_pte_bit is also set?

     if (xe->info.platform != XE_PVC && xe->info.has_atomic_enable_pte_bit)
          xe->info.has_device_atomics_on_smem = 1;

Or:
     xe->info.has_device_atomics_on_smem = (xe->info.platform != XE_PVC 
&& xe->info.has_atomic_enable_pte_bit);


-Brian


>   	xe->info.has_flat_ccs = graphics_desc->has_flat_ccs;
>   	xe->info.has_range_tlb_invalidation = graphics_desc->has_range_tlb_invalidation;
>   	xe->info.has_usm = graphics_desc->has_usm;