[PATCH 0/4] ARM: davinci: add/enable video capture for da850-lcdk

[PATCH 0/4] ARM: davinci: add/enable video capture for da850-lcdk

[Kevin Hilman]

Add DT description of da850 video capture (using VPIF) for the da850
SoC.  and the LCDK board.  This enables video capture using the
composite input on the LCDK board.

Series applies to Sekhar's v4.10/dt branch (and defconfig change will
have trivial add/add conflict when applied to Sekhar's defconfig branch.)

@Sekhar: I'm not sure if you want the first patch to go via your soc
branch or dt branch, but I kept it together with this series for now.
Feel free to break it up as needed.  It will also have a trivial
add/add conflict with some existing changes to that file.

Depenencies:
- [PATCH v3 0/4] [media] davinci: VPIF: add DT support
  https://marc.info/?l=devicetree&m=147982997317376&w=2

In video capture to work, this series depends on the above VPIF driver
update to enable DT support, however there are no build or boot-time
regressions if this series goes in independently of the driver
changes.  There will simply be no video capture support until the
driver changes are done (which the same as how things work today.)

Kevin Hilman (4):
  ARM: davinci: da8xx: VPIF: enable DT init
  ARM: dts: davinci: da850: add VPIF
  ARM: dts: davinci: da850-lcdk: enable VPIF capture via TVP5147
  ARM: davinci_all_defconfig: enable video capture as modules

 arch/arm/boot/dts/da850-lcdk.dts       | 30 ++++++++++++++++++++++++++++++
 arch/arm/boot/dts/da850.dtsi           | 28 ++++++++++++++++++++++++++++
 arch/arm/configs/davinci_all_defconfig |  6 ++++++
 arch/arm/mach-davinci/da8xx-dt.c       | 17 +++++++++++++++++
 4 files changed, 81 insertions(+)

-- 
2.9.3

[PATCH 1/4] ARM: davinci: da8xx: VPIF: enable DT init

[Kevin Hilman]

Add basic support for DT initializaion of VPIF (capture) via DT.  Clocks
and mux still need to happen in this file until there are real clock and
pinctrl drivers, but the video nodes and subdevs can all come from DT.

Signed-off-by: Kevin Hilman <khilman@baylibre.com>
---
 arch/arm/mach-davinci/da8xx-dt.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/arch/arm/mach-davinci/da8xx-dt.c b/arch/arm/mach-davinci/da8xx-dt.c
index c9f7e9274aa8..7b41611f2665 100644
--- a/arch/arm/mach-davinci/da8xx-dt.c
+++ b/arch/arm/mach-davinci/da8xx-dt.c
@@ -17,6 +17,7 @@
 #include <mach/common.h>
 #include "cp_intc.h"
 #include <mach/da8xx.h>
+#include <mach/mux.h>
 
 static struct of_dev_auxdata da850_auxdata_lookup[] __initdata = {
 	OF_DEV_AUXDATA("ti,davinci-i2c", 0x01c22000, "i2c_davinci.1", NULL),
@@ -38,14 +39,30 @@ static struct of_dev_auxdata da850_auxdata_lookup[] __initdata = {
 		       NULL),
 	OF_DEV_AUXDATA("ti,da830-mcasp-audio", 0x01d00000, "davinci-mcasp.0", NULL),
 	OF_DEV_AUXDATA("ti,da850-aemif", 0x68000000, "ti-aemif", NULL),
+	OF_DEV_AUXDATA("ti,da850-vpif", 0x01e17000, "vpif", NULL),
 	{}
 };
 
 #ifdef CONFIG_ARCH_DAVINCI_DA850
 
+#if IS_ENABLED(CONFIG_VIDEO_DAVINCI_VPIF_CAPTURE)
+static __init void da850_vpif_capture_init(void)
+{
+	int ret;
+
+	ret = davinci_cfg_reg_list(da850_vpif_capture_pins);
+	if (ret)
+		pr_warn("da850_evm_init: VPIF capture mux setup failed: %d\n",
+			ret);
+}
+#else
+#define da850_vpif_capture_init()
+#endif
+
 static void __init da850_init_machine(void)
 {
 	of_platform_default_populate(NULL, da850_auxdata_lookup, NULL);
+	da850_vpif_capture_init();
 }
 
 static const char *const da850_boards_compat[] __initconst = {
-- 
2.9.3

[PATCH 2/4] ARM: dts: davinci: da850: add VPIF

[Kevin Hilman]

Add VPIF and VPIF capture nodes to da850.  VPIF capture has two input
channels describe using the standard DT ports and enpoints.

Signed-off-by: Kevin Hilman <khilman@baylibre.com>
---
 arch/arm/boot/dts/da850.dtsi | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
index 6205917b4f59..e05e2bb834e8 100644
--- a/arch/arm/boot/dts/da850.dtsi
+++ b/arch/arm/boot/dts/da850.dtsi
@@ -453,7 +453,35 @@
 			interrupts = <52>;
 			status = "disabled";
 		};
+
+		vpif: video at 0x00217000 {
+			compatible = "ti,da850-vpif";
+			reg = <0x00217000 0x1000>;
+			status = "disabled";
+		};
+
+		vpif_capture: video-capture at 0x00217000 {
+			compatible = "ti,da850-vpif-capture";
+			reg = <0x00217000 0x1000>;
+			interrupts = <92>;
+			status = "disabled";
+
+			/* VPIF capture: input channels */
+			port {
+				vpif_ch0: endpoint at 0 {
+					  reg = <0>;
+					  bus-width = <8>;
+				};
+
+				vpif_ch1: endpoint at 1 {
+					  reg = <1>;
+					  bus-width = <8>;
+					  data-shift = <8>;
+				};
+			};
+		};
 	};
+
 	aemif: aemif at 68000000 {
 		compatible = "ti,da850-aemif";
 		#address-cells = <2>;
-- 
2.9.3

[PATCH 3/4] ARM: dts: davinci: da850-lcdk: enable VPIF capture via TVP5147

[Kevin Hilman]

Enable video capture via the on-board TVP5147 decoder hooked up to ch0
one of the VPIF capture input.

Signed-off-by: Kevin Hilman <khilman@baylibre.com>
---
 arch/arm/boot/dts/da850-lcdk.dts | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/arch/arm/boot/dts/da850-lcdk.dts b/arch/arm/boot/dts/da850-lcdk.dts
index 03f9bfda5385..39db2ffa1902 100644
--- a/arch/arm/boot/dts/da850-lcdk.dts
+++ b/arch/arm/boot/dts/da850-lcdk.dts
@@ -138,6 +138,24 @@
 		reg = <0x18>;
 		status = "okay";
 	};
+
+	tvp5147 at 5d {
+		compatible = "ti,tvp5147";
+		reg = <0x5d>;
+		status = "okay";
+
+		port {
+			composite: endpoint {
+				hsync-active = <1>;
+				vsync-active = <1>;
+				pclk-sample = <0>;
+
+				/* VPIF channel 0 (lower 8-bits) */
+				remote-endpoint = <&vpif_ch0>;
+				bus-width = <8>;
+			};
+		};
+	};
 };
 
 &mcasp0 {
@@ -227,3 +245,15 @@
 		};
 	};
 };
+
+&vpif {
+	status = "okay";
+};
+
+&vpif_capture {
+	status = "okay";
+};
+
+&vpif_ch0 {
+	remote-endpoint = <&composite>;
+};
-- 
2.9.3

[PATCH 4/4] ARM: davinci_all_defconfig: enable video capture as modules

[Kevin Hilman]

Enable media support and V4L2 capture, along with video decoders used
on da850 platforms.

Tested on da850-lcdk.

Signed-off-by: Kevin Hilman <khilman@baylibre.com>
---
 arch/arm/configs/davinci_all_defconfig | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/configs/davinci_all_defconfig b/arch/arm/configs/davinci_all_defconfig
index 5e5dd6bc5ed9..fd05bc35fb36 100644
--- a/arch/arm/configs/davinci_all_defconfig
+++ b/arch/arm/configs/davinci_all_defconfig
@@ -123,6 +123,12 @@ CONFIG_TPS6507X=y
 CONFIG_REGULATOR=y
 CONFIG_REGULATOR_FIXED_VOLTAGE=y
 CONFIG_REGULATOR_TPS6507X=y
+CONFIG_MEDIA_SUPPORT=m
+CONFIG_MEDIA_CAMERA_SUPPORT=y
+CONFIG_V4L_PLATFORM_DRIVERS=y
+CONFIG_VIDEO_DAVINCI_VPIF_CAPTURE=m
+# CONFIG_MEDIA_SUBDRV_AUTOSELECT is not set
+CONFIG_VIDEO_TVP514X=m
 CONFIG_FB=y
 CONFIG_FIRMWARE_EDID=y
 CONFIG_FB_DA8XX=y
-- 
2.9.3

[PATCH 2/4] ARM: dts: davinci: da850: add VPIF

[David Lechner]

On 11/22/2016 01:45 PM, Kevin Hilman wrote:
> Add VPIF and VPIF capture nodes to da850.  VPIF capture has two input
> channels describe using the standard DT ports and enpoints.
>
> Signed-off-by: Kevin Hilman <khilman@baylibre.com>
> ---
>  arch/arm/boot/dts/da850.dtsi | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
>
> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
> index 6205917b4f59..e05e2bb834e8 100644
> --- a/arch/arm/boot/dts/da850.dtsi
> +++ b/arch/arm/boot/dts/da850.dtsi
> @@ -453,7 +453,35 @@
>  			interrupts = <52>;
>  			status = "disabled";
>  		};
> +
> +		vpif: video at 0x00217000 {

Should be @217000

> +			compatible = "ti,da850-vpif";
> +			reg = <0x00217000 0x1000>;

Could omit leading 0's to be consistent with existing entries.

	reg = <0x217000 0x1000>;

> +			status = "disabled";
> +		};
> +
> +		vpif_capture: video-capture at 0x00217000 {

Again, @217000. But it seems odd to have two device nodes with the same 
address. Is enabling these mutually exclusive?


> +			compatible = "ti,da850-vpif-capture";
> +			reg = <0x00217000 0x1000>;

Ditto on the leading 0's.

> +			interrupts = <92>;
> +			status = "disabled";
> +
> +			/* VPIF capture: input channels */
> +			port {
> +				vpif_ch0: endpoint at 0 {
> +					  reg = <0>;
> +					  bus-width = <8>;
> +				};
> +
> +				vpif_ch1: endpoint at 1 {
> +					  reg = <1>;
> +					  bus-width = <8>;
> +					  data-shift = <8>;
> +				};
> +			};
> +		};
>  	};
> +
>  	aemif: aemif at 68000000 {
>  		compatible = "ti,da850-aemif";
>  		#address-cells = <2>;
>

[PATCH 1/4] ARM: davinci: da8xx: VPIF: enable DT init

[David Lechner]

On 11/22/2016 01:45 PM, Kevin Hilman wrote:
> Add basic support for DT initializaion of VPIF (capture) via DT.  Clocks
> and mux still need to happen in this file until there are real clock and
> pinctrl drivers, but the video nodes and subdevs can all come from DT.
>
> Signed-off-by: Kevin Hilman <khilman@baylibre.com>
> ---
>  arch/arm/mach-davinci/da8xx-dt.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
>
> diff --git a/arch/arm/mach-davinci/da8xx-dt.c b/arch/arm/mach-davinci/da8xx-dt.c
> index c9f7e9274aa8..7b41611f2665 100644
> --- a/arch/arm/mach-davinci/da8xx-dt.c
> +++ b/arch/arm/mach-davinci/da8xx-dt.c
> @@ -17,6 +17,7 @@
>  #include <mach/common.h>
>  #include "cp_intc.h"
>  #include <mach/da8xx.h>
> +#include <mach/mux.h>
>
>  static struct of_dev_auxdata da850_auxdata_lookup[] __initdata = {
>  	OF_DEV_AUXDATA("ti,davinci-i2c", 0x01c22000, "i2c_davinci.1", NULL),
> @@ -38,14 +39,30 @@ static struct of_dev_auxdata da850_auxdata_lookup[] __initdata = {
>  		       NULL),
>  	OF_DEV_AUXDATA("ti,da830-mcasp-audio", 0x01d00000, "davinci-mcasp.0", NULL),
>  	OF_DEV_AUXDATA("ti,da850-aemif", 0x68000000, "ti-aemif", NULL),
> +	OF_DEV_AUXDATA("ti,da850-vpif", 0x01e17000, "vpif", NULL),
>  	{}
>  };
>
>  #ifdef CONFIG_ARCH_DAVINCI_DA850
>
> +#if IS_ENABLED(CONFIG_VIDEO_DAVINCI_VPIF_CAPTURE)
> +static __init void da850_vpif_capture_init(void)
> +{
> +	int ret;
> +
> +	ret = davinci_cfg_reg_list(da850_vpif_capture_pins);

Why can't we use the existing pinctrl-single node in device tree for 
muxing the pins?

> +	if (ret)
> +		pr_warn("da850_evm_init: VPIF capture mux setup failed: %d\n",
> +			ret);
> +}
> +#else
> +#define da850_vpif_capture_init()
> +#endif
> +
>  static void __init da850_init_machine(void)
>  {
>  	of_platform_default_populate(NULL, da850_auxdata_lookup, NULL);
> +	da850_vpif_capture_init();
>  }
>
>  static const char *const da850_boards_compat[] __initconst = {
>

[PATCH 1/4] ARM: davinci: da8xx: VPIF: enable DT init

[Kevin Hilman]

David Lechner <david@lechnology.com> writes:

> On 11/22/2016 01:45 PM, Kevin Hilman wrote:
>> Add basic support for DT initializaion of VPIF (capture) via DT.  Clocks
>> and mux still need to happen in this file until there are real clock and
>> pinctrl drivers, but the video nodes and subdevs can all come from DT.
>>
>> Signed-off-by: Kevin Hilman <khilman@baylibre.com>
>> ---
>>  arch/arm/mach-davinci/da8xx-dt.c | 17 +++++++++++++++++
>>  1 file changed, 17 insertions(+)
>>
>> diff --git a/arch/arm/mach-davinci/da8xx-dt.c b/arch/arm/mach-davinci/da8xx-dt.c
>> index c9f7e9274aa8..7b41611f2665 100644
>> --- a/arch/arm/mach-davinci/da8xx-dt.c
>> +++ b/arch/arm/mach-davinci/da8xx-dt.c
>> @@ -17,6 +17,7 @@
>>  #include <mach/common.h>
>>  #include "cp_intc.h"
>>  #include <mach/da8xx.h>
>> +#include <mach/mux.h>
>>
>>  static struct of_dev_auxdata da850_auxdata_lookup[] __initdata = {
>>  	OF_DEV_AUXDATA("ti,davinci-i2c", 0x01c22000, "i2c_davinci.1", NULL),
>> @@ -38,14 +39,30 @@ static struct of_dev_auxdata da850_auxdata_lookup[] __initdata = {
>>  		       NULL),
>>  	OF_DEV_AUXDATA("ti,da830-mcasp-audio", 0x01d00000, "davinci-mcasp.0", NULL),
>>  	OF_DEV_AUXDATA("ti,da850-aemif", 0x68000000, "ti-aemif", NULL),
>> +	OF_DEV_AUXDATA("ti,da850-vpif", 0x01e17000, "vpif", NULL),
>>  	{}
>>  };
>>
>>  #ifdef CONFIG_ARCH_DAVINCI_DA850
>>
>> +#if IS_ENABLED(CONFIG_VIDEO_DAVINCI_VPIF_CAPTURE)
>> +static __init void da850_vpif_capture_init(void)
>> +{
>> +	int ret;
>> +
>> +	ret = davinci_cfg_reg_list(da850_vpif_capture_pins);
>
> Why can't we use the existing pinctrl-single node in device tree for
> muxing the pins?

Oops, you're right.  They can.

Kevin

[PATCH 2/4] ARM: dts: davinci: da850: add VPIF

[Kevin Hilman]

David Lechner <david@lechnology.com> writes:

> On 11/22/2016 01:45 PM, Kevin Hilman wrote:
>> Add VPIF and VPIF capture nodes to da850.  VPIF capture has two input
>> channels describe using the standard DT ports and enpoints.
>>
>> Signed-off-by: Kevin Hilman <khilman@baylibre.com>
>> ---
>>  arch/arm/boot/dts/da850.dtsi | 28 ++++++++++++++++++++++++++++
>>  1 file changed, 28 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
>> index 6205917b4f59..e05e2bb834e8 100644
>> --- a/arch/arm/boot/dts/da850.dtsi
>> +++ b/arch/arm/boot/dts/da850.dtsi
>> @@ -453,7 +453,35 @@
>>  			interrupts = <52>;
>>  			status = "disabled";
>>  		};
>> +
>> +		vpif: video at 0x00217000 {
>
> Should be @217000
>
>> +			compatible = "ti,da850-vpif";
>> +			reg = <0x00217000 0x1000>;
>
> Could omit leading 0's to be consistent with existing entries.
>
> 	reg = <0x217000 0x1000>;

Ugh, yeah. I hate that convention, but better to be consistent, I guess.

>> +			status = "disabled";
>> +		};
>> +
>> +		vpif_capture: video-capture at 0x00217000 {
>
> Again, @217000. But it seems odd to have two device nodes with the
> same address. Is enabling these mutually exclusive?

They're not mutually exclusive because the vpif is the one that actually
maps the register range (since it's shared between vpif_display and
vpif_capture) so I guess I should just drop the reg property from the
vpif_capture node.

>> +			compatible = "ti,da850-vpif-capture";
>> +			reg = <0x00217000 0x1000>;
>
> Ditto on the leading 0's.
>

Thanks for the review,

Kevin

[PATCH 2/4] ARM: dts: davinci: da850: add VPIF

[Sekhar Nori]

On Wednesday 23 November 2016 11:13 AM, Kevin Hilman wrote:
> David Lechner <david@lechnology.com> writes:
> 
>> On 11/22/2016 01:45 PM, Kevin Hilman wrote:
>>> Add VPIF and VPIF capture nodes to da850.  VPIF capture has two input
>>> channels describe using the standard DT ports and enpoints.
>>>
>>> Signed-off-by: Kevin Hilman <khilman@baylibre.com>
>>> ---
>>>  arch/arm/boot/dts/da850.dtsi | 28 ++++++++++++++++++++++++++++
>>>  1 file changed, 28 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
>>> index 6205917b4f59..e05e2bb834e8 100644
>>> --- a/arch/arm/boot/dts/da850.dtsi
>>> +++ b/arch/arm/boot/dts/da850.dtsi
>>> @@ -453,7 +453,35 @@
>>>  			interrupts = <52>;
>>>  			status = "disabled";
>>>  		};
>>> +
>>> +		vpif: video at 0x00217000 {
>>
>> Should be @217000
>>
>>> +			compatible = "ti,da850-vpif";
>>> +			reg = <0x00217000 0x1000>;
>>
>> Could omit leading 0's to be consistent with existing entries.
>>
>> 	reg = <0x217000 0x1000>;
> 
> Ugh, yeah. I hate that convention, but better to be consistent, I guess.
> 
>>> +			status = "disabled";
>>> +		};
>>> +
>>> +		vpif_capture: video-capture at 0x00217000 {
>>
>> Again, @217000. But it seems odd to have two device nodes with the
>> same address. Is enabling these mutually exclusive?
> 
> They're not mutually exclusive because the vpif is the one that actually
> maps the register range (since it's shared between vpif_display and
> vpif_capture) so I guess I should just drop the reg property from the
> vpif_capture node.

Reading the documentation, VPIF is presented as a single device, with
two channels dedicated to display and two for capture. Most of the
channel registers are independent, but there are are some like interrupt
enable which are common for all channels. So I believe we cannot use
simple-mfd. But I believe VPIF display and capture should be subdevices
of a single VPIF device.

It should look something like this, I think:

		vpif: video at 217000 {
			compatible = "ti,da850-vpif";
			reg = <0x217000 0x1000>;
			interrupts = <92>;
			status = "disabled";

			vpif_capture: video-capture {
				compatible = "ti,da850-vpif-capture"
				port {
					vpif_ch0: endpoint at 0 {
						  reg = <0>;
						  bus-width = <8>;
					};
	
					vpif_ch1: endpoint at 1 {
						  reg = <1>;
						  bus-width = <8>;
						  data-shift = <8>;
					};
				};
			};

			vpif_display: video-display {
				compatible = "ti,da850-vpif-display"
				port {
					vpif_ch2: endpoint at 2 {
						  reg = <2>;
						  bus-width = <8>;
						  data-shift = <16>;
					};

					vpif_ch3: endpoint at 3 {
						  reg = <3>;
						  bus-width = <8>;
						  data-shift = <24>;
					};
				};
			};
		};

The interrupt too, seems to be common interrupt for both display and
capture. So, it should not be under the capture node. BTW, I am sure
what exactly data-shift is used for. It does not seem to be used in the
driver patches too. I just extrapolated the values based on the pattern
I saw.

Thanks,
Sekhar

[PATCH 2/4] ARM: dts: davinci: da850: add VPIF

[Kevin Hilman]

Sekhar Nori <nsekhar@ti.com> writes:

> On Wednesday 23 November 2016 11:13 AM, Kevin Hilman wrote:
>> David Lechner <david@lechnology.com> writes:
>> 
>>> On 11/22/2016 01:45 PM, Kevin Hilman wrote:
>>>> Add VPIF and VPIF capture nodes to da850.  VPIF capture has two input
>>>> channels describe using the standard DT ports and enpoints.
>>>>
>>>> Signed-off-by: Kevin Hilman <khilman@baylibre.com>
>>>> ---
>>>>  arch/arm/boot/dts/da850.dtsi | 28 ++++++++++++++++++++++++++++
>>>>  1 file changed, 28 insertions(+)
>>>>
>>>> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
>>>> index 6205917b4f59..e05e2bb834e8 100644
>>>> --- a/arch/arm/boot/dts/da850.dtsi
>>>> +++ b/arch/arm/boot/dts/da850.dtsi
>>>> @@ -453,7 +453,35 @@
>>>>  			interrupts = <52>;
>>>>  			status = "disabled";
>>>>  		};
>>>> +
>>>> +		vpif: video at 0x00217000 {
>>>
>>> Should be @217000
>>>
>>>> +			compatible = "ti,da850-vpif";
>>>> +			reg = <0x00217000 0x1000>;
>>>
>>> Could omit leading 0's to be consistent with existing entries.
>>>
>>> 	reg = <0x217000 0x1000>;
>> 
>> Ugh, yeah. I hate that convention, but better to be consistent, I guess.
>> 
>>>> +			status = "disabled";
>>>> +		};
>>>> +
>>>> +		vpif_capture: video-capture at 0x00217000 {
>>>
>>> Again, @217000. But it seems odd to have two device nodes with the
>>> same address. Is enabling these mutually exclusive?
>> 
>> They're not mutually exclusive because the vpif is the one that actually
>> maps the register range (since it's shared between vpif_display and
>> vpif_capture) so I guess I should just drop the reg property from the
>> vpif_capture node.
>
> Reading the documentation, VPIF is presented as a single device, with
> two channels dedicated to display and two for capture. Most of the
> channel registers are independent, but there are are some like interrupt
> enable which are common for all channels. So I believe we cannot use
> simple-mfd. But I believe VPIF display and capture should be subdevices
> of a single VPIF device.

OK, I can give it a try that way.

> It should look something like this, I think:
>
> 		vpif: video at 217000 {
> 			compatible = "ti,da850-vpif";
> 			reg = <0x217000 0x1000>;
> 			interrupts = <92>;
> 			status = "disabled";
>
> 			vpif_capture: video-capture {
> 				compatible = "ti,da850-vpif-capture"
> 				port {
> 					vpif_ch0: endpoint at 0 {
> 						  reg = <0>;
> 						  bus-width = <8>;
> 					};
> 	
> 					vpif_ch1: endpoint at 1 {
> 						  reg = <1>;
> 						  bus-width = <8>;
> 						  data-shift = <8>;
> 					};
> 				};
> 			};
>
> 			vpif_display: video-display {
> 				compatible = "ti,da850-vpif-display"
> 				port {
> 					vpif_ch2: endpoint at 2 {
> 						  reg = <2>;
> 						  bus-width = <8>;
> 						  data-shift = <16>;
> 					};
>
> 					vpif_ch3: endpoint at 3 {
> 						  reg = <3>;
> 						  bus-width = <8>;
> 						  data-shift = <24>;
> 					};
> 				};
> 			};
> 		};

> The interrupt too, seems to be common interrupt for both display and
> capture. So, it should not be under the capture node.

Possibly.  That will require reworking of the way the driver works
today, since the vpif driver doesn't request interrupts, but the capture
and display drivers both register interrupts, one per channel.  I'll
have a closer look.

> BTW, I am sure
> what exactly data-shift is used for. It does not seem to be used in the
> driver patches too. I just extrapolated the values based on the pattern
> I saw.

For video out, the way I read the spec, and based on the schematics,
there appears to be a separate 16-bit parallel bus, so the data-shift on
the video-display endpoints should probably be zero and 16 like for
catpure.  Anyways, I'll get to that when I get to the display part.

Kevin