<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, 26 Oct 2021 at 10:34, Laurent Pinchart <<a href="mailto:laurent.pinchart@ideasonboard.com">laurent.pinchart@ideasonboard.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi Naush,<br>
On Tue, Oct 26, 2021 at 08:47:15AM +0100, Naushir Patuck wrote:<br>
> On Mon, 25 Oct 2021 at 17:44, Laurent Pinchart wrote:<br>
> > On Fri, Oct 22, 2021 at 04:29:34PM +0100, Naushir Patuck wrote:<br>
> > > On Fri, 22 Oct 2021 at 16:16, David Plowman wrote:<br>
> > ><br>
> > > > Hi Naush<br>
> > > ><br>
> > > > Thanks for the new version. I think a lot of stuff is tidier now that<br>
> > > > we're clear that we're selecting a subdev format.<br>
> > > ><br>
> > > > On Fri, 22 Oct 2021 at 15:40, Naushir Patuck wrote:<br>
> > > > ><br>
> > > > > Switch the pipeline handler to use the new Unicam media controller based driver.<br>
> > > > > With this change, we directly talk to the sensor device driver to set controls<br>
> > > > > and set/get formats in the pipeline handler.<br>
> > > > ><br>
> > > > > This change requires the accompanying Raspberry Pi linux kernel change at<br>
> > > > > <a href="https://github.com/raspberrypi/linux/pull/4645" rel="noreferrer" target="_blank">https://github.com/raspberrypi/linux/pull/4645</a>. If this kernel change is not<br>
> > > > > present, the pipeline handler will fail to run with an error message informing<br>
> > > > > the user to update the kernel build.<br>
> > > > ><br>
> > > > > Signed-off-by: Naushir Patuck <<a href="mailto:naush@raspberrypi.com" target="_blank">naush@raspberrypi.com</a>><br>
> > > > > ---<br>
> > > > > .../pipeline/raspberrypi/raspberrypi.cpp | 146 +++++++++++-------<br>
> > > > > 1 file changed, 90 insertions(+), 56 deletions(-)<br>
> > > > ><br>
> > > > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp<br>
> > > > > index 1634ca98f481..a31b0f81eba7 100644<br>
> > > > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp<br>
> > > > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp<br>
> > > > > @@ -48,6 +48,29 @@ LOG_DEFINE_CATEGORY(RPI)<br>
> > > > ><br>
> > > > > namespace {<br>
> > > > ><br>
> > > > > +/* Map of mbus codes to supported sizes reported by the sensor. */<br>
> > > > > +using SensorFormats = std::map<unsigned int, std::vector<Size>>;<br>
> > > > > +<br>
> > > > > +SensorFormats populateSensorFormats(std::unique_ptr<CameraSensor> &sensor)<br>
> > > > > +{<br>
> > > > > + SensorFormats formats;<br>
> > > > > +<br>
> > > > > + for (auto const mbusCode : sensor->mbusCodes())<br>
> > > > > + formats.emplace(mbusCode, sensor->sizes(mbusCode));<br>
> > > > > +<br>
> > > > > + return formats;<br>
> > > > > +}<br>
> > > > > +<br>
> > > > > +inline V4L2DeviceFormat toV4L2DeviceFormat(V4L2SubdeviceFormat &mode)<br>
> ><br>
> > s/mode/format/<br>
> ><br>
> > (you know how much I like the concept of sensor mode)<br>
> ><br>
> > > > > +{<br>
> > > > > + V4L2DeviceFormat deviceFormat;<br>
> > > > > + BayerFormat bayer = BayerFormat::fromMbusCode(mode.mbus_code);<br>
> > > > > +<br>
> > > > > + deviceFormat.fourcc = bayer.toV4L2PixelFormat();<br>
> > > > > + deviceFormat.size = mode.size;<br>
> > > > > + return deviceFormat;<br>
> > > > > +}<br>
> > > > > +<br>
> > > ><br>
> > > > I guess I wonder slightly whether this should be generally available,<br>
> > > > but it could easily be a change for another time. (Actually, same<br>
> > > > question for isRaw just below...)<br>
> > ><br>
> > > Agree, this can be addressed separately.<br>
> ><br>
> > I'm afraid it can't be this simple :-( There's no 1:1 mapping between<br>
> > media bus codes and V4L2 pixel formats. How a given format received on a<br>
> > bus by a DMA engine is stored in memory depends on the device. For<br>
> > instance, unicam will store BGGR RAW10 as V4L2_PIX_FMT_SBGGR10P, while<br>
> > another device may store it as V4L2_PIX_FMT_SBGGR10 (expanded to 2 bytes<br>
> > per pixel in memory). Yet another device may left-align the 10 bits,<br>
> > storing the data in V4L2_PIX_FMT_SBGGR16 format. This mapping can't be<br>
> > provided by the libcamera core. Using the BayerFormat class to perform<br>
> > the conversion with BayerFormat::fromMbusCode().toV4L2PixelFormat() may<br>
> > work in some cases, but not universally.<br>
> <br>
> I was also a bit unsure about what to do about mbus to v4l2 format conversions.<br>
> I went with using the BayerFormat class table as that was readily available, and<br>
> not needing to duplicate the table in the pipeline handler. Luckily, I think in all (standard)<br>
> cases, the conversion from mbus codes to v4l2 4cc's through the BayerFormat<br>
> class conversion table will be valid for Unicam, but I do take the point this is<br>
> not entirely correct.<br>
> <br>
> Perhaps I will just bite the bullet and add a similar table into our pipeline handler. As<br>
> you said, avoiding technical debt is a good thing.<br>
<br>
I think that would be best, to avoid introducing code that relies on the<br>
assumption that device-specific conversion can be handled in a standard<br>
class. It would be more difficult to refactor the BayerFormat class<br>
later otherwise.<br>
<br>
This being said, what bothers me slightly is that the BayerFormat class<br>
can do the job if the pipeline handler carefully sets the different<br>
members (for instance setting the packing member after converting from<br>
mbus code to BayerFormat and before converting to PixelFormat or<br>
V4L2PixelFormat, but in a different pipeline handler it may also require<br>
overriding the bitDepth member, when the hardware always expands RAW10<br>
to 16 bits with left alignment). I think the existing BayerFormat API is<br>
error-prone in that regard, it's easy to use it incorrectly. I'm not<br>
sure how we could improve that though, and it would be nice to use the<br>
class to perform conversions.<br>
<br>
On the other hand, the problem of converting from media bus codes to<br>
pixel formats isn't specific to bayer formats (there are YUV sensors<br>
too), so in the general case it needs to be handled manually in pipeline<br>
handlers anyway.<br>
<br>
If you have any clever idea to fix all that, feel free to include it in<br>
the next version :-)<br>
<br>
> > I'm actually surprised the conversion works here, as I think you'll get<br>
> > V4L2_PIX_FMT_SBGGR10 when you expect V4L2_PIX_FMT_SBGGR10P. One<br>
> > possibility to fix this would be to set bayer.packing before calling<br>
> > toV4L2PixelFormat().<br>
> <br>
> This works because Unicam can support both packed and unpacked formats<br>
> when writing out to SDRAM. As you have noticed, a subsequent change in this<br>
> series adds the option of choosing which to select.<br>
<br>
Indeed, I noticed later :-)<br>
<br>
By the way, on a semi-related note, I was wondering if unicam was about<br>
to convert RAW10 to RAW8.<br></blockquote><div><br></div><div>Yes, it should be possible, although I can't say I've ever attempted to do this</div><div>myself :-)</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> I'll add the new table in the next revision.<br>
> <br>
> > The API is fragile though, and the BayerFormat<br>
> > class a bit ill-defined (it doesn't tell whether it's supposed to<br>
> > represent a bus format or a pixel format, and as explained above, it<br>
> > can't be used interchangeably). This doesn't have to be fixed as part of<br>
> > this patch series, but I'd like to see that problem addressed, and we<br>
> > shouldn't build more technical debt.<br>
> ><br>
> > > > > bool isRaw(PixelFormat &pixFmt)<br>
> > > > > {<br>
> > > > > /*<br>
> > > > > @@ -74,11 +97,10 @@ double scoreFormat(double desired, double actual)<br>
> > > > > return score;<br>
> > > > > }<br>
> > > > ><br>
> > > > > -V4L2DeviceFormat findBestMode(V4L2VideoDevice::Formats &formatsMap,<br>
> > > > > - const Size &req)<br>
> > > > > +V4L2SubdeviceFormat findBestMode(const SensorFormats &formatsMap, const Size &req)<br>
> > > > > {<br>
> > > > > double bestScore = std::numeric_limits<double>::max(), score;<br>
> > > > > - V4L2DeviceFormat bestMode;<br>
> > > > > + V4L2SubdeviceFormat bestMode;<br>
> > > > ><br>
> > > > > #define PENALTY_AR 1500.0<br>
> > > > > #define PENALTY_8BIT 2000.0<br>
> > > > > @@ -88,18 +110,17 @@ V4L2DeviceFormat findBestMode(V4L2VideoDevice::Formats &formatsMap,<br>
> > > > ><br>
> > > > > /* Calculate the closest/best mode from the user requested size. */<br>
> > > > > for (const auto &iter : formatsMap) {<br>
> > > > > - V4L2PixelFormat v4l2Format = iter.first;<br>
> > > > > - const PixelFormatInfo &info = PixelFormatInfo::info(v4l2Format);<br>
> > > > > + const unsigned int mbus_code = iter.first;<br>
> ><br>
> > s/mbus_code/mbusCode/<br>
> ><br>
> > > > > + const PixelFormat format = BayerFormat::fromMbusCode(mbus_code).toPixelFormat();<br>
> ><br>
> > Same issue as above.<br>
> ><br>
> > > > > + const PixelFormatInfo &info = PixelFormatInfo::info(format);<br>
> > > > ><br>
> > > > > - for (const SizeRange &sz : iter.second) {<br>
> ><br>
> > s/sz/size/<br>
> ><br>
> > > > > - double modeWidth = sz.contains(req) ? req.width : sz.max.width;<br>
> > > > > - double modeHeight = sz.contains(req) ? req.height : sz.max.height;<br>
> > > > > + for (const Size &sz : iter.second) {<br>
> > > > > double reqAr = static_cast<double>(req.width) / req.height;<br>
> > > > > - double modeAr = modeWidth / modeHeight;<br>
> > > > > + double modeAr = sz.width / sz.height;<br>
> > > > ><br>
> > > > > /* Score the dimensions for closeness. */<br>
> > > > > - score = scoreFormat(req.width, modeWidth);<br>
> > > > > - score += scoreFormat(req.height, modeHeight);<br>
> > > > > + score = scoreFormat(req.width, sz.width);<br>
> > > > > + score += scoreFormat(req.height, sz.height);<br>
> > > > > score += PENALTY_AR * scoreFormat(reqAr, modeAr);<br>
> > > > ><br>
> > > > > /* Add any penalties... this is not an exact science! */<br>
> > > > > @@ -115,12 +136,12 @@ V4L2DeviceFormat findBestMode(V4L2VideoDevice::Formats &formatsMap,<br>
> > > > ><br>
> > > > > if (score <= bestScore) {<br>
> > > > > bestScore = score;<br>
> > > > > - bestMode.fourcc = v4l2Format;<br>
> > > > > - bestMode.size = Size(modeWidth, modeHeight);<br>
> > > > > + bestMode.mbus_code = mbus_code;<br>
> > > > > + bestMode.size = sz;<br>
> > > > > }<br>
> > > > ><br>
> > > > > - LOG(RPI, Info) << "Mode: " << modeWidth << "x" << modeHeight<br>
> > > > > - << " fmt " << v4l2Format.toString()<br>
> > > > > + LOG(RPI, Info) << "Mode: " << sz.width << "x" << sz.height<br>
> ><br>
> > You can use sz.toString().<br>
> ><br>
> > > > > + << " fmt " << format.toString()<br>
> > > > > << " Score: " << score<br>
> > > > > << " (best " << bestScore << ")";<br>
> > > > > }<br>
> > > > > @@ -170,6 +191,7 @@ public:<br>
> > > > > std::unique_ptr<ipa::RPi::IPAProxyRPi> ipa_;<br>
> > > > ><br>
> > > > > std::unique_ptr<CameraSensor> sensor_;<br>
> > > > > + SensorFormats sensorFormats_;<br>
> > > > > /* Array of Unicam and ISP device streams and associated buffers/streams. */<br>
> > > > > RPi::Device<Unicam, 2> unicam_;<br>
> > > > > RPi::Device<Isp, 4> isp_;<br>
> > > > > @@ -352,9 +374,9 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()<br>
> > > > > * Calculate the best sensor mode we can use based on<br>
> > > > > * the user request.<br>
> > > > > */<br>
> > > > > - V4L2VideoDevice::Formats fmts = data_->unicam_[Unicam::Image].dev()->formats();<br>
> > > > > - V4L2DeviceFormat sensorFormat = findBestMode(fmts, cfg.size);<br>
> > > > > - int ret = data_->unicam_[Unicam::Image].dev()->tryFormat(&sensorFormat);<br>
> > > > > + V4L2SubdeviceFormat sensorFormat = findBestMode(data_->sensorFormats_, cfg.size);<br>
> > > > > + V4L2DeviceFormat unicamFormat = toV4L2DeviceFormat(sensorFormat);<br>
> > > > > + int ret = data_->unicam_[Unicam::Image].dev()->tryFormat(&unicamFormat);<br>
> > > > > if (ret)<br>
> > > > > return Invalid;<br>
> > > > ><br>
> > > > > @@ -366,7 +388,7 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()<br>
> > > > > * fetch the "native" (i.e. untransformed) Bayer order,<br>
> > > > > * because the sensor may currently be flipped!<br>
> > > > > */<br>
> > > > > - V4L2PixelFormat fourcc = sensorFormat.fourcc;<br>
> > > > > + V4L2PixelFormat fourcc = unicamFormat.fourcc;<br>
> > > > > if (data_->flipsAlterBayerOrder_) {<br>
> > > > > BayerFormat bayer = BayerFormat::fromV4L2PixelFormat(fourcc);<br>
> > > > > bayer.order = data_->nativeBayerOrder_;<br>
> > > > > @@ -375,15 +397,15 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()<br>
> > > > > }<br>
> > > > ><br>
> > > > > PixelFormat sensorPixFormat = fourcc.toPixelFormat();<br>
> > > > > - if (cfg.size != sensorFormat.size ||<br>
> > > > > + if (cfg.size != unicamFormat.size ||<br>
> > > > > cfg.pixelFormat != sensorPixFormat) {<br>
> > > > > - cfg.size = sensorFormat.size;<br>
> > > > > + cfg.size = unicamFormat.size;<br>
> > > > > cfg.pixelFormat = sensorPixFormat;<br>
> > > > > status = Adjusted;<br>
> > > > > }<br>
> > > > ><br>
> > > > > - cfg.stride = sensorFormat.planes[0].bpl;<br>
> > > > > - cfg.frameSize = sensorFormat.planes[0].size;<br>
> > > > > + cfg.stride = unicamFormat.planes[0].bpl;<br>
> > > > > + cfg.frameSize = unicamFormat.planes[0].size;<br>
> > > > ><br>
> > > > > rawCount++;<br>
> > > > > } else {<br>
> > > > > @@ -472,7 +494,8 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera,<br>
> > > > > {<br>
> > > > > RPiCameraData *data = cameraData(camera);<br>
> > > > > CameraConfiguration *config = new RPiCameraConfiguration(data);<br>
> > > > > - V4L2DeviceFormat sensorFormat;<br>
> > > > > + V4L2SubdeviceFormat sensorFormat;<br>
> > > > > + V4L2DeviceFormat unicamFormat;<br>
> > > > > unsigned int bufferCount;<br>
> > > > > PixelFormat pixelFormat;<br>
> > > > > V4L2VideoDevice::Formats fmts;<br>
> > > > > @@ -487,9 +510,9 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera,<br>
> > > > > switch (role) {<br>
> > > > > case StreamRole::Raw:<br>
> > > > > size = data->sensor_->resolution();<br>
> > > > > - fmts = data->unicam_[Unicam::Image].dev()->formats();<br>
> > > > > - sensorFormat = findBestMode(fmts, size);<br>
> > > > > - pixelFormat = sensorFormat.fourcc.toPixelFormat();<br>
> > > > > + sensorFormat = findBestMode(data->sensorFormats_, size);<br>
> > > > > + unicamFormat = toV4L2DeviceFormat(sensorFormat);<br>
> > > > > + pixelFormat = BayerFormat::fromMbusCode(sensorFormat.mbus_code).toPixelFormat();<br>
> ><br>
> > Same issue as above.<br>
> ><br>
> > > > > ASSERT(pixelFormat.isValid());<br>
> > > > > bufferCount = 2;<br>
> > > > > rawCount++;<br>
> > > > > @@ -599,32 +622,30 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)<br>
> > > > > }<br>
> > > > ><br>
> > > > > /* First calculate the best sensor mode we can use based on the user request. */<br>
> > > > > - V4L2VideoDevice::Formats fmts = data->unicam_[Unicam::Image].dev()->formats();<br>
> > > > > - V4L2DeviceFormat sensorFormat = findBestMode(fmts, rawStream ? sensorSize : maxSize);<br>
> > > > > + V4L2SubdeviceFormat sensorFormat = findBestMode(data->sensorFormats_, rawStream ? sensorSize : maxSize);<br>
> > > > > + V4L2DeviceFormat unicamFormat = toV4L2DeviceFormat(sensorFormat);<br>
> ><br>
> > Shouldn't this go after sensor_->setFormat() in case the sensor adjusts<br>
> > the format ? Actually, as adjustments are not allowed in configure(),<br>
> > you should check the that format set on the sensor matches the requested<br>
> > format.<br>
> ><br>
> > > > > +<br>
> > > > > + ret = data->sensor_->setFormat(&sensorFormat);<br>
> > > > > + if (ret)<br>
> > > > > + return ret;<br>
> > > > ><br>
> > > > > /*<br>
> > > > > * Unicam image output format. The ISP input format gets set at start,<br>
> > > > > * just in case we have swapped bayer orders due to flips.<br>
> > > > > */<br>
> > > > > - ret = data->unicam_[Unicam::Image].dev()->setFormat(&sensorFormat);<br>
> > > > > + ret = data->unicam_[Unicam::Image].dev()->setFormat(&unicamFormat);<br>
> > > > > if (ret)<br>
> > > > > return ret;<br>
> > > > ><br>
> > > > > - /*<br>
> > > > > - * The control ranges associated with the sensor may need updating<br>
> > > > > - * after a format change.<br>
> > > > > - * \todo Use the CameraSensor::setFormat API instead.<br>
> > > > > - */<br>
> > > > > - data->sensor_->updateControlInfo();<br>
> > > > > -<br>
> > > > > LOG(RPI, Info) << "Sensor: " << camera->id()<br>
> > > > > - << " - Selected mode: " << sensorFormat.toString();<br>
> > > > > + << " - Selected sensor mode: " << sensorFormat.toString()<br>
> > > > > + << " - Selected unicam mode: " << unicamFormat.toString();<br>
> > <br>
> > s/mode/format/ on both lines<br>
> ><br>
> > > > ><br>
> > > > > /*<br>
> > > > > * This format may be reset on start() if the bayer order has changed<br>
> > > > > * because of flips in the sensor.<br>
> > > > > */<br>
> > > > > - ret = data->isp_[Isp::Input].dev()->setFormat(&sensorFormat);<br>
> > > > > + ret = data->isp_[Isp::Input].dev()->setFormat(&unicamFormat);<br>
> > > > > if (ret)<br>
> > > > > return ret;<br>
> > > > ><br>
> > > > > @@ -746,8 +767,8 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)<br>
> > > > > data->ispMinCropSize_ = testCrop.size();<br>
> > > > ><br>
> > > > > /* Adjust aspect ratio by providing crops on the input image. */<br>
> > > > > - Size size = sensorFormat.size.boundedToAspectRatio(maxSize);<br>
> > > > > - Rectangle crop = size.centeredTo(Rectangle(sensorFormat.size).center());<br>
> > > > > + Size size = unicamFormat.size.boundedToAspectRatio(maxSize);<br>
> > > > > + Rectangle crop = size.centeredTo(Rectangle(unicamFormat.size).center());<br>
> > > > > data->ispCrop_ = crop;<br>
> > > > ><br>
> > > > > data->isp_[Isp::Input].dev()->setSelection(V4L2_SEL_TGT_CROP, &crop);<br>
> > > > > @@ -761,8 +782,11 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)<br>
> > > > > * supports it.<br>
> > > > > */<br>
> > > > > if (data->sensorMetadata_) {<br>
> > > > > - format = {};<br>
> > > > > + V4L2SubdeviceFormat embeddedFormat;<br>
> > > > > +<br>
> > > > > + data->sensor_->device()->getFormat(1, &embeddedFormat);<br>
> > > > > format.fourcc = V4L2PixelFormat(V4L2_META_FMT_SENSOR_DATA);<br>
> > > > > + format.planes[0].size = embeddedFormat.size.width * embeddedFormat.size.height;<br>
> > > > ><br>
> > > > > LOG(RPI, Debug) << "Setting embedded data format.";<br>
> > > > > ret = data->unicam_[Unicam::Embedded].dev()->setFormat(&format);<br>
> > > > > @@ -847,9 +871,14 @@ int PipelineHandlerRPi::start(Camera *camera, const ControlList *controls)<br>
> > > > > * IPA configure may have changed the sensor flips - hence the bayer<br>
> > > > > * order. Get the sensor format and set the ISP input now.<br>
> > > > > */<br>
> > > > > - V4L2DeviceFormat sensorFormat;<br>
> > > > > - data->unicam_[Unicam::Image].dev()->getFormat(&sensorFormat);<br>
> > > > > - ret = data->isp_[Isp::Input].dev()->setFormat(&sensorFormat);<br>
> > > > > + V4L2SubdeviceFormat sensorFormat;<br>
> > > > > + data->sensor_->device()->getFormat(0, &sensorFormat);<br>
> > > > > +<br>
> > > > > + V4L2DeviceFormat ispFormat;<br>
> > > > > + ispFormat.fourcc = BayerFormat::fromMbusCode(sensorFormat.mbus_code).toV4L2PixelFormat();<br>
> > > ><br>
> > > > Generally there's the occasional assumption that fromMbusCode won't<br>
> > > > return invalid Bayer formats, and I guess we're comfortable with that?<br>
> > > > It certainly shouldn't be possible.<br>
> > ><br>
> > > Yes, I ought to add an assert to ensure the bayer format is valid.<br>
> > ><br>
> > > > But it seems good to me, so:<br>
> > > ><br>
> > > > Reviewed-by: David Plowman <<a href="mailto:david.plowman@raspberrypi.com" target="_blank">david.plowman@raspberrypi.com</a>><br>
> > > ><br>
> > > > > + ispFormat.size = sensorFormat.size;<br>
> > > > > +<br>
> > > > > + ret = data->isp_[Isp::Input].dev()->setFormat(&ispFormat);<br>
> > > > > if (ret) {<br>
> > > > > stop(camera);<br>
> > > > > return ret;<br>
> > > > > @@ -1004,6 +1033,8 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator)<br>
> > > > > if (data->sensor_->init())<br>
> > > > > return false;<br>
> > > > ><br>
> > > > > + data->sensorFormats_ = populateSensorFormats(data->sensor_);<br>
> > > > > +<br>
> > > > > ipa::RPi::SensorConfig sensorConfig;<br>
> > > > > if (data->loadIPA(&sensorConfig)) {<br>
> > > > > LOG(RPI, Error) << "Failed to load a suitable IPA library";<br>
> > > > > @@ -1030,6 +1061,11 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator)<br>
> > > > > return false;<br>
> > > > > }<br>
> > > > ><br>
> > > > > + if (!(data->unicam_[Unicam::Image].dev()->caps().device_caps() & V4L2_CAP_IO_MC)) {<br>
> > > > > + LOG(RPI, Error) << "Unicam driver did not advertise V4L2_CAP_IO_MC, please update your kernel!";<br>
> > > > > + return false;<br>
> > > > > + }<br>
> > > > > +<br>
> > > > > /*<br>
> > > > > * Setup our delayed control writer with the sensor default<br>
> > > > > * gain and exposure delays. Mark VBLANK for priority write.<br>
> > > > > @@ -1039,7 +1075,7 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator)<br>
> > > > > { V4L2_CID_EXPOSURE, { sensorConfig.exposureDelay, false } },<br>
> > > > > { V4L2_CID_VBLANK, { sensorConfig.vblankDelay, true } }<br>
> > > > > };<br>
> > > > > - data->delayedCtrls_ = std::make_unique<DelayedControls>(data->unicam_[Unicam::Image].dev(), params);<br>
> > > > > + data->delayedCtrls_ = std::make_unique<DelayedControls>(data->sensor_->device(), params);<br>
> > > > > data->sensorMetadata_ = sensorConfig.sensorMetadata;<br>
> > > > ><br>
> > > > > /* Register the controls that the Raspberry Pi IPA can handle. */<br>
> > > > > @@ -1066,15 +1102,14 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator)<br>
> > > > > * As part of answering the final question, we reset the camera to<br>
> > > > > * no transform at all.<br>
> > > > > */<br>
> > > > > -<br>
> > > > > - V4L2VideoDevice *dev = data->unicam_[Unicam::Image].dev();<br>
> > > > > - const struct v4l2_query_ext_ctrl *hflipCtrl = dev->controlInfo(V4L2_CID_HFLIP);<br>
> > > > > + const V4L2Subdevice *sensor = data->sensor_->device();<br>
> > > > > + const struct v4l2_query_ext_ctrl *hflipCtrl = sensor->controlInfo(V4L2_CID_HFLIP);<br>
> > > > > if (hflipCtrl) {<br>
> > > > > /* We assume it will support vflips too... */<br>
> > > > > data->supportsFlips_ = true;<br>
> > > > > data->flipsAlterBayerOrder_ = hflipCtrl->flags & V4L2_CTRL_FLAG_MODIFY_LAYOUT;<br>
> > > > ><br>
> > > > > - ControlList ctrls(dev->controls());<br>
> > > > > + ControlList ctrls(data->sensor_->controls());<br>
> > > > > ctrls.set(V4L2_CID_HFLIP, 0);<br>
> > > > > ctrls.set(V4L2_CID_VFLIP, 0);<br>
> > > > > data->setSensorControls(ctrls);<br>
> > > > > @@ -1082,9 +1117,8 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator)<br>
> > > > ><br>
> > > > > /* Look for a valid Bayer format. */<br>
> > > > > BayerFormat bayerFormat;<br>
> > > > > - for (const auto &iter : dev->formats()) {<br>
> > > > > - V4L2PixelFormat v4l2Format = iter.first;<br>
> > > > > - bayerFormat = BayerFormat::fromV4L2PixelFormat(v4l2Format);<br>
> > > > > + for (const auto &iter : data->sensorFormats_) {<br>
> > > > > + bayerFormat = BayerFormat::fromMbusCode(iter.first);<br>
> > > > > if (bayerFormat.isValid())<br>
> > > > > break;<br>
> > > > > }<br>
> > > > > @@ -1271,7 +1305,7 @@ int RPiCameraData::configureIPA(const CameraConfiguration *config)<br>
> > > > > }<br>
> > > > > }<br>
> > > > ><br>
> > > > > - entityControls.emplace(0, unicam_[Unicam::Image].dev()->controls());<br>
> > > > > + entityControls.emplace(0, sensor_->controls());<br>
> > > > > entityControls.emplace(1, isp_[Isp::Input].dev()->controls());<br>
> > > > ><br>
> > > > > /* Always send the user transform to the IPA. */<br>
> > > > > @@ -1406,10 +1440,10 @@ void RPiCameraData::setSensorControls(ControlList &controls)<br>
> > > > > ControlList vblank_ctrl;<br>
> > > > ><br>
> > > > > vblank_ctrl.set(V4L2_CID_VBLANK, controls.get(V4L2_CID_VBLANK));<br>
> > > > > - unicam_[Unicam::Image].dev()->setControls(&vblank_ctrl);<br>
> > > > > + sensor_->setControls(&vblank_ctrl);<br>
> > > > > }<br>
> > > > ><br>
> > > > > - unicam_[Unicam::Image].dev()->setControls(&controls);<br>
> > > > > + sensor_->setControls(&controls);<br>
> > > > > }<br>
> > > > ><br>
> > > > > void RPiCameraData::unicamBufferDequeue(FrameBuffer *buffer)<br>
<br>
-- <br>
Regards,<br>
<br>
Laurent Pinchart<br>
</blockquote></div></div>