[PATCH 2/2] drm/amdgpu: fill scheduler with device ptr

Christian König christian.koenig at amd.com
Thu Feb 17 07:39:12 UTC 2022


Am 17.02.22 um 03:46 schrieb Gu, JiaWei (Will):
> [AMD Official Use Only]
>
> Hi Christian,
>
> My same concern is that an additional parameter may affects other drivers which want to use public drm_sched_init(), and I want to reduce the scope of affection.
> Will it avoid potential compatibility issues if we keep the interface unchanged and let driver fill device pointer by itself?

No, that will just cause confusion. We keep all drivers in a single 
Linux kernel exactly to avoid stuff like that.

Regards,
Christian.

>
> And DRM_DEV_ERROR() print is fine with NULL device pointer, there's a NULL pointer check inside of it already.
>
> Best regards,
> Jiawei
>
> -----Original Message-----
> From: Koenig, Christian <Christian.Koenig at amd.com>
> Sent: Wednesday, February 16, 2022 7:17 PM
> To: Gu, JiaWei (Will) <JiaWei.Gu at amd.com>; amd-gfx at lists.freedesktop.org; Grodzovsky, Andrey <Andrey.Grodzovsky at amd.com>; Liu, Monk <Monk.Liu at amd.com>; Deng, Emily <Emily.Deng at amd.com>; Chen, Horace <Horace.Chen at amd.com>
> Subject: Re: [PATCH 2/2] drm/amdgpu: fill scheduler with device ptr
>
>
>
> Am 16.02.22 um 08:22 schrieb Jiawei Gu:
>> Now scheduler contains device ptr. Add it so scheduler printing can be
>> more reader-friendly under multiple GPU scenario.
>>
>> Signed-off-by: Jiawei Gu <Jiawei.Gu at amd.com>
>> ---
>>    drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c | 1 +
>>    1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
>> index 4787cb3acaed..da53983c93f9 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
>> @@ -506,6 +506,7 @@ int amdgpu_fence_driver_init_ring(struct amdgpu_ring *ring,
>>    		break;
>>    	}
>>    
>> +	ring->sched.dev = adev->dev;
> That should probably be a parameter to drm_sched_init() instead and I'm not sure what happens in the print when this is NULL.
>
> So make sure to update all other drivers which want to use
> drm_sched_init() as well.
>
> Regards,
> Christian.
>
>>    	r = drm_sched_init(&ring->sched, &amdgpu_sched_ops,
>>    			   num_hw_submission, amdgpu_job_hang_limit,
>>    			   timeout, sched_score, ring->name);



More information about the amd-gfx mailing list