RFC: Intel IPU4 driver proof of concept

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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





[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux