[U-Boot] [PATCH v5 0/3] sunxi: video: Add simplefb support

Hi All,
Here is v5 of my sunxi simplefb support set.
Changes since v4: -Make sunxi_simplefb_setup return error codes -Add a patch to make lcd_dt_simplefb_configure_node use the new fdt_setup_simplefb_node helper (only compile tested!)
Changes since v3: -Added a fdt_setup_simplefb_node() helper function (new patch) -Changed how u-boot finds the pre-populated simplefb node to use a compatible string
Changes since v2: -Detect and handle address and size #cells
Changes since v1: -Use fdt_setprop_string for strings
Simon, even though -rc1 is out I would still like to get this upstream for v2015.01 if possible. This is in essence just refactoring of code posted well before -rc1. If I can get your ack on both the fdt_setup_simplefb_node and the lcd patches, then I can take them upstream through the sunxi tree, or if you plan to do a pull-req soon anyways, you can take them upstream through your tree, and I'll wait for that.
Thanks & Regards,
Hans

Add a generic helper to fill and enable simplefb nodes.
The first user of this will be the sunxi display code.
lcd_dt_simplefb_configure_node is also a good candidate to be converted to use this, but that requires someone to run some tests first, as lcd_dt_simplefb_configure_node does not honor #address-cells and #size-cells, but simply assumes 1 and 1 for both.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- common/fdt_support.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++ include/fdt_support.h | 3 +++ 2 files changed, 68 insertions(+)
diff --git a/common/fdt_support.c b/common/fdt_support.c index 3f64156..0ffa711 100644 --- a/common/fdt_support.c +++ b/common/fdt_support.c @@ -1523,3 +1523,68 @@ int fdt_read_range(void *fdt, int node, int n, uint64_t *child_addr,
return 0; } + +/** + * fdt_setup_simplefb_node - Fill and enable a simplefb node + * + * @fdt: ptr to device tree + * @node: offset of the simplefb node + * @base_address: framebuffer base address + * @width: width in pixels + * @height: height in pixels + * @stride: bytes per line + * @format: pixel format string + * + * Convenience function to fill and enable a simplefb node. + */ +int fdt_setup_simplefb_node(void *fdt, int node, u64 base_address, u32 width, + u32 height, u32 stride, const char *format) +{ + char name[32]; + fdt32_t cells[4]; + int i, addrc, sizec, ret; + + of_bus_default_count_cells(fdt, fdt_parent_offset(fdt, node), + &addrc, &sizec); + i = 0; + if (addrc == 2) + cells[i++] = cpu_to_fdt32(base_address >> 32); + cells[i++] = cpu_to_fdt32(base_address); + if (sizec == 2) + cells[i++] = 0; + cells[i++] = cpu_to_fdt32(height * stride); + + ret = fdt_setprop(fdt, node, "reg", cells, sizeof(cells[0]) * i); + if (ret < 0) + return ret; + + snprintf(name, sizeof(name), "framebuffer@%llx", base_address); + ret = fdt_set_name(fdt, node, name); + if (ret < 0) + return ret; + + cells[0] = cpu_to_fdt32(width); + ret = fdt_setprop(fdt, node, "width", cells, sizeof(cells[0])); + if (ret < 0) + return ret; + + cells[0] = cpu_to_fdt32(height); + ret = fdt_setprop(fdt, node, "height", cells, sizeof(cells[0])); + if (ret < 0) + return ret; + + cells[0] = cpu_to_fdt32(stride); + ret = fdt_setprop(fdt, node, "stride", cells, sizeof(cells[0])); + if (ret < 0) + return ret; + + ret = fdt_setprop_string(fdt, node, "format", format); + if (ret < 0) + return ret; + + ret = fdt_setprop_string(fdt, node, "status", "okay"); + if (ret < 0) + return ret; + + return 0; +} diff --git a/include/fdt_support.h b/include/fdt_support.h index 55cef94..d5e09e6 100644 --- a/include/fdt_support.h +++ b/include/fdt_support.h @@ -147,6 +147,9 @@ void of_bus_default_count_cells(void *blob, int parentoffset, int ft_verify_fdt(void *fdt); int arch_fixup_memory_node(void *blob);
+int fdt_setup_simplefb_node(void *fdt, int node, u64 base_address, u32 width, + u32 height, u32 stride, const char *format); + #endif /* ifdef CONFIG_OF_LIBFDT */
#ifdef USE_HOSTCC

On 11/19/2014 06:31 AM, Hans de Goede wrote:
Add a generic helper to fill and enable simplefb nodes.
The first user of this will be the sunxi display code.
lcd_dt_simplefb_configure_node is also a good candidate to be converted to use this, but that requires someone to run some tests first, as lcd_dt_simplefb_configure_node does not honor #address-cells and #size-cells, but simply assumes 1 and 1 for both.
Sorry, I'd been ignoring all simplefb stuff and hadn't realized this was U-Boot related and needed testing.
Anyway, I've tested patches 1 and 2 on an RPi compute module with kernel next-20141031 and it seems to work fine.
Patches 1, 2, Tested-by: Stephen Warren swarren@wwwdotorg.org

Hi Hans,
On 19 November 2014 13:31, Hans de Goede hdegoede@redhat.com wrote:
Add a generic helper to fill and enable simplefb nodes.
The first user of this will be the sunxi display code.
lcd_dt_simplefb_configure_node is also a good candidate to be converted to use this, but that requires someone to run some tests first, as lcd_dt_simplefb_configure_node does not honor #address-cells and #size-cells, but simply assumes 1 and 1 for both.
Signed-off-by: Hans de Goede hdegoede@redhat.com
Acked-by: Simon Glass sjg@chromium.org
See below if you respin.
common/fdt_support.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++ include/fdt_support.h | 3 +++ 2 files changed, 68 insertions(+)
diff --git a/common/fdt_support.c b/common/fdt_support.c index 3f64156..0ffa711 100644 --- a/common/fdt_support.c +++ b/common/fdt_support.c @@ -1523,3 +1523,68 @@ int fdt_read_range(void *fdt, int node, int n, uint64_t *child_addr,
return 0;
}
+/**
- fdt_setup_simplefb_node - Fill and enable a simplefb node
- @fdt: ptr to device tree
- @node: offset of the simplefb node
- @base_address: framebuffer base address
- @width: width in pixels
- @height: height in pixels
- @stride: bytes per line
- @format: pixel format string
- Convenience function to fill and enable a simplefb node.
- */
Better to put the comment in the header IMO.
+int fdt_setup_simplefb_node(void *fdt, int node, u64 base_address, u32 width,
u32 height, u32 stride, const char *format)
+{
char name[32];
fdt32_t cells[4];
int i, addrc, sizec, ret;
of_bus_default_count_cells(fdt, fdt_parent_offset(fdt, node),
&addrc, &sizec);
i = 0;
if (addrc == 2)
cells[i++] = cpu_to_fdt32(base_address >> 32);
cells[i++] = cpu_to_fdt32(base_address);
if (sizec == 2)
cells[i++] = 0;
cells[i++] = cpu_to_fdt32(height * stride);
ret = fdt_setprop(fdt, node, "reg", cells, sizeof(cells[0]) * i);
if (ret < 0)
return ret;
snprintf(name, sizeof(name), "framebuffer@%llx", base_address);
ret = fdt_set_name(fdt, node, name);
if (ret < 0)
return ret;
cells[0] = cpu_to_fdt32(width);
ret = fdt_setprop(fdt, node, "width", cells, sizeof(cells[0]));
How about fdt_setprop_u32() instead?
if (ret < 0)
return ret;
cells[0] = cpu_to_fdt32(height);
ret = fdt_setprop(fdt, node, "height", cells, sizeof(cells[0]));
if (ret < 0)
return ret;
cells[0] = cpu_to_fdt32(stride);
ret = fdt_setprop(fdt, node, "stride", cells, sizeof(cells[0]));
if (ret < 0)
return ret;
ret = fdt_setprop_string(fdt, node, "format", format);
if (ret < 0)
return ret;
ret = fdt_setprop_string(fdt, node, "status", "okay");
if (ret < 0)
return ret;
return 0;
+} diff --git a/include/fdt_support.h b/include/fdt_support.h index 55cef94..d5e09e6 100644 --- a/include/fdt_support.h +++ b/include/fdt_support.h @@ -147,6 +147,9 @@ void of_bus_default_count_cells(void *blob, int parentoffset, int ft_verify_fdt(void *fdt); int arch_fixup_memory_node(void *blob);
+int fdt_setup_simplefb_node(void *fdt, int node, u64 base_address, u32 width,
u32 height, u32 stride, const char *format);
#endif /* ifdef CONFIG_OF_LIBFDT */
#ifdef USE_HOSTCC
2.1.0
Regards, Simon

Hi,
On 11/20/2014 06:46 PM, Simon Glass wrote:
Hi Hans,
On 19 November 2014 13:31, Hans de Goede hdegoede@redhat.com wrote:
Add a generic helper to fill and enable simplefb nodes.
The first user of this will be the sunxi display code.
lcd_dt_simplefb_configure_node is also a good candidate to be converted to use this, but that requires someone to run some tests first, as lcd_dt_simplefb_configure_node does not honor #address-cells and #size-cells, but simply assumes 1 and 1 for both.
Signed-off-by: Hans de Goede hdegoede@redhat.com
Acked-by: Simon Glass sjg@chromium.org
See below if you respin.
We agreed on getting this in via the sunxi custodian tree right ? Then I can fixup your comments (and re-test things on a sunxi board) before sending a pull-req.
common/fdt_support.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++ include/fdt_support.h | 3 +++ 2 files changed, 68 insertions(+)
diff --git a/common/fdt_support.c b/common/fdt_support.c index 3f64156..0ffa711 100644 --- a/common/fdt_support.c +++ b/common/fdt_support.c @@ -1523,3 +1523,68 @@ int fdt_read_range(void *fdt, int node, int n, uint64_t *child_addr,
return 0;
}
+/**
- fdt_setup_simplefb_node - Fill and enable a simplefb node
- @fdt: ptr to device tree
- @node: offset of the simplefb node
- @base_address: framebuffer base address
- @width: width in pixels
- @height: height in pixels
- @stride: bytes per line
- @format: pixel format string
- Convenience function to fill and enable a simplefb node.
- */
Better to put the comment in the header IMO.
I followed the example of other functions in fdt_support, there currently are no doxygen comments in fdt_support.h . Still I can move this over if you want.
+int fdt_setup_simplefb_node(void *fdt, int node, u64 base_address, u32 width,
u32 height, u32 stride, const char *format)
+{
char name[32];
fdt32_t cells[4];
int i, addrc, sizec, ret;
of_bus_default_count_cells(fdt, fdt_parent_offset(fdt, node),
&addrc, &sizec);
i = 0;
if (addrc == 2)
cells[i++] = cpu_to_fdt32(base_address >> 32);
cells[i++] = cpu_to_fdt32(base_address);
if (sizec == 2)
cells[i++] = 0;
cells[i++] = cpu_to_fdt32(height * stride);
ret = fdt_setprop(fdt, node, "reg", cells, sizeof(cells[0]) * i);
if (ret < 0)
return ret;
snprintf(name, sizeof(name), "framebuffer@%llx", base_address);
ret = fdt_set_name(fdt, node, name);
if (ret < 0)
return ret;
cells[0] = cpu_to_fdt32(width);
ret = fdt_setprop(fdt, node, "width", cells, sizeof(cells[0]));
How about fdt_setprop_u32() instead?
Yeah thats much better, will fix.
if (ret < 0)
return ret;
cells[0] = cpu_to_fdt32(height);
ret = fdt_setprop(fdt, node, "height", cells, sizeof(cells[0]));
if (ret < 0)
return ret;
cells[0] = cpu_to_fdt32(stride);
ret = fdt_setprop(fdt, node, "stride", cells, sizeof(cells[0]));
if (ret < 0)
return ret;
ret = fdt_setprop_string(fdt, node, "format", format);
if (ret < 0)
return ret;
ret = fdt_setprop_string(fdt, node, "status", "okay");
if (ret < 0)
return ret;
return 0;
+} diff --git a/include/fdt_support.h b/include/fdt_support.h index 55cef94..d5e09e6 100644 --- a/include/fdt_support.h +++ b/include/fdt_support.h @@ -147,6 +147,9 @@ void of_bus_default_count_cells(void *blob, int parentoffset, int ft_verify_fdt(void *fdt); int arch_fixup_memory_node(void *blob);
+int fdt_setup_simplefb_node(void *fdt, int node, u64 base_address, u32 width,
u32 height, u32 stride, const char *format);
#endif /* ifdef CONFIG_OF_LIBFDT */
#ifdef USE_HOSTCC
2.1.0
Regards, Simon
Regards,
Hans

Change lcd_dt_simplefb_configure_node into a wrapper around the new generic fdt_setup_simplefb_node helper function.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- common/lcd.c | 47 +++++------------------------------------------ 1 file changed, 5 insertions(+), 42 deletions(-)
diff --git a/common/lcd.c b/common/lcd.c index 37147af..54dd294 100644 --- a/common/lcd.c +++ b/common/lcd.c @@ -30,6 +30,7 @@ #include <splash.h> #include <asm/io.h> #include <asm/unaligned.h> +#include <fdt_support.h>
#if defined(CONFIG_CPU_PXA25X) || defined(CONFIG_CPU_PXA27X) || \ defined(CONFIG_CPU_MONAHANS) @@ -1172,51 +1173,13 @@ int lcd_get_screen_columns(void) #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; - static const char format[] = #if LCD_BPP == LCD_COLOR16 - "r5g6b5"; + return fdt_setup_simplefb_node(blob, off, gd->fb_base, + panel_info.vl_col, panel_info.vl_row, + panel_info.vl_col * 2, "r5g6b5"); #else - ""; + return -1; #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)

On 19 November 2014 13:31, Hans de Goede hdegoede@redhat.com wrote:
Change lcd_dt_simplefb_configure_node into a wrapper around the new generic fdt_setup_simplefb_node helper function.
Signed-off-by: Hans de Goede hdegoede@redhat.com
Acked-by: Simon Glass sjg@chromium.org

From: Luc Verhaegen libv@skynet.be
Add simplefb support, note this depends on the kernel having support for the clocks property which has recently been added to the simplefb devicetree binding.
Signed-off-by: Luc Verhaegen libv@skynet.be [hdegoede@redhat.com: Use pre-populated simplefb node under /chosen as disussed on the devicetree list] Signed-off-by: Hans de Goede hdegoede@redhat.com --- arch/arm/include/asm/arch-sunxi/display.h | 4 +++ board/sunxi/board.c | 11 ++++++++ drivers/video/sunxi_display.c | 42 +++++++++++++++++++++++++++++++ include/configs/sunxi-common.h | 8 ++++++ 4 files changed, 65 insertions(+)
diff --git a/arch/arm/include/asm/arch-sunxi/display.h b/arch/arm/include/asm/arch-sunxi/display.h index 8d80ceb..c9d81ba 100644 --- a/arch/arm/include/asm/arch-sunxi/display.h +++ b/arch/arm/include/asm/arch-sunxi/display.h @@ -195,4 +195,8 @@ struct sunxi_hdmi_reg { #define SUNXI_HDMI_PLL_DBG0_PLL3 (0 << 21) #define SUNXI_HDMI_PLL_DBG0_PLL7 (1 << 21)
+#ifdef CONFIG_VIDEO_DT_SIMPLEFB +int sunxi_simplefb_setup(void *blob); +#endif + #endif /* _SUNXI_DISPLAY_H */ diff --git a/board/sunxi/board.c b/board/sunxi/board.c index e6ec5b8..d4530e8 100644 --- a/board/sunxi/board.c +++ b/board/sunxi/board.c @@ -24,6 +24,7 @@ #endif #include <asm/arch/clock.h> #include <asm/arch/cpu.h> +#include <asm/arch/display.h> #include <asm/arch/dram.h> #include <asm/arch/gpio.h> #include <asm/arch/mmc.h> @@ -237,3 +238,13 @@ int misc_init_r(void) return 0; } #endif + +#ifdef CONFIG_OF_BOARD_SETUP +void +ft_board_setup(void *blob, bd_t *bd) +{ +#ifdef CONFIG_VIDEO_DT_SIMPLEFB + sunxi_simplefb_setup(blob); +#endif +} +#endif /* CONFIG_OF_BOARD_SETUP */ diff --git a/drivers/video/sunxi_display.c b/drivers/video/sunxi_display.c index 3f46c31..2f3f5db 100644 --- a/drivers/video/sunxi_display.c +++ b/drivers/video/sunxi_display.c @@ -13,6 +13,8 @@ #include <asm/arch/display.h> #include <asm/global_data.h> #include <asm/io.h> +#include <fdtdec.h> +#include <fdt_support.h> #include <linux/fb.h> #include <video_fb.h>
@@ -416,3 +418,43 @@ video_hw_init(void)
return graphic_device; } + +/* + * Simplefb support. + */ +#if defined(CONFIG_OF_BOARD_SETUP) && defined(CONFIG_VIDEO_DT_SIMPLEFB) +int +sunxi_simplefb_setup(void *blob) +{ + static GraphicDevice *graphic_device = &sunxi_display.graphic_device; + int offset, ret; + + if (!sunxi_display.enabled) + return 0; + + /* Find a framebuffer node, with pipeline == "de_be0-lcd0-hdmi" */ + offset = fdt_node_offset_by_compatible(blob, -1, + "allwinner,simple-framebuffer"); + while (offset >= 0) { + ret = fdt_find_string(blob, offset, "allwinner,pipeline", + "de_be0-lcd0-hdmi"); + if (ret == 0) + break; + offset = fdt_node_offset_by_compatible(blob, offset, + "allwinner,simple-framebuffer"); + } + if (offset < 0) { + eprintf("Cannot setup simplefb: node not found\n"); + return 0; /* Keep older kernels working */ + } + + ret = fdt_setup_simplefb_node(blob, offset, gd->fb_base, + graphic_device->winSizeX, graphic_device->winSizeY, + graphic_device->winSizeX * graphic_device->gdfBytesPP, + "x8r8g8b8"); + if (ret) + eprintf("Cannot setup simplefb: Error setting properties\n"); + + return ret; +} +#endif /* CONFIG_OF_BOARD_SETUP && CONFIG_VIDEO_DT_SIMPLEFB */ diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h index 900ef52..d5d907b 100644 --- a/include/configs/sunxi-common.h +++ b/include/configs/sunxi-common.h @@ -204,6 +204,9 @@ */ #define CONFIG_SUNXI_FB_SIZE (8 << 20)
+/* Do we want to initialize a simple FB? */ +#define CONFIG_VIDEO_DT_SIMPLEFB + #define CONFIG_VIDEO_SUNXI
#define CONFIG_CFB_CONSOLE @@ -217,6 +220,11 @@
#define CONFIG_SYS_MEM_TOP_HIDE ((CONFIG_SUNXI_FB_SIZE + 0xFFF) & ~0xFFF)
+/* To be able to hook simplefb into dt */ +#ifdef CONFIG_VIDEO_DT_SIMPLEFB +#define CONFIG_OF_BOARD_SETUP +#endif + #endif /* CONFIG_VIDEO */
/* Ethernet support */

On Wed, 2014-11-19 at 14:32 +0100, Hans de Goede wrote:
From: Luc Verhaegen libv@skynet.be
Add simplefb support, note this depends on the kernel having support for the clocks property which has recently been added to the simplefb devicetree binding.
Signed-off-by: Luc Verhaegen libv@skynet.be [hdegoede@redhat.com: Use pre-populated simplefb node under /chosen as disussed on the devicetree list] Signed-off-by: Hans de Goede hdegoede@redhat.com
Acked-by: Ian Campbell ijc@hellion.org.uk.
One non-blocking queries:
- /* Find a framebuffer node, with pipeline == "de_be0-lcd0-hdmi" */
- offset = fdt_node_offset_by_compatible(blob, -1,
"allwinner,simple-framebuffer");
- while (offset >= 0) {
ret = fdt_find_string(blob, offset, "allwinner,pipeline",
"de_be0-lcd0-hdmi");
if (ret == 0)
break;
offset = fdt_node_offset_by_compatible(blob, offset,
"allwinner,simple-framebuffer");
- }
Is this variant non-conformant with coding style?:
int offset = -1; while ( (offset = fdt_node_offset_by_compatible(blob, offset, "allwinner,simple-framebuffer") ) { LOOP BODY }
I expect it is because of the assignment within the while condition, which is a shame, since this is one case where it is IMHO leads to clearer code.

Hi,
On 11/20/2014 10:22 AM, Ian Campbell wrote:
On Wed, 2014-11-19 at 14:32 +0100, Hans de Goede wrote:
From: Luc Verhaegen libv@skynet.be
Add simplefb support, note this depends on the kernel having support for the clocks property which has recently been added to the simplefb devicetree binding.
Signed-off-by: Luc Verhaegen libv@skynet.be [hdegoede@redhat.com: Use pre-populated simplefb node under /chosen as disussed on the devicetree list] Signed-off-by: Hans de Goede hdegoede@redhat.com
Acked-by: Ian Campbell ijc@hellion.org.uk.
Thanks, any chance you could also review v2 of the CONFIG_USB_KEYBOARD patch ?
One non-blocking queries:
- /* Find a framebuffer node, with pipeline == "de_be0-lcd0-hdmi" */
- offset = fdt_node_offset_by_compatible(blob, -1,
"allwinner,simple-framebuffer");
- while (offset >= 0) {
ret = fdt_find_string(blob, offset, "allwinner,pipeline",
"de_be0-lcd0-hdmi");
if (ret == 0)
break;
offset = fdt_node_offset_by_compatible(blob, offset,
"allwinner,simple-framebuffer");
- }
Is this variant non-conformant with coding style?:
int offset = -1; while ( (offset = fdt_node_offset_by_compatible(blob, offset, "allwinner,simple-framebuffer") ) { LOOP BODY }
I expect it is because of the assignment within the while condition, which is a shame, since this is one case where it is IMHO leads to clearer code.
AFAIK this indeed would go against the coding style, and TBH I'm also not sure if I agree it would be cleaner (mainly because indentation would become (even more) messy due to the 80 columns limit).
Regards,
Hans

Hi Hans,
On 19 November 2014 13:32, Hans de Goede hdegoede@redhat.com wrote:
From: Luc Verhaegen libv@skynet.be
Add simplefb support, note this depends on the kernel having support for the clocks property which has recently been added to the simplefb devicetree binding.
Signed-off-by: Luc Verhaegen libv@skynet.be [hdegoede@redhat.com: Use pre-populated simplefb node under /chosen as disussed on the devicetree list] Signed-off-by: Hans de Goede hdegoede@redhat.com
arch/arm/include/asm/arch-sunxi/display.h | 4 +++ board/sunxi/board.c | 11 ++++++++ drivers/video/sunxi_display.c | 42 +++++++++++++++++++++++++++++++ include/configs/sunxi-common.h | 8 ++++++ 4 files changed, 65 insertions(+)
diff --git a/arch/arm/include/asm/arch-sunxi/display.h b/arch/arm/include/asm/arch-sunxi/display.h index 8d80ceb..c9d81ba 100644 --- a/arch/arm/include/asm/arch-sunxi/display.h +++ b/arch/arm/include/asm/arch-sunxi/display.h @@ -195,4 +195,8 @@ struct sunxi_hdmi_reg { #define SUNXI_HDMI_PLL_DBG0_PLL3 (0 << 21) #define SUNXI_HDMI_PLL_DBG0_PLL7 (1 << 21)
+#ifdef CONFIG_VIDEO_DT_SIMPLEFB +int sunxi_simplefb_setup(void *blob); +#endif
Do you really need this #ifdef?
#endif /* _SUNXI_DISPLAY_H */ diff --git a/board/sunxi/board.c b/board/sunxi/board.c index e6ec5b8..d4530e8 100644 --- a/board/sunxi/board.c +++ b/board/sunxi/board.c @@ -24,6 +24,7 @@ #endif #include <asm/arch/clock.h> #include <asm/arch/cpu.h> +#include <asm/arch/display.h> #include <asm/arch/dram.h> #include <asm/arch/gpio.h> #include <asm/arch/mmc.h> @@ -237,3 +238,13 @@ int misc_init_r(void) return 0; } #endif
+#ifdef CONFIG_OF_BOARD_SETUP +void +ft_board_setup(void *blob, bd_t *bd)
void on same line? Also can you make it return int. You could look at u-boot-fdt/next (I am waiting on resolving one patch before sending a pull).
+{ +#ifdef CONFIG_VIDEO_DT_SIMPLEFB
sunxi_simplefb_setup(blob);
+#endif +} +#endif /* CONFIG_OF_BOARD_SETUP */ diff --git a/drivers/video/sunxi_display.c b/drivers/video/sunxi_display.c index 3f46c31..2f3f5db 100644 --- a/drivers/video/sunxi_display.c +++ b/drivers/video/sunxi_display.c @@ -13,6 +13,8 @@ #include <asm/arch/display.h> #include <asm/global_data.h> #include <asm/io.h> +#include <fdtdec.h> +#include <fdt_support.h> #include <linux/fb.h> #include <video_fb.h>
@@ -416,3 +418,43 @@ video_hw_init(void)
return graphic_device;
}
+/*
- Simplefb support.
- */
+#if defined(CONFIG_OF_BOARD_SETUP) && defined(CONFIG_VIDEO_DT_SIMPLEFB) +int +sunxi_simplefb_setup(void *blob)
int on same line
+{
static GraphicDevice *graphic_device = &sunxi_display.graphic_device;
int offset, ret;
if (!sunxi_display.enabled)
return 0;
/* Find a framebuffer node, with pipeline == "de_be0-lcd0-hdmi" */
offset = fdt_node_offset_by_compatible(blob, -1,
"allwinner,simple-framebuffer");
while (offset >= 0) {
ret = fdt_find_string(blob, offset, "allwinner,pipeline",
"de_be0-lcd0-hdmi");
if (ret == 0)
break;
offset = fdt_node_offset_by_compatible(blob, offset,
"allwinner,simple-framebuffer");
}
Can you use do...while() to avoid repeating the 'offset =' line?
if (offset < 0) {
eprintf("Cannot setup simplefb: node not found\n");
return 0; /* Keep older kernels working */
}
ret = fdt_setup_simplefb_node(blob, offset, gd->fb_base,
graphic_device->winSizeX, graphic_device->winSizeY,
graphic_device->winSizeX * graphic_device->gdfBytesPP,
"x8r8g8b8");
if (ret)
eprintf("Cannot setup simplefb: Error setting properties\n");
return ret;
+} +#endif /* CONFIG_OF_BOARD_SETUP && CONFIG_VIDEO_DT_SIMPLEFB */ diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h index 900ef52..d5d907b 100644 --- a/include/configs/sunxi-common.h +++ b/include/configs/sunxi-common.h @@ -204,6 +204,9 @@ */ #define CONFIG_SUNXI_FB_SIZE (8 << 20)
+/* Do we want to initialize a simple FB? */ +#define CONFIG_VIDEO_DT_SIMPLEFB
#define CONFIG_VIDEO_SUNXI
#define CONFIG_CFB_CONSOLE @@ -217,6 +220,11 @@
#define CONFIG_SYS_MEM_TOP_HIDE ((CONFIG_SUNXI_FB_SIZE + 0xFFF) & ~0xFFF)
+/* To be able to hook simplefb into dt */ +#ifdef CONFIG_VIDEO_DT_SIMPLEFB +#define CONFIG_OF_BOARD_SETUP +#endif
#endif /* CONFIG_VIDEO */
/* Ethernet support */
2.1.0
Regards, Simon

Hi,
On 11/20/2014 06:40 PM, Simon Glass wrote:
Hi Hans,
On 19 November 2014 13:32, Hans de Goede hdegoede@redhat.com wrote:
From: Luc Verhaegen libv@skynet.be
Add simplefb support, note this depends on the kernel having support for the clocks property which has recently been added to the simplefb devicetree binding.
Signed-off-by: Luc Verhaegen libv@skynet.be [hdegoede@redhat.com: Use pre-populated simplefb node under /chosen as disussed on the devicetree list] Signed-off-by: Hans de Goede hdegoede@redhat.com
arch/arm/include/asm/arch-sunxi/display.h | 4 +++ board/sunxi/board.c | 11 ++++++++ drivers/video/sunxi_display.c | 42 +++++++++++++++++++++++++++++++ include/configs/sunxi-common.h | 8 ++++++ 4 files changed, 65 insertions(+)
diff --git a/arch/arm/include/asm/arch-sunxi/display.h b/arch/arm/include/asm/arch-sunxi/display.h index 8d80ceb..c9d81ba 100644 --- a/arch/arm/include/asm/arch-sunxi/display.h +++ b/arch/arm/include/asm/arch-sunxi/display.h @@ -195,4 +195,8 @@ struct sunxi_hdmi_reg { #define SUNXI_HDMI_PLL_DBG0_PLL3 (0 << 21) #define SUNXI_HDMI_PLL_DBG0_PLL7 (1 << 21)
+#ifdef CONFIG_VIDEO_DT_SIMPLEFB +int sunxi_simplefb_setup(void *blob); +#endif
Do you really need this #ifdef?
No, I'm not the original author of this patch and I inherited this from the original author, I'm not a big fan either, I'll drop it.
#endif /* _SUNXI_DISPLAY_H */ diff --git a/board/sunxi/board.c b/board/sunxi/board.c index e6ec5b8..d4530e8 100644 --- a/board/sunxi/board.c +++ b/board/sunxi/board.c @@ -24,6 +24,7 @@ #endif #include <asm/arch/clock.h> #include <asm/arch/cpu.h> +#include <asm/arch/display.h> #include <asm/arch/dram.h> #include <asm/arch/gpio.h> #include <asm/arch/mmc.h> @@ -237,3 +238,13 @@ int misc_init_r(void) return 0; } #endif
+#ifdef CONFIG_OF_BOARD_SETUP +void +ft_board_setup(void *blob, bd_t *bd)
void on same line?
Another inherited thing, will fixup (everywhere).
Also can you make it return int. You could look at u-boot-fdt/next (I am waiting on resolving one patch before sending a pull).
Ack, will also fix.
+{ +#ifdef CONFIG_VIDEO_DT_SIMPLEFB
sunxi_simplefb_setup(blob);
+#endif +} +#endif /* CONFIG_OF_BOARD_SETUP */ diff --git a/drivers/video/sunxi_display.c b/drivers/video/sunxi_display.c index 3f46c31..2f3f5db 100644 --- a/drivers/video/sunxi_display.c +++ b/drivers/video/sunxi_display.c @@ -13,6 +13,8 @@ #include <asm/arch/display.h> #include <asm/global_data.h> #include <asm/io.h> +#include <fdtdec.h> +#include <fdt_support.h> #include <linux/fb.h> #include <video_fb.h>
@@ -416,3 +418,43 @@ video_hw_init(void)
return graphic_device;
}
+/*
- Simplefb support.
- */
+#if defined(CONFIG_OF_BOARD_SETUP) && defined(CONFIG_VIDEO_DT_SIMPLEFB) +int +sunxi_simplefb_setup(void *blob)
int on same line
+{
static GraphicDevice *graphic_device = &sunxi_display.graphic_device;
int offset, ret;
if (!sunxi_display.enabled)
return 0;
/* Find a framebuffer node, with pipeline == "de_be0-lcd0-hdmi" */
offset = fdt_node_offset_by_compatible(blob, -1,
"allwinner,simple-framebuffer");
while (offset >= 0) {
ret = fdt_find_string(blob, offset, "allwinner,pipeline",
"de_be0-lcd0-hdmi");
if (ret == 0)
break;
offset = fdt_node_offset_by_compatible(blob, offset,
"allwinner,simple-framebuffer");
}
Can you use do...while() to avoid repeating the 'offset =' line?
if (offset < 0) {
eprintf("Cannot setup simplefb: node not found\n");
return 0; /* Keep older kernels working */
}
ret = fdt_setup_simplefb_node(blob, offset, gd->fb_base,
graphic_device->winSizeX, graphic_device->winSizeY,
graphic_device->winSizeX * graphic_device->gdfBytesPP,
"x8r8g8b8");
if (ret)
eprintf("Cannot setup simplefb: Error setting properties\n");
return ret;
+} +#endif /* CONFIG_OF_BOARD_SETUP && CONFIG_VIDEO_DT_SIMPLEFB */ diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h index 900ef52..d5d907b 100644 --- a/include/configs/sunxi-common.h +++ b/include/configs/sunxi-common.h @@ -204,6 +204,9 @@ */ #define CONFIG_SUNXI_FB_SIZE (8 << 20)
+/* Do we want to initialize a simple FB? */ +#define CONFIG_VIDEO_DT_SIMPLEFB
#define CONFIG_VIDEO_SUNXI
#define CONFIG_CFB_CONSOLE @@ -217,6 +220,11 @@
#define CONFIG_SYS_MEM_TOP_HIDE ((CONFIG_SUNXI_FB_SIZE + 0xFFF) & ~0xFFF)
+/* To be able to hook simplefb into dt */ +#ifdef CONFIG_VIDEO_DT_SIMPLEFB +#define CONFIG_OF_BOARD_SETUP +#endif
#endif /* CONFIG_VIDEO */
/* Ethernet support */
2.1.0
Regards, Simon
Regards,
Hans

Hi Hans,
On 19 November 2014 13:31, Hans de Goede hdegoede@redhat.com wrote:
Hi All,
Here is v5 of my sunxi simplefb support set.
Changes since v4: -Make sunxi_simplefb_setup return error codes -Add a patch to make lcd_dt_simplefb_configure_node use the new fdt_setup_simplefb_node helper (only compile tested!)
Changes since v3: -Added a fdt_setup_simplefb_node() helper function (new patch) -Changed how u-boot finds the pre-populated simplefb node to use a compatible string
Changes since v2: -Detect and handle address and size #cells
Changes since v1: -Use fdt_setprop_string for strings
Simon, even though -rc1 is out I would still like to get this upstream for v2015.01 if possible. This is in essence just refactoring of code posted well before -rc1. If I can get your ack on both the fdt_setup_simplefb_node and the lcd patches, then I can take them upstream through the sunxi tree, or if you plan to do a pull-req soon anyways, you can take them upstream through your tree, and I'll wait for that.
That seems good to me. I'll see if I can test it when I get back. I added Stephen also as he wrote the Raspberry Pi stuff.
Regards, Simon
participants (4)
-
Hans de Goede
-
Ian Campbell
-
Simon Glass
-
Stephen Warren