[PATCH v6 03/18] libcamera: dma_heaps: extend DmaHeap class to support system heap

Milan Zamazal mzamazal at redhat.com
Wed Mar 27 16:58:30 CET 2024


Hi Laurent,

thank you for the reviews.

Laurent Pinchart <laurent.pinchart at ideasonboard.com> writes:

>> + * By default \a flags are set to DmaHeap::DmaHeapFlag::Cma. The constructor goes
>> + * through the internal list of possible names of the CMA and system dma-heap devices
>> + * until the dma-heap device of the requested type is successfully opened. If more
>> + * than one dma-heap type is specified in flags the CMA heap is tried first. If it
>> + * fails to open any dma-heap device an invalid DmaHeap object is constructed.
>> + * A valid DmaHeap object owns a wrapped dma-heap file descriptor.
>>   *
>>   * Please check the new DmaHeap object with \ref DmaHeap::isValid before using it.
>
> That's even more internal implementation details. Let me try to help.
>
>  * The DMA heap type is selected with the \a type parameter, which defaults to
>  * the CMA heap. If no heap of the given type can be accessed, the constructed
>  * DmaHeap instance is invalid as indicated by the isValid() function.
>  *
>  * Multiple types can be selected by combining type flags, in which case the
>  * constructed DmaHeap will match one of the types. If the system provides
>  * multiple heaps that match the requested types, which heap is used is
>  * undefined.

I'm not sure we want the generalization described in the last paragraph.  The
case "use whatever heap is available, but use CMA one if available" would have
to be split in two DmaHeap construction attempts (one asking for CMA, the other
one asking for anything if the first one doesn't provide a valid instance).

I suppose it may make sense to ask for a CMA heap and use a system heap if no
CMA heap is available.  While if someone really wants "use whatever heap is
available, I don't care at all which one", applying "use whatever heap is
available, but use CMA one if available" is still compatible.

I admit that using two separate calls would be slightly cleaner but it'd be also
less convenient (and I think the current code should be changed if the docstring
is changed as suggested, since I suppose the CMA preference is on purpose).

Opinions?

Regards,
Milan



More information about the libcamera-devel mailing list