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

2021-03-26 Thread Philipp Zabel
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 
> ---
> 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 
>  #include 
>  
> -vpu: video-codec@3830 {
> +vpu_ctrl: syscon@3832 {
> + compatible = "nxp,imx8mq-vpu-ctrl", "syscon";
> + reg = <0x3832 0x1>;
> +};
> +
> +vpu_g1: video-codec@3830 {
>  compatible = "nxp,imx8mq-vpu";
> -reg = <0x3830 0x1>,
> -  <0x3831 0x1>,
> -  <0x3832 0x1>;
> -reg-names = "g1", "g2", "ctrl";
> -interrupts = ,
> - ;
> -interrupt-names = "g1", "g2";
> +reg = <0x3830 0x1>;
> +interrupts = ;
> +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
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v6 03/13] media: hantro: Use syscon instead of 'ctrl' register

2021-03-26 Thread Philipp Zabel
On Thu, Mar 18, 2021 at 09:20:36AM +0100, Benjamin Gaignard wrote:
> In order to be able to share the control hardware block between
> VPUs use a syscon instead a ioremap it in the driver.
> To keep the compatibility with older DT if 'nxp,imx8mq-vpu-ctrl'
> phandle is not found look at 'ctrl' reg-name.
> With the method it becomes useless to provide a list of register
> names so remove it.
> 
> Signed-off-by: Benjamin Gaignard 
> ---
> version 5:
>  - use syscon instead of VPU reset driver.
>  - if DT doesn't provide syscon keep backward compatibilty by using
>'ctrl' reg-name.
> 
>  drivers/staging/media/hantro/hantro.h   |  5 +-
>  drivers/staging/media/hantro/imx8m_vpu_hw.c | 52 -
>  2 files changed, 34 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/staging/media/hantro/hantro.h 
> b/drivers/staging/media/hantro/hantro.h
> index 65f9f7ea7dcf..a99a96b84b5e 100644
> --- a/drivers/staging/media/hantro/hantro.h
> +++ b/drivers/staging/media/hantro/hantro.h
> @@ -13,6 +13,7 @@
>  #define HANTRO_H_
>  
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -167,7 +168,7 @@ hantro_vdev_to_func(struct video_device *vdev)
>   * @reg_bases:   Mapped addresses of VPU registers.
>   * @enc_base:Mapped address of VPU encoder register for 
> convenience.
>   * @dec_base:Mapped address of VPU decoder register for 
> convenience.
> - * @ctrl_base:   Mapped address of VPU control block.
> + * @ctrl_base:   Regmap of VPU control block.
>   * @vpu_mutex:   Mutex to synchronize V4L2 calls.
>   * @irqlock: Spinlock to synchronize access to data structures
>   *   shared with interrupt handlers.
> @@ -186,7 +187,7 @@ struct hantro_dev {
>   void __iomem **reg_bases;
>   void __iomem *enc_base;
>   void __iomem *dec_base;
> - void __iomem *ctrl_base;
> + struct regmap *ctrl_base;
>  
>   struct mutex vpu_mutex; /* video_device lock */
>   spinlock_t irqlock;
> diff --git a/drivers/staging/media/hantro/imx8m_vpu_hw.c 
> b/drivers/staging/media/hantro/imx8m_vpu_hw.c
> index c222de075ef4..bd9d135dd440 100644
> --- a/drivers/staging/media/hantro/imx8m_vpu_hw.c
> +++ b/drivers/staging/media/hantro/imx8m_vpu_hw.c
> @@ -7,6 +7,7 @@
>  
>  #include 
>  #include 
> +#include 
>  
>  #include "hantro.h"
>  #include "hantro_jpeg.h"
> @@ -24,30 +25,28 @@
>  #define CTRL_G1_PP_FUSE  0x0c
>  #define CTRL_G2_DEC_FUSE 0x10
>  
> +static const struct regmap_config ctrl_regmap_ctrl = {
> + .reg_bits = 32,
> + .val_bits = 32,
> + .reg_stride = 0x14,
> +};
> +
>  static void imx8m_soft_reset(struct hantro_dev *vpu, u32 reset_bits)
>  {
> - u32 val;
> -
>   /* Assert */
> - val = readl(vpu->ctrl_base + CTRL_SOFT_RESET);
> - val &= ~reset_bits;
> - writel(val, vpu->ctrl_base + CTRL_SOFT_RESET);
> + regmap_update_bits(vpu->ctrl_base, CTRL_SOFT_RESET, reset_bits, 0);
>  
>   udelay(2);
>  
>   /* Release */
> - val = readl(vpu->ctrl_base + CTRL_SOFT_RESET);
> - val |= reset_bits;
> - writel(val, vpu->ctrl_base + CTRL_SOFT_RESET);
> + regmap_update_bits(vpu->ctrl_base, CTRL_SOFT_RESET,
> +reset_bits, reset_bits);
>  }
>  
>  static void imx8m_clk_enable(struct hantro_dev *vpu, u32 clock_bits)
>  {
> - u32 val;
> -
> - val = readl(vpu->ctrl_base + CTRL_CLOCK_ENABLE);
> - val |= clock_bits;
> - writel(val, vpu->ctrl_base + CTRL_CLOCK_ENABLE);
> + regmap_update_bits(vpu->ctrl_base, CTRL_CLOCK_ENABLE,
> +clock_bits, clock_bits);
>  }
>  
>  static int imx8mq_runtime_resume(struct hantro_dev *vpu)
> @@ -64,9 +63,9 @@ static int imx8mq_runtime_resume(struct hantro_dev *vpu)
>   imx8m_clk_enable(vpu, CLOCK_G1 | CLOCK_G2);
>  
>   /* Set values of the fuse registers */
> - writel(0x, vpu->ctrl_base + CTRL_G1_DEC_FUSE);
> - writel(0x, vpu->ctrl_base + CTRL_G1_PP_FUSE);
> - writel(0x, vpu->ctrl_base + CTRL_G2_DEC_FUSE);
> + regmap_write(vpu->ctrl_base, CTRL_G1_DEC_FUSE, 0x);
> + regmap_write(vpu->ctrl_base, CTRL_G1_PP_FUSE, 0x);
> + regmap_write(vpu->ctrl_base, CTRL_G2_DEC_FUSE, 0x);
>  
>   clk_bulk_disable_unprepare(vpu->variant->num_clocks, vpu->clocks);
>  
> @@ -150,8 +149,22 @@ static irqreturn_t imx8m_vpu_g1_irq(int irq, void 
> *dev_id)
>  
>  static int imx8mq_vpu_hw_init(struct hantro_dev *vpu)
>  {
> - vpu->dec_base = vpu->reg_bases[0];
> - vpu->ctrl_base = vpu->reg_bases[vpu->variant->num_regs - 1];
> + struct device_node *np = vpu->dev->of_node;
> +
> + vpu->ctrl_base = syscon_regmap_lookup_by_phandle(np, 
> "nxp,imx8mq-vpu-ctrl");

I think calling this nxp,imx8m-vpu-ctrl would allow to share this with
i.MX8MM later. Otherwise,

Reviewed-by: Philipp Zabel 

thanks
Philipp
___
devel ma

Re: [PATCH v6 12/13] media: hantro: IMX8M: add variant for G2/HEVC codec

2021-03-26 Thread Philipp Zabel
On Thu, Mar 18, 2021 at 09:20:45AM +0100, Benjamin Gaignard wrote:
> Add variant to IMX8M to enable G2/HEVC codec.
> Define the capabilities for the hardware up to 3840x2160.
> G2 doesn't have postprocessor, use the same clocks and got it
> own interruption.
> 
> Signed-off-by: Benjamin Gaignard 

Reviewed-by: Philipp Zabel 

regards
Philipp
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v6 13/13] arm64: dts: imx8mq: Add node to G2 hardware

2021-03-26 Thread Philipp Zabel
On Thu, Mar 18, 2021 at 09:20:46AM +0100, Benjamin Gaignard wrote:
> Split VPU node in two: one for G1 and one for G2 since they are
> different hardware blocks.
> Add syscon for hardware control block.
> Remove reg-names property that is useless.
> Each VPU node only need one interrupt.
> 
> Signed-off-by: Benjamin Gaignard 
> ---
> version 5:
>  - use syscon instead of VPU reset
> 
>  arch/arm64/boot/dts/freescale/imx8mq.dtsi | 43 ++-
>  1 file changed, 34 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi 
> b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> index 17c449e12c2e..b537d153ebbd 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> @@ -1329,15 +1329,16 @@ usb3_phy1: usb-phy@382f0040 {
>   status = "disabled";
>   };
>  
> - vpu: video-codec@3830 {
> + vpu_ctrl: syscon@3832 {
> + compatible = "nxp,imx8mq-vpu-ctrl", "syscon";
> + reg = <0x3832 0x1>;
> + };
> +
> + vpu_g1: video-codec@3830 {
>   compatible = "nxp,imx8mq-vpu";
> - reg = <0x3830 0x1>,
> -   <0x3831 0x1>,
> -   <0x3832 0x1>;
> - reg-names = "g1", "g2", "ctrl";
> - interrupts = ,
> -  ;
> - interrupt-names = "g1", "g2";
> + reg = <0x3830 0x1>;
> + interrupts = ;
> + interrupt-names = "g1";
>   clocks = <&clk IMX8MQ_CLK_VPU_G1_ROOT>,
><&clk IMX8MQ_CLK_VPU_G2_ROOT>,
><&clk IMX8MQ_CLK_VPU_DEC_ROOT>;
> @@ -1350,9 +1351,33 @@ vpu: video-codec@3830 {
><&clk IMX8MQ_VPU_PLL_OUT>,
><&clk IMX8MQ_SYS1_PLL_800M>,
><&clk IMX8MQ_VPU_PLL>;
> - assigned-clock-rates = <6>, <6>,
> + assigned-clock-rates = <6>, <3>,

I'd like to see this mentioned in the commit message.

> +<8>, <0>;
> + power-domains = <&pgc_vpu>;
> + nxp,imx8mq-vpu-ctrl = <&vpu_ctrl>;
> + };
> +
> + vpu_g2: video-codec@3831 {
> + compatible = "nxp,imx8mq-vpu-g2";
> + reg = <0x3831 0x1>;
> + interrupts = ;
> + 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";
> + assigned-clocks = <&clk IMX8MQ_CLK_VPU_G1>,

Can the G1 clock configuration be dropped from the G2 device node and
the G2 clock configuration from the G1 device node? It looks weird that
these devices configure each other's clocks.

regards
Philipp
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


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

2021-03-26 Thread Benjamin Gaignard


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 
---
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 
  #include 
  
-vpu: video-codec@3830 {

+vpu_ctrl: syscon@3832 {
+ compatible = "nxp,imx8mq-vpu-ctrl", "syscon";
+ reg = <0x3832 0x1>;
+};
+
+vpu_g1: video-codec@3830 {
  compatible = "nxp,imx8mq-vpu";
-reg = <0x3830 0x1>,
-  <0x3831 0x1>,
-  <0x3832 0x1>;
-reg-names = "g1", "g2", "ctrl";
-interrupts = ,
- ;
-interrupt-names = "g1", "g2";
+reg = <0x3830 0x1>;
+interrupts = ;
+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


___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v6 13/13] arm64: dts: imx8mq: Add node to G2 hardware

2021-03-26 Thread Benjamin Gaignard


Le 26/03/2021 à 15:24, Philipp Zabel a écrit :

On Thu, Mar 18, 2021 at 09:20:46AM +0100, Benjamin Gaignard wrote:

Split VPU node in two: one for G1 and one for G2 since they are
different hardware blocks.
Add syscon for hardware control block.
Remove reg-names property that is useless.
Each VPU node only need one interrupt.

Signed-off-by: Benjamin Gaignard 
---
version 5:
  - use syscon instead of VPU reset

  arch/arm64/boot/dts/freescale/imx8mq.dtsi | 43 ++-
  1 file changed, 34 insertions(+), 9 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi 
b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
index 17c449e12c2e..b537d153ebbd 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
@@ -1329,15 +1329,16 @@ usb3_phy1: usb-phy@382f0040 {
status = "disabled";
};
  
-		vpu: video-codec@3830 {

+   vpu_ctrl: syscon@3832 {
+   compatible = "nxp,imx8mq-vpu-ctrl", "syscon";
+   reg = <0x3832 0x1>;
+   };
+
+   vpu_g1: video-codec@3830 {
compatible = "nxp,imx8mq-vpu";
-   reg = <0x3830 0x1>,
- <0x3831 0x1>,
- <0x3832 0x1>;
-   reg-names = "g1", "g2", "ctrl";
-   interrupts = ,
-;
-   interrupt-names = "g1", "g2";
+   reg = <0x3830 0x1>;
+   interrupts = ;
+   interrupt-names = "g1";
clocks = <&clk IMX8MQ_CLK_VPU_G1_ROOT>,
 <&clk IMX8MQ_CLK_VPU_G2_ROOT>,
 <&clk IMX8MQ_CLK_VPU_DEC_ROOT>;
@@ -1350,9 +1351,33 @@ vpu: video-codec@3830 {
 <&clk IMX8MQ_VPU_PLL_OUT>,
 <&clk IMX8MQ_SYS1_PLL_800M>,
 <&clk IMX8MQ_VPU_PLL>;
-   assigned-clock-rates = <6>, <6>,
+   assigned-clock-rates = <6>, <3>,

I'd like to see this mentioned in the commit message.


Yes I would do that.
The value comes from the datasheet.




+  <8>, <0>;
+   power-domains = <&pgc_vpu>;
+   nxp,imx8mq-vpu-ctrl = <&vpu_ctrl>;
+   };
+
+   vpu_g2: video-codec@3831 {
+   compatible = "nxp,imx8mq-vpu-g2";
+   reg = <0x3831 0x1>;
+   interrupts = ;
+   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";
+   assigned-clocks = <&clk IMX8MQ_CLK_VPU_G1>,

Can the G1 clock configuration be dropped from the G2 device node and
the G2 clock configuration from the G1 device node? It looks weird that
these devices configure each other's clocks.


No because if only one device node is enabled we need to configure the both
clocks anyway.

Benjamin



regards
Philipp


___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


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

2021-03-26 Thread Philipp Zabel
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 
> > > ---
> > > 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 
> > >   #include 
> > > -vpu: video-codec@3830 {
> > > +vpu_ctrl: syscon@3832 {
> > > + compatible = "nxp,imx8mq-vpu-ctrl", "syscon";
> > > + reg = <0x3832 0x1>;
> > > +};
> > > +
> > > +vpu_g1: video-codec@3830 {
> > >   compatible = "nxp,imx8mq-vpu";
> > > -reg = <0x3830 0x1>,
> > > -  <0x3831 0x1>,
> > > -  <0x3832 0x1>;
> > > -reg-names = "g1", "g2", "ctrl";
> > > -interrupts = ,
> > > - ;
> > > -interrupt-names = "g1", "g2";
> > > +reg = <0x3830 0x1>;
> > > +interrupts = ;
> > > +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 

regards
Philipp
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v6 13/13] arm64: dts: imx8mq: Add node to G2 hardware

2021-03-26 Thread Ezequiel Garcia
On Fri, 2021-03-26 at 15:33 +0100, Benjamin Gaignard wrote:
> 
> Le 26/03/2021 à 15:24, Philipp Zabel a écrit :
> > On Thu, Mar 18, 2021 at 09:20:46AM +0100, Benjamin Gaignard wrote:
> > > Split VPU node in two: one for G1 and one for G2 since they are
> > > different hardware blocks.
> > > Add syscon for hardware control block.
> > > Remove reg-names property that is useless.
> > > Each VPU node only need one interrupt.
> > > 
> > > Signed-off-by: Benjamin Gaignard 
> > > ---
> > > version 5:
> > >   - use syscon instead of VPU reset
> > > 
> > >   arch/arm64/boot/dts/freescale/imx8mq.dtsi | 43 ++-
> > >   1 file changed, 34 insertions(+), 9 deletions(-)
> > > 
> > > diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi 
> > > b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> > > index 17c449e12c2e..b537d153ebbd 100644
> > > --- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> > > +++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> > > @@ -1329,15 +1329,16 @@ usb3_phy1: usb-phy@382f0040 {
> > > status = "disabled";
> > > };
> > >   
> > > -   vpu: video-codec@3830 {
> > > +   vpu_ctrl: syscon@3832 {
> > > +   compatible = "nxp,imx8mq-vpu-ctrl", "syscon";
> > > +   reg = <0x3832 0x1>;
> > > +   };
> > > +
> > > +   vpu_g1: video-codec@3830 {
> > > compatible = "nxp,imx8mq-vpu";
> > > -   reg = <0x3830 0x1>,
> > > - <0x3831 0x1>,
> > > - <0x3832 0x1>;
> > > -   reg-names = "g1", "g2", "ctrl";
> > > -   interrupts = ,
> > > -    ;
> > > -   interrupt-names = "g1", "g2";
> > > +   reg = <0x3830 0x1>;
> > > +   interrupts = ;
> > > +   interrupt-names = "g1";
> > > clocks = <&clk IMX8MQ_CLK_VPU_G1_ROOT>,
> > >  <&clk IMX8MQ_CLK_VPU_G2_ROOT>,
> > >  <&clk IMX8MQ_CLK_VPU_DEC_ROOT>;
> > > @@ -1350,9 +1351,33 @@ vpu: video-codec@3830 {
> > >  <&clk 
> > > IMX8MQ_VPU_PLL_OUT>,
> > >  <&clk 
> > > IMX8MQ_SYS1_PLL_800M>,
> > >  <&clk IMX8MQ_VPU_PLL>;
> > > -   assigned-clock-rates = <6>, <6>,
> > > +   assigned-clock-rates = <6>, <3>,
> > I'd like to see this mentioned in the commit message.
> 
> Yes I would do that.
> The value comes from the datasheet.
> 
> > 
> > > +  <8>, <0>;
> > > +   power-domains = <&pgc_vpu>;
> > > +   nxp,imx8mq-vpu-ctrl = <&vpu_ctrl>;
> > > +   };
> > > +
> > > +   vpu_g2: video-codec@3831 {
> > > +   compatible = "nxp,imx8mq-vpu-g2";
> > > +   reg = <0x3831 0x1>;
> > > +   interrupts = ;
> > > +   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";
> > > +   assigned-clocks = <&clk IMX8MQ_CLK_VPU_G1>,
> > Can the G1 clock configuration be dropped from the G2 device node and
> > the G2 clock configuration from the G1 device node? It looks weird that
> > these devices configure each other's clocks.
> 
> No because if only one device node is enabled we need to configure the both
> clocks anyway.
> 

Since this is akward, how about adding a comment here in the dtsi to clarify it?

Thanks,
Ezequiel

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[staging:staging-next] BUILD SUCCESS 9c15db92a8e56bcde0f58064ac1adc28c0579b51

2021-03-26 Thread kernel test robot
fconfig
sh   sh2007_defconfig
powerpc mpc834x_mds_defconfig
nds32 allnoconfig
sparc64 defconfig
arm   omap1_defconfig
sparc64  alldefconfig
riscv nommu_k210_sdcard_defconfig
arm  simpad_defconfig
powerpc  mpc885_ads_defconfig
powerpc  pcm030_defconfig
mipsomega2p_defconfig
powerpc  katmai_defconfig
powerpc mpc8315_rdb_defconfig
ia64 allmodconfig
ia64defconfig
m68kdefconfig
m68k allyesconfig
nios2   defconfig
arc  allyesconfig
nds32   defconfig
nios2allyesconfig
cskydefconfig
alpha   defconfig
alphaallyesconfig
xtensa   allyesconfig
h8300allyesconfig
arc defconfig
sh   allmodconfig
parisc  defconfig
s390 allyesconfig
s390 allmodconfig
parisc   allyesconfig
s390defconfig
sparcallyesconfig
i386   tinyconfig
i386defconfig
mips allyesconfig
mips allmodconfig
powerpc   allnoconfig
x86_64   randconfig-a002-20210327
x86_64   randconfig-a003-20210327
x86_64   randconfig-a006-20210327
x86_64   randconfig-a001-20210327
x86_64   randconfig-a004-20210327
x86_64   randconfig-a005-20210327
i386 randconfig-a004-20210326
i386 randconfig-a003-20210326
i386 randconfig-a001-20210326
i386 randconfig-a002-20210326
i386 randconfig-a006-20210326
i386 randconfig-a005-20210326
x86_64   randconfig-a012-20210326
x86_64   randconfig-a015-20210326
x86_64   randconfig-a014-20210326
x86_64   randconfig-a013-20210326
x86_64   randconfig-a016-20210326
x86_64   randconfig-a011-20210326
i386 randconfig-a014-20210326
i386 randconfig-a011-20210326
i386 randconfig-a015-20210326
i386 randconfig-a016-20210326
i386 randconfig-a012-20210326
i386 randconfig-a013-20210326
riscvnommu_k210_defconfig
riscvnommu_virt_defconfig
riscv allnoconfig
riscv   defconfig
riscv  rv32_defconfig
um   allmodconfig
umallnoconfig
um   allyesconfig
um  defconfig
x86_64rhel-8.3-kselftests
x86_64  defconfig
x86_64   rhel-8.3
x86_64  rhel-8.3-kbuiltin
x86_64  kexec

clang tested configs:
x86_64   randconfig-a002-20210326
x86_64   randconfig-a003-20210326
x86_64   randconfig-a001-20210326
x86_64   randconfig-a006-20210326
x86_64   randconfig-a004-20210326
x86_64   randconfig-a005-20210326

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[staging:staging-testing] BUILD SUCCESS a5bf1a101a19dbb38be7ffebe2650449e344c892

2021-03-26 Thread kernel test robot
   ci20_defconfig
arcnsimosci_defconfig
ia64 allmodconfig
ia64defconfig
ia64 allyesconfig
m68k allmodconfig
m68kdefconfig
m68k allyesconfig
nios2   defconfig
arc  allyesconfig
nds32 allnoconfig
nds32   defconfig
nios2allyesconfig
cskydefconfig
alpha   defconfig
xtensa   allyesconfig
h8300allyesconfig
arc defconfig
sh   allmodconfig
parisc  defconfig
s390 allyesconfig
s390 allmodconfig
parisc   allyesconfig
s390defconfig
sparcallyesconfig
sparc   defconfig
i386   tinyconfig
i386defconfig
mips allyesconfig
mips allmodconfig
powerpc   allnoconfig
x86_64   randconfig-a002-20210327
x86_64   randconfig-a003-20210327
x86_64   randconfig-a006-20210327
x86_64   randconfig-a001-20210327
x86_64   randconfig-a004-20210327
x86_64   randconfig-a005-20210327
i386 randconfig-a004-20210326
i386 randconfig-a003-20210326
i386 randconfig-a001-20210326
i386 randconfig-a002-20210326
i386 randconfig-a006-20210326
i386 randconfig-a005-20210326
x86_64   randconfig-a012-20210326
x86_64   randconfig-a015-20210326
x86_64   randconfig-a014-20210326
x86_64   randconfig-a013-20210326
x86_64   randconfig-a016-20210326
x86_64   randconfig-a011-20210326
i386 randconfig-a014-20210326
i386 randconfig-a011-20210326
i386 randconfig-a015-20210326
i386 randconfig-a016-20210326
i386 randconfig-a012-20210326
i386 randconfig-a013-20210326
riscvnommu_k210_defconfig
riscvnommu_virt_defconfig
riscv allnoconfig
riscv   defconfig
riscv  rv32_defconfig
um   allmodconfig
umallnoconfig
um   allyesconfig
um  defconfig
x86_64rhel-8.3-kselftests
x86_64  defconfig
x86_64   rhel-8.3
x86_64  rhel-8.3-kbuiltin
x86_64  kexec

clang tested configs:
x86_64   randconfig-a002-20210326
x86_64   randconfig-a003-20210326
x86_64   randconfig-a001-20210326
x86_64   randconfig-a006-20210326
x86_64   randconfig-a004-20210326
x86_64   randconfig-a005-20210326

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[driver-core:driver-core-testing] BUILD SUCCESS ecdc996baf291b903342cc704f4086a88c361967

2021-03-26 Thread kernel test robot
h8300allyesconfig
arc defconfig
sh   allmodconfig
parisc  defconfig
s390 allyesconfig
s390 allmodconfig
parisc   allyesconfig
s390defconfig
sparcallyesconfig
sparc   defconfig
i386   tinyconfig
i386defconfig
mips allyesconfig
mips allmodconfig
powerpc   allnoconfig
x86_64   randconfig-a002-20210327
x86_64   randconfig-a003-20210327
x86_64   randconfig-a006-20210327
x86_64   randconfig-a001-20210327
x86_64   randconfig-a004-20210327
x86_64   randconfig-a005-20210327
i386 randconfig-a004-20210326
i386 randconfig-a003-20210326
i386 randconfig-a001-20210326
i386 randconfig-a002-20210326
i386 randconfig-a006-20210326
i386 randconfig-a005-20210326
x86_64   randconfig-a012-20210326
x86_64   randconfig-a015-20210326
x86_64   randconfig-a014-20210326
x86_64   randconfig-a013-20210326
x86_64   randconfig-a016-20210326
x86_64   randconfig-a011-20210326
i386 randconfig-a014-20210326
i386 randconfig-a011-20210326
i386 randconfig-a015-20210326
i386 randconfig-a016-20210326
i386 randconfig-a012-20210326
i386 randconfig-a013-20210326
riscvnommu_k210_defconfig
riscvnommu_virt_defconfig
riscv allnoconfig
riscv   defconfig
riscv  rv32_defconfig
um   allmodconfig
umallnoconfig
um   allyesconfig
um  defconfig
x86_64rhel-8.3-kselftests
x86_64  defconfig
x86_64   rhel-8.3
x86_64  rhel-8.3-kbuiltin
x86_64  kexec

clang tested configs:
x86_64   randconfig-a002-20210326
x86_64   randconfig-a003-20210326
x86_64   randconfig-a001-20210326
x86_64   randconfig-a006-20210326
x86_64   randconfig-a004-20210326
x86_64   randconfig-a005-20210326

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel