| Message ID | 20200116004553.2885674-1-helen.koike@collabora.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
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@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
Hi Helen, Thanks for your work. On 2020-01-15 21:45:53 -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@collabora.com> I tested this change with the driver merged in the media-tree and it works as expected, great work! I have some small nits bellow but other then that I think it's ready to go. When testing this I was unable to locate any up-to-date patches updating devicetree files to the state of the driver in staging and had to hack something of my own to get it working. Is there patches somewhere? Is their a plan to upstream the DT changes for Scarlet ? > --- > > 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. > > It is also available at: > https://gitlab.collabora.com/koike/libcamera > > Thanks > Helen > > 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 I would s/receiver// > * 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() { I would split the changes to the capture script in it's own patch. > > $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 > -- > 2.24.0 > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hi, On 1/16/20 10:27 PM, Niklas Söderlund wrote: > Hi Helen, > > Thanks for your work. > > On 2020-01-15 21:45:53 -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@collabora.com> > > I tested this change with the driver merged in the media-tree and it > works as expected, great work! I have some small nits bellow but other > then that I think it's ready to go. > > When testing this I was unable to locate any up-to-date patches updating > devicetree files to the state of the driver in staging and had to hack > something of my own to get it working. Is there patches somewhere? Is > their a plan to upstream the DT changes for Scarlet ? I have a draft version, which is available in my tree: https://gitlab.collabora.com/koike/linux/tree/rockchip/isp/v13 I plan to upstream them, but the bindings are still in staging, and I want to take a closer look to this patchset before we move the bindings out of staging: https://patchwork.kernel.org/patch/10988217/ Looks like they conflict, or describe the same thing (I plan to take a look and reply to the patch). > >> --- >> >> 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. >> >> It is also available at: >> https://gitlab.collabora.com/koike/libcamera >> >> Thanks >> Helen >> >> 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 > > I would s/receiver// ack > >> * 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() { > > I would split the changes to the capture script in it's own patch. ok Thanks Helen > >> >> $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 >> -- >> 2.24.0 >> >> _______________________________________________ >> libcamera-devel mailing list >> libcamera-devel@lists.libcamera.org >> https://lists.libcamera.org/listinfo/libcamera-devel >
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
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@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. It is also available at: https://gitlab.collabora.com/koike/libcamera Thanks Helen src/libcamera/pipeline/rkisp1/rkisp1.cpp | 63 +++++++++++++----------- utils/rkisp1/rkisp1-capture.sh | 16 +++--- 2 files changed, 43 insertions(+), 36 deletions(-)