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 <niklas.soderlund+rene...@ragnatech.se>
> ---
>  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
>

Attachment: signature.asc
Description: PGP signature

Reply via email to