Message ID | 1526488352-898-4-git-send-email-jacopo+renesas@jmondi.org (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
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 <jacopo+renesas@jmondi.org> > --- > 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 >
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 <jacopo+renesas@jmondi.org> > > --- > > 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
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 <jacopo+renesas@jmondi.org> [...] MBR, Sergei
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"); + return -EINVAL; + } break; default: vin_err(vin, "Unknown media bus type\n");
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. 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 <jacopo+renesas@jmondi.org> --- drivers/media/platform/rcar-vin/rcar-core.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)