Patchwork [U-Boot,2/2] api: export LCD and video to external apps

login
register
mail settings
Submitter Che-liang Chiou
Date Oct. 4, 2011, 8:16 a.m.
Message ID <ce61e361e4b8456b9e0584a72f37d2e278ff2ab1.1317715598.git.clchiou@chromium.org>
Download mbox | patch
Permalink /patch/117589/
State Superseded
Delegated to: Anatolij Gustschin
Headers show

Comments

Che-liang Chiou - Oct. 4, 2011, 8:16 a.m.
This patch exports LCD and video information and bitmap-rendering
functions to external apps.

This patch is tested on a Seaboard, which does not have a video output.
So I only tested LCD code paths.

NOTE: The Seaboard LCD driver is not yet upstreamed; the test was done
in a local downstream repo.

Signed-off-by: Che-Liang Chiou <clchiou@chromium.org>
---
 api/Makefile         |    3 +-
 api/api.c            |   51 ++++++++++++++++++++++++++++++
 api/api_display.c    |   85 ++++++++++++++++++++++++++++++++++++++++++++++++++
 api/api_private.h    |    4 ++
 examples/api/demo.c  |   28 ++++++++++++++++
 examples/api/glue.c  |   31 ++++++++++++++++++
 examples/api/glue.h  |    5 +++
 include/api_public.h |   16 +++++++++
 include/video_font.h |    6 +++
 9 files changed, 228 insertions(+), 1 deletions(-)
 create mode 100644 api/api_display.c
Wolfgang Denk - Oct. 6, 2011, 6:33 p.m.
Dear Che-Liang Chiou,

In message <ce61e361e4b8456b9e0584a72f37d2e278ff2ab1.1317715598.git.clchiou@chromium.org> you wrote:
> This patch exports LCD and video information and bitmap-rendering
> functions to external apps.
> 
> This patch is tested on a Seaboard, which does not have a video output.
> So I only tested LCD code paths.
> 
> NOTE: The Seaboard LCD driver is not yet upstreamed; the test was done
> in a local downstream repo.
...

> +	switch (type) {
> +	default:
> +		debug("%s: unsupport display device type: %d\n", __FILE__, type);

Line too long.

> +
> +#ifdef CONFIG_LCD
> +	case DISPLAY_TYPE_LCD:
> +		di->pixel_width  = panel_info.vl_col;
> +		di->pixel_height = panel_info.vl_row;
> +		di->screen_rows = CONSOLE_ROWS;
> +		di->screen_cols = CONSOLE_COLS;
> +		break;
> +#endif
> +
> +#if defined(CONFIG_VIDEO) || defined(CONFIG_CFB_CONSOLE)
> +	case DISPLAY_TYPE_VIDEO:
> +		di->pixel_width  = VIDEO_VISIBLE_COLS;
> +		di->pixel_height = VIDEO_VISIBLE_ROWS;
> +		di->screen_rows = CONSOLE_ROWS;
> +		di->screen_cols = CONSOLE_COLS;
> +		break;
> +#endif
> +	}
> +
> +	di->type = type;
> +	return 0;
> +}
> +
> +int display_draw_bitmap(ulong bitmap, int x, int y)
> +{
> +	int err = 0;
> +
> +	/*
> +	 * Although CONFIG_LCD and CONFIG_VIDEO or CONFIG_CFB_CONSOLE
> +	 * generally should not be both set, if this does happen, I think
> +	 * drawing on both of them makes more sense.
> +	 */
> +#ifdef CONFIG_LCD
> +	err |= lcd_display_bitmap(bitmap, x, y);
> +#endif
> +#if defined(CONFIG_VIDEO) || defined(CONFIG_CFB_CONSOLE)
> +	err |= video_display_bitmap(bitmap, x, y);
> +#endif
> +
> +	return err;
> +}
> +
> +void display_clear(void)
> +{
> +#ifdef CONFIG_LCD
> +	lcd_clear();
> +#endif
> +#if defined(CONFIG_VIDEO) || defined(CONFIG_CFB_CONSOLE)
> +	video_clear();
> +#endif
> +}
> diff --git a/api/api_private.h b/api/api_private.h
> index 94a7fc5..988f702 100644
> --- a/api/api_private.h
> +++ b/api/api_private.h
> @@ -45,4 +45,8 @@ int		dev_write_net(void *, void *, int);
>  
>  void dev_stor_init(void);
>  
> +int display_get_info(int type, struct display_info *di);
> +int display_draw_bitmap(ulong bitmap, int x, int y);
> +void display_clear(void);
> +
>  #endif /* _API_PRIVATE_H_ */
> diff --git a/examples/api/demo.c b/examples/api/demo.c
> index 65e7491..191bd79 100644
> --- a/examples/api/demo.c
> +++ b/examples/api/demo.c
> @@ -176,6 +176,34 @@ int main(int argc, char * const argv[])
>  	while ((env = ub_env_enum(env)) != NULL)
>  		printf("%s = %s\n", env, ub_env_get(env));
>  
> +	printf("\n*** Display ***\n");
> +
> +	struct display_info disinfo;

Please don't mix declarations and code.

> +	if (ub_display_get_info(DISPLAY_TYPE_LCD, &disinfo))
> +		printf("LCD info: failed\n");
> +	else {

Use braces in both branches.  Please fix globally.

> +	/* this only clears messages on screen, not on serial port */
> +	ub_display_clear();

What happens here if no display is available?

Best regards,

Wolfgang Denk
Che-liang Chiou - Oct. 7, 2011, 8:32 a.m.
Dear Wolfgang Denk,
On Fri, Oct 7, 2011 at 2:33 AM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Che-Liang Chiou,
>
> In message <ce61e361e4b8456b9e0584a72f37d2e278ff2ab1.1317715598.git.clchiou@chromium.org> you wrote:
>> This patch exports LCD and video information and bitmap-rendering
>> functions to external apps.
>>
>> This patch is tested on a Seaboard, which does not have a video output.
>> So I only tested LCD code paths.
>>
>> NOTE: The Seaboard LCD driver is not yet upstreamed; the test was done
>> in a local downstream repo.
> ...
>
>> +     switch (type) {
>> +     default:
>> +             debug("%s: unsupport display device type: %d\n", __FILE__, type);
>
> Line too long.
>
>> +
>> +#ifdef CONFIG_LCD
>> +     case DISPLAY_TYPE_LCD:
>> +             di->pixel_width  = panel_info.vl_col;
>> +             di->pixel_height = panel_info.vl_row;
>> +             di->screen_rows = CONSOLE_ROWS;
>> +             di->screen_cols = CONSOLE_COLS;
>> +             break;
>> +#endif
>> +
>> +#if defined(CONFIG_VIDEO) || defined(CONFIG_CFB_CONSOLE)
>> +     case DISPLAY_TYPE_VIDEO:
>> +             di->pixel_width  = VIDEO_VISIBLE_COLS;
>> +             di->pixel_height = VIDEO_VISIBLE_ROWS;
>> +             di->screen_rows = CONSOLE_ROWS;
>> +             di->screen_cols = CONSOLE_COLS;
>> +             break;
>> +#endif
>> +     }
>> +
>> +     di->type = type;
>> +     return 0;
>> +}
>> +
>> +int display_draw_bitmap(ulong bitmap, int x, int y)
>> +{
>> +     int err = 0;
>> +
>> +     /*
>> +      * Although CONFIG_LCD and CONFIG_VIDEO or CONFIG_CFB_CONSOLE
>> +      * generally should not be both set, if this does happen, I think
>> +      * drawing on both of them makes more sense.
>> +      */
>> +#ifdef CONFIG_LCD
>> +     err |= lcd_display_bitmap(bitmap, x, y);
>> +#endif
>> +#if defined(CONFIG_VIDEO) || defined(CONFIG_CFB_CONSOLE)
>> +     err |= video_display_bitmap(bitmap, x, y);
>> +#endif
>> +
>> +     return err;
>> +}
>> +
>> +void display_clear(void)
>> +{
>> +#ifdef CONFIG_LCD
>> +     lcd_clear();
>> +#endif
>> +#if defined(CONFIG_VIDEO) || defined(CONFIG_CFB_CONSOLE)
>> +     video_clear();
>> +#endif
>> +}
>> diff --git a/api/api_private.h b/api/api_private.h
>> index 94a7fc5..988f702 100644
>> --- a/api/api_private.h
>> +++ b/api/api_private.h
>> @@ -45,4 +45,8 @@ int         dev_write_net(void *, void *, int);
>>
>>  void dev_stor_init(void);
>>
>> +int display_get_info(int type, struct display_info *di);
>> +int display_draw_bitmap(ulong bitmap, int x, int y);
>> +void display_clear(void);
>> +
>>  #endif /* _API_PRIVATE_H_ */
>> diff --git a/examples/api/demo.c b/examples/api/demo.c
>> index 65e7491..191bd79 100644
>> --- a/examples/api/demo.c
>> +++ b/examples/api/demo.c
>> @@ -176,6 +176,34 @@ int main(int argc, char * const argv[])
>>       while ((env = ub_env_enum(env)) != NULL)
>>               printf("%s = %s\n", env, ub_env_get(env));
>>
>> +     printf("\n*** Display ***\n");
>> +
>> +     struct display_info disinfo;
>
> Please don't mix declarations and code.
>
>> +     if (ub_display_get_info(DISPLAY_TYPE_LCD, &disinfo))
>> +             printf("LCD info: failed\n");
>> +     else {
>
> Use braces in both branches.  Please fix globally.
>
>> +     /* this only clears messages on screen, not on serial port */
>> +     ub_display_clear();
>
> What happens here if no display is available?

It is equivalent to a no-op. Or do you think we should print a warning
message if no display is available?

>
> Best regards,
>
> Wolfgang Denk
>
> --
> DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
> Randal said it would be tough to do in sed. He didn't say  he  didn't
> understand  sed.  Randal  understands sed quite well. Which is why he
> uses Perl. :-)         - Larry Wall in <7874@jpl-devvax.JPL.NASA.GOV>
>

Regards,
Che-Liang
Wolfgang Denk - Oct. 7, 2011, 9:05 a.m.
Dear Che-liang Chiou,

In message <CANJuy2KuhG+4kueEupj0PBeFKJdhAJJnt_8-vULXYUXh9oCWKQ@mail.gmail.com> you wrote:
>
> >> +       ub_display_clear();
> >
> > What happens here if no display is available?
> 
> It is equivalent to a no-op. Or do you think we should print a warning
> message if no display is available?

Please either add an if() and don;t run the function at all, or add a
comment that explains that this callis harmless if no display is
present.

Best regards,

Wolfgang Denk
Che-liang Chiou - Oct. 7, 2011, 9:45 a.m.
Hi Wolfgang Denk,

I have added following comments to this call in patch V2.
+       /*+        * This only clears messages on screen, not on
serial port. It is+        * equivalent to a no-op if no display is
available.+        */Do you think it is sufficient?

Regards,
Che-Liang

On Fri, Oct 7, 2011 at 5:05 PM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Che-liang Chiou,
>
> In message <CANJuy2KuhG+4kueEupj0PBeFKJdhAJJnt_8-vULXYUXh9oCWKQ@mail.gmail.com> you wrote:
>>
>> >> +       ub_display_clear();
>> >
>> > What happens here if no display is available?
>>
>> It is equivalent to a no-op. Or do you think we should print a warning
>> message if no display is available?
>
> Please either add an if() and don;t run the function at all, or add a
> comment that explains that this callis harmless if no display is
> present.
>
> Best regards,
>
> Wolfgang Denk
>
> --
> DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
> "He was so narrow minded he could see through  a  keyhole  with  both
> eyes ..."
>
Wolfgang Denk - Oct. 9, 2011, 8 p.m.
Dear Che-liang Chiou,

In message <CANJuy2LVSzvodiim096iqj2kJ3gGD4dR=9-bctO=HJm4nhfNxg@mail.gmail.com> you wrote:
> Hi Wolfgang Denk,
> 
> I have added following comments to this call in patch V2.
> +          /*+            * This only clears me=
> ssages on screen, not on
> serial port. It is+            * equivalent to a no-op if n=
> o display is
> available.+            */Do you think it is sufficient?

Yes, this is OK with me.  Thanks.

Best regards,

Wolfgang Denk

Patch

diff --git a/api/Makefile b/api/Makefile
index 2a64c4d..0e99f74 100644
--- a/api/Makefile
+++ b/api/Makefile
@@ -24,7 +24,8 @@  include $(TOPDIR)/config.mk
 
 LIB	= $(obj)libapi.o
 
-COBJS-$(CONFIG_API) += api.o api_net.o api_storage.o api_platform-$(ARCH).o
+COBJS-$(CONFIG_API) += api.o api_display.o api_net.o api_storage.o \
+		       api_platform-$(ARCH).o
 
 COBJS	:= $(COBJS-y)
 SRCS	:= $(COBJS:.o=.c)
diff --git a/api/api.c b/api/api.c
index 853f010..ddf2edd 100644
--- a/api/api.c
+++ b/api/api.c
@@ -553,6 +553,54 @@  static int API_env_enum(va_list ap)
 	return 0;
 }
 
+/*
+ * pseudo signature:
+ *
+ * int API_display_get_info(int type, struct display_info *di)
+ */
+static int API_display_get_info(va_list ap)
+{
+	int type;
+	struct display_info *di;
+
+	type = (int)va_arg(ap, u_int32_t);
+	di = (struct display_info *)va_arg(ap, u_int32_t);
+	if (!di)
+		return API_EINVAL;
+
+	return display_get_info(type, di);
+}
+
+/*
+ * pseudo signature:
+ *
+ * int API_display_draw_bitmap(void *bitmap, int x, int y)
+ */
+static int API_display_draw_bitmap(va_list ap)
+{
+	ulong bitmap;
+	int x, y;
+
+	bitmap = (ulong)va_arg(ap, ulong);
+	if (!bitmap)
+		return API_EINVAL;
+	x = (int)va_arg(ap, u_int32_t);
+	y = (int)va_arg(ap, u_int32_t);
+
+	return display_draw_bitmap(bitmap, x, y);
+}
+
+/*
+ * pseudo signature:
+ *
+ * void API_display_clear(void)
+ */
+static int API_display_clear(va_list ap)
+{
+	display_clear();
+	return 0;
+}
+
 static cfp_t calls_table[API_MAXCALL] = { NULL, };
 
 /*
@@ -616,6 +664,9 @@  void api_init(void)
 	calls_table[API_ENV_GET] = &API_env_get;
 	calls_table[API_ENV_SET] = &API_env_set;
 	calls_table[API_ENV_ENUM] = &API_env_enum;
+	calls_table[API_DISPLAY_GET_INFO] = &API_display_get_info;
+	calls_table[API_DISPLAY_DRAW_BITMAP] = &API_display_draw_bitmap;
+	calls_table[API_DISPLAY_CLEAR] = &API_display_clear;
 	calls_no = API_MAXCALL;
 
 	debugf("API initialized with %d calls\n", calls_no);
diff --git a/api/api_display.c b/api/api_display.c
new file mode 100644
index 0000000..6255848
--- /dev/null
+++ b/api/api_display.c
@@ -0,0 +1,85 @@ 
+/*
+ * Copyright (c) 2011 The Chromium OS Authors.
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include <common.h>
+#include <lcd.h>
+#include <video.h>
+#include <video_font.h> /* Get font width and height */
+#include <api_public.h>
+
+int display_get_info(int type, struct display_info *di)
+{
+	switch (type) {
+	default:
+		debug("%s: unsupport display device type: %d\n", __FILE__, type);
+		return API_ENODEV;
+
+#ifdef CONFIG_LCD
+	case DISPLAY_TYPE_LCD:
+		di->pixel_width  = panel_info.vl_col;
+		di->pixel_height = panel_info.vl_row;
+		di->screen_rows = CONSOLE_ROWS;
+		di->screen_cols = CONSOLE_COLS;
+		break;
+#endif
+
+#if defined(CONFIG_VIDEO) || defined(CONFIG_CFB_CONSOLE)
+	case DISPLAY_TYPE_VIDEO:
+		di->pixel_width  = VIDEO_VISIBLE_COLS;
+		di->pixel_height = VIDEO_VISIBLE_ROWS;
+		di->screen_rows = CONSOLE_ROWS;
+		di->screen_cols = CONSOLE_COLS;
+		break;
+#endif
+	}
+
+	di->type = type;
+	return 0;
+}
+
+int display_draw_bitmap(ulong bitmap, int x, int y)
+{
+	int err = 0;
+
+	/*
+	 * Although CONFIG_LCD and CONFIG_VIDEO or CONFIG_CFB_CONSOLE
+	 * generally should not be both set, if this does happen, I think
+	 * drawing on both of them makes more sense.
+	 */
+#ifdef CONFIG_LCD
+	err |= lcd_display_bitmap(bitmap, x, y);
+#endif
+#if defined(CONFIG_VIDEO) || defined(CONFIG_CFB_CONSOLE)
+	err |= video_display_bitmap(bitmap, x, y);
+#endif
+
+	return err;
+}
+
+void display_clear(void)
+{
+#ifdef CONFIG_LCD
+	lcd_clear();
+#endif
+#if defined(CONFIG_VIDEO) || defined(CONFIG_CFB_CONSOLE)
+	video_clear();
+#endif
+}
diff --git a/api/api_private.h b/api/api_private.h
index 94a7fc5..988f702 100644
--- a/api/api_private.h
+++ b/api/api_private.h
@@ -45,4 +45,8 @@  int		dev_write_net(void *, void *, int);
 
 void dev_stor_init(void);
 
+int display_get_info(int type, struct display_info *di);
+int display_draw_bitmap(ulong bitmap, int x, int y);
+void display_clear(void);
+
 #endif /* _API_PRIVATE_H_ */
diff --git a/examples/api/demo.c b/examples/api/demo.c
index 65e7491..191bd79 100644
--- a/examples/api/demo.c
+++ b/examples/api/demo.c
@@ -176,6 +176,34 @@  int main(int argc, char * const argv[])
 	while ((env = ub_env_enum(env)) != NULL)
 		printf("%s = %s\n", env, ub_env_get(env));
 
+	printf("\n*** Display ***\n");
+
+	struct display_info disinfo;
+	if (ub_display_get_info(DISPLAY_TYPE_LCD, &disinfo))
+		printf("LCD info: failed\n");
+	else {
+		printf("LCD info:\n");
+		printf("  pixel width:  %d\n", disinfo.pixel_width);
+		printf("  pixel height: %d\n", disinfo.pixel_height);
+		printf("  screen rows:  %d\n", disinfo.screen_rows);
+		printf("  screen cols:  %d\n", disinfo.screen_cols);
+	}
+	if (ub_display_get_info(DISPLAY_TYPE_VIDEO, &disinfo))
+		printf("video info: failed\n");
+	else {
+		printf("video info:\n");
+		printf("  pixel width:  %d\n", disinfo.pixel_width);
+		printf("  pixel height: %d\n", disinfo.pixel_height);
+		printf("  screen rows:  %d\n", disinfo.screen_rows);
+		printf("  screen cols:  %d\n", disinfo.screen_cols);
+	}
+
+	printf("*** Press any key to continue ***\n");
+	printf("got char 0x%x\n", ub_getc());
+
+	/* this only clears messages on screen, not on serial port */
+	ub_display_clear();
+
 	/* reset */
 	printf("\n*** Resetting board ***\n");
 	ub_reset();
diff --git a/examples/api/glue.c b/examples/api/glue.c
index eff6a7e..d907e3f 100644
--- a/examples/api/glue.c
+++ b/examples/api/glue.c
@@ -402,3 +402,34 @@  const char * ub_env_enum(const char *last)
 
 	return env_name;
 }
+
+/****************************************
+ *
+ * display
+ *
+ ****************************************/
+
+int ub_display_get_info(int type, struct display_info *di)
+{
+	int err = 0;
+
+	if (!syscall(API_DISPLAY_GET_INFO, &err, (uint32_t)type, (uint32_t)di))
+		return API_ESYSC;
+
+	return err;
+}
+
+int ub_display_draw_bitmap(ulong bitmap, int x, int y)
+{
+	int err = 0;
+
+	if (!syscall(API_DISPLAY_DRAW_BITMAP, &err, bitmap, x, y))
+		return API_ESYSC;
+
+	return err;
+}
+
+void ub_display_clear(void)
+{
+	syscall(API_DISPLAY_CLEAR, NULL);
+}
diff --git a/examples/api/glue.h b/examples/api/glue.h
index 6bf47d0..e43f7d9 100644
--- a/examples/api/glue.h
+++ b/examples/api/glue.h
@@ -77,4 +77,9 @@  int			ub_dev_send(int handle, void *buf, int len);
 int			ub_dev_recv(int handle, void *buf, int len, int *rlen);
 struct device_info *	ub_dev_get(int);
 
+/* display */
+int ub_display_get_info(int type, struct display_info *di);
+int ub_display_draw_bitmap(ulong bitmap, int x, int y);
+void ub_display_clear(void);
+
 #endif /* _API_GLUE_H_ */
diff --git a/include/api_public.h b/include/api_public.h
index 5940d81..4420c99 100644
--- a/include/api_public.h
+++ b/include/api_public.h
@@ -90,6 +90,9 @@  enum {
 	API_ENV_ENUM,
 	API_ENV_GET,
 	API_ENV_SET,
+	API_DISPLAY_GET_INFO,
+	API_DISPLAY_DRAW_BITMAP,
+	API_DISPLAY_CLEAR,
 	API_MAXCALL
 };
 
@@ -152,4 +155,17 @@  struct device_info {
 	int	state;
 };
 
+#define DISPLAY_TYPE_LCD	0x0001
+#define DISPLAY_TYPE_VIDEO	0x0002
+
+struct display_info {
+	int type;
+	/* screen size in pixels */
+	int pixel_width;
+	int pixel_height;
+	/* screen size in rows and columns of text */
+	int screen_rows;
+	int screen_cols;
+};
+
 #endif /* _API_PUBLIC_H_ */
diff --git a/include/video_font.h b/include/video_font.h
index 706e185..5571a87 100644
--- a/include/video_font.h
+++ b/include/video_font.h
@@ -29,6 +29,12 @@ 
 #define VIDEO_FONT_HEIGHT	16
 #define VIDEO_FONT_SIZE		(VIDEO_FONT_CHARS * VIDEO_FONT_HEIGHT)
 
+/*
+ * Adding unused attribute here so that compiler will not complain
+ * video_fontdata is not used in source files that only use
+ * VIDEO_FONT_* (e.g., api/api_display.c).
+ */
+__attribute__ ((unused))
 static unsigned char video_fontdata[VIDEO_FONT_SIZE] = {
 
 	/* 0 0x00 '^@' */