Hi Niklas, where is this patch supposed to be applied on? I tried master, media master, renesas-drivers and your rcar-csi2 and v4l2/mux branches, but it fails on all of them :( What am I doing wrong? On Sat, Feb 16, 2019 at 11:57:58PM +0100, Niklas Söderlund wrote: > Allow the hardware to to do proper field detection for interlaced field > formats by implementing s_std() and g_std(). Depending on which video > standard is selected the driver needs to setup the hardware to correctly > identify fields. > > Later versions of the datasheet have also been updated to make it clear > that FLD register should be set to 0 when dealing with none interlaced > field formats. > > Signed-off-by: Niklas Söderlund > --- > drivers/media/platform/rcar-vin/rcar-csi2.c | 33 +++++++++++++++++++-- > 1 file changed, 30 insertions(+), 3 deletions(-) > > diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c b/drivers/media/platform/rcar-vin/rcar-csi2.c > index f3099f3e536d808a..664d3784be2b9db9 100644 > --- a/drivers/media/platform/rcar-vin/rcar-csi2.c > +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c > @@ -361,6 +361,7 @@ struct rcar_csi2 { > struct v4l2_subdev *remote; > > struct v4l2_mbus_framefmt mf; > + v4l2_std_id std; > > struct mutex lock; > int stream_count; > @@ -389,6 +390,22 @@ static void rcsi2_write(struct rcar_csi2 *priv, unsigned int reg, u32 data) > iowrite32(data, priv->base + reg); > } > > +static int rcsi2_s_std(struct v4l2_subdev *sd, v4l2_std_id std) > +{ > + struct rcar_csi2 *priv = sd_to_csi2(sd); > + > + priv->std = std; > + return 0; Nit: (almost) all other functions in the file have an empty line before return... > +} > + > +static int rcsi2_g_std(struct v4l2_subdev *sd, v4l2_std_id *std) > +{ > + struct rcar_csi2 *priv = sd_to_csi2(sd); > + > + *std = priv->std; Should priv->std be initialized or STD_UNKNOWN is fine? > + return 0; > +} > + > static void rcsi2_standby_mode(struct rcar_csi2 *priv, int on) > { > if (!on) { > @@ -475,7 +492,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 +524,15 @@ static int rcsi2_start_receiver(struct rcar_csi2 *priv) > vcdt2 |= vcdt_part << ((i % 2) * 16); > } > > + if (priv->mf.field != V4L2_FIELD_NONE) { I cannot tell where rcsi2_start_receiver() is called, as I don't have it in my local version, but I suppose it has been break out from rcsi2_start() has they set the same register. So this is called at s_stream() time and priv->mf at set_format() time, right? > + fld = FLD_FLD_EN4 | FLD_FLD_EN3 | FLD_FLD_EN2 | FLD_FLD_EN; > + > + if (priv->std & V4L2_STD_525_60) > + fld |= FLD_FLD_NUM(2); > + else > + fld |= FLD_FLD_NUM(1); I haven't been able to find an explanation on why the field detection depends on this specific video standard... I guess it is defined in some standard I'm ignorant of, so I assume this is correct :) Thanks j > + } > + > phycnt = PHYCNT_ENABLECLK; > phycnt |= (1 << priv->lanes) - 1; > > @@ -519,8 +545,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); > @@ -662,6 +687,8 @@ static int rcsi2_get_pad_format(struct v4l2_subdev *sd, > } > > static const struct v4l2_subdev_video_ops rcar_csi2_video_ops = { > + .s_std = rcsi2_s_std, > + .g_std = rcsi2_g_std, > .s_stream = rcsi2_s_stream, > }; > > -- > 2.20.1 >