[Patch 0/6] ARM: dts/hwmod: Add CAL and VPE nodes

[Patch 0/6] ARM: dts/hwmod: Add CAL and VPE nodes

[Benoit Parrot]

This patch series adds the needed HWMOD and DTSI nodes
for the CAL and VPE modules.
We also document the VPE DT bindings.

Benoit Parrot (6):
  ARM: dts: DRA72: Add CAL dtsi node
  ARM: DRA7: hwmod: Add CAL nodes
  ARM: OMAP: DRA7xx: Make CAM clock domain SWSUP only
  dt-bindings: media: ti-vpe: Document VPE driver
  ARM: DRA7: hwmod: Add VPE nodes
  ARM: dts: dra7: Add VPE dtsi node

 Documentation/devicetree/bindings/media/ti-vpe.txt | 41 ++++++++++
 arch/arm/boot/dts/dra7.dtsi                        | 26 +++++++
 arch/arm/boot/dts/dra72x.dtsi                      | 31 ++++++++
 arch/arm/mach-omap2/clockdomains7xx_data.c         |  2 +-
 arch/arm/mach-omap2/omap_hwmod_7xx_data.c          | 87 ++++++++++++++++++++++
 5 files changed, 186 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/media/ti-vpe.txt

-- 
2.9.0

[Patch 1/6] ARM: dts: DRA72: Add CAL dtsi node

[Benoit Parrot]

This patch adds the required dtsi node to support the Camera
Adaptation Layer (CAL) for the DRA72 family of devices.

- Added CAL entry in dra72x.dtsi.

Signed-off-by: Benoit Parrot <bparrot@ti.com>
---
 arch/arm/boot/dts/dra72x.dtsi | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/arch/arm/boot/dts/dra72x.dtsi b/arch/arm/boot/dts/dra72x.dtsi
index 67107605fb4c..d0ba4f238084 100644
--- a/arch/arm/boot/dts/dra72x.dtsi
+++ b/arch/arm/boot/dts/dra72x.dtsi
@@ -17,6 +17,37 @@
 		interrupt-parent = <&wakeupgen>;
 		interrupts = <GIC_SPI 131 IRQ_TYPE_LEVEL_HIGH>;
 	};
+
+	ocp {
+		cal: cal@4845b000 {
+			compatible = "ti,dra72-cal";
+			ti,hwmods = "cal";
+			reg = <0x4845B000 0x400>,
+			      <0x4845B800 0x40>,
+			      <0x4845B900 0x40>,
+			      <0x4A002e94 0x4>;
+			reg-names = "cal_top",
+				    "cal_rx_core0",
+				    "cal_rx_core1",
+				    "camerrx_control";
+			interrupts = <GIC_SPI 119 IRQ_TYPE_LEVEL_HIGH>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "disabled";
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				csi2_0: port@0 {
+					reg = <0>;
+				};
+				csi2_1: port@1 {
+					reg = <1>;
+				};
+			};
+		};
+	};
 };
 
 &dss {
-- 
2.9.0

Re: [Patch 1/6] ARM: dts: DRA72: Add CAL dtsi node

[Tony Lindgren]

* Benoit Parrot <bparrot@ti.com> [171012 12:29]:
> This patch adds the required dtsi node to support the Camera
> Adaptation Layer (CAL) for the DRA72 family of devices.
> 
> - Added CAL entry in dra72x.dtsi.
> 
> Signed-off-by: Benoit Parrot <bparrot@ti.com>
> ---
>  arch/arm/boot/dts/dra72x.dtsi | 31 +++++++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/dra72x.dtsi b/arch/arm/boot/dts/dra72x.dtsi
> index 67107605fb4c..d0ba4f238084 100644
> --- a/arch/arm/boot/dts/dra72x.dtsi
> +++ b/arch/arm/boot/dts/dra72x.dtsi
> @@ -17,6 +17,37 @@
>  		interrupt-parent = <&wakeupgen>;
>  		interrupts = <GIC_SPI 131 IRQ_TYPE_LEVEL_HIGH>;
>  	};
> +
> +	ocp {
> +		cal: cal@4845b000 {
> +			compatible = "ti,dra72-cal";
> +			ti,hwmods = "cal";
> +			reg = <0x4845B000 0x400>,
> +			      <0x4845B800 0x40>,
> +			      <0x4845B900 0x40>,
> +			      <0x4A002e94 0x4>;

Care to fix the cAmelcasing here? All lower case hex is pretty much the
standard with linux, so might as well lower case them all while at it.

Regards,

Tony

Re: [Patch 1/6] ARM: dts: DRA72: Add CAL dtsi node

[Benoit Parrot]

Tony Lindgren <tony@atomide.com> wrote on Fri [2017-Oct-13 09:54:09 -0700]:
> * Benoit Parrot <bparrot@ti.com> [171012 12:29]:
> > This patch adds the required dtsi node to support the Camera
> > Adaptation Layer (CAL) for the DRA72 family of devices.
> > 
> > - Added CAL entry in dra72x.dtsi.
> > 
> > Signed-off-by: Benoit Parrot <bparrot@ti.com>
> > ---
> >  arch/arm/boot/dts/dra72x.dtsi | 31 +++++++++++++++++++++++++++++++
> >  1 file changed, 31 insertions(+)
> > 
> > diff --git a/arch/arm/boot/dts/dra72x.dtsi b/arch/arm/boot/dts/dra72x.dtsi
> > index 67107605fb4c..d0ba4f238084 100644
> > --- a/arch/arm/boot/dts/dra72x.dtsi
> > +++ b/arch/arm/boot/dts/dra72x.dtsi
> > @@ -17,6 +17,37 @@
> >  		interrupt-parent = <&wakeupgen>;
> >  		interrupts = <GIC_SPI 131 IRQ_TYPE_LEVEL_HIGH>;
> >  	};
> > +
> > +	ocp {
> > +		cal: cal@4845b000 {
> > +			compatible = "ti,dra72-cal";
> > +			ti,hwmods = "cal";
> > +			reg = <0x4845B000 0x400>,
> > +			      <0x4845B800 0x40>,
> > +			      <0x4845B900 0x40>,
> > +			      <0x4A002e94 0x4>;
> 
> Care to fix the cAmelcasing here? All lower case hex is pretty much the
> standard with linux, so might as well lower case them all while at it.

Sure thing, I'll fix it.

Benoit

> 
> Regards,
> 
> Tony

[Patch 2/6] ARM: DRA7: hwmod: Add CAL nodes

[Benoit Parrot]

This patch adds the required hwmod nodes to support the Camera
Adaptation Layer (CAL) for the DRA72 family of devices.

- Added CAL hwmod entry in the DRA72x section.

The DRA72x TRM (Literature Number SPRUHP2x) states that CAL only
support NO_IDLE, FORCE_IDLE and SMART_IDLE.
Although CAL does not support standby mode per se hwmod would not
enabled the functional/interface clock if module is not a master.
Hence the SWSUP mode flags ifor SIDLE ans MSTANDBY are also enabled.
This ensure that i/f clocks are available when CAL is enabled.

Signed-off-by: Benoit Parrot <bparrot@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod_7xx_data.c | 44 +++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
index 2f4f7002f38d..fc53b498975c 100644
--- a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
@@ -240,6 +240,41 @@ static struct omap_hwmod dra7xx_bb2d_hwmod = {
 };
 
 /*
+ * 'cal' class
+ *
+ */
+
+static struct omap_hwmod_class_sysconfig dra7xx_cal_sysc = {
+	.sysc_offs	= 0x0010,
+	.sysc_flags	= (SYSC_HAS_SIDLEMODE | SYSC_HAS_RESET_STATUS |
+			   SYSC_HAS_SOFTRESET | SYSC_HAS_MIDLEMODE),
+	.idlemodes	= (SIDLE_FORCE | SIDLE_NO | SIDLE_SMART |
+			   MSTANDBY_FORCE | MSTANDBY_NO),
+	.sysc_fields	= &omap_hwmod_sysc_type2,
+};
+
+static struct omap_hwmod_class dra7xx_cal_hwmod_class = {
+	.name	= "cal",
+	.sysc	= &dra7xx_cal_sysc,
+};
+
+/* cal */
+static struct omap_hwmod dra7xx_cal_hwmod = {
+	.name		= "cal",
+	.class		= &dra7xx_cal_hwmod_class,
+	.clkdm_name	= "cam_clkdm",
+	.main_clk	= "vip2_gclk_mux",
+	.flags		= (HWMOD_SWSUP_SIDLE | HWMOD_SWSUP_MSTANDBY),
+	.prcm = {
+		.omap4 = {
+			.clkctrl_offs = DRA7XX_CM_CAM_VIP2_CLKCTRL_OFFSET,
+			.context_offs = DRA7XX_RM_CAM_VIP2_CONTEXT_OFFSET,
+			.modulemode   = MODULEMODE_HWCTRL,
+		},
+	},
+};
+
+/*
  * 'counter' class
  *
  */
@@ -3901,6 +3936,14 @@ static struct omap_hwmod_ocp_if dra7xx_l4_per2__vcp2 = {
 	.user		= OCP_USER_MPU | OCP_USER_SDMA,
 };
 
+/* l4_per3 -> cal */
+static struct omap_hwmod_ocp_if dra7xx_l4_per3__cal = {
+	.master		= &dra7xx_l4_per3_hwmod,
+	.slave		= &dra7xx_cal_hwmod,
+	.clk		= "l3_iclk_div",
+	.user		= OCP_USER_MPU | OCP_USER_SDMA,
+};
+
 /* l4_wkup -> wd_timer2 */
 static struct omap_hwmod_ocp_if dra7xx_l4_wkup__wd_timer2 = {
 	.master		= &dra7xx_l4_wkup_hwmod,
@@ -4082,6 +4125,7 @@ static struct omap_hwmod_ocp_if *dra74x_hwmod_ocp_ifs[] __initdata = {
 };
 
 static struct omap_hwmod_ocp_if *dra72x_hwmod_ocp_ifs[] __initdata = {
+	&dra7xx_l4_per3__cal,
 	NULL,
 };
 
-- 
2.9.0

Re: [Patch 2/6] ARM: DRA7: hwmod: Add CAL nodes

[Tony Lindgren]

* Benoit Parrot <bparrot@ti.com> [171012 12:28]:
> This patch adds the required hwmod nodes to support the Camera
> Adaptation Layer (CAL) for the DRA72 family of devices.
...

> +static struct omap_hwmod_class_sysconfig dra7xx_cal_sysc = {
> +	.sysc_offs	= 0x0010,

Also has .rev_offs at 0 so please add that too.

Regards,

Tony

Re: [Patch 2/6] ARM: DRA7: hwmod: Add CAL nodes

[Benoit Parrot]

Tony Lindgren <tony@atomide.com> wrote on Fri [2017-Oct-13 09:56:54 -0700]:
> * Benoit Parrot <bparrot@ti.com> [171012 12:28]:
> > This patch adds the required hwmod nodes to support the Camera
> > Adaptation Layer (CAL) for the DRA72 family of devices.
> ...
> 
> > +static struct omap_hwmod_class_sysconfig dra7xx_cal_sysc = {
> > +	.sysc_offs	= 0x0010,
> 
> Also has .rev_offs at 0 so please add that too.

Ok, I'll add that.

Benoit

> 
> Regards,
> 
> Tony

[Patch 3/6] ARM: OMAP: DRA7xx: Make CAM clock domain SWSUP only

[Benoit Parrot]

HWSUP on this domain is only working when VIP1 probes.
If only VIP2 on DRA74x or CAL on DRA72x probes the domain does
not get enabled. This might indicates an issue in the HW Auto
state-machine for this domain.

Work around is to set the CAM domain to use SWSUP only.

Signed-off-by: Benoit Parrot <bparrot@ti.com>
---
 arch/arm/mach-omap2/clockdomains7xx_data.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mach-omap2/clockdomains7xx_data.c b/arch/arm/mach-omap2/clockdomains7xx_data.c
index 67ebff829cf2..dd37efa2c652 100644
--- a/arch/arm/mach-omap2/clockdomains7xx_data.c
+++ b/arch/arm/mach-omap2/clockdomains7xx_data.c
@@ -609,7 +609,7 @@ static struct clockdomain cam_7xx_clkdm = {
 	.dep_bit	  = DRA7XX_CAM_STATDEP_SHIFT,
 	.wkdep_srcs	  = cam_wkup_sleep_deps,
 	.sleepdep_srcs	  = cam_wkup_sleep_deps,
-	.flags		  = CLKDM_CAN_HWSUP_SWSUP,
+	.flags		  = CLKDM_CAN_SWSUP,
 };
 
 static struct clockdomain l4per_7xx_clkdm = {
-- 
2.9.0

Re: [Patch 3/6] ARM: OMAP: DRA7xx: Make CAM clock domain SWSUP only

[Tony Lindgren]

* Benoit Parrot <bparrot@ti.com> [171012 12:29]:
> HWSUP on this domain is only working when VIP1 probes.
> If only VIP2 on DRA74x or CAL on DRA72x probes the domain does
> not get enabled. This might indicates an issue in the HW Auto
> state-machine for this domain.
>
> Work around is to set the CAM domain to use SWSUP only.

Hmm this you might get fixed automatically by configuring the
parent interconnect target module to use "ti,sysc-omap4" and
adding VIP1 and VIP2 as children to it.

The reason why I suspect it will fix the issue is because
with the parent being "ti,sysc-omap4" with "ti,hwmods" being
in that parent node too, you automatically get PM runtime
refcounting keep the parent active for either child.

Maybe give it a try against today's Linux next and see for
example how it was done for musb:

https://patchwork.kernel.org/patch/9978783/

Just use "ti,sysc-omap2" for type1 and "ti,sysc-omap4"
for type2.

Regards,

Tony

Re: [Patch 3/6] ARM: OMAP: DRA7xx: Make CAM clock domain SWSUP only

[Benoit Parrot]

Tony Lindgren <tony@atomide.com> wrote on Fri [2017-Oct-13 10:01:13 -0700]:
> * Benoit Parrot <bparrot@ti.com> [171012 12:29]:
> > HWSUP on this domain is only working when VIP1 probes.
> > If only VIP2 on DRA74x or CAL on DRA72x probes the domain does
> > not get enabled. This might indicates an issue in the HW Auto
> > state-machine for this domain.
> >
> > Work around is to set the CAM domain to use SWSUP only.
> 
> Hmm this you might get fixed automatically by configuring the
> parent interconnect target module to use "ti,sysc-omap4" and
> adding VIP1 and VIP2 as children to it.
> 
> The reason why I suspect it will fix the issue is because
> with the parent being "ti,sysc-omap4" with "ti,hwmods" being
> in that parent node too, you automatically get PM runtime
> refcounting keep the parent active for either child.
> 
> Maybe give it a try against today's Linux next and see for
> example how it was done for musb:
> 
> https://patchwork.kernel.org/patch/9978783/
> 
> Just use "ti,sysc-omap2" for type1 and "ti,sysc-omap4"
> for type2.

Hmm interesting, I'll give that a try and if it fixes it.

Benoit

> 
> Regards,
> 
> Tony

Re: [Patch 3/6] ARM: OMAP: DRA7xx: Make CAM clock domain SWSUP only

[Tony Lindgren]

* Benoit Parrot <bparrot@ti.com> [171013 11:01]:
> Tony Lindgren <tony@atomide.com> wrote on Fri [2017-Oct-13 10:01:13 -0700]:
> > * Benoit Parrot <bparrot@ti.com> [171012 12:29]:
> > > HWSUP on this domain is only working when VIP1 probes.
> > > If only VIP2 on DRA74x or CAL on DRA72x probes the domain does
> > > not get enabled. This might indicates an issue in the HW Auto
> > > state-machine for this domain.
> > >
> > > Work around is to set the CAM domain to use SWSUP only.
> > 
> > Hmm this you might get fixed automatically by configuring the
> > parent interconnect target module to use "ti,sysc-omap4" and
> > adding VIP1 and VIP2 as children to it.
> > 
> > The reason why I suspect it will fix the issue is because
> > with the parent being "ti,sysc-omap4" with "ti,hwmods" being
> > in that parent node too, you automatically get PM runtime
> > refcounting keep the parent active for either child.
> > 
> > Maybe give it a try against today's Linux next and see for
> > example how it was done for musb:
> > 
> > https://patchwork.kernel.org/patch/9978783/
> > 
> > Just use "ti,sysc-omap2" for type1 and "ti,sysc-omap4"
> > for type2.
> 
> Hmm interesting, I'll give that a try and if it fixes it.

OK great. Note that you need to manually apply "[PATCH] ARM:
head-common.S: Clear lr before jumping to start_kernel()" on
today's next and make sure CONFIG_DRM and CONFIG_DRM_KMS_HELPER
are built-in and not modules if using them.

Regards,

Tony

[Patch 4/6] dt-bindings: media: ti-vpe: Document VPE driver

[Benoit Parrot]

Device Tree bindings for the Video Processing Engine (VPE) driver.

Signed-off-by: Benoit Parrot <bparrot@ti.com>
---
 Documentation/devicetree/bindings/media/ti-vpe.txt | 41 ++++++++++++++++++++++
 1 file changed, 41 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/ti-vpe.txt

diff --git a/Documentation/devicetree/bindings/media/ti-vpe.txt b/Documentation/devicetree/bindings/media/ti-vpe.txt
new file mode 100644
index 000000000000..c2ef93d08417
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/ti-vpe.txt
@@ -0,0 +1,41 @@
+Texas Instruments DRA7x VIDEO PROCESSING ENGINE (VPE)
+------------------------------------------------------
+
+The Video Processing Engine (VPE) is a key component for image post
+processing applications. VPE consist of a single memory to memory
+path which can perform chroma up/down sampling, deinterlacing,
+scaling and color space conversion.
+
+Required properties:
+- compatible: must be "ti,vpe"
+- reg:	physical base address and length of the registers set for the 8
+	memory regions required;
+- reg-names: name associated with the memory regions described is <reg>;
+- interrupts: should contain IRQ line for VPE;
+
+Example:
+	vpe {
+		compatible = "ti,vpe";
+		ti,hwmods = "vpe";
+		clocks = <&dpll_core_h23x2_ck>;
+		clock-names = "fck";
+		reg =	<0x489d0000 0x120>,
+			<0x489d0300 0x20>,
+			<0x489d0400 0x20>,
+			<0x489d0500 0x20>,
+			<0x489d0600 0x3c>,
+			<0x489d0700 0x80>,
+			<0x489d5700 0x18>,
+			<0x489dd000 0x400>;
+		reg-names =	"vpe_top",
+				"vpe_chr_us0",
+				"vpe_chr_us1",
+				"vpe_chr_us2",
+				"vpe_dei",
+				"sc",
+				"csc",
+				"vpdma";
+		interrupts = <GIC_SPI 354 IRQ_TYPE_LEVEL_HIGH>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+	};
-- 
2.9.0

Re: [Patch 4/6] dt-bindings: media: ti-vpe: Document VPE driver

[Rob Herring]

On Thu, Oct 12, 2017 at 02:27:17PM -0500, Benoit Parrot wrote:
> Device Tree bindings for the Video Processing Engine (VPE) driver.
> 
> Signed-off-by: Benoit Parrot <bparrot@ti.com>
> ---
>  Documentation/devicetree/bindings/media/ti-vpe.txt | 41 ++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/ti-vpe.txt
> 
> diff --git a/Documentation/devicetree/bindings/media/ti-vpe.txt b/Documentation/devicetree/bindings/media/ti-vpe.txt
> new file mode 100644
> index 000000000000..c2ef93d08417
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/ti-vpe.txt
> @@ -0,0 +1,41 @@
> +Texas Instruments DRA7x VIDEO PROCESSING ENGINE (VPE)
> +------------------------------------------------------
> +
> +The Video Processing Engine (VPE) is a key component for image post
> +processing applications. VPE consist of a single memory to memory
> +path which can perform chroma up/down sampling, deinterlacing,
> +scaling and color space conversion.
> +
> +Required properties:
> +- compatible: must be "ti,vpe"

Needs SoC specific compatibles.

> +- reg:	physical base address and length of the registers set for the 8
> +	memory regions required;
> +- reg-names: name associated with the memory regions described is <reg>;
> +- interrupts: should contain IRQ line for VPE;
> +
> +Example:
> +	vpe {
> +		compatible = "ti,vpe";
> +		ti,hwmods = "vpe";
> +		clocks = <&dpll_core_h23x2_ck>;
> +		clock-names = "fck";
> +		reg =	<0x489d0000 0x120>,
> +			<0x489d0300 0x20>,
> +			<0x489d0400 0x20>,
> +			<0x489d0500 0x20>,
> +			<0x489d0600 0x3c>,
> +			<0x489d0700 0x80>,

Is there other stuff between these regions?

> +			<0x489d5700 0x18>,
> +			<0x489dd000 0x400>;
> +		reg-names =	"vpe_top",
> +				"vpe_chr_us0",
> +				"vpe_chr_us1",
> +				"vpe_chr_us2",
> +				"vpe_dei",
> +				"sc",
> +				"csc",
> +				"vpdma";
> +		interrupts = <GIC_SPI 354 IRQ_TYPE_LEVEL_HIGH>;

> +		#address-cells = <1>;
> +		#size-cells = <0>;

These aren't needed.

> +	};
> -- 
> 2.9.0
> 

Re: [Patch 4/6] dt-bindings: media: ti-vpe: Document VPE driver

[Benoit Parrot]

Rob Herring <robh@kernel.org> wrote on Tue [2017-Oct-17 16:00:51 -0500]:
> On Thu, Oct 12, 2017 at 02:27:17PM -0500, Benoit Parrot wrote:
> > Device Tree bindings for the Video Processing Engine (VPE) driver.
> > 
> > Signed-off-by: Benoit Parrot <bparrot@ti.com>
> > ---
> >  Documentation/devicetree/bindings/media/ti-vpe.txt | 41 ++++++++++++++++++++++
> >  1 file changed, 41 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/media/ti-vpe.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/media/ti-vpe.txt b/Documentation/devicetree/bindings/media/ti-vpe.txt
> > new file mode 100644
> > index 000000000000..c2ef93d08417
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/ti-vpe.txt
> > @@ -0,0 +1,41 @@
> > +Texas Instruments DRA7x VIDEO PROCESSING ENGINE (VPE)
> > +------------------------------------------------------
> > +
> > +The Video Processing Engine (VPE) is a key component for image post
> > +processing applications. VPE consist of a single memory to memory
> > +path which can perform chroma up/down sampling, deinterlacing,
> > +scaling and color space conversion.
> > +
> > +Required properties:
> > +- compatible: must be "ti,vpe"
> 
> Needs SoC specific compatibles.

This particular I/P is present on all DRA7x family of devices
so would "ti,dra7-vpe" be acceptable?

> 
> > +- reg:	physical base address and length of the registers set for the 8
> > +	memory regions required;
> > +- reg-names: name associated with the memory regions described is <reg>;
> > +- interrupts: should contain IRQ line for VPE;
> > +
> > +Example:
> > +	vpe {
> > +		compatible = "ti,vpe";
> > +		ti,hwmods = "vpe";
> > +		clocks = <&dpll_core_h23x2_ck>;
> > +		clock-names = "fck";
> > +		reg =	<0x489d0000 0x120>,
> > +			<0x489d0300 0x20>,
> > +			<0x489d0400 0x20>,
> > +			<0x489d0500 0x20>,
> > +			<0x489d0600 0x3c>,
> > +			<0x489d0700 0x80>,
> 
> Is there other stuff between these regions?

No, they listed separately because each sub-region/module is
individually mapped and accessed using a starting 0 offset.

> 
> > +			<0x489d5700 0x18>,
> > +			<0x489dd000 0x400>;
> > +		reg-names =	"vpe_top",
> > +				"vpe_chr_us0",
> > +				"vpe_chr_us1",
> > +				"vpe_chr_us2",
> > +				"vpe_dei",
> > +				"sc",
> > +				"csc",
> > +				"vpdma";
> > +		interrupts = <GIC_SPI 354 IRQ_TYPE_LEVEL_HIGH>;
> 
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> 
> These aren't needed.

OK, I'll remove those.

Benoit

> 
> > +	};
> > -- 
> > 2.9.0
> > 

Re: [Patch 4/6] dt-bindings: media: ti-vpe: Document VPE driver

[Rob Herring]

On Wed, Oct 18, 2017 at 8:02 AM, Benoit Parrot <bparrot@ti.com> wrote:
> Rob Herring <robh@kernel.org> wrote on Tue [2017-Oct-17 16:00:51 -0500]:
>> On Thu, Oct 12, 2017 at 02:27:17PM -0500, Benoit Parrot wrote:
>> > Device Tree bindings for the Video Processing Engine (VPE) driver.
>> >
>> > Signed-off-by: Benoit Parrot <bparrot@ti.com>
>> > ---
>> >  Documentation/devicetree/bindings/media/ti-vpe.txt | 41 ++++++++++++++++++++++
>> >  1 file changed, 41 insertions(+)
>> >  create mode 100644 Documentation/devicetree/bindings/media/ti-vpe.txt
>> >
>> > diff --git a/Documentation/devicetree/bindings/media/ti-vpe.txt b/Documentation/devicetree/bindings/media/ti-vpe.txt
>> > new file mode 100644
>> > index 000000000000..c2ef93d08417
>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/media/ti-vpe.txt
>> > @@ -0,0 +1,41 @@
>> > +Texas Instruments DRA7x VIDEO PROCESSING ENGINE (VPE)
>> > +------------------------------------------------------
>> > +
>> > +The Video Processing Engine (VPE) is a key component for image post
>> > +processing applications. VPE consist of a single memory to memory
>> > +path which can perform chroma up/down sampling, deinterlacing,
>> > +scaling and color space conversion.
>> > +
>> > +Required properties:
>> > +- compatible: must be "ti,vpe"
>>
>> Needs SoC specific compatibles.
>
> This particular I/P is present on all DRA7x family of devices
> so would "ti,dra7-vpe" be acceptable?

Better, but it's proven not to be in other cases IIRC. I'll leave it
to TI maintainers to decide.

>> > +- reg:     physical base address and length of the registers set for the 8
>> > +   memory regions required;
>> > +- reg-names: name associated with the memory regions described is <reg>;
>> > +- interrupts: should contain IRQ line for VPE;
>> > +
>> > +Example:
>> > +   vpe {
>> > +           compatible = "ti,vpe";
>> > +           ti,hwmods = "vpe";
>> > +           clocks = <&dpll_core_h23x2_ck>;
>> > +           clock-names = "fck";
>> > +           reg =   <0x489d0000 0x120>,
>> > +                   <0x489d0300 0x20>,
>> > +                   <0x489d0400 0x20>,
>> > +                   <0x489d0500 0x20>,
>> > +                   <0x489d0600 0x3c>,
>> > +                   <0x489d0700 0x80>,
>>
>> Is there other stuff between these regions?
>
> No, they listed separately because each sub-region/module is
> individually mapped and accessed using a starting 0 offset.

So you are going to use 48KB of virtual memory to map 2KB of
registers? Because each ioremap uses 8KB (1 page plus 1 guard page)
last time i looked (which has been a while).

But it's your platform.

Re: [Patch 4/6] dt-bindings: media: ti-vpe: Document VPE driver

[Tony Lindgren]

* Rob Herring <robh@kernel.org> [171019 14:07]:
> On Wed, Oct 18, 2017 at 8:02 AM, Benoit Parrot <bparrot@ti.com> wrote:
> >> > +Example:
> >> > +   vpe {
> >> > +           compatible = "ti,vpe";
> >> > +           ti,hwmods = "vpe";
> >> > +           clocks = <&dpll_core_h23x2_ck>;
> >> > +           clock-names = "fck";
> >> > +           reg =   <0x489d0000 0x120>,
> >> > +                   <0x489d0300 0x20>,
> >> > +                   <0x489d0400 0x20>,
> >> > +                   <0x489d0500 0x20>,
> >> > +                   <0x489d0600 0x3c>,
> >> > +                   <0x489d0700 0x80>,
> >>
> >> Is there other stuff between these regions?
> >
> > No, they listed separately because each sub-region/module is
> > individually mapped and accessed using a starting 0 offset.
> 
> So you are going to use 48KB of virtual memory to map 2KB of
> registers? Because each ioremap uses 8KB (1 page plus 1 guard page)
> last time i looked (which has been a while).
> 
> But it's your platform.

We should have cached regions for all interconnects so this should
not be a problem. Worth checking that the areas are listed in
dra7xx_io_desc[] and for other SoCs too to avoid this issue.

Regards,

Tony

[Patch 5/6] ARM: DRA7: hwmod: Add VPE nodes

[Benoit Parrot]

Add hwmod entries for VPE (Video Processing Engine) hardware block
found in DRA7x family of devices.

DRA75x_DRA74x_ES1.1 Version T Technical Reference Manual
(Literature Number SPRUHI2T) states that VPE only support NO_STANDBY,
FORCE_STANDBY, NO_IDLE, FORCE_IDLE and SMART_IDLE.
The hwmod flags were set to reflect this fact and make sure that
both are software supervised (SWSUSP).

Signed-off-by: Benoit Parrot <bparrot@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod_7xx_data.c | 43 +++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
index fc53b498975c..c0bbc1099a11 100644
--- a/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_7xx_data.c
@@ -240,6 +240,40 @@ static struct omap_hwmod dra7xx_bb2d_hwmod = {
 };
 
 /*
+ * 'vpe' class
+ *
+ */
+
+static struct omap_hwmod_class_sysconfig dra7xx_vpe_sysc = {
+	.sysc_offs	= 0x0010,
+	.sysc_flags	= (SYSC_HAS_MIDLEMODE | SYSC_HAS_SIDLEMODE),
+	.idlemodes	= (SIDLE_FORCE | SIDLE_NO | SIDLE_SMART |
+			   MSTANDBY_FORCE | MSTANDBY_NO |
+			   MSTANDBY_SMART),
+	.sysc_fields	= &omap_hwmod_sysc_type2,
+};
+
+static struct omap_hwmod_class dra7xx_vpe_hwmod_class = {
+	.name	= "vpe",
+	.sysc	= &dra7xx_vpe_sysc,
+};
+
+/* vpe */
+static struct omap_hwmod dra7xx_vpe_hwmod = {
+	.name		= "vpe",
+	.class		= &dra7xx_vpe_hwmod_class,
+	.clkdm_name	= "vpe_clkdm",
+	.flags		= (HWMOD_SWSUP_SIDLE | HWMOD_SWSUP_MSTANDBY),
+	.prcm = {
+		.omap4 = {
+			.clkctrl_offs = DRA7XX_CM_VPE_VPE_CLKCTRL_OFFSET,
+			.context_offs = DRA7XX_RM_VPE_VPE_CONTEXT_OFFSET,
+			.modulemode   = MODULEMODE_HWCTRL,
+		},
+	},
+};
+
+/*
  * 'cal' class
  *
  */
@@ -3936,6 +3970,14 @@ static struct omap_hwmod_ocp_if dra7xx_l4_per2__vcp2 = {
 	.user		= OCP_USER_MPU | OCP_USER_SDMA,
 };
 
+/* l4_per3 -> vpe */
+static struct omap_hwmod_ocp_if dra7xx_l4_per3__vpe = {
+	.master		= &dra7xx_l4_per3_hwmod,
+	.slave		= &dra7xx_vpe_hwmod,
+	.clk		= "l3_iclk_div",
+	.user		= OCP_USER_MPU | OCP_USER_SDMA,
+};
+
 /* l4_per3 -> cal */
 static struct omap_hwmod_ocp_if dra7xx_l4_per3__cal = {
 	.master		= &dra7xx_l4_per3_hwmod,
@@ -4099,6 +4141,7 @@ static struct omap_hwmod_ocp_if *dra7xx_hwmod_ocp_ifs[] __initdata = {
 	&dra7xx_l4_per2__vcp1,
 	&dra7xx_l3_main_1__vcp2,
 	&dra7xx_l4_per2__vcp2,
+	&dra7xx_l4_per3__vpe,
 	&dra7xx_l4_wkup__wd_timer2,
 	&dra7xx_l4_per2__epwmss0,
 	&dra7xx_l4_per2__epwmss1,
-- 
2.9.0

Re: [Patch 5/6] ARM: DRA7: hwmod: Add VPE nodes

[Tony Lindgren]

* Benoit Parrot <bparrot@ti.com> [171012 12:28]:
> +static struct omap_hwmod_class_sysconfig dra7xx_vpe_sysc = {
> +	.sysc_offs	= 0x0010,
> +	.sysc_flags	= (SYSC_HAS_MIDLEMODE | SYSC_HAS_SIDLEMODE),
> +	.idlemodes	= (SIDLE_FORCE | SIDLE_NO | SIDLE_SMART |
> +			   MSTANDBY_FORCE | MSTANDBY_NO |
> +			   MSTANDBY_SMART),
> +	.sysc_fields	= &omap_hwmod_sysc_type2,
> +};

I think checkpatch.pl --strict would complain about unnecessary
parentheses, might as well check the whole series while at it.

Regards,

Tony

Re: [Patch 5/6] ARM: DRA7: hwmod: Add VPE nodes

[Benoit Parrot]

Tony Lindgren <tony@atomide.com> wrote on Fri [2017-Oct-13 10:05:13 -0700]:
> * Benoit Parrot <bparrot@ti.com> [171012 12:28]:
> > +static struct omap_hwmod_class_sysconfig dra7xx_vpe_sysc = {
> > +	.sysc_offs	= 0x0010,
> > +	.sysc_flags	= (SYSC_HAS_MIDLEMODE | SYSC_HAS_SIDLEMODE),
> > +	.idlemodes	= (SIDLE_FORCE | SIDLE_NO | SIDLE_SMART |
> > +			   MSTANDBY_FORCE | MSTANDBY_NO |
> > +			   MSTANDBY_SMART),
> > +	.sysc_fields	= &omap_hwmod_sysc_type2,
> > +};
> 
> I think checkpatch.pl --strict would complain about unnecessary
> parentheses, might as well check the whole series while at it.

I actually ran the whole series through "checkpatch.pl --strict"
before posting. And other then the usual MAINTAINER file needing
update warning for the binding patch it no other warning or error.

Based on the rest of the file I believe the parentheses around those
flags are at least consistent.

Now, would the .rev_offs comment also apply here?

Benoit

> 
> Regards,
> 
> Tony

Re: [Patch 5/6] ARM: DRA7: hwmod: Add VPE nodes

[Tony Lindgren]

* Benoit Parrot <bparrot@ti.com> [171013 11:06]:
> Tony Lindgren <tony@atomide.com> wrote on Fri [2017-Oct-13 10:05:13 -0700]:
> > * Benoit Parrot <bparrot@ti.com> [171012 12:28]:
> > > +static struct omap_hwmod_class_sysconfig dra7xx_vpe_sysc = {
> > > +	.sysc_offs	= 0x0010,
> > > +	.sysc_flags	= (SYSC_HAS_MIDLEMODE | SYSC_HAS_SIDLEMODE),
> > > +	.idlemodes	= (SIDLE_FORCE | SIDLE_NO | SIDLE_SMART |
> > > +			   MSTANDBY_FORCE | MSTANDBY_NO |
> > > +			   MSTANDBY_SMART),
> > > +	.sysc_fields	= &omap_hwmod_sysc_type2,
> > > +};
> > 
> > I think checkpatch.pl --strict would complain about unnecessary
> > parentheses, might as well check the whole series while at it.
> 
> I actually ran the whole series through "checkpatch.pl --strict"
> before posting. And other then the usual MAINTAINER file needing
> update warning for the binding patch it no other warning or error.
> 
> Based on the rest of the file I believe the parentheses around those
> flags are at least consistent.

OK fine thanks for checking.

> Now, would the .rev_offs comment also apply here?

I don't think it has it, you might want to dump out the value
at offset 0 and see if it contains anything.

Regards,

Tony

[Patch 6/6] ARM: dts: dra7: Add VPE dtsi node

[Benoit Parrot]

Add the necessary node and configuration data for the VPE (Video
Processing Engine) hardware block on DRA7x.

The corresponding driver for this entry is in
drivers/media/platform/ti-vpe/vpe.c.

Signed-off-by: Benoit Parrot <bparrot@ti.com>
---
 arch/arm/boot/dts/dra7.dtsi | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi
index 02a136a4661a..f302d2e5eede 100644
--- a/arch/arm/boot/dts/dra7.dtsi
+++ b/arch/arm/boot/dts/dra7.dtsi
@@ -2018,6 +2018,32 @@
 			clocks = <&l3_iclk_div>;
 			clock-names = "fck";
 		};
+
+		vpe {
+			compatible = "ti,vpe";
+			ti,hwmods = "vpe";
+			clocks = <&dpll_core_h23x2_ck>;
+			clock-names = "fck";
+			reg = <0x489d0000 0x120>,
+			      <0x489d0300 0x20>,
+			      <0x489d0400 0x20>,
+			      <0x489d0500 0x20>,
+			      <0x489d0600 0x3c>,
+			      <0x489d0700 0x80>,
+			      <0x489d5700 0x18>,
+			      <0x489dd000 0x400>;
+			reg-names = "vpe_top",
+				    "vpe_chr_us0",
+				    "vpe_chr_us1",
+				    "vpe_chr_us2",
+				    "vpe_dei",
+				    "sc",
+				    "csc",
+				    "vpdma";
+			interrupts = <GIC_SPI 354 IRQ_TYPE_LEVEL_HIGH>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+		};
 	};
 
 	thermal_zones: thermal-zones {
-- 
2.9.0