Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

meta: Track progress on moving video drivers and devicetree to use the video-interfaces binding and the endpoint DT macros #80514

Open
josuah opened this issue Oct 28, 2024 · 3 comments
Labels
area: Drivers area: Video Video subsystem Enhancement Changes/Updates/Additions to existing features platform: ESP32 Espressif ESP32 platform: NXP Drivers NXP Semiconductors, drivers platform: STM32 ST Micro STM32

Comments

@josuah
Copy link
Collaborator

josuah commented Oct 28, 2024

Describe the situation

Following #74415 and #80649, video drivers are in the middle of a migration away from an ad-hoc devicetree API.

Migration to use the video-interfaces binding and the endpoint DT macros is strongly recommended because of the following benefices:

  • We can actually use the remote-endpoint-label to retrieve the peer remote device object in the driver without the need of redundant direct phandle reference in DT.
  • We can avoid some sort of chicken-egg issue due to direct phandle reference as described here.
  • We can use the common properties defined in the video-interfaces binding to avoid declaring so many vendor custom properties in each vendor binding.

With the new video-interfaces binding, remote-endpoint must be replaced by remote-endpoint-label in the devicetree.
The reason for it was #57708 and the workaround is #74415. A final conversion from remote-endpoint-label to remote-endpoint will need to happen once #57708 is addressed, but will be trivial to propagate.

Old API, drivers that need conversion

&source_dev {
        status = "okay";
};

&sink_dev {
        source = <&source_dev>;
        /* or */
        sensor = <&source_dev>;
        /* or nothing */
};

New API, drivers that got converted

&source_dev {
        status = "okay";
        port {
                source_dev_ep_out: endpoint {
                        remote-endpoint-label = "sink_dev_ep_in";
                };
        };
};

&sink_dev {
        port {
                sink_dev_ep_in: endpoint {
                        remote-endpoint-label = "source_dev_ep_out";
                };
        };
};

Impact

  • Need to also convert the board and shields
  • Only one API through the sources, more similar to Linux.
  • A stable API that covers complex situations.

Additional context

Zephyr v4.0 is in feature-freeze: not possible to rush every vendor to complete the migration of their driver in time.

This can be done gradually as here, as commented in the following PR, for example:

@josuah josuah added bug The issue is a bug, or the PR is fixing a bug area: Drivers platform: STM32 ST Micro STM32 platform: ESP32 Espressif ESP32 area: Video Video subsystem platform: NXP Drivers NXP Semiconductors, drivers labels Oct 28, 2024
@josuah josuah added this to the v4.0.0 milestone Oct 28, 2024
@josuah
Copy link
Collaborator Author

josuah commented Oct 28, 2024

It might not be required to push any bugfix for v4.0.0:

Currently, the sensor drivers do not do anything with the remote-endpoint-label, so passing them remote-endpoint-label = "unused"; will work with MIPI/DVP controllers with the old API: not ideal but not broken.

Also, as pointed-out here, adding the remote-endpoint-label is not expected to break the compilation.

@josuah josuah removed this from the v4.0.0 milestone Oct 28, 2024
@ngphibang ngphibang removed the bug The issue is a bug, or the PR is fixing a bug label Oct 28, 2024
@ngphibang
Copy link
Contributor

ngphibang commented Oct 28, 2024

Thanks @josuah for creating the issue. I made some modifications in the description. Since it is not considered as "bug" with the current situation (i.e. declare remote-endpoint in DT but not use it in drivers and declare redundant phandle references source / sensor dev in DT) so I removed the bug label.

remote-endpoint is declared in some dts but not actually used in any Zephyr driver. And while we don't declare a binding for the endpoint, declaring or not the remote-endpoint or anything else doesn't matter, it still compiles.

However, migration to use the video-interfaces binding is strongly recommended because of the following benefits:

  • We can actually use the remote-endpoint-label to retrieve the peer remote device object in the driver without the need of extra direct phandle reference.
  • We can avoid some sort of chicken-egg issue due to direct phandle reference as described here.
  • We can use the common properties in the video-interfaces binding to avoid declaring so many vendor custom properties in each vendor binding.

@josuah josuah changed the title drivers: video: migrate from source=/sensor= to remote-endpoint-label= drivers: video: migrate from source/sensor to remote-endpoint-label Oct 28, 2024
@ngphibang ngphibang changed the title drivers: video: migrate from source/sensor to remote-endpoint-label drivers: video: migrate to use video-interfaces binding Oct 28, 2024
@henrikbrixandersen henrikbrixandersen added the Enhancement Changes/Updates/Additions to existing features label Nov 5, 2024
@ngphibang ngphibang changed the title drivers: video: migrate to use video-interfaces binding meta: Track progress on moving video drivers and devicetree to use the video-interfaces binding and the endpoint DT macros Dec 10, 2024
@ngphibang
Copy link
Contributor

ngphibang commented Dec 10, 2024

Now #80649 is merged, I think it's time to do the migration. At the beginning, I thought it's better to let the code owners do it because they have HW to test and they understand well which driver-specific properties could be replaced by the common video interfaces properties, for example, in esp32-cam.yaml or stm32-dcmi.yaml, most of the properties can be replaced by the common properties (with some slight modifications in code). However, it seems we should take the initiatives then ask for reviews from them.

I will start with the NXP smartDMA / ov7670. @josuah feel free to take some if you want :-).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Drivers area: Video Video subsystem Enhancement Changes/Updates/Additions to existing features platform: ESP32 Espressif ESP32 platform: NXP Drivers NXP Semiconductors, drivers platform: STM32 ST Micro STM32
Projects
None yet
Development

No branches or pull requests

3 participants