[v12,08/12] media: rkisp1: Support i.MX8MP's 34-bit DMA
Commit Message
On the ISP that is integrated in the i.MX8MP, DMA addresses have been
extended to 34 bits, with the 32 MSBs stored in the DMA address
registers and the 2 LSBs set to 0.
To support this:
- Shift the addresses to the right by 2 when writing to registers
- Set the dma mask to 34 bits
- Use dma_addr_t instead of u32 when storing the addresses
Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Tested-by: Alexander Stein <alexander.stein@ew.tq-group.com>
Tested-by: Adam Ford <aford173@gmail.com>
Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
Changes since v5:
- Improve the commit message
Changes since v4:
- Squash in fix from Tomi:
- https://gitlab.com/ideasonboard/nxp/linux/-/commit/d6477fe673b1c0d05d12ae21d8db9a03b07e7fea
Changes since v2:
- Document the RKISP1_FEATURE_DMA_34BIT bit
- Use the rkisp1_has_feature() macro
---
.../platform/rockchip/rkisp1/rkisp1-capture.c | 20 ++++++++++---------
.../platform/rockchip/rkisp1/rkisp1-common.h | 4 +++-
.../platform/rockchip/rkisp1/rkisp1-dev.c | 8 ++++++++
3 files changed, 22 insertions(+), 10 deletions(-)
Comments
Hi Paul,
thanks for the update.
Am Freitag, 16. Februar 2024, 10:54:54 CET schrieb Paul Elder:
> On the ISP that is integrated in the i.MX8MP, DMA addresses have been
> extended to 34 bits, with the 32 MSBs stored in the DMA address
> registers and the 2 LSBs set to 0.
>
> To support this:
> - Shift the addresses to the right by 2 when writing to registers
> - Set the dma mask to 34 bits
> - Use dma_addr_t instead of u32 when storing the addresses
>
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> Tested-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> Tested-by: Adam Ford <aford173@gmail.com>
> Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> ---
> Changes since v5:
>
> - Improve the commit message
>
> Changes since v4:
>
> - Squash in fix from Tomi:
> -
> https://gitlab.com/ideasonboard/nxp/linux/-/commit/d6477fe673b1c0d05d12ae21
> d8db9a03b07e7fea
>
> Changes since v2:
>
> - Document the RKISP1_FEATURE_DMA_34BIT bit
> - Use the rkisp1_has_feature() macro
> ---
> .../platform/rockchip/rkisp1/rkisp1-capture.c | 20 ++++++++++---------
> .../platform/rockchip/rkisp1/rkisp1-common.h | 4 +++-
> .../platform/rockchip/rkisp1/rkisp1-dev.c | 8 ++++++++
> 3 files changed, 22 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
> b/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c index
> ca95f62822fa..1ee7639c42b7 100644
> --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
> +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
> @@ -648,11 +648,13 @@ static void rkisp1_dummy_buf_destroy(struct
> rkisp1_capture *cap)
>
> static void rkisp1_set_next_buf(struct rkisp1_capture *cap)
> {
> + u8 shift = rkisp1_has_feature(cap->rkisp1, DMA_34BIT) ? 2 : 0;
> +
> cap->buf.curr = cap->buf.next;
> cap->buf.next = NULL;
>
> if (!list_empty(&cap->buf.queue)) {
> - u32 *buff_addr;
> + dma_addr_t *buff_addr;
>
> cap->buf.next = list_first_entry(&cap->buf.queue, struct
rkisp1_buffer,
> queue); list_del(&cap->buf.next->queue);
> @@ -660,7 +662,7 @@ static void rkisp1_set_next_buf(struct rkisp1_capture
> *cap) buff_addr = cap->buf.next->buff_addr;
>
> rkisp1_write(cap->rkisp1, cap->config->mi.y_base_ad_init,
> - buff_addr[RKISP1_PLANE_Y]);
> + buff_addr[RKISP1_PLANE_Y] >> shift);
> /*
> * In order to support grey format we capture
> * YUV422 planar format from the camera and
> @@ -669,17 +671,17 @@ static void rkisp1_set_next_buf(struct rkisp1_capture
> *cap) if (cap->pix.cfg->fourcc == V4L2_PIX_FMT_GREY) {
> rkisp1_write(cap->rkisp1,
> cap->config->mi.cb_base_ad_init,
> - cap->buf.dummy.dma_addr);
> + cap->buf.dummy.dma_addr >>
shift);
> rkisp1_write(cap->rkisp1,
> cap->config->mi.cr_base_ad_init,
> - cap->buf.dummy.dma_addr);
> + cap->buf.dummy.dma_addr >>
shift);
> } else {
> rkisp1_write(cap->rkisp1,
> cap->config->mi.cb_base_ad_init,
> - buff_addr[RKISP1_PLANE_CB]);
> + buff_addr[RKISP1_PLANE_CB] >>
shift);
> rkisp1_write(cap->rkisp1,
> cap->config->mi.cr_base_ad_init,
> - buff_addr[RKISP1_PLANE_CR]);
> + buff_addr[RKISP1_PLANE_CR] >>
shift);
> }
> } else {
> /*
> @@ -687,11 +689,11 @@ static void rkisp1_set_next_buf(struct rkisp1_capture
> *cap) * throw data if there is no available buffer.
> */
> rkisp1_write(cap->rkisp1, cap->config->mi.y_base_ad_init,
> - cap->buf.dummy.dma_addr);
> + cap->buf.dummy.dma_addr >> shift);
> rkisp1_write(cap->rkisp1, cap->config->mi.cb_base_ad_init,
> - cap->buf.dummy.dma_addr);
> + cap->buf.dummy.dma_addr >> shift);
> rkisp1_write(cap->rkisp1, cap->config->mi.cr_base_ad_init,
> - cap->buf.dummy.dma_addr);
> + cap->buf.dummy.dma_addr >> shift);
> }
>
> /* Set plane offsets */
> diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
> b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h index
> 69940014d597..26573f6ae575 100644
> --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
> +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
> @@ -114,6 +114,7 @@ enum rkisp1_isp_pad {
> * @RKISP1_FEATURE_MAIN_STRIDE: The ISP supports configurable stride on the
> main path * @RKISP1_FEATURE_SELF_PATH: The ISP has a self path
> * @RKISP1_FEATURE_DUAL_CROP: The ISP has the dual crop block at the
> resizer input + * @RKISP1_FEATURE_DMA_34BIT: The ISP uses 34-bit DMA
> addresses
> *
> * The ISP features are stored in a bitmask in &rkisp1_info.features and
> allow * the driver to implement support for features present in some ISP
> versions @@ -124,6 +125,7 @@ enum rkisp1_feature {
> RKISP1_FEATURE_MAIN_STRIDE = BIT(1),
> RKISP1_FEATURE_SELF_PATH = BIT(2),
> RKISP1_FEATURE_DUAL_CROP = BIT(3),
> + RKISP1_FEATURE_DMA_34BIT = BIT(4),
> };
>
> #define rkisp1_has_feature(rkisp1, feature) \
> @@ -239,7 +241,7 @@ struct rkisp1_vdev_node {
> struct rkisp1_buffer {
> struct vb2_v4l2_buffer vb;
> struct list_head queue;
> - u32 buff_addr[VIDEO_MAX_PLANES];
> + dma_addr_t buff_addr[VIDEO_MAX_PLANES];
> };
>
> /*
> diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
> b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c index
> d0a3a13d9dd7..54a62487a4e8 100644
> --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
> +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
> @@ -552,6 +552,7 @@ static int rkisp1_probe(struct platform_device *pdev)
> struct device *dev = &pdev->dev;
> struct rkisp1_device *rkisp1;
> struct v4l2_device *v4l2_dev;
> + unsigned long long dma_mask;
The signature for dma_set_mask_and_coherent() uses u64 for the mask, so I
would use the same type here as well.
Best regards,
Alexander
> unsigned int i;
> int ret, irq;
> u32 cif_id;
> @@ -566,6 +567,13 @@ static int rkisp1_probe(struct platform_device *pdev)
> dev_set_drvdata(dev, rkisp1);
> rkisp1->dev = dev;
>
> + dma_mask = rkisp1_has_feature(rkisp1, DMA_34BIT) ? DMA_BIT_MASK(34)
:
> +
DMA_BIT_MASK(32);
> +
> + ret = dma_set_mask_and_coherent(dev, dma_mask);
> + if (ret)
> + return ret;
> +
> mutex_init(&rkisp1->stream_lock);
>
> rkisp1->base_addr = devm_platform_ioremap_resource(pdev, 0);
Hi Alexander,
On Fri, Feb 16, 2024 at 11:31:46AM +0100, Alexander Stein wrote:
> Hi Paul,
>
> thanks for the update.
>
> Am Freitag, 16. Februar 2024, 10:54:54 CET schrieb Paul Elder:
> > On the ISP that is integrated in the i.MX8MP, DMA addresses have been
> > extended to 34 bits, with the 32 MSBs stored in the DMA address
> > registers and the 2 LSBs set to 0.
> >
> > To support this:
> > - Shift the addresses to the right by 2 when writing to registers
> > - Set the dma mask to 34 bits
> > - Use dma_addr_t instead of u32 when storing the addresses
> >
> > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> > Tested-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> > Tested-by: Adam Ford <aford173@gmail.com>
> > Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> > ---
> > Changes since v5:
> >
> > - Improve the commit message
> >
> > Changes since v4:
> >
> > - Squash in fix from Tomi:
> > -
> > https://gitlab.com/ideasonboard/nxp/linux/-/commit/d6477fe673b1c0d05d12ae21
> > d8db9a03b07e7fea
> >
> > Changes since v2:
> >
> > - Document the RKISP1_FEATURE_DMA_34BIT bit
> > - Use the rkisp1_has_feature() macro
> > ---
> > .../platform/rockchip/rkisp1/rkisp1-capture.c | 20 ++++++++++---------
> > .../platform/rockchip/rkisp1/rkisp1-common.h | 4 +++-
> > .../platform/rockchip/rkisp1/rkisp1-dev.c | 8 ++++++++
> > 3 files changed, 22 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
> > b/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c index
> > ca95f62822fa..1ee7639c42b7 100644
> > --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
> > +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
> > @@ -648,11 +648,13 @@ static void rkisp1_dummy_buf_destroy(struct
> > rkisp1_capture *cap)
> >
> > static void rkisp1_set_next_buf(struct rkisp1_capture *cap)
> > {
> > + u8 shift = rkisp1_has_feature(cap->rkisp1, DMA_34BIT) ? 2 : 0;
> > +
> > cap->buf.curr = cap->buf.next;
> > cap->buf.next = NULL;
> >
> > if (!list_empty(&cap->buf.queue)) {
> > - u32 *buff_addr;
> > + dma_addr_t *buff_addr;
> >
> > cap->buf.next = list_first_entry(&cap->buf.queue, struct
> rkisp1_buffer,
On a side note, any chance your mail client would have an option to
avoid this kind of wrapping ? :-)
> > queue); list_del(&cap->buf.next->queue);
> > @@ -660,7 +662,7 @@ static void rkisp1_set_next_buf(struct rkisp1_capture
> > *cap) buff_addr = cap->buf.next->buff_addr;
> >
> > rkisp1_write(cap->rkisp1, cap->config->mi.y_base_ad_init,
> > - buff_addr[RKISP1_PLANE_Y]);
> > + buff_addr[RKISP1_PLANE_Y] >> shift);
> > /*
> > * In order to support grey format we capture
> > * YUV422 planar format from the camera and
> > @@ -669,17 +671,17 @@ static void rkisp1_set_next_buf(struct rkisp1_capture
> > *cap) if (cap->pix.cfg->fourcc == V4L2_PIX_FMT_GREY) {
> > rkisp1_write(cap->rkisp1,
> > cap->config->mi.cb_base_ad_init,
> > - cap->buf.dummy.dma_addr);
> > + cap->buf.dummy.dma_addr >>
> shift);
> > rkisp1_write(cap->rkisp1,
> > cap->config->mi.cr_base_ad_init,
> > - cap->buf.dummy.dma_addr);
> > + cap->buf.dummy.dma_addr >>
> shift);
> > } else {
> > rkisp1_write(cap->rkisp1,
> > cap->config->mi.cb_base_ad_init,
> > - buff_addr[RKISP1_PLANE_CB]);
> > + buff_addr[RKISP1_PLANE_CB] >>
> shift);
> > rkisp1_write(cap->rkisp1,
> > cap->config->mi.cr_base_ad_init,
> > - buff_addr[RKISP1_PLANE_CR]);
> > + buff_addr[RKISP1_PLANE_CR] >>
> shift);
> > }
> > } else {
> > /*
> > @@ -687,11 +689,11 @@ static void rkisp1_set_next_buf(struct rkisp1_capture
> > *cap) * throw data if there is no available buffer.
> > */
> > rkisp1_write(cap->rkisp1, cap->config->mi.y_base_ad_init,
> > - cap->buf.dummy.dma_addr);
> > + cap->buf.dummy.dma_addr >> shift);
> > rkisp1_write(cap->rkisp1, cap->config->mi.cb_base_ad_init,
> > - cap->buf.dummy.dma_addr);
> > + cap->buf.dummy.dma_addr >> shift);
> > rkisp1_write(cap->rkisp1, cap->config->mi.cr_base_ad_init,
> > - cap->buf.dummy.dma_addr);
> > + cap->buf.dummy.dma_addr >> shift);
> > }
> >
> > /* Set plane offsets */
> > diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
> > b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h index
> > 69940014d597..26573f6ae575 100644
> > --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
> > +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
> > @@ -114,6 +114,7 @@ enum rkisp1_isp_pad {
> > * @RKISP1_FEATURE_MAIN_STRIDE: The ISP supports configurable stride on the
> > main path * @RKISP1_FEATURE_SELF_PATH: The ISP has a self path
> > * @RKISP1_FEATURE_DUAL_CROP: The ISP has the dual crop block at the
> > resizer input + * @RKISP1_FEATURE_DMA_34BIT: The ISP uses 34-bit DMA
> > addresses
> > *
> > * The ISP features are stored in a bitmask in &rkisp1_info.features and
> > allow * the driver to implement support for features present in some ISP
> > versions @@ -124,6 +125,7 @@ enum rkisp1_feature {
> > RKISP1_FEATURE_MAIN_STRIDE = BIT(1),
> > RKISP1_FEATURE_SELF_PATH = BIT(2),
> > RKISP1_FEATURE_DUAL_CROP = BIT(3),
> > + RKISP1_FEATURE_DMA_34BIT = BIT(4),
> > };
> >
> > #define rkisp1_has_feature(rkisp1, feature) \
> > @@ -239,7 +241,7 @@ struct rkisp1_vdev_node {
> > struct rkisp1_buffer {
> > struct vb2_v4l2_buffer vb;
> > struct list_head queue;
> > - u32 buff_addr[VIDEO_MAX_PLANES];
> > + dma_addr_t buff_addr[VIDEO_MAX_PLANES];
> > };
> >
> > /*
> > diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
> > b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c index
> > d0a3a13d9dd7..54a62487a4e8 100644
> > --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
> > +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
> > @@ -552,6 +552,7 @@ static int rkisp1_probe(struct platform_device *pdev)
> > struct device *dev = &pdev->dev;
> > struct rkisp1_device *rkisp1;
> > struct v4l2_device *v4l2_dev;
> > + unsigned long long dma_mask;
>
> The signature for dma_set_mask_and_coherent() uses u64 for the mask, so I
> would use the same type here as well.
I'll do so in v13.
> > unsigned int i;
> > int ret, irq;
> > u32 cif_id;
> > @@ -566,6 +567,13 @@ static int rkisp1_probe(struct platform_device *pdev)
> > dev_set_drvdata(dev, rkisp1);
> > rkisp1->dev = dev;
> >
> > + dma_mask = rkisp1_has_feature(rkisp1, DMA_34BIT) ? DMA_BIT_MASK(34)
> :
> > +
> DMA_BIT_MASK(32);
> > +
> > + ret = dma_set_mask_and_coherent(dev, dma_mask);
> > + if (ret)
> > + return ret;
> > +
> > mutex_init(&rkisp1->stream_lock);
> >
> > rkisp1->base_addr = devm_platform_ioremap_resource(pdev, 0);
@@ -648,11 +648,13 @@ static void rkisp1_dummy_buf_destroy(struct rkisp1_capture *cap)
static void rkisp1_set_next_buf(struct rkisp1_capture *cap)
{
+ u8 shift = rkisp1_has_feature(cap->rkisp1, DMA_34BIT) ? 2 : 0;
+
cap->buf.curr = cap->buf.next;
cap->buf.next = NULL;
if (!list_empty(&cap->buf.queue)) {
- u32 *buff_addr;
+ dma_addr_t *buff_addr;
cap->buf.next = list_first_entry(&cap->buf.queue, struct rkisp1_buffer, queue);
list_del(&cap->buf.next->queue);
@@ -660,7 +662,7 @@ static void rkisp1_set_next_buf(struct rkisp1_capture *cap)
buff_addr = cap->buf.next->buff_addr;
rkisp1_write(cap->rkisp1, cap->config->mi.y_base_ad_init,
- buff_addr[RKISP1_PLANE_Y]);
+ buff_addr[RKISP1_PLANE_Y] >> shift);
/*
* In order to support grey format we capture
* YUV422 planar format from the camera and
@@ -669,17 +671,17 @@ static void rkisp1_set_next_buf(struct rkisp1_capture *cap)
if (cap->pix.cfg->fourcc == V4L2_PIX_FMT_GREY) {
rkisp1_write(cap->rkisp1,
cap->config->mi.cb_base_ad_init,
- cap->buf.dummy.dma_addr);
+ cap->buf.dummy.dma_addr >> shift);
rkisp1_write(cap->rkisp1,
cap->config->mi.cr_base_ad_init,
- cap->buf.dummy.dma_addr);
+ cap->buf.dummy.dma_addr >> shift);
} else {
rkisp1_write(cap->rkisp1,
cap->config->mi.cb_base_ad_init,
- buff_addr[RKISP1_PLANE_CB]);
+ buff_addr[RKISP1_PLANE_CB] >> shift);
rkisp1_write(cap->rkisp1,
cap->config->mi.cr_base_ad_init,
- buff_addr[RKISP1_PLANE_CR]);
+ buff_addr[RKISP1_PLANE_CR] >> shift);
}
} else {
/*
@@ -687,11 +689,11 @@ static void rkisp1_set_next_buf(struct rkisp1_capture *cap)
* throw data if there is no available buffer.
*/
rkisp1_write(cap->rkisp1, cap->config->mi.y_base_ad_init,
- cap->buf.dummy.dma_addr);
+ cap->buf.dummy.dma_addr >> shift);
rkisp1_write(cap->rkisp1, cap->config->mi.cb_base_ad_init,
- cap->buf.dummy.dma_addr);
+ cap->buf.dummy.dma_addr >> shift);
rkisp1_write(cap->rkisp1, cap->config->mi.cr_base_ad_init,
- cap->buf.dummy.dma_addr);
+ cap->buf.dummy.dma_addr >> shift);
}
/* Set plane offsets */
@@ -114,6 +114,7 @@ enum rkisp1_isp_pad {
* @RKISP1_FEATURE_MAIN_STRIDE: The ISP supports configurable stride on the main path
* @RKISP1_FEATURE_SELF_PATH: The ISP has a self path
* @RKISP1_FEATURE_DUAL_CROP: The ISP has the dual crop block at the resizer input
+ * @RKISP1_FEATURE_DMA_34BIT: The ISP uses 34-bit DMA addresses
*
* The ISP features are stored in a bitmask in &rkisp1_info.features and allow
* the driver to implement support for features present in some ISP versions
@@ -124,6 +125,7 @@ enum rkisp1_feature {
RKISP1_FEATURE_MAIN_STRIDE = BIT(1),
RKISP1_FEATURE_SELF_PATH = BIT(2),
RKISP1_FEATURE_DUAL_CROP = BIT(3),
+ RKISP1_FEATURE_DMA_34BIT = BIT(4),
};
#define rkisp1_has_feature(rkisp1, feature) \
@@ -239,7 +241,7 @@ struct rkisp1_vdev_node {
struct rkisp1_buffer {
struct vb2_v4l2_buffer vb;
struct list_head queue;
- u32 buff_addr[VIDEO_MAX_PLANES];
+ dma_addr_t buff_addr[VIDEO_MAX_PLANES];
};
/*
@@ -552,6 +552,7 @@ static int rkisp1_probe(struct platform_device *pdev)
struct device *dev = &pdev->dev;
struct rkisp1_device *rkisp1;
struct v4l2_device *v4l2_dev;
+ unsigned long long dma_mask;
unsigned int i;
int ret, irq;
u32 cif_id;
@@ -566,6 +567,13 @@ static int rkisp1_probe(struct platform_device *pdev)
dev_set_drvdata(dev, rkisp1);
rkisp1->dev = dev;
+ dma_mask = rkisp1_has_feature(rkisp1, DMA_34BIT) ? DMA_BIT_MASK(34) :
+ DMA_BIT_MASK(32);
+
+ ret = dma_set_mask_and_coherent(dev, dma_mask);
+ if (ret)
+ return ret;
+
mutex_init(&rkisp1->stream_lock);
rkisp1->base_addr = devm_platform_ioremap_resource(pdev, 0);