[PATCH v1 7/9] drm/amd/pm: add sysfs attribute access wrappers
Pierre-Eric Pelloux-Prayer
pierre-eric at damsy.net
Mon Sep 30 16:54:29 UTC 2024
Le 25/09/2024 à 09:54, Pierre-Eric Pelloux-Prayer a écrit :
> All attributes do the same thing wrt to runtime power management,
> so we can consolidate the handling in 2 wrappers.
>
> For some setters this will change the behavior slightly, as rpm
> is now done before arguments validation - so the device will be
> resumed even if the arguments passed in are incorrect.
>
> 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 | 399 ++++---------------------
> drivers/gpu/drm/amd/pm/inc/amdgpu_pm.h | 16 +-
> 2 files changed, 78 insertions(+), 337 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
> index 13be5e017a01..1bac174e6d09 100644
> --- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
> +++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
> @@ -97,6 +97,61 @@ const char * const amdgpu_pp_profile_name[] = {
> "UNCAPPED",
> };
>
> +static ssize_t amdgpu_pp_attr_show_wrapper(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct drm_device *ddev = dev_get_drvdata(dev);
> + struct amdgpu_device *adev = drm_to_adev(ddev);
> + struct amdgpu_device_attr *aattr;
> + ssize_t r;
> + int ret;
> +
> + if (amdgpu_in_reset(adev))
> + return -EPERM;
> + ret = pm_runtime_get_if_active(dev, true);
> + if (ret <= 0)
> + return ret ?: -EPERM;
> +
> + aattr = to_amdgpu_device_attr(attr);
> +
> + r = aattr->attr_show(dev, attr, buf);
> +
> + pm_runtime_put_autosuspend(dev);
> +
> + return r;
> +}
> +
> +static ssize_t amdgpu_pp_attr_store_wrapper(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf,
> + size_t count)
> +{
> + struct drm_device *ddev = dev_get_drvdata(dev);
> + struct amdgpu_device *adev = drm_to_adev(ddev);
> + struct amdgpu_device_attr *aattr;
> + ssize_t r;
> + int ret;
> +
> + if (amdgpu_in_reset(adev))
> + return -EPERM;
> + if (adev->in_suspend && !adev->in_runpm)
> + return -EPERM;
> + ret = pm_runtime_resume_and_get(dev);
> + if (ret < 0)
> + return ret;
> +
> + aattr = to_amdgpu_device_attr(attr);
> +
> + r = aattr->attr_store(dev, attr, buf, count);
> +
> + pm_runtime_mark_last_busy(dev);
> + pm_runtime_put_autosuspend(dev);
> +
> + return r;
> +}
> +
> +
> /**
> * DOC: power_dpm_state
> *
> @@ -138,19 +193,9 @@ static ssize_t amdgpu_get_power_dpm_state(struct device *dev,
> struct drm_device *ddev = dev_get_drvdata(dev);
> struct amdgpu_device *adev = drm_to_adev(ddev);
> enum amd_pm_state_type pm;
> - int ret;
> -
> - if (amdgpu_in_reset(adev))
> - return -EPERM;
> -
> - ret = pm_runtime_get_if_active(ddev->dev, true);
> - if (ret <= 0)
> - return ret ?: -EPERM;
>
> amdgpu_dpm_get_current_power_state(adev, &pm);
>
> - pm_runtime_put_autosuspend(ddev->dev);
> -
> return sysfs_emit(buf, "%s\n",
> (pm == POWER_STATE_TYPE_BATTERY) ? "battery" :
> (pm == POWER_STATE_TYPE_BALANCED) ? "balanced" : "performance");
> @@ -164,12 +209,6 @@ static ssize_t amdgpu_set_power_dpm_state(struct device *dev,
> struct drm_device *ddev = dev_get_drvdata(dev);
> struct amdgpu_device *adev = drm_to_adev(ddev);
> enum amd_pm_state_type state;
> - int ret;
> -
> - if (amdgpu_in_reset(adev))
> - return -EPERM;
> - if (adev->in_suspend && !adev->in_runpm)
> - return -EPERM;
>
> if (strncmp("battery", buf, strlen("battery")) == 0)
> state = POWER_STATE_TYPE_BATTERY;
> @@ -180,15 +219,8 @@ static ssize_t amdgpu_set_power_dpm_state(struct device *dev,
> else
> return -EINVAL;
>
> - ret = pm_runtime_resume_and_get(ddev->dev);
> - if (ret < 0)
> - return ret;
> -
> amdgpu_dpm_set_power_state(adev, state);
>
> - pm_runtime_mark_last_busy(ddev->dev);
> - pm_runtime_put_autosuspend(ddev->dev);
> -
> return count;
> }
>
> @@ -259,19 +291,9 @@ static ssize_t amdgpu_get_power_dpm_force_performance_level(struct device *dev,
> struct drm_device *ddev = dev_get_drvdata(dev);
> struct amdgpu_device *adev = drm_to_adev(ddev);
> enum amd_dpm_forced_level level = 0xff;
> - int ret;
> -
> - if (amdgpu_in_reset(adev))
> - return -EPERM;
> -
> - ret = pm_runtime_get_if_active(ddev->dev, true);
> - if (ret <= 0)
> - return ret ?: -EPERM;
>
> level = amdgpu_dpm_get_performance_level(adev);
>
> - pm_runtime_put_autosuspend(ddev->dev);
> -
> return sysfs_emit(buf, "%s\n",
> (level == AMD_DPM_FORCED_LEVEL_AUTO) ? "auto" :
> (level == AMD_DPM_FORCED_LEVEL_LOW) ? "low" :
> @@ -293,12 +315,6 @@ static ssize_t amdgpu_set_power_dpm_force_performance_level(struct device *dev,
> struct drm_device *ddev = dev_get_drvdata(dev);
> struct amdgpu_device *adev = drm_to_adev(ddev);
> enum amd_dpm_forced_level level;
> - int ret = 0;
> -
> - if (amdgpu_in_reset(adev))
> - return -EPERM;
> - if (adev->in_suspend && !adev->in_runpm)
> - return -EPERM;
>
> if (strncmp("low", buf, strlen("low")) == 0) {
> level = AMD_DPM_FORCED_LEVEL_LOW;
> @@ -324,14 +340,8 @@ static ssize_t amdgpu_set_power_dpm_force_performance_level(struct device *dev,
> return -EINVAL;
> }
>
> - ret = pm_runtime_resume_and_get(ddev->dev);
> - if (ret < 0)
> - return ret;
> -
> mutex_lock(&adev->pm.stable_pstate_ctx_lock);
> if (amdgpu_dpm_force_performance_level(adev, level)) {
> - pm_runtime_mark_last_busy(ddev->dev);
> - pm_runtime_put_autosuspend(ddev->dev);
> mutex_unlock(&adev->pm.stable_pstate_ctx_lock);
> return -EINVAL;
> }
> @@ -339,9 +349,6 @@ static ssize_t amdgpu_set_power_dpm_force_performance_level(struct device *dev,
> adev->pm.stable_pstate_ctx = NULL;
> mutex_unlock(&adev->pm.stable_pstate_ctx_lock);
>
> - pm_runtime_mark_last_busy(ddev->dev);
> - pm_runtime_put_autosuspend(ddev->dev);
> -
> return count;
> }
>
> @@ -353,20 +360,11 @@ static ssize_t amdgpu_get_pp_num_states(struct device *dev,
> struct amdgpu_device *adev = drm_to_adev(ddev);
> struct pp_states_info data;
> uint32_t i;
> - int buf_len, ret;
> -
> - if (amdgpu_in_reset(adev))
> - return -EPERM;
> -
> - ret = pm_runtime_get_if_active(ddev->dev, true);
> - if (ret <= 0)
> - return ret ?: -EPERM;
> + int buf_len;
>
> if (amdgpu_dpm_get_pp_num_states(adev, &data))
> memset(&data, 0, sizeof(data));
>
> - pm_runtime_put_autosuspend(ddev->dev);
> -
> buf_len = sysfs_emit(buf, "states: %d\n", data.nums);
> for (i = 0; i < data.nums; i++)
> buf_len += sysfs_emit_at(buf, buf_len, "%d %s\n", i,
> @@ -386,21 +384,12 @@ static ssize_t amdgpu_get_pp_cur_state(struct device *dev,
> struct amdgpu_device *adev = drm_to_adev(ddev);
> struct pp_states_info data = {0};
> enum amd_pm_state_type pm = 0;
> - int i = 0, ret = 0;
> -
> - if (amdgpu_in_reset(adev))
> - return -EPERM;
> -
> - ret = pm_runtime_get_if_active(ddev->dev, true);
> - if (ret <= 0)
> - return ret ?: -EPERM;
> + int i, ret;
>
> amdgpu_dpm_get_current_power_state(adev, &pm);
>
> ret = amdgpu_dpm_get_pp_num_states(adev, &data);
>
> - pm_runtime_put_autosuspend(ddev->dev);
> -
> if (ret)
> return ret;
>
> @@ -422,11 +411,6 @@ static ssize_t amdgpu_get_pp_force_state(struct device *dev,
> struct drm_device *ddev = dev_get_drvdata(dev);
> struct amdgpu_device *adev = drm_to_adev(ddev);
>
> - if (amdgpu_in_reset(adev))
> - return -EPERM;
> - if (adev->in_suspend && !adev->in_runpm)
> - return -EPERM;
> -
> if (adev->pm.pp_force_state_enabled)
> return amdgpu_get_pp_cur_state(dev, attr, buf);
> else
> @@ -445,11 +429,6 @@ static ssize_t amdgpu_set_pp_force_state(struct device *dev,
> unsigned long idx;
> int ret;
>
> - if (amdgpu_in_reset(adev))
> - return -EPERM;
> - if (adev->in_suspend && !adev->in_runpm)
> - return -EPERM;
> -
> adev->pm.pp_force_state_enabled = false;
>
> if (strlen(buf) == 1)
> @@ -461,10 +440,6 @@ static ssize_t amdgpu_set_pp_force_state(struct device *dev,
>
> idx = array_index_nospec(idx, ARRAY_SIZE(data.states));
>
> - ret = pm_runtime_resume_and_get(ddev->dev);
> - if (ret < 0)
> - return ret;
> -
> ret = amdgpu_dpm_get_pp_num_states(adev, &data);
> if (ret)
> goto err_out;
> @@ -482,14 +457,9 @@ static ssize_t amdgpu_set_pp_force_state(struct device *dev,
> adev->pm.pp_force_state_enabled = true;
> }
>
> - pm_runtime_mark_last_busy(ddev->dev);
> - pm_runtime_put_autosuspend(ddev->dev);
> -
> return count;
>
> err_out:
> - pm_runtime_mark_last_busy(ddev->dev);
> - pm_runtime_put_autosuspend(ddev->dev);
> return ret;
> }
>
> @@ -511,19 +481,10 @@ static ssize_t amdgpu_get_pp_table(struct device *dev,
> struct drm_device *ddev = dev_get_drvdata(dev);
> struct amdgpu_device *adev = drm_to_adev(ddev);
> char *table = NULL;
> - int size, ret;
> -
> - if (amdgpu_in_reset(adev))
> - return -EPERM;
> -
> - ret = pm_runtime_get_if_active(ddev->dev, true);
> - if (ret <= 0)
> - return ret ?: -EPERM;
> + int size;
>
> size = amdgpu_dpm_get_pp_table(adev, &table);
>
> - pm_runtime_put_autosuspend(ddev->dev);
> -
> if (size <= 0)
> return size;
>
> @@ -542,22 +503,10 @@ static ssize_t amdgpu_set_pp_table(struct device *dev,
> {
> struct drm_device *ddev = dev_get_drvdata(dev);
> struct amdgpu_device *adev = drm_to_adev(ddev);
> - int ret = 0;
> -
> - if (amdgpu_in_reset(adev))
> - return -EPERM;
> - if (adev->in_suspend && !adev->in_runpm)
> - return -EPERM;
> -
> - ret = pm_runtime_resume_and_get(ddev->dev);
> - if (ret < 0)
> - return ret;
> + int ret;
>
> ret = amdgpu_dpm_set_pp_table(adev, buf, count);
>
> - pm_runtime_mark_last_busy(ddev->dev);
> - pm_runtime_put_autosuspend(ddev->dev);
> -
> if (ret)
> return ret;
>
> @@ -725,11 +674,6 @@ static ssize_t amdgpu_set_pp_od_clk_voltage(struct device *dev,
> const char delimiter[3] = {' ', '\n', '\0'};
> uint32_t type;
>
> - if (amdgpu_in_reset(adev))
> - return -EPERM;
> - if (adev->in_suspend && !adev->in_runpm)
> - return -EPERM;
> -
> if (count > 127 || count == 0)
> return -EINVAL;
>
> @@ -775,10 +719,6 @@ static ssize_t amdgpu_set_pp_od_clk_voltage(struct device *dev,
> tmp_str++;
> }
>
> - ret = pm_runtime_resume_and_get(ddev->dev);
> - if (ret < 0)
> - return ret;
> -
> if (amdgpu_dpm_set_fine_grain_clk_vol(adev,
> type,
> parameter,
> @@ -796,14 +736,9 @@ static ssize_t amdgpu_set_pp_od_clk_voltage(struct device *dev,
> goto err_out;
> }
>
> - pm_runtime_mark_last_busy(ddev->dev);
> - pm_runtime_put_autosuspend(ddev->dev);
> -
> return count;
>
> err_out:
> - pm_runtime_mark_last_busy(ddev->dev);
> - pm_runtime_put_autosuspend(ddev->dev);
> return -EINVAL;
> }
>
> @@ -825,13 +760,6 @@ static ssize_t amdgpu_get_pp_od_clk_voltage(struct device *dev,
> };
> uint clk_index;
>
> - if (amdgpu_in_reset(adev))
> - return -EPERM;
> -
> - ret = pm_runtime_get_if_active(ddev->dev, true);
> - if (ret <= 0)
> - return ret ?: -EPERM;
> -
> for (clk_index = 0 ; clk_index < 6 ; clk_index++) {
> ret = amdgpu_dpm_emit_clock_levels(adev, od_clocks[clk_index], buf, &size);
> if (ret)
> @@ -849,8 +777,6 @@ static ssize_t amdgpu_get_pp_od_clk_voltage(struct device *dev,
> if (size == 0)
> size = sysfs_emit(buf, "\n");
>
> - pm_runtime_put_autosuspend(ddev->dev);
> -
> return size;
> }
>
> @@ -880,24 +806,12 @@ static ssize_t amdgpu_set_pp_features(struct device *dev,
> uint64_t featuremask;
> int ret;
>
> - if (amdgpu_in_reset(adev))
> - return -EPERM;
> - if (adev->in_suspend && !adev->in_runpm)
> - return -EPERM;
> -
> ret = kstrtou64(buf, 0, &featuremask);
> if (ret)
> return -EINVAL;
>
> - ret = pm_runtime_resume_and_get(ddev->dev);
> - if (ret < 0)
> - return ret;
> -
> ret = amdgpu_dpm_set_ppfeature_status(adev, featuremask);
>
> - pm_runtime_mark_last_busy(ddev->dev);
> - pm_runtime_put_autosuspend(ddev->dev);
> -
> if (ret)
> return -EINVAL;
>
> @@ -911,21 +825,11 @@ static ssize_t amdgpu_get_pp_features(struct device *dev,
> struct drm_device *ddev = dev_get_drvdata(dev);
> struct amdgpu_device *adev = drm_to_adev(ddev);
> ssize_t size;
> - int ret;
> -
> - if (amdgpu_in_reset(adev))
> - return -EPERM;
> -
> - ret = pm_runtime_get_if_active(ddev->dev, true);
> - if (ret <= 0)
> - return ret ?: -EPERM;
>
> size = amdgpu_dpm_get_ppfeature_status(adev, buf);
> if (size <= 0)
> size = sysfs_emit(buf, "\n");
>
> - pm_runtime_put_autosuspend(ddev->dev);
> -
> return size;
> }
>
> @@ -975,14 +879,7 @@ static ssize_t amdgpu_get_pp_dpm_clock(struct device *dev,
> struct drm_device *ddev = dev_get_drvdata(dev);
> struct amdgpu_device *adev = drm_to_adev(ddev);
> int size = 0;
> - int ret = 0;
> -
> - if (amdgpu_in_reset(adev))
> - return -EPERM;
> -
> - ret = pm_runtime_get_if_active(ddev->dev, true);
> - if (ret <= 0)
> - return ret ?: -EPERM;
> + int ret;
>
> ret = amdgpu_dpm_emit_clock_levels(adev, type, buf, &size);
> if (ret == -ENOENT)
> @@ -991,8 +888,6 @@ static ssize_t amdgpu_get_pp_dpm_clock(struct device *dev,
> if (size == 0)
> size = sysfs_emit(buf, "\n");
>
> - pm_runtime_put_autosuspend(ddev->dev);
> -
> return size;
> }
>
> @@ -1041,24 +936,12 @@ static ssize_t amdgpu_set_pp_dpm_clock(struct device *dev,
> int ret;
> uint32_t mask = 0;
>
> - if (amdgpu_in_reset(adev))
> - return -EPERM;
> - if (adev->in_suspend && !adev->in_runpm)
> - return -EPERM;
> -
> ret = amdgpu_read_mask(buf, count, &mask);
> if (ret)
> return ret;
>
> - ret = pm_runtime_resume_and_get(ddev->dev);
> - if (ret < 0)
> - return ret;
> -
> ret = amdgpu_dpm_force_clock_level(adev, type, mask);
>
> - pm_runtime_mark_last_busy(ddev->dev);
> - pm_runtime_put_autosuspend(ddev->dev);
> -
> if (ret)
> return -EINVAL;
>
> @@ -1221,20 +1104,10 @@ static ssize_t amdgpu_get_pp_sclk_od(struct device *dev,
> {
> struct drm_device *ddev = dev_get_drvdata(dev);
> struct amdgpu_device *adev = drm_to_adev(ddev);
> - uint32_t value = 0;
> - int ret;
> -
> - if (amdgpu_in_reset(adev))
> - return -EPERM;
> -
> - ret = pm_runtime_get_if_active(ddev->dev, true);
> - if (ret <= 0)
> - return ret ?: -EPERM;
> + uint32_t value;
>
> value = amdgpu_dpm_get_sclk_od(adev);
>
> - pm_runtime_put_autosuspend(ddev->dev);
> -
> return sysfs_emit(buf, "%d\n", value);
> }
>
> @@ -1248,25 +1121,13 @@ static ssize_t amdgpu_set_pp_sclk_od(struct device *dev,
> int ret;
> long int value;
>
> - if (amdgpu_in_reset(adev))
> - return -EPERM;
> - if (adev->in_suspend && !adev->in_runpm)
> - return -EPERM;
> -
> ret = kstrtol(buf, 0, &value);
>
> if (ret)
> return -EINVAL;
>
> - ret = pm_runtime_resume_and_get(ddev->dev);
> - if (ret < 0)
> - return ret;
> -
> amdgpu_dpm_set_sclk_od(adev, (uint32_t)value);
>
> - pm_runtime_mark_last_busy(ddev->dev);
> - pm_runtime_put_autosuspend(ddev->dev);
> -
> return count;
> }
>
> @@ -1276,20 +1137,10 @@ static ssize_t amdgpu_get_pp_mclk_od(struct device *dev,
> {
> struct drm_device *ddev = dev_get_drvdata(dev);
> struct amdgpu_device *adev = drm_to_adev(ddev);
> - uint32_t value = 0;
> - int ret;
> -
> - if (amdgpu_in_reset(adev))
> - return -EPERM;
> -
> - ret = pm_runtime_get_if_active(ddev->dev, true);
> - if (ret <= 0)
> - return ret ?: -EPERM;
> + uint32_t value;
>
> value = amdgpu_dpm_get_mclk_od(adev);
>
> - pm_runtime_put_autosuspend(ddev->dev);
> -
> return sysfs_emit(buf, "%d\n", value);
> }
>
> @@ -1303,25 +1154,13 @@ static ssize_t amdgpu_set_pp_mclk_od(struct device *dev,
> int ret;
> long int value;
>
> - if (amdgpu_in_reset(adev))
> - return -EPERM;
> - if (adev->in_suspend && !adev->in_runpm)
> - return -EPERM;
> -
> ret = kstrtol(buf, 0, &value);
>
> if (ret)
> return -EINVAL;
>
> - ret = pm_runtime_resume_and_get(ddev->dev);
> - if (ret < 0)
> - return ret;
> -
> amdgpu_dpm_set_mclk_od(adev, (uint32_t)value);
>
> - pm_runtime_mark_last_busy(ddev->dev);
> - pm_runtime_put_autosuspend(ddev->dev);
> -
> return count;
> }
>
> @@ -1352,21 +1191,11 @@ static ssize_t amdgpu_get_pp_power_profile_mode(struct device *dev,
> struct drm_device *ddev = dev_get_drvdata(dev);
> struct amdgpu_device *adev = drm_to_adev(ddev);
> ssize_t size;
> - int ret;
> -
> - if (amdgpu_in_reset(adev))
> - return -EPERM;
> -
> - ret = pm_runtime_get_if_active(ddev->dev, true);
> - if (ret <= 0)
> - return ret ?: -EPERM;
>
> size = amdgpu_dpm_get_power_profile_mode(adev, buf);
> if (size <= 0)
> size = sysfs_emit(buf, "\n");
>
> - pm_runtime_put_autosuspend(ddev->dev);
> -
> return size;
> }
>
> @@ -1388,11 +1217,6 @@ static ssize_t amdgpu_set_pp_power_profile_mode(struct device *dev,
> long int profile_mode = 0;
> const char delimiter[3] = {' ', '\n', '\0'};
>
> - if (amdgpu_in_reset(adev))
> - return -EPERM;
> - if (adev->in_suspend && !adev->in_runpm)
> - return -EPERM;
> -
> tmp[0] = *(buf);
> tmp[1] = '\0';
> ret = kstrtol(tmp, 0, &profile_mode);
> @@ -1419,15 +1243,8 @@ static ssize_t amdgpu_set_pp_power_profile_mode(struct device *dev,
> }
> parameter[parameter_size] = profile_mode;
>
> - ret = pm_runtime_resume_and_get(ddev->dev);
> - if (ret < 0)
> - return ret;
> -
> ret = amdgpu_dpm_set_power_profile_mode(adev, parameter, parameter_size);
>
> - pm_runtime_mark_last_busy(ddev->dev);
> - pm_runtime_put_autosuspend(ddev->dev);
> -
> if (!ret)
> return count;
>
> @@ -1546,10 +1363,6 @@ static ssize_t amdgpu_get_pcie_bw(struct device *dev,
> struct drm_device *ddev = dev_get_drvdata(dev);
> struct amdgpu_device *adev = drm_to_adev(ddev);
> uint64_t count0 = 0, count1 = 0;
> - int ret;
> -
> - if (amdgpu_in_reset(adev))
> - return -EPERM;
>
> if (adev->flags & AMD_IS_APU)
> return -ENODATA;
> @@ -1557,14 +1370,8 @@ static ssize_t amdgpu_get_pcie_bw(struct device *dev,
> if (!adev->asic_funcs->get_pcie_usage)
> return -ENODATA;
>
> - ret = pm_runtime_get_if_active(ddev->dev, true);
> - if (ret <= 0)
> - return ret ?: -EPERM;
> -
> amdgpu_asic_get_pcie_usage(adev, &count0, &count1);
>
> - pm_runtime_put_autosuspend(ddev->dev);
> -
> return sysfs_emit(buf, "%llu %llu %i\n",
> count0, count1, pcie_get_mps(adev->pdev));
> }
> @@ -1586,11 +1393,6 @@ static ssize_t amdgpu_get_unique_id(struct device *dev,
> struct drm_device *ddev = dev_get_drvdata(dev);
> struct amdgpu_device *adev = drm_to_adev(ddev);
>
> - if (amdgpu_in_reset(adev))
> - return -EPERM;
> - if (adev->in_suspend && !adev->in_runpm)
> - return -EPERM;
> -
> if (adev->unique_id)
> return sysfs_emit(buf, "%016llx\n", adev->unique_id);
>
> @@ -1685,18 +1487,12 @@ static ssize_t amdgpu_get_apu_thermal_cap(struct device *dev,
> struct drm_device *ddev = dev_get_drvdata(dev);
> struct amdgpu_device *adev = drm_to_adev(ddev);
>
> - ret = pm_runtime_get_if_active(ddev->dev, true);
> - if (ret <= 0)
> - return ret ?: -EPERM;
> -
> ret = amdgpu_dpm_get_apu_thermal_limit(adev, &limit);
> if (!ret)
> size = sysfs_emit(buf, "%u\n", limit);
> else
> size = sysfs_emit(buf, "failed to get thermal limit\n");
>
> - pm_runtime_put_autosuspend(ddev->dev);
> -
> return size;
> }
>
> @@ -1719,21 +1515,12 @@ static ssize_t amdgpu_set_apu_thermal_cap(struct device *dev,
> return -EINVAL;
> }
>
> - ret = pm_runtime_resume_and_get(ddev->dev);
> - if (ret < 0)
> - return ret;
> -
> ret = amdgpu_dpm_set_apu_thermal_limit(adev, value);
> if (ret) {
> - pm_runtime_mark_last_busy(ddev->dev);
> - pm_runtime_put_autosuspend(ddev->dev);
> dev_err(dev, "failed to update thermal limit\n");
> return ret;
> }
>
> - pm_runtime_mark_last_busy(ddev->dev);
> - pm_runtime_put_autosuspend(ddev->dev);
> -
> return count;
> }
>
> @@ -1753,21 +1540,8 @@ static ssize_t amdgpu_get_pm_metrics(struct device *dev,
> {
> struct drm_device *ddev = dev_get_drvdata(dev);
> struct amdgpu_device *adev = drm_to_adev(ddev);
> - ssize_t size = 0;
> - int ret;
> -
> - if (amdgpu_in_reset(adev))
> - return -EPERM;
> -
> - ret = pm_runtime_get_if_active(ddev->dev, true);
> - if (ret <= 0)
> - return ret ?: -EPERM;
>
> - size = amdgpu_dpm_get_pm_metrics(adev, buf, PAGE_SIZE);
> -
> - pm_runtime_put_autosuspend(ddev->dev);
> -
> - return size;
> + return amdgpu_dpm_get_pm_metrics(adev, buf, PAGE_SIZE);
> }
>
> /**
> @@ -1789,15 +1563,7 @@ static ssize_t amdgpu_get_gpu_metrics(struct device *dev,
> struct drm_device *ddev = dev_get_drvdata(dev);
> struct amdgpu_device *adev = drm_to_adev(ddev);
> void *gpu_metrics;
> - ssize_t size = 0;
> - int ret;
> -
> - if (amdgpu_in_reset(adev))
> - return -EPERM;
> -
> - ret = pm_runtime_get_if_active(ddev->dev, true);
> - if (ret <= 0)
> - return ret ?: -EPERM;
> + ssize_t size;
>
> size = amdgpu_dpm_get_gpu_metrics(adev, &gpu_metrics);
> if (size <= 0)
> @@ -1809,8 +1575,6 @@ static ssize_t amdgpu_get_gpu_metrics(struct device *dev,
> memcpy(buf, gpu_metrics, size);
>
> out:
> - pm_runtime_put_autosuspend(ddev->dev);
> -
> return size;
> }
>
> @@ -1900,23 +1664,12 @@ static ssize_t amdgpu_set_smartshift_bias(struct device *dev,
> struct device_attribute *attr,
> const char *buf, size_t count)
> {
> - struct drm_device *ddev = dev_get_drvdata(dev);
> - struct amdgpu_device *adev = drm_to_adev(ddev);
> - int r = 0;
> + int r;
> int bias = 0;
>
> - if (amdgpu_in_reset(adev))
> - return -EPERM;
> - if (adev->in_suspend && !adev->in_runpm)
> - return -EPERM;
> -
> - r = pm_runtime_resume_and_get(ddev->dev);
> - if (r < 0)
> - return r;
> -
> r = kstrtoint(buf, 10, &bias);
> if (r)
> - goto out;
> + return r;
>
> if (bias > AMDGPU_SMARTSHIFT_MAX_BIAS)
> bias = AMDGPU_SMARTSHIFT_MAX_BIAS;
> @@ -1928,9 +1681,6 @@ static ssize_t amdgpu_set_smartshift_bias(struct device *dev,
>
> /* TODO: update bias level with SMU message */
>
> -out:
> - pm_runtime_mark_last_busy(ddev->dev);
> - pm_runtime_put_autosuspend(ddev->dev);
> return r;
> }
>
> @@ -2184,11 +1934,6 @@ static ssize_t amdgpu_get_pm_policy_attr(struct device *dev,
> policy_attr =
> container_of(attr, struct amdgpu_pm_policy_attr, dev_attr);
>
> - if (amdgpu_in_reset(adev))
> - return -EPERM;
> - if (adev->in_suspend && !adev->in_runpm)
> - return -EPERM;
> -
> return amdgpu_dpm_get_pm_policy_info(adev, policy_attr->id, buf);
> }
>
> @@ -2205,11 +1950,6 @@ static ssize_t amdgpu_set_pm_policy_attr(struct device *dev,
> char *tmp, *param;
> long val;
>
> - if (amdgpu_in_reset(adev))
> - return -EPERM;
> - if (adev->in_suspend && !adev->in_runpm)
> - return -EPERM;
> -
> count = min(count, sizeof(tmp_buf));
> memcpy(tmp_buf, buf, count);
> tmp_buf[count - 1] = '\0';
> @@ -2235,15 +1975,8 @@ static ssize_t amdgpu_set_pm_policy_attr(struct device *dev,
> policy_attr =
> container_of(attr, struct amdgpu_pm_policy_attr, dev_attr);
>
> - ret = pm_runtime_resume_and_get(ddev->dev);
> - if (ret < 0)
> - return ret;
> -
> ret = amdgpu_dpm_set_pm_policy(adev, policy_attr->id, val);
>
> - pm_runtime_mark_last_busy(ddev->dev);
> - pm_runtime_put_autosuspend(ddev->dev);
> -
> if (ret)
> return ret;
>
> diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_pm.h b/drivers/gpu/drm/amd/pm/inc/amdgpu_pm.h
> index c12ced32f780..bf4e27b28330 100644
> --- a/drivers/gpu/drm/amd/pm/inc/amdgpu_pm.h
> +++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_pm.h
> @@ -88,6 +88,10 @@ struct amdgpu_device_attr {
> int (*attr_update)(struct amdgpu_device *adev, struct amdgpu_device_attr *attr,
> uint32_t mask, enum amdgpu_device_attr_states *states);
>
> + ssize_t (*attr_show)(struct device *dev, struct device_attribute *attr,
> + char *buf);
> + ssize_t (*attr_store)(struct device *dev, struct device_attribute *attr,
> + const char *buf, size_t count);
> };
>
> struct amdgpu_device_attr_entry {
> @@ -98,10 +102,14 @@ struct amdgpu_device_attr_entry {
> #define to_amdgpu_device_attr(_dev_attr) \
> container_of(_dev_attr, struct amdgpu_device_attr, dev_attr)
>
> -#define __AMDGPU_DEVICE_ATTR(_name, _mode, _show, _store, _flags, ...) \
> - { .dev_attr = __ATTR(_name, _mode, _show, _store), \
> - .attr_id = device_attr_id__##_name, \
> - .flags = _flags, \
> +#define __AMDGPU_DEVICE_ATTR(_name, _mode, _show, _store, _flags, ...) \
> + { .dev_attr = __ATTR(_name, _mode, \
> + amdgpu_pp_attr_show_wrapper, \
> + _store ? amdgpu_pp_attr_store_wrapper : NULL), \
FYI this triggers a warning on some compilers (the address of xxx will always evaluate as true) so
I'll update this patch before merging to use AMDGPU_DEVICE_ATTR_RO / AMDGPU_DEVICE_ATTR_RW macros
from the next patch.
Thanks,
Pierre-Eric
> + .attr_id = device_attr_id__##_name, \
> + .flags = _flags, \
> + .attr_show = _show, \
> + .attr_store = _store, \
> ##__VA_ARGS__, }
>
> #define AMDGPU_DEVICE_ATTR(_name, _mode, _flags, ...) \
More information about the amd-gfx
mailing list