[libcamera-devel] [PATCH 2/2] test: fence: Signal fence once

[Umang Jain]

Hi Jacopo,

On 12/13/21 8:54 PM, Laurent Pinchart wrote:
> Hi Jacopo,
>
> Thank you for the patch.
>
> On Mon, Dec 13, 2021 at 03:51:43PM +0100, Jacopo Mondi wrote:
>> The unit test associates a fence with a framebuffer and makes sure
>> the fence is correctly signalled.
>>
>> Once a fence is correctly signalled, it is released by the core, and the
>> underlying file descriptor closed.
>>
>> The unit test however tries to write on the fence file descriptor every
>> time the designated Request is queued, and error which is now visible
> s/and/an/
>
>> as the return value of the fence signalling write() is checked.
>>
>> Fix that by associating a fence with a framebuffer only once.
>>
>> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
>> ---
>>   test/fence.cpp | 6 ++++--
>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/test/fence.cpp b/test/fence.cpp
>> index d2865398784e..524db2a10757 100644
>> --- a/test/fence.cpp
>> +++ b/test/fence.cpp
>> @@ -56,6 +56,7 @@ private:
>>   	Stream *stream_;
>>   
>>   	bool expectedCompletionResult_ = true;
>> +	bool setFence_ = true;
>>   
>>   	unsigned int completedRequest_;
>>   
>> @@ -193,7 +194,7 @@ void FenceTest::requestRequeue(Request *request)
>>   
>>   	request->reuse();
>>   
>> -	if (cookie == signalledRequestId_) {
>> +	if (cookie == signalledRequestId_ && setFence_) {
>>   		/*
>>   		 * The second time this request is queued add a fence to it.
>>   		 *
>> @@ -257,6 +258,7 @@ void FenceTest::signalFence()
>>   	if (ret != sizeof(value))
>>   		cerr << "Failed to signal fence" << endl;
>>   
>> +	setFence_ = false;
>>   	dispatcher_->processEvents();
>>   }
>>   
>> @@ -316,7 +318,7 @@ int FenceTest::run()
>>   	Timer timer;
>>   	timer.start(1000);
>>   	while (timer.isRunning() && expectedCompletionResult_) {
>> -		if (completedRequest_ == signalledRequestId_)
>> +		if (completedRequest_ == signalledRequestId_ && setFence_)
> A bit of a hack, but I suppose it works. The test is hard to read,
> there's room for improvement (and possibly a race condition or two), but
> that's not urgent.
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> Tested-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>


Reviewed-by: Umang Jain <umang.jain at ideasonboard.com>

>
>>   			/*
>>   			 * signalledRequestId_ has just completed and it has
>>   			 * been re-queued with a fence. Start the timer to