[PATCH 0/2] v4l: ti-vpe: Some VPE fixes

[PATCH 0/2] v4l: ti-vpe: Some VPE fixes

[Archit Taneja]

This series fixes 2 issues in the VPE driver. The first fix allows us to use
UYVY color format for source and destination buffers. The second fix makes sure
we don't set pixel format widths which the VPDMA HW can't support. None of these
fixes are fatal, so they don't necessarily need to go in for the 3.13-rc fixes.

Archit Taneja (2):
  v4l: ti-vpe: Fix the data_type value for UYVY VPDMA format
  v4l: ti-vpe: make sure VPDMA line stride constraints are met

 drivers/media/platform/ti-vpe/vpdma.c      |  4 +--
 drivers/media/platform/ti-vpe/vpdma.h      |  5 ++-
 drivers/media/platform/ti-vpe/vpdma_priv.h |  2 +-
 drivers/media/platform/ti-vpe/vpe.c        | 53 ++++++++++++++++++++++--------
 4 files changed, 47 insertions(+), 17 deletions(-)

-- 
1.8.3.2

[PATCH 1/2] v4l: ti-vpe: Fix the data_type value for UYVY VPDMA format

[Archit Taneja]

The data_type value to be programmed in the data descriptors to fetch/write a
UYVY buffer was not mentioned correctly in the older DRA7x documentation. This
caused VPE to fail with UYVY color formats.

Update the data_type value to fix functionality when UYVY format is used.

Signed-off-by: Archit Taneja <archit@ti.com>
---
 drivers/media/platform/ti-vpe/vpdma_priv.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/ti-vpe/vpdma_priv.h b/drivers/media/platform/ti-vpe/vpdma_priv.h
index f0e9a80..c1a6ce1 100644
--- a/drivers/media/platform/ti-vpe/vpdma_priv.h
+++ b/drivers/media/platform/ti-vpe/vpdma_priv.h
@@ -78,7 +78,7 @@
 #define DATA_TYPE_C420				0x6
 #define DATA_TYPE_YC422				0x7
 #define DATA_TYPE_YC444				0x8
-#define DATA_TYPE_CY422				0x23
+#define DATA_TYPE_CY422				0x27
 
 #define DATA_TYPE_RGB16_565			0x0
 #define DATA_TYPE_ARGB_1555			0x1
-- 
1.8.3.2

[PATCH 2/2] v4l: ti-vpe: make sure VPDMA line stride constraints are met

[Archit Taneja]

When VPDMA fetches or writes to an image buffer, the line stride must be a
multiple of 16 bytes. If it isn't, VPDMA HW will write/fetch until the next
16 byte boundry. This causes VPE to work incorrectly for source or destination
widths which don't satisfy the above alignment requirement.

In order to prevent this, we now make sure that when we set pix format for the
input and output buffers, the VPE source and destination image line strides are
16 byte aligned. Also, the motion vector buffers for the de-interlacer are
allocated in such a way that it ensures the same alignment.

Signed-off-by: Archit Taneja <archit@ti.com>
---
 drivers/media/platform/ti-vpe/vpdma.c |  4 +--
 drivers/media/platform/ti-vpe/vpdma.h |  5 +++-
 drivers/media/platform/ti-vpe/vpe.c   | 53 ++++++++++++++++++++++++++---------
 3 files changed, 46 insertions(+), 16 deletions(-)

diff --git a/drivers/media/platform/ti-vpe/vpdma.c b/drivers/media/platform/ti-vpe/vpdma.c
index af0a5ff..f97253f 100644
--- a/drivers/media/platform/ti-vpe/vpdma.c
+++ b/drivers/media/platform/ti-vpe/vpdma.c
@@ -602,7 +602,7 @@ void vpdma_add_out_dtd(struct vpdma_desc_list *list, struct v4l2_rect *c_rect,
 	if (fmt->data_type == DATA_TYPE_C420)
 		depth = 8;
 
-	stride = (depth * c_rect->width) >> 3;
+	stride = ALIGN((depth * c_rect->width) >> 3, VPDMA_STRIDE_ALIGN);
 	dma_addr += (c_rect->left * depth) >> 3;
 
 	dtd = list->next;
@@ -655,7 +655,7 @@ void vpdma_add_in_dtd(struct vpdma_desc_list *list, int frame_width,
 		depth = 8;
 	}
 
-	stride = (depth * c_rect->width) >> 3;
+	stride = ALIGN((depth * c_rect->width) >> 3, VPDMA_STRIDE_ALIGN);
 	dma_addr += (c_rect->left * depth) >> 3;
 
 	dtd = list->next;
diff --git a/drivers/media/platform/ti-vpe/vpdma.h b/drivers/media/platform/ti-vpe/vpdma.h
index eaa2a71..62dd143 100644
--- a/drivers/media/platform/ti-vpe/vpdma.h
+++ b/drivers/media/platform/ti-vpe/vpdma.h
@@ -45,7 +45,10 @@ struct vpdma_data_format {
 };
 
 #define VPDMA_DESC_ALIGN		16	/* 16-byte descriptor alignment */
-
+#define VPDMA_STRIDE_ALIGN		16	/*
+						 * line stride of source and dest
+						 * buffers should be 16 byte aligned
+						 */
 #define VPDMA_DTD_DESC_SIZE		32	/* 8 words */
 #define VPDMA_CFD_CTD_DESC_SIZE		16	/* 4 words */
 
diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c
index 4e58069..a5f7a35 100644
--- a/drivers/media/platform/ti-vpe/vpe.c
+++ b/drivers/media/platform/ti-vpe/vpe.c
@@ -30,6 +30,7 @@
 #include <linux/sched.h>
 #include <linux/slab.h>
 #include <linux/videodev2.h>
+#include <linux/log2.h>
 
 #include <media/v4l2-common.h>
 #include <media/v4l2-ctrls.h>
@@ -54,10 +55,6 @@
 /* required alignments */
 #define S_ALIGN		0	/* multiple of 1 */
 #define H_ALIGN		1	/* multiple of 2 */
-#define W_ALIGN		1	/* multiple of 2 */
-
-/* multiple of 128 bits, line stride, 16 bytes */
-#define L_ALIGN		4
 
 /* flags that indicate a format can be used for capture/output */
 #define VPE_FMT_TYPE_CAPTURE	(1 << 0)
@@ -780,12 +777,21 @@ static int set_srcdst_params(struct vpe_ctx *ctx)
 
 	if ((s_q_data->flags & Q_DATA_INTERLACED) &&
 			!(d_q_data->flags & Q_DATA_INTERLACED)) {
+		int bytes_per_line;
 		const struct vpdma_data_format *mv =
 			&vpdma_misc_fmts[VPDMA_DATA_FMT_MV];
 
 		ctx->deinterlacing = 1;
-		mv_buf_size =
-			(s_q_data->width * s_q_data->height * mv->depth) >> 3;
+		/*
+		 * we make sure that the source image has a 16 byte aligned
+		 * stride, we need to do the same for the motion vector buffer
+		 * by aligning it's stride to the next 16 byte boundry. this
+		 * extra space will not be used by the de-interlacer, but will
+		 * ensure that vpdma operates correctly
+		 */
+		bytes_per_line = ALIGN((s_q_data->width * mv->depth) >> 3,
+					VPDMA_STRIDE_ALIGN);
+		mv_buf_size = bytes_per_line * s_q_data->height;
 	} else {
 		ctx->deinterlacing = 0;
 		mv_buf_size = 0;
@@ -1352,7 +1358,8 @@ 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;
-	int i;
+	unsigned int w_align;
+	int i, depth, depth_bytes;
 
 	if (!fmt || !(fmt->types & type)) {
 		vpe_err(ctx->dev, "Fourcc format (0x%08x) invalid.\n",
@@ -1363,7 +1370,31 @@ static int __vpe_try_fmt(struct vpe_ctx *ctx, struct v4l2_format *f,
 	if (pix->field != V4L2_FIELD_NONE && pix->field != V4L2_FIELD_ALTERNATE)
 		pix->field = V4L2_FIELD_NONE;
 
-	v4l_bound_align_image(&pix->width, MIN_W, MAX_W, W_ALIGN,
+	depth = fmt->vpdma_fmt[VPE_LUMA]->depth;
+
+	/*
+	 * the line stride should 16 byte aligned for VPDMA to work, based on
+	 * the bytes per pixel, figure out how much the width should be aligned
+	 * to make sure line stride is 16 byte aligned
+	 */
+	depth_bytes = depth >> 3;
+
+	if (depth_bytes == 3)
+		/*
+		 * if bpp is 3(as in some RGB formats), the pixel width doesn't
+		 * really help in ensuring line stride is 16 byte aligned
+		 */
+		w_align = 4;
+	else
+		/*
+		 * for the remainder bpp(4, 2 and 1), the pixel width alignment
+		 * can ensure a line stride alignment of 16 bytes. For example,
+		 * if bpp is 2, then the line stride can be 16 byte aligned if
+		 * the width is 8 byte aligned
+		 */
+		w_align = order_base_2(VPDMA_DESC_ALIGN / depth_bytes);
+
+	v4l_bound_align_image(&pix->width, MIN_W, MAX_W, w_align,
 			      &pix->height, MIN_H, MAX_H, H_ALIGN,
 			      S_ALIGN);
 
@@ -1383,15 +1414,11 @@ static int __vpe_try_fmt(struct vpe_ctx *ctx, struct v4l2_format *f,
 	}
 
 	for (i = 0; i < pix->num_planes; i++) {
-		int depth;
-
 		plane_fmt = &pix->plane_fmt[i];
 		depth = fmt->vpdma_fmt[i]->depth;
 
 		if (i == VPE_LUMA)
-			plane_fmt->bytesperline =
-					round_up((pix->width * depth) >> 3,
-						1 << L_ALIGN);
+			plane_fmt->bytesperline = (pix->width * depth) >> 3;
 		else
 			plane_fmt->bytesperline = pix->width;
 
-- 
1.8.3.2

Re: [PATCH 0/2] v4l: ti-vpe: Some VPE fixes

[Hans Verkuil]

On 12/03/2013 12:51 PM, Archit Taneja wrote:
> This series fixes 2 issues in the VPE driver. The first fix allows us to use
> UYVY color format for source and destination buffers. The second fix makes sure
> we don't set pixel format widths which the VPDMA HW can't support. None of these
> fixes are fatal, so they don't necessarily need to go in for the 3.13-rc fixes.
> 
> Archit Taneja (2):
>   v4l: ti-vpe: Fix the data_type value for UYVY VPDMA format
>   v4l: ti-vpe: make sure VPDMA line stride constraints are met
> 
>  drivers/media/platform/ti-vpe/vpdma.c      |  4 +--
>  drivers/media/platform/ti-vpe/vpdma.h      |  5 ++-
>  drivers/media/platform/ti-vpe/vpdma_priv.h |  2 +-
>  drivers/media/platform/ti-vpe/vpe.c        | 53 ++++++++++++++++++++++--------
>  4 files changed, 47 insertions(+), 17 deletions(-)
> 

For this patch series:

Acked-by: Hans Verkuil <hans.verkuil@cisco.com>

Regards,

	Hans

Re: [PATCH 0/2] v4l: ti-vpe: Some VPE fixes

[Hans Verkuil]

On 12/17/2013 08:52 AM, Hans Verkuil wrote:
> On 12/03/2013 12:51 PM, Archit Taneja wrote:
>> This series fixes 2 issues in the VPE driver. The first fix allows us to use
>> UYVY color format for source and destination buffers. The second fix makes sure
>> we don't set pixel format widths which the VPDMA HW can't support. None of these
>> fixes are fatal, so they don't necessarily need to go in for the 3.13-rc fixes.
>>
>> Archit Taneja (2):
>>   v4l: ti-vpe: Fix the data_type value for UYVY VPDMA format
>>   v4l: ti-vpe: make sure VPDMA line stride constraints are met
>>
>>  drivers/media/platform/ti-vpe/vpdma.c      |  4 +--
>>  drivers/media/platform/ti-vpe/vpdma.h      |  5 ++-
>>  drivers/media/platform/ti-vpe/vpdma_priv.h |  2 +-
>>  drivers/media/platform/ti-vpe/vpe.c        | 53 ++++++++++++++++++++++--------
>>  4 files changed, 47 insertions(+), 17 deletions(-)
>>
> 
> For this patch series:
> 
> Acked-by: Hans Verkuil <hans.verkuil@cisco.com>

Ah, it's already merged. I missed that :-)

Regards,

	Hans