| Message ID | 20120806221424.GD1298@harshnoise.musicnaut.iki.fi (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tuesday 07 August 2012 03:44 AM, Aaro Koskinen wrote: > Hi, > > 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? Archit -- 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
On Tue, 2012-08-07 at 12:05 +0530, Archit Taneja wrote: > On Tuesday 07 August 2012 03:44 AM, Aaro Koskinen wrote: > > Hi, > > > > 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. 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(). Tomi
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? > 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. A. -- 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
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
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); + dss_mgr_set_lcd_config(dssdev->manager, &sdi.mgr_config); }