[libcamera-devel] [PATCH 3/5] uapi: raspberrypi: Update the bcm2835-isp header definition

[Kieran Bingham]

Hi David, Naush,

On 20/01/2021 10:04, Naushir Patuck wrote:
> Hi David,
> 
> Thank you for your review feedback.
> 
> On Wed, 20 Jan 2021 at 10:00, David Plowman
> <david.plowman at raspberrypi.com <mailto:david.plowman at raspberrypi.com>>
> wrote:
> 
>     Hi Naush
> 
>     Thanks for this patch.
> 
>     On Wed, 20 Jan 2021 at 08:35, Naushir Patuck <naush at raspberrypi.com
>     <mailto:naush at raspberrypi.com>> wrote:
>     >
>     > Update the bcm2835-isp.h file with the latest version available in the
>     > downstream Raspberryp Pi linux repo.

s/Raspberryp/Raspberry/

>     >
>     > This change adds support for colour denoise processing, and the
>     > following downstream kernel changes must be available:
>     > https://github.com/raspberrypi/linux/pull/4069
>     > https://github.com/raspberrypi/linux/pull/4083
>     >
>     > The Raspberry Pi image must also be running the latest firmware,
>     > obtained by running rpi-update.
>     >
>     > Signed-off-by: Naushir Patuck <naush at raspberrypi.com
>     <mailto:naush at raspberrypi.com>>
>     > ---
>     >  include/linux/bcm2835-isp.h | 32 +++++++++++++++++++++++++++++---
>     >  1 file changed, 29 insertions(+), 3 deletions(-)
>     >
>     > diff --git a/include/linux/bcm2835-isp.h b/include/linux/bcm2835-isp.h
>     > index 45abb681517e..94c3af947883 100644
>     > --- a/include/linux/bcm2835-isp.h
>     > +++ b/include/linux/bcm2835-isp.h
>     > @@ -31,7 +31,8 @@
>     >                                 (V4L2_CID_USER_BCM2835_ISP_BASE +
>     0x0007)
>     >  #define V4L2_CID_USER_BCM2835_ISP_DPC          \
>     >                                 (V4L2_CID_USER_BCM2835_ISP_BASE +
>     0x0008)
>     > -
>     > +#define V4L2_CID_USER_BCM2835_ISP_CDN \
>     > +                               (V4L2_CID_USER_BCM2835_ISP_BASE +
>     0x0009)
>     >  /*
>     >   * All structs below are directly mapped onto the equivalent
>     structs in
>     >   * drivers/staging/vc04_services/vchiq-mmal/mmal-parameters.h
>     > @@ -46,7 +47,7 @@
>     >   */
>     >  struct bcm2835_isp_rational {
>     >         __s32 num;
>     > -       __s32 den;
>     > +       __u32 den;
> 
>     Pickier people than myself might comment that this seems unrelated,
>     though I agree it looks sensible!
> 
> 
> In the commit message I mentioned that this is a port of the latest
> version of the uapi header for this very reason :-)  This includes all
> changes upto and including the new cdn definitions.  Hope folks won't
> find this converversal.
> 

No problem, this is fine.

We have to import kernel headers to be able to ensure we have compile
compatibility.

The import of those headers is a single action, and as long as it
matches the expectations of the kernel that's fine.

Ignore any checkstyle warnings under include/linux/ too. They should be
direct, unmodified imports.

I've got a local patch to ignore imported code that I've toyed with for
checkstyle.py... Now I just need time to make it clean.

Acked-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

> Regards,
> Naush
> 
>  
> 
> 
>     >  };
>     >
>     >  /**
>     > @@ -140,7 +141,7 @@ struct bcm2835_isp_black_level {
>     >         __u16 black_level_r;
>     >         __u16 black_level_g;
>     >         __u16 black_level_b;
>     > -       __u8 pad_[2]; /* Unused */
>     > +       __u8 padding[2]; /* Unused */
> 
>     Ditto!
> 
>     Apart from these small things:
> 
>     Reviewed-by: David Plowman <david.plowman at raspberrypi.com
>     <mailto:david.plowman at raspberrypi.com>>
> 
>     Thanks!
>     David
> 
>     >  };
>     >
>     >  /**
>     > @@ -175,6 +176,31 @@ struct bcm2835_isp_gamma {
>     >         __u16 y[BCM2835_NUM_GAMMA_PTS];
>     >  };
>     >
>     > +/**
>     > + * enum bcm2835_isp_cdn_mode - Mode of operation for colour denoise.
>     > + *
>     > +  * @CDN_MODE_FAST:            Fast (but lower quality) colour
>     denoise
>     > + *                             algorithm, typically used for
>     video recording.
>     > + * @CDN_HIGH_QUALITY:          High quality (but slower) colour
>     denoise
>     > + *                             algorithm, typically used for
>     stills capture.
>     > + */
>     > +enum bcm2835_isp_cdn_mode {
>     > +       CDN_MODE_FAST = 0,
>     > +       CDN_MODE_HIGH_QUALITY = 1,
>     > +};
>     > +
>     > +/**
>     > + * struct bcm2835_isp_cdn - Colour denoise parameters set with the
>     > + *                         V4L2_CID_USER_BCM2835_ISP_CDN ctrl.
>     > + *
>     > + * @enabled:   Enable colour denoise.
>     > + * @cdn_mode:  Colour denoise operating mode (see enum
>     &bcm2835_isp_cdn_mode)
>     > + */
>     > +struct bcm2835_isp_cdn {
>     > +       __u32 enabled;
>     > +       __u32 mode;
>     > +};
>     > +
>     >  /**
>     >   * struct bcm2835_isp_denoise - Denoise parameters set with the
>     >   *                             V4L2_CID_USER_BCM2835_ISP_DENOISE
>     ctrl.
>     > --
>     > 2.25.1
>     >
>     > _______________________________________________
>     > libcamera-devel mailing list
>     > libcamera-devel at lists.libcamera.org
>     <mailto:libcamera-devel at lists.libcamera.org>
>     > https://lists.libcamera.org/listinfo/libcamera-devel
> 
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
> 

-- 
Regards
--
Kieran