Hi Niklas,
On Fri, Apr 12, 2019 at 11:13:21AM +0200, Niklas Söderlund wrote:
> On 2019-04-12 10:43:46 +0200, Jacopo Mondi wrote:
> > Hi Niklas,
> > I'm rebasing v4l2-mux series on top of your new VIN patches and
> > on this on I'm not sure how to proceed. Please see below.
> >
> > Please also note this comment is not strictly on this patch, and should not
> > block its acceptance...
> >
> > On Thu, Apr 11, 2019 at 10:34:44PM +0200, Niklas Söderlund wrote:
> >> Depending on which video standard is used the driver needs to setup the
> >> hardware to correctly handle fields. If stream is identified as NTSC
> >> or PAL setup field detection and propagate the field detection signal.
> >>
> >> Later versions of the datasheet have been updated to make it clear
> >> that FLD register should be set to 0 when dealing with non-interlaced
> >> field formats.
> >>
> >> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@xxxxxxxxxxxx>
> >> ---
> >> drivers/media/platform/rcar-vin/rcar-csi2.c | 16 +++++++++++++---
> >> 1 file changed, 13 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c b/drivers/media/platform/rcar-vin/rcar-csi2.c
> >> index 799e526fd3df5554..c1b38ebd061dbc35 100644
> >> --- a/drivers/media/platform/rcar-vin/rcar-csi2.c
> >> +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
> >> @@ -68,6 +68,7 @@ struct rcar_csi2;
> >> /* Field Detection Control */
> >> #define FLD_REG 0x1c
> >> #define FLD_FLD_NUM(n) (((n) & 0xff) << 16)
> >> +#define FLD_DET_SEL(n) (((n) & 0x3) << 4)
> >> #define FLD_FLD_EN4 BIT(3)
> >> #define FLD_FLD_EN3 BIT(2)
> >> #define FLD_FLD_EN2 BIT(1)
> >> @@ -475,7 +476,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 +508,16 @@ static int rcsi2_start_receiver(struct rcar_csi2 *priv)
> >> vcdt2 |= vcdt_part << ((i % 2) * 16);
> >> }
> >>
> >> + if (priv->mf.field == V4L2_FIELD_ALTERNATE) {
> >
> > How would you handle this once priv->mf is expanded to become an array
> > of 'struct v4l2_mbus_framefmt' entries [1] ? Which of them is relevant for
> > field detection?
> >
> >
> >> + fld = FLD_DET_SEL(1) | FLD_FLD_EN4 | FLD_FLD_EN3 | FLD_FLD_EN2
> >> + | FLD_FLD_EN;
> >> +
> >> + if (priv->mf.height == 240)
> >
> > Same question here, even if I assume we could cycle on the entries and
> > collect the maximum size.
>
> Well as the hardware do not support field detection of PAL and NTSC at
> the same time one would need to either disallow that and fail stream on
> or pick one and move on. Not sure which one would be best.
Can't you do this through the VnMC.FOC bit ? I wonder if we shouldn't
hardcode FLD_NUM here and handle PAL vs. NTSC in the VIN.
> > [1] Introduced by:
> > "rcar-csi2: use frame description information to configure CSI-2 bus"
> >> + fld |= FLD_FLD_NUM(0);
> >> + else
> >> + fld |= FLD_FLD_NUM(1);
> >> + }
> >> +
> >> phycnt = PHYCNT_ENABLECLK;
> >> phycnt |= (1 << priv->lanes) - 1;
> >>
> >> @@ -549,8 +560,7 @@ static int rcsi2_start_receiver(struct rcar_csi2 *priv)
> >> rcsi2_write(priv, PHYCNT_REG, phycnt);
> >> rcsi2_write(priv, LINKCNT_REG, LINKCNT_MONITOR_EN |
> >> LINKCNT_REG_MONI_PACT_EN | LINKCNT_ICLK_NONSTOP);
> >> - 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, PHYCNT_REG, phycnt | PHYCNT_SHUTDOWNZ);
> >> rcsi2_write(priv, PHYCNT_REG, phycnt | PHYCNT_SHUTDOWNZ | PHYCNT_RSTZ);
> >>
--
Regards,
Laurent Pinchart
