Hi Guillaume,
On Tue, 09 Apr 2024 09:55:32 +0000, BRES, Guillaume wrote:
[...]
> > You are mixing the bindings of the downstream and the upstream driver. You have to stick to one binding to use one driver or the other.
>
> That was just our latest attempt. It is not very clear what we are supposed to declare (see following question).
> From what you said, I adapted the devicetree to the following, allegrodvt is now probed, it fails to locate IRQ#0, but I am just starting to debug, I'm confident we will find the solution to this :
>
> // vcu settings
> 1369 vcu_settings: vcu@a0241000 {
> 1370 compatible = "xlnx,vcu-settings", "syscon";
> 1371 reg = <0x0 0xa0241000 0x0 0x1000>;
> 1372 reg-names = "logicore";
> 1373 };
> // new section
> 1374 vcu_al5e: video-codec@a0009000 {
> 1375 compatible = "allegro,al5e-1.1", "allegro,al5e";
> 1376 reg = <0 0xa0009000 0 0x1000>, <0 0xa0000000 0 0x8000>;
The addresses look wrong. You configured the register mapping when
configuring the bitstream and this mapping needs to be reflected in the
device tree.
In your case the addresses probably should be
reg = <0 0xa0209000 0 0x1000>, <0 0xa0200000 0 0x8000>;
> 1377 reg-names = "regs", "sram";
> 1378 interrupts = <0 104 4>; // try to establish IRQ relationship, but not sure yet IRQ#0 is actually this one or another.
> 1379 clocks = <&vcu 0>, <&vcu 1>, <&zynqmp_clk 71>, <&zynqmp_clk 71>, <&zynqmp_clk 71>;
> 1380 clock-names = "core_clk", "mcu_clk", "m_axi_core_aclk", "m_axi_mcu_aclk", "s_axi_lite_aclk";
> 1381 };
> // still partially needed
> 1382 vcu: vcu@a0200000 {
> 1383 #address-cells = <2>;
> 1384 #clock-cells = <1>;
> 1385 #size-cells = <2>;
> 1386 clock-names = "pll_ref", "aclk", "vcu_core_enc", "vcu_core_dec", "vcu_mcu_enc", "vcu_mcu_dec";
> 1387 clocks = <&misc_clk_1>, <&zynqmp_clk 71>, <&vcu 1>, <&vcu 2>, <&vcu 3>, <&vcu 4>;
> 1388 compatible = "xlnx,vcu-1.2", "xlnx,vcu";
> 1389 interrupt-names = "vcu_host_interrupt";
> 1390 interrupt-parent = <&gic>;
> 1391 interrupts = <0 104 4>;
> 1392 ranges ;
> 1393 reg-names = "vcu_slcr";
> 1394 reg = <0x0 0xa0240000 0x0 0x1000>;
> 1395 reset-gpios = <&vfrmbuf_gpio 6 GPIO_ACTIVE_LOW>;
> 1396 };
This node doesn't follow the documented binding. There should be only
two clocks and no interrupt.
>
>
> > What kind of documentation did you expect and where did you expect it?
>
> I think the current kernel lacks some notes - no idea what their
> appropriate location might be, that explain the relationship between
> all those drivers, especially the syscon setup. For the simple reason
> that you need all of these drivers to work at the same time, and one
> of them by itself will never be enough.
The allegro,al5e-1.1 is the actual encoder.
The xlnx,vcu is more or less the clock controller for the VCU, and
controls the clocks for the encoder and the decoder.
The vcu-settings are the registers that expose the VCU configuration,
since the VCU can be configured with different settings. These settings
are shared with the decoder and a potential decoder driver would need to
have access to the register values in vcu-settings, too.
> > The upstream driver is product-driven, and only has the features
> > that are needed for the product
>
> Could you elaborate or point out to a location where we can have
> detailed documentation of the current capabilities ? so we can make a
> decision whether we should upgrade to mainline, or remain on the
> out-of-tree module ?
The driver is able to do h.264 and h.265 encoding with a few options for
quality and rate control. In contrast to the downstream driver, it
exposes a standard V4L2 video encoder interface to userspace.
Which features do you need? I'm very open to review and test patches
that add more features to improve and extend the upstream driver.
Michael
