[PATCH 1/6] drm/amd/amdgpu: Add a helper to copy the CSB buffer into the ring buffer

[Rodrigo Siqueira]

On 05/05, Christian König wrote:
> On 5/4/25 23:47, Rodrigo Siqueira wrote:
> > In the GFX code, there are multiple parsers of the CSB buffer, which can
> > be avoided. This data is parsed via get_csb_buffer() in earlier stages,
> > and the result can be checked in "adev->gfx.rlc.cs_ptr". To avoid
> > re-parser the CSB buffer, this commit introduces a helper that copies
> > the CSB buffer into the ring buffer.
> > 
> > Signed-off-by: Rodrigo Siqueira <siqueira at igalia.com>
> > ---
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 21 +++++++++++++++++++++
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h |  4 ++++
> >  2 files changed, 25 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> > index 8f1a2f7b03c1..dfd48670a0bf 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> > @@ -2323,6 +2323,27 @@ void amdgpu_gfx_csb_preamble_end(volatile u32 *buffer, u32 count)
> >  	buffer[count++] = cpu_to_le32(0);
> >  }
> >  
> > +/**
> > + * amdgpu_gfx_write_csb_to_ring - Write the CSB buffer into the ring
> > + *
> > + * @ring: Ring reference.
> > + * @csb_buffer: CSB buffer.
> > + * @csb_size: CSB buffer size.
> > + *
> > + * Usually, the adev->gfx.rlc.cs_ptr field is filled in earlier stages via
> > + * get_csb_buffer(). This function just gets the CSB buffer and fills it in the
> > + * ring buffer.
> > + */
> > +void amdgpu_gfx_write_csb_to_ring(struct amdgpu_ring *ring,
> 
> We already have the amdgpu_ring_write_multiple() function for exactly that.

I'll prepare a V2 that uses amdgpu_ring_write_multiple().

> 
> > +				  volatile u32 *csb_buffer,
> 
> Please drop volatile from all pointers inside the kernel and point me out if you still find some in existing code.
> 
> They are nearly always used incorrectly, see here https://docs.kernel.org/process/volatile-considered-harmful.html

Thanks for the link. I'll make a separate patchset for that.

Thanks
Siqueira

> 
> Thanks,
> Christian.
> 
> 
> > +				  u32 csb_size)
> 
> 
> 
> 
> > +{
> > +	int i;
> > +
> > +	for (i = 0; i < csb_size; i++)
> > +		amdgpu_ring_write(ring, csb_buffer[i]);
> > +}
> > +
> >  /*
> >   * debugfs for to enable/disable gfx job submission to specific core.
> >   */
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> > index 08f268dab8f5..ce684c3d3d89 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> > @@ -646,6 +646,10 @@ u32 amdgpu_gfx_csb_preamble_start(volatile u32 *buffer);
> >  u32 amdgpu_gfx_csb_data_parser(struct amdgpu_device *adev, volatile u32 *buffer, u32 count);
> >  void amdgpu_gfx_csb_preamble_end(volatile u32 *buffer, u32 count);
> >  
> > +void amdgpu_gfx_write_csb_to_ring(struct amdgpu_ring *ring,
> > +				  volatile u32 *csb_buffer,
> > +				  u32 csb_size);
> > +
> >  void amdgpu_debugfs_gfx_sched_mask_init(struct amdgpu_device *adev);
> >  void amdgpu_debugfs_compute_sched_mask_init(struct amdgpu_device *adev);
> >  
> 

-- 
Rodrigo Siqueira