[libcamera-devel,v2,3/4] ipa: raspberrypi: imx477: Get sensor temperature from embedded data

16365 diff mbox series

Fetch the sensor temperature value from the embedded data buffer and add it to
the DeviceStatus structure in CamHelperImx477::PopulateMetadata().

Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
 src/ipa/raspberrypi/cam_helper_imx477.cpp | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Hi Naush

Thanks for the patch!

On Fri, 24 Jun 2022 at 08:35, Naushir Patuck via libcamera-devel
<libcamera-devel@lists.libcamera.org> wrote:
>
> Fetch the sensor temperature value from the embedded data buffer and add it to
> the DeviceStatus structure in CamHelperImx477::PopulateMetadata().
>
> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> ---
>  src/ipa/raspberrypi/cam_helper_imx477.cpp | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/src/ipa/raspberrypi/cam_helper_imx477.cpp b/src/ipa/raspberrypi/cam_helper_imx477.cpp
> index 338fdc0c416a..0e1c0dbd142f 100644
> --- a/src/ipa/raspberrypi/cam_helper_imx477.cpp
> +++ b/src/ipa/raspberrypi/cam_helper_imx477.cpp
> @@ -5,6 +5,7 @@
>   * cam_helper_imx477.cpp - camera helper for imx477 sensor
>   */
>
> +#include <algorithm>
>  #include <assert.h>
>  #include <cmath>
>  #include <stddef.h>
> @@ -34,8 +35,9 @@ constexpr uint32_t gainHiReg = 0x0204;
>  constexpr uint32_t gainLoReg = 0x0205;
>  constexpr uint32_t frameLengthHiReg = 0x0340;
>  constexpr uint32_t frameLengthLoReg = 0x0341;
> +constexpr uint32_t temperatureReg = 0x013a;
>  constexpr std::initializer_list<uint32_t> registerList =
> -       { expHiReg, expLoReg, gainHiReg, gainLoReg, frameLengthHiReg, frameLengthLoReg  };
> +       { expHiReg, expLoReg, gainHiReg, gainLoReg, frameLengthHiReg, frameLengthLoReg, temperatureReg  };
>
>  class CamHelperImx477 : public CamHelper
>  {
> @@ -171,6 +173,7 @@ void CamHelperImx477::PopulateMetadata(const MdParser::RegisterMap &registers,
>         deviceStatus.shutter_speed = Exposure(registers.at(expHiReg) * 256 + registers.at(expLoReg));
>         deviceStatus.analogue_gain = Gain(registers.at(gainHiReg) * 256 + registers.at(gainLoReg));
>         deviceStatus.frame_length = registers.at(frameLengthHiReg) * 256 + registers.at(frameLengthLoReg);
> +       deviceStatus.sensor_temperature = std::clamp<int8_t>(registers.at(temperatureReg), -20, 80);

I guess I'm mildly curious if out-of-bounds values from the register
mean anything, but probably not worth worrying ourselves about too
much. So:

Reviewed-by: David Plowman <david.plowman@raspberrypi.com>

Thanks
David

>
>         metadata.Set("device.status", deviceStatus);
>  }
> --
> 2.25.1
>

Hi David,

Thanks for your feedback.

On Tue, 28 Jun 2022 at 09:30, David Plowman <david.plowman@raspberrypi.com>
wrote:

> Hi Naush
>
> Thanks for the patch!
>
> On Fri, 24 Jun 2022 at 08:35, Naushir Patuck via libcamera-devel
> <libcamera-devel@lists.libcamera.org> wrote:
> >
> > Fetch the sensor temperature value from the embedded data buffer and add
> it to
> > the DeviceStatus structure in CamHelperImx477::PopulateMetadata().
> >
> > Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> > ---
> >  src/ipa/raspberrypi/cam_helper_imx477.cpp | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/ipa/raspberrypi/cam_helper_imx477.cpp
> b/src/ipa/raspberrypi/cam_helper_imx477.cpp
> > index 338fdc0c416a..0e1c0dbd142f 100644
> > --- a/src/ipa/raspberrypi/cam_helper_imx477.cpp
> > +++ b/src/ipa/raspberrypi/cam_helper_imx477.cpp
> > @@ -5,6 +5,7 @@
> >   * cam_helper_imx477.cpp - camera helper for imx477 sensor
> >   */
> >
> > +#include <algorithm>
> >  #include <assert.h>
> >  #include <cmath>
> >  #include <stddef.h>
> > @@ -34,8 +35,9 @@ constexpr uint32_t gainHiReg = 0x0204;
> >  constexpr uint32_t gainLoReg = 0x0205;
> >  constexpr uint32_t frameLengthHiReg = 0x0340;
> >  constexpr uint32_t frameLengthLoReg = 0x0341;
> > +constexpr uint32_t temperatureReg = 0x013a;
> >  constexpr std::initializer_list<uint32_t> registerList =
> > -       { expHiReg, expLoReg, gainHiReg, gainLoReg, frameLengthHiReg,
> frameLengthLoReg  };
> > +       { expHiReg, expLoReg, gainHiReg, gainLoReg, frameLengthHiReg,
> frameLengthLoReg, temperatureReg  };
> >
> >  class CamHelperImx477 : public CamHelper
> >  {
> > @@ -171,6 +173,7 @@ void CamHelperImx477::PopulateMetadata(const
> MdParser::RegisterMap &registers,
> >         deviceStatus.shutter_speed = Exposure(registers.at(expHiReg) *
> 256 + registers.at(expLoReg));
> >         deviceStatus.analogue_gain = Gain(registers.at(gainHiReg) * 256
> + registers.at(gainLoReg));
> >         deviceStatus.frame_length = registers.at(frameLengthHiReg) *
> 256 + registers.at(frameLengthLoReg);
> > +       deviceStatus.sensor_temperature = std::clamp<int8_t>(
> registers.at(temperatureReg), -20, 80);
>
> I guess I'm mildly curious if out-of-bounds values from the register
> mean anything, but probably not worth worrying ourselves about too
> much.


From the datasheet, the register is treated as a signed 8-bit value, but
with the
-20 C to 80 C limits. I'm not sure if this means that these limits are
recommended and
the register can provide temperature measurements of -128C to 127C. Or
perhaps the
register never goes beyond the -20 C to 80 C limits..? Either way, I am not
worrying too
much about it.

Naush



> So:
>
> Reviewed-by: David Plowman <david.plowman@raspberrypi.com>
>
> Thanks
> David
>
> >
> >         metadata.Set("device.status", deviceStatus);
> >  }
> > --
> > 2.25.1
> >
>