[libcamera-devel] [PATCH 00/20] libcamera: ipu3: Rework pipe configuration

[Laurent Pinchart]

Hi Jacopo,

On Tue, Jul 14, 2020 at 12:41:52PM +0200, Jacopo Mondi wrote:
> Hello,
>   quite a big update compared to v2, even if most changes happens in 3 patches
> 
> The new stream size computations which happens at generateConfiguration()
> and validate() time now enforces a new rule
> -  All YUV streams should be aligned to the 64 pixels in widht and 32 pixels
>    in height multiple which is strictly smaller than the CIO2 output frame.
> 
> This is reflected in how configurations are generated, how the CIO2 size is
> computed when no raw stream is requested, and how the YUV streams are adjusted
> to respect this margin constraint and the device required alignment.
> 
> Most of this changes happens in patch [5/20] and [14/20]
> 
> I also have simplified a bit the ImgU pipe parameters calculation. There is
> a huge space for improvements there probably, but I'm very cautious in touching
> this. The changelog is in the patch for the records.

I understand your concern here, so I won't push too hard, and we can
improve the code further on top.

> Also, compared to v2, the ImgU pipe gets configured only if there's at least
> one YUV stream requested, otherwise setting sizes for the IF and BDS rectangle
> without actually capturing frames from any of the ImgU output, stalls the
> ImgU which require system to be rebooted to work correctly again.

That's a lovely one :-) Could you capture this in a comment in the code
? Should we avoid starting the ImgU at all in that case ? To be very
clear, it's not something that needs to be fixed in this series.

> Validated by using all the possible stream roles combinations, and inspecting
> images which despite the horrible quality (as there's no 3A working) are
> actually there.
> 
> I'm sure that poking the system with more size combinations will highlight more
> issues, but I still think this is an acceptable start.

Absolutely ! Thank you for your great work here.

> v2 -> v3:
> - Rework stream size logic to respect alignements and margins
> - Add utils::alignUp/utils::alignDown
> - Rework stream adjustments
> - Simplify a bit the ImgU pipe parameters calculation
> - Do not configure the ImgU if only RAW stream is requested
> 
> Jacopo Mondi (19):
>   libcamera: ipu3: Rename mbusCodesToInfo
>   libcamera: utils: Add alignUp and alignDown functions
>   libcamera: geometry: Add isNull() function to Rectangle class
>   libcamera: ipu3: Remove streams from generateConfiguration
>   libcamera: ipu3: Make sure the config is valid
>   libcamera: ipu3: cio2: Report format and sizes
>   libcamera: ipu3: Do not overwrite StreamConfiguration
>   libcamera: ipu3: Report StreamFormats
>   libcamera: ipu3: Remove initialization of Size
>   libcamera: ipu3: Validate the stream combination
>   libcamera: camera: Zero streams before validate()
>   libcamera: ipu3: cio2: Mark sensor() as const
>   libcamera: ipu3: Adjust and assign streams in validate()
>   libcamera: ipu3: Remove streams from IPU3CameraConfiguration
>   libcamera: ipu3: Remove camera_ from IPU3CameraConfiguration
>   libcamera: ipu3: imgu: Calculate ImgU pipe configuration
>   libcamera: ipu3: Validate the pipe configuration
>   libcamera: ipu3: Configure ImgU with the computed parameters
>   libcamera: ipu3: imgu: Rename configureInput()
> 
> Laurent Pinchart (1):
>   libcamera: geometry: Add helper functions to the Size class
> 
>  include/libcamera/geometry.h         |  34 +++
>  include/libcamera/internal/utils.h   |   3 +
>  src/libcamera/camera.cpp             |   4 +-
>  src/libcamera/geometry.cpp           |  42 +++
>  src/libcamera/pipeline/ipu3/cio2.cpp |  54 +++-
>  src/libcamera/pipeline/ipu3/cio2.h   |   7 +-
>  src/libcamera/pipeline/ipu3/imgu.    |   0
>  src/libcamera/pipeline/ipu3/imgu.cpp | 387 +++++++++++++++++++++++--
>  src/libcamera/pipeline/ipu3/imgu.h   |  22 +-
>  src/libcamera/pipeline/ipu3/ipu3.cpp | 414 +++++++++++++--------------
>  src/libcamera/utils.cpp              |  22 ++
>  test/geometry.cpp                    |  35 +++
>  test/utils.cpp                       |  18 ++
>  13 files changed, 799 insertions(+), 243 deletions(-)
>  create mode 100644 src/libcamera/pipeline/ipu3/imgu.

-- 
Regards,

Laurent Pinchart