[PATCH v2 0/6] drm/fsl-dcu: suspend/resume rework using atomic helpers

[PATCH v2 0/6] drm/fsl-dcu: suspend/resume rework using atomic helpers

[Stefan Agner]

This implements suspend/resume using the atomic update supsend/resume
helpers instead of the current implementation which uses regcache. The
code has been tested on a Colibri VF61 using the freeze suspend mode.

This version is a rebase ontop of the fix for the regmap cache issue:
https://lists.freedesktop.org/archives/dri-devel/2016-June/109625.html

Dave, Thierry: Could you have a look at patch 1? If that looks good
for you I will take it through my tree...

Changes since v1:
- Rebase ontop of drm-next + regmap cache fix

Stefan Agner (6):
  drm/fb_cma_helper: add suspend helper
  drm/fsl-dcu: store layer registers in soc_data
  drm/fsl-dcu: move layer initialization to plane file
  drm/fsl-dcu: use clk helpers
  drm/fsl-dcu: implement suspend/resume using atomic helpers
  drm/fsl-dcu: disable vblank events on CRTC disable

 drivers/gpu/drm/drm_fb_cma_helper.c         | 15 +++++++++
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c  | 21 ++++---------
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c   | 47 +++++++++++++++++++++--------
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h   |  2 ++
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c | 16 ++++++++++
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.h |  1 +
 include/drm/drm_fb_cma_helper.h             |  1 +
 7 files changed, 76 insertions(+), 27 deletions(-)

-- 
2.8.2

[PATCH v2 1/6] drm/fb_cma_helper: add suspend helper

[Stefan Agner]

Implement a suspend/resume helper for CMA users which calls
drm_fb_helper_set_suspend.

Suggested-by: Thierry Reding <thierry.reding@gmail.com>
Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 drivers/gpu/drm/drm_fb_cma_helper.c | 15 +++++++++++++++
 include/drm/drm_fb_cma_helper.h     |  1 +
 2 files changed, 16 insertions(+)

diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c
index 172cafe..6948d82 100644
--- a/drivers/gpu/drm/drm_fb_cma_helper.c
+++ b/drivers/gpu/drm/drm_fb_cma_helper.c
@@ -582,3 +582,18 @@ void drm_fbdev_cma_hotplug_event(struct drm_fbdev_cma *fbdev_cma)
 		drm_fb_helper_hotplug_event(&fbdev_cma->fb_helper);
 }
 EXPORT_SYMBOL_GPL(drm_fbdev_cma_hotplug_event);
+
+/**
+ * drm_fbdev_cma_set_suspend - wrapper around drm_fb_helper_set_suspend
+ * @fbdev_cma: The drm_fbdev_cma struct, may be NULL
+ * @state: desired state, zero to resume, non-zero to suspend
+ *
+ * Calls drm_fb_helper_set_suspend, which is a wrapper around
+ * fb_set_suspend implemented by fbdev core.
+ */
+void drm_fbdev_cma_set_suspend(struct drm_fbdev_cma *fbdev_cma, int state)
+{
+	if (fbdev_cma)
+		drm_fb_helper_set_suspend(&fbdev_cma->fb_helper, state);
+}
+EXPORT_SYMBOL(drm_fbdev_cma_set_suspend);
diff --git a/include/drm/drm_fb_cma_helper.h b/include/drm/drm_fb_cma_helper.h
index fd0dde9..f313211 100644
--- a/include/drm/drm_fb_cma_helper.h
+++ b/include/drm/drm_fb_cma_helper.h
@@ -23,6 +23,7 @@ void drm_fbdev_cma_fini(struct drm_fbdev_cma *fbdev_cma);
 
 void drm_fbdev_cma_restore_mode(struct drm_fbdev_cma *fbdev_cma);
 void drm_fbdev_cma_hotplug_event(struct drm_fbdev_cma *fbdev_cma);
+void drm_fbdev_cma_set_suspend(struct drm_fbdev_cma *fbdev_cma, int state);
 int drm_fbdev_cma_create_with_funcs(struct drm_fb_helper *helper,
 	struct drm_fb_helper_surface_size *sizes,
 	const struct drm_framebuffer_funcs *funcs);
-- 
2.8.2

Re: [PATCH v2 1/6] drm/fb_cma_helper: add suspend helper

[Daniel Vetter]

On Fri, Jun 03, 2016 at 03:56:39PM -0700, Stefan Agner wrote:
> Implement a suspend/resume helper for CMA users which calls
> drm_fb_helper_set_suspend.
> 
> Suggested-by: Thierry Reding <thierry.reding@gmail.com>
> Signed-off-by: Stefan Agner <stefan@agner.ch>

Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>

... for merging through fsl-du tree. Just add a small note about it in
your pull request.
-Daniel

> ---
>  drivers/gpu/drm/drm_fb_cma_helper.c | 15 +++++++++++++++
>  include/drm/drm_fb_cma_helper.h     |  1 +
>  2 files changed, 16 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c
> index 172cafe..6948d82 100644
> --- a/drivers/gpu/drm/drm_fb_cma_helper.c
> +++ b/drivers/gpu/drm/drm_fb_cma_helper.c
> @@ -582,3 +582,18 @@ void drm_fbdev_cma_hotplug_event(struct drm_fbdev_cma *fbdev_cma)
>  		drm_fb_helper_hotplug_event(&fbdev_cma->fb_helper);
>  }
>  EXPORT_SYMBOL_GPL(drm_fbdev_cma_hotplug_event);
> +
> +/**
> + * drm_fbdev_cma_set_suspend - wrapper around drm_fb_helper_set_suspend
> + * @fbdev_cma: The drm_fbdev_cma struct, may be NULL
> + * @state: desired state, zero to resume, non-zero to suspend
> + *
> + * Calls drm_fb_helper_set_suspend, which is a wrapper around
> + * fb_set_suspend implemented by fbdev core.
> + */
> +void drm_fbdev_cma_set_suspend(struct drm_fbdev_cma *fbdev_cma, int state)
> +{
> +	if (fbdev_cma)
> +		drm_fb_helper_set_suspend(&fbdev_cma->fb_helper, state);
> +}
> +EXPORT_SYMBOL(drm_fbdev_cma_set_suspend);
> diff --git a/include/drm/drm_fb_cma_helper.h b/include/drm/drm_fb_cma_helper.h
> index fd0dde9..f313211 100644
> --- a/include/drm/drm_fb_cma_helper.h
> +++ b/include/drm/drm_fb_cma_helper.h
> @@ -23,6 +23,7 @@ void drm_fbdev_cma_fini(struct drm_fbdev_cma *fbdev_cma);
>  
>  void drm_fbdev_cma_restore_mode(struct drm_fbdev_cma *fbdev_cma);
>  void drm_fbdev_cma_hotplug_event(struct drm_fbdev_cma *fbdev_cma);
> +void drm_fbdev_cma_set_suspend(struct drm_fbdev_cma *fbdev_cma, int state);
>  int drm_fbdev_cma_create_with_funcs(struct drm_fb_helper *helper,
>  	struct drm_fb_helper_surface_size *sizes,
>  	const struct drm_framebuffer_funcs *funcs);
> -- 
> 2.8.2
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[PATCH v2 2/6] drm/fsl-dcu: store layer registers in soc_data

[Stefan Agner]

Store the number of registers per layer in soc_data. This is
more consistent with how the rest of SoC specific data are
handled.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c | 8 ++------
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c  | 2 ++
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h  | 1 +
 3 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
index 89c0084..b024f90 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
@@ -138,7 +138,7 @@ int fsl_dcu_drm_crtc_create(struct fsl_dcu_drm_device *fsl_dev)
 {
 	struct drm_plane *primary;
 	struct drm_crtc *crtc = &fsl_dev->crtc;
-	unsigned int i, j, reg_num;
+	unsigned int i, j;
 	int ret;
 
 	primary = fsl_dcu_drm_primary_create_plane(fsl_dev->drm);
@@ -154,12 +154,8 @@ int fsl_dcu_drm_crtc_create(struct fsl_dcu_drm_device *fsl_dev)
 
 	drm_crtc_helper_add(crtc, &fsl_dcu_drm_crtc_helper_funcs);
 
-	if (!strcmp(fsl_dev->soc->name, "ls1021a"))
-		reg_num = LS1021A_LAYER_REG_NUM;
-	else
-		reg_num = VF610_LAYER_REG_NUM;
 	for (i = 0; i < fsl_dev->soc->total_layer; i++) {
-		for (j = 1; j <= reg_num; j++)
+		for (j = 1; j <= fsl_dev->soc->layer_regs; j++)
 			regmap_write(fsl_dev->regmap, DCU_CTRLDESCLN(i, j), 0);
 	}
 	regmap_update_bits(fsl_dev->regmap, DCU_DCU_MODE,
diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
index dc723f7..9daca1f 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
@@ -274,12 +274,14 @@ static const struct fsl_dcu_soc_data fsl_dcu_ls1021a_data = {
 	.name = "ls1021a",
 	.total_layer = 16,
 	.max_layer = 4,
+	.layer_regs = LS1021A_LAYER_REG_NUM,
 };
 
 static const struct fsl_dcu_soc_data fsl_dcu_vf610_data = {
 	.name = "vf610",
 	.total_layer = 64,
 	.max_layer = 6,
+	.layer_regs = VF610_LAYER_REG_NUM,
 };
 
 static const struct of_device_id fsl_dcu_of_match[] = {
diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h
index c275f90..b1bba3a 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h
@@ -175,6 +175,7 @@ struct fsl_dcu_soc_data {
 	unsigned int total_layer;
 	/*max layer number DCU supported*/
 	unsigned int max_layer;
+	unsigned int layer_regs;
 };
 
 struct fsl_dcu_drm_device {
-- 
2.8.2

[PATCH v2 3/6] drm/fsl-dcu: move layer initialization to plane file

[Stefan Agner]

Move the initialization code for layers into a separate function
in the plane file. This allows to reuse the function on resume.
Also move it at the very beginning which may not matter but makes
logically much more sense.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c  | 13 ++-----------
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c | 16 ++++++++++++++++
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.h |  1 +
 3 files changed, 19 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
index b024f90..ca0f7d83 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
@@ -138,9 +138,10 @@ int fsl_dcu_drm_crtc_create(struct fsl_dcu_drm_device *fsl_dev)
 {
 	struct drm_plane *primary;
 	struct drm_crtc *crtc = &fsl_dev->crtc;
-	unsigned int i, j;
 	int ret;
 
+	fsl_dcu_drm_init_planes(fsl_dev->drm);
+
 	primary = fsl_dcu_drm_primary_create_plane(fsl_dev->drm);
 	if (!primary)
 		return -ENOMEM;
@@ -154,15 +155,5 @@ int fsl_dcu_drm_crtc_create(struct fsl_dcu_drm_device *fsl_dev)
 
 	drm_crtc_helper_add(crtc, &fsl_dcu_drm_crtc_helper_funcs);
 
-	for (i = 0; i < fsl_dev->soc->total_layer; i++) {
-		for (j = 1; j <= fsl_dev->soc->layer_regs; j++)
-			regmap_write(fsl_dev->regmap, DCU_CTRLDESCLN(i, j), 0);
-	}
-	regmap_update_bits(fsl_dev->regmap, DCU_DCU_MODE,
-			   DCU_MODE_DCU_MODE_MASK,
-			   DCU_MODE_DCU_MODE(DCU_MODE_OFF));
-	regmap_write(fsl_dev->regmap, DCU_UPDATE_MODE,
-		     DCU_UPDATE_MODE_READREG);
-
 	return 0;
 }
diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
index 274558b..e50467a 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
@@ -217,6 +217,22 @@ static const u32 fsl_dcu_drm_plane_formats[] = {
 	DRM_FORMAT_YUV422,
 };
 
+void fsl_dcu_drm_init_planes(struct drm_device *dev)
+{
+	struct fsl_dcu_drm_device *fsl_dev = dev->dev_private;
+	int i, j;
+
+	for (i = 0; i < fsl_dev->soc->total_layer; i++) {
+		for (j = 1; j <= fsl_dev->soc->layer_regs; j++)
+			regmap_write(fsl_dev->regmap, DCU_CTRLDESCLN(i, j), 0);
+	}
+	regmap_update_bits(fsl_dev->regmap, DCU_DCU_MODE,
+			   DCU_MODE_DCU_MODE_MASK,
+			   DCU_MODE_DCU_MODE(DCU_MODE_OFF));
+	regmap_write(fsl_dev->regmap, DCU_UPDATE_MODE,
+		     DCU_UPDATE_MODE_READREG);
+}
+
 struct drm_plane *fsl_dcu_drm_primary_create_plane(struct drm_device *dev)
 {
 	struct drm_plane *primary;
diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.h b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.h
index d657f08..8ee45f8 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.h
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.h
@@ -12,6 +12,7 @@
 #ifndef __FSL_DCU_DRM_PLANE_H__
 #define __FSL_DCU_DRM_PLANE_H__
 
+void fsl_dcu_drm_init_planes(struct drm_device *dev);
 struct drm_plane *fsl_dcu_drm_primary_create_plane(struct drm_device *dev);
 
 #endif /* __FSL_DCU_DRM_PLANE_H__ */
-- 
2.8.2

[PATCH v2 4/6] drm/fsl-dcu: use clk helpers

[Stefan Agner]

Use clk_prepare_enable and clk_disable_unprepare helpers. This also
fixes a sequence issue in the enable path which lead to a warning
on resume.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
index 9daca1f..06a4d01 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
@@ -232,8 +232,7 @@ static int fsl_dcu_drm_pm_suspend(struct device *dev)
 	drm_kms_helper_poll_disable(fsl_dev->drm);
 	regcache_cache_only(fsl_dev->regmap, true);
 	regcache_mark_dirty(fsl_dev->regmap);
-	clk_disable(fsl_dev->clk);
-	clk_unprepare(fsl_dev->clk);
+	clk_disable_unprepare(fsl_dev->clk);
 
 	return 0;
 }
@@ -246,15 +245,9 @@ static int fsl_dcu_drm_pm_resume(struct device *dev)
 	if (!fsl_dev)
 		return 0;
 
-	ret = clk_enable(fsl_dev->clk);
+	ret = clk_prepare_enable(fsl_dev->clk);
 	if (ret < 0) {
 		dev_err(dev, "failed to enable dcu clk\n");
-		clk_unprepare(fsl_dev->clk);
-		return ret;
-	}
-	ret = clk_prepare(fsl_dev->clk);
-	if (ret < 0) {
-		dev_err(dev, "failed to prepare dcu clk\n");
 		return ret;
 	}
 
-- 
2.8.2

[PATCH v2 5/6] drm/fsl-dcu: implement suspend/resume using atomic helpers

[Stefan Agner]

Use the drm_atomic_helper_suspend() and drm_atomic_helper_resume()
helpers to implement subsystem-level suspend/resume. This replaces
the (non-functional) regmap cache based suspend resume functionality.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 40 ++++++++++++++++++++++++++-----
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h |  1 +
 2 files changed, 35 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
index 06a4d01..2bc4fa2 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
@@ -11,6 +11,7 @@
 
 #include <linux/clk.h>
 #include <linux/clk-provider.h>
+#include <linux/console.h>
 #include <linux/io.h>
 #include <linux/mfd/syscon.h>
 #include <linux/mm.h>
@@ -22,6 +23,7 @@
 #include <linux/regmap.h>
 
 #include <drm/drmP.h>
+#include <drm/drm_atomic_helper.h>
 #include <drm/drm_crtc_helper.h>
 #include <drm/drm_fb_cma_helper.h>
 #include <drm/drm_gem_cma_helper.h>
@@ -42,10 +44,8 @@ static const struct regmap_config fsl_dcu_regmap_config = {
 	.reg_bits = 32,
 	.reg_stride = 4,
 	.val_bits = 32,
-	.cache_type = REGCACHE_FLAT,
 
 	.volatile_reg = fsl_dcu_drm_is_volatile_reg,
-	.max_register = 0x11fc,
 };
 
 static int fsl_dcu_drm_irq_init(struct drm_device *dev)
@@ -229,9 +229,25 @@ static int fsl_dcu_drm_pm_suspend(struct device *dev)
 	if (!fsl_dev)
 		return 0;
 
+	disable_irq(fsl_dev->irq);
 	drm_kms_helper_poll_disable(fsl_dev->drm);
-	regcache_cache_only(fsl_dev->regmap, true);
-	regcache_mark_dirty(fsl_dev->regmap);
+
+	console_lock();
+	drm_fbdev_cma_set_suspend(fsl_dev->fbdev, 1);
+	console_unlock();
+
+	fsl_dev->state = drm_atomic_helper_suspend(fsl_dev->drm);
+	if (IS_ERR(fsl_dev->state)) {
+		console_lock();
+		drm_fbdev_cma_set_suspend(fsl_dev->fbdev, 0);
+		console_unlock();
+
+		drm_kms_helper_poll_enable(fsl_dev->drm);
+		enable_irq(fsl_dev->irq);
+		return PTR_ERR(fsl_dev->state);
+	}
+
+	clk_disable_unprepare(fsl_dev->pix_clk);
 	clk_disable_unprepare(fsl_dev->clk);
 
 	return 0;
@@ -251,9 +267,21 @@ static int fsl_dcu_drm_pm_resume(struct device *dev)
 		return ret;
 	}
 
+	ret = clk_prepare_enable(fsl_dev->pix_clk);
+	if (ret < 0) {
+		dev_err(dev, "failed to enable pix clk\n");
+		return ret;
+	}
+
+	fsl_dcu_drm_init_planes(fsl_dev->drm);
+	drm_atomic_helper_resume(fsl_dev->drm, fsl_dev->state);
+
+	console_lock();
+	drm_fbdev_cma_set_suspend(fsl_dev->fbdev, 0);
+	console_unlock();
+
 	drm_kms_helper_poll_enable(fsl_dev->drm);
-	regcache_cache_only(fsl_dev->regmap, false);
-	regcache_sync(fsl_dev->regmap);
+	enable_irq(fsl_dev->irq);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h
index b1bba3a..3b371fe7 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h
@@ -194,6 +194,7 @@ struct fsl_dcu_drm_device {
 	struct drm_encoder encoder;
 	struct fsl_dcu_drm_connector connector;
 	const struct fsl_dcu_soc_data *soc;
+	struct drm_atomic_state *state;
 };
 
 void fsl_dcu_fbdev_init(struct drm_device *dev);
-- 
2.8.2

Re: [PATCH v2 5/6] drm/fsl-dcu: implement suspend/resume using atomic helpers

[Daniel Vetter]

On Fri, Jun 03, 2016 at 03:56:43PM -0700, Stefan Agner wrote:
> Use the drm_atomic_helper_suspend() and drm_atomic_helper_resume()
> helpers to implement subsystem-level suspend/resume. This replaces
> the (non-functional) regmap cache based suspend resume functionality.
> 
> Signed-off-by: Stefan Agner <stefan@agner.ch>
> ---
>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 40 ++++++++++++++++++++++++++-----
>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h |  1 +
>  2 files changed, 35 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
> index 06a4d01..2bc4fa2 100644
> --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
> +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
> @@ -11,6 +11,7 @@
>  
>  #include <linux/clk.h>
>  #include <linux/clk-provider.h>
> +#include <linux/console.h>
>  #include <linux/io.h>
>  #include <linux/mfd/syscon.h>
>  #include <linux/mm.h>
> @@ -22,6 +23,7 @@
>  #include <linux/regmap.h>
>  
>  #include <drm/drmP.h>
> +#include <drm/drm_atomic_helper.h>
>  #include <drm/drm_crtc_helper.h>
>  #include <drm/drm_fb_cma_helper.h>
>  #include <drm/drm_gem_cma_helper.h>
> @@ -42,10 +44,8 @@ static const struct regmap_config fsl_dcu_regmap_config = {
>  	.reg_bits = 32,
>  	.reg_stride = 4,
>  	.val_bits = 32,
> -	.cache_type = REGCACHE_FLAT,
>  
>  	.volatile_reg = fsl_dcu_drm_is_volatile_reg,
> -	.max_register = 0x11fc,
>  };
>  
>  static int fsl_dcu_drm_irq_init(struct drm_device *dev)
> @@ -229,9 +229,25 @@ static int fsl_dcu_drm_pm_suspend(struct device *dev)
>  	if (!fsl_dev)
>  		return 0;
>  
> +	disable_irq(fsl_dev->irq);
>  	drm_kms_helper_poll_disable(fsl_dev->drm);
> -	regcache_cache_only(fsl_dev->regmap, true);
> -	regcache_mark_dirty(fsl_dev->regmap);
> +
> +	console_lock();
> +	drm_fbdev_cma_set_suspend(fsl_dev->fbdev, 1);
> +	console_unlock();

Hm, it would be nice to push that console_lock into the helper itself.
Plus maybe reuse the trick i915 uses to put the resume side into a work
item to avoid taking console_lock in a blocking way.

The reason for that is that in a normal suspend/resume operation
console_lock is highly contended due to all the prinkt noise flying
around. If someone disabled fbdev emulation (either in Kconfig or at
runtime) we really, really want to avoid taking it. And without fbdev
there's no need to call fb_set_suspend at all.

Signed up for that little bit of polish? Imo totally ok as a follow-up,
but would be real nice to have.
-Daniel

> +
> +	fsl_dev->state = drm_atomic_helper_suspend(fsl_dev->drm);
> +	if (IS_ERR(fsl_dev->state)) {
> +		console_lock();
> +		drm_fbdev_cma_set_suspend(fsl_dev->fbdev, 0);
> +		console_unlock();
> +
> +		drm_kms_helper_poll_enable(fsl_dev->drm);
> +		enable_irq(fsl_dev->irq);
> +		return PTR_ERR(fsl_dev->state);
> +	}
> +
> +	clk_disable_unprepare(fsl_dev->pix_clk);
>  	clk_disable_unprepare(fsl_dev->clk);
>  
>  	return 0;
> @@ -251,9 +267,21 @@ static int fsl_dcu_drm_pm_resume(struct device *dev)
>  		return ret;
>  	}
>  
> +	ret = clk_prepare_enable(fsl_dev->pix_clk);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to enable pix clk\n");
> +		return ret;
> +	}
> +
> +	fsl_dcu_drm_init_planes(fsl_dev->drm);
> +	drm_atomic_helper_resume(fsl_dev->drm, fsl_dev->state);
> +
> +	console_lock();
> +	drm_fbdev_cma_set_suspend(fsl_dev->fbdev, 0);
> +	console_unlock();
> +
>  	drm_kms_helper_poll_enable(fsl_dev->drm);
> -	regcache_cache_only(fsl_dev->regmap, false);
> -	regcache_sync(fsl_dev->regmap);
> +	enable_irq(fsl_dev->irq);
>  
>  	return 0;
>  }
> diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h
> index b1bba3a..3b371fe7 100644
> --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h
> +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h
> @@ -194,6 +194,7 @@ struct fsl_dcu_drm_device {
>  	struct drm_encoder encoder;
>  	struct fsl_dcu_drm_connector connector;
>  	const struct fsl_dcu_soc_data *soc;
> +	struct drm_atomic_state *state;
>  };
>  
>  void fsl_dcu_fbdev_init(struct drm_device *dev);
> -- 
> 2.8.2
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[PATCH v2 6/6] drm/fsl-dcu: disable vblank events on CRTC disable

[Stefan Agner]

Disable vblank events when CRTC gets disabled. This avoids an
external abort when entering suspend while disable_timer is still
active: On resume the timer might fire immediately and cause a
register access in fsl_dcu_drm_disable_vblank before clocks get
enabled by the resume function.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
index ca0f7d83..36df2eb 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
@@ -43,6 +43,8 @@ static void fsl_dcu_drm_disable_crtc(struct drm_crtc *crtc)
 	struct drm_device *dev = crtc->dev;
 	struct fsl_dcu_drm_device *fsl_dev = dev->dev_private;
 
+	drm_crtc_vblank_off(crtc);
+
 	regmap_update_bits(fsl_dev->regmap, DCU_DCU_MODE,
 			   DCU_MODE_DCU_MODE_MASK,
 			   DCU_MODE_DCU_MODE(DCU_MODE_OFF));
@@ -60,6 +62,8 @@ static void fsl_dcu_drm_crtc_enable(struct drm_crtc *crtc)
 			   DCU_MODE_DCU_MODE(DCU_MODE_NORMAL));
 	regmap_write(fsl_dev->regmap, DCU_UPDATE_MODE,
 		     DCU_UPDATE_MODE_READREG);
+
+	drm_crtc_vblank_on(crtc);
 }
 
 static void fsl_dcu_drm_crtc_mode_set_nofb(struct drm_crtc *crtc)
-- 
2.8.2