[PATCH 0/5] ipa: Add black level to camera sensor helpers

[Laurent Pinchart]

On Tue, Jul 02, 2024 at 10:56:59AM +0200, Jacopo Mondi wrote:
> Hi Stefan
> 
> On Mon, Jul 01, 2024 at 04:38:23PM GMT, Stefan Klug wrote:
> > For the tuning process, the sensor black level needs to be known. Many
> > sensors have either a fixed black level, or the current sensor kernel
> > driver sets a fixed value. It therefore makes sense to keep that
> 
> Where do kernel driver handles BLC and how ?

Currently they either leave the corresponding registers to their
power-on default values, or program them with fixed values. I'm not
aware of any driver in mainline that makes the data pedestal
configurable from userspace.

> > information inside libcamera. When the need arises to be more flexible
> > we can extend the sensor drivers to report black level information. This
> > series adds black level support to the camera sensor helpers and the
> > corresponding metadata to the rkisp1 ipa.

It could be useful to explain a bit better that we're covering the data
pedestal only at this point, and how that differs from the black level.
Ideally that information should be captured somewhere in libcamera.
Maybe in the documentation of the CameraSensorHelper::blackLevels()
function ?

> I always considered the camera_sensor_properties database a better
> place where to store immutable properties like this one, but the
> database is not accessibile by IPA. Have you gone for
> CameraSensorHelper for this reason ?

I told Stefan to go for CameraSensorHelper for that reason, yes.
Eventually the two sets of helpers should be merged into one.

> How does this play with the idea of moving CameraSensorHelpers on the
> pipeline side (something on our roadmap since a long time ?). Will we
> need in this case to anyway pass sensor's data from pipeline to the
> IPA ?

I'd rather address it at that point, with a solution that covers all the
information the IPA module needs, instead of adding a partial
implementation today that we would need to rework tomorrow.

> > My tuning series will be rebased on top of this one.
> >
> > Best regards,
> > Stefan
> >
> > Stefan Klug (5):
> >   ipa: libipa: Add black levels to camera sensor helper
> >   ipa: rkisp1: Move camHelper into IPAContext
> >   ipa: rkisp1: blc: Query black levels from camera sensor helper
> >   ipa: rkisp1: blc: Report sensor black levels in metadata
> >   ipa: rkisp1: data: Update tuning files for imx219 and imx258
> >
> >  src/ipa/libipa/camera_sensor_helper.cpp | 18 ++++++++++
> >  src/ipa/libipa/camera_sensor_helper.h   |  6 ++++
> >  src/ipa/rkisp1/algorithms/blc.cpp       | 47 ++++++++++++++++++++++---
> >  src/ipa/rkisp1/algorithms/blc.h         |  5 ++-
> >  src/ipa/rkisp1/data/imx219.yaml         |  4 ---
> >  src/ipa/rkisp1/data/imx258.yaml         |  1 +
> >  src/ipa/rkisp1/data/uncalibrated.yaml   |  1 +
> >  src/ipa/rkisp1/ipa_context.h            |  4 +++
> >  src/ipa/rkisp1/rkisp1.cpp               | 26 +++++++-------
> >  9 files changed, 89 insertions(+), 23 deletions(-)
> >

-- 
Regards,

Laurent Pinchart