[PATCH v2] drm/amd/pm: consistent approach for smartshift
Lazar, Lijo
lijo.lazar at amd.com
Thu May 12 11:07:40 UTC 2022
On 5/12/2022 4:26 PM, Sathishkumar S wrote:
> create smartshift sysfs attributes from dGPU device even
> on smartshift 1.0 platform to be consistent. Do not populate
> the attributes on platforms that have APU only but not dGPU
> or vice versa.
>
> V2: avoid checking for the number of VGA/DISPLAY devices (Lijo)
>
> Suggested-by: Alex Deucher <alexander.deucher at amd.com>
> Signed-off-by: Sathishkumar S <sathishkumar.sundararaju at amd.com>
> Acked-by: Alex Deucher <alexander.deucher at amd.com>
> ---
> drivers/gpu/drm/amd/pm/amdgpu_pm.c | 136 ++++++++++++++++-------------
> 1 file changed, 74 insertions(+), 62 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
> index d3228216b2da..292e8c241597 100644
> --- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
> +++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
> @@ -1734,22 +1734,11 @@ static ssize_t amdgpu_get_gpu_metrics(struct device *dev,
> return size;
> }
>
> -/**
> - * DOC: smartshift_apu_power
> - *
> - * The amdgpu driver provides a sysfs API for reporting APU power
> - * share if it supports smartshift. The value is expressed as
> - * the proportion of stapm limit where stapm limit is the total APU
> - * power limit. The result is in percentage. If APU power is 130% of
> - * STAPM, then APU is using 30% of the dGPU's headroom.
> - */
> -
> -static ssize_t amdgpu_get_smartshift_apu_power(struct device *dev, struct device_attribute *attr,
> - char *buf)
> +static int amdgpu_read_powershift_percent(struct amdgpu_device *adev,
> + uint32_t *ss_power, bool dgpu_share)
> {
> - struct drm_device *ddev = dev_get_drvdata(dev);
> - struct amdgpu_device *adev = drm_to_adev(ddev);
> - uint32_t ss_power, size;
> + struct drm_device *ddev = adev_to_drm(adev);
> + uint32_t size;
> int r = 0;
>
> if (amdgpu_in_reset(adev))
> @@ -1763,28 +1752,65 @@ static ssize_t amdgpu_get_smartshift_apu_power(struct device *dev, struct device
> return r;
> }
>
> - r = amdgpu_dpm_read_sensor(adev, AMDGPU_PP_SENSOR_SS_APU_SHARE,
> - (void *)&ss_power, &size);
> - if (r)
> - goto out;
> -
> - r = sysfs_emit(buf, "%u%%\n", ss_power);
> + if (dgpu_share)
> + r = amdgpu_dpm_read_sensor(adev, AMDGPU_PP_SENSOR_SS_DGPU_SHARE,
> + (void *)ss_power, &size);
> + else
> + r = amdgpu_dpm_read_sensor(adev, AMDGPU_PP_SENSOR_SS_APU_SHARE,
> + (void *)ss_power, &size);
>
> -out:
> pm_runtime_mark_last_busy(ddev->dev);
> pm_runtime_put_autosuspend(ddev->dev);
> return r;
> }
> +/**
> + * DOC: smartshift_apu_power
> + *
> + * The amdgpu driver provides a sysfs API for reporting APU power
> + * shift in percentage if platform supports smartshift. Value 0 means that
> + * there is no powershift and values between [1-100] means that the power
> + * is shifted to APU, the percentage of boost is with respect to APU power
> + * limit on the platform.
> + */
> +
> +static ssize_t amdgpu_get_smartshift_apu_power(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);
> + uint32_t ss_power = 0;
> + int r = 0, i;
> +
> + r = amdgpu_read_powershift_percent(adev, &ss_power, false);
> + if (r == -EOPNOTSUPP) {
> + /* sensor not available on dGPU, try to read from APU */
> + adev = NULL;
> + mutex_lock(&mgpu_info.mutex);
> + for (i = 0; i < mgpu_info.num_gpu; i++) {
> + if (mgpu_info.gpu_ins[i].adev->flags & AMD_IS_APU) {
> + adev = mgpu_info.gpu_ins[i].adev;
> + break;
> + }
> + }
> + mutex_unlock(&mgpu_info.mutex);
> + if (adev)
> + r = amdgpu_read_powershift_percent(adev, &ss_power, false);
> + }
> +
> + if (!r)
> + r = sysfs_emit(buf, "%u%%\n", ss_power);
> +
In v1 patch comment, I meant something like this
return amdgpu_show_powershift_percent(adev, false);
And the above logic is kept in amdgpu_show_powershift_percent();
amdgpu_get_smartshift_dgpu_power():
return amdgpu_show_powershift_percent(adev, true);
Thanks,
Lijo
> + return r;
> +}
>
> /**
> * DOC: smartshift_dgpu_power
> *
> - * The amdgpu driver provides a sysfs API for reporting the dGPU power
> - * share if the device is in HG and supports smartshift. The value
> - * is expressed as the proportion of stapm limit where stapm limit
> - * is the total APU power limit. The value is in percentage. If dGPU
> - * power is 20% higher than STAPM power(120%), it's using 20% of the
> - * APU's power headroom.
> + * The amdgpu driver provides a sysfs API for reporting dGPU power
> + * shift in percentage if platform supports smartshift. Value 0 means that
> + * there is no powershift and values between [1-100] means that the power is
> + * shifted to dGPU, the percentage of boost is with respect to dGPU power
> + * limit on the platform.
> */
>
> static ssize_t amdgpu_get_smartshift_dgpu_power(struct device *dev, struct device_attribute *attr,
> @@ -1792,31 +1818,28 @@ static ssize_t amdgpu_get_smartshift_dgpu_power(struct device *dev, struct devic
> {
> struct drm_device *ddev = dev_get_drvdata(dev);
> struct amdgpu_device *adev = drm_to_adev(ddev);
> - uint32_t ss_power, size;
> - int r = 0;
> -
> - if (amdgpu_in_reset(adev))
> - return -EPERM;
> - if (adev->in_suspend && !adev->in_runpm)
> - return -EPERM;
> -
> - r = pm_runtime_get_sync(ddev->dev);
> - if (r < 0) {
> - pm_runtime_put_autosuspend(ddev->dev);
> - return r;
> + uint32_t ss_power = 0;
> + int r = 0, i;
> +
> + r = amdgpu_read_powershift_percent(adev, &ss_power, true);
> + if (r == -EOPNOTSUPP) {
> + /* sensor not available on dGPU, try to read from APU */
> + adev = NULL;
> + mutex_lock(&mgpu_info.mutex);
> + for (i = 0; i < mgpu_info.num_gpu; i++) {
> + if (mgpu_info.gpu_ins[i].adev->flags & AMD_IS_APU) {
> + adev = mgpu_info.gpu_ins[i].adev;
> + break;
> + }
> + }
> + mutex_unlock(&mgpu_info.mutex);
> + if (adev)
> + r = amdgpu_read_powershift_percent(adev, &ss_power, true);
> }
>
> - r = amdgpu_dpm_read_sensor(adev, AMDGPU_PP_SENSOR_SS_DGPU_SHARE,
> - (void *)&ss_power, &size);
> -
> - if (r)
> - goto out;
> -
> - r = sysfs_emit(buf, "%u%%\n", ss_power);
> + if (!r)
> + r = sysfs_emit(buf, "%u%%\n", ss_power);
>
> -out:
> - pm_runtime_mark_last_busy(ddev->dev);
> - pm_runtime_put_autosuspend(ddev->dev);
> return r;
> }
>
> @@ -1884,18 +1907,7 @@ static ssize_t amdgpu_set_smartshift_bias(struct device *dev,
> static int ss_power_attr_update(struct amdgpu_device *adev, struct amdgpu_device_attr *attr,
> uint32_t mask, enum amdgpu_device_attr_states *states)
> {
> - uint32_t ss_power, size;
> -
> - if (!amdgpu_acpi_is_power_shift_control_supported())
> - *states = ATTR_STATE_UNSUPPORTED;
> - else if ((adev->flags & AMD_IS_PX) &&
> - !amdgpu_device_supports_smart_shift(adev_to_drm(adev)))
> - *states = ATTR_STATE_UNSUPPORTED;
> - else if (amdgpu_dpm_read_sensor(adev, AMDGPU_PP_SENSOR_SS_APU_SHARE,
> - (void *)&ss_power, &size))
> - *states = ATTR_STATE_UNSUPPORTED;
> - else if (amdgpu_dpm_read_sensor(adev, AMDGPU_PP_SENSOR_SS_DGPU_SHARE,
> - (void *)&ss_power, &size))
> + if (!amdgpu_device_supports_smart_shift(adev_to_drm(adev)))
> *states = ATTR_STATE_UNSUPPORTED;
>
> return 0;
>
More information about the amd-gfx
mailing list