RE: [PATCH 1/1] OMAP: DSS2: RFBI driver update

[Tomi Valkeinen <tomi.valkeinen@nokia.com>]

On Mon, 2009-10-12 at 17:25 +0200, ext Christensen, Mikkel wrote:
> On Mon, Oct 12, 2009 at 09:17:27, Tomi Valkeinen wrote:
> > Subject: RE: [PATCH 1/1] OMAP: DSS2: RFBI driver update
> > 
> > On Mon, 2009-10-12 at 16:03 +0200, ext Christensen, Mikkel wrote:
> > > On Mon, Oct 12, 2009 at 03:25:27, Tomi Valkeinen wrote:
> > > > Subject: Re: [PATCH 1/1] OMAP: DSS2: RFBI driver update
> > > > 
> > > > On Fri, 2009-10-09 at 23:08 +0200, ext Mikkel Christensen wrote:
> > > > > This patch adds suspend / resume functionality to the RFBI
> > > > driver along with missing callback functions needed by OMAP Frame 
> > > > buffer.
> > > > > 
> > > > > Signed-off-by: Mikkel Christensen <mlc@ti.com>
> > > > > ---
> > > > >  drivers/video/omap2/dss/rfbi.c |   76 
> > > > ++++++++++++++++++++++++++++++++++++++++
> > > > >  1 files changed, 76 insertions(+), 0 deletions(-)
> > > > > 
> > 
> > snip
> > 
> > > > > +static int rfbi_display_suspend(struct omap_dss_device
> > *dssdev) {
> > > > > +	unsigned long l;
> > > > > +
> > > > > +	if (dssdev->state != OMAP_DSS_DISPLAY_ACTIVE)
> > > > > +		return -EINVAL;
> > > > > +
> > > > > +	DSSDBG("rfbi_display_suspend\n");
> > > > > +
> > > > > +	if (dssdev->driver->suspend)
> > > > > +		dssdev->driver->suspend(dssdev);
> > > > > +
> > > > > +	dispc_enable_lcd_out(0);
> > > > 
> > > > I don't think this is needed. DSS hardware disables
> > lcd_out when the
> > > > transfer has finished. Although for correct operation suspend() 
> > > > should wait until the last transfer has been done before
> > disabling
> > > > clocks, which is something it currently doesn't.
> > > 
> > > This was done with reference to the DPI and SDI files that
> > do the same thing. It can be removed if not necessary.  
> > 
> > DPI and SDI work quite differently than RFBI, you can't just copy 
> > their functionality =).
> > 
> > It is more correct without disable/enable_lcd_out, but it's not 
> > correct as there may be a transfer ongoing when suspend call comes. 
> > That's why there should be some mechanism to wait until the transfer 
> > has been done.
> > DSI tries to do this (and hopefully correctly does =).
> 
> OK. Thanks I will try to incorporate this. Where in the DSI driver is this mechanism implemented?

It's the dsi_bus_lock mechanism. But it doesn't work as such for RFBI,
because the DSI uses a bit different mechanism for updates. But somehow
the suspend function needs to wait until the transfer has been
completed, and suspend only after that (and also make sure that no new
transfer starts before the suspend).

 Tomi