[U-Boot] [PATCH v4 0/4] dm: video: Correct color ANSI escape sequence support

Support special rendition code 0 - reset attributes. Support special rendition code 1 - increased intensity (bold). Get RGB sequence in pixels right (swap blue and red). Do not set reserved bits. Use u32 instead of unsigned for color bit mask.
qemu-system-i386 -display sdl -vga virtio and qemu-system-i386 -display sdl -vga cirrus now display the same colors as qemu-system-i386 -nographic
Testing is possible via
setenv efi_selftest test output bootefi selftest --- v4: Fix a build warning leading to a Travis error. Rename a variable. v3: Split single patch up into a series Use constants for colors v2: SGR 0 should reset the colors and the attributes. ---
Heinrich Schuchardt (4): dm: video: show correct colors in graphical console dm: video: correctly clean background in 16bit mode dm: video: use constants to refer to colors dm: video: support increased intensity (bold)
drivers/video/vidconsole-uclass.c | 86 ++++++++++++++++++++++++++------------- drivers/video/video-uclass.c | 38 +++++++++++++---- include/video.h | 13 +++++- include/video_console.h | 43 ++++++++++++++++++++ test/dm/video.c | 2 +- 5 files changed, 142 insertions(+), 40 deletions(-)

Get RGB sequence in pixels right (swap blue and red). Do not set reserved bits.
qemu-system-i386 -display sdl -vga virtio and qemu-system-i386 -display sdl -vga cirrus now display the similar colors (highlighting still missing) as qemu-system-i386 -nographic
Testing is possible via
setenv efi_selftest test output bootefi selftest
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de Reviewed-by: Simon Glass sjg@chromium.org --- v4 no change v3 no change v2 no change --- drivers/video/vidconsole-uclass.c | 13 ++++++------- test/dm/video.c | 2 +- 2 files changed, 7 insertions(+), 8 deletions(-)
diff --git a/drivers/video/vidconsole-uclass.c b/drivers/video/vidconsole-uclass.c index 5f63c12d6c5..8a2a377161f 100644 --- a/drivers/video/vidconsole-uclass.c +++ b/drivers/video/vidconsole-uclass.c @@ -127,15 +127,14 @@ static void set_color(struct video_priv *priv, unsigned idx, unsigned *c) { switch (priv->bpix) { case VIDEO_BPP16: - *c = ((colors[idx].r >> 3) << 0) | - ((colors[idx].g >> 2) << 5) | - ((colors[idx].b >> 3) << 11); + *c = ((colors[idx].r >> 3) << 11) | + ((colors[idx].g >> 2) << 5) | + ((colors[idx].b >> 3) << 0); break; case VIDEO_BPP32: - *c = 0xff000000 | - (colors[idx].r << 0) | - (colors[idx].g << 8) | - (colors[idx].b << 16); + *c = (colors[idx].r << 16) | + (colors[idx].g << 8) | + (colors[idx].b << 0); break; default: /* unsupported, leave current color in place */ diff --git a/test/dm/video.c b/test/dm/video.c index 29917d0c2d8..d158f1fcb39 100644 --- a/test/dm/video.c +++ b/test/dm/video.c @@ -186,7 +186,7 @@ static int dm_test_video_ansi(struct unit_test_state *uts) /* test colors (30-37 fg color, 40-47 bg color) */ vidconsole_put_string(con, ANSI_ESC"[30;41mfoo"); /* black on red */ vidconsole_put_string(con, ANSI_ESC"[33;44mbar"); /* yellow on blue */ - ut_asserteq(268, compress_frame_buffer(dev)); + ut_asserteq(267, compress_frame_buffer(dev));
return 0; }

In 16 bit mode we have to copy two bytes per pixels repeatedly and not four. Otherwise we will see a striped pattern.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de Reviewed-by: Simon Glass sjg@chromium.org --- v4 no change v3 no change v2 no change --- drivers/video/video-uclass.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/video/video-uclass.c b/drivers/video/video-uclass.c index dcaceed42c4..9a980ea3a1d 100644 --- a/drivers/video/video-uclass.c +++ b/drivers/video/video-uclass.c @@ -91,14 +91,26 @@ void video_clear(struct udevice *dev) { struct video_priv *priv = dev_get_uclass_priv(dev);
- if (priv->bpix == VIDEO_BPP32) { + switch (priv->bpix) { + case VIDEO_BPP16: { + u16 *ppix = priv->fb; + u16 *end = priv->fb + priv->fb_size; + + while (ppix < end) + *ppix++ = priv->colour_bg; + break; + } + case VIDEO_BPP32: { u32 *ppix = priv->fb; u32 *end = priv->fb + priv->fb_size;
while (ppix < end) *ppix++ = priv->colour_bg; - } else { + break; + } + default: memset(priv->fb, priv->colour_bg, priv->fb_size); + break; } }

Use constants to refer to colors. Adjust initialization of foreground and background color to avoid setting reserved bits. Consistently u32 instead of unsigned for color bit mask.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v4 Fix a build warning, that was treated as an error in Travis testing by conditional compling with CONFIG_DM_VIDEO. v3 Use color constants for initalizing the console. v2 no change --- drivers/video/vidconsole-uclass.c | 55 +++++++++++++++++++++++---------------- drivers/video/video-uclass.c | 19 +++++++++----- include/video.h | 11 ++++++-- include/video_console.h | 35 +++++++++++++++++++++++++ 4 files changed, 89 insertions(+), 31 deletions(-)
diff --git a/drivers/video/vidconsole-uclass.c b/drivers/video/vidconsole-uclass.c index 8a2a377161f..d32b1017581 100644 --- a/drivers/video/vidconsole-uclass.c +++ b/drivers/video/vidconsole-uclass.c @@ -15,6 +15,15 @@ #include <video_console.h> #include <video_font.h> /* Get font data, width and height */
+/* + * Structure to describe a console color + */ +struct vid_rgb { + u32 r; + u32 g; + u32 b; +}; + /* By default we scroll by a single line */ #ifndef CONFIG_CONSOLE_SCROLL_LINES #define CONFIG_CONSOLE_SCROLL_LINES 1 @@ -108,11 +117,7 @@ static void vidconsole_newline(struct udevice *dev) video_sync(dev->parent); }
-static const struct { - unsigned r; - unsigned g; - unsigned b; -} colors[] = { +static const struct vid_rgb colors[VID_COLOR_COUNT] = { { 0x00, 0x00, 0x00 }, /* black */ { 0xff, 0x00, 0x00 }, /* red */ { 0x00, 0xff, 0x00 }, /* green */ @@ -123,22 +128,26 @@ static const struct { { 0xff, 0xff, 0xff }, /* white */ };
-static void set_color(struct video_priv *priv, unsigned idx, unsigned *c) +u32 vid_console_color(struct video_priv *priv, unsigned int idx) { switch (priv->bpix) { case VIDEO_BPP16: - *c = ((colors[idx].r >> 3) << 11) | - ((colors[idx].g >> 2) << 5) | - ((colors[idx].b >> 3) << 0); - break; + return ((colors[idx].r >> 3) << 11) | + ((colors[idx].g >> 2) << 5) | + ((colors[idx].b >> 3) << 0); case VIDEO_BPP32: - *c = (colors[idx].r << 16) | - (colors[idx].g << 8) | - (colors[idx].b << 0); - break; + return (colors[idx].r << 16) | + (colors[idx].g << 8) | + (colors[idx].b << 0); default: - /* unsupported, leave current color in place */ - break; + /* + * For unknown bit arrangements just support + * black and white. + */ + if (idx) + return 0xffffff; /* white */ + else + return 0x000000; /* black */ } }
@@ -270,17 +279,17 @@ static void vidconsole_escape_char(struct udevice *dev, char ch)
switch (val) { case 30 ... 37: - /* fg color */ - set_color(vid_priv, val - 30, - (unsigned *)&vid_priv->colour_fg); + /* foreground color */ + vid_priv->colour_fg = vid_console_color( + vid_priv, val - 30); break; case 40 ... 47: - /* bg color */ - set_color(vid_priv, val - 40, - (unsigned *)&vid_priv->colour_bg); + /* background color */ + vid_priv->colour_bg = vid_console_color( + vid_priv, val - 40); break; default: - /* unknown/unsupported */ + /* ignore unsupported SGR parameter */ break; } } diff --git a/drivers/video/video-uclass.c b/drivers/video/video-uclass.c index 9a980ea3a1d..945b20ddfd7 100644 --- a/drivers/video/video-uclass.c +++ b/drivers/video/video-uclass.c @@ -114,6 +114,17 @@ void video_clear(struct udevice *dev) } }
+void video_set_default_colors(struct video_priv *priv) +{ +#ifdef CONFIG_SYS_WHITE_ON_BLACK + priv->colour_fg = vid_console_color(priv, VID_WHITE); + priv->colour_bg = vid_console_color(priv, VID_BLACK); +#else + priv->colour_fg = vid_console_color(priv, VID_BLACK); + priv->colour_bg = vid_console_color(priv, VID_WHITE); +#endif +} + /* Flush video activity to the caches */ void video_sync(struct udevice *vid) { @@ -203,12 +214,8 @@ static int video_post_probe(struct udevice *dev) priv->line_length = priv->xsize * VNBYTES(priv->bpix); priv->fb_size = priv->line_length * priv->ysize;
- /* Set up colours - we could in future support other colours */ -#ifdef CONFIG_SYS_WHITE_ON_BLACK - priv->colour_fg = 0xffffff; -#else - priv->colour_bg = 0xffffff; -#endif + /* Set up colors */ + video_set_default_colors(priv);
if (!CONFIG_IS_ENABLED(NO_FB_CLEAR)) video_clear(dev); diff --git a/include/video.h b/include/video.h index 61ff6531215..841f3dc56b1 100644 --- a/include/video.h +++ b/include/video.h @@ -84,8 +84,8 @@ struct video_priv { void *fb; int fb_size; int line_length; - int colour_fg; - int colour_bg; + u32 colour_fg; + u32 colour_bg; bool flush_dcache; ushort *cmap; }; @@ -183,6 +183,13 @@ int video_get_ysize(struct udevice *dev); */ void video_set_flush_dcache(struct udevice *dev, bool flush);
+/** + * Set default colors and attributes + * + * @priv device information + */ +void video_set_default_colors(struct video_priv *priv); + #endif /* CONFIG_DM_VIDEO */
#ifndef CONFIG_DM_VIDEO diff --git a/include/video_console.h b/include/video_console.h index 9dce234bd92..656a47295f6 100644 --- a/include/video_console.h +++ b/include/video_console.h @@ -7,11 +7,29 @@ #ifndef __video_console_h #define __video_console_h
+#include <video.h> + #define VID_FRAC_DIV 256
#define VID_TO_PIXEL(x) ((x) / VID_FRAC_DIV) #define VID_TO_POS(x) ((x) * VID_FRAC_DIV)
+/* + * The 8 colors supported by the console + */ +enum color_idx { + VID_BLACK = 0, + VID_RED, + VID_GREEN, + VID_YELLOW, + VID_BLUE, + VID_MAGENTA, + VID_CYAN, + VID_WHITE, + + VID_COLOR_COUNT +}; + /** * struct vidconsole_priv - uclass-private data about a console device * @@ -196,4 +214,21 @@ int vidconsole_put_char(struct udevice *dev, char ch); void vidconsole_position_cursor(struct udevice *dev, unsigned col, unsigned row);
+#ifdef CONFIG_DM_VIDEO + +/** + * vid_console_color() - convert a color code to a pixel's internal + * representation + * + * The caller has to guarantee that the color index is less than + * VID_COLOR_COUNT. + * + * @priv private data of the console device + * @idx color index + * @return color value + */ +u32 vid_console_color(struct video_priv *priv, unsigned int idx); + +#endif + #endif

On 8 February 2018 at 13:47, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
Use constants to refer to colors. Adjust initialization of foreground and background color to avoid setting reserved bits. Consistently u32 instead of unsigned for color bit mask.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
v4 Fix a build warning, that was treated as an error in Travis testing by conditional compling with CONFIG_DM_VIDEO. v3 Use color constants for initalizing the console. v2 no change
drivers/video/vidconsole-uclass.c | 55 +++++++++++++++++++++++---------------- drivers/video/video-uclass.c | 19 +++++++++----- include/video.h | 11 ++++++-- include/video_console.h | 35 +++++++++++++++++++++++++ 4 files changed, 89 insertions(+), 31 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org

Support special rendition code 0 - reset attributes. Support special rendition code 1 - increased intensity (bold).
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v4 Rename priv->fg to priv->fg_col_idx. v3 Add color constants. v2 SGR 0 should reset the colors and the attributes. --- drivers/video/vidconsole-uclass.c | 32 ++++++++++++++++++++++++++------ drivers/video/video-uclass.c | 5 ++++- include/video.h | 2 ++ include/video_console.h | 12 ++++++++++-- test/dm/video.c | 2 +- 5 files changed, 43 insertions(+), 10 deletions(-)
diff --git a/drivers/video/vidconsole-uclass.c b/drivers/video/vidconsole-uclass.c index d32b1017581..6f3988d49ea 100644 --- a/drivers/video/vidconsole-uclass.c +++ b/drivers/video/vidconsole-uclass.c @@ -119,12 +119,20 @@ static void vidconsole_newline(struct udevice *dev)
static const struct vid_rgb colors[VID_COLOR_COUNT] = { { 0x00, 0x00, 0x00 }, /* black */ - { 0xff, 0x00, 0x00 }, /* red */ - { 0x00, 0xff, 0x00 }, /* green */ + { 0xc0, 0x00, 0x00 }, /* red */ + { 0x00, 0xc0, 0x00 }, /* green */ + { 0xc0, 0x60, 0x00 }, /* brown */ + { 0x00, 0x00, 0xc0 }, /* blue */ + { 0xc0, 0x00, 0xc0 }, /* magenta */ + { 0x00, 0xc0, 0xc0 }, /* cyan */ + { 0xc0, 0xc0, 0xc0 }, /* light gray */ + { 0x80, 0x80, 0x80 }, /* gray */ + { 0xff, 0x00, 0x00 }, /* bright red */ + { 0x00, 0xff, 0x00 }, /* bright green */ { 0xff, 0xff, 0x00 }, /* yellow */ - { 0x00, 0x00, 0xff }, /* blue */ - { 0xff, 0x00, 0xff }, /* magenta */ - { 0x00, 0xff, 0xff }, /* cyan */ + { 0x00, 0x00, 0xff }, /* bright blue */ + { 0xff, 0x00, 0xff }, /* bright magenta */ + { 0x00, 0xff, 0xff }, /* bright cyan */ { 0xff, 0xff, 0xff }, /* white */ };
@@ -278,10 +286,22 @@ static void vidconsole_escape_char(struct udevice *dev, char ch) s++;
switch (val) { + case 0: + /* all attributes off */ + video_set_default_colors(vid_priv); + break; + case 1: + /* bold */ + vid_priv->fg_col_idx |= 8; + vid_priv->colour_fg = vid_console_color( + vid_priv, vid_priv->fg_col_idx); + break; case 30 ... 37: /* foreground color */ + vid_priv->fg_col_idx &= ~7; + vid_priv->fg_col_idx |= val - 30; vid_priv->colour_fg = vid_console_color( - vid_priv, val - 30); + vid_priv, vid_priv->fg_col_idx); break; case 40 ... 47: /* background color */ diff --git a/drivers/video/video-uclass.c b/drivers/video/video-uclass.c index 945b20ddfd7..b5bb8e0efde 100644 --- a/drivers/video/video-uclass.c +++ b/drivers/video/video-uclass.c @@ -117,9 +117,12 @@ void video_clear(struct udevice *dev) void video_set_default_colors(struct video_priv *priv) { #ifdef CONFIG_SYS_WHITE_ON_BLACK - priv->colour_fg = vid_console_color(priv, VID_WHITE); + /* White is used when switching to bold, use light gray here */ + priv->fg_col_idx = VID_LIGHT_GRAY; + priv->colour_fg = vid_console_color(priv, VID_LIGHT_GRAY); priv->colour_bg = vid_console_color(priv, VID_BLACK); #else + priv->fg_col_idx = VID_BLACK; priv->colour_fg = vid_console_color(priv, VID_BLACK); priv->colour_bg = vid_console_color(priv, VID_WHITE); #endif diff --git a/include/video.h b/include/video.h index 841f3dc56b1..ddc2eeb5a95 100644 --- a/include/video.h +++ b/include/video.h @@ -67,6 +67,7 @@ enum video_log2_bpp { * @flush_dcache: true to enable flushing of the data cache after * the LCD is updated * @cmap: Colour map for 8-bit-per-pixel displays + * @fg_col_idx: Foreground color code (bit 3 = bold, bit 0-2 = color) */ struct video_priv { /* Things set up by the driver: */ @@ -88,6 +89,7 @@ struct video_priv { u32 colour_bg; bool flush_dcache; ushort *cmap; + u8 fg_col_idx; };
/* Placeholder - there are no video operations at present */ diff --git a/include/video_console.h b/include/video_console.h index 656a47295f6..7621a189d2a 100644 --- a/include/video_console.h +++ b/include/video_console.h @@ -15,16 +15,24 @@ #define VID_TO_POS(x) ((x) * VID_FRAC_DIV)
/* - * The 8 colors supported by the console + * The 16 colors supported by the console */ enum color_idx { VID_BLACK = 0, VID_RED, VID_GREEN, - VID_YELLOW, + VID_BROWN, VID_BLUE, VID_MAGENTA, VID_CYAN, + VID_LIGHT_GRAY, + VID_GRAY, + VID_LIGHT_RED, + VID_LIGTH_GREEN, + VID_YELLOW, + VID_LIGHT_BLUE, + VID_LIGHT_MAGENTA, + VID_LIGHT_CYAN, VID_WHITE,
VID_COLOR_COUNT diff --git a/test/dm/video.c b/test/dm/video.c index d158f1fcb39..caca4969027 100644 --- a/test/dm/video.c +++ b/test/dm/video.c @@ -186,7 +186,7 @@ static int dm_test_video_ansi(struct unit_test_state *uts) /* test colors (30-37 fg color, 40-47 bg color) */ vidconsole_put_string(con, ANSI_ESC"[30;41mfoo"); /* black on red */ vidconsole_put_string(con, ANSI_ESC"[33;44mbar"); /* yellow on blue */ - ut_asserteq(267, compress_frame_buffer(dev)); + ut_asserteq(265, compress_frame_buffer(dev));
return 0; }

On 8 February 2018 at 13:47, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
Support special rendition code 0 - reset attributes. Support special rendition code 1 - increased intensity (bold).
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
v4 Rename priv->fg to priv->fg_col_idx. v3 Add color constants. v2 SGR 0 should reset the colors and the attributes.
drivers/video/vidconsole-uclass.c | 32 ++++++++++++++++++++++++++------ drivers/video/video-uclass.c | 5 ++++- include/video.h | 2 ++ include/video_console.h | 12 ++++++++++-- test/dm/video.c | 2 +- 5 files changed, 43 insertions(+), 10 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org

Hi Heinrich,
On Thu, 8 Feb 2018 21:47:08 +0100 Heinrich Schuchardt xypron.glpk@gmx.de wrote: ...
Heinrich Schuchardt (4): dm: video: show correct colors in graphical console dm: video: correctly clean background in 16bit mode dm: video: use constants to refer to colors dm: video: support increased intensity (bold)
drivers/video/vidconsole-uclass.c | 86 ++++++++++++++++++++++++++------------- drivers/video/video-uclass.c | 38 +++++++++++++---- include/video.h | 13 +++++- include/video_console.h | 43 ++++++++++++++++++++ test/dm/video.c | 2 +- 5 files changed, 142 insertions(+), 40 deletions(-)
Series applied to u-boot-video/next, thanks!
-- Anatolij
participants (3)
-
Anatolij Gustschin
-
Heinrich Schuchardt
-
Simon Glass