[U-Boot] [PATCH 1/2] lcd: add functions to setup up simplefb device tree

[Simon Glass]

Hi Stephen,

On Tue, May 7, 2013 at 10:21 PM, Stephen Warren <swarren at wwwdotorg.org> wrote:
> simple-framebuffer is a new device tree binding that describes a pre-
> configured frame-buffer memory region and its format. The Linux kernel
> contains a driver that supports this binding. Implement functions to
> create a DT node (or fill in an existing node) with parameters that
> describe the framebuffer format that U-Boot is using.
>
> This will be immediately used by the Raspberry Pi board in U-Boot, and
> likely will be used by the Samsung ARM ChromeBook support soon too. It
> could well be used by many other boards (e.g. Tegra boards with built-in
> LCD panels, which aren't yet supported by the Linux kernel).
>
> Signed-off-by: Stephen Warren <swarren at wwwdotorg.org>
> ---
>  common/lcd.c  |   87 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/lcd.h |    5 ++++
>  2 files changed, 92 insertions(+)

This looks OK - is a better place for it than the common lcd code? I
presume it is here because it accesses panel_info?

Also is there a binding file we can bring in?

>
> diff --git a/common/lcd.c b/common/lcd.c
> index edae835..3a60484 100644
> --- a/common/lcd.c
> +++ b/common/lcd.c
> @@ -57,6 +57,10 @@
>  #include <atmel_lcdc.h>
>  #endif
>
> +#if defined(CONFIG_LCD_DT_SIMPLEFB)
> +#include <libfdt.h>
> +#endif
> +
>  /************************************************************************/
>  /* ** FONT DATA                                                                */
>  /************************************************************************/
> @@ -1182,3 +1186,86 @@ int lcd_get_screen_columns(void)
>  {
>         return CONSOLE_COLS;
>  }
> +
> +#if defined(CONFIG_LCD_DT_SIMPLEFB)
> +static int lcd_dt_simplefb_configure_node(void *blob, int off)
> +{
> +       u32 stride;
> +       fdt32_t cells[2];
> +       int ret;
> +       const char format[] =
> +#if LCD_BPP == LCD_COLOR16
> +               "r5g6b5";
> +#else
> +               "";
> +#endif
> +
> +       if (!format[0])
> +               return -1;
> +
> +       stride = panel_info.vl_col * 2;
> +
> +       cells[0] = cpu_to_fdt32(gd->fb_base);
> +       cells[1] = cpu_to_fdt32(stride * panel_info.vl_row);
> +       ret = fdt_setprop(blob, off, "reg", cells, sizeof(cells[0]) * 2);
> +       if (ret < 0)
> +               return -1;
> +
> +       cells[0] = cpu_to_fdt32(panel_info.vl_col);
> +       ret = fdt_setprop(blob, off, "width", cells, sizeof(cells[0]));
> +       if (ret < 0)
> +               return -1;
> +
> +       cells[0] = cpu_to_fdt32(panel_info.vl_row);
> +       ret = fdt_setprop(blob, off, "height", cells, sizeof(cells[0]));
> +       if (ret < 0)
> +               return -1;
> +
> +       cells[0] = cpu_to_fdt32(stride);
> +       ret = fdt_setprop(blob, off, "stride", cells, sizeof(cells[0]));
> +       if (ret < 0)
> +               return -1;
> +
> +       ret = fdt_setprop(blob, off, "format", format, strlen(format) + 1);
> +       if (ret < 0)
> +               return -1;
> +
> +       ret = fdt_delprop(blob, off, "status");
> +       if (ret < 0)
> +               return -1;
> +
> +       return 0;
> +}
> +
> +int lcd_dt_simplefb_add_node(void *blob)

Can we not automatically find the node and update it?

> +{
> +       const char compat[] = "simple-framebuffer";
> +       const char disabled[] = "disabled";
> +       int off, ret;
> +
> +       off = fdt_add_subnode(blob, 0, "framebuffer");
> +       if (off < 0)
> +               return -1;
> +
> +       ret = fdt_setprop(blob, off, "status", disabled, sizeof(disabled));
> +       if (ret < 0)
> +               return -1;
> +
> +       ret = fdt_setprop(blob, off, "compatible", compat, sizeof(compat));
> +       if (ret < 0)
> +               return -1;
> +
> +       return lcd_dt_simplefb_configure_node(blob, off);
> +}
> +
> +int lcd_dt_simplefb_enable_existing_node(void *blob)
> +{
> +       int off;
> +
> +       off = fdt_node_offset_by_compatible(blob, -1, "simple-framebuffer");

Do we ever need to support more than one node, iwc perhaps we want to
pass in the offset? If not, then see above question re searching.

> +       if (off < 0)
> +               return -1;
> +
> +       return lcd_dt_simplefb_configure_node(blob, off);
> +}
> +#endif
> diff --git a/include/lcd.h b/include/lcd.h
> index c6e7fc5..7c5410d 100644
> --- a/include/lcd.h
> +++ b/include/lcd.h
> @@ -324,6 +324,11 @@ void lcd_show_board_info(void);
>  /* Return the size of the LCD frame buffer, and the line length */
>  int lcd_get_size(int *line_length);
>
> +#if defined(CONFIG_LCD_DT_SIMPLEFB)

Probably don't need this #ifdef? It will complicate things if we use
the autoconf series.

> +int lcd_dt_simplefb_add_node(void *blob);
> +int lcd_dt_simplefb_enable_existing_node(void *blob);
> +#endif
> +
>  /************************************************************************/
>  /* ** BITMAP DISPLAY SUPPORT                                           */
>  /************************************************************************/
> --
> 1.7.10.4
>

Regards,
Simon