Re: [PATCH] drm/i915/selftests: Set always_coherent to false when reading from CPU
On 5/17/2024 1:53 PM, Jani Nikula wrote:
On Fri, 17 May 2024, Nirmoy Das wrote:
Hi Jani,
On 5/17/2024 9:39 AM, Jani Nikula wrote:
On Thu, 16 May 2024, Nirmoy Das 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
Cc: Janusz Krzysztofik
Cc: Jonathan Cavitt
Signed-off-by: Nirmoy Das
---
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);
Re: [PATCH] drm/i915/selftests: Set always_coherent to false when reading from CPU
On Fri, 17 May 2024, Nirmoy Das wrote:
> Hi Jani,
>
> On 5/17/2024 9:39 AM, Jani Nikula wrote:
>> On Thu, 16 May 2024, Nirmoy Das 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?
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
>>> Cc: Janusz Krzysztofik
>>> Cc: Jonathan Cavitt
>>> Signed-off-by: Nirmoy Das
>>> ---
>>> 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);
--
Jani Nikula, Intel
Re: [PATCH] drm/i915/selftests: Set always_coherent to false when reading from CPU
Hi Nirmoy,
On Thu, May 16, 2024 at 05:14:03PM +0200, Nirmoy Das wrote:
> The previous commit 'commit 8d4ba9fc1c6c ("drm/i915/selftests: Pick
> 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
> Cc: Janusz Krzysztofik
> Cc: Jonathan Cavitt
> Signed-off-by: Nirmoy Das
you can add:
Reviewed-by: Andi Shyti
Thanks,
Andi
Re: [PATCH] drm/i915/selftests: Set always_coherent to false when reading from CPU
Hi Jani,
On 5/17/2024 9:39 AM, Jani Nikula wrote:
On Thu, 16 May 2024, Nirmoy Das 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
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
Cc: Janusz Krzysztofik
Cc: Jonathan Cavitt
Signed-off-by: Nirmoy Das
---
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);
Re: [PATCH] drm/i915/selftests: Set always_coherent to false when reading from CPU
On Thu, 16 May 2024, Nirmoy Das 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...
And probably add:
Fixes: 8d4ba9fc1c6c ("drm/i915/selftests: Pick correct caching mode.")
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
> Cc: Janusz Krzysztofik
> Cc: Jonathan Cavitt
> Signed-off-by: Nirmoy Das
> ---
> 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);
--
Jani Nikula, Intel
RE: [PATCH] drm/i915/selftests: Set always_coherent to false when reading from CPU
-Original Message-
From: Das, Nirmoy
Sent: Thursday, May 16, 2024 8:14 AM
To: intel-gfx@lists.freedesktop.org
Cc: dri-de...@lists.freedesktop.org; Das, Nirmoy ; Andi
Shyti ; Janusz Krzysztofik
; Cavitt, Jonathan
Subject: [PATCH] drm/i915/selftests: Set always_coherent to false when reading
from CPU
>
> The previous commit 'commit 8d4ba9fc1c6c ("drm/i915/selftests: Pick
> 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
> Cc: Janusz Krzysztofik
> Cc: Jonathan Cavitt
> Signed-off-by: Nirmoy Das
I see no problem with this
Reviewed-by: Jonathan Cavitt
-Jonathan Cavitt
> ---
> 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);
> --
> 2.42.0
>
>
