* [PATCH] drm/amdgpu: fix ocl test performance drop
@ 2017-05-18 7:45 Flora Cui
[not found] ` <1495093501-13313-1-git-send-email-Flora.Cui-5C7GfCeVMHo@public.gmane.org>
0 siblings, 1 reply; 14+ messages in thread
From: Flora Cui @ 2017-05-18 7:45 UTC (permalink / raw)
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW; +Cc: Flora Cui
partial revert commit <6971d3d> - drm/amdgpu: cleanup logic in
amdgpu_vm_flush
Change-Id: Iadce9d613dfe9a739643a74050cea55854832adb
Signed-off-by: Flora Cui <Flora.Cui@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 14 +++++---------
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 88420dc..a96bad6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -743,23 +743,19 @@ int amdgpu_vm_flush(struct amdgpu_ring *ring, struct amdgpu_job *job)
id->gws_size != job->gws_size ||
id->oa_base != job->oa_base ||
id->oa_size != job->oa_size);
- bool vm_flush_needed = job->vm_needs_flush ||
- amdgpu_vm_ring_has_compute_vm_bug(ring);
unsigned patch_offset = 0;
int r;
- if (amdgpu_vm_had_gpu_reset(adev, id)) {
- gds_switch_needed = true;
- vm_flush_needed = true;
- }
-
- if (!vm_flush_needed && !gds_switch_needed)
+ if (!job->vm_needs_flush && !gds_switch_needed &&
+ !amdgpu_vm_had_gpu_reset(adev, id) &&
+ !amdgpu_vm_ring_has_compute_vm_bug(ring))
return 0;
if (ring->funcs->init_cond_exec)
patch_offset = amdgpu_ring_init_cond_exec(ring);
- if (ring->funcs->emit_vm_flush && vm_flush_needed) {
+ if (ring->funcs->emit_vm_flush &&
+ (job->vm_needs_flush || amdgpu_vm_had_gpu_reset(adev, id))) {
struct fence *fence;
trace_amdgpu_vm_flush(ring, job->vm_id, job->vm_pd_addr);
--
2.7.4
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 14+ messages in thread[parent not found: <1495093501-13313-1-git-send-email-Flora.Cui-5C7GfCeVMHo@public.gmane.org>]
* Re: [PATCH] drm/amdgpu: fix ocl test performance drop [not found] ` <1495093501-13313-1-git-send-email-Flora.Cui-5C7GfCeVMHo@public.gmane.org> @ 2017-05-18 11:38 ` Christian König [not found] ` <4b2d410d-544f-4e72-11ae-8db4a1d797d3-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org> 0 siblings, 1 reply; 14+ messages in thread From: Christian König @ 2017-05-18 11:38 UTC (permalink / raw) To: Flora Cui, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW Am 18.05.2017 um 09:45 schrieb Flora Cui: > partial revert commit <6971d3d> - drm/amdgpu: cleanup logic in > amdgpu_vm_flush > > Change-Id: Iadce9d613dfe9a739643a74050cea55854832adb > Signed-off-by: Flora Cui <Flora.Cui@amd.com> I don't see how the revert should be faster than the original. Especially that amdgpu_vm_had_gpu_reset() is now called twice sounds like more overhead than necessary. Please explain further. Christian. > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 14 +++++--------- > 1 file changed, 5 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > index 88420dc..a96bad6 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > @@ -743,23 +743,19 @@ int amdgpu_vm_flush(struct amdgpu_ring *ring, struct amdgpu_job *job) > id->gws_size != job->gws_size || > id->oa_base != job->oa_base || > id->oa_size != job->oa_size); > - bool vm_flush_needed = job->vm_needs_flush || > - amdgpu_vm_ring_has_compute_vm_bug(ring); > unsigned patch_offset = 0; > int r; > > - if (amdgpu_vm_had_gpu_reset(adev, id)) { > - gds_switch_needed = true; > - vm_flush_needed = true; > - } > - > - if (!vm_flush_needed && !gds_switch_needed) > + if (!job->vm_needs_flush && !gds_switch_needed && > + !amdgpu_vm_had_gpu_reset(adev, id) && > + !amdgpu_vm_ring_has_compute_vm_bug(ring)) > return 0; > > if (ring->funcs->init_cond_exec) > patch_offset = amdgpu_ring_init_cond_exec(ring); > > - if (ring->funcs->emit_vm_flush && vm_flush_needed) { > + if (ring->funcs->emit_vm_flush && > + (job->vm_needs_flush || amdgpu_vm_had_gpu_reset(adev, id))) { > struct fence *fence; > > trace_amdgpu_vm_flush(ring, job->vm_id, job->vm_pd_addr); _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 14+ messages in thread
[parent not found: <4b2d410d-544f-4e72-11ae-8db4a1d797d3-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>]
* Re: [PATCH] drm/amdgpu: fix ocl test performance drop [not found] ` <4b2d410d-544f-4e72-11ae-8db4a1d797d3-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org> @ 2017-05-19 2:25 ` Flora Cui 2017-05-19 3:36 ` Flora Cui 2017-05-19 11:57 ` Christian König 0 siblings, 2 replies; 14+ messages in thread From: Flora Cui @ 2017-05-19 2:25 UTC (permalink / raw) To: Christian König; +Cc: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW On Thu, May 18, 2017 at 01:38:15PM +0200, Christian König wrote: > Am 18.05.2017 um 09:45 schrieb Flora Cui: > >partial revert commit <6971d3d> - drm/amdgpu: cleanup logic in > >amdgpu_vm_flush > > > >Change-Id: Iadce9d613dfe9a739643a74050cea55854832adb > >Signed-off-by: Flora Cui <Flora.Cui@amd.com> > > I don't see how the revert should be faster than the original. > > Especially that amdgpu_vm_had_gpu_reset() is now called twice sounds like > more overhead than necessary. > > Please explain further. > > Christian. > > >--- > > drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 14 +++++--------- > > 1 file changed, 5 insertions(+), 9 deletions(-) > > > >diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > >index 88420dc..a96bad6 100644 > >--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > >+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > >@@ -743,23 +743,19 @@ int amdgpu_vm_flush(struct amdgpu_ring *ring, struct amdgpu_job *job) > > id->gws_size != job->gws_size || > > id->oa_base != job->oa_base || > > id->oa_size != job->oa_size); > >- bool vm_flush_needed = job->vm_needs_flush || > >- amdgpu_vm_ring_has_compute_vm_bug(ring); > > unsigned patch_offset = 0; > > int r; > >- if (amdgpu_vm_had_gpu_reset(adev, id)) { > >- gds_switch_needed = true; > >- vm_flush_needed = true; > >- } > >- > >- if (!vm_flush_needed && !gds_switch_needed) > >+ if (!job->vm_needs_flush && !gds_switch_needed && > >+ !amdgpu_vm_had_gpu_reset(adev, id) && > >+ !amdgpu_vm_ring_has_compute_vm_bug(ring)) > > return 0; > > if (ring->funcs->init_cond_exec) > > patch_offset = amdgpu_ring_init_cond_exec(ring); > >- if (ring->funcs->emit_vm_flush && vm_flush_needed) { [flora]: for compute ring & amdgpu_vm_ring_has_compute_vm_bug(), a vm_flush is inserted. This might cause performance drop. > >+ if (ring->funcs->emit_vm_flush && > >+ (job->vm_needs_flush || amdgpu_vm_had_gpu_reset(adev, id))) { > > struct fence *fence; > > trace_amdgpu_vm_flush(ring, job->vm_id, job->vm_pd_addr); > > _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] drm/amdgpu: fix ocl test performance drop 2017-05-19 2:25 ` Flora Cui @ 2017-05-19 3:36 ` Flora Cui 2017-05-19 5:16 ` zhoucm1 2017-05-23 17:51 ` Deucher, Alexander 2017-05-19 11:57 ` Christian König 1 sibling, 2 replies; 14+ messages in thread From: Flora Cui @ 2017-05-19 3:36 UTC (permalink / raw) To: Christian K�nig; +Cc: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW btw, what's about gfx9 for amdgpu_vm_ring_has_compute_vm_bug()? Is the workaround still needed? On Fri, May 19, 2017 at 10:25:19AM +0800, Flora Cui wrote: > On Thu, May 18, 2017 at 01:38:15PM +0200, Christian König wrote: > > Am 18.05.2017 um 09:45 schrieb Flora Cui: > > >partial revert commit <6971d3d> - drm/amdgpu: cleanup logic in > > >amdgpu_vm_flush > > > > > >Change-Id: Iadce9d613dfe9a739643a74050cea55854832adb > > >Signed-off-by: Flora Cui <Flora.Cui@amd.com> > > > > I don't see how the revert should be faster than the original. > > > > Especially that amdgpu_vm_had_gpu_reset() is now called twice sounds like > > more overhead than necessary. > > > > Please explain further. > > > > Christian. > > > > >--- > > > drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 14 +++++--------- > > > 1 file changed, 5 insertions(+), 9 deletions(-) > > > > > >diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > > >index 88420dc..a96bad6 100644 > > >--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > > >+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > > >@@ -743,23 +743,19 @@ int amdgpu_vm_flush(struct amdgpu_ring *ring, struct amdgpu_job *job) > > > id->gws_size != job->gws_size || > > > id->oa_base != job->oa_base || > > > id->oa_size != job->oa_size); > > >- bool vm_flush_needed = job->vm_needs_flush || > > >- amdgpu_vm_ring_has_compute_vm_bug(ring); > > > unsigned patch_offset = 0; > > > int r; > > >- if (amdgpu_vm_had_gpu_reset(adev, id)) { > > >- gds_switch_needed = true; > > >- vm_flush_needed = true; > > >- } > > >- > > >- if (!vm_flush_needed && !gds_switch_needed) > > >+ if (!job->vm_needs_flush && !gds_switch_needed && > > >+ !amdgpu_vm_had_gpu_reset(adev, id) && > > >+ !amdgpu_vm_ring_has_compute_vm_bug(ring)) > > > return 0; > > > if (ring->funcs->init_cond_exec) > > > patch_offset = amdgpu_ring_init_cond_exec(ring); > > >- if (ring->funcs->emit_vm_flush && vm_flush_needed) { > [flora]: for compute ring & amdgpu_vm_ring_has_compute_vm_bug(), a vm_flush is > inserted. This might cause performance drop. > > >+ if (ring->funcs->emit_vm_flush && > > >+ (job->vm_needs_flush || amdgpu_vm_had_gpu_reset(adev, id))) { > > > struct fence *fence; > > > trace_amdgpu_vm_flush(ring, job->vm_id, job->vm_pd_addr); > > > > > _______________________________________________ > 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] drm/amdgpu: fix ocl test performance drop 2017-05-19 3:36 ` Flora Cui @ 2017-05-19 5:16 ` zhoucm1 [not found] ` <591E7FA9.4070201-5C7GfCeVMHo@public.gmane.org> 2017-05-23 17:51 ` Deucher, Alexander 1 sibling, 1 reply; 14+ messages in thread From: zhoucm1 @ 2017-05-19 5:16 UTC (permalink / raw) To: Flora Cui, Christian K�nig Cc: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW I also realized this problem when I did fix for sched fence, I thought if no issue, just left it as current code, it's more safe. As Flora point it out, it results in performance drop, then we need to correct it back. pipeline_sync in vm flush should be inserted only when amdgpu_vm_ring_has_compute_vm_bug() is true, not all time for every vm flush for all engines. The bug has been fixed in mec firmware from version 673 in gfx8. About gfx9 for amdgpu_vm_ring_has_compute_vm_bug(), current implementation is thinking gfx9 doesn't have compute vm bug by default. Not sure it's in there. If no vm fault related to this bug, then we can think gfx9 has no compute vm bug, I believe firmware team does that based on previous generation, otherwise we need identify every generation, that's crazy. Regards, David Zhou On 2017年05月19日 11:36, Flora Cui wrote: > btw, what's about gfx9 for amdgpu_vm_ring_has_compute_vm_bug()? Is the > workaround still needed? > > On Fri, May 19, 2017 at 10:25:19AM +0800, Flora Cui wrote: >> On Thu, May 18, 2017 at 01:38:15PM +0200, Christian König wrote: >>> Am 18.05.2017 um 09:45 schrieb Flora Cui: >>>> partial revert commit <6971d3d> - drm/amdgpu: cleanup logic in >>>> amdgpu_vm_flush >>>> >>>> Change-Id: Iadce9d613dfe9a739643a74050cea55854832adb >>>> Signed-off-by: Flora Cui <Flora.Cui@amd.com> >>> I don't see how the revert should be faster than the original. >>> >>> Especially that amdgpu_vm_had_gpu_reset() is now called twice sounds like >>> more overhead than necessary. >>> >>> Please explain further. >>> >>> Christian. >>> >>>> --- >>>> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 14 +++++--------- >>>> 1 file changed, 5 insertions(+), 9 deletions(-) >>>> >>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >>>> index 88420dc..a96bad6 100644 >>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >>>> @@ -743,23 +743,19 @@ int amdgpu_vm_flush(struct amdgpu_ring *ring, struct amdgpu_job *job) >>>> id->gws_size != job->gws_size || >>>> id->oa_base != job->oa_base || >>>> id->oa_size != job->oa_size); >>>> - bool vm_flush_needed = job->vm_needs_flush || >>>> - amdgpu_vm_ring_has_compute_vm_bug(ring); >>>> unsigned patch_offset = 0; >>>> int r; >>>> - if (amdgpu_vm_had_gpu_reset(adev, id)) { >>>> - gds_switch_needed = true; >>>> - vm_flush_needed = true; >>>> - } >>>> - >>>> - if (!vm_flush_needed && !gds_switch_needed) >>>> + if (!job->vm_needs_flush && !gds_switch_needed && >>>> + !amdgpu_vm_had_gpu_reset(adev, id) && >>>> + !amdgpu_vm_ring_has_compute_vm_bug(ring)) >>>> return 0; >>>> if (ring->funcs->init_cond_exec) >>>> patch_offset = amdgpu_ring_init_cond_exec(ring); >>>> - if (ring->funcs->emit_vm_flush && vm_flush_needed) { >> [flora]: for compute ring & amdgpu_vm_ring_has_compute_vm_bug(), a vm_flush is >> inserted. This might cause performance drop. >>>> + if (ring->funcs->emit_vm_flush && >>>> + (job->vm_needs_flush || amdgpu_vm_had_gpu_reset(adev, id))) { >>>> struct fence *fence; >>>> trace_amdgpu_vm_flush(ring, job->vm_id, job->vm_pd_addr); >>> >> _______________________________________________ >> 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 _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 14+ messages in thread
[parent not found: <591E7FA9.4070201-5C7GfCeVMHo@public.gmane.org>]
* Re: [PATCH] drm/amdgpu: fix ocl test performance drop [not found] ` <591E7FA9.4070201-5C7GfCeVMHo@public.gmane.org> @ 2017-05-19 11:59 ` Christian König [not found] ` <0246394a-ddbb-6478-b36f-8fb7e9213acb-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org> 0 siblings, 1 reply; 14+ messages in thread From: Christian König @ 2017-05-19 11:59 UTC (permalink / raw) To: zhoucm1, Flora Cui; +Cc: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW I'm pretty sure that bug is fixed in GFX9. So we don't need the check there any more. Regards, Christian. Am 19.05.2017 um 07:16 schrieb zhoucm1: > I also realized this problem when I did fix for sched fence, I thought > if no issue, just left it as current code, it's more safe. > As Flora point it out, it results in performance drop, then we need to > correct it back. > pipeline_sync in vm flush should be inserted only when > amdgpu_vm_ring_has_compute_vm_bug() is true, not all time for every vm > flush for all engines. The bug has been fixed in mec firmware from > version 673 in gfx8. > About gfx9 for amdgpu_vm_ring_has_compute_vm_bug(), current > implementation is thinking gfx9 doesn't have compute vm bug by > default. Not sure it's in there. If no vm fault related to this bug, > then we can think gfx9 has no compute vm bug, I believe firmware team > does that based on previous generation, otherwise we need identify > every generation, that's crazy. > > Regards, > David Zhou > On 2017年05月19日 11:36, Flora Cui wrote: >> btw, what's about gfx9 for amdgpu_vm_ring_has_compute_vm_bug()? Is the >> workaround still needed? >> >> On Fri, May 19, 2017 at 10:25:19AM +0800, Flora Cui wrote: >>> On Thu, May 18, 2017 at 01:38:15PM +0200, Christian König wrote: >>>> Am 18.05.2017 um 09:45 schrieb Flora Cui: >>>>> partial revert commit <6971d3d> - drm/amdgpu: cleanup logic in >>>>> amdgpu_vm_flush >>>>> >>>>> Change-Id: Iadce9d613dfe9a739643a74050cea55854832adb >>>>> Signed-off-by: Flora Cui <Flora.Cui@amd.com> >>>> I don't see how the revert should be faster than the original. >>>> >>>> Especially that amdgpu_vm_had_gpu_reset() is now called twice >>>> sounds like >>>> more overhead than necessary. >>>> >>>> Please explain further. >>>> >>>> Christian. >>>> >>>>> --- >>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 14 +++++--------- >>>>> 1 file changed, 5 insertions(+), 9 deletions(-) >>>>> >>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >>>>> index 88420dc..a96bad6 100644 >>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >>>>> @@ -743,23 +743,19 @@ int amdgpu_vm_flush(struct amdgpu_ring >>>>> *ring, struct amdgpu_job *job) >>>>> id->gws_size != job->gws_size || >>>>> id->oa_base != job->oa_base || >>>>> id->oa_size != job->oa_size); >>>>> - bool vm_flush_needed = job->vm_needs_flush || >>>>> - amdgpu_vm_ring_has_compute_vm_bug(ring); >>>>> unsigned patch_offset = 0; >>>>> int r; >>>>> - if (amdgpu_vm_had_gpu_reset(adev, id)) { >>>>> - gds_switch_needed = true; >>>>> - vm_flush_needed = true; >>>>> - } >>>>> - >>>>> - if (!vm_flush_needed && !gds_switch_needed) >>>>> + if (!job->vm_needs_flush && !gds_switch_needed && >>>>> + !amdgpu_vm_had_gpu_reset(adev, id) && >>>>> + !amdgpu_vm_ring_has_compute_vm_bug(ring)) >>>>> return 0; >>>>> if (ring->funcs->init_cond_exec) >>>>> patch_offset = amdgpu_ring_init_cond_exec(ring); >>>>> - if (ring->funcs->emit_vm_flush && vm_flush_needed) { >>> [flora]: for compute ring & amdgpu_vm_ring_has_compute_vm_bug(), a >>> vm_flush is >>> inserted. This might cause performance drop. >>>>> + if (ring->funcs->emit_vm_flush && >>>>> + (job->vm_needs_flush || amdgpu_vm_had_gpu_reset(adev, >>>>> id))) { >>>>> struct fence *fence; >>>>> trace_amdgpu_vm_flush(ring, job->vm_id, job->vm_pd_addr); >>>> >>> _______________________________________________ >>> 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 > > _______________________________________________ > 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
[parent not found: <0246394a-ddbb-6478-b36f-8fb7e9213acb-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>]
* Re: [PATCH] drm/amdgpu: fix ocl test performance drop [not found] ` <0246394a-ddbb-6478-b36f-8fb7e9213acb-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org> @ 2017-05-22 3:42 ` Flora Cui 2017-05-22 6:47 ` Christian König 0 siblings, 1 reply; 14+ messages in thread From: Flora Cui @ 2017-05-22 3:42 UTC (permalink / raw) To: Christian König; +Cc: zhoucm1, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW Could anyone give me a RB? On Fri, May 19, 2017 at 01:59:16PM +0200, Christian König wrote: > I'm pretty sure that bug is fixed in GFX9. So we don't need the check there > any more. > > Regards, > Christian. > > Am 19.05.2017 um 07:16 schrieb zhoucm1: > >I also realized this problem when I did fix for sched fence, I thought if > >no issue, just left it as current code, it's more safe. > >As Flora point it out, it results in performance drop, then we need to > >correct it back. > >pipeline_sync in vm flush should be inserted only when > >amdgpu_vm_ring_has_compute_vm_bug() is true, not all time for every vm > >flush for all engines. The bug has been fixed in mec firmware from version > >673 in gfx8. > >About gfx9 for amdgpu_vm_ring_has_compute_vm_bug(), current implementation > >is thinking gfx9 doesn't have compute vm bug by default. Not sure it's in > >there. If no vm fault related to this bug, then we can think gfx9 has no > >compute vm bug, I believe firmware team does that based on previous > >generation, otherwise we need identify every generation, that's crazy. > > > >Regards, > >David Zhou > >On 2017年05月19日 11:36, Flora Cui wrote: > >>btw, what's about gfx9 for amdgpu_vm_ring_has_compute_vm_bug()? Is the > >>workaround still needed? > >> > >>On Fri, May 19, 2017 at 10:25:19AM +0800, Flora Cui wrote: > >>>On Thu, May 18, 2017 at 01:38:15PM +0200, Christian König wrote: > >>>>Am 18.05.2017 um 09:45 schrieb Flora Cui: > >>>>>partial revert commit <6971d3d> - drm/amdgpu: cleanup logic in > >>>>>amdgpu_vm_flush > >>>>> > >>>>>Change-Id: Iadce9d613dfe9a739643a74050cea55854832adb > >>>>>Signed-off-by: Flora Cui <Flora.Cui@amd.com> > >>>>I don't see how the revert should be faster than the original. > >>>> > >>>>Especially that amdgpu_vm_had_gpu_reset() is now called twice sounds > >>>>like > >>>>more overhead than necessary. > >>>> > >>>>Please explain further. > >>>> > >>>>Christian. > >>>> > >>>>>--- > >>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 14 +++++--------- > >>>>> 1 file changed, 5 insertions(+), 9 deletions(-) > >>>>> > >>>>>diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > >>>>>b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > >>>>>index 88420dc..a96bad6 100644 > >>>>>--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > >>>>>+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > >>>>>@@ -743,23 +743,19 @@ int amdgpu_vm_flush(struct amdgpu_ring > >>>>>*ring, struct amdgpu_job *job) > >>>>> id->gws_size != job->gws_size || > >>>>> id->oa_base != job->oa_base || > >>>>> id->oa_size != job->oa_size); > >>>>>- bool vm_flush_needed = job->vm_needs_flush || > >>>>>- amdgpu_vm_ring_has_compute_vm_bug(ring); > >>>>> unsigned patch_offset = 0; > >>>>> int r; > >>>>>- if (amdgpu_vm_had_gpu_reset(adev, id)) { > >>>>>- gds_switch_needed = true; > >>>>>- vm_flush_needed = true; > >>>>>- } > >>>>>- > >>>>>- if (!vm_flush_needed && !gds_switch_needed) > >>>>>+ if (!job->vm_needs_flush && !gds_switch_needed && > >>>>>+ !amdgpu_vm_had_gpu_reset(adev, id) && > >>>>>+ !amdgpu_vm_ring_has_compute_vm_bug(ring)) > >>>>> return 0; > >>>>> if (ring->funcs->init_cond_exec) > >>>>> patch_offset = amdgpu_ring_init_cond_exec(ring); > >>>>>- if (ring->funcs->emit_vm_flush && vm_flush_needed) { > >>>[flora]: for compute ring & amdgpu_vm_ring_has_compute_vm_bug(), a > >>>vm_flush is > >>>inserted. This might cause performance drop. > >>>>>+ if (ring->funcs->emit_vm_flush && > >>>>>+ (job->vm_needs_flush || amdgpu_vm_had_gpu_reset(adev, > >>>>>id))) { > >>>>> struct fence *fence; > >>>>> trace_amdgpu_vm_flush(ring, job->vm_id, job->vm_pd_addr); > >>>> > >>>_______________________________________________ > >>>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 > > > >_______________________________________________ > >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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] drm/amdgpu: fix ocl test performance drop 2017-05-22 3:42 ` Flora Cui @ 2017-05-22 6:47 ` Christian König 0 siblings, 0 replies; 14+ messages in thread From: Christian König @ 2017-05-22 6:47 UTC (permalink / raw) To: Flora Cui; +Cc: zhoucm1, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW [-- Attachment #1.1: Type: text/plain, Size: 5047 bytes --] See my other comment about simplifying the patch in the other mail: > [flora]: for compute ring & amdgpu_vm_ring_has_compute_vm_bug(), a > vm_flush is > inserted. This might cause performance drop. > > Ah, I see. We only need the pipeline sync, but not the vm flush. > > In this case I suggest to just change the following line in > amdgpu_vm_flush(): >> - bool vm_flush_needed = job->vm_needs_flush || >> - amdgpu_vm_ring_has_compute_vm_bug(ring); > > We can keep it in amdgpu_vm_need_pipeline_sync(). With that done we can easily push it. Christian. Am 22.05.2017 um 05:42 schrieb Flora Cui: > Could anyone give me a RB? > > On Fri, May 19, 2017 at 01:59:16PM +0200, Christian König wrote: >> I'm pretty sure that bug is fixed in GFX9. So we don't need the check there >> any more. >> >> Regards, >> Christian. >> >> Am 19.05.2017 um 07:16 schrieb zhoucm1: >>> I also realized this problem when I did fix for sched fence, I thought if >>> no issue, just left it as current code, it's more safe. >>> As Flora point it out, it results in performance drop, then we need to >>> correct it back. >>> pipeline_sync in vm flush should be inserted only when >>> amdgpu_vm_ring_has_compute_vm_bug() is true, not all time for every vm >>> flush for all engines. The bug has been fixed in mec firmware from version >>> 673 in gfx8. >>> About gfx9 for amdgpu_vm_ring_has_compute_vm_bug(), current implementation >>> is thinking gfx9 doesn't have compute vm bug by default. Not sure it's in >>> there. If no vm fault related to this bug, then we can think gfx9 has no >>> compute vm bug, I believe firmware team does that based on previous >>> generation, otherwise we need identify every generation, that's crazy. >>> >>> Regards, >>> David Zhou >>> On 2017年05月19日 11:36, Flora Cui wrote: >>>> btw, what's about gfx9 for amdgpu_vm_ring_has_compute_vm_bug()? Is the >>>> workaround still needed? >>>> >>>> On Fri, May 19, 2017 at 10:25:19AM +0800, Flora Cui wrote: >>>>> On Thu, May 18, 2017 at 01:38:15PM +0200, Christian König wrote: >>>>>> Am 18.05.2017 um 09:45 schrieb Flora Cui: >>>>>>> partial revert commit <6971d3d> - drm/amdgpu: cleanup logic in >>>>>>> amdgpu_vm_flush >>>>>>> >>>>>>> Change-Id: Iadce9d613dfe9a739643a74050cea55854832adb >>>>>>> Signed-off-by: Flora Cui <Flora.Cui-5C7GfCeVMHo@public.gmane.org> >>>>>> I don't see how the revert should be faster than the original. >>>>>> >>>>>> Especially that amdgpu_vm_had_gpu_reset() is now called twice sounds >>>>>> like >>>>>> more overhead than necessary. >>>>>> >>>>>> Please explain further. >>>>>> >>>>>> Christian. >>>>>> >>>>>>> --- >>>>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 14 +++++--------- >>>>>>> 1 file changed, 5 insertions(+), 9 deletions(-) >>>>>>> >>>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >>>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >>>>>>> index 88420dc..a96bad6 100644 >>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >>>>>>> @@ -743,23 +743,19 @@ int amdgpu_vm_flush(struct amdgpu_ring >>>>>>> *ring, struct amdgpu_job *job) >>>>>>> id->gws_size != job->gws_size || >>>>>>> id->oa_base != job->oa_base || >>>>>>> id->oa_size != job->oa_size); >>>>>>> - bool vm_flush_needed = job->vm_needs_flush || >>>>>>> - amdgpu_vm_ring_has_compute_vm_bug(ring); >>>>>>> unsigned patch_offset = 0; >>>>>>> int r; >>>>>>> - if (amdgpu_vm_had_gpu_reset(adev, id)) { >>>>>>> - gds_switch_needed = true; >>>>>>> - vm_flush_needed = true; >>>>>>> - } >>>>>>> - >>>>>>> - if (!vm_flush_needed && !gds_switch_needed) >>>>>>> + if (!job->vm_needs_flush && !gds_switch_needed && >>>>>>> + !amdgpu_vm_had_gpu_reset(adev, id) && >>>>>>> + !amdgpu_vm_ring_has_compute_vm_bug(ring)) >>>>>>> return 0; >>>>>>> if (ring->funcs->init_cond_exec) >>>>>>> patch_offset = amdgpu_ring_init_cond_exec(ring); >>>>>>> - if (ring->funcs->emit_vm_flush && vm_flush_needed) { >>>>> [flora]: for compute ring & amdgpu_vm_ring_has_compute_vm_bug(), a >>>>> vm_flush is >>>>> inserted. This might cause performance drop. >>>>>>> + if (ring->funcs->emit_vm_flush && >>>>>>> + (job->vm_needs_flush || amdgpu_vm_had_gpu_reset(adev, >>>>>>> id))) { >>>>>>> struct fence *fence; >>>>>>> trace_amdgpu_vm_flush(ring, job->vm_id, job->vm_pd_addr); >>>>> _______________________________________________ >>>>> amd-gfx mailing list >>>>> amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org >>>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx >>>> _______________________________________________ >>>> amd-gfx mailing list >>>> amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org >>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx >>> _______________________________________________ >>> amd-gfx mailing list >>> amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org >>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx >> [-- Attachment #1.2: Type: text/html, Size: 7074 bytes --] [-- Attachment #2: Type: text/plain, Size: 154 bytes --] _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH] drm/amdgpu: fix ocl test performance drop 2017-05-19 3:36 ` Flora Cui 2017-05-19 5:16 ` zhoucm1 @ 2017-05-23 17:51 ` Deucher, Alexander 1 sibling, 0 replies; 14+ messages in thread From: Deucher, Alexander @ 2017-05-23 17:51 UTC (permalink / raw) To: Cui, Flora, Christian K�nig Cc: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org > -----Original Message----- > From: amd-gfx [mailto:amd-gfx-bounces@lists.freedesktop.org] On Behalf > Of Flora Cui > Sent: Thursday, May 18, 2017 11:37 PM > To: Christian K�nig > Cc: amd-gfx@lists.freedesktop.org > Subject: Re: [PATCH] drm/amdgpu: fix ocl test performance drop > > btw, what's about gfx9 for amdgpu_vm_ring_has_compute_vm_bug()? Is > the > workaround still needed? Gfx9 is not affected by that issue. Alex > > On Fri, May 19, 2017 at 10:25:19AM +0800, Flora Cui wrote: > > On Thu, May 18, 2017 at 01:38:15PM +0200, Christian König wrote: > > > Am 18.05.2017 um 09:45 schrieb Flora Cui: > > > >partial revert commit <6971d3d> - drm/amdgpu: cleanup logic in > > > >amdgpu_vm_flush > > > > > > > >Change-Id: Iadce9d613dfe9a739643a74050cea55854832adb > > > >Signed-off-by: Flora Cui <Flora.Cui@amd.com> > > > > > > I don't see how the revert should be faster than the original. > > > > > > Especially that amdgpu_vm_had_gpu_reset() is now called twice sounds > like > > > more overhead than necessary. > > > > > > Please explain further. > > > > > > Christian. > > > > > > >--- > > > > drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 14 +++++--------- > > > > 1 file changed, 5 insertions(+), 9 deletions(-) > > > > > > > >diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > > > >index 88420dc..a96bad6 100644 > > > >--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > > > >+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > > > >@@ -743,23 +743,19 @@ int amdgpu_vm_flush(struct amdgpu_ring > *ring, struct amdgpu_job *job) > > > > id->gws_size != job->gws_size || > > > > id->oa_base != job->oa_base || > > > > id->oa_size != job->oa_size); > > > >- bool vm_flush_needed = job->vm_needs_flush || > > > >- amdgpu_vm_ring_has_compute_vm_bug(ring); > > > > unsigned patch_offset = 0; > > > > int r; > > > >- if (amdgpu_vm_had_gpu_reset(adev, id)) { > > > >- gds_switch_needed = true; > > > >- vm_flush_needed = true; > > > >- } > > > >- > > > >- if (!vm_flush_needed && !gds_switch_needed) > > > >+ if (!job->vm_needs_flush && !gds_switch_needed && > > > >+ !amdgpu_vm_had_gpu_reset(adev, id) && > > > >+ !amdgpu_vm_ring_has_compute_vm_bug(ring)) > > > > return 0; > > > > if (ring->funcs->init_cond_exec) > > > > patch_offset = amdgpu_ring_init_cond_exec(ring); > > > >- if (ring->funcs->emit_vm_flush && vm_flush_needed) { > > [flora]: for compute ring & amdgpu_vm_ring_has_compute_vm_bug(), a > vm_flush is > > inserted. This might cause performance drop. > > > >+ if (ring->funcs->emit_vm_flush && > > > >+ (job->vm_needs_flush || amdgpu_vm_had_gpu_reset(adev, id))) > { > > > > struct fence *fence; > > > > trace_amdgpu_vm_flush(ring, job->vm_id, job- > >vm_pd_addr); > > > > > > > > _______________________________________________ > > 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 _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] drm/amdgpu: fix ocl test performance drop 2017-05-19 2:25 ` Flora Cui 2017-05-19 3:36 ` Flora Cui @ 2017-05-19 11:57 ` Christian König 1 sibling, 0 replies; 14+ messages in thread From: Christian König @ 2017-05-19 11:57 UTC (permalink / raw) To: Flora Cui; +Cc: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW Am 19.05.2017 um 04:25 schrieb Flora Cui: > On Thu, May 18, 2017 at 01:38:15PM +0200, Christian König wrote: >> Am 18.05.2017 um 09:45 schrieb Flora Cui: >>> partial revert commit <6971d3d> - drm/amdgpu: cleanup logic in >>> amdgpu_vm_flush >>> >>> Change-Id: Iadce9d613dfe9a739643a74050cea55854832adb >>> Signed-off-by: Flora Cui <Flora.Cui@amd.com> >> I don't see how the revert should be faster than the original. >> >> Especially that amdgpu_vm_had_gpu_reset() is now called twice sounds like >> more overhead than necessary. >> >> Please explain further. >> >> Christian. >> >>> --- >>> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 14 +++++--------- >>> 1 file changed, 5 insertions(+), 9 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >>> index 88420dc..a96bad6 100644 >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >>> @@ -743,23 +743,19 @@ int amdgpu_vm_flush(struct amdgpu_ring *ring, struct amdgpu_job *job) >>> id->gws_size != job->gws_size || >>> id->oa_base != job->oa_base || >>> id->oa_size != job->oa_size); >>> - bool vm_flush_needed = job->vm_needs_flush || >>> - amdgpu_vm_ring_has_compute_vm_bug(ring); >>> unsigned patch_offset = 0; >>> int r; >>> - if (amdgpu_vm_had_gpu_reset(adev, id)) { >>> - gds_switch_needed = true; >>> - vm_flush_needed = true; >>> - } >>> - >>> - if (!vm_flush_needed && !gds_switch_needed) >>> + if (!job->vm_needs_flush && !gds_switch_needed && >>> + !amdgpu_vm_had_gpu_reset(adev, id) && >>> + !amdgpu_vm_ring_has_compute_vm_bug(ring)) >>> return 0; >>> if (ring->funcs->init_cond_exec) >>> patch_offset = amdgpu_ring_init_cond_exec(ring); >>> - if (ring->funcs->emit_vm_flush && vm_flush_needed) { > [flora]: for compute ring & amdgpu_vm_ring_has_compute_vm_bug(), a vm_flush is > inserted. This might cause performance drop. Ah, I see. We only need the pipeline sync, but not the vm flush. In this case I suggest to just change the following line in amdgpu_vm_flush(): > - bool vm_flush_needed = job->vm_needs_flush || > - amdgpu_vm_ring_has_compute_vm_bug(ring); We can keep it in amdgpu_vm_need_pipeline_sync(). BTW: We should cache the result of amdgpu_vm_ring_has_compute_vm_bug() in the vm manager structure. Computing this on the fly for every command submissions is just a huge bunch of overhead. Regards, Christian. _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH] drm/amdgpu: fix ocl test performance drop
@ 2017-05-22 8:34 Flora Cui
[not found] ` <1495442049-7481-1-git-send-email-Flora.Cui-5C7GfCeVMHo@public.gmane.org>
0 siblings, 1 reply; 14+ messages in thread
From: Flora Cui @ 2017-05-22 8:34 UTC (permalink / raw)
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW; +Cc: Flora Cui
partial revert commit <6971d3d> - drm/amdgpu: cleanup logic in
amdgpu_vm_flush
Change-Id: Iadce9d613dfe9a739643a74050cea55854832adb
Signed-off-by: Flora Cui <Flora.Cui@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 4573d8c..11d58f7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -743,8 +743,7 @@ int amdgpu_vm_flush(struct amdgpu_ring *ring, struct amdgpu_job *job)
id->gws_size != job->gws_size ||
id->oa_base != job->oa_base ||
id->oa_size != job->oa_size);
- bool vm_flush_needed = job->vm_needs_flush ||
- amdgpu_vm_ring_has_compute_vm_bug(ring);
+ bool vm_flush_needed = job->vm_needs_flush;
unsigned patch_offset = 0;
int r;
@@ -753,7 +752,8 @@ int amdgpu_vm_flush(struct amdgpu_ring *ring, struct amdgpu_job *job)
vm_flush_needed = true;
}
- if (!vm_flush_needed && !gds_switch_needed)
+ if (!vm_flush_needed && !gds_switch_needed &&
+ !amdgpu_vm_ring_has_compute_vm_bug(ring))
return 0;
if (ring->funcs->init_cond_exec)
--
2.7.4
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 14+ messages in thread[parent not found: <1495442049-7481-1-git-send-email-Flora.Cui-5C7GfCeVMHo@public.gmane.org>]
* Re: [PATCH] drm/amdgpu: fix ocl test performance drop [not found] ` <1495442049-7481-1-git-send-email-Flora.Cui-5C7GfCeVMHo@public.gmane.org> @ 2017-05-22 8:58 ` zhoucm1 [not found] ` <5922A818.9010301-5C7GfCeVMHo@public.gmane.org> 0 siblings, 1 reply; 14+ messages in thread From: zhoucm1 @ 2017-05-22 8:58 UTC (permalink / raw) To: Flora Cui, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW On 2017年05月22日 16:34, Flora Cui wrote: > partial revert commit <6971d3d> - drm/amdgpu: cleanup logic in > amdgpu_vm_flush > > Change-Id: Iadce9d613dfe9a739643a74050cea55854832adb > Signed-off-by: Flora Cui <Flora.Cui@amd.com> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > index 4573d8c..11d58f7 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > @@ -743,8 +743,7 @@ int amdgpu_vm_flush(struct amdgpu_ring *ring, struct amdgpu_job *job) > id->gws_size != job->gws_size || > id->oa_base != job->oa_base || > id->oa_size != job->oa_size); > - bool vm_flush_needed = job->vm_needs_flush || > - amdgpu_vm_ring_has_compute_vm_bug(ring); > + bool vm_flush_needed = job->vm_needs_flush; > unsigned patch_offset = 0; > int r; > > @@ -753,7 +752,8 @@ int amdgpu_vm_flush(struct amdgpu_ring *ring, struct amdgpu_job *job) > vm_flush_needed = true; > } > > - if (!vm_flush_needed && !gds_switch_needed) > + if (!vm_flush_needed && !gds_switch_needed && > + !amdgpu_vm_ring_has_compute_vm_bug(ring)) > return 0; here change isn't needed. Regards, David Zhou > > if (ring->funcs->init_cond_exec) _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 14+ messages in thread
[parent not found: <5922A818.9010301-5C7GfCeVMHo@public.gmane.org>]
* Re: [PATCH] drm/amdgpu: fix ocl test performance drop [not found] ` <5922A818.9010301-5C7GfCeVMHo@public.gmane.org> @ 2017-05-22 10:10 ` Flora Cui 2017-05-22 11:16 ` Christian König 0 siblings, 1 reply; 14+ messages in thread From: Flora Cui @ 2017-05-22 10:10 UTC (permalink / raw) To: zhoucm1; +Cc: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW partial revert commit <6971d3d> - drm/amdgpu: cleanup logic in amdgpu_vm_flush Change-Id: Iadce9d613dfe9a739643a74050cea55854832adb Signed-off-by: Flora Cui <Flora.Cui@amd.com> --- drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c index 4573d8c..5b9a73d 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c @@ -743,8 +743,7 @@ int amdgpu_vm_flush(struct amdgpu_ring *ring, struct amdgpu_job *job) id->gws_size != job->gws_size || id->oa_base != job->oa_base || id->oa_size != job->oa_size); - bool vm_flush_needed = job->vm_needs_flush || - amdgpu_vm_ring_has_compute_vm_bug(ring); + bool vm_flush_needed = job->vm_needs_flush; unsigned patch_offset = 0; int r; -- 2.7.4 On Mon, May 22, 2017 at 04:58:00PM +0800, zhoucm1 wrote: > > > On 2017年05月22日 16:34, Flora Cui wrote: > >partial revert commit <6971d3d> - drm/amdgpu: cleanup logic in > >amdgpu_vm_flush > > > >Change-Id: Iadce9d613dfe9a739643a74050cea55854832adb > >Signed-off-by: Flora Cui <Flora.Cui@amd.com> > >--- > > drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > >diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > >index 4573d8c..11d58f7 100644 > >--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > >+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > >@@ -743,8 +743,7 @@ int amdgpu_vm_flush(struct amdgpu_ring *ring, struct amdgpu_job *job) > > id->gws_size != job->gws_size || > > id->oa_base != job->oa_base || > > id->oa_size != job->oa_size); > >- bool vm_flush_needed = job->vm_needs_flush || > >- amdgpu_vm_ring_has_compute_vm_bug(ring); > >+ bool vm_flush_needed = job->vm_needs_flush; > > unsigned patch_offset = 0; > > int r; > >@@ -753,7 +752,8 @@ int amdgpu_vm_flush(struct amdgpu_ring *ring, struct amdgpu_job *job) > > vm_flush_needed = true; > > } > >- if (!vm_flush_needed && !gds_switch_needed) > >+ if (!vm_flush_needed && !gds_switch_needed && > >+ !amdgpu_vm_ring_has_compute_vm_bug(ring)) > > return 0; > here change isn't needed. > > Regards, > David Zhou > > if (ring->funcs->init_cond_exec) > _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] drm/amdgpu: fix ocl test performance drop 2017-05-22 10:10 ` Flora Cui @ 2017-05-22 11:16 ` Christian König 0 siblings, 0 replies; 14+ messages in thread From: Christian König @ 2017-05-22 11:16 UTC (permalink / raw) To: Flora Cui, zhoucm1; +Cc: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW Am 22.05.2017 um 12:10 schrieb Flora Cui: > partial revert commit <6971d3d> - drm/amdgpu: cleanup logic in > amdgpu_vm_flush > > Change-Id: Iadce9d613dfe9a739643a74050cea55854832adb > Signed-off-by: Flora Cui <Flora.Cui@amd.com> Reviewed-by: Christian König <christian.koenig@amd.com> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > index 4573d8c..5b9a73d 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > @@ -743,8 +743,7 @@ int amdgpu_vm_flush(struct amdgpu_ring *ring, struct amdgpu_job *job) > id->gws_size != job->gws_size || > id->oa_base != job->oa_base || > id->oa_size != job->oa_size); > - bool vm_flush_needed = job->vm_needs_flush || > - amdgpu_vm_ring_has_compute_vm_bug(ring); > + bool vm_flush_needed = job->vm_needs_flush; > unsigned patch_offset = 0; > int r; > _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2017-05-23 17:51 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-05-18 7:45 [PATCH] drm/amdgpu: fix ocl test performance drop Flora Cui
[not found] ` <1495093501-13313-1-git-send-email-Flora.Cui-5C7GfCeVMHo@public.gmane.org>
2017-05-18 11:38 ` Christian König
[not found] ` <4b2d410d-544f-4e72-11ae-8db4a1d797d3-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
2017-05-19 2:25 ` Flora Cui
2017-05-19 3:36 ` Flora Cui
2017-05-19 5:16 ` zhoucm1
[not found] ` <591E7FA9.4070201-5C7GfCeVMHo@public.gmane.org>
2017-05-19 11:59 ` Christian König
[not found] ` <0246394a-ddbb-6478-b36f-8fb7e9213acb-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>
2017-05-22 3:42 ` Flora Cui
2017-05-22 6:47 ` Christian König
2017-05-23 17:51 ` Deucher, Alexander
2017-05-19 11:57 ` Christian König
-- strict thread matches above, loose matches on Subject: below --
2017-05-22 8:34 Flora Cui
[not found] ` <1495442049-7481-1-git-send-email-Flora.Cui-5C7GfCeVMHo@public.gmane.org>
2017-05-22 8:58 ` zhoucm1
[not found] ` <5922A818.9010301-5C7GfCeVMHo@public.gmane.org>
2017-05-22 10:10 ` Flora Cui
2017-05-22 11:16 ` Christian König
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.