On Thu, 2011-04-21 at 16:36 +0530, Janorkar, Mayuresh wrote: > > > -----Original Message----- > > From: Valkeinen, Tomi > > Sent: Tuesday, April 19, 2011 4:40 PM > > To: Janorkar, Mayuresh > > Cc: linux-omap@xxxxxxxxxxxxxxx; K, Mythri P > > Subject: Re: [PATCH 3/7] OMAP: DSS: Adding a picodlp panel driver > > > > On Mon, 2011-04-18 at 11:45 +0530, Mayuresh Janorkar wrote: > > > +static int picodlp_panel_power_on(struct omap_dss_device *dssdev) > > > +{ > > > + int r; > > > + > > > + if (dssdev->platform_enable) { > > > + r = dssdev->platform_enable(dssdev); > > > + if (r) > > > + return r; > > > + } > > > + > > > + r = omapdss_dpi_display_enable(dssdev); > > > + if (r) { > > > + dev_err(&dssdev->dev, "failed to enable DPI\n"); > > > + goto err; > > > + } > > > + /* after enabling, wait for some initialize sync interrupts */ > > > + msleep(675); > > > + dssdev->state = OMAP_DSS_DISPLAY_ACTIVE; > > > + > > > + return 0; > > > + > > > +err: > > > + if (dssdev->platform_disable) > > > + dssdev->platform_disable(dssdev); > > > + > > > + return r; > > > +} > > > > Why is the msleep needed there? It's a huge sleep, and can't find > > information about it from the documents you gave links to. I think I've > > asked this three times already. > > > > It is practically observed that without this delay the pico panel does not get initialized. > > > Also, it's rather strange to sleep at the end of the function. Normally > > you would sleep between two actions. > > Some of the panels in drivers/video/omap2/displays sleep after dpi_display_enable. Yes, but the drivers sleep between dpi_display_enable and platform_enable. They don't sleep at the end of a function, they sleep between two actions because the second action requires the first action to be stabilized. More exactly, the panel needs to get the pixel clock for a few frames to initialize itself, so that the image is stable when the panel is taken out of reset. > I agree here the sleep is huge. But I could not minimize it below this value. > I can handle it in some other ways: > See if the i2c_pakcet is sent. If not sent wait for some time ~50ms and then resend. You need to find the documentation describing the power-up times to understand what is required and where. Most likely the case here is that picodlp takes rather long to wake up after it's taken out of reset (i.e. platform_enabl. dpi_display_enable probably doesn't have anything to do with this), and you need to wait that time before sending i2c messages. And if that is the case, you don't need to sleep after platform_enable or dpi_display_enable, but before sending the first i2c message. And as the sleep here is such a long sleep, I think it warrants some optimization. If the sending of the i2c message happens in some other function, you can store the timestamp at the time when the reset is removed, and before sending the i2c message use the timestamp to sleep enough to be sure the picodlp is up and running. Tomi -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html