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@xxxxxx>
> > > > ---
> > > > 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?
> >
> > > > +
> > > > + /* Force idle */
> > > > + rfbi_enable_clocks(1);
> > > > + l = rfbi_read_reg(RFBI_SYSCONFIG);
> > > > + l &= ~(0x03 << 3);
> > > > + rfbi_write_reg(RFBI_SYSCONFIG, l);
> > > > + rfbi_enable_clocks(0);
> > >
> > > Why force idle?
> >
> > The RFBI module must be forced to idle on suspend to allow
> for the DSS module to idle. The
> CM_CLKSTST_DSS[CLKACTIVITY_DSS] bit does not change if the RFBI module
> was configured to autoidle, preventing DSS from entering retention or
> off.
>
> Why must it? Is there a hardware bug (what errata number?)?
>
> 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
