| Message ID | 20200326160819.4088361-6-niklas.soderlund@ragnatech.se |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Niklas, Thank you for the patch. On Thu, Mar 26, 2020 at 05:08:17PM +0100, Niklas Söderlund wrote: > Call the IPA start()/stop() functions before/after the camera is > started. This makes sure the IPA functions properly once thread > management is moved into the start/stop interface. > > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > --- > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 24 ++++++++++++++++++++++-- > 1 file changed, 22 insertions(+), 2 deletions(-) > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > index f49b3a7f0945ac92..3287fd3a18204cbc 100644 > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > @@ -113,8 +113,8 @@ class RkISP1CameraData : public CameraData > { > public: > RkISP1CameraData(PipelineHandler *pipe) > - : CameraData(pipe), sensor_(nullptr), frame_(0), > - frameInfo_(pipe) > + : CameraData(pipe), running_(false), sensor_(nullptr), > + frame_(0), frameInfo_(pipe) > { > } > > @@ -125,6 +125,7 @@ public: > > int loadIPA(); > > + bool running_; > Stream stream_; > CameraSensor *sensor_; > unsigned int frame_; > @@ -394,6 +395,9 @@ int RkISP1CameraData::loadIPA() > void RkISP1CameraData::queueFrameAction(unsigned int frame, > const IPAOperationData &action) > { > + if (!running_) > + return; > + > switch (action.operation) { > case RKISP1_IPA_ACTION_V4L2_SET: { > const ControlList &controls = action.controls[0]; > @@ -764,10 +768,19 @@ int PipelineHandlerRkISP1::start(Camera *camera) > if (ret) > return ret; > > + ret = data->ipa_->start(); > + if (ret) { > + freeBuffers(camera); > + LOG(RkISP1, Error) > + << "Failed to start IPA " << camera->name(); > + return ret; > + } > + I wonder if it would simplify error handling if you moved this last (the stop could go first at stop time then). Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > data->frame_ = 0; > > ret = param_->streamOn(); > if (ret) { > + data->ipa_->stop(); > freeBuffers(camera); > LOG(RkISP1, Error) > << "Failed to start parameters " << camera->name(); > @@ -777,6 +790,7 @@ int PipelineHandlerRkISP1::start(Camera *camera) > ret = stat_->streamOn(); > if (ret) { > param_->streamOff(); > + data->ipa_->stop(); > freeBuffers(camera); > LOG(RkISP1, Error) > << "Failed to start statistics " << camera->name(); > @@ -787,12 +801,14 @@ int PipelineHandlerRkISP1::start(Camera *camera) > if (ret) { > param_->streamOff(); > stat_->streamOff(); > + data->ipa_->stop(); > freeBuffers(camera); > > LOG(RkISP1, Error) > << "Failed to start camera " << camera->name(); > } > > + data->running_ = true; > activeCamera_ = camera; > > /* Inform IPA of stream configuration and sensor controls. */ > @@ -830,6 +846,10 @@ void PipelineHandlerRkISP1::stop(Camera *camera) > LOG(RkISP1, Warning) > << "Failed to stop parameters " << camera->name(); > > + data->running_ = false; > + > + data->ipa_->stop(); > + > data->timeline_.reset(); > > freeBuffers(camera);
diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index f49b3a7f0945ac92..3287fd3a18204cbc 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -113,8 +113,8 @@ class RkISP1CameraData : public CameraData { public: RkISP1CameraData(PipelineHandler *pipe) - : CameraData(pipe), sensor_(nullptr), frame_(0), - frameInfo_(pipe) + : CameraData(pipe), running_(false), sensor_(nullptr), + frame_(0), frameInfo_(pipe) { } @@ -125,6 +125,7 @@ public: int loadIPA(); + bool running_; Stream stream_; CameraSensor *sensor_; unsigned int frame_; @@ -394,6 +395,9 @@ int RkISP1CameraData::loadIPA() void RkISP1CameraData::queueFrameAction(unsigned int frame, const IPAOperationData &action) { + if (!running_) + return; + switch (action.operation) { case RKISP1_IPA_ACTION_V4L2_SET: { const ControlList &controls = action.controls[0]; @@ -764,10 +768,19 @@ int PipelineHandlerRkISP1::start(Camera *camera) if (ret) return ret; + ret = data->ipa_->start(); + if (ret) { + freeBuffers(camera); + LOG(RkISP1, Error) + << "Failed to start IPA " << camera->name(); + return ret; + } + data->frame_ = 0; ret = param_->streamOn(); if (ret) { + data->ipa_->stop(); freeBuffers(camera); LOG(RkISP1, Error) << "Failed to start parameters " << camera->name(); @@ -777,6 +790,7 @@ int PipelineHandlerRkISP1::start(Camera *camera) ret = stat_->streamOn(); if (ret) { param_->streamOff(); + data->ipa_->stop(); freeBuffers(camera); LOG(RkISP1, Error) << "Failed to start statistics " << camera->name(); @@ -787,12 +801,14 @@ int PipelineHandlerRkISP1::start(Camera *camera) if (ret) { param_->streamOff(); stat_->streamOff(); + data->ipa_->stop(); freeBuffers(camera); LOG(RkISP1, Error) << "Failed to start camera " << camera->name(); } + data->running_ = true; activeCamera_ = camera; /* Inform IPA of stream configuration and sensor controls. */ @@ -830,6 +846,10 @@ void PipelineHandlerRkISP1::stop(Camera *camera) LOG(RkISP1, Warning) << "Failed to stop parameters " << camera->name(); + data->running_ = false; + + data->ipa_->stop(); + data->timeline_.reset(); freeBuffers(camera);
Call the IPA start()/stop() functions before/after the camera is started. This makes sure the IPA functions properly once thread management is moved into the start/stop interface. Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> --- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-)