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

2011-10-09 Thread Wolfgang Denk
Dear Che-liang Chiou,

In message CANJuy2LVSzvodiim096iqj2kJ3gGD4dR=9-bctO=hjm4nhf...@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

-- 
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: w...@denx.de
As of 1992, they're called European Economic Community fries.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


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

2011-10-07 Thread Che-liang Chiou
Dear Wolfgang Denk,
On Fri, Oct 7, 2011 at 2:33 AM, Wolfgang Denk w...@denx.de wrote:
 Dear Che-Liang Chiou,

 In message 
 ce61e361e4b8456b9e0584a72f37d2e278ff2ab1.1317715598.git.clch...@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: w...@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 7...@jpl-devvax.jpl.nasa.gov


Regards,
Che-Liang
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


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

2011-10-07 Thread Wolfgang Denk
Dear Che-liang Chiou,

In message canjuy2kuhg+4kueeupj0pbefkjdhajjnt_8-vulxyuxh9oc...@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: w...@denx.de
He was so narrow minded he could see through  a  keyhole  with  both
eyes ...
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


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

2011-10-07 Thread Che-liang Chiou
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 w...@denx.de wrote:
 Dear Che-liang Chiou,

 In message 
 canjuy2kuhg+4kueeupj0pbefkjdhajjnt_8-vulxyuxh9oc...@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: w...@denx.de
 He was so narrow minded he could see through  a  keyhole  with  both
 eyes ...

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


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

2011-10-06 Thread Wolfgang Denk
Dear Che-Liang Chiou,

In message 
ce61e361e4b8456b9e0584a72f37d2e278ff2ab1.1317715598.git.clch...@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

-- 
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: w...@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 7...@jpl-devvax.jpl.nasa.gov
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


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

2011-10-04 Thread Che-Liang Chiou
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 clch...@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

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 000..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;
+