All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] media: imx: imx7-media-csi: Small fix and cleanups
@ 2022-09-07 19:15 Laurent Pinchart
  2022-09-07 19:15 ` [PATCH 1/3] media: imx: imx7-media-csi: Move variable to loop scope Laurent Pinchart
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Laurent Pinchart @ 2022-09-07 19:15 UTC (permalink / raw)
  To: linux-media
  Cc: Rui Miguel Silva, Steve Longerbeam, Philipp Zabel, Paul Elder,
	Jacopo Mondi, kernel, linux-imx

Hello,

This small series builds on top of [1]. It performs two small cleanups
(1/3 and 2/3) and fixes a, issue (in 3/3) in the imx7-media-csi driver.

Please see individual patches for details.

[1] https://lore.kernel.org/linux-media/20220907114737.1127612-1-paul.elder@ideasonboard.com

Laurent Pinchart (3):
  media: imx: imx7-media-csi: Move variable to loop scope
  media: imx: imx7-media-csi: Rename phys variables to dma_addr
  media: imx: imx7-media-csi: Clear BIT_MIPI_DOUBLE_CMPNT for <16b
    formats

 drivers/staging/media/imx/imx7-media-csi.c | 43 +++++++++++-----------
 1 file changed, 22 insertions(+), 21 deletions(-)

-- 
Regards,

Laurent Pinchart


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

* [PATCH 1/3] media: imx: imx7-media-csi: Move variable to loop scope
  2022-09-07 19:15 [PATCH 0/3] media: imx: imx7-media-csi: Small fix and cleanups Laurent Pinchart
@ 2022-09-07 19:15 ` Laurent Pinchart
  2022-09-08  3:20   ` paul.elder
  2022-09-07 19:15 ` [PATCH 2/3] media: imx: imx7-media-csi: Rename phys variables to dma_addr Laurent Pinchart
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 8+ messages in thread
From: Laurent Pinchart @ 2022-09-07 19:15 UTC (permalink / raw)
  To: linux-media
  Cc: Rui Miguel Silva, Steve Longerbeam, Philipp Zabel, Paul Elder,
	Jacopo Mondi, kernel, linux-imx

The phys variable is only used as a local loop variable in
imx7_csi_setup_vb2_buf(), with each entry in the array being used in the
corresponding iteration of the loop only. Move it to loop scope,
simplifying the array to a single variable.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/staging/media/imx/imx7-media-csi.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/media/imx/imx7-media-csi.c b/drivers/staging/media/imx/imx7-media-csi.c
index 336f746ae324..1aef2cf41745 100644
--- a/drivers/staging/media/imx/imx7-media-csi.c
+++ b/drivers/staging/media/imx/imx7-media-csi.c
@@ -399,21 +399,22 @@ static void imx7_csi_setup_vb2_buf(struct imx7_csi *csi)
 {
 	struct imx7_csi_vb2_buffer *buf;
 	struct vb2_buffer *vb2_buf;
-	dma_addr_t phys[2];
 	int i;
 
 	for (i = 0; i < 2; i++) {
+		dma_addr_t phys;
+
 		buf = imx7_csi_video_next_buf(csi);
 		if (buf) {
 			csi->active_vb2_buf[i] = buf;
 			vb2_buf = &buf->vbuf.vb2_buf;
-			phys[i] = vb2_dma_contig_plane_dma_addr(vb2_buf, 0);
+			phys = vb2_dma_contig_plane_dma_addr(vb2_buf, 0);
 		} else {
 			csi->active_vb2_buf[i] = NULL;
-			phys[i] = csi->underrun_buf.phys;
+			phys = csi->underrun_buf.phys;
 		}
 
-		imx7_csi_update_buf(csi, phys[i], i);
+		imx7_csi_update_buf(csi, phys, i);
 	}
 }
 
-- 
Regards,

Laurent Pinchart


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

* [PATCH 2/3] media: imx: imx7-media-csi: Rename phys variables to dma_addr
  2022-09-07 19:15 [PATCH 0/3] media: imx: imx7-media-csi: Small fix and cleanups Laurent Pinchart
  2022-09-07 19:15 ` [PATCH 1/3] media: imx: imx7-media-csi: Move variable to loop scope Laurent Pinchart
@ 2022-09-07 19:15 ` Laurent Pinchart
  2022-09-08  3:36   ` paul.elder
  2022-09-07 19:15 ` [PATCH 3/3] media: imx: imx7-media-csi: Clear BIT_MIPI_DOUBLE_CMPNT for <16b formats Laurent Pinchart
  2022-09-08 15:02 ` [PATCH 0/3] media: imx: imx7-media-csi: Small fix and cleanups Rui Miguel Silva
  3 siblings, 1 reply; 8+ messages in thread
From: Laurent Pinchart @ 2022-09-07 19:15 UTC (permalink / raw)
  To: linux-media
  Cc: Rui Miguel Silva, Steve Longerbeam, Philipp Zabel, Paul Elder,
	Jacopo Mondi, kernel, linux-imx

All the phys variables and structure fields store a DMA address, not a
physical address. Even if the two are effectively identical on all
platforms where this driver is used due to the lack of IOMMU, rename the
variables to dma_addr to make their usage clearer.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/staging/media/imx/imx7-media-csi.c | 36 +++++++++++-----------
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/drivers/staging/media/imx/imx7-media-csi.c b/drivers/staging/media/imx/imx7-media-csi.c
index 1aef2cf41745..03986445c0da 100644
--- a/drivers/staging/media/imx/imx7-media-csi.c
+++ b/drivers/staging/media/imx/imx7-media-csi.c
@@ -200,7 +200,7 @@ to_imx7_csi_vb2_buffer(struct vb2_buffer *vb)
 
 struct imx7_csi_dma_buf {
 	void *virt;
-	dma_addr_t phys;
+	dma_addr_t dma_addr;
 	unsigned long len;
 };
 
@@ -384,13 +384,13 @@ static void imx7_csi_dmareq_rff_disable(struct imx7_csi *csi)
 	imx7_csi_reg_write(csi, cr3, CSI_CSICR3);
 }
 
-static void imx7_csi_update_buf(struct imx7_csi *csi, dma_addr_t phys,
+static void imx7_csi_update_buf(struct imx7_csi *csi, dma_addr_t dma_addr,
 				int buf_num)
 {
 	if (buf_num == 1)
-		imx7_csi_reg_write(csi, phys, CSI_CSIDMASA_FB2);
+		imx7_csi_reg_write(csi, dma_addr, CSI_CSIDMASA_FB2);
 	else
-		imx7_csi_reg_write(csi, phys, CSI_CSIDMASA_FB1);
+		imx7_csi_reg_write(csi, dma_addr, CSI_CSIDMASA_FB1);
 }
 
 static struct imx7_csi_vb2_buffer *imx7_csi_video_next_buf(struct imx7_csi *csi);
@@ -402,19 +402,19 @@ static void imx7_csi_setup_vb2_buf(struct imx7_csi *csi)
 	int i;
 
 	for (i = 0; i < 2; i++) {
-		dma_addr_t phys;
+		dma_addr_t dma_addr;
 
 		buf = imx7_csi_video_next_buf(csi);
 		if (buf) {
 			csi->active_vb2_buf[i] = buf;
 			vb2_buf = &buf->vbuf.vb2_buf;
-			phys = vb2_dma_contig_plane_dma_addr(vb2_buf, 0);
+			dma_addr = vb2_dma_contig_plane_dma_addr(vb2_buf, 0);
 		} else {
 			csi->active_vb2_buf[i] = NULL;
-			phys = csi->underrun_buf.phys;
+			dma_addr = csi->underrun_buf.dma_addr;
 		}
 
-		imx7_csi_update_buf(csi, phys, i);
+		imx7_csi_update_buf(csi, dma_addr, i);
 	}
 }
 
@@ -441,10 +441,10 @@ static void imx7_csi_free_dma_buf(struct imx7_csi *csi,
 				  struct imx7_csi_dma_buf *buf)
 {
 	if (buf->virt)
-		dma_free_coherent(csi->dev, buf->len, buf->virt, buf->phys);
+		dma_free_coherent(csi->dev, buf->len, buf->virt, buf->dma_addr);
 
 	buf->virt = NULL;
-	buf->phys = 0;
+	buf->dma_addr = 0;
 }
 
 static int imx7_csi_alloc_dma_buf(struct imx7_csi *csi,
@@ -453,7 +453,7 @@ static int imx7_csi_alloc_dma_buf(struct imx7_csi *csi,
 	imx7_csi_free_dma_buf(csi, buf);
 
 	buf->len = PAGE_ALIGN(size);
-	buf->virt = dma_alloc_coherent(csi->dev, buf->len, &buf->phys,
+	buf->virt = dma_alloc_coherent(csi->dev, buf->len, &buf->dma_addr,
 				       GFP_DMA | GFP_KERNEL);
 	if (!buf->virt)
 		return -ENOMEM;
@@ -714,7 +714,7 @@ static void imx7_csi_vb2_buf_done(struct imx7_csi *csi)
 {
 	struct imx7_csi_vb2_buffer *done, *next;
 	struct vb2_buffer *vb;
-	dma_addr_t phys;
+	dma_addr_t dma_addr;
 
 	done = csi->active_vb2_buf[csi->buf_num];
 	if (done) {
@@ -729,14 +729,14 @@ static void imx7_csi_vb2_buf_done(struct imx7_csi *csi)
 	/* get next queued buffer */
 	next = imx7_csi_video_next_buf(csi);
 	if (next) {
-		phys = vb2_dma_contig_plane_dma_addr(&next->vbuf.vb2_buf, 0);
+		dma_addr = vb2_dma_contig_plane_dma_addr(&next->vbuf.vb2_buf, 0);
 		csi->active_vb2_buf[csi->buf_num] = next;
 	} else {
-		phys = csi->underrun_buf.phys;
+		dma_addr = csi->underrun_buf.dma_addr;
 		csi->active_vb2_buf[csi->buf_num] = NULL;
 	}
 
-	imx7_csi_update_buf(csi, phys, csi->buf_num);
+	imx7_csi_update_buf(csi, dma_addr, csi->buf_num);
 }
 
 static irqreturn_t imx7_csi_irq_handler(int irq, void *data)
@@ -1301,14 +1301,14 @@ static bool imx7_csi_fast_track_buffer(struct imx7_csi *csi,
 				       struct imx7_csi_vb2_buffer *buf)
 {
 	unsigned long flags;
-	dma_addr_t phys;
+	dma_addr_t dma_addr;
 	int buf_num;
 	u32 isr;
 
 	if (!csi->is_streaming)
 		return false;
 
-	phys = vb2_dma_contig_plane_dma_addr(&buf->vbuf.vb2_buf, 0);
+	dma_addr = vb2_dma_contig_plane_dma_addr(&buf->vbuf.vb2_buf, 0);
 
 	/*
 	 * buf_num holds the framebuffer ID of the most recently (*not* the next
@@ -1345,7 +1345,7 @@ static bool imx7_csi_fast_track_buffer(struct imx7_csi *csi,
 		return false;
 	}
 
-	imx7_csi_update_buf(csi, phys, buf_num);
+	imx7_csi_update_buf(csi, dma_addr, buf_num);
 
 	isr = imx7_csi_reg_read(csi, CSI_CSISR);
 	if (isr & (buf_num ? BIT_DMA_TSF_DONE_FB1 : BIT_DMA_TSF_DONE_FB2)) {
-- 
Regards,

Laurent Pinchart


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

* [PATCH 3/3] media: imx: imx7-media-csi: Clear BIT_MIPI_DOUBLE_CMPNT for <16b formats
  2022-09-07 19:15 [PATCH 0/3] media: imx: imx7-media-csi: Small fix and cleanups Laurent Pinchart
  2022-09-07 19:15 ` [PATCH 1/3] media: imx: imx7-media-csi: Move variable to loop scope Laurent Pinchart
  2022-09-07 19:15 ` [PATCH 2/3] media: imx: imx7-media-csi: Rename phys variables to dma_addr Laurent Pinchart
@ 2022-09-07 19:15 ` Laurent Pinchart
  2022-09-08  3:49   ` paul.elder
  2022-09-08 15:02 ` [PATCH 0/3] media: imx: imx7-media-csi: Small fix and cleanups Rui Miguel Silva
  3 siblings, 1 reply; 8+ messages in thread
From: Laurent Pinchart @ 2022-09-07 19:15 UTC (permalink / raw)
  To: linux-media
  Cc: Rui Miguel Silva, Steve Longerbeam, Philipp Zabel, Paul Elder,
	Jacopo Mondi, kernel, linux-imx

Commit 9babbbaaeb87 ("media: imx: imx7-media-csi: Use dual sampling for
YUV 1X16") set BIT_MIPI_DOUBLE_CMPNT in the CR18 register for 16-bit YUV
formats in imx7_csi_configure(). The CR18 register is always updated
with read-modify-write cycles, so if a 16-bit YUV format is selected,
the bit will stay set forever, even if the format is changed. Fix it by
clearing the bit at the beginning of the imx7_csi_configure() function.

While at it, swap two of the bits being cleared to match the MSB to LSB
order. This doesn't cause any functional change.

Fixes: 9babbbaaeb87 ("media: imx: imx7-media-csi: Use dual sampling for YUV 1X16")
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/staging/media/imx/imx7-media-csi.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/media/imx/imx7-media-csi.c b/drivers/staging/media/imx/imx7-media-csi.c
index 03986445c0da..21d6e56ffcd4 100644
--- a/drivers/staging/media/imx/imx7-media-csi.c
+++ b/drivers/staging/media/imx/imx7-media-csi.c
@@ -522,9 +522,9 @@ static void imx7_csi_configure(struct imx7_csi *csi)
 	cr18 = imx7_csi_reg_read(csi, CSI_CSICR18);
 
 	cr18 &= ~(BIT_CSI_HW_ENABLE | BIT_MIPI_DATA_FORMAT_MASK |
-		  BIT_DATA_FROM_MIPI | BIT_BASEADDR_CHG_ERR_EN |
-		  BIT_BASEADDR_SWITCH_EN | BIT_BASEADDR_SWITCH_SEL |
-		  BIT_DEINTERLACE_EN);
+		  BIT_DATA_FROM_MIPI | BIT_MIPI_DOUBLE_CMPNT |
+		  BIT_BASEADDR_CHG_ERR_EN | BIT_BASEADDR_SWITCH_SEL |
+		  BIT_BASEADDR_SWITCH_EN | BIT_DEINTERLACE_EN);
 
 	if (out_pix->field == V4L2_FIELD_INTERLACED) {
 		cr18 |= BIT_DEINTERLACE_EN;
-- 
Regards,

Laurent Pinchart


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

* Re: [PATCH 1/3] media: imx: imx7-media-csi: Move variable to loop scope
  2022-09-07 19:15 ` [PATCH 1/3] media: imx: imx7-media-csi: Move variable to loop scope Laurent Pinchart
@ 2022-09-08  3:20   ` paul.elder
  0 siblings, 0 replies; 8+ messages in thread
From: paul.elder @ 2022-09-08  3:20 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: linux-media, Rui Miguel Silva, Steve Longerbeam, Philipp Zabel,
	Jacopo Mondi, kernel, linux-imx

On Wed, Sep 07, 2022 at 10:15:45PM +0300, Laurent Pinchart wrote:
> The phys variable is only used as a local loop variable in
> imx7_csi_setup_vb2_buf(), with each entry in the array being used in the
> corresponding iteration of the loop only. Move it to loop scope,
> simplifying the array to a single variable.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>

> ---
>  drivers/staging/media/imx/imx7-media-csi.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/media/imx/imx7-media-csi.c b/drivers/staging/media/imx/imx7-media-csi.c
> index 336f746ae324..1aef2cf41745 100644
> --- a/drivers/staging/media/imx/imx7-media-csi.c
> +++ b/drivers/staging/media/imx/imx7-media-csi.c
> @@ -399,21 +399,22 @@ static void imx7_csi_setup_vb2_buf(struct imx7_csi *csi)
>  {
>  	struct imx7_csi_vb2_buffer *buf;
>  	struct vb2_buffer *vb2_buf;
> -	dma_addr_t phys[2];
>  	int i;
>  
>  	for (i = 0; i < 2; i++) {
> +		dma_addr_t phys;
> +
>  		buf = imx7_csi_video_next_buf(csi);
>  		if (buf) {
>  			csi->active_vb2_buf[i] = buf;
>  			vb2_buf = &buf->vbuf.vb2_buf;
> -			phys[i] = vb2_dma_contig_plane_dma_addr(vb2_buf, 0);
> +			phys = vb2_dma_contig_plane_dma_addr(vb2_buf, 0);
>  		} else {
>  			csi->active_vb2_buf[i] = NULL;
> -			phys[i] = csi->underrun_buf.phys;
> +			phys = csi->underrun_buf.phys;
>  		}
>  
> -		imx7_csi_update_buf(csi, phys[i], i);
> +		imx7_csi_update_buf(csi, phys, i);
>  	}
>  }

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

* Re: [PATCH 2/3] media: imx: imx7-media-csi: Rename phys variables to dma_addr
  2022-09-07 19:15 ` [PATCH 2/3] media: imx: imx7-media-csi: Rename phys variables to dma_addr Laurent Pinchart
@ 2022-09-08  3:36   ` paul.elder
  0 siblings, 0 replies; 8+ messages in thread
From: paul.elder @ 2022-09-08  3:36 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: linux-media, Rui Miguel Silva, Steve Longerbeam, Philipp Zabel,
	Jacopo Mondi, kernel, linux-imx

On Wed, Sep 07, 2022 at 10:15:46PM +0300, Laurent Pinchart wrote:
> All the phys variables and structure fields store a DMA address, not a
> physical address. Even if the two are effectively identical on all
> platforms where this driver is used due to the lack of IOMMU, rename the
> variables to dma_addr to make their usage clearer.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>

> ---
>  drivers/staging/media/imx/imx7-media-csi.c | 36 +++++++++++-----------
>  1 file changed, 18 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/staging/media/imx/imx7-media-csi.c b/drivers/staging/media/imx/imx7-media-csi.c
> index 1aef2cf41745..03986445c0da 100644
> --- a/drivers/staging/media/imx/imx7-media-csi.c
> +++ b/drivers/staging/media/imx/imx7-media-csi.c
> @@ -200,7 +200,7 @@ to_imx7_csi_vb2_buffer(struct vb2_buffer *vb)
>  
>  struct imx7_csi_dma_buf {
>  	void *virt;
> -	dma_addr_t phys;
> +	dma_addr_t dma_addr;
>  	unsigned long len;
>  };
>  
> @@ -384,13 +384,13 @@ static void imx7_csi_dmareq_rff_disable(struct imx7_csi *csi)
>  	imx7_csi_reg_write(csi, cr3, CSI_CSICR3);
>  }
>  
> -static void imx7_csi_update_buf(struct imx7_csi *csi, dma_addr_t phys,
> +static void imx7_csi_update_buf(struct imx7_csi *csi, dma_addr_t dma_addr,
>  				int buf_num)
>  {
>  	if (buf_num == 1)
> -		imx7_csi_reg_write(csi, phys, CSI_CSIDMASA_FB2);
> +		imx7_csi_reg_write(csi, dma_addr, CSI_CSIDMASA_FB2);
>  	else
> -		imx7_csi_reg_write(csi, phys, CSI_CSIDMASA_FB1);
> +		imx7_csi_reg_write(csi, dma_addr, CSI_CSIDMASA_FB1);
>  }
>  
>  static struct imx7_csi_vb2_buffer *imx7_csi_video_next_buf(struct imx7_csi *csi);
> @@ -402,19 +402,19 @@ static void imx7_csi_setup_vb2_buf(struct imx7_csi *csi)
>  	int i;
>  
>  	for (i = 0; i < 2; i++) {
> -		dma_addr_t phys;
> +		dma_addr_t dma_addr;
>  
>  		buf = imx7_csi_video_next_buf(csi);
>  		if (buf) {
>  			csi->active_vb2_buf[i] = buf;
>  			vb2_buf = &buf->vbuf.vb2_buf;
> -			phys = vb2_dma_contig_plane_dma_addr(vb2_buf, 0);
> +			dma_addr = vb2_dma_contig_plane_dma_addr(vb2_buf, 0);
>  		} else {
>  			csi->active_vb2_buf[i] = NULL;
> -			phys = csi->underrun_buf.phys;
> +			dma_addr = csi->underrun_buf.dma_addr;
>  		}
>  
> -		imx7_csi_update_buf(csi, phys, i);
> +		imx7_csi_update_buf(csi, dma_addr, i);
>  	}
>  }
>  
> @@ -441,10 +441,10 @@ static void imx7_csi_free_dma_buf(struct imx7_csi *csi,
>  				  struct imx7_csi_dma_buf *buf)
>  {
>  	if (buf->virt)
> -		dma_free_coherent(csi->dev, buf->len, buf->virt, buf->phys);
> +		dma_free_coherent(csi->dev, buf->len, buf->virt, buf->dma_addr);
>  
>  	buf->virt = NULL;
> -	buf->phys = 0;
> +	buf->dma_addr = 0;
>  }
>  
>  static int imx7_csi_alloc_dma_buf(struct imx7_csi *csi,
> @@ -453,7 +453,7 @@ static int imx7_csi_alloc_dma_buf(struct imx7_csi *csi,
>  	imx7_csi_free_dma_buf(csi, buf);
>  
>  	buf->len = PAGE_ALIGN(size);
> -	buf->virt = dma_alloc_coherent(csi->dev, buf->len, &buf->phys,
> +	buf->virt = dma_alloc_coherent(csi->dev, buf->len, &buf->dma_addr,
>  				       GFP_DMA | GFP_KERNEL);
>  	if (!buf->virt)
>  		return -ENOMEM;
> @@ -714,7 +714,7 @@ static void imx7_csi_vb2_buf_done(struct imx7_csi *csi)
>  {
>  	struct imx7_csi_vb2_buffer *done, *next;
>  	struct vb2_buffer *vb;
> -	dma_addr_t phys;
> +	dma_addr_t dma_addr;
>  
>  	done = csi->active_vb2_buf[csi->buf_num];
>  	if (done) {
> @@ -729,14 +729,14 @@ static void imx7_csi_vb2_buf_done(struct imx7_csi *csi)
>  	/* get next queued buffer */
>  	next = imx7_csi_video_next_buf(csi);
>  	if (next) {
> -		phys = vb2_dma_contig_plane_dma_addr(&next->vbuf.vb2_buf, 0);
> +		dma_addr = vb2_dma_contig_plane_dma_addr(&next->vbuf.vb2_buf, 0);
>  		csi->active_vb2_buf[csi->buf_num] = next;
>  	} else {
> -		phys = csi->underrun_buf.phys;
> +		dma_addr = csi->underrun_buf.dma_addr;
>  		csi->active_vb2_buf[csi->buf_num] = NULL;
>  	}
>  
> -	imx7_csi_update_buf(csi, phys, csi->buf_num);
> +	imx7_csi_update_buf(csi, dma_addr, csi->buf_num);
>  }
>  
>  static irqreturn_t imx7_csi_irq_handler(int irq, void *data)
> @@ -1301,14 +1301,14 @@ static bool imx7_csi_fast_track_buffer(struct imx7_csi *csi,
>  				       struct imx7_csi_vb2_buffer *buf)
>  {
>  	unsigned long flags;
> -	dma_addr_t phys;
> +	dma_addr_t dma_addr;
>  	int buf_num;
>  	u32 isr;
>  
>  	if (!csi->is_streaming)
>  		return false;
>  
> -	phys = vb2_dma_contig_plane_dma_addr(&buf->vbuf.vb2_buf, 0);
> +	dma_addr = vb2_dma_contig_plane_dma_addr(&buf->vbuf.vb2_buf, 0);
>  
>  	/*
>  	 * buf_num holds the framebuffer ID of the most recently (*not* the next
> @@ -1345,7 +1345,7 @@ static bool imx7_csi_fast_track_buffer(struct imx7_csi *csi,
>  		return false;
>  	}
>  
> -	imx7_csi_update_buf(csi, phys, buf_num);
> +	imx7_csi_update_buf(csi, dma_addr, buf_num);
>  
>  	isr = imx7_csi_reg_read(csi, CSI_CSISR);
>  	if (isr & (buf_num ? BIT_DMA_TSF_DONE_FB1 : BIT_DMA_TSF_DONE_FB2)) {

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

* Re: [PATCH 3/3] media: imx: imx7-media-csi: Clear BIT_MIPI_DOUBLE_CMPNT for <16b formats
  2022-09-07 19:15 ` [PATCH 3/3] media: imx: imx7-media-csi: Clear BIT_MIPI_DOUBLE_CMPNT for <16b formats Laurent Pinchart
@ 2022-09-08  3:49   ` paul.elder
  0 siblings, 0 replies; 8+ messages in thread
From: paul.elder @ 2022-09-08  3:49 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: linux-media, Rui Miguel Silva, Steve Longerbeam, Philipp Zabel,
	Jacopo Mondi, kernel, linux-imx

On Wed, Sep 07, 2022 at 10:15:47PM +0300, Laurent Pinchart wrote:
> Commit 9babbbaaeb87 ("media: imx: imx7-media-csi: Use dual sampling for
> YUV 1X16") set BIT_MIPI_DOUBLE_CMPNT in the CR18 register for 16-bit YUV
> formats in imx7_csi_configure(). The CR18 register is always updated
> with read-modify-write cycles, so if a 16-bit YUV format is selected,
> the bit will stay set forever, even if the format is changed. Fix it by
> clearing the bit at the beginning of the imx7_csi_configure() function.
> 
> While at it, swap two of the bits being cleared to match the MSB to LSB
> order. This doesn't cause any functional change.
> 
> Fixes: 9babbbaaeb87 ("media: imx: imx7-media-csi: Use dual sampling for YUV 1X16")
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>

> ---
>  drivers/staging/media/imx/imx7-media-csi.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/media/imx/imx7-media-csi.c b/drivers/staging/media/imx/imx7-media-csi.c
> index 03986445c0da..21d6e56ffcd4 100644
> --- a/drivers/staging/media/imx/imx7-media-csi.c
> +++ b/drivers/staging/media/imx/imx7-media-csi.c
> @@ -522,9 +522,9 @@ static void imx7_csi_configure(struct imx7_csi *csi)
>  	cr18 = imx7_csi_reg_read(csi, CSI_CSICR18);
>  
>  	cr18 &= ~(BIT_CSI_HW_ENABLE | BIT_MIPI_DATA_FORMAT_MASK |
> -		  BIT_DATA_FROM_MIPI | BIT_BASEADDR_CHG_ERR_EN |
> -		  BIT_BASEADDR_SWITCH_EN | BIT_BASEADDR_SWITCH_SEL |
> -		  BIT_DEINTERLACE_EN);
> +		  BIT_DATA_FROM_MIPI | BIT_MIPI_DOUBLE_CMPNT |
> +		  BIT_BASEADDR_CHG_ERR_EN | BIT_BASEADDR_SWITCH_SEL |
> +		  BIT_BASEADDR_SWITCH_EN | BIT_DEINTERLACE_EN);
>  
>  	if (out_pix->field == V4L2_FIELD_INTERLACED) {
>  		cr18 |= BIT_DEINTERLACE_EN;

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

* Re: [PATCH 0/3] media: imx: imx7-media-csi: Small fix and cleanups
  2022-09-07 19:15 [PATCH 0/3] media: imx: imx7-media-csi: Small fix and cleanups Laurent Pinchart
                   ` (2 preceding siblings ...)
  2022-09-07 19:15 ` [PATCH 3/3] media: imx: imx7-media-csi: Clear BIT_MIPI_DOUBLE_CMPNT for <16b formats Laurent Pinchart
@ 2022-09-08 15:02 ` Rui Miguel Silva
  3 siblings, 0 replies; 8+ messages in thread
From: Rui Miguel Silva @ 2022-09-08 15:02 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: linux-media, Steve Longerbeam, Philipp Zabel, Paul Elder,
	Jacopo Mondi, kernel, linux-imx

Hi Laurent,
On Wed, Sep 07, 2022 at 10:15:44PM +0300, Laurent Pinchart wrote:
> Hello,
> 
> This small series builds on top of [1]. It performs two small cleanups
> (1/3 and 2/3) and fixes a, issue (in 3/3) in the imx7-media-csi driver.
> 
> Please see individual patches for details.

All look good to me. Thanks for this.
For the all series:

Acked-by: Rui Miguel Silva <rmfrfs@gmail.com>

Cheers,
  Rui
> 
> [1] https://lore.kernel.org/linux-media/20220907114737.1127612-1-paul.elder@ideasonboard.com
> 
> Laurent Pinchart (3):
>   media: imx: imx7-media-csi: Move variable to loop scope
>   media: imx: imx7-media-csi: Rename phys variables to dma_addr
>   media: imx: imx7-media-csi: Clear BIT_MIPI_DOUBLE_CMPNT for <16b
>     formats
> 
>  drivers/staging/media/imx/imx7-media-csi.c | 43 +++++++++++-----------
>  1 file changed, 22 insertions(+), 21 deletions(-)
> 
> -- 
> Regards,
> 
> Laurent Pinchart
> 

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

end of thread, other threads:[~2022-09-08 15:02 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-07 19:15 [PATCH 0/3] media: imx: imx7-media-csi: Small fix and cleanups Laurent Pinchart
2022-09-07 19:15 ` [PATCH 1/3] media: imx: imx7-media-csi: Move variable to loop scope Laurent Pinchart
2022-09-08  3:20   ` paul.elder
2022-09-07 19:15 ` [PATCH 2/3] media: imx: imx7-media-csi: Rename phys variables to dma_addr Laurent Pinchart
2022-09-08  3:36   ` paul.elder
2022-09-07 19:15 ` [PATCH 3/3] media: imx: imx7-media-csi: Clear BIT_MIPI_DOUBLE_CMPNT for <16b formats Laurent Pinchart
2022-09-08  3:49   ` paul.elder
2022-09-08 15:02 ` [PATCH 0/3] media: imx: imx7-media-csi: Small fix and cleanups Rui Miguel Silva

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.