| Message ID | 20200628231934.29025-6-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Laurent, Thanks for your work. On 2020-06-29 02:19:30 +0300, Laurent Pinchart wrote: > The PipelineHandlerRPi::configureIPA() function accesses plenty of > member data from the RPiCameraData class and no member from the > PipelineHandlerRPi class. Move it to RPiCameraData where it logically > belongs. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > --- > .../pipeline/raspberrypi/raspberrypi.cpp | 117 +++++++++--------- > 1 file changed, 58 insertions(+), 59 deletions(-) > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > index 3b5cdf1e1849..0f9237a7f346 100644 > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > @@ -311,6 +311,8 @@ public: > void frameStarted(uint32_t sequence); > > int loadIPA(); > + int configureIPA(); > + > void queueFrameAction(unsigned int frame, const IPAOperationData &action); > > /* bufferComplete signal handlers. */ > @@ -396,8 +398,6 @@ private: > return static_cast<RPiCameraData *>(PipelineHandler::cameraData(camera)); > } > > - int configureIPA(Camera *camera); > - > int queueAllBuffers(Camera *camera); > int prepareBuffers(Camera *camera); > void freeBuffers(Camera *camera); > @@ -752,7 +752,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config) > crop.y = (sensorFormat.size.height - crop.height) >> 1; > data->isp_[Isp::Input].dev()->setSelection(V4L2_SEL_TGT_CROP, &crop); > > - ret = configureIPA(camera); > + ret = data->configureIPA(); > if (ret) > LOG(RPI, Error) << "Failed to configure the IPA: " << ret; > > @@ -968,62 +968,6 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator) > return true; > } > > -int PipelineHandlerRPi::configureIPA(Camera *camera) > -{ > - std::map<unsigned int, IPAStream> streamConfig; > - std::map<unsigned int, const ControlInfoMap &> entityControls; > - RPiCameraData *data = cameraData(camera); > - > - /* Get the device format to pass to the IPA. */ > - V4L2DeviceFormat sensorFormat; > - data->unicam_[Unicam::Image].dev()->getFormat(&sensorFormat); > - /* Inform IPA of stream configuration and sensor controls. */ > - unsigned int i = 0; > - for (auto const &stream : data->isp_) { > - if (stream.isExternal()) { > - streamConfig[i] = { > - .pixelFormat = stream.configuration().pixelFormat, > - .size = stream.configuration().size > - }; > - } > - } > - entityControls.emplace(0, data->unicam_[Unicam::Image].dev()->controls()); > - entityControls.emplace(1, data->isp_[Isp::Input].dev()->controls()); > - > - /* Allocate the lens shading table via vcsm and pass to the IPA. */ > - if (!data->lsTable_) { > - data->lsTable_ = data->vcsm_.alloc("ls_grid", MAX_LS_GRID_SIZE); > - uintptr_t ptr = reinterpret_cast<uintptr_t>(data->lsTable_); > - > - if (!data->lsTable_) > - return -ENOMEM; > - > - /* > - * The vcsm allocation will always be in the memory region > - * < 32-bits to allow Videocore to access the memory. > - */ > - IPAOperationData op; > - op.operation = RPI_IPA_EVENT_LS_TABLE_ALLOCATION; > - op.data = { static_cast<uint32_t>(ptr & 0xffffffff), > - data->vcsm_.getVCHandle(data->lsTable_) }; > - data->ipa_->processEvent(op); > - } > - > - CameraSensorInfo sensorInfo = {}; > - int ret = data->sensor_->sensorInfo(&sensorInfo); > - if (ret) { > - LOG(RPI, Error) << "Failed to retrieve camera sensor info"; > - return ret; > - } > - > - /* Ready the IPA - it must know about the sensor resolution. */ > - IPAOperationData ipaConfig; > - data->ipa_->configure(sensorInfo, streamConfig, entityControls, > - ipaConfig, nullptr); > - > - return 0; > -} > - > int PipelineHandlerRPi::queueAllBuffers(Camera *camera) > { > RPiCameraData *data = cameraData(camera); > @@ -1161,6 +1105,61 @@ int RPiCameraData::loadIPA() > return ipa_->start(); > } > > +int RPiCameraData::configureIPA() > +{ > + std::map<unsigned int, IPAStream> streamConfig; > + std::map<unsigned int, const ControlInfoMap &> entityControls; > + > + /* Get the device format to pass to the IPA. */ > + V4L2DeviceFormat sensorFormat; > + unicam_[Unicam::Image].dev()->getFormat(&sensorFormat); > + /* Inform IPA of stream configuration and sensor controls. */ > + unsigned int i = 0; > + for (auto const &stream : isp_) { > + if (stream.isExternal()) { > + streamConfig[i] = { > + .pixelFormat = stream.configuration().pixelFormat, > + .size = stream.configuration().size > + }; > + } > + } > + entityControls.emplace(0, unicam_[Unicam::Image].dev()->controls()); > + entityControls.emplace(1, isp_[Isp::Input].dev()->controls()); > + > + /* Allocate the lens shading table via vcsm and pass to the IPA. */ > + if (!lsTable_) { > + lsTable_ = vcsm_.alloc("ls_grid", MAX_LS_GRID_SIZE); > + uintptr_t ptr = reinterpret_cast<uintptr_t>(lsTable_); > + > + if (!lsTable_) > + return -ENOMEM; > + > + /* > + * The vcsm allocation will always be in the memory region > + * < 32-bits to allow Videocore to access the memory. > + */ > + IPAOperationData op; > + op.operation = RPI_IPA_EVENT_LS_TABLE_ALLOCATION; > + op.data = { static_cast<uint32_t>(ptr & 0xffffffff), > + vcsm_.getVCHandle(lsTable_) }; > + ipa_->processEvent(op); > + } > + > + CameraSensorInfo sensorInfo = {}; > + int ret = sensor_->sensorInfo(&sensorInfo); > + if (ret) { > + LOG(RPI, Error) << "Failed to retrieve camera sensor info"; > + return ret; > + } > + > + /* Ready the IPA - it must know about the sensor resolution. */ > + IPAOperationData ipaConfig; > + ipa_->configure(sensorInfo, streamConfig, entityControls, ipaConfig, > + nullptr); > + > + return 0; > +} > + > void RPiCameraData::queueFrameAction(unsigned int frame, const IPAOperationData &action) > { > /* > -- > Regards, > > Laurent Pinchart > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hi Laurent, Thank you for the patch. On Mon, 29 Jun 2020 at 15:33, Niklas Söderlund <niklas.soderlund@ragnatech.se> wrote: > > Hi Laurent, > > Thanks for your work. > > On 2020-06-29 02:19:30 +0300, Laurent Pinchart wrote: > > The PipelineHandlerRPi::configureIPA() function accesses plenty of > > member data from the RPiCameraData class and no member from the > > PipelineHandlerRPi class. Move it to RPiCameraData where it logically > > belongs. > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> Reviewed-by: Naushir Patuck <naush@raspberrypi.com> > > > --- > > .../pipeline/raspberrypi/raspberrypi.cpp | 117 +++++++++--------- > > 1 file changed, 58 insertions(+), 59 deletions(-) > > > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > index 3b5cdf1e1849..0f9237a7f346 100644 > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > @@ -311,6 +311,8 @@ public: > > void frameStarted(uint32_t sequence); > > > > int loadIPA(); > > + int configureIPA(); > > + > > void queueFrameAction(unsigned int frame, const IPAOperationData &action); > > > > /* bufferComplete signal handlers. */ > > @@ -396,8 +398,6 @@ private: > > return static_cast<RPiCameraData *>(PipelineHandler::cameraData(camera)); > > } > > > > - int configureIPA(Camera *camera); > > - > > int queueAllBuffers(Camera *camera); > > int prepareBuffers(Camera *camera); > > void freeBuffers(Camera *camera); > > @@ -752,7 +752,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config) > > crop.y = (sensorFormat.size.height - crop.height) >> 1; > > data->isp_[Isp::Input].dev()->setSelection(V4L2_SEL_TGT_CROP, &crop); > > > > - ret = configureIPA(camera); > > + ret = data->configureIPA(); > > if (ret) > > LOG(RPI, Error) << "Failed to configure the IPA: " << ret; > > > > @@ -968,62 +968,6 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator) > > return true; > > } > > > > -int PipelineHandlerRPi::configureIPA(Camera *camera) > > -{ > > - std::map<unsigned int, IPAStream> streamConfig; > > - std::map<unsigned int, const ControlInfoMap &> entityControls; > > - RPiCameraData *data = cameraData(camera); > > - > > - /* Get the device format to pass to the IPA. */ > > - V4L2DeviceFormat sensorFormat; > > - data->unicam_[Unicam::Image].dev()->getFormat(&sensorFormat); > > - /* Inform IPA of stream configuration and sensor controls. */ > > - unsigned int i = 0; > > - for (auto const &stream : data->isp_) { > > - if (stream.isExternal()) { > > - streamConfig[i] = { > > - .pixelFormat = stream.configuration().pixelFormat, > > - .size = stream.configuration().size > > - }; > > - } > > - } > > - entityControls.emplace(0, data->unicam_[Unicam::Image].dev()->controls()); > > - entityControls.emplace(1, data->isp_[Isp::Input].dev()->controls()); > > - > > - /* Allocate the lens shading table via vcsm and pass to the IPA. */ > > - if (!data->lsTable_) { > > - data->lsTable_ = data->vcsm_.alloc("ls_grid", MAX_LS_GRID_SIZE); > > - uintptr_t ptr = reinterpret_cast<uintptr_t>(data->lsTable_); > > - > > - if (!data->lsTable_) > > - return -ENOMEM; > > - > > - /* > > - * The vcsm allocation will always be in the memory region > > - * < 32-bits to allow Videocore to access the memory. > > - */ > > - IPAOperationData op; > > - op.operation = RPI_IPA_EVENT_LS_TABLE_ALLOCATION; > > - op.data = { static_cast<uint32_t>(ptr & 0xffffffff), > > - data->vcsm_.getVCHandle(data->lsTable_) }; > > - data->ipa_->processEvent(op); > > - } > > - > > - CameraSensorInfo sensorInfo = {}; > > - int ret = data->sensor_->sensorInfo(&sensorInfo); > > - if (ret) { > > - LOG(RPI, Error) << "Failed to retrieve camera sensor info"; > > - return ret; > > - } > > - > > - /* Ready the IPA - it must know about the sensor resolution. */ > > - IPAOperationData ipaConfig; > > - data->ipa_->configure(sensorInfo, streamConfig, entityControls, > > - ipaConfig, nullptr); > > - > > - return 0; > > -} > > - > > int PipelineHandlerRPi::queueAllBuffers(Camera *camera) > > { > > RPiCameraData *data = cameraData(camera); > > @@ -1161,6 +1105,61 @@ int RPiCameraData::loadIPA() > > return ipa_->start(); > > } > > > > +int RPiCameraData::configureIPA() > > +{ > > + std::map<unsigned int, IPAStream> streamConfig; > > + std::map<unsigned int, const ControlInfoMap &> entityControls; > > + > > + /* Get the device format to pass to the IPA. */ > > + V4L2DeviceFormat sensorFormat; > > + unicam_[Unicam::Image].dev()->getFormat(&sensorFormat); > > + /* Inform IPA of stream configuration and sensor controls. */ > > + unsigned int i = 0; > > + for (auto const &stream : isp_) { > > + if (stream.isExternal()) { > > + streamConfig[i] = { > > + .pixelFormat = stream.configuration().pixelFormat, > > + .size = stream.configuration().size > > + }; > > + } > > + } > > + entityControls.emplace(0, unicam_[Unicam::Image].dev()->controls()); > > + entityControls.emplace(1, isp_[Isp::Input].dev()->controls()); > > + > > + /* Allocate the lens shading table via vcsm and pass to the IPA. */ > > + if (!lsTable_) { > > + lsTable_ = vcsm_.alloc("ls_grid", MAX_LS_GRID_SIZE); > > + uintptr_t ptr = reinterpret_cast<uintptr_t>(lsTable_); > > + > > + if (!lsTable_) > > + return -ENOMEM; > > + > > + /* > > + * The vcsm allocation will always be in the memory region > > + * < 32-bits to allow Videocore to access the memory. > > + */ > > + IPAOperationData op; > > + op.operation = RPI_IPA_EVENT_LS_TABLE_ALLOCATION; > > + op.data = { static_cast<uint32_t>(ptr & 0xffffffff), > > + vcsm_.getVCHandle(lsTable_) }; > > + ipa_->processEvent(op); > > + } > > + > > + CameraSensorInfo sensorInfo = {}; > > + int ret = sensor_->sensorInfo(&sensorInfo); > > + if (ret) { > > + LOG(RPI, Error) << "Failed to retrieve camera sensor info"; > > + return ret; > > + } > > + > > + /* Ready the IPA - it must know about the sensor resolution. */ > > + IPAOperationData ipaConfig; > > + ipa_->configure(sensorInfo, streamConfig, entityControls, ipaConfig, > > + nullptr); > > + > > + return 0; > > +} > > + > > void RPiCameraData::queueFrameAction(unsigned int frame, const IPAOperationData &action) > > { > > /* > > -- > > Regards, > > > > Laurent Pinchart > > > > _______________________________________________ > > libcamera-devel mailing list > > libcamera-devel@lists.libcamera.org > > https://lists.libcamera.org/listinfo/libcamera-devel > > -- > Regards, > Niklas Söderlund > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hi Laurent, On Mon, Jun 29, 2020 at 02:19:30AM +0300, Laurent Pinchart wrote: > The PipelineHandlerRPi::configureIPA() function accesses plenty of > member data from the RPiCameraData class and no member from the > PipelineHandlerRPi class. Move it to RPiCameraData where it logically > belongs. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > .../pipeline/raspberrypi/raspberrypi.cpp | 117 +++++++++--------- > 1 file changed, 58 insertions(+), 59 deletions(-) > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > index 3b5cdf1e1849..0f9237a7f346 100644 > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > @@ -311,6 +311,8 @@ public: > void frameStarted(uint32_t sequence); > > int loadIPA(); > + int configureIPA(); > + > void queueFrameAction(unsigned int frame, const IPAOperationData &action); > > /* bufferComplete signal handlers. */ > @@ -396,8 +398,6 @@ private: > return static_cast<RPiCameraData *>(PipelineHandler::cameraData(camera)); > } > > - int configureIPA(Camera *camera); > - > int queueAllBuffers(Camera *camera); > int prepareBuffers(Camera *camera); > void freeBuffers(Camera *camera); > @@ -752,7 +752,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config) > crop.y = (sensorFormat.size.height - crop.height) >> 1; > data->isp_[Isp::Input].dev()->setSelection(V4L2_SEL_TGT_CROP, &crop); > > - ret = configureIPA(camera); > + ret = data->configureIPA(); > if (ret) > LOG(RPI, Error) << "Failed to configure the IPA: " << ret; > > @@ -968,62 +968,6 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator) > return true; > } > > -int PipelineHandlerRPi::configureIPA(Camera *camera) > -{ > - std::map<unsigned int, IPAStream> streamConfig; > - std::map<unsigned int, const ControlInfoMap &> entityControls; > - RPiCameraData *data = cameraData(camera); > - > - /* Get the device format to pass to the IPA. */ > - V4L2DeviceFormat sensorFormat; > - data->unicam_[Unicam::Image].dev()->getFormat(&sensorFormat); > - /* Inform IPA of stream configuration and sensor controls. */ > - unsigned int i = 0; > - for (auto const &stream : data->isp_) { > - if (stream.isExternal()) { > - streamConfig[i] = { > - .pixelFormat = stream.configuration().pixelFormat, > - .size = stream.configuration().size > - }; > - } > - } > - entityControls.emplace(0, data->unicam_[Unicam::Image].dev()->controls()); > - entityControls.emplace(1, data->isp_[Isp::Input].dev()->controls()); > - > - /* Allocate the lens shading table via vcsm and pass to the IPA. */ > - if (!data->lsTable_) { > - data->lsTable_ = data->vcsm_.alloc("ls_grid", MAX_LS_GRID_SIZE); > - uintptr_t ptr = reinterpret_cast<uintptr_t>(data->lsTable_); > - > - if (!data->lsTable_) > - return -ENOMEM; > - > - /* > - * The vcsm allocation will always be in the memory region > - * < 32-bits to allow Videocore to access the memory. > - */ > - IPAOperationData op; > - op.operation = RPI_IPA_EVENT_LS_TABLE_ALLOCATION; > - op.data = { static_cast<uint32_t>(ptr & 0xffffffff), > - data->vcsm_.getVCHandle(data->lsTable_) }; > - data->ipa_->processEvent(op); > - } > - > - CameraSensorInfo sensorInfo = {}; > - int ret = data->sensor_->sensorInfo(&sensorInfo); > - if (ret) { > - LOG(RPI, Error) << "Failed to retrieve camera sensor info"; > - return ret; > - } > - > - /* Ready the IPA - it must know about the sensor resolution. */ > - IPAOperationData ipaConfig; > - data->ipa_->configure(sensorInfo, streamConfig, entityControls, > - ipaConfig, nullptr); > - > - return 0; > -} > - > int PipelineHandlerRPi::queueAllBuffers(Camera *camera) > { > RPiCameraData *data = cameraData(camera); > @@ -1161,6 +1105,61 @@ int RPiCameraData::loadIPA() > return ipa_->start(); > } > > +int RPiCameraData::configureIPA() > +{ > + std::map<unsigned int, IPAStream> streamConfig; > + std::map<unsigned int, const ControlInfoMap &> entityControls; > + > + /* Get the device format to pass to the IPA. */ > + V4L2DeviceFormat sensorFormat; > + unicam_[Unicam::Image].dev()->getFormat(&sensorFormat); > + /* Inform IPA of stream configuration and sensor controls. */ > + unsigned int i = 0; > + for (auto const &stream : isp_) { > + if (stream.isExternal()) { > + streamConfig[i] = { > + .pixelFormat = stream.configuration().pixelFormat, > + .size = stream.configuration().size > + }; > + } > + } > + entityControls.emplace(0, unicam_[Unicam::Image].dev()->controls()); > + entityControls.emplace(1, isp_[Isp::Input].dev()->controls()); > + > + /* Allocate the lens shading table via vcsm and pass to the IPA. */ > + if (!lsTable_) { > + lsTable_ = vcsm_.alloc("ls_grid", MAX_LS_GRID_SIZE); > + uintptr_t ptr = reinterpret_cast<uintptr_t>(lsTable_); > + > + if (!lsTable_) > + return -ENOMEM; > + > + /* > + * The vcsm allocation will always be in the memory region > + * < 32-bits to allow Videocore to access the memory. > + */ > + IPAOperationData op; > + op.operation = RPI_IPA_EVENT_LS_TABLE_ALLOCATION; > + op.data = { static_cast<uint32_t>(ptr & 0xffffffff), > + vcsm_.getVCHandle(lsTable_) }; > + ipa_->processEvent(op); > + } > + > + CameraSensorInfo sensorInfo = {}; > + int ret = sensor_->sensorInfo(&sensorInfo); > + if (ret) { > + LOG(RPI, Error) << "Failed to retrieve camera sensor info"; > + return ret; > + } > + > + /* Ready the IPA - it must know about the sensor resolution. */ "Ready the IPA" ? I didn't get what you mean :) nit apart Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> Thanks j > + IPAOperationData ipaConfig; > + ipa_->configure(sensorInfo, streamConfig, entityControls, ipaConfig, > + nullptr); > + > + return 0; > +} > + > void RPiCameraData::queueFrameAction(unsigned int frame, const IPAOperationData &action) > { > /* > -- > Regards, > > Laurent Pinchart > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hi Jacopo, On Tue, Jun 30, 2020 at 12:38:37PM +0200, Jacopo Mondi wrote: > On Mon, Jun 29, 2020 at 02:19:30AM +0300, Laurent Pinchart wrote: > > The PipelineHandlerRPi::configureIPA() function accesses plenty of > > member data from the RPiCameraData class and no member from the > > PipelineHandlerRPi class. Move it to RPiCameraData where it logically > > belongs. > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > > .../pipeline/raspberrypi/raspberrypi.cpp | 117 +++++++++--------- > > 1 file changed, 58 insertions(+), 59 deletions(-) > > > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > index 3b5cdf1e1849..0f9237a7f346 100644 > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > @@ -311,6 +311,8 @@ public: > > void frameStarted(uint32_t sequence); > > > > int loadIPA(); > > + int configureIPA(); > > + > > void queueFrameAction(unsigned int frame, const IPAOperationData &action); > > > > /* bufferComplete signal handlers. */ > > @@ -396,8 +398,6 @@ private: > > return static_cast<RPiCameraData *>(PipelineHandler::cameraData(camera)); > > } > > > > - int configureIPA(Camera *camera); > > - > > int queueAllBuffers(Camera *camera); > > int prepareBuffers(Camera *camera); > > void freeBuffers(Camera *camera); > > @@ -752,7 +752,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config) > > crop.y = (sensorFormat.size.height - crop.height) >> 1; > > data->isp_[Isp::Input].dev()->setSelection(V4L2_SEL_TGT_CROP, &crop); > > > > - ret = configureIPA(camera); > > + ret = data->configureIPA(); > > if (ret) > > LOG(RPI, Error) << "Failed to configure the IPA: " << ret; > > > > @@ -968,62 +968,6 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator) > > return true; > > } > > > > -int PipelineHandlerRPi::configureIPA(Camera *camera) > > -{ > > - std::map<unsigned int, IPAStream> streamConfig; > > - std::map<unsigned int, const ControlInfoMap &> entityControls; > > - RPiCameraData *data = cameraData(camera); > > - > > - /* Get the device format to pass to the IPA. */ > > - V4L2DeviceFormat sensorFormat; > > - data->unicam_[Unicam::Image].dev()->getFormat(&sensorFormat); > > - /* Inform IPA of stream configuration and sensor controls. */ > > - unsigned int i = 0; > > - for (auto const &stream : data->isp_) { > > - if (stream.isExternal()) { > > - streamConfig[i] = { > > - .pixelFormat = stream.configuration().pixelFormat, > > - .size = stream.configuration().size > > - }; > > - } > > - } > > - entityControls.emplace(0, data->unicam_[Unicam::Image].dev()->controls()); > > - entityControls.emplace(1, data->isp_[Isp::Input].dev()->controls()); > > - > > - /* Allocate the lens shading table via vcsm and pass to the IPA. */ > > - if (!data->lsTable_) { > > - data->lsTable_ = data->vcsm_.alloc("ls_grid", MAX_LS_GRID_SIZE); > > - uintptr_t ptr = reinterpret_cast<uintptr_t>(data->lsTable_); > > - > > - if (!data->lsTable_) > > - return -ENOMEM; > > - > > - /* > > - * The vcsm allocation will always be in the memory region > > - * < 32-bits to allow Videocore to access the memory. > > - */ > > - IPAOperationData op; > > - op.operation = RPI_IPA_EVENT_LS_TABLE_ALLOCATION; > > - op.data = { static_cast<uint32_t>(ptr & 0xffffffff), > > - data->vcsm_.getVCHandle(data->lsTable_) }; > > - data->ipa_->processEvent(op); > > - } > > - > > - CameraSensorInfo sensorInfo = {}; > > - int ret = data->sensor_->sensorInfo(&sensorInfo); > > - if (ret) { > > - LOG(RPI, Error) << "Failed to retrieve camera sensor info"; > > - return ret; > > - } > > - > > - /* Ready the IPA - it must know about the sensor resolution. */ > > - IPAOperationData ipaConfig; > > - data->ipa_->configure(sensorInfo, streamConfig, entityControls, > > - ipaConfig, nullptr); > > - > > - return 0; > > -} > > - > > int PipelineHandlerRPi::queueAllBuffers(Camera *camera) > > { > > RPiCameraData *data = cameraData(camera); > > @@ -1161,6 +1105,61 @@ int RPiCameraData::loadIPA() > > return ipa_->start(); > > } > > > > +int RPiCameraData::configureIPA() > > +{ > > + std::map<unsigned int, IPAStream> streamConfig; > > + std::map<unsigned int, const ControlInfoMap &> entityControls; > > + > > + /* Get the device format to pass to the IPA. */ > > + V4L2DeviceFormat sensorFormat; > > + unicam_[Unicam::Image].dev()->getFormat(&sensorFormat); > > + /* Inform IPA of stream configuration and sensor controls. */ > > + unsigned int i = 0; > > + for (auto const &stream : isp_) { > > + if (stream.isExternal()) { > > + streamConfig[i] = { > > + .pixelFormat = stream.configuration().pixelFormat, > > + .size = stream.configuration().size > > + }; > > + } > > + } > > + entityControls.emplace(0, unicam_[Unicam::Image].dev()->controls()); > > + entityControls.emplace(1, isp_[Isp::Input].dev()->controls()); > > + > > + /* Allocate the lens shading table via vcsm and pass to the IPA. */ > > + if (!lsTable_) { > > + lsTable_ = vcsm_.alloc("ls_grid", MAX_LS_GRID_SIZE); > > + uintptr_t ptr = reinterpret_cast<uintptr_t>(lsTable_); > > + > > + if (!lsTable_) > > + return -ENOMEM; > > + > > + /* > > + * The vcsm allocation will always be in the memory region > > + * < 32-bits to allow Videocore to access the memory. > > + */ > > + IPAOperationData op; > > + op.operation = RPI_IPA_EVENT_LS_TABLE_ALLOCATION; > > + op.data = { static_cast<uint32_t>(ptr & 0xffffffff), > > + vcsm_.getVCHandle(lsTable_) }; > > + ipa_->processEvent(op); > > + } > > + > > + CameraSensorInfo sensorInfo = {}; > > + int ret = sensor_->sensorInfo(&sensorInfo); > > + if (ret) { > > + LOG(RPI, Error) << "Failed to retrieve camera sensor info"; > > + return ret; > > + } > > + > > + /* Ready the IPA - it must know about the sensor resolution. */ > > "Ready the IPA" ? I didn't get what you mean :) I didn't mean anything, this patch only moves code :-) But I think it means https://en.wiktionary.org/wiki/ready#Verb. We can update the comment, but I wouldn't do so in this patch. > nit apart > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> > > > + IPAOperationData ipaConfig; > > + ipa_->configure(sensorInfo, streamConfig, entityControls, ipaConfig, > > + nullptr); > > + > > + return 0; > > +} > > + > > void RPiCameraData::queueFrameAction(unsigned int frame, const IPAOperationData &action) > > { > > /*
diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp index 3b5cdf1e1849..0f9237a7f346 100644 --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp @@ -311,6 +311,8 @@ public: void frameStarted(uint32_t sequence); int loadIPA(); + int configureIPA(); + void queueFrameAction(unsigned int frame, const IPAOperationData &action); /* bufferComplete signal handlers. */ @@ -396,8 +398,6 @@ private: return static_cast<RPiCameraData *>(PipelineHandler::cameraData(camera)); } - int configureIPA(Camera *camera); - int queueAllBuffers(Camera *camera); int prepareBuffers(Camera *camera); void freeBuffers(Camera *camera); @@ -752,7 +752,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config) crop.y = (sensorFormat.size.height - crop.height) >> 1; data->isp_[Isp::Input].dev()->setSelection(V4L2_SEL_TGT_CROP, &crop); - ret = configureIPA(camera); + ret = data->configureIPA(); if (ret) LOG(RPI, Error) << "Failed to configure the IPA: " << ret; @@ -968,62 +968,6 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator) return true; } -int PipelineHandlerRPi::configureIPA(Camera *camera) -{ - std::map<unsigned int, IPAStream> streamConfig; - std::map<unsigned int, const ControlInfoMap &> entityControls; - RPiCameraData *data = cameraData(camera); - - /* Get the device format to pass to the IPA. */ - V4L2DeviceFormat sensorFormat; - data->unicam_[Unicam::Image].dev()->getFormat(&sensorFormat); - /* Inform IPA of stream configuration and sensor controls. */ - unsigned int i = 0; - for (auto const &stream : data->isp_) { - if (stream.isExternal()) { - streamConfig[i] = { - .pixelFormat = stream.configuration().pixelFormat, - .size = stream.configuration().size - }; - } - } - entityControls.emplace(0, data->unicam_[Unicam::Image].dev()->controls()); - entityControls.emplace(1, data->isp_[Isp::Input].dev()->controls()); - - /* Allocate the lens shading table via vcsm and pass to the IPA. */ - if (!data->lsTable_) { - data->lsTable_ = data->vcsm_.alloc("ls_grid", MAX_LS_GRID_SIZE); - uintptr_t ptr = reinterpret_cast<uintptr_t>(data->lsTable_); - - if (!data->lsTable_) - return -ENOMEM; - - /* - * The vcsm allocation will always be in the memory region - * < 32-bits to allow Videocore to access the memory. - */ - IPAOperationData op; - op.operation = RPI_IPA_EVENT_LS_TABLE_ALLOCATION; - op.data = { static_cast<uint32_t>(ptr & 0xffffffff), - data->vcsm_.getVCHandle(data->lsTable_) }; - data->ipa_->processEvent(op); - } - - CameraSensorInfo sensorInfo = {}; - int ret = data->sensor_->sensorInfo(&sensorInfo); - if (ret) { - LOG(RPI, Error) << "Failed to retrieve camera sensor info"; - return ret; - } - - /* Ready the IPA - it must know about the sensor resolution. */ - IPAOperationData ipaConfig; - data->ipa_->configure(sensorInfo, streamConfig, entityControls, - ipaConfig, nullptr); - - return 0; -} - int PipelineHandlerRPi::queueAllBuffers(Camera *camera) { RPiCameraData *data = cameraData(camera); @@ -1161,6 +1105,61 @@ int RPiCameraData::loadIPA() return ipa_->start(); } +int RPiCameraData::configureIPA() +{ + std::map<unsigned int, IPAStream> streamConfig; + std::map<unsigned int, const ControlInfoMap &> entityControls; + + /* Get the device format to pass to the IPA. */ + V4L2DeviceFormat sensorFormat; + unicam_[Unicam::Image].dev()->getFormat(&sensorFormat); + /* Inform IPA of stream configuration and sensor controls. */ + unsigned int i = 0; + for (auto const &stream : isp_) { + if (stream.isExternal()) { + streamConfig[i] = { + .pixelFormat = stream.configuration().pixelFormat, + .size = stream.configuration().size + }; + } + } + entityControls.emplace(0, unicam_[Unicam::Image].dev()->controls()); + entityControls.emplace(1, isp_[Isp::Input].dev()->controls()); + + /* Allocate the lens shading table via vcsm and pass to the IPA. */ + if (!lsTable_) { + lsTable_ = vcsm_.alloc("ls_grid", MAX_LS_GRID_SIZE); + uintptr_t ptr = reinterpret_cast<uintptr_t>(lsTable_); + + if (!lsTable_) + return -ENOMEM; + + /* + * The vcsm allocation will always be in the memory region + * < 32-bits to allow Videocore to access the memory. + */ + IPAOperationData op; + op.operation = RPI_IPA_EVENT_LS_TABLE_ALLOCATION; + op.data = { static_cast<uint32_t>(ptr & 0xffffffff), + vcsm_.getVCHandle(lsTable_) }; + ipa_->processEvent(op); + } + + CameraSensorInfo sensorInfo = {}; + int ret = sensor_->sensorInfo(&sensorInfo); + if (ret) { + LOG(RPI, Error) << "Failed to retrieve camera sensor info"; + return ret; + } + + /* Ready the IPA - it must know about the sensor resolution. */ + IPAOperationData ipaConfig; + ipa_->configure(sensorInfo, streamConfig, entityControls, ipaConfig, + nullptr); + + return 0; +} + void RPiCameraData::queueFrameAction(unsigned int frame, const IPAOperationData &action) { /*
The PipelineHandlerRPi::configureIPA() function accesses plenty of member data from the RPiCameraData class and no member from the PipelineHandlerRPi class. Move it to RPiCameraData where it logically belongs. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- .../pipeline/raspberrypi/raspberrypi.cpp | 117 +++++++++--------- 1 file changed, 58 insertions(+), 59 deletions(-)