[libcamera-devel] [PATCH] libcamera: pipeline: rkisp1: sync topology with upstream driver
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Thu Jan 16 23:41:21 CET 2020
Hi Helen,
Thank you for the patch.
On Wed, Jan 15, 2020 at 09:45:53PM -0300, Helen Koike wrote:
> rkisp1 kernel driver was merged upstream with minor changes in the
> topology from the original driver libcamera based it's first support to
> rkisp1.
>
> Adapt libramera to work with upstream driver.
>
> * Remove subdevice dphy from the pipeline.
> * Add resizer in the pipeline.
> * Fix links.
> * Update entity names.
>
> Signed-off-by: Helen Koike <helen.koike at collabora.com>
> ---
>
> Hi,
>
> I'm not sure if it is better to wait the driver to get out of staging,
> but in any case I'm sending it here in case others want to use it.
I don't think we should wait for the driver to get out of staging. As
far as I'm concerned, a driver in staging is better than an out-of-tree
driver. We will switch to this for Rockchip ISP development as soon as
we can validate it. We're working on it.
> It is also available at:
> https://gitlab.collabora.com/koike/libcamera
>
> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 63 +++++++++++++-----------
> utils/rkisp1/rkisp1-capture.sh | 16 +++---
> 2 files changed, 43 insertions(+), 36 deletions(-)
>
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 389a99c..279bbb6 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -207,8 +207,8 @@ private:
> int freeBuffers(Camera *camera);
>
> MediaDevice *media_;
> - V4L2Subdevice *dphy_;
> V4L2Subdevice *isp_;
> + V4L2Subdevice *resizer_;
> V4L2VideoDevice *video_;
> V4L2VideoDevice *param_;
> V4L2VideoDevice *stat_;
> @@ -513,7 +513,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()
> }
>
> PipelineHandlerRkISP1::PipelineHandlerRkISP1(CameraManager *manager)
> - : PipelineHandler(manager), dphy_(nullptr), isp_(nullptr),
> + : PipelineHandler(manager), isp_(nullptr), resizer_(nullptr),
> video_(nullptr), param_(nullptr), stat_(nullptr)
> {
> }
> @@ -523,8 +523,8 @@ PipelineHandlerRkISP1::~PipelineHandlerRkISP1()
> delete param_;
> delete stat_;
> delete video_;
> + delete resizer_;
> delete isp_;
> - delete dphy_;
> }
>
> /* -----------------------------------------------------------------------------
> @@ -564,7 +564,7 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
> * Configure the sensor links: enable the link corresponding to this
> * camera and disable all the other sensor links.
> */
> - const MediaPad *pad = dphy_->entity()->getPadByIndex(0);
> + const MediaPad *pad = isp_->entity()->getPadByIndex(0);
>
> for (MediaLink *link : pad->links()) {
> bool enable = link->source()->entity() == sensor->entity();
> @@ -596,16 +596,6 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
>
> LOG(RkISP1, Debug) << "Sensor configured with " << format.toString();
>
> - ret = dphy_->setFormat(0, &format);
> - if (ret < 0)
> - return ret;
> -
> - LOG(RkISP1, Debug) << "Configuring ISP input pad with " << format.toString();
> -
> - ret = dphy_->getFormat(1, &format);
> - if (ret < 0)
> - return ret;
> -
> ret = isp_->setFormat(0, &format);
> if (ret < 0)
> return ret;
> @@ -622,6 +612,22 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
>
> LOG(RkISP1, Debug) << "ISP output pad configured with " << format.toString();
>
> + ret = resizer_->setFormat(0, &format);
> + if (ret < 0)
> + return ret;
> +
> + LOG(RkISP1, Debug) << "Resizer input pad configured with " << format.toString();
> +
> + format.size = cfg.size;
> +
> + LOG(RkISP1, Debug) << "Configuring resizer output pad with " << format.toString();
> +
> + ret = resizer_->setFormat(1, &format);
> + if (ret < 0)
> + return ret;
> +
> + LOG(RkISP1, Debug) << "Resizer output pad configured with " << format.toString();
> +
> V4L2DeviceFormat outputFormat = {};
> outputFormat.fourcc = video_->toV4L2Fourcc(cfg.pixelFormat);
> outputFormat.size = cfg.size;
> @@ -868,7 +874,7 @@ int PipelineHandlerRkISP1::initLinks()
> if (ret < 0)
> return ret;
>
> - link = media_->link("rockchip-sy-mipi-dphy", 1, "rkisp1-isp-subdev", 0);
> + link = media_->link("rkisp1_isp", 2, "rkisp1_resizer_mainpath", 0);
> if (!link)
> return -ENODEV;
>
> @@ -876,7 +882,7 @@ int PipelineHandlerRkISP1::initLinks()
> if (ret < 0)
> return ret;
>
> - link = media_->link("rkisp1-isp-subdev", 2, "rkisp1_mainpath", 0);
> + link = media_->link("rkisp1_resizer_mainpath", 1, "rkisp1_mainpath", 0);
> if (!link)
> return -ENODEV;
>
> @@ -923,24 +929,25 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)
> const MediaPad *pad;
>
> DeviceMatch dm("rkisp1");
> - dm.add("rkisp1-isp-subdev");
> + dm.add("rkisp1_isp");
> + dm.add("rkisp1_resizer_selfpath");
> + dm.add("rkisp1_resizer_mainpath");
> dm.add("rkisp1_selfpath");
> dm.add("rkisp1_mainpath");
> - dm.add("rkisp1-statistics");
> - dm.add("rkisp1-input-params");
> - dm.add("rockchip-sy-mipi-dphy");
> + dm.add("rkisp1_stats");
> + dm.add("rkisp1_params");
>
> media_ = acquireMediaDevice(enumerator, dm);
> if (!media_)
> return false;
>
> /* Create the V4L2 subdevices we will need. */
> - dphy_ = V4L2Subdevice::fromEntityName(media_, "rockchip-sy-mipi-dphy");
> - if (dphy_->open() < 0)
> + isp_ = V4L2Subdevice::fromEntityName(media_, "rkisp1_isp");
> + if (isp_->open() < 0)
> return false;
>
> - isp_ = V4L2Subdevice::fromEntityName(media_, "rkisp1-isp-subdev");
> - if (isp_->open() < 0)
> + resizer_ = V4L2Subdevice::fromEntityName(media_, "rkisp1_resizer_mainpath");
> + if (resizer_->open() < 0)
> return false;
>
> /* Locate and open the capture video node. */
> @@ -948,11 +955,11 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)
> if (video_->open() < 0)
> return false;
>
> - stat_ = V4L2VideoDevice::fromEntityName(media_, "rkisp1-statistics");
> + stat_ = V4L2VideoDevice::fromEntityName(media_, "rkisp1_stats");
> if (stat_->open() < 0)
> return false;
>
> - param_ = V4L2VideoDevice::fromEntityName(media_, "rkisp1-input-params");
> + param_ = V4L2VideoDevice::fromEntityName(media_, "rkisp1_params");
> if (param_->open() < 0)
> return false;
>
> @@ -967,10 +974,10 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)
> }
>
> /*
> - * Enumerate all sensors connected to the CSI-2 receiver and create one
> + * Enumerate all sensors connected to the ISP receiver and create one
> * camera instance for each of them.
> */
> - pad = dphy_->entity()->getPadByIndex(0);
> + pad = isp_->entity()->getPadByIndex(0);
> if (!pad)
> return false;
>
> diff --git a/utils/rkisp1/rkisp1-capture.sh b/utils/rkisp1/rkisp1-capture.sh
> index cffe9fe..4d09f5d 100755
> --- a/utils/rkisp1/rkisp1-capture.sh
> +++ b/utils/rkisp1/rkisp1-capture.sh
> @@ -68,14 +68,14 @@ configure_pipeline() {
>
> $mediactl -r
>
> - $mediactl -l "'$sensor':0 -> 'rockchip-sy-mipi-dphy':0 [1]"
> - $mediactl -l "'rockchip-sy-mipi-dphy':1 -> 'rkisp1-isp-subdev':0 [1]"
> - $mediactl -l "'rkisp1-isp-subdev':2 -> 'rkisp1_mainpath':0 [1]"
> + $mediactl -l "'$sensor':0 -> 'rkisp1_isp':0 [1]"
> + $mediactl -l "'rkisp1_isp':2 -> 'rkisp1_resizer_mainpath':0 [1]"
>
> $mediactl -V "\"$sensor\":0 [$format]"
> - $mediactl -V "'rockchip-sy-mipi-dphy':1 [$format]"
> - $mediactl -V "'rkisp1-isp-subdev':0 [$format crop:(0,0)/$sensor_size]"
> - $mediactl -V "'rkisp1-isp-subdev':2 [fmt:$capture_mbus_code/$capture_size crop:(0,0)/$capture_size]"
> + $mediactl -V "'rkisp1_isp':0 [$format crop:(0,0)/$sensor_size]"
> + $mediactl -V "'rkisp1_isp':2 [fmt:$capture_mbus_code/$sensor_size crop:(0,0)/$sensor_size]"
> + $mediactl -V "'rkisp1_resizer_mainpath':0 [fmt:$capture_mbus_code/$sensor_size crop:(0,0)/$sensor_size]"
> + $mediactl -V "'rkisp1_resizer_mainpath':1 [fmt:$capture_mbus_code/$capture_size]"
> }
>
> # Capture frames
> @@ -161,8 +161,8 @@ fi
>
> sensor_name=$1
>
> -modprobe mipi_dphy_sy
> -modprobe video_rkisp1
> +modprobe phy_rockchip_dphy_rx0
> +modprobe rockchip_isp1
>
> sensor=$(find_sensor $sensor_name) || exit
> mdev=$(find_media_device rkisp1) || exit
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list