Merge tag 'video-next-20211228' of https://source.denx.de/u-boot/custodians/u-boot-video into next

- various fixes to the sandbox display support - support for showing a logo without splash screen config - support for BMP drawing to depths other than 16bpp - tests for the different types of supported BMP images - support showing a logo when running coreboot via qemu
2021-12-28 11:28:31 -05:00
parent 111a8b5735 92302ab1a2
commit 87a9aa604d
106 changed files with 687 additions and 547 deletions
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -14,6 +14,17 @@ config DM_VIDEO
 	  option compiles in the video uclass and routes all LCD/video access
 	  through this.

+config VIDEO_LOGO
+	bool "Show the U-Boot logo on the display"
+	depends on DM_VIDEO
+	default y if !SPLASH_SCREEN
+	select VIDEO_BMP_RLE8
+	help
+	  This enables showing the U-Boot logo on the display when a video
+	  device is probed. It appears at the top right. The logo itself is at
+	  tools/logos/u-boot_logo.bmp and looks best when the display has a
+	  black background.
+
 config BACKLIGHT
 	bool "Enable panel backlight uclass support"
 	depends on DM_VIDEO
--- a/drivers/video/Makefile
+++ b/drivers/video/Makefile
@@ -17,6 +17,9 @@ obj-$(CONFIG_DM_VIDEO) += video_bmp.o
 obj-$(CONFIG_PANEL) += panel-uclass.o
 obj-$(CONFIG_DM_PANEL_HX8238D) += hx8238d.o
 obj-$(CONFIG_SIMPLE_PANEL) += simple_panel.o
+
+obj-$(CONFIG_VIDEO_LOGO) += u_boot_logo.o
+
 endif

 obj-${CONFIG_EXYNOS_FB} += exynos/
--- a/drivers/video/cfb_console.c
+++ b/drivers/video/cfb_console.c
@@ -42,11 +42,6 @@
 * VIDEO_TSTC_FCT	      - keyboard_tstc function
 * VIDEO_GETC_FCT	      - keyboard_getc function
 *
- * CONFIG_VIDEO_LOGO	      - display Linux Logo in upper left corner.
- *				Use CONFIG_SPLASH_SCREEN_ALIGN with
- *				environment variable "splashpos" to place
- *				the logo on other position. In this case
- *				no CONSOLE_EXTRA_INFO is possible.
 * CONFIG_VIDEO_BMP_LOGO      - use bmp_logo instead of linux_logo
 * CONFIG_CONSOLE_EXTRA_INFO  - display additional board information
 *				strings that normaly goes to serial
@@ -127,34 +122,6 @@ void console_cursor(int state);
 #define CURSOR_SET video_set_cursor()
 #endif /* CONFIG_VIDEO_SW_CURSOR */

-#ifdef	CONFIG_VIDEO_LOGO
-#ifdef	CONFIG_VIDEO_BMP_LOGO
-#include <bmp_logo.h>
-#include <bmp_logo_data.h>
-#define VIDEO_LOGO_WIDTH	BMP_LOGO_WIDTH
-#define VIDEO_LOGO_HEIGHT	BMP_LOGO_HEIGHT
-#define VIDEO_LOGO_LUT_OFFSET	BMP_LOGO_OFFSET
-#define VIDEO_LOGO_COLORS	BMP_LOGO_COLORS
-
-#else  /* CONFIG_VIDEO_BMP_LOGO */
-#define LINUX_LOGO_WIDTH	80
-#define LINUX_LOGO_HEIGHT	80
-#define LINUX_LOGO_COLORS	214
-#define LINUX_LOGO_LUT_OFFSET	0x20
-#define __initdata
-#include <linux_logo.h>
-#define VIDEO_LOGO_WIDTH	LINUX_LOGO_WIDTH
-#define VIDEO_LOGO_HEIGHT	LINUX_LOGO_HEIGHT
-#define VIDEO_LOGO_LUT_OFFSET	LINUX_LOGO_LUT_OFFSET
-#define VIDEO_LOGO_COLORS	LINUX_LOGO_COLORS
-#endif /* CONFIG_VIDEO_BMP_LOGO */
-#define VIDEO_INFO_X		(VIDEO_LOGO_WIDTH)
-#define VIDEO_INFO_Y		(VIDEO_FONT_HEIGHT/2)
-#else  /* CONFIG_VIDEO_LOGO */
-#define VIDEO_LOGO_WIDTH	0
-#define VIDEO_LOGO_HEIGHT	0
-#endif /* CONFIG_VIDEO_LOGO */
-
 #define VIDEO_COLS		VIDEO_VISIBLE_COLS
 #define VIDEO_ROWS		VIDEO_VISIBLE_ROWS
 #ifndef VIDEO_LINE_LEN
@@ -163,11 +130,7 @@ void console_cursor(int state);
 #define VIDEO_SIZE		(VIDEO_ROWS * VIDEO_LINE_LEN)
 #define VIDEO_BURST_LEN		(VIDEO_COLS/8)

-#ifdef	CONFIG_VIDEO_LOGO
-#define CONSOLE_ROWS		((VIDEO_ROWS - video_logo_height) / VIDEO_FONT_HEIGHT)
-#else
 #define CONSOLE_ROWS		(VIDEO_ROWS / VIDEO_FONT_HEIGHT)
-#endif

 #define CONSOLE_COLS		(VIDEO_COLS / VIDEO_FONT_WIDTH)
 #define CONSOLE_ROW_SIZE	(VIDEO_FONT_HEIGHT * VIDEO_LINE_LEN)
@@ -214,7 +177,7 @@ static GraphicDevice *pGD;	/* Pointer to Graphic array */
 static void *video_fb_address;	/* frame buffer address */
 static void *video_console_address;	/* console buffer start address */

-static int video_logo_height = VIDEO_LOGO_HEIGHT;
+static int video_logo_height;		/* not supported anymore */

 static int __maybe_unused cursor_state;
 static int __maybe_unused old_col;
@@ -1670,292 +1633,6 @@ int video_display_bitmap(ulong bmp_image, int x, int y)
 }
 #endif

-
-#ifdef CONFIG_VIDEO_LOGO
-static int video_logo_xpos;
-static int video_logo_ypos;
-
-static void plot_logo_or_black(void *screen, int x, int y, int black);
-
-static void logo_plot(void *screen, int x, int y)
-{
-	plot_logo_or_black(screen, x, y, 0);
-}
-
-static void logo_black(void)
-{
-	plot_logo_or_black(video_fb_address, video_logo_xpos, video_logo_ypos,
-			1);
-}
-
-static int do_clrlogo(struct cmd_tbl *cmdtp, int flag, int argc,
-		      char *const argv[])
-{
-	if (argc != 1)
-		return cmd_usage(cmdtp);
-
-	logo_black();
-	return 0;
-}
-
-U_BOOT_CMD(
-	   clrlogo, 1, 0, do_clrlogo,
-	   "fill the boot logo area with black",
-	   " "
-	   );
-
-static void plot_logo_or_black(void *screen, int x, int y, int black)
-{
-
-	int xcount, i;
-	int skip = VIDEO_LINE_LEN - VIDEO_LOGO_WIDTH * VIDEO_PIXEL_SIZE;
-	int ycount = video_logo_height;
-	unsigned char r, g, b, *logo_red, *logo_blue, *logo_green;
-	unsigned char *source;
-	unsigned char *dest;
-
-#ifdef CONFIG_SPLASH_SCREEN_ALIGN
-	if (x == BMP_ALIGN_CENTER)
-		x = max(0, (int)(VIDEO_VISIBLE_COLS - VIDEO_LOGO_WIDTH) / 2);
-	else if (x < 0)
-		x = max(0, (int)(VIDEO_VISIBLE_COLS - VIDEO_LOGO_WIDTH + x + 1));
-
-	if (y == BMP_ALIGN_CENTER)
-		y = max(0, (int)(VIDEO_VISIBLE_ROWS - VIDEO_LOGO_HEIGHT) / 2);
-	else if (y < 0)
-		y = max(0, (int)(VIDEO_VISIBLE_ROWS - VIDEO_LOGO_HEIGHT + y + 1));
-#endif /* CONFIG_SPLASH_SCREEN_ALIGN */
-
-	dest = (unsigned char *)screen + y * VIDEO_LINE_LEN + x * VIDEO_PIXEL_SIZE;
-
-#ifdef CONFIG_VIDEO_BMP_LOGO
-	source = bmp_logo_bitmap;
-
-	/* Allocate temporary space for computing colormap */
-	logo_red = malloc(BMP_LOGO_COLORS);
-	logo_green = malloc(BMP_LOGO_COLORS);
-	logo_blue = malloc(BMP_LOGO_COLORS);
-	/* Compute color map */
-	for (i = 0; i < VIDEO_LOGO_COLORS; i++) {
-		logo_red[i] = (bmp_logo_palette[i] & 0x0f00) >> 4;
-		logo_green[i] = (bmp_logo_palette[i] & 0x00f0);
-		logo_blue[i] = (bmp_logo_palette[i] & 0x000f) << 4;
-	}
-#else
-	source = linux_logo;
-	logo_red = linux_logo_red;
-	logo_green = linux_logo_green;
-	logo_blue = linux_logo_blue;
-#endif
-
-	if (VIDEO_DATA_FORMAT == GDF__8BIT_INDEX) {
-		for (i = 0; i < VIDEO_LOGO_COLORS; i++) {
-			video_set_lut(i + VIDEO_LOGO_LUT_OFFSET,
-				      logo_red[i], logo_green[i],
-				      logo_blue[i]);
-		}
-	}
-
-	while (ycount--) {
-#if defined(VIDEO_FB_16BPP_PIXEL_SWAP)
-		int xpos = x;
-#endif
-		xcount = VIDEO_LOGO_WIDTH;
-		while (xcount--) {
-			if (black) {
-				r = 0x00;
-				g = 0x00;
-				b = 0x00;
-			} else {
-				r = logo_red[*source - VIDEO_LOGO_LUT_OFFSET];
-				g = logo_green[*source - VIDEO_LOGO_LUT_OFFSET];
-				b = logo_blue[*source - VIDEO_LOGO_LUT_OFFSET];
-			}
-
-			switch (VIDEO_DATA_FORMAT) {
-			case GDF__8BIT_INDEX:
-				*dest = *source;
-				break;
-			case GDF__8BIT_332RGB:
-				*dest = ((r >> 5) << 5) |
-					((g >> 5) << 2) |
-					 (b >> 6);
-				break;
-			case GDF_15BIT_555RGB:
-#if defined(VIDEO_FB_16BPP_PIXEL_SWAP)
-				fill_555rgb_pswap(dest, xpos++, r, g, b);
-#else
-				*(unsigned short *) dest =
-					SWAP16((unsigned short) (
-							((r >> 3) << 10) |
-							((g >> 3) <<  5) |
-							 (b >> 3)));
-#endif
-				break;
-			case GDF_16BIT_565RGB:
-				*(unsigned short *) dest =
-					SWAP16((unsigned short) (
-							((r >> 3) << 11) |
-							((g >> 2) <<  5) |
-							 (b >> 3)));
-				break;
-			case GDF_32BIT_X888RGB:
-				*(u32 *) dest =
-					SWAP32((u32) (
-							(r << 16) |
-							(g <<  8) |
-							 b));
-				break;
-			case GDF_24BIT_888RGB:
-#ifdef VIDEO_FB_LITTLE_ENDIAN
-				dest[0] = b;
-				dest[1] = g;
-				dest[2] = r;
-#else
-				dest[0] = r;
-				dest[1] = g;
-				dest[2] = b;
-#endif
-				break;
-			}
-			source++;
-			dest += VIDEO_PIXEL_SIZE;
-		}
-		dest += skip;
-	}
-#ifdef CONFIG_VIDEO_BMP_LOGO
-	free(logo_red);
-	free(logo_green);
-	free(logo_blue);
-#endif
-}
-
-static void *video_logo(void)
-{
-	char info[128];
-	__maybe_unused int y_off = 0;
-	__maybe_unused ulong addr;
-	__maybe_unused char *s;
-	__maybe_unused int len, ret, space;
-
-	splash_get_pos(&video_logo_xpos, &video_logo_ypos);
-
-#ifdef CONFIG_SPLASH_SCREEN
-	s = env_get("splashimage");
-	if (s != NULL) {
-		ret = splash_screen_prepare();
-		if (ret < 0)
-			return video_fb_address;
-		addr = hextoul(s, NULL);
-
-		if (video_display_bitmap(addr,
-					video_logo_xpos,
-					video_logo_ypos) == 0) {
-			video_logo_height = 0;
-			return ((void *) (video_fb_address));
-		}
-	}
-#endif /* CONFIG_SPLASH_SCREEN */
-
-	logo_plot(video_fb_address, video_logo_xpos, video_logo_ypos);
-
-#ifdef CONFIG_SPLASH_SCREEN_ALIGN
-	/*
-	 * when using splashpos for video_logo, skip any info
-	 * output on video console if the logo is not at 0,0
-	 */
-	if (video_logo_xpos || video_logo_ypos) {
-		/*
-		 * video_logo_height is used in text and cursor offset
-		 * calculations. Since the console is below the logo,
-		 * we need to adjust the logo height
-		 */
-		if (video_logo_ypos == BMP_ALIGN_CENTER)
-			video_logo_height += max(0, (int)(VIDEO_VISIBLE_ROWS -
-						     VIDEO_LOGO_HEIGHT) / 2);
-		else if (video_logo_ypos > 0)
-			video_logo_height += video_logo_ypos;
-
-		return video_fb_address + video_logo_height * VIDEO_LINE_LEN;
-	}
-#endif
-	if (board_cfb_skip())
-		return 0;
-
-	sprintf(info, " %s", version_string);
-
-#ifndef CONFIG_HIDE_LOGO_VERSION
-	space = (VIDEO_COLS - VIDEO_INFO_X) / VIDEO_FONT_WIDTH;
-	len = strlen(info);
-
-	if (len > space) {
-		int xx = VIDEO_INFO_X, yy = VIDEO_INFO_Y;
-		uchar *p = (uchar *) info;
-
-		while (len) {
-			if (len > space) {
-				video_drawchars(xx, yy, p, space);
-				len -= space;
-
-				p = (uchar *)p + space;
-
-				if (!y_off) {
-					xx += VIDEO_FONT_WIDTH;
-					space--;
-				}
-				yy += VIDEO_FONT_HEIGHT;
-
-				y_off++;
-			} else {
-				video_drawchars(xx, yy, p, len);
-				len = 0;
-			}
-		}
-	} else
-		video_drawstring(VIDEO_INFO_X, VIDEO_INFO_Y, (uchar *) info);
-
-#ifdef CONFIG_CONSOLE_EXTRA_INFO
-	{
-		int i, n =
-			((video_logo_height -
-			  VIDEO_FONT_HEIGHT) / VIDEO_FONT_HEIGHT);
-
-		for (i = 1; i < n; i++) {
-			video_get_info_str(i, info);
-			if (!*info)
-				continue;
-
-			len = strlen(info);
-			if (len > space) {
-				video_drawchars(VIDEO_INFO_X,
-						VIDEO_INFO_Y +
-						(i + y_off) *
-							VIDEO_FONT_HEIGHT,
-						(uchar *) info, space);
-				y_off++;
-				video_drawchars(VIDEO_INFO_X +
-						VIDEO_FONT_WIDTH,
-						VIDEO_INFO_Y +
-							(i + y_off) *
-							VIDEO_FONT_HEIGHT,
-						(uchar *) info + space,
-						len - space);
-			} else {
-				video_drawstring(VIDEO_INFO_X,
-						 VIDEO_INFO_Y +
-						 (i + y_off) *
-							VIDEO_FONT_HEIGHT,
-						 (uchar *) info);
-			}
-		}
-	}
-#endif
-#endif
-
-	return (video_fb_address + video_logo_height * VIDEO_LINE_LEN);
-}
-#endif
-
 static int cfb_fb_is_in_dram(void)
 {
 	struct bd_info *bd = gd->bd;
--- a/drivers/video/console_truetype.c
+++ b/drivers/video/console_truetype.c
@@ -274,6 +274,27 @@ static int console_truetype_putc_xy(struct udevice *dev, uint x, uint y,
 	 */
 	for (row = 0; row < height; row++) {
 		switch (vid_priv->bpix) {
+		case VIDEO_BPP8:
+			if (IS_ENABLED(CONFIG_VIDEO_BPP8)) {
+				u8 *dst = line + xoff;
+				int i;
+
+				for (i = 0; i < width; i++) {
+					int val = *bits;
+					int out;
+
+					if (vid_priv->colour_bg)
+						val = 255 - val;
+					out = val;
+					if (vid_priv->colour_fg)
+						*dst++ |= out;
+					else
+						*dst++ &= out;
+					bits++;
+				}
+				end = dst;
+			}
+			break;
 #ifdef CONFIG_VIDEO_BPP16
 		case VIDEO_BPP16: {
 			uint16_t *dst = (uint16_t *)line + xoff;
--- a/drivers/video/sandbox_sdl.c
+++ b/drivers/video/sandbox_sdl.c
@@ -12,6 +12,7 @@
 #include <asm/sdl.h>
 #include <asm/state.h>
 #include <asm/u-boot-sandbox.h>
+#include <dm/device-internal.h>
 #include <dm/test.h>

 DECLARE_GLOBAL_DATA_PTR;
@@ -43,27 +44,86 @@ static int sandbox_sdl_probe(struct udevice *dev)
 	uc_priv->vidconsole_drv_name = plat->vidconsole_drv_name;
 	uc_priv->font_size = plat->font_size;
 	if (IS_ENABLED(CONFIG_VIDEO_COPY))
-		uc_plat->copy_base = uc_plat->base - uc_plat->size / 2;
+		uc_plat->copy_base = uc_plat->base + uc_plat->size / 2;

 	return 0;
 }

+static void set_bpp(struct udevice *dev, enum video_log2_bpp l2bpp)
+{
+	struct video_uc_plat *uc_plat = dev_get_uclass_plat(dev);
+	struct sandbox_sdl_plat *plat = dev_get_plat(dev);
+
+	plat->bpix = l2bpp;
+
+	uc_plat->size = plat->xres * plat->yres * VNBYTES(plat->bpix);
+
+	/*
+	 * Set up to the maximum size we'll ever need. This is a strange case.
+	 * The video memory is allocated by video_post_bind() called from
+	 * board_init_r(). If a test changes the reoslution so it needs more
+	 * memory later (with sandbox_sdl_set_bpp()), it is too late to make
+	 * the frame buffer larger.
+	 *
+	 * So use a maximum size here.
+	 */
+	uc_plat->size = max(uc_plat->size, 1920U * 1080 * VNBYTES(VIDEO_BPP32));
+
+	/* Allow space for two buffers, the lower one being the copy buffer */
+	log_debug("Frame buffer size %x\n", uc_plat->size);
+
+	/*
+	 * If a copy framebuffer is used, double the size and use the last half
+	 * as the copy, with the first half as the normal frame buffer.
+	 */
+	if (IS_ENABLED(CONFIG_VIDEO_COPY))
+		uc_plat->size *= 2;
+}
+
+int sandbox_sdl_set_bpp(struct udevice *dev, enum video_log2_bpp l2bpp)
+{
+	struct video_uc_plat *uc_plat = dev_get_uclass_plat(dev);
+	int ret;
+
+	if (device_active(dev))
+		return -EINVAL;
+	sandbox_sdl_remove_display();
+
+	uc_plat->hide_logo = true;
+	set_bpp(dev, l2bpp);
+
+	ret = device_probe(dev);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int sandbox_sdl_remove(struct udevice *dev)
+{
+	/*
+	 * Removing the display it a bit annoying when running unit tests, since
+	 * they remove all devices. It is nice to be able to see what the test
+	 * wrote onto the display. So this comment is just here to show how to
+	 * do it, if we want to make it optional one day.
+	 *
+	 * sandbox_sdl_remove_display();
+	 */
+	return 0;
+}
+
 static int sandbox_sdl_bind(struct udevice *dev)
 {
-	struct video_uc_plat *uc_plat = dev_get_uclass_plat(dev);
 	struct sandbox_sdl_plat *plat = dev_get_plat(dev);
+	enum video_log2_bpp l2bpp;
 	int ret = 0;

 	plat->xres = dev_read_u32_default(dev, "xres", LCD_MAX_WIDTH);
 	plat->yres = dev_read_u32_default(dev, "yres", LCD_MAX_HEIGHT);
-	plat->bpix = dev_read_u32_default(dev, "log2-depth", VIDEO_BPP16);
+	l2bpp = dev_read_u32_default(dev, "log2-depth", VIDEO_BPP16);
 	plat->rot = dev_read_u32_default(dev, "rotate", 0);
-	uc_plat->size = plat->xres * plat->yres * (1 << plat->bpix) / 8;

-	/* Allow space for two buffers, the lower one being the copy buffer */
-	log_debug("Frame buffer size %x\n", uc_plat->size);
-	if (IS_ENABLED(CONFIG_VIDEO_COPY))
-		uc_plat->size *= 2;
+	set_bpp(dev, l2bpp);

 	return ret;
 }
@@ -79,5 +139,6 @@ U_BOOT_DRIVER(sandbox_lcd_sdl) = {
 	.of_match = sandbox_sdl_ids,
 	.bind	= sandbox_sdl_bind,
 	.probe	= sandbox_sdl_probe,
+	.remove	= sandbox_sdl_remove,
 	.plat_auto	= sizeof(struct sandbox_sdl_plat),
 };
--- a/drivers/video/u_boot_logo.bmp
+++ b/drivers/video/u_boot_logo.bmp
--- a/drivers/video/video-uclass.c
+++ b/drivers/video/video-uclass.c
@@ -279,10 +279,10 @@ int video_sync_copy(struct udevice *dev, void *from, void *to)
 		 */
 		if (offset < -priv->fb_size || offset > 2 * priv->fb_size) {
 #ifdef DEBUG
-			char str[80];
+			char str[120];

 			snprintf(str, sizeof(str),
-				 "[sync_copy fb=%p, from=%p, to=%p, offset=%lx]",
+				 "[** FAULT sync_copy fb=%p, from=%p, to=%p, offset=%lx]",
 				 priv->fb, from, to, offset);
 			console_puts_select_stderr(true, str);
 #endif
@@ -319,23 +319,20 @@ int video_sync_copy_all(struct udevice *dev)

 #endif

-/* Set up the colour map */
-static int video_pre_probe(struct udevice *dev)
+#define SPLASH_DECL(_name) \
+	extern u8 __splash_ ## _name ## _begin[]; \
+	extern u8 __splash_ ## _name ## _end[]
+
+#define SPLASH_START(_name)	__splash_ ## _name ## _begin
+
+SPLASH_DECL(u_boot_logo);
+
+static int show_splash(struct udevice *dev)
 {
-	struct video_priv *priv = dev_get_uclass_priv(dev);
+	u8 *data = SPLASH_START(u_boot_logo);
+	int ret;

-	priv->cmap = calloc(256, sizeof(ushort));
-	if (!priv->cmap)
-		return -ENOMEM;
-
-	return 0;
-}
-
-static int video_pre_remove(struct udevice *dev)
-{
-	struct video_priv *priv = dev_get_uclass_priv(dev);
-
-	free(priv->cmap);
+	ret = video_bmp_display(dev, map_to_sysmem(data), -4, 4, true);

 	return 0;
 }
@@ -405,6 +402,14 @@ static int video_post_probe(struct udevice *dev)
 		return ret;
 	}

+	if (IS_ENABLED(CONFIG_VIDEO_LOGO) && !plat->hide_logo) {
+		ret = show_splash(dev);
+		if (ret) {
+			log_debug("Cannot show splash screen\n");
+			return ret;
+		}
+	}
+
 	return 0;
 };

@@ -447,9 +452,7 @@ UCLASS_DRIVER(video) = {
 	.name		= "video",
 	.flags		= DM_UC_FLAG_SEQ_ALIAS,
 	.post_bind	= video_post_bind,
-	.pre_probe	= video_pre_probe,
 	.post_probe	= video_post_probe,
-	.pre_remove	= video_pre_remove,
 	.priv_auto	= sizeof(struct video_uc_priv),
 	.per_device_auto	= sizeof(struct video_priv),
 	.per_device_plat_auto	= sizeof(struct video_uc_plat),
--- a/drivers/video/video_bmp.c
+++ b/drivers/video/video_bmp.c
@@ -13,34 +13,90 @@
 #include <watchdog.h>
 #include <asm/unaligned.h>

-#ifdef CONFIG_VIDEO_BMP_RLE8
 #define BMP_RLE8_ESCAPE		0
 #define BMP_RLE8_EOL		0
 #define BMP_RLE8_EOBMP		1
 #define BMP_RLE8_DELTA		2

-static void draw_unencoded_bitmap(ushort **fbp, uchar *bmap, ushort *cmap,
-				  int cnt)
+/**
+ * get_bmp_col_16bpp() - Convert a colour-table entry into a 16bpp pixel value
+ *
+ * @return value to write to the 16bpp frame buffer for this palette entry
+ */
+static uint get_bmp_col_16bpp(struct bmp_color_table_entry cte)
 {
-	while (cnt > 0) {
-		*(*fbp)++ = cmap[*bmap++];
-		cnt--;
+	return ((cte.red   << 8) & 0xf800) |
+		((cte.green << 3) & 0x07e0) |
+		((cte.blue  >> 3) & 0x001f);
+}
+
+/**
+ * write_pix8() - Write a pixel from a BMP image into the framebuffer
+ *
+ * This handles frame buffers with 8, 16, 24 or 32 bits per pixel
+ *
+ * @fb: Place in frame buffer to update
+ * @bpix: Frame buffer bits-per-pixel, which controls how many bytes are written
+ * @palette: BMP palette table
+ * @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
+ */
+static void write_pix8(u8 *fb, uint bpix, struct bmp_color_table_entry *palette,
+		       u8 *bmap)
+{
+	if (bpix == 8) {
+		*fb++ = *bmap;
+	} else if (bpix == 16) {
+		*(u16 *)fb = get_bmp_col_16bpp(palette[*bmap]);
+	} else {
+		/* Only support big endian */
+		struct bmp_color_table_entry *cte = &palette[*bmap];
+
+		if (bpix == 24) {
+			*fb++ = cte->red;
+			*fb++ = cte->green;
+			*fb++ = cte->blue;
+		} else {
+			*fb++ = cte->blue;
+			*fb++ = cte->green;
+			*fb++ = cte->red;
+			*fb++ = 0;
+		}
 	}
 }

-static void draw_encoded_bitmap(ushort **fbp, ushort col, int cnt)
+static void draw_unencoded_bitmap(u8 **fbp, uint bpix, uchar *bmap,
+				  struct bmp_color_table_entry *palette,
+				  int cnt)
 {
-	ushort *fb = *fbp;
+	u8 *fb = *fbp;

 	while (cnt > 0) {
-		*fb++ = col;
+		write_pix8(fb, bpix, palette, bmap++);
+		fb += bpix / 8;
+		cnt--;
+	}
+	*fbp = fb;
+}
+
+static void draw_encoded_bitmap(u8 **fbp, uint bpix,
+				struct bmp_color_table_entry *palette, u8 *bmap,
+				int cnt)
+{
+	u8 *fb = *fbp;
+
+	while (cnt > 0) {
+		write_pix8(fb, bpix, palette, bmap);
+		fb += bpix / 8;
 		cnt--;
 	}
 	*fbp = fb;
 }

 static void video_display_rle8_bitmap(struct udevice *dev,
-				      struct bmp_image *bmp, ushort *cmap,
+				      struct bmp_image *bmp, uint bpix,
+				      struct bmp_color_table_entry *palette,
 				      uchar *fb, int x_off, int y_off,
 				      ulong width, ulong height)
 {
@@ -49,6 +105,7 @@ static void video_display_rle8_bitmap(struct udevice *dev,
 	ulong cnt, runlen;
 	int x, y;
 	int decode = 1;
+	uint bytes_per_pixel = bpix / 8;

 	debug("%s\n", __func__);
 	bmap = (uchar *)bmp + get_unaligned_le32(&bmp->header.data_offset);
@@ -64,8 +121,8 @@ static void video_display_rle8_bitmap(struct udevice *dev,
 				bmap += 2;
 				x = 0;
 				y--;
-				/* 16bpix, 2-byte per pixel, width should *2 */
-				fb -= (width * 2 + priv->line_length);
+				fb -= width * bytes_per_pixel +
+					priv->line_length;
 				break;
 			case BMP_RLE8_EOBMP:
 				/* end of bitmap */
@@ -75,9 +132,9 @@ static void video_display_rle8_bitmap(struct udevice *dev,
 				/* delta run */
 				x += bmap[2];
 				y -= bmap[3];
-				/* 16bpix, 2-byte per pixel, x should *2 */
-				fb = (uchar *)(priv->fb + (y + y_off - 1)
-					* priv->line_length + (x + x_off) * 2);
+				fb = (uchar *)(priv->fb +
+					(y + y_off - 1) * priv->line_length +
+					(x + x_off) * bytes_per_pixel);
 				bmap += 4;
 				break;
 			default:
@@ -91,8 +148,8 @@ static void video_display_rle8_bitmap(struct udevice *dev,
 						else
 							cnt = runlen;
 						draw_unencoded_bitmap(
-							(ushort **)&fb,
-							bmap, cmap, cnt);
+							&fb, bpix,
+							bmap, palette, cnt);
 					}
 					x += runlen;
 				}
@@ -116,8 +173,8 @@ static void video_display_rle8_bitmap(struct udevice *dev,
 						cnt = width - x;
 					else
 						cnt = runlen;
-					draw_encoded_bitmap((ushort **)&fb,
-						cmap[bmap[1]], cnt);
+					draw_encoded_bitmap(&fb, bpix, palette,
+							    &bmap[1], cnt);
 				}
 				x += runlen;
 			}
@@ -125,20 +182,6 @@ static void video_display_rle8_bitmap(struct udevice *dev,
 		}
 	}
 }
-#endif
-
-__weak void fb_put_byte(uchar **fb, uchar **from)
-{
-	*(*fb)++ = *(*from)++;
-}
-
-#if defined(CONFIG_BMP_16BPP)
-__weak void fb_put_word(uchar **fb, uchar **from)
-{
-	*(*fb)++ = *(*from)++;
-	*(*fb)++ = *(*from)++;
-}
-#endif /* CONFIG_BMP_16BPP */

 /**
 * video_splash_align_axis() - Align a single coordinate
@@ -169,28 +212,10 @@ static void video_splash_align_axis(int *axis, unsigned long panel_size,
 	*axis = max(0, (int)axis_alignment);
 }

-static void video_set_cmap(struct udevice *dev,
-			   struct bmp_color_table_entry *cte, unsigned colours)
-{
-	struct video_priv *priv = dev_get_uclass_priv(dev);
-	int i;
-	ushort *cmap = priv->cmap;
-
-	debug("%s: colours=%d\n", __func__, colours);
-	for (i = 0; i < colours; ++i) {
-		*cmap = ((cte->red   << 8) & 0xf800) |
-			((cte->green << 3) & 0x07e0) |
-			((cte->blue  >> 3) & 0x001f);
-		cmap++;
-		cte++;
-	}
-}
-
 int video_bmp_display(struct udevice *dev, ulong bmp_image, int x, int y,
 		      bool align)
 {
 	struct video_priv *priv = dev_get_uclass_priv(dev);
-	ushort *cmap_base = NULL;
 	int i, j;
 	uchar *start, *fb;
 	struct bmp_image *bmp = map_sysmem(bmp_image, 0);
@@ -246,9 +271,6 @@ int video_bmp_display(struct udevice *dev, ulong bmp_image, int x, int y,
 	debug("Display-bmp: %d x %d  with %d colours, display %d\n",
 	      (int)width, (int)height, (int)colours, 1 << bpix);

-	if (bmp_bpix == 8)
-		video_set_cmap(dev, palette, colours);
-
 	padded_width = (width & 0x3 ? (width & ~0x3) + 4 : width);

 	if (align) {
@@ -270,23 +292,19 @@ int video_bmp_display(struct udevice *dev, ulong bmp_image, int x, int y,

 	switch (bmp_bpix) {
 	case 1:
-	case 8: {
-		struct bmp_color_table_entry *cte;
-		cmap_base = priv->cmap;
-#ifdef CONFIG_VIDEO_BMP_RLE8
-		u32 compression = get_unaligned_le32(&bmp->header.compression);
-		debug("compressed %d %d\n", compression, BMP_BI_RLE8);
-		if (compression == BMP_BI_RLE8) {
-			if (bpix != 16) {
-				/* TODO implement render code for bpix != 16 */
-				printf("Error: only support 16 bpix");
-				return -EPROTONOSUPPORT;
+	case 8:
+		if (IS_ENABLED(CONFIG_VIDEO_BMP_RLE8)) {
+			u32 compression = get_unaligned_le32(
+				&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);
+				break;
 			}
-			video_display_rle8_bitmap(dev, bmp, cmap_base, fb, x,
-						  y, width, height);
-			break;
 		}
-#endif
+
+		/* Not compressed */
 		byte_width = width * (bpix / 8);
 		if (!byte_width)
 			byte_width = width;
@@ -294,81 +312,64 @@ int video_bmp_display(struct udevice *dev, ulong bmp_image, int x, int y,
 		for (i = 0; i < height; ++i) {
 			WATCHDOG_RESET();
 			for (j = 0; j < width; j++) {
-				if (bpix == 8) {
-					fb_put_byte(&fb, &bmap);
-				} else if (bpix == 16) {
-					*(uint16_t *)fb = cmap_base[*bmap];
-					bmap++;
-					fb += sizeof(uint16_t) / sizeof(*fb);
-				} else {
-					/* Only support big endian */
-					cte = &palette[*bmap];
-					bmap++;
-					if (bpix == 24) {
-						*(fb++) = cte->red;
-						*(fb++) = cte->green;
-						*(fb++) = cte->blue;
-					} else {
-						*(fb++) = cte->blue;
-						*(fb++) = cte->green;
-						*(fb++) = cte->red;
-						*(fb++) = 0;
-					}
-				}
+				write_pix8(fb, bpix, palette, bmap);
+				bmap++;
+				fb += bpix / 8;
 			}
 			bmap += (padded_width - width);
 			fb -= byte_width + priv->line_length;
 		}
 		break;
-	}
-#if defined(CONFIG_BMP_16BPP)
 	case 16:
-		for (i = 0; i < height; ++i) {
-			WATCHDOG_RESET();
-			for (j = 0; j < width; j++)
-				fb_put_word(&fb, &bmap);
-
-			bmap += (padded_width - width);
-			fb -= width * 2 + priv->line_length;
-		}
-		break;
-#endif /* CONFIG_BMP_16BPP */
-#if defined(CONFIG_BMP_24BPP)
-	case 24:
-		for (i = 0; i < height; ++i) {
-			for (j = 0; j < width; j++) {
-				if (bpix == 16) {
-					/* 16bit 555RGB format */
-					*(u16 *)fb = ((bmap[2] >> 3) << 10) |
-						((bmap[1] >> 3) << 5) |
-						(bmap[0] >> 3);
-					bmap += 3;
-					fb += 2;
-				} else {
-					*(fb++) = *(bmap++);
-					*(fb++) = *(bmap++);
-					*(fb++) = *(bmap++);
-					*(fb++) = 0;
+		if (IS_ENABLED(CONFIG_BMP_16BPP)) {
+			for (i = 0; i < height; ++i) {
+				WATCHDOG_RESET();
+				for (j = 0; j < width; j++) {
+					*fb++ = *bmap++;
+					*fb++ = *bmap++;
 				}
+				bmap += (padded_width - width);
+				fb -= width * 2 + priv->line_length;
+			}
+		}
+		break;
+	case 24:
+		if (IS_ENABLED(CONFIG_BMP_24BPP)) {
+			for (i = 0; i < height; ++i) {
+				for (j = 0; j < width; j++) {
+					if (bpix == 16) {
+						/* 16bit 565RGB format */
+						*(u16 *)fb = ((bmap[2] >> 3)
+							<< 11) |
+							((bmap[1] >> 2) << 5) |
+							(bmap[0] >> 3);
+						bmap += 3;
+						fb += 2;
+					} else {
+						*fb++ = *bmap++;
+						*fb++ = *bmap++;
+						*fb++ = *bmap++;
+						*fb++ = 0;
+					}
+				}
+				fb -= priv->line_length + width * (bpix / 8);
+				bmap += (padded_width - width);
 			}
-			fb -= priv->line_length + width * (bpix / 8);
-			bmap += (padded_width - width);
 		}
 		break;
-#endif /* CONFIG_BMP_24BPP */
-#if defined(CONFIG_BMP_32BPP)
 	case 32:
-		for (i = 0; i < height; ++i) {
-			for (j = 0; j < width; j++) {
-				*(fb++) = *(bmap++);
-				*(fb++) = *(bmap++);
-				*(fb++) = *(bmap++);
-				*(fb++) = *(bmap++);
+		if (IS_ENABLED(CONFIG_BMP_32BPP)) {
+			for (i = 0; i < height; ++i) {
+				for (j = 0; j < width; j++) {
+					*fb++ = *bmap++;
+					*fb++ = *bmap++;
+					*fb++ = *bmap++;
+					*fb++ = *bmap++;
+				}
+				fb -= priv->line_length + width * (bpix / 8);
 			}
-			fb -= priv->line_length + width * (bpix / 8);
 		}
 		break;
-#endif /* CONFIG_BMP_32BPP */
 	default:
 		break;
 	};