Skip to contentSkip to conversation

mx35_3ds: Add the TFT display to the board file

PATCH Superseded Michael Grzeschik 4 participants 13 messages Aug 24, 2010 – Sep 22, 2010

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 mx35_3ds: Add the TFT display to the board file

From: Juergen Beisert <[email protected]>

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.

---
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(-)
arch/arm/mach-mx3/mach-mx35_3ds.c +72 -0
index 1dd7baa..2e7468e 100644
@@ -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)
Signature · 1 line
1.7.1
#1 lore raw

PATCH v2 1/1 mx35_3ds: Add the TFT display to the board file

From: Juergen Beisert <[email protected]>

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.

---
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(-)
arch/arm/mach-mx3/mach-mx35_3ds.c +69 -0
index 1dd7baa..60d5753 100644
@@ -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)
Signature · 1 line
1.7.1
#2 lore raw

PATCH 1/1 mx35_3ds: Add the TFT display to the board file

From: Juergen Beisert <[email protected]>

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.

---
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(-)
arch/arm/mach-mx3/mach-mx35_3ds.c +69 -0
index 1dd7baa..60d5753 100644
@@ -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)
Signature · 1 line
1.7.2.3
#3 lore raw

cover fix mx35fb display mappings

In reply to 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(-)
#4 lore raw

cover fix mx35fb display mappings

Hi Michael,

Le 24/08/2010 17:27, Michael Grzeschik a ?crit :

Quoted 5 lines

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

#5 lore raw

cover fix mx35fb display mappings

Hi Eric,

Quoted 9 lines · Eric B?nard

Hi Michael,

Le 24/08/2010 17:27, Michael Grzeschik a ?crit :

Quoted 5 lines

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

Signature · 4 lines
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 |
#6 lore raw

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

In reply to Michael Grzeschik

From: Juergen Beisert <[email protected]>

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.

---
 arch/arm/plat-mxc/include/mach/mx3fb.h |    5 +++
 drivers/video/mx3fb.c                  |   62 +++++++++++++-------------------
 2 files changed, 30 insertions(+), 37 deletions(-)
arch/arm/plat-mxc/include/mach/mx3fb.h +5 -0
index ac24c5c..93bd032 100644
@@ -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
drivers/video/mx3fb.c +25 -37

80 lines not on this page · raw message

Signature · 1 line
1.7.1
#7 lore raw

PATCH v2 1/1 Separate display's pixel configuration from video mode

From: Juergen Beisert <[email protected]>

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.

---
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(-)
arch/arm/plat-mxc/include/mach/mx3fb.h +5 -0
index ac24c5c..93bd032 100644
@@ -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
drivers/video/mx3fb.c +39 -41

122 lines not on this page · raw message

Signature · 1 line
1.7.1
#8 lore raw

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

From: Juergen Beisert <[email protected]>

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.

---
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(-)
arch/arm/plat-mxc/include/mach/mx3fb.h +5 -0
index ac24c5c..93bd032 100644
@@ -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
drivers/video/mx3fb.c +39 -41

122 lines not on this page · raw message

Signature · 1 line
1.7.2.3
#9 lore raw

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

Quoted 116 lines · Michael Grzeschik

From: Juergen Beisert <[email protected]>

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.

---
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(-)
arch/arm/plat-mxc/include/mach/mx3fb.h +5 -0
index ac24c5c..93bd032 100644
@@ -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
drivers/video/mx3fb.c +3 -32
index 658f10a..5e552a7 100644
@@ -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:

drivers/video/mx3fb.c static int sdc_init_panel(struct mx3fb_data *mx3fb, enum ipu_panel panel, · line 535

Quoted 10 lines
drivers/video/mx3fb.c +3 -7
-		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

Quoted 4 lines
drivers/video/mx3fb.c +4 -0
+	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 */

drivers/video/mx3fb.c static int sdc_init_panel(struct mx3fb_data *mx3fb, enum ipu_panel panel, · line 549

Quoted 10 lines
drivers/video/mx3fb.c +10 -0
+
+	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)

Quoted 25 lines
drivers/video/mx3fb.c +9 -2
+
+	/* 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) {
+

drivers/video/mx3fb.c static int init_fb_chan(struct mx3fb_data *mx3fb, struct idmac_channel *ichan) · line 1367

Quoted 10 lines
drivers/video/mx3fb.c +10 -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"

Quoted 5 lines
drivers/video/mx3fb.c +0 -0
 	fb_videomode_to_modelist(mode, num_modes, &fbi->modelist);
 
 	/* Default Y virtual size is 2x panel size */
Signature · 1 line
1.7.2.3

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

#10 lore raw

PATCH v3 1/1 Separate display's pixel configuration from video mode

From: Juergen Beisert <[email protected]>

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.

---
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(-)
arch/arm/plat-mxc/include/mach/mx3fb.h +5 -0
index ac24c5c..93bd032 100644
@@ -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
drivers/video/mx3fb.c +40 -41

129 lines not on this page · raw message

Signature · 1 line
1.7.2.3
#11 lore raw

PATCH v3 1/1 Separate display's pixel configuration from video mode

Quoted 13 lines · Michael Grzeschik

From: Juergen Beisert <[email protected]>

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

Quoted 7 lines

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.

Quoted 174 lines

---
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(-)
arch/arm/plat-mxc/include/mach/mx3fb.h +5 -0
index ac24c5c..93bd032 100644
@@ -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
drivers/video/mx3fb.c +40 -41
index 658f10a..ca20da8 100644
@@ -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 */
Signature · 1 line
1.7.2.3

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

#12 lore raw

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

In reply to Michael Grzeschik

From: Juergen Beisert <[email protected]>

This platform uses a 18 bpp display.

---
 arch/arm/mach-mx3/mach-mx35_3ds.c |   15 +++++++++++++++
 1 files changed, 15 insertions(+), 0 deletions(-)
arch/arm/mach-mx3/mach-mx35_3ds.c +15 -0
index 60d5753..ba65463 100644
@@ -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 = {
Signature · 1 line
1.7.1