All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/amdgpu: right shift 2 bits for SDMA_GFX_RB_WPTR_POLL_ADDR_LO v2
@ 2017-09-25  6:38 Pixel Ding
       [not found] ` <1506321498-26896-1-git-send-email-Pixel.Ding-5C7GfCeVMHo@public.gmane.org>
  0 siblings, 1 reply; 6+ messages in thread
From: Pixel Ding @ 2017-09-25  6:38 UTC (permalink / raw)
  To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Pixel.Ding-5C7GfCeVMHo,
	Frank.Min-5C7GfCeVMHo, Monk.Liu-5C7GfCeVMHo,
	Alexander.Deucher-5C7GfCeVMHo

Both Tonga and Vega register SPECs indicate that this registers only
use 31:2 bits in DW. SRIOV test case immediately fails withtout this
shift.

v2: write to ADDR field

Signed-off-by: Pixel Ding <Pixel.Ding@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c | 9 +++++----
 drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 8 +++++---
 2 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
index 72f31cc..8b83b96 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
@@ -643,7 +643,7 @@ static void sdma_v3_0_enable(struct amdgpu_device *adev, bool enable)
 static int sdma_v3_0_gfx_resume(struct amdgpu_device *adev)
 {
 	struct amdgpu_ring *ring;
-	u32 rb_cntl, ib_cntl, wptr_poll_cntl;
+	u32 rb_cntl, ib_cntl, wptr_poll_cntl, wptr_poll_addr_lo;
 	u32 rb_bufsz;
 	u32 wb_offset;
 	u32 doorbell;
@@ -712,9 +712,10 @@ static int sdma_v3_0_gfx_resume(struct amdgpu_device *adev)
 
 		/* setup the wptr shadow polling */
 		wptr_gpu_addr = adev->wb.gpu_addr + (ring->wptr_offs * 4);
-
-		WREG32(mmSDMA0_GFX_RB_WPTR_POLL_ADDR_LO + sdma_offsets[i],
-		       lower_32_bits(wptr_gpu_addr));
+		wptr_poll_addr_lo = RREG32(mmSDMA0_GFX_RB_WPTR_POLL_ADDR_LO + sdma_offsets[i]);
+		wptr_poll_addr_lo = REG_SET_FIELD(wptr_poll_addr_lo, SDMA0_GFX_RB_WPTR_POLL_ADDR_LO,
+						  ADDR, lower_32_bits(wptr_gpu_addr) >> 2);
+		WREG32(mmSDMA0_GFX_RB_WPTR_POLL_ADDR_LO + sdma_offsets[i], wptr_poll_addr_lo;
 		WREG32(mmSDMA0_GFX_RB_WPTR_POLL_ADDR_HI + sdma_offsets[i],
 		       upper_32_bits(wptr_gpu_addr));
 		wptr_poll_cntl = RREG32(mmSDMA0_GFX_RB_WPTR_POLL_CNTL + sdma_offsets[i]);
diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
index c26d205..8b8338d 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
@@ -574,7 +574,7 @@ static void sdma_v4_0_enable(struct amdgpu_device *adev, bool enable)
 static int sdma_v4_0_gfx_resume(struct amdgpu_device *adev)
 {
 	struct amdgpu_ring *ring;
-	u32 rb_cntl, ib_cntl, wptr_poll_cntl;
+	u32 rb_cntl, ib_cntl, wptr_poll_cntl, wptr_poll_addr_lo
 	u32 rb_bufsz;
 	u32 wb_offset;
 	u32 doorbell;
@@ -664,8 +664,10 @@ static int sdma_v4_0_gfx_resume(struct amdgpu_device *adev)
 
 		/* setup the wptr shadow polling */
 		wptr_gpu_addr = adev->wb.gpu_addr + (ring->wptr_offs * 4);
-		WREG32(sdma_v4_0_get_reg_offset(i, mmSDMA0_GFX_RB_WPTR_POLL_ADDR_LO),
-		       lower_32_bits(wptr_gpu_addr));
+		wptr_poll_addr_lo = RREG32(sdma_v4_0_get_reg_offset(i, mmSDMA0_GFX_RB_WPTR_POLL_ADDR_LO));
+		wptr_poll_addr_lo = REG_SET_FIELD(wptr_poll_addr_lo, SDMA0_GFX_RB_WPTR_POLL_ADDR_LO,
+						  ADDR, lower_32_bits(wptr_gpu_addr) >> 2);
+		WREG32(sdma_v4_0_get_reg_offset(i, mmSDMA0_GFX_RB_WPTR_POLL_ADDR_LO), wptr_poll_addr_lo);
 		WREG32(sdma_v4_0_get_reg_offset(i, mmSDMA0_GFX_RB_WPTR_POLL_ADDR_HI),
 		       upper_32_bits(wptr_gpu_addr));
 		wptr_poll_cntl = RREG32(sdma_v4_0_get_reg_offset(i, mmSDMA0_GFX_RB_WPTR_POLL_CNTL));
-- 
2.7.4

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/amdgpu: right shift 2 bits for SDMA_GFX_RB_WPTR_POLL_ADDR_LO v2
       [not found] ` <1506321498-26896-1-git-send-email-Pixel.Ding-5C7GfCeVMHo@public.gmane.org>
@ 2017-09-25  6:55   ` Ding, Pixel
  2017-09-25  7:08   ` Christian König
  2017-09-25 16:36   ` Deucher, Alexander
  2 siblings, 0 replies; 6+ messages in thread
From: Ding, Pixel @ 2017-09-25  6:55 UTC (permalink / raw)
  To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Min, Frank, Liu, Monk,
	Deucher, Alexander, Yu, Xiangliang

+Xiangliang,
— 
Sincerely Yours,
Pixel








On 25/09/2017, 2:38 PM, "Pixel Ding" <Pixel.Ding@amd.com> wrote:

>Both Tonga and Vega register SPECs indicate that this registers only
>use 31:2 bits in DW. SRIOV test case immediately fails withtout this
>shift.
>
>v2: write to ADDR field
>
>Signed-off-by: Pixel Ding <Pixel.Ding@amd.com>
>---
> drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c | 9 +++++----
> drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 8 +++++---
> 2 files changed, 10 insertions(+), 7 deletions(-)
>
>diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
>index 72f31cc..8b83b96 100644
>--- a/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
>+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
>@@ -643,7 +643,7 @@ static void sdma_v3_0_enable(struct amdgpu_device *adev, bool enable)
> static int sdma_v3_0_gfx_resume(struct amdgpu_device *adev)
> {
> 	struct amdgpu_ring *ring;
>-	u32 rb_cntl, ib_cntl, wptr_poll_cntl;
>+	u32 rb_cntl, ib_cntl, wptr_poll_cntl, wptr_poll_addr_lo;
> 	u32 rb_bufsz;
> 	u32 wb_offset;
> 	u32 doorbell;
>@@ -712,9 +712,10 @@ static int sdma_v3_0_gfx_resume(struct amdgpu_device *adev)
> 
> 		/* setup the wptr shadow polling */
> 		wptr_gpu_addr = adev->wb.gpu_addr + (ring->wptr_offs * 4);
>-
>-		WREG32(mmSDMA0_GFX_RB_WPTR_POLL_ADDR_LO + sdma_offsets[i],
>-		       lower_32_bits(wptr_gpu_addr));
>+		wptr_poll_addr_lo = RREG32(mmSDMA0_GFX_RB_WPTR_POLL_ADDR_LO + sdma_offsets[i]);
>+		wptr_poll_addr_lo = REG_SET_FIELD(wptr_poll_addr_lo, SDMA0_GFX_RB_WPTR_POLL_ADDR_LO,
>+						  ADDR, lower_32_bits(wptr_gpu_addr) >> 2);
>+		WREG32(mmSDMA0_GFX_RB_WPTR_POLL_ADDR_LO + sdma_offsets[i], wptr_poll_addr_lo;
> 		WREG32(mmSDMA0_GFX_RB_WPTR_POLL_ADDR_HI + sdma_offsets[i],
> 		       upper_32_bits(wptr_gpu_addr));
> 		wptr_poll_cntl = RREG32(mmSDMA0_GFX_RB_WPTR_POLL_CNTL + sdma_offsets[i]);
>diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
>index c26d205..8b8338d 100644
>--- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
>+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
>@@ -574,7 +574,7 @@ static void sdma_v4_0_enable(struct amdgpu_device *adev, bool enable)
> static int sdma_v4_0_gfx_resume(struct amdgpu_device *adev)
> {
> 	struct amdgpu_ring *ring;
>-	u32 rb_cntl, ib_cntl, wptr_poll_cntl;
>+	u32 rb_cntl, ib_cntl, wptr_poll_cntl, wptr_poll_addr_lo
> 	u32 rb_bufsz;
> 	u32 wb_offset;
> 	u32 doorbell;
>@@ -664,8 +664,10 @@ static int sdma_v4_0_gfx_resume(struct amdgpu_device *adev)
> 
> 		/* setup the wptr shadow polling */
> 		wptr_gpu_addr = adev->wb.gpu_addr + (ring->wptr_offs * 4);
>-		WREG32(sdma_v4_0_get_reg_offset(i, mmSDMA0_GFX_RB_WPTR_POLL_ADDR_LO),
>-		       lower_32_bits(wptr_gpu_addr));
>+		wptr_poll_addr_lo = RREG32(sdma_v4_0_get_reg_offset(i, mmSDMA0_GFX_RB_WPTR_POLL_ADDR_LO));
>+		wptr_poll_addr_lo = REG_SET_FIELD(wptr_poll_addr_lo, SDMA0_GFX_RB_WPTR_POLL_ADDR_LO,
>+						  ADDR, lower_32_bits(wptr_gpu_addr) >> 2);
>+		WREG32(sdma_v4_0_get_reg_offset(i, mmSDMA0_GFX_RB_WPTR_POLL_ADDR_LO), wptr_poll_addr_lo);
> 		WREG32(sdma_v4_0_get_reg_offset(i, mmSDMA0_GFX_RB_WPTR_POLL_ADDR_HI),
> 		       upper_32_bits(wptr_gpu_addr));
> 		wptr_poll_cntl = RREG32(sdma_v4_0_get_reg_offset(i, mmSDMA0_GFX_RB_WPTR_POLL_CNTL));
>-- 
>2.7.4
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/amdgpu: right shift 2 bits for SDMA_GFX_RB_WPTR_POLL_ADDR_LO v2
       [not found] ` <1506321498-26896-1-git-send-email-Pixel.Ding-5C7GfCeVMHo@public.gmane.org>
  2017-09-25  6:55   ` Ding, Pixel
@ 2017-09-25  7:08   ` Christian König
       [not found]     ` <02108f90-185e-df0e-5662-c88ca055085f-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  2017-09-25 16:36   ` Deucher, Alexander
  2 siblings, 1 reply; 6+ messages in thread
From: Christian König @ 2017-09-25  7:08 UTC (permalink / raw)
  To: Pixel Ding, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	Frank.Min-5C7GfCeVMHo, Monk.Liu-5C7GfCeVMHo,
	Alexander.Deucher-5C7GfCeVMHo

NAK, that doesn't looks correct to me.

> Both Tonga and Vega register SPECs indicate that this registers only
> use 31:2 bits in DW.
This means that the value must be DW aligned and NOT that it needs to be 
shifted by 2!

Regards,
Christian.

Am 25.09.2017 um 08:38 schrieb Pixel Ding:
> Both Tonga and Vega register SPECs indicate that this registers only
> use 31:2 bits in DW. SRIOV test case immediately fails withtout this
> shift.
>
> v2: write to ADDR field
>
> Signed-off-by: Pixel Ding <Pixel.Ding@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c | 9 +++++----
>   drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 8 +++++---
>   2 files changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
> index 72f31cc..8b83b96 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
> @@ -643,7 +643,7 @@ static void sdma_v3_0_enable(struct amdgpu_device *adev, bool enable)
>   static int sdma_v3_0_gfx_resume(struct amdgpu_device *adev)
>   {
>   	struct amdgpu_ring *ring;
> -	u32 rb_cntl, ib_cntl, wptr_poll_cntl;
> +	u32 rb_cntl, ib_cntl, wptr_poll_cntl, wptr_poll_addr_lo;
>   	u32 rb_bufsz;
>   	u32 wb_offset;
>   	u32 doorbell;
> @@ -712,9 +712,10 @@ static int sdma_v3_0_gfx_resume(struct amdgpu_device *adev)
>   
>   		/* setup the wptr shadow polling */
>   		wptr_gpu_addr = adev->wb.gpu_addr + (ring->wptr_offs * 4);
> -
> -		WREG32(mmSDMA0_GFX_RB_WPTR_POLL_ADDR_LO + sdma_offsets[i],
> -		       lower_32_bits(wptr_gpu_addr));
> +		wptr_poll_addr_lo = RREG32(mmSDMA0_GFX_RB_WPTR_POLL_ADDR_LO + sdma_offsets[i]);
> +		wptr_poll_addr_lo = REG_SET_FIELD(wptr_poll_addr_lo, SDMA0_GFX_RB_WPTR_POLL_ADDR_LO,
> +						  ADDR, lower_32_bits(wptr_gpu_addr) >> 2);
> +		WREG32(mmSDMA0_GFX_RB_WPTR_POLL_ADDR_LO + sdma_offsets[i], wptr_poll_addr_lo;
>   		WREG32(mmSDMA0_GFX_RB_WPTR_POLL_ADDR_HI + sdma_offsets[i],
>   		       upper_32_bits(wptr_gpu_addr));
>   		wptr_poll_cntl = RREG32(mmSDMA0_GFX_RB_WPTR_POLL_CNTL + sdma_offsets[i]);
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> index c26d205..8b8338d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> @@ -574,7 +574,7 @@ static void sdma_v4_0_enable(struct amdgpu_device *adev, bool enable)
>   static int sdma_v4_0_gfx_resume(struct amdgpu_device *adev)
>   {
>   	struct amdgpu_ring *ring;
> -	u32 rb_cntl, ib_cntl, wptr_poll_cntl;
> +	u32 rb_cntl, ib_cntl, wptr_poll_cntl, wptr_poll_addr_lo
>   	u32 rb_bufsz;
>   	u32 wb_offset;
>   	u32 doorbell;
> @@ -664,8 +664,10 @@ static int sdma_v4_0_gfx_resume(struct amdgpu_device *adev)
>   
>   		/* setup the wptr shadow polling */
>   		wptr_gpu_addr = adev->wb.gpu_addr + (ring->wptr_offs * 4);
> -		WREG32(sdma_v4_0_get_reg_offset(i, mmSDMA0_GFX_RB_WPTR_POLL_ADDR_LO),
> -		       lower_32_bits(wptr_gpu_addr));
> +		wptr_poll_addr_lo = RREG32(sdma_v4_0_get_reg_offset(i, mmSDMA0_GFX_RB_WPTR_POLL_ADDR_LO));
> +		wptr_poll_addr_lo = REG_SET_FIELD(wptr_poll_addr_lo, SDMA0_GFX_RB_WPTR_POLL_ADDR_LO,
> +						  ADDR, lower_32_bits(wptr_gpu_addr) >> 2);
> +		WREG32(sdma_v4_0_get_reg_offset(i, mmSDMA0_GFX_RB_WPTR_POLL_ADDR_LO), wptr_poll_addr_lo);
>   		WREG32(sdma_v4_0_get_reg_offset(i, mmSDMA0_GFX_RB_WPTR_POLL_ADDR_HI),
>   		       upper_32_bits(wptr_gpu_addr));
>   		wptr_poll_cntl = RREG32(sdma_v4_0_get_reg_offset(i, mmSDMA0_GFX_RB_WPTR_POLL_CNTL));


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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/amdgpu: right shift 2 bits for SDMA_GFX_RB_WPTR_POLL_ADDR_LO v2
       [not found]     ` <02108f90-185e-df0e-5662-c88ca055085f-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2017-09-25  7:12       ` Ding, Pixel
       [not found]         ` <E035AF8E-BFA7-40C3-A859-946BFFCFBE69-5C7GfCeVMHo@public.gmane.org>
  0 siblings, 1 reply; 6+ messages in thread
From: Ding, Pixel @ 2017-09-25  7:12 UTC (permalink / raw)
  To: Koenig, Christian, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Min,
	Frank, Liu, Monk, Deucher, Alexander

Yes, it seems not related to the seen issue. The previous change simplifies the shift operations while the logic is same. Please ignore.
— 
Sincerely Yours,
Pixel








On 25/09/2017, 3:08 PM, "Christian König" <ckoenig.leichtzumerken@gmail.com> wrote:

>NAK, that doesn't looks correct to me.
>
>> Both Tonga and Vega register SPECs indicate that this registers only
>> use 31:2 bits in DW.
>This means that the value must be DW aligned and NOT that it needs to be 
>shifted by 2!
>
>Regards,
>Christian.
>
>Am 25.09.2017 um 08:38 schrieb Pixel Ding:
>> Both Tonga and Vega register SPECs indicate that this registers only
>> use 31:2 bits in DW. SRIOV test case immediately fails withtout this
>> shift.
>>
>> v2: write to ADDR field
>>
>> Signed-off-by: Pixel Ding <Pixel.Ding@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c | 9 +++++----
>>   drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 8 +++++---
>>   2 files changed, 10 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
>> index 72f31cc..8b83b96 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
>> @@ -643,7 +643,7 @@ static void sdma_v3_0_enable(struct amdgpu_device *adev, bool enable)
>>   static int sdma_v3_0_gfx_resume(struct amdgpu_device *adev)
>>   {
>>   	struct amdgpu_ring *ring;
>> -	u32 rb_cntl, ib_cntl, wptr_poll_cntl;
>> +	u32 rb_cntl, ib_cntl, wptr_poll_cntl, wptr_poll_addr_lo;
>>   	u32 rb_bufsz;
>>   	u32 wb_offset;
>>   	u32 doorbell;
>> @@ -712,9 +712,10 @@ static int sdma_v3_0_gfx_resume(struct amdgpu_device *adev)
>>   
>>   		/* setup the wptr shadow polling */
>>   		wptr_gpu_addr = adev->wb.gpu_addr + (ring->wptr_offs * 4);
>> -
>> -		WREG32(mmSDMA0_GFX_RB_WPTR_POLL_ADDR_LO + sdma_offsets[i],
>> -		       lower_32_bits(wptr_gpu_addr));
>> +		wptr_poll_addr_lo = RREG32(mmSDMA0_GFX_RB_WPTR_POLL_ADDR_LO + sdma_offsets[i]);
>> +		wptr_poll_addr_lo = REG_SET_FIELD(wptr_poll_addr_lo, SDMA0_GFX_RB_WPTR_POLL_ADDR_LO,
>> +						  ADDR, lower_32_bits(wptr_gpu_addr) >> 2);
>> +		WREG32(mmSDMA0_GFX_RB_WPTR_POLL_ADDR_LO + sdma_offsets[i], wptr_poll_addr_lo;
>>   		WREG32(mmSDMA0_GFX_RB_WPTR_POLL_ADDR_HI + sdma_offsets[i],
>>   		       upper_32_bits(wptr_gpu_addr));
>>   		wptr_poll_cntl = RREG32(mmSDMA0_GFX_RB_WPTR_POLL_CNTL + sdma_offsets[i]);
>> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
>> index c26d205..8b8338d 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
>> @@ -574,7 +574,7 @@ static void sdma_v4_0_enable(struct amdgpu_device *adev, bool enable)
>>   static int sdma_v4_0_gfx_resume(struct amdgpu_device *adev)
>>   {
>>   	struct amdgpu_ring *ring;
>> -	u32 rb_cntl, ib_cntl, wptr_poll_cntl;
>> +	u32 rb_cntl, ib_cntl, wptr_poll_cntl, wptr_poll_addr_lo
>>   	u32 rb_bufsz;
>>   	u32 wb_offset;
>>   	u32 doorbell;
>> @@ -664,8 +664,10 @@ static int sdma_v4_0_gfx_resume(struct amdgpu_device *adev)
>>   
>>   		/* setup the wptr shadow polling */
>>   		wptr_gpu_addr = adev->wb.gpu_addr + (ring->wptr_offs * 4);
>> -		WREG32(sdma_v4_0_get_reg_offset(i, mmSDMA0_GFX_RB_WPTR_POLL_ADDR_LO),
>> -		       lower_32_bits(wptr_gpu_addr));
>> +		wptr_poll_addr_lo = RREG32(sdma_v4_0_get_reg_offset(i, mmSDMA0_GFX_RB_WPTR_POLL_ADDR_LO));
>> +		wptr_poll_addr_lo = REG_SET_FIELD(wptr_poll_addr_lo, SDMA0_GFX_RB_WPTR_POLL_ADDR_LO,
>> +						  ADDR, lower_32_bits(wptr_gpu_addr) >> 2);
>> +		WREG32(sdma_v4_0_get_reg_offset(i, mmSDMA0_GFX_RB_WPTR_POLL_ADDR_LO), wptr_poll_addr_lo);
>>   		WREG32(sdma_v4_0_get_reg_offset(i, mmSDMA0_GFX_RB_WPTR_POLL_ADDR_HI),
>>   		       upper_32_bits(wptr_gpu_addr));
>>   		wptr_poll_cntl = RREG32(sdma_v4_0_get_reg_offset(i, mmSDMA0_GFX_RB_WPTR_POLL_CNTL));
>
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drm/amdgpu: right shift 2 bits for SDMA_GFX_RB_WPTR_POLL_ADDR_LO v2
       [not found]         ` <E035AF8E-BFA7-40C3-A859-946BFFCFBE69-5C7GfCeVMHo@public.gmane.org>
@ 2017-09-25  9:57           ` Ding, Pixel
  0 siblings, 0 replies; 6+ messages in thread
From: Ding, Pixel @ 2017-09-25  9:57 UTC (permalink / raw)
  To: Deucher, Alexander, Koenig, Christian
  Cc: Min, Frank, Liu, Monk, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

Hi Alex,

We found that this statement introduces issue.

In sdma_v3_0_ring_set_wptr():
WRITE_ONCE(*wb, (lower_32_bits(ring->wptr) << 2));

When changing it to normal assignment operation, issue is gone, also if adding some dumps here to make it slower, issue is gone…

Any idea? Is there some reason to use this macro?

---------------
Sincerely Yours,
Pixel







On 25/09/2017, 3:12 PM, "Ding, Pixel" <Pixel.Ding@amd.com> wrote:

>Yes, it seems not related to the seen issue. The previous change simplifies the shift operations while the logic is same. Please ignore.
>— 
>Sincerely Yours,
>Pixel
>
>
>
>
>
>
>
>
>On 25/09/2017, 3:08 PM, "Christian König" <ckoenig.leichtzumerken@gmail.com> wrote:
>
>>NAK, that doesn't looks correct to me.
>>
>>> Both Tonga and Vega register SPECs indicate that this registers only
>>> use 31:2 bits in DW.
>>This means that the value must be DW aligned and NOT that it needs to be 
>>shifted by 2!
>>
>>Regards,
>>Christian.
>>
>>Am 25.09.2017 um 08:38 schrieb Pixel Ding:
>>> Both Tonga and Vega register SPECs indicate that this registers only
>>> use 31:2 bits in DW. SRIOV test case immediately fails withtout this
>>> shift.
>>>
>>> v2: write to ADDR field
>>>
>>> Signed-off-by: Pixel Ding <Pixel.Ding@amd.com>
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c | 9 +++++----
>>>   drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 8 +++++---
>>>   2 files changed, 10 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
>>> index 72f31cc..8b83b96 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
>>> @@ -643,7 +643,7 @@ static void sdma_v3_0_enable(struct amdgpu_device *adev, bool enable)
>>>   static int sdma_v3_0_gfx_resume(struct amdgpu_device *adev)
>>>   {
>>>   	struct amdgpu_ring *ring;
>>> -	u32 rb_cntl, ib_cntl, wptr_poll_cntl;
>>> +	u32 rb_cntl, ib_cntl, wptr_poll_cntl, wptr_poll_addr_lo;
>>>   	u32 rb_bufsz;
>>>   	u32 wb_offset;
>>>   	u32 doorbell;
>>> @@ -712,9 +712,10 @@ static int sdma_v3_0_gfx_resume(struct amdgpu_device *adev)
>>>   
>>>   		/* setup the wptr shadow polling */
>>>   		wptr_gpu_addr = adev->wb.gpu_addr + (ring->wptr_offs * 4);
>>> -
>>> -		WREG32(mmSDMA0_GFX_RB_WPTR_POLL_ADDR_LO + sdma_offsets[i],
>>> -		       lower_32_bits(wptr_gpu_addr));
>>> +		wptr_poll_addr_lo = RREG32(mmSDMA0_GFX_RB_WPTR_POLL_ADDR_LO + sdma_offsets[i]);
>>> +		wptr_poll_addr_lo = REG_SET_FIELD(wptr_poll_addr_lo, SDMA0_GFX_RB_WPTR_POLL_ADDR_LO,
>>> +						  ADDR, lower_32_bits(wptr_gpu_addr) >> 2);
>>> +		WREG32(mmSDMA0_GFX_RB_WPTR_POLL_ADDR_LO + sdma_offsets[i], wptr_poll_addr_lo;
>>>   		WREG32(mmSDMA0_GFX_RB_WPTR_POLL_ADDR_HI + sdma_offsets[i],
>>>   		       upper_32_bits(wptr_gpu_addr));
>>>   		wptr_poll_cntl = RREG32(mmSDMA0_GFX_RB_WPTR_POLL_CNTL + sdma_offsets[i]);
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
>>> index c26d205..8b8338d 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
>>> @@ -574,7 +574,7 @@ static void sdma_v4_0_enable(struct amdgpu_device *adev, bool enable)
>>>   static int sdma_v4_0_gfx_resume(struct amdgpu_device *adev)
>>>   {
>>>   	struct amdgpu_ring *ring;
>>> -	u32 rb_cntl, ib_cntl, wptr_poll_cntl;
>>> +	u32 rb_cntl, ib_cntl, wptr_poll_cntl, wptr_poll_addr_lo
>>>   	u32 rb_bufsz;
>>>   	u32 wb_offset;
>>>   	u32 doorbell;
>>> @@ -664,8 +664,10 @@ static int sdma_v4_0_gfx_resume(struct amdgpu_device *adev)
>>>   
>>>   		/* setup the wptr shadow polling */
>>>   		wptr_gpu_addr = adev->wb.gpu_addr + (ring->wptr_offs * 4);
>>> -		WREG32(sdma_v4_0_get_reg_offset(i, mmSDMA0_GFX_RB_WPTR_POLL_ADDR_LO),
>>> -		       lower_32_bits(wptr_gpu_addr));
>>> +		wptr_poll_addr_lo = RREG32(sdma_v4_0_get_reg_offset(i, mmSDMA0_GFX_RB_WPTR_POLL_ADDR_LO));
>>> +		wptr_poll_addr_lo = REG_SET_FIELD(wptr_poll_addr_lo, SDMA0_GFX_RB_WPTR_POLL_ADDR_LO,
>>> +						  ADDR, lower_32_bits(wptr_gpu_addr) >> 2);
>>> +		WREG32(sdma_v4_0_get_reg_offset(i, mmSDMA0_GFX_RB_WPTR_POLL_ADDR_LO), wptr_poll_addr_lo);
>>>   		WREG32(sdma_v4_0_get_reg_offset(i, mmSDMA0_GFX_RB_WPTR_POLL_ADDR_HI),
>>>   		       upper_32_bits(wptr_gpu_addr));
>>>   		wptr_poll_cntl = RREG32(sdma_v4_0_get_reg_offset(i, mmSDMA0_GFX_RB_WPTR_POLL_CNTL));
>>
>>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH] drm/amdgpu: right shift 2 bits for SDMA_GFX_RB_WPTR_POLL_ADDR_LO v2
       [not found] ` <1506321498-26896-1-git-send-email-Pixel.Ding-5C7GfCeVMHo@public.gmane.org>
  2017-09-25  6:55   ` Ding, Pixel
  2017-09-25  7:08   ` Christian König
@ 2017-09-25 16:36   ` Deucher, Alexander
  2 siblings, 0 replies; 6+ messages in thread
From: Deucher, Alexander @ 2017-09-25 16:36 UTC (permalink / raw)
  To: Ding, Pixel, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

> -----Original Message-----
> From: Pixel Ding [mailto:Pixel.Ding@amd.com]
> Sent: Monday, September 25, 2017 2:38 AM
> To: amd-gfx@lists.freedesktop.org; Ding, Pixel; Min, Frank; Liu, Monk;
> Deucher, Alexander
> Subject: [PATCH] drm/amdgpu: right shift 2 bits for
> SDMA_GFX_RB_WPTR_POLL_ADDR_LO v2
> 
> Both Tonga and Vega register SPECs indicate that this registers only
> use 31:2 bits in DW. SRIOV test case immediately fails withtout this
> shift.

Are you sure this is correct?  I think the comment is just saying that the address is dword aligned.  Also, if it indeed needs to be shifted, I'd imaging the _HI addr field needs to be shifted as well.

Alex

> 
> v2: write to ADDR field
> 
> Signed-off-by: Pixel Ding <Pixel.Ding@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c | 9 +++++----
>  drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 8 +++++---
>  2 files changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
> b/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
> index 72f31cc..8b83b96 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
> @@ -643,7 +643,7 @@ static void sdma_v3_0_enable(struct amdgpu_device
> *adev, bool enable)
>  static int sdma_v3_0_gfx_resume(struct amdgpu_device *adev)
>  {
>  	struct amdgpu_ring *ring;
> -	u32 rb_cntl, ib_cntl, wptr_poll_cntl;
> +	u32 rb_cntl, ib_cntl, wptr_poll_cntl, wptr_poll_addr_lo;
>  	u32 rb_bufsz;
>  	u32 wb_offset;
>  	u32 doorbell;
> @@ -712,9 +712,10 @@ static int sdma_v3_0_gfx_resume(struct
> amdgpu_device *adev)
> 
>  		/* setup the wptr shadow polling */
>  		wptr_gpu_addr = adev->wb.gpu_addr + (ring->wptr_offs *
> 4);
> -
> -		WREG32(mmSDMA0_GFX_RB_WPTR_POLL_ADDR_LO +
> sdma_offsets[i],
> -		       lower_32_bits(wptr_gpu_addr));
> +		wptr_poll_addr_lo =
> RREG32(mmSDMA0_GFX_RB_WPTR_POLL_ADDR_LO + sdma_offsets[i]);
> +		wptr_poll_addr_lo = REG_SET_FIELD(wptr_poll_addr_lo,
> SDMA0_GFX_RB_WPTR_POLL_ADDR_LO,
> +						  ADDR,
> lower_32_bits(wptr_gpu_addr) >> 2);
> +		WREG32(mmSDMA0_GFX_RB_WPTR_POLL_ADDR_LO +
> sdma_offsets[i], wptr_poll_addr_lo;
>  		WREG32(mmSDMA0_GFX_RB_WPTR_POLL_ADDR_HI +
> sdma_offsets[i],
>  		       upper_32_bits(wptr_gpu_addr));
>  		wptr_poll_cntl =
> RREG32(mmSDMA0_GFX_RB_WPTR_POLL_CNTL + sdma_offsets[i]);
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> index c26d205..8b8338d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> @@ -574,7 +574,7 @@ static void sdma_v4_0_enable(struct amdgpu_device
> *adev, bool enable)
>  static int sdma_v4_0_gfx_resume(struct amdgpu_device *adev)
>  {
>  	struct amdgpu_ring *ring;
> -	u32 rb_cntl, ib_cntl, wptr_poll_cntl;
> +	u32 rb_cntl, ib_cntl, wptr_poll_cntl, wptr_poll_addr_lo
>  	u32 rb_bufsz;
>  	u32 wb_offset;
>  	u32 doorbell;
> @@ -664,8 +664,10 @@ static int sdma_v4_0_gfx_resume(struct
> amdgpu_device *adev)
> 
>  		/* setup the wptr shadow polling */
>  		wptr_gpu_addr = adev->wb.gpu_addr + (ring->wptr_offs *
> 4);
> -		WREG32(sdma_v4_0_get_reg_offset(i,
> mmSDMA0_GFX_RB_WPTR_POLL_ADDR_LO),
> -		       lower_32_bits(wptr_gpu_addr));
> +		wptr_poll_addr_lo = RREG32(sdma_v4_0_get_reg_offset(i,
> mmSDMA0_GFX_RB_WPTR_POLL_ADDR_LO));
> +		wptr_poll_addr_lo = REG_SET_FIELD(wptr_poll_addr_lo,
> SDMA0_GFX_RB_WPTR_POLL_ADDR_LO,
> +						  ADDR,
> lower_32_bits(wptr_gpu_addr) >> 2);
> +		WREG32(sdma_v4_0_get_reg_offset(i,
> mmSDMA0_GFX_RB_WPTR_POLL_ADDR_LO), wptr_poll_addr_lo);
>  		WREG32(sdma_v4_0_get_reg_offset(i,
> mmSDMA0_GFX_RB_WPTR_POLL_ADDR_HI),
>  		       upper_32_bits(wptr_gpu_addr));
>  		wptr_poll_cntl = RREG32(sdma_v4_0_get_reg_offset(i,
> mmSDMA0_GFX_RB_WPTR_POLL_CNTL));
> --
> 2.7.4

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2017-09-25 16:36 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-25  6:38 [PATCH] drm/amdgpu: right shift 2 bits for SDMA_GFX_RB_WPTR_POLL_ADDR_LO v2 Pixel Ding
     [not found] ` <1506321498-26896-1-git-send-email-Pixel.Ding-5C7GfCeVMHo@public.gmane.org>
2017-09-25  6:55   ` Ding, Pixel
2017-09-25  7:08   ` Christian König
     [not found]     ` <02108f90-185e-df0e-5662-c88ca055085f-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-09-25  7:12       ` Ding, Pixel
     [not found]         ` <E035AF8E-BFA7-40C3-A859-946BFFCFBE69-5C7GfCeVMHo@public.gmane.org>
2017-09-25  9:57           ` Ding, Pixel
2017-09-25 16:36   ` Deucher, Alexander

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.