[PATCH 0/6] drm/: Add simple modeset suspend/resume helpers

[PATCH 0/6] drm/: Add simple modeset suspend/resume helpers

[Noralf Trønnes]

This patchset adds some simple modeset suspend/resume helpers which
probably most atomic drivers can use.

For those that haven't followed dri-devel closely the past few days,
this patch put in place the fbdev piece necessary to do this:
drm: Add drm_device->fb_helper pointer
https://cgit.freedesktop.org/drm/drm-misc/commit/drivers/gpu/drm/drm_fb_helper.c?id=29ad20b22c8f3ab35e91c2f68b4c7956cee30fd0

I have converted the cma helper drivers as part of my ongoing cma helper
refactoring.

Noralf.

Noralf Trønnes (6):
  drm/probe-helper: Fix drm_kms_helper_poll_enable() docs
  drm/modeset-helper: Add simple modeset suspend/resume helpers
  drm/arm/mali: Use drm_mode_config_helper_suspend/resume()
  drm/fsl-dcu: Use drm_mode_config_helper_suspend/resume()
  drm/tinydrm: Use drm_mode_config_helper_suspend/resume()
  drm/docs: Add todo entry for simple modeset suspend/resume

 Documentation/gpu/todo.rst                  | 14 ++++--
 drivers/gpu/drm/arm/malidp_drv.c            | 24 ++-------
 drivers/gpu/drm/arm/malidp_drv.h            |  1 -
 drivers/gpu/drm/drm_modeset_helper.c        | 76 +++++++++++++++++++++++++++++
 drivers/gpu/drm/drm_probe_helper.c          |  3 +-
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c   | 23 ++-------
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h   |  1 -
 drivers/gpu/drm/tinydrm/core/tinydrm-core.c | 67 -------------------------
 drivers/gpu/drm/tinydrm/mi0283qt.c          |  7 ++-
 include/drm/drm_mode_config.h               |  9 ++++
 include/drm/drm_modeset_helper.h            |  3 ++
 include/drm/tinydrm/tinydrm.h               |  4 --
 12 files changed, 111 insertions(+), 121 deletions(-)

--
2.14.2

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

[PATCH 1/6] drm/probe-helper: Fix drm_kms_helper_poll_enable() docs

[Noralf Trønnes]

Fix docs to reflect code and drm_kms_helper_poll_disable() docs by saying
that calling drm_kms_helper_poll_enable() is fine even if output polling
is not enabled.

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/drm_probe_helper.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
index 5840aabbf24e..024a89bf0ba7 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -216,8 +216,7 @@ enum drm_mode_status drm_connector_mode_valid(struct drm_connector *connector,
  * suspend/resume.
  *
  * Drivers can call this helper from their device resume implementation. It is
- * an error to call this when the output polling support has not yet been set
- * up.
+ * not an error to call this even when output polling isn't enabled.
  *
  * Note that calls to enable and disable polling must be strictly ordered, which
  * is automatically the case when they're only call from suspend/resume
-- 
2.14.2

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

[PATCH 2/6] drm/modeset-helper: Add simple modeset suspend/resume helpers

[Noralf Trønnes]

Add drm_mode_config_helper_suspend/resume() which takes care of
atomic modeset suspend/resume for simple use cases.
The suspend state is stored in struct drm_mode_config.

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/drm_modeset_helper.c | 76 ++++++++++++++++++++++++++++++++++++
 include/drm/drm_mode_config.h        |  9 +++++
 include/drm/drm_modeset_helper.h     |  3 ++
 3 files changed, 88 insertions(+)

diff --git a/drivers/gpu/drm/drm_modeset_helper.c b/drivers/gpu/drm/drm_modeset_helper.c
index 9cb1eede0b4d..f1c24ab0ef09 100644
--- a/drivers/gpu/drm/drm_modeset_helper.c
+++ b/drivers/gpu/drm/drm_modeset_helper.c
@@ -20,6 +20,9 @@
  * OF THIS SOFTWARE.
  */
 
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_crtc_helper.h>
+#include <drm/drm_fb_helper.h>
 #include <drm/drm_modeset_helper.h>
 #include <drm/drm_plane_helper.h>
 
@@ -156,3 +159,76 @@ int drm_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,
 					 NULL);
 }
 EXPORT_SYMBOL(drm_crtc_init);
+
+/**
+ * drm_mode_config_helper_suspend - Modeset suspend helper
+ * @dev: DRM device
+ *
+ * This helper function takes care of suspending the modeset side. It disables
+ * output polling if initialized, suspends fbdev if used and finally calls
+ * drm_atomic_helper_suspend().
+ * If suspending fails, fbdev and polling is re-enabled.
+ *
+ * Returns:
+ * Zero on success, negative error code on error.
+ *
+ * See also:
+ * drm_kms_helper_poll_disable() and drm_fb_helper_set_suspend_unlocked().
+ */
+int drm_mode_config_helper_suspend(struct drm_device *dev)
+{
+	struct drm_atomic_state *state;
+
+	if (!dev)
+		return 0;
+
+	drm_kms_helper_poll_disable(dev);
+	drm_fb_helper_set_suspend_unlocked(dev->fb_helper, 1);
+	state = drm_atomic_helper_suspend(dev);
+	if (IS_ERR(state)) {
+		drm_fb_helper_set_suspend_unlocked(dev->fb_helper, 0);
+		drm_kms_helper_poll_enable(dev);
+		return PTR_ERR(state);
+	}
+
+	dev->mode_config.suspend_state = state;
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_mode_config_helper_suspend);
+
+/**
+ * drm_mode_config_helper_resume - Modeset resume helper
+ * @dev: DRM device
+ *
+ * This helper function takes care of resuming the modeset side. It calls
+ * drm_atomic_helper_resume(), resumes fbdev if used and enables output polling
+ * if initiaized.
+ *
+ * Returns:
+ * Zero on success, negative error code on error.
+ *
+ * See also:
+ * drm_fb_helper_set_suspend_unlocked() and drm_kms_helper_poll_enable().
+ */
+int drm_mode_config_helper_resume(struct drm_device *dev)
+{
+	int ret;
+
+	if (!dev)
+		return 0;
+
+	if (WARN_ON(!dev->mode_config.suspend_state))
+		return -EINVAL;
+
+	ret = drm_atomic_helper_resume(dev, dev->mode_config.suspend_state);
+	if (ret)
+		DRM_ERROR("Failed to resume (%d)\n", ret);
+	dev->mode_config.suspend_state = NULL;
+
+	drm_fb_helper_set_suspend_unlocked(dev->fb_helper, 0);
+	drm_kms_helper_poll_enable(dev);
+
+	return ret;
+}
+EXPORT_SYMBOL(drm_mode_config_helper_resume);
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index 1b37368416c8..5a872496b409 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -766,6 +766,15 @@ struct drm_mode_config {
 	/* cursor size */
 	uint32_t cursor_width, cursor_height;
 
+	/**
+	 * @suspend_state:
+	 *
+	 * Atomic state when suspended.
+	 * Set by drm_mode_config_helper_suspend() and cleared by
+	 * drm_mode_config_helper_resume().
+	 */
+	struct drm_atomic_state *suspend_state;
+
 	const struct drm_mode_config_helper_funcs *helper_private;
 };
 
diff --git a/include/drm/drm_modeset_helper.h b/include/drm/drm_modeset_helper.h
index cb0ec92e11e6..efa337f03129 100644
--- a/include/drm/drm_modeset_helper.h
+++ b/include/drm/drm_modeset_helper.h
@@ -34,4 +34,7 @@ void drm_helper_mode_fill_fb_struct(struct drm_device *dev,
 int drm_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,
 		  const struct drm_crtc_funcs *funcs);
 
+int drm_mode_config_helper_suspend(struct drm_device *dev);
+int drm_mode_config_helper_resume(struct drm_device *dev);
+
 #endif
-- 
2.14.2

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

[PATCH 3/6] drm/arm/mali: Use drm_mode_config_helper_suspend/resume()

[Noralf Trønnes]

These helpers take care of output polling, fbdev and atomic state.

Cc: Liviu Dudau <liviu.dudau@arm.com>
Cc: Brian Starkey <brian.starkey@arm.com>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/arm/malidp_drv.c | 24 +++---------------------
 drivers/gpu/drm/arm/malidp_drv.h |  1 -
 2 files changed, 3 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
index b8944666a18f..75f0bce33941 100644
--- a/drivers/gpu/drm/arm/malidp_drv.c
+++ b/drivers/gpu/drm/arm/malidp_drv.c
@@ -27,6 +27,7 @@
 #include <drm/drm_fb_cma_helper.h>
 #include <drm/drm_gem_cma_helper.h>
 #include <drm/drm_gem_framebuffer_helper.h>
+#include <drm/drm_modeset_helper.h>
 #include <drm/drm_of.h>
 
 #include "malidp_drv.h"
@@ -749,34 +750,15 @@ static int malidp_platform_remove(struct platform_device *pdev)
 static int __maybe_unused malidp_pm_suspend(struct device *dev)
 {
 	struct drm_device *drm = dev_get_drvdata(dev);
-	struct malidp_drm *malidp = drm->dev_private;
 
-	drm_kms_helper_poll_disable(drm);
-	console_lock();
-	drm_fbdev_cma_set_suspend(malidp->fbdev, 1);
-	console_unlock();
-	malidp->pm_state = drm_atomic_helper_suspend(drm);
-	if (IS_ERR(malidp->pm_state)) {
-		console_lock();
-		drm_fbdev_cma_set_suspend(malidp->fbdev, 0);
-		console_unlock();
-		drm_kms_helper_poll_enable(drm);
-		return PTR_ERR(malidp->pm_state);
-	}
-
-	return 0;
+	return drm_mode_config_helper_suspend(drm);
 }
 
 static int __maybe_unused malidp_pm_resume(struct device *dev)
 {
 	struct drm_device *drm = dev_get_drvdata(dev);
-	struct malidp_drm *malidp = drm->dev_private;
 
-	drm_atomic_helper_resume(drm, malidp->pm_state);
-	console_lock();
-	drm_fbdev_cma_set_suspend(malidp->fbdev, 0);
-	console_unlock();
-	drm_kms_helper_poll_enable(drm);
+	drm_mode_config_helper_resume(drm);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/arm/malidp_drv.h b/drivers/gpu/drm/arm/malidp_drv.h
index 2e2033140efc..70ed6aeccf05 100644
--- a/drivers/gpu/drm/arm/malidp_drv.h
+++ b/drivers/gpu/drm/arm/malidp_drv.h
@@ -24,7 +24,6 @@ struct malidp_drm {
 	struct drm_crtc crtc;
 	wait_queue_head_t wq;
 	atomic_t config_valid;
-	struct drm_atomic_state *pm_state;
 	u32 core_id;
 };
 
-- 
2.14.2

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

[PATCH 4/6] drm/fsl-dcu: Use drm_mode_config_helper_suspend/resume()

[Noralf Trønnes]

These helpers take care of output polling, fbdev and atomic state.

Cc: Stefan Agner <stefan@agner.ch>
Cc: Alison Wang <alison.wang@freescale.com>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 23 +++++------------------
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h |  1 -
 2 files changed, 5 insertions(+), 19 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 58e9e0601a61..0738d9c6f1aa 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
@@ -27,6 +27,7 @@
 #include <drm/drm_crtc_helper.h>
 #include <drm/drm_fb_cma_helper.h>
 #include <drm/drm_gem_cma_helper.h>
+#include <drm/drm_modeset_helper.h>
 
 #include "fsl_dcu_drm_crtc.h"
 #include "fsl_dcu_drm_drv.h"
@@ -193,21 +194,11 @@ static int fsl_dcu_drm_pm_suspend(struct device *dev)
 		return 0;
 
 	disable_irq(fsl_dev->irq);
-	drm_kms_helper_poll_disable(fsl_dev->drm);
 
-	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);
+	ret = drm_mode_config_helper_suspend(fsl_dev->drm);
+	if (ret) {
 		enable_irq(fsl_dev->irq);
-		return PTR_ERR(fsl_dev->state);
+		return ret;
 	}
 
 	clk_disable_unprepare(fsl_dev->pix_clk);
@@ -233,13 +224,9 @@ static int fsl_dcu_drm_pm_resume(struct device *dev)
 	if (fsl_dev->tcon)
 		fsl_tcon_bypass_enable(fsl_dev->tcon);
 	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_mode_config_helper_resume(fsl_dev->drm);
 
-	drm_kms_helper_poll_enable(fsl_dev->drm);
 	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 da9bfd432ca6..93bfb98012d4 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h
@@ -196,7 +196,6 @@ 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;
 };
 
 int fsl_dcu_drm_modeset_init(struct fsl_dcu_drm_device *fsl_dev);
-- 
2.14.2

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

[PATCH 5/6] drm/tinydrm: Use drm_mode_config_helper_suspend/resume()

[Noralf Trønnes]

These helpers take care of fbdev and atomic state.
Remove todo entry.

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 Documentation/gpu/todo.rst                  |  5 ---
 drivers/gpu/drm/tinydrm/core/tinydrm-core.c | 67 -----------------------------
 drivers/gpu/drm/tinydrm/mi0283qt.c          |  7 ++-
 include/drm/tinydrm/tinydrm.h               |  4 --
 4 files changed, 5 insertions(+), 78 deletions(-)

diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
index e9840d693a86..a44f379d2b25 100644
--- a/Documentation/gpu/todo.rst
+++ b/Documentation/gpu/todo.rst
@@ -404,11 +404,6 @@ those drivers as simple as possible, so lots of room for refactoring:
   a drm_device wrong. Doesn't matter, since everyone else gets it wrong
   too :-)
 
-- With the fbdev pointer in dev->mode_config we could also make
-  suspend/resume helpers entirely generic, at least if we add a
-  dev->mode_config.suspend_state. We could even provide a generic pm_ops
-  structure with those.
-
 - also rework the drm_framebuffer_funcs->dirty hook wire-up, see above.
 
 Contact: Noralf Trønnes, Daniel Vetter
diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-core.c b/drivers/gpu/drm/tinydrm/core/tinydrm-core.c
index 1a8a57cad431..bd7b82824a34 100644
--- a/drivers/gpu/drm/tinydrm/core/tinydrm-core.c
+++ b/drivers/gpu/drm/tinydrm/core/tinydrm-core.c
@@ -292,71 +292,4 @@ void tinydrm_shutdown(struct tinydrm_device *tdev)
 }
 EXPORT_SYMBOL(tinydrm_shutdown);
 
-/**
- * tinydrm_suspend - Suspend tinydrm
- * @tdev: tinydrm device
- *
- * Used in driver PM operations to suspend tinydrm.
- * Suspends fbdev and DRM.
- * Resume with tinydrm_resume().
- *
- * Returns:
- * Zero on success, negative error code on failure.
- */
-int tinydrm_suspend(struct tinydrm_device *tdev)
-{
-	struct drm_atomic_state *state;
-
-	if (tdev->suspend_state) {
-		DRM_ERROR("Failed to suspend: state already set\n");
-		return -EINVAL;
-	}
-
-	drm_fbdev_cma_set_suspend_unlocked(tdev->fbdev_cma, 1);
-	state = drm_atomic_helper_suspend(tdev->drm);
-	if (IS_ERR(state)) {
-		drm_fbdev_cma_set_suspend_unlocked(tdev->fbdev_cma, 0);
-		return PTR_ERR(state);
-	}
-
-	tdev->suspend_state = state;
-
-	return 0;
-}
-EXPORT_SYMBOL(tinydrm_suspend);
-
-/**
- * tinydrm_resume - Resume tinydrm
- * @tdev: tinydrm device
- *
- * Used in driver PM operations to resume tinydrm.
- * Suspend with tinydrm_suspend().
- *
- * Returns:
- * Zero on success, negative error code on failure.
- */
-int tinydrm_resume(struct tinydrm_device *tdev)
-{
-	struct drm_atomic_state *state = tdev->suspend_state;
-	int ret;
-
-	if (!state) {
-		DRM_ERROR("Failed to resume: state is not set\n");
-		return -EINVAL;
-	}
-
-	tdev->suspend_state = NULL;
-
-	ret = drm_atomic_helper_resume(tdev->drm, state);
-	if (ret) {
-		DRM_ERROR("Error resuming state: %d\n", ret);
-		return ret;
-	}
-
-	drm_fbdev_cma_set_suspend_unlocked(tdev->fbdev_cma, 0);
-
-	return 0;
-}
-EXPORT_SYMBOL(tinydrm_resume);
-
 MODULE_LICENSE("GPL");
diff --git a/drivers/gpu/drm/tinydrm/mi0283qt.c b/drivers/gpu/drm/tinydrm/mi0283qt.c
index 6a83b3093254..70ae4f76f455 100644
--- a/drivers/gpu/drm/tinydrm/mi0283qt.c
+++ b/drivers/gpu/drm/tinydrm/mi0283qt.c
@@ -9,6 +9,7 @@
  * (at your option) any later version.
  */
 
+#include <drm/drm_modeset_helper.h>
 #include <drm/tinydrm/ili9341.h>
 #include <drm/tinydrm/mipi-dbi.h>
 #include <drm/tinydrm/tinydrm-helpers.h>
@@ -231,7 +232,7 @@ static int __maybe_unused mi0283qt_pm_suspend(struct device *dev)
 	struct mipi_dbi *mipi = dev_get_drvdata(dev);
 	int ret;
 
-	ret = tinydrm_suspend(&mipi->tinydrm);
+	ret = drm_mode_config_helper_suspend(mipi->tinydrm.drm);
 	if (ret)
 		return ret;
 
@@ -249,7 +250,9 @@ static int __maybe_unused mi0283qt_pm_resume(struct device *dev)
 	if (ret)
 		return ret;
 
-	return tinydrm_resume(&mipi->tinydrm);
+	drm_mode_config_helper_resume(mipi->tinydrm.drm);
+
+	return 0;
 }
 
 static const struct dev_pm_ops mi0283qt_pm_ops = {
diff --git a/include/drm/tinydrm/tinydrm.h b/include/drm/tinydrm/tinydrm.h
index 4774fe3d4273..fb0d86600ea3 100644
--- a/include/drm/tinydrm/tinydrm.h
+++ b/include/drm/tinydrm/tinydrm.h
@@ -20,7 +20,6 @@
  * @pipe: Display pipe structure
  * @dirty_lock: Serializes framebuffer flushing
  * @fbdev_cma: CMA fbdev structure
- * @suspend_state: Atomic state when suspended
  * @fb_funcs: Framebuffer functions used when creating framebuffers
  */
 struct tinydrm_device {
@@ -28,7 +27,6 @@ struct tinydrm_device {
 	struct drm_simple_display_pipe pipe;
 	struct mutex dirty_lock;
 	struct drm_fbdev_cma *fbdev_cma;
-	struct drm_atomic_state *suspend_state;
 	const struct drm_framebuffer_funcs *fb_funcs;
 };
 
@@ -92,8 +90,6 @@ int devm_tinydrm_init(struct device *parent, struct tinydrm_device *tdev,
 		      struct drm_driver *driver);
 int devm_tinydrm_register(struct tinydrm_device *tdev);
 void tinydrm_shutdown(struct tinydrm_device *tdev);
-int tinydrm_suspend(struct tinydrm_device *tdev);
-int tinydrm_resume(struct tinydrm_device *tdev);
 
 void tinydrm_display_pipe_update(struct drm_simple_display_pipe *pipe,
 				 struct drm_plane_state *old_state);
-- 
2.14.2

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

[PATCH 6/6] drm/docs: Add todo entry for simple modeset suspend/resume

[Noralf Trønnes]

Add entry for conversion of drivers to new helpers.

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 Documentation/gpu/todo.rst | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
index a44f379d2b25..6bce1beafabe 100644
--- a/Documentation/gpu/todo.rst
+++ b/Documentation/gpu/todo.rst
@@ -185,6 +185,15 @@ are better.
 
 Contact: Sean Paul, Maintainer of the driver you plan to convert
 
+Convert drivers to use simple modeset suspend/resume
+----------------------------------------------------
+
+Most drivers (except i915 and nouveau) that use
+drm_atomic_helper_suspend/resume() can probably be converted to use
+drm_mode_config_helper_suspend/resume().
+
+Contact: Maintainer of the driver you plan to convert
+
 Core refactorings
 =================
 
-- 
2.14.2

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

Re: [PATCH 4/6] drm/fsl-dcu: Use drm_mode_config_helper_suspend/resume()

[Noralf Trønnes]

Den 02.11.2017 21.09, skrev Noralf Trønnes:
> These helpers take care of output polling, fbdev and atomic state.
>
> Cc: Stefan Agner <stefan@agner.ch>
> Cc: Alison Wang <alison.wang@freescale.com>
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> ---
>   drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 23 +++++------------------
>   drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h |  1 -
>   2 files changed, 5 insertions(+), 19 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 58e9e0601a61..0738d9c6f1aa 100644
> --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
> +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
> @@ -27,6 +27,7 @@
>   #include <drm/drm_crtc_helper.h>
>   #include <drm/drm_fb_cma_helper.h>
>   #include <drm/drm_gem_cma_helper.h>
> +#include <drm/drm_modeset_helper.h>
>   
>   #include "fsl_dcu_drm_crtc.h"
>   #include "fsl_dcu_drm_drv.h"
> @@ -193,21 +194,11 @@ static int fsl_dcu_drm_pm_suspend(struct device *dev)
>   		return 0;
>   

Somehow I missed that ret wasn't declared here.

     int ret;

Will fix in the next version.

Noralf.

>   	disable_irq(fsl_dev->irq);
> -	drm_kms_helper_poll_disable(fsl_dev->drm);
>   
> -	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);
> +	ret = drm_mode_config_helper_suspend(fsl_dev->drm);
> +	if (ret) {
>   		enable_irq(fsl_dev->irq);
> -		return PTR_ERR(fsl_dev->state);
> +		return ret;
>   	}
>   
>   	clk_disable_unprepare(fsl_dev->pix_clk);
> @@ -233,13 +224,9 @@ static int fsl_dcu_drm_pm_resume(struct device *dev)
>   	if (fsl_dev->tcon)
>   		fsl_tcon_bypass_enable(fsl_dev->tcon);
>   	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_mode_config_helper_resume(fsl_dev->drm);
>   
> -	drm_kms_helper_poll_enable(fsl_dev->drm);
>   	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 da9bfd432ca6..93bfb98012d4 100644
> --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h
> +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h
> @@ -196,7 +196,6 @@ 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;
>   };
>   
>   int fsl_dcu_drm_modeset_init(struct fsl_dcu_drm_device *fsl_dev);

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

Re: [PATCH 1/6] drm/probe-helper: Fix drm_kms_helper_poll_enable() docs

[Daniel Vetter]

On Thu, Nov 02, 2017 at 09:09:04PM +0100, Noralf Trønnes wrote:
> Fix docs to reflect code and drm_kms_helper_poll_disable() docs by saying
> that calling drm_kms_helper_poll_enable() is fine even if output polling
> is not enabled.
> 
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>

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

> ---
>  drivers/gpu/drm/drm_probe_helper.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
> index 5840aabbf24e..024a89bf0ba7 100644
> --- a/drivers/gpu/drm/drm_probe_helper.c
> +++ b/drivers/gpu/drm/drm_probe_helper.c
> @@ -216,8 +216,7 @@ enum drm_mode_status drm_connector_mode_valid(struct drm_connector *connector,
>   * suspend/resume.
>   *
>   * Drivers can call this helper from their device resume implementation. It is
> - * an error to call this when the output polling support has not yet been set
> - * up.
> + * not an error to call this even when output polling isn't enabled.
>   *
>   * Note that calls to enable and disable polling must be strictly ordered, which
>   * is automatically the case when they're only call from suspend/resume
> -- 
> 2.14.2
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/6] drm/modeset-helper: Add simple modeset suspend/resume helpers

[Daniel Vetter]

On Thu, Nov 02, 2017 at 09:09:05PM +0100, Noralf Trønnes wrote:
> Add drm_mode_config_helper_suspend/resume() which takes care of
> atomic modeset suspend/resume for simple use cases.
> The suspend state is stored in struct drm_mode_config.
> 
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>

It'd be great if we could add a paragraph somewhere more prominent that
references these 2 helpers. As-is they'll be impossible to find.

But I didn't find a good spot, so

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

on this one. If you have an idea, follow-up would be great.

> ---
>  drivers/gpu/drm/drm_modeset_helper.c | 76 ++++++++++++++++++++++++++++++++++++
>  include/drm/drm_mode_config.h        |  9 +++++
>  include/drm/drm_modeset_helper.h     |  3 ++
>  3 files changed, 88 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_modeset_helper.c b/drivers/gpu/drm/drm_modeset_helper.c
> index 9cb1eede0b4d..f1c24ab0ef09 100644
> --- a/drivers/gpu/drm/drm_modeset_helper.c
> +++ b/drivers/gpu/drm/drm_modeset_helper.c
> @@ -20,6 +20,9 @@
>   * OF THIS SOFTWARE.
>   */
>  
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_crtc_helper.h>
> +#include <drm/drm_fb_helper.h>
>  #include <drm/drm_modeset_helper.h>
>  #include <drm/drm_plane_helper.h>
>  
> @@ -156,3 +159,76 @@ int drm_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,
>  					 NULL);
>  }
>  EXPORT_SYMBOL(drm_crtc_init);
> +
> +/**
> + * drm_mode_config_helper_suspend - Modeset suspend helper
> + * @dev: DRM device
> + *
> + * This helper function takes care of suspending the modeset side. It disables
> + * output polling if initialized, suspends fbdev if used and finally calls
> + * drm_atomic_helper_suspend().

ocd nit: Either full new paragraph or merge the lines.
-Daniel

> + * If suspending fails, fbdev and polling is re-enabled.
> + *
> + * Returns:
> + * Zero on success, negative error code on error.
> + *
> + * See also:
> + * drm_kms_helper_poll_disable() and drm_fb_helper_set_suspend_unlocked().
> + */
> +int drm_mode_config_helper_suspend(struct drm_device *dev)
> +{
> +	struct drm_atomic_state *state;
> +
> +	if (!dev)
> +		return 0;
> +
> +	drm_kms_helper_poll_disable(dev);
> +	drm_fb_helper_set_suspend_unlocked(dev->fb_helper, 1);
> +	state = drm_atomic_helper_suspend(dev);
> +	if (IS_ERR(state)) {
> +		drm_fb_helper_set_suspend_unlocked(dev->fb_helper, 0);
> +		drm_kms_helper_poll_enable(dev);
> +		return PTR_ERR(state);
> +	}
> +
> +	dev->mode_config.suspend_state = state;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(drm_mode_config_helper_suspend);
> +
> +/**
> + * drm_mode_config_helper_resume - Modeset resume helper
> + * @dev: DRM device
> + *
> + * This helper function takes care of resuming the modeset side. It calls
> + * drm_atomic_helper_resume(), resumes fbdev if used and enables output polling
> + * if initiaized.
> + *
> + * Returns:
> + * Zero on success, negative error code on error.
> + *
> + * See also:
> + * drm_fb_helper_set_suspend_unlocked() and drm_kms_helper_poll_enable().
> + */
> +int drm_mode_config_helper_resume(struct drm_device *dev)
> +{
> +	int ret;
> +
> +	if (!dev)
> +		return 0;
> +
> +	if (WARN_ON(!dev->mode_config.suspend_state))
> +		return -EINVAL;
> +
> +	ret = drm_atomic_helper_resume(dev, dev->mode_config.suspend_state);
> +	if (ret)
> +		DRM_ERROR("Failed to resume (%d)\n", ret);
> +	dev->mode_config.suspend_state = NULL;
> +
> +	drm_fb_helper_set_suspend_unlocked(dev->fb_helper, 0);
> +	drm_kms_helper_poll_enable(dev);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL(drm_mode_config_helper_resume);
> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
> index 1b37368416c8..5a872496b409 100644
> --- a/include/drm/drm_mode_config.h
> +++ b/include/drm/drm_mode_config.h
> @@ -766,6 +766,15 @@ struct drm_mode_config {
>  	/* cursor size */
>  	uint32_t cursor_width, cursor_height;
>  
> +	/**
> +	 * @suspend_state:
> +	 *
> +	 * Atomic state when suspended.
> +	 * Set by drm_mode_config_helper_suspend() and cleared by
> +	 * drm_mode_config_helper_resume().
> +	 */
> +	struct drm_atomic_state *suspend_state;
> +
>  	const struct drm_mode_config_helper_funcs *helper_private;
>  };
>  
> diff --git a/include/drm/drm_modeset_helper.h b/include/drm/drm_modeset_helper.h
> index cb0ec92e11e6..efa337f03129 100644
> --- a/include/drm/drm_modeset_helper.h
> +++ b/include/drm/drm_modeset_helper.h
> @@ -34,4 +34,7 @@ void drm_helper_mode_fill_fb_struct(struct drm_device *dev,
>  int drm_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,
>  		  const struct drm_crtc_funcs *funcs);
>  
> +int drm_mode_config_helper_suspend(struct drm_device *dev);
> +int drm_mode_config_helper_resume(struct drm_device *dev);
> +
>  #endif
> -- 
> 2.14.2
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 6/6] drm/docs: Add todo entry for simple modeset suspend/resume

[Daniel Vetter]

On Thu, Nov 02, 2017 at 09:09:09PM +0100, Noralf Trønnes wrote:
> Add entry for conversion of drivers to new helpers.
> 
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>

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

I think you can merge this one as soon as the code has landed, no need to
wait for all the driver conversions to get acks.
-Daniel


> ---
>  Documentation/gpu/todo.rst | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
> index a44f379d2b25..6bce1beafabe 100644
> --- a/Documentation/gpu/todo.rst
> +++ b/Documentation/gpu/todo.rst
> @@ -185,6 +185,15 @@ are better.
>  
>  Contact: Sean Paul, Maintainer of the driver you plan to convert
>  
> +Convert drivers to use simple modeset suspend/resume
> +----------------------------------------------------
> +
> +Most drivers (except i915 and nouveau) that use
> +drm_atomic_helper_suspend/resume() can probably be converted to use
> +drm_mode_config_helper_suspend/resume().
> +
> +Contact: Maintainer of the driver you plan to convert
> +
>  Core refactorings
>  =================
>  
> -- 
> 2.14.2
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/6] drm/arm/mali: Use drm_mode_config_helper_suspend/resume()

[Liviu Dudau]

On Thu, Nov 02, 2017 at 09:09:06PM +0100, Noralf Trønnes wrote:
> These helpers take care of output polling, fbdev and atomic state.

Hmm, not much useful info here, tbh. Maybe something like:

"Replace driver's code with the generic helpers that do the same thing" ?

> 
> Cc: Liviu Dudau <liviu.dudau@arm.com>
> Cc: Brian Starkey <brian.starkey@arm.com>
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>

Otherwise, looks good to me. Thanks!

Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>

> ---
>  drivers/gpu/drm/arm/malidp_drv.c | 24 +++---------------------
>  drivers/gpu/drm/arm/malidp_drv.h |  1 -
>  2 files changed, 3 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
> index b8944666a18f..75f0bce33941 100644
> --- a/drivers/gpu/drm/arm/malidp_drv.c
> +++ b/drivers/gpu/drm/arm/malidp_drv.c
> @@ -27,6 +27,7 @@
>  #include <drm/drm_fb_cma_helper.h>
>  #include <drm/drm_gem_cma_helper.h>
>  #include <drm/drm_gem_framebuffer_helper.h>
> +#include <drm/drm_modeset_helper.h>
>  #include <drm/drm_of.h>
>  
>  #include "malidp_drv.h"
> @@ -749,34 +750,15 @@ static int malidp_platform_remove(struct platform_device *pdev)
>  static int __maybe_unused malidp_pm_suspend(struct device *dev)
>  {
>  	struct drm_device *drm = dev_get_drvdata(dev);
> -	struct malidp_drm *malidp = drm->dev_private;
>  
> -	drm_kms_helper_poll_disable(drm);
> -	console_lock();
> -	drm_fbdev_cma_set_suspend(malidp->fbdev, 1);
> -	console_unlock();
> -	malidp->pm_state = drm_atomic_helper_suspend(drm);
> -	if (IS_ERR(malidp->pm_state)) {
> -		console_lock();
> -		drm_fbdev_cma_set_suspend(malidp->fbdev, 0);
> -		console_unlock();
> -		drm_kms_helper_poll_enable(drm);
> -		return PTR_ERR(malidp->pm_state);
> -	}
> -
> -	return 0;
> +	return drm_mode_config_helper_suspend(drm);
>  }
>  
>  static int __maybe_unused malidp_pm_resume(struct device *dev)
>  {
>  	struct drm_device *drm = dev_get_drvdata(dev);
> -	struct malidp_drm *malidp = drm->dev_private;
>  
> -	drm_atomic_helper_resume(drm, malidp->pm_state);
> -	console_lock();
> -	drm_fbdev_cma_set_suspend(malidp->fbdev, 0);
> -	console_unlock();
> -	drm_kms_helper_poll_enable(drm);
> +	drm_mode_config_helper_resume(drm);
>  
>  	return 0;
>  }
> diff --git a/drivers/gpu/drm/arm/malidp_drv.h b/drivers/gpu/drm/arm/malidp_drv.h
> index 2e2033140efc..70ed6aeccf05 100644
> --- a/drivers/gpu/drm/arm/malidp_drv.h
> +++ b/drivers/gpu/drm/arm/malidp_drv.h
> @@ -24,7 +24,6 @@ struct malidp_drm {
>  	struct drm_crtc crtc;
>  	wait_queue_head_t wq;
>  	atomic_t config_valid;
> -	struct drm_atomic_state *pm_state;
>  	u32 core_id;
>  };
>  
> -- 
> 2.14.2
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel