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

[Christian König <christian.koenig@xxxxxxx>]

    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@xxxxxxx>
---
  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;
  }