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

Timur Kristóf <[email protected]>
Newsgroups org.freedesktop.lists.amd-gfx
Message-ID <2299222.Hq7AAxBmiT@timur-max>
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 <[email protected]>
> > ---
> > 
> >   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 <[email protected]>
> 
> 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,
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.