Hi Jacopo,
Thanks for your feedback.
On 2019-02-17 19:41:40 +0100, Jacopo Mondi wrote:
> 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?
You answered your own question in a later mail, thanks for that ;-)
>
> 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 <niklas.soderlund+renesas@xxxxxxxxxxxx>
> > ---
> > 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...
Will fix.
>
> > +}
> > +
> > +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?
STD_UNKNOWN is fine as if the standard is not explicitly set by the user
it is in fact unknown.
>
> > + 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?
Yes.
>
> > + 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 :)
It defines temporal order of field transmission (TOP or BOTTOM) is
transmitted first. PAL and NTSC differs in this regard and the R-Car
CSI-2 needs to be informed of what standard is received.
See:
https://linuxtv.org/downloads/v4l-dvb-apis/uapi/v4l/field-order.html
>
> 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
> >
--
Regards,
Niklas Söderlund
