[libcamera-devel] [PATCH] libcamera: pipeline: ipu3: Stop IPA before stopping devices

[Kieran Bingham]

On 08/03/2021 16:37, Niklas Söderlund wrote:
> Hi Kieran,
> 
> On 2021-03-08 16:29:15 +0000, Kieran Bingham wrote:
>> The IPA should be stopped before the hardware devices to ensure that
>> all asynchronous actions have completed within the IPA before resources
>> are removed and released.
>>
>> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>> ---
>>  src/libcamera/pipeline/ipu3/ipu3.cpp | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
>> index 2b4d31500533..498f85634a83 100644
>> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
>> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
>> @@ -764,13 +764,13 @@ void PipelineHandlerIPU3::stop(Camera *camera)
>>  	IPU3CameraData *data = cameraData(camera);
>>  	int ret = 0;
>>  
>> +	data->ipa_->stop();
>> +
>>  	ret |= data->imgu_->stop();
>>  	ret |= data->cio2_.stop();
>>  	if (ret)
>>  		LOG(IPU3, Warning) << "Failed to stop camera " << camera->id();
>>  
>> -	data->ipa_->stop();
>> -
> 
> IIRC correctly this order was picked because before the cio2 is stopped 
> buffers may still be dequeued and therefor handed to the IPA for 
> processing. This was however done before the new IPC change and with a 
> mock IPA. How does this work with the new IPC/IPA if the pipeline 
> handler tries to post buffers to be processed before the CIO2 is 
> stopped?

The IPA before stopped sends events that cause thigns to be queued to
the v4l2 devices.

This hits the fact that the v42l buffer cache is null because the
buffers have been released.

So we must stop the IPA before the devices..


But then we must also make sure the IPA is not processing frames and
events from buffer completions after it has stopped....


> 
>>  	freeBuffers(camera);
> 
> Should not all resources shared between pipeline and IPA be available 
> until the buffers are freed here?

We're battling a use-after-free I think at the minute so indeed, this
could be relevant.

> 
>>  }
>>  
>> -- 
>> 2.25.1
>>
>> _______________________________________________
>> libcamera-devel mailing list
>> libcamera-devel at lists.libcamera.org
>> https://lists.libcamera.org/listinfo/libcamera-devel
> 

-- 
Regards
--
Kieran