Re: [PATCH v3 00/14] media: rkisp1: Add support for i.MX8MP

[Adam Ford <aford173@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 6111 bytes --]

On Wed, Feb 15, 2023 at 5:55 PM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Adam,
>
> On Wed, Feb 15, 2023 at 07:57:53AM -0600, Adam Ford wrote:
> > On Fri, Nov 18, 2022 at 3:44 AM Paul Elder wrote:
> >
> > > This series depends on v3 of "dt-bindings: media: Add macros for video
> > > interface bus types" [1].
> > >
> > > This series extends the rkisp1 driver to support the ISP found in the
> > > NXP i.MX8MP SoC.
> >
> > I'm going to spend some time testing this over the weekend.  Is there a V4
> > pending, or should I just test whatever is in Laurent's repo?
>
> I've updated all the v6.2-based branches on
> https://gitlab.com/ideasonboard/nxp/linux.git (and renamed them to
> v6.2/*). Beside a rebase, the v6.2/isp branch contains (at the bottom) 6
> additional patches that I've previously posted to the linux-media
> mailing list (feel free to review them ;-)).

I grabbed your v6.2 series, and applied some updates to enable an
imx219 camera and routed it through the ISP and configured the camera
to SRGGB10_1X10/640x480 and had the ISP convert to YUYV8_2X8/640x480
and it captured just fine.

With that, I think you can add

Tested-by: Adam Ford <aford173@gmail.com> #imx8mp-beacon

I haven't experimented with the resizer yet,but  I did have some
questions on the AWB.  The AWB appears to be available on the 8MP per
the TRM, and  I see reference to AWB in the driver, but when I query
the subdev via yavta, I didn't see anything obvious on how to enable
it.  My pipeline (attached) shows klisp1_params as video2 and
rkisp1_stats as video1.  I attempted to query both without much
success.

root@beacon-imx8mp-kit:~# yavta -l /dev/video2
Device /dev/video2 opened.
Device `rkisp1_params' on `platform:rkisp1' (driver 'rkisp1') supports
meta-data, output, without mplanes.
unable to query control 0xc0000000: Inappropriate ioctl for device (25).
Meta-data format: RK1P (50314b52) buffer size 3048
root@beacon-imx8mp-kit:~# yavta -l /dev/video1
Device /dev/video1 opened.
Device `rkisp1_stats' on `platform:rkisp1' (driver 'rkisp1') supports
meta-data, capture, without mplanes.
unable to query control 0xc0000000: Inappropriate ioctl for device (25).
Meta-data format: RK1S (53314b52) buffer size 260
root@beacon-imx8mp-kit:~#

Is there documentation somewhere on where to test the AWB?  This is of
particular interest to me, because the RGGB format of the camera comes
across with a green tint.  I am able to remove this green-ness on a
different platform using some AWB on the ARM, but I'd rather do it in
hardware if possible.

Thanks

adam


>
> My only concern with this series is with patch "media: rkisp1: Add match
> data for i.MX8MP ISP", and in particular with the following hunk:
>
>  enum rkisp1_cif_isp_version {
>         RKISP1_V10 = 10,
>         RKISP1_V11,
>         RKISP1_V12,
>         RKISP1_V13,
> +       IMX8MP_V10,
>  };
>
> It's not a very nice versioning scheme :-S I'll see if I can find
> something better, but regardless of that, I'll post v4 with the goal of
> merging it in v6.4.
>
> > I have an IMX219 camera with 4-lane support and an i.MX8M Plus kit from
> > Beacon, and I want to test the RGGB bayer conversion to see how well it
> > works.
> >
> > > The ISP IP cores in the Rockchip RK3399 (known as the "Rockchip ISP1")
> > > and in the NXP i.MX8MP have the same origin, and have slightly diverged
> > > over time as they are now independently developed (afaik) by Rockchip
> > > and VeriSilicon. The latter is marketed under the name "ISP8000Nano",
> > > and is close enough to the RK3399 ISP that it can easily be supported by
> > > the same driver.
> >
> > Is there a reason the driver cannot be renamed to a more generic name than
> > rkisp1 if the Rockchip and VeriSilicon had similar origins?  Having the
> > name Rockchip referenced from an NXP i.MX8M Plus seems odd to me.
>
> The common roots of the IP core predate both Rockchip and VeriSilicon.
> Those two implementations have now diverged (as with all forks), so
> either name would be wrong in some cases :-S
>
> > > The last two patches add support for UYVY output format, which can be
> > > implemented on the ISP version in the i.MX8MP but not in the one in the
> > > RK3399.
> > >
> > > This version of the series specifically has been tested on a Polyhex
> > > Debix model A with an imx219 (Raspberry Pi cam v2).
> > >
> > > [1] https://lore.kernel.org/linux-media/20220615221410.27459-2-laurent.pinchart@ideasonboard.com/
> > >
> > > Laurent Pinchart (3):
> > >   dt-bindings: media: rkisp1: Add i.MX8MP ISP example
> > >   media: rkisp1: Add and use rkisp1_has_feature() macro
> > >   media: rkisp1: Configure gasket on i.MX8MP
> > >
> > > Paul Elder (11):
> > >   dt-bindings: media: rkisp1: Add i.MX8MP ISP to compatible
> > >   media: rkisp1: Add match data for i.MX8MP ISP
> > >   media: rkisp1: Add and set registers for crop for i.MX8MP
> > >   media: rkisp1: Add and set registers for output size config on i.MX8MP
> > >   media: rkisp1: Add i.MX8MP-specific registers for MI and resizer
> > >   media: rkisp1: Shift DMA buffer addresses on i.MX8MP
> > >   media: rkisp1: Add register definitions for the test pattern generator
> > >   media: rkisp1: Fix RSZ_CTRL bits for i.MX8MP
> > >   media: rkisp1: Support devices without self path
> > >   media: rkisp1: Add YC swap capability
> > >   media: rkisp1: Add UYVY as an output format
> > >
> > >  .../bindings/media/rockchip-isp1.yaml         |  79 ++++++++++-
> > >  .../platform/rockchip/rkisp1/rkisp1-capture.c | 102 +++++++++++---
> > >  .../platform/rockchip/rkisp1/rkisp1-common.h  |  32 +++++
> > >  .../platform/rockchip/rkisp1/rkisp1-debug.c   |  14 +-
> > >  .../platform/rockchip/rkisp1/rkisp1-dev.c     |  67 +++++++--
> > >  .../platform/rockchip/rkisp1/rkisp1-isp.c     | 128 +++++++++++++++++-
> > >  .../platform/rockchip/rkisp1/rkisp1-regs.h    |  90 ++++++++++++
> > >  .../platform/rockchip/rkisp1/rkisp1-resizer.c |  35 ++++-
> > >  include/uapi/linux/rkisp1-config.h            |   2 +
> > >  9 files changed, 509 insertions(+), 40 deletions(-)
>
> --
> Regards,
>
> Laurent Pinchart

[-- Attachment #2: imx8mp-beacon.png --]
[-- Type: image/png, Size: 35350 bytes --]