Re: [PATCH v2 5/7] drm/xe: Introduce has_device_atomics_on_smem device info

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

Hi Jose,

On 4/12/2024 3:31 PM, Souza, Jose wrote:
> On Fri, 2024-04-12 at 11:22 +0200, 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.
>>
>> Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
>> ---
>>   drivers/gpu/drm/xe/xe_device_types.h | 2 ++
>>   drivers/gpu/drm/xe/xe_pci.c          | 2 ++
>>   drivers/gpu/drm/xe/xe_pci_types.h    | 1 +
>>   3 files changed, 5 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
>> index bde42fe288ff..c06be1e00f1a 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;
>>   
> This one can be easily replaced by a simple gfx_ip >= 20.
>
> In my opinion it should only be added to xe_graphics_desc when feature check is more complex,
Did occur to me but I haven't seen any existing example so picked a safe 
option. I will add platform check in

xe_info_init() if no one objects.

Thanks,
Nirmoy

>   like the has_atomic_enable_pte_bit.
> Otherwise it will get complex and error prone to add new platforms.
>
>>   #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 9cf45559b915..4e4699962337 100644
>> --- a/drivers/gpu/drm/xe/xe_pci.c
>> +++ b/drivers/gpu/drm/xe/xe_pci.c
>> @@ -165,6 +165,7 @@ static const struct xe_graphics_desc graphics_xelpg = {
>>   	.dma_mask_size = 46, \
>>   	.has_asid = 1, \
>>   	.has_atomic_enable_pte_bit = 1, \
>> +	.has_device_atomics_on_smem = 1, \
>>   	.has_flat_ccs = 1, \
>>   	.has_range_tlb_invalidation = 1, \
>>   	.has_usm = 1, \
>> @@ -630,6 +631,7 @@ 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;
>> +	xe->info.has_device_atomics_on_smem = graphics_desc->has_device_atomics_on_smem;
>>   	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;
>> diff --git a/drivers/gpu/drm/xe/xe_pci_types.h b/drivers/gpu/drm/xe/xe_pci_types.h
>> index e1f2b4879fc2..d403814a7a84 100644
>> --- a/drivers/gpu/drm/xe/xe_pci_types.h
>> +++ b/drivers/gpu/drm/xe/xe_pci_types.h
>> @@ -26,6 +26,7 @@ struct xe_graphics_desc {
>>   
>>   	u8 has_asid:1;
>>   	u8 has_atomic_enable_pte_bit:1;
>> +	u8 has_device_atomics_on_smem:1;
>>   	u8 has_flat_ccs:1;
>>   	u8 has_range_tlb_invalidation:1;
>>   	u8 has_usm:1;