| Message ID | 1387469182-14398-6-git-send-email-treding@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 12/19/2013 09:06 AM, Thierry Reding wrote: > Venice2 has a 12.9" (2560x1700) panel connected to the eDP output of the > Tegra124. The panel has an EDID to describe the video timings but needs > a few extra nodes to get the backlight to come up. I started with next-20131219, merged your latest drm/for-next branch, and this doesn't seem to work. With Laxman's regulator patch applied and your 1/10 dropped since it's a duplicate, the LCD backlight doesn't light up. I extracted the following from this patch to fix Laxman's regulator definitions: gpio = <&gpio TEGRA_GPIO(P, 2) GPIO_ACTIVE_LOW>; + enable-active-high; }; and the backlight does light up, but that's all. With Laxman's regulator patch reverted and your patch 1/10 applied to replace it, I see the backlight working without having to manually fix up the device tree. However, that's still all; nothing is actually displayed. Again, can you please rebase this whole series on the latest Tegra for-3.14/dt and sort out the issues? Thanks.
On Thu, Dec 19, 2013 at 02:40:23PM -0700, Stephen Warren wrote: > On 12/19/2013 09:06 AM, Thierry Reding wrote: > > Venice2 has a 12.9" (2560x1700) panel connected to the eDP output of the > > Tegra124. The panel has an EDID to describe the video timings but needs > > a few extra nodes to get the backlight to come up. > > I started with next-20131219, merged your latest drm/for-next branch, > and this doesn't seem to work. > > With Laxman's regulator patch applied and your 1/10 dropped since it's a > duplicate, the LCD backlight doesn't light up. I extracted the following > from this patch to fix Laxman's regulator definitions: > > gpio = <&gpio TEGRA_GPIO(P, 2) GPIO_ACTIVE_LOW>; > + enable-active-high; > }; > > and the backlight does light up, but that's all. > > With Laxman's regulator patch reverted and your patch 1/10 applied to > replace it, I see the backlight working without having to manually fix > up the device tree. However, that's still all; nothing is actually > displayed. Nothing's displayed because the driver isn't actually there yet. It's still stuck in internal review for some reason. I should've probably been explicit about that. > Again, can you please rebase this whole series on the latest Tegra > for-3.14/dt and sort out the issues? Thanks. Grmpf... yes, I suppose I'll go do that then. That's exactly the reason why I said the other day that we shouldn't be adding regulators willy-nilly without any means of actually testing. We've seen this happen on Dalmore before, and it's now happening with Venice2. The same applies to pinmux. If we keep having to correct the DTS files because it was all applied at once "to avoid churn" we're not actually gaining anything. Thierry
On 12/20/2013 03:20 AM, Thierry Reding wrote: > On Thu, Dec 19, 2013 at 02:40:23PM -0700, Stephen Warren wrote: >> On 12/19/2013 09:06 AM, Thierry Reding wrote: >>> Venice2 has a 12.9" (2560x1700) panel connected to the eDP output of the >>> Tegra124. The panel has an EDID to describe the video timings but needs >>> a few extra nodes to get the backlight to come up. >> >> I started with next-20131219, merged your latest drm/for-next branch, >> and this doesn't seem to work. >> >> With Laxman's regulator patch applied and your 1/10 dropped since it's a >> duplicate, the LCD backlight doesn't light up. I extracted the following >> from this patch to fix Laxman's regulator definitions: >> >> gpio = <&gpio TEGRA_GPIO(P, 2) GPIO_ACTIVE_LOW>; >> + enable-active-high; >> }; >> >> and the backlight does light up, but that's all. >> >> With Laxman's regulator patch reverted and your patch 1/10 applied to >> replace it, I see the backlight working without having to manually fix >> up the device tree. However, that's still all; nothing is actually >> displayed. > > Nothing's displayed because the driver isn't actually there yet. It's > still stuck in internal review for some reason. > > I should've probably been explicit about that. > >> Again, can you please rebase this whole series on the latest Tegra >> for-3.14/dt and sort out the issues? Thanks. > > Grmpf... yes, I suppose I'll go do that then. That's exactly the reason > why I said the other day that we shouldn't be adding regulators > willy-nilly without any means of actually testing. > > We've seen this happen on Dalmore before, and it's now happening with > Venice2. The same applies to pinmux. If we keep having to correct the > DTS files because it was all applied at once "to avoid churn" we're not > actually gaining anything. Sorry. My original thoughts were that the schematics and specifications are all completely available (internally) for these boards, as is a complete known-working/tested DT for the board, so it's a relatively simple exercise to take that and upstream it. As such, if we just did all that in one go, it'd reduce churn by making a single patch to do the whole thing once, rather than having lots of patches that build the configuration up bit-by-bit. There's also the issue that we really do need to set up the complete pinctrl configuration once up-front, to avoid conflicts due to the same HW block being routed to multiple sets of pins. That's what I was thinking anyway. Evidently, I was wrong:-( Sorry.
diff --git a/arch/arm/boot/dts/tegra124-venice2.dts b/arch/arm/boot/dts/tegra124-venice2.dts index 7a4d5e23ddf4..b435cb5ec664 100644 --- a/arch/arm/boot/dts/tegra124-venice2.dts +++ b/arch/arm/boot/dts/tegra124-venice2.dts @@ -10,6 +10,20 @@ reg = <0x80000000 0x80000000>; }; + host1x@50000000 { + sor@54540000 { + status = "okay"; + + nvidia,dpaux = <&dpaux>; + nvidia,panel = <&panel>; + }; + + dpaux: dpaux@545c0000 { + vdd-supply = <&vdd_edp>; + status = "okay"; + }; + }; + pinmux: pinmux@70000868 { pinctrl-names = "default"; pinctrl-0 = <&pinmux_default>; @@ -594,6 +608,17 @@ }; }; + backlight: backlight { + compatible = "pwm-backlight"; + + enable-gpios = <&gpio TEGRA_GPIO(H, 2) GPIO_ACTIVE_HIGH>; + power-supply = <&vdd_led>; + pwms = <&pwm 1 1000000>; + + brightness-levels = <0 4 8 16 32 64 128 255>; + default-brightness-level = <6>; + }; + clocks { compatible = "simple-bus"; #address-cells = <1>; @@ -607,6 +632,13 @@ }; }; + panel: panel { + compatible = "lg,lp129qe", "simple-panel"; + + backlight = <&backlight>; + ddc-i2c-bus = <&dpaux>; + }; + regulators { compatible = "simple-bus"; #address-cells = <1>; @@ -617,6 +649,26 @@ reg = <0>; regulator-name = "sys_5v"; }; + + vdd_edp: regulator@1 { + compatible = "regulator-fixed"; + reg = <1>; + regulator-name = "avdd_lcd"; + regulator-min-microvolt = <3300000>; + regulator-max-microvolt = <3300000>; + gpio = <&pmic 4 0 GPIO_ACTIVE_HIGH>; + enable-active-high; + }; + + vdd_led: regulator@2 { + compatible = "regulator-fixed"; + reg = <2>; + regulator-name = "vdd_led"; + regulator-min-microvolt = <45000000>; + regulator-max-microvolt = <45000000>; + gpio = <&gpio TEGRA_GPIO(P, 2) 0 GPIO_ACTIVE_HIGH>; + enable-active-high; + }; }; sound {
Venice2 has a 12.9" (2560x1700) panel connected to the eDP output of the Tegra124. The panel has an EDID to describe the video timings but needs a few extra nodes to get the backlight to come up. Signed-off-by: Thierry Reding <treding@nvidia.com> --- arch/arm/boot/dts/tegra124-venice2.dts | 52 ++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+)