Please add proper lock in smu_set_soft_freq_range() then.
You can do necessary modification to existing code paths.
Otherwise without lock protection, you may run into race condition on the new 
debugfs intrace(amdgpu_debugfs_sclk_set) called by multiple instances.

-----Original Message-----
From: Gui, Jack <jack....@amd.com> 
Sent: Friday, February 28, 2020 12:35 PM
To: Quan, Evan <evan.q...@amd.com>; amd-gfx@lists.freedesktop.org
Cc: Feng, Kenneth <kenneth.f...@amd.com>; Xu, Feifei <feifei...@amd.com>
Subject: RE: [PATCH 2/2] drm/amdgpu: Add debugfs interface to set arbitrary 
sclk for navi14

[AMD Official Use Only - Internal Distribution Only]

Hi Evan,

No lock is created for smu_set_soft_freq_range() in this code path.
But some other sysfs interfaces calling  smu_set_soft_freq_range() indirectly 
have created lock in middle function to protect  smu_set_soft_freq_range() as 
critical resource.
So, if we want to lock the "issue message action" in smu_set_soft_freq_range(), 
another patch is needed.

BR,
Jack

-----Original Message-----
From: Quan, Evan <evan.q...@amd.com>
Sent: Friday, February 28, 2020 11:37 AM
To: Gui, Jack <jack....@amd.com>; amd-gfx@lists.freedesktop.org
Cc: Feng, Kenneth <kenneth.f...@amd.com>; Xu, Feifei <feifei...@amd.com>; Gui, 
Jack <jack....@amd.com>
Subject: RE: [PATCH 2/2] drm/amdgpu: Add debugfs interface to set arbitrary 
sclk for navi14

Please confirm whether smu_set_soft_freq_range() is properly lock protected.

-----Original Message-----
From: Chengming Gui <jack....@amd.com>
Sent: Friday, February 28, 2020 10:37 AM
To: amd-gfx@lists.freedesktop.org
Cc: Quan, Evan <evan.q...@amd.com>; Feng, Kenneth <kenneth.f...@amd.com>; Xu, 
Feifei <feifei...@amd.com>; Gui, Jack <jack....@amd.com>
Subject: [PATCH 2/2] drm/amdgpu: Add debugfs interface to set arbitrary sclk 
for navi14

add debugfs interface amdgpu_force_sclk
to set arbitrary sclk for navi14

Signed-off-by: Chengming Gui <jack....@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c    | 44 ++++++++++++++++++++++++++
 drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h |  3 ++
 2 files changed, 47 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
index 3bb7405..5ee7e92 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
@@ -1269,9 +1269,43 @@ static int amdgpu_debugfs_ib_preempt(void *data, u64 val)
        return 0;
 }
 
+static int amdgpu_debugfs_sclk_set(void *data, u64 val) {
+       int ret = 0;
+       uint32_t max_freq, min_freq;
+       struct amdgpu_device *adev = (struct amdgpu_device *)data;
+
+       if (amdgpu_sriov_vf(adev) && !amdgpu_sriov_is_pp_one_vf(adev))
+               return -EINVAL;
+
+       ret = pm_runtime_get_sync(adev->ddev->dev);
+       if (ret < 0)
+               return ret;
+
+       if (is_support_sw_smu(adev)) {
+               ret = smu_get_dpm_freq_range(&adev->smu, SMU_SCLK, &min_freq, 
&max_freq, true);
+               if (ret || val > max_freq || val < min_freq)
+                       return -EINVAL;
+               ret = smu_set_soft_freq_range(&adev->smu, SMU_SCLK, 
(uint32_t)val, (uint32_t)val);
+       } else {
+               return 0;
+       }
+
+       pm_runtime_mark_last_busy(adev->ddev->dev);
+       pm_runtime_put_autosuspend(adev->ddev->dev);
+
+       if (ret)
+               return -EINVAL;
+
+       return 0;
+}
+
 DEFINE_SIMPLE_ATTRIBUTE(fops_ib_preempt, NULL,
                        amdgpu_debugfs_ib_preempt, "%llu\n");
 
+DEFINE_SIMPLE_ATTRIBUTE(fops_sclk_set, NULL,
+                       amdgpu_debugfs_sclk_set, "%llu\n");
+
 int amdgpu_debugfs_init(struct amdgpu_device *adev)  {
        int r, i;
@@ -1285,6 +1319,15 @@ int amdgpu_debugfs_init(struct amdgpu_device *adev)
                return -EIO;
        }
 
+       adev->smu.debugfs_sclk =
+               debugfs_create_file("amdgpu_force_sclk", 0200,
+                                   adev->ddev->primary->debugfs_root, adev,
+                                   &fops_sclk_set);
+       if (!(adev->smu.debugfs_sclk)) {
+               DRM_ERROR("unable to create amdgpu_set_sclk debugsfs file\n");
+               return -EIO;
+       }
+
        /* Register debugfs entries for amdgpu_ttm */
        r = amdgpu_ttm_debugfs_init(adev);
        if (r) {
@@ -1353,6 +1396,7 @@ void amdgpu_debugfs_fini(struct amdgpu_device *adev)
        }
        amdgpu_ttm_debugfs_fini(adev);
        debugfs_remove(adev->debugfs_preempt);
+       debugfs_remove(adev->smu.debugfs_sclk);
 }
 
 #else
diff --git a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h 
b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
index 97b6714..36fe19c 100644
--- a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
+++ b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
@@ -371,6 +371,9 @@ struct smu_context
        struct amd_pp_display_configuration  *display_config;
        struct smu_baco_context         smu_baco;
        void *od_settings;
+#if defined(CONFIG_DEBUG_FS)
+       struct dentry                   *debugfs_sclk;
+#endif
 
        uint32_t pstate_sclk;
        uint32_t pstate_mclk;
--
2.7.4
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Reply via email to