[libcamera-devel] [PATCH v6 08/11] ipa: raspberrypi: Add mojom data definition file
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Thu Feb 4 02:44:19 CET 2021
Hi Paul,
Thank you for the patch.
On Thu, Dec 24, 2020 at 05:16:10PM +0900, Paul Elder wrote:
> Add a mojom data definition for raspberrypi pipeline handler's IPAs.
> This is a direct translation of what the raspberrypi pipeline handler
> and IPA was using before with IPAOperationData.
>
> Also move the enums from raspberrypi.h to raspberrypi.mojom
>
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
>
> ---
> Changes in v6:
> - rebase on "pipeline: ipa: raspberrypi: Handle failures during IPA
> configuration"
> - move enums and consts to the mojom file
> - use the custom start()
> - remove unnecessary ConfigParameters, and remove lsTableHandleStatic
> from ConfigInput
>
> Changes in v5:
> - rename some structs
>
> Changes in v4:
> - rename IPARPiCallbackInterface to IPARPiEventInterface
>
> Changes in v3:
> - remove stray comment about how our compiler will generate code
> - add ipa.rpi namespace
> - not ipa.RPi, because namespace conflict
> - remove RPi prefix from struct and enum names
> - add transform parameter to struct ConfigInput
>
> Changes in v2:
> - rebased on "libcamera: pipeline: ipa: raspberrypi: Rework drop frame
> signalling"
> - add license
> - move generic documentation to core.mojom
> - move documentation from IPA interface
> - customize the RPi IPA interface to make the pipeline and IPA code
> cleaner
> ---
> include/libcamera/ipa/meson.build | 4 +-
> include/libcamera/ipa/raspberrypi.h | 20 ----
> include/libcamera/ipa/raspberrypi.mojom | 134 ++++++++++++++++++++++++
> 3 files changed, 137 insertions(+), 21 deletions(-)
> create mode 100644 include/libcamera/ipa/raspberrypi.mojom
>
> diff --git a/include/libcamera/ipa/meson.build b/include/libcamera/ipa/meson.build
> index 499d1bc0..785c1241 100644
> --- a/include/libcamera/ipa/meson.build
> +++ b/include/libcamera/ipa/meson.build
> @@ -54,7 +54,9 @@ libcamera_generated_ipa_headers += custom_target('core_ipa_serializer_h',
> './' +'@INPUT@'
> ])
>
> -ipa_mojom_files = []
> +ipa_mojom_files = [
> + 'raspberrypi.mojom',
> +]
>
> ipa_mojoms = []
>
> diff --git a/include/libcamera/ipa/raspberrypi.h b/include/libcamera/ipa/raspberrypi.h
> index 01fe5abc..df30b4a2 100644
> --- a/include/libcamera/ipa/raspberrypi.h
> +++ b/include/libcamera/ipa/raspberrypi.h
> @@ -16,26 +16,6 @@ namespace libcamera {
>
> namespace RPi {
>
> -enum ConfigParameters {
> - IPA_CONFIG_LS_TABLE = (1 << 0),
> - IPA_CONFIG_STAGGERED_WRITE = (1 << 1),
> - IPA_CONFIG_SENSOR = (1 << 2),
> - IPA_CONFIG_DROP_FRAMES = (1 << 3),
> - IPA_CONFIG_FAILED = (1 << 4),
> - IPA_CONFIG_STARTUP = (1 << 5),
> -};
> -
> -enum Operations {
> - IPA_ACTION_V4L2_SET_STAGGERED = 1,
> - IPA_ACTION_V4L2_SET_ISP,
> - IPA_ACTION_STATS_METADATA_COMPLETE,
> - IPA_ACTION_RUN_ISP,
> - IPA_ACTION_EMBEDDED_COMPLETE,
> - IPA_EVENT_SIGNAL_STAT_READY,
> - IPA_EVENT_SIGNAL_ISP_PREPARE,
> - IPA_EVENT_QUEUE_REQUEST,
> -};
> -
> enum BufferMask {
> ID = 0x00ffff,
> STATS = 0x010000,
> diff --git a/include/libcamera/ipa/raspberrypi.mojom b/include/libcamera/ipa/raspberrypi.mojom
> new file mode 100644
> index 00000000..53521ce9
> --- /dev/null
> +++ b/include/libcamera/ipa/raspberrypi.mojom
> @@ -0,0 +1,134 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +
> +module ipa.rpi;
> +
> +import "include/libcamera/ipa/core.mojom";
> +
> +enum BufferMask {
> + MaskID = 0x00ffff,
> + MaskStats = 0x010000,
> + MaskEmbeddedData = 0x020000,
> + MaskBayerData = 0x040000,
> + MaskExternalBuffer = 0x100000,
> +};
> +
> +/* Size of the LS grid allocation. */
> +const uint32 MaxLsGridSize = 0x8000;
It's annoying that the mojom parse doesn't allow us to write 32 * 1024
:-( Should we file a bug to Google to get this fixed eventually ?
> +
> +enum ConfigParameters {
> + ConfigLsTable = 0x01,
> + ConfigStaggeredWrite = 0x02,
> + ConfigSensor = 0x04,
> + ConfigFailed = 0x08,
> +};
Now that we have a more dynamic interface, we could rework this. The
ConfigLsTable flag isn't needed, as we can instead check if the
lsTableHandle is valid or not.
ConfigFailed would be best returned through a dedicated status field in
ConfigOutput that would return 0 on success or a negative error code on
error, through a boolean field named "success" or "error", or through a
ret return value for the configure() function, like done with start().
We can keep ConfigStaggeredWrite and ConfigSensor for now (until we get
support for nullable values), but I would already rename
ConfigParameters to ConfigOutputParameters to emphasize the fact it's
used for ConfigOutput only. The ConfigOutput::op field should be renamed
to params, and the ConfigInput::op field should be dropped.
You can do this as part as this patch already (but it then wouldn't be a
direct translation anymore, as stated in the commit message) or as a
patch on top (which can be moved to a Part 4 to avoid blocking the merge
of parts 1 to 3), up to you.
> +
> +struct SensorConfig {
> + uint32 gainDelay;
> + uint32 exposureDelay;
> + uint32 sensorMetadata;
> +};
> +
> +struct ISPConfig {
> + uint32 embeddedbufferId;
> + uint32 bayerbufferId;
> +};
> +
> +struct ConfigInput {
> + uint32 op;
> + uint32 transform;
> + FileDescriptor lsTableHandle;
> +};
> +
> +struct ConfigOutput {
> + uint32 op;
> + SensorConfig sensorConfig;
> + ControlList controls;
> +};
> +
> +struct StartControls {
> + bool hasControls;
> + ControlList controls;
> + int32 dropFrameCount;
> +};
> +
> +interface IPARPiInterface {
> + init(IPASettings settings) => (int32 ret);
> + start(StartControls controls) => (StartControls result, int32 ret);
> + stop();
> +
> + /**
> + * \fn configure()
> + * \brief Configure the IPA stream and sensor settings
> + * \param[in] sensorInfo Camera sensor information
> + * \param[in] streamConfig Configuration of all active streams
> + * \param[in] entityControls Controls provided by the pipeline entities
> + * \param[in] ipaConfig Pipeline-handler-specific configuration data
> + * \param[out] result Pipeline-handler-specific configuration result
The return value is named "results".
> + *
> + * This method shall be called when the camera is started to inform the IPA of
"started" or "configured" ?
> + * the camera's streams and the sensor settings.
> + *
> + * The \a sensorInfo conveys information about the camera sensor settings that
> + * the pipeline handler has selected for the configuration.
> + *
> + * The \a ipaConfig and \a result parameters carry data passed by the
s/result/results/
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> + * pipeline handler to the IPA and back.
> + */
> + configure(CameraSensorInfo sensorInfo,
> + map<uint32, IPAStream> streamConfig,
> + map<uint32, ControlInfoMap> entityControls,
> + ConfigInput ipaConfig)
> + => (ConfigOutput results);
> +
> + /**
> + * \fn mapBuffers()
> + * \brief Map buffers shared between the pipeline handler and the IPA
> + * \param[in] buffers List of buffers to map
> + *
> + * This method informs the IPA module of memory buffers set up by the pipeline
> + * handler that the IPA needs to access. It provides dmabuf file handles for
> + * each buffer, and associates the buffers with unique numerical IDs.
> + *
> + * IPAs shall map the dmabuf file handles to their address space and keep a
> + * cache of the mappings, indexed by the buffer numerical IDs. The IDs are used
> + * in all other IPA interface methods to refer to buffers, including the
> + * unmapBuffers() method.
> + *
> + * All buffers that the pipeline handler wishes to share with an IPA shall be
> + * mapped with this method. Buffers may be mapped all at once with a single
> + * call, or mapped and unmapped dynamically at runtime, depending on the IPA
> + * protocol. Regardless of the protocol, all buffers mapped at a given time
> + * shall have unique numerical IDs.
> + *
> + * The numerical IDs have no meaning defined by the IPA interface, and
> + * should be treated as opaque handles by IPAs, with the only exception
> + * that ID zero is invalid.
> + *
> + * \sa unmapBuffers()
> + */
> + mapBuffers(array<IPABuffer> buffers);
> +
> + /**
> + * \fn unmapBuffers()
> + * \brief Unmap buffers shared by the pipeline to the IPA
> + * \param[in] ids List of buffer IDs to unmap
> + *
> + * This method removes mappings set up with mapBuffers(). Numerical IDs
> + * of unmapped buffers may be reused when mapping new buffers.
> + *
> + * \sa mapBuffers()
> + */
> + unmapBuffers(array<uint32> ids);
> +
> + [async] signalStatReady(uint32 bufferId);
> + [async] signalQueueRequest(ControlList controls);
> + [async] signalIspPrepare(ISPConfig data);
> +};
> +
> +interface IPARPiEventInterface {
> + statsMetadataComplete(uint32 bufferId, ControlList controls);
> + runIsp(uint32 bufferId);
> + embeddedComplete(uint32 bufferId);
> + setIsp(ControlList controls);
> + setStaggered(ControlList controls);
> +};
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list