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

Marijn Suijten
2023-05-22T14:52:05Z

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 <quic_jesszhan@quicinc.com>
> >> ---
> >> .../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 000000000000..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 <quic_jesszhan@quicinc.com>
> >> +
> >> +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

<snip>