Am 25.05.2018 um 17:33 schrieb Boyuan Zhang:
On 2018-05-25 05:07 AM, Christian König wrote:
Am 24.05.2018 um 22:15 schrieb [email protected]:
From: Boyuan Zhang <[email protected]>
Allocate extra space in vcn jpeg ring buffer and store the jpeg ring
patch
Signed-off-by: Boyuan Zhang <[email protected]>
---
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);
Taking a closer look that hack might not work after all. E.g. you need
to overwrite max_dw, buf_mask and ptr_mask after initialization.
Maybe add an "extra_dw" field into amdgpu_ring_funcs and use that in
amdgpu_ring_init().
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?
Yeah, that should at least work as expected. I would also add a comment
here why we actually do all this.
+
+ /* 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;
+ }
Would be nice if we could avoid that call as well, cause this is really
not ring buffer operation but rather setting up the environment.
+
+ /* 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)?
Well something like this should work:
/* pointer to the end of the ring buffer */
i = ring->max_dw * amdgpu_sched_hw_submission;
ring->ring[i++] = ....
ring->ring[i++] = ....
ring->ring[i++] = ....
ring->ring[i++] = ....
Regards,
Christian.
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;
}
_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
