Re: [PATCH v2] drm/amdgpu: add HW_IP_VCN_UNIFIED type
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
Sent: Friday, July 15, 2022 11:18 AM
To: Dong, Ruijing ; amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander ; Liu, Leo
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
---
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
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
---
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
RE: [PATCH v2] drm/amdgpu: add HW_IP_VCN_UNIFIED type
[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.
Thanks,
Ruijing
-Original Message-
From: Koenig, Christian
Sent: Friday, July 15, 2022 11:18 AM
To: Dong, Ruijing ; amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander ; Liu, Leo
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
> ---
> 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
>
