Hi Christian, response inline [Saleem]. Regards, Saleem On 07/07/23 12:35, Christian König wrote:
Am 06.07.23 um 16:47 schrieb Saleemkhan Jamadar:add session context buffer to decoder ring test. v4 - data type, explain change of ib size change (Christian) v3 - indent and v2 changes correction. (Christian) v2 - put the buffer at the end of the IB (Christian) Signed-off-by: Saleemkhan Jamadar <saleemkhan.jamadar@xxxxxxx> Acked-by: Leo Liu <leo.liu@xxxxxxx> --- drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 11 +++++++++-- drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h | 5 ++++- 2 files changed, 13 insertions(+), 3 deletions(-)diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.cindex 2d94f1b63bd6..9bdfe665f603 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c@@ -573,7 +573,8 @@ static int amdgpu_vcn_dec_get_create_msg(struct amdgpu_ring *ring, uint32_t handint r, i; memset(ib, 0, sizeof(*ib)); - r = amdgpu_ib_get(adev, NULL, AMDGPU_GPU_PAGE_SIZE * 2,+ /* 34 pages : 128KiB session context buffer size and 8KiB ib msg */+ r = amdgpu_ib_get(adev, NULL, AMDGPU_GPU_PAGE_SIZE * 34, AMDGPU_IB_POOL_DIRECT, ib); if (r)@@ -608,7 +609,8 @@ static int amdgpu_vcn_dec_get_destroy_msg(struct amdgpu_ring *ring, uint32_t hanint r, i; memset(ib, 0, sizeof(*ib)); - r = amdgpu_ib_get(adev, NULL, AMDGPU_GPU_PAGE_SIZE * 2, + /* 34 pages : 128KB session context buffer size and 8KB ib msg */ + r = amdgpu_ib_get(adev, NULL, AMDGPU_GPU_PAGE_SIZE * 34, AMDGPU_IB_POOL_DIRECT, ib);One more question here: Does the create and destroy message need to point to the same session context buffer or is it ok that we use a separate dummy for both? [Saleem] Both case works Ok. Version 1 change had same used for both cmds.
Either way we should probably clear the context buffer with zeros. [Saleem] Noted, will make change. Apart from that this now looks good to me, Christian.if (r)@@ -700,6 +702,7 @@ static int amdgpu_vcn_dec_sw_send_msg(struct amdgpu_ring *ring,struct amdgpu_job *job; struct amdgpu_ib *ib; uint64_t addr = AMDGPU_GPU_PAGE_ALIGN(ib_msg->gpu_addr);+ uint64_t session_ctx_buf_gaddr = AMDGPU_GPU_PAGE_ALIGN(ib_msg->gpu_addr + 8192);bool sq = amdgpu_vcn_using_unified_queue(ring); uint32_t *ib_checksum; uint32_t ib_pack_in_dw;@@ -730,6 +733,10 @@ static int amdgpu_vcn_dec_sw_send_msg(struct amdgpu_ring *ring,ib->length_dw += sizeof(struct amdgpu_vcn_decode_buffer) / 4; memset(decode_buffer, 0, sizeof(struct amdgpu_vcn_decode_buffer)); + decode_buffer->valid_buf_flag |= + cpu_to_le32(AMDGPU_VCN_CMD_FLAG_SESSION_CONTEXT_BUFFER);+ decode_buffer->session_context_buffer_address_hi = upper_32_bits(session_ctx_buf_gaddr); + decode_buffer->session_context_buffer_address_lo = lower_32_bits(session_ctx_buf_gaddr); decode_buffer->valid_buf_flag |= cpu_to_le32(AMDGPU_VCN_CMD_FLAG_MSG_BUFFER);decode_buffer->msg_buffer_address_hi = cpu_to_le32(addr >> 32); decode_buffer->msg_buffer_address_lo = cpu_to_le32(addr);diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.hindex f1397ef66fd7..2df43cd76c10 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h @@ -166,6 +166,7 @@ #define AMDGPU_VCN_IB_FLAG_DECODE_BUFFER 0x00000001 #define AMDGPU_VCN_CMD_FLAG_MSG_BUFFER 0x00000001 +#define AMDGPU_VCN_CMD_FLAG_SESSION_CONTEXT_BUFFER 0x00100000 #define VCN_CODEC_DISABLE_MASK_AV1 (1 << 0) #define VCN_CODEC_DISABLE_MASK_VP9 (1 << 1) @@ -357,7 +358,9 @@ struct amdgpu_vcn_decode_buffer { uint32_t valid_buf_flag; uint32_t msg_buffer_address_hi; uint32_t msg_buffer_address_lo; - uint32_t pad[30]; + uint32_t session_context_buffer_address_hi; + uint32_t session_context_buffer_address_lo; + uint32_t pad[28]; }; #define VCN_BLOCK_ENCODE_DISABLE_MASK 0x80