| Message ID | 1416166742-23684-2-git-send-email-hdegoede@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
On Sun, Nov 16, 2014 at 7:39 PM, 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> I'm not a U-Boot maintainer, so take my comments with a grain of salt... This patch seems quite short sighted. The code that calculates and updates the simple framebuffer node addresses is pretty much going to be identical for all platforms. Why is sunxi open coding it? Particularly when there could be fiddly bits around dealing with #address-cells/#size-cells. I would think the code to update a simplefb node would be a common library function. g. > --- > arch/arm/include/asm/arch-sunxi/display.h | 4 ++ > board/sunxi/board.c | 11 +++++ > drivers/video/sunxi_display.c | 73 +++++++++++++++++++++++++++++++ > include/configs/sunxi-common.h | 8 ++++ > 4 files changed, 96 insertions(+) > > diff --git a/arch/arm/include/asm/arch-sunxi/display.h b/arch/arm/include/asm/arch-sunxi/display.h > index 8d80ceb..4c694f8 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 > +void 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..0bd273e 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,74 @@ video_hw_init(void) > > return graphic_device; > } > + > +/* > + * Simplefb support. > + */ > +#if defined(CONFIG_OF_BOARD_SETUP) && defined(CONFIG_VIDEO_DT_SIMPLEFB) > +void > +sunxi_simplefb_setup(void *blob) > +{ > + static GraphicDevice *graphic_device = &sunxi_display.graphic_device; > + const char *node = "/chosen/framebuffer0-hdmi"; > + char name[32]; > + fdt32_t cells[4]; > + int i, offset, addrc, sizec, ret, stride; > + > + if (!sunxi_display.enabled) > + return; > + > + offset = fdt_path_offset(blob, node); > + if (offset < 0) { > + eprintf("Cannot setup simplefb: %s node not found\n", node); > + return; > + } > + > + snprintf(name, sizeof(name), "framebuffer@%08lx", gd->fb_base); > + ret = fdt_set_name(blob, offset, name); > + if (ret < 0) > + goto error; > + > + of_bus_default_count_cells(blob, fdt_parent_offset(blob, offset), > + &addrc, &sizec); > + i = 0; > + if (addrc == 2) > + cells[i++] = 0; > + cells[i++] = cpu_to_fdt32(gd->fb_base); > + if (sizec == 2) > + cells[i++] = 0; > + cells[i++] = cpu_to_fdt32(CONFIG_SUNXI_FB_SIZE); > + > + ret = fdt_setprop(blob, offset, "reg", cells, sizeof(cells[0]) * i); > + if (ret < 0) > + goto error; > + > + cells[0] = cpu_to_fdt32(graphic_device->winSizeX); > + ret = fdt_setprop(blob, offset, "width", cells, sizeof(cells[0])); > + if (ret < 0) > + goto error; > + > + cells[0] = cpu_to_fdt32(graphic_device->winSizeY); > + ret = fdt_setprop(blob, offset, "height", cells, sizeof(cells[0])); > + if (ret < 0) > + goto error; > + > + stride = graphic_device->winSizeX * graphic_device->gdfBytesPP; > + cells[0] = cpu_to_fdt32(stride); > + ret = fdt_setprop(blob, offset, "stride", cells, sizeof(cells[0])); > + if (ret < 0) > + goto error; > + > + ret = fdt_setprop_string(blob, offset, "format", "x8r8g8b8"); > + if (ret < 0) > + goto error; > + > + ret = fdt_setprop_string(blob, offset, "status", "okay"); > + if (ret < 0) > + goto error; > + > + return; > +error: > + eprintf("Cannot setup simplefb: Error setting properties\n"); > +} > +#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 > > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot
Hi, On 11/17/2014 10:35 AM, Grant Likely wrote: > On Sun, Nov 16, 2014 at 7:39 PM, 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> > > I'm not a U-Boot maintainer, so take my comments with a grain of salt... > > This patch seems quite short sighted. The code that calculates and > updates the simple framebuffer node addresses is pretty much going to > be identical for all platforms. Why is sunxi open coding it? > Particularly when there could be fiddly bits around dealing with > #address-cells/#size-cells. > > I would think the code to update a simplefb node would be a common > library function. You're right I've spun it out into a generic helper function for the next version of this patch-set. Regards, Hans
diff --git a/arch/arm/include/asm/arch-sunxi/display.h b/arch/arm/include/asm/arch-sunxi/display.h index 8d80ceb..4c694f8 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 +void 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..0bd273e 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,74 @@ video_hw_init(void) return graphic_device; } + +/* + * Simplefb support. + */ +#if defined(CONFIG_OF_BOARD_SETUP) && defined(CONFIG_VIDEO_DT_SIMPLEFB) +void +sunxi_simplefb_setup(void *blob) +{ + static GraphicDevice *graphic_device = &sunxi_display.graphic_device; + const char *node = "/chosen/framebuffer0-hdmi"; + char name[32]; + fdt32_t cells[4]; + int i, offset, addrc, sizec, ret, stride; + + if (!sunxi_display.enabled) + return; + + offset = fdt_path_offset(blob, node); + if (offset < 0) { + eprintf("Cannot setup simplefb: %s node not found\n", node); + return; + } + + snprintf(name, sizeof(name), "framebuffer@%08lx", gd->fb_base); + ret = fdt_set_name(blob, offset, name); + if (ret < 0) + goto error; + + of_bus_default_count_cells(blob, fdt_parent_offset(blob, offset), + &addrc, &sizec); + i = 0; + if (addrc == 2) + cells[i++] = 0; + cells[i++] = cpu_to_fdt32(gd->fb_base); + if (sizec == 2) + cells[i++] = 0; + cells[i++] = cpu_to_fdt32(CONFIG_SUNXI_FB_SIZE); + + ret = fdt_setprop(blob, offset, "reg", cells, sizeof(cells[0]) * i); + if (ret < 0) + goto error; + + cells[0] = cpu_to_fdt32(graphic_device->winSizeX); + ret = fdt_setprop(blob, offset, "width", cells, sizeof(cells[0])); + if (ret < 0) + goto error; + + cells[0] = cpu_to_fdt32(graphic_device->winSizeY); + ret = fdt_setprop(blob, offset, "height", cells, sizeof(cells[0])); + if (ret < 0) + goto error; + + stride = graphic_device->winSizeX * graphic_device->gdfBytesPP; + cells[0] = cpu_to_fdt32(stride); + ret = fdt_setprop(blob, offset, "stride", cells, sizeof(cells[0])); + if (ret < 0) + goto error; + + ret = fdt_setprop_string(blob, offset, "format", "x8r8g8b8"); + if (ret < 0) + goto error; + + ret = fdt_setprop_string(blob, offset, "status", "okay"); + if (ret < 0) + goto error; + + return; +error: + eprintf("Cannot setup simplefb: Error setting properties\n"); +} +#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 */