Re: [PATCH v4] rcar-csi2: Propagate the FLD signal for NTSC and PAL

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Laurent,

On 2019-04-12 12:47:22 +0300, Laurent Pinchart wrote:
> 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.

I don't think so. My interpretation is that VnMC.FOC controls how the 
fields are combined when using the VIN mode of interlacing ALTERNETING 
and providing a INTERLACED frame to user-space.

My interpretation is that the change in this patch is visible VnINTS.FIS 
which can be used to finaly add support for delivering ALTERNATE 
directly to user-space as well as doing the right thing when interlacing 
with the VnMC.FOC setting.

> 
> > > [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

-- 
Regards,
Niklas Söderlund



[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux