| Message ID | 20220929170502.1034040-5-diogo.ivo@tecnico.ulisboa.pt |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add JDI LPM102A188A display panel support | expand |
On 29/09/2022 19:05, Diogo Ivo wrote: > The Google Pixel C has a JDI LPM102A188A display panel. Add a > DT node for it. Tested on Pixel C. > > Signed-off-by: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt> > --- > arch/arm64/boot/dts/nvidia/tegra210-smaug.dts | 72 +++++++++++++++++++ > 1 file changed, 72 insertions(+) > > diff --git a/arch/arm64/boot/dts/nvidia/tegra210-smaug.dts b/arch/arm64/boot/dts/nvidia/tegra210-smaug.dts > index 20d092812984..271ef70747f1 100644 > --- a/arch/arm64/boot/dts/nvidia/tegra210-smaug.dts > +++ b/arch/arm64/boot/dts/nvidia/tegra210-smaug.dts > @@ -31,6 +31,39 @@ memory { > }; > > host1x@50000000 { > + dc@54200000 { > + status = "okay"; You should override by labels, not by full path. > + }; > + > + dsia: dsi@54300000 { > + avdd-dsi-csi-supply = <&vdd_dsi_csi>; > + nvidia,boot-on; > + status = "okay"; > + > + link2: panel@0 { > + compatible = "jdi,lpm102a188a"; > + reg = <0>; > + }; > + }; > + > + dsib: dsi@54400000 { > + avdd-dsi-csi-supply = <&vdd_dsi_csi>; > + nvidia,ganged-mode = <&dsia>; > + nvidia,boot-on; > + status = "okay"; > + > + link1: panel@0 { > + compatible = "jdi,lpm102a188a"; > + reg = <0>; > + power-supply = <&pplcd_vdd>; > + ddi-supply = <&pp1800_lcdio>; > + enable-gpios = <&gpio TEGRA_GPIO(V, 1) GPIO_ACTIVE_HIGH>; > + reset-gpios = <&gpio TEGRA_GPIO(V, 2) GPIO_ACTIVE_LOW>; > + link2 = <&link2>; > + backlight = <&backlight>; > + }; > + }; > + > dpaux: dpaux@545c0000 { > status = "okay"; > }; > @@ -1627,6 +1660,37 @@ nau8825@1a { > status = "okay"; > }; > > + backlight: lp8557-backlight@2c { Node names should be generic: backlight https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation > + compatible = "ti,lp8557"; > + reg = <0x2c>; > + power-supply = <&pplcd_vdd>; > + enable-supply = <&pp1800_lcdio>; > + bl-name = "lp8557-backlight"; > + dev-ctrl = /bits/ 8 <0x01>; > + init-brt = /bits/ 8 <0x80>; > + > + /* Full scale current, 20mA */ > + rom_11h { No underscores in node names, unless something requires it? > + rom-addr = /bits/ 8 <0x11>; > + rom-val = /bits/ 8 <0x05>; > + }; Best regards, Krzysztof
On Fri, Sep 30, 2022 at 12:51:07PM +0200, Krzysztof Kozlowski wrote: > On 29/09/2022 19:05, Diogo Ivo wrote: > > The Google Pixel C has a JDI LPM102A188A display panel. Add a > > DT node for it. Tested on Pixel C. > > > > Signed-off-by: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt> > > --- > > arch/arm64/boot/dts/nvidia/tegra210-smaug.dts | 72 +++++++++++++++++++ > > 1 file changed, 72 insertions(+) > > > > diff --git a/arch/arm64/boot/dts/nvidia/tegra210-smaug.dts b/arch/arm64/boot/dts/nvidia/tegra210-smaug.dts > > index 20d092812984..271ef70747f1 100644 > > --- a/arch/arm64/boot/dts/nvidia/tegra210-smaug.dts > > +++ b/arch/arm64/boot/dts/nvidia/tegra210-smaug.dts > > @@ -31,6 +31,39 @@ memory { > > }; > > > > host1x@50000000 { > > + dc@54200000 { > > + status = "okay"; > > You should override by labels, not by full path. Why exactly is that? I've always stayed away from that (and asked others not to do so, at least on Tegra) because I find it impossible to parse for my human brain. Replicating the original full hierarchy makes it much more obvious to me where the changes are happening than the spaghetti-like mess that you get from overriding by label reference. Thierry
On 30/09/2022 13:15, Thierry Reding wrote: > On Fri, Sep 30, 2022 at 12:51:07PM +0200, Krzysztof Kozlowski wrote: >> On 29/09/2022 19:05, Diogo Ivo wrote: >>> The Google Pixel C has a JDI LPM102A188A display panel. Add a >>> DT node for it. Tested on Pixel C. >>> >>> Signed-off-by: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt> >>> --- >>> arch/arm64/boot/dts/nvidia/tegra210-smaug.dts | 72 +++++++++++++++++++ >>> 1 file changed, 72 insertions(+) >>> >>> diff --git a/arch/arm64/boot/dts/nvidia/tegra210-smaug.dts b/arch/arm64/boot/dts/nvidia/tegra210-smaug.dts >>> index 20d092812984..271ef70747f1 100644 >>> --- a/arch/arm64/boot/dts/nvidia/tegra210-smaug.dts >>> +++ b/arch/arm64/boot/dts/nvidia/tegra210-smaug.dts >>> @@ -31,6 +31,39 @@ memory { >>> }; >>> >>> host1x@50000000 { >>> + dc@54200000 { >>> + status = "okay"; >> >> You should override by labels, not by full path. > > Why exactly is that? I've always stayed away from that (and asked others > not to do so, at least on Tegra) because I find it impossible to parse > for my human brain. Replicating the original full hierarchy makes it > much more obvious to me where the changes are happening than the > spaghetti-like mess that you get from overriding by label reference. Sure, it's entirely up to you. I forgot your preference. But it is a really nice way to have duplicated nodes and mistakes (which happen from time to time). Best regards, Krzysztof
On Fri, Sep 30, 2022 at 01:20:50PM +0200, Krzysztof Kozlowski wrote: > On 30/09/2022 13:15, Thierry Reding wrote: > > On Fri, Sep 30, 2022 at 12:51:07PM +0200, Krzysztof Kozlowski wrote: > >> On 29/09/2022 19:05, Diogo Ivo wrote: > >>> The Google Pixel C has a JDI LPM102A188A display panel. Add a > >>> DT node for it. Tested on Pixel C. > >>> > >>> Signed-off-by: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt> > >>> --- > >>> arch/arm64/boot/dts/nvidia/tegra210-smaug.dts | 72 +++++++++++++++++++ > >>> 1 file changed, 72 insertions(+) > >>> > >>> diff --git a/arch/arm64/boot/dts/nvidia/tegra210-smaug.dts b/arch/arm64/boot/dts/nvidia/tegra210-smaug.dts > >>> index 20d092812984..271ef70747f1 100644 > >>> --- a/arch/arm64/boot/dts/nvidia/tegra210-smaug.dts > >>> +++ b/arch/arm64/boot/dts/nvidia/tegra210-smaug.dts > >>> @@ -31,6 +31,39 @@ memory { > >>> }; > >>> > >>> host1x@50000000 { > >>> + dc@54200000 { > >>> + status = "okay"; > >> > >> You should override by labels, not by full path. > > > > Why exactly is that? I've always stayed away from that (and asked others > > not to do so, at least on Tegra) because I find it impossible to parse > > for my human brain. Replicating the original full hierarchy makes it > > much more obvious to me where the changes are happening than the > > spaghetti-like mess that you get from overriding by label reference. > > Sure, it's entirely up to you. I forgot your preference. > > But it is a really nice way to have duplicated nodes and mistakes (which > happen from time to time). We could have a schema or dtc check for that. We already warn for duplicate unit-addresses which would catch some typos. Checking for a node with only 'status' would probably work when that's the only addition. Maybe status without a compatible would be better? We also check for nodes without a specific schema, but child nodes in schemas aren't handled. Rob
On 30/09/2022 23:14, Rob Herring wrote: >>>>> + dc@54200000 { >>>>> + status = "okay"; >>>> >>>> You should override by labels, not by full path. >>> >>> Why exactly is that? I've always stayed away from that (and asked others >>> not to do so, at least on Tegra) because I find it impossible to parse >>> for my human brain. Replicating the original full hierarchy makes it >>> much more obvious to me where the changes are happening than the >>> spaghetti-like mess that you get from overriding by label reference. >> >> Sure, it's entirely up to you. I forgot your preference. >> >> But it is a really nice way to have duplicated nodes and mistakes (which >> happen from time to time). > > We could have a schema or dtc check for that. We already warn for > duplicate unit-addresses which would catch some typos. Checking for a > node with only 'status' would probably work when that's the only > addition. Maybe status without a compatible would be better? We also > check for nodes without a specific schema, but child nodes in schemas > aren't handled. Usually these are overrides of few properties and status=okay, so looking for nodes without a compatible would work. Except for all the cases where we do not have schema yet... Best regards, Krzysztof
diff --git a/arch/arm64/boot/dts/nvidia/tegra210-smaug.dts b/arch/arm64/boot/dts/nvidia/tegra210-smaug.dts index 20d092812984..271ef70747f1 100644 --- a/arch/arm64/boot/dts/nvidia/tegra210-smaug.dts +++ b/arch/arm64/boot/dts/nvidia/tegra210-smaug.dts @@ -31,6 +31,39 @@ memory { }; host1x@50000000 { + dc@54200000 { + status = "okay"; + }; + + dsia: dsi@54300000 { + avdd-dsi-csi-supply = <&vdd_dsi_csi>; + nvidia,boot-on; + status = "okay"; + + link2: panel@0 { + compatible = "jdi,lpm102a188a"; + reg = <0>; + }; + }; + + dsib: dsi@54400000 { + avdd-dsi-csi-supply = <&vdd_dsi_csi>; + nvidia,ganged-mode = <&dsia>; + nvidia,boot-on; + status = "okay"; + + link1: panel@0 { + compatible = "jdi,lpm102a188a"; + reg = <0>; + power-supply = <&pplcd_vdd>; + ddi-supply = <&pp1800_lcdio>; + enable-gpios = <&gpio TEGRA_GPIO(V, 1) GPIO_ACTIVE_HIGH>; + reset-gpios = <&gpio TEGRA_GPIO(V, 2) GPIO_ACTIVE_LOW>; + link2 = <&link2>; + backlight = <&backlight>; + }; + }; + dpaux: dpaux@545c0000 { status = "okay"; }; @@ -1627,6 +1660,37 @@ nau8825@1a { status = "okay"; }; + backlight: lp8557-backlight@2c { + compatible = "ti,lp8557"; + reg = <0x2c>; + power-supply = <&pplcd_vdd>; + enable-supply = <&pp1800_lcdio>; + bl-name = "lp8557-backlight"; + dev-ctrl = /bits/ 8 <0x01>; + init-brt = /bits/ 8 <0x80>; + + /* Full scale current, 20mA */ + rom_11h { + rom-addr = /bits/ 8 <0x11>; + rom-val = /bits/ 8 <0x05>; + }; + /* Frequency = 4.9kHz, magic undocumented val */ + rom_12h { + rom-addr = /bits/ 8 <0x12>; + rom-val = /bits/ 8 <0x29>; + }; + /* Boost freq = 1MHz, BComp option = 1 */ + rom_13h { + rom-addr = /bits/ 8 <0x13>; + rom-val = /bits/ 8 <0x03>; + }; + /* 4V OV, 6 output LED string enabled */ + rom_14h { + rom-addr = /bits/ 8 <0x14>; + rom-val = /bits/ 8 <0xbf>; + }; + }; + audio-codec@2d { compatible = "realtek,rt5677"; reg = <0x2d>; @@ -1908,4 +1972,12 @@ usbc_vbus: regulator-usbc-vbus { regulator-min-microvolt = <5000000>; regulator-max-microvolt = <5000000>; }; + + vdd_dsi_csi: regulator-vdd-dsi-csi { + compatible = "regulator-fixed"; + regulator-name = "AVDD_DSI_CSI_1V2"; + regulator-min-microvolt = <1200000>; + regulator-max-microvolt = <1200000>; + vin-supply = <&pp1200_avdd>; + }; };
The Google Pixel C has a JDI LPM102A188A display panel. Add a DT node for it. Tested on Pixel C. Signed-off-by: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt> --- arch/arm64/boot/dts/nvidia/tegra210-smaug.dts | 72 +++++++++++++++++++ 1 file changed, 72 insertions(+)