diff mbox series

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

Message ID 1535106262-13004-5-git-send-email-jacopo@jmondi.org (mailing list archive)
State Not Applicable
Delegated to: Geert Uytterhoeven
Headers show
Series vin-tests: Add D3 Draak support | expand

Commit Message

Jacopo Mondi Aug. 24, 2018, 10:24 a.m. UTC
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(-)

Comments

Niklas Söderlund Aug. 24, 2018, 4:27 p.m. UTC | #1
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
>
Jacopo Mondi Aug. 25, 2018, 9:29 a.m. UTC | #2
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
diff mbox series

Patch

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