[PATCH] imx-drm: imx-drm-core: add suspend/resume support

[PATCH] imx-drm: imx-drm-core: add suspend/resume support

[Shawn Guo]

HDMI currently stops working after a system suspend/resume cycle.  It
turns out that the cause is the imx-hdmi encoder .dpms hook doesn't get
called from anywhere across suspend/resume cycle.

The patch follows what exynos drm driver does to walk the list of
connectors and call their .dpms function from suspend/resume hook.  And
the connectors' .dpms function will in turn filter down to the .dpms
hooks of encoders and CRTCs.

With this change, HDMI can continue working after a suspend/resume
cycle.

Signed-off-by: Shawn Guo <shawn.guo@freescale.com>
---
Tested with HDMI and LVDS.  It'd be great if someone can help test TVE
to ensure the patch doesn't break anything.

 drivers/staging/imx-drm/imx-drm-core.c | 39 ++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/drivers/staging/imx-drm/imx-drm-core.c b/drivers/staging/imx-drm/imx-drm-core.c
index def8280d7ee6..b0ea1f0ed32f 100644
--- a/drivers/staging/imx-drm/imx-drm-core.c
+++ b/drivers/staging/imx-drm/imx-drm-core.c
@@ -696,6 +696,44 @@ static int imx_drm_platform_remove(struct platform_device *pdev)
 	return 0;
 }
 
+#if CONFIG_PM_SLEEP
+static int imx_drm_suspend(struct device *dev)
+{
+	struct drm_device *drm_dev = dev_get_drvdata(dev);
+	struct drm_connector *connector;
+
+	drm_kms_helper_poll_disable(drm_dev);
+
+	drm_modeset_lock_all(drm_dev);
+	list_for_each_entry(connector, &drm_dev->mode_config.connector_list, head) {
+		if (connector->funcs->dpms)
+			connector->funcs->dpms(connector, DRM_MODE_DPMS_OFF);
+	}
+	drm_modeset_unlock_all(drm_dev);
+
+	return 0;
+}
+
+static int imx_drm_resume(struct device *dev)
+{
+	struct drm_device *drm_dev = dev_get_drvdata(dev);
+	struct drm_connector *connector;
+
+	drm_modeset_lock_all(drm_dev);
+	list_for_each_entry(connector, &drm_dev->mode_config.connector_list, head) {
+		if (connector->funcs->dpms)
+			connector->funcs->dpms(connector, DRM_MODE_DPMS_ON);
+	}
+	drm_modeset_unlock_all(drm_dev);
+
+	drm_kms_helper_poll_enable(drm_dev);
+
+	return 0;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(imx_drm_pm_ops, imx_drm_suspend, imx_drm_resume);
+
 static const struct of_device_id imx_drm_dt_ids[] = {
 	{ .compatible = "fsl,imx-display-subsystem", },
 	{ /* sentinel */ },
@@ -708,6 +746,7 @@ static struct platform_driver imx_drm_pdrv = {
 	.driver		= {
 		.owner	= THIS_MODULE,
 		.name	= "imx-drm",
+		.pm	= &imx_drm_pm_ops,
 		.of_match_table = imx_drm_dt_ids,
 	},
 };
-- 
1.9.1

[PATCH] imx-drm: imx-drm-core: add suspend/resume support

[Philipp Zabel]

Hi Shawn,

Am Donnerstag, den 24.07.2014, 17:17 +0800 schrieb Shawn Guo:
> HDMI currently stops working after a system suspend/resume cycle.  It
> turns out that the cause is the imx-hdmi encoder .dpms hook doesn't get
> called from anywhere across suspend/resume cycle.
> 
> The patch follows what exynos drm driver does to walk the list of
> connectors and call their .dpms function from suspend/resume hook.  And
> the connectors' .dpms function will in turn filter down to the .dpms
> hooks of encoders and CRTCs.
> 
> With this change, HDMI can continue working after a suspend/resume
> cycle.
> 
> Signed-off-by: Shawn Guo <shawn.guo@freescale.com>
> ---
> Tested with HDMI and LVDS.  It'd be great if someone can help test TVE
> to ensure the patch doesn't break anything.

Tested-by: Philipp Zabel <p.zabel@pengutronix.de>

on i.MX53-QSB with VGA via TVE.

regards
Philipp

[PATCH] imx-drm: imx-drm-core: add suspend/resume support

[Lucas Stach]

Am Donnerstag, den 24.07.2014, 17:17 +0800 schrieb Shawn Guo:
> HDMI currently stops working after a system suspend/resume cycle.  It
> turns out that the cause is the imx-hdmi encoder .dpms hook doesn't get
> called from anywhere across suspend/resume cycle.
> 
> The patch follows what exynos drm driver does to walk the list of
> connectors and call their .dpms function from suspend/resume hook.  And
> the connectors' .dpms function will in turn filter down to the .dpms
> hooks of encoders and CRTCs.
> 
> With this change, HDMI can continue working after a suspend/resume
> cycle.
> 
> Signed-off-by: Shawn Guo <shawn.guo@freescale.com>
> ---
> Tested with HDMI and LVDS.  It'd be great if someone can help test TVE
> to ensure the patch doesn't break anything.
> 
>  drivers/staging/imx-drm/imx-drm-core.c | 39 ++++++++++++++++++++++++++++++++++
>  1 file changed, 39 insertions(+)
> 
> diff --git a/drivers/staging/imx-drm/imx-drm-core.c b/drivers/staging/imx-drm/imx-drm-core.c
> index def8280d7ee6..b0ea1f0ed32f 100644
> --- a/drivers/staging/imx-drm/imx-drm-core.c
> +++ b/drivers/staging/imx-drm/imx-drm-core.c
> @@ -696,6 +696,44 @@ static int imx_drm_platform_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> +#if CONFIG_PM_SLEEP

use #ifdef

> +static int imx_drm_suspend(struct device *dev)
> +{
> +	struct drm_device *drm_dev = dev_get_drvdata(dev);
> +	struct drm_connector *connector;
> +
> +	drm_kms_helper_poll_disable(drm_dev);
> +

That's ok.

> +	drm_modeset_lock_all(drm_dev);
> +	list_for_each_entry(connector, &drm_dev->mode_config.connector_list, head) {
> +		if (connector->funcs->dpms)
> +			connector->funcs->dpms(connector, DRM_MODE_DPMS_OFF);
> +	}

Don't touch DPMS state here. See below.

> +	drm_modeset_unlock_all(drm_dev);
> +
> +	return 0;
> +}
> +
> +static int imx_drm_resume(struct device *dev)
> +{
> +	struct drm_device *drm_dev = dev_get_drvdata(dev);
> +	struct drm_connector *connector;
> +
> +	drm_modeset_lock_all(drm_dev);
> +	list_for_each_entry(connector, &drm_dev->mode_config.connector_list, head) {
> +		if (connector->funcs->dpms)
> +			connector->funcs->dpms(connector, DRM_MODE_DPMS_ON);
> +	}
> +	drm_modeset_unlock_all(drm_dev);
> +
This forcefully enables all connectors, which might not have been the
state before suspend. Have a look at simply using
drm_helper_resume_force_mode(), which should do the right thing.

> +	drm_kms_helper_poll_enable(drm_dev);
> +
> +	return 0;
> +}
> +#endif
> +
> +static SIMPLE_DEV_PM_OPS(imx_drm_pm_ops, imx_drm_suspend, imx_drm_resume);
> +
>  static const struct of_device_id imx_drm_dt_ids[] = {
>  	{ .compatible = "fsl,imx-display-subsystem", },
>  	{ /* sentinel */ },
> @@ -708,6 +746,7 @@ static struct platform_driver imx_drm_pdrv = {
>  	.driver		= {
>  		.owner	= THIS_MODULE,
>  		.name	= "imx-drm",
> +		.pm	= &imx_drm_pm_ops,
>  		.of_match_table = imx_drm_dt_ids,
>  	},
>  };

Regards,
Lucas
-- 
Pengutronix e.K.             | Lucas Stach                 |
Industrial Linux Solutions   | http://www.pengutronix.de/  |

[PATCH] imx-drm: imx-drm-core: add suspend/resume support

[Russell King - ARM Linux]

On Thu, Jul 24, 2014 at 11:47:55AM +0200, Lucas Stach wrote:
> Am Donnerstag, den 24.07.2014, 17:17 +0800 schrieb Shawn Guo:
> > HDMI currently stops working after a system suspend/resume cycle.  It
> > turns out that the cause is the imx-hdmi encoder .dpms hook doesn't get
> > called from anywhere across suspend/resume cycle.
> > 
> > The patch follows what exynos drm driver does to walk the list of
> > connectors and call their .dpms function from suspend/resume hook.  And
> > the connectors' .dpms function will in turn filter down to the .dpms
> > hooks of encoders and CRTCs.
> > 
> > With this change, HDMI can continue working after a suspend/resume
> > cycle.
> > 
> > Signed-off-by: Shawn Guo <shawn.guo@freescale.com>
> > ---
> > Tested with HDMI and LVDS.  It'd be great if someone can help test TVE
> > to ensure the patch doesn't break anything.
> > 
> >  drivers/staging/imx-drm/imx-drm-core.c | 39 ++++++++++++++++++++++++++++++++++
> >  1 file changed, 39 insertions(+)
> > 
> > diff --git a/drivers/staging/imx-drm/imx-drm-core.c b/drivers/staging/imx-drm/imx-drm-core.c
> > index def8280d7ee6..b0ea1f0ed32f 100644
> > --- a/drivers/staging/imx-drm/imx-drm-core.c
> > +++ b/drivers/staging/imx-drm/imx-drm-core.c
> > @@ -696,6 +696,44 @@ static int imx_drm_platform_remove(struct platform_device *pdev)
> >  	return 0;
> >  }
> >  
> > +#if CONFIG_PM_SLEEP
> 
> use #ifdef
> 
> > +static int imx_drm_suspend(struct device *dev)
> > +{
> > +	struct drm_device *drm_dev = dev_get_drvdata(dev);
> > +	struct drm_connector *connector;
> > +
> > +	drm_kms_helper_poll_disable(drm_dev);
> > +
> 
> That's ok.
> 
> > +	drm_modeset_lock_all(drm_dev);
> > +	list_for_each_entry(connector, &drm_dev->mode_config.connector_list, head) {
> > +		if (connector->funcs->dpms)
> > +			connector->funcs->dpms(connector, DRM_MODE_DPMS_OFF);
> > +	}
> 
> Don't touch DPMS state here. See below.

In which case, how else does the hardware get placed into a low power
mode on suspend?

DRM has nothing provided, and this is left up to each DRM driver to
implement (probably because it tends to be very driver specific.)
i915 for example calls the CRTC DPMS function with DRM_MODE_DPMS_OFF.

This provides a similar mechanism, but also informs the connector, any
bridge, and encoder associated with the connector as well as the CRTC
to place themselves into a low power mode (which is what
DRM_MODE_DPMS_OFF should be doing anyway.)

> > +static int imx_drm_resume(struct device *dev)
> > +{
> > +	struct drm_device *drm_dev = dev_get_drvdata(dev);
> > +	struct drm_connector *connector;
> > +
> > +	drm_modeset_lock_all(drm_dev);
> > +	list_for_each_entry(connector, &drm_dev->mode_config.connector_list, head) {
> > +		if (connector->funcs->dpms)
> > +			connector->funcs->dpms(connector, DRM_MODE_DPMS_ON);
> > +	}
> > +	drm_modeset_unlock_all(drm_dev);
> > +
> This forcefully enables all connectors, which might not have been the
> state before suspend. Have a look at simply using
> drm_helper_resume_force_mode(), which should do the right thing.

Yes, we agree there.

-- 
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.

[PATCH] imx-drm: imx-drm-core: add suspend/resume support

[Marc Kleine-Budde]

On 07/24/2014 11:47 AM, Lucas Stach wrote:
> Am Donnerstag, den 24.07.2014, 17:17 +0800 schrieb Shawn Guo:
>> HDMI currently stops working after a system suspend/resume cycle.  It
>> turns out that the cause is the imx-hdmi encoder .dpms hook doesn't get
>> called from anywhere across suspend/resume cycle.
>>
>> The patch follows what exynos drm driver does to walk the list of
>> connectors and call their .dpms function from suspend/resume hook.  And
>> the connectors' .dpms function will in turn filter down to the .dpms
>> hooks of encoders and CRTCs.
>>
>> With this change, HDMI can continue working after a suspend/resume
>> cycle.
>>
>> Signed-off-by: Shawn Guo <shawn.guo@freescale.com>
>> ---
>> Tested with HDMI and LVDS.  It'd be great if someone can help test TVE
>> to ensure the patch doesn't break anything.
>>
>>  drivers/staging/imx-drm/imx-drm-core.c | 39 ++++++++++++++++++++++++++++++++++
>>  1 file changed, 39 insertions(+)
>>
>> diff --git a/drivers/staging/imx-drm/imx-drm-core.c b/drivers/staging/imx-drm/imx-drm-core.c
>> index def8280d7ee6..b0ea1f0ed32f 100644
>> --- a/drivers/staging/imx-drm/imx-drm-core.c
>> +++ b/drivers/staging/imx-drm/imx-drm-core.c
>> @@ -696,6 +696,44 @@ static int imx_drm_platform_remove(struct platform_device *pdev)
>>  	return 0;
>>  }
>>  
>> +#if CONFIG_PM_SLEEP
> 
> use #ifdef

...or remove #if/#ifdef and mark as __maybe_unused

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 242 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140724/453066e7/attachment.sig>

[PATCH] imx-drm: imx-drm-core: add suspend/resume support

[Daniel Vetter]

On Thu, Jul 24, 2014 at 10:54:30AM +0100, Russell King - ARM Linux wrote:
> On Thu, Jul 24, 2014 at 11:47:55AM +0200, Lucas Stach wrote:
> > Am Donnerstag, den 24.07.2014, 17:17 +0800 schrieb Shawn Guo:
> > > HDMI currently stops working after a system suspend/resume cycle.  It
> > > turns out that the cause is the imx-hdmi encoder .dpms hook doesn't get
> > > called from anywhere across suspend/resume cycle.
> > > 
> > > The patch follows what exynos drm driver does to walk the list of
> > > connectors and call their .dpms function from suspend/resume hook.  And
> > > the connectors' .dpms function will in turn filter down to the .dpms
> > > hooks of encoders and CRTCs.
> > > 
> > > With this change, HDMI can continue working after a suspend/resume
> > > cycle.
> > > 
> > > Signed-off-by: Shawn Guo <shawn.guo@freescale.com>
> > > ---
> > > Tested with HDMI and LVDS.  It'd be great if someone can help test TVE
> > > to ensure the patch doesn't break anything.
> > > 
> > >  drivers/staging/imx-drm/imx-drm-core.c | 39 ++++++++++++++++++++++++++++++++++
> > >  1 file changed, 39 insertions(+)
> > > 
> > > diff --git a/drivers/staging/imx-drm/imx-drm-core.c b/drivers/staging/imx-drm/imx-drm-core.c
> > > index def8280d7ee6..b0ea1f0ed32f 100644
> > > --- a/drivers/staging/imx-drm/imx-drm-core.c
> > > +++ b/drivers/staging/imx-drm/imx-drm-core.c
> > > @@ -696,6 +696,44 @@ static int imx_drm_platform_remove(struct platform_device *pdev)
> > >  	return 0;
> > >  }
> > >  
> > > +#if CONFIG_PM_SLEEP
> > 
> > use #ifdef
> > 
> > > +static int imx_drm_suspend(struct device *dev)
> > > +{
> > > +	struct drm_device *drm_dev = dev_get_drvdata(dev);
> > > +	struct drm_connector *connector;
> > > +
> > > +	drm_kms_helper_poll_disable(drm_dev);
> > > +
> > 
> > That's ok.
> > 
> > > +	drm_modeset_lock_all(drm_dev);
> > > +	list_for_each_entry(connector, &drm_dev->mode_config.connector_list, head) {
> > > +		if (connector->funcs->dpms)
> > > +			connector->funcs->dpms(connector, DRM_MODE_DPMS_OFF);
> > > +	}
> > 
> > Don't touch DPMS state here. See below.
> 
> In which case, how else does the hardware get placed into a low power
> mode on suspend?
> 
> DRM has nothing provided, and this is left up to each DRM driver to
> implement (probably because it tends to be very driver specific.)
> i915 for example calls the CRTC DPMS function with DRM_MODE_DPMS_OFF.
> 
> This provides a similar mechanism, but also informs the connector, any
> bridge, and encoder associated with the connector as well as the CRTC
> to place themselves into a low power mode (which is what
> DRM_MODE_DPMS_OFF should be doing anyway.)

Well you need to call internal functions to make sure you can restore the
state again. Not sure any more how that all works with the crtc helpers
and whether they restore dpms state properly at all. i915 uses something
completely different nowadays.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH] imx-drm: imx-drm-core: add suspend/resume support

[Shawn Guo]

On Thu, Jul 24, 2014 at 12:38:59PM +0200, Daniel Vetter wrote:
> > > > +static int imx_drm_suspend(struct device *dev)
> > > > +{
> > > > +	struct drm_device *drm_dev = dev_get_drvdata(dev);
> > > > +	struct drm_connector *connector;
> > > > +
> > > > +	drm_kms_helper_poll_disable(drm_dev);
> > > > +
> > > 
> > > That's ok.
> > > 
> > > > +	drm_modeset_lock_all(drm_dev);
> > > > +	list_for_each_entry(connector, &drm_dev->mode_config.connector_list, head) {
> > > > +		if (connector->funcs->dpms)
> > > > +			connector->funcs->dpms(connector, DRM_MODE_DPMS_OFF);
> > > > +	}
> > > 
> > > Don't touch DPMS state here. See below.
> > 
> > In which case, how else does the hardware get placed into a low power
> > mode on suspend?
> > 
> > DRM has nothing provided, and this is left up to each DRM driver to
> > implement (probably because it tends to be very driver specific.)
> > i915 for example calls the CRTC DPMS function with DRM_MODE_DPMS_OFF.
> > 
> > This provides a similar mechanism, but also informs the connector, any
> > bridge, and encoder associated with the connector as well as the CRTC
> > to place themselves into a low power mode (which is what
> > DRM_MODE_DPMS_OFF should be doing anyway.)
> 
> Well you need to call internal functions to make sure you can restore the
> state again. Not sure any more how that all works with the crtc helpers
> and whether they restore dpms state properly at all. i915 uses something
> completely different nowadays.

Is it okay to do what exynos driver does, i.e. saving/restoring the dpms
state before/after calling connector .dpms function?

        list_for_each_entry(connector, &drm_dev->mode_config.connector_list, head) {
                int old_dpms = connector->dpms;

                if (connector->funcs->dpms)
                        connector->funcs->dpms(connector, DRM_MODE_DPMS_OFF);

                /* Set the old mode back to the connector for resume */
                connector->dpms = old_dpms;
        }

Shawn

[PATCH] imx-drm: imx-drm-core: add suspend/resume support

[Shawn Guo]

On Thu, Jul 24, 2014 at 11:47:55AM +0200, Lucas Stach wrote:
> > +static int imx_drm_resume(struct device *dev)
> > +{
> > +	struct drm_device *drm_dev = dev_get_drvdata(dev);
> > +	struct drm_connector *connector;
> > +
> > +	drm_modeset_lock_all(drm_dev);
> > +	list_for_each_entry(connector, &drm_dev->mode_config.connector_list, head) {
> > +		if (connector->funcs->dpms)
> > +			connector->funcs->dpms(connector, DRM_MODE_DPMS_ON);
> > +	}
> > +	drm_modeset_unlock_all(drm_dev);
> > +
> This forcefully enables all connectors, which might not have been the
> state before suspend. Have a look at simply using
> drm_helper_resume_force_mode(), which should do the right thing.

With a bit more testing, I found that simply calling
drm_helper_resume_force_mode() from .resume hook can get HDMI back to
work.  So it seems the issue is not caused by that hdmi .dpms function
isn't being called but some mode setting states gets lost and isn't
being restored.

It can fix the HDMI issue we're facing right now, and I will send V2
with this change.  But it doesn't address Russell's concern, the HDMI
hardware isn't actually put into low power mode since hdmi .dpms
function isn't called anywhere during suspend/resume cycle.

Shawn

[PATCH] imx-drm: imx-drm-core: add suspend/resume support

[Shawn Guo]

On Thu, Jul 24, 2014 at 11:38:52AM +0200, Philipp Zabel wrote:
> Hi Shawn,
> 
> Am Donnerstag, den 24.07.2014, 17:17 +0800 schrieb Shawn Guo:
> > HDMI currently stops working after a system suspend/resume cycle.  It
> > turns out that the cause is the imx-hdmi encoder .dpms hook doesn't get
> > called from anywhere across suspend/resume cycle.
> > 
> > The patch follows what exynos drm driver does to walk the list of
> > connectors and call their .dpms function from suspend/resume hook.  And
> > the connectors' .dpms function will in turn filter down to the .dpms
> > hooks of encoders and CRTCs.
> > 
> > With this change, HDMI can continue working after a suspend/resume
> > cycle.
> > 
> > Signed-off-by: Shawn Guo <shawn.guo@freescale.com>
> > ---
> > Tested with HDMI and LVDS.  It'd be great if someone can help test TVE
> > to ensure the patch doesn't break anything.
> 
> Tested-by: Philipp Zabel <p.zabel@pengutronix.de>
> 
> on i.MX53-QSB with VGA via TVE.

Thanks much, Philipp.

Shawn

[PATCH] imx-drm: imx-drm-core: add suspend/resume support

[Shawn Guo]

On Thu, Jul 24, 2014 at 11:56:32AM +0200, Marc Kleine-Budde wrote:
> >> @@ -696,6 +696,44 @@ static int imx_drm_platform_remove(struct platform_device *pdev)
> >>  	return 0;
> >>  }
> >>  
> >> +#if CONFIG_PM_SLEEP
> > 
> > use #ifdef
> 
> ...or remove #if/#ifdef and mark as __maybe_unused

I personally do not like the idea of __maybe_unused.  Will there be a
compile error if I call some helper functions which are only available
with CONFIG_PM_SLEEP?

Shawn

[PATCH] imx-drm: imx-drm-core: add suspend/resume support

[Daniel Vetter]

On Thu, Jul 24, 2014 at 10:59:36PM +0800, Shawn Guo wrote:
> On Thu, Jul 24, 2014 at 12:38:59PM +0200, Daniel Vetter wrote:
> > > > > +static int imx_drm_suspend(struct device *dev)
> > > > > +{
> > > > > +	struct drm_device *drm_dev = dev_get_drvdata(dev);
> > > > > +	struct drm_connector *connector;
> > > > > +
> > > > > +	drm_kms_helper_poll_disable(drm_dev);
> > > > > +
> > > > 
> > > > That's ok.
> > > > 
> > > > > +	drm_modeset_lock_all(drm_dev);
> > > > > +	list_for_each_entry(connector, &drm_dev->mode_config.connector_list, head) {
> > > > > +		if (connector->funcs->dpms)
> > > > > +			connector->funcs->dpms(connector, DRM_MODE_DPMS_OFF);
> > > > > +	}
> > > > 
> > > > Don't touch DPMS state here. See below.
> > > 
> > > In which case, how else does the hardware get placed into a low power
> > > mode on suspend?
> > > 
> > > DRM has nothing provided, and this is left up to each DRM driver to
> > > implement (probably because it tends to be very driver specific.)
> > > i915 for example calls the CRTC DPMS function with DRM_MODE_DPMS_OFF.
> > > 
> > > This provides a similar mechanism, but also informs the connector, any
> > > bridge, and encoder associated with the connector as well as the CRTC
> > > to place themselves into a low power mode (which is what
> > > DRM_MODE_DPMS_OFF should be doing anyway.)
> > 
> > Well you need to call internal functions to make sure you can restore the
> > state again. Not sure any more how that all works with the crtc helpers
> > and whether they restore dpms state properly at all. i915 uses something
> > completely different nowadays.
> 
> Is it okay to do what exynos driver does, i.e. saving/restoring the dpms
> state before/after calling connector .dpms function?
> 
>         list_for_each_entry(connector, &drm_dev->mode_config.connector_list, head) {
>                 int old_dpms = connector->dpms;
> 
>                 if (connector->funcs->dpms)
>                         connector->funcs->dpms(connector, DRM_MODE_DPMS_OFF);
> 
>                 /* Set the old mode back to the connector for resume */
>                 connector->dpms = old_dpms;
>         }

I'm not sure whether that will get you correct behavior since iirc the
crtc helpers aren't terribly good at restoring the right dpms state.
drm_helper_resume_force_mode simple does a modeset, and that has an
implicit dpms on when enabling the crtc. Someone might want to tackle
this, but thus far it doesn't seem to have been too annoying. I think the
best way would be to do this as part of atomic modeset, where we can
supply the desired dpms state directly.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch