| Message ID | 20190706140746.29132-4-jacopo+renesas@jmondi.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm: rcar-du: Add Color Management Module (CMM) | expand |
Hi Jacopo, On 08/07/2019 04:11, Laurent Pinchart wrote: > Hi Jacopo, > > Thank you for the patch. > > On Sat, Jul 06, 2019 at 04:07:30PM +0200, Jacopo Mondi wrote: >> Update the 'vsps' property in the R-Car Gen3 SoC device tree files to >> match what's in in the documentation example. >> >> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> +1 from me too. This certainly improves readability/clarity IMO. Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > >> --- >> arch/arm64/boot/dts/renesas/r8a774c0.dtsi | 2 +- >> arch/arm64/boot/dts/renesas/r8a7795.dtsi | 2 +- >> arch/arm64/boot/dts/renesas/r8a77965.dtsi | 2 +- >> arch/arm64/boot/dts/renesas/r8a77990.dtsi | 2 +- >> arch/arm64/boot/dts/renesas/r8a77995.dtsi | 2 +- >> 5 files changed, 5 insertions(+), 5 deletions(-) >> >> diff --git a/arch/arm64/boot/dts/renesas/r8a774c0.dtsi b/arch/arm64/boot/dts/renesas/r8a774c0.dtsi >> index 3f86db199dbf..e643f9d3c102 100644 >> --- a/arch/arm64/boot/dts/renesas/r8a774c0.dtsi >> +++ b/arch/arm64/boot/dts/renesas/r8a774c0.dtsi >> @@ -1807,7 +1807,7 @@ >> clocks = <&cpg CPG_MOD 724>, >> <&cpg CPG_MOD 723>; >> clock-names = "du.0", "du.1"; >> - vsps = <&vspd0 0 &vspd1 0>; >> + vsps = <&vspd0 0>, <&vspd1 0>; >> status = "disabled"; >> >> ports { >> diff --git a/arch/arm64/boot/dts/renesas/r8a7795.dtsi b/arch/arm64/boot/dts/renesas/r8a7795.dtsi >> index 097538cc4b1f..432f4036a8a8 100644 >> --- a/arch/arm64/boot/dts/renesas/r8a7795.dtsi >> +++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi >> @@ -3098,7 +3098,7 @@ >> <&cpg CPG_MOD 722>, >> <&cpg CPG_MOD 721>; >> clock-names = "du.0", "du.1", "du.2", "du.3"; >> - vsps = <&vspd0 0 &vspd1 0 &vspd2 0 &vspd0 1>; >> + vsps = <&vspd0 0>, <&vspd1 0>, <&vspd2 0>, <&vspd0 1>; >> status = "disabled"; >> >> ports { >> diff --git a/arch/arm64/boot/dts/renesas/r8a77965.dtsi b/arch/arm64/boot/dts/renesas/r8a77965.dtsi >> index 2554b1742dbf..b701aeb4f438 100644 >> --- a/arch/arm64/boot/dts/renesas/r8a77965.dtsi >> +++ b/arch/arm64/boot/dts/renesas/r8a77965.dtsi >> @@ -2456,7 +2456,7 @@ >> clock-names = "du.0", "du.1", "du.3"; >> status = "disabled"; >> >> - vsps = <&vspd0 0 &vspd1 0 &vspd0 1>; >> + vsps = <&vspd0 0>, <&vspd1 0>, <&vspd0 1>; >> >> ports { >> #address-cells = <1>; >> diff --git a/arch/arm64/boot/dts/renesas/r8a77990.dtsi b/arch/arm64/boot/dts/renesas/r8a77990.dtsi >> index 56cb566ffa09..79db5441b7e7 100644 >> --- a/arch/arm64/boot/dts/renesas/r8a77990.dtsi >> +++ b/arch/arm64/boot/dts/renesas/r8a77990.dtsi >> @@ -1764,7 +1764,7 @@ >> clocks = <&cpg CPG_MOD 724>, >> <&cpg CPG_MOD 723>; >> clock-names = "du.0", "du.1"; >> - vsps = <&vspd0 0 &vspd1 0>; >> + vsps = <&vspd0 0>, <&vspd1 0>; >> status = "disabled"; >> >> ports { >> diff --git a/arch/arm64/boot/dts/renesas/r8a77995.dtsi b/arch/arm64/boot/dts/renesas/r8a77995.dtsi >> index 5bf3af246e14..49a11b4f55bd 100644 >> --- a/arch/arm64/boot/dts/renesas/r8a77995.dtsi >> +++ b/arch/arm64/boot/dts/renesas/r8a77995.dtsi >> @@ -1001,7 +1001,7 @@ >> clocks = <&cpg CPG_MOD 724>, >> <&cpg CPG_MOD 723>; >> clock-names = "du.0", "du.1"; >> - vsps = <&vspd0 0 &vspd1 0>; >> + vsps = <&vspd0 0>, <&vspd1 0>; >> status = "disabled"; >> >> ports { >
On Sat, Jul 6, 2019 at 4:07 PM Jacopo Mondi <jacopo+renesas@jmondi.org> wrote: > Update the 'vsps' property in the R-Car Gen3 SoC device tree files to > match what's in in the documentation example. double in (no worries, I'll fix that up myself) > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> Thanks! Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> i.e. will queue in renesas-devel for v5.4. BTW, any plans to add channel indices to the vsps properties in the remaining DTS files? Gr{oetje,eeting}s, Geert
Hi Geert, On Wed, Aug 21, 2019 at 02:16:02PM +0200, Geert Uytterhoeven wrote: > On Sat, Jul 6, 2019 at 4:07 PM Jacopo Mondi <jacopo+renesas@jmondi.org> wrote: > > Update the 'vsps' property in the R-Car Gen3 SoC device tree files to > > match what's in in the documentation example. > > double in (no worries, I'll fix that up myself) > > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> > > Thanks! > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > i.e. will queue in renesas-devel for v5.4. > > BTW, any plans to add channel indices to the vsps properties in the > remaining DTS files? According to what I read in the parsing 'vsps' proeprties parsing routine rcar_du_vsps_init(), if the channel index is not specified, it is defaulted to 0. /* Store the VSP pointer and pipe index in the CRTC. */ rcdu->crtcs[i].vsp = &rcdu->vsps[j]; rcdu->crtcs[i].vsp_pipe = cells >= 1 ? args.args[0] : 0; The DU bindings documentation does not state that the channel index is optional, so yes, it might be worth changing this and simplify the parsing routing to always assume the channel index is there. What does Laurent think? Is this worth the effort? Thanks j > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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
Hi Jacopo, On Thu, Aug 22, 2019 at 12:00:51PM +0200, Jacopo Mondi wrote: > On Wed, Aug 21, 2019 at 02:16:02PM +0200, Geert Uytterhoeven wrote: > > On Sat, Jul 6, 2019 at 4:07 PM Jacopo Mondi <jacopo+renesas@jmondi.org> wrote: > > > Update the 'vsps' property in the R-Car Gen3 SoC device tree files to > > > match what's in in the documentation example. > > > > double in (no worries, I'll fix that up myself) > > > > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> > > > > Thanks! > > > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > > i.e. will queue in renesas-devel for v5.4. > > > > BTW, any plans to add channel indices to the vsps properties in the > > remaining DTS files? > > According to what I read in the parsing 'vsps' proeprties parsing > routine rcar_du_vsps_init(), if the channel index is not specified, it > is defaulted to 0. > > /* Store the VSP pointer and pipe index in the CRTC. */ > rcdu->crtcs[i].vsp = &rcdu->vsps[j]; > rcdu->crtcs[i].vsp_pipe = cells >= 1 ? args.args[0] : 0; > > The DU bindings documentation does not state that the channel index is > optional, so yes, it might be worth changing this and simplify the > parsing routing to always assume the channel index is there. > > What does Laurent think? Is this worth the effort? I think it's worth the effort to keep the upstream DT sources in sync with the latest bindings, but We can't change the code as we need to ensure backward compatibility. So, yes to the DT update, but the driver should stay unmodified (or should receive a new comment explaining the required backward compatibility if there's not one already).
diff --git a/arch/arm64/boot/dts/renesas/r8a774c0.dtsi b/arch/arm64/boot/dts/renesas/r8a774c0.dtsi index 3f86db199dbf..e643f9d3c102 100644 --- a/arch/arm64/boot/dts/renesas/r8a774c0.dtsi +++ b/arch/arm64/boot/dts/renesas/r8a774c0.dtsi @@ -1807,7 +1807,7 @@ clocks = <&cpg CPG_MOD 724>, <&cpg CPG_MOD 723>; clock-names = "du.0", "du.1"; - vsps = <&vspd0 0 &vspd1 0>; + vsps = <&vspd0 0>, <&vspd1 0>; status = "disabled"; ports { diff --git a/arch/arm64/boot/dts/renesas/r8a7795.dtsi b/arch/arm64/boot/dts/renesas/r8a7795.dtsi index 097538cc4b1f..432f4036a8a8 100644 --- a/arch/arm64/boot/dts/renesas/r8a7795.dtsi +++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi @@ -3098,7 +3098,7 @@ <&cpg CPG_MOD 722>, <&cpg CPG_MOD 721>; clock-names = "du.0", "du.1", "du.2", "du.3"; - vsps = <&vspd0 0 &vspd1 0 &vspd2 0 &vspd0 1>; + vsps = <&vspd0 0>, <&vspd1 0>, <&vspd2 0>, <&vspd0 1>; status = "disabled"; ports { diff --git a/arch/arm64/boot/dts/renesas/r8a77965.dtsi b/arch/arm64/boot/dts/renesas/r8a77965.dtsi index 2554b1742dbf..b701aeb4f438 100644 --- a/arch/arm64/boot/dts/renesas/r8a77965.dtsi +++ b/arch/arm64/boot/dts/renesas/r8a77965.dtsi @@ -2456,7 +2456,7 @@ clock-names = "du.0", "du.1", "du.3"; status = "disabled"; - vsps = <&vspd0 0 &vspd1 0 &vspd0 1>; + vsps = <&vspd0 0>, <&vspd1 0>, <&vspd0 1>; ports { #address-cells = <1>; diff --git a/arch/arm64/boot/dts/renesas/r8a77990.dtsi b/arch/arm64/boot/dts/renesas/r8a77990.dtsi index 56cb566ffa09..79db5441b7e7 100644 --- a/arch/arm64/boot/dts/renesas/r8a77990.dtsi +++ b/arch/arm64/boot/dts/renesas/r8a77990.dtsi @@ -1764,7 +1764,7 @@ clocks = <&cpg CPG_MOD 724>, <&cpg CPG_MOD 723>; clock-names = "du.0", "du.1"; - vsps = <&vspd0 0 &vspd1 0>; + vsps = <&vspd0 0>, <&vspd1 0>; status = "disabled"; ports { diff --git a/arch/arm64/boot/dts/renesas/r8a77995.dtsi b/arch/arm64/boot/dts/renesas/r8a77995.dtsi index 5bf3af246e14..49a11b4f55bd 100644 --- a/arch/arm64/boot/dts/renesas/r8a77995.dtsi +++ b/arch/arm64/boot/dts/renesas/r8a77995.dtsi @@ -1001,7 +1001,7 @@ clocks = <&cpg CPG_MOD 724>, <&cpg CPG_MOD 723>; clock-names = "du.0", "du.1"; - vsps = <&vspd0 0 &vspd1 0>; + vsps = <&vspd0 0>, <&vspd1 0>; status = "disabled"; ports {
Update the 'vsps' property in the R-Car Gen3 SoC device tree files to match what's in in the documentation example. Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org> --- arch/arm64/boot/dts/renesas/r8a774c0.dtsi | 2 +- arch/arm64/boot/dts/renesas/r8a7795.dtsi | 2 +- arch/arm64/boot/dts/renesas/r8a77965.dtsi | 2 +- arch/arm64/boot/dts/renesas/r8a77990.dtsi | 2 +- arch/arm64/boot/dts/renesas/r8a77995.dtsi | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-)