[Letux-kernel] omapdrm: Pandora Blues

[Laurent Pinchart]

Hi Nikolaus,

On Tuesday, 7 November 2017 13:36:14 EET H. Nikolaus Schaller wrote:
> > Am 06.11.2017 um 17:00 schrieb Tomi Valkeinen <tomi.valkeinen at ti.com>:
> > 
> > On 06/11/17 16:04, H. Nikolaus Schaller wrote:
> >> Hi,
> >> some time after upgrading to 4.14-rc* I tried to boot the OpenPandora.
> >> It turned out that the display panel has a bug - it only shows black/blue
> >> colors instead of RGB.
> >> 
> >> Some tests revealed that something happened between 4.13.0 and 4.14-rc1.
> >> Here are screen photos:
> >> 
> >> 4.13.0:  
> >> http://download.goldelico.com/letux-kernel/files/thumb_DSC00812_1024.jpe
> >> g 4.14-rc1:
> >> http://download.goldelico.com/letux-kernel/files/thumb_DSC00813_1024.jpe
> >> g
> >> 
> >> But only for the OpenPandora. For the GTA04 it works.
> >> 
> >> Well, the GTA04 is using a different panel "toppoly,td028ttec1"
> >> and driver instead of "omapdss,tpo,td043mtea1".
> >> [BTW: there seems to be some mixup of "compatible" schemes].
> > 
> > Omapdss boot-time code will prepend the compatibles with "omapdss,". The
> > point is that the DTS is generic, but we'll still end up matching with
> > the omapdss specific drivers. It's a temporary hack, and gets dropped as
> > soon as we can use the common DRM panel model.
> > 
> >> And there is also another difference between both: the Pandora 600MHz
> >> uses an OMAP3530 while the GTA04 uses a DM3730.
> >> 
> >> So something has become incompatible with *some* DPI panel drivers.
> >> 
> >> After more than a week of bisecting in parallel to important other tasks
> >> (it takes ca. 30-60 minutes for each run to add local patches, compile,
> >> install, boot, check results - just to find some
> >> "[drm:omap_crtc_error_irq] *ERROR* lcd: errors: 00004000"), I ended up
> >> with a specific result:
> >> 
> >> 
> >> iMac:master hns$ git bisect bad
> >> d178e034d5653edfbd16d0c71eeeed467e33c96f is the first bad commit
> >> commit d178e034d5653edfbd16d0c71eeeed467e33c96f
> >> Author: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> >> Date:   Sat Aug 5 01:44:12 2017 +0300
> >> 
> >>   drm: omapdrm: Move FEAT_DPI_USES_VDDS_DSI feature to dpi code
> >>   
> >>   The FEAT_DPI_USES_VDDS_DSI feature is specific to the DPI, move it from
> >>   the omap_dss_features structure to the dpi code.
> > 
> > Well, the patch is simple enough... For some reason, the
> > soc_device_match(dpi_soc_devices) call there doesn't match Pandora.
> 
> Ok, I think I have found it. The pattern in dpi_soc_devices[] is wrong.
> 
> Here is what my printk hack() reports on the 600 MHz Pandora (OMAP3530):
> 
> [   20.171325] soc_device_match_attr()
> [   20.175445] attr
> [   20.177398]   machine = OMAP3430/3530
> [   20.181274]   family = OMAP3
> [   20.184295]   revision = ES3.1
> [   20.188232]   soc_id = null
> [   20.191192] match
> [   20.193237]   machine = null
> [   20.196411]   family = OMAP3[456]*
> [   20.199981]   revision = null
> [   20.203124]   soc_id = null
> [   20.182403] dpi_init_regulator() match
> 
> Changing the pattern to "OMAP[3456]*" makes it work but I am not sure what
> exactly is intended. Should the pattern match the .machine and not the
> .family?
> 
> The DM3730 based GTA04 reports (without patch):
> 
> [   18.618621] soc_device_match_attr()
> [   18.622314] attr
> [   18.624237]   machine = OMAP3630/DM3730
> [   18.628509]   family = OMAP3
> [   18.631561]   revision = ES1.2
> [   18.634765]   soc_id = null
> [   18.638000] match
> [   18.640014]   machine = null
> [   18.643035]   family = OMAP3[456]*
> [   18.646697]   revision = null
> [   18.649810]   soc_id = null
> [   18.652770] soc_device_match_attr()
> [   18.656616] attr
> [   18.658538]   machine = OMAP3630/DM3730
> [   18.662567]   family = OMAP3
> [   18.665649]   revision = ES1.2
> [   18.668853]   soc_id = null
> [   18.671813] match
> [   18.673828]   machine = null
> [   18.677001]   family = [AD]M37*
> [   18.680297]   revision = null
> [   18.683410]   soc_id = null
> [   18.686614] dpi_init_regulator() no match
> 
> Here we have no match and don't enable VDDS_DSI - but it still works (maybe
> VDDS_DSI is enabled elsewhere). And I do not see how the second pattern
> .family = "[AD]M37*" can ever match.
> 
> According to the comment in the original patch:
> 
> "The DPI uses the DSI VDDS on OMAP34xx, OMAP35xx, OMAP36xx, AM37xx and
> DM37xx only."
> 
> a single match for .family = "OMAP3" should suffice. Shouldn't it?

I think that would work. It would additionally match the TI814x, but that SoC 
doesn't seem to have a DSS anyway.

> And there is no family "AM/DM37*". Or is it?
> 
> A tiny and subtle flaw which has such a strange result...
> 
> But I don't know what would be correct.

As explained in a patch in another e-mail in this thread, the match should 
have been against machine, not family. We could either test that patch, or 
just match against machine = "OMAP3", which would be simpler.

Tomi, do you know any OMAP3 machine that doesn't use the DSI VDDS for DPI ?

-- 
Regards,

Laurent Pinchart