Hi Alper, On Mon, 21 Aug 2023 at 07:51, Alper Nebi Yasak <alpernebiya...@gmail.com> wrote: > > From: Alexander Graf <ag...@csgraf.de> > > 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.
I suppose for rotated consoles it copies whole lines, but otherwise it does a lot of small copies? > > After this, changes to the main frame buffer are not immediately copied > to the copy frame buffer, but postponed until the next video device > sync. So issue an explicit sync before inspecting the copy frame buffer > contents for the video tests. So how does the sync get done in this case? > > Signed-off-by: Alexander Graf <ag...@csgraf.de> > [Alper: Rebase for fontdata->height/w, fill_part(), fix memmove(dev), > drop from defconfig, use damage.xstart/yend, use IS_ENABLED(), > call video_sync() before copy_fb check, update video_copy test] > Co-developed-by: Alper Nebi Yasak <alpernebiya...@gmail.com> > Signed-off-by: Alper Nebi Yasak <alpernebiya...@gmail.com> > --- > > Changes in v5: > - Remove video_sync_copy() also from video_fill(), video_fill_part() > - Fix memmove() calls by removing the extra dev argument > - Call video_sync() before checking copy_fb in video tests > - Use xstart, ystart, xend, yend as names for damage region > - Use met->baseline instead of priv->baseline > - Use fontdata->height/width instead of VIDEO_FONT_HEIGHT/WIDTH > - Use xstart, ystart, xend, yend as names for damage region > - Use IS_ENABLED() instead of CONFIG_IS_ENABLED() > - Drop VIDEO_DAMAGE from sandbox defconfig added in a new patch > - Update dm_test_video_copy test added in a new patch > > Changes in v3: > - Make VIDEO_COPY always select VIDEO_DAMAGE > > Changes in v2: > - Add patch "video: Use VIDEO_DAMAGE for VIDEO_COPY" > > configs/sandbox_defconfig | 1 - > drivers/video/Kconfig | 5 ++ > drivers/video/console_normal.c | 13 +---- > drivers/video/console_rotate.c | 44 +++----------- > drivers/video/console_truetype.c | 16 +---- > drivers/video/vidconsole-uclass.c | 16 ----- > drivers/video/video-uclass.c | 97 ++++++++----------------------- > drivers/video/video_bmp.c | 7 --- > include/video.h | 37 ------------ > include/video_console.h | 52 ----------------- > test/dm/video.c | 3 +- > 11 files changed, 43 insertions(+), 248 deletions(-) > > diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig > index 51b820f13121..259f31f26cee 100644 > --- a/configs/sandbox_defconfig > +++ b/configs/sandbox_defconfig > @@ -307,7 +307,6 @@ CONFIG_USB_ETH_CDC=y > CONFIG_VIDEO=y > CONFIG_VIDEO_FONT_SUN12X22=y > CONFIG_VIDEO_COPY=y > -CONFIG_VIDEO_DAMAGE=y > CONFIG_CONSOLE_ROTATION=y > CONFIG_CONSOLE_TRUETYPE=y > CONFIG_CONSOLE_TRUETYPE_CANTORAONE=y > diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig > index 97f494a1340b..b3fbd9d7d9ca 100644 > --- a/drivers/video/Kconfig > +++ b/drivers/video/Kconfig > @@ -83,11 +83,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. > @@ -105,6 +108,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 a19ce6a2bc11..c44aa09473a3 100644 > --- a/drivers/video/console_normal.c > +++ b/drivers/video/console_normal.c > @@ -35,10 +35,6 @@ static int console_set_row(struct udevice *dev, uint row, > int clr) > fill_pixel_and_goto_next(&dst, clr, pbytes, pbytes); > end = dst; > > - ret = vidconsole_sync_copy(dev, line, end); > - if (ret) > - return ret; > - > video_damage(dev->parent, > 0, > fontdata->height * row, > @@ -57,14 +53,11 @@ static int console_move_rows(struct udevice *dev, uint > rowdst, > void *dst; > void *src; > int size; > - int ret; > > dst = vid_priv->fb + rowdst * fontdata->height * > vid_priv->line_length; > src = vid_priv->fb + rowsrc * fontdata->height * > vid_priv->line_length; > size = fontdata->height * vid_priv->line_length * count; > - ret = vidconsole_memmove(dev, dst, src, size); > - if (ret) > - return ret; > + memmove(dst, src, size); Why are you making that change? Regards, Simon