[PATCH] drm/amdgpu: right shift 2 bits for SDMA_GFX_RB_WPTR_POLL_ADDR_LO v2

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 at 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 at 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 at 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));
>>
>>


[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux