diff mbox

[3/7] OMAP: DSS: Adding a picodlp panel driver

Message ID 1303107350-22747-4-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>

A projector panel named picodlp is supported by OMAP.
panel driver is required to be added with the name picodlp_panel.

It is a WVGA panel with resolution 864 X 480 and panel timing data
is defined in the panel driver.

picodlp makes use of parallel (DPI) interface multiplexed with secondary lcd
in case of OMAP4.

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

Comments

Tomi Valkeinen April 19, 2011, 11:09 a.m. UTC | #1
On Mon, 2011-04-18 at 11:45 +0530, Mayuresh Janorkar wrote:
> From: Mythri P K <mythripk@ti.com>
> 
> A projector panel named picodlp is supported by OMAP.
> panel driver is required to be added with the name picodlp_panel.
> 
> It is a WVGA panel with resolution 864 X 480 and panel timing data
> is defined in the panel driver.
> 
> picodlp makes use of parallel (DPI) interface multiplexed with secondary lcd
> in case of OMAP4.
> 
> Signed-off-by: Mythri P K <mythripk@ti.com>
> Signed-off-by: Mayuresh Janorkar <mayur@ti.com>
> ---
>  drivers/video/omap2/displays/panel-picodlp.c |  228 ++++++++++++++++++++++++++
>  1 files changed, 228 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/video/omap2/displays/panel-picodlp.c
> 
> diff --git a/drivers/video/omap2/displays/panel-picodlp.c b/drivers/video/omap2/displays/panel-picodlp.c
> new file mode 100644
> index 0000000..4f12903
> --- /dev/null
> +++ b/drivers/video/omap2/displays/panel-picodlp.c
> @@ -0,0 +1,228 @@
> +/*
> + * 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/>.
> + */
> +
> +#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/delay.h>
> +
> +#include <plat/display.h>
> +#include <plat/panel-picodlp.h>
> +
> +struct picodlp_data {
> +	struct mutex lock;
> +};
> +
> +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 int picodlp_panel_power_on(struct omap_dss_device *dssdev)
> +{
> +	int r;
> +
> +	if (dssdev->platform_enable) {
> +		r = dssdev->platform_enable(dssdev);
> +		if (r)
> +			return r;
> +	}
> +
> +	r = omapdss_dpi_display_enable(dssdev);
> +	if (r) {
> +		dev_err(&dssdev->dev, "failed to enable DPI\n");
> +		goto err;
> +	}
> +	/* after enabling, wait for some initialize sync interrupts */
> +	msleep(675);
> +	dssdev->state = OMAP_DSS_DISPLAY_ACTIVE;
> +
> +	return 0;
> +
> +err:
> +	if (dssdev->platform_disable)
> +		dssdev->platform_disable(dssdev);
> +
> +	return r;
> +}

Why is the msleep needed there? It's a huge sleep, and can't find
information about it from the documents you gave links to. I think I've
asked this three times already.

Also, it's rather strange to sleep at the end of the function. Normally
you would sleep between two actions.

 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
MAYURESH JANORKAR April 21, 2011, 11:06 a.m. UTC | #2
> -----Original Message-----
> From: Valkeinen, Tomi
> Sent: Tuesday, April 19, 2011 4:40 PM
> To: Janorkar, Mayuresh
> Cc: linux-omap@vger.kernel.org; K, Mythri P
> Subject: Re: [PATCH 3/7] OMAP: DSS: Adding a picodlp panel driver
> 
> On Mon, 2011-04-18 at 11:45 +0530, Mayuresh Janorkar wrote:
> > From: Mythri P K <mythripk@ti.com>
> >
> > A projector panel named picodlp is supported by OMAP.
> > panel driver is required to be added with the name picodlp_panel.
> >
> > It is a WVGA panel with resolution 864 X 480 and panel timing data
> > is defined in the panel driver.
> >
> > picodlp makes use of parallel (DPI) interface multiplexed with secondary
> lcd
> > in case of OMAP4.
> >
> > Signed-off-by: Mythri P K <mythripk@ti.com>
> > Signed-off-by: Mayuresh Janorkar <mayur@ti.com>
> > ---
> >  drivers/video/omap2/displays/panel-picodlp.c |  228
> ++++++++++++++++++++++++++
> >  1 files changed, 228 insertions(+), 0 deletions(-)
> >  create mode 100644 drivers/video/omap2/displays/panel-picodlp.c
> >
> > diff --git a/drivers/video/omap2/displays/panel-picodlp.c
> b/drivers/video/omap2/displays/panel-picodlp.c
> > new file mode 100644
> > index 0000000..4f12903
> > --- /dev/null
> > +++ b/drivers/video/omap2/displays/panel-picodlp.c
> > @@ -0,0 +1,228 @@
> > +/*
> > + * 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/>.
> > + */
> > +
> > +#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/delay.h>
> > +
> > +#include <plat/display.h>
> > +#include <plat/panel-picodlp.h>
> > +
> > +struct picodlp_data {
> > +	struct mutex lock;
> > +};
> > +
> > +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 int picodlp_panel_power_on(struct omap_dss_device *dssdev)
> > +{
> > +	int r;
> > +
> > +	if (dssdev->platform_enable) {
> > +		r = dssdev->platform_enable(dssdev);
> > +		if (r)
> > +			return r;
> > +	}
> > +
> > +	r = omapdss_dpi_display_enable(dssdev);
> > +	if (r) {
> > +		dev_err(&dssdev->dev, "failed to enable DPI\n");
> > +		goto err;
> > +	}
> > +	/* after enabling, wait for some initialize sync interrupts */
> > +	msleep(675);
> > +	dssdev->state = OMAP_DSS_DISPLAY_ACTIVE;
> > +
> > +	return 0;
> > +
> > +err:
> > +	if (dssdev->platform_disable)
> > +		dssdev->platform_disable(dssdev);
> > +
> > +	return r;
> > +}
> 
> Why is the msleep needed there? It's a huge sleep, and can't find
> information about it from the documents you gave links to. I think I've
> asked this three times already.
> 

It is practically observed that without this delay the pico panel does not get initialized.

> Also, it's rather strange to sleep at the end of the function. Normally
> you would sleep between two actions.

Some of the panels in drivers/video/omap2/displays sleep after dpi_display_enable.

I agree here the sleep is huge. But I could not minimize it below this value.
I can handle it in some other ways:
See if the i2c_pakcet is sent. If not sent wait for some time ~50ms and then resend.

> 
>  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
Tomi Valkeinen April 26, 2011, 10:42 a.m. UTC | #3
On Thu, 2011-04-21 at 16:36 +0530, Janorkar, Mayuresh wrote:
> 
> > -----Original Message-----
> > From: Valkeinen, Tomi
> > Sent: Tuesday, April 19, 2011 4:40 PM
> > To: Janorkar, Mayuresh
> > Cc: linux-omap@vger.kernel.org; K, Mythri P
> > Subject: Re: [PATCH 3/7] OMAP: DSS: Adding a picodlp panel driver
> > 
> > On Mon, 2011-04-18 at 11:45 +0530, Mayuresh Janorkar wrote:

> > > +static int picodlp_panel_power_on(struct omap_dss_device *dssdev)
> > > +{
> > > +	int r;
> > > +
> > > +	if (dssdev->platform_enable) {
> > > +		r = dssdev->platform_enable(dssdev);
> > > +		if (r)
> > > +			return r;
> > > +	}
> > > +
> > > +	r = omapdss_dpi_display_enable(dssdev);
> > > +	if (r) {
> > > +		dev_err(&dssdev->dev, "failed to enable DPI\n");
> > > +		goto err;
> > > +	}
> > > +	/* after enabling, wait for some initialize sync interrupts */
> > > +	msleep(675);
> > > +	dssdev->state = OMAP_DSS_DISPLAY_ACTIVE;
> > > +
> > > +	return 0;
> > > +
> > > +err:
> > > +	if (dssdev->platform_disable)
> > > +		dssdev->platform_disable(dssdev);
> > > +
> > > +	return r;
> > > +}
> > 
> > Why is the msleep needed there? It's a huge sleep, and can't find
> > information about it from the documents you gave links to. I think I've
> > asked this three times already.
> > 
> 
> It is practically observed that without this delay the pico panel does not get initialized.
> 
> > Also, it's rather strange to sleep at the end of the function. Normally
> > you would sleep between two actions.
> 
> Some of the panels in drivers/video/omap2/displays sleep after dpi_display_enable.

Yes, but the drivers sleep between dpi_display_enable and
platform_enable. They don't sleep at the end of a function, they sleep
between two actions because the second action requires the first action
to be stabilized. More exactly, the panel needs to get the pixel clock
for a few frames to initialize itself, so that the image is stable when
the panel is taken out of reset.

> I agree here the sleep is huge. But I could not minimize it below this value.
> I can handle it in some other ways:
> See if the i2c_pakcet is sent. If not sent wait for some time ~50ms and then resend.

You need to find the documentation describing the power-up times to
understand what is required and where.

Most likely the case here is that picodlp takes rather long to wake up
after it's taken out of reset (i.e. platform_enabl. dpi_display_enable
probably doesn't have anything to do with this), and you need to wait
that time before sending i2c messages.

And if that is the case, you don't need to sleep after platform_enable
or dpi_display_enable, but before sending the first i2c message. And as
the sleep here is such a long sleep, I think it warrants some
optimization. If the sending of the i2c message happens in some other
function, you can store the timestamp at the time when the reset is
removed, and before sending the i2c message use the timestamp to sleep
enough to be sure the picodlp is up and running.

 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
new file mode 100644
index 0000000..4f12903
--- /dev/null
+++ b/drivers/video/omap2/displays/panel-picodlp.c
@@ -0,0 +1,228 @@ 
+/*
+ * 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/>.
+ */
+
+#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/delay.h>
+
+#include <plat/display.h>
+#include <plat/panel-picodlp.h>
+
+struct picodlp_data {
+	struct mutex lock;
+};
+
+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 int picodlp_panel_power_on(struct omap_dss_device *dssdev)
+{
+	int r;
+
+	if (dssdev->platform_enable) {
+		r = dssdev->platform_enable(dssdev);
+		if (r)
+			return r;
+	}
+
+	r = omapdss_dpi_display_enable(dssdev);
+	if (r) {
+		dev_err(&dssdev->dev, "failed to enable DPI\n");
+		goto err;
+	}
+	/* after enabling, wait for some initialize sync interrupts */
+	msleep(675);
+	dssdev->state = OMAP_DSS_DISPLAY_ACTIVE;
+
+	return 0;
+
+err:
+	if (dssdev->platform_disable)
+		dssdev->platform_disable(dssdev);
+
+	return r;
+}
+
+static void picodlp_panel_power_off(struct omap_dss_device *dssdev)
+{
+	omapdss_dpi_display_disable(dssdev);
+
+	if (dssdev->platform_disable)
+		dssdev->platform_disable(dssdev);
+}
+
+static int picodlp_panel_probe(struct omap_dss_device *dssdev)
+{
+	struct picodlp_data *picod;
+
+	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);
+	dev_set_drvdata(&dssdev->dev, picod);
+	return 0;
+}
+
+static void picodlp_panel_remove(struct omap_dss_device *dssdev)
+{
+	struct picodlp_data *picod = dev_get_drvdata(&dssdev->dev);
+
+	dev_set_drvdata(&dssdev->dev, NULL);
+	dev_dbg(&dssdev->dev, " removing picodlp panel\n");
+	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);
+
+	dev_dbg(&dssdev->dev, " disabling picodlp panel\n");
+	dssdev->state = OMAP_DSS_DISPLAY_DISABLED;
+
+	mutex_unlock(&picod->lock);
+}
+
+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) {
+		dev_dbg(&dssdev->dev, " unable to suspend picodlp panel,"\
+						" panel is not ACTIVE\n");
+		mutex_unlock(&picod->lock);
+		return -EINVAL;
+	}
+
+	picodlp_panel_power_off(dssdev);
+
+	dssdev->state = OMAP_DSS_DISPLAY_SUSPENDED;
+	dev_dbg(&dssdev->dev, " suspending picodlp panel\n");
+
+	mutex_unlock(&picod->lock);
+	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) {
+		dev_dbg(&dssdev->dev, " unable to resume picodlp panel,"
+					" panel is not ACTIVE\n");
+		mutex_unlock(&picod->lock);
+		return -EINVAL;
+	}
+
+	dev_dbg(&dssdev->dev, " resuming picodlp panel\n");
+	r = picodlp_panel_power_on(dssdev);
+	mutex_unlock(&picod->lock);
+
+	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)
+{
+	return omap_dss_register_driver(&picodlp_driver);
+}
+
+static void __exit picodlp_exit(void)
+{
+	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");