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

[Christian König]

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.

> +				  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,
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);
>