Hi Niklas, Thank you for the patch. On Wed, Mar 13, 2019 at 12:49:55AM +0100, 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(-) > --- > Hi, > > This patch depends on '[PATCH v3 0/2] rcar-csi2: Use standby mode > instead of resetting'. > > * Changes since v2 > - Set FLD_DET_SEL = 01 > > diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c b/drivers/media/platform/rcar-vin/rcar-csi2.c > index 10f1b4978ed7dcc6..6c7c7e6072ffb09e 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) { > + fld = FLD_DET_SEL(1) | FLD_FLD_EN4 | FLD_FLD_EN3 | FLD_FLD_EN2 > + | FLD_FLD_EN; > + > + if (priv->mf.height == 240) > + fld |= FLD_FLD_NUM(2); Should this be FLD_FLD_NUM(0) ? It won't make a difference in practice as FLD_DET_SEL(1) ensures that only bit 0 is taken into account, but I think the intent would be clearer (and the compiler will optimize it out as FLD_FLD_NUM(0) == 0). > + else > + fld |= FLD_FLD_NUM(1); > + } > + > phycnt = PHYCNT_ENABLECLK; > phycnt |= (1 << priv->lanes) - 1; > > @@ -519,8 +530,7 @@ static int rcsi2_start_receiver(struct rcar_csi2 *priv) > rcsi2_write(priv, PHTC_REG, 0); > > /* Configure */ > - 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, VCDT_REG, vcdt); > if (vcdt2) > rcsi2_write(priv, VCDT2_REG, vcdt2); -- Regards, Laurent Pinchart