[PATCH v2] drm/amdgpu: add HW_IP_VCN_UNIFIED type

[Christian König]

Am 15.07.22 um 17:25 schrieb Dong, Ruijing:
> [AMD Official Use Only - General]
>
>>> Why exactly do we need a new define for this? Essentially the encode queue is extended with new functionality, isn't it?
>>> So I think we should just stick to AMDGPU_HW_IP_VCN_ENC and not add an alias for it.
> Yes, it extended the encode queue to include new functionality, and that looks little confused when send
> decoding jobs to the encoding queue. Then I assume this bias can reduce the confusion.
>
> Does this change make sense in this regard? certainly we can stick to AMDGPU_HW_IP_VCN_ENC.

I'm a bit on the edge with that.

On the one hand I agree with you that using AMDGPU_HW_IP_VCN_ENC for 
decoding is then a bit confusing, but on the other hand adding another 
enum with the same value as AMDGPU_HW_IP_VCN_ENC might be even more 
confusing.

I think the best middle way would be to at least add a comment 
explaining what's going on.

Regards,
Christian.

>
> Thanks,
> Ruijing
>
> -----Original Message-----
> From: Koenig, Christian <Christian.Koenig at amd.com>
> Sent: Friday, July 15, 2022 11:18 AM
> To: Dong, Ruijing <Ruijing.Dong at amd.com>; amd-gfx at lists.freedesktop.org
> Cc: Deucher, Alexander <Alexander.Deucher at amd.com>; Liu, Leo <Leo.Liu at amd.com>
> Subject: Re: [PATCH v2] drm/amdgpu: add HW_IP_VCN_UNIFIED type
>
> Am 15.07.22 um 16:44 schrieb Ruijing Dong:
>> Define HW_IP_VCN_UNIFIED type the same as HW_IP_VCN_ENC.
>>
>> VCN4 support for libdrm needs a new definition for the unified queue,
>> so that it can align to the kernel.
>>
>> link:
>> https://gitlab.freedesktop.org/mesa/drm/-/merge_requests/245/commits
>>
>> Signed-off-by: Ruijing Dong <ruijing.dong at amd.com>
>> ---
>>    include/uapi/drm/amdgpu_drm.h | 1 +
>>    1 file changed, 1 insertion(+)
>>
>> diff --git a/include/uapi/drm/amdgpu_drm.h
>> b/include/uapi/drm/amdgpu_drm.h index 18d3246d636e..fe33db8441bc
>> 100644
>> --- a/include/uapi/drm/amdgpu_drm.h
>> +++ b/include/uapi/drm/amdgpu_drm.h
>> @@ -560,6 +560,7 @@ 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
>> +#define AMDGPU_HW_IP_VCN_UNIFIED  AMDGPU_HW_IP_VCN_ENC
> Why exactly do we need a new define for this? Essentially the encode queue is extended with new functionality, isn't it?
>
> So I think we should just stick to AMDGPU_HW_IP_VCN_ENC and not add an alias for it.
>
> Regards,
> Christian.
>
>>    #define AMDGPU_HW_IP_VCN_JPEG     8
>>    #define AMDGPU_HW_IP_NUM          9
>>