Skip to contentSkip to conversation

dt-bindings: display: panel: samsung,atna45dc02: Document ATNA45DC02

PATCH In review Rob Clark 5 participants 10 messages Jul 29, 2024 – Sep 26, 2024

Subsystems (6)
lore
Share & tools
lore

Rob Clark

author
#0 lore raw

PATCH v3 1/2 dt-bindings: display: panel: samsung,atna45dc02: Document ATNA45DC02

From: Rob Clark <[email protected]>

The Samsung ATNA45DC02 panel is an AMOLED eDP panel, similar to the existing ATNA45AF01 and ATNA33XC20 panel but with a higher resolution.

---
 .../bindings/display/panel/samsung,atna33xc20.yaml       | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)
Documentation/devicetree/bindings/display/panel/samsung,atna33xc20.yaml +6 -3
index 5192c93fbd67..87c601bcf20a 100644
@@ -17,10 +17,13 @@ properties:
     oneOf:
       # Samsung 13.3" FHD (1920x1080 pixels) eDP AMOLED panel
       - const: samsung,atna33xc20
-      # Samsung 14.5" WQXGA+ (2880x1800 pixels) eDP AMOLED panel
       - items:
-          - const: samsung,atna45af01
-          - const: samsung,atna33xc20
+        - enum:
+          # Samsung 14.5" WQXGA+ (2880x1800 pixels) eDP AMOLED panel
+          - samsung,atna45af01
+          # Samsung 14.5" 3K (2944x1840 pixels) eDP AMOLED panel
+          - samsung,atna45dc02
+        - const: samsung,atna33xc20
 
   enable-gpios: true
   port: true
Signature · 1 line
2.45.2
author
#1 lore raw

PATCH v3 2/2 arm64: dts: qcom: x1e80100-yoga: Update panel bindings

From: Rob Clark <[email protected]>

Use the correct panel compatible, and wire up enable-gpio. It is wired up in the same way as the x1e80100-crd.

---
Note that the power-source differs from the CRD. Figured that out by diffing $debugfs/gpio.

 .../dts/qcom/x1e80100-lenovo-yoga-slim7x.dts    | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)
arch/arm64/boot/dts/qcom/x1e80100-lenovo-yoga-slim7x.dts +15 -2
index 70977e9fa266..6f8d52c368a4 100644
@@ -190,7 +190,6 @@ vreg_edp_3p3: regulator-edp-3p3 {
 		pinctrl-0 = <&edp_reg_en>;
 		pinctrl-names = "default";
 
-		regulator-always-on;
 		regulator-boot-on;
 	};
 
@@ -592,9 +591,13 @@ &mdss_dp3 {
 
 	aux-bus {
 		panel {
-			compatible = "edp-panel";
+			compatible = "samsung,atna45dc02", "samsung,atna33xc20";
+			enable-gpios = <&pmc8380_3_gpios 4 GPIO_ACTIVE_HIGH>;
 			power-supply = <&vreg_edp_3p3>;
 
+			pinctrl-0 = <&edp_bl_en>;
+			pinctrl-names = "default";
+
 			port {
 				edp_panel_in: endpoint {
 					remote-endpoint = <&mdss_dp3_out>;
@@ -669,6 +672,16 @@ &pcie6a_phy {
 	status = "okay";
 };
 
+&pmc8380_3_gpios {
+	edp_bl_en: edp-bl-en-state {
+		pins = "gpio4";
+		function = "normal";
+		power-source = <0>; /* 1.8V */
+		input-disable;
+		output-enable;
+	};
+};
+
 &qupv3_0 {
 	status = "okay";
 };
Signature · 1 line
2.45.2
#2 lore raw

Re: [PATCH v3 2/2] arm64: dts: qcom: x1e80100-yoga: Update panel bindings

Quoted 20 lines · Rob Clark

From: Rob Clark <[email protected]>

Use the correct panel compatible, and wire up enable-gpio. It is wired up in the same way as the x1e80100-crd.

---
Note that the power-source differs from the CRD. Figured that out by diffing $debugfs/gpio.

 .../dts/qcom/x1e80100-lenovo-yoga-slim7x.dts    | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)
arch/arm64/boot/dts/qcom/x1e80100-lenovo-yoga-slim7x.dts +0 -0
index 70977e9fa266..6f8d52c368a4 100644
[...]

arch/arm64/boot/dts/qcom/x1e80100-lenovo-yoga-slim7x.dts &pcie6a_phy { · line 679

Quoted 8 lines
arch/arm64/boot/dts/qcom/x1e80100-lenovo-yoga-slim7x.dts +5 -0
@@ -669,6 +672,16 @@ &pcie6a_phy {
 	status = "okay";
 };
 
+&pmc8380_3_gpios {
+	edp_bl_en: edp-bl-en-state {
+		pins = "gpio4";
+		function = "normal";
+		power-source = <0>; /* 1.8V */

Hm, are you sure the firmware sets it like this?

power-source = <0> is typically VPH, i.e. the main battery supply voltage or similar. This will definitely be higher than 1.8V. It seems a bit odd to use a variable voltage source for a GPIO...

Thanks,
Stephan

author
#3 lore raw

Re: [PATCH v3 2/2] arm64: dts: qcom: x1e80100-yoga: Update panel bindings

Quoted 33 lines · Stephan Gerhold
Quoted 20 lines · Rob Clark

From: Rob Clark <[email protected]>

Use the correct panel compatible, and wire up enable-gpio. It is wired up in the same way as the x1e80100-crd.

---
Note that the power-source differs from the CRD. Figured that out by diffing $debugfs/gpio.

 .../dts/qcom/x1e80100-lenovo-yoga-slim7x.dts    | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)
arch/arm64/boot/dts/qcom/x1e80100-lenovo-yoga-slim7x.dts +0 -0
index 70977e9fa266..6f8d52c368a4 100644
[...]

arch/arm64/boot/dts/qcom/x1e80100-lenovo-yoga-slim7x.dts &pcie6a_phy { · line 679

Quoted 8 lines
arch/arm64/boot/dts/qcom/x1e80100-lenovo-yoga-slim7x.dts +5 -0
@@ -669,6 +672,16 @@ &pcie6a_phy {
      status = "okay";
 };
 
+&pmc8380_3_gpios {
+     edp_bl_en: edp-bl-en-state {
+             pins = "gpio4";
+             function = "normal";
+             power-source = <0>; /* 1.8V */

Hm, are you sure the firmware sets it like this?

Quoted 3 lines

power-source = <0> is typically VPH, i.e. the main battery supply voltage or similar. This will definitely be higher than 1.8V. It seems a bit odd to use a variable voltage source for a GPIO...

:shrug:

Board level stuff is a bit outside my area of expertise, but it's what made /sys/kernel/debug/gpio match for gpiochip3 gpio4 and it's what made the panel work. Using <1> resulted in a black screen.

AFAIU Srini has a yoga 7x as well, and perhaps knows how to read ACPI better than I do, so maybe he can double check.

BR,
-R

Quoted 2 lines

Thanks,
Stephan

#4 lore raw

Re: [PATCH v3 2/2] arm64: dts: qcom: x1e80100-yoga: Update panel bindings

Quoted 39 lines · Rob Clark
Quoted 33 lines · Stephan Gerhold
Quoted 20 lines · Rob Clark

From: Rob Clark <[email protected]>

Use the correct panel compatible, and wire up enable-gpio. It is wired up in the same way as the x1e80100-crd.

---
Note that the power-source differs from the CRD. Figured that out by diffing $debugfs/gpio.

 .../dts/qcom/x1e80100-lenovo-yoga-slim7x.dts    | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)
arch/arm64/boot/dts/qcom/x1e80100-lenovo-yoga-slim7x.dts +0 -0
index 70977e9fa266..6f8d52c368a4 100644
[...]

arch/arm64/boot/dts/qcom/x1e80100-lenovo-yoga-slim7x.dts &pcie6a_phy { · line 679

Quoted 8 lines
arch/arm64/boot/dts/qcom/x1e80100-lenovo-yoga-slim7x.dts +5 -0
@@ -669,6 +672,16 @@ &pcie6a_phy {
      status = "okay";
 };
 
+&pmc8380_3_gpios {
+     edp_bl_en: edp-bl-en-state {
+             pins = "gpio4";
+             function = "normal";
+             power-source = <0>; /* 1.8V */

Hm, are you sure the firmware sets it like this?

Quoted 3 lines

power-source = <0> is typically VPH, i.e. the main battery supply voltage or similar. This will definitely be higher than 1.8V. It seems a bit odd to use a variable voltage source for a GPIO...

Quoted 9 lines

:shrug:

Board level stuff is a bit outside my area of expertise, but it's what made /sys/kernel/debug/gpio match for gpiochip3 gpio4 and it's what made the panel work. Using <1> resulted in a black screen.

AFAIU Srini has a yoga 7x as well, and perhaps knows how to read ACPI better than I do, so maybe he can double check.

It looks like you're right, I think ACPI also configures power-source = <0>. Can you just remove the 1.8V comment then? I'm pretty sure it's not 1.8V.

Thanks,
Stephan

#5 lore raw

Re: [PATCH v3 2/2] arm64: dts: qcom: x1e80100-yoga: Update panel bindings

In reply to Stephan Gerhold
Quoted 32 lines · Stephan Gerhold
Quoted 20 lines · Rob Clark

From: Rob Clark <[email protected]>

Use the correct panel compatible, and wire up enable-gpio. It is wired up in the same way as the x1e80100-crd.

---
Note that the power-source differs from the CRD. Figured that out by diffing $debugfs/gpio.

 .../dts/qcom/x1e80100-lenovo-yoga-slim7x.dts    | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)
arch/arm64/boot/dts/qcom/x1e80100-lenovo-yoga-slim7x.dts +0 -0
index 70977e9fa266..6f8d52c368a4 100644
[...]

arch/arm64/boot/dts/qcom/x1e80100-lenovo-yoga-slim7x.dts &pcie6a_phy { · line 679

Quoted 8 lines
arch/arm64/boot/dts/qcom/x1e80100-lenovo-yoga-slim7x.dts +5 -0
@@ -669,6 +672,16 @@ &pcie6a_phy {
 	status = "okay";
 };
 
+&pmc8380_3_gpios {
+	edp_bl_en: edp-bl-en-state {
+		pins = "gpio4";
+		function = "normal";
+		power-source = <0>; /* 1.8V */

Hm, are you sure the firmware sets it like this?

Quoted 3 lines

power-source = <0> is typically VPH, i.e. the main battery supply voltage or similar. This will definitely be higher than 1.8V. It seems a bit odd to use a variable voltage source for a GPIO...

As you've mentioned it. Please use defined values rather than just <0> for power-source property.

Signature · 2 lines
With best wishes
Dmitry
Reviewed-by on v3 1/2
#6 lore raw
In reply to Rob Clark

Hi,

Quoted 11 lines · Rob Clark

From: Rob Clark <[email protected]>

The Samsung ATNA45DC02 panel is an AMOLED eDP panel, similar to the existing ATNA45AF01 and ATNA33XC20 panel but with a higher resolution.

---
 .../bindings/display/panel/samsung,atna33xc20.yaml       | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

I'll plan to land this in drm-misc-fixes next week unless someone objects. "fixes" instead of "next" for the same reasons discussed previously [1] that the dts patch should probably be considered a fix and there's a chance that the dts patch could land in an earlier version of mainline than the bindings unless we consider the bindings a fix.

[1] https://patchwork.freedesktop.org/patch/msgid/[email protected]

#7 lore raw

Hi,

Quoted 18 lines · Doug Anderson

Hi,

Quoted 11 lines · Rob Clark

From: Rob Clark <[email protected]>

The Samsung ATNA45DC02 panel is an AMOLED eDP panel, similar to the existing ATNA45AF01 and ATNA33XC20 panel but with a higher resolution.

---
 .../bindings/display/panel/samsung,atna33xc20.yaml       | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)
Quoted 8 lines

I'll plan to land this in drm-misc-fixes next week unless someone objects. "fixes" instead of "next" for the same reasons discussed previously [1] that the dts patch should probably be considered a fix and there's a chance that the dts patch could land in an earlier version of mainline than the bindings unless we consider the bindings a fix.

[1] https://patchwork.freedesktop.org/patch/msgid/[email protected]

Landed in drm-misc-fixes.

[1/2] dt-bindings: display: panel: samsung,atna45dc02: Document ATNA45DC02

      commit: 1c4a057d01f4432704c4dc8842b6e888a91d95df

-Doug

maintainer
#8 lore raw
Quoted 29 lines · Doug Anderson

Hi,

Quoted 25 lines · Doug Anderson

Hi,

Quoted 11 lines · Rob Clark

From: Rob Clark <[email protected]>

The Samsung ATNA45DC02 panel is an AMOLED eDP panel, similar to the existing ATNA45AF01 and ATNA33XC20 panel but with a higher resolution.

---
 .../bindings/display/panel/samsung,atna33xc20.yaml       | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

I'll plan to land this in drm-misc-fixes next week unless someone objects. "fixes" instead of "next" for the same reasons discussed previously [1] that the dts patch should probably be considered a fix and there's a chance that the dts patch could land in an earlier version of mainline than the bindings unless we consider the bindings a fix.

Quoted 6 lines
Quoted 1 line

[1] https://patchwork.freedesktop.org/patch/msgid/[email protected]

Landed in drm-misc-fixes.

[1/2] dt-bindings: display: panel: samsung,atna45dc02: Document ATNA45DC02

      commit: 1c4a057d01f4432704c4dc8842b6e888a91d95df

And now warning in linux-next:

./Documentation/devicetree/bindings/display/panel/samsung,atna33xc20.yaml:21:9:
[warning] wrong indentation: expected 10 but found 8 (indentation) ./Documentation/devicetree/bindings/display/panel/samsung,atna33xc20.yaml:23:11:
[warning] wrong indentation: expected 12 but found 10 (indentation)

Please send a fix.

Rob

#9 lore raw

Hi,

Quoted 28 lines · Rob Herring
Quoted 21 lines
Quoted 21 lines
Quoted 11 lines

From: Rob Clark <[email protected]>

The Samsung ATNA45DC02 panel is an AMOLED eDP panel, similar to the existing ATNA45AF01 and ATNA33XC20 panel but with a higher resolution.

---
 .../bindings/display/panel/samsung,atna33xc20.yaml       | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

I'll plan to land this in drm-misc-fixes next week unless someone objects. "fixes" instead of "next" for the same reasons discussed previously [1] that the dts patch should probably be considered a fix and there's a chance that the dts patch could land in an earlier version of mainline than the bindings unless we consider the bindings a fix.

Quoted 6 lines
Quoted 1 line

[1] https://patchwork.freedesktop.org/patch/msgid/[email protected]

Landed in drm-misc-fixes.

[1/2] dt-bindings: display: panel: samsung,atna45dc02: Document ATNA45DC02

      commit: 1c4a057d01f4432704c4dc8842b6e888a91d95df
Quoted 8 lines

And now warning in linux-next:

./Documentation/devicetree/bindings/display/panel/samsung,atna33xc20.yaml:21:9:
[warning] wrong indentation: expected 10 but found 8 (indentation) ./Documentation/devicetree/bindings/display/panel/samsung,atna33xc20.yaml:23:11:
[warning] wrong indentation: expected 12 but found 10 (indentation)

Please send a fix.

Doh! I'm just about to hop out on vacation, but here's a fix. If someone reviews in the next 30 minutes or so I'll land it. Otherwise hopefully someone else can land...

https://lore.kernel.org/r/20240808114407.1.I099e8e9e36407a0785d846b953031d40ea71e559@changeid