RE: [PATCH] drm/amdgpu/soc21: update VCN 4 max HEVC encoding resolution

[Dong, Ruijing]

[AMD Official Use Only - General]

Reviewed-by: Ruijing Dong 

Thanks,
Ruijing

-Original Message-
From: amd-gfx  On Behalf Of Thong
Sent: Tuesday, February 6, 2024 6:28 PM
To: amd-gfx@lists.freedesktop.org
Cc: Thai, Thong 
Subject: [PATCH] drm/amdgpu/soc21: update VCN 4 max HEVC encoding resolution

Update the maximum resolution reported for HEVC encoding on VCN 4 devices to 
reflect its 8K encoding capability.

Signed-off-by: Thong 
---
 drivers/gpu/drm/amd/amdgpu/soc21.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/soc21.c 
b/drivers/gpu/drm/amd/amdgpu/soc21.c
index 48c6efcdeac9..4d7188912edf 100644
--- a/drivers/gpu/drm/amd/amdgpu/soc21.c
+++ b/drivers/gpu/drm/amd/amdgpu/soc21.c
@@ -50,13 +50,13 @@ static const struct amd_ip_funcs soc21_common_ip_funcs;
 /* SOC21 */
 static const struct amdgpu_video_codec_info 
vcn_4_0_0_video_codecs_encode_array_vcn0[] = {
{codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_MPEG4_AVC, 4096, 
2304, 0)},
-   {codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_HEVC, 4096, 2304, 
0)},
+   {codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_HEVC, 8192, 4352,
+0)},
{codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_AV1, 8192, 4352, 
0)},  };

 static const struct amdgpu_video_codec_info 
vcn_4_0_0_video_codecs_encode_array_vcn1[] = {
{codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_MPEG4_AVC, 4096, 
2304, 0)},
-   {codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_HEVC, 4096, 2304, 
0)},
+   {codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_HEVC, 8192, 4352,
+0)},
 };

 static const struct amdgpu_video_codecs vcn_4_0_0_video_codecs_encode_vcn0 = {
--
2.34.1

RE: [PATCH 2/2] drm/amdgpu/vcn: not pause dpg for unified queue

[Dong, Ruijing]

[AMD Official Use Only - AMD Internal Distribution Only]

Just change the commit messages from "For previous generations" to " For VCN3 
and before" to be more specific.

With that all patches are
Reviewed-by: Ruijing Dong 

Thanks,
Ruijing

-Original Message-
From: amd-gfx  On Behalf Of 
boyuan.zh...@amd.com
Sent: Wednesday, July 10, 2024 4:30 PM
To: amd-gfx@lists.freedesktop.org
Cc: Zhang, Boyuan ; Deucher, Alexander 

Subject: [PATCH 2/2] drm/amdgpu/vcn: not pause dpg for unified queue

From: Boyuan Zhang 

For unified queue, DPG pause for encoding is done inside VCN firmware, so there 
is no need to pause dpg based on ring type in kernel.

For previous generations, pausing DPG for encoding in kernel is still needed.

v2: add more comments

Signed-off-by: Boyuan Zhang 
Acked-by: Alex Deucher 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 14 +++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
index f59e81be885d..00f3ac5f4572 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
@@ -412,7 +412,9 @@ static void amdgpu_vcn_idle_work_handler(struct work_struct 
*work)
for (i = 0; i < adev->vcn.num_enc_rings; ++i)
fence[j] += 
amdgpu_fence_count_emitted(&adev->vcn.inst[j].ring_enc[i]);

-   if (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG){
+   /* Only set DPG pause for VCN3 or below, VCN4 and above will be 
handled by FW */
+   if (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG &&
+   !adev->vcn.using_unified_queue) {
struct dpg_pause_state new_state;

if (fence[j] ||
@@ -458,7 +460,9 @@ void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring)
amdgpu_device_ip_set_powergating_state(adev, AMD_IP_BLOCK_TYPE_VCN,
   AMD_PG_STATE_UNGATE);

-   if (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG){
+   /* Only set DPG pause for VCN3 or below, VCN4 and above will be handled 
by FW */
+   if (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG &&
+   !adev->vcn.using_unified_queue) {
struct dpg_pause_state new_state;

if (ring->funcs->type == AMDGPU_RING_TYPE_VCN_ENC) { @@ -484,8 
+488,12 @@ void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring)

 void amdgpu_vcn_ring_end_use(struct amdgpu_ring *ring)  {
+   struct amdgpu_device *adev = ring->adev;
+
+   /* Only set DPG pause for VCN3 or below, VCN4 and above will be
+handled by FW */
if (ring->adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG &&
-   ring->funcs->type == AMDGPU_RING_TYPE_VCN_ENC)
+   ring->funcs->type == AMDGPU_RING_TYPE_VCN_ENC &&
+   !adev->vcn.using_unified_queue)

atomic_dec(&ring->adev->vcn.inst[ring->me].dpg_enc_submission_cnt);

atomic_dec(&ring->adev->vcn.total_submission_cnt);
--
2.34.1

RE: [PATCH] drm/amdgpu: Update kmd_fw_shared for VCN5

[Dong, Ruijing]

[AMD Official Use Only - AMD Internal Distribution Only]

Hi Yinjie,

Please join the email group from
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

After signed up successfully, please resend this email with

Reviewed-by: Ruijing Dong 

Thanks,
Ruijing

-Original Message-
From: Yao, Yinjie 
Sent: Tuesday, August 6, 2024 2:03 PM
To: amd-gfx@lists.freedesktop.org; Koenig, Christian 
Cc: Deucher, Alexander ; Liu, Leo ; 
Dong, Ruijing ; Yao, Yinjie 
Subject: [PATCH] drm/amdgpu: Update kmd_fw_shared for VCN5

kmd_fw_shared changed in VCN5

Signed-off-by: Yinjie Yao 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
index f127eccf59d7..2a1f3dbb14d3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
@@ -464,8 +464,11 @@ struct amdgpu_vcn5_fw_shared {
struct amdgpu_fw_shared_unified_queue_struct sq;
uint8_t pad1[8];
struct amdgpu_fw_shared_fw_logging fw_log;
+   uint8_t pad2[20];
struct amdgpu_fw_shared_rb_setup rb_setup;
-   uint8_t pad2[4];
+   struct amdgpu_fw_shared_smu_interface_info smu_dpm_interface;
+   struct amdgpu_fw_shared_drm_key_wa drm_key_wa;
+   uint8_t pad3[9];
 };

 #define VCN_BLOCK_ENCODE_DISABLE_MASK 0x80
--
2.34.1

RE: [PATCH] drm/amdgpu: Update kmd_fw_shared for VCN5

[Dong, Ruijing]

[AMD Official Use Only - AMD Internal Distribution Only]

Hi Alex,

Previously we used packed and found out it didn't match with FW interface, and 
the one without packed working as it matches with
FW interface, this sounds tricky though.

Thanks,
Ruijing

-Original Message-
From: Alex Deucher 
Sent: Tuesday, August 6, 2024 3:45 PM
To: Yao, Yinjie 
Cc: amd-gfx@lists.freedesktop.org; Koenig, Christian 
; Deucher, Alexander ; 
Liu, Leo ; Dong, Ruijing 
Subject: Re: [PATCH] drm/amdgpu: Update kmd_fw_shared for VCN5

On Tue, Aug 6, 2024 at 3:38 PM yinjiyao  wrote:
>
> kmd_fw_shared changed in VCN5
>
> Signed-off-by: Yinjie Yao 
> Reviewed-by: Ruijing Dong 

Acked-by: Alex Deucher 

BTW, should these structures be marked as packed?  I suspect they should?

Alex


> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> index f127eccf59d7..2a1f3dbb14d3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> @@ -464,8 +464,11 @@ struct amdgpu_vcn5_fw_shared {
> struct amdgpu_fw_shared_unified_queue_struct sq;
> uint8_t pad1[8];
> struct amdgpu_fw_shared_fw_logging fw_log;
> +   uint8_t pad2[20];
> struct amdgpu_fw_shared_rb_setup rb_setup;
> -   uint8_t pad2[4];
> +   struct amdgpu_fw_shared_smu_interface_info smu_dpm_interface;
> +   struct amdgpu_fw_shared_drm_key_wa drm_key_wa;
> +   uint8_t pad3[9];
>  };
>
>  #define VCN_BLOCK_ENCODE_DISABLE_MASK 0x80
> --
> 2.34.1
>

RE: [PATCH] drm/amdgpu/sdma5.2: limit wptr workaround to sdma 5.2.1

[Dong, Ruijing]

[AMD Official Use Only - AMD Internal Distribution Only]

Acked-by: Ruijing Dong 

-Original Message-
From: amd-gfx  On Behalf Of Alex Deucher
Sent: Wednesday, August 14, 2024 10:43 AM
To: amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander 
Subject: [PATCH] drm/amdgpu/sdma5.2: limit wptr workaround to sdma 5.2.1

The workaround seems to cause stability issues on other SDMA 5.2.x IPs.

Fixes: a03ebf116303 ("drm/amdgpu/sdma5.2: Update wptr registers as well as 
doorbell")
Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/3556
Signed-off-by: Alex Deucher 
---
 drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c | 18 ++
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c 
b/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
index d740255edf5a..bc9b240a3488 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
@@ -225,14 +225,16 @@ static void sdma_v5_2_ring_set_wptr(struct amdgpu_ring 
*ring)
DRM_DEBUG("calling WDOORBELL64(0x%08x, 0x%016llx)\n",
ring->doorbell_index, ring->wptr << 2);
WDOORBELL64(ring->doorbell_index, ring->wptr << 2);
-   /* SDMA seems to miss doorbells sometimes when powergating 
kicks in.
-* Updating the wptr directly will wake it. This is only safe 
because
-* we disallow gfxoff in begin_use() and then allow it again in 
end_use().
-*/
-   WREG32(sdma_v5_2_get_reg_offset(adev, ring->me, 
mmSDMA0_GFX_RB_WPTR),
-  lower_32_bits(ring->wptr << 2));
-   WREG32(sdma_v5_2_get_reg_offset(adev, ring->me, 
mmSDMA0_GFX_RB_WPTR_HI),
-  upper_32_bits(ring->wptr << 2));
+   if (amdgpu_ip_version(adev, SDMA0_HWIP, 0) == IP_VERSION(5, 2, 
1)) {
+   /* SDMA seems to miss doorbells sometimes when 
powergating kicks in.
+* Updating the wptr directly will wake it. This is 
only safe because
+* we disallow gfxoff in begin_use() and then allow it 
again in end_use().
+*/
+   WREG32(sdma_v5_2_get_reg_offset(adev, ring->me, 
mmSDMA0_GFX_RB_WPTR),
+  lower_32_bits(ring->wptr << 2));
+   WREG32(sdma_v5_2_get_reg_offset(adev, ring->me, 
mmSDMA0_GFX_RB_WPTR_HI),
+  upper_32_bits(ring->wptr << 2));
+   }
} else {
DRM_DEBUG("Not using doorbell -- "
"mmSDMA%i_GFX_RB_WPTR == 0x%08x "
--
2.46.0

RE: [PATCH] drm/amdgpu: update fw_share for VCN5

[Dong, Ruijing]

[AMD Official Use Only - General]

Reviewed-by: Ruijing Dong 

Thanks,
Ruijing

-Original Message-
From: amd-gfx  On Behalf Of Sonny Jiang
Sent: Tuesday, April 23, 2024 2:41 PM
To: amd-gfx@lists.freedesktop.org
Cc: Jiang, Sonny 
Subject: [PATCH] drm/amdgpu: update fw_share for VCN5

kmd_fw_shared changed in VCN5

Signed-off-by: Sonny Jiang 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c |  5 -  
drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h | 10 ++  
drivers/gpu/drm/amd/amdgpu/vcn_v5_0_0.c | 14 +++---
 3 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
index 2bebdaaff533..9ea341b76165 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
@@ -185,7 +185,10 @@ int amdgpu_vcn_sw_init(struct amdgpu_device *adev)
if (adev->firmware.load_type != AMDGPU_FW_LOAD_PSP)
bo_size += 
AMDGPU_GPU_PAGE_ALIGN(le32_to_cpu(hdr->ucode_size_bytes) + 8);

-   if (amdgpu_ip_version(adev, UVD_HWIP, 0) >= IP_VERSION(4, 0, 0)) {
+   if (amdgpu_ip_version(adev, UVD_HWIP, 0) >= IP_VERSION(5, 0, 0)) {
+   fw_shared_size = AMDGPU_GPU_PAGE_ALIGN(sizeof(struct 
amdgpu_vcn5_fw_shared));
+   log_offset = offsetof(struct amdgpu_vcn5_fw_shared, fw_log);
+   } else if (amdgpu_ip_version(adev, UVD_HWIP, 0) >= IP_VERSION(4, 0,
+0)) {
fw_shared_size = AMDGPU_GPU_PAGE_ALIGN(sizeof(struct 
amdgpu_vcn4_fw_shared));
log_offset = offsetof(struct amdgpu_vcn4_fw_shared, fw_log);
} else {
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
index a418393d89ec..9f06def236fd 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
@@ -454,6 +454,16 @@ struct amdgpu_vcn_rb_metadata {
uint8_t pad[26];
 };

+struct amdgpu_vcn5_fw_shared {
+   uint32_t present_flag_0;
+   uint8_t pad[12];
+   struct amdgpu_fw_shared_unified_queue_struct sq;
+   uint8_t pad1[8];
+   struct amdgpu_fw_shared_fw_logging fw_log;
+   struct amdgpu_fw_shared_rb_setup rb_setup;
+   uint8_t pad2[4];
+};
+
 #define VCN_BLOCK_ENCODE_DISABLE_MASK 0x80  #define 
VCN_BLOCK_DECODE_DISABLE_MASK 0x40  #define VCN_BLOCK_QUEUE_DISABLE_MASK 0xC0 
diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v5_0_0.c 
b/drivers/gpu/drm/amd/amdgpu/vcn_v5_0_0.c
index b9455b6efa17..851975b5ce29 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v5_0_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v5_0_0.c
@@ -95,7 +95,7 @@ static int vcn_v5_0_0_sw_init(void *handle)
return r;

for (i = 0; i < adev->vcn.num_vcn_inst; i++) {
-   volatile struct amdgpu_vcn4_fw_shared *fw_shared;
+   volatile struct amdgpu_vcn5_fw_shared *fw_shared;

if (adev->vcn.harvest_config & (1 << i))
continue;
@@ -154,7 +154,7 @@ static int vcn_v5_0_0_sw_fini(void *handle)

if (drm_dev_enter(adev_to_drm(adev), &idx)) {
for (i = 0; i < adev->vcn.num_vcn_inst; i++) {
-   volatile struct amdgpu_vcn4_fw_shared *fw_shared;
+   volatile struct amdgpu_vcn5_fw_shared *fw_shared;

if (adev->vcn.harvest_config & (1 << i))
continue;
@@ -335,7 +335,7 @@ static void vcn_v5_0_0_mc_resume(struct amdgpu_device 
*adev, int inst)
upper_32_bits(adev->vcn.inst[inst].fw_shared.gpu_addr));
WREG32_SOC15(VCN, inst, regUVD_VCPU_NONCACHE_OFFSET0, 0);
WREG32_SOC15(VCN, inst, regUVD_VCPU_NONCACHE_SIZE0,
-   AMDGPU_GPU_PAGE_ALIGN(sizeof(struct amdgpu_vcn4_fw_shared)));
+   AMDGPU_GPU_PAGE_ALIGN(sizeof(struct amdgpu_vcn5_fw_shared)));
 }

 /**
@@ -439,7 +439,7 @@ static void vcn_v5_0_0_mc_resume_dpg_mode(struct 
amdgpu_device *adev, int inst_i
VCN, inst_idx, regUVD_VCPU_NONCACHE_OFFSET0), 0, 0, indirect);
WREG32_SOC24_DPG_MODE(inst_idx, SOC24_DPG_MODE_OFFSET(
VCN, inst_idx, regUVD_VCPU_NONCACHE_SIZE0),
-   AMDGPU_GPU_PAGE_ALIGN(sizeof(struct amdgpu_vcn4_fw_shared)), 0, 
indirect);
+   AMDGPU_GPU_PAGE_ALIGN(sizeof(struct amdgpu_vcn5_fw_shared)), 0,
+indirect);

/* VCN global tiling registers */
WREG32_SOC24_DPG_MODE(inst_idx, SOC24_DPG_MODE_OFFSET( @@ -616,7 +616,7 
@@ static void vcn_v5_0_0_enable_clock_gating(struct amdgpu_device *adev, int 
inst)
  */
 static int vcn_v5_0_0_start_dpg_mode(struct amdgpu_device *adev, int inst_idx, 
bool indirect)  {
-   volatile struct amdgpu_vcn4_fw_shared *fw_shared = 
adev->vcn.inst[inst_idx].fw_shared.cpu_addr;
+   volatile struct amdgpu_vcn5_fw_shared *fw_shared =
+adev->vcn.inst[inst_idx].fw_shared.cpu_addr;
struct amdgpu_ring *ring;
uint32_t tmp;

@@ -713,7 +713,7 @@ static int vcn_v5_0_0_start_dpg_mode(struct amd

RE: [PATCH v3] drm/amdgpu: IB test encode test package change for VCN5

[Dong, Ruijing]

[AMD Official Use Only - General]

Reviewed-by: Ruijing Dong ruijing.d...@amd.com

Thanks,
Ruijing

From: amd-gfx  On Behalf Of Jiang, Sonny
Sent: Monday, April 29, 2024 9:49 AM
To: amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH v3] drm/amdgpu: IB test encode test package change for VCN5


[AMD Official Use Only - General]


[AMD Official Use Only - General]

Ping.

Sonny

From: Jiang, Sonny mailto:sonny.ji...@amd.com>>
Sent: Thursday, April 25, 2024 4:12 PM
To: amd-gfx@lists.freedesktop.org 
mailto:amd-gfx@lists.freedesktop.org>>
Subject: Re: [PATCH v3] drm/amdgpu: IB test encode test package change for VCN5

By tests, I didn't find error on VCN1 to VCN4.

Thanks,
Sonny


From: Jiang, Sonny mailto:sonny.ji...@amd.com>>
Sent: Thursday, April 25, 2024 4:10 PM
To: amd-gfx@lists.freedesktop.org 
mailto:amd-gfx@lists.freedesktop.org>>
Cc: Jiang, Sonny mailto:sonny.ji...@amd.com>>; Jiang, 
Sonny mailto:sonny.ji...@amd.com>>
Subject: [PATCH v3] drm/amdgpu: IB test encode test package change for VCN5

From: Sonny Jiang mailto:sonji...@amd.com>>

VCN5 session info package interface changed

Signed-off-by: Sonny Jiang mailto:sonny.ji...@amd.com>>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
index 677eb141554e..b89605b400c0 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
@@ -885,7 +885,7 @@ 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++] = 0x000b;
+   ib->ptr[ib->length_dw++] = 0x;

 ib->ptr[ib->length_dw++] = 0x0014;
 ib->ptr[ib->length_dw++] = 0x0002; /* task info */
@@ -952,7 +952,7 @@ 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++] = 0x000b;
+   ib->ptr[ib->length_dw++] = 0x;

 ib->ptr[ib->length_dw++] = 0x0014;
 ib->ptr[ib->length_dw++] = 0x0002;
--
2.43.2

RE: [PATCH] drm/amdgpu: drop some kernel messages in VCN code

[Dong, Ruijing]

[AMD Official Use Only - AMD Internal Distribution Only]

Please see my question inline below.

Thanks,
Ruijing

-Original Message-
From: Wu, David 
Sent: Thursday, May 23, 2024 11:05 AM
To: amd-gfx@lists.freedesktop.org; Koenig, Christian 
Cc: Deucher, Alexander ; Liu, Leo ; 
Jiang, Sonny ; Dong, Ruijing 
Subject: [PATCH] drm/amdgpu: drop some kernel messages in VCN code

We have messages when the VCN fails to initialize and there is no need to 
report on success.
Also PSP loading FWs is the default for production.

Signed-off-by: David (Ming Qiang) Wu 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c  |  1 -  
drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_0.c |  3 ---  
drivers/gpu/drm/amd/amdgpu/vcn_v5_0_0.c  | 10 +-
 3 files changed, 1 insertion(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
index b89605b400c0..5e2b7c340724 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
@@ -1078,7 +1078,6 @@ void amdgpu_vcn_setup_ucode(struct amdgpu_device *adev)
IP_VERSION(4, 0, 3))
break;
}
-   dev_info(adev->dev, "Will use PSP to load VCN firmware\n");
}
 }

diff --git a/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_0.c 
b/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_0.c
index 64c856bfe0cb..68ef29bc70e2 100644
--- a/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_0.c
@@ -145,8 +145,6 @@ static int jpeg_v5_0_0_hw_init(void *handle)
if (r)
return r;

-   DRM_DEV_INFO(adev->dev, "JPEG decode initialized successfully.\n");
-
return 0;
 }

@@ -549,7 +547,6 @@ static const struct amdgpu_ring_funcs 
jpeg_v5_0_0_dec_ring_vm_funcs = {  static void 
jpeg_v5_0_0_set_dec_ring_funcs(struct amdgpu_device *adev)  {
adev->jpeg.inst->ring_dec->funcs = &jpeg_v5_0_0_dec_ring_vm_funcs;
-   DRM_DEV_INFO(adev->dev, "JPEG decode is enabled in VM mode\n");
 }

 static const struct amdgpu_irq_src_funcs jpeg_v5_0_0_irq_funcs = { diff --git 
a/drivers/gpu/drm/amd/amdgpu/vcn_v5_0_0.c 
b/drivers/gpu/drm/amd/amdgpu/vcn_v5_0_0.c
index 36d4ca645c56..070b56610c7d 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v5_0_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v5_0_0.c
@@ -200,16 +200,10 @@ static int vcn_v5_0_0_hw_init(void *handle)

r = amdgpu_ring_test_helper(ring);
if (r)
-   goto done;
+   return r;
}
 [Ruijing] Are we assuming the hw init process always be successful?
return 0;
-done:
-   if (!r)
-   DRM_INFO("VCN decode and encode initialized successfully(under 
%s).\n",
-   (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG)?"DPG 
Mode":"SPG Mode");
-
-   return r;
 }

 /**
@@ -1122,8 +1116,6 @@ static void vcn_v5_0_0_set_unified_ring_funcs(struct 
amdgpu_device *adev)

adev->vcn.inst[i].ring_enc[0].funcs = 
&vcn_v5_0_0_unified_ring_vm_funcs;
adev->vcn.inst[i].ring_enc[0].me = i;
-
-   DRM_INFO("VCN(%d) encode/decode are enabled in VM mode\n", i);
}
 }

--
2.34.1

RE: [PATCH] drm/amdgpu: drop some kernel messages in VCN code

[Dong, Ruijing]

[AMD Official Use Only - AMD Internal Distribution Only]

Thanks for the response, and it looks good to me.

Ruijing

-Original Message-
From: Wu, David 
Sent: Thursday, May 23, 2024 12:55 PM
To: Dong, Ruijing ; Wu, David ; 
amd-gfx@lists.freedesktop.org; Koenig, Christian 
Cc: Deucher, Alexander ; Liu, Leo ; 
Jiang, Sonny 
Subject: Re: [PATCH] drm/amdgpu: drop some kernel messages in VCN code

please see in line.

On 2024-05-23 12:02, Dong, Ruijing wrote:
> [AMD Official Use Only - AMD Internal Distribution Only]
>
> Please see my question inline below.
>
> Thanks,
> Ruijing
>
> -Original Message-
> From: Wu, David 
> Sent: Thursday, May 23, 2024 11:05 AM
> To: amd-gfx@lists.freedesktop.org; Koenig, Christian
> 
> Cc: Deucher, Alexander ; Liu, Leo
> ; Jiang, Sonny ; Dong, Ruijing
> 
> Subject: [PATCH] drm/amdgpu: drop some kernel messages in VCN code
>
> We have messages when the VCN fails to initialize and there is no need to 
> report on success.
> Also PSP loading FWs is the default for production.
>
> Signed-off-by: David (Ming Qiang) Wu 
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c  |  1 -  
> drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_0.c |  3 ---  
> drivers/gpu/drm/amd/amdgpu/vcn_v5_0_0.c  | 10 +-
>   3 files changed, 1 insertion(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> index b89605b400c0..5e2b7c340724 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> @@ -1078,7 +1078,6 @@ void amdgpu_vcn_setup_ucode(struct amdgpu_device *adev)
>  IP_VERSION(4, 0, 3))
>  break;
>  }
> -   dev_info(adev->dev, "Will use PSP to load VCN firmware\n");
>  }
>   }
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_0.c
> b/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_0.c
> index 64c856bfe0cb..68ef29bc70e2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_0.c
> @@ -145,8 +145,6 @@ static int jpeg_v5_0_0_hw_init(void *handle)
>  if (r)
>  return r;
>
> -   DRM_DEV_INFO(adev->dev, "JPEG decode initialized successfully.\n");
> -
>  return 0;
>   }
>
> @@ -549,7 +547,6 @@ static const struct amdgpu_ring_funcs 
> jpeg_v5_0_0_dec_ring_vm_funcs = {  static void 
> jpeg_v5_0_0_set_dec_ring_funcs(struct amdgpu_device *adev)  {
>  adev->jpeg.inst->ring_dec->funcs = &jpeg_v5_0_0_dec_ring_vm_funcs;
> -   DRM_DEV_INFO(adev->dev, "JPEG decode is enabled in VM mode\n");
>   }
>
>   static const struct amdgpu_irq_src_funcs jpeg_v5_0_0_irq_funcs = {
> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v5_0_0.c
> b/drivers/gpu/drm/amd/amdgpu/vcn_v5_0_0.c
> index 36d4ca645c56..070b56610c7d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v5_0_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v5_0_0.c
> @@ -200,16 +200,10 @@ static int vcn_v5_0_0_hw_init(void *handle)
>
>  r = amdgpu_ring_test_helper(ring);
>  if (r)
> -   goto done;
> +   return r;
>  }
>   [Ruijing] Are we assuming the hw init process always be successful?

No - it could fail with errors and in this case the top level will report 
error. Otherwise it will succeed(and no need to report successful message).

David

>  return 0;
> -done:
> -   if (!r)
> -   DRM_INFO("VCN decode and encode initialized 
> successfully(under %s).\n",
> -   (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG)?"DPG 
> Mode":"SPG Mode");
> -
> -   return r;
>   }
>
>   /**
> @@ -1122,8 +1116,6 @@ static void
> vcn_v5_0_0_set_unified_ring_funcs(struct amdgpu_device *adev)
>
>  adev->vcn.inst[i].ring_enc[0].funcs = 
> &vcn_v5_0_0_unified_ring_vm_funcs;
>  adev->vcn.inst[i].ring_enc[0].me = i;
> -
> -   DRM_INFO("VCN(%d) encode/decode are enabled in VM mode\n", i);
>  }
>   }
>
> --
> 2.34.1
>

RE: [PATCH] drm/amdgpu: align between libdrm and drm api

[Dong, Ruijing]

[AMD Official Use Only - General]

Hi Christian,

You are right, when process the libdrm code review (not committed yet), we 
realized the corresponding file needs to align to the kernel.
So we will need to have this header file changed first, then to process libdrm 
code again.

Thanks,
Ruijing

-Original Message-
From: Christian König 
Sent: Friday, July 15, 2022 4:41 AM
To: Dong, Ruijing ; amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander ; Liu, Leo ; 
Koenig, Christian 
Subject: Re: [PATCH] drm/amdgpu: align between libdrm and drm api

Am 14.07.22 um 23:22 schrieb Ruijing Dong:
> define HW_IP_VCN_UNIFIED the same as HW_IP_VCN_ENC

Usually that should be the other way around, libdrm aligns to the kernel.

Why was that modification committed to libdrm first? There are usually plenty 
of warnings before we can do that.

Regards,
Christian.

>
> 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
>   #define AMDGPU_HW_IP_VCN_JPEG 8
>   #define AMDGPU_HW_IP_NUM  9
>

RE: [PATCH v2] drm/amdgpu: add HW_IP_VCN_UNIFIED type

[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.

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 v4] drm/amdgpu: add HW_IP_VCN_UNIFIED type

[Dong, Ruijing]

[AMD Official Use Only - General]

>> What happened is that the encode ring was extended with decode 
>> functionality. In other words we still use the same format for encoding, we 
>> just added another one for decoding as well.

Just to clarify the format difference between legacy encoding and unified 
queue, we can do either way in the code.

Unified queue requires a different format. The original encoding/decoding 
format cannot be used in unified queue, which requires to have two new headers, 
"engine info" to indicate the coming IB package type, encoding or decoding; and 
the "signature header" to guarantee the coming IB package's integrity, if this 
failed, the whole IB package will be discarded by VCN FW. That is why I was 
thinking to have a bias of AMDGPU_HW_IP_VCN_ENC could be better in the future.

Thanks,
Ruijing

-Original Message-
From: Koenig, Christian 
Sent: Monday, July 18, 2022 9:54 AM
To: Liu, Leo ; Dong, Ruijing ; 
amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander 
Subject: Re: [PATCH v4] drm/amdgpu: add HW_IP_VCN_UNIFIED type

Am 18.07.22 um 15:48 schrieb Leo Liu:
>
> 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 
>>> ---
>>>   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.

Yeah, I understand your reasoning I just don't see it this way.

What happened is that the encode ring was extended with decode functionality. 
In other words we still use the same format for encoding, we just added another 
one for decoding as well.

Renaming the enum and adding AMDGPU_HW_IP_VCN_UNIFIED suggests that this is 
something completely new, which is not the case here. The encoding commands 
stay the same, don't they?

So to sum it up my suggestion is to stick with AMDGPU_HW_IP_VCN_ENC and just 
document on the definition that this is used for both encode as well as decode 
starting with VCN4.

Regards,
Christian.

>
> 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
>>

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

[Dong, Ruijing]

[AMD Official Use Only - General]

No, we don't plan to clone another one.  I will modify the comment only and 
remove the bias.

Thanks
Ruijing

-Original Message-
From: Koenig, Christian 
Sent: Monday, July 18, 2022 10:37 AM
To: Dong, Ruijing ; Liu, Leo ; 
amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander 
Subject: Re: [PATCH v4] drm/amdgpu: add HW_IP_VCN_UNIFIED type

Am 18.07.22 um 16:14 schrieb Dong, Ruijing:
> [AMD Official Use Only - General]
>
>>> What happened is that the encode ring was extended with decode 
>>> functionality. In other words we still use the same format for encoding, we 
>>> just added another one for decoding as well.
> Just to clarify the format difference between legacy encoding and unified 
> queue, we can do either way in the code.
>
> Unified queue requires a different format. The original encoding/decoding 
> format cannot be used in unified queue, which requires to have two new 
> headers, "engine info" to indicate the coming IB package type, encoding or 
> decoding; and the "signature header" to guarantee the coming IB package's 
> integrity, if this failed, the whole IB package will be discarded by VCN FW. 
> That is why I was thinking to have a bias of AMDGPU_HW_IP_VCN_ENC could be 
> better in the future.

Yeah, but the remaining packet format is the same. And as far as I know you 
guys don't plan do clone that for some reason, don't you?

Adding another name for the same enum value can potentially be much more 
confusing than the fact that the encode ring now takes a bunch of more commands 
to do both encoding and decoding.

Regards,
Christian.

>
> Thanks,
> Ruijing
>
> -Original Message-
> From: Koenig, Christian 
> Sent: Monday, July 18, 2022 9:54 AM
> To: Liu, Leo ; Dong, Ruijing ;
> amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander 
> Subject: Re: [PATCH v4] drm/amdgpu: add HW_IP_VCN_UNIFIED type
>
> Am 18.07.22 um 15:48 schrieb Leo Liu:
>> 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/commit
>>>> s
>>>>
>>>> Signed-off-by: Ruijing Dong 
>>>> ---
>>>>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.
> Yeah, I understand your reasoning I just don't see it this way.
>
> What happened is that the encode ring was extended with decode functionality. 
> In other words we still use the same format for encoding, we just added 
> another one for decoding as well.
>
> Renaming the enum and adding AMDGPU_HW_IP_VCN_UNIFIED suggests that this is 
> something completely new, which is not the case here. The encoding commands 
> stay the same, don't they?
>
> So to sum it up my suggestion is to stick with AMDGPU_HW_IP_VCN_ENC and just 
> document on the definition that this is used for both encode as well as 
> decode starting with VCN4.
>
> Regards,
> Christian.
>
>> 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

RE: [PATCH linux-next] drm/amdgpu/vcn: Remove unused assignment in vcn_v4_0_stop

[Dong, Ruijing]

[AMD Official Use Only - General]

Sorry, which "r" value was overwritten?  I didn't see the point of making this 
change.

Thanks
Ruijing

-Original Message-
From: Khalid Masum 
Sent: Monday, August 15, 2022 3:01 AM
To: amd-gfx@lists.freedesktop.org; dri-de...@lists.freedesktop.org; 
linux-ker...@vger.kernel.org; linux-kernel-ment...@lists.linuxfoundation.org
Cc: Deucher, Alexander ; Koenig, Christian 
; Pan, Xinhui ; David Airlie 
; Daniel Vetter ; Zhu, James 
; Jiang, Sonny ; Dong, Ruijing 
; Wan Jiabing ; Liu, Leo 
; Khalid Masum 
Subject: [PATCH linux-next] drm/amdgpu/vcn: Remove unused assignment in 
vcn_v4_0_stop

The value assigned from vcn_v4_0_stop_dbg_mode to r is overwritten before it 
can be used. Remove this assignment.

Addresses-Coverity: 1504988 ("Unused value")
Fixes: 8da1170a16e4 ("drm/amdgpu: add VCN4 ip block support")
Signed-off-by: Khalid Masum 
---
 drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c 
b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
index ca14c3ef742e..80b8a2c66b36 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
@@ -1154,7 +1154,7 @@ static int vcn_v4_0_stop(struct amdgpu_device *adev)
fw_shared->sq.queue_mode |= FW_QUEUE_DPG_HOLD_OFF;

if (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG) {
-   r = vcn_v4_0_stop_dpg_mode(adev, i);
+   vcn_v4_0_stop_dpg_mode(adev, i);
continue;
}

--
2.37.1

RE: [PATCH linux-next] drm/amdgpu/vcn: Remove unused assignment in vcn_v4_0_stop

[Dong, Ruijing]

[AMD Official Use Only - General]

If the condition was met and it came to execute vcn_4_0_stop_dpg_mode, then it 
would never have a chance to go for /*wait for vcn idle*/, isn't it?
I still didn't see obvious purpose of this change.

if (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG) {
 ==>  r = vcn_v4_0_stop_dpg_mode(adev, i);
 continue;
 }

 /* wait for vcn idle */
 r = SOC15_WAIT_ON_RREG(VCN, i, regUVD_STATUS, 
UVD_STATUS__IDLE, 0x7);

Thanks
Ruijing

-Original Message-
From: Khalid Masum 
Sent: Monday, August 15, 2022 11:11 AM
To: Dong, Ruijing ; amd-gfx@lists.freedesktop.org; 
dri-de...@lists.freedesktop.org; linux-ker...@vger.kernel.org; 
linux-kernel-ment...@lists.linuxfoundation.org
Cc: Deucher, Alexander ; Koenig, Christian 
; Pan, Xinhui ; David Airlie 
; Daniel Vetter ; Zhu, James 
; Jiang, Sonny ; Wan Jiabing 
; Liu, Leo 
Subject: Re: [PATCH linux-next] drm/amdgpu/vcn: Remove unused assignment in 
vcn_v4_0_stop

On 8/15/22 20:15, Dong, Ruijing wrote:
> [AMD Official Use Only - General]
>
> Sorry, which "r" value was overwritten?  I didn't see the point of making 
> this change.
>
> Thanks
> Ruijing
>
> -Original Message-
> From: Khalid Masum 
> Sent: Monday, August 15, 2022 3:01 AM
> To: amd-gfx@lists.freedesktop.org; dri-de...@lists.freedesktop.org;
> linux-ker...@vger.kernel.org;
> linux-kernel-ment...@lists.linuxfoundation.org
> Cc: Deucher, Alexander ; Koenig, Christian
> ; Pan, Xinhui ; David
> Airlie ; Daniel Vetter ; Zhu, James
> ; Jiang, Sonny ; Dong, Ruijing
> ; Wan Jiabing ; Liu, Leo
> ; Khalid Masum 
> Subject: [PATCH linux-next] drm/amdgpu/vcn: Remove unused assignment
> in vcn_v4_0_stop
>
> The value assigned from vcn_v4_0_stop_dbg_mode to r is overwritten before it 
> can be used. Remove this assignment.
>
> Addresses-Coverity: 1504988 ("Unused value")
> Fixes: 8da1170a16e4 ("drm/amdgpu: add VCN4 ip block support")
> Signed-off-by: Khalid Masum 
> ---
>   drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
> b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
> index ca14c3ef742e..80b8a2c66b36 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
> @@ -1154,7 +1154,7 @@ static int vcn_v4_0_stop(struct amdgpu_device *adev)
>  fw_shared->sq.queue_mode |= FW_QUEUE_DPG_HOLD_OFF;
>
>  if (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG) {
> -   r = vcn_v4_0_stop_dpg_mode(adev, i);
> +   vcn_v4_0_stop_dpg_mode(adev, i);
>  continue;
>  }
>
> --
> 2.37.1
>

After value is overwritten soon right after the diff.

See:
drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c

static int vcn_v4_0_stop(struct amdgpu_device *adev) {
 volatile struct amdgpu_vcn4_fw_shared *fw_shared; ...

 for (i = 0; i < adev->vcn.num_vcn_inst; ++i) {
 fw_shared = adev->vcn.inst[i].fw_shared.cpu_addr;
 fw_shared->sq.queue_mode |= FW_QUEUE_DPG_HOLD_OFF;

 if (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG) {
 r = vcn_v4_0_stop_dpg_mode(adev, i);
 continue;
 }

 /* wait for vcn idle */
 r = SOC15_WAIT_ON_RREG(VCN, i, regUVD_STATUS, 
UVD_STATUS__IDLE, 0x7);

Here, any value assigned to r is overwritten before it could be used. So the 
assignment in the true branch of the if statement here can be removed.

Thanks,
   -- Khalid Masum

RE: [PATCH linux-next] drm/amdgpu/vcn: Remove unused assignment in vcn_v4_0_stop

[Dong, Ruijing]

[AMD Official Use Only - General]

Then please update commit message, this change is due to "value r is never 
used, and remove unnecessary assignment", that makes sense to me.

Thanks
Ruijing

-Original Message-
From: Khalid Masum 
Sent: Monday, August 15, 2022 11:54 AM
To: Dong, Ruijing ; amd-gfx@lists.freedesktop.org; 
dri-de...@lists.freedesktop.org; linux-ker...@vger.kernel.org; 
linux-kernel-ment...@lists.linuxfoundation.org
Cc: Deucher, Alexander ; Koenig, Christian 
; Pan, Xinhui ; David Airlie 
; Daniel Vetter ; Zhu, James 
; Jiang, Sonny ; Wan Jiabing 
; Liu, Leo 
Subject: Re: [PATCH linux-next] drm/amdgpu/vcn: Remove unused assignment in 
vcn_v4_0_stop

On 8/15/22 21:17, Dong, Ruijing wrote:
> [AMD Official Use Only - General]
>
> If the condition was met and it came to execute vcn_4_0_stop_dpg_mode, then 
> it would never have a chance to go for /*wait for vcn idle*/, isn't it?

Hypothetically, some other thread might set adev->pg_flags NULL and in that 
case it will get the chance to go for /* wait for vcn idle */.


> I still didn't see obvious purpose of this change.
>
>  if (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG) {
>   ==>  r = vcn_v4_0_stop_dpg_mode(adev, i);

Regardless of that, this assignment to r is unnecessary. Because this
value of r is never used. This patch simply removes this unnecessary
assignment.

>   continue;
>   }
>
>   /* wait for vcn idle */
>   r = SOC15_WAIT_ON_RREG(VCN, i, regUVD_STATUS, 
> UVD_STATUS__IDLE, 0x7);
>
> Thanks
> Ruijing
>

Thanks,
   -- Khalid Masum

RE: [PATCH linux-next] drm/amdgpu/vcn: Return void from the stop_dbg_mode

[Dong, Ruijing]

[AMD Official Use Only - General]

This patch is

Reviewed-by: Ruijing Dong 

Thanks,
Ruijing

-Original Message-
From: Khalid Masum 
Sent: Monday, August 15, 2022 2:34 PM
To: Dong, Ruijing ; amd-gfx@lists.freedesktop.org; 
dri-de...@lists.freedesktop.org; linux-ker...@vger.kernel.org
Cc: Deucher, Alexander ; Koenig, Christian 
; Pan, Xinhui ; David Airlie 
; Daniel Vetter ; Zhu, James 
; Liu, Leo ; Jiang, Sonny 
; Wan Jiabing ; Greg Kroah-Hartman 
; Khalid Masum 
Subject: [PATCH linux-next] drm/amdgpu/vcn: Return void from the stop_dbg_mode

There is no point in returning an int here. It only returns 0 which the caller 
never uses. Therefore return void and remove the unnecessary assignment.

Addresses-Coverity: 1504988 ("Unused value")
Fixes: 8da1170a16e4 ("drm/amdgpu: add VCN4 ip block support")
Suggested-by: Ruijing Dong 
Suggested-by: Greg Kroah-Hartman 
Signed-off-by: Khalid Masum 
---
Past discussions:
- V1 Link: 
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Flkml%2F20220815070056.10816-1-khalid.masum.92%40gmail.com%2F&data=05%7C01%7Cruijing.dong%40amd.com%7C017222a9e81f49ea336f08da7eecd6c8%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637961852950464412%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=TyXtoF2flWNqabtiJ%2BDVcR2vdsnLZ19qr3b%2BQT2DBQA%3D&reserved=0

Changes since V1:
- Make stop_dbg_mode return void
- Update commit description

 drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c 
b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
index ca14c3ef742e..fb2d74f30448 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
@@ -1115,7 +1115,7 @@ static int vcn_v4_0_start(struct amdgpu_device *adev)
  *
  * Stop VCN block with dpg mode
  */
-static int vcn_v4_0_stop_dpg_mode(struct amdgpu_device *adev, int inst_idx)
+static void vcn_v4_0_stop_dpg_mode(struct amdgpu_device *adev, int
+inst_idx)
 {
uint32_t tmp;

@@ -1133,7 +1133,6 @@ static int vcn_v4_0_stop_dpg_mode(struct amdgpu_device 
*adev, int inst_idx)
/* disable dynamic power gating mode */
WREG32_P(SOC15_REG_OFFSET(VCN, inst_idx, regUVD_POWER_STATUS), 0,
~UVD_POWER_STATUS__UVD_PG_MODE_MASK);
-   return 0;
 }

 /**
@@ -1154,7 +1153,7 @@ static int vcn_v4_0_stop(struct amdgpu_device *adev)
fw_shared->sq.queue_mode |= FW_QUEUE_DPG_HOLD_OFF;

if (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG) {
-   r = vcn_v4_0_stop_dpg_mode(adev, i);
+   vcn_v4_0_stop_dpg_mode(adev, i);
continue;
}

--
2.37.1

RE: [PATCH] drm/amdgpu: support more AV1 encoding requests

[Dong, Ruijing]

[AMD Official Use Only - General]

Thanks Christian,

This is just to cover possible valid ways in kernel as a preparation, av1 
encoding in Mesa is still under developing.

Thanks,
Ruijing

-Original Message-
From: Christian König 
Sent: Thursday, February 23, 2023 1:48 AM
To: Wu, David ; amd-gfx@lists.freedesktop.org; Koenig, 
Christian 
Cc: Deucher, Alexander ; Dong, Ruijing 
; Liu, Leo 
Subject: Re: [PATCH] drm/amdgpu: support more AV1 encoding requests

Am 23.02.23 um 00:11 schrieb David (Ming Qiang) Wu:
> Ensuring accurate IB package searching and covers more corners for AV1
> encoding requests.

That at least looks much cleaner now. Do we already have the Mesa patches ready 
which use this?

Regards,
Christian.

>
> Signed-off-by: David (Ming Qiang) Wu 
> Reviewed-by: Ruijing Dong 
> ---
>   drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c | 81 +--
>   1 file changed, 75 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
> b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
> index 22a41766a8c7..8235ff3820ed 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
> @@ -1726,6 +1726,7 @@ static int vcn_v4_0_dec_msg(struct
> amdgpu_cs_parser *p, struct amdgpu_job *job,
>
>   #define RADEON_VCN_ENGINE_TYPE_ENCODE   (0x0002)
>   #define RADEON_VCN_ENGINE_TYPE_DECODE   (0x0003)
> +#define RADEON_VCN_ENGINE_TYPE_ENCODE_QUEUE  (0x0004)
>
>   #define RADEON_VCN_ENGINE_INFO  (0x3001)
>   #define RADEON_VCN_ENGINE_INFO_MAX_OFFSET   16
> @@ -1733,21 +1734,86 @@ static int vcn_v4_0_dec_msg(struct amdgpu_cs_parser 
> *p, struct amdgpu_job *job,
>   #define RENCODE_ENCODE_STANDARD_AV1 2
>   #define RENCODE_IB_PARAM_SESSION_INIT   0x0003
>   #define RENCODE_IB_PARAM_SESSION_INIT_MAX_OFFSET64
> +#define RENCODE_IB_ENC_QUE_INSTRUCTION   (0x3201)
> +#define RENCODE_IB_ENC_QUE_INSTRUCTION_MAX_OFFSET64
>
>   /* return the offset in ib if id is found, -1 otherwise
>* to speed up the searching we only search upto max_offset
>*/
> -static int vcn_v4_0_enc_find_ib_param(struct amdgpu_ib *ib, uint32_t
> id, int max_offset)
> +static int vcn_v4_0_enc_find_ib_param(uint32_t *ptr, int size,
> +uint32_t id, int max_offset)
>   {
>   int i;
>
> - for (i = 0; i < ib->length_dw && i < max_offset && ib->ptr[i] >= 8; i 
> += ib->ptr[i]/4) {
> - if (ib->ptr[i + 1] == id)
> + for (i = 0; i < size && i < max_offset && ptr[i] >= 8; i += ptr[i] / 4) 
> {
> + if (ptr[i + 1] == id)
>   return i;
>   }
>   return -1;
>   }
>
> +static int vcn_v4_0_enc_queue_msg(struct amdgpu_cs_parser *p,
> +   struct amdgpu_job *job,
> +   struct amdgpu_ib *ib)
> +{
> + struct ttm_operation_ctx ctx = { false, false };
> + struct amdgpu_bo_va_mapping *map;
> + struct amdgpu_bo *bo;
> + uint64_t start, end;
> + int i;
> + void *ptr;
> + int r;
> + int data_size = 0;
> + uint64_t addr;
> + uint32_t *msg;
> +
> + i = vcn_v4_0_enc_find_ib_param(ib->ptr, ib->length_dw, 
> RENCODE_IB_ENC_QUE_INSTRUCTION,
> + RENCODE_IB_ENC_QUE_INSTRUCTION_MAX_OFFSET);
> + if (i >= 0) {
> + addr = ((uint64_t)ib->ptr[i + 3]) << 32 | ib->ptr[i + 2];
> + data_size = ib->ptr[i + 4];
> + }
> +
> + if (!data_size) /* did not find */
> + return 0;
> +
> + addr &= AMDGPU_GMC_HOLE_MASK;
> + r = amdgpu_cs_find_mapping(p, addr, &bo, &map);
> + if (r) {
> + DRM_ERROR("Can't find BO for addr 0x%08llx\n", addr);
> + return r;
> + }
> +
> + start = map->start * AMDGPU_GPU_PAGE_SIZE;
> + end = (map->last + 1) * AMDGPU_GPU_PAGE_SIZE;
> + if (addr & 0x7) {
> + DRM_ERROR("VCN messages must be 8 byte aligned!\n");
> + return -EINVAL;
> + }
> +
> + bo->flags |= AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED;
> + amdgpu_bo_placement_from_domain(bo, bo->allowed_domains);
> + r = ttm_bo_validate(&bo->tbo, &bo->placement, &ctx);
> + if (r) {
> + DRM_ERROR("Failed validating the VCN message BO (%d)!\n", r);
> + return r;
> + }
> +
> + r = amdgpu_bo_kmap(bo, &ptr);
> + if (r) {
> + DRM_ERROR("

RE: [PATCH] drm/amdgpu: support more AV1 encoding requests

[Dong, Ruijing]

[AMD Official Use Only - General]

If no other comment or concern, we will push it.

Thanks,
Ruijing

-Original Message-
From: amd-gfx  On Behalf Of Dong, Ruijing
Sent: Thursday, February 23, 2023 10:19 AM
To: Christian König ; Wu, David 
; amd-gfx@lists.freedesktop.org; Koenig, Christian 

Cc: Deucher, Alexander ; Liu, Leo 
Subject: RE: [PATCH] drm/amdgpu: support more AV1 encoding requests

[AMD Official Use Only - General]

[AMD Official Use Only - General]

Thanks Christian,

This is just to cover possible valid ways in kernel as a preparation, av1 
encoding in Mesa is still under developing.

Thanks,
Ruijing

-Original Message-
From: Christian König 
Sent: Thursday, February 23, 2023 1:48 AM
To: Wu, David ; amd-gfx@lists.freedesktop.org; Koenig, 
Christian 
Cc: Deucher, Alexander ; Dong, Ruijing 
; Liu, Leo 
Subject: Re: [PATCH] drm/amdgpu: support more AV1 encoding requests

Am 23.02.23 um 00:11 schrieb David (Ming Qiang) Wu:
> Ensuring accurate IB package searching and covers more corners for AV1
> encoding requests.

That at least looks much cleaner now. Do we already have the Mesa patches ready 
which use this?

Regards,
Christian.

>
> Signed-off-by: David (Ming Qiang) Wu 
> Reviewed-by: Ruijing Dong 
> ---
>   drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c | 81 +--
>   1 file changed, 75 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
> b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
> index 22a41766a8c7..8235ff3820ed 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
> @@ -1726,6 +1726,7 @@ static int vcn_v4_0_dec_msg(struct
> amdgpu_cs_parser *p, struct amdgpu_job *job,
>
>   #define RADEON_VCN_ENGINE_TYPE_ENCODE   (0x0002)
>   #define RADEON_VCN_ENGINE_TYPE_DECODE   (0x0003)
> +#define RADEON_VCN_ENGINE_TYPE_ENCODE_QUEUE  (0x0004)
>
>   #define RADEON_VCN_ENGINE_INFO  (0x3001)
>   #define RADEON_VCN_ENGINE_INFO_MAX_OFFSET   16
> @@ -1733,21 +1734,86 @@ static int vcn_v4_0_dec_msg(struct amdgpu_cs_parser 
> *p, struct amdgpu_job *job,
>   #define RENCODE_ENCODE_STANDARD_AV1 2
>   #define RENCODE_IB_PARAM_SESSION_INIT   0x0003
>   #define RENCODE_IB_PARAM_SESSION_INIT_MAX_OFFSET64
> +#define RENCODE_IB_ENC_QUE_INSTRUCTION   (0x3201)
> +#define RENCODE_IB_ENC_QUE_INSTRUCTION_MAX_OFFSET64
>
>   /* return the offset in ib if id is found, -1 otherwise
>* to speed up the searching we only search upto max_offset
>*/
> -static int vcn_v4_0_enc_find_ib_param(struct amdgpu_ib *ib, uint32_t
> id, int max_offset)
> +static int vcn_v4_0_enc_find_ib_param(uint32_t *ptr, int size,
> +uint32_t id, int max_offset)
>   {
>   int i;
>
> - for (i = 0; i < ib->length_dw && i < max_offset && ib->ptr[i] >= 8; i 
> += ib->ptr[i]/4) {
> - if (ib->ptr[i + 1] == id)
> + for (i = 0; i < size && i < max_offset && ptr[i] >= 8; i += ptr[i] / 4) 
> {
> + if (ptr[i + 1] == id)
>   return i;
>   }
>   return -1;
>   }
>
> +static int vcn_v4_0_enc_queue_msg(struct amdgpu_cs_parser *p,
> +   struct amdgpu_job *job,
> +   struct amdgpu_ib *ib) {
> + struct ttm_operation_ctx ctx = { false, false };
> + struct amdgpu_bo_va_mapping *map;
> + struct amdgpu_bo *bo;
> + uint64_t start, end;
> + int i;
> + void *ptr;
> + int r;
> + int data_size = 0;
> + uint64_t addr;
> + uint32_t *msg;
> +
> + i = vcn_v4_0_enc_find_ib_param(ib->ptr, ib->length_dw, 
> RENCODE_IB_ENC_QUE_INSTRUCTION,
> + RENCODE_IB_ENC_QUE_INSTRUCTION_MAX_OFFSET);
> + if (i >= 0) {
> + addr = ((uint64_t)ib->ptr[i + 3]) << 32 | ib->ptr[i + 2];
> + data_size = ib->ptr[i + 4];
> + }
> +
> + if (!data_size) /* did not find */
> + return 0;
> +
> + addr &= AMDGPU_GMC_HOLE_MASK;
> + r = amdgpu_cs_find_mapping(p, addr, &bo, &map);
> + if (r) {
> + DRM_ERROR("Can't find BO for addr 0x%08llx\n", addr);
> + return r;
> + }
> +
> + start = map->start * AMDGPU_GPU_PAGE_SIZE;
> + end = (map->last + 1) * AMDGPU_GPU_PAGE_SIZE;
> + if (addr & 0x7) {
> + DRM_ERROR("VCN messages must be 8 byte aligned!\n");
> + return -EINVAL;
> + }
> +
> + bo->flags |= AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED;
&

RE: [PATCH] drm/amdgpu: fix limiting AV1 to the first instance on VCN3

[Dong, Ruijing]

[AMD Official Use Only - General]

I can see for VCN4, AV1 dec/enc also need to limit to the first instance.

Thanks,
Ruijing

-Original Message-
From: amd-gfx  On Behalf Of Alex Deucher
Sent: Friday, June 3, 2022 10:12 AM
To: Christian König 
Cc: Pelloux-Prayer, Pierre-Eric ; amd-gfx 
list 
Subject: Re: [PATCH] drm/amdgpu: fix limiting AV1 to the first instance on VCN3

Do the other uvd/vce/vcn ring parse functions need a similar fix?

Alex


On Fri, Jun 3, 2022 at 10:08 AM Alex Deucher  wrote:
>
> On Fri, Jun 3, 2022 at 8:10 AM Christian König
>  wrote:
> >
> > Am 03.06.22 um 14:08 schrieb Pierre-Eric Pelloux-Prayer:
> > > Hi Christian,
> > >
> > > The patch is: Tested-by: Pierre-Eric Pelloux-Prayer
> > > 
> > >
> > > Could you add a reference to 
> > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.freedesktop.org%2Fdrm%2Famd%2F-%2Fissues%2F2037&data=05%7C01%7CRuijing.Dong%40amd.com%7C5ba73dfe91ba47e21dd608da456b0609%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637898623221806051%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=WgIZD299Xe0XVG%2Ftb2rn14njS%2FgHIhtIHIDAZ2Fj40k%3D&reserved=0
> > >  in the commit message?
> >
> > Sure, can you also give me an rb or acked-by so that I can push it?
>
> Reviewed-by: Alex Deucher 
>
> >
> > Thanks,
> > Christian.
> >
> > >
> > > Thanks,
> > > Pierre-Eric
> > >
> > > On 03/06/2022 12:21, Christian König wrote:
> > >> The job is not yet initialized here.
> > >>
> > >> Signed-off-by: Christian König 
> > >> Fixes: 1027d5d791b7 ("drm/amdgpu: use job and ib structures
> > >> directly in CS parsers")
> > >> ---
> > >>   drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c | 17 +++--
> > >>   1 file changed, 7 insertions(+), 10 deletions(-)
> > >>
> > >> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
> > >> b/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
> > >> index 3cabceee5f57..39405f0db824 100644
> > >> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
> > >> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
> > >> @@ -1761,23 +1761,21 @@ static const struct amdgpu_ring_funcs 
> > >> vcn_v3_0_dec_sw_ring_vm_funcs = {
> > >>  .emit_reg_write_reg_wait = 
> > >> amdgpu_ring_emit_reg_write_reg_wait_helper,
> > >>   };
> > >>
> > >> -static int vcn_v3_0_limit_sched(struct amdgpu_cs_parser *p,
> > >> -struct amdgpu_job *job)
> > >> +static int vcn_v3_0_limit_sched(struct amdgpu_cs_parser *p)
> > >>   {
> > >>  struct drm_gpu_scheduler **scheds;
> > >>
> > >>  /* The create msg must be in the first IB submitted */
> > >> -if (atomic_read(&job->base.entity->fence_seq))
> > >> +if (atomic_read(&p->entity->fence_seq))
> > >>  return -EINVAL;
> > >>
> > >>  scheds = p->adev->gpu_sched[AMDGPU_HW_IP_VCN_DEC]
> > >>  [AMDGPU_RING_PRIO_DEFAULT].sched;
> > >> -drm_sched_entity_modify_sched(job->base.entity, scheds, 1);
> > >> +drm_sched_entity_modify_sched(p->entity, scheds, 1);
> > >>  return 0;
> > >>   }
> > >>
> > >> -static int vcn_v3_0_dec_msg(struct amdgpu_cs_parser *p, struct 
> > >> amdgpu_job *job,
> > >> -uint64_t addr)
> > >> +static int vcn_v3_0_dec_msg(struct amdgpu_cs_parser *p, uint64_t
> > >> +addr)
> > >>   {
> > >>  struct ttm_operation_ctx ctx = { false, false };
> > >>  struct amdgpu_bo_va_mapping *map; @@ -1848,7 +1846,7 @@
> > >> static int vcn_v3_0_dec_msg(struct amdgpu_cs_parser *p, struct 
> > >> amdgpu_job *job,
> > >>  if (create[0] == 0x7 || create[0] == 0x10 || create[0] == 
> > >> 0x11)
> > >>  continue;
> > >>
> > >> -r = vcn_v3_0_limit_sched(p, job);
> > >> +r = vcn_v3_0_limit_sched(p);
> > >>  if (r)
> > >>  goto out;
> > >>  }
> > >> @@ -1862,7 +1860,7 @@ static int vcn_v3_0_ring_patch_cs_in_place(struct 
> > >> amdgpu_cs_parser *p,
> > >> struct amdgpu_job *job,
> > >> struct amdgpu_ib *ib)
> > >>   {
> > >> -struct amdgpu_ring *ring = to_amdgpu_ring(job->base.sched);
> > >> +struct amdgpu_ring *ring =
> > >> + to_amdgpu_ring(p->entity->rq->sched);
> > >>  uint32_t msg_lo = 0, msg_hi = 0;
> > >>  unsigned i;
> > >>  int r;
> > >> @@ -1881,8 +1879,7 @@ static int vcn_v3_0_ring_patch_cs_in_place(struct 
> > >> amdgpu_cs_parser *p,
> > >>  msg_hi = val;
> > >>  } else if (reg == PACKET0(p->adev->vcn.internal.cmd, 0) &&
> > >> val == 0) {
> > >> -r = vcn_v3_0_dec_msg(p, job,
> > >> - ((u64)msg_hi) << 32 | msg_lo);
> > >> +r = vcn_v3_0_dec_msg(p, ((u64)msg_hi) << 32
> > >> + | msg_lo);
> > >>  if (r)
> > >>  return r;
> > >>  }
> > >>
> >

RE: [PATCH 2/2] drm/amdgpu/jpeg5.0.1: use num_jpeg_inst for SR-IOV

[Dong, Ruijing]

[AMD Official Use Only - AMD Internal Distribution Only]

The series is:

Reviewed-by: Ruijing Dong 

-Original Message-
From: amd-gfx  On Behalf Of Alex Deucher
Sent: Tuesday, December 10, 2024 4:47 PM
To: amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander 
Subject: [PATCH 2/2] drm/amdgpu/jpeg5.0.1: use num_jpeg_inst for SR-IOV

They should be the same, but use the proper variable.

Signed-off-by: Alex Deucher 
---
 drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_1.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_1.c 
b/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_1.c
index 8bfa400e7a874..40d4c32a8c2a6 100644
--- a/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_1.c
+++ b/drivers/gpu/drm/amd/amdgpu/jpeg_v5_0_1.c
@@ -183,7 +183,7 @@ static int jpeg_v5_0_1_hw_init(struct amdgpu_ip_block 
*ip_block)

if (amdgpu_sriov_vf(adev)) {
/* jpeg_v5_0_1_start_sriov(adev); */
-   for (i = 0; i < adev->vcn.num_vcn_inst; ++i) {
+   for (i = 0; i < adev->jpeg.num_jpeg_inst; ++i) {
for (j = 0; j < adev->jpeg.num_jpeg_rings; ++j) {
ring = &adev->jpeg.inst[i].ring_dec[j];
ring->wptr = 0;
--
2.47.1

RE: [PATCH] drm/amdgpu/vcn4.0.5: Fix GFX10_ADDR_CONFIG programming for vcn1.

[Dong, Ruijing]

[AMD Official Use Only - AMD Internal Distribution Only]

Thanks Saleem, I will redo it as v2, in fact the original implementation in 
vcn_v4_0_5 is correct.
However it missed other part.

Ruijing

-Original Message-
From: Jamadar, Saleemkhan 
Sent: Thursday, May 1, 2025 8:49 AM
To: Dong, Ruijing ; amd-gfx@lists.freedesktop.org; 
Koenig, Christian 
Cc: Deucher, Alexander ; Liu, Leo 
Subject: Re: [PATCH] drm/amdgpu/vcn4.0.5: Fix GFX10_ADDR_CONFIG programming for 
vcn1.

Looks good to me.
Reviewed by : Saleemkhan Jamadar 

On 5/1/2025 5:57 AM, Ruijing Dong wrote:
> The UVD_GFX10_ADDR_CONFIG's offset for vcn1 was programmed
> incorrectly, which causes the corrupted output from VCN1.
>
> This fixes the issue, copied from UVD_GFX10_ADDR_CONFIG programming
> from other VCN generations.
>
> Signed-off-by: Ruijing Dong 
> ---
>   drivers/gpu/drm/amd/amdgpu/vcn_v4_0_5.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0_5.c
> b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0_5.c
> index a8cfc63713ad..31cb19e144fe 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0_5.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0_5.c
> @@ -563,7 +563,7 @@ static void vcn_v4_0_5_mc_resume_dpg_mode(struct
> amdgpu_vcn_inst *vinst,
>
>   /* VCN global tiling registers */
>   WREG32_SOC15_DPG_MODE(inst_idx, SOC15_DPG_MODE_OFFSET(
> - VCN, inst_idx, regUVD_GFX10_ADDR_CONFIG),
> + VCN, 0, regUVD_GFX10_ADDR_CONFIG),
>   adev->gfx.config.gb_addr_config, 0, indirect);
>   }
>

RE: [PATCH 1/4] drm/amdgpu: Fix MPEG2, MPEG4 and VC1 video caps max size

[Dong, Ruijing]

[AMD Official Use Only - AMD Internal Distribution Only]

The series is:

Reviewed-by: Ruijing Dong 

-Original Message-
From: amd-gfx  On Behalf Of David Rosca
Sent: Friday, February 28, 2025 10:10 AM
To: Alex Deucher 
Cc: amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 1/4] drm/amdgpu: Fix MPEG2, MPEG4 and VC1 video caps max 
size

On 28. 02. 25 15:21, Alex Deucher wrote:
> On Fri, Feb 28, 2025 at 8:34 AM David Rosca  wrote:
>> 1920x1088 is the maximum supported resolution.
>>
>> Signed-off-by: David Rosca 
>> ---
>>   drivers/gpu/drm/amd/amdgpu/nv.c | 18 -
>>   drivers/gpu/drm/amd/amdgpu/vi.c | 36
>> -
> I think soc15.c needs similar fixes?
Fixed in v2.
>Also, I think we need to bump
> the driver version so mesa knows what kernel version has the proper
> limits.

I think for now we can ignore the kernel caps in Mesa for these codecs as the 
maximum size is the same for all asics. We can later switch to kernel caps 
again, no need to bump the version just for this.

David

>
> Alex
>
>>   2 files changed, 27 insertions(+), 27 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/nv.c
>> b/drivers/gpu/drm/amd/amdgpu/nv.c index 8068f384f56c..a18e6fb9fa3f
>> 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/nv.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/nv.c
>> @@ -78,10 +78,10 @@ static const struct amdgpu_video_codecs
>> nv_video_codecs_encode = {
>>
>>   /* Navi1x */
>>   static const struct amdgpu_video_codec_info nv_video_codecs_decode_array[] 
>> = {
>> -   {codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_MPEG2, 4096, 
>> 4096, 3)},
>> -   {codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_MPEG4, 4096, 
>> 4096, 5)},
>> +   {codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_MPEG2, 1920, 
>> 1088, 3)},
>> +   {codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_MPEG4,
>> + 1920, 1088, 5)},
>>  {codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_MPEG4_AVC, 4096, 
>> 4096, 52)},
>> -   {codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_VC1, 4096, 4096, 
>> 4)},
>> +   {codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_VC1, 1920,
>> + 1088, 4)},
>>  {codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_HEVC, 8192, 
>> 4352, 186)},
>>  {codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_JPEG, 4096, 
>> 4096, 0)},
>>  {codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_VP9,
>> 8192, 4352, 0)}, @@ -104,10 +104,10 @@ static const struct 
>> amdgpu_video_codecs sc_video_codecs_encode = {
>>   };
>>
>>   static const struct amdgpu_video_codec_info 
>> sc_video_codecs_decode_array_vcn0[] = {
>> -   {codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_MPEG2, 4096, 
>> 4096, 3)},
>> -   {codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_MPEG4, 4096, 
>> 4096, 5)},
>> +   {codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_MPEG2, 1920, 
>> 1088, 3)},
>> +   {codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_MPEG4,
>> + 1920, 1088, 5)},
>>  {codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_MPEG4_AVC, 4096, 
>> 4096, 52)},
>> -   {codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_VC1, 4096, 4096, 
>> 4)},
>> +   {codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_VC1, 1920,
>> + 1088, 4)},
>>  {codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_HEVC, 8192, 
>> 4352, 186)},
>>  {codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_JPEG, 16384, 
>> 16384, 0)},
>>  {codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_VP9,
>> 8192, 4352, 0)}, @@ -115,10 +115,10 @@ static const struct 
>> amdgpu_video_codec_info sc_video_codecs_decode_array_vcn0[]
>>   };
>>
>>   static const struct amdgpu_video_codec_info 
>> sc_video_codecs_decode_array_vcn1[] = {
>> -   {codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_MPEG2, 4096, 
>> 4096, 3)},
>> -   {codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_MPEG4, 4096, 
>> 4096, 5)},
>> +   {codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_MPEG2, 1920, 
>> 1088, 3)},
>> +   {codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_MPEG4,
>> + 1920, 1088, 5)},
>>  {codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_MPEG4_AVC, 4096, 
>> 4096, 52)},
>> -   {codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_VC1, 4096, 4096, 
>> 4)},
>> +   {codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_VC1, 1920,
>> + 1088, 4)},
>>  {codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_HEVC, 8192, 
>> 4352, 186)},
>>  {codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_JPEG, 16384, 
>> 16384, 0)},
>>  {codec_info_build(AMDGPU_INFO_VIDEO_CAPS_CODEC_IDX_VP9,
>> 8192, 4352, 0)}, diff --git a/drivers/gpu/drm/amd/amdgpu/vi.c
>> b/drivers/gpu/drm/amd/amdgpu/vi.c index 375242d9..9b3510e53112
>> 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/vi.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/vi.c
>> @@ -167,16 +167,16 @@ static const struct amdgpu_video_codec_info 
>> tonga_video_codecs_decode_array[] =
>>   {
>>  {
>>   

RE: [PATCH] drm/amdgpu/display: Allow DCC for video formats on GFX12

[Dong, Ruijing]

[AMD Official Use Only - AMD Internal Distribution Only]

Reviewed-by: Ruijing Dong 

-Original Message-
From: amd-gfx  On Behalf Of David Rosca
Sent: Thursday, February 13, 2025 12:07 PM
To: amd-gfx@lists.freedesktop.org
Cc: Rosca, David 
Subject: [PATCH] drm/amdgpu/display: Allow DCC for video formats on GFX12

We advertise DCC as supported for NV12/P010 formats on GFX12, but it would fail 
on this check on commit.

Signed-off-by: David Rosca 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
index 774cc3f4f3fd..92472109f84a 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
@@ -277,8 +277,11 @@ static int amdgpu_dm_plane_validate_dcc(struct 
amdgpu_device *adev,
if (!dcc->enable)
return 0;

-   if (format >= SURFACE_PIXEL_FORMAT_VIDEO_BEGIN ||
-   !dc->cap_funcs.get_dcc_compression_cap)
+   if (adev->family < AMDGPU_FAMILY_GC_12_0_0 &&
+   format >= SURFACE_PIXEL_FORMAT_VIDEO_BEGIN)
+   return -EINVAL;
+
+   if (!dc->cap_funcs.get_dcc_compression_cap)
return -EINVAL;

input.format = format;
--
2.43.0

RE: [PATCH 2/2] drm/amdgpu/vcn: send session ctx along with msg buffer

[Dong, Ruijing]

[AMD Official Use Only - AMD Internal Distribution Only]

This series is

Reviewed-by: Ruijing Dong 

-Original Message-
From: amd-gfx  On Behalf Of 
boyuan.zh...@amd.com
Sent: Friday, February 21, 2025 9:06 PM
To: amd-gfx@lists.freedesktop.org
Cc: Zhang, Boyuan ; Yao, Yinjie 
Subject: [PATCH 2/2] drm/amdgpu/vcn: send session ctx along with msg buffer

From: Boyuan Zhang 

Session context buffer is required to be sent along with message buffer

Signed-off-by: Boyuan Zhang 
Tested-by: Yinjie Yao 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 43 ++---
 1 file changed, 31 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
index 83faf6e6788a..8d2cce3ea7af 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
@@ -602,18 +602,29 @@ static int amdgpu_vcn_dec_send_msg(struct amdgpu_ring 
*ring,  }

 static int amdgpu_vcn_dec_get_create_msg(struct amdgpu_ring *ring, uint32_t 
handle,
-   struct amdgpu_ib *ib)
+   struct amdgpu_ib *ib, struct amdgpu_ib *ctx)
 {
struct amdgpu_device *adev = ring->adev;
uint32_t *msg;
int r, i;

-   memset(ib, 0, sizeof(*ib));
-   r = amdgpu_ib_get(adev, NULL, AMDGPU_GPU_PAGE_SIZE * 2,
-   AMDGPU_IB_POOL_DIRECT,
-   ib);
-   if (r)
-   return r;
+   if (ctx) {
+   memset(ctx, 0, sizeof(*ctx));
+   r = amdgpu_ib_get(adev, NULL, AMDGPU_GPU_PAGE_SIZE * 32,
+   AMDGPU_IB_POOL_DIRECT,
+   ctx);
+   if (r)
+   return r;
+   }
+
+   if (ib) {
+   memset(ib, 0, sizeof(*ib));
+   r = amdgpu_ib_get(adev, NULL, AMDGPU_GPU_PAGE_SIZE * 2,
+   AMDGPU_IB_POOL_DIRECT,
+   ib);
+   if (r)
+   return r;
+   }

msg = (uint32_t *)AMDGPU_GPU_PAGE_ALIGN((unsigned long)ib->ptr);
msg[0] = cpu_to_le32(0x0028);
@@ -669,7 +680,7 @@ int amdgpu_vcn_dec_ring_test_ib(struct amdgpu_ring *ring, 
long timeout)
struct amdgpu_ib ib;
long r;

-   r = amdgpu_vcn_dec_get_create_msg(ring, 1, &ib);
+   r = amdgpu_vcn_dec_get_create_msg(ring, 1, &ib, NULL);
if (r)
goto error;

@@ -727,6 +738,7 @@ static void amdgpu_vcn_unified_ring_ib_checksum(uint32_t 
**ib_checksum,

 static int amdgpu_vcn_dec_sw_send_msg(struct amdgpu_ring *ring,
  struct amdgpu_ib *ib_msg,
+ struct amdgpu_ib *ib_ctx,
  struct dma_fence **fence)
 {
struct amdgpu_vcn_decode_buffer *decode_buffer = NULL; @@ -735,6 +747,7 
@@ static int amdgpu_vcn_dec_sw_send_msg(struct amdgpu_ring *ring,
struct dma_fence *f = NULL;
struct amdgpu_job *job;
struct amdgpu_ib *ib;
+   uint64_t addr_ctx = AMDGPU_GPU_PAGE_ALIGN(ib_ctx->gpu_addr);
uint64_t addr = AMDGPU_GPU_PAGE_ALIGN(ib_msg->gpu_addr);
uint32_t *ib_checksum;
uint32_t ib_pack_in_dw;
@@ -765,6 +778,10 @@ static int amdgpu_vcn_dec_sw_send_msg(struct amdgpu_ring 
*ring,
ib->length_dw += sizeof(struct amdgpu_vcn_decode_buffer) / 4;
memset(decode_buffer, 0, sizeof(struct amdgpu_vcn_decode_buffer));

+   decode_buffer->valid_buf_flag |= 
cpu_to_le32(AMDGPU_VCN_CMD_FLAG_SESSION_CONTEXT_BUFFER);
+   decode_buffer->session_ctx_buffer_address_hi = cpu_to_le32(addr_ctx >> 
32);
+   decode_buffer->session_ctx_buffer_address_lo = cpu_to_le32(addr_ctx);
+
decode_buffer->valid_buf_flag |= 
cpu_to_le32(AMDGPU_VCN_CMD_FLAG_MSG_BUFFER);
decode_buffer->msg_buffer_address_hi = cpu_to_le32(addr >> 32);
decode_buffer->msg_buffer_address_lo = cpu_to_le32(addr); @@ -780,6 
+797,7 @@ static int amdgpu_vcn_dec_sw_send_msg(struct amdgpu_ring *ring,
goto err_free;

amdgpu_ib_free(ib_msg, f);
+   amdgpu_ib_free(ib_ctx, f);

if (fence)
*fence = dma_fence_get(f);
@@ -791,27 +809,28 @@ static int amdgpu_vcn_dec_sw_send_msg(struct amdgpu_ring 
*ring,
amdgpu_job_free(job);
 err:
amdgpu_ib_free(ib_msg, f);
+   amdgpu_ib_free(ib_ctx, f);
return r;
 }

 int amdgpu_vcn_dec_sw_ring_test_ib(struct amdgpu_ring *ring, long timeout)  {
struct dma_fence *fence = NULL;
-   struct amdgpu_ib ib;
+   struct amdgpu_ib ib, ctx;
long r;

-   r = amdgpu_vcn_dec_get_create_msg(ring, 1, &ib);
+   r = amdgpu_vcn_dec_get_create_msg(ring, 1, &ib, &ctx);
if (r)
goto error;

-   r = amdgpu_vcn_dec_sw_send_msg(ring, &ib, NULL);
+   r = amdgpu_vcn_dec_sw_send_msg(ring, &ib, &ctx, NULL);
if (r)
goto error;
r = amdgpu_vcn_dec_ge

RE: [PATCH V2 2/2] drm/amdgpu: read back register after written for VCN v4.0.0 and v5.0.0

[Dong, Ruijing]

[AMD Official Use Only - AMD Internal Distribution Only]

Hi David,

The last register operation in vcn_v4_0_start and vcn_v5_0_0_start is write, I 
guess we can extend this little more to do a dummy read as the last register 
operation.

Thanks,
Ruijing

-Original Message-
From: Wu, David 
Sent: Tuesday, May 13, 2025 2:23 PM
To: amd-gfx@lists.freedesktop.org; Koenig, Christian 
Cc: Deucher, Alexander ; Liu, Leo ; 
Jiang, Sonny ; Dong, Ruijing ; 
Limonciello, Mario 
Subject: [PATCH V2 2/2] drm/amdgpu: read back register after written for VCN 
v4.0.0 and v5.0.0

V2: not to add extra read-back in vcn_v4_0_start and vcn_v5_0_0_start as
there are read-back calls already. New comments for better understanding.

Similar to the previous changes made for VCN v4.0.5, the addition of register 
read-back support in VCN v4.0.0 and v5.0.0 is intended to prevent potential 
race conditions, even though such issues have not been observed yet. This 
change ensures consistency across different VCN variants and helps avoid 
similar issues on newer or closely related GPUs. The overhead introduced by 
this read-back is negligible.

Signed-off-by: David (Ming Qiang) Wu 
Reviewed-by: Mario Limonciello 
---
 drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c   | 4 
 drivers/gpu/drm/amd/amdgpu/vcn_v5_0_0.c | 4 
 2 files changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c 
b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
index 8fff470bce873..070a2a8cdf6f4 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
@@ -1122,6 +1122,10 @@ static int vcn_v4_0_start_dpg_mode(struct 
amdgpu_vcn_inst *vinst, bool indirect)
ring->doorbell_index << VCN_RB1_DB_CTRL__OFFSET__SHIFT |
VCN_RB1_DB_CTRL__EN_MASK);

+   /* Keeping one read-back to ensure all register writes are done, 
otherwise
+* it may introduce race conditions */
+   RREG32_SOC15(VCN, inst_idx, regVCN_RB1_DB_CTRL);
+
return 0;
 }

diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v5_0_0.c 
b/drivers/gpu/drm/amd/amdgpu/vcn_v5_0_0.c
index 27dcc6f37a730..77c27a317e4c8 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v5_0_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v5_0_0.c
@@ -794,6 +794,10 @@ static int vcn_v5_0_0_start_dpg_mode(struct 
amdgpu_vcn_inst *vinst,
ring->doorbell_index << VCN_RB1_DB_CTRL__OFFSET__SHIFT |
VCN_RB1_DB_CTRL__EN_MASK);

+   /* Keeping one read-back to ensure all register writes are done, 
otherwise
+* it may introduce race conditions */
+   RREG32_SOC15(VCN, inst_idx, regVCN_RB1_DB_CTRL);
+
return 0;
 }

--
2.34.1

RE: [PATCH 1/2] drm/amdgpu: read back DB_CTRL register after written for VCN v4.0.5

[Dong, Ruijing]

[AMD Official Use Only - AMD Internal Distribution Only]

Then dummy read should not be limited to this register regVCN_RB1_DB_CTRL, it 
can be any valid readable register just for posting the previous write 
operations.

Thanks,
Ruijing

-Original Message-
From: Wu, David 
Sent: Tuesday, May 13, 2025 12:48 PM
To: Alex Deucher ; Wu, David 
Cc: amd-gfx@lists.freedesktop.org; Koenig, Christian 
; Deucher, Alexander ; 
Liu, Leo ; Jiang, Sonny ; Dong, Ruijing 
; sta...@vger.kernel.org
Subject: Re: [PATCH 1/2] drm/amdgpu: read back DB_CTRL register after written 
for VCN v4.0.5

sounds great! will adjust accordingly.

David

On 2025-05-13 12:44, Alex Deucher wrote:
> On Tue, May 13, 2025 at 12:38 PM David (Ming Qiang) Wu
>  wrote:
>> On VCN v4.0.5 there is a race condition where the WPTR is not updated
>> after starting from idle when doorbell is used. The read-back of
>> regVCN_RB1_DB_CTRL register after written is to ensure the
>> doorbell_index is updated before it can work properly.
>>
>> Link: https://gitlab.freedesktop.org/mesa/mesa/-/issues/12528
>> Signed-off-by: David (Ming Qiang) Wu 
>> ---
>>   drivers/gpu/drm/amd/amdgpu/vcn_v4_0_5.c | 4 
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0_5.c
>> b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0_5.c
>> index ed00d35039c1..d6be8b05d7a2 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0_5.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0_5.c
>> @@ -1033,6 +1033,8 @@ static int vcn_v4_0_5_start_dpg_mode(struct 
>> amdgpu_vcn_inst *vinst,
>>  WREG32_SOC15(VCN, inst_idx, regVCN_RB1_DB_CTRL,
>>  ring->doorbell_index << 
>> VCN_RB1_DB_CTRL__OFFSET__SHIFT |
>>  VCN_RB1_DB_CTRL__EN_MASK);
>> +   /* Read DB_CTRL to flush the write DB_CTRL command. */
>> +   RREG32_SOC15(VCN, inst_idx, regVCN_RB1_DB_CTRL);
>>
>>  return 0;
>>   }
>> @@ -1195,6 +1197,8 @@ static int vcn_v4_0_5_start(struct amdgpu_vcn_inst 
>> *vinst)
>>  WREG32_SOC15(VCN, i, regVCN_RB1_DB_CTRL,
>>   ring->doorbell_index << VCN_RB1_DB_CTRL__OFFSET__SHIFT 
>> |
>>   VCN_RB1_DB_CTRL__EN_MASK);
>> +   /* Read DB_CTRL to flush the write DB_CTRL command. */
>> +   RREG32_SOC15(VCN, i, regVCN_RB1_DB_CTRL);
> You might want to move this one down to the end of the function to
> post the other subsequent writes.  Arguably all of the VCNs should do
> something similar.  If you want to make sure a PCIe write goes
> through, you need to issue a subsequent read.  Doing this at the end
> of each function should post all previous writes.
>
> Alex
>
>>  WREG32_SOC15(VCN, i, regUVD_RB_BASE_LO, ring->gpu_addr);
>>  WREG32_SOC15(VCN, i, regUVD_RB_BASE_HI,
>> upper_32_bits(ring->gpu_addr));
>> --
>> 2.49.0
>>

RE: [PATCH v3 3/3] drm/amdgpu: read back register after written for VCN v5.0.0

[Dong, Ruijing]

[AMD Official Use Only - AMD Internal Distribution Only]

The series is
Reviewed-by: Ruijing Dong 

-Original Message-
From: Wu, David 
Sent: Tuesday, May 13, 2025 3:22 PM
To: amd-gfx@lists.freedesktop.org; Koenig, Christian 
Cc: Deucher, Alexander ; Liu, Leo ; 
Jiang, Sonny ; Dong, Ruijing ; 
Limonciello, Mario 
Subject: [PATCH v3 3/3] drm/amdgpu: read back register after written for VCN 
v5.0.0

V3: patch for VCN v5.0.0 only

Similar to the previous changes made for VCN v4.0.5, the addition of register 
read-back support in VCN v5.0.0 is intended to prevent potential race 
conditions, even though such issues have not been observed yet.
This change ensures consistency across different VCN variants and helps avoid 
similar issues on newer or closely related GPUs. The overhead introduced by 
this read-back is negligible.

Signed-off-by: David (Ming Qiang) Wu 
Reviewed-by: Mario Limonciello 
---
 drivers/gpu/drm/amd/amdgpu/vcn_v5_0_0.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v5_0_0.c 
b/drivers/gpu/drm/amd/amdgpu/vcn_v5_0_0.c
index 27dcc6f37a73..77c27a317e4c 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v5_0_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v5_0_0.c
@@ -794,6 +794,10 @@ static int vcn_v5_0_0_start_dpg_mode(struct 
amdgpu_vcn_inst *vinst,
ring->doorbell_index << VCN_RB1_DB_CTRL__OFFSET__SHIFT |
VCN_RB1_DB_CTRL__EN_MASK);

+   /* Keeping one read-back to ensure all register writes are done, 
otherwise
+* it may introduce race conditions */
+   RREG32_SOC15(VCN, inst_idx, regVCN_RB1_DB_CTRL);
+
return 0;
 }

--
2.34.1

RE: [PATCH v1 1/8] drm/amdgpu: read back register after written

[Dong, Ruijing]

[AMD Official Use Only - AMD Internal Distribution Only]

This patch is not needed as it has the read-back in jpeg_v1_0_start();

Thanks,
Ruijing

-Original Message-
From: Wu, David 
Sent: Wednesday, May 14, 2025 1:23 PM
To: amd-gfx@lists.freedesktop.org; Deucher, Alexander 

Cc: Koenig, Christian ; Liu, Leo ; 
Jiang, Sonny ; Dong, Ruijing 
Subject: [PATCH v1 1/8] drm/amdgpu: read back register after written

The addition of register read-back in VCN v1.0 is intended to prevent potential 
race conditions.

Signed-off-by: David (Ming Qiang) Wu 
---
 drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c 
b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
index 21b57c29bf7d..f56b623713c9 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
@@ -1009,6 +1009,11 @@ static int vcn_v1_0_start_spg_mode(struct 
amdgpu_vcn_inst *vinst)

jpeg_v1_0_start(adev, 0);

+   /* Keeping one read-back to ensure all register writes are done, 
otherwise
+* it may introduce race conditions
+*/
+   RREG32_SOC15(UVD, 0, mmUVD_RBC_RB_WPTR);
+
return 0;
 }

@@ -1154,6 +1159,11 @@ static int vcn_v1_0_start_dpg_mode(struct 
amdgpu_vcn_inst *vinst)

jpeg_v1_0_start(adev, 1);

+   /* Keeping one read-back to ensure all register writes are done, 
otherwise
+* it may introduce race conditions
+*/
+   RREG32_SOC15(UVD, 0, mmUVD_RBC_RB_WPTR);
+
return 0;
 }

--
2.49.0

RE: [PATCH v1 2/8] drm/amdgpu: read back register after written

[Dong, Ruijing]

[AMD Official Use Only - AMD Internal Distribution Only]

Only in start() ? what about stop(), pause() and etc?

Thanks,
Ruijing

-Original Message-
From: Wu, David 
Sent: Wednesday, May 14, 2025 1:23 PM
To: amd-gfx@lists.freedesktop.org; Deucher, Alexander 

Cc: Koenig, Christian ; Liu, Leo ; 
Jiang, Sonny ; Dong, Ruijing 
Subject: [PATCH v1 2/8] drm/amdgpu: read back register after written

The addition of register read-back in VCN v2.0 is intended to prevent potential 
race conditions.

Signed-off-by: David (Ming Qiang) Wu 
---
 drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c 
b/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
index b8d835c9e17e..e6a008d7e67c 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
@@ -978,6 +978,12 @@ static int vcn_v2_0_start_dpg_mode(struct amdgpu_vcn_inst 
*vinst, bool indirect)
/* Unstall DPG */
WREG32_P(SOC15_REG_OFFSET(UVD, 0, mmUVD_POWER_STATUS),
0, ~UVD_POWER_STATUS__STALL_DPG_POWER_UP_MASK);
+
+   /* Keeping one read-back to ensure all register writes are done, 
otherwise
+* it may introduce race conditions
+*/
+   RREG32_SOC15(UVD, 0, mmUVD_RBC_RB_WPTR);
+
return 0;
 }

@@ -1152,6 +1158,11 @@ static int vcn_v2_0_start(struct amdgpu_vcn_inst *vinst)
WREG32_SOC15(UVD, 0, mmUVD_RB_SIZE2, ring->ring_size / 4);
fw_shared->multi_queue.encode_lowlatency_queue_mode &= 
~FW_QUEUE_RING_RESET;

+   /* Keeping one read-back to ensure all register writes are done, 
otherwise
+* it may introduce race conditions
+*/
+   RREG32_SOC15(UVD, 0, mmUVD_RBC_RB_WPTR);
+
return 0;
 }

--
2.49.0

RE: [PATCH v2 5/9] drm/amdgpu: read back register after written

[Dong, Ruijing]

[AMD Official Use Only - AMD Internal Distribution Only]

-Original Message-
From: Wu, David 
Sent: Thursday, May 15, 2025 12:41 PM
To: amd-gfx@lists.freedesktop.org; Koenig, Christian 
Cc: Deucher, Alexander ; Liu, Leo ; 
Jiang, Sonny ; Dong, Ruijing 
Subject: [PATCH v2 5/9] drm/amdgpu: read back register after written

The addition of register read-back in VCN v4.0.0 is intended to prevent 
potential race conditions.

Signed-off-by: David (Ming Qiang) Wu 
---
 drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c | 20 
 1 file changed, 20 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c 
b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
index 8fff470bce87..5acdf8fd5a62 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
@@ -1122,6 +1122,11 @@ static int vcn_v4_0_start_dpg_mode(struct 
amdgpu_vcn_inst *vinst, bool indirect)
ring->doorbell_index << VCN_RB1_DB_CTRL__OFFSET__SHIFT |
VCN_RB1_DB_CTRL__EN_MASK);

+   /* Keeping one read-back to ensure all register writes are done,
+* otherwise it may introduce race conditions.
+*/
+   RREG32_SOC15(VCN, inst_idx, regUVD_RB_WPTR);
+



Use the same register regUVD_STATUS?


return 0;
 }

@@ -1303,6 +1308,11 @@ static int vcn_v4_0_start(struct amdgpu_vcn_inst *vinst)
WREG32_SOC15(VCN, i, regVCN_RB_ENABLE, tmp);
fw_shared->sq.queue_mode &= ~(FW_QUEUE_RING_RESET | 
FW_QUEUE_DPG_HOLD_OFF);

+   /* Keeping one read-back to ensure all register writes are done,
+* otherwise it may introduce race conditions.
+*/
+   RREG32_SOC15(VCN, i, regUVD_RB_WPTR);
+
return 0;
 }

@@ -1583,6 +1593,11 @@ static void vcn_v4_0_stop_dpg_mode(struct 
amdgpu_vcn_inst *vinst)
/* disable dynamic power gating mode */
WREG32_P(SOC15_REG_OFFSET(VCN, inst_idx, regUVD_POWER_STATUS), 0,
~UVD_POWER_STATUS__UVD_PG_MODE_MASK);
+
+   /* Keeping one read-back to ensure all register writes are done,
+* otherwise it may introduce race conditions.
+*/
+   RREG32_SOC15(VCN, inst_idx, regUVD_STATUS);
 }

 /**
@@ -1666,6 +1681,11 @@ static int vcn_v4_0_stop(struct amdgpu_vcn_inst *vinst)
/* enable VCN power gating */
vcn_v4_0_enable_static_power_gating(vinst);

+   /* Keeping one read-back to ensure all register writes are done,
+* otherwise it may introduce race conditions.
+*/
+   RREG32_SOC15(VCN, i, regUVD_STATUS);
+
 done:
if (adev->pm.dpm_enabled)
amdgpu_dpm_enable_vcn(adev, false, i);
--
2.49.0

RE: [PATCH v2 5/9] drm/amdgpu: read back register after written

[Dong, Ruijing]

[AMD Official Use Only - AMD Internal Distribution Only]

With the change,

The series is
Reviewed-by: Ruijing Dong 

Thanks,
Ruijing

-Original Message-
From: Wu, David 
Sent: Thursday, May 15, 2025 1:30 PM
To: Dong, Ruijing ; Wu, David ; 
amd-gfx@lists.freedesktop.org; Koenig, Christian 
Cc: Deucher, Alexander ; Liu, Leo ; 
Jiang, Sonny 
Subject: Re: [PATCH v2 5/9] drm/amdgpu: read back register after written


On 2025-05-15 13:25, Dong, Ruijing wrote:
> [AMD Official Use Only - AMD Internal Distribution Only]
>
> -Original Message-
> From: Wu, David 
> Sent: Thursday, May 15, 2025 12:41 PM
> To: amd-gfx@lists.freedesktop.org; Koenig, Christian 
> 
> Cc: Deucher, Alexander ; Liu, Leo 
> ; Jiang, Sonny ; Dong, Ruijing 
> 
> Subject: [PATCH v2 5/9] drm/amdgpu: read back register after written
>
> The addition of register read-back in VCN v4.0.0 is intended to prevent 
> potential race conditions.
>
> Signed-off-by: David (Ming Qiang) Wu 
> ---
>   drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c | 20 
>   1 file changed, 20 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c 
> b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
> index 8fff470bce87..5acdf8fd5a62 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
> @@ -1122,6 +1122,11 @@ static int vcn_v4_0_start_dpg_mode(struct 
> amdgpu_vcn_inst *vinst, bool indirect)
>  ring->doorbell_index << 
> VCN_RB1_DB_CTRL__OFFSET__SHIFT |
>  VCN_RB1_DB_CTRL__EN_MASK);
>
> +   /* Keeping one read-back to ensure all register writes are done,
> +* otherwise it may introduce race conditions.
> +*/
> +   RREG32_SOC15(VCN, inst_idx, regUVD_RB_WPTR);
> +
>
>
>
> Use the same register regUVD_STATUS?
good catch - I will change them.
David
>
>
>  return 0;
>   }
>
> @@ -1303,6 +1308,11 @@ static int vcn_v4_0_start(struct amdgpu_vcn_inst 
> *vinst)
>  WREG32_SOC15(VCN, i, regVCN_RB_ENABLE, tmp);
>  fw_shared->sq.queue_mode &= ~(FW_QUEUE_RING_RESET | 
> FW_QUEUE_DPG_HOLD_OFF);
>
> +   /* Keeping one read-back to ensure all register writes are done,
> +* otherwise it may introduce race conditions.
> +*/
> +   RREG32_SOC15(VCN, i, regUVD_RB_WPTR);
> +
>  return 0;
>   }
>
> @@ -1583,6 +1593,11 @@ static void vcn_v4_0_stop_dpg_mode(struct 
> amdgpu_vcn_inst *vinst)
>  /* disable dynamic power gating mode */
>  WREG32_P(SOC15_REG_OFFSET(VCN, inst_idx, regUVD_POWER_STATUS), 0,
>  ~UVD_POWER_STATUS__UVD_PG_MODE_MASK);
> +
> +   /* Keeping one read-back to ensure all register writes are done,
> +* otherwise it may introduce race conditions.
> +*/
> +   RREG32_SOC15(VCN, inst_idx, regUVD_STATUS);
>   }
>
>   /**
> @@ -1666,6 +1681,11 @@ static int vcn_v4_0_stop(struct amdgpu_vcn_inst *vinst)
>  /* enable VCN power gating */
>  vcn_v4_0_enable_static_power_gating(vinst);
>
> +   /* Keeping one read-back to ensure all register writes are done,
> +* otherwise it may introduce race conditions.
> +*/
> +   RREG32_SOC15(VCN, i, regUVD_STATUS);
> +
>   done:
>  if (adev->pm.dpm_enabled)
>  amdgpu_dpm_enable_vcn(adev, false, i);
> --
> 2.49.0
>

RE: [PATCH v3 9/9] drm/amdgpu: read back register after written

[Dong, Ruijing]

[AMD Official Use Only - AMD Internal Distribution Only]

The series is

`Reviewed-by: Ruijing Dong `


-Original Message-
From: Wu, David 
Sent: Wednesday, May 21, 2025 4:24 PM
To: amd-gfx@lists.freedesktop.org; Koenig, Christian 
Cc: Deucher, Alexander ; Liu, Leo ; 
Jiang, Sonny ; Dong, Ruijing 
Subject: [PATCH v3 9/9] drm/amdgpu: read back register after written

The addition of register read-back in VCN v5.0.1 is intended to prevent 
potential race conditions.

Signed-off-by: David (Ming Qiang) Wu 
---
 drivers/gpu/drm/amd/amdgpu/vcn_v5_0_1.c | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v5_0_1.c 
b/drivers/gpu/drm/amd/amdgpu/vcn_v5_0_1.c
index 1e9d2aedf2799..338cf43c45fe7 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v5_0_1.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v5_0_1.c
@@ -1038,6 +1038,11 @@ static int vcn_v5_0_1_start(struct amdgpu_vcn_inst 
*vinst)
WREG32_SOC15(VCN, vcn_inst, regVCN_RB_ENABLE, tmp);
fw_shared->sq.queue_mode &= ~(FW_QUEUE_RING_RESET | 
FW_QUEUE_DPG_HOLD_OFF);

+   /* Keeping one read-back to ensure all register writes are done,
+* otherwise it may introduce race conditions.
+*/
+   RREG32_SOC15(VCN, vcn_inst, regUVD_STATUS);
+
return 0;
 }

@@ -1072,6 +1077,11 @@ static void vcn_v5_0_1_stop_dpg_mode(struct 
amdgpu_vcn_inst *vinst)
/* disable dynamic power gating mode */
WREG32_P(SOC15_REG_OFFSET(VCN, vcn_inst, regUVD_POWER_STATUS), 0,
~UVD_POWER_STATUS__UVD_PG_MODE_MASK);
+
+   /* Keeping one read-back to ensure all register writes are done,
+* otherwise it may introduce race conditions.
+*/
+   RREG32_SOC15(VCN, vcn_inst, regUVD_STATUS);
 }

 /**
@@ -1147,6 +1157,11 @@ static int vcn_v5_0_1_stop(struct amdgpu_vcn_inst *vinst)
/* clear status */
WREG32_SOC15(VCN, vcn_inst, regUVD_STATUS, 0);

+   /* Keeping one read-back to ensure all register writes are done,
+* otherwise it may introduce race conditions.
+*/
+   RREG32_SOC15(VCN, vcn_inst, regUVD_STATUS);
+
return 0;
 }

--
2.34.1