[PATCH 01/10] drm/amdgpu: handle runtime pm in drm pre/post close

[PATCH 01/10] drm/amdgpu: handle runtime pm in drm pre/post close

[Alex Deucher]

Close was not handled correctly.

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index f1b9e0f..45f98b4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -600,6 +600,9 @@ void amdgpu_driver_postclose_kms(struct drm_device *dev,
 
 	kfree(fpriv);
 	file_priv->driver_priv = NULL;
+
+	pm_runtime_mark_last_busy(dev->dev);
+	pm_runtime_put_autosuspend(dev->dev);
 }
 
 /**
@@ -614,6 +617,7 @@ void amdgpu_driver_postclose_kms(struct drm_device *dev,
 void amdgpu_driver_preclose_kms(struct drm_device *dev,
 				struct drm_file *file_priv)
 {
+	pm_runtime_get_sync(dev->dev);
 }
 
 /*
-- 
2.5.5

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 02/10] drm/amdgpu: handle runtime pm correctly in amdgpu_driver_open_kms

[Alex Deucher]

Need to fix the error pathes.

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index 45f98b4..c2c7fb1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -542,12 +542,16 @@ int amdgpu_driver_open_kms(struct drm_device *dev, struct drm_file *file_priv)
 		return r;
 
 	fpriv = kzalloc(sizeof(*fpriv), GFP_KERNEL);
-	if (unlikely(!fpriv))
-		return -ENOMEM;
+	if (unlikely(!fpriv)) {
+		r = -ENOMEM;
+		goto out_suspend;
+	}
 
 	r = amdgpu_vm_init(adev, &fpriv->vm);
-	if (r)
-		goto error_free;
+	if (r) {
+		kfree(fpriv);
+		goto out_suspend;
+	}
 
 	mutex_init(&fpriv->bo_list_lock);
 	idr_init(&fpriv->bo_list_handles);
@@ -556,12 +560,9 @@ int amdgpu_driver_open_kms(struct drm_device *dev, struct drm_file *file_priv)
 
 	file_priv->driver_priv = fpriv;
 
+out_suspend:
 	pm_runtime_mark_last_busy(dev->dev);
 	pm_runtime_put_autosuspend(dev->dev);
-	return 0;
-
-error_free:
-	kfree(fpriv);
 
 	return r;
 }
-- 
2.5.5

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 03/10] drm/amdgpu: handle runtime pm in fbcon

[Alex Deucher]

Ported from nouveau.

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
index 9191467..eaa5dcc 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
@@ -26,6 +26,7 @@
 #include <linux/module.h>
 #include <linux/slab.h>
 #include <linux/fb.h>
+#include <linux/pm_runtime.h>
 
 #include <drm/drmP.h>
 #include <drm/drm_crtc.h>
@@ -48,8 +49,32 @@ struct amdgpu_fbdev {
 	struct amdgpu_device *adev;
 };
 
+static int
+amdgpufb_open(struct fb_info *info, int user)
+{
+	struct amdgpu_fbdev *rfbdev = info->par;
+	struct amdgpu_device *adev = rfbdev->adev;
+	int ret = pm_runtime_get_sync(adev->ddev->dev);
+	if (ret < 0 && ret != -EACCES)
+		return ret;
+	return 0;
+}
+
+static int
+amdgpufb_release(struct fb_info *info, int user)
+{
+	struct amdgpu_fbdev *rfbdev = info->par;
+	struct amdgpu_device *adev = rfbdev->adev;
+
+	pm_runtime_mark_last_busy(adev->ddev->dev);
+	pm_runtime_put_autosuspend(adev->ddev->dev);
+	return 0;
+}
+
 static struct fb_ops amdgpufb_ops = {
 	.owner = THIS_MODULE,
+	.fb_open = amdgpufb_open,
+	.fb_release = amdgpufb_release,
 	.fb_check_var = drm_fb_helper_check_var,
 	.fb_set_par = drm_fb_helper_set_par,
 	.fb_fillrect = drm_fb_helper_cfb_fillrect,
-- 
2.5.5

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 04/10] drm/amdgpu: skip suspend/resume on DRM_SWITCH_POWER_DYNAMIC_OFF

[Alex Deucher]

Missed this case previously. No need to do anything if the
device is already off.  runtime pm will handle it.

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 2f96334..e6b34dc 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -1842,7 +1842,8 @@ int amdgpu_device_suspend(struct drm_device *dev, bool suspend, bool fbcon,
 
 	adev = dev->dev_private;
 
-	if (dev->switch_power_state == DRM_SWITCH_POWER_OFF)
+	if (dev->switch_power_state == DRM_SWITCH_POWER_OFF ||
+	    dev->switch_power_state == DRM_SWITCH_POWER_DYNAMIC_OFF)
 		return 0;
 
 	drm_kms_helper_poll_disable(dev);
@@ -1926,7 +1927,8 @@ int amdgpu_device_resume(struct drm_device *dev, bool resume, bool fbcon,
 	struct drm_crtc *crtc;
 	int r;
 
-	if (dev->switch_power_state == DRM_SWITCH_POWER_OFF)
+	if (dev->switch_power_state == DRM_SWITCH_POWER_OFF ||
+	    dev->switch_power_state == DRM_SWITCH_POWER_DYNAMIC_OFF)
 		return 0;
 
 	if (fbcon) {
-- 
2.5.5

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 05/10] drm/amdgpu: set runtime pm state to active on resume

[Alex Deucher]

The sbios always powers up the dGPU on resume.

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 716adb5..3426ae2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -495,6 +495,14 @@ static int amdgpu_pmops_resume(struct device *dev)
 {
 	struct pci_dev *pdev = to_pci_dev(dev);
 	struct drm_device *drm_dev = pci_get_drvdata(pdev);
+
+	/* GPU comes up enabled by the bios on resume */
+	if (amdgpu_device_is_px(drm_dev)) {
+		pm_runtime_disable(dev);
+		pm_runtime_set_active(dev);
+		pm_runtime_enable(dev);
+	}
+
 	return amdgpu_device_resume(drm_dev, true, true, false);
 }
 
-- 
2.5.5

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 06/10] drm/radeon: handle runtime pm in drm pre/post close

[Alex Deucher]

Close was not handled correctly.

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/radeon/radeon_kms.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/radeon/radeon_kms.c b/drivers/gpu/drm/radeon/radeon_kms.c
index 835563c..0252527 100644
--- a/drivers/gpu/drm/radeon/radeon_kms.c
+++ b/drivers/gpu/drm/radeon/radeon_kms.c
@@ -717,6 +717,8 @@ void radeon_driver_postclose_kms(struct drm_device *dev,
 		kfree(fpriv);
 		file_priv->driver_priv = NULL;
 	}
+	pm_runtime_mark_last_busy(dev->dev);
+	pm_runtime_put_autosuspend(dev->dev);
 }
 
 /**
@@ -733,6 +735,8 @@ void radeon_driver_preclose_kms(struct drm_device *dev,
 {
 	struct radeon_device *rdev = dev->dev_private;
 
+	pm_runtime_get_sync(dev->dev);
+
 	mutex_lock(&rdev->gem.mutex);
 	if (rdev->hyperz_filp == file_priv)
 		rdev->hyperz_filp = NULL;
-- 
2.5.5

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 07/10] drm/radeon: handle runtime pm correctly in amdgpu_driver_open_kms

[Alex Deucher]

Need to fix the error paths.

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/radeon/radeon_kms.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_kms.c b/drivers/gpu/drm/radeon/radeon_kms.c
index 0252527..4388dde 100644
--- a/drivers/gpu/drm/radeon/radeon_kms.c
+++ b/drivers/gpu/drm/radeon/radeon_kms.c
@@ -641,11 +641,11 @@ int radeon_driver_open_kms(struct drm_device *dev, struct drm_file *file_priv)
 	if (rdev->family >= CHIP_CAYMAN) {
 		struct radeon_fpriv *fpriv;
 		struct radeon_vm *vm;
-		int r;
 
 		fpriv = kzalloc(sizeof(*fpriv), GFP_KERNEL);
 		if (unlikely(!fpriv)) {
-			return -ENOMEM;
+			r = -ENOMEM;
+			goto out_suspend;
 		}
 
 		if (rdev->accel_working) {
@@ -653,14 +653,14 @@ int radeon_driver_open_kms(struct drm_device *dev, struct drm_file *file_priv)
 			r = radeon_vm_init(rdev, vm);
 			if (r) {
 				kfree(fpriv);
-				return r;
+				goto out_suspend;
 			}
 
 			r = radeon_bo_reserve(rdev->ring_tmp_bo.bo, false);
 			if (r) {
 				radeon_vm_fini(rdev, vm);
 				kfree(fpriv);
-				return r;
+				goto out_suspend;
 			}
 
 			/* map the ib pool buffer read only into
@@ -674,15 +674,16 @@ int radeon_driver_open_kms(struct drm_device *dev, struct drm_file *file_priv)
 			if (r) {
 				radeon_vm_fini(rdev, vm);
 				kfree(fpriv);
-				return r;
+				goto out_suspend;
 			}
 		}
 		file_priv->driver_priv = fpriv;
 	}
 
+out_suspend:
 	pm_runtime_mark_last_busy(dev->dev);
 	pm_runtime_put_autosuspend(dev->dev);
-	return 0;
+	return r;
 }
 
 /**
-- 
2.5.5

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 08/10] drm/radeon: handle runtime pm in fbcon

[Alex Deucher]

Ported from nouveau.

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/radeon/radeon_fb.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/drivers/gpu/drm/radeon/radeon_fb.c b/drivers/gpu/drm/radeon/radeon_fb.c
index 0e3143a..1ae32c9 100644
--- a/drivers/gpu/drm/radeon/radeon_fb.c
+++ b/drivers/gpu/drm/radeon/radeon_fb.c
@@ -26,6 +26,7 @@
 #include <linux/module.h>
 #include <linux/slab.h>
 #include <linux/fb.h>
+#include <linux/pm_runtime.h>
 
 #include <drm/drmP.h>
 #include <drm/drm_crtc.h>
@@ -47,8 +48,32 @@ struct radeon_fbdev {
 	struct radeon_device *rdev;
 };
 
+static int
+radeonfb_open(struct fb_info *info, int user)
+{
+	struct radeon_fbdev *rfbdev = info->par;
+	struct radeon_device *rdev = rfbdev->rdev;
+	int ret = pm_runtime_get_sync(rdev->ddev->dev);
+	if (ret < 0 && ret != -EACCES)
+		return ret;
+	return 0;
+}
+
+static int
+radeonfb_release(struct fb_info *info, int user)
+{
+	struct radeon_fbdev *rfbdev = info->par;
+	struct radeon_device *rdev = rfbdev->rdev;
+
+	pm_runtime_mark_last_busy(rdev->ddev->dev);
+	pm_runtime_put_autosuspend(rdev->ddev->dev);
+	return 0;
+}
+
 static struct fb_ops radeonfb_ops = {
 	.owner = THIS_MODULE,
+	.fb_open = radeonfb_open,
+	.fb_release = radeonfb_release,
 	.fb_check_var = drm_fb_helper_check_var,
 	.fb_set_par = drm_fb_helper_set_par,
 	.fb_fillrect = drm_fb_helper_cfb_fillrect,
-- 
2.5.5

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 09/10] drm/radeon: skip suspend/resume on DRM_SWITCH_POWER_DYNAMIC_OFF

[Alex Deucher]

Missed this case previously. No need to do anything if the
device is already off.  runtime pm will handle it.

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/radeon/radeon_device.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c
index 6b0d26f..8425b12 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -1594,7 +1594,8 @@ int radeon_suspend_kms(struct drm_device *dev, bool suspend,
 
 	rdev = dev->dev_private;
 
-	if (dev->switch_power_state == DRM_SWITCH_POWER_OFF)
+	if (dev->switch_power_state == DRM_SWITCH_POWER_OFF ||
+	    dev->switch_power_state == DRM_SWITCH_POWER_DYNAMIC_OFF)
 		return 0;
 
 	drm_kms_helper_poll_disable(dev);
@@ -1689,7 +1690,8 @@ int radeon_resume_kms(struct drm_device *dev, bool resume, bool fbcon)
 	struct drm_crtc *crtc;
 	int r;
 
-	if (dev->switch_power_state == DRM_SWITCH_POWER_OFF)
+	if (dev->switch_power_state == DRM_SWITCH_POWER_OFF ||
+	    dev->switch_power_state == DRM_SWITCH_POWER_DYNAMIC_OFF)
 		return 0;
 
 	if (fbcon) {
-- 
2.5.5

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 10/10] drm/radeon: set runtime pm state to active on resume

[Alex Deucher]

The sbios always powers up the dGPU on resume.

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/radeon/radeon_drv.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c
index 081e95e..5fdd48c 100644
--- a/drivers/gpu/drm/radeon/radeon_drv.c
+++ b/drivers/gpu/drm/radeon/radeon_drv.c
@@ -386,6 +386,14 @@ static int radeon_pmops_resume(struct device *dev)
 {
 	struct pci_dev *pdev = to_pci_dev(dev);
 	struct drm_device *drm_dev = pci_get_drvdata(pdev);
+
+	/* GPU comes up enabled by the bios on resume */
+	if (radeon_is_px(drm_dev)) {
+		pm_runtime_disable(dev);
+		pm_runtime_set_active(dev);
+		pm_runtime_enable(dev);
+	}
+
 	return radeon_resume_kms(drm_dev, true, true);
 }
 
-- 
2.5.5

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 01/10] drm/amdgpu: handle runtime pm in drm pre/post close

[Michel Dänzer]

On 01/09/16 07:08 AM, Alex Deucher wrote:
> Close was not handled correctly.
> 
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> index f1b9e0f..45f98b4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> @@ -600,6 +600,9 @@ void amdgpu_driver_postclose_kms(struct drm_device *dev,
>  
>  	kfree(fpriv);
>  	file_priv->driver_priv = NULL;
> +
> +	pm_runtime_mark_last_busy(dev->dev);
> +	pm_runtime_put_autosuspend(dev->dev);
>  }
>  
>  /**
> @@ -614,6 +617,7 @@ void amdgpu_driver_postclose_kms(struct drm_device *dev,
>  void amdgpu_driver_preclose_kms(struct drm_device *dev,
>  				struct drm_file *file_priv)
>  {
> +	pm_runtime_get_sync(dev->dev);
>  }

What if pm_runtime_get_sync returns an error?

Same for patch 6.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 02/10] drm/amdgpu: handle runtime pm correctly in amdgpu_driver_open_kms

[Michel Dänzer]

On 01/09/16 07:08 AM, Alex Deucher wrote:
> Need to fix the error pathes.

Typo? "pathes"


> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 17 +++++++++--------
>  1 file changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> index 45f98b4..c2c7fb1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> @@ -542,12 +542,16 @@ int amdgpu_driver_open_kms(struct drm_device *dev, struct drm_file *file_priv)
>  		return r;
>  
>  	fpriv = kzalloc(sizeof(*fpriv), GFP_KERNEL);
> -	if (unlikely(!fpriv))
> -		return -ENOMEM;
> +	if (unlikely(!fpriv)) {
> +		r = -ENOMEM;
> +		goto out_suspend;
> +	}
>  
>  	r = amdgpu_vm_init(adev, &fpriv->vm);
> -	if (r)
> -		goto error_free;
> +	if (r) {
> +		kfree(fpriv);
> +		goto out_suspend;
> +	}
>  
>  	mutex_init(&fpriv->bo_list_lock);
>  	idr_init(&fpriv->bo_list_handles);
> @@ -556,12 +560,9 @@ int amdgpu_driver_open_kms(struct drm_device *dev, struct drm_file *file_priv)
>  
>  	file_priv->driver_priv = fpriv;
>  
> +out_suspend:
>  	pm_runtime_mark_last_busy(dev->dev);
>  	pm_runtime_put_autosuspend(dev->dev);
> -	return 0;
> -
> -error_free:
> -	kfree(fpriv);

This patch and patch 7 are

Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 03/10] drm/amdgpu: handle runtime pm in fbcon

[Michel Dänzer]

On 01/09/16 07:08 AM, Alex Deucher wrote:
> Ported from nouveau.
> 
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>

[...]

> +static int
> +amdgpufb_open(struct fb_info *info, int user)
> +{
> +	struct amdgpu_fbdev *rfbdev = info->par;
> +	struct amdgpu_device *adev = rfbdev->adev;
> +	int ret = pm_runtime_get_sync(adev->ddev->dev);
> +	if (ret < 0 && ret != -EACCES)
> +		return ret;
> +	return 0;
> +}
> +
> +static int
> +amdgpufb_release(struct fb_info *info, int user)
> +{
> +	struct amdgpu_fbdev *rfbdev = info->par;
> +	struct amdgpu_device *adev = rfbdev->adev;
> +
> +	pm_runtime_mark_last_busy(adev->ddev->dev);
> +	pm_runtime_put_autosuspend(adev->ddev->dev);
> +	return 0;
> +}

If pm_runtime_get_sync returns -EACCES, won't there be an imbalance with
the pm_runtime_put_autosuspend call, which might result in the GPU
powering off even when there's something else which is supposed to keep
it on?

Same for patch 8.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 04/10] drm/amdgpu: skip suspend/resume on DRM_SWITCH_POWER_DYNAMIC_OFF

[Michel Dänzer]

On 01/09/16 07:08 AM, Alex Deucher wrote:
> Missed this case previously. No need to do anything if the
> device is already off.  runtime pm will handle it.
> 
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 2f96334..e6b34dc 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -1842,7 +1842,8 @@ int amdgpu_device_suspend(struct drm_device *dev, bool suspend, bool fbcon,
>  
>  	adev = dev->dev_private;
>  
> -	if (dev->switch_power_state == DRM_SWITCH_POWER_OFF)
> +	if (dev->switch_power_state == DRM_SWITCH_POWER_OFF ||
> +	    dev->switch_power_state == DRM_SWITCH_POWER_DYNAMIC_OFF)
>  		return 0;
>  
>  	drm_kms_helper_poll_disable(dev);
> @@ -1926,7 +1927,8 @@ int amdgpu_device_resume(struct drm_device *dev, bool resume, bool fbcon,
>  	struct drm_crtc *crtc;
>  	int r;
>  
> -	if (dev->switch_power_state == DRM_SWITCH_POWER_OFF)
> +	if (dev->switch_power_state == DRM_SWITCH_POWER_OFF ||
> +	    dev->switch_power_state == DRM_SWITCH_POWER_DYNAMIC_OFF)
>  		return 0;
>  
>  	if (fbcon) {
> 

This patch and patches 5, 9 & 10 are

Acked-by: Michel Dänzer <michel.daenzer@amd.com>


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 01/10] drm/amdgpu: handle runtime pm in drm pre/post close

[Alex Deucher]

On Wed, Aug 31, 2016 at 9:03 PM, Michel Dänzer <michel@daenzer.net> wrote:
> On 01/09/16 07:08 AM, Alex Deucher wrote:
>> Close was not handled correctly.
>>
>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>> ---
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>> index f1b9e0f..45f98b4 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>> @@ -600,6 +600,9 @@ void amdgpu_driver_postclose_kms(struct drm_device *dev,
>>
>>       kfree(fpriv);
>>       file_priv->driver_priv = NULL;
>> +
>> +     pm_runtime_mark_last_busy(dev->dev);
>> +     pm_runtime_put_autosuspend(dev->dev);
>>  }
>>
>>  /**
>> @@ -614,6 +617,7 @@ void amdgpu_driver_postclose_kms(struct drm_device *dev,
>>  void amdgpu_driver_preclose_kms(struct drm_device *dev,
>>                               struct drm_file *file_priv)
>>  {
>> +     pm_runtime_get_sync(dev->dev);
>>  }
>
> What if pm_runtime_get_sync returns an error?
>
> Same for patch 6.
>

I guess we could report it.  drm_release() doesn't check the return
value however.

Alex

>
> --
> Earthling Michel Dänzer               |               http://www.amd.com
> Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 03/10] drm/amdgpu: handle runtime pm in fbcon

[Alex Deucher]

On Wed, Aug 31, 2016 at 9:04 PM, Michel Dänzer <michel@daenzer.net> wrote:
> On 01/09/16 07:08 AM, Alex Deucher wrote:
>> Ported from nouveau.
>>
>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>
> [...]
>
>> +static int
>> +amdgpufb_open(struct fb_info *info, int user)
>> +{
>> +     struct amdgpu_fbdev *rfbdev = info->par;
>> +     struct amdgpu_device *adev = rfbdev->adev;
>> +     int ret = pm_runtime_get_sync(adev->ddev->dev);
>> +     if (ret < 0 && ret != -EACCES)
>> +             return ret;
>> +     return 0;
>> +}
>> +
>> +static int
>> +amdgpufb_release(struct fb_info *info, int user)
>> +{
>> +     struct amdgpu_fbdev *rfbdev = info->par;
>> +     struct amdgpu_device *adev = rfbdev->adev;
>> +
>> +     pm_runtime_mark_last_busy(adev->ddev->dev);
>> +     pm_runtime_put_autosuspend(adev->ddev->dev);
>> +     return 0;
>> +}
>
> If pm_runtime_get_sync returns -EACCES, won't there be an imbalance with
> the pm_runtime_put_autosuspend call, which might result in the GPU
> powering off even when there's something else which is supposed to keep
> it on?
>
> Same for patch 8.

pm_runtime_get_sync only returns -EACCES when runtime pm has been
disabled in which case the GPU is on and pm_runtime_put_autosuspend
won't do anything.

Alex

>
>
> --
> Earthling Michel Dänzer               |               http://www.amd.com
> Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 03/10] drm/amdgpu: handle runtime pm in fbcon

[Michel Dänzer]

On 02/09/16 05:44 AM, Alex Deucher wrote:
> On Wed, Aug 31, 2016 at 9:04 PM, Michel Dänzer <michel@daenzer.net> wrote:
>> On 01/09/16 07:08 AM, Alex Deucher wrote:
>>> Ported from nouveau.
>>>
>>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>>
>> [...]
>>
>>> +static int
>>> +amdgpufb_open(struct fb_info *info, int user)
>>> +{
>>> +     struct amdgpu_fbdev *rfbdev = info->par;
>>> +     struct amdgpu_device *adev = rfbdev->adev;
>>> +     int ret = pm_runtime_get_sync(adev->ddev->dev);
>>> +     if (ret < 0 && ret != -EACCES)
>>> +             return ret;
>>> +     return 0;
>>> +}
>>> +
>>> +static int
>>> +amdgpufb_release(struct fb_info *info, int user)
>>> +{
>>> +     struct amdgpu_fbdev *rfbdev = info->par;
>>> +     struct amdgpu_device *adev = rfbdev->adev;
>>> +
>>> +     pm_runtime_mark_last_busy(adev->ddev->dev);
>>> +     pm_runtime_put_autosuspend(adev->ddev->dev);
>>> +     return 0;
>>> +}
>>
>> If pm_runtime_get_sync returns -EACCES, won't there be an imbalance with
>> the pm_runtime_put_autosuspend call, which might result in the GPU
>> powering off even when there's something else which is supposed to keep
>> it on?
>>
>> Same for patch 8.
> 
> pm_runtime_get_sync only returns -EACCES when runtime pm has been
> disabled in which case the GPU is on and pm_runtime_put_autosuspend
> won't do anything.

It decreases dev->power.usage_count, and my worry was that
pm_runtime_get_sync might not have increased it when it returns an
error. But it turns out that it does increase it unconditionally.


However, I'm afraid there seems to be another related problem: If
amdgpufb_open returns an error, amdgpufb_release won't be called AFAICT.
In which case amdgpufb_open needs to call pm_runtime_put_autosuspend in
the error case, or the GPU can never power off after that.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 01/10] drm/amdgpu: handle runtime pm in drm pre/post close

[Michel Dänzer]

On 02/09/16 05:38 AM, Alex Deucher wrote:
> On Wed, Aug 31, 2016 at 9:03 PM, Michel Dänzer <michel@daenzer.net> wrote:
>> On 01/09/16 07:08 AM, Alex Deucher wrote:
>>> Close was not handled correctly.
>>>
>>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>>> ---
>>>  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 4 ++++
>>>  1 file changed, 4 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>> index f1b9e0f..45f98b4 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>> @@ -600,6 +600,9 @@ void amdgpu_driver_postclose_kms(struct drm_device *dev,
>>>
>>>       kfree(fpriv);
>>>       file_priv->driver_priv = NULL;
>>> +
>>> +     pm_runtime_mark_last_busy(dev->dev);
>>> +     pm_runtime_put_autosuspend(dev->dev);
>>>  }
>>>
>>>  /**
>>> @@ -614,6 +617,7 @@ void amdgpu_driver_postclose_kms(struct drm_device *dev,
>>>  void amdgpu_driver_preclose_kms(struct drm_device *dev,
>>>                               struct drm_file *file_priv)
>>>  {
>>> +     pm_runtime_get_sync(dev->dev);
>>>  }
>>
>> What if pm_runtime_get_sync returns an error?
>>
>> Same for patch 6.
> 
> I guess we could report it.

Based on the discussion of patch 3, it doesn't seem necessary. So this
patch and patch 6 are

Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH 03/10] drm/amdgpu: handle runtime pm in fbcon

[Deucher, Alexander]

> -----Original Message-----
> From: Michel Dänzer [mailto:michel@daenzer.net]
> Sent: Thursday, September 01, 2016 9:47 PM
> To: Alex Deucher
> Cc: amd-gfx list; Deucher, Alexander
> Subject: Re: [PATCH 03/10] drm/amdgpu: handle runtime pm in fbcon
> 
> On 02/09/16 05:44 AM, Alex Deucher wrote:
> > On Wed, Aug 31, 2016 at 9:04 PM, Michel Dänzer <michel@daenzer.net>
> wrote:
> >> On 01/09/16 07:08 AM, Alex Deucher wrote:
> >>> Ported from nouveau.
> >>>
> >>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> >>
> >> [...]
> >>
> >>> +static int
> >>> +amdgpufb_open(struct fb_info *info, int user)
> >>> +{
> >>> +     struct amdgpu_fbdev *rfbdev = info->par;
> >>> +     struct amdgpu_device *adev = rfbdev->adev;
> >>> +     int ret = pm_runtime_get_sync(adev->ddev->dev);
> >>> +     if (ret < 0 && ret != -EACCES)
> >>> +             return ret;
> >>> +     return 0;
> >>> +}
> >>> +
> >>> +static int
> >>> +amdgpufb_release(struct fb_info *info, int user)
> >>> +{
> >>> +     struct amdgpu_fbdev *rfbdev = info->par;
> >>> +     struct amdgpu_device *adev = rfbdev->adev;
> >>> +
> >>> +     pm_runtime_mark_last_busy(adev->ddev->dev);
> >>> +     pm_runtime_put_autosuspend(adev->ddev->dev);
> >>> +     return 0;
> >>> +}
> >>
> >> If pm_runtime_get_sync returns -EACCES, won't there be an imbalance
> with
> >> the pm_runtime_put_autosuspend call, which might result in the GPU
> >> powering off even when there's something else which is supposed to
> keep
> >> it on?
> >>
> >> Same for patch 8.
> >
> > pm_runtime_get_sync only returns -EACCES when runtime pm has been
> > disabled in which case the GPU is on and pm_runtime_put_autosuspend
> > won't do anything.
> 
> It decreases dev->power.usage_count, and my worry was that
> pm_runtime_get_sync might not have increased it when it returns an
> error. But it turns out that it does increase it unconditionally.
> 
> 
> However, I'm afraid there seems to be another related problem: If
> amdgpufb_open returns an error, amdgpufb_release won't be called
> AFAICT.
> In which case amdgpufb_open needs to call pm_runtime_put_autosuspend
> in
> the error case, or the GPU can never power off after that.

Good point.  I'll fix that up.

Alex

> 
> 
> --
> Earthling Michel Dänzer               |               http://www.amd.com
> Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx