Improve the ov5640 video driver to provide parallel interface (DVP) support. #74353

CharlesDias · 2024-06-15T22:09:40Z

Is your enhancement proposal related to a problem? Please describe.
The current ov5640 does not support the parallel interface, it supports MIPI CSI-2 interface.

Describe the solution you'd like
Improve the ov5640 video driver to provide parallel interface support as well.

Additional context
Anyone planning on implementing this improvement?

Recently, the OV5640 video driver was added. However, it only supports the MIPI CSI-2 interface. I plan to implement this driver update, if no one is implementing it, and validate it on a STMicroelectronics board via DCMI driver.

update the ov5640 driver (#76124).
add support for ST B-CAMS-OMV-MB1683 shield (#76143)
implement a sample code to run on STM32 board.

/cc @ngphibang @kartben @erwango @ABOSTM @FRASTM @josuah @loicpoulain @danieldegrasse

josuah · 2024-06-16T00:32:09Z

I have worked a bit with that sensor in the past in parallel (non-MIPI) mode, so could give a hand.
Available in the chat.

This might be the first sensor that would be possible to configure in either modes.

There is a source = <&phandle>; devicetree property that would allow the controller to configure the sensor into the correct mode, maybe that is the way.

On Linux, the MIPI parameters are passed as part of the endpoint:

ports {
        port {
                endpoint {
                        mipi-lanes = <1 2 3 4>;
                        remote-endpoint = <&other>;
                };
        }
};

That opens these possibilities:

Encode the selection of the mode in the devicetree
Encode a phandle in the devicetree and have the controller call control commands to select the DVP or MIPI configuration for the sensor.

CharlesDias · 2024-06-16T10:35:58Z

Thanks for your availability, @josuah! I am not a specialist, but could be a good opportunity to learn more about camera sensor and Zephyr drivers as well.

CharlesDias · 2024-06-16T12:37:12Z

This was the partial DTS that I'm using in the initial tests with OV5640 sensor and ST DCMI driver on STM32H7B3I-DK board.

&i2c4 {
	pinctrl-0 = <&i2c4_scl_pd12 &i2c4_sda_pd13>;
	pinctrl-names = "default";
	status = "okay";
	clock-frequency = <I2C_BITRATE_FAST>;

	ov5640: ov5640@3c {
		compatible = "ovti,ov5640";
		reg = <0x3c>;
		status = "okay";
		reset-gpios = <&gpioa 0 GPIO_ACTIVE_LOW>;
		powerdown-gpios = <&gpioa 7 GPIO_ACTIVE_HIGH>;

		port {
			ov5640_ep_out: endpoint {
				remote-endpoint = <&dcmi_ep_in>;
			};
		};
	};
};

&dcmi {

	status = "okay";
	sensor = <&ov5640>;
	pinctrl-0 = <&dcmi_hsync_pa4 &dcmi_pixclk_pa6 &dcmi_vsync_pb7
		&dcmi_d0_pc6 &dcmi_d1_pc7 &dcmi_d2_pg10 &dcmi_d3_pc9
		&dcmi_d4_pc11 &dcmi_d5_pd3 &dcmi_d6_pb8 &dcmi_d7_pb9>;
	pinctrl-names = "default";
	bus-width = <8>;
	hsync-active = <0>;
	vsync-active = <0>;
	pixelclk-active = <1>;
	capture-rate = <1>;
	dmas = <&dma1 0 75 (STM32_DMA_PERIPH_TO_MEMORY | STM32_DMA_PERIPH_NO_INC |
		STM32_DMA_MEM_INC | STM32_DMA_PERIPH_8BITS | STM32_DMA_MEM_32BITS |
		STM32_DMA_PRIORITY_HIGH) STM32_DMA_FIFO_1_4>;

	port {
		dcmi_ep_in: endpoint {
			remote-endpoint = <&ov5640_ep_out>;
		};
	};
};

document_5147857391924020551.mp4

CharlesDias · 2024-06-16T12:47:34Z

@josuah, if I got your suggestion, the previous DTS should be updated to something like below? I'm using the interface-typeproperty to specify the interface.

Parallel interface is used.

&i2c4 {
	pinctrl-0 = <&i2c4_scl_pd12 &i2c4_sda_pd13>;
	pinctrl-names = "default";
	status = "okay";
	clock-frequency = <I2C_BITRATE_FAST>;

	ov5640: ov5640@3c {
		compatible = "ovti,ov5640";
		reg = <0x3c>;
		status = "okay";
	      
		reset-gpios = <&gpioa 0 GPIO_ACTIVE_LOW>;
		powerdown-gpios = <&gpioa 7 GPIO_ACTIVE_HIGH>;
		
	        interface-type = "dvp";
		port {
		    dvp_ep_out: endpoint {
			bus-width = <8>;
			hsync-active = <0>;
			vsync-active = <0>;
			pixelclk-active = <1>;
		        remote-endpoint = <&dcmi_ep_in>;
		    };
		};
	};
};

&dcmi {

	status = "okay";
	sensor = <&ov5640>;
	pinctrl-0 = <&dcmi_hsync_pa4 &dcmi_pixclk_pa6 &dcmi_vsync_pb7
		&dcmi_d0_pc6 &dcmi_d1_pc7 &dcmi_d2_pg10 &dcmi_d3_pc9
		&dcmi_d4_pc11 &dcmi_d5_pd3 &dcmi_d6_pb8 &dcmi_d7_pb9>;
	pinctrl-names = "default";
	capture-rate = <1>;
	dmas = <&dma1 0 75 (STM32_DMA_PERIPH_TO_MEMORY | STM32_DMA_PERIPH_NO_INC |
		STM32_DMA_MEM_INC | STM32_DMA_PERIPH_8BITS | STM32_DMA_MEM_32BITS |
		STM32_DMA_PRIORITY_HIGH) STM32_DMA_FIFO_1_4>;

	port {
		dcmi_ep_in: endpoint {
			remote-endpoint = <&dvp_ep_out>;
		};
	};
};

Parallel interface is used.

&i2c4 {
	pinctrl-0 = <&i2c4_scl_pd12 &i2c4_sda_pd13>;
	pinctrl-names = "default";
	status = "okay";
	clock-frequency = <I2C_BITRATE_FAST>;

	ov5640: ov5640@3c {
		compatible = "ovti,ov5640";
		reg = <0x3c>;
		status = "okay";
	       
		reset-gpios = <&gpioa 0 GPIO_ACTIVE_LOW>;
		powerdown-gpios = <&gpioa 7 GPIO_ACTIVE_HIGH>;
		
	        interface-type = "mipi";
		port {
		    mipi_ep_out: endpoint {
		        data-lanes = <1 2>;
		        clock-lanes = <0>;
		        bus-width = <2>;
		        remote-endpoint = <&mipi_csi_ep_in>;
		    };
		};
	};
};

&mipi_csi {
    status = "okay";

    /* Add other configs. /*
    
    port {
        mipi_csi_ep_in: endpoint {
            remote-endpoint = <&mipi_ep_out>;
        };
    };
};

josuah · 2024-06-16T13:02:31Z

That would be something like this in Linux (with the ~~mipi-lanes~~ data-lanes (I misremembered) on the mipi_csi controller block I think), but currently, there is no semantics given in Zephyr for the remote-endpoint. Configuring it is pure documentation and not used in the driver.

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/st/stm32mp15xx-dhcor-avenger96.dtsi#n156

&dcmi {
        pinctrl-names = "default", "sleep";
        pinctrl-0 = <&dcmi_pins_c>;
        pinctrl-1 = <&dcmi_sleep_pins_c>;
        status = "disabled";

        port {
                dcmi_0: endpoint {
                        remote-endpoint = <&stmipi_2>;
                        bus-type = <5>;
                        bus-width = <8>;
                        pclk-sample = <0>;
                };
        };
};

And another example when MIPI is used:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/nxp/imx/imx6q-h100.dts#n337

&mipi_csi {
        status = "okay";

        port {
                mipi_csi2_in: endpoint {
                        remote-endpoint = <&tc358743_out>;
                        data-lanes = <1 2 3 4>;
                        clock-lanes = <0>;
                        clock-noncontinuous;
                        link-frequencies = /bits/ 64 <297000000>;
                };
        };
};

Although Zephyr does not have any of these, and configuring them here might not be necessary to get them to work.

Even having any remote-endpoint declared at all is just symbolic currently.

CharlesDias · 2024-06-16T13:44:24Z

Thanks, @josuah ! I'll take a better look at those links.

josuah · 2024-06-16T13:59:21Z

In Linux, the sensor detects whether it is configured on a MIPI bus or on a DVP/DMCI bus:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/media/i2c/ov5640.c#n2712

This means a OV5640 camera devicetree node would be nested into either a MIPI or DMCI node/block, and this would sets which type it is configured with.

I think there lack a devicetree bindings declaration on-bus: mipi; which would allow to introduce that difference in type.

But of course, MIPI devices aare "on-bus" for their configuration interface: I2C, not MIPI or CSI!

This is the "on-bus" for their remote-endpopint that is considered here: sensor->ep.bus_type

This means we would probably need something like this to be merged first: #73023

Maybe a simple devicetree configuration could be done in the meantime? So that you are not blocked by this.
Some bus-type = with a comment mentioning #73023, at least to allow other members to give their feedback on this.

ngphibang · 2024-06-17T12:35:13Z

+1 from me. I think this would be a nice extension for ov5640. FYI, we are currently adding framerate changing to this driver but this shouldn't be a problem.

I think there lack a devicetree bindings declaration on-bus: mipi; which would allow to introduce that difference in type.

But of course, MIPI devices aare "on-bus" for their configuration interface: I2C, not MIPI or CSI!

This is the "on-bus" for their remote-endpopint that is considered here: sensor->ep.bus_type

This means we would probably need something like this to be merged first: #73023

@josuah , @CharlesDias : For this CSI / DVP (parallel) extension, as you said, we just need a "bus-type" property defined in the devicetree. We should group all these common properties into a "video-interfaces.yaml" binding (to avoid repeating them for each driver), some thing like this which I picked from Linux. Then, in the driver, we can get the property by : DT_PROP(DT_INST_CHILD(DT_INST_CHILD(id, port), endpoint), bus_type).

This will be similar to what actually done for display, as here.

Support of remote-endpoint in Zephyr would be nicer but I think it is not a blocking point for this case.

josuah · 2024-06-17T13:05:54Z

some thing like #74415 which I picked from Linux

Exciting! This is some good way to structure the various video drivers.

Then, in the driver, we can get the property by : DT_PROP(DT_INST_CHILD(DT_INST_CHILD(id, port), endpoint), bus_type).

I agree and this is good to have it this way in the meantime.

@CharlesDias would a bus-type property on the devicetree work for you?

Something like this:

&i2c4 {
		port {
			mipi_ep_out: endpoint {
				bus-type = <...>;
			};
		};
	};
};

CharlesDias · 2024-06-17T13:46:17Z

@CharlesDias would a bus-type property on the devicetree work for you?

@josuah, I think so!

Thank you, @josuah and @ngphibang! I'll wait a little more for other members' to confirm that no one is working on this.

CharlesDias · 2024-06-18T21:28:27Z

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/st/stm32mp15xx-dhcor-avenger96.dtsi#n156

Hi, @josuah and @ngphibang. I hope you can help me with the following question. On the Linux driver, from the link above, we have duplicate properties declared for both endpoints, dcmi and i2c. I thought this duplication strange. My questions are:

Will we do this duplication on Zephyr?
On Linux, the bus-type is defined into DCMI endpoint instead of OV5640 endpoint, but I believe for Zephyr it doesn't make sense. So, will we add the bus-type in the OV5640 endpoint, right?

Thanks!

&i2c4 {
	stmipi: stmipi@14 {
                . . . 
		ports {
                 . . .
			port@2 {
				reg = <2>;
				stmipi_2: endpoint {
					bus-width = <8>;
					hsync-active = <0>;
					vsync-active = <0>;
					pclk-sample = <0>;
					remote-endpoint = <&dcmi_0>;
				};
			};
		};
	};

&dcmi {
        . . . 

	port {
		dcmi_0: endpoint {
			remote-endpoint = <&stmipi_2>;
			bus-type = <5>;
			bus-width = <8>;
			pclk-sample = <0>;
		};
	};
};

ngphibang · 2024-06-18T22:46:06Z

@CharlesDias

Firstly, I think this is not a full description of the ov5640 camera node, as you can see, it lacks some things like reset-gpios, pwdn-gpios, etc. These things will be completed by a camera overlay dts where you may find the bus-type property. Unlike Zephyr, overlay devicetrees are not integrated into Linux so you will not see them in upstream.
Secondly, the present of bus-width already indicated that this is a parallel interface as pointed out here and even without bus-type property in the devicetree, the parsing function can guess the actual bus-type from this property.
Thirdly, what we should base on is the ov5640 driver, you can see here and here , it really parses the endpoint of the ov5640 to get the bus-type.
Then, as a reference, you can see here where bus-type is declared for ov5640 node.
Finally, there is a low possibility that we don't need to define bus-type in the sensor node but we get it from the dcmi via remote-endpoint. But, remote-endpoint is currently not supported in Zephyr.

So, in any of above cases, I think we should define bus-type in ov5640 node.

ngphibang · 2024-06-18T22:59:17Z

For the "duplicated" properties, do you mean bus-width and pclk-sample ? IMO, it is because these properties are not necessarily identical for sensor and dcmi (although we see that they are the same in most cases), for example, pclk-sample can be rising edge for ov5640 and both rising / falling edge for dcmi (?) when there is signal modification on the bus, e.g. a logic signal inverter is present. Anyways, in ov5640 driver, you can see it is obtained from the endpoint of ov5640 here and in dcmi driver, it is also obtained from endpoint of dcmi node here as separate properties.

CharlesDias · 2024-06-19T09:51:48Z

So, in any of above cases, I think we should define bus-type in ov5640 node.

Also make sense for me to define the bus-type in ov5640 node. I will continue with that in mind.

For the "duplicated" properties, do you mean bus-width and pclk-sample ? IMO, it is because these properties does not necessarily have the same value for sensor and dcmi (although we see that they are the same in devicetree),

I did not know that! :-)

Thanks so much @ngphibang for the explanation!

ngphibang · 2024-08-21T15:35:29Z

Hi @CharlesDias ,
FYI, I finalized the video binding PR and put it into review (it was in Draft). An example of using this can be found here and here. Sorry for the delay.

You could also rebase the DCMI binding on this common binding if you want.

Thanks !

CharlesDias · 2024-08-23T17:39:52Z

Thanks, @ngphibang! Well done!

CodingGhost · 2024-10-24T10:22:34Z

@CharlesDias I am trying to use the ov5460 on a STM32H7a3zi. Im using latest Zephyr master but I get
video_ov5640: Unsupported pixel format or resolution.

Is there any PR I need for this to run thats not yet in master? Or did I do something wrong? Im using the lvgl sample but removed all lvgl code as I dont have a display.

josuah · 2024-10-24T12:55:36Z

@CodingGhost I suspect this is related to a different PR/issue, and opened a discussion about it here:

capture to LVGL sample: ov5460 on a STM32H7a3zi #80371

Do feel free to comment the OV5640 video driver and DVP support here, though let's troubleshoot your issue on #80371 if that's ok. :)

CharlesDias added the Enhancement Changes/Updates/Additions to existing features label Jun 15, 2024

ngphibang mentioned this issue Jun 18, 2024

dts: bindings: video: Add common video interface binding #74415

Merged

henrikbrixandersen added the area: Video Video subsystem label Jun 18, 2024

CharlesDias mentioned this issue Jul 19, 2024

Add parallel interface (DVP) support for ov5640 driver #76124

Open

josuah mentioned this issue Oct 24, 2024

capture to LVGL sample: ov5460 on a STM32H7a3zi #80370

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve the ov5640 video driver to provide parallel interface (DVP) support. #74353

Improve the ov5640 video driver to provide parallel interface (DVP) support. #74353

CharlesDias commented Jun 15, 2024 •

edited

Loading

josuah commented Jun 16, 2024

CharlesDias commented Jun 16, 2024

CharlesDias commented Jun 16, 2024

CharlesDias commented Jun 16, 2024

josuah commented Jun 16, 2024 •

edited

Loading

CharlesDias commented Jun 16, 2024

josuah commented Jun 16, 2024

ngphibang commented Jun 17, 2024 •

edited

Loading

josuah commented Jun 17, 2024 •

edited

Loading

CharlesDias commented Jun 17, 2024

CharlesDias commented Jun 18, 2024

ngphibang commented Jun 18, 2024 •

edited

Loading

ngphibang commented Jun 18, 2024 •

edited

Loading

CharlesDias commented Jun 19, 2024

ngphibang commented Aug 21, 2024

CharlesDias commented Aug 23, 2024

CodingGhost commented Oct 24, 2024

josuah commented Oct 24, 2024

Improve the ov5640 video driver to provide parallel interface (DVP) support. #74353

Improve the ov5640 video driver to provide parallel interface (DVP) support. #74353

Comments

CharlesDias commented Jun 15, 2024 • edited Loading

josuah commented Jun 16, 2024

CharlesDias commented Jun 16, 2024

CharlesDias commented Jun 16, 2024

CharlesDias commented Jun 16, 2024

josuah commented Jun 16, 2024 • edited Loading

CharlesDias commented Jun 16, 2024

josuah commented Jun 16, 2024

ngphibang commented Jun 17, 2024 • edited Loading

josuah commented Jun 17, 2024 • edited Loading

CharlesDias commented Jun 17, 2024

CharlesDias commented Jun 18, 2024

ngphibang commented Jun 18, 2024 • edited Loading

ngphibang commented Jun 18, 2024 • edited Loading

CharlesDias commented Jun 19, 2024

ngphibang commented Aug 21, 2024

CharlesDias commented Aug 23, 2024

CodingGhost commented Oct 24, 2024

josuah commented Oct 24, 2024

CharlesDias commented Jun 15, 2024 •

edited

Loading

josuah commented Jun 16, 2024 •

edited

Loading

ngphibang commented Jun 17, 2024 •

edited

Loading

josuah commented Jun 17, 2024 •

edited

Loading

ngphibang commented Jun 18, 2024 •

edited

Loading

ngphibang commented Jun 18, 2024 •

edited

Loading