| Message ID | cover.1510822218.git.hns@goldelico.com |
|---|---|
| Headers | show |
| Series | Fixes for omapdrm on OpenPandora and GTA04 | expand |
On 16/11/17 10:50, H. Nikolaus Schaller wrote: > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > 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. > > The result was observed on OpenPandora with OMAP3530 where the panel only > had the Blue channel and Red&Green were missing. It was not observed on > GTA04 with DM3730. > > Fixes: d178e034d565 ("drm: omapdrm: Move FEAT_DPI_USES_VDDS_DSI feature to dpi code") > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Reported-by: H. Nikolaus Schaller <hns@goldelico.com> > Tested-by: H. Nikolaus Schaller <hns@goldelico.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 4ed5fde11313..a91e5f1a0490 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 */ } > }; > > I have picked this one. I think the rest of the patches are more of a cleanup, right? And you'll be sending v3 at some point. Tomi
Hi, > Am 21.11.2017 um 11:25 schrieb Tomi Valkeinen <tomi.valkeinen@ti.com>: > > On 16/11/17 10:50, H. Nikolaus Schaller wrote: >> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com> >> >> 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. >> >> The result was observed on OpenPandora with OMAP3530 where the panel only >> had the Blue channel and Red&Green were missing. It was not observed on >> GTA04 with DM3730. >> >> Fixes: d178e034d565 ("drm: omapdrm: Move FEAT_DPI_USES_VDDS_DSI feature to dpi code") >> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> >> Reported-by: H. Nikolaus Schaller <hns@goldelico.com> >> Tested-by: H. Nikolaus Schaller <hns@goldelico.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 4ed5fde11313..a91e5f1a0490 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 */ } >> }; >> >> > > I have picked this one. Fine. > I think the rest of the patches are more of a > cleanup, right? And you'll be sending v3 at some point. Yes. Should we wait for more comments or should I send now? BR and thanks, Nikolaus Schaller > > Tomi > > -- > Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. > Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
* H. Nikolaus Schaller <hns@goldelico.com> [171116 08:53]: > Vendor string is "tpo" and not "toppoly". > > Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com> > --- > arch/arm/boot/dts/omap3-gta04.dtsi | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/arm/boot/dts/omap3-gta04.dtsi b/arch/arm/boot/dts/omap3-gta04.dtsi > index 4504908c23fe..ec27ed67a22a 100644 > --- a/arch/arm/boot/dts/omap3-gta04.dtsi > +++ b/arch/arm/boot/dts/omap3-gta04.dtsi > @@ -86,7 +86,7 @@ > > /* lcd panel */ > lcd: td028ttec1@0 { > - compatible = "toppoly,td028ttec1"; > + compatible = "tpo,td028ttec1"; > reg = <0>; > spi-max-frequency = <100000>; > spi-cpol; Applying into omap-for-v4.15/fixes thanks. The other dts patch seems like it can wait for v4.16 merge window. Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Tony, > Am 28.11.2017 um 15:57 schrieb Tony Lindgren <tony@atomide.com>: > > * H. Nikolaus Schaller <hns@goldelico.com> [171116 08:53]: >> Vendor string is "tpo" and not "toppoly". >> >> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com> >> --- >> arch/arm/boot/dts/omap3-gta04.dtsi | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/arch/arm/boot/dts/omap3-gta04.dtsi b/arch/arm/boot/dts/omap3-gta04.dtsi >> index 4504908c23fe..ec27ed67a22a 100644 >> --- a/arch/arm/boot/dts/omap3-gta04.dtsi >> +++ b/arch/arm/boot/dts/omap3-gta04.dtsi >> @@ -86,7 +86,7 @@ >> >> /* lcd panel */ >> lcd: td028ttec1@0 { >> - compatible = "toppoly,td028ttec1"; >> + compatible = "tpo,td028ttec1"; >> reg = <0>; >> spi-max-frequency = <100000>; >> spi-cpol; > > Applying into omap-for-v4.15/fixes thanks. The other dts patch seems > like it can wait for v4.16 merge window. Hm. Not really. It needs the panel driver to match. So either both or none should be applied or it will break the panel from working. I am just 1-2 hours away from submitting a -v3 (rebased and tested on top of 4.15-rc1). BR and thanks, Nikolaus -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
* H. Nikolaus Schaller <hns@goldelico.com> [171128 15:04]: > Hi Tony, > > > Am 28.11.2017 um 15:57 schrieb Tony Lindgren <tony@atomide.com>: > > > > * H. Nikolaus Schaller <hns@goldelico.com> [171116 08:53]: > >> Vendor string is "tpo" and not "toppoly". > >> > >> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com> > >> --- > >> arch/arm/boot/dts/omap3-gta04.dtsi | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/arch/arm/boot/dts/omap3-gta04.dtsi b/arch/arm/boot/dts/omap3-gta04.dtsi > >> index 4504908c23fe..ec27ed67a22a 100644 > >> --- a/arch/arm/boot/dts/omap3-gta04.dtsi > >> +++ b/arch/arm/boot/dts/omap3-gta04.dtsi > >> @@ -86,7 +86,7 @@ > >> > >> /* lcd panel */ > >> lcd: td028ttec1@0 { > >> - compatible = "toppoly,td028ttec1"; > >> + compatible = "tpo,td028ttec1"; > >> reg = <0>; > >> spi-max-frequency = <100000>; > >> spi-cpol; > > > > Applying into omap-for-v4.15/fixes thanks. The other dts patch seems > > like it can wait for v4.16 merge window. > > Hm. Not really. It needs the panel driver to match. So either both or > none should be applied or it will break the panel from working. > > I am just 1-2 hours away from submitting a -v3 (rebased and tested > on top of 4.15-rc1). OK fine dropping both. Please update the description in both dts patches to make it clear they are needed as a fix. Preferrably with a proper fixes tag. Having "We can remove the "omapdss," prefix" in the description sure does not sounds like it's needed as a fix :) Sounds like maybe these two should be just a single patch for a proper fix? Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Tony, > Am 28.11.2017 um 16:10 schrieb Tony Lindgren <tony@atomide.com>: > > * H. Nikolaus Schaller <hns@goldelico.com> [171128 15:04]: >> Hi Tony, >> >>> Am 28.11.2017 um 15:57 schrieb Tony Lindgren <tony@atomide.com>: >>> >>> * H. Nikolaus Schaller <hns@goldelico.com> [171116 08:53]: >>>> Vendor string is "tpo" and not "toppoly". >>>> >>>> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com> >>>> --- >>>> arch/arm/boot/dts/omap3-gta04.dtsi | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/arch/arm/boot/dts/omap3-gta04.dtsi b/arch/arm/boot/dts/omap3-gta04.dtsi >>>> index 4504908c23fe..ec27ed67a22a 100644 >>>> --- a/arch/arm/boot/dts/omap3-gta04.dtsi >>>> +++ b/arch/arm/boot/dts/omap3-gta04.dtsi >>>> @@ -86,7 +86,7 @@ >>>> >>>> /* lcd panel */ >>>> lcd: td028ttec1@0 { >>>> - compatible = "toppoly,td028ttec1"; >>>> + compatible = "tpo,td028ttec1"; >>>> reg = <0>; >>>> spi-max-frequency = <100000>; >>>> spi-cpol; >>> >>> Applying into omap-for-v4.15/fixes thanks. The other dts patch seems >>> like it can wait for v4.16 merge window. >> >> Hm. Not really. It needs the panel driver to match. So either both or >> none should be applied or it will break the panel from working. >> >> I am just 1-2 hours away from submitting a -v3 (rebased and tested >> on top of 4.15-rc1). > > OK fine dropping both. Please update the description in both dts > patches to make it clear they are needed as a fix. Preferrably > with a proper fixes tag. Well, it is not "needed" in a strong sense since current mainline&stable works. It is more a style and consistency fix to use "tpo," everywhere. > > Having "We can remove the "omapdss," prefix" in the description sure > does not sounds like it's needed as a fix :) The description has been improved on -v3. > > Sounds like maybe these two should be just a single patch for > a proper fix? Hm. I usually get the feedback to separate DT and driver fixes into separate commits... Therefore I submit patch sets in the hope they are not picked apart :) As promised -v3 is following now. BR, Nikolaus -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
* H. Nikolaus Schaller <hns@goldelico.com> [171128 15:51]: > > Am 28.11.2017 um 16:10 schrieb Tony Lindgren <tony@atomide.com>: > > OK fine dropping both. Please update the description in both dts > > patches to make it clear they are needed as a fix. Preferrably > > with a proper fixes tag. > > Well, it is not "needed" in a strong sense since current mainline&stable > works. It is more a style and consistency fix to use "tpo," everywhere. > > > > > Having "We can remove the "omapdss," prefix" in the description sure > > does not sounds like it's needed as a fix :) > > The description has been improved on -v3. Thanks. > > Sounds like maybe these two should be just a single patch for > > a proper fix? > > Hm. I usually get the feedback to separate DT and driver fixes into > separate commits... Therefore I submit patch sets in the hope they > are not picked apart :) See "both dts patches" part above. Yes the dts patches can and should be sent separately. In almost every case if the dts patches cannot be applied separately it means your driver changes are breaking things. Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> Am 28.11.2017 um 17:00 schrieb Tony Lindgren <tony@atomide.com>: > > * H. Nikolaus Schaller <hns@goldelico.com> [171128 15:51]: >>> Am 28.11.2017 um 16:10 schrieb Tony Lindgren <tony@atomide.com>: >>> OK fine dropping both. Please update the description in both dts >>> patches to make it clear they are needed as a fix. Preferrably >>> with a proper fixes tag. >> >> Well, it is not "needed" in a strong sense since current mainline&stable >> works. It is more a style and consistency fix to use "tpo," everywhere. >> >>> >>> Having "We can remove the "omapdss," prefix" in the description sure >>> does not sounds like it's needed as a fix :) >> >> The description has been improved on -v3. > > Thanks. > >>> Sounds like maybe these two should be just a single patch for >>> a proper fix? >> >> Hm. I usually get the feedback to separate DT and driver fixes into >> separate commits... Therefore I submit patch sets in the hope they >> are not picked apart :) > > See "both dts patches" part above. Yes the dts patches can and should > be sent separately. In almost every case if the dts patches cannot be > applied separately it means your driver changes are breaking things. That is why the v3 driver now accepts both, the old and the new vendor name. This means applying the driver alone is safe. Applying or not applying DTS patch afterwards is also safe. BR, Nikolaus -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
V2: * replaced patch to fix DSI VDDS for OMAP3 by equivalent patch from Laurent Pinchart * keep previous compatibility option in panel driver to handle older device tree binaries 2017-11-08 22:09:36: This patch set fixes vendor names of the panels and fixes a problem on omapdrm with enabling VDD_DSI for OMAP3 which is needed for displaying the Red and Green channel on OMAP3530 (Pandora). H. Nikolaus Schaller (3): omapdrm: fix compatible string for td028ttec1 DTS: GTA04: fix panel compatibility string DTS: Pandora: fix panel compatibility string Laurent Pinchart (1): drm: omapdrm: Fix DPI on platforms using the DSI VDDS .../display/panel/{toppoly,td028ttec1.txt => tpo,td028ttec1.txt} | 4 ++-- arch/arm/boot/dts/omap3-gta04.dtsi | 2 +- arch/arm/boot/dts/omap3-pandora-common.dtsi | 2 +- drivers/gpu/drm/omapdrm/displays/panel-tpo-td028ttec1.c | 4 ++-- drivers/gpu/drm/omapdrm/dss/dpi.c | 4 ++-- drivers/video/fbdev/omap2/omapfb/displays/panel-tpo-td028ttec1.c | 4 +++- 6 files changed, 11 insertions(+), 9 deletions(-) rename Documentation/devicetree/bindings/display/panel/{toppoly,td028ttec1.txt => tpo,td028ttec1.txt} (84%)