Hi, As mentioned previously in Bingbu's IPU6 patch series, I'm working on porting the driver to IPU4. I've now got a hole through so I think it makes sense sense to share the code. I'm able to capture frames with yavta with the current code, but there are several issues that needs to be fixed for it to be complete. # How it is tested ================== The hardware is a custom x86 PC-like embedded device with the following video pipeline: Endoscope -> FPGA -> tc358748 -> IPU4 (E3950/Apollo Lake) See my colleague Claus' description[2] for more info. There is currently no V4L2 subdevice for the FPGA, so we have a custom ambu-tc358748.c driver which pretends to be an image sensor. $ media-ctl -v \ -V "\ \"tc358748 0-000e\" :0 [fmt:RGB888_1X24/800x800],\ \"Intel IPU4 CSI2 0\" :0 [fmt:RGB888_1X24/800x800],\ \"Intel IPU4 CSI2 0\" :1 [fmt:RGB888_1X24/800x800]\ "\ -l "\ \"tc358748 0-000e\" :0 -> \"Intel IPU4 CSI2 0\" :0 [1],\ \"Intel IPU4 CSI2 0\" :1 -> \"Intel IPU4 ISYS Capture 12\" :0 [5]\ " $ yavta --data-prefix -c2 -n2 -I -s 800x800 --file=/tmp/frame-#.bin \ -f XBGR32 /dev/video12 This produces frame-*.bin files containing 800x800x4 bytes of valid "BGR0" data. # The code ========== The code is available at the tag https://github.com/Kleist/linux/tree/kleist-v6.6-ipu4-hacks-1 (15245fe26e07) Note that I haven't renamed the files to ipu4, to make it clear what the changes are compared to the IPU6 driver. It is based on v6.6 with the IPU6 v2 patches[1] on top, and then my hacks to make the IPU4 work. This is not meant for upstreaming as it is. The commits are a cleaned up version of the chronological order I made the port in. It is not yet in a state where I think an RFC PATCH series makes sense yet, but I wanted to share it anyway. ## Changes compared to IPU6 diff --stat of the changes in ../ipu6/ compared to the IPU6 v2 patches: drivers/media/pci/intel/ipu6/Kconfig | 12 +- drivers/media/pci/intel/ipu6/Makefile | 13 +- drivers/media/pci/intel/ipu6/ipu6-bus.c | 2 +- drivers/media/pci/intel/ipu6/ipu6-bus.h | 6 +- drivers/media/pci/intel/ipu6/ipu6-buttress.c | 71 ++- drivers/media/pci/intel/ipu6/ipu6-buttress.h | 8 +- drivers/media/pci/intel/ipu6/ipu6-fw-com.c | 45 +- drivers/media/pci/intel/ipu6/ipu6-fw-com.h | 2 +- drivers/media/pci/intel/ipu6/ipu6-fw-isys.c | 171 ++++--- drivers/media/pci/intel/ipu6/ipu6-fw-isys.h | 237 ++++++---- drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c | 219 +++++---- drivers/media/pci/intel/ipu6/ipu6-isys-csi2.h | 11 +- drivers/media/pci/intel/ipu6/ipu6-isys-queue.c | 33 +- drivers/media/pci/intel/ipu6/ipu6-isys-queue.h | 8 +- drivers/media/pci/intel/ipu6/ipu6-isys-video.c | 212 +++------ drivers/media/pci/intel/ipu6/ipu6-isys-video.h | 4 - drivers/media/pci/intel/ipu6/ipu6-isys.c | 435 +++---------- ----- drivers/media/pci/intel/ipu6/ipu6-isys.h | 18 +- drivers/media/pci/intel/ipu6/ipu6-mmu.c | 130 +++++- .../pci/intel/ipu6/ipu6-platform-buttress-regs.h | 98 +--- .../pci/intel/ipu6/ipu6-platform-isys-csi2-reg.h | 226 ++------- drivers/media/pci/intel/ipu6/ipu6-platform-regs.h | 172 ++----- drivers/media/pci/intel/ipu6/ipu6.c | 511 ++++++++----- -------- drivers/media/pci/intel/ipu6/ipu6.h | 37 +- 24 files changed, 1032 insertions(+), 1649 deletions(-) Note that most of the deleted lines are removed because they are not used in IPU4. E.g. the watermark handling, which I haven't seen an equivalent for in the old IPU4 driver. ## Ambu-specific tweaks Note that I'm using a hacked ipu-bridge (AMBU_IPU_BRIDGE) to setup the fwnode graph for our hardware. You don't want if you're testing this, so revert at least the "ambu: Add AMBU_IPU_BRIDGE" commit. I'm not sure the right approach for handling this would be going forward. Of course the ambu-ipu-bridge shouldn't be upstreamed, so I'm wondering how we can achieve something similar? The ACPI tables from our BIOS unfortunately don't contain any info about the Toshiba Bridge (tc358748), so we can't derive the information from there. Maybe some kind of platform driver could be created which tweaks the ACPI info before the ipu-bridge driver reads it? What do you typically do when you have some proprietary hardware that does not provide proper ACPI information? We could carry the ambu-ipu- bridge patches in our internal kernel tree, but that is not desirable in the long term. # Inspiration for the IPU4 port =============================== We are currently using a Intel LTS 4.19.217 based kernel[3], which contains the old IPU4 driver. The port was basically made by comparing mmiotrace's between the old IPU4 driver and the new driver. We're using the IPU4 FW ipu4_cpd_b0.bin extracted from a ClearLinux package[4]. # Known issues ============== ## Doesn't yet work with gstreamer for unknown reasons I get "Unexpected buffer address:" errors from ipu6_isys_queue_buf_ready, and don't get an image through. ## 64 byte chunks of wrong data We occasionally get 64 byte aligned 64 byte wrong data (all 0xCC) in the captured frame*.bin files. This could be a cache invalidation issue, we haven't looked into this yet. The code currently doesn't use zlw_invalidate, even though it was ported from the old driver. We haven't yet tested if enabling this fixes the issue. # Upstreaming ============= We would like to upstream this driver, probably after the IPU6 driver has been merged. We're definitely not ready yet (either), but I already have a couple of questions, that it would be nice to get some input on from the community. ## How to share code between IPU4 and IPU6 Big parts of the code (approximately 6k out of 7k lines) does not need to be changed compared to the IPU6 driver, so there is clearly a big overlap in what the two drivers need to do. I'm not sure how the best approach would be for sharing this functionality. I see a few options: 1. Shared driver that supports both IPU's (still split in PCI driver and -isys driver) 2. Shared PCI driver that supports both IPU's, but device-specific intel-ipu4-isys/intel-ipu6-isys drivers 3. Separate drivers that use a shared "library module" (for lack of a better term) My gut feeling is that 2. is the right choice, especially if we moved the shared code in to the PCI driver and the more version-specific code was moved into the specific drivers. The answer to this could also be input to Bingbu's IPU6 series, maybe it would make sense to place some files differently if they eventually will be used in both IPU4 and IPU6 drivers? ## How to implement our platform specific fwnode graph? As mentioned above, we currently have a hacked ambu-ipu-bridge driver, which is clearly not upstreamable. What would you typically do if you need to make a v4l setup where the ACPI table information about sensors/bridges is missing? /Andreas [1]https://lore.kernel.org/all/20231024112924.3934228-1-bingbu.cao@xxxxxxxxx/ [2] https://lore.kernel.org/all/471df7ffdf34b73d186c429a366cfee62963015f.camel@xxxxxxxxx/ [3] https://github.com/intel/linux-intel-lts/tree/lts-v4.19.217-base-211118T072627Z [4] https://download.clearlinux.org/releases/32370/clear/source/SRPMS/linux-firmware-ipu-19ww39-104.src.rpm