* [PATCH v2] media: rkisp1: fix grey format iommu page faults
@ 2021-12-07 11:59 Dafna Hirschfeld
2022-01-17 11:34 ` Kieran Bingham
0 siblings, 1 reply; 7+ messages in thread
From: Dafna Hirschfeld @ 2021-12-07 11:59 UTC (permalink / raw)
To: linux-media
Cc: nicolas.dufresne, kieran.bingham, Dafna Hirschfeld,
laurent.pinchart, hverkuil, kernel, dafna3, sakari.ailus,
mchehab
Currently capturing grey format produces page faults
on both selfpath and mainpath. To support greyscale
we can capture YUV422 planar format and configure the U, V
buffers to the dummy buffer.
Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
---
This is v2 of the patch "media: rkisp1: remove support for V4L2_PIX_FMT_GREY"
In v1 I removed the grey format. In this version it is 'fixed'
.../platform/rockchip/rkisp1/rkisp1-capture.c | 28 ++++++++++++++-----
1 file changed, 21 insertions(+), 7 deletions(-)
diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
index 768987d5f2dd..8e982dd0c740 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
@@ -249,7 +249,7 @@ static const struct rkisp1_capture_fmt_cfg rkisp1_sp_fmts[] = {
.fourcc = V4L2_PIX_FMT_GREY,
.uv_swap = 0,
.write_format = RKISP1_MI_CTRL_SP_WRITE_PLA,
- .output_format = RKISP1_MI_CTRL_SP_OUTPUT_YUV400,
+ .output_format = RKISP1_MI_CTRL_SP_OUTPUT_YUV422,
.mbus = MEDIA_BUS_FMT_YUYV8_2X8,
},
/* rgb */
@@ -631,12 +631,26 @@ static void rkisp1_set_next_buf(struct rkisp1_capture *cap)
rkisp1_write(cap->rkisp1,
buff_addr[RKISP1_PLANE_Y],
cap->config->mi.y_base_ad_init);
- rkisp1_write(cap->rkisp1,
- buff_addr[RKISP1_PLANE_CB],
- cap->config->mi.cb_base_ad_init);
- rkisp1_write(cap->rkisp1,
- buff_addr[RKISP1_PLANE_CR],
- cap->config->mi.cr_base_ad_init);
+ /*
+ * In order to support grey format we capture
+ * YUV422 planar format from the camera and
+ * set the U and V planes to the dummy buffer
+ */
+ if (cap->pix.cfg->fourcc == V4L2_PIX_FMT_GREY) {
+ rkisp1_write(cap->rkisp1,
+ cap->buf.dummy.dma_addr,
+ cap->config->mi.cb_base_ad_init);
+ rkisp1_write(cap->rkisp1,
+ cap->buf.dummy.dma_addr,
+ cap->config->mi.cr_base_ad_init);
+ } else {
+ rkisp1_write(cap->rkisp1,
+ buff_addr[RKISP1_PLANE_CB],
+ cap->config->mi.cb_base_ad_init);
+ rkisp1_write(cap->rkisp1,
+ buff_addr[RKISP1_PLANE_CR],
+ cap->config->mi.cr_base_ad_init);
+ }
} else {
/*
* Use the dummy space allocated by dma_alloc_coherent to
--
2.17.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2] media: rkisp1: fix grey format iommu page faults
2021-12-07 11:59 [PATCH v2] media: rkisp1: fix grey format iommu page faults Dafna Hirschfeld
@ 2022-01-17 11:34 ` Kieran Bingham
2022-01-23 9:50 ` Dafna Hirschfeld
0 siblings, 1 reply; 7+ messages in thread
From: Kieran Bingham @ 2022-01-17 11:34 UTC (permalink / raw)
To: Dafna Hirschfeld, linux-media
Cc: nicolas.dufresne, Dafna Hirschfeld, laurent.pinchart, hverkuil,
kernel, dafna3, sakari.ailus, mchehab
Hi Dafna,
Quoting Dafna Hirschfeld (2021-12-07 11:59:23)
> Currently capturing grey format produces page faults
> on both selfpath and mainpath. To support greyscale
> we can capture YUV422 planar format and configure the U, V
> buffers to the dummy buffer.
>
> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
> ---
> This is v2 of the patch "media: rkisp1: remove support for V4L2_PIX_FMT_GREY"
> In v1 I removed the grey format. In this version it is 'fixed'
>
> .../platform/rockchip/rkisp1/rkisp1-capture.c | 28 ++++++++++++++-----
> 1 file changed, 21 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
> index 768987d5f2dd..8e982dd0c740 100644
> --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
> +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
> @@ -249,7 +249,7 @@ static const struct rkisp1_capture_fmt_cfg rkisp1_sp_fmts[] = {
> .fourcc = V4L2_PIX_FMT_GREY,
> .uv_swap = 0,
> .write_format = RKISP1_MI_CTRL_SP_WRITE_PLA,
> - .output_format = RKISP1_MI_CTRL_SP_OUTPUT_YUV400,
> + .output_format = RKISP1_MI_CTRL_SP_OUTPUT_YUV422,
> .mbus = MEDIA_BUS_FMT_YUYV8_2X8,
> },
> /* rgb */
> @@ -631,12 +631,26 @@ static void rkisp1_set_next_buf(struct rkisp1_capture *cap)
> rkisp1_write(cap->rkisp1,
> buff_addr[RKISP1_PLANE_Y],
> cap->config->mi.y_base_ad_init);
> - rkisp1_write(cap->rkisp1,
> - buff_addr[RKISP1_PLANE_CB],
> - cap->config->mi.cb_base_ad_init);
> - rkisp1_write(cap->rkisp1,
> - buff_addr[RKISP1_PLANE_CR],
> - cap->config->mi.cr_base_ad_init);
> + /*
> + * In order to support grey format we capture
> + * YUV422 planar format from the camera and
> + * set the U and V planes to the dummy buffer
> + */
> + if (cap->pix.cfg->fourcc == V4L2_PIX_FMT_GREY) {
> + rkisp1_write(cap->rkisp1,
> + cap->buf.dummy.dma_addr,
> + cap->config->mi.cb_base_ad_init);
> + rkisp1_write(cap->rkisp1,
> + cap->buf.dummy.dma_addr,
> + cap->config->mi.cr_base_ad_init);
> + } else {
> + rkisp1_write(cap->rkisp1,
> + buff_addr[RKISP1_PLANE_CB],
> + cap->config->mi.cb_base_ad_init);
> + rkisp1_write(cap->rkisp1,
> + buff_addr[RKISP1_PLANE_CR],
> + cap->config->mi.cr_base_ad_init);
> + }
> } else {
Looking at this function, I think I would have initialised a local array
of addresses (either to zero, or to the dummy address?) to then set
values when appropriate, and reduce the number of calls to
rkisp1_write() to a single set of three after the processing.
It might make the function simpler, and more readable, but it's more
effort, and this does look like it will solve the greyscale format issue
as discussed in earlier threads so I'd leave it up to you if you want to
refactor.
Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> /*
> * Use the dummy space allocated by dma_alloc_coherent to
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] media: rkisp1: fix grey format iommu page faults
2022-01-17 11:34 ` Kieran Bingham
@ 2022-01-23 9:50 ` Dafna Hirschfeld
2022-01-23 14:32 ` Laurent Pinchart
0 siblings, 1 reply; 7+ messages in thread
From: Dafna Hirschfeld @ 2022-01-23 9:50 UTC (permalink / raw)
To: Kieran Bingham, linux-media
Cc: nicolas.dufresne, laurent.pinchart, hverkuil, kernel, dafna3,
sakari.ailus, mchehab
On 17.01.22 13:34, Kieran Bingham wrote:
> Hi Dafna,
>
> Quoting Dafna Hirschfeld (2021-12-07 11:59:23)
>> Currently capturing grey format produces page faults
>> on both selfpath and mainpath. To support greyscale
>> we can capture YUV422 planar format and configure the U, V
>> buffers to the dummy buffer.
>>
>> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
>> ---
>> This is v2 of the patch "media: rkisp1: remove support for V4L2_PIX_FMT_GREY"
>> In v1 I removed the grey format. In this version it is 'fixed'
>>
>> .../platform/rockchip/rkisp1/rkisp1-capture.c | 28 ++++++++++++++-----
>> 1 file changed, 21 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
>> index 768987d5f2dd..8e982dd0c740 100644
>> --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
>> +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
>> @@ -249,7 +249,7 @@ static const struct rkisp1_capture_fmt_cfg rkisp1_sp_fmts[] = {
>> .fourcc = V4L2_PIX_FMT_GREY,
>> .uv_swap = 0,
>> .write_format = RKISP1_MI_CTRL_SP_WRITE_PLA,
>> - .output_format = RKISP1_MI_CTRL_SP_OUTPUT_YUV400,
>> + .output_format = RKISP1_MI_CTRL_SP_OUTPUT_YUV422,
>> .mbus = MEDIA_BUS_FMT_YUYV8_2X8,
>> },
>> /* rgb */
>> @@ -631,12 +631,26 @@ static void rkisp1_set_next_buf(struct rkisp1_capture *cap)
>> rkisp1_write(cap->rkisp1,
>> buff_addr[RKISP1_PLANE_Y],
>> cap->config->mi.y_base_ad_init);
>> - rkisp1_write(cap->rkisp1,
>> - buff_addr[RKISP1_PLANE_CB],
>> - cap->config->mi.cb_base_ad_init);
>> - rkisp1_write(cap->rkisp1,
>> - buff_addr[RKISP1_PLANE_CR],
>> - cap->config->mi.cr_base_ad_init);
>> + /*
>> + * In order to support grey format we capture
>> + * YUV422 planar format from the camera and
>> + * set the U and V planes to the dummy buffer
>> + */
>> + if (cap->pix.cfg->fourcc == V4L2_PIX_FMT_GREY) {
>> + rkisp1_write(cap->rkisp1,
>> + cap->buf.dummy.dma_addr,
>> + cap->config->mi.cb_base_ad_init);
>> + rkisp1_write(cap->rkisp1,
>> + cap->buf.dummy.dma_addr,
>> + cap->config->mi.cr_base_ad_init);
>> + } else {
>> + rkisp1_write(cap->rkisp1,
>> + buff_addr[RKISP1_PLANE_CB],
>> + cap->config->mi.cb_base_ad_init);
>> + rkisp1_write(cap->rkisp1,
>> + buff_addr[RKISP1_PLANE_CR],
>> + cap->config->mi.cr_base_ad_init);
>> + }
>> } else {
>
> Looking at this function, I think I would have initialised a local array
> of addresses (either to zero, or to the dummy address?) to then set
> values when appropriate, and reduce the number of calls to
> rkisp1_write() to a single set of three after the processing.
>
> It might make the function simpler, and more readable, but it's more
> effort, and this does look like it will solve the greyscale format issue
> as discussed in earlier threads so I'd leave it up to you if you want to
> refactor.
Hi,
Yes, I'll do that.
Interestingly I found out that the patch causing the iommu page fault is
https://www.spinics.net/lists/linux-media/msg176089.html
Before that patch there are no iommu page faults but the video is corrupted.
I can't explain how I didn't find it before, I clearly remember testing the grey format.
Thanks,
Dafna
>
> Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
>
>
>> /*
>> * Use the dummy space allocated by dma_alloc_coherent to
>> --
>> 2.17.1
>>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] media: rkisp1: fix grey format iommu page faults
2022-01-23 9:50 ` Dafna Hirschfeld
@ 2022-01-23 14:32 ` Laurent Pinchart
2022-01-23 16:31 ` Dafna Hirschfeld
0 siblings, 1 reply; 7+ messages in thread
From: Laurent Pinchart @ 2022-01-23 14:32 UTC (permalink / raw)
To: Dafna Hirschfeld
Cc: Kieran Bingham, linux-media, nicolas.dufresne, hverkuil, kernel,
dafna3, sakari.ailus, mchehab
Hi Dafna,
On Sun, Jan 23, 2022 at 11:50:26AM +0200, Dafna Hirschfeld wrote:
> On 17.01.22 13:34, Kieran Bingham wrote:
> > Quoting Dafna Hirschfeld (2021-12-07 11:59:23)
> >> Currently capturing grey format produces page faults
> >> on both selfpath and mainpath. To support greyscale
> >> we can capture YUV422 planar format and configure the U, V
> >> buffers to the dummy buffer.
> >>
> >> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
> >> ---
> >> This is v2 of the patch "media: rkisp1: remove support for V4L2_PIX_FMT_GREY"
> >> In v1 I removed the grey format. In this version it is 'fixed'
> >>
> >> .../platform/rockchip/rkisp1/rkisp1-capture.c | 28 ++++++++++++++-----
> >> 1 file changed, 21 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
> >> index 768987d5f2dd..8e982dd0c740 100644
> >> --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
> >> +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
> >> @@ -249,7 +249,7 @@ static const struct rkisp1_capture_fmt_cfg rkisp1_sp_fmts[] = {
> >> .fourcc = V4L2_PIX_FMT_GREY,
> >> .uv_swap = 0,
> >> .write_format = RKISP1_MI_CTRL_SP_WRITE_PLA,
> >> - .output_format = RKISP1_MI_CTRL_SP_OUTPUT_YUV400,
> >> + .output_format = RKISP1_MI_CTRL_SP_OUTPUT_YUV422,
> >> .mbus = MEDIA_BUS_FMT_YUYV8_2X8,
> >> },
> >> /* rgb */
> >> @@ -631,12 +631,26 @@ static void rkisp1_set_next_buf(struct rkisp1_capture *cap)
> >> rkisp1_write(cap->rkisp1,
> >> buff_addr[RKISP1_PLANE_Y],
> >> cap->config->mi.y_base_ad_init);
> >> - rkisp1_write(cap->rkisp1,
> >> - buff_addr[RKISP1_PLANE_CB],
> >> - cap->config->mi.cb_base_ad_init);
> >> - rkisp1_write(cap->rkisp1,
> >> - buff_addr[RKISP1_PLANE_CR],
> >> - cap->config->mi.cr_base_ad_init);
> >> + /*
> >> + * In order to support grey format we capture
> >> + * YUV422 planar format from the camera and
> >> + * set the U and V planes to the dummy buffer
> >> + */
> >> + if (cap->pix.cfg->fourcc == V4L2_PIX_FMT_GREY) {
> >> + rkisp1_write(cap->rkisp1,
> >> + cap->buf.dummy.dma_addr,
> >> + cap->config->mi.cb_base_ad_init);
> >> + rkisp1_write(cap->rkisp1,
> >> + cap->buf.dummy.dma_addr,
> >> + cap->config->mi.cr_base_ad_init);
> >> + } else {
> >> + rkisp1_write(cap->rkisp1,
> >> + buff_addr[RKISP1_PLANE_CB],
> >> + cap->config->mi.cb_base_ad_init);
> >> + rkisp1_write(cap->rkisp1,
> >> + buff_addr[RKISP1_PLANE_CR],
> >> + cap->config->mi.cr_base_ad_init);
> >> + }
> >> } else {
> >
> > Looking at this function, I think I would have initialised a local array
> > of addresses (either to zero, or to the dummy address?) to then set
> > values when appropriate, and reduce the number of calls to
> > rkisp1_write() to a single set of three after the processing.
> >
> > It might make the function simpler, and more readable, but it's more
> > effort, and this does look like it will solve the greyscale format issue
> > as discussed in earlier threads so I'd leave it up to you if you want to
> > refactor.
>
> Hi,
> Yes, I'll do that.
> Interestingly I found out that the patch causing the iommu page fault is
>
> https://www.spinics.net/lists/linux-media/msg176089.html
>
> Before that patch there are no iommu page faults but the video is corrupted.
>
> I can't explain how I didn't find it before, I clearly remember testing the grey format.
It seems really weird indeed.
Are you getting IOMMU faults on both the main and self paths ?
> > Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> >
> >> /*
> >> * Use the dummy space allocated by dma_alloc_coherent to
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] media: rkisp1: fix grey format iommu page faults
2022-01-23 14:32 ` Laurent Pinchart
@ 2022-01-23 16:31 ` Dafna Hirschfeld
2022-01-23 16:55 ` Dafna Hirschfeld
0 siblings, 1 reply; 7+ messages in thread
From: Dafna Hirschfeld @ 2022-01-23 16:31 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Kieran Bingham, linux-media, nicolas.dufresne, hverkuil, kernel,
dafna3, sakari.ailus, mchehab
On 23.01.22 16:32, Laurent Pinchart wrote:
> Hi Dafna,
>
> On Sun, Jan 23, 2022 at 11:50:26AM +0200, Dafna Hirschfeld wrote:
>> On 17.01.22 13:34, Kieran Bingham wrote:
>>> Quoting Dafna Hirschfeld (2021-12-07 11:59:23)
>>>> Currently capturing grey format produces page faults
>>>> on both selfpath and mainpath. To support greyscale
>>>> we can capture YUV422 planar format and configure the U, V
>>>> buffers to the dummy buffer.
>>>>
>>>> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
>>>> ---
>>>> This is v2 of the patch "media: rkisp1: remove support for V4L2_PIX_FMT_GREY"
>>>> In v1 I removed the grey format. In this version it is 'fixed'
>>>>
>>>> .../platform/rockchip/rkisp1/rkisp1-capture.c | 28 ++++++++++++++-----
>>>> 1 file changed, 21 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
>>>> index 768987d5f2dd..8e982dd0c740 100644
>>>> --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
>>>> +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
>>>> @@ -249,7 +249,7 @@ static const struct rkisp1_capture_fmt_cfg rkisp1_sp_fmts[] = {
>>>> .fourcc = V4L2_PIX_FMT_GREY,
>>>> .uv_swap = 0,
>>>> .write_format = RKISP1_MI_CTRL_SP_WRITE_PLA,
>>>> - .output_format = RKISP1_MI_CTRL_SP_OUTPUT_YUV400,
>>>> + .output_format = RKISP1_MI_CTRL_SP_OUTPUT_YUV422,
>>>> .mbus = MEDIA_BUS_FMT_YUYV8_2X8,
>>>> },
>>>> /* rgb */
>>>> @@ -631,12 +631,26 @@ static void rkisp1_set_next_buf(struct rkisp1_capture *cap)
>>>> rkisp1_write(cap->rkisp1,
>>>> buff_addr[RKISP1_PLANE_Y],
>>>> cap->config->mi.y_base_ad_init);
>>>> - rkisp1_write(cap->rkisp1,
>>>> - buff_addr[RKISP1_PLANE_CB],
>>>> - cap->config->mi.cb_base_ad_init);
>>>> - rkisp1_write(cap->rkisp1,
>>>> - buff_addr[RKISP1_PLANE_CR],
>>>> - cap->config->mi.cr_base_ad_init);
>>>> + /*
>>>> + * In order to support grey format we capture
>>>> + * YUV422 planar format from the camera and
>>>> + * set the U and V planes to the dummy buffer
>>>> + */
>>>> + if (cap->pix.cfg->fourcc == V4L2_PIX_FMT_GREY) {
>>>> + rkisp1_write(cap->rkisp1,
>>>> + cap->buf.dummy.dma_addr,
>>>> + cap->config->mi.cb_base_ad_init);
>>>> + rkisp1_write(cap->rkisp1,
>>>> + cap->buf.dummy.dma_addr,
>>>> + cap->config->mi.cr_base_ad_init);
>>>> + } else {
>>>> + rkisp1_write(cap->rkisp1,
>>>> + buff_addr[RKISP1_PLANE_CB],
>>>> + cap->config->mi.cb_base_ad_init);
>>>> + rkisp1_write(cap->rkisp1,
>>>> + buff_addr[RKISP1_PLANE_CR],
>>>> + cap->config->mi.cr_base_ad_init);
>>>> + }
>>>> } else {
>>>
>>> Looking at this function, I think I would have initialised a local array
>>> of addresses (either to zero, or to the dummy address?) to then set
>>> values when appropriate, and reduce the number of calls to
>>> rkisp1_write() to a single set of three after the processing.
>>>
>>> It might make the function simpler, and more readable, but it's more
>>> effort, and this does look like it will solve the greyscale format issue
>>> as discussed in earlier threads so I'd leave it up to you if you want to
>>> refactor.
>>
>> Hi,
>> Yes, I'll do that.
>> Interestingly I found out that the patch causing the iommu page fault is
>>
>> https://www.spinics.net/lists/linux-media/msg176089.html
>>
>> Before that patch there are no iommu page faults but the video is corrupted.
>>
>> I can't explain how I didn't find it before, I clearly remember testing the grey format.
>
> It seems really weird indeed.
>
> Are you getting IOMMU faults on both the main and self paths ?
Yes, both pathes. I get the page faults when checking out to this commit.
Maybe when I tested back then, the iommu was somehow disabled?
>
>>> Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
>>>
>>>> /*
>>>> * Use the dummy space allocated by dma_alloc_coherent to
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] media: rkisp1: fix grey format iommu page faults
2022-01-23 16:31 ` Dafna Hirschfeld
@ 2022-01-23 16:55 ` Dafna Hirschfeld
2022-01-31 15:31 ` Dafna Hirschfeld
0 siblings, 1 reply; 7+ messages in thread
From: Dafna Hirschfeld @ 2022-01-23 16:55 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Kieran Bingham, linux-media, nicolas.dufresne, hverkuil, kernel,
dafna3, sakari.ailus, mchehab
On 23.01.22 18:31, Dafna Hirschfeld wrote:
>
>
> On 23.01.22 16:32, Laurent Pinchart wrote:
>> Hi Dafna,
>>
>> On Sun, Jan 23, 2022 at 11:50:26AM +0200, Dafna Hirschfeld wrote:
>>> On 17.01.22 13:34, Kieran Bingham wrote:
>>>> Quoting Dafna Hirschfeld (2021-12-07 11:59:23)
>>>>> Currently capturing grey format produces page faults
>>>>> on both selfpath and mainpath. To support greyscale
>>>>> we can capture YUV422 planar format and configure the U, V
>>>>> buffers to the dummy buffer.
>>>>>
>>>>> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
>>>>> ---
>>>>> This is v2 of the patch "media: rkisp1: remove support for V4L2_PIX_FMT_GREY"
>>>>> In v1 I removed the grey format. In this version it is 'fixed'
>>>>>
>>>>> .../platform/rockchip/rkisp1/rkisp1-capture.c | 28 ++++++++++++++-----
>>>>> 1 file changed, 21 insertions(+), 7 deletions(-)
>>>>>
>>>>> diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
>>>>> index 768987d5f2dd..8e982dd0c740 100644
>>>>> --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
>>>>> +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
>>>>> @@ -249,7 +249,7 @@ static const struct rkisp1_capture_fmt_cfg rkisp1_sp_fmts[] = {
>>>>> .fourcc = V4L2_PIX_FMT_GREY,
>>>>> .uv_swap = 0,
>>>>> .write_format = RKISP1_MI_CTRL_SP_WRITE_PLA,
>>>>> - .output_format = RKISP1_MI_CTRL_SP_OUTPUT_YUV400,
>>>>> + .output_format = RKISP1_MI_CTRL_SP_OUTPUT_YUV422,
>>>>> .mbus = MEDIA_BUS_FMT_YUYV8_2X8,
>>>>> },
>>>>> /* rgb */
>>>>> @@ -631,12 +631,26 @@ static void rkisp1_set_next_buf(struct rkisp1_capture *cap)
>>>>> rkisp1_write(cap->rkisp1,
>>>>> buff_addr[RKISP1_PLANE_Y],
>>>>> cap->config->mi.y_base_ad_init);
>>>>> - rkisp1_write(cap->rkisp1,
>>>>> - buff_addr[RKISP1_PLANE_CB],
>>>>> - cap->config->mi.cb_base_ad_init);
>>>>> - rkisp1_write(cap->rkisp1,
>>>>> - buff_addr[RKISP1_PLANE_CR],
>>>>> - cap->config->mi.cr_base_ad_init);
>>>>> + /*
>>>>> + * In order to support grey format we capture
>>>>> + * YUV422 planar format from the camera and
>>>>> + * set the U and V planes to the dummy buffer
>>>>> + */
>>>>> + if (cap->pix.cfg->fourcc == V4L2_PIX_FMT_GREY) {
>>>>> + rkisp1_write(cap->rkisp1,
>>>>> + cap->buf.dummy.dma_addr,
>>>>> + cap->config->mi.cb_base_ad_init);
>>>>> + rkisp1_write(cap->rkisp1,
>>>>> + cap->buf.dummy.dma_addr,
>>>>> + cap->config->mi.cr_base_ad_init);
>>>>> + } else {
>>>>> + rkisp1_write(cap->rkisp1,
>>>>> + buff_addr[RKISP1_PLANE_CB],
>>>>> + cap->config->mi.cb_base_ad_init);
>>>>> + rkisp1_write(cap->rkisp1,
>>>>> + buff_addr[RKISP1_PLANE_CR],
>>>>> + cap->config->mi.cr_base_ad_init);
>>>>> + }
>>>>> } else {
>>>>
>>>> Looking at this function, I think I would have initialised a local array
>>>> of addresses (either to zero, or to the dummy address?) to then set
>>>> values when appropriate, and reduce the number of calls to
>>>> rkisp1_write() to a single set of three after the processing.
>>>>
>>>> It might make the function simpler, and more readable, but it's more
>>>> effort, and this does look like it will solve the greyscale format issue
>>>> as discussed in earlier threads so I'd leave it up to you if you want to
>>>> refactor.
>>>
>>> Hi,
>>> Yes, I'll do that.
>>> Interestingly I found out that the patch causing the iommu page fault is
>>>
>>> https://www.spinics.net/lists/linux-media/msg176089.html
>>>
>>> Before that patch there are no iommu page faults but the video is corrupted.
>>>
>>> I can't explain how I didn't find it before, I clearly remember testing the grey format.
>>
>> It seems really weird indeed.
>>
>> Are you getting IOMMU faults on both the main and self paths ?
>
> Yes, both pathes. I get the page faults when checking out to this commit.
> Maybe when I tested back then, the iommu was somehow disabled?
I see now that it works "fine" if CONFIG_ROCKCHIP_IOMMU is turned off, so
it might be that I tested without iommu, though it is set in arch/arm64/configs/defconfig
>
>>
>>>> Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
>>>>
>>>>> /*
>>>>> * Use the dummy space allocated by dma_alloc_coherent to
>>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] media: rkisp1: fix grey format iommu page faults
2022-01-23 16:55 ` Dafna Hirschfeld
@ 2022-01-31 15:31 ` Dafna Hirschfeld
0 siblings, 0 replies; 7+ messages in thread
From: Dafna Hirschfeld @ 2022-01-31 15:31 UTC (permalink / raw)
To: Laurent Pinchart, Kieran Bingham
Cc: linux-media, nicolas.dufresne, hverkuil, kernel, dafna3,
sakari.ailus, mchehab
On 23.01.22 18:55, Dafna Hirschfeld wrote:
>
>
> On 23.01.22 18:31, Dafna Hirschfeld wrote:
>>
>>
>> On 23.01.22 16:32, Laurent Pinchart wrote:
>>> Hi Dafna,
>>>
>>> On Sun, Jan 23, 2022 at 11:50:26AM +0200, Dafna Hirschfeld wrote:
>>>> On 17.01.22 13:34, Kieran Bingham wrote:
>>>>> Quoting Dafna Hirschfeld (2021-12-07 11:59:23)
>>>>>> Currently capturing grey format produces page faults
>>>>>> on both selfpath and mainpath. To support greyscale
>>>>>> we can capture YUV422 planar format and configure the U, V
>>>>>> buffers to the dummy buffer.
>>>>>>
>>>>>> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>
>>>>>> ---
>>>>>> This is v2 of the patch "media: rkisp1: remove support for V4L2_PIX_FMT_GREY"
>>>>>> In v1 I removed the grey format. In this version it is 'fixed'
>>>>>>
>>>>>> .../platform/rockchip/rkisp1/rkisp1-capture.c | 28 ++++++++++++++-----
>>>>>> 1 file changed, 21 insertions(+), 7 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
>>>>>> index 768987d5f2dd..8e982dd0c740 100644
>>>>>> --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
>>>>>> +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
>>>>>> @@ -249,7 +249,7 @@ static const struct rkisp1_capture_fmt_cfg rkisp1_sp_fmts[] = {
>>>>>> .fourcc = V4L2_PIX_FMT_GREY,
>>>>>> .uv_swap = 0,
>>>>>> .write_format = RKISP1_MI_CTRL_SP_WRITE_PLA,
>>>>>> - .output_format = RKISP1_MI_CTRL_SP_OUTPUT_YUV400,
>>>>>> + .output_format = RKISP1_MI_CTRL_SP_OUTPUT_YUV422,
>>>>>> .mbus = MEDIA_BUS_FMT_YUYV8_2X8,
>>>>>> },
>>>>>> /* rgb */
>>>>>> @@ -631,12 +631,26 @@ static void rkisp1_set_next_buf(struct rkisp1_capture *cap)
>>>>>> rkisp1_write(cap->rkisp1,
>>>>>> buff_addr[RKISP1_PLANE_Y],
>>>>>> cap->config->mi.y_base_ad_init);
>>>>>> - rkisp1_write(cap->rkisp1,
>>>>>> - buff_addr[RKISP1_PLANE_CB],
>>>>>> - cap->config->mi.cb_base_ad_init);
>>>>>> - rkisp1_write(cap->rkisp1,
>>>>>> - buff_addr[RKISP1_PLANE_CR],
>>>>>> - cap->config->mi.cr_base_ad_init);
>>>>>> + /*
>>>>>> + * In order to support grey format we capture
>>>>>> + * YUV422 planar format from the camera and
>>>>>> + * set the U and V planes to the dummy buffer
>>>>>> + */
>>>>>> + if (cap->pix.cfg->fourcc == V4L2_PIX_FMT_GREY) {
>>>>>> + rkisp1_write(cap->rkisp1,
>>>>>> + cap->buf.dummy.dma_addr,
>>>>>> + cap->config->mi.cb_base_ad_init);
>>>>>> + rkisp1_write(cap->rkisp1,
>>>>>> + cap->buf.dummy.dma_addr,
>>>>>> + cap->config->mi.cr_base_ad_init);
>>>>>> + } else {
>>>>>> + rkisp1_write(cap->rkisp1,
>>>>>> + buff_addr[RKISP1_PLANE_CB],
>>>>>> + cap->config->mi.cb_base_ad_init);
>>>>>> + rkisp1_write(cap->rkisp1,
>>>>>> + buff_addr[RKISP1_PLANE_CR],
>>>>>> + cap->config->mi.cr_base_ad_init);
>>>>>> + }
>>>>>> } else {
>>>>>
>>>>> Looking at this function, I think I would have initialised a local array
>>>>> of addresses (either to zero, or to the dummy address?) to then set
>>>>> values when appropriate, and reduce the number of calls to
>>>>> rkisp1_write() to a single set of three after the processing.
>>>>>
>>>>> It might make the function simpler, and more readable, but it's more
>>>>> effort, and this does look like it will solve the greyscale format issue
>>>>> as discussed in earlier threads so I'd leave it up to you if you want to
>>>>> refactor.
>>>>
>>>> Hi,
>>>> Yes, I'll do that.
>>>> Interestingly I found out that the patch causing the iommu page fault is
>>>>
>>>> https://www.spinics.net/lists/linux-media/msg176089.html
>>>>
>>>> Before that patch there are no iommu page faults but the video is corrupted.
>>>>
>>>> I can't explain how I didn't find it before, I clearly remember testing the grey format.
>>>
>>> It seems really weird indeed.
>>>
>>> Are you getting IOMMU faults on both the main and self paths ?
>>
>> Yes, both pathes. I get the page faults when checking out to this commit.
>> Maybe when I tested back then, the iommu was somehow disabled?
>
> I see now that it works "fine" if CONFIG_ROCKCHIP_IOMMU is turned off, so
> it might be that I tested without iommu, though it is set in arch/arm64/configs/defconfig
I have a script that iterates all supported formats and stream them.
I see that when running the script, streaming gray format succeed, but when
testing grey format alone I get those page faults. I guess somehow buffers happen
to be mapped when running the script so it misses the faults.
>
>>
>>>
>>>>> Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
>>>>>
>>>>>> /*
>>>>>> * Use the dummy space allocated by dma_alloc_coherent to
>>>
>>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-01-31 15:31 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-07 11:59 [PATCH v2] media: rkisp1: fix grey format iommu page faults Dafna Hirschfeld
2022-01-17 11:34 ` Kieran Bingham
2022-01-23 9:50 ` Dafna Hirschfeld
2022-01-23 14:32 ` Laurent Pinchart
2022-01-23 16:31 ` Dafna Hirschfeld
2022-01-23 16:55 ` Dafna Hirschfeld
2022-01-31 15:31 ` Dafna Hirschfeld
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).