* [PATCH] Revert "drm/amdgpu: Implement insert_end for VCE 3"
@ 2026-08-14 14:54 David Rosca
2026-08-14 15:08 ` Alex Deucher
0 siblings, 1 reply; 4+ messages in thread
From: David Rosca @ 2026-08-14 14:54 UTC (permalink / raw)
To: amd-gfx; +Cc: David Rosca
This doesn't solve the issue, VCE still hangs in some cases.
4959138b6461 ("drm/amdgpu: handle GDS and SPM without a VM fence")
fixes the regression, originally introduced in
83a8dee2bdfb ("drm/amdgpu: always emit the job vm fence").
This reverts commit 7591335141a06d5b176510054576d9c64519f348.
Signed-off-by: David Rosca <david.rosca@amd.com>
---
drivers/gpu/drm/amd/amdgpu/vce_v3_0.c | 26 ++++----------------------
1 file changed, 4 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c b/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
index a9497e2e07f7..9f4e88440c0a 100644
--- a/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
@@ -809,23 +809,6 @@ static void vce_v3_0_ring_emit_ib(struct amdgpu_ring *ring,
amdgpu_ring_write(ring, ib->length_dw);
}
-static void vce_v3_0_ring_emit_fence(struct amdgpu_ring *ring, u64 addr,
- u64 seq, unsigned flags)
-{
- WARN_ON(flags & AMDGPU_FENCE_FLAG_64BIT);
-
- amdgpu_ring_write(ring, VCE_CMD_FENCE);
- amdgpu_ring_write(ring, addr);
- amdgpu_ring_write(ring, upper_32_bits(addr));
- amdgpu_ring_write(ring, seq);
- amdgpu_ring_write(ring, VCE_CMD_TRAP);
-}
-
-static void vce_v3_0_ring_insert_end(struct amdgpu_ring *ring)
-{
- amdgpu_ring_write(ring, VCE_CMD_END);
-}
-
static void vce_v3_0_emit_vm_flush(struct amdgpu_ring *ring,
unsigned int vmid, uint64_t pd_addr)
{
@@ -835,6 +818,7 @@ static void vce_v3_0_emit_vm_flush(struct amdgpu_ring *ring,
amdgpu_ring_write(ring, VCE_CMD_FLUSH_TLB);
amdgpu_ring_write(ring, vmid);
+ amdgpu_ring_write(ring, VCE_CMD_END);
}
static void vce_v3_0_emit_pipeline_sync(struct amdgpu_ring *ring)
@@ -900,19 +884,17 @@ static const struct amdgpu_ring_funcs vce_v3_0_ring_vm_funcs = {
.set_wptr = vce_v3_0_ring_set_wptr,
.patch_cs_in_place = amdgpu_vce_ring_parse_cs_vm,
.emit_frame_size =
- 5 + /* vce_v3_0_emit_vm_flush */
+ 6 + /* vce_v3_0_emit_vm_flush */
4 + /* vce_v3_0_emit_pipeline_sync */
- 5 + 5 + /* vce_v3_0_ring_emit_fence x2 vm fence */
- 1, /* vce_v3_0_ring_insert_end */
+ 6 + 6, /* amdgpu_vce_ring_emit_fence x2 vm fence */
.emit_ib_size = 5, /* vce_v3_0_ring_emit_ib */
.emit_ib = vce_v3_0_ring_emit_ib,
.emit_vm_flush = vce_v3_0_emit_vm_flush,
.emit_pipeline_sync = vce_v3_0_emit_pipeline_sync,
- .emit_fence = vce_v3_0_ring_emit_fence,
+ .emit_fence = amdgpu_vce_ring_emit_fence,
.test_ring = amdgpu_vce_ring_test_ring,
.test_ib = amdgpu_vce_ring_test_ib,
.insert_nop = amdgpu_ring_insert_nop,
- .insert_end = vce_v3_0_ring_insert_end,
.pad_ib = amdgpu_ring_generic_pad_ib,
.begin_use = amdgpu_vce_ring_begin_use,
.end_use = amdgpu_vce_ring_end_use,
--
2.43.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] Revert "drm/amdgpu: Implement insert_end for VCE 3"
2026-08-14 14:54 [PATCH] Revert "drm/amdgpu: Implement insert_end for VCE 3" David Rosca
@ 2026-08-14 15:08 ` Alex Deucher
2026-08-14 15:13 ` David Rosca
0 siblings, 1 reply; 4+ messages in thread
From: Alex Deucher @ 2026-08-14 15:08 UTC (permalink / raw)
To: David Rosca; +Cc: amd-gfx
On Fri, Aug 14, 2026 at 11:05 AM David Rosca <david.rosca@amd.com> wrote:
>
> This doesn't solve the issue, VCE still hangs in some cases.
>
> 4959138b6461 ("drm/amdgpu: handle GDS and SPM without a VM fence")
> fixes the regression, originally introduced in
> 83a8dee2bdfb ("drm/amdgpu: always emit the job vm fence").
This doesn't make sense to me. GDS and SPM are not supported on VCN
in the first place so they would never be emitted for VCN.
Alex
>
> This reverts commit 7591335141a06d5b176510054576d9c64519f348.
>
> Signed-off-by: David Rosca <david.rosca@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/vce_v3_0.c | 26 ++++----------------------
> 1 file changed, 4 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c b/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
> index a9497e2e07f7..9f4e88440c0a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
> @@ -809,23 +809,6 @@ static void vce_v3_0_ring_emit_ib(struct amdgpu_ring *ring,
> amdgpu_ring_write(ring, ib->length_dw);
> }
>
> -static void vce_v3_0_ring_emit_fence(struct amdgpu_ring *ring, u64 addr,
> - u64 seq, unsigned flags)
> -{
> - WARN_ON(flags & AMDGPU_FENCE_FLAG_64BIT);
> -
> - amdgpu_ring_write(ring, VCE_CMD_FENCE);
> - amdgpu_ring_write(ring, addr);
> - amdgpu_ring_write(ring, upper_32_bits(addr));
> - amdgpu_ring_write(ring, seq);
> - amdgpu_ring_write(ring, VCE_CMD_TRAP);
> -}
> -
> -static void vce_v3_0_ring_insert_end(struct amdgpu_ring *ring)
> -{
> - amdgpu_ring_write(ring, VCE_CMD_END);
> -}
> -
> static void vce_v3_0_emit_vm_flush(struct amdgpu_ring *ring,
> unsigned int vmid, uint64_t pd_addr)
> {
> @@ -835,6 +818,7 @@ static void vce_v3_0_emit_vm_flush(struct amdgpu_ring *ring,
>
> amdgpu_ring_write(ring, VCE_CMD_FLUSH_TLB);
> amdgpu_ring_write(ring, vmid);
> + amdgpu_ring_write(ring, VCE_CMD_END);
> }
>
> static void vce_v3_0_emit_pipeline_sync(struct amdgpu_ring *ring)
> @@ -900,19 +884,17 @@ static const struct amdgpu_ring_funcs vce_v3_0_ring_vm_funcs = {
> .set_wptr = vce_v3_0_ring_set_wptr,
> .patch_cs_in_place = amdgpu_vce_ring_parse_cs_vm,
> .emit_frame_size =
> - 5 + /* vce_v3_0_emit_vm_flush */
> + 6 + /* vce_v3_0_emit_vm_flush */
> 4 + /* vce_v3_0_emit_pipeline_sync */
> - 5 + 5 + /* vce_v3_0_ring_emit_fence x2 vm fence */
> - 1, /* vce_v3_0_ring_insert_end */
> + 6 + 6, /* amdgpu_vce_ring_emit_fence x2 vm fence */
> .emit_ib_size = 5, /* vce_v3_0_ring_emit_ib */
> .emit_ib = vce_v3_0_ring_emit_ib,
> .emit_vm_flush = vce_v3_0_emit_vm_flush,
> .emit_pipeline_sync = vce_v3_0_emit_pipeline_sync,
> - .emit_fence = vce_v3_0_ring_emit_fence,
> + .emit_fence = amdgpu_vce_ring_emit_fence,
> .test_ring = amdgpu_vce_ring_test_ring,
> .test_ib = amdgpu_vce_ring_test_ib,
> .insert_nop = amdgpu_ring_insert_nop,
> - .insert_end = vce_v3_0_ring_insert_end,
> .pad_ib = amdgpu_ring_generic_pad_ib,
> .begin_use = amdgpu_vce_ring_begin_use,
> .end_use = amdgpu_vce_ring_end_use,
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] Revert "drm/amdgpu: Implement insert_end for VCE 3"
2026-08-14 15:08 ` Alex Deucher
@ 2026-08-14 15:13 ` David Rosca
2026-08-14 15:31 ` Alex Deucher
0 siblings, 1 reply; 4+ messages in thread
From: David Rosca @ 2026-08-14 15:13 UTC (permalink / raw)
To: Alex Deucher; +Cc: amd-gfx
On 8/14/26 17:08, Alex Deucher wrote:
> On Fri, Aug 14, 2026 at 11:05 AM David Rosca <david.rosca@amd.com> wrote:
>> This doesn't solve the issue, VCE still hangs in some cases.
>>
>> 4959138b6461 ("drm/amdgpu: handle GDS and SPM without a VM fence")
>> fixes the regression, originally introduced in
>> 83a8dee2bdfb ("drm/amdgpu: always emit the job vm fence").
> This doesn't make sense to me. GDS and SPM are not supported on VCN
> in the first place so they would never be emitted for VCN.
The issue was the extra fence when only pipeline sync was needed.
With 4959138b6461 ("drm/amdgpu: handle GDS and SPM without a VM fence")
the fence is not emitted in that case.
David
>
> Alex
>
>> This reverts commit 7591335141a06d5b176510054576d9c64519f348.
>>
>> Signed-off-by: David Rosca <david.rosca@amd.com>
>> ---
>> drivers/gpu/drm/amd/amdgpu/vce_v3_0.c | 26 ++++----------------------
>> 1 file changed, 4 insertions(+), 22 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c b/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
>> index a9497e2e07f7..9f4e88440c0a 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
>> @@ -809,23 +809,6 @@ static void vce_v3_0_ring_emit_ib(struct amdgpu_ring *ring,
>> amdgpu_ring_write(ring, ib->length_dw);
>> }
>>
>> -static void vce_v3_0_ring_emit_fence(struct amdgpu_ring *ring, u64 addr,
>> - u64 seq, unsigned flags)
>> -{
>> - WARN_ON(flags & AMDGPU_FENCE_FLAG_64BIT);
>> -
>> - amdgpu_ring_write(ring, VCE_CMD_FENCE);
>> - amdgpu_ring_write(ring, addr);
>> - amdgpu_ring_write(ring, upper_32_bits(addr));
>> - amdgpu_ring_write(ring, seq);
>> - amdgpu_ring_write(ring, VCE_CMD_TRAP);
>> -}
>> -
>> -static void vce_v3_0_ring_insert_end(struct amdgpu_ring *ring)
>> -{
>> - amdgpu_ring_write(ring, VCE_CMD_END);
>> -}
>> -
>> static void vce_v3_0_emit_vm_flush(struct amdgpu_ring *ring,
>> unsigned int vmid, uint64_t pd_addr)
>> {
>> @@ -835,6 +818,7 @@ static void vce_v3_0_emit_vm_flush(struct amdgpu_ring *ring,
>>
>> amdgpu_ring_write(ring, VCE_CMD_FLUSH_TLB);
>> amdgpu_ring_write(ring, vmid);
>> + amdgpu_ring_write(ring, VCE_CMD_END);
>> }
>>
>> static void vce_v3_0_emit_pipeline_sync(struct amdgpu_ring *ring)
>> @@ -900,19 +884,17 @@ static const struct amdgpu_ring_funcs vce_v3_0_ring_vm_funcs = {
>> .set_wptr = vce_v3_0_ring_set_wptr,
>> .patch_cs_in_place = amdgpu_vce_ring_parse_cs_vm,
>> .emit_frame_size =
>> - 5 + /* vce_v3_0_emit_vm_flush */
>> + 6 + /* vce_v3_0_emit_vm_flush */
>> 4 + /* vce_v3_0_emit_pipeline_sync */
>> - 5 + 5 + /* vce_v3_0_ring_emit_fence x2 vm fence */
>> - 1, /* vce_v3_0_ring_insert_end */
>> + 6 + 6, /* amdgpu_vce_ring_emit_fence x2 vm fence */
>> .emit_ib_size = 5, /* vce_v3_0_ring_emit_ib */
>> .emit_ib = vce_v3_0_ring_emit_ib,
>> .emit_vm_flush = vce_v3_0_emit_vm_flush,
>> .emit_pipeline_sync = vce_v3_0_emit_pipeline_sync,
>> - .emit_fence = vce_v3_0_ring_emit_fence,
>> + .emit_fence = amdgpu_vce_ring_emit_fence,
>> .test_ring = amdgpu_vce_ring_test_ring,
>> .test_ib = amdgpu_vce_ring_test_ib,
>> .insert_nop = amdgpu_ring_insert_nop,
>> - .insert_end = vce_v3_0_ring_insert_end,
>> .pad_ib = amdgpu_ring_generic_pad_ib,
>> .begin_use = amdgpu_vce_ring_begin_use,
>> .end_use = amdgpu_vce_ring_end_use,
>> --
>> 2.43.0
>>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] Revert "drm/amdgpu: Implement insert_end for VCE 3"
2026-08-14 15:13 ` David Rosca
@ 2026-08-14 15:31 ` Alex Deucher
0 siblings, 0 replies; 4+ messages in thread
From: Alex Deucher @ 2026-08-14 15:31 UTC (permalink / raw)
To: David Rosca; +Cc: amd-gfx
On Fri, Aug 14, 2026 at 11:13 AM David Rosca <david.rosca@amd.com> wrote:
>
>
> On 8/14/26 17:08, Alex Deucher wrote:
> > On Fri, Aug 14, 2026 at 11:05 AM David Rosca <david.rosca@amd.com> wrote:
> >> This doesn't solve the issue, VCE still hangs in some cases.
> >>
> >> 4959138b6461 ("drm/amdgpu: handle GDS and SPM without a VM fence")
> >> fixes the regression, originally introduced in
> >> 83a8dee2bdfb ("drm/amdgpu: always emit the job vm fence").
> > This doesn't make sense to me. GDS and SPM are not supported on VCN
> > in the first place so they would never be emitted for VCN.
>
> The issue was the extra fence when only pipeline sync was needed.
> With 4959138b6461 ("drm/amdgpu: handle GDS and SPM without a VM fence")
> the fence is not emitted in that case.
If only the pipeline sync is needed that should also be moved up to
the caller as well. I'll send a patch to fix that.
Alex
>
> David
>
> >
> > Alex
> >
> >> This reverts commit 7591335141a06d5b176510054576d9c64519f348.
> >>
> >> Signed-off-by: David Rosca <david.rosca@amd.com>
> >> ---
> >> drivers/gpu/drm/amd/amdgpu/vce_v3_0.c | 26 ++++----------------------
> >> 1 file changed, 4 insertions(+), 22 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c b/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
> >> index a9497e2e07f7..9f4e88440c0a 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
> >> @@ -809,23 +809,6 @@ static void vce_v3_0_ring_emit_ib(struct amdgpu_ring *ring,
> >> amdgpu_ring_write(ring, ib->length_dw);
> >> }
> >>
> >> -static void vce_v3_0_ring_emit_fence(struct amdgpu_ring *ring, u64 addr,
> >> - u64 seq, unsigned flags)
> >> -{
> >> - WARN_ON(flags & AMDGPU_FENCE_FLAG_64BIT);
> >> -
> >> - amdgpu_ring_write(ring, VCE_CMD_FENCE);
> >> - amdgpu_ring_write(ring, addr);
> >> - amdgpu_ring_write(ring, upper_32_bits(addr));
> >> - amdgpu_ring_write(ring, seq);
> >> - amdgpu_ring_write(ring, VCE_CMD_TRAP);
> >> -}
> >> -
> >> -static void vce_v3_0_ring_insert_end(struct amdgpu_ring *ring)
> >> -{
> >> - amdgpu_ring_write(ring, VCE_CMD_END);
> >> -}
> >> -
> >> static void vce_v3_0_emit_vm_flush(struct amdgpu_ring *ring,
> >> unsigned int vmid, uint64_t pd_addr)
> >> {
> >> @@ -835,6 +818,7 @@ static void vce_v3_0_emit_vm_flush(struct amdgpu_ring *ring,
> >>
> >> amdgpu_ring_write(ring, VCE_CMD_FLUSH_TLB);
> >> amdgpu_ring_write(ring, vmid);
> >> + amdgpu_ring_write(ring, VCE_CMD_END);
> >> }
> >>
> >> static void vce_v3_0_emit_pipeline_sync(struct amdgpu_ring *ring)
> >> @@ -900,19 +884,17 @@ static const struct amdgpu_ring_funcs vce_v3_0_ring_vm_funcs = {
> >> .set_wptr = vce_v3_0_ring_set_wptr,
> >> .patch_cs_in_place = amdgpu_vce_ring_parse_cs_vm,
> >> .emit_frame_size =
> >> - 5 + /* vce_v3_0_emit_vm_flush */
> >> + 6 + /* vce_v3_0_emit_vm_flush */
> >> 4 + /* vce_v3_0_emit_pipeline_sync */
> >> - 5 + 5 + /* vce_v3_0_ring_emit_fence x2 vm fence */
> >> - 1, /* vce_v3_0_ring_insert_end */
> >> + 6 + 6, /* amdgpu_vce_ring_emit_fence x2 vm fence */
> >> .emit_ib_size = 5, /* vce_v3_0_ring_emit_ib */
> >> .emit_ib = vce_v3_0_ring_emit_ib,
> >> .emit_vm_flush = vce_v3_0_emit_vm_flush,
> >> .emit_pipeline_sync = vce_v3_0_emit_pipeline_sync,
> >> - .emit_fence = vce_v3_0_ring_emit_fence,
> >> + .emit_fence = amdgpu_vce_ring_emit_fence,
> >> .test_ring = amdgpu_vce_ring_test_ring,
> >> .test_ib = amdgpu_vce_ring_test_ib,
> >> .insert_nop = amdgpu_ring_insert_nop,
> >> - .insert_end = vce_v3_0_ring_insert_end,
> >> .pad_ib = amdgpu_ring_generic_pad_ib,
> >> .begin_use = amdgpu_vce_ring_begin_use,
> >> .end_use = amdgpu_vce_ring_end_use,
> >> --
> >> 2.43.0
> >>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-08-14 15:31 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-14 14:54 [PATCH] Revert "drm/amdgpu: Implement insert_end for VCE 3" David Rosca
2026-08-14 15:08 ` Alex Deucher
2026-08-14 15:13 ` David Rosca
2026-08-14 15:31 ` Alex Deucher
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.