[PATCH v2 0/6] BCM2835 / simple framebuffer support

[PATCH v2 0/6] BCM2835 / simple framebuffer support

[Andre Heider]

This is v2 of my patch set posted on 10/24.
It adds framebuffer support for the RPi and provides an additional config
knob to set up the kernel's simplefb driver. The active barebox framebuffer
is basis for that feature, so that other platforms can make use of it.

This simplefb option depends on the bcm2835 fb driver since drivers need
to be aware of simplefb: framebuffers need to stay configured and cannot be
teared down.

The RPi case is rather simple in this regard. The allocated framebuffer lies
within the memory range of the VideoCore of the ARM/VC memory split. Hence,
there is no memory range that needs to be reserved as far as the ARM side
of things is concerned.
This likely needs to be added once other fb driver want to use simplefb.

Changes since v1:
* renamed "pitch" to "line_length" to match the kernel
* the bcm2835 framebuffer driver is now standalone
* configuring simplefb for the kernel is based on the active barebox
  framebuffer


Andre Heider (6):
  fb: add a line_length value to struct fb_info
  gui: convert graphic utils to respect line_length
  gui: convert the bmp renderer to respect line_length
  ARM: bcm2835: add missing mbox overscan response field
  video: add a BCM2835 framebuffer driver
  video: set up the kernel's simple framebuffer driver

 arch/arm/boards/raspberry-pi/rpi.c        |   1 +
 arch/arm/mach-bcm2835/include/mach/core.h |   5 +
 arch/arm/mach-bcm2835/include/mach/mbox.h |   1 +
 drivers/video/Kconfig                     |  14 +++
 drivers/video/Makefile                    |   2 +
 drivers/video/bcm2835.c                   | 136 ++++++++++++++++++++++++
 drivers/video/fb.c                        |  11 +-
 drivers/video/simplefb.c                  | 171 ++++++++++++++++++++++++++++++
 include/fb.h                              |   1 +
 lib/gui/bmp.c                             |   8 +-
 lib/gui/graphic_utils.c                   |  10 +-
 11 files changed, 349 insertions(+), 11 deletions(-)
 create mode 100644 drivers/video/bcm2835.c
 create mode 100644 drivers/video/simplefb.c

-- 
1.8.3.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v2 1/6] fb: add a line_length value to struct fb_info

[Andre Heider]

Add support for framebuffers with noncontiguous horizontal lines.

Video drivers can set this value if the hardware requires it.
In case a driver does not set it, the current value of
xres * (bpp / 8) is used instead.

Signed-off-by: Andre Heider <a.heider@gmail.com>
---
 drivers/video/fb.c | 11 +++++++++--
 include/fb.h       |  1 +
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/video/fb.c b/drivers/video/fb.c
index 420e4e3..0159994 100644
--- a/drivers/video/fb.c
+++ b/drivers/video/fb.c
@@ -72,12 +72,16 @@ static int fb_setup_mode(struct device_d *dev, struct param_d *param,
 
 	info->xres = info->mode->xres;
 	info->yres = info->mode->yres;
+	info->line_length = 0;
 
 	ret = info->fbops->fb_activate_var(info);
 
+	if (!info->line_length)
+		info->line_length = info->xres * (info->bits_per_pixel >> 3);
+
 	if (!ret) {
 		dev->resource[0].start = (resource_size_t)info->screen_base;
-		info->cdev.size = info->xres * info->yres * (info->bits_per_pixel >> 3);
+		info->cdev.size = info->line_length * info->yres;
 		dev->resource[0].end = dev->resource[0].start + info->cdev.size - 1;
 		dev_param_set_generic(dev, param, val);
 	} else
@@ -122,9 +126,12 @@ int register_framebuffer(struct fb_info *info)
 
 	dev = &info->dev;
 
+	if (!info->line_length)
+		info->line_length = info->xres * (info->bits_per_pixel >> 3);
+
 	info->cdev.ops = &fb_ops;
 	info->cdev.name = asprintf("fb%d", id);
-	info->cdev.size = info->xres * info->yres * (info->bits_per_pixel >> 3);
+	info->cdev.size = info->line_length * info->yres;
 	info->cdev.dev = dev;
 	info->cdev.priv = info;
 	dev->resource = xzalloc(sizeof(struct resource));
diff --git a/include/fb.h b/include/fb.h
index df4ba8e..cf9ffa0 100644
--- a/include/fb.h
+++ b/include/fb.h
@@ -94,6 +94,7 @@ struct fb_info {
 	u32 xres;			/* visible resolution		*/
 	u32 yres;
 	u32 bits_per_pixel;		/* guess what			*/
+	u32 line_length;		/* length of a line in bytes	*/
 
 	u32 grayscale;			/* != 0 Graylevels instead of colors */
 
-- 
1.8.3.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v2 2/6] gui: convert graphic utils to respect line_length

[Andre Heider]

Signed-off-by: Andre Heider <a.heider@gmail.com>
---
 lib/gui/graphic_utils.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/lib/gui/graphic_utils.c b/lib/gui/graphic_utils.c
index 95687df..300c525 100644
--- a/lib/gui/graphic_utils.c
+++ b/lib/gui/graphic_utils.c
@@ -167,18 +167,18 @@ void rgba_blend(struct fb_info *info, struct image *img, void* buf, int height,
 {
 	unsigned char *adr;
 	int x, y;
-	int xres;
+	int line_length;
 	int img_byte_per_pixel = 3;
 	void *image;
 
 	if (is_rgba)
 		img_byte_per_pixel++;
 
-	xres = info->xres;
+	line_length = info->line_length;
 
 	for (y = 0; y < height; y++) {
-		adr = buf + ((y + starty) * xres + startx) *
-				(info->bits_per_pixel >> 3);
+		adr = buf + (y + starty) * line_length +
+				startx * (info->bits_per_pixel >> 3);
 		image = img->data + (y * img->width *img_byte_per_pixel);
 
 		for (x = 0; x < width; x++) {
@@ -219,7 +219,7 @@ int fb_open(const char * fbdev, struct screen *sc, bool offscreen)
 	sc->s.y = 0;
 	sc->s.width = sc->info.xres;
 	sc->s.height = sc->info.yres;
-	sc->fbsize = sc->s.width * sc->s.height * (sc->info.bits_per_pixel >> 3);
+	sc->fbsize = sc->info.line_length * sc->s.height;
 
 	if (offscreen) {
 		/* Don't fail if malloc fails, just continue rendering directly
-- 
1.8.3.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v2 3/6] gui: convert the bmp renderer to respect line_length

[Andre Heider]

Signed-off-by: Andre Heider <a.heider@gmail.com>
---
 lib/gui/bmp.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/gui/bmp.c b/lib/gui/bmp.c
index 6bf8cd0..dcf3095 100644
--- a/lib/gui/bmp.c
+++ b/lib/gui/bmp.c
@@ -78,8 +78,8 @@ static int bmp_renderer(struct screen *sc, struct surface *s, struct image *img)
 			image = (char *)bmp +
 					le32_to_cpu(bmp->header.data_offset);
 			image += (img->height - y - 1) * img->width * (bits_per_pixel >> 3);
-			adr = buf + ((y + starty) * sc->s.width + startx) *
-					(sc->info.bits_per_pixel >> 3);
+			adr = buf + (y + starty) * sc->info.line_length +
+					startx * (sc->info.bits_per_pixel >> 3);
 			for (x = 0; x < width; x++) {
 				int pixel;
 
@@ -100,8 +100,8 @@ static int bmp_renderer(struct screen *sc, struct surface *s, struct image *img)
 			image = (char *)bmp +
 					le32_to_cpu(bmp->header.data_offset);
 			image += (img->height - y - 1) * img->width * (bits_per_pixel >> 3);
-			adr = buf + ((y + starty) * sc->s.width + startx) *
-					(sc->info.bits_per_pixel >> 3);
+			adr = buf + (y + starty) * sc->info.line_length +
+					startx * (sc->info.bits_per_pixel >> 3);
 			for (x = 0; x < width; x++) {
 				char *pixel;
 
-- 
1.8.3.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v2 4/6] ARM: bcm2835: add missing mbox overscan response field

[Andre Heider]

Add the missing "right" field to struct bcm2835_mbox_tag_overscan.

Signed-off-by: Andre Heider <a.heider@gmail.com>
---
 arch/arm/mach-bcm2835/include/mach/mbox.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/mach-bcm2835/include/mach/mbox.h b/arch/arm/mach-bcm2835/include/mach/mbox.h
index 6c36389..fb8a9bf 100644
--- a/arch/arm/mach-bcm2835/include/mach/mbox.h
+++ b/arch/arm/mach-bcm2835/include/mach/mbox.h
@@ -355,6 +355,7 @@ struct bcm2835_mbox_tag_overscan {
 			u32 top;
 			u32 bottom;
 			u32 left;
+			u32 right;
 		} resp;
 	} body;
 };
-- 
1.8.3.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v2 5/6] video: add a BCM2835 framebuffer driver

[Andre Heider]

Use the mailbox driver to set up a framebuffer based on the firmware
configuration.

Signed-off-by: Andre Heider <a.heider@gmail.com>
---
 arch/arm/boards/raspberry-pi/rpi.c        |   1 +
 arch/arm/mach-bcm2835/include/mach/core.h |   5 ++
 drivers/video/Kconfig                     |   6 ++
 drivers/video/Makefile                    |   1 +
 drivers/video/bcm2835.c                   | 136 ++++++++++++++++++++++++++++++
 5 files changed, 149 insertions(+)
 create mode 100644 drivers/video/bcm2835.c

diff --git a/arch/arm/boards/raspberry-pi/rpi.c b/arch/arm/boards/raspberry-pi/rpi.c
index 2717fee..96025ba 100644
--- a/arch/arm/boards/raspberry-pi/rpi.c
+++ b/arch/arm/boards/raspberry-pi/rpi.c
@@ -140,6 +140,7 @@ static int rpi_env_init(void)
 static int rpi_devices_init(void)
 {
 	bcm2835_register_mci();
+	bcm2835_register_fb();
 	armlinux_set_architecture(MACH_TYPE_BCM2708);
 	armlinux_set_bootparams((void *)(0x00000100));
 	rpi_env_init();
diff --git a/arch/arm/mach-bcm2835/include/mach/core.h b/arch/arm/mach-bcm2835/include/mach/core.h
index a095db8..477ecb9 100644
--- a/arch/arm/mach-bcm2835/include/mach/core.h
+++ b/arch/arm/mach-bcm2835/include/mach/core.h
@@ -27,4 +27,9 @@ static void inline bcm2835_register_mci(void)
 			IORESOURCE_MEM, NULL);
 }
 
+static void inline bcm2835_register_fb(void)
+{
+	add_generic_device("bcm2835_fb", 0, NULL, 0, 0, 0, NULL);
+}
+
 #endif
diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 0639d9c..b532e7d 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -71,4 +71,10 @@ config DRIVER_VIDEO_PXA
 	  Add support for the frame buffer device found on the PXA270
 	  CPU.
 
+config DRIVER_VIDEO_BCM2835
+	bool "BCM2835 framebuffer driver"
+	depends on ARCH_BCM2835
+	help
+	  Add support for the BCM2835/VideoCore frame buffer device.
+
 endif
diff --git a/drivers/video/Makefile b/drivers/video/Makefile
index 67169d1..244feab 100644
--- a/drivers/video/Makefile
+++ b/drivers/video/Makefile
@@ -9,3 +9,4 @@ obj-$(CONFIG_DRIVER_VIDEO_S3C24XX) += s3c24xx.o
 obj-$(CONFIG_DRIVER_VIDEO_PXA) += pxa.o
 obj-$(CONFIG_DRIVER_VIDEO_SDL) += sdl.o
 obj-$(CONFIG_DRIVER_VIDEO_OMAP) += omap.o
+obj-$(CONFIG_DRIVER_VIDEO_BCM2835) += bcm2835.o
diff --git a/drivers/video/bcm2835.c b/drivers/video/bcm2835.c
new file mode 100644
index 0000000..3d52f8b
--- /dev/null
+++ b/drivers/video/bcm2835.c
@@ -0,0 +1,136 @@
+/*
+ * BCM2835 framebuffer driver
+ *
+ * Copyright (C) 2013 Andre Heider
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <driver.h>
+#include <errno.h>
+#include <fb.h>
+#include <init.h>
+#include <malloc.h>
+#include <xfuncs.h>
+
+#include <mach/mbox.h>
+
+struct bcm2835fb_info {
+	struct fb_info fbi;
+	struct fb_videomode mode;
+};
+
+struct msg_fb_query {
+	struct bcm2835_mbox_hdr hdr;
+	struct bcm2835_mbox_tag_physical_w_h physical_w_h;
+	u32 end_tag;
+};
+
+struct msg_fb_setup {
+	struct bcm2835_mbox_hdr hdr;
+	struct bcm2835_mbox_tag_physical_w_h physical_w_h;
+	struct bcm2835_mbox_tag_virtual_w_h virtual_w_h;
+	struct bcm2835_mbox_tag_depth depth;
+	struct bcm2835_mbox_tag_pixel_order pixel_order;
+	struct bcm2835_mbox_tag_alpha_mode alpha_mode;
+	struct bcm2835_mbox_tag_virtual_offset virtual_offset;
+	struct bcm2835_mbox_tag_allocate_buffer allocate_buffer;
+	struct bcm2835_mbox_tag_pitch pitch;
+	u32 end_tag;
+};
+
+static void bcm2835fb_enable(struct fb_info *info)
+{
+}
+
+static void bcm2835fb_disable(struct fb_info *info)
+{
+}
+
+static struct fb_ops bcm2835fb_ops = {
+	.fb_enable		= bcm2835fb_enable,
+	.fb_disable		= bcm2835fb_disable,
+};
+
+static int bcm2835fb_probe(struct device_d *dev)
+{
+	BCM2835_MBOX_STACK_ALIGN(struct msg_fb_query, msg_query);
+	BCM2835_MBOX_STACK_ALIGN(struct msg_fb_setup, msg_setup);
+	struct bcm2835fb_info *info;
+	u32 w, h;
+	int ret;
+
+	BCM2835_MBOX_INIT_HDR(msg_query);
+	BCM2835_MBOX_INIT_TAG_NO_REQ(&msg_query->physical_w_h,
+					GET_PHYSICAL_W_H);
+	ret = bcm2835_mbox_call_prop(BCM2835_MBOX_PROP_CHAN, &msg_query->hdr);
+	if (ret) {
+		dev_err(dev, "could not query display resolution\n");
+		return ret;
+	}
+
+	w = msg_query->physical_w_h.body.resp.width;
+	h = msg_query->physical_w_h.body.resp.height;
+
+	BCM2835_MBOX_INIT_HDR(msg_setup);
+	BCM2835_MBOX_INIT_TAG(&msg_setup->physical_w_h, SET_PHYSICAL_W_H);
+	msg_setup->physical_w_h.body.req.width = w;
+	msg_setup->physical_w_h.body.req.height = h;
+	BCM2835_MBOX_INIT_TAG(&msg_setup->virtual_w_h, SET_VIRTUAL_W_H);
+	msg_setup->virtual_w_h.body.req.width = w;
+	msg_setup->virtual_w_h.body.req.height = h;
+	BCM2835_MBOX_INIT_TAG(&msg_setup->depth, SET_DEPTH);
+	msg_setup->depth.body.req.bpp = 16;
+	BCM2835_MBOX_INIT_TAG(&msg_setup->pixel_order, SET_PIXEL_ORDER);
+	msg_setup->pixel_order.body.req.order = BCM2835_MBOX_PIXEL_ORDER_BGR;
+	BCM2835_MBOX_INIT_TAG(&msg_setup->alpha_mode, SET_ALPHA_MODE);
+	msg_setup->alpha_mode.body.req.alpha = BCM2835_MBOX_ALPHA_MODE_IGNORED;
+	BCM2835_MBOX_INIT_TAG(&msg_setup->virtual_offset, SET_VIRTUAL_OFFSET);
+	msg_setup->virtual_offset.body.req.x = 0;
+	msg_setup->virtual_offset.body.req.y = 0;
+	BCM2835_MBOX_INIT_TAG(&msg_setup->allocate_buffer, ALLOCATE_BUFFER);
+	msg_setup->allocate_buffer.body.req.alignment = 0x100;
+	BCM2835_MBOX_INIT_TAG_NO_REQ(&msg_setup->pitch, GET_PITCH);
+
+	ret = bcm2835_mbox_call_prop(BCM2835_MBOX_PROP_CHAN, &msg_setup->hdr);
+	if (ret) {
+		dev_err(dev, "could not configure display\n");
+		return ret;
+	}
+
+	info = xzalloc(sizeof *info);
+	info->fbi.fbops = &bcm2835fb_ops;
+	info->fbi.screen_base =
+	   (void *)msg_setup->allocate_buffer.body.resp.fb_address;
+	info->fbi.xres = msg_setup->physical_w_h.body.resp.width;
+	info->fbi.yres = msg_setup->physical_w_h.body.resp.height;
+	info->fbi.bits_per_pixel = 16;
+	info->fbi.line_length = msg_setup->pitch.body.resp.pitch;
+	info->fbi.red.length = 5;
+	info->fbi.red.offset = 11;
+	info->fbi.green.length = 6;
+	info->fbi.green.offset = 5;
+	info->fbi.blue.length = 5;
+	info->fbi.blue.offset = 0;
+
+	info->fbi.mode = &info->mode;
+	info->fbi.mode->xres = info->fbi.xres;
+	info->fbi.mode->yres = info->fbi.yres;
+
+	ret = register_framebuffer(&info->fbi);
+	if (ret) {
+		free(info);
+		dev_err(dev, "failed to register framebuffer: %d\n", ret);
+		return ret;
+	}
+
+	dev_info(dev, "registered\n");
+	return 0;
+}
+
+static struct driver_d bcm2835fb_driver = {
+	.name	= "bcm2835_fb",
+	.probe	= bcm2835fb_probe,
+};
+device_platform_driver(bcm2835fb_driver);
-- 
1.8.3.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v2 6/6] video: set up the kernel's simple framebuffer driver

[Andre Heider]

Add support to configure the active framebuffer for the kernel through
device tree.

Signed-off-by: Andre Heider <a.heider@gmail.com>
---
 drivers/video/Kconfig    |   8 +++
 drivers/video/Makefile   |   1 +
 drivers/video/simplefb.c | 171 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 180 insertions(+)
 create mode 100644 drivers/video/simplefb.c

diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index b532e7d..ccfaba8 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -77,4 +77,12 @@ config DRIVER_VIDEO_BCM2835
 	help
 	  Add support for the BCM2835/VideoCore frame buffer device.
 
+config DRIVER_VIDEO_SIMPLEFB
+	bool "Simple framebuffer support"
+	depends on OFTREE
+	depends on DRIVER_VIDEO_BCM2835
+	help
+	  Add support for setting up the kernel's simple framebuffer driver
+	  based on the active barebox framebuffer.
+
 endif
diff --git a/drivers/video/Makefile b/drivers/video/Makefile
index 244feab..31edfca 100644
--- a/drivers/video/Makefile
+++ b/drivers/video/Makefile
@@ -10,3 +10,4 @@ obj-$(CONFIG_DRIVER_VIDEO_PXA) += pxa.o
 obj-$(CONFIG_DRIVER_VIDEO_SDL) += sdl.o
 obj-$(CONFIG_DRIVER_VIDEO_OMAP) += omap.o
 obj-$(CONFIG_DRIVER_VIDEO_BCM2835) += bcm2835.o
+obj-$(CONFIG_DRIVER_VIDEO_SIMPLEFB) += simplefb.o
diff --git a/drivers/video/simplefb.c b/drivers/video/simplefb.c
new file mode 100644
index 0000000..3ea4ad4
--- /dev/null
+++ b/drivers/video/simplefb.c
@@ -0,0 +1,171 @@
+/*
+ * SimpleFB driver
+ *
+ * Copyright (C) 2013 Andre Heider
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#define pr_fmt(fmt) "simplefb: " fmt
+
+#include <common.h>
+#include <errno.h>
+#include <fb.h>
+#include <fcntl.h>
+#include <fs.h>
+#include <init.h>
+#include <xfuncs.h>
+
+struct simplefb_mode {
+	const char *format;
+	u32 bpp;
+	struct fb_bitfield red;
+	struct fb_bitfield green;
+	struct fb_bitfield blue;
+	struct fb_bitfield transp;
+};
+
+/*
+ * These values have to match the kernel's simplefb driver.
+ * See Documentation/devicetree/bindings/video/simple-framebuffer.txt
+ */
+static const struct simplefb_mode simplefb_modes[] = {
+	{
+		.format	= "r5g6b5",
+		.bpp	= 16,
+		.red	= { .length = 5, .offset = 11 },
+		.green	= { .length = 6, .offset = 5 },
+		.blue	= { .length = 5, .offset = 0 },
+		.transp	= { .length = 0, .offset = 0 },
+	},
+};
+
+static bool simplefb_bitfield_cmp(const struct fb_bitfield *a,
+					const struct fb_bitfield *b)
+{
+	if (a->offset != b->offset)
+		return false;
+	if (a->length != b->length)
+		return false;
+	if (a->msb_right != b->msb_right)
+		return false;
+
+	return true;
+}
+
+static const struct simplefb_mode *simplefb_find_mode(const struct fb_info *fbi)
+{
+	const struct simplefb_mode *mode;
+	u32 i;
+
+	for (i = 0; i < ARRAY_SIZE(simplefb_modes); ++i) {
+		mode = &simplefb_modes[i];
+
+		if (fbi->bits_per_pixel != mode->bpp)
+			continue;
+		if (!simplefb_bitfield_cmp(&fbi->red, &mode->red))
+			continue;
+		if (!simplefb_bitfield_cmp(&fbi->green, &mode->green))
+			continue;
+		if (!simplefb_bitfield_cmp(&fbi->blue, &mode->blue))
+			continue;
+		if (!simplefb_bitfield_cmp(&fbi->transp, &mode->transp))
+			continue;
+
+		return mode;
+	}
+
+	return NULL;
+}
+
+static int simplefb_create_node(struct device_node *root,
+				const struct fb_info *fbi, const char *format)
+{
+	const char *compat = "simple-framebuffer";
+	const char *disabled = "disabled";
+	const char *okay = "okay";
+	struct device_node *node;
+	u32 cells[2];
+	int ret;
+
+	node = of_create_node(root, "/framebuffer");
+	if (!node)
+		return -ENOMEM;
+
+	ret = of_set_property(node, "status", disabled,
+				strlen(disabled) + 1, 1);
+	if (ret < 0)
+		return ret;
+
+	ret = of_set_property(node, "compatible", compat, strlen(compat) + 1, 1);
+	if (ret)
+		return ret;
+
+	cells[0] = cpu_to_be32((u32)fbi->screen_base);
+	cells[1] = cpu_to_be32(fbi->line_length * fbi->yres);
+	ret = of_set_property(node, "reg", cells, sizeof(cells[0]) * 2, 1);
+	if (ret < 0)
+		return ret;
+
+	cells[0] = cpu_to_be32(fbi->xres);
+	ret = of_set_property(node, "width", cells, sizeof(cells[0]), 1);
+	if (ret < 0)
+		return ret;
+
+	cells[0] = cpu_to_be32(fbi->yres);
+	ret = of_set_property(node, "height", cells, sizeof(cells[0]), 1);
+	if (ret < 0)
+		return ret;
+
+	cells[0] = cpu_to_be32(fbi->line_length);
+	ret = of_set_property(node, "stride", cells, sizeof(cells[0]), 1);
+	if (ret < 0)
+		return ret;
+
+	ret = of_set_property(node, "format", format, strlen(format) + 1, 1);
+	if (ret < 0)
+		return ret;
+
+	return of_set_property(node, "status", okay, strlen(okay) + 1, 1);
+}
+
+static int simplefb_of_fixup(struct device_node *root)
+{
+	struct fb_info fbi;
+	const struct simplefb_mode *mode;
+	int fd, ret;
+
+	fd = open("/dev/fb0", O_RDONLY);
+	if (fd < 0) {
+		pr_err("failed to open /dev/fb0: %d\n", fd);
+		return fd;
+	}
+
+	ret = ioctl(fd, FBIOGET_SCREENINFO, &fbi);
+	close(fd);
+
+	if (ret) {
+		pr_err("failed to get the framebuffer info: %d\n", ret);
+		return ret;
+	}
+
+	mode = simplefb_find_mode(&fbi);
+	if (!mode) {
+		pr_err("the framebuffer format is incompatible\n");
+		return -EINVAL;
+	}
+
+	ret = simplefb_create_node(root, &fbi, mode->format);
+	if (ret)
+		pr_err("failed to create node: %d\n", ret);
+	else
+		pr_info("created %s node\n", mode->format);
+
+	return ret;
+}
+
+static int simplefb_init(void) {
+	of_register_fixup(simplefb_of_fixup);
+	return 0;
+}
+late_initcall(simplefb_init);
-- 
1.8.3.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH v2 1/6] fb: add a line_length value to struct fb_info

[Alexander Aring]

On Tue, Nov 05, 2013 at 12:00:59AM +0100, Andre Heider wrote:
> Add support for framebuffers with noncontiguous horizontal lines.
> 
> Video drivers can set this value if the hardware requires it.
> In case a driver does not set it, the current value of
> xres * (bpp / 8) is used instead.
> 
> Signed-off-by: Andre Heider <a.heider@gmail.com>
> ---
>  drivers/video/fb.c | 11 +++++++++--
>  include/fb.h       |  1 +
>  2 files changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/video/fb.c b/drivers/video/fb.c
> index 420e4e3..0159994 100644
> --- a/drivers/video/fb.c
> +++ b/drivers/video/fb.c
> @@ -72,12 +72,16 @@ static int fb_setup_mode(struct device_d *dev, struct param_d *param,
>  
>  	info->xres = info->mode->xres;
>  	info->yres = info->mode->yres;
> +	info->line_length = 0;
>  
>  	ret = info->fbops->fb_activate_var(info);
>  
> +	if (!info->line_length)
> +		info->line_length = info->xres * (info->bits_per_pixel >> 3);
> +
Isn't this already set from the register_framebuffer function?
You wanna see that somebody changed this value in info->fbops->fb_activate_var(info)?

Then I would do nothing here.

- Alex

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH v2 1/6] fb: add a line_length value to struct fb_info

[Sascha Hauer]

On Tue, Nov 05, 2013 at 01:06:22AM +0100, Alexander Aring wrote:
> On Tue, Nov 05, 2013 at 12:00:59AM +0100, Andre Heider wrote:
> > Add support for framebuffers with noncontiguous horizontal lines.
> > 
> > Video drivers can set this value if the hardware requires it.
> > In case a driver does not set it, the current value of
> > xres * (bpp / 8) is used instead.
> > 
> > Signed-off-by: Andre Heider <a.heider@gmail.com>
> > ---
> >  drivers/video/fb.c | 11 +++++++++--
> >  include/fb.h       |  1 +
> >  2 files changed, 10 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/video/fb.c b/drivers/video/fb.c
> > index 420e4e3..0159994 100644
> > --- a/drivers/video/fb.c
> > +++ b/drivers/video/fb.c
> > @@ -72,12 +72,16 @@ static int fb_setup_mode(struct device_d *dev, struct param_d *param,
> >  
> >  	info->xres = info->mode->xres;
> >  	info->yres = info->mode->yres;
> > +	info->line_length = 0;
> >  
> >  	ret = info->fbops->fb_activate_var(info);
> >  
> > +	if (!info->line_length)
> > +		info->line_length = info->xres * (info->bits_per_pixel >> 3);
> > +
> Isn't this already set from the register_framebuffer function?
> You wanna see that somebody changed this value in info->fbops->fb_activate_var(info)?
> 
> Then I would do nothing here.

It is set from register_framebuffer, but the mode setup invalidates it,
So Andre sets it to 0 first and updates it after calling into the driver
if necessary. The code looks correct. Maybe I misunderstand you?

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH v2 1/6] fb: add a line_length value to struct fb_info

[Alexander Aring]

Hi Sascha,

On Tue, Nov 05, 2013 at 09:00:57AM +0100, Sascha Hauer wrote:
> On Tue, Nov 05, 2013 at 01:06:22AM +0100, Alexander Aring wrote:
> > On Tue, Nov 05, 2013 at 12:00:59AM +0100, Andre Heider wrote:
> > > Add support for framebuffers with noncontiguous horizontal lines.
> > > 
> > > Video drivers can set this value if the hardware requires it.
> > > In case a driver does not set it, the current value of
> > > xres * (bpp / 8) is used instead.
> > > 
> > > Signed-off-by: Andre Heider <a.heider@gmail.com>
> > > ---
> > >  drivers/video/fb.c | 11 +++++++++--
> > >  include/fb.h       |  1 +
> > >  2 files changed, 10 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/video/fb.c b/drivers/video/fb.c
> > > index 420e4e3..0159994 100644
> > > --- a/drivers/video/fb.c
> > > +++ b/drivers/video/fb.c
> > > @@ -72,12 +72,16 @@ static int fb_setup_mode(struct device_d *dev, struct param_d *param,
> > >  
> > >  	info->xres = info->mode->xres;
> > >  	info->yres = info->mode->yres;
> > > +	info->line_length = 0;
> > >  
> > >  	ret = info->fbops->fb_activate_var(info);
> > >  
> > > +	if (!info->line_length)
> > > +		info->line_length = info->xres * (info->bits_per_pixel >> 3);
> > > +
> > Isn't this already set from the register_framebuffer function?
> > You wanna see that somebody changed this value in info->fbops->fb_activate_var(info)?
> > 
> > Then I would do nothing here.
> 
> It is set from register_framebuffer, but the mode setup invalidates it,
> So Andre sets it to 0 first and updates it after calling into the driver
> if necessary. The code looks correct. Maybe I misunderstand you?
> 
ok. Was a hard day and night for me ;-)
Maybe I was too tired and look all day into another code and other
things ;-).

I think I need a coffee or beer now.


I mean we have the same code in register_framebuffer:

if (!info->line_length)
        info->line_length = info->xres * (info->bits_per_pixel >> 3);

I don't know the exactly context to call fb_setup_mode but then we set
it twice and we don't need to check if the value has changed from
fb_activate_var function.

It's only a nitpick, but this code is correct. Of course.

- Alex

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH v2 1/6] fb: add a line_length value to struct fb_info

[Andre Heider]

Hi Alexander,

On Tue, Nov 05, 2013 at 09:17:44AM +0100, Alexander Aring wrote:
> Hi Sascha,
> 
> On Tue, Nov 05, 2013 at 09:00:57AM +0100, Sascha Hauer wrote:
> > On Tue, Nov 05, 2013 at 01:06:22AM +0100, Alexander Aring wrote:
> > > On Tue, Nov 05, 2013 at 12:00:59AM +0100, Andre Heider wrote:
> > > > Add support for framebuffers with noncontiguous horizontal lines.
> > > > 
> > > > Video drivers can set this value if the hardware requires it.
> > > > In case a driver does not set it, the current value of
> > > > xres * (bpp / 8) is used instead.
> > > > 
> > > > Signed-off-by: Andre Heider <a.heider@gmail.com>
> > > > ---
> > > >  drivers/video/fb.c | 11 +++++++++--
> > > >  include/fb.h       |  1 +
> > > >  2 files changed, 10 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/drivers/video/fb.c b/drivers/video/fb.c
> > > > index 420e4e3..0159994 100644
> > > > --- a/drivers/video/fb.c
> > > > +++ b/drivers/video/fb.c
> > > > @@ -72,12 +72,16 @@ static int fb_setup_mode(struct device_d *dev, struct param_d *param,
> > > >  
> > > >  	info->xres = info->mode->xres;
> > > >  	info->yres = info->mode->yres;
> > > > +	info->line_length = 0;
> > > >  
> > > >  	ret = info->fbops->fb_activate_var(info);
> > > >  
> > > > +	if (!info->line_length)
> > > > +		info->line_length = info->xres * (info->bits_per_pixel >> 3);
> > > > +
> > > Isn't this already set from the register_framebuffer function?
> > > You wanna see that somebody changed this value in info->fbops->fb_activate_var(info)?
> > > 
> > > Then I would do nothing here.
> > 
> > It is set from register_framebuffer, but the mode setup invalidates it,
> > So Andre sets it to 0 first and updates it after calling into the driver
> > if necessary. The code looks correct. Maybe I misunderstand you?
> > 
> ok. Was a hard day and night for me ;-)
> Maybe I was too tired and look all day into another code and other
> things ;-).
> 
> I think I need a coffee or beer now.
> 
> 
> I mean we have the same code in register_framebuffer:
> 
> if (!info->line_length)
>         info->line_length = info->xres * (info->bits_per_pixel >> 3);
> 
> I don't know the exactly context to call fb_setup_mode but then we set
> it twice and we don't need to check if the value has changed from
> fb_activate_var function.

With this patch we have a new line_length value, but fb drivers don't
have to set it, its optional. What you see in register_framebuffer()
is a default value for the initial mode. The framework sets it in case
the driver doesn't.

But we can also change modes, so if a fb driver doesn't update
line_length in the fb_activate_var callback, we have a stale line_length
value from the old mode.
So I set it to zero before the callback and check afterwards if the
driver set it. Without that there would be no way to distinguish if the
value is valid for the new mode or a stale one from the prior mode.

Thanks,
Andre

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH v2 1/6] fb: add a line_length value to struct fb_info

[Alexander Aring]

On Tue, Nov 05, 2013 at 09:48:07AM +0100, Andre Heider wrote:
> Hi Alexander,
> 
> On Tue, Nov 05, 2013 at 09:17:44AM +0100, Alexander Aring wrote:
> > Hi Sascha,
> > 
> > On Tue, Nov 05, 2013 at 09:00:57AM +0100, Sascha Hauer wrote:
> > > On Tue, Nov 05, 2013 at 01:06:22AM +0100, Alexander Aring wrote:
> > > > On Tue, Nov 05, 2013 at 12:00:59AM +0100, Andre Heider wrote:
> > > > > Add support for framebuffers with noncontiguous horizontal lines.
> > > > > 
> > > > > Video drivers can set this value if the hardware requires it.
> > > > > In case a driver does not set it, the current value of
> > > > > xres * (bpp / 8) is used instead.
> > > > > 
> > > > > Signed-off-by: Andre Heider <a.heider@gmail.com>
> > > > > ---
> > > > >  drivers/video/fb.c | 11 +++++++++--
> > > > >  include/fb.h       |  1 +
> > > > >  2 files changed, 10 insertions(+), 2 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/video/fb.c b/drivers/video/fb.c
> > > > > index 420e4e3..0159994 100644
> > > > > --- a/drivers/video/fb.c
> > > > > +++ b/drivers/video/fb.c
> > > > > @@ -72,12 +72,16 @@ static int fb_setup_mode(struct device_d *dev, struct param_d *param,
> > > > >  
> > > > >  	info->xres = info->mode->xres;
> > > > >  	info->yres = info->mode->yres;
> > > > > +	info->line_length = 0;
> > > > >  
> > > > >  	ret = info->fbops->fb_activate_var(info);
> > > > >  
> > > > > +	if (!info->line_length)
> > > > > +		info->line_length = info->xres * (info->bits_per_pixel >> 3);
> > > > > +
> > > > Isn't this already set from the register_framebuffer function?
> > > > You wanna see that somebody changed this value in info->fbops->fb_activate_var(info)?
> > > > 
> > > > Then I would do nothing here.
> > > 
> > > It is set from register_framebuffer, but the mode setup invalidates it,
> > > So Andre sets it to 0 first and updates it after calling into the driver
> > > if necessary. The code looks correct. Maybe I misunderstand you?
> > > 
> > ok. Was a hard day and night for me ;-)
> > Maybe I was too tired and look all day into another code and other
> > things ;-).
> > 
> > I think I need a coffee or beer now.
> > 
> > 
> > I mean we have the same code in register_framebuffer:
> > 
> > if (!info->line_length)
> >         info->line_length = info->xres * (info->bits_per_pixel >> 3);
> > 
> > I don't know the exactly context to call fb_setup_mode but then we set
> > it twice and we don't need to check if the value has changed from
> > fb_activate_var function.
> 
> With this patch we have a new line_length value, but fb drivers don't
> have to set it, its optional. What you see in register_framebuffer()
> is a default value for the initial mode. The framework sets it in case
> the driver doesn't.
> 
> But we can also change modes, so if a fb driver doesn't update
> line_length in the fb_activate_var callback, we have a stale line_length
> value from the old mode.
> So I set it to zero before the callback and check afterwards if the
> driver set it. Without that there would be no way to distinguish if the
> value is valid for the new mode or a stale one from the prior mode.
> 
ok, thanks for the little explaination.

- Alex

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH v2 0/6] BCM2835 / simple framebuffer support

[Sascha Hauer]

Hi Andre,

On Tue, Nov 05, 2013 at 12:00:58AM +0100, Andre Heider wrote:
> This is v2 of my patch set posted on 10/24.
> It adds framebuffer support for the RPi and provides an additional config
> knob to set up the kernel's simplefb driver. The active barebox framebuffer
> is basis for that feature, so that other platforms can make use of it.
> 
> This simplefb option depends on the bcm2835 fb driver since drivers need
> to be aware of simplefb: framebuffers need to stay configured and cannot be
> teared down.
> 
> The RPi case is rather simple in this regard. The allocated framebuffer lies
> within the memory range of the VideoCore of the ARM/VC memory split. Hence,
> there is no memory range that needs to be reserved as far as the ARM side
> of things is concerned.
> This likely needs to be added once other fb driver want to use simplefb.
> 
> Changes since v1:
> * renamed "pitch" to "line_length" to match the kernel
> * the bcm2835 framebuffer driver is now standalone
> * configuring simplefb for the kernel is based on the active barebox
>   framebuffer

I applied this series without the simplefb patch.

For the simplefb I'd like to use the following patch instead. It's your
patch with some adjustments:

- add a register_simplefb device parameter to make it configurable
  whether a simplefb node should be created or not.
- register the OF fixup in register_framebuffer(). This way we always
  have a simplefb node once a framebuffer is registered without getting
  spurious warnings when there is no framebuffer present.
- check if the framebuffer is actually enabled. If it's not, we
  shouldn't register a simplefb node.

Are you fine with this and if yes, could you give the patch a test?
It depends on the add-context-pointer-to-of_register_fixup patch I just
posted. Just try the current -next branch, it should contain all you
need for testing.

One oddity I just realized is that my patch will try and create a
/framebuffer node multiple times if multiple framebuffers exist.
I would have to fix this.

Sascha

8<----------------------------------------------------------

From 97607e85cdc44824e9617c74325bc9bc6405c383 Mon Sep 17 00:00:00 2001
From: Andre Heider <a.heider@gmail.com>
Date: Tue, 5 Nov 2013 00:01:04 +0100
Subject: [PATCH] video: set up the kernel's simple framebuffer driver

Add support to configure the active framebuffer for the kernel through
device tree.

Signed-off-by: Andre Heider <a.heider@gmail.com>
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/video/Kconfig    |   7 ++
 drivers/video/Makefile   |   1 +
 drivers/video/fb.c       |   7 ++
 drivers/video/simplefb.c | 167 +++++++++++++++++++++++++++++++++++++++++++++++
 include/fb.h             |   7 ++
 5 files changed, 189 insertions(+)
 create mode 100644 drivers/video/simplefb.c

diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index b532e7d..5539266 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -77,4 +77,11 @@ config DRIVER_VIDEO_BCM2835
 	help
 	  Add support for the BCM2835/VideoCore frame buffer device.
 
+config DRIVER_VIDEO_SIMPLEFB
+	bool "Simple framebuffer support"
+	depends on OFTREE
+	help
+	  Add support for setting up the kernel's simple framebuffer driver
+	  based on the active barebox framebuffer.
+
 endif
diff --git a/drivers/video/Makefile b/drivers/video/Makefile
index 244feab..31edfca 100644
--- a/drivers/video/Makefile
+++ b/drivers/video/Makefile
@@ -10,3 +10,4 @@ obj-$(CONFIG_DRIVER_VIDEO_PXA) += pxa.o
 obj-$(CONFIG_DRIVER_VIDEO_SDL) += sdl.o
 obj-$(CONFIG_DRIVER_VIDEO_OMAP) += omap.o
 obj-$(CONFIG_DRIVER_VIDEO_BCM2835) += bcm2835.o
+obj-$(CONFIG_DRIVER_VIDEO_SIMPLEFB) += simplefb.o
diff --git a/drivers/video/fb.c b/drivers/video/fb.c
index 0159994..4263027 100644
--- a/drivers/video/fb.c
+++ b/drivers/video/fb.c
@@ -163,6 +163,13 @@ int register_framebuffer(struct fb_info *info)
 	if (ret)
 		goto err_unregister;
 
+	if (IS_ENABLED(CONFIG_DRIVER_VIDEO_SIMPLEFB)) {
+		ret = fb_register_simplefb(info);
+		if (ret)
+			dev_err(&info->dev, "failed to register simplefb: %s\n",
+					strerror(-ret));
+	}
+
 	return 0;
 
 err_unregister:
diff --git a/drivers/video/simplefb.c b/drivers/video/simplefb.c
new file mode 100644
index 0000000..834704b
--- /dev/null
+++ b/drivers/video/simplefb.c
@@ -0,0 +1,167 @@
+/*
+ * SimpleFB driver
+ *
+ * Copyright (C) 2013 Andre Heider
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#define pr_fmt(fmt) "simplefb: " fmt
+
+#include <common.h>
+#include <errno.h>
+#include <fb.h>
+#include <fcntl.h>
+#include <fs.h>
+#include <init.h>
+#include <xfuncs.h>
+
+struct simplefb_mode {
+	const char *format;
+	u32 bpp;
+	struct fb_bitfield red;
+	struct fb_bitfield green;
+	struct fb_bitfield blue;
+	struct fb_bitfield transp;
+};
+
+/*
+ * These values have to match the kernel's simplefb driver.
+ * See Documentation/devicetree/bindings/video/simple-framebuffer.txt
+ */
+static const struct simplefb_mode simplefb_modes[] = {
+	{
+		.format	= "r5g6b5",
+		.bpp	= 16,
+		.red	= { .length = 5, .offset = 11 },
+		.green	= { .length = 6, .offset = 5 },
+		.blue	= { .length = 5, .offset = 0 },
+		.transp	= { .length = 0, .offset = 0 },
+	},
+};
+
+static bool simplefb_bitfield_cmp(const struct fb_bitfield *a,
+					const struct fb_bitfield *b)
+{
+	if (a->offset != b->offset)
+		return false;
+	if (a->length != b->length)
+		return false;
+	if (a->msb_right != b->msb_right)
+		return false;
+
+	return true;
+}
+
+static const struct simplefb_mode *simplefb_find_mode(const struct fb_info *fbi)
+{
+	const struct simplefb_mode *mode;
+	u32 i;
+
+	for (i = 0; i < ARRAY_SIZE(simplefb_modes); ++i) {
+		mode = &simplefb_modes[i];
+
+		if (fbi->bits_per_pixel != mode->bpp)
+			continue;
+		if (!simplefb_bitfield_cmp(&fbi->red, &mode->red))
+			continue;
+		if (!simplefb_bitfield_cmp(&fbi->green, &mode->green))
+			continue;
+		if (!simplefb_bitfield_cmp(&fbi->blue, &mode->blue))
+			continue;
+		if (!simplefb_bitfield_cmp(&fbi->transp, &mode->transp))
+			continue;
+
+		return mode;
+	}
+
+	return NULL;
+}
+
+static int simplefb_create_node(struct device_node *root,
+				const struct fb_info *fbi, const char *format)
+{
+	const char *compat = "simple-framebuffer";
+	const char *disabled = "disabled";
+	const char *okay = "okay";
+	struct device_node *node;
+	u32 cells[2];
+	int ret;
+
+	node = of_create_node(root, "/framebuffer");
+	if (!node)
+		return -ENOMEM;
+
+	ret = of_set_property(node, "status", disabled,
+				strlen(disabled) + 1, 1);
+	if (ret < 0)
+		return ret;
+
+	ret = of_set_property(node, "compatible", compat, strlen(compat) + 1, 1);
+	if (ret)
+		return ret;
+
+	cells[0] = cpu_to_be32((u32)fbi->screen_base);
+	cells[1] = cpu_to_be32(fbi->line_length * fbi->yres);
+	ret = of_set_property(node, "reg", cells, sizeof(cells[0]) * 2, 1);
+	if (ret < 0)
+		return ret;
+
+	cells[0] = cpu_to_be32(fbi->xres);
+	ret = of_set_property(node, "width", cells, sizeof(cells[0]), 1);
+	if (ret < 0)
+		return ret;
+
+	cells[0] = cpu_to_be32(fbi->yres);
+	ret = of_set_property(node, "height", cells, sizeof(cells[0]), 1);
+	if (ret < 0)
+		return ret;
+
+	cells[0] = cpu_to_be32(fbi->line_length);
+	ret = of_set_property(node, "stride", cells, sizeof(cells[0]), 1);
+	if (ret < 0)
+		return ret;
+
+	ret = of_set_property(node, "format", format, strlen(format) + 1, 1);
+	if (ret < 0)
+		return ret;
+
+	return of_set_property(node, "status", okay, strlen(okay) + 1, 1);
+}
+
+static int simplefb_of_fixup(struct device_node *root, void *ctx)
+{
+	struct fb_info *info = ctx;
+	const struct simplefb_mode *mode;
+	int ret;
+
+	/* only create node if we are requested to */
+	if (!info->register_simplefb)
+		return 0;
+
+	/* do not create node for disabled framebuffers */
+	if (!info->enabled)
+		return 0;
+
+	mode = simplefb_find_mode(info);
+	if (!mode) {
+		dev_err(&info->dev, "fb format is incompatible with simplefb\n");
+		return -EINVAL;
+	}
+
+	ret = simplefb_create_node(root, info, mode->format);
+	if (ret)
+		dev_err(&info->dev, "failed to create node: %d\n", ret);
+	else
+		dev_info(&info->dev, "created %s node\n", mode->format);
+
+	return ret;
+}
+
+int fb_register_simplefb(struct fb_info *info)
+{
+	dev_add_param_bool(&info->dev, "register_simplefb",
+			NULL, NULL, &info->register_simplefb, NULL);
+
+	return of_register_fixup(simplefb_of_fixup, info);
+}
diff --git a/include/fb.h b/include/fb.h
index cf9ffa0..98d5a03 100644
--- a/include/fb.h
+++ b/include/fb.h
@@ -105,6 +105,9 @@ struct fb_info {
 
 	int enabled;
 	int p_enable;
+	int register_simplefb;		/* If true a simplefb device node will
+					 * be created.
+					 */
 };
 
 int register_framebuffer(struct fb_info *info);
@@ -115,5 +118,9 @@ int register_framebuffer(struct fb_info *info);
 
 extern struct bus_type fb_bus;
 
+/* fb internal functions */
+
+int fb_register_simplefb(struct fb_info *info);
+
 #endif /* __FB_H */
 
-- 
1.8.4.rc3


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH v2 0/6] BCM2835 / simple framebuffer support

[Sascha Hauer]

On Wed, Nov 06, 2013 at 11:04:02AM +0100, Alexander Aring wrote:
> Hi Sascha,
> 
> On Wed, Nov 06, 2013 at 10:46:28AM +0100, Sascha Hauer wrote:
> > Hi Andre,
> > @@ -163,6 +163,13 @@ int register_framebuffer(struct fb_info *info)
> >  	if (ret)
> >  		goto err_unregister;
> >  
> > +	if (IS_ENABLED(CONFIG_DRIVER_VIDEO_SIMPLEFB)) {
> > +		ret = fb_register_simplefb(info);
> > +		if (ret)
> > +			dev_err(&info->dev, "failed to register simplefb: %s\n",
> > +					strerror(-ret));
> maybe we should return ret here?

I think it's fine to continue without simplefb here. In the end the
framebuffer works, just without simplefb.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH v2 0/6] BCM2835 / simple framebuffer support

[Alexander Aring]

Hi Sascha,

On Wed, Nov 06, 2013 at 10:46:28AM +0100, Sascha Hauer wrote:
> Hi Andre,
> @@ -163,6 +163,13 @@ int register_framebuffer(struct fb_info *info)
>  	if (ret)
>  		goto err_unregister;
>  
> +	if (IS_ENABLED(CONFIG_DRIVER_VIDEO_SIMPLEFB)) {
> +		ret = fb_register_simplefb(info);
> +		if (ret)
> +			dev_err(&info->dev, "failed to register simplefb: %s\n",
> +					strerror(-ret));
maybe we should return ret here?

> +	}
> +
>  	return 0;
>  
>  err_unregister:
> diff --git a/drivers/video/simplefb.c b/drivers/video/simplefb.c

- Alex

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH v2 0/6] BCM2835 / simple framebuffer support

[Alexander Aring]

On Wed, Nov 06, 2013 at 11:02:33AM +0100, Sascha Hauer wrote:
> On Wed, Nov 06, 2013 at 11:04:02AM +0100, Alexander Aring wrote:
> > Hi Sascha,
> > 
> > On Wed, Nov 06, 2013 at 10:46:28AM +0100, Sascha Hauer wrote:
> > > Hi Andre,
> > > @@ -163,6 +163,13 @@ int register_framebuffer(struct fb_info *info)
> > >  	if (ret)
> > >  		goto err_unregister;
> > >  
> > > +	if (IS_ENABLED(CONFIG_DRIVER_VIDEO_SIMPLEFB)) {
> > > +		ret = fb_register_simplefb(info);
> > > +		if (ret)
> > > +			dev_err(&info->dev, "failed to register simplefb: %s\n",
> > > +					strerror(-ret));
> > maybe we should return ret here?
> 
> I think it's fine to continue without simplefb here. In the end the
> framebuffer works, just without simplefb.
> 
ok, thanks.

- Alex

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH v2 0/6] BCM2835 / simple framebuffer support

[Andre Heider]

Hi Sascha,

On Wed, Nov 06, 2013 at 10:46:28AM +0100, Sascha Hauer wrote:
> Hi Andre,
> 
> On Tue, Nov 05, 2013 at 12:00:58AM +0100, Andre Heider wrote:
> > This is v2 of my patch set posted on 10/24.
> > It adds framebuffer support for the RPi and provides an additional config
> > knob to set up the kernel's simplefb driver. The active barebox framebuffer
> > is basis for that feature, so that other platforms can make use of it.
> > 
> > This simplefb option depends on the bcm2835 fb driver since drivers need
> > to be aware of simplefb: framebuffers need to stay configured and cannot be
> > teared down.
> > 
> > The RPi case is rather simple in this regard. The allocated framebuffer lies
> > within the memory range of the VideoCore of the ARM/VC memory split. Hence,
> > there is no memory range that needs to be reserved as far as the ARM side
> > of things is concerned.
> > This likely needs to be added once other fb driver want to use simplefb.
> > 
> > Changes since v1:
> > * renamed "pitch" to "line_length" to match the kernel
> > * the bcm2835 framebuffer driver is now standalone
> > * configuring simplefb for the kernel is based on the active barebox
> >   framebuffer
> 
> I applied this series without the simplefb patch.

thanks!

> 
> For the simplefb I'd like to use the following patch instead. It's your
> patch with some adjustments:
> 
> - add a register_simplefb device parameter to make it configurable
>   whether a simplefb node should be created or not.
> - register the OF fixup in register_framebuffer(). This way we always
>   have a simplefb node once a framebuffer is registered without getting
>   spurious warnings when there is no framebuffer present.
> - check if the framebuffer is actually enabled. If it's not, we
>   shouldn't register a simplefb node.
> 
> Are you fine with this and if yes, could you give the patch a test?
> It depends on the add-context-pointer-to-of_register_fixup patch I just
> posted. Just try the current -next branch, it should contain all you
> need for testing.

Neat, that looks nicer, so no objection here ;)

And yes, it works if I set fb0.enable=1 and fb0.register_simplefb=1,
both are 0 per default.

Now I wonder who/where those are supposed to get set? Is that a job for
the runtime environment?

I didn't really care about the enabled parameter until now, there isn't
anything to do in the fb_enable callback for this fb driver and `splash`
doesn't seem to care either.

Regards,
Andre

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH v2 0/6] BCM2835 / simple framebuffer support

[Andre Heider]

Hi Sascha,

On Wed, Nov 06, 2013 at 06:40:49PM +0100, Andre Heider wrote:
> And yes, it works if I set fb0.enable=1 and fb0.register_simplefb=1,
> both are 0 per default.
> 
> Now I wonder who/where those are supposed to get set? Is that a job for
> the runtime environment?

if possible I'd like to make the fb work out of the box. Setting these
two parameters to default to "1" feels wrong (and I didn't find other
drivers to do so). Any suggestions?

Thanks,
Andre

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH v2 0/6] BCM2835 / simple framebuffer support

[Sascha Hauer]

On Tue, Nov 12, 2013 at 01:14:51PM +0100, Andre Heider wrote:
> Hi Sascha,
> 
> On Wed, Nov 06, 2013 at 06:40:49PM +0100, Andre Heider wrote:
> > And yes, it works if I set fb0.enable=1 and fb0.register_simplefb=1,
> > both are 0 per default.
> > 
> > Now I wonder who/where those are supposed to get set? Is that a job for
> > the runtime environment?
> 
> if possible I'd like to make the fb work out of the box. Setting these
> two parameters to default to "1" feels wrong (and I didn't find other
> drivers to do so). Any suggestions?

Normally the policy is that a framebuffer is not enabled during
registration. This makes sense because normally the framebuffer doesn't
contain a useful picture, so users should set a picture and enable the
framebuffer afterwards to avoid flickering.

In your case the situation is different though. The display hardware is
already enabled when you enter barebox, right? In this case I suggest
that you set info->p_enable to true before calling register_framebuffer
from the bcm2835 driver. This should be the right thing to do since it
only reflects the hardware status.

Also I think you can set info->register_simplefb to true aswell in the
bcm2835 driver since on that platform this is probably a sane default.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH v2 0/6] BCM2835 / simple framebuffer support

[Andre Heider]

On Tue, Nov 12, 2013 at 3:19 PM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> On Tue, Nov 12, 2013 at 01:14:51PM +0100, Andre Heider wrote:
>> Hi Sascha,
>>
>> On Wed, Nov 06, 2013 at 06:40:49PM +0100, Andre Heider wrote:
>> > And yes, it works if I set fb0.enable=1 and fb0.register_simplefb=1,
>> > both are 0 per default.
>> >
>> > Now I wonder who/where those are supposed to get set? Is that a job for
>> > the runtime environment?
>>
>> if possible I'd like to make the fb work out of the box. Setting these
>> two parameters to default to "1" feels wrong (and I didn't find other
>> drivers to do so). Any suggestions?
>
> Normally the policy is that a framebuffer is not enabled during
> registration. This makes sense because normally the framebuffer doesn't
> contain a useful picture, so users should set a picture and enable the
> framebuffer afterwards to avoid flickering.
>
> In your case the situation is different though. The display hardware is
> already enabled when you enter barebox, right?

Right.

> In this case I suggest
> that you set info->p_enable to true before calling register_framebuffer
> from the bcm2835 driver. This should be the right thing to do since it
> only reflects the hardware status.
>
> Also I think you can set info->register_simplefb to true aswell in the
> bcm2835 driver since on that platform this is probably a sane default.

Ok, easy enough ;)

Thanks,
Andre

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox