diff mbox series

[v6,02/13] dt-bindings: media: nxp,imx8mq-vpu: Update the bindings for G2 support

Message ID 20210318082046.51546-3-benjamin.gaignard@collabora.com (mailing list archive)
State New, archived
Headers show
Series Add HANTRO G2/HEVC decoder support for IMX8MQ | expand

Commit Message

Benjamin Gaignard March 18, 2021, 8:20 a.m. UTC
Introducing G2 hevc video decoder lead to modify the bindings to allow
to get one node per VPUs.
VPUs share one hardware control block which is provided as a phandle on
an syscon.
Each node got now one reg and one interrupt.
Add a compatible for G2 hardware block: nxp,imx8mq-vpu-g2.

To be compatible with older DT the driver is still capable to use 'ctrl'
reg-name even if it is deprecated now.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
---
version 5:
- This version doesn't break the backward compatibilty between kernel
  and DT.

 .../bindings/media/nxp,imx8mq-vpu.yaml        | 53 ++++++++++++-------
 1 file changed, 34 insertions(+), 19 deletions(-)

Comments

Rob Herring (Arm) March 23, 2021, 10:35 p.m. UTC | #1
On Thu, 18 Mar 2021 09:20:35 +0100, Benjamin Gaignard wrote:
> Introducing G2 hevc video decoder lead to modify the bindings to allow
> to get one node per VPUs.
> VPUs share one hardware control block which is provided as a phandle on
> an syscon.
> Each node got now one reg and one interrupt.
> Add a compatible for G2 hardware block: nxp,imx8mq-vpu-g2.
> 
> To be compatible with older DT the driver is still capable to use 'ctrl'
> reg-name even if it is deprecated now.
> 
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
> ---
> version 5:
> - This version doesn't break the backward compatibilty between kernel
>   and DT.
> 
>  .../bindings/media/nxp,imx8mq-vpu.yaml        | 53 ++++++++++++-------
>  1 file changed, 34 insertions(+), 19 deletions(-)
> 

Reviewed-by: Rob Herring <robh@kernel.org>
Philipp Zabel March 26, 2021, 2:11 p.m. UTC | #2
On Thu, Mar 18, 2021 at 09:20:35AM +0100, Benjamin Gaignard wrote:
> Introducing G2 hevc video decoder lead to modify the bindings to allow
> to get one node per VPUs.
> VPUs share one hardware control block which is provided as a phandle on
> an syscon.
> Each node got now one reg and one interrupt.
> Add a compatible for G2 hardware block: nxp,imx8mq-vpu-g2.
> 
> To be compatible with older DT the driver is still capable to use 'ctrl'
> reg-name even if it is deprecated now.
> 
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
> ---
> version 5:
> - This version doesn't break the backward compatibilty between kernel
>   and DT.
> 
>  .../bindings/media/nxp,imx8mq-vpu.yaml        | 53 ++++++++++++-------
>  1 file changed, 34 insertions(+), 19 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/nxp,imx8mq-vpu.yaml b/Documentation/devicetree/bindings/media/nxp,imx8mq-vpu.yaml
> index 762be3f96ce9..79502fc8bde5 100644
> --- a/Documentation/devicetree/bindings/media/nxp,imx8mq-vpu.yaml
> +++ b/Documentation/devicetree/bindings/media/nxp,imx8mq-vpu.yaml
> @@ -15,22 +15,18 @@ description:
>  
>  properties:
>    compatible:
> -    const: nxp,imx8mq-vpu
> +    oneOf:
> +      - const: nxp,imx8mq-vpu
> +      - const: nxp,imx8mq-vpu-g2
>  
>    reg:
> -    maxItems: 3
> -
> -  reg-names:
> -    items:
> -      - const: g1
> -      - const: g2
> -      - const: ctrl
> +    maxItems: 1
>  
>    interrupts:
> -    maxItems: 2
> +    maxItems: 1
>  
>    interrupt-names:
> -    items:
> +    oneOf:
>        - const: g1
>        - const: g2
>  
> @@ -46,14 +42,18 @@ properties:
>    power-domains:
>      maxItems: 1
>  
> +  nxp,imx8mq-vpu-ctrl:
> +    description: Specifies a phandle to syscon VPU hardware control block
> +    $ref: "/schemas/types.yaml#/definitions/phandle"
> +

Should we drop the 'q' here, i.e. nxp,imx8m-vpu-ctrl so we can use the same
binding for i.MX8MM later?

>  required:
>    - compatible
>    - reg
> -  - reg-names
>    - interrupts
>    - interrupt-names
>    - clocks
>    - clock-names
> +  - nxp,imx8mq-vpu-ctrl
>  
>  additionalProperties: false
>  
> @@ -62,18 +62,33 @@ examples:
>          #include <dt-bindings/clock/imx8mq-clock.h>
>          #include <dt-bindings/interrupt-controller/arm-gic.h>
>  
> -        vpu: video-codec@38300000 {
> +        vpu_ctrl: syscon@38320000 {
> +                 compatible = "nxp,imx8mq-vpu-ctrl", "syscon";
> +                 reg = <0x38320000 0x10000>;
> +        };
> +
> +        vpu_g1: video-codec@38300000 {
>                  compatible = "nxp,imx8mq-vpu";
> -                reg = <0x38300000 0x10000>,
> -                      <0x38310000 0x10000>,
> -                      <0x38320000 0x10000>;
> -                reg-names = "g1", "g2", "ctrl";
> -                interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>,
> -                             <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>;
> -                interrupt-names = "g1", "g2";
> +                reg = <0x38300000 0x10000>;
> +                interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>;
> +                interrupt-names = "g1";
> +                clocks = <&clk IMX8MQ_CLK_VPU_G1_ROOT>,
> +                         <&clk IMX8MQ_CLK_VPU_G2_ROOT>,

Does the G1 VPU require the G2 clock and vice versa?

regards
Philipp
Benjamin Gaignard March 26, 2021, 2:26 p.m. UTC | #3
Le 26/03/2021 à 15:11, Philipp Zabel a écrit :
> On Thu, Mar 18, 2021 at 09:20:35AM +0100, Benjamin Gaignard wrote:
>> Introducing G2 hevc video decoder lead to modify the bindings to allow
>> to get one node per VPUs.
>> VPUs share one hardware control block which is provided as a phandle on
>> an syscon.
>> Each node got now one reg and one interrupt.
>> Add a compatible for G2 hardware block: nxp,imx8mq-vpu-g2.
>>
>> To be compatible with older DT the driver is still capable to use 'ctrl'
>> reg-name even if it is deprecated now.
>>
>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
>> ---
>> version 5:
>> - This version doesn't break the backward compatibilty between kernel
>>    and DT.
>>
>>   .../bindings/media/nxp,imx8mq-vpu.yaml        | 53 ++++++++++++-------
>>   1 file changed, 34 insertions(+), 19 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/media/nxp,imx8mq-vpu.yaml b/Documentation/devicetree/bindings/media/nxp,imx8mq-vpu.yaml
>> index 762be3f96ce9..79502fc8bde5 100644
>> --- a/Documentation/devicetree/bindings/media/nxp,imx8mq-vpu.yaml
>> +++ b/Documentation/devicetree/bindings/media/nxp,imx8mq-vpu.yaml
>> @@ -15,22 +15,18 @@ description:
>>   
>>   properties:
>>     compatible:
>> -    const: nxp,imx8mq-vpu
>> +    oneOf:
>> +      - const: nxp,imx8mq-vpu
>> +      - const: nxp,imx8mq-vpu-g2
>>   
>>     reg:
>> -    maxItems: 3
>> -
>> -  reg-names:
>> -    items:
>> -      - const: g1
>> -      - const: g2
>> -      - const: ctrl
>> +    maxItems: 1
>>   
>>     interrupts:
>> -    maxItems: 2
>> +    maxItems: 1
>>   
>>     interrupt-names:
>> -    items:
>> +    oneOf:
>>         - const: g1
>>         - const: g2
>>   
>> @@ -46,14 +42,18 @@ properties:
>>     power-domains:
>>       maxItems: 1
>>   
>> +  nxp,imx8mq-vpu-ctrl:
>> +    description: Specifies a phandle to syscon VPU hardware control block
>> +    $ref: "/schemas/types.yaml#/definitions/phandle"
>> +
> Should we drop the 'q' here, i.e. nxp,imx8m-vpu-ctrl so we can use the same
> binding for i.MX8MM later?

I don't know if the control block is the same or not on IMX8MM, so I have only
put a compatible targeting IMX8MQ.

>
>>   required:
>>     - compatible
>>     - reg
>> -  - reg-names
>>     - interrupts
>>     - interrupt-names
>>     - clocks
>>     - clock-names
>> +  - nxp,imx8mq-vpu-ctrl
>>   
>>   additionalProperties: false
>>   
>> @@ -62,18 +62,33 @@ examples:
>>           #include <dt-bindings/clock/imx8mq-clock.h>
>>           #include <dt-bindings/interrupt-controller/arm-gic.h>
>>   
>> -        vpu: video-codec@38300000 {
>> +        vpu_ctrl: syscon@38320000 {
>> +                 compatible = "nxp,imx8mq-vpu-ctrl", "syscon";
>> +                 reg = <0x38320000 0x10000>;
>> +        };
>> +
>> +        vpu_g1: video-codec@38300000 {
>>                   compatible = "nxp,imx8mq-vpu";
>> -                reg = <0x38300000 0x10000>,
>> -                      <0x38310000 0x10000>,
>> -                      <0x38320000 0x10000>;
>> -                reg-names = "g1", "g2", "ctrl";
>> -                interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>,
>> -                             <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>;
>> -                interrupt-names = "g1", "g2";
>> +                reg = <0x38300000 0x10000>;
>> +                interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>;
>> +                interrupt-names = "g1";
>> +                clocks = <&clk IMX8MQ_CLK_VPU_G1_ROOT>,
>> +                         <&clk IMX8MQ_CLK_VPU_G2_ROOT>,
> Does the G1 VPU require the G2 clock and vice versa?

Yes either the control hardware block won't work.

Benjamin

>
> regards
> Philipp
>
Philipp Zabel March 26, 2021, 2:37 p.m. UTC | #4
On Fri, Mar 26, 2021 at 03:26:15PM +0100, Benjamin Gaignard wrote:
> 
> Le 26/03/2021 à 15:11, Philipp Zabel a écrit :
> > On Thu, Mar 18, 2021 at 09:20:35AM +0100, Benjamin Gaignard wrote:
> > > Introducing G2 hevc video decoder lead to modify the bindings to allow
> > > to get one node per VPUs.
> > > VPUs share one hardware control block which is provided as a phandle on
> > > an syscon.
> > > Each node got now one reg and one interrupt.
> > > Add a compatible for G2 hardware block: nxp,imx8mq-vpu-g2.
> > > 
> > > To be compatible with older DT the driver is still capable to use 'ctrl'
> > > reg-name even if it is deprecated now.
> > > 
> > > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
> > > ---
> > > version 5:
> > > - This version doesn't break the backward compatibilty between kernel
> > >    and DT.
> > > 
> > >   .../bindings/media/nxp,imx8mq-vpu.yaml        | 53 ++++++++++++-------
> > >   1 file changed, 34 insertions(+), 19 deletions(-)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/media/nxp,imx8mq-vpu.yaml b/Documentation/devicetree/bindings/media/nxp,imx8mq-vpu.yaml
> > > index 762be3f96ce9..79502fc8bde5 100644
> > > --- a/Documentation/devicetree/bindings/media/nxp,imx8mq-vpu.yaml
> > > +++ b/Documentation/devicetree/bindings/media/nxp,imx8mq-vpu.yaml
> > > @@ -15,22 +15,18 @@ description:
> > >   properties:
> > >     compatible:
> > > -    const: nxp,imx8mq-vpu
> > > +    oneOf:
> > > +      - const: nxp,imx8mq-vpu
> > > +      - const: nxp,imx8mq-vpu-g2
> > >     reg:
> > > -    maxItems: 3
> > > -
> > > -  reg-names:
> > > -    items:
> > > -      - const: g1
> > > -      - const: g2
> > > -      - const: ctrl
> > > +    maxItems: 1
> > >     interrupts:
> > > -    maxItems: 2
> > > +    maxItems: 1
> > >     interrupt-names:
> > > -    items:
> > > +    oneOf:
> > >         - const: g1
> > >         - const: g2
> > > @@ -46,14 +42,18 @@ properties:
> > >     power-domains:
> > >       maxItems: 1
> > > +  nxp,imx8mq-vpu-ctrl:
> > > +    description: Specifies a phandle to syscon VPU hardware control block
> > > +    $ref: "/schemas/types.yaml#/definitions/phandle"
> > > +
> > Should we drop the 'q' here, i.e. nxp,imx8m-vpu-ctrl so we can use the same
> > binding for i.MX8MM later?
> 
> I don't know if the control block is the same or not on IMX8MM, so I have only
> put a compatible targeting IMX8MQ.

Oh, the compatible property of the control handle node can be different.
I'm just suggesting that this phandle property be called the same.
Otherwise we'd have to add another nxp,imx8mm-vpu-ctrl property and then
mark either of the two as required, depending on the compatible.

> > 
> > >   required:
> > >     - compatible
> > >     - reg
> > > -  - reg-names
> > >     - interrupts
> > >     - interrupt-names
> > >     - clocks
> > >     - clock-names
> > > +  - nxp,imx8mq-vpu-ctrl
> > >   additionalProperties: false
> > > @@ -62,18 +62,33 @@ examples:
> > >           #include <dt-bindings/clock/imx8mq-clock.h>
> > >           #include <dt-bindings/interrupt-controller/arm-gic.h>
> > > -        vpu: video-codec@38300000 {
> > > +        vpu_ctrl: syscon@38320000 {
> > > +                 compatible = "nxp,imx8mq-vpu-ctrl", "syscon";
> > > +                 reg = <0x38320000 0x10000>;
> > > +        };
> > > +
> > > +        vpu_g1: video-codec@38300000 {
> > >                   compatible = "nxp,imx8mq-vpu";
> > > -                reg = <0x38300000 0x10000>,
> > > -                      <0x38310000 0x10000>,
> > > -                      <0x38320000 0x10000>;
> > > -                reg-names = "g1", "g2", "ctrl";
> > > -                interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>,
> > > -                             <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>;
> > > -                interrupt-names = "g1", "g2";
> > > +                reg = <0x38300000 0x10000>;
> > > +                interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>;
> > > +                interrupt-names = "g1";
> > > +                clocks = <&clk IMX8MQ_CLK_VPU_G1_ROOT>,
> > > +                         <&clk IMX8MQ_CLK_VPU_G2_ROOT>,
> > Does the G1 VPU require the G2 clock and vice versa?
> 
> Yes either the control hardware block won't work.

Ok.

Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>

regards
Philipp
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/media/nxp,imx8mq-vpu.yaml b/Documentation/devicetree/bindings/media/nxp,imx8mq-vpu.yaml
index 762be3f96ce9..79502fc8bde5 100644
--- a/Documentation/devicetree/bindings/media/nxp,imx8mq-vpu.yaml
+++ b/Documentation/devicetree/bindings/media/nxp,imx8mq-vpu.yaml
@@ -15,22 +15,18 @@  description:
 
 properties:
   compatible:
-    const: nxp,imx8mq-vpu
+    oneOf:
+      - const: nxp,imx8mq-vpu
+      - const: nxp,imx8mq-vpu-g2
 
   reg:
-    maxItems: 3
-
-  reg-names:
-    items:
-      - const: g1
-      - const: g2
-      - const: ctrl
+    maxItems: 1
 
   interrupts:
-    maxItems: 2
+    maxItems: 1
 
   interrupt-names:
-    items:
+    oneOf:
       - const: g1
       - const: g2
 
@@ -46,14 +42,18 @@  properties:
   power-domains:
     maxItems: 1
 
+  nxp,imx8mq-vpu-ctrl:
+    description: Specifies a phandle to syscon VPU hardware control block
+    $ref: "/schemas/types.yaml#/definitions/phandle"
+
 required:
   - compatible
   - reg
-  - reg-names
   - interrupts
   - interrupt-names
   - clocks
   - clock-names
+  - nxp,imx8mq-vpu-ctrl
 
 additionalProperties: false
 
@@ -62,18 +62,33 @@  examples:
         #include <dt-bindings/clock/imx8mq-clock.h>
         #include <dt-bindings/interrupt-controller/arm-gic.h>
 
-        vpu: video-codec@38300000 {
+        vpu_ctrl: syscon@38320000 {
+                 compatible = "nxp,imx8mq-vpu-ctrl", "syscon";
+                 reg = <0x38320000 0x10000>;
+        };
+
+        vpu_g1: video-codec@38300000 {
                 compatible = "nxp,imx8mq-vpu";
-                reg = <0x38300000 0x10000>,
-                      <0x38310000 0x10000>,
-                      <0x38320000 0x10000>;
-                reg-names = "g1", "g2", "ctrl";
-                interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>,
-                             <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>;
-                interrupt-names = "g1", "g2";
+                reg = <0x38300000 0x10000>;
+                interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>;
+                interrupt-names = "g1";
+                clocks = <&clk IMX8MQ_CLK_VPU_G1_ROOT>,
+                         <&clk IMX8MQ_CLK_VPU_G2_ROOT>,
+                         <&clk IMX8MQ_CLK_VPU_DEC_ROOT>;
+                clock-names = "g1", "g2", "bus";
+                power-domains = <&pgc_vpu>;
+                nxp,imx8mq-vpu-ctrl = <&vpu_ctrl>;
+        };
+
+        vpu_g2: video-codec@38310000 {
+                compatible = "nxp,imx8mq-vpu-g2";
+                reg = <0x38300000 0x10000>;
+                interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>;
+                interrupt-names = "g2";
                 clocks = <&clk IMX8MQ_CLK_VPU_G1_ROOT>,
                          <&clk IMX8MQ_CLK_VPU_G2_ROOT>,
                          <&clk IMX8MQ_CLK_VPU_DEC_ROOT>;
                 clock-names = "g1", "g2", "bus";
                 power-domains = <&pgc_vpu>;
+                nxp,imx8mq-vpu-ctrl = <&vpu_ctrl>;
         };