* [U-Boot] [PATCH v5 1/3] fdt_support: Add a fdt_setup_simplefb_node helper function
2014-11-19 13:31 [U-Boot] [PATCH v5 0/3] sunxi: video: Add simplefb support Hans de Goede
@ 2014-11-19 13:31 ` Hans de Goede
2014-11-20 3:59 ` Stephen Warren
2014-11-20 17:46 ` Simon Glass
2014-11-19 13:31 ` [U-Boot] [PATCH v5 2/3] common/lcd: Make lcd_dt_simplefb_configure_node use fdt_setup_simplefb_node Hans de Goede
` (2 subsequent siblings)
3 siblings, 2 replies; 13+ messages in thread
From: Hans de Goede @ 2014-11-19 13:31 UTC (permalink / raw)
To: u-boot
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
--
2.1.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* [U-Boot] [PATCH v5 1/3] fdt_support: Add a fdt_setup_simplefb_node helper function
2014-11-19 13:31 ` [U-Boot] [PATCH v5 1/3] fdt_support: Add a fdt_setup_simplefb_node helper function Hans de Goede
@ 2014-11-20 3:59 ` Stephen Warren
2014-11-20 17:46 ` Simon Glass
1 sibling, 0 replies; 13+ messages in thread
From: Stephen Warren @ 2014-11-20 3:59 UTC (permalink / raw)
To: u-boot
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>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH v5 1/3] fdt_support: Add a fdt_setup_simplefb_node helper function
2014-11-19 13:31 ` [U-Boot] [PATCH v5 1/3] fdt_support: Add a fdt_setup_simplefb_node helper function Hans de Goede
2014-11-20 3:59 ` Stephen Warren
@ 2014-11-20 17:46 ` Simon Glass
2014-11-20 19:40 ` Hans de Goede
1 sibling, 1 reply; 13+ messages in thread
From: Simon Glass @ 2014-11-20 17:46 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 13+ messages in thread* [U-Boot] [PATCH v5 1/3] fdt_support: Add a fdt_setup_simplefb_node helper function
2014-11-20 17:46 ` Simon Glass
@ 2014-11-20 19:40 ` Hans de Goede
0 siblings, 0 replies; 13+ messages in thread
From: Hans de Goede @ 2014-11-20 19:40 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH v5 2/3] common/lcd: Make lcd_dt_simplefb_configure_node use fdt_setup_simplefb_node
2014-11-19 13:31 [U-Boot] [PATCH v5 0/3] sunxi: video: Add simplefb support Hans de Goede
2014-11-19 13:31 ` [U-Boot] [PATCH v5 1/3] fdt_support: Add a fdt_setup_simplefb_node helper function Hans de Goede
@ 2014-11-19 13:31 ` Hans de Goede
2014-11-20 17:48 ` Simon Glass
2014-11-19 13:32 ` [U-Boot] [PATCH v5 3/3] sunxi: video: Add simplefb support Hans de Goede
2014-11-19 22:18 ` [U-Boot] [PATCH v5 0/3] " Simon Glass
3 siblings, 1 reply; 13+ messages in thread
From: Hans de Goede @ 2014-11-19 13:31 UTC (permalink / raw)
To: u-boot
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)
--
2.1.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* [U-Boot] [PATCH v5 3/3] sunxi: video: Add simplefb support
2014-11-19 13:31 [U-Boot] [PATCH v5 0/3] sunxi: video: Add simplefb support Hans de Goede
2014-11-19 13:31 ` [U-Boot] [PATCH v5 1/3] fdt_support: Add a fdt_setup_simplefb_node helper function Hans de Goede
2014-11-19 13:31 ` [U-Boot] [PATCH v5 2/3] common/lcd: Make lcd_dt_simplefb_configure_node use fdt_setup_simplefb_node Hans de Goede
@ 2014-11-19 13:32 ` Hans de Goede
2014-11-20 9:22 ` Ian Campbell
2014-11-20 17:40 ` Simon Glass
2014-11-19 22:18 ` [U-Boot] [PATCH v5 0/3] " Simon Glass
3 siblings, 2 replies; 13+ messages in thread
From: Hans de Goede @ 2014-11-19 13:32 UTC (permalink / raw)
To: u-boot
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 at 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 */
--
2.1.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* [U-Boot] [PATCH v5 3/3] sunxi: video: Add simplefb support
2014-11-19 13:32 ` [U-Boot] [PATCH v5 3/3] sunxi: video: Add simplefb support Hans de Goede
@ 2014-11-20 9:22 ` Ian Campbell
2014-11-20 14:13 ` Hans de Goede
2014-11-20 17:40 ` Simon Glass
1 sibling, 1 reply; 13+ messages in thread
From: Ian Campbell @ 2014-11-20 9:22 UTC (permalink / raw)
To: u-boot
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 at 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.
^ permalink raw reply [flat|nested] 13+ messages in thread* [U-Boot] [PATCH v5 3/3] sunxi: video: Add simplefb support
2014-11-20 9:22 ` Ian Campbell
@ 2014-11-20 14:13 ` Hans de Goede
0 siblings, 0 replies; 13+ messages in thread
From: Hans de Goede @ 2014-11-20 14:13 UTC (permalink / raw)
To: u-boot
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 at 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
^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH v5 3/3] sunxi: video: Add simplefb support
2014-11-19 13:32 ` [U-Boot] [PATCH v5 3/3] sunxi: video: Add simplefb support Hans de Goede
2014-11-20 9:22 ` Ian Campbell
@ 2014-11-20 17:40 ` Simon Glass
2014-11-20 20:07 ` Hans de Goede
1 sibling, 1 reply; 13+ messages in thread
From: Simon Glass @ 2014-11-20 17:40 UTC (permalink / raw)
To: u-boot
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 at 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
^ permalink raw reply [flat|nested] 13+ messages in thread* [U-Boot] [PATCH v5 3/3] sunxi: video: Add simplefb support
2014-11-20 17:40 ` Simon Glass
@ 2014-11-20 20:07 ` Hans de Goede
0 siblings, 0 replies; 13+ messages in thread
From: Hans de Goede @ 2014-11-20 20:07 UTC (permalink / raw)
To: u-boot
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 at 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
^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH v5 0/3] sunxi: video: Add simplefb support
2014-11-19 13:31 [U-Boot] [PATCH v5 0/3] sunxi: video: Add simplefb support Hans de Goede
` (2 preceding siblings ...)
2014-11-19 13:32 ` [U-Boot] [PATCH v5 3/3] sunxi: video: Add simplefb support Hans de Goede
@ 2014-11-19 22:18 ` Simon Glass
3 siblings, 0 replies; 13+ messages in thread
From: Simon Glass @ 2014-11-19 22:18 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 13+ messages in thread