| Message ID | 20240514152328.21415-4-detlev.casanova@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/rockchip: vop2: Add VP clock resets support | expand |
On Tue, May 14, 2024 at 11:19:47AM -0400, Detlev Casanova wrote: > Add the documentation for VOP2 video ports reset clocks. > One reset can be set per video port. > > Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com> Are these resets valid for all VOPs or just the one on 3588? > --- > .../display/rockchip/rockchip-vop2.yaml | 27 +++++++++++++++++++ > 1 file changed, 27 insertions(+) > > diff --git a/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml b/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml > index 2531726af306b..941fd059498d4 100644 > --- a/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml > +++ b/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml > @@ -65,6 +65,22 @@ properties: > - const: dclk_vp3 > - const: pclk_vop > > + resets: > + minItems: 3 > + items: > + - description: Pixel clock reset for video port 0. > + - description: Pixel clock reset for video port 1. > + - description: Pixel clock reset for video port 2. > + - description: Pixel clock reset for video port 3. > + > + reset-names: > + minItems: 3 > + items: > + - const: dclk_vp0 > + - const: dclk_vp1 > + - const: dclk_vp2 > + - const: dclk_vp3 > + > rockchip,grf: > $ref: /schemas/types.yaml#/definitions/phandle > description: > @@ -128,6 +144,11 @@ allOf: > clock-names: > minItems: 7 > > + resets: > + minItems: 4 > + reset-names: > + minItems: 4 > + > ports: > required: > - port@0 > @@ -183,6 +204,12 @@ examples: > "dclk_vp0", > "dclk_vp1", > "dclk_vp2"; > + resets = <&cru SRST_VOP0>, > + <&cru SRST_VOP1>, > + <&cru SRST_VOP2>; > + reset-names = "dclk_vp0", > + "dclk_vp1", > + "dclk_vp2"; > power-domains = <&power RK3568_PD_VO>; > iommus = <&vop_mmu>; > vop_out: ports { > -- > 2.43.2 >
Am Mittwoch, 15. Mai 2024, 18:19:29 CEST schrieb Conor Dooley: > On Tue, May 14, 2024 at 11:19:47AM -0400, Detlev Casanova wrote: > > Add the documentation for VOP2 video ports reset clocks. > > One reset can be set per video port. > > > > Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com> > > Are these resets valid for all VOPs or just the one on 3588? Not in that form. I.e. rk3588 has 4 video-ports (0-3), while rk3568 has 3 (0-2). So the binding should take into account that rk3568 also has the SRST_VOP0 ... SRST_VOP2. Also, I guess we might not want to limit ourself to stuff we use? I.e. the new VOP-design is one block with multiple video-ports So for rk3568 I see #define SRST_A_VOP #define SRST_H_VOP #define SRST_VOP0 #define SRST_VOP1 #define SRST_VOP2 similarly rk3588 has #define SRST_H_VOP #define SRST_A_VOP #define SRST_D_VOP0 #define SRST_D_VOP1 #define SRST_D_VOP2 #define SRST_D_VOP3 as generalized reset lines. > > > --- > > .../display/rockchip/rockchip-vop2.yaml | 27 +++++++++++++++++++ > > 1 file changed, 27 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml b/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml > > index 2531726af306b..941fd059498d4 100644 > > --- a/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml > > +++ b/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml > > @@ -65,6 +65,22 @@ properties: > > - const: dclk_vp3 > > - const: pclk_vop > > > > + resets: > > + minItems: 3 > > + items: > > + - description: Pixel clock reset for video port 0. > > + - description: Pixel clock reset for video port 1. > > + - description: Pixel clock reset for video port 2. > > + - description: Pixel clock reset for video port 3. > > + > > + reset-names: > > + minItems: 3 > > + items: > > + - const: dclk_vp0 > > + - const: dclk_vp1 > > + - const: dclk_vp2 > > + - const: dclk_vp3 > > + > > rockchip,grf: > > $ref: /schemas/types.yaml#/definitions/phandle > > description: > > @@ -128,6 +144,11 @@ allOf: > > clock-names: > > minItems: 7 > > > > + resets: > > + minItems: 4 > > + reset-names: > > + minItems: 4 > > + > > ports: > > required: > > - port@0 > > @@ -183,6 +204,12 @@ examples: > > "dclk_vp0", > > "dclk_vp1", > > "dclk_vp2"; > > + resets = <&cru SRST_VOP0>, > > + <&cru SRST_VOP1>, > > + <&cru SRST_VOP2>; > > + reset-names = "dclk_vp0", > > + "dclk_vp1", > > + "dclk_vp2"; > > power-domains = <&power RK3568_PD_VO>; > > iommus = <&vop_mmu>; > > vop_out: ports { >
On Wednesday, May 15, 2024 12:33:22 P.M. EDT Heiko Stübner wrote: > Am Mittwoch, 15. Mai 2024, 18:19:29 CEST schrieb Conor Dooley: > > On Tue, May 14, 2024 at 11:19:47AM -0400, Detlev Casanova wrote: > > > Add the documentation for VOP2 video ports reset clocks. > > > One reset can be set per video port. > > > > > > Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com> > > > > Are these resets valid for all VOPs or just the one on 3588? > > Not in that form. > I.e. rk3588 has 4 video-ports (0-3), while rk3568 has 3 (0-2). > > So the binding should take into account that rk3568 also has the > SRST_VOP0 ... SRST_VOP2. That is what is set in the example and the reason why I set minItems to 3 in the main bindings. Then, the rk3588 specific part sets it to 4. Isn't that enough ? > Also, I guess we might not want to limit ourself to stuff we use? > I.e. the new VOP-design is one block with multiple video-ports > > So for rk3568 I see > #define SRST_A_VOP > #define SRST_H_VOP > #define SRST_VOP0 > #define SRST_VOP1 > #define SRST_VOP2 > > similarly rk3588 has > > #define SRST_H_VOP > #define SRST_A_VOP > #define SRST_D_VOP0 > #define SRST_D_VOP1 > #define SRST_D_VOP2 > #define SRST_D_VOP3 > > as generalized reset lines. Argh, I added them at first then removed them as they are not used. Will add them again then. Detlev. > > > --- > > > > > > .../display/rockchip/rockchip-vop2.yaml | 27 +++++++++++++++++++ > > > 1 file changed, 27 insertions(+) > > > > > > diff --git > > > a/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml > > > b/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml > > > index 2531726af306b..941fd059498d4 100644 > > > --- > > > a/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml > > > +++ > > > b/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml > > > > > > @@ -65,6 +65,22 @@ properties: > > > - const: dclk_vp3 > > > - const: pclk_vop > > > > > > + resets: > > > + minItems: 3 > > > + items: > > > + - description: Pixel clock reset for video port 0. > > > + - description: Pixel clock reset for video port 1. > > > + - description: Pixel clock reset for video port 2. > > > + - description: Pixel clock reset for video port 3. > > > + > > > + reset-names: > > > + minItems: 3 > > > + items: > > > + - const: dclk_vp0 > > > + - const: dclk_vp1 > > > + - const: dclk_vp2 > > > + - const: dclk_vp3 > > > + > > > > > > rockchip,grf: > > > $ref: /schemas/types.yaml#/definitions/phandle > > > > > > description: > > > @@ -128,6 +144,11 @@ allOf: > > > clock-names: > > > minItems: 7 > > > > > > + resets: > > > + minItems: 4 > > > + reset-names: > > > + minItems: 4 > > > + > > > > > > ports: > > > required: > > > - port@0 > > > > > > @@ -183,6 +204,12 @@ examples: > > > "dclk_vp0", > > > "dclk_vp1", > > > "dclk_vp2"; > > > > > > + resets = <&cru SRST_VOP0>, > > > + <&cru SRST_VOP1>, > > > + <&cru SRST_VOP2>; > > > + reset-names = "dclk_vp0", > > > + "dclk_vp1", > > > + "dclk_vp2"; > > > > > > power-domains = <&power RK3568_PD_VO>; > > > iommus = <&vop_mmu>; > > > vop_out: ports {
On Tue, May 21, 2024 at 01:15:46PM -0400, Detlev Casanova wrote: > On Wednesday, May 15, 2024 12:33:22 P.M. EDT Heiko Stübner wrote: > > Am Mittwoch, 15. Mai 2024, 18:19:29 CEST schrieb Conor Dooley: > > > On Tue, May 14, 2024 at 11:19:47AM -0400, Detlev Casanova wrote: > > > > Add the documentation for VOP2 video ports reset clocks. > > > > One reset can be set per video port. > > > > > > > > Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com> > > > > > > Are these resets valid for all VOPs or just the one on 3588? > > > > Not in that form. > > I.e. rk3588 has 4 video-ports (0-3), while rk3568 has 3 (0-2). > > > > So the binding should take into account that rk3568 also has the > > SRST_VOP0 ... SRST_VOP2. > > That is what is set in the example and the reason why I set minItems to 3 in > the main bindings. > Then, the rk3588 specific part sets it to 4. > > Isn't that enough ? Not quite - you need to restrict maxItems to 3 for the other devices if the clocks are not valid. What you've got says that 4 clocks are possible but not needed on !rk3588. Cheers, Conor.
On Tuesday, May 21, 2024 2:31:51 P.M. EDT Conor Dooley wrote: > On Tue, May 21, 2024 at 01:15:46PM -0400, Detlev Casanova wrote: > > On Wednesday, May 15, 2024 12:33:22 P.M. EDT Heiko Stübner wrote: > > > Am Mittwoch, 15. Mai 2024, 18:19:29 CEST schrieb Conor Dooley: > > > > On Tue, May 14, 2024 at 11:19:47AM -0400, Detlev Casanova wrote: > > > > > Add the documentation for VOP2 video ports reset clocks. > > > > > One reset can be set per video port. > > > > > > > > > > Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com> > > > > > > > > Are these resets valid for all VOPs or just the one on 3588? > > > > > > Not in that form. > > > I.e. rk3588 has 4 video-ports (0-3), while rk3568 has 3 (0-2). > > > > > > So the binding should take into account that rk3568 also has the > > > SRST_VOP0 ... SRST_VOP2. > > > > That is what is set in the example and the reason why I set minItems to 3 > > in the main bindings. > > Then, the rk3588 specific part sets it to 4. > > > > Isn't that enough ? > > Not quite - you need to restrict maxItems to 3 for the other devices if > the clocks are not valid. What you've got says that 4 clocks are > possible but not needed on !rk3588. > > Cheers, > Conor. I don't understand what "properties: resets: minItems: 3" means then. I thought it means that all devices should have at least 3 resets. Then the allOf below specifies the special case of rk3588 which has a minimum of 4 resets. Do I need to add resets: minItems: 3 reset-names: minItems: 3 in the "else:" ? So in that case, I can remove "properties: resets: minItems: 3" above ? Also, what do you mean "If the clocks are not valid" ? Detlev.
On Wed, May 22, 2024 at 11:31:36AM -0400, Detlev Casanova wrote: > On Tuesday, May 21, 2024 2:31:51 P.M. EDT Conor Dooley wrote: > > On Tue, May 21, 2024 at 01:15:46PM -0400, Detlev Casanova wrote: > > > On Wednesday, May 15, 2024 12:33:22 P.M. EDT Heiko Stübner wrote: > > > > Am Mittwoch, 15. Mai 2024, 18:19:29 CEST schrieb Conor Dooley: > > > > > On Tue, May 14, 2024 at 11:19:47AM -0400, Detlev Casanova wrote: > > > > > > Add the documentation for VOP2 video ports reset clocks. > > > > > > One reset can be set per video port. > > > > > > > > > > > > Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com> > > > > > > > > > > Are these resets valid for all VOPs or just the one on 3588? > > > > > > > > Not in that form. > > > > I.e. rk3588 has 4 video-ports (0-3), while rk3568 has 3 (0-2). > > > > > > > > So the binding should take into account that rk3568 also has the > > > > SRST_VOP0 ... SRST_VOP2. > > > > > > That is what is set in the example and the reason why I set minItems to 3 > > > in the main bindings. > > > Then, the rk3588 specific part sets it to 4. > > > > > > Isn't that enough ? > > > > Not quite - you need to restrict maxItems to 3 for the other devices if > > the clocks are not valid. What you've got says that 4 clocks are > > possible but not needed on !rk3588. > > > I don't understand what "properties: resets: minItems: 3" means then. I > thought it means that all devices should have at least 3 resets. Then the > allOf below specifies the special case of rk3588 which has a minimum of 4 > resets. The change you made to the bindings allows someone to define either 3 (because of minItems 3) or 4 (because there are 4 items in the list) resets for the rk3568. > Do I need to add > resets: > minItems: 3 > reset-names: > minItems: 3 > in the "else:" ? No, you need to add maxItems: 3 to the else. > So in that case, I can remove "properties: resets: minItems: 3" above ? > > Also, what do you mean "If the clocks are not valid" ? s/clocks/resets/ ;)
diff --git a/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml b/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml index 2531726af306b..941fd059498d4 100644 --- a/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml +++ b/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml @@ -65,6 +65,22 @@ properties: - const: dclk_vp3 - const: pclk_vop + resets: + minItems: 3 + items: + - description: Pixel clock reset for video port 0. + - description: Pixel clock reset for video port 1. + - description: Pixel clock reset for video port 2. + - description: Pixel clock reset for video port 3. + + reset-names: + minItems: 3 + items: + - const: dclk_vp0 + - const: dclk_vp1 + - const: dclk_vp2 + - const: dclk_vp3 + rockchip,grf: $ref: /schemas/types.yaml#/definitions/phandle description: @@ -128,6 +144,11 @@ allOf: clock-names: minItems: 7 + resets: + minItems: 4 + reset-names: + minItems: 4 + ports: required: - port@0 @@ -183,6 +204,12 @@ examples: "dclk_vp0", "dclk_vp1", "dclk_vp2"; + resets = <&cru SRST_VOP0>, + <&cru SRST_VOP1>, + <&cru SRST_VOP2>; + reset-names = "dclk_vp0", + "dclk_vp1", + "dclk_vp2"; power-domains = <&power RK3568_PD_VO>; iommus = <&vop_mmu>; vop_out: ports {
Add the documentation for VOP2 video ports reset clocks. One reset can be set per video port. Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com> --- .../display/rockchip/rockchip-vop2.yaml | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+)