On Wed, Sep 25, 2024 at 9:38 AM Lazar, Lijo <lijo.lazar@xxxxxxx> wrote: > > > > On 9/25/2024 1:24 PM, Pierre-Eric Pelloux-Prayer wrote: > > pm_runtime_get_if_in_use already checks if the GPU is active, > > so there's no need for manually checking runtimepm status: > > > > if (adev->in_suspend && !adev->in_runpm) > > return -EPERM; > > > > Tested-by: Mario Limonciello <mario.limonciello@xxxxxxx> > > Signed-off-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@xxxxxxx> > > --- > > drivers/gpu/drm/amd/pm/amdgpu_pm.c | 46 ------------------------------ > > 1 file changed, 46 deletions(-) > > > > diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c b/drivers/gpu/drm/amd/pm/amdgpu_pm.c > > index f1f339b75380..13be5e017a01 100644 > > --- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c > > +++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c > > @@ -142,8 +142,6 @@ static ssize_t amdgpu_get_power_dpm_state(struct device *dev, > > > > if (amdgpu_in_reset(adev)) > > return -EPERM; > > - if (adev->in_suspend && !adev->in_runpm) > > - return -EPERM; > > > > I believe this check is for accesses before the device is fully resumed > from a suspend sequence. That is not tied to runtime PM. In theory, user processes should not be resumed until the kernel drivers have resumed so I think the check was probably not needed in the first place. Alex > > Thanks, > Lijo > > > ret = pm_runtime_get_if_active(ddev->dev, true); > > if (ret <= 0) > > @@ -265,8 +263,6 @@ static ssize_t amdgpu_get_power_dpm_force_performance_level(struct device *dev, > > > > if (amdgpu_in_reset(adev)) > > return -EPERM; > > - if (adev->in_suspend && !adev->in_runpm) > > - return -EPERM; > > > > ret = pm_runtime_get_if_active(ddev->dev, true); > > if (ret <= 0) > > @@ -361,8 +357,6 @@ static ssize_t amdgpu_get_pp_num_states(struct device *dev, > > > > if (amdgpu_in_reset(adev)) > > return -EPERM; > > - if (adev->in_suspend && !adev->in_runpm) > > - return -EPERM; > > > > ret = pm_runtime_get_if_active(ddev->dev, true); > > if (ret <= 0) > > @@ -396,8 +390,6 @@ static ssize_t amdgpu_get_pp_cur_state(struct device *dev, > > > > if (amdgpu_in_reset(adev)) > > return -EPERM; > > - if (adev->in_suspend && !adev->in_runpm) > > - return -EPERM; > > > > ret = pm_runtime_get_if_active(ddev->dev, true); > > if (ret <= 0) > > @@ -523,8 +515,6 @@ static ssize_t amdgpu_get_pp_table(struct device *dev, > > > > if (amdgpu_in_reset(adev)) > > return -EPERM; > > - if (adev->in_suspend && !adev->in_runpm) > > - return -EPERM; > > > > ret = pm_runtime_get_if_active(ddev->dev, true); > > if (ret <= 0) > > @@ -837,8 +827,6 @@ static ssize_t amdgpu_get_pp_od_clk_voltage(struct device *dev, > > > > if (amdgpu_in_reset(adev)) > > return -EPERM; > > - if (adev->in_suspend && !adev->in_runpm) > > - return -EPERM; > > > > ret = pm_runtime_get_if_active(ddev->dev, true); > > if (ret <= 0) > > @@ -927,8 +915,6 @@ static ssize_t amdgpu_get_pp_features(struct device *dev, > > > > if (amdgpu_in_reset(adev)) > > return -EPERM; > > - if (adev->in_suspend && !adev->in_runpm) > > - return -EPERM; > > > > ret = pm_runtime_get_if_active(ddev->dev, true); > > if (ret <= 0) > > @@ -993,8 +979,6 @@ static ssize_t amdgpu_get_pp_dpm_clock(struct device *dev, > > > > if (amdgpu_in_reset(adev)) > > return -EPERM; > > - if (adev->in_suspend && !adev->in_runpm) > > - return -EPERM; > > > > ret = pm_runtime_get_if_active(ddev->dev, true); > > if (ret <= 0) > > @@ -1242,8 +1226,6 @@ static ssize_t amdgpu_get_pp_sclk_od(struct device *dev, > > > > if (amdgpu_in_reset(adev)) > > return -EPERM; > > - if (adev->in_suspend && !adev->in_runpm) > > - return -EPERM; > > > > ret = pm_runtime_get_if_active(ddev->dev, true); > > if (ret <= 0) > > @@ -1299,8 +1281,6 @@ static ssize_t amdgpu_get_pp_mclk_od(struct device *dev, > > > > if (amdgpu_in_reset(adev)) > > return -EPERM; > > - if (adev->in_suspend && !adev->in_runpm) > > - return -EPERM; > > > > ret = pm_runtime_get_if_active(ddev->dev, true); > > if (ret <= 0) > > @@ -1376,8 +1356,6 @@ static ssize_t amdgpu_get_pp_power_profile_mode(struct device *dev, > > > > if (amdgpu_in_reset(adev)) > > return -EPERM; > > - if (adev->in_suspend && !adev->in_runpm) > > - return -EPERM; > > > > ret = pm_runtime_get_if_active(ddev->dev, true); > > if (ret <= 0) > > @@ -1464,8 +1442,6 @@ static int amdgpu_hwmon_get_sensor_generic(struct amdgpu_device *adev, > > > > if (amdgpu_in_reset(adev)) > > return -EPERM; > > - if (adev->in_suspend && !adev->in_runpm) > > - return -EPERM; > > > > r = pm_runtime_get_if_active(adev->dev, true); > > if (r <= 0) > > @@ -1574,8 +1550,6 @@ static ssize_t amdgpu_get_pcie_bw(struct device *dev, > > > > if (amdgpu_in_reset(adev)) > > return -EPERM; > > - if (adev->in_suspend && !adev->in_runpm) > > - return -EPERM; > > > > if (adev->flags & AMD_IS_APU) > > return -ENODATA; > > @@ -1784,8 +1758,6 @@ static ssize_t amdgpu_get_pm_metrics(struct device *dev, > > > > if (amdgpu_in_reset(adev)) > > return -EPERM; > > - if (adev->in_suspend && !adev->in_runpm) > > - return -EPERM; > > > > ret = pm_runtime_get_if_active(ddev->dev, true); > > if (ret <= 0) > > @@ -1822,8 +1794,6 @@ static ssize_t amdgpu_get_gpu_metrics(struct device *dev, > > > > if (amdgpu_in_reset(adev)) > > return -EPERM; > > - if (adev->in_suspend && !adev->in_runpm) > > - return -EPERM; > > > > ret = pm_runtime_get_if_active(ddev->dev, true); > > if (ret <= 0) > > @@ -2697,8 +2667,6 @@ static ssize_t amdgpu_hwmon_get_pwm1_enable(struct device *dev, > > > > if (amdgpu_in_reset(adev)) > > return -EPERM; > > - if (adev->in_suspend && !adev->in_runpm) > > - return -EPERM; > > > > ret = pm_runtime_get_if_active(adev->dev, true); > > if (ret <= 0) > > @@ -2825,8 +2793,6 @@ static ssize_t amdgpu_hwmon_get_pwm1(struct device *dev, > > > > if (amdgpu_in_reset(adev)) > > return -EPERM; > > - if (adev->in_suspend && !adev->in_runpm) > > - return -EPERM; > > > > err = pm_runtime_get_if_active(adev->dev, true); > > if (err <= 0) > > @@ -2852,8 +2818,6 @@ static ssize_t amdgpu_hwmon_get_fan1_input(struct device *dev, > > > > if (amdgpu_in_reset(adev)) > > return -EPERM; > > - if (adev->in_suspend && !adev->in_runpm) > > - return -EPERM; > > > > err = pm_runtime_get_if_active(adev->dev, true); > > if (err <= 0) > > @@ -2913,8 +2877,6 @@ static ssize_t amdgpu_hwmon_get_fan1_target(struct device *dev, > > > > if (amdgpu_in_reset(adev)) > > return -EPERM; > > - if (adev->in_suspend && !adev->in_runpm) > > - return -EPERM; > > > > err = pm_runtime_get_if_active(adev->dev, true); > > if (err <= 0) > > @@ -2983,8 +2945,6 @@ static ssize_t amdgpu_hwmon_get_fan1_enable(struct device *dev, > > > > if (amdgpu_in_reset(adev)) > > return -EPERM; > > - if (adev->in_suspend && !adev->in_runpm) > > - return -EPERM; > > > > ret = pm_runtime_get_if_active(adev->dev, true); > > if (ret <= 0) > > @@ -3149,8 +3109,6 @@ static ssize_t amdgpu_hwmon_show_power_cap_generic(struct device *dev, > > > > if (amdgpu_in_reset(adev)) > > return -EPERM; > > - if (adev->in_suspend && !adev->in_runpm) > > - return -EPERM; > > > > r = pm_runtime_get_if_active(adev->dev, true); > > if (r <= 0) > > @@ -3682,8 +3640,6 @@ static int amdgpu_retrieve_od_settings(struct amdgpu_device *adev, > > > > if (amdgpu_in_reset(adev)) > > return -EPERM; > > - if (adev->in_suspend && !adev->in_runpm) > > - return -EPERM; > > > > ret = pm_runtime_get_if_active(adev->dev, true); > > if (ret <= 0) > > @@ -4649,8 +4605,6 @@ static int amdgpu_debugfs_pm_info_show(struct seq_file *m, void *unused) > > > > if (amdgpu_in_reset(adev)) > > return -EPERM; > > - if (adev->in_suspend && !adev->in_runpm) > > - return -EPERM; > > > > r = pm_runtime_resume_and_get(dev->dev); > > if (r < 0)
