Linux-Renesas-SoC Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v5] dt-bindings: display: renesas: du: Document optional reset properties
@ 2020-02-14  8:26 Geert Uytterhoeven
  2020-02-14  9:35 ` Kieran Bingham
  2020-02-19 16:04 ` Laurent Pinchart
  0 siblings, 2 replies; 5+ messages in thread
From: Geert Uytterhoeven @ 2020-02-14  8:26 UTC (permalink / raw)
  To: Laurent Pinchart, Kieran Bingham, David Airlie, Daniel Vetter,
	Rob Herring, Mark Rutland
  Cc: dri-devel, linux-renesas-soc, devicetree, Geert Uytterhoeven

Document the optional properties for describing module resets, to
support resetting display channels on R-Car Gen2 and Gen3.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Rob Herring <robh@kernel.org>
---
Who's taking this kind of patches?
V1 was submmitted in March 2017.

v5:
  - Rebase on top of renesas,cmms and renesas,vsps patches,

v4:
  - Use "All but R8A7779" instead of "R8A779[0123456]", to reduce future
    churn,

v3:
  - Add Acked-by,
  - Drop LVDS resets, as LVDS is now covered by a separate binding,
  - Update the example.

v2:
  - s/phandles/phandle/.
---
 .../devicetree/bindings/display/renesas,du.txt         | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/renesas,du.txt b/Documentation/devicetree/bindings/display/renesas,du.txt
index eb4ae41fe41f83c7..51cd4d1627703a15 100644
--- a/Documentation/devicetree/bindings/display/renesas,du.txt
+++ b/Documentation/devicetree/bindings/display/renesas,du.txt
@@ -50,6 +50,14 @@ Required Properties:
     VSP instance that serves the DU channel, and the channel index identifies
     the LIF instance in that VSP.
 
+Optional properties:
+  - resets: A list of phandle + reset-specifier pairs, one for each entry in
+    the reset-names property.
+  - reset-names: Names of the resets. This property is model-dependent.
+    - All but R8A7779 use one reset for a group of one or more successive
+      channels. The resets must be named "du.x" with "x" being the numerical
+      index of the lowest channel in the group.
+
 Required nodes:
 
 The connections to the DU output video ports are modeled using the OF graph
@@ -96,6 +104,8 @@ Example: R8A7795 (R-Car H3) ES2.0 DU
 			 <&cpg CPG_MOD 722>,
 			 <&cpg CPG_MOD 721>;
 		clock-names = "du.0", "du.1", "du.2", "du.3";
+		resets = <&cpg 724>, <&cpg 722>;
+		reset-names = "du.0", "du.2";
 		renesas,cmms = <&cmm0>, <&cmm1>, <&cmm2>, <&cmm3>;
 		renesas,vsps = <&vspd0 0>, <&vspd1 0>, <&vspd2 0>, <&vspd0 1>;
 
-- 
2.17.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v5] dt-bindings: display: renesas: du: Document optional reset properties
  2020-02-14  8:26 [PATCH v5] dt-bindings: display: renesas: du: Document optional reset properties Geert Uytterhoeven
@ 2020-02-14  9:35 ` Kieran Bingham
  2020-02-19 16:04 ` Laurent Pinchart
  1 sibling, 0 replies; 5+ messages in thread
From: Kieran Bingham @ 2020-02-14  9:35 UTC (permalink / raw)
  To: Geert Uytterhoeven, Laurent Pinchart, David Airlie,
	Daniel Vetter, Rob Herring, Mark Rutland
  Cc: dri-devel, linux-renesas-soc, devicetree

Hi Geert,

On 14/02/2020 08:26, Geert Uytterhoeven wrote:
> Document the optional properties for describing module resets, to
> support resetting display channels on R-Car Gen2 and Gen3.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Acked-by: Rob Herring <robh@kernel.org>

Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

> ---
> Who's taking this kind of patches?
> V1 was submmitted in March 2017.

Hrm ... presumably through the DRM subsystem trees?

Or just for Laurent to pick up ...
--
KB


> 
> v5:
>   - Rebase on top of renesas,cmms and renesas,vsps patches,
> 
> v4:
>   - Use "All but R8A7779" instead of "R8A779[0123456]", to reduce future
>     churn,
> 
> v3:
>   - Add Acked-by,
>   - Drop LVDS resets, as LVDS is now covered by a separate binding,
>   - Update the example.
> 
> v2:
>   - s/phandles/phandle/.
> ---
>  .../devicetree/bindings/display/renesas,du.txt         | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/renesas,du.txt b/Documentation/devicetree/bindings/display/renesas,du.txt
> index eb4ae41fe41f83c7..51cd4d1627703a15 100644
> --- a/Documentation/devicetree/bindings/display/renesas,du.txt
> +++ b/Documentation/devicetree/bindings/display/renesas,du.txt
> @@ -50,6 +50,14 @@ Required Properties:
>      VSP instance that serves the DU channel, and the channel index identifies
>      the LIF instance in that VSP.
>  
> +Optional properties:
> +  - resets: A list of phandle + reset-specifier pairs, one for each entry in
> +    the reset-names property.
> +  - reset-names: Names of the resets. This property is model-dependent.
> +    - All but R8A7779 use one reset for a group of one or more successive
> +      channels. The resets must be named "du.x" with "x" being the numerical
> +      index of the lowest channel in the group.
> +
>  Required nodes:
>  
>  The connections to the DU output video ports are modeled using the OF graph
> @@ -96,6 +104,8 @@ Example: R8A7795 (R-Car H3) ES2.0 DU
>  			 <&cpg CPG_MOD 722>,
>  			 <&cpg CPG_MOD 721>;
>  		clock-names = "du.0", "du.1", "du.2", "du.3";
> +		resets = <&cpg 724>, <&cpg 722>;
> +		reset-names = "du.0", "du.2";
>  		renesas,cmms = <&cmm0>, <&cmm1>, <&cmm2>, <&cmm3>;
>  		renesas,vsps = <&vspd0 0>, <&vspd1 0>, <&vspd2 0>, <&vspd0 1>;
>  
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v5] dt-bindings: display: renesas: du: Document optional reset properties
  2020-02-14  8:26 [PATCH v5] dt-bindings: display: renesas: du: Document optional reset properties Geert Uytterhoeven
  2020-02-14  9:35 ` Kieran Bingham
@ 2020-02-19 16:04 ` Laurent Pinchart
  2020-02-19 16:36   ` Geert Uytterhoeven
  1 sibling, 1 reply; 5+ messages in thread
From: Laurent Pinchart @ 2020-02-19 16:04 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Kieran Bingham, David Airlie, Daniel Vetter, Rob Herring,
	Mark Rutland, dri-devel, linux-renesas-soc, devicetree

Hi Geert,

On Fri, Feb 14, 2020 at 09:26:23AM +0100, Geert Uytterhoeven wrote:
> Document the optional properties for describing module resets, to
> support resetting display channels on R-Car Gen2 and Gen3.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Acked-by: Rob Herring <robh@kernel.org>
> ---
> Who's taking this kind of patches?
> V1 was submmitted in March 2017.

My bad.

> 
> v5:
>   - Rebase on top of renesas,cmms and renesas,vsps patches,
> 
> v4:
>   - Use "All but R8A7779" instead of "R8A779[0123456]", to reduce future
>     churn,
> 
> v3:
>   - Add Acked-by,
>   - Drop LVDS resets, as LVDS is now covered by a separate binding,
>   - Update the example.
> 
> v2:
>   - s/phandles/phandle/.
> ---
>  .../devicetree/bindings/display/renesas,du.txt         | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/renesas,du.txt b/Documentation/devicetree/bindings/display/renesas,du.txt
> index eb4ae41fe41f83c7..51cd4d1627703a15 100644
> --- a/Documentation/devicetree/bindings/display/renesas,du.txt
> +++ b/Documentation/devicetree/bindings/display/renesas,du.txt
> @@ -50,6 +50,14 @@ Required Properties:
>      VSP instance that serves the DU channel, and the channel index identifies
>      the LIF instance in that VSP.
>  
> +Optional properties:
> +  - resets: A list of phandle + reset-specifier pairs, one for each entry in
> +    the reset-names property.
> +  - reset-names: Names of the resets. This property is model-dependent.
> +    - All but R8A7779 use one reset for a group of one or more successive
> +      channels. The resets must be named "du.x" with "x" being the numerical
> +      index of the lowest channel in the group.

I've now reviewed the patches that add those properties to our .dtsi
files, and I wonder how we should handle the two SoCs that have DU0, DU1
and DU3, but not DU2. The reset resource is tied to a group of two
channels, so we would use du.0 and du.2 respectively, but that conflicts
with the above text.

I'm trying to think about the implementation on the driver side, where
group resources are associated with a group object, whose index is
computed by dividing the channel number by 2. We could have a special
case in group initialization that uses du.3 instead of du.2 for the
second group.

What do you think ? Probably overkill, and we should go for du.3 ?

> +
>  Required nodes:
>  
>  The connections to the DU output video ports are modeled using the OF graph
> @@ -96,6 +104,8 @@ Example: R8A7795 (R-Car H3) ES2.0 DU
>  			 <&cpg CPG_MOD 722>,
>  			 <&cpg CPG_MOD 721>;
>  		clock-names = "du.0", "du.1", "du.2", "du.3";
> +		resets = <&cpg 724>, <&cpg 722>;
> +		reset-names = "du.0", "du.2";
>  		renesas,cmms = <&cmm0>, <&cmm1>, <&cmm2>, <&cmm3>;
>  		renesas,vsps = <&vspd0 0>, <&vspd1 0>, <&vspd2 0>, <&vspd0 1>;
>  

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v5] dt-bindings: display: renesas: du: Document optional reset properties
  2020-02-19 16:04 ` Laurent Pinchart
@ 2020-02-19 16:36   ` Geert Uytterhoeven
  2020-02-19 16:48     ` Laurent Pinchart
  0 siblings, 1 reply; 5+ messages in thread
From: Geert Uytterhoeven @ 2020-02-19 16:36 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Geert Uytterhoeven, Kieran Bingham, David Airlie, Daniel Vetter,
	Rob Herring, Mark Rutland, DRI Development, Linux-Renesas,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

Hi Laurent,

On Wed, Feb 19, 2020 at 5:04 PM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Fri, Feb 14, 2020 at 09:26:23AM +0100, Geert Uytterhoeven wrote:
> > Document the optional properties for describing module resets, to
> > support resetting display channels on R-Car Gen2 and Gen3.
> >
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Acked-by: Rob Herring <robh@kernel.org>

> > --- a/Documentation/devicetree/bindings/display/renesas,du.txt
> > +++ b/Documentation/devicetree/bindings/display/renesas,du.txt
> > @@ -50,6 +50,14 @@ Required Properties:
> >      VSP instance that serves the DU channel, and the channel index identifies
> >      the LIF instance in that VSP.
> >
> > +Optional properties:
> > +  - resets: A list of phandle + reset-specifier pairs, one for each entry in
> > +    the reset-names property.
> > +  - reset-names: Names of the resets. This property is model-dependent.
> > +    - All but R8A7779 use one reset for a group of one or more successive
> > +      channels. The resets must be named "du.x" with "x" being the numerical
> > +      index of the lowest channel in the group.
>
> I've now reviewed the patches that add those properties to our .dtsi
> files, and I wonder how we should handle the two SoCs that have DU0, DU1
> and DU3, but not DU2. The reset resource is tied to a group of two
> channels, so we would use du.0 and du.2 respectively, but that conflicts
> with the above text.
>
> I'm trying to think about the implementation on the driver side, where
> group resources are associated with a group object, whose index is
> computed by dividing the channel number by 2. We could have a special
> case in group initialization that uses du.3 instead of du.2 for the
> second group.
>
> What do you think ? Probably overkill, and we should go for du.3 ?

The "division by 2" rule is valid for R-Car Gen3, but not for R-Car
Gen2, where there is only a single reset for all channels.

Originally we had "du.0-1" and "du.2-3" (hmm, somehow I missed adding
this to the changelog for the bindings,  but it is present in the
changelog for the DTS files), but after switching to "du.0" and "du.2",
I always envisioned implementing this by finding a "du.x" reset by
looping from the current channel index to 0.  That algorithm works for all
supported SoCs (irrespective of naming the second reset on R-Car H3-N
and M3-N "du.2" or "du.3" ;-)

As per your comment about single resets, we could drop reset-names on
R-Car Gen2, but doing so would mean another special case in the driver.

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v5] dt-bindings: display: renesas: du: Document optional reset properties
  2020-02-19 16:36   ` Geert Uytterhoeven
@ 2020-02-19 16:48     ` Laurent Pinchart
  0 siblings, 0 replies; 5+ messages in thread
From: Laurent Pinchart @ 2020-02-19 16:48 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Geert Uytterhoeven, Kieran Bingham, David Airlie, Daniel Vetter,
	Rob Herring, Mark Rutland, DRI Development, Linux-Renesas,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

Hi Geert,

On Wed, Feb 19, 2020 at 05:36:57PM +0100, Geert Uytterhoeven wrote:
> On Wed, Feb 19, 2020 at 5:04 PM Laurent Pinchart wrote:
> > On Fri, Feb 14, 2020 at 09:26:23AM +0100, Geert Uytterhoeven wrote:
> > > Document the optional properties for describing module resets, to
> > > support resetting display channels on R-Car Gen2 and Gen3.
> > >
> > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > > Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > Acked-by: Rob Herring <robh@kernel.org>
> 
> > > --- a/Documentation/devicetree/bindings/display/renesas,du.txt
> > > +++ b/Documentation/devicetree/bindings/display/renesas,du.txt
> > > @@ -50,6 +50,14 @@ Required Properties:
> > >      VSP instance that serves the DU channel, and the channel index identifies
> > >      the LIF instance in that VSP.
> > >
> > > +Optional properties:
> > > +  - resets: A list of phandle + reset-specifier pairs, one for each entry in
> > > +    the reset-names property.
> > > +  - reset-names: Names of the resets. This property is model-dependent.
> > > +    - All but R8A7779 use one reset for a group of one or more successive
> > > +      channels. The resets must be named "du.x" with "x" being the numerical
> > > +      index of the lowest channel in the group.
> >
> > I've now reviewed the patches that add those properties to our .dtsi
> > files, and I wonder how we should handle the two SoCs that have DU0, DU1
> > and DU3, but not DU2. The reset resource is tied to a group of two
> > channels, so we would use du.0 and du.2 respectively, but that conflicts
> > with the above text.
> >
> > I'm trying to think about the implementation on the driver side, where
> > group resources are associated with a group object, whose index is
> > computed by dividing the channel number by 2. We could have a special
> > case in group initialization that uses du.3 instead of du.2 for the
> > second group.
> >
> > What do you think ? Probably overkill, and we should go for du.3 ?
> 
> The "division by 2" rule is valid for R-Car Gen3, but not for R-Car
> Gen2, where there is only a single reset for all channels.
> 
> Originally we had "du.0-1" and "du.2-3" (hmm, somehow I missed adding
> this to the changelog for the bindings,  but it is present in the
> changelog for the DTS files), but after switching to "du.0" and "du.2",
> I always envisioned implementing this by finding a "du.x" reset by
> looping from the current channel index to 0.  That algorithm works for all
> supported SoCs (irrespective of naming the second reset on R-Car H3-N
> and M3-N "du.2" or "du.3" ;-)
> 
> As per your comment about single resets, we could drop reset-names on
> R-Car Gen2, but doing so would mean another special case in the driver.

Probably not worth it indeed. We can handle all this in the driver,
let's keep it as-is.

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-14  8:26 [PATCH v5] dt-bindings: display: renesas: du: Document optional reset properties Geert Uytterhoeven
2020-02-14  9:35 ` Kieran Bingham
2020-02-19 16:04 ` Laurent Pinchart
2020-02-19 16:36   ` Geert Uytterhoeven
2020-02-19 16:48     ` Laurent Pinchart

Linux-Renesas-SoC Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-renesas-soc/0 linux-renesas-soc/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-renesas-soc linux-renesas-soc/ https://lore.kernel.org/linux-renesas-soc \
		linux-renesas-soc@vger.kernel.org
	public-inbox-index linux-renesas-soc

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-renesas-soc


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git