Re: [PATCH 4/4] commands: version: Add framebuffer output support

2018-02-16 Thread Andrey Smirnov
On Thu, Feb 8, 2018 at 11:29 PM, Sascha Hauer  wrote:
> 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

2018-02-08 Thread Sascha Hauer
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

2018-02-05 Thread Andrey Smirnov
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;
+