Hi Jacopo,
Thanks for your feedback.
On 2019-04-12 10:43:46 +0200, Jacopo Mondi wrote:
> Hi Niklas,
> I'm rebasing v4l2-mux series on top of your new VIN patches and
> on this on I'm not sure how to proceed. Please see below.
>
> Please also note this comment is not strictly on this patch, and should not
> block its acceptance...
>
> On Thu, Apr 11, 2019 at 10:34:44PM +0200, Niklas Söderlund wrote:
> > Depending on which video standard is used the driver needs to setup the
> > hardware to correctly handle fields. If stream is identified as NTSC
> > or PAL setup field detection and propagate the field detection signal.
> >
> > Later versions of the datasheet have been updated to make it clear
> > that FLD register should be set to 0 when dealing with non-interlaced
> > field formats.
> >
> > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@xxxxxxxxxxxx>
> > ---
> > drivers/media/platform/rcar-vin/rcar-csi2.c | 16 +++++++++++++---
> > 1 file changed, 13 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c b/drivers/media/platform/rcar-vin/rcar-csi2.c
> > index 799e526fd3df5554..c1b38ebd061dbc35 100644
> > --- a/drivers/media/platform/rcar-vin/rcar-csi2.c
> > +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
> > @@ -68,6 +68,7 @@ struct rcar_csi2;
> > /* Field Detection Control */
> > #define FLD_REG 0x1c
> > #define FLD_FLD_NUM(n) (((n) & 0xff) << 16)
> > +#define FLD_DET_SEL(n) (((n) & 0x3) << 4)
> > #define FLD_FLD_EN4 BIT(3)
> > #define FLD_FLD_EN3 BIT(2)
> > #define FLD_FLD_EN2 BIT(1)
> > @@ -475,7 +476,7 @@ static int rcsi2_calc_mbps(struct rcar_csi2 *priv, unsigned int bpp)
> > static int rcsi2_start_receiver(struct rcar_csi2 *priv)
> > {
> > const struct rcar_csi2_format *format;
> > - u32 phycnt, vcdt = 0, vcdt2 = 0;
> > + u32 phycnt, vcdt = 0, vcdt2 = 0, fld = 0;
> > unsigned int i;
> > int mbps, ret;
> >
> > @@ -507,6 +508,16 @@ static int rcsi2_start_receiver(struct rcar_csi2 *priv)
> > vcdt2 |= vcdt_part << ((i % 2) * 16);
> > }
> >
> > + if (priv->mf.field == V4L2_FIELD_ALTERNATE) {
>
> How would you handle this once priv->mf is expanded to become an array
> of 'struct v4l2_mbus_framefmt' entries [1] ? Which of them is relevant for
> field detection?
>
>
> > + fld = FLD_DET_SEL(1) | FLD_FLD_EN4 | FLD_FLD_EN3 | FLD_FLD_EN2
> > + | FLD_FLD_EN;
> > +
> > + if (priv->mf.height == 240)
>
> Same question here, even if I assume we could cycle on the entries and
> collect the maximum size.
Well as the hardware do not support field detection of PAL and NTSC at
the same time one would need to either disallow that and fail stream on
or pick one and move on. Not sure which one would be best.
>
> Thanks
> j
>
> [1] Introduced by:
> "rcar-csi2: use frame description information to configure CSI-2 bus"
> > + fld |= FLD_FLD_NUM(0);
> > + else
> > + fld |= FLD_FLD_NUM(1);
> > + }
> > +
> > phycnt = PHYCNT_ENABLECLK;
> > phycnt |= (1 << priv->lanes) - 1;
> >
> > @@ -549,8 +560,7 @@ static int rcsi2_start_receiver(struct rcar_csi2 *priv)
> > rcsi2_write(priv, PHYCNT_REG, phycnt);
> > rcsi2_write(priv, LINKCNT_REG, LINKCNT_MONITOR_EN |
> > LINKCNT_REG_MONI_PACT_EN | LINKCNT_ICLK_NONSTOP);
> > - rcsi2_write(priv, FLD_REG, FLD_FLD_NUM(2) | FLD_FLD_EN4 |
> > - FLD_FLD_EN3 | FLD_FLD_EN2 | FLD_FLD_EN);
> > + rcsi2_write(priv, FLD_REG, fld);
> > rcsi2_write(priv, PHYCNT_REG, phycnt | PHYCNT_SHUTDOWNZ);
> > rcsi2_write(priv, PHYCNT_REG, phycnt | PHYCNT_SHUTDOWNZ | PHYCNT_RSTZ);
> >
> > --
> > 2.21.0
> >
--
Regards,
Niklas Söderlund
