[libcamera-devel] [PATCH v2] libcamera: ipu3: set V4L2_CID_INTEL_IPU3_MODE
Kieran Bingham
kieran.bingham at ideasonboard.com
Fri Feb 19 16:48:03 CET 2021
On 19/02/2021 14:19, Jean-Michel Hautbois wrote:
> Hi Kieran,
>
> On 19/02/2021 15:12, Kieran Bingham wrote:
>> Hi Jean-Michel,
>>
>> On 12/02/2021 15:27, Jacopo Mondi wrote:
>>> Hi Laurent,
>>>
>>> On Fri, Feb 12, 2021 at 04:17:49PM +0200, Laurent Pinchart wrote:
>>>> Hi Jean-Michel,
>>>>
>>>> Thank you for the patch.
>>>>
>>>> On Fri, Feb 12, 2021 at 02:18:51PM +0100, Jean-Michel Hautbois wrote:
>>>>> In order to get the stats back, the imgu subdev needs to have the
>>>>> V4L2_CID_INTEL_IPU3_MODE control set.
>>>>> Set it to video mode by default to get the stats at each frame.
>>>>>
>>>>> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois at ideasonboard.com>
>>>>> Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
>>>>> ---
>>>>> src/libcamera/pipeline/ipu3/ipu3.cpp | 8 ++++++--
>>>>> 1 file changed, 6 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
>>>>> index 61f7bf43..6957e3a9 100644
>>>>> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
>>>>> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
>>>>> @@ -551,9 +551,13 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)
>>>>>
>>>>> /* Apply the "pipe_mode" control to the ImgU subdevice. */
>>>>> ControlList ctrls(imgu->imgu_->controls());
>>>>> + /*
>>>>> + * \todo Set the ImgU pipe mode to 'Video' unconditionally to have statistics
>>>>> + * generated and re-consider if 'Still Capture' should be used for
>>>>> + * high quality RAW capture operations that do not involve the IPA.
>>>>
>>>> Raw images are captured at the output of the CIO2, the ImgU won't be in
>>>> the pipeline. However, when capturing raw frames we still want to run
>>>> the IPA, so we'll need statistics, which will be produced by the ImgU.
>>>> The output images will be discarded in that case.
>>>
>>> Correct, sorry I was confused, of course RAW won't go through the ImgU
>>>
>>>>
>>>> I however agree that we need to check what the use cases for still
>>>> capture mode are. If the mode exists, it's probably meant for something
>>>> (although one never knows, I think you have first hand experience with
>>>> this). How about the following comment ?
>>>>
>>>> /*
>>>> * Set the ImgU pipe mode to 'Video' unconditionally to have statistics
>>>> * generated.
>>>> *
>>>> * \todo Figure out what the 'Still Capture' mode is meant for, and use
>>>> * it accordingly.
>>>> */
>>>
>>> Agree. The question stays, what is still capture for if it does not
>>> generate statistics and goes through the ImgU...
>>
>> Are you happy with the updated comment? Will you post a v3? or should I
>> just apply Laurent's suggestion before merging?
>
> Oh dear, forgot about it :-(.
> Yes, Laurent's comment it better, I can send a v3 or let you apply it
> with updated comment, as you prefer !
Pushed with the updated comment.
--
Kieran
>
> JM
>
>> --
>> Kieran
>>
>>
>>>
>>>>
>>>> With this,
>>>>
>>>> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>>>>
>>>>> + */
>>>>> ctrls.set(V4L2_CID_IPU3_PIPE_MODE,
>>>>> - static_cast<int32_t>(vfCfg ? IPU3PipeModeVideo :
>>>>> - IPU3PipeModeStillCapture));
>>>>> + static_cast<int32_t>(IPU3PipeModeVideo));
>>>>> ret = imgu->imgu_->setControls(&ctrls);
>>>>> if (ret) {
>>>>> LOG(IPU3, Error) << "Unable to set pipe_mode control";
>>>>
>>>> --
>>>> Regards,
>>>>
>>>> Laurent Pinchart
>>>> _______________________________________________
>>>> libcamera-devel mailing list
>>>> libcamera-devel at lists.libcamera.org
>>>> https://lists.libcamera.org/listinfo/libcamera-devel
>>> _______________________________________________
>>> libcamera-devel mailing list
>>> libcamera-devel at lists.libcamera.org
>>> https://lists.libcamera.org/listinfo/libcamera-devel
>>>
>>
>
--
Regards
--
Kieran
More information about the libcamera-devel
mailing list