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@xxxxxxxxxxxxx>
> >> ---
> >> 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@xxxxxxxxxxxxxxxx>
> >
> >> /*
> >> * Use the dummy space allocated by dma_alloc_coherent to
--
Regards,
Laurent Pinchart
