Re: [PATCH 20/22] staging: media: Add support for the Allwinner A31 ISP

[Maxime Ripard]

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:
> > 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.
> 
> 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@xxxxxxxxxxxxxx/

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

Attachment: signature.asc
Description: PGP signature