raspberrypi / linux

Kernel source tree for Raspberry Pi-provided kernel builds. Issues unrelated to the linux kernel should be posted on the community forum at https://forums.raspberrypi.com/
Other
11.03k stars 4.96k forks source link

bcm2835-unicam and cfe metadata byte alignment problem #5713

Open ArduCAM opened 10 months ago

ArduCAM commented 10 months ago

Describe the bug

We tested the IMX519 and found that metadata can be output normally on Pi4, but on Pi5, the cfe driver reported "Wrong metadata width/height/code". After a simple calculation, we found that it seems that the cfe driver requires the length of Metadata to be 64bit aligned. Any ideas? image

Steps to reproduce the behaviour

Running IMX519 on Pi5 using libcamera

Device (s)

Other

System

cat /etc/rpi-issue Raspberry Pi reference 2023-10-10 Generated using pi-gen, https://github.com/RPi-Distro/pi-gen, fb56ad562991cf3ae5c96ab50983e1deeaefc7b6, stage4

vcgencmd version 2023/10/18 18:30:17 Copyright (c) 2012 Broadcom version c2da2ae7 (release) (embedded)

uname -a Linux raspberrypi 6.1.0-rpi6-rpi-v8 #1 SMP PREEMPT Debian 1:6.1.58-1+rpt2 (2023-10-27) aarch64 GNU/Linux

Logs

[    8.020555] macb 1f00100000.ethernet: gem-ptp-timer ptp clock registered.
[    8.051440] brcmfmac: brcmf_cfg80211_set_power_mgmt: power save enabled
[    9.914906] Bluetooth: RFCOMM TTY layer initialized
[    9.914919] Bluetooth: RFCOMM socket layer initialized
[    9.914929] Bluetooth: RFCOMM ver 1.11
[   12.126407] macb 1f00100000.ethernet eth0: Link is Up - 1Gbps/Full - flow control off
[   12.126427] IPv6: ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready
[   34.596407] rp1-cfe 1f00128000.csi: WARNING: Wrong metadata width/height/code 17472x1 00007002 (remote pad set to 17460x1 00007002)

dmesg.log

Additional context

No response

ArduCAM commented 10 months ago

Hi @6by9

Any ideas? Should I modify the driver directly to align the length? I'm worried if other platforms won't match.

6by9 commented 10 months ago

The metadata implementation using 2 pads is a totally downstream set of patches, therefore there are no other platforms that could be affected.

Mainline are still working on their solution, but it will be based around https://patchwork.linuxtv.org/project/linux-media/cover/20231106122539.1268265-1-sakari.ailus@linux.intel.com/ and the streams support that is now merged.

The buffer sizing is solely used for memory allocation. Unicam is very limited in options for handling the non-image data (it writes all incoming to the buffer with no further formatting), hence why it has always been width x 1 despite the incoming data potentially being spread over multiple CSI2 lines.

ArduCAM commented 10 months ago

The metadata implementation using 2 pads is a totally downstream set of patches, therefore there are no other platforms that could be affected.

Mainline are still working on their solution, but it will be based around https://patchwork.linuxtv.org/project/linux-media/cover/20231106122539.1268265-1-sakari.ailus@linux.intel.com/ and the streams support that is now merged.

The buffer sizing is solely used for memory allocation. Unicam is very limited in options for handling the non-image data (it writes all incoming to the buffer with no further formatting), hence why it has always been width x 1 despite the incoming data potentially being spread over multiple CSI2 lines.

Got it, I think I need to submit a pull request to fix it.

ArduCAM commented 10 months ago

Hi @6by9 We found a bad news. The data type of the PD data of IMX519 is 0x36, and it cannot be modified through registers. Is there any way for us to configure it? We looked at the code for the csi2 part and it seems that it is fixed at 0x12. (In addition, the data type of embedded data other than PD is 0x12)

6by9 commented 10 months ago

Unicam just dumps all non-image data into the metadata buffer.

CFE does have full VC/DT filtering, and currently assumes that V4L2_META_FMT_SENSOR_DATA is 0x12 due to https://github.com/raspberrypi/linux/blob/rpi-6.1.y/drivers/media/platform/raspberrypi/rp1_cfe/cfe_fmts.h#L282-L286. The streams support that mainline are developing is the right way to solve this, as the PDAF data would come as a third stream from the sensor, and come out via a further /dev/videoN node. Pass as to when that will be sorted though.

6by9 commented 10 months ago

@naushir As just discussed, not sure if there are other options available in the RP1 CSI2 receiver.

naushir commented 10 months ago

I'm reluctant to change data type handling right now given the testing we've done. Hopefully the streams api ought to allow different data ids to be muxed/demuxed from a channel correctly. This should be available to us very soon.

ArduCAM commented 6 months ago

I'm reluctant to change data type handling right now given the testing we've done. Hopefully the streams api ought to allow different data ids to be muxed/demuxed from a channel correctly. This should be available to us very soon.

Hi naushir, Is there any progress in this regard at the moment?

naushir commented 6 months ago

The V4l2 streams API is being worked on by the libcamera team. Work is progressing, but I don't have a timeline for when it'll be completed fully.

ArduCAM commented 6 months ago

The V4l2 streams API is being worked on by the libcamera team. Work is progressing, but I don't have a timeline for when it'll be completed fully.

Got it, thank you for the information.

nighthawk31337 commented 1 day ago

@naushir any progress on this issue?

naushir commented 1 day ago

The API has not yet been merged upstream. I'm not directly involved with this so I cannot give any specific timelines.