[PATCH 00/22] video: Enhancements to support a pointer
From: Simon Glass <sjg@chromium.org> The video subsystem has a video_sync() function which does quite a few things and is rather hard to predict and control: - it may or may not actually sync, depending on a timer, etc - it may be called when U-Boot is idle, so any time delays can cause a sync These two problems make it hard to test. This series introduces a deterministic video_manual_sync() which does exactly what it is told, using a set of flags: VIDSYNC_FORCE - the force flag was provided (info for the driver) VIDSYNC_FLUSH - framebuffer changes should be flushed to hardware VIDSYNC_COPY - the copy framebuffer (if any) should be updated The video_sync() method is renamed to sync() and is passed the flags, so that drivers can find out what to do. This allows the sandbox-specific code in video_sync() to move to the driver. These features will (later) be used by expo to provide a better user experience, e.g. to sync only part of the display, or to sync quickly when there is mouse movement. The pointer also needs to be drawn with transparency which is not well supported by the BMP code. This series adds support for a simple transparency colour for now, although an alpha channel may be appropriate in the future. Simon Glass (22): doc: sandbox: Update command-line options documentation sandbox: Add -V flag for video test delay sandbox: Add --video_frames option to capture test frames sandbox: Add a comment for sandbox_sdl_get_mouse_event() video: Add a missing comment in struct video_uc_priv video: Add a pointer image video: Check drawing the mouse pointer video: Move setting of cte in write_pix8() to the top video: Support transparency in the inner bmp functions video: Use variables for each colour component video: Support transparency with 24bpp BMP image video: Support transparency with BMP palette video: Export a function to draw a BMP with transparency video: Add a struct video_bbox for damage tracking video: Rename video_sync() to sync() in struct video_ops video: Tidy up the comment for sync() video: Provide a way for expo to control video sync video: Add flags parameter to sync() operation video: Refactor video_sync() to use video_manual_sync() video: sandbox: Optimise drawing based on damage video: sandbox: Add sync() method for video video: sandbox: Add test for sync() damage rectangle arch/sandbox/cpu/sdl.c | 49 +++- arch/sandbox/cpu/start.c | 32 +++ arch/sandbox/include/asm/sdl.h | 16 +- arch/sandbox/include/asm/state.h | 3 + doc/arch/sandbox/sandbox.rst | 21 +- drivers/video/efi.c | 4 +- drivers/video/images/Makefile | 3 + drivers/video/images/riscos_arrow.bmp | Bin 0 -> 3798 bytes drivers/video/mcde_simple.c | 4 +- drivers/video/sandbox_sdl.c | 36 +++ drivers/video/seps525.c | 4 +- drivers/video/video-uclass.c | 110 ++++++-- drivers/video/video_bmp.c | 121 ++++++--- include/dm/test.h | 15 + include/video.h | 88 ++++-- include/video_defs.h | 37 +++ test/dm/video.c | 376 ++++++++++++++++++++++++-- 17 files changed, 802 insertions(+), 117 deletions(-) create mode 100644 drivers/video/images/riscos_arrow.bmp -- 2.43.0 base-commit: a598eddb938d1b62bba672aacda440c80352fcb7 branch: proc
From: Simon Glass <sjg@chromium.org> Some recent flags don't have documentation included. Before adding more flags, fix these: -A, --no_term_present: For pager testing -k, --select_unittests: Select specific unit tests -P, --pager_bypass: Enable pager-bypass mode -W, --title: Set window title Also fix the documentation for --double_lcd which uses -K not -k Co-developed-by: Claude <noreply@anthropic.com> Signed-off-by: Simon Glass <sjg@chromium.org> --- doc/arch/sandbox/sandbox.rst | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/doc/arch/sandbox/sandbox.rst b/doc/arch/sandbox/sandbox.rst index 89255cb14dd..31a670ce2ab 100644 --- a/doc/arch/sandbox/sandbox.rst +++ b/doc/arch/sandbox/sandbox.rst @@ -103,6 +103,9 @@ available options. Some of these are described below: even if CONFIG_AUTOBOOT_KEYED is enabled, since it interfers with tests and normal usage +-A, --no_term_present + Assume no terminal is present. This is used for pager testing. + -b. boot The distro boot feature doesn't run by default on sandbox, since it normally not vert useful. For the distro_bootcmds to succeed, quite a bit of setup is @@ -162,7 +165,10 @@ available options. Some of these are described below: writes an elf file containing the extracted portion, then execs it. This argument provides the filename, so it can be removed before U-Boot exits. --k, --double_lcd +-k, --select_unittests <arg> + Select specific unit tests to run. This is only used with SPL. + +-K, --double_lcd Doubles the size of the emulated LCD, so that it appears bigger. This can be useful on large or high-resolution displays. @@ -199,6 +205,9 @@ available options. Some of these are described below: each program is extracted from the original image and executed (see -j), this is the only way that subsequent phases can locate the full image. +-P, --pager_bypass + Enable pager bypass mode for testing. + -r, --read Read driver state from a dtb file. In conjunction with `-w`, this allows sandbox to save and restore emulated hardware state (such as a TPM) across @@ -248,6 +257,9 @@ available options. Some of these are described below: sandbox to save and restore emulated hardware state (such as a TPM) across each U-Boot phase. +-W, --title <title> + Set the window title for the sandbox display. + Environment Variables --------------------- -- 2.43.0
From: Simon Glass <sjg@chromium.org> Add a new -V flag to sandbox which accepts a delay in milliseconds. This causes video tests to pause after each assertion, allowing the display output to be visually inspected. This is useful for debugging video tests and understanding what is being drawn at each step. Co-developed-by: Claude <noreply@anthropic.com> Signed-off-by: Simon Glass <sjg@chromium.org> --- arch/sandbox/cpu/start.c | 10 ++++++++++ arch/sandbox/include/asm/state.h | 1 + doc/arch/sandbox/sandbox.rst | 4 ++++ test/dm/video.c | 7 +++++-- 4 files changed, 20 insertions(+), 2 deletions(-) diff --git a/arch/sandbox/cpu/start.c b/arch/sandbox/cpu/start.c index 43adc4cdb4f..559c11a2dbb 100644 --- a/arch/sandbox/cpu/start.c +++ b/arch/sandbox/cpu/start.c @@ -365,6 +365,16 @@ static int sandbox_cmdline_cb_double_lcd(struct sandbox_state *state, SANDBOX_CMDLINE_OPT_SHORT(double_lcd, 'K', 0, "Double the LCD display size in each direction"); +static int sandbox_cmdline_cb_video_test(struct sandbox_state *state, + const char *arg) +{ + state->video_test = simple_strtol(arg, NULL, 10); + + return 0; +} +SANDBOX_CMDLINE_OPT_SHORT(video_test, 'V', 1, + "Enable video test mode (ms delay between asserts)"); + static const char *term_args[STATE_TERM_COUNT] = { "raw-with-sigs", "raw", diff --git a/arch/sandbox/include/asm/state.h b/arch/sandbox/include/asm/state.h index af92f4e6fcd..fc3c39cde27 100644 --- a/arch/sandbox/include/asm/state.h +++ b/arch/sandbox/include/asm/state.h @@ -177,6 +177,7 @@ struct sandbox_state { bool soft_fail; /* Continue on failure */ bool pager_bypass; /* Enable pager-bypass mode */ bool no_term_present; /* Assume no terminal present */ + int video_test; /* ms to wait before next assert */ /* Pointer to information for each SPI bus/cs */ struct sandbox_spi_info spi[CONFIG_SANDBOX_SPI_MAX_BUS] diff --git a/doc/arch/sandbox/sandbox.rst b/doc/arch/sandbox/sandbox.rst index 31a670ce2ab..bfc0ee70834 100644 --- a/doc/arch/sandbox/sandbox.rst +++ b/doc/arch/sandbox/sandbox.rst @@ -252,6 +252,10 @@ available options. Some of these are described below: -v, --verbose Show console output from tests. Normally this is suppressed. +-V, --video_test <ms> + Enable video test mode with a delay (in milliseconds) between assertions. This + allows time to examine the display during testing. + -w, --write Write driver state to state file on exit. In conjunction with `-r`, this allows sandbox to save and restore emulated hardware state (such as a TPM) across diff --git a/test/dm/video.c b/test/dm/video.c index b1bf08a99f7..f8c126dba5c 100644 --- a/test/dm/video.c +++ b/test/dm/video.c @@ -50,6 +50,7 @@ DM_TEST(dm_test_video_base, UTF_SCAN_PDATA | UTF_SCAN_FDT); int video_compress_fb(struct unit_test_state *uts, struct udevice *dev, bool use_copy) { + struct sandbox_state *state = state_get_current(); struct video_priv *priv = dev_get_uclass_priv(dev); uint destlen; void *dest; @@ -70,8 +71,10 @@ int video_compress_fb(struct unit_test_state *uts, struct udevice *dev, if (ret) return ret; - /* provide a useful delay if #define LOG_DEBUG at the top of file */ - if (_DEBUG) + /* provide a useful delay if -V flag is used or LOG_DEBUG is set */ + if (state->video_test) + mdelay(state->video_test); + else if (_DEBUG) mdelay(300); return destlen; -- 2.43.0
From: Simon Glass <sjg@chromium.org> Add a new --video_frames option to sandbox which accepts a directory path. When set, every video test assertion writes a BMP file (frame0.bmp, frame1.bmp, etc.) to the specified directory, allowing visual inspection of the framebuffer at each test step. A new video_write_bmp() function writes 16/32bpp framebuffer contents as Windows BMP files. On startup, any existing frame*.bmp files in the directory are removed to ensure a clean state. Usage example: ./u-boot --video_frames /tmp/frames -c "ut dm video_text" Co-developed-by: Claude <noreply@anthropic.com> Signed-off-by: Simon Glass <sjg@chromium.org> --- arch/sandbox/cpu/start.c | 22 ++++++++ arch/sandbox/include/asm/state.h | 2 + doc/arch/sandbox/sandbox.rst | 3 ++ test/dm/video.c | 86 ++++++++++++++++++++++++++++++++ 4 files changed, 113 insertions(+) diff --git a/arch/sandbox/cpu/start.c b/arch/sandbox/cpu/start.c index 559c11a2dbb..30d4f83b6ee 100644 --- a/arch/sandbox/cpu/start.c +++ b/arch/sandbox/cpu/start.c @@ -375,6 +375,15 @@ static int sandbox_cmdline_cb_video_test(struct sandbox_state *state, SANDBOX_CMDLINE_OPT_SHORT(video_test, 'V', 1, "Enable video test mode (ms delay between asserts)"); +static int sandbox_cmdline_cb_video_frames(struct sandbox_state *state, + const char *arg) +{ + state->video_frames_dir = arg; + + return 0; +} +SANDBOX_CMDLINE_OPT(video_frames, 1, "Directory to write video frames"); + static const char *term_args[STATE_TERM_COUNT] = { "raw-with-sigs", "raw", @@ -655,6 +664,19 @@ int sandbox_init(int argc, char *argv[], struct global_data *data) if (os_parse_args(state, argc, argv)) return 1; + /* Remove old frame*.bmp files if video_frames_dir is set */ + if (state->video_frames_dir) { + char pattern[256]; + int i; + + for (i = 0; i < 1000; i++) { + snprintf(pattern, sizeof(pattern), "%s/frame%d.bmp", + state->video_frames_dir, i); + if (os_unlink(pattern)) + break; + } + } + /* Detect if serial console is connected to a terminal */ state->serial_is_tty = os_isatty(1) && state->term_raw != STATE_TERM_COOKED; diff --git a/arch/sandbox/include/asm/state.h b/arch/sandbox/include/asm/state.h index fc3c39cde27..6a89f0ca5ef 100644 --- a/arch/sandbox/include/asm/state.h +++ b/arch/sandbox/include/asm/state.h @@ -178,6 +178,8 @@ struct sandbox_state { bool pager_bypass; /* Enable pager-bypass mode */ bool no_term_present; /* Assume no terminal present */ int video_test; /* ms to wait before next assert */ + const char *video_frames_dir; /* Directory to write video frames */ + int video_frame_count; /* Number of frames written */ /* Pointer to information for each SPI bus/cs */ struct sandbox_spi_info spi[CONFIG_SANDBOX_SPI_MAX_BUS] diff --git a/doc/arch/sandbox/sandbox.rst b/doc/arch/sandbox/sandbox.rst index bfc0ee70834..fc2b7c482f4 100644 --- a/doc/arch/sandbox/sandbox.rst +++ b/doc/arch/sandbox/sandbox.rst @@ -252,6 +252,9 @@ available options. Some of these are described below: -v, --verbose Show console output from tests. Normally this is suppressed. +--video_frames <dir> + Write video frames to the specified directory for debugging purposes. + -V, --video_test <ms> Enable video test mode with a delay (in milliseconds) between assertions. This allows time to examine the display during testing. diff --git a/test/dm/video.c b/test/dm/video.c index f8c126dba5c..56e63a6f5cf 100644 --- a/test/dm/video.c +++ b/test/dm/video.c @@ -4,6 +4,7 @@ * Written by Simon Glass <sjg@chromium.org> */ +#include <bmp_layout.h> #include <bzlib.h> #include <dm.h> #include <gzip.h> @@ -47,6 +48,79 @@ static int dm_test_video_base(struct unit_test_state *uts) } DM_TEST(dm_test_video_base, UTF_SCAN_PDATA | UTF_SCAN_FDT); +/** + * video_write_bmp() - Write framebuffer to BMP file + * + * This writes the current framebuffer contents to a BMP file on the host + * filesystem. Useful for debugging video tests. + * + * @uts: Test state + * @dev: Video device + * @fname: Filename to write to + * Return: 0 if OK, -ve on error + */ +static int video_write_bmp(struct unit_test_state *uts, struct udevice *dev, + const char *fname) +{ + struct video_priv *priv = dev_get_uclass_priv(dev); + struct bmp_image *bmp; + u32 width = priv->xsize; + u32 height = priv->ysize; + u32 row_bytes, bmp_size, bpp, bytes_per_pixel; + void *bmp_data; + int ret, y; + + /* Support 16bpp and 32bpp */ + switch (priv->bpix) { + case VIDEO_BPP16: + bpp = 16; + bytes_per_pixel = 2; + break; + case VIDEO_BPP32: + bpp = 32; + bytes_per_pixel = 4; + break; + default: + return -ENOSYS; + } + + /* BMP rows are padded to 4-byte boundary */ + row_bytes = ALIGN(width * bytes_per_pixel, BMP_DATA_ALIGN); + bmp_size = sizeof(struct bmp_header) + row_bytes * height; + + bmp = malloc(bmp_size); + if (!bmp) + return -ENOMEM; + + memset(bmp, 0, bmp_size); + + /* Fill in BMP header */ + bmp->header.signature[0] = 'B'; + bmp->header.signature[1] = 'M'; + bmp->header.file_size = cpu_to_le32(bmp_size); + bmp->header.data_offset = cpu_to_le32(sizeof(struct bmp_header)); + bmp->header.size = cpu_to_le32(40); + bmp->header.width = cpu_to_le32(width); + bmp->header.height = cpu_to_le32(height); + bmp->header.planes = cpu_to_le16(1); + bmp->header.bit_count = cpu_to_le16(bpp); + bmp->header.compression = cpu_to_le32(BMP_BI_RGB); + + /* Copy framebuffer data (BMP is bottom-up) */ + bmp_data = (void *)bmp + sizeof(struct bmp_header); + for (y = 0; y < height; y++) { + void *src = priv->fb + (height - 1 - y) * priv->line_length; + void *dst = bmp_data + y * row_bytes; + + memcpy(dst, src, width * bytes_per_pixel); + } + + ret = os_write_file(fname, bmp, bmp_size); + free(bmp); + + return ret; +} + int video_compress_fb(struct unit_test_state *uts, struct udevice *dev, bool use_copy) { @@ -71,6 +145,18 @@ int video_compress_fb(struct unit_test_state *uts, struct udevice *dev, if (ret) return ret; + /* Write frame to file if --video-frames option is set */ + if (state->video_frames_dir) { + char filename[256]; + + snprintf(filename, sizeof(filename), "%s/frame%d.bmp", + state->video_frames_dir, state->video_frame_count++); + ret = video_write_bmp(uts, dev, filename); + if (ret) + printf("Failed to write frame to %s: %d\n", filename, + ret); + } + /* provide a useful delay if -V flag is used or LOG_DEBUG is set */ if (state->video_test) mdelay(state->video_test); -- 2.43.0
From: Simon Glass <sjg@chromium.org> This function was added without a comment. Fix it. Signed-off-by: Simon Glass <sjg@chromium.org> --- arch/sandbox/include/asm/sdl.h | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/arch/sandbox/include/asm/sdl.h b/arch/sandbox/include/asm/sdl.h index 86368c8a0a6..735652de6bd 100644 --- a/arch/sandbox/include/asm/sdl.h +++ b/arch/sandbox/include/asm/sdl.h @@ -106,6 +106,15 @@ int sandbox_sdl_sound_init(int rate, int channels); */ int sandbox_sdl_set_bpp(struct udevice *dev, enum video_log2_bpp l2bpp); +/** + * sandbox_sdl_get_mouse_event() - Read a mouse event from SDL + * + * If a mouse event has been recorded since the last call, this marked the event + * as used and then returns its. + * + * @evt: Mouse event + * ReturnL 0 if OK, -EAGAIN if there is no event available + */ int sandbox_sdl_get_mouse_event(struct mouse_event *evt); #else -- 2.43.0
From: Simon Glass <sjg@chromium.org> Document the cyc_active member and arrange things in the correct order. Signed-off-by: Simon Glass <sjg@chromium.org> --- drivers/video/video-uclass.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/video/video-uclass.c b/drivers/video/video-uclass.c index eaf6cf9fc71..f5cd1727fce 100644 --- a/drivers/video/video-uclass.c +++ b/drivers/video/video-uclass.c @@ -64,6 +64,7 @@ struct cyclic_info; * available address to use for a device's framebuffer. It starts at * gd->video_top and works downwards, running out of space when it hits * gd->video_bottom. + * @cyc_active: true if cyclic video sync is currently registered * @cyc: handle for cyclic-execution function, or NULL if none */ struct video_uc_priv { -- 2.43.0
From: Simon Glass <sjg@chromium.org> When the mouse is being used we need a suitable pointer image to draw on the display. Add the RISC OS pointer, in honour of the first GUI available on ARM devices. Signed-off-by: Simon Glass <sjg@chromium.org> --- drivers/video/images/Makefile | 3 +++ drivers/video/images/riscos_arrow.bmp | Bin 0 -> 3798 bytes test/dm/video.c | 3 ++- 3 files changed, 5 insertions(+), 1 deletion(-) create mode 100644 drivers/video/images/riscos_arrow.bmp diff --git a/drivers/video/images/Makefile b/drivers/video/images/Makefile index 9b0d85cd2c8..9019b36c918 100644 --- a/drivers/video/images/Makefile +++ b/drivers/video/images/Makefile @@ -3,6 +3,9 @@ # Copyright 2025 Simon Glass <sjg@chromium.org> obj-$(CONFIG_VIDEO_LOGO) += u_boot.o +ifdef CONFIG_MOUSE +obj-$(CONFIG_EXPO) += riscos_arrow.o +endif ifdef CONFIG_$(PHASE_)GENERATE_ACPI_TABLE obj-y += bgrt.o diff --git a/drivers/video/images/riscos_arrow.bmp b/drivers/video/images/riscos_arrow.bmp new file mode 100644 index 0000000000000000000000000000000000000000..5327bfd9b5438b8711e11f353903085808cd000c GIT binary patch literal 3798 zcmeH~O-LI-7>3`(7JnM6XehSyu!kr;=%p3$7}J6X3L#qX@6g?ii9&V_y9r)A3SNaC z`g5sm554r<UVG@ZP!Mbj#k=51v6mw2JF|(2#`V(KgC#>|=jEIEo|*aPCroN;_cml2 z;r9WvR>=|cQ8C4U-dle54S?57%?LCj@P9^tr#rj@4o}=3A@L>ry7I5r;4dtq^1)x& zC*klbTSYmX_d=D)F3x@`w|yP<9ADMRx{U94BxdpNy#({wOQ?JW@PPGIGjjh=<+Avw z8F>I-CA=X%is9$^>fv?#P~cU5Sa8V?5zZ?ux?M0W_eK)1?2?liPieDr^IGdZ+R=kM zxQlMxbc<u@^kntT&{+Q2MMPM4h6Jmz{z^+{cF8q?h{je)v*?tlpHYvjm5M^Yq>h`* zdY1Yl^`OH6QvVco!`BBw=cSHPBJRv&9&>JSYAs)<jXG^EI~LmvvrQsv+2luOeVrA0 zU1+ON=GbHsGmI_SPp4TXnD1j86PQAp4=n>_te}7rhFIFPxX7?R72tnQ`lsn-&}#)I zJ)U3{7If(ku?B3zREw>fd-PD6BC!;3bI<dR`hnKxz{!^9{eA0sC+~ok-@q5U=D)fP zj2_dzRkJ1D09_lvr>`}e@dEg~3H1Fc>W(h^Y2i1T%K?WUfX+F-9KQn{3x3>KC+hM3 e2Qb4}O@0Gjy#@yIWG@EHyNwQKal5ijC4Q$t=#ZTN literal 0 HcmV?d00001 diff --git a/test/dm/video.c b/test/dm/video.c index 56e63a6f5cf..6c96eee1b8b 100644 --- a/test/dm/video.c +++ b/test/dm/video.c @@ -1211,9 +1211,10 @@ static int dm_test_video_images(struct unit_test_state *uts) ut_assert_nextline("Name Size"); ut_assert_nextline("-------------------- ----------"); ut_assert_nextline("bgrt 43926"); + ut_assert_nextline("riscos_arrow 3798"); ut_assert_nextline("u_boot 6932"); ut_assert_skip_to_line(""); - ut_assert_nextline("Total images: 2"); + ut_assert_nextline("Total images: 3"); ut_assert_console_end(); return 0; -- 2.43.0
From: Simon Glass <sjg@chromium.org> Add some simple tests for drawing the mouse pointer on a white and black background. Signed-off-by: Simon Glass <sjg@chromium.org> --- test/dm/video.c | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/test/dm/video.c b/test/dm/video.c index 6c96eee1b8b..a802228b790 100644 --- a/test/dm/video.c +++ b/test/dm/video.c @@ -667,6 +667,35 @@ static int dm_test_video_comp_bmp8(struct unit_test_state *uts) } DM_TEST(dm_test_video_comp_bmp8, UTF_SCAN_PDATA | UTF_SCAN_FDT); +/* Test drawing the riscos pointer */ +static int dm_test_video_bmp_alpha(struct unit_test_state *uts) +{ + struct video_priv *priv; + struct udevice *dev; + ulong addr; + + ut_assertok(video_get_nologo(uts, &dev)); + priv = dev_get_uclass_priv(dev); + addr = map_to_sysmem(video_image_getptr(riscos_arrow)); + + /* Draw a black rectangle first */ + video_draw_box(dev, 100, 100, 200, 200, 0, + video_index_to_colour(priv, VID_BLACK), true); + + /* Draw the pointer on top of the black rectangle */ + ut_assertok(video_bmp_display(dev, addr, 110, 110, false)); + ut_asserteq(174, video_compress_fb(uts, dev, false)); + ut_assertok(video_check_copy_fb(uts, dev)); + + /* Draw the pointer on top of the black rectangle */ + ut_assertok(video_bmp_display(dev, addr, 350, 110, false)); + ut_asserteq(249, video_compress_fb(uts, dev, false)); + ut_assertok(video_check_copy_fb(uts, dev)); + + return 0; +} +DM_TEST(dm_test_video_bmp_alpha, UTF_SCAN_PDATA | UTF_SCAN_FDT); + /* Test TrueType console */ static int dm_test_video_truetype(struct unit_test_state *uts) { -- 2.43.0
From: Simon Glass <sjg@chromium.org> The compiler can deal with the complexity of deciding whether to set up this local. Move it to the top of the function. Signed-off-by: Simon Glass <sjg@chromium.org> --- drivers/video/video_bmp.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/video/video_bmp.c b/drivers/video/video_bmp.c index 478b8fbe5fd..390996dbaf1 100644 --- a/drivers/video/video_bmp.c +++ b/drivers/video/video_bmp.c @@ -68,14 +68,14 @@ static u32 get_bmp_col_rgba8888(struct bmp_color_table_entry *cte) static void write_pix8(u8 *fb, uint bpix, enum video_format eformat, struct bmp_color_table_entry *palette, u8 *bmap) { + struct bmp_color_table_entry *cte = &palette[*bmap]; + if (bpix == 8) { *fb++ = *bmap; } else if (bpix == 16) { - *(u16 *)fb = get_bmp_col_16bpp(palette[*bmap]); + *(u16 *)fb = get_bmp_col_16bpp(*cte); } else { /* Only support big endian */ - struct bmp_color_table_entry *cte = &palette[*bmap]; - if (bpix == 24) { *fb++ = cte->red; *fb++ = cte->green; -- 2.43.0
From: Simon Glass <sjg@chromium.org> Provide an alpha and acol parameter to control whether transparency is used. For now alpha is always false so this does nothing. Make the current video_bmp_display() an internal static function and add a new video_bmp_display() to call it. Signed-off-by: Simon Glass <sjg@chromium.org> --- drivers/video/video_bmp.c | 43 ++++++++++++++++++++++++++------------- 1 file changed, 29 insertions(+), 14 deletions(-) diff --git a/drivers/video/video_bmp.c b/drivers/video/video_bmp.c index 390996dbaf1..4c9157486cb 100644 --- a/drivers/video/video_bmp.c +++ b/drivers/video/video_bmp.c @@ -64,9 +64,12 @@ static u32 get_bmp_col_rgba8888(struct bmp_color_table_entry *cte) * @bmap: Pointer to BMP bitmap position to write. This contains a single byte * which is either written directly (bpix == 8) or used to look up the * palette to get a colour to write + * @alpha: Enable alpha transparency + * @acol: Alpha color (RGB888 format) */ static void write_pix8(u8 *fb, uint bpix, enum video_format eformat, - struct bmp_color_table_entry *palette, u8 *bmap) + struct bmp_color_table_entry *palette, u8 *bmap, + bool alpha, u32 acol) { struct bmp_color_table_entry *cte = &palette[*bmap]; @@ -96,12 +99,12 @@ static void write_pix8(u8 *fb, uint bpix, enum video_format eformat, static void draw_unencoded_bitmap(u8 **fbp, uint bpix, enum video_format eformat, uchar *bmap, struct bmp_color_table_entry *palette, - int cnt) + int cnt, bool alpha, u32 acolour) { u8 *fb = *fbp; while (cnt > 0) { - write_pix8(fb, bpix, eformat, palette, bmap++); + write_pix8(fb, bpix, eformat, palette, bmap++, alpha, acolour); fb += bpix / 8; cnt--; } @@ -110,12 +113,12 @@ static void draw_unencoded_bitmap(u8 **fbp, uint bpix, static void draw_encoded_bitmap(u8 **fbp, uint bpix, enum video_format eformat, struct bmp_color_table_entry *palette, u8 *bmap, - int cnt) + int cnt, bool alpha, u32 acolour) { u8 *fb = *fbp; while (cnt > 0) { - write_pix8(fb, bpix, eformat, palette, bmap); + write_pix8(fb, bpix, eformat, palette, bmap, alpha, acolour); fb += bpix / 8; cnt--; } @@ -126,7 +129,8 @@ static void video_display_rle8_bitmap(struct udevice *dev, struct bmp_image *bmp, uint bpix, struct bmp_color_table_entry *palette, uchar *fb, int x_off, int y_off, - ulong width, ulong height) + ulong width, ulong height, bool alpha, + u32 acolour) { struct video_priv *priv = dev_get_uclass_priv(dev); uchar *bmap; @@ -178,7 +182,8 @@ static void video_display_rle8_bitmap(struct udevice *dev, cnt = runlen; draw_unencoded_bitmap( &fb, bpix, eformat, - bmap, palette, cnt); + bmap, palette, cnt, + alpha, acolour); } x += runlen; } @@ -204,7 +209,8 @@ static void video_display_rle8_bitmap(struct udevice *dev, cnt = runlen; draw_encoded_bitmap(&fb, bpix, eformat, palette, &bmap[1], - cnt); + cnt, alpha, + acolour); } x += runlen; } @@ -252,8 +258,8 @@ void video_bmp_get_info(const void *bmp_image, ulong *widthp, ulong *heightp, *bpixp = get_unaligned_le16(&bmp->header.bit_count); } -int video_bmp_display(struct udevice *dev, ulong bmp_image, int x, int y, - bool align) +static int draw_bmp(struct udevice *dev, ulong bmp_image, int x, int y, + bool align, bool alpha, u32 acolour) { struct video_priv *priv = dev_get_uclass_priv(dev); int i, j; @@ -337,8 +343,10 @@ int video_bmp_display(struct udevice *dev, ulong bmp_image, int x, int y, &bmp->header.compression); debug("compressed %d %d\n", compression, BMP_BI_RLE8); if (compression == BMP_BI_RLE8) { - video_display_rle8_bitmap(dev, bmp, bpix, palette, fb, - x, y, width, height); + video_display_rle8_bitmap(dev, bmp, bpix, + palette, fb, x, y, + width, height, alpha, + acolour); break; } } @@ -351,9 +359,10 @@ int video_bmp_display(struct udevice *dev, ulong bmp_image, int x, int y, for (i = 0; i < height; ++i) { schedule(); for (j = 0; j < width; j++) { - write_pix8(fb, bpix, eformat, palette, bmap); - bmap++; + write_pix8(fb, bpix, eformat, palette, bmap, + alpha, acolour); fb += bpix / 8; + bmap++; } bmap += (padded_width - width); fb -= byte_width + priv->line_length; @@ -462,3 +471,9 @@ int video_bmp_display(struct udevice *dev, ulong bmp_image, int x, int y, return video_sync(dev, false); } + +int video_bmp_display(struct udevice *dev, ulong bmp_image, int x, int y, + bool align) +{ + return draw_bmp(dev, bmp_image, x, y, align, false, 0); +} -- 2.43.0
From: Simon Glass <sjg@chromium.org> Add red, green and blue variables when processing a 24bpp BMP image. Signed-off-by: Simon Glass <sjg@chromium.org> --- drivers/video/video_bmp.c | 40 +++++++++++++++++++++------------------ 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/drivers/video/video_bmp.c b/drivers/video/video_bmp.c index 4c9157486cb..88a327e4a6d 100644 --- a/drivers/video/video_bmp.c +++ b/drivers/video/video_bmp.c @@ -385,20 +385,23 @@ static int draw_bmp(struct udevice *dev, ulong bmp_image, int x, int y, if (CONFIG_IS_ENABLED(BMP_24BPP)) { for (i = 0; i < height; ++i) { for (j = 0; j < width; j++) { + u8 red = bmap[0]; + u8 green = bmap[1]; + u8 blue = bmap[2]; + if (bpix == 16) { /* 16bit 565RGB format */ - *(u16 *)fb = ((bmap[2] >> 3) - << 11) | - ((bmap[1] >> 2) << 5) | - (bmap[0] >> 3); - bmap += 3; + *(u16 *)fb = + ((blue >> 3) << 11) | + ((green >> 2) << 5) | + (red >> 3); fb += 2; } else if (eformat == VIDEO_X2R10G10B10) { u32 pix; - pix = *bmap++ << 2U; - pix |= *bmap++ << 12U; - pix |= *bmap++ << 22U; + pix = blue << 2U; + pix |= green << 12U; + pix |= red << 22U; *fb++ = pix & 0xff; *fb++ = (pix >> 8) & 0xff; *fb++ = (pix >> 16) & 0xff; @@ -406,20 +409,21 @@ static int draw_bmp(struct udevice *dev, ulong bmp_image, int x, int y, } else if (eformat == VIDEO_RGBA8888) { u32 pix; - pix = *bmap++ << 8U; /* blue */ - pix |= *bmap++ << 16U; /* green */ - pix |= *bmap++ << 24U; /* red */ - - *fb++ = (pix >> 24) & 0xff; - *fb++ = (pix >> 16) & 0xff; + pix = red << 24U; + pix |= green << 16U; + pix |= blue << 8U; + pix |= 0xff; + *fb++ = pix & 0xff; *fb++ = (pix >> 8) & 0xff; - *fb++ = 0xff; + *fb++ = (pix >> 16) & 0xff; + *fb++ = pix >> 24; } else { - *fb++ = *bmap++; - *fb++ = *bmap++; - *fb++ = *bmap++; + *fb++ = red; + *fb++ = green; + *fb++ = blue; *fb++ = 0; } + bmap += 3; } fb -= priv->line_length + width * (bpix / 8); bmap += (padded_width - width); -- 2.43.0
From: Simon Glass <sjg@chromium.org> When the pixel colour matches the transparency colour, skip writing it to the display. Signed-off-by: Simon Glass <sjg@chromium.org> --- drivers/video/video_bmp.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/drivers/video/video_bmp.c b/drivers/video/video_bmp.c index 88a327e4a6d..9c9d9b2091b 100644 --- a/drivers/video/video_bmp.c +++ b/drivers/video/video_bmp.c @@ -388,6 +388,15 @@ static int draw_bmp(struct udevice *dev, ulong bmp_image, int x, int y, u8 red = bmap[0]; u8 green = bmap[1]; u8 blue = bmap[2]; + u32 pixel_color = (red << 16) | + (green << 8) | blue; + + bmap += 3; + /* Skip transparent pixels if alpha enabled */ + if (alpha && pixel_color == acolour) { + fb += bpix / 8; + continue; + } if (bpix == 16) { /* 16bit 565RGB format */ @@ -423,7 +432,6 @@ static int draw_bmp(struct udevice *dev, ulong bmp_image, int x, int y, *fb++ = blue; *fb++ = 0; } - bmap += 3; } fb -= priv->line_length + width * (bpix / 8); bmap += (padded_width - width); -- 2.43.0
From: Simon Glass <sjg@chromium.org> Check the palette entry against the transparency colour and skip writing it to the display if it matches. Signed-off-by: Simon Glass <sjg@chromium.org> --- drivers/video/video_bmp.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/drivers/video/video_bmp.c b/drivers/video/video_bmp.c index 9c9d9b2091b..ebdf73e25a9 100644 --- a/drivers/video/video_bmp.c +++ b/drivers/video/video_bmp.c @@ -53,6 +53,20 @@ static u32 get_bmp_col_rgba8888(struct bmp_color_table_entry *cte) (cte->blue << 16U) | 0xff << 24U); } +/** + * matches_alpha() - Check if a palette entry matches the alpha color + * + * @ent: BMP palette entry + * @col: Alpha color to compare (RGB888 format) + * Return: true if the palette entry matches the alpha color + */ +static bool matches_alpha(struct bmp_color_table_entry *ent, u32 col) +{ + u32 colour = (ent->red << 16) | (ent->green << 8) | ent->blue; + + return colour == col; +} + /** * write_pix8() - Write a pixel from a BMP image into the framebuffer * @@ -73,6 +87,10 @@ static void write_pix8(u8 *fb, uint bpix, enum video_format eformat, { struct bmp_color_table_entry *cte = &palette[*bmap]; + /* Check for transparent pixel */ + if (alpha && matches_alpha(cte, acol)) + return; + if (bpix == 8) { *fb++ = *bmap; } else if (bpix == 16) { -- 2.43.0
From: Simon Glass <sjg@chromium.org> Provide a new BMP function which can draw a bitmap while regarding a chosen colour as transparent. Signed-off-by: Simon Glass <sjg@chromium.org> --- drivers/video/video_bmp.c | 6 ++++ include/video.h | 16 ++++++++++ test/dm/video.c | 62 ++++++++++++++++++++++++++++++++------- 3 files changed, 74 insertions(+), 10 deletions(-) diff --git a/drivers/video/video_bmp.c b/drivers/video/video_bmp.c index ebdf73e25a9..43aa45f0efe 100644 --- a/drivers/video/video_bmp.c +++ b/drivers/video/video_bmp.c @@ -507,3 +507,9 @@ int video_bmp_display(struct udevice *dev, ulong bmp_image, int x, int y, { return draw_bmp(dev, bmp_image, x, y, align, false, 0); } + +int video_bmp_displaya(struct udevice *dev, ulong bmp_image, int x, int y, + bool align, bool alpha, u32 acolour) +{ + return draw_bmp(dev, bmp_image, x, y, align, alpha, acolour); +} diff --git a/include/video.h b/include/video.h index 9d9a725d30d..1ad5868e2f6 100644 --- a/include/video.h +++ b/include/video.h @@ -383,6 +383,22 @@ void video_bmp_get_info(const void *bmp_image, ulong *widthp, ulong *heightp, int video_bmp_display(struct udevice *dev, ulong bmp_image, int x, int y, bool align); +/** + * video_bmp_displaya() - Display a BMP image with alpha transparency + * + * @dev: Device to use + * @bmp_image: Address of BMP image + * @x: X position to draw image + * @y: Y position to draw image + * @align: true to adjust the coordinates to centre the image (see + * video_bmp_display() for details) + * @alpha: true to enable alpha transparency + * @acolour: Color to treat as transparent (RGB888 format: 0xRRGGBB) + * Return: 0 if OK, -ve on error + */ +int video_bmp_displaya(struct udevice *dev, ulong bmp_image, int x, int y, + bool align, bool alpha, u32 acolour); + /** * video_get_xsize() - Get the width of the display in pixels * diff --git a/test/dm/video.c b/test/dm/video.c index a802228b790..3defa184b14 100644 --- a/test/dm/video.c +++ b/test/dm/video.c @@ -667,15 +667,25 @@ static int dm_test_video_comp_bmp8(struct unit_test_state *uts) } DM_TEST(dm_test_video_comp_bmp8, UTF_SCAN_PDATA | UTF_SCAN_FDT); -/* Test drawing the riscos pointer */ -static int dm_test_video_bmp_alpha(struct unit_test_state *uts) +/** + * check_bmp_alpha() - Test drawing the riscos pointer with transparency + * + * Draws the riscos pointer BMP in various positions with and without + * transparency to verify alpha blending works correctly + * + * @uts: Test state + * @dev: Video device + * @first: Expected compression after first pointer draw + * @second: Expected compression after second pointer draw + * @trans: Expected compression after drawing with transparency + * Return: 0 if OK, -ve on error + */ +static int check_bmp_alpha(struct unit_test_state *uts, struct udevice *dev, + int first, int second, int trans) { - struct video_priv *priv; - struct udevice *dev; + struct video_priv *priv = dev_get_uclass_priv(dev); ulong addr; - ut_assertok(video_get_nologo(uts, &dev)); - priv = dev_get_uclass_priv(dev); addr = map_to_sysmem(video_image_getptr(riscos_arrow)); /* Draw a black rectangle first */ @@ -684,17 +694,49 @@ static int dm_test_video_bmp_alpha(struct unit_test_state *uts) /* Draw the pointer on top of the black rectangle */ ut_assertok(video_bmp_display(dev, addr, 110, 110, false)); - ut_asserteq(174, video_compress_fb(uts, dev, false)); + ut_asserteq(first, video_compress_fb(uts, dev, false)); ut_assertok(video_check_copy_fb(uts, dev)); - /* Draw the pointer on top of the black rectangle */ + /* Draw the pointer on top of the white background */ ut_assertok(video_bmp_display(dev, addr, 350, 110, false)); - ut_asserteq(249, video_compress_fb(uts, dev, false)); + ut_asserteq(second, video_compress_fb(uts, dev, false)); + + /* Draw the pointer with white (0xffffff) as transparent */ + ut_assertok(video_bmp_displaya(dev, addr, 110, 160, false, true, + 0xffffff)); + ut_assertok(video_bmp_displaya(dev, addr, 350, 160, false, true, + 0xffffff)); + ut_asserteq(trans, video_compress_fb(uts, dev, false)); ut_assertok(video_check_copy_fb(uts, dev)); return 0; } -DM_TEST(dm_test_video_bmp_alpha, UTF_SCAN_PDATA | UTF_SCAN_FDT); + +/* Test drawing the riscos pointer on a 16bpp display */ +static int dm_test_video_bmp_alpha16(struct unit_test_state *uts) +{ + struct udevice *dev; + + ut_assertok(video_get_nologo(uts, &dev)); + ut_assertok(check_bmp_alpha(uts, dev, 174, 249, 358)); + + return 0; +} +DM_TEST(dm_test_video_bmp_alpha16, UTF_SCAN_PDATA | UTF_SCAN_FDT); + +/* Test drawing the riscos pointer on 32bpp display */ +static int dm_test_video_bmp_alpha32(struct unit_test_state *uts) +{ + struct udevice *dev; + + ut_assertok(uclass_find_first_device(UCLASS_VIDEO, &dev)); + ut_assertnonnull(dev); + ut_assertok(sandbox_sdl_set_bpp(dev, VIDEO_BPP32)); + ut_assertok(check_bmp_alpha(uts, dev, 641, 710, 869)); + + return 0; +} +DM_TEST(dm_test_video_bmp_alpha32, UTF_SCAN_PDATA | UTF_SCAN_FDT); /* Test TrueType console */ static int dm_test_video_truetype(struct unit_test_state *uts) -- 2.43.0
From: Simon Glass <sjg@chromium.org> Replace the anonymous struct in video_priv with struct video_bbox for the damage field. Update all references from xstart/ystart/xend/yend to x0/y0/x1/y1 to match the video_bbox field names. Add local video_bbox pointers in functions that access damage fields repeatedly to improve readability. Co-developed-by: Claude <noreply@anthropic.com> Signed-off-by: Simon Glass <sjg@chromium.org> --- drivers/video/video-uclass.c | 36 +++++++++++++++++++---------------- include/video.h | 13 ++----------- include/video_defs.h | 37 ++++++++++++++++++++++++++++++++++++ test/dm/video.c | 34 +++++++++++++++++---------------- 4 files changed, 77 insertions(+), 43 deletions(-) diff --git a/drivers/video/video-uclass.c b/drivers/video/video-uclass.c index f5cd1727fce..5c2cb251683 100644 --- a/drivers/video/video-uclass.c +++ b/drivers/video/video-uclass.c @@ -411,6 +411,7 @@ void video_set_default_colors(struct udevice *dev, bool invert) void video_damage(struct udevice *vid, int x, int y, int width, int height) { struct video_priv *priv = dev_get_uclass_priv(vid); + struct video_bbox *damage = &priv->damage; int xend = x + width; int yend = y + height; @@ -427,10 +428,10 @@ void video_damage(struct udevice *vid, int x, int y, int width, int height) yend = priv->ysize; /* Span a rectangle across all old and new damage */ - priv->damage.xstart = min(x, priv->damage.xstart); - priv->damage.ystart = min(y, priv->damage.ystart); - priv->damage.xend = max(xend, priv->damage.xend); - priv->damage.yend = max(yend, priv->damage.yend); + damage->x0 = min(x, damage->x0); + damage->y0 = min(y, damage->y0); + damage->x1 = max(xend, damage->x1); + damage->y1 = max(yend, damage->y1); } #endif @@ -457,12 +458,12 @@ static void video_flush_dcache(struct udevice *vid, bool use_copy) return; } - if (priv->damage.xend && priv->damage.yend) { - int lstart = priv->damage.xstart * VNBYTES(priv->bpix); - int lend = priv->damage.xend * VNBYTES(priv->bpix); + if (priv->damage.x1 && priv->damage.y1) { + int lstart = priv->damage.x0 * VNBYTES(priv->bpix); + int lend = priv->damage.x1 * VNBYTES(priv->bpix); int y; - for (y = priv->damage.ystart; y < priv->damage.yend; y++) { + for (y = priv->damage.y0; y < priv->damage.y1; y++) { ulong start = fb + (y * priv->line_length) + lstart; ulong end = start + lend - lstart; @@ -477,16 +478,17 @@ static void video_flush_dcache(struct udevice *vid, bool use_copy) static void video_flush_copy(struct udevice *vid) { struct video_priv *priv = dev_get_uclass_priv(vid); + struct video_bbox *damage = &priv->damage; if (!priv->copy_fb) return; - if (priv->damage.xend && priv->damage.yend) { - int lstart = priv->damage.xstart * VNBYTES(priv->bpix); - int lend = priv->damage.xend * VNBYTES(priv->bpix); + if (damage->x1 && damage->y1) { + int lstart = damage->x0 * VNBYTES(priv->bpix); + int lend = damage->x1 * VNBYTES(priv->bpix); int y; - for (y = priv->damage.ystart; y < priv->damage.yend; y++) { + for (y = damage->y0; y < damage->y1; y++) { ulong offset = (y * priv->line_length) + lstart; ulong len = lend - lstart; @@ -527,10 +529,12 @@ int video_sync(struct udevice *vid, bool force) priv->last_sync = get_timer(0); if (IS_ENABLED(CONFIG_VIDEO_DAMAGE)) { - priv->damage.xstart = priv->xsize; - priv->damage.ystart = priv->ysize; - priv->damage.xend = 0; - priv->damage.yend = 0; + struct video_bbox *damage = &priv->damage; + + damage->x0 = priv->xsize; + damage->y0 = priv->ysize; + damage->x1 = 0; + damage->y1 = 0; } return 0; diff --git a/include/video.h b/include/video.h index 1ad5868e2f6..92dca60a872 100644 --- a/include/video.h +++ b/include/video.h @@ -90,11 +90,7 @@ enum video_format { * @fb_size: Frame buffer size * @copy_fb: Copy of the frame buffer to keep up to date; see struct * video_uc_plat - * @damage: A bounding box of framebuffer regions updated since last sync - * @damage.xstart: X start position in pixels from the left - * @damage.ystart: Y start position in pixels from the top - * @damage.xend: X end position in pixels from the left - * @damage.xend: Y end position in pixels from the top + * @damage: Bounding box of framebuffer regions updated since last sync * @line_length: Length of each frame buffer line, in bytes. This can be * set by the driver, but if not, the uclass will set it after * probing @@ -124,12 +120,7 @@ struct video_priv { void *fb; int fb_size; void *copy_fb; - struct { - int xstart; - int ystart; - int xend; - int yend; - } damage; + struct video_bbox damage; int line_length; u32 colour_fg; u32 colour_bg; diff --git a/include/video_defs.h b/include/video_defs.h index 6bd822dc99e..c17959e3146 100644 --- a/include/video_defs.h +++ b/include/video_defs.h @@ -12,4 +12,41 @@ /* Maximum length of an embedded image name */ #define VIDEO_IMAGE_NAMELEN 16 +#ifndef __ASSEMBLY__ + +#include <stdbool.h> + +/** + * struct video_bbox - Represents a bounding box for video operations + * + * The bounding box is only valid if x1 > x0 and y1 > y0. An invalid bounding + * box (where x1 <= x0 or y1 <= y0) indicates that there is no area to process. + * + * @x0: X start position in pixels from the left + * @y0: Y start position in pixels from the top + * @x1: X end position in pixels from the left + * @y1: Y end position in pixels from the top + */ +struct video_bbox { + int x0; + int y0; + int x1; + int y1; +}; + +/** + * video_bbox_valid() - Check if a bounding box is valid + * + * A valid bounding box has x1 > x0 and y1 > y0. An invalid/inverted bounding + * box (where x1 <= x0 or y1 <= y0) indicates that there is no area to process. + * + * @bbox: Bounding box to check + * Return: true if valid, false if invalid/inverted + */ +static inline bool video_bbox_valid(const struct video_bbox *bbox) +{ + return bbox->x1 > bbox->x0 && bbox->y1 > bbox->y0; +} +#endif /* __ASSEMBLY__ */ + #endif /* __VIDEO_DEFS_H */ diff --git a/test/dm/video.c b/test/dm/video.c index 3defa184b14..7ada4c75bf7 100644 --- a/test/dm/video.c +++ b/test/dm/video.c @@ -903,32 +903,34 @@ static int dm_test_video_damage(struct unit_test_state *uts) ut_assertok(uclass_get_device(UCLASS_VIDEO_CONSOLE, 0, &con)); priv = dev_get_uclass_priv(dev); + struct video_bbox *damage = &priv->damage; + vidconsole_position_cursor(con, 14, 10); vidconsole_put_string(con, test_string_2); - ut_asserteq(449, priv->damage.xstart); - ut_asserteq(325, priv->damage.ystart); - ut_asserteq(661, priv->damage.xend); - ut_asserteq(350, priv->damage.yend); + ut_asserteq(449, damage->x0); + ut_asserteq(325, damage->y0); + ut_asserteq(661, damage->x1); + ut_asserteq(350, damage->y1); vidconsole_position_cursor(con, 7, 5); vidconsole_put_string(con, test_string_1); - ut_asserteq(225, priv->damage.xstart); - ut_asserteq(164, priv->damage.ystart); - ut_asserteq(661, priv->damage.xend); - ut_asserteq(350, priv->damage.yend); + ut_asserteq(225, damage->x0); + ut_asserteq(164, damage->y0); + ut_asserteq(661, damage->x1); + ut_asserteq(350, damage->y1); vidconsole_position_cursor(con, 21, 15); vidconsole_put_string(con, test_string_3); - ut_asserteq(225, priv->damage.xstart); - ut_asserteq(164, priv->damage.ystart); - ut_asserteq(1280, priv->damage.xend); - ut_asserteq(510, priv->damage.yend); + ut_asserteq(225, damage->x0); + ut_asserteq(164, damage->y0); + ut_asserteq(1280, damage->x1); + ut_asserteq(510, damage->y1); video_sync(dev, true); - ut_asserteq(priv->xsize, priv->damage.xstart); - ut_asserteq(priv->ysize, priv->damage.ystart); - ut_asserteq(0, priv->damage.xend); - ut_asserteq(0, priv->damage.yend); + ut_asserteq(priv->xsize, damage->x0); + ut_asserteq(priv->ysize, damage->y0); + ut_asserteq(0, damage->x1); + ut_asserteq(0, damage->y1); ut_asserteq(7335, video_compress_fb(uts, dev, false)); ut_assertok(video_check_copy_fb(uts, dev)); -- 2.43.0
From: Simon Glass <sjg@chromium.org> Shorten the video_sync member in struct video_ops to just sync() since the struct name already provides the context that this is a video operation. Update all implementations and callers of this operation. Co-developed-by: Claude <noreply@anthropic.com> Signed-off-by: Simon Glass <sjg@chromium.org> --- drivers/video/efi.c | 2 +- drivers/video/mcde_simple.c | 2 +- drivers/video/seps525.c | 2 +- drivers/video/video-uclass.c | 4 ++-- include/video.h | 12 ++++++------ 5 files changed, 11 insertions(+), 11 deletions(-) diff --git a/drivers/video/efi.c b/drivers/video/efi.c index 1fe76f27bcc..578392c3908 100644 --- a/drivers/video/efi.c +++ b/drivers/video/efi.c @@ -291,7 +291,7 @@ static int efi_video_bind(struct udevice *dev) } const static struct video_ops efi_video_ops = { - .video_sync = efi_video_sync, + .sync = efi_video_sync, }; static const struct udevice_id efi_video_ids[] = { diff --git a/drivers/video/mcde_simple.c b/drivers/video/mcde_simple.c index 2ba5d0de152..7591e088e38 100644 --- a/drivers/video/mcde_simple.c +++ b/drivers/video/mcde_simple.c @@ -122,7 +122,7 @@ static int mcde_simple_video_sync(struct udevice *dev) } static struct video_ops mcde_simple_ops = { - .video_sync = mcde_simple_video_sync, + .sync = mcde_simple_video_sync, }; static const struct udevice_id mcde_simple_ids[] = { diff --git a/drivers/video/seps525.c b/drivers/video/seps525.c index 86cd301c4b9..11293872a5c 100644 --- a/drivers/video/seps525.c +++ b/drivers/video/seps525.c @@ -306,7 +306,7 @@ static int seps525_bind(struct udevice *dev) } static const struct video_ops seps525_ops = { - .video_sync = seps525_sync, + .sync = seps525_sync, }; static const struct udevice_id seps525_ids[] = { diff --git a/drivers/video/video-uclass.c b/drivers/video/video-uclass.c index 5c2cb251683..bf633861ef3 100644 --- a/drivers/video/video-uclass.c +++ b/drivers/video/video-uclass.c @@ -507,8 +507,8 @@ int video_sync(struct udevice *vid, bool force) if (IS_ENABLED(CONFIG_VIDEO_COPY)) video_flush_copy(vid); - if (ops && ops->video_sync) { - ret = ops->video_sync(vid); + if (ops && ops->sync) { + ret = ops->sync(vid); if (ret) return ret; } diff --git a/include/video.h b/include/video.h index 92dca60a872..116a10afc93 100644 --- a/include/video.h +++ b/include/video.h @@ -133,14 +133,14 @@ struct video_priv { /** * struct video_ops - structure for keeping video operations - * @video_sync: Synchronize FB with device. Some device like SPI based LCD - * displays needs synchronization when data in an FB is available. - * For these devices implement video_sync hook to call a sync - * function. vid is pointer to video device udevice. Function - * should return 0 on success video_sync and error code otherwise + * @sync: Synchronize FB with device. Some device like SPI based LCD + * displays needs synchronization when data in an FB is available. + * For these devices implement sync hook to call a sync function. + * vid is pointer to video device udevice. Function should return 0 + * on success and error code otherwise */ struct video_ops { - int (*video_sync)(struct udevice *vid); + int (*sync)(struct udevice *vid); }; #define video_get_ops(dev) ((struct video_ops *)(dev)->driver->ops) -- 2.43.0
From: Simon Glass <sjg@chromium.org> Adjust this comment to match the style of other uclasses. Signed-off-by: Simon Glass <sjg@chromium.org> --- include/video.h | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/include/video.h b/include/video.h index 116a10afc93..3ce603384dc 100644 --- a/include/video.h +++ b/include/video.h @@ -133,14 +133,22 @@ struct video_priv { /** * struct video_ops - structure for keeping video operations - * @sync: Synchronize FB with device. Some device like SPI based LCD - * displays needs synchronization when data in an FB is available. - * For these devices implement sync hook to call a sync function. - * vid is pointer to video device udevice. Function should return 0 - * on success and error code otherwise */ struct video_ops { - int (*sync)(struct udevice *vid); + /** + * @sync() - Synchronize FB with device + * + * Some devices like SPI-based LCD displays needs synchronization when + * data in a framebuffer is available. These devices can implement this + * method which is called whenever a video device is synced. + * + * Note that if CONFIG_VIDEO_DAMAGE is enabled, the driver can use this + * to optimise the region to redraw. + * + * @dev: Video device + * Return 0 on success, or -ve error code + */ + int (*sync)(struct udevice *dev); }; #define video_get_ops(dev) ((struct video_ops *)(dev)->driver->ops) -- 2.43.0
From: Simon Glass <sjg@chromium.org> When expo is being used, it should only redraw the display when it has made changes. Add a new 'manual sync' mode which tells the video subsystem to ignore video syncs from other sources. This also disables the idle feature, since it can interfere with tests. Signed-off-by: Simon Glass <sjg@chromium.org> --- drivers/video/video-uclass.c | 35 ++++++++++++++++++++++++ include/video.h | 10 +++++++ test/dm/video.c | 52 ++++++++++++++++++++++++++++++++++++ 3 files changed, 97 insertions(+) diff --git a/drivers/video/video-uclass.c b/drivers/video/video-uclass.c index bf633861ef3..f8ba3abc5f4 100644 --- a/drivers/video/video-uclass.c +++ b/drivers/video/video-uclass.c @@ -65,11 +65,13 @@ struct cyclic_info; * gd->video_top and works downwards, running out of space when it hits * gd->video_bottom. * @cyc_active: true if cyclic video sync is currently registered + * @manual_sync: true if manual-sync mode is active (caller controls video sync) * @cyc: handle for cyclic-execution function, or NULL if none */ struct video_uc_priv { ulong video_ptr; bool cyc_active; + bool manual_sync; struct cyclic_info cyc; }; @@ -501,9 +503,14 @@ static void video_flush_copy(struct udevice *vid) int video_sync(struct udevice *vid, bool force) { struct video_priv *priv = dev_get_uclass_priv(vid); + struct video_uc_priv *uc_priv = uclass_get_priv(vid->uclass); struct video_ops *ops = video_get_ops(vid); int ret; + /* Skip sync if manual-sync mode is active */ + if (uc_priv->manual_sync) + return 0; + if (IS_ENABLED(CONFIG_VIDEO_COPY)) video_flush_copy(vid); @@ -622,6 +629,20 @@ int video_default_font_height(struct udevice *dev) static void video_idle(struct cyclic_info *cyc) { + struct video_uc_priv *uc_priv; + struct uclass *uc; + int ret; + + ret = uclass_get(UCLASS_VIDEO, &uc); + if (ret) + return; + + uc_priv = uclass_get_priv(uc); + + /* Skip sync if manual-sync mode is active */ + if (uc_priv->manual_sync) + return; + if (CONFIG_IS_ENABLED(CURSOR)) { struct udevice *cons; struct uclass *uc; @@ -809,6 +830,20 @@ __maybe_unused static int video_destroy(struct uclass *uc) return 0; } +void video_set_manual_sync(bool enable) +{ + struct video_uc_priv *uc_priv; + struct uclass *uc; + int ret; + + ret = uclass_get(UCLASS_VIDEO, &uc); + if (ret) + return; + + uc_priv = uclass_get_priv(uc); + uc_priv->manual_sync = enable; +} + UCLASS_DRIVER(video) = { .id = UCLASS_VIDEO, .name = "video", diff --git a/include/video.h b/include/video.h index 3ce603384dc..3f5c8cbd45a 100644 --- a/include/video.h +++ b/include/video.h @@ -543,4 +543,14 @@ static inline bool video_is_visible(void) #endif } +/** + * video_set_manual_sync() - Set manual-sync mode for video subsystem + * + * When manual-sync mode is enabled, automatic video sync operations are + * suppressed to allow the caller to control rendering timing. + * + * @enable: true to enable manual-sync mode, false to disable + */ +void video_set_manual_sync(bool enable); + #endif diff --git a/test/dm/video.c b/test/dm/video.c index 7ada4c75bf7..cee9e528689 100644 --- a/test/dm/video.c +++ b/test/dm/video.c @@ -1293,3 +1293,55 @@ static int dm_test_video_images(struct unit_test_state *uts) return 0; } DM_TEST(dm_test_video_images, UTF_SCAN_PDATA | UTF_SCAN_FDT | UTF_CONSOLE); + +/* Test manual-sync mode suppresses auto-sync */ +static int dm_test_video_manual_sync(struct unit_test_state *uts) +{ + struct video_priv *priv; + struct udevice *dev, *con; + + ut_assertok(select_vidconsole(uts, "vidconsole0")); + ut_assertok(video_get_nologo(uts, &dev)); + ut_assertok(uclass_get_device(UCLASS_VIDEO_CONSOLE, 0, &con)); + priv = dev_get_uclass_priv(dev); + + /* Write some text and verify it appears in the framebuffer */ + vidconsole_put_string(con, "Test"); + ut_asserteq(118, video_compress_fb(uts, dev, false)); + + /* Sync to copy buffer before enabling manual-sync mode */ + ut_assertok(video_sync(dev, true)); + + /* Enable manual-sync mode - sync should be suppressed */ + video_set_manual_sync(true); + + /* Clear and write new text - auto-sync should not happen */ + video_clear(dev); + vidconsole_put_string(con, "Manual Sync"); + + /* should do nothing in manual-sync mode */ + ut_assertok(video_sync(dev, false)); + + /* The copy framebuffer should still show old content */ + if (IS_ENABLED(CONFIG_VIDEO_COPY)) { + ut_assertf(memcmp(priv->fb, priv->copy_fb, priv->fb_size), + "Copy fb should not match fb in manual-sync mode"); + } + + /* + * video_sync() with force=true should still do nothing, except of + * course that without a copy framebuffer the string will be present on + * (only) framebuffer + */ + ut_assertok(video_sync(dev, true)); + if (IS_ENABLED(CONFIG_VIDEO_COPY)) { + ut_asserteq(118, video_compress_fb(uts, dev, true)); + ut_assertf(memcmp(priv->fb, priv->copy_fb, priv->fb_size), + "Copy fb should not match fb in manual-sync mode"); + } else { + ut_asserteq(183, video_compress_fb(uts, dev, true)); + } + + return 0; +} +DM_TEST(dm_test_video_manual_sync, UTF_SCAN_PDATA | UTF_SCAN_FDT); -- 2.43.0
From: Simon Glass <sjg@chromium.org> Add an enum video_sync_flags with VIDSYNC_FORCE, VIDSYNC_FLUSH, and VIDSYNC_COPY flags and update the sync() operation in struct video_ops to accept a flags parameter. This allows for more flexible control of video sync behavior. The VIDSYNC_FLUSH flag is set when a full flush should be performed, and the VIDSYNC_COPY flag indicates whether the framebuffer should be flushed to the copy buffer. The logic is now calculated before calling sync() so drivers can observe these states. Co-developed-by: Claude <noreply@anthropic.com> Signed-off-by: Simon Glass <sjg@chromium.org> --- drivers/video/efi.c | 2 +- drivers/video/mcde_simple.c | 2 +- drivers/video/seps525.c | 2 +- drivers/video/video-uclass.c | 14 +++++++++++--- include/video.h | 15 ++++++++++++++- 5 files changed, 28 insertions(+), 7 deletions(-) diff --git a/drivers/video/efi.c b/drivers/video/efi.c index 578392c3908..b5e472395e9 100644 --- a/drivers/video/efi.c +++ b/drivers/video/efi.c @@ -43,7 +43,7 @@ struct efi_video_priv { bool use_blit; }; -static int efi_video_sync(struct udevice *dev) +static int efi_video_sync(struct udevice *dev, uint flags) { struct video_priv *vid_priv = dev_get_uclass_priv(dev); struct efi_video_priv *priv = dev_get_priv(dev); diff --git a/drivers/video/mcde_simple.c b/drivers/video/mcde_simple.c index 7591e088e38..9dd9c382644 100644 --- a/drivers/video/mcde_simple.c +++ b/drivers/video/mcde_simple.c @@ -94,7 +94,7 @@ static int mcde_simple_probe(struct udevice *dev) return 0; } -static int mcde_simple_video_sync(struct udevice *dev) +static int mcde_simple_video_sync(struct udevice *dev, uint flags) { struct mcde_simple_priv *priv = dev_get_priv(dev); unsigned int val; diff --git a/drivers/video/seps525.c b/drivers/video/seps525.c index 11293872a5c..aaf716cb65d 100644 --- a/drivers/video/seps525.c +++ b/drivers/video/seps525.c @@ -221,7 +221,7 @@ static int seps525_spi_startup(struct udevice *dev) return 0; } -static int seps525_sync(struct udevice *vid) +static int seps525_sync(struct udevice *vid, uint flags) { struct video_priv *uc_priv = dev_get_uclass_priv(vid); struct seps525_priv *priv = dev_get_priv(vid); diff --git a/drivers/video/video-uclass.c b/drivers/video/video-uclass.c index f8ba3abc5f4..8f5689a8a51 100644 --- a/drivers/video/video-uclass.c +++ b/drivers/video/video-uclass.c @@ -505,23 +505,31 @@ int video_sync(struct udevice *vid, bool force) struct video_priv *priv = dev_get_uclass_priv(vid); struct video_uc_priv *uc_priv = uclass_get_priv(vid->uclass); struct video_ops *ops = video_get_ops(vid); + uint flags = 0; int ret; /* Skip sync if manual-sync mode is active */ if (uc_priv->manual_sync) return 0; + if (force) + flags |= VIDSYNC_FORCE; + + /* Check if sync should do full flush */ + if (!CONFIG_IS_ENABLED(CYCLIC) || force || + get_timer(priv->last_sync) >= CONFIG_VIDEO_SYNC_MS) + flags |= VIDSYNC_FLUSH; + if (IS_ENABLED(CONFIG_VIDEO_COPY)) video_flush_copy(vid); if (ops && ops->sync) { - ret = ops->sync(vid); + ret = ops->sync(vid, flags); if (ret) return ret; } - if (CONFIG_IS_ENABLED(CYCLIC) && !force && - get_timer(priv->last_sync) < CONFIG_VIDEO_SYNC_MS) + if (!(flags & VIDSYNC_FLUSH)) return 0; video_flush_dcache(vid, false); diff --git a/include/video.h b/include/video.h index 3f5c8cbd45a..d08781d3a78 100644 --- a/include/video.h +++ b/include/video.h @@ -10,6 +10,7 @@ #include <linker_lists.h> #include <stdio_dev.h> #include <video_defs.h> +#include <linux/bitops.h> #ifdef CONFIG_SANDBOX #include <asm/state.h> #endif @@ -74,6 +75,17 @@ enum video_format { VIDEO_X2R10G10B10, }; +/** + * enum video_sync_flags - Flags for video_sync() operations + * + * @VIDSYNC_FORCE: Force sync even if recently synced or in manual-sync mode + * @VIDSYNC_FLUSH: Flush dcache and perform full sync operations + */ +enum video_sync_flags { + VIDSYNC_FORCE = BIT(0), + VIDSYNC_FLUSH = BIT(1), +}; + /** * struct video_priv - Device information used by the video uclass * @@ -146,9 +158,10 @@ struct video_ops { * to optimise the region to redraw. * * @dev: Video device + * @flags: Flags for the sync operation (enum video_sync_flags) * Return 0 on success, or -ve error code */ - int (*sync)(struct udevice *dev); + int (*sync)(struct udevice *dev, uint flags); }; #define video_get_ops(dev) ((struct video_ops *)(dev)->driver->ops) -- 2.43.0
From: Simon Glass <sjg@chromium.org> Extract the core sync logic from video_sync() into a new video_manual_sync() function that accepts flags directly. This allows callers to perform sync operations with explicit control over the sync behavior. The video_sync() function now: - Determines the appropriate flags (VIDSYNC_FORCE, VIDSYNC_FLUSH, VIDSYNC_COPY) based on the force parameter and timing - Calls video_manual_sync() with those flags The video_manual_sync() function: - Takes explicit flags as a parameter - Handles VIDSYNC_COPY flag to control copy framebuffer flush - Performs the actual sync operations This separation allows manual-sync mode users (and tests) to call video_manual_sync() directly with custom flags while video_sync() continues to work as before for normal scenarios. Add tests for video_manual_sync() to check the behaviour with different flag combinations. Co-developed-by: Claude <noreply@anthropic.com> Signed-off-by: Simon Glass <sjg@chromium.org> --- drivers/video/video-uclass.c | 46 ++++++++++++++++++++++-------------- include/video.h | 16 +++++++++++++ test/dm/video.c | 27 +++++++++++++++++++++ 3 files changed, 71 insertions(+), 18 deletions(-) diff --git a/drivers/video/video-uclass.c b/drivers/video/video-uclass.c index 8f5689a8a51..a8849cf6289 100644 --- a/drivers/video/video-uclass.c +++ b/drivers/video/video-uclass.c @@ -499,28 +499,13 @@ static void video_flush_copy(struct udevice *vid) } } -/* Flush video activity to the caches */ -int video_sync(struct udevice *vid, bool force) +int video_manual_sync(struct udevice *vid, uint flags) { struct video_priv *priv = dev_get_uclass_priv(vid); - struct video_uc_priv *uc_priv = uclass_get_priv(vid->uclass); struct video_ops *ops = video_get_ops(vid); - uint flags = 0; int ret; - /* Skip sync if manual-sync mode is active */ - if (uc_priv->manual_sync) - return 0; - - if (force) - flags |= VIDSYNC_FORCE; - - /* Check if sync should do full flush */ - if (!CONFIG_IS_ENABLED(CYCLIC) || force || - get_timer(priv->last_sync) >= CONFIG_VIDEO_SYNC_MS) - flags |= VIDSYNC_FLUSH; - - if (IS_ENABLED(CONFIG_VIDEO_COPY)) + if (IS_ENABLED(CONFIG_VIDEO_COPY) && (flags & VIDSYNC_COPY)) video_flush_copy(vid); if (ops && ops->sync) { @@ -534,7 +519,7 @@ int video_sync(struct udevice *vid, bool force) video_flush_dcache(vid, false); - if (IS_ENABLED(CONFIG_VIDEO_COPY)) + if (IS_ENABLED(CONFIG_VIDEO_COPY) && (flags & VIDSYNC_COPY)) video_flush_dcache(vid, true); #if defined(CONFIG_VIDEO_SANDBOX_SDL) @@ -555,6 +540,31 @@ int video_sync(struct udevice *vid, bool force) return 0; } +/* Flush video activity to the caches */ +int video_sync(struct udevice *vid, bool force) +{ + struct video_priv *priv = dev_get_uclass_priv(vid); + struct video_uc_priv *uc_priv = uclass_get_priv(vid->uclass); + uint flags = 0; + + /* Skip sync if manual-sync mode is active */ + if (uc_priv->manual_sync) + return 0; + + if (force) + flags |= VIDSYNC_FORCE; + + /* Check if sync should do full flush */ + if (!CONFIG_IS_ENABLED(CYCLIC) || force || + get_timer(priv->last_sync) >= CONFIG_VIDEO_SYNC_MS) + flags |= VIDSYNC_FLUSH; + + if (IS_ENABLED(CONFIG_VIDEO_COPY)) + flags |= VIDSYNC_COPY; + + return video_manual_sync(vid, flags); +} + void video_sync_all(void) { struct udevice *dev; diff --git a/include/video.h b/include/video.h index d08781d3a78..d8ba27ab8a7 100644 --- a/include/video.h +++ b/include/video.h @@ -80,10 +80,12 @@ enum video_format { * * @VIDSYNC_FORCE: Force sync even if recently synced or in manual-sync mode * @VIDSYNC_FLUSH: Flush dcache and perform full sync operations + * @VIDSYNC_COPY: Flush framebuffer to copy buffer */ enum video_sync_flags { VIDSYNC_FORCE = BIT(0), VIDSYNC_FLUSH = BIT(1), + VIDSYNC_COPY = BIT(2), }; /** @@ -341,6 +343,20 @@ int video_fill_part(struct udevice *dev, int xstart, int ystart, int xend, int video_draw_box(struct udevice *dev, int x0, int y0, int x1, int y1, int width, u32 colour, bool fill); +/** + * video_manual_sync() - Manually sync a device's frame buffer with its hardware + * + * @vid: Device to sync + * @flags: Flags for the sync (enum video_sync_flags) + * + * @return: 0 on success, error code otherwise + * + * Performs the actual sync operation with the provided flags. This is called + * by video_sync() after determining the appropriate flags, but can also be + * called directly when manual-sync mode is enabled. + */ +int video_manual_sync(struct udevice *vid, uint flags); + /** * video_sync() - Sync a device's frame buffer with its hardware * diff --git a/test/dm/video.c b/test/dm/video.c index cee9e528689..080297d8350 100644 --- a/test/dm/video.c +++ b/test/dm/video.c @@ -1342,6 +1342,33 @@ static int dm_test_video_manual_sync(struct unit_test_state *uts) ut_asserteq(183, video_compress_fb(uts, dev, true)); } + /* Now test video_manual_sync() directly with VIDSYNC_FORCE and COPY */ + ut_assertok(video_manual_sync(dev, VIDSYNC_FORCE | VIDSYNC_FLUSH | + VIDSYNC_COPY)); + ut_asserteq(183, video_compress_fb(uts, dev, false)); + + /* The copy framebuffer should now match since we forced the sync */ + ut_assertok(video_check_copy_fb(uts, dev)); + + /* Write new text again */ + vidconsole_put_string(con, "Test2"); + + /* without VIDSYNC_FLUSH or COPY - should do nothing */ + ut_assertok(video_manual_sync(dev, 0)); + + /* Copy fb should not match since neither flush nor copy occurred */ + if (IS_ENABLED(CONFIG_VIDEO_COPY)) { + ut_assertf(memcmp(priv->fb, priv->copy_fb, priv->fb_size), + "Copy fb shouldn't match fb w/o VIDSYNC_FLUSH/COPY"); + } + + /* video_manual_sync() with full flags - should perform full sync */ + ut_assertok(video_manual_sync(dev, VIDSYNC_FLUSH | VIDSYNC_COPY)); + ut_assertok(video_check_copy_fb(uts, dev)); + + /* Disable manual-sync mode */ + video_set_manual_sync(false); + return 0; } DM_TEST(dm_test_video_manual_sync, UTF_SCAN_PDATA | UTF_SCAN_FDT); -- 2.43.0
From: Simon Glass <sjg@chromium.org> Update sandbox_sdl_sync() to accept an optional damage rectangle and update only the damaged region. This should reduce the amount of work performed by the driver when only a small part of the display needs to be updated. Co-developed-by: Claude <noreply@anthropic.com> Signed-off-by: Simon Glass <sjg@chromium.org> --- arch/sandbox/cpu/sdl.c | 46 ++++++++++++++++++++++++++++++++++-------- 1 file changed, 38 insertions(+), 8 deletions(-) diff --git a/arch/sandbox/cpu/sdl.c b/arch/sandbox/cpu/sdl.c index fa431010b11..f4038c2431a 100644 --- a/arch/sandbox/cpu/sdl.c +++ b/arch/sandbox/cpu/sdl.c @@ -6,6 +6,7 @@ #include <errno.h> #include <mouse.h> #include <unistd.h> +#include <video_defs.h> #include <stdbool.h> #include <sysreset.h> #include <linux/input.h> @@ -232,17 +233,46 @@ int sandbox_sdl_init_display(int width, int height, int log2_bpp, return 0; } -static int copy_to_texture(void *lcd_base) +static int copy_to_texture(void *lcd_base, const struct video_bbox *damage) { char *dest; int pitch, x, y; int src_pitch; void *pixels; char *src; + SDL_Rect rect, *rectp = NULL; int ret; + int x0, y0, x1, y1; + + /* Set up damage region if provided */ + if (damage && damage->x1 > damage->x0 && damage->y1 > damage->y0) { + rect.x = damage->x0; + rect.y = damage->y0; + rect.w = damage->x1 - damage->x0; + rect.h = damage->y1 - damage->y0; + rectp = ▭ + x0 = damage->x0; + y0 = damage->y0; + x1 = damage->x1; + y1 = damage->y1; + } else { + x0 = 0; + y0 = 0; + x1 = sdl.width; + y1 = sdl.height; + } if (sdl.src_depth == sdl.depth) { - SDL_UpdateTexture(sdl.texture, NULL, lcd_base, sdl.pitch); + if (rectp) { + /* Update only the damaged region */ + src_pitch = sdl.width * sdl.src_depth / 8; + src = lcd_base + y0 * src_pitch + + x0 * sdl.src_depth / 8; + SDL_UpdateTexture(sdl.texture, rectp, src, src_pitch); + } else { + SDL_UpdateTexture(sdl.texture, NULL, lcd_base, + sdl.pitch); + } return 0; } @@ -255,7 +285,7 @@ static int copy_to_texture(void *lcd_base) return -EINVAL; } - ret = SDL_LockTexture(sdl.texture, NULL, &pixels, &pitch); + ret = SDL_LockTexture(sdl.texture, rectp, &pixels, &pitch); if (ret) { printf("SDL lock %d: %s\n", ret, SDL_GetError()); return ret; @@ -263,12 +293,12 @@ static int copy_to_texture(void *lcd_base) /* Copy the pixels one by one */ src_pitch = sdl.width * sdl.src_depth / 8; - for (y = 0; y < sdl.height; y++) { + for (y = y0; y < y1; y++) { char val; - dest = pixels + y * pitch; - src = lcd_base + src_pitch * y; - for (x = 0; x < sdl.width; x++, dest += 4) { + dest = pixels + (y - y0) * pitch; + src = lcd_base + src_pitch * y + x0; + for (x = x0; x < x1; x++, dest += 4) { val = *src++; dest[0] = val; dest[1] = val; @@ -289,7 +319,7 @@ int sandbox_sdl_sync(void *lcd_base) if (!sdl.texture) return 0; SDL_RenderClear(sdl.renderer); - ret = copy_to_texture(lcd_base); + ret = copy_to_texture(lcd_base, NULL); if (ret) { printf("copy_to_texture: %d: %s\n", ret, SDL_GetError()); return -EIO; -- 2.43.0
From: Simon Glass <sjg@chromium.org> At present, video_sync() has a special case for sandbox. Remove this and add a sync() method in the sandbox_sdl driver instead. The sync() method checks the VIDSYNC_FLUSH flag and skips sync operation if it's not set, avoiding unnecessary SDL updates. If CONFIG_VIDEO_DAMAGE is enabled, the SDL portion accepts an optional damage rectangle parameter to support only updating the damaged region. Use the struct video_bbox definition from the video_defs.h header as this avoids trying to include too many U-Boot headers in code that is build with system headers. Co-developed-by: Claude <noreply@anthropic.com> Signed-off-by: Simon Glass <sjg@chromium.org> --- arch/sandbox/cpu/sdl.c | 5 +++-- arch/sandbox/include/asm/sdl.h | 7 +++++-- drivers/video/sandbox_sdl.c | 19 +++++++++++++++++++ drivers/video/video-uclass.c | 4 ---- 4 files changed, 27 insertions(+), 8 deletions(-) diff --git a/arch/sandbox/cpu/sdl.c b/arch/sandbox/cpu/sdl.c index f4038c2431a..246fa37d457 100644 --- a/arch/sandbox/cpu/sdl.c +++ b/arch/sandbox/cpu/sdl.c @@ -12,6 +12,7 @@ #include <linux/input.h> #include <SDL2/SDL.h> #include <asm/state.h> +#include <video_defs.h> /** * struct buf_info - a data buffer holding audio data @@ -311,7 +312,7 @@ static int copy_to_texture(void *lcd_base, const struct video_bbox *damage) return 0; } -int sandbox_sdl_sync(void *lcd_base) +int sandbox_sdl_sync(void *lcd_base, const struct video_bbox *damage) { struct SDL_Rect rect; int ret; @@ -319,7 +320,7 @@ int sandbox_sdl_sync(void *lcd_base) if (!sdl.texture) return 0; SDL_RenderClear(sdl.renderer); - ret = copy_to_texture(lcd_base, NULL); + ret = copy_to_texture(lcd_base, damage); if (ret) { printf("copy_to_texture: %d: %s\n", ret, SDL_GetError()); return -EIO; diff --git a/arch/sandbox/include/asm/sdl.h b/arch/sandbox/include/asm/sdl.h index 735652de6bd..93d934f9c54 100644 --- a/arch/sandbox/include/asm/sdl.h +++ b/arch/sandbox/include/asm/sdl.h @@ -10,6 +10,7 @@ #include <video.h> struct mouse_event; +struct video_bbox; #ifdef CONFIG_SANDBOX_SDL @@ -42,9 +43,10 @@ int sandbox_sdl_remove_display(void); * user can see it. * * @lcd_base: Base of frame buffer + * @damage: Optional damage rectangle to limit the update region (may be NULL) * Return: 0 if screen was updated, -ENODEV is there is no screen. */ -int sandbox_sdl_sync(void *lcd_base); +int sandbox_sdl_sync(void *lcd_base, const struct video_bbox *damage); /** * sandbox_sdl_scan_keys() - scan for pressed keys @@ -129,7 +131,8 @@ static inline int sandbox_sdl_remove_display(void) return -ENODEV; } -static inline int sandbox_sdl_sync(void *lcd_base) +static inline int sandbox_sdl_sync(void *lcd_base, + const struct video_bbox *damage) { return -ENODEV; } diff --git a/drivers/video/sandbox_sdl.c b/drivers/video/sandbox_sdl.c index 69dfa930273..67b4b6c7911 100644 --- a/drivers/video/sandbox_sdl.c +++ b/drivers/video/sandbox_sdl.c @@ -127,6 +127,24 @@ static int sandbox_sdl_bind(struct udevice *dev) return ret; } +static int sandbox_sdl_video_sync(struct udevice *vid, uint flags) +{ + struct video_priv *priv = dev_get_uclass_priv(vid); + const struct video_bbox *damage = NULL; + + if (!(flags & VIDSYNC_FLUSH)) + return 0; + + if (IS_ENABLED(CONFIG_VIDEO_DAMAGE)) + damage = &priv->damage; + + return sandbox_sdl_sync(priv->fb, damage); +} + +static const struct video_ops sandbox_sdl_ops = { + .sync = sandbox_sdl_video_sync, +}; + static const struct udevice_id sandbox_sdl_ids[] = { { .compatible = "sandbox,lcd-sdl" }, { } @@ -139,5 +157,6 @@ U_BOOT_DRIVER(sandbox_lcd_sdl) = { .bind = sandbox_sdl_bind, .probe = sandbox_sdl_probe, .remove = sandbox_sdl_remove, + .ops = &sandbox_sdl_ops, .plat_auto = sizeof(struct sandbox_sdl_plat), }; diff --git a/drivers/video/video-uclass.c b/drivers/video/video-uclass.c index a8849cf6289..491dd23bff0 100644 --- a/drivers/video/video-uclass.c +++ b/drivers/video/video-uclass.c @@ -522,10 +522,6 @@ int video_manual_sync(struct udevice *vid, uint flags) if (IS_ENABLED(CONFIG_VIDEO_COPY) && (flags & VIDSYNC_COPY)) video_flush_dcache(vid, true); -#if defined(CONFIG_VIDEO_SANDBOX_SDL) - /* to see the copy framebuffer, use priv->copy_fb */ - sandbox_sdl_sync(priv->fb); -#endif priv->last_sync = get_timer(0); if (IS_ENABLED(CONFIG_VIDEO_DAMAGE)) { -- 2.43.0
From: Simon Glass <sjg@chromium.org> Add a test that verifies the sandbox SDL driver receives the correct damage rectangle in its sync() method. The test: - Clears the display and verifies full-screen damage is passed to sync() - Writes text at a specific position and verifies smaller damage region - Draws a box and verifies the damage matches the box dimensions The damage rectangle is recorded in sandbox_sdl_plat.last_sync_damage and can be retrieved using sandbox_sdl_get_sync_damage() for testing purposes. Co-developed-by: Claude <noreply@anthropic.com> Signed-off-by: Simon Glass <sjg@chromium.org> --- drivers/video/sandbox_sdl.c | 23 +++++++-- include/dm/test.h | 15 ++++++ test/dm/video.c | 96 +++++++++++++++++++++++++++++++++++++ 3 files changed, 131 insertions(+), 3 deletions(-) diff --git a/drivers/video/sandbox_sdl.c b/drivers/video/sandbox_sdl.c index 67b4b6c7911..fc5843a2822 100644 --- a/drivers/video/sandbox_sdl.c +++ b/drivers/video/sandbox_sdl.c @@ -129,22 +129,39 @@ static int sandbox_sdl_bind(struct udevice *dev) static int sandbox_sdl_video_sync(struct udevice *vid, uint flags) { - struct video_priv *priv = dev_get_uclass_priv(vid); + struct sandbox_sdl_plat *plat = dev_get_plat(vid); + struct video_priv *uc_priv = dev_get_uclass_priv(vid); const struct video_bbox *damage = NULL; if (!(flags & VIDSYNC_FLUSH)) return 0; if (IS_ENABLED(CONFIG_VIDEO_DAMAGE)) - damage = &priv->damage; + damage = &uc_priv->damage; - return sandbox_sdl_sync(priv->fb, damage); + /* Record the damage box for testing */ + if (damage) + plat->last_sync_damage = *damage; + else + memset(&plat->last_sync_damage, '\0', + sizeof(plat->last_sync_damage)); + + return sandbox_sdl_sync(uc_priv->fb, damage); } static const struct video_ops sandbox_sdl_ops = { .sync = sandbox_sdl_video_sync, }; +int sandbox_sdl_get_sync_damage(struct udevice *dev, struct video_bbox *damage) +{ + struct sandbox_sdl_plat *plat = dev_get_plat(dev); + + *damage = plat->last_sync_damage; + + return 0; +} + static const struct udevice_id sandbox_sdl_ids[] = { { .compatible = "sandbox,lcd-sdl" }, { } diff --git a/include/dm/test.h b/include/dm/test.h index 4aabb4603b9..10fd63e759f 100644 --- a/include/dm/test.h +++ b/include/dm/test.h @@ -7,6 +7,7 @@ #define __DM_TEST_H #include <linux/types.h> +#include <video_defs.h> struct udevice; @@ -157,6 +158,7 @@ extern struct unit_test_state global_dm_test_state; * 2=upside down, 3=90 degree counterclockwise) * @vidconsole_drv_name: Name of video console driver (set by tests) * @font_size: Console font size to select (set by tests) + * @last_sync_damage: Last damage rectangle passed to sync() method (for testing) */ struct sandbox_sdl_plat { int xres; @@ -165,6 +167,7 @@ struct sandbox_sdl_plat { int rot; const char *vidconsole_drv_name; int font_size; + struct video_bbox last_sync_damage; }; /** @@ -232,4 +235,16 @@ void dm_leak_check_start(struct unit_test_state *uts); * @dms: Overall test state */int dm_leak_check_end(struct unit_test_state *uts); +/** + * sandbox_sdl_get_sync_damage() - Get the last damage rect passed to sync() + * + * This is used for testing to verify that the correct damage rectangle was + * passed to the driver's sync() method. + * + * @dev: Video device + * @damage: Returns the last damage rectangle + * Return: 0 if OK, -ve on error + */ +int sandbox_sdl_get_sync_damage(struct udevice *dev, struct video_bbox *damage); + #endif diff --git a/test/dm/video.c b/test/dm/video.c index 080297d8350..5a5d8053856 100644 --- a/test/dm/video.c +++ b/test/dm/video.c @@ -1372,3 +1372,99 @@ static int dm_test_video_manual_sync(struct unit_test_state *uts) return 0; } DM_TEST(dm_test_video_manual_sync, UTF_SCAN_PDATA | UTF_SCAN_FDT); + +/* Test that sync() receives the correct damage rectangle */ +static int dm_test_video_sync_damage(struct unit_test_state *uts) +{ + struct video_bbox damage; + struct udevice *dev, *con; + struct video_priv *priv; + + if (!IS_ENABLED(CONFIG_VIDEO_DAMAGE)) + return -EAGAIN; + + ut_assertok(select_vidconsole(uts, "vidconsole0")); + ut_assertok(video_get_nologo(uts, &dev)); + ut_assertok(uclass_get_device(UCLASS_VIDEO_CONSOLE, 0, &con)); + ut_assertok(vidconsole_select_font(con, "8x16", 0)); + priv = dev_get_uclass_priv(dev); + + /* Use manual sync to prevent interference with the test */ + video_set_manual_sync(true); + + /* Clear the display - this creates a full-screen damage and syncs */ + video_clear(dev); + ut_assertok(video_manual_sync(dev, VIDSYNC_FLUSH | VIDSYNC_COPY)); + ut_asserteq(46, video_compress_fb(uts, dev, false)); + + /* Get the damage rectangle that was passed to sync() */ + ut_assertok(sandbox_sdl_get_sync_damage(dev, &damage)); + + /* Should be the full screen */ + ut_assert(video_bbox_valid(&damage)); + ut_asserteq(0, damage.x0); + ut_asserteq(0, damage.y0); + ut_asserteq(priv->xsize, damage.x1); + ut_asserteq(priv->ysize, damage.y1); + + /* Sync again with no changes - should have empty damage */ + ut_assertok(video_manual_sync(dev, VIDSYNC_FLUSH | VIDSYNC_COPY)); + ut_assertok(sandbox_sdl_get_sync_damage(dev, &damage)); + ut_assert(!video_bbox_valid(&damage)); + + /* Check that priv->damage is still reset to empty */ + ut_assert(!video_bbox_valid(&priv->damage)); + + /* Write a small piece of text at a specific position */ + vidconsole_putc_xy(con, VID_TO_POS(400), 67, 'T'); + + /* Check priv->damage before sync - should have text damage */ + ut_assert(video_bbox_valid(&priv->damage)); + ut_asserteq(400, priv->damage.x0); + ut_asserteq(67, priv->damage.y0); + ut_asserteq(400 + 8, priv->damage.x1); /* 8x16 font */ + ut_asserteq(67 + 16, priv->damage.y1); + + ut_assertok(video_manual_sync(dev, VIDSYNC_FLUSH | VIDSYNC_COPY)); + + /* Get the damage rectangle that was passed to sync() */ + ut_assertok(sandbox_sdl_get_sync_damage(dev, &damage)); + + /* The damage should cover just the character */ + ut_assert(video_bbox_valid(&damage)); + ut_asserteq(400, damage.x0); + ut_asserteq(67, damage.y0); + ut_asserteq(400 + 8, damage.x1); + ut_asserteq(67 + 16, damage.y1); + + /* Check priv->damage after sync - should be reset to empty */ + ut_assert(!video_bbox_valid(&priv->damage)); + + /* Draw a filled box at a different position */ + ut_assertok(video_draw_box(dev, 200, 300, 250, 340, 1, 0xffffff, true)); + + /* Check priv->damage before sync - should have box damage */ + ut_assert(video_bbox_valid(&priv->damage)); + ut_asserteq(200, priv->damage.x0); + ut_asserteq(300, priv->damage.y0); + ut_asserteq(250, priv->damage.x1); + ut_asserteq(340, priv->damage.y1); + + ut_assertok(video_manual_sync(dev, VIDSYNC_FLUSH | VIDSYNC_COPY)); + + /* Get the damage rectangle for the box */ + ut_assertok(sandbox_sdl_get_sync_damage(dev, &damage)); + + /* The damage should cover the box area */ + ut_assert(video_bbox_valid(&damage)); + ut_asserteq(200, damage.x0); + ut_asserteq(300, damage.y0); + ut_asserteq(250, damage.x1); + ut_asserteq(340, damage.y1); + + /* Check priv->damage after sync - should be reset to inverted/empty */ + ut_assert(!video_bbox_valid(&priv->damage)); + + return 0; +} +DM_TEST(dm_test_video_sync_damage, UTF_SCAN_PDATA | UTF_SCAN_FDT); -- 2.43.0
participants (1)
-
Simon Glass