| Message ID | 29623226.aNzM8WBJeK@avalon (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Laurent, sorry but I missed this mail somehow. > Am 07.11.2017 um 08:35 schrieb Laurent Pinchart <laurent.pinchart@ideasonboard.com>: > > Hello, > > 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@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.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@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. >> >> 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@ideasonboard.com> > 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@ideasonboard.com> > --- > 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. Is there a list of all potential strings or do we have to reverse engineer? 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 BR, Nikolaus -- 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
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@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. > >>>> 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@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. > >> > >> 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@ideasonboard.com> > > 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@ideasonboard.com> --- > > 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
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]*" }, + { .machine = "[AD]M37*" }, { /* sentinel */ } };