[PATCH] drm/radeon/pm: handle powered down GPUs properly in sysfs/debugfs

[PATCH] drm/radeon/pm: handle powered down GPUs properly in sysfs/debugfs

[Alex Deucher]

When we power down the dGPU on a PX system, bail if the
user tried to adjust the power state or check the temperature.
The GPU is powered down, so it doesn't make sense to actually
do anything.  We could power up the dGPU to complete the
operation, but it would just be undone again as soon as it was
completed as the card would be powered down again.  Return 0
for temperature and return -EINVAL for other interfaces.

bug:
https://bugzilla.kernel.org/show_bug.cgi?id=70651

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Cc: stable@vger.kernel.org
---
 drivers/gpu/drm/radeon/radeon_pm.c | 29 ++++++++++++++++++++++++++---
 1 file changed, 26 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_pm.c b/drivers/gpu/drm/radeon/radeon_pm.c
index 8e8153e..6f20bb0 100644
--- a/drivers/gpu/drm/radeon/radeon_pm.c
+++ b/drivers/gpu/drm/radeon/radeon_pm.c
@@ -67,6 +67,11 @@ int radeon_pm_get_type_index(struct radeon_device *rdev,
 
 void radeon_pm_acpi_event_handler(struct radeon_device *rdev)
 {
+	struct drm_device *ddev = rdev->ddev;
+
+	if (ddev->switch_power_state == DRM_SWITCH_POWER_OFF)
+		return;
+
 	if ((rdev->pm.pm_method == PM_METHOD_DPM) && rdev->pm.dpm_enabled) {
 		mutex_lock(&rdev->pm.mutex);
 		if (power_supply_is_system_supplied() > 0)
@@ -361,6 +366,9 @@ static ssize_t radeon_set_pm_profile(struct device *dev,
 	struct drm_device *ddev = dev_get_drvdata(dev);
 	struct radeon_device *rdev = ddev->dev_private;
 
+	if (ddev->switch_power_state == DRM_SWITCH_POWER_OFF)
+		return -EINVAL;
+
 	mutex_lock(&rdev->pm.mutex);
 	if (rdev->pm.pm_method == PM_METHOD_PROFILE) {
 		if (strncmp("default", buf, strlen("default")) == 0)
@@ -410,7 +418,8 @@ static ssize_t radeon_set_pm_method(struct device *dev,
 	struct radeon_device *rdev = ddev->dev_private;
 
 	/* we don't support the legacy modes with dpm */
-	if (rdev->pm.pm_method == PM_METHOD_DPM) {
+	if ((rdev->pm.pm_method == PM_METHOD_DPM) ||
+	    (ddev->switch_power_state == DRM_SWITCH_POWER_OFF)) {
 		count = -EINVAL;
 		goto fail;
 	}
@@ -459,6 +468,11 @@ static ssize_t radeon_set_dpm_state(struct device *dev,
 	struct drm_device *ddev = dev_get_drvdata(dev);
 	struct radeon_device *rdev = ddev->dev_private;
 
+	if (ddev->switch_power_state == DRM_SWITCH_POWER_OFF) {
+		count = -EINVAL;
+		goto fail;
+	}
+
 	mutex_lock(&rdev->pm.mutex);
 	if (strncmp("battery", buf, strlen("battery")) == 0)
 		rdev->pm.dpm.user_state = POWER_STATE_TYPE_BATTERY;
@@ -500,6 +514,9 @@ static ssize_t radeon_set_dpm_forced_performance_level(struct device *dev,
 	enum radeon_dpm_forced_level level;
 	int ret = 0;
 
+	if (ddev->switch_power_state == DRM_SWITCH_POWER_OFF)
+		return -EINVAL;
+
 	mutex_lock(&rdev->pm.mutex);
 	if (strncmp("low", buf, strlen("low")) == 0) {
 		level = RADEON_DPM_FORCED_LEVEL_LOW;
@@ -538,9 +555,13 @@ static ssize_t radeon_hwmon_show_temp(struct device *dev,
 				      char *buf)
 {
 	struct radeon_device *rdev = dev_get_drvdata(dev);
+	struct drm_device *ddev = rdev->ddev;
 	int temp;
 
-	if (rdev->asic->pm.get_temperature)
+	/* return 0 if PX card is off */
+	if (ddev->switch_power_state == DRM_SWITCH_POWER_OFF)
+		temp = 0;
+	else if (rdev->asic->pm.get_temperature)
 		temp = radeon_get_temperature(rdev);
 	else
 		temp = 0;
@@ -1579,7 +1600,9 @@ static int radeon_debugfs_pm_info(struct seq_file *m, void *data)
 	struct drm_device *dev = node->minor->dev;
 	struct radeon_device *rdev = dev->dev_private;
 
-	if (rdev->pm.dpm_enabled) {
+	if (dev->switch_power_state == DRM_SWITCH_POWER_OFF) {
+		seq_printf(m, "Chip powered off\n");
+	} else if (rdev->pm.dpm_enabled) {
 		mutex_lock(&rdev->pm.mutex);
 		if (rdev->asic->dpm.debugfs_print_current_performance_level)
 			radeon_dpm_debugfs_print_current_performance_level(rdev, m);
-- 
1.8.3.1

Re: [PATCH] drm/radeon/pm: handle powered down GPUs properly in sysfs/debugfs

[Christian König]

Am 21.02.2014 16:50, schrieb Alex Deucher:
> When we power down the dGPU on a PX system, bail if the
> user tried to adjust the power state or check the temperature.
> The GPU is powered down, so it doesn't make sense to actually
> do anything.  We could power up the dGPU to complete the
> operation, but it would just be undone again as soon as it was
> completed as the card would be powered down again.  Return 0
> for temperature and return -EINVAL for other interfaces.
>
> bug:
> https://bugzilla.kernel.org/show_bug.cgi?id=70651
>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> Cc: stable@vger.kernel.org

Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>   drivers/gpu/drm/radeon/radeon_pm.c | 29 ++++++++++++++++++++++++++---
>   1 file changed, 26 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_pm.c b/drivers/gpu/drm/radeon/radeon_pm.c
> index 8e8153e..6f20bb0 100644
> --- a/drivers/gpu/drm/radeon/radeon_pm.c
> +++ b/drivers/gpu/drm/radeon/radeon_pm.c
> @@ -67,6 +67,11 @@ int radeon_pm_get_type_index(struct radeon_device *rdev,
>   
>   void radeon_pm_acpi_event_handler(struct radeon_device *rdev)
>   {
> +	struct drm_device *ddev = rdev->ddev;
> +
> +	if (ddev->switch_power_state == DRM_SWITCH_POWER_OFF)
> +		return;
> +
>   	if ((rdev->pm.pm_method == PM_METHOD_DPM) && rdev->pm.dpm_enabled) {
>   		mutex_lock(&rdev->pm.mutex);
>   		if (power_supply_is_system_supplied() > 0)
> @@ -361,6 +366,9 @@ static ssize_t radeon_set_pm_profile(struct device *dev,
>   	struct drm_device *ddev = dev_get_drvdata(dev);
>   	struct radeon_device *rdev = ddev->dev_private;
>   
> +	if (ddev->switch_power_state == DRM_SWITCH_POWER_OFF)
> +		return -EINVAL;
> +
>   	mutex_lock(&rdev->pm.mutex);
>   	if (rdev->pm.pm_method == PM_METHOD_PROFILE) {
>   		if (strncmp("default", buf, strlen("default")) == 0)
> @@ -410,7 +418,8 @@ static ssize_t radeon_set_pm_method(struct device *dev,
>   	struct radeon_device *rdev = ddev->dev_private;
>   
>   	/* we don't support the legacy modes with dpm */
> -	if (rdev->pm.pm_method == PM_METHOD_DPM) {
> +	if ((rdev->pm.pm_method == PM_METHOD_DPM) ||
> +	    (ddev->switch_power_state == DRM_SWITCH_POWER_OFF)) {
>   		count = -EINVAL;
>   		goto fail;
>   	}
> @@ -459,6 +468,11 @@ static ssize_t radeon_set_dpm_state(struct device *dev,
>   	struct drm_device *ddev = dev_get_drvdata(dev);
>   	struct radeon_device *rdev = ddev->dev_private;
>   
> +	if (ddev->switch_power_state == DRM_SWITCH_POWER_OFF) {
> +		count = -EINVAL;
> +		goto fail;
> +	}
> +
>   	mutex_lock(&rdev->pm.mutex);
>   	if (strncmp("battery", buf, strlen("battery")) == 0)
>   		rdev->pm.dpm.user_state = POWER_STATE_TYPE_BATTERY;
> @@ -500,6 +514,9 @@ static ssize_t radeon_set_dpm_forced_performance_level(struct device *dev,
>   	enum radeon_dpm_forced_level level;
>   	int ret = 0;
>   
> +	if (ddev->switch_power_state == DRM_SWITCH_POWER_OFF)
> +		return -EINVAL;
> +
>   	mutex_lock(&rdev->pm.mutex);
>   	if (strncmp("low", buf, strlen("low")) == 0) {
>   		level = RADEON_DPM_FORCED_LEVEL_LOW;
> @@ -538,9 +555,13 @@ static ssize_t radeon_hwmon_show_temp(struct device *dev,
>   				      char *buf)
>   {
>   	struct radeon_device *rdev = dev_get_drvdata(dev);
> +	struct drm_device *ddev = rdev->ddev;
>   	int temp;
>   
> -	if (rdev->asic->pm.get_temperature)
> +	/* return 0 if PX card is off */
> +	if (ddev->switch_power_state == DRM_SWITCH_POWER_OFF)
> +		temp = 0;
> +	else if (rdev->asic->pm.get_temperature)
>   		temp = radeon_get_temperature(rdev);
>   	else
>   		temp = 0;
> @@ -1579,7 +1600,9 @@ static int radeon_debugfs_pm_info(struct seq_file *m, void *data)
>   	struct drm_device *dev = node->minor->dev;
>   	struct radeon_device *rdev = dev->dev_private;
>   
> -	if (rdev->pm.dpm_enabled) {
> +	if (dev->switch_power_state == DRM_SWITCH_POWER_OFF) {
> +		seq_printf(m, "Chip powered off\n");
> +	} else if (rdev->pm.dpm_enabled) {
>   		mutex_lock(&rdev->pm.mutex);
>   		if (rdev->asic->dpm.debugfs_print_current_performance_level)
>   			radeon_dpm_debugfs_print_current_performance_level(rdev, m);