Re: [PATCH 1/2] dt-bindings: display: panel: Add Visionox R66451 AMOLED DSI panel bindings

2023-05-29 Thread Marijn Suijten 
On 2023-05-26 09:42:33, Neil Armstrong wrote:
> On 22/05/2023 16:51, Marijn Suijten wrote:
> > On 2023-05-22 11:05:38, Neil Armstrong wrote:
> >> On 21/05/2023 12:30, Marijn Suijten wrote:
> >>> On 2023-05-16 13:20:30, Jessica Zhang wrote:
>  Document the 1080x2340 Visionox R66451 AMOLED DSI panel bindings
> 
>  Signed-off-by: Jessica Zhang 
>  ---
> .../bindings/display/panel/visionox,r66451.yaml| 59 
>  ++
> 1 file changed, 59 insertions(+)
> 
>  diff --git 
>  a/Documentation/devicetree/bindings/display/panel/visionox,r66451.yaml 
>  b/Documentation/devicetree/bindings/display/panel/visionox,r66451.yaml
>  new file mode 100644
>  index ..6ba323683921
>  --- /dev/null
>  +++ 
>  b/Documentation/devicetree/bindings/display/panel/visionox,r66451.yaml
>  @@ -0,0 +1,59 @@
>  +# SPDX-License-Identifier: GPL-2.0-only or BSD-2-Clause
>  +%YAML 1.2
>  +---
>  +$id: http://devicetree.org/schemas/display/panel/visionox,r66451.yaml#
>  +$schema: http://devicetree.org/meta-schemas/core.yaml#
>  +
>  +title: Visionox R66451 AMOLED DSI Panel
>  +
>  +maintainers:
>  +  - Jessica Zhang 
>  +
>  +allOf:
>  +  - $ref: panel-common.yaml#
>  +
>  +properties:
>  +  compatible:
>  +const: visionox,r66451
>  +
>  +  reg:
>  +maxItems: 1
>  +description: DSI virtual channel
>  +
>  +  vddio-supply: true
>  +  vdd-supply: true
>  +  port: true
>  +  reset-gpios: true
> >>>
> >>> Normally for cmd-mode panels there is also a `disp-te` pin which is
> >>> optionally registered in dsi_host.c as GPIOD_IN, but on **ALL** my Sony
> >>> phones this breaks vsync (as in: mdp5 stops receiving the interrupt, but
> >>> we can see disp-te in /proc/interrupts then).
> >>
> >> Describing it as a gpio is wrong, it should be described as a pinctrl 
> >> state instead.
> > 
> > We defined both in our DTS, what weirdness does it cause when then
> > requested using GPIOD_IN?  It'd still be beneficial to see the vsync
> > interrupt raise in /proc/interrupts (but it's just a waste of CPU cycles
> > OTOH, this is all handled in the MDP hardware after all, so it's not
> > something I'd like to enable by default).
> 
> Sure, but it's a sw hack, the pin has a TE function which directly goes to
> the DSI logic, claiming it as a GPIO will set it as GPIO function.
> 
> On some platforms, PINMUX is only on output and input is always directed
> to all HW blocks, seems it's not the case here !

Ah that makes total sense!  The PINGROUP() is only passed this mdp_vsync
function but internally provides the gpio function as well, which it'd
have to use to read it as GPIO from the SoC-side: and this indeed seems
to "disconnect" that pin from the MDP HW block.  Thanks for mentioning
this, I totally overlooked it.

Should we document/clarify this in any way, or perhaps remove the
disp-te handling altogether (dsi_host.c doesn't use this interrupt for
anything, though we could leave it for debug purposes if describing /
wrapping it more clearly).  Downstream also sets this pin in DT but
doesn't ever request a GPIO/IRQ on it, afaik.

- Marijn

> > Anyway, this is what we ended up doing to "fix" the bug (only bias the
> > pin via pinctrl, omit the disp-te DTS property).  Thanks for confirming!
> > 
> > - Marijn
> > 
> >>
> >> Neil
> > 
> > 
>

Re: [PATCH 1/2] dt-bindings: display: panel: Add Visionox R66451 AMOLED DSI panel bindings

2023-05-26 Thread Neil Armstrong 

On 22/05/2023 16:51, Marijn Suijten wrote:

On 2023-05-22 11:05:38, Neil Armstrong wrote:

On 21/05/2023 12:30, Marijn Suijten wrote:

On 2023-05-16 13:20:30, Jessica Zhang wrote:

Document the 1080x2340 Visionox R66451 AMOLED DSI panel bindings

Signed-off-by: Jessica Zhang 
---
   .../bindings/display/panel/visionox,r66451.yaml| 59 
++
   1 file changed, 59 insertions(+)

diff --git 
a/Documentation/devicetree/bindings/display/panel/visionox,r66451.yaml 
b/Documentation/devicetree/bindings/display/panel/visionox,r66451.yaml
new file mode 100644
index ..6ba323683921
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/visionox,r66451.yaml
@@ -0,0 +1,59 @@
+# SPDX-License-Identifier: GPL-2.0-only or BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/panel/visionox,r66451.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Visionox R66451 AMOLED DSI Panel
+
+maintainers:
+  - Jessica Zhang 
+
+allOf:
+  - $ref: panel-common.yaml#
+
+properties:
+  compatible:
+const: visionox,r66451
+
+  reg:
+maxItems: 1
+description: DSI virtual channel
+
+  vddio-supply: true
+  vdd-supply: true
+  port: true
+  reset-gpios: true


Normally for cmd-mode panels there is also a `disp-te` pin which is
optionally registered in dsi_host.c as GPIOD_IN, but on **ALL** my Sony
phones this breaks vsync (as in: mdp5 stops receiving the interrupt, but
we can see disp-te in /proc/interrupts then).


Describing it as a gpio is wrong, it should be described as a pinctrl state 
instead.


We defined both in our DTS, what weirdness does it cause when then
requested using GPIOD_IN?  It'd still be beneficial to see the vsync
interrupt raise in /proc/interrupts (but it's just a waste of CPU cycles
OTOH, this is all handled in the MDP hardware after all, so it's not
something I'd like to enable by default).


Sure, but it's a sw hack, the pin has a TE function which directly goes to
the DSI logic, claiming it as a GPIO will set it as GPIO function.

On some platforms, PINMUX is only on output and input is always directed
to all HW blocks, seems it's not the case here !



Anyway, this is what we ended up doing to "fix" the bug (only bias the
pin via pinctrl, omit the disp-te DTS property).  Thanks for confirming!

- Marijn



Neil

Re: [PATCH 1/2] dt-bindings: display: panel: Add Visionox R66451 AMOLED DSI panel bindings

2023-05-22 Thread Marijn Suijten 
On 2023-05-22 11:05:38, Neil Armstrong wrote:
> On 21/05/2023 12:30, Marijn Suijten wrote:
> > On 2023-05-16 13:20:30, Jessica Zhang wrote:
> >> Document the 1080x2340 Visionox R66451 AMOLED DSI panel bindings
> >>
> >> Signed-off-by: Jessica Zhang 
> >> ---
> >>   .../bindings/display/panel/visionox,r66451.yaml| 59 
> >> ++
> >>   1 file changed, 59 insertions(+)
> >>
> >> diff --git 
> >> a/Documentation/devicetree/bindings/display/panel/visionox,r66451.yaml 
> >> b/Documentation/devicetree/bindings/display/panel/visionox,r66451.yaml
> >> new file mode 100644
> >> index ..6ba323683921
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/display/panel/visionox,r66451.yaml
> >> @@ -0,0 +1,59 @@
> >> +# SPDX-License-Identifier: GPL-2.0-only or BSD-2-Clause
> >> +%YAML 1.2
> >> +---
> >> +$id: http://devicetree.org/schemas/display/panel/visionox,r66451.yaml#
> >> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >> +
> >> +title: Visionox R66451 AMOLED DSI Panel
> >> +
> >> +maintainers:
> >> +  - Jessica Zhang 
> >> +
> >> +allOf:
> >> +  - $ref: panel-common.yaml#
> >> +
> >> +properties:
> >> +  compatible:
> >> +const: visionox,r66451
> >> +
> >> +  reg:
> >> +maxItems: 1
> >> +description: DSI virtual channel
> >> +
> >> +  vddio-supply: true
> >> +  vdd-supply: true
> >> +  port: true
> >> +  reset-gpios: true
> > 
> > Normally for cmd-mode panels there is also a `disp-te` pin which is
> > optionally registered in dsi_host.c as GPIOD_IN, but on **ALL** my Sony
> > phones this breaks vsync (as in: mdp5 stops receiving the interrupt, but
> > we can see disp-te in /proc/interrupts then).
> 
> Describing it as a gpio is wrong, it should be described as a pinctrl state 
> instead.

We defined both in our DTS, what weirdness does it cause when then
requested using GPIOD_IN?  It'd still be beneficial to see the vsync
interrupt raise in /proc/interrupts (but it's just a waste of CPU cycles
OTOH, this is all handled in the MDP hardware after all, so it's not
something I'd like to enable by default).

Anyway, this is what we ended up doing to "fix" the bug (only bias the
pin via pinctrl, omit the disp-te DTS property).  Thanks for confirming!

- Marijn

> 
> Neil

Re: [PATCH 1/2] dt-bindings: display: panel: Add Visionox R66451 AMOLED DSI panel bindings

2023-05-22 Thread Neil Armstrong 

On 21/05/2023 12:30, Marijn Suijten wrote:

On 2023-05-16 13:20:30, Jessica Zhang wrote:

Document the 1080x2340 Visionox R66451 AMOLED DSI panel bindings

Signed-off-by: Jessica Zhang 
---
  .../bindings/display/panel/visionox,r66451.yaml| 59 ++
  1 file changed, 59 insertions(+)

diff --git 
a/Documentation/devicetree/bindings/display/panel/visionox,r66451.yaml 
b/Documentation/devicetree/bindings/display/panel/visionox,r66451.yaml
new file mode 100644
index ..6ba323683921
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/visionox,r66451.yaml
@@ -0,0 +1,59 @@
+# SPDX-License-Identifier: GPL-2.0-only or BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/panel/visionox,r66451.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Visionox R66451 AMOLED DSI Panel
+
+maintainers:
+  - Jessica Zhang 
+
+allOf:
+  - $ref: panel-common.yaml#
+
+properties:
+  compatible:
+const: visionox,r66451
+
+  reg:
+maxItems: 1
+description: DSI virtual channel
+
+  vddio-supply: true
+  vdd-supply: true
+  port: true
+  reset-gpios: true


Normally for cmd-mode panels there is also a `disp-te` pin which is
optionally registered in dsi_host.c as GPIOD_IN, but on **ALL** my Sony
phones this breaks vsync (as in: mdp5 stops receiving the interrupt, but
we can see disp-te in /proc/interrupts then).


Describing it as a gpio is wrong, it should be described as a pinctrl state 
instead.

Neil



- Marijn


+additionalProperties: false
+
+required:
+  - compatible
+  - reg
+  - vddio-supply
+  - vdd-supply
+  - reset-gpios
+  - port
+
+examples:
+  - |
+#include 
+dsi {
+#address-cells = <1>;
+#size-cells = <0>;
+panel@0 {
+compatible = "visionox,r66451";
+reg = <0>;
+vddio-supply = <&vreg_l12c_1p8>;
+vdd-supply = <&vreg_l13c_3p0>;
+
+reset-gpios = <&tlmm 24 GPIO_ACTIVE_LOW>;
+
+port {
+panel0_in: endpoint {
+remote-endpoint = <&dsi0_out>;
+};
+};
+};
+};
+...

--
2.40.1

Re: [PATCH 1/2] dt-bindings: display: panel: Add Visionox R66451 AMOLED DSI panel bindings

2023-05-21 Thread Marijn Suijten 
On 2023-05-16 13:20:30, Jessica Zhang wrote:
> Document the 1080x2340 Visionox R66451 AMOLED DSI panel bindings
> 
> Signed-off-by: Jessica Zhang 
> ---
>  .../bindings/display/panel/visionox,r66451.yaml| 59 
> ++
>  1 file changed, 59 insertions(+)
> 
> diff --git 
> a/Documentation/devicetree/bindings/display/panel/visionox,r66451.yaml 
> b/Documentation/devicetree/bindings/display/panel/visionox,r66451.yaml
> new file mode 100644
> index ..6ba323683921
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/panel/visionox,r66451.yaml
> @@ -0,0 +1,59 @@
> +# SPDX-License-Identifier: GPL-2.0-only or BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/panel/visionox,r66451.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Visionox R66451 AMOLED DSI Panel
> +
> +maintainers:
> +  - Jessica Zhang 
> +
> +allOf:
> +  - $ref: panel-common.yaml#
> +
> +properties:
> +  compatible:
> +const: visionox,r66451
> +
> +  reg:
> +maxItems: 1
> +description: DSI virtual channel
> +
> +  vddio-supply: true
> +  vdd-supply: true
> +  port: true
> +  reset-gpios: true

Normally for cmd-mode panels there is also a `disp-te` pin which is
optionally registered in dsi_host.c as GPIOD_IN, but on **ALL** my Sony
phones this breaks vsync (as in: mdp5 stops receiving the interrupt, but
we can see disp-te in /proc/interrupts then).

- Marijn

> +additionalProperties: false
> +
> +required:
> +  - compatible
> +  - reg
> +  - vddio-supply
> +  - vdd-supply
> +  - reset-gpios
> +  - port
> +
> +examples:
> +  - |
> +#include 
> +dsi {
> +#address-cells = <1>;
> +#size-cells = <0>;
> +panel@0 {
> +compatible = "visionox,r66451";
> +reg = <0>;
> +vddio-supply = <&vreg_l12c_1p8>;
> +vdd-supply = <&vreg_l13c_3p0>;
> +
> +reset-gpios = <&tlmm 24 GPIO_ACTIVE_LOW>;
> +
> +port {
> +panel0_in: endpoint {
> +remote-endpoint = <&dsi0_out>;
> +};
> +};
> +};
> +};
> +...
> 
> -- 
> 2.40.1
>

Re: [PATCH 1/2] dt-bindings: display: panel: Add Visionox R66451 AMOLED DSI panel bindings

2023-05-17 Thread Krzysztof Kozlowski 
On 16/05/2023 22:20, Jessica Zhang wrote:
> Document the 1080x2340 Visionox R66451 AMOLED DSI panel bindings
> 
> Signed-off-by: Jessica Zhang 
> ---
>  .../bindings/display/panel/visionox,r66451.yaml| 59 
> ++
>  1 file changed, 59 insertions(+)

If there is going to be new version:
A nit, subject: drop second/last, redundant "bindings". The
"dt-bindings" prefix is already stating that these are bindings.


Reviewed-by: Krzysztof Kozlowski 

Best regards,
Krzysztof

[PATCH 1/2] dt-bindings: display: panel: Add Visionox R66451 AMOLED DSI panel bindings

2023-05-16 Thread Jessica Zhang 
Document the 1080x2340 Visionox R66451 AMOLED DSI panel bindings

Signed-off-by: Jessica Zhang 
---
 .../bindings/display/panel/visionox,r66451.yaml| 59 ++
 1 file changed, 59 insertions(+)

diff --git 
a/Documentation/devicetree/bindings/display/panel/visionox,r66451.yaml 
b/Documentation/devicetree/bindings/display/panel/visionox,r66451.yaml
new file mode 100644
index ..6ba323683921
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/panel/visionox,r66451.yaml
@@ -0,0 +1,59 @@
+# SPDX-License-Identifier: GPL-2.0-only or BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/panel/visionox,r66451.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Visionox R66451 AMOLED DSI Panel
+
+maintainers:
+  - Jessica Zhang 
+
+allOf:
+  - $ref: panel-common.yaml#
+
+properties:
+  compatible:
+const: visionox,r66451
+
+  reg:
+maxItems: 1
+description: DSI virtual channel
+
+  vddio-supply: true
+  vdd-supply: true
+  port: true
+  reset-gpios: true
+
+additionalProperties: false
+
+required:
+  - compatible
+  - reg
+  - vddio-supply
+  - vdd-supply
+  - reset-gpios
+  - port
+
+examples:
+  - |
+#include 
+dsi {
+#address-cells = <1>;
+#size-cells = <0>;
+panel@0 {
+compatible = "visionox,r66451";
+reg = <0>;
+vddio-supply = <&vreg_l12c_1p8>;
+vdd-supply = <&vreg_l13c_3p0>;
+
+reset-gpios = <&tlmm 24 GPIO_ACTIVE_LOW>;
+
+port {
+panel0_in: endpoint {
+remote-endpoint = <&dsi0_out>;
+};
+};
+};
+};
+...

-- 
2.40.1