Jmondi/rkisp1 ipu3 ipa/kbingham rfc/v2

include/libcamera/internal/request.h

@@ -42,6 +42,8 @@ class Request::Private : public Extensible::Private
 	void prepare(std::chrono::milliseconds timeout = 0ms);
 	Signal<> prepared;
 
+	void setError(Errors error);
+
 private:
 	friend class PipelineHandler;
 	friend std::ostream &operator<<(std::ostream &out, const Request &r);
@@ -59,6 +61,8 @@ class Request::Private : public Extensible::Private
 	std::unordered_set<FrameBuffer *> pending_;
 	std::map<FrameBuffer *, std::unique_ptr<EventNotifier>> notifiers_;
 	std::unique_ptr<Timer> timer_;
+
+	Errors error_;
 };
 
 } /* namespace libcamera */

include/libcamera/request.h

@@ -15,6 +15,7 @@
 #include <unordered_set>
 
 #include <libcamera/base/class.h>
+#include <libcamera/base/flags.h>
 #include <libcamera/base/signal.h>
 
 #include <libcamera/controls.h>
@@ -43,6 +44,14 @@ class Request : public Extensible
 		ReuseBuffers = (1 << 0),
 	};
 
+	enum ErrorId {
+		NoError = 0,
+		ControlError = (1 << 0),
+		PFCError = (1 << 1),
+	};
+
+	using Errors = Flags<ErrorId>;
+
 	using BufferMap = std::map<const Stream *, FrameBuffer *>;
 
 	Request(Camera *camera, uint64_t cookie = 0);
@@ -60,6 +69,7 @@ class Request : public Extensible
 	uint32_t sequence() const;
 	uint64_t cookie() const { return cookie_; }
 	Status status() const { return status_; }
+	Errors error() const;
 
 	bool hasPendingBuffers() const;
 

src/ipa/ipu3/ipa_context.cpp

@@ -180,27 +180,10 @@ namespace libcamera::ipa::ipu3 {
  * <linux/intel-ipu3.h> struct ipu3_uapi_gamma_corr_lut for further details.
  */
 
-/**
- * \brief Default constructor for IPAFrameContext
- */
-IPAFrameContext::IPAFrameContext() = default;
-
-/**
- * \brief Construct a IPAFrameContext instance
- */
-IPAFrameContext::IPAFrameContext(uint32_t id, const ControlList &reqControls)
-	: frame(id), frameControls(reqControls)
-{
-	sensor = {};
-}
-
 /**
  * \var IPAFrameContext::frame
  * \brief The frame number
  *
- * \var IPAFrameContext::frameControls
- * \brief Controls sent in by the application while queuing the request
- *
  * \var IPAFrameContext::sensor
  * \brief Effective sensor values that were applied for the frame
  *
@@ -209,6 +192,9 @@ IPAFrameContext::IPAFrameContext(uint32_t id, const ControlList &reqControls)
  *
  * \var IPAFrameContext::sensor.gain
  * \brief Analogue gain multiplier
+ *
+ * \var IPAFrameContext::error
+ * \brief The error flags for this frame context
  */
 
 } /* namespace libcamera::ipa::ipu3 */

src/ipa/ipu3/ipa_context.h

@@ -8,22 +8,20 @@
 
 #pragma once
 
-#include <array>
-
 #include <linux/intel-ipu3.h>
 
 #include <libcamera/base/utils.h>
 
 #include <libcamera/controls.h>
 #include <libcamera/geometry.h>
+#include <libcamera/request.h>
+
+#include <libipa/fc_queue.h>
 
 namespace libcamera {
 
 namespace ipa::ipu3 {
 
-/* Maximum number of frame contexts to be held */
-static constexpr uint32_t kMaxFrameContexts = 16;
-
 struct IPASessionConfiguration {
 	struct {
 		ipu3_uapi_grid_config bdsGrid;
@@ -77,23 +75,21 @@ struct IPAActiveState {
 };
 
 struct IPAFrameContext {
-	IPAFrameContext();
-	IPAFrameContext(uint32_t id, const ControlList &reqControls);
+	uint32_t frame;
 
 	struct {
 		uint32_t exposure;
 		double gain;
 	} sensor;
 
-	uint32_t frame;
-	ControlList frameControls;
+	Request::Errors error;
 };
 
 struct IPAContext {
 	IPASessionConfiguration configuration;
 	IPAActiveState activeState;
 
-	std::array<IPAFrameContext, kMaxFrameContexts> frameContexts;
+	FCQueue<IPAFrameContext> frameContexts;
 };
 
 } /* namespace ipa::ipu3 */

src/ipa/ipu3/ipu3.cpp

@@ -38,6 +38,8 @@
 #include "algorithms/tone_mapping.h"
 #include "libipa/camera_sensor_helper.h"
 
+#include "ipa_context.h"
+
 /* Minimum grid width, expressed as a number of cells */
 static constexpr uint32_t kMinGridWidth = 16;
 /* Maximum grid width, expressed as a number of cells */
@@ -348,6 +350,9 @@ int IPAIPU3::start()
  */
 void IPAIPU3::stop()
 {
+	/* Clean the IPA context at the end of the streaming session. */
+	context_.frameContexts.clear();
+	context_ = {};
 }
 
 /**
@@ -454,11 +459,6 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo,
 
 	calculateBdsGrid(configInfo.bdsOutputSize);
 
-	/* Clean IPAActiveState at each reconfiguration. */
-	context_.activeState = {};
-	IPAFrameContext initFrameContext;
-	context_.frameContexts.fill(initFrameContext);
-
 	if (!validateSensorControls()) {
 		LOG(IPAIPU3, Error) << "Sensor control validation failed.";
 		return -EINVAL;
@@ -569,7 +569,7 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame,
 	const ipu3_uapi_stats_3a *stats =
 		reinterpret_cast<ipu3_uapi_stats_3a *>(mem.data());
 
-	IPAFrameContext &frameContext = context_.frameContexts[frame % kMaxFrameContexts];
+	IPAFrameContext &frameContext = context_.frameContexts.get(frame);
 
 	if (frameContext.frame != frame)
 		LOG(IPAIPU3, Warning) << "Frame " << frame << " does not match its frame context";
@@ -618,7 +618,11 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame,
 void IPAIPU3::queueRequest(const uint32_t frame, const ControlList &controls)
 {
 	/* \todo Start processing for 'frame' based on 'controls'. */
-	context_.frameContexts[frame % kMaxFrameContexts] = { frame, controls };
+	IPAFrameContext &frameContext = context_.frameContexts.initialise(frame);
+
+	/* \todo Implement queueRequest to each algorithm. */
+	(void)frameContext;
+	(void)controls;
 }
 
 /**

src/ipa/libipa/fc_queue.cpp

@@ -0,0 +1,112 @@
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+/*
+ * Copyright (C) 2022, Google Inc.
+ *
+ * fc_queue.cpp - IPA Frame context queue
+ */
+
+#include "fc_queue.h"
+
+#include <libcamera/base/log.h>
+
+namespace libcamera {
+
+LOG_DEFINE_CATEGORY(FCQueue)
+
+namespace ipa {
+
+/**
+ * \file fc_queue.h
+ * \brief Queue to access per-frame Context
+ */
+
+/**
+ * \class FCQueue
+ * \brief A support class for queueing FrameContext instances through the IPA
+ * \tparam FrameContext The IPA specific FrameContext derived class type
+ *
+ * The frame context queue provides a simple circular buffer implementation to
+ * store IPA specific context for each frame through its lifetime within the
+ * IPA.
+ *
+ * FrameContext instances are expected to be referenced by a monotonically
+ * increasing sequence count corresponding to a frame sequence number.
+ *
+ * A FrameContext is initialised for a given frame when the corresponding
+ * Request is first queued into the IPA. From that point on the FrameContext can
+ * be obtained by the IPA and its algorithms by referencing it from the frame
+ * sequence number.
+ *
+ * A frame sequence number from the image source must correspond to the request
+ * sequence number for this implementation to be supported in an IPA. It is
+ * expected that the same sequence number will be used to reference the context
+ * of the Frame from the point of queueing the request, specifying controls for
+ * a given frame, and processing of any ISP related controls and statistics for
+ * the same corresponding image.
+ *
+ * IPA specific frame context implementations shall inherit from the
+ * IPAFrameContext base class to support the minimum required features for a
+ * FrameContext, including the frame number and the error flags that relate to
+ * the frame.
+ *
+ * FrameContexts are overwritten when they are recycled and re-initialised by
+ * the first access made on them by either initialise(frame) or get(frame). It
+ * is required that the number of available slots in the frame context queue is
+ * larger or equal to the maximum number of in-flight requests a pipeline can
+ * handle to avoid overwriting frame context instances that still have to be
+ * processed.
+ *
+ * In the case an application does not queue requests to the Camera fast
+ * enough, frames can be produced and processed by the IPA without a
+ * corresponding Request being queued. In this case the IPA algorithm
+ * will try to access the FrameContext with a call to get() before it
+ * had been initialized at queueRequest() time. In this case there
+ * is now way the controls associated with the late Request could be
+ * applied in time, as the frame as already been processed by the IPA,
+ * the PFCError flag is automatically raised on the FrameContext.
+ */
+
+/**
+ * \fn FCQueue::clear()
+ * \brief Clear the context queue
+ *
+ * Reset the queue and ensure all FrameContext slots are re-initialised.
+ * IPA modules are expected to clear the frame context queue at the beginning of
+ * a new streaming session, in IPAModule::start().
+ */
+
+/**
+ * \fn FCQueue::initialise(uint32_t frame)
+ * \brief Initialize and return the Frame Context at slot specified by \a frame
+ * \param[in] frame The frame context sequence number
+ *
+ * The first call to obtain a FrameContext from the FCQueue should be handled
+ * through this call. The FrameContext will be initialised for the frame and
+ * returned to the caller if it was not already initialised.
+ *
+ * If the FrameContext was already initialized for this sequence, a warning will
+ * be reported and the previously initialized FrameContext is returned.
+ *
+ * Frame contexts are expected to be initialised when a Request is first passed
+ * to the IPA module in IPAModule::queueRequest().
+ *
+ * \return A reference to the FrameContext for sequence \a frame
+ */
+
+/**
+ * \fn FCQueue::get()
+ * \brief Obtain the Frame Context at slot specified by \a frame
+ * \param[in] frame The frame context sequence number
+ *
+ * Obtains an existing FrameContext from the queue and returns it to the caller.
+ *
+ * If the FrameContext is not correctly initialised for the \a frame, it will be
+ * initialised and a PFCError will be flagged on the context to be transferred
+ * to the Request when it completes.
+ *
+ * \return A reference to the FrameContext for sequence \a frame
+ */
+
+} /* namespace ipa */
+
+} /* namespace libcamera */