Re: [PATCH 1/2] drm/amdgpu: use a dummy owner for sysfs triggered cleaner shaders v2

["Christian König" <ckoenig.leichtzumerken@gmail.com>]

Am 03.04.25 um 05:15 schrieb SRINIVASAN SHANMUGAM:
>
> On 4/2/2025 7:32 PM, Christian König wrote:
>> Otherwise triggering sysfs multiple times without other submissions in
>> between only runs the shader once.
>>
>> v2: add some comment
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 14 +++++++++++---
>>   1 file changed, 11 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
>> index f64675b2ab75..9a24be43e035 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
>> @@ -1439,9 +1439,11 @@ static int amdgpu_gfx_run_cleaner_shader_job(struct amdgpu_ring *ring)
>>       struct amdgpu_device *adev = ring->adev;
>>       struct drm_gpu_scheduler *sched = &ring->sched;
>>       struct drm_sched_entity entity;
>> +    static atomic_t counter;
>>       struct dma_fence *f;
>>       struct amdgpu_job *job;
>>       struct amdgpu_ib *ib;
>> +    void * owner;
>>       int i, r;
>>         /* Initialize the scheduler entity */
>> @@ -1452,9 +1454,15 @@ static int amdgpu_gfx_run_cleaner_shader_job(struct amdgpu_ring *ring)
>>           goto err;
>>       }
>>   -    r = amdgpu_job_alloc_with_ib(ring->adev, &entity, NULL,
>> -                     64, 0,
>> -                     &job);
>> +    /*
>> +     * Use some unique dummy value as the owner to make sure we execute
>> +     * the cleaner shader on each submission. The value just need to change
>> +     * for each submission and is otherwise meaningless.
>> +     */
>> +    owner = (unsigned long)atomic_inc_return(&counter),
>
> Should this "atomic_inc_return()" needs to be casted to (void*), as it is return type is int?

No, I had that in the first version of the patch but it's actually sufficient to cast it to a long (which has the same number of bits as void*).

Regards,
Christian.

>
> Regards,
> Srini
>
>> +
>> +    r = amdgpu_job_alloc_with_ib(ring->adev, &entity, owner,
>> +                     64, 0, &job);
>>       if (r)
>>           goto err;
>>