Re: [PATCH i-g-t] tests/intel/xe_vm: Fix WB caching PAT index for coherency mode

["Randhawa, Jagmeet" <jagmeet.randhawa@intel.com>]

On 3/26/2024 11:46 AM, Zeng, Oak wrote:
> Hi Jagmeet,
>
>> -----Original Message-----
>> From: Randhawa, Jagmeet <jagmeet.randhawa@intel.com>
>> Sent: Monday, March 25, 2024 5:27 PM
>> Cc: igt-dev@lists.freedesktop.org; Zeng, Oak <oak.zeng@intel.com>; Cavitt,
>> Jonathan <jonathan.cavitt@intel.com>; Auld, Matthew
>> <matthew.auld@intel.com>; kamil.konieczny@linux.intel.com; Randhawa,
>> Jagmeet <jagmeet.randhawa@intel.com>
>> Subject: [PATCH i-g-t] tests/intel/xe_vm: Fix WB caching PAT index for coherency
>> mode
>>
>> Resolve a coherency conflict by setting the
>> correct PAT index for WB caching with
>> intel_get_pat_idx_wb(fd), addressing the
>> mismatch between the default "XE_COH_NONE"
>> coherency mode and DRM_XE_GEM_CPU_CACHING_WB.
> I feel the description here is still not very clear. Isn't a confliction b/t the default cpu caching_mode and default pat_index?
>
>   Again, XE_COH_NONE is a xekmd *internal* concept, not part of xekmd uAPI. User can't set the XE_COH_NONE mode. I think this is determined by pat_index....
>
> By looking at driver, I figured the only not allowed setting is: cpu_caching == WB && a pat_index which indicates a XE_COH_NONE coherence mode...
>
> So maybe something like: driver doesn't allow cpu_caching == WB && a pat_index which indicates a XE_COH_NONE coherence mode
>
> The code looks good to me. So it is reviewed-by: Oak Zeng <oak.zeng@intel.com>. Improve a little the description message if you want.
>
>
> Oak

I agree, the description has now been updated and better explains the 
code change,

Thank you!

>
>
>> This conflict occurs for VLK:54352.
>>
>> Cc: Jonathan Cavitt <jonathan.cavitt@intel.com>
>> Cc: Matthew Auld <matthew.auld@intel.com>
>> Cc: Oak Zeng <oak.zeng@intel.com>
>> Signed-off-by: Jagmeet Randhawa <jagmeet.randhawa@intel.com>
>> ---
>>   tests/intel/xe_vm.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/tests/intel/xe_vm.c b/tests/intel/xe_vm.c
>> index 7c061c497..1f458174d 100644
>> --- a/tests/intel/xe_vm.c
>> +++ b/tests/intel/xe_vm.c
>> @@ -1789,6 +1789,7 @@ static void bind_flag_invalid(int fd)
>>   	bind.bind.range = bo_size;
>>   	bind.bind.addr = addr;
>>   	bind.bind.op = DRM_XE_VM_BIND_OP_MAP;
>> +	bind.bind.pat_index = intel_get_pat_idx_wb(fd);
>>   	bind.num_syncs = 1;
>>   	bind.syncs = (uintptr_t)sync;
>>
>> --
>> 2.25.1