[{"id":28733,"web_url":"https://patchwork.libcamera.org/comment/28733/","msgid":"<74nstgdru2jpkry22imfmrcelvvzkfektet6cdo72gl5hqgz6t@vr6q6rtvti27>","date":"2024-02-26T13:49:09","subject":"Re: [PATCH v1 2/2] libcamera: formats: Add PiSP specific image and\n\tconfig buffer formats","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Naush\n\nOn Thu, Feb 15, 2024 at 01:27:10PM +0000, Naushir Patuck wrote:\n> Add the Raspberry Pi 5 PiSP specific compressed Bayer format types 1/2:\n> - V4L2_PIX_FMT_PISP_COMP1_xxx\n> - V4L2_PIX_FMT_PISP_COMP2_xxx\n>\n> Add the Raspberry Pi 5 PiSP Backend config format:\n> - V4L2_META_FMT_RPI_BE_CFG\n>\n> Additionally, we also extend libcamera format handlers to support 16-bit\n> Bayer formats across the media bus.\n>\n> Signed-off-by: Naushir Patuck \n> ---\n> include/libcamera/internal/bayer_format.h | 2 +\n> include/linux/drm_fourcc.h | 4 ++\n> include/linux/videodev2.h | 15 +++++++\n> src/libcamera/bayer_format.cpp | 18 ++++++++\n> src/libcamera/formats.cpp | 51 ++++++++++++++++++++++-\n> src/libcamera/formats.yaml | 16 +++++++\n> src/libcamera/v4l2_pixelformat.cpp | 10 +++++\n> src/libcamera/v4l2_subdevice.cpp | 4 ++\n> 8 files changed, 119 insertions(+), 1 deletion(-)\n>\n> diff --git a/include/libcamera/internal/bayer_format.h b/include/libcamera/internal/bayer_format.h\n> index 78ba3969913d..164743f7e9f6 100644\n> --- a/include/libcamera/internal/bayer_format.h\n> +++ b/include/libcamera/internal/bayer_format.h\n> @@ -34,6 +34,8 @@ public:\n> \t\tNone = 0,\n> \t\tCSI2 = 1,\n> \t\tIPU3 = 2,\n> +\t\tPISP1 = 3,\n> +\t\tPISP2 = 4,\n> \t};\n>\n> \tconstexpr BayerFormat()\n> diff --git a/include/linux/drm_fourcc.h b/include/linux/drm_fourcc.h\n> index 4ee421b95730..eff27fbe5a1e 100644\n> --- a/include/linux/drm_fourcc.h\n> +++ b/include/linux/drm_fourcc.h\n> @@ -490,6 +490,7 @@ extern \"C\" {\n> #define DRM_FORMAT_MOD_VENDOR_ALLWINNER 0x09\n> #define DRM_FORMAT_MOD_VENDOR_AMLOGIC 0x0a\n> #define DRM_FORMAT_MOD_VENDOR_MIPI 0x0b\n> +#define DRM_FORMAT_MOD_VENDOR_RPI 0x0c\n>\n> /* add more to the end as needed */\n>\n> @@ -1670,6 +1671,9 @@ drm_fourcc_canonicalize_nvidia_format_mod(__u64 modifier)\n> */\n> #define MIPI_FORMAT_MOD_CSI2_PACKED fourcc_mod_code(MIPI, 1)\n>\n> +#define PISP_FORMAT_MOD_COMPRESS_MODE1 fourcc_mod_code(RPI, 1)\n> +#define PISP_FORMAT_MOD_COMPRESS_MODE2 fourcc_mod_code(RPI, 2)\n> +\n\nI'll send a patch to DRM/KMS for these formats as well\n\n> #if defined(__cplusplus)\n> }\n> #endif\n> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h\n> index ba48d2c89726..59af6f794680 100644\n> --- a/include/linux/videodev2.h\n> +++ b/include/linux/videodev2.h\n> @@ -789,6 +789,18 @@ struct v4l2_pix_format {\n> #define V4L2_PIX_FMT_IPU3_SGRBG10\tv4l2_fourcc('i', 'p', '3', 'G') /* IPU3 packed 10-bit GRBG bayer */\n> #define V4L2_PIX_FMT_IPU3_SRGGB10\tv4l2_fourcc('i', 'p', '3', 'r') /* IPU3 packed 10-bit RGGB bayer */\n>\n> +/* The pixel format for all our buffers (the precise format is found in the config buffer). */\n> +#define V4L2_PIX_FMT_PISP_COMP1_RGGB\tv4l2_fourcc('P', 'C', '1', 'R')\n> +#define V4L2_PIX_FMT_PISP_COMP1_GRBG\tv4l2_fourcc('P', 'C', '1', 'G')\n> +#define V4L2_PIX_FMT_PISP_COMP1_GBRG\tv4l2_fourcc('P', 'C', '1', 'g')\n> +#define V4L2_PIX_FMT_PISP_COMP1_BGGR\tv4l2_fourcc('P', 'C', '1', 'B')\n> +#define V4L2_PIX_FMT_PISP_COMP1_MONO\tv4l2_fourcc('P', 'C', '1', 'M')\n> +#define V4L2_PIX_FMT_PISP_COMP2_RGGB\tv4l2_fourcc('P', 'C', '2', 'R')\n> +#define V4L2_PIX_FMT_PISP_COMP2_GRBG\tv4l2_fourcc('P', 'C', '2', 'G')\n> +#define V4L2_PIX_FMT_PISP_COMP2_GBRG\tv4l2_fourcc('P', 'C', '2', 'g')\n> +#define V4L2_PIX_FMT_PISP_COMP2_BGGR\tv4l2_fourcc('P', 'C', '2', 'B')\n> +#define V4L2_PIX_FMT_PISP_COMP2_MONO\tv4l2_fourcc('P', 'C', '2', 'M')\n> +\n\nI would use here what has been sent upstream for the BE support\n\n/* Raspberry Pi PiSP compressed formats. */\n#define V4L2_PIX_FMT_PISP_COMP1_RGGB\tv4l2_fourcc('P', 'C', '1', 'R') /* PiSP 8-bit mode 1 compressed RGGB bayer */\n#define V4L2_PIX_FMT_PISP_COMP1_GRBG\tv4l2_fourcc('P', 'C', '1', 'G') /* PiSP 8-bit mode 1 compressed GRBG bayer */\n#define V4L2_PIX_FMT_PISP_COMP1_GBRG\tv4l2_fourcc('P', 'C', '1', 'g') /* PiSP 8-bit mode 1 compressed GBRG bayer */\n#define V4L2_PIX_FMT_PISP_COMP1_BGGR\tv4l2_fourcc('P', 'C', '1', 'B') /* PiSP 8-bit mode 1 compressed BGGR bayer */\n#define V4L2_PIX_FMT_PISP_COMP1_MONO\tv4l2_fourcc('P', 'C', '1', 'M') /* PiSP 8-bit mode 1 compressed monochrome */\n#define V4L2_PIX_FMT_PISP_COMP2_RGGB\tv4l2_fourcc('P', 'C', '2', 'R') /* PiSP 8-bit mode 2 compressed RGGB bayer */\n#define V4L2_PIX_FMT_PISP_COMP2_GRBG\tv4l2_fourcc('P', 'C', '2', 'G') /* PiSP 8-bit mode 2 compressed GRBG bayer */\n#define V4L2_PIX_FMT_PISP_COMP2_GBRG\tv4l2_fourcc('P', 'C', '2', 'g') /* PiSP 8-bit mode 2 compressed GBRG bayer */\n#define V4L2_PIX_FMT_PISP_COMP2_BGGR\tv4l2_fourcc('P', 'C', '2', 'B') /* PiSP 8-bit mode 2 compressed BGGR bayer */\n#define V4L2_PIX_FMT_PISP_COMP2_MONO\tv4l2_fourcc('P', 'C', '2', 'M') /* PiSP 8-bit mode 2 compressed monochrome */\n\nTo make it easier to update it when these will land in upstream\n\n> /* SDR formats - used only for Software Defined Radio devices */\n> #define V4L2_SDR_FMT_CU8 v4l2_fourcc('C', 'U', '0', '8') /* IQ u8 */\n> #define V4L2_SDR_FMT_CU16LE v4l2_fourcc('C', 'U', '1', '6') /* IQ u16le */\n> @@ -818,6 +830,9 @@ struct v4l2_pix_format {\n> #define V4L2_META_FMT_RK_ISP1_PARAMS\tv4l2_fourcc('R', 'K', '1', 'P') /* Rockchip ISP1 3A Parameters */\n> #define V4L2_META_FMT_RK_ISP1_STAT_3A\tv4l2_fourcc('R', 'K', '1', 'S') /* Rockchip ISP1 3A Statistics */\n>\n> +/* Vendor specific - used for RaspberryPi PiSP */\n> +#define V4L2_META_FMT_RPI_BE_CFG v4l2_fourcc('R', 'P', 'B', 'C') /* PiSP BE configuration */\n> /* priv field value to indicates that subsequent fields are valid. */\n> #define V4L2_PIX_FMT_PRIV_MAGIC\t\t0xfeedcafe\n>\n> diff --git a/src/libcamera/bayer_format.cpp b/src/libcamera/bayer_format.cpp\n> index 20aedfa6d925..333b1117f531 100644\n> --- a/src/libcamera/bayer_format.cpp\n> +++ b/src/libcamera/bayer_format.cpp\n> @@ -164,6 +164,14 @@ const std::map bayerToFormat{\n> \t\t{ formats::SGRBG16, V4L2PixelFormat(V4L2_PIX_FMT_SGRBG16) } },\n> \t{ { BayerFormat::RGGB, 16, BayerFormat::Packing::None },\n> \t\t{ formats::SRGGB16, V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16) } },\n> +\t{ { BayerFormat::BGGR, 16, BayerFormat::Packing::PISP1 },\n> +\t\t{ formats::BGGR16_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_BGGR) } },\n> +\t{ { BayerFormat::GBRG, 16, BayerFormat::Packing::PISP1 },\n> +\t\t{ formats::GBRG16_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GBRG) } },\n> +\t{ { BayerFormat::GRBG, 16, BayerFormat::Packing::PISP1 },\n> +\t\t{ formats::GRBG16_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GRBG) } },\n> +\t{ { BayerFormat::RGGB, 16, BayerFormat::Packing::PISP1 },\n> +\t\t{ formats::RGGB16_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_RGGB) } },\n> \t{ { BayerFormat::MONO, 8, BayerFormat::Packing::None },\n> \t\t{ formats::R8, V4L2PixelFormat(V4L2_PIX_FMT_GREY) } },\n> \t{ { BayerFormat::MONO, 10, BayerFormat::Packing::None },\n> @@ -174,6 +182,8 @@ const std::map bayerToFormat{\n> \t\t{ formats::R12, V4L2PixelFormat(V4L2_PIX_FMT_Y12) } },\n> \t{ { BayerFormat::MONO, 16, BayerFormat::Packing::None },\n> \t\t{ formats::R16, V4L2PixelFormat(V4L2_PIX_FMT_Y16) } },\n> +\t{ { BayerFormat::MONO, 16, BayerFormat::Packing::PISP1 },\n> +\t\t{ formats::MONO_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_MONO) } },\n\nShould the COMP2 versions be added as well ?\n\n> };\n>\n> const std::unordered_map mbusCodeToBayer{\n> @@ -209,6 +219,10 @@ const std::unordered_map mbusCodeToBayer{\n> \t{ MEDIA_BUS_FMT_SGBRG16_1X16, { BayerFormat::GBRG, 16, BayerFormat::Packing::None } },\n> \t{ MEDIA_BUS_FMT_SGRBG16_1X16, { BayerFormat::GRBG, 16, BayerFormat::Packing::None } },\n> \t{ MEDIA_BUS_FMT_SRGGB16_1X16, { BayerFormat::RGGB, 16, BayerFormat::Packing::None } },\n> +\t{ MEDIA_BUS_FMT_SBGGR16_1X16, { BayerFormat::BGGR, 16, BayerFormat::Packing::PISP1 } },\n> +\t{ MEDIA_BUS_FMT_SGBRG16_1X16, { BayerFormat::GBRG, 16, BayerFormat::Packing::PISP1 } },\n> +\t{ MEDIA_BUS_FMT_SGRBG16_1X16, { BayerFormat::GRBG, 16, BayerFormat::Packing::PISP1 } },\n> +\t{ MEDIA_BUS_FMT_SRGGB16_1X16, { BayerFormat::RGGB, 16, BayerFormat::Packing::PISP1 } },\n\nmbusCodeToBayer is an unordered_map which\n\"is an associative container that contains key-value pairs with unique\nkeys.\"\n\nI'm afraid adding a new entry with the same key overwrite the existing\none ?\n\nWe already had issues with this map, as we had similar problems with the\n_LE and _BE version of RGB565 which resolv to the same BayerFormat\n\n\t{ MEDIA_BUS_FMT_SBGGR10_2X8_PADHI_BE, { BayerFormat::BGGR, 10, BayerFormat::Packing::None } },\n\t{ MEDIA_BUS_FMT_SBGGR10_2X8_PADHI_LE, { BayerFormat::BGGR, 10, BayerFormat::Packing::None } },\n\t{ MEDIA_BUS_FMT_SBGGR10_2X8_PADLO_BE, { BayerFormat::BGGR, 10, BayerFormat::Packing::None } },\n\t{ MEDIA_BUS_FMT_SBGGR10_2X8_PADLO_LE, { BayerFormat::BGGR, 10, BayerFormat::Packing::None } },\n\nIn general, we know that resolving mbus_codes to a unique format is\nill-defined. How are you using these entries in the pi5 support ?\n\n> \t{ MEDIA_BUS_FMT_Y8_1X8, { BayerFormat::MONO, 8, BayerFormat::Packing::None } },\n> \t{ MEDIA_BUS_FMT_Y10_1X10, { BayerFormat::MONO, 10, BayerFormat::Packing::None } },\n> \t{ MEDIA_BUS_FMT_Y12_1X12, { BayerFormat::MONO, 12, BayerFormat::Packing::None } },\n> @@ -303,6 +317,10 @@ std::ostream &operator<<(std::ostream &out, const BayerFormat &f)\n> \t\tout << \"-CSI2P\";\n> \telse if (f.packing == BayerFormat::Packing::IPU3)\n> \t\tout << \"-IPU3P\";\n> +\telse if (f.packing == BayerFormat::Packing::PISP1)\n> +\t\tout << \"-PISP1\";\n> +\telse if (f.packing == BayerFormat::Packing::PISP2)\n> +\t\tout << \"-PISP2\";\n>\n> \treturn out;\n> }\n> diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp\n> index a674f4179cc8..e603a7eda579 100644\n> --- a/src/libcamera/formats.cpp\n> +++ b/src/libcamera/formats.cpp\n> @@ -547,6 +547,16 @@ const std::map pixelFormatInfo{\n> \t\t.pixelsPerGroup = 1,\n> \t\t.planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n> +\t{ formats::MONO_PISP_COMP1, {\n> +\t\t.name = \"MONO_PISP_COMP1\",\n> +\t\t.format = formats::MONO_PISP_COMP1,\n> +\t\t.v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_MONO), },\n> +\t\t.bitsPerPixel = 16,\n> +\t\t.colourEncoding = PixelFormatInfo::ColourEncodingYUV,\n> +\t\t.packed = true,\n> +\t\t.pixelsPerGroup = 1,\n> +\t\t.planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},\n> +\t} },\n>\n> \t/* Bayer formats. */\n> \t{ formats::SBGGR8, {\n> @@ -910,7 +920,46 @@ const std::map pixelFormatInfo{\n> \t\t.pixelsPerGroup = 25,\n> \t\t.planes = {{ { 32, 1 }, { 0, 0 }, { 0, 0 } }},\n> \t} },\n> -\n> +\t{ formats::BGGR16_PISP_COMP1, {\n> +\t\t.name = \"BGGR16_PISP_COMP1\",\n> +\t\t.format = formats::BGGR16_PISP_COMP1,\n> +\t\t.v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_BGGR), },\n> +\t\t.bitsPerPixel = 16,\n> +\t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> +\t\t.packed = true,\n> +\t\t.pixelsPerGroup = 2,\n> +\t\t.planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},\n> +\t} },\n\nFor regular 16-bit formats this should be\n\n\t\t.pixelsPerGroup = 2,\n\t\t.planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},\n\nAs 2 pixels of 16 bits each consume 4 bytes. Is this {2, 1}\nintentional and due to compression ?\n\n> +\t{ formats::GBRG16_PISP_COMP1, {\n> +\t\t.name = \"GBRG16_PISP_COMP1\",\n> +\t\t.format = formats::GBRG16_PISP_COMP1,\n> +\t\t.v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GBRG), },\n> +\t\t.bitsPerPixel = 16,\n> +\t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> +\t\t.packed = true,\n> +\t\t.pixelsPerGroup = 2,\n> +\t\t.planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},\n> +\t} },\n> +\t{ formats::GRBG16_PISP_COMP1, {\n> +\t\t.name = \"GRBG16_PISP_COMP1\",\n> +\t\t.format = formats::GRBG16_PISP_COMP1,\n> +\t\t.v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GRBG), },\n> +\t\t.bitsPerPixel = 16,\n> +\t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> +\t\t.packed = true,\n> +\t\t.pixelsPerGroup = 2,\n> +\t\t.planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},\n> +\t} },\n> +\t{ formats::RGGB16_PISP_COMP1, {\n> +\t\t.name = \"RGGB16_PISP_COMP1\",\n> +\t\t.format = formats::RGGB16_PISP_COMP1,\n> +\t\t.v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_RGGB), },\n> +\t\t.bitsPerPixel = 16,\n> +\t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> +\t\t.packed = true,\n> +\t\t.pixelsPerGroup = 2,\n> +\t\t.planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},\n> +\t} },\n> \t/* Compressed formats. */\n> \t{ formats::MJPEG, {\n> \t\t.name = \"MJPEG\",\n> diff --git a/src/libcamera/formats.yaml b/src/libcamera/formats.yaml\n> index bde2cc803b98..f6df721243d0 100644\n> --- a/src/libcamera/formats.yaml\n> +++ b/src/libcamera/formats.yaml\n> @@ -190,4 +190,20 @@ formats:\n> - SBGGR10_IPU3:\n> fourcc: DRM_FORMAT_SBGGR10\n> mod: IPU3_FORMAT_MOD_PACKED\n> +\n> + - RGGB16_PISP_COMP1:\n> + fourcc: DRM_FORMAT_SRGGB16\n> + mod: PISP_FORMAT_MOD_COMPRESS_MODE1\n> + - GRBG16_PISP_COMP1:\n> + fourcc: DRM_FORMAT_SGRBG16\n> + mod: PISP_FORMAT_MOD_COMPRESS_MODE1\n> + - GBRG16_PISP_COMP1:\n> + fourcc: DRM_FORMAT_SGBRG16\n> + mod: PISP_FORMAT_MOD_COMPRESS_MODE1\n> + - BGGR16_PISP_COMP1:\n> + fourcc: DRM_FORMAT_SBGGR16\n> + mod: PISP_FORMAT_MOD_COMPRESS_MODE1\n> + - MONO_PISP_COMP1:\n> + fourcc: DRM_FORMAT_R16\n> + mod: PISP_FORMAT_MOD_COMPRESS_MODE1\n> ...\n> diff --git a/src/libcamera/v4l2_pixelformat.cpp b/src/libcamera/v4l2_pixelformat.cpp\n> index efb6f2940235..47baaf60199d 100644\n> --- a/src/libcamera/v4l2_pixelformat.cpp\n> +++ b/src/libcamera/v4l2_pixelformat.cpp\n> @@ -207,6 +207,16 @@ const std::map vpf2pf{\n> \t\t{ formats::SGRBG16, \"16-bit Bayer GRGR/BGBG\" } },\n> \t{ V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16),\n> \t\t{ formats::SRGGB16, \"16-bit Bayer RGRG/GBGB\" } },\n> +\t{ V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_BGGR),\n> +\t\t{ formats::BGGR16_PISP_COMP1, \"16-bit Bayer BGBG/GRGR PiSP Compress Mode 1\" } },\n> +\t{ V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GBRG),\n> +\t\t{ formats::GBRG16_PISP_COMP1, \"16-bit Bayer GBGB/RGRG PiSP Compress Mode 1\" } },\n> +\t{ V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GRBG),\n> +\t\t{ formats::GRBG16_PISP_COMP1, \"16-bit Bayer GRGR/BGBG PiSP Compress Mode 1\" } },\n> +\t{ V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_RGGB),\n> +\t\t{ formats::RGGB16_PISP_COMP1, \"16-bit Bayer RGRG/GBGB PiSP Compress Mode 1\" } },\n> +\t{ V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_MONO),\n> +\t\t{ formats::MONO_PISP_COMP1, \"16-bit Mono PiSP Compress Mode 1\" } },\n>\n> \t/* Compressed formats. */\n> \t{ V4L2PixelFormat(V4L2_PIX_FMT_MJPEG),\n> diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp\n> index 6d0785b7b484..aea90abaf9ef 100644\n> --- a/src/libcamera/v4l2_subdevice.cpp\n> +++ b/src/libcamera/v4l2_subdevice.cpp\n> @@ -134,6 +134,10 @@ const std::map formatInfoMap = {\n> \t{ MEDIA_BUS_FMT_SGBRG12_1X12, { 12, \"SGBRG12_1X12\", PixelFormatInfo::ColourEncodingRAW } },\n> \t{ MEDIA_BUS_FMT_SGRBG12_1X12, { 12, \"SGRBG12_1X12\", PixelFormatInfo::ColourEncodingRAW } },\n> \t{ MEDIA_BUS_FMT_SRGGB12_1X12, { 12, \"SRGGB12_1X12\", PixelFormatInfo::ColourEncodingRAW } },\n> +\t{ MEDIA_BUS_FMT_SBGGR16_1X16, { 16, \"SBGGR16_1x16\", PixelFormatInfo::ColourEncodingRAW } },\n> +\t{ MEDIA_BUS_FMT_SGBRG16_1X16, { 16, \"SGBRG16_1x16\", PixelFormatInfo::ColourEncodingRAW } },\n> +\t{ MEDIA_BUS_FMT_SGRBG16_1X16, { 16, \"SGRBG16_1x16\", PixelFormatInfo::ColourEncodingRAW } },\n> +\t{ MEDIA_BUS_FMT_SRGGB16_1X16, { 16, \"SRGGB16_1x16\", PixelFormatInfo::ColourEncodingRAW } },\n> \t/* \\todo Clarify colour encoding for HSV formats */\n> \t{ MEDIA_BUS_FMT_AHSV8888_1X32, { 32, \"AHSV8888_1X32\", PixelFormatInfo::ColourEncodingRGB } },\n> \t{ MEDIA_BUS_FMT_JPEG_1X8, { 8, \"JPEG_1X8\", PixelFormatInfo::ColourEncodingYUV } },\n> --\n> 2.34.1\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id A4A53BD80A\n\tfor ;\n\tMon, 26 Feb 2024 13:49:14 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 11A2862871;\n\tMon, 26 Feb 2024 14:49:14 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id EBEEE6285F\n\tfor ;\n\tMon, 26 Feb 2024 14:49:12 +0100 (CET)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 73D3613AB;\n\tMon, 26 Feb 2024 14:49:01 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"mZGSWxuO\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1708955341;\n\tbh=lVxzYZStQ1nZ6TY7RWGCJXicx5ILJqtJ8FU/OcJp01o=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=mZGSWxuO/XDCmQMTgv/m9gvlgRMF8T8tyRNxZBUkh4r1Whrq4e3nKARKTtpdAbJ4d\n\tANNQi4xX6g6OjC0O7B2GJ5AtK/g4SgImaS5il36YH9rrZVdWOjvru5A0ZhMqcxzPQM\n\tVxnv7t9fE0NFIvrpoB0DgmkDzq1dgQ7HGnUOaZDY=","Date":"Mon, 26 Feb 2024 14:49:09 +0100","From":"Jacopo Mondi ","To":"Naushir Patuck ","Subject":"Re: [PATCH v1 2/2] libcamera: formats: Add PiSP specific image and\n\tconfig buffer formats","Message-ID":"<74nstgdru2jpkry22imfmrcelvvzkfektet6cdo72gl5hqgz6t@vr6q6rtvti27>","References":"<20240215132710.810-1-naush@raspberrypi.com>\n\t<20240215132710.810-3-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240215132710.810-3-naush@raspberrypi.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28734,"web_url":"https://patchwork.libcamera.org/comment/28734/","msgid":"","date":"2024-02-26T13:55:41","subject":"Re: [PATCH v1 2/2] libcamera: formats: Add PiSP specific image and\n\tconfig buffer formats","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Actually...\n\nOn Mon, Feb 26, 2024 at 02:49:09PM +0100, Jacopo Mondi wrote:\n> Hi Naush\n>\n> On Thu, Feb 15, 2024 at 01:27:10PM +0000, Naushir Patuck wrote:\n> > Add the Raspberry Pi 5 PiSP specific compressed Bayer format types 1/2:\n> > - V4L2_PIX_FMT_PISP_COMP1_xxx\n> > - V4L2_PIX_FMT_PISP_COMP2_xxx\n> >\n> > Add the Raspberry Pi 5 PiSP Backend config format:\n> > - V4L2_META_FMT_RPI_BE_CFG\n> >\n> > Additionally, we also extend libcamera format handlers to support 16-bit\n> > Bayer formats across the media bus.\n> >\n> > Signed-off-by: Naushir Patuck \n> > ---\n> > include/libcamera/internal/bayer_format.h | 2 +\n> > include/linux/drm_fourcc.h | 4 ++\n> > include/linux/videodev2.h | 15 +++++++\n> > src/libcamera/bayer_format.cpp | 18 ++++++++\n> > src/libcamera/formats.cpp | 51 ++++++++++++++++++++++-\n> > src/libcamera/formats.yaml | 16 +++++++\n> > src/libcamera/v4l2_pixelformat.cpp | 10 +++++\n> > src/libcamera/v4l2_subdevice.cpp | 4 ++\n> > 8 files changed, 119 insertions(+), 1 deletion(-)\n> >\n> > diff --git a/include/libcamera/internal/bayer_format.h b/include/libcamera/internal/bayer_format.h\n> > index 78ba3969913d..164743f7e9f6 100644\n> > --- a/include/libcamera/internal/bayer_format.h\n> > +++ b/include/libcamera/internal/bayer_format.h\n> > @@ -34,6 +34,8 @@ public:\n> > \t\tNone = 0,\n> > \t\tCSI2 = 1,\n> > \t\tIPU3 = 2,\n> > +\t\tPISP1 = 3,\n> > +\t\tPISP2 = 4,\n> > \t};\n> >\n> > \tconstexpr BayerFormat()\n> > diff --git a/include/linux/drm_fourcc.h b/include/linux/drm_fourcc.h\n> > index 4ee421b95730..eff27fbe5a1e 100644\n> > --- a/include/linux/drm_fourcc.h\n> > +++ b/include/linux/drm_fourcc.h\n> > @@ -490,6 +490,7 @@ extern \"C\" {\n> > #define DRM_FORMAT_MOD_VENDOR_ALLWINNER 0x09\n> > #define DRM_FORMAT_MOD_VENDOR_AMLOGIC 0x0a\n> > #define DRM_FORMAT_MOD_VENDOR_MIPI 0x0b\n> > +#define DRM_FORMAT_MOD_VENDOR_RPI 0x0c\n> >\n> > /* add more to the end as needed */\n> >\n> > @@ -1670,6 +1671,9 @@ drm_fourcc_canonicalize_nvidia_format_mod(__u64 modifier)\n> > */\n> > #define MIPI_FORMAT_MOD_CSI2_PACKED fourcc_mod_code(MIPI, 1)\n> >\n> > +#define PISP_FORMAT_MOD_COMPRESS_MODE1 fourcc_mod_code(RPI, 1)\n> > +#define PISP_FORMAT_MOD_COMPRESS_MODE2 fourcc_mod_code(RPI, 2)\n> > +\n>\n> I'll send a patch to DRM/KMS for these formats as well\n>\n\nCan these be upstreamed if no DRM format is actually using them ? Or\nshould we add a DRM FourCC for the compressed formats, even if they\nwon't be used outside of the PiSP FE/BE loop ?\n\n> > #if defined(__cplusplus)\n> > }\n> > #endif\n> > diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h\n> > index ba48d2c89726..59af6f794680 100644\n> > --- a/include/linux/videodev2.h\n> > +++ b/include/linux/videodev2.h\n> > @@ -789,6 +789,18 @@ struct v4l2_pix_format {\n> > #define V4L2_PIX_FMT_IPU3_SGRBG10\tv4l2_fourcc('i', 'p', '3', 'G') /* IPU3 packed 10-bit GRBG bayer */\n> > #define V4L2_PIX_FMT_IPU3_SRGGB10\tv4l2_fourcc('i', 'p', '3', 'r') /* IPU3 packed 10-bit RGGB bayer */\n> >\n> > +/* The pixel format for all our buffers (the precise format is found in the config buffer). */\n> > +#define V4L2_PIX_FMT_PISP_COMP1_RGGB\tv4l2_fourcc('P', 'C', '1', 'R')\n> > +#define V4L2_PIX_FMT_PISP_COMP1_GRBG\tv4l2_fourcc('P', 'C', '1', 'G')\n> > +#define V4L2_PIX_FMT_PISP_COMP1_GBRG\tv4l2_fourcc('P', 'C', '1', 'g')\n> > +#define V4L2_PIX_FMT_PISP_COMP1_BGGR\tv4l2_fourcc('P', 'C', '1', 'B')\n> > +#define V4L2_PIX_FMT_PISP_COMP1_MONO\tv4l2_fourcc('P', 'C', '1', 'M')\n> > +#define V4L2_PIX_FMT_PISP_COMP2_RGGB\tv4l2_fourcc('P', 'C', '2', 'R')\n> > +#define V4L2_PIX_FMT_PISP_COMP2_GRBG\tv4l2_fourcc('P', 'C', '2', 'G')\n> > +#define V4L2_PIX_FMT_PISP_COMP2_GBRG\tv4l2_fourcc('P', 'C', '2', 'g')\n> > +#define V4L2_PIX_FMT_PISP_COMP2_BGGR\tv4l2_fourcc('P', 'C', '2', 'B')\n> > +#define V4L2_PIX_FMT_PISP_COMP2_MONO\tv4l2_fourcc('P', 'C', '2', 'M')\n> > +\n>\n> I would use here what has been sent upstream for the BE support\n>\n> /* Raspberry Pi PiSP compressed formats. */\n> #define V4L2_PIX_FMT_PISP_COMP1_RGGB\tv4l2_fourcc('P', 'C', '1', 'R') /* PiSP 8-bit mode 1 compressed RGGB bayer */\n> #define V4L2_PIX_FMT_PISP_COMP1_GRBG\tv4l2_fourcc('P', 'C', '1', 'G') /* PiSP 8-bit mode 1 compressed GRBG bayer */\n> #define V4L2_PIX_FMT_PISP_COMP1_GBRG\tv4l2_fourcc('P', 'C', '1', 'g') /* PiSP 8-bit mode 1 compressed GBRG bayer */\n> #define V4L2_PIX_FMT_PISP_COMP1_BGGR\tv4l2_fourcc('P', 'C', '1', 'B') /* PiSP 8-bit mode 1 compressed BGGR bayer */\n> #define V4L2_PIX_FMT_PISP_COMP1_MONO\tv4l2_fourcc('P', 'C', '1', 'M') /* PiSP 8-bit mode 1 compressed monochrome */\n> #define V4L2_PIX_FMT_PISP_COMP2_RGGB\tv4l2_fourcc('P', 'C', '2', 'R') /* PiSP 8-bit mode 2 compressed RGGB bayer */\n> #define V4L2_PIX_FMT_PISP_COMP2_GRBG\tv4l2_fourcc('P', 'C', '2', 'G') /* PiSP 8-bit mode 2 compressed GRBG bayer */\n> #define V4L2_PIX_FMT_PISP_COMP2_GBRG\tv4l2_fourcc('P', 'C', '2', 'g') /* PiSP 8-bit mode 2 compressed GBRG bayer */\n> #define V4L2_PIX_FMT_PISP_COMP2_BGGR\tv4l2_fourcc('P', 'C', '2', 'B') /* PiSP 8-bit mode 2 compressed BGGR bayer */\n> #define V4L2_PIX_FMT_PISP_COMP2_MONO\tv4l2_fourcc('P', 'C', '2', 'M') /* PiSP 8-bit mode 2 compressed monochrome */\n>\n> To make it easier to update it when these will land in upstream\n>\n> > /* SDR formats - used only for Software Defined Radio devices */\n> > #define V4L2_SDR_FMT_CU8 v4l2_fourcc('C', 'U', '0', '8') /* IQ u8 */\n> > #define V4L2_SDR_FMT_CU16LE v4l2_fourcc('C', 'U', '1', '6') /* IQ u16le */\n> > @@ -818,6 +830,9 @@ struct v4l2_pix_format {\n> > #define V4L2_META_FMT_RK_ISP1_PARAMS\tv4l2_fourcc('R', 'K', '1', 'P') /* Rockchip ISP1 3A Parameters */\n> > #define V4L2_META_FMT_RK_ISP1_STAT_3A\tv4l2_fourcc('R', 'K', '1', 'S') /* Rockchip ISP1 3A Statistics */\n> >\n> > +/* Vendor specific - used for RaspberryPi PiSP */\n> > +#define V4L2_META_FMT_RPI_BE_CFG v4l2_fourcc('R', 'P', 'B', 'C') /* PiSP BE configuration */\n> > /* priv field value to indicates that subsequent fields are valid. */\n> > #define V4L2_PIX_FMT_PRIV_MAGIC\t\t0xfeedcafe\n> >\n> > diff --git a/src/libcamera/bayer_format.cpp b/src/libcamera/bayer_format.cpp\n> > index 20aedfa6d925..333b1117f531 100644\n> > --- a/src/libcamera/bayer_format.cpp\n> > +++ b/src/libcamera/bayer_format.cpp\n> > @@ -164,6 +164,14 @@ const std::map bayerToFormat{\n> > \t\t{ formats::SGRBG16, V4L2PixelFormat(V4L2_PIX_FMT_SGRBG16) } },\n> > \t{ { BayerFormat::RGGB, 16, BayerFormat::Packing::None },\n> > \t\t{ formats::SRGGB16, V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16) } },\n> > +\t{ { BayerFormat::BGGR, 16, BayerFormat::Packing::PISP1 },\n> > +\t\t{ formats::BGGR16_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_BGGR) } },\n> > +\t{ { BayerFormat::GBRG, 16, BayerFormat::Packing::PISP1 },\n> > +\t\t{ formats::GBRG16_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GBRG) } },\n> > +\t{ { BayerFormat::GRBG, 16, BayerFormat::Packing::PISP1 },\n> > +\t\t{ formats::GRBG16_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GRBG) } },\n> > +\t{ { BayerFormat::RGGB, 16, BayerFormat::Packing::PISP1 },\n> > +\t\t{ formats::RGGB16_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_RGGB) } },\n> > \t{ { BayerFormat::MONO, 8, BayerFormat::Packing::None },\n> > \t\t{ formats::R8, V4L2PixelFormat(V4L2_PIX_FMT_GREY) } },\n> > \t{ { BayerFormat::MONO, 10, BayerFormat::Packing::None },\n> > @@ -174,6 +182,8 @@ const std::map bayerToFormat{\n> > \t\t{ formats::R12, V4L2PixelFormat(V4L2_PIX_FMT_Y12) } },\n> > \t{ { BayerFormat::MONO, 16, BayerFormat::Packing::None },\n> > \t\t{ formats::R16, V4L2PixelFormat(V4L2_PIX_FMT_Y16) } },\n> > +\t{ { BayerFormat::MONO, 16, BayerFormat::Packing::PISP1 },\n> > +\t\t{ formats::MONO_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_MONO) } },\n>\n> Should the COMP2 versions be added as well ?\n>\n> > };\n> >\n> > const std::unordered_map mbusCodeToBayer{\n> > @@ -209,6 +219,10 @@ const std::unordered_map mbusCodeToBayer{\n> > \t{ MEDIA_BUS_FMT_SGBRG16_1X16, { BayerFormat::GBRG, 16, BayerFormat::Packing::None } },\n> > \t{ MEDIA_BUS_FMT_SGRBG16_1X16, { BayerFormat::GRBG, 16, BayerFormat::Packing::None } },\n> > \t{ MEDIA_BUS_FMT_SRGGB16_1X16, { BayerFormat::RGGB, 16, BayerFormat::Packing::None } },\n> > +\t{ MEDIA_BUS_FMT_SBGGR16_1X16, { BayerFormat::BGGR, 16, BayerFormat::Packing::PISP1 } },\n> > +\t{ MEDIA_BUS_FMT_SGBRG16_1X16, { BayerFormat::GBRG, 16, BayerFormat::Packing::PISP1 } },\n> > +\t{ MEDIA_BUS_FMT_SGRBG16_1X16, { BayerFormat::GRBG, 16, BayerFormat::Packing::PISP1 } },\n> > +\t{ MEDIA_BUS_FMT_SRGGB16_1X16, { BayerFormat::RGGB, 16, BayerFormat::Packing::PISP1 } },\n>\n> mbusCodeToBayer is an unordered_map which\n> \"is an associative container that contains key-value pairs with unique\n> keys.\"\n>\n> I'm afraid adding a new entry with the same key overwrite the existing\n> one ?\n>\n> We already had issues with this map, as we had similar problems with the\n> _LE and _BE version of RGB565 which resolv to the same BayerFormat\n>\n> \t{ MEDIA_BUS_FMT_SBGGR10_2X8_PADHI_BE, { BayerFormat::BGGR, 10, BayerFormat::Packing::None } },\n> \t{ MEDIA_BUS_FMT_SBGGR10_2X8_PADHI_LE, { BayerFormat::BGGR, 10, BayerFormat::Packing::None } },\n> \t{ MEDIA_BUS_FMT_SBGGR10_2X8_PADLO_BE, { BayerFormat::BGGR, 10, BayerFormat::Packing::None } },\n> \t{ MEDIA_BUS_FMT_SBGGR10_2X8_PADLO_LE, { BayerFormat::BGGR, 10, BayerFormat::Packing::None } },\n>\n> In general, we know that resolving mbus_codes to a unique format is\n> ill-defined. How are you using these entries in the pi5 support ?\n>\n> > \t{ MEDIA_BUS_FMT_Y8_1X8, { BayerFormat::MONO, 8, BayerFormat::Packing::None } },\n> > \t{ MEDIA_BUS_FMT_Y10_1X10, { BayerFormat::MONO, 10, BayerFormat::Packing::None } },\n> > \t{ MEDIA_BUS_FMT_Y12_1X12, { BayerFormat::MONO, 12, BayerFormat::Packing::None } },\n> > @@ -303,6 +317,10 @@ std::ostream &operator<<(std::ostream &out, const BayerFormat &f)\n> > \t\tout << \"-CSI2P\";\n> > \telse if (f.packing == BayerFormat::Packing::IPU3)\n> > \t\tout << \"-IPU3P\";\n> > +\telse if (f.packing == BayerFormat::Packing::PISP1)\n> > +\t\tout << \"-PISP1\";\n> > +\telse if (f.packing == BayerFormat::Packing::PISP2)\n> > +\t\tout << \"-PISP2\";\n> >\n> > \treturn out;\n> > }\n> > diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp\n> > index a674f4179cc8..e603a7eda579 100644\n> > --- a/src/libcamera/formats.cpp\n> > +++ b/src/libcamera/formats.cpp\n> > @@ -547,6 +547,16 @@ const std::map pixelFormatInfo{\n> > \t\t.pixelsPerGroup = 1,\n> > \t\t.planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},\n> > \t} },\n> > +\t{ formats::MONO_PISP_COMP1, {\n> > +\t\t.name = \"MONO_PISP_COMP1\",\n> > +\t\t.format = formats::MONO_PISP_COMP1,\n> > +\t\t.v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_MONO), },\n> > +\t\t.bitsPerPixel = 16,\n> > +\t\t.colourEncoding = PixelFormatInfo::ColourEncodingYUV,\n> > +\t\t.packed = true,\n> > +\t\t.pixelsPerGroup = 1,\n> > +\t\t.planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},\n> > +\t} },\n> >\n> > \t/* Bayer formats. */\n> > \t{ formats::SBGGR8, {\n> > @@ -910,7 +920,46 @@ const std::map pixelFormatInfo{\n> > \t\t.pixelsPerGroup = 25,\n> > \t\t.planes = {{ { 32, 1 }, { 0, 0 }, { 0, 0 } }},\n> > \t} },\n> > -\n> > +\t{ formats::BGGR16_PISP_COMP1, {\n> > +\t\t.name = \"BGGR16_PISP_COMP1\",\n> > +\t\t.format = formats::BGGR16_PISP_COMP1,\n> > +\t\t.v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_BGGR), },\n> > +\t\t.bitsPerPixel = 16,\n> > +\t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> > +\t\t.packed = true,\n> > +\t\t.pixelsPerGroup = 2,\n> > +\t\t.planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},\n> > +\t} },\n>\n> For regular 16-bit formats this should be\n>\n> \t\t.pixelsPerGroup = 2,\n> \t\t.planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},\n>\n> As 2 pixels of 16 bits each consume 4 bytes. Is this {2, 1}\n> intentional and due to compression ?\n>\n> > +\t{ formats::GBRG16_PISP_COMP1, {\n> > +\t\t.name = \"GBRG16_PISP_COMP1\",\n> > +\t\t.format = formats::GBRG16_PISP_COMP1,\n> > +\t\t.v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GBRG), },\n> > +\t\t.bitsPerPixel = 16,\n> > +\t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> > +\t\t.packed = true,\n> > +\t\t.pixelsPerGroup = 2,\n> > +\t\t.planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},\n> > +\t} },\n> > +\t{ formats::GRBG16_PISP_COMP1, {\n> > +\t\t.name = \"GRBG16_PISP_COMP1\",\n> > +\t\t.format = formats::GRBG16_PISP_COMP1,\n> > +\t\t.v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GRBG), },\n> > +\t\t.bitsPerPixel = 16,\n> > +\t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> > +\t\t.packed = true,\n> > +\t\t.pixelsPerGroup = 2,\n> > +\t\t.planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},\n> > +\t} },\n> > +\t{ formats::RGGB16_PISP_COMP1, {\n> > +\t\t.name = \"RGGB16_PISP_COMP1\",\n> > +\t\t.format = formats::RGGB16_PISP_COMP1,\n> > +\t\t.v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_RGGB), },\n> > +\t\t.bitsPerPixel = 16,\n> > +\t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> > +\t\t.packed = true,\n> > +\t\t.pixelsPerGroup = 2,\n> > +\t\t.planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},\n> > +\t} },\n> > \t/* Compressed formats. */\n> > \t{ formats::MJPEG, {\n> > \t\t.name = \"MJPEG\",\n> > diff --git a/src/libcamera/formats.yaml b/src/libcamera/formats.yaml\n> > index bde2cc803b98..f6df721243d0 100644\n> > --- a/src/libcamera/formats.yaml\n> > +++ b/src/libcamera/formats.yaml\n> > @@ -190,4 +190,20 @@ formats:\n> > - SBGGR10_IPU3:\n> > fourcc: DRM_FORMAT_SBGGR10\n> > mod: IPU3_FORMAT_MOD_PACKED\n> > +\n> > + - RGGB16_PISP_COMP1:\n> > + fourcc: DRM_FORMAT_SRGGB16\n> > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1\n> > + - GRBG16_PISP_COMP1:\n> > + fourcc: DRM_FORMAT_SGRBG16\n> > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1\n> > + - GBRG16_PISP_COMP1:\n> > + fourcc: DRM_FORMAT_SGBRG16\n> > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1\n> > + - BGGR16_PISP_COMP1:\n> > + fourcc: DRM_FORMAT_SBGGR16\n> > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1\n> > + - MONO_PISP_COMP1:\n> > + fourcc: DRM_FORMAT_R16\n> > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1\n> > ...\n> > diff --git a/src/libcamera/v4l2_pixelformat.cpp b/src/libcamera/v4l2_pixelformat.cpp\n> > index efb6f2940235..47baaf60199d 100644\n> > --- a/src/libcamera/v4l2_pixelformat.cpp\n> > +++ b/src/libcamera/v4l2_pixelformat.cpp\n> > @@ -207,6 +207,16 @@ const std::map vpf2pf{\n> > \t\t{ formats::SGRBG16, \"16-bit Bayer GRGR/BGBG\" } },\n> > \t{ V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16),\n> > \t\t{ formats::SRGGB16, \"16-bit Bayer RGRG/GBGB\" } },\n> > +\t{ V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_BGGR),\n> > +\t\t{ formats::BGGR16_PISP_COMP1, \"16-bit Bayer BGBG/GRGR PiSP Compress Mode 1\" } },\n> > +\t{ V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GBRG),\n> > +\t\t{ formats::GBRG16_PISP_COMP1, \"16-bit Bayer GBGB/RGRG PiSP Compress Mode 1\" } },\n> > +\t{ V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GRBG),\n> > +\t\t{ formats::GRBG16_PISP_COMP1, \"16-bit Bayer GRGR/BGBG PiSP Compress Mode 1\" } },\n> > +\t{ V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_RGGB),\n> > +\t\t{ formats::RGGB16_PISP_COMP1, \"16-bit Bayer RGRG/GBGB PiSP Compress Mode 1\" } },\n> > +\t{ V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_MONO),\n> > +\t\t{ formats::MONO_PISP_COMP1, \"16-bit Mono PiSP Compress Mode 1\" } },\n> >\n> > \t/* Compressed formats. */\n> > \t{ V4L2PixelFormat(V4L2_PIX_FMT_MJPEG),\n> > diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp\n> > index 6d0785b7b484..aea90abaf9ef 100644\n> > --- a/src/libcamera/v4l2_subdevice.cpp\n> > +++ b/src/libcamera/v4l2_subdevice.cpp\n> > @@ -134,6 +134,10 @@ const std::map formatInfoMap = {\n> > \t{ MEDIA_BUS_FMT_SGBRG12_1X12, { 12, \"SGBRG12_1X12\", PixelFormatInfo::ColourEncodingRAW } },\n> > \t{ MEDIA_BUS_FMT_SGRBG12_1X12, { 12, \"SGRBG12_1X12\", PixelFormatInfo::ColourEncodingRAW } },\n> > \t{ MEDIA_BUS_FMT_SRGGB12_1X12, { 12, \"SRGGB12_1X12\", PixelFormatInfo::ColourEncodingRAW } },\n> > +\t{ MEDIA_BUS_FMT_SBGGR16_1X16, { 16, \"SBGGR16_1x16\", PixelFormatInfo::ColourEncodingRAW } },\n> > +\t{ MEDIA_BUS_FMT_SGBRG16_1X16, { 16, \"SGBRG16_1x16\", PixelFormatInfo::ColourEncodingRAW } },\n> > +\t{ MEDIA_BUS_FMT_SGRBG16_1X16, { 16, \"SGRBG16_1x16\", PixelFormatInfo::ColourEncodingRAW } },\n> > +\t{ MEDIA_BUS_FMT_SRGGB16_1X16, { 16, \"SRGGB16_1x16\", PixelFormatInfo::ColourEncodingRAW } },\n> > \t/* \\todo Clarify colour encoding for HSV formats */\n> > \t{ MEDIA_BUS_FMT_AHSV8888_1X32, { 32, \"AHSV8888_1X32\", PixelFormatInfo::ColourEncodingRGB } },\n> > \t{ MEDIA_BUS_FMT_JPEG_1X8, { 8, \"JPEG_1X8\", PixelFormatInfo::ColourEncodingYUV } },\n> > --\n> > 2.34.1\n> >","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id BA014BD160\n\tfor ;\n\tMon, 26 Feb 2024 13:55:45 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0C74E6285F;\n\tMon, 26 Feb 2024 14:55:45 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 07D096285F\n\tfor ;\n\tMon, 26 Feb 2024 14:55:44 +0100 (CET)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7BF3E673;\n\tMon, 26 Feb 2024 14:55:32 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"LNL69yZR\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1708955732;\n\tbh=y4ouGB+1AwwdVWaZSb2DsNWSW3//KhL0mUx/y+GE9H4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=LNL69yZRoxQJ0MIaP2TKMN4t6S2hPtvZ5+MsqxUqSs59nyybrh480exgPx7HfPjVT\n\tkOGUH3qBzDw3Uej5W1GvM+olRaEGkhcDOsyrxMf//UKCgw5uKeEbCHKSdAzLOkvfBZ\n\trqI6HhNicl9GROoNpA7khbrN8fQ25G6HOIiiUsLo=","Date":"Mon, 26 Feb 2024 14:55:41 +0100","From":"Jacopo Mondi ","To":"Jacopo Mondi ","Subject":"Re: [PATCH v1 2/2] libcamera: formats: Add PiSP specific image and\n\tconfig buffer formats","Message-ID":"","References":"<20240215132710.810-1-naush@raspberrypi.com>\n\t<20240215132710.810-3-naush@raspberrypi.com>\n\t<74nstgdru2jpkry22imfmrcelvvzkfektet6cdo72gl5hqgz6t@vr6q6rtvti27>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<74nstgdru2jpkry22imfmrcelvvzkfektet6cdo72gl5hqgz6t@vr6q6rtvti27>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28735,"web_url":"https://patchwork.libcamera.org/comment/28735/","msgid":"<20240226140616.GB26163@pendragon.ideasonboard.com>","date":"2024-02-26T14:06:16","subject":"Re: [PATCH v1 2/2] libcamera: formats: Add PiSP specific image and\n\tconfig buffer formats","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Mon, Feb 26, 2024 at 02:55:41PM +0100, Jacopo Mondi wrote:\n> Actually...\n> \n> On Mon, Feb 26, 2024 at 02:49:09PM +0100, Jacopo Mondi wrote:\n> > Hi Naush\n> >\n> > On Thu, Feb 15, 2024 at 01:27:10PM +0000, Naushir Patuck wrote:\n> > > Add the Raspberry Pi 5 PiSP specific compressed Bayer format types 1/2:\n> > > - V4L2_PIX_FMT_PISP_COMP1_xxx\n> > > - V4L2_PIX_FMT_PISP_COMP2_xxx\n> > >\n> > > Add the Raspberry Pi 5 PiSP Backend config format:\n> > > - V4L2_META_FMT_RPI_BE_CFG\n> > >\n> > > Additionally, we also extend libcamera format handlers to support 16-bit\n> > > Bayer formats across the media bus.\n> > >\n> > > Signed-off-by: Naushir Patuck \n> > > ---\n> > > include/libcamera/internal/bayer_format.h | 2 +\n> > > include/linux/drm_fourcc.h | 4 ++\n> > > include/linux/videodev2.h | 15 +++++++\n> > > src/libcamera/bayer_format.cpp | 18 ++++++++\n> > > src/libcamera/formats.cpp | 51 ++++++++++++++++++++++-\n> > > src/libcamera/formats.yaml | 16 +++++++\n> > > src/libcamera/v4l2_pixelformat.cpp | 10 +++++\n> > > src/libcamera/v4l2_subdevice.cpp | 4 ++\n> > > 8 files changed, 119 insertions(+), 1 deletion(-)\n> > >\n> > > diff --git a/include/libcamera/internal/bayer_format.h b/include/libcamera/internal/bayer_format.h\n> > > index 78ba3969913d..164743f7e9f6 100644\n> > > --- a/include/libcamera/internal/bayer_format.h\n> > > +++ b/include/libcamera/internal/bayer_format.h\n> > > @@ -34,6 +34,8 @@ public:\n> > > \t\tNone = 0,\n> > > \t\tCSI2 = 1,\n> > > \t\tIPU3 = 2,\n> > > +\t\tPISP1 = 3,\n> > > +\t\tPISP2 = 4,\n> > > \t};\n> > >\n> > > \tconstexpr BayerFormat()\n> > > diff --git a/include/linux/drm_fourcc.h b/include/linux/drm_fourcc.h\n> > > index 4ee421b95730..eff27fbe5a1e 100644\n> > > --- a/include/linux/drm_fourcc.h\n> > > +++ b/include/linux/drm_fourcc.h\n> > > @@ -490,6 +490,7 @@ extern \"C\" {\n> > > #define DRM_FORMAT_MOD_VENDOR_ALLWINNER 0x09\n> > > #define DRM_FORMAT_MOD_VENDOR_AMLOGIC 0x0a\n> > > #define DRM_FORMAT_MOD_VENDOR_MIPI 0x0b\n> > > +#define DRM_FORMAT_MOD_VENDOR_RPI 0x0c\n> > >\n> > > /* add more to the end as needed */\n> > >\n> > > @@ -1670,6 +1671,9 @@ drm_fourcc_canonicalize_nvidia_format_mod(__u64 modifier)\n> > > */\n> > > #define MIPI_FORMAT_MOD_CSI2_PACKED fourcc_mod_code(MIPI, 1)\n> > >\n> > > +#define PISP_FORMAT_MOD_COMPRESS_MODE1 fourcc_mod_code(RPI, 1)\n> > > +#define PISP_FORMAT_MOD_COMPRESS_MODE2 fourcc_mod_code(RPI, 2)\n> > > +\n> >\n> > I'll send a patch to DRM/KMS for these formats as well\n> \n> Can these be upstreamed if no DRM format is actually using them ? Or\n> should we add a DRM FourCC for the compressed formats, even if they\n> won't be used outside of the PiSP FE/BE loop ?\n\nI think an RFC on the dri-devel mailing list would be useful.\n\n> > > #if defined(__cplusplus)\n> > > }\n> > > #endif\n> > > diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h\n> > > index ba48d2c89726..59af6f794680 100644\n> > > --- a/include/linux/videodev2.h\n> > > +++ b/include/linux/videodev2.h\n> > > @@ -789,6 +789,18 @@ struct v4l2_pix_format {\n> > > #define V4L2_PIX_FMT_IPU3_SGRBG10\tv4l2_fourcc('i', 'p', '3', 'G') /* IPU3 packed 10-bit GRBG bayer */\n> > > #define V4L2_PIX_FMT_IPU3_SRGGB10\tv4l2_fourcc('i', 'p', '3', 'r') /* IPU3 packed 10-bit RGGB bayer */\n> > >\n> > > +/* The pixel format for all our buffers (the precise format is found in the config buffer). */\n> > > +#define V4L2_PIX_FMT_PISP_COMP1_RGGB\tv4l2_fourcc('P', 'C', '1', 'R')\n> > > +#define V4L2_PIX_FMT_PISP_COMP1_GRBG\tv4l2_fourcc('P', 'C', '1', 'G')\n> > > +#define V4L2_PIX_FMT_PISP_COMP1_GBRG\tv4l2_fourcc('P', 'C', '1', 'g')\n> > > +#define V4L2_PIX_FMT_PISP_COMP1_BGGR\tv4l2_fourcc('P', 'C', '1', 'B')\n> > > +#define V4L2_PIX_FMT_PISP_COMP1_MONO\tv4l2_fourcc('P', 'C', '1', 'M')\n> > > +#define V4L2_PIX_FMT_PISP_COMP2_RGGB\tv4l2_fourcc('P', 'C', '2', 'R')\n> > > +#define V4L2_PIX_FMT_PISP_COMP2_GRBG\tv4l2_fourcc('P', 'C', '2', 'G')\n> > > +#define V4L2_PIX_FMT_PISP_COMP2_GBRG\tv4l2_fourcc('P', 'C', '2', 'g')\n> > > +#define V4L2_PIX_FMT_PISP_COMP2_BGGR\tv4l2_fourcc('P', 'C', '2', 'B')\n> > > +#define V4L2_PIX_FMT_PISP_COMP2_MONO\tv4l2_fourcc('P', 'C', '2', 'M')\n> > > +\n> >\n> > I would use here what has been sent upstream for the BE support\n> >\n> > /* Raspberry Pi PiSP compressed formats. */\n> > #define V4L2_PIX_FMT_PISP_COMP1_RGGB\tv4l2_fourcc('P', 'C', '1', 'R') /* PiSP 8-bit mode 1 compressed RGGB bayer */\n> > #define V4L2_PIX_FMT_PISP_COMP1_GRBG\tv4l2_fourcc('P', 'C', '1', 'G') /* PiSP 8-bit mode 1 compressed GRBG bayer */\n> > #define V4L2_PIX_FMT_PISP_COMP1_GBRG\tv4l2_fourcc('P', 'C', '1', 'g') /* PiSP 8-bit mode 1 compressed GBRG bayer */\n> > #define V4L2_PIX_FMT_PISP_COMP1_BGGR\tv4l2_fourcc('P', 'C', '1', 'B') /* PiSP 8-bit mode 1 compressed BGGR bayer */\n> > #define V4L2_PIX_FMT_PISP_COMP1_MONO\tv4l2_fourcc('P', 'C', '1', 'M') /* PiSP 8-bit mode 1 compressed monochrome */\n> > #define V4L2_PIX_FMT_PISP_COMP2_RGGB\tv4l2_fourcc('P', 'C', '2', 'R') /* PiSP 8-bit mode 2 compressed RGGB bayer */\n> > #define V4L2_PIX_FMT_PISP_COMP2_GRBG\tv4l2_fourcc('P', 'C', '2', 'G') /* PiSP 8-bit mode 2 compressed GRBG bayer */\n> > #define V4L2_PIX_FMT_PISP_COMP2_GBRG\tv4l2_fourcc('P', 'C', '2', 'g') /* PiSP 8-bit mode 2 compressed GBRG bayer */\n> > #define V4L2_PIX_FMT_PISP_COMP2_BGGR\tv4l2_fourcc('P', 'C', '2', 'B') /* PiSP 8-bit mode 2 compressed BGGR bayer */\n> > #define V4L2_PIX_FMT_PISP_COMP2_MONO\tv4l2_fourcc('P', 'C', '2', 'M') /* PiSP 8-bit mode 2 compressed monochrome */\n> >\n> > To make it easier to update it when these will land in upstream\n> >\n> > > /* SDR formats - used only for Software Defined Radio devices */\n> > > #define V4L2_SDR_FMT_CU8 v4l2_fourcc('C', 'U', '0', '8') /* IQ u8 */\n> > > #define V4L2_SDR_FMT_CU16LE v4l2_fourcc('C', 'U', '1', '6') /* IQ u16le */\n> > > @@ -818,6 +830,9 @@ struct v4l2_pix_format {\n> > > #define V4L2_META_FMT_RK_ISP1_PARAMS\tv4l2_fourcc('R', 'K', '1', 'P') /* Rockchip ISP1 3A Parameters */\n> > > #define V4L2_META_FMT_RK_ISP1_STAT_3A\tv4l2_fourcc('R', 'K', '1', 'S') /* Rockchip ISP1 3A Statistics */\n> > >\n> > > +/* Vendor specific - used for RaspberryPi PiSP */\n> > > +#define V4L2_META_FMT_RPI_BE_CFG v4l2_fourcc('R', 'P', 'B', 'C') /* PiSP BE configuration */\n> > > /* priv field value to indicates that subsequent fields are valid. */\n> > > #define V4L2_PIX_FMT_PRIV_MAGIC\t\t0xfeedcafe\n> > >\n> > > diff --git a/src/libcamera/bayer_format.cpp b/src/libcamera/bayer_format.cpp\n> > > index 20aedfa6d925..333b1117f531 100644\n> > > --- a/src/libcamera/bayer_format.cpp\n> > > +++ b/src/libcamera/bayer_format.cpp\n> > > @@ -164,6 +164,14 @@ const std::map bayerToFormat{\n> > > \t\t{ formats::SGRBG16, V4L2PixelFormat(V4L2_PIX_FMT_SGRBG16) } },\n> > > \t{ { BayerFormat::RGGB, 16, BayerFormat::Packing::None },\n> > > \t\t{ formats::SRGGB16, V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16) } },\n> > > +\t{ { BayerFormat::BGGR, 16, BayerFormat::Packing::PISP1 },\n> > > +\t\t{ formats::BGGR16_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_BGGR) } },\n> > > +\t{ { BayerFormat::GBRG, 16, BayerFormat::Packing::PISP1 },\n> > > +\t\t{ formats::GBRG16_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GBRG) } },\n> > > +\t{ { BayerFormat::GRBG, 16, BayerFormat::Packing::PISP1 },\n> > > +\t\t{ formats::GRBG16_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GRBG) } },\n> > > +\t{ { BayerFormat::RGGB, 16, BayerFormat::Packing::PISP1 },\n> > > +\t\t{ formats::RGGB16_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_RGGB) } },\n> > > \t{ { BayerFormat::MONO, 8, BayerFormat::Packing::None },\n> > > \t\t{ formats::R8, V4L2PixelFormat(V4L2_PIX_FMT_GREY) } },\n> > > \t{ { BayerFormat::MONO, 10, BayerFormat::Packing::None },\n> > > @@ -174,6 +182,8 @@ const std::map bayerToFormat{\n> > > \t\t{ formats::R12, V4L2PixelFormat(V4L2_PIX_FMT_Y12) } },\n> > > \t{ { BayerFormat::MONO, 16, BayerFormat::Packing::None },\n> > > \t\t{ formats::R16, V4L2PixelFormat(V4L2_PIX_FMT_Y16) } },\n> > > +\t{ { BayerFormat::MONO, 16, BayerFormat::Packing::PISP1 },\n> > > +\t\t{ formats::MONO_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_MONO) } },\n> >\n> > Should the COMP2 versions be added as well ?\n> >\n> > > };\n> > >\n> > > const std::unordered_map mbusCodeToBayer{\n> > > @@ -209,6 +219,10 @@ const std::unordered_map mbusCodeToBayer{\n> > > \t{ MEDIA_BUS_FMT_SGBRG16_1X16, { BayerFormat::GBRG, 16, BayerFormat::Packing::None } },\n> > > \t{ MEDIA_BUS_FMT_SGRBG16_1X16, { BayerFormat::GRBG, 16, BayerFormat::Packing::None } },\n> > > \t{ MEDIA_BUS_FMT_SRGGB16_1X16, { BayerFormat::RGGB, 16, BayerFormat::Packing::None } },\n> > > +\t{ MEDIA_BUS_FMT_SBGGR16_1X16, { BayerFormat::BGGR, 16, BayerFormat::Packing::PISP1 } },\n> > > +\t{ MEDIA_BUS_FMT_SGBRG16_1X16, { BayerFormat::GBRG, 16, BayerFormat::Packing::PISP1 } },\n> > > +\t{ MEDIA_BUS_FMT_SGRBG16_1X16, { BayerFormat::GRBG, 16, BayerFormat::Packing::PISP1 } },\n> > > +\t{ MEDIA_BUS_FMT_SRGGB16_1X16, { BayerFormat::RGGB, 16, BayerFormat::Packing::PISP1 } },\n> >\n> > mbusCodeToBayer is an unordered_map which\n> > \"is an associative container that contains key-value pairs with unique\n> > keys.\"\n> >\n> > I'm afraid adding a new entry with the same key overwrite the existing\n> > one ?\n> >\n> > We already had issues with this map, as we had similar problems with the\n> > _LE and _BE version of RGB565 which resolv to the same BayerFormat\n> >\n> > \t{ MEDIA_BUS_FMT_SBGGR10_2X8_PADHI_BE, { BayerFormat::BGGR, 10, BayerFormat::Packing::None } },\n> > \t{ MEDIA_BUS_FMT_SBGGR10_2X8_PADHI_LE, { BayerFormat::BGGR, 10, BayerFormat::Packing::None } },\n> > \t{ MEDIA_BUS_FMT_SBGGR10_2X8_PADLO_BE, { BayerFormat::BGGR, 10, BayerFormat::Packing::None } },\n> > \t{ MEDIA_BUS_FMT_SBGGR10_2X8_PADLO_LE, { BayerFormat::BGGR, 10, BayerFormat::Packing::None } },\n> >\n> > In general, we know that resolving mbus_codes to a unique format is\n> > ill-defined. How are you using these entries in the pi5 support ?\n> >\n> > > \t{ MEDIA_BUS_FMT_Y8_1X8, { BayerFormat::MONO, 8, BayerFormat::Packing::None } },\n> > > \t{ MEDIA_BUS_FMT_Y10_1X10, { BayerFormat::MONO, 10, BayerFormat::Packing::None } },\n> > > \t{ MEDIA_BUS_FMT_Y12_1X12, { BayerFormat::MONO, 12, BayerFormat::Packing::None } },\n> > > @@ -303,6 +317,10 @@ std::ostream &operator<<(std::ostream &out, const BayerFormat &f)\n> > > \t\tout << \"-CSI2P\";\n> > > \telse if (f.packing == BayerFormat::Packing::IPU3)\n> > > \t\tout << \"-IPU3P\";\n> > > +\telse if (f.packing == BayerFormat::Packing::PISP1)\n> > > +\t\tout << \"-PISP1\";\n> > > +\telse if (f.packing == BayerFormat::Packing::PISP2)\n> > > +\t\tout << \"-PISP2\";\n> > >\n> > > \treturn out;\n> > > }\n> > > diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp\n> > > index a674f4179cc8..e603a7eda579 100644\n> > > --- a/src/libcamera/formats.cpp\n> > > +++ b/src/libcamera/formats.cpp\n> > > @@ -547,6 +547,16 @@ const std::map pixelFormatInfo{\n> > > \t\t.pixelsPerGroup = 1,\n> > > \t\t.planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},\n> > > \t} },\n> > > +\t{ formats::MONO_PISP_COMP1, {\n> > > +\t\t.name = \"MONO_PISP_COMP1\",\n> > > +\t\t.format = formats::MONO_PISP_COMP1,\n> > > +\t\t.v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_MONO), },\n> > > +\t\t.bitsPerPixel = 16,\n> > > +\t\t.colourEncoding = PixelFormatInfo::ColourEncodingYUV,\n> > > +\t\t.packed = true,\n> > > +\t\t.pixelsPerGroup = 1,\n> > > +\t\t.planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},\n> > > +\t} },\n> > >\n> > > \t/* Bayer formats. */\n> > > \t{ formats::SBGGR8, {\n> > > @@ -910,7 +920,46 @@ const std::map pixelFormatInfo{\n> > > \t\t.pixelsPerGroup = 25,\n> > > \t\t.planes = {{ { 32, 1 }, { 0, 0 }, { 0, 0 } }},\n> > > \t} },\n> > > -\n> > > +\t{ formats::BGGR16_PISP_COMP1, {\n> > > +\t\t.name = \"BGGR16_PISP_COMP1\",\n> > > +\t\t.format = formats::BGGR16_PISP_COMP1,\n> > > +\t\t.v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_BGGR), },\n> > > +\t\t.bitsPerPixel = 16,\n> > > +\t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> > > +\t\t.packed = true,\n> > > +\t\t.pixelsPerGroup = 2,\n> > > +\t\t.planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},\n> > > +\t} },\n> >\n> > For regular 16-bit formats this should be\n> >\n> > \t\t.pixelsPerGroup = 2,\n> > \t\t.planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},\n> >\n> > As 2 pixels of 16 bits each consume 4 bytes. Is this {2, 1}\n> > intentional and due to compression ?\n> >\n> > > +\t{ formats::GBRG16_PISP_COMP1, {\n> > > +\t\t.name = \"GBRG16_PISP_COMP1\",\n> > > +\t\t.format = formats::GBRG16_PISP_COMP1,\n> > > +\t\t.v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GBRG), },\n> > > +\t\t.bitsPerPixel = 16,\n> > > +\t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> > > +\t\t.packed = true,\n> > > +\t\t.pixelsPerGroup = 2,\n> > > +\t\t.planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},\n> > > +\t} },\n> > > +\t{ formats::GRBG16_PISP_COMP1, {\n> > > +\t\t.name = \"GRBG16_PISP_COMP1\",\n> > > +\t\t.format = formats::GRBG16_PISP_COMP1,\n> > > +\t\t.v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GRBG), },\n> > > +\t\t.bitsPerPixel = 16,\n> > > +\t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> > > +\t\t.packed = true,\n> > > +\t\t.pixelsPerGroup = 2,\n> > > +\t\t.planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},\n> > > +\t} },\n> > > +\t{ formats::RGGB16_PISP_COMP1, {\n> > > +\t\t.name = \"RGGB16_PISP_COMP1\",\n> > > +\t\t.format = formats::RGGB16_PISP_COMP1,\n> > > +\t\t.v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_RGGB), },\n> > > +\t\t.bitsPerPixel = 16,\n> > > +\t\t.colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> > > +\t\t.packed = true,\n> > > +\t\t.pixelsPerGroup = 2,\n> > > +\t\t.planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},\n> > > +\t} },\n> > > \t/* Compressed formats. */\n> > > \t{ formats::MJPEG, {\n> > > \t\t.name = \"MJPEG\",\n> > > diff --git a/src/libcamera/formats.yaml b/src/libcamera/formats.yaml\n> > > index bde2cc803b98..f6df721243d0 100644\n> > > --- a/src/libcamera/formats.yaml\n> > > +++ b/src/libcamera/formats.yaml\n> > > @@ -190,4 +190,20 @@ formats:\n> > > - SBGGR10_IPU3:\n> > > fourcc: DRM_FORMAT_SBGGR10\n> > > mod: IPU3_FORMAT_MOD_PACKED\n> > > +\n> > > + - RGGB16_PISP_COMP1:\n> > > + fourcc: DRM_FORMAT_SRGGB16\n> > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1\n> > > + - GRBG16_PISP_COMP1:\n> > > + fourcc: DRM_FORMAT_SGRBG16\n> > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1\n> > > + - GBRG16_PISP_COMP1:\n> > > + fourcc: DRM_FORMAT_SGBRG16\n> > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1\n> > > + - BGGR16_PISP_COMP1:\n> > > + fourcc: DRM_FORMAT_SBGGR16\n> > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1\n> > > + - MONO_PISP_COMP1:\n> > > + fourcc: DRM_FORMAT_R16\n> > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1\n> > > ...\n> > > diff --git a/src/libcamera/v4l2_pixelformat.cpp b/src/libcamera/v4l2_pixelformat.cpp\n> > > index efb6f2940235..47baaf60199d 100644\n> > > --- a/src/libcamera/v4l2_pixelformat.cpp\n> > > +++ b/src/libcamera/v4l2_pixelformat.cpp\n> > > @@ -207,6 +207,16 @@ const std::map vpf2pf{\n> > > \t\t{ formats::SGRBG16, \"16-bit Bayer GRGR/BGBG\" } },\n> > > \t{ V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16),\n> > > \t\t{ formats::SRGGB16, \"16-bit Bayer RGRG/GBGB\" } },\n> > > +\t{ V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_BGGR),\n> > > +\t\t{ formats::BGGR16_PISP_COMP1, \"16-bit Bayer BGBG/GRGR PiSP Compress Mode 1\" } },\n> > > +\t{ V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GBRG),\n> > > +\t\t{ formats::GBRG16_PISP_COMP1, \"16-bit Bayer GBGB/RGRG PiSP Compress Mode 1\" } },\n> > > +\t{ V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GRBG),\n> > > +\t\t{ formats::GRBG16_PISP_COMP1, \"16-bit Bayer GRGR/BGBG PiSP Compress Mode 1\" } },\n> > > +\t{ V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_RGGB),\n> > > +\t\t{ formats::RGGB16_PISP_COMP1, \"16-bit Bayer RGRG/GBGB PiSP Compress Mode 1\" } },\n> > > +\t{ V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_MONO),\n> > > +\t\t{ formats::MONO_PISP_COMP1, \"16-bit Mono PiSP Compress Mode 1\" } },\n> > >\n> > > \t/* Compressed formats. */\n> > > \t{ V4L2PixelFormat(V4L2_PIX_FMT_MJPEG),\n> > > diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp\n> > > index 6d0785b7b484..aea90abaf9ef 100644\n> > > --- a/src/libcamera/v4l2_subdevice.cpp\n> > > +++ b/src/libcamera/v4l2_subdevice.cpp\n> > > @@ -134,6 +134,10 @@ const std::map formatInfoMap = {\n> > > \t{ MEDIA_BUS_FMT_SGBRG12_1X12, { 12, \"SGBRG12_1X12\", PixelFormatInfo::ColourEncodingRAW } },\n> > > \t{ MEDIA_BUS_FMT_SGRBG12_1X12, { 12, \"SGRBG12_1X12\", PixelFormatInfo::ColourEncodingRAW } },\n> > > \t{ MEDIA_BUS_FMT_SRGGB12_1X12, { 12, \"SRGGB12_1X12\", PixelFormatInfo::ColourEncodingRAW } },\n> > > +\t{ MEDIA_BUS_FMT_SBGGR16_1X16, { 16, \"SBGGR16_1x16\", PixelFormatInfo::ColourEncodingRAW } },\n> > > +\t{ MEDIA_BUS_FMT_SGBRG16_1X16, { 16, \"SGBRG16_1x16\", PixelFormatInfo::ColourEncodingRAW } },\n> > > +\t{ MEDIA_BUS_FMT_SGRBG16_1X16, { 16, \"SGRBG16_1x16\", PixelFormatInfo::ColourEncodingRAW } },\n> > > +\t{ MEDIA_BUS_FMT_SRGGB16_1X16, { 16, \"SRGGB16_1x16\", PixelFormatInfo::ColourEncodingRAW } },\n> > > \t/* \\todo Clarify colour encoding for HSV formats */\n> > > \t{ MEDIA_BUS_FMT_AHSV8888_1X32, { 32, \"AHSV8888_1X32\", PixelFormatInfo::ColourEncodingRGB } },\n> > > \t{ MEDIA_BUS_FMT_JPEG_1X8, { 8, \"JPEG_1X8\", PixelFormatInfo::ColourEncodingYUV } },","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 0FD06BD80A\n\tfor ;\n\tMon, 26 Feb 2024 14:06:19 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5B6EB62871;\n\tMon, 26 Feb 2024 15:06:18 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 27CCF6285F\n\tfor ;\n\tMon, 26 Feb 2024 15:06:15 +0100 (CET)","from pendragon.ideasonboard.com (89-27-53-110.bb.dnainternet.fi\n\t[89.27.53.110])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 2D02713AB;\n\tMon, 26 Feb 2024 15:06:03 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"EP1/x8sJ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1708956363;\n\tbh=vlJEYoxCFDo2IqMGESC5o7LTeDvj/8jGY731nt/lbPg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=EP1/x8sJIYIIMQWaYbMXAjSku7XqIdXMoFx/II/ByByeIbDGK/r/dwAQT4Sr2nZu3\n\tfeBKhnQ2vaW/wgmnCTI6QLyMrHbMe3+ADkqOV1rV0PH2/syTRvCV+xERFja5ecWDmx\n\tTV2gMfO9kpvu5XO9rCWJ3AXaUjmDNpy2YxwyXIwA=","Date":"Mon, 26 Feb 2024 16:06:16 +0200","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Subject":"Re: [PATCH v1 2/2] libcamera: formats: Add PiSP specific image and\n\tconfig buffer formats","Message-ID":"<20240226140616.GB26163@pendragon.ideasonboard.com>","References":"<20240215132710.810-1-naush@raspberrypi.com>\n\t<20240215132710.810-3-naush@raspberrypi.com>\n\t<74nstgdru2jpkry22imfmrcelvvzkfektet6cdo72gl5hqgz6t@vr6q6rtvti27>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28749,"web_url":"https://patchwork.libcamera.org/comment/28749/","msgid":"","date":"2024-02-27T10:20:40","subject":"Re: [PATCH v1 2/2] libcamera: formats: Add PiSP specific image and\n\tconfig buffer formats","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Jacopo,\n\nThanks for the feedback, and sending the updates to the DRM mailing list!\n\nOn Mon, 26 Feb 2024 at 13:49, Jacopo Mondi\n wrote:\n>\n> Hi Naush\n>\n> On Thu, Feb 15, 2024 at 01:27:10PM +0000, Naushir Patuck wrote:\n> > Add the Raspberry Pi 5 PiSP specific compressed Bayer format types 1/2:\n> > - V4L2_PIX_FMT_PISP_COMP1_xxx\n> > - V4L2_PIX_FMT_PISP_COMP2_xxx\n> >\n> > Add the Raspberry Pi 5 PiSP Backend config format:\n> > - V4L2_META_FMT_RPI_BE_CFG\n> >\n> > Additionally, we also extend libcamera format handlers to support 16-bit\n> > Bayer formats across the media bus.\n> >\n> > Signed-off-by: Naushir Patuck \n> > ---\n> > include/libcamera/internal/bayer_format.h | 2 +\n> > include/linux/drm_fourcc.h | 4 ++\n> > include/linux/videodev2.h | 15 +++++++\n> > src/libcamera/bayer_format.cpp | 18 ++++++++\n> > src/libcamera/formats.cpp | 51 ++++++++++++++++++++++-\n> > src/libcamera/formats.yaml | 16 +++++++\n> > src/libcamera/v4l2_pixelformat.cpp | 10 +++++\n> > src/libcamera/v4l2_subdevice.cpp | 4 ++\n> > 8 files changed, 119 insertions(+), 1 deletion(-)\n> >\n> > diff --git a/include/libcamera/internal/bayer_format.h b/include/libcamera/internal/bayer_format.h\n> > index 78ba3969913d..164743f7e9f6 100644\n> > --- a/include/libcamera/internal/bayer_format.h\n> > +++ b/include/libcamera/internal/bayer_format.h\n> > @@ -34,6 +34,8 @@ public:\n> > None = 0,\n> > CSI2 = 1,\n> > IPU3 = 2,\n> > + PISP1 = 3,\n> > + PISP2 = 4,\n> > };\n> >\n> > constexpr BayerFormat()\n> > diff --git a/include/linux/drm_fourcc.h b/include/linux/drm_fourcc.h\n> > index 4ee421b95730..eff27fbe5a1e 100644\n> > --- a/include/linux/drm_fourcc.h\n> > +++ b/include/linux/drm_fourcc.h\n> > @@ -490,6 +490,7 @@ extern \"C\" {\n> > #define DRM_FORMAT_MOD_VENDOR_ALLWINNER 0x09\n> > #define DRM_FORMAT_MOD_VENDOR_AMLOGIC 0x0a\n> > #define DRM_FORMAT_MOD_VENDOR_MIPI 0x0b\n> > +#define DRM_FORMAT_MOD_VENDOR_RPI 0x0c\n> >\n> > /* add more to the end as needed */\n> >\n> > @@ -1670,6 +1671,9 @@ drm_fourcc_canonicalize_nvidia_format_mod(__u64 modifier)\n> > */\n> > #define MIPI_FORMAT_MOD_CSI2_PACKED fourcc_mod_code(MIPI, 1)\n> >\n> > +#define PISP_FORMAT_MOD_COMPRESS_MODE1 fourcc_mod_code(RPI, 1)\n> > +#define PISP_FORMAT_MOD_COMPRESS_MODE2 fourcc_mod_code(RPI, 2)\n> > +\n>\n> I'll send a patch to DRM/KMS for these formats as well\n>\n> > #if defined(__cplusplus)\n> > }\n> > #endif\n> > diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h\n> > index ba48d2c89726..59af6f794680 100644\n> > --- a/include/linux/videodev2.h\n> > +++ b/include/linux/videodev2.h\n> > @@ -789,6 +789,18 @@ struct v4l2_pix_format {\n> > #define V4L2_PIX_FMT_IPU3_SGRBG10 v4l2_fourcc('i', 'p', '3', 'G') /* IPU3 packed 10-bit GRBG bayer */\n> > #define V4L2_PIX_FMT_IPU3_SRGGB10 v4l2_fourcc('i', 'p', '3', 'r') /* IPU3 packed 10-bit RGGB bayer */\n> >\n> > +/* The pixel format for all our buffers (the precise format is found in the config buffer). */\n> > +#define V4L2_PIX_FMT_PISP_COMP1_RGGB v4l2_fourcc('P', 'C', '1', 'R')\n> > +#define V4L2_PIX_FMT_PISP_COMP1_GRBG v4l2_fourcc('P', 'C', '1', 'G')\n> > +#define V4L2_PIX_FMT_PISP_COMP1_GBRG v4l2_fourcc('P', 'C', '1', 'g')\n> > +#define V4L2_PIX_FMT_PISP_COMP1_BGGR v4l2_fourcc('P', 'C', '1', 'B')\n> > +#define V4L2_PIX_FMT_PISP_COMP1_MONO v4l2_fourcc('P', 'C', '1', 'M')\n> > +#define V4L2_PIX_FMT_PISP_COMP2_RGGB v4l2_fourcc('P', 'C', '2', 'R')\n> > +#define V4L2_PIX_FMT_PISP_COMP2_GRBG v4l2_fourcc('P', 'C', '2', 'G')\n> > +#define V4L2_PIX_FMT_PISP_COMP2_GBRG v4l2_fourcc('P', 'C', '2', 'g')\n> > +#define V4L2_PIX_FMT_PISP_COMP2_BGGR v4l2_fourcc('P', 'C', '2', 'B')\n> > +#define V4L2_PIX_FMT_PISP_COMP2_MONO v4l2_fourcc('P', 'C', '2', 'M')\n> > +\n>\n> I would use here what has been sent upstream for the BE support\n>\n> /* Raspberry Pi PiSP compressed formats. */\n> #define V4L2_PIX_FMT_PISP_COMP1_RGGB v4l2_fourcc('P', 'C', '1', 'R') /* PiSP 8-bit mode 1 compressed RGGB bayer */\n> #define V4L2_PIX_FMT_PISP_COMP1_GRBG v4l2_fourcc('P', 'C', '1', 'G') /* PiSP 8-bit mode 1 compressed GRBG bayer */\n> #define V4L2_PIX_FMT_PISP_COMP1_GBRG v4l2_fourcc('P', 'C', '1', 'g') /* PiSP 8-bit mode 1 compressed GBRG bayer */\n> #define V4L2_PIX_FMT_PISP_COMP1_BGGR v4l2_fourcc('P', 'C', '1', 'B') /* PiSP 8-bit mode 1 compressed BGGR bayer */\n> #define V4L2_PIX_FMT_PISP_COMP1_MONO v4l2_fourcc('P', 'C', '1', 'M') /* PiSP 8-bit mode 1 compressed monochrome */\n> #define V4L2_PIX_FMT_PISP_COMP2_RGGB v4l2_fourcc('P', 'C', '2', 'R') /* PiSP 8-bit mode 2 compressed RGGB bayer */\n> #define V4L2_PIX_FMT_PISP_COMP2_GRBG v4l2_fourcc('P', 'C', '2', 'G') /* PiSP 8-bit mode 2 compressed GRBG bayer */\n> #define V4L2_PIX_FMT_PISP_COMP2_GBRG v4l2_fourcc('P', 'C', '2', 'g') /* PiSP 8-bit mode 2 compressed GBRG bayer */\n> #define V4L2_PIX_FMT_PISP_COMP2_BGGR v4l2_fourcc('P', 'C', '2', 'B') /* PiSP 8-bit mode 2 compressed BGGR bayer */\n> #define V4L2_PIX_FMT_PISP_COMP2_MONO v4l2_fourcc('P', 'C', '2', 'M') /* PiSP 8-bit mode 2 compressed monochrome */\n>\n> To make it easier to update it when these will land in upstream\n\nAck.\n\n>\n> > /* SDR formats - used only for Software Defined Radio devices */\n> > #define V4L2_SDR_FMT_CU8 v4l2_fourcc('C', 'U', '0', '8') /* IQ u8 */\n> > #define V4L2_SDR_FMT_CU16LE v4l2_fourcc('C', 'U', '1', '6') /* IQ u16le */\n> > @@ -818,6 +830,9 @@ struct v4l2_pix_format {\n> > #define V4L2_META_FMT_RK_ISP1_PARAMS v4l2_fourcc('R', 'K', '1', 'P') /* Rockchip ISP1 3A Parameters */\n> > #define V4L2_META_FMT_RK_ISP1_STAT_3A v4l2_fourcc('R', 'K', '1', 'S') /* Rockchip ISP1 3A Statistics */\n> >\n> > +/* Vendor specific - used for RaspberryPi PiSP */\n> > +#define V4L2_META_FMT_RPI_BE_CFG v4l2_fourcc('R', 'P', 'B', 'C') /* PiSP BE configuration */\n> > /* priv field value to indicates that subsequent fields are valid. */\n> > #define V4L2_PIX_FMT_PRIV_MAGIC 0xfeedcafe\n> >\n> > diff --git a/src/libcamera/bayer_format.cpp b/src/libcamera/bayer_format.cpp\n> > index 20aedfa6d925..333b1117f531 100644\n> > --- a/src/libcamera/bayer_format.cpp\n> > +++ b/src/libcamera/bayer_format.cpp\n> > @@ -164,6 +164,14 @@ const std::map bayerToFormat{\n> > { formats::SGRBG16, V4L2PixelFormat(V4L2_PIX_FMT_SGRBG16) } },\n> > { { BayerFormat::RGGB, 16, BayerFormat::Packing::None },\n> > { formats::SRGGB16, V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16) } },\n> > + { { BayerFormat::BGGR, 16, BayerFormat::Packing::PISP1 },\n> > + { formats::BGGR16_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_BGGR) } },\n> > + { { BayerFormat::GBRG, 16, BayerFormat::Packing::PISP1 },\n> > + { formats::GBRG16_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GBRG) } },\n> > + { { BayerFormat::GRBG, 16, BayerFormat::Packing::PISP1 },\n> > + { formats::GRBG16_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GRBG) } },\n> > + { { BayerFormat::RGGB, 16, BayerFormat::Packing::PISP1 },\n> > + { formats::RGGB16_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_RGGB) } },\n> > { { BayerFormat::MONO, 8, BayerFormat::Packing::None },\n> > { formats::R8, V4L2PixelFormat(V4L2_PIX_FMT_GREY) } },\n> > { { BayerFormat::MONO, 10, BayerFormat::Packing::None },\n> > @@ -174,6 +182,8 @@ const std::map bayerToFormat{\n> > { formats::R12, V4L2PixelFormat(V4L2_PIX_FMT_Y12) } },\n> > { { BayerFormat::MONO, 16, BayerFormat::Packing::None },\n> > { formats::R16, V4L2PixelFormat(V4L2_PIX_FMT_Y16) } },\n> > + { { BayerFormat::MONO, 16, BayerFormat::Packing::PISP1 },\n> > + { formats::MONO_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_MONO) } },\n>\n> Should the COMP2 versions be added as well ?\n\nTo be honest, I doubt we will ever use COMP2, COMP1 is the better\noption almost always. But I'll add it for completeness.\n\n\n>\n> > };\n> >\n> > const std::unordered_map mbusCodeToBayer{\n> > @@ -209,6 +219,10 @@ const std::unordered_map mbusCodeToBayer{\n> > { MEDIA_BUS_FMT_SGBRG16_1X16, { BayerFormat::GBRG, 16, BayerFormat::Packing::None } },\n> > { MEDIA_BUS_FMT_SGRBG16_1X16, { BayerFormat::GRBG, 16, BayerFormat::Packing::None } },\n> > { MEDIA_BUS_FMT_SRGGB16_1X16, { BayerFormat::RGGB, 16, BayerFormat::Packing::None } },\n> > + { MEDIA_BUS_FMT_SBGGR16_1X16, { BayerFormat::BGGR, 16, BayerFormat::Packing::PISP1 } },\n> > + { MEDIA_BUS_FMT_SGBRG16_1X16, { BayerFormat::GBRG, 16, BayerFormat::Packing::PISP1 } },\n> > + { MEDIA_BUS_FMT_SGRBG16_1X16, { BayerFormat::GRBG, 16, BayerFormat::Packing::PISP1 } },\n> > + { MEDIA_BUS_FMT_SRGGB16_1X16, { BayerFormat::RGGB, 16, BayerFormat::Packing::PISP1 } },\n>\n> mbusCodeToBayer is an unordered_map which\n> \"is an associative container that contains key-value pairs with unique\n> keys.\"\n>\n> I'm afraid adding a new entry with the same key overwrite the existing\n> one ?\n\nOops you are right! I think this came about from when we originally\nhad MBUS codes for the PISP comp formats, and those subsequently got\nremoved.\n\n>\n> We already had issues with this map, as we had similar problems with the\n> _LE and _BE version of RGB565 which resolv to the same BayerFormat\n>\n> { MEDIA_BUS_FMT_SBGGR10_2X8_PADHI_BE, { BayerFormat::BGGR, 10, BayerFormat::Packing::None } },\n> { MEDIA_BUS_FMT_SBGGR10_2X8_PADHI_LE, { BayerFormat::BGGR, 10, BayerFormat::Packing::None } },\n> { MEDIA_BUS_FMT_SBGGR10_2X8_PADLO_BE, { BayerFormat::BGGR, 10, BayerFormat::Packing::None } },\n> { MEDIA_BUS_FMT_SBGGR10_2X8_PADLO_LE, { BayerFormat::BGGR, 10, BayerFormat::Packing::None } },\n>\n> In general, we know that resolving mbus_codes to a unique format is\n> ill-defined. How are you using these entries in the pi5 support ?\n\nAs luck would have it, the Pi 5 pipeline handler never calls\nBayerFormat::fromMbusCode() with a PISP_COMP type format so the code\nnever gets exercised and goes wrong!\n\nI think the right solution is to remove MEDIA_BUS_FMT_* for the\nPISP_COMP formats as they genuinely don't have MBUS codes associated\nwith them.\n\n>\n> > { MEDIA_BUS_FMT_Y8_1X8, { BayerFormat::MONO, 8, BayerFormat::Packing::None } },\n> > { MEDIA_BUS_FMT_Y10_1X10, { BayerFormat::MONO, 10, BayerFormat::Packing::None } },\n> > { MEDIA_BUS_FMT_Y12_1X12, { BayerFormat::MONO, 12, BayerFormat::Packing::None } },\n> > @@ -303,6 +317,10 @@ std::ostream &operator<<(std::ostream &out, const BayerFormat &f)\n> > out << \"-CSI2P\";\n> > else if (f.packing == BayerFormat::Packing::IPU3)\n> > out << \"-IPU3P\";\n> > + else if (f.packing == BayerFormat::Packing::PISP1)\n> > + out << \"-PISP1\";\n> > + else if (f.packing == BayerFormat::Packing::PISP2)\n> > + out << \"-PISP2\";\n> >\n> > return out;\n> > }\n> > diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp\n> > index a674f4179cc8..e603a7eda579 100644\n> > --- a/src/libcamera/formats.cpp\n> > +++ b/src/libcamera/formats.cpp\n> > @@ -547,6 +547,16 @@ const std::map pixelFormatInfo{\n> > .pixelsPerGroup = 1,\n> > .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},\n> > } },\n> > + { formats::MONO_PISP_COMP1, {\n> > + .name = \"MONO_PISP_COMP1\",\n> > + .format = formats::MONO_PISP_COMP1,\n> > + .v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_MONO), },\n> > + .bitsPerPixel = 16,\n> > + .colourEncoding = PixelFormatInfo::ColourEncodingYUV,\n> > + .packed = true,\n> > + .pixelsPerGroup = 1,\n> > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},\n> > + } },\n> >\n> > /* Bayer formats. */\n> > { formats::SBGGR8, {\n> > @@ -910,7 +920,46 @@ const std::map pixelFormatInfo{\n> > .pixelsPerGroup = 25,\n> > .planes = {{ { 32, 1 }, { 0, 0 }, { 0, 0 } }},\n> > } },\n> > -\n> > + { formats::BGGR16_PISP_COMP1, {\n> > + .name = \"BGGR16_PISP_COMP1\",\n> > + .format = formats::BGGR16_PISP_COMP1,\n> > + .v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_BGGR), },\n> > + .bitsPerPixel = 16,\n> > + .colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> > + .packed = true,\n> > + .pixelsPerGroup = 2,\n> > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},\n> > + } },\n>\n> For regular 16-bit formats this should be\n>\n> .pixelsPerGroup = 2,\n> .planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},\n>\n> As 2 pixels of 16 bits each consume 4 bytes. Is this {2, 1}\n> intentional and due to compression ?\n\nYes, I used {2, 1} to denote the 8-bit/sample after compression. Do\nyou think I should change to {4, 1}?\n\nRegards,\nNaush\n\n\n>\n> > + { formats::GBRG16_PISP_COMP1, {\n> > + .name = \"GBRG16_PISP_COMP1\",\n> > + .format = formats::GBRG16_PISP_COMP1,\n> > + .v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GBRG), },\n> > + .bitsPerPixel = 16,\n> > + .colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> > + .packed = true,\n> > + .pixelsPerGroup = 2,\n> > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},\n> > + } },\n> > + { formats::GRBG16_PISP_COMP1, {\n> > + .name = \"GRBG16_PISP_COMP1\",\n> > + .format = formats::GRBG16_PISP_COMP1,\n> > + .v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GRBG), },\n> > + .bitsPerPixel = 16,\n> > + .colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> > + .packed = true,\n> > + .pixelsPerGroup = 2,\n> > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},\n> > + } },\n> > + { formats::RGGB16_PISP_COMP1, {\n> > + .name = \"RGGB16_PISP_COMP1\",\n> > + .format = formats::RGGB16_PISP_COMP1,\n> > + .v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_RGGB), },\n> > + .bitsPerPixel = 16,\n> > + .colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> > + .packed = true,\n> > + .pixelsPerGroup = 2,\n> > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},\n> > + } },\n> > /* Compressed formats. */\n> > { formats::MJPEG, {\n> > .name = \"MJPEG\",\n> > diff --git a/src/libcamera/formats.yaml b/src/libcamera/formats.yaml\n> > index bde2cc803b98..f6df721243d0 100644\n> > --- a/src/libcamera/formats.yaml\n> > +++ b/src/libcamera/formats.yaml\n> > @@ -190,4 +190,20 @@ formats:\n> > - SBGGR10_IPU3:\n> > fourcc: DRM_FORMAT_SBGGR10\n> > mod: IPU3_FORMAT_MOD_PACKED\n> > +\n> > + - RGGB16_PISP_COMP1:\n> > + fourcc: DRM_FORMAT_SRGGB16\n> > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1\n> > + - GRBG16_PISP_COMP1:\n> > + fourcc: DRM_FORMAT_SGRBG16\n> > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1\n> > + - GBRG16_PISP_COMP1:\n> > + fourcc: DRM_FORMAT_SGBRG16\n> > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1\n> > + - BGGR16_PISP_COMP1:\n> > + fourcc: DRM_FORMAT_SBGGR16\n> > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1\n> > + - MONO_PISP_COMP1:\n> > + fourcc: DRM_FORMAT_R16\n> > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1\n> > ...\n> > diff --git a/src/libcamera/v4l2_pixelformat.cpp b/src/libcamera/v4l2_pixelformat.cpp\n> > index efb6f2940235..47baaf60199d 100644\n> > --- a/src/libcamera/v4l2_pixelformat.cpp\n> > +++ b/src/libcamera/v4l2_pixelformat.cpp\n> > @@ -207,6 +207,16 @@ const std::map vpf2pf{\n> > { formats::SGRBG16, \"16-bit Bayer GRGR/BGBG\" } },\n> > { V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16),\n> > { formats::SRGGB16, \"16-bit Bayer RGRG/GBGB\" } },\n> > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_BGGR),\n> > + { formats::BGGR16_PISP_COMP1, \"16-bit Bayer BGBG/GRGR PiSP Compress Mode 1\" } },\n> > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GBRG),\n> > + { formats::GBRG16_PISP_COMP1, \"16-bit Bayer GBGB/RGRG PiSP Compress Mode 1\" } },\n> > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GRBG),\n> > + { formats::GRBG16_PISP_COMP1, \"16-bit Bayer GRGR/BGBG PiSP Compress Mode 1\" } },\n> > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_RGGB),\n> > + { formats::RGGB16_PISP_COMP1, \"16-bit Bayer RGRG/GBGB PiSP Compress Mode 1\" } },\n> > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_MONO),\n> > + { formats::MONO_PISP_COMP1, \"16-bit Mono PiSP Compress Mode 1\" } },\n> >\n> > /* Compressed formats. */\n> > { V4L2PixelFormat(V4L2_PIX_FMT_MJPEG),\n> > diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp\n> > index 6d0785b7b484..aea90abaf9ef 100644\n> > --- a/src/libcamera/v4l2_subdevice.cpp\n> > +++ b/src/libcamera/v4l2_subdevice.cpp\n> > @@ -134,6 +134,10 @@ const std::map formatInfoMap = {\n> > { MEDIA_BUS_FMT_SGBRG12_1X12, { 12, \"SGBRG12_1X12\", PixelFormatInfo::ColourEncodingRAW } },\n> > { MEDIA_BUS_FMT_SGRBG12_1X12, { 12, \"SGRBG12_1X12\", PixelFormatInfo::ColourEncodingRAW } },\n> > { MEDIA_BUS_FMT_SRGGB12_1X12, { 12, \"SRGGB12_1X12\", PixelFormatInfo::ColourEncodingRAW } },\n> > + { MEDIA_BUS_FMT_SBGGR16_1X16, { 16, \"SBGGR16_1x16\", PixelFormatInfo::ColourEncodingRAW } },\n> > + { MEDIA_BUS_FMT_SGBRG16_1X16, { 16, \"SGBRG16_1x16\", PixelFormatInfo::ColourEncodingRAW } },\n> > + { MEDIA_BUS_FMT_SGRBG16_1X16, { 16, \"SGRBG16_1x16\", PixelFormatInfo::ColourEncodingRAW } },\n> > + { MEDIA_BUS_FMT_SRGGB16_1X16, { 16, \"SRGGB16_1x16\", PixelFormatInfo::ColourEncodingRAW } },\n> > /* \\todo Clarify colour encoding for HSV formats */\n> > { MEDIA_BUS_FMT_AHSV8888_1X32, { 32, \"AHSV8888_1X32\", PixelFormatInfo::ColourEncodingRGB } },\n> > { MEDIA_BUS_FMT_JPEG_1X8, { 8, \"JPEG_1X8\", PixelFormatInfo::ColourEncodingYUV } },\n> > --\n> > 2.34.1\n> >","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 42566BD160\n\tfor ;\n\tTue, 27 Feb 2024 10:21:21 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 91FC962806;\n\tTue, 27 Feb 2024 11:21:20 +0100 (CET)","from mail-yb1-xb2c.google.com (mail-yb1-xb2c.google.com\n\t[IPv6:2607:f8b0:4864:20::b2c])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C7B7B61C95\n\tfor ;\n\tTue, 27 Feb 2024 11:21:17 +0100 (CET)","by mail-yb1-xb2c.google.com with SMTP id\n\t3f1490d57ef6-dc23bf7e5aaso4144874276.0\n\tfor ;\n\tTue, 27 Feb 2024 02:21:17 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"mKF1dAU2\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1709029277; x=1709634077;\n\tdarn=lists.libcamera.org; \n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=vxxl9Rex7NSeC4rjxxHEfVQZ/L4vYyB44xDBAJyyZlA=;\n\tb=mKF1dAU2NgAyV4H2qQV396FIpXizh3zdqZXvfT7gwZ6CC/WWdMQc9dZRYiDXADEkZZ\n\t5TjYKsu/WGsUbpeEWgqjBh5PaLFir7W5JPqs3DgklJ5yOco7Z6O2gELV5IeoFyXSVXvv\n\tGpIj2Mixd3cSGAlZVuJ09Bcr92QDtVA/XFGGviziu7O9Tq9CsX2Pcd3s/YqgvFxbZTYB\n\tlnzw1RLY/RzZemLipgZnsdc3+ru8snkh4Xa6DoTvDh2ReFMofer95P+sbib+ZSzFOVAK\n\tChGm471ErY57vqsVFPccwcP7e2sHeb0jBTv4MkTWpMCFnIsq/YNk0H5DvtZf0Rm574zi\n\tB5JQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1709029277; x=1709634077;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=vxxl9Rex7NSeC4rjxxHEfVQZ/L4vYyB44xDBAJyyZlA=;\n\tb=sliuv1dSo3j6beHdnZMsL8dA2fJ4triu0G1OC3RIzn3iUNFBst4QWUB1XAiSHMjiyW\n\t7hAROFaN2ExLXea4xkFDeP2hxbBVHdvxGuFfYABAF/6uiH5Q44RZvbFbfqjVHCDwbqgM\n\t+XySfLenYVJ/jSuR1fN+NbX504UGohWUjT5XEG7SErkvT1gAFCvP+a3yyQB1mQ6W+lpd\n\tfGdlq1wGhIM80PoNVamchGXmVNBjzIzBfZSHIa4M2JuDiA5yawkMNLqsai6Y8cMGYYAw\n\tf8f9N9RP4P5xuR4gNR+zP3o7v7cvdbjDKmLzGaLV6a1XOyXM6/k5Wdl8LJUrL4W12qe+\n\tWVmA==","X-Gm-Message-State":"AOJu0YxJN2oKHN5cHPKO3zG+vIRILzTUE+88zxQA542G6ysjl/jIx8jx\n\tr1PhSPb3yMOSTlSquBEU2dHrJOU2h81AYEDcPF/UjgxyIZcztFzVQc2PeSuHoEBktsSzUTlVp2n\n\tYiPPcK+EmCv0oAnN/N7Mn/KQ1ZT8qBllcZ2gvxYyWGL04kVC0Vks=","X-Google-Smtp-Source":"AGHT+IF4307xGRJsAbq5I9oW5d5BXlSYJNS/f+vxV2XPuShz00dSmtVflGoiUfCELc+24QTXY6rnNnti7Jb9HU+3SJY=","X-Received":"by 2002:a25:ae67:0:b0:dc7:4546:d107 with SMTP id\n\tg39-20020a25ae67000000b00dc74546d107mr1767573ybe.23.1709029276550;\n\tTue, 27 Feb 2024 02:21:16 -0800 (PST)","MIME-Version":"1.0","References":"<20240215132710.810-1-naush@raspberrypi.com>\n\t<20240215132710.810-3-naush@raspberrypi.com>\n\t<74nstgdru2jpkry22imfmrcelvvzkfektet6cdo72gl5hqgz6t@vr6q6rtvti27>","In-Reply-To":"<74nstgdru2jpkry22imfmrcelvvzkfektet6cdo72gl5hqgz6t@vr6q6rtvti27>","From":"Naushir Patuck ","Date":"Tue, 27 Feb 2024 10:20:40 +0000","Message-ID":"","Subject":"Re: [PATCH v1 2/2] libcamera: formats: Add PiSP specific image and\n\tconfig buffer formats","To":"Jacopo Mondi ","Content-Type":"text/plain; charset=\"UTF-8\"","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28790,"web_url":"https://patchwork.libcamera.org/comment/28790/","msgid":"","date":"2024-02-28T11:14:20","subject":"Re: [PATCH v1 2/2] libcamera: formats: Add PiSP specific image and\n\tconfig buffer formats","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Naush\n\nOn Tue, Feb 27, 2024 at 10:20:40AM +0000, Naushir Patuck wrote:\n> Hi Jacopo,\n>\n> Thanks for the feedback, and sending the updates to the DRM mailing list!\n>\n\nAs usual, nothing's easy :)\n\n> On Mon, 26 Feb 2024 at 13:49, Jacopo Mondi\n> wrote:\n> >\n> > Hi Naush\n> >\n> > On Thu, Feb 15, 2024 at 01:27:10PM +0000, Naushir Patuck wrote:\n> > > Add the Raspberry Pi 5 PiSP specific compressed Bayer format types 1/2:\n> > > - V4L2_PIX_FMT_PISP_COMP1_xxx\n> > > - V4L2_PIX_FMT_PISP_COMP2_xxx\n> > >\n> > > Add the Raspberry Pi 5 PiSP Backend config format:\n> > > - V4L2_META_FMT_RPI_BE_CFG\n> > >\n> > > Additionally, we also extend libcamera format handlers to support 16-bit\n> > > Bayer formats across the media bus.\n> > >\n> > > Signed-off-by: Naushir Patuck \n> > > ---\n> > > include/libcamera/internal/bayer_format.h | 2 +\n> > > include/linux/drm_fourcc.h | 4 ++\n> > > include/linux/videodev2.h | 15 +++++++\n> > > src/libcamera/bayer_format.cpp | 18 ++++++++\n> > > src/libcamera/formats.cpp | 51 ++++++++++++++++++++++-\n> > > src/libcamera/formats.yaml | 16 +++++++\n> > > src/libcamera/v4l2_pixelformat.cpp | 10 +++++\n> > > src/libcamera/v4l2_subdevice.cpp | 4 ++\n> > > 8 files changed, 119 insertions(+), 1 deletion(-)\n> > >\n> > > diff --git a/include/libcamera/internal/bayer_format.h b/include/libcamera/internal/bayer_format.h\n> > > index 78ba3969913d..164743f7e9f6 100644\n> > > --- a/include/libcamera/internal/bayer_format.h\n> > > +++ b/include/libcamera/internal/bayer_format.h\n> > > @@ -34,6 +34,8 @@ public:\n> > > None = 0,\n> > > CSI2 = 1,\n> > > IPU3 = 2,\n> > > + PISP1 = 3,\n> > > + PISP2 = 4,\n> > > };\n> > >\n> > > constexpr BayerFormat()\n> > > diff --git a/include/linux/drm_fourcc.h b/include/linux/drm_fourcc.h\n> > > index 4ee421b95730..eff27fbe5a1e 100644\n> > > --- a/include/linux/drm_fourcc.h\n> > > +++ b/include/linux/drm_fourcc.h\n> > > @@ -490,6 +490,7 @@ extern \"C\" {\n> > > #define DRM_FORMAT_MOD_VENDOR_ALLWINNER 0x09\n> > > #define DRM_FORMAT_MOD_VENDOR_AMLOGIC 0x0a\n> > > #define DRM_FORMAT_MOD_VENDOR_MIPI 0x0b\n> > > +#define DRM_FORMAT_MOD_VENDOR_RPI 0x0c\n> > >\n> > > /* add more to the end as needed */\n> > >\n> > > @@ -1670,6 +1671,9 @@ drm_fourcc_canonicalize_nvidia_format_mod(__u64 modifier)\n> > > */\n> > > #define MIPI_FORMAT_MOD_CSI2_PACKED fourcc_mod_code(MIPI, 1)\n> > >\n> > > +#define PISP_FORMAT_MOD_COMPRESS_MODE1 fourcc_mod_code(RPI, 1)\n> > > +#define PISP_FORMAT_MOD_COMPRESS_MODE2 fourcc_mod_code(RPI, 2)\n> > > +\n> >\n> > I'll send a patch to DRM/KMS for these formats as well\n> >\n> > > #if defined(__cplusplus)\n> > > }\n> > > #endif\n> > > diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h\n> > > index ba48d2c89726..59af6f794680 100644\n> > > --- a/include/linux/videodev2.h\n> > > +++ b/include/linux/videodev2.h\n> > > @@ -789,6 +789,18 @@ struct v4l2_pix_format {\n> > > #define V4L2_PIX_FMT_IPU3_SGRBG10 v4l2_fourcc('i', 'p', '3', 'G') /* IPU3 packed 10-bit GRBG bayer */\n> > > #define V4L2_PIX_FMT_IPU3_SRGGB10 v4l2_fourcc('i', 'p', '3', 'r') /* IPU3 packed 10-bit RGGB bayer */\n> > >\n> > > +/* The pixel format for all our buffers (the precise format is found in the config buffer). */\n> > > +#define V4L2_PIX_FMT_PISP_COMP1_RGGB v4l2_fourcc('P', 'C', '1', 'R')\n> > > +#define V4L2_PIX_FMT_PISP_COMP1_GRBG v4l2_fourcc('P', 'C', '1', 'G')\n> > > +#define V4L2_PIX_FMT_PISP_COMP1_GBRG v4l2_fourcc('P', 'C', '1', 'g')\n> > > +#define V4L2_PIX_FMT_PISP_COMP1_BGGR v4l2_fourcc('P', 'C', '1', 'B')\n> > > +#define V4L2_PIX_FMT_PISP_COMP1_MONO v4l2_fourcc('P', 'C', '1', 'M')\n> > > +#define V4L2_PIX_FMT_PISP_COMP2_RGGB v4l2_fourcc('P', 'C', '2', 'R')\n> > > +#define V4L2_PIX_FMT_PISP_COMP2_GRBG v4l2_fourcc('P', 'C', '2', 'G')\n> > > +#define V4L2_PIX_FMT_PISP_COMP2_GBRG v4l2_fourcc('P', 'C', '2', 'g')\n> > > +#define V4L2_PIX_FMT_PISP_COMP2_BGGR v4l2_fourcc('P', 'C', '2', 'B')\n> > > +#define V4L2_PIX_FMT_PISP_COMP2_MONO v4l2_fourcc('P', 'C', '2', 'M')\n> > > +\n> >\n> > I would use here what has been sent upstream for the BE support\n> >\n> > /* Raspberry Pi PiSP compressed formats. */\n> > #define V4L2_PIX_FMT_PISP_COMP1_RGGB v4l2_fourcc('P', 'C', '1', 'R') /* PiSP 8-bit mode 1 compressed RGGB bayer */\n> > #define V4L2_PIX_FMT_PISP_COMP1_GRBG v4l2_fourcc('P', 'C', '1', 'G') /* PiSP 8-bit mode 1 compressed GRBG bayer */\n> > #define V4L2_PIX_FMT_PISP_COMP1_GBRG v4l2_fourcc('P', 'C', '1', 'g') /* PiSP 8-bit mode 1 compressed GBRG bayer */\n> > #define V4L2_PIX_FMT_PISP_COMP1_BGGR v4l2_fourcc('P', 'C', '1', 'B') /* PiSP 8-bit mode 1 compressed BGGR bayer */\n> > #define V4L2_PIX_FMT_PISP_COMP1_MONO v4l2_fourcc('P', 'C', '1', 'M') /* PiSP 8-bit mode 1 compressed monochrome */\n> > #define V4L2_PIX_FMT_PISP_COMP2_RGGB v4l2_fourcc('P', 'C', '2', 'R') /* PiSP 8-bit mode 2 compressed RGGB bayer */\n> > #define V4L2_PIX_FMT_PISP_COMP2_GRBG v4l2_fourcc('P', 'C', '2', 'G') /* PiSP 8-bit mode 2 compressed GRBG bayer */\n> > #define V4L2_PIX_FMT_PISP_COMP2_GBRG v4l2_fourcc('P', 'C', '2', 'g') /* PiSP 8-bit mode 2 compressed GBRG bayer */\n> > #define V4L2_PIX_FMT_PISP_COMP2_BGGR v4l2_fourcc('P', 'C', '2', 'B') /* PiSP 8-bit mode 2 compressed BGGR bayer */\n> > #define V4L2_PIX_FMT_PISP_COMP2_MONO v4l2_fourcc('P', 'C', '2', 'M') /* PiSP 8-bit mode 2 compressed monochrome */\n> >\n> > To make it easier to update it when these will land in upstream\n>\n> Ack.\n>\n> >\n> > > /* SDR formats - used only for Software Defined Radio devices */\n> > > #define V4L2_SDR_FMT_CU8 v4l2_fourcc('C', 'U', '0', '8') /* IQ u8 */\n> > > #define V4L2_SDR_FMT_CU16LE v4l2_fourcc('C', 'U', '1', '6') /* IQ u16le */\n> > > @@ -818,6 +830,9 @@ struct v4l2_pix_format {\n> > > #define V4L2_META_FMT_RK_ISP1_PARAMS v4l2_fourcc('R', 'K', '1', 'P') /* Rockchip ISP1 3A Parameters */\n> > > #define V4L2_META_FMT_RK_ISP1_STAT_3A v4l2_fourcc('R', 'K', '1', 'S') /* Rockchip ISP1 3A Statistics */\n> > >\n> > > +/* Vendor specific - used for RaspberryPi PiSP */\n> > > +#define V4L2_META_FMT_RPI_BE_CFG v4l2_fourcc('R', 'P', 'B', 'C') /* PiSP BE configuration */\n> > > /* priv field value to indicates that subsequent fields are valid. */\n> > > #define V4L2_PIX_FMT_PRIV_MAGIC 0xfeedcafe\n> > >\n> > > diff --git a/src/libcamera/bayer_format.cpp b/src/libcamera/bayer_format.cpp\n> > > index 20aedfa6d925..333b1117f531 100644\n> > > --- a/src/libcamera/bayer_format.cpp\n> > > +++ b/src/libcamera/bayer_format.cpp\n> > > @@ -164,6 +164,14 @@ const std::map bayerToFormat{\n> > > { formats::SGRBG16, V4L2PixelFormat(V4L2_PIX_FMT_SGRBG16) } },\n> > > { { BayerFormat::RGGB, 16, BayerFormat::Packing::None },\n> > > { formats::SRGGB16, V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16) } },\n> > > + { { BayerFormat::BGGR, 16, BayerFormat::Packing::PISP1 },\n> > > + { formats::BGGR16_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_BGGR) } },\n> > > + { { BayerFormat::GBRG, 16, BayerFormat::Packing::PISP1 },\n> > > + { formats::GBRG16_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GBRG) } },\n> > > + { { BayerFormat::GRBG, 16, BayerFormat::Packing::PISP1 },\n> > > + { formats::GRBG16_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GRBG) } },\n> > > + { { BayerFormat::RGGB, 16, BayerFormat::Packing::PISP1 },\n> > > + { formats::RGGB16_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_RGGB) } },\n> > > { { BayerFormat::MONO, 8, BayerFormat::Packing::None },\n> > > { formats::R8, V4L2PixelFormat(V4L2_PIX_FMT_GREY) } },\n> > > { { BayerFormat::MONO, 10, BayerFormat::Packing::None },\n> > > @@ -174,6 +182,8 @@ const std::map bayerToFormat{\n> > > { formats::R12, V4L2PixelFormat(V4L2_PIX_FMT_Y12) } },\n> > > { { BayerFormat::MONO, 16, BayerFormat::Packing::None },\n> > > { formats::R16, V4L2PixelFormat(V4L2_PIX_FMT_Y16) } },\n> > > + { { BayerFormat::MONO, 16, BayerFormat::Packing::PISP1 },\n> > > + { formats::MONO_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_MONO) } },\n> >\n> > Should the COMP2 versions be added as well ?\n>\n> To be honest, I doubt we will ever use COMP2, COMP1 is the better\n> option almost always. But I'll add it for completeness.\n>\n\nIf you don't think they're useful feel free to skip them or add them\nlater\n\n>\n> >\n> > > };\n> > >\n> > > const std::unordered_map mbusCodeToBayer{\n> > > @@ -209,6 +219,10 @@ const std::unordered_map mbusCodeToBayer{\n> > > { MEDIA_BUS_FMT_SGBRG16_1X16, { BayerFormat::GBRG, 16, BayerFormat::Packing::None } },\n> > > { MEDIA_BUS_FMT_SGRBG16_1X16, { BayerFormat::GRBG, 16, BayerFormat::Packing::None } },\n> > > { MEDIA_BUS_FMT_SRGGB16_1X16, { BayerFormat::RGGB, 16, BayerFormat::Packing::None } },\n> > > + { MEDIA_BUS_FMT_SBGGR16_1X16, { BayerFormat::BGGR, 16, BayerFormat::Packing::PISP1 } },\n> > > + { MEDIA_BUS_FMT_SGBRG16_1X16, { BayerFormat::GBRG, 16, BayerFormat::Packing::PISP1 } },\n> > > + { MEDIA_BUS_FMT_SGRBG16_1X16, { BayerFormat::GRBG, 16, BayerFormat::Packing::PISP1 } },\n> > > + { MEDIA_BUS_FMT_SRGGB16_1X16, { BayerFormat::RGGB, 16, BayerFormat::Packing::PISP1 } },\n> >\n> > mbusCodeToBayer is an unordered_map which\n> > \"is an associative container that contains key-value pairs with unique\n> > keys.\"\n> >\n> > I'm afraid adding a new entry with the same key overwrite the existing\n> > one ?\n>\n> Oops you are right! I think this came about from when we originally\n> had MBUS codes for the PISP comp formats, and those subsequently got\n> removed.\n>\n> >\n> > We already had issues with this map, as we had similar problems with the\n> > _LE and _BE version of RGB565 which resolv to the same BayerFormat\n> >\n> > { MEDIA_BUS_FMT_SBGGR10_2X8_PADHI_BE, { BayerFormat::BGGR, 10, BayerFormat::Packing::None } },\n> > { MEDIA_BUS_FMT_SBGGR10_2X8_PADHI_LE, { BayerFormat::BGGR, 10, BayerFormat::Packing::None } },\n> > { MEDIA_BUS_FMT_SBGGR10_2X8_PADLO_BE, { BayerFormat::BGGR, 10, BayerFormat::Packing::None } },\n> > { MEDIA_BUS_FMT_SBGGR10_2X8_PADLO_LE, { BayerFormat::BGGR, 10, BayerFormat::Packing::None } },\n> >\n> > In general, we know that resolving mbus_codes to a unique format is\n> > ill-defined. How are you using these entries in the pi5 support ?\n>\n> As luck would have it, the Pi 5 pipeline handler never calls\n> BayerFormat::fromMbusCode() with a PISP_COMP type format so the code\n> never gets exercised and goes wrong!\n>\n> I think the right solution is to remove MEDIA_BUS_FMT_* for the\n> PISP_COMP formats as they genuinely don't have MBUS codes associated\n> with them.\n>\n\nNot 100% sure I get this. Are you proposing not to add the PISP1\ncompressed formats to the mbusCodeToBayer map ?\n\n> >\n> > > { MEDIA_BUS_FMT_Y8_1X8, { BayerFormat::MONO, 8, BayerFormat::Packing::None } },\n> > > { MEDIA_BUS_FMT_Y10_1X10, { BayerFormat::MONO, 10, BayerFormat::Packing::None } },\n> > > { MEDIA_BUS_FMT_Y12_1X12, { BayerFormat::MONO, 12, BayerFormat::Packing::None } },\n> > > @@ -303,6 +317,10 @@ std::ostream &operator<<(std::ostream &out, const BayerFormat &f)\n> > > out << \"-CSI2P\";\n> > > else if (f.packing == BayerFormat::Packing::IPU3)\n> > > out << \"-IPU3P\";\n> > > + else if (f.packing == BayerFormat::Packing::PISP1)\n> > > + out << \"-PISP1\";\n> > > + else if (f.packing == BayerFormat::Packing::PISP2)\n> > > + out << \"-PISP2\";\n> > >\n> > > return out;\n> > > }\n> > > diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp\n> > > index a674f4179cc8..e603a7eda579 100644\n> > > --- a/src/libcamera/formats.cpp\n> > > +++ b/src/libcamera/formats.cpp\n> > > @@ -547,6 +547,16 @@ const std::map pixelFormatInfo{\n> > > .pixelsPerGroup = 1,\n> > > .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},\n> > > } },\n> > > + { formats::MONO_PISP_COMP1, {\n> > > + .name = \"MONO_PISP_COMP1\",\n> > > + .format = formats::MONO_PISP_COMP1,\n> > > + .v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_MONO), },\n> > > + .bitsPerPixel = 16,\n> > > + .colourEncoding = PixelFormatInfo::ColourEncodingYUV,\n> > > + .packed = true,\n> > > + .pixelsPerGroup = 1,\n> > > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},\n> > > + } },\n> > >\n> > > /* Bayer formats. */\n> > > { formats::SBGGR8, {\n> > > @@ -910,7 +920,46 @@ const std::map pixelFormatInfo{\n> > > .pixelsPerGroup = 25,\n> > > .planes = {{ { 32, 1 }, { 0, 0 }, { 0, 0 } }},\n> > > } },\n> > > -\n> > > + { formats::BGGR16_PISP_COMP1, {\n> > > + .name = \"BGGR16_PISP_COMP1\",\n> > > + .format = formats::BGGR16_PISP_COMP1,\n> > > + .v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_BGGR), },\n> > > + .bitsPerPixel = 16,\n> > > + .colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> > > + .packed = true,\n> > > + .pixelsPerGroup = 2,\n> > > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},\n> > > + } },\n> >\n> > For regular 16-bit formats this should be\n> >\n> > .pixelsPerGroup = 2,\n> > .planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},\n> >\n> > As 2 pixels of 16 bits each consume 4 bytes. Is this {2, 1}\n> > intentional and due to compression ?\n>\n> Yes, I used {2, 1} to denote the 8-bit/sample after compression. Do\n> you think I should change to {4, 1}?\n\nDepends on how many bytes the format actually consumes :)\n\nFor 16bpp RAW formats, we have pixelsPerGroup = 2 and each sample\nconsumes 2 bytes.\n\nAccording to the pixelsPerGroup definition:\n\n * A pixel group is defined as the minimum number of pixels (including padding)\n * necessary in a row when the image has only one column of effective pixels.\n\nSo for a RAW Bayer formats, to get a complete full-color \"superpixel\"\nwe need at least two pixels per row.\n\n RG\n GB\n\nEach \"RG\" or \"GB\" pair is composed by 2 16-bit samples, for a total of\n32bits of data, from which the {4, 1} value in planes[0]\n\nAccording to \"Table 2\", Chapter 5 of the PiSP datasheet, the\ncompressed formats are described as\n\n 8-bits per pixel single channel compressed with mode n, for n = 1, 2, 3\n\nWhich suggests each \"RG\" or \"GB\" pair is actually 2 samples of 8 bits\neach, for a plane size of {2, 1}.\n\nIs this correct ? Is the actual .bitsPerPixel value for the compressed\nformats 8 instead of 16 then ?\n\nThanks\n j\n\n>\n> Regards,\n> Naush\n>\n>\n> >\n> > > + { formats::GBRG16_PISP_COMP1, {\n> > > + .name = \"GBRG16_PISP_COMP1\",\n> > > + .format = formats::GBRG16_PISP_COMP1,\n> > > + .v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GBRG), },\n> > > + .bitsPerPixel = 16,\n> > > + .colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> > > + .packed = true,\n> > > + .pixelsPerGroup = 2,\n> > > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},\n> > > + } },\n> > > + { formats::GRBG16_PISP_COMP1, {\n> > > + .name = \"GRBG16_PISP_COMP1\",\n> > > + .format = formats::GRBG16_PISP_COMP1,\n> > > + .v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GRBG), },\n> > > + .bitsPerPixel = 16,\n> > > + .colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> > > + .packed = true,\n> > > + .pixelsPerGroup = 2,\n> > > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},\n> > > + } },\n> > > + { formats::RGGB16_PISP_COMP1, {\n> > > + .name = \"RGGB16_PISP_COMP1\",\n> > > + .format = formats::RGGB16_PISP_COMP1,\n> > > + .v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_RGGB), },\n> > > + .bitsPerPixel = 16,\n> > > + .colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> > > + .packed = true,\n> > > + .pixelsPerGroup = 2,\n> > > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},\n> > > + } },\n> > > /* Compressed formats. */\n> > > { formats::MJPEG, {\n> > > .name = \"MJPEG\",\n> > > diff --git a/src/libcamera/formats.yaml b/src/libcamera/formats.yaml\n> > > index bde2cc803b98..f6df721243d0 100644\n> > > --- a/src/libcamera/formats.yaml\n> > > +++ b/src/libcamera/formats.yaml\n> > > @@ -190,4 +190,20 @@ formats:\n> > > - SBGGR10_IPU3:\n> > > fourcc: DRM_FORMAT_SBGGR10\n> > > mod: IPU3_FORMAT_MOD_PACKED\n> > > +\n> > > + - RGGB16_PISP_COMP1:\n> > > + fourcc: DRM_FORMAT_SRGGB16\n> > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1\n> > > + - GRBG16_PISP_COMP1:\n> > > + fourcc: DRM_FORMAT_SGRBG16\n> > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1\n> > > + - GBRG16_PISP_COMP1:\n> > > + fourcc: DRM_FORMAT_SGBRG16\n> > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1\n> > > + - BGGR16_PISP_COMP1:\n> > > + fourcc: DRM_FORMAT_SBGGR16\n> > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1\n> > > + - MONO_PISP_COMP1:\n> > > + fourcc: DRM_FORMAT_R16\n> > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1\n> > > ...\n> > > diff --git a/src/libcamera/v4l2_pixelformat.cpp b/src/libcamera/v4l2_pixelformat.cpp\n> > > index efb6f2940235..47baaf60199d 100644\n> > > --- a/src/libcamera/v4l2_pixelformat.cpp\n> > > +++ b/src/libcamera/v4l2_pixelformat.cpp\n> > > @@ -207,6 +207,16 @@ const std::map vpf2pf{\n> > > { formats::SGRBG16, \"16-bit Bayer GRGR/BGBG\" } },\n> > > { V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16),\n> > > { formats::SRGGB16, \"16-bit Bayer RGRG/GBGB\" } },\n> > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_BGGR),\n> > > + { formats::BGGR16_PISP_COMP1, \"16-bit Bayer BGBG/GRGR PiSP Compress Mode 1\" } },\n> > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GBRG),\n> > > + { formats::GBRG16_PISP_COMP1, \"16-bit Bayer GBGB/RGRG PiSP Compress Mode 1\" } },\n> > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GRBG),\n> > > + { formats::GRBG16_PISP_COMP1, \"16-bit Bayer GRGR/BGBG PiSP Compress Mode 1\" } },\n> > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_RGGB),\n> > > + { formats::RGGB16_PISP_COMP1, \"16-bit Bayer RGRG/GBGB PiSP Compress Mode 1\" } },\n> > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_MONO),\n> > > + { formats::MONO_PISP_COMP1, \"16-bit Mono PiSP Compress Mode 1\" } },\n> > >\n> > > /* Compressed formats. */\n> > > { V4L2PixelFormat(V4L2_PIX_FMT_MJPEG),\n> > > diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp\n> > > index 6d0785b7b484..aea90abaf9ef 100644\n> > > --- a/src/libcamera/v4l2_subdevice.cpp\n> > > +++ b/src/libcamera/v4l2_subdevice.cpp\n> > > @@ -134,6 +134,10 @@ const std::map formatInfoMap = {\n> > > { MEDIA_BUS_FMT_SGBRG12_1X12, { 12, \"SGBRG12_1X12\", PixelFormatInfo::ColourEncodingRAW } },\n> > > { MEDIA_BUS_FMT_SGRBG12_1X12, { 12, \"SGRBG12_1X12\", PixelFormatInfo::ColourEncodingRAW } },\n> > > { MEDIA_BUS_FMT_SRGGB12_1X12, { 12, \"SRGGB12_1X12\", PixelFormatInfo::ColourEncodingRAW } },\n> > > + { MEDIA_BUS_FMT_SBGGR16_1X16, { 16, \"SBGGR16_1x16\", PixelFormatInfo::ColourEncodingRAW } },\n> > > + { MEDIA_BUS_FMT_SGBRG16_1X16, { 16, \"SGBRG16_1x16\", PixelFormatInfo::ColourEncodingRAW } },\n> > > + { MEDIA_BUS_FMT_SGRBG16_1X16, { 16, \"SGRBG16_1x16\", PixelFormatInfo::ColourEncodingRAW } },\n> > > + { MEDIA_BUS_FMT_SRGGB16_1X16, { 16, \"SRGGB16_1x16\", PixelFormatInfo::ColourEncodingRAW } },\n> > > /* \\todo Clarify colour encoding for HSV formats */\n> > > { MEDIA_BUS_FMT_AHSV8888_1X32, { 32, \"AHSV8888_1X32\", PixelFormatInfo::ColourEncodingRGB } },\n> > > { MEDIA_BUS_FMT_JPEG_1X8, { 8, \"JPEG_1X8\", PixelFormatInfo::ColourEncodingYUV } },\n> > > --\n> > > 2.34.1\n> > >","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id C0B34BD160\n\tfor ;\n\tWed, 28 Feb 2024 11:14:24 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 727856286D;\n\tWed, 28 Feb 2024 12:14:24 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9E2E1627F9\n\tfor ;\n\tWed, 28 Feb 2024 12:14:22 +0100 (CET)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E90F92B3;\n\tWed, 28 Feb 2024 12:14:09 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"mPMf8wJW\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1709118850;\n\tbh=TovBKJA1EWTPpvWK2XXT8p4NuAyZOrPikoR48DRY3pU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=mPMf8wJWPzkQ8R4W21C6C1AwQovEa63qYSnMCXhvBo/gvj+435O1WOZalKHXmOkP4\n\tjPL1hYs3DHRC4VkTmFv123TnHMmRBnwdGE3d++A3j/c7AHQwUA5REj/UGjmRVh+WrE\n\tINUW1hBI0AwSl5FY/VwJnp+ixDhp+NEgC4z33QU4=","Date":"Wed, 28 Feb 2024 12:14:20 +0100","From":"Jacopo Mondi ","To":"Naushir Patuck ","Subject":"Re: [PATCH v1 2/2] libcamera: formats: Add PiSP specific image and\n\tconfig buffer formats","Message-ID":"","References":"<20240215132710.810-1-naush@raspberrypi.com>\n\t<20240215132710.810-3-naush@raspberrypi.com>\n\t<74nstgdru2jpkry22imfmrcelvvzkfektet6cdo72gl5hqgz6t@vr6q6rtvti27>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"Jacopo Mondi ,\n\tlibcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28794,"web_url":"https://patchwork.libcamera.org/comment/28794/","msgid":"","date":"2024-02-28T16:45:54","subject":"Re: [PATCH v1 2/2] libcamera: formats: Add PiSP specific image and\n\tconfig buffer formats","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Jacopo,\n\nOn Wed, 28 Feb 2024 at 11:14, Jacopo Mondi\n wrote:\n>\n> Hi Naush\n>\n> On Tue, Feb 27, 2024 at 10:20:40AM +0000, Naushir Patuck wrote:\n> > Hi Jacopo,\n> >\n> > Thanks for the feedback, and sending the updates to the DRM mailing list!\n> >\n>\n> As usual, nothing's easy :)\n>\n> > On Mon, 26 Feb 2024 at 13:49, Jacopo Mondi\n> > wrote:\n> > >\n> > > Hi Naush\n> > >\n> > > On Thu, Feb 15, 2024 at 01:27:10PM +0000, Naushir Patuck wrote:\n> > > > Add the Raspberry Pi 5 PiSP specific compressed Bayer format types 1/2:\n> > > > - V4L2_PIX_FMT_PISP_COMP1_xxx\n> > > > - V4L2_PIX_FMT_PISP_COMP2_xxx\n> > > >\n> > > > Add the Raspberry Pi 5 PiSP Backend config format:\n> > > > - V4L2_META_FMT_RPI_BE_CFG\n> > > >\n> > > > Additionally, we also extend libcamera format handlers to support 16-bit\n> > > > Bayer formats across the media bus.\n> > > >\n> > > > Signed-off-by: Naushir Patuck \n> > > > ---\n> > > > include/libcamera/internal/bayer_format.h | 2 +\n> > > > include/linux/drm_fourcc.h | 4 ++\n> > > > include/linux/videodev2.h | 15 +++++++\n> > > > src/libcamera/bayer_format.cpp | 18 ++++++++\n> > > > src/libcamera/formats.cpp | 51 ++++++++++++++++++++++-\n> > > > src/libcamera/formats.yaml | 16 +++++++\n> > > > src/libcamera/v4l2_pixelformat.cpp | 10 +++++\n> > > > src/libcamera/v4l2_subdevice.cpp | 4 ++\n> > > > 8 files changed, 119 insertions(+), 1 deletion(-)\n> > > >\n> > > > diff --git a/include/libcamera/internal/bayer_format.h b/include/libcamera/internal/bayer_format.h\n> > > > index 78ba3969913d..164743f7e9f6 100644\n> > > > --- a/include/libcamera/internal/bayer_format.h\n> > > > +++ b/include/libcamera/internal/bayer_format.h\n> > > > @@ -34,6 +34,8 @@ public:\n> > > > None = 0,\n> > > > CSI2 = 1,\n> > > > IPU3 = 2,\n> > > > + PISP1 = 3,\n> > > > + PISP2 = 4,\n> > > > };\n> > > >\n> > > > constexpr BayerFormat()\n> > > > diff --git a/include/linux/drm_fourcc.h b/include/linux/drm_fourcc.h\n> > > > index 4ee421b95730..eff27fbe5a1e 100644\n> > > > --- a/include/linux/drm_fourcc.h\n> > > > +++ b/include/linux/drm_fourcc.h\n> > > > @@ -490,6 +490,7 @@ extern \"C\" {\n> > > > #define DRM_FORMAT_MOD_VENDOR_ALLWINNER 0x09\n> > > > #define DRM_FORMAT_MOD_VENDOR_AMLOGIC 0x0a\n> > > > #define DRM_FORMAT_MOD_VENDOR_MIPI 0x0b\n> > > > +#define DRM_FORMAT_MOD_VENDOR_RPI 0x0c\n> > > >\n> > > > /* add more to the end as needed */\n> > > >\n> > > > @@ -1670,6 +1671,9 @@ drm_fourcc_canonicalize_nvidia_format_mod(__u64 modifier)\n> > > > */\n> > > > #define MIPI_FORMAT_MOD_CSI2_PACKED fourcc_mod_code(MIPI, 1)\n> > > >\n> > > > +#define PISP_FORMAT_MOD_COMPRESS_MODE1 fourcc_mod_code(RPI, 1)\n> > > > +#define PISP_FORMAT_MOD_COMPRESS_MODE2 fourcc_mod_code(RPI, 2)\n> > > > +\n> > >\n> > > I'll send a patch to DRM/KMS for these formats as well\n> > >\n> > > > #if defined(__cplusplus)\n> > > > }\n> > > > #endif\n> > > > diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h\n> > > > index ba48d2c89726..59af6f794680 100644\n> > > > --- a/include/linux/videodev2.h\n> > > > +++ b/include/linux/videodev2.h\n> > > > @@ -789,6 +789,18 @@ struct v4l2_pix_format {\n> > > > #define V4L2_PIX_FMT_IPU3_SGRBG10 v4l2_fourcc('i', 'p', '3', 'G') /* IPU3 packed 10-bit GRBG bayer */\n> > > > #define V4L2_PIX_FMT_IPU3_SRGGB10 v4l2_fourcc('i', 'p', '3', 'r') /* IPU3 packed 10-bit RGGB bayer */\n> > > >\n> > > > +/* The pixel format for all our buffers (the precise format is found in the config buffer). */\n> > > > +#define V4L2_PIX_FMT_PISP_COMP1_RGGB v4l2_fourcc('P', 'C', '1', 'R')\n> > > > +#define V4L2_PIX_FMT_PISP_COMP1_GRBG v4l2_fourcc('P', 'C', '1', 'G')\n> > > > +#define V4L2_PIX_FMT_PISP_COMP1_GBRG v4l2_fourcc('P', 'C', '1', 'g')\n> > > > +#define V4L2_PIX_FMT_PISP_COMP1_BGGR v4l2_fourcc('P', 'C', '1', 'B')\n> > > > +#define V4L2_PIX_FMT_PISP_COMP1_MONO v4l2_fourcc('P', 'C', '1', 'M')\n> > > > +#define V4L2_PIX_FMT_PISP_COMP2_RGGB v4l2_fourcc('P', 'C', '2', 'R')\n> > > > +#define V4L2_PIX_FMT_PISP_COMP2_GRBG v4l2_fourcc('P', 'C', '2', 'G')\n> > > > +#define V4L2_PIX_FMT_PISP_COMP2_GBRG v4l2_fourcc('P', 'C', '2', 'g')\n> > > > +#define V4L2_PIX_FMT_PISP_COMP2_BGGR v4l2_fourcc('P', 'C', '2', 'B')\n> > > > +#define V4L2_PIX_FMT_PISP_COMP2_MONO v4l2_fourcc('P', 'C', '2', 'M')\n> > > > +\n> > >\n> > > I would use here what has been sent upstream for the BE support\n> > >\n> > > /* Raspberry Pi PiSP compressed formats. */\n> > > #define V4L2_PIX_FMT_PISP_COMP1_RGGB v4l2_fourcc('P', 'C', '1', 'R') /* PiSP 8-bit mode 1 compressed RGGB bayer */\n> > > #define V4L2_PIX_FMT_PISP_COMP1_GRBG v4l2_fourcc('P', 'C', '1', 'G') /* PiSP 8-bit mode 1 compressed GRBG bayer */\n> > > #define V4L2_PIX_FMT_PISP_COMP1_GBRG v4l2_fourcc('P', 'C', '1', 'g') /* PiSP 8-bit mode 1 compressed GBRG bayer */\n> > > #define V4L2_PIX_FMT_PISP_COMP1_BGGR v4l2_fourcc('P', 'C', '1', 'B') /* PiSP 8-bit mode 1 compressed BGGR bayer */\n> > > #define V4L2_PIX_FMT_PISP_COMP1_MONO v4l2_fourcc('P', 'C', '1', 'M') /* PiSP 8-bit mode 1 compressed monochrome */\n> > > #define V4L2_PIX_FMT_PISP_COMP2_RGGB v4l2_fourcc('P', 'C', '2', 'R') /* PiSP 8-bit mode 2 compressed RGGB bayer */\n> > > #define V4L2_PIX_FMT_PISP_COMP2_GRBG v4l2_fourcc('P', 'C', '2', 'G') /* PiSP 8-bit mode 2 compressed GRBG bayer */\n> > > #define V4L2_PIX_FMT_PISP_COMP2_GBRG v4l2_fourcc('P', 'C', '2', 'g') /* PiSP 8-bit mode 2 compressed GBRG bayer */\n> > > #define V4L2_PIX_FMT_PISP_COMP2_BGGR v4l2_fourcc('P', 'C', '2', 'B') /* PiSP 8-bit mode 2 compressed BGGR bayer */\n> > > #define V4L2_PIX_FMT_PISP_COMP2_MONO v4l2_fourcc('P', 'C', '2', 'M') /* PiSP 8-bit mode 2 compressed monochrome */\n> > >\n> > > To make it easier to update it when these will land in upstream\n> >\n> > Ack.\n> >\n> > >\n> > > > /* SDR formats - used only for Software Defined Radio devices */\n> > > > #define V4L2_SDR_FMT_CU8 v4l2_fourcc('C', 'U', '0', '8') /* IQ u8 */\n> > > > #define V4L2_SDR_FMT_CU16LE v4l2_fourcc('C', 'U', '1', '6') /* IQ u16le */\n> > > > @@ -818,6 +830,9 @@ struct v4l2_pix_format {\n> > > > #define V4L2_META_FMT_RK_ISP1_PARAMS v4l2_fourcc('R', 'K', '1', 'P') /* Rockchip ISP1 3A Parameters */\n> > > > #define V4L2_META_FMT_RK_ISP1_STAT_3A v4l2_fourcc('R', 'K', '1', 'S') /* Rockchip ISP1 3A Statistics */\n> > > >\n> > > > +/* Vendor specific - used for RaspberryPi PiSP */\n> > > > +#define V4L2_META_FMT_RPI_BE_CFG v4l2_fourcc('R', 'P', 'B', 'C') /* PiSP BE configuration */\n> > > > /* priv field value to indicates that subsequent fields are valid. */\n> > > > #define V4L2_PIX_FMT_PRIV_MAGIC 0xfeedcafe\n> > > >\n> > > > diff --git a/src/libcamera/bayer_format.cpp b/src/libcamera/bayer_format.cpp\n> > > > index 20aedfa6d925..333b1117f531 100644\n> > > > --- a/src/libcamera/bayer_format.cpp\n> > > > +++ b/src/libcamera/bayer_format.cpp\n> > > > @@ -164,6 +164,14 @@ const std::map bayerToFormat{\n> > > > { formats::SGRBG16, V4L2PixelFormat(V4L2_PIX_FMT_SGRBG16) } },\n> > > > { { BayerFormat::RGGB, 16, BayerFormat::Packing::None },\n> > > > { formats::SRGGB16, V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16) } },\n> > > > + { { BayerFormat::BGGR, 16, BayerFormat::Packing::PISP1 },\n> > > > + { formats::BGGR16_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_BGGR) } },\n> > > > + { { BayerFormat::GBRG, 16, BayerFormat::Packing::PISP1 },\n> > > > + { formats::GBRG16_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GBRG) } },\n> > > > + { { BayerFormat::GRBG, 16, BayerFormat::Packing::PISP1 },\n> > > > + { formats::GRBG16_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GRBG) } },\n> > > > + { { BayerFormat::RGGB, 16, BayerFormat::Packing::PISP1 },\n> > > > + { formats::RGGB16_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_RGGB) } },\n> > > > { { BayerFormat::MONO, 8, BayerFormat::Packing::None },\n> > > > { formats::R8, V4L2PixelFormat(V4L2_PIX_FMT_GREY) } },\n> > > > { { BayerFormat::MONO, 10, BayerFormat::Packing::None },\n> > > > @@ -174,6 +182,8 @@ const std::map bayerToFormat{\n> > > > { formats::R12, V4L2PixelFormat(V4L2_PIX_FMT_Y12) } },\n> > > > { { BayerFormat::MONO, 16, BayerFormat::Packing::None },\n> > > > { formats::R16, V4L2PixelFormat(V4L2_PIX_FMT_Y16) } },\n> > > > + { { BayerFormat::MONO, 16, BayerFormat::Packing::PISP1 },\n> > > > + { formats::MONO_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_MONO) } },\n> > >\n> > > Should the COMP2 versions be added as well ?\n> >\n> > To be honest, I doubt we will ever use COMP2, COMP1 is the better\n> > option almost always. But I'll add it for completeness.\n> >\n>\n> If you don't think they're useful feel free to skip them or add them\n> later\n\nI think that's best.\n\n>\n> >\n> > >\n> > > > };\n> > > >\n> > > > const std::unordered_map mbusCodeToBayer{\n> > > > @@ -209,6 +219,10 @@ const std::unordered_map mbusCodeToBayer{\n> > > > { MEDIA_BUS_FMT_SGBRG16_1X16, { BayerFormat::GBRG, 16, BayerFormat::Packing::None } },\n> > > > { MEDIA_BUS_FMT_SGRBG16_1X16, { BayerFormat::GRBG, 16, BayerFormat::Packing::None } },\n> > > > { MEDIA_BUS_FMT_SRGGB16_1X16, { BayerFormat::RGGB, 16, BayerFormat::Packing::None } },\n> > > > + { MEDIA_BUS_FMT_SBGGR16_1X16, { BayerFormat::BGGR, 16, BayerFormat::Packing::PISP1 } },\n> > > > + { MEDIA_BUS_FMT_SGBRG16_1X16, { BayerFormat::GBRG, 16, BayerFormat::Packing::PISP1 } },\n> > > > + { MEDIA_BUS_FMT_SGRBG16_1X16, { BayerFormat::GRBG, 16, BayerFormat::Packing::PISP1 } },\n> > > > + { MEDIA_BUS_FMT_SRGGB16_1X16, { BayerFormat::RGGB, 16, BayerFormat::Packing::PISP1 } },\n> > >\n> > > mbusCodeToBayer is an unordered_map which\n> > > \"is an associative container that contains key-value pairs with unique\n> > > keys.\"\n> > >\n> > > I'm afraid adding a new entry with the same key overwrite the existing\n> > > one ?\n> >\n> > Oops you are right! I think this came about from when we originally\n> > had MBUS codes for the PISP comp formats, and those subsequently got\n> > removed.\n> >\n> > >\n> > > We already had issues with this map, as we had similar problems with the\n> > > _LE and _BE version of RGB565 which resolv to the same BayerFormat\n> > >\n> > > { MEDIA_BUS_FMT_SBGGR10_2X8_PADHI_BE, { BayerFormat::BGGR, 10, BayerFormat::Packing::None } },\n> > > { MEDIA_BUS_FMT_SBGGR10_2X8_PADHI_LE, { BayerFormat::BGGR, 10, BayerFormat::Packing::None } },\n> > > { MEDIA_BUS_FMT_SBGGR10_2X8_PADLO_BE, { BayerFormat::BGGR, 10, BayerFormat::Packing::None } },\n> > > { MEDIA_BUS_FMT_SBGGR10_2X8_PADLO_LE, { BayerFormat::BGGR, 10, BayerFormat::Packing::None } },\n> > >\n> > > In general, we know that resolving mbus_codes to a unique format is\n> > > ill-defined. How are you using these entries in the pi5 support ?\n> >\n> > As luck would have it, the Pi 5 pipeline handler never calls\n> > BayerFormat::fromMbusCode() with a PISP_COMP type format so the code\n> > never gets exercised and goes wrong!\n> >\n> > I think the right solution is to remove MEDIA_BUS_FMT_* for the\n> > PISP_COMP formats as they genuinely don't have MBUS codes associated\n> > with them.\n> >\n>\n> Not 100% sure I get this. Are you proposing not to add the PISP1\n> compressed formats to the mbusCodeToBayer map ?\n\nThat's correct. PISP_COMP formats do not actually have mubs codes as\nthey are not strictly formats used across a bus, only on the output.\nSo we remove them from this table. The Pi 5 pipeline handler will\nnever need to access an mbus code for a PISP_COMP format.\n\n>\n> > >\n> > > > { MEDIA_BUS_FMT_Y8_1X8, { BayerFormat::MONO, 8, BayerFormat::Packing::None } },\n> > > > { MEDIA_BUS_FMT_Y10_1X10, { BayerFormat::MONO, 10, BayerFormat::Packing::None } },\n> > > > { MEDIA_BUS_FMT_Y12_1X12, { BayerFormat::MONO, 12, BayerFormat::Packing::None } },\n> > > > @@ -303,6 +317,10 @@ std::ostream &operator<<(std::ostream &out, const BayerFormat &f)\n> > > > out << \"-CSI2P\";\n> > > > else if (f.packing == BayerFormat::Packing::IPU3)\n> > > > out << \"-IPU3P\";\n> > > > + else if (f.packing == BayerFormat::Packing::PISP1)\n> > > > + out << \"-PISP1\";\n> > > > + else if (f.packing == BayerFormat::Packing::PISP2)\n> > > > + out << \"-PISP2\";\n> > > >\n> > > > return out;\n> > > > }\n> > > > diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp\n> > > > index a674f4179cc8..e603a7eda579 100644\n> > > > --- a/src/libcamera/formats.cpp\n> > > > +++ b/src/libcamera/formats.cpp\n> > > > @@ -547,6 +547,16 @@ const std::map pixelFormatInfo{\n> > > > .pixelsPerGroup = 1,\n> > > > .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},\n> > > > } },\n> > > > + { formats::MONO_PISP_COMP1, {\n> > > > + .name = \"MONO_PISP_COMP1\",\n> > > > + .format = formats::MONO_PISP_COMP1,\n> > > > + .v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_MONO), },\n> > > > + .bitsPerPixel = 16,\n> > > > + .colourEncoding = PixelFormatInfo::ColourEncodingYUV,\n> > > > + .packed = true,\n> > > > + .pixelsPerGroup = 1,\n> > > > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},\n> > > > + } },\n> > > >\n> > > > /* Bayer formats. */\n> > > > { formats::SBGGR8, {\n> > > > @@ -910,7 +920,46 @@ const std::map pixelFormatInfo{\n> > > > .pixelsPerGroup = 25,\n> > > > .planes = {{ { 32, 1 }, { 0, 0 }, { 0, 0 } }},\n> > > > } },\n> > > > -\n> > > > + { formats::BGGR16_PISP_COMP1, {\n> > > > + .name = \"BGGR16_PISP_COMP1\",\n> > > > + .format = formats::BGGR16_PISP_COMP1,\n> > > > + .v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_BGGR), },\n> > > > + .bitsPerPixel = 16,\n> > > > + .colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> > > > + .packed = true,\n> > > > + .pixelsPerGroup = 2,\n> > > > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},\n> > > > + } },\n> > >\n> > > For regular 16-bit formats this should be\n> > >\n> > > .pixelsPerGroup = 2,\n> > > .planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},\n> > >\n> > > As 2 pixels of 16 bits each consume 4 bytes. Is this {2, 1}\n> > > intentional and due to compression ?\n> >\n> > Yes, I used {2, 1} to denote the 8-bit/sample after compression. Do\n> > you think I should change to {4, 1}?\n>\n> Depends on how many bytes the format actually consumes :)\n>\n> For 16bpp RAW formats, we have pixelsPerGroup = 2 and each sample\n> consumes 2 bytes.\n>\n> According to the pixelsPerGroup definition:\n>\n> * A pixel group is defined as the minimum number of pixels (including padding)\n> * necessary in a row when the image has only one column of effective pixels.\n>\n> So for a RAW Bayer formats, to get a complete full-color \"superpixel\"\n> we need at least two pixels per row.\n>\n> RG\n> GB\n>\n> Each \"RG\" or \"GB\" pair is composed by 2 16-bit samples, for a total of\n> 32bits of data, from which the {4, 1} value in planes[0]\n>\n> According to \"Table 2\", Chapter 5 of the PiSP datasheet, the\n> compressed formats are described as\n>\n> 8-bits per pixel single channel compressed with mode n, for n = 1, 2, 3\n>\n> Which suggests each \"RG\" or \"GB\" pair is actually 2 samples of 8 bits\n> each, for a plane size of {2, 1}.\n>\n> Is this correct ? Is the actual .bitsPerPixel value for the compressed\n> formats 8 instead of 16 then ?\n\nThis is correct, each sample is 8-bits so \"RG\"/\"GB\" pairs are 16-bits each.\nSo it sounds like I need to use .planes = {2, 1}, but .bitsPerPixel = 8 maybe?\n\nNaush\n\n>\n> Thanks\n> j\n>\n> >\n> > Regards,\n> > Naush\n> >\n> >\n> > >\n> > > > + { formats::GBRG16_PISP_COMP1, {\n> > > > + .name = \"GBRG16_PISP_COMP1\",\n> > > > + .format = formats::GBRG16_PISP_COMP1,\n> > > > + .v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GBRG), },\n> > > > + .bitsPerPixel = 16,\n> > > > + .colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> > > > + .packed = true,\n> > > > + .pixelsPerGroup = 2,\n> > > > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},\n> > > > + } },\n> > > > + { formats::GRBG16_PISP_COMP1, {\n> > > > + .name = \"GRBG16_PISP_COMP1\",\n> > > > + .format = formats::GRBG16_PISP_COMP1,\n> > > > + .v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GRBG), },\n> > > > + .bitsPerPixel = 16,\n> > > > + .colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> > > > + .packed = true,\n> > > > + .pixelsPerGroup = 2,\n> > > > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},\n> > > > + } },\n> > > > + { formats::RGGB16_PISP_COMP1, {\n> > > > + .name = \"RGGB16_PISP_COMP1\",\n> > > > + .format = formats::RGGB16_PISP_COMP1,\n> > > > + .v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_RGGB), },\n> > > > + .bitsPerPixel = 16,\n> > > > + .colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> > > > + .packed = true,\n> > > > + .pixelsPerGroup = 2,\n> > > > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},\n> > > > + } },\n> > > > /* Compressed formats. */\n> > > > { formats::MJPEG, {\n> > > > .name = \"MJPEG\",\n> > > > diff --git a/src/libcamera/formats.yaml b/src/libcamera/formats.yaml\n> > > > index bde2cc803b98..f6df721243d0 100644\n> > > > --- a/src/libcamera/formats.yaml\n> > > > +++ b/src/libcamera/formats.yaml\n> > > > @@ -190,4 +190,20 @@ formats:\n> > > > - SBGGR10_IPU3:\n> > > > fourcc: DRM_FORMAT_SBGGR10\n> > > > mod: IPU3_FORMAT_MOD_PACKED\n> > > > +\n> > > > + - RGGB16_PISP_COMP1:\n> > > > + fourcc: DRM_FORMAT_SRGGB16\n> > > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1\n> > > > + - GRBG16_PISP_COMP1:\n> > > > + fourcc: DRM_FORMAT_SGRBG16\n> > > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1\n> > > > + - GBRG16_PISP_COMP1:\n> > > > + fourcc: DRM_FORMAT_SGBRG16\n> > > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1\n> > > > + - BGGR16_PISP_COMP1:\n> > > > + fourcc: DRM_FORMAT_SBGGR16\n> > > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1\n> > > > + - MONO_PISP_COMP1:\n> > > > + fourcc: DRM_FORMAT_R16\n> > > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1\n> > > > ...\n> > > > diff --git a/src/libcamera/v4l2_pixelformat.cpp b/src/libcamera/v4l2_pixelformat.cpp\n> > > > index efb6f2940235..47baaf60199d 100644\n> > > > --- a/src/libcamera/v4l2_pixelformat.cpp\n> > > > +++ b/src/libcamera/v4l2_pixelformat.cpp\n> > > > @@ -207,6 +207,16 @@ const std::map vpf2pf{\n> > > > { formats::SGRBG16, \"16-bit Bayer GRGR/BGBG\" } },\n> > > > { V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16),\n> > > > { formats::SRGGB16, \"16-bit Bayer RGRG/GBGB\" } },\n> > > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_BGGR),\n> > > > + { formats::BGGR16_PISP_COMP1, \"16-bit Bayer BGBG/GRGR PiSP Compress Mode 1\" } },\n> > > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GBRG),\n> > > > + { formats::GBRG16_PISP_COMP1, \"16-bit Bayer GBGB/RGRG PiSP Compress Mode 1\" } },\n> > > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GRBG),\n> > > > + { formats::GRBG16_PISP_COMP1, \"16-bit Bayer GRGR/BGBG PiSP Compress Mode 1\" } },\n> > > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_RGGB),\n> > > > + { formats::RGGB16_PISP_COMP1, \"16-bit Bayer RGRG/GBGB PiSP Compress Mode 1\" } },\n> > > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_MONO),\n> > > > + { formats::MONO_PISP_COMP1, \"16-bit Mono PiSP Compress Mode 1\" } },\n> > > >\n> > > > /* Compressed formats. */\n> > > > { V4L2PixelFormat(V4L2_PIX_FMT_MJPEG),\n> > > > diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp\n> > > > index 6d0785b7b484..aea90abaf9ef 100644\n> > > > --- a/src/libcamera/v4l2_subdevice.cpp\n> > > > +++ b/src/libcamera/v4l2_subdevice.cpp\n> > > > @@ -134,6 +134,10 @@ const std::map formatInfoMap = {\n> > > > { MEDIA_BUS_FMT_SGBRG12_1X12, { 12, \"SGBRG12_1X12\", PixelFormatInfo::ColourEncodingRAW } },\n> > > > { MEDIA_BUS_FMT_SGRBG12_1X12, { 12, \"SGRBG12_1X12\", PixelFormatInfo::ColourEncodingRAW } },\n> > > > { MEDIA_BUS_FMT_SRGGB12_1X12, { 12, \"SRGGB12_1X12\", PixelFormatInfo::ColourEncodingRAW } },\n> > > > + { MEDIA_BUS_FMT_SBGGR16_1X16, { 16, \"SBGGR16_1x16\", PixelFormatInfo::ColourEncodingRAW } },\n> > > > + { MEDIA_BUS_FMT_SGBRG16_1X16, { 16, \"SGBRG16_1x16\", PixelFormatInfo::ColourEncodingRAW } },\n> > > > + { MEDIA_BUS_FMT_SGRBG16_1X16, { 16, \"SGRBG16_1x16\", PixelFormatInfo::ColourEncodingRAW } },\n> > > > + { MEDIA_BUS_FMT_SRGGB16_1X16, { 16, \"SRGGB16_1x16\", PixelFormatInfo::ColourEncodingRAW } },\n> > > > /* \\todo Clarify colour encoding for HSV formats */\n> > > > { MEDIA_BUS_FMT_AHSV8888_1X32, { 32, \"AHSV8888_1X32\", PixelFormatInfo::ColourEncodingRGB } },\n> > > > { MEDIA_BUS_FMT_JPEG_1X8, { 8, \"JPEG_1X8\", PixelFormatInfo::ColourEncodingYUV } },\n> > > > --\n> > > > 2.34.1\n> > > >","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id CBF41BE080\n\tfor ;\n\tWed, 28 Feb 2024 16:46:34 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0F82C62868;\n\tWed, 28 Feb 2024 17:46:34 +0100 (CET)","from mail-yw1-x1133.google.com (mail-yw1-x1133.google.com\n\t[IPv6:2607:f8b0:4864:20::1133])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8478761C94\n\tfor ;\n\tWed, 28 Feb 2024 17:46:31 +0100 (CET)","by mail-yw1-x1133.google.com with SMTP id\n\t00721157ae682-6089b64f4eeso56103527b3.2\n\tfor ;\n\tWed, 28 Feb 2024 08:46:31 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"FKudGUxa\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1709138790; x=1709743590;\n\tdarn=lists.libcamera.org; \n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=zUXHq2AejLABDpAGU7CtXDenVtFCwcZMMND5QDkBSPw=;\n\tb=FKudGUxaWXbQ1gdKPXHndfAgeeqrgDUErV5pEnir2EvEhwGbSPcRQFyNfLc9H/y1wK\n\tuptAoPNyKaZ8ninpeIv5bqChOTXHkBo6+FpQSb9tbiVmbEXzy1PWuL6/ybLAz1gn7IlB\n\tjUzdbtmUIWc1KEA5OVteDnbz2K7wJkd9pD8/Z17OyZJViwcHHZ+g78NAJLWSn+R3GjmJ\n\t/TFIzW4gkJhAzc7vpOEdVl8UdOS35j1Z1Hlauxyflu3T7M1zhRvhxGAwlbDm816R4cmX\n\t1+85LMYs9inuwPg//r2EJsomKi8iODlIgjjrMIRrc+b2EzFTTwgN79JOufWqFmv9o2qb\n\tIRDA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1709138790; x=1709743590;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=zUXHq2AejLABDpAGU7CtXDenVtFCwcZMMND5QDkBSPw=;\n\tb=rZkPVQsozXb+2WuNnB4lCLe0urDQfP6eMrHUdv9zcNmGsRzVh3Uz8IDsRcNygVwTt4\n\tAN3/zf0l73Giyo6p4yjV+umVh2ohUzdg+K0lDfcIfJbKMHvWjAEqsDyNzTXHC49UFrP5\n\tS2N1OXYG5IXT4AbZgoQPBNhfu9qCjU8NnaQlJaSsRLPL+4oerUx/lxYtRQhJkngaYL3e\n\t5eQJVMn2Wp4F/8WscdbdfRBSeBG4b71flRhEUQqKF+u2wRXL3QvMB+eetQJ815N+2xe7\n\tBlrek+TCbx2BnR+s510nfpehZ1gHKTGQG8UccsG2OC63rj/dugEojOoUZS2xr8SC412P\n\tv+EQ==","X-Gm-Message-State":"AOJu0YxAi4vAgWfQz8kpbvFLK447rdyRQ6CGRBWA1fOGFnqsEW5COoNR\n\tOTvfhc8H/hzNK/9hMEkZM0x5aUgGHzkc4LCBjhVzQFGiSusyBEW7ulZ7+tm2gvXLLrRzv6/OlED\n\t2aQpVdRJald+YUl+C9QHaQVHPSiiS/i8R8Y7waAuT2DZYIVNJ","X-Google-Smtp-Source":"AGHT+IGANVBnB1Ix93YZ/0UGCz3FuhDTQFp7zrSECL8wfSKb6IYgZK6WKB6mnwpjoioGUM2XSsARVevzii2oW+ZAWDc=","X-Received":"by 2002:a81:400a:0:b0:608:bf09:a152 with SMTP id\n\tl10-20020a81400a000000b00608bf09a152mr5564038ywn.3.1709138789944;\n\tWed, 28 Feb 2024 08:46:29 -0800 (PST)","MIME-Version":"1.0","References":"<20240215132710.810-1-naush@raspberrypi.com>\n\t<20240215132710.810-3-naush@raspberrypi.com>\n\t<74nstgdru2jpkry22imfmrcelvvzkfektet6cdo72gl5hqgz6t@vr6q6rtvti27>\n\t\n\t","In-Reply-To":"","From":"Naushir Patuck ","Date":"Wed, 28 Feb 2024 16:45:54 +0000","Message-ID":"","Subject":"Re: [PATCH v1 2/2] libcamera: formats: Add PiSP specific image and\n\tconfig buffer formats","To":"Jacopo Mondi ","Content-Type":"text/plain; charset=\"UTF-8\"","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28795,"web_url":"https://patchwork.libcamera.org/comment/28795/","msgid":"","date":"2024-02-28T17:16:09","subject":"Re: [PATCH v1 2/2] libcamera: formats: Add PiSP specific image and\n\tconfig buffer formats","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Naush\n\nOn Wed, Feb 28, 2024 at 04:45:54PM +0000, Naushir Patuck wrote:\n> Hi Jacopo,\n>\n> > > > > -\n> > > > > + { formats::BGGR16_PISP_COMP1, {\n> > > > > + .name = \"BGGR16_PISP_COMP1\",\n> > > > > + .format = formats::BGGR16_PISP_COMP1,\n> > > > > + .v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_BGGR), },\n> > > > > + .bitsPerPixel = 16,\n> > > > > + .colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> > > > > + .packed = true,\n> > > > > + .pixelsPerGroup = 2,\n> > > > > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},\n> > > > > + } },\n> > > >\n> > > > For regular 16-bit formats this should be\n> > > >\n> > > > .pixelsPerGroup = 2,\n> > > > .planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},\n> > > >\n> > > > As 2 pixels of 16 bits each consume 4 bytes. Is this {2, 1}\n> > > > intentional and due to compression ?\n> > >\n> > > Yes, I used {2, 1} to denote the 8-bit/sample after compression. Do\n> > > you think I should change to {4, 1}?\n> >\n> > Depends on how many bytes the format actually consumes :)\n> >\n> > For 16bpp RAW formats, we have pixelsPerGroup = 2 and each sample\n> > consumes 2 bytes.\n> >\n> > According to the pixelsPerGroup definition:\n> >\n> > * A pixel group is defined as the minimum number of pixels (including padding)\n> > * necessary in a row when the image has only one column of effective pixels.\n> >\n> > So for a RAW Bayer formats, to get a complete full-color \"superpixel\"\n> > we need at least two pixels per row.\n> >\n> > RG\n> > GB\n> >\n> > Each \"RG\" or \"GB\" pair is composed by 2 16-bit samples, for a total of\n> > 32bits of data, from which the {4, 1} value in planes[0]\n> >\n> > According to \"Table 2\", Chapter 5 of the PiSP datasheet, the\n> > compressed formats are described as\n> >\n> > 8-bits per pixel single channel compressed with mode n, for n = 1, 2, 3\n> >\n> > Which suggests each \"RG\" or \"GB\" pair is actually 2 samples of 8 bits\n> > each, for a plane size of {2, 1}.\n> >\n> > Is this correct ? Is the actual .bitsPerPixel value for the compressed\n> > formats 8 instead of 16 then ?\n>\n> This is correct, each sample is 8-bits so \"RG\"/\"GB\" pairs are 16-bits each.\n> So it sounds like I need to use .planes = {2, 1}, but .bitsPerPixel = 8 maybe?\n\nI now wonder if the '16' in the format names is correct then :)\n\n>\n> Naush\n>\n> >\n> > Thanks\n> > j\n> >\n> > >\n> > > Regards,\n> > > Naush\n> > >\n> > >\n> > > >\n> > > > > + { formats::GBRG16_PISP_COMP1, {\n> > > > > + .name = \"GBRG16_PISP_COMP1\",\n> > > > > + .format = formats::GBRG16_PISP_COMP1,\n> > > > > + .v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GBRG), },\n> > > > > + .bitsPerPixel = 16,\n> > > > > + .colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> > > > > + .packed = true,\n> > > > > + .pixelsPerGroup = 2,\n> > > > > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},\n> > > > > + } },\n> > > > > + { formats::GRBG16_PISP_COMP1, {\n> > > > > + .name = \"GRBG16_PISP_COMP1\",\n> > > > > + .format = formats::GRBG16_PISP_COMP1,\n> > > > > + .v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GRBG), },\n> > > > > + .bitsPerPixel = 16,\n> > > > > + .colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> > > > > + .packed = true,\n> > > > > + .pixelsPerGroup = 2,\n> > > > > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},\n> > > > > + } },\n> > > > > + { formats::RGGB16_PISP_COMP1, {\n> > > > > + .name = \"RGGB16_PISP_COMP1\",\n> > > > > + .format = formats::RGGB16_PISP_COMP1,\n> > > > > + .v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_RGGB), },\n> > > > > + .bitsPerPixel = 16,\n> > > > > + .colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> > > > > + .packed = true,\n> > > > > + .pixelsPerGroup = 2,\n> > > > > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},\n> > > > > + } },\n> > > > > /* Compressed formats. */\n> > > > > { formats::MJPEG, {\n> > > > > .name = \"MJPEG\",\n> > > > > diff --git a/src/libcamera/formats.yaml b/src/libcamera/formats.yaml\n> > > > > index bde2cc803b98..f6df721243d0 100644\n> > > > > --- a/src/libcamera/formats.yaml\n> > > > > +++ b/src/libcamera/formats.yaml\n> > > > > @@ -190,4 +190,20 @@ formats:\n> > > > > - SBGGR10_IPU3:\n> > > > > fourcc: DRM_FORMAT_SBGGR10\n> > > > > mod: IPU3_FORMAT_MOD_PACKED\n> > > > > +\n> > > > > + - RGGB16_PISP_COMP1:\n> > > > > + fourcc: DRM_FORMAT_SRGGB16\n> > > > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1\n> > > > > + - GRBG16_PISP_COMP1:\n> > > > > + fourcc: DRM_FORMAT_SGRBG16\n> > > > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1\n> > > > > + - GBRG16_PISP_COMP1:\n> > > > > + fourcc: DRM_FORMAT_SGBRG16\n> > > > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1\n> > > > > + - BGGR16_PISP_COMP1:\n> > > > > + fourcc: DRM_FORMAT_SBGGR16\n> > > > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1\n> > > > > + - MONO_PISP_COMP1:\n> > > > > + fourcc: DRM_FORMAT_R16\n> > > > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1\n> > > > > ...\n> > > > > diff --git a/src/libcamera/v4l2_pixelformat.cpp b/src/libcamera/v4l2_pixelformat.cpp\n> > > > > index efb6f2940235..47baaf60199d 100644\n> > > > > --- a/src/libcamera/v4l2_pixelformat.cpp\n> > > > > +++ b/src/libcamera/v4l2_pixelformat.cpp\n> > > > > @@ -207,6 +207,16 @@ const std::map vpf2pf{\n> > > > > { formats::SGRBG16, \"16-bit Bayer GRGR/BGBG\" } },\n> > > > > { V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16),\n> > > > > { formats::SRGGB16, \"16-bit Bayer RGRG/GBGB\" } },\n> > > > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_BGGR),\n> > > > > + { formats::BGGR16_PISP_COMP1, \"16-bit Bayer BGBG/GRGR PiSP Compress Mode 1\" } },\n> > > > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GBRG),\n> > > > > + { formats::GBRG16_PISP_COMP1, \"16-bit Bayer GBGB/RGRG PiSP Compress Mode 1\" } },\n> > > > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GRBG),\n> > > > > + { formats::GRBG16_PISP_COMP1, \"16-bit Bayer GRGR/BGBG PiSP Compress Mode 1\" } },\n> > > > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_RGGB),\n> > > > > + { formats::RGGB16_PISP_COMP1, \"16-bit Bayer RGRG/GBGB PiSP Compress Mode 1\" } },\n> > > > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_MONO),\n> > > > > + { formats::MONO_PISP_COMP1, \"16-bit Mono PiSP Compress Mode 1\" } },\n> > > > >\n> > > > > /* Compressed formats. */\n> > > > > { V4L2PixelFormat(V4L2_PIX_FMT_MJPEG),\n> > > > > diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp\n> > > > > index 6d0785b7b484..aea90abaf9ef 100644\n> > > > > --- a/src/libcamera/v4l2_subdevice.cpp\n> > > > > +++ b/src/libcamera/v4l2_subdevice.cpp\n> > > > > @@ -134,6 +134,10 @@ const std::map formatInfoMap = {\n> > > > > { MEDIA_BUS_FMT_SGBRG12_1X12, { 12, \"SGBRG12_1X12\", PixelFormatInfo::ColourEncodingRAW } },\n> > > > > { MEDIA_BUS_FMT_SGRBG12_1X12, { 12, \"SGRBG12_1X12\", PixelFormatInfo::ColourEncodingRAW } },\n> > > > > { MEDIA_BUS_FMT_SRGGB12_1X12, { 12, \"SRGGB12_1X12\", PixelFormatInfo::ColourEncodingRAW } },\n> > > > > + { MEDIA_BUS_FMT_SBGGR16_1X16, { 16, \"SBGGR16_1x16\", PixelFormatInfo::ColourEncodingRAW } },\n> > > > > + { MEDIA_BUS_FMT_SGBRG16_1X16, { 16, \"SGBRG16_1x16\", PixelFormatInfo::ColourEncodingRAW } },\n> > > > > + { MEDIA_BUS_FMT_SGRBG16_1X16, { 16, \"SGRBG16_1x16\", PixelFormatInfo::ColourEncodingRAW } },\n> > > > > + { MEDIA_BUS_FMT_SRGGB16_1X16, { 16, \"SRGGB16_1x16\", PixelFormatInfo::ColourEncodingRAW } },\n> > > > > /* \\todo Clarify colour encoding for HSV formats */\n> > > > > { MEDIA_BUS_FMT_AHSV8888_1X32, { 32, \"AHSV8888_1X32\", PixelFormatInfo::ColourEncodingRGB } },\n> > > > > { MEDIA_BUS_FMT_JPEG_1X8, { 8, \"JPEG_1X8\", PixelFormatInfo::ColourEncodingYUV } },\n> > > > > --\n> > > > > 2.34.1\n> > > > >","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id CFD1DBE080\n\tfor ;\n\tWed, 28 Feb 2024 17:16:15 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DE90162868;\n\tWed, 28 Feb 2024 18:16:14 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 41E0661C94\n\tfor ;\n\tWed, 28 Feb 2024 18:16:13 +0100 (CET)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 343A96B3;\n\tWed, 28 Feb 2024 18:16:00 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"TTcfDoPx\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1709140560;\n\tbh=Mo7HRV3nsgvSDetuqWmZK91WewTADIgfTmR3kK8nHRA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=TTcfDoPxxrL2iFzjRKIawnRXQtqq+jY2bU7ZYyc7sRIbLIMTQ0+jD6g0LpDxUckTr\n\tWgZKsdMcFIsWWguuL2CaquJ5/oH5c9co3Gktk67dWkGJlkmN09ytIkadHTm8r4/Eny\n\tHLfT+KJZbjvxyCrrFnAFqkxkBODLkiA/SbimbULo=","Date":"Wed, 28 Feb 2024 18:16:09 +0100","From":"Jacopo Mondi ","To":"Naushir Patuck ","Subject":"Re: [PATCH v1 2/2] libcamera: formats: Add PiSP specific image and\n\tconfig buffer formats","Message-ID":"","References":"<20240215132710.810-1-naush@raspberrypi.com>\n\t<20240215132710.810-3-naush@raspberrypi.com>\n\t<74nstgdru2jpkry22imfmrcelvvzkfektet6cdo72gl5hqgz6t@vr6q6rtvti27>\n\t\n\t\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"Jacopo Mondi ,\n\tlibcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28796,"web_url":"https://patchwork.libcamera.org/comment/28796/","msgid":"","date":"2024-02-28T17:19:47","subject":"Re: [PATCH v1 2/2] libcamera: formats: Add PiSP specific image and\n\tconfig buffer formats","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"On Wed, 28 Feb 2024, 5:16 pm Jacopo Mondi, \nwrote:\n\n> Hi Naush\n>\n> On Wed, Feb 28, 2024 at 04:45:54PM +0000, Naushir Patuck wrote:\n> > Hi Jacopo,\n> >\n> > > > > > -\n> > > > > > + { formats::BGGR16_PISP_COMP1, {\n> > > > > > + .name = \"BGGR16_PISP_COMP1\",\n> > > > > > + .format = formats::BGGR16_PISP_COMP1,\n> > > > > > + .v4l2Formats = {\n> V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_BGGR), },\n> > > > > > + .bitsPerPixel = 16,\n> > > > > > + .colourEncoding =\n> PixelFormatInfo::ColourEncodingRAW,\n> > > > > > + .packed = true,\n> > > > > > + .pixelsPerGroup = 2,\n> > > > > > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},\n> > > > > > + } },\n> > > > >\n> > > > > For regular 16-bit formats this should be\n> > > > >\n> > > > > .pixelsPerGroup = 2,\n> > > > > .planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},\n> > > > >\n> > > > > As 2 pixels of 16 bits each consume 4 bytes. Is this {2, 1}\n> > > > > intentional and due to compression ?\n> > > >\n> > > > Yes, I used {2, 1} to denote the 8-bit/sample after compression. Do\n> > > > you think I should change to {4, 1}?\n> > >\n> > > Depends on how many bytes the format actually consumes :)\n> > >\n> > > For 16bpp RAW formats, we have pixelsPerGroup = 2 and each sample\n> > > consumes 2 bytes.\n> > >\n> > > According to the pixelsPerGroup definition:\n> > >\n> > > * A pixel group is defined as the minimum number of pixels (including\n> padding)\n> > > * necessary in a row when the image has only one column of effective\n> pixels.\n> > >\n> > > So for a RAW Bayer formats, to get a complete full-color \"superpixel\"\n> > > we need at least two pixels per row.\n> > >\n> > > RG\n> > > GB\n> > >\n> > > Each \"RG\" or \"GB\" pair is composed by 2 16-bit samples, for a total of\n> > > 32bits of data, from which the {4, 1} value in planes[0]\n> > >\n> > > According to \"Table 2\", Chapter 5 of the PiSP datasheet, the\n> > > compressed formats are described as\n> > >\n> > > 8-bits per pixel single channel compressed with mode n, for n =\n> 1, 2, 3\n> > >\n> > > Which suggests each \"RG\" or \"GB\" pair is actually 2 samples of 8 bits\n> > > each, for a plane size of {2, 1}.\n> > >\n> > > Is this correct ? Is the actual .bitsPerPixel value for the compressed\n> > > formats 8 instead of 16 then ?\n> >\n> > This is correct, each sample is 8-bits so \"RG\"/\"GB\" pairs are 16-bits\n> each.\n> > So it sounds like I need to use .planes = {2, 1}, but .bitsPerPixel = 8\n> maybe?\n>\n> I now wonder if the '16' in the format names is correct then :)\n>\n\nIt's meant to signify 16-bits compressed to 8-bits (in this case). Maybe a\ndefined naming scheme is needed for such cases?\n\n\n> >\n> > Naush\n> >\n> > >\n> > > Thanks\n> > > j\n> > >\n> > > >\n> > > > Regards,\n> > > > Naush\n> > > >\n> > > >\n> > > > >\n> > > > > > + { formats::GBRG16_PISP_COMP1, {\n> > > > > > + .name = \"GBRG16_PISP_COMP1\",\n> > > > > > + .format = formats::GBRG16_PISP_COMP1,\n> > > > > > + .v4l2Formats = {\n> V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GBRG), },\n> > > > > > + .bitsPerPixel = 16,\n> > > > > > + .colourEncoding =\n> PixelFormatInfo::ColourEncodingRAW,\n> > > > > > + .packed = true,\n> > > > > > + .pixelsPerGroup = 2,\n> > > > > > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},\n> > > > > > + } },\n> > > > > > + { formats::GRBG16_PISP_COMP1, {\n> > > > > > + .name = \"GRBG16_PISP_COMP1\",\n> > > > > > + .format = formats::GRBG16_PISP_COMP1,\n> > > > > > + .v4l2Formats = {\n> V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GRBG), },\n> > > > > > + .bitsPerPixel = 16,\n> > > > > > + .colourEncoding =\n> PixelFormatInfo::ColourEncodingRAW,\n> > > > > > + .packed = true,\n> > > > > > + .pixelsPerGroup = 2,\n> > > > > > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},\n> > > > > > + } },\n> > > > > > + { formats::RGGB16_PISP_COMP1, {\n> > > > > > + .name = \"RGGB16_PISP_COMP1\",\n> > > > > > + .format = formats::RGGB16_PISP_COMP1,\n> > > > > > + .v4l2Formats = {\n> V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_RGGB), },\n> > > > > > + .bitsPerPixel = 16,\n> > > > > > + .colourEncoding =\n> PixelFormatInfo::ColourEncodingRAW,\n> > > > > > + .packed = true,\n> > > > > > + .pixelsPerGroup = 2,\n> > > > > > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},\n> > > > > > + } },\n> > > > > > /* Compressed formats. */\n> > > > > > { formats::MJPEG, {\n> > > > > > .name = \"MJPEG\",\n> > > > > > diff --git a/src/libcamera/formats.yaml\n> b/src/libcamera/formats.yaml\n> > > > > > index bde2cc803b98..f6df721243d0 100644\n> > > > > > --- a/src/libcamera/formats.yaml\n> > > > > > +++ b/src/libcamera/formats.yaml\n> > > > > > @@ -190,4 +190,20 @@ formats:\n> > > > > > - SBGGR10_IPU3:\n> > > > > > fourcc: DRM_FORMAT_SBGGR10\n> > > > > > mod: IPU3_FORMAT_MOD_PACKED\n> > > > > > +\n> > > > > > + - RGGB16_PISP_COMP1:\n> > > > > > + fourcc: DRM_FORMAT_SRGGB16\n> > > > > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1\n> > > > > > + - GRBG16_PISP_COMP1:\n> > > > > > + fourcc: DRM_FORMAT_SGRBG16\n> > > > > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1\n> > > > > > + - GBRG16_PISP_COMP1:\n> > > > > > + fourcc: DRM_FORMAT_SGBRG16\n> > > > > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1\n> > > > > > + - BGGR16_PISP_COMP1:\n> > > > > > + fourcc: DRM_FORMAT_SBGGR16\n> > > > > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1\n> > > > > > + - MONO_PISP_COMP1:\n> > > > > > + fourcc: DRM_FORMAT_R16\n> > > > > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1\n> > > > > > ...\n> > > > > > diff --git a/src/libcamera/v4l2_pixelformat.cpp\n> b/src/libcamera/v4l2_pixelformat.cpp\n> > > > > > index efb6f2940235..47baaf60199d 100644\n> > > > > > --- a/src/libcamera/v4l2_pixelformat.cpp\n> > > > > > +++ b/src/libcamera/v4l2_pixelformat.cpp\n> > > > > > @@ -207,6 +207,16 @@ const std::map V4L2PixelFormat::Info> vpf2pf{\n> > > > > > { formats::SGRBG16, \"16-bit Bayer GRGR/BGBG\" } },\n> > > > > > { V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16),\n> > > > > > { formats::SRGGB16, \"16-bit Bayer RGRG/GBGB\" } },\n> > > > > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_BGGR),\n> > > > > > + { formats::BGGR16_PISP_COMP1, \"16-bit Bayer\n> BGBG/GRGR PiSP Compress Mode 1\" } },\n> > > > > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GBRG),\n> > > > > > + { formats::GBRG16_PISP_COMP1, \"16-bit Bayer\n> GBGB/RGRG PiSP Compress Mode 1\" } },\n> > > > > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GRBG),\n> > > > > > + { formats::GRBG16_PISP_COMP1, \"16-bit Bayer\n> GRGR/BGBG PiSP Compress Mode 1\" } },\n> > > > > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_RGGB),\n> > > > > > + { formats::RGGB16_PISP_COMP1, \"16-bit Bayer\n> RGRG/GBGB PiSP Compress Mode 1\" } },\n> > > > > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_MONO),\n> > > > > > + { formats::MONO_PISP_COMP1, \"16-bit Mono PiSP\n> Compress Mode 1\" } },\n> > > > > >\n> > > > > > /* Compressed formats. */\n> > > > > > { V4L2PixelFormat(V4L2_PIX_FMT_MJPEG),\n> > > > > > diff --git a/src/libcamera/v4l2_subdevice.cpp\n> b/src/libcamera/v4l2_subdevice.cpp\n> > > > > > index 6d0785b7b484..aea90abaf9ef 100644\n> > > > > > --- a/src/libcamera/v4l2_subdevice.cpp\n> > > > > > +++ b/src/libcamera/v4l2_subdevice.cpp\n> > > > > > @@ -134,6 +134,10 @@ const std::map V4L2SubdeviceFormatInfo> formatInfoMap = {\n> > > > > > { MEDIA_BUS_FMT_SGBRG12_1X12, { 12, \"SGBRG12_1X12\",\n> PixelFormatInfo::ColourEncodingRAW } },\n> > > > > > { MEDIA_BUS_FMT_SGRBG12_1X12, { 12, \"SGRBG12_1X12\",\n> PixelFormatInfo::ColourEncodingRAW } },\n> > > > > > { MEDIA_BUS_FMT_SRGGB12_1X12, { 12, \"SRGGB12_1X12\",\n> PixelFormatInfo::ColourEncodingRAW } },\n> > > > > > + { MEDIA_BUS_FMT_SBGGR16_1X16, { 16, \"SBGGR16_1x16\",\n> PixelFormatInfo::ColourEncodingRAW } },\n> > > > > > + { MEDIA_BUS_FMT_SGBRG16_1X16, { 16, \"SGBRG16_1x16\",\n> PixelFormatInfo::ColourEncodingRAW } },\n> > > > > > + { MEDIA_BUS_FMT_SGRBG16_1X16, { 16, \"SGRBG16_1x16\",\n> PixelFormatInfo::ColourEncodingRAW } },\n> > > > > > + { MEDIA_BUS_FMT_SRGGB16_1X16, { 16, \"SRGGB16_1x16\",\n> PixelFormatInfo::ColourEncodingRAW } },\n> > > > > > /* \\todo Clarify colour encoding for HSV formats */\n> > > > > > { MEDIA_BUS_FMT_AHSV8888_1X32, { 32, \"AHSV8888_1X32\",\n> PixelFormatInfo::ColourEncodingRGB } },\n> > > > > > { MEDIA_BUS_FMT_JPEG_1X8, { 8, \"JPEG_1X8\",\n> PixelFormatInfo::ColourEncodingYUV } },\n> > > > > > --\n> > > > > > 2.34.1\n> > > > > >\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id AFF23BE080\n\tfor ;\n\tWed, 28 Feb 2024 17:20:02 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id F01A0627FC;\n\tWed, 28 Feb 2024 18:20:01 +0100 (CET)","from mail-yw1-x1129.google.com (mail-yw1-x1129.google.com\n\t[IPv6:2607:f8b0:4864:20::1129])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CAF48627F9\n\tfor ;\n\tWed, 28 Feb 2024 18:19:59 +0100 (CET)","by mail-yw1-x1129.google.com with SMTP id\n\t00721157ae682-60821136c5aso40958477b3.1\n\tfor ;\n\tWed, 28 Feb 2024 09:19:59 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"jGSDChpj\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1709140798; x=1709745598;\n\tdarn=lists.libcamera.org; \n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=M/U5lRKiJXIWoVBhKgEzYyF8A0EU4QyfO7Xv3vKSYDM=;\n\tb=jGSDChpj7NlbqWwCy4g0n9wQppqi1cJUYQ1rLskNUJJwkGLHClDXjbRmDmPRD6TTt1\n\tG6Z6Ozjtw9stjO7h9E0bAanzBokfG3TbEDft0wdiij/t4T/T7BR56CWGrTII0QIx8BM8\n\t/1pGBiL6FevO9BitvZgCqa8oMHtKx/mWbh3QOump4KX/zF6r7V7E4hbT4PwPJbzh654r\n\ti2+3kZKxwa/126oXFIkQK2VQ/cdKwdxYDNhUJMeMdIO3YQZWiPZ5Ea/x3gkG3ENitUZ/\n\t0pTzNZ//Ez02nRhqUFSWZSOVe7KVREBOf2mZRjgxZAjNwemvvN3GvemRtWLC4Hwv5p5u\n\tAUKw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1709140798; x=1709745598;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=M/U5lRKiJXIWoVBhKgEzYyF8A0EU4QyfO7Xv3vKSYDM=;\n\tb=un8KQP8d1PlwVm2qcdH7zRYVYJbsVaeW1SRbAmTteppIy3WQ3Jaeqhd8n6CNy7NxRI\n\tAZUlZns4Lep8V+VX/PJCg+BPxuztiF7hqyW5aTvQNe2Xr3YEQ7YtDCq3ngsnacNUWudt\n\tYAUBYJ5ZehK8ToP0HMKmriQtRHUHRkAHdk9blspZ8MmaNjiVEEm1MoV0kJIry8UWl0ry\n\t6ljib6v5m9VxUvGOtHaGswHXVg5LYw2kwd+THoA4jv9r8rMkq2FFZ+KWFQfVSS3QNuG3\n\txhJvih7Q+or3OVyMNbT0jmLcvWnodZNOTBR92/sRSXvkj/e048zY3TqnNYX+r5v5QueN\n\tqsvA==","X-Gm-Message-State":"AOJu0YwGlTfLR5kxvjxZguTUbihescK71xIxdzvQ43Y+1g5ztCWeihTR\n\tfaPNTrXFwtVGmzFXsRADqMzdmAI9DcP6sxIo7A9iGZWQOf2zIm33XUVkQVQbtqXWaYPrI+dXzaY\n\tLw2rMcMnBdPPP5eCnpVtktG7zcd4L5VVkXczhCgJ0Ufo4amXd","X-Google-Smtp-Source":"AGHT+IFfAG7yRHAcQ6yMkHYg8IyuuKQ5i8NBSxfuqi74SNYY7u9rJsJYNcxi2kde5+KcS8k96iQhjAnZruRtwIji8K0=","X-Received":"by 2002:a81:bc03:0:b0:608:2ad1:bac6 with SMTP id\n\ta3-20020a81bc03000000b006082ad1bac6mr5897523ywi.27.1709140798699;\n\tWed, 28 Feb 2024 09:19:58 -0800 (PST)","MIME-Version":"1.0","References":"<20240215132710.810-1-naush@raspberrypi.com>\n\t<20240215132710.810-3-naush@raspberrypi.com>\n\t<74nstgdru2jpkry22imfmrcelvvzkfektet6cdo72gl5hqgz6t@vr6q6rtvti27>\n\t\n\t\n\t\n\t","In-Reply-To":"","From":"Naushir Patuck ","Date":"Wed, 28 Feb 2024 17:19:47 +0000","Message-ID":"","Subject":"Re: [PATCH v1 2/2] libcamera: formats: Add PiSP specific image and\n\tconfig buffer formats","To":"Jacopo Mondi ","Content-Type":"multipart/alternative; boundary=\"0000000000002d91be0612745aaf\"","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28798,"web_url":"https://patchwork.libcamera.org/comment/28798/","msgid":"","date":"2024-02-29T07:40:32","subject":"Re: [PATCH v1 2/2] libcamera: formats: Add PiSP specific image and\n\tconfig buffer formats","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Naush\n\nOn Wed, Feb 28, 2024 at 05:19:47PM +0000, Naushir Patuck wrote:\n> On Wed, 28 Feb 2024, 5:16 pm Jacopo Mondi, \n> wrote:\n>\n> > Hi Naush\n> >\n> > On Wed, Feb 28, 2024 at 04:45:54PM +0000, Naushir Patuck wrote:\n> > > Hi Jacopo,\n> > >\n> > > > > > > -\n> > > > > > > + { formats::BGGR16_PISP_COMP1, {\n> > > > > > > + .name = \"BGGR16_PISP_COMP1\",\n> > > > > > > + .format = formats::BGGR16_PISP_COMP1,\n> > > > > > > + .v4l2Formats = {\n> > V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_BGGR), },\n> > > > > > > + .bitsPerPixel = 16,\n> > > > > > > + .colourEncoding =\n> > PixelFormatInfo::ColourEncodingRAW,\n> > > > > > > + .packed = true,\n> > > > > > > + .pixelsPerGroup = 2,\n> > > > > > > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},\n> > > > > > > + } },\n> > > > > >\n> > > > > > For regular 16-bit formats this should be\n> > > > > >\n> > > > > > .pixelsPerGroup = 2,\n> > > > > > .planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},\n> > > > > >\n> > > > > > As 2 pixels of 16 bits each consume 4 bytes. Is this {2, 1}\n> > > > > > intentional and due to compression ?\n> > > > >\n> > > > > Yes, I used {2, 1} to denote the 8-bit/sample after compression. Do\n> > > > > you think I should change to {4, 1}?\n> > > >\n> > > > Depends on how many bytes the format actually consumes :)\n> > > >\n> > > > For 16bpp RAW formats, we have pixelsPerGroup = 2 and each sample\n> > > > consumes 2 bytes.\n> > > >\n> > > > According to the pixelsPerGroup definition:\n> > > >\n> > > > * A pixel group is defined as the minimum number of pixels (including\n> > padding)\n> > > > * necessary in a row when the image has only one column of effective\n> > pixels.\n> > > >\n> > > > So for a RAW Bayer formats, to get a complete full-color \"superpixel\"\n> > > > we need at least two pixels per row.\n> > > >\n> > > > RG\n> > > > GB\n> > > >\n> > > > Each \"RG\" or \"GB\" pair is composed by 2 16-bit samples, for a total of\n> > > > 32bits of data, from which the {4, 1} value in planes[0]\n> > > >\n> > > > According to \"Table 2\", Chapter 5 of the PiSP datasheet, the\n> > > > compressed formats are described as\n> > > >\n> > > > 8-bits per pixel single channel compressed with mode n, for n =\n> > 1, 2, 3\n> > > >\n> > > > Which suggests each \"RG\" or \"GB\" pair is actually 2 samples of 8 bits\n> > > > each, for a plane size of {2, 1}.\n> > > >\n> > > > Is this correct ? Is the actual .bitsPerPixel value for the compressed\n> > > > formats 8 instead of 16 then ?\n> > >\n> > > This is correct, each sample is 8-bits so \"RG\"/\"GB\" pairs are 16-bits\n> > each.\n> > > So it sounds like I need to use .planes = {2, 1}, but .bitsPerPixel = 8\n> > maybe?\n> >\n> > I now wonder if the '16' in the format names is correct then :)\n> >\n>\n> It's meant to signify 16-bits compressed to 8-bits (in this case). Maybe a\n> defined naming scheme is needed for such cases?\n>\n\nWhat about simply leaving the bitdpeth out from the format name like\nit is done for the V4L2 pixel formats ?\n\n#define V4L2_PIX_FMT_PISP_COMP1_RGGB v4l2_fourcc('P', 'C', '1', 'R') /* PiSP 8-bit mode 1 compressed RGGB bayer */\n\nSomething like\n\n { formats::BGGR_PISP_COMP1, {\n .name = \"BGGR_PISP_COMP1\",\n .format = formats::BGGR_PISP_COMP1,\n .v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_BGGR), },\n .bitsPerPixel = 8,\n .colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n .packed = true,\n .pixelsPerGroup = 2,\n .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},\n } },\n\n?\n\n>\n> > >\n> > > Naush\n> > >\n> > > >\n> > > > Thanks\n> > > > j\n> > > >\n> > > > >\n> > > > > Regards,\n> > > > > Naush\n> > > > >\n> > > > >\n> > > > > >\n> > > > > > > + { formats::GBRG16_PISP_COMP1, {\n> > > > > > > + .name = \"GBRG16_PISP_COMP1\",\n> > > > > > > + .format = formats::GBRG16_PISP_COMP1,\n> > > > > > > + .v4l2Formats = {\n> > V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GBRG), },\n> > > > > > > + .bitsPerPixel = 16,\n> > > > > > > + .colourEncoding =\n> > PixelFormatInfo::ColourEncodingRAW,\n> > > > > > > + .packed = true,\n> > > > > > > + .pixelsPerGroup = 2,\n> > > > > > > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},\n> > > > > > > + } },\n> > > > > > > + { formats::GRBG16_PISP_COMP1, {\n> > > > > > > + .name = \"GRBG16_PISP_COMP1\",\n> > > > > > > + .format = formats::GRBG16_PISP_COMP1,\n> > > > > > > + .v4l2Formats = {\n> > V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GRBG), },\n> > > > > > > + .bitsPerPixel = 16,\n> > > > > > > + .colourEncoding =\n> > PixelFormatInfo::ColourEncodingRAW,\n> > > > > > > + .packed = true,\n> > > > > > > + .pixelsPerGroup = 2,\n> > > > > > > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},\n> > > > > > > + } },\n> > > > > > > + { formats::RGGB16_PISP_COMP1, {\n> > > > > > > + .name = \"RGGB16_PISP_COMP1\",\n> > > > > > > + .format = formats::RGGB16_PISP_COMP1,\n> > > > > > > + .v4l2Formats = {\n> > V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_RGGB), },\n> > > > > > > + .bitsPerPixel = 16,\n> > > > > > > + .colourEncoding =\n> > PixelFormatInfo::ColourEncodingRAW,\n> > > > > > > + .packed = true,\n> > > > > > > + .pixelsPerGroup = 2,\n> > > > > > > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},\n> > > > > > > + } },\n> > > > > > > /* Compressed formats. */\n> > > > > > > { formats::MJPEG, {\n> > > > > > > .name = \"MJPEG\",\n> > > > > > > diff --git a/src/libcamera/formats.yaml\n> > b/src/libcamera/formats.yaml\n> > > > > > > index bde2cc803b98..f6df721243d0 100644\n> > > > > > > --- a/src/libcamera/formats.yaml\n> > > > > > > +++ b/src/libcamera/formats.yaml\n> > > > > > > @@ -190,4 +190,20 @@ formats:\n> > > > > > > - SBGGR10_IPU3:\n> > > > > > > fourcc: DRM_FORMAT_SBGGR10\n> > > > > > > mod: IPU3_FORMAT_MOD_PACKED\n> > > > > > > +\n> > > > > > > + - RGGB16_PISP_COMP1:\n> > > > > > > + fourcc: DRM_FORMAT_SRGGB16\n> > > > > > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1\n> > > > > > > + - GRBG16_PISP_COMP1:\n> > > > > > > + fourcc: DRM_FORMAT_SGRBG16\n> > > > > > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1\n> > > > > > > + - GBRG16_PISP_COMP1:\n> > > > > > > + fourcc: DRM_FORMAT_SGBRG16\n> > > > > > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1\n> > > > > > > + - BGGR16_PISP_COMP1:\n> > > > > > > + fourcc: DRM_FORMAT_SBGGR16\n> > > > > > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1\n> > > > > > > + - MONO_PISP_COMP1:\n> > > > > > > + fourcc: DRM_FORMAT_R16\n> > > > > > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1\n> > > > > > > ...\n> > > > > > > diff --git a/src/libcamera/v4l2_pixelformat.cpp\n> > b/src/libcamera/v4l2_pixelformat.cpp\n> > > > > > > index efb6f2940235..47baaf60199d 100644\n> > > > > > > --- a/src/libcamera/v4l2_pixelformat.cpp\n> > > > > > > +++ b/src/libcamera/v4l2_pixelformat.cpp\n> > > > > > > @@ -207,6 +207,16 @@ const std::map > V4L2PixelFormat::Info> vpf2pf{\n> > > > > > > { formats::SGRBG16, \"16-bit Bayer GRGR/BGBG\" } },\n> > > > > > > { V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16),\n> > > > > > > { formats::SRGGB16, \"16-bit Bayer RGRG/GBGB\" } },\n> > > > > > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_BGGR),\n> > > > > > > + { formats::BGGR16_PISP_COMP1, \"16-bit Bayer\n> > BGBG/GRGR PiSP Compress Mode 1\" } },\n> > > > > > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GBRG),\n> > > > > > > + { formats::GBRG16_PISP_COMP1, \"16-bit Bayer\n> > GBGB/RGRG PiSP Compress Mode 1\" } },\n> > > > > > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GRBG),\n> > > > > > > + { formats::GRBG16_PISP_COMP1, \"16-bit Bayer\n> > GRGR/BGBG PiSP Compress Mode 1\" } },\n> > > > > > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_RGGB),\n> > > > > > > + { formats::RGGB16_PISP_COMP1, \"16-bit Bayer\n> > RGRG/GBGB PiSP Compress Mode 1\" } },\n> > > > > > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_MONO),\n> > > > > > > + { formats::MONO_PISP_COMP1, \"16-bit Mono PiSP\n> > Compress Mode 1\" } },\n> > > > > > >\n> > > > > > > /* Compressed formats. */\n> > > > > > > { V4L2PixelFormat(V4L2_PIX_FMT_MJPEG),\n> > > > > > > diff --git a/src/libcamera/v4l2_subdevice.cpp\n> > b/src/libcamera/v4l2_subdevice.cpp\n> > > > > > > index 6d0785b7b484..aea90abaf9ef 100644\n> > > > > > > --- a/src/libcamera/v4l2_subdevice.cpp\n> > > > > > > +++ b/src/libcamera/v4l2_subdevice.cpp\n> > > > > > > @@ -134,6 +134,10 @@ const std::map > V4L2SubdeviceFormatInfo> formatInfoMap = {\n> > > > > > > { MEDIA_BUS_FMT_SGBRG12_1X12, { 12, \"SGBRG12_1X12\",\n> > PixelFormatInfo::ColourEncodingRAW } },\n> > > > > > > { MEDIA_BUS_FMT_SGRBG12_1X12, { 12, \"SGRBG12_1X12\",\n> > PixelFormatInfo::ColourEncodingRAW } },\n> > > > > > > { MEDIA_BUS_FMT_SRGGB12_1X12, { 12, \"SRGGB12_1X12\",\n> > PixelFormatInfo::ColourEncodingRAW } },\n> > > > > > > + { MEDIA_BUS_FMT_SBGGR16_1X16, { 16, \"SBGGR16_1x16\",\n> > PixelFormatInfo::ColourEncodingRAW } },\n> > > > > > > + { MEDIA_BUS_FMT_SGBRG16_1X16, { 16, \"SGBRG16_1x16\",\n> > PixelFormatInfo::ColourEncodingRAW } },\n> > > > > > > + { MEDIA_BUS_FMT_SGRBG16_1X16, { 16, \"SGRBG16_1x16\",\n> > PixelFormatInfo::ColourEncodingRAW } },\n> > > > > > > + { MEDIA_BUS_FMT_SRGGB16_1X16, { 16, \"SRGGB16_1x16\",\n> > PixelFormatInfo::ColourEncodingRAW } },\n> > > > > > > /* \\todo Clarify colour encoding for HSV formats */\n> > > > > > > { MEDIA_BUS_FMT_AHSV8888_1X32, { 32, \"AHSV8888_1X32\",\n> > PixelFormatInfo::ColourEncodingRGB } },\n> > > > > > > { MEDIA_BUS_FMT_JPEG_1X8, { 8, \"JPEG_1X8\",\n> > PixelFormatInfo::ColourEncodingYUV } },\n> > > > > > > --\n> > > > > > > 2.34.1\n> > > > > > >\n> >","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 71FD2BD160\n\tfor ;\n\tThu, 29 Feb 2024 07:40:38 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AD18462868;\n\tThu, 29 Feb 2024 08:40:37 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1141261C8F\n\tfor ;\n\tThu, 29 Feb 2024 08:40:36 +0100 (CET)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 927152E7;\n\tThu, 29 Feb 2024 08:40:22 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"TXSWSTSp\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1709192422;\n\tbh=vCv+J1+DZvGvc9qa2NnqxrgcXAV1rJqchowqLbCG2wc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=TXSWSTSpojBuhaK8JkSxrX819yx3dHLcvLB/XpofVeMG6jHW6S82rWXBcB7WOTD2Y\n\tZ7kppyY6hxAYO7VtyNxiHTdAamToi9cVtN4WICjC3Od3GByRQwRxkP7Iycy6LyHIz1\n\tODQ/nj9UPF5AKgeK8z7OoAoZrCw6V1yEbqg/aYdo=","Date":"Thu, 29 Feb 2024 08:40:32 +0100","From":"Jacopo Mondi ","To":"Naushir Patuck ","Subject":"Re: [PATCH v1 2/2] libcamera: formats: Add PiSP specific image and\n\tconfig buffer formats","Message-ID":"","References":"<20240215132710.810-1-naush@raspberrypi.com>\n\t<20240215132710.810-3-naush@raspberrypi.com>\n\t<74nstgdru2jpkry22imfmrcelvvzkfektet6cdo72gl5hqgz6t@vr6q6rtvti27>\n\t\n\t\n\t\n\t\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"Jacopo Mondi ,\n\tlibcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28799,"web_url":"https://patchwork.libcamera.org/comment/28799/","msgid":"","date":"2024-02-29T08:18:46","subject":"Re: [PATCH v1 2/2] libcamera: formats: Add PiSP specific image and\n\tconfig buffer formats","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hey Jacopo,\n\nOn Thu, 29 Feb 2024 at 07:40, Jacopo Mondi\n wrote:\n>\n> Hi Naush\n>\n> On Wed, Feb 28, 2024 at 05:19:47PM +0000, Naushir Patuck wrote:\n> > On Wed, 28 Feb 2024, 5:16 pm Jacopo Mondi, \n> > wrote:\n> >\n> > > Hi Naush\n> > >\n> > > On Wed, Feb 28, 2024 at 04:45:54PM +0000, Naushir Patuck wrote:\n> > > > Hi Jacopo,\n> > > >\n> > > > > > > > -\n> > > > > > > > + { formats::BGGR16_PISP_COMP1, {\n> > > > > > > > + .name = \"BGGR16_PISP_COMP1\",\n> > > > > > > > + .format = formats::BGGR16_PISP_COMP1,\n> > > > > > > > + .v4l2Formats = {\n> > > V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_BGGR), },\n> > > > > > > > + .bitsPerPixel = 16,\n> > > > > > > > + .colourEncoding =\n> > > PixelFormatInfo::ColourEncodingRAW,\n> > > > > > > > + .packed = true,\n> > > > > > > > + .pixelsPerGroup = 2,\n> > > > > > > > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},\n> > > > > > > > + } },\n> > > > > > >\n> > > > > > > For regular 16-bit formats this should be\n> > > > > > >\n> > > > > > > .pixelsPerGroup = 2,\n> > > > > > > .planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},\n> > > > > > >\n> > > > > > > As 2 pixels of 16 bits each consume 4 bytes. Is this {2, 1}\n> > > > > > > intentional and due to compression ?\n> > > > > >\n> > > > > > Yes, I used {2, 1} to denote the 8-bit/sample after compression. Do\n> > > > > > you think I should change to {4, 1}?\n> > > > >\n> > > > > Depends on how many bytes the format actually consumes :)\n> > > > >\n> > > > > For 16bpp RAW formats, we have pixelsPerGroup = 2 and each sample\n> > > > > consumes 2 bytes.\n> > > > >\n> > > > > According to the pixelsPerGroup definition:\n> > > > >\n> > > > > * A pixel group is defined as the minimum number of pixels (including\n> > > padding)\n> > > > > * necessary in a row when the image has only one column of effective\n> > > pixels.\n> > > > >\n> > > > > So for a RAW Bayer formats, to get a complete full-color \"superpixel\"\n> > > > > we need at least two pixels per row.\n> > > > >\n> > > > > RG\n> > > > > GB\n> > > > >\n> > > > > Each \"RG\" or \"GB\" pair is composed by 2 16-bit samples, for a total of\n> > > > > 32bits of data, from which the {4, 1} value in planes[0]\n> > > > >\n> > > > > According to \"Table 2\", Chapter 5 of the PiSP datasheet, the\n> > > > > compressed formats are described as\n> > > > >\n> > > > > 8-bits per pixel single channel compressed with mode n, for n =\n> > > 1, 2, 3\n> > > > >\n> > > > > Which suggests each \"RG\" or \"GB\" pair is actually 2 samples of 8 bits\n> > > > > each, for a plane size of {2, 1}.\n> > > > >\n> > > > > Is this correct ? Is the actual .bitsPerPixel value for the compressed\n> > > > > formats 8 instead of 16 then ?\n> > > >\n> > > > This is correct, each sample is 8-bits so \"RG\"/\"GB\" pairs are 16-bits\n> > > each.\n> > > > So it sounds like I need to use .planes = {2, 1}, but .bitsPerPixel = 8\n> > > maybe?\n> > >\n> > > I now wonder if the '16' in the format names is correct then :)\n> > >\n> >\n> > It's meant to signify 16-bits compressed to 8-bits (in this case). Maybe a\n> > defined naming scheme is needed for such cases?\n> >\n>\n> What about simply leaving the bitdpeth out from the format name like\n> it is done for the V4L2 pixel formats ?\n\nThat sounds good to me! I'll make the change in the next patch.\n\n>\n> #define V4L2_PIX_FMT_PISP_COMP1_RGGB v4l2_fourcc('P', 'C', '1', 'R') /* PiSP 8-bit mode 1 compressed RGGB bayer */\n>\n> Something like\n>\n> { formats::BGGR_PISP_COMP1, {\n> .name = \"BGGR_PISP_COMP1\",\n> .format = formats::BGGR_PISP_COMP1,\n> .v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_BGGR), },\n> .bitsPerPixel = 8,\n> .colourEncoding = PixelFormatInfo::ColourEncodingRAW,\n> .packed = true,\n> .pixelsPerGroup = 2,\n> .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},\n> } },\n>\n> ?\n>\n> >\n> > > >\n> > > > Naush\n> > > >\n> > > > >\n> > > > > Thanks\n> > > > > j\n> > > > >\n> > > > > >\n> > > > > > Regards,\n> > > > > > Naush\n> > > > > >\n> > > > > >\n> > > > > > >\n> > > > > > > > + { formats::GBRG16_PISP_COMP1, {\n> > > > > > > > + .name = \"GBRG16_PISP_COMP1\",\n> > > > > > > > + .format = formats::GBRG16_PISP_COMP1,\n> > > > > > > > + .v4l2Formats = {\n> > > V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GBRG), },\n> > > > > > > > + .bitsPerPixel = 16,\n> > > > > > > > + .colourEncoding =\n> > > PixelFormatInfo::ColourEncodingRAW,\n> > > > > > > > + .packed = true,\n> > > > > > > > + .pixelsPerGroup = 2,\n> > > > > > > > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},\n> > > > > > > > + } },\n> > > > > > > > + { formats::GRBG16_PISP_COMP1, {\n> > > > > > > > + .name = \"GRBG16_PISP_COMP1\",\n> > > > > > > > + .format = formats::GRBG16_PISP_COMP1,\n> > > > > > > > + .v4l2Formats = {\n> > > V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GRBG), },\n> > > > > > > > + .bitsPerPixel = 16,\n> > > > > > > > + .colourEncoding =\n> > > PixelFormatInfo::ColourEncodingRAW,\n> > > > > > > > + .packed = true,\n> > > > > > > > + .pixelsPerGroup = 2,\n> > > > > > > > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},\n> > > > > > > > + } },\n> > > > > > > > + { formats::RGGB16_PISP_COMP1, {\n> > > > > > > > + .name = \"RGGB16_PISP_COMP1\",\n> > > > > > > > + .format = formats::RGGB16_PISP_COMP1,\n> > > > > > > > + .v4l2Formats = {\n> > > V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_RGGB), },\n> > > > > > > > + .bitsPerPixel = 16,\n> > > > > > > > + .colourEncoding =\n> > > PixelFormatInfo::ColourEncodingRAW,\n> > > > > > > > + .packed = true,\n> > > > > > > > + .pixelsPerGroup = 2,\n> > > > > > > > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},\n> > > > > > > > + } },\n> > > > > > > > /* Compressed formats. */\n> > > > > > > > { formats::MJPEG, {\n> > > > > > > > .name = \"MJPEG\",\n> > > > > > > > diff --git a/src/libcamera/formats.yaml\n> > > b/src/libcamera/formats.yaml\n> > > > > > > > index bde2cc803b98..f6df721243d0 100644\n> > > > > > > > --- a/src/libcamera/formats.yaml\n> > > > > > > > +++ b/src/libcamera/formats.yaml\n> > > > > > > > @@ -190,4 +190,20 @@ formats:\n> > > > > > > > - SBGGR10_IPU3:\n> > > > > > > > fourcc: DRM_FORMAT_SBGGR10\n> > > > > > > > mod: IPU3_FORMAT_MOD_PACKED\n> > > > > > > > +\n> > > > > > > > + - RGGB16_PISP_COMP1:\n> > > > > > > > + fourcc: DRM_FORMAT_SRGGB16\n> > > > > > > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1\n> > > > > > > > + - GRBG16_PISP_COMP1:\n> > > > > > > > + fourcc: DRM_FORMAT_SGRBG16\n> > > > > > > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1\n> > > > > > > > + - GBRG16_PISP_COMP1:\n> > > > > > > > + fourcc: DRM_FORMAT_SGBRG16\n> > > > > > > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1\n> > > > > > > > + - BGGR16_PISP_COMP1:\n> > > > > > > > + fourcc: DRM_FORMAT_SBGGR16\n> > > > > > > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1\n> > > > > > > > + - MONO_PISP_COMP1:\n> > > > > > > > + fourcc: DRM_FORMAT_R16\n> > > > > > > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1\n> > > > > > > > ...\n> > > > > > > > diff --git a/src/libcamera/v4l2_pixelformat.cpp\n> > > b/src/libcamera/v4l2_pixelformat.cpp\n> > > > > > > > index efb6f2940235..47baaf60199d 100644\n> > > > > > > > --- a/src/libcamera/v4l2_pixelformat.cpp\n> > > > > > > > +++ b/src/libcamera/v4l2_pixelformat.cpp\n> > > > > > > > @@ -207,6 +207,16 @@ const std::map > > V4L2PixelFormat::Info> vpf2pf{\n> > > > > > > > { formats::SGRBG16, \"16-bit Bayer GRGR/BGBG\" } },\n> > > > > > > > { V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16),\n> > > > > > > > { formats::SRGGB16, \"16-bit Bayer RGRG/GBGB\" } },\n> > > > > > > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_BGGR),\n> > > > > > > > + { formats::BGGR16_PISP_COMP1, \"16-bit Bayer\n> > > BGBG/GRGR PiSP Compress Mode 1\" } },\n> > > > > > > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GBRG),\n> > > > > > > > + { formats::GBRG16_PISP_COMP1, \"16-bit Bayer\n> > > GBGB/RGRG PiSP Compress Mode 1\" } },\n> > > > > > > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GRBG),\n> > > > > > > > + { formats::GRBG16_PISP_COMP1, \"16-bit Bayer\n> > > GRGR/BGBG PiSP Compress Mode 1\" } },\n> > > > > > > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_RGGB),\n> > > > > > > > + { formats::RGGB16_PISP_COMP1, \"16-bit Bayer\n> > > RGRG/GBGB PiSP Compress Mode 1\" } },\n> > > > > > > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_MONO),\n> > > > > > > > + { formats::MONO_PISP_COMP1, \"16-bit Mono PiSP\n> > > Compress Mode 1\" } },\n> > > > > > > >\n> > > > > > > > /* Compressed formats. */\n> > > > > > > > { V4L2PixelFormat(V4L2_PIX_FMT_MJPEG),\n> > > > > > > > diff --git a/src/libcamera/v4l2_subdevice.cpp\n> > > b/src/libcamera/v4l2_subdevice.cpp\n> > > > > > > > index 6d0785b7b484..aea90abaf9ef 100644\n> > > > > > > > --- a/src/libcamera/v4l2_subdevice.cpp\n> > > > > > > > +++ b/src/libcamera/v4l2_subdevice.cpp\n> > > > > > > > @@ -134,6 +134,10 @@ const std::map > > V4L2SubdeviceFormatInfo> formatInfoMap = {\n> > > > > > > > { MEDIA_BUS_FMT_SGBRG12_1X12, { 12, \"SGBRG12_1X12\",\n> > > PixelFormatInfo::ColourEncodingRAW } },\n> > > > > > > > { MEDIA_BUS_FMT_SGRBG12_1X12, { 12, \"SGRBG12_1X12\",\n> > > PixelFormatInfo::ColourEncodingRAW } },\n> > > > > > > > { MEDIA_BUS_FMT_SRGGB12_1X12, { 12, \"SRGGB12_1X12\",\n> > > PixelFormatInfo::ColourEncodingRAW } },\n> > > > > > > > + { MEDIA_BUS_FMT_SBGGR16_1X16, { 16, \"SBGGR16_1x16\",\n> > > PixelFormatInfo::ColourEncodingRAW } },\n> > > > > > > > + { MEDIA_BUS_FMT_SGBRG16_1X16, { 16, \"SGBRG16_1x16\",\n> > > PixelFormatInfo::ColourEncodingRAW } },\n> > > > > > > > + { MEDIA_BUS_FMT_SGRBG16_1X16, { 16, \"SGRBG16_1x16\",\n> > > PixelFormatInfo::ColourEncodingRAW } },\n> > > > > > > > + { MEDIA_BUS_FMT_SRGGB16_1X16, { 16, \"SRGGB16_1x16\",\n> > > PixelFormatInfo::ColourEncodingRAW } },\n> > > > > > > > /* \\todo Clarify colour encoding for HSV formats */\n> > > > > > > > { MEDIA_BUS_FMT_AHSV8888_1X32, { 32, \"AHSV8888_1X32\",\n> > > PixelFormatInfo::ColourEncodingRGB } },\n> > > > > > > > { MEDIA_BUS_FMT_JPEG_1X8, { 8, \"JPEG_1X8\",\n> > > PixelFormatInfo::ColourEncodingYUV } },\n> > > > > > > > --\n> > > > > > > > 2.34.1\n> > > > > > > >\n> > >","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 12882BE080\n\tfor ;\n\tThu, 29 Feb 2024 08:19:26 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5B4EC6285F;\n\tThu, 29 Feb 2024 09:19:25 +0100 (CET)","from mail-yw1-x1132.google.com (mail-yw1-x1132.google.com\n\t[IPv6:2607:f8b0:4864:20::1132])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 32D8A61C96\n\tfor ;\n\tThu, 29 Feb 2024 09:19:23 +0100 (CET)","by mail-yw1-x1132.google.com with SMTP id\n\t00721157ae682-60925c4235eso6485517b3.3\n\tfor ;\n\tThu, 29 Feb 2024 00:19:23 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"JeDvJtOP\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1709194762; x=1709799562;\n\tdarn=lists.libcamera.org; \n\th=content-transfer-encoding:cc:to:subject:message-id:date:from\n\t:in-reply-to:references:mime-version:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=WQbtLLROwDwDsIFptwvFPM9A4TofBeR13xxd4/9gcXY=;\n\tb=JeDvJtOPRfA0CPYIqgH2qgWOraEZ6HZrHAS0Y0AodJA1/OwD7ISUii9sgS/Fj3jJVu\n\tdkav00kOdGirwje9OMyJ3tGRLilsfY/RcYfTXn7mDUlMMZWeWXzkoAwCxqd2At3muPS6\n\tZFb/U8XUnUXVZOGHgMgWzgbMSZFJdv0syqwYXRu7vMspde7L77gFDPZMTFHN4kJJ61IG\n\t/6laayJ3BdEpsrGxbGSmDZzM3tSSLc/iwyMDiaXNmrZiW+k5qtcNMyM9QFcCthI1rsvQ\n\tPEBxw5BDMzgdiM73ytEWPlPiPIY1z2v7iSflnJZ3XLsrIiDLCn0ULpjXhzjyENQImwFK\n\tzNeA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1709194762; x=1709799562;\n\th=content-transfer-encoding:cc:to:subject:message-id:date:from\n\t:in-reply-to:references:mime-version:x-gm-message-state:from:to:cc\n\t:subject:date:message-id:reply-to;\n\tbh=WQbtLLROwDwDsIFptwvFPM9A4TofBeR13xxd4/9gcXY=;\n\tb=M9/sbfoq+Q3BmwXN9PSTAyRdFTx8x4lQLogThroWk1nfYW0AUvT1mJR71x1oGY7RhE\n\tWN6bbJm1Hlz5/5z8gpJzPOS2kJGUcgRqxo0vd3mcYQ/EADfJgQ/XpV0e3GK6IK2PutkI\n\tomnMbCzBGXIVB1sDiWRGu3QhcAtdmUG1GNviDAhCd5Kr1xeV0OHIVhSH44yfbQe2ypCg\n\tBB9pYKDEND5ddrlPb/vIsvDjNq9fMmqt3SJksS0Gtl+5Fm56D1AwU7UpB21oooTjjXbZ\n\tsnxlbNfkHYBSlB7kvkzMqQJ+vEa+paUN9jFtZTyZHvX8R7tweDGk0w/cZM9nAzM5Xq+f\n\tHOIQ==","X-Gm-Message-State":"AOJu0YxIz2MV4PC7RMorxnjb3CvQVjr6/k31SmUAH2sH9gvgwxff0JaI\n\t3gpB6C7YPbdueS9ETXwWzBn/6uiAlAK6qVlLSiK7fq0aKMaOOPFNJTlcibtddU3/JsHEqkSiacK\n\tKQ5Hm1wmXsCp/bhGL9lv4rO8tAhLR9tW7i+WK1RMJAi++b8Zv","X-Google-Smtp-Source":"AGHT+IFwDLT7w3SvwUc+xQgYMylFGYvhTZA5w03oLzYourVa0iCl6mqiFNwyyPzozeJUZd6cGipG/uOcwM7fFFOcyDo=","X-Received":"by 2002:a81:9e42:0:b0:608:718c:c4e7 with SMTP id\n\tn2-20020a819e42000000b00608718cc4e7mr1482444ywj.43.1709194761938;\n\tThu, 29 Feb 2024 00:19:21 -0800 (PST)","MIME-Version":"1.0","References":"<20240215132710.810-1-naush@raspberrypi.com>\n\t<20240215132710.810-3-naush@raspberrypi.com>\n\t<74nstgdru2jpkry22imfmrcelvvzkfektet6cdo72gl5hqgz6t@vr6q6rtvti27>\n\t\n\t\n\t\n\t\n\t\n\t","In-Reply-To":"","From":"Naushir Patuck ","Date":"Thu, 29 Feb 2024 08:18:46 +0000","Message-ID":"","Subject":"Re: [PATCH v1 2/2] libcamera: formats: Add PiSP specific image and\n\tconfig buffer formats","To":"Jacopo Mondi ","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]