| Message ID | 20230516-b4-r66451-panel-driver-v1-1-4210bcbb1649@quicinc.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | Add support for Visionox R66451 AMOLED DSI panel | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | warning | total: 0 errors, 1 warnings, 59 lines checked |
| robh/patch-applied | success | |
| robh/dt-meta-schema | fail | build log |
On 16/05/2023 22:20, 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(+) 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 <krzysztof.kozlowski@linaro.org> Best regards, Krzysztof
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). - Marijn > +additionalProperties: false > + > +required: > + - compatible > + - reg > + - vddio-supply > + - vdd-supply > + - reset-gpios > + - port > + > +examples: > + - | > + #include <dt-bindings/gpio/gpio.h> > + 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 >
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. Neil > > - Marijn > >> +additionalProperties: false >> + >> +required: >> + - compatible >> + - reg >> + - vddio-supply >> + - vdd-supply >> + - reset-gpios >> + - port >> + >> +examples: >> + - | >> + #include <dt-bindings/gpio/gpio.h> >> + 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 >>
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>
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 <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). 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 > > <snip>
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 <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). > > 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 > > > > <snip> >
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 + +additionalProperties: false + +required: + - compatible + - reg + - vddio-supply + - vdd-supply + - reset-gpios + - port + +examples: + - | + #include <dt-bindings/gpio/gpio.h> + 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>; + }; + }; + }; + }; +...
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(+)