[libcamera-devel] [PATCH/RFC] libcamera: pipeline: simple: Support camera sensors that contain an ISP

[Laurent Pinchart]

Hi Jacopo,

On Mon, Feb 08, 2021 at 05:08:25PM +0100, Jacopo Mondi wrote:
> On Mon, Feb 08, 2021 at 05:15:58PM +0200, Laurent Pinchart wrote:
> > Camera sensors can include an ISP. For instance, the AP1302 external ISP
> > can be connected to up to two raw camera sensors, and the combination of
> > the sensors and ISP is considered as a (smart) camera sensor from
> > libcamera's point of view.
> >
> > The CameraSensor class has limited support for this already. Extend the
> > simple pipeline handler to support such sensors, by using the media
> > entity corresponding to the ISP instead of the raw camera sensor's
> > entity.
> >
> > We don't need to handle the case where an entity in the SoC would expose
> > the MEDIA_ENT_F_PROC_VIDEO_ISP function, as pipeline containing an ISP
> > would have a dedicated pipeline handler.
> >
> > The implementation is limited as it won't support other multi-entity
> > camera sensors (such as CCS). While this would be worth supporting, we
> > don't have a test platform with a CCS-compatible sensor at this point,
> > so let's not over-engineer the solution. Extending support to CCS (and
> > possibly other sensor topologies) will likely involve helpers that can
> > be used by other pipeline handlers (such as generic graph walk helpers
> > for instance) and extensions to the CameraSensor class.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> >  src/libcamera/pipeline/simple/simple.cpp | 87 ++++++++++++++++++++----
> >  1 file changed, 74 insertions(+), 13 deletions(-)
> >
> > This patch depends on "[PATCH/RFC] libcamera: camera_sensor: Accept
> > entities exposing the ISP function" ([1]). It has been tested on a
> > MediaTek Pumpkin i500 board with an ON Semiconductor AP1302 and AR0330
> > and AR0144 sensors.
> >
> > [1] https://patchwork.libcamera.org/patch/11182/
> 
> Doesn't it depend on the introduction of such entity function in
> kernel space too ?

Yes it does. That's scheduled for v5.12-rc1.

> Also, I'm not sure if the model for sensor that expose multiple
> entities has really been finalized. In example I don't see any driver
> for those sensors in mainline. Am I mistaken ?

You're not. The AP1302 driver should be posted soon. I wanted to get
this patch out as an RFC first to see if there was a general agreement
on the direction and approach, and I'll maintain it out of tree as long
as needed.

> > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> > index 9d468f7c9cc4..2df33a3a79ea 100644
> > --- a/src/libcamera/pipeline/simple/simple.cpp
> > +++ b/src/libcamera/pipeline/simple/simple.cpp
> > @@ -240,6 +240,8 @@ private:
> >  			PipelineHandler::cameraData(camera));
> >  	}
> >
> > +	std::vector<MediaEntity *> locateSensors();
> > +
> >  	void bufferReady(FrameBuffer *buffer);
> >  	void converterInputDone(FrameBuffer *buffer);
> >  	void converterOutputDone(FrameBuffer *buffer);
> > @@ -865,6 +867,77 @@ int SimplePipelineHandler::queueRequestDevice(Camera *camera, Request *request)
> >   * Match and Setup
> >   */
> >
> > +std::vector<MediaEntity *> SimplePipelineHandler::locateSensors()
> > +{
> > +	std::vector<MediaEntity *> entities;
> > +
> > +	/*
> > +	 * Gather all the camera sensor entities based on the function they
> > +	 * expose.
> > +	 */
> > +	for (MediaEntity *entity : media_->entities()) {
> > +		if (entity->function() == MEDIA_ENT_F_CAM_SENSOR)
> > +			entities.push_back(entity);
> > +	}
> > +
> > +	if (entities.empty())
> > +		return {};
> > +
> > +	/*
> > +	 * Sensors can be made of multiple entities. For instance, a raw sensor
> > +	 * can be connected to an ISP, and the combination of both should be
> > +	 * treated as one sensor. To support this, as a crude heuristic, check
> > +	 * the downstream entity from the camera sensor, and if it is an ISP,
> > +	 * use it instead of the sensor.
> > +	 */
> > +	std::vector<MediaEntity *> sensors;
> > +
> > +	for (MediaEntity *entity : entities) {
> > +		/*
> > +		 * Locate the downstream entity by following the first enabled
> > +		 * link from a source pad.
> > +		 */
> > +		const MediaPad *pad = nullptr;
> > +		const MediaLink *link = nullptr;
> > +
> > +		for (const MediaPad *p : entity->pads()) {
> > +			if (p->flags() & MEDIA_PAD_FL_SOURCE) {
> > +				pad = p;
> > +				break;
> > +			}
> > +		}
> > +
> > +		if (!pad)
> > +			continue;
> > +
> > +		for (const MediaLink *l : pad->links()) {
> > +			if (l->flags() & MEDIA_LNK_FL_ENABLED) {
> > +				link = l;
> > +				break;
> > +			}
> > +		}
> > +
> > +		if (!link)
> > +			continue;
> > +
> > +		MediaEntity *remote = link->sink()->entity();
> > +		if (remote->function() == MEDIA_ENT_F_PROC_VIDEO_ISP)
> > +			sensors.push_back(remote);
> > +		else
> > +			sensors.push_back(entity);
> > +	}
> > +
> > +	/*
> > +	 * Remove duplicates, in case multiple sensors are connected to the
> > +	 * same ISP.
> > +	 */
> > +	std::sort(sensors.begin(), sensors.end());
> > +	auto last = std::unique(sensors.begin(), sensors.end());
> > +	sensors.erase(last, sensors.end());
> > +
> > +	return sensors;
> > +}
> > +
> >  bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)
> >  {
> >  	const SimplePipelineInfo *info = nullptr;
> > @@ -888,19 +961,7 @@ bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)
> >  	}
> >
> >  	/* Locate the sensors. */
> > -	std::vector<MediaEntity *> sensors;
> > -
> > -	for (MediaEntity *entity : media_->entities()) {
> > -		switch (entity->function()) {
> > -		case MEDIA_ENT_F_CAM_SENSOR:
> > -			sensors.push_back(entity);
> > -			break;
> > -
> > -		default:
> > -			break;
> > -		}
> > -	}
> > -
> > +	std::vector<MediaEntity *> sensors = locateSensors();
> >  	if (sensors.empty()) {
> >  		LOG(SimplePipeline, Error) << "No sensor found";
> >  		return false;

-- 
Regards,

Laurent Pinchart