* [U-Boot] [PATCH] dm: video: make ANSI escape sequence support optional
@ 2017-09-30 8:19 Anatolij Gustschin
2017-09-30 16:20 ` Rob Clark
2017-10-06 16:15 ` Anatolij Gustschin
0 siblings, 2 replies; 6+ messages in thread
From: Anatolij Gustschin @ 2017-09-30 8:19 UTC (permalink / raw)
To: u-boot
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.
Signed-off-by: Anatolij Gustschin <agust@denx.de>
---
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(+)
diff --git a/drivers/video/vidconsole-uclass.c b/drivers/video/vidconsole-uclass.c
index 5f63c12d6c..f62e9f9308 100644
--- a/drivers/video/vidconsole-uclass.c
+++ b/drivers/video/vidconsole-uclass.c
@@ -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;
diff --git a/include/video_console.h b/include/video_console.h
index 9dce234bd9..f07f43c1e2 100644
--- a/include/video_console.h
+++ b/include/video_console.h
@@ -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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] dm: video: make ANSI escape sequence support optional
2017-09-30 8:19 [U-Boot] [PATCH] dm: video: make ANSI escape sequence support optional Anatolij Gustschin
@ 2017-09-30 16:20 ` Rob Clark
2017-10-06 16:15 ` Anatolij Gustschin
1 sibling, 0 replies; 6+ messages in thread
From: Rob Clark @ 2017-09-30 16:20 UTC (permalink / raw)
To: u-boot
On Sat, Sep 30, 2017 at 4:19 AM, Anatolij Gustschin <agust@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_ANSI
> is 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
> Signed-off-by: Anatolij Gustschin <agust@denx.de>
> ---
> 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(+)
>
> diff --git a/drivers/video/vidconsole-uclass.c b/drivers/video/vidconsole-uclass.c
> index 5f63c12d6c..f62e9f9308 100644
> --- a/drivers/video/vidconsole-uclass.c
> +++ b/drivers/video/vidconsole-uclass.c
> @@ -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;
> diff --git a/include/video_console.h b/include/video_console.h
> index 9dce234bd9..f07f43c1e2 100644
> --- a/include/video_console.h
> +++ b/include/video_console.h
> @@ -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
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] dm: video: make ANSI escape sequence support optional
2017-09-30 8:19 [U-Boot] [PATCH] dm: video: make ANSI escape sequence support optional Anatolij Gustschin
2017-09-30 16:20 ` Rob Clark
@ 2017-10-06 16:15 ` Anatolij Gustschin
2017-10-06 16:56 ` Rob Clark
1 sibling, 1 reply; 6+ messages in thread
From: Anatolij Gustschin @ 2017-10-06 16:15 UTC (permalink / raw)
To: u-boot
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_ANSI
> is not enabled.
>
> Signed-off-by: Anatolij Gustschin <agust@denx.de>
> ---
> 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(+)
Applied to u-boot-video/master.
--
Anatolij
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] dm: video: make ANSI escape sequence support optional
2017-10-06 16:15 ` Anatolij Gustschin
@ 2017-10-06 16:56 ` Rob Clark
2017-10-06 17:04 ` Simon Glass
0 siblings, 1 reply; 6+ messages in thread
From: Rob Clark @ 2017-10-06 16:56 UTC (permalink / raw)
To: u-boot
On Fri, Oct 6, 2017 at 12:15 PM, Anatolij Gustschin <agust@denx.de> wrote:
> 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_ANSI
>> is not enabled.
>>
>> Signed-off-by: Anatolij Gustschin <agust@denx.de>
>> ---
>> 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(+)
>
> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] dm: video: make ANSI escape sequence support optional
2017-10-06 16:56 ` Rob Clark
@ 2017-10-06 17:04 ` Simon Glass
2017-10-09 8:15 ` Anatolij Gustschin
0 siblings, 1 reply; 6+ messages in thread
From: Simon Glass @ 2017-10-06 17:04 UTC (permalink / raw)
To: u-boot
Hi Rob,
On 6 October 2017 at 10:56, Rob Clark <robdclark@gmail.com> wrote:
>
> On Fri, Oct 6, 2017 at 12:15 PM, Anatolij Gustschin <agust@denx.de> wrote:
> > 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_ANSI
> >> is not enabled.
> >>
> >> Signed-off-by: Anatolij Gustschin <agust@denx.de>
> >> ---
> >> 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(+)
> >
> > 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
Seems reasonable to me.
Regards,
Simon
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] dm: video: make ANSI escape sequence support optional
2017-10-06 17:04 ` Simon Glass
@ 2017-10-09 8:15 ` Anatolij Gustschin
0 siblings, 0 replies; 6+ messages in thread
From: Anatolij Gustschin @ 2017-10-09 8:15 UTC (permalink / raw)
To: u-boot
Hi Rob, Simon,
On Fri, 6 Oct 2017 11:04:24 -0600
Simon Glass sjg at chromium.org wrote:
...
> > So I think you should drop/revert this patch
>
> Seems reasonable to me.
OK, I dropped it.
Thanks,
--
Anatolij
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-10-09 8:15 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-09-30 8:19 [U-Boot] [PATCH] dm: video: make ANSI escape sequence support optional Anatolij Gustschin
2017-09-30 16:20 ` Rob Clark
2017-10-06 16:15 ` Anatolij Gustschin
2017-10-06 16:56 ` Rob Clark
2017-10-06 17:04 ` Simon Glass
2017-10-09 8:15 ` Anatolij Gustschin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox