This series depends on Boris' "[PATCH v2] drm: atmel-hlcdc: Atomic
mode-setting conversion"
<1423236143-6494-1-git-send-email-boris.brezillon@free-electrons.com>
plus a few fixes which are going to be in v3 of Boris' patch.
This series adds basic PM support for Atmel HLCDC.
Sylvain Rochet (2):
drm: atmel-hlcdc: Add PM suspend/resume support
drm: atmel-hlcdc: Add pinctrl PM select sleep,default state in CRTC
suspend/resume
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 3 ++
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 60 ++++++++++++++++++++++++++
2 files changed, 63 insertions(+)
--
2.1.4
On suspend: switch off CRTC if not already suspended with runtime PM
On resume: switch on CRTC if we were not already suspended from runtime
PM while suspending.
Signed-off-by: Sylvain Rochet <[email protected]>
---
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 60 ++++++++++++++++++++++++++++
1 file changed, 60 insertions(+)
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
index 22c3cca..4c89bd2 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
@@ -427,6 +427,36 @@ static void atmel_hlcdc_dc_lastclose(struct drm_device *dev)
drm_fbdev_cma_restore_mode(dc->fbdev);
}
+static int atmel_hlcdc_dc_suspend(struct drm_device *dev, pm_message_t state)
+{
+ struct drm_crtc *crtc;
+
+ drm_modeset_lock_all(dev);
+
+ list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
+ struct drm_crtc_helper_funcs *crtc_funcs = crtc->helper_private;
+ (*crtc_funcs->disable)(crtc);
+ }
+
+ drm_modeset_unlock_all(dev);
+ return 0;
+}
+
+static int atmel_hlcdc_dc_resume(struct drm_device *dev)
+{
+ struct drm_crtc *crtc;
+
+ drm_modeset_lock_all(dev);
+
+ list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
+ struct drm_crtc_helper_funcs *crtc_funcs = crtc->helper_private;
+ (*crtc_funcs->enable)(crtc);
+ }
+
+ drm_modeset_unlock_all(dev);
+ return 0;
+}
+
static int atmel_hlcdc_dc_irq_postinstall(struct drm_device *dev)
{
struct atmel_hlcdc_dc *dc = dev->dev_private;
@@ -488,6 +518,8 @@ static struct drm_driver atmel_hlcdc_dc_driver = {
.driver_features = DRIVER_HAVE_IRQ | DRIVER_GEM | DRIVER_MODESET,
.preclose = atmel_hlcdc_dc_preclose,
.lastclose = atmel_hlcdc_dc_lastclose,
+ .suspend = atmel_hlcdc_dc_suspend,
+ .resume = atmel_hlcdc_dc_resume,
.irq_handler = atmel_hlcdc_dc_irq_handler,
.irq_preinstall = atmel_hlcdc_dc_irq_uninstall,
.irq_postinstall = atmel_hlcdc_dc_irq_postinstall,
@@ -559,6 +591,33 @@ static int atmel_hlcdc_dc_drm_remove(struct platform_device *pdev)
return 0;
}
+#ifdef CONFIG_PM
+static int atmel_hlcdc_dc_drm_suspend(struct device *dev)
+{
+ struct drm_device *drm_dev = dev_get_drvdata(dev);
+ pm_message_t message;
+
+ if (pm_runtime_suspended(dev) || !drm_dev)
+ return 0;
+
+ message.event = PM_EVENT_SUSPEND;
+ return atmel_hlcdc_dc_suspend(drm_dev, message);
+}
+
+static int atmel_hlcdc_dc_drm_resume(struct device *dev)
+{
+ struct drm_device *drm_dev = dev_get_drvdata(dev);
+
+ if (pm_runtime_suspended(dev) || !drm_dev)
+ return 0;
+
+ return atmel_hlcdc_dc_resume(drm_dev);
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(atmel_hlcdc_dc_drm_pm_ops,
+ atmel_hlcdc_dc_drm_suspend, atmel_hlcdc_dc_drm_resume);
+
static const struct of_device_id atmel_hlcdc_dc_of_match[] = {
{ .compatible = "atmel,hlcdc-display-controller" },
{ },
@@ -569,6 +628,7 @@ static struct platform_driver atmel_hlcdc_dc_platform_driver = {
.remove = atmel_hlcdc_dc_drm_remove,
.driver = {
.name = "atmel-hlcdc-display-controller",
+ .pm = &atmel_hlcdc_dc_drm_pm_ops,
.of_match_table = atmel_hlcdc_dc_of_match,
},
};
--
2.1.4
Some LCD panels have back-powering issue when un-powered, allows users
to use an alternate pinctrl "sleep" in order to clamp outputs to a
wanted state at suspend.
Signed-off-by: Sylvain Rochet <[email protected]>
---
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
index 79c9a19..decd9ea 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
@@ -21,6 +21,7 @@
#include <linux/clk.h>
#include <linux/pm.h>
#include <linux/pm_runtime.h>
+#include <linux/pinctrl/consumer.h>
#include <drm/drm_crtc.h>
#include <drm/drm_crtc_helper.h>
@@ -156,6 +157,7 @@ static void atmel_hlcdc_crtc_disable(struct drm_crtc *c)
cpu_relax();
clk_disable_unprepare(crtc->dc->hlcdc->sys_clk);
+ pinctrl_pm_select_sleep_state(dev->dev);
pm_runtime_allow(dev->dev);
@@ -178,6 +180,7 @@ static void atmel_hlcdc_crtc_enable(struct drm_crtc *c)
pm_runtime_forbid(dev->dev);
+ pinctrl_pm_select_default_state(dev->dev);
clk_prepare_enable(crtc->dc->hlcdc->sys_clk);
regmap_write(regmap, ATMEL_HLCDC_EN, ATMEL_HLCDC_PIXEL_CLK);
--
2.1.4
Hi Sylvain,
On Tue, 10 Feb 2015 14:40:45 +0100
Sylvain Rochet <[email protected]> wrote:
> On suspend: switch off CRTC if not already suspended with runtime PM
>
> On resume: switch on CRTC if we were not already suspended from runtime
> PM while suspending.
>
> Signed-off-by: Sylvain Rochet <[email protected]>
> ---
> drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 60 ++++++++++++++++++++++++++++
> 1 file changed, 60 insertions(+)
>
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> index 22c3cca..4c89bd2 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> @@ -427,6 +427,36 @@ static void atmel_hlcdc_dc_lastclose(struct drm_device *dev)
> drm_fbdev_cma_restore_mode(dc->fbdev);
> }
>
> +static int atmel_hlcdc_dc_suspend(struct drm_device *dev, pm_message_t state)
> +{
> + struct drm_crtc *crtc;
> +
> + drm_modeset_lock_all(dev);
> +
> + list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
> + struct drm_crtc_helper_funcs *crtc_funcs = crtc->helper_private;
> + (*crtc_funcs->disable)(crtc);
Please replace that line by:
crtc_funcs->disable(crtc);
> + }
> +
> + drm_modeset_unlock_all(dev);
> + return 0;
> +}
> +
> +static int atmel_hlcdc_dc_resume(struct drm_device *dev)
> +{
> + struct drm_crtc *crtc;
> +
> + drm_modeset_lock_all(dev);
> +
> + list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
> + struct drm_crtc_helper_funcs *crtc_funcs = crtc->helper_private;
> + (*crtc_funcs->enable)(crtc);
Ditto:
crtc_funcs->enable(crtc);
> + }
> +
> + drm_modeset_unlock_all(dev);
> + return 0;
> +}
> +
> static int atmel_hlcdc_dc_irq_postinstall(struct drm_device *dev)
> {
> struct atmel_hlcdc_dc *dc = dev->dev_private;
> @@ -488,6 +518,8 @@ static struct drm_driver atmel_hlcdc_dc_driver = {
> .driver_features = DRIVER_HAVE_IRQ | DRIVER_GEM | DRIVER_MODESET,
> .preclose = atmel_hlcdc_dc_preclose,
> .lastclose = atmel_hlcdc_dc_lastclose,
> + .suspend = atmel_hlcdc_dc_suspend,
> + .resume = atmel_hlcdc_dc_resume,
> .irq_handler = atmel_hlcdc_dc_irq_handler,
> .irq_preinstall = atmel_hlcdc_dc_irq_uninstall,
> .irq_postinstall = atmel_hlcdc_dc_irq_postinstall,
> @@ -559,6 +591,33 @@ static int atmel_hlcdc_dc_drm_remove(struct platform_device *pdev)
> return 0;
> }
>
> +#ifdef CONFIG_PM
> +static int atmel_hlcdc_dc_drm_suspend(struct device *dev)
> +{
> + struct drm_device *drm_dev = dev_get_drvdata(dev);
> + pm_message_t message;
> +
> + if (pm_runtime_suspended(dev) || !drm_dev)
> + return 0;
> +
> + message.event = PM_EVENT_SUSPEND;
> + return atmel_hlcdc_dc_suspend(drm_dev, message);
> +}
> +
> +static int atmel_hlcdc_dc_drm_resume(struct device *dev)
> +{
> + struct drm_device *drm_dev = dev_get_drvdata(dev);
> +
> + if (pm_runtime_suspended(dev) || !drm_dev)
> + return 0;
> +
> + return atmel_hlcdc_dc_resume(drm_dev);
> +}
> +#endif
> +
> +static SIMPLE_DEV_PM_OPS(atmel_hlcdc_dc_drm_pm_ops,
> + atmel_hlcdc_dc_drm_suspend, atmel_hlcdc_dc_drm_resume);
> +
Do we really need to register both SIMPLE_DEV_PM_OPS and drm_driver
suspend/resume functions.
I thought the suspend/resume callbacks were called by DRM core code
as part of the PM class specific operations.
However, exynos driver seems to do the same, so this might well be
required.
Best Regards,
Boris
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
On Tue, 10 Feb 2015 14:40:46 +0100
Sylvain Rochet <[email protected]> wrote:
> Some LCD panels have back-powering issue when un-powered, allows users
> to use an alternate pinctrl "sleep" in order to clamp outputs to a
> wanted state at suspend.
>
> Signed-off-by: Sylvain Rochet <[email protected]>
Acked-by: Boris Brezillon <[email protected]>
> ---
> drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> index 79c9a19..decd9ea 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> @@ -21,6 +21,7 @@
> #include <linux/clk.h>
> #include <linux/pm.h>
> #include <linux/pm_runtime.h>
> +#include <linux/pinctrl/consumer.h>
>
> #include <drm/drm_crtc.h>
> #include <drm/drm_crtc_helper.h>
> @@ -156,6 +157,7 @@ static void atmel_hlcdc_crtc_disable(struct drm_crtc *c)
> cpu_relax();
>
> clk_disable_unprepare(crtc->dc->hlcdc->sys_clk);
> + pinctrl_pm_select_sleep_state(dev->dev);
>
> pm_runtime_allow(dev->dev);
>
> @@ -178,6 +180,7 @@ static void atmel_hlcdc_crtc_enable(struct drm_crtc *c)
>
> pm_runtime_forbid(dev->dev);
>
> + pinctrl_pm_select_default_state(dev->dev);
> clk_prepare_enable(crtc->dc->hlcdc->sys_clk);
>
> regmap_write(regmap, ATMEL_HLCDC_EN, ATMEL_HLCDC_PIXEL_CLK);
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
Hello Boris,
On Tue, Feb 10, 2015 at 03:05:57PM +0100, Boris Brezillon wrote:
> On Tue, 10 Feb 2015 14:40:45 +0100
> Sylvain Rochet <[email protected]> wrote:
> > +
> > +static SIMPLE_DEV_PM_OPS(atmel_hlcdc_dc_drm_pm_ops,
> > + atmel_hlcdc_dc_drm_suspend, atmel_hlcdc_dc_drm_resume);
> > +
>
> Do we really need to register both SIMPLE_DEV_PM_OPS and drm_driver
> suspend/resume functions.
> I thought the suspend/resume callbacks were called by DRM core code
> as part of the PM class specific operations.
> However, exynos driver seems to do the same, so this might well be
> required.
Looks like we need, if I comment out SIMPLE_DEV_PM_OPS ops registration
we don't suspend or resume anymore. I guess drm_driver suspend/resume
functions are only used for PM runtime support, which we don't support
yet, at least we added the basic requirements to add PM runtime support
at some point in the future.
Anyway, even without PM runtime support we need to suspend and resume
this device, other DRM drivers do this as well.
Sylvain