diff mbox series

[1/4] dt-bindings: display: Add bindings for JDI LPM102A188A

Message ID 20220929170502.1034040-2-diogo.ivo@tecnico.ulisboa.pt
State Superseded
Headers show
Series Add JDI LPM102A188A display panel support | expand

Commit Message

Diogo Ivo Sept. 29, 2022, 5:04 p.m. UTC
The LPM102A188A is a 10.2" 2560x1800 IPS panel found in
the Google Pixel C.

Signed-off-by: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>
---
 .../display/panel/jdi,lpm102a188a.yaml        | 100 ++++++++++++++++++
 1 file changed, 100 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/panel/jdi,lpm102a188a.yaml

Comments

Krzysztof Kozlowski Sept. 30, 2022, 10:49 a.m. UTC | #1
On 29/09/2022 19:04, Diogo Ivo wrote:
> The LPM102A188A is a 10.2" 2560x1800 IPS panel found in
> the Google Pixel C.
> 


Thank you for your patch. There is something to discuss/improve.

> +  Each of the DSI channels controls a separate DSI peripheral. The peripheral
> +  driven by the first link (DSI-LINK1) is considered the primary peripheral
> +  and controls the device. The 'link2' property contains a phandle to the
> +  peripheral driven by the second link (DSI-LINK2).
> +
> +allOf:
> +  - $ref: panel-common.yaml#
> +
> +properties:
> +  compatible:
> +    const: jdi,lpm102a188a
> +
> +  reg: true
> +  enable-gpios: true
> +  reset-gpios: true
> +  power-supply: true
> +  backlight: true
> +
> +  ts-reset-gpios:
> +    maxItems: 1
> +    description: |
> +      Specifier for a GPIO connected to the touchscreen reset control signal.
> +      The reset signal is active low.

Isn't touchscreen a separate (input) device?

> +
> +  ddi-supply:
> +    description: The regulator that provides IOVCC (1.8V).
> +
> +  link2:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description: |
> +      phandle to the DSI peripheral on the secondary link. Note that the
> +      presence of this property marks the containing node as DSI-LINK1.

Best regards,
Krzysztof
Diogo Ivo Oct. 3, 2022, 5:06 p.m. UTC | #2
On Fri, Sep 30, 2022 at 12:49:31PM +0200, Krzysztof Kozlowski wrote:
> > +  ts-reset-gpios:
> > +    maxItems: 1
> > +    description: |
> > +      Specifier for a GPIO connected to the touchscreen reset control signal.
> > +      The reset signal is active low.
> 
> Isn't touchscreen a separate (input) device?

Hello, thank you for the feedback.

According to the downstream kernel's log, it seems like the panel and
the touchscreen controller are considered to be embedded in the same unit
(for example in [1]), with the touch input being transmitted via HID-over-I2C,
and since I did not find any reset gpio handling in that driver I opted to
include this reset here, unless there is a better way of going about this.

Best regards,

Diogo

[1]: https://android.googlesource.com/kernel/tegra/+/bca61c34db9f72113af058f53eeb9fbd5e69a1d0
Krzysztof Kozlowski Oct. 4, 2022, 11:05 a.m. UTC | #3
On 03/10/2022 19:06, Diogo Ivo wrote:
> On Fri, Sep 30, 2022 at 12:49:31PM +0200, Krzysztof Kozlowski wrote:
>>> +  ts-reset-gpios:
>>> +    maxItems: 1
>>> +    description: |
>>> +      Specifier for a GPIO connected to the touchscreen reset control signal.
>>> +      The reset signal is active low.
>>
>> Isn't touchscreen a separate (input) device?
> 
> Hello, thank you for the feedback.
> 
> According to the downstream kernel's log, it seems like the panel and
> the touchscreen controller are considered to be embedded in the same unit
> (for example in [1]), 

Downstream kernel is not a proof of proper description of hardware. If
downstream says orange is an apple, does it mean orange is really an
apple? No... Downstream creates a lot of junk, hacks and workarounds.

> with the touch input being transmitted via HID-over-I2C,
> and since I did not find any reset gpio handling in that driver I opted to
> include this reset here, unless there is a better way of going about this.

Instead it should be in touch screen device.

> 
> Best regards,
> 
> Diogo
> 
> [1]: https://android.googlesource.com/kernel/tegra/+/bca61c34db9f72113af058f53eeb9fbd5e69a1d0

Where is the DTS of that device?

Best regards,
Krzysztof
Diogo Ivo Oct. 4, 2022, 4:37 p.m. UTC | #4
On Tue, Oct 04, 2022 at 01:05:04PM +0200, Krzysztof Kozlowski wrote:
> On 03/10/2022 19:06, Diogo Ivo wrote:
> > On Fri, Sep 30, 2022 at 12:49:31PM +0200, Krzysztof Kozlowski wrote:
> >> Isn't touchscreen a separate (input) device?
> > 
> > Hello, thank you for the feedback.
> > 
> > According to the downstream kernel's log, it seems like the panel and
> > the touchscreen controller are considered to be embedded in the same unit
> > (for example in [1]), 
> 
> Downstream kernel is not a proof of proper description of hardware. If
> downstream says orange is an apple, does it mean orange is really an
> apple? No... Downstream creates a lot of junk, hacks and workarounds.

After some searching (which I should have done sooner, so
apologies) I came across a teardown of the Pixel C ([1], for completeness),
which incorporates this panel. Indeed a separate touch controller was found,
so it seems the downstream kernel threw me off as per your warning.

[1]: https://www.ifixit.com/Teardown/Google+Pixel+C+Teardown/62277 (Step 4)

> > with the touch input being transmitted via HID-over-I2C,
> > and since I did not find any reset gpio handling in that driver I opted to
> > include this reset here, unless there is a better way of going about this.
> 
> Instead it should be in touch screen device.

Noted, I will remove it from the binding in the next version. 

> Where is the DTS of that device?

The relevant part of the DTS can be found here:
https://android.googlesource.com/kernel/tegra/+/refs/heads/android-tegra-dragon-3.18-oreo/arch/arm64/boot/dts/tegra/tegra210-smaug.dtsi

Best regards,
Diogo
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/display/panel/jdi,lpm102a188a.yaml b/Documentation/devicetree/bindings/display/panel/jdi,lpm102a188a.yaml
new file mode 100644
index 000000000000..97f9db7152da
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/jdi,lpm102a188a.yaml
@@ -0,0 +1,100 @@ 
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/panel/jdi,lpm102a188a.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: JDI LPM102A188A 2560x1800 10.2" DSI Panel
+
+maintainers:
+  - Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>
+
+description: |
+  This panel requires a dual-channel DSI host to operate. It supports two modes:
+  - left-right: each channel drives the left or right half of the screen
+  - even-odd: each channel drives the even or odd lines of the screen
+
+  Each of the DSI channels controls a separate DSI peripheral. The peripheral
+  driven by the first link (DSI-LINK1) is considered the primary peripheral
+  and controls the device. The 'link2' property contains a phandle to the
+  peripheral driven by the second link (DSI-LINK2).
+
+allOf:
+  - $ref: panel-common.yaml#
+
+properties:
+  compatible:
+    const: jdi,lpm102a188a
+
+  reg: true
+  enable-gpios: true
+  reset-gpios: true
+  power-supply: true
+  backlight: true
+
+  ts-reset-gpios:
+    maxItems: 1
+    description: |
+      Specifier for a GPIO connected to the touchscreen reset control signal.
+      The reset signal is active low.
+
+  ddi-supply:
+    description: The regulator that provides IOVCC (1.8V).
+
+  link2:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description: |
+      phandle to the DSI peripheral on the secondary link. Note that the
+      presence of this property marks the containing node as DSI-LINK1.
+
+required:
+  - compatible
+  - reg
+
+if:
+  required:
+    - link2
+then:
+  required:
+    - power-supply
+    - ddi-supply
+    - enable-gpios
+    - reset-gpios
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/gpio/tegra-gpio.h>
+
+    dsia: dsi@54300000 {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        reg = <0x0 0x54300000 0x0 0x00040000>;
+
+        link2: panel@0 {
+            compatible = "jdi,lpm102a188a";
+            reg = <0>;
+        };
+    };
+
+    dsib: dsi@54400000{
+        #address-cells = <1>;
+        #size-cells = <0>;
+        reg = <0x0 0x54400000 0x0 0x00040000>;
+        nvidia,ganged-mode = <&dsia>;
+
+        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>;
+        };
+    };
+
+...