| Message ID | 20210120083449.642418-4-naush@raspberrypi.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Naush Thanks for this patch. On Wed, 20 Jan 2021 at 08:35, Naushir Patuck <naush@raspberrypi.com> wrote: > > Update the bcm2835-isp.h file with the latest version available in the > downstream Raspberryp Pi linux repo. > > 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@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! > }; > > /** > @@ -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@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@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hi David, Thank you for your review feedback. On Wed, 20 Jan 2021 at 10:00, David Plowman <david.plowman@raspberrypi.com> wrote: > Hi Naush > > Thanks for this patch. > > On Wed, 20 Jan 2021 at 08:35, Naushir Patuck <naush@raspberrypi.com> > wrote: > > > > Update the bcm2835-isp.h file with the latest version available in the > > downstream Raspberryp Pi linux repo. > > > > 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@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. 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@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@lists.libcamera.org > > https://lists.libcamera.org/listinfo/libcamera-devel >
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@raspberrypi.com <mailto:david.plowman@raspberrypi.com>> > wrote: > > Hi Naush > > Thanks for this patch. > > On Wed, 20 Jan 2021 at 08:35, Naushir Patuck <naush@raspberrypi.com > <mailto:naush@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@raspberrypi.com > <mailto:naush@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@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@raspberrypi.com > <mailto:david.plowman@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@lists.libcamera.org > <mailto:libcamera-devel@lists.libcamera.org> > > https://lists.libcamera.org/listinfo/libcamera-devel > > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel >
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; }; /** @@ -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 */ }; /** @@ -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.
Update the bcm2835-isp.h file with the latest version available in the downstream Raspberryp Pi linux repo. 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@raspberrypi.com> --- include/linux/bcm2835-isp.h | 32 +++++++++++++++++++++++++++++--- 1 file changed, 29 insertions(+), 3 deletions(-)