diff mbox series

[v3,1/7] dm: video: Add damage tracking API

Message ID 20221230195828.88134-2-agraf@csgraf.de
State Superseded
Delegated to: Anatolij Gustschin
Headers show
Series Add video damage tracking | expand

Commit Message

Alexander Graf Dec. 30, 2022, 7:58 p.m. UTC
We are going to introduce image damage tracking to fasten up screen
refresh on large displays. This patch adds damage tracking for up to
one rectangle of the screen which is typically enough to hold blt or
text print updates. Callers into this API and a reduced dcache flush
code path will follow in later patches.

Signed-off-by: Alexander Graf <agraf@csgraf.de>
Reported-by: Da Xue <da@libre.computer>

---

v1 -> v2:

  - Remove ifdefs

v2 -> v3:

  - Adapt Kconfig to DM always
---
 drivers/video/Kconfig        | 14 ++++++++++++
 drivers/video/video-uclass.c | 41 ++++++++++++++++++++++++++++++++++++
 include/video.h              | 29 +++++++++++++++++++++++--
 3 files changed, 82 insertions(+), 2 deletions(-)

Comments

Heinrich Schuchardt Dec. 30, 2022, 8:49 p.m. UTC | #1
On 12/30/22 20:58, Alexander Graf wrote:
> We are going to introduce image damage tracking to fasten up screen
> refresh on large displays. This patch adds damage tracking for up to
> one rectangle of the screen which is typically enough to hold blt or
> text print updates. Callers into this API and a reduced dcache flush
> code path will follow in later patches.
>
> Signed-off-by: Alexander Graf <agraf@csgraf.de>
> Reported-by: Da Xue <da@libre.computer>
>
> ---
>
> v1 -> v2:
>
>    - Remove ifdefs
>
> v2 -> v3:
>
>    - Adapt Kconfig to DM always
> ---
>   drivers/video/Kconfig        | 14 ++++++++++++
>   drivers/video/video-uclass.c | 41 ++++++++++++++++++++++++++++++++++++
>   include/video.h              | 29 +++++++++++++++++++++++--
>   3 files changed, 82 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> index f539977d9b..e12457c654 100644
> --- a/drivers/video/Kconfig
> +++ b/drivers/video/Kconfig
> @@ -62,6 +62,20 @@ config VIDEO_COPY
>   	  To use this, your video driver must set @copy_base in
>   	  struct video_uc_plat.
>
> +config VIDEO_DAMAGE
> +	bool "Enable damage tracking of frame buffer regions"
> +	default y if ARM && !SYS_DCACHE_OFF
> +	help
> +	  On some machines (most ARM), the display frame buffer resides in
> +	  RAM. To make the display controller pick up screen updates, we
> +	  have to flush frame buffer contents from CPU caches into RAM which
> +	  can be a slow operation.
> +
> +	  This feature adds damage tracking to collect information about regions
> +	  that received updates. When we want to sync, we then only flush
> +	  regions of the frame buffer that were modified before, speeding up
> +	  screen refreshes significantly.
> +
>   config BACKLIGHT_PWM
>   	bool "Generic PWM based Backlight Driver"
>   	depends on BACKLIGHT && DM_PWM
> diff --git a/drivers/video/video-uclass.c b/drivers/video/video-uclass.c
> index 0ce376ca3f..48a053841e 100644
> --- a/drivers/video/video-uclass.c
> +++ b/drivers/video/video-uclass.c
> @@ -21,6 +21,8 @@
>   #include <dm/device_compat.h>
>   #include <dm/device-internal.h>
>   #include <dm/uclass-internal.h>
> +#include <linux/types.h>
> +#include <linux/bitmap.h>
>   #ifdef CONFIG_SANDBOX
>   #include <asm/sdl.h>
>   #endif
> @@ -254,6 +256,45 @@ void video_set_default_colors(struct udevice *dev, bool invert)
>   	priv->colour_bg = video_index_to_colour(priv, back);
>   }
>
> +/* Notify about changes in the frame buffer */
> +int video_damage(struct udevice *vid, int x, int y, int width, int height)
> +{
> +	struct video_priv *priv = dev_get_uclass_priv(vid);
> +	int endx = x + width;
> +	int endy = y + height;
> +
> +	if (!CONFIG_IS_ENABLED(VIDEO_DAMAGE))
> +		return 0;
> +
> +	if (x > priv->xsize)
> +		return 0;
> +
> +	if (y > priv->ysize)
> +		return 0;
> +
> +	if (endx > priv->xsize)
> +		endx = priv->xsize;
> +
> +	if (endy > priv->ysize)
> +		endy = priv->ysize;
> +
> +	if (priv->damage.endx && priv->damage.endy) {

This if is superfluous if you initialize x = priv->xsize, y =
priv->ysize in video_flush_dcache().

Please, simplify the code.

Best regards

Heinrich

> +		/* Span a rectangle across all old and new damage */
> +		priv->damage.x = min(x, priv->damage.x);
> +		priv->damage.y = min(y, priv->damage.y);
> +		priv->damage.endx = max(endx, priv->damage.endx);
> +		priv->damage.endy = max(endy, priv->damage.endy);
> +	} else {
> +		/* First damage, setting the rectangle to span it */
> +		priv->damage.x = x;
> +		priv->damage.y = y;
> +		priv->damage.endx = endx;
> +		priv->damage.endy = endy;
> +	}
> +
> +	return 0;
> +}
> +
>   /* Flush video activity to the caches */
>   int video_sync(struct udevice *vid, bool force)
>   {
> diff --git a/include/video.h b/include/video.h
> index 43f2e2c02f..4b35e97f79 100644
> --- a/include/video.h
> +++ b/include/video.h
> @@ -109,6 +109,12 @@ struct video_priv {
>   	void *fb;
>   	int fb_size;
>   	void *copy_fb;
> +	struct {
> +		int x;
> +		int y;
> +		int endx;
> +		int endy;
> +	} damage;
>   	int line_length;
>   	u32 colour_fg;
>   	u32 colour_bg;
> @@ -211,8 +217,9 @@ int video_fill(struct udevice *dev, u32 colour);
>    * @return: 0 on success, error code otherwise
>    *
>    * Some frame buffers are cached or have a secondary frame buffer. This
> - * function syncs these up so that the current contents of the U-Boot frame
> - * buffer are displayed to the user.
> + * function syncs the damaged parts of them up so that the current contents
> + * of the U-Boot frame buffer are displayed to the user. It clears the damage
> + * buffer.
>    */
>   int video_sync(struct udevice *vid, bool force);
>
> @@ -332,6 +339,24 @@ static inline int video_sync_copy_all(struct udevice *dev)
>
>   #endif
>
> +/**
> + * video_damage() - Notify the video subsystem about screen updates.
> + *
> + * @vid:	Device to sync
> + * @x:	        Upper left X coordinate of the damaged rectangle
> + * @y:	        Upper left Y coordinate of the damaged rectangle
> + * @width:	Width of the damaged rectangle
> + * @height:	Height of the damaged rectangle
> + *
> + * @return: 0
> + *
> + * Some frame buffers are cached or have a secondary frame buffer. This
> + * function notifies the video subsystem about rectangles that were updated
> + * within the frame buffer. They may only get written to the screen on the
> + * next call to video_sync().
> + */
> +int video_damage(struct udevice *vid, int x, int y, int width, int height);
> +
>   /**
>    * video_is_active() - Test if one video device it active
>    *
Heinrich Schuchardt Dec. 30, 2022, 9:20 p.m. UTC | #2
On 12/30/22 20:58, Alexander Graf wrote:
> We are going to introduce image damage tracking to fasten up screen
> refresh on large displays. This patch adds damage tracking for up to
> one rectangle of the screen which is typically enough to hold blt or
> text print updates. Callers into this API and a reduced dcache flush
> code path will follow in later patches.
>
> Signed-off-by: Alexander Graf <agraf@csgraf.de>
> Reported-by: Da Xue <da@libre.computer>
>
> ---
>
> v1 -> v2:
>
>    - Remove ifdefs
>
> v2 -> v3:
>
>    - Adapt Kconfig to DM always
> ---
>   drivers/video/Kconfig        | 14 ++++++++++++
>   drivers/video/video-uclass.c | 41 ++++++++++++++++++++++++++++++++++++
>   include/video.h              | 29 +++++++++++++++++++++++--
>   3 files changed, 82 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> index f539977d9b..e12457c654 100644
> --- a/drivers/video/Kconfig
> +++ b/drivers/video/Kconfig
> @@ -62,6 +62,20 @@ config VIDEO_COPY
>   	  To use this, your video driver must set @copy_base in
>   	  struct video_uc_plat.
>
> +config VIDEO_DAMAGE
> +	bool "Enable damage tracking of frame buffer regions"
> +	default y if ARM && !SYS_DCACHE_OFF

If CONFIG_VIDEO_DAMAGE make sense or not, does not depend on the
architecture but on caching of the video buffer being enabled.

default=y should only be set for those video drivers calling
video_set_flush_dcache(dev, true).

Best regards

Heinrich

> +	help
> +	  On some machines (most ARM), the display frame buffer resides in
> +	  RAM. To make the display controller pick up screen updates, we
> +	  have to flush frame buffer contents from CPU caches into RAM which
> +	  can be a slow operation.
> +
> +	  This feature adds damage tracking to collect information about regions
> +	  that received updates. When we want to sync, we then only flush
> +	  regions of the frame buffer that were modified before, speeding up
> +	  screen refreshes significantly.
> +
>   config BACKLIGHT_PWM
>   	bool "Generic PWM based Backlight Driver"
>   	depends on BACKLIGHT && DM_PWM
> diff --git a/drivers/video/video-uclass.c b/drivers/video/video-uclass.c
> index 0ce376ca3f..48a053841e 100644
> --- a/drivers/video/video-uclass.c
> +++ b/drivers/video/video-uclass.c
> @@ -21,6 +21,8 @@
>   #include <dm/device_compat.h>
>   #include <dm/device-internal.h>
>   #include <dm/uclass-internal.h>
> +#include <linux/types.h>
> +#include <linux/bitmap.h>
>   #ifdef CONFIG_SANDBOX
>   #include <asm/sdl.h>
>   #endif
> @@ -254,6 +256,45 @@ void video_set_default_colors(struct udevice *dev, bool invert)
>   	priv->colour_bg = video_index_to_colour(priv, back);
>   }
>
> +/* Notify about changes in the frame buffer */
> +int video_damage(struct udevice *vid, int x, int y, int width, int height)
> +{
> +	struct video_priv *priv = dev_get_uclass_priv(vid);
> +	int endx = x + width;
> +	int endy = y + height;
> +
> +	if (!CONFIG_IS_ENABLED(VIDEO_DAMAGE))
> +		return 0;
> +
> +	if (x > priv->xsize)
> +		return 0;
> +
> +	if (y > priv->ysize)
> +		return 0;
> +
> +	if (endx > priv->xsize)
> +		endx = priv->xsize;
> +
> +	if (endy > priv->ysize)
> +		endy = priv->ysize;
> +
> +	if (priv->damage.endx && priv->damage.endy) {
> +		/* Span a rectangle across all old and new damage */
> +		priv->damage.x = min(x, priv->damage.x);
> +		priv->damage.y = min(y, priv->damage.y);
> +		priv->damage.endx = max(endx, priv->damage.endx);
> +		priv->damage.endy = max(endy, priv->damage.endy);
> +	} else {
> +		/* First damage, setting the rectangle to span it */
> +		priv->damage.x = x;
> +		priv->damage.y = y;
> +		priv->damage.endx = endx;
> +		priv->damage.endy = endy;
> +	}
> +
> +	return 0;
> +}
> +
>   /* Flush video activity to the caches */
>   int video_sync(struct udevice *vid, bool force)
>   {
> diff --git a/include/video.h b/include/video.h
> index 43f2e2c02f..4b35e97f79 100644
> --- a/include/video.h
> +++ b/include/video.h
> @@ -109,6 +109,12 @@ struct video_priv {
>   	void *fb;
>   	int fb_size;
>   	void *copy_fb;
> +	struct {
> +		int x;
> +		int y;
> +		int endx;
> +		int endy;
> +	} damage;
>   	int line_length;
>   	u32 colour_fg;
>   	u32 colour_bg;
> @@ -211,8 +217,9 @@ int video_fill(struct udevice *dev, u32 colour);
>    * @return: 0 on success, error code otherwise
>    *
>    * Some frame buffers are cached or have a secondary frame buffer. This
> - * function syncs these up so that the current contents of the U-Boot frame
> - * buffer are displayed to the user.
> + * function syncs the damaged parts of them up so that the current contents
> + * of the U-Boot frame buffer are displayed to the user. It clears the damage
> + * buffer.
>    */
>   int video_sync(struct udevice *vid, bool force);
>
> @@ -332,6 +339,24 @@ static inline int video_sync_copy_all(struct udevice *dev)
>
>   #endif
>
> +/**
> + * video_damage() - Notify the video subsystem about screen updates.
> + *
> + * @vid:	Device to sync
> + * @x:	        Upper left X coordinate of the damaged rectangle
> + * @y:	        Upper left Y coordinate of the damaged rectangle
> + * @width:	Width of the damaged rectangle
> + * @height:	Height of the damaged rectangle
> + *
> + * @return: 0
> + *
> + * Some frame buffers are cached or have a secondary frame buffer. This
> + * function notifies the video subsystem about rectangles that were updated
> + * within the frame buffer. They may only get written to the screen on the
> + * next call to video_sync().
> + */
> +int video_damage(struct udevice *vid, int x, int y, int width, int height);
> +
>   /**
>    * video_is_active() - Test if one video device it active
>    *
diff mbox series

Patch

diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index f539977d9b..e12457c654 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -62,6 +62,20 @@  config VIDEO_COPY
 	  To use this, your video driver must set @copy_base in
 	  struct video_uc_plat.
 
+config VIDEO_DAMAGE
+	bool "Enable damage tracking of frame buffer regions"
+	default y if ARM && !SYS_DCACHE_OFF
+	help
+	  On some machines (most ARM), the display frame buffer resides in
+	  RAM. To make the display controller pick up screen updates, we
+	  have to flush frame buffer contents from CPU caches into RAM which
+	  can be a slow operation.
+
+	  This feature adds damage tracking to collect information about regions
+	  that received updates. When we want to sync, we then only flush
+	  regions of the frame buffer that were modified before, speeding up
+	  screen refreshes significantly.
+
 config BACKLIGHT_PWM
 	bool "Generic PWM based Backlight Driver"
 	depends on BACKLIGHT && DM_PWM
diff --git a/drivers/video/video-uclass.c b/drivers/video/video-uclass.c
index 0ce376ca3f..48a053841e 100644
--- a/drivers/video/video-uclass.c
+++ b/drivers/video/video-uclass.c
@@ -21,6 +21,8 @@ 
 #include <dm/device_compat.h>
 #include <dm/device-internal.h>
 #include <dm/uclass-internal.h>
+#include <linux/types.h>
+#include <linux/bitmap.h>
 #ifdef CONFIG_SANDBOX
 #include <asm/sdl.h>
 #endif
@@ -254,6 +256,45 @@  void video_set_default_colors(struct udevice *dev, bool invert)
 	priv->colour_bg = video_index_to_colour(priv, back);
 }
 
+/* Notify about changes in the frame buffer */
+int video_damage(struct udevice *vid, int x, int y, int width, int height)
+{
+	struct video_priv *priv = dev_get_uclass_priv(vid);
+	int endx = x + width;
+	int endy = y + height;
+
+	if (!CONFIG_IS_ENABLED(VIDEO_DAMAGE))
+		return 0;
+
+	if (x > priv->xsize)
+		return 0;
+
+	if (y > priv->ysize)
+		return 0;
+
+	if (endx > priv->xsize)
+		endx = priv->xsize;
+
+	if (endy > priv->ysize)
+		endy = priv->ysize;
+
+	if (priv->damage.endx && priv->damage.endy) {
+		/* Span a rectangle across all old and new damage */
+		priv->damage.x = min(x, priv->damage.x);
+		priv->damage.y = min(y, priv->damage.y);
+		priv->damage.endx = max(endx, priv->damage.endx);
+		priv->damage.endy = max(endy, priv->damage.endy);
+	} else {
+		/* First damage, setting the rectangle to span it */
+		priv->damage.x = x;
+		priv->damage.y = y;
+		priv->damage.endx = endx;
+		priv->damage.endy = endy;
+	}
+
+	return 0;
+}
+
 /* Flush video activity to the caches */
 int video_sync(struct udevice *vid, bool force)
 {
diff --git a/include/video.h b/include/video.h
index 43f2e2c02f..4b35e97f79 100644
--- a/include/video.h
+++ b/include/video.h
@@ -109,6 +109,12 @@  struct video_priv {
 	void *fb;
 	int fb_size;
 	void *copy_fb;
+	struct {
+		int x;
+		int y;
+		int endx;
+		int endy;
+	} damage;
 	int line_length;
 	u32 colour_fg;
 	u32 colour_bg;
@@ -211,8 +217,9 @@  int video_fill(struct udevice *dev, u32 colour);
  * @return: 0 on success, error code otherwise
  *
  * Some frame buffers are cached or have a secondary frame buffer. This
- * function syncs these up so that the current contents of the U-Boot frame
- * buffer are displayed to the user.
+ * function syncs the damaged parts of them up so that the current contents
+ * of the U-Boot frame buffer are displayed to the user. It clears the damage
+ * buffer.
  */
 int video_sync(struct udevice *vid, bool force);
 
@@ -332,6 +339,24 @@  static inline int video_sync_copy_all(struct udevice *dev)
 
 #endif
 
+/**
+ * video_damage() - Notify the video subsystem about screen updates.
+ *
+ * @vid:	Device to sync
+ * @x:	        Upper left X coordinate of the damaged rectangle
+ * @y:	        Upper left Y coordinate of the damaged rectangle
+ * @width:	Width of the damaged rectangle
+ * @height:	Height of the damaged rectangle
+ *
+ * @return: 0
+ *
+ * Some frame buffers are cached or have a secondary frame buffer. This
+ * function notifies the video subsystem about rectangles that were updated
+ * within the frame buffer. They may only get written to the screen on the
+ * next call to video_sync().
+ */
+int video_damage(struct udevice *vid, int x, int y, int width, int height);
+
 /**
  * video_is_active() - Test if one video device it active
  *