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

[jacopo mondi <jacopo@jmondi.org>]

[-- 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 --]