Re: [PATCH v4 7/9] video: Use VIDEO_DAMAGE for VIDEO_COPY
Hi Heinrich, On Sat, 7 Jan 2023 at 16:22, Heinrich Schuchardt wrote: > > On 1/7/23 01:13, Simon Glass wrote: > > Hi Alexander, > > > > On Tue, 3 Jan 2023 at 14:50, Alexander Graf wrote: > >> > >> CONFIG_VIDEO_COPY implemented a range based copying mechanism: If we > > > > range-based > > > >> print a single character, it will always copy the full range of bytes > >> from the top left corner of the character to the lower right onto the > >> uncached frame buffer. This includes pretty much the full line contents > >> of the printed character. > >> > >> Since we now have proper damage tracking, let's make use of that to reduce > >> the amount of data we need to copy. With this patch applied, we will only > >> copy the tiny rectangle surrounding characters when we print them, > >> speeding up the video console. > >> > >> As a bonus, we remove a lot of code. > >> > >> Signed-off-by: Alexander Graf > >> > >> --- > >> > >> v2 -> v3: > >> > >>- Rebase > >>- Make CONFIG_COPY always select VIDEO_DAMAGE > >> --- > >> drivers/video/Kconfig | 5 ++ > >> drivers/video/console_normal.c| 14 + > >> drivers/video/console_rotate.c| 37 ++--- > >> drivers/video/console_truetype.c | 17 +- > >> drivers/video/vidconsole-uclass.c | 16 -- > >> drivers/video/video-uclass.c | 91 --- > >> drivers/video/video_bmp.c | 7 --- > >> include/video.h | 37 - > >> include/video_console.h | 49 - > >> 9 files changed, 37 insertions(+), 236 deletions(-) > >> > > > > This feature needs some tests in test/dm/video.c > > > > For sandbox, I think you will need to allow it to be enabled / > > disabled at runtime, so the some tests can use it and some not? > > It should be good enough to enable the feature in one of the sandbox > defconfigs and disable it in another. Yes that would work, e.g. enable in sandbox but not sandbox_flattree Regards, Simon
Re: [PATCH v4 7/9] video: Use VIDEO_DAMAGE for VIDEO_COPY
On 1/7/23 01:13, Simon Glass wrote: Hi Alexander, On Tue, 3 Jan 2023 at 14:50, Alexander Graf wrote: CONFIG_VIDEO_COPY implemented a range based copying mechanism: If we range-based print a single character, it will always copy the full range of bytes from the top left corner of the character to the lower right onto the uncached frame buffer. This includes pretty much the full line contents of the printed character. Since we now have proper damage tracking, let's make use of that to reduce the amount of data we need to copy. With this patch applied, we will only copy the tiny rectangle surrounding characters when we print them, speeding up the video console. As a bonus, we remove a lot of code. Signed-off-by: Alexander Graf --- v2 -> v3: - Rebase - Make CONFIG_COPY always select VIDEO_DAMAGE --- drivers/video/Kconfig | 5 ++ drivers/video/console_normal.c| 14 + drivers/video/console_rotate.c| 37 ++--- drivers/video/console_truetype.c | 17 +- drivers/video/vidconsole-uclass.c | 16 -- drivers/video/video-uclass.c | 91 --- drivers/video/video_bmp.c | 7 --- include/video.h | 37 - include/video_console.h | 49 - 9 files changed, 37 insertions(+), 236 deletions(-) This feature needs some tests in test/dm/video.c For sandbox, I think you will need to allow it to be enabled / disabled at runtime, so the some tests can use it and some not? It should be good enough to enable the feature in one of the sandbox defconfigs and disable it in another. Best regards Heinrich Regards, Simon
Re: [PATCH v4 7/9] video: Use VIDEO_DAMAGE for VIDEO_COPY
Hi Alexander, On Tue, 3 Jan 2023 at 14:50, Alexander Graf wrote: > > CONFIG_VIDEO_COPY implemented a range based copying mechanism: If we range-based > print a single character, it will always copy the full range of bytes > from the top left corner of the character to the lower right onto the > uncached frame buffer. This includes pretty much the full line contents > of the printed character. > > Since we now have proper damage tracking, let's make use of that to reduce > the amount of data we need to copy. With this patch applied, we will only > copy the tiny rectangle surrounding characters when we print them, > speeding up the video console. > > As a bonus, we remove a lot of code. > > Signed-off-by: Alexander Graf > > --- > > v2 -> v3: > > - Rebase > - Make CONFIG_COPY always select VIDEO_DAMAGE > --- > drivers/video/Kconfig | 5 ++ > drivers/video/console_normal.c| 14 + > drivers/video/console_rotate.c| 37 ++--- > drivers/video/console_truetype.c | 17 +- > drivers/video/vidconsole-uclass.c | 16 -- > drivers/video/video-uclass.c | 91 --- > drivers/video/video_bmp.c | 7 --- > include/video.h | 37 - > include/video_console.h | 49 - > 9 files changed, 37 insertions(+), 236 deletions(-) > This feature needs some tests in test/dm/video.c For sandbox, I think you will need to allow it to be enabled / disabled at runtime, so the some tests can use it and some not? Regards, Simon
[PATCH v4 7/9] video: Use VIDEO_DAMAGE for VIDEO_COPY
CONFIG_VIDEO_COPY implemented a range based copying mechanism: If we print a single character, it will always copy the full range of bytes from the top left corner of the character to the lower right onto the uncached frame buffer. This includes pretty much the full line contents of the printed character. Since we now have proper damage tracking, let's make use of that to reduce the amount of data we need to copy. With this patch applied, we will only copy the tiny rectangle surrounding characters when we print them, speeding up the video console. As a bonus, we remove a lot of code. Signed-off-by: Alexander Graf --- v2 -> v3: - Rebase - Make CONFIG_COPY always select VIDEO_DAMAGE --- drivers/video/Kconfig | 5 ++ drivers/video/console_normal.c| 14 + drivers/video/console_rotate.c| 37 ++--- drivers/video/console_truetype.c | 17 +- drivers/video/vidconsole-uclass.c | 16 -- drivers/video/video-uclass.c | 91 --- drivers/video/video_bmp.c | 7 --- include/video.h | 37 - include/video_console.h | 49 - 9 files changed, 37 insertions(+), 236 deletions(-) diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig index 826a1a6587..472e9c7c61 100644 --- a/drivers/video/Kconfig +++ b/drivers/video/Kconfig @@ -53,11 +53,14 @@ config VIDEO_PCI_DEFAULT_FB_SIZE config VIDEO_COPY bool "Enable copying the frame buffer to a hardware copy" + select VIDEO_DAMAGE help On some machines (e.g. x86), reading from the frame buffer is very slow because it is uncached. To improve performance, this feature allows the frame buffer to be kept in cached memory (allocated by U-Boot) and then copied to the hardware frame-buffer as needed. + It uses the VIDEO_DAMAGE feature to keep track of regions to copy + and will only copy actually touched regions. To use this, your video driver must set @copy_base in struct video_uc_plat. @@ -75,6 +78,8 @@ config VIDEO_DAMAGE regions of the frame buffer that were modified before, speeding up screen refreshes significantly. + It is also used by VIDEO_COPY to identify which regions changed. + config BACKLIGHT_PWM bool "Generic PWM based Backlight Driver" depends on BACKLIGHT && DM_PWM diff --git a/drivers/video/console_normal.c b/drivers/video/console_normal.c index 5b5586fd3e..625d14516f 100644 --- a/drivers/video/console_normal.c +++ b/drivers/video/console_normal.c @@ -18,7 +18,6 @@ static int console_normal_set_row(struct udevice *dev, uint row, int clr) struct video_priv *vid_priv = dev_get_uclass_priv(dev->parent); void *line, *end; int pixels = VIDEO_FONT_HEIGHT * vid_priv->xsize; - int ret; int i; line = vid_priv->fb + row * VIDEO_FONT_HEIGHT * vid_priv->line_length; @@ -53,9 +52,6 @@ static int console_normal_set_row(struct udevice *dev, uint row, int clr) default: return -ENOSYS; } - ret = vidconsole_sync_copy(dev, line, end); - if (ret) - return ret; video_damage(dev->parent, 0, VIDEO_FONT_HEIGHT * row, vid_priv->xsize, VIDEO_FONT_HEIGHT); @@ -70,14 +66,11 @@ static int console_normal_move_rows(struct udevice *dev, uint rowdst, void *dst; void *src; int size; - int ret; dst = vid_priv->fb + rowdst * VIDEO_FONT_HEIGHT * vid_priv->line_length; src = vid_priv->fb + rowsrc * VIDEO_FONT_HEIGHT * vid_priv->line_length; size = VIDEO_FONT_HEIGHT * vid_priv->line_length * count; - ret = vidconsole_memmove(dev, dst, src, size); - if (ret) - return ret; + memmove(dst, src, size); video_damage(dev->parent, 0, VIDEO_FONT_HEIGHT * rowdst, vid_priv->xsize, VIDEO_FONT_HEIGHT * count); @@ -94,7 +87,6 @@ static int console_normal_putc_xy(struct udevice *dev, uint x_frac, uint y, int i, row; void *start; void *line; - int ret; start = vid_priv->fb + y * vid_priv->line_length + VID_TO_PIXEL(x_frac) * VNBYTES(vid_priv->bpix); @@ -153,10 +145,6 @@ static int console_normal_putc_xy(struct udevice *dev, uint x_frac, uint y, video_damage(dev->parent, VID_TO_PIXEL(x_frac), y, VIDEO_FONT_WIDTH, VIDEO_FONT_HEIGHT); - ret = vidconsole_sync_copy(dev, start, line); - if (ret) - return ret; - return VID_TO_POS(VIDEO_FONT_WIDTH); } diff --git a/drivers/video/console_rotate.c b/drivers/video/console_rotate.c index 56e20bb4f3..9b179a45b6 100644 --- a/drivers/video/console_rotate.c +++ b/drivers/video/console_rotate.c @@ -53,9 +53,6 @@ static int console_set_row_1(struct udevice *dev, uint row, int clr) } line
