> -----Original Message-----
> From: Hans Verkuil [mailto:hverk...@xs4all.nl]
> Sent: Monday, December 01, 2014 4:22 PM
> To: Devshatwar, Nikhil; linux-media@vger.kernel.org
> Subject: Re: [PATCH v3 4/4] media: ti-vpe: Add support for SEQ_TB
> buffers
> 
> 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...@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.
> 
OK no problem
I think that should be easy to add
I will add this support and repost the patchset
> 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;
> >             }
> >     }
> >

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to