[RFC 0/2] PM and DRM: Add runtime PM to Exynos DSI

[RFC 0/2] PM and DRM: Add runtime PM to Exynos DSI

[Krzysztof Kozlowski]

Hi,

The goal of the patch 2 is to add runtime PM to the Exynos DSI driver.
This allows LCD power domain to be turned off.

However after adding this patch an interesing issue came in. The DSI
driver could not runtime resume during sustem resume because power
domain was not allowed to power on.

The issue is solved by patch 1.

Tested on Trats2 board.

Comments are welcomed.

Best regards,
Krzysztof

Krzysztof Kozlowski (2):
  PM / Domains: Power on domain early during system resume
  drm/exynos/dsi: Add runtime PM so LCD power domain could be turned off

 drivers/base/power/domain.c             | 3 +--
 drivers/gpu/drm/exynos/exynos_drm_dsi.c | 9 +++++++++
 2 files changed, 10 insertions(+), 2 deletions(-)

-- 
1.9.1

[RFC 0/2] PM and DRM: Add runtime PM to Exynos DSI

[Krzysztof Kozlowski]

Hi,

The goal of the patch 2 is to add runtime PM to the Exynos DSI driver.
This allows LCD power domain to be turned off.

However after adding this patch an interesing issue came in. The DSI
driver could not runtime resume during sustem resume because power
domain was not allowed to power on.

The issue is solved by patch 1.

Tested on Trats2 board.

Comments are welcomed.

Best regards,
Krzysztof

Krzysztof Kozlowski (2):
  PM / Domains: Power on domain early during system resume
  drm/exynos/dsi: Add runtime PM so LCD power domain could be turned off

 drivers/base/power/domain.c             | 3 +--
 drivers/gpu/drm/exynos/exynos_drm_dsi.c | 9 +++++++++
 2 files changed, 10 insertions(+), 2 deletions(-)

-- 
1.9.1

[RFC 1/2] PM / Domains: Power on domain early during system resume

[Krzysztof Kozlowski]

When resuming the system the power domain has to be powered on early so
any runtime PM aware devices could resume.

This fixes following scenario reproduced on Exynos DRM:
1. Power domain is off before suspending the system.
2. System is suspended to RAM.
3. Resuming starts. The Exynos DRM driver resume callback is called.
4. The Exynos DRM driver calls drm_helper_resume_force_mode which turns
   the screen on by calling exynos_dsi_dpms with DRM_MODE_DPMS_ON.
5. The Exynos DSI driver calls pm_runtime_get. The driver runtime
   resumes and this should turn LCD power domain on.
6. Unfortunately the domain cannot be turned on because system resume is
   in progress and genpd->prepared_count is positive.

Steps to reproduce:
1. Add runtime PM to Exynos DSI driver.
2. Build Exynos DRM/FB without FRAMEBUFFER_CONSOLE.
3. Enable the connector and screen (e.g. with modeset-vsync application).
4. echo 3 > /sys/devices/platform/exynos-drm/graphics/fb0/blank
5. echo mem > /sys/power/state
6. Resume.
[   77.712469] PM: early resume of devices complete after 3.854 msecs
[   77.712739] exynos-dsi 11c80000.dsi: pm_genpd_resume()
[   77.712758] exynos4-fimc 11800000.fimc: pm_genpd_resume()
[   77.712774] exynos4-fimc 11810000.fimc: pm_genpd_resume()
[   77.712787] exynos-drm-fimc 11820000.fimc: pm_genpd_resume()
[   77.712802] exynos-drm-fimc 11830000.fimc: pm_genpd_resume()
[   77.712815] s5p-mipi-csis 11880000.csis: pm_genpd_resume()
[   77.712829] s5p-mipi-csis 11890000.csis: pm_genpd_resume()
[   77.712843] exynos-fimc-lite 12390000.fimc-lite: pm_genpd_resume()
[   77.712856] exynos-fimc-lite 123a0000.fimc-lite: pm_genpd_resume()
[   77.713788] exynos4-fb 11c00000.fimd: pm_genpd_resume()
[   77.713912] wake disabled for irq 184
[   77.713923] wake disabled for irq 185
[   77.714082] wake disabled for irq 173
[   77.715676] wake disabled for irq 176
[   77.718540] exynos4-fb 11c00000.fimd: pm_genpd_runtime_resume()
[   77.718567] exynos4-fb 11c00000.fimd: state restore latency exceeded, new value 1708 ns
[   77.718636] exynos-dsi 11c80000.dsi: pm_genpd_runtime_resume()
[   77.892366] exynos-dsi 11c80000.dsi: PLL failed to stabilize
[   77.892377] exynos-dsi 11c80000.dsi: failed to configure DSI PLL
[   78.192168] exynos-dsi 11c80000.dsi: timeout waiting for reset
[   78.211578] exynos-dsi 11c80000.dsi: waiting for bus lanes timed out
[   78.307173] exynos-dsi 11c80000.dsi: xfer timed out: d1 00 (null)
[   78.307190] panel_s6e8aa0 11c80000.dsi.0: error -110 reading dcs seq(0xd1)
[   78.307199] panel_s6e8aa0 11c80000.dsi.0: read id failed

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
---
 drivers/base/power/domain.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 40bc2f4072cc..4fdfe404a04c 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -179,8 +179,7 @@ static int __pm_genpd_poweron(struct generic_pm_domain *genpd)
 	}
 	finish_wait(&genpd->status_wait_queue, &wait);
 
-	if (genpd->status == GPD_STATE_ACTIVE
-	    || (genpd->prepared_count > 0 && genpd->suspend_power_off))
+	if (genpd->status == GPD_STATE_ACTIVE)
 		return 0;
 
 	if (genpd->status != GPD_STATE_POWER_OFF) {
-- 
1.9.1

[RFC 1/2] PM / Domains: Power on domain early during system resume

[Krzysztof Kozlowski]

When resuming the system the power domain has to be powered on early so
any runtime PM aware devices could resume.

This fixes following scenario reproduced on Exynos DRM:
1. Power domain is off before suspending the system.
2. System is suspended to RAM.
3. Resuming starts. The Exynos DRM driver resume callback is called.
4. The Exynos DRM driver calls drm_helper_resume_force_mode which turns
   the screen on by calling exynos_dsi_dpms with DRM_MODE_DPMS_ON.
5. The Exynos DSI driver calls pm_runtime_get. The driver runtime
   resumes and this should turn LCD power domain on.
6. Unfortunately the domain cannot be turned on because system resume is
   in progress and genpd->prepared_count is positive.

Steps to reproduce:
1. Add runtime PM to Exynos DSI driver.
2. Build Exynos DRM/FB without FRAMEBUFFER_CONSOLE.
3. Enable the connector and screen (e.g. with modeset-vsync application).
4. echo 3 > /sys/devices/platform/exynos-drm/graphics/fb0/blank
5. echo mem > /sys/power/state
6. Resume.
[   77.712469] PM: early resume of devices complete after 3.854 msecs
[   77.712739] exynos-dsi 11c80000.dsi: pm_genpd_resume()
[   77.712758] exynos4-fimc 11800000.fimc: pm_genpd_resume()
[   77.712774] exynos4-fimc 11810000.fimc: pm_genpd_resume()
[   77.712787] exynos-drm-fimc 11820000.fimc: pm_genpd_resume()
[   77.712802] exynos-drm-fimc 11830000.fimc: pm_genpd_resume()
[   77.712815] s5p-mipi-csis 11880000.csis: pm_genpd_resume()
[   77.712829] s5p-mipi-csis 11890000.csis: pm_genpd_resume()
[   77.712843] exynos-fimc-lite 12390000.fimc-lite: pm_genpd_resume()
[   77.712856] exynos-fimc-lite 123a0000.fimc-lite: pm_genpd_resume()
[   77.713788] exynos4-fb 11c00000.fimd: pm_genpd_resume()
[   77.713912] wake disabled for irq 184
[   77.713923] wake disabled for irq 185
[   77.714082] wake disabled for irq 173
[   77.715676] wake disabled for irq 176
[   77.718540] exynos4-fb 11c00000.fimd: pm_genpd_runtime_resume()
[   77.718567] exynos4-fb 11c00000.fimd: state restore latency exceeded, new value 1708 ns
[   77.718636] exynos-dsi 11c80000.dsi: pm_genpd_runtime_resume()
[   77.892366] exynos-dsi 11c80000.dsi: PLL failed to stabilize
[   77.892377] exynos-dsi 11c80000.dsi: failed to configure DSI PLL
[   78.192168] exynos-dsi 11c80000.dsi: timeout waiting for reset
[   78.211578] exynos-dsi 11c80000.dsi: waiting for bus lanes timed out
[   78.307173] exynos-dsi 11c80000.dsi: xfer timed out: d1 00 (null)
[   78.307190] panel_s6e8aa0 11c80000.dsi.0: error -110 reading dcs seq(0xd1)
[   78.307199] panel_s6e8aa0 11c80000.dsi.0: read id failed

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
---
 drivers/base/power/domain.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 40bc2f4072cc..4fdfe404a04c 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -179,8 +179,7 @@ static int __pm_genpd_poweron(struct generic_pm_domain *genpd)
 	}
 	finish_wait(&genpd->status_wait_queue, &wait);
 
-	if (genpd->status == GPD_STATE_ACTIVE
-	    || (genpd->prepared_count > 0 && genpd->suspend_power_off))
+	if (genpd->status == GPD_STATE_ACTIVE)
 		return 0;
 
 	if (genpd->status != GPD_STATE_POWER_OFF) {
-- 
1.9.1

[RFC 1/2] PM / Domains: Power on domain early during system resume

[Krzysztof Kozlowski]

When resuming the system the power domain has to be powered on early so
any runtime PM aware devices could resume.

This fixes following scenario reproduced on Exynos DRM:
1. Power domain is off before suspending the system.
2. System is suspended to RAM.
3. Resuming starts. The Exynos DRM driver resume callback is called.
4. The Exynos DRM driver calls drm_helper_resume_force_mode which turns
   the screen on by calling exynos_dsi_dpms with DRM_MODE_DPMS_ON.
5. The Exynos DSI driver calls pm_runtime_get. The driver runtime
   resumes and this should turn LCD power domain on.
6. Unfortunately the domain cannot be turned on because system resume is
   in progress and genpd->prepared_count is positive.

Steps to reproduce:
1. Add runtime PM to Exynos DSI driver.
2. Build Exynos DRM/FB without FRAMEBUFFER_CONSOLE.
3. Enable the connector and screen (e.g. with modeset-vsync application).
4. echo 3 > /sys/devices/platform/exynos-drm/graphics/fb0/blank
5. echo mem > /sys/power/state
6. Resume.
[   77.712469] PM: early resume of devices complete after 3.854 msecs
[   77.712739] exynos-dsi 11c80000.dsi: pm_genpd_resume()
[   77.712758] exynos4-fimc 11800000.fimc: pm_genpd_resume()
[   77.712774] exynos4-fimc 11810000.fimc: pm_genpd_resume()
[   77.712787] exynos-drm-fimc 11820000.fimc: pm_genpd_resume()
[   77.712802] exynos-drm-fimc 11830000.fimc: pm_genpd_resume()
[   77.712815] s5p-mipi-csis 11880000.csis: pm_genpd_resume()
[   77.712829] s5p-mipi-csis 11890000.csis: pm_genpd_resume()
[   77.712843] exynos-fimc-lite 12390000.fimc-lite: pm_genpd_resume()
[   77.712856] exynos-fimc-lite 123a0000.fimc-lite: pm_genpd_resume()
[   77.713788] exynos4-fb 11c00000.fimd: pm_genpd_resume()
[   77.713912] wake disabled for irq 184
[   77.713923] wake disabled for irq 185
[   77.714082] wake disabled for irq 173
[   77.715676] wake disabled for irq 176
[   77.718540] exynos4-fb 11c00000.fimd: pm_genpd_runtime_resume()
[   77.718567] exynos4-fb 11c00000.fimd: state restore latency exceeded, new value 1708 ns
[   77.718636] exynos-dsi 11c80000.dsi: pm_genpd_runtime_resume()
[   77.892366] exynos-dsi 11c80000.dsi: PLL failed to stabilize
[   77.892377] exynos-dsi 11c80000.dsi: failed to configure DSI PLL
[   78.192168] exynos-dsi 11c80000.dsi: timeout waiting for reset
[   78.211578] exynos-dsi 11c80000.dsi: waiting for bus lanes timed out
[   78.307173] exynos-dsi 11c80000.dsi: xfer timed out: d1 00 (null)
[   78.307190] panel_s6e8aa0 11c80000.dsi.0: error -110 reading dcs seq(0xd1)
[   78.307199] panel_s6e8aa0 11c80000.dsi.0: read id failed

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
---
 drivers/base/power/domain.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 40bc2f4072cc..4fdfe404a04c 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -179,8 +179,7 @@ static int __pm_genpd_poweron(struct generic_pm_domain *genpd)
 	}
 	finish_wait(&genpd->status_wait_queue, &wait);
 
-	if (genpd->status == GPD_STATE_ACTIVE
-	    || (genpd->prepared_count > 0 && genpd->suspend_power_off))
+	if (genpd->status == GPD_STATE_ACTIVE)
 		return 0;
 
 	if (genpd->status != GPD_STATE_POWER_OFF) {
-- 
1.9.1

[RFC 2/2] drm/exynos/dsi: Add runtime PM so LCD power domain could be turned off

[Krzysztof Kozlowski]

Add runtime Power Management to the Exynos DSI driver so the LCD power
domain could be turned off.

This slightly reduces the energy consumption when screen is completely
turned off. On Trats2 board when the system was idle the energy
consumption dropped by 1% (from 92.2 mA to 91.1 mA).

Before the patch:
$ cat cat /sys/kernel/debug/pm_genpd/pm_genpd_summary
lcd0-power-domain               on
    /devices/11c00000.fimd                              suspended
    /devices/11c80000.dsi                               unsupported
After applying the patch:
lcd0-power-domain               off
    /devices/11c00000.fimd                              suspended
    /devices/11c80000.dsi                               suspended

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_dsi.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
index 24741d8758e8..19ed36d2d557 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
@@ -21,6 +21,7 @@
 #include <linux/of_device.h>
 #include <linux/of_gpio.h>
 #include <linux/phy/phy.h>
+#include <linux/pm_runtime.h>
 #include <linux/regulator/consumer.h>
 #include <linux/component.h>
 
@@ -1356,6 +1357,8 @@ static int exynos_dsi_enable(struct exynos_dsi *dsi)
 	if (dsi->state & DSIM_STATE_ENABLED)
 		return 0;
 
+	pm_runtime_get_sync(dsi->dev);
+
 	ret = exynos_dsi_poweron(dsi);
 	if (ret < 0)
 		return ret;
@@ -1392,6 +1395,8 @@ static void exynos_dsi_disable(struct exynos_dsi *dsi)
 	drm_panel_unprepare(dsi->panel);
 	exynos_dsi_poweroff(dsi);
 
+	pm_runtime_put_sync(dsi->dev);
+
 	dsi->state &= ~DSIM_STATE_ENABLED;
 }
 
@@ -1772,6 +1777,8 @@ static int exynos_dsi_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_del_component;
 
+	pm_runtime_enable(&pdev->dev);
+
 	return ret;
 
 err_del_component:
@@ -1781,6 +1788,8 @@ err_del_component:
 
 static int exynos_dsi_remove(struct platform_device *pdev)
 {
+	pm_runtime_disable(&pdev->dev);
+
 	component_del(&pdev->dev, &exynos_dsi_component_ops);
 	exynos_drm_component_del(&pdev->dev, EXYNOS_DEVICE_TYPE_CONNECTOR);
 
-- 
1.9.1

[RFC 2/2] drm/exynos/dsi: Add runtime PM so LCD power domain could be turned off

[Krzysztof Kozlowski]

Add runtime Power Management to the Exynos DSI driver so the LCD power
domain could be turned off.

This slightly reduces the energy consumption when screen is completely
turned off. On Trats2 board when the system was idle the energy
consumption dropped by 1% (from 92.2 mA to 91.1 mA).

Before the patch:
$ cat cat /sys/kernel/debug/pm_genpd/pm_genpd_summary
lcd0-power-domain               on
    /devices/11c00000.fimd                              suspended
    /devices/11c80000.dsi                               unsupported
After applying the patch:
lcd0-power-domain               off
    /devices/11c00000.fimd                              suspended
    /devices/11c80000.dsi                               suspended

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_dsi.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
index 24741d8758e8..19ed36d2d557 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
@@ -21,6 +21,7 @@
 #include <linux/of_device.h>
 #include <linux/of_gpio.h>
 #include <linux/phy/phy.h>
+#include <linux/pm_runtime.h>
 #include <linux/regulator/consumer.h>
 #include <linux/component.h>
 
@@ -1356,6 +1357,8 @@ static int exynos_dsi_enable(struct exynos_dsi *dsi)
 	if (dsi->state & DSIM_STATE_ENABLED)
 		return 0;
 
+	pm_runtime_get_sync(dsi->dev);
+
 	ret = exynos_dsi_poweron(dsi);
 	if (ret < 0)
 		return ret;
@@ -1392,6 +1395,8 @@ static void exynos_dsi_disable(struct exynos_dsi *dsi)
 	drm_panel_unprepare(dsi->panel);
 	exynos_dsi_poweroff(dsi);
 
+	pm_runtime_put_sync(dsi->dev);
+
 	dsi->state &= ~DSIM_STATE_ENABLED;
 }
 
@@ -1772,6 +1777,8 @@ static int exynos_dsi_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_del_component;
 
+	pm_runtime_enable(&pdev->dev);
+
 	return ret;
 
 err_del_component:
@@ -1781,6 +1788,8 @@ err_del_component:
 
 static int exynos_dsi_remove(struct platform_device *pdev)
 {
+	pm_runtime_disable(&pdev->dev);
+
 	component_del(&pdev->dev, &exynos_dsi_component_ops);
 	exynos_drm_component_del(&pdev->dev, EXYNOS_DEVICE_TYPE_CONNECTOR);
 
-- 
1.9.1

[RFC 2/2] drm/exynos/dsi: Add runtime PM so LCD power domain could be turned off

[Krzysztof Kozlowski]

Add runtime Power Management to the Exynos DSI driver so the LCD power
domain could be turned off.

This slightly reduces the energy consumption when screen is completely
turned off. On Trats2 board when the system was idle the energy
consumption dropped by 1% (from 92.2 mA to 91.1 mA).

Before the patch:
$ cat cat /sys/kernel/debug/pm_genpd/pm_genpd_summary
lcd0-power-domain               on
    /devices/11c00000.fimd                              suspended
    /devices/11c80000.dsi                               unsupported
After applying the patch:
lcd0-power-domain               off
    /devices/11c00000.fimd                              suspended
    /devices/11c80000.dsi                               suspended

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_drm_dsi.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
index 24741d8758e8..19ed36d2d557 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
@@ -21,6 +21,7 @@
 #include <linux/of_device.h>
 #include <linux/of_gpio.h>
 #include <linux/phy/phy.h>
+#include <linux/pm_runtime.h>
 #include <linux/regulator/consumer.h>
 #include <linux/component.h>
 
@@ -1356,6 +1357,8 @@ static int exynos_dsi_enable(struct exynos_dsi *dsi)
 	if (dsi->state & DSIM_STATE_ENABLED)
 		return 0;
 
+	pm_runtime_get_sync(dsi->dev);
+
 	ret = exynos_dsi_poweron(dsi);
 	if (ret < 0)
 		return ret;
@@ -1392,6 +1395,8 @@ static void exynos_dsi_disable(struct exynos_dsi *dsi)
 	drm_panel_unprepare(dsi->panel);
 	exynos_dsi_poweroff(dsi);
 
+	pm_runtime_put_sync(dsi->dev);
+
 	dsi->state &= ~DSIM_STATE_ENABLED;
 }
 
@@ -1772,6 +1777,8 @@ static int exynos_dsi_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_del_component;
 
+	pm_runtime_enable(&pdev->dev);
+
 	return ret;
 
 err_del_component:
@@ -1781,6 +1788,8 @@ err_del_component:
 
 static int exynos_dsi_remove(struct platform_device *pdev)
 {
+	pm_runtime_disable(&pdev->dev);
+
 	component_del(&pdev->dev, &exynos_dsi_component_ops);
 	exynos_drm_component_del(&pdev->dev, EXYNOS_DEVICE_TYPE_CONNECTOR);
 
-- 
1.9.1

Re: [RFC 1/2] PM / Domains: Power on domain early during system resume

[Grygorii Strashko]

Hi Krzysztof,

On 10/23/2014 04:48 PM, Krzysztof Kozlowski wrote:
> When resuming the system the power domain has to be powered on early so
> any runtime PM aware devices could resume.
> 
> This fixes following scenario reproduced on Exynos DRM:
> 1. Power domain is off before suspending the system.
> 2. System is suspended to RAM.
> 3. Resuming starts. The Exynos DRM driver resume callback is called.
> 4. The Exynos DRM driver calls drm_helper_resume_force_mode which turns
>     the screen on by calling exynos_dsi_dpms with DRM_MODE_DPMS_ON.
> 5. The Exynos DSI driver calls pm_runtime_get. The driver runtime
>     resumes and this should turn LCD power domain on.
> 6. Unfortunately the domain cannot be turned on because system resume is
>     in progress and genpd->prepared_count is positive.

Just interesting, what value will be returned by pm_runtime_enabled()
from any of your .resume() callback (for any device which belongs to
some Generic PM domain)?

I'm asking, because as I can see Runtime PM can be disabled from pm_genpd_prepare().

Thank you.

Oh. I've just found that you might get this issue if you will try to do
suspend when PM domain is ON ;)

Any way, In my opinion, It might be better to fix pm_genpd_prepare() so
it will not increment prepared_count when initial state of the GPD is
GPD_STATE_POWER_OFF. Seems it's needed only in opposite case -
when state of GPD has to be restored from pm_genpd_resume_noirq().

> 
> Steps to reproduce:
> 1. Add runtime PM to Exynos DSI driver.
> 2. Build Exynos DRM/FB without FRAMEBUFFER_CONSOLE.
> 3. Enable the connector and screen (e.g. with modeset-vsync application).
> 4. echo 3 > /sys/devices/platform/exynos-drm/graphics/fb0/blank
> 5. echo mem > /sys/power/state
> 6. Resume.
> [   77.712469] PM: early resume of devices complete after 3.854 msecs
> [   77.712739] exynos-dsi 11c80000.dsi: pm_genpd_resume()
> [   77.712758] exynos4-fimc 11800000.fimc: pm_genpd_resume()
> [   77.712774] exynos4-fimc 11810000.fimc: pm_genpd_resume()
> [   77.712787] exynos-drm-fimc 11820000.fimc: pm_genpd_resume()
> [   77.712802] exynos-drm-fimc 11830000.fimc: pm_genpd_resume()
> [   77.712815] s5p-mipi-csis 11880000.csis: pm_genpd_resume()
> [   77.712829] s5p-mipi-csis 11890000.csis: pm_genpd_resume()
> [   77.712843] exynos-fimc-lite 12390000.fimc-lite: pm_genpd_resume()
> [   77.712856] exynos-fimc-lite 123a0000.fimc-lite: pm_genpd_resume()
> [   77.713788] exynos4-fb 11c00000.fimd: pm_genpd_resume()
> [   77.713912] wake disabled for irq 184
> [   77.713923] wake disabled for irq 185
> [   77.714082] wake disabled for irq 173
> [   77.715676] wake disabled for irq 176
> [   77.718540] exynos4-fb 11c00000.fimd: pm_genpd_runtime_resume()
> [   77.718567] exynos4-fb 11c00000.fimd: state restore latency exceeded, new value 1708 ns
> [   77.718636] exynos-dsi 11c80000.dsi: pm_genpd_runtime_resume()
> [   77.892366] exynos-dsi 11c80000.dsi: PLL failed to stabilize
> [   77.892377] exynos-dsi 11c80000.dsi: failed to configure DSI PLL
> [   78.192168] exynos-dsi 11c80000.dsi: timeout waiting for reset
> [   78.211578] exynos-dsi 11c80000.dsi: waiting for bus lanes timed out
> [   78.307173] exynos-dsi 11c80000.dsi: xfer timed out: d1 00 (null)
> [   78.307190] panel_s6e8aa0 11c80000.dsi.0: error -110 reading dcs seq(0xd1)
> [   78.307199] panel_s6e8aa0 11c80000.dsi.0: read id failed
> 
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> ---
>   drivers/base/power/domain.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 40bc2f4072cc..4fdfe404a04c 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -179,8 +179,7 @@ static int __pm_genpd_poweron(struct generic_pm_domain *genpd)
>   	}
>   	finish_wait(&genpd->status_wait_queue, &wait);
>   
> -	if (genpd->status == GPD_STATE_ACTIVE
> -	    || (genpd->prepared_count > 0 && genpd->suspend_power_off))
> +	if (genpd->status == GPD_STATE_ACTIVE)
>   		return 0;
>   
>   	if (genpd->status != GPD_STATE_POWER_OFF) {
> 

regards,
-grygorii

Re: [RFC 1/2] PM / Domains: Power on domain early during system resume

[Grygorii Strashko]

Hi Krzysztof,

On 10/23/2014 04:48 PM, Krzysztof Kozlowski wrote:
> When resuming the system the power domain has to be powered on early so
> any runtime PM aware devices could resume.
> 
> This fixes following scenario reproduced on Exynos DRM:
> 1. Power domain is off before suspending the system.
> 2. System is suspended to RAM.
> 3. Resuming starts. The Exynos DRM driver resume callback is called.
> 4. The Exynos DRM driver calls drm_helper_resume_force_mode which turns
>     the screen on by calling exynos_dsi_dpms with DRM_MODE_DPMS_ON.
> 5. The Exynos DSI driver calls pm_runtime_get. The driver runtime
>     resumes and this should turn LCD power domain on.
> 6. Unfortunately the domain cannot be turned on because system resume is
>     in progress and genpd->prepared_count is positive.

Just interesting, what value will be returned by pm_runtime_enabled()
from any of your .resume() callback (for any device which belongs to
some Generic PM domain)?

I'm asking, because as I can see Runtime PM can be disabled from pm_genpd_prepare().

Thank you.

Oh. I've just found that you might get this issue if you will try to do
suspend when PM domain is ON ;)

Any way, In my opinion, It might be better to fix pm_genpd_prepare() so
it will not increment prepared_count when initial state of the GPD is
GPD_STATE_POWER_OFF. Seems it's needed only in opposite case -
when state of GPD has to be restored from pm_genpd_resume_noirq().

> 
> Steps to reproduce:
> 1. Add runtime PM to Exynos DSI driver.
> 2. Build Exynos DRM/FB without FRAMEBUFFER_CONSOLE.
> 3. Enable the connector and screen (e.g. with modeset-vsync application).
> 4. echo 3 > /sys/devices/platform/exynos-drm/graphics/fb0/blank
> 5. echo mem > /sys/power/state
> 6. Resume.
> [   77.712469] PM: early resume of devices complete after 3.854 msecs
> [   77.712739] exynos-dsi 11c80000.dsi: pm_genpd_resume()
> [   77.712758] exynos4-fimc 11800000.fimc: pm_genpd_resume()
> [   77.712774] exynos4-fimc 11810000.fimc: pm_genpd_resume()
> [   77.712787] exynos-drm-fimc 11820000.fimc: pm_genpd_resume()
> [   77.712802] exynos-drm-fimc 11830000.fimc: pm_genpd_resume()
> [   77.712815] s5p-mipi-csis 11880000.csis: pm_genpd_resume()
> [   77.712829] s5p-mipi-csis 11890000.csis: pm_genpd_resume()
> [   77.712843] exynos-fimc-lite 12390000.fimc-lite: pm_genpd_resume()
> [   77.712856] exynos-fimc-lite 123a0000.fimc-lite: pm_genpd_resume()
> [   77.713788] exynos4-fb 11c00000.fimd: pm_genpd_resume()
> [   77.713912] wake disabled for irq 184
> [   77.713923] wake disabled for irq 185
> [   77.714082] wake disabled for irq 173
> [   77.715676] wake disabled for irq 176
> [   77.718540] exynos4-fb 11c00000.fimd: pm_genpd_runtime_resume()
> [   77.718567] exynos4-fb 11c00000.fimd: state restore latency exceeded, new value 1708 ns
> [   77.718636] exynos-dsi 11c80000.dsi: pm_genpd_runtime_resume()
> [   77.892366] exynos-dsi 11c80000.dsi: PLL failed to stabilize
> [   77.892377] exynos-dsi 11c80000.dsi: failed to configure DSI PLL
> [   78.192168] exynos-dsi 11c80000.dsi: timeout waiting for reset
> [   78.211578] exynos-dsi 11c80000.dsi: waiting for bus lanes timed out
> [   78.307173] exynos-dsi 11c80000.dsi: xfer timed out: d1 00 (null)
> [   78.307190] panel_s6e8aa0 11c80000.dsi.0: error -110 reading dcs seq(0xd1)
> [   78.307199] panel_s6e8aa0 11c80000.dsi.0: read id failed
> 
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> ---
>   drivers/base/power/domain.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 40bc2f4072cc..4fdfe404a04c 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -179,8 +179,7 @@ static int __pm_genpd_poweron(struct generic_pm_domain *genpd)
>   	}
>   	finish_wait(&genpd->status_wait_queue, &wait);
>   
> -	if (genpd->status == GPD_STATE_ACTIVE
> -	    || (genpd->prepared_count > 0 && genpd->suspend_power_off))
> +	if (genpd->status == GPD_STATE_ACTIVE)
>   		return 0;
>   
>   	if (genpd->status != GPD_STATE_POWER_OFF) {
> 

regards,
-grygorii

[RFC 1/2] PM / Domains: Power on domain early during system resume

[Grygorii Strashko]

Hi Krzysztof,

On 10/23/2014 04:48 PM, Krzysztof Kozlowski wrote:
> When resuming the system the power domain has to be powered on early so
> any runtime PM aware devices could resume.
> 
> This fixes following scenario reproduced on Exynos DRM:
> 1. Power domain is off before suspending the system.
> 2. System is suspended to RAM.
> 3. Resuming starts. The Exynos DRM driver resume callback is called.
> 4. The Exynos DRM driver calls drm_helper_resume_force_mode which turns
>     the screen on by calling exynos_dsi_dpms with DRM_MODE_DPMS_ON.
> 5. The Exynos DSI driver calls pm_runtime_get. The driver runtime
>     resumes and this should turn LCD power domain on.
> 6. Unfortunately the domain cannot be turned on because system resume is
>     in progress and genpd->prepared_count is positive.

Just interesting, what value will be returned by pm_runtime_enabled()
from any of your .resume() callback (for any device which belongs to
some Generic PM domain)?

I'm asking, because as I can see Runtime PM can be disabled from pm_genpd_prepare().

Thank you.

Oh. I've just found that you might get this issue if you will try to do
suspend when PM domain is ON ;)

Any way, In my opinion, It might be better to fix pm_genpd_prepare() so
it will not increment prepared_count when initial state of the GPD is
GPD_STATE_POWER_OFF. Seems it's needed only in opposite case -
when state of GPD has to be restored from pm_genpd_resume_noirq().

> 
> Steps to reproduce:
> 1. Add runtime PM to Exynos DSI driver.
> 2. Build Exynos DRM/FB without FRAMEBUFFER_CONSOLE.
> 3. Enable the connector and screen (e.g. with modeset-vsync application).
> 4. echo 3 > /sys/devices/platform/exynos-drm/graphics/fb0/blank
> 5. echo mem > /sys/power/state
> 6. Resume.
> [   77.712469] PM: early resume of devices complete after 3.854 msecs
> [   77.712739] exynos-dsi 11c80000.dsi: pm_genpd_resume()
> [   77.712758] exynos4-fimc 11800000.fimc: pm_genpd_resume()
> [   77.712774] exynos4-fimc 11810000.fimc: pm_genpd_resume()
> [   77.712787] exynos-drm-fimc 11820000.fimc: pm_genpd_resume()
> [   77.712802] exynos-drm-fimc 11830000.fimc: pm_genpd_resume()
> [   77.712815] s5p-mipi-csis 11880000.csis: pm_genpd_resume()
> [   77.712829] s5p-mipi-csis 11890000.csis: pm_genpd_resume()
> [   77.712843] exynos-fimc-lite 12390000.fimc-lite: pm_genpd_resume()
> [   77.712856] exynos-fimc-lite 123a0000.fimc-lite: pm_genpd_resume()
> [   77.713788] exynos4-fb 11c00000.fimd: pm_genpd_resume()
> [   77.713912] wake disabled for irq 184
> [   77.713923] wake disabled for irq 185
> [   77.714082] wake disabled for irq 173
> [   77.715676] wake disabled for irq 176
> [   77.718540] exynos4-fb 11c00000.fimd: pm_genpd_runtime_resume()
> [   77.718567] exynos4-fb 11c00000.fimd: state restore latency exceeded, new value 1708 ns
> [   77.718636] exynos-dsi 11c80000.dsi: pm_genpd_runtime_resume()
> [   77.892366] exynos-dsi 11c80000.dsi: PLL failed to stabilize
> [   77.892377] exynos-dsi 11c80000.dsi: failed to configure DSI PLL
> [   78.192168] exynos-dsi 11c80000.dsi: timeout waiting for reset
> [   78.211578] exynos-dsi 11c80000.dsi: waiting for bus lanes timed out
> [   78.307173] exynos-dsi 11c80000.dsi: xfer timed out: d1 00 (null)
> [   78.307190] panel_s6e8aa0 11c80000.dsi.0: error -110 reading dcs seq(0xd1)
> [   78.307199] panel_s6e8aa0 11c80000.dsi.0: read id failed
> 
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> ---
>   drivers/base/power/domain.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 40bc2f4072cc..4fdfe404a04c 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -179,8 +179,7 @@ static int __pm_genpd_poweron(struct generic_pm_domain *genpd)
>   	}
>   	finish_wait(&genpd->status_wait_queue, &wait);
>   
> -	if (genpd->status == GPD_STATE_ACTIVE
> -	    || (genpd->prepared_count > 0 && genpd->suspend_power_off))
> +	if (genpd->status == GPD_STATE_ACTIVE)
>   		return 0;
>   
>   	if (genpd->status != GPD_STATE_POWER_OFF) {
> 

regards,
-grygorii

Re: [RFC 1/2] PM / Domains: Power on domain early during system resume

[Krzysztof Kozlowski]

On czw, 2014-10-23 at 19:20 +0300, Grygorii Strashko wrote:
> Hi Krzysztof,
> 
> On 10/23/2014 04:48 PM, Krzysztof Kozlowski wrote:
> > When resuming the system the power domain has to be powered on early so
> > any runtime PM aware devices could resume.
> > 
> > This fixes following scenario reproduced on Exynos DRM:
> > 1. Power domain is off before suspending the system.
> > 2. System is suspended to RAM.
> > 3. Resuming starts. The Exynos DRM driver resume callback is called.
> > 4. The Exynos DRM driver calls drm_helper_resume_force_mode which turns
> >     the screen on by calling exynos_dsi_dpms with DRM_MODE_DPMS_ON.
> > 5. The Exynos DSI driver calls pm_runtime_get. The driver runtime
> >     resumes and this should turn LCD power domain on.
> > 6. Unfortunately the domain cannot be turned on because system resume is
> >     in progress and genpd->prepared_count is positive.
> 
> Just interesting, what value will be returned by pm_runtime_enabled()
> from any of your .resume() callback (for any device which belongs to
> some Generic PM domain)?

exynos_drm_resume: false
exynos_dsi_enable: true

Full backtrace leading to exynos_dsi_enable:
[   37.944830] ------------[ cut here ]------------
[   37.944860] WARNING: CPU: 0 PID: 3125 at drivers/gpu/drm/exynos/exynos_drm_dsi.c:1360 exynos_dsi_dpms+0xc0/0x398()
[   37.944869] Modules linked in:
[   37.944883] CPU: 0 PID: 3125 Comm: bash Tainted: G        W      3.17.0-next-20141020-00050-g844ed80678d5-dirty #479
[   37.944923] [<c0013d64>] (unwind_backtrace) from [<c0010eb0>] (show_stack+0x10/0x14)
[   37.944949] [<c0010eb0>] (show_stack) from [<c04a37b0>] (dump_stack+0x70/0xbc)
[   37.944977] [<c04a37b0>] (dump_stack) from [<c0021420>] (warn_slowpath_common+0x64/0x88)
[   37.944992] [<c0021420>] (warn_slowpath_common) from [<c0021460>] (warn_slowpath_null+0x1c/0x24)
[   37.945011] [<c0021460>] (warn_slowpath_null) from [<c0268940>] (exynos_dsi_dpms+0xc0/0x398)
[   37.945024] [<c0268940>] (exynos_dsi_dpms) from [<c0263520>] (exynos_drm_encoder_commit+0x24/0x40)
[   37.945044] [<c0263520>] (exynos_drm_encoder_commit) from [<c023f0b4>] (drm_crtc_helper_set_mode+0x400/0x4e8)
[   37.945057] [<c023f0b4>] (drm_crtc_helper_set_mode) from [<c023f204>] (drm_helper_resume_force_mode+0x68/0x124)
[   37.945083] [<c023f204>] (drm_helper_resume_force_mode) from [<c0262b9c>] (exynos_drm_resume+0x80/0x90)
[   37.945100] [<c0262b9c>] (exynos_drm_resume) from [<c02832f8>] (platform_pm_resume+0x2c/0x4c)
[   37.945118] [<c02832f8>] (platform_pm_resume) from [<c028a320>] (dpm_run_callback.isra.7+0x2c/0x64)
[   37.945130] [<c028a320>] (dpm_run_callback.isra.7) from [<c028a404>] (device_resume+0xac/0x180)
[   37.945141] [<c028a404>] (device_resume) from [<c028b65c>] (dpm_resume+0xe8/0x20c)
[   37.945152] [<c028b65c>] (dpm_resume) from [<c028b91c>] (dpm_resume_end+0xc/0x18)
[   37.945175] [<c028b91c>] (dpm_resume_end) from [<c0054b48>] (suspend_devices_and_enter+0x230/0x3c8)
[   37.945190] [<c0054b48>] (suspend_devices_and_enter) from [<c0054f48>] (pm_suspend+0x268/0x29c)
[   37.945202] [<c0054f48>] (pm_suspend) from [<c0053a74>] (state_store+0x6c/0xbc)
[   37.945220] [<c0053a74>] (state_store) from [<c01d0890>] (kobj_attr_store+0x14/0x20)
[   37.945235] [<c01d0890>] (kobj_attr_store) from [<c011e56c>] (sysfs_kf_write+0x44/0x48)
[   37.945245] [<c011e56c>] (sysfs_kf_write) from [<c011dc10>] (kernfs_fop_write+0xc0/0x17c)
[   37.945268] [<c011dc10>] (kernfs_fop_write) from [<c00c7bfc>] (vfs_write+0xa0/0x1a8)
[   37.945282] [<c00c7bfc>] (vfs_write) from [<c00c7f14>] (SyS_write+0x40/0x8c)
[   37.945299] [<c00c7f14>] (SyS_write) from [<c000e6e0>] (ret_fast_syscall+0x0/0x30)
[   37.945307] ---[ end trace e930e0edfd9a5ad2 ]---


> I'm asking, because as I can see Runtime PM can be disabled from pm_genpd_prepare().
> 
> Thank you.
> 
> Oh. I've just found that you might get this issue if you will try to do
> suspend when PM domain is ON ;)
> 
> Any way, In my opinion, It might be better to fix pm_genpd_prepare() so
> it will not increment prepared_count when initial state of the GPD is
> GPD_STATE_POWER_OFF. Seems it's needed only in opposite case -
> when state of GPD has to be restored from pm_genpd_resume_noirq().

That sounds good but what about cases when the device will runtime
resume during suspend (early at suspend)? Actually I seen this with
framebuffer console. Right after starting suspend the console is flushed
and poked which leads to powering on LCD.

Best regards,
Krzysztof

Re: [RFC 1/2] PM / Domains: Power on domain early during system resume

[Krzysztof Kozlowski]

On czw, 2014-10-23 at 19:20 +0300, Grygorii Strashko wrote:
> Hi Krzysztof,
> 
> On 10/23/2014 04:48 PM, Krzysztof Kozlowski wrote:
> > When resuming the system the power domain has to be powered on early so
> > any runtime PM aware devices could resume.
> > 
> > This fixes following scenario reproduced on Exynos DRM:
> > 1. Power domain is off before suspending the system.
> > 2. System is suspended to RAM.
> > 3. Resuming starts. The Exynos DRM driver resume callback is called.
> > 4. The Exynos DRM driver calls drm_helper_resume_force_mode which turns
> >     the screen on by calling exynos_dsi_dpms with DRM_MODE_DPMS_ON.
> > 5. The Exynos DSI driver calls pm_runtime_get. The driver runtime
> >     resumes and this should turn LCD power domain on.
> > 6. Unfortunately the domain cannot be turned on because system resume is
> >     in progress and genpd->prepared_count is positive.
> 
> Just interesting, what value will be returned by pm_runtime_enabled()
> from any of your .resume() callback (for any device which belongs to
> some Generic PM domain)?

exynos_drm_resume: false
exynos_dsi_enable: true

Full backtrace leading to exynos_dsi_enable:
[   37.944830] ------------[ cut here ]------------
[   37.944860] WARNING: CPU: 0 PID: 3125 at drivers/gpu/drm/exynos/exynos_drm_dsi.c:1360 exynos_dsi_dpms+0xc0/0x398()
[   37.944869] Modules linked in:
[   37.944883] CPU: 0 PID: 3125 Comm: bash Tainted: G        W      3.17.0-next-20141020-00050-g844ed80678d5-dirty #479
[   37.944923] [<c0013d64>] (unwind_backtrace) from [<c0010eb0>] (show_stack+0x10/0x14)
[   37.944949] [<c0010eb0>] (show_stack) from [<c04a37b0>] (dump_stack+0x70/0xbc)
[   37.944977] [<c04a37b0>] (dump_stack) from [<c0021420>] (warn_slowpath_common+0x64/0x88)
[   37.944992] [<c0021420>] (warn_slowpath_common) from [<c0021460>] (warn_slowpath_null+0x1c/0x24)
[   37.945011] [<c0021460>] (warn_slowpath_null) from [<c0268940>] (exynos_dsi_dpms+0xc0/0x398)
[   37.945024] [<c0268940>] (exynos_dsi_dpms) from [<c0263520>] (exynos_drm_encoder_commit+0x24/0x40)
[   37.945044] [<c0263520>] (exynos_drm_encoder_commit) from [<c023f0b4>] (drm_crtc_helper_set_mode+0x400/0x4e8)
[   37.945057] [<c023f0b4>] (drm_crtc_helper_set_mode) from [<c023f204>] (drm_helper_resume_force_mode+0x68/0x124)
[   37.945083] [<c023f204>] (drm_helper_resume_force_mode) from [<c0262b9c>] (exynos_drm_resume+0x80/0x90)
[   37.945100] [<c0262b9c>] (exynos_drm_resume) from [<c02832f8>] (platform_pm_resume+0x2c/0x4c)
[   37.945118] [<c02832f8>] (platform_pm_resume) from [<c028a320>] (dpm_run_callback.isra.7+0x2c/0x64)
[   37.945130] [<c028a320>] (dpm_run_callback.isra.7) from [<c028a404>] (device_resume+0xac/0x180)
[   37.945141] [<c028a404>] (device_resume) from [<c028b65c>] (dpm_resume+0xe8/0x20c)
[   37.945152] [<c028b65c>] (dpm_resume) from [<c028b91c>] (dpm_resume_end+0xc/0x18)
[   37.945175] [<c028b91c>] (dpm_resume_end) from [<c0054b48>] (suspend_devices_and_enter+0x230/0x3c8)
[   37.945190] [<c0054b48>] (suspend_devices_and_enter) from [<c0054f48>] (pm_suspend+0x268/0x29c)
[   37.945202] [<c0054f48>] (pm_suspend) from [<c0053a74>] (state_store+0x6c/0xbc)
[   37.945220] [<c0053a74>] (state_store) from [<c01d0890>] (kobj_attr_store+0x14/0x20)
[   37.945235] [<c01d0890>] (kobj_attr_store) from [<c011e56c>] (sysfs_kf_write+0x44/0x48)
[   37.945245] [<c011e56c>] (sysfs_kf_write) from [<c011dc10>] (kernfs_fop_write+0xc0/0x17c)
[   37.945268] [<c011dc10>] (kernfs_fop_write) from [<c00c7bfc>] (vfs_write+0xa0/0x1a8)
[   37.945282] [<c00c7bfc>] (vfs_write) from [<c00c7f14>] (SyS_write+0x40/0x8c)
[   37.945299] [<c00c7f14>] (SyS_write) from [<c000e6e0>] (ret_fast_syscall+0x0/0x30)
[   37.945307] ---[ end trace e930e0edfd9a5ad2 ]---


> I'm asking, because as I can see Runtime PM can be disabled from pm_genpd_prepare().
> 
> Thank you.
> 
> Oh. I've just found that you might get this issue if you will try to do
> suspend when PM domain is ON ;)
> 
> Any way, In my opinion, It might be better to fix pm_genpd_prepare() so
> it will not increment prepared_count when initial state of the GPD is
> GPD_STATE_POWER_OFF. Seems it's needed only in opposite case -
> when state of GPD has to be restored from pm_genpd_resume_noirq().

That sounds good but what about cases when the device will runtime
resume during suspend (early at suspend)? Actually I seen this with
framebuffer console. Right after starting suspend the console is flushed
and poked which leads to powering on LCD.

Best regards,
Krzysztof

[RFC 1/2] PM / Domains: Power on domain early during system resume

[Krzysztof Kozlowski]

On czw, 2014-10-23 at 19:20 +0300, Grygorii Strashko wrote:
> Hi Krzysztof,
> 
> On 10/23/2014 04:48 PM, Krzysztof Kozlowski wrote:
> > When resuming the system the power domain has to be powered on early so
> > any runtime PM aware devices could resume.
> > 
> > This fixes following scenario reproduced on Exynos DRM:
> > 1. Power domain is off before suspending the system.
> > 2. System is suspended to RAM.
> > 3. Resuming starts. The Exynos DRM driver resume callback is called.
> > 4. The Exynos DRM driver calls drm_helper_resume_force_mode which turns
> >     the screen on by calling exynos_dsi_dpms with DRM_MODE_DPMS_ON.
> > 5. The Exynos DSI driver calls pm_runtime_get. The driver runtime
> >     resumes and this should turn LCD power domain on.
> > 6. Unfortunately the domain cannot be turned on because system resume is
> >     in progress and genpd->prepared_count is positive.
> 
> Just interesting, what value will be returned by pm_runtime_enabled()
> from any of your .resume() callback (for any device which belongs to
> some Generic PM domain)?

exynos_drm_resume: false
exynos_dsi_enable: true

Full backtrace leading to exynos_dsi_enable:
[   37.944830] ------------[ cut here ]------------
[   37.944860] WARNING: CPU: 0 PID: 3125 at drivers/gpu/drm/exynos/exynos_drm_dsi.c:1360 exynos_dsi_dpms+0xc0/0x398()
[   37.944869] Modules linked in:
[   37.944883] CPU: 0 PID: 3125 Comm: bash Tainted: G        W      3.17.0-next-20141020-00050-g844ed80678d5-dirty #479
[   37.944923] [<c0013d64>] (unwind_backtrace) from [<c0010eb0>] (show_stack+0x10/0x14)
[   37.944949] [<c0010eb0>] (show_stack) from [<c04a37b0>] (dump_stack+0x70/0xbc)
[   37.944977] [<c04a37b0>] (dump_stack) from [<c0021420>] (warn_slowpath_common+0x64/0x88)
[   37.944992] [<c0021420>] (warn_slowpath_common) from [<c0021460>] (warn_slowpath_null+0x1c/0x24)
[   37.945011] [<c0021460>] (warn_slowpath_null) from [<c0268940>] (exynos_dsi_dpms+0xc0/0x398)
[   37.945024] [<c0268940>] (exynos_dsi_dpms) from [<c0263520>] (exynos_drm_encoder_commit+0x24/0x40)
[   37.945044] [<c0263520>] (exynos_drm_encoder_commit) from [<c023f0b4>] (drm_crtc_helper_set_mode+0x400/0x4e8)
[   37.945057] [<c023f0b4>] (drm_crtc_helper_set_mode) from [<c023f204>] (drm_helper_resume_force_mode+0x68/0x124)
[   37.945083] [<c023f204>] (drm_helper_resume_force_mode) from [<c0262b9c>] (exynos_drm_resume+0x80/0x90)
[   37.945100] [<c0262b9c>] (exynos_drm_resume) from [<c02832f8>] (platform_pm_resume+0x2c/0x4c)
[   37.945118] [<c02832f8>] (platform_pm_resume) from [<c028a320>] (dpm_run_callback.isra.7+0x2c/0x64)
[   37.945130] [<c028a320>] (dpm_run_callback.isra.7) from [<c028a404>] (device_resume+0xac/0x180)
[   37.945141] [<c028a404>] (device_resume) from [<c028b65c>] (dpm_resume+0xe8/0x20c)
[   37.945152] [<c028b65c>] (dpm_resume) from [<c028b91c>] (dpm_resume_end+0xc/0x18)
[   37.945175] [<c028b91c>] (dpm_resume_end) from [<c0054b48>] (suspend_devices_and_enter+0x230/0x3c8)
[   37.945190] [<c0054b48>] (suspend_devices_and_enter) from [<c0054f48>] (pm_suspend+0x268/0x29c)
[   37.945202] [<c0054f48>] (pm_suspend) from [<c0053a74>] (state_store+0x6c/0xbc)
[   37.945220] [<c0053a74>] (state_store) from [<c01d0890>] (kobj_attr_store+0x14/0x20)
[   37.945235] [<c01d0890>] (kobj_attr_store) from [<c011e56c>] (sysfs_kf_write+0x44/0x48)
[   37.945245] [<c011e56c>] (sysfs_kf_write) from [<c011dc10>] (kernfs_fop_write+0xc0/0x17c)
[   37.945268] [<c011dc10>] (kernfs_fop_write) from [<c00c7bfc>] (vfs_write+0xa0/0x1a8)
[   37.945282] [<c00c7bfc>] (vfs_write) from [<c00c7f14>] (SyS_write+0x40/0x8c)
[   37.945299] [<c00c7f14>] (SyS_write) from [<c000e6e0>] (ret_fast_syscall+0x0/0x30)
[   37.945307] ---[ end trace e930e0edfd9a5ad2 ]---


> I'm asking, because as I can see Runtime PM can be disabled from pm_genpd_prepare().
> 
> Thank you.
> 
> Oh. I've just found that you might get this issue if you will try to do
> suspend when PM domain is ON ;)
> 
> Any way, In my opinion, It might be better to fix pm_genpd_prepare() so
> it will not increment prepared_count when initial state of the GPD is
> GPD_STATE_POWER_OFF. Seems it's needed only in opposite case -
> when state of GPD has to be restored from pm_genpd_resume_noirq().

That sounds good but what about cases when the device will runtime
resume during suspend (early at suspend)? Actually I seen this with
framebuffer console. Right after starting suspend the console is flushed
and poked which leads to powering on LCD.

Best regards,
Krzysztof

Re: [RFC 1/2] PM / Domains: Power on domain early during system resume

[Kevin Hilman]

Krzysztof Kozlowski <k.kozlowski@samsung.com> writes:

> When resuming the system the power domain has to be powered on early so
> any runtime PM aware devices could resume.
>
> This fixes following scenario reproduced on Exynos DRM:
> 1. Power domain is off before suspending the system.
> 2. System is suspended to RAM.
> 3. Resuming starts. The Exynos DRM driver resume callback is called.
> 4. The Exynos DRM driver calls drm_helper_resume_force_mode which turns
>    the screen on by calling exynos_dsi_dpms with DRM_MODE_DPMS_ON.

Dumb Q: if the device (and power domain) were off before (and during)
suspend, why are they being resumed?

Shouldn't the resume path restore things to the same state they were
before suspend?

Kevin

[RFC 1/2] PM / Domains: Power on domain early during system resume

[Kevin Hilman]

Krzysztof Kozlowski <k.kozlowski@samsung.com> writes:

> When resuming the system the power domain has to be powered on early so
> any runtime PM aware devices could resume.
>
> This fixes following scenario reproduced on Exynos DRM:
> 1. Power domain is off before suspending the system.
> 2. System is suspended to RAM.
> 3. Resuming starts. The Exynos DRM driver resume callback is called.
> 4. The Exynos DRM driver calls drm_helper_resume_force_mode which turns
>    the screen on by calling exynos_dsi_dpms with DRM_MODE_DPMS_ON.

Dumb Q: if the device (and power domain) were off before (and during)
suspend, why are they being resumed?

Shouldn't the resume path restore things to the same state they were
before suspend?

Kevin

Re: [RFC 1/2] PM / Domains: Power on domain early during system resume

[Krzysztof Kozlowski]

On śro, 2014-10-29 at 10:46 -0700, Kevin Hilman wrote:
> Krzysztof Kozlowski <k.kozlowski@samsung.com> writes:
> 
> > When resuming the system the power domain has to be powered on early so
> > any runtime PM aware devices could resume.
> >
> > This fixes following scenario reproduced on Exynos DRM:
> > 1. Power domain is off before suspending the system.
> > 2. System is suspended to RAM.
> > 3. Resuming starts. The Exynos DRM driver resume callback is called.
> > 4. The Exynos DRM driver calls drm_helper_resume_force_mode which turns
> >    the screen on by calling exynos_dsi_dpms with DRM_MODE_DPMS_ON.
> 
> Dumb Q: if the device (and power domain) were off before (and during)
> suspend, why are they being resumed?
> 
> Shouldn't the resume path restore things to the same state they were
> before suspend?

One could expect that... but the Exynos DRM driver behaves differently
(and some other drivers also). In resume method it calls
drm_helper_resume_force_mode() which forces restoring mode setting
configuration. Apparently setting a mode needs DPMS on:
static void exynos_drm_crtc_commit(struct drm_crtc *crtc)
{
	...
	exynos_drm_crtc_dpms(crtc, DRM_MODE_DPMS_ON);
	...

The previous DPMS status (status during suspend) is completely ignored
here.

I dunno why... maybe someone from DRM could share some thoughts?


Best regards,
Krzysztof

[RFC 1/2] PM / Domains: Power on domain early during system resume

[Krzysztof Kozlowski]

On ?ro, 2014-10-29 at 10:46 -0700, Kevin Hilman wrote:
> Krzysztof Kozlowski <k.kozlowski@samsung.com> writes:
> 
> > When resuming the system the power domain has to be powered on early so
> > any runtime PM aware devices could resume.
> >
> > This fixes following scenario reproduced on Exynos DRM:
> > 1. Power domain is off before suspending the system.
> > 2. System is suspended to RAM.
> > 3. Resuming starts. The Exynos DRM driver resume callback is called.
> > 4. The Exynos DRM driver calls drm_helper_resume_force_mode which turns
> >    the screen on by calling exynos_dsi_dpms with DRM_MODE_DPMS_ON.
> 
> Dumb Q: if the device (and power domain) were off before (and during)
> suspend, why are they being resumed?
> 
> Shouldn't the resume path restore things to the same state they were
> before suspend?

One could expect that... but the Exynos DRM driver behaves differently
(and some other drivers also). In resume method it calls
drm_helper_resume_force_mode() which forces restoring mode setting
configuration. Apparently setting a mode needs DPMS on:
static void exynos_drm_crtc_commit(struct drm_crtc *crtc)
{
	...
	exynos_drm_crtc_dpms(crtc, DRM_MODE_DPMS_ON);
	...

The previous DPMS status (status during suspend) is completely ignored
here.

I dunno why... maybe someone from DRM could share some thoughts?


Best regards,
Krzysztof

Re: [RFC 1/2] PM / Domains: Power on domain early during system resume

[Andrzej Hajda]

On 10/30/2014 08:36 AM, Krzysztof Kozlowski wrote:
> On śro, 2014-10-29 at 10:46 -0700, Kevin Hilman wrote:
>> Krzysztof Kozlowski <k.kozlowski@samsung.com> writes:
>>
>>> When resuming the system the power domain has to be powered on early so
>>> any runtime PM aware devices could resume.
>>>
>>> This fixes following scenario reproduced on Exynos DRM:
>>> 1. Power domain is off before suspending the system.
>>> 2. System is suspended to RAM.
>>> 3. Resuming starts. The Exynos DRM driver resume callback is called.
>>> 4. The Exynos DRM driver calls drm_helper_resume_force_mode which turns
>>>    the screen on by calling exynos_dsi_dpms with DRM_MODE_DPMS_ON.
>> Dumb Q: if the device (and power domain) were off before (and during)
>> suspend, why are they being resumed?
>>
>> Shouldn't the resume path restore things to the same state they were
>> before suspend?
> One could expect that... but the Exynos DRM driver behaves differently
> (and some other drivers also). In resume method it calls
> drm_helper_resume_force_mode() which forces restoring mode setting
> configuration. Apparently setting a mode needs DPMS on:
> static void exynos_drm_crtc_commit(struct drm_crtc *crtc)
> {
> 	...
> 	exynos_drm_crtc_dpms(crtc, DRM_MODE_DPMS_ON);
> 	...
>
> The previous DPMS status (status during suspend) is completely ignored
> here.

Suspend callback switches off all connectors (thus all other devs in
their pipeline) by calling dpms_off,
in restore callback all devs are restored to their previous state by
calling appropriate dpms.
So I guess drm_helper_resume_force_mode() call at the end of resume is
incorrect.
On the other side it is present in many other drivers, so I am also
little bit confused.

Regards
Andrzej

Re: [RFC 1/2] PM / Domains: Power on domain early during system resume

[Andrzej Hajda]

On 10/30/2014 08:36 AM, Krzysztof Kozlowski wrote:
> On śro, 2014-10-29 at 10:46 -0700, Kevin Hilman wrote:
>> Krzysztof Kozlowski <k.kozlowski@samsung.com> writes:
>>
>>> When resuming the system the power domain has to be powered on early so
>>> any runtime PM aware devices could resume.
>>>
>>> This fixes following scenario reproduced on Exynos DRM:
>>> 1. Power domain is off before suspending the system.
>>> 2. System is suspended to RAM.
>>> 3. Resuming starts. The Exynos DRM driver resume callback is called.
>>> 4. The Exynos DRM driver calls drm_helper_resume_force_mode which turns
>>>    the screen on by calling exynos_dsi_dpms with DRM_MODE_DPMS_ON.
>> Dumb Q: if the device (and power domain) were off before (and during)
>> suspend, why are they being resumed?
>>
>> Shouldn't the resume path restore things to the same state they were
>> before suspend?
> One could expect that... but the Exynos DRM driver behaves differently
> (and some other drivers also). In resume method it calls
> drm_helper_resume_force_mode() which forces restoring mode setting
> configuration. Apparently setting a mode needs DPMS on:
> static void exynos_drm_crtc_commit(struct drm_crtc *crtc)
> {
> 	...
> 	exynos_drm_crtc_dpms(crtc, DRM_MODE_DPMS_ON);
> 	...
>
> The previous DPMS status (status during suspend) is completely ignored
> here.

Suspend callback switches off all connectors (thus all other devs in
their pipeline) by calling dpms_off,
in restore callback all devs are restored to their previous state by
calling appropriate dpms.
So I guess drm_helper_resume_force_mode() call at the end of resume is
incorrect.
On the other side it is present in many other drivers, so I am also
little bit confused.

Regards
Andrzej


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[RFC 1/2] PM / Domains: Power on domain early during system resume

[Andrzej Hajda]

On 10/30/2014 08:36 AM, Krzysztof Kozlowski wrote:
> On ?ro, 2014-10-29 at 10:46 -0700, Kevin Hilman wrote:
>> Krzysztof Kozlowski <k.kozlowski@samsung.com> writes:
>>
>>> When resuming the system the power domain has to be powered on early so
>>> any runtime PM aware devices could resume.
>>>
>>> This fixes following scenario reproduced on Exynos DRM:
>>> 1. Power domain is off before suspending the system.
>>> 2. System is suspended to RAM.
>>> 3. Resuming starts. The Exynos DRM driver resume callback is called.
>>> 4. The Exynos DRM driver calls drm_helper_resume_force_mode which turns
>>>    the screen on by calling exynos_dsi_dpms with DRM_MODE_DPMS_ON.
>> Dumb Q: if the device (and power domain) were off before (and during)
>> suspend, why are they being resumed?
>>
>> Shouldn't the resume path restore things to the same state they were
>> before suspend?
> One could expect that... but the Exynos DRM driver behaves differently
> (and some other drivers also). In resume method it calls
> drm_helper_resume_force_mode() which forces restoring mode setting
> configuration. Apparently setting a mode needs DPMS on:
> static void exynos_drm_crtc_commit(struct drm_crtc *crtc)
> {
> 	...
> 	exynos_drm_crtc_dpms(crtc, DRM_MODE_DPMS_ON);
> 	...
>
> The previous DPMS status (status during suspend) is completely ignored
> here.

Suspend callback switches off all connectors (thus all other devs in
their pipeline) by calling dpms_off,
in restore callback all devs are restored to their previous state by
calling appropriate dpms.
So I guess drm_helper_resume_force_mode() call at the end of resume is
incorrect.
On the other side it is present in many other drivers, so I am also
little bit confused.

Regards
Andrzej

Re: [RFC 1/2] PM / Domains: Power on domain early during system resume

[Kevin Hilman]

Andrzej Hajda <a.hajda@samsung.com> writes:

> On 10/30/2014 08:36 AM, Krzysztof Kozlowski wrote:
>> On śro, 2014-10-29 at 10:46 -0700, Kevin Hilman wrote:
>>> Krzysztof Kozlowski <k.kozlowski@samsung.com> writes:
>>>
>>>> When resuming the system the power domain has to be powered on early so
>>>> any runtime PM aware devices could resume.
>>>>
>>>> This fixes following scenario reproduced on Exynos DRM:
>>>> 1. Power domain is off before suspending the system.
>>>> 2. System is suspended to RAM.
>>>> 3. Resuming starts. The Exynos DRM driver resume callback is called.
>>>> 4. The Exynos DRM driver calls drm_helper_resume_force_mode which turns
>>>>    the screen on by calling exynos_dsi_dpms with DRM_MODE_DPMS_ON.
>>> Dumb Q: if the device (and power domain) were off before (and during)
>>> suspend, why are they being resumed?
>>>
>>> Shouldn't the resume path restore things to the same state they were
>>> before suspend?
>> One could expect that... but the Exynos DRM driver behaves differently
>> (and some other drivers also). In resume method it calls
>> drm_helper_resume_force_mode() which forces restoring mode setting
>> configuration. Apparently setting a mode needs DPMS on:
>> static void exynos_drm_crtc_commit(struct drm_crtc *crtc)
>> {
>> 	...
>> 	exynos_drm_crtc_dpms(crtc, DRM_MODE_DPMS_ON);
>> 	...
>>
>> The previous DPMS status (status during suspend) is completely ignored
>> here.
>
> Suspend callback switches off all connectors (thus all other devs in
> their pipeline) by calling dpms_off,
> in restore callback all devs are restored to their previous state by
> calling appropriate dpms.
> So I guess drm_helper_resume_force_mode() call at the end of resume is
> incorrect.

Though I'm not terribly familiar with DRM, it seems incorrect because I
expect resume to restore the state of things when suspend happened, not
forcibly resume everything.

> On the other side it is present in many other drivers, so I am also
> little bit confused.

Many other DRM drivers?  or other drivers too?

Kevin

[RFC 1/2] PM / Domains: Power on domain early during system resume

[Kevin Hilman]

Andrzej Hajda <a.hajda@samsung.com> writes:

> On 10/30/2014 08:36 AM, Krzysztof Kozlowski wrote:
>> On ?ro, 2014-10-29 at 10:46 -0700, Kevin Hilman wrote:
>>> Krzysztof Kozlowski <k.kozlowski@samsung.com> writes:
>>>
>>>> When resuming the system the power domain has to be powered on early so
>>>> any runtime PM aware devices could resume.
>>>>
>>>> This fixes following scenario reproduced on Exynos DRM:
>>>> 1. Power domain is off before suspending the system.
>>>> 2. System is suspended to RAM.
>>>> 3. Resuming starts. The Exynos DRM driver resume callback is called.
>>>> 4. The Exynos DRM driver calls drm_helper_resume_force_mode which turns
>>>>    the screen on by calling exynos_dsi_dpms with DRM_MODE_DPMS_ON.
>>> Dumb Q: if the device (and power domain) were off before (and during)
>>> suspend, why are they being resumed?
>>>
>>> Shouldn't the resume path restore things to the same state they were
>>> before suspend?
>> One could expect that... but the Exynos DRM driver behaves differently
>> (and some other drivers also). In resume method it calls
>> drm_helper_resume_force_mode() which forces restoring mode setting
>> configuration. Apparently setting a mode needs DPMS on:
>> static void exynos_drm_crtc_commit(struct drm_crtc *crtc)
>> {
>> 	...
>> 	exynos_drm_crtc_dpms(crtc, DRM_MODE_DPMS_ON);
>> 	...
>>
>> The previous DPMS status (status during suspend) is completely ignored
>> here.
>
> Suspend callback switches off all connectors (thus all other devs in
> their pipeline) by calling dpms_off,
> in restore callback all devs are restored to their previous state by
> calling appropriate dpms.
> So I guess drm_helper_resume_force_mode() call at the end of resume is
> incorrect.

Though I'm not terribly familiar with DRM, it seems incorrect because I
expect resume to restore the state of things when suspend happened, not
forcibly resume everything.

> On the other side it is present in many other drivers, so I am also
> little bit confused.

Many other DRM drivers?  or other drivers too?

Kevin

Re: [RFC 1/2] PM / Domains: Power on domain early during system resume

[Grygorii Strashko]

On 11/03/2014 06:13 PM, Kevin Hilman wrote:
> Andrzej Hajda <a.hajda@samsung.com> writes:
> 
>> On 10/30/2014 08:36 AM, Krzysztof Kozlowski wrote:
>>> On śro, 2014-10-29 at 10:46 -0700, Kevin Hilman wrote:
>>>> Krzysztof Kozlowski <k.kozlowski@samsung.com> writes:
>>>>
>>>>> When resuming the system the power domain has to be powered on early so
>>>>> any runtime PM aware devices could resume.
>>>>>
>>>>> This fixes following scenario reproduced on Exynos DRM:
>>>>> 1. Power domain is off before suspending the system.
>>>>> 2. System is suspended to RAM.
>>>>> 3. Resuming starts. The Exynos DRM driver resume callback is called.
>>>>> 4. The Exynos DRM driver calls drm_helper_resume_force_mode which turns
>>>>>     the screen on by calling exynos_dsi_dpms with DRM_MODE_DPMS_ON.
>>>> Dumb Q: if the device (and power domain) were off before (and during)
>>>> suspend, why are they being resumed?
>>>>
>>>> Shouldn't the resume path restore things to the same state they were
>>>> before suspend?
>>> One could expect that... but the Exynos DRM driver behaves differently
>>> (and some other drivers also). In resume method it calls
>>> drm_helper_resume_force_mode() which forces restoring mode setting
>>> configuration. Apparently setting a mode needs DPMS on:
>>> static void exynos_drm_crtc_commit(struct drm_crtc *crtc)
>>> {
>>> 	...
>>> 	exynos_drm_crtc_dpms(crtc, DRM_MODE_DPMS_ON);
>>> 	...
>>>
>>> The previous DPMS status (status during suspend) is completely ignored
>>> here.
>>
>> Suspend callback switches off all connectors (thus all other devs in
>> their pipeline) by calling dpms_off,
>> in restore callback all devs are restored to their previous state by
>> calling appropriate dpms.
>> So I guess drm_helper_resume_force_mode() call at the end of resume is
>> incorrect.
> 
> Though I'm not terribly familiar with DRM, it seems incorrect because I
> expect resume to restore the state of things when suspend happened, not
> forcibly resume everything.
> 
>> On the other side it is present in many other drivers, so I am also
>> little bit confused.
> 
> Many other DRM drivers?  or other drivers too?

If I understand GPD code right :)
There is "small" problem :( Now if PM domain is OFF before suspend
- it's prohibited to turn it on during suspending/resuming.
   commit 596ba34bcd2978ee9823cc1d84df230576f8ffb9
   PM / Domains: System-wide transitions support for generic domains (v5)

But, it is possible to have devices which supports few power states, like:
on, sleep/low power, off (for example OMAP devices and also I saw this
for some I2C devices - can recollect only that it was some sensor and touchscreen). 
In normal operational mode Runtime PM switches device between on and sleep/low power states,
but during suspend device need to be woken up and reconfigured to off state.

Also I found, that It looks like due to continuous refactoring the call of
pm_generic_suspend_noirq(dev) was finally dropped from pm_genpd_suspend_noirq(),
so .suspend_noirq() callback will never be called for devices from GPD.

Seems, that is the problem which this patch tries to fix and looks like there are
 will be more such kind of report as GPD is become used widely.

regards,
-grygorii

Re: [RFC 1/2] PM / Domains: Power on domain early during system resume

[Grygorii Strashko]

On 11/03/2014 06:13 PM, Kevin Hilman wrote:
> Andrzej Hajda <a.hajda@samsung.com> writes:
> 
>> On 10/30/2014 08:36 AM, Krzysztof Kozlowski wrote:
>>> On śro, 2014-10-29 at 10:46 -0700, Kevin Hilman wrote:
>>>> Krzysztof Kozlowski <k.kozlowski@samsung.com> writes:
>>>>
>>>>> When resuming the system the power domain has to be powered on early so
>>>>> any runtime PM aware devices could resume.
>>>>>
>>>>> This fixes following scenario reproduced on Exynos DRM:
>>>>> 1. Power domain is off before suspending the system.
>>>>> 2. System is suspended to RAM.
>>>>> 3. Resuming starts. The Exynos DRM driver resume callback is called.
>>>>> 4. The Exynos DRM driver calls drm_helper_resume_force_mode which turns
>>>>>     the screen on by calling exynos_dsi_dpms with DRM_MODE_DPMS_ON.
>>>> Dumb Q: if the device (and power domain) were off before (and during)
>>>> suspend, why are they being resumed?
>>>>
>>>> Shouldn't the resume path restore things to the same state they were
>>>> before suspend?
>>> One could expect that... but the Exynos DRM driver behaves differently
>>> (and some other drivers also). In resume method it calls
>>> drm_helper_resume_force_mode() which forces restoring mode setting
>>> configuration. Apparently setting a mode needs DPMS on:
>>> static void exynos_drm_crtc_commit(struct drm_crtc *crtc)
>>> {
>>> 	...
>>> 	exynos_drm_crtc_dpms(crtc, DRM_MODE_DPMS_ON);
>>> 	...
>>>
>>> The previous DPMS status (status during suspend) is completely ignored
>>> here.
>>
>> Suspend callback switches off all connectors (thus all other devs in
>> their pipeline) by calling dpms_off,
>> in restore callback all devs are restored to their previous state by
>> calling appropriate dpms.
>> So I guess drm_helper_resume_force_mode() call at the end of resume is
>> incorrect.
> 
> Though I'm not terribly familiar with DRM, it seems incorrect because I
> expect resume to restore the state of things when suspend happened, not
> forcibly resume everything.
> 
>> On the other side it is present in many other drivers, so I am also
>> little bit confused.
> 
> Many other DRM drivers?  or other drivers too?

If I understand GPD code right :)
There is "small" problem :( Now if PM domain is OFF before suspend
- it's prohibited to turn it on during suspending/resuming.
   commit 596ba34bcd2978ee9823cc1d84df230576f8ffb9
   PM / Domains: System-wide transitions support for generic domains (v5)

But, it is possible to have devices which supports few power states, like:
on, sleep/low power, off (for example OMAP devices and also I saw this
for some I2C devices - can recollect only that it was some sensor and touchscreen). 
In normal operational mode Runtime PM switches device between on and sleep/low power states,
but during suspend device need to be woken up and reconfigured to off state.

Also I found, that It looks like due to continuous refactoring the call of
pm_generic_suspend_noirq(dev) was finally dropped from pm_genpd_suspend_noirq(),
so .suspend_noirq() callback will never be called for devices from GPD.

Seems, that is the problem which this patch tries to fix and looks like there are
 will be more such kind of report as GPD is become used widely.

regards,
-grygorii

[RFC 1/2] PM / Domains: Power on domain early during system resume

[Grygorii Strashko]

On 11/03/2014 06:13 PM, Kevin Hilman wrote:
> Andrzej Hajda <a.hajda@samsung.com> writes:
> 
>> On 10/30/2014 08:36 AM, Krzysztof Kozlowski wrote:
>>> On ?ro, 2014-10-29 at 10:46 -0700, Kevin Hilman wrote:
>>>> Krzysztof Kozlowski <k.kozlowski@samsung.com> writes:
>>>>
>>>>> When resuming the system the power domain has to be powered on early so
>>>>> any runtime PM aware devices could resume.
>>>>>
>>>>> This fixes following scenario reproduced on Exynos DRM:
>>>>> 1. Power domain is off before suspending the system.
>>>>> 2. System is suspended to RAM.
>>>>> 3. Resuming starts. The Exynos DRM driver resume callback is called.
>>>>> 4. The Exynos DRM driver calls drm_helper_resume_force_mode which turns
>>>>>     the screen on by calling exynos_dsi_dpms with DRM_MODE_DPMS_ON.
>>>> Dumb Q: if the device (and power domain) were off before (and during)
>>>> suspend, why are they being resumed?
>>>>
>>>> Shouldn't the resume path restore things to the same state they were
>>>> before suspend?
>>> One could expect that... but the Exynos DRM driver behaves differently
>>> (and some other drivers also). In resume method it calls
>>> drm_helper_resume_force_mode() which forces restoring mode setting
>>> configuration. Apparently setting a mode needs DPMS on:
>>> static void exynos_drm_crtc_commit(struct drm_crtc *crtc)
>>> {
>>> 	...
>>> 	exynos_drm_crtc_dpms(crtc, DRM_MODE_DPMS_ON);
>>> 	...
>>>
>>> The previous DPMS status (status during suspend) is completely ignored
>>> here.
>>
>> Suspend callback switches off all connectors (thus all other devs in
>> their pipeline) by calling dpms_off,
>> in restore callback all devs are restored to their previous state by
>> calling appropriate dpms.
>> So I guess drm_helper_resume_force_mode() call at the end of resume is
>> incorrect.
> 
> Though I'm not terribly familiar with DRM, it seems incorrect because I
> expect resume to restore the state of things when suspend happened, not
> forcibly resume everything.
> 
>> On the other side it is present in many other drivers, so I am also
>> little bit confused.
> 
> Many other DRM drivers?  or other drivers too?

If I understand GPD code right :)
There is "small" problem :( Now if PM domain is OFF before suspend
- it's prohibited to turn it on during suspending/resuming.
   commit 596ba34bcd2978ee9823cc1d84df230576f8ffb9
   PM / Domains: System-wide transitions support for generic domains (v5)

But, it is possible to have devices which supports few power states, like:
on, sleep/low power, off (for example OMAP devices and also I saw this
for some I2C devices - can recollect only that it was some sensor and touchscreen). 
In normal operational mode Runtime PM switches device between on and sleep/low power states,
but during suspend device need to be woken up and reconfigured to off state.

Also I found, that It looks like due to continuous refactoring the call of
pm_generic_suspend_noirq(dev) was finally dropped from pm_genpd_suspend_noirq(),
so .suspend_noirq() callback will never be called for devices from GPD.

Seems, that is the problem which this patch tries to fix and looks like there are
 will be more such kind of report as GPD is become used widely.

regards,
-grygorii