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

Tomi Valkeinen
2011-04-19T11:09:51Z

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