Re: v3.6-rc1 DSS issues/regression

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

[-- Attachment #1: Type: text/plain, Size: 4038 bytes --]

On Wed, 2012-08-15 at 00:31 +0300, Aaro Koskinen wrote:
> Hi,
> 
> On Tue, Aug 07, 2012 at 04:22:02PM +0300, Tomi Valkeinen wrote:
> > On Tue, 2012-08-07 at 12:05 +0530, Archit Taneja wrote:
> > > On Tuesday 07 August 2012 03:44 AM, Aaro Koskinen wrote:
> > > > On Mon, Aug 06, 2012 at 11:06:28PM +0530, Archit Taneja wrote:
> > > >>> On Mon, 2012-08-06 at 19:47 +0300, Aaro Koskinen wrote:
> > > >>>> I can't get the display on N900 (SDI, acx565akm) to work with v3.6-rc1
> > > >>>> kernel, it's just full of flicker/noise.
> > > >>>>
> > > >>>> According to git-bisect, the problem is introduced by the commit:
> > > >>>>
> > > >>>>      commit f476ae9dab3234532d41d36beb4ba7be838fa786
> > > >>>>      Author: Archit Taneja <archit@ti.com>
> > > >>>>      Date:   Fri Jun 29 14:37:03 2012 +0530
> > > >>>>
> > > >>>>      OMAPDSS: APPLY: Remove DISPC writes to manager's lcd parameters in interface
> > > >
> > > > [...]
> > > >
> > > >> The diff I have shared introduces the register writes back. This
> > > >> should make it work like before. But we need to figure out which
> > > >> parameter write needs to be done immediately. If this works, could
> > > >> you remove each dispc register write turn by turn, and point out
> > > >> which is the culprit one?
> > > >
> > > > Thanks, the following one makes the display to work again:
> > > >
> > > > diff --git a/drivers/video/omap2/dss/sdi.c b/drivers/video/omap2/dss/sdi.c
> > > > index 5d31699..3c9f598 100644
> > > > --- a/drivers/video/omap2/dss/sdi.c
> > > > +++ b/drivers/video/omap2/dss/sdi.c
> > > > @@ -46,6 +46,9 @@ static void sdi_config_lcd_manager(struct omap_dss_device *dssdev)
> > > >   	sdi.mgr_config.video_port_width = 24;
> > > >   	sdi.mgr_config.lcden_sig_polarity = 1;
> > > >
> > > > +	dispc_mgr_set_clock_div(dssdev->manager->id,
> > > > +				&sdi.mgr_config.clock_info);
> > > > +
> > > 
> > > Thanks for finding this. It's a bit peculiar why this is happening. The 
> > > DISPC_DIVISOR is a shadow register for sure.
> > > 
> > > I don't know much about SDI, but it looks like the SDI PLL needs the 
> > > free running pixel clock from the LCD manager. To achieve this, we set 
> > > PCKFREEENABLE. The thing I don't understand is that whether the free 
> > > running pixel clock at this point would be derived out of the old LCD 
> > > and PCD values, or the new ones? It should have been old since LCD and 
> > > PCD are shadowed registers.
> > > 
> > > In other words, I am suspecting that the field PCKFREEENABLE does a copy 
> > > of the LCD and PCD dividers from the shadow registers to the working 
> > > registers.
> > > 
> > > The regdump shows that there were issues in SDI initialization.
> > > 
> > > Tomi,
> > > 
> > > Any ideas about this?
> > 
> > No real ideas. I think adding the line above, and a big HACK comment
> > above it, is ok for the time being. It's quite simple hack and shouldn't
> > cause any problems anywhere.
> 
> Will you be sending a patch (i.e. the HACK mentioned above) to fix this
> during the 3.6-rc cycle?

Yes, I'll queue it up.

> > Aaro, if you have spare time, you could test and move the call to
> > dispc_mgr_set_clock_div later, and see where calling it stops fixing the
> > issue. 
> > 
> > My guess is that calling it after dss_sdi_init() will still work, but
> > calling it after dss_sdi_enable() does not work. And most likely inside
> > dss_sdi_enable() it won't work if placed after dispc_pck_free_enable(). 
> 
> You are right that dss_sdi_enable() is the critical function. According
> to my tests, the exact breaking point is the
> 
> 	/* Waiting for PLL to lock */
> 
> loop. If dispc_mgr_set_clock_div() is called before it works, but if I
> put it after the loop the display fails to work.

That's interesting. I think that basically tells us that the shadow
register mechanism is somehow bypassed for pck-free. And if so, then the
above hack is not really a hack at all, but something required.

 Tomi


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]