Re: omapdrm: Pandora Blues

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Nikolaus,

On Wednesday, 8 November 2017 09:52:27 EET H. Nikolaus Schaller wrote:
> Hi Laurent,
> sorry but I missed this mail somehow.

No worries.

> > Am 07.11.2017 um 08:35 schrieb Laurent Pinchart:
> > On Monday, 6 November 2017 19:52:53 EET H. Nikolaus Schaller wrote:
> >> Am 06.11.2017 um 17:00 schrieb Tomi Valkeinen <tomi.valkeinen@xxxxxx>:
> >>> 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.
> >>>> jpeg
> >>>> 4.14-rc1:
> >>>> http://download.goldelico.com/letux-kernel/files/thumb_DSC00813_1024.
> >>>> jpeg
> >>>> 
> >>>> 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.
> >> 
> >> So which one is the "future proof"?
> >> 
> >> compatible = "toppoly,td028ttec1"
> >> 
> >> or
> >> 
> >> compatible = "omapdss,tpo,td043mtea1"
> >> 
> >> Somehow, both seem to work (up to 4.13.x) on different devices.
> >> 
> >> Anyways, "toppoly" should be "tpo" according to
> >> Documentation/devicetree/bindings/vendor-prefixes.txt I'll add this to
> >> the patches we will need...
> >> 
> >>>> 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@xxxxxxxxxxxxxxxx>
> >>>> 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.
> >> 
> >> Ah, I see.
> >> 
> >> If VDDS_DSI isn't enabled it garbles the panel RGB signals. That would
> >> explain a lot. It is still funny why it only affects the Red and Green
> >> channel, but that may have to do something with the pad drivers inside
> >> the chip. Maybe those for Blue are powered differently...
> >> 
> >>> I don't have a device up and running now, but I think the "family"
> >>> string that it tries to match can also be seen somewhere under /proc, so
> >>> you could perhaps find it and see if it actually is OMAP3530, which
> >>> should be matched by the code.
> >> 
> >> Or I add a printk() to watch what it tries to find. That should be a
> >> quite simple test.
> > 
> > Sorry for having broken the Pandora :(
> > 
> > I believe I incorrectly matched on .family instead of .machine. Could you
> > try the following patch ? I've only compile-tested it as I don't have
> > access to any OMAP3 board using the DPI output right now.
> > 
> > From a2df9cde5b854cbb19a88b9dac9337d515dda713 Mon Sep 17 00:00:00 2001
> > From: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> > Date: Tue, 7 Nov 2017 08:23:54 +0200
> > Subject: [PATCH] drm: omapdrm: Fix DPI on platforms using the DSI VDDS
> > 
> > Commit d178e034d565 ("drm: omapdrm: Move FEAT_DPI_USES_VDDS_DSI feature
> > to dpi code") replaced usage of platform data version with SoC matching
> > to configure DPI VDDS. The SoC match entries were incorrect, they should
> > have matched on the machine name instead of the SoC family. Fix it.
> > 
> > Fixes: d178e034d565 ("drm: omapdrm: Move FEAT_DPI_USES_VDDS_DSI feature to
> > dpi code") Signed-off-by: Laurent Pinchart
> > <laurent.pinchart@xxxxxxxxxxxxxxxx> ---
> > drivers/gpu/drm/omapdrm/dss/dpi.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/omapdrm/dss/dpi.c
> > b/drivers/gpu/drm/omapdrm/dss/dpi.c index daf286fc8a40..ca1e3b489540
> > 100644
> > --- a/drivers/gpu/drm/omapdrm/dss/dpi.c
> > +++ b/drivers/gpu/drm/omapdrm/dss/dpi.c
> > @@ -566,8 +566,8 @@ static int dpi_verify_pll(struct dss_pll *pll)
> > }
> > 
> > static const struct soc_device_attribute dpi_soc_devices[] = {
> > -	{ .family = "OMAP3[456]*" },
> > -	{ .family = "[AD]M37*" },
> > +	{ .machine = "OMAP3[456]*" },
> 
> this pattern matches:
> Pandora:	.machine = OMAP3430/3530
> GTA04:		.machine = OMAP3630/DM3730
> 
> Both devices work.
> 
> > +	{ .machine = "[AD]M37*" },
> 
> Both devices work without this line.
> 
> So I am not sure if such a .machine value exists and we need this line at
> all.

The AM3703 and DM3725 chips use DSI VDDS for DPI so we need both match 
entries.

> Is there a list of all potential strings or do we have to reverse engineer?

They are all in arch/arm/mach-omap2/id.c. You will find the old model 
information passed through platform data to the DSS driver in arch/arm/mach-
omap2/display.c before commit d178e034d565. Matching the two isn't much fun.

As explained in another e-mail another option would be to keep matching on the 
family with a match string set to "OMAP3*" but I haven't ruled out the 
possibility of false positives in that case.

> Looking into the other patterns there is "AM35*" and "AM43*", but no AM37 or
> DM37.
> 
> iMac:master hns$ fgrep -R .machine drivers/gpu/drm/omapdrm/
> drivers/gpu/drm/omapdrm//dss/dispc.c:	{ .machine = "OMAP3[45]*",
> drivers/gpu/drm/omapdrm//dss/dispc.c:	{ .machine = "OMAP3[45]*",	.data =
> &omap34xx_rev3_0_dispc_feats }, drivers/gpu/drm/omapdrm//dss/dispc.c:	{
> .machine = "AM35*",		.data = &omap34xx_rev3_0_dispc_feats },
> drivers/gpu/drm/omapdrm//dss/dispc.c:	{ .machine = "AM43*",		.data =
> &am43xx_dispc_feats }, drivers/gpu/drm/omapdrm//dss/dsi.c:	{ .machine =
> "OMAP3[45]*",	.data = &dsi_of_data_omap34xx },
> drivers/gpu/drm/omapdrm//dss/dsi.c:	{ .machine = "AM35*",		.data =
> &dsi_of_data_omap34xx }, drivers/gpu/drm/omapdrm//dss/dss.c:	{ .machine =
> "OMAP3430/3530", .data = &omap34xx_dss_feats },
> drivers/gpu/drm/omapdrm//dss/dss.c:	{ .machine = "AM35??",        .data =
> &omap34xx_dss_feats }, drivers/gpu/drm/omapdrm//dss/venc.c:	{ .machine =
> "OMAP3[45]*" },
> drivers/gpu/drm/omapdrm//dss/venc.c:	{ .machine = "AM35*" },
> iMac:master hns$ fgrep -R .family drivers/gpu/drm/omapdrm/
> drivers/gpu/drm/omapdrm//dss/dpi.c:	{ .family = "OMAP3[456]*" },
> drivers/gpu/drm/omapdrm//dss/dpi.c:	{ .family = "[AD]M37*" },
> drivers/gpu/drm/omapdrm//dss/dss.c:	{ .family  = "AM43xx",        .data =
> &am43xx_dss_feats }, drivers/gpu/drm/omapdrm//dss/hdmi4_core.c:	{ .family =
> "OMAP4", .revision = "ES1.?", .data = &hdmi4_es1_features },
> drivers/gpu/drm/omapdrm//dss/hdmi4_core.c:	{ .family = "OMAP4", .revision =
> "ES2.?", .data = &hdmi4_es2_features },
> drivers/gpu/drm/omapdrm//dss/hdmi4_core.c:	{ .family = "OMAP4",			  
.data =
> &hdmi4_es3_features }, drivers/gpu/drm/omapdrm//omap_drv.c:	{ .family =
> "OMAP3", .data = (void *)0x3430 }, drivers/gpu/drm/omapdrm//omap_drv.c:	{
> .family = "OMAP4", .data = (void *)0x4430 },
> drivers/gpu/drm/omapdrm//omap_drv.c:	{ .family = "OMAP5", .data = (void
> *)0x5430 }, drivers/gpu/drm/omapdrm//omap_drv.c:	{ .family = "DRA7",  .data
> = (void *)0x0752 }, iMac:master hns$
> 
> > 	{ /* sentinel */ }
> > 
> > };
> 
> So here what I see to match and properly enable regulators (without the
> "[AD]M37*" pattern):
> 
> Pandora / OMAP3530
> 
> [   19.895721] attr
> [   19.897674]   machine = OMAP3430/3530
> [   19.901519]   family = OMAP3
> [   19.904571]   revision = ES3.1
> [   19.908386]   soc_id = null
> [   19.911315] match
> [   19.913360]   machine = OMAP3[456]*
> [   19.917297]   family = null
> [   19.920257]   revision = null
> [   19.923370]   soc_id = null
> [   19.926452] dpi_init_regulator() match
> 
> GTA04 / DM3730
> 
> [   19.584136] attr
> [   19.586395]   machine = OMAP3630/DM3730
> [   19.590423]   family = OMAP3
> [   19.593444]   revision = ES1.2
> [   19.598083]   soc_id = null
> [   19.601043] match
> [   19.603057]   machine = OMAP3[456]*
> [   19.606933]   family = null
> [   19.609863]   revision = null
> [   19.613006]   soc_id = null
> [   19.616027] dpi_init_regulator() match

-- 
Regards,

Laurent Pinchart

--
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



[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux