On 2024-03-13 13:43, Dewan Alam wrote:
IH Retry CAM should be enabled by register reads instead of always being set to
true.
This explanation sounds odd. Your code is still writing the register
first. What's the reason for reading back the register? I assume it's
not needed for enabling the CAM, but to check whether it was enabled
successfully. What are the configurations where it cannot be enabled
successfully?
Two more nit-picks inline ...
Signed-off-by: Dewan Alam <[email protected]>
---
drivers/gpu/drm/amd/amdgpu/vega20_ih.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/vega20_ih.c
b/drivers/gpu/drm/amd/amdgpu/vega20_ih.c
index b9e785846637..c330f5a88a06 100644
--- a/drivers/gpu/drm/amd/amdgpu/vega20_ih.c
+++ b/drivers/gpu/drm/amd/amdgpu/vega20_ih.c
@@ -337,13 +337,20 @@ static int vega20_ih_irq_init(struct amdgpu_device *adev)
/* Enable IH Retry CAM */
if (amdgpu_ip_version(adev, OSSSYS_HWIP, 0) == IP_VERSION(4, 4, 0) ||
- amdgpu_ip_version(adev, OSSSYS_HWIP, 0) == IP_VERSION(4, 4, 2))
+ amdgpu_ip_version(adev, OSSSYS_HWIP, 0) == IP_VERSION(4, 4, 2)) {
WREG32_FIELD15(OSSSYS, 0, IH_RETRY_INT_CAM_CNTL_ALDEBARAN,
ENABLE, 1);
- else
+ adev->irq.retry_cam_enabled = REG_GET_FIELD(
+ RREG32_SOC15(OSSSYS, 0,
+ mmIH_RETRY_INT_CAM_CNTL_ALDEBARAN),
+ IH_RETRY_INT_CAM_CNTL_ALDEBARAN, ENABLE);
+ } else {
Indentation looks wrong here.
WREG32_FIELD15(OSSSYS, 0, IH_RETRY_INT_CAM_CNTL, ENABLE, 1);
-
- adev->irq.retry_cam_enabled = true;
+ adev->irq.retry_cam_enabled = REG_GET_FIELD(
+ RREG32_SOC15(OSSSYS, 0,
+ mmIH_RETRY_INT_CAM_CNTL),
+ IH_RETRY_INT_CAM_CNTL, ENABLE);
+ }
Wrong indentation.
Regards,
Felix
/* enable interrupts */
ret = vega20_ih_toggle_interrupts(adev, true);
