On Tue, 2025-09-02 at 08:41 +0200, Christian König wrote:
> On 01.09.25 12:00, Timur Kristóf wrote:
> > The amdgpu_bo_create_kernel function takes a byte count,
> > so we need to multiply the extra dword count by four.
> > (The ring_size is already in bytes so that one is correct here.)
>
> Good catch, it just doesn't make a difference in practice since
> everything is rounded up to 4k anyway.
Yes. It looks like extra_dw is only used by jpeg_v1_0 and vcn_v4_0 and
fortunately both of these are small enough that it doesn't cause a
difference in practice.
I'd still like to fix it though to avoid potential issues in the
future.
>
> But I'm really wondering if we shouldn't replace the extra_dw with
> extra_bytes instead.
It's up to you. I'm happy to change it to extra_bytes if you think
that's better. I would prefer to keep extra_dw for two reasons:
- Avoid unnecessary churn
- Most of the code related to rings are in dwords, so IMO it's better
to have the extra space in dwords too for consistency.
>
> It should only be used by some multimedia engines anyway.
Right. vcn_v4_0 has this:
.extra_dw = sizeof(struct amdgpu_vcn_rb_metadata)
Which would also need to be fixed because it's not really in dwords.
> >
> > Fixes: c8c1a1d2ef04 ("drm/amdgpu: define and add extra dword for
> > jpeg ring")
> > Signed-off-by: Timur Kristóf <[email protected]>
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c | 11 ++++++-----
> > 1 file changed, 6 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> > index 6379bb25bf5c..13f0f0209cbe 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> > @@ -364,11 +364,12 @@ int amdgpu_ring_init(struct amdgpu_device
> > *adev, struct amdgpu_ring *ring,
> >
> > /* Allocate ring buffer */
> > if (ring->ring_obj == NULL) {
> > - r = amdgpu_bo_create_kernel(adev, ring->ring_size
> > + ring->funcs->extra_dw, PAGE_SIZE,
> > - AMDGPU_GEM_DOMAIN_GTT,
> > - &ring->ring_obj,
> > - &ring->gpu_addr,
> > - (void **)&ring->ring);
> > + r = amdgpu_bo_create_kernel(adev, ring->ring_size
> > + ring->funcs->extra_dw * 4,
> > + PAGE_SIZE,
> > + AMDGPU_GEM_DOMAIN_
> > GTT,
> > + &ring->ring_obj,
> > + &ring->gpu_addr,
> > + (void **)&ring-
> > >ring);
> > if (r) {
> > dev_err(adev->dev, "(%d) ring create
> > failed\n", r);
> > kvfree(ring->ring_backup);
