Re: [PATCH v2 1/2] drm/amdgpu: move error log from ring write to commit

["Khatri, Sunil" <sunil.khatri@xxxxxxx>]

On 10/7/2024 7:18 PM, Christian König wrote:
> Am 03.10.24 um 10:28 schrieb Sunil Khatri:
> > Move the error message from ring write as an optimization
> > to avoid printing that message on every write instead
> > print once during commit if it exceeds write the allocated
> > size i.e ring->count_dw.
> >
> > Also we do not want to log the error message in between a
> > ring write and complete the write as its mostly not harmful
> > as it will overwrite stale data only as GPU read from ring
> > is faster than CPU write to ring.
> >
> > This reduces the size of amdgpu.ko module by around
> > 600 Kb as write is very often used function and hence
> > the print.
> >
> > Signed-off-by: Sunil Khatri <sunil.khatri@xxxxxxx>
> > Suggested-by: Christian König <christian.koenig@xxxxxxx>
> > ---
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c | 3 +++
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h | 2 --
> >   2 files changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c 
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> > index 690976665cf6..05b3480ecec7 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> > @@ -141,6 +141,9 @@ void amdgpu_ring_commit(struct amdgpu_ring *ring)
> >   {
> >       uint32_t count;
> >   +    if (ring->count_dw <= 0)
> > +        DRM_ERROR("amdgpu: writing more dwords to the ring than 
> > expected!\n");
> > +
>
> Oh, I missed this on the first review a count_dw of 0 is actually ok 
> since that means we have written exactly as much dw as estimated.
Sure i will replace it with below code

 if (ring->count_dw < 0)

Regards
Sunil khatri

> Regards,
> Christian.
>
> >       /* We pad to match fetch size */
> >       count = ring->funcs->align_mask + 1 -
> >           (ring->wptr & ring->funcs->align_mask);
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h 
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> > index f93f51002201..af8824e8da49 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> > @@ -377,8 +377,6 @@ static inline void amdgpu_ring_clear_ring(struct 
> > amdgpu_ring *ring)
> >     static inline void amdgpu_ring_write(struct amdgpu_ring *ring, 
> > uint32_t v)
> >   {
> > -    if (ring->count_dw <= 0)
> > -        DRM_ERROR("amdgpu: writing more dwords to the ring than 
> > expected!\n");
> >       ring->ring[ring->wptr++ & ring->buf_mask] = v;
> >       ring->wptr &= ring->ptr_mask;
> >       ring->count_dw--;