Re: [PATCH 1/3] drm/amdgpu: fix IB test MCBP bug

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Am 02.03.20 um 10:39 schrieb Liu, Monk:
-		if (!(ib->flags & AMDGPU_IB_FLAG_CE))
+		if (!(ib->flags & AMDGPU_IB_FLAG_CE) && vmid)
Kernel copies also don't use a VMID, so I think that this won't work correctly.

Do you have the job available as well? That would probably be better to test for since only IB tests don't use a job.

[ML]
The @job is not passed to the gfx_v10_0_ring_emit_ib_gfx() routine so I tend to modify as less as possible
Besides, for kernel copies: I believe they also do not support MCBP (can you point me the code in source of those kernel copies ?) , so use VMID to test is fine (To support MCBP on gfx ring you need a set of additional preamble IB co work with your workload iB and kernel copies apparently don't have those stuffs)

Ah, sorry! My incorrect thinking, kernel copies also don't use the CP but rather the SDMA. So the change is completely irrelevant for the copies.

Acked-by: Christian König <christian.koenig@xxxxxxx> for the patch.

Regards,
Christian.


Thanks
_____________________________________
Monk Liu|GPU Virtualization Team |AMD


-----Original Message-----
From: Christian König <ckoenig.leichtzumerken@xxxxxxxxx>
Sent: Monday, March 2, 2020 5:35 PM
To: Liu, Monk <Monk.Liu@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx
Subject: Re: [PATCH 1/3] drm/amdgpu: fix IB test MCBP bug

Am 02.03.20 um 10:22 schrieb Monk Liu:
1)for gfx IB test we shouldn't insert DE meta data

2)we should make sure IB test finished before we send event 3 to
hypervisor otherwise the IDLE from event 3 will preempt IB test, which
is not designed as a compatible structure for MCBP

Signed-off-by: Monk Liu <Monk.Liu@xxxxxxx>
---
   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 6 ++++++
   drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c    | 3 ---
   drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c     | 2 +-
   drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c      | 2 +-
   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c      | 2 +-
   5 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 351096a..572eb6e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -3195,6 +3195,12 @@ void amdgpu_device_fini(struct amdgpu_device *adev)
   	flush_delayed_work(&adev->delayed_init_work);
   	adev->shutdown = true;
+ /* make sure IB test finished before entering exclusive mode
+	 * to avoid preemption on IB test
+	 * */
+	if (amdgpu_sriov_vf(adev))
+		amdgpu_virt_request_full_gpu(adev, false);
+
   	/* disable all interrupts */
   	amdgpu_irq_disable_all(adev);
   	if (adev->mode_info.mode_config_initialized){
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index 0f35639..0b1511a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -88,9 +88,6 @@ void amdgpu_driver_unload_kms(struct drm_device *dev)
   	if (adev->rmmio == NULL)
   		goto done_free;
- if (amdgpu_sriov_vf(adev))
-		amdgpu_virt_request_full_gpu(adev, false);
-
   	if (adev->runpm) {
   		pm_runtime_get_sync(dev->dev);
   		pm_runtime_forbid(dev->dev);
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
index 94ca9ff..0555989 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
@@ -4432,7 +4432,7 @@ static void gfx_v10_0_ring_emit_ib_gfx(struct amdgpu_ring *ring,
   		if (flags & AMDGPU_IB_PREEMPTED)
   			control |= INDIRECT_BUFFER_PRE_RESUME(1);
- if (!(ib->flags & AMDGPU_IB_FLAG_CE))
+		if (!(ib->flags & AMDGPU_IB_FLAG_CE) && vmid)
   			gfx_v10_0_ring_emit_de_meta(ring,
   				    (!amdgpu_sriov_vf(ring->adev) && flags & AMDGPU_IB_PREEMPTED) ? true : false);
   	}
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
index 393a132..b14f46a3 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
@@ -6116,7 +6116,7 @@ static void gfx_v8_0_ring_emit_ib_gfx(struct amdgpu_ring *ring,
   	if (amdgpu_sriov_vf(ring->adev) && (ib->flags & AMDGPU_IB_FLAG_PREEMPT)) {
   		control |= INDIRECT_BUFFER_PRE_ENB(1);
- if (!(ib->flags & AMDGPU_IB_FLAG_CE))
+		if (!(ib->flags & AMDGPU_IB_FLAG_CE) && vmid)
Kernel copies also don't use a VMID, so I think that this won't work correctly.

Do you have the job available as well? That would probably be better to test for since only IB tests don't use a job.

Christian.

   			gfx_v8_0_ring_emit_de_meta(ring);
   	}
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
index 0156479..d8d256e6 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
@@ -4985,7 +4985,7 @@ static void gfx_v9_0_ring_emit_ib_gfx(struct amdgpu_ring *ring,
   	if (amdgpu_sriov_vf(ring->adev) && (ib->flags & AMDGPU_IB_FLAG_PREEMPT)) {
   		control |= INDIRECT_BUFFER_PRE_ENB(1);
- if (!(ib->flags & AMDGPU_IB_FLAG_CE))
+		if (!(ib->flags & AMDGPU_IB_FLAG_CE) && vmid)
   			gfx_v9_0_ring_emit_de_meta(ring);
   	}

_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux