Re: [PATCH v4 15/15] drm/amdgpu: validate get_clockgating_state before use

["Khatri, Sunil" <sunil.khatri@amd.com>]

[-- Attachment #1: Type: text/plain, Size: 3836 bytes --]


On 10/17/2024 5:50 PM, Christian König wrote:
> Am 17.10.24 um 12:06 schrieb Sunil Khatri:
>> Validate the function pointer for get_clockgating_state
>> before making a function call.
>
> Oh, I'm not sure if that is necessary or not. The NBIO, HDP and SMUIO 
> functions are not IP specific.
>
For many socs this check is added and only missing in the 
files/functions mentioned below. SOC where these functions are called 
are nv_common, soc15_common, soc21_common, soc24_common
eg: SOC15 already have these changes and its safe to add for other socs 
that i mentioned above.
soc15_common_get_clockgating_state Regards Sunil
> Christian.
>
>>
>> Signed-off-by: Sunil Khatri <sunil.khatri@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/nv.c    | 9 ++++++---
>>   drivers/gpu/drm/amd/amdgpu/soc21.c | 6 ++++--
>>   drivers/gpu/drm/amd/amdgpu/soc24.c | 6 ++++--
>>   3 files changed, 14 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/nv.c 
>> b/drivers/gpu/drm/amd/amdgpu/nv.c
>> index 6b72169be8f8..40c720b32c59 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/nv.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/nv.c
>> @@ -1084,11 +1084,14 @@ static void 
>> nv_common_get_clockgating_state(void *handle, u64 *flags)
>>       if (amdgpu_sriov_vf(adev))
>>           *flags = 0;
>>   -    adev->nbio.funcs->get_clockgating_state(adev, flags);
>> +    if (adev->nbio.funcs && adev->nbio.funcs->get_clockgating_state)
>> +        adev->nbio.funcs->get_clockgating_state(adev, flags);
>>   -    adev->hdp.funcs->get_clock_gating_state(adev, flags);
>> +    if (adev->hdp.funcs && adev->hdp.funcs->get_clock_gating_state)
>> +        adev->hdp.funcs->get_clock_gating_state(adev, flags);
>>   -    adev->smuio.funcs->get_clock_gating_state(adev, flags);
>> +    if (adev->smuio.funcs && adev->smuio.funcs->get_clock_gating_state)
>> +        adev->smuio.funcs->get_clock_gating_state(adev, flags);
>>   }
>>     static const struct amd_ip_funcs nv_common_ip_funcs = {
>> diff --git a/drivers/gpu/drm/amd/amdgpu/soc21.c 
>> b/drivers/gpu/drm/amd/amdgpu/soc21.c
>> index 1c07ebdc0d1f..196286be35b4 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/soc21.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/soc21.c
>> @@ -975,9 +975,11 @@ static void 
>> soc21_common_get_clockgating_state(void *handle, u64 *flags)
>>   {
>>       struct amdgpu_device *adev = (struct amdgpu_device *)handle;
>>   -    adev->nbio.funcs->get_clockgating_state(adev, flags);
>> +    if (adev->nbio.funcs && adev->nbio.funcs->get_clockgating_state)
>> +        adev->nbio.funcs->get_clockgating_state(adev, flags);
>>   -    adev->hdp.funcs->get_clock_gating_state(adev, flags);
>> +    if (adev->hdp.funcs && adev->hdp.funcs->get_clock_gating_state)
>> +        adev->hdp.funcs->get_clock_gating_state(adev, flags);
>>   }
>>     static const struct amd_ip_funcs soc21_common_ip_funcs = {
>> diff --git a/drivers/gpu/drm/amd/amdgpu/soc24.c 
>> b/drivers/gpu/drm/amd/amdgpu/soc24.c
>> index 3af10ef4b793..f4278a0fa8f7 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/soc24.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/soc24.c
>> @@ -564,9 +564,11 @@ static void 
>> soc24_common_get_clockgating_state(void *handle, u64 *flags)
>>   {
>>       struct amdgpu_device *adev = (struct amdgpu_device *)handle;
>>   -    adev->nbio.funcs->get_clockgating_state(adev, flags);
>> +    if (adev->nbio.funcs && adev->nbio.funcs->get_clockgating_state)
>> +        adev->nbio.funcs->get_clockgating_state(adev, flags);
>>   -    adev->hdp.funcs->get_clock_gating_state(adev, flags);
>> +    if (adev->hdp.funcs && adev->hdp.funcs->get_clock_gating_state)
>> +        adev->hdp.funcs->get_clock_gating_state(adev, flags);
>>         return;
>>   }
>

[-- Attachment #2: Type: text/html, Size: 6801 bytes --]