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

["Christian König" <christian.koenig@amd.com>]

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

Am 17.10.24 um 14:55 schrieb Khatri, Sunil:
>
> 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.

You mean that the checks are in almost all places, but here they are 
missing? Mhm, that's strange.

Let me investigate that further,
Christian.

> 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: 6985 bytes --]