[libcamera-devel,v2,1/2] pipeline: raspberrypi: Add an error state
diff mbox series

Message ID 20220921080258.5268-2-naush@raspberrypi.com
State Accepted
Commit 883e2089db307955abecfcdc95f1a85ee0be56c0
Headers show
Series
  • Raspberry Pi: Timeout error handling
Related show

Commit Message

Naushir Patuck Sept. 21, 2022, 8:02 a.m. UTC 
Add an error state used internally in the Raspberry Pi pipeline handler.
Currently this state is never set, but will be in a subsequent commit when a
device timeout has been signalled.

Add a isRunning() helper to identify if the state machine is in a stopped/error
state.

Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
 .../pipeline/raspberrypi/raspberrypi.cpp      | 22 ++++++++++++-------
 1 file changed, 14 insertions(+), 8 deletions(-)

Comments

David Plowman Sept. 23, 2022, 9:07 a.m. UTC | #1 
Hi Naush

Thanks for this patch!

On Wed, 21 Sept 2022 at 09:03, Naushir Patuck via libcamera-devel
<libcamera-devel@lists.libcamera.org> wrote:
>
> Add an error state used internally in the Raspberry Pi pipeline handler.
> Currently this state is never set, but will be in a subsequent commit when a
> device timeout has been signalled.
>
> Add a isRunning() helper to identify if the state machine is in a stopped/error
> state.
>
> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

Reviewed-by: David Plowman <david.plowman@raspberrypi.com>

Thanks
David

> ---
>  .../pipeline/raspberrypi/raspberrypi.cpp      | 22 ++++++++++++-------
>  1 file changed, 14 insertions(+), 8 deletions(-)
>
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index b4094898ca6c..5c8c89585200 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -253,9 +253,14 @@ public:
>          * thread. So, we do not need to have any mutex to protect access to any
>          * of the variables below.
>          */
> -       enum class State { Stopped, Idle, Busy, IpaComplete };
> +       enum class State { Stopped, Idle, Busy, IpaComplete, Error };
>         State state_;
>
> +       bool isRunning()
> +       {
> +               return state_ != State::Stopped && state_ != State::Error;
> +       }
> +
>         struct BayerFrame {
>                 FrameBuffer *buffer;
>                 ControlList controls;
> @@ -1109,7 +1114,7 @@ int PipelineHandlerRPi::queueRequestDevice(Camera *camera, Request *request)
>  {
>         RPiCameraData *data = cameraData(camera);
>
> -       if (data->state_ == RPiCameraData::State::Stopped)
> +       if (!data->isRunning())
>                 return -EINVAL;
>
>         LOG(RPI, Debug) << "queueRequestDevice: New request.";
> @@ -1708,7 +1713,7 @@ void RPiCameraData::enumerateVideoDevices(MediaLink *link)
>
>  void RPiCameraData::statsMetadataComplete(uint32_t bufferId, const ControlList &controls)
>  {
> -       if (state_ == State::Stopped)
> +       if (!isRunning())
>                 return;
>
>         FrameBuffer *buffer = isp_[Isp::Stats].getBuffers().at(bufferId);
> @@ -1744,7 +1749,7 @@ void RPiCameraData::statsMetadataComplete(uint32_t bufferId, const ControlList &
>
>  void RPiCameraData::runIsp(uint32_t bufferId)
>  {
> -       if (state_ == State::Stopped)
> +       if (!isRunning())
>                 return;
>
>         FrameBuffer *buffer = unicam_[Unicam::Image].getBuffers().at(bufferId);
> @@ -1759,7 +1764,7 @@ void RPiCameraData::runIsp(uint32_t bufferId)
>
>  void RPiCameraData::embeddedComplete(uint32_t bufferId)
>  {
> -       if (state_ == State::Stopped)
> +       if (!isRunning())
>                 return;
>
>         FrameBuffer *buffer = unicam_[Unicam::Embedded].getBuffers().at(bufferId);
> @@ -1825,7 +1830,7 @@ void RPiCameraData::unicamBufferDequeue(FrameBuffer *buffer)
>         RPi::Stream *stream = nullptr;
>         int index;
>
> -       if (state_ == State::Stopped)
> +       if (!isRunning())
>                 return;
>
>         for (RPi::Stream &s : unicam_) {
> @@ -1864,7 +1869,7 @@ void RPiCameraData::unicamBufferDequeue(FrameBuffer *buffer)
>
>  void RPiCameraData::ispInputDequeue(FrameBuffer *buffer)
>  {
> -       if (state_ == State::Stopped)
> +       if (!isRunning())
>                 return;
>
>         LOG(RPI, Debug) << "Stream ISP Input buffer complete"
> @@ -1881,7 +1886,7 @@ void RPiCameraData::ispOutputDequeue(FrameBuffer *buffer)
>         RPi::Stream *stream = nullptr;
>         int index;
>
> -       if (state_ == State::Stopped)
> +       if (!isRunning())
>                 return;
>
>         for (RPi::Stream &s : isp_) {
> @@ -1991,6 +1996,7 @@ void RPiCameraData::handleState()
>         switch (state_) {
>         case State::Stopped:
>         case State::Busy:
> +       case State::Error:
>                 break;
>
>         case State::IpaComplete:
> --
> 2.25.1
>

Patch
diff mbox series 

diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
index b4094898ca6c..5c8c89585200 100644
--- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
+++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
@@ -253,9 +253,14 @@  public:
 	 * thread. So, we do not need to have any mutex to protect access to any
 	 * of the variables below.
 	 */
-	enum class State { Stopped, Idle, Busy, IpaComplete };
+	enum class State { Stopped, Idle, Busy, IpaComplete, Error };
 	State state_;
 
+	bool isRunning()
+	{
+		return state_ != State::Stopped && state_ != State::Error;
+	}
+
 	struct BayerFrame {
 		FrameBuffer *buffer;
 		ControlList controls;
@@ -1109,7 +1114,7 @@  int PipelineHandlerRPi::queueRequestDevice(Camera *camera, Request *request)
 {
 	RPiCameraData *data = cameraData(camera);
 
-	if (data->state_ == RPiCameraData::State::Stopped)
+	if (!data->isRunning())
 		return -EINVAL;
 
 	LOG(RPI, Debug) << "queueRequestDevice: New request.";
@@ -1708,7 +1713,7 @@  void RPiCameraData::enumerateVideoDevices(MediaLink *link)
 
 void RPiCameraData::statsMetadataComplete(uint32_t bufferId, const ControlList &controls)
 {
-	if (state_ == State::Stopped)
+	if (!isRunning())
 		return;
 
 	FrameBuffer *buffer = isp_[Isp::Stats].getBuffers().at(bufferId);
@@ -1744,7 +1749,7 @@  void RPiCameraData::statsMetadataComplete(uint32_t bufferId, const ControlList &
 
 void RPiCameraData::runIsp(uint32_t bufferId)
 {
-	if (state_ == State::Stopped)
+	if (!isRunning())
 		return;
 
 	FrameBuffer *buffer = unicam_[Unicam::Image].getBuffers().at(bufferId);
@@ -1759,7 +1764,7 @@  void RPiCameraData::runIsp(uint32_t bufferId)
 
 void RPiCameraData::embeddedComplete(uint32_t bufferId)
 {
-	if (state_ == State::Stopped)
+	if (!isRunning())
 		return;
 
 	FrameBuffer *buffer = unicam_[Unicam::Embedded].getBuffers().at(bufferId);
@@ -1825,7 +1830,7 @@  void RPiCameraData::unicamBufferDequeue(FrameBuffer *buffer)
 	RPi::Stream *stream = nullptr;
 	int index;
 
-	if (state_ == State::Stopped)
+	if (!isRunning())
 		return;
 
 	for (RPi::Stream &s : unicam_) {
@@ -1864,7 +1869,7 @@  void RPiCameraData::unicamBufferDequeue(FrameBuffer *buffer)
 
 void RPiCameraData::ispInputDequeue(FrameBuffer *buffer)
 {
-	if (state_ == State::Stopped)
+	if (!isRunning())
 		return;
 
 	LOG(RPI, Debug) << "Stream ISP Input buffer complete"
@@ -1881,7 +1886,7 @@  void RPiCameraData::ispOutputDequeue(FrameBuffer *buffer)
 	RPi::Stream *stream = nullptr;
 	int index;
 
-	if (state_ == State::Stopped)
+	if (!isRunning())
 		return;
 
 	for (RPi::Stream &s : isp_) {
@@ -1991,6 +1996,7 @@  void RPiCameraData::handleState()
 	switch (state_) {
 	case State::Stopped:
 	case State::Busy:
+	case State::Error:
 		break;
 
 	case State::IpaComplete: