That won't work either.
We still need to initialize the control registers and tell the hardware
that we have properly setup the ring buffers.
Just add the error message to psp_reg_program() instead of duplicating
that over and over again.
Christian.
Am 07.06.21 um 19:33 schrieb Khaire, Rohit:
[AMD Public Use]
OK.
I will just skip the function call for SRIOV and resend.
Rohit
-----Original Message-----
From: Koenig, Christian <Christian.Koenig@xxxxxxx>
Sent: June 7, 2021 12:42 PM
To: Kuehling, Felix <Felix.Kuehling@xxxxxxx>; Khaire, Rohit <Rohit.Khaire@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx; Deucher, Alexander <Alexander.Deucher@xxxxxxx>; Zhang, Hawking <Hawking.Zhang@xxxxxxx>; Deng, Emily <Emily.Deng@xxxxxxx>; Liu, Monk <Monk.Liu@xxxxxxx>; Zhou, Peng Ju <PengJu.Zhou@xxxxxxx>; Chen, Horace <Horace.Chen@xxxxxxx>
Cc: Ming, Davis <Davis.Ming@xxxxxxx>
Subject: Re: [PATCH] drm/amdgpu: Use PSP to program IH_RB_CNTL_RING1/2 on SRIOV
Ah, good point. In this case we should probably rather save than sorry.
Then I suggest to clean up this patch, repeating the psp_reg_program() and error message is pretty horrible coding style.
Christian.
Am 07.06.21 um 18:36 schrieb Felix Kuehling:
With SRIOV, the interrupt routing is setup by the hypervisor driver.
We need the secondary IH rings in case the hypervisor enabled
rerouting of page fault interrupts. I'm not sure what the hypervisor driver does today.
Regards,
Felix
Am 2021-06-07 um 12:29 p.m. schrieb Christian König:
That's a workaround for bare metal and as far as I know doesn't apply
to SRIOV.
We only need the additional IH rings for page fault handling or log
handling and as far as I know that is incompatible with SRIOV for the
moment. But Felix might have some more updates on this.
So as long as we don't support that under SRIOV we don't need this
patch either.
Christian.
Am 07.06.21 um 17:59 schrieb Khaire, Rohit:
[AMD Public Use]
The hash is 5ea6f9c
Rohit
-----Original Message-----
From: Koenig, Christian <Christian.Koenig@xxxxxxx>
Sent: June 7, 2021 11:58 AM
To: Khaire, Rohit <Rohit.Khaire@xxxxxxx>;
amd-gfx@xxxxxxxxxxxxxxxxxxxxx; Deucher, Alexander
<Alexander.Deucher@xxxxxxx>; Zhang, Hawking <Hawking.Zhang@xxxxxxx>;
Deng, Emily <Emily.Deng@xxxxxxx>; Liu, Monk <Monk.Liu@xxxxxxx>;
Zhou, Peng Ju <PengJu.Zhou@xxxxxxx>; Chen, Horace
<Horace.Chen@xxxxxxx>
Cc: Ming, Davis <Davis.Ming@xxxxxxx>
Subject: Re: [PATCH] drm/amdgpu: Use PSP to program
IH_RB_CNTL_RING1/2 on SRIOV
Do you have the hash for this commit?
Thanks,
Christian.
Am 07.06.21 um 17:30 schrieb Khaire, Rohit:
[AMD Public Use]
We don't need RING1 and RING2 functionality for SRIOV afaik.
But looking at the description of the original commit message it
affects RING0 too?
" drm/amdgpu: add timeout flush mechanism to update wptr for self
interrupt (v2)
outstanding log reaches threshold will trigger IH ring1/2's wptr
reported, that will avoid generating interrupts to ring0 too frequent.
But if ring1/2's wptr hasn't been increased for a long time, the
outstanding log can't reach threshold so that driver can't get
latest wptr info and miss some interrupts."
Rohit
-----Original Message-----
From: Koenig, Christian <Christian.Koenig@xxxxxxx>
Sent: June 7, 2021 10:31 AM
To: Khaire, Rohit <Rohit.Khaire@xxxxxxx>;
amd-gfx@xxxxxxxxxxxxxxxxxxxxx; Deucher, Alexander
<Alexander.Deucher@xxxxxxx>; Zhang, Hawking
<Hawking.Zhang@xxxxxxx>; Deng, Emily <Emily.Deng@xxxxxxx>; Liu,
Monk <Monk.Liu@xxxxxxx>; Zhou, Peng Ju <PengJu.Zhou@xxxxxxx>; Chen,
Horace <Horace.Chen@xxxxxxx>
Cc: Ming, Davis <Davis.Ming@xxxxxxx>
Subject: Re: [PATCH] drm/amdgpu: Use PSP to program
IH_RB_CNTL_RING1/2 on SRIOV
Why are the ring 1&2 enabled on SRIOV in the first place?
Christian.
Am 07.06.21 um 16:23 schrieb Rohit Khaire:
This is similar to IH_RB_CNTL programming in
navi10_ih_toggle_ring_interrupts
Signed-off-by: Rohit Khaire <rohit.khaire@xxxxxxx>
---
drivers/gpu/drm/amd/amdgpu/navi10_ih.c | 20
++++++++++++++++++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/navi10_ih.c
b/drivers/gpu/drm/amd/amdgpu/navi10_ih.c
index eac564e8dd52..e41188c04846 100644
--- a/drivers/gpu/drm/amd/amdgpu/navi10_ih.c
+++ b/drivers/gpu/drm/amd/amdgpu/navi10_ih.c
@@ -120,11 +120,27 @@ force_update_wptr_for_self_int(struct
amdgpu_device *adev,
ih_rb_cntl = REG_SET_FIELD(ih_rb_cntl, IH_RB_CNTL_RING1,
RB_USED_INT_THRESHOLD, threshold);
- WREG32_SOC15(OSSSYS, 0, mmIH_RB_CNTL_RING1, ih_rb_cntl);
+ if (amdgpu_sriov_vf(adev) &&
amdgpu_sriov_reg_indirect_ih(adev)) {
+ if (psp_reg_program(&adev->psp, PSP_REG_IH_RB_CNTL_RING1,
ih_rb_cntl)) {
+ DRM_ERROR("PSP program IH_RB_CNTL_RING1 failed!\n");
+ return;
+ }
+ } else {
+ WREG32_SOC15(OSSSYS, 0, mmIH_RB_CNTL_RING1, ih_rb_cntl);
+ }
+
ih_rb_cntl = RREG32_SOC15(OSSSYS, 0, mmIH_RB_CNTL_RING2);
ih_rb_cntl = REG_SET_FIELD(ih_rb_cntl, IH_RB_CNTL_RING2,
RB_USED_INT_THRESHOLD, threshold);
- WREG32_SOC15(OSSSYS, 0, mmIH_RB_CNTL_RING2, ih_rb_cntl);
+ if (amdgpu_sriov_vf(adev) &&
amdgpu_sriov_reg_indirect_ih(adev)) {
+ if (psp_reg_program(&adev->psp, PSP_REG_IH_RB_CNTL_RING2,
ih_rb_cntl)) {
+ DRM_ERROR("PSP program IH_RB_CNTL_RING2 failed!\n");
+ return;
+ }
+ } else {
+ WREG32_SOC15(OSSSYS, 0, mmIH_RB_CNTL_RING2, ih_rb_cntl);
+ }
+
WREG32_SOC15(OSSSYS, 0, mmIH_CNTL2, ih_cntl);
}
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
