[v2,4/5] drm/tinydrm: Use damage helper for dirtyfb

Submitted by Noralf Trønnes on Jan. 11, 2019, 8:12 p.m.

Details

Message ID 20190111201206.45018-5-noralf@tronnes.org
State New
Series "drm/tinydrm: Use damage helper for dirtyfb"
Headers show

Commit Message

Noralf Trønnes Jan. 11, 2019, 8:12 p.m.
This switches to drm_atomic_helper_dirtyfb() as the framebuffer dirty
handler. All flushing will now happen in the pipe functions.

Also enable the damage plane property for all except repaper which can
only do full updates.

ili9225:
This change made ili9225_init() equal to mipi_dbi_init() so use it.

v2: Remove fb check in mipi_dbi_enable_flush() it can't be NULL
    (kbuild test robot)

Cc: David Lechner <david@lechnology.com>
Cc: Eric Anholt <eric@anholt.net>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
Acked-by: Sam Ravnborg <sam@ravnborg.org>
Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/tinydrm/core/tinydrm-core.c   |  21 +---
 .../gpu/drm/tinydrm/core/tinydrm-helpers.c    |  91 +---------------
 drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c   |  30 ------
 drivers/gpu/drm/tinydrm/hx8357d.c             |   2 +-
 drivers/gpu/drm/tinydrm/ili9225.c             | 101 ++++++------------
 drivers/gpu/drm/tinydrm/ili9341.c             |   2 +-
 drivers/gpu/drm/tinydrm/mi0283qt.c            |   2 +-
 drivers/gpu/drm/tinydrm/mipi-dbi.c            |  74 ++++++++-----
 drivers/gpu/drm/tinydrm/repaper.c             |  39 ++++---
 drivers/gpu/drm/tinydrm/st7586.c              |  50 +++++----
 drivers/gpu/drm/tinydrm/st7735r.c             |   2 +-
 include/drm/tinydrm/mipi-dbi.h                |   2 +
 include/drm/tinydrm/tinydrm-helpers.h         |  11 +-
 include/drm/tinydrm/tinydrm.h                 |  26 -----
 14 files changed, 138 insertions(+), 315 deletions(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-core.c b/drivers/gpu/drm/tinydrm/core/tinydrm-core.c
index aeb93eadb047..614f532ea89f 100644
--- a/drivers/gpu/drm/tinydrm/core/tinydrm-core.c
+++ b/drivers/gpu/drm/tinydrm/core/tinydrm-core.c
@@ -39,31 +39,17 @@ 
  * and registers the DRM device using devm_tinydrm_register().
  */
 
-static struct drm_framebuffer *
-tinydrm_fb_create(struct drm_device *drm, struct drm_file *file_priv,
-		  const struct drm_mode_fb_cmd2 *mode_cmd)
-{
-	struct tinydrm_device *tdev = drm->dev_private;
-
-	return drm_gem_fb_create_with_funcs(drm, file_priv, mode_cmd,
-					    tdev->fb_funcs);
-}
-
 static const struct drm_mode_config_funcs tinydrm_mode_config_funcs = {
-	.fb_create = tinydrm_fb_create,
+	.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,
-			const struct drm_framebuffer_funcs *fb_funcs,
 			struct drm_driver *driver)
 {
 	struct drm_device *drm;
 
-	mutex_init(&tdev->dirty_lock);
-	tdev->fb_funcs = fb_funcs;
-
 	/*
 	 * We don't embed drm_device, because that prevent us from using
 	 * devm_kzalloc() to allocate tinydrm_device in the driver since
@@ -86,7 +72,6 @@  static int tinydrm_init(struct device *parent, struct tinydrm_device *tdev,
 static void tinydrm_fini(struct tinydrm_device *tdev)
 {
 	drm_mode_config_cleanup(tdev->drm);
-	mutex_destroy(&tdev->dirty_lock);
 	tdev->drm->dev_private = NULL;
 	drm_dev_put(tdev->drm);
 }
@@ -100,7 +85,6 @@  static void devm_tinydrm_release(void *data)
  * devm_tinydrm_init - Initialize tinydrm device
  * @parent: Parent device object
  * @tdev: tinydrm device
- * @fb_funcs: Framebuffer functions
  * @driver: DRM driver
  *
  * This function initializes @tdev, the underlying DRM device and it's
@@ -111,12 +95,11 @@  static void devm_tinydrm_release(void *data)
  * Zero on success, negative error code on failure.
  */
 int devm_tinydrm_init(struct device *parent, struct tinydrm_device *tdev,
-		      const struct drm_framebuffer_funcs *fb_funcs,
 		      struct drm_driver *driver)
 {
 	int ret;
 
-	ret = tinydrm_init(parent, tdev, fb_funcs, driver);
+	ret = tinydrm_init(parent, tdev, driver);
 	if (ret)
 		return ret;
 
diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
index d0ece6ad4a1c..2737b6fdadc8 100644
--- a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
+++ b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
@@ -17,104 +17,15 @@ 
 #include <drm/drm_device.h>
 #include <drm/drm_drv.h>
 #include <drm/drm_fourcc.h>
+#include <drm/drm_framebuffer.h>
 #include <drm/drm_print.h>
 #include <drm/drm_rect.h>
-#include <drm/tinydrm/tinydrm.h>
 #include <drm/tinydrm/tinydrm-helpers.h>
-#include <uapi/drm/drm.h>
 
 static unsigned int spi_max;
 module_param(spi_max, uint, 0400);
 MODULE_PARM_DESC(spi_max, "Set a lower SPI max transfer size");
 
-/**
- * tinydrm_merge_clips - Merge clip rectangles
- * @dst: Destination clip rectangle
- * @src: Source clip rectangle(s)
- * @num_clips: Number of @src clip rectangles
- * @flags: Dirty fb ioctl flags
- * @max_width: Maximum width of @dst
- * @max_height: Maximum height of @dst
- *
- * This function merges @src clip rectangle(s) into @dst. If @src is NULL,
- * @max_width and @min_width is used to set a full @dst clip rectangle.
- *
- * Returns:
- * true if it's a full clip, false otherwise
- */
-bool tinydrm_merge_clips(struct drm_rect *dst,
-			 struct drm_clip_rect *src, unsigned int num_clips,
-			 unsigned int flags, u32 max_width, u32 max_height)
-{
-	unsigned int i;
-
-	if (!src || !num_clips) {
-		dst->x1 = 0;
-		dst->x2 = max_width;
-		dst->y1 = 0;
-		dst->y2 = max_height;
-		return true;
-	}
-
-	dst->x1 = ~0;
-	dst->y1 = ~0;
-	dst->x2 = 0;
-	dst->y2 = 0;
-
-	for (i = 0; i < num_clips; i++) {
-		if (flags & DRM_MODE_FB_DIRTY_ANNOTATE_COPY)
-			i++;
-		dst->x1 = min_t(int, dst->x1, src[i].x1);
-		dst->x2 = max_t(int, dst->x2, src[i].x2);
-		dst->y1 = min_t(int, dst->y1, src[i].y1);
-		dst->y2 = max_t(int, dst->y2, src[i].y2);
-	}
-
-	if (dst->x2 > max_width || dst->y2 > max_height ||
-	    dst->x1 >= dst->x2 || dst->y1 >= dst->y2) {
-		DRM_DEBUG_KMS("Illegal clip: x1=%u, x2=%u, y1=%u, y2=%u\n",
-			      dst->x1, dst->x2, dst->y1, dst->y2);
-		dst->x1 = 0;
-		dst->y1 = 0;
-		dst->x2 = max_width;
-		dst->y2 = max_height;
-	}
-
-	return (dst->x2 - dst->x1) == max_width &&
-	       (dst->y2 - dst->y1) == max_height;
-}
-EXPORT_SYMBOL(tinydrm_merge_clips);
-
-int tinydrm_fb_dirty(struct drm_framebuffer *fb,
-		     struct drm_file *file_priv,
-		     unsigned int flags, unsigned int color,
-		     struct drm_clip_rect *clips,
-		     unsigned int num_clips)
-{
-	struct tinydrm_device *tdev = fb->dev->dev_private;
-	struct drm_plane *plane = &tdev->pipe.plane;
-	int ret = 0;
-
-	drm_modeset_lock(&plane->mutex, NULL);
-
-	/* fbdev can flush even when we're not interested */
-	if (plane->state->fb == fb) {
-		mutex_lock(&tdev->dirty_lock);
-		ret = tdev->fb_dirty(fb, file_priv, flags,
-				     color, clips, num_clips);
-		mutex_unlock(&tdev->dirty_lock);
-	}
-
-	drm_modeset_unlock(&plane->mutex);
-
-	if (ret)
-		dev_err_once(fb->dev->dev,
-			     "Failed to update display %d\n", ret);
-
-	return ret;
-}
-EXPORT_SYMBOL(tinydrm_fb_dirty);
-
 /**
  * tinydrm_memcpy - Copy clip buffer
  * @dst: Destination buffer
diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c b/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c
index d4576d6e8ce4..a4796ddb08f0 100644
--- a/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c
+++ b/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c
@@ -111,36 +111,6 @@  tinydrm_connector_create(struct drm_device *drm,
 	return connector;
 }
 
-/**
- * tinydrm_display_pipe_update - Display pipe update helper
- * @pipe: Simple display pipe
- * @old_state: Old plane state
- *
- * This function does a full framebuffer flush if the plane framebuffer
- * has changed. It also handles vblank events. Drivers can use this as their
- * &drm_simple_display_pipe_funcs->update callback.
- */
-void tinydrm_display_pipe_update(struct drm_simple_display_pipe *pipe,
-				 struct drm_plane_state *old_state)
-{
-	struct tinydrm_device *tdev = pipe_to_tinydrm(pipe);
-	struct drm_framebuffer *fb = pipe->plane.state->fb;
-	struct drm_crtc *crtc = &tdev->pipe.crtc;
-
-	if (fb && (fb != old_state->fb)) {
-		if (tdev->fb_dirty)
-			tdev->fb_dirty(fb, NULL, 0, 0, NULL, 0);
-	}
-
-	if (crtc->state->event) {
-		spin_lock_irq(&crtc->dev->event_lock);
-		drm_crtc_send_vblank_event(crtc, crtc->state->event);
-		spin_unlock_irq(&crtc->dev->event_lock);
-		crtc->state->event = NULL;
-	}
-}
-EXPORT_SYMBOL(tinydrm_display_pipe_update);
-
 static int tinydrm_rotate_mode(struct drm_display_mode *mode,
 			       unsigned int rotation)
 {
diff --git a/drivers/gpu/drm/tinydrm/hx8357d.c b/drivers/gpu/drm/tinydrm/hx8357d.c
index 3ae11aa4b73b..8bbd0beafc6a 100644
--- a/drivers/gpu/drm/tinydrm/hx8357d.c
+++ b/drivers/gpu/drm/tinydrm/hx8357d.c
@@ -176,7 +176,7 @@  static void yx240qv29_enable(struct drm_simple_display_pipe *pipe,
 static const struct drm_simple_display_pipe_funcs hx8357d_pipe_funcs = {
 	.enable = yx240qv29_enable,
 	.disable = mipi_dbi_pipe_disable,
-	.update = tinydrm_display_pipe_update,
+	.update = mipi_dbi_pipe_update,
 	.prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb,
 };
 
diff --git a/drivers/gpu/drm/tinydrm/ili9225.c b/drivers/gpu/drm/tinydrm/ili9225.c
index ad644069089f..aa1376a22bda 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_damage_helper.h>
 #include <drm/drm_drv.h>
 #include <drm/drm_fb_cma_helper.h>
 #include <drm/drm_fourcc.h>
@@ -76,17 +77,14 @@  static inline int ili9225_command(struct mipi_dbi *mipi, u8 cmd, u16 data)
 	return mipi_dbi_command_buf(mipi, cmd, par, 2);
 }
 
-static int ili9225_fb_dirty(struct drm_framebuffer *fb,
-			    struct drm_file *file_priv, unsigned int flags,
-			    unsigned int color, struct drm_clip_rect *clips,
-			    unsigned int num_clips)
+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);
+	unsigned int height = rect->y2 - rect->y1;
+	unsigned int width = rect->x2 - rect->x1;
 	bool swap = mipi->swap_bytes;
-	struct drm_rect clip;
-	struct drm_rect *rect = &clip;
 	u16 x_start, y_start;
 	u16 x1, x2, y1, y2;
 	int ret = 0;
@@ -94,10 +92,9 @@  static int ili9225_fb_dirty(struct drm_framebuffer *fb,
 	void *tr;
 
 	if (!mipi->enabled)
-		return 0;
+		return;
 
-	full = tinydrm_merge_clips(&clip, clips, num_clips, flags,
-				   fb->width, fb->height);
+	full = width == fb->width && height == fb->height;
 
 	DRM_DEBUG_KMS("Flushing [FB:%d] " DRM_RECT_FMT "\n", fb->base.id, DRM_RECT_ARG(rect));
 
@@ -106,7 +103,7 @@  static int ili9225_fb_dirty(struct drm_framebuffer *fb,
 		tr = mipi->tx_buf;
 		ret = mipi_dbi_buf_copy(mipi->tx_buf, fb, rect, swap);
 		if (ret)
-			return ret;
+			goto err_msg;
 	} else {
 		tr = cma_obj->vaddr;
 	}
@@ -155,16 +152,29 @@  static int ili9225_fb_dirty(struct drm_framebuffer *fb,
 	ili9225_command(mipi, ILI9225_RAM_ADDRESS_SET_2, y_start);
 
 	ret = mipi_dbi_command_buf(mipi, ILI9225_WRITE_DATA_TO_GRAM, tr,
-				   (rect->x2 - rect->x1) * (rect->y2 - rect->y1) * 2);
-
-	return ret;
+				   width * height * 2);
+err_msg:
+	if (ret)
+		dev_err_once(fb->dev->dev, "Failed to update display %d\n", ret);
 }
 
-static const struct drm_framebuffer_funcs ili9225_fb_funcs = {
-	.destroy	= drm_gem_fb_destroy,
-	.create_handle	= drm_gem_fb_create_handle,
-	.dirty		= tinydrm_fb_dirty,
-};
+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_crtc *crtc = &pipe->crtc;
+	struct drm_rect rect;
+
+	if (drm_atomic_helper_damage_merged(old_state, state, &rect))
+		ili9225_fb_dirty(state->fb, &rect);
+
+	if (crtc->state->event) {
+		spin_lock_irq(&crtc->dev->event_lock);
+		drm_crtc_send_vblank_event(crtc, crtc->state->event);
+		spin_unlock_irq(&crtc->dev->event_lock);
+		crtc->state->event = NULL;
+	}
+}
 
 static void ili9225_pipe_enable(struct drm_simple_display_pipe *pipe,
 				struct drm_crtc_state *crtc_state,
@@ -305,59 +315,10 @@  static int ili9225_dbi_command(struct mipi_dbi *mipi, u8 cmd, u8 *par,
 	return tinydrm_spi_transfer(spi, speed_hz, NULL, bpw, par, num);
 }
 
-static const u32 ili9225_formats[] = {
-	DRM_FORMAT_RGB565,
-	DRM_FORMAT_XRGB8888,
-};
-
-static int ili9225_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 * mode->hdisplay * sizeof(u16);
-	struct tinydrm_device *tdev = &mipi->tinydrm;
-	int ret;
-
-	if (!mipi->command)
-		return -EINVAL;
-
-	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, &ili9225_fb_funcs, driver);
-	if (ret)
-		return ret;
-
-	tdev->fb_dirty = ili9225_fb_dirty;
-
-	ret = tinydrm_display_pipe_init(tdev, pipe_funcs,
-					DRM_MODE_CONNECTOR_VIRTUAL,
-					ili9225_formats,
-					ARRAY_SIZE(ili9225_formats), mode,
-					rotation);
-	if (ret)
-		return ret;
-
-	tdev->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);
-
-	return 0;
-}
-
 static const struct drm_simple_display_pipe_funcs ili9225_pipe_funcs = {
 	.enable		= ili9225_pipe_enable,
 	.disable	= ili9225_pipe_disable,
-	.update		= tinydrm_display_pipe_update,
+	.update		= ili9225_pipe_update,
 	.prepare_fb	= drm_gem_fb_simple_display_pipe_prepare_fb,
 };
 
@@ -424,8 +385,8 @@  static int ili9225_probe(struct spi_device *spi)
 	/* override the command function set in  mipi_dbi_spi_init() */
 	mipi->command = ili9225_dbi_command;
 
-	ret = ili9225_init(&spi->dev, mipi, &ili9225_pipe_funcs,
-			   &ili9225_driver, &ili9225_mode, rotation);
+	ret = mipi_dbi_init(&spi->dev, mipi, &ili9225_pipe_funcs,
+			    &ili9225_driver, &ili9225_mode, rotation);
 	if (ret)
 		return ret;
 
diff --git a/drivers/gpu/drm/tinydrm/ili9341.c b/drivers/gpu/drm/tinydrm/ili9341.c
index bcdf10906ade..713bb2dd7e04 100644
--- a/drivers/gpu/drm/tinydrm/ili9341.c
+++ b/drivers/gpu/drm/tinydrm/ili9341.c
@@ -132,7 +132,7 @@  static void yx240qv29_enable(struct drm_simple_display_pipe *pipe,
 static const struct drm_simple_display_pipe_funcs ili9341_pipe_funcs = {
 	.enable = yx240qv29_enable,
 	.disable = mipi_dbi_pipe_disable,
-	.update = tinydrm_display_pipe_update,
+	.update = mipi_dbi_pipe_update,
 	.prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb,
 };
 
diff --git a/drivers/gpu/drm/tinydrm/mi0283qt.c b/drivers/gpu/drm/tinydrm/mi0283qt.c
index 97805ca37a04..82a92ec9ae3c 100644
--- a/drivers/gpu/drm/tinydrm/mi0283qt.c
+++ b/drivers/gpu/drm/tinydrm/mi0283qt.c
@@ -140,7 +140,7 @@  static void mi0283qt_enable(struct drm_simple_display_pipe *pipe,
 static const struct drm_simple_display_pipe_funcs mi0283qt_pipe_funcs = {
 	.enable = mi0283qt_enable,
 	.disable = mipi_dbi_pipe_disable,
-	.update = tinydrm_display_pipe_update,
+	.update = mipi_dbi_pipe_update,
 	.prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb,
 };
 
diff --git a/drivers/gpu/drm/tinydrm/mipi-dbi.c b/drivers/gpu/drm/tinydrm/mipi-dbi.c
index 1e7e8cfc99f0..680692a83f94 100644
--- a/drivers/gpu/drm/tinydrm/mipi-dbi.c
+++ b/drivers/gpu/drm/tinydrm/mipi-dbi.c
@@ -17,6 +17,7 @@ 
 #include <linux/regulator/consumer.h>
 #include <linux/spi/spi.h>
 
+#include <drm/drm_damage_helper.h>
 #include <drm/drm_drv.h>
 #include <drm/drm_fb_cma_helper.h>
 #include <drm/drm_gem_cma_helper.h>
@@ -25,7 +26,6 @@ 
 #include <drm/drm_rect.h>
 #include <drm/tinydrm/mipi-dbi.h>
 #include <drm/tinydrm/tinydrm-helpers.h>
-#include <uapi/drm/drm.h>
 #include <video/mipi_display.h>
 
 #define MIPI_DBI_MAX_SPI_READ_SPEED 2000000 /* 2MHz */
@@ -212,27 +212,22 @@  int mipi_dbi_buf_copy(void *dst, struct drm_framebuffer *fb,
 }
 EXPORT_SYMBOL(mipi_dbi_buf_copy);
 
-static int mipi_dbi_fb_dirty(struct drm_framebuffer *fb,
-			     struct drm_file *file_priv,
-			     unsigned int flags, unsigned int color,
-			     struct drm_clip_rect *clips,
-			     unsigned int num_clips)
+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);
+	unsigned int height = rect->y2 - rect->y1;
+	unsigned int width = rect->x2 - rect->x1;
 	bool swap = mipi->swap_bytes;
-	struct drm_rect clip;
-	struct drm_rect *rect = &clip;
 	int ret = 0;
 	bool full;
 	void *tr;
 
 	if (!mipi->enabled)
-		return 0;
+		return;
 
-	full = tinydrm_merge_clips(&clip, clips, num_clips, flags,
-				   fb->width, fb->height);
+	full = width == fb->width && height == fb->height;
 
 	DRM_DEBUG_KMS("Flushing [FB:%d] " DRM_RECT_FMT "\n", fb->base.id, DRM_RECT_ARG(rect));
 
@@ -241,7 +236,7 @@  static int mipi_dbi_fb_dirty(struct drm_framebuffer *fb,
 		tr = mipi->tx_buf;
 		ret = mipi_dbi_buf_copy(mipi->tx_buf, fb, rect, swap);
 		if (ret)
-			return ret;
+			goto err_msg;
 	} else {
 		tr = cma_obj->vaddr;
 	}
@@ -254,16 +249,38 @@  static int mipi_dbi_fb_dirty(struct drm_framebuffer *fb,
 			 ((rect->y2 - 1) >> 8) & 0xff, (rect->y2 - 1) & 0xff);
 
 	ret = mipi_dbi_command_buf(mipi, MIPI_DCS_WRITE_MEMORY_START, tr,
-				   (rect->x2 - rect->x1) * (rect->y2 - rect->y1) * 2);
-
-	return ret;
+				   width * height * 2);
+err_msg:
+	if (ret)
+		dev_err_once(fb->dev->dev, "Failed to update display %d\n", ret);
 }
 
-static const struct drm_framebuffer_funcs mipi_dbi_fb_funcs = {
-	.destroy	= drm_gem_fb_destroy,
-	.create_handle	= drm_gem_fb_create_handle,
-	.dirty		= tinydrm_fb_dirty,
-};
+/**
+ * mipi_dbi_pipe_update - Display pipe update helper
+ * @pipe: Simple display pipe
+ * @old_state: Old plane state
+ *
+ * This function handles framebuffer flushing and vblank events. Drivers can use
+ * this as their &drm_simple_display_pipe_funcs->update callback.
+ */
+void mipi_dbi_pipe_update(struct drm_simple_display_pipe *pipe,
+			  struct drm_plane_state *old_state)
+{
+	struct drm_plane_state *state = pipe->plane.state;
+	struct drm_crtc *crtc = &pipe->crtc;
+	struct drm_rect rect;
+
+	if (drm_atomic_helper_damage_merged(old_state, state, &rect))
+		mipi_dbi_fb_dirty(state->fb, &rect);
+
+	if (crtc->state->event) {
+		spin_lock_irq(&crtc->dev->event_lock);
+		drm_crtc_send_vblank_event(crtc, crtc->state->event);
+		spin_unlock_irq(&crtc->dev->event_lock);
+		crtc->state->event = NULL;
+	}
+}
+EXPORT_SYMBOL(mipi_dbi_pipe_update);
 
 /**
  * mipi_dbi_enable_flush - MIPI DBI enable helper
@@ -279,13 +296,16 @@  void mipi_dbi_enable_flush(struct mipi_dbi *mipi,
 			   struct drm_crtc_state *crtc_state,
 			   struct drm_plane_state *plane_state)
 {
-	struct tinydrm_device *tdev = &mipi->tinydrm;
 	struct drm_framebuffer *fb = plane_state->fb;
+	struct drm_rect rect = {
+		.x1 = 0,
+		.x2 = fb->width,
+		.y1 = 0,
+		.y2 = fb->height,
+	};
 
 	mipi->enabled = true;
-	if (fb)
-		tdev->fb_dirty(fb, NULL, 0, 0, NULL, 0);
-
+	mipi_dbi_fb_dirty(fb, &rect);
 	backlight_enable(mipi->backlight);
 }
 EXPORT_SYMBOL(mipi_dbi_enable_flush);
@@ -377,12 +397,10 @@  int mipi_dbi_init(struct device *dev, struct mipi_dbi *mipi,
 	if (!mipi->tx_buf)
 		return -ENOMEM;
 
-	ret = devm_tinydrm_init(dev, tdev, &mipi_dbi_fb_funcs, driver);
+	ret = devm_tinydrm_init(dev, tdev, driver);
 	if (ret)
 		return ret;
 
-	tdev->fb_dirty = mipi_dbi_fb_dirty;
-
 	/* TODO: Maybe add DRM_MODE_CONNECTOR_SPI */
 	ret = tinydrm_display_pipe_init(tdev, pipe_funcs,
 					DRM_MODE_CONNECTOR_VIRTUAL,
@@ -392,6 +410,8 @@  int mipi_dbi_init(struct device *dev, struct mipi_dbi *mipi,
 	if (ret)
 		return ret;
 
+	drm_plane_enable_fb_damage_clips(&tdev->pipe.plane);
+
 	tdev->drm->mode_config.preferred_depth = 16;
 	mipi->rotation = rotation;
 
diff --git a/drivers/gpu/drm/tinydrm/repaper.c b/drivers/gpu/drm/tinydrm/repaper.c
index 238515de449e..f9cacf49f16b 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_damage_helper.h>
 #include <drm/drm_drv.h>
 #include <drm/drm_fb_cma_helper.h>
 #include <drm/drm_gem_cma_helper.h>
@@ -523,11 +524,7 @@  static void repaper_gray8_to_mono_reversed(u8 *buf, u32 width, u32 height)
 		}
 }
 
-static int repaper_fb_dirty(struct drm_framebuffer *fb,
-			    struct drm_file *file_priv,
-			    unsigned int flags, unsigned int color,
-			    struct drm_clip_rect *clips,
-			    unsigned int num_clips)
+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;
@@ -626,12 +623,6 @@  static int repaper_fb_dirty(struct drm_framebuffer *fb,
 	return ret;
 }
 
-static const struct drm_framebuffer_funcs repaper_fb_funcs = {
-	.destroy	= drm_gem_fb_destroy,
-	.create_handle	= drm_gem_fb_create_handle,
-	.dirty		= tinydrm_fb_dirty,
-};
-
 static void power_off(struct repaper_epd *epd)
 {
 	/* Turn off power and all signals */
@@ -795,9 +786,7 @@  static void repaper_pipe_disable(struct drm_simple_display_pipe *pipe)
 
 	DRM_DEBUG_DRIVER("\n");
 
-	mutex_lock(&tdev->dirty_lock);
 	epd->enabled = false;
-	mutex_unlock(&tdev->dirty_lock);
 
 	/* Nothing frame */
 	for (line = 0; line < epd->height; line++)
@@ -840,10 +829,28 @@  static void repaper_pipe_disable(struct drm_simple_display_pipe *pipe)
 	power_off(epd);
 }
 
+static void repaper_pipe_update(struct drm_simple_display_pipe *pipe,
+				struct drm_plane_state *old_state)
+{
+	struct drm_plane_state *state = pipe->plane.state;
+	struct drm_crtc *crtc = &pipe->crtc;
+	struct drm_rect rect;
+
+	if (drm_atomic_helper_damage_merged(old_state, state, &rect))
+		repaper_fb_dirty(state->fb);
+
+	if (crtc->state->event) {
+		spin_lock_irq(&crtc->dev->event_lock);
+		drm_crtc_send_vblank_event(crtc, crtc->state->event);
+		spin_unlock_irq(&crtc->dev->event_lock);
+		crtc->state->event = NULL;
+	}
+}
+
 static const struct drm_simple_display_pipe_funcs repaper_pipe_funcs = {
 	.enable = repaper_pipe_enable,
 	.disable = repaper_pipe_disable,
-	.update = tinydrm_display_pipe_update,
+	.update = repaper_pipe_update,
 	.prepare_fb = drm_gem_fb_simple_display_pipe_prepare_fb,
 };
 
@@ -1057,12 +1064,10 @@  static int repaper_probe(struct spi_device *spi)
 
 	tdev = &epd->tinydrm;
 
-	ret = devm_tinydrm_init(dev, tdev, &repaper_fb_funcs, &repaper_driver);
+	ret = devm_tinydrm_init(dev, tdev, &repaper_driver);
 	if (ret)
 		return ret;
 
-	tdev->fb_dirty = repaper_fb_dirty;
-
 	ret = tinydrm_display_pipe_init(tdev, &repaper_pipe_funcs,
 					DRM_MODE_CONNECTOR_VIRTUAL,
 					repaper_formats,
diff --git a/drivers/gpu/drm/tinydrm/st7586.c b/drivers/gpu/drm/tinydrm/st7586.c
index a52bc3b02606..50e370ce5a59 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_damage_helper.h>
 #include <drm/drm_drv.h>
 #include <drm/drm_fb_cma_helper.h>
 #include <drm/drm_gem_cma_helper.h>
@@ -112,23 +113,15 @@  static int st7586_buf_copy(void *dst, struct drm_framebuffer *fb,
 	return ret;
 }
 
-static int st7586_fb_dirty(struct drm_framebuffer *fb,
-			   struct drm_file *file_priv, unsigned int flags,
-			   unsigned int color, struct drm_clip_rect *clips,
-			   unsigned int num_clips)
+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 drm_rect clip;
-	struct drm_rect *rect = &clip;
 	int start, end;
 	int ret = 0;
 
 	if (!mipi->enabled)
-		return 0;
-
-	tinydrm_merge_clips(&clip, clips, num_clips, flags, fb->width,
-			    fb->height);
+		return;
 
 	/* 3 pixels per byte, so grow clip to nearest multiple of 3 */
 	rect->x1 = rounddown(rect->x1, 3);
@@ -138,7 +131,7 @@  static int st7586_fb_dirty(struct drm_framebuffer *fb,
 
 	ret = st7586_buf_copy(mipi->tx_buf, fb, rect);
 	if (ret)
-		return ret;
+		goto err_msg;
 
 	/* Pixels are packed 3 per byte */
 	start = rect->x1 / 3;
@@ -154,15 +147,28 @@  static int st7586_fb_dirty(struct drm_framebuffer *fb,
 	ret = mipi_dbi_command_buf(mipi, MIPI_DCS_WRITE_MEMORY_START,
 				   (u8 *)mipi->tx_buf,
 				   (end - start) * (rect->y2 - rect->y1));
-
-	return ret;
+err_msg:
+	if (ret)
+		dev_err_once(fb->dev->dev, "Failed to update display %d\n", ret);
 }
 
-static const struct drm_framebuffer_funcs st7586_fb_funcs = {
-	.destroy	= drm_gem_fb_destroy,
-	.create_handle	= drm_gem_fb_create_handle,
-	.dirty		= tinydrm_fb_dirty,
-};
+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_crtc *crtc = &pipe->crtc;
+	struct drm_rect rect;
+
+	if (drm_atomic_helper_damage_merged(old_state, state, &rect))
+		st7586_fb_dirty(state->fb, &rect);
+
+	if (crtc->state->event) {
+		spin_lock_irq(&crtc->dev->event_lock);
+		drm_crtc_send_vblank_event(crtc, crtc->state->event);
+		spin_unlock_irq(&crtc->dev->event_lock);
+		crtc->state->event = NULL;
+	}
+}
 
 static void st7586_pipe_enable(struct drm_simple_display_pipe *pipe,
 			       struct drm_crtc_state *crtc_state,
@@ -264,12 +270,10 @@  static int st7586_init(struct device *dev, struct mipi_dbi *mipi,
 	if (!mipi->tx_buf)
 		return -ENOMEM;
 
-	ret = devm_tinydrm_init(dev, tdev, &st7586_fb_funcs, driver);
+	ret = devm_tinydrm_init(dev, tdev, driver);
 	if (ret)
 		return ret;
 
-	tdev->fb_dirty = st7586_fb_dirty;
-
 	ret = tinydrm_display_pipe_init(tdev, pipe_funcs,
 					DRM_MODE_CONNECTOR_VIRTUAL,
 					st7586_formats,
@@ -278,6 +282,8 @@  static int st7586_init(struct device *dev, struct mipi_dbi *mipi,
 	if (ret)
 		return ret;
 
+	drm_plane_enable_fb_damage_clips(&tdev->pipe.plane);
+
 	tdev->drm->mode_config.preferred_depth = 32;
 	mipi->rotation = rotation;
 
@@ -292,7 +298,7 @@  static int st7586_init(struct device *dev, struct mipi_dbi *mipi,
 static const struct drm_simple_display_pipe_funcs st7586_pipe_funcs = {
 	.enable		= st7586_pipe_enable,
 	.disable	= st7586_pipe_disable,
-	.update		= tinydrm_display_pipe_update,
+	.update		= st7586_pipe_update,
 	.prepare_fb	= drm_gem_fb_simple_display_pipe_prepare_fb,
 };
 
diff --git a/drivers/gpu/drm/tinydrm/st7735r.c b/drivers/gpu/drm/tinydrm/st7735r.c
index 9bc93d5a0401..3bab9a9569a6 100644
--- a/drivers/gpu/drm/tinydrm/st7735r.c
+++ b/drivers/gpu/drm/tinydrm/st7735r.c
@@ -106,7 +106,7 @@  static void jd_t18003_t01_pipe_enable(struct drm_simple_display_pipe *pipe,
 static const struct drm_simple_display_pipe_funcs jd_t18003_t01_pipe_funcs = {
 	.enable		= jd_t18003_t01_pipe_enable,
 	.disable	= mipi_dbi_pipe_disable,
-	.update		= tinydrm_display_pipe_update,
+	.update		= mipi_dbi_pipe_update,
 	.prepare_fb	= drm_gem_fb_simple_display_pipe_prepare_fb,
 };
 
diff --git a/include/drm/tinydrm/mipi-dbi.h b/include/drm/tinydrm/mipi-dbi.h
index b52f32897f23..f4ec2834bc22 100644
--- a/include/drm/tinydrm/mipi-dbi.h
+++ b/include/drm/tinydrm/mipi-dbi.h
@@ -68,6 +68,8 @@  int mipi_dbi_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);
+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,
 			   struct drm_crtc_state *crtc_state,
 			   struct drm_plane_state *plan_state);
diff --git a/include/drm/tinydrm/tinydrm-helpers.h b/include/drm/tinydrm/tinydrm-helpers.h
index 8edb75df4e31..f0d598789e4d 100644
--- a/include/drm/tinydrm/tinydrm-helpers.h
+++ b/include/drm/tinydrm/tinydrm-helpers.h
@@ -11,8 +11,7 @@ 
 #define __LINUX_TINYDRM_HELPERS_H
 
 struct backlight_device;
-struct tinydrm_device;
-struct drm_clip_rect;
+struct drm_framebuffer;
 struct drm_rect;
 struct spi_transfer;
 struct spi_message;
@@ -34,14 +33,6 @@  static inline bool tinydrm_machine_little_endian(void)
 #endif
 }
 
-bool tinydrm_merge_clips(struct drm_rect *dst,
-			 struct drm_clip_rect *src, unsigned int num_clips,
-			 unsigned int flags, u32 max_width, u32 max_height);
-int tinydrm_fb_dirty(struct drm_framebuffer *fb,
-		     struct drm_file *file_priv,
-		     unsigned int flags, unsigned int color,
-		     struct drm_clip_rect *clips,
-		     unsigned int num_clips);
 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 448aa5ea4722..5621688edcc0 100644
--- a/include/drm/tinydrm/tinydrm.h
+++ b/include/drm/tinydrm/tinydrm.h
@@ -10,14 +10,9 @@ 
 #ifndef __LINUX_TINYDRM_H
 #define __LINUX_TINYDRM_H
 
-#include <linux/mutex.h>
 #include <drm/drm_simple_kms_helper.h>
 
-struct drm_clip_rect;
 struct drm_driver;
-struct drm_file;
-struct drm_framebuffer;
-struct drm_framebuffer_funcs;
 
 /**
  * struct tinydrm_device - tinydrm device
@@ -32,24 +27,6 @@  struct tinydrm_device {
 	 * @pipe: Display pipe structure
 	 */
 	struct drm_simple_display_pipe pipe;
-
-	/**
-	 * @dirty_lock: Serializes framebuffer flushing
-	 */
-	struct mutex dirty_lock;
-
-	/**
-	 * @fb_funcs: Framebuffer functions used when creating framebuffers
-	 */
-	const struct drm_framebuffer_funcs *fb_funcs;
-
-	/**
-	 * @fb_dirty: Framebuffer dirty callback
-	 */
-	int (*fb_dirty)(struct drm_framebuffer *framebuffer,
-			struct drm_file *file_priv, unsigned flags,
-			unsigned color, struct drm_clip_rect *clips,
-			unsigned num_clips);
 };
 
 static inline struct tinydrm_device *
@@ -82,13 +59,10 @@  pipe_to_tinydrm(struct drm_simple_display_pipe *pipe)
 	.clock = 1 /* pass validation */
 
 int devm_tinydrm_init(struct device *parent, struct tinydrm_device *tdev,
-		      const struct drm_framebuffer_funcs *fb_funcs,
 		      struct drm_driver *driver);
 int devm_tinydrm_register(struct tinydrm_device *tdev);
 void tinydrm_shutdown(struct tinydrm_device *tdev);
 
-void tinydrm_display_pipe_update(struct drm_simple_display_pipe *pipe,
-				 struct drm_plane_state *old_state);
 int
 tinydrm_display_pipe_init(struct tinydrm_device *tdev,
 			  const struct drm_simple_display_pipe_funcs *funcs,

Comments

David Lechner Jan. 13, 2019, 9:19 p.m.
On 1/11/19 2:12 PM, Noralf Trønnes wrote:
> This switches to drm_atomic_helper_dirtyfb() as the framebuffer dirty
> handler. All flushing will now happen in the pipe functions.
> 
> Also enable the damage plane property for all except repaper which can
> only do full updates.
> 
> ili9225:
> This change made ili9225_init() equal to mipi_dbi_init() so use it.
> 
> v2: Remove fb check in mipi_dbi_enable_flush() it can't be NULL
>      (kbuild test robot)
> 
> Cc: David Lechner <david@lechnology.com>
> Cc: Eric Anholt <eric@anholt.net>
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> Acked-by: Sam Ravnborg <sam@ravnborg.org>
> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---

Tested-by: David Lechner <david@lechnology.com>
Reviewed-by: David Lechner <david@lechnology.com>

Tested with ST7586 on LEGO MINDSTORMS EV3 and ILI9225 on
BeagleBone Green. (FWIW, I encounter other issues that had to
be fixed to make things work on both of these, but none were
actually related to this series.)
David Lechner Jan. 14, 2019, 2:15 a.m.
On 1/13/19 3:19 PM, David Lechner wrote:
> On 1/11/19 2:12 PM, Noralf Trønnes wrote:
>> This switches to drm_atomic_helper_dirtyfb() as the framebuffer dirty
>> handler. All flushing will now happen in the pipe functions.
>>
>> Also enable the damage plane property for all except repaper which can
>> only do full updates.
>>
>> ili9225:
>> This change made ili9225_init() equal to mipi_dbi_init() so use it.
>>
>> v2: Remove fb check in mipi_dbi_enable_flush() it can't be NULL
>>      (kbuild test robot)
>>
>> Cc: David Lechner <david@lechnology.com>
>> Cc: Eric Anholt <eric@anholt.net>
>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>> Acked-by: Sam Ravnborg <sam@ravnborg.org>
>> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>> ---
> 
> Tested-by: David Lechner <david@lechnology.com>
> Reviewed-by: David Lechner <david@lechnology.com>
> 
> Tested with ST7586 on LEGO MINDSTORMS EV3 and ILI9225 on
> BeagleBone Green. (FWIW, I encounter other issues that had to
> be fixed to make things work on both of these, but none were
> actually related to this series.)
> 

Actually, I have to take this back. It appears that the problems
that I had on LEGO MINDSTORMS EV3 are actually caused by this
series. I did a git bisect to be sure and ended up with this
patch as the bad commit. I'm guessing that the damage helper
must be causing some kind of memory corruption. Here is the
crash I am getting:

Unable to handle kernel NULL pointer dereference at virtual address 00000004
pgd = (ptrval)
[00000004] *pgd=00000000
Internal error: Oops: 5 [#1] PREEMPT ARM
Modules linked in:
CPU: 0 PID: 1 Comm: swapper Not tainted 5.0.0-rc1-00125-ge9acadba5409 #1389
Hardware name: Generic DA850/OMAP-L138/AM18x
PC is at rb_insert_color+0x1c/0x1a4
LR is at kernfs_link_sibling+0x94/0xcc
pc : [<c04b95ec>]    lr : [<c014bfdc>]    psr: 60000013
sp : c2831b38  ip : 00000000  fp : c06b762c
r10: 00000000  r9 : c06b835c  r8 : 00000000
r7 : c2963f00  r6 : c066b028  r5 : c2016cc0  r4 : 00000000
r3 : 00000000  r2 : c2983010  r1 : c2963f2c  r0 : c2016cd0
Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment none
Control: 0005317f  Table: c0004000  DAC: 00000053
Process swapper (pid: 1, stack limit = 0x(ptrval))
Stack: (0xc2831b38 to 0xc2832000)
1b20:                                                       00000000 c2016cc0
1b40: c066b028 c014bfdc c2016cc0 c2963f00 c066b028 c014c768 c2963f00 c014c448
1b60: 00000000 c2016cc0 c2963f00 c066b028 c2963f00 c014c860 00000000 00000001
1b80: c0589938 c2b4b408 00000000 c014ec70 00000000 c2b4b408 00000000 c04c4cb0
1ba0: 0000070f 00000000 00000000 9fd04dd9 00000000 c2b4b408 c066b028 00000000
1bc0: c293ac98 c04b58f8 c059081c 00000000 c2b4b408 c066b028 00000000 00000000
1be0: 00000000 c04b5d64 c2831c08 9fd04dd9 c2b4b3c0 c293ac80 c2bd16c0 c2b4b408
1c00: c00d650c c059081c c2bd16c0 c28e3a80 c2b4b3c0 00000000 c2b4b3c0 00000000
1c20: c06b7634 00020000 c06b835c c00d72f8 00000000 c00b0c24 00000000 00000074
1c40: c0590728 00000000 c0590728 c2b4b3c0 00000074 c0590728 00020000 00000000
1c60: c06b762c c00b0958 00000000 00380bc6 00000000 c06bbf1c c2bd9c00 c2bfe000
1c80: c06bbf14 0000000c 00000000 00000000 c2831e0c c00b0a90 00000000 00000000
1ca0: 00000000 00000000 003b2580 c017d6e4 00000000 00000000 00000000 c2bfe000
1cc0: c2bd9c00 00000000 003b2580 00000000 00000000 c01971a0 00001103 00000000
1ce0: c2bd8400 00000400 00000400 9fd04dd9 00000000 0000000a 00000002 00000000
1d00: ffffffff 00000000 ffffffff 00000000 00000000 00000001 00000a04 00000032
1d20: 00000000 0000000c 00000004 c00af550 c2bd9ecc 9fd04dd9 c2404480 c2bd9eb4
1d40: c2bd8400 c2404480 00000002 00000001 00000000 00000000 00000001 00000000
1d60: 00000000 00000000 00000000 00000000 00000000 00000000 00000001 00000000
1d80: 00380bc6 00000000 003b257f 00000000 00000077 0000ffff 00000200 00000002
1da0: 00000001 0000ffff 00000000 00000401 c2bfe22c 00000000 00000000 00000000
1dc0: 00000000 00000000 c2012400 c24be100 00001000 00000000 c2404480 00000000
1de0: 00004003 9fd04dd9 00000000 c2bd9c00 c2404480 00000083 c24044fc 00008000
1e00: 00000000 00000020 00008000 c00e4d88 c2404480 00000000 00000000 c0190afc
1e20: c2bd0be0 c0692898 c0692898 00000000 00000020 c0190b10 c0194f48 c2bd0be0
1e40: c066b370 c00e568c c29f5380 c01002d4 c29f5380 c2bd0be0 00008000 c0100488
1e60: c0692898 00000000 c066b028 c2bd0be0 c2bd0bc0 c01033ec 00000000 ffffff00
1e80: ffff0a00 c0575ff4 c2bd0be0 c281a010 c2417098 c2bd0be0 0000000a 00000000
1ea0: 0000000a 9fd04dd9 0000000a c2bd0be0 c2bd0bc0 00000000 c0575ff8 00008000
1ec0: c066b028 00008000 c0575ff4 c0104178 00000000 00000000 c2014000 c2014005
1ee0: c0575ff8 c3fb1280 c0652868 c062b1ec 00000000 c01013d8 c24c3558 00000000
1f00: 00000000 00006180 c24c3558 c00f17c4 e10c76ac 0b300002 c2858015 c281a010
1f20: c2415da8 9fd04dd9 00000000 00000002 c06ad310 c066b028 c066da68 00000000
1f40: 00000000 00000000 00000000 c062b478 00000000 c0037200 00306b6c 9fd00000
1f60: c0576098 9fd04dd9 00000002 c06ad310 c0652878 00000000 00000000 00000000
1f80: 00000000 00000000 00000000 c062b5e4 00000000 00000000 00000000 00000000
1fa0: c04c7860 c04c7868 00000000 c00090e0 00000000 00000000 00000000 00000000
1fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
1fe0: 00000000 00000000 00000000 00000000 00000013 00000000 00000000 00000000
[<c04b95ec>] (rb_insert_color) from [<c014bfdc>] (kernfs_link_sibling+0x94/0xcc)
[<c014bfdc>] (kernfs_link_sibling) from [<c014c768>] (kernfs_add_one+0x90/0x140)
[<c014c768>] (kernfs_add_one) from [<c014c860>] (kernfs_create_dir_ns+0x48/0x74)
[<c014c860>] (kernfs_create_dir_ns) from [<c014ec70>] (sysfs_create_dir_ns+0x68/0xd0)
[<c014ec70>] (sysfs_create_dir_ns) from [<c04b58f8>] (kobject_add_internal+0x9c/0x2c4)
[<c04b58f8>] (kobject_add_internal) from [<c04b5d64>] (kobject_init_and_add+0x54/0x94)
[<c04b5d64>] (kobject_init_and_add) from [<c00d650c>] (sysfs_slab_add+0x10c/0x220)
[<c00d650c>] (sysfs_slab_add) from [<c00d72f8>] (__kmem_cache_create+0x1d8/0x338)
[<c00d72f8>] (__kmem_cache_create) from [<c00b0958>] (kmem_cache_create_usercopy+0x180/0x298)
[<c00b0958>] (kmem_cache_create_usercopy) from [<c00b0a90>] (kmem_cache_create+0x20/0x28)
[<c00b0a90>] (kmem_cache_create) from [<c017d6e4>] (ext4_mb_init+0x374/0x44c)
[<c017d6e4>] (ext4_mb_init) from [<c01971a0>] (ext4_fill_super+0x2258/0x2ef0)
[<c01971a0>] (ext4_fill_super) from [<c00e4d88>] (mount_bdev+0x154/0x18c)
[<c00e4d88>] (mount_bdev) from [<c0190b10>] (ext4_mount+0x14/0x20)
[<c0190b10>] (ext4_mount) from [<c00e568c>] (mount_fs+0x14/0xa8)
[<c00e568c>] (mount_fs) from [<c0100488>] (vfs_kern_mount+0x48/0xf0)
[<c0100488>] (vfs_kern_mount) from [<c01033ec>] (do_mount+0x180/0xb9c)
[<c01033ec>] (do_mount) from [<c0104178>] (ksys_mount+0x8c/0xb4)
[<c0104178>] (ksys_mount) from [<c062b1ec>] (mount_block_root+0x128/0x2a4)
[<c062b1ec>] (mount_block_root) from [<c062b478>] (mount_root+0x110/0x154)
[<c062b478>] (mount_root) from [<c062b5e4>] (prepare_namespace+0x128/0x188)
[<c062b5e4>] (prepare_namespace) from [<c04c7868>] (kernel_init+0x8/0xf4)
[<c04c7868>] (kernel_init) from [<c00090e0>] (ret_from_fork+0x14/0x34)
Exception stack(0xc2831fb0 to 0xc2831ff8)
1fa0:                                     00000000 00000000 00000000 00000000
1fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
1fe0: 00000000 00000000 00000000 00000000 00000013 00000000
Code: e5923000 e3130001 1a000054 e92d4070 (e593c004)
---[ end trace 2c8fc104914a532b ]---
Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
---[ end Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b ]---
David Lechner Jan. 14, 2019, 10:06 p.m.
On 1/11/19 2:12 PM, Noralf Trønnes wrote:
> This switches to drm_atomic_helper_dirtyfb() as the framebuffer dirty
> handler. All flushing will now happen in the pipe functions.
> 
> Also enable the damage plane property for all except repaper which can
> only do full updates.
> 
> ili9225:
> This change made ili9225_init() equal to mipi_dbi_init() so use it.
> 
> v2: Remove fb check in mipi_dbi_enable_flush() it can't be NULL
>      (kbuild test robot)
> 
> Cc: David Lechner <david@lechnology.com>
> Cc: Eric Anholt <eric@anholt.net>
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> Acked-by: Sam Ravnborg <sam@ravnborg.org>
> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---


...

> diff --git a/drivers/gpu/drm/tinydrm/st7586.c b/drivers/gpu/drm/tinydrm/st7586.c
> index a52bc3b02606..50e370ce5a59 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_damage_helper.h>
>   #include <drm/drm_drv.h>
>   #include <drm/drm_fb_cma_helper.h>
>   #include <drm/drm_gem_cma_helper.h>
> @@ -112,23 +113,15 @@ static int st7586_buf_copy(void *dst, struct drm_framebuffer *fb,
>   	return ret;
>   }
>   
> -static int st7586_fb_dirty(struct drm_framebuffer *fb,
> -			   struct drm_file *file_priv, unsigned int flags,
> -			   unsigned int color, struct drm_clip_rect *clips,
> -			   unsigned int num_clips)
> +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 drm_rect clip;
> -	struct drm_rect *rect = &clip;

This function later modifies the fields of the struct pointed to by rect.
Are all callers of this function OK with having it modified or should we
make a local copy of rect and modify that instead?
Noralf Trønnes Jan. 14, 2019, 10:46 p.m.
Den 14.01.2019 23.06, skrev David Lechner:
> On 1/11/19 2:12 PM, Noralf Trønnes wrote:
>> This switches to drm_atomic_helper_dirtyfb() as the framebuffer dirty
>> handler. All flushing will now happen in the pipe functions.
>>
>> Also enable the damage plane property for all except repaper which can
>> only do full updates.
>>
>> ili9225:
>> This change made ili9225_init() equal to mipi_dbi_init() so use it.
>>
>> v2: Remove fb check in mipi_dbi_enable_flush() it can't be NULL
>>      (kbuild test robot)
>>
>> Cc: David Lechner <david@lechnology.com>
>> Cc: Eric Anholt <eric@anholt.net>
>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>> Acked-by: Sam Ravnborg <sam@ravnborg.org>
>> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>> ---
> 
> 
> ...
> 
>> diff --git a/drivers/gpu/drm/tinydrm/st7586.c
>> b/drivers/gpu/drm/tinydrm/st7586.c
>> index a52bc3b02606..50e370ce5a59 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_damage_helper.h>
>>   #include <drm/drm_drv.h>
>>   #include <drm/drm_fb_cma_helper.h>
>>   #include <drm/drm_gem_cma_helper.h>
>> @@ -112,23 +113,15 @@ static int st7586_buf_copy(void *dst, struct
>> drm_framebuffer *fb,
>>       return ret;
>>   }
>>   -static int st7586_fb_dirty(struct drm_framebuffer *fb,
>> -               struct drm_file *file_priv, unsigned int flags,
>> -               unsigned int color, struct drm_clip_rect *clips,
>> -               unsigned int num_clips)
>> +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 drm_rect clip;
>> -    struct drm_rect *rect = &clip;
> 
> This function later modifies the fields of the struct pointed to by rect.
> Are all callers of this function OK with having it modified or should we
> make a local copy of rect and modify that instead?

It's safe to modify it. st7586_pipe_update() owns the structure and it
doesn't use it after calling st7586_fb_dirty().

Noralf.