[PATCH v1 2/2] libcamera: formats: Add PiSP specific image and config buffer formats
Jacopo Mondi
jacopo.mondi at ideasonboard.com
Mon Feb 26 14:49:09 CET 2024
Hi Naush
On Thu, Feb 15, 2024 at 01:27:10PM +0000, Naushir Patuck wrote:
> Add the Raspberry Pi 5 PiSP specific compressed Bayer format types 1/2:
> - V4L2_PIX_FMT_PISP_COMP1_xxx
> - V4L2_PIX_FMT_PISP_COMP2_xxx
>
> Add the Raspberry Pi 5 PiSP Backend config format:
> - V4L2_META_FMT_RPI_BE_CFG
>
> Additionally, we also extend libcamera format handlers to support 16-bit
> Bayer formats across the media bus.
>
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> ---
> include/libcamera/internal/bayer_format.h | 2 +
> include/linux/drm_fourcc.h | 4 ++
> include/linux/videodev2.h | 15 +++++++
> src/libcamera/bayer_format.cpp | 18 ++++++++
> src/libcamera/formats.cpp | 51 ++++++++++++++++++++++-
> src/libcamera/formats.yaml | 16 +++++++
> src/libcamera/v4l2_pixelformat.cpp | 10 +++++
> src/libcamera/v4l2_subdevice.cpp | 4 ++
> 8 files changed, 119 insertions(+), 1 deletion(-)
>
> diff --git a/include/libcamera/internal/bayer_format.h b/include/libcamera/internal/bayer_format.h
> index 78ba3969913d..164743f7e9f6 100644
> --- a/include/libcamera/internal/bayer_format.h
> +++ b/include/libcamera/internal/bayer_format.h
> @@ -34,6 +34,8 @@ public:
> None = 0,
> CSI2 = 1,
> IPU3 = 2,
> + PISP1 = 3,
> + PISP2 = 4,
> };
>
> constexpr BayerFormat()
> diff --git a/include/linux/drm_fourcc.h b/include/linux/drm_fourcc.h
> index 4ee421b95730..eff27fbe5a1e 100644
> --- a/include/linux/drm_fourcc.h
> +++ b/include/linux/drm_fourcc.h
> @@ -490,6 +490,7 @@ extern "C" {
> #define DRM_FORMAT_MOD_VENDOR_ALLWINNER 0x09
> #define DRM_FORMAT_MOD_VENDOR_AMLOGIC 0x0a
> #define DRM_FORMAT_MOD_VENDOR_MIPI 0x0b
> +#define DRM_FORMAT_MOD_VENDOR_RPI 0x0c
>
> /* add more to the end as needed */
>
> @@ -1670,6 +1671,9 @@ drm_fourcc_canonicalize_nvidia_format_mod(__u64 modifier)
> */
> #define MIPI_FORMAT_MOD_CSI2_PACKED fourcc_mod_code(MIPI, 1)
>
> +#define PISP_FORMAT_MOD_COMPRESS_MODE1 fourcc_mod_code(RPI, 1)
> +#define PISP_FORMAT_MOD_COMPRESS_MODE2 fourcc_mod_code(RPI, 2)
> +
I'll send a patch to DRM/KMS for these formats as well
> #if defined(__cplusplus)
> }
> #endif
> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
> index ba48d2c89726..59af6f794680 100644
> --- a/include/linux/videodev2.h
> +++ b/include/linux/videodev2.h
> @@ -789,6 +789,18 @@ struct v4l2_pix_format {
> #define V4L2_PIX_FMT_IPU3_SGRBG10 v4l2_fourcc('i', 'p', '3', 'G') /* IPU3 packed 10-bit GRBG bayer */
> #define V4L2_PIX_FMT_IPU3_SRGGB10 v4l2_fourcc('i', 'p', '3', 'r') /* IPU3 packed 10-bit RGGB bayer */
>
> +/* The pixel format for all our buffers (the precise format is found in the config buffer). */
> +#define V4L2_PIX_FMT_PISP_COMP1_RGGB v4l2_fourcc('P', 'C', '1', 'R')
> +#define V4L2_PIX_FMT_PISP_COMP1_GRBG v4l2_fourcc('P', 'C', '1', 'G')
> +#define V4L2_PIX_FMT_PISP_COMP1_GBRG v4l2_fourcc('P', 'C', '1', 'g')
> +#define V4L2_PIX_FMT_PISP_COMP1_BGGR v4l2_fourcc('P', 'C', '1', 'B')
> +#define V4L2_PIX_FMT_PISP_COMP1_MONO v4l2_fourcc('P', 'C', '1', 'M')
> +#define V4L2_PIX_FMT_PISP_COMP2_RGGB v4l2_fourcc('P', 'C', '2', 'R')
> +#define V4L2_PIX_FMT_PISP_COMP2_GRBG v4l2_fourcc('P', 'C', '2', 'G')
> +#define V4L2_PIX_FMT_PISP_COMP2_GBRG v4l2_fourcc('P', 'C', '2', 'g')
> +#define V4L2_PIX_FMT_PISP_COMP2_BGGR v4l2_fourcc('P', 'C', '2', 'B')
> +#define V4L2_PIX_FMT_PISP_COMP2_MONO v4l2_fourcc('P', 'C', '2', 'M')
> +
I would use here what has been sent upstream for the BE support
/* Raspberry Pi PiSP compressed formats. */
#define V4L2_PIX_FMT_PISP_COMP1_RGGB v4l2_fourcc('P', 'C', '1', 'R') /* PiSP 8-bit mode 1 compressed RGGB bayer */
#define V4L2_PIX_FMT_PISP_COMP1_GRBG v4l2_fourcc('P', 'C', '1', 'G') /* PiSP 8-bit mode 1 compressed GRBG bayer */
#define V4L2_PIX_FMT_PISP_COMP1_GBRG v4l2_fourcc('P', 'C', '1', 'g') /* PiSP 8-bit mode 1 compressed GBRG bayer */
#define V4L2_PIX_FMT_PISP_COMP1_BGGR v4l2_fourcc('P', 'C', '1', 'B') /* PiSP 8-bit mode 1 compressed BGGR bayer */
#define V4L2_PIX_FMT_PISP_COMP1_MONO v4l2_fourcc('P', 'C', '1', 'M') /* PiSP 8-bit mode 1 compressed monochrome */
#define V4L2_PIX_FMT_PISP_COMP2_RGGB v4l2_fourcc('P', 'C', '2', 'R') /* PiSP 8-bit mode 2 compressed RGGB bayer */
#define V4L2_PIX_FMT_PISP_COMP2_GRBG v4l2_fourcc('P', 'C', '2', 'G') /* PiSP 8-bit mode 2 compressed GRBG bayer */
#define V4L2_PIX_FMT_PISP_COMP2_GBRG v4l2_fourcc('P', 'C', '2', 'g') /* PiSP 8-bit mode 2 compressed GBRG bayer */
#define V4L2_PIX_FMT_PISP_COMP2_BGGR v4l2_fourcc('P', 'C', '2', 'B') /* PiSP 8-bit mode 2 compressed BGGR bayer */
#define V4L2_PIX_FMT_PISP_COMP2_MONO v4l2_fourcc('P', 'C', '2', 'M') /* PiSP 8-bit mode 2 compressed monochrome */
To make it easier to update it when these will land in upstream
> /* SDR formats - used only for Software Defined Radio devices */
> #define V4L2_SDR_FMT_CU8 v4l2_fourcc('C', 'U', '0', '8') /* IQ u8 */
> #define V4L2_SDR_FMT_CU16LE v4l2_fourcc('C', 'U', '1', '6') /* IQ u16le */
> @@ -818,6 +830,9 @@ struct v4l2_pix_format {
> #define V4L2_META_FMT_RK_ISP1_PARAMS v4l2_fourcc('R', 'K', '1', 'P') /* Rockchip ISP1 3A Parameters */
> #define V4L2_META_FMT_RK_ISP1_STAT_3A v4l2_fourcc('R', 'K', '1', 'S') /* Rockchip ISP1 3A Statistics */
>
> +/* Vendor specific - used for RaspberryPi PiSP */
> +#define V4L2_META_FMT_RPI_BE_CFG v4l2_fourcc('R', 'P', 'B', 'C') /* PiSP BE configuration */
> /* priv field value to indicates that subsequent fields are valid. */
> #define V4L2_PIX_FMT_PRIV_MAGIC 0xfeedcafe
>
> diff --git a/src/libcamera/bayer_format.cpp b/src/libcamera/bayer_format.cpp
> index 20aedfa6d925..333b1117f531 100644
> --- a/src/libcamera/bayer_format.cpp
> +++ b/src/libcamera/bayer_format.cpp
> @@ -164,6 +164,14 @@ const std::map<BayerFormat, Formats, BayerFormatComparator> bayerToFormat{
> { formats::SGRBG16, V4L2PixelFormat(V4L2_PIX_FMT_SGRBG16) } },
> { { BayerFormat::RGGB, 16, BayerFormat::Packing::None },
> { formats::SRGGB16, V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16) } },
> + { { BayerFormat::BGGR, 16, BayerFormat::Packing::PISP1 },
> + { formats::BGGR16_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_BGGR) } },
> + { { BayerFormat::GBRG, 16, BayerFormat::Packing::PISP1 },
> + { formats::GBRG16_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GBRG) } },
> + { { BayerFormat::GRBG, 16, BayerFormat::Packing::PISP1 },
> + { formats::GRBG16_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GRBG) } },
> + { { BayerFormat::RGGB, 16, BayerFormat::Packing::PISP1 },
> + { formats::RGGB16_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_RGGB) } },
> { { BayerFormat::MONO, 8, BayerFormat::Packing::None },
> { formats::R8, V4L2PixelFormat(V4L2_PIX_FMT_GREY) } },
> { { BayerFormat::MONO, 10, BayerFormat::Packing::None },
> @@ -174,6 +182,8 @@ const std::map<BayerFormat, Formats, BayerFormatComparator> bayerToFormat{
> { formats::R12, V4L2PixelFormat(V4L2_PIX_FMT_Y12) } },
> { { BayerFormat::MONO, 16, BayerFormat::Packing::None },
> { formats::R16, V4L2PixelFormat(V4L2_PIX_FMT_Y16) } },
> + { { BayerFormat::MONO, 16, BayerFormat::Packing::PISP1 },
> + { formats::MONO_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_MONO) } },
Should the COMP2 versions be added as well ?
> };
>
> const std::unordered_map<unsigned int, BayerFormat> mbusCodeToBayer{
> @@ -209,6 +219,10 @@ const std::unordered_map<unsigned int, BayerFormat> mbusCodeToBayer{
> { MEDIA_BUS_FMT_SGBRG16_1X16, { BayerFormat::GBRG, 16, BayerFormat::Packing::None } },
> { MEDIA_BUS_FMT_SGRBG16_1X16, { BayerFormat::GRBG, 16, BayerFormat::Packing::None } },
> { MEDIA_BUS_FMT_SRGGB16_1X16, { BayerFormat::RGGB, 16, BayerFormat::Packing::None } },
> + { MEDIA_BUS_FMT_SBGGR16_1X16, { BayerFormat::BGGR, 16, BayerFormat::Packing::PISP1 } },
> + { MEDIA_BUS_FMT_SGBRG16_1X16, { BayerFormat::GBRG, 16, BayerFormat::Packing::PISP1 } },
> + { MEDIA_BUS_FMT_SGRBG16_1X16, { BayerFormat::GRBG, 16, BayerFormat::Packing::PISP1 } },
> + { MEDIA_BUS_FMT_SRGGB16_1X16, { BayerFormat::RGGB, 16, BayerFormat::Packing::PISP1 } },
mbusCodeToBayer is an unordered_map which
"is an associative container that contains key-value pairs with unique
keys."
I'm afraid adding a new entry with the same key overwrite the existing
one ?
We already had issues with this map, as we had similar problems with the
_LE and _BE version of RGB565 which resolv to the same BayerFormat
{ MEDIA_BUS_FMT_SBGGR10_2X8_PADHI_BE, { BayerFormat::BGGR, 10, BayerFormat::Packing::None } },
{ MEDIA_BUS_FMT_SBGGR10_2X8_PADHI_LE, { BayerFormat::BGGR, 10, BayerFormat::Packing::None } },
{ MEDIA_BUS_FMT_SBGGR10_2X8_PADLO_BE, { BayerFormat::BGGR, 10, BayerFormat::Packing::None } },
{ MEDIA_BUS_FMT_SBGGR10_2X8_PADLO_LE, { BayerFormat::BGGR, 10, BayerFormat::Packing::None } },
In general, we know that resolving mbus_codes to a unique format is
ill-defined. How are you using these entries in the pi5 support ?
> { MEDIA_BUS_FMT_Y8_1X8, { BayerFormat::MONO, 8, BayerFormat::Packing::None } },
> { MEDIA_BUS_FMT_Y10_1X10, { BayerFormat::MONO, 10, BayerFormat::Packing::None } },
> { MEDIA_BUS_FMT_Y12_1X12, { BayerFormat::MONO, 12, BayerFormat::Packing::None } },
> @@ -303,6 +317,10 @@ std::ostream &operator<<(std::ostream &out, const BayerFormat &f)
> out << "-CSI2P";
> else if (f.packing == BayerFormat::Packing::IPU3)
> out << "-IPU3P";
> + else if (f.packing == BayerFormat::Packing::PISP1)
> + out << "-PISP1";
> + else if (f.packing == BayerFormat::Packing::PISP2)
> + out << "-PISP2";
>
> return out;
> }
> diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp
> index a674f4179cc8..e603a7eda579 100644
> --- a/src/libcamera/formats.cpp
> +++ b/src/libcamera/formats.cpp
> @@ -547,6 +547,16 @@ const std::map<PixelFormat, PixelFormatInfo> pixelFormatInfo{
> .pixelsPerGroup = 1,
> .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},
> } },
> + { formats::MONO_PISP_COMP1, {
> + .name = "MONO_PISP_COMP1",
> + .format = formats::MONO_PISP_COMP1,
> + .v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_MONO), },
> + .bitsPerPixel = 16,
> + .colourEncoding = PixelFormatInfo::ColourEncodingYUV,
> + .packed = true,
> + .pixelsPerGroup = 1,
> + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},
> + } },
>
> /* Bayer formats. */
> { formats::SBGGR8, {
> @@ -910,7 +920,46 @@ const std::map<PixelFormat, PixelFormatInfo> pixelFormatInfo{
> .pixelsPerGroup = 25,
> .planes = {{ { 32, 1 }, { 0, 0 }, { 0, 0 } }},
> } },
> -
> + { formats::BGGR16_PISP_COMP1, {
> + .name = "BGGR16_PISP_COMP1",
> + .format = formats::BGGR16_PISP_COMP1,
> + .v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_BGGR), },
> + .bitsPerPixel = 16,
> + .colourEncoding = PixelFormatInfo::ColourEncodingRAW,
> + .packed = true,
> + .pixelsPerGroup = 2,
> + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},
> + } },
For regular 16-bit formats this should be
.pixelsPerGroup = 2,
.planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},
As 2 pixels of 16 bits each consume 4 bytes. Is this {2, 1}
intentional and due to compression ?
> + { formats::GBRG16_PISP_COMP1, {
> + .name = "GBRG16_PISP_COMP1",
> + .format = formats::GBRG16_PISP_COMP1,
> + .v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GBRG), },
> + .bitsPerPixel = 16,
> + .colourEncoding = PixelFormatInfo::ColourEncodingRAW,
> + .packed = true,
> + .pixelsPerGroup = 2,
> + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},
> + } },
> + { formats::GRBG16_PISP_COMP1, {
> + .name = "GRBG16_PISP_COMP1",
> + .format = formats::GRBG16_PISP_COMP1,
> + .v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GRBG), },
> + .bitsPerPixel = 16,
> + .colourEncoding = PixelFormatInfo::ColourEncodingRAW,
> + .packed = true,
> + .pixelsPerGroup = 2,
> + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},
> + } },
> + { formats::RGGB16_PISP_COMP1, {
> + .name = "RGGB16_PISP_COMP1",
> + .format = formats::RGGB16_PISP_COMP1,
> + .v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_RGGB), },
> + .bitsPerPixel = 16,
> + .colourEncoding = PixelFormatInfo::ColourEncodingRAW,
> + .packed = true,
> + .pixelsPerGroup = 2,
> + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},
> + } },
> /* Compressed formats. */
> { formats::MJPEG, {
> .name = "MJPEG",
> diff --git a/src/libcamera/formats.yaml b/src/libcamera/formats.yaml
> index bde2cc803b98..f6df721243d0 100644
> --- a/src/libcamera/formats.yaml
> +++ b/src/libcamera/formats.yaml
> @@ -190,4 +190,20 @@ formats:
> - SBGGR10_IPU3:
> fourcc: DRM_FORMAT_SBGGR10
> mod: IPU3_FORMAT_MOD_PACKED
> +
> + - RGGB16_PISP_COMP1:
> + fourcc: DRM_FORMAT_SRGGB16
> + mod: PISP_FORMAT_MOD_COMPRESS_MODE1
> + - GRBG16_PISP_COMP1:
> + fourcc: DRM_FORMAT_SGRBG16
> + mod: PISP_FORMAT_MOD_COMPRESS_MODE1
> + - GBRG16_PISP_COMP1:
> + fourcc: DRM_FORMAT_SGBRG16
> + mod: PISP_FORMAT_MOD_COMPRESS_MODE1
> + - BGGR16_PISP_COMP1:
> + fourcc: DRM_FORMAT_SBGGR16
> + mod: PISP_FORMAT_MOD_COMPRESS_MODE1
> + - MONO_PISP_COMP1:
> + fourcc: DRM_FORMAT_R16
> + mod: PISP_FORMAT_MOD_COMPRESS_MODE1
> ...
> diff --git a/src/libcamera/v4l2_pixelformat.cpp b/src/libcamera/v4l2_pixelformat.cpp
> index efb6f2940235..47baaf60199d 100644
> --- a/src/libcamera/v4l2_pixelformat.cpp
> +++ b/src/libcamera/v4l2_pixelformat.cpp
> @@ -207,6 +207,16 @@ const std::map<V4L2PixelFormat, V4L2PixelFormat::Info> vpf2pf{
> { formats::SGRBG16, "16-bit Bayer GRGR/BGBG" } },
> { V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16),
> { formats::SRGGB16, "16-bit Bayer RGRG/GBGB" } },
> + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_BGGR),
> + { formats::BGGR16_PISP_COMP1, "16-bit Bayer BGBG/GRGR PiSP Compress Mode 1" } },
> + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GBRG),
> + { formats::GBRG16_PISP_COMP1, "16-bit Bayer GBGB/RGRG PiSP Compress Mode 1" } },
> + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GRBG),
> + { formats::GRBG16_PISP_COMP1, "16-bit Bayer GRGR/BGBG PiSP Compress Mode 1" } },
> + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_RGGB),
> + { formats::RGGB16_PISP_COMP1, "16-bit Bayer RGRG/GBGB PiSP Compress Mode 1" } },
> + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_MONO),
> + { formats::MONO_PISP_COMP1, "16-bit Mono PiSP Compress Mode 1" } },
>
> /* Compressed formats. */
> { V4L2PixelFormat(V4L2_PIX_FMT_MJPEG),
> diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp
> index 6d0785b7b484..aea90abaf9ef 100644
> --- a/src/libcamera/v4l2_subdevice.cpp
> +++ b/src/libcamera/v4l2_subdevice.cpp
> @@ -134,6 +134,10 @@ const std::map<uint32_t, V4L2SubdeviceFormatInfo> formatInfoMap = {
> { MEDIA_BUS_FMT_SGBRG12_1X12, { 12, "SGBRG12_1X12", PixelFormatInfo::ColourEncodingRAW } },
> { MEDIA_BUS_FMT_SGRBG12_1X12, { 12, "SGRBG12_1X12", PixelFormatInfo::ColourEncodingRAW } },
> { MEDIA_BUS_FMT_SRGGB12_1X12, { 12, "SRGGB12_1X12", PixelFormatInfo::ColourEncodingRAW } },
> + { MEDIA_BUS_FMT_SBGGR16_1X16, { 16, "SBGGR16_1x16", PixelFormatInfo::ColourEncodingRAW } },
> + { MEDIA_BUS_FMT_SGBRG16_1X16, { 16, "SGBRG16_1x16", PixelFormatInfo::ColourEncodingRAW } },
> + { MEDIA_BUS_FMT_SGRBG16_1X16, { 16, "SGRBG16_1x16", PixelFormatInfo::ColourEncodingRAW } },
> + { MEDIA_BUS_FMT_SRGGB16_1X16, { 16, "SRGGB16_1x16", PixelFormatInfo::ColourEncodingRAW } },
> /* \todo Clarify colour encoding for HSV formats */
> { MEDIA_BUS_FMT_AHSV8888_1X32, { 32, "AHSV8888_1X32", PixelFormatInfo::ColourEncodingRGB } },
> { MEDIA_BUS_FMT_JPEG_1X8, { 8, "JPEG_1X8", PixelFormatInfo::ColourEncodingYUV } },
> --
> 2.34.1
>
More information about the libcamera-devel
mailing list