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

Che-liang Chiou
2011-10-07T08:32:39Z

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 at 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