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.koe...@amd.com>
Sent: Friday, July 15, 2022 11:18 AM
To: Dong, Ruijing <ruijing.d...@amd.com>; amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander <alexander.deuc...@amd.com>; Liu, Leo <leo....@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.d...@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


Reply via email to