* Re: [PATCH] rcar-csi2: Allow configuring of video standard
2019-02-16 22:57 [PATCH] rcar-csi2: Allow configuring of video standard Niklas Söderlund
@ 2019-02-17 8:47 ` Sergei Shtylyov
2019-03-07 0:11 ` Niklas Söderlund
2019-02-17 18:41 ` Jacopo Mondi
` (2 subsequent siblings)
3 siblings, 1 reply; 12+ messages in thread
From: Sergei Shtylyov @ 2019-02-17 8:47 UTC (permalink / raw)
To: Niklas Söderlund, Laurent Pinchart, linux-media; +Cc: linux-renesas-soc
Hello!
On 17.02.2019 1:57, 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
Non-interlaced, perhaps?
> field formats.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
[...]
MBR, Sergei
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] rcar-csi2: Allow configuring of video standard
2019-02-17 8:47 ` Sergei Shtylyov
@ 2019-03-07 0:11 ` Niklas Söderlund
0 siblings, 0 replies; 12+ messages in thread
From: Niklas Söderlund @ 2019-03-07 0:11 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: Laurent Pinchart, linux-media, linux-renesas-soc
Hi Sergei,
Thanks for your feedback.
On 2019-02-17 11:47:28 +0300, Sergei Shtylyov wrote:
> Hello!
>
> On 17.02.2019 1:57, 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
>
> Non-interlaced, perhaps?
Thanks, will fix in v2.
>
> > field formats.
> >
> > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> [...]
>
> MBR, Sergei
--
Regards,
Niklas Söderlund
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] rcar-csi2: Allow configuring of video standard
2019-02-16 22:57 [PATCH] rcar-csi2: Allow configuring of video standard Niklas Söderlund
2019-02-17 8:47 ` Sergei Shtylyov
@ 2019-02-17 18:41 ` Jacopo Mondi
2019-03-07 0:10 ` Niklas Söderlund
2019-03-07 0:13 ` Laurent Pinchart
2019-03-08 14:12 ` Hans Verkuil
3 siblings, 1 reply; 12+ messages in thread
From: Jacopo Mondi @ 2019-02-17 18:41 UTC (permalink / raw)
To: Niklas Söderlund; +Cc: Laurent Pinchart, linux-media, linux-renesas-soc
[-- Attachment #1: Type: text/plain, Size: 4033 bytes --]
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+renesas@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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] rcar-csi2: Allow configuring of video standard
2019-02-17 18:41 ` Jacopo Mondi
@ 2019-03-07 0:10 ` Niklas Söderlund
0 siblings, 0 replies; 12+ messages in thread
From: Niklas Söderlund @ 2019-03-07 0:10 UTC (permalink / raw)
To: Jacopo Mondi; +Cc: Laurent Pinchart, linux-media, linux-renesas-soc
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@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...
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] rcar-csi2: Allow configuring of video standard
2019-02-16 22:57 [PATCH] rcar-csi2: Allow configuring of video standard Niklas Söderlund
2019-02-17 8:47 ` Sergei Shtylyov
2019-02-17 18:41 ` Jacopo Mondi
@ 2019-03-07 0:13 ` Laurent Pinchart
2019-03-07 0:22 ` Niklas Söderlund
2019-03-08 14:12 ` Hans Verkuil
3 siblings, 1 reply; 12+ messages in thread
From: Laurent Pinchart @ 2019-03-07 0:13 UTC (permalink / raw)
To: Niklas Söderlund; +Cc: linux-media, linux-renesas-soc
Hi Niklas,
Thank you for the patch.
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.
I don't think this belongs to the CSI-2 receiver. Standards are really
an analog concept, and should be handled by the analog front-end. At the
CSI-2 level there's no concept of analog standard anymore.
> 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@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,
Laurent Pinchart
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] rcar-csi2: Allow configuring of video standard
2019-03-07 0:13 ` Laurent Pinchart
@ 2019-03-07 0:22 ` Niklas Söderlund
2019-03-07 0:26 ` Laurent Pinchart
0 siblings, 1 reply; 12+ messages in thread
From: Niklas Söderlund @ 2019-03-07 0:22 UTC (permalink / raw)
To: Laurent Pinchart; +Cc: linux-media, linux-renesas-soc
Hi Laurent,
Thanks for your feedback.
On 2019-03-07 02:13:18 +0200, Laurent Pinchart wrote:
> Hi Niklas,
>
> Thank you for the patch.
>
> 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.
>
> I don't think this belongs to the CSI-2 receiver. Standards are really
> an analog concept, and should be handled by the analog front-end. At the
> CSI-2 level there's no concept of analog standard anymore.
I agree it should be handled by the analog front-end. This is patch just
lets the user propagate the information in the pipeline. The driver
could instead find its source subdevice in the media graph and ask which
standard it's supplying.
I wrestled a bit with this and went with this approach as it then works
the same as with other format information, such as dimensions and pixel
format. If the driver acquires the standard by itself why should it no
the same for the format? I'm willing to change this but I would like to
understand where the divider for format propagating in kernel and
user-space is :-)
Also what if there are subdevices between rcar-csi2 and the analog
front-end which do not support the g_std operation?
>
> > 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@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,
>
> Laurent Pinchart
--
Regards,
Niklas Söderlund
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] rcar-csi2: Allow configuring of video standard
2019-03-07 0:22 ` Niklas Söderlund
@ 2019-03-07 0:26 ` Laurent Pinchart
2019-03-07 0:38 ` Niklas Söderlund
0 siblings, 1 reply; 12+ messages in thread
From: Laurent Pinchart @ 2019-03-07 0:26 UTC (permalink / raw)
To: Niklas Söderlund; +Cc: linux-media, linux-renesas-soc
Hi Niklas,
On Thu, Mar 07, 2019 at 01:22:36AM +0100, Niklas Söderlund wrote:
> On 2019-03-07 02:13:18 +0200, Laurent Pinchart wrote:
> > 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.
> >
> > I don't think this belongs to the CSI-2 receiver. Standards are really
> > an analog concept, and should be handled by the analog front-end. At the
> > CSI-2 level there's no concept of analog standard anymore.
>
> I agree it should be handled by the analog front-end. This is patch just
> lets the user propagate the information in the pipeline. The driver
> could instead find its source subdevice in the media graph and ask which
> standard it's supplying.
>
> I wrestled a bit with this and went with this approach as it then works
> the same as with other format information, such as dimensions and pixel
> format. If the driver acquires the standard by itself why should it no
> the same for the format? I'm willing to change this but I would like to
> understand where the divider for format propagating in kernel and
> user-space is :-)
>
> Also what if there are subdevices between rcar-csi2 and the analog
> front-end which do not support the g_std operation?
My point is that the analog standard shouldn't be propagated at all,
neither inside the kernel nor in userspace, as it is not applicable to
CSI-2.
> >> 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@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,
Laurent Pinchart
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] rcar-csi2: Allow configuring of video standard
2019-03-07 0:26 ` Laurent Pinchart
@ 2019-03-07 0:38 ` Niklas Söderlund
2019-03-08 13:03 ` Laurent Pinchart
0 siblings, 1 reply; 12+ messages in thread
From: Niklas Söderlund @ 2019-03-07 0:38 UTC (permalink / raw)
To: Laurent Pinchart; +Cc: linux-media, linux-renesas-soc
Hi Laurent,
On 2019-03-07 02:26:45 +0200, Laurent Pinchart wrote:
> Hi Niklas,
>
> On Thu, Mar 07, 2019 at 01:22:36AM +0100, Niklas Söderlund wrote:
> > On 2019-03-07 02:13:18 +0200, Laurent Pinchart wrote:
> > > 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.
> > >
> > > I don't think this belongs to the CSI-2 receiver. Standards are really
> > > an analog concept, and should be handled by the analog front-end. At the
> > > CSI-2 level there's no concept of analog standard anymore.
> >
> > I agree it should be handled by the analog front-end. This is patch just
> > lets the user propagate the information in the pipeline. The driver
> > could instead find its source subdevice in the media graph and ask which
> > standard it's supplying.
> >
> > I wrestled a bit with this and went with this approach as it then works
> > the same as with other format information, such as dimensions and pixel
> > format. If the driver acquires the standard by itself why should it no
> > the same for the format? I'm willing to change this but I would like to
> > understand where the divider for format propagating in kernel and
> > user-space is :-)
> >
> > Also what if there are subdevices between rcar-csi2 and the analog
> > front-end which do not support the g_std operation?
>
> My point is that the analog standard shouldn't be propagated at all,
> neither inside the kernel nor in userspace, as it is not applicable to
> CSI-2.
This is not related to CSI-2 if I understand it. It is related to the
outputed field signal on the parallel output from CSI-2 facing the VINs.
See chapter "25.4.5 FLD Signal" in the datasheet.
>
> > >> 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@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,
>
> Laurent Pinchart
--
Regards,
Niklas Söderlund
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] rcar-csi2: Allow configuring of video standard
2019-03-07 0:38 ` Niklas Söderlund
@ 2019-03-08 13:03 ` Laurent Pinchart
0 siblings, 0 replies; 12+ messages in thread
From: Laurent Pinchart @ 2019-03-08 13:03 UTC (permalink / raw)
To: Niklas Söderlund; +Cc: linux-media, linux-renesas-soc, Sakari Ailus
Hi Niklas,
(CC'ing Sakari)
On Thu, Mar 07, 2019 at 01:38:20AM +0100, Niklas Söderlund wrote:
> On 2019-03-07 02:26:45 +0200, Laurent Pinchart wrote:
> > On Thu, Mar 07, 2019 at 01:22:36AM +0100, Niklas Söderlund wrote:
> >> On 2019-03-07 02:13:18 +0200, Laurent Pinchart wrote:
> >>> 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.
> >>>
> >>> I don't think this belongs to the CSI-2 receiver. Standards are really
> >>> an analog concept, and should be handled by the analog front-end. At the
> >>> CSI-2 level there's no concept of analog standard anymore.
> >>
> >> I agree it should be handled by the analog front-end. This is patch just
> >> lets the user propagate the information in the pipeline. The driver
> >> could instead find its source subdevice in the media graph and ask which
> >> standard it's supplying.
> >>
> >> I wrestled a bit with this and went with this approach as it then works
> >> the same as with other format information, such as dimensions and pixel
> >> format. If the driver acquires the standard by itself why should it no
> >> the same for the format? I'm willing to change this but I would like to
> >> understand where the divider for format propagating in kernel and
> >> user-space is :-)
> >>
> >> Also what if there are subdevices between rcar-csi2 and the analog
> >> front-end which do not support the g_std operation?
> >
> > My point is that the analog standard shouldn't be propagated at all,
> > neither inside the kernel nor in userspace, as it is not applicable to
> > CSI-2.
>
> This is not related to CSI-2 if I understand it. It is related to the
> outputed field signal on the parallel output from CSI-2 facing the VINs.
> See chapter "25.4.5 FLD Signal" in the datasheet.
I would solely use the information contained in v4l2_mbus_framefmt. You
could use the frame height for instance to detect the type of standard.
> >>>> 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@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,
Laurent Pinchart
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] rcar-csi2: Allow configuring of video standard
2019-02-16 22:57 [PATCH] rcar-csi2: Allow configuring of video standard Niklas Söderlund
` (2 preceding siblings ...)
2019-03-07 0:13 ` Laurent Pinchart
@ 2019-03-08 14:12 ` Hans Verkuil
2019-03-08 16:11 ` Niklas Söderlund
3 siblings, 1 reply; 12+ messages in thread
From: Hans Verkuil @ 2019-03-08 14:12 UTC (permalink / raw)
To: Niklas Söderlund, Laurent Pinchart, linux-media; +Cc: linux-renesas-soc
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-cisco@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).
Regards,
Hans
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@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,
> };
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] rcar-csi2: Allow configuring of video standard
2019-03-08 14:12 ` Hans Verkuil
@ 2019-03-08 16:11 ` Niklas Söderlund
0 siblings, 0 replies; 12+ messages in thread
From: Niklas Söderlund @ 2019-03-08 16:11 UTC (permalink / raw)
To: Hans Verkuil; +Cc: Laurent Pinchart, linux-media, linux-renesas-soc
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-cisco@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+renesas@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
^ permalink raw reply [flat|nested] 12+ messages in thread