libcamera: dma_buf_allocator: Make provider priority configurable

28237 diff mbox series

DmaBufAllocator selects a provider using a fixed priority (CMA heap,
system heap, then udmabuf) and picks the first one that can be opened.
On platforms with a small CMA region this is suboptimal: CMA is always
chosen when present even though it may be too small, while udmabuf,
backed by pageable system memory, would work.

Make the provider priority order configurable. DmaBufAllocator now
accepts an ordered list of preferred provider names; providers not
listed are tried afterwards in the built-in order. When the list is
empty the historical order is kept, so behaviour is unchanged by
default.

The allocator itself stays free of any configuration dependency. The
priority list is read from the 'dma_buf_allocator/provider_priority'
global configuration option by the components that allow multiple
providers (software ISP and the virtual pipeline). This lets a platform
prefer udmabuf by shipping a configuration.yaml.

The set of providers stays limited to what the caller requested, so a
component that requires physically-contiguous memory (CMA only) is not
given a udmabuf-backed buffer.

Document the new option in runtime_configuration.rst.

Signed-off-by: Wenmeng Liu <wenmeng.liu@oss.qualcomm.com>
---
 Documentation/runtime_configuration.rst        | 30 +++++++++
 include/libcamera/internal/dma_buf_allocator.h |  3 +-
 src/libcamera/dma_buf_allocator.cpp            | 92 +++++++++++++++++++++-----
 src/libcamera/pipeline/virtual/virtual.cpp     |  6 +-
 src/libcamera/software_isp/software_isp.cpp    |  6 +-
 5 files changed, 115 insertions(+), 22 deletions(-)


---
base-commit: 87c7285663aaad7608fdc18d5216ec6811c685c7
change-id: 20260911-udma-a002f890212c

Best regards,

Hi Wenmeng,

On 14-Sep-26 4:44 AM, Wenmeng Liu wrote:
> DmaBufAllocator selects a provider using a fixed priority (CMA heap,
> system heap, then udmabuf) and picks the first one that can be opened.
> On platforms with a small CMA region this is suboptimal: CMA is always
> chosen when present even though it may be too small, while udmabuf,
> backed by pageable system memory, would work.
> 
> Make the provider priority order configurable. DmaBufAllocator now
> accepts an ordered list of preferred provider names; providers not
> listed are tried afterwards in the built-in order. When the list is
> empty the historical order is kept, so behaviour is unchanged by
> default.
> 
> The allocator itself stays free of any configuration dependency. The
> priority list is read from the 'dma_buf_allocator/provider_priority'
> global configuration option by the components that allow multiple
> providers (software ISP and the virtual pipeline). This lets a platform
> prefer udmabuf by shipping a configuration.yaml.
> 
> The set of providers stays limited to what the caller requested, so a
> component that requires physically-contiguous memory (CMA only) is not
> given a udmabuf-backed buffer.
> 
> Document the new option in runtime_configuration.rst.
> 
> Signed-off-by: Wenmeng Liu <wenmeng.liu@oss.qualcomm.com>

First of all some background to this for other libcamera developers.

This started as a downstream modification to libcamera's packages in Qualcomm's
Linux distro: https://github.com/qualcomm-linux/meta-qcom-distro/pull/464

The downsstream change simply hardcoded udmabuf as the provider to try first.

And I believe that is the right thing to do in many cases. But some platforms
may require CMA to e.g. allow a hw video-encoder to access the buffers.

So hence the idea to make this configurable. Wenmeng, thank you for implementing
this.

> ---
>  Documentation/runtime_configuration.rst        | 30 +++++++++
>  include/libcamera/internal/dma_buf_allocator.h |  3 +-
>  src/libcamera/dma_buf_allocator.cpp            | 92 +++++++++++++++++++++-----
>  src/libcamera/pipeline/virtual/virtual.cpp     |  6 +-
>  src/libcamera/software_isp/software_isp.cpp    |  6 +-
>  5 files changed, 115 insertions(+), 22 deletions(-)
> 
> diff --git a/Documentation/runtime_configuration.rst b/Documentation/runtime_configuration.rst
> index a71b715a17245fc5fea405165323a607827a9ecb..0919f2f1ba3e817cbf37338f83a38aa1e3309b53 100644
> --- a/Documentation/runtime_configuration.rst
> +++ b/Documentation/runtime_configuration.rst
> @@ -33,6 +33,9 @@ file structure:
>  ::
>  
>    configuration:
> +    dma_buf_allocator:
> +      provider_priority:
> +        - ... # dma-buf provider name: cma, system, or udmabuf
>      ipa:
>        force_isolation: # true/false
>        config_paths:
> @@ -62,6 +65,11 @@ Configuration file example
>     ---
>     version: 1
>     configuration:
> +     dma_buf_allocator:
> +       provider_priority:
> +         - udmabuf
> +         - cma
> +         - system
>       ipa:
>         config_paths:
>           - /home/user/.libcamera/share/ipa
> @@ -150,6 +158,28 @@ LIBCAMERA_SOFTISP_MODE, software_isp.mode
>  
>     Example value: ``gpu``
>  
> +dma_buf_allocator.provider_priority
> +   Define an ordered list of dma-buf providers to try when libcamera
> +   allocates buffers internally (for example for the software ISP). The
> +   first requested provider that is available on the system is used. Valid
> +   provider names are ``cma`` (CMA dma-heap), ``system`` (system dma-heap)
> +   and ``udmabuf`` (memfd + /dev/udmabuf). Providers accepted by a component
> +   but not listed here are tried after the listed ones, in libcamera's
> +   built-in order.
> +
> +   This is useful on platforms with a small CMA region, where preferring
> +   ``udmabuf`` avoids exhausting the CMA heap.

Not sure if we need this last "This is useful ..." sentence here. That is more
something for the commit message then for the docs.

> +
> +   Example value:
> +
> +   ::
> +
> +      dma_buf_allocator:
> +        provider_priority:
> +          - udmabuf
> +          - cma
> +          - system
> +
>  pipelines.simple.supported_devices.driver, pipelines.simple.supported_devices.software_isp
>     Override whether software ISP is enabled for the given driver.
>  
> diff --git a/include/libcamera/internal/dma_buf_allocator.h b/include/libcamera/internal/dma_buf_allocator.h
> index 13600915a066d61c6f0dac2bfc7ec21d7fef33ef..a713675c77e174170356d5e191b5a9756a8b8e2e 100644
> --- a/include/libcamera/internal/dma_buf_allocator.h
> +++ b/include/libcamera/internal/dma_buf_allocator.h
> @@ -31,7 +31,8 @@ public:
>  
>  	using DmaBufAllocatorFlags = Flags<DmaBufAllocatorFlag>;
>  
> -	DmaBufAllocator(DmaBufAllocatorFlags flags = DmaBufAllocatorFlag::CmaHeap);
> +	DmaBufAllocator(DmaBufAllocatorFlags flags = DmaBufAllocatorFlag::CmaHeap,
> +			const std::vector<std::string> &providerPriority = {});
>  	~DmaBufAllocator();
>  	bool isValid() const { return providerHandle_.isValid(); }
>  	UniqueFD alloc(const char *name, std::size_t size);
> diff --git a/src/libcamera/dma_buf_allocator.cpp b/src/libcamera/dma_buf_allocator.cpp
> index c673d23ec761e62b746f8984dfbdd0b01a858a38..ae33fc5d95be1ad8ea67c01d87d004733aedc217 100644
> --- a/src/libcamera/dma_buf_allocator.cpp
> +++ b/src/libcamera/dma_buf_allocator.cpp
> @@ -8,13 +8,16 @@
>  
>  #include "libcamera/internal/dma_buf_allocator.h"
>  
> +#include <algorithm>
>  #include <array>
>  #include <fcntl.h>
> +#include <optional>
>  #include <sys/ioctl.h>
>  #include <sys/mman.h>
>  #include <sys/stat.h>
>  #include <sys/types.h>
>  #include <unistd.h>
> +#include <vector>
>  
>  #include <linux/dma-buf.h>
>  #include <linux/dma-heap.h>
> @@ -51,6 +54,26 @@ static constexpr std::array<DmaBufAllocatorInfo, 4> providerInfos = { {
>  	{ DmaBufAllocator::DmaBufAllocatorFlag::UDmaBuf, "/dev/udmabuf" },
>  } };
>  
> +/* Built-in provider priority, used when no other order is specified. */
> +static constexpr std::array<DmaBufAllocator::DmaBufAllocatorFlag, 3>
> +	defaultProviderPriority = { {
> +		DmaBufAllocator::DmaBufAllocatorFlag::CmaHeap,
> +		DmaBufAllocator::DmaBufAllocatorFlag::SystemHeap,
> +		DmaBufAllocator::DmaBufAllocatorFlag::UDmaBuf,
> +	} };
> +
> +static std::optional<DmaBufAllocator::DmaBufAllocatorFlag>
> +providerFromName(const std::string &name)
> +{
> +	if (name == "cma")
> +		return DmaBufAllocator::DmaBufAllocatorFlag::CmaHeap;
> +	if (name == "system")
> +		return DmaBufAllocator::DmaBufAllocatorFlag::SystemHeap;
> +	if (name == "udmabuf")
> +		return DmaBufAllocator::DmaBufAllocatorFlag::UDmaBuf;
> +	return {};
> +}
> +
>  LOG_DEFINE_CATEGORY(DmaBufAllocator)
>  
>  /**
> @@ -84,35 +107,70 @@ LOG_DEFINE_CATEGORY(DmaBufAllocator)
>  /**
>   * \brief Construct a DmaBufAllocator of a given type
>   * \param[in] type The type(s) of the dma-buf providers to allocate from
> + * \param[in] providerPriority Ordered list of preferred provider names
>   *
>   * The dma-buf provider type is selected with the \a type parameter, which
>   * defaults to the CMA heap. If no provider of the given type can be accessed,
>   * the constructed DmaBufAllocator instance is invalid as indicated by
>   * the isValid() function.
>   *
> - * Multiple types can be selected by combining type flags, in which case
> - * the constructed DmaBufAllocator will match one of the types. If multiple
> - * requested types can work on the system, which provider is used is undefined.
> + * Multiple types can be selected by combining type flags. In that case the
> + * provider to use is chosen by priority: the names listed in \a
> + * providerPriority (valid names are "cma", "system" and "udmabuf") take
> + * precedence, followed by libcamera's built-in order (CMA heap, system heap,
> + * then udmabuf). The first provider that is both requested and accessible is
> + * used, so passing an empty \a providerPriority keeps the built-in order.
>   */
> -DmaBufAllocator::DmaBufAllocator(DmaBufAllocatorFlags type)
> +DmaBufAllocator::DmaBufAllocator(DmaBufAllocatorFlags type,
> +				 const std::vector<std::string> &providerPriority)
>  {
> -	for (const auto &info : providerInfos) {	
> -		if (!(type & info.type))
> -			continue;
> +	std::vector<DmaBufAllocatorFlag> priority;
>  
> -		int ret = ::open(info.deviceNodeName, O_RDONLY | O_CLOEXEC, 0);
> -		if (ret < 0) {
> -			ret = errno;
> -			LOG(DmaBufAllocator, Debug)
> -				<< "Failed to open " << info.deviceNodeName << ": "
> -				<< strerror(ret);
> +	/*
> +	 * The caller can override the provider priority; providers not listed
> +	 * are appended in the built-in order, so no acceptable provider is ever
> +	 * dropped.
> +	 */
> +	for (const std::string &name : providerPriority) {
> +		auto flag = providerFromName(name);
> +		if (flag)
> +			priority.push_back(*flag);
> +		else
> +			LOG(DmaBufAllocator, Warning)
> +				<< "Ignoring unknown dma-buf provider \""
> +				<< name << "\"";
> +	}
> +
> +	for (DmaBufAllocatorFlag flag : defaultProviderPriority) {
> +		if (std::find(priority.begin(), priority.end(), flag) == priority.end())
> +			priority.push_back(flag);
> +	}
> +
> +	for (DmaBufAllocatorFlag flag : priority) {
> +		if (!(type & flag))
>  			continue;
> +
> +		for (const auto &info : providerInfos) {
> +			if (info.type != flag)
> +				continue;
> +
> +			int ret = ::open(info.deviceNodeName, O_RDONLY | O_CLOEXEC, 0);
> +			if (ret < 0) {
> +				ret = errno;
> +				LOG(DmaBufAllocator, Debug)
> +					<< "Failed to open " << info.deviceNodeName
> +					<< ": " << strerror(ret);
> +				continue;
> +			}
> +
> +			LOG(DmaBufAllocator, Debug) << "Using " << info.deviceNodeName;
> +			providerHandle_ = UniqueFD(ret);
> +			type_ = info.type;
> +			break;
>  		}
>  
> -		LOG(DmaBufAllocator, Debug) << "Using " << info.deviceNodeName;
> -		providerHandle_ = UniqueFD(ret);
> -		type_ = info.type;
> -		break;
> +		if (providerHandle_.isValid())
> +			break;
>  	}
>  
>  	if (!providerHandle_.isValid())
> diff --git a/src/libcamera/pipeline/virtual/virtual.cpp b/src/libcamera/pipeline/virtual/virtual.cpp
> index e25dd85a7391b61bcea50e71e6264500d1e8a5b3..815542b5520c73dd1958e06899a16169732e6c37 100644
> --- a/src/libcamera/pipeline/virtual/virtual.cpp
> +++ b/src/libcamera/pipeline/virtual/virtual.cpp
> @@ -32,6 +32,7 @@
>  #include <libcamera/property_ids.h>
>  
>  #include "libcamera/internal/camera.h"
> +#include "libcamera/internal/camera_manager.h"
>  #include "libcamera/internal/dma_buf_allocator.h"
>  #include "libcamera/internal/formats.h"
>  #include "libcamera/internal/framebuffer.h"
> @@ -239,8 +240,9 @@ bool PipelineHandlerVirtual::created_ = false;
>  PipelineHandlerVirtual::PipelineHandlerVirtual(CameraManager *manager)
>  	: PipelineHandler(manager),
>  	  dmaBufAllocator_(DmaBufAllocator::DmaBufAllocatorFlag::CmaHeap |
> -			   DmaBufAllocator::DmaBufAllocatorFlag::SystemHeap |
> -			   DmaBufAllocator::DmaBufAllocatorFlag::UDmaBuf)
> +				   DmaBufAllocator::DmaBufAllocatorFlag::SystemHeap |
> +				   DmaBufAllocator::DmaBufAllocatorFlag::UDmaBuf,
> +			   manager->_d()->configuration().listOption({ "dma_buf_allocator", "provider_priority" }).value_or(utils::defopt))
>  {
>  }
>  

Hmm, so your putting the parsing of the config file here.

I was thinking I would rather have you move this call to DmaBufAllocator::DmaBufAllocator()
and have the caller only provide defaults with the config-file overriding those.

This requires access to the configuration() object which the DmaBufAllocator code does
not have atm. You can add a `const CameraManager &cm` argument to the DmaBufAllocator to
give it access to the GlobalConfiguration object. IMHO that is better then having all
the callers do the config file parsing them selves.

Regards,

Hans



> diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp
> index f8efd24424a3981fd3eca03d2fcf27723480d08c..2cf0dff837d76169744a37e6556ae9ab6cd09830 100644
> --- a/src/libcamera/software_isp/software_isp.cpp
> +++ b/src/libcamera/software_isp/software_isp.cpp
> @@ -24,6 +24,7 @@
>  #include <libcamera/stream.h>
>  
>  #include "libcamera/internal/bayer_format.h"
> +#include "libcamera/internal/camera_manager.h"
>  #include "libcamera/internal/framebuffer.h"
>  #include "libcamera/internal/software_isp/debayer_params.h"
>  
> @@ -82,8 +83,9 @@ SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const CameraSensor *sensor,
>  			 ControlInfoMap *ipaControls)
>  	: ispWorkerThread_("SWIspWorker"),
>  	  dmaHeap_(DmaBufAllocator::DmaBufAllocatorFlag::CmaHeap |
> -		   DmaBufAllocator::DmaBufAllocatorFlag::SystemHeap |
> -		   DmaBufAllocator::DmaBufAllocatorFlag::UDmaBuf)
> +			   DmaBufAllocator::DmaBufAllocatorFlag::SystemHeap |
> +			   DmaBufAllocator::DmaBufAllocatorFlag::UDmaBuf,
> +		   pipe->cameraManager()->_d()->configuration().listOption({ "dma_buf_allocator", "provider_priority" }).value_or(utils::defopt))
>  {
>  	if (!dmaHeap_.isValid()) {
>  		LOG(SoftwareIsp, Error) << "Failed to create DmaBufAllocator object";
> 
> ---
> base-commit: 87c7285663aaad7608fdc18d5216ec6811c685c7
> change-id: 20260911-udma-a002f890212c
> 
> Best regards,

Hi Hans,

On 9/14/2026 4:28 PM, johannes.goede@oss.qualcomm.com wrote:
> Hi Wenmeng,
> 
> On 14-Sep-26 4:44 AM, Wenmeng Liu wrote:
>> DmaBufAllocator selects a provider using a fixed priority (CMA heap,
>> system heap, then udmabuf) and picks the first one that can be opened.
>> On platforms with a small CMA region this is suboptimal: CMA is always
>> chosen when present even though it may be too small, while udmabuf,
>> backed by pageable system memory, would work.
>>
>> Make the provider priority order configurable. DmaBufAllocator now
>> accepts an ordered list of preferred provider names; providers not
>> listed are tried afterwards in the built-in order. When the list is
>> empty the historical order is kept, so behaviour is unchanged by
>> default.
>>
>> The allocator itself stays free of any configuration dependency. The
>> priority list is read from the 'dma_buf_allocator/provider_priority'
>> global configuration option by the components that allow multiple
>> providers (software ISP and the virtual pipeline). This lets a platform
>> prefer udmabuf by shipping a configuration.yaml.
>>
>> The set of providers stays limited to what the caller requested, so a
>> component that requires physically-contiguous memory (CMA only) is not
>> given a udmabuf-backed buffer.
>>
>> Document the new option in runtime_configuration.rst.
>>
>> Signed-off-by: Wenmeng Liu <wenmeng.liu@oss.qualcomm.com>
> 
> First of all some background to this for other libcamera developers.
> 
> This started as a downstream modification to libcamera's packages in Qualcomm's
> Linux distro: https://github.com/qualcomm-linux/meta-qcom-distro/pull/464
> 
> The downsstream change simply hardcoded udmabuf as the provider to try first.
> 
> And I believe that is the right thing to do in many cases. But some platforms
> may require CMA to e.g. allow a hw video-encoder to access the buffers.
> 
> So hence the idea to make this configurable. Wenmeng, thank you for implementing
> this.
> 
Thank you for the review and for helping provide the background and 
context behind this issue.

>> ---
>>   Documentation/runtime_configuration.rst        | 30 +++++++++
>>   include/libcamera/internal/dma_buf_allocator.h |  3 +-
>>   src/libcamera/dma_buf_allocator.cpp            | 92 +++++++++++++++++++++-----
>>   src/libcamera/pipeline/virtual/virtual.cpp     |  6 +-
>>   src/libcamera/software_isp/software_isp.cpp    |  6 +-
>>   5 files changed, 115 insertions(+), 22 deletions(-)
>>
>> diff --git a/Documentation/runtime_configuration.rst b/Documentation/runtime_configuration.rst
>> index a71b715a17245fc5fea405165323a607827a9ecb..0919f2f1ba3e817cbf37338f83a38aa1e3309b53 100644
>> --- a/Documentation/runtime_configuration.rst
>> +++ b/Documentation/runtime_configuration.rst
>> @@ -33,6 +33,9 @@ file structure:
>>   ::
>>   
>>     configuration:
>> +    dma_buf_allocator:
>> +      provider_priority:
>> +        - ... # dma-buf provider name: cma, system, or udmabuf
>>       ipa:
>>         force_isolation: # true/false
>>         config_paths:
>> @@ -62,6 +65,11 @@ Configuration file example
>>      ---
>>      version: 1
>>      configuration:
>> +     dma_buf_allocator:
>> +       provider_priority:
>> +         - udmabuf
>> +         - cma
>> +         - system
>>        ipa:
>>          config_paths:
>>            - /home/user/.libcamera/share/ipa
>> @@ -150,6 +158,28 @@ LIBCAMERA_SOFTISP_MODE, software_isp.mode
>>   
>>      Example value: ``gpu``
>>   
>> +dma_buf_allocator.provider_priority
>> +   Define an ordered list of dma-buf providers to try when libcamera
>> +   allocates buffers internally (for example for the software ISP). The
>> +   first requested provider that is available on the system is used. Valid
>> +   provider names are ``cma`` (CMA dma-heap), ``system`` (system dma-heap)
>> +   and ``udmabuf`` (memfd + /dev/udmabuf). Providers accepted by a component
>> +   but not listed here are tried after the listed ones, in libcamera's
>> +   built-in order.
>> +
>> +   This is useful on platforms with a small CMA region, where preferring
>> +   ``udmabuf`` avoids exhausting the CMA heap.
> 
> Not sure if we need this last "This is useful ..." sentence here. That is more
> something for the commit message then for the docs.
> 
Agreed, I've dropped it from the docs. The rationale is already covered 
by the commit message.

>> +
>> +   Example value:
>> +
>> +   ::
>> +
>> +      dma_buf_allocator:
>> +        provider_priority:
>> +          - udmabuf
>> +          - cma
>> +          - system
>> +
>>   pipelines.simple.supported_devices.driver, pipelines.simple.supported_devices.software_isp
>>      Override whether software ISP is enabled for the given driver.
>>   
>> diff --git a/include/libcamera/internal/dma_buf_allocator.h b/include/libcamera/internal/dma_buf_allocator.h
>> index 13600915a066d61c6f0dac2bfc7ec21d7fef33ef..a713675c77e174170356d5e191b5a9756a8b8e2e 100644
>> --- a/include/libcamera/internal/dma_buf_allocator.h
>> +++ b/include/libcamera/internal/dma_buf_allocator.h
>> @@ -31,7 +31,8 @@ public:
>>   
>>   	using DmaBufAllocatorFlags = Flags<DmaBufAllocatorFlag>;
>>   
>> -	DmaBufAllocator(DmaBufAllocatorFlags flags = DmaBufAllocatorFlag::CmaHeap);
>> +	DmaBufAllocator(DmaBufAllocatorFlags flags = DmaBufAllocatorFlag::CmaHeap,
>> +			const std::vector<std::string> &providerPriority = {});
>>   	~DmaBufAllocator();
>>   	bool isValid() const { return providerHandle_.isValid(); }
>>   	UniqueFD alloc(const char *name, std::size_t size);
>> diff --git a/src/libcamera/dma_buf_allocator.cpp b/src/libcamera/dma_buf_allocator.cpp
>> index c673d23ec761e62b746f8984dfbdd0b01a858a38..ae33fc5d95be1ad8ea67c01d87d004733aedc217 100644
>> --- a/src/libcamera/dma_buf_allocator.cpp
>> +++ b/src/libcamera/dma_buf_allocator.cpp
>> @@ -8,13 +8,16 @@
>>   
>>   #include "libcamera/internal/dma_buf_allocator.h"
>>   
>> +#include <algorithm>
>>   #include <array>
>>   #include <fcntl.h>
>> +#include <optional>
>>   #include <sys/ioctl.h>
>>   #include <sys/mman.h>
>>   #include <sys/stat.h>
>>   #include <sys/types.h>
>>   #include <unistd.h>
>> +#include <vector>
>>   
>>   #include <linux/dma-buf.h>
>>   #include <linux/dma-heap.h>
>> @@ -51,6 +54,26 @@ static constexpr std::array<DmaBufAllocatorInfo, 4> providerInfos = { {
>>   	{ DmaBufAllocator::DmaBufAllocatorFlag::UDmaBuf, "/dev/udmabuf" },
>>   } };
>>   
>> +/* Built-in provider priority, used when no other order is specified. */
>> +static constexpr std::array<DmaBufAllocator::DmaBufAllocatorFlag, 3>
>> +	defaultProviderPriority = { {
>> +		DmaBufAllocator::DmaBufAllocatorFlag::CmaHeap,
>> +		DmaBufAllocator::DmaBufAllocatorFlag::SystemHeap,
>> +		DmaBufAllocator::DmaBufAllocatorFlag::UDmaBuf,
>> +	} };
>> +
>> +static std::optional<DmaBufAllocator::DmaBufAllocatorFlag>
>> +providerFromName(const std::string &name)
>> +{
>> +	if (name == "cma")
>> +		return DmaBufAllocator::DmaBufAllocatorFlag::CmaHeap;
>> +	if (name == "system")
>> +		return DmaBufAllocator::DmaBufAllocatorFlag::SystemHeap;
>> +	if (name == "udmabuf")
>> +		return DmaBufAllocator::DmaBufAllocatorFlag::UDmaBuf;
>> +	return {};
>> +}
>> +
>>   LOG_DEFINE_CATEGORY(DmaBufAllocator)
>>   
>>   /**
>> @@ -84,35 +107,70 @@ LOG_DEFINE_CATEGORY(DmaBufAllocator)
>>   /**
>>    * \brief Construct a DmaBufAllocator of a given type
>>    * \param[in] type The type(s) of the dma-buf providers to allocate from
>> + * \param[in] providerPriority Ordered list of preferred provider names
>>    *
>>    * The dma-buf provider type is selected with the \a type parameter, which
>>    * defaults to the CMA heap. If no provider of the given type can be accessed,
>>    * the constructed DmaBufAllocator instance is invalid as indicated by
>>    * the isValid() function.
>>    *
>> - * Multiple types can be selected by combining type flags, in which case
>> - * the constructed DmaBufAllocator will match one of the types. If multiple
>> - * requested types can work on the system, which provider is used is undefined.
>> + * Multiple types can be selected by combining type flags. In that case the
>> + * provider to use is chosen by priority: the names listed in \a
>> + * providerPriority (valid names are "cma", "system" and "udmabuf") take
>> + * precedence, followed by libcamera's built-in order (CMA heap, system heap,
>> + * then udmabuf). The first provider that is both requested and accessible is
>> + * used, so passing an empty \a providerPriority keeps the built-in order.
>>    */
>> -DmaBufAllocator::DmaBufAllocator(DmaBufAllocatorFlags type)
>> +DmaBufAllocator::DmaBufAllocator(DmaBufAllocatorFlags type,
>> +				 const std::vector<std::string> &providerPriority)
>>   {
>> -	for (const auto &info : providerInfos) {	
>> -		if (!(type & info.type))
>> -			continue;
>> +	std::vector<DmaBufAllocatorFlag> priority;
>>   
>> -		int ret = ::open(info.deviceNodeName, O_RDONLY | O_CLOEXEC, 0);
>> -		if (ret < 0) {
>> -			ret = errno;
>> -			LOG(DmaBufAllocator, Debug)
>> -				<< "Failed to open " << info.deviceNodeName << ": "
>> -				<< strerror(ret);
>> +	/*
>> +	 * The caller can override the provider priority; providers not listed
>> +	 * are appended in the built-in order, so no acceptable provider is ever
>> +	 * dropped.
>> +	 */
>> +	for (const std::string &name : providerPriority) {
>> +		auto flag = providerFromName(name);
>> +		if (flag)
>> +			priority.push_back(*flag);
>> +		else
>> +			LOG(DmaBufAllocator, Warning)
>> +				<< "Ignoring unknown dma-buf provider \""
>> +				<< name << "\"";
>> +	}
>> +
>> +	for (DmaBufAllocatorFlag flag : defaultProviderPriority) {
>> +		if (std::find(priority.begin(), priority.end(), flag) == priority.end())
>> +			priority.push_back(flag);
>> +	}
>> +
>> +	for (DmaBufAllocatorFlag flag : priority) {
>> +		if (!(type & flag))
>>   			continue;
>> +
>> +		for (const auto &info : providerInfos) {
>> +			if (info.type != flag)
>> +				continue;
>> +
>> +			int ret = ::open(info.deviceNodeName, O_RDONLY | O_CLOEXEC, 0);
>> +			if (ret < 0) {
>> +				ret = errno;
>> +				LOG(DmaBufAllocator, Debug)
>> +					<< "Failed to open " << info.deviceNodeName
>> +					<< ": " << strerror(ret);
>> +				continue;
>> +			}
>> +
>> +			LOG(DmaBufAllocator, Debug) << "Using " << info.deviceNodeName;
>> +			providerHandle_ = UniqueFD(ret);
>> +			type_ = info.type;
>> +			break;
>>   		}
>>   
>> -		LOG(DmaBufAllocator, Debug) << "Using " << info.deviceNodeName;
>> -		providerHandle_ = UniqueFD(ret);
>> -		type_ = info.type;
>> -		break;
>> +		if (providerHandle_.isValid())
>> +			break;
>>   	}
>>   
>>   	if (!providerHandle_.isValid())
>> diff --git a/src/libcamera/pipeline/virtual/virtual.cpp b/src/libcamera/pipeline/virtual/virtual.cpp
>> index e25dd85a7391b61bcea50e71e6264500d1e8a5b3..815542b5520c73dd1958e06899a16169732e6c37 100644
>> --- a/src/libcamera/pipeline/virtual/virtual.cpp
>> +++ b/src/libcamera/pipeline/virtual/virtual.cpp
>> @@ -32,6 +32,7 @@
>>   #include <libcamera/property_ids.h>
>>   
>>   #include "libcamera/internal/camera.h"
>> +#include "libcamera/internal/camera_manager.h"
>>   #include "libcamera/internal/dma_buf_allocator.h"
>>   #include "libcamera/internal/formats.h"
>>   #include "libcamera/internal/framebuffer.h"
>> @@ -239,8 +240,9 @@ bool PipelineHandlerVirtual::created_ = false;
>>   PipelineHandlerVirtual::PipelineHandlerVirtual(CameraManager *manager)
>>   	: PipelineHandler(manager),
>>   	  dmaBufAllocator_(DmaBufAllocator::DmaBufAllocatorFlag::CmaHeap |
>> -			   DmaBufAllocator::DmaBufAllocatorFlag::SystemHeap |
>> -			   DmaBufAllocator::DmaBufAllocatorFlag::UDmaBuf)
>> +				   DmaBufAllocator::DmaBufAllocatorFlag::SystemHeap |
>> +				   DmaBufAllocator::DmaBufAllocatorFlag::UDmaBuf,
>> +			   manager->_d()->configuration().listOption({ "dma_buf_allocator", "provider_priority" }).value_or(utils::defopt))
>>   {
>>   }
>>   
> 
> Hmm, so your putting the parsing of the config file here.
> 
> I was thinking I would rather have you move this call to DmaBufAllocator::DmaBufAllocator()
> and have the caller only provide defaults with the config-file overriding those.
> 
> This requires access to the configuration() object which the DmaBufAllocator code does
> not have atm. You can add a `const CameraManager &cm` argument to the DmaBufAllocator to
> give it access to the GlobalConfiguration object. IMHO that is better then having all
> the callers do the config file parsing them selves.
> 

Makes sense, thanks. For v2 I've moved the config parsing into
DmaBufAllocator's constructor.


Thanks,
Wenmeng

> Regards,
> 
> Hans
> 
> 
> 
>> diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp
>> index f8efd24424a3981fd3eca03d2fcf27723480d08c..2cf0dff837d76169744a37e6556ae9ab6cd09830 100644
>> --- a/src/libcamera/software_isp/software_isp.cpp
>> +++ b/src/libcamera/software_isp/software_isp.cpp
>> @@ -24,6 +24,7 @@
>>   #include <libcamera/stream.h>
>>   
>>   #include "libcamera/internal/bayer_format.h"
>> +#include "libcamera/internal/camera_manager.h"
>>   #include "libcamera/internal/framebuffer.h"
>>   #include "libcamera/internal/software_isp/debayer_params.h"
>>   
>> @@ -82,8 +83,9 @@ SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const CameraSensor *sensor,
>>   			 ControlInfoMap *ipaControls)
>>   	: ispWorkerThread_("SWIspWorker"),
>>   	  dmaHeap_(DmaBufAllocator::DmaBufAllocatorFlag::CmaHeap |
>> -		   DmaBufAllocator::DmaBufAllocatorFlag::SystemHeap |
>> -		   DmaBufAllocator::DmaBufAllocatorFlag::UDmaBuf)
>> +			   DmaBufAllocator::DmaBufAllocatorFlag::SystemHeap |
>> +			   DmaBufAllocator::DmaBufAllocatorFlag::UDmaBuf,
>> +		   pipe->cameraManager()->_d()->configuration().listOption({ "dma_buf_allocator", "provider_priority" }).value_or(utils::defopt))
>>   {
>>   	if (!dmaHeap_.isValid()) {
>>   		LOG(SoftwareIsp, Error) << "Failed to create DmaBufAllocator object";
>>
>> ---
>> base-commit: 87c7285663aaad7608fdc18d5216ec6811c685c7
>> change-id: 20260911-udma-a002f890212c
>>
>> Best regards,
>