[PATCH v2 00/12] drm/tinydrm: Remove tinydrm_device

[PATCH v2 00/12] drm/tinydrm: Remove tinydrm_device

[Noralf Trønnes]

This patchset is part of the effort to remove tinydrm.ko. It removes
struct tinydrm_device and tinydrm.h.

Main changes in this version:

Take a ref on the parent device for the drm_device lifetime.

Drop devm_drm_dev_register() and add driver remove callbacks. The reason
is that drm_atomic_helper_shutdown() must be called after unregister and
it's not allowed to touch hardware in a devm release action.

Drop drm_simple_connector. I couldn't do rotation the proposed way so I
need more time to sort it out.

I've also started on a driver example for the documentation.

Note:
The series depends on this:

drm/drv: Rework drm_dev_unplug()
https://patchwork.freedesktop.org/series/56406/

Noralf.

Noralf Trønnes (12):
  drm/drv: Hold ref on parent device during drm_device lifetime
  drm: Add devm_drm_dev_init()
  drm/drv: DOC: Add driver example code
  drm/modes: Add DRM_SIMPLE_MODE()
  drm/tinydrm: tinydrm_display_pipe_init() don't use tinydrm_device
  drm/tinydrm: Remove tinydrm_shutdown()
  drm/tinydrm/mipi-dbi: Add drm_to_mipi_dbi()
  drm/tinydrm/repaper: Drop using tinydrm_device
  drm/tinydrm: Drop using tinydrm_device
  drm/tinydrm: Remove tinydrm_device
  drm/tinydrm: Use drm_dev_enter/exit()
  drm/fb-helper: generic: Don't take module ref for fbcon

 Documentation/driver-model/devres.txt         |   3 +
 Documentation/gpu/tinydrm.rst                 |  32 +--
 Documentation/gpu/todo.rst                    |   4 -
 drivers/gpu/drm/drm_drv.c                     | 162 +++++++++++++++-
 drivers/gpu/drm/drm_fb_helper.c               |   6 +-
 drivers/gpu/drm/tinydrm/core/Makefile         |   2 +-
 drivers/gpu/drm/tinydrm/core/tinydrm-core.c   | 183 ------------------
 .../gpu/drm/tinydrm/core/tinydrm-helpers.c    |   2 +
 drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c   |  24 +--
 drivers/gpu/drm/tinydrm/hx8357d.c             |  59 ++++--
 drivers/gpu/drm/tinydrm/ili9225.c             |  81 ++++++--
 drivers/gpu/drm/tinydrm/ili9341.c             |  59 ++++--
 drivers/gpu/drm/tinydrm/mi0283qt.c            |  67 +++++--
 drivers/gpu/drm/tinydrm/mipi-dbi.c            | 118 +++++++----
 drivers/gpu/drm/tinydrm/repaper.c             | 144 ++++++++++----
 drivers/gpu/drm/tinydrm/st7586.c              | 145 ++++++++------
 drivers/gpu/drm/tinydrm/st7735r.c             |  59 ++++--
 include/drm/drm_drv.h                         |   3 +
 include/drm/drm_modes.h                       |  17 ++
 include/drm/tinydrm/mipi-dbi.h                |  27 ++-
 include/drm/tinydrm/tinydrm-helpers.h         |  13 ++
 include/drm/tinydrm/tinydrm.h                 |  75 -------
 22 files changed, 762 insertions(+), 523 deletions(-)
 delete mode 100644 drivers/gpu/drm/tinydrm/core/tinydrm-core.c
 delete mode 100644 include/drm/tinydrm/tinydrm.h

-- 
2.20.1

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

[PATCH v2 01/12] drm/drv: Hold ref on parent device during drm_device lifetime

[Noralf Trønnes]

This makes it safe to access drm_device->dev after the parent device has
been removed/unplugged.

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/drm_drv.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index b04982101fcb..27017c15315a 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -496,7 +496,7 @@ int drm_dev_init(struct drm_device *dev,
 	BUG_ON(!parent);
 
 	kref_init(&dev->ref);
-	dev->dev = parent;
+	dev->dev = get_device(parent);
 	dev->driver = driver;
 
 	/* no per-device feature limits by default */
@@ -566,6 +566,7 @@ int drm_dev_init(struct drm_device *dev,
 	drm_minor_free(dev, DRM_MINOR_RENDER);
 	drm_fs_inode_free(dev->anon_inode);
 err_free:
+	put_device(dev->dev);
 	mutex_destroy(&dev->master_mutex);
 	mutex_destroy(&dev->ctxlist_mutex);
 	mutex_destroy(&dev->clientlist_mutex);
@@ -601,6 +602,8 @@ void drm_dev_fini(struct drm_device *dev)
 	drm_minor_free(dev, DRM_MINOR_PRIMARY);
 	drm_minor_free(dev, DRM_MINOR_RENDER);
 
+	put_device(dev->dev);
+
 	mutex_destroy(&dev->master_mutex);
 	mutex_destroy(&dev->ctxlist_mutex);
 	mutex_destroy(&dev->clientlist_mutex);
-- 
2.20.1

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

[PATCH v2 02/12] drm: Add devm_drm_dev_init()

[Noralf Trønnes]

This adds a resource managed (devres) version of drm_dev_init().

v2: Remove devm_drm_dev_register() since we can't touch hw in devm
    release functions and drivers want to disable hw on driver module
    unload (Daniel Vetter, Greg KH)

Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Greg KH <gregkh@linuxfoundation.org>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 Documentation/driver-model/devres.txt |  3 +++
 drivers/gpu/drm/drm_drv.c             | 39 +++++++++++++++++++++++++++
 include/drm/drm_drv.h                 |  3 +++
 3 files changed, 45 insertions(+)

diff --git a/Documentation/driver-model/devres.txt b/Documentation/driver-model/devres.txt
index b277cafce71e..351b7ac65a1e 100644
--- a/Documentation/driver-model/devres.txt
+++ b/Documentation/driver-model/devres.txt
@@ -254,6 +254,9 @@ DMA
   dmam_pool_create()
   dmam_pool_destroy()
 
+DRM
+  devm_drm_dev_init()
+
 GPIO
   devm_gpiod_get()
   devm_gpiod_get_index()
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 27017c15315a..351f128ec4b7 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -576,6 +576,45 @@ int drm_dev_init(struct drm_device *dev,
 }
 EXPORT_SYMBOL(drm_dev_init);
 
+static void devm_drm_dev_init_release(void *data)
+{
+	drm_dev_put(data);
+}
+
+/**
+ * devm_drm_dev_init - Resource managed drm_dev_init()
+ * @parent: Parent device object
+ * @dev: DRM device
+ * @driver: DRM driver
+ *
+ * Managed drm_dev_init(). The DRM device initialized with this function is
+ * automatically put on driver detach using drm_dev_put(). You must supply a
+ * &drm_driver.release callback to control the finalization explicitly.
+ *
+ * RETURNS:
+ * 0 on success, or error code on failure.
+ */
+int devm_drm_dev_init(struct device *parent,
+		      struct drm_device *dev,
+		      struct drm_driver *driver)
+{
+	int ret;
+
+	if (WARN_ON(!parent || !driver->release))
+		return -EINVAL;
+
+	ret = drm_dev_init(dev, driver, parent);
+	if (ret)
+		return ret;
+
+	ret = devm_add_action(parent, devm_drm_dev_init_release, dev);
+	if (ret)
+		devm_drm_dev_init_release(dev);
+
+	return ret;
+}
+EXPORT_SYMBOL(devm_drm_dev_init);
+
 /**
  * drm_dev_fini - Finalize a dead DRM device
  * @dev: DRM device
diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
index ca46a45a9cce..e81bce2698e3 100644
--- a/include/drm/drm_drv.h
+++ b/include/drm/drm_drv.h
@@ -718,6 +718,9 @@ extern unsigned int drm_debug;
 int drm_dev_init(struct drm_device *dev,
 		 struct drm_driver *driver,
 		 struct device *parent);
+int devm_drm_dev_init(struct device *parent,
+		      struct drm_device *dev,
+		      struct drm_driver *driver);
 void drm_dev_fini(struct drm_device *dev);
 
 struct drm_device *drm_dev_alloc(struct drm_driver *driver,
-- 
2.20.1

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

[PATCH v2 03/12] drm/drv: DOC: Add driver example code

[Noralf Trønnes]

Add driver example that shows how devm_drm_dev_init() can be used.

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---

I'm not sure how detailed such an example such be and a description of 
some kind is also required. Help is needed :-)

Noralf.

 drivers/gpu/drm/drm_drv.c | 118 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 118 insertions(+)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 351f128ec4b7..99ca3551688f 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -286,6 +286,124 @@ void drm_minor_release(struct drm_minor *minor)
  * Note that the lifetime rules for &drm_device instance has still a lot of
  * historical baggage. Hence use the reference counting provided by
  * drm_dev_get() and drm_dev_put() only carefully.
+ *
+ * Display driver example
+ * ~~~~~~~~~~~~~~~~~~~~~~
+ *
+ * .. code-block:: c
+ *
+ *	struct driver_device {
+ *		struct drm_device drm;
+ *		void *userspace_facing;
+ *		struct clk *pclk;
+ *	};
+ *
+ *	static inline struct driver_device *drm_to_priv(struct drm_device *drm)
+ *	{
+ *		return container_of(drm, struct driver_device, drm);
+ *	}
+ *
+ *	static void driver_drm_release(struct drm_device *drm)
+ *	{
+ *		struct driver_device *priv = drm_to_priv(drm);
+ *
+ *		drm_mode_config_cleanup(drm);
+ *		drm_dev_fini(drm);
+ *		kfree(priv->userspace_facing);
+ *		kfree(priv);
+ *	}
+ *
+ *	static struct drm_driver driver_drm_driver = {
+ *		[...]
+ *		.release = driver_drm_release,
+ *	};
+ *
+ *	static int driver_probe(struct platform_device *pdev)
+ *	{
+ *		struct driver_device *priv;
+ *		struct drm_device *drm;
+ *		int ret;
+ *
+ *		priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+ *		if (!priv)
+ *			return -ENOMEM;
+ *
+ *		drm = &priv->drm;
+ *
+ *		ret = devm_drm_dev_init(&pdev->dev, drm, &driver_drm_driver);
+ *		if (ret) {
+ *			kfree(drm);
+ *			return ret;
+ *		}
+ *
+ *		drm_mode_config_init(drm);
+ *
+ *		priv->userspace_facing = kzalloc(..., GFP_KERNEL);
+ *		if (!priv->userspace_facing)
+ *			return -ENOMEM;
+ *
+ *		priv->pclk = devm_clk_get(dev, "PCLK");
+ *		if (IS_ERR(priv->pclk))
+ *			return PTR_ERR(priv->pclk);
+ *
+ *		[ Further setup, display pipeline etc ]
+ *
+ *		drm_mode_config_reset(drm);
+ *
+ *		ret = drm_dev_register(drm);
+ *		if (ret)
+ *			return ret;
+ *
+ *		platform_set_drvdata(pdev, drm);
+ *
+ *		drm_fbdev_generic_setup(drm, 32);
+ *
+ *		return 0;
+ *	}
+ *
+ *	[ This function is called before the devm_ resources are released ]
+ *	static int driver_remove(struct platform_device *pdev)
+ *	{
+ *		struct drm_device *drm = platform_get_drvdata(pdev);
+ *
+ *		drm_dev_unregister(drm);
+ *		drm_atomic_helper_shutdown(drm)
+ *
+ *		return 0;
+ *	}
+ *
+ *	static void driver_shutdown(struct platform_device *pdev)
+ *	{
+ *		drm_atomic_helper_shutdown(platform_get_drvdata(pdev));
+ *	}
+ *
+ *	static int __maybe_unused driver_pm_suspend(struct device *dev)
+ *	{
+ *		return drm_mode_config_helper_suspend(dev_get_drvdata(dev));
+ *	}
+ *
+ *	static int __maybe_unused driver_pm_resume(struct device *dev)
+ *	{
+ *		drm_mode_config_helper_resume(dev_get_drvdata(dev));
+ *
+ *		return 0;
+ *	}
+ *
+ *	static const struct dev_pm_ops driver_pm_ops = {
+ *		SET_SYSTEM_SLEEP_PM_OPS(driver_pm_suspend, driver_pm_resume)
+ *	};
+ *
+ *	static struct platform_driver driver_driver = {
+ *		.driver = {
+ *			[...]
+ *			.pm = &driver_pm_ops,
+ *		},
+ *		.probe = driver_probe,
+ *		.remove = driver_remove,
+ *		.shutdown = driver_shutdown,
+ *	};
+ *	module_platform_driver(driver_driver);
+ *
  */
 
 /**
-- 
2.20.1

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

[PATCH v2 04/12] drm/modes: Add DRM_SIMPLE_MODE()

[Noralf Trønnes]

This adds a helper macro to specify modes that only contain info about
resolution.

v2: Actually set the width and height (Ilia Mirkin)

Cc: Ilia Mirkin <imirkin@alum.mit.edu>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/tinydrm/hx8357d.c  |  2 +-
 drivers/gpu/drm/tinydrm/ili9225.c  |  2 +-
 drivers/gpu/drm/tinydrm/ili9341.c  |  2 +-
 drivers/gpu/drm/tinydrm/mi0283qt.c |  2 +-
 drivers/gpu/drm/tinydrm/repaper.c  |  8 ++++----
 drivers/gpu/drm/tinydrm/st7586.c   |  2 +-
 drivers/gpu/drm/tinydrm/st7735r.c  |  2 +-
 include/drm/drm_modes.h            | 17 +++++++++++++++++
 include/drm/tinydrm/tinydrm.h      | 23 -----------------------
 9 files changed, 27 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/tinydrm/hx8357d.c b/drivers/gpu/drm/tinydrm/hx8357d.c
index 8bbd0beafc6a..5a1ec0451c19 100644
--- a/drivers/gpu/drm/tinydrm/hx8357d.c
+++ b/drivers/gpu/drm/tinydrm/hx8357d.c
@@ -181,7 +181,7 @@ static const struct drm_simple_display_pipe_funcs hx8357d_pipe_funcs = {
 };
 
 static const struct drm_display_mode yx350hv15_mode = {
-	TINYDRM_MODE(320, 480, 60, 75),
+	DRM_SIMPLE_MODE(320, 480, 60, 75),
 };
 
 DEFINE_DRM_GEM_CMA_FOPS(hx8357d_fops);
diff --git a/drivers/gpu/drm/tinydrm/ili9225.c b/drivers/gpu/drm/tinydrm/ili9225.c
index 43a3b68d90a2..d40814d370e2 100644
--- a/drivers/gpu/drm/tinydrm/ili9225.c
+++ b/drivers/gpu/drm/tinydrm/ili9225.c
@@ -332,7 +332,7 @@ static const struct drm_simple_display_pipe_funcs ili9225_pipe_funcs = {
 };
 
 static const struct drm_display_mode ili9225_mode = {
-	TINYDRM_MODE(176, 220, 35, 44),
+	DRM_SIMPLE_MODE(176, 220, 35, 44),
 };
 
 DEFINE_DRM_GEM_CMA_FOPS(ili9225_fops);
diff --git a/drivers/gpu/drm/tinydrm/ili9341.c b/drivers/gpu/drm/tinydrm/ili9341.c
index 713bb2dd7e04..063f4f07f811 100644
--- a/drivers/gpu/drm/tinydrm/ili9341.c
+++ b/drivers/gpu/drm/tinydrm/ili9341.c
@@ -137,7 +137,7 @@ static const struct drm_simple_display_pipe_funcs ili9341_pipe_funcs = {
 };
 
 static const struct drm_display_mode yx240qv29_mode = {
-	TINYDRM_MODE(240, 320, 37, 49),
+	DRM_SIMPLE_MODE(240, 320, 37, 49),
 };
 
 DEFINE_DRM_GEM_CMA_FOPS(ili9341_fops);
diff --git a/drivers/gpu/drm/tinydrm/mi0283qt.c b/drivers/gpu/drm/tinydrm/mi0283qt.c
index 82a92ec9ae3c..3d067c2ba1bc 100644
--- a/drivers/gpu/drm/tinydrm/mi0283qt.c
+++ b/drivers/gpu/drm/tinydrm/mi0283qt.c
@@ -145,7 +145,7 @@ static const struct drm_simple_display_pipe_funcs mi0283qt_pipe_funcs = {
 };
 
 static const struct drm_display_mode mi0283qt_mode = {
-	TINYDRM_MODE(320, 240, 58, 43),
+	DRM_SIMPLE_MODE(320, 240, 58, 43),
 };
 
 DEFINE_DRM_GEM_CMA_FOPS(mi0283qt_fops);
diff --git a/drivers/gpu/drm/tinydrm/repaper.c b/drivers/gpu/drm/tinydrm/repaper.c
index b037c6540cf3..72d30151ecd8 100644
--- a/drivers/gpu/drm/tinydrm/repaper.c
+++ b/drivers/gpu/drm/tinydrm/repaper.c
@@ -860,28 +860,28 @@ static const uint32_t repaper_formats[] = {
 };
 
 static const struct drm_display_mode repaper_e1144cs021_mode = {
-	TINYDRM_MODE(128, 96, 29, 22),
+	DRM_SIMPLE_MODE(128, 96, 29, 22),
 };
 
 static const u8 repaper_e1144cs021_cs[] = { 0x00, 0x00, 0x00, 0x00,
 					    0x00, 0x0f, 0xff, 0x00 };
 
 static const struct drm_display_mode repaper_e1190cs021_mode = {
-	TINYDRM_MODE(144, 128, 36, 32),
+	DRM_SIMPLE_MODE(144, 128, 36, 32),
 };
 
 static const u8 repaper_e1190cs021_cs[] = { 0x00, 0x00, 0x00, 0x03,
 					    0xfc, 0x00, 0x00, 0xff };
 
 static const struct drm_display_mode repaper_e2200cs021_mode = {
-	TINYDRM_MODE(200, 96, 46, 22),
+	DRM_SIMPLE_MODE(200, 96, 46, 22),
 };
 
 static const u8 repaper_e2200cs021_cs[] = { 0x00, 0x00, 0x00, 0x00,
 					    0x01, 0xff, 0xe0, 0x00 };
 
 static const struct drm_display_mode repaper_e2271cs021_mode = {
-	TINYDRM_MODE(264, 176, 57, 38),
+	DRM_SIMPLE_MODE(264, 176, 57, 38),
 };
 
 static const u8 repaper_e2271cs021_cs[] = { 0x00, 0x00, 0x00, 0x7f,
diff --git a/drivers/gpu/drm/tinydrm/st7586.c b/drivers/gpu/drm/tinydrm/st7586.c
index 01a8077954b3..5ee7db561349 100644
--- a/drivers/gpu/drm/tinydrm/st7586.c
+++ b/drivers/gpu/drm/tinydrm/st7586.c
@@ -312,7 +312,7 @@ static const struct drm_simple_display_pipe_funcs st7586_pipe_funcs = {
 };
 
 static const struct drm_display_mode st7586_mode = {
-	TINYDRM_MODE(178, 128, 37, 27),
+	DRM_SIMPLE_MODE(178, 128, 37, 27),
 };
 
 DEFINE_DRM_GEM_CMA_FOPS(st7586_fops);
diff --git a/drivers/gpu/drm/tinydrm/st7735r.c b/drivers/gpu/drm/tinydrm/st7735r.c
index 3bab9a9569a6..6c7904c205f0 100644
--- a/drivers/gpu/drm/tinydrm/st7735r.c
+++ b/drivers/gpu/drm/tinydrm/st7735r.c
@@ -111,7 +111,7 @@ static const struct drm_simple_display_pipe_funcs jd_t18003_t01_pipe_funcs = {
 };
 
 static const struct drm_display_mode jd_t18003_t01_mode = {
-	TINYDRM_MODE(128, 160, 28, 35),
+	DRM_SIMPLE_MODE(128, 160, 28, 35),
 };
 
 DEFINE_DRM_GEM_CMA_FOPS(st7735r_fops);
diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h
index be4fed97e727..083f16747369 100644
--- a/include/drm/drm_modes.h
+++ b/include/drm/drm_modes.h
@@ -138,6 +138,23 @@ enum drm_mode_status {
 	.vsync_start = (vss), .vsync_end = (vse), .vtotal = (vt), \
 	.vscan = (vs), .flags = (f)
 
+/**
+ * DRM_SIMPLE_MODE - Simple display mode
+ * @hd: Horizontal resolution, width
+ * @vd: Vertical resolution, height
+ * @hd_mm: Display width in millimeters
+ * @vd_mm: Display height in millimeters
+ *
+ * This macro initializes a &drm_display_mode that only contains info about
+ * resolution and physical size.
+ */
+#define DRM_SIMPLE_MODE(hd, vd, hd_mm, vd_mm) \
+	.type = DRM_MODE_TYPE_DRIVER, .clock = 1 /* pass validation */, \
+	.hdisplay = (hd), .hsync_start = (hd), .hsync_end = (hd), \
+	.htotal = (hd), .vdisplay = (vd), .vsync_start = (vd), \
+	.vsync_end = (vd), .vtotal = (vd), .width_mm = (hd_mm), \
+	.height_mm = (vd_mm)
+
 #define CRTC_INTERLACE_HALVE_V	(1 << 0) /* halve V values for interlacing */
 #define CRTC_STEREO_DOUBLE	(1 << 1) /* adjust timings for stereo modes */
 #define CRTC_NO_DBLSCAN		(1 << 2) /* don't adjust doublescan */
diff --git a/include/drm/tinydrm/tinydrm.h b/include/drm/tinydrm/tinydrm.h
index 5621688edcc0..87e7f9b93a37 100644
--- a/include/drm/tinydrm/tinydrm.h
+++ b/include/drm/tinydrm/tinydrm.h
@@ -35,29 +35,6 @@ pipe_to_tinydrm(struct drm_simple_display_pipe *pipe)
 	return container_of(pipe, struct tinydrm_device, pipe);
 }
 
-/**
- * TINYDRM_MODE - tinydrm display mode
- * @hd: Horizontal resolution, width
- * @vd: Vertical resolution, height
- * @hd_mm: Display width in millimeters
- * @vd_mm: Display height in millimeters
- *
- * This macro creates a &drm_display_mode for use with tinydrm.
- */
-#define TINYDRM_MODE(hd, vd, hd_mm, vd_mm) \
-	.hdisplay = (hd), \
-	.hsync_start = (hd), \
-	.hsync_end = (hd), \
-	.htotal = (hd), \
-	.vdisplay = (vd), \
-	.vsync_start = (vd), \
-	.vsync_end = (vd), \
-	.vtotal = (vd), \
-	.width_mm = (hd_mm), \
-	.height_mm = (vd_mm), \
-	.type = DRM_MODE_TYPE_DRIVER, \
-	.clock = 1 /* pass validation */
-
 int devm_tinydrm_init(struct device *parent, struct tinydrm_device *tdev,
 		      struct drm_driver *driver);
 int devm_tinydrm_register(struct tinydrm_device *tdev);
-- 
2.20.1

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

[PATCH v2 05/12] drm/tinydrm: tinydrm_display_pipe_init() don't use tinydrm_device

[Noralf Trønnes]

Rework function signature so tinydrm_device can be avoided.

Move definition to tinydrm-helpers.h so tinydrm.h can be deleted in a
later patch.

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c | 24 ++++++++++-----------
 drivers/gpu/drm/tinydrm/mipi-dbi.c          |  2 +-
 drivers/gpu/drm/tinydrm/repaper.c           |  2 +-
 drivers/gpu/drm/tinydrm/st7586.c            |  2 +-
 include/drm/tinydrm/tinydrm-helpers.h       | 13 +++++++++++
 include/drm/tinydrm/tinydrm.h               |  9 --------
 6 files changed, 28 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c b/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c
index bb5b1c1e21ba..bb8a7ed8ddf6 100644
--- a/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c
+++ b/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c
@@ -13,7 +13,7 @@
 #include <drm/drm_modes.h>
 #include <drm/drm_probe_helper.h>
 #include <drm/drm_print.h>
-#include <drm/tinydrm/tinydrm.h>
+#include <drm/drm_simple_kms_helper.h>
 
 struct tinydrm_connector {
 	struct drm_connector base;
@@ -129,7 +129,8 @@ static int tinydrm_rotate_mode(struct drm_display_mode *mode,
 
 /**
  * tinydrm_display_pipe_init - Initialize display pipe
- * @tdev: tinydrm device
+ * @drm: DRM device
+ * @pipe: Display pipe
  * @funcs: Display pipe functions
  * @connector_type: Connector type
  * @formats: Array of supported formats (DRM_FORMAT\_\*)
@@ -143,16 +144,15 @@ static int tinydrm_rotate_mode(struct drm_display_mode *mode,
  * Returns:
  * Zero on success, negative error code on failure.
  */
-int
-tinydrm_display_pipe_init(struct tinydrm_device *tdev,
-			  const struct drm_simple_display_pipe_funcs *funcs,
-			  int connector_type,
-			  const uint32_t *formats,
-			  unsigned int format_count,
-			  const struct drm_display_mode *mode,
-			  unsigned int rotation)
+int tinydrm_display_pipe_init(struct drm_device *drm,
+			      struct drm_simple_display_pipe *pipe,
+			      const struct drm_simple_display_pipe_funcs *funcs,
+			      int connector_type,
+			      const uint32_t *formats,
+			      unsigned int format_count,
+			      const struct drm_display_mode *mode,
+			      unsigned int rotation)
 {
-	struct drm_device *drm = tdev->drm;
 	struct drm_display_mode mode_copy;
 	struct drm_connector *connector;
 	int ret;
@@ -177,7 +177,7 @@ tinydrm_display_pipe_init(struct tinydrm_device *tdev,
 	if (IS_ERR(connector))
 		return PTR_ERR(connector);
 
-	return drm_simple_display_pipe_init(drm, &tdev->pipe, funcs, formats,
+	return drm_simple_display_pipe_init(drm, pipe, funcs, formats,
 					    format_count, modifiers, connector);
 }
 EXPORT_SYMBOL(tinydrm_display_pipe_init);
diff --git a/drivers/gpu/drm/tinydrm/mipi-dbi.c b/drivers/gpu/drm/tinydrm/mipi-dbi.c
index 918f77c7de34..6ce7c01522ad 100644
--- a/drivers/gpu/drm/tinydrm/mipi-dbi.c
+++ b/drivers/gpu/drm/tinydrm/mipi-dbi.c
@@ -407,7 +407,7 @@ int mipi_dbi_init(struct device *dev, struct mipi_dbi *mipi,
 		return ret;
 
 	/* TODO: Maybe add DRM_MODE_CONNECTOR_SPI */
-	ret = tinydrm_display_pipe_init(tdev, pipe_funcs,
+	ret = tinydrm_display_pipe_init(tdev->drm, &tdev->pipe, pipe_funcs,
 					DRM_MODE_CONNECTOR_VIRTUAL,
 					mipi_dbi_formats,
 					ARRAY_SIZE(mipi_dbi_formats), mode,
diff --git a/drivers/gpu/drm/tinydrm/repaper.c b/drivers/gpu/drm/tinydrm/repaper.c
index 72d30151ecd8..0f80f4e41215 100644
--- a/drivers/gpu/drm/tinydrm/repaper.c
+++ b/drivers/gpu/drm/tinydrm/repaper.c
@@ -1069,7 +1069,7 @@ static int repaper_probe(struct spi_device *spi)
 	if (ret)
 		return ret;
 
-	ret = tinydrm_display_pipe_init(tdev, &repaper_pipe_funcs,
+	ret = tinydrm_display_pipe_init(tdev->drm, &tdev->pipe, &repaper_pipe_funcs,
 					DRM_MODE_CONNECTOR_VIRTUAL,
 					repaper_formats,
 					ARRAY_SIZE(repaper_formats), mode, 0);
diff --git a/drivers/gpu/drm/tinydrm/st7586.c b/drivers/gpu/drm/tinydrm/st7586.c
index 5ee7db561349..eb66e36bcd71 100644
--- a/drivers/gpu/drm/tinydrm/st7586.c
+++ b/drivers/gpu/drm/tinydrm/st7586.c
@@ -283,7 +283,7 @@ static int st7586_init(struct device *dev, struct mipi_dbi *mipi,
 	if (ret)
 		return ret;
 
-	ret = tinydrm_display_pipe_init(tdev, pipe_funcs,
+	ret = tinydrm_display_pipe_init(tdev->drm, &tdev->pipe, pipe_funcs,
 					DRM_MODE_CONNECTOR_VIRTUAL,
 					st7586_formats,
 					ARRAY_SIZE(st7586_formats),
diff --git a/include/drm/tinydrm/tinydrm-helpers.h b/include/drm/tinydrm/tinydrm-helpers.h
index f0d598789e4d..ae4a6abc43b5 100644
--- a/include/drm/tinydrm/tinydrm-helpers.h
+++ b/include/drm/tinydrm/tinydrm-helpers.h
@@ -11,8 +11,12 @@
 #define __LINUX_TINYDRM_HELPERS_H
 
 struct backlight_device;
+struct drm_device;
+struct drm_display_mode;
 struct drm_framebuffer;
 struct drm_rect;
+struct drm_simple_display_pipe;
+struct drm_simple_display_pipe_funcs;
 struct spi_transfer;
 struct spi_message;
 struct spi_device;
@@ -33,6 +37,15 @@ static inline bool tinydrm_machine_little_endian(void)
 #endif
 }
 
+int tinydrm_display_pipe_init(struct drm_device *drm,
+			      struct drm_simple_display_pipe *pipe,
+			      const struct drm_simple_display_pipe_funcs *funcs,
+			      int connector_type,
+			      const uint32_t *formats,
+			      unsigned int format_count,
+			      const struct drm_display_mode *mode,
+			      unsigned int rotation);
+
 void tinydrm_memcpy(void *dst, void *vaddr, struct drm_framebuffer *fb,
 		    struct drm_rect *clip);
 void tinydrm_swab16(u16 *dst, void *vaddr, struct drm_framebuffer *fb,
diff --git a/include/drm/tinydrm/tinydrm.h b/include/drm/tinydrm/tinydrm.h
index 87e7f9b93a37..69c4363fd88e 100644
--- a/include/drm/tinydrm/tinydrm.h
+++ b/include/drm/tinydrm/tinydrm.h
@@ -40,13 +40,4 @@ int devm_tinydrm_init(struct device *parent, struct tinydrm_device *tdev,
 int devm_tinydrm_register(struct tinydrm_device *tdev);
 void tinydrm_shutdown(struct tinydrm_device *tdev);
 
-int
-tinydrm_display_pipe_init(struct tinydrm_device *tdev,
-			  const struct drm_simple_display_pipe_funcs *funcs,
-			  int connector_type,
-			  const uint32_t *formats,
-			  unsigned int format_count,
-			  const struct drm_display_mode *mode,
-			  unsigned int rotation);
-
 #endif /* __LINUX_TINYDRM_H */
-- 
2.20.1

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

[PATCH v2 06/12] drm/tinydrm: Remove tinydrm_shutdown()

[Noralf Trønnes]

It's just a wrapper around drm_atomic_helper_shutdown() now.
Also store drm_device in the drvdata field, since that's what's used.

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
---
 drivers/gpu/drm/tinydrm/core/tinydrm-core.c | 14 --------------
 drivers/gpu/drm/tinydrm/hx8357d.c           |  7 +++----
 drivers/gpu/drm/tinydrm/ili9225.c           |  7 +++----
 drivers/gpu/drm/tinydrm/ili9341.c           |  7 +++----
 drivers/gpu/drm/tinydrm/mi0283qt.c          | 15 +++++----------
 drivers/gpu/drm/tinydrm/repaper.c           |  8 ++++----
 drivers/gpu/drm/tinydrm/st7586.c            |  7 +++----
 drivers/gpu/drm/tinydrm/st7735r.c           |  7 +++----
 include/drm/tinydrm/tinydrm.h               |  1 -
 9 files changed, 24 insertions(+), 49 deletions(-)

diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-core.c b/drivers/gpu/drm/tinydrm/core/tinydrm-core.c
index 554abd5d3b53..2366a33fd62f 100644
--- a/drivers/gpu/drm/tinydrm/core/tinydrm-core.c
+++ b/drivers/gpu/drm/tinydrm/core/tinydrm-core.c
@@ -166,18 +166,4 @@ int devm_tinydrm_register(struct tinydrm_device *tdev)
 }
 EXPORT_SYMBOL(devm_tinydrm_register);
 
-/**
- * tinydrm_shutdown - Shutdown tinydrm
- * @tdev: tinydrm device
- *
- * This function makes sure that the display pipeline is disabled.
- * Used by drivers in their shutdown callback to turn off the display
- * on machine shutdown and reboot.
- */
-void tinydrm_shutdown(struct tinydrm_device *tdev)
-{
-	drm_atomic_helper_shutdown(tdev->drm);
-}
-EXPORT_SYMBOL(tinydrm_shutdown);
-
 MODULE_LICENSE("GPL");
diff --git a/drivers/gpu/drm/tinydrm/hx8357d.c b/drivers/gpu/drm/tinydrm/hx8357d.c
index 5a1ec0451c19..cb0ea98306a0 100644
--- a/drivers/gpu/drm/tinydrm/hx8357d.c
+++ b/drivers/gpu/drm/tinydrm/hx8357d.c
@@ -16,6 +16,7 @@
 #include <linux/property.h>
 #include <linux/spi/spi.h>
 
+#include <drm/drm_atomic_helper.h>
 #include <drm/drm_drv.h>
 #include <drm/drm_gem_cma_helper.h>
 #include <drm/drm_gem_framebuffer_helper.h>
@@ -243,16 +244,14 @@ static int hx8357d_probe(struct spi_device *spi)
 	if (ret)
 		return ret;
 
-	spi_set_drvdata(spi, mipi);
+	spi_set_drvdata(spi, mipi->tinydrm.drm);
 
 	return devm_tinydrm_register(&mipi->tinydrm);
 }
 
 static void hx8357d_shutdown(struct spi_device *spi)
 {
-	struct mipi_dbi *mipi = spi_get_drvdata(spi);
-
-	tinydrm_shutdown(&mipi->tinydrm);
+	drm_atomic_helper_shutdown(spi_get_drvdata(spi));
 }
 
 static struct spi_driver hx8357d_spi_driver = {
diff --git a/drivers/gpu/drm/tinydrm/ili9225.c b/drivers/gpu/drm/tinydrm/ili9225.c
index d40814d370e2..7837b785a993 100644
--- a/drivers/gpu/drm/tinydrm/ili9225.c
+++ b/drivers/gpu/drm/tinydrm/ili9225.c
@@ -20,6 +20,7 @@
 #include <linux/spi/spi.h>
 #include <video/mipi_display.h>
 
+#include <drm/drm_atomic_helper.h>
 #include <drm/drm_damage_helper.h>
 #include <drm/drm_drv.h>
 #include <drm/drm_fb_cma_helper.h>
@@ -399,16 +400,14 @@ static int ili9225_probe(struct spi_device *spi)
 	if (ret)
 		return ret;
 
-	spi_set_drvdata(spi, mipi);
+	spi_set_drvdata(spi, mipi->tinydrm.drm);
 
 	return devm_tinydrm_register(&mipi->tinydrm);
 }
 
 static void ili9225_shutdown(struct spi_device *spi)
 {
-	struct mipi_dbi *mipi = spi_get_drvdata(spi);
-
-	tinydrm_shutdown(&mipi->tinydrm);
+	drm_atomic_helper_shutdown(spi_get_drvdata(spi));
 }
 
 static struct spi_driver ili9225_spi_driver = {
diff --git a/drivers/gpu/drm/tinydrm/ili9341.c b/drivers/gpu/drm/tinydrm/ili9341.c
index 063f4f07f811..87400a970c1a 100644
--- a/drivers/gpu/drm/tinydrm/ili9341.c
+++ b/drivers/gpu/drm/tinydrm/ili9341.c
@@ -15,6 +15,7 @@
 #include <linux/property.h>
 #include <linux/spi/spi.h>
 
+#include <drm/drm_atomic_helper.h>
 #include <drm/drm_drv.h>
 #include <drm/drm_gem_cma_helper.h>
 #include <drm/drm_gem_framebuffer_helper.h>
@@ -205,16 +206,14 @@ static int ili9341_probe(struct spi_device *spi)
 	if (ret)
 		return ret;
 
-	spi_set_drvdata(spi, mipi);
+	spi_set_drvdata(spi, mipi->tinydrm.drm);
 
 	return devm_tinydrm_register(&mipi->tinydrm);
 }
 
 static void ili9341_shutdown(struct spi_device *spi)
 {
-	struct mipi_dbi *mipi = spi_get_drvdata(spi);
-
-	tinydrm_shutdown(&mipi->tinydrm);
+	drm_atomic_helper_shutdown(spi_get_drvdata(spi));
 }
 
 static struct spi_driver ili9341_spi_driver = {
diff --git a/drivers/gpu/drm/tinydrm/mi0283qt.c b/drivers/gpu/drm/tinydrm/mi0283qt.c
index 3d067c2ba1bc..1fa0cda0ddb5 100644
--- a/drivers/gpu/drm/tinydrm/mi0283qt.c
+++ b/drivers/gpu/drm/tinydrm/mi0283qt.c
@@ -17,6 +17,7 @@
 #include <linux/regulator/consumer.h>
 #include <linux/spi/spi.h>
 
+#include <drm/drm_atomic_helper.h>
 #include <drm/drm_drv.h>
 #include <drm/drm_gem_cma_helper.h>
 #include <drm/drm_gem_framebuffer_helper.h>
@@ -218,30 +219,24 @@ static int mi0283qt_probe(struct spi_device *spi)
 	if (ret)
 		return ret;
 
-	spi_set_drvdata(spi, mipi);
+	spi_set_drvdata(spi, mipi->tinydrm.drm);
 
 	return devm_tinydrm_register(&mipi->tinydrm);
 }
 
 static void mi0283qt_shutdown(struct spi_device *spi)
 {
-	struct mipi_dbi *mipi = spi_get_drvdata(spi);
-
-	tinydrm_shutdown(&mipi->tinydrm);
+	drm_atomic_helper_shutdown(spi_get_drvdata(spi));
 }
 
 static int __maybe_unused mi0283qt_pm_suspend(struct device *dev)
 {
-	struct mipi_dbi *mipi = dev_get_drvdata(dev);
-
-	return drm_mode_config_helper_suspend(mipi->tinydrm.drm);
+	return drm_mode_config_helper_suspend(dev_get_drvdata(dev));
 }
 
 static int __maybe_unused mi0283qt_pm_resume(struct device *dev)
 {
-	struct mipi_dbi *mipi = dev_get_drvdata(dev);
-
-	drm_mode_config_helper_resume(mipi->tinydrm.drm);
+	drm_mode_config_helper_resume(dev_get_drvdata(dev));
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/tinydrm/repaper.c b/drivers/gpu/drm/tinydrm/repaper.c
index 0f80f4e41215..a32dda09458a 100644
--- a/drivers/gpu/drm/tinydrm/repaper.c
+++ b/drivers/gpu/drm/tinydrm/repaper.c
@@ -26,6 +26,7 @@
 #include <linux/spi/spi.h>
 #include <linux/thermal.h>
 
+#include <drm/drm_atomic_helper.h>
 #include <drm/drm_damage_helper.h>
 #include <drm/drm_drv.h>
 #include <drm/drm_fb_cma_helper.h>
@@ -1077,7 +1078,8 @@ static int repaper_probe(struct spi_device *spi)
 		return ret;
 
 	drm_mode_config_reset(tdev->drm);
-	spi_set_drvdata(spi, tdev);
+
+	spi_set_drvdata(spi, tdev->drm);
 
 	DRM_DEBUG_DRIVER("SPI speed: %uMHz\n", spi->max_speed_hz / 1000000);
 
@@ -1086,9 +1088,7 @@ static int repaper_probe(struct spi_device *spi)
 
 static void repaper_shutdown(struct spi_device *spi)
 {
-	struct tinydrm_device *tdev = spi_get_drvdata(spi);
-
-	tinydrm_shutdown(tdev);
+	drm_atomic_helper_shutdown(spi_get_drvdata(spi));
 }
 
 static struct spi_driver repaper_spi_driver = {
diff --git a/drivers/gpu/drm/tinydrm/st7586.c b/drivers/gpu/drm/tinydrm/st7586.c
index eb66e36bcd71..20d79a1aa539 100644
--- a/drivers/gpu/drm/tinydrm/st7586.c
+++ b/drivers/gpu/drm/tinydrm/st7586.c
@@ -17,6 +17,7 @@
 #include <linux/spi/spi.h>
 #include <video/mipi_display.h>
 
+#include <drm/drm_atomic_helper.h>
 #include <drm/drm_damage_helper.h>
 #include <drm/drm_drv.h>
 #include <drm/drm_fb_cma_helper.h>
@@ -389,16 +390,14 @@ static int st7586_probe(struct spi_device *spi)
 	if (ret)
 		return ret;
 
-	spi_set_drvdata(spi, mipi);
+	spi_set_drvdata(spi, mipi->tinydrm.drm);
 
 	return devm_tinydrm_register(&mipi->tinydrm);
 }
 
 static void st7586_shutdown(struct spi_device *spi)
 {
-	struct mipi_dbi *mipi = spi_get_drvdata(spi);
-
-	tinydrm_shutdown(&mipi->tinydrm);
+	drm_atomic_helper_shutdown(spi_get_drvdata(spi));
 }
 
 static struct spi_driver st7586_spi_driver = {
diff --git a/drivers/gpu/drm/tinydrm/st7735r.c b/drivers/gpu/drm/tinydrm/st7735r.c
index 6c7904c205f0..9b47d8632f1a 100644
--- a/drivers/gpu/drm/tinydrm/st7735r.c
+++ b/drivers/gpu/drm/tinydrm/st7735r.c
@@ -14,6 +14,7 @@
 #include <linux/spi/spi.h>
 #include <video/mipi_display.h>
 
+#include <drm/drm_atomic_helper.h>
 #include <drm/drm_drv.h>
 #include <drm/drm_gem_cma_helper.h>
 #include <drm/drm_gem_framebuffer_helper.h>
@@ -183,16 +184,14 @@ static int st7735r_probe(struct spi_device *spi)
 	if (ret)
 		return ret;
 
-	spi_set_drvdata(spi, mipi);
+	spi_set_drvdata(spi, mipi->tinydrm.drm);
 
 	return devm_tinydrm_register(&mipi->tinydrm);
 }
 
 static void st7735r_shutdown(struct spi_device *spi)
 {
-	struct mipi_dbi *mipi = spi_get_drvdata(spi);
-
-	tinydrm_shutdown(&mipi->tinydrm);
+	drm_atomic_helper_shutdown(spi_get_drvdata(spi));
 }
 
 static struct spi_driver st7735r_spi_driver = {
diff --git a/include/drm/tinydrm/tinydrm.h b/include/drm/tinydrm/tinydrm.h
index 69c4363fd88e..ee9b17759391 100644
--- a/include/drm/tinydrm/tinydrm.h
+++ b/include/drm/tinydrm/tinydrm.h
@@ -38,6 +38,5 @@ pipe_to_tinydrm(struct drm_simple_display_pipe *pipe)
 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);
 
 #endif /* __LINUX_TINYDRM_H */
-- 
2.20.1

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

[PATCH v2 07/12] drm/tinydrm/mipi-dbi: Add drm_to_mipi_dbi()

[Noralf Trønnes]

Add a function to derive mipi_dbi from drm_device now that tinydrm_device
is going away.

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
---
 drivers/gpu/drm/tinydrm/hx8357d.c  |  3 +--
 drivers/gpu/drm/tinydrm/ili9225.c  | 11 ++++-------
 drivers/gpu/drm/tinydrm/ili9341.c  |  3 +--
 drivers/gpu/drm/tinydrm/mi0283qt.c |  3 +--
 drivers/gpu/drm/tinydrm/mipi-dbi.c |  9 +++------
 drivers/gpu/drm/tinydrm/st7586.c   |  9 +++------
 drivers/gpu/drm/tinydrm/st7735r.c  |  3 +--
 include/drm/tinydrm/mipi-dbi.h     |  5 +++--
 8 files changed, 17 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/tinydrm/hx8357d.c b/drivers/gpu/drm/tinydrm/hx8357d.c
index cb0ea98306a0..84dda622df85 100644
--- a/drivers/gpu/drm/tinydrm/hx8357d.c
+++ b/drivers/gpu/drm/tinydrm/hx8357d.c
@@ -47,8 +47,7 @@ static void yx240qv29_enable(struct drm_simple_display_pipe *pipe,
 			     struct drm_crtc_state *crtc_state,
 			     struct drm_plane_state *plane_state)
 {
-	struct tinydrm_device *tdev = pipe_to_tinydrm(pipe);
-	struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev);
+	struct mipi_dbi *mipi = drm_to_mipi_dbi(pipe->crtc.dev);
 	u8 addr_mode;
 	int ret;
 
diff --git a/drivers/gpu/drm/tinydrm/ili9225.c b/drivers/gpu/drm/tinydrm/ili9225.c
index 7837b785a993..3f59cfbd31ba 100644
--- a/drivers/gpu/drm/tinydrm/ili9225.c
+++ b/drivers/gpu/drm/tinydrm/ili9225.c
@@ -82,8 +82,7 @@ static inline int ili9225_command(struct mipi_dbi *mipi, u8 cmd, u16 data)
 static void ili9225_fb_dirty(struct drm_framebuffer *fb, struct drm_rect *rect)
 {
 	struct drm_gem_cma_object *cma_obj = drm_fb_cma_get_gem_obj(fb, 0);
-	struct tinydrm_device *tdev = fb->dev->dev_private;
-	struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev);
+	struct mipi_dbi *mipi = drm_to_mipi_dbi(fb->dev);
 	unsigned int height = rect->y2 - rect->y1;
 	unsigned int width = rect->x2 - rect->x1;
 	bool swap = mipi->swap_bytes;
@@ -182,10 +181,9 @@ static void ili9225_pipe_enable(struct drm_simple_display_pipe *pipe,
 				struct drm_crtc_state *crtc_state,
 				struct drm_plane_state *plane_state)
 {
-	struct tinydrm_device *tdev = pipe_to_tinydrm(pipe);
-	struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev);
+	struct mipi_dbi *mipi = drm_to_mipi_dbi(pipe->crtc.dev);
 	struct drm_framebuffer *fb = plane_state->fb;
-	struct device *dev = tdev->drm->dev;
+	struct device *dev = pipe->crtc.dev->dev;
 	struct drm_rect rect = {
 		.x1 = 0,
 		.x2 = fb->width,
@@ -285,8 +283,7 @@ static void ili9225_pipe_enable(struct drm_simple_display_pipe *pipe,
 
 static void ili9225_pipe_disable(struct drm_simple_display_pipe *pipe)
 {
-	struct tinydrm_device *tdev = pipe_to_tinydrm(pipe);
-	struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev);
+	struct mipi_dbi *mipi = drm_to_mipi_dbi(pipe->crtc.dev);
 
 	DRM_DEBUG_KMS("\n");
 
diff --git a/drivers/gpu/drm/tinydrm/ili9341.c b/drivers/gpu/drm/tinydrm/ili9341.c
index 87400a970c1a..3b7565a6311e 100644
--- a/drivers/gpu/drm/tinydrm/ili9341.c
+++ b/drivers/gpu/drm/tinydrm/ili9341.c
@@ -53,8 +53,7 @@ static void yx240qv29_enable(struct drm_simple_display_pipe *pipe,
 			     struct drm_crtc_state *crtc_state,
 			     struct drm_plane_state *plane_state)
 {
-	struct tinydrm_device *tdev = pipe_to_tinydrm(pipe);
-	struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev);
+	struct mipi_dbi *mipi = drm_to_mipi_dbi(pipe->crtc.dev);
 	u8 addr_mode;
 	int ret;
 
diff --git a/drivers/gpu/drm/tinydrm/mi0283qt.c b/drivers/gpu/drm/tinydrm/mi0283qt.c
index 1fa0cda0ddb5..da326fd1450d 100644
--- a/drivers/gpu/drm/tinydrm/mi0283qt.c
+++ b/drivers/gpu/drm/tinydrm/mi0283qt.c
@@ -55,8 +55,7 @@ static void mi0283qt_enable(struct drm_simple_display_pipe *pipe,
 			    struct drm_crtc_state *crtc_state,
 			    struct drm_plane_state *plane_state)
 {
-	struct tinydrm_device *tdev = pipe_to_tinydrm(pipe);
-	struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev);
+	struct mipi_dbi *mipi = drm_to_mipi_dbi(pipe->crtc.dev);
 	u8 addr_mode;
 	int ret;
 
diff --git a/drivers/gpu/drm/tinydrm/mipi-dbi.c b/drivers/gpu/drm/tinydrm/mipi-dbi.c
index 6ce7c01522ad..045b840812c3 100644
--- a/drivers/gpu/drm/tinydrm/mipi-dbi.c
+++ b/drivers/gpu/drm/tinydrm/mipi-dbi.c
@@ -216,8 +216,7 @@ EXPORT_SYMBOL(mipi_dbi_buf_copy);
 static void mipi_dbi_fb_dirty(struct drm_framebuffer *fb, struct drm_rect *rect)
 {
 	struct drm_gem_cma_object *cma_obj = drm_fb_cma_get_gem_obj(fb, 0);
-	struct tinydrm_device *tdev = fb->dev->dev_private;
-	struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev);
+	struct mipi_dbi *mipi = drm_to_mipi_dbi(fb->dev);
 	unsigned int height = rect->y2 - rect->y1;
 	unsigned int width = rect->x2 - rect->x1;
 	bool swap = mipi->swap_bytes;
@@ -342,8 +341,7 @@ static void mipi_dbi_blank(struct mipi_dbi *mipi)
  */
 void mipi_dbi_pipe_disable(struct drm_simple_display_pipe *pipe)
 {
-	struct tinydrm_device *tdev = pipe_to_tinydrm(pipe);
-	struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev);
+	struct mipi_dbi *mipi = drm_to_mipi_dbi(pipe->crtc.dev);
 
 	DRM_DEBUG_KMS("\n");
 
@@ -1088,8 +1086,7 @@ static const struct file_operations mipi_dbi_debugfs_command_fops = {
  */
 int mipi_dbi_debugfs_init(struct drm_minor *minor)
 {
-	struct tinydrm_device *tdev = minor->dev->dev_private;
-	struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev);
+	struct mipi_dbi *mipi = drm_to_mipi_dbi(minor->dev);
 	umode_t mode = S_IFREG | S_IWUSR;
 
 	if (mipi->read_commands)
diff --git a/drivers/gpu/drm/tinydrm/st7586.c b/drivers/gpu/drm/tinydrm/st7586.c
index 20d79a1aa539..6097585913f2 100644
--- a/drivers/gpu/drm/tinydrm/st7586.c
+++ b/drivers/gpu/drm/tinydrm/st7586.c
@@ -117,8 +117,7 @@ static int st7586_buf_copy(void *dst, struct drm_framebuffer *fb,
 
 static void st7586_fb_dirty(struct drm_framebuffer *fb, struct drm_rect *rect)
 {
-	struct tinydrm_device *tdev = fb->dev->dev_private;
-	struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev);
+	struct mipi_dbi *mipi = drm_to_mipi_dbi(fb->dev);
 	int start, end;
 	int ret = 0;
 
@@ -176,8 +175,7 @@ static void st7586_pipe_enable(struct drm_simple_display_pipe *pipe,
 			       struct drm_crtc_state *crtc_state,
 			       struct drm_plane_state *plane_state)
 {
-	struct tinydrm_device *tdev = pipe_to_tinydrm(pipe);
-	struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev);
+	struct mipi_dbi *mipi = drm_to_mipi_dbi(pipe->crtc.dev);
 	struct drm_framebuffer *fb = plane_state->fb;
 	struct drm_rect rect = {
 		.x1 = 0,
@@ -249,8 +247,7 @@ static void st7586_pipe_enable(struct drm_simple_display_pipe *pipe,
 
 static void st7586_pipe_disable(struct drm_simple_display_pipe *pipe)
 {
-	struct tinydrm_device *tdev = pipe_to_tinydrm(pipe);
-	struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev);
+	struct mipi_dbi *mipi = drm_to_mipi_dbi(pipe->crtc.dev);
 
 	DRM_DEBUG_KMS("\n");
 
diff --git a/drivers/gpu/drm/tinydrm/st7735r.c b/drivers/gpu/drm/tinydrm/st7735r.c
index 9b47d8632f1a..bfa7e2221540 100644
--- a/drivers/gpu/drm/tinydrm/st7735r.c
+++ b/drivers/gpu/drm/tinydrm/st7735r.c
@@ -42,8 +42,7 @@ static void jd_t18003_t01_pipe_enable(struct drm_simple_display_pipe *pipe,
 				      struct drm_crtc_state *crtc_state,
 				      struct drm_plane_state *plane_state)
 {
-	struct tinydrm_device *tdev = pipe_to_tinydrm(pipe);
-	struct mipi_dbi *mipi = mipi_dbi_from_tinydrm(tdev);
+	struct mipi_dbi *mipi = drm_to_mipi_dbi(pipe->crtc.dev);
 	int ret;
 	u8 addr_mode;
 
diff --git a/include/drm/tinydrm/mipi-dbi.h b/include/drm/tinydrm/mipi-dbi.h
index f4ec2834bc22..ad7e6bedab5f 100644
--- a/include/drm/tinydrm/mipi-dbi.h
+++ b/include/drm/tinydrm/mipi-dbi.h
@@ -56,9 +56,10 @@ struct mipi_dbi {
 	struct regulator *regulator;
 };
 
-static inline struct mipi_dbi *
-mipi_dbi_from_tinydrm(struct tinydrm_device *tdev)
+static inline struct mipi_dbi *drm_to_mipi_dbi(struct drm_device *drm)
 {
+	struct tinydrm_device *tdev = drm->dev_private;
+
 	return container_of(tdev, struct mipi_dbi, tinydrm);
 }
 
-- 
2.20.1

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

[PATCH v2 08/12] drm/tinydrm/repaper: Drop using tinydrm_device

[Noralf Trønnes]

Use devm_drm_dev_init() and drop using tinydrm_device.

v2: devm_drm_dev_register() was dropped so add a driver release callback.

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/tinydrm/repaper.c | 84 ++++++++++++++++++++++---------
 1 file changed, 61 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/tinydrm/repaper.c b/drivers/gpu/drm/tinydrm/repaper.c
index a32dda09458a..b1acf5aebe32 100644
--- a/drivers/gpu/drm/tinydrm/repaper.c
+++ b/drivers/gpu/drm/tinydrm/repaper.c
@@ -30,11 +30,12 @@
 #include <drm/drm_damage_helper.h>
 #include <drm/drm_drv.h>
 #include <drm/drm_fb_cma_helper.h>
+#include <drm/drm_fb_helper.h>
 #include <drm/drm_gem_cma_helper.h>
 #include <drm/drm_gem_framebuffer_helper.h>
 #include <drm/drm_rect.h>
 #include <drm/drm_vblank.h>
-#include <drm/tinydrm/tinydrm.h>
+#include <drm/drm_simple_kms_helper.h>
 #include <drm/tinydrm/tinydrm-helpers.h>
 
 #define REPAPER_RID_G2_COG_ID	0x12
@@ -60,7 +61,8 @@ enum repaper_epd_border_byte {
 };
 
 struct repaper_epd {
-	struct tinydrm_device tinydrm;
+	struct drm_device drm;
+	struct drm_simple_display_pipe pipe;
 	struct spi_device *spi;
 
 	struct gpio_desc *panel_on;
@@ -89,10 +91,9 @@ struct repaper_epd {
 	bool partial;
 };
 
-static inline struct repaper_epd *
-epd_from_tinydrm(struct tinydrm_device *tdev)
+static inline struct repaper_epd *drm_to_epd(struct drm_device *drm)
 {
-	return container_of(tdev, struct repaper_epd, tinydrm);
+	return container_of(drm, struct repaper_epd, drm);
 }
 
 static int repaper_spi_transfer(struct spi_device *spi, u8 header,
@@ -530,8 +531,7 @@ static int repaper_fb_dirty(struct drm_framebuffer *fb)
 {
 	struct drm_gem_cma_object *cma_obj = drm_fb_cma_get_gem_obj(fb, 0);
 	struct dma_buf_attachment *import_attach = cma_obj->base.import_attach;
-	struct tinydrm_device *tdev = fb->dev->dev_private;
-	struct repaper_epd *epd = epd_from_tinydrm(tdev);
+	struct repaper_epd *epd = drm_to_epd(fb->dev);
 	struct drm_rect clip;
 	u8 *buf = NULL;
 	int ret = 0;
@@ -646,8 +646,7 @@ static void repaper_pipe_enable(struct drm_simple_display_pipe *pipe,
 				struct drm_crtc_state *crtc_state,
 				struct drm_plane_state *plane_state)
 {
-	struct tinydrm_device *tdev = pipe_to_tinydrm(pipe);
-	struct repaper_epd *epd = epd_from_tinydrm(tdev);
+	struct repaper_epd *epd = drm_to_epd(pipe->crtc.dev);
 	struct spi_device *spi = epd->spi;
 	struct device *dev = &spi->dev;
 	bool dc_ok = false;
@@ -781,8 +780,7 @@ static void repaper_pipe_enable(struct drm_simple_display_pipe *pipe,
 
 static void repaper_pipe_disable(struct drm_simple_display_pipe *pipe)
 {
-	struct tinydrm_device *tdev = pipe_to_tinydrm(pipe);
-	struct repaper_epd *epd = epd_from_tinydrm(tdev);
+	struct repaper_epd *epd = drm_to_epd(pipe->crtc.dev);
 	struct spi_device *spi = epd->spi;
 	unsigned int line;
 
@@ -856,6 +854,23 @@ static const struct drm_simple_display_pipe_funcs repaper_pipe_funcs = {
 	.prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb,
 };
 
+static const struct drm_mode_config_funcs repaper_mode_config_funcs = {
+	.fb_create = drm_gem_fb_create_with_dirty,
+	.atomic_check = drm_atomic_helper_check,
+	.atomic_commit = drm_atomic_helper_commit,
+};
+
+static void repaper_release(struct drm_device *drm)
+{
+	struct repaper_epd *epd = drm_to_epd(drm);
+
+	DRM_DEBUG_DRIVER("\n");
+
+	drm_mode_config_cleanup(drm);
+	drm_dev_fini(drm);
+	kfree(epd);
+}
+
 static const uint32_t repaper_formats[] = {
 	DRM_FORMAT_XRGB8888,
 };
@@ -894,6 +909,7 @@ static struct drm_driver repaper_driver = {
 	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME |
 				  DRIVER_ATOMIC,
 	.fops			= &repaper_fops,
+	.release		= repaper_release,
 	DRM_GEM_CMA_VMAP_DRIVER_OPS,
 	.name			= "repaper",
 	.desc			= "Pervasive Displays RePaper e-ink panels",
@@ -926,11 +942,11 @@ static int repaper_probe(struct spi_device *spi)
 	const struct spi_device_id *spi_id;
 	const struct of_device_id *match;
 	struct device *dev = &spi->dev;
-	struct tinydrm_device *tdev;
 	enum repaper_model model;
 	const char *thermal_zone;
 	struct repaper_epd *epd;
 	size_t line_buffer_size;
+	struct drm_device *drm;
 	int ret;
 
 	match = of_match_device(repaper_of_match, dev);
@@ -950,10 +966,21 @@ static int repaper_probe(struct spi_device *spi)
 		}
 	}
 
-	epd = devm_kzalloc(dev, sizeof(*epd), GFP_KERNEL);
+	epd = kzalloc(sizeof(*epd), GFP_KERNEL);
 	if (!epd)
 		return -ENOMEM;
 
+	drm = &epd->drm;
+
+	ret = devm_drm_dev_init(dev, drm, &repaper_driver);
+	if (ret) {
+		kfree(epd);
+		return ret;
+	}
+
+	drm_mode_config_init(drm);
+	drm->mode_config.funcs = &repaper_mode_config_funcs;
+
 	epd->spi = spi;
 
 	epd->panel_on = devm_gpiod_get(dev, "panel-on", GPIOD_OUT_LOW);
@@ -1064,26 +1091,36 @@ static int repaper_probe(struct spi_device *spi)
 	if (!epd->current_frame)
 		return -ENOMEM;
 
-	tdev = &epd->tinydrm;
-
-	ret = devm_tinydrm_init(dev, tdev, &repaper_driver);
-	if (ret)
-		return ret;
-
-	ret = tinydrm_display_pipe_init(tdev->drm, &tdev->pipe, &repaper_pipe_funcs,
+	ret = tinydrm_display_pipe_init(drm, &epd->pipe, &repaper_pipe_funcs,
 					DRM_MODE_CONNECTOR_VIRTUAL,
 					repaper_formats,
 					ARRAY_SIZE(repaper_formats), mode, 0);
 	if (ret)
 		return ret;
 
-	drm_mode_config_reset(tdev->drm);
+	drm_mode_config_reset(drm);
 
-	spi_set_drvdata(spi, tdev->drm);
+	ret = drm_dev_register(drm, 0);
+	if (ret)
+		return ret;
+
+	spi_set_drvdata(spi, drm);
 
 	DRM_DEBUG_DRIVER("SPI speed: %uMHz\n", spi->max_speed_hz / 1000000);
 
-	return devm_tinydrm_register(tdev);
+	drm_fbdev_generic_setup(drm, 32);
+
+	return 0;
+}
+
+static int repaper_remove(struct spi_device *spi)
+{
+	struct drm_device *drm = spi_get_drvdata(spi);
+
+	drm_dev_unplug(drm);
+	drm_atomic_helper_shutdown(drm);
+
+	return 0;
 }
 
 static void repaper_shutdown(struct spi_device *spi)
@@ -1099,6 +1136,7 @@ static struct spi_driver repaper_spi_driver = {
 	},
 	.id_table = repaper_id,
 	.probe = repaper_probe,
+	.remove = repaper_remove,
 	.shutdown = repaper_shutdown,
 };
 module_spi_driver(repaper_spi_driver);
-- 
2.20.1

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

[PATCH v2 09/12] drm/tinydrm: Drop using tinydrm_device

[Noralf Trønnes]

Use devm_drm_dev_init() and drop using tinydrm_device.

v2: devm_drm_dev_register() was dropped so add driver release callbacks.

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/tinydrm/hx8357d.c  |  40 +++++++++--
 drivers/gpu/drm/tinydrm/ili9225.c  |  40 +++++++++--
 drivers/gpu/drm/tinydrm/ili9341.c  |  40 +++++++++--
 drivers/gpu/drm/tinydrm/mi0283qt.c |  40 +++++++++--
 drivers/gpu/drm/tinydrm/mipi-dbi.c |  67 +++++++++++-------
 drivers/gpu/drm/tinydrm/st7586.c   | 105 ++++++++++++++++-------------
 drivers/gpu/drm/tinydrm/st7735r.c  |  40 +++++++++--
 include/drm/tinydrm/mipi-dbi.h     |  26 ++++---
 8 files changed, 294 insertions(+), 104 deletions(-)

diff --git a/drivers/gpu/drm/tinydrm/hx8357d.c b/drivers/gpu/drm/tinydrm/hx8357d.c
index 84dda622df85..e9b9e08fafc7 100644
--- a/drivers/gpu/drm/tinydrm/hx8357d.c
+++ b/drivers/gpu/drm/tinydrm/hx8357d.c
@@ -18,6 +18,7 @@
 
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_drv.h>
+#include <drm/drm_fb_helper.h>
 #include <drm/drm_gem_cma_helper.h>
 #include <drm/drm_gem_framebuffer_helper.h>
 #include <drm/drm_modeset_helper.h>
@@ -189,6 +190,7 @@ DEFINE_DRM_GEM_CMA_FOPS(hx8357d_fops);
 static struct drm_driver hx8357d_driver = {
 	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME | DRIVER_ATOMIC,
 	.fops			= &hx8357d_fops,
+	.release		= mipi_dbi_release,
 	DRM_GEM_CMA_VMAP_DRIVER_OPS,
 	.debugfs_init		= mipi_dbi_debugfs_init,
 	.name			= "hx8357d",
@@ -213,15 +215,25 @@ MODULE_DEVICE_TABLE(spi, hx8357d_id);
 static int hx8357d_probe(struct spi_device *spi)
 {
 	struct device *dev = &spi->dev;
+	struct drm_device *drm;
 	struct mipi_dbi *mipi;
 	struct gpio_desc *dc;
 	u32 rotation = 0;
 	int ret;
 
-	mipi = devm_kzalloc(dev, sizeof(*mipi), GFP_KERNEL);
+	mipi = kzalloc(sizeof(*mipi), GFP_KERNEL);
 	if (!mipi)
 		return -ENOMEM;
 
+	drm = &mipi->drm;
+	ret = devm_drm_dev_init(dev, drm, &hx8357d_driver);
+	if (ret) {
+		kfree(mipi);
+		return ret;
+	}
+
+	drm_mode_config_init(drm);
+
 	dc = devm_gpiod_get(dev, "dc", GPIOD_OUT_LOW);
 	if (IS_ERR(dc)) {
 		DRM_DEV_ERROR(dev, "Failed to get gpio 'dc'\n");
@@ -238,14 +250,31 @@ static int hx8357d_probe(struct spi_device *spi)
 	if (ret)
 		return ret;
 
-	ret = mipi_dbi_init(&spi->dev, mipi, &hx8357d_pipe_funcs,
-			    &hx8357d_driver, &yx350hv15_mode, rotation);
+	ret = mipi_dbi_init(mipi, &hx8357d_pipe_funcs, &yx350hv15_mode, rotation);
 	if (ret)
 		return ret;
 
-	spi_set_drvdata(spi, mipi->tinydrm.drm);
+	drm_mode_config_reset(drm);
 
-	return devm_tinydrm_register(&mipi->tinydrm);
+	ret = drm_dev_register(drm, 0);
+	if (ret)
+		return ret;
+
+	spi_set_drvdata(spi, drm);
+
+	drm_fbdev_generic_setup(drm, 32);
+
+	return 0;
+}
+
+static int hx8357d_remove(struct spi_device *spi)
+{
+	struct drm_device *drm = spi_get_drvdata(spi);
+
+	drm_dev_unplug(drm);
+	drm_atomic_helper_shutdown(drm);
+
+	return 0;
 }
 
 static void hx8357d_shutdown(struct spi_device *spi)
@@ -260,6 +289,7 @@ static struct spi_driver hx8357d_spi_driver = {
 	},
 	.id_table = hx8357d_id,
 	.probe = hx8357d_probe,
+	.remove = hx8357d_remove,
 	.shutdown = hx8357d_shutdown,
 };
 module_spi_driver(hx8357d_spi_driver);
diff --git a/drivers/gpu/drm/tinydrm/ili9225.c b/drivers/gpu/drm/tinydrm/ili9225.c
index 3f59cfbd31ba..4d387a07c48b 100644
--- a/drivers/gpu/drm/tinydrm/ili9225.c
+++ b/drivers/gpu/drm/tinydrm/ili9225.c
@@ -24,6 +24,7 @@
 #include <drm/drm_damage_helper.h>
 #include <drm/drm_drv.h>
 #include <drm/drm_fb_cma_helper.h>
+#include <drm/drm_fb_helper.h>
 #include <drm/drm_fourcc.h>
 #include <drm/drm_gem_cma_helper.h>
 #include <drm/drm_gem_framebuffer_helper.h>
@@ -339,6 +340,7 @@ static struct drm_driver ili9225_driver = {
 	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME |
 				  DRIVER_ATOMIC,
 	.fops			= &ili9225_fops,
+	.release		= mipi_dbi_release,
 	DRM_GEM_CMA_VMAP_DRIVER_OPS,
 	.name			= "ili9225",
 	.desc			= "Ilitek ILI9225",
@@ -362,15 +364,25 @@ MODULE_DEVICE_TABLE(spi, ili9225_id);
 static int ili9225_probe(struct spi_device *spi)
 {
 	struct device *dev = &spi->dev;
+	struct drm_device *drm;
 	struct mipi_dbi *mipi;
 	struct gpio_desc *rs;
 	u32 rotation = 0;
 	int ret;
 
-	mipi = devm_kzalloc(dev, sizeof(*mipi), GFP_KERNEL);
+	mipi = kzalloc(sizeof(*mipi), GFP_KERNEL);
 	if (!mipi)
 		return -ENOMEM;
 
+	drm = &mipi->drm;
+	ret = devm_drm_dev_init(dev, drm, &ili9225_driver);
+	if (ret) {
+		kfree(mipi);
+		return ret;
+	}
+
+	drm_mode_config_init(drm);
+
 	mipi->reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
 	if (IS_ERR(mipi->reset)) {
 		DRM_DEV_ERROR(dev, "Failed to get gpio 'reset'\n");
@@ -392,14 +404,31 @@ static int ili9225_probe(struct spi_device *spi)
 	/* override the command function set in  mipi_dbi_spi_init() */
 	mipi->command = ili9225_dbi_command;
 
-	ret = mipi_dbi_init(&spi->dev, mipi, &ili9225_pipe_funcs,
-			    &ili9225_driver, &ili9225_mode, rotation);
+	ret = mipi_dbi_init(mipi, &ili9225_pipe_funcs, &ili9225_mode, rotation);
 	if (ret)
 		return ret;
 
-	spi_set_drvdata(spi, mipi->tinydrm.drm);
+	drm_mode_config_reset(drm);
 
-	return devm_tinydrm_register(&mipi->tinydrm);
+	ret = drm_dev_register(drm, 0);
+	if (ret)
+		return ret;
+
+	spi_set_drvdata(spi, drm);
+
+	drm_fbdev_generic_setup(drm, 32);
+
+	return 0;
+}
+
+static int ili9225_remove(struct spi_device *spi)
+{
+	struct drm_device *drm = spi_get_drvdata(spi);
+
+	drm_dev_unplug(drm);
+	drm_atomic_helper_shutdown(drm);
+
+	return 0;
 }
 
 static void ili9225_shutdown(struct spi_device *spi)
@@ -415,6 +444,7 @@ static struct spi_driver ili9225_spi_driver = {
 	},
 	.id_table = ili9225_id,
 	.probe = ili9225_probe,
+	.remove = ili9225_remove,
 	.shutdown = ili9225_shutdown,
 };
 module_spi_driver(ili9225_spi_driver);
diff --git a/drivers/gpu/drm/tinydrm/ili9341.c b/drivers/gpu/drm/tinydrm/ili9341.c
index 3b7565a6311e..850ce9ed6dd2 100644
--- a/drivers/gpu/drm/tinydrm/ili9341.c
+++ b/drivers/gpu/drm/tinydrm/ili9341.c
@@ -17,6 +17,7 @@
 
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_drv.h>
+#include <drm/drm_fb_helper.h>
 #include <drm/drm_gem_cma_helper.h>
 #include <drm/drm_gem_framebuffer_helper.h>
 #include <drm/drm_modeset_helper.h>
@@ -145,6 +146,7 @@ DEFINE_DRM_GEM_CMA_FOPS(ili9341_fops);
 static struct drm_driver ili9341_driver = {
 	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME | DRIVER_ATOMIC,
 	.fops			= &ili9341_fops,
+	.release		= mipi_dbi_release,
 	DRM_GEM_CMA_VMAP_DRIVER_OPS,
 	.debugfs_init		= mipi_dbi_debugfs_init,
 	.name			= "ili9341",
@@ -169,15 +171,25 @@ MODULE_DEVICE_TABLE(spi, ili9341_id);
 static int ili9341_probe(struct spi_device *spi)
 {
 	struct device *dev = &spi->dev;
+	struct drm_device *drm;
 	struct mipi_dbi *mipi;
 	struct gpio_desc *dc;
 	u32 rotation = 0;
 	int ret;
 
-	mipi = devm_kzalloc(dev, sizeof(*mipi), GFP_KERNEL);
+	mipi = kzalloc(sizeof(*mipi), GFP_KERNEL);
 	if (!mipi)
 		return -ENOMEM;
 
+	drm = &mipi->drm;
+	ret = devm_drm_dev_init(dev, drm, &ili9341_driver);
+	if (ret) {
+		kfree(mipi);
+		return ret;
+	}
+
+	drm_mode_config_init(drm);
+
 	mipi->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
 	if (IS_ERR(mipi->reset)) {
 		DRM_DEV_ERROR(dev, "Failed to get gpio 'reset'\n");
@@ -200,14 +212,31 @@ static int ili9341_probe(struct spi_device *spi)
 	if (ret)
 		return ret;
 
-	ret = mipi_dbi_init(&spi->dev, mipi, &ili9341_pipe_funcs,
-			    &ili9341_driver, &yx240qv29_mode, rotation);
+	ret = mipi_dbi_init(mipi, &ili9341_pipe_funcs, &yx240qv29_mode, rotation);
 	if (ret)
 		return ret;
 
-	spi_set_drvdata(spi, mipi->tinydrm.drm);
+	drm_mode_config_reset(drm);
 
-	return devm_tinydrm_register(&mipi->tinydrm);
+	ret = drm_dev_register(drm, 0);
+	if (ret)
+		return ret;
+
+	spi_set_drvdata(spi, drm);
+
+	drm_fbdev_generic_setup(drm, 32);
+
+	return 0;
+}
+
+static int ili9341_remove(struct spi_device *spi)
+{
+	struct drm_device *drm = spi_get_drvdata(spi);
+
+	drm_dev_unplug(drm);
+	drm_atomic_helper_shutdown(drm);
+
+	return 0;
 }
 
 static void ili9341_shutdown(struct spi_device *spi)
@@ -222,6 +251,7 @@ static struct spi_driver ili9341_spi_driver = {
 	},
 	.id_table = ili9341_id,
 	.probe = ili9341_probe,
+	.remove = ili9341_remove,
 	.shutdown = ili9341_shutdown,
 };
 module_spi_driver(ili9341_spi_driver);
diff --git a/drivers/gpu/drm/tinydrm/mi0283qt.c b/drivers/gpu/drm/tinydrm/mi0283qt.c
index da326fd1450d..7aee05586e92 100644
--- a/drivers/gpu/drm/tinydrm/mi0283qt.c
+++ b/drivers/gpu/drm/tinydrm/mi0283qt.c
@@ -19,6 +19,7 @@
 
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_drv.h>
+#include <drm/drm_fb_helper.h>
 #include <drm/drm_gem_cma_helper.h>
 #include <drm/drm_gem_framebuffer_helper.h>
 #include <drm/drm_modeset_helper.h>
@@ -154,6 +155,7 @@ static struct drm_driver mi0283qt_driver = {
 	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME |
 				  DRIVER_ATOMIC,
 	.fops			= &mi0283qt_fops,
+	.release		= mipi_dbi_release,
 	DRM_GEM_CMA_VMAP_DRIVER_OPS,
 	.debugfs_init		= mipi_dbi_debugfs_init,
 	.name			= "mi0283qt",
@@ -178,15 +180,25 @@ MODULE_DEVICE_TABLE(spi, mi0283qt_id);
 static int mi0283qt_probe(struct spi_device *spi)
 {
 	struct device *dev = &spi->dev;
+	struct drm_device *drm;
 	struct mipi_dbi *mipi;
 	struct gpio_desc *dc;
 	u32 rotation = 0;
 	int ret;
 
-	mipi = devm_kzalloc(dev, sizeof(*mipi), GFP_KERNEL);
+	mipi = kzalloc(sizeof(*mipi), GFP_KERNEL);
 	if (!mipi)
 		return -ENOMEM;
 
+	drm = &mipi->drm;
+	ret = devm_drm_dev_init(dev, drm, &mi0283qt_driver);
+	if (ret) {
+		kfree(mipi);
+		return ret;
+	}
+
+	drm_mode_config_init(drm);
+
 	mipi->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
 	if (IS_ERR(mipi->reset)) {
 		DRM_DEV_ERROR(dev, "Failed to get gpio 'reset'\n");
@@ -213,14 +225,31 @@ static int mi0283qt_probe(struct spi_device *spi)
 	if (ret)
 		return ret;
 
-	ret = mipi_dbi_init(&spi->dev, mipi, &mi0283qt_pipe_funcs,
-			    &mi0283qt_driver, &mi0283qt_mode, rotation);
+	ret = mipi_dbi_init(mipi, &mi0283qt_pipe_funcs, &mi0283qt_mode, rotation);
 	if (ret)
 		return ret;
 
-	spi_set_drvdata(spi, mipi->tinydrm.drm);
+	drm_mode_config_reset(drm);
 
-	return devm_tinydrm_register(&mipi->tinydrm);
+	ret = drm_dev_register(drm, 0);
+	if (ret)
+		return ret;
+
+	spi_set_drvdata(spi, drm);
+
+	drm_fbdev_generic_setup(drm, 32);
+
+	return 0;
+}
+
+static int mi0283qt_remove(struct spi_device *spi)
+{
+	struct drm_device *drm = spi_get_drvdata(spi);
+
+	drm_dev_unplug(drm);
+	drm_atomic_helper_shutdown(drm);
+
+	return 0;
 }
 
 static void mi0283qt_shutdown(struct spi_device *spi)
@@ -253,6 +282,7 @@ static struct spi_driver mi0283qt_spi_driver = {
 	},
 	.id_table = mi0283qt_id,
 	.probe = mi0283qt_probe,
+	.remove = mi0283qt_remove,
 	.shutdown = mi0283qt_shutdown,
 };
 module_spi_driver(mi0283qt_spi_driver);
diff --git a/drivers/gpu/drm/tinydrm/mipi-dbi.c b/drivers/gpu/drm/tinydrm/mipi-dbi.c
index 045b840812c3..ecfee357436d 100644
--- a/drivers/gpu/drm/tinydrm/mipi-dbi.c
+++ b/drivers/gpu/drm/tinydrm/mipi-dbi.c
@@ -316,7 +316,7 @@ EXPORT_SYMBOL(mipi_dbi_enable_flush);
 
 static void mipi_dbi_blank(struct mipi_dbi *mipi)
 {
-	struct drm_device *drm = mipi->tinydrm.drm;
+	struct drm_device *drm = &mipi->drm;
 	u16 height = drm->mode_config.min_height;
 	u16 width = drm->mode_config.min_width;
 	size_t len = width * height * 2;
@@ -357,6 +357,12 @@ void mipi_dbi_pipe_disable(struct drm_simple_display_pipe *pipe)
 }
 EXPORT_SYMBOL(mipi_dbi_pipe_disable);
 
+static const struct drm_mode_config_funcs mipi_dbi_mode_config_funcs = {
+	.fb_create = drm_gem_fb_create_with_dirty,
+	.atomic_check = drm_atomic_helper_check,
+	.atomic_commit = drm_atomic_helper_commit,
+};
+
 static const uint32_t mipi_dbi_formats[] = {
 	DRM_FORMAT_RGB565,
 	DRM_FORMAT_XRGB8888,
@@ -364,31 +370,27 @@ static const uint32_t mipi_dbi_formats[] = {
 
 /**
  * mipi_dbi_init - MIPI DBI initialization
- * @dev: Parent device
  * @mipi: &mipi_dbi structure to initialize
- * @pipe_funcs: Display pipe functions
- * @driver: DRM driver
+ * @funcs: Display pipe functions
  * @mode: Display mode
  * @rotation: Initial rotation in degrees Counter Clock Wise
  *
- * This function initializes a &mipi_dbi structure and it's underlying
- * @tinydrm_device. It also sets up the display pipeline.
+ * This function sets up a &drm_simple_display_pipe with a &drm_connector that
+ * has one fixed &drm_display_mode which is rotated according to @rotation.
+ * This mode is used to set the mode config min/max width/height properties.
+ * Additionally &mipi_dbi.tx_buf is allocated.
  *
  * Supported formats: Native RGB565 and emulated XRGB8888.
  *
- * Objects created by this function will be automatically freed on driver
- * detach (devres).
- *
  * Returns:
  * Zero on success, negative error code on failure.
  */
-int mipi_dbi_init(struct device *dev, struct mipi_dbi *mipi,
-		  const struct drm_simple_display_pipe_funcs *pipe_funcs,
-		  struct drm_driver *driver,
+int mipi_dbi_init(struct mipi_dbi *mipi,
+		  const struct drm_simple_display_pipe_funcs *funcs,
 		  const struct drm_display_mode *mode, unsigned int rotation)
 {
 	size_t bufsize = mode->vdisplay * mode->hdisplay * sizeof(u16);
-	struct tinydrm_device *tdev = &mipi->tinydrm;
+	struct drm_device *drm = &mipi->drm;
 	int ret;
 
 	if (!mipi->command)
@@ -396,16 +398,12 @@ int mipi_dbi_init(struct device *dev, struct mipi_dbi *mipi,
 
 	mutex_init(&mipi->cmdlock);
 
-	mipi->tx_buf = devm_kmalloc(dev, bufsize, GFP_KERNEL);
+	mipi->tx_buf = devm_kmalloc(drm->dev, bufsize, GFP_KERNEL);
 	if (!mipi->tx_buf)
 		return -ENOMEM;
 
-	ret = devm_tinydrm_init(dev, tdev, driver);
-	if (ret)
-		return ret;
-
 	/* TODO: Maybe add DRM_MODE_CONNECTOR_SPI */
-	ret = tinydrm_display_pipe_init(tdev->drm, &tdev->pipe, pipe_funcs,
+	ret = tinydrm_display_pipe_init(drm, &mipi->pipe, funcs,
 					DRM_MODE_CONNECTOR_VIRTUAL,
 					mipi_dbi_formats,
 					ARRAY_SIZE(mipi_dbi_formats), mode,
@@ -413,20 +411,39 @@ int mipi_dbi_init(struct device *dev, struct mipi_dbi *mipi,
 	if (ret)
 		return ret;
 
-	drm_plane_enable_fb_damage_clips(&tdev->pipe.plane);
+	drm_plane_enable_fb_damage_clips(&mipi->pipe.plane);
 
-	tdev->drm->mode_config.preferred_depth = 16;
+	drm->mode_config.funcs = &mipi_dbi_mode_config_funcs;
+	drm->mode_config.preferred_depth = 16;
 	mipi->rotation = rotation;
 
-	drm_mode_config_reset(tdev->drm);
-
 	DRM_DEBUG_KMS("preferred_depth=%u, rotation = %u\n",
-		      tdev->drm->mode_config.preferred_depth, rotation);
+		      drm->mode_config.preferred_depth, rotation);
 
 	return 0;
 }
 EXPORT_SYMBOL(mipi_dbi_init);
 
+/**
+ * mipi_dbi_release - DRM driver release helper
+ * @drm: DRM device
+ *
+ * This function finalizes and frees &mipi_dbi.
+ *
+ * Drivers can use this as their &drm_driver->release callback.
+ */
+void mipi_dbi_release(struct drm_device *drm)
+{
+	struct mipi_dbi *dbi = drm_to_mipi_dbi(drm);
+
+	DRM_DEBUG_DRIVER("\n");
+
+	drm_mode_config_cleanup(drm);
+	drm_dev_fini(drm);
+	kfree(dbi);
+}
+EXPORT_SYMBOL(mipi_dbi_release);
+
 /**
  * mipi_dbi_hw_reset - Hardware reset of controller
  * @mipi: MIPI DBI structure
@@ -479,7 +496,7 @@ EXPORT_SYMBOL(mipi_dbi_display_is_on);
 
 static int mipi_dbi_poweron_reset_conditional(struct mipi_dbi *mipi, bool cond)
 {
-	struct device *dev = mipi->tinydrm.drm->dev;
+	struct device *dev = mipi->drm.dev;
 	int ret;
 
 	if (mipi->regulator) {
diff --git a/drivers/gpu/drm/tinydrm/st7586.c b/drivers/gpu/drm/tinydrm/st7586.c
index 6097585913f2..6e92de809b63 100644
--- a/drivers/gpu/drm/tinydrm/st7586.c
+++ b/drivers/gpu/drm/tinydrm/st7586.c
@@ -21,6 +21,7 @@
 #include <drm/drm_damage_helper.h>
 #include <drm/drm_drv.h>
 #include <drm/drm_fb_cma_helper.h>
+#include <drm/drm_fb_helper.h>
 #include <drm/drm_gem_cma_helper.h>
 #include <drm/drm_gem_framebuffer_helper.h>
 #include <drm/drm_rect.h>
@@ -262,46 +263,6 @@ static const u32 st7586_formats[] = {
 	DRM_FORMAT_XRGB8888,
 };
 
-static int st7586_init(struct device *dev, struct mipi_dbi *mipi,
-		const struct drm_simple_display_pipe_funcs *pipe_funcs,
-		struct drm_driver *driver, const struct drm_display_mode *mode,
-		unsigned int rotation)
-{
-	size_t bufsize = (mode->vdisplay + 2) / 3 * mode->hdisplay;
-	struct tinydrm_device *tdev = &mipi->tinydrm;
-	int ret;
-
-	mutex_init(&mipi->cmdlock);
-
-	mipi->tx_buf = devm_kmalloc(dev, bufsize, GFP_KERNEL);
-	if (!mipi->tx_buf)
-		return -ENOMEM;
-
-	ret = devm_tinydrm_init(dev, tdev, driver);
-	if (ret)
-		return ret;
-
-	ret = tinydrm_display_pipe_init(tdev->drm, &tdev->pipe, pipe_funcs,
-					DRM_MODE_CONNECTOR_VIRTUAL,
-					st7586_formats,
-					ARRAY_SIZE(st7586_formats),
-					mode, rotation);
-	if (ret)
-		return ret;
-
-	drm_plane_enable_fb_damage_clips(&tdev->pipe.plane);
-
-	tdev->drm->mode_config.preferred_depth = 32;
-	mipi->rotation = rotation;
-
-	drm_mode_config_reset(tdev->drm);
-
-	DRM_DEBUG_KMS("preferred_depth=%u, rotation = %u\n",
-		      tdev->drm->mode_config.preferred_depth, rotation);
-
-	return 0;
-}
-
 static const struct drm_simple_display_pipe_funcs st7586_pipe_funcs = {
 	.enable		= st7586_pipe_enable,
 	.disable	= st7586_pipe_disable,
@@ -309,6 +270,12 @@ static const struct drm_simple_display_pipe_funcs st7586_pipe_funcs = {
 	.prepare_fb	= drm_gem_fb_simple_display_pipe_prepare_fb,
 };
 
+static const struct drm_mode_config_funcs st7586_mode_config_funcs = {
+	.fb_create = drm_gem_fb_create_with_dirty,
+	.atomic_check = drm_atomic_helper_check,
+	.atomic_commit = drm_atomic_helper_commit,
+};
+
 static const struct drm_display_mode st7586_mode = {
 	DRM_SIMPLE_MODE(178, 128, 37, 27),
 };
@@ -319,6 +286,7 @@ static struct drm_driver st7586_driver = {
 	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME |
 				  DRIVER_ATOMIC,
 	.fops			= &st7586_fops,
+	.release		= mipi_dbi_release,
 	DRM_GEM_CMA_VMAP_DRIVER_OPS,
 	.debugfs_init		= mipi_dbi_debugfs_init,
 	.name			= "st7586",
@@ -343,15 +311,35 @@ MODULE_DEVICE_TABLE(spi, st7586_id);
 static int st7586_probe(struct spi_device *spi)
 {
 	struct device *dev = &spi->dev;
+	struct drm_device *drm;
 	struct mipi_dbi *mipi;
 	struct gpio_desc *a0;
 	u32 rotation = 0;
+	size_t bufsize;
 	int ret;
 
-	mipi = devm_kzalloc(dev, sizeof(*mipi), GFP_KERNEL);
+	mipi = kzalloc(sizeof(*mipi), GFP_KERNEL);
 	if (!mipi)
 		return -ENOMEM;
 
+	drm = &mipi->drm;
+	ret = devm_drm_dev_init(dev, drm, &st7586_driver);
+	if (ret) {
+		kfree(mipi);
+		return ret;
+	}
+
+	drm_mode_config_init(drm);
+	drm->mode_config.preferred_depth = 32;
+	drm->mode_config.funcs = &st7586_mode_config_funcs;
+
+	mutex_init(&mipi->cmdlock);
+
+	bufsize = (st7586_mode.vdisplay + 2) / 3 * st7586_mode.hdisplay;
+	mipi->tx_buf = devm_kmalloc(dev, bufsize, GFP_KERNEL);
+	if (!mipi->tx_buf)
+		return -ENOMEM;
+
 	mipi->reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
 	if (IS_ERR(mipi->reset)) {
 		DRM_DEV_ERROR(dev, "Failed to get gpio 'reset'\n");
@@ -365,6 +353,7 @@ static int st7586_probe(struct spi_device *spi)
 	}
 
 	device_property_read_u32(dev, "rotation", &rotation);
+	mipi->rotation = rotation;
 
 	ret = mipi_dbi_spi_init(spi, mipi, a0);
 	if (ret)
@@ -382,14 +371,39 @@ static int st7586_probe(struct spi_device *spi)
 	 */
 	mipi->swap_bytes = true;
 
-	ret = st7586_init(&spi->dev, mipi, &st7586_pipe_funcs, &st7586_driver,
-			  &st7586_mode, rotation);
+	ret = tinydrm_display_pipe_init(drm, &mipi->pipe, &st7586_pipe_funcs,
+					DRM_MODE_CONNECTOR_VIRTUAL,
+					st7586_formats, ARRAY_SIZE(st7586_formats),
+					&st7586_mode, rotation);
 	if (ret)
 		return ret;
 
-	spi_set_drvdata(spi, mipi->tinydrm.drm);
+	drm_plane_enable_fb_damage_clips(&mipi->pipe.plane);
 
-	return devm_tinydrm_register(&mipi->tinydrm);
+	drm_mode_config_reset(drm);
+
+	ret = drm_dev_register(drm, 0);
+	if (ret)
+		return ret;
+
+	spi_set_drvdata(spi, drm);
+
+	DRM_DEBUG_KMS("preferred_depth=%u, rotation = %u\n",
+		      drm->mode_config.preferred_depth, rotation);
+
+	drm_fbdev_generic_setup(drm, 32);
+
+	return 0;
+}
+
+static int st7586_remove(struct spi_device *spi)
+{
+	struct drm_device *drm = spi_get_drvdata(spi);
+
+	drm_dev_unplug(drm);
+	drm_atomic_helper_shutdown(drm);
+
+	return 0;
 }
 
 static void st7586_shutdown(struct spi_device *spi)
@@ -405,6 +419,7 @@ static struct spi_driver st7586_spi_driver = {
 	},
 	.id_table = st7586_id,
 	.probe = st7586_probe,
+	.remove = st7586_remove,
 	.shutdown = st7586_shutdown,
 };
 module_spi_driver(st7586_spi_driver);
diff --git a/drivers/gpu/drm/tinydrm/st7735r.c b/drivers/gpu/drm/tinydrm/st7735r.c
index bfa7e2221540..0f8a346026ac 100644
--- a/drivers/gpu/drm/tinydrm/st7735r.c
+++ b/drivers/gpu/drm/tinydrm/st7735r.c
@@ -16,6 +16,7 @@
 
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_drv.h>
+#include <drm/drm_fb_helper.h>
 #include <drm/drm_gem_cma_helper.h>
 #include <drm/drm_gem_framebuffer_helper.h>
 #include <drm/tinydrm/mipi-dbi.h>
@@ -120,6 +121,7 @@ static struct drm_driver st7735r_driver = {
 	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME |
 				  DRIVER_ATOMIC,
 	.fops			= &st7735r_fops,
+	.release		= mipi_dbi_release,
 	DRM_GEM_CMA_VMAP_DRIVER_OPS,
 	.debugfs_init		= mipi_dbi_debugfs_init,
 	.name			= "st7735r",
@@ -144,15 +146,25 @@ MODULE_DEVICE_TABLE(spi, st7735r_id);
 static int st7735r_probe(struct spi_device *spi)
 {
 	struct device *dev = &spi->dev;
+	struct drm_device *drm;
 	struct mipi_dbi *mipi;
 	struct gpio_desc *dc;
 	u32 rotation = 0;
 	int ret;
 
-	mipi = devm_kzalloc(dev, sizeof(*mipi), GFP_KERNEL);
+	mipi = kzalloc(sizeof(*mipi), GFP_KERNEL);
 	if (!mipi)
 		return -ENOMEM;
 
+	drm = &mipi->drm;
+	ret = devm_drm_dev_init(dev, drm, &st7735r_driver);
+	if (ret) {
+		kfree(mipi);
+		return ret;
+	}
+
+	drm_mode_config_init(drm);
+
 	mipi->reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
 	if (IS_ERR(mipi->reset)) {
 		DRM_DEV_ERROR(dev, "Failed to get gpio 'reset'\n");
@@ -178,14 +190,31 @@ static int st7735r_probe(struct spi_device *spi)
 	/* Cannot read from Adafruit 1.8" display via SPI */
 	mipi->read_commands = NULL;
 
-	ret = mipi_dbi_init(&spi->dev, mipi, &jd_t18003_t01_pipe_funcs,
-			    &st7735r_driver, &jd_t18003_t01_mode, rotation);
+	ret = mipi_dbi_init(mipi, &jd_t18003_t01_pipe_funcs, &jd_t18003_t01_mode, rotation);
 	if (ret)
 		return ret;
 
-	spi_set_drvdata(spi, mipi->tinydrm.drm);
+	drm_mode_config_reset(drm);
 
-	return devm_tinydrm_register(&mipi->tinydrm);
+	ret = drm_dev_register(drm, 0);
+	if (ret)
+		return ret;
+
+	spi_set_drvdata(spi, drm);
+
+	drm_fbdev_generic_setup(drm, 32);
+
+	return 0;
+}
+
+static int st7735r_remove(struct spi_device *spi)
+{
+	struct drm_device *drm = spi_get_drvdata(spi);
+
+	drm_dev_unplug(drm);
+	drm_atomic_helper_shutdown(drm);
+
+	return 0;
 }
 
 static void st7735r_shutdown(struct spi_device *spi)
@@ -201,6 +230,7 @@ static struct spi_driver st7735r_spi_driver = {
 	},
 	.id_table = st7735r_id,
 	.probe = st7735r_probe,
+	.remove = st7735r_remove,
 	.shutdown = st7735r_shutdown,
 };
 module_spi_driver(st7735r_spi_driver);
diff --git a/include/drm/tinydrm/mipi-dbi.h b/include/drm/tinydrm/mipi-dbi.h
index ad7e6bedab5f..301b2c34cd08 100644
--- a/include/drm/tinydrm/mipi-dbi.h
+++ b/include/drm/tinydrm/mipi-dbi.h
@@ -12,7 +12,9 @@
 #ifndef __LINUX_MIPI_DBI_H
 #define __LINUX_MIPI_DBI_H
 
-#include <drm/tinydrm/tinydrm.h>
+#include <linux/mutex.h>
+#include <drm/drm_device.h>
+#include <drm/drm_simple_kms_helper.h>
 
 struct drm_rect;
 struct spi_device;
@@ -21,7 +23,6 @@ struct regulator;
 
 /**
  * struct mipi_dbi - MIPI DBI controller
- * @tinydrm: tinydrm base
  * @spi: SPI device
  * @enabled: Pipeline is enabled
  * @cmdlock: Command lock
@@ -39,7 +40,16 @@ struct regulator;
  * @regulator: power regulator (optional)
  */
 struct mipi_dbi {
-	struct tinydrm_device tinydrm;
+	/**
+	 * @drm: DRM device
+	 */
+	struct drm_device drm;
+
+	/**
+	 * @pipe: Display pipe structure
+	 */
+	struct drm_simple_display_pipe pipe;
+
 	struct spi_device *spi;
 	bool enabled;
 	struct mutex cmdlock;
@@ -58,17 +68,15 @@ struct mipi_dbi {
 
 static inline struct mipi_dbi *drm_to_mipi_dbi(struct drm_device *drm)
 {
-	struct tinydrm_device *tdev = drm->dev_private;
-
-	return container_of(tdev, struct mipi_dbi, tinydrm);
+	return container_of(drm, struct mipi_dbi, drm);
 }
 
 int mipi_dbi_spi_init(struct spi_device *spi, struct mipi_dbi *mipi,
 		      struct gpio_desc *dc);
-int mipi_dbi_init(struct device *dev, struct mipi_dbi *mipi,
-		  const struct drm_simple_display_pipe_funcs *pipe_funcs,
-		  struct drm_driver *driver,
+int mipi_dbi_init(struct mipi_dbi *mipi,
+		  const struct drm_simple_display_pipe_funcs *funcs,
 		  const struct drm_display_mode *mode, unsigned int rotation);
+void mipi_dbi_release(struct drm_device *drm);
 void mipi_dbi_pipe_update(struct drm_simple_display_pipe *pipe,
 			  struct drm_plane_state *old_state);
 void mipi_dbi_enable_flush(struct mipi_dbi *mipi,
-- 
2.20.1

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

[PATCH v2 10/12] drm/tinydrm: Remove tinydrm_device

[Noralf Trønnes]

No more users left so it can go alongside its helpers.
Update the tinydrm docs description and remove todo entry.

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 Documentation/gpu/tinydrm.rst                 |  32 ++--
 Documentation/gpu/todo.rst                    |   4 -
 drivers/gpu/drm/tinydrm/core/Makefile         |   2 +-
 drivers/gpu/drm/tinydrm/core/tinydrm-core.c   | 169 ------------------
 .../gpu/drm/tinydrm/core/tinydrm-helpers.c    |   2 +
 include/drm/tinydrm/tinydrm.h                 |  42 -----
 6 files changed, 13 insertions(+), 238 deletions(-)
 delete mode 100644 drivers/gpu/drm/tinydrm/core/tinydrm-core.c
 delete mode 100644 include/drm/tinydrm/tinydrm.h

diff --git a/Documentation/gpu/tinydrm.rst b/Documentation/gpu/tinydrm.rst
index a913644bfc19..33a41544f659 100644
--- a/Documentation/gpu/tinydrm.rst
+++ b/Documentation/gpu/tinydrm.rst
@@ -1,27 +1,12 @@
-==========================
-drm/tinydrm Driver library
-==========================
+============================
+drm/tinydrm Tiny DRM drivers
+============================
 
-.. kernel-doc:: drivers/gpu/drm/tinydrm/core/tinydrm-core.c
-   :doc: overview
+tinydrm is a collection of DRM drivers that are so small they can fit in a
+single source file.
 
-Core functionality
-==================
-
-.. kernel-doc:: drivers/gpu/drm/tinydrm/core/tinydrm-core.c
-   :doc: core
-
-.. kernel-doc:: include/drm/tinydrm/tinydrm.h
-   :internal:
-
-.. kernel-doc:: drivers/gpu/drm/tinydrm/core/tinydrm-core.c
-   :export:
-
-.. kernel-doc:: drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c
-   :export:
-
-Additional helpers
-==================
+Helpers
+=======
 
 .. kernel-doc:: include/drm/tinydrm/tinydrm-helpers.h
    :internal:
@@ -29,6 +14,9 @@ Additional helpers
 .. kernel-doc:: drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
    :export:
 
+.. kernel-doc:: drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c
+   :export:
+
 MIPI DBI Compatible Controllers
 ===============================
 
diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
index cda4a37a02f0..b34f59a0d3d5 100644
--- a/Documentation/gpu/todo.rst
+++ b/Documentation/gpu/todo.rst
@@ -473,10 +473,6 @@ those drivers as simple as possible, so lots of room for refactoring:
   one of the ideas for having a shared dsi/dbi helper, abstracting away the
   transport details more.
 
-- Quick aside: The unregister devm stuff is kinda getting the lifetimes of
-  a drm_device wrong. Doesn't matter, since everyone else gets it wrong
-  too :-)
-
 Contact: Noralf Trønnes, Daniel Vetter
 
 AMD DC Display Driver
diff --git a/drivers/gpu/drm/tinydrm/core/Makefile b/drivers/gpu/drm/tinydrm/core/Makefile
index fb221e6f8885..6f8f764560e0 100644
--- a/drivers/gpu/drm/tinydrm/core/Makefile
+++ b/drivers/gpu/drm/tinydrm/core/Makefile
@@ -1,3 +1,3 @@
-tinydrm-y := tinydrm-core.o tinydrm-pipe.o tinydrm-helpers.o
+tinydrm-y := tinydrm-pipe.o tinydrm-helpers.o
 
 obj-$(CONFIG_DRM_TINYDRM) += tinydrm.o
diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-core.c b/drivers/gpu/drm/tinydrm/core/tinydrm-core.c
deleted file mode 100644
index 2366a33fd62f..000000000000
--- a/drivers/gpu/drm/tinydrm/core/tinydrm-core.c
+++ /dev/null
@@ -1,169 +0,0 @@
-/*
- * Copyright (C) 2016 Noralf Trønnes
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- */
-
-#include <drm/drm_atomic.h>
-#include <drm/drm_atomic_helper.h>
-#include <drm/drm_drv.h>
-#include <drm/drm_fb_helper.h>
-#include <drm/drm_gem_framebuffer_helper.h>
-#include <drm/drm_probe_helper.h>
-#include <drm/drm_print.h>
-#include <drm/tinydrm/tinydrm.h>
-#include <linux/device.h>
-#include <linux/dma-buf.h>
-#include <linux/module.h>
-
-/**
- * DOC: overview
- *
- * This library provides driver helpers for very simple display hardware.
- *
- * It is based on &drm_simple_display_pipe coupled with a &drm_connector which
- * has only one fixed &drm_display_mode. The framebuffers are backed by the
- * cma helper and have support for framebuffer flushing (dirty).
- * fbdev support is also included.
- *
- */
-
-/**
- * DOC: core
- *
- * The driver allocates &tinydrm_device, initializes it using
- * devm_tinydrm_init(), sets up the pipeline using tinydrm_display_pipe_init()
- * and registers the DRM device using devm_tinydrm_register().
- */
-
-static const struct drm_mode_config_funcs tinydrm_mode_config_funcs = {
-	.fb_create = drm_gem_fb_create_with_dirty,
-	.atomic_check = drm_atomic_helper_check,
-	.atomic_commit = drm_atomic_helper_commit,
-};
-
-static int tinydrm_init(struct device *parent, struct tinydrm_device *tdev,
-			struct drm_driver *driver)
-{
-	struct drm_device *drm;
-
-	/*
-	 * We don't embed drm_device, because that prevent us from using
-	 * devm_kzalloc() to allocate tinydrm_device in the driver since
-	 * drm_dev_put() frees the structure. The devm_ functions provide
-	 * for easy error handling.
-	 */
-	drm = drm_dev_alloc(driver, parent);
-	if (IS_ERR(drm))
-		return PTR_ERR(drm);
-
-	tdev->drm = drm;
-	drm->dev_private = tdev;
-	drm_mode_config_init(drm);
-	drm->mode_config.funcs = &tinydrm_mode_config_funcs;
-	drm->mode_config.allow_fb_modifiers = true;
-
-	return 0;
-}
-
-static void tinydrm_fini(struct tinydrm_device *tdev)
-{
-	drm_mode_config_cleanup(tdev->drm);
-	tdev->drm->dev_private = NULL;
-	drm_dev_put(tdev->drm);
-}
-
-static void devm_tinydrm_release(void *data)
-{
-	tinydrm_fini(data);
-}
-
-/**
- * devm_tinydrm_init - Initialize tinydrm device
- * @parent: Parent device object
- * @tdev: tinydrm device
- * @driver: DRM driver
- *
- * This function initializes @tdev, the underlying DRM device and it's
- * mode_config. Resources will be automatically freed on driver detach (devres)
- * using drm_mode_config_cleanup() and drm_dev_put().
- *
- * Returns:
- * Zero on success, negative error code on failure.
- */
-int devm_tinydrm_init(struct device *parent, struct tinydrm_device *tdev,
-		      struct drm_driver *driver)
-{
-	int ret;
-
-	ret = tinydrm_init(parent, tdev, driver);
-	if (ret)
-		return ret;
-
-	ret = devm_add_action(parent, devm_tinydrm_release, tdev);
-	if (ret)
-		tinydrm_fini(tdev);
-
-	return ret;
-}
-EXPORT_SYMBOL(devm_tinydrm_init);
-
-static int tinydrm_register(struct tinydrm_device *tdev)
-{
-	struct drm_device *drm = tdev->drm;
-	int ret;
-
-	ret = drm_dev_register(tdev->drm, 0);
-	if (ret)
-		return ret;
-
-	ret = drm_fbdev_generic_setup(drm, 0);
-	if (ret)
-		DRM_ERROR("Failed to initialize fbdev: %d\n", ret);
-
-	return 0;
-}
-
-static void tinydrm_unregister(struct tinydrm_device *tdev)
-{
-	drm_atomic_helper_shutdown(tdev->drm);
-	drm_dev_unregister(tdev->drm);
-}
-
-static void devm_tinydrm_register_release(void *data)
-{
-	tinydrm_unregister(data);
-}
-
-/**
- * devm_tinydrm_register - Register tinydrm device
- * @tdev: tinydrm device
- *
- * This function registers the underlying DRM device and fbdev.
- * These resources will be automatically unregistered on driver detach (devres)
- * and the display pipeline will be disabled.
- *
- * Returns:
- * Zero on success, negative error code on failure.
- */
-int devm_tinydrm_register(struct tinydrm_device *tdev)
-{
-	struct device *dev = tdev->drm->dev;
-	int ret;
-
-	ret = tinydrm_register(tdev);
-	if (ret)
-		return ret;
-
-	ret = devm_add_action(dev, devm_tinydrm_register_release, tdev);
-	if (ret)
-		tinydrm_unregister(tdev);
-
-	return ret;
-}
-EXPORT_SYMBOL(devm_tinydrm_register);
-
-MODULE_LICENSE("GPL");
diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
index 2737b6fdadc8..d7b38dfb6438 100644
--- a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
+++ b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
@@ -365,3 +365,5 @@ int tinydrm_spi_transfer(struct spi_device *spi, u32 speed_hz,
 EXPORT_SYMBOL(tinydrm_spi_transfer);
 
 #endif /* CONFIG_SPI */
+
+MODULE_LICENSE("GPL");
diff --git a/include/drm/tinydrm/tinydrm.h b/include/drm/tinydrm/tinydrm.h
deleted file mode 100644
index ee9b17759391..000000000000
--- a/include/drm/tinydrm/tinydrm.h
+++ /dev/null
@@ -1,42 +0,0 @@
-/*
- * Copyright (C) 2016 Noralf Trønnes
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- */
-
-#ifndef __LINUX_TINYDRM_H
-#define __LINUX_TINYDRM_H
-
-#include <drm/drm_simple_kms_helper.h>
-
-struct drm_driver;
-
-/**
- * struct tinydrm_device - tinydrm device
- */
-struct tinydrm_device {
-	/**
-	 * @drm: DRM device
-	 */
-	struct drm_device *drm;
-
-	/**
-	 * @pipe: Display pipe structure
-	 */
-	struct drm_simple_display_pipe pipe;
-};
-
-static inline struct tinydrm_device *
-pipe_to_tinydrm(struct drm_simple_display_pipe *pipe)
-{
-	return container_of(pipe, struct tinydrm_device, pipe);
-}
-
-int devm_tinydrm_init(struct device *parent, struct tinydrm_device *tdev,
-		      struct drm_driver *driver);
-int devm_tinydrm_register(struct tinydrm_device *tdev);
-
-#endif /* __LINUX_TINYDRM_H */
-- 
2.20.1

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

[PATCH v2 11/12] drm/tinydrm: Use drm_dev_enter/exit()

[Noralf Trønnes]

This protects device resources from use after device removal.

There are 3 ways for driver-device unbinding to happen:
- The driver module is unloaded causing the driver to be unregistered.
  This can't happen as long as there are open file handles because a
  reference is taken on the module.
- The device is removed (Device Tree overlay unloading).
  This can happen at any time.
- The driver sysfs unbind file can be used to unbind the driver from the
  device. This can happen any time.

v2: Since drm_atomic_helper_shutdown() has to be called after
    drm_dev_unplug() we don't want do block ->disable after unplug.

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/tinydrm/hx8357d.c  |  9 ++++--
 drivers/gpu/drm/tinydrm/ili9225.c  | 23 +++++++++++++--
 drivers/gpu/drm/tinydrm/ili9341.c  |  9 ++++--
 drivers/gpu/drm/tinydrm/mi0283qt.c |  9 ++++--
 drivers/gpu/drm/tinydrm/mipi-dbi.c | 42 +++++++++++++++++++++++----
 drivers/gpu/drm/tinydrm/repaper.c  | 46 ++++++++++++++++++++++--------
 drivers/gpu/drm/tinydrm/st7586.c   | 24 +++++++++++++---
 drivers/gpu/drm/tinydrm/st7735r.c  |  9 ++++--
 8 files changed, 139 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/tinydrm/hx8357d.c b/drivers/gpu/drm/tinydrm/hx8357d.c
index e9b9e08fafc7..fab961dded87 100644
--- a/drivers/gpu/drm/tinydrm/hx8357d.c
+++ b/drivers/gpu/drm/tinydrm/hx8357d.c
@@ -50,13 +50,16 @@ static void yx240qv29_enable(struct drm_simple_display_pipe *pipe,
 {
 	struct mipi_dbi *mipi = drm_to_mipi_dbi(pipe->crtc.dev);
 	u8 addr_mode;
-	int ret;
+	int ret, idx;
+
+	if (!drm_dev_enter(pipe->crtc.dev, &idx))
+		return;
 
 	DRM_DEBUG_KMS("\n");
 
 	ret = mipi_dbi_poweron_conditional_reset(mipi);
 	if (ret < 0)
-		return;
+		goto out_exit;
 	if (ret == 1)
 		goto out_enable;
 
@@ -172,6 +175,8 @@ static void yx240qv29_enable(struct drm_simple_display_pipe *pipe,
 	}
 	mipi_dbi_command(mipi, MIPI_DCS_SET_ADDRESS_MODE, addr_mode);
 	mipi_dbi_enable_flush(mipi, crtc_state, plane_state);
+out_exit:
+	drm_dev_exit(idx);
 }
 
 static const struct drm_simple_display_pipe_funcs hx8357d_pipe_funcs = {
diff --git a/drivers/gpu/drm/tinydrm/ili9225.c b/drivers/gpu/drm/tinydrm/ili9225.c
index 4d387a07c48b..0e9fde47b53b 100644
--- a/drivers/gpu/drm/tinydrm/ili9225.c
+++ b/drivers/gpu/drm/tinydrm/ili9225.c
@@ -89,13 +89,16 @@ static void ili9225_fb_dirty(struct drm_framebuffer *fb, struct drm_rect *rect)
 	bool swap = mipi->swap_bytes;
 	u16 x_start, y_start;
 	u16 x1, x2, y1, y2;
-	int ret = 0;
+	int idx, ret = 0;
 	bool full;
 	void *tr;
 
 	if (!mipi->enabled)
 		return;
 
+	if (!drm_dev_enter(fb->dev, &idx))
+		return;
+
 	full = width == fb->width && height == fb->height;
 
 	DRM_DEBUG_KMS("Flushing [FB:%d] " DRM_RECT_FMT "\n", fb->base.id, DRM_RECT_ARG(rect));
@@ -158,6 +161,8 @@ static void ili9225_fb_dirty(struct drm_framebuffer *fb, struct drm_rect *rect)
 err_msg:
 	if (ret)
 		dev_err_once(fb->dev->dev, "Failed to update display %d\n", ret);
+
+	drm_dev_exit(idx);
 }
 
 static void ili9225_pipe_update(struct drm_simple_display_pipe *pipe,
@@ -191,9 +196,12 @@ static void ili9225_pipe_enable(struct drm_simple_display_pipe *pipe,
 		.y1 = 0,
 		.y2 = fb->height,
 	};
-	int ret;
+	int ret, idx;
 	u8 am_id;
 
+	if (!drm_dev_enter(pipe->crtc.dev, &idx))
+		return;
+
 	DRM_DEBUG_KMS("\n");
 
 	mipi_dbi_hw_reset(mipi);
@@ -207,7 +215,7 @@ static void ili9225_pipe_enable(struct drm_simple_display_pipe *pipe,
 	ret = ili9225_command(mipi, ILI9225_POWER_CONTROL_1, 0x0000);
 	if (ret) {
 		DRM_DEV_ERROR(dev, "Error sending command %d\n", ret);
-		return;
+		goto out_exit;
 	}
 	ili9225_command(mipi, ILI9225_POWER_CONTROL_2, 0x0000);
 	ili9225_command(mipi, ILI9225_POWER_CONTROL_3, 0x0000);
@@ -280,6 +288,8 @@ static void ili9225_pipe_enable(struct drm_simple_display_pipe *pipe,
 
 	mipi->enabled = true;
 	ili9225_fb_dirty(fb, &rect);
+out_exit:
+	drm_dev_exit(idx);
 }
 
 static void ili9225_pipe_disable(struct drm_simple_display_pipe *pipe)
@@ -288,6 +298,13 @@ static void ili9225_pipe_disable(struct drm_simple_display_pipe *pipe)
 
 	DRM_DEBUG_KMS("\n");
 
+	/*
+	 * This callback is not protected by drm_dev_enter/exit since we want to
+	 * turn off the display on regular driver unload. It's highly unlikely
+	 * that the underlying SPI controller is gone should this be called after
+	 * unplug.
+	 */
+
 	if (!mipi->enabled)
 		return;
 
diff --git a/drivers/gpu/drm/tinydrm/ili9341.c b/drivers/gpu/drm/tinydrm/ili9341.c
index 850ce9ed6dd2..d15f85e837ae 100644
--- a/drivers/gpu/drm/tinydrm/ili9341.c
+++ b/drivers/gpu/drm/tinydrm/ili9341.c
@@ -56,13 +56,16 @@ static void yx240qv29_enable(struct drm_simple_display_pipe *pipe,
 {
 	struct mipi_dbi *mipi = drm_to_mipi_dbi(pipe->crtc.dev);
 	u8 addr_mode;
-	int ret;
+	int ret, idx;
+
+	if (!drm_dev_enter(pipe->crtc.dev, &idx))
+		return;
 
 	DRM_DEBUG_KMS("\n");
 
 	ret = mipi_dbi_poweron_conditional_reset(mipi);
 	if (ret < 0)
-		return;
+		goto out_exit;
 	if (ret == 1)
 		goto out_enable;
 
@@ -128,6 +131,8 @@ static void yx240qv29_enable(struct drm_simple_display_pipe *pipe,
 	addr_mode |= ILI9341_MADCTL_BGR;
 	mipi_dbi_command(mipi, MIPI_DCS_SET_ADDRESS_MODE, addr_mode);
 	mipi_dbi_enable_flush(mipi, crtc_state, plane_state);
+out_exit:
+	drm_dev_exit(idx);
 }
 
 static const struct drm_simple_display_pipe_funcs ili9341_pipe_funcs = {
diff --git a/drivers/gpu/drm/tinydrm/mi0283qt.c b/drivers/gpu/drm/tinydrm/mi0283qt.c
index 7aee05586e92..c6dc31084a4e 100644
--- a/drivers/gpu/drm/tinydrm/mi0283qt.c
+++ b/drivers/gpu/drm/tinydrm/mi0283qt.c
@@ -58,13 +58,16 @@ static void mi0283qt_enable(struct drm_simple_display_pipe *pipe,
 {
 	struct mipi_dbi *mipi = drm_to_mipi_dbi(pipe->crtc.dev);
 	u8 addr_mode;
-	int ret;
+	int ret, idx;
+
+	if (!drm_dev_enter(pipe->crtc.dev, &idx))
+		return;
 
 	DRM_DEBUG_KMS("\n");
 
 	ret = mipi_dbi_poweron_conditional_reset(mipi);
 	if (ret < 0)
-		return;
+		goto out_exit;
 	if (ret == 1)
 		goto out_enable;
 
@@ -136,6 +139,8 @@ static void mi0283qt_enable(struct drm_simple_display_pipe *pipe,
 	addr_mode |= ILI9341_MADCTL_BGR;
 	mipi_dbi_command(mipi, MIPI_DCS_SET_ADDRESS_MODE, addr_mode);
 	mipi_dbi_enable_flush(mipi, crtc_state, plane_state);
+out_exit:
+	drm_dev_exit(idx);
 }
 
 static const struct drm_simple_display_pipe_funcs mi0283qt_pipe_funcs = {
diff --git a/drivers/gpu/drm/tinydrm/mipi-dbi.c b/drivers/gpu/drm/tinydrm/mipi-dbi.c
index ecfee357436d..ac4baa5fcc81 100644
--- a/drivers/gpu/drm/tinydrm/mipi-dbi.c
+++ b/drivers/gpu/drm/tinydrm/mipi-dbi.c
@@ -220,13 +220,16 @@ static void mipi_dbi_fb_dirty(struct drm_framebuffer *fb, struct drm_rect *rect)
 	unsigned int height = rect->y2 - rect->y1;
 	unsigned int width = rect->x2 - rect->x1;
 	bool swap = mipi->swap_bytes;
-	int ret = 0;
+	int idx, ret = 0;
 	bool full;
 	void *tr;
 
 	if (!mipi->enabled)
 		return;
 
+	if (!drm_dev_enter(fb->dev, &idx))
+		return;
+
 	full = width == fb->width && height == fb->height;
 
 	DRM_DEBUG_KMS("Flushing [FB:%d] " DRM_RECT_FMT "\n", fb->base.id, DRM_RECT_ARG(rect));
@@ -253,6 +256,8 @@ static void mipi_dbi_fb_dirty(struct drm_framebuffer *fb, struct drm_rect *rect)
 err_msg:
 	if (ret)
 		dev_err_once(fb->dev->dev, "Failed to update display %d\n", ret);
+
+	drm_dev_exit(idx);
 }
 
 /**
@@ -307,10 +312,16 @@ void mipi_dbi_enable_flush(struct mipi_dbi *mipi,
 		.y1 = 0,
 		.y2 = fb->height,
 	};
+	int idx;
+
+	if (!drm_dev_enter(&mipi->drm, &idx))
+		return;
 
 	mipi->enabled = true;
 	mipi_dbi_fb_dirty(fb, &rect);
 	backlight_enable(mipi->backlight);
+
+	drm_dev_exit(idx);
 }
 EXPORT_SYMBOL(mipi_dbi_enable_flush);
 
@@ -320,6 +331,10 @@ static void mipi_dbi_blank(struct mipi_dbi *mipi)
 	u16 height = drm->mode_config.min_height;
 	u16 width = drm->mode_config.min_width;
 	size_t len = width * height * 2;
+	int idx;
+
+	if (!drm_dev_enter(drm, &idx))
+		return;
 
 	memset(mipi->tx_buf, 0, len);
 
@@ -329,6 +344,8 @@ static void mipi_dbi_blank(struct mipi_dbi *mipi)
 			 (height >> 8) & 0xFF, (height - 1) & 0xFF);
 	mipi_dbi_command_buf(mipi, MIPI_DCS_WRITE_MEMORY_START,
 			     (u8 *)mipi->tx_buf, len);
+
+	drm_dev_exit(idx);
 }
 
 /**
@@ -343,6 +360,9 @@ void mipi_dbi_pipe_disable(struct drm_simple_display_pipe *pipe)
 {
 	struct mipi_dbi *mipi = drm_to_mipi_dbi(pipe->crtc.dev);
 
+	if (!mipi->enabled)
+		return;
+
 	DRM_DEBUG_KMS("\n");
 
 	mipi->enabled = false;
@@ -991,11 +1011,16 @@ static ssize_t mipi_dbi_debugfs_command_write(struct file *file,
 	u8 val, cmd = 0, parameters[64];
 	char *buf, *pos, *token;
 	unsigned int i;
-	int ret;
+	int ret, idx;
+
+	if (!drm_dev_enter(&mipi->drm, &idx))
+		return -ENODEV;
 
 	buf = memdup_user_nul(ubuf, count);
-	if (IS_ERR(buf))
-		return PTR_ERR(buf);
+	if (IS_ERR(buf)) {
+		ret = PTR_ERR(buf);
+		goto err_exit;
+	}
 
 	/* strip trailing whitespace */
 	for (i = count - 1; i > 0; i--)
@@ -1031,6 +1056,8 @@ static ssize_t mipi_dbi_debugfs_command_write(struct file *file,
 
 err_free:
 	kfree(buf);
+err_exit:
+	drm_dev_exit(idx);
 
 	return ret < 0 ? ret : count;
 }
@@ -1039,8 +1066,11 @@ static int mipi_dbi_debugfs_command_show(struct seq_file *m, void *unused)
 {
 	struct mipi_dbi *mipi = m->private;
 	u8 cmd, val[4];
+	int ret, idx;
 	size_t len;
-	int ret;
+
+	if (!drm_dev_enter(&mipi->drm, &idx))
+		return -ENODEV;
 
 	for (cmd = 0; cmd < 255; cmd++) {
 		if (!mipi_dbi_command_is_read(mipi, cmd))
@@ -1071,6 +1101,8 @@ static int mipi_dbi_debugfs_command_show(struct seq_file *m, void *unused)
 		seq_printf(m, "%*phN\n", (int)len, val);
 	}
 
+	drm_dev_exit(idx);
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/tinydrm/repaper.c b/drivers/gpu/drm/tinydrm/repaper.c
index b1acf5aebe32..3f3632457079 100644
--- a/drivers/gpu/drm/tinydrm/repaper.c
+++ b/drivers/gpu/drm/tinydrm/repaper.c
@@ -533,8 +533,14 @@ static int repaper_fb_dirty(struct drm_framebuffer *fb)
 	struct dma_buf_attachment *import_attach = cma_obj->base.import_attach;
 	struct repaper_epd *epd = drm_to_epd(fb->dev);
 	struct drm_rect clip;
+	int idx, ret = 0;
 	u8 *buf = NULL;
-	int ret = 0;
+
+	if (!epd->enabled)
+		return 0;
+
+	if (!drm_dev_enter(fb->dev, &idx))
+		return -ENODEV;
 
 	/* repaper can't do partial updates */
 	clip.x1 = 0;
@@ -542,17 +548,16 @@ static int repaper_fb_dirty(struct drm_framebuffer *fb)
 	clip.y1 = 0;
 	clip.y2 = fb->height;
 
-	if (!epd->enabled)
-		return 0;
-
 	repaper_get_temperature(epd);
 
 	DRM_DEBUG("Flushing [FB:%d] st=%ums\n", fb->base.id,
 		  epd->factored_stage_time);
 
 	buf = kmalloc_array(fb->width, fb->height, GFP_KERNEL);
-	if (!buf)
-		return -ENOMEM;
+	if (!buf) {
+		ret = -ENOMEM;
+		goto out_exit;
+	}
 
 	if (import_attach) {
 		ret = dma_buf_begin_cpu_access(import_attach->dmabuf,
@@ -621,6 +626,8 @@ static int repaper_fb_dirty(struct drm_framebuffer *fb)
 
 out_free:
 	kfree(buf);
+out_exit:
+	drm_dev_exit(idx);
 
 	return ret;
 }
@@ -650,7 +657,10 @@ static void repaper_pipe_enable(struct drm_simple_display_pipe *pipe,
 	struct spi_device *spi = epd->spi;
 	struct device *dev = &spi->dev;
 	bool dc_ok = false;
-	int i, ret;
+	int i, ret, idx;
+
+	if (!drm_dev_enter(pipe->crtc.dev, &idx))
+		return;
 
 	DRM_DEBUG_DRIVER("\n");
 
@@ -689,7 +699,7 @@ static void repaper_pipe_enable(struct drm_simple_display_pipe *pipe,
 	if (!i) {
 		DRM_DEV_ERROR(dev, "timeout waiting for panel to become ready.\n");
 		power_off(epd);
-		return;
+		goto out_exit;
 	}
 
 	repaper_read_id(spi);
@@ -700,7 +710,7 @@ static void repaper_pipe_enable(struct drm_simple_display_pipe *pipe,
 		else
 			dev_err(dev, "wrong COG ID 0x%02x\n", ret);
 		power_off(epd);
-		return;
+		goto out_exit;
 	}
 
 	/* Disable OE */
@@ -713,7 +723,7 @@ static void repaper_pipe_enable(struct drm_simple_display_pipe *pipe,
 		else
 			DRM_DEV_ERROR(dev, "panel is reported broken\n");
 		power_off(epd);
-		return;
+		goto out_exit;
 	}
 
 	/* Power saving mode */
@@ -753,7 +763,7 @@ static void repaper_pipe_enable(struct drm_simple_display_pipe *pipe,
 		if (ret < 0) {
 			DRM_DEV_ERROR(dev, "failed to read chip (%d)\n", ret);
 			power_off(epd);
-			return;
+			goto out_exit;
 		}
 
 		if (ret & 0x40) {
@@ -765,7 +775,7 @@ static void repaper_pipe_enable(struct drm_simple_display_pipe *pipe,
 	if (!dc_ok) {
 		DRM_DEV_ERROR(dev, "dc/dc failed\n");
 		power_off(epd);
-		return;
+		goto out_exit;
 	}
 
 	/*
@@ -776,6 +786,8 @@ static void repaper_pipe_enable(struct drm_simple_display_pipe *pipe,
 
 	epd->enabled = true;
 	epd->partial = false;
+out_exit:
+	drm_dev_exit(idx);
 }
 
 static void repaper_pipe_disable(struct drm_simple_display_pipe *pipe)
@@ -784,6 +796,16 @@ static void repaper_pipe_disable(struct drm_simple_display_pipe *pipe)
 	struct spi_device *spi = epd->spi;
 	unsigned int line;
 
+	/*
+	 * This callback is not protected by drm_dev_enter/exit since we want to
+	 * turn off the display on regular driver unload. It's highly unlikely
+	 * that the underlying SPI controller is gone should this be called after
+	 * unplug.
+	 */
+
+	if (!epd->enabled)
+		return;
+
 	DRM_DEBUG_DRIVER("\n");
 
 	epd->enabled = false;
diff --git a/drivers/gpu/drm/tinydrm/st7586.c b/drivers/gpu/drm/tinydrm/st7586.c
index 6e92de809b63..d99957bac532 100644
--- a/drivers/gpu/drm/tinydrm/st7586.c
+++ b/drivers/gpu/drm/tinydrm/st7586.c
@@ -119,12 +119,14 @@ static int st7586_buf_copy(void *dst, struct drm_framebuffer *fb,
 static void st7586_fb_dirty(struct drm_framebuffer *fb, struct drm_rect *rect)
 {
 	struct mipi_dbi *mipi = drm_to_mipi_dbi(fb->dev);
-	int start, end;
-	int ret = 0;
+	int start, end, idx, ret = 0;
 
 	if (!mipi->enabled)
 		return;
 
+	if (!drm_dev_enter(fb->dev, &idx))
+		return;
+
 	/* 3 pixels per byte, so grow clip to nearest multiple of 3 */
 	rect->x1 = rounddown(rect->x1, 3);
 	rect->x2 = roundup(rect->x2, 3);
@@ -152,6 +154,8 @@ static void st7586_fb_dirty(struct drm_framebuffer *fb, struct drm_rect *rect)
 err_msg:
 	if (ret)
 		dev_err_once(fb->dev->dev, "Failed to update display %d\n", ret);
+
+	drm_dev_exit(idx);
 }
 
 static void st7586_pipe_update(struct drm_simple_display_pipe *pipe,
@@ -184,14 +188,17 @@ static void st7586_pipe_enable(struct drm_simple_display_pipe *pipe,
 		.y1 = 0,
 		.y2 = fb->height,
 	};
-	int ret;
+	int idx, ret;
 	u8 addr_mode;
 
+	if (!drm_dev_enter(pipe->crtc.dev, &idx))
+		return;
+
 	DRM_DEBUG_KMS("\n");
 
 	ret = mipi_dbi_poweron_reset(mipi);
 	if (ret)
-		return;
+		goto out_exit;
 
 	mipi_dbi_command(mipi, ST7586_AUTO_READ_CTRL, 0x9f);
 	mipi_dbi_command(mipi, ST7586_OTP_RW_CTRL, 0x00);
@@ -244,12 +251,21 @@ static void st7586_pipe_enable(struct drm_simple_display_pipe *pipe,
 	st7586_fb_dirty(fb, &rect);
 
 	mipi_dbi_command(mipi, MIPI_DCS_SET_DISPLAY_ON);
+out_exit:
+	drm_dev_exit(idx);
 }
 
 static void st7586_pipe_disable(struct drm_simple_display_pipe *pipe)
 {
 	struct mipi_dbi *mipi = drm_to_mipi_dbi(pipe->crtc.dev);
 
+	/*
+	 * This callback is not protected by drm_dev_enter/exit since we want to
+	 * turn off the display on regular driver unload. It's highly unlikely
+	 * that the underlying SPI controller is gone should this be called after
+	 * unplug.
+	 */
+
 	DRM_DEBUG_KMS("\n");
 
 	if (!mipi->enabled)
diff --git a/drivers/gpu/drm/tinydrm/st7735r.c b/drivers/gpu/drm/tinydrm/st7735r.c
index 0f8a346026ac..022e9849b95b 100644
--- a/drivers/gpu/drm/tinydrm/st7735r.c
+++ b/drivers/gpu/drm/tinydrm/st7735r.c
@@ -44,14 +44,17 @@ static void jd_t18003_t01_pipe_enable(struct drm_simple_display_pipe *pipe,
 				      struct drm_plane_state *plane_state)
 {
 	struct mipi_dbi *mipi = drm_to_mipi_dbi(pipe->crtc.dev);
-	int ret;
+	int ret, idx;
 	u8 addr_mode;
 
+	if (!drm_dev_enter(pipe->crtc.dev, &idx))
+		return;
+
 	DRM_DEBUG_KMS("\n");
 
 	ret = mipi_dbi_poweron_reset(mipi);
 	if (ret)
-		return;
+		goto out_exit;
 
 	msleep(150);
 
@@ -102,6 +105,8 @@ static void jd_t18003_t01_pipe_enable(struct drm_simple_display_pipe *pipe,
 	msleep(20);
 
 	mipi_dbi_enable_flush(mipi, crtc_state, plane_state);
+out_exit:
+	drm_dev_exit(idx);
 }
 
 static const struct drm_simple_display_pipe_funcs jd_t18003_t01_pipe_funcs = {
-- 
2.20.1

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

[PATCH v2 12/12] drm/fb-helper: generic: Don't take module ref for fbcon

[Noralf Trønnes]

It's now safe to let fbcon unbind automatically on fbdev unregister.
The crash problem was fixed in commit 2122b40580dd
("fbdev: fbcon: Fix unregister crash when more than one framebuffer")

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_fb_helper.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 523b1067f1ed..18a5593cc9ec 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -2999,7 +2999,8 @@ static int drm_fbdev_fb_open(struct fb_info *info, int user)
 {
 	struct drm_fb_helper *fb_helper = info->par;
 
-	if (!try_module_get(fb_helper->dev->driver->fops->owner))
+	/* No need to take a ref for fbcon because it unbinds on unregister */
+	if (user && !try_module_get(fb_helper->dev->driver->fops->owner))
 		return -ENODEV;
 
 	return 0;
@@ -3009,7 +3010,8 @@ static int drm_fbdev_fb_release(struct fb_info *info, int user)
 {
 	struct drm_fb_helper *fb_helper = info->par;
 
-	module_put(fb_helper->dev->driver->fops->owner);
+	if (user)
+		module_put(fb_helper->dev->driver->fops->owner);
 
 	return 0;
 }
-- 
2.20.1

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

Re: [PATCH v2 03/12] drm/drv: DOC: Add driver example code

[Sam Ravnborg]

Hi Noralf

On Sun, Feb 10, 2019 at 02:10:30PM +0100, Noralf Trønnes wrote:
> Add driver example that shows how devm_drm_dev_init() can be used.
> 
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>

Always good with examples!


> ---
> 
> I'm not sure how detailed such an example such be and a description of 
> some kind is also required. Help is needed :-)
> 
> Noralf.
> 
>  drivers/gpu/drm/drm_drv.c | 118 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 118 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 351f128ec4b7..99ca3551688f 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -286,6 +286,124 @@ void drm_minor_release(struct drm_minor *minor)
>   * Note that the lifetime rules for &drm_device instance has still a lot of
>   * historical baggage. Hence use the reference counting provided by
>   * drm_dev_get() and drm_dev_put() only carefully.
> + *
> + * Display driver example
> + * ~~~~~~~~~~~~~~~~~~~~~~

Maybe add something like this:

The following example shows a typical structure of a DRM display driver.
The example focus on the probe() function and the other functions that is
almost always present.
The example is used to demonstrate the use of devm_drm_dev_init(), and
shows how the use of a device managed drm_device allows for simpler
remove() and shuttdown() functions.


> + *
> + * .. code-block:: c
> + *
> + *	struct driver_device {
> + *		struct drm_device drm;
> + *		void *userspace_facing;
> + *		struct clk *pclk;
> + *	};
> + *
> + *	static inline struct driver_device *drm_to_priv(struct drm_device *drm)
> + *	{
> + *		return container_of(drm, struct driver_device, drm);
> + *	}
> + *
> + *	static void driver_drm_release(struct drm_device *drm)
> + *	{
> + *		struct driver_device *priv = drm_to_priv(drm);
> + *
> + *		drm_mode_config_cleanup(drm);
> + *		drm_dev_fini(drm);
> + *		kfree(priv->userspace_facing);
> + *		kfree(priv);
> + *	}
> + *
> + *	static struct drm_driver driver_drm_driver = {
> + *		[...]
> + *		.release = driver_drm_release,
> + *	};
> + *
> + *	static int driver_probe(struct platform_device *pdev)
> + *	{
> + *		struct driver_device *priv;
> + *		struct drm_device *drm;
> + *		int ret;
> + *
> + *		priv = kzalloc(sizeof(*priv), GFP_KERNEL);
Add a comment that explains why we cannot use devm_kzalloc()?
It is used in a lot of places today...

> + *		if (!priv)
> + *			return -ENOMEM;
> + *
> + *		drm = &priv->drm;
> + *
> + *		ret = devm_drm_dev_init(&pdev->dev, drm, &driver_drm_driver);
> + *		if (ret) {
> + *			kfree(drm);
This should be kfree(priv);

> + *			return ret;
> + *		}
> + *
> + *		drm_mode_config_init(drm);
> + *
> + *		priv->userspace_facing = kzalloc(..., GFP_KERNEL);
> + *		if (!priv->userspace_facing)
> + *			return -ENOMEM;
If we fail here we should also free priv?

> + *
> + *		priv->pclk = devm_clk_get(dev, "PCLK");
> + *		if (IS_ERR(priv->pclk))
> + *			return PTR_ERR(priv->pclk);
> + *
> + *		[ Further setup, display pipeline etc ]
> + *
> + *		drm_mode_config_reset(drm);
> + *
> + *		ret = drm_dev_register(drm);
> + *		if (ret)
> + *			return ret;
> + *
> + *		platform_set_drvdata(pdev, drm);
> + *
> + *		drm_fbdev_generic_setup(drm, 32);
> + *
> + *		return 0;
> + *	}
> + *
> + *	[ This function is called before the devm_ resources are released ]
> + *	static int driver_remove(struct platform_device *pdev)
> + *	{
> + *		struct drm_device *drm = platform_get_drvdata(pdev);
> + *
> + *		drm_dev_unregister(drm);
> + *		drm_atomic_helper_shutdown(drm)
> + *
> + *		return 0;
> + *	}
> + *
> + *	static void driver_shutdown(struct platform_device *pdev)
> + *	{
> + *		drm_atomic_helper_shutdown(platform_get_drvdata(pdev));
> + *	}
> + *
> + *	static int __maybe_unused driver_pm_suspend(struct device *dev)
> + *	{
> + *		return drm_mode_config_helper_suspend(dev_get_drvdata(dev));
> + *	}
> + *
> + *	static int __maybe_unused driver_pm_resume(struct device *dev)
> + *	{
> + *		drm_mode_config_helper_resume(dev_get_drvdata(dev));
> + *
> + *		return 0;
> + *	}
> + *
> + *	static const struct dev_pm_ops driver_pm_ops = {
> + *		SET_SYSTEM_SLEEP_PM_OPS(driver_pm_suspend, driver_pm_resume)
> + *	};
> + *
> + *	static struct platform_driver driver_driver = {
> + *		.driver = {
> + *			[...]
> + *			.pm = &driver_pm_ops,
> + *		},
> + *		.probe = driver_probe,
> + *		.remove = driver_remove,
> + *		.shutdown = driver_shutdown,
> + *	};
> + *	module_platform_driver(driver_driver);
> + *
>   */
>  
>  /**
> -- 
> 2.20.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 04/12] drm/modes: Add DRM_SIMPLE_MODE()

[Sam Ravnborg]

On Sun, Feb 10, 2019 at 02:10:31PM +0100, Noralf Trønnes wrote:
> This adds a helper macro to specify modes that only contain info about
> resolution.
> 
> v2: Actually set the width and height (Ilia Mirkin)
> 
> Cc: Ilia Mirkin <imirkin@alum.mit.edu>
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 05/12] drm/tinydrm: tinydrm_display_pipe_init() don't use tinydrm_device

[Sam Ravnborg]

On Sun, Feb 10, 2019 at 02:10:32PM +0100, Noralf Trønnes wrote:
> Rework function signature so tinydrm_device can be avoided.
> 
> Move definition to tinydrm-helpers.h so tinydrm.h can be deleted in a
> later patch.
> 
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 08/12] drm/tinydrm/repaper: Drop using tinydrm_device

[Sam Ravnborg]

Hi Noralf.

One detail below. When addressed you can add my:
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>

	Sam

On Sun, Feb 10, 2019 at 02:10:35PM +0100, Noralf Trønnes wrote:
> Use devm_drm_dev_init() and drop using tinydrm_device.
> 
> v2: devm_drm_dev_register() was dropped so add a driver release callback.
> 
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> ---
>  drivers/gpu/drm/tinydrm/repaper.c | 84 ++++++++++++++++++++++---------
>  1 file changed, 61 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tinydrm/repaper.c b/drivers/gpu/drm/tinydrm/repaper.c
> index a32dda09458a..b1acf5aebe32 100644
> --- a/drivers/gpu/drm/tinydrm/repaper.c
> +++ b/drivers/gpu/drm/tinydrm/repaper.c
> @@ -30,11 +30,12 @@
>  #include <drm/drm_damage_helper.h>
>  #include <drm/drm_drv.h>
>  #include <drm/drm_fb_cma_helper.h>
> +#include <drm/drm_fb_helper.h>
>  #include <drm/drm_gem_cma_helper.h>
>  #include <drm/drm_gem_framebuffer_helper.h>
>  #include <drm/drm_rect.h>
>  #include <drm/drm_vblank.h>
> -#include <drm/tinydrm/tinydrm.h>
> +#include <drm/drm_simple_kms_helper.h>
>  #include <drm/tinydrm/tinydrm-helpers.h>
>  
>  #define REPAPER_RID_G2_COG_ID	0x12
> @@ -60,7 +61,8 @@ enum repaper_epd_border_byte {
>  };
>  
>  struct repaper_epd {
> -	struct tinydrm_device tinydrm;
> +	struct drm_device drm;
> +	struct drm_simple_display_pipe pipe;
>  	struct spi_device *spi;
>  
>  	struct gpio_desc *panel_on;
> @@ -89,10 +91,9 @@ struct repaper_epd {
>  	bool partial;
>  };
>  
> -static inline struct repaper_epd *
> -epd_from_tinydrm(struct tinydrm_device *tdev)
> +static inline struct repaper_epd *drm_to_epd(struct drm_device *drm)
>  {
> -	return container_of(tdev, struct repaper_epd, tinydrm);
> +	return container_of(drm, struct repaper_epd, drm);
>  }
>  
>  static int repaper_spi_transfer(struct spi_device *spi, u8 header,
> @@ -530,8 +531,7 @@ static int repaper_fb_dirty(struct drm_framebuffer *fb)
>  {
>  	struct drm_gem_cma_object *cma_obj = drm_fb_cma_get_gem_obj(fb, 0);
>  	struct dma_buf_attachment *import_attach = cma_obj->base.import_attach;
> -	struct tinydrm_device *tdev = fb->dev->dev_private;
> -	struct repaper_epd *epd = epd_from_tinydrm(tdev);
> +	struct repaper_epd *epd = drm_to_epd(fb->dev);
>  	struct drm_rect clip;
>  	u8 *buf = NULL;
>  	int ret = 0;
> @@ -646,8 +646,7 @@ static void repaper_pipe_enable(struct drm_simple_display_pipe *pipe,
>  				struct drm_crtc_state *crtc_state,
>  				struct drm_plane_state *plane_state)
>  {
> -	struct tinydrm_device *tdev = pipe_to_tinydrm(pipe);
> -	struct repaper_epd *epd = epd_from_tinydrm(tdev);
> +	struct repaper_epd *epd = drm_to_epd(pipe->crtc.dev);
>  	struct spi_device *spi = epd->spi;
>  	struct device *dev = &spi->dev;
>  	bool dc_ok = false;
> @@ -781,8 +780,7 @@ static void repaper_pipe_enable(struct drm_simple_display_pipe *pipe,
>  
>  static void repaper_pipe_disable(struct drm_simple_display_pipe *pipe)
>  {
> -	struct tinydrm_device *tdev = pipe_to_tinydrm(pipe);
> -	struct repaper_epd *epd = epd_from_tinydrm(tdev);
> +	struct repaper_epd *epd = drm_to_epd(pipe->crtc.dev);
>  	struct spi_device *spi = epd->spi;
>  	unsigned int line;
>  
> @@ -856,6 +854,23 @@ static const struct drm_simple_display_pipe_funcs repaper_pipe_funcs = {
>  	.prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb,
>  };
>  
> +static const struct drm_mode_config_funcs repaper_mode_config_funcs = {
> +	.fb_create = drm_gem_fb_create_with_dirty,
> +	.atomic_check = drm_atomic_helper_check,
> +	.atomic_commit = drm_atomic_helper_commit,
> +};
> +
> +static void repaper_release(struct drm_device *drm)
> +{
> +	struct repaper_epd *epd = drm_to_epd(drm);
> +
> +	DRM_DEBUG_DRIVER("\n");
> +
> +	drm_mode_config_cleanup(drm);
> +	drm_dev_fini(drm);
> +	kfree(epd);
> +}
> +
>  static const uint32_t repaper_formats[] = {
>  	DRM_FORMAT_XRGB8888,
>  };
> @@ -894,6 +909,7 @@ static struct drm_driver repaper_driver = {
>  	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME |
>  				  DRIVER_ATOMIC,
>  	.fops			= &repaper_fops,
> +	.release		= repaper_release,
>  	DRM_GEM_CMA_VMAP_DRIVER_OPS,
>  	.name			= "repaper",
>  	.desc			= "Pervasive Displays RePaper e-ink panels",
> @@ -926,11 +942,11 @@ static int repaper_probe(struct spi_device *spi)
>  	const struct spi_device_id *spi_id;
>  	const struct of_device_id *match;
>  	struct device *dev = &spi->dev;
> -	struct tinydrm_device *tdev;
>  	enum repaper_model model;
>  	const char *thermal_zone;
>  	struct repaper_epd *epd;
>  	size_t line_buffer_size;
> +	struct drm_device *drm;
>  	int ret;
>  
>  	match = of_match_device(repaper_of_match, dev);
> @@ -950,10 +966,21 @@ static int repaper_probe(struct spi_device *spi)
>  		}
>  	}
>  
> -	epd = devm_kzalloc(dev, sizeof(*epd), GFP_KERNEL);
> +	epd = kzalloc(sizeof(*epd), GFP_KERNEL);
>  	if (!epd)
>  		return -ENOMEM;
>  
> +	drm = &epd->drm;
> +
> +	ret = devm_drm_dev_init(dev, drm, &repaper_driver);
> +	if (ret) {
> +		kfree(epd);
> +		return ret;
> +	}
> +
> +	drm_mode_config_init(drm);
> +	drm->mode_config.funcs = &repaper_mode_config_funcs;
> +
>  	epd->spi = spi;
>  
>  	epd->panel_on = devm_gpiod_get(dev, "panel-on", GPIOD_OUT_LOW);
> @@ -1064,26 +1091,36 @@ static int repaper_probe(struct spi_device *spi)
>  	if (!epd->current_frame)
>  		return -ENOMEM;
>  
> -	tdev = &epd->tinydrm;
> -
> -	ret = devm_tinydrm_init(dev, tdev, &repaper_driver);
> -	if (ret)
> -		return ret;
> -
> -	ret = tinydrm_display_pipe_init(tdev->drm, &tdev->pipe, &repaper_pipe_funcs,
> +	ret = tinydrm_display_pipe_init(drm, &epd->pipe, &repaper_pipe_funcs,
>  					DRM_MODE_CONNECTOR_VIRTUAL,
>  					repaper_formats,
>  					ARRAY_SIZE(repaper_formats), mode, 0);
>  	if (ret)
>  		return ret;
Do we leak epd here?
(I recall we already discussed this, but I am not sure...)

If this in handled by devm_drm_dev_init() then maybe add  comment about
it in the example provided in previous patch.

>  
> -	drm_mode_config_reset(tdev->drm);
> +	drm_mode_config_reset(drm);
>  
> -	spi_set_drvdata(spi, tdev->drm);
> +	ret = drm_dev_register(drm, 0);
> +	if (ret)
> +		return ret;
> +
> +	spi_set_drvdata(spi, drm);
>  
>  	DRM_DEBUG_DRIVER("SPI speed: %uMHz\n", spi->max_speed_hz / 1000000);
>  
> -	return devm_tinydrm_register(tdev);
> +	drm_fbdev_generic_setup(drm, 32);
> +
> +	return 0;
> +}
> +
> +static int repaper_remove(struct spi_device *spi)
> +{
> +	struct drm_device *drm = spi_get_drvdata(spi);
> +
> +	drm_dev_unplug(drm);
The sample code uses drm_dev_unregister() here?


> +	drm_atomic_helper_shutdown(drm);
> +
> +	return 0;
>  }
>  
>  static void repaper_shutdown(struct spi_device *spi)
> @@ -1099,6 +1136,7 @@ static struct spi_driver repaper_spi_driver = {
>  	},
>  	.id_table = repaper_id,
>  	.probe = repaper_probe,
> +	.remove = repaper_remove,
>  	.shutdown = repaper_shutdown,
>  };
>  module_spi_driver(repaper_spi_driver);
> -- 
> 2.20.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 09/12] drm/tinydrm: Drop using tinydrm_device

[Sam Ravnborg]

Hi Noralf.

On Sun, Feb 10, 2019 at 02:10:36PM +0100, Noralf Trønnes wrote:
> Use devm_drm_dev_init() and drop using tinydrm_device.
> 
> v2: devm_drm_dev_register() was dropped so add driver release callbacks.
> 
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> ---
>  drivers/gpu/drm/tinydrm/hx8357d.c  |  40 +++++++++--
>  drivers/gpu/drm/tinydrm/ili9225.c  |  40 +++++++++--
>  drivers/gpu/drm/tinydrm/ili9341.c  |  40 +++++++++--
>  drivers/gpu/drm/tinydrm/mi0283qt.c |  40 +++++++++--
>  drivers/gpu/drm/tinydrm/mipi-dbi.c |  67 +++++++++++-------
>  drivers/gpu/drm/tinydrm/st7586.c   | 105 ++++++++++++++++-------------
>  drivers/gpu/drm/tinydrm/st7735r.c  |  40 +++++++++--
>  include/drm/tinydrm/mipi-dbi.h     |  26 ++++---
>  8 files changed, 294 insertions(+), 104 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tinydrm/hx8357d.c b/drivers/gpu/drm/tinydrm/hx8357d.c
> index 84dda622df85..e9b9e08fafc7 100644
> --- a/drivers/gpu/drm/tinydrm/hx8357d.c
> +++ b/drivers/gpu/drm/tinydrm/hx8357d.c
> @@ -18,6 +18,7 @@
>  
>  #include <drm/drm_atomic_helper.h>
>  #include <drm/drm_drv.h>
> +#include <drm/drm_fb_helper.h>
>  #include <drm/drm_gem_cma_helper.h>
>  #include <drm/drm_gem_framebuffer_helper.h>
>  #include <drm/drm_modeset_helper.h>
> @@ -189,6 +190,7 @@ DEFINE_DRM_GEM_CMA_FOPS(hx8357d_fops);
>  static struct drm_driver hx8357d_driver = {
>  	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME | DRIVER_ATOMIC,
>  	.fops			= &hx8357d_fops,
> +	.release		= mipi_dbi_release,
>  	DRM_GEM_CMA_VMAP_DRIVER_OPS,
>  	.debugfs_init		= mipi_dbi_debugfs_init,
>  	.name			= "hx8357d",
> @@ -213,15 +215,25 @@ MODULE_DEVICE_TABLE(spi, hx8357d_id);
>  static int hx8357d_probe(struct spi_device *spi)
>  {
>  	struct device *dev = &spi->dev;
> +	struct drm_device *drm;
>  	struct mipi_dbi *mipi;
>  	struct gpio_desc *dc;
>  	u32 rotation = 0;
>  	int ret;
>  
> -	mipi = devm_kzalloc(dev, sizeof(*mipi), GFP_KERNEL);
> +	mipi = kzalloc(sizeof(*mipi), GFP_KERNEL);
>  	if (!mipi)
>  		return -ENOMEM;
>  
> +	drm = &mipi->drm;
> +	ret = devm_drm_dev_init(dev, drm, &hx8357d_driver);
> +	if (ret) {
> +		kfree(mipi);
> +		return ret;
> +	}
> +
> +	drm_mode_config_init(drm);
> +
>  	dc = devm_gpiod_get(dev, "dc", GPIOD_OUT_LOW);
>  	if (IS_ERR(dc)) {
>  		DRM_DEV_ERROR(dev, "Failed to get gpio 'dc'\n");
> @@ -238,14 +250,31 @@ static int hx8357d_probe(struct spi_device *spi)
>  	if (ret)
>  		return ret;
>  
> -	ret = mipi_dbi_init(&spi->dev, mipi, &hx8357d_pipe_funcs,
> -			    &hx8357d_driver, &yx350hv15_mode, rotation);
> +	ret = mipi_dbi_init(mipi, &hx8357d_pipe_funcs, &yx350hv15_mode, rotation);
>  	if (ret)
>  		return ret;
>  
> -	spi_set_drvdata(spi, mipi->tinydrm.drm);
> +	drm_mode_config_reset(drm);
>  
> -	return devm_tinydrm_register(&mipi->tinydrm);
> +	ret = drm_dev_register(drm, 0);
> +	if (ret)
> +		return ret;
> +
> +	spi_set_drvdata(spi, drm);
> +
> +	drm_fbdev_generic_setup(drm, 32);
> +
> +	return 0;
> +}
> +
> +static int hx8357d_remove(struct spi_device *spi)
> +{
> +	struct drm_device *drm = spi_get_drvdata(spi);
> +
> +	drm_dev_unplug(drm);
> +	drm_atomic_helper_shutdown(drm);
> +
> +	return 0;
>  }
The sample code uses drm_dev_unregister() in the _remove() function.
Do I miss something obvious?

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

Re: [PATCH v2 03/12] drm/drv: DOC: Add driver example code

[Noralf Trønnes]

Den 10.02.2019 22.03, skrev Sam Ravnborg:
> Hi Noralf
> 
> On Sun, Feb 10, 2019 at 02:10:30PM +0100, Noralf Trønnes wrote:
>> Add driver example that shows how devm_drm_dev_init() can be used.
>>
>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> 
> Always good with examples!
> 
> 
>> ---
>>
>> I'm not sure how detailed such an example such be and a description of 
>> some kind is also required. Help is needed :-)
>>
>> Noralf.
>>
>>  drivers/gpu/drm/drm_drv.c | 118 ++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 118 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
>> index 351f128ec4b7..99ca3551688f 100644
>> --- a/drivers/gpu/drm/drm_drv.c
>> +++ b/drivers/gpu/drm/drm_drv.c
>> @@ -286,6 +286,124 @@ void drm_minor_release(struct drm_minor *minor)
>>   * Note that the lifetime rules for &drm_device instance has still a lot of
>>   * historical baggage. Hence use the reference counting provided by
>>   * drm_dev_get() and drm_dev_put() only carefully.
>> + *
>> + * Display driver example
>> + * ~~~~~~~~~~~~~~~~~~~~~~
> 
> Maybe add something like this:
> 
> The following example shows a typical structure of a DRM display driver.
> The example focus on the probe() function and the other functions that is
> almost always present.
> The example is used to demonstrate the use of devm_drm_dev_init(), and
> shows how the use of a device managed drm_device allows for simpler
> remove() and shuttdown() functions.

Using devm_drm_dev_init() doesn't affect the use of shutdown(), and for
remove() it only saves one drm_dev_put(). The main benefit that I see is
for a simpler probe() function.

Maybe the last part can be:

The example is used to demonstrate the use of devm_drm_dev_init() and
its accompanying drm_driver->release callback.

Thanks for helping me with this. Daniel is big on documentation so I try
to follow up, but words for humans isn't my strong side :-)

> 
> 
>> + *
>> + * .. code-block:: c
>> + *
>> + *	struct driver_device {
>> + *		struct drm_device drm;

Maybe I should add a comment for userspace_facing:

     *		[ Represents a resource that is accessed by userspace ]
>> + *		void *userspace_facing;
>> + *		struct clk *pclk;
>> + *	};
>> + *
>> + *	static inline struct driver_device *drm_to_priv(struct drm_device *drm)
>> + *	{
>> + *		return container_of(drm, struct driver_device, drm);
>> + *	}
>> + *
>> + *	static void driver_drm_release(struct drm_device *drm)
>> + *	{
>> + *		struct driver_device *priv = drm_to_priv(drm);
>> + *
>> + *		drm_mode_config_cleanup(drm);
>> + *		drm_dev_fini(drm);
>> + *		kfree(priv->userspace_facing);
>> + *		kfree(priv);
>> + *	}
>> + *
>> + *	static struct drm_driver driver_drm_driver = {
>> + *		[...]
>> + *		.release = driver_drm_release,
>> + *	};
>> + *
>> + *	static int driver_probe(struct platform_device *pdev)
>> + *	{
>> + *		struct driver_device *priv;
>> + *		struct drm_device *drm;
>> + *		int ret;
>> + *
>> + *		priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> Add a comment that explains why we cannot use devm_kzalloc()?
> It is used in a lot of places today...

That makes sense:

devm_kzalloc() can't be used here because the drm_device lifetime can
exceed the device lifetime if driver unbind happens when userspace still
has open file descriptors.

> 
>> + *		if (!priv)
>> + *			return -ENOMEM;
>> + *
>> + *		drm = &priv->drm;
>> + *
>> + *		ret = devm_drm_dev_init(&pdev->dev, drm, &driver_drm_driver);
>> + *		if (ret) {
>> + *			kfree(drm);
> This should be kfree(priv);
> 
>> + *			return ret;
>> + *		}
>> + *
>> + *		drm_mode_config_init(drm);
>> + *
>> + *		priv->userspace_facing = kzalloc(..., GFP_KERNEL);
>> + *		if (!priv->userspace_facing)
>> + *			return -ENOMEM;
> If we fail here we should also free priv?
> 

This is where one benfit of device managed resources kicks in.
If probing fails, happens in drivers/base/dd.c:really_probe(),
devres_release_all() is called which results in:
devm_drm_dev_init_release() -> drm_dev_put() -> driver_drm_release().

If the driver only uses devm_ managed resources or releases the
resource(s) in drm_driver->release, there's no need for an error path in
the probe function.

The ->userspace_facing resource is an attempt to showcase a resource
that is released in drm_driver->release, but I'm unsure if it's really a
good example. It would have been better to have a real world example.

>> + *
>> + *		priv->pclk = devm_clk_get(dev, "PCLK");
>> + *		if (IS_ERR(priv->pclk))
>> + *			return PTR_ERR(priv->pclk);
>> + *
>> + *		[ Further setup, display pipeline etc ]
>> + *
>> + *		drm_mode_config_reset(drm);
>> + *
>> + *		ret = drm_dev_register(drm);
>> + *		if (ret)
>> + *			return ret;
>> + *
>> + *		platform_set_drvdata(pdev, drm);
>> + *
>> + *		drm_fbdev_generic_setup(drm, 32);
>> + *
>> + *		return 0;
>> + *	}
>> + *
>> + *	[ This function is called before the devm_ resources are released ]
>> + *	static int driver_remove(struct platform_device *pdev)
>> + *	{
>> + *		struct drm_device *drm = platform_get_drvdata(pdev);
>> + *
>> + *		drm_dev_unregister(drm);

You asked about drm_dev_unplug() in the other patch. It is used if the
driver supports device unplug, like for USB or unloading a Device Tree
overlay (not sure if all the pieces exists in mainline yet for that).

Maybe I can add this section after the example:

Drivers that want to support device unplugging (USB, DT overlay unload)
should use drm_dev_unplug() instead of drm_dev_unregister(). The driver
must protect regions that is accessing device resources to avoid use
after they're released. This is done using drm_dev_enter() and
drm_dev_exit(). There is one shortcoming however, drm_dev_unplug() marks
the drm_device as unplugged before drm_atomic_helper_shutdown() is
called. This means that if the disable code paths are protected, they
will not run on regular driver module unload, possibily leaving the
hardware enabled.


>> + *		drm_atomic_helper_shutdown(drm)
>> + *
>> + *		return 0;
>> + *	}
>> + *

I can add a comment for shutdown:

This function is called on kernel restart and shutdown

>> + *	static void driver_shutdown(struct platform_device *pdev)
>> + *	{
>> + *		drm_atomic_helper_shutdown(platform_get_drvdata(pdev));
>> + *	}
>> + *
>> + *	static int __maybe_unused driver_pm_suspend(struct device *dev)
>> + *	{
>> + *		return drm_mode_config_helper_suspend(dev_get_drvdata(dev));
>> + *	}
>> + *
>> + *	static int __maybe_unused driver_pm_resume(struct device *dev)
>> + *	{
>> + *		drm_mode_config_helper_resume(dev_get_drvdata(dev));
>> + *
>> + *		return 0;
>> + *	}
>> + *
>> + *	static const struct dev_pm_ops driver_pm_ops = {
>> + *		SET_SYSTEM_SLEEP_PM_OPS(driver_pm_suspend, driver_pm_resume)
>> + *	};
>> + *
>> + *	static struct platform_driver driver_driver = {
>> + *		.driver = {
>> + *			[...]
>> + *			.pm = &driver_pm_ops,
>> + *		},
>> + *		.probe = driver_probe,
>> + *		.remove = driver_remove,
>> + *		.shutdown = driver_shutdown,
>> + *	};
>> + *	module_platform_driver(driver_driver);
>> + *
>>   */
>>  
>>  /**
>> -- 
>> 2.20.1
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 03/12] drm/drv: DOC: Add driver example code

[Daniel Vetter]

On Sun, Feb 10, 2019 at 02:10:30PM +0100, Noralf Trønnes wrote:
> Add driver example that shows how devm_drm_dev_init() can be used.
> 
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> ---
> 
> I'm not sure how detailed such an example such be and a description of 
> some kind is also required. Help is needed :-)

Personally I'd make it more minimal, pseudo-code style, removing most of
the boilerplate (e.g. the container_of wrapper). That way it can highlight
the important bits more. But it's already very minimal.

Wrt text, maybe a minimal intro paragraph. Aside from that looks all good
to me.

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

One question below.
> 
> Noralf.
> 
>  drivers/gpu/drm/drm_drv.c | 118 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 118 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 351f128ec4b7..99ca3551688f 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -286,6 +286,124 @@ void drm_minor_release(struct drm_minor *minor)
>   * Note that the lifetime rules for &drm_device instance has still a lot of
>   * historical baggage. Hence use the reference counting provided by
>   * drm_dev_get() and drm_dev_put() only carefully.
> + *
> + * Display driver example
> + * ~~~~~~~~~~~~~~~~~~~~~~
> + *
> + * .. code-block:: c
> + *
> + *	struct driver_device {
> + *		struct drm_device drm;
> + *		void *userspace_facing;
> + *		struct clk *pclk;
> + *	};
> + *
> + *	static inline struct driver_device *drm_to_priv(struct drm_device *drm)
> + *	{
> + *		return container_of(drm, struct driver_device, drm);
> + *	}
> + *
> + *	static void driver_drm_release(struct drm_device *drm)
> + *	{
> + *		struct driver_device *priv = drm_to_priv(drm);
> + *
> + *		drm_mode_config_cleanup(drm);
> + *		drm_dev_fini(drm);
> + *		kfree(priv->userspace_facing);
> + *		kfree(priv);
> + *	}
> + *
> + *	static struct drm_driver driver_drm_driver = {
> + *		[...]
> + *		.release = driver_drm_release,
> + *	};
> + *
> + *	static int driver_probe(struct platform_device *pdev)
> + *	{
> + *		struct driver_device *priv;
> + *		struct drm_device *drm;
> + *		int ret;
> + *
> + *		priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> + *		if (!priv)
> + *			return -ENOMEM;
> + *
> + *		drm = &priv->drm;
> + *
> + *		ret = devm_drm_dev_init(&pdev->dev, drm, &driver_drm_driver);
> + *		if (ret) {
> + *			kfree(drm);
> + *			return ret;
> + *		}
> + *
> + *		drm_mode_config_init(drm);
> + *
> + *		priv->userspace_facing = kzalloc(..., GFP_KERNEL);
> + *		if (!priv->userspace_facing)
> + *			return -ENOMEM;
> + *
> + *		priv->pclk = devm_clk_get(dev, "PCLK");
> + *		if (IS_ERR(priv->pclk))
> + *			return PTR_ERR(priv->pclk);
> + *
> + *		[ Further setup, display pipeline etc ]
> + *
> + *		drm_mode_config_reset(drm);
> + *
> + *		ret = drm_dev_register(drm);
> + *		if (ret)
> + *			return ret;
> + *
> + *		platform_set_drvdata(pdev, drm);

Hm, don't we need to set that before registering? Or even before
drm_mode_config_reset? Kinda depends upon what the driver is using the
drvdata for.

> + *
> + *		drm_fbdev_generic_setup(drm, 32);
> + *
> + *		return 0;
> + *	}
> + *
> + *	[ This function is called before the devm_ resources are released ]
> + *	static int driver_remove(struct platform_device *pdev)
> + *	{
> + *		struct drm_device *drm = platform_get_drvdata(pdev);
> + *
> + *		drm_dev_unregister(drm);
> + *		drm_atomic_helper_shutdown(drm)
> + *
> + *		return 0;
> + *	}
> + *
> + *	static void driver_shutdown(struct platform_device *pdev)
> + *	{
> + *		drm_atomic_helper_shutdown(platform_get_drvdata(pdev));
> + *	}
> + *
> + *	static int __maybe_unused driver_pm_suspend(struct device *dev)
> + *	{
> + *		return drm_mode_config_helper_suspend(dev_get_drvdata(dev));
> + *	}
> + *
> + *	static int __maybe_unused driver_pm_resume(struct device *dev)
> + *	{
> + *		drm_mode_config_helper_resume(dev_get_drvdata(dev));
> + *
> + *		return 0;
> + *	}
> + *
> + *	static const struct dev_pm_ops driver_pm_ops = {
> + *		SET_SYSTEM_SLEEP_PM_OPS(driver_pm_suspend, driver_pm_resume)
> + *	};
> + *
> + *	static struct platform_driver driver_driver = {
> + *		.driver = {
> + *			[...]
> + *			.pm = &driver_pm_ops,
> + *		},
> + *		.probe = driver_probe,
> + *		.remove = driver_remove,
> + *		.shutdown = driver_shutdown,
> + *	};
> + *	module_platform_driver(driver_driver);
> + *
>   */
>  
>  /**
> -- 
> 2.20.1
> 

-- 
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 v2 03/12] drm/drv: DOC: Add driver example code

[Noralf Trønnes]

Den 12.02.2019 15.06, skrev Daniel Vetter:
> On Sun, Feb 10, 2019 at 02:10:30PM +0100, Noralf Trønnes wrote:
>> Add driver example that shows how devm_drm_dev_init() can be used.
>>
>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>> ---
>>
>> I'm not sure how detailed such an example such be and a description of 
>> some kind is also required. Help is needed :-)
> 
> Personally I'd make it more minimal, pseudo-code style, removing most of
> the boilerplate (e.g. the container_of wrapper). That way it can highlight
> the important bits more. But it's already very minimal.
> 
> Wrt text, maybe a minimal intro paragraph. Aside from that looks all good
> to me.
> 
> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> One question below.
>>
>> Noralf.
>>
>>  drivers/gpu/drm/drm_drv.c | 118 ++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 118 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
>> index 351f128ec4b7..99ca3551688f 100644
>> --- a/drivers/gpu/drm/drm_drv.c
>> +++ b/drivers/gpu/drm/drm_drv.c
>> @@ -286,6 +286,124 @@ void drm_minor_release(struct drm_minor *minor)
>>   * Note that the lifetime rules for &drm_device instance has still a lot of
>>   * historical baggage. Hence use the reference counting provided by
>>   * drm_dev_get() and drm_dev_put() only carefully.
>> + *
>> + * Display driver example
>> + * ~~~~~~~~~~~~~~~~~~~~~~
>> + *
>> + * .. code-block:: c
>> + *
>> + *	struct driver_device {
>> + *		struct drm_device drm;
>> + *		void *userspace_facing;
>> + *		struct clk *pclk;
>> + *	};
>> + *
>> + *	static inline struct driver_device *drm_to_priv(struct drm_device *drm)
>> + *	{
>> + *		return container_of(drm, struct driver_device, drm);
>> + *	}
>> + *
>> + *	static void driver_drm_release(struct drm_device *drm)
>> + *	{
>> + *		struct driver_device *priv = drm_to_priv(drm);
>> + *
>> + *		drm_mode_config_cleanup(drm);
>> + *		drm_dev_fini(drm);
>> + *		kfree(priv->userspace_facing);
>> + *		kfree(priv);
>> + *	}
>> + *
>> + *	static struct drm_driver driver_drm_driver = {
>> + *		[...]
>> + *		.release = driver_drm_release,
>> + *	};
>> + *
>> + *	static int driver_probe(struct platform_device *pdev)
>> + *	{
>> + *		struct driver_device *priv;
>> + *		struct drm_device *drm;
>> + *		int ret;
>> + *
>> + *		priv = kzalloc(sizeof(*priv), GFP_KERNEL);
>> + *		if (!priv)
>> + *			return -ENOMEM;
>> + *
>> + *		drm = &priv->drm;
>> + *
>> + *		ret = devm_drm_dev_init(&pdev->dev, drm, &driver_drm_driver);
>> + *		if (ret) {
>> + *			kfree(drm);
>> + *			return ret;
>> + *		}
>> + *
>> + *		drm_mode_config_init(drm);
>> + *
>> + *		priv->userspace_facing = kzalloc(..., GFP_KERNEL);
>> + *		if (!priv->userspace_facing)
>> + *			return -ENOMEM;
>> + *
>> + *		priv->pclk = devm_clk_get(dev, "PCLK");
>> + *		if (IS_ERR(priv->pclk))
>> + *			return PTR_ERR(priv->pclk);
>> + *
>> + *		[ Further setup, display pipeline etc ]
>> + *
>> + *		drm_mode_config_reset(drm);
>> + *
>> + *		ret = drm_dev_register(drm);
>> + *		if (ret)
>> + *			return ret;
>> + *
>> + *		platform_set_drvdata(pdev, drm);
> 
> Hm, don't we need to set that before registering? Or even before
> drm_mode_config_reset? Kinda depends upon what the driver is using the
> drvdata for.
> 

Yeah, it depends. I can move up though, it doesn't hurt, since drvdata
is cleared by the driver core if probe fails.

Noralf.

>> + *
>> + *		drm_fbdev_generic_setup(drm, 32);
>> + *
>> + *		return 0;
>> + *	}
>> + *
>> + *	[ This function is called before the devm_ resources are released ]
>> + *	static int driver_remove(struct platform_device *pdev)
>> + *	{
>> + *		struct drm_device *drm = platform_get_drvdata(pdev);
>> + *
>> + *		drm_dev_unregister(drm);
>> + *		drm_atomic_helper_shutdown(drm)
>> + *
>> + *		return 0;
>> + *	}
>> + *
>> + *	static void driver_shutdown(struct platform_device *pdev)
>> + *	{
>> + *		drm_atomic_helper_shutdown(platform_get_drvdata(pdev));
>> + *	}
>> + *
>> + *	static int __maybe_unused driver_pm_suspend(struct device *dev)
>> + *	{
>> + *		return drm_mode_config_helper_suspend(dev_get_drvdata(dev));
>> + *	}
>> + *
>> + *	static int __maybe_unused driver_pm_resume(struct device *dev)
>> + *	{
>> + *		drm_mode_config_helper_resume(dev_get_drvdata(dev));
>> + *
>> + *		return 0;
>> + *	}
>> + *
>> + *	static const struct dev_pm_ops driver_pm_ops = {
>> + *		SET_SYSTEM_SLEEP_PM_OPS(driver_pm_suspend, driver_pm_resume)
>> + *	};
>> + *
>> + *	static struct platform_driver driver_driver = {
>> + *		.driver = {
>> + *			[...]
>> + *			.pm = &driver_pm_ops,
>> + *		},
>> + *		.probe = driver_probe,
>> + *		.remove = driver_remove,
>> + *		.shutdown = driver_shutdown,
>> + *	};
>> + *	module_platform_driver(driver_driver);
>> + *
>>   */
>>  
>>  /**
>> -- 
>> 2.20.1
>>
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel