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

Message ID 20221014131846.27169-1-naush@raspberrypi.com
Headers show
Series
  • Raspberry Pi: Platform configuration and buffer allocation improvements
Related show

Message

Naushir Patuck Oct. 14, 2022, 1:18 p.m. UTC 
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.

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

Comments

Naushir Patuck Oct. 14, 2022, 2:55 p.m. UTC | #1 
On Fri, 14 Oct 2022 at 14:18, Naushir Patuck <naush@raspberrypi.com> 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.

Naush


>
> 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
>
> --
> 2.25.1
>
>
Kieran Bingham Oct. 17, 2022, 9 a.m. UTC | #2 
Quoting Naushir Patuck via libcamera-devel (2022-10-14 15:55:46)
> On Fri, 14 Oct 2022 at 14:18, Naushir Patuck <naush@raspberrypi.com> 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?

--
Kieran
 
> Naush
> 
> 
> >
> > 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
> >
> > --
> > 2.25.1
> >
> >
Naushir Patuck Oct. 17, 2022, 9:10 a.m. UTC | #3 
On Mon, 17 Oct 2022 at 10:00, Kieran Bingham <
kieran.bingham@ideasonboard.com> wrote:

> Quoting Naushir Patuck via libcamera-devel (2022-10-14 15:55:46)
> > On Fri, 14 Oct 2022 at 14:18, Naushir Patuck <naush@raspberrypi.com>
> 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.




>
> --
> Kieran
>
> > Naush
> >
> >
> > >
> > > 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
> > >
> > > --
> > > 2.25.1
> > >
> > >
>
Laurent Pinchart Oct. 17, 2022, 9:30 a.m. UTC | #4 
Hi Naush,

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 ?

> > > > 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
Naushir Patuck Oct. 17, 2022, 9:38 a.m. UTC | #5 
On Mon, 17 Oct 2022 at 10:30, Laurent Pinchart <
laurent.pinchart@ideasonboard.com> wrote:

> Hi Naush,
>
> 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.

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.

Naush



>
> > > > > 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
>
Dave Stevenson Oct. 17, 2022, 10:42 a.m. UTC | #6 
On Mon, 17 Oct 2022 at 10:39, Naushir Patuck via libcamera-devel
<libcamera-devel@lists.libcamera.org> wrote:
>
>
>
> On Mon, 17 Oct 2022 at 10:30, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:
>>
>> Hi Naush,
>>
>> 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.

  Dave

[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.
>
> Naush
>
>
>>
>>
>> > > > > 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
Laurent Pinchart Oct. 19, 2022, 9:19 p.m. UTC | #7 
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