[libcamera-devel] [PATCH v1 00/10] Raspberry Pi: Platform configuration and buffer allocation improvements

[Laurent Pinchart]

Hi Dave,

On Mon, Oct 17, 2022 at 11:42:39AM +0100, Dave Stevenson wrote:
> On Mon, 17 Oct 2022 at 10:39, Naushir Patuck wrote:
> > On Mon, 17 Oct 2022 at 10:30, Laurent Pinchart wrote:
> >> On Mon, Oct 17, 2022 at 10:10:43AM +0100, Naushir Patuck via libcamera-devel wrote:
> >> > On Mon, 17 Oct 2022 at 10:00, Kieran Bingham wrote:
> >> > > Quoting Naushir Patuck via libcamera-devel (2022-10-14 15:55:46)
> >> > > > On Fri, 14 Oct 2022 at 14:18, Naushir Patuck wrote:
> >> > > > > Hi,
> >> > > > >
> >> > > > > One big issue that is plaguing libcamera users on Raspberry Pi is memory usage.
> >> > > > > All our frame buffers have to be allocated in CMA space because of hardware
> >> > > > > restrictions.  Unfortunately CMA space is very limited (particularly on the
> >> > > > > Raspberry Pi Zero), and is prone to frequent fragmentation as display/rendering
> >> > > > > buffers also use this pool. Not to mention that the kernel is also allowed to
> >> > > > > allocate in the CMA pool for its own needs. As a result, some of our libcamera
> >> > > > > applications intermittently fail to run because of buffer allocation failure,
> >> > > > > and some use-cases (e.g. > 20MPix captures) will simply not be able to run on
> >> > > > > some platforms at all.
> >> > > > >
> >> > > > > This patch series attempts to try and fix this by easing the memory usage of
> >> > > > > the Raspberry Pi pipeline handler. It does this by using a platform configuration
> >> > > > > file which (amongst other things) instructs the pipeline handler on how many
> >> > > > > internal buffers it should allocate. There may be a performance penality (e.g
> >> > > > > dropped frames, or longer time taken for initial AE/AWB convergence) that have
> >> > > > > been documented.  But if the application can guarantee certain behavior, we can
> >> > > > > ensure the pipeline can still process requests with the same performance as we
> >> > > > > have today. The Raspberry Pi libcamera-apps and picamera2 framework already use
> >> > > > > this behavior, so the change should be transparent to applications built on
> >> > > > > these, apart from using significantly less memory.
> >> > > > >
> >> > > > > Patches 1 to 4 and the framework for using a configuration file to setup the
> >> > > > > buffer allocation parameters
> >> > > > >
> >> > > > > Patch 5 to 7 improve the buffer handling dealing with dropped frames, allowing
> >> > > > > us to use fewer internal buffers.
> >> > > >
> >> > > > I should add that this particular change requires a firmware update to work correctly.
> >> > > > If you want to test this out, it's best to run "rpi-update" to get the
> >> > > > latest firmware.
> >> > >
> >> > > Mostly from curiosity - is there a 'standard' way for the Raspberry Pi
> >> > > pipeline handler to interogate or query the rpi firmware to do a check
> >> > > in case it is not the correct version?
> >> >
> >> > Unfortunately not in an easy way.
> >> > We could run a shell command "vcgencmd version" to get a sha
> >> > hash, but I would find that really ugly to do in the pipeline hanalder.
> >>
> >> Not great indeed.
> >>
> >> Is this something we can improve going forward ? I'm thinking of a
> >> message to query the vc4 firmware version at runtime, that the ISP
> >> driver would use to retrieve the version. We could then expose it to
> >> userspace through the ISP video nodes.
> >>
> >> Is there a "traditional" version number for the firmware, or just a git
> >> sha1 ?
> >
> >
> > We don't have a traditional version number available, only the sha1 hash.
> > I doubt this is something that folks over here will have an appetite to change.
> 
> We do have both the hash and build date available through the firmware
> interface. The firmware driver logs it on boot
> [    0.070048] raspberrypi-firmware soc:firmware: Attached to firmware
> from 2022-08-26T14:03:16, variant start
> Code at [1].
> 
> The ISP driver could read that and add it to either a custom control,
> or via part of the device description.

No urgency, but I think that wouldbe nice to have. As you're fully in
control of the firmware, I assume that a more recent release date will
always mean a more recent version, and that could be correlated with a
feature set ?

> [1] https://github.com/raspberrypi/linux/blob/rpi-5.15.y/drivers/firmware/raspberrypi.c#L253
> 
> > However, things are not really that bad for the regular user.  Our apt dependency
> > setup will ensure that if people get the latest libcamera which requires a firmware
> > change, the firmware blob will also be updated.  This only really hits folks who
> > do manual builds of libcamera.
> >
> >> > > > > Patch 8 adds a parameter to modify drop frame behavior - not related to this
> >> > > > > problem, but a nice to have.
> >> > > > >
> >> > > > > Patches 9 and 10 add configuration files for minimal memory configuration, and
> >> > > > > another for optimal memory usage for libcamera-apps and picamera2.
> >> > > > >
> >> > > > > I've considered how these various performance tuning parameters could be made
> >> > > > > part of the core StreamConfiguration structure or even as a Control. However,
> >> > > > > they are entirely platform specific, and I am struggling to come up with a clean
> >> > > > > way to put this into libcamera core with polluting it with various platform specific
> >> > > > > parameters.  If others have any ideas or suggestions on using alternate means
> >> > > > > of passing platform specific parameters into the pipeline handler, please do
> >> > > > > comment on this review.
> >> > > > >
> >> > > > > Thanks,
> >> > > > > Naush
> >> > > > >
> >> > > > > Naushir Patuck (10):
> >> > > > >   libcamera: pipeline: Add a platform configuration file helper
> >> > > > >   pipeline: raspberrypi: Add a pipeline config structure
> >> > > > >   pipeline: raspberrypi: Split out ISP Output0 buffer allocation
> >> > > > >   pipeline: raspberrypi: Read config parameters from a file
> >> > > > >   pipeline: raspberrypi: Disable StreamOn for ISP Output0/1 when
> >> > > > >     dropping frames
> >> > > > >   pipeline: raspberrypi: Reorder startup drop frame initialisation
> >> > > > >   pipeline: raspberrypi: Add a parameter to disable startup drop frames
> >> > > > >   pipeline: raspberrypi: Allow pipeline handler to always use the newest
> >> > > > >     frame
> >> > > > >   pipeline: raspberrypi: Add minimal memory usage config file
> >> > > > >   pipeline: raspberrypi: Add libcamera-apps and picamera2 config file
> >> > > > >
> >> > > > >  include/libcamera/internal/pipeline_handler.h |   2 +
> >> > > > >  .../pipeline/raspberrypi/data/default.json    |  28 +++
> >> > > > >  .../pipeline/raspberrypi/data/meson.build     |  10 +
> >> > > > >  .../raspberrypi/data/minimal_mem.json         |  28 +++
> >> > > > >  .../pipeline/raspberrypi/data/rpi_apps.json   |  28 +++
> >> > > > >  .../pipeline/raspberrypi/meson.build          |   2 +
> >> > > > >  .../pipeline/raspberrypi/raspberrypi.cpp      | 215 +++++++++++++++---
> >> > > > >  src/libcamera/pipeline_handler.cpp            |  57 +++++
> >> > > > >  8 files changed, 339 insertions(+), 31 deletions(-)
> >> > > > >  create mode 100644 src/libcamera/pipeline/raspberrypi/data/default.json
> >> > > > >  create mode 100644 src/libcamera/pipeline/raspberrypi/data/meson.build
> >> > > > >  create mode 100644 src/libcamera/pipeline/raspberrypi/data/minimal_mem.json
> >> > > > >  create mode 100644 src/libcamera/pipeline/raspberrypi/data/rpi_apps.json

-- 
Regards,

Laurent Pinchart