RE: [PATCH 1/2] drm/amdgpu: avoid using invalidate semaphore for SRIOV
[AMD Official Use Only - Internal Distribution Only]
OK Chris.
I'll try and test it.
BR,
Changfeng.
-Original Message-
From: Christian König
Sent: Tuesday, December 3, 2019 8:18 PM
To: Zhu, Changfeng ; amd-gfx@lists.freedesktop.org;
Koenig, Christian ; Huang, Ray ;
Huang, Shimmer ; Deucher, Alexander
Subject: Re: [PATCH 1/2] drm/amdgpu: avoid using invalidate semaphore for SRIOV
Am 03.12.19 um 09:50 schrieb Changfeng.Zhu:
> From: changzhu
>
> It may fail to load guest driver in round 2 when using invalidate
> semaphore for SRIOV. So it needs to avoid using invalidate semaphore
> for SRIOV.
That sounds like the registers are just not correctly initialized when the
driver is reloaded.
I would just add that to mmhub_*_program_invalidation(). Something like this
should already do it:
> for (i = 0; i < 18; ++i) {
> WREG32_SOC15_OFFSET(MMHUB, 0,
> mmVM_INVALIDATE_ENG0_ADDR_RANGE_LO32,
> 2 * i, 0x);
> WREG32_SOC15_OFFSET(MMHUB, 0,
> mmVM_INVALIDATE_ENG0_ADDR_RANGE_HI32,
> 2 * i, 0x1f);
WREG32_SOC15_OFFSET(MMHUB, 0, mmVM_INVALIDATE_ENG0_SEM, i, 0x0);
> }
Regards,
Christian.
>
> Change-Id: I8db1dc6f990fd0c458953571936467551cd4102d
> Signed-off-by: changzhu
> ---
> drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c | 21 +
> drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 20
> 2 files changed, 25 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
> b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
> index 381bb709f021..d4c7d0319650 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
> @@ -243,8 +243,9 @@ static void gmc_v10_0_flush_vm_hub(struct amdgpu_device
> *adev, uint32_t vmid,
>*/
>
> /* TODO: It needs to continue working on debugging with semaphore for
> GFXHUB as well. */
> - if (vmhub == AMDGPU_MMHUB_0 ||
> - vmhub == AMDGPU_MMHUB_1) {
> + if ((vmhub == AMDGPU_MMHUB_0 ||
> + vmhub == AMDGPU_MMHUB_1) &&
> + (!amdgpu_sriov_vf(adev))) {
> for (i = 0; i < adev->usec_timeout; i++) {
> /* a read return value of 1 means semaphore acuqire */
> tmp = RREG32_NO_KIQ(hub->vm_inv_eng0_sem + eng); @@
> -277,8 +278,9
> @@ static void gmc_v10_0_flush_vm_hub(struct amdgpu_device *adev, uint32_t
> vmid,
> }
>
> /* TODO: It needs to continue working on debugging with semaphore for
> GFXHUB as well. */
> - if (vmhub == AMDGPU_MMHUB_0 ||
> - vmhub == AMDGPU_MMHUB_1)
> + if ((vmhub == AMDGPU_MMHUB_0 ||
> + vmhub == AMDGPU_MMHUB_1) &&
> + (!amdgpu_sriov_vf(adev)))
> /*
>* add semaphore release after invalidation,
>* write with 0 means semaphore release @@ -369,6 +371,7 @@
> static
> void gmc_v10_0_flush_gpu_tlb(struct amdgpu_device *adev, uint32_t vmid,
> static uint64_t gmc_v10_0_emit_flush_gpu_tlb(struct amdgpu_ring *ring,
>unsigned vmid, uint64_t pd_addr)
> {
> + struct amdgpu_device *adev = ring->adev;
> struct amdgpu_vmhub *hub = >adev->vmhub[ring->funcs->vmhub];
> uint32_t req = gmc_v10_0_get_invalidate_req(vmid, 0);
> unsigned eng = ring->vm_inv_eng;
> @@ -381,8 +384,9 @@ static uint64_t gmc_v10_0_emit_flush_gpu_tlb(struct
> amdgpu_ring *ring,
>*/
>
> /* TODO: It needs to continue working on debugging with semaphore for
> GFXHUB as well. */
> - if (ring->funcs->vmhub == AMDGPU_MMHUB_0 ||
> - ring->funcs->vmhub == AMDGPU_MMHUB_1)
> + if ((ring->funcs->vmhub == AMDGPU_MMHUB_0 ||
> + ring->funcs->vmhub == AMDGPU_MMHUB_1) &&
> + (!amdgpu_sriov_vf(adev)))
> /* a read return value of 1 means semaphore acuqire */
> amdgpu_ring_emit_reg_wait(ring,
> hub->vm_inv_eng0_sem + eng, 0x1,
> 0x1); @@ -398,8 +402,9 @@
> static uint64_t gmc_v10_0_emit_flush_gpu_tlb(struct amdgpu_ring *ring,
> req, 1 << vmid);
>
> /* TODO: It needs to continue working on debugging with semaphore for
> GFXHUB as well. */
> - if (ring->funcs->vmhub == AMDGPU_MMHUB_0 ||
> - ring->funcs->vmhub == AMDGPU_MMHUB_1)
> + if ((ring->funcs->vmhub == AMDGPU_MMHUB_0 ||
> + ring->funcs->vmhub == AMDGPU_MMHUB_1) &&
> + (!amdgpu_sriov_vf(adev)))
>
Re: [PATCH 1/2] drm/amdgpu: avoid using invalidate semaphore for SRIOV
Am 03.12.19 um 09:50 schrieb Changfeng.Zhu:
From: changzhu
It may fail to load guest driver in round 2 when using invalidate
semaphore for SRIOV. So it needs to avoid using invalidate semaphore for
SRIOV.
That sounds like the registers are just not correctly initialized when
the driver is reloaded.
I would just add that to mmhub_*_program_invalidation(). Something like
this should already do it:
for (i = 0; i < 18; ++i) {
WREG32_SOC15_OFFSET(MMHUB, 0,
mmVM_INVALIDATE_ENG0_ADDR_RANGE_LO32,
2 * i, 0x);
WREG32_SOC15_OFFSET(MMHUB, 0,
mmVM_INVALIDATE_ENG0_ADDR_RANGE_HI32,
2 * i, 0x1f);
WREG32_SOC15_OFFSET(MMHUB, 0, mmVM_INVALIDATE_ENG0_SEM, i, 0x0);
}
Regards,
Christian.
Change-Id: I8db1dc6f990fd0c458953571936467551cd4102d
Signed-off-by: changzhu
---
drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c | 21 +
drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 20
2 files changed, 25 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
index 381bb709f021..d4c7d0319650 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
@@ -243,8 +243,9 @@ static void gmc_v10_0_flush_vm_hub(struct amdgpu_device
*adev, uint32_t vmid,
*/
/* TODO: It needs to continue working on debugging with semaphore for GFXHUB as well. */
- if (vmhub == AMDGPU_MMHUB_0 ||
- vmhub == AMDGPU_MMHUB_1) {
+ if ((vmhub == AMDGPU_MMHUB_0 ||
+vmhub == AMDGPU_MMHUB_1) &&
+ (!amdgpu_sriov_vf(adev))) {
for (i = 0; i < adev->usec_timeout; i++) {
/* a read return value of 1 means semaphore acuqire */
tmp = RREG32_NO_KIQ(hub->vm_inv_eng0_sem + eng);
@@ -277,8 +278,9 @@ static void gmc_v10_0_flush_vm_hub(struct amdgpu_device
*adev, uint32_t vmid,
}
/* TODO: It needs to continue working on debugging with semaphore for GFXHUB as well. */
- if (vmhub == AMDGPU_MMHUB_0 ||
- vmhub == AMDGPU_MMHUB_1)
+ if ((vmhub == AMDGPU_MMHUB_0 ||
+vmhub == AMDGPU_MMHUB_1) &&
+ (!amdgpu_sriov_vf(adev)))
/*
* add semaphore release after invalidation,
* write with 0 means semaphore release
@@ -369,6 +371,7 @@ static void gmc_v10_0_flush_gpu_tlb(struct amdgpu_device
*adev, uint32_t vmid,
static uint64_t gmc_v10_0_emit_flush_gpu_tlb(struct amdgpu_ring *ring,
unsigned vmid, uint64_t pd_addr)
{
+ struct amdgpu_device *adev = ring->adev;
struct amdgpu_vmhub *hub = >adev->vmhub[ring->funcs->vmhub];
uint32_t req = gmc_v10_0_get_invalidate_req(vmid, 0);
unsigned eng = ring->vm_inv_eng;
@@ -381,8 +384,9 @@ static uint64_t gmc_v10_0_emit_flush_gpu_tlb(struct
amdgpu_ring *ring,
*/
/* TODO: It needs to continue working on debugging with semaphore for GFXHUB as well. */
- if (ring->funcs->vmhub == AMDGPU_MMHUB_0 ||
- ring->funcs->vmhub == AMDGPU_MMHUB_1)
+ if ((ring->funcs->vmhub == AMDGPU_MMHUB_0 ||
+ring->funcs->vmhub == AMDGPU_MMHUB_1) &&
+ (!amdgpu_sriov_vf(adev)))
/* a read return value of 1 means semaphore acuqire */
amdgpu_ring_emit_reg_wait(ring,
hub->vm_inv_eng0_sem + eng, 0x1, 0x1);
@@ -398,8 +402,9 @@ static uint64_t gmc_v10_0_emit_flush_gpu_tlb(struct
amdgpu_ring *ring,
req, 1 << vmid);
/* TODO: It needs to continue working on debugging with semaphore for GFXHUB as well. */
- if (ring->funcs->vmhub == AMDGPU_MMHUB_0 ||
- ring->funcs->vmhub == AMDGPU_MMHUB_1)
+ if ((ring->funcs->vmhub == AMDGPU_MMHUB_0 ||
+ring->funcs->vmhub == AMDGPU_MMHUB_1) &&
+ (!amdgpu_sriov_vf(adev)))
/*
* add semaphore release after invalidation,
* write with 0 means semaphore release
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
index 231ea9762cb5..6c9a9c09cdb1 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -464,8 +464,9 @@ static void gmc_v9_0_flush_gpu_tlb(struct amdgpu_device
*adev, uint32_t vmid,
*/
/* TODO: It needs to continue working on debugging with semaphore for GFXHUB as well. */
- if (vmhub == AMDGPU_MMHUB_0 ||
- vmhub == AMDGPU_MMHUB_1) {
+ if ((vmhub == AMDGPU_MMHUB_0 ||
+vmhub == AMDGPU_MMHUB_1) &&
+ (!amdgpu_sriov_vf(adev))) {
for (j = 0; j < adev->usec_timeout; j++) {
/* a read return value of 1 means semaphore acuqire */
