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

[Heinrich Schuchardt]

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 at csgraf.de>
> Reported-by: Da Xue <da at 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
>    *