| Message ID | 20190323013002.27117-4-andre.przywara@arm.com |
|---|---|
| State | Accepted |
| Commit | 29c158b90d9fe1b8e007ee6b43f85c7f4b5f848b |
| Delegated to: | Anatolij Gustschin |
| Headers | show |
| Series | video/console: Fix various DM_VIDEO console issues | expand |
On Fri, 22 Mar 2019 at 19:32, Andre Przywara <andre.przywara@arm.com> wrote: > > The ANSI terminal escapce sequence standard defines relative cursor > movement commands (ESC [ A-F). So far the DM_VIDEO console code was > ignoring them. > > Interpret those sequences and move the cursor by the requested amount of > rows or columns in the right direction. This brings the code on par with > the legacy video console driver (cfb_console). > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > --- > drivers/video/vidconsole-uclass.c | 37 +++++++++++++++++++++++++++++++++++++ > 1 file changed, 37 insertions(+) Reviewed-by: Simon Glass <sjg@chromium.org>
On Sat, 23 Mar 2019 01:29:57 +0000 Andre Przywara andre.przywara@arm.com wrote: > The ANSI terminal escapce sequence standard defines relative cursor > movement commands (ESC [ A-F). So far the DM_VIDEO console code was > ignoring them. > > Interpret those sequences and move the cursor by the requested amount of > rows or columns in the right direction. This brings the code on par with > the legacy video console driver (cfb_console). > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > --- > drivers/video/vidconsole-uclass.c | 37 +++++++++++++++++++++++++++++++++++++ > 1 file changed, 37 insertions(+) Applied to u-boot-video/master, thanks! -- Anatolij
On Tue, 9 Apr 2019 23:05:11 +0200 Anatolij Gustschin agust@denx.de wrote: ... > > drivers/video/vidconsole-uclass.c | 37 +++++++++++++++++++++++++++++++++++++ > > 1 file changed, 37 insertions(+) > > Applied to u-boot-video/master, thanks! I've dropped all applied patches of this series now, some of them introduced dm video_ansi test error [1]. Please fix. Thanks! [1] https://travis-ci.org/vdsao/u-boot-video/jobs/517944146#L1081 -- Anatolij
On 11/04/2019 13:09, Anatolij Gustschin wrote: Hi Anatolij, thanks for the heads up! > On Tue, 9 Apr 2019 23:05:11 +0200 > Anatolij Gustschin agust@denx.de wrote: > ... >>> drivers/video/vidconsole-uclass.c | 37 +++++++++++++++++++++++++++++++++++++ >>> 1 file changed, 37 insertions(+) >> >> Applied to u-boot-video/master, thanks! > > I've dropped all applied patches of this series now, some of them > introduced dm video_ansi test error [1]. Please fix. Thanks! Hmh, good one. Didn't find an easy way to get to the bottom of this within the ut test system, so I copied the ANSI sequences out and replayed them with a custom command, inspecting the (sandbox) screen manually. Is there a canonical way to trace down those issues? Anyway, the fix for patch 2/8 is rather simple (see below), do you want to fix this up in your tree? Or shall I sent a v2? The head of my tree passes the video_ansi test now. (This patch won't apply cleanly (blame Thunderbird), but I think you get the idea...) --- a/drivers/video/vidconsole-uclass.c +++ b/drivers/video/vidconsole-uclass.c @@ -464,7 +464,7 @@ static void vidconsole_escape_char(struct udevice *dev, char ch) break; case 40 ... 47: - /* background color */ - vid_priv->bg_col_idx &= ~7; + /* background color, also mask the bold bit */ + vid_priv->bg_col_idx &= ~0xf; vid_priv->bg_col_idx |= val - 40; vid_priv->colour_bg = vid_console_color( vid_priv, vid_priv->bg_col_idx); Cheers, Andre.
Hi André, On Sat, 13 Apr 2019 22:40:03 +0100 André Przywara andre.przywara@arm.com wrote: ... > > I've dropped all applied patches of this series now, some of them > > introduced dm video_ansi test error [1]. Please fix. Thanks! > > Hmh, good one. Didn't find an easy way to get to the bottom of this > within the ut test system, so I copied the ANSI sequences out and > replayed them with a custom command, inspecting the (sandbox) screen > manually. Is there a canonical way to trace down those issues? You can build sandbox defconfig an run U-Boot with "--show_lcd" option, then run the "ut dm video_ansi" command to inspect the rendered characters and colors, e.g.: $ make sandbox_defconfig $ make $ ./u-boot -d arch/sandbox/dts/test.dtb -l U-Boot 2019.04-00326-g7b64a70a3a (Apr 14 2019 - 14:37:44 +0200) Model: sandbox DRAM: 128 MiB MMC: mmc2: 2 (SD), mmc1: 1 (SD), mmc0: 0 (SD) In: serial Out: vidconsole Err: vidconsole Model: sandbox SCSI: Net: eth0: eth@10002000, eth5: eth@10003000, eth3: sbe5, eth1: eth@10004000 Hit any key to stop autoboot: 0 => ut dm video_ansi Test: dm_test_video_ansi: video.c Failures: 0 In the "U-Boot" window you will see the output of the video_ansi test. Without your fix for masking bit 3 in the bg index, the high-intensity background colors will be used, you will see brighter BG colors. The test expects results for standard background colors (indexes 0-7). > Anyway, the fix for patch 2/8 is rather simple (see below), do you want > to fix this up in your tree? Or shall I sent a v2? Thanks! I've merged your fix for patch 2/8, no need to resend. Build- testing now. -- Anatolij
On 14/04/2019 13:54, Anatolij Gustschin wrote: > Hi André, > > On Sat, 13 Apr 2019 22:40:03 +0100 > André Przywara andre.przywara@arm.com wrote: > ... >>> I've dropped all applied patches of this series now, some of them >>> introduced dm video_ansi test error [1]. Please fix. Thanks! >> >> Hmh, good one. Didn't find an easy way to get to the bottom of this >> within the ut test system, so I copied the ANSI sequences out and >> replayed them with a custom command, inspecting the (sandbox) screen >> manually. Is there a canonical way to trace down those issues? > > You can build sandbox defconfig an run U-Boot with "--show_lcd" > option, then run the "ut dm video_ansi" command to inspect the > rendered characters and colors, e.g.: > > $ make sandbox_defconfig > $ make > $ ./u-boot -d arch/sandbox/dts/test.dtb -l Ah, I did everything exactly as you, except I was using "-D" (sandbox64.dtb), not test.dtb. With sandbox64.dtb I could run the test and see the test summary report, but didn't see the actual graphical output. > U-Boot 2019.04-00326-g7b64a70a3a (Apr 14 2019 - 14:37:44 +0200) > > Model: sandbox > DRAM: 128 MiB > MMC: mmc2: 2 (SD), mmc1: 1 (SD), mmc0: 0 (SD) > In: serial > Out: vidconsole > Err: vidconsole > Model: sandbox > SCSI: > Net: eth0: eth@10002000, eth5: eth@10003000, eth3: sbe5, eth1: eth@10004000 > Hit any key to stop autoboot: 0 > => ut dm video_ansi > Test: dm_test_video_ansi: video.c > Failures: 0 > > In the "U-Boot" window you will see the output of the video_ansi test. > > Without your fix for masking bit 3 in the bg index, the high-intensity > background colors will be used, you will see brighter BG colors. The test > expects results for standard background colors (indexes 0-7). Yes, that's what I figured after sending the same escape sequences as the test from a hacked "cls" command. But it took me actually a screenshot to spot the difference ;-) >> Anyway, the fix for patch 2/8 is rather simple (see below), do you want >> to fix this up in your tree? Or shall I sent a v2? > > Thanks! I've merged your fix for patch 2/8, no need to resend. Build- > testing now. Great, thanks a lot! Cheers, Andre.
diff --git a/drivers/video/vidconsole-uclass.c b/drivers/video/vidconsole-uclass.c index 87f43c2030..cbd63f0ce8 100644 --- a/drivers/video/vidconsole-uclass.c +++ b/drivers/video/vidconsole-uclass.c @@ -259,6 +259,43 @@ static void vidconsole_escape_char(struct udevice *dev, char ch) priv->escape = 0; switch (ch) { + case 'A': + case 'B': + case 'C': + case 'D': + case 'E': + case 'F': { + int row, col, num; + char *s = priv->escape_buf; + + /* + * Cursor up/down: [%dA, [%dB, [%dE, [%dF + * Cursor left/right: [%dD, [%dC + */ + s++; /* [ */ + s = parsenum(s, &num); + if (num == 0) /* No digit in sequence ... */ + num = 1; /* ... means "move by 1". */ + + get_cursor_position(priv, &row, &col); + if (ch == 'A' || ch == 'F') + row -= num; + if (ch == 'C') + col += num; + if (ch == 'D') + col -= num; + if (ch == 'B' || ch == 'E') + row += num; + if (ch == 'E' || ch == 'F') + col = 0; + if (col < 0) + col = 0; + if (row < 0) + row = 0; + /* Right and bottom overflows are handled in the callee. */ + set_cursor_position(priv, row, col); + break; + } case 'H': case 'f': { int row, col;
The ANSI terminal escapce sequence standard defines relative cursor movement commands (ESC [ A-F). So far the DM_VIDEO console code was ignoring them. Interpret those sequences and move the cursor by the requested amount of rows or columns in the right direction. This brings the code on par with the legacy video console driver (cfb_console). Signed-off-by: Andre Przywara <andre.przywara@arm.com> --- drivers/video/vidconsole-uclass.c | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+)