[PATCH] mx35_3ds: Add the TFT display to the board file

[PATCH] mx35_3ds: Add the TFT display to the board file

[Michael Grzeschik]

From: Juergen Beisert <jbe@pengutronix.de>

Add the personality board's display to the board file. Used display is:

Manufacturer: CTP
Type: CLAA070LC0ACW

This is a 800 x 480 wide display with 27 MHz pixel clock and 60 Hz refresh
rate with 18 bit data interface. Sync is based on DE only.

Signed-off-by: Juergen Beisert <jbe@pengutronix.de>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
The power for the display is driven by the mc9s08.

Since patch 3aa519797e79ed4d85cd6604cdf642f99d4bb2ef in the barebox
bootloader we toggle the power to the LCD by software.

 arch/arm/mach-mx3/mach-mx35_3ds.c |   72 +++++++++++++++++++++++++++++++++++++
 1 files changed, 72 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-mx3/mach-mx35_3ds.c b/arch/arm/mach-mx3/mach-mx35_3ds.c
index 1dd7baa..2e7468e 100644
--- a/arch/arm/mach-mx3/mach-mx35_3ds.c
+++ b/arch/arm/mach-mx3/mach-mx35_3ds.c
@@ -39,10 +39,44 @@
 #include <mach/common.h>
 #include <mach/iomux-mx35.h>
 #include <mach/mxc_ehci.h>
+#include <mach/ipu.h>
+#include <mach/mx3fb.h>
+#include <mach/irqs.h>
 
 #include "devices-imx35.h"
 #include "devices.h"
 
+static const struct fb_videomode fb_personality[] = {
+	{
+		/* 800x480 @ 60 Hz */
+		.name		= "CTP-CLAA070LC0ACW",
+		.refresh	= 60,
+		.xres		= 800,
+		.yres		= 480,
+		.pixclock	= KHZ2PICOS(27000),
+		.left_margin	= 50,
+		.right_margin	= 50,	/* whole line should have 900 clocks */
+		.upper_margin	= 10,
+		.lower_margin	= 10,	/* whole frame should have 500 lines */
+		.hsync_len	= 1,	/* note: DE only display */
+		.vsync_len	= 1,	/* note: DE only display */
+		.sync		= FB_SYNC_CLK_IDLE_EN,
+		.vmode		= FB_VMODE_NONINTERLACED,
+		.flag		= 0,
+	},
+};
+
+static struct ipu_platform_data mx3_ipu_data = {
+	.irq_base = MXC_IPU_IRQ_START,
+};
+
+static struct mx3fb_platform_data mx3fb_pdata = {
+	.dma_dev	= &mx3_ipu.dev,
+	.name		= "CTP-CLAA070LC0ACW",
+	.mode		= fb_personality,
+	.num_modes	= ARRAY_SIZE(fb_personality),
+};
+
 static const struct imxuart_platform_data uart_pdata __initconst = {
 	.flags = IMXUART_HAVE_RTSCTS,
 };
@@ -109,6 +143,41 @@ static struct pad_desc mx35pdk_pads[] = {
 	/* USBH1 */
 	MX35_PAD_I2C2_CLK__USB_TOP_USBH2_PWR,
 	MX35_PAD_I2C2_DAT__USB_TOP_USBH2_OC,
+	/* Display */
+	MX35_PAD_LD0__IPU_DISPB_DAT_0,
+	MX35_PAD_LD1__IPU_DISPB_DAT_1,
+	MX35_PAD_LD2__IPU_DISPB_DAT_2,
+	MX35_PAD_LD3__IPU_DISPB_DAT_3,
+	MX35_PAD_LD4__IPU_DISPB_DAT_4,
+	MX35_PAD_LD5__IPU_DISPB_DAT_5,
+	MX35_PAD_LD6__IPU_DISPB_DAT_6,
+	MX35_PAD_LD7__IPU_DISPB_DAT_7,
+	MX35_PAD_LD8__IPU_DISPB_DAT_8,
+	MX35_PAD_LD9__IPU_DISPB_DAT_9,
+	MX35_PAD_LD10__IPU_DISPB_DAT_10,
+	MX35_PAD_LD11__IPU_DISPB_DAT_11,
+	MX35_PAD_LD12__IPU_DISPB_DAT_12,
+	MX35_PAD_LD13__IPU_DISPB_DAT_13,
+	MX35_PAD_LD14__IPU_DISPB_DAT_14,
+	MX35_PAD_LD15__IPU_DISPB_DAT_15,
+	MX35_PAD_LD16__IPU_DISPB_DAT_16,
+	MX35_PAD_LD17__IPU_DISPB_DAT_17,
+#if 0
+	/* 24 bit display (for the records) */
+	MX35_PAD_LD18__IPU_DISPB_DAT_18,
+	MX35_PAD_LD19__IPU_DISPB_DAT_19,
+	MX35_PAD_LD20__IPU_DISPB_DAT_20,
+	MX35_PAD_LD21__IPU_DISPB_DAT_21,
+	MX35_PAD_LD22__IPU_DISPB_DAT_22,
+	MX35_PAD_LD23__IPU_DISPB_DAT_23,
+#endif
+	MX35_PAD_D3_HSYNC__IPU_DISPB_D3_HSYNC,	/* not used with the CTP display */
+	MX35_PAD_D3_FPSHIFT__IPU_DISPB_D3_CLK,
+	MX35_PAD_D3_DRDY__IPU_DISPB_D3_DRDY,
+	MX35_PAD_CONTRAST__IPU_DISPB_CONTR,
+	MX35_PAD_D3_VSYNC__IPU_DISPB_D3_VSYNC,	/* not used with the CTP display */
+	MX35_PAD_D3_REV__IPU_DISPB_D3_REV,
+	MX35_PAD_D3_CLS__IPU_DISPB_D3_CLS,
 };
 
 /* OTG config */
@@ -140,6 +209,9 @@ static void __init mxc_board_init(void)
 	mxc_register_device(&mxc_usbh1, &usb_host_pdata);
 
 	imx35_add_mxc_nand(&mx35pdk_nand_board_info);
+
+	mxc_register_device(&mx3_ipu, &mx3_ipu_data);
+	mxc_register_device(&mx3_fb, &mx3fb_pdata);
 }
 
 static void __init mx35pdk_timer_init(void)
-- 
1.7.1

[PATCH v2] mx35_3ds: Add the TFT display to the board file

[Michael Grzeschik]

From: Juergen Beisert <jbe@pengutronix.de>

Add the personality board's display to the board file. Used display is:

Manufacturer: CTP
Type: CLAA070LC0ACW

This is a 800 x 480 wide display with 27 MHz pixel clock and 60 Hz refresh
rate with 18 bit data interface. Sync is based on DE only.

Signed-off-by: Juergen Beisert <jbe@pengutronix.de>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
v1 -> v2
	* removed superfluous comments
	* added S-o-b

The power for the display is driven by the mc9s08.

Since patch 3aa519797e79ed4d85cd6604cdf642f99d4bb2ef in the barebox
bootloader we toggle the power to the LCD by software.

 arch/arm/mach-mx3/mach-mx35_3ds.c |   69 +++++++++++++++++++++++++++++++++++++
 1 files changed, 69 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-mx3/mach-mx35_3ds.c b/arch/arm/mach-mx3/mach-mx35_3ds.c
index 1dd7baa..60d5753 100644
--- a/arch/arm/mach-mx3/mach-mx35_3ds.c
+++ b/arch/arm/mach-mx3/mach-mx35_3ds.c
@@ -39,10 +39,44 @@
 #include <mach/common.h>
 #include <mach/iomux-mx35.h>
 #include <mach/mxc_ehci.h>
+#include <mach/ipu.h>
+#include <mach/mx3fb.h>
+#include <mach/irqs.h>
 
 #include "devices-imx35.h"
 #include "devices.h"
 
+static const struct fb_videomode fb_personality[] = {
+	{
+		/* 800x480 @ 60 Hz */
+		.name		= "CTP-CLAA070LC0ACW",
+		.refresh	= 60,
+		.xres		= 800,
+		.yres		= 480,
+		.pixclock	= KHZ2PICOS(27000),
+		.left_margin	= 50,
+		.right_margin	= 50,	/* whole line should have 900 clocks */
+		.upper_margin	= 10,
+		.lower_margin	= 10,	/* whole frame should have 500 lines */
+		.hsync_len	= 1,	/* note: DE only display */
+		.vsync_len	= 1,	/* note: DE only display */
+		.sync		= FB_SYNC_CLK_IDLE_EN,
+		.vmode		= FB_VMODE_NONINTERLACED,
+		.flag		= 0,
+	},
+};
+
+static struct ipu_platform_data mx3_ipu_data = {
+	.irq_base = MXC_IPU_IRQ_START,
+};
+
+static struct mx3fb_platform_data mx3fb_pdata = {
+	.dma_dev	= &mx3_ipu.dev,
+	.name		= "CTP-CLAA070LC0ACW",
+	.mode		= fb_personality,
+	.num_modes	= ARRAY_SIZE(fb_personality),
+};
+
 static const struct imxuart_platform_data uart_pdata __initconst = {
 	.flags = IMXUART_HAVE_RTSCTS,
 };
@@ -109,6 +143,38 @@ static struct pad_desc mx35pdk_pads[] = {
 	/* USBH1 */
 	MX35_PAD_I2C2_CLK__USB_TOP_USBH2_PWR,
 	MX35_PAD_I2C2_DAT__USB_TOP_USBH2_OC,
+	/* Display */
+	MX35_PAD_LD0__IPU_DISPB_DAT_0,
+	MX35_PAD_LD1__IPU_DISPB_DAT_1,
+	MX35_PAD_LD2__IPU_DISPB_DAT_2,
+	MX35_PAD_LD3__IPU_DISPB_DAT_3,
+	MX35_PAD_LD4__IPU_DISPB_DAT_4,
+	MX35_PAD_LD5__IPU_DISPB_DAT_5,
+	MX35_PAD_LD6__IPU_DISPB_DAT_6,
+	MX35_PAD_LD7__IPU_DISPB_DAT_7,
+	MX35_PAD_LD8__IPU_DISPB_DAT_8,
+	MX35_PAD_LD9__IPU_DISPB_DAT_9,
+	MX35_PAD_LD10__IPU_DISPB_DAT_10,
+	MX35_PAD_LD11__IPU_DISPB_DAT_11,
+	MX35_PAD_LD12__IPU_DISPB_DAT_12,
+	MX35_PAD_LD13__IPU_DISPB_DAT_13,
+	MX35_PAD_LD14__IPU_DISPB_DAT_14,
+	MX35_PAD_LD15__IPU_DISPB_DAT_15,
+	MX35_PAD_LD16__IPU_DISPB_DAT_16,
+	MX35_PAD_LD17__IPU_DISPB_DAT_17,
+	MX35_PAD_LD18__IPU_DISPB_DAT_18,
+	MX35_PAD_LD19__IPU_DISPB_DAT_19,
+	MX35_PAD_LD20__IPU_DISPB_DAT_20,
+	MX35_PAD_LD21__IPU_DISPB_DAT_21,
+	MX35_PAD_LD22__IPU_DISPB_DAT_22,
+	MX35_PAD_LD23__IPU_DISPB_DAT_23,
+	MX35_PAD_D3_HSYNC__IPU_DISPB_D3_HSYNC,
+	MX35_PAD_D3_FPSHIFT__IPU_DISPB_D3_CLK,
+	MX35_PAD_D3_DRDY__IPU_DISPB_D3_DRDY,
+	MX35_PAD_CONTRAST__IPU_DISPB_CONTR,
+	MX35_PAD_D3_VSYNC__IPU_DISPB_D3_VSYNC,
+	MX35_PAD_D3_REV__IPU_DISPB_D3_REV,
+	MX35_PAD_D3_CLS__IPU_DISPB_D3_CLS,
 };
 
 /* OTG config */
@@ -140,6 +206,9 @@ static void __init mxc_board_init(void)
 	mxc_register_device(&mxc_usbh1, &usb_host_pdata);
 
 	imx35_add_mxc_nand(&mx35pdk_nand_board_info);
+
+	mxc_register_device(&mx3_ipu, &mx3_ipu_data);
+	mxc_register_device(&mx3_fb, &mx3fb_pdata);
 }
 
 static void __init mx35pdk_timer_init(void)
-- 
1.7.1

[PATCH 0/2] fix mx35fb display mappings

[Michael Grzeschik]

This patches introduce a fix for the display mappings
in the mx35 framebuffer. It also sets the platformspecific
display mapping for the mx35_3ds development board's tft panel
added with this patch:

http://lists.infradead.org/pipermail/linux-arm-kernel/2010-August/023914.html

Juergen Beisert (2):
  Separate display's pixel configuration from video mode
  mx35_3ds: Make usage of the display connection feature

 arch/arm/mach-mx3/mach-mx35_3ds.c      |   15 ++++++++
 arch/arm/plat-mxc/include/mach/mx3fb.h |    5 +++
 drivers/video/mx3fb.c                  |   62 +++++++++++++-------------------
 3 files changed, 45 insertions(+), 37 deletions(-)

[PATCH 1/2] Separate display's pixel configuration from video mode

[Michael Grzeschik]

From: Juergen Beisert <jbe@pengutronix.de>

Currently the pixel mapping to the display depends on the video mode. This
seems not true as the IPU transfers the data read from the video memory in an
internal 32 bit bus to the DI unit. The DI mapping only depends on the settings
the IPU should read the data from video memory. If all these settings do it in
the same byte order, only one DI mapping per display is required.

There are three locations where we can mix the colour components:
 - in the video mode description, colour offsets
 - in the IPU while reading pixel data from memory
 - in the DI unit while mapping it to the connected display

If one of these settings are incorrect, the displayed colours are broken.

With this patch the platform describes the display connection and the driver
uses this information independently from the selected video mode.

Signed-off-by: Juergen Beisert <jbe@pengutronix.de>
Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
 arch/arm/plat-mxc/include/mach/mx3fb.h |    5 +++
 drivers/video/mx3fb.c                  |   62 +++++++++++++-------------------
 2 files changed, 30 insertions(+), 37 deletions(-)

diff --git a/arch/arm/plat-mxc/include/mach/mx3fb.h b/arch/arm/plat-mxc/include/mach/mx3fb.h
index ac24c5c..93bd032 100644
--- a/arch/arm/plat-mxc/include/mach/mx3fb.h
+++ b/arch/arm/plat-mxc/include/mach/mx3fb.h
@@ -33,6 +33,11 @@ struct mx3fb_platform_data {
 	const char			*name;
 	const struct fb_videomode	*mode;
 	int				num_modes;
+
+	/* display's mapping description */
+	struct fb_bitfield red;
+	struct fb_bitfield green;
+	struct fb_bitfield blue;
 };
 
 #endif
diff --git a/drivers/video/mx3fb.c b/drivers/video/mx3fb.c
index 658f10a..c335a72 100644
--- a/drivers/video/mx3fb.c
+++ b/drivers/video/mx3fb.c
@@ -286,13 +286,6 @@ static void mx3fb_write_reg(struct mx3fb_data *mx3fb, u32 value, unsigned long r
 	__raw_writel(value, mx3fb->reg_base + reg);
 }
 
-static const uint32_t di_mappings[] = {
-	0x1600AAAA, 0x00E05555, 0x00070000, 3,	/* RGB888 */
-	0x0005000F, 0x000B000F, 0x0011000F, 1,	/* RGB666 */
-	0x0011000F, 0x000B000F, 0x0005000F, 1,	/* BGR666 */
-	0x0004003F, 0x000A000F, 0x000F003F, 1	/* RGB565 */
-};
-
 static void sdc_fb_init(struct mx3fb_info *fbi)
 {
 	struct mx3fb_data *mx3fb = fbi->mx3fb;
@@ -450,6 +443,9 @@ static int sdc_init_panel(struct mx3fb_data *mx3fb, enum ipu_panel panel,
 	uint32_t div;
 	struct clk *ipu_clk;
 
+	struct device *dev = mx3fb->dev;
+	struct mx3fb_platform_data *mx3fb_pdata = dev->platform_data;
+
 	dev_dbg(mx3fb->dev, "panel size = %d x %d", width, height);
 
 	if (v_sync_width == 0 || h_sync_width == 0)
@@ -536,36 +532,28 @@ static int sdc_init_panel(struct mx3fb_data *mx3fb, enum ipu_panel panel,
 		sig.Vsync_pol << DI_D3_VSYNC_POL_SHIFT;
 	mx3fb_write_reg(mx3fb, old_conf, DI_DISP_SIG_POL);
 
-	switch (pixel_fmt) {
-	case IPU_PIX_FMT_RGB24:
-		mx3fb_write_reg(mx3fb, di_mappings[0], DI_DISP3_B0_MAP);
-		mx3fb_write_reg(mx3fb, di_mappings[1], DI_DISP3_B1_MAP);
-		mx3fb_write_reg(mx3fb, di_mappings[2], DI_DISP3_B2_MAP);
-		mx3fb_write_reg(mx3fb, mx3fb_read_reg(mx3fb, DI_DISP_ACC_CC) |
-			     ((di_mappings[3] - 1) << 12), DI_DISP_ACC_CC);
-		break;
-	case IPU_PIX_FMT_RGB666:
-		mx3fb_write_reg(mx3fb, di_mappings[4], DI_DISP3_B0_MAP);
-		mx3fb_write_reg(mx3fb, di_mappings[5], DI_DISP3_B1_MAP);
-		mx3fb_write_reg(mx3fb, di_mappings[6], DI_DISP3_B2_MAP);
-		mx3fb_write_reg(mx3fb, mx3fb_read_reg(mx3fb, DI_DISP_ACC_CC) |
-			     ((di_mappings[7] - 1) << 12), DI_DISP_ACC_CC);
-		break;
-	case IPU_PIX_FMT_BGR666:
-		mx3fb_write_reg(mx3fb, di_mappings[8], DI_DISP3_B0_MAP);
-		mx3fb_write_reg(mx3fb, di_mappings[9], DI_DISP3_B1_MAP);
-		mx3fb_write_reg(mx3fb, di_mappings[10], DI_DISP3_B2_MAP);
-		mx3fb_write_reg(mx3fb, mx3fb_read_reg(mx3fb, DI_DISP_ACC_CC) |
-			     ((di_mappings[11] - 1) << 12), DI_DISP_ACC_CC);
-		break;
-	default:
-		mx3fb_write_reg(mx3fb, di_mappings[12], DI_DISP3_B0_MAP);
-		mx3fb_write_reg(mx3fb, di_mappings[13], DI_DISP3_B1_MAP);
-		mx3fb_write_reg(mx3fb, di_mappings[14], DI_DISP3_B2_MAP);
-		mx3fb_write_reg(mx3fb, mx3fb_read_reg(mx3fb, DI_DISP_ACC_CC) |
-			     ((di_mappings[15] - 1) << 12), DI_DISP_ACC_CC);
-		break;
-	}
+	/* setup the pixel mapping to the display. This seems independent
+	 * from the internal video mode in use
+	 */
+	reg = 0xffff & ~(((1 << (mx3fb_pdata->blue.length * 2)) - 1) << (16 - (mx3fb_pdata->blue.length * 2)));
+	reg |= ((mx3fb_pdata->blue.offset + mx3fb_pdata->blue.length - 1)) << 16;
+	pr_debug("b: %d %d 0x%08x\n", mx3fb_pdata->blue.offset, mx3fb_pdata->blue.length, reg);
+	mx3fb_write_reg(mx3fb, reg, DI_DISP3_B0_MAP); /* blue */
+
+	reg = 0xffff & ~(((1 << (mx3fb_pdata->green.length * 2)) - 1) << (16 - (mx3fb_pdata->green.length * 2)));
+	reg |= ((mx3fb_pdata->green.offset + mx3fb_pdata->green.length - 1)) << 16;
+	pr_debug("g: %d %d 0x%08x\n", mx3fb_pdata->green.offset, mx3fb_pdata->green.length, reg);
+	mx3fb_write_reg(mx3fb, reg, DI_DISP3_B1_MAP); /* green */
+
+	reg = 0xffff & ~(((1 << (mx3fb_pdata->red.length * 2)) - 1) << (16 - (mx3fb_pdata->red.length * 2)));
+	reg |= ((mx3fb_pdata->red.offset + mx3fb_pdata->red.length - 1)) << 16;
+	pr_debug("r: %d %d 0x%08x\n", mx3fb_pdata->red.offset, mx3fb_pdata->red.length, reg);
+	mx3fb_write_reg(mx3fb, reg, DI_DISP3_B2_MAP); /* red */
+
+	/* the TFT displays expecting one pixel per clock */
+	reg = mx3fb_read_reg(mx3fb, DI_DISP_ACC_CC) & ~(3 << 12);
+	reg |= 0 << 12;	/* 0 means one clock */
+	mx3fb_write_reg(mx3fb, reg, DI_DISP_ACC_CC);
 
 	spin_unlock_irqrestore(&mx3fb->lock, lock_flags);
 
-- 
1.7.1

[PATCH 2/2] mx35_3ds: Make usage of the display connection feature

[Michael Grzeschik]

From: Juergen Beisert <jbe@pengutronix.de>

This platform uses a 18 bpp display.

Signed-off-by: Juergen Beisert <jbe@pengutronix.de>
Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
 arch/arm/mach-mx3/mach-mx35_3ds.c |   15 +++++++++++++++
 1 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-mx3/mach-mx35_3ds.c b/arch/arm/mach-mx3/mach-mx35_3ds.c
index 60d5753..ba65463 100644
--- a/arch/arm/mach-mx3/mach-mx35_3ds.c
+++ b/arch/arm/mach-mx3/mach-mx35_3ds.c
@@ -75,6 +75,21 @@ static struct mx3fb_platform_data mx3fb_pdata = {
 	.name		= "CTP-CLAA070LC0ACW",
 	.mode		= fb_personality,
 	.num_modes	= ARRAY_SIZE(fb_personality),
+	/* D[12..17] red */
+	.red = {
+		.offset = 12,
+		.length = 6,
+	},
+	/* D[6..11] green */
+	.green = {
+		.offset = 6,
+		.length = 6,
+	},
+	/* D[0..5] blue */
+	.blue = {
+		.offset = 0,
+		.length = 6,
+	},
 };
 
 static const struct imxuart_platform_data uart_pdata __initconst = {
-- 
1.7.1

[PATCH 0/2] fix mx35fb display mappings

[Eric Bénard]

Hi Michael,

Le 24/08/2010 17:27, Michael Grzeschik a ?crit :
> This patches introduce a fix for the display mappings
> in the mx35 framebuffer. It also sets the platformspecific
> display mapping for the mx35_3ds development board's tft panel
> added with this patch:
>
won't this patch break other platforms which are using mx3fb ?

Eric

[PATCH 0/2] fix mx35fb display mappings

[Michael Grzeschik]

Hi Eric,

On Tue, Aug 24, 2010 at 07:13:50PM +0200, Eric B?nard wrote:
> Hi Michael,
>
> Le 24/08/2010 17:27, Michael Grzeschik a ?crit :
>> This patches introduce a fix for the display mappings
>> in the mx35 framebuffer. It also sets the platformspecific
>> display mapping for the mx35_3ds development board's tft panel
>> added with this patch:
>>
> won't this patch break other platforms which are using mx3fb ?
Yes i suppose it will. But i am currently looking for a proper solution
to strange coloring issues on different pannels.

I figured out a different but related issue in the mx3fb code:
The sdc_init_panel function, which does the display mapping, is only
called with the IPU_PIX_FMT_BGR666 or IPU_PIX_FMT_RGB666 pixfmt.
So this pixfmt should better be set over the function property
with proper di_mappings.

So i think this patches can be seen as an RFC.

Thanks,
Michael

-- 
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 |

[PATCH v2] Separate display's pixel configuration from video mode

[Michael Grzeschik]

From: Juergen Beisert <jbe@pengutronix.de>

Currently the pixel mapping to the display depends on the video mode. This
seems not true as the IPU transfers the data read from the video memory in an
internal 32 bit bus to the DI unit. The DI mapping only depends on the settings
the IPU should read the data from video memory. If all these settings do it in
the same byte order, only one DI mapping per display is required.

There are three locations where we can mix the colour components:
 - in the video mode description, colour offsets
 - in the IPU while reading pixel data from memory
 - in the DI unit while mapping it to the connected display

If one of these settings are incorrect, the displayed colours are broken.

With this patch the platform describes the display connection and the driver
uses this information independently from the selected video mode.

Signed-off-by: Juergen Beisert <jbe@pengutronix.de>
Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
v1 - v2:
	added default fallback ipu fb_bitfield values
	for the case the platform doesn't define these.

 arch/arm/plat-mxc/include/mach/mx3fb.h |    5 ++
 drivers/video/mx3fb.c                  |   80 +++++++++++++++----------------
 2 files changed, 44 insertions(+), 41 deletions(-)

diff --git a/arch/arm/plat-mxc/include/mach/mx3fb.h b/arch/arm/plat-mxc/include/mach/mx3fb.h
index ac24c5c..93bd032 100644
--- a/arch/arm/plat-mxc/include/mach/mx3fb.h
+++ b/arch/arm/plat-mxc/include/mach/mx3fb.h
@@ -33,6 +33,11 @@ struct mx3fb_platform_data {
 	const char			*name;
 	const struct fb_videomode	*mode;
 	int				num_modes;
+
+	/* display's mapping description */
+	struct fb_bitfield red;
+	struct fb_bitfield green;
+	struct fb_bitfield blue;
 };
 
 #endif
diff --git a/drivers/video/mx3fb.c b/drivers/video/mx3fb.c
index 658f10a..5e552a7 100644
--- a/drivers/video/mx3fb.c
+++ b/drivers/video/mx3fb.c
@@ -286,13 +286,6 @@ static void mx3fb_write_reg(struct mx3fb_data *mx3fb, u32 value, unsigned long r
 	__raw_writel(value, mx3fb->reg_base + reg);
 }
 
-static const uint32_t di_mappings[] = {
-	0x1600AAAA, 0x00E05555, 0x00070000, 3,	/* RGB888 */
-	0x0005000F, 0x000B000F, 0x0011000F, 1,	/* RGB666 */
-	0x0011000F, 0x000B000F, 0x0005000F, 1,	/* BGR666 */
-	0x0004003F, 0x000A000F, 0x000F003F, 1	/* RGB565 */
-};
-
 static void sdc_fb_init(struct mx3fb_info *fbi)
 {
 	struct mx3fb_data *mx3fb = fbi->mx3fb;
@@ -421,7 +414,6 @@ static int sdc_set_window_pos(struct mx3fb_data *mx3fb, enum ipu_channel channel
  * @pixel_clk:		desired pixel clock frequency in Hz.
  * @width:		width of panel in pixels.
  * @height:		height of panel in pixels.
- * @pixel_fmt:		pixel format of buffer as FOURCC ASCII code.
  * @h_start_width:	number of pixel clocks between the HSYNC signal pulse
  *			and the start of valid data.
  * @h_sync_width:	width of the HSYNC signal in units of pixel clocks.
@@ -438,7 +430,6 @@ static int sdc_set_window_pos(struct mx3fb_data *mx3fb, enum ipu_channel channel
 static int sdc_init_panel(struct mx3fb_data *mx3fb, enum ipu_panel panel,
 			  uint32_t pixel_clk,
 			  uint16_t width, uint16_t height,
-			  enum pixel_fmt pixel_fmt,
 			  uint16_t h_start_width, uint16_t h_sync_width,
 			  uint16_t h_end_width, uint16_t v_start_width,
 			  uint16_t v_sync_width, uint16_t v_end_width,
@@ -450,6 +441,9 @@ static int sdc_init_panel(struct mx3fb_data *mx3fb, enum ipu_panel panel,
 	uint32_t div;
 	struct clk *ipu_clk;
 
+	struct device *dev = mx3fb->dev;
+	struct mx3fb_platform_data *mx3fb_pdata = dev->platform_data;
+
 	dev_dbg(mx3fb->dev, "panel size = %d x %d", width, height);
 
 	if (v_sync_width == 0 || h_sync_width == 0)
@@ -536,36 +530,28 @@ static int sdc_init_panel(struct mx3fb_data *mx3fb, enum ipu_panel panel,
 		sig.Vsync_pol << DI_D3_VSYNC_POL_SHIFT;
 	mx3fb_write_reg(mx3fb, old_conf, DI_DISP_SIG_POL);
 
-	switch (pixel_fmt) {
-	case IPU_PIX_FMT_RGB24:
-		mx3fb_write_reg(mx3fb, di_mappings[0], DI_DISP3_B0_MAP);
-		mx3fb_write_reg(mx3fb, di_mappings[1], DI_DISP3_B1_MAP);
-		mx3fb_write_reg(mx3fb, di_mappings[2], DI_DISP3_B2_MAP);
-		mx3fb_write_reg(mx3fb, mx3fb_read_reg(mx3fb, DI_DISP_ACC_CC) |
-			     ((di_mappings[3] - 1) << 12), DI_DISP_ACC_CC);
-		break;
-	case IPU_PIX_FMT_RGB666:
-		mx3fb_write_reg(mx3fb, di_mappings[4], DI_DISP3_B0_MAP);
-		mx3fb_write_reg(mx3fb, di_mappings[5], DI_DISP3_B1_MAP);
-		mx3fb_write_reg(mx3fb, di_mappings[6], DI_DISP3_B2_MAP);
-		mx3fb_write_reg(mx3fb, mx3fb_read_reg(mx3fb, DI_DISP_ACC_CC) |
-			     ((di_mappings[7] - 1) << 12), DI_DISP_ACC_CC);
-		break;
-	case IPU_PIX_FMT_BGR666:
-		mx3fb_write_reg(mx3fb, di_mappings[8], DI_DISP3_B0_MAP);
-		mx3fb_write_reg(mx3fb, di_mappings[9], DI_DISP3_B1_MAP);
-		mx3fb_write_reg(mx3fb, di_mappings[10], DI_DISP3_B2_MAP);
-		mx3fb_write_reg(mx3fb, mx3fb_read_reg(mx3fb, DI_DISP_ACC_CC) |
-			     ((di_mappings[11] - 1) << 12), DI_DISP_ACC_CC);
-		break;
-	default:
-		mx3fb_write_reg(mx3fb, di_mappings[12], DI_DISP3_B0_MAP);
-		mx3fb_write_reg(mx3fb, di_mappings[13], DI_DISP3_B1_MAP);
-		mx3fb_write_reg(mx3fb, di_mappings[14], DI_DISP3_B2_MAP);
-		mx3fb_write_reg(mx3fb, mx3fb_read_reg(mx3fb, DI_DISP_ACC_CC) |
-			     ((di_mappings[15] - 1) << 12), DI_DISP_ACC_CC);
-		break;
-	}
+	/* setup the pixel mapping to the display. This seems independent
+	 * from the internal video mode in use
+	 */
+	reg = 0xffff & ~(((1 << (mx3fb_pdata->blue.length * 2)) - 1) << (16 - (mx3fb_pdata->blue.length * 2)));
+	reg |= ((mx3fb_pdata->blue.offset + mx3fb_pdata->blue.length - 1)) << 16;
+	pr_debug("b: %d %d 0x%08x\n", mx3fb_pdata->blue.offset, mx3fb_pdata->blue.length, reg);
+	mx3fb_write_reg(mx3fb, reg, DI_DISP3_B0_MAP); /* blue */
+
+	reg = 0xffff & ~(((1 << (mx3fb_pdata->green.length * 2)) - 1) << (16 - (mx3fb_pdata->green.length * 2)));
+	reg |= ((mx3fb_pdata->green.offset + mx3fb_pdata->green.length - 1)) << 16;
+	pr_debug("g: %d %d 0x%08x\n", mx3fb_pdata->green.offset, mx3fb_pdata->green.length, reg);
+	mx3fb_write_reg(mx3fb, reg, DI_DISP3_B1_MAP); /* green */
+
+	reg = 0xffff & ~(((1 << (mx3fb_pdata->red.length * 2)) - 1) << (16 - (mx3fb_pdata->red.length * 2)));
+	reg |= ((mx3fb_pdata->red.offset + mx3fb_pdata->red.length - 1)) << 16;
+	pr_debug("r: %d %d 0x%08x\n", mx3fb_pdata->red.offset, mx3fb_pdata->red.length, reg);
+	mx3fb_write_reg(mx3fb, reg, DI_DISP3_B2_MAP); /* red */
+
+	/* the TFT displays expecting one pixel per clock */
+	reg = mx3fb_read_reg(mx3fb, DI_DISP_ACC_CC) & ~(3 << 12);
+	reg |= 0 << 12;	/* 0 means one clock */
+	mx3fb_write_reg(mx3fb, reg, DI_DISP_ACC_CC);
 
 	spin_unlock_irqrestore(&mx3fb->lock, lock_flags);
 
@@ -776,8 +762,6 @@ static int __set_par(struct fb_info *fbi, bool lock)
 		if (sdc_init_panel(mx3fb, mode,
 				   (PICOS2KHZ(fbi->var.pixclock)) * 1000UL,
 				   fbi->var.xres, fbi->var.yres,
-				   (fbi->var.sync & FB_SYNC_SWAP_RGB) ?
-				   IPU_PIX_FMT_BGR666 : IPU_PIX_FMT_RGB666,
 				   fbi->var.left_margin,
 				   fbi->var.hsync_len,
 				   fbi->var.right_margin +
@@ -1367,6 +1351,20 @@ static int init_fb_chan(struct mx3fb_data *mx3fb, struct idmac_channel *ichan)
 		goto emode;
 	}
 
+	if (mx3fb_pdata->red.length == 0 &&
+	    mx3fb_pdata->green.length == 0 &&
+	    mx3fb_pdata->blue.length == 0) {
+
+		mx3fb_pdata->red.length = 6;
+		mx3fb_pdata->red.offset = 12;
+
+		mx3fb_pdata->green.length = 6;
+		mx3fb_pdata->green.offset = 6;
+
+		mx3fb_pdata->blue.length = 6;
+		mx3fb_pdata->blue.offset = 0;
+	}
+
 	fb_videomode_to_modelist(mode, num_modes, &fbi->modelist);
 
 	/* Default Y virtual size is 2x panel size */
-- 
1.7.1

[PATCH v2|RESEND] mx35_3ds: Add the TFT display to the board file

[Michael Grzeschik]

From: Juergen Beisert <jbe@pengutronix.de>

Add the personality board's display to the board file. Used display is:

Manufacturer: CTP
Type: CLAA070LC0ACW

This is a 800 x 480 wide display with 27 MHz pixel clock and 60 Hz refresh
rate with 18 bit data interface. Sync is based on DE only.

Signed-off-by: Juergen Beisert <jbe@pengutronix.de>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
v1 -> v2
        * removed superfluous comments
        * added S-o-b

 arch/arm/mach-mx3/mach-mx35_3ds.c |   69 +++++++++++++++++++++++++++++++++++++
 1 files changed, 69 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-mx3/mach-mx35_3ds.c b/arch/arm/mach-mx3/mach-mx35_3ds.c
index 1dd7baa..60d5753 100644
--- a/arch/arm/mach-mx3/mach-mx35_3ds.c
+++ b/arch/arm/mach-mx3/mach-mx35_3ds.c
@@ -39,10 +39,44 @@
 #include <mach/common.h>
 #include <mach/iomux-mx35.h>
 #include <mach/mxc_ehci.h>
+#include <mach/ipu.h>
+#include <mach/mx3fb.h>
+#include <mach/irqs.h>
 
 #include "devices-imx35.h"
 #include "devices.h"
 
+static const struct fb_videomode fb_personality[] = {
+	{
+		/* 800x480 @ 60 Hz */
+		.name		= "CTP-CLAA070LC0ACW",
+		.refresh	= 60,
+		.xres		= 800,
+		.yres		= 480,
+		.pixclock	= KHZ2PICOS(27000),
+		.left_margin	= 50,
+		.right_margin	= 50,	/* whole line should have 900 clocks */
+		.upper_margin	= 10,
+		.lower_margin	= 10,	/* whole frame should have 500 lines */
+		.hsync_len	= 1,	/* note: DE only display */
+		.vsync_len	= 1,	/* note: DE only display */
+		.sync		= FB_SYNC_CLK_IDLE_EN,
+		.vmode		= FB_VMODE_NONINTERLACED,
+		.flag		= 0,
+	},
+};
+
+static struct ipu_platform_data mx3_ipu_data = {
+	.irq_base = MXC_IPU_IRQ_START,
+};
+
+static struct mx3fb_platform_data mx3fb_pdata = {
+	.dma_dev	= &mx3_ipu.dev,
+	.name		= "CTP-CLAA070LC0ACW",
+	.mode		= fb_personality,
+	.num_modes	= ARRAY_SIZE(fb_personality),
+};
+
 static const struct imxuart_platform_data uart_pdata __initconst = {
 	.flags = IMXUART_HAVE_RTSCTS,
 };
@@ -109,6 +143,38 @@ static struct pad_desc mx35pdk_pads[] = {
 	/* USBH1 */
 	MX35_PAD_I2C2_CLK__USB_TOP_USBH2_PWR,
 	MX35_PAD_I2C2_DAT__USB_TOP_USBH2_OC,
+	/* Display */
+	MX35_PAD_LD0__IPU_DISPB_DAT_0,
+	MX35_PAD_LD1__IPU_DISPB_DAT_1,
+	MX35_PAD_LD2__IPU_DISPB_DAT_2,
+	MX35_PAD_LD3__IPU_DISPB_DAT_3,
+	MX35_PAD_LD4__IPU_DISPB_DAT_4,
+	MX35_PAD_LD5__IPU_DISPB_DAT_5,
+	MX35_PAD_LD6__IPU_DISPB_DAT_6,
+	MX35_PAD_LD7__IPU_DISPB_DAT_7,
+	MX35_PAD_LD8__IPU_DISPB_DAT_8,
+	MX35_PAD_LD9__IPU_DISPB_DAT_9,
+	MX35_PAD_LD10__IPU_DISPB_DAT_10,
+	MX35_PAD_LD11__IPU_DISPB_DAT_11,
+	MX35_PAD_LD12__IPU_DISPB_DAT_12,
+	MX35_PAD_LD13__IPU_DISPB_DAT_13,
+	MX35_PAD_LD14__IPU_DISPB_DAT_14,
+	MX35_PAD_LD15__IPU_DISPB_DAT_15,
+	MX35_PAD_LD16__IPU_DISPB_DAT_16,
+	MX35_PAD_LD17__IPU_DISPB_DAT_17,
+	MX35_PAD_LD18__IPU_DISPB_DAT_18,
+	MX35_PAD_LD19__IPU_DISPB_DAT_19,
+	MX35_PAD_LD20__IPU_DISPB_DAT_20,
+	MX35_PAD_LD21__IPU_DISPB_DAT_21,
+	MX35_PAD_LD22__IPU_DISPB_DAT_22,
+	MX35_PAD_LD23__IPU_DISPB_DAT_23,
+	MX35_PAD_D3_HSYNC__IPU_DISPB_D3_HSYNC,
+	MX35_PAD_D3_FPSHIFT__IPU_DISPB_D3_CLK,
+	MX35_PAD_D3_DRDY__IPU_DISPB_D3_DRDY,
+	MX35_PAD_CONTRAST__IPU_DISPB_CONTR,
+	MX35_PAD_D3_VSYNC__IPU_DISPB_D3_VSYNC,
+	MX35_PAD_D3_REV__IPU_DISPB_D3_REV,
+	MX35_PAD_D3_CLS__IPU_DISPB_D3_CLS,
 };
 
 /* OTG config */
@@ -140,6 +206,9 @@ static void __init mxc_board_init(void)
 	mxc_register_device(&mxc_usbh1, &usb_host_pdata);
 
 	imx35_add_mxc_nand(&mx35pdk_nand_board_info);
+
+	mxc_register_device(&mx3_ipu, &mx3_ipu_data);
+	mxc_register_device(&mx3_fb, &mx3fb_pdata);
 }
 
 static void __init mx35pdk_timer_init(void)
-- 
1.7.2.3

[PATCH v2|RESEND] Separate display's pixel configuration from video mode

[Michael Grzeschik]

From: Juergen Beisert <jbe@pengutronix.de>

Currently the pixel mapping to the display depends on the video mode. This
seems not true as the IPU transfers the data read from the video memory in an
internal 32 bit bus to the DI unit. The DI mapping only depends on the settings
the IPU should read the data from video memory. If all these settings do it in
the same byte order, only one DI mapping per display is required.

There are three locations where we can mix the colour components:
 - in the video mode description, colour offsets
 - in the IPU while reading pixel data from memory
 - in the DI unit while mapping it to the connected display

If one of these settings are incorrect, the displayed colours are broken.

With this patch the platform describes the display connection and the driver
uses this information independently from the selected video mode.

Signed-off-by: Juergen Beisert <jbe@pengutronix.de>
Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
v1 - v2:
        added default fallback ipu fb_bitfield values
        for the case the platform doesn't define these.

 arch/arm/plat-mxc/include/mach/mx3fb.h |    5 ++
 drivers/video/mx3fb.c                  |   80 +++++++++++++++----------------
 2 files changed, 44 insertions(+), 41 deletions(-)

diff --git a/arch/arm/plat-mxc/include/mach/mx3fb.h b/arch/arm/plat-mxc/include/mach/mx3fb.h
index ac24c5c..93bd032 100644
--- a/arch/arm/plat-mxc/include/mach/mx3fb.h
+++ b/arch/arm/plat-mxc/include/mach/mx3fb.h
@@ -33,6 +33,11 @@ struct mx3fb_platform_data {
 	const char			*name;
 	const struct fb_videomode	*mode;
 	int				num_modes;
+
+	/* display's mapping description */
+	struct fb_bitfield red;
+	struct fb_bitfield green;
+	struct fb_bitfield blue;
 };
 
 #endif
diff --git a/drivers/video/mx3fb.c b/drivers/video/mx3fb.c
index 658f10a..5e552a7 100644
--- a/drivers/video/mx3fb.c
+++ b/drivers/video/mx3fb.c
@@ -286,13 +286,6 @@ static void mx3fb_write_reg(struct mx3fb_data *mx3fb, u32 value, unsigned long r
 	__raw_writel(value, mx3fb->reg_base + reg);
 }
 
-static const uint32_t di_mappings[] = {
-	0x1600AAAA, 0x00E05555, 0x00070000, 3,	/* RGB888 */
-	0x0005000F, 0x000B000F, 0x0011000F, 1,	/* RGB666 */
-	0x0011000F, 0x000B000F, 0x0005000F, 1,	/* BGR666 */
-	0x0004003F, 0x000A000F, 0x000F003F, 1	/* RGB565 */
-};
-
 static void sdc_fb_init(struct mx3fb_info *fbi)
 {
 	struct mx3fb_data *mx3fb = fbi->mx3fb;
@@ -421,7 +414,6 @@ static int sdc_set_window_pos(struct mx3fb_data *mx3fb, enum ipu_channel channel
  * @pixel_clk:		desired pixel clock frequency in Hz.
  * @width:		width of panel in pixels.
  * @height:		height of panel in pixels.
- * @pixel_fmt:		pixel format of buffer as FOURCC ASCII code.
  * @h_start_width:	number of pixel clocks between the HSYNC signal pulse
  *			and the start of valid data.
  * @h_sync_width:	width of the HSYNC signal in units of pixel clocks.
@@ -438,7 +430,6 @@ static int sdc_set_window_pos(struct mx3fb_data *mx3fb, enum ipu_channel channel
 static int sdc_init_panel(struct mx3fb_data *mx3fb, enum ipu_panel panel,
 			  uint32_t pixel_clk,
 			  uint16_t width, uint16_t height,
-			  enum pixel_fmt pixel_fmt,
 			  uint16_t h_start_width, uint16_t h_sync_width,
 			  uint16_t h_end_width, uint16_t v_start_width,
 			  uint16_t v_sync_width, uint16_t v_end_width,
@@ -450,6 +441,9 @@ static int sdc_init_panel(struct mx3fb_data *mx3fb, enum ipu_panel panel,
 	uint32_t div;
 	struct clk *ipu_clk;
 
+	struct device *dev = mx3fb->dev;
+	struct mx3fb_platform_data *mx3fb_pdata = dev->platform_data;
+
 	dev_dbg(mx3fb->dev, "panel size = %d x %d", width, height);
 
 	if (v_sync_width == 0 || h_sync_width == 0)
@@ -536,36 +530,28 @@ static int sdc_init_panel(struct mx3fb_data *mx3fb, enum ipu_panel panel,
 		sig.Vsync_pol << DI_D3_VSYNC_POL_SHIFT;
 	mx3fb_write_reg(mx3fb, old_conf, DI_DISP_SIG_POL);
 
-	switch (pixel_fmt) {
-	case IPU_PIX_FMT_RGB24:
-		mx3fb_write_reg(mx3fb, di_mappings[0], DI_DISP3_B0_MAP);
-		mx3fb_write_reg(mx3fb, di_mappings[1], DI_DISP3_B1_MAP);
-		mx3fb_write_reg(mx3fb, di_mappings[2], DI_DISP3_B2_MAP);
-		mx3fb_write_reg(mx3fb, mx3fb_read_reg(mx3fb, DI_DISP_ACC_CC) |
-			     ((di_mappings[3] - 1) << 12), DI_DISP_ACC_CC);
-		break;
-	case IPU_PIX_FMT_RGB666:
-		mx3fb_write_reg(mx3fb, di_mappings[4], DI_DISP3_B0_MAP);
-		mx3fb_write_reg(mx3fb, di_mappings[5], DI_DISP3_B1_MAP);
-		mx3fb_write_reg(mx3fb, di_mappings[6], DI_DISP3_B2_MAP);
-		mx3fb_write_reg(mx3fb, mx3fb_read_reg(mx3fb, DI_DISP_ACC_CC) |
-			     ((di_mappings[7] - 1) << 12), DI_DISP_ACC_CC);
-		break;
-	case IPU_PIX_FMT_BGR666:
-		mx3fb_write_reg(mx3fb, di_mappings[8], DI_DISP3_B0_MAP);
-		mx3fb_write_reg(mx3fb, di_mappings[9], DI_DISP3_B1_MAP);
-		mx3fb_write_reg(mx3fb, di_mappings[10], DI_DISP3_B2_MAP);
-		mx3fb_write_reg(mx3fb, mx3fb_read_reg(mx3fb, DI_DISP_ACC_CC) |
-			     ((di_mappings[11] - 1) << 12), DI_DISP_ACC_CC);
-		break;
-	default:
-		mx3fb_write_reg(mx3fb, di_mappings[12], DI_DISP3_B0_MAP);
-		mx3fb_write_reg(mx3fb, di_mappings[13], DI_DISP3_B1_MAP);
-		mx3fb_write_reg(mx3fb, di_mappings[14], DI_DISP3_B2_MAP);
-		mx3fb_write_reg(mx3fb, mx3fb_read_reg(mx3fb, DI_DISP_ACC_CC) |
-			     ((di_mappings[15] - 1) << 12), DI_DISP_ACC_CC);
-		break;
-	}
+	/* setup the pixel mapping to the display. This seems independent
+	 * from the internal video mode in use
+	 */
+	reg = 0xffff & ~(((1 << (mx3fb_pdata->blue.length * 2)) - 1) << (16 - (mx3fb_pdata->blue.length * 2)));
+	reg |= ((mx3fb_pdata->blue.offset + mx3fb_pdata->blue.length - 1)) << 16;
+	pr_debug("b: %d %d 0x%08x\n", mx3fb_pdata->blue.offset, mx3fb_pdata->blue.length, reg);
+	mx3fb_write_reg(mx3fb, reg, DI_DISP3_B0_MAP); /* blue */
+
+	reg = 0xffff & ~(((1 << (mx3fb_pdata->green.length * 2)) - 1) << (16 - (mx3fb_pdata->green.length * 2)));
+	reg |= ((mx3fb_pdata->green.offset + mx3fb_pdata->green.length - 1)) << 16;
+	pr_debug("g: %d %d 0x%08x\n", mx3fb_pdata->green.offset, mx3fb_pdata->green.length, reg);
+	mx3fb_write_reg(mx3fb, reg, DI_DISP3_B1_MAP); /* green */
+
+	reg = 0xffff & ~(((1 << (mx3fb_pdata->red.length * 2)) - 1) << (16 - (mx3fb_pdata->red.length * 2)));
+	reg |= ((mx3fb_pdata->red.offset + mx3fb_pdata->red.length - 1)) << 16;
+	pr_debug("r: %d %d 0x%08x\n", mx3fb_pdata->red.offset, mx3fb_pdata->red.length, reg);
+	mx3fb_write_reg(mx3fb, reg, DI_DISP3_B2_MAP); /* red */
+
+	/* the TFT displays expecting one pixel per clock */
+	reg = mx3fb_read_reg(mx3fb, DI_DISP_ACC_CC) & ~(3 << 12);
+	reg |= 0 << 12;	/* 0 means one clock */
+	mx3fb_write_reg(mx3fb, reg, DI_DISP_ACC_CC);
 
 	spin_unlock_irqrestore(&mx3fb->lock, lock_flags);
 
@@ -776,8 +762,6 @@ static int __set_par(struct fb_info *fbi, bool lock)
 		if (sdc_init_panel(mx3fb, mode,
 				   (PICOS2KHZ(fbi->var.pixclock)) * 1000UL,
 				   fbi->var.xres, fbi->var.yres,
-				   (fbi->var.sync & FB_SYNC_SWAP_RGB) ?
-				   IPU_PIX_FMT_BGR666 : IPU_PIX_FMT_RGB666,
 				   fbi->var.left_margin,
 				   fbi->var.hsync_len,
 				   fbi->var.right_margin +
@@ -1367,6 +1351,20 @@ static int init_fb_chan(struct mx3fb_data *mx3fb, struct idmac_channel *ichan)
 		goto emode;
 	}
 
+	if (mx3fb_pdata->red.length == 0 &&
+	    mx3fb_pdata->green.length == 0 &&
+	    mx3fb_pdata->blue.length == 0) {
+
+		mx3fb_pdata->red.length = 6;
+		mx3fb_pdata->red.offset = 12;
+
+		mx3fb_pdata->green.length = 6;
+		mx3fb_pdata->green.offset = 6;
+
+		mx3fb_pdata->blue.length = 6;
+		mx3fb_pdata->blue.offset = 0;
+	}
+
 	fb_videomode_to_modelist(mode, num_modes, &fbi->modelist);
 
 	/* Default Y virtual size is 2x panel size */
-- 
1.7.2.3

[PATCH v2|RESEND] Separate display's pixel configuration from video mode

[Guennadi Liakhovetski]

On Wed, 22 Sep 2010, Michael Grzeschik wrote:

> From: Juergen Beisert <jbe@pengutronix.de>
> 
> Currently the pixel mapping to the display depends on the video mode. This
> seems not true as the IPU transfers the data read from the video memory in an
> internal 32 bit bus to the DI unit. The DI mapping only depends on the settings
> the IPU should read the data from video memory. If all these settings do it in
> the same byte order, only one DI mapping per display is required.
> 
> There are three locations where we can mix the colour components:
>  - in the video mode description, colour offsets
>  - in the IPU while reading pixel data from memory
>  - in the DI unit while mapping it to the connected display
> 
> If one of these settings are incorrect, the displayed colours are broken.
> 
> With this patch the platform describes the display connection and the driver
> uses this information independently from the selected video mode.
> 
> Signed-off-by: Juergen Beisert <jbe@pengutronix.de>
> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
> ---
> v1 - v2:
>         added default fallback ipu fb_bitfield values
>         for the case the platform doesn't define these.
> 
>  arch/arm/plat-mxc/include/mach/mx3fb.h |    5 ++
>  drivers/video/mx3fb.c                  |   80 +++++++++++++++----------------
>  2 files changed, 44 insertions(+), 41 deletions(-)
> 
> diff --git a/arch/arm/plat-mxc/include/mach/mx3fb.h b/arch/arm/plat-mxc/include/mach/mx3fb.h
> index ac24c5c..93bd032 100644
> --- a/arch/arm/plat-mxc/include/mach/mx3fb.h
> +++ b/arch/arm/plat-mxc/include/mach/mx3fb.h
> @@ -33,6 +33,11 @@ struct mx3fb_platform_data {
>  	const char			*name;
>  	const struct fb_videomode	*mode;
>  	int				num_modes;
> +
> +	/* display's mapping description */
> +	struct fb_bitfield red;
> +	struct fb_bitfield green;
> +	struct fb_bitfield blue;
>  };
>  
>  #endif
> diff --git a/drivers/video/mx3fb.c b/drivers/video/mx3fb.c
> index 658f10a..5e552a7 100644
> --- a/drivers/video/mx3fb.c
> +++ b/drivers/video/mx3fb.c
> @@ -286,13 +286,6 @@ static void mx3fb_write_reg(struct mx3fb_data *mx3fb, u32 value, unsigned long r
>  	__raw_writel(value, mx3fb->reg_base + reg);
>  }
>  
> -static const uint32_t di_mappings[] = {
> -	0x1600AAAA, 0x00E05555, 0x00070000, 3,	/* RGB888 */
> -	0x0005000F, 0x000B000F, 0x0011000F, 1,	/* RGB666 */
> -	0x0011000F, 0x000B000F, 0x0005000F, 1,	/* BGR666 */
> -	0x0004003F, 0x000A000F, 0x000F003F, 1	/* RGB565 */
> -};
> -
>  static void sdc_fb_init(struct mx3fb_info *fbi)
>  {
>  	struct mx3fb_data *mx3fb = fbi->mx3fb;
> @@ -421,7 +414,6 @@ static int sdc_set_window_pos(struct mx3fb_data *mx3fb, enum ipu_channel channel
>   * @pixel_clk:		desired pixel clock frequency in Hz.
>   * @width:		width of panel in pixels.
>   * @height:		height of panel in pixels.
> - * @pixel_fmt:		pixel format of buffer as FOURCC ASCII code.
>   * @h_start_width:	number of pixel clocks between the HSYNC signal pulse
>   *			and the start of valid data.
>   * @h_sync_width:	width of the HSYNC signal in units of pixel clocks.
> @@ -438,7 +430,6 @@ static int sdc_set_window_pos(struct mx3fb_data *mx3fb, enum ipu_channel channel
>  static int sdc_init_panel(struct mx3fb_data *mx3fb, enum ipu_panel panel,
>  			  uint32_t pixel_clk,
>  			  uint16_t width, uint16_t height,
> -			  enum pixel_fmt pixel_fmt,
>  			  uint16_t h_start_width, uint16_t h_sync_width,
>  			  uint16_t h_end_width, uint16_t v_start_width,
>  			  uint16_t v_sync_width, uint16_t v_end_width,
> @@ -450,6 +441,9 @@ static int sdc_init_panel(struct mx3fb_data *mx3fb, enum ipu_panel panel,
>  	uint32_t div;
>  	struct clk *ipu_clk;
>  
> +	struct device *dev = mx3fb->dev;
> +	struct mx3fb_platform_data *mx3fb_pdata = dev->platform_data;
> +
>  	dev_dbg(mx3fb->dev, "panel size = %d x %d", width, height);
>  
>  	if (v_sync_width == 0 || h_sync_width == 0)
> @@ -536,36 +530,28 @@ static int sdc_init_panel(struct mx3fb_data *mx3fb, enum ipu_panel panel,
>  		sig.Vsync_pol << DI_D3_VSYNC_POL_SHIFT;
>  	mx3fb_write_reg(mx3fb, old_conf, DI_DISP_SIG_POL);
>  
> -	switch (pixel_fmt) {
> -	case IPU_PIX_FMT_RGB24:
> -		mx3fb_write_reg(mx3fb, di_mappings[0], DI_DISP3_B0_MAP);
> -		mx3fb_write_reg(mx3fb, di_mappings[1], DI_DISP3_B1_MAP);
> -		mx3fb_write_reg(mx3fb, di_mappings[2], DI_DISP3_B2_MAP);
> -		mx3fb_write_reg(mx3fb, mx3fb_read_reg(mx3fb, DI_DISP_ACC_CC) |
> -			     ((di_mappings[3] - 1) << 12), DI_DISP_ACC_CC);
> -		break;
> -	case IPU_PIX_FMT_RGB666:
> -		mx3fb_write_reg(mx3fb, di_mappings[4], DI_DISP3_B0_MAP);
> -		mx3fb_write_reg(mx3fb, di_mappings[5], DI_DISP3_B1_MAP);
> -		mx3fb_write_reg(mx3fb, di_mappings[6], DI_DISP3_B2_MAP);
> -		mx3fb_write_reg(mx3fb, mx3fb_read_reg(mx3fb, DI_DISP_ACC_CC) |
> -			     ((di_mappings[7] - 1) << 12), DI_DISP_ACC_CC);
> -		break;
> -	case IPU_PIX_FMT_BGR666:
> -		mx3fb_write_reg(mx3fb, di_mappings[8], DI_DISP3_B0_MAP);
> -		mx3fb_write_reg(mx3fb, di_mappings[9], DI_DISP3_B1_MAP);
> -		mx3fb_write_reg(mx3fb, di_mappings[10], DI_DISP3_B2_MAP);
> -		mx3fb_write_reg(mx3fb, mx3fb_read_reg(mx3fb, DI_DISP_ACC_CC) |
> -			     ((di_mappings[11] - 1) << 12), DI_DISP_ACC_CC);
> -		break;
> -	default:
> -		mx3fb_write_reg(mx3fb, di_mappings[12], DI_DISP3_B0_MAP);
> -		mx3fb_write_reg(mx3fb, di_mappings[13], DI_DISP3_B1_MAP);
> -		mx3fb_write_reg(mx3fb, di_mappings[14], DI_DISP3_B2_MAP);
> -		mx3fb_write_reg(mx3fb, mx3fb_read_reg(mx3fb, DI_DISP_ACC_CC) |
> -			     ((di_mappings[15] - 1) << 12), DI_DISP_ACC_CC);
> -		break;
> -	}
> +	/* setup the pixel mapping to the display. This seems independent
> +	 * from the internal video mode in use
> +	 */

Multiline comment style

> +	reg = 0xffff & ~(((1 << (mx3fb_pdata->blue.length * 2)) - 1) << (16 - (mx3fb_pdata->blue.length * 2)));
> +	reg |= ((mx3fb_pdata->blue.offset + mx3fb_pdata->blue.length - 1)) << 16;
> +	pr_debug("b: %d %d 0x%08x\n", mx3fb_pdata->blue.offset, mx3fb_pdata->blue.length, reg);
> +	mx3fb_write_reg(mx3fb, reg, DI_DISP3_B0_MAP); /* blue */
> +
> +	reg = 0xffff & ~(((1 << (mx3fb_pdata->green.length * 2)) - 1) << (16 - (mx3fb_pdata->green.length * 2)));
> +	reg |= ((mx3fb_pdata->green.offset + mx3fb_pdata->green.length - 1)) << 16;
> +	pr_debug("g: %d %d 0x%08x\n", mx3fb_pdata->green.offset, mx3fb_pdata->green.length, reg);
> +	mx3fb_write_reg(mx3fb, reg, DI_DISP3_B1_MAP); /* green */
> +
> +	reg = 0xffff & ~(((1 << (mx3fb_pdata->red.length * 2)) - 1) << (16 - (mx3fb_pdata->red.length * 2)));
> +	reg |= ((mx3fb_pdata->red.offset + mx3fb_pdata->red.length - 1)) << 16;
> +	pr_debug("r: %d %d 0x%08x\n", mx3fb_pdata->red.offset, mx3fb_pdata->red.length, reg);
> +	mx3fb_write_reg(mx3fb, reg, DI_DISP3_B2_MAP); /* red */

All three calculations exactly identical. How about a

	set_colour_mapping(mx3fb, &mx3fb_pdata->blue, DI_DISP3_B0_MAP);
	set_colour_mapping(mx3fb, &mx3fb_pdata->green, DI_DISP3_B1_MAP);
	set_colour_mapping(mx3fb, &mx3fb_pdata->red, DI_DISP3_B2_MAP);

(feel free to come up with a better name)

> +
> +	/* the TFT displays expecting one pixel per clock */
> +	reg = mx3fb_read_reg(mx3fb, DI_DISP_ACC_CC) & ~(3 << 12);
> +	reg |= 0 << 12;	/* 0 means one clock */
> +	mx3fb_write_reg(mx3fb, reg, DI_DISP_ACC_CC);
>  
>  	spin_unlock_irqrestore(&mx3fb->lock, lock_flags);
>  
> @@ -776,8 +762,6 @@ static int __set_par(struct fb_info *fbi, bool lock)
>  		if (sdc_init_panel(mx3fb, mode,
>  				   (PICOS2KHZ(fbi->var.pixclock)) * 1000UL,
>  				   fbi->var.xres, fbi->var.yres,
> -				   (fbi->var.sync & FB_SYNC_SWAP_RGB) ?
> -				   IPU_PIX_FMT_BGR666 : IPU_PIX_FMT_RGB666,
>  				   fbi->var.left_margin,
>  				   fbi->var.hsync_len,
>  				   fbi->var.right_margin +
> @@ -1367,6 +1351,20 @@ static int init_fb_chan(struct mx3fb_data *mx3fb, struct idmac_channel *ichan)
>  		goto emode;
>  	}
>  
> +	if (mx3fb_pdata->red.length == 0 &&
> +	    mx3fb_pdata->green.length == 0 &&
> +	    mx3fb_pdata->blue.length == 0) {
> +
> +		mx3fb_pdata->red.length = 6;
> +		mx3fb_pdata->red.offset = 12;
> +
> +		mx3fb_pdata->green.length = 6;
> +		mx3fb_pdata->green.offset = 6;
> +
> +		mx3fb_pdata->blue.length = 6;
> +		mx3fb_pdata->blue.offset = 0;
> +	}
> +

Ok, your register values, that you calculate from these default, are 
different, from what the di_mappings[] array contains but it doesn't seem 
to break the fb (judging only by the penguin, the kernel doesn't boot for 
some reason). So, once the above are fixed, you'll get my "Tested-by"

>  	fb_videomode_to_modelist(mode, num_modes, &fbi->modelist);
>  
>  	/* Default Y virtual size is 2x panel size */
> -- 
> 1.7.2.3

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

[PATCH v3] Separate display's pixel configuration from video mode

[Michael Grzeschik]

From: Juergen Beisert <jbe@pengutronix.de>

Currently the pixel mapping to the display depends on the video mode. This
seems not true as the IPU transfers the data read from the video memory in an
internal 32 bit bus to the DI unit. The DI mapping only depends on the settings
the IPU should read the data from video memory. If all these settings do it in
the same byte order, only one DI mapping per display is required.

There are three locations where we can mix the colour components:
 - in the video mode description, colour offsets
 - in the IPU while reading pixel data from memory
 - in the DI unit while mapping it to the connected display

If one of these settings are incorrect, the displayed colours are broken.

With this patch the platform describes the display connection and the driver
uses this information independently from the selected video mode.

Signed-off-by: Juergen Beisert <jbe@pengutronix.de>
Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
 v2 - v3:
	changed to multiline comment
	added a generic set_di_bus_mapping function

 v1 - v2:
        added default fallback ipu fb_bitfield values
        for the case the platform doesn't define these.

 arch/arm/plat-mxc/include/mach/mx3fb.h |    5 ++
 drivers/video/mx3fb.c                  |   81 ++++++++++++++++----------------
 2 files changed, 45 insertions(+), 41 deletions(-)

diff --git a/arch/arm/plat-mxc/include/mach/mx3fb.h b/arch/arm/plat-mxc/include/mach/mx3fb.h
index ac24c5c..93bd032 100644
--- a/arch/arm/plat-mxc/include/mach/mx3fb.h
+++ b/arch/arm/plat-mxc/include/mach/mx3fb.h
@@ -33,6 +33,11 @@ struct mx3fb_platform_data {
 	const char			*name;
 	const struct fb_videomode	*mode;
 	int				num_modes;
+
+	/* display's mapping description */
+	struct fb_bitfield red;
+	struct fb_bitfield green;
+	struct fb_bitfield blue;
 };
 
 #endif
diff --git a/drivers/video/mx3fb.c b/drivers/video/mx3fb.c
index 658f10a..ca20da8 100644
--- a/drivers/video/mx3fb.c
+++ b/drivers/video/mx3fb.c
@@ -286,13 +286,6 @@ static void mx3fb_write_reg(struct mx3fb_data *mx3fb, u32 value, unsigned long r
 	__raw_writel(value, mx3fb->reg_base + reg);
 }
 
-static const uint32_t di_mappings[] = {
-	0x1600AAAA, 0x00E05555, 0x00070000, 3,	/* RGB888 */
-	0x0005000F, 0x000B000F, 0x0011000F, 1,	/* RGB666 */
-	0x0011000F, 0x000B000F, 0x0005000F, 1,	/* BGR666 */
-	0x0004003F, 0x000A000F, 0x000F003F, 1	/* RGB565 */
-};
-
 static void sdc_fb_init(struct mx3fb_info *fbi)
 {
 	struct mx3fb_data *mx3fb = fbi->mx3fb;
@@ -414,6 +407,17 @@ static int sdc_set_window_pos(struct mx3fb_data *mx3fb, enum ipu_channel channel
 	return 0;
 }
 
+static void set_di_bus_mapping(struct mx3fb_data *mx3fb, struct fb_bitfield *bf,
+			      unsigned long reg)
+{
+	u32 val = 0xffff;
+
+	val &= ~(((1 << (bf->length * 2)) - 1) << (16 - (bf->length * 2)));
+	val |= ((bf->offset + bf->length - 1)) << 16;
+	pr_debug("b: %d %d 0x%08x\n", bf->offset, bf->length, val);
+	mx3fb_write_reg(mx3fb, val, reg);
+}
+
 /**
  * sdc_init_panel() - initialize a synchronous LCD panel.
  * @mx3fb:		mx3fb context.
@@ -421,7 +425,6 @@ static int sdc_set_window_pos(struct mx3fb_data *mx3fb, enum ipu_channel channel
  * @pixel_clk:		desired pixel clock frequency in Hz.
  * @width:		width of panel in pixels.
  * @height:		height of panel in pixels.
- * @pixel_fmt:		pixel format of buffer as FOURCC ASCII code.
  * @h_start_width:	number of pixel clocks between the HSYNC signal pulse
  *			and the start of valid data.
  * @h_sync_width:	width of the HSYNC signal in units of pixel clocks.
@@ -438,7 +441,6 @@ static int sdc_set_window_pos(struct mx3fb_data *mx3fb, enum ipu_channel channel
 static int sdc_init_panel(struct mx3fb_data *mx3fb, enum ipu_panel panel,
 			  uint32_t pixel_clk,
 			  uint16_t width, uint16_t height,
-			  enum pixel_fmt pixel_fmt,
 			  uint16_t h_start_width, uint16_t h_sync_width,
 			  uint16_t h_end_width, uint16_t v_start_width,
 			  uint16_t v_sync_width, uint16_t v_end_width,
@@ -450,6 +452,9 @@ static int sdc_init_panel(struct mx3fb_data *mx3fb, enum ipu_panel panel,
 	uint32_t div;
 	struct clk *ipu_clk;
 
+	struct device *dev = mx3fb->dev;
+	struct mx3fb_platform_data *mx3fb_pdata = dev->platform_data;
+
 	dev_dbg(mx3fb->dev, "panel size = %d x %d", width, height);
 
 	if (v_sync_width == 0 || h_sync_width == 0)
@@ -536,36 +541,18 @@ static int sdc_init_panel(struct mx3fb_data *mx3fb, enum ipu_panel panel,
 		sig.Vsync_pol << DI_D3_VSYNC_POL_SHIFT;
 	mx3fb_write_reg(mx3fb, old_conf, DI_DISP_SIG_POL);
 
-	switch (pixel_fmt) {
-	case IPU_PIX_FMT_RGB24:
-		mx3fb_write_reg(mx3fb, di_mappings[0], DI_DISP3_B0_MAP);
-		mx3fb_write_reg(mx3fb, di_mappings[1], DI_DISP3_B1_MAP);
-		mx3fb_write_reg(mx3fb, di_mappings[2], DI_DISP3_B2_MAP);
-		mx3fb_write_reg(mx3fb, mx3fb_read_reg(mx3fb, DI_DISP_ACC_CC) |
-			     ((di_mappings[3] - 1) << 12), DI_DISP_ACC_CC);
-		break;
-	case IPU_PIX_FMT_RGB666:
-		mx3fb_write_reg(mx3fb, di_mappings[4], DI_DISP3_B0_MAP);
-		mx3fb_write_reg(mx3fb, di_mappings[5], DI_DISP3_B1_MAP);
-		mx3fb_write_reg(mx3fb, di_mappings[6], DI_DISP3_B2_MAP);
-		mx3fb_write_reg(mx3fb, mx3fb_read_reg(mx3fb, DI_DISP_ACC_CC) |
-			     ((di_mappings[7] - 1) << 12), DI_DISP_ACC_CC);
-		break;
-	case IPU_PIX_FMT_BGR666:
-		mx3fb_write_reg(mx3fb, di_mappings[8], DI_DISP3_B0_MAP);
-		mx3fb_write_reg(mx3fb, di_mappings[9], DI_DISP3_B1_MAP);
-		mx3fb_write_reg(mx3fb, di_mappings[10], DI_DISP3_B2_MAP);
-		mx3fb_write_reg(mx3fb, mx3fb_read_reg(mx3fb, DI_DISP_ACC_CC) |
-			     ((di_mappings[11] - 1) << 12), DI_DISP_ACC_CC);
-		break;
-	default:
-		mx3fb_write_reg(mx3fb, di_mappings[12], DI_DISP3_B0_MAP);
-		mx3fb_write_reg(mx3fb, di_mappings[13], DI_DISP3_B1_MAP);
-		mx3fb_write_reg(mx3fb, di_mappings[14], DI_DISP3_B2_MAP);
-		mx3fb_write_reg(mx3fb, mx3fb_read_reg(mx3fb, DI_DISP_ACC_CC) |
-			     ((di_mappings[15] - 1) << 12), DI_DISP_ACC_CC);
-		break;
-	}
+	/*
+	 * setup the pixel mapping to the display. This seems independent
+	 * from the internal video mode in use
+	 */
+	set_di_bus_mapping(mx3fb, &mx3fb_pdata->blue, DI_DISP3_B0_MAP);
+	set_di_bus_mapping(mx3fb, &mx3fb_pdata->green, DI_DISP3_B1_MAP);
+	set_di_bus_mapping(mx3fb, &mx3fb_pdata->red, DI_DISP3_B2_MAP);
+
+	/* the TFT displays expecting one pixel per clock */
+	reg = mx3fb_read_reg(mx3fb, DI_DISP_ACC_CC) & ~(3 << 12);
+	reg |= 0 << 12;	/* 0 means one clock */
+	mx3fb_write_reg(mx3fb, reg, DI_DISP_ACC_CC);
 
 	spin_unlock_irqrestore(&mx3fb->lock, lock_flags);
 
@@ -776,8 +763,6 @@ static int __set_par(struct fb_info *fbi, bool lock)
 		if (sdc_init_panel(mx3fb, mode,
 				   (PICOS2KHZ(fbi->var.pixclock)) * 1000UL,
 				   fbi->var.xres, fbi->var.yres,
-				   (fbi->var.sync & FB_SYNC_SWAP_RGB) ?
-				   IPU_PIX_FMT_BGR666 : IPU_PIX_FMT_RGB666,
 				   fbi->var.left_margin,
 				   fbi->var.hsync_len,
 				   fbi->var.right_margin +
@@ -1367,6 +1352,20 @@ static int init_fb_chan(struct mx3fb_data *mx3fb, struct idmac_channel *ichan)
 		goto emode;
 	}
 
+	if (mx3fb_pdata->red.length == 0 &&
+	    mx3fb_pdata->green.length == 0 &&
+	    mx3fb_pdata->blue.length == 0) {
+
+		mx3fb_pdata->red.length = 6;
+		mx3fb_pdata->red.offset = 12;
+
+		mx3fb_pdata->green.length = 6;
+		mx3fb_pdata->green.offset = 6;
+
+		mx3fb_pdata->blue.length = 6;
+		mx3fb_pdata->blue.offset = 0;
+	}
+
 	fb_videomode_to_modelist(mode, num_modes, &fbi->modelist);
 
 	/* Default Y virtual size is 2x panel size */
-- 
1.7.2.3

[PATCH v3] Separate display's pixel configuration from video mode

[Guennadi Liakhovetski]

On Wed, 22 Sep 2010, Michael Grzeschik wrote:

> From: Juergen Beisert <jbe@pengutronix.de>
> 
> Currently the pixel mapping to the display depends on the video mode. This
> seems not true as the IPU transfers the data read from the video memory in an
> internal 32 bit bus to the DI unit. The DI mapping only depends on the settings
> the IPU should read the data from video memory. If all these settings do it in
> the same byte order, only one DI mapping per display is required.
> 
> There are three locations where we can mix the colour components:
>  - in the video mode description, colour offsets
>  - in the IPU while reading pixel data from memory
>  - in the DI unit while mapping it to the connected display
> 
> If one of these settings are incorrect, the displayed colours are broken.
> 
> With this patch the platform describes the display connection and the driver
> uses this information independently from the selected video mode.
> 
> Signed-off-by: Juergen Beisert <jbe@pengutronix.de>
> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>

Tested-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>

> ---
>  v2 - v3:
> 	changed to multiline comment
> 	added a generic set_di_bus_mapping function
> 
>  v1 - v2:
>         added default fallback ipu fb_bitfield values
>         for the case the platform doesn't define these.
> 
>  arch/arm/plat-mxc/include/mach/mx3fb.h |    5 ++
>  drivers/video/mx3fb.c                  |   81 ++++++++++++++++----------------
>  2 files changed, 45 insertions(+), 41 deletions(-)
> 
> diff --git a/arch/arm/plat-mxc/include/mach/mx3fb.h b/arch/arm/plat-mxc/include/mach/mx3fb.h
> index ac24c5c..93bd032 100644
> --- a/arch/arm/plat-mxc/include/mach/mx3fb.h
> +++ b/arch/arm/plat-mxc/include/mach/mx3fb.h
> @@ -33,6 +33,11 @@ struct mx3fb_platform_data {
>  	const char			*name;
>  	const struct fb_videomode	*mode;
>  	int				num_modes;
> +
> +	/* display's mapping description */
> +	struct fb_bitfield red;
> +	struct fb_bitfield green;
> +	struct fb_bitfield blue;
>  };
>  
>  #endif
> diff --git a/drivers/video/mx3fb.c b/drivers/video/mx3fb.c
> index 658f10a..ca20da8 100644
> --- a/drivers/video/mx3fb.c
> +++ b/drivers/video/mx3fb.c
> @@ -286,13 +286,6 @@ static void mx3fb_write_reg(struct mx3fb_data *mx3fb, u32 value, unsigned long r
>  	__raw_writel(value, mx3fb->reg_base + reg);
>  }
>  
> -static const uint32_t di_mappings[] = {
> -	0x1600AAAA, 0x00E05555, 0x00070000, 3,	/* RGB888 */
> -	0x0005000F, 0x000B000F, 0x0011000F, 1,	/* RGB666 */
> -	0x0011000F, 0x000B000F, 0x0005000F, 1,	/* BGR666 */
> -	0x0004003F, 0x000A000F, 0x000F003F, 1	/* RGB565 */
> -};
> -
>  static void sdc_fb_init(struct mx3fb_info *fbi)
>  {
>  	struct mx3fb_data *mx3fb = fbi->mx3fb;
> @@ -414,6 +407,17 @@ static int sdc_set_window_pos(struct mx3fb_data *mx3fb, enum ipu_channel channel
>  	return 0;
>  }
>  
> +static void set_di_bus_mapping(struct mx3fb_data *mx3fb, struct fb_bitfield *bf,
> +			      unsigned long reg)
> +{
> +	u32 val = 0xffff;
> +
> +	val &= ~(((1 << (bf->length * 2)) - 1) << (16 - (bf->length * 2)));
> +	val |= ((bf->offset + bf->length - 1)) << 16;
> +	pr_debug("b: %d %d 0x%08x\n", bf->offset, bf->length, val);
> +	mx3fb_write_reg(mx3fb, val, reg);
> +}
> +
>  /**
>   * sdc_init_panel() - initialize a synchronous LCD panel.
>   * @mx3fb:		mx3fb context.
> @@ -421,7 +425,6 @@ static int sdc_set_window_pos(struct mx3fb_data *mx3fb, enum ipu_channel channel
>   * @pixel_clk:		desired pixel clock frequency in Hz.
>   * @width:		width of panel in pixels.
>   * @height:		height of panel in pixels.
> - * @pixel_fmt:		pixel format of buffer as FOURCC ASCII code.
>   * @h_start_width:	number of pixel clocks between the HSYNC signal pulse
>   *			and the start of valid data.
>   * @h_sync_width:	width of the HSYNC signal in units of pixel clocks.
> @@ -438,7 +441,6 @@ static int sdc_set_window_pos(struct mx3fb_data *mx3fb, enum ipu_channel channel
>  static int sdc_init_panel(struct mx3fb_data *mx3fb, enum ipu_panel panel,
>  			  uint32_t pixel_clk,
>  			  uint16_t width, uint16_t height,
> -			  enum pixel_fmt pixel_fmt,
>  			  uint16_t h_start_width, uint16_t h_sync_width,
>  			  uint16_t h_end_width, uint16_t v_start_width,
>  			  uint16_t v_sync_width, uint16_t v_end_width,
> @@ -450,6 +452,9 @@ static int sdc_init_panel(struct mx3fb_data *mx3fb, enum ipu_panel panel,
>  	uint32_t div;
>  	struct clk *ipu_clk;
>  
> +	struct device *dev = mx3fb->dev;
> +	struct mx3fb_platform_data *mx3fb_pdata = dev->platform_data;
> +
>  	dev_dbg(mx3fb->dev, "panel size = %d x %d", width, height);
>  
>  	if (v_sync_width == 0 || h_sync_width == 0)
> @@ -536,36 +541,18 @@ static int sdc_init_panel(struct mx3fb_data *mx3fb, enum ipu_panel panel,
>  		sig.Vsync_pol << DI_D3_VSYNC_POL_SHIFT;
>  	mx3fb_write_reg(mx3fb, old_conf, DI_DISP_SIG_POL);
>  
> -	switch (pixel_fmt) {
> -	case IPU_PIX_FMT_RGB24:
> -		mx3fb_write_reg(mx3fb, di_mappings[0], DI_DISP3_B0_MAP);
> -		mx3fb_write_reg(mx3fb, di_mappings[1], DI_DISP3_B1_MAP);
> -		mx3fb_write_reg(mx3fb, di_mappings[2], DI_DISP3_B2_MAP);
> -		mx3fb_write_reg(mx3fb, mx3fb_read_reg(mx3fb, DI_DISP_ACC_CC) |
> -			     ((di_mappings[3] - 1) << 12), DI_DISP_ACC_CC);
> -		break;
> -	case IPU_PIX_FMT_RGB666:
> -		mx3fb_write_reg(mx3fb, di_mappings[4], DI_DISP3_B0_MAP);
> -		mx3fb_write_reg(mx3fb, di_mappings[5], DI_DISP3_B1_MAP);
> -		mx3fb_write_reg(mx3fb, di_mappings[6], DI_DISP3_B2_MAP);
> -		mx3fb_write_reg(mx3fb, mx3fb_read_reg(mx3fb, DI_DISP_ACC_CC) |
> -			     ((di_mappings[7] - 1) << 12), DI_DISP_ACC_CC);
> -		break;
> -	case IPU_PIX_FMT_BGR666:
> -		mx3fb_write_reg(mx3fb, di_mappings[8], DI_DISP3_B0_MAP);
> -		mx3fb_write_reg(mx3fb, di_mappings[9], DI_DISP3_B1_MAP);
> -		mx3fb_write_reg(mx3fb, di_mappings[10], DI_DISP3_B2_MAP);
> -		mx3fb_write_reg(mx3fb, mx3fb_read_reg(mx3fb, DI_DISP_ACC_CC) |
> -			     ((di_mappings[11] - 1) << 12), DI_DISP_ACC_CC);
> -		break;
> -	default:
> -		mx3fb_write_reg(mx3fb, di_mappings[12], DI_DISP3_B0_MAP);
> -		mx3fb_write_reg(mx3fb, di_mappings[13], DI_DISP3_B1_MAP);
> -		mx3fb_write_reg(mx3fb, di_mappings[14], DI_DISP3_B2_MAP);
> -		mx3fb_write_reg(mx3fb, mx3fb_read_reg(mx3fb, DI_DISP_ACC_CC) |
> -			     ((di_mappings[15] - 1) << 12), DI_DISP_ACC_CC);
> -		break;
> -	}
> +	/*
> +	 * setup the pixel mapping to the display. This seems independent
> +	 * from the internal video mode in use
> +	 */
> +	set_di_bus_mapping(mx3fb, &mx3fb_pdata->blue, DI_DISP3_B0_MAP);
> +	set_di_bus_mapping(mx3fb, &mx3fb_pdata->green, DI_DISP3_B1_MAP);
> +	set_di_bus_mapping(mx3fb, &mx3fb_pdata->red, DI_DISP3_B2_MAP);
> +
> +	/* the TFT displays expecting one pixel per clock */
> +	reg = mx3fb_read_reg(mx3fb, DI_DISP_ACC_CC) & ~(3 << 12);
> +	reg |= 0 << 12;	/* 0 means one clock */
> +	mx3fb_write_reg(mx3fb, reg, DI_DISP_ACC_CC);
>  
>  	spin_unlock_irqrestore(&mx3fb->lock, lock_flags);
>  
> @@ -776,8 +763,6 @@ static int __set_par(struct fb_info *fbi, bool lock)
>  		if (sdc_init_panel(mx3fb, mode,
>  				   (PICOS2KHZ(fbi->var.pixclock)) * 1000UL,
>  				   fbi->var.xres, fbi->var.yres,
> -				   (fbi->var.sync & FB_SYNC_SWAP_RGB) ?
> -				   IPU_PIX_FMT_BGR666 : IPU_PIX_FMT_RGB666,
>  				   fbi->var.left_margin,
>  				   fbi->var.hsync_len,
>  				   fbi->var.right_margin +
> @@ -1367,6 +1352,20 @@ static int init_fb_chan(struct mx3fb_data *mx3fb, struct idmac_channel *ichan)
>  		goto emode;
>  	}
>  
> +	if (mx3fb_pdata->red.length == 0 &&
> +	    mx3fb_pdata->green.length == 0 &&
> +	    mx3fb_pdata->blue.length == 0) {
> +
> +		mx3fb_pdata->red.length = 6;
> +		mx3fb_pdata->red.offset = 12;
> +
> +		mx3fb_pdata->green.length = 6;
> +		mx3fb_pdata->green.offset = 6;
> +
> +		mx3fb_pdata->blue.length = 6;
> +		mx3fb_pdata->blue.offset = 0;
> +	}
> +
>  	fb_videomode_to_modelist(mode, num_modes, &fbi->modelist);
>  
>  	/* Default Y virtual size is 2x panel size */
> -- 
> 1.7.2.3
> 
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/