[PATCH v3 0/7] picodlp projector driver

[PATCH v3 0/7] picodlp projector driver

[Mayuresh Janorkar]

picodlp projector is supported by OMAP.
OMAP4430 SDP and EVM boards have an on board projector called as picodlp projector.
picodlp would be connected to display sub system as a display panel.
It has a dlp pico processor dpp2600.

The panel would be connected using 24 bit parallel interface.
GPIO pin 59 called as DISPLAY SELECT pin selects either secondary lcd
or picodlp.

It is a WVGA panel with 864 X 480 resolution.

To know more about picodlp please visit:
http://omappedia.org/wiki/PicoDLP_projector_guide

GPIO pin settings are required for powering_on the DLP and it is done in board file.
The programming sequence states that processor has to send LOW on PWRGOOD (GPIO 45)
then wait for a fraction of seconds and then send HIGH.
EMU_DONE pin (GPIO 44) would be set to low once the DLP is ready for operation.

Configuartion of picodlp involves passing dpp2600 commands through i2c.
All these commands are defined in a panel header file.
And more info on the commands can be found at:
https://focus.ti.com/myti/docs/extranet.tsp?sectionId=403

About DLP (Digital Light Processing):
DLP is Texas Instruments award-winning display technology which has powered
the worlds top projectors and displays, delivering pictures rich with color,
contrast, clarity and brightness to screens of all sizes.
Every DLP chip features an array of up to 2.2 million microscopic mirrors that
switch at ultra high speeds  an innovative advantage that remains cutting edge
and ideal for current and future applications alike. The results are
high-resolution, highly reliable, razor-sharp images, that even work with
fast motion video.
To learn more about DLP technology, please visit www.DLP.com

picodlp on OMAP4430 boards would make use of same technology.
picodlp makes use of i2c bus device at 0x1b address for sending configuration
commands to panel. In software picodlp panel driver has an i2c client.

To know more about picodlp configuration commands please visit:
http://focus.ti.com/lit/ug/dlpu002a/dlpu002a.pdf
The link talks more about the timing specific things:
http://focus.ti.com/lit/ds/dlps019b/dlps019b.pdf

To know more about i2c_client model please visit:
http://lxr.linux.no/#linux+v2.6.38/Documentation/i2c/writing-clients

------------------------------------------------------------------------------
These patches have been developed on top of master branch of
Tomi's gitorious tree.

I am maintaining a gitorious tree for these patches and can be found at:
http://gitorious.org/~mayuresh/linux-omap-dss2/mayuresh-picodlp/commits/picodlp

The driver has been tested when compiled as a module.

Validated with a Penguin logos on OMAP4430 SDP ES2.1

Checkpatch.pl warnings:
WARNING: msleep < 20ms can sleep for up to 20ms; see Documentation/timers/timers-howto.txt
I think this warning can be ignored

New compilation warnings introduced: Nil

Changes since v2:
		1. Merged picodlp panel patches into a single patch
		2. Changed GPIO names and handling
		3. Minimized sleep
		4. DMA_DONE polling is introduced
		
-----------------------------------------------------------------------------
Mayuresh Janorkar (3):
  OMAP: DSS: Adding a header file for picodlp panel data
  OMAP4: DSS: Adding a picodlp in OMAP4430 SDP board file
  OMAP: DSS: Add picodlp panel driver

Mythri P K (1):
  OMAP: DSS: Adding a picodlp panel header file

 arch/arm/mach-omap2/board-4430sdp.c             |   58 +++
 arch/arm/plat-omap/include/plat/panel-picodlp.h |   23 +
 drivers/video/omap2/displays/Kconfig            |    7 +
 drivers/video/omap2/displays/Makefile           |    1 +
 drivers/video/omap2/displays/panel-picodlp.c    |  627 +++++++++++++++++++++++
 drivers/video/omap2/displays/panel-picodlp.h    |  288 +++++++++++
 6 files changed, 1004 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/plat-omap/include/plat/panel-picodlp.h
 create mode 100644 drivers/video/omap2/displays/panel-picodlp.c
 create mode 100644 drivers/video/omap2/displays/panel-picodlp.h

[PATCH 1/4] OMAP: DSS: Adding a header file for picodlp data

[Mayuresh Janorkar]

picodlp is a TI projector supported by OMAP
picodlp is interfaced with host processor using pwrgood and emu_done pins.
In programming sequence it is required to send high to pwrgood to low
wait for a fraction of seconds and then wait for emu_done signal to go low.
picodlp would be ready to send/ receive i2c commands 500 mili-seconds after
emu_done goes low.

A new header file has been added for panel data and
picodlp_panel_data struct has been introduced

Signed-off-by: Mayuresh Janorkar <mayur@ti.com>
---
Changes since v2:
	Changed gpio names to emu_done_gpio and pwrgood_gpio

Changes since v1: Nil

 arch/arm/plat-omap/include/plat/panel-picodlp.h |   23 +++++++++++++++++++++++
 1 files changed, 23 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/plat-omap/include/plat/panel-picodlp.h

diff --git a/arch/arm/plat-omap/include/plat/panel-picodlp.h b/arch/arm/plat-omap/include/plat/panel-picodlp.h
new file mode 100644
index 0000000..333f7d6
--- /dev/null
+++ b/arch/arm/plat-omap/include/plat/panel-picodlp.h
@@ -0,0 +1,23 @@
+/*
+ * panel data for picodlp panel
+ *
+ * Copyright (C) 2011 Texas Instruments
+ *
+ * Author: Mayuresh Janorkar <mayur@ti.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 __ARCH_ARM_PLAT_PANEL_PICODLP_H
+#define __ARCH_ARM_PLAT_PANEL_PICODLP_H
+/**
+ * struct : picodlp panel data
+ * picodlp_adapter_id:	i2c_adapter number for picodlp
+ */
+struct picodlp_panel_data {
+	int picodlp_adapter_id;
+	int emu_done_gpio;
+	int pwrgood_gpio;
+};
+#endif /* __ARCH_ARM_PLAT_PANEL_PICODLP_H */
-- 
1.7.1

[PATCH v3 2/4] OMAP: DSS: Adding a picodlp header file

[Mayuresh Janorkar]

From: Mythri P K <mythripk@ti.com>

picodlp is a projector connected supported by OMAP.

DLP used in OMAP4 is dpp2600 (DLP Pico Projector)
The DLP requires commands to be sent over i2c for configurations,
Commands are defined together in this header file

To know more about dpp2600 commands please visit:
https://focus.ti.com/myti/docs/extranet.tsp?sectionId=403

Signed-off-by: Mythri P K <mythripk@ti.com>
Signed-off-by: Mayuresh Janorkar <mayur@ti.com>
---
Changes since v2:
		Added DMA_STATUS definition

Changes since v1: Nil

 drivers/video/omap2/displays/panel-picodlp.h |  288 ++++++++++++++++++++++++++
 1 files changed, 288 insertions(+), 0 deletions(-)
 create mode 100644 drivers/video/omap2/displays/panel-picodlp.h

diff --git a/drivers/video/omap2/displays/panel-picodlp.h b/drivers/video/omap2/displays/panel-picodlp.h
new file mode 100644
index 0000000..a34b431
--- /dev/null
+++ b/drivers/video/omap2/displays/panel-picodlp.h
@@ -0,0 +1,288 @@
+/*
+ * Header file required by picodlp panel driver
+ *
+ * Copyright (C) 2009-2011 Texas Instruments
+ * Author: Mythri P K <mythripk@ti.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.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+#ifndef __OMAP2_DISPLAY_PANEL_PICODLP_H
+#define __OMAP2_DISPLAY_PANEL_PICODLP_H
+
+/* Commands used for configuring picodlp panel */
+
+#define MAIN_STATUS			0x03
+#define PBC_CONTROL			0x08
+#define INPUT_SOURCE			0x0B
+#define INPUT_RESOLUTION		0x0C
+#define DATA_FORMAT			0x0D
+#define IMG_ROTATION			0x0E
+#define LONG_FLIP			0x0F
+#define SHORT_FLIP			0x10
+#define TEST_PAT_SELECT			0x11
+#define R_DRIVE_CURRENT			0x12
+#define G_DRIVE_CURRENT			0x13
+#define B_DRIVE_CURRENT			0x14
+#define READ_REG_SELECT			0x15
+#define RGB_DRIVER_ENABLE		0x16
+
+#define CPU_IF_MODE			0x18
+#define FRAME_RATE			0x19
+#define CPU_IF_SYNC_METHOD		0x1A
+#define CPU_IF_SOF			0x1B
+#define CPU_IF_EOF			0x1C
+#define CPU_IF_SLEEP			0x1D
+
+#define SEQUENCE_MODE			0x1E
+#define SOFT_RESET			0x1F
+#define FRONT_END_RESET			0x21
+#define AUTO_PWR_ENABLE			0x22
+
+#define VSYNC_LINE_DELAY		0x23
+#define CPU_PI_HORIZ_START		0x24
+#define CPU_PI_VERT_START		0x25
+#define CPU_PI_HORIZ_WIDTH		0x26
+#define CPU_PI_VERT_HEIGHT		0x27
+
+#define PIXEL_MASK_CROP			0x28
+#define CROP_FIRST_LINE			0x29
+#define CROP_LAST_LINE			0x2A
+#define CROP_FIRST_PIXEL		0x2B
+#define CROP_LAST_PIXEL			0x2C
+#define DMD_PARK_TRIGGER		0x2D
+
+#define MISC_REG			0x30
+
+/* AGC registers */
+#define AGC_CTRL			0x50
+#define AGC_CLIPPED_PIXS		0x55
+#define AGC_BRIGHT_PIXS			0x56
+#define AGC_BG_PIXS			0x57
+#define AGC_SAFETY_MARGIN		0x17
+
+/* Color Coordinate Adjustment registers */
+#define CCA_ENABLE		0x5E
+#define CCA_C1A			0x5F
+#define CCA_C1B			0x60
+#define CCA_C1C			0x61
+#define CCA_C2A			0x62
+#define CCA_C2B			0x63
+#define CCA_C2C			0x64
+#define CCA_C3A			0x65
+#define CCA_C3B			0x66
+#define CCA_C3C			0x67
+#define CCA_C7A			0x71
+#define CCA_C7B			0x72
+#define CCA_C7C			0x73
+
+/**
+ * DLP Pico Processor 2600 comes with flash
+ * We can do DMA operations from flash for accessing Look Up Tables
+ */
+#define DMA_STATUS			0x100
+#define FLASH_ADDR_BYTES		0x74
+#define FLASH_DUMMY_BYTES		0x75
+#define FLASH_WRITE_BYTES		0x76
+#define FLASH_READ_BYTES		0x77
+#define FLASH_OPCODE			0x78
+#define FLASH_START_ADDR		0x79
+#define FLASH_DUMMY2			0x7A
+#define FLASH_WRITE_DATA		0x7B
+
+#define TEMPORAL_DITH_DISABLE		0x7E
+#define SEQ_CONTROL			0x82
+#define SEQ_VECTOR			0x83
+
+/* DMD is Digital Micromirror Device */
+#define DMD_BLOCK_COUNT			0x84
+#define DMD_VCC_CONTROL			0x86
+#define DMD_PARK_PULSE_COUNT		0x87
+#define DMD_PARK_PULSE_WIDTH		0x88
+#define DMD_PARK_DELAY			0x89
+#define DMD_SHADOW_ENABLE		0x8E
+#define SEQ_STATUS			0x8F
+#define FLASH_CLOCK_CONTROL		0x98
+#define DMD_PARK			0x2D
+
+#define SDRAM_BIST_ENABLE		0x46
+#define DDR_DRIVER_STRENGTH		0x9A
+#define SDC_ENABLE			0x9D
+#define SDC_BUFF_SWAP_DISABLE		0xA3
+#define CURTAIN_CONTROL			0xA6
+#define DDR_BUS_SWAP_ENABLE		0xA7
+#define DMD_TRC_ENABLE			0xA8
+#define DMD_BUS_SWAP_ENABLE		0xA9
+
+#define ACTGEN_ENABLE			0xAE
+#define ACTGEN_CONTROL			0xAF
+#define ACTGEN_HORIZ_BP			0xB0
+#define ACTGEN_VERT_BP			0xB1
+
+/* Look Up Table access */
+#define CMT_SPLASH_LUT_START_ADDR	0xFA
+#define CMT_SPLASH_LUT_DEST_SELECT	0xFB
+#define CMT_SPLASH_LUT_DATA		0xFC
+#define SEQ_RESET_LUT_START_ADDR	0xFD
+#define SEQ_RESET_LUT_DEST_SELECT	0xFE
+#define SEQ_RESET_LUT_DATA		0xFF
+
+/* Input source definitions */
+#define PARALLEL_RGB		0
+#define INT_TEST_PATTERN	1
+#define SPLASH_SCREEN		2
+#define CPU_INTF		3
+#define BT656			4
+
+/* Standard input resolution definitions */
+#define QWVGA_LANDSCAPE		3	/* (427h*240v) */
+#define WVGA_864_LANDSCAPE	21	/* (864h*480v) */
+#define WVGA_DMD_OPTICAL_TEST	35	/* (608h*684v) */
+
+/* Standard data format definitions */
+#define RGB565			0
+#define RGB666			1
+#define RGB888			2
+
+/* Test Pattern definitions */
+#define TPG_CHECKERBOARD	0
+#define TPG_BLACK		1
+#define TPG_WHITE		2
+#define TPG_RED			3
+#define TPG_BLUE		4
+#define TPG_GREEN		5
+#define TPG_VLINES_BLACK	6
+#define TPG_HLINES_BLACK	7
+#define TPG_VLINES_ALT		8
+#define TPG_HLINES_ALT		9
+#define TPG_DIAG_LINES		10
+#define TPG_GREYRAMP_VERT	11
+#define TPG_GREYRAMP_HORIZ	12
+#define TPG_ANSI_CHECKERBOARD	13
+
+/* sequence mode definitions */
+#define SEQ_FREE_RUN		0
+#define SEQ_LOCK		1
+
+/* curtain color definitions */
+#define CURTAIN_BLACK		0
+#define CURTAIN_RED		1
+#define CURTAIN_GREEN		2
+#define CURTAIN_BLUE		3
+#define CURTAIN_YELLOW		4
+#define CURTAIN_MAGENTA		5
+#define CURTAIN_CYAN		6
+#define CURTAIN_WHITE		7
+
+/* LUT definitions */
+#define CMT_LUT_NONE		0
+#define CMT_LUT_GREEN		1
+#define CMT_LUT_RED		2
+#define CMT_LUT_BLUE		3
+#define CMT_LUT_ALL		4
+#define SPLASH_LUT		5
+
+#define SEQ_LUT_NONE		0
+#define SEQ_DRC_LUT_0		1
+#define SEQ_DRC_LUT_1		2
+#define SEQ_DRC_LUT_2		3
+#define SEQ_DRC_LUT_3		4
+#define SEQ_SEQ_LUT		5
+#define SEQ_DRC_LUT_ALL		6
+#define WPC_PROGRAM_LUT		7
+
+#define BITSTREAM_START_ADDR		0x00000000
+#define BITSTREAM_SIZE			0x00040000
+
+#define WPC_FW_0_START_ADDR		0x00040000
+#define WPC_FW_0_SIZE			0x00000ce8
+
+#define SEQUENCE_0_START_ADDR		0x00044000
+#define SEQUENCE_0_SIZE			0x00001000
+
+#define SEQUENCE_1_START_ADDR		0x00045000
+#define SEQUENCE_1_SIZE			0x00000d10
+
+#define SEQUENCE_2_START_ADDR		0x00046000
+#define SEQUENCE_2_SIZE			0x00000d10
+
+#define SEQUENCE_3_START_ADDR		0x00047000
+#define SEQUENCE_3_SIZE			0x00000d10
+
+#define SEQUENCE_4_START_ADDR		0x00048000
+#define SEQUENCE_4_SIZE			0x00000d10
+
+#define SEQUENCE_5_START_ADDR		0x00049000
+#define SEQUENCE_5_SIZE			0x00000d10
+
+#define SEQUENCE_6_START_ADDR		0x0004a000
+#define SEQUENCE_6_SIZE			0x00000d10
+
+#define CMT_LUT_0_START_ADDR		0x0004b200
+#define CMT_LUT_0_SIZE			0x00000600
+
+#define CMT_LUT_1_START_ADDR		0x0004b800
+#define CMT_LUT_1_SIZE			0x00000600
+
+#define CMT_LUT_2_START_ADDR		0x0004be00
+#define CMT_LUT_2_SIZE			0x00000600
+
+#define CMT_LUT_3_START_ADDR		0x0004c400
+#define CMT_LUT_3_SIZE			0x00000600
+
+#define CMT_LUT_4_START_ADDR		0x0004ca00
+#define CMT_LUT_4_SIZE			0x00000600
+
+#define CMT_LUT_5_START_ADDR		0x0004d000
+#define CMT_LUT_5_SIZE			0x00000600
+
+#define CMT_LUT_6_START_ADDR		0x0004d600
+#define CMT_LUT_6_SIZE			0x00000600
+
+#define DRC_TABLE_0_START_ADDR		0x0004dc00
+#define DRC_TABLE_0_SIZE		0x00000100
+
+#define SPLASH_0_START_ADDR		0x0004dd00
+#define SPLASH_0_SIZE			0x00032280
+
+#define SEQUENCE_7_START_ADDR		0x00080000
+#define SEQUENCE_7_SIZE			0x00000d10
+
+#define SEQUENCE_8_START_ADDR		0x00081800
+#define SEQUENCE_8_SIZE			0x00000d10
+
+#define SEQUENCE_9_START_ADDR		0x00083000
+#define SEQUENCE_9_SIZE			0x00000d10
+
+#define CMT_LUT_7_START_ADDR		0x0008e000
+#define CMT_LUT_7_SIZE			0x00000600
+
+#define CMT_LUT_8_START_ADDR		0x0008e800
+#define CMT_LUT_8_SIZE			0x00000600
+
+#define CMT_LUT_9_START_ADDR		0x0008f000
+#define CMT_LUT_9_SIZE			0x00000600
+
+#define SPLASH_1_START_ADDR		0x0009a000
+#define SPLASH_1_SIZE			0x00032280
+
+#define SPLASH_2_START_ADDR		0x000cd000
+#define SPLASH_2_SIZE			0x00032280
+
+#define SPLASH_3_START_ADDR		0x00100000
+#define SPLASH_3_SIZE			0x00032280
+
+#define OPT_SPLASH_0_START_ADDR		0x00134000
+#define OPT_SPLASH_0_SIZE		0x000cb100
+
+#endif
-- 
1.7.1

[PATCH v3 3/4] OMAP4: DSS: Adding a picodlp in OMAP4430 SDP board file

[Mayuresh Janorkar]

An on-board projector named picodlp is available for OMAP4430 SDP.

Entry for this picodlp as a panel is being added in dss_devices array
to the board file.
It needs 4 GPIO pins for interfacing with host processor
and these are defined and two of them are configured in board file.
Two GPIOs power_on and display_select are configured here.
picodlp also needs an i2c client over i2c controller-2 at address 0x1b.

Signed-off-by: Mayuresh Janorkar <mayur@ti.com>
Signed-off-by: Mythri P K <mythripk@ti.com>
---
Changes since v2:
	 Changed GPIO names to power_on, emu_done and pwrgood

 arch/arm/mach-omap2/board-4430sdp.c |   58 +++++++++++++++++++++++++++++++++++
 1 files changed, 58 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/board-4430sdp.c b/arch/arm/mach-omap2/board-4430sdp.c
index 570e83f..060aded 100644
--- a/arch/arm/mach-omap2/board-4430sdp.c
+++ b/arch/arm/mach-omap2/board-4430sdp.c
@@ -38,6 +38,7 @@
 #include <plat/omap4-keypad.h>
 #include <plat/display.h>
 #include <plat/nokia-dsi-panel.h>
+#include <plat/panel-picodlp.h>
 
 #include "mux.h"
 #include "hsmmc.h"
@@ -52,6 +53,8 @@
 #define HDMI_GPIO_HPD 60 /* Hot plug pin for HDMI */
 #define HDMI_GPIO_LS_OE 41 /* Level shifter for HDMI */
 #define LCD_BL_GPIO		27	/* LCD Backlight GPIO */
+#define OMAP4_DLP_POWER_ON_GPIO		40
+#define OMAP4_DLP_DISPLAY_SELECT_GPIO	59
 /* PWM2 and TOGGLE3 register offsets */
 #define LED_PWM2ON		0x03
 #define LED_PWM2OFF		0x04
@@ -812,9 +815,64 @@ static struct omap_dss_device sdp4430_hdmi_device = {
 	.channel = OMAP_DSS_CHANNEL_DIGIT,
 };
 
+static struct picodlp_panel_data sdp4430_picodlp_pdata = {
+	.picodlp_adapter_id	= 2,
+	.emu_done_gpio		= 44,
+	.pwrgood_gpio		= 45,
+};
+
+static int sdp4430_panel_enable_picodlp(struct omap_dss_device *dssdev)
+{
+	int status;
+	struct gpio picodlp_gpios[] = {
+		{OMAP4_DLP_POWER_ON_GPIO, GPIOF_OUT_INIT_LOW,
+			"DLP POWER ON"},
+		{OMAP4_DLP_DISPLAY_SELECT_GPIO, GPIOF_OUT_INIT_LOW,
+			"DLP DISPLAY SELECT"},
+	};
+
+	status = gpio_request_array(picodlp_gpios, ARRAY_SIZE(picodlp_gpios));
+	if (status)
+		goto error1;
+
+	gpio_set_value(OMAP4_DLP_DISPLAY_SELECT_GPIO, 1);
+	gpio_set_value(OMAP4_DLP_POWER_ON_GPIO, 1);
+
+	return 0;
+error1:
+	pr_err("Cannot request OMAP4_DLP_GPIOs, error %d\n", status);
+	return status;
+}
+
+static void sdp4430_panel_disable_picodlp(struct omap_dss_device *dssdev)
+{
+	struct gpio picodlp_gpios[] = {
+		{OMAP4_DLP_POWER_ON_GPIO, GPIOF_OUT_INIT_LOW,
+			"DLP POWER ON"},
+		{OMAP4_DLP_DISPLAY_SELECT_GPIO, GPIOF_OUT_INIT_LOW,
+			"DLP DISPLAY SELECT"},
+	};
+	gpio_set_value(OMAP4_DLP_POWER_ON_GPIO, 0);
+	gpio_set_value(OMAP4_DLP_DISPLAY_SELECT_GPIO, 0);
+
+	gpio_free_array(picodlp_gpios, ARRAY_SIZE(picodlp_gpios));
+}
+
+static struct omap_dss_device sdp4430_picodlp_device = {
+	.name			= "picodlp",
+	.driver_name		= "picodlp_panel",
+	.type			= OMAP_DISPLAY_TYPE_DPI,
+	.phy.dpi.data_lines	= 24,
+	.channel		= OMAP_DSS_CHANNEL_LCD2,
+	.platform_enable	= sdp4430_panel_enable_picodlp,
+	.platform_disable	= sdp4430_panel_disable_picodlp,
+	.data			= &sdp4430_picodlp_pdata,
+};
+
 static struct omap_dss_device *sdp4430_dss_devices[] = {
 	&sdp4430_lcd_device,
 	&sdp4430_hdmi_device,
+	&sdp4430_picodlp_device,
 };
 
 static struct omap_dss_board_info sdp4430_dss_data = {
-- 
1.7.1

[PATCH v3 4/4] OMAP: DSS: Add picodlp panel driver

[Mayuresh Janorkar]

picodlp panel driver is required for driving DLP dpp2600.
It consists of a dss driver and an i2c_client.

i2c_client is required for sending commands to dpp (DLP Pico Projector).

The power-up sequence consists of:
Setting PWRGOOD to a logic low state. Once power is stable and thus the DPP2600
is stable, then PWRGOOD should be deactivated (Set to a logic high).
The DPP2600 will then perform a power-up initialization routine that will first
configure and lock its PLL followed by loading self configuration data from
external flash. DPP2600 would be activated and the EMU_DONE signal will be
driven high. After this once internal initialization routine,
which will take approximately 510ms, i2c commands can be sent.

To know more please visit: http://www.omappedia.org/wiki/PicoDLP_projector_guide

Signed-off-by: Mayuresh Janorkar <mayur@ti.com>
---
Changes since v2:
	Merged panel picodlp patches into a single patch
	Introduced DMA_DONE polling between flash transfer
	Changed GPIO handling
	Reduced sleeps

 drivers/video/omap2/displays/Kconfig         |    7 +
 drivers/video/omap2/displays/Makefile        |    1 +
 drivers/video/omap2/displays/panel-picodlp.c |  627 ++++++++++++++++++++++++++
 3 files changed, 635 insertions(+), 0 deletions(-)
 create mode 100644 drivers/video/omap2/displays/panel-picodlp.c

diff --git a/drivers/video/omap2/displays/Kconfig b/drivers/video/omap2/displays/Kconfig
index 609a280..0b05593 100644
--- a/drivers/video/omap2/displays/Kconfig
+++ b/drivers/video/omap2/displays/Kconfig
@@ -30,6 +30,13 @@ config PANEL_NEC_NL8048HL11_01B
 		This NEC NL8048HL11-01B panel is TFT LCD
 		used in the Zoom2/3/3630 sdp boards.
 
+config PANEL_PICODLP
+	tristate "OMAP PICO DLP Panel"
+	depends on OMAP2_DSS
+	help
+		Projector Panel used in TI's SDP4430 and EVM boards
+		For more info please visit http://www.dlp.com/projector/
+
 config PANEL_TAAL
         tristate "Taal DSI Panel"
         depends on OMAP2_DSS_DSI
diff --git a/drivers/video/omap2/displays/Makefile b/drivers/video/omap2/displays/Makefile
index 0f601ab..d90f73c 100644
--- a/drivers/video/omap2/displays/Makefile
+++ b/drivers/video/omap2/displays/Makefile
@@ -4,5 +4,6 @@ obj-$(CONFIG_PANEL_SHARP_LS037V7DW01) += panel-sharp-ls037v7dw01.o
 obj-$(CONFIG_PANEL_NEC_NL8048HL11_01B) += panel-nec-nl8048hl11-01b.o
 
 obj-$(CONFIG_PANEL_TAAL) += panel-taal.o
+obj-$(CONFIG_PANEL_PICODLP) +=  panel-picodlp.o
 obj-$(CONFIG_PANEL_TPO_TD043MTEA1) += panel-tpo-td043mtea1.o
 obj-$(CONFIG_PANEL_ACX565AKM) += panel-acx565akm.o
diff --git a/drivers/video/omap2/displays/panel-picodlp.c b/drivers/video/omap2/displays/panel-picodlp.c
new file mode 100644
index 0000000..432f987
--- /dev/null
+++ b/drivers/video/omap2/displays/panel-picodlp.c
@@ -0,0 +1,627 @@
+/*
+
+ * picodlp panel driver
+ * picodlp_i2c_driver: i2c_client driver
+ *
+ * Copyright (C) 2009-2011 Texas Instruments
+ * Author: Mythri P K <mythripk@ti.com>
+ * Mayuresh Janorkar <mayur@ti.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.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/input.h>
+#include <linux/platform_device.h>
+#include <linux/interrupt.h>
+#include <linux/firmware.h>
+#include <linux/slab.h>
+#include <linux/mutex.h>
+#include <linux/i2c.h>
+#include <linux/delay.h>
+#include <linux/gpio.h>
+
+#include <plat/display.h>
+#include <plat/panel-picodlp.h>
+
+#include "panel-picodlp.h"
+
+#define DRIVER_NAME	"picodlp_i2c_driver"
+
+/* This defines the minit of data which is allowed into single write block */
+#define MAX_I2C_WRITE_BLOCK_SIZE	32
+#define PICO_MAJOR			1 /* 2 bits */
+#define PICO_MINOR			1 /* 2 bits */
+#define MAX_TRIAL_VALUE		50
+
+struct picodlp_data {
+	struct mutex lock;
+	struct i2c_client *picodlp_i2c_client;
+};
+
+static struct i2c_board_info picodlp_i2c_board_info = {
+	I2C_BOARD_INFO("picodlp_i2c_driver", 0x1b),
+};
+
+struct picodlp_i2c_data {
+	struct mutex xfer_lock;
+};
+
+struct i2c_device_id picodlp_i2c_id[] = {
+	{ "picodlp_i2c_driver", 0 },
+};
+
+struct picodlp_i2c_command {
+	u8 reg;
+	u32 value;
+};
+
+static struct omap_video_timings pico_ls_timings = {
+	.x_res		= 864,
+	.y_res		= 480,
+	.hsw		= 7,
+	.hfp		= 11,
+	.hbp		= 7,
+
+	.pixel_clock	= 19200,
+
+	.vsw		= 2,
+	.vfp		= 3,
+	.vbp		= 14,
+};
+
+static inline struct picodlp_panel_data
+		*get_panel_data(const struct omap_dss_device *dssdev)
+{
+	return (struct picodlp_panel_data *) dssdev->data;
+}
+
+static u32 picodlp_i2c_read(struct i2c_client *client, u8 reg)
+{
+	u8 read_cmd[] = {READ_REG_SELECT, reg}, data[4];
+	struct picodlp_i2c_data *picodlp_i2c_data = i2c_get_clientdata(client);
+	struct i2c_msg msg[2];
+
+	mutex_lock(&picodlp_i2c_data->xfer_lock);
+
+	msg[0].addr = client->addr;
+	msg[0].flags = 0;
+	msg[0].len = 2;
+	msg[0].buf = read_cmd;
+
+	msg[1].addr = client->addr;
+	msg[1].flags = I2C_M_RD;
+	msg[1].len = 4;
+	msg[1].buf = data;
+
+	i2c_transfer(client->adapter, msg, 2);
+	mutex_unlock(&picodlp_i2c_data->xfer_lock);
+	return (data[3] | (data[2] << 8) | (data[1] << 16) | (data[0] << 24));
+}
+
+static int picodlp_i2c_write_block(struct i2c_client *client,
+					u8 *data, int len)
+{
+	struct i2c_msg msg;
+	int i, r, msg_count = 1;
+
+	struct picodlp_i2c_data *picodlp_i2c_data = i2c_get_clientdata(client);
+
+	if (len < 1 || len > MAX_I2C_WRITE_BLOCK_SIZE) {
+		dev_err(&client->dev,
+			"too long syn_write_block len %d\n", len);
+		return -EIO;
+	}
+	mutex_lock(&picodlp_i2c_data->xfer_lock);
+
+	msg.addr = client->addr;
+	msg.flags = 0;
+	msg.len = len;
+	msg.buf = data;
+	r = i2c_transfer(client->adapter, &msg, msg_count);
+	mutex_unlock(&picodlp_i2c_data->xfer_lock);
+
+	/*
+	 * i2c_transfer returns:
+	 * number of messages sent in case of success
+	 * a negative error number in case of failure
+	 * i2c controller might timeout, in such a case sleep for sometime
+	 * and then retry
+	 */
+	if (r != msg_count)
+		goto err;
+
+	/* In case of success */
+	for (i = 0; i < len; i++)
+		dev_dbg(&client->dev,
+			"addr %x bw 0x%02x[%d]: 0x%02x\n",
+			client->addr, data[0] + i, i, data[i]);
+
+	return 0;
+err:
+	dev_err(&client->dev, "picodlp_i2c_write error\n");
+	return r;
+}
+
+static int picodlp_i2c_write(struct i2c_client *client, u8 reg, u32 value)
+{
+	u8 data[5];
+	int i;
+
+	data[0] = reg;
+	for (i = 1; i < 5; i++)
+		data[i] = (value >> (32 - (i) * 8)) & 0xFF;
+
+	return picodlp_i2c_write_block(client, data, 5);
+}
+
+static int picodlp_i2c_write_array(struct i2c_client *client,
+			const struct picodlp_i2c_command commands[],
+			int count)
+{
+	int i, r = 0;
+	for (i = 0; i < count; i++) {
+		r = picodlp_i2c_write(client, commands[i].reg,
+						commands[i].value);
+		if (r)
+			return r;
+	}
+	return r;
+}
+
+static bool picodlp_wait_for_dma_done(struct i2c_client *client)
+{
+	u8 trial = MAX_TRIAL_VALUE;
+
+	do {
+		msleep(1);
+		if (!trial--)
+			return -ETIMEDOUT;
+	} while (picodlp_i2c_read(client, MAIN_STATUS) & DMA_STATUS);
+
+	return 0;
+}
+
+/**
+ * picodlp_i2c_init:	i2c_initialization routine
+ * client:	i2c_client for communication
+ *
+ * @return
+ *		0	: Success, no error
+ *	error code	: Failure
+ */
+static int picodlp_i2c_init(struct i2c_client *client)
+{
+	int r;
+	static const struct picodlp_i2c_command init_cmd_set1[] = {
+		{SOFT_RESET, 1},
+		{DMD_PARK_TRIGGER, 1},
+		{MISC_REG, (PICO_MAJOR << 2 | PICO_MINOR)},
+		{SEQ_CONTROL, 0},
+		{SEQ_VECTOR, 0x100},
+		{DMD_BLOCK_COUNT, 7},
+		{DMD_VCC_CONTROL, 0x109},
+		{DMD_PARK_PULSE_COUNT, 0xA},
+		{DMD_PARK_PULSE_WIDTH, 0xB},
+		{DMD_PARK_DELAY, 0x2ED},
+		{DMD_SHADOW_ENABLE, 0},
+		{FLASH_OPCODE, 0xB},
+		{FLASH_DUMMY_BYTES, 1},
+		{FLASH_ADDR_BYTES, 3},
+		{PBC_CONTROL, 0},
+		{FLASH_START_ADDR, CMT_LUT_0_START_ADDR},
+		{FLASH_READ_BYTES, CMT_LUT_0_SIZE},
+		{CMT_SPLASH_LUT_START_ADDR, 0},
+		{CMT_SPLASH_LUT_DEST_SELECT, CMT_LUT_ALL},
+		{PBC_CONTROL, 1},
+	};
+
+	static const struct picodlp_i2c_command init_cmd_set2[] = {
+		{PBC_CONTROL, 0},
+		{CMT_SPLASH_LUT_DEST_SELECT, 0},
+		{PBC_CONTROL, 0},
+		{FLASH_START_ADDR, SEQUENCE_0_START_ADDR},
+		{FLASH_READ_BYTES, SEQUENCE_0_SIZE},
+		{SEQ_RESET_LUT_START_ADDR, 0},
+		{SEQ_RESET_LUT_DEST_SELECT, SEQ_SEQ_LUT},
+		{PBC_CONTROL, 1},
+	};
+
+	static const struct picodlp_i2c_command init_cmd_set3[] = {
+		{PBC_CONTROL, 0},
+		{SEQ_RESET_LUT_DEST_SELECT, 0},
+		{PBC_CONTROL, 0},
+		{FLASH_START_ADDR, DRC_TABLE_0_START_ADDR},
+		{FLASH_READ_BYTES, DRC_TABLE_0_SIZE},
+		{SEQ_RESET_LUT_START_ADDR, 0},
+		{SEQ_RESET_LUT_DEST_SELECT, SEQ_DRC_LUT_ALL},
+		{PBC_CONTROL, 1},
+	};
+
+	static const struct picodlp_i2c_command init_cmd_set4[] = {
+		{PBC_CONTROL, 0},
+		{SEQ_RESET_LUT_DEST_SELECT, 0},
+		{SDC_ENABLE, 1},
+		{AGC_CTRL, 7},
+		{CCA_C1A, 0x100},
+		{CCA_C1B, 0x0},
+		{CCA_C1C, 0x0},
+		{CCA_C2A, 0x0},
+		{CCA_C2B, 0x100},
+		{CCA_C2C, 0x0},
+		{CCA_C3A, 0x0},
+		{CCA_C3B, 0x0},
+		{CCA_C3C, 0x100},
+		{CCA_C7A, 0x100},
+		{CCA_C7B, 0x100},
+		{CCA_C7C, 0x100},
+		{CCA_ENABLE, 1},
+		{CPU_IF_MODE, 1},
+		{SHORT_FLIP, 1},
+		{CURTAIN_CONTROL, 0},
+		{DMD_PARK_TRIGGER, 0},
+		{R_DRIVE_CURRENT, 0x298},
+		{G_DRIVE_CURRENT, 0x298},
+		{B_DRIVE_CURRENT, 0x298},
+		{RGB_DRIVER_ENABLE, 7},
+		{SEQ_CONTROL, 0},
+		{ACTGEN_CONTROL, 0x10},
+		{SEQUENCE_MODE, SEQ_LOCK},
+		{DATA_FORMAT, RGB888},
+		{INPUT_RESOLUTION, WVGA_864_LANDSCAPE},
+		{INPUT_SOURCE, PARALLEL_RGB},
+		{CPU_IF_SYNC_METHOD, 1},
+		{SEQ_CONTROL, 1}
+	};
+
+	msleep(510);
+	r = picodlp_i2c_write_array(client, init_cmd_set1,
+						ARRAY_SIZE(init_cmd_set1));
+	if (r)
+		return r;
+
+	r = picodlp_wait_for_dma_done(client);
+	if (r)
+		return r;
+
+	r = picodlp_i2c_write_array(client, init_cmd_set2,
+					ARRAY_SIZE(init_cmd_set2));
+	if (r)
+		return r;
+
+	r = picodlp_wait_for_dma_done(client);
+	if (r)
+		return r;
+
+	r = picodlp_i2c_write_array(client, init_cmd_set3,
+					ARRAY_SIZE(init_cmd_set3));
+	if (r)
+		return r;
+
+	r = picodlp_wait_for_dma_done(client);
+	if (r)
+		return r;
+
+	r = picodlp_i2c_write_array(client, init_cmd_set4,
+					ARRAY_SIZE(init_cmd_set4));
+	if (r)
+		return r;
+
+	return 0;
+}
+
+static int picodlp_i2c_probe(struct i2c_client *client,
+		const struct i2c_device_id *id)
+{
+	struct picodlp_i2c_data *picodlp_i2c_data;
+
+	picodlp_i2c_data = kzalloc(sizeof(struct picodlp_i2c_data), GFP_KERNEL);
+
+	if (!picodlp_i2c_data)
+		return -ENOMEM;
+
+	i2c_set_clientdata(client, picodlp_i2c_data);
+
+	return 0;
+}
+
+static int picodlp_i2c_remove(struct i2c_client *client)
+{
+	struct picodlp_i2c_data *picodlp_i2c_data =
+					i2c_get_clientdata(client);
+
+	kfree(picodlp_i2c_data);
+	i2c_unregister_device(client);
+	return 0;
+}
+
+static struct i2c_driver picodlp_i2c_driver = {
+	.driver = {
+		.name	= "picodlp_i2c_driver",
+	},
+	.probe		= picodlp_i2c_probe,
+	.remove		= picodlp_i2c_remove,
+	.id_table	= picodlp_i2c_id,
+};
+
+static int picodlp_panel_power_on(struct omap_dss_device *dssdev)
+{
+	int r = 0, trial = MAX_TRIAL_VALUE;
+	struct picodlp_data *picod = dev_get_drvdata(&dssdev->dev);
+	struct picodlp_panel_data *picodlp_pdata;
+	struct picodlp_i2c_data *picodlp_i2c_data;
+
+	picodlp_pdata = get_panel_data(dssdev);
+	if (dssdev->platform_enable) {
+		r = dssdev->platform_enable(dssdev);
+		if (r)
+			return r;
+	}
+
+	gpio_set_value(picodlp_pdata->pwrgood_gpio, 0);
+	msleep(1);
+	gpio_set_value(picodlp_pdata->pwrgood_gpio, 1);
+
+	while (!gpio_get_value(picodlp_pdata->emu_done_gpio)) {
+		if (!trial--) {
+			dev_err(&dssdev->dev, "emu_done signal not"
+						" going down\n");
+			return -ETIMEDOUT;
+		}
+		msleep(5);
+	}
+
+	r = omapdss_dpi_display_enable(dssdev);
+	if (r) {
+		dev_err(&dssdev->dev, "failed to enable DPI\n");
+		goto err;
+	}
+	dssdev->state = OMAP_DSS_DISPLAY_ACTIVE;
+
+	picodlp_i2c_data =
+		i2c_get_clientdata(picod->picodlp_i2c_client);
+
+	r = picodlp_i2c_init(picod->picodlp_i2c_client);
+	if (r)
+		goto err;
+
+	return r;
+err:
+	if (dssdev->platform_disable)
+		dssdev->platform_disable(dssdev);
+
+	return r;
+}
+
+static void picodlp_panel_power_off(struct omap_dss_device *dssdev)
+{
+	struct picodlp_panel_data *picodlp_pdata = get_panel_data(dssdev);
+
+	omapdss_dpi_display_disable(dssdev);
+
+	gpio_set_value(picodlp_pdata->emu_done_gpio, 0);
+	gpio_set_value(picodlp_pdata->pwrgood_gpio, 0);
+
+	if (dssdev->platform_disable)
+		dssdev->platform_disable(dssdev);
+}
+
+static int picodlp_panel_probe(struct omap_dss_device *dssdev)
+{
+	struct picodlp_data *picod;
+	struct picodlp_panel_data *picodlp_pdata = get_panel_data(dssdev);
+	struct i2c_adapter *adapter;
+	struct i2c_client *picodlp_i2c_client;
+	struct picodlp_i2c_data *picodlp_i2c_data;
+	struct gpio picodlp_gpios[] = {
+		{picodlp_pdata->emu_done_gpio, GPIOF_IN, "DLP EMU DONE"},
+		{picodlp_pdata->pwrgood_gpio, GPIOF_INIT_LOW, "DLP PWRGOOD"},
+	};
+
+	int r = 0, picodlp_adapter_id;
+
+	dssdev->panel.config = OMAP_DSS_LCD_TFT | OMAP_DSS_LCD_ONOFF |
+				OMAP_DSS_LCD_IHS | OMAP_DSS_LCD_IVS;
+	dssdev->panel.acb = 0x0;
+	dssdev->panel.timings = pico_ls_timings;
+
+	picod =  kzalloc(sizeof(struct picodlp_data), GFP_KERNEL);
+	if (!picod)
+		return -ENOMEM;
+
+	mutex_init(&picod->lock);
+
+	r = gpio_request_array(picodlp_gpios, ARRAY_SIZE(picodlp_gpios));
+	if (r) {
+		dev_err(&dssdev->dev, "can't get dlp gpios\n");
+		goto err;
+	}
+
+	picodlp_adapter_id = picodlp_pdata->picodlp_adapter_id;
+
+	adapter = i2c_get_adapter(picodlp_adapter_id);
+	if (!adapter) {
+		dev_err(&dssdev->dev, "can't get i2c adapter\n");
+		r = -ENODEV;
+		goto err;
+	}
+
+	picodlp_i2c_client = i2c_new_device(adapter, &picodlp_i2c_board_info);
+	if (!picodlp_i2c_client) {
+		dev_err(&dssdev->dev, "can't add i2c device::"
+					 " picodlp_i2c_client is NULL\n");
+		r = -ENODEV;
+		goto err;
+	}
+
+	picod->picodlp_i2c_client = picodlp_i2c_client;
+
+	picodlp_i2c_data =
+		i2c_get_clientdata(picod->picodlp_i2c_client);
+
+	mutex_init(&picodlp_i2c_data->xfer_lock);
+	dev_set_drvdata(&dssdev->dev, picod);
+	return r;
+
+err:
+	gpio_free_array(picodlp_gpios, ARRAY_SIZE(picodlp_gpios));
+	kfree(picod);
+	return r;
+}
+
+static void picodlp_panel_remove(struct omap_dss_device *dssdev)
+{
+	struct picodlp_data *picod = dev_get_drvdata(&dssdev->dev);
+	struct picodlp_panel_data *picodlp_pdata = get_panel_data(dssdev);
+	struct gpio picodlp_gpios[] = {
+		{picodlp_pdata->emu_done_gpio, GPIOF_IN, "DLP EMU DONE"},
+		{picodlp_pdata->pwrgood_gpio, GPIOF_INIT_LOW, "DLP PWRGOOD"},
+	};
+
+	dev_set_drvdata(&dssdev->dev, NULL);
+	dev_dbg(&dssdev->dev, "removing picodlp panel\n");
+
+	gpio_free_array(picodlp_gpios, ARRAY_SIZE(picodlp_gpios));
+
+	kfree(picod);
+}
+
+static int picodlp_panel_enable(struct omap_dss_device *dssdev)
+{
+	struct picodlp_data *picod = dev_get_drvdata(&dssdev->dev);
+	int r;
+
+	dev_dbg(&dssdev->dev, "enabling picodlp panel\n");
+
+	mutex_lock(&picod->lock);
+	if (dssdev->state != OMAP_DSS_DISPLAY_DISABLED) {
+		mutex_unlock(&picod->lock);
+		return -EINVAL;
+	}
+
+	r = picodlp_panel_power_on(dssdev);
+	mutex_unlock(&picod->lock);
+
+	return r;
+}
+
+static void picodlp_panel_disable(struct omap_dss_device *dssdev)
+{
+	struct picodlp_data *picod = dev_get_drvdata(&dssdev->dev);
+
+	mutex_lock(&picod->lock);
+	/* Turn off DLP Power */
+	if (dssdev->state == OMAP_DSS_DISPLAY_ACTIVE)
+		picodlp_panel_power_off(dssdev);
+
+	dssdev->state = OMAP_DSS_DISPLAY_DISABLED;
+	mutex_unlock(&picod->lock);
+
+	dev_dbg(&dssdev->dev, "disabling picodlp panel\n");
+}
+
+static int picodlp_panel_suspend(struct omap_dss_device *dssdev)
+{
+	struct picodlp_data *picod = dev_get_drvdata(&dssdev->dev);
+
+	mutex_lock(&picod->lock);
+	/* Turn off DLP Power */
+	if (dssdev->state != OMAP_DSS_DISPLAY_ACTIVE) {
+		mutex_unlock(&picod->lock);
+		dev_err(&dssdev->dev, "unable to suspend picodlp panel,"
+					" panel is not ACTIVE\n");
+		return -EINVAL;
+	}
+
+	picodlp_panel_power_off(dssdev);
+
+	dssdev->state = OMAP_DSS_DISPLAY_SUSPENDED;
+	mutex_unlock(&picod->lock);
+
+	dev_dbg(&dssdev->dev, "suspending picodlp panel\n");
+	return 0;
+}
+
+static int picodlp_panel_resume(struct omap_dss_device *dssdev)
+{
+	struct picodlp_data *picod = dev_get_drvdata(&dssdev->dev);
+	int r;
+
+	mutex_lock(&picod->lock);
+	if (dssdev->state != OMAP_DSS_DISPLAY_SUSPENDED) {
+		mutex_unlock(&picod->lock);
+		dev_err(&dssdev->dev, "unable to resume picodlp panel,"
+			" panel is not ACTIVE\n");
+		return -EINVAL;
+	}
+
+	r = picodlp_panel_power_on(dssdev);
+	mutex_unlock(&picod->lock);
+	dev_dbg(&dssdev->dev, "resuming picodlp panel\n");
+	return r;
+}
+
+static void picodlp_get_resolution(struct omap_dss_device *dssdev,
+					u16 *xres, u16 *yres)
+{
+	*xres = dssdev->panel.timings.x_res;
+	*yres = dssdev->panel.timings.y_res;
+}
+
+static struct omap_dss_driver picodlp_driver = {
+	.probe		= picodlp_panel_probe,
+	.remove		= picodlp_panel_remove,
+
+	.enable		= picodlp_panel_enable,
+	.disable	= picodlp_panel_disable,
+
+	.get_resolution	= picodlp_get_resolution,
+
+	.suspend	= picodlp_panel_suspend,
+	.resume		= picodlp_panel_resume,
+
+	.driver		= {
+		.name	= "picodlp_panel",
+		.owner	= THIS_MODULE,
+	},
+};
+
+static int __init picodlp_init(void)
+{
+	int r = 0;
+
+	r = i2c_add_driver(&picodlp_i2c_driver);
+	if (r) {
+		printk(KERN_WARNING DRIVER_NAME
+			"driver registration failed\n");
+		return r;
+	}
+
+	r = omap_dss_register_driver(&picodlp_driver);
+	if (r)
+		i2c_del_driver(&picodlp_i2c_driver);
+
+	return r;
+}
+
+static void __exit picodlp_exit(void)
+{
+	i2c_del_driver(&picodlp_i2c_driver);
+	omap_dss_unregister_driver(&picodlp_driver);
+}
+
+module_init(picodlp_init);
+module_exit(picodlp_exit);
+
+MODULE_AUTHOR("Mythri P K <mythripk@ti.com>");
+MODULE_DESCRIPTION("picodlp driver");
+MODULE_LICENSE("GPL");
-- 
1.7.1

Re: [PATCH v3 4/4] OMAP: DSS: Add picodlp panel driver

[Tomi Valkeinen]

On Mon, 2011-05-09 at 20:48 +0530, Mayuresh Janorkar wrote:
> picodlp panel driver is required for driving DLP dpp2600.
> It consists of a dss driver and an i2c_client.
> 
> i2c_client is required for sending commands to dpp (DLP Pico Projector).
> 
> The power-up sequence consists of:
> Setting PWRGOOD to a logic low state. Once power is stable and thus the DPP2600
> is stable, then PWRGOOD should be deactivated (Set to a logic high).
> The DPP2600 will then perform a power-up initialization routine that will first
> configure and lock its PLL followed by loading self configuration data from
> external flash. DPP2600 would be activated and the EMU_DONE signal will be
> driven high. After this once internal initialization routine,
> which will take approximately 510ms, i2c commands can be sent.
> 
> To know more please visit: http://www.omappedia.org/wiki/PicoDLP_projector_guide
> 
> Signed-off-by: Mayuresh Janorkar <mayur@ti.com>
> ---
> Changes since v2:
> 	Merged panel picodlp patches into a single patch
> 	Introduced DMA_DONE polling between flash transfer
> 	Changed GPIO handling
> 	Reduced sleeps
> 
>  drivers/video/omap2/displays/Kconfig         |    7 +
>  drivers/video/omap2/displays/Makefile        |    1 +
>  drivers/video/omap2/displays/panel-picodlp.c |  627 ++++++++++++++++++++++++++
>  3 files changed, 635 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/video/omap2/displays/panel-picodlp.c
> 
> diff --git a/drivers/video/omap2/displays/Kconfig b/drivers/video/omap2/displays/Kconfig
> index 609a280..0b05593 100644
> --- a/drivers/video/omap2/displays/Kconfig
> +++ b/drivers/video/omap2/displays/Kconfig
> @@ -30,6 +30,13 @@ config PANEL_NEC_NL8048HL11_01B
>  		This NEC NL8048HL11-01B panel is TFT LCD
>  		used in the Zoom2/3/3630 sdp boards.
>  
> +config PANEL_PICODLP
> +	tristate "OMAP PICO DLP Panel"
> +	depends on OMAP2_DSS
> +	help
> +		Projector Panel used in TI's SDP4430 and EVM boards
> +		For more info please visit http://www.dlp.com/projector/
> +
>  config PANEL_TAAL
>          tristate "Taal DSI Panel"
>          depends on OMAP2_DSS_DSI
> diff --git a/drivers/video/omap2/displays/Makefile b/drivers/video/omap2/displays/Makefile
> index 0f601ab..d90f73c 100644
> --- a/drivers/video/omap2/displays/Makefile
> +++ b/drivers/video/omap2/displays/Makefile
> @@ -4,5 +4,6 @@ obj-$(CONFIG_PANEL_SHARP_LS037V7DW01) += panel-sharp-ls037v7dw01.o
>  obj-$(CONFIG_PANEL_NEC_NL8048HL11_01B) += panel-nec-nl8048hl11-01b.o
>  
>  obj-$(CONFIG_PANEL_TAAL) += panel-taal.o
> +obj-$(CONFIG_PANEL_PICODLP) +=  panel-picodlp.o
>  obj-$(CONFIG_PANEL_TPO_TD043MTEA1) += panel-tpo-td043mtea1.o
>  obj-$(CONFIG_PANEL_ACX565AKM) += panel-acx565akm.o
> diff --git a/drivers/video/omap2/displays/panel-picodlp.c b/drivers/video/omap2/displays/panel-picodlp.c
> new file mode 100644
> index 0000000..432f987
> --- /dev/null
> +++ b/drivers/video/omap2/displays/panel-picodlp.c
> @@ -0,0 +1,627 @@
> +/*
> +
> + * picodlp panel driver
> + * picodlp_i2c_driver: i2c_client driver
> + *
> + * Copyright (C) 2009-2011 Texas Instruments
> + * Author: Mythri P K <mythripk@ti.com>
> + * Mayuresh Janorkar <mayur@ti.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.
> + *
> + * This program is distributed in the hope that it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <linux/input.h>
> +#include <linux/platform_device.h>
> +#include <linux/interrupt.h>
> +#include <linux/firmware.h>
> +#include <linux/slab.h>
> +#include <linux/mutex.h>
> +#include <linux/i2c.h>
> +#include <linux/delay.h>
> +#include <linux/gpio.h>
> +
> +#include <plat/display.h>
> +#include <plat/panel-picodlp.h>
> +
> +#include "panel-picodlp.h"
> +
> +#define DRIVER_NAME	"picodlp_i2c_driver"
> +
> +/* This defines the minit of data which is allowed into single write block */
> +#define MAX_I2C_WRITE_BLOCK_SIZE	32
> +#define PICO_MAJOR			1 /* 2 bits */
> +#define PICO_MINOR			1 /* 2 bits */
> +#define MAX_TRIAL_VALUE		50

The name of this define is not descriptive and you use this define in
two places which have nothing to do with each other. I think it's better
just to use the value where it's needed.

> +struct picodlp_data {
> +	struct mutex lock;
> +	struct i2c_client *picodlp_i2c_client;
> +};
> +
> +static struct i2c_board_info picodlp_i2c_board_info = {
> +	I2C_BOARD_INFO("picodlp_i2c_driver", 0x1b),
> +};
> +
> +struct picodlp_i2c_data {
> +	struct mutex xfer_lock;
> +};
> +
> +struct i2c_device_id picodlp_i2c_id[] = {
> +	{ "picodlp_i2c_driver", 0 },
> +};
> +
> +struct picodlp_i2c_command {
> +	u8 reg;
> +	u32 value;
> +};
> +
> +static struct omap_video_timings pico_ls_timings = {
> +	.x_res		= 864,
> +	.y_res		= 480,
> +	.hsw		= 7,
> +	.hfp		= 11,
> +	.hbp		= 7,
> +
> +	.pixel_clock	= 19200,
> +
> +	.vsw		= 2,
> +	.vfp		= 3,
> +	.vbp		= 14,
> +};
> +
> +static inline struct picodlp_panel_data
> +		*get_panel_data(const struct omap_dss_device *dssdev)
> +{
> +	return (struct picodlp_panel_data *) dssdev->data;
> +}
> +
> +static u32 picodlp_i2c_read(struct i2c_client *client, u8 reg)
> +{
> +	u8 read_cmd[] = {READ_REG_SELECT, reg}, data[4];
> +	struct picodlp_i2c_data *picodlp_i2c_data = i2c_get_clientdata(client);
> +	struct i2c_msg msg[2];
> +
> +	mutex_lock(&picodlp_i2c_data->xfer_lock);
> +
> +	msg[0].addr = client->addr;
> +	msg[0].flags = 0;
> +	msg[0].len = 2;
> +	msg[0].buf = read_cmd;
> +
> +	msg[1].addr = client->addr;
> +	msg[1].flags = I2C_M_RD;
> +	msg[1].len = 4;
> +	msg[1].buf = data;
> +
> +	i2c_transfer(client->adapter, msg, 2);
> +	mutex_unlock(&picodlp_i2c_data->xfer_lock);
> +	return (data[3] | (data[2] << 8) | (data[1] << 16) | (data[0] << 24));
> +}
> +
> +static int picodlp_i2c_write_block(struct i2c_client *client,
> +					u8 *data, int len)
> +{
> +	struct i2c_msg msg;
> +	int i, r, msg_count = 1;
> +
> +	struct picodlp_i2c_data *picodlp_i2c_data = i2c_get_clientdata(client);
> +
> +	if (len < 1 || len > MAX_I2C_WRITE_BLOCK_SIZE) {
> +		dev_err(&client->dev,
> +			"too long syn_write_block len %d\n", len);
> +		return -EIO;
> +	}
> +	mutex_lock(&picodlp_i2c_data->xfer_lock);
> +
> +	msg.addr = client->addr;
> +	msg.flags = 0;
> +	msg.len = len;
> +	msg.buf = data;
> +	r = i2c_transfer(client->adapter, &msg, msg_count);
> +	mutex_unlock(&picodlp_i2c_data->xfer_lock);
> +
> +	/*
> +	 * i2c_transfer returns:
> +	 * number of messages sent in case of success
> +	 * a negative error number in case of failure
> +	 * i2c controller might timeout, in such a case sleep for sometime
> +	 * and then retry
> +	 */

The comment here doesn't look right, there's no retry.

> +	if (r != msg_count)
> +		goto err;
> +
> +	/* In case of success */
> +	for (i = 0; i < len; i++)
> +		dev_dbg(&client->dev,
> +			"addr %x bw 0x%02x[%d]: 0x%02x\n",
> +			client->addr, data[0] + i, i, data[i]);
> +
> +	return 0;
> +err:
> +	dev_err(&client->dev, "picodlp_i2c_write error\n");
> +	return r;
> +}
> +
> +static int picodlp_i2c_write(struct i2c_client *client, u8 reg, u32 value)
> +{
> +	u8 data[5];
> +	int i;
> +
> +	data[0] = reg;
> +	for (i = 1; i < 5; i++)
> +		data[i] = (value >> (32 - (i) * 8)) & 0xFF;
> +
> +	return picodlp_i2c_write_block(client, data, 5);
> +}
> +
> +static int picodlp_i2c_write_array(struct i2c_client *client,
> +			const struct picodlp_i2c_command commands[],
> +			int count)
> +{
> +	int i, r = 0;
> +	for (i = 0; i < count; i++) {
> +		r = picodlp_i2c_write(client, commands[i].reg,
> +						commands[i].value);
> +		if (r)
> +			return r;
> +	}
> +	return r;
> +}
> +
> +static bool picodlp_wait_for_dma_done(struct i2c_client *client)
> +{
> +	u8 trial = MAX_TRIAL_VALUE;
> +
> +	do {
> +		msleep(1);
> +		if (!trial--)
> +			return -ETIMEDOUT;
> +	} while (picodlp_i2c_read(client, MAIN_STATUS) & DMA_STATUS);
> +
> +	return 0;
> +}
> +
> +/**
> + * picodlp_i2c_init:	i2c_initialization routine
> + * client:	i2c_client for communication
> + *
> + * @return
> + *		0	: Success, no error
> + *	error code	: Failure
> + */
> +static int picodlp_i2c_init(struct i2c_client *client)
> +{
> +	int r;
> +	static const struct picodlp_i2c_command init_cmd_set1[] = {
> +		{SOFT_RESET, 1},
> +		{DMD_PARK_TRIGGER, 1},
> +		{MISC_REG, (PICO_MAJOR << 2 | PICO_MINOR)},
> +		{SEQ_CONTROL, 0},
> +		{SEQ_VECTOR, 0x100},
> +		{DMD_BLOCK_COUNT, 7},
> +		{DMD_VCC_CONTROL, 0x109},
> +		{DMD_PARK_PULSE_COUNT, 0xA},
> +		{DMD_PARK_PULSE_WIDTH, 0xB},
> +		{DMD_PARK_DELAY, 0x2ED},
> +		{DMD_SHADOW_ENABLE, 0},
> +		{FLASH_OPCODE, 0xB},
> +		{FLASH_DUMMY_BYTES, 1},
> +		{FLASH_ADDR_BYTES, 3},
> +		{PBC_CONTROL, 0},
> +		{FLASH_START_ADDR, CMT_LUT_0_START_ADDR},
> +		{FLASH_READ_BYTES, CMT_LUT_0_SIZE},
> +		{CMT_SPLASH_LUT_START_ADDR, 0},
> +		{CMT_SPLASH_LUT_DEST_SELECT, CMT_LUT_ALL},
> +		{PBC_CONTROL, 1},
> +	};
> +
> +	static const struct picodlp_i2c_command init_cmd_set2[] = {
> +		{PBC_CONTROL, 0},
> +		{CMT_SPLASH_LUT_DEST_SELECT, 0},
> +		{PBC_CONTROL, 0},
> +		{FLASH_START_ADDR, SEQUENCE_0_START_ADDR},
> +		{FLASH_READ_BYTES, SEQUENCE_0_SIZE},
> +		{SEQ_RESET_LUT_START_ADDR, 0},
> +		{SEQ_RESET_LUT_DEST_SELECT, SEQ_SEQ_LUT},
> +		{PBC_CONTROL, 1},
> +	};
> +
> +	static const struct picodlp_i2c_command init_cmd_set3[] = {
> +		{PBC_CONTROL, 0},
> +		{SEQ_RESET_LUT_DEST_SELECT, 0},
> +		{PBC_CONTROL, 0},
> +		{FLASH_START_ADDR, DRC_TABLE_0_START_ADDR},
> +		{FLASH_READ_BYTES, DRC_TABLE_0_SIZE},
> +		{SEQ_RESET_LUT_START_ADDR, 0},
> +		{SEQ_RESET_LUT_DEST_SELECT, SEQ_DRC_LUT_ALL},
> +		{PBC_CONTROL, 1},
> +	};
> +
> +	static const struct picodlp_i2c_command init_cmd_set4[] = {
> +		{PBC_CONTROL, 0},
> +		{SEQ_RESET_LUT_DEST_SELECT, 0},
> +		{SDC_ENABLE, 1},
> +		{AGC_CTRL, 7},
> +		{CCA_C1A, 0x100},
> +		{CCA_C1B, 0x0},
> +		{CCA_C1C, 0x0},
> +		{CCA_C2A, 0x0},
> +		{CCA_C2B, 0x100},
> +		{CCA_C2C, 0x0},
> +		{CCA_C3A, 0x0},
> +		{CCA_C3B, 0x0},
> +		{CCA_C3C, 0x100},
> +		{CCA_C7A, 0x100},
> +		{CCA_C7B, 0x100},
> +		{CCA_C7C, 0x100},
> +		{CCA_ENABLE, 1},
> +		{CPU_IF_MODE, 1},
> +		{SHORT_FLIP, 1},
> +		{CURTAIN_CONTROL, 0},
> +		{DMD_PARK_TRIGGER, 0},
> +		{R_DRIVE_CURRENT, 0x298},
> +		{G_DRIVE_CURRENT, 0x298},
> +		{B_DRIVE_CURRENT, 0x298},
> +		{RGB_DRIVER_ENABLE, 7},
> +		{SEQ_CONTROL, 0},
> +		{ACTGEN_CONTROL, 0x10},
> +		{SEQUENCE_MODE, SEQ_LOCK},
> +		{DATA_FORMAT, RGB888},
> +		{INPUT_RESOLUTION, WVGA_864_LANDSCAPE},
> +		{INPUT_SOURCE, PARALLEL_RGB},
> +		{CPU_IF_SYNC_METHOD, 1},
> +		{SEQ_CONTROL, 1}
> +	};
> +
> +	msleep(510);

You are again sleeping in a place where there are no clear actions after
and before the sleep. You should always sleep between two well defined
actions. In this case it's the emu_done going up and the first i2c
transaction, so a better place for this sleep is in the power_on
function, before calling this function. With a comment saying what we
are waiting for.

> +	r = picodlp_i2c_write_array(client, init_cmd_set1,
> +						ARRAY_SIZE(init_cmd_set1));
> +	if (r)
> +		return r;
> +
> +	r = picodlp_wait_for_dma_done(client);
> +	if (r)
> +		return r;
> +
> +	r = picodlp_i2c_write_array(client, init_cmd_set2,
> +					ARRAY_SIZE(init_cmd_set2));
> +	if (r)
> +		return r;
> +
> +	r = picodlp_wait_for_dma_done(client);
> +	if (r)
> +		return r;
> +
> +	r = picodlp_i2c_write_array(client, init_cmd_set3,
> +					ARRAY_SIZE(init_cmd_set3));
> +	if (r)
> +		return r;
> +
> +	r = picodlp_wait_for_dma_done(client);
> +	if (r)
> +		return r;
> +
> +	r = picodlp_i2c_write_array(client, init_cmd_set4,
> +					ARRAY_SIZE(init_cmd_set4));
> +	if (r)
> +		return r;
> +
> +	return 0;
> +}
> +
> +static int picodlp_i2c_probe(struct i2c_client *client,
> +		const struct i2c_device_id *id)
> +{
> +	struct picodlp_i2c_data *picodlp_i2c_data;
> +
> +	picodlp_i2c_data = kzalloc(sizeof(struct picodlp_i2c_data), GFP_KERNEL);
> +
> +	if (!picodlp_i2c_data)
> +		return -ENOMEM;
> +
> +	i2c_set_clientdata(client, picodlp_i2c_data);
> +
> +	return 0;
> +}
> +
> +static int picodlp_i2c_remove(struct i2c_client *client)
> +{
> +	struct picodlp_i2c_data *picodlp_i2c_data =
> +					i2c_get_clientdata(client);
> +
> +	kfree(picodlp_i2c_data);
> +	i2c_unregister_device(client);
> +	return 0;
> +}
> +
> +static struct i2c_driver picodlp_i2c_driver = {
> +	.driver = {
> +		.name	= "picodlp_i2c_driver",
> +	},
> +	.probe		= picodlp_i2c_probe,
> +	.remove		= picodlp_i2c_remove,
> +	.id_table	= picodlp_i2c_id,
> +};
> +
> +static int picodlp_panel_power_on(struct omap_dss_device *dssdev)
> +{
> +	int r = 0, trial = MAX_TRIAL_VALUE;
> +	struct picodlp_data *picod = dev_get_drvdata(&dssdev->dev);
> +	struct picodlp_panel_data *picodlp_pdata;
> +	struct picodlp_i2c_data *picodlp_i2c_data;
> +
> +	picodlp_pdata = get_panel_data(dssdev);
> +	if (dssdev->platform_enable) {
> +		r = dssdev->platform_enable(dssdev);
> +		if (r)
> +			return r;
> +	}
> +
> +	gpio_set_value(picodlp_pdata->pwrgood_gpio, 0);
> +	msleep(1);
> +	gpio_set_value(picodlp_pdata->pwrgood_gpio, 1);
> +
> +	while (!gpio_get_value(picodlp_pdata->emu_done_gpio)) {
> +		if (!trial--) {
> +			dev_err(&dssdev->dev, "emu_done signal not"
> +						" going down\n");
> +			return -ETIMEDOUT;
> +		}
> +		msleep(5);
> +	}

You defined the trial value to 50, which means this could timeout after
250ms. If I remember right, it could take 500ms until emu_done changes.
And your error says "going down", doesn't the emu_done gpio go up when
it's ready?

> +
> +	r = omapdss_dpi_display_enable(dssdev);
> +	if (r) {
> +		dev_err(&dssdev->dev, "failed to enable DPI\n");
> +		goto err;
> +	}
> +	dssdev->state = OMAP_DSS_DISPLAY_ACTIVE;
> +
> +	picodlp_i2c_data =
> +		i2c_get_clientdata(picod->picodlp_i2c_client);
> +
> +	r = picodlp_i2c_init(picod->picodlp_i2c_client);
> +	if (r)
> +		goto err;
> +
> +	return r;
> +err:
> +	if (dssdev->platform_disable)
> +		dssdev->platform_disable(dssdev);
> +
> +	return r;
> +}
> +
> +static void picodlp_panel_power_off(struct omap_dss_device *dssdev)
> +{
> +	struct picodlp_panel_data *picodlp_pdata = get_panel_data(dssdev);
> +
> +	omapdss_dpi_display_disable(dssdev);
> +
> +	gpio_set_value(picodlp_pdata->emu_done_gpio, 0);

emu_done is an input pin, you can't set its value.

> +	gpio_set_value(picodlp_pdata->pwrgood_gpio, 0);
> +
> +	if (dssdev->platform_disable)
> +		dssdev->platform_disable(dssdev);
> +}
> +
> +static int picodlp_panel_probe(struct omap_dss_device *dssdev)
> +{
> +	struct picodlp_data *picod;
> +	struct picodlp_panel_data *picodlp_pdata = get_panel_data(dssdev);
> +	struct i2c_adapter *adapter;
> +	struct i2c_client *picodlp_i2c_client;
> +	struct picodlp_i2c_data *picodlp_i2c_data;
> +	struct gpio picodlp_gpios[] = {
> +		{picodlp_pdata->emu_done_gpio, GPIOF_IN, "DLP EMU DONE"},
> +		{picodlp_pdata->pwrgood_gpio, GPIOF_INIT_LOW, "DLP PWRGOOD"},
> +	};

pwrgood_gpio should be GPIOF_OUT_INIT_LOW.

It is better to request and initialize the GPIOs in the board file. The
reason for this is that the PicoDLP device should be off if it's not
used, or if the driver is not even compiled in. So the board file should
make sure that the GPIOs are in such state that the device is off.

> +
> +	int r = 0, picodlp_adapter_id;
> +
> +	dssdev->panel.config = OMAP_DSS_LCD_TFT | OMAP_DSS_LCD_ONOFF |
> +				OMAP_DSS_LCD_IHS | OMAP_DSS_LCD_IVS;
> +	dssdev->panel.acb = 0x0;
> +	dssdev->panel.timings = pico_ls_timings;
> +
> +	picod =  kzalloc(sizeof(struct picodlp_data), GFP_KERNEL);
> +	if (!picod)
> +		return -ENOMEM;
> +
> +	mutex_init(&picod->lock);
> +
> +	r = gpio_request_array(picodlp_gpios, ARRAY_SIZE(picodlp_gpios));
> +	if (r) {
> +		dev_err(&dssdev->dev, "can't get dlp gpios\n");
> +		goto err;
> +	}
> +
> +	picodlp_adapter_id = picodlp_pdata->picodlp_adapter_id;
> +
> +	adapter = i2c_get_adapter(picodlp_adapter_id);
> +	if (!adapter) {
> +		dev_err(&dssdev->dev, "can't get i2c adapter\n");
> +		r = -ENODEV;
> +		goto err;
> +	}
> +
> +	picodlp_i2c_client = i2c_new_device(adapter, &picodlp_i2c_board_info);
> +	if (!picodlp_i2c_client) {
> +		dev_err(&dssdev->dev, "can't add i2c device::"
> +					 " picodlp_i2c_client is NULL\n");
> +		r = -ENODEV;
> +		goto err;
> +	}
> +
> +	picod->picodlp_i2c_client = picodlp_i2c_client;
> +
> +	picodlp_i2c_data =
> +		i2c_get_clientdata(picod->picodlp_i2c_client);
> +
> +	mutex_init(&picodlp_i2c_data->xfer_lock);

I don't think this is right. You have no guarantees that the i2c device
probe would have been called when the execution reaches here. So
picodlp_i2c_data could be NULL.

And anyway, why would you initialize the mutex here, as the mutex
belongs to the i2c side. The i2c probe would be more logical choice in
any case.

 Tomi

RE: [PATCH v3 4/4] OMAP: DSS: Add picodlp panel driver

[Janorkar, Mayuresh]

> -----Original Message-----
> From: Valkeinen, Tomi
> Sent: Tuesday, May 10, 2011 3:03 PM
> To: Janorkar, Mayuresh
> Cc: linux-omap@vger.kernel.org
> Subject: Re: [PATCH v3 4/4] OMAP: DSS: Add picodlp panel driver
> 
> On Mon, 2011-05-09 at 20:48 +0530, Mayuresh Janorkar wrote:
> >


<snip>

> > +/* This defines the minit of data which is allowed into single write
> block */
> > +#define MAX_I2C_WRITE_BLOCK_SIZE	32
> > +#define PICO_MAJOR			1 /* 2 bits */
> > +#define PICO_MINOR			1 /* 2 bits */
> > +#define MAX_TRIAL_VALUE		50
> 
> The name of this define is not descriptive and you use this define in
> two places which have nothing to do with each other. I think it's better
> just to use the value where it's needed.

Looks fine.

> 
> > +struct picodlp_data {
> > +	struct mutex lock;
> > +	struct i2c_client *picodlp_i2c_client;
> > +};
> > +
> > +static struct i2c_board_info picodlp_i2c_board_info = {
> > +	I2C_BOARD_INFO("picodlp_i2c_driver", 0x1b),
> > +};
> > +	r = i2c_transfer(client->adapter, &msg, msg_count);
> > +	mutex_unlock(&picodlp_i2c_data->xfer_lock);
> > +
> > +	/*
> > +	 * i2c_transfer returns:
> > +	 * number of messages sent in case of success
> > +	 * a negative error number in case of failure
> > +	 * i2c controller might timeout, in such a case sleep for sometime
> > +	 * and then retry
> > +	 */
> 
> The comment here doesn't look right, there's no retry.

Fine.

> 
> > +	if (r != msg_count)
> > +		goto err;
> > +
> > +	/* In case of success */
> > +	for (i = 0; i < len; i++)
> > +		dev_dbg(&client->dev,


> > +		{SEQ_CONTROL, 1}
> > +	};
> > +
> > +	msleep(510);
> 
> You are again sleeping in a place where there are no clear actions after
> and before the sleep. You should always sleep between two well defined
> actions. In this case it's the emu_done going up and the first i2c
> transaction, so a better place for this sleep is in the power_on
> function, before calling this function. With a comment saying what we
> are waiting for.

I would move this to power_on function with an appropriate comment.

> 
> > +	r = picodlp_i2c_write_array(client, init_cmd_set1,
> > +						ARRAY_SIZE(init_cmd_set1));
> > +	if (r)
> > +		return r;
> > +		msleep(5);
> > +	}
> 
> You defined the trial value to 50, which means this could timeout after
> 250ms. If I remember right, it could take 500ms until emu_done changes.
> And your error says "going down", doesn't the emu_done gpio go up when
> it's ready?

Looks fine. I would change the counter.
My bad. I meant to say emu_done is still down.

> 
> > +
> > +	r = omapdss_dpi_display_enable(dssdev);
> > +	if (r) {
> > +		dev_err(&dssdev->dev, "failed to enable DPI\n");
> > +		goto err;
> > +static void picodlp_panel_power_off(struct omap_dss_device *dssdev)
> > +{
> > +	struct picodlp_panel_data *picodlp_pdata = get_panel_data(dssdev);
> > +
> > +	omapdss_dpi_display_disable(dssdev);
> > +
> > +	gpio_set_value(picodlp_pdata->emu_done_gpio, 0);
> 
> emu_done is an input pin, you can't set its value.
Yes.

> 
> > +	gpio_set_value(picodlp_pdata->pwrgood_gpio, 0);
> > +
> > +	if (dssdev->platform_disable)
> > +		dssdev->platform_disable(dssdev);
> > +}
> > +
> > +static int picodlp_panel_probe(struct omap_dss_device *dssdev)
> > +{
> > +	struct picodlp_data *picod;
> > +	struct picodlp_panel_data *picodlp_pdata = get_panel_data(dssdev);
> > +	struct i2c_adapter *adapter;
> > +	struct i2c_client *picodlp_i2c_client;
> > +	struct picodlp_i2c_data *picodlp_i2c_data;
> > +	struct gpio picodlp_gpios[] = {
> > +		{picodlp_pdata->emu_done_gpio, GPIOF_IN, "DLP EMU DONE"},
> > +		{picodlp_pdata->pwrgood_gpio, GPIOF_INIT_LOW, "DLP PWRGOOD"},
> > +	};
> 
> pwrgood_gpio should be GPIOF_OUT_INIT_LOW.

Looks fine

> 
> It is better to request and initialize the GPIOs in the board file. The
> reason for this is that the PicoDLP device should be off if it's not
> used, or if the driver is not even compiled in. So the board file should
> make sure that the GPIOs are in such state that the device is off.

The board file has only init. There is no exit.
So if I request gpios in init once, I could not find a place to free them.

Is it a good idea to request gpios in platform_enable and free them in platform_disable?

> 
> > +
> > +	int r = 0, picodlp_adapter_id;
> > +
> > +	dssdev->panel.config = OMAP_DSS_LCD_TFT | OMAP_DSS_LCD_ONOFF |
> > +				OMAP_DSS_LCD_IHS | OMAP_DSS_LCD_IVS;
> > +		r = -ENODEV;
> > +		goto err;
> > +	}
> > +
> > +	picod->picodlp_i2c_client = picodlp_i2c_client;
> > +
> > +	picodlp_i2c_data =
> > +		i2c_get_clientdata(picod->picodlp_i2c_client);
> > +
> > +	mutex_init(&picodlp_i2c_data->xfer_lock);
> 
> I don't think this is right. You have no guarantees that the i2c device
> probe would have been called when the execution reaches here. So
> picodlp_i2c_data could be NULL.

I would add a check for this.

> 
> And anyway, why would you initialize the mutex here, as the mutex
> belongs to the i2c side. The i2c probe would be more logical choice in
> any case.

I will move this to i2c_probe.

> 
>  Tomi
> 

RE: [PATCH v3 4/4] OMAP: DSS: Add picodlp panel driver

[Tomi Valkeinen]

On Tue, 2011-05-10 at 15:57 +0530, Janorkar, Mayuresh wrote:

> > It is better to request and initialize the GPIOs in the board file. The
> > reason for this is that the PicoDLP device should be off if it's not
> > used, or if the driver is not even compiled in. So the board file should
> > make sure that the GPIOs are in such state that the device is off.
> 
> The board file has only init. There is no exit.
> So if I request gpios in init once, I could not find a place to free them.

If the GPIOs are not shared, and they go only to picodlp, there's not
really any need to free them. They are not a shared resource, and they
can be kept reserved for picodlp all the time.

> Is it a good idea to request gpios in platform_enable and free them in platform_disable?

No, that wouldn't fix the problem.

For example, consider a case where the kernel is compiled without
picodlp driver. So platform_enable/disable is never called, and the
GPIOs are never requested or initialized. This means that depending on
the values of the GPIOs, the picodlp could be always on, consuming
power.

So my suggestion is to request all the GPIOs in the board file, in
display init, and initialize the GPIOs to some sane value so that the
picodlp is off. Then pass the two GPIOs to the picodlp driver, which can
then use the GPIOs, and handle the two other GPIOs in the board file in
platform_enable/disable. 

 Tomi

RE: [PATCH v3 4/4] OMAP: DSS: Add picodlp panel driver

[Janorkar, Mayuresh]

> -----Original Message-----
> From: Valkeinen, Tomi
> Sent: Tuesday, May 10, 2011 4:44 PM
> To: Janorkar, Mayuresh
> Cc: linux-omap@vger.kernel.org
> Subject: RE: [PATCH v3 4/4] OMAP: DSS: Add picodlp panel driver
> 
> On Tue, 2011-05-10 at 15:57 +0530, Janorkar, Mayuresh wrote:
> 
> > > It is better to request and initialize the GPIOs in the board file.
> The
> > > reason for this is that the PicoDLP device should be off if it's not
> > > used, or if the driver is not even compiled in. So the board file
> should
> > > make sure that the GPIOs are in such state that the device is off.
> >
> > The board file has only init. There is no exit.
> > So if I request gpios in init once, I could not find a place to free
> them.
> 
> If the GPIOs are not shared, and they go only to picodlp, there's not
> really any need to free them. They are not a shared resource, and they
> can be kept reserved for picodlp all the time.
> 
> > Is it a good idea to request gpios in platform_enable and free them in
> platform_disable?
> 
> No, that wouldn't fix the problem.
> 
> For example, consider a case where the kernel is compiled without
> picodlp driver. So platform_enable/disable is never called, and the
> GPIOs are never requested or initialized. This means that depending on
> the values of the GPIOs, the picodlp could be always on, consuming
> power.
> 
> So my suggestion is to request all the GPIOs in the board file, in
> display init, and initialize the GPIOs to some sane value so that the
> picodlp is off. Then pass the two GPIOs to the picodlp driver, which can
> then use the GPIOs, and handle the two other GPIOs in the board file in
> platform_enable/disable.

Looks fine.
I would set:
OMAP4_DLP_DISPLAY_SELECT_GPIO to 0,
OMAP4_DLP_POWER_ON_GPIO to 0,
And pwrgood_gpio to 1,
as init values of these gpios.

> 
>  Tomi
>