[PATCH 0/4] MIPI DSI support for SH-mobile, common header, switch

[PATCH 0/4] MIPI DSI support for SH-mobile, common header, switch

[Guennadi Liakhovetski]

This patch series adds support for the MIPI DSI unit on sh-mobile SoCs, to 
eliminate redundancy a common mipi_dsi.h header is added. The SH-mobile 
part depends on pending clock and INTC patches by Magnus.

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

[PATCH 1/4] fbdev: add a MIPI DSI header

[Guennadi Liakhovetski]

This header adds defines for MIPI DSI and DCS commands and data formats. See
http://www.mipi.org/ for details.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 include/video/mipi_dsi.h |   99 ++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 99 insertions(+), 0 deletions(-)
 create mode 100644 include/video/mipi_dsi.h

diff --git a/include/video/mipi_dsi.h b/include/video/mipi_dsi.h
new file mode 100644
index 0000000..5d3a6d6
--- /dev/null
+++ b/include/video/mipi_dsi.h
@@ -0,0 +1,99 @@
+/*
+ * Mobile Industry Processor Interface (MIPI(R)) defines
+ *
+ * Copyright (C) 2010 Guennadi Liakhovetski <g.liakhovetski@gmx.de>
+ * Copyright (C) 2006 Nokia Corporation
+ * Author: Imre Deak <imre.deak@nokia.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#ifndef MIPI_DSI_H
+#define MIPI_DSI_H
+
+#include <linux/types.h>
+
+enum mipi_dsi_cmd {
+	MIPI_DSI_DCS_SHORT_WRITE			= 5,
+	MIPI_DSI_DCS_SHORT_WRITE_PARAM			= 0x15,
+	MIPI_DSI_COLOR_MODE_OFF				= 2,
+	MIPI_DSI_COLOR_MODE_ON				= 0x12,
+	MIPI_DSI_SHUTDOWN_PERIPHERAL			= 0x22,
+	MIPI_DSI_TURN_ON_PERIPHERAL			= 0x32,
+	MIPI_DSI_GENERIC_SHORT_WRITE_0_PARAM		= 3,
+	MIPI_DSI_GENERIC_SHORT_WRITE_1_PARAM		= 0x13,
+	MIPI_DSI_GENERIC_SHORT_WRITE_2_PARAM		= 0x23,
+	MIPI_DSI_GENERIC_READ_REQUEST_0_PARAM		= 4,
+	MIPI_DSI_GENERIC_READ_REQUEST_1_PARAM		= 0x14,
+	MIPI_DSI_GENERIC_READ_REQUEST_2_PARAM		= 0x24,
+	MIPI_DSI_DCS_LONG_WRITE				= 0x39,
+	MIPI_DSI_SET_MAXIMUM_RETURN_PACKET_SIZE		= 0x37,
+	MIPI_DSI_NULL_PACKET				= 9,
+	MIPI_DSI_BLANKING_PACKET			= 0x19,
+	MIPI_DSI_GENERIC_LONG_WRITE			= 0x29,
+	MIPI_DSI_LOOSELY_PACKED_PIXEL_STREAM_YCBCR20	= 0xc,
+	MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR24		= 0x1c,
+	MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR16		= 0x2c,
+	MIPI_DSI_PACKED_PIXEL_STREAM_30			= 0xd,
+	MIPI_DSI_PACKED_PIXEL_STREAM_36			= 0x1d,
+	MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR12		= 0x3d,
+	MIPI_DSI_PACKED_PIXEL_STREAM_16			= 0xe,
+	MIPI_DSI_PACKED_PIXEL_STREAM_18			= 0x1e,
+	MIPI_DSI_PIXEL_STREAM_3BYTE_18			= 0x2e,
+	MIPI_DSI_PACKED_PIXEL_STREAM_24			= 0x3e,
+};
+
+enum mipi_dcs_cmd {
+	MIPI_DCS_NOP			= 0x00,
+	MIPI_DCS_SOFT_RESET		= 0x01,
+	MIPI_DCS_GET_DISPLAY_ID		= 0x04,
+	MIPI_DCS_GET_RED_CHANNEL	= 0x06,
+	MIPI_DCS_GET_GREEN_CHANNEL	= 0x07,
+	MIPI_DCS_GET_BLUE_CHANNEL	= 0x08,
+	MIPI_DCS_GET_DISPLAY_STATUS	= 0x09,
+	MIPI_DCS_GET_POWER_MODE		= 0x0A,
+	MIPI_DCS_GET_ADDRESS_MODE	= 0x0B,
+	MIPI_DCS_GET_PIXEL_FORMAT	= 0x0C,
+	MIPI_DCS_GET_DISPLAY_MODE	= 0x0D,
+	MIPI_DCS_GET_SIGNAL_MODE	= 0x0E,
+	MIPI_DCS_GET_DIAGNOSTIC_RESULT	= 0x0F,
+	MIPI_DCS_ENTER_SLEEP_MODE	= 0x10,
+	MIPI_DCS_EXIT_SLEEP_MODE	= 0x11,
+	MIPI_DCS_ENTER_PARTIAL_MODE	= 0x12,
+	MIPI_DCS_ENTER_NORMAL_MODE	= 0x13,
+	MIPI_DCS_EXIT_INVERT_MODE	= 0x20,
+	MIPI_DCS_ENTER_INVERT_MODE	= 0x21,
+	MIPI_DCS_SET_GAMMA_CURVE	= 0x26,
+	MIPI_DCS_SET_DISPLAY_OFF	= 0x28,
+	MIPI_DCS_SET_DISPLAY_ON		= 0x29,
+	MIPI_DCS_SET_COLUMN_ADDRESS	= 0x2A,
+	MIPI_DCS_SET_PAGE_ADDRESS	= 0x2B,
+	MIPI_DCS_WRITE_MEMORY_START	= 0x2C,
+	MIPI_DCS_WRITE_LUT		= 0x2D,
+	MIPI_DCS_READ_MEMORY_START	= 0x2E,
+	MIPI_DCS_SET_PARTIAL_AREA	= 0x30,
+	MIPI_DCS_SET_SCROLL_AREA	= 0x33,
+	MIPI_DCS_SET_TEAR_OFF		= 0x34,
+	MIPI_DCS_SET_TEAR_ON		= 0x35,
+	MIPI_DCS_SET_ADDRESS_MODE	= 0x36,
+	MIPI_DCS_SET_SCROLL_START	= 0x37,
+	MIPI_DCS_EXIT_IDLE_MODE		= 0x38,
+	MIPI_DCS_ENTER_IDLE_MODE	= 0x39,
+	MIPI_DCS_SET_PIXEL_FORMAT	= 0x3A,
+	MIPI_DCS_WRITE_MEMORY_CONTINUE	= 0x3C,
+	MIPI_DCS_READ_MEMORY_CONTINUE	= 0x3E,
+	MIPI_DCS_SET_TEAR_SCANLINE	= 0x44,
+	MIPI_DCS_GET_SCANLINE		= 0x45,
+	MIPI_DCS_READ_DDB_START		= 0xA1,
+	MIPI_DCS_READ_DDB_CONTINUE	= 0xA8,
+};
+
+#define MIPI_DCS_PIXEL_FMT_24BIT	7
+#define MIPI_DCS_PIXEL_FMT_18BIT	6
+#define MIPI_DCS_PIXEL_FMT_16BIT	5
+#define MIPI_DCS_PIXEL_FMT_12BIT	3
+#define MIPI_DCS_PIXEL_FMT_8BIT		2
+#define MIPI_DCS_PIXEL_FMT_3BIT		1
+
+#endif
-- 
1.6.2.4

Re: [PATCH 1/4] fbdev: add a MIPI DSI header

[Damian]

(4:59), Guennadi Liakhovetski wrote:
> This header adds defines for MIPI DSI and DCS commands and data formats. See
> http://www.mipi.org/ for details.
> 
> Signed-off-by: Guennadi Liakhovetski<g.liakhovetski@gmx.de>
Console framebuffer tested on sh-2.6 tree and sh/dmaengine branch with
the necessary clock and intc patches applied.
Boot logo and framebuffer console displayed with no problems or unusual
effects.

Tested-by: Damian Hobson-Garcia <dhobsong@igel.co.jp>
> ---
>   include/video/mipi_dsi.h |   99 ++++++++++++++++++++++++++++++++++++++++++++++
>   1 files changed, 99 insertions(+), 0 deletions(-)
>   create mode 100644 include/video/mipi_dsi.h
> 
> diff --git a/include/video/mipi_dsi.h b/include/video/mipi_dsi.h
> new file mode 100644
> index 0000000..5d3a6d6
> --- /dev/null
> +++ b/include/video/mipi_dsi.h
> @@ -0,0 +1,99 @@
> +/*
> + * Mobile Industry Processor Interface (MIPI(R)) defines
> + *
> + * Copyright (C) 2010 Guennadi Liakhovetski<g.liakhovetski@gmx.de>
> + * Copyright (C) 2006 Nokia Corporation
> + * Author: Imre Deak<imre.deak@nokia.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +#ifndef MIPI_DSI_H
> +#define MIPI_DSI_H
> +
> +#include<linux/types.h>
> +
> +enum mipi_dsi_cmd {
> +	MIPI_DSI_DCS_SHORT_WRITE			= 5,
> +	MIPI_DSI_DCS_SHORT_WRITE_PARAM			= 0x15,
> +	MIPI_DSI_COLOR_MODE_OFF				= 2,
> +	MIPI_DSI_COLOR_MODE_ON				= 0x12,
> +	MIPI_DSI_SHUTDOWN_PERIPHERAL			= 0x22,
> +	MIPI_DSI_TURN_ON_PERIPHERAL			= 0x32,
> +	MIPI_DSI_GENERIC_SHORT_WRITE_0_PARAM		= 3,
> +	MIPI_DSI_GENERIC_SHORT_WRITE_1_PARAM		= 0x13,
> +	MIPI_DSI_GENERIC_SHORT_WRITE_2_PARAM		= 0x23,
> +	MIPI_DSI_GENERIC_READ_REQUEST_0_PARAM		= 4,
> +	MIPI_DSI_GENERIC_READ_REQUEST_1_PARAM		= 0x14,
> +	MIPI_DSI_GENERIC_READ_REQUEST_2_PARAM		= 0x24,
> +	MIPI_DSI_DCS_LONG_WRITE				= 0x39,
> +	MIPI_DSI_SET_MAXIMUM_RETURN_PACKET_SIZE		= 0x37,
> +	MIPI_DSI_NULL_PACKET				= 9,
> +	MIPI_DSI_BLANKING_PACKET			= 0x19,
> +	MIPI_DSI_GENERIC_LONG_WRITE			= 0x29,
> +	MIPI_DSI_LOOSELY_PACKED_PIXEL_STREAM_YCBCR20	= 0xc,
> +	MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR24		= 0x1c,
> +	MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR16		= 0x2c,
> +	MIPI_DSI_PACKED_PIXEL_STREAM_30			= 0xd,
> +	MIPI_DSI_PACKED_PIXEL_STREAM_36			= 0x1d,
> +	MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR12		= 0x3d,
> +	MIPI_DSI_PACKED_PIXEL_STREAM_16			= 0xe,
> +	MIPI_DSI_PACKED_PIXEL_STREAM_18			= 0x1e,
> +	MIPI_DSI_PIXEL_STREAM_3BYTE_18			= 0x2e,
> +	MIPI_DSI_PACKED_PIXEL_STREAM_24			= 0x3e,
> +};
> +
> +enum mipi_dcs_cmd {
> +	MIPI_DCS_NOP			= 0x00,
> +	MIPI_DCS_SOFT_RESET		= 0x01,
> +	MIPI_DCS_GET_DISPLAY_ID		= 0x04,
> +	MIPI_DCS_GET_RED_CHANNEL	= 0x06,
> +	MIPI_DCS_GET_GREEN_CHANNEL	= 0x07,
> +	MIPI_DCS_GET_BLUE_CHANNEL	= 0x08,
> +	MIPI_DCS_GET_DISPLAY_STATUS	= 0x09,
> +	MIPI_DCS_GET_POWER_MODE		= 0x0A,
> +	MIPI_DCS_GET_ADDRESS_MODE	= 0x0B,
> +	MIPI_DCS_GET_PIXEL_FORMAT	= 0x0C,
> +	MIPI_DCS_GET_DISPLAY_MODE	= 0x0D,
> +	MIPI_DCS_GET_SIGNAL_MODE	= 0x0E,
> +	MIPI_DCS_GET_DIAGNOSTIC_RESULT	= 0x0F,
> +	MIPI_DCS_ENTER_SLEEP_MODE	= 0x10,
> +	MIPI_DCS_EXIT_SLEEP_MODE	= 0x11,
> +	MIPI_DCS_ENTER_PARTIAL_MODE	= 0x12,
> +	MIPI_DCS_ENTER_NORMAL_MODE	= 0x13,
> +	MIPI_DCS_EXIT_INVERT_MODE	= 0x20,
> +	MIPI_DCS_ENTER_INVERT_MODE	= 0x21,
> +	MIPI_DCS_SET_GAMMA_CURVE	= 0x26,
> +	MIPI_DCS_SET_DISPLAY_OFF	= 0x28,
> +	MIPI_DCS_SET_DISPLAY_ON		= 0x29,
> +	MIPI_DCS_SET_COLUMN_ADDRESS	= 0x2A,
> +	MIPI_DCS_SET_PAGE_ADDRESS	= 0x2B,
> +	MIPI_DCS_WRITE_MEMORY_START	= 0x2C,
> +	MIPI_DCS_WRITE_LUT		= 0x2D,
> +	MIPI_DCS_READ_MEMORY_START	= 0x2E,
> +	MIPI_DCS_SET_PARTIAL_AREA	= 0x30,
> +	MIPI_DCS_SET_SCROLL_AREA	= 0x33,
> +	MIPI_DCS_SET_TEAR_OFF		= 0x34,
> +	MIPI_DCS_SET_TEAR_ON		= 0x35,
> +	MIPI_DCS_SET_ADDRESS_MODE	= 0x36,
> +	MIPI_DCS_SET_SCROLL_START	= 0x37,
> +	MIPI_DCS_EXIT_IDLE_MODE		= 0x38,
> +	MIPI_DCS_ENTER_IDLE_MODE	= 0x39,
> +	MIPI_DCS_SET_PIXEL_FORMAT	= 0x3A,
> +	MIPI_DCS_WRITE_MEMORY_CONTINUE	= 0x3C,
> +	MIPI_DCS_READ_MEMORY_CONTINUE	= 0x3E,
> +	MIPI_DCS_SET_TEAR_SCANLINE	= 0x44,
> +	MIPI_DCS_GET_SCANLINE		= 0x45,
> +	MIPI_DCS_READ_DDB_START		= 0xA1,
> +	MIPI_DCS_READ_DDB_CONTINUE	= 0xA8,
> +};
> +
> +#define MIPI_DCS_PIXEL_FMT_24BIT	7
> +#define MIPI_DCS_PIXEL_FMT_18BIT	6
> +#define MIPI_DCS_PIXEL_FMT_16BIT	5
> +#define MIPI_DCS_PIXEL_FMT_12BIT	3
> +#define MIPI_DCS_PIXEL_FMT_8BIT		2
> +#define MIPI_DCS_PIXEL_FMT_3BIT		1
> +
> +#endif


-- 
Damian Hobson-Garcia
IGEL Co.,Ltd
http://www.igel.co.jp

Re: [PATCH 1/4] fbdev: add a MIPI DSI header

[Tomi Valkeinen]

Hi,

On Fri, 2010-05-07 at 11:07 +0200, ext Guennadi Liakhovetski wrote:
> This header adds defines for MIPI DSI and DCS commands and data formats. See
> http://www.mipi.org/ for details.

Did you notice that I have implemented MIPI DSI command mode support for
OMAP processors? It's located in drivers/video/omap2/dss/dsi.c and one
DSI panel driver is located in drivers/video/omap2/displays/panel-taal.c

> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> ---
>  include/video/mipi_dsi.h |   99 ++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 99 insertions(+), 0 deletions(-)
>  create mode 100644 include/video/mipi_dsi.h
> 
> diff --git a/include/video/mipi_dsi.h b/include/video/mipi_dsi.h
> new file mode 100644
> index 0000000..5d3a6d6
> --- /dev/null
> +++ b/include/video/mipi_dsi.h
> @@ -0,0 +1,99 @@
> +/*
> + * Mobile Industry Processor Interface (MIPI(R)) defines

The file name is mipi_dsi.h, the comment talks about MIPI, and the file
contains defines for MIPI DSI and MIPI DCS. If you want the file to
contain defines about all MIPI specs, I think the file name should be
just mipi.h.

> + *
> + * Copyright (C) 2010 Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> + * Copyright (C) 2006 Nokia Corporation
> + * Author: Imre Deak <imre.deak@nokia.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +#ifndef MIPI_DSI_H
> +#define MIPI_DSI_H
> +
> +#include <linux/types.h>
> +
> +enum mipi_dsi_cmd {

I think these are DSI datatypes, not commands.

> +	MIPI_DSI_DCS_SHORT_WRITE			= 5,

Please use the same number format for all enums. So this should be 0x05.

> +	MIPI_DSI_DCS_SHORT_WRITE_PARAM			= 0x15,
> +	MIPI_DSI_COLOR_MODE_OFF				= 2,
> +	MIPI_DSI_COLOR_MODE_ON				= 0x12,
> +	MIPI_DSI_SHUTDOWN_PERIPHERAL			= 0x22,
> +	MIPI_DSI_TURN_ON_PERIPHERAL			= 0x32,
> +	MIPI_DSI_GENERIC_SHORT_WRITE_0_PARAM		= 3,
> +	MIPI_DSI_GENERIC_SHORT_WRITE_1_PARAM		= 0x13,
> +	MIPI_DSI_GENERIC_SHORT_WRITE_2_PARAM		= 0x23,
> +	MIPI_DSI_GENERIC_READ_REQUEST_0_PARAM		= 4,
> +	MIPI_DSI_GENERIC_READ_REQUEST_1_PARAM		= 0x14,
> +	MIPI_DSI_GENERIC_READ_REQUEST_2_PARAM		= 0x24,
> +	MIPI_DSI_DCS_LONG_WRITE				= 0x39,
> +	MIPI_DSI_SET_MAXIMUM_RETURN_PACKET_SIZE		= 0x37,
> +	MIPI_DSI_NULL_PACKET				= 9,
> +	MIPI_DSI_BLANKING_PACKET			= 0x19,
> +	MIPI_DSI_GENERIC_LONG_WRITE			= 0x29,
> +	MIPI_DSI_LOOSELY_PACKED_PIXEL_STREAM_YCBCR20	= 0xc,
> +	MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR24		= 0x1c,
> +	MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR16		= 0x2c,
> +	MIPI_DSI_PACKED_PIXEL_STREAM_30			= 0xd,
> +	MIPI_DSI_PACKED_PIXEL_STREAM_36			= 0x1d,
> +	MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR12		= 0x3d,
> +	MIPI_DSI_PACKED_PIXEL_STREAM_16			= 0xe,
> +	MIPI_DSI_PACKED_PIXEL_STREAM_18			= 0x1e,
> +	MIPI_DSI_PIXEL_STREAM_3BYTE_18			= 0x2e,
> +	MIPI_DSI_PACKED_PIXEL_STREAM_24			= 0x3e,
> +};
> +
> +enum mipi_dcs_cmd {

While the MIPI specs define a certain set of commands, the real panels
usually implement also their own custom commands. That's why I don't
think enum is a good choice here.

 Tomi

Re: [PATCH 1/4] fbdev: add a MIPI DSI header

[Guennadi Liakhovetski]

Hi Tomi

On Wed, 19 May 2010, Tomi Valkeinen wrote:

> Hi,
> 
> On Fri, 2010-05-07 at 11:07 +0200, ext Guennadi Liakhovetski wrote:
> > This header adds defines for MIPI DSI and DCS commands and data formats. See
> > http://www.mipi.org/ for details.
> 
> Did you notice that I have implemented MIPI DSI command mode support for
> OMAP processors? It's located in drivers/video/omap2/dss/dsi.c and one
> DSI panel driver is located in drivers/video/omap2/displays/panel-taal.c

No, didn't see those, I grepped for MIPI...

> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > ---
> >  include/video/mipi_dsi.h |   99 ++++++++++++++++++++++++++++++++++++++++++++++
> >  1 files changed, 99 insertions(+), 0 deletions(-)
> >  create mode 100644 include/video/mipi_dsi.h
> > 
> > diff --git a/include/video/mipi_dsi.h b/include/video/mipi_dsi.h
> > new file mode 100644
> > index 0000000..5d3a6d6
> > --- /dev/null
> > +++ b/include/video/mipi_dsi.h
> > @@ -0,0 +1,99 @@
> > +/*
> > + * Mobile Industry Processor Interface (MIPI(R)) defines
> 
> The file name is mipi_dsi.h, the comment talks about MIPI, and the file
> contains defines for MIPI DSI and MIPI DCS. If you want the file to
> contain defines about all MIPI specs, I think the file name should be
> just mipi.h.

No, the header is just intended for DSI, the comment could be made more 
specific, yes, but DCS is a part of DSI, isn't it?

> > + *
> > + * Copyright (C) 2010 Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > + * Copyright (C) 2006 Nokia Corporation
> > + * Author: Imre Deak <imre.deak@nokia.com>
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + */
> > +#ifndef MIPI_DSI_H
> > +#define MIPI_DSI_H
> > +
> > +#include <linux/types.h>
> > +
> > +enum mipi_dsi_cmd {
> 
> I think these are DSI datatypes, not commands.

hm, "write," "shut down," "mode off," "mode on" sound like commands to me, 
and I think I saw them called commands in the spec, but yes, some of them 
are just packet or data types... I'll double-check, thanks.

> > +	MIPI_DSI_DCS_SHORT_WRITE			= 5,
> 
> Please use the same number format for all enums. So this should be 0x05.

Where does this requirement come from? Is it in CodingStyle?;)

> > +	MIPI_DSI_DCS_SHORT_WRITE_PARAM			= 0x15,
> > +	MIPI_DSI_COLOR_MODE_OFF				= 2,
> > +	MIPI_DSI_COLOR_MODE_ON				= 0x12,
> > +	MIPI_DSI_SHUTDOWN_PERIPHERAL			= 0x22,
> > +	MIPI_DSI_TURN_ON_PERIPHERAL			= 0x32,
> > +	MIPI_DSI_GENERIC_SHORT_WRITE_0_PARAM		= 3,
> > +	MIPI_DSI_GENERIC_SHORT_WRITE_1_PARAM		= 0x13,
> > +	MIPI_DSI_GENERIC_SHORT_WRITE_2_PARAM		= 0x23,
> > +	MIPI_DSI_GENERIC_READ_REQUEST_0_PARAM		= 4,
> > +	MIPI_DSI_GENERIC_READ_REQUEST_1_PARAM		= 0x14,
> > +	MIPI_DSI_GENERIC_READ_REQUEST_2_PARAM		= 0x24,
> > +	MIPI_DSI_DCS_LONG_WRITE				= 0x39,
> > +	MIPI_DSI_SET_MAXIMUM_RETURN_PACKET_SIZE		= 0x37,
> > +	MIPI_DSI_NULL_PACKET				= 9,
> > +	MIPI_DSI_BLANKING_PACKET			= 0x19,
> > +	MIPI_DSI_GENERIC_LONG_WRITE			= 0x29,
> > +	MIPI_DSI_LOOSELY_PACKED_PIXEL_STREAM_YCBCR20	= 0xc,
> > +	MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR24		= 0x1c,
> > +	MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR16		= 0x2c,
> > +	MIPI_DSI_PACKED_PIXEL_STREAM_30			= 0xd,
> > +	MIPI_DSI_PACKED_PIXEL_STREAM_36			= 0x1d,
> > +	MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR12		= 0x3d,
> > +	MIPI_DSI_PACKED_PIXEL_STREAM_16			= 0xe,
> > +	MIPI_DSI_PACKED_PIXEL_STREAM_18			= 0x1e,
> > +	MIPI_DSI_PIXEL_STREAM_3BYTE_18			= 0x2e,
> > +	MIPI_DSI_PACKED_PIXEL_STREAM_24			= 0x3e,
> > +};
> > +
> > +enum mipi_dcs_cmd {
> 
> While the MIPI specs define a certain set of commands, the real panels
> usually implement also their own custom commands. That's why I don't
> think enum is a good choice here.

Yes, that's a valid point, I'll have to think about it more.

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

Re: [PATCH 1/4] fbdev: add a MIPI DSI header

[Tomi Valkeinen]

Hi,

On Wed, 2010-05-19 at 10:08 +0200, ext Guennadi Liakhovetski wrote:
> Hi Tomi
> 
> On Wed, 19 May 2010, Tomi Valkeinen wrote:

> > The file name is mipi_dsi.h, the comment talks about MIPI, and the file
> > contains defines for MIPI DSI and MIPI DCS. If you want the file to
> > contain defines about all MIPI specs, I think the file name should be
> > just mipi.h.
> 
> No, the header is just intended for DSI, the comment could be made more 
> specific, yes, but DCS is a part of DSI, isn't it?

No, DCS is a spec of its own. DCS can be used on non-DSI displays also.
For example N800 and N900 use DCS commands, but they are not DSI
displays.

> > > + *
> > > + * Copyright (C) 2010 Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > > + * Copyright (C) 2006 Nokia Corporation
> > > + * Author: Imre Deak <imre.deak@nokia.com>
> > > + *
> > > + * This program is free software; you can redistribute it and/or modify
> > > + * it under the terms of the GNU General Public License version 2 as
> > > + * published by the Free Software Foundation.
> > > + */
> > > +#ifndef MIPI_DSI_H
> > > +#define MIPI_DSI_H
> > > +
> > > +#include <linux/types.h>
> > > +
> > > +enum mipi_dsi_cmd {
> > 
> > I think these are DSI datatypes, not commands.
> 
> hm, "write," "shut down," "mode off," "mode on" sound like commands to me, 
> and I think I saw them called commands in the spec, but yes, some of them 
> are just packet or data types... I'll double-check, thanks.

Well, true, this is not a clear thing. MIPI DSI spec talks about "turn
on peripheral command". But the numbers are DSI data types, according to
the spec, and not all of them are commands.

> > > +	MIPI_DSI_DCS_SHORT_WRITE			= 5,
> > 
> > Please use the same number format for all enums. So this should be 0x05.
> 
> Where does this requirement come from? Is it in CodingStyle?;)

I don't know, but mixing different formats looks ugly to me =).

> > > +	MIPI_DSI_DCS_SHORT_WRITE_PARAM			= 0x15,
> > > +	MIPI_DSI_COLOR_MODE_OFF				= 2,
> > > +	MIPI_DSI_COLOR_MODE_ON				= 0x12,
> > > +	MIPI_DSI_SHUTDOWN_PERIPHERAL			= 0x22,
> > > +	MIPI_DSI_TURN_ON_PERIPHERAL			= 0x32,
> > > +	MIPI_DSI_GENERIC_SHORT_WRITE_0_PARAM		= 3,
> > > +	MIPI_DSI_GENERIC_SHORT_WRITE_1_PARAM		= 0x13,
> > > +	MIPI_DSI_GENERIC_SHORT_WRITE_2_PARAM		= 0x23,
> > > +	MIPI_DSI_GENERIC_READ_REQUEST_0_PARAM		= 4,
> > > +	MIPI_DSI_GENERIC_READ_REQUEST_1_PARAM		= 0x14,
> > > +	MIPI_DSI_GENERIC_READ_REQUEST_2_PARAM		= 0x24,
> > > +	MIPI_DSI_DCS_LONG_WRITE				= 0x39,
> > > +	MIPI_DSI_SET_MAXIMUM_RETURN_PACKET_SIZE		= 0x37,
> > > +	MIPI_DSI_NULL_PACKET				= 9,
> > > +	MIPI_DSI_BLANKING_PACKET			= 0x19,
> > > +	MIPI_DSI_GENERIC_LONG_WRITE			= 0x29,
> > > +	MIPI_DSI_LOOSELY_PACKED_PIXEL_STREAM_YCBCR20	= 0xc,
> > > +	MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR24		= 0x1c,
> > > +	MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR16		= 0x2c,
> > > +	MIPI_DSI_PACKED_PIXEL_STREAM_30			= 0xd,
> > > +	MIPI_DSI_PACKED_PIXEL_STREAM_36			= 0x1d,
> > > +	MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR12		= 0x3d,
> > > +	MIPI_DSI_PACKED_PIXEL_STREAM_16			= 0xe,
> > > +	MIPI_DSI_PACKED_PIXEL_STREAM_18			= 0x1e,
> > > +	MIPI_DSI_PIXEL_STREAM_3BYTE_18			= 0x2e,
> > > +	MIPI_DSI_PACKED_PIXEL_STREAM_24			= 0x3e,
> > > +};
> > > +
> > > +enum mipi_dcs_cmd {
> > 
> > While the MIPI specs define a certain set of commands, the real panels
> > usually implement also their own custom commands. That's why I don't
> > think enum is a good choice here.
> 
> Yes, that's a valid point, I'll have to think about it more.

I think a simple solution would be to just use defines, and have
functions that take the command as u8. That's what the OMAP DSI driver
does. If you have better ideas, please share =).

The same applies for the DSI commands/datatypes, although I haven't seen
a panel that has custom DSI datatypes.

 Tomi

Re: [PATCH 1/4] fbdev: add a MIPI DSI header

[Ville Syrjälä]

On Wed, May 19, 2010 at 10:21:48AM +0200, Valkeinen Tomi (Nokia-D/Helsinki) wrote:
> Hi,
> 
> On Wed, 2010-05-19 at 10:08 +0200, ext Guennadi Liakhovetski wrote:
> > Hi Tomi
> > 
> > On Wed, 19 May 2010, Tomi Valkeinen wrote:
> > > > +	MIPI_DSI_DCS_SHORT_WRITE			= 5,
> > > 
> > > Please use the same number format for all enums. So this should be 0x05.
> > 
> > Where does this requirement come from? Is it in CodingStyle?;)
> 
> I don't know, but mixing different formats looks ugly to me =).

One case where it would perhaps make sense if you define some fields
with shifts and masks. Specifying shifts as decimal and masks as hex
is clearer IMO. But in this case I agree it just looks ugly and only
serves to confuse the reader as he tries to figure out if there's
some magic meaning to the different formats.

> 
> > > > +	MIPI_DSI_DCS_SHORT_WRITE_PARAM			= 0x15,
> > > > +	MIPI_DSI_COLOR_MODE_OFF				= 2,
> > > > +	MIPI_DSI_COLOR_MODE_ON				= 0x12,
> > > > +	MIPI_DSI_SHUTDOWN_PERIPHERAL			= 0x22,
> > > > +	MIPI_DSI_TURN_ON_PERIPHERAL			= 0x32,
> > > > +	MIPI_DSI_GENERIC_SHORT_WRITE_0_PARAM		= 3,
> > > > +	MIPI_DSI_GENERIC_SHORT_WRITE_1_PARAM		= 0x13,
> > > > +	MIPI_DSI_GENERIC_SHORT_WRITE_2_PARAM		= 0x23,
> > > > +	MIPI_DSI_GENERIC_READ_REQUEST_0_PARAM		= 4,
> > > > +	MIPI_DSI_GENERIC_READ_REQUEST_1_PARAM		= 0x14,
> > > > +	MIPI_DSI_GENERIC_READ_REQUEST_2_PARAM		= 0x24,
> > > > +	MIPI_DSI_DCS_LONG_WRITE				= 0x39,
> > > > +	MIPI_DSI_SET_MAXIMUM_RETURN_PACKET_SIZE		= 0x37,
> > > > +	MIPI_DSI_NULL_PACKET				= 9,
> > > > +	MIPI_DSI_BLANKING_PACKET			= 0x19,
> > > > +	MIPI_DSI_GENERIC_LONG_WRITE			= 0x29,
> > > > +	MIPI_DSI_LOOSELY_PACKED_PIXEL_STREAM_YCBCR20	= 0xc,
> > > > +	MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR24		= 0x1c,
> > > > +	MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR16		= 0x2c,
> > > > +	MIPI_DSI_PACKED_PIXEL_STREAM_30			= 0xd,
> > > > +	MIPI_DSI_PACKED_PIXEL_STREAM_36			= 0x1d,
> > > > +	MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR12		= 0x3d,
> > > > +	MIPI_DSI_PACKED_PIXEL_STREAM_16			= 0xe,
> > > > +	MIPI_DSI_PACKED_PIXEL_STREAM_18			= 0x1e,
> > > > +	MIPI_DSI_PIXEL_STREAM_3BYTE_18			= 0x2e,
> > > > +	MIPI_DSI_PACKED_PIXEL_STREAM_24			= 0x3e,
> > > > +};
> > > > +
> > > > +enum mipi_dcs_cmd {
> > > 
> > > While the MIPI specs define a certain set of commands, the real panels
> > > usually implement also their own custom commands. That's why I don't
> > > think enum is a good choice here.
> > 
> > Yes, that's a valid point, I'll have to think about it more.
> 
> I think a simple solution would be to just use defines, and have
> functions that take the command as u8. That's what the OMAP DSI driver
> does. If you have better ideas, please share =).

I find enums easier on the eye than defines. Less irrelevant junk on
each line. There's no reason you can't pass enum values as u8. But in
that case giving the enum a name doesn't really make sense.

-- 
Ville Syrjälä

Re: [PATCH 1/4] fbdev: add a MIPI DSI header

[Paul Mundt]

On Wed, May 19, 2010 at 05:27:32PM +0300, Ville Syrj?l? wrote:
> On Wed, May 19, 2010 at 10:21:48AM +0200, Valkeinen Tomi (Nokia-D/Helsinki) wrote:
> > I think a simple solution would be to just use defines, and have
> > functions that take the command as u8. That's what the OMAP DSI driver
> > does. If you have better ideas, please share =).
> 
> I find enums easier on the eye than defines. Less irrelevant junk on
> each line. There's no reason you can't pass enum values as u8. But in
> that case giving the enum a name doesn't really make sense.
> 
enums are cleaner for these cases, but you also have the case where the
enum type itself is variable size depending on the ABI being used. If
the type in question isn't being packed in to a user-visible data
structure then this will never matter, but it does help to be a bit
careful here regardless. Many people were bitten by this in the ARM
OABI -> EABI conversion, while other architectures generally managed to
get it right from the onset.

Re: [PATCH 1/4] fbdev: add a MIPI DSI header

[Ville Syrjälä]

On Wed, May 19, 2010 at 05:00:58PM +0200, ext Paul Mundt wrote:
> On Wed, May 19, 2010 at 05:27:32PM +0300, Ville Syrj?l? wrote:
> > On Wed, May 19, 2010 at 10:21:48AM +0200, Valkeinen Tomi (Nokia-D/Helsinki) wrote:
> > > I think a simple solution would be to just use defines, and have
> > > functions that take the command as u8. That's what the OMAP DSI driver
> > > does. If you have better ideas, please share =).
> > 
> > I find enums easier on the eye than defines. Less irrelevant junk on
> > each line. There's no reason you can't pass enum values as u8. But in
> > that case giving the enum a name doesn't really make sense.
> > 
> enums are cleaner for these cases, but you also have the case where the
> enum type itself is variable size depending on the ABI being used. If
> the type in question isn't being packed in to a user-visible data
> structure then this will never matter, but it does help to be a bit
> careful here regardless. Many people were bitten by this in the ARM
> OABI -> EABI conversion, while other architectures generally managed to
> get it right from the onset.

Yeah using the enum type in ABI is a bad idea since actual type is
implementation defined. But if you don't give the enum an identifier
there's no way to even accidentally use it as a type.

-- 
Ville Syrjälä

Re: [PATCH 1/4] fbdev: add a MIPI DSI header

[Guennadi Liakhovetski]

On Wed, 19 May 2010, Tomi Valkeinen wrote:

> Hi,
> 
> On Wed, 2010-05-19 at 10:08 +0200, ext Guennadi Liakhovetski wrote:
> > Hi Tomi
> > 
> > On Wed, 19 May 2010, Tomi Valkeinen wrote:
> 
> > > The file name is mipi_dsi.h, the comment talks about MIPI, and the file
> > > contains defines for MIPI DSI and MIPI DCS. If you want the file to
> > > contain defines about all MIPI specs, I think the file name should be
> > > just mipi.h.
> > 
> > No, the header is just intended for DSI, the comment could be made more 
> > specific, yes, but DCS is a part of DSI, isn't it?
> 
> No, DCS is a spec of its own. DCS can be used on non-DSI displays also.
> For example N800 and N900 use DCS commands, but they are not DSI
> displays.

Yes, right, I should have said "they are related";) How about calling the 
header mipi_display.h? This would then unite DSI, DCS, DBI and DPI? 
Creating a separate header for each of these standards seems like an 
overkill to me. We could then put MIPI CSI and CPI standards in an 
include/media/mipi_camera.h. Not sure where to put various other MIPI 
standards, I guess, we'll have to think about it as a need arises.

> > > > + *
> > > > + * Copyright (C) 2010 Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > > > + * Copyright (C) 2006 Nokia Corporation
> > > > + * Author: Imre Deak <imre.deak@nokia.com>
> > > > + *
> > > > + * This program is free software; you can redistribute it and/or modify
> > > > + * it under the terms of the GNU General Public License version 2 as
> > > > + * published by the Free Software Foundation.
> > > > + */
> > > > +#ifndef MIPI_DSI_H
> > > > +#define MIPI_DSI_H
> > > > +
> > > > +#include <linux/types.h>
> > > > +
> > > > +enum mipi_dsi_cmd {
> > > 
> > > I think these are DSI datatypes, not commands.
> > 
> > hm, "write," "shut down," "mode off," "mode on" sound like commands to me, 
> > and I think I saw them called commands in the spec, but yes, some of them 
> > are just packet or data types... I'll double-check, thanks.
> 
> Well, true, this is not a clear thing. MIPI DSI spec talks about "turn
> on peripheral command". But the numbers are DSI data types, according to
> the spec, and not all of them are commands.

Ok, how about "telegram types" then?

> > > > +	MIPI_DSI_DCS_SHORT_WRITE			= 5,
> > > 
> > > Please use the same number format for all enums. So this should be 0x05.
> > 
> > Where does this requirement come from? Is it in CodingStyle?;)
> 
> I don't know, but mixing different formats looks ugly to me =).

This seems to be subjective;) But I'm not religious about it, and since 
Nokia (C) is the first in this file I might as well just accept your 
proposal;)

> > > > +	MIPI_DSI_DCS_SHORT_WRITE_PARAM			= 0x15,
> > > > +	MIPI_DSI_COLOR_MODE_OFF				= 2,
> > > > +	MIPI_DSI_COLOR_MODE_ON				= 0x12,
> > > > +	MIPI_DSI_SHUTDOWN_PERIPHERAL			= 0x22,
> > > > +	MIPI_DSI_TURN_ON_PERIPHERAL			= 0x32,
> > > > +	MIPI_DSI_GENERIC_SHORT_WRITE_0_PARAM		= 3,
> > > > +	MIPI_DSI_GENERIC_SHORT_WRITE_1_PARAM		= 0x13,
> > > > +	MIPI_DSI_GENERIC_SHORT_WRITE_2_PARAM		= 0x23,
> > > > +	MIPI_DSI_GENERIC_READ_REQUEST_0_PARAM		= 4,
> > > > +	MIPI_DSI_GENERIC_READ_REQUEST_1_PARAM		= 0x14,
> > > > +	MIPI_DSI_GENERIC_READ_REQUEST_2_PARAM		= 0x24,
> > > > +	MIPI_DSI_DCS_LONG_WRITE				= 0x39,
> > > > +	MIPI_DSI_SET_MAXIMUM_RETURN_PACKET_SIZE		= 0x37,
> > > > +	MIPI_DSI_NULL_PACKET				= 9,
> > > > +	MIPI_DSI_BLANKING_PACKET			= 0x19,
> > > > +	MIPI_DSI_GENERIC_LONG_WRITE			= 0x29,
> > > > +	MIPI_DSI_LOOSELY_PACKED_PIXEL_STREAM_YCBCR20	= 0xc,
> > > > +	MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR24		= 0x1c,
> > > > +	MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR16		= 0x2c,
> > > > +	MIPI_DSI_PACKED_PIXEL_STREAM_30			= 0xd,
> > > > +	MIPI_DSI_PACKED_PIXEL_STREAM_36			= 0x1d,
> > > > +	MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR12		= 0x3d,
> > > > +	MIPI_DSI_PACKED_PIXEL_STREAM_16			= 0xe,
> > > > +	MIPI_DSI_PACKED_PIXEL_STREAM_18			= 0x1e,
> > > > +	MIPI_DSI_PIXEL_STREAM_3BYTE_18			= 0x2e,
> > > > +	MIPI_DSI_PACKED_PIXEL_STREAM_24			= 0x3e,
> > > > +};
> > > > +
> > > > +enum mipi_dcs_cmd {
> > > 
> > > While the MIPI specs define a certain set of commands, the real panels
> > > usually implement also their own custom commands. That's why I don't
> > > think enum is a good choice here.
> > 
> > Yes, that's a valid point, I'll have to think about it more.
> 
> I think a simple solution would be to just use defines, and have
> functions that take the command as u8. That's what the OMAP DSI driver
> does. If you have better ideas, please share =).
> 
> The same applies for the DSI commands/datatypes, although I haven't seen
> a panel that has custom DSI datatypes.

As others voted for unnamed enums, how about using them?

Concerning omap2 display drivers, AFAICS the only thing we want to change 
there is to switch them too to using the common header and telegram type 
and command names? So far I don't see a need for a generic MIPI (display) 
subsystem in the kernel with an own bus type, API etc. We could of course 
create a simpe bus with callbacks for sending short and long packets and 
reading data back, but do we really need it ATM?

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

Re: [PATCH 1/4] fbdev: add a MIPI DSI header

[Tomi Valkeinen]

On Thu, 2010-05-20 at 10:07 +0200, ext Guennadi Liakhovetski wrote:

> Yes, right, I should have said "they are related";) How about calling the 
> header mipi_display.h? This would then unite DSI, DCS, DBI and DPI? 
> Creating a separate header for each of these standards seems like an 
> overkill to me. We could then put MIPI CSI and CPI standards in an 
> include/media/mipi_camera.h. Not sure where to put various other MIPI 
> standards, I guess, we'll have to think about it as a need arises.

mipi_display.h sounds good to me.

> > Well, true, this is not a clear thing. MIPI DSI spec talks about "turn
> > on peripheral command". But the numbers are DSI data types, according to
> > the spec, and not all of them are commands.
> 
> Ok, how about "telegram types" then?

I wouldn't invent a new word for this =). The DSI spec talks about
commands, data types and transactions, I think we should pick one of
them.

Perhaps this is already approaching nitpicking, but:

As only some of the commands/datatypes/transactions are commands, I
think that's not proper word. All of them have a data type number, and I
guess they all are transactions. So "Turn On Peripheral Command" is a
transaction, and its data type is 0x32.

I guess if the enum is named, it should then be mipi_dsi_transaction.

But then, which one of these would be more correct:

dsi_send(enum mipi_dsi_transaction transaction)
dsi_send(u8 datatype)

As I said previously, I haven't seen any panel using custom datatypes,
but I wouldn't be surprised if some panel does. In that sense I would go
for using u8, and then perhaps leaving the enum unnamed.

What do you think?

> As others voted for unnamed enums, how about using them?

Sounds good.

> Concerning omap2 display drivers, AFAICS the only thing we want to change 
> there is to switch them too to using the common header and telegram type 

Yep, I think that's the only change for now.

> and command names? So far I don't see a need for a generic MIPI (display) 
> subsystem in the kernel with an own bus type, API etc. We could of course 
> create a simpe bus with callbacks for sending short and long packets and 
> reading data back, but do we really need it ATM?

This is something I've been thinking about for some time. I even made
some prototypes for it, but I didn't have time to go forward with it.

It would of course be nice to use the same panel driver on different
boards, so I think this is definitely something to think about in the
future.

 Tomi

Re: [PATCH 1/4] fbdev: add a MIPI DSI header

[Felipe Balbi]

On Thu, May 20, 2010 at 11:32:49AM +0300, Tomi Valkeinen wrote:
> On Thu, 2010-05-20 at 10:07 +0200, ext Guennadi Liakhovetski wrote:
> 
> > Yes, right, I should have said "they are related";) How about calling the 
> > header mipi_display.h? This would then unite DSI, DCS, DBI and DPI? 
> > Creating a separate header for each of these standards seems like an 
> > overkill to me. We could then put MIPI CSI and CPI standards in an 
> > include/media/mipi_camera.h. Not sure where to put various other MIPI 
> > standards, I guess, we'll have to think about it as a need arises.
> 
> mipi_display.h sounds good to me.

if there are extra mipi standards which should be taken care of, why
not include/linux/mipi/[display.h camera.h other1.h other2.h...] ?

-- 
balbi

Re: [PATCH 1/4] fbdev: add a MIPI DSI header

[Guennadi Liakhovetski]

On Thu, 20 May 2010, Felipe Balbi wrote:

> On Thu, May 20, 2010 at 11:32:49AM +0300, Tomi Valkeinen wrote:
> > On Thu, 2010-05-20 at 10:07 +0200, ext Guennadi Liakhovetski wrote:
> > 
> > > Yes, right, I should have said "they are related";) How about calling the 
> > > header mipi_display.h? This would then unite DSI, DCS, DBI and DPI? 
> > > Creating a separate header for each of these standards seems like an 
> > > overkill to me. We could then put MIPI CSI and CPI standards in an 
> > > include/media/mipi_camera.h. Not sure where to put various other MIPI 
> > > standards, I guess, we'll have to think about it as a need arises.
> > 
> > mipi_display.h sounds good to me.
> 
> if there are extra mipi standards which should be taken care of, why
> not include/linux/mipi/[display.h camera.h other1.h other2.h...] ?

I thought about this too, but I prefer to separate them thematically: 
put display standards under video/, camera under media/ etc. This way 
including these headers from respective drivers and platform code would 
look more naturally, IMHO.

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

Re: [PATCH 1/4] fbdev: add a MIPI DSI header

[Guennadi Liakhovetski]

On Thu, 20 May 2010, Tomi Valkeinen wrote:

> On Thu, 2010-05-20 at 10:07 +0200, ext Guennadi Liakhovetski wrote:
> 
> > Yes, right, I should have said "they are related";) How about calling the 
> > header mipi_display.h? This would then unite DSI, DCS, DBI and DPI? 
> > Creating a separate header for each of these standards seems like an 
> > overkill to me. We could then put MIPI CSI and CPI standards in an 
> > include/media/mipi_camera.h. Not sure where to put various other MIPI 
> > standards, I guess, we'll have to think about it as a need arises.
> 
> mipi_display.h sounds good to me.
> 
> > > Well, true, this is not a clear thing. MIPI DSI spec talks about "turn
> > > on peripheral command". But the numbers are DSI data types, according to
> > > the spec, and not all of them are commands.
> > 
> > Ok, how about "telegram types" then?
> 
> I wouldn't invent a new word for this =). The DSI spec talks about
> commands, data types and transactions, I think we should pick one of
> them.
> 
> Perhaps this is already approaching nitpicking, but:
> 
> As only some of the commands/datatypes/transactions are commands, I
> think that's not proper word. All of them have a data type number, and I
> guess they all are transactions. So "Turn On Peripheral Command" is a
> transaction, and its data type is 0x32.

Yes, transaction looks good to me.

> I guess if the enum is named, it should then be mipi_dsi_transaction.
> 
> But then, which one of these would be more correct:
> 
> dsi_send(enum mipi_dsi_transaction transaction)
> dsi_send(u8 datatype)
> 
> As I said previously, I haven't seen any panel using custom datatypes,
> but I wouldn't be surprised if some panel does. In that sense I would go
> for using u8, and then perhaps leaving the enum unnamed.
> 
> What do you think?

Yes, I'd use u8 too, because the specs define "transaction types" to take 
6 bits and "DCS commands" to be 8-bit ints. But, you know, doesn't our 
case fall under case (b) of Chapter 5 "Typedefs" of CodingStyle?:) I mean, 
wouldn't it make sense to create two typedefs for these to add some 
type-safety? In fact, transaction types cannot be user-defined / 
proprietary, right? So, for that an enum would work. It's only DCS 
commands that allow extensions. But making transaction type an enum and 
command a typedef would be ugly, and making a typedef for an enum is 
frowned upon too...

> > As others voted for unnamed enums, how about using them?
> 
> Sounds good.
> 
> > Concerning omap2 display drivers, AFAICS the only thing we want to change 
> > there is to switch them too to using the common header and telegram type 
> 
> Yep, I think that's the only change for now.
> 
> > and command names? So far I don't see a need for a generic MIPI (display) 
> > subsystem in the kernel with an own bus type, API etc. We could of course 
> > create a simpe bus with callbacks for sending short and long packets and 
> > reading data back, but do we really need it ATM?
> 
> This is something I've been thinking about for some time. I even made
> some prototypes for it, but I didn't have time to go forward with it.
> 
> It would of course be nice to use the same panel driver on different
> boards, so I think this is definitely something to think about in the
> future.

But you only need specific panel drivers if you want to support their 
proprietary commands? Otherwise you only need a set of parameters - 
timeing requirements etc. - which you can perfectly just pass in platform 
data?

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

Re: [PATCH 1/4] fbdev: add a MIPI DSI header

[Tomi Valkeinen]

On Thu, 2010-05-20 at 13:03 +0200, ext Guennadi Liakhovetski wrote:

> Yes, I'd use u8 too, because the specs define "transaction types" to take 
> 6 bits and "DCS commands" to be 8-bit ints. But, you know, doesn't our 
> case fall under case (b) of Chapter 5 "Typedefs" of CodingStyle?:) I mean, 

CodingStyle says: "if there is a clear reason for why it under certain
circumstances might be an "unsigned int" and under other configurations
might be "unsigned long", then by all means go ahead and use a typedef."

In our case the DSI transactions and DCS commands are always 6 and 8
bits, so I don't think it applies.

And the transaction datatypes and DCS commands are really just payload
sent to the peripheral, and the peripheral can do anything it wants with
those.

> wouldn't it make sense to create two typedefs for these to add some 

But wouldn't a typedef basically lock the possible transactions or DCS
commands that you can give to the functions? And that was the original
problem also.

> type-safety? In fact, transaction types cannot be user-defined / 
> proprietary, right? So, for that an enum would work. It's only DCS 

Why they cannot be? I don't see any technical problem there, at least.
And while I don't have any current use to extend transactions, I
wouldn't want to limit it for any possible future use either.

> But you only need specific panel drivers if you want to support their 
> proprietary commands? Otherwise you only need a set of parameters - 
> timeing requirements etc. - which you can perfectly just pass in platform 
> data?

That's true. I guess you've gotten it easy, if you only have DSI video
mode panels that do not need any special care =). Command mode panels
are usually slightly more complex, and especially early prototypes of
the panels can require quite a lot of trickery. And then there are DSI
hubs, which make DSI command mode panels look trivial...

 Tomi

[PATCH 2/4] sh-mobile: add support for displays, connected over the

[Guennadi Liakhovetski]

Some SH-mobile SoCs have a MIPI DSI controller, that can be used to connect
MIPI displays to LCDC. This patch adds a platform driver for SH-mobile MIPI DSI
unit. It uses existing hooks in the sh_mobile_lcdcfb.c driver for display
activation and deactivation.

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

Nothing OMAP-relevant here, just for completeness.

 drivers/video/Kconfig       |    8 +
 drivers/video/Makefile      |    1 +
 drivers/video/sh_mipi_dsi.c |  500 +++++++++++++++++++++++++++++++++++++++++++
 include/video/sh_mipi_dsi.h |   35 +++
 4 files changed, 544 insertions(+), 0 deletions(-)
 create mode 100644 drivers/video/sh_mipi_dsi.c
 create mode 100644 include/video/sh_mipi_dsi.h

diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 6e16244..773c4a6 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -1879,6 +1879,13 @@ config FB_W100
 
 	  If unsure, say N.
 
+config SH_MIPI_DSI
+	tristate
+	depends on (SUPERH || ARCH_SHMOBILE) && HAVE_CLK
+
+config SH_LCD_MIPI_DSI
+	bool
+
 config FB_SH_MOBILE_LCDC
 	tristate "SuperH Mobile LCDC framebuffer support"
 	depends on FB && (SUPERH || ARCH_SHMOBILE) && HAVE_CLK
@@ -1887,6 +1894,7 @@ config FB_SH_MOBILE_LCDC
 	select FB_SYS_IMAGEBLIT
 	select FB_SYS_FOPS
 	select FB_DEFERRED_IO
+	select SH_MIPI_DSI if SH_LCD_MIPI_DSI
 	---help---
 	  Frame buffer driver for the on-chip SH-Mobile LCD controller.
 
diff --git a/drivers/video/Makefile b/drivers/video/Makefile
index ddc2af2..3c3bf86 100644
--- a/drivers/video/Makefile
+++ b/drivers/video/Makefile
@@ -123,6 +123,7 @@ obj-$(CONFIG_FB_IBM_GXT4500)	  += gxt4500.o
 obj-$(CONFIG_FB_PS3)		  += ps3fb.o
 obj-$(CONFIG_FB_SM501)            += sm501fb.o
 obj-$(CONFIG_FB_XILINX)           += xilinxfb.o
+obj-$(CONFIG_SH_MIPI_DSI)	  += sh_mipi_dsi.o
 obj-$(CONFIG_FB_SH_MOBILE_LCDC)	  += sh_mobile_lcdcfb.o
 obj-$(CONFIG_FB_OMAP)             += omap/
 obj-y                             += omap2/
diff --git a/drivers/video/sh_mipi_dsi.c b/drivers/video/sh_mipi_dsi.c
new file mode 100644
index 0000000..af0208f
--- /dev/null
+++ b/drivers/video/sh_mipi_dsi.c
@@ -0,0 +1,500 @@
+/*
+ * Renesas SH-mobile MIPI DSI support
+ *
+ * Copyright (C) 2010 Guennadi Liakhovetski <g.liakhovetski@gmx.de>
+ *
+ * This is free software; you can redistribute it and/or modify
+ * it under the terms of version 2 of the GNU General Public License as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/platform_device.h>
+#include <linux/string.h>
+
+#include <video/mipi_dsi.h>
+#include <video/sh_mipi_dsi.h>
+#include <video/sh_mobile_lcdc.h>
+
+#define CMTSRTCTR	0x80d0
+#define CMTSRTREQ	0x8070
+
+#define DSIINTE		0x0060
+
+/* E.g., sh7372 has 2 MIPI-DSIs - one for each LCDC */
+#define MAX_SH_MIPI_DSI 2
+
+struct sh_mipi {
+	void __iomem	*base;
+	struct clk	*dsit_clk;
+	struct clk	*dsip_clk;
+};
+
+static struct sh_mipi *mipi_dsi[MAX_SH_MIPI_DSI];
+
+/* Protect the above array */
+static DEFINE_MUTEX(array_lock);
+
+static struct sh_mipi *sh_mipi_by_handle(int handle)
+{
+	if (handle >= ARRAY_SIZE(mipi_dsi) || handle < 0)
+		return NULL;
+
+	return mipi_dsi[handle];
+}
+
+static int sh_mipi_send_short(struct sh_mipi *mipi, enum mipi_dsi_cmd dsi_cmd,
+			      enum mipi_dcs_cmd cmd, u8 param)
+{
+	u32 data = (dsi_cmd << 24) | (cmd << 16) | (param << 8);
+	int cnt = 100;
+
+	/* transmit a short packet to LCD panel */
+	iowrite32(1 | data, mipi->base + 0x80d0); /* CMTSRTCTR */
+	iowrite32(1, mipi->base + 0x8070); /* CMTSRTREQ */
+
+	while ((ioread32(mipi->base + 0x8070) & 1) && --cnt)
+		udelay(1);
+
+	return cnt ? 0 : -ETIMEDOUT;
+}
+
+#define LCD_CHAN2MIPI(c) ((c) < LCDC_CHAN_MAINLCD || (c) > LCDC_CHAN_SUBLCD ? \
+				-EINVAL : (c) - 1)
+
+static int sh_mipi_dcs(int handle, enum mipi_dcs_cmd cmd)
+{
+	struct sh_mipi *mipi = sh_mipi_by_handle(LCD_CHAN2MIPI(handle));
+	if (!mipi)
+		return -ENODEV;
+	return sh_mipi_send_short(mipi, MIPI_DSI_DCS_SHORT_WRITE, cmd, 0);
+}
+
+static int sh_mipi_dcs_param(int handle, enum mipi_dcs_cmd cmd, u8 param)
+{
+	struct sh_mipi *mipi = sh_mipi_by_handle(LCD_CHAN2MIPI(handle));
+	if (!mipi)
+		return -ENODEV;
+	return sh_mipi_send_short(mipi, MIPI_DSI_DCS_SHORT_WRITE_PARAM, cmd,
+				  param);
+}
+
+static void sh_mipi_dsi_enable(struct sh_mipi *mipi, bool enable)
+{
+	/*
+	 * enable LCDC data tx, transition to LPS after completion of each HS
+	 * packet
+	 */
+	iowrite32(0x00000002 | enable, mipi->base + 0x8000); /* DTCTR */
+}
+
+static void sh_mipi_shutdown(struct platform_device *pdev)
+{
+	struct sh_mipi *mipi = platform_get_drvdata(pdev);
+
+	sh_mipi_dsi_enable(mipi, false);
+}
+
+static void mipi_display_on(void *arg)
+{
+	struct sh_mipi *mipi = arg;
+
+	sh_mipi_dsi_enable(mipi, true);
+}
+
+static void mipi_display_off(void *arg)
+{
+	struct sh_mipi *mipi = arg;
+
+	sh_mipi_dsi_enable(mipi, false);
+}
+
+static int __init sh_mipi_setup(struct sh_mipi *mipi,
+				struct sh_mipi_dsi_info *pdata)
+{
+	void __iomem *base = mipi->base;
+	struct sh_mobile_lcdc_chan_cfg *ch = pdata->lcd_chan;
+	u32 pctype, datatype, pixfmt;
+	u32 linelength;
+	bool yuv;
+
+	/* Select data format */
+	switch (pdata->data_format) {
+	case MIPI_RGB888:
+		pctype = 0;
+		datatype = MIPI_DSI_PACKED_PIXEL_STREAM_24;
+		pixfmt = MIPI_DCS_PIXEL_FMT_24BIT;
+		linelength = ch->lcd_cfg.xres * 3;
+		yuv = false;
+		break;
+	case MIPI_RGB565:
+		pctype = 1;
+		datatype = MIPI_DSI_PACKED_PIXEL_STREAM_16;
+		pixfmt = MIPI_DCS_PIXEL_FMT_16BIT;
+		linelength = ch->lcd_cfg.xres * 2;
+		yuv = false;
+		break;
+	case MIPI_RGB666_LP:
+		pctype = 2;
+		datatype = MIPI_DSI_PIXEL_STREAM_3BYTE_18;
+		pixfmt = MIPI_DCS_PIXEL_FMT_24BIT;
+		linelength = ch->lcd_cfg.xres * 3;
+		yuv = false;
+		break;
+	case MIPI_RGB666:
+		pctype = 3;
+		datatype = MIPI_DSI_PACKED_PIXEL_STREAM_18;
+		pixfmt = MIPI_DCS_PIXEL_FMT_18BIT;
+		linelength = (ch->lcd_cfg.xres * 18 + 7) / 8;
+		yuv = false;
+		break;
+	case MIPI_BGR888:
+		pctype = 8;
+		datatype = MIPI_DSI_PACKED_PIXEL_STREAM_24;
+		pixfmt = MIPI_DCS_PIXEL_FMT_24BIT;
+		linelength = ch->lcd_cfg.xres * 3;
+		yuv = false;
+		break;
+	case MIPI_BGR565:
+		pctype = 9;
+		datatype = MIPI_DSI_PACKED_PIXEL_STREAM_16;
+		pixfmt = MIPI_DCS_PIXEL_FMT_16BIT;
+		linelength = ch->lcd_cfg.xres * 2;
+		yuv = false;
+		break;
+	case MIPI_BGR666_LP:
+		pctype = 0xa;
+		datatype = MIPI_DSI_PIXEL_STREAM_3BYTE_18;
+		pixfmt = MIPI_DCS_PIXEL_FMT_24BIT;
+		linelength = ch->lcd_cfg.xres * 3;
+		yuv = false;
+		break;
+	case MIPI_BGR666:
+		pctype = 0xb;
+		datatype = MIPI_DSI_PACKED_PIXEL_STREAM_18;
+		pixfmt = MIPI_DCS_PIXEL_FMT_18BIT;
+		linelength = (ch->lcd_cfg.xres * 18 + 7) / 8;
+		yuv = false;
+		break;
+	case MIPI_YUYV:
+		pctype = 4;
+		datatype = MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR16;
+		pixfmt = MIPI_DCS_PIXEL_FMT_16BIT;
+		linelength = ch->lcd_cfg.xres * 2;
+		yuv = true;
+		break;
+	case MIPI_UYVY:
+		pctype = 5;
+		datatype = MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR16;
+		pixfmt = MIPI_DCS_PIXEL_FMT_16BIT;
+		linelength = ch->lcd_cfg.xres * 2;
+		yuv = true;
+		break;
+	case MIPI_YUV420_L:
+		pctype = 6;
+		datatype = MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR12;
+		pixfmt = MIPI_DCS_PIXEL_FMT_12BIT;
+		linelength = (ch->lcd_cfg.xres * 12 + 7) / 8;
+		yuv = true;
+		break;
+	case MIPI_YUV420:
+		pctype = 7;
+		datatype = MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR12;
+		pixfmt = MIPI_DCS_PIXEL_FMT_12BIT;
+		/* Length of U/V line */
+		linelength = (ch->lcd_cfg.xres + 1) / 2;
+		yuv = true;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	if ((yuv && ch->interface_type != YUV422) ||
+	    (!yuv && ch->interface_type != RGB24))
+		return -EINVAL;
+
+	/* reset DSI link */
+	iowrite32(0x00000001, base); /* SYSCTRL */
+	/* Hold reset for 100 cycles of the slowest of bus, HS byte and LP clock */
+	udelay(50);
+	iowrite32(0x00000000, base); /* SYSCTRL */
+
+	/* setup DSI link */
+
+	/*
+	 * Default = ULPS enable |
+	 *	Contention detection enabled |
+	 *	EoT packet transmission enable |
+	 *	CRC check enable |
+	 *	ECC check enable
+	 * additionally enable first two lanes
+	 */
+	iowrite32(0x00003703, base + 0x04); /* SYSCONF */
+	/*
+	 * T_wakeup = 0x7000
+	 * T_hs-trail = 3
+	 * T_hs-prepare = 3
+	 * T_clk-trail = 3
+	 * T_clk-prepare = 2
+	 */
+	iowrite32(0x70003332, base + 0x08); /* TIMSET */
+	/* no responses requested */
+	iowrite32(0x00000000, base + 0x18); /* RESREQSET0 */
+	/* request response to packets of type 0x28 */
+	iowrite32(0x00000100, base + 0x1c); /* RESREQSET1 */
+	/* High-speed transmission timeout, default 0xffffffff */
+	iowrite32(0x0fffffff, base + 0x20); /* HSTTOVSET */
+	/* LP reception timeout, default 0xffffffff */
+	iowrite32(0x0fffffff, base + 0x24); /* LPRTOVSET */
+	/* Turn-around timeout, default 0xffffffff */
+	iowrite32(0x0fffffff, base + 0x28); /* TATOVSET */
+	/* Peripheral reset timeout, default 0xffffffff */
+	iowrite32(0x0fffffff, base + 0x2c); /* PRTOVSET */
+	/* Enable timeout counters */
+	iowrite32(0x00000f00, base + 0x30); /* DSICTRL */
+	/* Interrupts not used, disable all */
+	iowrite32(0, base + DSIINTE);
+	/* DSI-Tx bias on */
+	iowrite32(0x00000001, base + 0x70); /* PHYCTRL */
+	udelay(200);
+	/* Deassert resets, power on */
+	iowrite32(0x03070b01, base + 0x70); /* PHYCTRL */
+
+	/* setup l-bridge */
+
+	/*
+	 * Enable transmission of all packets,
+	 * transmit LPS after each HS packet completion
+	 */
+	iowrite32(0x00000006, base + 0x8000); /* DTCTR */
+	/* VSYNC width = 2 (<< 17) */
+	iowrite32(0x00040000 | (pctype << 12) | datatype, base + 0x8020); /* VMCTR1 */
+	/* VSE and HSE are outputs, HSA period allowed */
+	iowrite32(0x00e00000, base + 0x8024); /* VMCTR2 */
+	/*
+	 * 0x660 = 1632 bytes per line (RGB24, 544 pixels: see
+	 * sh_mobile_lcdc_info.ch[0].lcd_cfg.xres), HSALEN = 1 - default
+	 * (unused, since VMCTR2[HSABM] = 0)
+	 */
+	iowrite32(1 | (linelength << 16), base + 0x8028); /* VMLEN1 */
+
+	msleep(5);
+
+	/* setup LCD panel */
+
+	/* cf. drivers/video/omap/lcd_mipid.c */
+	sh_mipi_dcs(ch->chan, MIPI_DCS_EXIT_SLEEP_MODE);
+	msleep(120);
+	/*
+	 * [7] - Page Address Mode
+	 * [6] - Column Address Mode
+	 * [5] - Page / Column Address Mode
+	 * [4] - Display Device Line Refresh Order
+	 * [3] - RGB/BGR Order
+	 * [2] - Display Data Latch Data Order
+	 * [1] - Flip Horizontal
+	 * [0] - Flip Vertical
+	 */
+	sh_mipi_dcs_param(ch->chan, MIPI_DCS_SET_ADDRESS_MODE, 0x00);
+	/* cf. set_data_lines() */
+	sh_mipi_dcs_param(ch->chan, MIPI_DCS_SET_PIXEL_FORMAT,
+			  pixfmt << 4);
+	sh_mipi_dcs(ch->chan, MIPI_DCS_SET_DISPLAY_ON);
+
+	return 0;
+}
+
+static int __init sh_mipi_probe(struct platform_device *pdev)
+{
+	struct sh_mipi *mipi;
+	struct sh_mipi_dsi_info *pdata = pdev->dev.platform_data;
+	struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	unsigned long rate, f_current;
+	int idx = pdev->id, ret;
+	char dsip_clk[] = "dsi.p_clk";
+
+	if (!res || idx >= ARRAY_SIZE(mipi_dsi) || !pdata)
+		return -ENODEV;
+
+	mutex_lock(&array_lock);
+	if (idx < 0)
+		for (idx = 0; idx < ARRAY_SIZE(mipi_dsi) && mipi_dsi[idx]; idx++)
+			;
+
+	if (idx = ARRAY_SIZE(mipi_dsi)) {
+		ret = -EBUSY;
+		goto efindslot;
+	}
+
+	mipi = kzalloc(sizeof(*mipi), GFP_KERNEL);
+	if (!mipi) {
+		ret = -ENOMEM;
+		goto ealloc;
+	}
+
+	if (!request_mem_region(res->start, resource_size(res), pdev->name)) {
+		dev_err(&pdev->dev, "MIPI register region already claimed\n");
+		ret = -EBUSY;
+		goto ereqreg;
+	}
+
+	mipi->base = ioremap(res->start, resource_size(res));
+	if (!mipi->base) {
+		ret = -ENOMEM;
+		goto emap;
+	}
+
+	mipi->dsit_clk = clk_get(&pdev->dev, "dsit_clk");
+	if (IS_ERR(mipi->dsit_clk)) {
+		ret = PTR_ERR(mipi->dsit_clk);
+		goto eclktget;
+	}
+
+	f_current = clk_get_rate(mipi->dsit_clk);
+	/* 80MHz required by the datasheet */
+	rate = clk_round_rate(mipi->dsit_clk, 80000000);
+	if (rate > 0 && rate != f_current)
+		ret = clk_set_rate(mipi->dsit_clk, rate);
+	else
+		ret = rate;
+	if (ret < 0)
+		goto esettrate;
+
+	dev_dbg(&pdev->dev, "DSI-T clk %lu -> %lu\n", f_current, rate);
+
+	sprintf(dsip_clk, "dsi%1.1dp_clk", idx);
+	mipi->dsip_clk = clk_get(&pdev->dev, dsip_clk);
+	if (IS_ERR(mipi->dsip_clk)) {
+		ret = PTR_ERR(mipi->dsip_clk);
+		goto eclkpget;
+	}
+
+	f_current = clk_get_rate(mipi->dsip_clk);
+	/* Between 10 and 50MHz */
+	rate = clk_round_rate(mipi->dsip_clk, 24000000);
+	if (rate > 0 && rate != f_current)
+		ret = clk_set_rate(mipi->dsip_clk, rate);
+	else
+		ret = rate;
+	if (ret < 0)
+		goto esetprate;
+
+	dev_dbg(&pdev->dev, "DSI-P clk %lu -> %lu\n", f_current, rate);
+
+	msleep(10);
+
+	ret = clk_enable(mipi->dsit_clk);
+	if (ret < 0)
+		goto eclkton;
+
+	ret = clk_enable(mipi->dsip_clk);
+	if (ret < 0)
+		goto eclkpon;
+
+	mipi_dsi[idx] = mipi;
+
+	ret = sh_mipi_setup(mipi, pdata);
+	if (ret < 0)
+		goto emipisetup;
+
+	mutex_unlock(&array_lock);
+	platform_set_drvdata(pdev, mipi);
+
+	/* Set up LCDC callbacks */
+	pdata->lcd_chan->board_cfg.board_data = mipi;
+	pdata->lcd_chan->board_cfg.display_on = mipi_display_on;
+	pdata->lcd_chan->board_cfg.display_off = mipi_display_off;
+
+	return 0;
+
+emipisetup:
+	mipi_dsi[idx] = NULL;
+	clk_disable(mipi->dsip_clk);
+eclkpon:
+	clk_disable(mipi->dsit_clk);
+eclkton:
+esetprate:
+	clk_put(mipi->dsip_clk);
+eclkpget:
+esettrate:
+	clk_put(mipi->dsit_clk);
+eclktget:
+	iounmap(mipi->base);
+emap:
+	release_mem_region(res->start, resource_size(res));
+ereqreg:
+	kfree(mipi);
+ealloc:
+efindslot:
+	mutex_unlock(&array_lock);
+
+	return ret;
+}
+
+static int __exit sh_mipi_remove(struct platform_device *pdev)
+{
+	struct sh_mipi_dsi_info *pdata = pdev->dev.platform_data;
+	struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	struct sh_mipi *mipi = platform_get_drvdata(pdev);
+	int i, ret;
+
+	mutex_lock(&array_lock);
+
+	for (i = 0; i < ARRAY_SIZE(mipi_dsi) && mipi_dsi[i] != mipi; i++)
+		;
+
+	if (i = ARRAY_SIZE(mipi_dsi)) {
+		ret = -EINVAL;
+	} else {
+		ret = 0;
+		mipi_dsi[i] = NULL;
+	}
+
+	mutex_unlock(&array_lock);
+
+	if (ret < 0)
+		return ret;
+
+	pdata->lcd_chan->board_cfg.display_on = NULL;
+	pdata->lcd_chan->board_cfg.display_off = NULL;
+	pdata->lcd_chan->board_cfg.board_data = NULL;
+
+	clk_disable(mipi->dsip_clk);
+	clk_disable(mipi->dsit_clk);
+	clk_put(mipi->dsit_clk);
+	clk_put(mipi->dsip_clk);
+	iounmap(mipi->base);
+	if (res)
+		release_mem_region(res->start, resource_size(res));
+	platform_set_drvdata(pdev, NULL);
+	kfree(mipi);
+
+	return 0;
+}
+
+static struct platform_driver sh_mipi_driver = {
+	.remove		= __exit_p(sh_mipi_remove),
+	.shutdown	= sh_mipi_shutdown,
+	.driver = {
+		.name	= "sh-mipi-dsi",
+	},
+};
+
+static int __init sh_mipi_init(void)
+{
+	return platform_driver_probe(&sh_mipi_driver, sh_mipi_probe);
+}
+module_init(sh_mipi_init);
+
+static void __exit sh_mipi_exit(void)
+{
+	platform_driver_unregister(&sh_mipi_driver);
+}
+module_exit(sh_mipi_exit);
+
+MODULE_AUTHOR("Guennadi Liakhovetski <g.liakhovetski@gmx.de>");
+MODULE_DESCRIPTION("SuperH / ARM-shmobile MIPI DSI driver");
+MODULE_LICENSE("GPL v2");
diff --git a/include/video/sh_mipi_dsi.h b/include/video/sh_mipi_dsi.h
new file mode 100644
index 0000000..18bca08
--- /dev/null
+++ b/include/video/sh_mipi_dsi.h
@@ -0,0 +1,35 @@
+/*
+ * Public SH-mobile MIPI DSI header
+ *
+ * Copyright (C) 2010 Guennadi Liakhovetski <g.liakhovetski@gmx.de>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#ifndef VIDEO_SH_MIPI_DSI_H
+#define VIDEO_SH_MIPI_DSI_H
+
+enum sh_mipi_dsi_data_fmt {
+	MIPI_RGB888,
+	MIPI_RGB565,
+	MIPI_RGB666_LP,
+	MIPI_RGB666,
+	MIPI_BGR888,
+	MIPI_BGR565,
+	MIPI_BGR666_LP,
+	MIPI_BGR666,
+	MIPI_YUYV,
+	MIPI_UYVY,
+	MIPI_YUV420_L,
+	MIPI_YUV420,
+};
+
+struct sh_mobile_lcdc_chan_cfg;
+
+struct sh_mipi_dsi_info {
+	enum sh_mipi_dsi_data_fmt	data_format;
+	struct sh_mobile_lcdc_chan_cfg	*lcd_chan;
+};
+
+#endif
-- 
1.6.2.4

Re: [PATCH 2/4] sh-mobile: add support for displays, connected over

[Damian]

(4:59), Guennadi Liakhovetski wrote:
> Some SH-mobile SoCs have a MIPI DSI controller, that can be used to connect
> MIPI displays to LCDC. This patch adds a platform driver for SH-mobile MIPI DSI
> unit. It uses existing hooks in the sh_mobile_lcdcfb.c driver for display
> activation and deactivation.
> 
> Signed-off-by: Guennadi Liakhovetski<g.liakhovetski@gmx.de>
Console framebuffer tested on sh-2.6 tree and sh/dmaengine branch with
the necessary clock and intc patches applied.
Boot logo and framebuffer console displayed with no problems or unusual
effects.

Tested-by: Damian Hobson-Garcia <dhobsong@igel.co.jp>
> ---
> 
> Nothing OMAP-relevant here, just for completeness.
> 
>   drivers/video/Kconfig       |    8 +
>   drivers/video/Makefile      |    1 +
>   drivers/video/sh_mipi_dsi.c |  500 +++++++++++++++++++++++++++++++++++++++++++
>   include/video/sh_mipi_dsi.h |   35 +++
>   4 files changed, 544 insertions(+), 0 deletions(-)
>   create mode 100644 drivers/video/sh_mipi_dsi.c
>   create mode 100644 include/video/sh_mipi_dsi.h
> 
> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> index 6e16244..773c4a6 100644
> --- a/drivers/video/Kconfig
> +++ b/drivers/video/Kconfig
> @@ -1879,6 +1879,13 @@ config FB_W100
> 
>   	  If unsure, say N.
> 
> +config SH_MIPI_DSI
> +	tristate
> +	depends on (SUPERH || ARCH_SHMOBILE)&&  HAVE_CLK
> +
> +config SH_LCD_MIPI_DSI
> +	bool
> +
>   config FB_SH_MOBILE_LCDC
>   	tristate "SuperH Mobile LCDC framebuffer support"
>   	depends on FB&&  (SUPERH || ARCH_SHMOBILE)&&  HAVE_CLK
> @@ -1887,6 +1894,7 @@ config FB_SH_MOBILE_LCDC
>   	select FB_SYS_IMAGEBLIT
>   	select FB_SYS_FOPS
>   	select FB_DEFERRED_IO
> +	select SH_MIPI_DSI if SH_LCD_MIPI_DSI
>   	---help---
>   	  Frame buffer driver for the on-chip SH-Mobile LCD controller.
> 
> diff --git a/drivers/video/Makefile b/drivers/video/Makefile
> index ddc2af2..3c3bf86 100644
> --- a/drivers/video/Makefile
> +++ b/drivers/video/Makefile
> @@ -123,6 +123,7 @@ obj-$(CONFIG_FB_IBM_GXT4500)	  += gxt4500.o
>   obj-$(CONFIG_FB_PS3)		  += ps3fb.o
>   obj-$(CONFIG_FB_SM501)            += sm501fb.o
>   obj-$(CONFIG_FB_XILINX)           += xilinxfb.o
> +obj-$(CONFIG_SH_MIPI_DSI)	  += sh_mipi_dsi.o
>   obj-$(CONFIG_FB_SH_MOBILE_LCDC)	  += sh_mobile_lcdcfb.o
>   obj-$(CONFIG_FB_OMAP)             += omap/
>   obj-y                             += omap2/
> diff --git a/drivers/video/sh_mipi_dsi.c b/drivers/video/sh_mipi_dsi.c
> new file mode 100644
> index 0000000..af0208f
> --- /dev/null
> +++ b/drivers/video/sh_mipi_dsi.c
> @@ -0,0 +1,500 @@
> +/*
> + * Renesas SH-mobile MIPI DSI support
> + *
> + * Copyright (C) 2010 Guennadi Liakhovetski<g.liakhovetski@gmx.de>
> + *
> + * This is free software; you can redistribute it and/or modify
> + * it under the terms of version 2 of the GNU General Public License as
> + * published by the Free Software Foundation.
> + */
> +
> +#include<linux/clk.h>
> +#include<linux/delay.h>
> +#include<linux/init.h>
> +#include<linux/io.h>
> +#include<linux/platform_device.h>
> +#include<linux/string.h>
> +
> +#include<video/mipi_dsi.h>
> +#include<video/sh_mipi_dsi.h>
> +#include<video/sh_mobile_lcdc.h>
> +
> +#define CMTSRTCTR	0x80d0
> +#define CMTSRTREQ	0x8070
> +
> +#define DSIINTE		0x0060
> +
> +/* E.g., sh7372 has 2 MIPI-DSIs - one for each LCDC */
> +#define MAX_SH_MIPI_DSI 2
> +
> +struct sh_mipi {
> +	void __iomem	*base;
> +	struct clk	*dsit_clk;
> +	struct clk	*dsip_clk;
> +};
> +
> +static struct sh_mipi *mipi_dsi[MAX_SH_MIPI_DSI];
> +
> +/* Protect the above array */
> +static DEFINE_MUTEX(array_lock);
> +
> +static struct sh_mipi *sh_mipi_by_handle(int handle)
> +{
> +	if (handle>= ARRAY_SIZE(mipi_dsi) || handle<  0)
> +		return NULL;
> +
> +	return mipi_dsi[handle];
> +}
> +
> +static int sh_mipi_send_short(struct sh_mipi *mipi, enum mipi_dsi_cmd dsi_cmd,
> +			      enum mipi_dcs_cmd cmd, u8 param)
> +{
> +	u32 data = (dsi_cmd<<  24) | (cmd<<  16) | (param<<  8);
> +	int cnt = 100;
> +
> +	/* transmit a short packet to LCD panel */
> +	iowrite32(1 | data, mipi->base + 0x80d0); /* CMTSRTCTR */
> +	iowrite32(1, mipi->base + 0x8070); /* CMTSRTREQ */
> +
> +	while ((ioread32(mipi->base + 0x8070)&  1)&&  --cnt)
> +		udelay(1);
> +
> +	return cnt ? 0 : -ETIMEDOUT;
> +}
> +
> +#define LCD_CHAN2MIPI(c) ((c)<  LCDC_CHAN_MAINLCD || (c)>  LCDC_CHAN_SUBLCD ? \
> +				-EINVAL : (c) - 1)
> +
> +static int sh_mipi_dcs(int handle, enum mipi_dcs_cmd cmd)
> +{
> +	struct sh_mipi *mipi = sh_mipi_by_handle(LCD_CHAN2MIPI(handle));
> +	if (!mipi)
> +		return -ENODEV;
> +	return sh_mipi_send_short(mipi, MIPI_DSI_DCS_SHORT_WRITE, cmd, 0);
> +}
> +
> +static int sh_mipi_dcs_param(int handle, enum mipi_dcs_cmd cmd, u8 param)
> +{
> +	struct sh_mipi *mipi = sh_mipi_by_handle(LCD_CHAN2MIPI(handle));
> +	if (!mipi)
> +		return -ENODEV;
> +	return sh_mipi_send_short(mipi, MIPI_DSI_DCS_SHORT_WRITE_PARAM, cmd,
> +				  param);
> +}
> +
> +static void sh_mipi_dsi_enable(struct sh_mipi *mipi, bool enable)
> +{
> +	/*
> +	 * enable LCDC data tx, transition to LPS after completion of each HS
> +	 * packet
> +	 */
> +	iowrite32(0x00000002 | enable, mipi->base + 0x8000); /* DTCTR */
> +}
> +
> +static void sh_mipi_shutdown(struct platform_device *pdev)
> +{
> +	struct sh_mipi *mipi = platform_get_drvdata(pdev);
> +
> +	sh_mipi_dsi_enable(mipi, false);
> +}
> +
> +static void mipi_display_on(void *arg)
> +{
> +	struct sh_mipi *mipi = arg;
> +
> +	sh_mipi_dsi_enable(mipi, true);
> +}
> +
> +static void mipi_display_off(void *arg)
> +{
> +	struct sh_mipi *mipi = arg;
> +
> +	sh_mipi_dsi_enable(mipi, false);
> +}
> +
> +static int __init sh_mipi_setup(struct sh_mipi *mipi,
> +				struct sh_mipi_dsi_info *pdata)
> +{
> +	void __iomem *base = mipi->base;
> +	struct sh_mobile_lcdc_chan_cfg *ch = pdata->lcd_chan;
> +	u32 pctype, datatype, pixfmt;
> +	u32 linelength;
> +	bool yuv;
> +
> +	/* Select data format */
> +	switch (pdata->data_format) {
> +	case MIPI_RGB888:
> +		pctype = 0;
> +		datatype = MIPI_DSI_PACKED_PIXEL_STREAM_24;
> +		pixfmt = MIPI_DCS_PIXEL_FMT_24BIT;
> +		linelength = ch->lcd_cfg.xres * 3;
> +		yuv = false;
> +		break;
> +	case MIPI_RGB565:
> +		pctype = 1;
> +		datatype = MIPI_DSI_PACKED_PIXEL_STREAM_16;
> +		pixfmt = MIPI_DCS_PIXEL_FMT_16BIT;
> +		linelength = ch->lcd_cfg.xres * 2;
> +		yuv = false;
> +		break;
> +	case MIPI_RGB666_LP:
> +		pctype = 2;
> +		datatype = MIPI_DSI_PIXEL_STREAM_3BYTE_18;
> +		pixfmt = MIPI_DCS_PIXEL_FMT_24BIT;
> +		linelength = ch->lcd_cfg.xres * 3;
> +		yuv = false;
> +		break;
> +	case MIPI_RGB666:
> +		pctype = 3;
> +		datatype = MIPI_DSI_PACKED_PIXEL_STREAM_18;
> +		pixfmt = MIPI_DCS_PIXEL_FMT_18BIT;
> +		linelength = (ch->lcd_cfg.xres * 18 + 7) / 8;
> +		yuv = false;
> +		break;
> +	case MIPI_BGR888:
> +		pctype = 8;
> +		datatype = MIPI_DSI_PACKED_PIXEL_STREAM_24;
> +		pixfmt = MIPI_DCS_PIXEL_FMT_24BIT;
> +		linelength = ch->lcd_cfg.xres * 3;
> +		yuv = false;
> +		break;
> +	case MIPI_BGR565:
> +		pctype = 9;
> +		datatype = MIPI_DSI_PACKED_PIXEL_STREAM_16;
> +		pixfmt = MIPI_DCS_PIXEL_FMT_16BIT;
> +		linelength = ch->lcd_cfg.xres * 2;
> +		yuv = false;
> +		break;
> +	case MIPI_BGR666_LP:
> +		pctype = 0xa;
> +		datatype = MIPI_DSI_PIXEL_STREAM_3BYTE_18;
> +		pixfmt = MIPI_DCS_PIXEL_FMT_24BIT;
> +		linelength = ch->lcd_cfg.xres * 3;
> +		yuv = false;
> +		break;
> +	case MIPI_BGR666:
> +		pctype = 0xb;
> +		datatype = MIPI_DSI_PACKED_PIXEL_STREAM_18;
> +		pixfmt = MIPI_DCS_PIXEL_FMT_18BIT;
> +		linelength = (ch->lcd_cfg.xres * 18 + 7) / 8;
> +		yuv = false;
> +		break;
> +	case MIPI_YUYV:
> +		pctype = 4;
> +		datatype = MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR16;
> +		pixfmt = MIPI_DCS_PIXEL_FMT_16BIT;
> +		linelength = ch->lcd_cfg.xres * 2;
> +		yuv = true;
> +		break;
> +	case MIPI_UYVY:
> +		pctype = 5;
> +		datatype = MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR16;
> +		pixfmt = MIPI_DCS_PIXEL_FMT_16BIT;
> +		linelength = ch->lcd_cfg.xres * 2;
> +		yuv = true;
> +		break;
> +	case MIPI_YUV420_L:
> +		pctype = 6;
> +		datatype = MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR12;
> +		pixfmt = MIPI_DCS_PIXEL_FMT_12BIT;
> +		linelength = (ch->lcd_cfg.xres * 12 + 7) / 8;
> +		yuv = true;
> +		break;
> +	case MIPI_YUV420:
> +		pctype = 7;
> +		datatype = MIPI_DSI_PACKED_PIXEL_STREAM_YCBCR12;
> +		pixfmt = MIPI_DCS_PIXEL_FMT_12BIT;
> +		/* Length of U/V line */
> +		linelength = (ch->lcd_cfg.xres + 1) / 2;
> +		yuv = true;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	if ((yuv&&  ch->interface_type != YUV422) ||
> +	    (!yuv&&  ch->interface_type != RGB24))
> +		return -EINVAL;
> +
> +	/* reset DSI link */
> +	iowrite32(0x00000001, base); /* SYSCTRL */
> +	/* Hold reset for 100 cycles of the slowest of bus, HS byte and LP clock */
> +	udelay(50);
> +	iowrite32(0x00000000, base); /* SYSCTRL */
> +
> +	/* setup DSI link */
> +
> +	/*
> +	 * Default = ULPS enable |
> +	 *	Contention detection enabled |
> +	 *	EoT packet transmission enable |
> +	 *	CRC check enable |
> +	 *	ECC check enable
> +	 * additionally enable first two lanes
> +	 */
> +	iowrite32(0x00003703, base + 0x04); /* SYSCONF */
> +	/*
> +	 * T_wakeup = 0x7000
> +	 * T_hs-trail = 3
> +	 * T_hs-prepare = 3
> +	 * T_clk-trail = 3
> +	 * T_clk-prepare = 2
> +	 */
> +	iowrite32(0x70003332, base + 0x08); /* TIMSET */
> +	/* no responses requested */
> +	iowrite32(0x00000000, base + 0x18); /* RESREQSET0 */
> +	/* request response to packets of type 0x28 */
> +	iowrite32(0x00000100, base + 0x1c); /* RESREQSET1 */
> +	/* High-speed transmission timeout, default 0xffffffff */
> +	iowrite32(0x0fffffff, base + 0x20); /* HSTTOVSET */
> +	/* LP reception timeout, default 0xffffffff */
> +	iowrite32(0x0fffffff, base + 0x24); /* LPRTOVSET */
> +	/* Turn-around timeout, default 0xffffffff */
> +	iowrite32(0x0fffffff, base + 0x28); /* TATOVSET */
> +	/* Peripheral reset timeout, default 0xffffffff */
> +	iowrite32(0x0fffffff, base + 0x2c); /* PRTOVSET */
> +	/* Enable timeout counters */
> +	iowrite32(0x00000f00, base + 0x30); /* DSICTRL */
> +	/* Interrupts not used, disable all */
> +	iowrite32(0, base + DSIINTE);
> +	/* DSI-Tx bias on */
> +	iowrite32(0x00000001, base + 0x70); /* PHYCTRL */
> +	udelay(200);
> +	/* Deassert resets, power on */
> +	iowrite32(0x03070b01, base + 0x70); /* PHYCTRL */
> +
> +	/* setup l-bridge */
> +
> +	/*
> +	 * Enable transmission of all packets,
> +	 * transmit LPS after each HS packet completion
> +	 */
> +	iowrite32(0x00000006, base + 0x8000); /* DTCTR */
> +	/* VSYNC width = 2 (<<  17) */
> +	iowrite32(0x00040000 | (pctype<<  12) | datatype, base + 0x8020); /* VMCTR1 */
> +	/* VSE and HSE are outputs, HSA period allowed */
> +	iowrite32(0x00e00000, base + 0x8024); /* VMCTR2 */
> +	/*
> +	 * 0x660 = 1632 bytes per line (RGB24, 544 pixels: see
> +	 * sh_mobile_lcdc_info.ch[0].lcd_cfg.xres), HSALEN = 1 - default
> +	 * (unused, since VMCTR2[HSABM] = 0)
> +	 */
> +	iowrite32(1 | (linelength<<  16), base + 0x8028); /* VMLEN1 */
> +
> +	msleep(5);
> +
> +	/* setup LCD panel */
> +
> +	/* cf. drivers/video/omap/lcd_mipid.c */
> +	sh_mipi_dcs(ch->chan, MIPI_DCS_EXIT_SLEEP_MODE);
> +	msleep(120);
> +	/*
> +	 * [7] - Page Address Mode
> +	 * [6] - Column Address Mode
> +	 * [5] - Page / Column Address Mode
> +	 * [4] - Display Device Line Refresh Order
> +	 * [3] - RGB/BGR Order
> +	 * [2] - Display Data Latch Data Order
> +	 * [1] - Flip Horizontal
> +	 * [0] - Flip Vertical
> +	 */
> +	sh_mipi_dcs_param(ch->chan, MIPI_DCS_SET_ADDRESS_MODE, 0x00);
> +	/* cf. set_data_lines() */
> +	sh_mipi_dcs_param(ch->chan, MIPI_DCS_SET_PIXEL_FORMAT,
> +			  pixfmt<<  4);
> +	sh_mipi_dcs(ch->chan, MIPI_DCS_SET_DISPLAY_ON);
> +
> +	return 0;
> +}
> +
> +static int __init sh_mipi_probe(struct platform_device *pdev)
> +{
> +	struct sh_mipi *mipi;
> +	struct sh_mipi_dsi_info *pdata = pdev->dev.platform_data;
> +	struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	unsigned long rate, f_current;
> +	int idx = pdev->id, ret;
> +	char dsip_clk[] = "dsi.p_clk";
> +
> +	if (!res || idx>= ARRAY_SIZE(mipi_dsi) || !pdata)
> +		return -ENODEV;
> +
> +	mutex_lock(&array_lock);
> +	if (idx<  0)
> +		for (idx = 0; idx<  ARRAY_SIZE(mipi_dsi)&&  mipi_dsi[idx]; idx++)
> +			;
> +
> +	if (idx = ARRAY_SIZE(mipi_dsi)) {
> +		ret = -EBUSY;
> +		goto efindslot;
> +	}
> +
> +	mipi = kzalloc(sizeof(*mipi), GFP_KERNEL);
> +	if (!mipi) {
> +		ret = -ENOMEM;
> +		goto ealloc;
> +	}
> +
> +	if (!request_mem_region(res->start, resource_size(res), pdev->name)) {
> +		dev_err(&pdev->dev, "MIPI register region already claimed\n");
> +		ret = -EBUSY;
> +		goto ereqreg;
> +	}
> +
> +	mipi->base = ioremap(res->start, resource_size(res));
> +	if (!mipi->base) {
> +		ret = -ENOMEM;
> +		goto emap;
> +	}
> +
> +	mipi->dsit_clk = clk_get(&pdev->dev, "dsit_clk");
> +	if (IS_ERR(mipi->dsit_clk)) {
> +		ret = PTR_ERR(mipi->dsit_clk);
> +		goto eclktget;
> +	}
> +
> +	f_current = clk_get_rate(mipi->dsit_clk);
> +	/* 80MHz required by the datasheet */
> +	rate = clk_round_rate(mipi->dsit_clk, 80000000);
> +	if (rate>  0&&  rate != f_current)
> +		ret = clk_set_rate(mipi->dsit_clk, rate);
> +	else
> +		ret = rate;
> +	if (ret<  0)
> +		goto esettrate;
> +
> +	dev_dbg(&pdev->dev, "DSI-T clk %lu ->  %lu\n", f_current, rate);
> +
> +	sprintf(dsip_clk, "dsi%1.1dp_clk", idx);
> +	mipi->dsip_clk = clk_get(&pdev->dev, dsip_clk);
> +	if (IS_ERR(mipi->dsip_clk)) {
> +		ret = PTR_ERR(mipi->dsip_clk);
> +		goto eclkpget;
> +	}
> +
> +	f_current = clk_get_rate(mipi->dsip_clk);
> +	/* Between 10 and 50MHz */
> +	rate = clk_round_rate(mipi->dsip_clk, 24000000);
> +	if (rate>  0&&  rate != f_current)
> +		ret = clk_set_rate(mipi->dsip_clk, rate);
> +	else
> +		ret = rate;
> +	if (ret<  0)
> +		goto esetprate;
> +
> +	dev_dbg(&pdev->dev, "DSI-P clk %lu ->  %lu\n", f_current, rate);
> +
> +	msleep(10);
> +
> +	ret = clk_enable(mipi->dsit_clk);
> +	if (ret<  0)
> +		goto eclkton;
> +
> +	ret = clk_enable(mipi->dsip_clk);
> +	if (ret<  0)
> +		goto eclkpon;
> +
> +	mipi_dsi[idx] = mipi;
> +
> +	ret = sh_mipi_setup(mipi, pdata);
> +	if (ret<  0)
> +		goto emipisetup;
> +
> +	mutex_unlock(&array_lock);
> +	platform_set_drvdata(pdev, mipi);
> +
> +	/* Set up LCDC callbacks */
> +	pdata->lcd_chan->board_cfg.board_data = mipi;
> +	pdata->lcd_chan->board_cfg.display_on = mipi_display_on;
> +	pdata->lcd_chan->board_cfg.display_off = mipi_display_off;
> +
> +	return 0;
> +
> +emipisetup:
> +	mipi_dsi[idx] = NULL;
> +	clk_disable(mipi->dsip_clk);
> +eclkpon:
> +	clk_disable(mipi->dsit_clk);
> +eclkton:
> +esetprate:
> +	clk_put(mipi->dsip_clk);
> +eclkpget:
> +esettrate:
> +	clk_put(mipi->dsit_clk);
> +eclktget:
> +	iounmap(mipi->base);
> +emap:
> +	release_mem_region(res->start, resource_size(res));
> +ereqreg:
> +	kfree(mipi);
> +ealloc:
> +efindslot:
> +	mutex_unlock(&array_lock);
> +
> +	return ret;
> +}
> +
> +static int __exit sh_mipi_remove(struct platform_device *pdev)
> +{
> +	struct sh_mipi_dsi_info *pdata = pdev->dev.platform_data;
> +	struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	struct sh_mipi *mipi = platform_get_drvdata(pdev);
> +	int i, ret;
> +
> +	mutex_lock(&array_lock);
> +
> +	for (i = 0; i<  ARRAY_SIZE(mipi_dsi)&&  mipi_dsi[i] != mipi; i++)
> +		;
> +
> +	if (i = ARRAY_SIZE(mipi_dsi)) {
> +		ret = -EINVAL;
> +	} else {
> +		ret = 0;
> +		mipi_dsi[i] = NULL;
> +	}
> +
> +	mutex_unlock(&array_lock);
> +
> +	if (ret<  0)
> +		return ret;
> +
> +	pdata->lcd_chan->board_cfg.display_on = NULL;
> +	pdata->lcd_chan->board_cfg.display_off = NULL;
> +	pdata->lcd_chan->board_cfg.board_data = NULL;
> +
> +	clk_disable(mipi->dsip_clk);
> +	clk_disable(mipi->dsit_clk);
> +	clk_put(mipi->dsit_clk);
> +	clk_put(mipi->dsip_clk);
> +	iounmap(mipi->base);
> +	if (res)
> +		release_mem_region(res->start, resource_size(res));
> +	platform_set_drvdata(pdev, NULL);
> +	kfree(mipi);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver sh_mipi_driver = {
> +	.remove		= __exit_p(sh_mipi_remove),
> +	.shutdown	= sh_mipi_shutdown,
> +	.driver = {
> +		.name	= "sh-mipi-dsi",
> +	},
> +};
> +
> +static int __init sh_mipi_init(void)
> +{
> +	return platform_driver_probe(&sh_mipi_driver, sh_mipi_probe);
> +}
> +module_init(sh_mipi_init);
> +
> +static void __exit sh_mipi_exit(void)
> +{
> +	platform_driver_unregister(&sh_mipi_driver);
> +}
> +module_exit(sh_mipi_exit);
> +
> +MODULE_AUTHOR("Guennadi Liakhovetski<g.liakhovetski@gmx.de>");
> +MODULE_DESCRIPTION("SuperH / ARM-shmobile MIPI DSI driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/video/sh_mipi_dsi.h b/include/video/sh_mipi_dsi.h
> new file mode 100644
> index 0000000..18bca08
> --- /dev/null
> +++ b/include/video/sh_mipi_dsi.h
> @@ -0,0 +1,35 @@
> +/*
> + * Public SH-mobile MIPI DSI header
> + *
> + * Copyright (C) 2010 Guennadi Liakhovetski<g.liakhovetski@gmx.de>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +#ifndef VIDEO_SH_MIPI_DSI_H
> +#define VIDEO_SH_MIPI_DSI_H
> +
> +enum sh_mipi_dsi_data_fmt {
> +	MIPI_RGB888,
> +	MIPI_RGB565,
> +	MIPI_RGB666_LP,
> +	MIPI_RGB666,
> +	MIPI_BGR888,
> +	MIPI_BGR565,
> +	MIPI_BGR666_LP,
> +	MIPI_BGR666,
> +	MIPI_YUYV,
> +	MIPI_UYVY,
> +	MIPI_YUV420_L,
> +	MIPI_YUV420,
> +};
> +
> +struct sh_mobile_lcdc_chan_cfg;
> +
> +struct sh_mipi_dsi_info {
> +	enum sh_mipi_dsi_data_fmt	data_format;
> +	struct sh_mobile_lcdc_chan_cfg	*lcd_chan;
> +};
> +
> +#endif


-- 
Damian Hobson-Garcia
IGEL Co.,Ltd
http://www.igel.co.jp

[PATCH 3/4] ARM: add framebuffer support for ap4evb

[Guennadi Liakhovetski]

ap4evb uses an LCD, connected to the SoC over the MIPI bus. This patch adds
platform data to configure this display and a static clock activation.

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

Nothing OMAP-relevant here, just for completeness.

 arch/arm/mach-shmobile/Kconfig        |    1 +
 arch/arm/mach-shmobile/board-ap4evb.c |  121 +++++++++++++++++++++++++++++++++
 2 files changed, 122 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-shmobile/Kconfig b/arch/arm/mach-shmobile/Kconfig
index 9003ea9..e216132 100644
--- a/arch/arm/mach-shmobile/Kconfig
+++ b/arch/arm/mach-shmobile/Kconfig
@@ -42,6 +42,7 @@ config MACH_AP4EVB
 	bool "AP4EVB board"
 	depends on ARCH_SH7372
 	select ARCH_REQUIRE_GPIOLIB
+	select SH_LCD_MIPI_DSI
 
 comment "SH-Mobile System Configuration"
 
diff --git a/arch/arm/mach-shmobile/board-ap4evb.c b/arch/arm/mach-shmobile/board-ap4evb.c
index d2bb7b0..e3f5375 100644
--- a/arch/arm/mach-shmobile/board-ap4evb.c
+++ b/arch/arm/mach-shmobile/board-ap4evb.c
@@ -17,6 +17,7 @@
  * along with this program; if not, write to the Free Software
  * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
  */
+#include <linux/clk.h>
 #include <linux/kernel.h>
 #include <linux/init.h>
 #include <linux/interrupt.h>
@@ -32,8 +33,14 @@
 #include <linux/gpio.h>
 #include <linux/input.h>
 #include <linux/input/sh_keysc.h>
+
+#include <video/sh_mobile_lcdc.h>
+#include <video/sh_mipi_dsi.h>
+
 #include <mach/common.h>
+#include <mach/irqs.h>
 #include <mach/sh7372.h>
+
 #include <asm/mach-types.h>
 #include <asm/mach/arch.h>
 #include <asm/mach/map.h>
@@ -237,11 +244,86 @@ static struct platform_device sdhi0_device = {
 	},
 };
 
+static struct sh_mobile_lcdc_info sh_mobile_lcdc_info = {
+	.clock_source = LCDC_CLK_PERIPHERAL, /* One of interface clocks */
+	.ch[0] = {
+		.chan = LCDC_CHAN_MAINLCD,
+		.bpp = 16,
+		.interface_type = RGB24,
+		.clock_divider = 1,
+		.flags = LCDC_FLAGS_DWPOL,
+		.lcd_cfg = {
+			.name = "R63302(QHD)",
+			.xres = 544,
+			.yres = 961,
+			.left_margin = 72,
+			.right_margin = 1080,
+			.hsync_len = 16,
+			.upper_margin = 8,
+			.lower_margin = 8,
+			.vsync_len = 2,
+			.sync = FB_SYNC_VERT_HIGH_ACT | FB_SYNC_HOR_HIGH_ACT,
+		},
+		.lcd_size_cfg = {
+			.width = 44,
+			.height = 79,
+		},
+	}
+};
+
+static struct resource lcdc_resources[] = {
+	[0] = {
+		.name	= "LCDC",
+		.start	= 0xfe940000, /* P4-only space */
+		.end	= 0xfe943fff,
+		.flags	= IORESOURCE_MEM,
+	},
+	[1] = {
+		.start	= evt2irq(0x580),
+		.flags	= IORESOURCE_IRQ,
+	},
+};
+
+static struct platform_device lcdc_device = {
+	.name		= "sh_mobile_lcdc_fb",
+	.num_resources	= ARRAY_SIZE(lcdc_resources),
+	.resource	= lcdc_resources,
+	.dev	= {
+		.platform_data	= &sh_mobile_lcdc_info,
+		.coherent_dma_mask = ~0,
+	},
+};
+
+static struct resource mipidsi0_resources[] = {
+	[0] = {
+		.start  = 0xffc60000,
+		.end    = 0xffc68fff,
+		.flags  = IORESOURCE_MEM,
+	},
+};
+
+static struct sh_mipi_dsi_info mipidsi0_info = {
+	.data_format	= MIPI_RGB888,
+	.lcd_chan	= &sh_mobile_lcdc_info.ch[0],
+};
+
+static struct platform_device mipidsi0_device = {
+	.name           = "sh-mipi-dsi",
+	.num_resources  = ARRAY_SIZE(mipidsi0_resources),
+	.resource       = mipidsi0_resources,
+	.id             = 0,
+	.dev	= {
+		.platform_data	= &mipidsi0_info,
+	},
+};
+
 static struct platform_device *ap4evb_devices[] __initdata = {
 	&nor_flash_device,
 	&smc911x_device,
 	&keysc_device,
 	&sdhi0_device,
+	&lcdc_device,
+	&mipidsi0_device,
 };
 
 static struct map_desc ap4evb_io_desc[] __initdata = {
@@ -265,6 +347,45 @@ static void __init ap4evb_map_io(void)
 	shmobile_setup_console();
 }
 
+/* This function will disappear when we switch to (runtime) PM */
+static int __init ap4evb_init_display_clk(void)
+{
+	struct clk *lcdc_clk;
+	struct clk *dsitx_clk;
+	int ret;
+
+	lcdc_clk = clk_get(&lcdc_device.dev, "lcdc0");
+	if (IS_ERR(lcdc_clk))
+		return PTR_ERR(lcdc_clk);
+
+	dsitx_clk = clk_get(&mipidsi0_device.dev, "dsitx");
+	if (IS_ERR(dsitx_clk)) {
+		ret = PTR_ERR(dsitx_clk);
+		goto eclkdsitxget;
+	}
+
+	ret = clk_enable(lcdc_clk);
+	if (ret < 0)
+		goto eclklcdcon;
+
+	ret = clk_enable(dsitx_clk);
+	if (ret < 0)
+		goto eclkdsitxon;
+
+	return 0;
+
+eclkdsitxon:
+	clk_disable(lcdc_clk);
+eclklcdcon:
+	clk_put(dsitx_clk);
+eclkdsitxget:
+	clk_put(lcdc_clk);
+
+	return ret;
+}
+
+device_initcall(ap4evb_init_display_clk);
+
 static void __init ap4evb_init(void)
 {
 	sh7372_pinmux_init();
-- 
1.6.2.4

[PATCH 3/4 v2] ARM: add framebuffer support for ap4evb

[Guennadi Liakhovetski]

ap4evb uses an LCD, connected to the SoC over the MIPI bus. This patch adds
platform data to configure this display and a static clock activation.

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

v1 -> v2:

Use the correct interrupt number for LCDC, based on INTCS.

 arch/arm/mach-shmobile/Kconfig        |    1 +
 arch/arm/mach-shmobile/board-ap4evb.c |  121 +++++++++++++++++++++++++++++++++
 2 files changed, 122 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-shmobile/Kconfig b/arch/arm/mach-shmobile/Kconfig
index 9003ea9..e216132 100644
--- a/arch/arm/mach-shmobile/Kconfig
+++ b/arch/arm/mach-shmobile/Kconfig
@@ -42,6 +42,7 @@ config MACH_AP4EVB
 	bool "AP4EVB board"
 	depends on ARCH_SH7372
 	select ARCH_REQUIRE_GPIOLIB
+	select SH_LCD_MIPI_DSI
 
 comment "SH-Mobile System Configuration"
 
diff --git a/arch/arm/mach-shmobile/board-ap4evb.c b/arch/arm/mach-shmobile/board-ap4evb.c
index d2bb7b0..e3f5375 100644
--- a/arch/arm/mach-shmobile/board-ap4evb.c
+++ b/arch/arm/mach-shmobile/board-ap4evb.c
@@ -17,6 +17,7 @@
  * along with this program; if not, write to the Free Software
  * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
  */
+#include <linux/clk.h>
 #include <linux/kernel.h>
 #include <linux/init.h>
 #include <linux/interrupt.h>
@@ -32,8 +33,14 @@
 #include <linux/gpio.h>
 #include <linux/input.h>
 #include <linux/input/sh_keysc.h>
+
+#include <video/sh_mobile_lcdc.h>
+#include <video/sh_mipi_dsi.h>
+
 #include <mach/common.h>
+#include <mach/irqs.h>
 #include <mach/sh7372.h>
+
 #include <asm/mach-types.h>
 #include <asm/mach/arch.h>
 #include <asm/mach/map.h>
@@ -237,11 +244,86 @@ static struct platform_device sdhi0_device = {
 	},
 };
 
+static struct sh_mobile_lcdc_info sh_mobile_lcdc_info = {
+	.clock_source = LCDC_CLK_PERIPHERAL, /* One of interface clocks */
+	.ch[0] = {
+		.chan = LCDC_CHAN_MAINLCD,
+		.bpp = 16,
+		.interface_type = RGB24,
+		.clock_divider = 1,
+		.flags = LCDC_FLAGS_DWPOL,
+		.lcd_cfg = {
+			.name = "R63302(QHD)",
+			.xres = 544,
+			.yres = 961,
+			.left_margin = 72,
+			.right_margin = 1080,
+			.hsync_len = 16,
+			.upper_margin = 8,
+			.lower_margin = 8,
+			.vsync_len = 2,
+			.sync = FB_SYNC_VERT_HIGH_ACT | FB_SYNC_HOR_HIGH_ACT,
+		},
+		.lcd_size_cfg = {
+			.width = 44,
+			.height = 79,
+		},
+	}
+};
+
+static struct resource lcdc_resources[] = {
+	[0] = {
+		.name	= "LCDC",
+		.start	= 0xfe940000, /* P4-only space */
+		.end	= 0xfe943fff,
+		.flags	= IORESOURCE_MEM,
+	},
+	[1] = {
+		.start	= intcs_evt2irq(0x580),
+		.flags	= IORESOURCE_IRQ,
+	},
+};
+
+static struct platform_device lcdc_device = {
+	.name		= "sh_mobile_lcdc_fb",
+	.num_resources	= ARRAY_SIZE(lcdc_resources),
+	.resource	= lcdc_resources,
+	.dev	= {
+		.platform_data	= &sh_mobile_lcdc_info,
+		.coherent_dma_mask = ~0,
+	},
+};
+
+static struct resource mipidsi0_resources[] = {
+	[0] = {
+		.start  = 0xffc60000,
+		.end    = 0xffc68fff,
+		.flags  = IORESOURCE_MEM,
+	},
+};
+
+static struct sh_mipi_dsi_info mipidsi0_info = {
+	.data_format	= MIPI_RGB888,
+	.lcd_chan	= &sh_mobile_lcdc_info.ch[0],
+};
+
+static struct platform_device mipidsi0_device = {
+	.name           = "sh-mipi-dsi",
+	.num_resources  = ARRAY_SIZE(mipidsi0_resources),
+	.resource       = mipidsi0_resources,
+	.id             = 0,
+	.dev	= {
+		.platform_data	= &mipidsi0_info,
+	},
+};
+
 static struct platform_device *ap4evb_devices[] __initdata = {
 	&nor_flash_device,
 	&smc911x_device,
 	&keysc_device,
 	&sdhi0_device,
+	&lcdc_device,
+	&mipidsi0_device,
 };
 
 static struct map_desc ap4evb_io_desc[] __initdata = {
@@ -265,6 +347,45 @@ static void __init ap4evb_map_io(void)
 	shmobile_setup_console();
 }
 
+/* This function will disappear when we switch to (runtime) PM */
+static int __init ap4evb_init_display_clk(void)
+{
+	struct clk *lcdc_clk;
+	struct clk *dsitx_clk;
+	int ret;
+
+	lcdc_clk = clk_get(&lcdc_device.dev, "lcdc0");
+	if (IS_ERR(lcdc_clk))
+		return PTR_ERR(lcdc_clk);
+
+	dsitx_clk = clk_get(&mipidsi0_device.dev, "dsitx");
+	if (IS_ERR(dsitx_clk)) {
+		ret = PTR_ERR(dsitx_clk);
+		goto eclkdsitxget;
+	}
+
+	ret = clk_enable(lcdc_clk);
+	if (ret < 0)
+		goto eclklcdcon;
+
+	ret = clk_enable(dsitx_clk);
+	if (ret < 0)
+		goto eclkdsitxon;
+
+	return 0;
+
+eclkdsitxon:
+	clk_disable(lcdc_clk);
+eclklcdcon:
+	clk_put(dsitx_clk);
+eclkdsitxget:
+	clk_put(lcdc_clk);
+
+	return ret;
+}
+
+device_initcall(ap4evb_init_display_clk);
+
 static void __init ap4evb_init(void)
 {
 	sh7372_pinmux_init();
-- 
1.6.2.4

Re: [PATCH 3/4 v2] ARM: add framebuffer support for ap4evb

[Kuninori Morimoto]

Dear Guennadi

Thank you for your hard work

> diff --git a/arch/arm/mach-shmobile/Kconfig b/arch/arm/mach-shmobile/Kconfig
> index 9003ea9..e216132 100644
> --- a/arch/arm/mach-shmobile/Kconfig
> +++ b/arch/arm/mach-shmobile/Kconfig
> @@ -42,6 +42,7 @@ config MACH_AP4EVB
>  	bool "AP4EVB board"
>  	depends on ARCH_SH7372
>  	select ARCH_REQUIRE_GPIOLIB
> +	select SH_LCD_MIPI_DSI

Why is this "select" needed here ?
Do you want "force select MIPI DSI" on AP4 ?

I asked because AP4 board can select other type LCD.
(not only MIPI DSI)

Best regards
--
Kuninori Morimoto
 

Re: [PATCH 3/4 v2] ARM: add framebuffer support for ap4evb

[Guennadi Liakhovetski]

Hello Morimoto-san

On Mon, 10 May 2010, Kuninori Morimoto wrote:

> > diff --git a/arch/arm/mach-shmobile/Kconfig b/arch/arm/mach-shmobile/Kconfig
> > index 9003ea9..e216132 100644
> > --- a/arch/arm/mach-shmobile/Kconfig
> > +++ b/arch/arm/mach-shmobile/Kconfig
> > @@ -42,6 +42,7 @@ config MACH_AP4EVB
> >  	bool "AP4EVB board"
> >  	depends on ARCH_SH7372
> >  	select ARCH_REQUIRE_GPIOLIB
> > +	select SH_LCD_MIPI_DSI
> 
> Why is this "select" needed here ?
> Do you want "force select MIPI DSI" on AP4 ?

Not on all AP4 systems, only on ap4evb board, because I thought, that's 
the display, that it is using.

> I asked because AP4 board can select other type LCD.
> (not only MIPI DSI)

ap4evb can also have other LCDs attached to it? Then this will have to 
become a "choice" between various display types, where the MIPI DSI one 
will select SH_LCD_MIPI_DSI. We can either do this now or later when more 
LCD types are added.

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

Re: [PATCH 3/4 v2] ARM: add framebuffer support for ap4evb

[Kuninori Morimoto]

Dear Guennadi

> > Why is this "select" needed here ?
> > Do you want "force select MIPI DSI" on AP4 ?
> 
> Not on all AP4 systems, only on ap4evb board, because I thought, that's 
> the display, that it is using.

sorry, it mean AP4EVB board

> > I asked because AP4 board can select other type LCD.
> > (not only MIPI DSI)
> 
> ap4evb can also have other LCDs attached to it? Then this will have to 
> become a "choice" between various display types, where the MIPI DSI one 
> will select SH_LCD_MIPI_DSI. We can either do this now or later when more 
> LCD types are added.

OK. I understand.

Best regards
--
Kuninori Morimoto
 

Re: [PATCH 3/4 v2] ARM: add framebuffer support for ap4evb

[Damian]

(4:59), Guennadi Liakhovetski wrote:
> ap4evb uses an LCD, connected to the SoC over the MIPI bus. This patch adds
> platform data to configure this display and a static clock activation.
>
> Signed-off-by: Guennadi Liakhovetski<g.liakhovetski@gmx.de>
Console framebuffer tested on sh-2.6 tree and sh/dmaengine branch with 
the necessary clock and intc patches applied.
Boot logo and framebuffer console displayed with no problems or unusual 
effects.

Tested-by: Damian Hobson-Garcia <dhobsong@igel.co.jp>
> ---
>
> v1 ->  v2:
>
> Use the correct interrupt number for LCDC, based on INTCS.
>
>   arch/arm/mach-shmobile/Kconfig        |    1 +
>   arch/arm/mach-shmobile/board-ap4evb.c |  121 +++++++++++++++++++++++++++++++++
>   2 files changed, 122 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mach-shmobile/Kconfig b/arch/arm/mach-shmobile/Kconfig
> index 9003ea9..e216132 100644
> --- a/arch/arm/mach-shmobile/Kconfig
> +++ b/arch/arm/mach-shmobile/Kconfig
> @@ -42,6 +42,7 @@ config MACH_AP4EVB
>   	bool "AP4EVB board"
>   	depends on ARCH_SH7372
>   	select ARCH_REQUIRE_GPIOLIB
> +	select SH_LCD_MIPI_DSI
>
>   comment "SH-Mobile System Configuration"
>
> diff --git a/arch/arm/mach-shmobile/board-ap4evb.c b/arch/arm/mach-shmobile/board-ap4evb.c
> index d2bb7b0..e3f5375 100644
> --- a/arch/arm/mach-shmobile/board-ap4evb.c
> +++ b/arch/arm/mach-shmobile/board-ap4evb.c
> @@ -17,6 +17,7 @@
>    * along with this program; if not, write to the Free Software
>    * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
>    */
> +#include<linux/clk.h>
>   #include<linux/kernel.h>
>   #include<linux/init.h>
>   #include<linux/interrupt.h>
> @@ -32,8 +33,14 @@
>   #include<linux/gpio.h>
>   #include<linux/input.h>
>   #include<linux/input/sh_keysc.h>
> +
> +#include<video/sh_mobile_lcdc.h>
> +#include<video/sh_mipi_dsi.h>
> +
>   #include<mach/common.h>
> +#include<mach/irqs.h>
>   #include<mach/sh7372.h>
> +
>   #include<asm/mach-types.h>
>   #include<asm/mach/arch.h>
>   #include<asm/mach/map.h>
> @@ -237,11 +244,86 @@ static struct platform_device sdhi0_device = {
>   	},
>   };
>
> +static struct sh_mobile_lcdc_info sh_mobile_lcdc_info = {
> +	.clock_source = LCDC_CLK_PERIPHERAL, /* One of interface clocks */
> +	.ch[0] = {
> +		.chan = LCDC_CHAN_MAINLCD,
> +		.bpp = 16,
> +		.interface_type = RGB24,
> +		.clock_divider = 1,
> +		.flags = LCDC_FLAGS_DWPOL,
> +		.lcd_cfg = {
> +			.name = "R63302(QHD)",
> +			.xres = 544,
> +			.yres = 961,
> +			.left_margin = 72,
> +			.right_margin = 1080,
> +			.hsync_len = 16,
> +			.upper_margin = 8,
> +			.lower_margin = 8,
> +			.vsync_len = 2,
> +			.sync = FB_SYNC_VERT_HIGH_ACT | FB_SYNC_HOR_HIGH_ACT,
> +		},
> +		.lcd_size_cfg = {
> +			.width = 44,
> +			.height = 79,
> +		},
> +	}
> +};
> +
> +static struct resource lcdc_resources[] = {
> +	[0] = {
> +		.name	= "LCDC",
> +		.start	= 0xfe940000, /* P4-only space */
> +		.end	= 0xfe943fff,
> +		.flags	= IORESOURCE_MEM,
> +	},
> +	[1] = {
> +		.start	= intcs_evt2irq(0x580),
> +		.flags	= IORESOURCE_IRQ,
> +	},
> +};
> +
> +static struct platform_device lcdc_device = {
> +	.name		= "sh_mobile_lcdc_fb",
> +	.num_resources	= ARRAY_SIZE(lcdc_resources),
> +	.resource	= lcdc_resources,
> +	.dev	= {
> +		.platform_data	=&sh_mobile_lcdc_info,
> +		.coherent_dma_mask = ~0,
> +	},
> +};
> +
> +static struct resource mipidsi0_resources[] = {
> +	[0] = {
> +		.start  = 0xffc60000,
> +		.end    = 0xffc68fff,
> +		.flags  = IORESOURCE_MEM,
> +	},
> +};
> +
> +static struct sh_mipi_dsi_info mipidsi0_info = {
> +	.data_format	= MIPI_RGB888,
> +	.lcd_chan	=&sh_mobile_lcdc_info.ch[0],
> +};
> +
> +static struct platform_device mipidsi0_device = {
> +	.name           = "sh-mipi-dsi",
> +	.num_resources  = ARRAY_SIZE(mipidsi0_resources),
> +	.resource       = mipidsi0_resources,
> +	.id             = 0,
> +	.dev	= {
> +		.platform_data	=&mipidsi0_info,
> +	},
> +};
> +
>   static struct platform_device *ap4evb_devices[] __initdata = {
>   	&nor_flash_device,
>   	&smc911x_device,
>   	&keysc_device,
>   	&sdhi0_device,
> +	&lcdc_device,
> +	&mipidsi0_device,
>   };
>
>   static struct map_desc ap4evb_io_desc[] __initdata = {
> @@ -265,6 +347,45 @@ static void __init ap4evb_map_io(void)
>   	shmobile_setup_console();
>   }
>
> +/* This function will disappear when we switch to (runtime) PM */
> +static int __init ap4evb_init_display_clk(void)
> +{
> +	struct clk *lcdc_clk;
> +	struct clk *dsitx_clk;
> +	int ret;
> +
> +	lcdc_clk = clk_get(&lcdc_device.dev, "lcdc0");
> +	if (IS_ERR(lcdc_clk))
> +		return PTR_ERR(lcdc_clk);
> +
> +	dsitx_clk = clk_get(&mipidsi0_device.dev, "dsitx");
> +	if (IS_ERR(dsitx_clk)) {
> +		ret = PTR_ERR(dsitx_clk);
> +		goto eclkdsitxget;
> +	}
> +
> +	ret = clk_enable(lcdc_clk);
> +	if (ret<  0)
> +		goto eclklcdcon;
> +
> +	ret = clk_enable(dsitx_clk);
> +	if (ret<  0)
> +		goto eclkdsitxon;
> +
> +	return 0;
> +
> +eclkdsitxon:
> +	clk_disable(lcdc_clk);
> +eclklcdcon:
> +	clk_put(dsitx_clk);
> +eclkdsitxget:
> +	clk_put(lcdc_clk);
> +
> +	return ret;
> +}
> +
> +device_initcall(ap4evb_init_display_clk);
> +
>   static void __init ap4evb_init(void)
>   {
>   	sh7372_pinmux_init();


-- 
Damian Hobson-Garcia
IGEL Co.,Ltd
http://www.igel.co.jp

Re: [PATCH 3/4] ARM: add framebuffer support for ap4evb

[Damian]

(4:59), Guennadi Liakhovetski wrote:
> ap4evb uses an LCD, connected to the SoC over the MIPI bus. This patch adds
> platform data to configure this display and a static clock activation.
> 
> Signed-off-by: Guennadi Liakhovetski<g.liakhovetski@gmx.de>
Console framebuffer tested on sh-2.6 tree and sh/dmaengine branch with
the necessary clock and intc patches applied.
Boot logo and framebuffer console displayed with no problems or unusual
effects.

Tested-by: Damian Hobson-Garcia <dhobsong@igel.co.jp>
> ---
> 
> Nothing OMAP-relevant here, just for completeness.
> 
>   arch/arm/mach-shmobile/Kconfig        |    1 +
>   arch/arm/mach-shmobile/board-ap4evb.c |  121 +++++++++++++++++++++++++++++++++
>   2 files changed, 122 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/mach-shmobile/Kconfig b/arch/arm/mach-shmobile/Kconfig
> index 9003ea9..e216132 100644
> --- a/arch/arm/mach-shmobile/Kconfig
> +++ b/arch/arm/mach-shmobile/Kconfig
> @@ -42,6 +42,7 @@ config MACH_AP4EVB
>   	bool "AP4EVB board"
>   	depends on ARCH_SH7372
>   	select ARCH_REQUIRE_GPIOLIB
> +	select SH_LCD_MIPI_DSI
> 
>   comment "SH-Mobile System Configuration"
> 
> diff --git a/arch/arm/mach-shmobile/board-ap4evb.c b/arch/arm/mach-shmobile/board-ap4evb.c
> index d2bb7b0..e3f5375 100644
> --- a/arch/arm/mach-shmobile/board-ap4evb.c
> +++ b/arch/arm/mach-shmobile/board-ap4evb.c
> @@ -17,6 +17,7 @@
>    * along with this program; if not, write to the Free Software
>    * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
>    */
> +#include<linux/clk.h>
>   #include<linux/kernel.h>
>   #include<linux/init.h>
>   #include<linux/interrupt.h>
> @@ -32,8 +33,14 @@
>   #include<linux/gpio.h>
>   #include<linux/input.h>
>   #include<linux/input/sh_keysc.h>
> +
> +#include<video/sh_mobile_lcdc.h>
> +#include<video/sh_mipi_dsi.h>
> +
>   #include<mach/common.h>
> +#include<mach/irqs.h>
>   #include<mach/sh7372.h>
> +
>   #include<asm/mach-types.h>
>   #include<asm/mach/arch.h>
>   #include<asm/mach/map.h>
> @@ -237,11 +244,86 @@ static struct platform_device sdhi0_device = {
>   	},
>   };
> 
> +static struct sh_mobile_lcdc_info sh_mobile_lcdc_info = {
> +	.clock_source = LCDC_CLK_PERIPHERAL, /* One of interface clocks */
> +	.ch[0] = {
> +		.chan = LCDC_CHAN_MAINLCD,
> +		.bpp = 16,
> +		.interface_type = RGB24,
> +		.clock_divider = 1,
> +		.flags = LCDC_FLAGS_DWPOL,
> +		.lcd_cfg = {
> +			.name = "R63302(QHD)",
> +			.xres = 544,
> +			.yres = 961,
> +			.left_margin = 72,
> +			.right_margin = 1080,
> +			.hsync_len = 16,
> +			.upper_margin = 8,
> +			.lower_margin = 8,
> +			.vsync_len = 2,
> +			.sync = FB_SYNC_VERT_HIGH_ACT | FB_SYNC_HOR_HIGH_ACT,
> +		},
> +		.lcd_size_cfg = {
> +			.width = 44,
> +			.height = 79,
> +		},
> +	}
> +};
> +
> +static struct resource lcdc_resources[] = {
> +	[0] = {
> +		.name	= "LCDC",
> +		.start	= 0xfe940000, /* P4-only space */
> +		.end	= 0xfe943fff,
> +		.flags	= IORESOURCE_MEM,
> +	},
> +	[1] = {
> +		.start	= evt2irq(0x580),
> +		.flags	= IORESOURCE_IRQ,
> +	},
> +};
> +
> +static struct platform_device lcdc_device = {
> +	.name		= "sh_mobile_lcdc_fb",
> +	.num_resources	= ARRAY_SIZE(lcdc_resources),
> +	.resource	= lcdc_resources,
> +	.dev	= {
> +		.platform_data	=&sh_mobile_lcdc_info,
> +		.coherent_dma_mask = ~0,
> +	},
> +};
> +
> +static struct resource mipidsi0_resources[] = {
> +	[0] = {
> +		.start  = 0xffc60000,
> +		.end    = 0xffc68fff,
> +		.flags  = IORESOURCE_MEM,
> +	},
> +};
> +
> +static struct sh_mipi_dsi_info mipidsi0_info = {
> +	.data_format	= MIPI_RGB888,
> +	.lcd_chan	=&sh_mobile_lcdc_info.ch[0],
> +};
> +
> +static struct platform_device mipidsi0_device = {
> +	.name           = "sh-mipi-dsi",
> +	.num_resources  = ARRAY_SIZE(mipidsi0_resources),
> +	.resource       = mipidsi0_resources,
> +	.id             = 0,
> +	.dev	= {
> +		.platform_data	=&mipidsi0_info,
> +	},
> +};
> +
>   static struct platform_device *ap4evb_devices[] __initdata = {
>   	&nor_flash_device,
>   	&smc911x_device,
>   	&keysc_device,
>   	&sdhi0_device,
> +	&lcdc_device,
> +	&mipidsi0_device,
>   };
> 
>   static struct map_desc ap4evb_io_desc[] __initdata = {
> @@ -265,6 +347,45 @@ static void __init ap4evb_map_io(void)
>   	shmobile_setup_console();
>   }
> 
> +/* This function will disappear when we switch to (runtime) PM */
> +static int __init ap4evb_init_display_clk(void)
> +{
> +	struct clk *lcdc_clk;
> +	struct clk *dsitx_clk;
> +	int ret;
> +
> +	lcdc_clk = clk_get(&lcdc_device.dev, "lcdc0");
> +	if (IS_ERR(lcdc_clk))
> +		return PTR_ERR(lcdc_clk);
> +
> +	dsitx_clk = clk_get(&mipidsi0_device.dev, "dsitx");
> +	if (IS_ERR(dsitx_clk)) {
> +		ret = PTR_ERR(dsitx_clk);
> +		goto eclkdsitxget;
> +	}
> +
> +	ret = clk_enable(lcdc_clk);
> +	if (ret<  0)
> +		goto eclklcdcon;
> +
> +	ret = clk_enable(dsitx_clk);
> +	if (ret<  0)
> +		goto eclkdsitxon;
> +
> +	return 0;
> +
> +eclkdsitxon:
> +	clk_disable(lcdc_clk);
> +eclklcdcon:
> +	clk_put(dsitx_clk);
> +eclkdsitxget:
> +	clk_put(lcdc_clk);
> +
> +	return ret;
> +}
> +
> +device_initcall(ap4evb_init_display_clk);
> +
>   static void __init ap4evb_init(void)
>   {
>   	sh7372_pinmux_init();


-- 
Damian Hobson-Garcia
IGEL Co.,Ltd
http://www.igel.co.jp

[PATCH 4/4] video: switch OMAP LCD MIPI driver to use the common

[Guennadi Liakhovetski]

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/video/omap/lcd_mipid.c |   37 ++++++++++++++-----------------------
 1 files changed, 14 insertions(+), 23 deletions(-)

diff --git a/drivers/video/omap/lcd_mipid.c b/drivers/video/omap/lcd_mipid.c
index abe1c76..523393e 100644
--- a/drivers/video/omap/lcd_mipid.c
+++ b/drivers/video/omap/lcd_mipid.c
@@ -23,23 +23,14 @@
 #include <linux/workqueue.h>
 #include <linux/spi/spi.h>
 
+#include <video/mipi_dsi.h>
+
 #include <plat/lcd_mipid.h>
 
 #include "omapfb.h"
 
 #define MIPID_MODULE_NAME		"lcd_mipid"
 
-#define MIPID_CMD_READ_DISP_ID		0x04
-#define MIPID_CMD_READ_RED		0x06
-#define MIPID_CMD_READ_GREEN		0x07
-#define MIPID_CMD_READ_BLUE		0x08
-#define MIPID_CMD_READ_DISP_STATUS	0x09
-#define MIPID_CMD_RDDSDR		0x0F
-#define MIPID_CMD_SLEEP_IN		0x10
-#define MIPID_CMD_SLEEP_OUT		0x11
-#define MIPID_CMD_DISP_OFF		0x28
-#define MIPID_CMD_DISP_ON		0x29
-
 #define MIPID_ESD_CHECK_PERIOD		msecs_to_jiffies(5000)
 
 #define to_mipid_device(p)		container_of(p, struct mipid_device, \
@@ -152,14 +143,14 @@ static void set_data_lines(struct mipid_device *md, int data_lines)
 		par = 0x170;
 		break;
 	}
-	mipid_write(md, 0x3a, (u8 *)&par, 2);
+	mipid_write(md, MIPI_DCS_SET_PIXEL_FORMAT, (u8 *)&par, 2);
 }
 
 static void send_init_string(struct mipid_device *md)
 {
 	u16 initpar[] = { 0x0102, 0x0100, 0x0100 };
 
-	mipid_write(md, 0xc2, (u8 *)initpar, sizeof(initpar));
+	mipid_write(md, 0xc2, (u8 *)initpar, sizeof(initpar) * 2);
 	set_data_lines(md, md->panel.data_lines);
 }
 
@@ -184,9 +175,9 @@ static void set_sleep_mode(struct mipid_device *md, int on)
 	int cmd, sleep_time = 50;
 
 	if (on)
-		cmd = MIPID_CMD_SLEEP_IN;
+		cmd = MIPI_DCS_ENTER_SLEEP_MODE;
 	else
-		cmd = MIPID_CMD_SLEEP_OUT;
+		cmd = MIPI_DCS_EXIT_SLEEP_MODE;
 	hw_guard_wait(md);
 	mipid_cmd(md, cmd);
 	hw_guard_start(md, 120);
@@ -203,7 +194,7 @@ static void set_sleep_mode(struct mipid_device *md, int on)
 
 static void set_display_state(struct mipid_device *md, int enabled)
 {
-	int cmd = enabled ? MIPID_CMD_DISP_ON : MIPID_CMD_DISP_OFF;
+	int cmd = enabled ? MIPI_DCS_SET_DISPLAY_ON : MIPI_DCS_SET_DISPLAY_OFF;
 
 	mipid_cmd(md, cmd);
 }
@@ -258,9 +249,9 @@ static u16 read_first_pixel(struct mipid_device *md)
 	u8 red, green, blue;
 
 	mutex_lock(&md->mutex);
-	mipid_read(md, MIPID_CMD_READ_RED, &red, 1);
-	mipid_read(md, MIPID_CMD_READ_GREEN, &green, 1);
-	mipid_read(md, MIPID_CMD_READ_BLUE, &blue, 1);
+	mipid_read(md, MIPI_DCS_GET_RED_CHANNEL, &red, 1);
+	mipid_read(md, MIPI_DCS_GET_GREEN_CHANNEL, &green, 1);
+	mipid_read(md, MIPI_DCS_GET_BLUE_CHANNEL, &blue, 1);
 	mutex_unlock(&md->mutex);
 
 	switch (md->panel.data_lines) {
@@ -330,9 +321,9 @@ static void ls041y3_esd_check_mode1(struct mipid_device *md)
 {
 	u8 state1, state2;
 
-	mipid_read(md, MIPID_CMD_RDDSDR, &state1, 1);
+	mipid_read(md, MIPI_DCS_GET_DIAGNOSTIC_RESULT, &state1, 1);
 	set_sleep_mode(md, 0);
-	mipid_read(md, MIPID_CMD_RDDSDR, &state2, 1);
+	mipid_read(md, MIPI_DCS_GET_DIAGNOSTIC_RESULT, &state2, 1);
 	dev_dbg(&md->spi->dev, "ESD mode 1 state1 %02x state2 %02x\n",
 		state1, state2);
 	/* Each sleep out command will trigger a self diagnostic and flip
@@ -459,7 +450,7 @@ static int panel_enabled(struct mipid_device *md)
 	u32 disp_status;
 	int enabled;
 
-	mipid_read(md, MIPID_CMD_READ_DISP_STATUS, (u8 *)&disp_status, 4);
+	mipid_read(md, MIPI_DCS_GET_DISPLAY_STATUS, (u8 *)&disp_status, 4);
 	disp_status = __be32_to_cpu(disp_status);
 	enabled = (disp_status & (1 << 17)) && (disp_status & (1 << 10));
 	dev_dbg(&md->spi->dev,
@@ -537,7 +528,7 @@ static int mipid_detect(struct mipid_device *md)
 		return -ENOENT;
 	}
 
-	mipid_read(md, MIPID_CMD_READ_DISP_ID, display_id, 3);
+	mipid_read(md, MIPI_DCS_GET_DISPLAY_ID, display_id, 3);
 	dev_dbg(&md->spi->dev, "MIPI display ID: %02x%02x%02x\n",
 		display_id[0], display_id[1], display_id[2]);
 
-- 
1.6.2.4

[PATCH 2.5/4] ARM: add LCDC and MIPI DSI-Tx clock definitions to

[Guennadi Liakhovetski]

Define clock objects for switching on and off clocks for LCDC0, LCDC1 and MIPI
DSI-Tx.

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

Sorry, one more patch to add to the patch-series. In fact nothing breaks 
if you apply this patch before or after the whole series, so, no 
regressions, but to get LCD to actually work you also need this one;) And 
best put it between patches 2 and 3 of the series - that's where 2.5 comes 
from.

 arch/arm/mach-shmobile/clock-sh7372.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-shmobile/clock-sh7372.c b/arch/arm/mach-shmobile/clock-sh7372.c
index abbc380..66ee47a 100644
--- a/arch/arm/mach-shmobile/clock-sh7372.c
+++ b/arch/arm/mach-shmobile/clock-sh7372.c
@@ -264,8 +264,12 @@ struct clk div6_clks[] = {
 
 #define R_CLK (&r_clk)
 #define HP_CLK (&div4_clks[DIV4_HP])
+#define B_CLK (&div4_clks[DIV4_B])
 
 static struct clk mstp_clks[] = {
+	MSTP("lcdc0", B_CLK, SMSTPCR1, 0, 0),
+	MSTP("lcdc1", B_CLK, SMSTPCR1, 17, 0),
+	MSTP("dsitx", B_CLK, SMSTPCR1, 18, 0),
 	MSTP("sdhi0", HP_CLK, SMSTPCR3, 14, 0),
 	MSTP("sdhi1", HP_CLK, SMSTPCR3, 13, 0),
 	MSTP("keysc0", R_CLK, SMSTPCR4, 3, 0),
-- 
1.6.2.4

[PATCH 1.5/4] sh: add a YUV422 output data format, that is also

[Guennadi Liakhovetski]

The LCDC block is allowed to use one of the two output data formats, when used
with MIPI DSI: RGB24 and YUV422. YUV422 is not currently handled by the LCDC
driver, but we have to add a define for it for MIPI. Fix indentation while 
at it.

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

Hm, this one is actually worse, it actually has to go in before patch 2/4. 
Sorry again.

 include/video/sh_mobile_lcdc.h |   39 +++++++++++++++++++++------------------
 1 files changed, 21 insertions(+), 18 deletions(-)

diff --git a/include/video/sh_mobile_lcdc.h b/include/video/sh_mobile_lcdc.h
index 2cc893f..5eaea78 100644
--- a/include/video/sh_mobile_lcdc.h
+++ b/include/video/sh_mobile_lcdc.h
@@ -3,24 +3,27 @@
 
 #include <linux/fb.h>
 
-enum { RGB8,   /* 24bpp, 8:8:8 */
-       RGB9,   /* 18bpp, 9:9 */
-       RGB12A, /* 24bpp, 12:12 */
-       RGB12B, /* 12bpp */
-       RGB16,  /* 16bpp */
-       RGB18,  /* 18bpp */
-       RGB24,  /* 24bpp */
-       SYS8A,  /* 24bpp, 8:8:8 */
-       SYS8B,  /* 18bpp, 8:8:2 */
-       SYS8C,  /* 18bpp, 2:8:8 */
-       SYS8D,  /* 16bpp, 8:8 */
-       SYS9,   /* 18bpp, 9:9 */
-       SYS12,  /* 24bpp, 12:12 */
-       SYS16A, /* 16bpp */
-       SYS16B, /* 18bpp, 16:2 */
-       SYS16C, /* 18bpp, 2:16 */
-       SYS18,  /* 18bpp */
-       SYS24 };/* 24bpp */
+enum {
+	RGB8,   /* 24bpp, 8:8:8 */
+	RGB9,   /* 18bpp, 9:9 */
+	RGB12A, /* 24bpp, 12:12 */
+	RGB12B, /* 12bpp */
+	RGB16,  /* 16bpp */
+	RGB18,  /* 18bpp */
+	RGB24,  /* 24bpp */
+	YUV422, /* 16bpp */
+	SYS8A,  /* 24bpp, 8:8:8 */
+	SYS8B,  /* 18bpp, 8:8:2 */
+	SYS8C,  /* 18bpp, 2:8:8 */
+	SYS8D,  /* 16bpp, 8:8 */
+	SYS9,   /* 18bpp, 9:9 */
+	SYS12,  /* 24bpp, 12:12 */
+	SYS16A, /* 16bpp */
+	SYS16B, /* 18bpp, 16:2 */
+	SYS16C, /* 18bpp, 2:16 */
+	SYS18,  /* 18bpp */
+	SYS24,  /* 24bpp */
+};
 
 enum { LCDC_CHAN_DISABLED = 0,
        LCDC_CHAN_MAINLCD,
-- 
1.6.2.4

Re: [PATCH 1.5/4] sh: add a YUV422 output data format, that is also

[Damian]

(4:59), Guennadi Liakhovetski wrote:
> The LCDC block is allowed to use one of the two output data formats, when used
> with MIPI DSI: RGB24 and YUV422. YUV422 is not currently handled by the LCDC
> driver, but we have to add a define for it for MIPI. Fix indentation while
> at it.
> 
> Signed-off-by: Guennadi Liakhovetski<g.liakhovetski@gmx.de>
> ---
> 
> Hm, this one is actually worse, it actually has to go in before patch 2/4.
> Sorry again.
>
Console framebuffer tested on sh-2.6 tree and sh/dmaengine branch with
the necessary clock and intc patches applied.
Boot logo and framebuffer console displayed with no problems or unusual
effects.

Tested-by: Damian Hobson-Garcia <dhobsong@igel.co.jp>

>   include/video/sh_mobile_lcdc.h |   39 +++++++++++++++++++++------------------
>   1 files changed, 21 insertions(+), 18 deletions(-)
> 
> diff --git a/include/video/sh_mobile_lcdc.h b/include/video/sh_mobile_lcdc.h
> index 2cc893f..5eaea78 100644
> --- a/include/video/sh_mobile_lcdc.h
> +++ b/include/video/sh_mobile_lcdc.h
> @@ -3,24 +3,27 @@
> 
>   #include<linux/fb.h>
> 
> -enum { RGB8,   /* 24bpp, 8:8:8 */
> -       RGB9,   /* 18bpp, 9:9 */
> -       RGB12A, /* 24bpp, 12:12 */
> -       RGB12B, /* 12bpp */
> -       RGB16,  /* 16bpp */
> -       RGB18,  /* 18bpp */
> -       RGB24,  /* 24bpp */
> -       SYS8A,  /* 24bpp, 8:8:8 */
> -       SYS8B,  /* 18bpp, 8:8:2 */
> -       SYS8C,  /* 18bpp, 2:8:8 */
> -       SYS8D,  /* 16bpp, 8:8 */
> -       SYS9,   /* 18bpp, 9:9 */
> -       SYS12,  /* 24bpp, 12:12 */
> -       SYS16A, /* 16bpp */
> -       SYS16B, /* 18bpp, 16:2 */
> -       SYS16C, /* 18bpp, 2:16 */
> -       SYS18,  /* 18bpp */
> -       SYS24 };/* 24bpp */
> +enum {
> +	RGB8,   /* 24bpp, 8:8:8 */
> +	RGB9,   /* 18bpp, 9:9 */
> +	RGB12A, /* 24bpp, 12:12 */
> +	RGB12B, /* 12bpp */
> +	RGB16,  /* 16bpp */
> +	RGB18,  /* 18bpp */
> +	RGB24,  /* 24bpp */
> +	YUV422, /* 16bpp */
> +	SYS8A,  /* 24bpp, 8:8:8 */
> +	SYS8B,  /* 18bpp, 8:8:2 */
> +	SYS8C,  /* 18bpp, 2:8:8 */
> +	SYS8D,  /* 16bpp, 8:8 */
> +	SYS9,   /* 18bpp, 9:9 */
> +	SYS12,  /* 24bpp, 12:12 */
> +	SYS16A, /* 16bpp */
> +	SYS16B, /* 18bpp, 16:2 */
> +	SYS16C, /* 18bpp, 2:16 */
> +	SYS18,  /* 18bpp */
> +	SYS24,  /* 24bpp */
> +};
> 
>   enum { LCDC_CHAN_DISABLED = 0,
>          LCDC_CHAN_MAINLCD,


-- 
Damian Hobson-Garcia
IGEL Co.,Ltd
http://www.igel.co.jp