| Message ID | 20260724-mira220-v2-1-240003749dd0@ideasonboard.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series |
|
| Related | show |
Quoting Jacopo Mondi (2026-07-24 16:47:40) > From: Philippe Baetens <philippebaetens@gmail.com> > > AMS-OSRAM Mira220 is a global shutter sensor with a maximum > resolution of 1600x1400. > > This patch upports the Mira220 support available at > https://github.com/ams-OSRAM/libcamera.git at revision d7d5e17ec961 > ("update json files so they are truly mono") I'd put that after the '---' I don't think it's needed in the commit message. Anyway, Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > Signed-off-by: Philippe Baetens <philippebaetens@gmail.com> > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > Acked-by: Naushir Patuck <naush@raspberrypi.com> > --- > src/ipa/rpi/cam_helper/cam_helper_mira220.cpp | 55 +++++++++++++++++++++++++++ > src/ipa/rpi/cam_helper/meson.build | 1 + > 2 files changed, 56 insertions(+) > > diff --git a/src/ipa/rpi/cam_helper/cam_helper_mira220.cpp b/src/ipa/rpi/cam_helper/cam_helper_mira220.cpp > new file mode 100644 > index 000000000000..f0bd1ecbfff6 > --- /dev/null > +++ b/src/ipa/rpi/cam_helper/cam_helper_mira220.cpp > @@ -0,0 +1,55 @@ > +/* SPDX-License-Identifier: BSD-2-Clause */ > +/* > + * Copyright (C) 2026, ams-OSRAM > + * > + * cam_helper_Mira220.cpp - camera information for Mira220 sensor > + */ > + > +#include <assert.h> > + > +#include "cam_helper.h" > + > +using namespace RPiController; > + > +class CamHelperMira220 : public CamHelper > +{ > +public: > + CamHelperMira220(); > + uint32_t gainCode(double gain) const override; > + double gain(uint32_t gainCode) const override; > + unsigned int hideFramesModeSwitch() const override; > + > +private: > + /* > + * Smallest difference between the frame length and integration time, > + * in units of lines. > + */ > + static constexpr int frameIntegrationDiff = 4; > +}; > + > +/* > + * Mira220 doesn't output metadata, so we have to use the delayed controls which > + * works by counting frames. > + */ > + > +CamHelperMira220::CamHelperMira220() > + : CamHelper({}, frameIntegrationDiff) > +{ > +} > + > +uint32_t CamHelperMira220::gainCode(double gain) const > +{ > + return static_cast<uint32_t>(2048.0 - 2048.0 / gain); > +} > + > +double CamHelperMira220::gain(uint32_t gainCode) const > +{ > + return static_cast<double>(2048.0 / (2048 - gainCode)); > +} > + > +static CamHelper *create() > +{ > + return new CamHelperMira220(); > +} > + > +static RegisterCamHelper reg("mira220", &create); > diff --git a/src/ipa/rpi/cam_helper/meson.build b/src/ipa/rpi/cam_helper/meson.build > index eabd55dce5a1..a8756a1a0226 100644 > --- a/src/ipa/rpi/cam_helper/meson.build > +++ b/src/ipa/rpi/cam_helper/meson.build > @@ -12,6 +12,7 @@ rpi_ipa_cam_helper_sources = files([ > 'cam_helper_imx519.cpp', > 'cam_helper_imx678.cpp', > 'cam_helper_imx708.cpp', > + 'cam_helper_mira220.cpp', > 'cam_helper_ov64a40.cpp', > 'cam_helper_ov7251.cpp', > 'cam_helper_ov9281.cpp', > > -- > 2.54.0 >
Hi Kieran On Fri, Jul 24, 2026 at 05:02:32PM +0100, Kieran Bingham wrote: > Quoting Jacopo Mondi (2026-07-24 16:47:40) > > From: Philippe Baetens <philippebaetens@gmail.com> > > > > AMS-OSRAM Mira220 is a global shutter sensor with a maximum > > resolution of 1600x1400. > > > > This patch upports the Mira220 support available at > > https://github.com/ams-OSRAM/libcamera.git at revision d7d5e17ec961 > > ("update json files so they are truly mono") > > I'd put that after the '---' I don't think it's needed in the commit > message. Isn't it worth to have this information in the commit history ? I would say it is. What do others think ? > > Anyway, > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > Signed-off-by: Philippe Baetens <philippebaetens@gmail.com> > > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > > Acked-by: Naushir Patuck <naush@raspberrypi.com> > > --- > > src/ipa/rpi/cam_helper/cam_helper_mira220.cpp | 55 +++++++++++++++++++++++++++ > > src/ipa/rpi/cam_helper/meson.build | 1 + > > 2 files changed, 56 insertions(+) > > > > diff --git a/src/ipa/rpi/cam_helper/cam_helper_mira220.cpp b/src/ipa/rpi/cam_helper/cam_helper_mira220.cpp > > new file mode 100644 > > index 000000000000..f0bd1ecbfff6 > > --- /dev/null > > +++ b/src/ipa/rpi/cam_helper/cam_helper_mira220.cpp > > @@ -0,0 +1,55 @@ > > +/* SPDX-License-Identifier: BSD-2-Clause */ > > +/* > > + * Copyright (C) 2026, ams-OSRAM > > + * > > + * cam_helper_Mira220.cpp - camera information for Mira220 sensor > > + */ > > + > > +#include <assert.h> > > + > > +#include "cam_helper.h" > > + > > +using namespace RPiController; > > + > > +class CamHelperMira220 : public CamHelper > > +{ > > +public: > > + CamHelperMira220(); > > + uint32_t gainCode(double gain) const override; > > + double gain(uint32_t gainCode) const override; > > + unsigned int hideFramesModeSwitch() const override; > > + > > +private: > > + /* > > + * Smallest difference between the frame length and integration time, > > + * in units of lines. > > + */ > > + static constexpr int frameIntegrationDiff = 4; > > +}; > > + > > +/* > > + * Mira220 doesn't output metadata, so we have to use the delayed controls which > > + * works by counting frames. > > + */ > > + > > +CamHelperMira220::CamHelperMira220() > > + : CamHelper({}, frameIntegrationDiff) > > +{ > > +} > > + > > +uint32_t CamHelperMira220::gainCode(double gain) const > > +{ > > + return static_cast<uint32_t>(2048.0 - 2048.0 / gain); > > +} > > + > > +double CamHelperMira220::gain(uint32_t gainCode) const > > +{ > > + return static_cast<double>(2048.0 / (2048 - gainCode)); > > +} > > + > > +static CamHelper *create() > > +{ > > + return new CamHelperMira220(); > > +} > > + > > +static RegisterCamHelper reg("mira220", &create); > > diff --git a/src/ipa/rpi/cam_helper/meson.build b/src/ipa/rpi/cam_helper/meson.build > > index eabd55dce5a1..a8756a1a0226 100644 > > --- a/src/ipa/rpi/cam_helper/meson.build > > +++ b/src/ipa/rpi/cam_helper/meson.build > > @@ -12,6 +12,7 @@ rpi_ipa_cam_helper_sources = files([ > > 'cam_helper_imx519.cpp', > > 'cam_helper_imx678.cpp', > > 'cam_helper_imx708.cpp', > > + 'cam_helper_mira220.cpp', > > 'cam_helper_ov64a40.cpp', > > 'cam_helper_ov7251.cpp', > > 'cam_helper_ov9281.cpp', > > > > -- > > 2.54.0 > >
On Fri, Jul 31, 2026 at 09:31:54AM +0200, Jacopo Mondi wrote: > On Fri, Jul 24, 2026 at 05:02:32PM +0100, Kieran Bingham wrote: > > Quoting Jacopo Mondi (2026-07-24 16:47:40) > > > From: Philippe Baetens <philippebaetens@gmail.com> > > > > > > AMS-OSRAM Mira220 is a global shutter sensor with a maximum > > > resolution of 1600x1400. > > > > > > This patch upports the Mira220 support available at > > > https://github.com/ams-OSRAM/libcamera.git at revision d7d5e17ec961 You can also include the commit ID in the URL, that would be easier to copy & paste: https://github.com/ams-OSRAM/libcamera/commit/d7d5e17ec961d3f1d2b217217b70ec242e6d8bd4 > > > ("update json files so they are truly mono") > > > > I'd put that after the '---' I don't think it's needed in the commit > > message. > > Isn't it worth to have this information in the commit history ? I > would say it is. What do others think ? I would probably have put the information below a --- line, but I don't have a strong preference in this case. > > Anyway, > > > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > > > Signed-off-by: Philippe Baetens <philippebaetens@gmail.com> > > > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > > > Acked-by: Naushir Patuck <naush@raspberrypi.com> > > > --- > > > src/ipa/rpi/cam_helper/cam_helper_mira220.cpp | 55 +++++++++++++++++++++++++++ > > > src/ipa/rpi/cam_helper/meson.build | 1 + > > > 2 files changed, 56 insertions(+) > > > > > > diff --git a/src/ipa/rpi/cam_helper/cam_helper_mira220.cpp b/src/ipa/rpi/cam_helper/cam_helper_mira220.cpp > > > new file mode 100644 > > > index 000000000000..f0bd1ecbfff6 > > > --- /dev/null > > > +++ b/src/ipa/rpi/cam_helper/cam_helper_mira220.cpp > > > @@ -0,0 +1,55 @@ > > > +/* SPDX-License-Identifier: BSD-2-Clause */ > > > +/* > > > + * Copyright (C) 2026, ams-OSRAM > > > + * > > > + * cam_helper_Mira220.cpp - camera information for Mira220 sensor Drop the file name. > > > + */ > > > + > > > +#include <assert.h> > > > + > > > +#include "cam_helper.h" > > > + > > > +using namespace RPiController; > > > + > > > +class CamHelperMira220 : public CamHelper > > > +{ > > > +public: > > > + CamHelperMira220(); > > > + uint32_t gainCode(double gain) const override; > > > + double gain(uint32_t gainCode) const override; > > > + unsigned int hideFramesModeSwitch() const override; > > > + > > > +private: > > > + /* > > > + * Smallest difference between the frame length and integration time, > > > + * in units of lines. > > > + */ > > > + static constexpr int frameIntegrationDiff = 4; I don't think this is correct. The datasheet (v9.00) states on page 39 tEXP,max = Tframe - tGLOB where tGLOB is 1928 * TCLK_IN. > > > +}; > > > + > > > +/* > > > + * Mira220 doesn't output metadata, so we have to use the delayed controls which > > > + * works by counting frames. > > > + */ I would have put that in the commit message. > > > + > > > +CamHelperMira220::CamHelperMira220() > > > + : CamHelper({}, frameIntegrationDiff) > > > +{ > > > +} > > > + > > > +uint32_t CamHelperMira220::gainCode(double gain) const > > > +{ > > > + return static_cast<uint32_t>(2048.0 - 2048.0 / gain); > > > +} > > > + > > > +double CamHelperMira220::gain(uint32_t gainCode) const > > > +{ > > > + return static_cast<double>(2048.0 / (2048 - gainCode)); > > > +} > > > + > > > +static CamHelper *create() > > > +{ > > > + return new CamHelperMira220(); > > > +} > > > + > > > +static RegisterCamHelper reg("mira220", &create); > > > diff --git a/src/ipa/rpi/cam_helper/meson.build b/src/ipa/rpi/cam_helper/meson.build > > > index eabd55dce5a1..a8756a1a0226 100644 > > > --- a/src/ipa/rpi/cam_helper/meson.build > > > +++ b/src/ipa/rpi/cam_helper/meson.build > > > @@ -12,6 +12,7 @@ rpi_ipa_cam_helper_sources = files([ > > > 'cam_helper_imx519.cpp', > > > 'cam_helper_imx678.cpp', > > > 'cam_helper_imx708.cpp', > > > + 'cam_helper_mira220.cpp', > > > 'cam_helper_ov64a40.cpp', > > > 'cam_helper_ov7251.cpp', > > > 'cam_helper_ov9281.cpp',
diff --git a/src/ipa/rpi/cam_helper/cam_helper_mira220.cpp b/src/ipa/rpi/cam_helper/cam_helper_mira220.cpp new file mode 100644 index 000000000000..f0bd1ecbfff6 --- /dev/null +++ b/src/ipa/rpi/cam_helper/cam_helper_mira220.cpp @@ -0,0 +1,55 @@ +/* SPDX-License-Identifier: BSD-2-Clause */ +/* + * Copyright (C) 2026, ams-OSRAM + * + * cam_helper_Mira220.cpp - camera information for Mira220 sensor + */ + +#include <assert.h> + +#include "cam_helper.h" + +using namespace RPiController; + +class CamHelperMira220 : public CamHelper +{ +public: + CamHelperMira220(); + uint32_t gainCode(double gain) const override; + double gain(uint32_t gainCode) const override; + unsigned int hideFramesModeSwitch() const override; + +private: + /* + * Smallest difference between the frame length and integration time, + * in units of lines. + */ + static constexpr int frameIntegrationDiff = 4; +}; + +/* + * Mira220 doesn't output metadata, so we have to use the delayed controls which + * works by counting frames. + */ + +CamHelperMira220::CamHelperMira220() + : CamHelper({}, frameIntegrationDiff) +{ +} + +uint32_t CamHelperMira220::gainCode(double gain) const +{ + return static_cast<uint32_t>(2048.0 - 2048.0 / gain); +} + +double CamHelperMira220::gain(uint32_t gainCode) const +{ + return static_cast<double>(2048.0 / (2048 - gainCode)); +} + +static CamHelper *create() +{ + return new CamHelperMira220(); +} + +static RegisterCamHelper reg("mira220", &create); diff --git a/src/ipa/rpi/cam_helper/meson.build b/src/ipa/rpi/cam_helper/meson.build index eabd55dce5a1..a8756a1a0226 100644 --- a/src/ipa/rpi/cam_helper/meson.build +++ b/src/ipa/rpi/cam_helper/meson.build @@ -12,6 +12,7 @@ rpi_ipa_cam_helper_sources = files([ 'cam_helper_imx519.cpp', 'cam_helper_imx678.cpp', 'cam_helper_imx708.cpp', + 'cam_helper_mira220.cpp', 'cam_helper_ov64a40.cpp', 'cam_helper_ov7251.cpp', 'cam_helper_ov9281.cpp',