[PATCH v5 00/13] Add video damage tracking
Simon Glass
sjg at chromium.org
Mon Aug 21 21:11:59 CEST 2023
Hi Alper,
On Mon, 21 Aug 2023 at 07:51, Alper Nebi Yasak <alpernebiyasak at gmail.com> wrote:
>
> This is a rebase of Alexander Graf's video damage tracking series, with
> some tests and other changes. The original cover letter is as follows:
>
> > This patch set speeds up graphics output on ARM by a factor of 60x.
> >
> > On most ARM SBCs, we keep the frame buffer in DRAM and map it as cached,
> > but need it accessible by the display controller which reads directly
> > from a later point of consistency. Hence, we flush the frame buffer to
> > DRAM on every change. The full frame buffer.
It should not, see below.
> >
> > Unfortunately, with the advent of 4k displays, we are seeing frame buffers
> > that can take a while to flush out. This was reported by Da Xue with grub,
> > which happily print 1000s of spaces on the screen to draw a menu. Every
> > printed space triggers a cache flush.
That is a bug somewhere in EFI.
> >
> > This patch set implements the easiest mitigation against this problem:
> > Damage tracking. We remember the lowest common denominator region that was
> > touched since the last video_sync() call and only flush that. The most
> > typical writer to the frame buffer is the video console, which always
> > writes rectangles of characters on the screen and syncs afterwards.
> >
> > With this patch set applied, we reduce drawing a large grub menu (with
> > serial console attached for size information) on an RK3399-ROC system
> > at 1440p from 55 seconds to less than 1 second.
> >
> > Version 2 also implements VIDEO_COPY using this mechanism, reducing its
> > overhead compared to before as well. So even x86 systems should be faster
> > with this now :).
> >
> >
> > Alternatives considered:
> >
> > 1) Lazy sync - Sandbox does this. It only calls video_sync(true) ever
> > so often. We are missing timers to do this generically.
> >
> > 2) Double buffering - We could try to identify whether anything changed
> > at all and only draw to the FB if it did. That would require
> > maintaining a second buffer that we need to scan.
> >
> > 3) Text buffer - Maintain a buffer of all text printed on the screen with
> > respective location. Don't write if the old and new character are
> > identical. This would limit applicability to text only and is an
> > optimization on top of this patch set.
> >
> > 4) Hash screen lines - Create a hash (sha256?) over every line when it
> > changes. Only flush when it does. I'm not sure if this would waste
> > more time, memory and cache than the current approach. It would make
> > full screen updates much more expensive.
5) Fix the bug mentioned above?
>
> Changes in v5:
> - Add patch "video: test: Split copy frame buffer check into a function"
> - Add patch "video: test: Support checking copy frame buffer contents"
> - Add patch "video: test: Test partial updates of hardware frame buffer"
> - Use xstart, ystart, xend, yend as names for damage region
> - Document damage struct and fields in struct video_priv comment
> - Return void from video_damage()
> - Fix undeclared priv error in video_sync()
> - Drop unused headers from video-uclass.c
> - Use IS_ENABLED() instead of CONFIG_IS_ENABLED()
> - Call video_damage() also in video_fill_part()
> - Use met->baseline instead of priv->baseline
> - Use fontdata->height/width instead of VIDEO_FONT_HEIGHT/WIDTH
> - Update console_rotate.c video_damage() calls to pass video tests
> - Remove mention about not having minimal damage for console_rotate.c
> - Add patch "video: test: Test video damage tracking via vidconsole"
> - Document new vdev field in struct efi_gop_obj comment
> - 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
> - Imply VIDEO_DAMAGE for video drivers instead of selecting it
> - Imply VIDEO_DAMAGE also for VIDEO_TIDSS
>
> v4: https://lore.kernel.org/all/20230103215004.22646-1-agraf@csgraf.de/
>
> Changes in v4:
> - Move damage clear to patch "dm: video: Add damage tracking API"
> - Simplify first damage logic
> - Remove VIDEO_DAMAGE default for ARM
> - Skip damage on EfiBltVideoToBltBuffer
> - Add patch "video: Always compile cache flushing code"
> - Add patch "video: Enable VIDEO_DAMAGE for drivers that need it"
>
> v3: https://lore.kernel.org/all/20221230195828.88134-1-agraf@csgraf.de/
>
> Changes in v3:
> - Adapt to always assume DM is used
> - Adapt to always assume DM is used
> - Make VIDEO_COPY always select VIDEO_DAMAGE
>
> v2: https://lore.kernel.org/all/20220609225921.62462-1-agraf@csgraf.de/
>
> Changes in v2:
> - Remove ifdefs
> - Fix ranges in truetype target
> - Limit rotate to necessary damage
> - Remove ifdefs from gop
> - Fix dcache range; we were flushing too much before
> - Add patch "video: Use VIDEO_DAMAGE for VIDEO_COPY"
>
> v1: https://lore.kernel.org/all/20220606234336.5021-1-agraf@csgraf.de/
>
> Alexander Graf (9):
> dm: video: Add damage tracking API
> dm: video: Add damage notification on display fills
> vidconsole: Add damage notifications to all vidconsole drivers
> video: Add damage notification on bmp display
> efi_loader: GOP: Add damage notification on BLT
> video: Only dcache flush damaged lines
> video: Use VIDEO_DAMAGE for VIDEO_COPY
> video: Always compile cache flushing code
> video: Enable VIDEO_DAMAGE for drivers that need it
>
> Alper Nebi Yasak (4):
> video: test: Split copy frame buffer check into a function
> video: test: Support checking copy frame buffer contents
> video: test: Test partial updates of hardware frame buffer
> video: test: Test video damage tracking via vidconsole
>
> arch/arm/mach-omap2/omap3/Kconfig | 1 +
> arch/arm/mach-sunxi/Kconfig | 1 +
> drivers/video/Kconfig | 26 +++
> drivers/video/console_normal.c | 27 ++--
> drivers/video/console_rotate.c | 94 +++++++----
> drivers/video/console_truetype.c | 37 +++--
> drivers/video/exynos/Kconfig | 1 +
> drivers/video/imx/Kconfig | 1 +
> drivers/video/meson/Kconfig | 1 +
> drivers/video/rockchip/Kconfig | 1 +
> drivers/video/stm32/Kconfig | 1 +
> drivers/video/tegra20/Kconfig | 1 +
> drivers/video/tidss/Kconfig | 1 +
> drivers/video/vidconsole-uclass.c | 16 --
> drivers/video/video-uclass.c | 190 ++++++++++++----------
> drivers/video/video_bmp.c | 7 +-
> include/video.h | 59 +++----
> include/video_console.h | 52 ------
> lib/efi_loader/efi_gop.c | 7 +
> test/dm/video.c | 256 ++++++++++++++++++++++++------
> 20 files changed, 483 insertions(+), 297 deletions(-)
It is good to see this tidied up into something that can be applied!
I am unsure what is going on with the EFI performance, though. It
should not flush the cache after every character, only after a new
line. Is there something wrong in here? If so, we should fix that bug
first and it should be patch 1 of this series.
Regards,
Simon
More information about the U-Boot
mailing list