[PATCH v1 6/9] drm/amd/pm: stop extra checks for runtime pm state
Lazar, Lijo
lijo.lazar at amd.com
Thu Sep 26 10:11:19 UTC 2024
On 9/25/2024 7:59 PM, Pierre-Eric Pelloux-Prayer wrote:
>
>
> Le 25/09/2024 à 15:37, Lazar, Lijo a écrit :
>>
>>
>> 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 at amd.com>
>>> Signed-off-by: Pierre-Eric Pelloux-Prayer
>>> <pierre-eric.pelloux-prayer at amd.com>
>>> ---
>>> 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.
>
> AFAICT in_suspend / in_runpm are only set from resume / suspend
> sequences, so checking runtime_status != RPM_ACTIVE like
> pm_runtime_get_if_in_use does should provide the same result.
>
> (= during resume the device status is RPM_RESUMING)
>
> Pierre-Eric
>
On devices whose runtime PM is forbidden, I'm not sure if it goes
through RPM_ state changes or just statically remains at RPM_ACTIVE.
Regardless, these checks are removed only for sys/debug fs attributes.
Hence as Alex mentioned this access check was not required in the first
place.
Thanks,
Lijo
>
>>
>> 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)
More information about the amd-gfx
mailing list