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

Reply via email to