diff mbox series

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

Message ID 20240729205726.7923-1-robdclark@gmail.com
State New
Headers show
Series [v3,1/2] dt-bindings: display: panel: samsung,atna45dc02: Document ATNA45DC02 | expand

Commit Message

Rob Clark July 29, 2024, 8:57 p.m. UTC
From: Rob Clark <robdclark@chromium.org>

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

Signed-off-by: Rob Clark <robdclark@chromium.org>
Acked-by: Conor Dooley <conor.dooley@microchip.com>
---
 .../bindings/display/panel/samsung,atna33xc20.yaml       | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

Comments

Stephan Gerhold July 30, 2024, 10 a.m. UTC | #1
On Mon, Jul 29, 2024 at 01:57:25PM -0700, Rob Clark wrote:
> From: Rob Clark <robdclark@chromium.org>
> 
> Use the correct panel compatible, and wire up enable-gpio.  It is wired
> up in the same way as the x1e80100-crd.
> 
> Signed-off-by: Rob Clark <robdclark@chromium.org>
> Reviewed-by: Abel Vesa <abel.vesa@linaro.org>
> ---
> 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(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/x1e80100-lenovo-yoga-slim7x.dts b/arch/arm64/boot/dts/qcom/x1e80100-lenovo-yoga-slim7x.dts
> index 70977e9fa266..6f8d52c368a4 100644
> --- a/arch/arm64/boot/dts/qcom/x1e80100-lenovo-yoga-slim7x.dts
> +++ b/arch/arm64/boot/dts/qcom/x1e80100-lenovo-yoga-slim7x.dts
> [...]
> @@ -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
Stephan Gerhold Aug. 2, 2024, 10:26 a.m. UTC | #2
On Tue, Jul 30, 2024 at 07:26:27AM -0700, Rob Clark wrote:
> On Tue, Jul 30, 2024 at 3:00 AM Stephan Gerhold
> <stephan.gerhold@linaro.org> wrote:
> >
> > On Mon, Jul 29, 2024 at 01:57:25PM -0700, Rob Clark wrote:
> > > From: Rob Clark <robdclark@chromium.org>
> > >
> > > Use the correct panel compatible, and wire up enable-gpio.  It is wired
> > > up in the same way as the x1e80100-crd.
> > >
> > > Signed-off-by: Rob Clark <robdclark@chromium.org>
> > > Reviewed-by: Abel Vesa <abel.vesa@linaro.org>
> > > ---
> > > 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(-)
> > >
> > > diff --git a/arch/arm64/boot/dts/qcom/x1e80100-lenovo-yoga-slim7x.dts b/arch/arm64/boot/dts/qcom/x1e80100-lenovo-yoga-slim7x.dts
> > > index 70977e9fa266..6f8d52c368a4 100644
> > > --- a/arch/arm64/boot/dts/qcom/x1e80100-lenovo-yoga-slim7x.dts
> > > +++ b/arch/arm64/boot/dts/qcom/x1e80100-lenovo-yoga-slim7x.dts
> > > [...]
> > > @@ -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...
> 
> :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
Doug Anderson Aug. 6, 2024, 5:52 p.m. UTC | #3
Hi,

On Wed, Jul 31, 2024 at 4:39 PM Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Mon, Jul 29, 2024 at 1:57 PM Rob Clark <robdclark@gmail.com> wrote:
> >
> > From: Rob Clark <robdclark@chromium.org>
> >
> > The Samsung ATNA45DC02 panel is an AMOLED eDP panel, similar to the
> > existing ATNA45AF01 and ATNA33XC20 panel but with a higher resolution.
> >
> > Signed-off-by: Rob Clark <robdclark@chromium.org>
> > Acked-by: Conor Dooley <conor.dooley@microchip.com>
> > ---
> >  .../bindings/display/panel/samsung,atna33xc20.yaml       | 9 ++++++---
> >  1 file changed, 6 insertions(+), 3 deletions(-)
>
> Reviewed-by: Douglas Anderson <dianders@chromium.org>
>
> 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/20240715-x1e80100-crd-backlight-v2-1-31b7f2f658a3@linaro.org

Landed in drm-misc-fixes.

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

-Doug
Rob Herring (Arm) Aug. 8, 2024, 6:14 p.m. UTC | #4
On Tue, Aug 6, 2024 at 11:52 AM Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Wed, Jul 31, 2024 at 4:39 PM Doug Anderson <dianders@chromium.org> wrote:
> >
> > Hi,
> >
> > On Mon, Jul 29, 2024 at 1:57 PM Rob Clark <robdclark@gmail.com> wrote:
> > >
> > > From: Rob Clark <robdclark@chromium.org>
> > >
> > > The Samsung ATNA45DC02 panel is an AMOLED eDP panel, similar to the
> > > existing ATNA45AF01 and ATNA33XC20 panel but with a higher resolution.
> > >
> > > Signed-off-by: Rob Clark <robdclark@chromium.org>
> > > Acked-by: Conor Dooley <conor.dooley@microchip.com>
> > > ---
> > >  .../bindings/display/panel/samsung,atna33xc20.yaml       | 9 ++++++---
> > >  1 file changed, 6 insertions(+), 3 deletions(-)
> >
> > Reviewed-by: Douglas Anderson <dianders@chromium.org>
> >
> > 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/20240715-x1e80100-crd-backlight-v2-1-31b7f2f658a3@linaro.org
>
> 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
Doug Anderson Aug. 8, 2024, 6:45 p.m. UTC | #5
Hi,

On Thu, Aug 8, 2024 at 11:14 AM Rob Herring <robh@kernel.org> wrote:
>
> > > > From: Rob Clark <robdclark@chromium.org>
> > > >
> > > > The Samsung ATNA45DC02 panel is an AMOLED eDP panel, similar to the
> > > > existing ATNA45AF01 and ATNA33XC20 panel but with a higher resolution.
> > > >
> > > > Signed-off-by: Rob Clark <robdclark@chromium.org>
> > > > Acked-by: Conor Dooley <conor.dooley@microchip.com>
> > > > ---
> > > >  .../bindings/display/panel/samsung,atna33xc20.yaml       | 9 ++++++---
> > > >  1 file changed, 6 insertions(+), 3 deletions(-)
> > >
> > > Reviewed-by: Douglas Anderson <dianders@chromium.org>
> > >
> > > 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/20240715-x1e80100-crd-backlight-v2-1-31b7f2f658a3@linaro.org
> >
> > 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.

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
Dmitry Baryshkov Sept. 26, 2024, 7:45 a.m. UTC | #6
On Tue, Jul 30, 2024 at 12:00:13PM GMT, Stephan Gerhold wrote:
> On Mon, Jul 29, 2024 at 01:57:25PM -0700, Rob Clark wrote:
> > From: Rob Clark <robdclark@chromium.org>
> > 
> > Use the correct panel compatible, and wire up enable-gpio.  It is wired
> > up in the same way as the x1e80100-crd.
> > 
> > Signed-off-by: Rob Clark <robdclark@chromium.org>
> > Reviewed-by: Abel Vesa <abel.vesa@linaro.org>
> > ---
> > 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(-)
> > 
> > diff --git a/arch/arm64/boot/dts/qcom/x1e80100-lenovo-yoga-slim7x.dts b/arch/arm64/boot/dts/qcom/x1e80100-lenovo-yoga-slim7x.dts
> > index 70977e9fa266..6f8d52c368a4 100644
> > --- a/arch/arm64/boot/dts/qcom/x1e80100-lenovo-yoga-slim7x.dts
> > +++ b/arch/arm64/boot/dts/qcom/x1e80100-lenovo-yoga-slim7x.dts
> > [...]
> > @@ -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...

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

Patch

diff --git a/Documentation/devicetree/bindings/display/panel/samsung,atna33xc20.yaml b/Documentation/devicetree/bindings/display/panel/samsung,atna33xc20.yaml
index 5192c93fbd67..87c601bcf20a 100644
--- a/Documentation/devicetree/bindings/display/panel/samsung,atna33xc20.yaml
+++ b/Documentation/devicetree/bindings/display/panel/samsung,atna33xc20.yaml
@@ -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