[PATCH v3] drm/amdgpu:update kernel vcn ring test

[Christian König]

Am 06.07.23 um 14:49 schrieb Saleemkhan Jamadar:
> add session context buffer to decoder ring test.
>
> 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 at amd.com>
> Acked-by: Leo Liu <leo.liu at amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 13 +++++++++++--
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h |  3 +++
>   2 files changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> index 2d94f1b63bd6..15627fd94fbc 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> @@ -573,7 +573,7 @@ static int amdgpu_vcn_dec_get_create_msg(struct amdgpu_ring *ring, uint32_t hand
>   	int r, i;
>   
>   	memset(ib, 0, sizeof(*ib));
> -	r = amdgpu_ib_get(adev, NULL, AMDGPU_GPU_PAGE_SIZE * 2,
> +	r = amdgpu_ib_get(adev, NULL, AMDGPU_GPU_PAGE_SIZE * 34,
>   			AMDGPU_IB_POOL_DIRECT,
>   			ib);
>   	if (r)
> @@ -608,7 +608,7 @@ static int amdgpu_vcn_dec_get_destroy_msg(struct amdgpu_ring *ring, uint32_t han
>   	int r, i;
>   
>   	memset(ib, 0, sizeof(*ib));
> -	r = amdgpu_ib_get(adev, NULL, AMDGPU_GPU_PAGE_SIZE * 2,
> +	r = amdgpu_ib_get(adev, NULL, AMDGPU_GPU_PAGE_SIZE * 34,
>   			AMDGPU_IB_POOL_DIRECT,
>   			ib);

A comment or some define which explains how this size came to be would 
be nice to have.

>   	if (r)
> @@ -700,6 +700,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 +731,14 @@ 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));
>   
> +	if (session_ctx_buf_gaddr) {

This if can be dropped now. The address is always true.

> +		decode_buffer->valid_buf_flag |=
> +					cpu_to_le32(AMDGPU_VCN_CMD_FLAG_SESSION_CONTEXT_BUFFER);
> +		decode_buffer->session_context_buffer_address_hi =
> +							cpu_to_le32(session_ctx_buf_gaddr >> 32);
> +		decode_buffer->session_context_buffer_address_lo =
> +							cpu_to_le32(session_ctx_buf_gaddr);

We have upper_32_bits() and lower_32_bits() macros for that.

> +	}
>   	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.h
> index f1397ef66fd7..06f9ee91a1e1 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,6 +358,8 @@ struct amdgpu_vcn_decode_buffer {
>   	uint32_t valid_buf_flag;
>   	uint32_t msg_buffer_address_hi;
>   	uint32_t msg_buffer_address_lo;
> +	unsigned int session_context_buffer_address_hi;
> +	unsigned int session_context_buffer_address_lo;
>   	uint32_t pad[30];

Please use uint32_t instead of just unsigned int and I think you need to 
reduce the number of pad fields at the end.

Apart from that looks good to me,
Christian.

>   };
>