On 02.09.25 13:30, Timur Kristóf wrote:
> On Mon, 2025-09-01 at 11:13 +0100, Tvrtko Ursulin wrote:
>>
>> Hi,
>>
>> On 01/09/2025 11:00, Timur Kristóf wrote:
>>> Technically not necessary, but clear the extra dwords too,
>>> so that the command processors don't read uninitialized memory.
>>>
>>> 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.h | 5 +++++
>>> 1 file changed, 5 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
>>> index 7670f5d82b9e..6a55a85744a9 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
>>> @@ -474,6 +474,11 @@ static inline void
>>> amdgpu_ring_clear_ring(struct amdgpu_ring *ring)
>>> while (i <= ring->buf_mask)
>>> ring->ring[i++] = ring->funcs->nop;
>>>
>>> + /* Technically not necessary, but clear the extra dwords
>>> too,
>>> + * so that the command processors don't read uninitialized
>>> memory.
>>> + */
>>> + for (i = 0; i < ring->funcs->extra_dw; i++)
>>> + ring->ring[ring->ring_size / 4 + i] = ring->funcs-
>>>> nop;
>>
>> Should I resend this maybe?
>
>
> Hi Tvrtko,
>
> The patch you commented on is going to be dropped.
>
> However, your patch makes good sense, so I can include it in the next
> version of this series if that's OK.
>
> @Christian - does that sound alright to you?
Works for me.
Regards,
Christian.
>
>
>>
>> commit 11b0b5d942fe46bfb01f021cdb0616c8385d5ea8
>> Author: Tvrtko Ursulin <[email protected]>
>> Date: Thu Dec 26 16:12:37 2024 +0000
>>
>> drm/amdgpu: Use memset32 for ring clearing
>>
>> Use memset32 instead of open coding it, just because it is
>> a tiny bit nicer.
>>
>> Signed-off-by: Tvrtko Ursulin <[email protected]>
>> Cc: Christian König <[email protected]>
>> Cc: Sunil Khatri <[email protected]>
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
>> index dee5a1b4e572..96bfc0c23413 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
>> @@ -369,10 +369,7 @@ static inline void
>> amdgpu_ring_set_preempt_cond_exec(struct amdgpu_ring *ring,
>>
>> static inline void amdgpu_ring_clear_ring(struct amdgpu_ring *ring)
>> {
>> - int i = 0;
>> - while (i <= ring->buf_mask)
>> - ring->ring[i++] = ring->funcs->nop;
>> -
>> + memset32(ring->ring, ring->funcs->nop, ring->buf_mask + 1);
>> }
>>
>> static inline void amdgpu_ring_write(struct amdgpu_ring *ring,
>> uint32_t v)
>>
>> Looks like with two loops it would made even more sense to
>> consolidate.
>>
>> Regards,
>>
>> Tvrtko
>>
>>> }
>>>
>>> static inline void amdgpu_ring_write(struct amdgpu_ring *ring,
>>> uint32_t v)
>>
