All of lore.kernel.org
 help / color / mirror / Atom feed
From: daniel@ffwll.ch (Daniel Vetter)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] imx-drm: imx-drm-core: add suspend/resume support
Date: Thu, 24 Jul 2014 12:38:59 +0200	[thread overview]
Message-ID: <20140724103859.GS15237@phenom.ffwll.local> (raw)
In-Reply-To: <20140724095430.GM21766@n2100.arm.linux.org.uk>

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

WARNING: multiple messages have this Message-ID (diff)
From: Daniel Vetter <daniel@ffwll.ch>
To: Russell King - ARM Linux <linux@arm.linux.org.uk>
Cc: Shawn Guo <shawn.guo@freescale.com>,
	linux-arm-kernel@lists.infradead.org,
	dri-devel@lists.freedesktop.org, kernel@pengutronix.de
Subject: Re: [PATCH] imx-drm: imx-drm-core: add suspend/resume support
Date: Thu, 24 Jul 2014 12:38:59 +0200	[thread overview]
Message-ID: <20140724103859.GS15237@phenom.ffwll.local> (raw)
In-Reply-To: <20140724095430.GM21766@n2100.arm.linux.org.uk>

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

  reply	other threads:[~2014-07-24 10:38 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-07-24  9:17 [PATCH] imx-drm: imx-drm-core: add suspend/resume support Shawn Guo
2014-07-24  9:17 ` Shawn Guo
2014-07-24  9:38 ` Philipp Zabel
2014-07-24  9:38   ` Philipp Zabel
2014-07-25  6:31   ` Shawn Guo
2014-07-25  6:31     ` Shawn Guo
2014-07-24  9:47 ` Lucas Stach
2014-07-24  9:47   ` Lucas Stach
2014-07-24  9:54   ` Russell King - ARM Linux
2014-07-24  9:54     ` Russell King - ARM Linux
2014-07-24 10:38     ` Daniel Vetter [this message]
2014-07-24 10:38       ` Daniel Vetter
2014-07-24 14:59       ` Shawn Guo
2014-07-24 14:59         ` Shawn Guo
2014-07-25  8:20         ` Daniel Vetter
2014-07-25  8:20           ` Daniel Vetter
2014-07-24  9:56   ` Marc Kleine-Budde
2014-07-24  9:56     ` Marc Kleine-Budde
2014-07-25  6:34     ` Shawn Guo
2014-07-25  6:34       ` Shawn Guo
2014-07-25  5:46   ` Shawn Guo
2014-07-25  5:46     ` Shawn Guo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140724103859.GS15237@phenom.ffwll.local \
    --to=daniel@ffwll.ch \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.