On Tue, Sep 14, 2021 at 02:11:18PM +0300, Laurent Pinchart wrote:

Hi Paul,

On Tue, Sep 14, 2021 at 09:50:41AM +0200, Paul Kocialkowski wrote:

quoted

On Mon 13 Sep 21, 10:31, Maxime Ripard wrote:

quoted

On Fri, Sep 10, 2021 at 08:41:45PM +0200, Paul Kocialkowski wrote:

quoted

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.

Maxime is right in the sense that uAPI can always be extended, but it
has to be done in a backward-compatible manner, and staging is sometimes
considered as not being covered by the ABI stability requirements of the
kernel. Not everybody agrees on this, but there are clear cases where
userspace really can't expect staging ABIs to be stable (for instance
when the driver doesn't even compile).

I think there's value in having the driver in staging to facilitate
development until we consider the ABI stable, but I'm not entirely sure
if there should be another step taken to mark this ABI is not being
ready yet.

The rule seems to be about whether or not the user-space gets broken in
the process:

https://lore.kernel.org/lkml/CAHk-=wiVi7mSrsMP=fLXQrXK_UimybW=ziLOwSzFTtoXUacWVQ@mail.gmail.com/ (local)

Something that wouldn't compile cannot generate a regression, since it
never worked in the first place. Changing the semantic of an ioctl does.

Maxime