Re: [PATCH] drm/amdgpu: do not execute 0-sized IBs
On 03/10/2019 15:12, Deucher, Alexander wrote:
> Does some variant of the patch on this thread help?
> https://patchwork.freedesktop.org/patch/333068/
Hi Alex,
The added condition in this patch is:
!(adev->asic_type >= CHIP_NAVI10 && adev->asic_type <= CHIP_NAVI12) ||
which will evaluate to "false ||" on Navi10 so I don't think it'll help.
I'll send an updated version of my patch that will only modify
gmc_v10_0_flush_gpu_tlb
to not send a 0-sized IB.
Thanks,
Pierre-Eric
>
> Alex
>
> --
> *From:* amd-gfx on behalf of
> Pelloux-prayer, Pierre-eric
> *Sent:* Thursday, October 3, 2019 4:25 AM
> *To:* Koenig, Christian ;
> amd-gfx@lists.freedesktop.org
> *Subject:* Re: [PATCH] drm/amdgpu: do not execute 0-sized IBs
>
>
> On 03/10/2019 10:09, Christian König wrote:
>> Am 03.10.19 um 10:03 schrieb Pelloux-prayer, Pierre-eric:
>>> This can be safely skipped entirely.
>>> This seems to help with https://bugs.freedesktop.org/show_bug.cgi?id=111481.
>>
>> NAK, please instead fix gmc_v10_0_flush_gpu_tlb to include at least some NOP
>> in the submitted IBs.
>
> Is there any interest in executing an empty (or only filled with NOPs) IB?
>
> Anyway I can modify the patch to do this.
>
> Thanks,
> Pierre-Eric
>
>>
>> Christian.
>>
>>>
>>> Signed-off-by: Pierre-Eric Pelloux-Prayer
>>>
>>> ---
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c | 5 +
>>> 1 file changed, 5 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
>>> index 60655834d649..aa163e679f1f 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
>>> @@ -227,6 +227,11 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring,
>>> unsigned num_ibs,
>>> !amdgpu_sriov_vf(adev)) /* for SRIOV preemption, Preamble CE
>>> ib must be inserted anyway */
>>> continue;
>>> + if (ib->length_dw == 0) {
>>> + /* On Navi gmc_v10_0_flush_gpu_tlb emits 0 sized IB */
>>> + continue;
>>> + }
>>> +
>>> amdgpu_ring_emit_ib(ring, job, ib, status);
>>> status &= ~AMDGPU_HAVE_CTX_SWITCH;
>>> }
>>
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: do not execute 0-sized IBs
Does some variant of the patch on this thread help?
https://patchwork.freedesktop.org/patch/333068/
Alex
From: amd-gfx on behalf of
Pelloux-prayer, Pierre-eric
Sent: Thursday, October 3, 2019 4:25 AM
To: Koenig, Christian ; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/amdgpu: do not execute 0-sized IBs
On 03/10/2019 10:09, Christian König wrote:
> Am 03.10.19 um 10:03 schrieb Pelloux-prayer, Pierre-eric:
>> This can be safely skipped entirely.
>> This seems to help with https://bugs.freedesktop.org/show_bug.cgi?id=111481.
>
> NAK, please instead fix gmc_v10_0_flush_gpu_tlb to include at least some NOP
> in the submitted IBs.
Is there any interest in executing an empty (or only filled with NOPs) IB?
Anyway I can modify the patch to do this.
Thanks,
Pierre-Eric
>
> Christian.
>
>>
>> Signed-off-by: Pierre-Eric Pelloux-Prayer
>>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c | 5 +
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
>> index 60655834d649..aa163e679f1f 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
>> @@ -227,6 +227,11 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring,
>> unsigned num_ibs,
>> !amdgpu_sriov_vf(adev)) /* for SRIOV preemption, Preamble CE
>> ib must be inserted anyway */
>> continue;
>> +if (ib->length_dw == 0) {
>> +/* On Navi gmc_v10_0_flush_gpu_tlb emits 0 sized IB */
>> +continue;
>> +}
>> +
>> amdgpu_ring_emit_ib(ring, job, ib, status);
>> status &= ~AMDGPU_HAVE_CTX_SWITCH;
>> }
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: do not execute 0-sized IBs
Am 03.10.2019 10:25 schrieb "Pelloux-prayer, Pierre-eric"
:
On 03/10/2019 10:09, Christian König wrote:
> Am 03.10.19 um 10:03 schrieb Pelloux-prayer, Pierre-eric:
>> This can be safely skipped entirely.
>> This seems to help with https://bugs.freedesktop.org/show_bug.cgi?id=111481.
>
> NAK, please instead fix gmc_v10_0_flush_gpu_tlb to include at least some NOP
> in the submitted IBs.
Is there any interest in executing an empty (or only filled with NOPs) IB?
Yeah, we used to have some dummy zero sized IBs for the MM engines which
otherwise couldn't execute a fence command.
It shouldn't matter for modern firmware/hardware, but you could actually
silently break somewhere else with this, so better not do this.
Sorry should have mentioned that directly,
Christian.
Anyway I can modify the patch to do this.
Thanks,
Pierre-Eric
>
> Christian.
>
>>
>> Signed-off-by: Pierre-Eric Pelloux-Prayer
>>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c | 5 +
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
>> index 60655834d649..aa163e679f1f 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
>> @@ -227,6 +227,11 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring,
>> unsigned num_ibs,
>> !amdgpu_sriov_vf(adev)) /* for SRIOV preemption, Preamble CE
>> ib must be inserted anyway */
>> continue;
>> +if (ib->length_dw == 0) {
>> +/* On Navi gmc_v10_0_flush_gpu_tlb emits 0 sized IB */
>> +continue;
>> +}
>> +
>> amdgpu_ring_emit_ib(ring, job, ib, status);
>> status &= ~AMDGPU_HAVE_CTX_SWITCH;
>> }
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: do not execute 0-sized IBs
On 03/10/2019 10:09, Christian König wrote:
> Am 03.10.19 um 10:03 schrieb Pelloux-prayer, Pierre-eric:
>> This can be safely skipped entirely.
>> This seems to help with https://bugs.freedesktop.org/show_bug.cgi?id=111481.
>
> NAK, please instead fix gmc_v10_0_flush_gpu_tlb to include at least some NOP
> in the submitted IBs.
Is there any interest in executing an empty (or only filled with NOPs) IB?
Anyway I can modify the patch to do this.
Thanks,
Pierre-Eric
>
> Christian.
>
>>
>> Signed-off-by: Pierre-Eric Pelloux-Prayer
>>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c | 5 +
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
>> index 60655834d649..aa163e679f1f 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
>> @@ -227,6 +227,11 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring,
>> unsigned num_ibs,
>> !amdgpu_sriov_vf(adev)) /* for SRIOV preemption, Preamble CE
>> ib must be inserted anyway */
>> continue;
>> + if (ib->length_dw == 0) {
>> + /* On Navi gmc_v10_0_flush_gpu_tlb emits 0 sized IB */
>> + continue;
>> + }
>> +
>> amdgpu_ring_emit_ib(ring, job, ib, status);
>> status &= ~AMDGPU_HAVE_CTX_SWITCH;
>> }
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: do not execute 0-sized IBs
Am 03.10.19 um 10:03 schrieb Pelloux-prayer, Pierre-eric:
This can be safely skipped entirely.
This seems to help with https://bugs.freedesktop.org/show_bug.cgi?id=111481.
NAK, please instead fix gmc_v10_0_flush_gpu_tlb to include at least some
NOP in the submitted IBs.
Christian.
Signed-off-by: Pierre-Eric Pelloux-Prayer
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
index 60655834d649..aa163e679f1f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
@@ -227,6 +227,11 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned
num_ibs,
!amdgpu_sriov_vf(adev)) /* for SRIOV preemption, Preamble
CE ib must be inserted anyway */
continue;
+ if (ib->length_dw == 0) {
+ /* On Navi gmc_v10_0_flush_gpu_tlb emits 0 sized IB */
+ continue;
+ }
+
amdgpu_ring_emit_ib(ring, job, ib, status);
status &= ~AMDGPU_HAVE_CTX_SWITCH;
}
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amdgpu: do not execute 0-sized IBs
This can be safely skipped entirely.
This seems to help with https://bugs.freedesktop.org/show_bug.cgi?id=111481.
Signed-off-by: Pierre-Eric Pelloux-Prayer
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
index 60655834d649..aa163e679f1f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
@@ -227,6 +227,11 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned
num_ibs,
!amdgpu_sriov_vf(adev)) /* for SRIOV preemption, Preamble
CE ib must be inserted anyway */
continue;
+ if (ib->length_dw == 0) {
+ /* On Navi gmc_v10_0_flush_gpu_tlb emits 0 sized IB */
+ continue;
+ }
+
amdgpu_ring_emit_ib(ring, job, ib, status);
status &= ~AMDGPU_HAVE_CTX_SWITCH;
}
--
2.23.0.rc1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
