On 2018-05-25 05:07 AM, Christian König wrote:
> Am 24.05.2018 um 22:15 schrieb boyuan.zhang at amd.com:
>> From: Boyuan Zhang <boyuan.zhang at amd.com>
>>
>> Allocate extra space in vcn jpeg ring buffer and store the jpeg ring
>> patch
>>
>> Signed-off-by: Boyuan Zhang <boyuan.zhang at amd.com>
>> ---
>> Â drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c | 27 ++++++++++++++++++++++++++-
>> Â 1 file changed, 26 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
>> b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
>> index dcd1a9a..2e4bd26 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
>> @@ -119,7 +119,8 @@ static int vcn_v1_0_sw_init(void *handle)
>> Â Â Â Â Â Â ring = &adev->vcn.ring_jpeg;
>> Â Â Â Â Â sprintf(ring->name, "vcn_jpeg");
>> -Â Â Â r = amdgpu_ring_init(adev, ring, 512, &adev->vcn.irq, 0);
>> +Â Â Â /* allocate extra dw in ring buffer for storing patch commands */
>> +Â Â Â r = amdgpu_ring_init(adev, ring, 512 + 64, &adev->vcn.irq, 0);
>> Â Â Â Â Â if (r)
>> Â Â Â Â Â Â Â Â Â return r;
>> Â @@ -679,6 +680,30 @@ static int vcn_v1_0_start(struct amdgpu_device
>> *adev)
>> Â Â Â Â Â WREG32_SOC15(UVD, 0, mmUVD_JRBC_RB_WPTR, 0);
>> Â Â Â Â Â WREG32_SOC15(UVD, 0, mmUVD_JRBC_RB_CNTL, 0x00000002L);
>> Â +Â Â Â /* set wptr to the extra allocated space in ring buffer */
>> +Â Â Â ring->wptr = RREG32_SOC15(UVD, 0, mmUVD_JRBC_RB_WPTR);
>> +Â Â Â ring->wptr += ring->max_dw * amdgpu_sched_hw_submission;
>> +
>> +Â Â Â /* increase mask to allow to write to the extra space */
>> +Â Â Â ring->buf_mask += 64 * 4;
>> +Â Â Â ring->ptr_mask += 64 * 4;
>
> Well that is rather ugly. buf_mask and ptr_mask are bit masks, you
> could set them to 0xffffffff but what you do here might not work as
> expected.
OK, so maybe setting both mask to 0xffffffff temporarily to allow
writing to extra space as mu as needed, then later on set them back?
>
>> +
>> +Â Â Â /* allocate extra space */
>> +Â Â Â r = amdgpu_ring_alloc(ring, 64);
>> +Â Â Â if (r) {
>> +Â Â Â Â Â Â Â DRM_ERROR("amdgpu: cp failed to lock ring %d (%d).\n",
>> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â ring->idx, r);
>> +Â Â Â Â Â Â Â return r;
>> +Â Â Â }
>> +
>> +Â Â Â /* copy patch commands to the extra space */
>> +Â Â Â vcn_v1_0_jpeg_ring_set_patch_ring(ring);
>
> Can't vcn_v1_0_jpeg_ring_set_patch_ring() just patch the command
> directly to he end of the ring buffer?
>
> In other words why do we need to use amdgpu_ring_write() in
> vcn_v1_0_jpeg_ring_set_patch_ring()?
>
> Christian.
Could you give me some more detailed info on how to do that? I thought
amdgpu_ring_write() is the best way to write commands to the ring, what
is the other/better way to do it in this case? Could you point me to a
simple example how to write command directly to the end of ring (without
ring_write)?
Thanks,
Boyuan
>
>> +
>> +Â Â Â /* reset wptr and mask */
>> +Â Â Â ring->wptr = RREG32_SOC15(UVD, 0, mmUVD_JRBC_RB_WPTR);
>> +Â Â Â ring->buf_mask -= 0x100;
>> +Â Â Â ring->ptr_mask -= 0x100;
>> +
>> Â Â Â Â Â return 0;
>> Â }
>
