1 files changed, 48 insertions, 29 deletions

diff --git a/test/fence.cpp b/test/fence.cpp
index 1e38bc2f..8095b228 100644
--- a/test/fence.cpp
+++ b/test/fence.cpp
@@ -2,7 +2,7 @@
 /*
  * Copyright (C) 2021, Google Inc.
  *
- * fence.cpp - Fence test
+ * Fence test
  */
 
 #include <iostream>
@@ -43,7 +43,6 @@ private:
 
 	void signalFence();
 
-	std::unique_ptr<Fence> fence_;
 	EventDispatcher *dispatcher_;
 	UniqueFD eventFd_;
 	UniqueFD eventFd2_;
@@ -58,8 +57,11 @@ private:
 	bool expectedCompletionResult_ = true;
 	bool setFence_ = true;
 
-	unsigned int completedRequest_;
-
+	/*
+	 * Request IDs track the number of requests that have completed. They
+	 * are one-based, and don't wrap.
+	 */
+	unsigned int completedRequestId_;
 	unsigned int signalledRequestId_;
 	unsigned int expiredRequestId_;
 	unsigned int nbuffers_;
@@ -127,8 +129,19 @@ int FenceTest::init()
 		return TestFail;
 	}
 
-	signalledRequestId_ = nbuffers_ - 2;
-	expiredRequestId_ = nbuffers_ - 1;
+	completedRequestId_ = 0;
+
+	/*
+	 * All but two requests are queued without a fence. Request
+	 * expiredRequestId_ will be queued with a fence that we won't signal
+	 * (which is then expected to expire), and request signalledRequestId_
+	 * will be queued with a fence that gets signalled. Select nbuffers_
+	 * and nbuffers_ * 2 for those two requests, to space them by a few
+	 * frames while still not requiring a long time for the test to
+	 * complete.
+	 */
+	expiredRequestId_ = nbuffers_;
+	signalledRequestId_ = nbuffers_ * 2;
 
 	return TestPass;
 }
@@ -190,16 +203,16 @@ void FenceTest::requestRequeue(Request *request)
 	const Request::BufferMap &buffers = request->buffers();
 	const Stream *stream = buffers.begin()->first;
 	FrameBuffer *buffer = buffers.begin()->second;
-	uint64_t cookie = request->cookie();
 
 	request->reuse();
 
-	if (cookie == signalledRequestId_ && setFence_) {
+	if (completedRequestId_ == signalledRequestId_ - nbuffers_ && setFence_) {
 		/*
-		 * The second time this request is queued add a fence to it.
-		 *
-		 * The main loop signals it by using a timer to write to the
-		 * efd2_ file descriptor before the fence expires.
+		 * This is the request that will be used to test fence
+		 * signalling when it completes next time. Add a fence to it,
+		 * using efd2_. The main loop will signal the fence by using a
+		 * timer to write to the efd2_ file descriptor before the fence
+		 * expires.
 		 */
 		std::unique_ptr<Fence> fence =
 			std::make_unique<Fence>(std::move(eventFd2_));
@@ -214,16 +227,15 @@ void FenceTest::requestRequeue(Request *request)
 
 void FenceTest::requestComplete(Request *request)
 {
-	uint64_t cookie = request->cookie();
-	completedRequest_ = cookie;
+	completedRequestId_++;
 
 	/*
-	 * The last request is expected to fail as its fence has not been
-	 * signaled.
+	 * Request expiredRequestId_ is expected to fail as its fence has not
+	 * been signalled.
 	 *
 	 * Validate the fence status but do not re-queue it.
 	 */
-	if (cookie == expiredRequestId_) {
+	if (completedRequestId_ == expiredRequestId_) {
 		if (validateExpiredRequest(request) != TestPass)
 			expectedCompletionResult_ = false;
 
@@ -231,7 +243,7 @@ void FenceTest::requestComplete(Request *request)
 		return;
 	}
 
-	/* Validate all requests but the last. */
+	/* Validate all other requests. */
 	if (validateRequest(request) != TestPass) {
 		expectedCompletionResult_ = false;
 
@@ -272,15 +284,16 @@ int FenceTest::run()
 		}
 
 		int ret;
-		if (i == expiredRequestId_) {
+		if (i == expiredRequestId_ - 1) {
 			/* This request will have a fence, and it will expire. */
-			fence_ = std::make_unique<Fence>(std::move(eventFd_));
-			if (!fence_->isValid()) {
+			std::unique_ptr<Fence> fence =
+				std::make_unique<Fence>(std::move(eventFd_));
+			if (!fence->isValid()) {
 				cerr << "Fence should be valid" << endl;
 				return TestFail;
 			}
 
-			ret = request->addBuffer(stream_, buffer.get(), std::move(fence_));
+			ret = request->addBuffer(stream_, buffer.get(), std::move(fence));
 		} else {
 			/* All other requests will have no Fence. */
 			ret = request->addBuffer(stream_, buffer.get());
@@ -314,15 +327,21 @@ int FenceTest::run()
 	Timer fenceTimer;
 	fenceTimer.timeout.connect(this, &FenceTest::signalFence);
 
-	/* Loop for one second. */
+	/*
+	 * Loop long enough for all requests to complete, allowing 500ms per
+	 * request.
+	 */
 	Timer timer;
-	timer.start(1000ms);
-	while (timer.isRunning() && expectedCompletionResult_) {
-		if (completedRequest_ == signalledRequestId_ && setFence_)
+	timer.start(500ms * (signalledRequestId_ + 1));
+	while (timer.isRunning() && expectedCompletionResult_ &&
+	       completedRequestId_ <= signalledRequestId_ + 1) {
+		if (completedRequestId_ == signalledRequestId_ - 1 && setFence_)
 			/*
-			 * signalledRequestId_ has just completed and it has
-			 * been re-queued with a fence. Start the timer to
-			 * signal the fence in 10 msec.
+			 * The request just before signalledRequestId_ has just
+			 * completed. Request signalledRequestId_ has been
+			 * queued with a fence, and libcamera is likely already
+			 * waiting on the fence, or will soon. Start the timer
+			 * to signal the fence in 10 msec.
 			 */
 			fenceTimer.start(10ms);