drivers: video: "remote-endpoint" does nothing

[josuah]

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

• drivers: video: API enhancement #72959

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

• Use the <&phandle> to make the drivers communicate with each other. For instance how it is done in https://github.com/zephyrproject-rtos/zephyr/blob/main/tests/subsys/usb/uac2/app.overlay#L53 to allow connecting a different video source by changing the device tree.

Describe alternatives you've considered

• Removing the remote-endpoint everywhere from Zephyr and give-up on using devicetree for video
• Add a subsystem to handle this glue for compatibility reasons

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:

• changing the image sensor resolution when switching a display resolution
• stopping/starting the display transparently stopping/starting the camera source
• implement the diversity of devices present on the Linux kernel

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]

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:

zephyr/samples/subsys/usb/console/app.overlay

Line 9 in 5cbf7b1

zephyr,console = &cdc_acm_uart0;

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

• Controls triggered from the sink to the source => "give me this format"
• Data enqueued by the sink => "fill me this buffer with pixels"

Affected open PRs:

• Support circular DTS node references #57708
• samples: drivers: video: add capture to lvgl sample #71463
• drivers: video: Add support for arducam mega #66994

[josuah]

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]

@CharlesDias This might interest you.

[josuah]

V4L2 needs 2 kinds of interface:

• Interface with userspace: https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/video.html
• Interface between drivers: https://www.kernel.org/doc/html/latest/driver-api/media/v4l2-subdev.html

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

• Interface between anything: https://static.linaro.org/connect/san19/presentations/san19-503.pdf#page=12

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:

zephyr/samples/subsys/video/capture/src/main.c

Line 92 in 638f1d5

video_enqueue(video, VIDEO_EP_OUT, buffers[i]);

No "OUT" on the DeviceTree, though, only an "IN":

zephyr/boards/nxp/mimxrt1064_evk/mimxrt1064_evk.dts

Lines 280 to 282 in 638f1d5

	csi_ep_in: endpoint {
	remote-endpoint = <&mt9m114_ep_out>;
	};

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

zephyr/drivers/video/video_mcux_csi.c

Lines 266 to 268 in 638f1d5

	if (ep != VIDEO_EP_OUT) {
	return -EINVAL;
	}

And no way to say "2nd MIPI input" with the enum:

zephyr/include/zephyr/drivers/video.h

Lines 126 to 131 in 638f1d5

	enum video_endpoint_id {
	VIDEO_EP_NONE,
	VIDEO_EP_ANY,
	VIDEO_EP_IN,
	VIDEO_EP_OUT,
	};

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]

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]

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):

• Sources push to the FIFO on interrupt
• Sinks pull from the FIFO on interrupt

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]

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 (via MIPI DMA peripheral) 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]

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]

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]

Follow-up issue and pull-request series:

• drivers: video: API enhancement #72959

[josuah]

As seen on #76798, it is possible to implement an UVC class without introducing remote-endpoint, so I am closing the issue.
The above is still interesting as an article about possible evolution.