Implement video control framework

[ngphibang]

The control APIs seems simple enough, but quickly becomes very hard to implement correctly in drivers. Also, much of the code needed to handle controls is actually not driver specific and can be moved to the video core framework.

This PR implements the video control framework which not only aims to make life as easy as possible for the driver developers but also to enable handling of different types of controls which actually, cannot be handled by the current APIs.

Note that the implementation relies on the presence of a struct video_device which will be also served as a basis for futher improvements of the video framework in the future such as pipeline management, buffer management, m2m support, etc.

All the implementation happens inside the framework and for drivers-only usage, no public header added.

Currently, the set/get_ctrl() APIs in the video framework are defined as below:

static inline int video_get/set_ctrl(const struct device *dev, unsigned int cid, void *value)
{
	const struct video_driver_api *api = (const struct video_driver_api *)dev->api;

	if (api->get/set_ctrl == NULL) {
		return -ENOSYS;
	}

	return api->get/set_ctrl(dev, cid, value);
}

It is also the common pattern for other video APIs (e.g. get/set_format) as well as other subsystems (e.g. display), i.e. the subsystems just define the APIs but do nothing, all the work are handled by the drivers themselves.

Apart from this, a problem with this API is that, it makes all drivers in a pipeline propagate the get_ctrl() from root to sensor. At the sensor, the control value is obtained by reading directly the registers. While this works for some "volatile" controls like gain, exposure, in most of the cases, reading control value directly from registers are not only inefficient but also sometimes impossible, e.g. controls that need "complex" computation and "scatter" through several registers, e.g. hue control in ov5640 driver. That's why controls need to be cached in memory.

Tested on

• ov5640 -> mipicsi2rx -> csi (i.MX RT1170)
• mt9m114 -> csi (i.MX RT1064)
• video sw generator

Features list

• Add get_volatile_ctrl() API for controls that needs to be read freshly from hardware e.g. gain, exposure (in some contexts).
• Add control cluster/auto-cluster (group of dependent controls represented by a master control)
• Add query_ctrl() API to query min, max, default value of a control
• Add (more) supports for controls of menu type
• Add correct support for controls of int64 type (pixel rate)

main: - Supported controls:
                       Brightness 0x00980900 (int)    (flags=0x00) : min=-15 max=15 step=1 default=0 value=0 
                         Contrast 0x00980901 (int)    (flags=0x00) : min=0 max=255 step=1 default=0 value=0 
                       Saturation 0x00980902 (int)    (flags=0x00) : min=0 max=255 step=1 default=64 value=64 
                              Hue 0x00980903 (int)    (flags=0x00) : min=0 max=359 step=1 default=0 value=0 
                  Gain, Automatic 0x00980912 (int)    (flags=0x10) : min=0 max=1 step=1 default=1 value=1 
                  Horizontal Flip 0x00980914 (bool)   (flags=0x00) : min=0 max=1 step=1 default=0 value=0 
                    Vertical Flip 0x00980915 (bool)   (flags=0x00) : min=0 max=1 step=1 default=0 value=0 
             Power Line Frequency 0x00980918 (menu)   (flags=0x00) : min=0 max=3 step=1 default=1 value=1 
                                  0: Disabled
                                  1: 50 Hz
                                  2: 60 Hz
                                  3: Auto
                    Analogue Gain 0x009e0903 (int)    (flags=0x0c) : min=0 max=1023 step=1 default=0 value=16 
                       Pixel Rate 0x009f0902 (int64)  (flags=0x01) : min=24000000 max=96000000 step=1 default=48000000 value=48000000 
                     Test Pattern 0x009f0903 (menu)   (flags=0x00) : min=0 max=4 step=1 default=0 value=0 
                                  0: Disabled
                                  1: Color bars
                                  2: Color bars with rolling bar
                                  3: Color squares
                                  4: Color squares with rolling bar

[decsny]

Is this going to be tested?

[jilaypandya]

shall we return the struct by pointer instead?

[josuah]

I like the direction it is going! Extending the video APIs is mandatory to support some hardware. It also could benefit from reusing mechanisms already present in Zephyr to make it fit more naturally (presentation below). Unless I missed some requirements or goals.

Here is how I understand it [EDIT: the JPEG encoder is a "memory-to-memory" type (m2m) and not part of the devicetree chain like it is suggested below]

Currently every driver stores their const struct device *source_dev somewhere locally in their struct xxx_config without a standard way to access or declare these pointers.

AFAIU, your proposal is to register source_dev along with other related structs into iterable sections that can be looped through to navigate this graph of video devices at runtime:

video_async_init(&data->notifier, dev, &data->vdev, NULL, config->source_devs, 1);

Going from a struct device * to a struct video_notifier * requires looping through all the struct video_notifiers * registered as there is no standard offset for the struct video_notifier *.

It seems like grouping all of these related structures into just one struct video_data accessible like this would remove the need to have all these accessor, register, ... functions like what USB UDC drivers do:

struct video_data *data = dev->data;
struct xxx_data = data->priv;

For the Network Ethernet drivers, it seems like being solved with helper functions:
https://github.com/zephyrproject-rtos/zephyr/blob/main/include/zephyr/net/ethernet.h#L583-L584

struct video_data *data = get_video_data(dev);

It looked like this could avoid any iterable sections this way.

Was it something you considered and discarded the idea? If so, I'd be curious to learn why as maybe I missed something important in this.

I will continue the review when I get a chance. Thank you!

[josuah]

Is this going to be tested?

There is a fake MIPI RX driver and fake imager made for this user-case of unit-testing API changes:

• video: emul: virtual driver for an imager and RX engine #79482

These are "fake" but actually produce frames, so can be tested in hardware too.

[josuah]

This PR could also provide some way to list all existing video devices via DEVICE_API_IS(video, dev):

• meta: Track progress on moving device drivers APIs to iterable structs #82102
• drivers: Move API driver structs to an iterable section #71773

[ngphibang]

This PR could also provide some way to list all existing video devices via DEVICE_API_IS(video, dev):

#82102
#71773

I will rebase this PR once the video API is moved to use DEVICE_API but I don't think this PR is related to that one.

[Copilot]

Copilot reviewed 23 out of 26 changed files in this pull request and generated 2 comments.

Files not reviewed (3)

• cmake/linker_script/common/common-ram.cmake: Language not supported
• drivers/video/CMakeLists.txt: Language not supported
• drivers/video/video.ld: Language not supported

Comments suppressed due to low confidence (1)

drivers/video/ov5640.c:1307

• The initialization for the JPEG compression quality control is applied to the 'saturation' field instead of the dedicated 'jpeg' field defined in the control structure; update to use the correct field for clarity and functionality.

ret = video_init_ctrl(&ctrls->saturation, dev, VIDEO_CID_JPEG_COMPRESSION_QUALITY, ...);

[ngphibang]

Force push:

• Fixed copilot's founded issues.
• Added some proposed comments to the code.
• Added API changes to the release notes 4.2. I didn't think the 4.2 release note is already available :-).

Addressed - thanks!

[ngphibang]

Just rebased on the latest main as release-note-4.2 has evolved.

[ngphibang]

@kartben @josuah @loicpoulain : Could you revisit this ? I don't know if it needs a re-approval ...
Thanks !

[josuah]

I could not find more issues in it so far.

I feel like it will not be able to spot everything down the last small glitch by looking at the code, and testing it will help like it did for the zephyr,video-sw-generator. In which cases more bugfix PRs can be sent :)

• The CI tested it (zephyr,video-emul-imager and zephyr,video-emul-rx) and no bug found.
• The video shell PR tested it and no bug found.

[ngphibang]

I tested this on the video sw generator, mt9m114 and ov5640. The ov5640 currently has the most complete and complex control types, especially the menu control, clustered control. There are currently very few drivers implementing controls and amongst these drivers, very few control types are implemented, so not much things to test. I believe one of the reason is the lack of support in the framework.

Hope that having this will encourage people to implement more interesting controls in their drivers, like the one in #88825.