[PATCH 2/2] drm/amdgpu: stop masking the wptr all the time

[Christian König]

Am 08.10.24 um 20:43 schrieb Khatri, Sunil:
>
>
> On 10/8/2024 11:41 PM, Christian König wrote:
>> Stop masking the wptr and decrementing the count_dw while writing into
>> the ring buffer. We can do that all at once while pushing the changes to
>> the HW.
>>
>> Signed-off-by: Christian König<christian.koenig at amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c | 11 +++++------
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h |  4 ----
>>   2 files changed, 5 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
>> index a6e28fe3f8d6..364466d6d3de 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
>> @@ -90,12 +90,11 @@ int amdgpu_ring_alloc(struct amdgpu_ring *ring, unsigned int ndw)
>>   	if (WARN_ON_ONCE(ndw > ring->max_dw))
>>   		return -ENOMEM;
>>   
>> -	ring->count_dw = ndw;
>> -	ring->wptr_old = ring->wptr;
>> -
>>   	if (ring->funcs->begin_use)
>>   		ring->funcs->begin_use(ring);
>>   
>> +	ring->count_dw = ndw;
>> +	ring->wptr_old = ring->wptr;
> Does moving these after begin_use makes any difference? Otherwise 
> looks good to me.

Yes, it does. The VCN begin_use() callback modifies the wptr on resume.

So we can only safe wptr_old after making the callback.

> Reviewed-by: Sunil Khatri <sunil.khatri at amd.com> 
> <mailto:sunil.khatri at amd.com>
>

Thanks,
Christian.

> Regards
> Sunil Khatri
>
>
>>   	return 0;
>>   }
>>   
>> @@ -122,8 +121,6 @@ void amdgpu_ring_insert_nop(struct amdgpu_ring *ring, uint32_t count)
>>   		memset32(ring->ring, ring->funcs->nop, chunk2);
>>   
>>   	ring->wptr += count;
>> -	ring->wptr &= ring->ptr_mask;
>> -	ring->count_dw -= count;
>>   }
>>   
>>   /**
>> @@ -153,9 +150,11 @@ void amdgpu_ring_commit(struct amdgpu_ring *ring)
>>   {
>>   	uint32_t count;
>>   
>> -	if (ring->count_dw < 0)
>> +	if ((ring->wptr - ring->wptr_old) > ring->count_dw)
>>   		DRM_ERROR("amdgpu: writing more dwords to the ring than expected!\n");
>>   
>> +	ring->wptr &= ring->ptr_mask;
>> +
>>   	/* 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 36fc9578c53c..439793206b89 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
>> @@ -378,8 +378,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)
>>   {
>>   	ring->ring[ring->wptr++ & ring->buf_mask] = v;
>> -	ring->wptr &= ring->ptr_mask;
>> -	ring->count_dw--;
>>   }
>>   
>>   static inline void amdgpu_ring_write_multiple(struct amdgpu_ring *ring,
>> @@ -403,8 +401,6 @@ static inline void amdgpu_ring_write_multiple(struct amdgpu_ring *ring,
>>   	}
>>   
>>   	ring->wptr += count_dw;
>> -	ring->wptr &= ring->ptr_mask;
>> -	ring->count_dw -= count_dw;
>>   }
>>   
>>   /**
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20241009/af15f103/attachment-0001.htm>