Re: [PATCH 3/6] media: rcar-vin: Handle data-active property

[Sergei Shtylyov]

Hello!

On 5/16/2018 7:32 PM, Jacopo Mondi wrote:


The data-active property has to be specified when running with embedded


   Prop names are typically enclosed in "".


synchronization. The VIN peripheral can use HSYNC in place of CLOCKENB


   CLKENB, maybe?


when the CLOCKENB pin is not connected, this requires explicit
synchronization to be in use.

Now that the driver supports 'data-active' property, it makes not sense


   "data-active", s/not/no/.


to zero the mbus configuration flags when running with implicit synch


   Sync is better. :-)


(V4L2_MBUS_BT656).

Signed-off-by: Jacopo Mondi 

[...]

MBR, Sergei

Re: [PATCH 3/6] media: rcar-vin: Handle data-active property

[jacopo mondi]

Hi Niklas,

On Wed, May 16, 2018 at 11:58:47PM +0200, Niklas Söderlund wrote:
> Hi Jacopo,
>
> Thanks for your work.
>
> On 2018-05-16 18:32:29 +0200, Jacopo Mondi wrote:
> > The data-active property has to be specified when running with embedded
> > synchronization. The VIN peripheral can use HSYNC in place of CLOCKENB
> > when the CLOCKENB pin is not connected, this requires explicit
> > synchronization to be in use.
>
> Is this really the intent of the data-active property? I read the
> video-interfaces.txt document as such as if no hsync-active,
> vsync-active and data-active we should use the embedded synchronization
> else set the polarity for the requested pins. What am I not
> understanding here?

Almost correct.

The presence of hsync-active, vsync-active and field-evev-active
properties determinate the bus type we're running on. If none of the
is specified, the bus is marked 'BT656' and we assume the system is
using embedded synchronization.

data-active does not take part in the bus identification, and my
reasoning was the other way around as explained in reply to your
comment on [2/6], and as explained there my reasoning is probably
wrong, and we should set CHS -only- when running with explicit
synchronization, instead of making it mandatory when running with
embedded syncs.

Thanks and sorry for my confusion.

j
>
> >
> > Now that the driver supports 'data-active' property, it makes not sense
> > to zero the mbus configuration flags when running with implicit synch
> > (V4L2_MBUS_BT656).
> >
> > Signed-off-by: Jacopo Mondi 
> > ---
> >  drivers/media/platform/rcar-vin/rcar-core.c | 10 --
> >  1 file changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/media/platform/rcar-vin/rcar-core.c 
> > b/drivers/media/platform/rcar-vin/rcar-core.c
> > index d3072e1..075d08f 100644
> > --- a/drivers/media/platform/rcar-vin/rcar-core.c
> > +++ b/drivers/media/platform/rcar-vin/rcar-core.c
> > @@ -531,15 +531,21 @@ static int rvin_digital_parse_v4l2(struct device *dev,
> > return -ENOTCONN;
> >
> > vin->mbus_cfg.type = vep->bus_type;
> > +   vin->mbus_cfg.flags = vep->bus.parallel.flags;
> >
> > switch (vin->mbus_cfg.type) {
> > case V4L2_MBUS_PARALLEL:
> > vin_dbg(vin, "Found PARALLEL media bus\n");
> > -   vin->mbus_cfg.flags = vep->bus.parallel.flags;
> > break;
> > case V4L2_MBUS_BT656:
> > vin_dbg(vin, "Found BT656 media bus\n");
> > -   vin->mbus_cfg.flags = 0;
> > +
> > +   if (!(vin->mbus_cfg.flags & V4L2_MBUS_DATA_ACTIVE_HIGH) &&
> > +   !(vin->mbus_cfg.flags & V4L2_MBUS_DATA_ACTIVE_LOW)) {
> > +   vin_err(vin,
> > +   "Missing data enable signal polarity 
> > property\n");
>
> I fear this can't be an error as that would break backward comp ability
> with existing dtb's.
>
> > +   return -EINVAL;
> > +   }
> > break;
> > default:
> > vin_err(vin, "Unknown media bus type\n");
> > --
> > 2.7.4
> >
>
> --
> Regards,
> Niklas Söderlund


signature.asc
Description: PGP signature

Re: [PATCH 3/6] media: rcar-vin: Handle data-active property

[Niklas Söderlund]

Hi Jacopo,

Thanks for your work.

On 2018-05-16 18:32:29 +0200, Jacopo Mondi wrote:
> The data-active property has to be specified when running with embedded
> synchronization. The VIN peripheral can use HSYNC in place of CLOCKENB
> when the CLOCKENB pin is not connected, this requires explicit
> synchronization to be in use.

Is this really the intent of the data-active property? I read the 
video-interfaces.txt document as such as if no hsync-active, 
vsync-active and data-active we should use the embedded synchronization 
else set the polarity for the requested pins. What am I not 
understanding here?

> 
> Now that the driver supports 'data-active' property, it makes not sense
> to zero the mbus configuration flags when running with implicit synch
> (V4L2_MBUS_BT656).
> 
> Signed-off-by: Jacopo Mondi 
> ---
>  drivers/media/platform/rcar-vin/rcar-core.c | 10 --
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-core.c 
> b/drivers/media/platform/rcar-vin/rcar-core.c
> index d3072e1..075d08f 100644
> --- a/drivers/media/platform/rcar-vin/rcar-core.c
> +++ b/drivers/media/platform/rcar-vin/rcar-core.c
> @@ -531,15 +531,21 @@ static int rvin_digital_parse_v4l2(struct device *dev,
>   return -ENOTCONN;
>  
>   vin->mbus_cfg.type = vep->bus_type;
> + vin->mbus_cfg.flags = vep->bus.parallel.flags;
>  
>   switch (vin->mbus_cfg.type) {
>   case V4L2_MBUS_PARALLEL:
>   vin_dbg(vin, "Found PARALLEL media bus\n");
> - vin->mbus_cfg.flags = vep->bus.parallel.flags;
>   break;
>   case V4L2_MBUS_BT656:
>   vin_dbg(vin, "Found BT656 media bus\n");
> - vin->mbus_cfg.flags = 0;
> +
> + if (!(vin->mbus_cfg.flags & V4L2_MBUS_DATA_ACTIVE_HIGH) &&
> + !(vin->mbus_cfg.flags & V4L2_MBUS_DATA_ACTIVE_LOW)) {
> + vin_err(vin,
> + "Missing data enable signal polarity 
> property\n");

I fear this can't be an error as that would break backward comp ability 
with existing dtb's.

> + return -EINVAL;
> + }
>   break;
>   default:
>   vin_err(vin, "Unknown media bus type\n");
> -- 
> 2.7.4
> 

-- 
Regards,
Niklas Söderlund