Re: [PATCH] drm/i915/selftests: Set always_coherent to false when reading from CPU

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

On 5/17/2024 1:53 PM, Jani Nikula wrote:
> On Fri, 17 May 2024, Nirmoy Das <nirmoy.das@linux.intel.com> wrote:
>> Hi Jani,
>>
>> On 5/17/2024 9:39 AM, Jani Nikula wrote:
>>> On Thu, 16 May 2024, Nirmoy Das <nirmoy.das@intel.com> wrote:
>>>> The previous commit 'commit 8d4ba9fc1c6c ("drm/i915/selftests: Pick
>>> "previous commit" is a fairly vague reference once this gets
>>> committed. It's not going to be "previous" in any meaningful sense.
>>>
>>> Please just start with:
>>>
>>> Commit 8d4ba9fc1c6c ("drm/i915/selftests: Pick correct caching mode.")
>>> was not complete...
>> Will do that.
>>
>>
>>> And probably add:
>>>
>>> Fixes: 8d4ba9fc1c6c ("drm/i915/selftests: Pick correct caching mode.")
>> Do we need Fixes for selftest ? I always assumed it is not required as
>> this code is for debug/CI
> Maybe not for stuff that's already in stable, but we do run CI on
> drm-next and -rc kernels, and if this causes issues there, why not have
> them fixed?

Not sure a commit with Fixes flows from drm-intel-next to drm-next/-rc 
but I see no issue adding Fixes without CC-ing to stable.

Pushed it to drm-intel-next with above modifications.  b4-shazam picked 
Fixes as well which was nice.


Thanks,

Nirmoy

>
> BR,
> Jani.
>
>>
>> Thanks,
>>
>> Nirmoy
>>
>>> BR,
>>> Jani.
>>>
>>>> correct caching mode.")' was not complete as for non LLC  sharing platforms
>>>> cpu read can happen from LLC which probably doesn't have the latest
>>>> changes made by GPU.
>>>>
>>>> Cc: Andi Shyti <andi.shyti@linux.intel.com>
>>>> Cc: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
>>>> Cc: Jonathan Cavitt <jonathan.cavitt@intel.com>
>>>> Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
>>>> ---
>>>>    drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c | 2 +-
>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c
>>>> index 65a931ea80e9..3527b8f446fe 100644
>>>> --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c
>>>> +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c
>>>> @@ -196,7 +196,7 @@ static int verify_access(struct drm_i915_private *i915,
>>>>    	if (err)
>>>>    		goto out_file;
>>>>    
>>>> -	mode = intel_gt_coherent_map_type(to_gt(i915), native_obj, true);
>>>> +	mode = intel_gt_coherent_map_type(to_gt(i915), native_obj, false);
>>>>    	vaddr = i915_gem_object_pin_map_unlocked(native_obj, mode);
>>>>    	if (IS_ERR(vaddr)) {
>>>>    		err = PTR_ERR(vaddr);