Re: [PATCH 7/9] drm/amdgpu/gfx7: Use COND_EXEC

["Timur Kristóf" <timur.kristof@gmail.com>]

On 2026. július 15., szerda 11:38:40 közép-európai nyári idő Tvrtko Ursulin 
wrote:
> On 13/07/2026 13:58, Timur Kristóf wrote:
> > COND_EXEC tells the CP to discard the dwords following it
> > when its condition is zero (false).
> > 
> > This is useful for GPU recovery because it can help reduce
> > collateral damage during GFX IP block soft reset, meaning
> > that it reduces the likelyhood that we fail some jobs which
> > are not guilty of the hang as the IP block soft reset
> > mechanism clears the condition before doing the reset.
> > 
> > Signed-off-by: Timur Kristóf <timur.kristof@gmail.com>
> > ---
> > 
> >   drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c | 22 ++++++++++++++++++++++
> >   1 file changed, 22 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> > b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c index 915612628f9a..6d52b8710437
> > 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> > @@ -3162,6 +3162,22 @@ static void gfx_v7_0_ring_emit_vm_flush(struct
> > amdgpu_ring *ring,> 
> >   	}
> >   
> >   }
> > 
> > +static unsigned int gfx_v7_0_ring_emit_init_cond_exec(struct amdgpu_ring
> > *ring, +						      
uint64_t gpu_addr)
> > +{
> > +	unsigned int ret;
> > +
> > +	/* Discard following DWs after this packet when gpu_addr==0 */
> > +	amdgpu_ring_write(ring, PACKET3(PACKET3_COND_EXEC, 3));
> > +	amdgpu_ring_write(ring, lower_32_bits(gpu_addr));
> > +	amdgpu_ring_write(ring, upper_32_bits(gpu_addr));
> > +	amdgpu_ring_write(ring, 0);
> > +	ret = ring->wptr & ring->buf_mask;
> > +	/* patch dummy value later */
> > +	amdgpu_ring_write(ring, 0);
> > +	return ret;
> > +}
> > +
> > 
> >   static void gfx_v7_0_ring_emit_wreg(struct amdgpu_ring *ring,
> >   
> >   				    uint32_t reg, uint32_t val)
> >   
> >   {
> > 
> > @@ -4943,6 +4959,8 @@ static const struct amdgpu_ring_funcs
> > gfx_v7_0_ring_funcs_gfx = {> 
> >   	.get_wptr = gfx_v7_0_ring_get_wptr_gfx,
> >   	.set_wptr = gfx_v7_0_ring_set_wptr_gfx,
> >   	.emit_frame_size =
> > 
> > +		5 + /* gfx_v7_0_ring_emit_init_cond_exec (from 
amdgpu_ib_schedule) */
> > +		5 + /* gfx_v7_0_ring_emit_init_cond_exec (from 
amdgpu_vm_flush) */
> 
> gfx8 version /appears/ to make an effort to keep the additions here
> sorted in the emission order. I think at least.

As far as I see it was already not in emission order here.

> Not sure how much value
> it adds since the whole emit_frame_size mechanism is "fragilotedious".
> 
> :) Another task for a rainy day - we see if it can be replaced with
> 
> something more manageable.

If you have ideas how to clean it up, I'd be happy to hear it.

> Anyway, digressions aside:
> 
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
> 
> Regards,
> 
> Tvrtko
> 
> >   		20 + /* gfx_v7_0_ring_emit_gds_switch */
> >   		7 + /* gfx_v7_0_ring_emit_hdp_flush */
> >   		5 + /* hdp invalidate */
> > 
> > @@ -4965,6 +4983,7 @@ static const struct amdgpu_ring_funcs
> > gfx_v7_0_ring_funcs_gfx = {> 
> >   	.pad_ib = amdgpu_ring_generic_pad_ib,
> >   	.emit_switch_buffer = gfx_v7_0_ring_emit_sb,
> >   	.emit_cntxcntl = gfx_v7_ring_emit_cntxcntl,
> > 
> > +	.init_cond_exec = gfx_v7_0_ring_emit_init_cond_exec,
> > 
> >   	.emit_wreg = gfx_v7_0_ring_emit_wreg,
> >   	.soft_recovery = gfx_v7_0_ring_soft_recovery,
> >   	.emit_mem_sync = gfx_v7_0_emit_mem_sync,
> > 
> > @@ -4979,6 +4998,8 @@ static const struct amdgpu_ring_funcs
> > gfx_v7_0_ring_funcs_compute = {> 
> >   	.get_wptr = gfx_v7_0_ring_get_wptr_compute,
> >   	.set_wptr = gfx_v7_0_ring_set_wptr_compute,
> >   	.emit_frame_size =
> > 
> > +		5 + /* gfx_v7_0_ring_emit_init_cond_exec (from 
amdgpu_ib_schedule) */
> > +		5 + /* gfx_v7_0_ring_emit_init_cond_exec (from 
amdgpu_vm_flush) */
> > 
> >   		20 + /* gfx_v7_0_ring_emit_gds_switch */
> >   		7 + /* gfx_v7_0_ring_emit_hdp_flush */
> >   		5 + /* hdp invalidate */
> > 
> > @@ -4997,6 +5018,7 @@ static const struct amdgpu_ring_funcs
> > gfx_v7_0_ring_funcs_compute = {> 
> >   	.test_ib = gfx_v7_0_ring_test_ib,
> >   	.insert_nop = amdgpu_ring_insert_nop,
> >   	.pad_ib = amdgpu_ring_generic_pad_ib,
> > 
> > +	.init_cond_exec = gfx_v7_0_ring_emit_init_cond_exec,
> > 
> >   	.emit_wreg = gfx_v7_0_ring_emit_wreg,
> >   	.soft_recovery = gfx_v7_0_ring_soft_recovery,
> >   	.emit_mem_sync = gfx_v7_0_emit_mem_sync_compute,