Re: vsps and channel indices (was: Re: [PATCH v3 1/2] arm64: dts: renesas: r8a77980: add FCPVD/VSPD/DU/LVDS support)
Hi Laurent,
On Wed, Aug 29, 2018 at 11:12 AM Laurent Pinchart
wrote:
> On Tuesday, 28 August 2018 15:10:52 EEST Geert Uytterhoeven wrote:
> > On Wed, Jun 13, 2018 at 10:11 PM Sergei Shtylyov wrote:
> > > Describe the interconnected FCPVD0, VSPD0, DU, and LVDS0 devices in the
> > > R8A77980 device tree...
> > >
> > > Based on the original (and large) patch by Vladimir Barinov.
> > >
> > > Signed-off-by: Vladimir Barinov
> > > Signed-off-by: Sergei Shtylyov
> > > Reviewed-by: Laurent Pinchart
> > >
> > > --- renesas.orig/arch/arm64/boot/dts/renesas/r8a77980.dtsi
> > > +++ renesas/arch/arm64/boot/dts/renesas/r8a77980.dtsi
> > >
> > > + du: display@feb0 {
> > > + compatible = "renesas,du-r8a77980",
> > > +"renesas,du-r8a77970";
> > > + reg = <0 0xfeb0 0 0x8>;
> > > + interrupts = ;
> > > + clocks = <&cpg CPG_MOD 724>;
> > > + clock-names = "du.0";
> > > + power-domains = <&sysc R8A77980_PD_ALWAYS_ON>;
> > > + resets = <&cpg 724>;
> > > + vsps = <&vspd0>;
> >
> > According to the bindings, the vsps property should also contain a
> > channel index.
> >
> > Laurent added the indices to r8a7795.dtsi, but r8a7795-es.dtsi overrides
> > that with a version without the indices.
> > Kieran included the indices when adding DU support for r8a77965 and
> > r8a77995.
> >
> > r8a7796.dtsi, r8a77970.dtsi, and r8a77980.dtsi lack the indices.
> >
> > Commit fd57d77f9c649cf9 ("dt-bindings: display: rcar-du: Add a VSP channel
> > index to the vsps DT property") says a backwards-compatibility mode can be
> > implemented, but I fail to see how this can work, as rcar_du_vsps_init()
> > will just return an error, which is propagated.
> >
> > What am I missing?
>
> We're missing a DT validator :-/
>
> The vsps property shoould indeed contain indices. However, rcar_du_vsps_init()
> implements a backward-compatibility mode by checking the length of the
> property:
>
> cells = of_property_count_u32_elems(np, "vsps") / rcdu->num_crtcs - 1;
>
> and using that as an argument to of_parse_phandle_with_fixed_args().
Thanks for the explanation! I had seen that call, but failed to discover the
actual fallback mechanism in the code...
Perhaps updating the comment to describe the backward-compatibility mode
would help:
/*
* First parse the DT vsps property to populate the list of VSPs. Each
* entry contains a pointer to the VSP DT node and a bitmask of the
* connected DU CRTCs.
*/
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: vsps and channel indices (was: Re: [PATCH v3 1/2] arm64: dts: renesas: r8a77980: add FCPVD/VSPD/DU/LVDS support)
Hi Geert,
On Tuesday, 28 August 2018 15:10:52 EEST Geert Uytterhoeven wrote:
> Hi Sergei, Laurent, Kieran,
>
> On Wed, Jun 13, 2018 at 10:11 PM Sergei Shtylyov wrote:
> > Describe the interconnected FCPVD0, VSPD0, DU, and LVDS0 devices in the
> > R8A77980 device tree...
> >
> > Based on the original (and large) patch by Vladimir Barinov.
> >
> > Signed-off-by: Vladimir Barinov
> > Signed-off-by: Sergei Shtylyov
> > Reviewed-by: Laurent Pinchart
> >
> > --- renesas.orig/arch/arm64/boot/dts/renesas/r8a77980.dtsi
> > +++ renesas/arch/arm64/boot/dts/renesas/r8a77980.dtsi
> >
> > + du: display@feb0 {
> > + compatible = "renesas,du-r8a77980",
> > +"renesas,du-r8a77970";
> > + reg = <0 0xfeb0 0 0x8>;
> > + interrupts = ;
> > + clocks = <&cpg CPG_MOD 724>;
> > + clock-names = "du.0";
> > + power-domains = <&sysc R8A77980_PD_ALWAYS_ON>;
> > + resets = <&cpg 724>;
> > + vsps = <&vspd0>;
>
> According to the bindings, the vsps property should also contain a
> channel index.
>
> Laurent added the indices to r8a7795.dtsi, but r8a7795-es.dtsi overrides
> that with a version without the indices.
> Kieran included the indices when adding DU support for r8a77965 and
> r8a77995.
>
> r8a7796.dtsi, r8a77970.dtsi, and r8a77980.dtsi lack the indices.
>
> Commit fd57d77f9c649cf9 ("dt-bindings: display: rcar-du: Add a VSP channel
> index to the vsps DT property") says a backwards-compatibility mode can be
> implemented, but I fail to see how this can work, as rcar_du_vsps_init()
> will just return an error, which is propagated.
>
> What am I missing?
We're missing a DT validator :-/
The vsps property shoould indeed contain indices. However, rcar_du_vsps_init()
implements a backward-compatibility mode by checking the length of the
property:
cells = of_property_count_u32_elems(np, "vsps") / rcdu->num_crtcs - 1;
and using that as an argument to of_parse_phandle_with_fixed_args().
--
Regards,
Laurent Pinchart
vsps and channel indices (was: Re: [PATCH v3 1/2] arm64: dts: renesas: r8a77980: add FCPVD/VSPD/DU/LVDS support)
Hi Sergei, Laurent, Kieran,
On Wed, Jun 13, 2018 at 10:11 PM Sergei Shtylyov
wrote:
> Describe the interconnected FCPVD0, VSPD0, DU, and LVDS0 devices in the
> R8A77980 device tree...
>
> Based on the original (and large) patch by Vladimir Barinov.
>
> Signed-off-by: Vladimir Barinov
> Signed-off-by: Sergei Shtylyov
> Reviewed-by: Laurent Pinchart
> --- renesas.orig/arch/arm64/boot/dts/renesas/r8a77980.dtsi
> +++ renesas/arch/arm64/boot/dts/renesas/r8a77980.dtsi
> + du: display@feb0 {
> + compatible = "renesas,du-r8a77980",
> +"renesas,du-r8a77970";
> + reg = <0 0xfeb0 0 0x8>;
> + interrupts = ;
> + clocks = <&cpg CPG_MOD 724>;
> + clock-names = "du.0";
> + power-domains = <&sysc R8A77980_PD_ALWAYS_ON>;
> + resets = <&cpg 724>;
> + vsps = <&vspd0>;
According to the bindings, the vsps property should also contain a
channel index.
Laurent added the indices to r8a7795.dtsi, but r8a7795-es.dtsi overrides
that with a version without the indices.
Kieran included the indices when adding DU support for r8a77965 and
r8a77995.
r8a7796.dtsi, r8a77970.dtsi, and r8a77980.dtsi lack the indices.
Commit fd57d77f9c649cf9 ("dt-bindings: display: rcar-du: Add a VSP channel
index to the vsps DT property") says a backwards-compatibility mode can be
implemented, but I fail to see how this can work, as rcar_du_vsps_init()
will just return an error, which is propagated.
What am I missing?
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH v3 1/2] arm64: dts: renesas: r8a77980: add FCPVD/VSPD/DU/LVDS support
On Wed, Jun 13, 2018 at 11:11:27PM +0300, Sergei Shtylyov wrote: > Describe the interconnected FCPVD0, VSPD0, DU, and LVDS0 devices in the > R8A77980 device tree... > > Based on the original (and large) patch by Vladimir Barinov. > > Signed-off-by: Vladimir Barinov > Signed-off-by: Sergei Shtylyov > Reviewed-by: Laurent Pinchart > > --- > Changes in version 3: > - merged in the VSPD/DU/LVDS patches, renamed the patch, and updated the patch > description accordingly; > - fixed the VSPD0's "reg" property. Thanks Sergei, applied.
[PATCH v3 1/2] arm64: dts: renesas: r8a77980: add FCPVD/VSPD/DU/LVDS support
Describe the interconnected FCPVD0, VSPD0, DU, and LVDS0 devices in the
R8A77980 device tree...
Based on the original (and large) patch by Vladimir Barinov.
Signed-off-by: Vladimir Barinov
Signed-off-by: Sergei Shtylyov
Reviewed-by: Laurent Pinchart
---
Changes in version 3:
- merged in the VSPD/DU/LVDS patches, renamed the patch, and updated the patch
description accordingly;
- fixed the VSPD0's "reg" property.
arch/arm64/boot/dts/renesas/r8a77980.dtsi | 77 ++
1 file changed, 77 insertions(+)
Index: renesas/arch/arm64/boot/dts/renesas/r8a77980.dtsi
===
--- renesas.orig/arch/arm64/boot/dts/renesas/r8a77980.dtsi
+++ renesas/arch/arm64/boot/dts/renesas/r8a77980.dtsi
@@ -653,6 +653,83 @@
resets = <&cpg 408>;
};
+ vspd0: vsp@fea2 {
+ compatible = "renesas,vsp2";
+ reg = <0 0xfea2 0 0x5000>;
+ interrupts = ;
+ clocks = <&cpg CPG_MOD 623>;
+ power-domains = <&sysc R8A77980_PD_ALWAYS_ON>;
+ resets = <&cpg 623>;
+ renesas,fcp = <&fcpvd0>;
+ };
+
+ fcpvd0: fcp@fea27000 {
+ compatible = "renesas,fcpv";
+ reg = <0 0xfea27000 0 0x200>;
+ clocks = <&cpg CPG_MOD 603>;
+ power-domains = <&sysc R8A77980_PD_ALWAYS_ON>;
+ resets = <&cpg 603>;
+ };
+
+ du: display@feb0 {
+ compatible = "renesas,du-r8a77980",
+"renesas,du-r8a77970";
+ reg = <0 0xfeb0 0 0x8>;
+ interrupts = ;
+ clocks = <&cpg CPG_MOD 724>;
+ clock-names = "du.0";
+ power-domains = <&sysc R8A77980_PD_ALWAYS_ON>;
+ resets = <&cpg 724>;
+ vsps = <&vspd0>;
+ status = "disabled";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ du_out_rgb: endpoint {
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+ du_out_lvds0: endpoint {
+ remote-endpoint = <&lvds0_in>;
+ };
+ };
+ };
+ };
+
+ lvds0: lvds-encoder@feb9 {
+ compatible = "renesas,r8a77980-lvds";
+ reg = <0 0xfeb9 0 0x14>;
+ clocks = <&cpg CPG_MOD 727>;
+ power-domains = <&sysc R8A77980_PD_ALWAYS_ON>;
+ resets = <&cpg 727>;
+ status = "disabled";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ lvds0_in: endpoint {
+ remote-endpoint =
+ <&du_out_lvds0>;
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+ lvds0_out: endpoint {
+ };
+ };
+ };
+ };
+
prr: chipid@fff00044 {
compatible = "renesas,prr";
reg = <0 0xfff00044 0 4>;
