dmaengine Archive on lore.kernel.org
 help / color / Atom feed
* [PATCHv2] omap-dma/omap_vout_vrfb: fix off-by-one fi value
@ 2019-08-09  8:32 Hans Verkuil
  2019-08-09  8:39 ` Mauro Carvalho Chehab
  2019-08-09 11:04 ` Vinod Koul
  0 siblings, 2 replies; 3+ messages in thread
From: Hans Verkuil @ 2019-08-09  8:32 UTC (permalink / raw)
  To: Linux Media Mailing List, dmaengine
  Cc: Vinod Koul, Peter Ujfalusi, Laurent Pinchart, Mauro Carvalho Chehab

The OMAP 4 TRM specifies that when using double-index addressing
the address increases by the ES plus the EI value minus 1 within
a frame. When a full frame is transferred, the address increases
by the ES plus the frame index (FI) value minus 1.

The omap-dma code didn't account for the 'minus 1' in the FI register.
To get correct addressing, add 1 to the src_icg value.

This was found when testing a hacked version of the media m2m-deinterlace.c
driver on a Pandaboard.

The only other source that uses this feature is omap_vout_vrfb.c,
and that adds a + 1 when setting the dst_icg. This is a workaround
for the broken omap-dma.c behavior. So remove the workaround at the
same time that we fix omap-dma.c.

I tested the omap_vout driver with a Beagle XM board to check that
the '+ 1' in omap_vout_vrfb.c was indeed a workaround for the omap-dma
bug.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
Changes since v1: removed unnecessary parenthesis in omap_vout_vrfb.c
as suggested by Laurent.

It makes sense that this patch goes in through the dmaengine subsystem
(Mauro, can you Ack this patch?), but if preferred it can also go in
through the media subsystem if we get an Ack from Vinod.
---
 drivers/dma/ti/omap-dma.c                    | 4 ++--
 drivers/media/platform/omap/omap_vout_vrfb.c | 3 +--
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/dma/ti/omap-dma.c b/drivers/dma/ti/omap-dma.c
index ba2489d4ea24..ba27802efcd0 100644
--- a/drivers/dma/ti/omap-dma.c
+++ b/drivers/dma/ti/omap-dma.c
@@ -1234,7 +1234,7 @@ static struct dma_async_tx_descriptor *omap_dma_prep_dma_interleaved(
 	if (src_icg) {
 		d->ccr |= CCR_SRC_AMODE_DBLIDX;
 		d->ei = 1;
-		d->fi = src_icg;
+		d->fi = src_icg + 1;
 	} else if (xt->src_inc) {
 		d->ccr |= CCR_SRC_AMODE_POSTINC;
 		d->fi = 0;
@@ -1249,7 +1249,7 @@ static struct dma_async_tx_descriptor *omap_dma_prep_dma_interleaved(
 	if (dst_icg) {
 		d->ccr |= CCR_DST_AMODE_DBLIDX;
 		sg->ei = 1;
-		sg->fi = dst_icg;
+		sg->fi = dst_icg + 1;
 	} else if (xt->dst_inc) {
 		d->ccr |= CCR_DST_AMODE_POSTINC;
 		sg->fi = 0;
diff --git a/drivers/media/platform/omap/omap_vout_vrfb.c b/drivers/media/platform/omap/omap_vout_vrfb.c
index 29e3f5da59c1..11ec048929e8 100644
--- a/drivers/media/platform/omap/omap_vout_vrfb.c
+++ b/drivers/media/platform/omap/omap_vout_vrfb.c
@@ -253,8 +253,7 @@ int omap_vout_prepare_vrfb(struct omap_vout_device *vout,
 	 */

 	pixsize = vout->bpp * vout->vrfb_bpp;
-	dst_icg = ((MAX_PIXELS_PER_LINE * pixsize) -
-		  (vout->pix.width * vout->bpp)) + 1;
+	dst_icg = MAX_PIXELS_PER_LINE * pixsize - vout->pix.width * vout->bpp;

 	xt->src_start = vout->buf_phy_addr[vb->i];
 	xt->dst_start = vout->vrfb_context[vb->i].paddr[0];
-- 
2.20.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCHv2] omap-dma/omap_vout_vrfb: fix off-by-one fi value
  2019-08-09  8:32 [PATCHv2] omap-dma/omap_vout_vrfb: fix off-by-one fi value Hans Verkuil
@ 2019-08-09  8:39 ` Mauro Carvalho Chehab
  2019-08-09 11:04 ` Vinod Koul
  1 sibling, 0 replies; 3+ messages in thread
From: Mauro Carvalho Chehab @ 2019-08-09  8:39 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Linux Media Mailing List, dmaengine, Vinod Koul, Peter Ujfalusi,
	Laurent Pinchart

Em Fri, 9 Aug 2019 10:32:40 +0200
Hans Verkuil <hverkuil@xs4all.nl> escreveu:

> The OMAP 4 TRM specifies that when using double-index addressing
> the address increases by the ES plus the EI value minus 1 within
> a frame. When a full frame is transferred, the address increases
> by the ES plus the frame index (FI) value minus 1.
> 
> The omap-dma code didn't account for the 'minus 1' in the FI register.
> To get correct addressing, add 1 to the src_icg value.
> 
> This was found when testing a hacked version of the media m2m-deinterlace.c
> driver on a Pandaboard.
> 
> The only other source that uses this feature is omap_vout_vrfb.c,
> and that adds a + 1 when setting the dst_icg. This is a workaround
> for the broken omap-dma.c behavior. So remove the workaround at the
> same time that we fix omap-dma.c.
> 
> I tested the omap_vout driver with a Beagle XM board to check that
> the '+ 1' in omap_vout_vrfb.c was indeed a workaround for the omap-dma
> bug.
> 
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Acked-by: Peter Ujfalusi <peter.ujfalusi@ti.com>

Acked-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>

Regards,
Mauro

> ---
> Changes since v1: removed unnecessary parenthesis in omap_vout_vrfb.c
> as suggested by Laurent.
> 
> It makes sense that this patch goes in through the dmaengine subsystem
> (Mauro, can you Ack this patch?), but if preferred it can also go in
> through the media subsystem if we get an Ack from Vinod.
> ---
>  drivers/dma/ti/omap-dma.c                    | 4 ++--
>  drivers/media/platform/omap/omap_vout_vrfb.c | 3 +--
>  2 files changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/dma/ti/omap-dma.c b/drivers/dma/ti/omap-dma.c
> index ba2489d4ea24..ba27802efcd0 100644
> --- a/drivers/dma/ti/omap-dma.c
> +++ b/drivers/dma/ti/omap-dma.c
> @@ -1234,7 +1234,7 @@ static struct dma_async_tx_descriptor *omap_dma_prep_dma_interleaved(
>  	if (src_icg) {
>  		d->ccr |= CCR_SRC_AMODE_DBLIDX;
>  		d->ei = 1;
> -		d->fi = src_icg;
> +		d->fi = src_icg + 1;
>  	} else if (xt->src_inc) {
>  		d->ccr |= CCR_SRC_AMODE_POSTINC;
>  		d->fi = 0;
> @@ -1249,7 +1249,7 @@ static struct dma_async_tx_descriptor *omap_dma_prep_dma_interleaved(
>  	if (dst_icg) {
>  		d->ccr |= CCR_DST_AMODE_DBLIDX;
>  		sg->ei = 1;
> -		sg->fi = dst_icg;
> +		sg->fi = dst_icg + 1;
>  	} else if (xt->dst_inc) {
>  		d->ccr |= CCR_DST_AMODE_POSTINC;
>  		sg->fi = 0;
> diff --git a/drivers/media/platform/omap/omap_vout_vrfb.c b/drivers/media/platform/omap/omap_vout_vrfb.c
> index 29e3f5da59c1..11ec048929e8 100644
> --- a/drivers/media/platform/omap/omap_vout_vrfb.c
> +++ b/drivers/media/platform/omap/omap_vout_vrfb.c
> @@ -253,8 +253,7 @@ int omap_vout_prepare_vrfb(struct omap_vout_device *vout,
>  	 */
> 
>  	pixsize = vout->bpp * vout->vrfb_bpp;
> -	dst_icg = ((MAX_PIXELS_PER_LINE * pixsize) -
> -		  (vout->pix.width * vout->bpp)) + 1;
> +	dst_icg = MAX_PIXELS_PER_LINE * pixsize - vout->pix.width * vout->bpp;
> 
>  	xt->src_start = vout->buf_phy_addr[vb->i];
>  	xt->dst_start = vout->vrfb_context[vb->i].paddr[0];

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCHv2] omap-dma/omap_vout_vrfb: fix off-by-one fi value
  2019-08-09  8:32 [PATCHv2] omap-dma/omap_vout_vrfb: fix off-by-one fi value Hans Verkuil
  2019-08-09  8:39 ` Mauro Carvalho Chehab
@ 2019-08-09 11:04 ` Vinod Koul
  1 sibling, 0 replies; 3+ messages in thread
From: Vinod Koul @ 2019-08-09 11:04 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Linux Media Mailing List, dmaengine, Peter Ujfalusi,
	Laurent Pinchart, Mauro Carvalho Chehab

On 09-08-19, 10:32, Hans Verkuil wrote:
> The OMAP 4 TRM specifies that when using double-index addressing
> the address increases by the ES plus the EI value minus 1 within
> a frame. When a full frame is transferred, the address increases
> by the ES plus the frame index (FI) value minus 1.
> 
> The omap-dma code didn't account for the 'minus 1' in the FI register.
> To get correct addressing, add 1 to the src_icg value.
> 
> This was found when testing a hacked version of the media m2m-deinterlace.c
> driver on a Pandaboard.
> 
> The only other source that uses this feature is omap_vout_vrfb.c,
> and that adds a + 1 when setting the dst_icg. This is a workaround
> for the broken omap-dma.c behavior. So remove the workaround at the
> same time that we fix omap-dma.c.
> 
> I tested the omap_vout driver with a Beagle XM board to check that
> the '+ 1' in omap_vout_vrfb.c was indeed a workaround for the omap-dma
> bug.

Applied, thanks

-- 
~Vinod

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-09  8:32 [PATCHv2] omap-dma/omap_vout_vrfb: fix off-by-one fi value Hans Verkuil
2019-08-09  8:39 ` Mauro Carvalho Chehab
2019-08-09 11:04 ` Vinod Koul

dmaengine Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/dmaengine/0 dmaengine/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dmaengine dmaengine/ https://lore.kernel.org/dmaengine \
		dmaengine@vger.kernel.org dmaengine@archiver.kernel.org
	public-inbox-index dmaengine


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.dmaengine


AGPL code for this site: git clone https://public-inbox.org/ public-inbox