Hi Hawking
The problem is amdgpu_mcbp only controls OS preemption feature, and OS
preemption cannot enabled together with world switch MCBP
That's why we still need to differentiate them separately
Besides, there are some logic handling different between them, you can check
the details in my patch:
@@ -4522,9 +4522,9 @@ static void gfx_v10_0_ring_emit_cntxcntl(struct
amdgpu_ring *ring,
{
uint32_t dw2 = 0;
- if (amdgpu_mcbp)
+ if (amdgpu_mcbp || amdgpu_sriov_vf(ring->adev))
gfx_v10_0_ring_emit_ce_meta(ring,
- flags & AMDGPU_IB_PREEMPTED ? true : false);
+ (!amdgpu_sriov_vf(ring->adev) && flags &
AMDGPU_IB_PREEMPTED) ? true : false);
@@ -4369,7 +4369,7 @@ static void gfx_v10_0_ring_emit_ib_gfx(struct amdgpu_ring
*ring,
if (!(ib->flags & AMDGPU_IB_FLAG_CE))
gfx_v10_0_ring_emit_de_meta(ring,
- flags & AMDGPU_IB_PREEMPTED ? true : false);
+ (!amdgpu_sriov_vf(ring->adev) && flags &
AMDGPU_IB_PREEMPTED) ? true : false);
- if (vmid == 0 || !amdgpu_mcbp)
+ /* don't enable OS preemption on SDMA under SRIOV */
+ if (amdgpu_sriov_vf(adev) || vmid == 0 || !amdgpu_mcbp)
return 0;
see that above three part need different handling for OS preemption and world
switch preemption
BTW: the OS preemption implement in our driver is actually not too perfect to
me, e.g.: the CSA buffer is static (which is what SRIOV want), and static CSA
buffer cannot handle double preemption case sequence like :
MCBP in IB-A --> run IB-B --> and MCBP in IB-B, since we are using static CSA,
so the MCBP on IB-B will overwrite CSA content which is left by MCBP in IB-A,
so IB-A resume will not work. But it is not my interest currently so I didn’t
touch OS preemption path
thanks !
-----邮件原件-----
发件人: Zhang, Hawking <[email protected]>
发送时间: 2020年2月18日 19:32
收件人: Liu, Monk <[email protected]>; [email protected]
抄送: Liu, Monk <[email protected]>
主题: RE: [PATCH 3/3] drm/amdgpu: fix colliding of preemption
[AMD Official Use Only - Internal Distribution Only]
It's some kind of annoying to check vf in every place that is required for mcbp
until amdgpu_mcbp is enabled by default. What's more, when amdgpu_mcbp is
enabled by default, there will be many unnecessary vf check that can be removed
as most of mcbp function actually can be shared between world switch preemption
and os preemption.
I'd prefer to enable amdgpu_mcbp for sriov in amdgpu_device_check_arguments to
reduce the vf specific check everywhere.
Regards,
Hawking
-----Original Message-----
From: amd-gfx <[email protected]> On Behalf Of Monk Liu
Sent: Tuesday, February 18, 2020 10:54
To: [email protected]
Cc: Liu, Monk <[email protected]>
Subject: [PATCH 3/3] drm/amdgpu: fix colliding of preemption
what:
some os preemption path is messed up with world switch preemption
fix:
cleanup those logics so os preemption not mixed with world switch
this patch is a general fix for issues comes from SRIOV MCBP, but there is
still UMD side issues not resovlved yet, so this patch cannot fix all world
switch bug.
Signed-off-by: Monk Liu <[email protected]>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c | 3 ++-
drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 8 ++++----
2 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
index a2ee30b..7854c05 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
@@ -70,7 +70,8 @@ uint64_t amdgpu_sdma_get_csa_mc_addr(struct amdgpu_ring *ring,
uint32_t index = 0;
int r;
- if (vmid == 0 || !amdgpu_mcbp)
+ /* don't enable OS preemption on SDMA under SRIOV */
+ if (amdgpu_sriov_vf(adev) || vmid == 0 || !amdgpu_mcbp)
return 0;
r = amdgpu_sdma_get_index_from_ring(ring, &index); diff --git
a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
index 5e9fb09..7b61583 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
@@ -4413,7 +4413,7 @@ static void gfx_v10_0_ring_emit_ib_gfx(struct amdgpu_ring
*ring,
control |= ib->length_dw | (vmid << 24);
- if (amdgpu_mcbp && (ib->flags & AMDGPU_IB_FLAG_PREEMPT)) {
+ if ((amdgpu_sriov_vf(ring->adev) || amdgpu_mcbp) && (ib->flags &
+AMDGPU_IB_FLAG_PREEMPT)) {
control |= INDIRECT_BUFFER_PRE_ENB(1);
if (flags & AMDGPU_IB_PREEMPTED)
@@ -4421,7 +4421,7 @@ static void gfx_v10_0_ring_emit_ib_gfx(struct amdgpu_ring
*ring,
if (!(ib->flags & AMDGPU_IB_FLAG_CE))
gfx_v10_0_ring_emit_de_meta(ring,
- flags & AMDGPU_IB_PREEMPTED ? true : false);
+ (!amdgpu_sriov_vf(ring->adev) && flags &
AMDGPU_IB_PREEMPTED) ?
+true : false);
}
amdgpu_ring_write(ring, header);
@@ -4569,9 +4569,9 @@ static void gfx_v10_0_ring_emit_cntxcntl(struct
amdgpu_ring *ring, {
uint32_t dw2 = 0;
- if (amdgpu_mcbp)
+ if (amdgpu_mcbp || amdgpu_sriov_vf(ring->adev))
gfx_v10_0_ring_emit_ce_meta(ring,
- flags & AMDGPU_IB_PREEMPTED ? true : false);
+ (!amdgpu_sriov_vf(ring->adev) && flags &
AMDGPU_IB_PREEMPTED) ?
+true : false);
dw2 |= 0x80000000; /* set load_enable otherwise this package is just
NOPs */
if (flags & AMDGPU_HAVE_CTX_SWITCH) {
--
2.7.4
_______________________________________________
amd-gfx mailing list
[email protected]
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=02%7C01%7Chawking.zhang%40amd.com%7C903db505082546d708fb08d7b41ddf24%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637175912709424350&sdata=7ZqQFR%2BqKEbWnJrdNvvdJgVuoL1RIWUAm8KoYAl490g%3D&reserved=0
_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
