Re: [PATCH v2] drm/amdgpu: IB test encode test package change for VCN5

[Christian König <ckoenig.leichtzumerken@xxxxxxxxx>]

Am 22.04.24 um 21:59 schrieb Sonny Jiang:
> From: Sonny Jiang <sonjiang@xxxxxxx>
>
> VCN5 session info package interface changed
>
> Signed-off-by: Sonny Jiang <sonny.jiang@xxxxxxx>

Mhm, in general we should push back on FW changes which makes stuff like 
that necessary. So what is the justification?

If the FW has a good justification for it then in theory we should 
create new hw generation specific functions. But copying the whole 
function for vcn_v5_0.c is overkill as well.

Regards,
Christian.

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 12 ++++++++++--
>   1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> index bb85772b1374..2bebdaaff533 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> @@ -851,6 +851,7 @@ static int amdgpu_vcn_enc_get_create_msg(struct amdgpu_ring *ring, uint32_t hand
>   					 struct amdgpu_ib *ib_msg,
>   					 struct dma_fence **fence)
>   {
> +	struct amdgpu_device *adev = ring->adev;
>   	unsigned int ib_size_dw = 16;
>   	struct amdgpu_job *job;
>   	struct amdgpu_ib *ib;
> @@ -882,7 +883,10 @@ static int amdgpu_vcn_enc_get_create_msg(struct amdgpu_ring *ring, uint32_t hand
>   	ib->ptr[ib->length_dw++] = handle;
>   	ib->ptr[ib->length_dw++] = upper_32_bits(addr);
>   	ib->ptr[ib->length_dw++] = addr;
> -	ib->ptr[ib->length_dw++] = 0x0000000b;
> +	if (amdgpu_ip_version(adev, UVD_HWIP, 0) < IP_VERSION(5, 0, 0))
> +		ib->ptr[ib->length_dw++] = 0x0000000b;
> +	else
> +		ib->ptr[ib->length_dw++] = 0x00000000;
>   
>   	ib->ptr[ib->length_dw++] = 0x00000014;
>   	ib->ptr[ib->length_dw++] = 0x00000002; /* task info */
> @@ -918,6 +922,7 @@ static int amdgpu_vcn_enc_get_destroy_msg(struct amdgpu_ring *ring, uint32_t han
>   					  struct amdgpu_ib *ib_msg,
>   					  struct dma_fence **fence)
>   {
> +	struct amdgpu_device *adev = ring->adev;
>   	unsigned int ib_size_dw = 16;
>   	struct amdgpu_job *job;
>   	struct amdgpu_ib *ib;
> @@ -949,7 +954,10 @@ static int amdgpu_vcn_enc_get_destroy_msg(struct amdgpu_ring *ring, uint32_t han
>   	ib->ptr[ib->length_dw++] = handle;
>   	ib->ptr[ib->length_dw++] = upper_32_bits(addr);
>   	ib->ptr[ib->length_dw++] = addr;
> -	ib->ptr[ib->length_dw++] = 0x0000000b;
> +	if (amdgpu_ip_version(adev, UVD_HWIP, 0) < IP_VERSION(5, 0, 0))
> +		ib->ptr[ib->length_dw++] = 0x0000000b;
> +	else
> +		ib->ptr[ib->length_dw++] = 0x00000000;
>   
>   	ib->ptr[ib->length_dw++] = 0x00000014;
>   	ib->ptr[ib->length_dw++] = 0x00000002;