Message ID | ce61e361e4b8456b9e0584a72f37d2e278ff2ab1.1317715598.git.clchiou@chromium.org |
---|---|
State | Superseded |
Delegated to: | Anatolij Gustschin |
Headers | show |
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
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
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
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 ..." >
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
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 '^@' */
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