raspberrypi
Pispbe/rpi 6.7.y/be unsquashed
NOT FOR MERGE only for review.
Un-squashed list of changes to the BE driver to prepare for upstreaming for earlier review/comments. Mostly cosmetics, apart from a few commits that rework slightly the job scheduling routines.
Tested with this branch based on Dave's mainline rpi1 support https://github.com/jmondi/raspberrypi-linux/tree/pispbe/6by9_mainline_2712_rp1/libcamera-testing
with single and dual camera capture.
The actual commits for the mainline patches, based on media-staging are at https://github.com/jmondi/raspberrypi-linux/tree/pispbe/media-staging/be-upstream
This last push adds documentation for the image formats and the isp and breaks the uapi header out from the driver directory.
The only missing part is the documentation of the opaque format as I've found none in the PiSP specs.
All of these commits are now collected in: https://github.com/jmondi/raspberrypi-linux/commits/pispbe/media-staging/be-upstream/
which represent what I intend to send upstream once this PR has been approved
Just trying these changes now on top of a rpi-6.6.y tree and unfortunately when I run rpicam-apps or cam, I get the following dmesg output:
[ 34.710292] pispbe 1000880000.pisp_be: ISP BE config error: check if ISP RAMs enabled?
and then the system locks up. I'm sure I've missed something obvious, @jmondi any thoughts?
Ah, ~~perhaps~~ this is the fix:
diff --git a/drivers/media/platform/raspberrypi/pisp_be/pisp_be.c b/drivers/media/platform/raspberrypi/pisp_be/pisp_be.c
index 07f0fb8aa85c..6e88491b20ba 100644
--- a/drivers/media/platform/raspberrypi/pisp_be/pisp_be.c
+++ b/drivers/media/platform/raspberrypi/pisp_be/pisp_be.c
@@ -265,7 +265,7 @@ static void pispbe_queue_job(struct pispbe_dev *pispbe,
for (unsigned int u = 0; u < N_HW_ADDRESSES; ++u) {
pispbe_wr(pispbe, PISP_BE_IO_ADDR_LOW(u),
lower_32_bits(job->hw_dma_addrs[u]));
- pispbe_wr(pispbe, PISP_BE_IO_ADDR_HIGH(u) + 4,
+ pispbe_wr(pispbe, PISP_BE_IO_ADDR_HIGH(u),
upper_32_bits(job->hw_dma_addrs[u]));
}
pispbe_wr(pispbe, PISP_BE_GLOBAL_BAYER_ENABLE, job->hw_enables[0]);
Ah, ~perhaps~ this is the fix:
Why I don't see it ?
Linux version 6.7.0+ (jmondi@fedora) (aarch64-linux-gnu-gcc (GCC) 13.2.1 20230728 (Red Hat Cross 13.2.1-1), GNU ld version 2.39-4.fc38) #31 SMP PREEMPT Thu Feb 8 15:20:13 CET 2024
pi@raspberrypi:~$ cam -c1 -C
[0:01:22.094832560] [982] INFO Camera camera_manager.cpp:284 libcamera v0.1.0+118-563cd78e
[0:01:22.182109208] [987] INFO RPI pisp.cpp:653 libpisp version v1.0.3 7d49ac3beab3 24-01-2024 (08:36:04)
[0:01:22.404264727] [987] INFO RPI pisp.cpp:1112 Registered camera /base/axi/pcie@120000/rp1/i2c@80000/imx219@10 to CFE device /dev/media2 and ISP device /dev/media0 using PiSP variant BCM2712_C0
[0:01:22.405509467] [982] WARN V4L2 v4l2_pixelformat.cpp:338 Unsupported V4L2 pixel format Y16
[0:01:22.405751875] [982] WARN V4L2 v4l2_pixelformat.cpp:338 Unsupported V4L2 pixel format RGB6
[0:01:22.405778079] [982] WARN V4L2 v4l2_pixelformat.cpp:338 Unsupported V4L2 pixel format BGR6
[0:01:22.405818116] [982] WARN V4L2 v4l2_pixelformat.cpp:338 Unsupported V4L2 pixel format PC1M
Using camera /base/axi/pcie@120000/rp1/i2c@80000/imx219@10 as cam0
[0:01:22.407122079] [982] INFO Camera camera.cpp:1183 configuring streams: (0) 800x600-XRGB8888
[0:01:22.408457505] [987] INFO RPI pisp.cpp:1396 Sensor: /base/axi/pcie@120000/rp1/i2c@80000/imx219@10 - Selected sensor format: 1640x1232-SBGGR10_1X10 - Selected CFE format: 1640x1232-PC1B
cam0: Capture until user interrupts by SIGINT
82.789795 (0.00 fps) cam0-stream0 seq: 000007 bytesused: 1920000
82.848400 (17.06 fps) cam0-stream0 seq: 000008 bytesused: 1920000
82.908455 (16.65 fps) cam0-stream0 seq: 000009 bytesused: 1920000
82.968481 (16.66 fps) cam0-stream0 seq: 000010 bytesused: 1920000
83.028505 (16.66 fps) cam0-stream0 seq: 000011 bytesused: 1920000
83.088546 (16.66 fps) cam0-stream0 seq: 000012 bytesused: 1920000
83.148504 (16.68 fps) cam0-stream0 seq: 000013 bytesused: 1920000
83.208769 (16.59 fps) cam0-stream0 seq: 000014 bytesused: 1920000
83.268785 (16.66 fps) cam0-stream0 seq: 000015 bytesused: 1920000
83.328856 (16.65 fps) cam0-stream0 seq: 000016 bytesused: 1920000
83.388942 (16.64 fps) cam0-stream0 seq: 000017 bytesused: 1920000
Ah, ~perhaps~ this is the fix:
diff --git a/drivers/media/platform/raspberrypi/pisp_be/pisp_be.c b/drivers/media/platform/raspberrypi/pisp_be/pisp_be.c index 07f0fb8aa85c..6e88491b20ba 100644 --- a/drivers/media/platform/raspberrypi/pisp_be/pisp_be.c +++ b/drivers/media/platform/raspberrypi/pisp_be/pisp_be.c @@ -265,7 +265,7 @@ static void pispbe_queue_job(struct pispbe_dev *pispbe, for (unsigned int u = 0; u < N_HW_ADDRESSES; ++u) { pispbe_wr(pispbe, PISP_BE_IO_ADDR_LOW(u), lower_32_bits(job->hw_dma_addrs[u])); - pispbe_wr(pispbe, PISP_BE_IO_ADDR_HIGH(u) + 4, + pispbe_wr(pispbe, PISP_BE_IO_ADDR_HIGH(u), upper_32_bits(job->hw_dma_addrs[u])); } pispbe_wr(pispbe, PISP_BE_GLOBAL_BAYER_ENABLE, job->hw_enables[0]);
ah yes, this is correct indeed, the + 4 is included in the macro definition.
Weird I can't trigger it :/
Weird I can't trigger it :/
Probably some kernel difference means you are running without IOMMU, and your addresses were all in the lower 32 bits.
Another fix that is needed is
diff --git a/drivers/media/platform/raspberrypi/pisp_be/pisp_be_formats.h b/drivers/media/platform/raspberrypi/pisp_be/pisp_be_formats.h
index 0b9dcc7d52c5..51c8859723c2 100644
--- a/drivers/media/platform/raspberrypi/pisp_be/pisp_be_formats.h
+++ b/drivers/media/platform/raspberrypi/pisp_be/pisp_be_formats.h
@@ -27,6 +27,8 @@ struct pisp_be_format {
#define V4L2_COLORSPACE_MASK(colorspace) BIT(colorspace)
+#define V4L2_COLORSPACE_MASK_JPEG \
+ V4L2_COLORSPACE_MASK(V4L2_COLORSPACE_JPEG)
#define V4L2_COLORSPACE_MASK_SMPTE170M \
V4L2_COLORSPACE_MASK(V4L2_COLORSPACE_SMPTE170M)
#define V4L2_COLORSPACE_MASK_REC709 \
@@ -46,7 +48,8 @@ struct pisp_be_format {
* producing an RGB format. In turn this requires us to allow all these colour
* spaces for every YUV/RGB output format.
*/
-#define V4L2_COLORSPACE_MASK_ALL_SRGB (V4L2_COLORSPACE_MASK_SRGB | \
+#define V4L2_COLORSPACE_MASK_ALL_SRGB (V4L2_COLORSPACE_MASK_JPEG | \
+ V4L2_COLORSPACE_MASK_SRGB | \
V4L2_COLORSPACE_MASK_SMPTE170M | \
V4L2_COLORSPACE_MASK_REC709)
Without this some of our colour space tests fail. Things should be fine for the compliance errors, as our default colour spaces are not JPEG and still SMTPE170M.
Another fix that is needed is
diff --git a/drivers/media/platform/raspberrypi/pisp_be/pisp_be_formats.h b/drivers/media/platform/raspberrypi/pisp_be/pisp_be_formats.h index 0b9dcc7d52c5..51c8859723c2 100644 --- a/drivers/media/platform/raspberrypi/pisp_be/pisp_be_formats.h +++ b/drivers/media/platform/raspberrypi/pisp_be/pisp_be_formats.h @@ -27,6 +27,8 @@ struct pisp_be_format { #define V4L2_COLORSPACE_MASK(colorspace) BIT(colorspace) +#define V4L2_COLORSPACE_MASK_JPEG \ + V4L2_COLORSPACE_MASK(V4L2_COLORSPACE_JPEG) #define V4L2_COLORSPACE_MASK_SMPTE170M \ V4L2_COLORSPACE_MASK(V4L2_COLORSPACE_SMPTE170M) #define V4L2_COLORSPACE_MASK_REC709 \ @@ -46,7 +48,8 @@ struct pisp_be_format { * producing an RGB format. In turn this requires us to allow all these colour * spaces for every YUV/RGB output format. */ -#define V4L2_COLORSPACE_MASK_ALL_SRGB (V4L2_COLORSPACE_MASK_SRGB | \ +#define V4L2_COLORSPACE_MASK_ALL_SRGB (V4L2_COLORSPACE_MASK_JPEG | \ + V4L2_COLORSPACE_MASK_SRGB | \ V4L2_COLORSPACE_MASK_SMPTE170M | \ V4L2_COLORSPACE_MASK_REC709)Without this some of our colour space tests fail. Things should be fine for the compliance errors, as our default colour spaces are not JPEG and still SMTPE170M.
I'll take it in and re-run v4l2-compliance
With these 2 patches applied on top, all our tests are passing!
nice, thanks for testing and sorry for the + 4 which went unnoticed in my testing.
On top of which of your kernel branches would you like me to send a squashed version of these patches ?
One other thing, I'm working with @tomba to make sure the uAPI headers that I'll upstream for BE will be compatible with the ones used for his forthcoming version of the FE driver. The uAPI headers might change location (include/uapi/media/ vs the current include/uapi/linux/). Just an heads-up!
On top of which of your kernel branches would you like me to send a squashed version of these patches ?
I think make a PR against rpi-6.6.y. We will merge it to that and then backport to rpi-6.1.y manually.
I think make a PR against rpi-6.6.y. We will merge it to that and then backport to rpi-6.1.y manually.
We're not expecting to release any further 6.1 kernel updates to apt. so I don't think it's worth backporting. We should be moving apt to 6.6 kernel soon enough.
We're not expecting to release any further 6.1 kernel updates to apt. so I don't think it's worth backporting. We should be moving apt to 6.6 kernel soon enough.
That also works for me!