[Patch 0/2] media: ti-vpe: allow user specified stride

[Patch 0/2] media: ti-vpe: allow user specified stride

[Benoit Parrot]

This patch series enables user specified buffer stride to be used
instead of always forcing the stride from the driver side.

Benoit Parrot (2):
  media: ti-vpe: vpdma: add support for user specified stride
  media: ti-vpe: vpe: allow use of user specified stride

 drivers/media/platform/ti-vpe/vpdma.c | 14 ++++----------
 drivers/media/platform/ti-vpe/vpdma.h |  6 +++---
 drivers/media/platform/ti-vpe/vpe.c   | 34 ++++++++++++++++++++++++----------
 3 files changed, 31 insertions(+), 23 deletions(-)

-- 
2.9.0

[Patch 1/2] media: ti-vpe: vpdma: add support for user specified stride

[Benoit Parrot]

This patch introduce the needed vpdma API changes to support
user space specified stride instead of forcing a driver calculated
one.

Signed-off-by: Benoit Parrot <bparrot@ti.com>
---
 drivers/media/platform/ti-vpe/vpdma.c | 14 ++++----------
 drivers/media/platform/ti-vpe/vpdma.h |  6 +++---
 drivers/media/platform/ti-vpe/vpe.c   |  6 ++++--
 3 files changed, 11 insertions(+), 15 deletions(-)

diff --git a/drivers/media/platform/ti-vpe/vpdma.c b/drivers/media/platform/ti-vpe/vpdma.c
index 23472e3784ff..e2cf2b90e500 100644
--- a/drivers/media/platform/ti-vpe/vpdma.c
+++ b/drivers/media/platform/ti-vpe/vpdma.c
@@ -801,17 +801,17 @@ static void dump_dtd(struct vpdma_dtd *dtd)
  * flags: VPDMA flags to configure some descriptor fileds
  */
 void vpdma_add_out_dtd(struct vpdma_desc_list *list, int width,
-		const struct v4l2_rect *c_rect,
+		int stride, const struct v4l2_rect *c_rect,
 		const struct vpdma_data_format *fmt, dma_addr_t dma_addr,
 		int max_w, int max_h, enum vpdma_channel chan, u32 flags)
 {
-	vpdma_rawchan_add_out_dtd(list, width, c_rect, fmt, dma_addr,
+	vpdma_rawchan_add_out_dtd(list, width, stride, c_rect, fmt, dma_addr,
 				  max_w, max_h, chan_info[chan].num, flags);
 }
 EXPORT_SYMBOL(vpdma_add_out_dtd);
 
 void vpdma_rawchan_add_out_dtd(struct vpdma_desc_list *list, int width,
-		const struct v4l2_rect *c_rect,
+		int stride, const struct v4l2_rect *c_rect,
 		const struct vpdma_data_format *fmt, dma_addr_t dma_addr,
 		int max_w, int max_h, int raw_vpdma_chan, u32 flags)
 {
@@ -821,7 +821,6 @@ void vpdma_rawchan_add_out_dtd(struct vpdma_desc_list *list, int width,
 	int channel, next_chan;
 	struct v4l2_rect rect = *c_rect;
 	int depth = fmt->depth;
-	int stride;
 	struct vpdma_dtd *dtd;
 
 	channel = next_chan = raw_vpdma_chan;
@@ -833,8 +832,6 @@ void vpdma_rawchan_add_out_dtd(struct vpdma_desc_list *list, int width,
 		depth = 8;
 	}
 
-	stride = ALIGN((depth * width) >> 3, VPDMA_STRIDE_ALIGN);
-
 	dma_addr += rect.top * stride + (rect.left * depth >> 3);
 
 	dtd = list->next;
@@ -882,7 +879,7 @@ EXPORT_SYMBOL(vpdma_rawchan_add_out_dtd);
  *			contribute to the client)
  */
 void vpdma_add_in_dtd(struct vpdma_desc_list *list, int width,
-		const struct v4l2_rect *c_rect,
+		int stride, const struct v4l2_rect *c_rect,
 		const struct vpdma_data_format *fmt, dma_addr_t dma_addr,
 		enum vpdma_channel chan, int field, u32 flags, int frame_width,
 		int frame_height, int start_h, int start_v)
@@ -892,7 +889,6 @@ void vpdma_add_in_dtd(struct vpdma_desc_list *list, int width,
 	int depth = fmt->depth;
 	int channel, next_chan;
 	struct v4l2_rect rect = *c_rect;
-	int stride;
 	struct vpdma_dtd *dtd;
 
 	channel = next_chan = chan_info[chan].num;
@@ -904,8 +900,6 @@ void vpdma_add_in_dtd(struct vpdma_desc_list *list, int width,
 		depth = 8;
 	}
 
-	stride = ALIGN((depth * width) >> 3, VPDMA_STRIDE_ALIGN);
-
 	dma_addr += rect.top * stride + (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 131700c112b2..7e611501c291 100644
--- a/drivers/media/platform/ti-vpe/vpdma.h
+++ b/drivers/media/platform/ti-vpe/vpdma.h
@@ -242,16 +242,16 @@ void vpdma_add_sync_on_channel_ctd(struct vpdma_desc_list *list,
 void vpdma_add_abort_channel_ctd(struct vpdma_desc_list *list,
 		int chan_num);
 void vpdma_add_out_dtd(struct vpdma_desc_list *list, int width,
-		const struct v4l2_rect *c_rect,
+		int stride, const struct v4l2_rect *c_rect,
 		const struct vpdma_data_format *fmt, dma_addr_t dma_addr,
 		int max_w, int max_h, enum vpdma_channel chan, u32 flags);
 void vpdma_rawchan_add_out_dtd(struct vpdma_desc_list *list, int width,
-		const struct v4l2_rect *c_rect,
+		int stride, const struct v4l2_rect *c_rect,
 		const struct vpdma_data_format *fmt, dma_addr_t dma_addr,
 		int max_w, int max_h, int raw_vpdma_chan, u32 flags);
 
 void vpdma_add_in_dtd(struct vpdma_desc_list *list, int width,
-		const struct v4l2_rect *c_rect,
+		int stride, const struct v4l2_rect *c_rect,
 		const struct vpdma_data_format *fmt, dma_addr_t dma_addr,
 		enum vpdma_channel chan, int field, u32 flags, int frame_width,
 		int frame_height, int start_h, int start_v);
diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c
index f0156b7759e9..2dd67232b3bc 100644
--- a/drivers/media/platform/ti-vpe/vpe.c
+++ b/drivers/media/platform/ti-vpe/vpe.c
@@ -1085,7 +1085,8 @@ static void add_out_dtd(struct vpe_ctx *ctx, int port)
 	vpdma_set_max_size(ctx->dev->vpdma, VPDMA_MAX_SIZE1,
 			   MAX_W, MAX_H);
 
-	vpdma_add_out_dtd(&ctx->desc_list, q_data->width, &q_data->c_rect,
+	vpdma_add_out_dtd(&ctx->desc_list, q_data->width,
+			  q_data->bytesperline[VPE_LUMA], &q_data->c_rect,
 			  vpdma_fmt, dma_addr, MAX_OUT_WIDTH_REG1,
 			  MAX_OUT_HEIGHT_REG1, p_data->channel, flags);
 }
@@ -1169,7 +1170,8 @@ static void add_in_dtd(struct vpe_ctx *ctx, int port)
 	if (p_data->vb_part && fmt->fourcc == V4L2_PIX_FMT_NV12)
 		frame_height /= 2;
 
-	vpdma_add_in_dtd(&ctx->desc_list, q_data->width, &q_data->c_rect,
+	vpdma_add_in_dtd(&ctx->desc_list, q_data->width,
+			 q_data->bytesperline[VPE_LUMA], &q_data->c_rect,
 		vpdma_fmt, dma_addr, p_data->channel, field, flags, frame_width,
 		frame_height, 0, 0);
 }
-- 
2.9.0

[Patch 2/2] media: ti-vpe: vpe: allow use of user specified stride

[Benoit Parrot]

Bytesperline/stride was always overwritten by VPE to the most
adequate value based on needed alignment.

However in order to make use of arbitrary size DMA buffer it
is better to use the user space provide stride instead.

The driver will still calculate an appropriate stride but will
use the provided one when it is larger than the calculated one.

Signed-off-by: Benoit Parrot <bparrot@ti.com>
---
 drivers/media/platform/ti-vpe/vpe.c | 28 ++++++++++++++++++++--------
 1 file changed, 20 insertions(+), 8 deletions(-)

diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c
index 2dd67232b3bc..c47151495b6f 100644
--- a/drivers/media/platform/ti-vpe/vpe.c
+++ b/drivers/media/platform/ti-vpe/vpe.c
@@ -1597,6 +1597,7 @@ static int __vpe_try_fmt(struct vpe_ctx *ctx, struct v4l2_format *f,
 	struct v4l2_plane_pix_format *plane_fmt;
 	unsigned int w_align;
 	int i, depth, depth_bytes, height;
+	unsigned int stride = 0;
 
 	if (!fmt || !(fmt->types & type)) {
 		vpe_err(ctx->dev, "Fourcc format (0x%08x) invalid.\n",
@@ -1683,16 +1684,27 @@ static int __vpe_try_fmt(struct vpe_ctx *ctx, struct v4l2_format *f,
 		plane_fmt = &pix->plane_fmt[i];
 		depth = fmt->vpdma_fmt[i]->depth;
 
-		if (i == VPE_LUMA)
-			plane_fmt->bytesperline = (pix->width * depth) >> 3;
-		else
-			plane_fmt->bytesperline = pix->width;
+		stride = (pix->width * fmt->vpdma_fmt[VPE_LUMA]->depth) >> 3;
+		if (stride > plane_fmt->bytesperline)
+			plane_fmt->bytesperline = stride;
+
+		plane_fmt->bytesperline = ALIGN(plane_fmt->bytesperline,
+						VPDMA_STRIDE_ALIGN);
 
-		if (pix->num_planes == 1 && fmt->coplanar)
-			depth += fmt->vpdma_fmt[VPE_CHROMA]->depth;
-		plane_fmt->sizeimage =
-				(pix->height * pix->width * depth) >> 3;
+		if (i == VPE_LUMA) {
+			plane_fmt->sizeimage = pix->height *
+					       plane_fmt->bytesperline;
 
+			if (pix->num_planes == 1 && fmt->coplanar)
+				plane_fmt->sizeimage += pix->height *
+					plane_fmt->bytesperline *
+					fmt->vpdma_fmt[VPE_CHROMA]->depth >> 3;
+
+		} else { /* i == VIP_CHROMA */
+			plane_fmt->sizeimage = (pix->height *
+					       plane_fmt->bytesperline *
+					       depth) >> 3;
+		}
 		memset(plane_fmt->reserved, 0, sizeof(plane_fmt->reserved));
 	}
 
-- 
2.9.0

Re: [Patch 2/2] media: ti-vpe: vpe: allow use of user specified stride

[Tomi Valkeinen]

[-- Attachment #1.1: Type: text/plain, Size: 1797 bytes --]

Hi,

On 13/02/17 15:06, Benoit Parrot wrote:
> Bytesperline/stride was always overwritten by VPE to the most
> adequate value based on needed alignment.
> 
> However in order to make use of arbitrary size DMA buffer it
> is better to use the user space provide stride instead.
> 
> The driver will still calculate an appropriate stride but will
> use the provided one when it is larger than the calculated one.
> 
> Signed-off-by: Benoit Parrot <bparrot@ti.com>
> ---
>  drivers/media/platform/ti-vpe/vpe.c | 28 ++++++++++++++++++++--------
>  1 file changed, 20 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c
> index 2dd67232b3bc..c47151495b6f 100644
> --- a/drivers/media/platform/ti-vpe/vpe.c
> +++ b/drivers/media/platform/ti-vpe/vpe.c
> @@ -1597,6 +1597,7 @@ static int __vpe_try_fmt(struct vpe_ctx *ctx, struct v4l2_format *f,
>  	struct v4l2_plane_pix_format *plane_fmt;
>  	unsigned int w_align;
>  	int i, depth, depth_bytes, height;
> +	unsigned int stride = 0;
>  
>  	if (!fmt || !(fmt->types & type)) {
>  		vpe_err(ctx->dev, "Fourcc format (0x%08x) invalid.\n",
> @@ -1683,16 +1684,27 @@ static int __vpe_try_fmt(struct vpe_ctx *ctx, struct v4l2_format *f,
>  		plane_fmt = &pix->plane_fmt[i];
>  		depth = fmt->vpdma_fmt[i]->depth;
>  
> -		if (i == VPE_LUMA)
> -			plane_fmt->bytesperline = (pix->width * depth) >> 3;
> -		else
> -			plane_fmt->bytesperline = pix->width;
> +		stride = (pix->width * fmt->vpdma_fmt[VPE_LUMA]->depth) >> 3;
> +		if (stride > plane_fmt->bytesperline)
> +			plane_fmt->bytesperline = stride;

The old code calculates different bytes per line for luma and chroma,
but the new one calculates only for luma. Is that correct?

 Tomi


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Patch 2/2] media: ti-vpe: vpe: allow use of user specified stride

[Benoit Parrot]

Tomi Valkeinen <tomi.valkeinen@ti.com> wrote on Wed [2017-Feb-15 13:22:42 +0200]:
> Hi,
> 
> On 13/02/17 15:06, Benoit Parrot wrote:
> > Bytesperline/stride was always overwritten by VPE to the most
> > adequate value based on needed alignment.
> > 
> > However in order to make use of arbitrary size DMA buffer it
> > is better to use the user space provide stride instead.
> > 
> > The driver will still calculate an appropriate stride but will
> > use the provided one when it is larger than the calculated one.
> > 
> > Signed-off-by: Benoit Parrot <bparrot@ti.com>
> > ---
> >  drivers/media/platform/ti-vpe/vpe.c | 28 ++++++++++++++++++++--------
> >  1 file changed, 20 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c
> > index 2dd67232b3bc..c47151495b6f 100644
> > --- a/drivers/media/platform/ti-vpe/vpe.c
> > +++ b/drivers/media/platform/ti-vpe/vpe.c
> > @@ -1597,6 +1597,7 @@ static int __vpe_try_fmt(struct vpe_ctx *ctx, struct v4l2_format *f,
> >  	struct v4l2_plane_pix_format *plane_fmt;
> >  	unsigned int w_align;
> >  	int i, depth, depth_bytes, height;
> > +	unsigned int stride = 0;
> >  
> >  	if (!fmt || !(fmt->types & type)) {
> >  		vpe_err(ctx->dev, "Fourcc format (0x%08x) invalid.\n",
> > @@ -1683,16 +1684,27 @@ static int __vpe_try_fmt(struct vpe_ctx *ctx, struct v4l2_format *f,
> >  		plane_fmt = &pix->plane_fmt[i];
> >  		depth = fmt->vpdma_fmt[i]->depth;
> >  
> > -		if (i == VPE_LUMA)
> > -			plane_fmt->bytesperline = (pix->width * depth) >> 3;
> > -		else
> > -			plane_fmt->bytesperline = pix->width;
> > +		stride = (pix->width * fmt->vpdma_fmt[VPE_LUMA]->depth) >> 3;
> > +		if (stride > plane_fmt->bytesperline)
> > +			plane_fmt->bytesperline = stride;
> 
> The old code calculates different bytes per line for luma and chroma,
> but the new one calculates only for luma. Is that correct?

The previous method which happened to produce the correct value was
not proper as the spec for NV12/NV16 states that the chroma bytes per
line/stride should be the same as the LUMA stride only the number of
lines might differ which affect how the sizeimage component is
calculated. This patch takes that into account.

Benoit

> 
>  Tomi
> 

Re: [Patch 0/2] media: ti-vpe: allow user specified stride

[Tomi Valkeinen]

[-- Attachment #1.1: Type: text/plain, Size: 646 bytes --]

On 13/02/17 15:06, Benoit Parrot wrote:
> This patch series enables user specified buffer stride to be used
> instead of always forcing the stride from the driver side.
> 
> Benoit Parrot (2):
>   media: ti-vpe: vpdma: add support for user specified stride
>   media: ti-vpe: vpe: allow use of user specified stride
> 
>  drivers/media/platform/ti-vpe/vpdma.c | 14 ++++----------
>  drivers/media/platform/ti-vpe/vpdma.h |  6 +++---
>  drivers/media/platform/ti-vpe/vpe.c   | 34 ++++++++++++++++++++++++----------
>  3 files changed, 31 insertions(+), 23 deletions(-)

Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ti.com>

 Tomi


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Patch 0/2] media: ti-vpe: allow user specified stride

[Benoit Parrot]

Hans, Mauro,

Ping.

Regards,
Benoit

Tomi Valkeinen <tomi.valkeinen@ti.com> wrote on Fri [2017-Feb-17 11:45:41 +0200]:
> On 13/02/17 15:06, Benoit Parrot wrote:
> > This patch series enables user specified buffer stride to be used
> > instead of always forcing the stride from the driver side.
> > 
> > Benoit Parrot (2):
> >   media: ti-vpe: vpdma: add support for user specified stride
> >   media: ti-vpe: vpe: allow use of user specified stride
> > 
> >  drivers/media/platform/ti-vpe/vpdma.c | 14 ++++----------
> >  drivers/media/platform/ti-vpe/vpdma.h |  6 +++---
> >  drivers/media/platform/ti-vpe/vpe.c   | 34 ++++++++++++++++++++++++----------
> >  3 files changed, 31 insertions(+), 23 deletions(-)
> 
> Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> 
>  Tomi
>