Re: [PATCH 4/4] commands: version: Add framebuffer output support
On Thu, Feb 8, 2018 at 11:29 PM, Sascha Hauerwrote: > Hi Andrey, > > On Mon, Feb 05, 2018 at 09:29:35AM -0800, Andrey Smirnov wrote: >> Extend "version" command to be capable of rendeing version information >> on framebuffer devices. >> >> Signed-off-by: Andrey Smirnov >> --- >> commands/Kconfig | 8 +++ >> commands/version.c | 198 >> - >> 2 files changed, 204 insertions(+), 2 deletions(-) >> >> diff --git a/commands/Kconfig b/commands/Kconfig >> index 095536849..1cad5d608 100644 >> --- a/commands/Kconfig >> +++ b/commands/Kconfig >> @@ -238,6 +238,14 @@ config CMD_VERSION >> >> barebox 2014.05.0-00142-gb289373 #177 Mon May 12 20:35:55 CEST 2014 >> >> +config CMD_VERSION_OUTPUT_TO_FRAMEBUFFER >> + bool >> + depends on CMD_VERSION >> + prompt "support displaying version via framebuffer" >> + help >> + Selecting this option will enable version command to output >> + version information onto display backed by a frambuffer >> + >> config CMD_MMC_EXTCSD >> tristate >> prompt "read/write eMMC ext. CSD register" >> diff --git a/commands/version.c b/commands/version.c >> index 090f2dd13..ba74a993c 100644 >> --- a/commands/version.c >> +++ b/commands/version.c >> @@ -20,16 +20,210 @@ >> #include >> #include >> #include >> +#include >> +#include >> +#include >> +#include >> + >> +#define FRAME_SIZE 8 >> + >> +struct placement { >> + int x, y; >> +}; >> + >> +typedef struct placement placement_function_t (struct screen *, int, int); >> + >> +struct placement >> +placement_center(struct screen *sc, int text_width, int text_height) >> +{ >> + struct placement p; >> + >> + p.x = (sc->info->xres - text_width) / 2; >> + p.y = (sc->info->yres - text_height) / 2; >> + >> + return p; >> +} >> + >> +struct placement >> +placement_upper_left(struct screen *sc, int text_width, int text_height) >> +{ >> + struct placement p; >> + >> + p.x = FRAME_SIZE; >> + p.y = FRAME_SIZE; >> + >> + return p; >> +} >> + >> +struct placement >> +placement_upper_right(struct screen *sc, int text_width, int text_height) >> +{ >> + struct placement p; >> + >> + p.x = sc->info->xres - 1 - text_width - FRAME_SIZE; >> + p.y = FRAME_SIZE; >> + >> + return p; >> +} >> + >> +struct placement >> +placement_lower_left(struct screen *sc, int text_width, int text_height) >> +{ >> + struct placement p; >> + >> + p.x = FRAME_SIZE; >> + p.y = sc->info->yres - 1 - text_height - FRAME_SIZE; >> + >> + return p; >> +} >> + >> +struct placement >> +placement_lower_right(struct screen *sc, int text_width, int text_height) >> +{ >> + struct placement p; >> + >> + p.x = sc->info->xres - 1 - text_width - FRAME_SIZE; >> + p.y = sc->info->yres - 1 - text_height - FRAME_SIZE; >> + >> + return p; >> +} >> >> static int do_version(int argc, char *argv[]) >> { >> - printf ("\n%s\n", version_string); >> + const struct font_desc *font; >> + const char *fbdev = NULL; >> + struct screen *sc; >> + u8 fg[3], bg[3]; >> + int text_width; >> + int text_height; >> + >> + placement_function_t *place; >> + struct placement placement; >> + >> + if (IS_ENABLED(CONFIG_CMD_VERSION_OUTPUT_TO_FRAMEBUFFER)) { > > This is quite much code added to the version command, given that the > command previously only was a one-liner. I think this should rather be a > separate command. This could depend on framebuffer and other needed > stuff which would make the first two patches unnecessary. I think > stubbing away graphics functions when the user is graphics-only code is > a bit over the top. > OK, will change in v2. > The placement functions look as if they should be moved to some font > handling generic code. OK, will do. > > Also it's always nice to have a C API for anything we also have as > command. It has proven to be useful so many times. > Sure, will do. >> +BAREBOX_CMD_HELP_OPT ("-f color\t", "foreground color, in hex RRGGBB >> format") >> +BAREBOX_CMD_HELP_OPT ("-b color\t", "background color, in hex RRGGBB >> format") > > I'm not sure how useful these are as parameters to a command. How about > a nv variable? This could be used by the framebuffer console aswell. > Sure, sounds reasonable. Thanks, Andrey Smirnov ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
Re: [PATCH 4/4] commands: version: Add framebuffer output support
Hi Andrey, On Mon, Feb 05, 2018 at 09:29:35AM -0800, Andrey Smirnov wrote: > Extend "version" command to be capable of rendeing version information > on framebuffer devices. > > Signed-off-by: Andrey Smirnov> --- > commands/Kconfig | 8 +++ > commands/version.c | 198 > - > 2 files changed, 204 insertions(+), 2 deletions(-) > > diff --git a/commands/Kconfig b/commands/Kconfig > index 095536849..1cad5d608 100644 > --- a/commands/Kconfig > +++ b/commands/Kconfig > @@ -238,6 +238,14 @@ config CMD_VERSION > > barebox 2014.05.0-00142-gb289373 #177 Mon May 12 20:35:55 CEST 2014 > > +config CMD_VERSION_OUTPUT_TO_FRAMEBUFFER > + bool > + depends on CMD_VERSION > + prompt "support displaying version via framebuffer" > + help > + Selecting this option will enable version command to output > + version information onto display backed by a frambuffer > + > config CMD_MMC_EXTCSD > tristate > prompt "read/write eMMC ext. CSD register" > diff --git a/commands/version.c b/commands/version.c > index 090f2dd13..ba74a993c 100644 > --- a/commands/version.c > +++ b/commands/version.c > @@ -20,16 +20,210 @@ > #include > #include > #include > +#include > +#include > +#include > +#include > + > +#define FRAME_SIZE 8 > + > +struct placement { > + int x, y; > +}; > + > +typedef struct placement placement_function_t (struct screen *, int, int); > + > +struct placement > +placement_center(struct screen *sc, int text_width, int text_height) > +{ > + struct placement p; > + > + p.x = (sc->info->xres - text_width) / 2; > + p.y = (sc->info->yres - text_height) / 2; > + > + return p; > +} > + > +struct placement > +placement_upper_left(struct screen *sc, int text_width, int text_height) > +{ > + struct placement p; > + > + p.x = FRAME_SIZE; > + p.y = FRAME_SIZE; > + > + return p; > +} > + > +struct placement > +placement_upper_right(struct screen *sc, int text_width, int text_height) > +{ > + struct placement p; > + > + p.x = sc->info->xres - 1 - text_width - FRAME_SIZE; > + p.y = FRAME_SIZE; > + > + return p; > +} > + > +struct placement > +placement_lower_left(struct screen *sc, int text_width, int text_height) > +{ > + struct placement p; > + > + p.x = FRAME_SIZE; > + p.y = sc->info->yres - 1 - text_height - FRAME_SIZE; > + > + return p; > +} > + > +struct placement > +placement_lower_right(struct screen *sc, int text_width, int text_height) > +{ > + struct placement p; > + > + p.x = sc->info->xres - 1 - text_width - FRAME_SIZE; > + p.y = sc->info->yres - 1 - text_height - FRAME_SIZE; > + > + return p; > +} > > static int do_version(int argc, char *argv[]) > { > - printf ("\n%s\n", version_string); > + const struct font_desc *font; > + const char *fbdev = NULL; > + struct screen *sc; > + u8 fg[3], bg[3]; > + int text_width; > + int text_height; > + > + placement_function_t *place; > + struct placement placement; > + > + if (IS_ENABLED(CONFIG_CMD_VERSION_OUTPUT_TO_FRAMEBUFFER)) { This is quite much code added to the version command, given that the command previously only was a one-liner. I think this should rather be a separate command. This could depend on framebuffer and other needed stuff which would make the first two patches unnecessary. I think stubbing away graphics functions when the user is graphics-only code is a bit over the top. The placement functions look as if they should be moved to some font handling generic code. Also it's always nice to have a C API for anything we also have as command. It has proven to be useful so many times. > +BAREBOX_CMD_HELP_OPT ("-f color\t", "foreground color, in hex RRGGBB format") > +BAREBOX_CMD_HELP_OPT ("-b color\t", "background color, in hex RRGGBB format") I'm not sure how useful these are as parameters to a command. How about a nv variable? This could be used by the framebuffer console aswell. Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
[PATCH 4/4] commands: version: Add framebuffer output support
Extend "version" command to be capable of rendeing version information on framebuffer devices. Signed-off-by: Andrey Smirnov--- commands/Kconfig | 8 +++ commands/version.c | 198 - 2 files changed, 204 insertions(+), 2 deletions(-) diff --git a/commands/Kconfig b/commands/Kconfig index 095536849..1cad5d608 100644 --- a/commands/Kconfig +++ b/commands/Kconfig @@ -238,6 +238,14 @@ config CMD_VERSION barebox 2014.05.0-00142-gb289373 #177 Mon May 12 20:35:55 CEST 2014 +config CMD_VERSION_OUTPUT_TO_FRAMEBUFFER + bool + depends on CMD_VERSION + prompt "support displaying version via framebuffer" + help + Selecting this option will enable version command to output + version information onto display backed by a frambuffer + config CMD_MMC_EXTCSD tristate prompt "read/write eMMC ext. CSD register" diff --git a/commands/version.c b/commands/version.c index 090f2dd13..ba74a993c 100644 --- a/commands/version.c +++ b/commands/version.c @@ -20,16 +20,210 @@ #include #include #include +#include +#include +#include +#include + +#define FRAME_SIZE 8 + +struct placement { + int x, y; +}; + +typedef struct placement placement_function_t (struct screen *, int, int); + +struct placement +placement_center(struct screen *sc, int text_width, int text_height) +{ + struct placement p; + + p.x = (sc->info->xres - text_width) / 2; + p.y = (sc->info->yres - text_height) / 2; + + return p; +} + +struct placement +placement_upper_left(struct screen *sc, int text_width, int text_height) +{ + struct placement p; + + p.x = FRAME_SIZE; + p.y = FRAME_SIZE; + + return p; +} + +struct placement +placement_upper_right(struct screen *sc, int text_width, int text_height) +{ + struct placement p; + + p.x = sc->info->xres - 1 - text_width - FRAME_SIZE; + p.y = FRAME_SIZE; + + return p; +} + +struct placement +placement_lower_left(struct screen *sc, int text_width, int text_height) +{ + struct placement p; + + p.x = FRAME_SIZE; + p.y = sc->info->yres - 1 - text_height - FRAME_SIZE; + + return p; +} + +struct placement +placement_lower_right(struct screen *sc, int text_width, int text_height) +{ + struct placement p; + + p.x = sc->info->xres - 1 - text_width - FRAME_SIZE; + p.y = sc->info->yres - 1 - text_height - FRAME_SIZE; + + return p; +} static int do_version(int argc, char *argv[]) { - printf ("\n%s\n", version_string); + const struct font_desc *font; + const char *fbdev = NULL; + struct screen *sc; + u8 fg[3], bg[3]; + int text_width; + int text_height; + + placement_function_t *place; + struct placement placement; + + if (IS_ENABLED(CONFIG_CMD_VERSION_OUTPUT_TO_FRAMEBUFFER)) { + const struct { + const char *name; + placement_function_t *func; + } placements[] = { + { "center", placement_center }, + { "upper-left", placement_upper_left }, + { "lower-left", placement_lower_left }, + { "upper-right", placement_upper_right }, + { "lower-right", placement_lower_right }, + }; + u32 color; + size_t i; + int opt; + u8 *c; + + place = placement_center; + + memset(fg, 0xff, sizeof(fg)); /* default to white */ + memset(bg, 0x00, sizeof(bg)); /* default to black */ + + while((opt = getopt(argc, argv, "d:f:b:p:")) > 0) { + switch(opt) { + case 'd': + fbdev = optarg; + break; + case 'f': + /* FALLTHROUGH */ + case 'b': + c = (opt == 'f') ? fg : bg; + color = simple_strtoul(optarg, NULL, 16); + + c[0] = color >> 16; + c[1] = color >> 8; + c[2] = color >> 0; + break; + case 'p': + place = NULL; + for (i = 0; i < ARRAY_SIZE(placements); i++) + if (!strcmp(optarg, placements[i].name)) + place = placements[i].func; + + if (!place) { + pr_err("Unknown placement: %s\n", + optarg); + return -EINVAL; +