Not too sure about this, but it looks strange in the code when all other 
similar code uses 64-bit.

"Maybe it makes more sense to think of 64-bit doorbells as using 2 doorbell 
indexes". You mean the alternative is to multiply all the current 64 doorbell 
index constants with 2, right? That might be easier and cleaner, and we need to 
make sure that the *2 and << 1 conversions from 64-bit index to dword index are 
all removed.

Regards,
Yong
________________________________
From: Kuehling, Felix
Sent: Wednesday, February 6, 2019 11:26 AM
To: Zhao, Yong; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 2/3] drm/amdgpu: Fix a typo in vega10_ih_set_rptr()

Are you sure about this? Typically 64-bit doorbells don't wrap around. But this 
one does. If the IH doorbell wraps around, there is no reason why it needs to 
be 64-bit, so I suspect it may still be a 32-bit doorbell.

AFAIK, not all doorbells on Vega10 are 64-bit. It depends on the IP block. 
Therefore, maybe some of your other renaming changes should be reconsidered if 
they assume that all doorbells are 64-bit. Maybe it makes more sense to think 
of 64-bit doorbells as using 2 doorbell indexes.

Regards,
  Felix

________________________________________
From: amd-gfx <amd-gfx-boun...@lists.freedesktop.org> on behalf of Zhao, Yong 
<yong.z...@amd.com>
Sent: Wednesday, February 6, 2019 10:49 AM
To: amd-gfx@lists.freedesktop.org
Cc: Zhao, Yong
Subject: [PATCH 2/3] drm/amdgpu: Fix a typo in vega10_ih_set_rptr()

Clearly, it should be a 64-bit doorbell operation.

Change-Id: I644a2ebcb18c2ede24ee15692a6189efad10a35c
Signed-off-by: Yong Zhao <yong.z...@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/vega10_ih.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/vega10_ih.c 
b/drivers/gpu/drm/amd/amdgpu/vega10_ih.c
index 796004896661..36f0e3cada30 100644
--- a/drivers/gpu/drm/amd/amdgpu/vega10_ih.c
+++ b/drivers/gpu/drm/amd/amdgpu/vega10_ih.c
@@ -377,7 +377,7 @@ static void vega10_ih_set_rptr(struct amdgpu_device *adev,
        if (ih->use_doorbell) {
                /* XXX check if swapping is necessary on BE */
                *ih->rptr_cpu = ih->rptr;
-               WDOORBELL32(ih->doorbell_idx_in_dw, ih->rptr);
+               WDOORBELL64(ih->doorbell_idx_in_dw, ih->rptr);
        } else if (ih == &adev->irq.ih) {
                WREG32_SOC15(OSSSYS, 0, mmIH_RB_RPTR, ih->rptr);
        } else if (ih == &adev->irq.ih1) {
--
2.17.1

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

Reply via email to