Re: [PATCH v4] drm/amdgpu: add HW_IP_VCN_UNIFIED type

[Leo Liu <leo.liu@amd.com>]

On 2022-07-18 02:57, Christian König wrote:
> Am 15.07.22 um 22:04 schrieb Ruijing Dong:
>>  From VCN4, AMDGPU_HW_IP_VCN_UNIFIED is used to support
>> both encoding and decoding jobs, it re-uses the same
>> queue number of AMDGPU_HW_IP_VCN_ENC.
>>
>> link: 
>> https://gitlab.freedesktop.org/mesa/drm/-/merge_requests/245/commits
>>
>> Signed-off-by: Ruijing Dong <ruijing.dong@amd.com>
>> ---
>>   include/uapi/drm/amdgpu_drm.h | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/include/uapi/drm/amdgpu_drm.h 
>> b/include/uapi/drm/amdgpu_drm.h
>> index 18d3246d636e..e268cd3cdb12 100644
>> --- a/include/uapi/drm/amdgpu_drm.h
>> +++ b/include/uapi/drm/amdgpu_drm.h
>> @@ -560,6 +560,12 @@ struct drm_amdgpu_gem_va {
>>   #define AMDGPU_HW_IP_UVD_ENC      5
>>   #define AMDGPU_HW_IP_VCN_DEC      6
>>   #define AMDGPU_HW_IP_VCN_ENC      7
>> +/**
>
> Please don't use "/**" here, that is badly formated for a kerneldoc 
> comment.
>
>> + * From VCN4, AMDGPU_HW_IP_VCN_UNIFIED is used to support
>> + * both encoding and decoding jobs, it re-uses the same
>> + * queue number of AMDGPU_HW_IP_VCN_ENC.
>> + */
>> +#define AMDGPU_HW_IP_VCN_UNIFIED  AMDGPU_HW_IP_VCN_ENC
>
> I'm still in doubt that adding another define with the same value as 
> AMDGPU_HW_IP_VCN_ENC is a good idea.

Hi Christian,

 From VCN4, there is no VCN dec and enc ring type any more, the 
decode/encode will go through the unified queue, so using 
AMDGPU_HW_IP_VCN_ENC is no longer accurate . Keeping 
AMDGPU_HW_IP_VCN_ENC type is for legacy HW, and the new 
AMDGPU_HW_IP_VCN_UNIFIED just happen to use the same HW ring as legacy 
encode ring, so reuse the value, and that is the whole idea.

Thanks,

Leo


>
>
> Instead we should just add the comment to AMDGPU_HW_IP_VCN_ENC.
>
> Regards,
> Christian.
>
>>   #define AMDGPU_HW_IP_VCN_JPEG     8
>>   #define AMDGPU_HW_IP_NUM          9
>