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

[Jacopo Mondi]

Hi Laurent,

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 ?

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 ?

>
> 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
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel