Message ID | 20210208151558.5697-1-laurent.pinchart@ideasonboard.com |
---|---|
State | Superseded |
Delegated to: | Laurent Pinchart |
Headers | show |
Series |
|
Related | show |
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@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@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
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@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;
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;
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@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/