diff mbox

[5/7] OMAP: DSS: Adding initialization routine to picodlp panel

Message ID 1303107350-22747-6-git-send-email-mayur@ti.com (mailing list archive)
State Superseded
Delegated to: Tomi Valkeinen
Headers show

Commit Message

MAYURESH JANORKAR April 18, 2011, 6:15 a.m. UTC
From: Mythri P K <mythripk@ti.com>

picodlp_i2c_client needs to send commands over i2c as a part of initialiazation.
system controller dlp pico processor dpp2600 is used.
It configures the splash screen of picodlp using a sequence of commands.
A programmer's guide is available at:
http://focus.ti.com/lit/ug/dlpu002a/dlpu002a.pdf

API is defined for sending command over i2c as an i2c_write operation.

Signed-off-by: Mythri P K <mythripk@ti.com>
Signed-off-by: Mayuresh Janorkar <mayur@ti.com>
---
 drivers/video/omap2/displays/panel-picodlp.c |  317 ++++++++++++++++++++++++++
 1 files changed, 317 insertions(+), 0 deletions(-)

Comments

Tomi Valkeinen April 19, 2011, 11:52 a.m. UTC | #1
On Mon, 2011-04-18 at 11:45 +0530, Mayuresh Janorkar wrote:
> From: Mythri P K <mythripk@ti.com>
> 
> picodlp_i2c_client needs to send commands over i2c as a part of initialiazation.
> system controller dlp pico processor dpp2600 is used.
> It configures the splash screen of picodlp using a sequence of commands.
> A programmer's guide is available at:
> http://focus.ti.com/lit/ug/dlpu002a/dlpu002a.pdf
> 
> API is defined for sending command over i2c as an i2c_write operation.
> 
> Signed-off-by: Mythri P K <mythripk@ti.com>
> Signed-off-by: Mayuresh Janorkar <mayur@ti.com>
> ---
>  drivers/video/omap2/displays/panel-picodlp.c |  317 ++++++++++++++++++++++++++
>  1 files changed, 317 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/video/omap2/displays/panel-picodlp.c b/drivers/video/omap2/displays/panel-picodlp.c
> index e361674..785e406 100644
> --- a/drivers/video/omap2/displays/panel-picodlp.c
> +++ b/drivers/video/omap2/displays/panel-picodlp.c
> @@ -32,7 +32,15 @@
>  #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 */
> +
>  struct picodlp_data {
>  	struct mutex lock;
>  	struct i2c_client *picodlp_i2c_client;
> @@ -50,6 +58,11 @@ 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,
> @@ -70,6 +83,305 @@ static inline struct picodlp_panel_data
>  	return (struct picodlp_panel_data *) dssdev->data;
>  }
>  
> +static int picodlp_i2c_write_block(struct i2c_client *client,
> +					u8 *data, int len)
> +{
> +	struct i2c_msg msg;
> +	int i, r;
> +
> +	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, 1);
> +	mutex_unlock(&picodlp_i2c_data->xfer_lock);
> +
> +	if (r == 1) {
> +		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;
> +	}
> +
> +	dev_err(&client->dev, " picodlp_i2c_write error\n");
> +	return r;

This is, at least to my eyes, a bit uncommon way to do this. Usually the
error path is handled inside an if(), and the ok path is at the end of
the function.

> +}
> +
> +static int picodlp_i2c_write(struct i2c_client *client, u8 reg, u32 value)
> +{
> +	u8 data[5];
> +
> +	data[0] = reg;
> +	data[1] = (value & 0xFF000000) >> 24;
> +	data[2] = (value & 0x00FF0000) >> 16;
> +	data[3] = (value & 0x0000FF00) >> 8;
> +	data[4] = (value & 0x000000FF);
> +
> +	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;
> +}
> +
> +/**
> + * picodlp_dpp2600_flash_dma - return parameter as 0 on success or error code
> + * Configure datapath for splash image operation
> + * dpp2600 : digitally programmable potentiometer
> + * It is a system controller used for picodlp
> + *
> + * @client:		i2c_client required for operations
> + * @flash_address:	splash image to be loaded from flash
> + * @flash_num_bytes:	size in bytes for flash
> + * @cmt_seqz:		select mailbox to load data to
> + *			0 = sequence/DRC,
> + *			1 = CMT/splash
> + * @table_number:	table_number to load flash
> + *
> + * @return
> + *		0	-	no errors
> + *		-ENXIO	- invalid flash address specified
> + *		-EINVAL - invalid mailbox specified OR invalid table_number
> + *				OR mailbox combination
> + */
> +static int picodlp_dpp2600_flash_dma(struct i2c_client *client,
> +		int flash_address, int flash_num_bytes, int cmt_seqz,
> +		int table_number)
> +{
> +	int mailbox_address, mailbox_select;
> +	int r = 0;
> +
> +	/* check argument validity */
> +	if (flash_address > 0x1fffff)
> +		return -ENXIO;
> +
> +	if ((cmt_seqz > 1) || (cmt_seqz == 0 && table_number > 6) ||
> +				(cmt_seqz == 1 && table_number > 5))
> +		return -EINVAL;
> +
> +	/* set mailbox parameters */
> +	if (cmt_seqz) {
> +		mailbox_address = CMT_SPLASH_LUT_START_ADDR;
> +		mailbox_select = CMT_SPLASH_LUT_DEST_SELECT;
> +	} else {
> +		mailbox_address = SEQ_RESET_LUT_START_ADDR;
> +		mailbox_select = SEQ_RESET_LUT_DEST_SELECT;
> +	}
> +
> +	/* configure DMA from flash to LUT */
> +	r = picodlp_i2c_write(client, PBC_CONTROL, 0);
> +	if (r)
> +		return r;
> +
> +	 r = picodlp_i2c_write(client, FLASH_START_ADDR, flash_address);
> +	 if (r)
> +		return r;
> +
> +	 r = picodlp_i2c_write(client, FLASH_READ_BYTES, flash_num_bytes);
> +	if (r)
> +		return r;
> +
> +	r = picodlp_i2c_write(client, mailbox_address, 0);
> +	if (r)
> +		return r;
> +
> +	r = picodlp_i2c_write(client, mailbox_select, table_number);
> +	if (r)
> +		return r;
> +
> +	/* transfer control to flash controller */
> +	r = picodlp_i2c_write(client, PBC_CONTROL, 1);
> +	if (r)
> +		return r;
> +
> +	/**
> +	 * wait for control transfer
> +	 * this usually takes close to 5ms
> +	 */
> +	msleep(5);
> +
> +	/* return register access to i2c client */
> +	 r = picodlp_i2c_write(client, PBC_CONTROL, 0);
> +	 if (r)
> +		return r;
> +
> +	/* close LUT access */
> +	 r = picodlp_i2c_write(client, mailbox_select, 0);
> +	 if (r)
> +		return r;
> +
> +	return 0;
> +}
> +
> +/**
> + * picodlp_dpp2600_config_rgb: returns 0 on success and error code on failure
> + * Configure datapath for parallel RGB operation
> + * dpp2600 : digitally programmable potentiometer
> + * It is a system controller used for picodlp
> + *
> + * @client: i2c_client:		i2c_client required for communication
> + * @return:
> + *			0	: Success, no error
> + *		 error code	: Failure
> + */
> +static int picodlp_dpp2600_config_rgb(struct i2c_client *client)
> +{
> +	int r = 0;
> +
> +	static const struct picodlp_i2c_command config_commands[] = {
> +		{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}
> +	};
> +
> +	r = picodlp_i2c_write_array(client, config_commands,
> +					ARRAY_SIZE(config_commands));
> +	return r;
> +}
> +
> +/**
> + * picodlp_dpp2600_config_splash returns 0 on success and
> + *					error code in case of failure
> + * Configure datapath for splash image operation
> + * dpp2600 : digitally programmable potentiometer
> + * It is a system controller used for picodlp
> + *
> + * @return
> + *		0	- no errors
> + *		-EINVAL - invalid image_number specified
> + */
> +static int picodlp_dpp2600_config_splash(struct i2c_client *client)
> +{
> +	int r;
> +
> +	static const struct picodlp_i2c_command splash_cmd[] = {
> +		{SEQ_CONTROL, 0}, {SEQUENCE_MODE, SEQ_FREE_RUN},
> +		{DATA_FORMAT, RGB565}, {INPUT_RESOLUTION, QWVGA_LANDSCAPE},
> +		{INPUT_SOURCE, SPLASH_SCREEN}
> +	};
> +
> +	r = picodlp_i2c_write_array(client, splash_cmd,
> +					ARRAY_SIZE(splash_cmd));
> +	if (r)
> +		return r;
> +
> +	r = picodlp_dpp2600_flash_dma(client, SPLASH_1_START_ADDR,
> +					SPLASH_1_SIZE, 1, SPLASH_LUT);
> +	if (r)
> +		return r;
> +
> +	/* turn image back on */
> +	r = picodlp_i2c_write(client, SEQ_CONTROL, 1);
> +	if (r)
> +		return r;
> +
> +	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},
> +	};

It would make these command lists clearer if there was only one command
per line.

> +
> +	static const struct picodlp_i2c_command init_cmd_set2[] = {
> +		{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},
> +	};
> +
> +	static const struct picodlp_i2c_command init_cmd_set3[] = {
> +		{DMD_PARK_TRIGGER, 0}, {R_DRIVE_CURRENT, 0x298},
> +		{G_DRIVE_CURRENT, 0x298}, {B_DRIVE_CURRENT, 0x298},
> +		{RGB_DRIVER_ENABLE, 7},
> +	};
> +
> +	r = picodlp_i2c_write_array(client, init_cmd_set1,
> +						ARRAY_SIZE(init_cmd_set1));
> +	if (r)
> +		return r;
> +
> +	/* configure DMA from flash to LUT */
> +	r = picodlp_dpp2600_flash_dma(client, CMT_LUT_0_START_ADDR,
> +					CMT_LUT_0_SIZE, 1, CMT_LUT_ALL);
> +	if (r)
> +		return r;
> +
> +	/* SEQ and DRC look-up tables */
> +	r = picodlp_dpp2600_flash_dma(client, SEQUENCE_0_START_ADDR,
> +					SEQUENCE_0_SIZE, 0, SEQ_SEQ_LUT);
> +	if (r)
> +		return r;
> +
> +	r = picodlp_dpp2600_flash_dma(client, DRC_TABLE_0_START_ADDR,
> +					DRC_TABLE_0_SIZE, 0, SEQ_DRC_LUT_ALL);
> +	if (r)
> +		return r;
> +
> +	r = picodlp_i2c_write_array(client, init_cmd_set2,
> +					ARRAY_SIZE(init_cmd_set2));
> +	if (r)
> +		return r;
> +
> +	/* display logo splash image */
> +	r = picodlp_dpp2600_config_splash(client);
> +	if (r)
> +		return r;

So this will show some hardcoded splash screen? How much of the code
could be removed if the splash screen feature is removed? I don't see
much point in such a feature, and it looks to me that at least two of
the functions above are just for the splash screen. It's just extra code
for a feature nobody wants.

> +	r = picodlp_i2c_write_array(client, init_cmd_set3,
> +					ARRAY_SIZE(init_cmd_set3));
> +	if (r)
> +		return r;
> +
> +	r = picodlp_dpp2600_config_rgb(client);
> +	if (r)
> +		return r;
> +
> +	return 0;
> +}
> +
>  static int picodlp_i2c_probe(struct i2c_client *client,
>  		const struct i2c_device_id *id)
>  {
> @@ -134,6 +446,11 @@ static int picodlp_panel_power_on(struct omap_dss_device *dssdev)
>  	picodlp_i2c_data =
>  		i2c_get_clientdata(picod->picodlp_i2c_client);
>  
> +	msleep(700); /* sleep till panel is settled */

And another huge sleep. This, unlike the other sleeps, make some sense,
as there's an i2c transaction done below.

Somehow I get the feeling that you've just put big sleeps here and there
until the driver started working... Can you point me to the
documentation that describes the delays required?

> +	r = picodlp_i2c_init(picod->picodlp_i2c_client);
> +	if (r)
> +		goto err;
> +
>  	return r;
>  err:
>  	if (dssdev->platform_disable)


--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
MAYURESH JANORKAR April 21, 2011, 11:17 a.m. UTC | #2
> -----Original Message-----
> From: Valkeinen, Tomi
> Sent: Tuesday, April 19, 2011 5:23 PM
> To: Janorkar, Mayuresh
> Cc: linux-omap@vger.kernel.org; K, Mythri P
> Subject: Re: [PATCH 5/7] OMAP: DSS: Adding initialization routine to
> picodlp panel
> 
> On Mon, 2011-04-18 at 11:45 +0530, Mayuresh Janorkar wrote:
> > From: Mythri P K <mythripk@ti.com>
> >
> > picodlp_i2c_client needs to send commands over i2c as a part of
> initialiazation.
> > system controller dlp pico processor dpp2600 is used.
> > It configures the splash screen of picodlp using a sequence of commands.
> > A programmer's guide is available at:
> > http://focus.ti.com/lit/ug/dlpu002a/dlpu002a.pdf
> >
> > API is defined for sending command over i2c as an i2c_write operation.
> >
> > Signed-off-by: Mythri P K <mythripk@ti.com>
> > Signed-off-by: Mayuresh Janorkar <mayur@ti.com>
> > ---
> >  drivers/video/omap2/displays/panel-picodlp.c |  317
> ++++++++++++++++++++++++++
> >  1 files changed, 317 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/video/omap2/displays/panel-picodlp.c
> b/drivers/video/omap2/displays/panel-picodlp.c
> > index e361674..785e406 100644
> > --- a/drivers/video/omap2/displays/panel-picodlp.c
> > +++ b/drivers/video/omap2/displays/panel-picodlp.c
> > @@ -32,7 +32,15 @@
> >  #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 */
> > +
> >  struct picodlp_data {
> >  	struct mutex lock;
> >  	struct i2c_client *picodlp_i2c_client;
> > @@ -50,6 +58,11 @@ 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,
> > @@ -70,6 +83,305 @@ static inline struct picodlp_panel_data
> >  	return (struct picodlp_panel_data *) dssdev->data;
> >  }
> >
> > +static int picodlp_i2c_write_block(struct i2c_client *client,
> > +					u8 *data, int len)
> > +{
> > +	struct i2c_msg msg;
> > +	int i, r;
> > +
> > +	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, 1);
> > +	mutex_unlock(&picodlp_i2c_data->xfer_lock);
> > +
> > +	if (r == 1) {
> > +		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;
> > +	}
> > +
> > +	dev_err(&client->dev, " picodlp_i2c_write error\n");
> > +	return r;
> 
> This is, at least to my eyes, a bit uncommon way to do this. Usually the
> error path is handled inside an if(), and the ok path is at the end of
> the function.

Would change this as per the most used formats.

> 
> > +}
> > +	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},
> > +	};
> 
> It would make these command lists clearer if there was only one command
> per line.


Fine

> 
> > +
> > +	if (r)
> > +		return r;
> > +
> > +	/* display logo splash image */
> > +	r = picodlp_dpp2600_config_splash(client);
> > +	if (r)
> > +		return r;
> 
> So this will show some hardcoded splash screen? How much of the code
> could be removed if the splash screen feature is removed? I don't see
> much point in such a feature, and it looks to me that at least two of
> the functions above are just for the splash screen. It's just extra code
> for a feature nobody wants.

Yes, this would show a hardcoded splash screen.

> 
> > +	r = picodlp_i2c_write_array(client, init_cmd_set3,
> > +					ARRAY_SIZE(init_cmd_set3));
> > +	if (r)
> > +		return r;
> > +
> > +	r = picodlp_dpp2600_config_rgb(client);
> > +	if (r)
> > +		return r;
> > +
> > +	return 0;
> > +}
> > +
> >  static int picodlp_i2c_probe(struct i2c_client *client,
> >  		const struct i2c_device_id *id)
> >  {
> > @@ -134,6 +446,11 @@ static int picodlp_panel_power_on(struct
> omap_dss_device *dssdev)
> >  	picodlp_i2c_data =
> >  		i2c_get_clientdata(picod->picodlp_i2c_client);
> >
> > +	msleep(700); /* sleep till panel is settled */
> 
> And another huge sleep. This, unlike the other sleeps, make some sense,
> as there's an i2c transaction done below.
> 
> Somehow I get the feeling that you've just put big sleeps here and there
> until the driver started working... Can you point me to the
> documentation that describes the delays required?

Except msleep(5) in
/* transfer control to flash controller */
r = picodlp_i2c_write(client, PBC_CONTROL, 1);
        msleep(5);
r = picodlp_i2c_write(client, PBC_CONTROL, 0); 

Other delays are not documented. But it is practically observed that we need to wait otherwise the i2c_packet does not succeed.

As I mentioned earlier, I can follow one approach, in i2c_write see if packet transfer happens. If not, sleep for some time and retry.

This approach would remove these delays from this place.

> > +	r = picodlp_i2c_init(picod->picodlp_i2c_client);
> > +	if (r)
> > +		goto err;
> > +
> >  	return r;
> >  err:
> >  	if (dssdev->platform_disable)
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tomi Valkeinen April 26, 2011, 10:47 a.m. UTC | #3
On Thu, 2011-04-21 at 16:47 +0530, Janorkar, Mayuresh wrote:
> 
> > -----Original Message-----
> > From: Valkeinen, Tomi
> > Sent: Tuesday, April 19, 2011 5:23 PM
> > To: Janorkar, Mayuresh
> > Cc: linux-omap@vger.kernel.org; K, Mythri P
> > Subject: Re: [PATCH 5/7] OMAP: DSS: Adding initialization routine to
> > picodlp panel
> > 
> > On Mon, 2011-04-18 at 11:45 +0530, Mayuresh Janorkar wrote:

<snip>

> > >  static int picodlp_i2c_probe(struct i2c_client *client,
> > >  		const struct i2c_device_id *id)
> > >  {
> > > @@ -134,6 +446,11 @@ static int picodlp_panel_power_on(struct
> > omap_dss_device *dssdev)
> > >  	picodlp_i2c_data =
> > >  		i2c_get_clientdata(picod->picodlp_i2c_client);
> > >
> > > +	msleep(700); /* sleep till panel is settled */
> > 
> > And another huge sleep. This, unlike the other sleeps, make some sense,
> > as there's an i2c transaction done below.
> > 
> > Somehow I get the feeling that you've just put big sleeps here and there
> > until the driver started working... Can you point me to the
> > documentation that describes the delays required?
> 
> Except msleep(5) in
> /* transfer control to flash controller */
> r = picodlp_i2c_write(client, PBC_CONTROL, 1);
>         msleep(5);
> r = picodlp_i2c_write(client, PBC_CONTROL, 0); 
> 
> Other delays are not documented. But it is practically observed that we need to wait otherwise the i2c_packet does not succeed.

Then we need to contact the HW guys developing picodlp. They are TIers,
aren't they? We can't just put sleeps here and there and be sure it'll
work properly.

What if your sleep of 700ms has, by luck, caused a 900ms sleep in your
tests (sleeps can take considerably longer than the given value), and
the hardware requires 800ms sleep? Your tests show it works, but for
some other user the sleep may cause a 750ms sleep, causing picodlp to
fail to start.

 Tomi


--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/video/omap2/displays/panel-picodlp.c b/drivers/video/omap2/displays/panel-picodlp.c
index e361674..785e406 100644
--- a/drivers/video/omap2/displays/panel-picodlp.c
+++ b/drivers/video/omap2/displays/panel-picodlp.c
@@ -32,7 +32,15 @@ 
 #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 */
+
 struct picodlp_data {
 	struct mutex lock;
 	struct i2c_client *picodlp_i2c_client;
@@ -50,6 +58,11 @@  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,
@@ -70,6 +83,305 @@  static inline struct picodlp_panel_data
 	return (struct picodlp_panel_data *) dssdev->data;
 }
 
+static int picodlp_i2c_write_block(struct i2c_client *client,
+					u8 *data, int len)
+{
+	struct i2c_msg msg;
+	int i, r;
+
+	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, 1);
+	mutex_unlock(&picodlp_i2c_data->xfer_lock);
+
+	if (r == 1) {
+		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;
+	}
+
+	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];
+
+	data[0] = reg;
+	data[1] = (value & 0xFF000000) >> 24;
+	data[2] = (value & 0x00FF0000) >> 16;
+	data[3] = (value & 0x0000FF00) >> 8;
+	data[4] = (value & 0x000000FF);
+
+	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;
+}
+
+/**
+ * picodlp_dpp2600_flash_dma - return parameter as 0 on success or error code
+ * Configure datapath for splash image operation
+ * dpp2600 : digitally programmable potentiometer
+ * It is a system controller used for picodlp
+ *
+ * @client:		i2c_client required for operations
+ * @flash_address:	splash image to be loaded from flash
+ * @flash_num_bytes:	size in bytes for flash
+ * @cmt_seqz:		select mailbox to load data to
+ *			0 = sequence/DRC,
+ *			1 = CMT/splash
+ * @table_number:	table_number to load flash
+ *
+ * @return
+ *		0	-	no errors
+ *		-ENXIO	- invalid flash address specified
+ *		-EINVAL - invalid mailbox specified OR invalid table_number
+ *				OR mailbox combination
+ */
+static int picodlp_dpp2600_flash_dma(struct i2c_client *client,
+		int flash_address, int flash_num_bytes, int cmt_seqz,
+		int table_number)
+{
+	int mailbox_address, mailbox_select;
+	int r = 0;
+
+	/* check argument validity */
+	if (flash_address > 0x1fffff)
+		return -ENXIO;
+
+	if ((cmt_seqz > 1) || (cmt_seqz == 0 && table_number > 6) ||
+				(cmt_seqz == 1 && table_number > 5))
+		return -EINVAL;
+
+	/* set mailbox parameters */
+	if (cmt_seqz) {
+		mailbox_address = CMT_SPLASH_LUT_START_ADDR;
+		mailbox_select = CMT_SPLASH_LUT_DEST_SELECT;
+	} else {
+		mailbox_address = SEQ_RESET_LUT_START_ADDR;
+		mailbox_select = SEQ_RESET_LUT_DEST_SELECT;
+	}
+
+	/* configure DMA from flash to LUT */
+	r = picodlp_i2c_write(client, PBC_CONTROL, 0);
+	if (r)
+		return r;
+
+	 r = picodlp_i2c_write(client, FLASH_START_ADDR, flash_address);
+	 if (r)
+		return r;
+
+	 r = picodlp_i2c_write(client, FLASH_READ_BYTES, flash_num_bytes);
+	if (r)
+		return r;
+
+	r = picodlp_i2c_write(client, mailbox_address, 0);
+	if (r)
+		return r;
+
+	r = picodlp_i2c_write(client, mailbox_select, table_number);
+	if (r)
+		return r;
+
+	/* transfer control to flash controller */
+	r = picodlp_i2c_write(client, PBC_CONTROL, 1);
+	if (r)
+		return r;
+
+	/**
+	 * wait for control transfer
+	 * this usually takes close to 5ms
+	 */
+	msleep(5);
+
+	/* return register access to i2c client */
+	 r = picodlp_i2c_write(client, PBC_CONTROL, 0);
+	 if (r)
+		return r;
+
+	/* close LUT access */
+	 r = picodlp_i2c_write(client, mailbox_select, 0);
+	 if (r)
+		return r;
+
+	return 0;
+}
+
+/**
+ * picodlp_dpp2600_config_rgb: returns 0 on success and error code on failure
+ * Configure datapath for parallel RGB operation
+ * dpp2600 : digitally programmable potentiometer
+ * It is a system controller used for picodlp
+ *
+ * @client: i2c_client:		i2c_client required for communication
+ * @return:
+ *			0	: Success, no error
+ *		 error code	: Failure
+ */
+static int picodlp_dpp2600_config_rgb(struct i2c_client *client)
+{
+	int r = 0;
+
+	static const struct picodlp_i2c_command config_commands[] = {
+		{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}
+	};
+
+	r = picodlp_i2c_write_array(client, config_commands,
+					ARRAY_SIZE(config_commands));
+	return r;
+}
+
+/**
+ * picodlp_dpp2600_config_splash returns 0 on success and
+ *					error code in case of failure
+ * Configure datapath for splash image operation
+ * dpp2600 : digitally programmable potentiometer
+ * It is a system controller used for picodlp
+ *
+ * @return
+ *		0	- no errors
+ *		-EINVAL - invalid image_number specified
+ */
+static int picodlp_dpp2600_config_splash(struct i2c_client *client)
+{
+	int r;
+
+	static const struct picodlp_i2c_command splash_cmd[] = {
+		{SEQ_CONTROL, 0}, {SEQUENCE_MODE, SEQ_FREE_RUN},
+		{DATA_FORMAT, RGB565}, {INPUT_RESOLUTION, QWVGA_LANDSCAPE},
+		{INPUT_SOURCE, SPLASH_SCREEN}
+	};
+
+	r = picodlp_i2c_write_array(client, splash_cmd,
+					ARRAY_SIZE(splash_cmd));
+	if (r)
+		return r;
+
+	r = picodlp_dpp2600_flash_dma(client, SPLASH_1_START_ADDR,
+					SPLASH_1_SIZE, 1, SPLASH_LUT);
+	if (r)
+		return r;
+
+	/* turn image back on */
+	r = picodlp_i2c_write(client, SEQ_CONTROL, 1);
+	if (r)
+		return r;
+
+	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},
+	};
+
+	static const struct picodlp_i2c_command init_cmd_set2[] = {
+		{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},
+	};
+
+	static const struct picodlp_i2c_command init_cmd_set3[] = {
+		{DMD_PARK_TRIGGER, 0}, {R_DRIVE_CURRENT, 0x298},
+		{G_DRIVE_CURRENT, 0x298}, {B_DRIVE_CURRENT, 0x298},
+		{RGB_DRIVER_ENABLE, 7},
+	};
+
+	r = picodlp_i2c_write_array(client, init_cmd_set1,
+						ARRAY_SIZE(init_cmd_set1));
+	if (r)
+		return r;
+
+	/* configure DMA from flash to LUT */
+	r = picodlp_dpp2600_flash_dma(client, CMT_LUT_0_START_ADDR,
+					CMT_LUT_0_SIZE, 1, CMT_LUT_ALL);
+	if (r)
+		return r;
+
+	/* SEQ and DRC look-up tables */
+	r = picodlp_dpp2600_flash_dma(client, SEQUENCE_0_START_ADDR,
+					SEQUENCE_0_SIZE, 0, SEQ_SEQ_LUT);
+	if (r)
+		return r;
+
+	r = picodlp_dpp2600_flash_dma(client, DRC_TABLE_0_START_ADDR,
+					DRC_TABLE_0_SIZE, 0, SEQ_DRC_LUT_ALL);
+	if (r)
+		return r;
+
+	r = picodlp_i2c_write_array(client, init_cmd_set2,
+					ARRAY_SIZE(init_cmd_set2));
+	if (r)
+		return r;
+
+	/* display logo splash image */
+	r = picodlp_dpp2600_config_splash(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_dpp2600_config_rgb(client);
+	if (r)
+		return r;
+
+	return 0;
+}
+
 static int picodlp_i2c_probe(struct i2c_client *client,
 		const struct i2c_device_id *id)
 {
@@ -134,6 +446,11 @@  static int picodlp_panel_power_on(struct omap_dss_device *dssdev)
 	picodlp_i2c_data =
 		i2c_get_clientdata(picod->picodlp_i2c_client);
 
+	msleep(700); /* sleep till panel is settled */
+	r = picodlp_i2c_init(picod->picodlp_i2c_client);
+	if (r)
+		goto err;
+
 	return r;
 err:
 	if (dssdev->platform_disable)