dm: video: make ANSI escape sequence support optional
PATCH 1/1 dm: video: make ANSI escape sequence support optional
As mentioned in review comments for ANSI escape sequence support patches, this should be optional to reduce code size. Disable escape sequence support when CONFIG_VIDEO_ANSI is not enabled.
---
This patch is based on basic ANSI escape seq. support series:
https://www.mail-archive.com/u-boot at lists.denx.de/msg263777.html
drivers/video/vidconsole-uclass.c | 6 ++++++ include/video_console.h | 4 ++++ 2 files changed, 10 insertions(+)
drivers/video/vidconsole-uclass.c +6 -0
@@ -108,6 +108,7 @@ static void vidconsole_newline(struct udevice *dev)video_sync (dev->parent); } +#if CONFIG_IS_ENABLED(VIDEO_ANSI) static const struct { unsigned r; unsigned g;
@@ -299,22 +300,27 @@ error:/* something went wrong, just revert to normal mode: */ priv->escape = 0; } +#endif int vidconsole_put_char (struct udevice *dev,char ch) {struct vidconsole_priv *priv =dev_get_uclass_priv (dev);int ret; +#if CONFIG_IS_ENABLED (VIDEO_ANSI )if (priv->escape) {vidconsole_escape_char (dev, ch);return 0 ; } +#endif switch (ch) { +#if CONFIG_IS_ENABLED (VIDEO_ANSI )case '\x1b' : priv->escape_len =0 ; priv->escape =1 ;break ; +#endif case '\a' :/* beep */ break ;
include/video_console.h +4 -0
@@ -7,6 +7,8 @@#ifndef __video_console_h#define __video_console_h +#include <linux/kconfig.h> +#define VID_FRAC_DIV 256#define VID_TO_PIXEL (x) ((x) / VID_FRAC_DIV)
@@ -45,6 +47,7 @@ struct vidconsole_priv {int xsize_frac;int xstart_frac;int last_ch; +#if CONFIG_IS_ENABLED (VIDEO_ANSI )/* * ANSI escape sequences are accumulated character by character, * starting after the ESC char (0x1b) until the entire sequence
@@ -53,6 +56,7 @@ struct vidconsole_priv {int escape;int escape_len;char escape_buf[32 ]; +#endif };/**
Signature · 1 line
2.11.0
PATCH 1/1 dm: video: make ANSI escape sequence support optional
Quoted 4 lines · Anatolij Gustschin
As mentioned in review comments for ANSI escape sequence support patches, this should be optional to reduce code size. Disable escape sequence support when
CONFIG_VIDEO_ANSIis not enabled.
Assuming the later version of the patch was applied there should be, at the top of vidconsole_escape_char:
if (!IS_ENABLED(CONFIG_VIDEO_ANSI))
goto error;which (at least if not building with -O0) should be enough to strip the rest out, with somewhat less ifdef than this approach..
BR,
-R
Quoted 81 lines
Signed-off-by Anatolij Gustschin <[email protected]>---
This patch is based on basic ANSI escape seq. support series:
https://www.mail-archive.com/u-boot at lists.denx.de/msg263777.htmldrivers/video/vidconsole-uclass.c | 6 ++++++ include/video_console.h | 4 ++++ 2 files changed, 10 insertions(+)drivers/video/vidconsole-uclass.c +6
-0@@ -108,6 +108,7 @@ static void vidconsole_newline(struct udevice *dev)video_sync (dev->parent); } +#if CONFIG_IS_ENABLED(VIDEO_ANSI) static const struct { unsigned r; unsigned g;@@ -299,22 +300,27 @@ error:/* something went wrong, just revert to normal mode: */ priv->escape = 0; } +#endif int vidconsole_put_char (struct udevice *dev,char ch) {struct vidconsole_priv *priv =dev_get_uclass_priv (dev);int ret; +#if CONFIG_IS_ENABLED (VIDEO_ANSI )if (priv->escape) {vidconsole_escape_char (dev, ch);return 0 ; } +#endif switch (ch) { +#if CONFIG_IS_ENABLED (VIDEO_ANSI )case '\x1b' : priv->escape_len =0 ; priv->escape =1 ;break ; +#endif case '\a' :/* beep */ break ;include/video_console.h +4
-1@@ -7,6 +7,8 @@#ifndef __video_console_h#define __video_console_h +#include <linux/kconfig.h> +#define VID_FRAC_DIV 256#define VID_TO_PIXEL (x) ((x) / VID_FRAC_DIV)@@ -45,6 +47,7 @@ struct vidconsole_priv {int xsize_frac;int xstart_frac;int last_ch; +#if CONFIG_IS_ENABLED (VIDEO_ANSI )/* * ANSI escape sequences are accumulated character by character, * starting after the ESC char (0x1b) until the entire sequence @@ -53,6 +56,7 @@ struct vidconsole_priv {int escape;int escape_len;char escape_buf[32 ]; +#endif };/** -- 2.11.0
PATCH 1/1 dm: video: make ANSI escape sequence support optional
In reply to Anatolij GustschinQuoted 5 lines · On Sat, 30 Sep 2017 10:19:17 +0200 Anatolij Gustschin agust at denx.de wrote:
As mentioned in review comments for ANSI escape sequence support patches, this should be optional to reduce code size. Disable escape sequence support when
CONFIG_VIDEO_ANSIis not enabled.
Quoted 8 lines
Signed-off-by Anatolij Gustschin <[email protected]>---
This patch is based on basic ANSI escape seq. support series:
https://www.mail-archive.com/u-boot at lists.denx.de/msg263777.htmldrivers/video/vidconsole-uclass.c | 6 ++++++ include/video_console.h | 4 ++++ 2 files changed, 10 insertions(+)
Applied to u-boot-video/master.
--
Anatolij
PATCH 1/1 dm: video: make ANSI escape sequence support optional
Quoted 13 lines · Anatolij Gustschin
Quoted 10 lines · On Sat, 30 Sep 2017 10:19:17 +0200 Anatolij Gustschin agust at denx.de wrote:
As mentioned in review comments for ANSI escape sequence support patches, this should be optional to reduce code size. Disable escape sequence support when
CONFIG_VIDEO_ANSIis not enabled.Signed-off-by Anatolij Gustschin <[email protected]>---
This patch is based on basic ANSI escape seq. support series:
https://www.mail-archive.com/u-boot at lists.denx.de/msg263777.html
Quoted 6 lines
Quoted 3 lines
drivers/video/vidconsole-uclass.c | 6 ++++++ include/video_console.h | 4 ++++ 2 files changed, 10 insertions(+)Applied to u-boot-video/master.
Like I mentioned before, we shouldn't really need this patch.. It only makes a trivial difference in size (which you could probably get back by just adding #ifdef VIDEO_ANSI / #endif around the single case statement in vidconsole_put_char())
text data bss dec hex 1937 224 0 2161 871 VIDEO_ANSI disabled with this patch 2002 224 0 2226 8b2 VIDEO_ANSI disabled without this patch 2698 224 0 2922 b6a VIDEO_ANSI enabled
And it makes the code a lot uglier and removes at least compile-time testing when VIDEO_ANSI is disabled.
So I think you should drop/revert this patch
BR,
-R
PATCH 1/1 dm: video: make ANSI escape sequence support optional
Hi Rob,
Quoted 32 lines · Rob Clark
Quoted 13 lines · Anatolij Gustschin
Quoted 10 lines · On Sat, 30 Sep 2017 10:19:17 +0200 Anatolij Gustschin agust at denx.de wrote:
As mentioned in review comments for ANSI escape sequence support patches, this should be optional to reduce code size. Disable escape sequence support when
CONFIG_VIDEO_ANSIis not enabled.Signed-off-by Anatolij Gustschin <[email protected]>---
This patch is based on basic ANSI escape seq. support series:
https://www.mail-archive.com/u-boot at lists.denx.de/msg263777.htmlQuoted 6 lines
Quoted 3 lines
drivers/video/vidconsole-uclass.c | 6 ++++++ include/video_console.h | 4 ++++ 2 files changed, 10 insertions(+)Applied to u-boot-video/master.
Like I mentioned before, we shouldn't really need this patch.. It only makes a trivial difference in size (which you could probably get back by just adding #ifdef
VIDEO_ANSI/ #endif around the single case statement invidconsole_put_char())text data bss dec hex 1937 224 0 2161 871 VIDEO_ANSI disabled with this patch 2002 224 0 2226 8b2 VIDEO_ANSI disabled without this patch 2698 224 0 2922 b6a VIDEO_ANSI enabled
Quoted 4 lines
And it makes the code a lot uglier and removes at least compile-time testing when
VIDEO_ANSIis disabled.So I think you should drop/revert this patch
Seems reasonable to me.
Regards,
Simon
PATCH 1/1 dm: video: make ANSI escape sequence support optional
Hi Rob, Simon,
On Fri, 6 Oct 2017 11:04:24 -0600
Simon Glass sjg at chromium.org wrote:
...
Quoted 3 lines
Quoted 1 line
So I think you should drop/revert this patch
Seems reasonable to me.
OK, I dropped it.
Thanks,
--
Anatolij