diff mbox

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

Message ID 1317976092-7498-3-git-send-email-clchiou@chromium.org
State Superseded
Delegated to: Anatolij Gustschin
Headers show

Commit Message

Che-liang Chiou Oct. 7, 2011, 8:28 a.m. UTC 
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>
---

Changes since V1
  Fix style errors

 api/Makefile         |    3 +-
 api/api.c            |   51 +++++++++++++++++++++++++++++
 api/api_display.c    |   86 ++++++++++++++++++++++++++++++++++++++++++++++++++
 api/api_private.h    |    4 ++
 examples/api/demo.c  |   31 ++++++++++++++++++
 examples/api/glue.c  |   31 ++++++++++++++++++
 examples/api/glue.h  |    5 +++
 include/api_public.h |   16 +++++++++
 include/video_font.h |    6 +++
 9 files changed, 232 insertions(+), 1 deletions(-)
 create mode 100644 api/api_display.c

Comments

Anatolij Gustschin Oct. 17, 2011, 9:13 p.m. UTC | #1 
Hi,

thanks for the patch and style fixes! I've some comments on it.
Please see below.

On Fri,  7 Oct 2011 16:28:12 +0800
Che-Liang Chiou <clchiou@chromium.org> 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.

Many boards defining CONFIG_LCD also define CONFIG_LCD_LOGO.
Enabling CONFIG_API for such board configurations will break
compiling, e.g.:

$ ./MAKEALL TQM823L_LCD
Configuring for TQM823L_LCD - Board: TQM823L, Options: LCD,NEC_NL6448BC20
api_display.c: In function 'display_get_info':
api_display.c:40: error: 'BMP_LOGO_HEIGHT' undeclared (first use in this function)
api_display.c:40: error: (Each undeclared identifier is reported only once
api_display.c:40: error: for each function it appears in.)
make[1]: *** [api_display.o] Error 1
make: *** [api/libapi.o] Error 2

Any idea how to fix this issue?

Similar problem exists for boards using cfb_console driver, e.g. enabling
CONFIG_API breaks compiling for tqm5200 board:

$ ./MAKEALL TQM5200
Configuring for TQM5200 board...
api_display.c: In function 'display_get_info':
api_display.c:47: error: 'VIDEO_VISIBLE_COLS' undeclared (first use in this function)
api_display.c:47: error: (Each undeclared identifier is reported only once
api_display.c:47: error: for each function it appears in.)
api_display.c:48: error: 'VIDEO_VISIBLE_ROWS' undeclared (first use in this function)
make[1]: *** [api_display.o] Error 1
make: *** [api/libapi.o] Error 2

We need to resolve these issues before committing this patch, i think.

Thanks,
Anatolij

> Signed-off-by: Che-Liang Chiou <clchiou@chromium.org>
> ---
> 
> Changes since V1
>   Fix style errors
> 
>  api/Makefile         |    3 +-
>  api/api.c            |   51 +++++++++++++++++++++++++++++
>  api/api_display.c    |   86 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  api/api_private.h    |    4 ++
>  examples/api/demo.c  |   31 ++++++++++++++++++
>  examples/api/glue.c  |   31 ++++++++++++++++++
>  examples/api/glue.h  |    5 +++
>  include/api_public.h |   16 +++++++++
>  include/video_font.h |    6 +++
>  9 files changed, 232 insertions(+), 1 deletions(-)
>  create mode 100644 api/api_display.c
Che-liang Chiou Oct. 18, 2011, 6:12 a.m. UTC | #2 
Hi Anatolij,

Thanks for testing this patch. Please see below.

On Tue, Oct 18, 2011 at 5:13 AM, Anatolij Gustschin <agust@denx.de> wrote:
> Hi,
>
> thanks for the patch and style fixes! I've some comments on it.
> Please see below.
>
> On Fri,  7 Oct 2011 16:28:12 +0800
> Che-Liang Chiou <clchiou@chromium.org> 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.
>
> Many boards defining CONFIG_LCD also define CONFIG_LCD_LOGO.
> Enabling CONFIG_API for such board configurations will break
> compiling, e.g.:
>
> $ ./MAKEALL TQM823L_LCD
> Configuring for TQM823L_LCD - Board: TQM823L, Options: LCD,NEC_NL6448BC20
> api_display.c: In function 'display_get_info':
> api_display.c:40: error: 'BMP_LOGO_HEIGHT' undeclared (first use in this function)
> api_display.c:40: error: (Each undeclared identifier is reported only once
> api_display.c:40: error: for each function it appears in.)
> make[1]: *** [api_display.o] Error 1
> make: *** [api/libapi.o] Error 2
>
> Any idea how to fix this issue?
>

BMP_LOGO_HEIGHT is defined bmp_logo.h, which is automatically
generated by tools/bmp_logo.c when CONFIG_LCD_LOGO or
CONFIG_VIDEO_LOGO is set. So I guess this is quite easy to fix. We
could include bmp_logo.h in api_display.c.

> Similar problem exists for boards using cfb_console driver, e.g. enabling
> CONFIG_API breaks compiling for tqm5200 board:
>
> $ ./MAKEALL TQM5200
> Configuring for TQM5200 board...
> api_display.c: In function 'display_get_info':
> api_display.c:47: error: 'VIDEO_VISIBLE_COLS' undeclared (first use in this function)
> api_display.c:47: error: (Each undeclared identifier is reported only once
> api_display.c:47: error: for each function it appears in.)
> api_display.c:48: error: 'VIDEO_VISIBLE_ROWS' undeclared (first use in this function)
> make[1]: *** [api_display.o] Error 1
> make: *** [api/libapi.o] Error 2
>
> We need to resolve these issues before committing this patch, i think.
>

VIDEO_VISIBLE_{COLS,ROLS} are just macros defined in cfb_console.c,
this is my mistake. I should use GraphicDevice->{winSizeX,winSizeY}
that are VIDEO_VISIBLE_{COLS,ROLS} defined upon.

> Thanks,
> Anatolij
>
>> Signed-off-by: Che-Liang Chiou <clchiou@chromium.org>
>> ---
>>
>> Changes since V1
>>   Fix style errors
>>
>>  api/Makefile         |    3 +-
>>  api/api.c            |   51 +++++++++++++++++++++++++++++
>>  api/api_display.c    |   86 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>  api/api_private.h    |    4 ++
>>  examples/api/demo.c  |   31 ++++++++++++++++++
>>  examples/api/glue.c  |   31 ++++++++++++++++++
>>  examples/api/glue.h  |    5 +++
>>  include/api_public.h |   16 +++++++++
>>  include/video_font.h |    6 +++
>>  9 files changed, 232 insertions(+), 1 deletions(-)
>>  create mode 100644 api/api_display.c
>

Regards,
Che-Liang
Anatolij Gustschin Oct. 18, 2011, 7:17 a.m. UTC | #3 
Hi,

On Tue, 18 Oct 2011 14:12:59 +0800
Che-liang Chiou <clchiou@chromium.org> wrote:
...
> > Many boards defining CONFIG_LCD also define CONFIG_LCD_LOGO.
> > Enabling CONFIG_API for such board configurations will break
> > compiling, e.g.:
> >
> > $ ./MAKEALL TQM823L_LCD
> > Configuring for TQM823L_LCD - Board: TQM823L, Options: LCD,NEC_NL6448BC20
> > api_display.c: In function 'display_get_info':
> > api_display.c:40: error: 'BMP_LOGO_HEIGHT' undeclared (first use in this function)
> > api_display.c:40: error: (Each undeclared identifier is reported only once
> > api_display.c:40: error: for each function it appears in.)
> > make[1]: *** [api_display.o] Error 1
> > make: *** [api/libapi.o] Error 2
> >
> > Any idea how to fix this issue?
> >
> 
> BMP_LOGO_HEIGHT is defined bmp_logo.h, which is automatically
> generated by tools/bmp_logo.c when CONFIG_LCD_LOGO or
> CONFIG_VIDEO_LOGO is set. So I guess this is quite easy to fix. We
> could include bmp_logo.h in api_display.c.

This won't work I'm afraid. bmp_logo.h is included elsewhere an including
it in libapi will cause multiple definition of `bmp_logo_bitmap' and
`bmp_logo_palette' compile errors.

Thanks,
Anatolij
Che-liang Chiou Oct. 18, 2011, 8:24 a.m. UTC | #4 
Hi Anatolij,

On Tue, Oct 18, 2011 at 3:17 PM, Anatolij Gustschin <agust@denx.de> wrote:
> Hi,
>
> On Tue, 18 Oct 2011 14:12:59 +0800
> Che-liang Chiou <clchiou@chromium.org> wrote:
> ...
>> > Many boards defining CONFIG_LCD also define CONFIG_LCD_LOGO.
>> > Enabling CONFIG_API for such board configurations will break
>> > compiling, e.g.:
>> >
>> > $ ./MAKEALL TQM823L_LCD
>> > Configuring for TQM823L_LCD - Board: TQM823L, Options: LCD,NEC_NL6448BC20
>> > api_display.c: In function 'display_get_info':
>> > api_display.c:40: error: 'BMP_LOGO_HEIGHT' undeclared (first use in this function)
>> > api_display.c:40: error: (Each undeclared identifier is reported only once
>> > api_display.c:40: error: for each function it appears in.)
>> > make[1]: *** [api_display.o] Error 1
>> > make: *** [api/libapi.o] Error 2
>> >
>> > Any idea how to fix this issue?
>> >
>>
>> BMP_LOGO_HEIGHT is defined bmp_logo.h, which is automatically
>> generated by tools/bmp_logo.c when CONFIG_LCD_LOGO or
>> CONFIG_VIDEO_LOGO is set. So I guess this is quite easy to fix. We
>> could include bmp_logo.h in api_display.c.
>
> This won't work I'm afraid. bmp_logo.h is included elsewhere an including
> it in libapi will cause multiple definition of `bmp_logo_bitmap' and
> `bmp_logo_palette' compile errors.
>
> Thanks,
> Anatolij
>

I was planning to add a 'static __attribute__((unused))' to it to
avoid this problem. What do you think?

Regards,
Che-Liang
diff mbox

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..4746d20
--- /dev/null
+++ b/api/api_display.c
@@ -0,0 +1,86 @@ 
+/*
+ * 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..19d38f6 100644
--- a/examples/api/demo.c
+++ b/examples/api/demo.c
@@ -48,6 +48,7 @@  int main(int argc, char * const argv[])
 	ulong start, now;
 	struct device_info *di;
 	lbasize_t rlen;
+	struct display_info disinfo;
 
 	if (!api_search_sig(&sig))
 		return -1;
@@ -176,6 +177,36 @@  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");
+
+	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. It is
+	 * equivalent to a no-op if no display is available.
+	 */
+	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 '^@' */