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