Re: [PATCH v3 4/4] media: ti-vpe: Add support for SEQ_TB buffers

[Hans Verkuil <hverkuil@xs4all.nl>]

On 11/29/2014 11:27 AM, Nikhil Devshatwar wrote:
> The video source can generate the data in the SEQ_TB buffer format.
> In the case of TI SoC, the IVA_HD can generate the interlaced content in
> the SEQ_TB buffer format. This is the format where the top and bottom field
> data can be contained in a single buffer. For example, for NV12, interlaced
> format, the data in Y buffer will be arranged as Y-top followed by Y-bottom.
> And likewise for UV plane.
> 
> Also, queueing one buffer of SEQ_TB is euivalent to queueing two different

queueing -> queuing 
euivalent -> equivalent

> buffers for top and bottom fields. Driver needs to take care of this when
> handling source buffer lists.
> 
> Signed-off-by: Nikhil Devshatwar <nikhil.nd@ti.com>

Currently you are supporting SEQ_TB. Should SEQ_BT also be supported? I don't
know if you are aware but NTSC transmits the bottom field before the top field,
while PAL/SECAM transmit top before bottom.

So for NTSC support having SEQ_BT might be useful. Up to you, though.

Regards,

	Hans

> ---
> Changes from v1:
>  * Add check for valid field in qbuf ioctl
>  * Fix issue with swapped fields
> 
>  drivers/media/platform/ti-vpe/vpe.c |  124 ++++++++++++++++++++++++++++-------
>  1 file changed, 102 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c
> index 6a96bbf..b53aea5 100644
> --- a/drivers/media/platform/ti-vpe/vpe.c
> +++ b/drivers/media/platform/ti-vpe/vpe.c
> @@ -319,9 +319,13 @@ struct vpe_q_data {
>  };
>  
>  /* vpe_q_data flag bits */
> -#define	Q_DATA_FRAME_1D		(1 << 0)
> -#define	Q_DATA_MODE_TILED	(1 << 1)
> -#define	Q_DATA_INTERLACED	(1 << 2)
> +#define	Q_DATA_FRAME_1D			(1 << 0)
> +#define	Q_DATA_MODE_TILED		(1 << 1)
> +#define	Q_DATA_INTERLACED_ALTERNATE	(1 << 2)
> +#define	Q_DATA_INTERLACED_SEQ_TB	(1 << 3)
> +
> +#define Q_IS_INTERLACED		(Q_DATA_INTERLACED_ALTERNATE | \
> +				Q_DATA_INTERLACED_SEQ_TB)
>  
>  enum {
>  	Q_DATA_SRC = 0,
> @@ -647,7 +651,7 @@ static void set_us_coefficients(struct vpe_ctx *ctx)
>  
>  	cp = &us_coeffs[0].anchor_fid0_c0;
>  
> -	if (s_q_data->flags & Q_DATA_INTERLACED)	/* interlaced */
> +	if (s_q_data->flags & Q_IS_INTERLACED)		/* interlaced */
>  		cp += sizeof(us_coeffs[0]) / sizeof(*cp);
>  
>  	end_cp = cp + sizeof(us_coeffs[0]) / sizeof(*cp);
> @@ -774,8 +778,7 @@ static void set_dei_regs(struct vpe_ctx *ctx)
>  	 * for both progressive and interlace content in interlace bypass mode.
>  	 * It has been recommended not to use progressive bypass mode.
>  	 */
> -	if ((!ctx->deinterlacing && (s_q_data->flags & Q_DATA_INTERLACED)) ||
> -			!(s_q_data->flags & Q_DATA_INTERLACED)) {
> +	if (!(s_q_data->flags & Q_IS_INTERLACED) || !ctx->deinterlacing) {
>  		deinterlace = false;
>  		val = VPE_DEI_INTERLACE_BYPASS;
>  	}
> @@ -843,8 +846,8 @@ static int set_srcdst_params(struct vpe_ctx *ctx)
>  	ctx->sequence = 0;
>  	ctx->field = V4L2_FIELD_TOP;
>  
> -	if ((s_q_data->flags & Q_DATA_INTERLACED) &&
> -			!(d_q_data->flags & Q_DATA_INTERLACED)) {
> +	if ((s_q_data->flags & Q_IS_INTERLACED) &&
> +			!(d_q_data->flags & Q_IS_INTERLACED)) {
>  		int bytes_per_line;
>  		const struct vpdma_data_format *mv =
>  			&vpdma_misc_fmts[VPDMA_DATA_FMT_MV];
> @@ -1068,6 +1071,27 @@ static void add_in_dtd(struct vpe_ctx *ctx, int port)
>  		vpdma_fmt = fmt->vpdma_fmt[plane];
>  
>  		dma_addr = vb2_dma_addr_plus_data_offset(vb, plane);
> +
> +		if (q_data->flags & Q_DATA_INTERLACED_SEQ_TB) {
> +			/*
> +			 * Use top or bottom field from same vb alternately
> +			 * f,f-1,f-2 = TBT when seq is even
> +			 * f,f-1,f-2 = BTB when seq is odd
> +			 */
> +			field = (p_data->vb_index + (ctx->sequence % 2)) % 2;
> +
> +			if (field) {
> +				/* bottom field of a SEQ_TB buffer
> +				 * Skip the top field data by */
> +				int height = q_data->height / 2;
> +				int bpp = fmt->fourcc == V4L2_PIX_FMT_NV12 ?
> +						1 : (vpdma_fmt->depth >> 3);
> +				if (plane)
> +					height /= 2;
> +				dma_addr += q_data->width * height * bpp;
> +			}
> +		}
> +
>  		if (!dma_addr) {
>  			vpe_err(ctx->dev,
>  				"acquiring input buffer(%d) dma_addr failed\n",
> @@ -1122,9 +1146,22 @@ static void device_run(void *priv)
>  	struct vpe_ctx *ctx = priv;
>  	struct sc_data *sc = ctx->dev->sc;
>  	struct vpe_q_data *d_q_data = &ctx->q_data[Q_DATA_DST];
> +	struct vpe_q_data *s_q_data = &ctx->q_data[Q_DATA_SRC];
> +
> +	if (ctx->deinterlacing && s_q_data->flags & Q_DATA_INTERLACED_SEQ_TB &&
> +		ctx->sequence % 2 == 0) {
> +		/* When using SEQ_TB buffers, When using it first time,
> +		 * No need to remove the buffer as the next field is present
> +		 * in the same buffer. (so that job_ready won't fail)
> +		 * It will be removed when using bottom field
> +		 */
> +		ctx->src_vbs[0] = v4l2_m2m_next_src_buf(ctx->m2m_ctx);
> +		WARN_ON(ctx->src_vbs[0] == NULL);
> +	} else {
> +		ctx->src_vbs[0] = v4l2_m2m_src_buf_remove(ctx->m2m_ctx);
> +		WARN_ON(ctx->src_vbs[0] == NULL);
> +	}
>  
> -	ctx->src_vbs[0] = v4l2_m2m_src_buf_remove(ctx->m2m_ctx);
> -	WARN_ON(ctx->src_vbs[0] == NULL);
>  	ctx->dst_vb = v4l2_m2m_dst_buf_remove(ctx->m2m_ctx);
>  	WARN_ON(ctx->dst_vb == NULL);
>  
> @@ -1256,6 +1293,7 @@ static irqreturn_t vpe_irq(int irq_vpe, void *data)
>  	struct vpe_dev *dev = (struct vpe_dev *)data;
>  	struct vpe_ctx *ctx;
>  	struct vpe_q_data *d_q_data;
> +	struct vpe_q_data *s_q_data;
>  	struct vb2_buffer *s_vb, *d_vb;
>  	struct v4l2_buffer *s_buf, *d_buf;
>  	unsigned long flags;
> @@ -1332,7 +1370,7 @@ static irqreturn_t vpe_irq(int irq_vpe, void *data)
>  	d_buf->sequence = ctx->sequence;
>  
>  	d_q_data = &ctx->q_data[Q_DATA_DST];
> -	if (d_q_data->flags & Q_DATA_INTERLACED) {
> +	if (d_q_data->flags & Q_IS_INTERLACED) {
>  		d_buf->field = ctx->field;
>  		if (ctx->field == V4L2_FIELD_BOTTOM) {
>  			ctx->sequence++;
> @@ -1346,12 +1384,28 @@ static irqreturn_t vpe_irq(int irq_vpe, void *data)
>  		ctx->sequence++;
>  	}
>  
> -	if (ctx->deinterlacing)
> -		s_vb = ctx->src_vbs[2];
> +	s_q_data = &ctx->q_data[Q_DATA_SRC];
> +
> +	if (ctx->deinterlacing) {
> +		/* Allow source buffer to be dequeued only if it won't be used
> +		 * in the next iteration. All vbs are initialized to first
> +		 * buffer and we are shifting buffers every iteration, for the
> +		 * first two iterations, no buffer will be dequeued.
> +		 * This ensures that driver will keep (n-2)th (n-1)th and (n)th
> +		 * field when deinterlacing is enabled */
> +		if (ctx->src_vbs[2] != ctx->src_vbs[1])
> +			s_vb = ctx->src_vbs[2];
> +		else
> +			s_vb = NULL;
> +	}
>  
>  	spin_lock_irqsave(&dev->lock, flags);
> -	v4l2_m2m_buf_done(s_vb, VB2_BUF_STATE_DONE);
> +
> +	if (s_vb)
> +		v4l2_m2m_buf_done(s_vb, VB2_BUF_STATE_DONE);
> +
>  	v4l2_m2m_buf_done(d_vb, VB2_BUF_STATE_DONE);
> +
>  	spin_unlock_irqrestore(&dev->lock, flags);
>  
>  	if (ctx->deinterlacing) {
> @@ -1467,7 +1521,7 @@ static int __vpe_try_fmt(struct vpe_ctx *ctx, struct v4l2_format *f,
>  	struct v4l2_pix_format_mplane *pix = &f->fmt.pix_mp;
>  	struct v4l2_plane_pix_format *plane_fmt;
>  	unsigned int w_align;
> -	int i, depth, depth_bytes;
> +	int i, depth, depth_bytes, height;
>  
>  	if (!fmt || !(fmt->types & type)) {
>  		vpe_err(ctx->dev, "Fourcc format (0x%08x) invalid.\n",
> @@ -1475,7 +1529,8 @@ static int __vpe_try_fmt(struct vpe_ctx *ctx, struct v4l2_format *f,
>  		return -EINVAL;
>  	}
>  
> -	if (pix->field != V4L2_FIELD_NONE && pix->field != V4L2_FIELD_ALTERNATE)
> +	if (pix->field != V4L2_FIELD_NONE && pix->field != V4L2_FIELD_ALTERNATE
> +			&& pix->field != V4L2_FIELD_SEQ_TB)
>  		pix->field = V4L2_FIELD_NONE;
>  
>  	depth = fmt->vpdma_fmt[VPE_LUMA]->depth;
> @@ -1509,6 +1564,14 @@ static int __vpe_try_fmt(struct vpe_ctx *ctx, struct v4l2_format *f,
>  	pix->num_planes = fmt->coplanar ? 2 : 1;
>  	pix->pixelformat = fmt->fourcc;
>  
> +	/* for the actual image parameters, we need to consider the field height
> +	 * of the image for SEQ_TB buffers.
> +	 */
> +	if (pix->field == V4L2_FIELD_SEQ_TB)
> +		height = pix->height / 2;
> +	else
> +		height = pix->height;
> +
>  	if (!pix->colorspace) {
>  		if (fmt->fourcc == V4L2_PIX_FMT_RGB24 ||
>  				fmt->fourcc == V4L2_PIX_FMT_BGR24 ||
> @@ -1516,7 +1579,7 @@ static int __vpe_try_fmt(struct vpe_ctx *ctx, struct v4l2_format *f,
>  				fmt->fourcc == V4L2_PIX_FMT_BGR32) {
>  			pix->colorspace = V4L2_COLORSPACE_SRGB;
>  		} else {
> -			if (pix->height > 1280)	/* HD */
> +			if (height > 1280)	/* HD */
>  				pix->colorspace = V4L2_COLORSPACE_REC709;
>  			else			/* SD */
>  				pix->colorspace = V4L2_COLORSPACE_SMPTE170M;
> @@ -1593,9 +1656,15 @@ static int __vpe_s_fmt(struct vpe_ctx *ctx, struct v4l2_format *f)
>  	q_data->c_rect.height	= q_data->height;
>  
>  	if (q_data->field == V4L2_FIELD_ALTERNATE)
> -		q_data->flags |= Q_DATA_INTERLACED;
> +		q_data->flags |= Q_DATA_INTERLACED_ALTERNATE;
> +	else if (q_data->field == V4L2_FIELD_SEQ_TB)
> +		q_data->flags |= Q_DATA_INTERLACED_SEQ_TB;
>  	else
> -		q_data->flags &= ~Q_DATA_INTERLACED;
> +		q_data->flags &= ~Q_IS_INTERLACED;
> +
> +	/* the crop height is halved for the case of SEQ_TB buffers */
> +	if (q_data->flags & Q_DATA_INTERLACED_SEQ_TB)
> +		q_data->c_rect.height /= 2;
>  
>  	vpe_dbg(ctx->dev, "Setting format for type %d, wxh: %dx%d, fmt: %d bpl_y %d",
>  		f->type, q_data->width, q_data->height, q_data->fmt->fourcc,
> @@ -1631,6 +1700,7 @@ static int vpe_s_fmt(struct file *file, void *priv, struct v4l2_format *f)
>  static int __vpe_try_selection(struct vpe_ctx *ctx, struct v4l2_selection *s)
>  {
>  	struct vpe_q_data *q_data;
> +	int height;
>  
>  	if ((s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE) &&
>  	    (s->type != V4L2_BUF_TYPE_VIDEO_OUTPUT))
> @@ -1665,13 +1735,22 @@ static int __vpe_try_selection(struct vpe_ctx *ctx, struct v4l2_selection *s)
>  		return -EINVAL;
>  	}
>  
> +	/*
> +	* For SEQ_TB buffers, crop height should be less than the height of
> +	* the field height, not the buffer height
> +	*/
> +	if (q_data->flags & Q_DATA_INTERLACED_SEQ_TB)
> +		height = q_data->height / 2;
> +	else
> +		height = q_data->height;
> +
>  	if (s->r.top < 0 || s->r.left < 0) {
>  		vpe_err(ctx->dev, "negative values for top and left\n");
>  		s->r.top = s->r.left = 0;
>  	}
>  
>  	v4l_bound_align_image(&s->r.width, MIN_W, q_data->width, 1,
> -		&s->r.height, MIN_H, q_data->height, H_ALIGN, S_ALIGN);
> +		&s->r.height, MIN_H, height, H_ALIGN, S_ALIGN);
>  
>  	/* adjust left/top if cropping rectangle is out of bounds */
>  	if (s->r.left + s->r.width > q_data->width)
> @@ -1919,11 +1998,12 @@ static int vpe_buf_prepare(struct vb2_buffer *vb)
>  	num_planes = q_data->fmt->coplanar ? 2 : 1;
>  
>  	if (vb->vb2_queue->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) {
> -		if (!(q_data->flags & Q_DATA_INTERLACED)) {
> +		if (!(q_data->flags & Q_IS_INTERLACED)) {
>  			vb->v4l2_buf.field = V4L2_FIELD_NONE;
>  		} else {
>  			if (vb->v4l2_buf.field != V4L2_FIELD_TOP &&
> -					vb->v4l2_buf.field != V4L2_FIELD_BOTTOM)
> +			vb->v4l2_buf.field != V4L2_FIELD_BOTTOM &&
> +			vb->v4l2_buf.field != V4L2_FIELD_SEQ_TB)
>  				return -EINVAL;
>  		}
>  	}
>