[libcamera-devel] [PATCH/RFC] libcamera: camera_sensor: Accept entities exposing the ISP function

[Laurent Pinchart]

Hi Niklas,

On Mon, Feb 08, 2021 at 09:16:01PM +0100, Niklas Söderlund wrote:
> On 2021-02-08 03:39:03 +0200, Laurent Pinchart wrote:
> > Camera sensors can include an ISP, which may be reported as a separate
> > entity from the pixel array in the media graph.
> > 
> > Support such sensors by accepting MEDIA_ENT_F_PROC_VIDEO_ISP as a valid
> > entity type. This allows using sensors that can be fully (or at least
> > meaningfully) configured through the ISP's source pad only. Sensors that
> > require further configuration, on the ISP sink pad and/or on the pixel
> > array's source pad, will require further extension to the CameraSensor
> > class.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> >  src/libcamera/camera_sensor.cpp | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> > 
> > This patch depends on the new MEDIA_ENT_F_PROC_VIDEO_ISP entity
> > function, which will be available in v5.12-rc1. I'll update the kernel
> > headers when that kernel will be released, in a few weeks from now.
> 
> Do I understand things correctly that such sensors would primarily be 
> used with the simple pipeline handler?

That's correct, it's the primary target.

> How would this work if someones 
> wires up a sensor with an ISP to a pipeline with an IPA? Is the inline 
> ISP close to the sensor always "passive" and controlled by user-space or 
> can we create a tug-of-war between the sensor inline ISP and a pipeline 
> IPA/ISP?

If someone wired a "smart" sensor to an SoC with an ISP, we would likely
need to disable one of the two ISPs. Such a design could occur with the
aim of using the camera sensor in raw mode with the SoC-side ISP (if the
camera sensor supports raw capture in addition of YUV), or to use the
sensor-side ISP only. This isn't a situation we support today, and we
would need to further extend the CameraSensor class, as well as the
pipeline handlers.

Note that this patch in itself will not cause the CameraSensor class to
incorrectly try to handle the SoC-side ISP for a raw sensor connected
directly to an SoC ISP entity, as it's the pipeline handler that decides
which entity to pass to the CameraSensor class.

> > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
> > index 59834ffcdd94..8158a84b63db 100644
> > --- a/src/libcamera/camera_sensor.cpp
> > +++ b/src/libcamera/camera_sensor.cpp
> > @@ -198,7 +198,12 @@ int CameraSensor::init()
> >  		return -EINVAL;
> >  	}
> >  
> > -	if (entity_->function() != MEDIA_ENT_F_CAM_SENSOR) {
> > +	switch (entity_->function()) {
> > +	case MEDIA_ENT_F_CAM_SENSOR:
> > +	case MEDIA_ENT_F_PROC_VIDEO_ISP:
> > +		break;
> > +
> 
> nit: I would remove this blank line.

I like the blank line :-) But I can drop it too.

> > +	default:
> >  		LOG(CameraSensor, Error)
> >  			<< "Invalid sensor function "
> >  			<< utils::hex(entity_->function());

-- 
Regards,

Laurent Pinchart