| Message ID | 20221010074232.2404-1-naush@raspberrypi.com |
|---|---|
| State | Accepted |
| Commit | e5fc0132f80d644757373ab0e0e3db00fe6ee010 |
| Headers | show |
| Series |
|
| Related | show |
Hi Naush, Thank you for the patch. On Mon, Oct 10, 2022 at 08:42:32AM +0100, Naushir Patuck via libcamera-devel wrote: > Add fields for minimum and maximum line length (in units of pixels) to the > IPACameraSensorInfo structure. This replaces the existing lineLength field. > > Update the ipu3, raspberrypi and rkisp1 IPAs to use IPACameraSensorInfo::minLineLength > instead of IPACameraSensorInfo::lineLength, as logically we will always want to > use the fastest sensor readout by default. > > Since the IPAs now use minLineLength for their calculations, set the starting > value of the V4L2_CID_HBLANK control to its minimum in CameraSensor::init(). > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > Tested-by: Dave Stevenson <dave.stevenson@raspberrypi.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > include/libcamera/ipa/core.mojom | 21 +++++++++++++------ > src/ipa/ipu3/ipu3.cpp | 6 ++++-- > src/ipa/raspberrypi/raspberrypi.cpp | 2 +- > src/ipa/rkisp1/rkisp1.cpp | 2 +- > src/libcamera/camera_sensor.cpp | 32 +++++++++++++++++++++++++++-- > 5 files changed, 51 insertions(+), 12 deletions(-) > > diff --git a/include/libcamera/ipa/core.mojom b/include/libcamera/ipa/core.mojom > index 74f3339e56f2..2bc3028c22d6 100644 > --- a/include/libcamera/ipa/core.mojom > +++ b/include/libcamera/ipa/core.mojom > @@ -172,10 +172,17 @@ module libcamera; > */ > > /** > - * \var IPACameraSensorInfo::lineLength > - * \brief Total line length in pixels > + * \var IPACameraSensorInfo::minLineLength > + * \brief The minimum line length in pixels > * > - * The total line length in pixel clock periods, including blanking. > + * The minimum allowable line length in pixel clock periods, including blanking. > + */ > + > +/** > + * \var IPACameraSensorInfo::maxLineLength > + * \brief The maximum line length in pixels > + * > + * The maximum allowable line length in pixel clock periods, including blanking. > */ > > /** > @@ -189,7 +196,7 @@ module libcamera; > * To obtain the minimum frame duration: > * > * \verbatim > - frameDuration(s) = minFrameLength(lines) * lineLength(pixels) / pixelRate(pixels per second) > + frameDuration(s) = minFrameLength(lines) * minLineLength(pixels) / pixelRate(pixels per second) > \endverbatim > */ > > @@ -204,7 +211,7 @@ module libcamera; > * To obtain the maximum frame duration: > * > * \verbatim > - frameDuration(s) = maxFrameLength(lines) * lineLength(pixels) / pixelRate(pixels per second) > + frameDuration(s) = maxFrameLength(lines) * maxLineLength(pixels) / pixelRate(pixels per second) > \endverbatim > */ > struct IPACameraSensorInfo { > @@ -217,7 +224,9 @@ struct IPACameraSensorInfo { > Size outputSize; > > uint64 pixelRate; > - uint32 lineLength; > + > + uint32 minLineLength; > + uint32 maxLineLength; > > uint32 minFrameLength; > uint32 maxFrameLength; > diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp > index b93a09d40c39..c89f76c56ee3 100644 > --- a/src/ipa/ipu3/ipu3.cpp > +++ b/src/ipa/ipu3/ipu3.cpp > @@ -336,7 +336,8 @@ int IPAIPU3::init(const IPASettings &settings, > > /* Clean context */ > context_.configuration = {}; > - context_.configuration.sensor.lineDuration = sensorInfo.lineLength * 1.0s / sensorInfo.pixelRate; > + context_.configuration.sensor.lineDuration = sensorInfo.minLineLength > + * 1.0s / sensorInfo.pixelRate; > > /* Load the tuning data file. */ > File file(settings.configurationFile); > @@ -499,7 +500,8 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo, > context_.frameContexts.clear(); > > /* Initialise the sensor configuration. */ > - context_.configuration.sensor.lineDuration = sensorInfo_.lineLength * 1.0s / sensorInfo_.pixelRate; > + context_.configuration.sensor.lineDuration = sensorInfo_.minLineLength > + * 1.0s / sensorInfo_.pixelRate; > > /* > * Compute the sensor V4L2 controls to be used by the algorithms and > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp > index 8d731435764e..358a119da222 100644 > --- a/src/ipa/raspberrypi/raspberrypi.cpp > +++ b/src/ipa/raspberrypi/raspberrypi.cpp > @@ -356,7 +356,7 @@ void IPARPi::setMode(const IPACameraSensorInfo &sensorInfo) > * Calculate the line length as the ratio between the line length in > * pixels and the pixel rate. > */ > - mode_.lineLength = sensorInfo.lineLength * (1.0s / sensorInfo.pixelRate); > + mode_.lineLength = sensorInfo.minLineLength * (1.0s / sensorInfo.pixelRate); > > /* > * Set the frame length limits for the mode to ensure exposure and > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp > index 32feb1682749..ddb22d98eb41 100644 > --- a/src/ipa/rkisp1/rkisp1.cpp > +++ b/src/ipa/rkisp1/rkisp1.cpp > @@ -246,7 +246,7 @@ int IPARkISP1::configure([[maybe_unused]] const IPACameraSensorInfo &info, > context_.configuration.hw.revision = hwRevision_; > > context_.configuration.sensor.size = info.outputSize; > - context_.configuration.sensor.lineDuration = info.lineLength * 1.0s / info.pixelRate; > + context_.configuration.sensor.lineDuration = info.minLineLength * 1.0s / info.pixelRate; > > /* > * When the AGC computes the new exposure values for a frame, it needs > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp > index 911fd0beae4e..572a313a8f99 100644 > --- a/src/libcamera/camera_sensor.cpp > +++ b/src/libcamera/camera_sensor.cpp > @@ -176,6 +176,32 @@ int CameraSensor::init() > if (ret) > return ret; > > + /* > + * Set HBLANK to the minimum to start with a well-defined line length, > + * allowing IPA modules that do not modify HBLANK to use the sensor > + * minimum line length in their calculations. > + * > + * At present, there is no way of knowing if a control is read-only. > + * As a workaround, assume that if the minimum and maximum values of > + * the V4L2_CID_HBLANK control are the same, it implies the control > + * is read-only. > + * > + * \todo The control API ought to have a flag to specify if a control > + * is read-only which could be used below. > + */ > + const ControlInfo hblank = ctrls.infoMap()->at(V4L2_CID_HBLANK); > + const int32_t hblankMin = hblank.min().get<int32_t>(); > + const int32_t hblankMax = hblank.max().get<int32_t>(); > + > + if (hblankMin != hblankMax) { > + ControlList ctrl(subdev_->controls()); > + > + ctrl.set(V4L2_CID_HBLANK, hblankMin); > + ret = subdev_->setControls(&ctrl); > + if (ret) > + return ret; > + } > + > return applyTestPatternMode(controls::draft::TestPatternModeEnum::TestPatternModeOff); > } > > @@ -883,10 +909,12 @@ int CameraSensor::sensorInfo(IPACameraSensorInfo *info) const > return -EINVAL; > } > > - int32_t hblank = ctrls.get(V4L2_CID_HBLANK).get<int32_t>(); > - info->lineLength = info->outputSize.width + hblank; > info->pixelRate = ctrls.get(V4L2_CID_PIXEL_RATE).get<int64_t>(); > > + const ControlInfo hblank = ctrls.infoMap()->at(V4L2_CID_HBLANK); > + info->minLineLength = info->outputSize.width + hblank.min().get<int32_t>(); > + info->maxLineLength = info->outputSize.width + hblank.max().get<int32_t>(); > + > const ControlInfo vblank = ctrls.infoMap()->at(V4L2_CID_VBLANK); > info->minFrameLength = info->outputSize.height + vblank.min().get<int32_t>(); > info->maxFrameLength = info->outputSize.height + vblank.max().get<int32_t>();
diff --git a/include/libcamera/ipa/core.mojom b/include/libcamera/ipa/core.mojom index 74f3339e56f2..2bc3028c22d6 100644 --- a/include/libcamera/ipa/core.mojom +++ b/include/libcamera/ipa/core.mojom @@ -172,10 +172,17 @@ module libcamera; */ /** - * \var IPACameraSensorInfo::lineLength - * \brief Total line length in pixels + * \var IPACameraSensorInfo::minLineLength + * \brief The minimum line length in pixels * - * The total line length in pixel clock periods, including blanking. + * The minimum allowable line length in pixel clock periods, including blanking. + */ + +/** + * \var IPACameraSensorInfo::maxLineLength + * \brief The maximum line length in pixels + * + * The maximum allowable line length in pixel clock periods, including blanking. */ /** @@ -189,7 +196,7 @@ module libcamera; * To obtain the minimum frame duration: * * \verbatim - frameDuration(s) = minFrameLength(lines) * lineLength(pixels) / pixelRate(pixels per second) + frameDuration(s) = minFrameLength(lines) * minLineLength(pixels) / pixelRate(pixels per second) \endverbatim */ @@ -204,7 +211,7 @@ module libcamera; * To obtain the maximum frame duration: * * \verbatim - frameDuration(s) = maxFrameLength(lines) * lineLength(pixels) / pixelRate(pixels per second) + frameDuration(s) = maxFrameLength(lines) * maxLineLength(pixels) / pixelRate(pixels per second) \endverbatim */ struct IPACameraSensorInfo { @@ -217,7 +224,9 @@ struct IPACameraSensorInfo { Size outputSize; uint64 pixelRate; - uint32 lineLength; + + uint32 minLineLength; + uint32 maxLineLength; uint32 minFrameLength; uint32 maxFrameLength; diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp index b93a09d40c39..c89f76c56ee3 100644 --- a/src/ipa/ipu3/ipu3.cpp +++ b/src/ipa/ipu3/ipu3.cpp @@ -336,7 +336,8 @@ int IPAIPU3::init(const IPASettings &settings, /* Clean context */ context_.configuration = {}; - context_.configuration.sensor.lineDuration = sensorInfo.lineLength * 1.0s / sensorInfo.pixelRate; + context_.configuration.sensor.lineDuration = sensorInfo.minLineLength + * 1.0s / sensorInfo.pixelRate; /* Load the tuning data file. */ File file(settings.configurationFile); @@ -499,7 +500,8 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo, context_.frameContexts.clear(); /* Initialise the sensor configuration. */ - context_.configuration.sensor.lineDuration = sensorInfo_.lineLength * 1.0s / sensorInfo_.pixelRate; + context_.configuration.sensor.lineDuration = sensorInfo_.minLineLength + * 1.0s / sensorInfo_.pixelRate; /* * Compute the sensor V4L2 controls to be used by the algorithms and diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp index 8d731435764e..358a119da222 100644 --- a/src/ipa/raspberrypi/raspberrypi.cpp +++ b/src/ipa/raspberrypi/raspberrypi.cpp @@ -356,7 +356,7 @@ void IPARPi::setMode(const IPACameraSensorInfo &sensorInfo) * Calculate the line length as the ratio between the line length in * pixels and the pixel rate. */ - mode_.lineLength = sensorInfo.lineLength * (1.0s / sensorInfo.pixelRate); + mode_.lineLength = sensorInfo.minLineLength * (1.0s / sensorInfo.pixelRate); /* * Set the frame length limits for the mode to ensure exposure and diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp index 32feb1682749..ddb22d98eb41 100644 --- a/src/ipa/rkisp1/rkisp1.cpp +++ b/src/ipa/rkisp1/rkisp1.cpp @@ -246,7 +246,7 @@ int IPARkISP1::configure([[maybe_unused]] const IPACameraSensorInfo &info, context_.configuration.hw.revision = hwRevision_; context_.configuration.sensor.size = info.outputSize; - context_.configuration.sensor.lineDuration = info.lineLength * 1.0s / info.pixelRate; + context_.configuration.sensor.lineDuration = info.minLineLength * 1.0s / info.pixelRate; /* * When the AGC computes the new exposure values for a frame, it needs diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp index 911fd0beae4e..572a313a8f99 100644 --- a/src/libcamera/camera_sensor.cpp +++ b/src/libcamera/camera_sensor.cpp @@ -176,6 +176,32 @@ int CameraSensor::init() if (ret) return ret; + /* + * Set HBLANK to the minimum to start with a well-defined line length, + * allowing IPA modules that do not modify HBLANK to use the sensor + * minimum line length in their calculations. + * + * At present, there is no way of knowing if a control is read-only. + * As a workaround, assume that if the minimum and maximum values of + * the V4L2_CID_HBLANK control are the same, it implies the control + * is read-only. + * + * \todo The control API ought to have a flag to specify if a control + * is read-only which could be used below. + */ + const ControlInfo hblank = ctrls.infoMap()->at(V4L2_CID_HBLANK); + const int32_t hblankMin = hblank.min().get<int32_t>(); + const int32_t hblankMax = hblank.max().get<int32_t>(); + + if (hblankMin != hblankMax) { + ControlList ctrl(subdev_->controls()); + + ctrl.set(V4L2_CID_HBLANK, hblankMin); + ret = subdev_->setControls(&ctrl); + if (ret) + return ret; + } + return applyTestPatternMode(controls::draft::TestPatternModeEnum::TestPatternModeOff); } @@ -883,10 +909,12 @@ int CameraSensor::sensorInfo(IPACameraSensorInfo *info) const return -EINVAL; } - int32_t hblank = ctrls.get(V4L2_CID_HBLANK).get<int32_t>(); - info->lineLength = info->outputSize.width + hblank; info->pixelRate = ctrls.get(V4L2_CID_PIXEL_RATE).get<int64_t>(); + const ControlInfo hblank = ctrls.infoMap()->at(V4L2_CID_HBLANK); + info->minLineLength = info->outputSize.width + hblank.min().get<int32_t>(); + info->maxLineLength = info->outputSize.width + hblank.max().get<int32_t>(); + const ControlInfo vblank = ctrls.infoMap()->at(V4L2_CID_VBLANK); info->minFrameLength = info->outputSize.height + vblank.min().get<int32_t>(); info->maxFrameLength = info->outputSize.height + vblank.max().get<int32_t>();