Message-ID: <YUBUUQxBaGUkjzMP@aptenodytes>
Date:   Tue, 14 Sep 2021 09:50:41 +0200
From:   Paul Kocialkowski <paul.kocialkowski@...tlin.com>
To:     Maxime Ripard <maxime@...no.tech>
Cc:     linux-media@...r.kernel.org, devicetree@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org, linux-sunxi@...ts.linux.dev,
        linux-kernel@...r.kernel.org, linux-phy@...ts.infradead.org,
        linux-clk@...r.kernel.org, linux-staging@...ts.linux.dev,
        Yong Deng <yong.deng@...ewell.com>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Rob Herring <robh+dt@...nel.org>,
        Sakari Ailus <sakari.ailus@...ux.intel.com>,
        Hans Verkuil <hans.verkuil@...co.com>,
        Chen-Yu Tsai <wens@...e.org>,
        Jernej Skrabec <jernej.skrabec@...il.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Helen Koike <helen.koike@...labora.com>,
        Laurent Pinchart <laurent.pinchart@...asonboard.com>,
        Thomas Petazzoni <thomas.petazzoni@...tlin.com>
Subject: Re: [PATCH 20/22] staging: media: Add support for the Allwinner A31
 ISP

Hi,

On Mon 13 Sep 21, 10:31, Maxime Ripard wrote:
> On Fri, Sep 10, 2021 at 08:41:45PM +0200, Paul Kocialkowski wrote:
> > Some Allwinner platforms come with an Image Signal Processor, which
> > supports various features in order to enhance and transform data
> > received by image sensors into good-looking pictures. In most cases,
> > the data is raw bayer, which gets internally converted to RGB and
> > finally YUV, which is what the hardware produces.
> > 
> > This driver supports ISPs that are similar to the A31 ISP, which was
> > the first standalone ISP found in Allwinner platforms. Simpler ISP
> > blocks were found in the A10 and A20, where they are tied to a CSI
> > controller. Newer generations of Allwinner SoCs (starting with the
> > H6, H616, etc) come with a new camera subsystem and revised ISP.
> > Even though these previous and next-generation ISPs are somewhat
> > similar to the A31 ISP, they have enough significant differences to
> > be out of the scope of this driver.
> > 
> > While the ISP supports many features, including 3A and many
> > enhancement blocks, this implementation is limited to the following:
> > - V3s (V3/S3) platform support;
> > - Bayer media bus formats as input;
> > - Semi-planar YUV (NV12/NV21) as output;
> > - Debayering with per-component gain and offset configuration;
> > - 2D noise filtering with configurable coefficients.
> > 
> > Since many features are missing from the associated uAPI, the driver
> > is aimed to integrate staging until all features are properly
> > described.
> 
> We can add new features/interfaces to a !staging driver. Why do you
> think staging is required?

This is true for the driver but not so much for the uAPI, so it seems that
the uAPI must be added to staging in some way. Then I'm not sure it makes sense
to have a !staging driver that depends on a staging uAPI.

Besides that, I added it to staging because that's the process that was
followed by rkisp1, which is a very similar case.

> > On the technical side, it uses the v4l2 and media controller APIs,
> > with a video node for capture, a processor subdev and a video node
> > for parameters submission. A specific uAPI structure and associated
> > v4l2 meta format are used to configure parameters of the supported
> > modules.
> 
> This meta format needs to be documented

You're right, there should probably be a pixfmt-meta-sun6i-isp.rst
documentation file. I guess it should live along in the staging driver
directory for now and be destaged later.

Cheers,

Paul

-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)

Powered by blists - more mailing lists