[libcamera-devel] [PATCH 1/2] libcamera: pipeline: RKISP1 remove rockchip-sy-mipi-dphy
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Thu Jul 4 00:39:58 CEST 2019
Hi Helen,
Thank you for the patch.
On Wed, Jul 03, 2019 at 05:21:53PM -0300, Helen Koike wrote:
> Remove subdevice rockchip-sy-mipi-dphy from the pipeline.
> Sensors are connected direcly to rkisp1-isp-subdev.
>
> Signed-off-by: Helen Koike <helen.koike at collabora.com>
>
> ---
> Hello,
>
> This depends on the v7[1] of driver being accepted upstream. But I'm
> submitting anyway for reference.
>
> [1] https://patchwork.kernel.org/project/linux-media/list/?series=141793
>
> Thanks
> Helen
> ---
> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 34 +++++-------------------
> utils/rkisp1/rkisp1-capture.sh | 4 +--
> 2 files changed, 8 insertions(+), 30 deletions(-)
>
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 4a5898d..358e2c8 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -104,7 +104,6 @@ private:
> void bufferReady(Buffer *buffer);
>
> MediaDevice *media_;
> - V4L2Subdevice *dphy_;
> V4L2Subdevice *isp_;
> V4L2VideoDevice *video_;
>
> @@ -201,8 +200,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()
> }
>
> PipelineHandlerRkISP1::PipelineHandlerRkISP1(CameraManager *manager)
> - : PipelineHandler(manager), dphy_(nullptr), isp_(nullptr),
> - video_(nullptr)
> + : PipelineHandler(manager), isp_(nullptr), video_(nullptr)
> {
> }
>
> @@ -210,7 +208,6 @@ PipelineHandlerRkISP1::~PipelineHandlerRkISP1()
> {
> delete video_;
> delete isp_;
> - delete dphy_;
> }
>
> /* -----------------------------------------------------------------------------
> @@ -250,7 +247,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();
> @@ -282,18 +279,14 @@ 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;
> -
> - ret = dphy_->getFormat(1, &format);
> - if (ret < 0)
> - return ret;
> + LOG(RkISP1, Debug) << "Configuring ISP with " << format.toString();
I think you can skip this, as the previous debugging message prints the
exact same format.
>
> ret = isp_->setFormat(0, &format);
> if (ret < 0)
> return ret;
>
> + LOG(RkISP1, Debug) << "ISP configured with " << format.toString();
> +
> V4L2DeviceFormat outputFormat = {};
> outputFormat.fourcc = cfg.pixelFormat;
> outputFormat.size = cfg.size;
> @@ -393,14 +386,6 @@ int PipelineHandlerRkISP1::initLinks()
> if (ret < 0)
> return ret;
>
> - link = media_->link("rockchip-sy-mipi-dphy", 1, "rkisp1-isp-subdev", 0);
> - if (!link)
> - return -ENODEV;
> -
> - ret = link->setEnabled(true);
> - if (ret < 0)
> - return ret;
> -
> link = media_->link("rkisp1-isp-subdev", 2, "rkisp1_mainpath", 0);
> if (!link)
> return -ENODEV;
> @@ -442,17 +427,12 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)
> dm.add("rkisp1_mainpath");
> dm.add("rkisp1-statistics");
> dm.add("rkisp1-input-params");
> - dm.add("rockchip-sy-mipi-dphy");
>
> 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)
> - return false;
> -
> isp_ = V4L2Subdevice::fromEntityName(media_, "rkisp1-isp-subdev");
> if (isp_->open() < 0)
> return false;
> @@ -471,10 +451,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
s/ISP/ISP CSI-2/
Apart from these small issues this looks good to me, so
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
I will however wait until the rkisp1 patch series goes through review
before applying this patch.
> * 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..8a6f6eb 100755
> --- a/utils/rkisp1/rkisp1-capture.sh
> +++ b/utils/rkisp1/rkisp1-capture.sh
> @@ -68,12 +68,10 @@ 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 "'$sensor':0 -> 'rkisp1-isp-subdev':0 [1]"
> $mediactl -l "'rkisp1-isp-subdev':2 -> 'rkisp1_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]"
> }
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list