Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* [PATCHv2 12/15] OMAP: stalker: Remove LCD device from board file
From: Tomi Valkeinen @ 2011-09-12  9:13 UTC (permalink / raw)
  To: linux-omap, linux-fbdev; +Cc: archit, Tomi Valkeinen, Jason Lam
In-Reply-To: <1315818818-18733-1-git-send-email-tomi.valkeinen@ti.com>

OMAP3 Stalker board has definitions for LCD, but uses the generic driver
without any information what kind of LCD it has. The board should use a
particular panel type from panel-generic-dpi driver, not the generic
one.

As I haven't gotten response the signer-off of stalker board about the
issue, this patch removes the LCD support from the board file. This will
allow us to clean up the panel-generic-dpi driver and make it support
only fixed size panels.

CC: Jason Lam <lzg@ema-tech.com>
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 arch/arm/mach-omap2/board-omap3stalker.c |   34 ------------------------------
 1 files changed, 0 insertions(+), 34 deletions(-)

diff --git a/arch/arm/mach-omap2/board-omap3stalker.c b/arch/arm/mach-omap2/board-omap3stalker.c
index 8ab99f1..106a2ba 100644
--- a/arch/arm/mach-omap2/board-omap3stalker.c
+++ b/arch/arm/mach-omap2/board-omap3stalker.c
@@ -108,39 +108,6 @@ static void __init omap3_stalker_display_init(void)
 	return;
 }
 
-static int omap3_stalker_enable_lcd(struct omap_dss_device *dssdev)
-{
-	if (dvi_enabled) {
-		printk(KERN_ERR "cannot enable LCD, DVI is enabled\n");
-		return -EINVAL;
-	}
-	gpio_set_value(DSS_ENABLE_GPIO, 1);
-	gpio_set_value(LCD_PANEL_BKLIGHT_GPIO, 1);
-	lcd_enabled = 1;
-	return 0;
-}
-
-static void omap3_stalker_disable_lcd(struct omap_dss_device *dssdev)
-{
-	gpio_set_value(DSS_ENABLE_GPIO, 0);
-	gpio_set_value(LCD_PANEL_BKLIGHT_GPIO, 0);
-	lcd_enabled = 0;
-}
-
-static struct panel_generic_dpi_data lcd_panel = {
-	.name			= "generic",
-	.platform_enable	= omap3_stalker_enable_lcd,
-	.platform_disable	= omap3_stalker_disable_lcd,
-};
-
-static struct omap_dss_device omap3_stalker_lcd_device = {
-	.name			= "lcd",
-	.driver_name		= "generic_dpi_panel",
-	.data			= &lcd_panel,
-	.phy.dpi.data_lines	= 24,
-	.type			= OMAP_DISPLAY_TYPE_DPI,
-};
-
 static int omap3_stalker_enable_tv(struct omap_dss_device *dssdev)
 {
 	return 0;
@@ -194,7 +161,6 @@ static struct omap_dss_device omap3_stalker_dvi_device = {
 };
 
 static struct omap_dss_device *omap3_stalker_dss_devices[] = {
-	&omap3_stalker_lcd_device,
 	&omap3_stalker_tv_device,
 	&omap3_stalker_dvi_device,
 };
-- 
1.7.4.1


^ permalink raw reply related

* [PATCHv2 13/15] OMAP: DSS2: panel-generic-dpi: remove "generic" panel
From: Tomi Valkeinen @ 2011-09-12  9:13 UTC (permalink / raw)
  To: linux-omap, linux-fbdev; +Cc: archit, Tomi Valkeinen
In-Reply-To: <1315818818-18733-1-git-send-email-tomi.valkeinen@ti.com>

Remove the "generic" panel config entry, which is not used.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/video/omap2/displays/panel-generic-dpi.c |   24 ----------------------
 1 files changed, 0 insertions(+), 24 deletions(-)

diff --git a/drivers/video/omap2/displays/panel-generic-dpi.c b/drivers/video/omap2/displays/panel-generic-dpi.c
index b401304..e0399c3 100644
--- a/drivers/video/omap2/displays/panel-generic-dpi.c
+++ b/drivers/video/omap2/displays/panel-generic-dpi.c
@@ -58,30 +58,6 @@ struct panel_config {
 
 /* Panel configurations */
 static struct panel_config generic_dpi_panels[] = {
-	/* Generic Panel */
-	{
-		{
-			.x_res		= 640,
-			.y_res		= 480,
-
-			.pixel_clock	= 23500,
-
-			.hfp		= 48,
-			.hsw		= 32,
-			.hbp		= 80,
-
-			.vfp		= 3,
-			.vsw		= 4,
-			.vbp		= 7,
-		},
-		.acbi			= 0x0,
-		.acb			= 0x0,
-		.config			= OMAP_DSS_LCD_TFT,
-		.power_on_delay		= 0,
-		.power_off_delay	= 0,
-		.name			= "generic",
-	},
-
 	/* Sharp LQ043T1DG01 */
 	{
 		{
-- 
1.7.4.1


^ permalink raw reply related

* [PATCHv2 14/15] OMAP: Panda, Beagle, Overo: DVI: Add i2c_bus_num
From: Tomi Valkeinen @ 2011-09-12  9:13 UTC (permalink / raw)
  To: linux-omap, linux-fbdev; +Cc: archit, Tomi Valkeinen
In-Reply-To: <1315818818-18733-1-git-send-email-tomi.valkeinen@ti.com>

Add i2c bus number for DVI output. The driver uses this to detect if a
panel is connected and to read EDID.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 arch/arm/mach-omap2/board-omap3beagle.c |    1 +
 arch/arm/mach-omap2/board-omap4panda.c  |    1 +
 arch/arm/mach-omap2/board-overo.c       |    1 +
 3 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/board-omap3beagle.c b/arch/arm/mach-omap2/board-omap3beagle.c
index 26bc860..742ac45 100644
--- a/arch/arm/mach-omap2/board-omap3beagle.c
+++ b/arch/arm/mach-omap2/board-omap3beagle.c
@@ -206,6 +206,7 @@ static void beagle_disable_dvi(struct omap_dss_device *dssdev)
 static struct panel_dvi_platform_data dvi_panel = {
 	.platform_enable = beagle_enable_dvi,
 	.platform_disable = beagle_disable_dvi,
+	.i2c_bus_num = 3,
 };
 
 static struct omap_dss_device beagle_dvi_device = {
diff --git a/arch/arm/mach-omap2/board-omap4panda.c b/arch/arm/mach-omap2/board-omap4panda.c
index c35384e..a38ed273 100644
--- a/arch/arm/mach-omap2/board-omap4panda.c
+++ b/arch/arm/mach-omap2/board-omap4panda.c
@@ -458,6 +458,7 @@ static void omap4_panda_disable_dvi(struct omap_dss_device *dssdev)
 static struct panel_dvi_platform_data omap4_dvi_panel = {
 	.platform_enable	= omap4_panda_enable_dvi,
 	.platform_disable	= omap4_panda_disable_dvi,
+	.i2c_bus_num = 3,
 };
 
 struct omap_dss_device omap4_panda_dvi_device = {
diff --git a/arch/arm/mach-omap2/board-overo.c b/arch/arm/mach-omap2/board-overo.c
index 06064d5..da94376 100644
--- a/arch/arm/mach-omap2/board-overo.c
+++ b/arch/arm/mach-omap2/board-overo.c
@@ -186,6 +186,7 @@ static void overo_panel_disable_dvi(struct omap_dss_device *dssdev)
 static struct panel_dvi_platform_data dvi_panel = {
 	.platform_enable	= overo_panel_enable_dvi,
 	.platform_disable	= overo_panel_disable_dvi,
+	.i2c_bus_num		= 3,
 };
 
 static struct omap_dss_device overo_dvi_device = {
-- 
1.7.4.1


^ permalink raw reply related

* [PATCHv2 15/15] OMAPFB: find best mode from edid
From: Tomi Valkeinen @ 2011-09-12  9:13 UTC (permalink / raw)
  To: linux-omap, linux-fbdev; +Cc: archit, Tomi Valkeinen
In-Reply-To: <1315818818-18733-1-git-send-email-tomi.valkeinen@ti.com>

Use the new read_edid() function to get EDID information from the
display (when available), and use the information to use a suitable mode
at initialization time.

Hot-plug is not yet supported, so the timings selected at init time will
stay even if the monitor would be changed.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/video/omap2/omapfb/omapfb-main.c |  109 +++++++++++++++++++++++++++---
 1 files changed, 99 insertions(+), 10 deletions(-)

diff --git a/drivers/video/omap2/omapfb/omapfb-main.c b/drivers/video/omap2/omapfb/omapfb-main.c
index cd2cae8e..c84cc29 100644
--- a/drivers/video/omap2/omapfb/omapfb-main.c
+++ b/drivers/video/omap2/omapfb/omapfb-main.c
@@ -2276,6 +2276,87 @@ static int omapfb_parse_def_modes(struct omapfb2_device *fbdev)
 	return r;
 }
 
+static void fb_videomode_to_omap_timings(struct fb_videomode *m,
+		struct omap_video_timings *t)
+{
+	t->x_res = m->xres;
+	t->y_res = m->yres;
+	t->pixel_clock = PICOS2KHZ(m->pixclock);
+	t->hsw = m->hsync_len;
+	t->hfp = m->right_margin;
+	t->hbp = m->left_margin;
+	t->vsw = m->vsync_len;
+	t->vfp = m->lower_margin;
+	t->vbp = m->upper_margin;
+}
+
+static int omapfb_find_best_mode(struct omap_dss_device *display,
+		struct omap_video_timings *timings)
+{
+	struct fb_monspecs *specs;
+	u8 *edid;
+	int r, i, best_xres, best_idx, len;
+
+	if (!display->driver->read_edid)
+		return -ENODEV;
+
+	len = 0x80 * 2;
+	edid = kmalloc(len, GFP_KERNEL);
+
+	r = display->driver->read_edid(display, edid, len);
+	if (r < 0)
+		goto err1;
+
+	specs = kzalloc(sizeof(*specs), GFP_KERNEL);
+
+	fb_edid_to_monspecs(edid, specs);
+
+	if (edid[126] > 0)
+		fb_edid_add_monspecs(edid + 0x80, specs);
+
+	best_xres = 0;
+	best_idx = -1;
+
+	for (i = 0; i < specs->modedb_len; ++i) {
+		struct fb_videomode *m;
+		struct omap_video_timings t;
+
+		m = &specs->modedb[i];
+
+		if (m->pixclock = 0)
+			continue;
+
+		/* skip repeated pixel modes */
+		if (m->xres = 2880 || m->xres = 1440)
+			continue;
+
+		fb_videomode_to_omap_timings(m, &t);
+
+		r = display->driver->check_timings(display, &t);
+		if (r = 0 && best_xres < m->xres) {
+			best_xres = m->xres;
+			best_idx = i;
+		}
+	}
+
+	if (best_xres = 0) {
+		r = -ENOENT;
+		goto err2;
+	}
+
+	fb_videomode_to_omap_timings(&specs->modedb[best_idx], timings);
+
+	r = 0;
+
+err2:
+	fb_destroy_modedb(specs->modedb);
+	kfree(specs);
+err1:
+	kfree(edid);
+
+	return r;
+}
+
 static int omapfb_init_display(struct omapfb2_device *fbdev,
 		struct omap_dss_device *dssdev)
 {
@@ -2404,9 +2485,27 @@ static int omapfb_probe(struct platform_device *pdev)
 	for (i = 0; i < fbdev->num_managers; i++)
 		fbdev->managers[i] = omap_dss_get_overlay_manager(i);
 
+	/* gfx overlay should be the default one. find a display
+	 * connected to that, and use it as default display */
+	ovl = omap_dss_get_overlay(0);
+	if (ovl->manager && ovl->manager->device) {
+		def_display = ovl->manager->device;
+	} else {
+		dev_warn(&pdev->dev, "cannot find default display\n");
+		def_display = NULL;
+	}
+
 	if (def_mode && strlen(def_mode) > 0) {
 		if (omapfb_parse_def_modes(fbdev))
 			dev_warn(&pdev->dev, "cannot parse default modes\n");
+	} else if (def_display && def_display->driver->set_timings &&
+			def_display->driver->check_timings) {
+		struct omap_video_timings t;
+
+		r = omapfb_find_best_mode(def_display, &t);
+
+		if (r = 0)
+			def_display->driver->set_timings(def_display, &t);
 	}
 
 	r = omapfb_create_framebuffers(fbdev);
@@ -2423,16 +2522,6 @@ static int omapfb_probe(struct platform_device *pdev)
 
 	DBG("mgr->apply'ed\n");
 
-	/* gfx overlay should be the default one. find a display
-	 * connected to that, and use it as default display */
-	ovl = omap_dss_get_overlay(0);
-	if (ovl->manager && ovl->manager->device) {
-		def_display = ovl->manager->device;
-	} else {
-		dev_warn(&pdev->dev, "cannot find default display\n");
-		def_display = NULL;
-	}
-
 	if (def_display) {
 		r = omapfb_init_display(fbdev, def_display);
 		if (r) {
-- 
1.7.4.1


^ permalink raw reply related

* Re: [PATCHv2 14/15] OMAP: Panda, Beagle, Overo: DVI: Add i2c_bus_num
From: Enric Balletbò i Serra @ 2011-09-12  9:58 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev, archit
In-Reply-To: <1315818818-18733-15-git-send-email-tomi.valkeinen@ti.com>

2011/9/12 Tomi Valkeinen <tomi.valkeinen@ti.com>:
> Add i2c bus number for DVI output. The driver uses this to detect if a
> panel is connected and to read EDID.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
>  arch/arm/mach-omap2/board-omap3beagle.c |    1 +
>  arch/arm/mach-omap2/board-omap4panda.c  |    1 +
>  arch/arm/mach-omap2/board-overo.c       |    1 +
>  3 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/board-omap3beagle.c b/arch/arm/mach-omap2/board-omap3beagle.c
> index 26bc860..742ac45 100644
> --- a/arch/arm/mach-omap2/board-omap3beagle.c
> +++ b/arch/arm/mach-omap2/board-omap3beagle.c
> @@ -206,6 +206,7 @@ static void beagle_disable_dvi(struct omap_dss_device *dssdev)
>  static struct panel_dvi_platform_data dvi_panel = {
>        .platform_enable = beagle_enable_dvi,
>        .platform_disable = beagle_disable_dvi,
> +       .i2c_bus_num = 3,
>  };
>
>  static struct omap_dss_device beagle_dvi_device = {
> diff --git a/arch/arm/mach-omap2/board-omap4panda.c b/arch/arm/mach-omap2/board-omap4panda.c
> index c35384e..a38ed273 100644
> --- a/arch/arm/mach-omap2/board-omap4panda.c
> +++ b/arch/arm/mach-omap2/board-omap4panda.c
> @@ -458,6 +458,7 @@ static void omap4_panda_disable_dvi(struct omap_dss_device *dssdev)
>  static struct panel_dvi_platform_data omap4_dvi_panel = {
>        .platform_enable        = omap4_panda_enable_dvi,
>        .platform_disable       = omap4_panda_disable_dvi,
> +       .i2c_bus_num = 3,
>  };
>
>  struct omap_dss_device omap4_panda_dvi_device = {
> diff --git a/arch/arm/mach-omap2/board-overo.c b/arch/arm/mach-omap2/board-overo.c
> index 06064d5..da94376 100644
> --- a/arch/arm/mach-omap2/board-overo.c
> +++ b/arch/arm/mach-omap2/board-overo.c
> @@ -186,6 +186,7 @@ static void overo_panel_disable_dvi(struct omap_dss_device *dssdev)
>  static struct panel_dvi_platform_data dvi_panel = {
>        .platform_enable        = overo_panel_enable_dvi,
>        .platform_disable       = overo_panel_disable_dvi,
> +       .i2c_bus_num            = 3,
>  };
>
>  static struct omap_dss_device overo_dvi_device = {
> --
> 1.7.4.1
>
> --
> 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
>

Please, can also include the same modification for IGEP v2 board
(arch/arm/mach-omap2/board-igep0020.c) ?

Best regards,
   Enric

^ permalink raw reply

* Re: [PATCHv2 14/15] OMAP: Panda, Beagle, Overo: DVI: Add
From: Tomi Valkeinen @ 2011-09-12 10:17 UTC (permalink / raw)
  To: Enric Balletbò i Serra; +Cc: linux-omap, linux-fbdev, archit
In-Reply-To: <CAFqH_53ttom-c75NihgmMsckp4RQTpskXhYjAEAee2LWsHXW0g@mail.gmail.com>

On Mon, 2011-09-12 at 11:58 +0200, Enric Balletbò i Serra wrote:
> 2011/9/12 Tomi Valkeinen <tomi.valkeinen@ti.com>:
> > Add i2c bus number for DVI output. The driver uses this to detect if a
> > panel is connected and to read EDID.
> >
> > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> > ---
> >  arch/arm/mach-omap2/board-omap3beagle.c |    1 +
> >  arch/arm/mach-omap2/board-omap4panda.c  |    1 +
> >  arch/arm/mach-omap2/board-overo.c       |    1 +
> >  3 files changed, 3 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/arm/mach-omap2/board-omap3beagle.c b/arch/arm/mach-omap2/board-omap3beagle.c
> > index 26bc860..742ac45 100644
> > --- a/arch/arm/mach-omap2/board-omap3beagle.c
> > +++ b/arch/arm/mach-omap2/board-omap3beagle.c
> > @@ -206,6 +206,7 @@ static void beagle_disable_dvi(struct omap_dss_device *dssdev)
> >  static struct panel_dvi_platform_data dvi_panel = {
> >        .platform_enable = beagle_enable_dvi,
> >        .platform_disable = beagle_disable_dvi,
> > +       .i2c_bus_num = 3,
> >  };
> >
> >  static struct omap_dss_device beagle_dvi_device = {
> > diff --git a/arch/arm/mach-omap2/board-omap4panda.c b/arch/arm/mach-omap2/board-omap4panda.c
> > index c35384e..a38ed273 100644
> > --- a/arch/arm/mach-omap2/board-omap4panda.c
> > +++ b/arch/arm/mach-omap2/board-omap4panda.c
> > @@ -458,6 +458,7 @@ static void omap4_panda_disable_dvi(struct omap_dss_device *dssdev)
> >  static struct panel_dvi_platform_data omap4_dvi_panel = {
> >        .platform_enable        = omap4_panda_enable_dvi,
> >        .platform_disable       = omap4_panda_disable_dvi,
> > +       .i2c_bus_num = 3,
> >  };
> >
> >  struct omap_dss_device omap4_panda_dvi_device = {
> > diff --git a/arch/arm/mach-omap2/board-overo.c b/arch/arm/mach-omap2/board-overo.c
> > index 06064d5..da94376 100644
> > --- a/arch/arm/mach-omap2/board-overo.c
> > +++ b/arch/arm/mach-omap2/board-overo.c
> > @@ -186,6 +186,7 @@ static void overo_panel_disable_dvi(struct omap_dss_device *dssdev)
> >  static struct panel_dvi_platform_data dvi_panel = {
> >        .platform_enable        = overo_panel_enable_dvi,
> >        .platform_disable       = overo_panel_disable_dvi,
> > +       .i2c_bus_num            = 3,
> >  };
> >
> >  static struct omap_dss_device overo_dvi_device = {
> > --
> > 1.7.4.1
> >
> > --
> > 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
> >
> 
> Please, can also include the same modification for IGEP v2 board
> (arch/arm/mach-omap2/board-igep0020.c) ?

Sure. You have tested that the i2c bus is 3 and it works?

 Tomi



^ permalink raw reply

* Re: [PATCHv2 8/8] OMAP: DSS2: HDMI: improve hdmi output enable
From: Tomi Valkeinen @ 2011-09-12 11:06 UTC (permalink / raw)
  To: Archit Taneja
  Cc: linux-omap@vger.kernel.org, linux-fbdev@vger.kernel.org,
	K, Mythri P
In-Reply-To: <4E6DE68F.3020809@ti.com>

On Mon, 2011-09-12 at 16:31 +0530, Archit Taneja wrote:
> On Monday 12 September 2011 02:42 PM, Valkeinen, Tomi wrote:
> > Enabling HDMI output often causes sync lost errors, and almost always
> > causes timeout errors being printed from dispc_mgr_enable_digit_out().
> >
> > The sync lost problem seems to go lessen greatly if we first enable the
> > HDMI output, and only then enable the DISPC output. However, as this is
> > only based on observations, the fix may not be perfect as the problem
> > may lie somewhere else. Nevertheless, HDMI works better with this patch.
> >
> > This will also fix the dispc's dispc_mgr_enable_digit_out(), as the code
> > waits for two VSYNCs after enabling the output. If the HDMI output is
> > disabled (as it was previously), there are no VSYNCs and
> > dispc_mgr_enable_digit_out() will print timeout errors.
> >
> > Cc: Mythri P K<mythripk@ti.com>
> > Signed-off-by: Tomi Valkeinen<tomi.valkeinen@ti.com>
> > ---
> >   drivers/video/omap2/dss/hdmi.c |    4 ++--
> >   1 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c
> > index 4752137..06a78b2 100644
> > --- a/drivers/video/omap2/dss/hdmi.c
> > +++ b/drivers/video/omap2/dss/hdmi.c
> > @@ -529,10 +529,10 @@ static int hdmi_power_on(struct omap_dss_device *dssdev)
> >   	dispc_set_digit_size(dssdev->panel.timings.x_res,
> >   			dssdev->panel.timings.y_res);
> >
> > -	dispc_mgr_enable(OMAP_DSS_CHANNEL_DIGIT, 1);
> > -
> >   	hdmi.ip_data.ops->video_enable(&hdmi.ip_data, 1);
> >
> > +	dispc_mgr_enable(OMAP_DSS_CHANNEL_DIGIT, 1);
> > +
> 
> What content would HDMI push out till the time DIGIT is enabled? It 
> might be interesting to put some milliseconds of delay here and see what 
> happens.

I guess HDMI works independently of DISPC. So HDMI will output whatever
is in the video port from DISPC to HDMI, regardless of what DISPC does.
My guess is it's either random bits or, more probably, zeroes.

I think this way is the same as used for VENC: first we configure and
enable VENC, then we enable DISPC digit output.

 Tomi



^ permalink raw reply

* Re: [PATCHv2 8/8] OMAP: DSS2: HDMI: improve hdmi output enable
From: Archit Taneja @ 2011-09-12 11:13 UTC (permalink / raw)
  To: Valkeinen, Tomi
  Cc: linux-omap@vger.kernel.org, linux-fbdev@vger.kernel.org,
	K, Mythri P
In-Reply-To: <1315818773-18660-9-git-send-email-tomi.valkeinen@ti.com>

On Monday 12 September 2011 02:42 PM, Valkeinen, Tomi wrote:
> Enabling HDMI output often causes sync lost errors, and almost always
> causes timeout errors being printed from dispc_mgr_enable_digit_out().
>
> The sync lost problem seems to go lessen greatly if we first enable the
> HDMI output, and only then enable the DISPC output. However, as this is
> only based on observations, the fix may not be perfect as the problem
> may lie somewhere else. Nevertheless, HDMI works better with this patch.
>
> This will also fix the dispc's dispc_mgr_enable_digit_out(), as the code
> waits for two VSYNCs after enabling the output. If the HDMI output is
> disabled (as it was previously), there are no VSYNCs and
> dispc_mgr_enable_digit_out() will print timeout errors.
>
> Cc: Mythri P K<mythripk@ti.com>
> Signed-off-by: Tomi Valkeinen<tomi.valkeinen@ti.com>
> ---
>   drivers/video/omap2/dss/hdmi.c |    4 ++--
>   1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c
> index 4752137..06a78b2 100644
> --- a/drivers/video/omap2/dss/hdmi.c
> +++ b/drivers/video/omap2/dss/hdmi.c
> @@ -529,10 +529,10 @@ static int hdmi_power_on(struct omap_dss_device *dssdev)
>   	dispc_set_digit_size(dssdev->panel.timings.x_res,
>   			dssdev->panel.timings.y_res);
>
> -	dispc_mgr_enable(OMAP_DSS_CHANNEL_DIGIT, 1);
> -
>   	hdmi.ip_data.ops->video_enable(&hdmi.ip_data, 1);
>
> +	dispc_mgr_enable(OMAP_DSS_CHANNEL_DIGIT, 1);
> +

What content would HDMI push out till the time DIGIT is enabled? It 
might be interesting to put some milliseconds of delay here and see what 
happens.

Archit

>   	return 0;
>   err:
>   	hdmi_runtime_put();


^ permalink raw reply

* Re: [PATCHv2 14/15] OMAP: Panda, Beagle, Overo: DVI: Add i2c_bus_num
From: Enric Balletbò i Serra @ 2011-09-12 11:13 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev, archit
In-Reply-To: <1315822645.2177.23.camel@deskari>

2011/9/12 Tomi Valkeinen <tomi.valkeinen@ti.com>:
> On Mon, 2011-09-12 at 11:58 +0200, Enric Balletbò i Serra wrote:
>> 2011/9/12 Tomi Valkeinen <tomi.valkeinen@ti.com>:
>> > Add i2c bus number for DVI output. The driver uses this to detect if a
>> > panel is connected and to read EDID.
>> >
>> > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
>> > ---
>> >  arch/arm/mach-omap2/board-omap3beagle.c |    1 +
>> >  arch/arm/mach-omap2/board-omap4panda.c  |    1 +
>> >  arch/arm/mach-omap2/board-overo.c       |    1 +
>> >  3 files changed, 3 insertions(+), 0 deletions(-)
>> >
>> > diff --git a/arch/arm/mach-omap2/board-omap3beagle.c b/arch/arm/mach-omap2/board-omap3beagle.c
>> > index 26bc860..742ac45 100644
>> > --- a/arch/arm/mach-omap2/board-omap3beagle.c
>> > +++ b/arch/arm/mach-omap2/board-omap3beagle.c
>> > @@ -206,6 +206,7 @@ static void beagle_disable_dvi(struct omap_dss_device *dssdev)
>> >  static struct panel_dvi_platform_data dvi_panel = {
>> >        .platform_enable = beagle_enable_dvi,
>> >        .platform_disable = beagle_disable_dvi,
>> > +       .i2c_bus_num = 3,
>> >  };
>> >
>> >  static struct omap_dss_device beagle_dvi_device = {
>> > diff --git a/arch/arm/mach-omap2/board-omap4panda.c b/arch/arm/mach-omap2/board-omap4panda.c
>> > index c35384e..a38ed273 100644
>> > --- a/arch/arm/mach-omap2/board-omap4panda.c
>> > +++ b/arch/arm/mach-omap2/board-omap4panda.c
>> > @@ -458,6 +458,7 @@ static void omap4_panda_disable_dvi(struct omap_dss_device *dssdev)
>> >  static struct panel_dvi_platform_data omap4_dvi_panel = {
>> >        .platform_enable        = omap4_panda_enable_dvi,
>> >        .platform_disable       = omap4_panda_disable_dvi,
>> > +       .i2c_bus_num = 3,
>> >  };
>> >
>> >  struct omap_dss_device omap4_panda_dvi_device = {
>> > diff --git a/arch/arm/mach-omap2/board-overo.c b/arch/arm/mach-omap2/board-overo.c
>> > index 06064d5..da94376 100644
>> > --- a/arch/arm/mach-omap2/board-overo.c
>> > +++ b/arch/arm/mach-omap2/board-overo.c
>> > @@ -186,6 +186,7 @@ static void overo_panel_disable_dvi(struct omap_dss_device *dssdev)
>> >  static struct panel_dvi_platform_data dvi_panel = {
>> >        .platform_enable        = overo_panel_enable_dvi,
>> >        .platform_disable       = overo_panel_disable_dvi,
>> > +       .i2c_bus_num            = 3,
>> >  };
>> >
>> >  static struct omap_dss_device overo_dvi_device = {
>> > --
>> > 1.7.4.1
>> >
>> > --
>> > 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
>> >
>>
>> Please, can also include the same modification for IGEP v2 board
>> (arch/arm/mach-omap2/board-igep0020.c) ?
>
> Sure. You have tested that the i2c bus is 3 and it works?

Yes, the i2c bus is 3 on IGEP v2 board and I'm just testing and seem is working.

>
>  Tomi
>
>
>

^ permalink raw reply

* Re: [PATCHv2 8/8] OMAP: DSS2: HDMI: improve hdmi output enable
From: Tomi Valkeinen @ 2011-09-12 11:29 UTC (permalink / raw)
  To: Archit Taneja
  Cc: linux-omap@vger.kernel.org, linux-fbdev@vger.kernel.org,
	K, Mythri P
In-Reply-To: <4E6DEBB6.6090703@ti.com>

On Mon, 2011-09-12 at 16:53 +0530, Archit Taneja wrote:
> On Monday 12 September 2011 04:36 PM, Valkeinen, Tomi wrote:
> > On Mon, 2011-09-12 at 16:31 +0530, Archit Taneja wrote:
> >> On Monday 12 September 2011 02:42 PM, Valkeinen, Tomi wrote:
> >>> Enabling HDMI output often causes sync lost errors, and almost always
> >>> causes timeout errors being printed from dispc_mgr_enable_digit_out().
> >>>
> >>> The sync lost problem seems to go lessen greatly if we first enable the
> >>> HDMI output, and only then enable the DISPC output. However, as this is
> >>> only based on observations, the fix may not be perfect as the problem
> >>> may lie somewhere else. Nevertheless, HDMI works better with this patch.
> >>>
> >>> This will also fix the dispc's dispc_mgr_enable_digit_out(), as the code
> >>> waits for two VSYNCs after enabling the output. If the HDMI output is
> >>> disabled (as it was previously), there are no VSYNCs and
> >>> dispc_mgr_enable_digit_out() will print timeout errors.
> >>>
> >>> Cc: Mythri P K<mythripk@ti.com>
> >>> Signed-off-by: Tomi Valkeinen<tomi.valkeinen@ti.com>
> >>> ---
> >>>    drivers/video/omap2/dss/hdmi.c |    4 ++--
> >>>    1 files changed, 2 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c
> >>> index 4752137..06a78b2 100644
> >>> --- a/drivers/video/omap2/dss/hdmi.c
> >>> +++ b/drivers/video/omap2/dss/hdmi.c
> >>> @@ -529,10 +529,10 @@ static int hdmi_power_on(struct omap_dss_device *dssdev)
> >>>    	dispc_set_digit_size(dssdev->panel.timings.x_res,
> >>>    			dssdev->panel.timings.y_res);
> >>>
> >>> -	dispc_mgr_enable(OMAP_DSS_CHANNEL_DIGIT, 1);
> >>> -
> >>>    	hdmi.ip_data.ops->video_enable(&hdmi.ip_data, 1);
> >>>
> >>> +	dispc_mgr_enable(OMAP_DSS_CHANNEL_DIGIT, 1);
> >>> +
> >>
> >> What content would HDMI push out till the time DIGIT is enabled? It
> >> might be interesting to put some milliseconds of delay here and see what
> >> happens.
> >
> > I guess HDMI works independently of DISPC. So HDMI will output whatever
> > is in the video port from DISPC to HDMI, regardless of what DISPC does.
> > My guess is it's either random bits or, more probably, zeroes.
> 
> Okay. I thought HDMI block may probe the video port's DE line or 
> something to check if it is pushing out valid content. However, with 
> this patch, we are more aligned with what we do in power_off(), i.e 
> first disable the manager and then disable HDMI.

Yep. But as I said in the desc, this is just based on my observations.
If this is the wrong way to enable tv-output, we should check whether
venc is also working the wrong way.

 Tomi



^ permalink raw reply

* Re: [PATCHv2 8/8] OMAP: DSS2: HDMI: improve hdmi output enable
From: Archit Taneja @ 2011-09-12 11:35 UTC (permalink / raw)
  To: Valkeinen, Tomi
  Cc: linux-omap@vger.kernel.org, linux-fbdev@vger.kernel.org,
	K, Mythri P
In-Reply-To: <1315825587.2177.27.camel@deskari>

On Monday 12 September 2011 04:36 PM, Valkeinen, Tomi wrote:
> On Mon, 2011-09-12 at 16:31 +0530, Archit Taneja wrote:
>> On Monday 12 September 2011 02:42 PM, Valkeinen, Tomi wrote:
>>> Enabling HDMI output often causes sync lost errors, and almost always
>>> causes timeout errors being printed from dispc_mgr_enable_digit_out().
>>>
>>> The sync lost problem seems to go lessen greatly if we first enable the
>>> HDMI output, and only then enable the DISPC output. However, as this is
>>> only based on observations, the fix may not be perfect as the problem
>>> may lie somewhere else. Nevertheless, HDMI works better with this patch.
>>>
>>> This will also fix the dispc's dispc_mgr_enable_digit_out(), as the code
>>> waits for two VSYNCs after enabling the output. If the HDMI output is
>>> disabled (as it was previously), there are no VSYNCs and
>>> dispc_mgr_enable_digit_out() will print timeout errors.
>>>
>>> Cc: Mythri P K<mythripk@ti.com>
>>> Signed-off-by: Tomi Valkeinen<tomi.valkeinen@ti.com>
>>> ---
>>>    drivers/video/omap2/dss/hdmi.c |    4 ++--
>>>    1 files changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c
>>> index 4752137..06a78b2 100644
>>> --- a/drivers/video/omap2/dss/hdmi.c
>>> +++ b/drivers/video/omap2/dss/hdmi.c
>>> @@ -529,10 +529,10 @@ static int hdmi_power_on(struct omap_dss_device *dssdev)
>>>    	dispc_set_digit_size(dssdev->panel.timings.x_res,
>>>    			dssdev->panel.timings.y_res);
>>>
>>> -	dispc_mgr_enable(OMAP_DSS_CHANNEL_DIGIT, 1);
>>> -
>>>    	hdmi.ip_data.ops->video_enable(&hdmi.ip_data, 1);
>>>
>>> +	dispc_mgr_enable(OMAP_DSS_CHANNEL_DIGIT, 1);
>>> +
>>
>> What content would HDMI push out till the time DIGIT is enabled? It
>> might be interesting to put some milliseconds of delay here and see what
>> happens.
>
> I guess HDMI works independently of DISPC. So HDMI will output whatever
> is in the video port from DISPC to HDMI, regardless of what DISPC does.
> My guess is it's either random bits or, more probably, zeroes.

Okay. I thought HDMI block may probe the video port's DE line or 
something to check if it is pushing out valid content. However, with 
this patch, we are more aligned with what we do in power_off(), i.e 
first disable the manager and then disable HDMI.

Archit

>
> I think this way is the same as used for VENC: first we configure and
> enable VENC, then we enable DISPC digit output.
>
>   Tomi
>
>


^ permalink raw reply

* Re: [PATCHv2 09/15] OMAP: DSS2: HDMI: implement detect()
From: K, Mythri P @ 2011-09-12 13:36 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev, archit
In-Reply-To: <1315818818-18733-10-git-send-email-tomi.valkeinen@ti.com>

Hi Tomi,

On Mon, Sep 12, 2011 at 2:43 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> Implement detect() by checking the hot plug detect status.
>
> The implementation is not very good, as it always turns on the HDMI
> output to get the detection working. HDMI driver needs improvements so
> that we could enable only core parts of it.
>
> Cc: Mythri P K <mythripk@ti.com>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
>  drivers/video/omap2/dss/dss.h             |    1 +
>  drivers/video/omap2/dss/dss_features.c    |    1 +
>  drivers/video/omap2/dss/hdmi.c            |   17 +++++++++++++++++
>  drivers/video/omap2/dss/hdmi_panel.c      |   25 +++++++++++++++++++++++++
>  drivers/video/omap2/dss/ti_hdmi.h         |    3 +++
>  drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c |   12 ++++++++++++
>  6 files changed, 59 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/video/omap2/dss/dss.h b/drivers/video/omap2/dss/dss.h
> index 2e7799c..f58c302 100644
> --- a/drivers/video/omap2/dss/dss.h
> +++ b/drivers/video/omap2/dss/dss.h
> @@ -495,6 +495,7 @@ void omapdss_hdmi_display_set_timing(struct omap_dss_device *dssdev);
>  int omapdss_hdmi_display_check_timing(struct omap_dss_device *dssdev,
>                                        struct omap_video_timings *timings);
>  int omapdss_hdmi_read_edid(u8 *buf, int len);
> +bool omapdss_hdmi_detect(void);
>  int hdmi_panel_init(void);
>  void hdmi_panel_exit(void);
>
> diff --git a/drivers/video/omap2/dss/dss_features.c b/drivers/video/omap2/dss/dss_features.c
> index 076f399..ab41665 100644
> --- a/drivers/video/omap2/dss/dss_features.c
> +++ b/drivers/video/omap2/dss/dss_features.c
> @@ -440,6 +440,7 @@ static const struct ti_hdmi_ip_ops omap4_hdmi_functions = {
>        .phy_enable             =       ti_hdmi_4xxx_phy_enable,
>        .phy_disable            =       ti_hdmi_4xxx_phy_disable,
>        .read_edid              =       ti_hdmi_4xxx_read_edid,
> +       .detect                 =       ti_hdmi_4xxx_detect,
>        .pll_enable             =       ti_hdmi_4xxx_pll_enable,
>        .pll_disable            =       ti_hdmi_4xxx_pll_disable,
>        .video_enable           =       ti_hdmi_4xxx_wp_video_start,
> diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c
> index fb85ce5..7818670 100644
> --- a/drivers/video/omap2/dss/hdmi.c
> +++ b/drivers/video/omap2/dss/hdmi.c
> @@ -449,6 +449,23 @@ int omapdss_hdmi_read_edid(u8 *buf, int len)
>        return r;
>  }
>
> +bool omapdss_hdmi_detect(void)
> +{
> +       int r;
> +
> +       mutex_lock(&hdmi.lock);
> +
> +       r = hdmi_runtime_get();
> +       BUG_ON(r);
> +
> +       r = hdmi.ip_data.ops->detect(&hdmi.ip_data);
> +
> +       hdmi_runtime_put();
> +       mutex_unlock(&hdmi.lock);
> +
> +       return r = 1;
> +}
> +
>  int omapdss_hdmi_display_enable(struct omap_dss_device *dssdev)
>  {
>        int r = 0;
> diff --git a/drivers/video/omap2/dss/hdmi_panel.c b/drivers/video/omap2/dss/hdmi_panel.c
> index 71aa813..533d5dc 100644
> --- a/drivers/video/omap2/dss/hdmi_panel.c
> +++ b/drivers/video/omap2/dss/hdmi_panel.c
> @@ -25,6 +25,7 @@
>  #include <linux/mutex.h>
>  #include <linux/module.h>
>  #include <video/omapdss.h>
> +#include <linux/slab.h>
>
>  #include "dss.h"
>
> @@ -198,6 +199,29 @@ err:
>        return r;
>  }
>
> +static bool hdmi_detect(struct omap_dss_device *dssdev)
> +{
> +       int r;
> +
> +       mutex_lock(&hdmi.hdmi_lock);
> +
> +       if (dssdev->state != OMAP_DSS_DISPLAY_ACTIVE) {
> +               r = omapdss_hdmi_display_enable(dssdev);
> +               if (r)
> +                       goto err;
> +       }
> +
> +       r = omapdss_hdmi_detect();
> +
> +       if (dssdev->state = OMAP_DSS_DISPLAY_DISABLED ||
> +                       dssdev->state = OMAP_DSS_DISPLAY_SUSPENDED)
> +               omapdss_hdmi_display_disable(dssdev);
> +err:
> +       mutex_unlock(&hdmi.hdmi_lock);
> +
> +       return r;
> +}
> +
>  static struct omap_dss_driver hdmi_driver = {
>        .probe          = hdmi_panel_probe,
>        .remove         = hdmi_panel_remove,
> @@ -209,6 +233,7 @@ static struct omap_dss_driver hdmi_driver = {
>        .set_timings    = hdmi_set_timings,
>        .check_timings  = hdmi_check_timings,
>        .read_edid      = hdmi_read_edid,
> +       .detect         = hdmi_detect,
>        .driver                 = {
>                .name   = "hdmi_panel",
>                .owner  = THIS_MODULE,
> diff --git a/drivers/video/omap2/dss/ti_hdmi.h b/drivers/video/omap2/dss/ti_hdmi.h
> index 390cd85b..d48603c 100644
> --- a/drivers/video/omap2/dss/ti_hdmi.h
> +++ b/drivers/video/omap2/dss/ti_hdmi.h
> @@ -94,6 +94,8 @@ struct ti_hdmi_ip_ops {
>
>        int (*read_edid)(struct hdmi_ip_data *ip_data, u8 *edid, int len);
>
> +       bool (*detect)(struct hdmi_ip_data *ip_data);
> +
>        int (*pll_enable)(struct hdmi_ip_data *ip_data);
>
>        void (*pll_disable)(struct hdmi_ip_data *ip_data);
> @@ -114,6 +116,7 @@ struct hdmi_ip_data {
>  int ti_hdmi_4xxx_phy_enable(struct hdmi_ip_data *ip_data);
>  void ti_hdmi_4xxx_phy_disable(struct hdmi_ip_data *ip_data);
>  int ti_hdmi_4xxx_read_edid(struct hdmi_ip_data *ip_data, u8 *edid, int len);
> +bool ti_hdmi_4xxx_detect(struct hdmi_ip_data *ip_data);
>  void ti_hdmi_4xxx_wp_video_start(struct hdmi_ip_data *ip_data, bool start);
>  int ti_hdmi_4xxx_pll_enable(struct hdmi_ip_data *ip_data);
>  void ti_hdmi_4xxx_pll_disable(struct hdmi_ip_data *ip_data);
> diff --git a/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c b/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c
> index e9885dc..da7fe50 100644
> --- a/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c
> +++ b/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c
> @@ -416,6 +416,18 @@ int ti_hdmi_4xxx_read_edid(struct hdmi_ip_data *ip_data,
>        return l;
>  }
>
> +bool ti_hdmi_4xxx_detect(struct hdmi_ip_data *ip_data)
> +{
> +       int r;
> +
> +       void __iomem *base = hdmi_core_sys_base(ip_data);
> +
> +       /* HPD */
> +       r = REG_GET(base, HDMI_CORE_SYS_SYS_STAT, 1, 1);
> +
> +       return r = 1;
> +}
> +
For HPD the probe should also be on the core interrupt first , and the
detect should be dynamic, ie based on the cable connect and disconnect
event.So this approach for HPD is not really the way.
Also that should be based on the GPIO(63) , I am planning to push a
patch on that shortly.

>  static void hdmi_core_init(struct hdmi_core_video_config *video_cfg,
>                        struct hdmi_core_infoframe_avi *avi_cfg,
>                        struct hdmi_core_packet_enable_repeat *repeat_cfg)
> --
> 1.7.4.1
>
>
Thanks and regards,
Mythri.

^ permalink raw reply

* Re: [PATCHv2 09/15] OMAP: DSS2: HDMI: implement detect()
From: Tomi Valkeinen @ 2011-09-12 16:18 UTC (permalink / raw)
  To: K, Mythri P; +Cc: linux-omap, linux-fbdev, archit
In-Reply-To: <CAP5A+B9_Jsg23oR49q2bkBGVLDWHBiBZj_=_G1SRf7ftrMt_iw@mail.gmail.com>

On Mon, 2011-09-12 at 18:54 +0530, K, Mythri P wrote:
> Hi Tomi,
> 
> On Mon, Sep 12, 2011 at 2:43 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> > Implement detect() by checking the hot plug detect status.
> >
> > The implementation is not very good, as it always turns on the HDMI
> > output to get the detection working. HDMI driver needs improvements so
> > that we could enable only core parts of it.
> >
> > Cc: Mythri P K <mythripk@ti.com>
> > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> > ---

<snip>

> > +bool ti_hdmi_4xxx_detect(struct hdmi_ip_data *ip_data)
> > +{
> > +       int r;
> > +
> > +       void __iomem *base = hdmi_core_sys_base(ip_data);
> > +
> > +       /* HPD */
> > +       r = REG_GET(base, HDMI_CORE_SYS_SYS_STAT, 1, 1);
> > +
> > +       return r = 1;
> > +}
> > +
> For HPD the probe should also be on the core interrupt first , and the
> detect should be dynamic, ie based on the cable connect and disconnect
> event.So this approach for HPD is not really the way.

This is not for the event, this is for polling. There is currently no
hot plug event mechanism in the DSS.

Do we get an interrupt when the driver is loaded and the cable is
already connected? And do you plan to keep the plugged in/out state
stored somewhere, or how do you implement detect()?

> Also that should be based on the GPIO(63) , I am planning to push a
> patch on that shortly.

What is gpio 63? What does the HDMI_CORE_SYS_STAT HDP bit tell us then?

 Tomi



^ permalink raw reply

* Re: [PATCHv2 09/15] OMAP: DSS2: HDMI: implement detect()
From: Rob Clark @ 2011-09-12 16:46 UTC (permalink / raw)
  To: K, Mythri P; +Cc: Tomi Valkeinen, linux-omap, linux-fbdev, archit
In-Reply-To: <CAP5A+B9_Jsg23oR49q2bkBGVLDWHBiBZj_=_G1SRf7ftrMt_iw@mail.gmail.com>

On Mon, Sep 12, 2011 at 8:24 AM, K, Mythri P <mythripk@ti.com> wrote:
>> +bool ti_hdmi_4xxx_detect(struct hdmi_ip_data *ip_data)
>> +{
>> +       int r;
>> +
>> +       void __iomem *base = hdmi_core_sys_base(ip_data);
>> +
>> +       /* HPD */
>> +       r = REG_GET(base, HDMI_CORE_SYS_SYS_STAT, 1, 1);
>> +
>> +       return r = 1;
>> +}
>> +
> For HPD the probe should also be on the core interrupt first , and the
> detect should be dynamic, ie based on the cable connect and disconnect
> event.So this approach for HPD is not really the way.
> Also that should be based on the GPIO(63) , I am planning to push a
> patch on that shortly.


Fwiw, we do still need a dssdrv->detect() function from omapdrm
driver..  if there is another way to implement that function, such as
with a GPIO, that is great.  But somehow or another we need the detect
function.  The implementation can always change later.

BR,
-R

^ permalink raw reply

* [REPOST][PATCH 1/2] fbdev: fix indentation in modedb.c
From: Timur Tabi @ 2011-09-13 16:05 UTC (permalink / raw)
  To: linux-fbdev

Fix the incorrect indentation in functions fb_try_mode() and fb_find_mode().

Signed-off-by: Timur Tabi <timur@freescale.com>
---
 drivers/video/modedb.c |  444 ++++++++++++++++++++++++------------------------
 1 files changed, 225 insertions(+), 219 deletions(-)

diff --git a/drivers/video/modedb.c b/drivers/video/modedb.c
index cb175fe..a9a907c 100644
--- a/drivers/video/modedb.c
+++ b/drivers/video/modedb.c
@@ -491,55 +491,56 @@ EXPORT_SYMBOL(vesa_modes);
 static int fb_try_mode(struct fb_var_screeninfo *var, struct fb_info *info,
 		       const struct fb_videomode *mode, unsigned int bpp)
 {
-    int err = 0;
-
-    DPRINTK("Trying mode %s %dx%d-%d@%d\n", mode->name ? mode->name : "noname",
-	    mode->xres, mode->yres, bpp, mode->refresh);
-    var->xres = mode->xres;
-    var->yres = mode->yres;
-    var->xres_virtual = mode->xres;
-    var->yres_virtual = mode->yres;
-    var->xoffset = 0;
-    var->yoffset = 0;
-    var->bits_per_pixel = bpp;
-    var->activate |= FB_ACTIVATE_TEST;
-    var->pixclock = mode->pixclock;
-    var->left_margin = mode->left_margin;
-    var->right_margin = mode->right_margin;
-    var->upper_margin = mode->upper_margin;
-    var->lower_margin = mode->lower_margin;
-    var->hsync_len = mode->hsync_len;
-    var->vsync_len = mode->vsync_len;
-    var->sync = mode->sync;
-    var->vmode = mode->vmode;
-    if (info->fbops->fb_check_var)
-    	err = info->fbops->fb_check_var(var, info);
-    var->activate &= ~FB_ACTIVATE_TEST;
-    return err;
+	int err = 0;
+
+	DPRINTK("Trying mode %s %dx%d-%d@%d\n",
+		mode->name ? mode->name : "noname",
+		mode->xres, mode->yres, bpp, mode->refresh);
+	var->xres = mode->xres;
+	var->yres = mode->yres;
+	var->xres_virtual = mode->xres;
+	var->yres_virtual = mode->yres;
+	var->xoffset = 0;
+	var->yoffset = 0;
+	var->bits_per_pixel = bpp;
+	var->activate |= FB_ACTIVATE_TEST;
+	var->pixclock = mode->pixclock;
+	var->left_margin = mode->left_margin;
+	var->right_margin = mode->right_margin;
+	var->upper_margin = mode->upper_margin;
+	var->lower_margin = mode->lower_margin;
+	var->hsync_len = mode->hsync_len;
+	var->vsync_len = mode->vsync_len;
+	var->sync = mode->sync;
+	var->vmode = mode->vmode;
+	if (info->fbops->fb_check_var)
+		err = info->fbops->fb_check_var(var, info);
+	var->activate &= ~FB_ACTIVATE_TEST;
+	return err;
 }
 
 /**
- *	fb_find_mode - finds a valid video mode
- *	@var: frame buffer user defined part of display
- *	@info: frame buffer info structure
- *	@mode_option: string video mode to find
- *	@db: video mode database
- *	@dbsize: size of @db
- *	@default_mode: default video mode to fall back to
- *	@default_bpp: default color depth in bits per pixel
+ *     fb_find_mode - finds a valid video mode
+ *     @var: frame buffer user defined part of display
+ *     @info: frame buffer info structure
+ *     @mode_option: string video mode to find
+ *     @db: video mode database
+ *     @dbsize: size of @db
+ *     @default_mode: default video mode to fall back to
+ *     @default_bpp: default color depth in bits per pixel
  *
- *	Finds a suitable video mode, starting with the specified mode
- *	in @mode_option with fallback to @default_mode.  If
- *	@default_mode fails, all modes in the video mode database will
- *	be tried.
+ *     Finds a suitable video mode, starting with the specified mode
+ *     in @mode_option with fallback to @default_mode.  If
+ *     @default_mode fails, all modes in the video mode database will
+ *     be tried.
  *
- *	Valid mode specifiers for @mode_option:
+ *     Valid mode specifiers for @mode_option:
  *
- *	<xres>x<yres>[M][R][-<bpp>][@<refresh>][i][m] or
- *	<name>[-<bpp>][@<refresh>]
+ *     <xres>x<yres>[M][R][-<bpp>][@<refresh>][i][m] or
+ *     <name>[-<bpp>][@<refresh>]
  *
- *	with <xres>, <yres>, <bpp> and <refresh> decimal numbers and
- *	<name> a string.
+ *     with <xres>, <yres>, <bpp> and <refresh> decimal numbers and
+ *     <name> a string.
  *
  *      If 'M' is present after yres (and before refresh/bpp if present),
  *      the function will compute the timings using VESA(tm) Coordinated
@@ -551,12 +552,12 @@ static int fb_try_mode(struct fb_var_screeninfo *var, struct fb_info *info,
  *
  *      1024x768MR-8@60m - Reduced blank with margins at 60Hz.
  *
- *	NOTE: The passed struct @var is _not_ cleared!  This allows you
- *	to supply values for e.g. the grayscale and accel_flags fields.
+ *     NOTE: The passed struct @var is _not_ cleared!  This allows you
+ *     to supply values for e.g. the grayscale and accel_flags fields.
  *
- *	Returns zero for failure, 1 if using specified @mode_option,
- *	2 if using specified @mode_option with an ignored refresh rate,
- *	3 if default mode is used, 4 if fall back to any valid mode.
+ *     Returns zero for failure, 1 if using specified @mode_option,
+ *     2 if using specified @mode_option with an ignored refresh rate,
+ *     3 if default mode is used, 4 if fall back to any valid mode.
  *
  */
 
@@ -566,198 +567,203 @@ int fb_find_mode(struct fb_var_screeninfo *var,
 		 const struct fb_videomode *default_mode,
 		 unsigned int default_bpp)
 {
-    int i;
-
-    /* Set up defaults */
-    if (!db) {
-	db = modedb;
-	dbsize = ARRAY_SIZE(modedb);
-    }
-
-    if (!default_mode)
-	default_mode = &db[0];
-
-    if (!default_bpp)
-	default_bpp = 8;
-
-    /* Did the user specify a video mode? */
-    if (!mode_option)
-	mode_option = fb_mode_option;
-    if (mode_option) {
-	const char *name = mode_option;
-	unsigned int namelen = strlen(name);
-	int res_specified = 0, bpp_specified = 0, refresh_specified = 0;
-	unsigned int xres = 0, yres = 0, bpp = default_bpp, refresh = 0;
-	int yres_specified = 0, cvt = 0, rb = 0, interlace = 0, margins = 0;
-	u32 best, diff, tdiff;
-
-	for (i = namelen-1; i >= 0; i--) {
-	    switch (name[i]) {
-		case '@':
-		    namelen = i;
-		    if (!refresh_specified && !bpp_specified &&
-			!yres_specified) {
-			refresh = simple_strtol(&name[i+1], NULL, 10);
-			refresh_specified = 1;
-			if (cvt || rb)
-			    cvt = 0;
-		    } else
-			goto done;
-		    break;
-		case '-':
-		    namelen = i;
-		    if (!bpp_specified && !yres_specified) {
-			bpp = simple_strtol(&name[i+1], NULL, 10);
-			bpp_specified = 1;
-			if (cvt || rb)
-			    cvt = 0;
-		    } else
-			goto done;
-		    break;
-		case 'x':
-		    if (!yres_specified) {
-			yres = simple_strtol(&name[i+1], NULL, 10);
-			yres_specified = 1;
-		    } else
-			goto done;
-		    break;
-		case '0' ... '9':
-		    break;
-		case 'M':
-		    if (!yres_specified)
-			cvt = 1;
-		    break;
-		case 'R':
-		    if (!cvt)
-			rb = 1;
-		    break;
-		case 'm':
-		    if (!cvt)
-			margins = 1;
-		    break;
-		case 'i':
-		    if (!cvt)
-			interlace = 1;
-		    break;
-		default:
-		    goto done;
-	    }
-	}
-	if (i < 0 && yres_specified) {
-	    xres = simple_strtol(name, NULL, 10);
-	    res_specified = 1;
-	}
-done:
-	if (cvt) {
-	    struct fb_videomode cvt_mode;
-	    int ret;
-
-	    DPRINTK("CVT mode %dx%d@%dHz%s%s%s\n", xres, yres,
-		    (refresh) ? refresh : 60, (rb) ? " reduced blanking" :
-		    "", (margins) ? " with margins" : "", (interlace) ?
-		    " interlaced" : "");
-
-	    memset(&cvt_mode, 0, sizeof(cvt_mode));
-	    cvt_mode.xres = xres;
-	    cvt_mode.yres = yres;
-	    cvt_mode.refresh = (refresh) ? refresh : 60;
+	int i;
 
-	    if (interlace)
-		cvt_mode.vmode |= FB_VMODE_INTERLACED;
-	    else
-		cvt_mode.vmode &= ~FB_VMODE_INTERLACED;
+	/* Set up defaults */
+	if (!db) {
+		db = modedb;
+		dbsize = ARRAY_SIZE(modedb);
+	}
 
-	    ret = fb_find_mode_cvt(&cvt_mode, margins, rb);
+	if (!default_mode)
+		default_mode = &db[0];
+
+	if (!default_bpp)
+		default_bpp = 8;
+
+	/* Did the user specify a video mode? */
+	if (!mode_option)
+		mode_option = fb_mode_option;
+	if (mode_option) {
+		const char *name = mode_option;
+		unsigned int namelen = strlen(name);
+		int res_specified = 0, bpp_specified = 0, refresh_specified = 0;
+		unsigned int xres = 0, yres = 0, bpp = default_bpp, refresh = 0;
+		int yres_specified = 0, cvt = 0, rb = 0, interlace = 0;
+		int margins = 0;
+		u32 best, diff, tdiff;
+
+		for (i = namelen-1; i >= 0; i--) {
+			switch (name[i]) {
+			case '@':
+				namelen = i;
+				if (!refresh_specified && !bpp_specified &&
+				    !yres_specified) {
+					refresh = simple_strtol(&name[i+1], NULL,
+								10);
+					refresh_specified = 1;
+					if (cvt || rb)
+						cvt = 0;
+				} else
+					goto done;
+				break;
+			case '-':
+				namelen = i;
+				if (!bpp_specified && !yres_specified) {
+					bpp = simple_strtol(&name[i+1], NULL,
+							    10);
+					bpp_specified = 1;
+					if (cvt || rb)
+						cvt = 0;
+				} else
+					goto done;
+				break;
+			case 'x':
+				if (!yres_specified) {
+					yres = simple_strtol(&name[i+1], NULL,
+							     10);
+					yres_specified = 1;
+				} else
+					goto done;
+				break;
+			case '0' ... '9':
+				break;
+			case 'M':
+				if (!yres_specified)
+					cvt = 1;
+				break;
+			case 'R':
+				if (!cvt)
+					rb = 1;
+				break;
+			case 'm':
+				if (!cvt)
+					margins = 1;
+				break;
+			case 'i':
+				if (!cvt)
+					interlace = 1;
+				break;
+			default:
+				goto done;
+			}
+		}
+		if (i < 0 && yres_specified) {
+			xres = simple_strtol(name, NULL, 10);
+			res_specified = 1;
+		}
+done:
+		if (cvt) {
+			struct fb_videomode cvt_mode;
+			int ret;
+
+			DPRINTK("CVT mode %dx%d@%dHz%s%s%s\n", xres, yres,
+				(refresh) ? refresh : 60,
+				(rb) ? " reduced blanking" : "",
+				(margins) ? " with margins" : "",
+				(interlace) ? " interlaced" : "");
+
+			memset(&cvt_mode, 0, sizeof(cvt_mode));
+			cvt_mode.xres = xres;
+			cvt_mode.yres = yres;
+			cvt_mode.refresh = (refresh) ? refresh : 60;
+
+			if (interlace)
+				cvt_mode.vmode |= FB_VMODE_INTERLACED;
+			else
+				cvt_mode.vmode &= ~FB_VMODE_INTERLACED;
+
+			ret = fb_find_mode_cvt(&cvt_mode, margins, rb);
+
+			if (!ret && !fb_try_mode(var, info, &cvt_mode, bpp)) {
+				DPRINTK("modedb CVT: CVT mode ok\n");
+				return 1;
+			}
 
-	    if (!ret && !fb_try_mode(var, info, &cvt_mode, bpp)) {
-		DPRINTK("modedb CVT: CVT mode ok\n");
-		return 1;
-	    }
+			DPRINTK("CVT mode invalid, getting mode from database\n");
+		}
 
-	    DPRINTK("CVT mode invalid, getting mode from database\n");
-	}
+		DPRINTK("Trying specified video mode%s %ix%i\n",
+			refresh_specified ? "" : " (ignoring refresh rate)",
+			xres, yres);
 
-	DPRINTK("Trying specified video mode%s %ix%i\n",
-	    refresh_specified ? "" : " (ignoring refresh rate)", xres, yres);
-
-	if (!refresh_specified) {
-		/*
-		 * If the caller has provided a custom mode database and a
-		 * valid monspecs structure, we look for the mode with the
-		 * highest refresh rate.  Otherwise we play it safe it and
-		 * try to find a mode with a refresh rate closest to the
-		 * standard 60 Hz.
-		 */
-		if (db != modedb &&
-		    info->monspecs.vfmin && info->monspecs.vfmax &&
-		    info->monspecs.hfmin && info->monspecs.hfmax &&
-		    info->monspecs.dclkmax) {
-			refresh = 1000;
-		} else {
-			refresh = 60;
+		if (!refresh_specified) {
+			/*
+			 * If the caller has provided a custom mode database and
+			 * a valid monspecs structure, we look for the mode with
+			 * the highest refresh rate.  Otherwise we play it safe
+			 * it and try to find a mode with a refresh rate closest
+			 * to the standard 60 Hz.
+			 */
+			if (db != modedb &&
+			    info->monspecs.vfmin && info->monspecs.vfmax &&
+			    info->monspecs.hfmin && info->monspecs.hfmax &&
+			    info->monspecs.dclkmax) {
+				refresh = 1000;
+			} else {
+				refresh = 60;
+			}
 		}
-	}
 
-	diff = -1;
-	best = -1;
-	for (i = 0; i < dbsize; i++) {
-		if ((name_matches(db[i], name, namelen) ||
-		    (res_specified && res_matches(db[i], xres, yres))) &&
-		    !fb_try_mode(var, info, &db[i], bpp)) {
-			if (refresh_specified && db[i].refresh = refresh) {
-				return 1;
-			} else {
+		diff = -1;
+		best = -1;
+		for (i = 0; i < dbsize; i++) {
+			if ((name_matches(db[i], name, namelen) ||
+			     (res_specified && res_matches(db[i], xres, yres))) &&
+			    !fb_try_mode(var, info, &db[i], bpp)) {
+				if (refresh_specified && db[i].refresh = refresh)
+					return 1;
+
 				if (abs(db[i].refresh - refresh) < diff) {
 					diff = abs(db[i].refresh - refresh);
 					best = i;
 				}
 			}
 		}
-	}
-	if (best != -1) {
-		fb_try_mode(var, info, &db[best], bpp);
-		return (refresh_specified) ? 2 : 1;
-	}
-
-	diff = 2 * (xres + yres);
-	best = -1;
-	DPRINTK("Trying best-fit modes\n");
-	for (i = 0; i < dbsize; i++) {
-		DPRINTK("Trying %ix%i\n", db[i].xres, db[i].yres);
-		if (!fb_try_mode(var, info, &db[i], bpp)) {
-			tdiff = abs(db[i].xres - xres) +
-				abs(db[i].yres - yres);
-
-			/*
-			 * Penalize modes with resolutions smaller
-			 * than requested.
-			 */
-			if (xres > db[i].xres || yres > db[i].yres)
-				tdiff += xres + yres;
+		if (best != -1) {
+			fb_try_mode(var, info, &db[best], bpp);
+			return (refresh_specified) ? 2 : 1;
+		}
 
-			if (diff > tdiff) {
-				diff = tdiff;
-				best = i;
+		diff = 2 * (xres + yres);
+		best = -1;
+		DPRINTK("Trying best-fit modes\n");
+		for (i = 0; i < dbsize; i++) {
+			DPRINTK("Trying %ix%i\n", db[i].xres, db[i].yres);
+			if (!fb_try_mode(var, info, &db[i], bpp)) {
+				tdiff = abs(db[i].xres - xres) +
+					abs(db[i].yres - yres);
+
+				/*
+				 * Penalize modes with resolutions smaller
+				 * than requested.
+				 */
+				if (xres > db[i].xres || yres > db[i].yres)
+					tdiff += xres + yres;
+
+				if (diff > tdiff) {
+					diff = tdiff;
+					best = i;
+				}
 			}
 		}
+		if (best != -1) {
+			fb_try_mode(var, info, &db[best], bpp);
+			return 5;
+		}
 	}
-	if (best != -1) {
-	    fb_try_mode(var, info, &db[best], bpp);
-	    return 5;
-	}
-    }
 
-    DPRINTK("Trying default video mode\n");
-    if (!fb_try_mode(var, info, default_mode, default_bpp))
-	return 3;
+	DPRINTK("Trying default video mode\n");
+	if (!fb_try_mode(var, info, default_mode, default_bpp))
+		return 3;
 
-    DPRINTK("Trying all modes\n");
-    for (i = 0; i < dbsize; i++)
-	if (!fb_try_mode(var, info, &db[i], default_bpp))
-	    return 4;
+	DPRINTK("Trying all modes\n");
+	for (i = 0; i < dbsize; i++)
+		if (!fb_try_mode(var, info, &db[i], default_bpp))
+			return 4;
 
-    DPRINTK("No valid mode found\n");
-    return 0;
+	DPRINTK("No valid mode found\n");
+	return 0;
 }
 
 /**
-- 
1.7.3.4



^ permalink raw reply related

* [REPOST][PATCH 2/2] video: miscellaneous minor changes to the Freescale DIU driver
From: Timur Tabi @ 2011-09-13 16:05 UTC (permalink / raw)
  To: linux-fbdev

Make several minor, miscellaneous changes to the Freescale DIU framebuffer
driver.  These changes "lighten" the code by removing crud, fixing small
bugs, and fixing some coding style problems.  These changes will make it
easier to make more substantial fixes in the future.

1. Fix incorrect indentation and spacing with some code.
2. Remove debug printks (they don't actually help in debugging the code).
3. Clean up some other printks (e.g. use pr_xxx, clean up the text, etc).
4. Remove the "default" videomode object since it's just a dupe of the
   first element in the videomode array.
5. Remove some superfluous local variables.
6. Rename ofdev to pdev, since it's a platform device not an OF device.
7. Fix some device tree operations.
8. Fix some build warnings.
9. Removed some unused structures from the header file.
10. Other minor bug fixes and changes.

Signed-off-by: Timur Tabi <timur@freescale.com>
---
 drivers/video/fsl-diu-fb.c |  362 +++++++++++++++-----------------------------
 include/linux/fsl-diu-fb.h |   12 +--
 2 files changed, 125 insertions(+), 249 deletions(-)

diff --git a/drivers/video/fsl-diu-fb.c b/drivers/video/fsl-diu-fb.c
index 0f1933b..1470581 100644
--- a/drivers/video/fsl-diu-fb.c
+++ b/drivers/video/fsl-diu-fb.c
@@ -36,26 +36,10 @@
 #include "edid.h"
 
 /*
- * These parameters give default parameters
- * for video output 1024x768,
- * FIXME - change timing to proper amounts
- * hsync 31.5kHz, vsync 60Hz
+ * List of supported video modes
+ *
+ * The first entry is the default video mode
  */
-static struct fb_videomode __devinitdata fsl_diu_default_mode = {
-	.refresh	= 60,
-	.xres		= 1024,
-	.yres		= 768,
-	.pixclock	= 15385,
-	.left_margin	= 160,
-	.right_margin	= 24,
-	.upper_margin	= 29,
-	.lower_margin	= 3,
-	.hsync_len	= 136,
-	.vsync_len	= 6,
-	.sync		= FB_SYNC_COMP_HIGH_ACT | FB_SYNC_VERT_HIGH_ACT,
-	.vmode		= FB_VMODE_NONINTERLACED
-};
-
 static struct fb_videomode __devinitdata fsl_diu_mode_db[] = {
 	{
 		.name		= "1024x768-60",
@@ -217,59 +201,59 @@ struct mfb_info {
 	int x_aoi_d;		/* aoi display x offset to physical screen */
 	int y_aoi_d;		/* aoi display y offset to physical screen */
 	struct fsl_diu_data *parent;
-	u8 *edid_data;
+	void *edid_data;
 };
 
 
 static struct mfb_info mfb_template[] = {
 	{		/* AOI 0 for plane 0 */
-	.index = 0,
-	.type = MFB_TYPE_OUTPUT,
-	.id = "Panel0",
-	.registered = 0,
-	.count = 0,
-	.x_aoi_d = 0,
-	.y_aoi_d = 0,
+		.index = 0,
+		.type = MFB_TYPE_OUTPUT,
+		.id = "Panel0",
+		.registered = 0,
+		.count = 0,
+		.x_aoi_d = 0,
+		.y_aoi_d = 0,
 	},
 	{		/* AOI 0 for plane 1 */
-	.index = 1,
-	.type = MFB_TYPE_OUTPUT,
-	.id = "Panel1 AOI0",
-	.registered = 0,
-	.g_alpha = 0xff,
-	.count = 0,
-	.x_aoi_d = 0,
-	.y_aoi_d = 0,
+		.index = 1,
+		.type = MFB_TYPE_OUTPUT,
+		.id = "Panel1 AOI0",
+		.registered = 0,
+		.g_alpha = 0xff,
+		.count = 0,
+		.x_aoi_d = 0,
+		.y_aoi_d = 0,
 	},
 	{		/* AOI 1 for plane 1 */
-	.index = 2,
-	.type = MFB_TYPE_OUTPUT,
-	.id = "Panel1 AOI1",
-	.registered = 0,
-	.g_alpha = 0xff,
-	.count = 0,
-	.x_aoi_d = 0,
-	.y_aoi_d = 480,
+		.index = 2,
+		.type = MFB_TYPE_OUTPUT,
+		.id = "Panel1 AOI1",
+		.registered = 0,
+		.g_alpha = 0xff,
+		.count = 0,
+		.x_aoi_d = 0,
+		.y_aoi_d = 480,
 	},
 	{		/* AOI 0 for plane 2 */
-	.index = 3,
-	.type = MFB_TYPE_OUTPUT,
-	.id = "Panel2 AOI0",
-	.registered = 0,
-	.g_alpha = 0xff,
-	.count = 0,
-	.x_aoi_d = 640,
-	.y_aoi_d = 0,
+		.index = 3,
+		.type = MFB_TYPE_OUTPUT,
+		.id = "Panel2 AOI0",
+		.registered = 0,
+		.g_alpha = 0xff,
+		.count = 0,
+		.x_aoi_d = 640,
+		.y_aoi_d = 0,
 	},
 	{		/* AOI 1 for plane 2 */
-	.index = 4,
-	.type = MFB_TYPE_OUTPUT,
-	.id = "Panel2 AOI1",
-	.registered = 0,
-	.g_alpha = 0xff,
-	.count = 0,
-	.x_aoi_d = 640,
-	.y_aoi_d = 480,
+		.index = 4,
+		.type = MFB_TYPE_OUTPUT,
+		.id = "Panel2 AOI1",
+		.registered = 0,
+		.g_alpha = 0xff,
+		.count = 0,
+		.x_aoi_d = 640,
+		.y_aoi_d = 480,
 	},
 };
 
@@ -322,14 +306,9 @@ static void *fsl_diu_alloc(size_t size, phys_addr_t *phys)
 {
 	void *virt;
 
-	pr_debug("size=%zu\n", size);
-
 	virt = alloc_pages_exact(size, GFP_DMA | __GFP_ZERO);
-	if (virt) {
+	if (virt)
 		*phys = virt_to_phys(virt);
-		pr_debug("virt=%p phys=%llx\n", virt,
-			(unsigned long long)*phys);
-	}
 
 	return virt;
 }
@@ -343,8 +322,6 @@ static void *fsl_diu_alloc(size_t size, phys_addr_t *phys)
  */
 static void fsl_diu_free(void *virt, size_t size)
 {
-	pr_debug("virt=%p size=%zu\n", virt, size);
-
 	if (virt && size)
 		free_pages_exact(virt, size);
 }
@@ -368,7 +345,6 @@ static int fsl_diu_enable_panel(struct fb_info *info)
 	struct fsl_diu_data *machine_data = mfbi->parent;
 	int res = 0;
 
-	pr_debug("enable_panel index %d\n", mfbi->index);
 	if (mfbi->type != MFB_TYPE_OFF) {
 		switch (mfbi->index) {
 		case 0:				/* plane 0 */
@@ -585,9 +561,6 @@ static void adjust_aoi_size_position(struct fb_var_screeninfo *var,
 static int fsl_diu_check_var(struct fb_var_screeninfo *var,
 				struct fb_info *info)
 {
-	pr_debug("check_var xres: %d\n", var->xres);
-	pr_debug("check_var yres: %d\n", var->yres);
-
 	if (var->xres_virtual < var->xres)
 		var->xres_virtual = var->xres;
 	if (var->yres_virtual < var->yres)
@@ -682,7 +655,7 @@ static void set_fix(struct fb_info *info)
 	struct fb_var_screeninfo *var = &info->var;
 	struct mfb_info *mfbi = info->par;
 
-	strncpy(fix->id, mfbi->id, strlen(mfbi->id));
+	strncpy(fix->id, mfbi->id, sizeof(fix->id));
 	fix->line_length = var->xres_virtual * var->bits_per_pixel / 8;
 	fix->type = FB_TYPE_PACKED_PIXELS;
 	fix->accel = FB_ACCEL_NONE;
@@ -715,12 +688,11 @@ static void update_lcdc(struct fb_info *info)
 	/* Prep for DIU init  - gamma table, cursor table */
 
 	for (i = 0; i <= 2; i++)
-	   for (j = 0; j <= 255; j++)
-	      *gamma_table_base++ = j;
+		for (j = 0; j <= 255; j++)
+			*gamma_table_base++ = j;
 
 	diu_ops.set_gamma_table(machine_data->monitor_port, pool.gamma.vaddr);
 
-	pr_debug("update-lcdc: HW - %p\n Disabling DIU\n", hw);
 	disable_lcdc(info);
 
 	/* Program DIU registers */
@@ -732,9 +704,6 @@ static void update_lcdc(struct fb_info *info)
 	out_be32(&hw->bgnd_wb, 0); 		/* BGND_WB */
 	out_be32(&hw->disp_size, (var->yres << 16 | var->xres));
 						/* DISP SIZE */
-	pr_debug("DIU xres: %d\n", var->xres);
-	pr_debug("DIU yres: %d\n", var->yres);
-
 	out_be32(&hw->wb_size, 0); /* WB SIZE */
 	out_be32(&hw->wb_mem_addr, 0); /* WB MEM ADDR */
 
@@ -751,15 +720,6 @@ static void update_lcdc(struct fb_info *info)
 
 	out_be32(&hw->vsyn_para, temp);
 
-	pr_debug("DIU right_margin - %d\n", var->right_margin);
-	pr_debug("DIU left_margin - %d\n", var->left_margin);
-	pr_debug("DIU hsync_len - %d\n", var->hsync_len);
-	pr_debug("DIU upper_margin - %d\n", var->upper_margin);
-	pr_debug("DIU lower_margin - %d\n", var->lower_margin);
-	pr_debug("DIU vsync_len - %d\n", var->vsync_len);
-	pr_debug("DIU HSYNC - 0x%08x\n", hw->hsyn_para);
-	pr_debug("DIU VSYNC - 0x%08x\n", hw->vsyn_para);
-
 	diu_ops.set_pixel_clock(var->pixclock);
 
 	out_be32(&hw->syn_pol, 0);	/* SYNC SIGNALS POLARITY */
@@ -776,14 +736,9 @@ static int map_video_memory(struct fb_info *info)
 	phys_addr_t phys;
 	u32 smem_len = info->fix.line_length * info->var.yres_virtual;
 
-	pr_debug("info->var.xres_virtual = %d\n", info->var.xres_virtual);
-	pr_debug("info->var.yres_virtual = %d\n", info->var.yres_virtual);
-	pr_debug("info->fix.line_length  = %d\n", info->fix.line_length);
-	pr_debug("MAP_VIDEO_MEMORY: smem_len = %u\n", smem_len);
-
 	info->screen_base = fsl_diu_alloc(smem_len, &phys);
 	if (info->screen_base = NULL) {
-		printk(KERN_ERR "Unable to allocate fb memory\n");
+		dev_err(info->dev, "unable to allocate fb memory\n");
 		return -ENOMEM;
 	}
 	mutex_lock(&info->mm_lock);
@@ -792,10 +747,6 @@ static int map_video_memory(struct fb_info *info)
 	mutex_unlock(&info->mm_lock);
 	info->screen_size = info->fix.smem_len;
 
-	pr_debug("Allocated fb @ paddr=0x%08lx, size=%d.\n",
-		 info->fix.smem_start, info->fix.smem_len);
-	pr_debug("screen base %p\n", info->screen_base);
-
 	return 0;
 }
 
@@ -852,11 +803,10 @@ static int fsl_diu_set_par(struct fb_info *info)
 	if (len != info->fix.smem_len) {
 		if (info->fix.smem_start)
 			unmap_video_memory(info);
-		pr_debug("SET PAR: smem_len = %d\n", info->fix.smem_len);
 
 		/* Memory allocation for framebuffer */
 		if (map_video_memory(info)) {
-			printk(KERN_ERR "Unable to allocate fb memory 1\n");
+			dev_err(info->dev, "unable to allocate fb memory 1\n");
 			return -ENOMEM;
 		}
 	}
@@ -887,7 +837,7 @@ static int fsl_diu_set_par(struct fb_info *info)
 
 static inline __u32 CNVT_TOHW(__u32 val, __u32 width)
 {
-	return ((val<<width) + 0x7FFF - val)>>16;
+	return ((val << width) + 0x7FFF - val) >> 16;
 }
 
 /*
@@ -899,8 +849,9 @@ static inline __u32 CNVT_TOHW(__u32 val, __u32 width)
  * pseudo_palette in struct fb_info. For pseudocolor mode we have a limited
  * color palette.
  */
-static int fsl_diu_setcolreg(unsigned regno, unsigned red, unsigned green,
-			   unsigned blue, unsigned transp, struct fb_info *info)
+static int fsl_diu_setcolreg(unsigned int regno, unsigned int red,
+			     unsigned int green, unsigned int blue,
+			     unsigned int transp, struct fb_info *info)
 {
 	int ret = 1;
 
@@ -935,9 +886,6 @@ static int fsl_diu_setcolreg(unsigned regno, unsigned red, unsigned green,
 			ret = 0;
 		}
 		break;
-	case FB_VISUAL_STATIC_PSEUDOCOLOR:
-	case FB_VISUAL_PSEUDOCOLOR:
-		break;
 	}
 
 	return ret;
@@ -1022,21 +970,17 @@ static int fsl_diu_ioctl(struct fb_info *info, unsigned int cmd,
 		if (copy_from_user(&pix_fmt, buf, sizeof(pix_fmt)))
 			return -EFAULT;
 		ad->pix_fmt = pix_fmt;
-		pr_debug("Set pixel format to 0x%08x\n", ad->pix_fmt);
 		break;
 	case MFB_GET_PIXFMT:
 		pix_fmt = ad->pix_fmt;
 		if (copy_to_user(buf, &pix_fmt, sizeof(pix_fmt)))
 			return -EFAULT;
-		pr_debug("get pixel format 0x%08x\n", ad->pix_fmt);
 		break;
 	case MFB_SET_AOID:
 		if (copy_from_user(&aoi_d, buf, sizeof(aoi_d)))
 			return -EFAULT;
 		mfbi->x_aoi_d = aoi_d.x_aoi_d;
 		mfbi->y_aoi_d = aoi_d.y_aoi_d;
-		pr_debug("set AOI display offset of index %d to (%d,%d)\n",
-				 mfbi->index, aoi_d.x_aoi_d, aoi_d.y_aoi_d);
 		fsl_diu_check_var(&info->var, info);
 		fsl_diu_set_aoi(info);
 		break;
@@ -1045,14 +989,11 @@ static int fsl_diu_ioctl(struct fb_info *info, unsigned int cmd,
 		aoi_d.y_aoi_d = mfbi->y_aoi_d;
 		if (copy_to_user(buf, &aoi_d, sizeof(aoi_d)))
 			return -EFAULT;
-		pr_debug("get AOI display offset of index %d (%d,%d)\n",
-				mfbi->index, aoi_d.x_aoi_d, aoi_d.y_aoi_d);
 		break;
 	case MFB_GET_ALPHA:
 		global_alpha = mfbi->g_alpha;
 		if (copy_to_user(buf, &global_alpha, sizeof(global_alpha)))
 			return -EFAULT;
-		pr_debug("get global alpha of index %d\n", mfbi->index);
 		break;
 	case MFB_SET_ALPHA:
 		/* set panel information */
@@ -1061,7 +1002,6 @@ static int fsl_diu_ioctl(struct fb_info *info, unsigned int cmd,
 		ad->src_size_g_alpha = (ad->src_size_g_alpha & (~0xff)) |
 							(global_alpha & 0xff);
 		mfbi->g_alpha = global_alpha;
-		pr_debug("set global alpha for index %d\n", mfbi->index);
 		break;
 	case MFB_SET_CHROMA_KEY:
 		/* set panel winformation */
@@ -1089,7 +1029,6 @@ static int fsl_diu_ioctl(struct fb_info *info, unsigned int cmd,
 			ad->ckmin_g = ck.green_min;
 			ad->ckmin_b = ck.blue_min;
 		}
-		pr_debug("set chroma key\n");
 		break;
 	case FBIOGET_GWINFO:
 		if (mfbi->type = MFB_TYPE_OFF)
@@ -1098,18 +1037,9 @@ static int fsl_diu_ioctl(struct fb_info *info, unsigned int cmd,
 		if (copy_to_user(buf, ad, sizeof(*ad)))
 			return -EFAULT;
 		break;
-	case FBIOGET_HWCINFO:
-		pr_debug("FBIOGET_HWCINFO:0x%08x\n", FBIOGET_HWCINFO);
-		break;
-	case FBIOPUT_MODEINFO:
-		pr_debug("FBIOPUT_MODEINFO:0x%08x\n", FBIOPUT_MODEINFO);
-		break;
-	case FBIOGET_DISPINFO:
-		pr_debug("FBIOGET_DISPINFO:0x%08x\n", FBIOGET_DISPINFO);
-		break;
 
 	default:
-		printk(KERN_ERR "Unknown ioctl command (0x%08X)\n", cmd);
+		dev_err(info->dev, "unknown ioctl command (0x%08X)\n", cmd);
 		return -ENOIOCTLCMD;
 	}
 
@@ -1130,7 +1060,6 @@ static int fsl_diu_open(struct fb_info *info, int user)
 	spin_lock(&diu_lock);
 	mfbi->count++;
 	if (mfbi->count = 1) {
-		pr_debug("open plane index %d\n", mfbi->index);
 		fsl_diu_check_var(&info->var, info);
 		res = fsl_diu_set_par(info);
 		if (res < 0)
@@ -1156,7 +1085,6 @@ static int fsl_diu_release(struct fb_info *info, int user)
 	spin_lock(&diu_lock);
 	mfbi->count--;
 	if (mfbi->count = 0) {
-		pr_debug("release plane index %d\n", mfbi->index);
 		res = fsl_diu_disable_panel(info);
 		if (res < 0)
 			mfbi->count++;
@@ -1221,26 +1149,9 @@ static int __devinit install_fb(struct fb_info *info)
 	} else {
 		aoi_mode = init_aoi_mode;
 	}
-	pr_debug("mode used = %s\n", aoi_mode);
-	rc = fb_find_mode(&info->var, info, aoi_mode, db, dbsize,
-			  &fsl_diu_default_mode, default_bpp);
-	switch (rc) {
-	case 1:
-		pr_debug("using mode specified in @mode\n");
-		break;
-	case 2:
-		pr_debug("using mode specified in @mode "
-			"with ignored refresh rate\n");
-		break;
-	case 3:
-		pr_debug("using mode default mode\n");
-		break;
-	case 4:
-		pr_debug("using mode from list\n");
-		break;
-	default:
-		pr_debug("rc = %d\n", rc);
-		pr_debug("failed to find mode\n");
+	rc = fb_find_mode(&info->var, info, aoi_mode, db, dbsize, NULL,
+			  default_bpp);
+	if (!rc) {
 		/*
 		 * For plane 0 we continue and look into
 		 * driver's internal modedb.
@@ -1249,15 +1160,12 @@ static int __devinit install_fb(struct fb_info *info)
 			has_default_mode = 0;
 		else
 			return -EINVAL;
-		break;
 	}
 
 	if (!has_default_mode) {
 		rc = fb_find_mode(&info->var, info, aoi_mode, fsl_diu_mode_db,
-				  ARRAY_SIZE(fsl_diu_mode_db),
-				  &fsl_diu_default_mode,
-				  default_bpp);
-		if (rc > 0 && rc < 5)
+			ARRAY_SIZE(fsl_diu_mode_db), NULL, default_bpp);
+		if (rc)
 			has_default_mode = 1;
 	}
 
@@ -1285,33 +1193,27 @@ static int __devinit install_fb(struct fb_info *info)
 		fb_videomode_to_var(&info->var, modedb);
 	}
 
-	pr_debug("xres_virtual %d\n", info->var.xres_virtual);
-	pr_debug("bits_per_pixel %d\n", info->var.bits_per_pixel);
-
-	pr_debug("info->var.yres_virtual = %d\n", info->var.yres_virtual);
-	pr_debug("info->fix.line_length = %d\n", info->fix.line_length);
-
 	if (mfbi->type = MFB_TYPE_OFF)
 		mfbi->blank = FB_BLANK_NORMAL;
 	else
 		mfbi->blank = FB_BLANK_UNBLANK;
 
 	if (fsl_diu_check_var(&info->var, info)) {
-		printk(KERN_ERR "fb_check_var failed");
+		dev_err(info->dev, "fsl_diu_check_var failed\n");
+		unmap_video_memory(info);
 		fb_dealloc_cmap(&info->cmap);
 		return -EINVAL;
 	}
 
 	if (register_framebuffer(info) < 0) {
-		printk(KERN_ERR "register_framebuffer failed");
+		dev_err(info->dev, "register_framebuffer failed\n");
 		unmap_video_memory(info);
 		fb_dealloc_cmap(&info->cmap);
 		return -EINVAL;
 	}
 
 	mfbi->registered = 1;
-	printk(KERN_INFO "fb%d: %s fb device registered successfully.\n",
-		 info->node, info->fix.id);
+	dev_info(info->dev, "%s registered successfully\n", mfbi->id);
 
 	return 0;
 }
@@ -1343,13 +1245,13 @@ static irqreturn_t fsl_diu_isr(int irq, void *dev_id)
 		/* This is the workaround for underrun */
 		if (status & INT_UNDRUN) {
 			out_be32(&hw->diu_mode, 0);
-			pr_debug("Err: DIU occurs underrun!\n");
 			udelay(1);
 			out_be32(&hw->diu_mode, 1);
 		}
 #if defined(CONFIG_NOT_COHERENT_CACHE)
 		else if (status & INT_VSYNC) {
 			unsigned int i;
+
 			for (i = 0; i < coherence_data_size;
 				i += d_cache_line_size)
 				__asm__ __volatile__ (
@@ -1364,30 +1266,30 @@ static irqreturn_t fsl_diu_isr(int irq, void *dev_id)
 
 static int request_irq_local(int irq)
 {
-	unsigned long status, ints;
+	u32 ints;
 	struct diu *hw;
 	int ret;
 
 	hw = dr.diu_reg;
 
 	/* Read to clear the status */
-	status = in_be32(&hw->int_status);
+	in_be32(&hw->int_status);
 
-	ret = request_irq(irq, fsl_diu_isr, 0, "diu", NULL);
-	if (ret)
-		pr_info("Request diu IRQ failed.\n");
-	else {
+	ret = request_irq(irq, fsl_diu_isr, 0, "fsl-diu-fb", NULL);
+	if (!ret) {
 		ints = INT_PARERR | INT_LS_BF_VS;
 #if !defined(CONFIG_NOT_COHERENT_CACHE)
 		ints |=	INT_VSYNC;
 #endif
+
 		if (dr.mode = MFB_MODE2 || dr.mode = MFB_MODE3)
 			ints |= INT_VSYNC_WB;
 
 		/* Read to clear the status */
-		status = in_be32(&hw->int_status);
+		in_be32(&hw->int_status);
 		out_be32(&hw->int_mask, ints);
 	}
+
 	return ret;
 }
 
@@ -1435,34 +1337,31 @@ static int fsl_diu_resume(struct platform_device *ofdev)
 static int allocate_buf(struct device *dev, struct diu_addr *buf, u32 size,
 			u32 bytes_align)
 {
-	u32 offset, ssize;
-	u32 mask;
-	dma_addr_t paddr = 0;
+	u32 offset;
+	dma_addr_t mask;
 
-	ssize = size + bytes_align;
-	buf->vaddr = dma_alloc_coherent(dev, ssize, &paddr, GFP_DMA |
-							     __GFP_ZERO);
+	buf->vaddr +		dma_alloc_coherent(dev, size + bytes_align, &buf->paddr,
+				   GFP_DMA | __GFP_ZERO);
 	if (!buf->vaddr)
 		return -ENOMEM;
 
-	buf->paddr = (__u32) paddr;
-
 	mask = bytes_align - 1;
-	offset = (u32)buf->paddr & mask;
+	offset = buf->paddr & mask;
 	if (offset) {
 		buf->offset = bytes_align - offset;
-		buf->paddr = (u32)buf->paddr + offset;
+		buf->paddr = buf->paddr + offset;
 	} else
 		buf->offset = 0;
+
 	return 0;
 }
 
 static void free_buf(struct device *dev, struct diu_addr *buf, u32 size,
 		     u32 bytes_align)
 {
-	dma_free_coherent(dev, size + bytes_align,
-				buf->vaddr, (buf->paddr - buf->offset));
-	return;
+	dma_free_coherent(dev, size + bytes_align, buf->vaddr,
+			  buf->paddr - buf->offset);
 }
 
 static ssize_t store_monitor(struct device *device,
@@ -1506,13 +1405,12 @@ static ssize_t show_monitor(struct device *device,
 	return 0;
 }
 
-static int __devinit fsl_diu_probe(struct platform_device *ofdev)
+static int __devinit fsl_diu_probe(struct platform_device *pdev)
 {
-	struct device_node *np = ofdev->dev.of_node;
+	struct device_node *np = pdev->dev.of_node;
 	struct mfb_info *mfbi;
-	phys_addr_t dummy_ad_addr;
+	phys_addr_t dummy_ad_addr = 0;
 	int ret, i, error = 0;
-	struct resource res;
 	struct fsl_diu_data *machine_data;
 	int diu_mode;
 
@@ -1522,9 +1420,9 @@ static int __devinit fsl_diu_probe(struct platform_device *ofdev)
 
 	for (i = 0; i < ARRAY_SIZE(machine_data->fsl_diu_info); i++) {
 		machine_data->fsl_diu_info[i] -			framebuffer_alloc(sizeof(struct mfb_info), &ofdev->dev);
+			framebuffer_alloc(sizeof(struct mfb_info), &pdev->dev);
 		if (!machine_data->fsl_diu_info[i]) {
-			dev_err(&ofdev->dev, "cannot allocate memory\n");
+			dev_err(&pdev->dev, "cannot allocate memory\n");
 			ret = -ENOMEM;
 			goto error2;
 		}
@@ -1544,20 +1442,9 @@ static int __devinit fsl_diu_probe(struct platform_device *ofdev)
 		}
 	}
 
-	ret = of_address_to_resource(np, 0, &res);
-	if (ret) {
-		dev_err(&ofdev->dev, "could not obtain DIU address\n");
-		goto error;
-	}
-	if (!res.start) {
-		dev_err(&ofdev->dev, "invalid DIU address\n");
-		goto error;
-	}
-	dev_dbg(&ofdev->dev, "%s, res.start: 0x%08x\n", __func__, res.start);
-
-	dr.diu_reg = ioremap(res.start, sizeof(struct diu));
+	dr.diu_reg = of_iomap(np, 0);
 	if (!dr.diu_reg) {
-		dev_err(&ofdev->dev, "Err: can't map DIU registers!\n");
+		dev_err(&pdev->dev, "cannot map DIU registers\n");
 		ret = -EFAULT;
 		goto error2;
 	}
@@ -1570,25 +1457,25 @@ static int __devinit fsl_diu_probe(struct platform_device *ofdev)
 	machine_data->irq = irq_of_parse_and_map(np, 0);
 
 	if (!machine_data->irq) {
-		dev_err(&ofdev->dev, "could not get DIU IRQ\n");
+		dev_err(&pdev->dev, "could not get DIU IRQ\n");
 		ret = -EINVAL;
 		goto error;
 	}
 	machine_data->monitor_port = monitor_port;
 
 	/* Area descriptor memory pool aligns to 64-bit boundary */
-	if (allocate_buf(&ofdev->dev, &pool.ad,
+	if (allocate_buf(&pdev->dev, &pool.ad,
 			 sizeof(struct diu_ad) * FSL_AOI_NUM, 8))
 		return -ENOMEM;
 
 	/* Get memory for Gamma Table  - 32-byte aligned memory */
-	if (allocate_buf(&ofdev->dev, &pool.gamma, 768, 32)) {
+	if (allocate_buf(&pdev->dev, &pool.gamma, 768, 32)) {
 		ret = -ENOMEM;
 		goto error;
 	}
 
 	/* For performance, cursor bitmap buffer aligns to 32-byte boundary */
-	if (allocate_buf(&ofdev->dev, &pool.cursor, MAX_CURS * MAX_CURS * 2,
+	if (allocate_buf(&pdev->dev, &pool.cursor, MAX_CURS * MAX_CURS * 2,
 			 32)) {
 		ret = -ENOMEM;
 		goto error;
@@ -1630,16 +1517,13 @@ static int __devinit fsl_diu_probe(struct platform_device *ofdev)
 		mfbi->ad->paddr = pool.ad.paddr + i * sizeof(struct diu_ad);
 		ret = install_fb(machine_data->fsl_diu_info[i]);
 		if (ret) {
-			dev_err(&ofdev->dev,
-				"Failed to register framebuffer %d\n",
-				i);
+			dev_err(&pdev->dev, "could not register fb %d\n", i);
 			goto error;
 		}
 	}
 
 	if (request_irq_local(machine_data->irq)) {
-		dev_err(machine_data->fsl_diu_info[0]->dev,
-			"could not request irq for diu.");
+		dev_err(&pdev->dev, "could not claim irq\n");
 		goto error;
 	}
 
@@ -1651,25 +1535,24 @@ static int __devinit fsl_diu_probe(struct platform_device *ofdev)
 	error = device_create_file(machine_data->fsl_diu_info[0]->dev,
 				  &machine_data->dev_attr);
 	if (error) {
-		dev_err(machine_data->fsl_diu_info[0]->dev,
-			"could not create sysfs %s file\n",
+		dev_err(&pdev->dev, "could not create sysfs file %s\n",
 			machine_data->dev_attr.attr.name);
 	}
 
-	dev_set_drvdata(&ofdev->dev, machine_data);
+	dev_set_drvdata(&pdev->dev, machine_data);
 	return 0;
 
 error:
-	for (i = ARRAY_SIZE(machine_data->fsl_diu_info);
-		i > 0; i--)
-		uninstall_fb(machine_data->fsl_diu_info[i - 1]);
+	for (i = 0; i < ARRAY_SIZE(machine_data->fsl_diu_info); i++)
+		uninstall_fb(machine_data->fsl_diu_info[i]);
+
 	if (pool.ad.vaddr)
-		free_buf(&ofdev->dev, &pool.ad,
+		free_buf(&pdev->dev, &pool.ad,
 			 sizeof(struct diu_ad) * FSL_AOI_NUM, 8);
 	if (pool.gamma.vaddr)
-		free_buf(&ofdev->dev, &pool.gamma, 768, 32);
+		free_buf(&pdev->dev, &pool.gamma, 768, 32);
 	if (pool.cursor.vaddr)
-		free_buf(&ofdev->dev, &pool.cursor, MAX_CURS * MAX_CURS * 2,
+		free_buf(&pdev->dev, &pool.cursor, MAX_CURS * MAX_CURS * 2,
 			 32);
 	if (machine_data->dummy_aoi_virt)
 		fsl_diu_free(machine_data->dummy_aoi_virt, 64);
@@ -1684,25 +1567,23 @@ error2:
 	return ret;
 }
 
-
-static int fsl_diu_remove(struct platform_device *ofdev)
+static int fsl_diu_remove(struct platform_device *pdev)
 {
 	struct fsl_diu_data *machine_data;
 	int i;
 
-	machine_data = dev_get_drvdata(&ofdev->dev);
+	machine_data = dev_get_drvdata(&pdev->dev);
 	disable_lcdc(machine_data->fsl_diu_info[0]);
 	free_irq_local(machine_data->irq);
-	for (i = ARRAY_SIZE(machine_data->fsl_diu_info); i > 0; i--)
-		uninstall_fb(machine_data->fsl_diu_info[i - 1]);
+	for (i = 0; i < ARRAY_SIZE(machine_data->fsl_diu_info); i++)
+		uninstall_fb(machine_data->fsl_diu_info[i]);
 	if (pool.ad.vaddr)
-		free_buf(&ofdev->dev, &pool.ad,
+		free_buf(&pdev->dev, &pool.ad,
 			 sizeof(struct diu_ad) * FSL_AOI_NUM, 8);
 	if (pool.gamma.vaddr)
-		free_buf(&ofdev->dev, &pool.gamma, 768, 32);
+		free_buf(&pdev->dev, &pool.gamma, 768, 32);
 	if (pool.cursor.vaddr)
-		free_buf(&ofdev->dev, &pool.cursor, MAX_CURS * MAX_CURS * 2,
-			 32);
+		free_buf(&pdev->dev, &pool.cursor, MAX_CURS * MAX_CURS * 2, 32);
 	if (machine_data->dummy_aoi_virt)
 		fsl_diu_free(machine_data->dummy_aoi_virt, 64);
 	iounmap(dr.diu_reg);
@@ -1754,7 +1635,7 @@ MODULE_DEVICE_TABLE(of, fsl_diu_match);
 
 static struct platform_driver fsl_diu_driver = {
 	.driver = {
-		.name = "fsl_diu",
+		.name = "fsl-diu-fb",
 		.owner = THIS_MODULE,
 		.of_match_table = fsl_diu_match,
 	},
@@ -1783,43 +1664,48 @@ static int __init fsl_diu_init(void)
 #else
 	monitor_port = fsl_diu_name_to_port(monitor_string);
 #endif
-	printk(KERN_INFO "Freescale DIU driver\n");
+	pr_info("Freescale Display Interface Unit (DIU) framebuffer driver\n");
 
 #ifdef CONFIG_NOT_COHERENT_CACHE
 	np = of_find_node_by_type(NULL, "cpu");
 	if (!np) {
-		printk(KERN_ERR "Err: can't find device node 'cpu'\n");
+		pr_err("fsl-diu-fb: can't find 'cpu' device node\n");
 		return -ENODEV;
 	}
 
 	prop = of_get_property(np, "d-cache-size", NULL);
 	if (prop = NULL) {
+		pr_err("fsl-diu-fb: missing 'd-cache-size' property' "
+		       "in 'cpu' node\n");
 		of_node_put(np);
 		return -ENODEV;
 	}
 
-	/* Freescale PLRU requires 13/8 times the cache size to do a proper
-	   displacement flush
+	/*
+	 * Freescale PLRU requires 13/8 times the cache size to do a proper
+	 * displacement flush
 	 */
-	coherence_data_size = *prop * 13;
+	coherence_data_size = be32_to_cpup(prop) * 13;
 	coherence_data_size /= 8;
 
 	prop = of_get_property(np, "d-cache-line-size", NULL);
 	if (prop = NULL) {
+		pr_err("fsl-diu-fb: missing 'd-cache-line-size' property' "
+		       "in 'cpu' node\n");
 		of_node_put(np);
 		return -ENODEV;
 	}
-	d_cache_line_size = *prop;
+	d_cache_line_size = be32_to_cpup(prop);
 
 	of_node_put(np);
 	coherence_data = vmalloc(coherence_data_size);
 	if (!coherence_data)
 		return -ENOMEM;
 #endif
+
 	ret = platform_driver_register(&fsl_diu_driver);
 	if (ret) {
-		printk(KERN_ERR
-			"fsl-diu: failed to register platform driver\n");
+		pr_err("fsl-diu-fb: failed to register platform driver\n");
 #if defined(CONFIG_NOT_COHERENT_CACHE)
 		vfree(coherence_data);
 #endif
@@ -1847,7 +1733,7 @@ module_param_named(mode, fb_mode, charp, 0);
 MODULE_PARM_DESC(mode,
 	"Specify resolution as \"<xres>x<yres>[-<bpp>][@<refresh>]\" ");
 module_param_named(bpp, default_bpp, ulong, 0);
-MODULE_PARM_DESC(bpp, "Specify bit-per-pixel if not specified mode");
+MODULE_PARM_DESC(bpp, "Specify bit-per-pixel if not specified in \"mode\"");
 module_param_named(monitor, monitor_string, charp, 0);
 MODULE_PARM_DESC(monitor, "Specify the monitor port "
 	"(\"dvi\", \"lvds\", or \"dlvds\") if supported by the platform");
diff --git a/include/linux/fsl-diu-fb.h b/include/linux/fsl-diu-fb.h
index daa9952..df23f59 100644
--- a/include/linux/fsl-diu-fb.h
+++ b/include/linux/fsl-diu-fb.h
@@ -20,18 +20,8 @@
 #ifndef __FSL_DIU_FB_H__
 #define __FSL_DIU_FB_H__
 
-/* Arbitrary threshold to determine the allocation method
- * See mpc8610fb_set_par(), map_video_memory(), and unmap_video_memory()
- */
-#define MEM_ALLOC_THRESHOLD (1024*768*4+32)
-
 #include <linux/types.h>
 
-struct mfb_alpha {
-	int enable;
-	int alpha;
-};
-
 struct mfb_chroma_key {
 	int enable;
 	__u8  red_max;
@@ -167,7 +157,7 @@ struct diu_hw {
 };
 
 struct diu_addr {
-	__u8 __iomem *vaddr;	/* Virtual address */
+	void *vaddr;		/* Virtual address */
 	dma_addr_t paddr;	/* Physical address */
 	__u32 	   offset;
 };
-- 
1.7.3.4



^ permalink raw reply related

* Re: [PATCHv2 09/15] OMAP: DSS2: HDMI: implement detect()
From: K, Mythri P @ 2011-09-14  5:46 UTC (permalink / raw)
  To: Rob Clark; +Cc: Tomi Valkeinen, linux-omap, linux-fbdev, archit
In-Reply-To: <CAF6AEGuhjZgH9zFuigjfvHGM9r7jSLY2TTCD0LjZbT+Ubrb12A@mail.gmail.com>

Hi,

On Mon, Sep 12, 2011 at 10:16 PM, Rob Clark <robdclark@gmail.com> wrote:
> On Mon, Sep 12, 2011 at 8:24 AM, K, Mythri P <mythripk@ti.com> wrote:
>>> +bool ti_hdmi_4xxx_detect(struct hdmi_ip_data *ip_data)
>>> +{
>>> +       int r;
>>> +
>>> +       void __iomem *base = hdmi_core_sys_base(ip_data);
>>> +
>>> +       /* HPD */
>>> +       r = REG_GET(base, HDMI_CORE_SYS_SYS_STAT, 1, 1);
>>> +
>>> +       return r = 1;
>>> +}
>>> +
>> For HPD the probe should also be on the core interrupt first , and the
>> detect should be dynamic, ie based on the cable connect and disconnect
>> event.So this approach for HPD is not really the way.
>> Also that should be based on the GPIO(63) , I am planning to push a
>> patch on that shortly.
>
>
> Fwiw, we do still need a dssdrv->detect() function from omapdrm
> driver..  if there is another way to implement that function, such as
> with a GPIO, that is great.  But somehow or another we need the detect
> function.  The implementation can always change later.
Yes we still need a detect , but the implementation would be different
, from the prior experience with the Hot-plug detection it wad found
that the interrupt based way to handle HPD was not the best ,but if
this is just to poll the status then it should be fine.
>
> BR,
> -R
>
Thanks and regards,
Mythri.

^ permalink raw reply

* Re: [REPOST][PATCH 2/2] video: miscellaneous minor changes to the
From: Tormod Volden @ 2011-09-14  6:54 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <1315929909-26311-2-git-send-email-timur@freescale.com>

On Tue, Sep 13, 2011 at 6:05 PM, Timur Tabi <timur@freescale.com> wrote:
> Make several minor, miscellaneous changes to the Freescale DIU framebuffer
> driver.  These changes "lighten" the code by removing crud, fixing small
> bugs, and fixing some coding style problems.  These changes will make it
> easier to make more substantial fixes in the future.

It would be much easier to review this if it is split up into several
commits. At least have the whitespace fixes in a separate commit, and
also the actual bug fixes. "git add -p" is your friend.

> 1. Fix incorrect indentation and spacing with some code.
> 2. Remove debug printks (they don't actually help in debugging the code).
> 3. Clean up some other printks (e.g. use pr_xxx, clean up the text, etc).
> 4. Remove the "default" videomode object since it's just a dupe of the
>   first element in the videomode array.
> 5. Remove some superfluous local variables.
> 6. Rename ofdev to pdev, since it's a platform device not an OF device.
> 7. Fix some device tree operations.
> 8. Fix some build warnings.
> 9. Removed some unused structures from the header file.
> 10. Other minor bug fixes and changes.

I would have found natural to split it up into commits like for
example: 1, 2+3, 4, 5+8+9, 10.

> @@ -217,59 +201,59 @@ struct mfb_info {
>        int x_aoi_d;            /* aoi display x offset to physical screen */
>        int y_aoi_d;            /* aoi display y offset to physical screen */
>        struct fsl_diu_data *parent;
> -       u8 *edid_data;
> +       void *edid_data;
>  };

Why do you convert edid_data from pointer to u8 to pointer to void?

Regards,
Tormod

^ permalink raw reply

* Re: [PATCHv2 09/15] OMAP: DSS2: HDMI: implement detect()
From: Tomi Valkeinen @ 2011-09-14  7:14 UTC (permalink / raw)
  To: K, Mythri P; +Cc: Rob Clark, linux-omap, linux-fbdev, archit
In-Reply-To: <CAP5A+B87w6Hir-0+SNnckQaDcO=ixgq-U8pFo2U42dwQgtUrZQ@mail.gmail.com>

On Wed, 2011-09-14 at 11:04 +0530, K, Mythri P wrote:
> Hi,
> 
> On Mon, Sep 12, 2011 at 10:16 PM, Rob Clark <robdclark@gmail.com> wrote:
> > On Mon, Sep 12, 2011 at 8:24 AM, K, Mythri P <mythripk@ti.com> wrote:
> >>> +bool ti_hdmi_4xxx_detect(struct hdmi_ip_data *ip_data)
> >>> +{
> >>> +       int r;
> >>> +
> >>> +       void __iomem *base = hdmi_core_sys_base(ip_data);
> >>> +
> >>> +       /* HPD */
> >>> +       r = REG_GET(base, HDMI_CORE_SYS_SYS_STAT, 1, 1);
> >>> +
> >>> +       return r = 1;
> >>> +}
> >>> +
> >> For HPD the probe should also be on the core interrupt first , and the
> >> detect should be dynamic, ie based on the cable connect and disconnect
> >> event.So this approach for HPD is not really the way.
> >> Also that should be based on the GPIO(63) , I am planning to push a
> >> patch on that shortly.
> >
> >
> > Fwiw, we do still need a dssdrv->detect() function from omapdrm
> > driver..  if there is another way to implement that function, such as
> > with a GPIO, that is great.  But somehow or another we need the detect
> > function.  The implementation can always change later.
> Yes we still need a detect , but the implementation would be different
> , from the prior experience with the Hot-plug detection it wad found
> that the interrupt based way to handle HPD was not the best ,but if
> this is just to poll the status then it should be fine.

I'm not sure I understood. First you say the implementation should be
different, but then you say this should be fine. So is this a valid
implementation for detect() or is there a better way to do it?

 Tomi




^ permalink raw reply

* Re: [PATCHv2 09/15] OMAP: DSS2: HDMI: implement detect()
From: Tomi Valkeinen @ 2011-09-14  8:34 UTC (permalink / raw)
  To: K, Mythri P; +Cc: Rob Clark, linux-omap, linux-fbdev, archit
In-Reply-To: <CAP5A+B9uDj4kCowbEKgX1J74zWvBarp64Cn4YVom-a8BYtab4w@mail.gmail.com>

On Wed, 2011-09-14 at 13:57 +0530, K, Mythri P wrote:
> On Wed, Sep 14, 2011 at 12:44 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> > On Wed, 2011-09-14 at 11:04 +0530, K, Mythri P wrote:
> >> Hi,
> >>
> >> On Mon, Sep 12, 2011 at 10:16 PM, Rob Clark <robdclark@gmail.com> wrote:
> >> > On Mon, Sep 12, 2011 at 8:24 AM, K, Mythri P <mythripk@ti.com> wrote:
> >> >>> +bool ti_hdmi_4xxx_detect(struct hdmi_ip_data *ip_data)
> >> >>> +{
> >> >>> +       int r;
> >> >>> +
> >> >>> +       void __iomem *base = hdmi_core_sys_base(ip_data);
> >> >>> +
> >> >>> +       /* HPD */
> >> >>> +       r = REG_GET(base, HDMI_CORE_SYS_SYS_STAT, 1, 1);
> >> >>> +
> >> >>> +       return r = 1;
> >> >>> +}
> >> >>> +
> >> >> For HPD the probe should also be on the core interrupt first , and the
> >> >> detect should be dynamic, ie based on the cable connect and disconnect
> >> >> event.So this approach for HPD is not really the way.
> >> >> Also that should be based on the GPIO(63) , I am planning to push a
> >> >> patch on that shortly.
> >> >
> >> >
> >> > Fwiw, we do still need a dssdrv->detect() function from omapdrm
> >> > driver..  if there is another way to implement that function, such as
> >> > with a GPIO, that is great.  But somehow or another we need the detect
> >> > function.  The implementation can always change later.
> >> Yes we still need a detect , but the implementation would be different
> >> , from the prior experience with the Hot-plug detection it wad found
> >> that the interrupt based way to handle HPD was not the best ,but if
> >> this is just to poll the status then it should be fine.
> >
> > I'm not sure I understood. First you say the implementation should be
> > different, but then you say this should be fine. So is this a valid
> > implementation for detect() or is there a better way to do it?
> >
> There is a better way to handle Hot-plug detection and notification..
> But depends on what is the purpose of this function, Ideally a detect

The purpose of the detect function is to return true or false, depending
on whether a (preferably powered-on) monitor is connecter via a cable or
not. So it tells if there's a display that can be used or not.

> would be the case to dynamically detect whether the cable is connected
> on not , But all this function does is to see the state of the HPD bit
> in core state statically.

I don't understand this one. How could this be more dynamic? The
function checks the HPD bit, which (based on my observation) shows the
status whether a display is connected or not.

> So I said if the purpose of this function is only to check for the HPD
> state bit it is fine.

What does HPD bit tell us then?

 Tomi



^ permalink raw reply

* Re: [PATCHv2 09/15] OMAP: DSS2: HDMI: implement detect()
From: K, Mythri P @ 2011-09-14  8:39 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: Rob Clark, linux-omap, linux-fbdev, archit
In-Reply-To: <1315984468.2172.10.camel@deskari>

On Wed, Sep 14, 2011 at 12:44 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On Wed, 2011-09-14 at 11:04 +0530, K, Mythri P wrote:
>> Hi,
>>
>> On Mon, Sep 12, 2011 at 10:16 PM, Rob Clark <robdclark@gmail.com> wrote:
>> > On Mon, Sep 12, 2011 at 8:24 AM, K, Mythri P <mythripk@ti.com> wrote:
>> >>> +bool ti_hdmi_4xxx_detect(struct hdmi_ip_data *ip_data)
>> >>> +{
>> >>> +       int r;
>> >>> +
>> >>> +       void __iomem *base = hdmi_core_sys_base(ip_data);
>> >>> +
>> >>> +       /* HPD */
>> >>> +       r = REG_GET(base, HDMI_CORE_SYS_SYS_STAT, 1, 1);
>> >>> +
>> >>> +       return r = 1;
>> >>> +}
>> >>> +
>> >> For HPD the probe should also be on the core interrupt first , and the
>> >> detect should be dynamic, ie based on the cable connect and disconnect
>> >> event.So this approach for HPD is not really the way.
>> >> Also that should be based on the GPIO(63) , I am planning to push a
>> >> patch on that shortly.
>> >
>> >
>> > Fwiw, we do still need a dssdrv->detect() function from omapdrm
>> > driver..  if there is another way to implement that function, such as
>> > with a GPIO, that is great.  But somehow or another we need the detect
>> > function.  The implementation can always change later.
>> Yes we still need a detect , but the implementation would be different
>> , from the prior experience with the Hot-plug detection it wad found
>> that the interrupt based way to handle HPD was not the best ,but if
>> this is just to poll the status then it should be fine.
>
> I'm not sure I understood. First you say the implementation should be
> different, but then you say this should be fine. So is this a valid
> implementation for detect() or is there a better way to do it?
>
There is a better way to handle Hot-plug detection and notification..
But depends on what is the purpose of this function, Ideally a detect
would be the case to dynamically detect whether the cable is connected
on not , But all this function does is to see the state of the HPD bit
in core state statically.
So I said if the purpose of this function is only to check for the HPD
state bit it is fine.

Thanks and regards,
Mythri.
>  Tomi
>
>
>
>

^ permalink raw reply

* Re: [PATCHv2 09/15] OMAP: DSS2: HDMI: implement detect()
From: K, Mythri P @ 2011-09-14  8:48 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: Rob Clark, linux-omap, linux-fbdev, archit
In-Reply-To: <1315989282.2172.36.camel@deskari>

On Wed, Sep 14, 2011 at 2:04 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On Wed, 2011-09-14 at 13:57 +0530, K, Mythri P wrote:
>> On Wed, Sep 14, 2011 at 12:44 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>> > On Wed, 2011-09-14 at 11:04 +0530, K, Mythri P wrote:
>> >> Hi,
>> >>
>> >> On Mon, Sep 12, 2011 at 10:16 PM, Rob Clark <robdclark@gmail.com> wrote:
>> >> > On Mon, Sep 12, 2011 at 8:24 AM, K, Mythri P <mythripk@ti.com> wrote:
>> >> >>> +bool ti_hdmi_4xxx_detect(struct hdmi_ip_data *ip_data)
>> >> >>> +{
>> >> >>> +       int r;
>> >> >>> +
>> >> >>> +       void __iomem *base = hdmi_core_sys_base(ip_data);
>> >> >>> +
>> >> >>> +       /* HPD */
>> >> >>> +       r = REG_GET(base, HDMI_CORE_SYS_SYS_STAT, 1, 1);
>> >> >>> +
>> >> >>> +       return r = 1;
>> >> >>> +}
>> >> >>> +
>> >> >> For HPD the probe should also be on the core interrupt first , and the
>> >> >> detect should be dynamic, ie based on the cable connect and disconnect
>> >> >> event.So this approach for HPD is not really the way.
>> >> >> Also that should be based on the GPIO(63) , I am planning to push a
>> >> >> patch on that shortly.
>> >> >
>> >> >
>> >> > Fwiw, we do still need a dssdrv->detect() function from omapdrm
>> >> > driver..  if there is another way to implement that function, such as
>> >> > with a GPIO, that is great.  But somehow or another we need the detect
>> >> > function.  The implementation can always change later.
>> >> Yes we still need a detect , but the implementation would be different
>> >> , from the prior experience with the Hot-plug detection it wad found
>> >> that the interrupt based way to handle HPD was not the best ,but if
>> >> this is just to poll the status then it should be fine.
>> >
>> > I'm not sure I understood. First you say the implementation should be
>> > different, but then you say this should be fine. So is this a valid
>> > implementation for detect() or is there a better way to do it?
>> >
>> There is a better way to handle Hot-plug detection and notification..
>> But depends on what is the purpose of this function, Ideally a detect
>
> The purpose of the detect function is to return true or false, depending
> on whether a (preferably powered-on) monitor is connecter via a cable or
> not. So it tells if there's a display that can be used or not.
>
>> would be the case to dynamically detect whether the cable is connected
>> on not , But all this function does is to see the state of the HPD bit
>> in core state statically.
>
> I don't understand this one. How could this be more dynamic? The
> function checks the HPD bit, which (based on my observation) shows the
> status whether a display is connected or not.
There is a GPIO which detects the +3.3V on the line and detects the
cable connect , there is also an interrupt based way.This is ideally
called a Hot-plug detect event according to the spec in HDMI terms.
But what you are saying here is that it is just a poll on the state?
>
>> So I said if the purpose of this function is only to check for the HPD
>> state bit it is fine.
>
> What does HPD bit tell us then?

HPD state bit tells whether the cable is connected and whether EDID is
ready to be read, But this is a static check that is done in this
function.

Thanks and regards,
Mythri.

^ permalink raw reply

* Re: [PATCHv2 09/15] OMAP: DSS2: HDMI: implement detect()
From: Tomi Valkeinen @ 2011-09-14  8:57 UTC (permalink / raw)
  To: K, Mythri P; +Cc: Rob Clark, linux-omap, linux-fbdev, archit
In-Reply-To: <CAP5A+B-GWN+oJhcVbB24swfmH=-dE4sViNqF+hX6=D1owzOLzA@mail.gmail.com>

On Wed, 2011-09-14 at 14:18 +0530, K, Mythri P wrote:
> On Wed, Sep 14, 2011 at 2:04 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> > On Wed, 2011-09-14 at 13:57 +0530, K, Mythri P wrote:
> >> On Wed, Sep 14, 2011 at 12:44 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:

<snip>

> > I don't understand this one. How could this be more dynamic? The
> > function checks the HPD bit, which (based on my observation) shows the
> > status whether a display is connected or not.
> There is a GPIO which detects the +3.3V on the line and detects the
> cable connect , there is also an interrupt based way.This is ideally
> called a Hot-plug detect event according to the spec in HDMI terms.
> But what you are saying here is that it is just a poll on the state?

Yes, it's just for polling, but I don't quite see the difference. A
hot-plug event notifies when the display is connected or disconnected,
and detect() tells if a display is connected. They are all about the
same thing.

> >> So I said if the purpose of this function is only to check for the HPD
> >> state bit it is fine.
> >
> > What does HPD bit tell us then?
> 
> HPD state bit tells whether the cable is connected and whether EDID is

This sounds like a good bit to test then. So is there something wrong
with using HPD? How does the GPIO differ from HPD bit?

> ready to be read, But this is a static check that is done in this
> function.

I don't understand what you mean with "static". The bit changes
dynamically according to the connect/disconnect state, and the bit is
checked dynamically when detect() is called.

 Tomi



^ permalink raw reply

* Re: [PATCHv2 09/15] OMAP: DSS2: HDMI: implement detect()
From: K, Mythri P @ 2011-09-14 12:32 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: Rob Clark, linux-omap, linux-fbdev, archit
In-Reply-To: <1315990630.2172.51.camel@deskari>

Hi,

On Wed, Sep 14, 2011 at 2:27 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On Wed, 2011-09-14 at 14:18 +0530, K, Mythri P wrote:
>> On Wed, Sep 14, 2011 at 2:04 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>> > On Wed, 2011-09-14 at 13:57 +0530, K, Mythri P wrote:
>> >> On Wed, Sep 14, 2011 at 12:44 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>
> <snip>
>
>> > I don't understand this one. How could this be more dynamic? The
>> > function checks the HPD bit, which (based on my observation) shows the
>> > status whether a display is connected or not.
>> There is a GPIO which detects the +3.3V on the line and detects the
>> cable connect , there is also an interrupt based way.This is ideally
>> called a Hot-plug detect event according to the spec in HDMI terms.
>> But what you are saying here is that it is just a poll on the state?
>
> Yes, it's just for polling, but I don't quite see the difference. A
> hot-plug event notifies when the display is connected or disconnected,
> and detect() tells if a display is connected. They are all about the
> same thing.
>
>> >> So I said if the purpose of this function is only to check for the HPD
>> >> state bit it is fine.
>> >
>> > What does HPD bit tell us then?
>>
>> HPD state bit tells whether the cable is connected and whether EDID is
>
> This sounds like a good bit to test then. So is there something wrong
> with using HPD? How does the GPIO differ from HPD bit?
>
>> ready to be read, But this is a static check that is done in this
>> function.
>
> I don't understand what you mean with "static". The bit changes
> dynamically according to the connect/disconnect state, and the bit is
> checked dynamically when detect() is called.
>
Well ! Who would call the detect and why ? By Dynamic i meant when the
cable is physically disconnected and connected there is detection
logic which can be implemented either by GPIo/Interrupts.
When you say the cable is connected , what happens in this case when
the cable is connected to say monitor of one resolution and then
plugged out and put to the other. Instead with dynamic method the
based on the physical connect and disconnect the notification would be
sent to any listener.

Thanks and regards,
Mythri.
>  Tomi
>
>
>

^ permalink raw reply

* Re: [PATCHv2 09/15] OMAP: DSS2: HDMI: implement detect()
From: Tomi Valkeinen @ 2011-09-14 14:11 UTC (permalink / raw)
  To: K, Mythri P; +Cc: Rob Clark, linux-omap, linux-fbdev, archit
In-Reply-To: <CAP5A+B9Me0RX3PY7Uwf=VuaHzaPtoNY0q9gxFg9H-Q3N2giC2Q@mail.gmail.com>

On Wed, 2011-09-14 at 17:50 +0530, K, Mythri P wrote:
> Hi,
> 
> On Wed, Sep 14, 2011 at 2:27 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> > On Wed, 2011-09-14 at 14:18 +0530, K, Mythri P wrote:
> >> On Wed, Sep 14, 2011 at 2:04 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> >> > On Wed, 2011-09-14 at 13:57 +0530, K, Mythri P wrote:
> >> >> On Wed, Sep 14, 2011 at 12:44 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> >
> > <snip>
> >
> >> > I don't understand this one. How could this be more dynamic? The
> >> > function checks the HPD bit, which (based on my observation) shows the
> >> > status whether a display is connected or not.
> >> There is a GPIO which detects the +3.3V on the line and detects the
> >> cable connect , there is also an interrupt based way.This is ideally
> >> called a Hot-plug detect event according to the spec in HDMI terms.
> >> But what you are saying here is that it is just a poll on the state?
> >
> > Yes, it's just for polling, but I don't quite see the difference. A
> > hot-plug event notifies when the display is connected or disconnected,
> > and detect() tells if a display is connected. They are all about the
> > same thing.
> >
> >> >> So I said if the purpose of this function is only to check for the HPD
> >> >> state bit it is fine.
> >> >
> >> > What does HPD bit tell us then?
> >>
> >> HPD state bit tells whether the cable is connected and whether EDID is
> >
> > This sounds like a good bit to test then. So is there something wrong
> > with using HPD? How does the GPIO differ from HPD bit?
> >
> >> ready to be read, But this is a static check that is done in this
> >> function.
> >
> > I don't understand what you mean with "static". The bit changes
> > dynamically according to the connect/disconnect state, and the bit is
> > checked dynamically when detect() is called.
> >
> Well ! Who would call the detect and why ? By Dynamic i meant when the
> cable is physically disconnected and connected there is detection
> logic which can be implemented either by GPIo/Interrupts.
> When you say the cable is connected , what happens in this case when
> the cable is connected to say monitor of one resolution and then
> plugged out and put to the other. Instead with dynamic method the
> based on the physical connect and disconnect the notification would be
> sent to any listener.

Ok, I see now what you mean.

Yes, you are right, detect() does not "know" if the monitor has changed
between polls, so both notification and polling are needed. I
implemented only polling as there's no HPD event mechanism yet in
omapdss, and also because this was simple and gives DRM basic ability to
detect a monitor.

 Tomi



^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox