[PATCH 1/3] video: fb: add optional damage tracking

[PATCH 1/3] video: fb: add optional damage tracking

[Philipp Zabel]

Add an optional fb_damage operation that drivers may use to accumulate
damage on the framebuffer until fb_flush is called. The accumulated
damage can be used to support partial updates for displays with an
integrated framebuffer.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/video/fb.c | 6 ++++++
 include/fb.h       | 9 +++++++++
 2 files changed, 15 insertions(+)

diff --git a/drivers/video/fb.c b/drivers/video/fb.c
index 44754065e7d9..6f412d62c434 100644
--- a/drivers/video/fb.c
+++ b/drivers/video/fb.c
@@ -43,6 +43,12 @@ static int fb_close(struct cdev *cdev)
 	return 0;
 }
 
+void fb_damage(struct fb_info *info, struct fb_rect *rect)
+{
+	if (info->fbops->fb_damage)
+		info->fbops->fb_damage(info, rect);
+}
+
 static int fb_op_flush(struct cdev *cdev)
 {
 	struct fb_info *info = cdev->priv;
diff --git a/include/fb.h b/include/fb.h
index 15bb74b99576..4e3eb611db48 100644
--- a/include/fb.h
+++ b/include/fb.h
@@ -80,6 +80,13 @@ struct fb_bitfield {
 
 struct fb_info;
 
+struct fb_rect {
+	u32 x1;
+	u32 y1;
+	u32 x2;
+	u32 y2;
+};
+
 struct fb_ops {
 	/* set color register */
 	int (*fb_setcolreg)(unsigned regno, unsigned red, unsigned green,
@@ -87,6 +94,7 @@ struct fb_ops {
 	void (*fb_enable)(struct fb_info *info);
 	void (*fb_disable)(struct fb_info *info);
 	int (*fb_activate_var)(struct fb_info *info);
+	void (*fb_damage)(struct fb_info *info, struct fb_rect *rect);
 	void (*fb_flush)(struct fb_info *info);
 };
 
@@ -156,6 +164,7 @@ int register_framebuffer(struct fb_info *info);
 
 int fb_enable(struct fb_info *info);
 int fb_disable(struct fb_info *info);
+void fb_damage(struct fb_info *info, struct fb_rect *rect);
 void fb_flush(struct fb_info *info);
 
 #define FBIOGET_SCREENINFO	_IOR('F', 1, loff_t)
-- 
2.39.2

[PATCH 2/3] video: fbconsole: add optional damage tracking

[Philipp Zabel]

Annotate framebuffer updates with damage rectangles so drivers may
implement partial updates for displays with an integrated framebuffer.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/video/fbconsole.c | 32 ++++++++++++++++++++++++++++----
 1 file changed, 28 insertions(+), 4 deletions(-)

diff --git a/drivers/video/fbconsole.c b/drivers/video/fbconsole.c
index 070378aa2352..25d7ea685ff6 100644
--- a/drivers/video/fbconsole.c
+++ b/drivers/video/fbconsole.c
@@ -18,6 +18,7 @@ enum state_t {
 struct fbc_priv {
 	struct console_device cdev;
 	struct fb_info *fb;
+	struct fb_rect rect;
 
 	struct screen *sc;
 
@@ -60,9 +61,11 @@ static int fbc_tstc(struct console_device *cdev)
 static void cls(struct fbc_priv *priv)
 {
 	void *buf = gui_screen_render_buffer(priv->sc);
+	struct fb_info *fb = priv->fb;
 
 	memset(buf, 0, priv->fb->line_length * priv->fb->yres);
 	gu_screen_blit(priv->sc);
+	fb_damage(fb, &priv->rect);
 }
 
 struct rgb {
@@ -139,13 +142,20 @@ static void drawchar(struct fbc_priv *priv, int x, int y, int c)
 static void video_invertchar(struct fbc_priv *priv, int x, int y)
 {
 	void *buf;
+	struct fb_rect rect;
+
+	rect.x1 = x * priv->font->width;
+	rect.y1 = y * priv->font->height;
+	rect.x2 = rect.x1 + priv->font->width;
+	rect.y2 = rect.y1 + priv->font->height;
 
 	buf = gui_screen_render_buffer(priv->sc);
 
-	gu_invert_area(priv->fb, buf, x * priv->font->width, y * priv->font->height,
+	gu_invert_area(priv->fb, buf, rect.x1, rect.y1,
 			priv->font->width, priv->font->height);
-	gu_screen_blit_area(priv->sc, x * priv->font->width, y * priv->font->height,
+	gu_screen_blit_area(priv->sc, rect.x1, rect.y1,
 			priv->font->width, priv->font->height);
+	fb_damage(priv->fb, &rect);
 }
 
 static void show_cursor(struct fbc_priv *priv, int x, int y)
@@ -156,6 +166,9 @@ static void show_cursor(struct fbc_priv *priv, int x, int y)
 
 static void printchar(struct fbc_priv *priv, int c)
 {
+	struct fb_info *fb = priv->fb;
+	struct fb_rect rect;
+
 	video_invertchar(priv, priv->x, priv->y);
 
 	switch (c) {
@@ -185,9 +198,14 @@ static void printchar(struct fbc_priv *priv, int c)
 	default:
 		drawchar(priv, priv->x, priv->y, c);
 
-		gu_screen_blit_area(priv->sc, priv->x * priv->font->width,
-				priv->y * priv->font->height,
+		rect.x1 = priv->x * priv->font->width;
+		rect.y1 = priv->y * priv->font->height;
+		rect.x2 =  rect.x1 + priv->font->width;
+		rect.y2 =  rect.y1 + priv->font->height;
+
+		gu_screen_blit_area(priv->sc, rect.x1, rect.y1,
 				priv->font->width, priv->font->height);
+		fb_damage(fb, &rect);
 
 		priv->x++;
 		if (priv->x > priv->cols) {
@@ -207,6 +225,7 @@ static void printchar(struct fbc_priv *priv, int c)
 		memset(buf + line_height * priv->rows, 0, line_height);
 
 		gu_screen_blit(priv->sc);
+		fb_damage(fb, &priv->rect);
 		priv->y = priv->rows;
 	}
 
@@ -508,6 +527,11 @@ int register_fbconsole(struct fb_info *fb)
 			set_font, NULL,
 			&priv->par_font_val, priv);
 
+	priv->rect.x1 = 0;
+	priv->rect.y1 = 0;
+	priv->rect.x2 = fb->xres;
+	priv->rect.y2 = fb->yres;
+
 	pr_info("registered as %s%d\n", cdev->class_dev.name, cdev->class_dev.id);
 
 	return 0;
-- 
2.39.2

[PATCH 3/3] video: mipi_dbi: add damage tracking and partial updates

[Philipp Zabel]

Track framebuffer damage with a simple rectangle that can be used to
issue partial updates during fb_flush. This speeds up fbconsole.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/video/mipi_dbi.c       | 83 ++++++++++++++++++++++++++++------
 drivers/video/panel-mipi-dbi.c |  1 +
 include/video/mipi_dbi.h       |  6 +++
 3 files changed, 75 insertions(+), 15 deletions(-)

diff --git a/drivers/video/mipi_dbi.c b/drivers/video/mipi_dbi.c
index 61b0fbcc49c6..3107725b224b 100644
--- a/drivers/video/mipi_dbi.c
+++ b/drivers/video/mipi_dbi.c
@@ -199,19 +199,33 @@ int mipi_dbi_command_stackbuf(struct mipi_dbi *dbi, u8 cmd, const u8 *data,
 EXPORT_SYMBOL(mipi_dbi_command_stackbuf);
 
 /**
- * mipi_dbi_buf_copy_swap - Copy a framebuffer swapping MSB/LSB of 16-bit values
+ * mipi_dbi_buf_copy - Copy a framebuffer, transforming it if necessary
  * @dst: The destination buffer
  * @info: The source framebuffer info
+ * @clip: Clipping rectangle of the area to be copied
+ * @swap: When true, swap MSB/LSB of 16-bit values
  */
-static void mipi_dbi_buf_copy_swap(u16 *dst, struct fb_info *info)
+static void mipi_dbi_buf_copy(u16 *dst, struct fb_info *info,
+			      struct fb_rect *clip, bool swap)
 {
 	u16 *src = (u16 *)info->screen_base;
-	size_t len = info->xres * info->yres;
-	int i;
+	unsigned int height = clip->y2 - clip->y1;
+	unsigned int width = clip->x2 - clip->x1;
+	int x, y;
 
-	for (i = 0; i < len; i++) {
-		*dst++ = *src << 8 | *src >> 8;
-		src++;
+	src += clip->y1 * info->xres + clip->x1;
+	if (swap) {
+		for (y = 0; y < height; y++) {
+			for (x = 0; x < width; x++)
+				*dst++ = src[x] << 8 | src[x] >> 8;
+			src += info->xres;
+		}
+	} else {
+		for (y = 0; y < height; y++) {
+			memcpy(dst, src, 2 * width);
+			dst += width;
+			src += info->xres;
+		}
 	}
 }
 
@@ -232,28 +246,38 @@ 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 mipi_dbi_dev *dbidev, struct fb_info *info)
+static void mipi_dbi_fb_dirty(struct mipi_dbi_dev *dbidev, struct fb_info *info,
+			      struct fb_rect *rect)
 {
+	unsigned int height = rect->y2 - rect->y1;
+	unsigned int width = rect->x2 - rect->x1;
 	struct mipi_dbi *dbi = &dbidev->dbi;
-	unsigned int height = info->yres;
-	unsigned int width = info->xres;
 	bool swap = dbi->swap_bytes;
 	int ret;
+	bool full;
 	void *tr;
 
-	if (swap) {
+	full = width == info->xres && height == info->yres;
+
+	if (!full || swap) {
 		tr = dbidev->tx_buf;
-		mipi_dbi_buf_copy_swap(tr, info);
+		mipi_dbi_buf_copy(tr, info, rect, swap);
 	} else {
 		tr = info->screen_base;
 	}
 
-	mipi_dbi_set_window_address(dbidev, 0, width - 1, 0, height - 1);
+	mipi_dbi_set_window_address(dbidev, rect->x1, rect->x2 - 1, rect->y1,
+				    rect->y2 - 1);
 
 	ret = mipi_dbi_command_buf(dbi, MIPI_DCS_WRITE_MEMORY_START, tr,
 				   width * height * 2);
 	if (ret)
 		pr_err_once("Failed to update display %d\n", ret);
+
+	dbidev->damage.x1 = 0;
+	dbidev->damage.y1 = 0;
+	dbidev->damage.x2 = 0;
+	dbidev->damage.y2 = 0;
 }
 
 /**
@@ -267,7 +291,14 @@ static void mipi_dbi_fb_dirty(struct mipi_dbi_dev *dbidev, struct fb_info *info)
 void mipi_dbi_enable_flush(struct mipi_dbi_dev *dbidev,
 			   struct fb_info *info)
 {
-	mipi_dbi_fb_dirty(dbidev, info);
+	struct fb_rect rect = {
+		.x1 = 0,
+		.y1 = 0,
+		.x2 = info->xres,
+		.y2 = info->yres
+	};
+
+	mipi_dbi_fb_dirty(dbidev, info, &rect);
 
 	if (dbidev->backlight)
 		backlight_set_brightness_default(dbidev->backlight);
@@ -309,11 +340,33 @@ void mipi_dbi_fb_disable(struct fb_info *info)
 }
 EXPORT_SYMBOL(mipi_dbi_fb_disable);
 
+void mipi_dbi_fb_damage(struct fb_info *info, struct fb_rect *rect)
+{
+	struct mipi_dbi_dev *dbidev = container_of(info, struct mipi_dbi_dev, info);
+
+	if (dbidev->damage.x2 && dbidev->damage.y2) {
+		dbidev->damage.x1 = min(dbidev->damage.x1, rect->x1);
+		dbidev->damage.y1 = min(dbidev->damage.y1, rect->y1);
+		dbidev->damage.x2 = max(dbidev->damage.x2, rect->x2);
+		dbidev->damage.y2 = max(dbidev->damage.y2, rect->y2);
+	} else {
+		dbidev->damage = *rect;
+	}
+}
+EXPORT_SYMBOL(mipi_dbi_fb_damage);
+
 void mipi_dbi_fb_flush(struct fb_info *info)
 {
 	struct mipi_dbi_dev *dbidev = container_of(info, struct mipi_dbi_dev, info);
 
-	mipi_dbi_fb_dirty(dbidev, info);
+	if (!dbidev->damage.x2 || !dbidev->damage.y2) {
+		dbidev->damage.x1 = 0;
+		dbidev->damage.y1 = 0;
+		dbidev->damage.x2 = info->xres;
+		dbidev->damage.y2 = info->yres;
+	}
+
+	mipi_dbi_fb_dirty(dbidev, info, &dbidev->damage);
 }
 EXPORT_SYMBOL(mipi_dbi_fb_flush);
 
diff --git a/drivers/video/panel-mipi-dbi.c b/drivers/video/panel-mipi-dbi.c
index ac6f585be32c..7fada69d6f26 100644
--- a/drivers/video/panel-mipi-dbi.c
+++ b/drivers/video/panel-mipi-dbi.c
@@ -218,6 +218,7 @@ static void panel_mipi_dbi_enable(struct fb_info *info)
 static struct fb_ops panel_mipi_dbi_ops = {
 	.fb_enable = panel_mipi_dbi_enable,
 	.fb_disable = mipi_dbi_fb_disable,
+	.fb_damage = mipi_dbi_fb_damage,
 	.fb_flush = mipi_dbi_fb_flush,
 };
 
diff --git a/include/video/mipi_dbi.h b/include/video/mipi_dbi.h
index 917f7ddd597f..7bacd4a88bc7 100644
--- a/include/video/mipi_dbi.h
+++ b/include/video/mipi_dbi.h
@@ -109,6 +109,11 @@ struct mipi_dbi_dev {
 	 * @driver_private: Driver private data.
 	 */
 	void *driver_private;
+
+	/**
+	 * @damage: Damage rectangle.
+	 */
+	struct fb_rect damage;
 };
 
 static inline const char *mipi_dbi_name(struct mipi_dbi *dbi)
@@ -120,6 +125,7 @@ int mipi_dbi_spi_init(struct spi_device *spi, struct mipi_dbi *dbi,
 		      int dc);
 int mipi_dbi_dev_init(struct mipi_dbi_dev *dbidev,
 		      struct fb_ops *ops, struct fb_videomode *mode);
+void mipi_dbi_fb_damage(struct fb_info *info, struct fb_rect *rect);
 void mipi_dbi_fb_flush(struct fb_info *info);
 void mipi_dbi_enable_flush(struct mipi_dbi_dev *dbidev,
 			   struct fb_info *info);
-- 
2.39.2

Re: [PATCH 1/3] video: fb: add optional damage tracking

[Ahmad Fatoum]

On 03.04.23 14:18, Philipp Zabel wrote:
> Add an optional fb_damage operation that drivers may use to accumulate
> damage on the framebuffer until fb_flush is called. The accumulated
> damage can be used to support partial updates for displays with an
> integrated framebuffer.
> 
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> ---
>  drivers/video/fb.c | 6 ++++++
>  include/fb.h       | 9 +++++++++
>  2 files changed, 15 insertions(+)
> 
> diff --git a/drivers/video/fb.c b/drivers/video/fb.c
> index 44754065e7d9..6f412d62c434 100644
> --- a/drivers/video/fb.c
> +++ b/drivers/video/fb.c
> @@ -43,6 +43,12 @@ static int fb_close(struct cdev *cdev)
>  	return 0;
>  }
>  
> +void fb_damage(struct fb_info *info, struct fb_rect *rect)
> +{
> +	if (info->fbops->fb_damage)
> +		info->fbops->fb_damage(info, rect);
> +}
> +
>  static int fb_op_flush(struct cdev *cdev)
>  {
>  	struct fb_info *info = cdev->priv;
> diff --git a/include/fb.h b/include/fb.h
> index 15bb74b99576..4e3eb611db48 100644
> --- a/include/fb.h
> +++ b/include/fb.h
> @@ -80,6 +80,13 @@ struct fb_bitfield {
>  
>  struct fb_info;
>  
> +struct fb_rect {
> +	u32 x1;
> +	u32 y1;
> +	u32 x2;
> +	u32 y2;
> +};
> +
>  struct fb_ops {
>  	/* set color register */
>  	int (*fb_setcolreg)(unsigned regno, unsigned red, unsigned green,
> @@ -87,6 +94,7 @@ struct fb_ops {
>  	void (*fb_enable)(struct fb_info *info);
>  	void (*fb_disable)(struct fb_info *info);
>  	int (*fb_activate_var)(struct fb_info *info);
> +	void (*fb_damage)(struct fb_info *info, struct fb_rect *rect);

const struct fb_rect * ?

>  	void (*fb_flush)(struct fb_info *info);
>  };
>  
> @@ -156,6 +164,7 @@ int register_framebuffer(struct fb_info *info);
>  
>  int fb_enable(struct fb_info *info);
>  int fb_disable(struct fb_info *info);
> +void fb_damage(struct fb_info *info, struct fb_rect *rect);
>  void fb_flush(struct fb_info *info);
>  
>  #define FBIOGET_SCREENINFO	_IOR('F', 1, loff_t)

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 2/3] video: fbconsole: add optional damage tracking

[Sascha Hauer]

On Mon, Apr 03, 2023 at 02:18:43PM +0200, Philipp Zabel wrote:
> Annotate framebuffer updates with damage rectangles so drivers may
> implement partial updates for displays with an integrated framebuffer.
> 
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> ---
>  drivers/video/fbconsole.c | 32 ++++++++++++++++++++++++++++----
>  1 file changed, 28 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/video/fbconsole.c b/drivers/video/fbconsole.c
> index 070378aa2352..25d7ea685ff6 100644
> --- a/drivers/video/fbconsole.c
> +++ b/drivers/video/fbconsole.c
> @@ -18,6 +18,7 @@ enum state_t {
>  struct fbc_priv {
>  	struct console_device cdev;
>  	struct fb_info *fb;
> +	struct fb_rect rect;
>  
>  	struct screen *sc;
>  
> @@ -60,9 +61,11 @@ static int fbc_tstc(struct console_device *cdev)
>  static void cls(struct fbc_priv *priv)
>  {
>  	void *buf = gui_screen_render_buffer(priv->sc);
> +	struct fb_info *fb = priv->fb;
>  
>  	memset(buf, 0, priv->fb->line_length * priv->fb->yres);
>  	gu_screen_blit(priv->sc);
> +	fb_damage(fb, &priv->rect);
>  }
>  
>  struct rgb {
> @@ -139,13 +142,20 @@ static void drawchar(struct fbc_priv *priv, int x, int y, int c)
>  static void video_invertchar(struct fbc_priv *priv, int x, int y)
>  {
>  	void *buf;
> +	struct fb_rect rect;
> +
> +	rect.x1 = x * priv->font->width;
> +	rect.y1 = y * priv->font->height;
> +	rect.x2 = rect.x1 + priv->font->width;
> +	rect.y2 = rect.y1 + priv->font->height;
>  
>  	buf = gui_screen_render_buffer(priv->sc);
>  
> -	gu_invert_area(priv->fb, buf, x * priv->font->width, y * priv->font->height,
> +	gu_invert_area(priv->fb, buf, rect.x1, rect.y1,
>  			priv->font->width, priv->font->height);
> -	gu_screen_blit_area(priv->sc, x * priv->font->width, y * priv->font->height,
> +	gu_screen_blit_area(priv->sc, rect.x1, rect.y1,
>  			priv->font->width, priv->font->height);
> +	fb_damage(priv->fb, &rect);

Does it make sense to call fb_damage() from gu_screen_blit_area() and
gu_screen_blit() instead?

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 1/3] video: fb: add optional damage tracking

[Philipp Zabel]

On Di, 2023-04-04 at 14:24 +0200, Ahmad Fatoum wrote:
> On 03.04.23 14:18, Philipp Zabel wrote:
> > Add an optional fb_damage operation that drivers may use to accumulate
> > damage on the framebuffer until fb_flush is called. The accumulated
> > damage can be used to support partial updates for displays with an
> > integrated framebuffer.
> > 
> > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> > ---
> >  drivers/video/fb.c | 6 ++++++
> >  include/fb.h       | 9 +++++++++
> >  2 files changed, 15 insertions(+)
> > 
> > diff --git a/drivers/video/fb.c b/drivers/video/fb.c
> > index 44754065e7d9..6f412d62c434 100644
> > --- a/drivers/video/fb.c
> > +++ b/drivers/video/fb.c
> > @@ -43,6 +43,12 @@ static int fb_close(struct cdev *cdev)
> >  	return 0;
> >  }
> >  
> > 
> > +void fb_damage(struct fb_info *info, struct fb_rect *rect)
> > +{
> > +	if (info->fbops->fb_damage)
> > +		info->fbops->fb_damage(info, rect);
> > +}
> > +
> >  static int fb_op_flush(struct cdev *cdev)
> >  {
> >  	struct fb_info *info = cdev->priv;
> > diff --git a/include/fb.h b/include/fb.h
> > index 15bb74b99576..4e3eb611db48 100644
> > --- a/include/fb.h
> > +++ b/include/fb.h
> > @@ -80,6 +80,13 @@ struct fb_bitfield {
> >  
> > 
> >  struct fb_info;
> >  
> > 
> > +struct fb_rect {
> > +	u32 x1;
> > +	u32 y1;
> > +	u32 x2;
> > +	u32 y2;
> > +};
> > +
> >  struct fb_ops {
> >  	/* set color register */
> >  	int (*fb_setcolreg)(unsigned regno, unsigned red, unsigned green,
> > @@ -87,6 +94,7 @@ struct fb_ops {
> >  	void (*fb_enable)(struct fb_info *info);
> >  	void (*fb_disable)(struct fb_info *info);
> >  	int (*fb_activate_var)(struct fb_info *info);
> > +	void (*fb_damage)(struct fb_info *info, struct fb_rect *rect);
> 
> const struct fb_rect * ?

Yes, changed to const in v2.

regards
Philipp

Re: [PATCH 2/3] video: fbconsole: add optional damage tracking

[Philipp Zabel]

On Mi, 2023-04-05 at 13:13 +0200, Sascha Hauer wrote:
> On Mon, Apr 03, 2023 at 02:18:43PM +0200, Philipp Zabel wrote:
[...]
> > +	gu_screen_blit_area(priv->sc, rect.x1, rect.y1,
> >  			priv->font->width, priv->font->height);
> > +	fb_damage(priv->fb, &rect);
> 
> Does it make sense to call fb_damage() from gu_screen_blit_area() and
> gu_screen_blit() instead?

Makes sense to me, changed in v2.

regards
Philipp