[PATCH v2 0/8] drm/mipi-dbi: Convert to shadow-plane helpers

[PATCH v2 0/8] drm/mipi-dbi: Convert to shadow-plane helpers

[Thomas Zimmermann]

Convert the MIPI-DBI-based drivers to shadow-plane helpers. The
drivers vmap/vunmap GEM buffer memory during the atomic commit.
Shadow-plane helpers automate this process.

Patches 1 to 4 prepare the MIPI code for the change and simplify
the rest of the patchset.

Patches 5 to 7 rework the vmap code in the MIPI-DBI drivers and add
shadow-plane helpers. Most of the affected drivers call MIPI-DBI
helpers and get the update automatically. Only ili9225 and st7586
require changes to their source code.

Patch 8 simplifies drm_dev_enter() and _exit(). It's not strictly
needed, but streamlines the driver code and make sense overall.

Testing is welcome, as I don't have any hardware to test these
changes myself.

v2:
	* fix vmap()/vaddr confusion (Noralf)
	* drop struct mipi_dbi_plane_state (Noralf)

Thomas Zimmermann (8):
  drm/simple-kms: Remove drm_gem_simple_display_pipe_prepare_fb()
  drm/ili9225: Call MIPI DBI mode_valid helper
  drm/st7586: Call MIPI DBI mode_valid helper
  drm/mipi-dbi: Initialize default driver functions with macro
  drm/mipi-dbi: Prepare framebuffer copy operation in pipe-update
    helpers
  drm/mipi-dbi: Support shadow-plane state
  drm/mipi-dbi: Use shadow-plane mappings
  drm/mipi-dbi: Move drm_dev_{enter,exit}() out from fb_dirty functions

 drivers/gpu/drm/drm_gem_atomic_helper.c      |  31 +---
 drivers/gpu/drm/drm_mipi_dbi.c               | 144 ++++++++++++++-----
 drivers/gpu/drm/drm_simple_kms_helper.c      |   2 +-
 drivers/gpu/drm/panel/panel-ilitek-ili9341.c |   6 +-
 drivers/gpu/drm/tiny/hx8357d.c               |   5 +-
 drivers/gpu/drm/tiny/ili9163.c               |   6 +-
 drivers/gpu/drm/tiny/ili9225.c               |  36 +++--
 drivers/gpu/drm/tiny/ili9341.c               |   5 +-
 drivers/gpu/drm/tiny/ili9486.c               |   5 +-
 drivers/gpu/drm/tiny/mi0283qt.c              |   5 +-
 drivers/gpu/drm/tiny/panel-mipi-dbi.c        |   5 +-
 drivers/gpu/drm/tiny/st7586.c                |  39 +++--
 drivers/gpu/drm/tiny/st7735r.c               |   5 +-
 include/drm/drm_gem_atomic_helper.h          |   2 -
 include/drm/drm_mipi_dbi.h                   |  36 ++++-
 include/drm/drm_plane.h                      |   4 +-
 include/drm/drm_simple_kms_helper.h          |   4 +-
 17 files changed, 205 insertions(+), 135 deletions(-)


base-commit: ad232f8a0287b805a7a167eddad30fe33fbec9d5
-- 
2.38.1

[PATCH v2 1/8] drm/simple-kms: Remove drm_gem_simple_display_pipe_prepare_fb()

[Thomas Zimmermann]

The helper drm_gem_simple_display_pipe_prepare_fb() is simple-KMS'
default implementation for prepare_fb. Remove the call from drivers
that set it explicitly. Then inline the helper into the only caller
within simple-kms helpers. No functional changes.

Simple-KMS drivers that implement the prepare_fb callback should call
drm_gem_plane_helper_prepare_fb() directly.

v2:
	* fix typo in commit message

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/drm_gem_atomic_helper.c      | 31 +-------------------
 drivers/gpu/drm/drm_simple_kms_helper.c      |  2 +-
 drivers/gpu/drm/panel/panel-ilitek-ili9341.c |  1 -
 drivers/gpu/drm/tiny/ili9163.c               |  1 -
 include/drm/drm_gem_atomic_helper.h          |  2 --
 include/drm/drm_plane.h                      |  4 +--
 include/drm/drm_simple_kms_helper.h          |  4 +--
 7 files changed, 6 insertions(+), 39 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem_atomic_helper.c b/drivers/gpu/drm/drm_gem_atomic_helper.c
index e42800718f51..5d4b9cd077f7 100644
--- a/drivers/gpu/drm/drm_gem_atomic_helper.c
+++ b/drivers/gpu/drm/drm_gem_atomic_helper.c
@@ -26,11 +26,8 @@
  * call drm_gem_plane_helper_prepare_fb() from their implementation of
  * struct &drm_plane_helper.prepare_fb . It sets the plane's fence from
  * the framebuffer so that the DRM core can synchronize access automatically.
- *
  * drm_gem_plane_helper_prepare_fb() can also be used directly as
- * implementation of prepare_fb. For drivers based on
- * struct drm_simple_display_pipe, drm_gem_simple_display_pipe_prepare_fb()
- * provides equivalent functionality.
+ * implementation of prepare_fb.
  *
  * .. code-block:: c
  *
@@ -41,11 +38,6 @@
  *		. prepare_fb = drm_gem_plane_helper_prepare_fb,
  *	};
  *
- *	struct drm_simple_display_pipe_funcs driver_pipe_funcs = {
- *		...,
- *		. prepare_fb = drm_gem_simple_display_pipe_prepare_fb,
- *	};
- *
  * A driver using a shadow buffer copies the content of the shadow buffers
  * into the HW's framebuffer memory during an atomic update. This requires
  * a mapping of the shadow buffer into kernel address space. The mappings
@@ -205,27 +197,6 @@ int drm_gem_plane_helper_prepare_fb(struct drm_plane *plane,
 }
 EXPORT_SYMBOL_GPL(drm_gem_plane_helper_prepare_fb);
 
-/**
- * drm_gem_simple_display_pipe_prepare_fb - prepare_fb helper for &drm_simple_display_pipe
- * @pipe: Simple display pipe
- * @plane_state: Plane state
- *
- * This function uses drm_gem_plane_helper_prepare_fb() to extract the fences
- * from &drm_gem_object.resv and attaches them to the plane state for the atomic
- * helper to wait on. This is necessary to correctly implement implicit
- * synchronization for any buffers shared as a struct &dma_buf. Drivers can use
- * this as their &drm_simple_display_pipe_funcs.prepare_fb callback.
- *
- * See drm_gem_plane_helper_prepare_fb() for a discussion of implicit and
- * explicit fencing in atomic modeset updates.
- */
-int drm_gem_simple_display_pipe_prepare_fb(struct drm_simple_display_pipe *pipe,
-					   struct drm_plane_state *plane_state)
-{
-	return drm_gem_plane_helper_prepare_fb(&pipe->plane, plane_state);
-}
-EXPORT_SYMBOL(drm_gem_simple_display_pipe_prepare_fb);
-
 /*
  * Shadow-buffered Planes
  */
diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c b/drivers/gpu/drm/drm_simple_kms_helper.c
index 3ef420ec4534..270523ae36d4 100644
--- a/drivers/gpu/drm/drm_simple_kms_helper.c
+++ b/drivers/gpu/drm/drm_simple_kms_helper.c
@@ -267,7 +267,7 @@ static int drm_simple_kms_plane_prepare_fb(struct drm_plane *plane,
 
 		WARN_ON_ONCE(pipe->funcs && pipe->funcs->cleanup_fb);
 
-		return drm_gem_simple_display_pipe_prepare_fb(pipe, state);
+		return drm_gem_plane_helper_prepare_fb(plane, state);
 	}
 
 	return pipe->funcs->prepare_fb(pipe, state);
diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
index 384a724f2822..be088983aa7c 100644
--- a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
+++ b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
@@ -581,7 +581,6 @@ static const struct drm_simple_display_pipe_funcs ili9341_dbi_funcs = {
 	.enable = ili9341_dbi_enable,
 	.disable = mipi_dbi_pipe_disable,
 	.update = mipi_dbi_pipe_update,
-	.prepare_fb = drm_gem_simple_display_pipe_prepare_fb,
 };
 
 static const struct drm_display_mode ili9341_dbi_mode = {
diff --git a/drivers/gpu/drm/tiny/ili9163.c b/drivers/gpu/drm/tiny/ili9163.c
index ca0451f79962..835ed12792d5 100644
--- a/drivers/gpu/drm/tiny/ili9163.c
+++ b/drivers/gpu/drm/tiny/ili9163.c
@@ -104,7 +104,6 @@ static const struct drm_simple_display_pipe_funcs ili9163_pipe_funcs = {
 	.enable = yx240qv29_enable,
 	.disable = mipi_dbi_pipe_disable,
 	.update = mipi_dbi_pipe_update,
-	.prepare_fb = drm_gem_simple_display_pipe_prepare_fb,
 };
 
 static const struct drm_display_mode yx240qv29_mode = {
diff --git a/include/drm/drm_gem_atomic_helper.h b/include/drm/drm_gem_atomic_helper.h
index 6970ccb787e2..40b8b039518e 100644
--- a/include/drm/drm_gem_atomic_helper.h
+++ b/include/drm/drm_gem_atomic_helper.h
@@ -15,8 +15,6 @@ struct drm_simple_display_pipe;
  */
 
 int drm_gem_plane_helper_prepare_fb(struct drm_plane *plane, struct drm_plane_state *state);
-int drm_gem_simple_display_pipe_prepare_fb(struct drm_simple_display_pipe *pipe,
-					   struct drm_plane_state *plane_state);
 
 /*
  * Helpers for planes with shadow buffers
diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
index 447e664e49d5..51291983ea44 100644
--- a/include/drm/drm_plane.h
+++ b/include/drm/drm_plane.h
@@ -77,8 +77,8 @@ struct drm_plane_state {
 	 * write this field directly for a driver's implicit fence.
 	 *
 	 * Drivers should store any implicit fence in this from their
-	 * &drm_plane_helper_funcs.prepare_fb callback. See drm_gem_plane_helper_prepare_fb()
-	 * and drm_gem_simple_display_pipe_prepare_fb() for suitable helpers.
+	 * &drm_plane_helper_funcs.prepare_fb callback. See
+	 * drm_gem_plane_helper_prepare_fb() for a suitable helper.
 	 */
 	struct dma_fence *fence;
 
diff --git a/include/drm/drm_simple_kms_helper.h b/include/drm/drm_simple_kms_helper.h
index 2298fe3af4cd..b2486d073763 100644
--- a/include/drm/drm_simple_kms_helper.h
+++ b/include/drm/drm_simple_kms_helper.h
@@ -117,9 +117,9 @@ struct drm_simple_display_pipe_funcs {
 	 * more details.
 	 *
 	 * For GEM drivers who neither have a @prepare_fb nor @cleanup_fb hook
-	 * set drm_gem_simple_display_pipe_prepare_fb() is called automatically
+	 * set, drm_gem_plane_helper_prepare_fb() is called automatically
 	 * to implement this. Other drivers which need additional plane
-	 * processing can call drm_gem_simple_display_pipe_prepare_fb() from
+	 * processing can call drm_gem_plane_helper_prepare_fb() from
 	 * their @prepare_fb hook.
 	 */
 	int (*prepare_fb)(struct drm_simple_display_pipe *pipe,
-- 
2.38.1

[PATCH v2 2/8] drm/ili9225: Call MIPI DBI mode_valid helper

[Thomas Zimmermann]

MIPI DBI drivers validate each mode against their native resolution.
Add this test to ili9225.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/tiny/ili9225.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/tiny/ili9225.c b/drivers/gpu/drm/tiny/ili9225.c
index 815bab285823..f05a2d25866c 100644
--- a/drivers/gpu/drm/tiny/ili9225.c
+++ b/drivers/gpu/drm/tiny/ili9225.c
@@ -326,6 +326,7 @@ static int ili9225_dbi_command(struct mipi_dbi *dbi, u8 *cmd, u8 *par,
 }
 
 static const struct drm_simple_display_pipe_funcs ili9225_pipe_funcs = {
+	.mode_valid	= mipi_dbi_pipe_mode_valid,
 	.enable		= ili9225_pipe_enable,
 	.disable	= ili9225_pipe_disable,
 	.update		= ili9225_pipe_update,
-- 
2.38.1

[PATCH v2 3/8] drm/st7586: Call MIPI DBI mode_valid helper

[Thomas Zimmermann]

MIPI DBI drivers validate each mode against their native resolution.
Add this test to st7586.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/tiny/st7586.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/tiny/st7586.c b/drivers/gpu/drm/tiny/st7586.c
index ce57fa9917e5..6bdd23e2a47c 100644
--- a/drivers/gpu/drm/tiny/st7586.c
+++ b/drivers/gpu/drm/tiny/st7586.c
@@ -263,6 +263,7 @@ static const u32 st7586_formats[] = {
 };
 
 static const struct drm_simple_display_pipe_funcs st7586_pipe_funcs = {
+	.mode_valid	= mipi_dbi_pipe_mode_valid,
 	.enable		= st7586_pipe_enable,
 	.disable	= st7586_pipe_disable,
 	.update		= st7586_pipe_update,
-- 
2.38.1

[PATCH v2 4/8] drm/mipi-dbi: Initialize default driver functions with macro

[Thomas Zimmermann]

Introduce DRM_MIPI_DBI_SIMPLE_DISPLAY_PIPE_FUNCS to initialize MIPI-DBI
helpers to default values and convert drivers. The prepare_fb function
set by some drivers is called implicitly by simple-kms helpers, so leave
it out.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Noralf Trønnes <noralf@tronnes.org>
Tested-by: Javier Martinez Canillas <javierm@redhat.com>
Tested-by: Noralf Trønnes <noralf@tronnes.org> # drm/tiny/mi0283qt
---
 drivers/gpu/drm/panel/panel-ilitek-ili9341.c |  5 +----
 drivers/gpu/drm/tiny/hx8357d.c               |  5 +----
 drivers/gpu/drm/tiny/ili9163.c               |  5 +----
 drivers/gpu/drm/tiny/ili9341.c               |  5 +----
 drivers/gpu/drm/tiny/ili9486.c               |  5 +----
 drivers/gpu/drm/tiny/mi0283qt.c              |  5 +----
 drivers/gpu/drm/tiny/panel-mipi-dbi.c        |  5 +----
 drivers/gpu/drm/tiny/st7735r.c               |  5 +----
 include/drm/drm_mipi_dbi.h                   | 16 ++++++++++++++++
 9 files changed, 24 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
index be088983aa7c..3fdf884b3257 100644
--- a/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
+++ b/drivers/gpu/drm/panel/panel-ilitek-ili9341.c
@@ -577,10 +577,7 @@ static void ili9341_dbi_enable(struct drm_simple_display_pipe *pipe,
 }
 
 static const struct drm_simple_display_pipe_funcs ili9341_dbi_funcs = {
-	.mode_valid = mipi_dbi_pipe_mode_valid,
-	.enable = ili9341_dbi_enable,
-	.disable = mipi_dbi_pipe_disable,
-	.update = mipi_dbi_pipe_update,
+	DRM_MIPI_DBI_SIMPLE_DISPLAY_PIPE_FUNCS(ili9341_dbi_enable),
 };
 
 static const struct drm_display_mode ili9341_dbi_mode = {
diff --git a/drivers/gpu/drm/tiny/hx8357d.c b/drivers/gpu/drm/tiny/hx8357d.c
index 9f634f720817..cdc4486e059b 100644
--- a/drivers/gpu/drm/tiny/hx8357d.c
+++ b/drivers/gpu/drm/tiny/hx8357d.c
@@ -181,10 +181,7 @@ static void yx240qv29_enable(struct drm_simple_display_pipe *pipe,
 }
 
 static const struct drm_simple_display_pipe_funcs hx8357d_pipe_funcs = {
-	.mode_valid = mipi_dbi_pipe_mode_valid,
-	.enable = yx240qv29_enable,
-	.disable = mipi_dbi_pipe_disable,
-	.update = mipi_dbi_pipe_update,
+	DRM_MIPI_DBI_SIMPLE_DISPLAY_PIPE_FUNCS(yx240qv29_enable),
 };
 
 static const struct drm_display_mode yx350hv15_mode = {
diff --git a/drivers/gpu/drm/tiny/ili9163.c b/drivers/gpu/drm/tiny/ili9163.c
index 835ed12792d5..bc4384d410fc 100644
--- a/drivers/gpu/drm/tiny/ili9163.c
+++ b/drivers/gpu/drm/tiny/ili9163.c
@@ -100,10 +100,7 @@ static void yx240qv29_enable(struct drm_simple_display_pipe *pipe,
 }
 
 static const struct drm_simple_display_pipe_funcs ili9163_pipe_funcs = {
-	.mode_valid = mipi_dbi_pipe_mode_valid,
-	.enable = yx240qv29_enable,
-	.disable = mipi_dbi_pipe_disable,
-	.update = mipi_dbi_pipe_update,
+	DRM_MIPI_DBI_SIMPLE_DISPLAY_PIPE_FUNCS(yx240qv29_enable),
 };
 
 static const struct drm_display_mode yx240qv29_mode = {
diff --git a/drivers/gpu/drm/tiny/ili9341.c b/drivers/gpu/drm/tiny/ili9341.c
index 420f6005a956..47b61c3bf145 100644
--- a/drivers/gpu/drm/tiny/ili9341.c
+++ b/drivers/gpu/drm/tiny/ili9341.c
@@ -137,10 +137,7 @@ static void yx240qv29_enable(struct drm_simple_display_pipe *pipe,
 }
 
 static const struct drm_simple_display_pipe_funcs ili9341_pipe_funcs = {
-	.mode_valid = mipi_dbi_pipe_mode_valid,
-	.enable = yx240qv29_enable,
-	.disable = mipi_dbi_pipe_disable,
-	.update = mipi_dbi_pipe_update,
+	DRM_MIPI_DBI_SIMPLE_DISPLAY_PIPE_FUNCS(yx240qv29_enable),
 };
 
 static const struct drm_display_mode yx240qv29_mode = {
diff --git a/drivers/gpu/drm/tiny/ili9486.c b/drivers/gpu/drm/tiny/ili9486.c
index 1bb847466b10..9f735d84d85d 100644
--- a/drivers/gpu/drm/tiny/ili9486.c
+++ b/drivers/gpu/drm/tiny/ili9486.c
@@ -150,10 +150,7 @@ static void waveshare_enable(struct drm_simple_display_pipe *pipe,
 }
 
 static const struct drm_simple_display_pipe_funcs waveshare_pipe_funcs = {
-	.mode_valid = mipi_dbi_pipe_mode_valid,
-	.enable = waveshare_enable,
-	.disable = mipi_dbi_pipe_disable,
-	.update = mipi_dbi_pipe_update,
+	DRM_MIPI_DBI_SIMPLE_DISPLAY_PIPE_FUNCS(waveshare_enable),
 };
 
 static const struct drm_display_mode waveshare_mode = {
diff --git a/drivers/gpu/drm/tiny/mi0283qt.c b/drivers/gpu/drm/tiny/mi0283qt.c
index 47df2b5a3048..01ff43c8ac3f 100644
--- a/drivers/gpu/drm/tiny/mi0283qt.c
+++ b/drivers/gpu/drm/tiny/mi0283qt.c
@@ -141,10 +141,7 @@ static void mi0283qt_enable(struct drm_simple_display_pipe *pipe,
 }
 
 static const struct drm_simple_display_pipe_funcs mi0283qt_pipe_funcs = {
-	.mode_valid = mipi_dbi_pipe_mode_valid,
-	.enable = mi0283qt_enable,
-	.disable = mipi_dbi_pipe_disable,
-	.update = mipi_dbi_pipe_update,
+	DRM_MIPI_DBI_SIMPLE_DISPLAY_PIPE_FUNCS(mi0283qt_enable),
 };
 
 static const struct drm_display_mode mi0283qt_mode = {
diff --git a/drivers/gpu/drm/tiny/panel-mipi-dbi.c b/drivers/gpu/drm/tiny/panel-mipi-dbi.c
index 03a7d569cd56..2ed23ded5199 100644
--- a/drivers/gpu/drm/tiny/panel-mipi-dbi.c
+++ b/drivers/gpu/drm/tiny/panel-mipi-dbi.c
@@ -212,10 +212,7 @@ static void panel_mipi_dbi_enable(struct drm_simple_display_pipe *pipe,
 }
 
 static const struct drm_simple_display_pipe_funcs panel_mipi_dbi_pipe_funcs = {
-	.mode_valid = mipi_dbi_pipe_mode_valid,
-	.enable = panel_mipi_dbi_enable,
-	.disable = mipi_dbi_pipe_disable,
-	.update = mipi_dbi_pipe_update,
+	DRM_MIPI_DBI_SIMPLE_DISPLAY_PIPE_FUNCS(panel_mipi_dbi_enable),
 };
 
 DEFINE_DRM_GEM_DMA_FOPS(panel_mipi_dbi_fops);
diff --git a/drivers/gpu/drm/tiny/st7735r.c b/drivers/gpu/drm/tiny/st7735r.c
index 15d9cf283c66..477eb36fbb70 100644
--- a/drivers/gpu/drm/tiny/st7735r.c
+++ b/drivers/gpu/drm/tiny/st7735r.c
@@ -133,10 +133,7 @@ static void st7735r_pipe_enable(struct drm_simple_display_pipe *pipe,
 }
 
 static const struct drm_simple_display_pipe_funcs st7735r_pipe_funcs = {
-	.mode_valid	= mipi_dbi_pipe_mode_valid,
-	.enable		= st7735r_pipe_enable,
-	.disable	= mipi_dbi_pipe_disable,
-	.update		= mipi_dbi_pipe_update,
+	DRM_MIPI_DBI_SIMPLE_DISPLAY_PIPE_FUNCS(st7735r_pipe_enable),
 };
 
 static const struct st7735r_cfg jd_t18003_t01_cfg = {
diff --git a/include/drm/drm_mipi_dbi.h b/include/drm/drm_mipi_dbi.h
index 14eaecb1825c..8c4ea7956d61 100644
--- a/include/drm/drm_mipi_dbi.h
+++ b/include/drm/drm_mipi_dbi.h
@@ -207,4 +207,20 @@ void mipi_dbi_debugfs_init(struct drm_minor *minor);
 static inline void mipi_dbi_debugfs_init(struct drm_minor *minor) {}
 #endif
 
+/**
+ * DRM_MIPI_DBI_SIMPLE_DISPLAY_PIPE_FUNCS - Initializes struct drm_simple_display_pipe_funcs
+ *                                          for MIPI-DBI devices
+ * @enable_: Enable-callback implementation
+ *
+ * This macro initializes struct drm_simple_display_pipe_funcs with default
+ * values for MIPI-DBI-based devices. The only callback that depends on the
+ * hardware is @enable, for which the driver has to provide an implementation.
+ * MIPI-based drivers are encouraged to use this macro for initialization.
+ */
+#define DRM_MIPI_DBI_SIMPLE_DISPLAY_PIPE_FUNCS(enable_) \
+	.mode_valid = mipi_dbi_pipe_mode_valid, \
+	.enable = (enable_), \
+	.disable = mipi_dbi_pipe_disable, \
+	.update = mipi_dbi_pipe_update
+
 #endif /* __LINUX_MIPI_DBI_H */
-- 
2.38.1

[PATCH v2 5/8] drm/mipi-dbi: Prepare framebuffer copy operation in pipe-update helpers

[Thomas Zimmermann]

Move the vmap/vunmap blocks from the inner fb_dirty helpers into the
MIPI DBI update helpers. The function calls can result in waiting and/or
processing overhead. Reduce the penalties by executing the functions once
in the outer-most function of the pipe update.

This change also prepares for MIPI DBI for shadow-plane helpers. With
shadow-plane helpers, transfer source buffers are mapped into kernel
address space automatically.

v2:
	* keep each driver's existing buffer-mapping patter (Noralf)
	* zero-initialize iosys_map arrays (Noralf)

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Noralf Trønnes <noralf@tronnes.org>
Tested-by: Javier Martinez Canillas <javierm@redhat.com>
Tested-by: Noralf Trønnes <noralf@tronnes.org> # drm/tiny/mi0283qt
---
 drivers/gpu/drm/drm_mipi_dbi.c | 63 ++++++++++++++++++----------------
 drivers/gpu/drm/tiny/ili9225.c | 25 ++++++++++----
 drivers/gpu/drm/tiny/st7586.c  | 28 ++++++++++-----
 include/drm/drm_mipi_dbi.h     |  6 ++--
 4 files changed, 75 insertions(+), 47 deletions(-)

diff --git a/drivers/gpu/drm/drm_mipi_dbi.c b/drivers/gpu/drm/drm_mipi_dbi.c
index a6ac56580876..f58123327ed6 100644
--- a/drivers/gpu/drm/drm_mipi_dbi.c
+++ b/drivers/gpu/drm/drm_mipi_dbi.c
@@ -192,6 +192,7 @@ EXPORT_SYMBOL(mipi_dbi_command_stackbuf);
 /**
  * mipi_dbi_buf_copy - Copy a framebuffer, transforming it if necessary
  * @dst: The destination buffer
+ * @src: The source buffer
  * @fb: The source framebuffer
  * @clip: Clipping rectangle of the area to be copied
  * @swap: When true, swap MSB/LSB of 16-bit values
@@ -199,12 +200,10 @@ EXPORT_SYMBOL(mipi_dbi_command_stackbuf);
  * Returns:
  * Zero on success, negative error code on failure.
  */
-int mipi_dbi_buf_copy(void *dst, struct drm_framebuffer *fb,
+int mipi_dbi_buf_copy(void *dst, struct iosys_map *src, struct drm_framebuffer *fb,
 		      struct drm_rect *clip, bool swap)
 {
 	struct drm_gem_object *gem = drm_gem_fb_get_obj(fb, 0);
-	struct iosys_map map[DRM_FORMAT_MAX_PLANES];
-	struct iosys_map data[DRM_FORMAT_MAX_PLANES];
 	struct iosys_map dst_map = IOSYS_MAP_INIT_VADDR(dst);
 	int ret;
 
@@ -212,19 +211,15 @@ int mipi_dbi_buf_copy(void *dst, struct drm_framebuffer *fb,
 	if (ret)
 		return ret;
 
-	ret = drm_gem_fb_vmap(fb, map, data);
-	if (ret)
-		goto out_drm_gem_fb_end_cpu_access;
-
 	switch (fb->format->format) {
 	case DRM_FORMAT_RGB565:
 		if (swap)
-			drm_fb_swab(&dst_map, NULL, data, fb, clip, !gem->import_attach);
+			drm_fb_swab(&dst_map, NULL, src, fb, clip, !gem->import_attach);
 		else
-			drm_fb_memcpy(&dst_map, NULL, data, fb, clip);
+			drm_fb_memcpy(&dst_map, NULL, src, fb, clip);
 		break;
 	case DRM_FORMAT_XRGB8888:
-		drm_fb_xrgb8888_to_rgb565(&dst_map, NULL, data, fb, clip, swap);
+		drm_fb_xrgb8888_to_rgb565(&dst_map, NULL, src, fb, clip, swap);
 		break;
 	default:
 		drm_err_once(fb->dev, "Format is not supported: %p4cc\n",
@@ -232,8 +227,6 @@ int mipi_dbi_buf_copy(void *dst, struct drm_framebuffer *fb,
 		ret = -EINVAL;
 	}
 
-	drm_gem_fb_vunmap(fb, map);
-out_drm_gem_fb_end_cpu_access:
 	drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE);
 
 	return ret;
@@ -257,10 +250,9 @@ static void mipi_dbi_set_window_address(struct mipi_dbi_dev *dbidev,
 			 ys & 0xff, (ye >> 8) & 0xff, ye & 0xff);
 }
 
-static void mipi_dbi_fb_dirty(struct drm_framebuffer *fb, struct drm_rect *rect)
+static void mipi_dbi_fb_dirty(struct iosys_map *src, struct drm_framebuffer *fb,
+			      struct drm_rect *rect)
 {
-	struct iosys_map map[DRM_FORMAT_MAX_PLANES];
-	struct iosys_map data[DRM_FORMAT_MAX_PLANES];
 	struct mipi_dbi_dev *dbidev = drm_to_mipi_dbi_dev(fb->dev);
 	unsigned int height = rect->y2 - rect->y1;
 	unsigned int width = rect->x2 - rect->x1;
@@ -270,16 +262,9 @@ static void mipi_dbi_fb_dirty(struct drm_framebuffer *fb, struct drm_rect *rect)
 	bool full;
 	void *tr;
 
-	if (WARN_ON(!fb))
-		return;
-
 	if (!drm_dev_enter(fb->dev, &idx))
 		return;
 
-	ret = drm_gem_fb_vmap(fb, map, data);
-	if (ret)
-		goto err_drm_dev_exit;
-
 	full = width == fb->width && height == fb->height;
 
 	DRM_DEBUG_KMS("Flushing [FB:%d] " DRM_RECT_FMT "\n", fb->base.id, DRM_RECT_ARG(rect));
@@ -287,11 +272,11 @@ static void mipi_dbi_fb_dirty(struct drm_framebuffer *fb, struct drm_rect *rect)
 	if (!dbi->dc || !full || swap ||
 	    fb->format->format == DRM_FORMAT_XRGB8888) {
 		tr = dbidev->tx_buf;
-		ret = mipi_dbi_buf_copy(dbidev->tx_buf, fb, rect, swap);
+		ret = mipi_dbi_buf_copy(tr, src, fb, rect, swap);
 		if (ret)
 			goto err_msg;
 	} else {
-		tr = data[0].vaddr; /* TODO: Use mapping abstraction properly */
+		tr = src->vaddr; /* TODO: Use mapping abstraction properly */
 	}
 
 	mipi_dbi_set_window_address(dbidev, rect->x1, rect->x2 - 1, rect->y1,
@@ -303,9 +288,6 @@ static void mipi_dbi_fb_dirty(struct drm_framebuffer *fb, struct drm_rect *rect)
 	if (ret)
 		drm_err_once(fb->dev, "Failed to update display %d\n", ret);
 
-	drm_gem_fb_vunmap(fb, map);
-
-err_drm_dev_exit:
 	drm_dev_exit(idx);
 }
 
@@ -338,14 +320,27 @@ EXPORT_SYMBOL(mipi_dbi_pipe_mode_valid);
 void mipi_dbi_pipe_update(struct drm_simple_display_pipe *pipe,
 			  struct drm_plane_state *old_state)
 {
+	struct iosys_map map[DRM_FORMAT_MAX_PLANES] = { };
+	struct iosys_map data[DRM_FORMAT_MAX_PLANES] = { };
 	struct drm_plane_state *state = pipe->plane.state;
+	struct drm_framebuffer *fb = state->fb;
 	struct drm_rect rect;
+	int ret;
 
 	if (!pipe->crtc.state->active)
 		return;
 
+	if (WARN_ON(!fb))
+		return;
+
+	ret = drm_gem_fb_vmap(fb, map, data);
+	if (ret)
+		return;
+
 	if (drm_atomic_helper_damage_merged(old_state, state, &rect))
-		mipi_dbi_fb_dirty(state->fb, &rect);
+		mipi_dbi_fb_dirty(&data[0], fb, &rect);
+
+	drm_gem_fb_vunmap(fb, map);
 }
 EXPORT_SYMBOL(mipi_dbi_pipe_update);
 
@@ -373,14 +368,22 @@ void mipi_dbi_enable_flush(struct mipi_dbi_dev *dbidev,
 		.y1 = 0,
 		.y2 = fb->height,
 	};
-	int idx;
+	struct iosys_map map[DRM_FORMAT_MAX_PLANES] = { };
+	struct iosys_map data[DRM_FORMAT_MAX_PLANES] = { };
+	int idx, ret;
 
 	if (!drm_dev_enter(&dbidev->drm, &idx))
 		return;
 
-	mipi_dbi_fb_dirty(fb, &rect);
+	ret = drm_gem_fb_vmap(fb, map, data);
+	if (ret)
+		goto err_drm_dev_exit;
+
+	mipi_dbi_fb_dirty(&data[0], fb, &rect);
 	backlight_enable(dbidev->backlight);
 
+	drm_gem_fb_vunmap(fb, map);
+err_drm_dev_exit:
 	drm_dev_exit(idx);
 }
 EXPORT_SYMBOL(mipi_dbi_enable_flush);
diff --git a/drivers/gpu/drm/tiny/ili9225.c b/drivers/gpu/drm/tiny/ili9225.c
index f05a2d25866c..ae94c74d0163 100644
--- a/drivers/gpu/drm/tiny/ili9225.c
+++ b/drivers/gpu/drm/tiny/ili9225.c
@@ -25,6 +25,7 @@
 #include <drm/drm_framebuffer.h>
 #include <drm/drm_gem_atomic_helper.h>
 #include <drm/drm_gem_dma_helper.h>
+#include <drm/drm_gem_framebuffer_helper.h>
 #include <drm/drm_managed.h>
 #include <drm/drm_mipi_dbi.h>
 #include <drm/drm_rect.h>
@@ -76,9 +77,9 @@ static inline int ili9225_command(struct mipi_dbi *dbi, u8 cmd, u16 data)
 	return mipi_dbi_command_buf(dbi, cmd, par, 2);
 }
 
-static void ili9225_fb_dirty(struct drm_framebuffer *fb, struct drm_rect *rect)
+static void ili9225_fb_dirty(struct iosys_map *src, struct drm_framebuffer *fb,
+			     struct drm_rect *rect)
 {
-	struct drm_gem_dma_object *dma_obj = drm_fb_dma_get_gem_obj(fb, 0);
 	struct mipi_dbi_dev *dbidev = drm_to_mipi_dbi_dev(fb->dev);
 	unsigned int height = rect->y2 - rect->y1;
 	unsigned int width = rect->x2 - rect->x1;
@@ -100,11 +101,11 @@ static void ili9225_fb_dirty(struct drm_framebuffer *fb, struct drm_rect *rect)
 	if (!dbi->dc || !full || swap ||
 	    fb->format->format == DRM_FORMAT_XRGB8888) {
 		tr = dbidev->tx_buf;
-		ret = mipi_dbi_buf_copy(dbidev->tx_buf, fb, rect, swap);
+		ret = mipi_dbi_buf_copy(tr, src, fb, rect, swap);
 		if (ret)
 			goto err_msg;
 	} else {
-		tr = dma_obj->vaddr;
+		tr = src->vaddr; /* TODO: Use mapping abstraction properly */
 	}
 
 	switch (dbidev->rotation) {
@@ -163,13 +164,19 @@ static void ili9225_pipe_update(struct drm_simple_display_pipe *pipe,
 				struct drm_plane_state *old_state)
 {
 	struct drm_plane_state *state = pipe->plane.state;
+	struct drm_framebuffer *fb = state->fb;
+	struct drm_gem_dma_object *dma_obj;
+	struct iosys_map src;
 	struct drm_rect rect;
 
 	if (!pipe->crtc.state->active)
 		return;
 
+	dma_obj = drm_fb_dma_get_gem_obj(fb, 0);
+	iosys_map_set_vaddr(&src, dma_obj->vaddr);
+
 	if (drm_atomic_helper_damage_merged(old_state, state, &rect))
-		ili9225_fb_dirty(state->fb, &rect);
+		ili9225_fb_dirty(&src, fb, &rect);
 }
 
 static void ili9225_pipe_enable(struct drm_simple_display_pipe *pipe,
@@ -186,6 +193,8 @@ static void ili9225_pipe_enable(struct drm_simple_display_pipe *pipe,
 		.y1 = 0,
 		.y2 = fb->height,
 	};
+	struct drm_gem_dma_object *dma_obj;
+	struct iosys_map src;
 	int ret, idx;
 	u8 am_id;
 
@@ -276,7 +285,11 @@ static void ili9225_pipe_enable(struct drm_simple_display_pipe *pipe,
 
 	ili9225_command(dbi, ILI9225_DISPLAY_CONTROL_1, 0x1017);
 
-	ili9225_fb_dirty(fb, &rect);
+	dma_obj = drm_fb_dma_get_gem_obj(fb, 0);
+	iosys_map_set_vaddr(&src, dma_obj->vaddr);
+
+	ili9225_fb_dirty(&src, fb, &rect);
+
 out_exit:
 	drm_dev_exit(idx);
 }
diff --git a/drivers/gpu/drm/tiny/st7586.c b/drivers/gpu/drm/tiny/st7586.c
index 6bdd23e2a47c..e773b1f2fd5f 100644
--- a/drivers/gpu/drm/tiny/st7586.c
+++ b/drivers/gpu/drm/tiny/st7586.c
@@ -92,25 +92,24 @@ static void st7586_xrgb8888_to_gray332(u8 *dst, void *vaddr,
 	kfree(buf);
 }
 
-static int st7586_buf_copy(void *dst, struct drm_framebuffer *fb,
+static int st7586_buf_copy(void *dst, struct iosys_map *src, struct drm_framebuffer *fb,
 			   struct drm_rect *clip)
 {
-	struct drm_gem_dma_object *dma_obj = drm_fb_dma_get_gem_obj(fb, 0);
-	void *src = dma_obj->vaddr;
-	int ret = 0;
+	int ret;
 
 	ret = drm_gem_fb_begin_cpu_access(fb, DMA_FROM_DEVICE);
 	if (ret)
 		return ret;
 
-	st7586_xrgb8888_to_gray332(dst, src, fb, clip);
+	st7586_xrgb8888_to_gray332(dst, src->vaddr, fb, clip);
 
 	drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE);
 
 	return 0;
 }
 
-static void st7586_fb_dirty(struct drm_framebuffer *fb, struct drm_rect *rect)
+static void st7586_fb_dirty(struct iosys_map *src, struct drm_framebuffer *fb,
+			    struct drm_rect *rect)
 {
 	struct mipi_dbi_dev *dbidev = drm_to_mipi_dbi_dev(fb->dev);
 	struct mipi_dbi *dbi = &dbidev->dbi;
@@ -125,7 +124,7 @@ static void st7586_fb_dirty(struct drm_framebuffer *fb, struct drm_rect *rect)
 
 	DRM_DEBUG_KMS("Flushing [FB:%d] " DRM_RECT_FMT "\n", fb->base.id, DRM_RECT_ARG(rect));
 
-	ret = st7586_buf_copy(dbidev->tx_buf, fb, rect);
+	ret = st7586_buf_copy(dbidev->tx_buf, src, fb, rect);
 	if (ret)
 		goto err_msg;
 
@@ -154,13 +153,19 @@ static void st7586_pipe_update(struct drm_simple_display_pipe *pipe,
 			       struct drm_plane_state *old_state)
 {
 	struct drm_plane_state *state = pipe->plane.state;
+	struct drm_framebuffer *fb = state->fb;
+	struct drm_gem_dma_object *dma_obj;
+	struct iosys_map src;
 	struct drm_rect rect;
 
 	if (!pipe->crtc.state->active)
 		return;
 
+	dma_obj = drm_fb_dma_get_gem_obj(fb, 0);
+	iosys_map_set_vaddr(&src, dma_obj->vaddr);
+
 	if (drm_atomic_helper_damage_merged(old_state, state, &rect))
-		st7586_fb_dirty(state->fb, &rect);
+		st7586_fb_dirty(&src, fb, &rect);
 }
 
 static void st7586_pipe_enable(struct drm_simple_display_pipe *pipe,
@@ -176,6 +181,8 @@ static void st7586_pipe_enable(struct drm_simple_display_pipe *pipe,
 		.y1 = 0,
 		.y2 = fb->height,
 	};
+	struct drm_gem_dma_object *dma_obj;
+	struct iosys_map src;
 	int idx, ret;
 	u8 addr_mode;
 
@@ -235,7 +242,10 @@ static void st7586_pipe_enable(struct drm_simple_display_pipe *pipe,
 
 	msleep(100);
 
-	st7586_fb_dirty(fb, &rect);
+	dma_obj = drm_fb_dma_get_gem_obj(fb, 0);
+	iosys_map_set_vaddr(&src, dma_obj->vaddr);
+
+	st7586_fb_dirty(&src, fb, &rect);
 
 	mipi_dbi_command(dbi, MIPI_DCS_SET_DISPLAY_ON);
 out_exit:
diff --git a/include/drm/drm_mipi_dbi.h b/include/drm/drm_mipi_dbi.h
index 8c4ea7956d61..36ac8495566b 100644
--- a/include/drm/drm_mipi_dbi.h
+++ b/include/drm/drm_mipi_dbi.h
@@ -13,9 +13,10 @@
 #include <drm/drm_simple_kms_helper.h>
 
 struct drm_rect;
-struct spi_device;
 struct gpio_desc;
+struct iosys_map;
 struct regulator;
+struct spi_device;
 
 /**
  * struct mipi_dbi - MIPI DBI interface
@@ -176,8 +177,9 @@ int mipi_dbi_command_read(struct mipi_dbi *dbi, u8 cmd, u8 *val);
 int mipi_dbi_command_buf(struct mipi_dbi *dbi, u8 cmd, u8 *data, size_t len);
 int mipi_dbi_command_stackbuf(struct mipi_dbi *dbi, u8 cmd, const u8 *data,
 			      size_t len);
-int mipi_dbi_buf_copy(void *dst, struct drm_framebuffer *fb,
+int mipi_dbi_buf_copy(void *dst, struct iosys_map *src, struct drm_framebuffer *fb,
 		      struct drm_rect *clip, bool swap);
+
 /**
  * mipi_dbi_command - MIPI DCS command with optional parameter(s)
  * @dbi: MIPI DBI structure
-- 
2.38.1

[PATCH v2 6/8] drm/mipi-dbi: Support shadow-plane state

[Thomas Zimmermann]

Implement MIPI DBI planes with struct drm_shadow_plane_state, so that the
respective drivers can use the vmap'ed GEM-buffer memory. Implement state
helpers, the {begin,end}_fb_access helpers and wire up everything.

With this commit, MIPI DBI drivers can access the GEM object's memory
that is provided by shadow-plane state. The actual changes to drivers
are implemented separately.

v2:
	* use shadow-plane state directly (Noralf)

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Tested-by: Javier Martinez Canillas <javierm@redhat.com>
Tested-by: Noralf Trønnes <noralf@tronnes.org> # drm/tiny/mi0283qt
---
 drivers/gpu/drm/drm_mipi_dbi.c | 85 ++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/tiny/ili9225.c |  5 ++
 drivers/gpu/drm/tiny/st7586.c  |  5 ++
 include/drm/drm_mipi_dbi.h     | 16 ++++++-
 4 files changed, 110 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_mipi_dbi.c b/drivers/gpu/drm/drm_mipi_dbi.c
index f58123327ed6..b808de61c5bc 100644
--- a/drivers/gpu/drm/drm_mipi_dbi.c
+++ b/drivers/gpu/drm/drm_mipi_dbi.c
@@ -21,6 +21,7 @@
 #include <drm/drm_fourcc.h>
 #include <drm/drm_framebuffer.h>
 #include <drm/drm_gem.h>
+#include <drm/drm_gem_atomic_helper.h>
 #include <drm/drm_gem_framebuffer_helper.h>
 #include <drm/drm_mipi_dbi.h>
 #include <drm/drm_modes.h>
@@ -433,6 +434,90 @@ void mipi_dbi_pipe_disable(struct drm_simple_display_pipe *pipe)
 }
 EXPORT_SYMBOL(mipi_dbi_pipe_disable);
 
+/**
+ * mipi_dbi_pipe_begin_fb_access - MIPI DBI pipe begin-access helper
+ * @pipe: Display pipe
+ * @plane_state: Plane state
+ *
+ * This function implements struct &drm_simple_display_funcs.begin_fb_access.
+ *
+ * See drm_gem_begin_shadow_fb_access() for details and mipi_dbi_pipe_cleanup_fb()
+ * for cleanup.
+ *
+ * Returns:
+ * 0 on success, or a negative errno code otherwise.
+ */
+int mipi_dbi_pipe_begin_fb_access(struct drm_simple_display_pipe *pipe,
+				  struct drm_plane_state *plane_state)
+{
+	return drm_gem_begin_shadow_fb_access(&pipe->plane, plane_state);
+}
+EXPORT_SYMBOL(mipi_dbi_pipe_begin_fb_access);
+
+/**
+ * mipi_dbi_pipe_end_fb_access - MIPI DBI pipe end-access helper
+ * @pipe: Display pipe
+ * @plane_state: Plane state
+ *
+ * This function implements struct &drm_simple_display_funcs.end_fb_access.
+ *
+ * See mipi_dbi_pipe_begin_fb_access().
+ */
+void mipi_dbi_pipe_end_fb_access(struct drm_simple_display_pipe *pipe,
+				 struct drm_plane_state *plane_state)
+{
+	drm_gem_end_shadow_fb_access(&pipe->plane, plane_state);
+}
+EXPORT_SYMBOL(mipi_dbi_pipe_end_fb_access);
+
+/**
+ * mipi_dbi_pipe_reset_plane - MIPI DBI plane-reset helper
+ * @pipe: Display pipe
+ *
+ * This function implements struct &drm_simple_display_funcs.reset_plane
+ * for MIPI DBI planes.
+ */
+void mipi_dbi_pipe_reset_plane(struct drm_simple_display_pipe *pipe)
+{
+	drm_gem_reset_shadow_plane(&pipe->plane);
+}
+EXPORT_SYMBOL(mipi_dbi_pipe_reset_plane);
+
+/**
+ * mipi_dbi_pipe_duplicate_plane_state - duplicates MIPI DBI plane state
+ * @pipe: Display pipe
+ *
+ * This function implements struct &drm_simple_display_funcs.duplicate_plane_state
+ * for MIPI DBI planes.
+ *
+ * See drm_gem_duplicate_shadow_plane_state() for additional details.
+ *
+ * Returns:
+ * A pointer to a new plane state on success, or NULL otherwise.
+ */
+struct drm_plane_state *mipi_dbi_pipe_duplicate_plane_state(struct drm_simple_display_pipe *pipe)
+{
+	return drm_gem_duplicate_shadow_plane_state(&pipe->plane);
+}
+EXPORT_SYMBOL(mipi_dbi_pipe_duplicate_plane_state);
+
+/**
+ * mipi_dbi_pipe_destroy_plane_state - cleans up MIPI DBI plane state
+ * @pipe: Display pipe
+ * @plane_state: Plane state
+ *
+ * This function implements struct drm_simple_display_funcs.destroy_plane_state
+ * for MIPI DBI planes.
+ *
+ * See drm_gem_destroy_shadow_plane_state() for additional details.
+ */
+void mipi_dbi_pipe_destroy_plane_state(struct drm_simple_display_pipe *pipe,
+				       struct drm_plane_state *plane_state)
+{
+	drm_gem_destroy_shadow_plane_state(&pipe->plane, plane_state);
+}
+EXPORT_SYMBOL(mipi_dbi_pipe_destroy_plane_state);
+
 static int mipi_dbi_connector_get_modes(struct drm_connector *connector)
 {
 	struct mipi_dbi_dev *dbidev = drm_to_mipi_dbi_dev(connector->dev);
diff --git a/drivers/gpu/drm/tiny/ili9225.c b/drivers/gpu/drm/tiny/ili9225.c
index ae94c74d0163..a69aec8402bc 100644
--- a/drivers/gpu/drm/tiny/ili9225.c
+++ b/drivers/gpu/drm/tiny/ili9225.c
@@ -343,6 +343,11 @@ static const struct drm_simple_display_pipe_funcs ili9225_pipe_funcs = {
 	.enable		= ili9225_pipe_enable,
 	.disable	= ili9225_pipe_disable,
 	.update		= ili9225_pipe_update,
+	.begin_fb_access = mipi_dbi_pipe_begin_fb_access,
+	.end_fb_access	= mipi_dbi_pipe_end_fb_access,
+	.reset_plane	= mipi_dbi_pipe_reset_plane,
+	.duplicate_plane_state = mipi_dbi_pipe_duplicate_plane_state,
+	.destroy_plane_state = mipi_dbi_pipe_destroy_plane_state,
 };
 
 static const struct drm_display_mode ili9225_mode = {
diff --git a/drivers/gpu/drm/tiny/st7586.c b/drivers/gpu/drm/tiny/st7586.c
index e773b1f2fd5f..76b13cefc904 100644
--- a/drivers/gpu/drm/tiny/st7586.c
+++ b/drivers/gpu/drm/tiny/st7586.c
@@ -277,6 +277,11 @@ static const struct drm_simple_display_pipe_funcs st7586_pipe_funcs = {
 	.enable		= st7586_pipe_enable,
 	.disable	= st7586_pipe_disable,
 	.update		= st7586_pipe_update,
+	.begin_fb_access = mipi_dbi_pipe_begin_fb_access,
+	.end_fb_access	= mipi_dbi_pipe_end_fb_access,
+	.reset_plane	= mipi_dbi_pipe_reset_plane,
+	.duplicate_plane_state = mipi_dbi_pipe_duplicate_plane_state,
+	.destroy_plane_state = mipi_dbi_pipe_destroy_plane_state,
 };
 
 static const struct drm_display_mode st7586_mode = {
diff --git a/include/drm/drm_mipi_dbi.h b/include/drm/drm_mipi_dbi.h
index 36ac8495566b..304fbecdc66a 100644
--- a/include/drm/drm_mipi_dbi.h
+++ b/include/drm/drm_mipi_dbi.h
@@ -164,6 +164,15 @@ void mipi_dbi_enable_flush(struct mipi_dbi_dev *dbidev,
 			   struct drm_crtc_state *crtc_state,
 			   struct drm_plane_state *plan_state);
 void mipi_dbi_pipe_disable(struct drm_simple_display_pipe *pipe);
+int mipi_dbi_pipe_begin_fb_access(struct drm_simple_display_pipe *pipe,
+				  struct drm_plane_state *plane_state);
+void mipi_dbi_pipe_end_fb_access(struct drm_simple_display_pipe *pipe,
+				 struct drm_plane_state *plane_state);
+void mipi_dbi_pipe_reset_plane(struct drm_simple_display_pipe *pipe);
+struct drm_plane_state *mipi_dbi_pipe_duplicate_plane_state(struct drm_simple_display_pipe *pipe);
+void mipi_dbi_pipe_destroy_plane_state(struct drm_simple_display_pipe *pipe,
+				       struct drm_plane_state *plane_state);
+
 void mipi_dbi_hw_reset(struct mipi_dbi *dbi);
 bool mipi_dbi_display_is_on(struct mipi_dbi *dbi);
 int mipi_dbi_poweron_reset(struct mipi_dbi_dev *dbidev);
@@ -223,6 +232,11 @@ static inline void mipi_dbi_debugfs_init(struct drm_minor *minor) {}
 	.mode_valid = mipi_dbi_pipe_mode_valid, \
 	.enable = (enable_), \
 	.disable = mipi_dbi_pipe_disable, \
-	.update = mipi_dbi_pipe_update
+	.update = mipi_dbi_pipe_update, \
+	.begin_fb_access = mipi_dbi_pipe_begin_fb_access, \
+	.end_fb_access = mipi_dbi_pipe_end_fb_access, \
+	.reset_plane = mipi_dbi_pipe_reset_plane, \
+	.duplicate_plane_state = mipi_dbi_pipe_duplicate_plane_state, \
+	.destroy_plane_state = mipi_dbi_pipe_destroy_plane_state
 
 #endif /* __LINUX_MIPI_DBI_H */
-- 
2.38.1

[PATCH v2 7/8] drm/mipi-dbi: Use shadow-plane mappings

[Thomas Zimmermann]

Use the buffer mappings provided by shadow-plane helpers. As the
mappings are established while the commit can still fail, errors
are now reported correctly to callers.

v2:
	* use shadow-plane state directly (Noralf)

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Noralf Trønnes <noralf@tronnes.org>
Tested-by: Javier Martinez Canillas <javierm@redhat.com>
Tested-by: Noralf Trønnes <noralf@tronnes.org> # drm/tiny/mi0283qt
---
 drivers/gpu/drm/drm_mipi_dbi.c | 25 +++++--------------------
 drivers/gpu/drm/tiny/ili9225.c | 16 ++++------------
 drivers/gpu/drm/tiny/st7586.c  | 16 ++++------------
 3 files changed, 13 insertions(+), 44 deletions(-)

diff --git a/drivers/gpu/drm/drm_mipi_dbi.c b/drivers/gpu/drm/drm_mipi_dbi.c
index b808de61c5bc..0ef5e81ba5e1 100644
--- a/drivers/gpu/drm/drm_mipi_dbi.c
+++ b/drivers/gpu/drm/drm_mipi_dbi.c
@@ -321,12 +321,10 @@ EXPORT_SYMBOL(mipi_dbi_pipe_mode_valid);
 void mipi_dbi_pipe_update(struct drm_simple_display_pipe *pipe,
 			  struct drm_plane_state *old_state)
 {
-	struct iosys_map map[DRM_FORMAT_MAX_PLANES] = { };
-	struct iosys_map data[DRM_FORMAT_MAX_PLANES] = { };
 	struct drm_plane_state *state = pipe->plane.state;
+	struct drm_shadow_plane_state *shadow_plane_state = to_drm_shadow_plane_state(state);
 	struct drm_framebuffer *fb = state->fb;
 	struct drm_rect rect;
-	int ret;
 
 	if (!pipe->crtc.state->active)
 		return;
@@ -334,14 +332,8 @@ void mipi_dbi_pipe_update(struct drm_simple_display_pipe *pipe,
 	if (WARN_ON(!fb))
 		return;
 
-	ret = drm_gem_fb_vmap(fb, map, data);
-	if (ret)
-		return;
-
 	if (drm_atomic_helper_damage_merged(old_state, state, &rect))
-		mipi_dbi_fb_dirty(&data[0], fb, &rect);
-
-	drm_gem_fb_vunmap(fb, map);
+		mipi_dbi_fb_dirty(&shadow_plane_state->data[0], fb, &rect);
 }
 EXPORT_SYMBOL(mipi_dbi_pipe_update);
 
@@ -362,6 +354,7 @@ void mipi_dbi_enable_flush(struct mipi_dbi_dev *dbidev,
 			   struct drm_crtc_state *crtc_state,
 			   struct drm_plane_state *plane_state)
 {
+	struct drm_shadow_plane_state *shadow_plane_state = to_drm_shadow_plane_state(plane_state);
 	struct drm_framebuffer *fb = plane_state->fb;
 	struct drm_rect rect = {
 		.x1 = 0,
@@ -369,22 +362,14 @@ void mipi_dbi_enable_flush(struct mipi_dbi_dev *dbidev,
 		.y1 = 0,
 		.y2 = fb->height,
 	};
-	struct iosys_map map[DRM_FORMAT_MAX_PLANES] = { };
-	struct iosys_map data[DRM_FORMAT_MAX_PLANES] = { };
-	int idx, ret;
+	int idx;
 
 	if (!drm_dev_enter(&dbidev->drm, &idx))
 		return;
 
-	ret = drm_gem_fb_vmap(fb, map, data);
-	if (ret)
-		goto err_drm_dev_exit;
-
-	mipi_dbi_fb_dirty(&data[0], fb, &rect);
+	mipi_dbi_fb_dirty(&shadow_plane_state->data[0], fb, &rect);
 	backlight_enable(dbidev->backlight);
 
-	drm_gem_fb_vunmap(fb, map);
-err_drm_dev_exit:
 	drm_dev_exit(idx);
 }
 EXPORT_SYMBOL(mipi_dbi_enable_flush);
diff --git a/drivers/gpu/drm/tiny/ili9225.c b/drivers/gpu/drm/tiny/ili9225.c
index a69aec8402bc..0ba5177deca7 100644
--- a/drivers/gpu/drm/tiny/ili9225.c
+++ b/drivers/gpu/drm/tiny/ili9225.c
@@ -164,19 +164,15 @@ static void ili9225_pipe_update(struct drm_simple_display_pipe *pipe,
 				struct drm_plane_state *old_state)
 {
 	struct drm_plane_state *state = pipe->plane.state;
+	struct drm_shadow_plane_state *shadow_plane_state = to_drm_shadow_plane_state(state);
 	struct drm_framebuffer *fb = state->fb;
-	struct drm_gem_dma_object *dma_obj;
-	struct iosys_map src;
 	struct drm_rect rect;
 
 	if (!pipe->crtc.state->active)
 		return;
 
-	dma_obj = drm_fb_dma_get_gem_obj(fb, 0);
-	iosys_map_set_vaddr(&src, dma_obj->vaddr);
-
 	if (drm_atomic_helper_damage_merged(old_state, state, &rect))
-		ili9225_fb_dirty(&src, fb, &rect);
+		ili9225_fb_dirty(&shadow_plane_state->data[0], fb, &rect);
 }
 
 static void ili9225_pipe_enable(struct drm_simple_display_pipe *pipe,
@@ -184,6 +180,7 @@ static void ili9225_pipe_enable(struct drm_simple_display_pipe *pipe,
 				struct drm_plane_state *plane_state)
 {
 	struct mipi_dbi_dev *dbidev = drm_to_mipi_dbi_dev(pipe->crtc.dev);
+	struct drm_shadow_plane_state *shadow_plane_state = to_drm_shadow_plane_state(plane_state);
 	struct drm_framebuffer *fb = plane_state->fb;
 	struct device *dev = pipe->crtc.dev->dev;
 	struct mipi_dbi *dbi = &dbidev->dbi;
@@ -193,8 +190,6 @@ static void ili9225_pipe_enable(struct drm_simple_display_pipe *pipe,
 		.y1 = 0,
 		.y2 = fb->height,
 	};
-	struct drm_gem_dma_object *dma_obj;
-	struct iosys_map src;
 	int ret, idx;
 	u8 am_id;
 
@@ -285,10 +280,7 @@ static void ili9225_pipe_enable(struct drm_simple_display_pipe *pipe,
 
 	ili9225_command(dbi, ILI9225_DISPLAY_CONTROL_1, 0x1017);
 
-	dma_obj = drm_fb_dma_get_gem_obj(fb, 0);
-	iosys_map_set_vaddr(&src, dma_obj->vaddr);
-
-	ili9225_fb_dirty(&src, fb, &rect);
+	ili9225_fb_dirty(&shadow_plane_state->data[0], fb, &rect);
 
 out_exit:
 	drm_dev_exit(idx);
diff --git a/drivers/gpu/drm/tiny/st7586.c b/drivers/gpu/drm/tiny/st7586.c
index 76b13cefc904..53dca9272d4d 100644
--- a/drivers/gpu/drm/tiny/st7586.c
+++ b/drivers/gpu/drm/tiny/st7586.c
@@ -153,19 +153,15 @@ static void st7586_pipe_update(struct drm_simple_display_pipe *pipe,
 			       struct drm_plane_state *old_state)
 {
 	struct drm_plane_state *state = pipe->plane.state;
+	struct drm_shadow_plane_state *shadow_plane_state = to_drm_shadow_plane_state(state);
 	struct drm_framebuffer *fb = state->fb;
-	struct drm_gem_dma_object *dma_obj;
-	struct iosys_map src;
 	struct drm_rect rect;
 
 	if (!pipe->crtc.state->active)
 		return;
 
-	dma_obj = drm_fb_dma_get_gem_obj(fb, 0);
-	iosys_map_set_vaddr(&src, dma_obj->vaddr);
-
 	if (drm_atomic_helper_damage_merged(old_state, state, &rect))
-		st7586_fb_dirty(&src, fb, &rect);
+		st7586_fb_dirty(&shadow_plane_state->data[0], fb, &rect);
 }
 
 static void st7586_pipe_enable(struct drm_simple_display_pipe *pipe,
@@ -173,6 +169,7 @@ static void st7586_pipe_enable(struct drm_simple_display_pipe *pipe,
 			       struct drm_plane_state *plane_state)
 {
 	struct mipi_dbi_dev *dbidev = drm_to_mipi_dbi_dev(pipe->crtc.dev);
+	struct drm_shadow_plane_state *shadow_plane_state = to_drm_shadow_plane_state(plane_state);
 	struct drm_framebuffer *fb = plane_state->fb;
 	struct mipi_dbi *dbi = &dbidev->dbi;
 	struct drm_rect rect = {
@@ -181,8 +178,6 @@ static void st7586_pipe_enable(struct drm_simple_display_pipe *pipe,
 		.y1 = 0,
 		.y2 = fb->height,
 	};
-	struct drm_gem_dma_object *dma_obj;
-	struct iosys_map src;
 	int idx, ret;
 	u8 addr_mode;
 
@@ -242,10 +237,7 @@ static void st7586_pipe_enable(struct drm_simple_display_pipe *pipe,
 
 	msleep(100);
 
-	dma_obj = drm_fb_dma_get_gem_obj(fb, 0);
-	iosys_map_set_vaddr(&src, dma_obj->vaddr);
-
-	st7586_fb_dirty(&src, fb, &rect);
+	st7586_fb_dirty(&shadow_plane_state->data[0], fb, &rect);
 
 	mipi_dbi_command(dbi, MIPI_DCS_SET_DISPLAY_ON);
 out_exit:
-- 
2.38.1

[PATCH v2 8/8] drm/mipi-dbi: Move drm_dev_{enter, exit}() out from fb_dirty functions

[Thomas Zimmermann]

Call drm_dev_enter() and drm_dev_exit() in the outer-most callbacks
of the modesetting pipeline. If drm_dev_enter() fails, the driver can
thus avoid unnecessary work.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Noralf Trønnes <noralf@tronnes.org>
Tested-by: Javier Martinez Canillas <javierm@redhat.com>
Tested-by: Noralf Trønnes <noralf@tronnes.org> # drm/tiny/mi0283qt
---
 drivers/gpu/drm/drm_mipi_dbi.c | 13 +++++++------
 drivers/gpu/drm/tiny/ili9225.c | 13 +++++++------
 drivers/gpu/drm/tiny/st7586.c  | 13 +++++++------
 3 files changed, 21 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/drm_mipi_dbi.c b/drivers/gpu/drm/drm_mipi_dbi.c
index 0ef5e81ba5e1..eb27d000bcfb 100644
--- a/drivers/gpu/drm/drm_mipi_dbi.c
+++ b/drivers/gpu/drm/drm_mipi_dbi.c
@@ -259,13 +259,10 @@ static void mipi_dbi_fb_dirty(struct iosys_map *src, struct drm_framebuffer *fb,
 	unsigned int width = rect->x2 - rect->x1;
 	struct mipi_dbi *dbi = &dbidev->dbi;
 	bool swap = dbi->swap_bytes;
-	int idx, ret = 0;
+	int ret = 0;
 	bool full;
 	void *tr;
 
-	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));
@@ -288,8 +285,6 @@ static void mipi_dbi_fb_dirty(struct iosys_map *src, struct drm_framebuffer *fb,
 err_msg:
 	if (ret)
 		drm_err_once(fb->dev, "Failed to update display %d\n", ret);
-
-	drm_dev_exit(idx);
 }
 
 /**
@@ -325,6 +320,7 @@ void mipi_dbi_pipe_update(struct drm_simple_display_pipe *pipe,
 	struct drm_shadow_plane_state *shadow_plane_state = to_drm_shadow_plane_state(state);
 	struct drm_framebuffer *fb = state->fb;
 	struct drm_rect rect;
+	int idx;
 
 	if (!pipe->crtc.state->active)
 		return;
@@ -332,8 +328,13 @@ void mipi_dbi_pipe_update(struct drm_simple_display_pipe *pipe,
 	if (WARN_ON(!fb))
 		return;
 
+	if (!drm_dev_enter(fb->dev, &idx))
+		return;
+
 	if (drm_atomic_helper_damage_merged(old_state, state, &rect))
 		mipi_dbi_fb_dirty(&shadow_plane_state->data[0], fb, &rect);
+
+	drm_dev_exit(idx);
 }
 EXPORT_SYMBOL(mipi_dbi_pipe_update);
 
diff --git a/drivers/gpu/drm/tiny/ili9225.c b/drivers/gpu/drm/tiny/ili9225.c
index 0ba5177deca7..077c6ff5a2e1 100644
--- a/drivers/gpu/drm/tiny/ili9225.c
+++ b/drivers/gpu/drm/tiny/ili9225.c
@@ -87,13 +87,10 @@ static void ili9225_fb_dirty(struct iosys_map *src, struct drm_framebuffer *fb,
 	bool swap = dbi->swap_bytes;
 	u16 x_start, y_start;
 	u16 x1, x2, y1, y2;
-	int idx, ret = 0;
+	int ret = 0;
 	bool full;
 	void *tr;
 
-	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));
@@ -156,8 +153,6 @@ static void ili9225_fb_dirty(struct iosys_map *src, struct drm_framebuffer *fb,
 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,
@@ -167,12 +162,18 @@ static void ili9225_pipe_update(struct drm_simple_display_pipe *pipe,
 	struct drm_shadow_plane_state *shadow_plane_state = to_drm_shadow_plane_state(state);
 	struct drm_framebuffer *fb = state->fb;
 	struct drm_rect rect;
+	int idx;
 
 	if (!pipe->crtc.state->active)
 		return;
 
+	if (!drm_dev_enter(fb->dev, &idx))
+		return;
+
 	if (drm_atomic_helper_damage_merged(old_state, state, &rect))
 		ili9225_fb_dirty(&shadow_plane_state->data[0], fb, &rect);
+
+	drm_dev_exit(idx);
 }
 
 static void ili9225_pipe_enable(struct drm_simple_display_pipe *pipe,
diff --git a/drivers/gpu/drm/tiny/st7586.c b/drivers/gpu/drm/tiny/st7586.c
index 53dca9272d4d..3cf4eec16a81 100644
--- a/drivers/gpu/drm/tiny/st7586.c
+++ b/drivers/gpu/drm/tiny/st7586.c
@@ -113,10 +113,7 @@ static void st7586_fb_dirty(struct iosys_map *src, struct drm_framebuffer *fb,
 {
 	struct mipi_dbi_dev *dbidev = drm_to_mipi_dbi_dev(fb->dev);
 	struct mipi_dbi *dbi = &dbidev->dbi;
-	int start, end, idx, ret = 0;
-
-	if (!drm_dev_enter(fb->dev, &idx))
-		return;
+	int start, end, ret = 0;
 
 	/* 3 pixels per byte, so grow clip to nearest multiple of 3 */
 	rect->x1 = rounddown(rect->x1, 3);
@@ -145,8 +142,6 @@ static void st7586_fb_dirty(struct iosys_map *src, struct drm_framebuffer *fb,
 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,
@@ -156,12 +151,18 @@ static void st7586_pipe_update(struct drm_simple_display_pipe *pipe,
 	struct drm_shadow_plane_state *shadow_plane_state = to_drm_shadow_plane_state(state);
 	struct drm_framebuffer *fb = state->fb;
 	struct drm_rect rect;
+	int idx;
 
 	if (!pipe->crtc.state->active)
 		return;
 
+	if (!drm_dev_enter(fb->dev, &idx))
+		return;
+
 	if (drm_atomic_helper_damage_merged(old_state, state, &rect))
 		st7586_fb_dirty(&shadow_plane_state->data[0], fb, &rect);
+
+	drm_dev_exit(idx);
 }
 
 static void st7586_pipe_enable(struct drm_simple_display_pipe *pipe,
-- 
2.38.1

Re: [PATCH v2 6/8] drm/mipi-dbi: Support shadow-plane state

[Noralf Trønnes]

Den 02.12.2022 13.56, skrev Thomas Zimmermann:
> Implement MIPI DBI planes with struct drm_shadow_plane_state, so that the
> respective drivers can use the vmap'ed GEM-buffer memory. Implement state
> helpers, the {begin,end}_fb_access helpers and wire up everything.
> 
> With this commit, MIPI DBI drivers can access the GEM object's memory
> that is provided by shadow-plane state. The actual changes to drivers
> are implemented separately.
> 
> v2:
> 	* use shadow-plane state directly (Noralf)
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Tested-by: Javier Martinez Canillas <javierm@redhat.com>
> Tested-by: Noralf Trønnes <noralf@tronnes.org> # drm/tiny/mi0283qt
> ---
>  drivers/gpu/drm/drm_mipi_dbi.c | 85 ++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/tiny/ili9225.c |  5 ++
>  drivers/gpu/drm/tiny/st7586.c  |  5 ++
>  include/drm/drm_mipi_dbi.h     | 16 ++++++-
>  4 files changed, 110 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_mipi_dbi.c b/drivers/gpu/drm/drm_mipi_dbi.c
> index f58123327ed6..b808de61c5bc 100644
> --- a/drivers/gpu/drm/drm_mipi_dbi.c
> +++ b/drivers/gpu/drm/drm_mipi_dbi.c
> @@ -21,6 +21,7 @@
>  #include <drm/drm_fourcc.h>
>  #include <drm/drm_framebuffer.h>
>  #include <drm/drm_gem.h>
> +#include <drm/drm_gem_atomic_helper.h>
>  #include <drm/drm_gem_framebuffer_helper.h>
>  #include <drm/drm_mipi_dbi.h>
>  #include <drm/drm_modes.h>
> @@ -433,6 +434,90 @@ void mipi_dbi_pipe_disable(struct drm_simple_display_pipe *pipe)
>  }
>  EXPORT_SYMBOL(mipi_dbi_pipe_disable);
>  
> +/**
> + * mipi_dbi_pipe_begin_fb_access - MIPI DBI pipe begin-access helper
> + * @pipe: Display pipe
> + * @plane_state: Plane state
> + *
> + * This function implements struct &drm_simple_display_funcs.begin_fb_access.
> + *
> + * See drm_gem_begin_shadow_fb_access() for details and mipi_dbi_pipe_cleanup_fb()
> + * for cleanup.
> + *
> + * Returns:
> + * 0 on success, or a negative errno code otherwise.
> + */
> +int mipi_dbi_pipe_begin_fb_access(struct drm_simple_display_pipe *pipe,
> +				  struct drm_plane_state *plane_state)
> +{
> +	return drm_gem_begin_shadow_fb_access(&pipe->plane, plane_state);
> +}
> +EXPORT_SYMBOL(mipi_dbi_pipe_begin_fb_access);
> +
> +/**
> + * mipi_dbi_pipe_end_fb_access - MIPI DBI pipe end-access helper
> + * @pipe: Display pipe
> + * @plane_state: Plane state
> + *
> + * This function implements struct &drm_simple_display_funcs.end_fb_access.
> + *
> + * See mipi_dbi_pipe_begin_fb_access().
> + */
> +void mipi_dbi_pipe_end_fb_access(struct drm_simple_display_pipe *pipe,
> +				 struct drm_plane_state *plane_state)
> +{
> +	drm_gem_end_shadow_fb_access(&pipe->plane, plane_state);
> +}
> +EXPORT_SYMBOL(mipi_dbi_pipe_end_fb_access);
> +
> +/**
> + * mipi_dbi_pipe_reset_plane - MIPI DBI plane-reset helper
> + * @pipe: Display pipe
> + *
> + * This function implements struct &drm_simple_display_funcs.reset_plane
> + * for MIPI DBI planes.
> + */
> +void mipi_dbi_pipe_reset_plane(struct drm_simple_display_pipe *pipe)
> +{
> +	drm_gem_reset_shadow_plane(&pipe->plane);
> +}
> +EXPORT_SYMBOL(mipi_dbi_pipe_reset_plane);
> +
> +/**
> + * mipi_dbi_pipe_duplicate_plane_state - duplicates MIPI DBI plane state
> + * @pipe: Display pipe
> + *
> + * This function implements struct &drm_simple_display_funcs.duplicate_plane_state
> + * for MIPI DBI planes.
> + *
> + * See drm_gem_duplicate_shadow_plane_state() for additional details.
> + *
> + * Returns:
> + * A pointer to a new plane state on success, or NULL otherwise.
> + */
> +struct drm_plane_state *mipi_dbi_pipe_duplicate_plane_state(struct drm_simple_display_pipe *pipe)
> +{
> +	return drm_gem_duplicate_shadow_plane_state(&pipe->plane);
> +}
> +EXPORT_SYMBOL(mipi_dbi_pipe_duplicate_plane_state);
> +
> +/**
> + * mipi_dbi_pipe_destroy_plane_state - cleans up MIPI DBI plane state
> + * @pipe: Display pipe
> + * @plane_state: Plane state
> + *
> + * This function implements struct drm_simple_display_funcs.destroy_plane_state
> + * for MIPI DBI planes.
> + *
> + * See drm_gem_destroy_shadow_plane_state() for additional details.
> + */
> +void mipi_dbi_pipe_destroy_plane_state(struct drm_simple_display_pipe *pipe,
> +				       struct drm_plane_state *plane_state)
> +{
> +	drm_gem_destroy_shadow_plane_state(&pipe->plane, plane_state);
> +}
> +EXPORT_SYMBOL(mipi_dbi_pipe_destroy_plane_state);
> +
>  static int mipi_dbi_connector_get_modes(struct drm_connector *connector)
>  {
>  	struct mipi_dbi_dev *dbidev = drm_to_mipi_dbi_dev(connector->dev);
> diff --git a/drivers/gpu/drm/tiny/ili9225.c b/drivers/gpu/drm/tiny/ili9225.c
> index ae94c74d0163..a69aec8402bc 100644
> --- a/drivers/gpu/drm/tiny/ili9225.c
> +++ b/drivers/gpu/drm/tiny/ili9225.c
> @@ -343,6 +343,11 @@ static const struct drm_simple_display_pipe_funcs ili9225_pipe_funcs = {
>  	.enable		= ili9225_pipe_enable,
>  	.disable	= ili9225_pipe_disable,
>  	.update		= ili9225_pipe_update,
> +	.begin_fb_access = mipi_dbi_pipe_begin_fb_access,
> +	.end_fb_access	= mipi_dbi_pipe_end_fb_access,
> +	.reset_plane	= mipi_dbi_pipe_reset_plane,
> +	.duplicate_plane_state = mipi_dbi_pipe_duplicate_plane_state,
> +	.destroy_plane_state = mipi_dbi_pipe_destroy_plane_state,
>  };

Why not just use DRM_GEM_SIMPLE_DISPLAY_PIPE_SHADOW_PLANE_FUNCS?

Noralf.

>  
>  static const struct drm_display_mode ili9225_mode = {
> diff --git a/drivers/gpu/drm/tiny/st7586.c b/drivers/gpu/drm/tiny/st7586.c
> index e773b1f2fd5f..76b13cefc904 100644
> --- a/drivers/gpu/drm/tiny/st7586.c
> +++ b/drivers/gpu/drm/tiny/st7586.c
> @@ -277,6 +277,11 @@ static const struct drm_simple_display_pipe_funcs st7586_pipe_funcs = {
>  	.enable		= st7586_pipe_enable,
>  	.disable	= st7586_pipe_disable,
>  	.update		= st7586_pipe_update,
> +	.begin_fb_access = mipi_dbi_pipe_begin_fb_access,
> +	.end_fb_access	= mipi_dbi_pipe_end_fb_access,
> +	.reset_plane	= mipi_dbi_pipe_reset_plane,
> +	.duplicate_plane_state = mipi_dbi_pipe_duplicate_plane_state,
> +	.destroy_plane_state = mipi_dbi_pipe_destroy_plane_state,
>  };
>  
>  static const struct drm_display_mode st7586_mode = {
> diff --git a/include/drm/drm_mipi_dbi.h b/include/drm/drm_mipi_dbi.h
> index 36ac8495566b..304fbecdc66a 100644
> --- a/include/drm/drm_mipi_dbi.h
> +++ b/include/drm/drm_mipi_dbi.h
> @@ -164,6 +164,15 @@ void mipi_dbi_enable_flush(struct mipi_dbi_dev *dbidev,
>  			   struct drm_crtc_state *crtc_state,
>  			   struct drm_plane_state *plan_state);
>  void mipi_dbi_pipe_disable(struct drm_simple_display_pipe *pipe);
> +int mipi_dbi_pipe_begin_fb_access(struct drm_simple_display_pipe *pipe,
> +				  struct drm_plane_state *plane_state);
> +void mipi_dbi_pipe_end_fb_access(struct drm_simple_display_pipe *pipe,
> +				 struct drm_plane_state *plane_state);
> +void mipi_dbi_pipe_reset_plane(struct drm_simple_display_pipe *pipe);
> +struct drm_plane_state *mipi_dbi_pipe_duplicate_plane_state(struct drm_simple_display_pipe *pipe);
> +void mipi_dbi_pipe_destroy_plane_state(struct drm_simple_display_pipe *pipe,
> +				       struct drm_plane_state *plane_state);
> +
>  void mipi_dbi_hw_reset(struct mipi_dbi *dbi);
>  bool mipi_dbi_display_is_on(struct mipi_dbi *dbi);
>  int mipi_dbi_poweron_reset(struct mipi_dbi_dev *dbidev);
> @@ -223,6 +232,11 @@ static inline void mipi_dbi_debugfs_init(struct drm_minor *minor) {}
>  	.mode_valid = mipi_dbi_pipe_mode_valid, \
>  	.enable = (enable_), \
>  	.disable = mipi_dbi_pipe_disable, \
> -	.update = mipi_dbi_pipe_update
> +	.update = mipi_dbi_pipe_update, \
> +	.begin_fb_access = mipi_dbi_pipe_begin_fb_access, \
> +	.end_fb_access = mipi_dbi_pipe_end_fb_access, \
> +	.reset_plane = mipi_dbi_pipe_reset_plane, \
> +	.duplicate_plane_state = mipi_dbi_pipe_duplicate_plane_state, \
> +	.destroy_plane_state = mipi_dbi_pipe_destroy_plane_state
>  
>  #endif /* __LINUX_MIPI_DBI_H */

Re: [PATCH v2 6/8] drm/mipi-dbi: Support shadow-plane state

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 8802 bytes --]

Hi

Am 03.12.22 um 15:23 schrieb Noralf Trønnes:
> 
> 
> Den 02.12.2022 13.56, skrev Thomas Zimmermann:
>> Implement MIPI DBI planes with struct drm_shadow_plane_state, so that the
>> respective drivers can use the vmap'ed GEM-buffer memory. Implement state
>> helpers, the {begin,end}_fb_access helpers and wire up everything.
>>
>> With this commit, MIPI DBI drivers can access the GEM object's memory
>> that is provided by shadow-plane state. The actual changes to drivers
>> are implemented separately.
>>
>> v2:
>> 	* use shadow-plane state directly (Noralf)
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> Tested-by: Javier Martinez Canillas <javierm@redhat.com>
>> Tested-by: Noralf Trønnes <noralf@tronnes.org> # drm/tiny/mi0283qt
>> ---
>>   drivers/gpu/drm/drm_mipi_dbi.c | 85 ++++++++++++++++++++++++++++++++++
>>   drivers/gpu/drm/tiny/ili9225.c |  5 ++
>>   drivers/gpu/drm/tiny/st7586.c  |  5 ++
>>   include/drm/drm_mipi_dbi.h     | 16 ++++++-
>>   4 files changed, 110 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/drm_mipi_dbi.c b/drivers/gpu/drm/drm_mipi_dbi.c
>> index f58123327ed6..b808de61c5bc 100644
>> --- a/drivers/gpu/drm/drm_mipi_dbi.c
>> +++ b/drivers/gpu/drm/drm_mipi_dbi.c
>> @@ -21,6 +21,7 @@
>>   #include <drm/drm_fourcc.h>
>>   #include <drm/drm_framebuffer.h>
>>   #include <drm/drm_gem.h>
>> +#include <drm/drm_gem_atomic_helper.h>
>>   #include <drm/drm_gem_framebuffer_helper.h>
>>   #include <drm/drm_mipi_dbi.h>
>>   #include <drm/drm_modes.h>
>> @@ -433,6 +434,90 @@ void mipi_dbi_pipe_disable(struct drm_simple_display_pipe *pipe)
>>   }
>>   EXPORT_SYMBOL(mipi_dbi_pipe_disable);
>>   
>> +/**
>> + * mipi_dbi_pipe_begin_fb_access - MIPI DBI pipe begin-access helper
>> + * @pipe: Display pipe
>> + * @plane_state: Plane state
>> + *
>> + * This function implements struct &drm_simple_display_funcs.begin_fb_access.
>> + *
>> + * See drm_gem_begin_shadow_fb_access() for details and mipi_dbi_pipe_cleanup_fb()
>> + * for cleanup.
>> + *
>> + * Returns:
>> + * 0 on success, or a negative errno code otherwise.
>> + */
>> +int mipi_dbi_pipe_begin_fb_access(struct drm_simple_display_pipe *pipe,
>> +				  struct drm_plane_state *plane_state)
>> +{
>> +	return drm_gem_begin_shadow_fb_access(&pipe->plane, plane_state);
>> +}
>> +EXPORT_SYMBOL(mipi_dbi_pipe_begin_fb_access);
>> +
>> +/**
>> + * mipi_dbi_pipe_end_fb_access - MIPI DBI pipe end-access helper
>> + * @pipe: Display pipe
>> + * @plane_state: Plane state
>> + *
>> + * This function implements struct &drm_simple_display_funcs.end_fb_access.
>> + *
>> + * See mipi_dbi_pipe_begin_fb_access().
>> + */
>> +void mipi_dbi_pipe_end_fb_access(struct drm_simple_display_pipe *pipe,
>> +				 struct drm_plane_state *plane_state)
>> +{
>> +	drm_gem_end_shadow_fb_access(&pipe->plane, plane_state);
>> +}
>> +EXPORT_SYMBOL(mipi_dbi_pipe_end_fb_access);
>> +
>> +/**
>> + * mipi_dbi_pipe_reset_plane - MIPI DBI plane-reset helper
>> + * @pipe: Display pipe
>> + *
>> + * This function implements struct &drm_simple_display_funcs.reset_plane
>> + * for MIPI DBI planes.
>> + */
>> +void mipi_dbi_pipe_reset_plane(struct drm_simple_display_pipe *pipe)
>> +{
>> +	drm_gem_reset_shadow_plane(&pipe->plane);
>> +}
>> +EXPORT_SYMBOL(mipi_dbi_pipe_reset_plane);
>> +
>> +/**
>> + * mipi_dbi_pipe_duplicate_plane_state - duplicates MIPI DBI plane state
>> + * @pipe: Display pipe
>> + *
>> + * This function implements struct &drm_simple_display_funcs.duplicate_plane_state
>> + * for MIPI DBI planes.
>> + *
>> + * See drm_gem_duplicate_shadow_plane_state() for additional details.
>> + *
>> + * Returns:
>> + * A pointer to a new plane state on success, or NULL otherwise.
>> + */
>> +struct drm_plane_state *mipi_dbi_pipe_duplicate_plane_state(struct drm_simple_display_pipe *pipe)
>> +{
>> +	return drm_gem_duplicate_shadow_plane_state(&pipe->plane);
>> +}
>> +EXPORT_SYMBOL(mipi_dbi_pipe_duplicate_plane_state);
>> +
>> +/**
>> + * mipi_dbi_pipe_destroy_plane_state - cleans up MIPI DBI plane state
>> + * @pipe: Display pipe
>> + * @plane_state: Plane state
>> + *
>> + * This function implements struct drm_simple_display_funcs.destroy_plane_state
>> + * for MIPI DBI planes.
>> + *
>> + * See drm_gem_destroy_shadow_plane_state() for additional details.
>> + */
>> +void mipi_dbi_pipe_destroy_plane_state(struct drm_simple_display_pipe *pipe,
>> +				       struct drm_plane_state *plane_state)
>> +{
>> +	drm_gem_destroy_shadow_plane_state(&pipe->plane, plane_state);
>> +}
>> +EXPORT_SYMBOL(mipi_dbi_pipe_destroy_plane_state);
>> +
>>   static int mipi_dbi_connector_get_modes(struct drm_connector *connector)
>>   {
>>   	struct mipi_dbi_dev *dbidev = drm_to_mipi_dbi_dev(connector->dev);
>> diff --git a/drivers/gpu/drm/tiny/ili9225.c b/drivers/gpu/drm/tiny/ili9225.c
>> index ae94c74d0163..a69aec8402bc 100644
>> --- a/drivers/gpu/drm/tiny/ili9225.c
>> +++ b/drivers/gpu/drm/tiny/ili9225.c
>> @@ -343,6 +343,11 @@ static const struct drm_simple_display_pipe_funcs ili9225_pipe_funcs = {
>>   	.enable		= ili9225_pipe_enable,
>>   	.disable	= ili9225_pipe_disable,
>>   	.update		= ili9225_pipe_update,
>> +	.begin_fb_access = mipi_dbi_pipe_begin_fb_access,
>> +	.end_fb_access	= mipi_dbi_pipe_end_fb_access,
>> +	.reset_plane	= mipi_dbi_pipe_reset_plane,
>> +	.duplicate_plane_state = mipi_dbi_pipe_duplicate_plane_state,
>> +	.destroy_plane_state = mipi_dbi_pipe_destroy_plane_state,
>>   };
> 
> Why not just use DRM_GEM_SIMPLE_DISPLAY_PIPE_SHADOW_PLANE_FUNCS?

The simple-kms mid-layer is more of a liability than a feature. It 
requires us to have additional wrappers around common helpers, such the 
shadow-plane code. I'd like to get rid of those wrappers as much as 
possible. Not using them here is a first step.

Best regards
Thomas

> 
> Noralf.
> 
>>   
>>   static const struct drm_display_mode ili9225_mode = {
>> diff --git a/drivers/gpu/drm/tiny/st7586.c b/drivers/gpu/drm/tiny/st7586.c
>> index e773b1f2fd5f..76b13cefc904 100644
>> --- a/drivers/gpu/drm/tiny/st7586.c
>> +++ b/drivers/gpu/drm/tiny/st7586.c
>> @@ -277,6 +277,11 @@ static const struct drm_simple_display_pipe_funcs st7586_pipe_funcs = {
>>   	.enable		= st7586_pipe_enable,
>>   	.disable	= st7586_pipe_disable,
>>   	.update		= st7586_pipe_update,
>> +	.begin_fb_access = mipi_dbi_pipe_begin_fb_access,
>> +	.end_fb_access	= mipi_dbi_pipe_end_fb_access,
>> +	.reset_plane	= mipi_dbi_pipe_reset_plane,
>> +	.duplicate_plane_state = mipi_dbi_pipe_duplicate_plane_state,
>> +	.destroy_plane_state = mipi_dbi_pipe_destroy_plane_state,
>>   };
>>   
>>   static const struct drm_display_mode st7586_mode = {
>> diff --git a/include/drm/drm_mipi_dbi.h b/include/drm/drm_mipi_dbi.h
>> index 36ac8495566b..304fbecdc66a 100644
>> --- a/include/drm/drm_mipi_dbi.h
>> +++ b/include/drm/drm_mipi_dbi.h
>> @@ -164,6 +164,15 @@ void mipi_dbi_enable_flush(struct mipi_dbi_dev *dbidev,
>>   			   struct drm_crtc_state *crtc_state,
>>   			   struct drm_plane_state *plan_state);
>>   void mipi_dbi_pipe_disable(struct drm_simple_display_pipe *pipe);
>> +int mipi_dbi_pipe_begin_fb_access(struct drm_simple_display_pipe *pipe,
>> +				  struct drm_plane_state *plane_state);
>> +void mipi_dbi_pipe_end_fb_access(struct drm_simple_display_pipe *pipe,
>> +				 struct drm_plane_state *plane_state);
>> +void mipi_dbi_pipe_reset_plane(struct drm_simple_display_pipe *pipe);
>> +struct drm_plane_state *mipi_dbi_pipe_duplicate_plane_state(struct drm_simple_display_pipe *pipe);
>> +void mipi_dbi_pipe_destroy_plane_state(struct drm_simple_display_pipe *pipe,
>> +				       struct drm_plane_state *plane_state);
>> +
>>   void mipi_dbi_hw_reset(struct mipi_dbi *dbi);
>>   bool mipi_dbi_display_is_on(struct mipi_dbi *dbi);
>>   int mipi_dbi_poweron_reset(struct mipi_dbi_dev *dbidev);
>> @@ -223,6 +232,11 @@ static inline void mipi_dbi_debugfs_init(struct drm_minor *minor) {}
>>   	.mode_valid = mipi_dbi_pipe_mode_valid, \
>>   	.enable = (enable_), \
>>   	.disable = mipi_dbi_pipe_disable, \
>> -	.update = mipi_dbi_pipe_update
>> +	.update = mipi_dbi_pipe_update, \
>> +	.begin_fb_access = mipi_dbi_pipe_begin_fb_access, \
>> +	.end_fb_access = mipi_dbi_pipe_end_fb_access, \
>> +	.reset_plane = mipi_dbi_pipe_reset_plane, \
>> +	.duplicate_plane_state = mipi_dbi_pipe_duplicate_plane_state, \
>> +	.destroy_plane_state = mipi_dbi_pipe_destroy_plane_state
>>   
>>   #endif /* __LINUX_MIPI_DBI_H */

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH v2 6/8] drm/mipi-dbi: Support shadow-plane state

[Noralf Trønnes]

Den 02.12.2022 13.56, skrev Thomas Zimmermann:
> Implement MIPI DBI planes with struct drm_shadow_plane_state, so that the
> respective drivers can use the vmap'ed GEM-buffer memory. Implement state
> helpers, the {begin,end}_fb_access helpers and wire up everything.
> 
> With this commit, MIPI DBI drivers can access the GEM object's memory
> that is provided by shadow-plane state. The actual changes to drivers
> are implemented separately.
> 
> v2:
> 	* use shadow-plane state directly (Noralf)
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Tested-by: Javier Martinez Canillas <javierm@redhat.com>
> Tested-by: Noralf Trønnes <noralf@tronnes.org> # drm/tiny/mi0283qt
> ---

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

Re: [PATCH v2 6/8] drm/mipi-dbi: Support shadow-plane state

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 1049 bytes --]



Am 03.12.22 um 17:11 schrieb Noralf Trønnes:
> 
> 
> Den 02.12.2022 13.56, skrev Thomas Zimmermann:
>> Implement MIPI DBI planes with struct drm_shadow_plane_state, so that the
>> respective drivers can use the vmap'ed GEM-buffer memory. Implement state
>> helpers, the {begin,end}_fb_access helpers and wire up everything.
>>
>> With this commit, MIPI DBI drivers can access the GEM object's memory
>> that is provided by shadow-plane state. The actual changes to drivers
>> are implemented separately.
>>
>> v2:
>> 	* use shadow-plane state directly (Noralf)
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> Tested-by: Javier Martinez Canillas <javierm@redhat.com>
>> Tested-by: Noralf Trønnes <noralf@tronnes.org> # drm/tiny/mi0283qt
>> ---
> 
> Reviewed-by: Noralf Trønnes <noralf@tronnes.org>

Thanks a lot.


-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]