Yes and exactly that is the total mess I was talking about :)

The backend should be given clear orders which it act on. In other words "load_global_*", "load_ce_ram" etc...

The middle layer in amdgpu_ib_schedule()  then decides based of the flags if the what orders the backend should execute.

If you want to do this right I would suggest that you clean it up thoughtfully.

If you want to just get it working for now I suggest to adjust the skip_preamble variable in amdgpu_ib_schedule() and make sure that we never skip a preamble when gfxoff is enabled.

Regards,
Christian.

Am 17.05.21 um 09:20 schrieb Chen, Jiansong (Simon):
[AMD Official Use Only]

Does't the below code in gfx_v8_ring_emit_cntxcntl do almost the same thing as 
dropping the preamble ib. I cannot understand why bother to duplicate the 
optimization and cause a mess
In the common code.
                 /* set load_ce_ram if preamble presented */
                 if (AMDGPU_PREAMBLE_IB_PRESENT & flags)
                         dw2 |= 0x10000000;
         } else {
                 /* still load_ce_ram if this is the first time preamble 
presented
                  * although there is no context switch happens.
                  */
                 if (AMDGPU_PREAMBLE_IB_PRESENT_FIRST & flags)
                         dw2 |= 0x10000000;
         }

-----Original Message-----
From: Christian König <[email protected]>
Sent: Monday, May 17, 2021 2:56 PM
To: Chen, Jiansong (Simon) <[email protected]>; Koenig, Christian 
<[email protected]>; [email protected]
Subject: Re: [PATCH] drm/amdgpu: optimize to drop preamble IB for old GPUs

Am 17.05.21 um 08:51 schrieb Chen, Jiansong (Simon):
[AMD Official Use Only]

Doesn't  current solution always enable the optimization in a safe and more 
clear way?
No, we also need this for gfx8 where gfxoff is currently not implemented.

Additional to that we mix common frontend handling into the backend with this 
approach.

But you could clean up the code in amdgpu_ib_schedule() quite a bit.

Regards,
Christian.

1. for gfx8/9/10 we use load_ce_ram in context_control to control the 
optimization.
2. for gfx6/7, we directly drop the preamble ib.

Regards,
Jiansong
-----Original Message-----
From: Koenig, Christian <[email protected]>
Sent: Monday, May 17, 2021 2:42 PM
To: Chen, Jiansong (Simon) <[email protected]>;
[email protected]
Subject: Re: [PATCH] drm/amdgpu: optimize to drop preamble IB for old
GPUs

Well NAK, as discussed checking the global flag is more flexible since it will 
still enable the preamble drop when gfxoff is disabled.

Christian.

Am 17.05.21 um 06:39 schrieb Jiansong Chen:
The optimization is safe for old GPUs and can help performance.

Signed-off-by: Jiansong Chen <[email protected]>
Change-Id: Id3b1250f1fe46dddbe8498894fb97e9753b7cafe
---
    drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c | 6 ++++++
    drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c | 6 ++++++
    2 files changed, 12 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
index 3a8d52a54873..c915cc439484 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
@@ -1873,6 +1873,12 @@ static void gfx_v6_0_ring_emit_ib(struct amdgpu_ring 
*ring,
                amdgpu_ring_write(ring, 0);
        }

+     /* drop the CE preamble IB for the same context */
+     if ((ib->flags & AMDGPU_IB_FLAG_PREAMBLE) &&
+         !(flags & AMDGPU_HAVE_CTX_SWITCH) &&
+         !(flags & AMDGPU_PREAMBLE_IB_PRESENT_FIRST))
+             return;
+
        if (ib->flags & AMDGPU_IB_FLAG_CE)
                header = PACKET3(PACKET3_INDIRECT_BUFFER_CONST, 2);
        else
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
index c35fdd2ef2d4..6d9ccae48024 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
@@ -2269,6 +2269,12 @@ static void gfx_v7_0_ring_emit_ib_gfx(struct amdgpu_ring 
*ring,
                amdgpu_ring_write(ring, 0);
        }

+     /* drop the CE preamble IB for the same context */
+     if ((ib->flags & AMDGPU_IB_FLAG_PREAMBLE) &&
+         !(flags & AMDGPU_HAVE_CTX_SWITCH) &&
+         !(flags & AMDGPU_PREAMBLE_IB_PRESENT_FIRST))
+             return;
+
        if (ib->flags & AMDGPU_IB_FLAG_CE)
                header = PACKET3(PACKET3_INDIRECT_BUFFER_CONST, 2);
        else
_______________________________________________
amd-gfx mailing list
[email protected]
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flist
s.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=04%7C01%7CJi
ansong.Chen%40amd.com%7Cf80f7d9888f4427c2b1408d91900e335%7C3dd8961fe48
84e608e11a82d994e183d%7C0%7C0%7C637568313869095131%7CUnknown%7CTWFpbGZ
sb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3
D%7C1000&amp;sdata=MF1%2BhakHpB8N9B8JwXCA9yB1hIy4CNNMok6ASz3AOU0%3D&am
p;reserved=0

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Reply via email to