* [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
* 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
* [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
* 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
* [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 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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox