Hi Hans,

Thanks for your feedback.

On 2019-03-08 15:12:15 +0100, Hans Verkuil wrote:
> On 2/16/19 11:57 PM, 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.
> 
> Nacked-by: Hans Verkuil <hverkuil-ci...@xs4all.nl>
> 
> The G/S_STD and QUERYSTD ioctls are specific for SDTV video receivers
> (composite, S-Video, analog tuner) and can't be used for CSI devices.
> 
> struct v4l2_mbus_framefmt already has a 'field' value that is explicit
> about the field ordering (TB vs BT) or the field ordering can be deduced
> from the frame height (FIELD_INTERLACED).

I will drop this patch and write a new one using field and height as 
suggested by you and Laurent. Thanks for the suggestion!

> 
> Regards,
> 
>       Hans
> 
> > 
> > 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;
> > +}
> > +
> > +static int rcsi2_g_std(struct v4l2_subdev *sd, v4l2_std_id *std)
> > +{
> > +   struct rcar_csi2 *priv = sd_to_csi2(sd);
> > +
> > +   *std = priv->std;
> > +   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) {
> > +           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);
> > +   }
> > +
> >     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,
> >  };
> >  
> > 
> 

-- 
Regards,
Niklas Söderlund

Reply via email to