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

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

[Stephen Warren]

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@wwwdotorg.org>
---
 common/lcd.c  |   87 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 include/lcd.h |    5 ++++
 2 files changed, 92 insertions(+)

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)
+{
+	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");
+	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)
+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

[U-Boot] [PATCH 2/2] ARM: bcm2835: add simplefb DT node during bootz/m

[Stephen Warren]

Add a DT simple-framebuffer node to DT when booting the Linux kernel.
This will allow the kernel to inherit the framebuffer configuration from
U-Boot, and display a graphical boot console, and even run a full SW-
rendered X server.

Signed-off-by: Stephen Warren <swarren@wwwdotorg.org>
---
 board/raspberrypi/rpi_b/rpi_b.c |   14 +++++++++++++-
 include/configs/rpi_b.h         |    2 ++
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/board/raspberrypi/rpi_b/rpi_b.c b/board/raspberrypi/rpi_b/rpi_b.c
index 6b3e095..16d442a 100644
--- a/board/raspberrypi/rpi_b/rpi_b.c
+++ b/board/raspberrypi/rpi_b/rpi_b.c
@@ -1,5 +1,5 @@
 /*
- * (C) Copyright 2012 Stephen Warren
+ * (C) Copyright 2012-2013 Stephen Warren
  *
  * See file CREDITS for list of people who contributed to this
  * project.
@@ -15,6 +15,8 @@
  */
 
 #include <common.h>
+#include <config.h>
+#include <lcd.h>
 #include <asm/arch/mbox.h>
 #include <asm/arch/sdhci.h>
 #include <asm/global_data.h>
@@ -77,3 +79,13 @@ int board_mmc_init(void)
 	return bcm2835_sdhci_init(BCM2835_SDHCI_BASE,
 				  msg_clk->get_clock_rate.body.resp.rate_hz);
 }
+
+void ft_board_setup(void *blob, bd_t *bd)
+{
+	/*
+	 * For now, we simply always add the simplefb DT node. Later, we
+	 * should be more intelligent, and e.g. only do this if no enabled DT
+	 * node exists for the "real" graphics driver.
+	 */
+	lcd_dt_simplefb_add_node(blob);
+}
diff --git a/include/configs/rpi_b.h b/include/configs/rpi_b.h
index c18b35b..216c6cb 100644
--- a/include/configs/rpi_b.h
+++ b/include/configs/rpi_b.h
@@ -61,6 +61,7 @@
 #define CONFIG_BCM2835_GPIO
 /* LCD */
 #define CONFIG_LCD
+#define CONFIG_LCD_DT_SIMPLEFB
 #define LCD_BPP				LCD_COLOR16
 /*
  * Prevent allocation of RAM for FB; the real FB address is queried
@@ -175,6 +176,7 @@
 
 /* Device tree support for bootm/bootz */
 #define CONFIG_OF_LIBFDT
+#define CONFIG_OF_BOARD_SETUP
 /* ATAGs support for bootm/bootz */
 #define CONFIG_SETUP_MEMORY_TAGS
 #define CONFIG_CMDLINE_TAG
-- 
1.7.10.4

[U-Boot] [PATCH 2/2] ARM: bcm2835: add simplefb DT node during bootz/m

[Simon Glass]

On Tue, May 7, 2013 at 10:21 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> Add a DT simple-framebuffer node to DT when booting the Linux kernel.
> This will allow the kernel to inherit the framebuffer configuration from
> U-Boot, and display a graphical boot console, and even run a full SW-
> rendered X server.
>
> Signed-off-by: Stephen Warren <swarren@wwwdotorg.org>

Acked-by: Simon Glass <sjg@chromium.org>

[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@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@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

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

[Stephen Warren]

On 05/08/2013 09:33 PM, Simon Glass wrote:
> Hi Stephen,
> 
> On Tue, May 7, 2013 at 10:21 PM, Stephen Warren <swarren@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).

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

I believe it really is common code; the DT content this code generates
is defined by a generic binding and isn't SoC-/board-specific. Perhaps a
common DT-related file would be OK too, although as you mention I'm not
sure if any other file would have access to the required LCD variables.

> Also is there a binding file we can bring in?

Yes, there's a simple-framebuffer.txt I could copy from the kernel.

>> +int lcd_dt_simplefb_add_node(void *blob)
> 
> Can we not automatically find the node and update it?

Some DTs might have the node already exist and want to edit it, whereas
others might not contain it at all and need it added. This is rather up
to the author of individual boards' DTs. I wanted to make the code
explicitly select between those two options so that the U-Boot board
author always thought about exactly which behaviour they wanted.

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

I don't think U-Boot supports multiple panels does it? If the DT
contained multiple nodes to start with, I think they'd have to all be
initially disabled since some firmware would be required to fill in the
fields before they were useful.

As such, finding and editing the first node in all cases seems to make
sense for now. If this ever becomes false, we can add an index parameter
quite easily without significant impact.

>> diff --git a/include/lcd.h b/include/lcd.h

>> +#if defined(CONFIG_LCD_DT_SIMPLEFB)
> 
> Probably don't need this #ifdef? It will complicate things if we use
> the autoconf series.

What's the autoconf series? I did this in order to get compile errors
rather than link errors if the functions were used without being
enabled, but I guess most linkers generate useful error messages so
perhaps this isn't needed.

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

[Simon Glass]

Hi Stephen,

On Fri, May 10, 2013 at 9:35 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 05/08/2013 09:33 PM, Simon Glass wrote:
>> Hi Stephen,
>>
>> On Tue, May 7, 2013 at 10:21 PM, Stephen Warren <swarren@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).
>
>> This looks OK - is a better place for it than the common lcd code? I
>> presume it is here because it accesses panel_info?
>
> I believe it really is common code; the DT content this code generates
> is defined by a generic binding and isn't SoC-/board-specific. Perhaps a
> common DT-related file would be OK too, although as you mention I'm not
> sure if any other file would have access to the required LCD variables.
>
>> Also is there a binding file we can bring in?
>
> Yes, there's a simple-framebuffer.txt I could copy from the kernel.

Yes, it looks good.

>
>>> +int lcd_dt_simplefb_add_node(void *blob)
>>
>> Can we not automatically find the node and update it?
>
> Some DTs might have the node already exist and want to edit it, whereas
> others might not contain it at all and need it added. This is rather up
> to the author of individual boards' DTs. I wanted to make the code
> explicitly select between those two options so that the U-Boot board
> author always thought about exactly which behaviour they wanted.

OK.

>
>>> +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.
>
> I don't think U-Boot supports multiple panels does it? If the DT
> contained multiple nodes to start with, I think they'd have to all be
> initially disabled since some firmware would be required to fill in the
> fields before they were useful.
>
> As such, finding and editing the first node in all cases seems to make
> sense for now. If this ever becomes false, we can add an index parameter
> quite easily without significant impact.

Sounds reasonable. I guess U-Boot will support multiple panels once
driver model is fully installed, but for now it does not.

>
>>> diff --git a/include/lcd.h b/include/lcd.h
>
>>> +#if defined(CONFIG_LCD_DT_SIMPLEFB)
>>
>> Probably don't need this #ifdef? It will complicate things if we use
>> the autoconf series.
>
> What's the autoconf series? I did this in order to get compile errors
> rather than link errors if the functions were used without being
> enabled, but I guess most linkers generate useful error messages so
> perhaps this isn't needed.
>

It was posted 26 Feb - first patch is here:
http://patchwork.ozlabs.org/patch/223278/

Also while you are in review mode, I'd appreciate any comment you have
on U-Boot driver model:

http://patchwork.ozlabs.org/patch/242451/

Regards,
Simon