[PATCH v2 0/4] vin-tests: Add D3 Draak support

[PATCH v2 0/4] vin-tests: Add D3 Draak support

[Jacopo Mondi]

Hi Niklas,
   with these patches I can use yavta-hdmi script on Draak D3 board.

Compared to v1 I have rebased on top of your latest changes, with the V3M
integration that greatly simplified my life.

There are a few hack though [3/4] in particular, so I let you judge if you
want to include these changes or not.

For the record, the capture procedure on D3 is:

[root@alarm vin-tests]# ./set-edid
EDID set successfully.

[root@alarm vin-tests]# ./yavta-hdmi 4
WARING: overriding parallel format to: RGB888_1X24
format: RGB888_1X24 size: 640x480/640x480 field: none/none vdev: /dev/video3
Device /dev/video3 opened.
Device `R_Car_VIN' on `platform:e6ef4000.video' (driver 'rcar_vin') supports video, capture, without mplanes.
Video format set: XBGR32 (34325258) 640x480 (stride 2560) field none buffer size 1228800
Video format: XBGR32 (34325258) 640x480 (stride 2560) field none buffer size 1228800
Device /dev/video3 opened.
Device `R_Car_VIN' on `platform:e6ef4000.video' (driver 'rcar_vin') supports video, capture, without mplanes.
Video format: XBGR32 (34325258) 640x480 (stride 2560) field none buffer size 1228800
4 buffers requested.
length: 1228800 offset: 0 timestamp type/source: mono/EoF
Buffer 0/0 mapped at address 0xffffa3034000.
length: 1228800 offset: 1228800 timestamp type/source: mono/EoF
Buffer 1/0 mapped at address 0xffffa2f08000.
length: 1228800 offset: 2457600 timestamp type/source: mono/EoF
Buffer 2/0 mapped at address 0xffffa2ddc000.
length: 1228800 offset: 3686400 timestamp type/source: mono/EoF
Buffer 3/0 mapped at address 0xffffa2cb0000.
0 (1) [-] none 0 1228800 B 6196.484974 6196.485053 22.681 fps ts mono/EoF
1 (2) [-] none 1 1228800 B 6196.501657 6196.508303 59.941 fps ts mono/EoF
2 (0) [-] none 2 1228800 B 6196.518341 6196.531824 59.938 fps ts mono/EoF
3 (3) [-] none 3 1228800 B 6196.535027 6196.555485 59.930 fps ts mono/EoF
4 (1) [-] none 5 1228800 B 6196.568386 6196.579076 29.977 fps ts mono/EoF
5 (2) [-] none 6 1228800 B 6196.585073 6196.602437 59.927 fps ts mono/EoF
6 (0) [-] none 8 1228800 B 6196.618447 6196.625953 29.963 fps ts mono/EoF
7 (3) [-] none 9 1228800 B 6196.635130 6196.649540 59.941 fps ts mono/EoF
8 (1) [-] none 11 1228800 B 6196.668488 6196.673171 29.978 fps ts mono/EoF
9 (2) [-] none 12 1228800 B 6196.685175 6196.696770 59.927 fps ts mono/EoF
Captured 10 frames in 0.255885 seconds (39.080054 fps, 48021569.750941 B/s).

And I have converted images with:
$for i in `ls frame-*.bin`; do raw2rgbpnm -f BGR32 -s640x480 $i /tmp/$i.pnm; done

I am not sure about the formats I have used, but they give me the best results,
even if the images are flickered and not completely centered :/

Thanks
  j

Jacopo Mondi (4):
  vin-tests: Add support for D3 Draak
  vin-tests: Fix vdev name in propagate_parallel
  vin-tests: Add capture format for parallel input
  vin-tests: yavta-hdmi: Add VIN4 and parallel link

 scripts/boards.sh    | 11 +++++++++++
 scripts/vin-tests.sh |  3 ++-
 yavta-hdmi           | 13 +++++++++++--
 3 files changed, 24 insertions(+), 3 deletions(-)

--
2.7.4

[PATCH v2 1/4] vin-tests: Add support for D3 Draak

[Jacopo Mondi]

Add support for D3 Draak board.

Draak has its HDMI input connected to an ADV7612 which connects to VIN4
parallel data inputs.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 scripts/boards.sh | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/scripts/boards.sh b/scripts/boards.sh
index 26fdab9..7eb3a27 100644
--- a/scripts/boards.sh
+++ b/scripts/boards.sh
@@ -29,6 +29,13 @@ case $info in
         # for V3M, but results in an image.
         parallelformat="YUYV8_1X16"
         ;;
+    "Renesas Draak board based on r8a77995")
+        gen="gen3"
+        vins="4"
+        parallelname="adv7612 0-004c"
+        # FIXME: This is a hackfor D3, but results in an image.
+        parallelformat="YUYV8_1X16"
+        ;;
     "Koelsch")
         gen="gen2"
 
@@ -70,6 +77,8 @@ if [[ "$gen" == "gen3" ]]; then
 
         txaname="adv748x 0-0070 txa"
         txbname="adv748x 0-0070 txb"
+    elif [[ "$info" == "Renesas Draak board based on r8a77995" ]]; then
+        hdminame="adv7612 0-004c"
     else
         cvbsname="adv748x 4-0070 afe"
         hdminame="adv748x 4-0070 hdmi"
-- 
2.7.4

[PATCH v2 2/4] vin-tests: Fix vdev name in propagate_parallel

[Jacopo Mondi]

The $vdev variable was not defined in propagate_parallel() function.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 scripts/vin-tests.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/scripts/vin-tests.sh b/scripts/vin-tests.sh
index ebb1477..0c5b29a 100644
--- a/scripts/vin-tests.sh
+++ b/scripts/vin-tests.sh
@@ -83,6 +83,7 @@ mc_propagate_cvbs() {
 }
 
 mc_propagate_parallel() {
+    vin="$1"
     mdev=$(mc_get_mdev)
 
     cam="'$parallelname':1"
-- 
2.7.4

[PATCH v2 3/4] vin-tests: Add capture format for parallel input

[Jacopo Mondi]

Add configurable capture format to propagate_parallel() function.
The capture format is the image format set on the VIN nodes.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 scripts/boards.sh    | 2 ++
 scripts/vin-tests.sh | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/scripts/boards.sh b/scripts/boards.sh
index 7eb3a27..b379af6 100644
--- a/scripts/boards.sh
+++ b/scripts/boards.sh
@@ -28,6 +28,7 @@ case $info in
         # FIXME: This is a hack and not the correct mbus format
         # for V3M, but results in an image.
         parallelformat="YUYV8_1X16"
+	parallel_captureformat="RGB565"
         ;;
     "Renesas Draak board based on r8a77995")
         gen="gen3"
@@ -35,6 +36,7 @@ case $info in
         parallelname="adv7612 0-004c"
         # FIXME: This is a hackfor D3, but results in an image.
         parallelformat="YUYV8_1X16"
+	parallel_captureformat="RGB565"
         ;;
     "Koelsch")
         gen="gen2"
diff --git a/scripts/vin-tests.sh b/scripts/vin-tests.sh
index 0c5b29a..e7b7a48 100644
--- a/scripts/vin-tests.sh
+++ b/scripts/vin-tests.sh
@@ -111,5 +111,5 @@ mc_propagate_parallel() {
     echo "format: $format size: $size/$vinsize field: $field/$vinfield vdev: $vdev"
 
     $mediactl -d $mdev -V "$cam [fmt:$format/$size field:$field]"
-    yavta -f RGB565 -s $vinsize --field $vinfield $vdev
+    yavta -f $parallel_captureformat -s $vinsize --field $vinfield $vdev
 }
-- 
2.7.4

[PATCH v2 4/4] vin-tests: yavta-hdmi: Add VIN4 and parallel link

[Jacopo Mondi]

Add support for VIN4 to yavta-hdmi and check if format propagation should
go through 'mc_propagate_parallel()' if the HDMI receiver chip is an
ADV7612 one.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 yavta-hdmi | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/yavta-hdmi b/yavta-hdmi
index fdec546..2e3b625 100755
--- a/yavta-hdmi
+++ b/yavta-hdmi
@@ -33,14 +33,23 @@ case $vc in
         dev=/dev/$vin3
         csipad=4
         ;;
+    4)
+        vinname=$vinname4
+        dev=/dev/$vin4
+        ;;
     *)
         echo "Unkown VC '$vc'"
         exit 1
 esac
 
 mc_reset
-mc_set_link "$csi40name" $csipad "$vinname" 1
-mc_propagate_format "$hdminame" 1 "$txaname" 0 "$csi40name" $csipad "$vinname"
+if [[ "$hdminame" == "adv7612 0-004c" ]]; then
+	mc_set_link "$hdminame" 1  "$vinname" 1
+	mc_propagate_parallel "$vinname"
+else
+	mc_set_link "$csi40name" $csipad "$vinname" 1
+	mc_propagate_format "$hdminame" 1 "$txaname" 0 "$csi40name" $csipad "$vinname"
+fi
 
 out=/tmp/vin-tests
 rm -fr $out
-- 
2.7.4

Re: [PATCH v2 1/4] vin-tests: Add support for D3 Draak

[Niklas Söderlund]

Hi Jacopo,

Thanks for your work.

On 2018-08-24 12:24:19 +0200, Jacopo Mondi wrote:
> Add support for D3 Draak board.
> 
> Draak has its HDMI input connected to an ADV7612 which connects to VIN4
> parallel data inputs.
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> ---
>  scripts/boards.sh | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/scripts/boards.sh b/scripts/boards.sh
> index 26fdab9..7eb3a27 100644
> --- a/scripts/boards.sh
> +++ b/scripts/boards.sh
> @@ -29,6 +29,13 @@ case $info in
>          # for V3M, but results in an image.
>          parallelformat="YUYV8_1X16"
>          ;;
> +    "Renesas Draak board based on r8a77995")
> +        gen="gen3"
> +        vins="4"
> +        parallelname="adv7612 0-004c"
> +        # FIXME: This is a hackfor D3, but results in an image.
> +        parallelformat="YUYV8_1X16"
> +        ;;
>      "Koelsch")
>          gen="gen2"
>  
> @@ -70,6 +77,8 @@ if [[ "$gen" == "gen3" ]]; then
>  
>          txaname="adv748x 0-0070 txa"
>          txbname="adv748x 0-0070 txb"
> +    elif [[ "$info" == "Renesas Draak board based on r8a77995" ]]; then
> +        hdminame="adv7612 0-004c"

I don't have the D3 schematics at hand but this feels wrong. Why do you 
define the adv7612 as both a parallel and CSI-2 source? IIRC the D3 have 
no CSI-2 IP?

The reason this exists for V3M is that it has both parallel input in the 
form of a adv7612 and a CSI-2 input in the for of a adv7482 connected to 
the CSI40 IP.

>      else
>          cvbsname="adv748x 4-0070 afe"
>          hdminame="adv748x 4-0070 hdmi"
> -- 
> 2.7.4
> 

-- 
Regards,
Niklas S�derlund

Re: [PATCH v2 2/4] vin-tests: Fix vdev name in propagate_parallel

[Niklas Söderlund]

Hi Jacopo,

Thanks for finding this bug.

On 2018-08-24 12:24:20 +0200, Jacopo Mondi wrote:
> The $vdev variable was not defined in propagate_parallel() function.
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>

Reviewed-by: Niklas S�derlund <niklas.soderlund+renesas@ragnatech.se>

Applied to my tree.

> ---
>  scripts/vin-tests.sh | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/scripts/vin-tests.sh b/scripts/vin-tests.sh
> index ebb1477..0c5b29a 100644
> --- a/scripts/vin-tests.sh
> +++ b/scripts/vin-tests.sh
> @@ -83,6 +83,7 @@ mc_propagate_cvbs() {
>  }
>  
>  mc_propagate_parallel() {
> +    vin="$1"
>      mdev=$(mc_get_mdev)
>  
>      cam="'$parallelname':1"
> -- 
> 2.7.4
> 

-- 
Regards,
Niklas S�derlund

Re: [PATCH v2 3/4] vin-tests: Add capture format for parallel input

[Niklas Söderlund]

Hi Jacopo,

Thanks for your work.

On 2018-08-24 12:24:21 +0200, Jacopo Mondi wrote:
> Add configurable capture format to propagate_parallel() function.
> The capture format is the image format set on the VIN nodes.
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> ---
>  scripts/boards.sh    | 2 ++
>  scripts/vin-tests.sh | 2 +-
>  2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/scripts/boards.sh b/scripts/boards.sh
> index 7eb3a27..b379af6 100644
> --- a/scripts/boards.sh
> +++ b/scripts/boards.sh
> @@ -28,6 +28,7 @@ case $info in
>          # FIXME: This is a hack and not the correct mbus format
>          # for V3M, but results in an image.
>          parallelformat="YUYV8_1X16"
> +	parallel_captureformat="RGB565"

I'm sorry I don't see the value of this change, am I missing something?  
There is no functional change but I assume you use this for something 
but until I figure out what I will leave this change hanging.

>          ;;
>      "Renesas Draak board based on r8a77995")
>          gen="gen3"
> @@ -35,6 +36,7 @@ case $info in
>          parallelname="adv7612 0-004c"
>          # FIXME: This is a hackfor D3, but results in an image.
>          parallelformat="YUYV8_1X16"
> +	parallel_captureformat="RGB565"
>          ;;
>      "Koelsch")
>          gen="gen2"
> diff --git a/scripts/vin-tests.sh b/scripts/vin-tests.sh
> index 0c5b29a..e7b7a48 100644
> --- a/scripts/vin-tests.sh
> +++ b/scripts/vin-tests.sh
> @@ -111,5 +111,5 @@ mc_propagate_parallel() {
>      echo "format: $format size: $size/$vinsize field: $field/$vinfield vdev: $vdev"
>  
>      $mediactl -d $mdev -V "$cam [fmt:$format/$size field:$field]"
> -    yavta -f RGB565 -s $vinsize --field $vinfield $vdev
> +    yavta -f $parallel_captureformat -s $vinsize --field $vinfield $vdev
>  }
> -- 
> 2.7.4
> 

-- 
Regards,
Niklas S�derlund

Re: [PATCH v2 3/4] vin-tests: Add capture format for parallel input

[jacopo mondi]

[-- Attachment #1: Type: text/plain, Size: 2233 bytes --]

Hi Niklas,

On Fri, Aug 24, 2018 at 06:18:13PM +0200, Niklas Söderlund wrote:
> Hi Jacopo,
>
> Thanks for your work.
>
> On 2018-08-24 12:24:21 +0200, Jacopo Mondi wrote:
> > Add configurable capture format to propagate_parallel() function.
> > The capture format is the image format set on the VIN nodes.
> >
> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> > ---
> >  scripts/boards.sh    | 2 ++
> >  scripts/vin-tests.sh | 2 +-
> >  2 files changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/scripts/boards.sh b/scripts/boards.sh
> > index 7eb3a27..b379af6 100644
> > --- a/scripts/boards.sh
> > +++ b/scripts/boards.sh
> > @@ -28,6 +28,7 @@ case $info in
> >          # FIXME: This is a hack and not the correct mbus format
> >          # for V3M, but results in an image.
> >          parallelformat="YUYV8_1X16"
> > +	parallel_captureformat="RGB565"
>
> I'm sorry I don't see the value of this change, am I missing something?
> There is no functional change but I assume you use this for something
> but until I figure out what I will leave this change hanging.
> >          ;;
> >      "Renesas Draak board based on r8a77995")
> >          gen="gen3"
> > @@ -35,6 +36,7 @@ case $info in
> >          parallelname="adv7612 0-004c"
> >          # FIXME: This is a hackfor D3, but results in an image.
> >          parallelformat="YUYV8_1X16"
> > +	parallel_captureformat="RGB565"

Oops, this was supposed to be a different format, that's why I have
added this new variable.

I'll fix it in next iteration.

> >          ;;
> >      "Koelsch")
> >          gen="gen2"
> > diff --git a/scripts/vin-tests.sh b/scripts/vin-tests.sh
> > index 0c5b29a..e7b7a48 100644
> > --- a/scripts/vin-tests.sh
> > +++ b/scripts/vin-tests.sh
> > @@ -111,5 +111,5 @@ mc_propagate_parallel() {
> >      echo "format: $format size: $size/$vinsize field: $field/$vinfield vdev: $vdev"
> >
> >      $mediactl -d $mdev -V "$cam [fmt:$format/$size field:$field]"
> > -    yavta -f RGB565 -s $vinsize --field $vinfield $vdev
> > +    yavta -f $parallel_captureformat -s $vinsize --field $vinfield $vdev


> >  }
> > --
> > 2.7.4
> >
>
> --
> Regards,
> Niklas Söderlund

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v2 4/4] vin-tests: yavta-hdmi: Add VIN4 and parallel link

[Niklas Söderlund]

Hi Jacopo,

Thanks for your patch.

On 2018-08-24 12:24:22 +0200, Jacopo Mondi wrote:
> Add support for VIN4 to yavta-hdmi and check if format propagation should
> go through 'mc_propagate_parallel()' if the HDMI receiver chip is an
> ADV7612 one.
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> ---
>  yavta-hdmi | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/yavta-hdmi b/yavta-hdmi
> index fdec546..2e3b625 100755
> --- a/yavta-hdmi
> +++ b/yavta-hdmi
> @@ -33,14 +33,23 @@ case $vc in
>          dev=/dev/$vin3
>          csipad=4
>          ;;
> +    4)
> +        vinname=$vinname4
> +        dev=/dev/$vin4

I think you should also add a csipad declaration here as if the script 
is used on a board which is not D3 VIN4 would be connected to a CSI-2 
bus. Writing that I realise a new var 'csidev' or something would be 
needed here to expand this to cover the full range of VIN0-VIN7.

> +        ;;
>      *)
>          echo "Unkown VC '$vc'"
>          exit 1
>  esac
>  
>  mc_reset
> -mc_set_link "$csi40name" $csipad "$vinname" 1
> -mc_propagate_format "$hdminame" 1 "$txaname" 0 "$csi40name" $csipad "$vinname"
> +if [[ "$hdminame" == "adv7612 0-004c" ]]; then


You should use $parallelname here not $hdminame. Furthermore I thin you 
should check if the variable is empty or not and not target it for a 
specific board.

A good (or only) example of how I think this should be done can be found 
in test-qv4l2.sh.

         if [[ "$csi20name" != "" ]]; then
             mc_set_link "$csi20name" 1 "$vinname1" 1
             mc_propagate_cvbs "$vinname1"
             qv4l2 -d /dev/$vin1
         fi

         if [[ "$parallelname" != "" ]]; then
             mc_reset
             mc_set_link "$parallelname" 1 "$vinname0" 1
             mc_propagate_parallel "$vinname0"
             qv4l2 -d /dev/$vin0
         fi

> +	mc_set_link "$hdminame" 1  "$vinname" 1
> +	mc_propagate_parallel "$vinname"
> +else
> +	mc_set_link "$csi40name" $csipad "$vinname" 1
> +	mc_propagate_format "$hdminame" 1 "$txaname" 0 "$csi40name" $csipad "$vinname"
> +fi
>  
>  out=/tmp/vin-tests
>  rm -fr $out
> -- 
> 2.7.4
> 

-- 
Regards,
Niklas S�derlund

Re: [PATCH v2 1/4] vin-tests: Add support for D3 Draak

[jacopo mondi]

[-- Attachment #1: Type: text/plain, Size: 2109 bytes --]

Hi Niklas,

On Fri, Aug 24, 2018 at 06:12:29PM +0200, Niklas Söderlund wrote:
> Hi Jacopo,
>
> Thanks for your work.
>
> On 2018-08-24 12:24:19 +0200, Jacopo Mondi wrote:
> > Add support for D3 Draak board.
> >
> > Draak has its HDMI input connected to an ADV7612 which connects to VIN4
> > parallel data inputs.
> >
> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> > ---
> >  scripts/boards.sh | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> >
> > diff --git a/scripts/boards.sh b/scripts/boards.sh
> > index 26fdab9..7eb3a27 100644
> > --- a/scripts/boards.sh
> > +++ b/scripts/boards.sh
> > @@ -29,6 +29,13 @@ case $info in
> >          # for V3M, but results in an image.
> >          parallelformat="YUYV8_1X16"
> >          ;;
> > +    "Renesas Draak board based on r8a77995")
> > +        gen="gen3"
> > +        vins="4"
> > +        parallelname="adv7612 0-004c"
> > +        # FIXME: This is a hackfor D3, but results in an image.
> > +        parallelformat="YUYV8_1X16"
> > +        ;;
> >      "Koelsch")
> >          gen="gen2"
> >
> > @@ -70,6 +77,8 @@ if [[ "$gen" == "gen3" ]]; then
> >
> >          txaname="adv748x 0-0070 txa"
> >          txbname="adv748x 0-0070 txb"
> > +    elif [[ "$info" == "Renesas Draak board based on r8a77995" ]]; then
> > +        hdminame="adv7612 0-004c"
>
> I don't have the D3 schematics at hand but this feels wrong. Why do you
> define the adv7612 as both a parallel and CSI-2 source? IIRC the D3 have
> no CSI-2 IP?
>

Why is "hdminame" variable the CSI-2 source? It might as well be the
VIN source, as for D3.

To me, and I have added it here because it is used in yavta-hdmi in
this way, is the HDMI input component.

Want to change the name?

> The reason this exists for V3M is that it has both parallel input in the
> form of a adv7612 and a CSI-2 input in the for of a adv7482 connected to
> the CSI40 IP.
>
> >      else
> >          cvbsname="adv748x 4-0070 afe"
> >          hdminame="adv748x 4-0070 hdmi"
> > --
> > 2.7.4
> >
>
> --
> Regards,
> Niklas Söderlund

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v2 1/4] vin-tests: Add support for D3 Draak

[Niklas Söderlund]

Hi Jacopo,

On 2018-08-24 18:28:49 +0200, Jacopo Mondi wrote:
> Hi Niklas,
> 
> On Fri, Aug 24, 2018 at 06:12:29PM +0200, Niklas S�derlund wrote:
> > Hi Jacopo,
> >
> > Thanks for your work.
> >
> > On 2018-08-24 12:24:19 +0200, Jacopo Mondi wrote:
> > > Add support for D3 Draak board.
> > >
> > > Draak has its HDMI input connected to an ADV7612 which connects to VIN4
> > > parallel data inputs.
> > >
> > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> > > ---
> > >  scripts/boards.sh | 9 +++++++++
> > >  1 file changed, 9 insertions(+)
> > >
> > > diff --git a/scripts/boards.sh b/scripts/boards.sh
> > > index 26fdab9..7eb3a27 100644
> > > --- a/scripts/boards.sh
> > > +++ b/scripts/boards.sh
> > > @@ -29,6 +29,13 @@ case $info in
> > >          # for V3M, but results in an image.
> > >          parallelformat="YUYV8_1X16"
> > >          ;;
> > > +    "Renesas Draak board based on r8a77995")
> > > +        gen="gen3"
> > > +        vins="4"
> > > +        parallelname="adv7612 0-004c"
> > > +        # FIXME: This is a hackfor D3, but results in an image.
> > > +        parallelformat="YUYV8_1X16"
> > > +        ;;
> > >      "Koelsch")
> > >          gen="gen2"
> > >
> > > @@ -70,6 +77,8 @@ if [[ "$gen" == "gen3" ]]; then
> > >
> > >          txaname="adv748x 0-0070 txa"
> > >          txbname="adv748x 0-0070 txb"
> > > +    elif [[ "$info" == "Renesas Draak board based on r8a77995" ]]; then
> > > +        hdminame="adv7612 0-004c"
> >
> > I don't have the D3 schematics at hand but this feels wrong. Why do you
> > define the adv7612 as both a parallel and CSI-2 source? IIRC the D3 have
> > no CSI-2 IP?
> >
> 
> Why is "hdminame" variable the CSI-2 source? It might as well be the
> VIN source, as for D3.

Well the design is currently that the parallel source is named 
'parallelname'. I agree that the name hdminame is not veary good in this 
context but it came from the time when a parallel source was not 
supported by the driver and the 'parallelname' variable was added once 
V3M support where added.

> 
> To me, and I have added it here because it is used in yavta-hdmi in
> this way, is the HDMI input component.

Correct yavta-hdmi uses only 'hdminame' variable as that script do not 
(yet) support parallel video source, only test-qv4l2.sh do this at the 
moment.

> 
> Want to change the name?
> 
> > The reason this exists for V3M is that it has both parallel input in the
> > form of a adv7612 and a CSI-2 input in the for of a adv7482 connected to
> > the CSI40 IP.
> >
> > >      else
> > >          cvbsname="adv748x 4-0070 afe"
> > >          hdminame="adv748x 4-0070 hdmi"
> > > --
> > > 2.7.4
> > >
> >
> > --
> > Regards,
> > Niklas S�derlund



-- 
Regards,
Niklas S�derlund

Re: [PATCH v2 4/4] vin-tests: yavta-hdmi: Add VIN4 and parallel link

[jacopo mondi]

[-- Attachment #1: Type: text/plain, Size: 2615 bytes --]

Hi Niklas,

On Fri, Aug 24, 2018 at 06:27:02PM +0200, Niklas Söderlund wrote:
> Hi Jacopo,
>
> Thanks for your patch.
>
> On 2018-08-24 12:24:22 +0200, Jacopo Mondi wrote:
> > Add support for VIN4 to yavta-hdmi and check if format propagation should
> > go through 'mc_propagate_parallel()' if the HDMI receiver chip is an
> > ADV7612 one.
> >
> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> > ---
> >  yavta-hdmi | 13 +++++++++++--
> >  1 file changed, 11 insertions(+), 2 deletions(-)
> >
> > diff --git a/yavta-hdmi b/yavta-hdmi
> > index fdec546..2e3b625 100755
> > --- a/yavta-hdmi
> > +++ b/yavta-hdmi
> > @@ -33,14 +33,23 @@ case $vc in
> >          dev=/dev/$vin3
> >          csipad=4
> >          ;;
> > +    4)
> > +        vinname=$vinname4
> > +        dev=/dev/$vin4
>
> I think you should also add a csipad declaration here as if the script
> is used on a board which is not D3 VIN4 would be connected to a CSI-2
> bus. Writing that I realise a new var 'csidev' or something would be
> needed here to expand this to cover the full range of VIN0-VIN7.
>
> > +        ;;
> >      *)
> >          echo "Unkown VC '$vc'"
> >          exit 1
> >  esac
> >
> >  mc_reset
> > -mc_set_link "$csi40name" $csipad "$vinname" 1
> > -mc_propagate_format "$hdminame" 1 "$txaname" 0 "$csi40name" $csipad "$vinname"
> > +if [[ "$hdminame" == "adv7612 0-004c" ]]; then
>
>
> You should use $parallelname here not $hdminame. Furthermore I thin you
> should check if the variable is empty or not and not target it for a
> specific board.
>
> A good (or only) example of how I think this should be done can be found
> in test-qv4l2.sh.
>
>          if [[ "$csi20name" != "" ]]; then
>              mc_set_link "$csi20name" 1 "$vinname1" 1
>              mc_propagate_cvbs "$vinname1"
>              qv4l2 -d /dev/$vin1
>          fi
>
>          if [[ "$parallelname" != "" ]]; then
>              mc_reset
>              mc_set_link "$parallelname" 1 "$vinname0" 1
>              mc_propagate_parallel "$vinname0"
>              qv4l2 -d /dev/$vin0
>          fi
>

This will solve the naming thing in boards.sh

I'll try that and have a look at test-qv4l2.sh too!

Thanks
  j

> > +	mc_set_link "$hdminame" 1  "$vinname" 1
> > +	mc_propagate_parallel "$vinname"
> > +else
> > +	mc_set_link "$csi40name" $csipad "$vinname" 1
> > +	mc_propagate_format "$hdminame" 1 "$txaname" 0 "$csi40name" $csipad "$vinname"
> > +fi
> >
> >  out=/tmp/vin-tests
> >  rm -fr $out
> > --
> > 2.7.4
> >
>
> --
> Regards,
> Niklas Söderlund

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]