zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
9.91k stars 6.1k forks source link

drivers: video: "remote-endpoint" does nothing #72311

Open josuah opened 1 month ago

josuah commented 1 month ago

This is part of a series of proposal for incremental changes for the Video API:

Is your enhancement proposal related to a problem? Please describe. In the video drivers, the device tree declaration Linux-style remote-endpoint is used nowhere. It is up to the user to connect drivers together in a way that hopefully matches the devicetree.

Describe the solution you'd like

Describe alternatives you've considered

Additional context

My final goal is to implement the USB Video Class (UVC) to implement USB3 webcams with Zephyr.

Currently in Zephyr like in other RTOS, it is up to the user to glue video devices together. remote-endpoint: is ignored by everything: video_common.c, video.h, main.c, video_sw_generator.c

Using the devicetree information to let the driver perform I/O would allow:

Thank you for this wonderful Video4Zephyr lean and mean driver API introduced in 2019! Glad to see more interest coming into it after a few years!

[EDIT: typo and wording]

josuah commented 1 month ago

I believe this can greatly simplify examples of video applications on Zephyr, as well as an enabler for implementing an USB Video Class (UVC) where all it takes to implement it is passing a camera sensor phandle on the device tree like done for CDC ACM: https://github.com/zephyrproject-rtos/zephyr/blob/5cbf7b1b9d13a39f36ad8dabfaeaec21d98cf5ff/samples/subsys/usb/console/app.overlay#L9

Drivers could want to affect their source (i.e. image sensor) and sinks (i.e. display):

Affected open PRs:

josuah commented 1 month ago

I am willing to bring an implementation for it, but wanted to discuss it early.

The strategy sketched does not seems to break the devicetree or drivers! Only add some code to drivers to make them use their remote-endpoint: <&phandle>;

=> I will provide a real RFC at some point, you can wait for it

ajarmouni-st commented 1 month ago

@CharlesDias This might interest you.

josuah commented 1 month ago

V4L2 needs 2 kinds of interface:

Zephyr only needs one interface, merged between the two: sub-devices are the final user API:

So no implicit "driver" for a pipeline: we need to use the same mechanism as the one to interconnect drivers: DeviceTree.

No DeviceTree involved to select an endpopint on Zephyr though: https://github.com/zephyrproject-rtos/zephyr/blob/638f1d58dfcbfd924ea7c49a12c530d5d12c072e/samples/subsys/video/capture/src/main.c#L92

No "OUT" on the DeviceTree, though, only an "IN": https://github.com/zephyrproject-rtos/zephyr/blob/638f1d58dfcbfd924ea7c49a12c530d5d12c072e/boards/nxp/mimxrt1064_evk/mimxrt1064_evk.dts#L280-L282

The "OUT" is actually implicit, only defined inside the driver as a sanity check:

https://github.com/zephyrproject-rtos/zephyr/blob/638f1d58dfcbfd924ea7c49a12c530d5d12c072e/drivers/video/video_mcux_csi.c#L266-L268

And no way to say "2nd MIPI input" with the enum: https://github.com/zephyrproject-rtos/zephyr/blob/638f1d58dfcbfd924ea7c49a12c530d5d12c072e/include/zephyr/drivers/video.h#L126-L131

Zephyr already comes with an API to refer things defined in the DeviceTree.

=> Should we replace enum video_endpoint_id ep by a video_dt_spec?

josuah commented 1 month ago

One way that could be used to replace the enum video_endpoint_id is to use a struct video_dt_spec:

This permits to build a const struct video_dt_spec * that can replace all the const struct device * of the API. As well as navigate through the devicetree of video devices.

/**
 * @brief Get a node identifier of a node's remote-endpoint connection.
 *
 * This will follow the phandle pointed to by the remote-endpoint property.
 * This requires a port child with an endoint child, and the endpoint holding
 * the remote-endpoint property.
 *
 * @param node_id node identifier
 * @param port name of the port to follow, port_0 for port@0
 * @param endpoint name of the endpoint to follow, endpoint_0 for endpoint@0
 * @return node ID of the endpoint linked to the given port and endpoint
 */
#define VIDEO_DT_REMOTE_ENDPOINT(node_id, port, endpoint)                                          \
        DT_PROP(DT_CHILD(DT_CHILD(node_id, port), endpoint), remote_endpoint)

/**
 * @brief Get a device reference for the node identifier's remote endpoint
 *
 * This will follow the phandle pointed to by the remote-endpoint property.
 *
 * @param node_id node identifier of a remote-endpoint phandle property
 *
 * @see VIDEO_DT_REMOTE_ENDPOINT()
 */
#define VIDEO_DT_SPEC_GET(node_id)                                                                 \
        {                                                                                          \
                .dev =  DEVICE_DT_GET(DT_GPARENT(node_id)),                                        \
                .port = DT_PROP_OR(DT_PARENT(node_id), reg, 0),                                    \
                .endpoint = DT_PROP_OR(node_id, reg, 0)                                            \
        }

/**
 * @brief Video Device Tree endpoint identifier
 *
 * Identify a particular port and particular endpoint of a video device,
 * providing all the context necessary to act upon a video device, via one of
 * its endpoint.
 */
struct video_dt_spec {
        /** Device instance of the remote video device to communicate with. */
        const struct device *dev;
        /** Port Device Tree address, the reg property of the port. */
        uint16_t port;
        /** Endpoint Device Tree address, the reg property of the endpoint. */
        uint16_t endpoint;
};

=> A pull request will proposed when the new API is complete

josuah commented 1 month ago

If keeping remote-endpoint, then who should call enqueue()? The source? The sink? The most intuitive is that drivers push to their sinks... But then an application implementing a sink would needs to declare a video_driver_api too and become a driver so that it is connected in the pipeline...

This can be solved by sharing a FIFO between the two sides of the API (source and sink):

Example: a "funnel" driver/application that selects one active source among many, and pass it to down:

k_fifo_put(&sink_fifo, k_fifo_get(&active_source_fifo, K_FOREVER));

=> API for data movement was found, I will open a separate RFC for it

josuah commented 1 month ago

Here is an illustration of a complex pipeline, so that it is easier to review the data flow.

┌───────────┐      ┌───────────────┐      ┌───────────┐      ┌───────────┐      ┌───────┐
│ImageSensor│┌────┐│YUYV2RGB565    │┌────┐│Resizer    │┌────┐│Selector   │┌────┐│Display│
│           ││FIFO││               ││FIFO││           ││FIFO││ zero-copy ││FIFO││       │
│    new ──────>──────────*───────────>─────┐    new┌────>───────────=─┬────>──────┐    │
│    buf    │└────┘│  (in-place    │└────┘│ │    buf│ │└────┘│ ┌─────x─┘ │└────┘│  │    │
│   YUYV    │      │   conversion) │      │ │free   │ │      │ │wait     │      │  │free│
└───^^^^────┘      └───────────────┘      └─v───────^─┘      └─│─────────┘      └──v────┘
  Hardware                                  Hardware           │                 Hardware
                                                               │
                                          ┌───────────┐        │
                                          │ImageSensor│┌────┐  │
                                          │     new   ││FIFO│  │
                                          │     vbuf ────>─────┘
                                          │           │└────┘
                                          │  RGB565   │
                                          └───^^^^────┘
                                            Hardware

FIFOs are placed between each driver, so that there is no need to decide who should enqueue and who should implement an enqueue API, which would get in the way if an application (main.c) should be interleaved.

ImageSensor come in different format and size, so one of them is resized and pixel format is converted.

YUYV2RGB re-uses the same buffer

Resizer frees and allocates a new buffer because the buffer size is different between source/destination. Buffer types like net_buf handles buffer allocated in one place and freed at another one gracefully.

Selector is a pure software implementation that decides which input buffer should wait and which input buffer can be passed down unmodified.

Display can implement a video API, or eventually be a generic driver to convert between video and display API.

josuah commented 1 month ago

There is already a subsystem in Zephyr in charge of handling I/O of many devices efficiently in Zephyr: [RTIO]().

The problem it has (and that the Video driver API also has) is that it is only meant for communication between the application and a device driver, rather than moving data directly from driver to driver: Linux's splice(2).

Maybe it is possible to implement splice(2) in RTIO, using RTIO_SQE_MULTISHOT, which resubmits immediately an operation as soon as it is completed. This can be used to submit an RX completion to the next iodev as soon as a TX is ready in the previous iodev.

As soon as one device completes a TX request, the remote-endpoint's device would receive an RX request: I/O directly between video devices.

┌───────────┐     ┌────────────┐     ┌─────────┐     ┌───────┐
│ImageSensor│     │YUYV2RGB565 │     │Resizer  │     │Display│
│           │     │            │     │         │     │       │
│     ┌─────>─┐ ┌─>────────────>─┐ ┌─>───┐ ┌───>─┐ ┌─>──┐    │
│     │     │ │ │ │ Software   │ │ │ │   │ │   │ │ │ │  │    │
│   YUYV    │ │ │ │ conversion │ │ │ │   │ │   │ │ │ │  │    │
└─────^─────┘ │ │ └────────────┘ │ │ └───v─^───┘ │ │ └──v────┘
  Hardware    │ │                │ │   Hardware  │ │  Hardware
              │ │                │ │             │ │
         ┌────│──────────────────│───────────────│──────┐
         │    └>┘                └>┘             └>┘    │
         │  resubmit           resubmit        resubmit │
         │                                              │
         │ RTIO: a small, concurrent, lock-less         │
         │ Real-Time I/O scheduler                      │
         │                                              │
         └──────────────────────────────────────────────┘

=> remote-endpoint looks viable, PRs to integrate it gradually can now be proposed

josuah commented 3 weeks ago

Currently, only the Video driver uses remote-endpoint, but the discussion broadens to anything that has remote-endpoint in Linux (HDMI, MIPI, Encoders, Audio, USB connectors), if we want to interconnect more things in the devicetree.

Linux has this set of generic APIs for remote-endpoint: https://www.kernel.org/doc/html/latest/devicetree/kernel-api.html#c.of_graph_get_endpoint_by_regs https://www.kernel.org/doc/html/latest/devicetree/kernel-api.html#c.of_graph_get_remote_endpoint

Chaining the calls like this: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/sun4i/sun8i_mixer.c#n344

josuah commented 3 weeks ago

Follow-up issue and pull-request series: