[PATCH 1/2] drm/amdgpu: validate if sw_init is defined or NULL

Khatri, Sunil sunil.khatri at amd.com
Wed Oct 9 11:59:11 UTC 2024


On 10/9/2024 4:19 PM, Christian König wrote:
> Am 09.10.24 um 10:48 schrieb Sunil Khatri:
>> Before making a function call to sw_init, validate
>> the function pointer.
>
> Maybe add " like we do for hw_init." or some similar example of 
> optional callback.
Sure will update the commit message also
>
>>
>> Signed-off-by: Sunil Khatri <sunil.khatri at amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 12 +++++++-----
>>   1 file changed, 7 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> index 38a7423101f3..4a6def74964e 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> @@ -2851,13 +2851,15 @@ static int amdgpu_device_ip_init(struct 
>> amdgpu_device *adev)
>>       for (i = 0; i < adev->num_ip_blocks; i++) {
>>           if (!adev->ip_blocks[i].status.valid)
>>               continue;
>> -        r = 
>> adev->ip_blocks[i].version->funcs->sw_init(&adev->ip_blocks[i]);
>> -        if (r) {
>> -            DRM_ERROR("sw_init of IP block <%s> failed %d\n",
>> +        if (adev->ip_blocks[i].version->funcs->sw_init) {
>> +            r = 
>> adev->ip_blocks[i].version->funcs->sw_init(&adev->ip_blocks[i]);
>> +            if (r) {
>> +                DRM_ERROR("sw_init of IP block <%s> failed %d\n",
>> adev->ip_blocks[i].version->funcs->name, r);
>> -            goto init_failed;
>> +                goto init_failed;
>> +            }
>> +            adev->ip_blocks[i].status.sw = true;
>>           }
>> -        adev->ip_blocks[i].status.sw = true;
>
> I think we should set that to true regardless of the callback being 
> defined or not.
That exactly is done in v2 patch set already.
>
> Could be that an IP has a sw_fini callback, but not a sw_init one.
> Yes there are example of it where sw_init is not there but sw_fini is 
> there
>
> Regards,
> Christian.
>
>>             if (!amdgpu_ip_member_of_hwini(
>>                   adev, adev->ip_blocks[i].version->type))
>


More information about the amd-gfx mailing list