Mailing List Archive

[PATCH v3 8/8] xen/arm: introduce a driver for the ARM HDLCD controller
Read the screen resolution setting from device tree, find the
corresponding modeline in a small table of standard video modes, set the
hardware accordingly.

Use vexpress_syscfg to configure the pixel clock.

Use the generic framebuffer functions to print on the screen.

Changes in v2:
- read mode from DT;
- support multiple resolutions;
- use vexpress_syscfg to set the pixclock.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
xen/arch/arm/Rules.mk | 1 +
xen/drivers/video/Makefile | 1 +
xen/drivers/video/arm_hdlcd.c | 282 +++++++++++++++++++++++++++++++++++++++++
xen/drivers/video/modelines.h | 69 ++++++++++
xen/include/asm-arm/config.h | 2 +
5 files changed, 355 insertions(+), 0 deletions(-)
create mode 100644 xen/drivers/video/arm_hdlcd.c
create mode 100644 xen/drivers/video/modelines.h

diff --git a/xen/arch/arm/Rules.mk b/xen/arch/arm/Rules.mk
index fa9f9c1..9580e6b 100644
--- a/xen/arch/arm/Rules.mk
+++ b/xen/arch/arm/Rules.mk
@@ -8,6 +8,7 @@

HAS_DEVICE_TREE := y
HAS_VIDEO := y
+HAS_ARM_HDLCD := y

CFLAGS += -fno-builtin -fno-common -Wredundant-decls
CFLAGS += -iwithprefix include -Werror -Wno-pointer-arith -pipe
diff --git a/xen/drivers/video/Makefile b/xen/drivers/video/Makefile
index 3b3eb43..8a6f5da 100644
--- a/xen/drivers/video/Makefile
+++ b/xen/drivers/video/Makefile
@@ -4,3 +4,4 @@ obj-$(HAS_VIDEO) += font_8x16.o
obj-$(HAS_VIDEO) += font_8x8.o
obj-$(HAS_VIDEO) += fb.o
obj-$(HAS_VGA) += vesa.o
+obj-$(HAS_ARM_HDLCD) += arm_hdlcd.o
diff --git a/xen/drivers/video/arm_hdlcd.c b/xen/drivers/video/arm_hdlcd.c
new file mode 100644
index 0000000..9e69856
--- /dev/null
+++ b/xen/drivers/video/arm_hdlcd.c
@@ -0,0 +1,282 @@
+/*
+ * xen/drivers/video/arm_hdlcd.c
+ *
+ * Driver for ARM HDLCD Controller
+ *
+ * Stefano Stabellini <stefano.stabellini@eu.citrix.com>
+ * Copyright (c) 2012 Citrix Systems.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <asm/delay.h>
+#include <asm/types.h>
+#include <asm/platform_vexpress.h>
+#include <xen/config.h>
+#include <xen/device_tree.h>
+#include <xen/libfdt/libfdt.h>
+#include <xen/init.h>
+#include <xen/mm.h>
+#include "font.h"
+#include "fb.h"
+#include "modelines.h"
+
+#define HDLCD ((volatile uint32_t *) FIXMAP_ADDR(FIXMAP_MISC))
+
+#define HDLCD_INTMASK (0x18/4)
+#define HDLCD_FBBASE (0x100/4)
+#define HDLCD_LINELENGTH (0x104/4)
+#define HDLCD_LINECOUNT (0x108/4)
+#define HDLCD_LINEPITCH (0x10C/4)
+#define HDLCD_BUS (0x110/4)
+#define HDLCD_VSYNC (0x200/4)
+#define HDLCD_VBACK (0x204/4)
+#define HDLCD_VDATA (0x208/4)
+#define HDLCD_VFRONT (0x20C/4)
+#define HDLCD_HSYNC (0x210/4)
+#define HDLCD_HBACK (0x214/4)
+#define HDLCD_HDATA (0x218/4)
+#define HDLCD_HFRONT (0x21C/4)
+#define HDLCD_POLARITIES (0x220/4)
+#define HDLCD_COMMAND (0x230/4)
+#define HDLCD_PF (0x240/4)
+#define HDLCD_RED (0x244/4)
+#define HDLCD_GREEN (0x248/4)
+#define HDLCD_BLUE (0x24C/4)
+
+static void vga_noop_puts(const char *s) {}
+void (*video_puts)(const char *) = vga_noop_puts;
+
+static void hdlcd_flush(void)
+{
+ dsb();
+}
+
+static void set_color_masks(int bpp,
+ int *red_shift, int *green_shift, int *blue_shift,
+ int *red_size, int *green_size, int *blue_size)
+{
+ switch (bpp) {
+ case 2:
+ *red_shift = 0;
+ *green_shift = 5;
+ *blue_shift = 11;
+ *red_size = 5;
+ *green_size = 6;
+ *blue_size = 5;
+ break;
+ case 3:
+ case 4:
+ *red_shift = 0;
+ *green_shift = 8;
+ *blue_shift = 16;
+ *red_size = 8;
+ *green_size = 8;
+ *blue_size = 8;
+ break;
+ default:
+ BUG();
+ break;
+ }
+}
+
+static void set_pixclock(uint32_t pixclock)
+{
+ vexpress_syscfg(1, V2M_SYS_CFG_OSC_FUNC, V2M_SYS_CFG_OSC5, &pixclock);
+}
+
+void __init video_init(void)
+{
+ int node, depth;
+ u32 address_cells, size_cells;
+ struct fb_prop fbp;
+ unsigned char *lfb;
+ paddr_t hdlcd_start, hdlcd_size;
+ paddr_t framebuffer_start, framebuffer_size;
+ const struct fdt_property *prop;
+ const u32 *cell;
+ const char *mode_string;
+ char _mode_string[16];
+ int bpp;
+ int red_shift, green_shift, blue_shift;
+ int red_size, green_size, blue_size;
+ struct modeline *videomode = NULL;
+ int i;
+
+ if ( find_compatible_node("arm,hdlcd", &node, &depth,
+ &address_cells, &size_cells) <= 0 )
+ return;
+
+ prop = fdt_get_property(device_tree_flattened, node, "reg", NULL);
+ if ( !prop )
+ return;
+
+ cell = (const u32 *)prop->data;
+ device_tree_get_reg(&cell, address_cells, size_cells,
+ &hdlcd_start, &hdlcd_size);
+
+ prop = fdt_get_property(device_tree_flattened, node, "framebuffer", NULL);
+ if ( !prop )
+ return;
+
+ cell = (const u32 *)prop->data;
+ device_tree_get_reg(&cell, address_cells, size_cells,
+ &framebuffer_start, &framebuffer_size);
+
+ mode_string = fdt_getprop(device_tree_flattened, node, "mode", NULL);
+ if ( !mode_string )
+ {
+ bpp = 4;
+ set_color_masks(bpp, &red_shift, &green_shift, &blue_shift,
+ &red_size, &green_size, &blue_size);
+ memcpy(_mode_string, "1280x1024@60", strlen("1280x1024@60"));
+ }
+ if ( strlen(mode_string) < strlen("800x600@60") )
+ {
+ printk("HDLCD: invalid modeline=%s\n", mode_string);
+ return;
+ } else {
+ char *s = strchr(mode_string, '-');
+ if ( !s )
+ {
+ printk("HDLCD: bpp not found in modeline %s, assume 32 bpp\n",
+ mode_string);
+ bpp = 4;
+ set_color_masks(bpp, &red_shift, &green_shift, &blue_shift,
+ &red_size, &green_size, &blue_size);
+ memcpy(_mode_string, mode_string, strlen(mode_string) + 1);
+ } else {
+ if ( strlen(s) < 6 )
+ {
+ printk("HDLCD: invalid mode %s\n", mode_string);
+ return;
+ }
+ s++;
+ if ( !strncmp(s, "16", 2) )
+ {
+ bpp = 2;
+ set_color_masks(bpp, &red_shift, &green_shift, &blue_shift,
+ &red_size, &green_size, &blue_size);
+ }
+ else if ( !strncmp(s, "24", 2) )
+ {
+ bpp = 3;
+ set_color_masks(bpp, &red_shift, &green_shift, &blue_shift,
+ &red_size, &green_size, &blue_size);
+ }
+ else if ( !strncmp(s, "32", 2) )
+ {
+ bpp = 4;
+ set_color_masks(bpp, &red_shift, &green_shift, &blue_shift,
+ &red_size, &green_size, &blue_size);
+ } else {
+ printk("HDLCD: unsupported bpp %s\n", s);
+ return;
+ }
+ i = s - mode_string - 1;
+ memcpy(_mode_string, mode_string, i);
+ memcpy(_mode_string + i, mode_string + i + 3, 4);
+ }
+ }
+
+ for ( i = 0; i < ARRAY_SIZE(videomodes); i++ )
+ {
+ if ( !strcmp(_mode_string, videomodes[i].mode) )
+ {
+ videomode = &videomodes[i];
+ break;
+ }
+ }
+ if ( !videomode )
+ {
+ printk("HDLCD: unsupported videomode %s\n", _mode_string);
+ return;
+ }
+
+
+ if ( !hdlcd_start || !framebuffer_start )
+ return;
+
+ if ( framebuffer_size < bpp * videomode->xres * videomode->yres )
+ {
+ printk("HDLCD: the framebuffer is too small, disable the HDLCD driver\n");
+ return;
+ }
+
+ printk("Initializing HDLCD driver\n");
+
+ lfb = early_ioremap(framebuffer_start, framebuffer_size, DEV_WC);
+ if ( !lfb )
+ {
+ printk("Couldn't map the framebuffer\n");
+ return;
+ }
+ memset(lfb, 0x00, bpp * videomode->xres * videomode->yres);
+
+ /* uses FIXMAP_MISC */
+ set_pixclock(videomode->pixclock);
+
+ set_fixmap(FIXMAP_MISC, hdlcd_start >> PAGE_SHIFT, DEV_SHARED);
+ HDLCD[HDLCD_COMMAND] = 0;
+
+ HDLCD[HDLCD_LINELENGTH] = videomode->xres * bpp;
+ HDLCD[HDLCD_LINECOUNT] = videomode->yres - 1;
+ HDLCD[HDLCD_LINEPITCH] = videomode->xres * bpp;
+ HDLCD[HDLCD_PF] = ((bpp - 1) << 3);
+ HDLCD[HDLCD_INTMASK] = 0;
+ HDLCD[HDLCD_FBBASE] = framebuffer_start;
+ HDLCD[HDLCD_BUS] = 0xf00 | (1 << 4);
+ HDLCD[HDLCD_VBACK] = videomode->vback - 1;
+ HDLCD[HDLCD_VSYNC] = videomode->vsync - 1;
+ HDLCD[HDLCD_VDATA] = videomode->yres - 1;
+ HDLCD[HDLCD_VFRONT] = videomode->vfront - 1;
+ HDLCD[HDLCD_HBACK] = videomode->hback - 1;
+ HDLCD[HDLCD_HSYNC] = videomode->hsync - 1;
+ HDLCD[HDLCD_HDATA] = videomode->xres - 1;
+ HDLCD[HDLCD_HFRONT] = videomode->hfront - 1;
+ HDLCD[HDLCD_POLARITIES] = (1 << 2) | (1 << 3);
+ HDLCD[HDLCD_RED] = (red_size << 8) | red_shift;
+ HDLCD[HDLCD_GREEN] = (green_size << 8) | green_shift;
+ HDLCD[HDLCD_BLUE] = (blue_size << 8) | blue_shift;
+ HDLCD[HDLCD_COMMAND] = 1;
+ clear_fixmap(FIXMAP_MISC);
+
+ fbp.pixel_on = (((1 << red_size) - 1) << red_shift) |
+ (((1 << green_size) - 1) << green_shift) |
+ (((1 << blue_size) - 1) << blue_shift);
+ fbp.lfb = lfb;
+ fbp.font = &font_vga_8x16;
+ fbp.bits_per_pixel = bpp*8;
+ fbp.bytes_per_line = bpp*videomode->xres;
+ fbp.width = videomode->xres;
+ fbp.height = videomode->yres;
+ fbp.flush = hdlcd_flush;
+ fbp.text_columns = videomode->xres / 8;
+ fbp.text_rows = videomode->yres / 16;
+ if ( fb_init(fbp) < 0 )
+ return;
+ video_puts = fb_scroll_puts;
+}
+
+void video_endboot(void)
+{
+ if ( video_puts != vga_noop_puts )
+ fb_alloc();
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-set-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/drivers/video/modelines.h b/xen/drivers/video/modelines.h
new file mode 100644
index 0000000..b91368d
--- /dev/null
+++ b/xen/drivers/video/modelines.h
@@ -0,0 +1,69 @@
+/*
+ * xen/drivers/video/modelines.h
+ *
+ * Timings for many popular monitor resolutions
+ *
+ * Stefano Stabellini <stefano.stabellini@eu.citrix.com>
+ * Copyright (c) 2012 Citrix Systems.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef _XEN_MODLINES_H
+#define _XEN_MODLINES_H
+
+struct modeline {
+ const char* mode; /* in the form 1280x1024@60 */
+ uint32_t pixclock; /* Khz */
+ uint32_t xres;
+ uint32_t hfront; /* horizontal front porch in pixels */
+ uint32_t hsync; /* horizontal sync pulse in pixels */
+ uint32_t hback; /* horizontal back porch in pixels */
+ uint32_t yres;
+ uint32_t vfront; /* vertical front porch in lines */
+ uint32_t vsync; /* vertical sync pulse in lines */
+ uint32_t vback; /* vertical back porch in lines */
+};
+
+struct modeline __initdata videomodes[] = {
+ { "640x480@60", 25175, 640, 16, 96, 48, 480, 11, 2, 31 },
+ { "640x480@72", 31500, 640, 24, 40, 128, 480, 9, 3, 28 },
+ { "640x480@75", 31500, 640, 16, 96, 48, 480, 11, 2, 32 },
+ { "640x480@85", 36000, 640, 32, 48, 112, 480, 1, 3, 25 },
+ { "800x600@56", 38100, 800, 32, 128, 128, 600, 1, 4, 14 },
+ { "800x600@60", 40000, 800, 40, 128, 88 , 600, 1, 4, 23 },
+ { "800x600@72", 50000, 800, 56, 120, 64 , 600, 37, 6, 23 },
+ { "800x600@75", 49500, 800, 16, 80, 160, 600, 1, 2, 21 },
+ { "800x600@85", 56250, 800, 32, 64, 152, 600, 1, 3, 27 },
+ { "1024x768@60", 65000, 1024, 24, 136, 160, 768, 3, 6, 29 },
+ { "1024x768@70", 75000, 1024, 24, 136, 144, 768, 3, 6, 29 },
+ { "1024x768@75", 78750, 1024, 16, 96, 176, 768, 1, 3, 28 },
+ { "1024x768@85", 94500, 1024, 48, 96, 208, 768, 1, 3, 36 },
+ { "1280x1024@60", 108000, 1280, 48, 112, 248, 1024, 1, 3, 38 },
+ { "1280x1024@75", 135000, 1280, 16, 144, 248, 1024, 1, 3, 38 },
+ { "1280x1024@85", 157500, 1280, 64, 160, 224, 1024, 1, 3, 44 },
+ { "1400x1050@60", 122610, 1400, 88, 152, 240, 1050, 1, 3, 33 },
+ { "1400x1050@75", 155850, 1400, 96, 152, 248, 1050, 1, 3, 42 },
+ { "1600x1200@60", 162000, 1600, 64, 192, 304, 1200, 1, 3, 46 },
+ { "1600x1200@65", 175500, 1600, 64, 192, 304, 1200, 1, 3, 46 },
+ { "1600x1200@70", 189000, 1600, 64, 192, 304, 1200, 1, 3, 46 },
+ { "1600x1200@75", 202500, 1600, 64, 192, 304, 1200, 1, 3, 46 },
+ { "1600x1200@85", 229500, 1600, 64, 192, 304, 1200, 1, 3, 46 },
+ { "1792x1344@60", 204800, 1792, 128, 200, 328, 1344, 1, 3, 46 },
+ { "1792x1344@75", 261000, 1792, 96, 216, 352, 1344, 1, 3, 69 },
+ { "1856x1392@60", 218300, 1856, 96, 224, 352, 1392, 1, 3, 43 },
+ { "1856x1392@75", 288000, 1856, 128, 224, 352, 1392, 1, 3, 104 },
+ { "1920x1200@75", 193160, 1920, 128, 208, 336, 1200, 1, 3, 38 },
+ { "1920x1440@60", 234000, 1920, 128, 208, 344, 1440, 1, 3, 56 },
+ { "1920x1440@75", 297000, 1920, 144, 224, 352, 1440, 1, 3, 56 },
+};
+
+#endif
diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h
index 87db0d1..d8aa66b 100644
--- a/xen/include/asm-arm/config.h
+++ b/xen/include/asm-arm/config.h
@@ -19,6 +19,8 @@

#define CONFIG_DOMAIN_PAGE 1

+#define CONFIG_VIDEO 1
+
#define OPT_CONSOLE_STR "com1"

#ifdef MAX_PHYS_CPUS
--
1.7.2.5


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [PATCH v3 8/8] xen/arm: introduce a driver for the ARM HDLCD controller [ In reply to ]
On Tue, 2012-12-18 at 18:46 +0000, Stefano Stabellini wrote:
> +static void set_color_masks(int bpp,
> + int *red_shift, int *green_shift, int *blue_shift,
> + int *red_size, int *green_size, int *blue_size)

This is crying out for a pointer to a struct.

> +{
> + switch (bpp) {
> + case 2:
> + *red_shift = 0;
> + *green_shift = 5;
> + *blue_shift = 11;
> + *red_size = 5;
> + *green_size = 6;
> + *blue_size = 5;
> + break;
> + case 3:
> + case 4:
> + *red_shift = 0;
> + *green_shift = 8;
> + *blue_shift = 16;
> + *red_size = 8;
> + *green_size = 8;
> + *blue_size = 8;
> + break;
> + default:
> + BUG();
> + break;
> + }
> +}
> +
> +static void set_pixclock(uint32_t pixclock)
> +{

Doesn't there need to be some sort of "are we running on a vexpress"
check here? e.g. a DTB compatibility node check or something?

> + vexpress_syscfg(1, V2M_SYS_CFG_OSC_FUNC, V2M_SYS_CFG_OSC5, &pixclock);
> +}
> +
> +void __init video_init(void)
> +{
> + int node, depth;
> + u32 address_cells, size_cells;
> + struct fb_prop fbp;
> + unsigned char *lfb;
> + paddr_t hdlcd_start, hdlcd_size;
> + paddr_t framebuffer_start, framebuffer_size;
> + const struct fdt_property *prop;
> + const u32 *cell;
> + const char *mode_string;
> + char _mode_string[16];
> + int bpp;
> + int red_shift, green_shift, blue_shift;
> + int red_size, green_size, blue_size;
> + struct modeline *videomode = NULL;
> + int i;
> +
> + if ( find_compatible_node("arm,hdlcd", &node, &depth,
> + &address_cells, &size_cells) <= 0 )
> + return;
> +
> + prop = fdt_get_property(device_tree_flattened, node, "reg", NULL);
> + if ( !prop )
> + return;
> +
> + cell = (const u32 *)prop->data;
> + device_tree_get_reg(&cell, address_cells, size_cells,
> + &hdlcd_start, &hdlcd_size);
> +
> + prop = fdt_get_property(device_tree_flattened, node, "framebuffer", NULL);
> + if ( !prop )
> + return;
> +
> + cell = (const u32 *)prop->data;
> + device_tree_get_reg(&cell, address_cells, size_cells,
> + &framebuffer_start, &framebuffer_size);
> +
> + mode_string = fdt_getprop(device_tree_flattened, node, "mode", NULL);
> + if ( !mode_string )
> + {
> + bpp = 4;
> + set_color_masks(bpp, &red_shift, &green_shift, &blue_shift,
> + &red_size, &green_size, &blue_size);
> + memcpy(_mode_string, "1280x1024@60", strlen("1280x1024@60"));

What associates mode_string with this _mode_string?

> + }

Or should there be an else here? Otherwise can't mode_string be NULL?

> + if ( strlen(mode_string) < strlen("800x600@60") )
> + {
> + printk("HDLCD: invalid modeline=%s\n", mode_string);
> + return;
> + } else {
> + char *s = strchr(mode_string, '-');
> + if ( !s )
> + {
> + printk("HDLCD: bpp not found in modeline %s, assume 32 bpp\n",
> + mode_string);
> + bpp = 4;
> + set_color_masks(bpp, &red_shift, &green_shift, &blue_shift,
> + &red_size, &green_size, &blue_size);
> + memcpy(_mode_string, mode_string, strlen(mode_string) + 1);

What if strlen(mode_string)+1 > 16 ?

> + } else {
> + if ( strlen(s) < 6 )
> + {
> + printk("HDLCD: invalid mode %s\n", mode_string);
> + return;
> + }

Indentation

> + s++;
> + if ( !strncmp(s, "16", 2) )
> + {
> + bpp = 2;
> + set_color_masks(bpp, &red_shift, &green_shift, &blue_shift,
> + &red_size, &green_size, &blue_size);
> + }
> + else if ( !strncmp(s, "24", 2) )
> + {
> + bpp = 3;
> + set_color_masks(bpp, &red_shift, &green_shift, &blue_shift,
> + &red_size, &green_size, &blue_size);
> + }
> + else if ( !strncmp(s, "32", 2) )
> + {
> + bpp = 4;
> + set_color_masks(bpp, &red_shift, &green_shift, &blue_shift,
> + &red_size, &green_size, &blue_size);

This all smells like a lookup table to me.

> + } else {

extra space here.

> + printk("HDLCD: unsupported bpp %s\n", s);
> + return;
> + }
> + i = s - mode_string - 1;
> + memcpy(_mode_string, mode_string, i);
> + memcpy(_mode_string + i, mode_string + i + 3, 4);
> + }
> + }
> +
> + for ( i = 0; i < ARRAY_SIZE(videomodes); i++ )
> + {
> + if ( !strcmp(_mode_string, videomodes[i].mode) )
> + {
> + videomode = &videomodes[i];
> + break;
> + }
> + }
> + if ( !videomode )
> + {
> + printk("HDLCD: unsupported videomode %s\n", _mode_string);
> + return;
> + }
> +
> +
> + if ( !hdlcd_start || !framebuffer_start )

Worth a message? Also couldn't you have checked this much earlier
(before the mode parsing stuff).

> + return;
> +
> + if ( framebuffer_size < bpp * videomode->xres * videomode->yres )
> + {
> + printk("HDLCD: the framebuffer is too small, disable the HDLCD driver\n");

"disable" or "disabling" ?

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [PATCH v3 8/8] xen/arm: introduce a driver for the ARM HDLCD controller [ In reply to ]
On Wed, 19 Dec 2012, Ian Campbell wrote:
> On Tue, 2012-12-18 at 18:46 +0000, Stefano Stabellini wrote:
> > +static void set_color_masks(int bpp,
> > + int *red_shift, int *green_shift, int *blue_shift,
> > + int *red_size, int *green_size, int *blue_size)
>
> This is crying out for a pointer to a struct.

OK


> > +{
> > + switch (bpp) {
> > + case 2:
> > + *red_shift = 0;
> > + *green_shift = 5;
> > + *blue_shift = 11;
> > + *red_size = 5;
> > + *green_size = 6;
> > + *blue_size = 5;
> > + break;
> > + case 3:
> > + case 4:
> > + *red_shift = 0;
> > + *green_shift = 8;
> > + *blue_shift = 16;
> > + *red_size = 8;
> > + *green_size = 8;
> > + *blue_size = 8;
> > + break;
> > + default:
> > + BUG();
> > + break;
> > + }
> > +}
> > +
> > +static void set_pixclock(uint32_t pixclock)
> > +{
>
> Doesn't there need to be some sort of "are we running on a vexpress"
> check here? e.g. a DTB compatibility node check or something?

Yes. TBH the ideal thing to have would be a set of generic platform
functions that get initialized early in start_xen and then used lated
on; but considering that at the moment we only have one platform, it is
even difficult to figure out what functions we would need to export.


> > + vexpress_syscfg(1, V2M_SYS_CFG_OSC_FUNC, V2M_SYS_CFG_OSC5, &pixclock);
> > +}
> > +
> > +void __init video_init(void)
> > +{
> > + int node, depth;
> > + u32 address_cells, size_cells;
> > + struct fb_prop fbp;
> > + unsigned char *lfb;
> > + paddr_t hdlcd_start, hdlcd_size;
> > + paddr_t framebuffer_start, framebuffer_size;
> > + const struct fdt_property *prop;
> > + const u32 *cell;
> > + const char *mode_string;
> > + char _mode_string[16];
> > + int bpp;
> > + int red_shift, green_shift, blue_shift;
> > + int red_size, green_size, blue_size;
> > + struct modeline *videomode = NULL;
> > + int i;
> > +
> > + if ( find_compatible_node("arm,hdlcd", &node, &depth,
> > + &address_cells, &size_cells) <= 0 )
> > + return;
> > +
> > + prop = fdt_get_property(device_tree_flattened, node, "reg", NULL);
> > + if ( !prop )
> > + return;
> > +
> > + cell = (const u32 *)prop->data;
> > + device_tree_get_reg(&cell, address_cells, size_cells,
> > + &hdlcd_start, &hdlcd_size);
> > +
> > + prop = fdt_get_property(device_tree_flattened, node, "framebuffer", NULL);
> > + if ( !prop )
> > + return;
> > +
> > + cell = (const u32 *)prop->data;
> > + device_tree_get_reg(&cell, address_cells, size_cells,
> > + &framebuffer_start, &framebuffer_size);
> > +
> > + mode_string = fdt_getprop(device_tree_flattened, node, "mode", NULL);
> > + if ( !mode_string )
> > + {
> > + bpp = 4;
> > + set_color_masks(bpp, &red_shift, &green_shift, &blue_shift,
> > + &red_size, &green_size, &blue_size);
> > + memcpy(_mode_string, "1280x1024@60", strlen("1280x1024@60"));
>
> What associates mode_string with this _mode_string?

_mode_string is RW while mode_string is RO.
This particular mode string (1280x1024@60) is chosen arbitrarily.


> > + if ( strlen(mode_string) < strlen("800x600@60") )
> > + {
> > + printk("HDLCD: invalid modeline=%s\n", mode_string);
> > + return;
> > + } else {
> > + char *s = strchr(mode_string, '-');
> > + if ( !s )
> > + {
> > + printk("HDLCD: bpp not found in modeline %s, assume 32 bpp\n",
> > + mode_string);
> > + bpp = 4;
> > + set_color_masks(bpp, &red_shift, &green_shift, &blue_shift,
> > + &red_size, &green_size, &blue_size);
> > + memcpy(_mode_string, mode_string, strlen(mode_string) + 1);
>
> What if strlen(mode_string)+1 > 16 ?

in theory it can't be if it follows the DT spec, but I'll add a check
for it

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel