Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pispbe/rpi 6.6.y/backport mainline pispbe #6249

Conversation

jmondi
Copy link
Contributor

@jmondi jmondi commented Jul 2, 2024

This MR contains the PiSP BE mainline version as per
https://lore.kernel.org/linux-media/[email protected]/
https://lore.kernel.org/linux-media/[email protected]/

backported to rpi-6.6.y with re-introduction of multi-context support that has been dropped from the mainline kernel version.

I applied patches from both series series without squashing, in order to maintain the history as close as possible to mainline.

The MR introduces the include/uapi/linux/media/raspberrypi/pisp_be_config.h uAPI header which will now be the authoritative header for pisp_be. libpisp and libcamera will have to be ported to use this new header, and once I have published this MR I'll create one for the two projects referencing this one.

The combination of this MR, libpisp and libcamera has been tested on RPi5 with 2 cameras (imx219 and ov5647) with both the libcamera utilities (cam and qcam) and with rpicam-apps.

This are 2 qcam instances running concurrently with 2 cameras

2024-07-02-16-46-47-255

Jacopo Mondi added 14 commits July 2, 2024 17:10
The Y10P, Y12P and Y14P format variants are packed according to
the RAW10, RAW12 and RAW14 formats as defined by the MIPI CSI-2
specification. Document it.

Signed-off-by: Jacopo Mondi <[email protected]>
Reviewed-by: Laurent Pinchart <[email protected]>
Reviewed-by: Naushir Patuck <[email protected]>
Add BGR48 and RGB48 16-bit per component image formats.

Signed-off-by: Jacopo Mondi <[email protected]>
Reviewed-by: Kieran Bingham <[email protected]>
Reviewed-by: Naushir Patuck <[email protected]>
Add the Raspberry Pi PiSP Back End uAPI header.

The header defines the data type used to configure the PiSP Back End
ISP.

The detailed description of the types and of the ISP configuration
procedure is available at
https://datasheets.raspberrypi.com/camera/raspberry-pi-image-signal-processor-specification.pdf

Signed-off-by: Jacopo Mondi <[email protected]>
Add format description for the PiSP Back End configuration parameter
buffer.

Signed-off-by: Jacopo Mondi <[email protected]>
Reviewed-by: Naushir Patuck <[email protected]>
Document the Raspberry Pi compressed RAW Bayer formats.

The compression algorithm description is provided by Nick Hollinghurst
<[email protected]> from Raspberry Pi.

Signed-off-by: Jacopo Mondi <[email protected]>
Reviewed-by: Naushir Patuck <[email protected]>
Add bindings for the Raspberry Pi PiSP Back End memory-to-memory image
signal processor.

Datasheet:
https://datasheets.raspberrypi.com/camera/raspberry-pi-image-signal-processor-specification.pdf

Signed-off-by: Jacopo Mondi <[email protected]>
Reviewed-by: Rob Herring <[email protected]>
Reviewed-by: Naushir Patuck <[email protected]>
Add documentation for the PiSP Back End memory-to-memory ISP.

Signed-off-by: Jacopo Mondi <[email protected]>
Backport to rpi-6.6.y the mainline version of the PiSP BE driver.

The backported version of the driver corresponds to the one available
at:
https://lore.kernel.org/all/[email protected]/

Signed-off-by: Jacopo Mondi <[email protected]>
The pisp_be_config.h uAPI header file contains a bit-field definition
that uses the BIT() helper macro.

As the BIT() identifier is not defined in userspace, drop it from the
uAPI header.

Fixes: c6c49ba ("media: uapi: Add Raspberry Pi PiSP Back End uAPI")
Signed-off-by: Jacopo Mondi <[email protected]>
Reviewed-by: Tomi Valkeinen <[email protected]>
Add definition and test for 32-bits image formats to the pisp_common.h
uAPI header.

Signed-off-by: Jacopo Mondi <[email protected]>
The macro used to inspect an image format characteristic use a mixture
of capitalized and non-capitalized letters, which is rather unusual for
the Linux kernel style.

Capitalize all identifiers.

Signed-off-by: Jacopo Mondi <[email protected]>
The order of the members of pisp_be_tiles_config is relevant
as the driver logic assumes 'config' to be at offset 0.

Re-sort the member to match the driver's expectations.

Fixes: c6c49ba ("media: uapi: Add Raspberry Pi PiSP Back End uAPI")
Signed-off-by: Jacopo Mondi <[email protected]>
Complete the pisp_be_config strcture by adding fields that even if not
written to the HW are relevant to complete the uAPI and put it in par
with the BSP driver.

Fixes: c6c49ba ("media: uapi: Add Raspberry Pi PiSP Back End uAPI")
Signed-off-by: Jacopo Mondi <[email protected]>
Re-introduce multi-context support that was dropped from the
mainline driver version.

Signed-off-by: Jacopo Mondi <[email protected]>
@jmondi
Copy link
Contributor Author

jmondi commented Jul 2, 2024

@naushir
Copy link
Contributor

naushir commented Jul 4, 2024

All our tests look to be passing with this change. It also keeps backward compatibility with existing userland libraries (mostly, see below) which is great.

When we decide to break userland combability with the CFE merge, we ought to get rid of 23e1b4d and 7e13ef2 to match the upstream UAPI fully.

@naushir
Copy link
Contributor

naushir commented Jul 4, 2024

@jmondi one thing that does break is picamera2 because it assumes that the CFE dev nodes come before the backend dev nodes. We will fix this in picamera2, but for now, can we add a commit with the following diff:

diff --git a/drivers/media/platform/raspberrypi/pisp_be/pisp_be.c b/drivers/media/platform/raspberrypi/pisp_be/pisp_be.c
index f0365c7305cf..4dbff250cafb 100644
--- a/drivers/media/platform/raspberrypi/pisp_be/pisp_be.c
+++ b/drivers/media/platform/raspberrypi/pisp_be/pisp_be.c
@@ -21,6 +21,9 @@

 #include "pisp_be_formats.h"

+/* Offset to use when registering the /dev/videoX node */
+#define PISPBE_VIDEO_NODE_OFFSET 20
+
 /* Maximum number of config buffers possible */
 #define PISP_BE_NUM_CONFIG_BUFFERS VB2_MAX_FRAME

@@ -1484,7 +1487,8 @@ static int pispbe_init_node(struct pispbe_node_group *node_group,
                goto err_unregister_queue;
        }

-       ret = video_register_device(vdev, VFL_TYPE_VIDEO, -1);
+       ret = video_register_device(vdev, VFL_TYPE_VIDEO,
+                                   PISP_BE_VIDEO_NODE_OFFSET);

This can also be removed when we finally do break userland when the CFE lands.

@naushir
Copy link
Contributor

naushir commented Jul 8, 2024

@jmondi I'm keen on getting this merged soon. Are you ok with me adding the above change to the PR?
Anyone else have any objections to this or the rest of the patchset?

@jmondi
Copy link
Contributor Author

jmondi commented Jul 8, 2024

@jmondi I'm keen on getting this merged soon. Are you ok with me adding the above change to the PR? Anyone else have any objections to this or the rest of the patchset?

@naushir the PISP_BE_VIDEO_NODE_OFFSET one ? Sure, please go ahead with that patch on top!

@naushir
Copy link
Contributor

naushir commented Jul 8, 2024

@naushir the PISP_BE_VIDEO_NODE_OFFSET one ? Sure, please go ahead with that patch on top!

That's the one!

@naushir
Copy link
Contributor

naushir commented Jul 8, 2024

Closing this and opened a new PR at #6259 with the extra commit.

@naushir naushir closed this Jul 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants