| Message ID | 20240215132710.810-3-naush@raspberrypi.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Hi Naush On Thu, Feb 15, 2024 at 01:27:10PM +0000, Naushir Patuck wrote: > Add the Raspberry Pi 5 PiSP specific compressed Bayer format types 1/2: > - V4L2_PIX_FMT_PISP_COMP1_xxx > - V4L2_PIX_FMT_PISP_COMP2_xxx > > Add the Raspberry Pi 5 PiSP Backend config format: > - V4L2_META_FMT_RPI_BE_CFG > > Additionally, we also extend libcamera format handlers to support 16-bit > Bayer formats across the media bus. > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > --- > include/libcamera/internal/bayer_format.h | 2 + > include/linux/drm_fourcc.h | 4 ++ > include/linux/videodev2.h | 15 +++++++ > src/libcamera/bayer_format.cpp | 18 ++++++++ > src/libcamera/formats.cpp | 51 ++++++++++++++++++++++- > src/libcamera/formats.yaml | 16 +++++++ > src/libcamera/v4l2_pixelformat.cpp | 10 +++++ > src/libcamera/v4l2_subdevice.cpp | 4 ++ > 8 files changed, 119 insertions(+), 1 deletion(-) > > diff --git a/include/libcamera/internal/bayer_format.h b/include/libcamera/internal/bayer_format.h > index 78ba3969913d..164743f7e9f6 100644 > --- a/include/libcamera/internal/bayer_format.h > +++ b/include/libcamera/internal/bayer_format.h > @@ -34,6 +34,8 @@ public: > None = 0, > CSI2 = 1, > IPU3 = 2, > + PISP1 = 3, > + PISP2 = 4, > }; > > constexpr BayerFormat() > diff --git a/include/linux/drm_fourcc.h b/include/linux/drm_fourcc.h > index 4ee421b95730..eff27fbe5a1e 100644 > --- a/include/linux/drm_fourcc.h > +++ b/include/linux/drm_fourcc.h > @@ -490,6 +490,7 @@ extern "C" { > #define DRM_FORMAT_MOD_VENDOR_ALLWINNER 0x09 > #define DRM_FORMAT_MOD_VENDOR_AMLOGIC 0x0a > #define DRM_FORMAT_MOD_VENDOR_MIPI 0x0b > +#define DRM_FORMAT_MOD_VENDOR_RPI 0x0c > > /* add more to the end as needed */ > > @@ -1670,6 +1671,9 @@ drm_fourcc_canonicalize_nvidia_format_mod(__u64 modifier) > */ > #define MIPI_FORMAT_MOD_CSI2_PACKED fourcc_mod_code(MIPI, 1) > > +#define PISP_FORMAT_MOD_COMPRESS_MODE1 fourcc_mod_code(RPI, 1) > +#define PISP_FORMAT_MOD_COMPRESS_MODE2 fourcc_mod_code(RPI, 2) > + I'll send a patch to DRM/KMS for these formats as well > #if defined(__cplusplus) > } > #endif > diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h > index ba48d2c89726..59af6f794680 100644 > --- a/include/linux/videodev2.h > +++ b/include/linux/videodev2.h > @@ -789,6 +789,18 @@ struct v4l2_pix_format { > #define V4L2_PIX_FMT_IPU3_SGRBG10 v4l2_fourcc('i', 'p', '3', 'G') /* IPU3 packed 10-bit GRBG bayer */ > #define V4L2_PIX_FMT_IPU3_SRGGB10 v4l2_fourcc('i', 'p', '3', 'r') /* IPU3 packed 10-bit RGGB bayer */ > > +/* The pixel format for all our buffers (the precise format is found in the config buffer). */ > +#define V4L2_PIX_FMT_PISP_COMP1_RGGB v4l2_fourcc('P', 'C', '1', 'R') > +#define V4L2_PIX_FMT_PISP_COMP1_GRBG v4l2_fourcc('P', 'C', '1', 'G') > +#define V4L2_PIX_FMT_PISP_COMP1_GBRG v4l2_fourcc('P', 'C', '1', 'g') > +#define V4L2_PIX_FMT_PISP_COMP1_BGGR v4l2_fourcc('P', 'C', '1', 'B') > +#define V4L2_PIX_FMT_PISP_COMP1_MONO v4l2_fourcc('P', 'C', '1', 'M') > +#define V4L2_PIX_FMT_PISP_COMP2_RGGB v4l2_fourcc('P', 'C', '2', 'R') > +#define V4L2_PIX_FMT_PISP_COMP2_GRBG v4l2_fourcc('P', 'C', '2', 'G') > +#define V4L2_PIX_FMT_PISP_COMP2_GBRG v4l2_fourcc('P', 'C', '2', 'g') > +#define V4L2_PIX_FMT_PISP_COMP2_BGGR v4l2_fourcc('P', 'C', '2', 'B') > +#define V4L2_PIX_FMT_PISP_COMP2_MONO v4l2_fourcc('P', 'C', '2', 'M') > + I would use here what has been sent upstream for the BE support /* Raspberry Pi PiSP compressed formats. */ #define V4L2_PIX_FMT_PISP_COMP1_RGGB v4l2_fourcc('P', 'C', '1', 'R') /* PiSP 8-bit mode 1 compressed RGGB bayer */ #define V4L2_PIX_FMT_PISP_COMP1_GRBG v4l2_fourcc('P', 'C', '1', 'G') /* PiSP 8-bit mode 1 compressed GRBG bayer */ #define V4L2_PIX_FMT_PISP_COMP1_GBRG v4l2_fourcc('P', 'C', '1', 'g') /* PiSP 8-bit mode 1 compressed GBRG bayer */ #define V4L2_PIX_FMT_PISP_COMP1_BGGR v4l2_fourcc('P', 'C', '1', 'B') /* PiSP 8-bit mode 1 compressed BGGR bayer */ #define V4L2_PIX_FMT_PISP_COMP1_MONO v4l2_fourcc('P', 'C', '1', 'M') /* PiSP 8-bit mode 1 compressed monochrome */ #define V4L2_PIX_FMT_PISP_COMP2_RGGB v4l2_fourcc('P', 'C', '2', 'R') /* PiSP 8-bit mode 2 compressed RGGB bayer */ #define V4L2_PIX_FMT_PISP_COMP2_GRBG v4l2_fourcc('P', 'C', '2', 'G') /* PiSP 8-bit mode 2 compressed GRBG bayer */ #define V4L2_PIX_FMT_PISP_COMP2_GBRG v4l2_fourcc('P', 'C', '2', 'g') /* PiSP 8-bit mode 2 compressed GBRG bayer */ #define V4L2_PIX_FMT_PISP_COMP2_BGGR v4l2_fourcc('P', 'C', '2', 'B') /* PiSP 8-bit mode 2 compressed BGGR bayer */ #define V4L2_PIX_FMT_PISP_COMP2_MONO v4l2_fourcc('P', 'C', '2', 'M') /* PiSP 8-bit mode 2 compressed monochrome */ To make it easier to update it when these will land in upstream > /* SDR formats - used only for Software Defined Radio devices */ > #define V4L2_SDR_FMT_CU8 v4l2_fourcc('C', 'U', '0', '8') /* IQ u8 */ > #define V4L2_SDR_FMT_CU16LE v4l2_fourcc('C', 'U', '1', '6') /* IQ u16le */ > @@ -818,6 +830,9 @@ struct v4l2_pix_format { > #define V4L2_META_FMT_RK_ISP1_PARAMS v4l2_fourcc('R', 'K', '1', 'P') /* Rockchip ISP1 3A Parameters */ > #define V4L2_META_FMT_RK_ISP1_STAT_3A v4l2_fourcc('R', 'K', '1', 'S') /* Rockchip ISP1 3A Statistics */ > > +/* Vendor specific - used for RaspberryPi PiSP */ > +#define V4L2_META_FMT_RPI_BE_CFG v4l2_fourcc('R', 'P', 'B', 'C') /* PiSP BE configuration */ > /* priv field value to indicates that subsequent fields are valid. */ > #define V4L2_PIX_FMT_PRIV_MAGIC 0xfeedcafe > > diff --git a/src/libcamera/bayer_format.cpp b/src/libcamera/bayer_format.cpp > index 20aedfa6d925..333b1117f531 100644 > --- a/src/libcamera/bayer_format.cpp > +++ b/src/libcamera/bayer_format.cpp > @@ -164,6 +164,14 @@ const std::map<BayerFormat, Formats, BayerFormatComparator> bayerToFormat{ > { formats::SGRBG16, V4L2PixelFormat(V4L2_PIX_FMT_SGRBG16) } }, > { { BayerFormat::RGGB, 16, BayerFormat::Packing::None }, > { formats::SRGGB16, V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16) } }, > + { { BayerFormat::BGGR, 16, BayerFormat::Packing::PISP1 }, > + { formats::BGGR16_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_BGGR) } }, > + { { BayerFormat::GBRG, 16, BayerFormat::Packing::PISP1 }, > + { formats::GBRG16_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GBRG) } }, > + { { BayerFormat::GRBG, 16, BayerFormat::Packing::PISP1 }, > + { formats::GRBG16_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GRBG) } }, > + { { BayerFormat::RGGB, 16, BayerFormat::Packing::PISP1 }, > + { formats::RGGB16_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_RGGB) } }, > { { BayerFormat::MONO, 8, BayerFormat::Packing::None }, > { formats::R8, V4L2PixelFormat(V4L2_PIX_FMT_GREY) } }, > { { BayerFormat::MONO, 10, BayerFormat::Packing::None }, > @@ -174,6 +182,8 @@ const std::map<BayerFormat, Formats, BayerFormatComparator> bayerToFormat{ > { formats::R12, V4L2PixelFormat(V4L2_PIX_FMT_Y12) } }, > { { BayerFormat::MONO, 16, BayerFormat::Packing::None }, > { formats::R16, V4L2PixelFormat(V4L2_PIX_FMT_Y16) } }, > + { { BayerFormat::MONO, 16, BayerFormat::Packing::PISP1 }, > + { formats::MONO_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_MONO) } }, Should the COMP2 versions be added as well ? > }; > > const std::unordered_map<unsigned int, BayerFormat> mbusCodeToBayer{ > @@ -209,6 +219,10 @@ const std::unordered_map<unsigned int, BayerFormat> mbusCodeToBayer{ > { MEDIA_BUS_FMT_SGBRG16_1X16, { BayerFormat::GBRG, 16, BayerFormat::Packing::None } }, > { MEDIA_BUS_FMT_SGRBG16_1X16, { BayerFormat::GRBG, 16, BayerFormat::Packing::None } }, > { MEDIA_BUS_FMT_SRGGB16_1X16, { BayerFormat::RGGB, 16, BayerFormat::Packing::None } }, > + { MEDIA_BUS_FMT_SBGGR16_1X16, { BayerFormat::BGGR, 16, BayerFormat::Packing::PISP1 } }, > + { MEDIA_BUS_FMT_SGBRG16_1X16, { BayerFormat::GBRG, 16, BayerFormat::Packing::PISP1 } }, > + { MEDIA_BUS_FMT_SGRBG16_1X16, { BayerFormat::GRBG, 16, BayerFormat::Packing::PISP1 } }, > + { MEDIA_BUS_FMT_SRGGB16_1X16, { BayerFormat::RGGB, 16, BayerFormat::Packing::PISP1 } }, mbusCodeToBayer is an unordered_map which "is an associative container that contains key-value pairs with unique keys." I'm afraid adding a new entry with the same key overwrite the existing one ? We already had issues with this map, as we had similar problems with the _LE and _BE version of RGB565 which resolv to the same BayerFormat { MEDIA_BUS_FMT_SBGGR10_2X8_PADHI_BE, { BayerFormat::BGGR, 10, BayerFormat::Packing::None } }, { MEDIA_BUS_FMT_SBGGR10_2X8_PADHI_LE, { BayerFormat::BGGR, 10, BayerFormat::Packing::None } }, { MEDIA_BUS_FMT_SBGGR10_2X8_PADLO_BE, { BayerFormat::BGGR, 10, BayerFormat::Packing::None } }, { MEDIA_BUS_FMT_SBGGR10_2X8_PADLO_LE, { BayerFormat::BGGR, 10, BayerFormat::Packing::None } }, In general, we know that resolving mbus_codes to a unique format is ill-defined. How are you using these entries in the pi5 support ? > { MEDIA_BUS_FMT_Y8_1X8, { BayerFormat::MONO, 8, BayerFormat::Packing::None } }, > { MEDIA_BUS_FMT_Y10_1X10, { BayerFormat::MONO, 10, BayerFormat::Packing::None } }, > { MEDIA_BUS_FMT_Y12_1X12, { BayerFormat::MONO, 12, BayerFormat::Packing::None } }, > @@ -303,6 +317,10 @@ std::ostream &operator<<(std::ostream &out, const BayerFormat &f) > out << "-CSI2P"; > else if (f.packing == BayerFormat::Packing::IPU3) > out << "-IPU3P"; > + else if (f.packing == BayerFormat::Packing::PISP1) > + out << "-PISP1"; > + else if (f.packing == BayerFormat::Packing::PISP2) > + out << "-PISP2"; > > return out; > } > diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp > index a674f4179cc8..e603a7eda579 100644 > --- a/src/libcamera/formats.cpp > +++ b/src/libcamera/formats.cpp > @@ -547,6 +547,16 @@ const std::map<PixelFormat, PixelFormatInfo> pixelFormatInfo{ > .pixelsPerGroup = 1, > .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, > } }, > + { formats::MONO_PISP_COMP1, { > + .name = "MONO_PISP_COMP1", > + .format = formats::MONO_PISP_COMP1, > + .v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_MONO), }, > + .bitsPerPixel = 16, > + .colourEncoding = PixelFormatInfo::ColourEncodingYUV, > + .packed = true, > + .pixelsPerGroup = 1, > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, > + } }, > > /* Bayer formats. */ > { formats::SBGGR8, { > @@ -910,7 +920,46 @@ const std::map<PixelFormat, PixelFormatInfo> pixelFormatInfo{ > .pixelsPerGroup = 25, > .planes = {{ { 32, 1 }, { 0, 0 }, { 0, 0 } }}, > } }, > - > + { formats::BGGR16_PISP_COMP1, { > + .name = "BGGR16_PISP_COMP1", > + .format = formats::BGGR16_PISP_COMP1, > + .v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_BGGR), }, > + .bitsPerPixel = 16, > + .colourEncoding = PixelFormatInfo::ColourEncodingRAW, > + .packed = true, > + .pixelsPerGroup = 2, > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, > + } }, For regular 16-bit formats this should be .pixelsPerGroup = 2, .planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }}, As 2 pixels of 16 bits each consume 4 bytes. Is this {2, 1} intentional and due to compression ? > + { formats::GBRG16_PISP_COMP1, { > + .name = "GBRG16_PISP_COMP1", > + .format = formats::GBRG16_PISP_COMP1, > + .v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GBRG), }, > + .bitsPerPixel = 16, > + .colourEncoding = PixelFormatInfo::ColourEncodingRAW, > + .packed = true, > + .pixelsPerGroup = 2, > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, > + } }, > + { formats::GRBG16_PISP_COMP1, { > + .name = "GRBG16_PISP_COMP1", > + .format = formats::GRBG16_PISP_COMP1, > + .v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GRBG), }, > + .bitsPerPixel = 16, > + .colourEncoding = PixelFormatInfo::ColourEncodingRAW, > + .packed = true, > + .pixelsPerGroup = 2, > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, > + } }, > + { formats::RGGB16_PISP_COMP1, { > + .name = "RGGB16_PISP_COMP1", > + .format = formats::RGGB16_PISP_COMP1, > + .v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_RGGB), }, > + .bitsPerPixel = 16, > + .colourEncoding = PixelFormatInfo::ColourEncodingRAW, > + .packed = true, > + .pixelsPerGroup = 2, > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, > + } }, > /* Compressed formats. */ > { formats::MJPEG, { > .name = "MJPEG", > diff --git a/src/libcamera/formats.yaml b/src/libcamera/formats.yaml > index bde2cc803b98..f6df721243d0 100644 > --- a/src/libcamera/formats.yaml > +++ b/src/libcamera/formats.yaml > @@ -190,4 +190,20 @@ formats: > - SBGGR10_IPU3: > fourcc: DRM_FORMAT_SBGGR10 > mod: IPU3_FORMAT_MOD_PACKED > + > + - RGGB16_PISP_COMP1: > + fourcc: DRM_FORMAT_SRGGB16 > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1 > + - GRBG16_PISP_COMP1: > + fourcc: DRM_FORMAT_SGRBG16 > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1 > + - GBRG16_PISP_COMP1: > + fourcc: DRM_FORMAT_SGBRG16 > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1 > + - BGGR16_PISP_COMP1: > + fourcc: DRM_FORMAT_SBGGR16 > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1 > + - MONO_PISP_COMP1: > + fourcc: DRM_FORMAT_R16 > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1 > ... > diff --git a/src/libcamera/v4l2_pixelformat.cpp b/src/libcamera/v4l2_pixelformat.cpp > index efb6f2940235..47baaf60199d 100644 > --- a/src/libcamera/v4l2_pixelformat.cpp > +++ b/src/libcamera/v4l2_pixelformat.cpp > @@ -207,6 +207,16 @@ const std::map<V4L2PixelFormat, V4L2PixelFormat::Info> vpf2pf{ > { formats::SGRBG16, "16-bit Bayer GRGR/BGBG" } }, > { V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16), > { formats::SRGGB16, "16-bit Bayer RGRG/GBGB" } }, > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_BGGR), > + { formats::BGGR16_PISP_COMP1, "16-bit Bayer BGBG/GRGR PiSP Compress Mode 1" } }, > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GBRG), > + { formats::GBRG16_PISP_COMP1, "16-bit Bayer GBGB/RGRG PiSP Compress Mode 1" } }, > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GRBG), > + { formats::GRBG16_PISP_COMP1, "16-bit Bayer GRGR/BGBG PiSP Compress Mode 1" } }, > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_RGGB), > + { formats::RGGB16_PISP_COMP1, "16-bit Bayer RGRG/GBGB PiSP Compress Mode 1" } }, > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_MONO), > + { formats::MONO_PISP_COMP1, "16-bit Mono PiSP Compress Mode 1" } }, > > /* Compressed formats. */ > { V4L2PixelFormat(V4L2_PIX_FMT_MJPEG), > diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp > index 6d0785b7b484..aea90abaf9ef 100644 > --- a/src/libcamera/v4l2_subdevice.cpp > +++ b/src/libcamera/v4l2_subdevice.cpp > @@ -134,6 +134,10 @@ const std::map<uint32_t, V4L2SubdeviceFormatInfo> formatInfoMap = { > { MEDIA_BUS_FMT_SGBRG12_1X12, { 12, "SGBRG12_1X12", PixelFormatInfo::ColourEncodingRAW } }, > { MEDIA_BUS_FMT_SGRBG12_1X12, { 12, "SGRBG12_1X12", PixelFormatInfo::ColourEncodingRAW } }, > { MEDIA_BUS_FMT_SRGGB12_1X12, { 12, "SRGGB12_1X12", PixelFormatInfo::ColourEncodingRAW } }, > + { MEDIA_BUS_FMT_SBGGR16_1X16, { 16, "SBGGR16_1x16", PixelFormatInfo::ColourEncodingRAW } }, > + { MEDIA_BUS_FMT_SGBRG16_1X16, { 16, "SGBRG16_1x16", PixelFormatInfo::ColourEncodingRAW } }, > + { MEDIA_BUS_FMT_SGRBG16_1X16, { 16, "SGRBG16_1x16", PixelFormatInfo::ColourEncodingRAW } }, > + { MEDIA_BUS_FMT_SRGGB16_1X16, { 16, "SRGGB16_1x16", PixelFormatInfo::ColourEncodingRAW } }, > /* \todo Clarify colour encoding for HSV formats */ > { MEDIA_BUS_FMT_AHSV8888_1X32, { 32, "AHSV8888_1X32", PixelFormatInfo::ColourEncodingRGB } }, > { MEDIA_BUS_FMT_JPEG_1X8, { 8, "JPEG_1X8", PixelFormatInfo::ColourEncodingYUV } }, > -- > 2.34.1 >
Actually... On Mon, Feb 26, 2024 at 02:49:09PM +0100, Jacopo Mondi wrote: > Hi Naush > > On Thu, Feb 15, 2024 at 01:27:10PM +0000, Naushir Patuck wrote: > > Add the Raspberry Pi 5 PiSP specific compressed Bayer format types 1/2: > > - V4L2_PIX_FMT_PISP_COMP1_xxx > > - V4L2_PIX_FMT_PISP_COMP2_xxx > > > > Add the Raspberry Pi 5 PiSP Backend config format: > > - V4L2_META_FMT_RPI_BE_CFG > > > > Additionally, we also extend libcamera format handlers to support 16-bit > > Bayer formats across the media bus. > > > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > > --- > > include/libcamera/internal/bayer_format.h | 2 + > > include/linux/drm_fourcc.h | 4 ++ > > include/linux/videodev2.h | 15 +++++++ > > src/libcamera/bayer_format.cpp | 18 ++++++++ > > src/libcamera/formats.cpp | 51 ++++++++++++++++++++++- > > src/libcamera/formats.yaml | 16 +++++++ > > src/libcamera/v4l2_pixelformat.cpp | 10 +++++ > > src/libcamera/v4l2_subdevice.cpp | 4 ++ > > 8 files changed, 119 insertions(+), 1 deletion(-) > > > > diff --git a/include/libcamera/internal/bayer_format.h b/include/libcamera/internal/bayer_format.h > > index 78ba3969913d..164743f7e9f6 100644 > > --- a/include/libcamera/internal/bayer_format.h > > +++ b/include/libcamera/internal/bayer_format.h > > @@ -34,6 +34,8 @@ public: > > None = 0, > > CSI2 = 1, > > IPU3 = 2, > > + PISP1 = 3, > > + PISP2 = 4, > > }; > > > > constexpr BayerFormat() > > diff --git a/include/linux/drm_fourcc.h b/include/linux/drm_fourcc.h > > index 4ee421b95730..eff27fbe5a1e 100644 > > --- a/include/linux/drm_fourcc.h > > +++ b/include/linux/drm_fourcc.h > > @@ -490,6 +490,7 @@ extern "C" { > > #define DRM_FORMAT_MOD_VENDOR_ALLWINNER 0x09 > > #define DRM_FORMAT_MOD_VENDOR_AMLOGIC 0x0a > > #define DRM_FORMAT_MOD_VENDOR_MIPI 0x0b > > +#define DRM_FORMAT_MOD_VENDOR_RPI 0x0c > > > > /* add more to the end as needed */ > > > > @@ -1670,6 +1671,9 @@ drm_fourcc_canonicalize_nvidia_format_mod(__u64 modifier) > > */ > > #define MIPI_FORMAT_MOD_CSI2_PACKED fourcc_mod_code(MIPI, 1) > > > > +#define PISP_FORMAT_MOD_COMPRESS_MODE1 fourcc_mod_code(RPI, 1) > > +#define PISP_FORMAT_MOD_COMPRESS_MODE2 fourcc_mod_code(RPI, 2) > > + > > I'll send a patch to DRM/KMS for these formats as well > Can these be upstreamed if no DRM format is actually using them ? Or should we add a DRM FourCC for the compressed formats, even if they won't be used outside of the PiSP FE/BE loop ? > > #if defined(__cplusplus) > > } > > #endif > > diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h > > index ba48d2c89726..59af6f794680 100644 > > --- a/include/linux/videodev2.h > > +++ b/include/linux/videodev2.h > > @@ -789,6 +789,18 @@ struct v4l2_pix_format { > > #define V4L2_PIX_FMT_IPU3_SGRBG10 v4l2_fourcc('i', 'p', '3', 'G') /* IPU3 packed 10-bit GRBG bayer */ > > #define V4L2_PIX_FMT_IPU3_SRGGB10 v4l2_fourcc('i', 'p', '3', 'r') /* IPU3 packed 10-bit RGGB bayer */ > > > > +/* The pixel format for all our buffers (the precise format is found in the config buffer). */ > > +#define V4L2_PIX_FMT_PISP_COMP1_RGGB v4l2_fourcc('P', 'C', '1', 'R') > > +#define V4L2_PIX_FMT_PISP_COMP1_GRBG v4l2_fourcc('P', 'C', '1', 'G') > > +#define V4L2_PIX_FMT_PISP_COMP1_GBRG v4l2_fourcc('P', 'C', '1', 'g') > > +#define V4L2_PIX_FMT_PISP_COMP1_BGGR v4l2_fourcc('P', 'C', '1', 'B') > > +#define V4L2_PIX_FMT_PISP_COMP1_MONO v4l2_fourcc('P', 'C', '1', 'M') > > +#define V4L2_PIX_FMT_PISP_COMP2_RGGB v4l2_fourcc('P', 'C', '2', 'R') > > +#define V4L2_PIX_FMT_PISP_COMP2_GRBG v4l2_fourcc('P', 'C', '2', 'G') > > +#define V4L2_PIX_FMT_PISP_COMP2_GBRG v4l2_fourcc('P', 'C', '2', 'g') > > +#define V4L2_PIX_FMT_PISP_COMP2_BGGR v4l2_fourcc('P', 'C', '2', 'B') > > +#define V4L2_PIX_FMT_PISP_COMP2_MONO v4l2_fourcc('P', 'C', '2', 'M') > > + > > I would use here what has been sent upstream for the BE support > > /* Raspberry Pi PiSP compressed formats. */ > #define V4L2_PIX_FMT_PISP_COMP1_RGGB v4l2_fourcc('P', 'C', '1', 'R') /* PiSP 8-bit mode 1 compressed RGGB bayer */ > #define V4L2_PIX_FMT_PISP_COMP1_GRBG v4l2_fourcc('P', 'C', '1', 'G') /* PiSP 8-bit mode 1 compressed GRBG bayer */ > #define V4L2_PIX_FMT_PISP_COMP1_GBRG v4l2_fourcc('P', 'C', '1', 'g') /* PiSP 8-bit mode 1 compressed GBRG bayer */ > #define V4L2_PIX_FMT_PISP_COMP1_BGGR v4l2_fourcc('P', 'C', '1', 'B') /* PiSP 8-bit mode 1 compressed BGGR bayer */ > #define V4L2_PIX_FMT_PISP_COMP1_MONO v4l2_fourcc('P', 'C', '1', 'M') /* PiSP 8-bit mode 1 compressed monochrome */ > #define V4L2_PIX_FMT_PISP_COMP2_RGGB v4l2_fourcc('P', 'C', '2', 'R') /* PiSP 8-bit mode 2 compressed RGGB bayer */ > #define V4L2_PIX_FMT_PISP_COMP2_GRBG v4l2_fourcc('P', 'C', '2', 'G') /* PiSP 8-bit mode 2 compressed GRBG bayer */ > #define V4L2_PIX_FMT_PISP_COMP2_GBRG v4l2_fourcc('P', 'C', '2', 'g') /* PiSP 8-bit mode 2 compressed GBRG bayer */ > #define V4L2_PIX_FMT_PISP_COMP2_BGGR v4l2_fourcc('P', 'C', '2', 'B') /* PiSP 8-bit mode 2 compressed BGGR bayer */ > #define V4L2_PIX_FMT_PISP_COMP2_MONO v4l2_fourcc('P', 'C', '2', 'M') /* PiSP 8-bit mode 2 compressed monochrome */ > > To make it easier to update it when these will land in upstream > > > /* SDR formats - used only for Software Defined Radio devices */ > > #define V4L2_SDR_FMT_CU8 v4l2_fourcc('C', 'U', '0', '8') /* IQ u8 */ > > #define V4L2_SDR_FMT_CU16LE v4l2_fourcc('C', 'U', '1', '6') /* IQ u16le */ > > @@ -818,6 +830,9 @@ struct v4l2_pix_format { > > #define V4L2_META_FMT_RK_ISP1_PARAMS v4l2_fourcc('R', 'K', '1', 'P') /* Rockchip ISP1 3A Parameters */ > > #define V4L2_META_FMT_RK_ISP1_STAT_3A v4l2_fourcc('R', 'K', '1', 'S') /* Rockchip ISP1 3A Statistics */ > > > > +/* Vendor specific - used for RaspberryPi PiSP */ > > +#define V4L2_META_FMT_RPI_BE_CFG v4l2_fourcc('R', 'P', 'B', 'C') /* PiSP BE configuration */ > > /* priv field value to indicates that subsequent fields are valid. */ > > #define V4L2_PIX_FMT_PRIV_MAGIC 0xfeedcafe > > > > diff --git a/src/libcamera/bayer_format.cpp b/src/libcamera/bayer_format.cpp > > index 20aedfa6d925..333b1117f531 100644 > > --- a/src/libcamera/bayer_format.cpp > > +++ b/src/libcamera/bayer_format.cpp > > @@ -164,6 +164,14 @@ const std::map<BayerFormat, Formats, BayerFormatComparator> bayerToFormat{ > > { formats::SGRBG16, V4L2PixelFormat(V4L2_PIX_FMT_SGRBG16) } }, > > { { BayerFormat::RGGB, 16, BayerFormat::Packing::None }, > > { formats::SRGGB16, V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16) } }, > > + { { BayerFormat::BGGR, 16, BayerFormat::Packing::PISP1 }, > > + { formats::BGGR16_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_BGGR) } }, > > + { { BayerFormat::GBRG, 16, BayerFormat::Packing::PISP1 }, > > + { formats::GBRG16_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GBRG) } }, > > + { { BayerFormat::GRBG, 16, BayerFormat::Packing::PISP1 }, > > + { formats::GRBG16_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GRBG) } }, > > + { { BayerFormat::RGGB, 16, BayerFormat::Packing::PISP1 }, > > + { formats::RGGB16_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_RGGB) } }, > > { { BayerFormat::MONO, 8, BayerFormat::Packing::None }, > > { formats::R8, V4L2PixelFormat(V4L2_PIX_FMT_GREY) } }, > > { { BayerFormat::MONO, 10, BayerFormat::Packing::None }, > > @@ -174,6 +182,8 @@ const std::map<BayerFormat, Formats, BayerFormatComparator> bayerToFormat{ > > { formats::R12, V4L2PixelFormat(V4L2_PIX_FMT_Y12) } }, > > { { BayerFormat::MONO, 16, BayerFormat::Packing::None }, > > { formats::R16, V4L2PixelFormat(V4L2_PIX_FMT_Y16) } }, > > + { { BayerFormat::MONO, 16, BayerFormat::Packing::PISP1 }, > > + { formats::MONO_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_MONO) } }, > > Should the COMP2 versions be added as well ? > > > }; > > > > const std::unordered_map<unsigned int, BayerFormat> mbusCodeToBayer{ > > @@ -209,6 +219,10 @@ const std::unordered_map<unsigned int, BayerFormat> mbusCodeToBayer{ > > { MEDIA_BUS_FMT_SGBRG16_1X16, { BayerFormat::GBRG, 16, BayerFormat::Packing::None } }, > > { MEDIA_BUS_FMT_SGRBG16_1X16, { BayerFormat::GRBG, 16, BayerFormat::Packing::None } }, > > { MEDIA_BUS_FMT_SRGGB16_1X16, { BayerFormat::RGGB, 16, BayerFormat::Packing::None } }, > > + { MEDIA_BUS_FMT_SBGGR16_1X16, { BayerFormat::BGGR, 16, BayerFormat::Packing::PISP1 } }, > > + { MEDIA_BUS_FMT_SGBRG16_1X16, { BayerFormat::GBRG, 16, BayerFormat::Packing::PISP1 } }, > > + { MEDIA_BUS_FMT_SGRBG16_1X16, { BayerFormat::GRBG, 16, BayerFormat::Packing::PISP1 } }, > > + { MEDIA_BUS_FMT_SRGGB16_1X16, { BayerFormat::RGGB, 16, BayerFormat::Packing::PISP1 } }, > > mbusCodeToBayer is an unordered_map which > "is an associative container that contains key-value pairs with unique > keys." > > I'm afraid adding a new entry with the same key overwrite the existing > one ? > > We already had issues with this map, as we had similar problems with the > _LE and _BE version of RGB565 which resolv to the same BayerFormat > > { MEDIA_BUS_FMT_SBGGR10_2X8_PADHI_BE, { BayerFormat::BGGR, 10, BayerFormat::Packing::None } }, > { MEDIA_BUS_FMT_SBGGR10_2X8_PADHI_LE, { BayerFormat::BGGR, 10, BayerFormat::Packing::None } }, > { MEDIA_BUS_FMT_SBGGR10_2X8_PADLO_BE, { BayerFormat::BGGR, 10, BayerFormat::Packing::None } }, > { MEDIA_BUS_FMT_SBGGR10_2X8_PADLO_LE, { BayerFormat::BGGR, 10, BayerFormat::Packing::None } }, > > In general, we know that resolving mbus_codes to a unique format is > ill-defined. How are you using these entries in the pi5 support ? > > > { MEDIA_BUS_FMT_Y8_1X8, { BayerFormat::MONO, 8, BayerFormat::Packing::None } }, > > { MEDIA_BUS_FMT_Y10_1X10, { BayerFormat::MONO, 10, BayerFormat::Packing::None } }, > > { MEDIA_BUS_FMT_Y12_1X12, { BayerFormat::MONO, 12, BayerFormat::Packing::None } }, > > @@ -303,6 +317,10 @@ std::ostream &operator<<(std::ostream &out, const BayerFormat &f) > > out << "-CSI2P"; > > else if (f.packing == BayerFormat::Packing::IPU3) > > out << "-IPU3P"; > > + else if (f.packing == BayerFormat::Packing::PISP1) > > + out << "-PISP1"; > > + else if (f.packing == BayerFormat::Packing::PISP2) > > + out << "-PISP2"; > > > > return out; > > } > > diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp > > index a674f4179cc8..e603a7eda579 100644 > > --- a/src/libcamera/formats.cpp > > +++ b/src/libcamera/formats.cpp > > @@ -547,6 +547,16 @@ const std::map<PixelFormat, PixelFormatInfo> pixelFormatInfo{ > > .pixelsPerGroup = 1, > > .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, > > } }, > > + { formats::MONO_PISP_COMP1, { > > + .name = "MONO_PISP_COMP1", > > + .format = formats::MONO_PISP_COMP1, > > + .v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_MONO), }, > > + .bitsPerPixel = 16, > > + .colourEncoding = PixelFormatInfo::ColourEncodingYUV, > > + .packed = true, > > + .pixelsPerGroup = 1, > > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, > > + } }, > > > > /* Bayer formats. */ > > { formats::SBGGR8, { > > @@ -910,7 +920,46 @@ const std::map<PixelFormat, PixelFormatInfo> pixelFormatInfo{ > > .pixelsPerGroup = 25, > > .planes = {{ { 32, 1 }, { 0, 0 }, { 0, 0 } }}, > > } }, > > - > > + { formats::BGGR16_PISP_COMP1, { > > + .name = "BGGR16_PISP_COMP1", > > + .format = formats::BGGR16_PISP_COMP1, > > + .v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_BGGR), }, > > + .bitsPerPixel = 16, > > + .colourEncoding = PixelFormatInfo::ColourEncodingRAW, > > + .packed = true, > > + .pixelsPerGroup = 2, > > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, > > + } }, > > For regular 16-bit formats this should be > > .pixelsPerGroup = 2, > .planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }}, > > As 2 pixels of 16 bits each consume 4 bytes. Is this {2, 1} > intentional and due to compression ? > > > + { formats::GBRG16_PISP_COMP1, { > > + .name = "GBRG16_PISP_COMP1", > > + .format = formats::GBRG16_PISP_COMP1, > > + .v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GBRG), }, > > + .bitsPerPixel = 16, > > + .colourEncoding = PixelFormatInfo::ColourEncodingRAW, > > + .packed = true, > > + .pixelsPerGroup = 2, > > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, > > + } }, > > + { formats::GRBG16_PISP_COMP1, { > > + .name = "GRBG16_PISP_COMP1", > > + .format = formats::GRBG16_PISP_COMP1, > > + .v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GRBG), }, > > + .bitsPerPixel = 16, > > + .colourEncoding = PixelFormatInfo::ColourEncodingRAW, > > + .packed = true, > > + .pixelsPerGroup = 2, > > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, > > + } }, > > + { formats::RGGB16_PISP_COMP1, { > > + .name = "RGGB16_PISP_COMP1", > > + .format = formats::RGGB16_PISP_COMP1, > > + .v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_RGGB), }, > > + .bitsPerPixel = 16, > > + .colourEncoding = PixelFormatInfo::ColourEncodingRAW, > > + .packed = true, > > + .pixelsPerGroup = 2, > > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, > > + } }, > > /* Compressed formats. */ > > { formats::MJPEG, { > > .name = "MJPEG", > > diff --git a/src/libcamera/formats.yaml b/src/libcamera/formats.yaml > > index bde2cc803b98..f6df721243d0 100644 > > --- a/src/libcamera/formats.yaml > > +++ b/src/libcamera/formats.yaml > > @@ -190,4 +190,20 @@ formats: > > - SBGGR10_IPU3: > > fourcc: DRM_FORMAT_SBGGR10 > > mod: IPU3_FORMAT_MOD_PACKED > > + > > + - RGGB16_PISP_COMP1: > > + fourcc: DRM_FORMAT_SRGGB16 > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1 > > + - GRBG16_PISP_COMP1: > > + fourcc: DRM_FORMAT_SGRBG16 > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1 > > + - GBRG16_PISP_COMP1: > > + fourcc: DRM_FORMAT_SGBRG16 > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1 > > + - BGGR16_PISP_COMP1: > > + fourcc: DRM_FORMAT_SBGGR16 > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1 > > + - MONO_PISP_COMP1: > > + fourcc: DRM_FORMAT_R16 > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1 > > ... > > diff --git a/src/libcamera/v4l2_pixelformat.cpp b/src/libcamera/v4l2_pixelformat.cpp > > index efb6f2940235..47baaf60199d 100644 > > --- a/src/libcamera/v4l2_pixelformat.cpp > > +++ b/src/libcamera/v4l2_pixelformat.cpp > > @@ -207,6 +207,16 @@ const std::map<V4L2PixelFormat, V4L2PixelFormat::Info> vpf2pf{ > > { formats::SGRBG16, "16-bit Bayer GRGR/BGBG" } }, > > { V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16), > > { formats::SRGGB16, "16-bit Bayer RGRG/GBGB" } }, > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_BGGR), > > + { formats::BGGR16_PISP_COMP1, "16-bit Bayer BGBG/GRGR PiSP Compress Mode 1" } }, > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GBRG), > > + { formats::GBRG16_PISP_COMP1, "16-bit Bayer GBGB/RGRG PiSP Compress Mode 1" } }, > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GRBG), > > + { formats::GRBG16_PISP_COMP1, "16-bit Bayer GRGR/BGBG PiSP Compress Mode 1" } }, > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_RGGB), > > + { formats::RGGB16_PISP_COMP1, "16-bit Bayer RGRG/GBGB PiSP Compress Mode 1" } }, > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_MONO), > > + { formats::MONO_PISP_COMP1, "16-bit Mono PiSP Compress Mode 1" } }, > > > > /* Compressed formats. */ > > { V4L2PixelFormat(V4L2_PIX_FMT_MJPEG), > > diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp > > index 6d0785b7b484..aea90abaf9ef 100644 > > --- a/src/libcamera/v4l2_subdevice.cpp > > +++ b/src/libcamera/v4l2_subdevice.cpp > > @@ -134,6 +134,10 @@ const std::map<uint32_t, V4L2SubdeviceFormatInfo> formatInfoMap = { > > { MEDIA_BUS_FMT_SGBRG12_1X12, { 12, "SGBRG12_1X12", PixelFormatInfo::ColourEncodingRAW } }, > > { MEDIA_BUS_FMT_SGRBG12_1X12, { 12, "SGRBG12_1X12", PixelFormatInfo::ColourEncodingRAW } }, > > { MEDIA_BUS_FMT_SRGGB12_1X12, { 12, "SRGGB12_1X12", PixelFormatInfo::ColourEncodingRAW } }, > > + { MEDIA_BUS_FMT_SBGGR16_1X16, { 16, "SBGGR16_1x16", PixelFormatInfo::ColourEncodingRAW } }, > > + { MEDIA_BUS_FMT_SGBRG16_1X16, { 16, "SGBRG16_1x16", PixelFormatInfo::ColourEncodingRAW } }, > > + { MEDIA_BUS_FMT_SGRBG16_1X16, { 16, "SGRBG16_1x16", PixelFormatInfo::ColourEncodingRAW } }, > > + { MEDIA_BUS_FMT_SRGGB16_1X16, { 16, "SRGGB16_1x16", PixelFormatInfo::ColourEncodingRAW } }, > > /* \todo Clarify colour encoding for HSV formats */ > > { MEDIA_BUS_FMT_AHSV8888_1X32, { 32, "AHSV8888_1X32", PixelFormatInfo::ColourEncodingRGB } }, > > { MEDIA_BUS_FMT_JPEG_1X8, { 8, "JPEG_1X8", PixelFormatInfo::ColourEncodingYUV } }, > > -- > > 2.34.1 > >
On Mon, Feb 26, 2024 at 02:55:41PM +0100, Jacopo Mondi wrote: > Actually... > > On Mon, Feb 26, 2024 at 02:49:09PM +0100, Jacopo Mondi wrote: > > Hi Naush > > > > On Thu, Feb 15, 2024 at 01:27:10PM +0000, Naushir Patuck wrote: > > > Add the Raspberry Pi 5 PiSP specific compressed Bayer format types 1/2: > > > - V4L2_PIX_FMT_PISP_COMP1_xxx > > > - V4L2_PIX_FMT_PISP_COMP2_xxx > > > > > > Add the Raspberry Pi 5 PiSP Backend config format: > > > - V4L2_META_FMT_RPI_BE_CFG > > > > > > Additionally, we also extend libcamera format handlers to support 16-bit > > > Bayer formats across the media bus. > > > > > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > > > --- > > > include/libcamera/internal/bayer_format.h | 2 + > > > include/linux/drm_fourcc.h | 4 ++ > > > include/linux/videodev2.h | 15 +++++++ > > > src/libcamera/bayer_format.cpp | 18 ++++++++ > > > src/libcamera/formats.cpp | 51 ++++++++++++++++++++++- > > > src/libcamera/formats.yaml | 16 +++++++ > > > src/libcamera/v4l2_pixelformat.cpp | 10 +++++ > > > src/libcamera/v4l2_subdevice.cpp | 4 ++ > > > 8 files changed, 119 insertions(+), 1 deletion(-) > > > > > > diff --git a/include/libcamera/internal/bayer_format.h b/include/libcamera/internal/bayer_format.h > > > index 78ba3969913d..164743f7e9f6 100644 > > > --- a/include/libcamera/internal/bayer_format.h > > > +++ b/include/libcamera/internal/bayer_format.h > > > @@ -34,6 +34,8 @@ public: > > > None = 0, > > > CSI2 = 1, > > > IPU3 = 2, > > > + PISP1 = 3, > > > + PISP2 = 4, > > > }; > > > > > > constexpr BayerFormat() > > > diff --git a/include/linux/drm_fourcc.h b/include/linux/drm_fourcc.h > > > index 4ee421b95730..eff27fbe5a1e 100644 > > > --- a/include/linux/drm_fourcc.h > > > +++ b/include/linux/drm_fourcc.h > > > @@ -490,6 +490,7 @@ extern "C" { > > > #define DRM_FORMAT_MOD_VENDOR_ALLWINNER 0x09 > > > #define DRM_FORMAT_MOD_VENDOR_AMLOGIC 0x0a > > > #define DRM_FORMAT_MOD_VENDOR_MIPI 0x0b > > > +#define DRM_FORMAT_MOD_VENDOR_RPI 0x0c > > > > > > /* add more to the end as needed */ > > > > > > @@ -1670,6 +1671,9 @@ drm_fourcc_canonicalize_nvidia_format_mod(__u64 modifier) > > > */ > > > #define MIPI_FORMAT_MOD_CSI2_PACKED fourcc_mod_code(MIPI, 1) > > > > > > +#define PISP_FORMAT_MOD_COMPRESS_MODE1 fourcc_mod_code(RPI, 1) > > > +#define PISP_FORMAT_MOD_COMPRESS_MODE2 fourcc_mod_code(RPI, 2) > > > + > > > > I'll send a patch to DRM/KMS for these formats as well > > Can these be upstreamed if no DRM format is actually using them ? Or > should we add a DRM FourCC for the compressed formats, even if they > won't be used outside of the PiSP FE/BE loop ? I think an RFC on the dri-devel mailing list would be useful. > > > #if defined(__cplusplus) > > > } > > > #endif > > > diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h > > > index ba48d2c89726..59af6f794680 100644 > > > --- a/include/linux/videodev2.h > > > +++ b/include/linux/videodev2.h > > > @@ -789,6 +789,18 @@ struct v4l2_pix_format { > > > #define V4L2_PIX_FMT_IPU3_SGRBG10 v4l2_fourcc('i', 'p', '3', 'G') /* IPU3 packed 10-bit GRBG bayer */ > > > #define V4L2_PIX_FMT_IPU3_SRGGB10 v4l2_fourcc('i', 'p', '3', 'r') /* IPU3 packed 10-bit RGGB bayer */ > > > > > > +/* The pixel format for all our buffers (the precise format is found in the config buffer). */ > > > +#define V4L2_PIX_FMT_PISP_COMP1_RGGB v4l2_fourcc('P', 'C', '1', 'R') > > > +#define V4L2_PIX_FMT_PISP_COMP1_GRBG v4l2_fourcc('P', 'C', '1', 'G') > > > +#define V4L2_PIX_FMT_PISP_COMP1_GBRG v4l2_fourcc('P', 'C', '1', 'g') > > > +#define V4L2_PIX_FMT_PISP_COMP1_BGGR v4l2_fourcc('P', 'C', '1', 'B') > > > +#define V4L2_PIX_FMT_PISP_COMP1_MONO v4l2_fourcc('P', 'C', '1', 'M') > > > +#define V4L2_PIX_FMT_PISP_COMP2_RGGB v4l2_fourcc('P', 'C', '2', 'R') > > > +#define V4L2_PIX_FMT_PISP_COMP2_GRBG v4l2_fourcc('P', 'C', '2', 'G') > > > +#define V4L2_PIX_FMT_PISP_COMP2_GBRG v4l2_fourcc('P', 'C', '2', 'g') > > > +#define V4L2_PIX_FMT_PISP_COMP2_BGGR v4l2_fourcc('P', 'C', '2', 'B') > > > +#define V4L2_PIX_FMT_PISP_COMP2_MONO v4l2_fourcc('P', 'C', '2', 'M') > > > + > > > > I would use here what has been sent upstream for the BE support > > > > /* Raspberry Pi PiSP compressed formats. */ > > #define V4L2_PIX_FMT_PISP_COMP1_RGGB v4l2_fourcc('P', 'C', '1', 'R') /* PiSP 8-bit mode 1 compressed RGGB bayer */ > > #define V4L2_PIX_FMT_PISP_COMP1_GRBG v4l2_fourcc('P', 'C', '1', 'G') /* PiSP 8-bit mode 1 compressed GRBG bayer */ > > #define V4L2_PIX_FMT_PISP_COMP1_GBRG v4l2_fourcc('P', 'C', '1', 'g') /* PiSP 8-bit mode 1 compressed GBRG bayer */ > > #define V4L2_PIX_FMT_PISP_COMP1_BGGR v4l2_fourcc('P', 'C', '1', 'B') /* PiSP 8-bit mode 1 compressed BGGR bayer */ > > #define V4L2_PIX_FMT_PISP_COMP1_MONO v4l2_fourcc('P', 'C', '1', 'M') /* PiSP 8-bit mode 1 compressed monochrome */ > > #define V4L2_PIX_FMT_PISP_COMP2_RGGB v4l2_fourcc('P', 'C', '2', 'R') /* PiSP 8-bit mode 2 compressed RGGB bayer */ > > #define V4L2_PIX_FMT_PISP_COMP2_GRBG v4l2_fourcc('P', 'C', '2', 'G') /* PiSP 8-bit mode 2 compressed GRBG bayer */ > > #define V4L2_PIX_FMT_PISP_COMP2_GBRG v4l2_fourcc('P', 'C', '2', 'g') /* PiSP 8-bit mode 2 compressed GBRG bayer */ > > #define V4L2_PIX_FMT_PISP_COMP2_BGGR v4l2_fourcc('P', 'C', '2', 'B') /* PiSP 8-bit mode 2 compressed BGGR bayer */ > > #define V4L2_PIX_FMT_PISP_COMP2_MONO v4l2_fourcc('P', 'C', '2', 'M') /* PiSP 8-bit mode 2 compressed monochrome */ > > > > To make it easier to update it when these will land in upstream > > > > > /* SDR formats - used only for Software Defined Radio devices */ > > > #define V4L2_SDR_FMT_CU8 v4l2_fourcc('C', 'U', '0', '8') /* IQ u8 */ > > > #define V4L2_SDR_FMT_CU16LE v4l2_fourcc('C', 'U', '1', '6') /* IQ u16le */ > > > @@ -818,6 +830,9 @@ struct v4l2_pix_format { > > > #define V4L2_META_FMT_RK_ISP1_PARAMS v4l2_fourcc('R', 'K', '1', 'P') /* Rockchip ISP1 3A Parameters */ > > > #define V4L2_META_FMT_RK_ISP1_STAT_3A v4l2_fourcc('R', 'K', '1', 'S') /* Rockchip ISP1 3A Statistics */ > > > > > > +/* Vendor specific - used for RaspberryPi PiSP */ > > > +#define V4L2_META_FMT_RPI_BE_CFG v4l2_fourcc('R', 'P', 'B', 'C') /* PiSP BE configuration */ > > > /* priv field value to indicates that subsequent fields are valid. */ > > > #define V4L2_PIX_FMT_PRIV_MAGIC 0xfeedcafe > > > > > > diff --git a/src/libcamera/bayer_format.cpp b/src/libcamera/bayer_format.cpp > > > index 20aedfa6d925..333b1117f531 100644 > > > --- a/src/libcamera/bayer_format.cpp > > > +++ b/src/libcamera/bayer_format.cpp > > > @@ -164,6 +164,14 @@ const std::map<BayerFormat, Formats, BayerFormatComparator> bayerToFormat{ > > > { formats::SGRBG16, V4L2PixelFormat(V4L2_PIX_FMT_SGRBG16) } }, > > > { { BayerFormat::RGGB, 16, BayerFormat::Packing::None }, > > > { formats::SRGGB16, V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16) } }, > > > + { { BayerFormat::BGGR, 16, BayerFormat::Packing::PISP1 }, > > > + { formats::BGGR16_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_BGGR) } }, > > > + { { BayerFormat::GBRG, 16, BayerFormat::Packing::PISP1 }, > > > + { formats::GBRG16_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GBRG) } }, > > > + { { BayerFormat::GRBG, 16, BayerFormat::Packing::PISP1 }, > > > + { formats::GRBG16_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GRBG) } }, > > > + { { BayerFormat::RGGB, 16, BayerFormat::Packing::PISP1 }, > > > + { formats::RGGB16_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_RGGB) } }, > > > { { BayerFormat::MONO, 8, BayerFormat::Packing::None }, > > > { formats::R8, V4L2PixelFormat(V4L2_PIX_FMT_GREY) } }, > > > { { BayerFormat::MONO, 10, BayerFormat::Packing::None }, > > > @@ -174,6 +182,8 @@ const std::map<BayerFormat, Formats, BayerFormatComparator> bayerToFormat{ > > > { formats::R12, V4L2PixelFormat(V4L2_PIX_FMT_Y12) } }, > > > { { BayerFormat::MONO, 16, BayerFormat::Packing::None }, > > > { formats::R16, V4L2PixelFormat(V4L2_PIX_FMT_Y16) } }, > > > + { { BayerFormat::MONO, 16, BayerFormat::Packing::PISP1 }, > > > + { formats::MONO_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_MONO) } }, > > > > Should the COMP2 versions be added as well ? > > > > > }; > > > > > > const std::unordered_map<unsigned int, BayerFormat> mbusCodeToBayer{ > > > @@ -209,6 +219,10 @@ const std::unordered_map<unsigned int, BayerFormat> mbusCodeToBayer{ > > > { MEDIA_BUS_FMT_SGBRG16_1X16, { BayerFormat::GBRG, 16, BayerFormat::Packing::None } }, > > > { MEDIA_BUS_FMT_SGRBG16_1X16, { BayerFormat::GRBG, 16, BayerFormat::Packing::None } }, > > > { MEDIA_BUS_FMT_SRGGB16_1X16, { BayerFormat::RGGB, 16, BayerFormat::Packing::None } }, > > > + { MEDIA_BUS_FMT_SBGGR16_1X16, { BayerFormat::BGGR, 16, BayerFormat::Packing::PISP1 } }, > > > + { MEDIA_BUS_FMT_SGBRG16_1X16, { BayerFormat::GBRG, 16, BayerFormat::Packing::PISP1 } }, > > > + { MEDIA_BUS_FMT_SGRBG16_1X16, { BayerFormat::GRBG, 16, BayerFormat::Packing::PISP1 } }, > > > + { MEDIA_BUS_FMT_SRGGB16_1X16, { BayerFormat::RGGB, 16, BayerFormat::Packing::PISP1 } }, > > > > mbusCodeToBayer is an unordered_map which > > "is an associative container that contains key-value pairs with unique > > keys." > > > > I'm afraid adding a new entry with the same key overwrite the existing > > one ? > > > > We already had issues with this map, as we had similar problems with the > > _LE and _BE version of RGB565 which resolv to the same BayerFormat > > > > { MEDIA_BUS_FMT_SBGGR10_2X8_PADHI_BE, { BayerFormat::BGGR, 10, BayerFormat::Packing::None } }, > > { MEDIA_BUS_FMT_SBGGR10_2X8_PADHI_LE, { BayerFormat::BGGR, 10, BayerFormat::Packing::None } }, > > { MEDIA_BUS_FMT_SBGGR10_2X8_PADLO_BE, { BayerFormat::BGGR, 10, BayerFormat::Packing::None } }, > > { MEDIA_BUS_FMT_SBGGR10_2X8_PADLO_LE, { BayerFormat::BGGR, 10, BayerFormat::Packing::None } }, > > > > In general, we know that resolving mbus_codes to a unique format is > > ill-defined. How are you using these entries in the pi5 support ? > > > > > { MEDIA_BUS_FMT_Y8_1X8, { BayerFormat::MONO, 8, BayerFormat::Packing::None } }, > > > { MEDIA_BUS_FMT_Y10_1X10, { BayerFormat::MONO, 10, BayerFormat::Packing::None } }, > > > { MEDIA_BUS_FMT_Y12_1X12, { BayerFormat::MONO, 12, BayerFormat::Packing::None } }, > > > @@ -303,6 +317,10 @@ std::ostream &operator<<(std::ostream &out, const BayerFormat &f) > > > out << "-CSI2P"; > > > else if (f.packing == BayerFormat::Packing::IPU3) > > > out << "-IPU3P"; > > > + else if (f.packing == BayerFormat::Packing::PISP1) > > > + out << "-PISP1"; > > > + else if (f.packing == BayerFormat::Packing::PISP2) > > > + out << "-PISP2"; > > > > > > return out; > > > } > > > diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp > > > index a674f4179cc8..e603a7eda579 100644 > > > --- a/src/libcamera/formats.cpp > > > +++ b/src/libcamera/formats.cpp > > > @@ -547,6 +547,16 @@ const std::map<PixelFormat, PixelFormatInfo> pixelFormatInfo{ > > > .pixelsPerGroup = 1, > > > .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, > > > } }, > > > + { formats::MONO_PISP_COMP1, { > > > + .name = "MONO_PISP_COMP1", > > > + .format = formats::MONO_PISP_COMP1, > > > + .v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_MONO), }, > > > + .bitsPerPixel = 16, > > > + .colourEncoding = PixelFormatInfo::ColourEncodingYUV, > > > + .packed = true, > > > + .pixelsPerGroup = 1, > > > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, > > > + } }, > > > > > > /* Bayer formats. */ > > > { formats::SBGGR8, { > > > @@ -910,7 +920,46 @@ const std::map<PixelFormat, PixelFormatInfo> pixelFormatInfo{ > > > .pixelsPerGroup = 25, > > > .planes = {{ { 32, 1 }, { 0, 0 }, { 0, 0 } }}, > > > } }, > > > - > > > + { formats::BGGR16_PISP_COMP1, { > > > + .name = "BGGR16_PISP_COMP1", > > > + .format = formats::BGGR16_PISP_COMP1, > > > + .v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_BGGR), }, > > > + .bitsPerPixel = 16, > > > + .colourEncoding = PixelFormatInfo::ColourEncodingRAW, > > > + .packed = true, > > > + .pixelsPerGroup = 2, > > > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, > > > + } }, > > > > For regular 16-bit formats this should be > > > > .pixelsPerGroup = 2, > > .planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }}, > > > > As 2 pixels of 16 bits each consume 4 bytes. Is this {2, 1} > > intentional and due to compression ? > > > > > + { formats::GBRG16_PISP_COMP1, { > > > + .name = "GBRG16_PISP_COMP1", > > > + .format = formats::GBRG16_PISP_COMP1, > > > + .v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GBRG), }, > > > + .bitsPerPixel = 16, > > > + .colourEncoding = PixelFormatInfo::ColourEncodingRAW, > > > + .packed = true, > > > + .pixelsPerGroup = 2, > > > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, > > > + } }, > > > + { formats::GRBG16_PISP_COMP1, { > > > + .name = "GRBG16_PISP_COMP1", > > > + .format = formats::GRBG16_PISP_COMP1, > > > + .v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GRBG), }, > > > + .bitsPerPixel = 16, > > > + .colourEncoding = PixelFormatInfo::ColourEncodingRAW, > > > + .packed = true, > > > + .pixelsPerGroup = 2, > > > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, > > > + } }, > > > + { formats::RGGB16_PISP_COMP1, { > > > + .name = "RGGB16_PISP_COMP1", > > > + .format = formats::RGGB16_PISP_COMP1, > > > + .v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_RGGB), }, > > > + .bitsPerPixel = 16, > > > + .colourEncoding = PixelFormatInfo::ColourEncodingRAW, > > > + .packed = true, > > > + .pixelsPerGroup = 2, > > > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, > > > + } }, > > > /* Compressed formats. */ > > > { formats::MJPEG, { > > > .name = "MJPEG", > > > diff --git a/src/libcamera/formats.yaml b/src/libcamera/formats.yaml > > > index bde2cc803b98..f6df721243d0 100644 > > > --- a/src/libcamera/formats.yaml > > > +++ b/src/libcamera/formats.yaml > > > @@ -190,4 +190,20 @@ formats: > > > - SBGGR10_IPU3: > > > fourcc: DRM_FORMAT_SBGGR10 > > > mod: IPU3_FORMAT_MOD_PACKED > > > + > > > + - RGGB16_PISP_COMP1: > > > + fourcc: DRM_FORMAT_SRGGB16 > > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1 > > > + - GRBG16_PISP_COMP1: > > > + fourcc: DRM_FORMAT_SGRBG16 > > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1 > > > + - GBRG16_PISP_COMP1: > > > + fourcc: DRM_FORMAT_SGBRG16 > > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1 > > > + - BGGR16_PISP_COMP1: > > > + fourcc: DRM_FORMAT_SBGGR16 > > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1 > > > + - MONO_PISP_COMP1: > > > + fourcc: DRM_FORMAT_R16 > > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1 > > > ... > > > diff --git a/src/libcamera/v4l2_pixelformat.cpp b/src/libcamera/v4l2_pixelformat.cpp > > > index efb6f2940235..47baaf60199d 100644 > > > --- a/src/libcamera/v4l2_pixelformat.cpp > > > +++ b/src/libcamera/v4l2_pixelformat.cpp > > > @@ -207,6 +207,16 @@ const std::map<V4L2PixelFormat, V4L2PixelFormat::Info> vpf2pf{ > > > { formats::SGRBG16, "16-bit Bayer GRGR/BGBG" } }, > > > { V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16), > > > { formats::SRGGB16, "16-bit Bayer RGRG/GBGB" } }, > > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_BGGR), > > > + { formats::BGGR16_PISP_COMP1, "16-bit Bayer BGBG/GRGR PiSP Compress Mode 1" } }, > > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GBRG), > > > + { formats::GBRG16_PISP_COMP1, "16-bit Bayer GBGB/RGRG PiSP Compress Mode 1" } }, > > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GRBG), > > > + { formats::GRBG16_PISP_COMP1, "16-bit Bayer GRGR/BGBG PiSP Compress Mode 1" } }, > > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_RGGB), > > > + { formats::RGGB16_PISP_COMP1, "16-bit Bayer RGRG/GBGB PiSP Compress Mode 1" } }, > > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_MONO), > > > + { formats::MONO_PISP_COMP1, "16-bit Mono PiSP Compress Mode 1" } }, > > > > > > /* Compressed formats. */ > > > { V4L2PixelFormat(V4L2_PIX_FMT_MJPEG), > > > diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp > > > index 6d0785b7b484..aea90abaf9ef 100644 > > > --- a/src/libcamera/v4l2_subdevice.cpp > > > +++ b/src/libcamera/v4l2_subdevice.cpp > > > @@ -134,6 +134,10 @@ const std::map<uint32_t, V4L2SubdeviceFormatInfo> formatInfoMap = { > > > { MEDIA_BUS_FMT_SGBRG12_1X12, { 12, "SGBRG12_1X12", PixelFormatInfo::ColourEncodingRAW } }, > > > { MEDIA_BUS_FMT_SGRBG12_1X12, { 12, "SGRBG12_1X12", PixelFormatInfo::ColourEncodingRAW } }, > > > { MEDIA_BUS_FMT_SRGGB12_1X12, { 12, "SRGGB12_1X12", PixelFormatInfo::ColourEncodingRAW } }, > > > + { MEDIA_BUS_FMT_SBGGR16_1X16, { 16, "SBGGR16_1x16", PixelFormatInfo::ColourEncodingRAW } }, > > > + { MEDIA_BUS_FMT_SGBRG16_1X16, { 16, "SGBRG16_1x16", PixelFormatInfo::ColourEncodingRAW } }, > > > + { MEDIA_BUS_FMT_SGRBG16_1X16, { 16, "SGRBG16_1x16", PixelFormatInfo::ColourEncodingRAW } }, > > > + { MEDIA_BUS_FMT_SRGGB16_1X16, { 16, "SRGGB16_1x16", PixelFormatInfo::ColourEncodingRAW } }, > > > /* \todo Clarify colour encoding for HSV formats */ > > > { MEDIA_BUS_FMT_AHSV8888_1X32, { 32, "AHSV8888_1X32", PixelFormatInfo::ColourEncodingRGB } }, > > > { MEDIA_BUS_FMT_JPEG_1X8, { 8, "JPEG_1X8", PixelFormatInfo::ColourEncodingYUV } },
Hi Jacopo, Thanks for the feedback, and sending the updates to the DRM mailing list! On Mon, 26 Feb 2024 at 13:49, Jacopo Mondi <jacopo.mondi@ideasonboard.com> wrote: > > Hi Naush > > On Thu, Feb 15, 2024 at 01:27:10PM +0000, Naushir Patuck wrote: > > Add the Raspberry Pi 5 PiSP specific compressed Bayer format types 1/2: > > - V4L2_PIX_FMT_PISP_COMP1_xxx > > - V4L2_PIX_FMT_PISP_COMP2_xxx > > > > Add the Raspberry Pi 5 PiSP Backend config format: > > - V4L2_META_FMT_RPI_BE_CFG > > > > Additionally, we also extend libcamera format handlers to support 16-bit > > Bayer formats across the media bus. > > > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > > --- > > include/libcamera/internal/bayer_format.h | 2 + > > include/linux/drm_fourcc.h | 4 ++ > > include/linux/videodev2.h | 15 +++++++ > > src/libcamera/bayer_format.cpp | 18 ++++++++ > > src/libcamera/formats.cpp | 51 ++++++++++++++++++++++- > > src/libcamera/formats.yaml | 16 +++++++ > > src/libcamera/v4l2_pixelformat.cpp | 10 +++++ > > src/libcamera/v4l2_subdevice.cpp | 4 ++ > > 8 files changed, 119 insertions(+), 1 deletion(-) > > > > diff --git a/include/libcamera/internal/bayer_format.h b/include/libcamera/internal/bayer_format.h > > index 78ba3969913d..164743f7e9f6 100644 > > --- a/include/libcamera/internal/bayer_format.h > > +++ b/include/libcamera/internal/bayer_format.h > > @@ -34,6 +34,8 @@ public: > > None = 0, > > CSI2 = 1, > > IPU3 = 2, > > + PISP1 = 3, > > + PISP2 = 4, > > }; > > > > constexpr BayerFormat() > > diff --git a/include/linux/drm_fourcc.h b/include/linux/drm_fourcc.h > > index 4ee421b95730..eff27fbe5a1e 100644 > > --- a/include/linux/drm_fourcc.h > > +++ b/include/linux/drm_fourcc.h > > @@ -490,6 +490,7 @@ extern "C" { > > #define DRM_FORMAT_MOD_VENDOR_ALLWINNER 0x09 > > #define DRM_FORMAT_MOD_VENDOR_AMLOGIC 0x0a > > #define DRM_FORMAT_MOD_VENDOR_MIPI 0x0b > > +#define DRM_FORMAT_MOD_VENDOR_RPI 0x0c > > > > /* add more to the end as needed */ > > > > @@ -1670,6 +1671,9 @@ drm_fourcc_canonicalize_nvidia_format_mod(__u64 modifier) > > */ > > #define MIPI_FORMAT_MOD_CSI2_PACKED fourcc_mod_code(MIPI, 1) > > > > +#define PISP_FORMAT_MOD_COMPRESS_MODE1 fourcc_mod_code(RPI, 1) > > +#define PISP_FORMAT_MOD_COMPRESS_MODE2 fourcc_mod_code(RPI, 2) > > + > > I'll send a patch to DRM/KMS for these formats as well > > > #if defined(__cplusplus) > > } > > #endif > > diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h > > index ba48d2c89726..59af6f794680 100644 > > --- a/include/linux/videodev2.h > > +++ b/include/linux/videodev2.h > > @@ -789,6 +789,18 @@ struct v4l2_pix_format { > > #define V4L2_PIX_FMT_IPU3_SGRBG10 v4l2_fourcc('i', 'p', '3', 'G') /* IPU3 packed 10-bit GRBG bayer */ > > #define V4L2_PIX_FMT_IPU3_SRGGB10 v4l2_fourcc('i', 'p', '3', 'r') /* IPU3 packed 10-bit RGGB bayer */ > > > > +/* The pixel format for all our buffers (the precise format is found in the config buffer). */ > > +#define V4L2_PIX_FMT_PISP_COMP1_RGGB v4l2_fourcc('P', 'C', '1', 'R') > > +#define V4L2_PIX_FMT_PISP_COMP1_GRBG v4l2_fourcc('P', 'C', '1', 'G') > > +#define V4L2_PIX_FMT_PISP_COMP1_GBRG v4l2_fourcc('P', 'C', '1', 'g') > > +#define V4L2_PIX_FMT_PISP_COMP1_BGGR v4l2_fourcc('P', 'C', '1', 'B') > > +#define V4L2_PIX_FMT_PISP_COMP1_MONO v4l2_fourcc('P', 'C', '1', 'M') > > +#define V4L2_PIX_FMT_PISP_COMP2_RGGB v4l2_fourcc('P', 'C', '2', 'R') > > +#define V4L2_PIX_FMT_PISP_COMP2_GRBG v4l2_fourcc('P', 'C', '2', 'G') > > +#define V4L2_PIX_FMT_PISP_COMP2_GBRG v4l2_fourcc('P', 'C', '2', 'g') > > +#define V4L2_PIX_FMT_PISP_COMP2_BGGR v4l2_fourcc('P', 'C', '2', 'B') > > +#define V4L2_PIX_FMT_PISP_COMP2_MONO v4l2_fourcc('P', 'C', '2', 'M') > > + > > I would use here what has been sent upstream for the BE support > > /* Raspberry Pi PiSP compressed formats. */ > #define V4L2_PIX_FMT_PISP_COMP1_RGGB v4l2_fourcc('P', 'C', '1', 'R') /* PiSP 8-bit mode 1 compressed RGGB bayer */ > #define V4L2_PIX_FMT_PISP_COMP1_GRBG v4l2_fourcc('P', 'C', '1', 'G') /* PiSP 8-bit mode 1 compressed GRBG bayer */ > #define V4L2_PIX_FMT_PISP_COMP1_GBRG v4l2_fourcc('P', 'C', '1', 'g') /* PiSP 8-bit mode 1 compressed GBRG bayer */ > #define V4L2_PIX_FMT_PISP_COMP1_BGGR v4l2_fourcc('P', 'C', '1', 'B') /* PiSP 8-bit mode 1 compressed BGGR bayer */ > #define V4L2_PIX_FMT_PISP_COMP1_MONO v4l2_fourcc('P', 'C', '1', 'M') /* PiSP 8-bit mode 1 compressed monochrome */ > #define V4L2_PIX_FMT_PISP_COMP2_RGGB v4l2_fourcc('P', 'C', '2', 'R') /* PiSP 8-bit mode 2 compressed RGGB bayer */ > #define V4L2_PIX_FMT_PISP_COMP2_GRBG v4l2_fourcc('P', 'C', '2', 'G') /* PiSP 8-bit mode 2 compressed GRBG bayer */ > #define V4L2_PIX_FMT_PISP_COMP2_GBRG v4l2_fourcc('P', 'C', '2', 'g') /* PiSP 8-bit mode 2 compressed GBRG bayer */ > #define V4L2_PIX_FMT_PISP_COMP2_BGGR v4l2_fourcc('P', 'C', '2', 'B') /* PiSP 8-bit mode 2 compressed BGGR bayer */ > #define V4L2_PIX_FMT_PISP_COMP2_MONO v4l2_fourcc('P', 'C', '2', 'M') /* PiSP 8-bit mode 2 compressed monochrome */ > > To make it easier to update it when these will land in upstream Ack. > > > /* SDR formats - used only for Software Defined Radio devices */ > > #define V4L2_SDR_FMT_CU8 v4l2_fourcc('C', 'U', '0', '8') /* IQ u8 */ > > #define V4L2_SDR_FMT_CU16LE v4l2_fourcc('C', 'U', '1', '6') /* IQ u16le */ > > @@ -818,6 +830,9 @@ struct v4l2_pix_format { > > #define V4L2_META_FMT_RK_ISP1_PARAMS v4l2_fourcc('R', 'K', '1', 'P') /* Rockchip ISP1 3A Parameters */ > > #define V4L2_META_FMT_RK_ISP1_STAT_3A v4l2_fourcc('R', 'K', '1', 'S') /* Rockchip ISP1 3A Statistics */ > > > > +/* Vendor specific - used for RaspberryPi PiSP */ > > +#define V4L2_META_FMT_RPI_BE_CFG v4l2_fourcc('R', 'P', 'B', 'C') /* PiSP BE configuration */ > > /* priv field value to indicates that subsequent fields are valid. */ > > #define V4L2_PIX_FMT_PRIV_MAGIC 0xfeedcafe > > > > diff --git a/src/libcamera/bayer_format.cpp b/src/libcamera/bayer_format.cpp > > index 20aedfa6d925..333b1117f531 100644 > > --- a/src/libcamera/bayer_format.cpp > > +++ b/src/libcamera/bayer_format.cpp > > @@ -164,6 +164,14 @@ const std::map<BayerFormat, Formats, BayerFormatComparator> bayerToFormat{ > > { formats::SGRBG16, V4L2PixelFormat(V4L2_PIX_FMT_SGRBG16) } }, > > { { BayerFormat::RGGB, 16, BayerFormat::Packing::None }, > > { formats::SRGGB16, V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16) } }, > > + { { BayerFormat::BGGR, 16, BayerFormat::Packing::PISP1 }, > > + { formats::BGGR16_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_BGGR) } }, > > + { { BayerFormat::GBRG, 16, BayerFormat::Packing::PISP1 }, > > + { formats::GBRG16_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GBRG) } }, > > + { { BayerFormat::GRBG, 16, BayerFormat::Packing::PISP1 }, > > + { formats::GRBG16_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GRBG) } }, > > + { { BayerFormat::RGGB, 16, BayerFormat::Packing::PISP1 }, > > + { formats::RGGB16_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_RGGB) } }, > > { { BayerFormat::MONO, 8, BayerFormat::Packing::None }, > > { formats::R8, V4L2PixelFormat(V4L2_PIX_FMT_GREY) } }, > > { { BayerFormat::MONO, 10, BayerFormat::Packing::None }, > > @@ -174,6 +182,8 @@ const std::map<BayerFormat, Formats, BayerFormatComparator> bayerToFormat{ > > { formats::R12, V4L2PixelFormat(V4L2_PIX_FMT_Y12) } }, > > { { BayerFormat::MONO, 16, BayerFormat::Packing::None }, > > { formats::R16, V4L2PixelFormat(V4L2_PIX_FMT_Y16) } }, > > + { { BayerFormat::MONO, 16, BayerFormat::Packing::PISP1 }, > > + { formats::MONO_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_MONO) } }, > > Should the COMP2 versions be added as well ? To be honest, I doubt we will ever use COMP2, COMP1 is the better option almost always. But I'll add it for completeness. > > > }; > > > > const std::unordered_map<unsigned int, BayerFormat> mbusCodeToBayer{ > > @@ -209,6 +219,10 @@ const std::unordered_map<unsigned int, BayerFormat> mbusCodeToBayer{ > > { MEDIA_BUS_FMT_SGBRG16_1X16, { BayerFormat::GBRG, 16, BayerFormat::Packing::None } }, > > { MEDIA_BUS_FMT_SGRBG16_1X16, { BayerFormat::GRBG, 16, BayerFormat::Packing::None } }, > > { MEDIA_BUS_FMT_SRGGB16_1X16, { BayerFormat::RGGB, 16, BayerFormat::Packing::None } }, > > + { MEDIA_BUS_FMT_SBGGR16_1X16, { BayerFormat::BGGR, 16, BayerFormat::Packing::PISP1 } }, > > + { MEDIA_BUS_FMT_SGBRG16_1X16, { BayerFormat::GBRG, 16, BayerFormat::Packing::PISP1 } }, > > + { MEDIA_BUS_FMT_SGRBG16_1X16, { BayerFormat::GRBG, 16, BayerFormat::Packing::PISP1 } }, > > + { MEDIA_BUS_FMT_SRGGB16_1X16, { BayerFormat::RGGB, 16, BayerFormat::Packing::PISP1 } }, > > mbusCodeToBayer is an unordered_map which > "is an associative container that contains key-value pairs with unique > keys." > > I'm afraid adding a new entry with the same key overwrite the existing > one ? Oops you are right! I think this came about from when we originally had MBUS codes for the PISP comp formats, and those subsequently got removed. > > We already had issues with this map, as we had similar problems with the > _LE and _BE version of RGB565 which resolv to the same BayerFormat > > { MEDIA_BUS_FMT_SBGGR10_2X8_PADHI_BE, { BayerFormat::BGGR, 10, BayerFormat::Packing::None } }, > { MEDIA_BUS_FMT_SBGGR10_2X8_PADHI_LE, { BayerFormat::BGGR, 10, BayerFormat::Packing::None } }, > { MEDIA_BUS_FMT_SBGGR10_2X8_PADLO_BE, { BayerFormat::BGGR, 10, BayerFormat::Packing::None } }, > { MEDIA_BUS_FMT_SBGGR10_2X8_PADLO_LE, { BayerFormat::BGGR, 10, BayerFormat::Packing::None } }, > > In general, we know that resolving mbus_codes to a unique format is > ill-defined. How are you using these entries in the pi5 support ? As luck would have it, the Pi 5 pipeline handler never calls BayerFormat::fromMbusCode() with a PISP_COMP type format so the code never gets exercised and goes wrong! I think the right solution is to remove MEDIA_BUS_FMT_* for the PISP_COMP formats as they genuinely don't have MBUS codes associated with them. > > > { MEDIA_BUS_FMT_Y8_1X8, { BayerFormat::MONO, 8, BayerFormat::Packing::None } }, > > { MEDIA_BUS_FMT_Y10_1X10, { BayerFormat::MONO, 10, BayerFormat::Packing::None } }, > > { MEDIA_BUS_FMT_Y12_1X12, { BayerFormat::MONO, 12, BayerFormat::Packing::None } }, > > @@ -303,6 +317,10 @@ std::ostream &operator<<(std::ostream &out, const BayerFormat &f) > > out << "-CSI2P"; > > else if (f.packing == BayerFormat::Packing::IPU3) > > out << "-IPU3P"; > > + else if (f.packing == BayerFormat::Packing::PISP1) > > + out << "-PISP1"; > > + else if (f.packing == BayerFormat::Packing::PISP2) > > + out << "-PISP2"; > > > > return out; > > } > > diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp > > index a674f4179cc8..e603a7eda579 100644 > > --- a/src/libcamera/formats.cpp > > +++ b/src/libcamera/formats.cpp > > @@ -547,6 +547,16 @@ const std::map<PixelFormat, PixelFormatInfo> pixelFormatInfo{ > > .pixelsPerGroup = 1, > > .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, > > } }, > > + { formats::MONO_PISP_COMP1, { > > + .name = "MONO_PISP_COMP1", > > + .format = formats::MONO_PISP_COMP1, > > + .v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_MONO), }, > > + .bitsPerPixel = 16, > > + .colourEncoding = PixelFormatInfo::ColourEncodingYUV, > > + .packed = true, > > + .pixelsPerGroup = 1, > > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, > > + } }, > > > > /* Bayer formats. */ > > { formats::SBGGR8, { > > @@ -910,7 +920,46 @@ const std::map<PixelFormat, PixelFormatInfo> pixelFormatInfo{ > > .pixelsPerGroup = 25, > > .planes = {{ { 32, 1 }, { 0, 0 }, { 0, 0 } }}, > > } }, > > - > > + { formats::BGGR16_PISP_COMP1, { > > + .name = "BGGR16_PISP_COMP1", > > + .format = formats::BGGR16_PISP_COMP1, > > + .v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_BGGR), }, > > + .bitsPerPixel = 16, > > + .colourEncoding = PixelFormatInfo::ColourEncodingRAW, > > + .packed = true, > > + .pixelsPerGroup = 2, > > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, > > + } }, > > For regular 16-bit formats this should be > > .pixelsPerGroup = 2, > .planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }}, > > As 2 pixels of 16 bits each consume 4 bytes. Is this {2, 1} > intentional and due to compression ? Yes, I used {2, 1} to denote the 8-bit/sample after compression. Do you think I should change to {4, 1}? Regards, Naush > > > + { formats::GBRG16_PISP_COMP1, { > > + .name = "GBRG16_PISP_COMP1", > > + .format = formats::GBRG16_PISP_COMP1, > > + .v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GBRG), }, > > + .bitsPerPixel = 16, > > + .colourEncoding = PixelFormatInfo::ColourEncodingRAW, > > + .packed = true, > > + .pixelsPerGroup = 2, > > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, > > + } }, > > + { formats::GRBG16_PISP_COMP1, { > > + .name = "GRBG16_PISP_COMP1", > > + .format = formats::GRBG16_PISP_COMP1, > > + .v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GRBG), }, > > + .bitsPerPixel = 16, > > + .colourEncoding = PixelFormatInfo::ColourEncodingRAW, > > + .packed = true, > > + .pixelsPerGroup = 2, > > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, > > + } }, > > + { formats::RGGB16_PISP_COMP1, { > > + .name = "RGGB16_PISP_COMP1", > > + .format = formats::RGGB16_PISP_COMP1, > > + .v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_RGGB), }, > > + .bitsPerPixel = 16, > > + .colourEncoding = PixelFormatInfo::ColourEncodingRAW, > > + .packed = true, > > + .pixelsPerGroup = 2, > > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, > > + } }, > > /* Compressed formats. */ > > { formats::MJPEG, { > > .name = "MJPEG", > > diff --git a/src/libcamera/formats.yaml b/src/libcamera/formats.yaml > > index bde2cc803b98..f6df721243d0 100644 > > --- a/src/libcamera/formats.yaml > > +++ b/src/libcamera/formats.yaml > > @@ -190,4 +190,20 @@ formats: > > - SBGGR10_IPU3: > > fourcc: DRM_FORMAT_SBGGR10 > > mod: IPU3_FORMAT_MOD_PACKED > > + > > + - RGGB16_PISP_COMP1: > > + fourcc: DRM_FORMAT_SRGGB16 > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1 > > + - GRBG16_PISP_COMP1: > > + fourcc: DRM_FORMAT_SGRBG16 > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1 > > + - GBRG16_PISP_COMP1: > > + fourcc: DRM_FORMAT_SGBRG16 > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1 > > + - BGGR16_PISP_COMP1: > > + fourcc: DRM_FORMAT_SBGGR16 > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1 > > + - MONO_PISP_COMP1: > > + fourcc: DRM_FORMAT_R16 > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1 > > ... > > diff --git a/src/libcamera/v4l2_pixelformat.cpp b/src/libcamera/v4l2_pixelformat.cpp > > index efb6f2940235..47baaf60199d 100644 > > --- a/src/libcamera/v4l2_pixelformat.cpp > > +++ b/src/libcamera/v4l2_pixelformat.cpp > > @@ -207,6 +207,16 @@ const std::map<V4L2PixelFormat, V4L2PixelFormat::Info> vpf2pf{ > > { formats::SGRBG16, "16-bit Bayer GRGR/BGBG" } }, > > { V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16), > > { formats::SRGGB16, "16-bit Bayer RGRG/GBGB" } }, > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_BGGR), > > + { formats::BGGR16_PISP_COMP1, "16-bit Bayer BGBG/GRGR PiSP Compress Mode 1" } }, > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GBRG), > > + { formats::GBRG16_PISP_COMP1, "16-bit Bayer GBGB/RGRG PiSP Compress Mode 1" } }, > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GRBG), > > + { formats::GRBG16_PISP_COMP1, "16-bit Bayer GRGR/BGBG PiSP Compress Mode 1" } }, > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_RGGB), > > + { formats::RGGB16_PISP_COMP1, "16-bit Bayer RGRG/GBGB PiSP Compress Mode 1" } }, > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_MONO), > > + { formats::MONO_PISP_COMP1, "16-bit Mono PiSP Compress Mode 1" } }, > > > > /* Compressed formats. */ > > { V4L2PixelFormat(V4L2_PIX_FMT_MJPEG), > > diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp > > index 6d0785b7b484..aea90abaf9ef 100644 > > --- a/src/libcamera/v4l2_subdevice.cpp > > +++ b/src/libcamera/v4l2_subdevice.cpp > > @@ -134,6 +134,10 @@ const std::map<uint32_t, V4L2SubdeviceFormatInfo> formatInfoMap = { > > { MEDIA_BUS_FMT_SGBRG12_1X12, { 12, "SGBRG12_1X12", PixelFormatInfo::ColourEncodingRAW } }, > > { MEDIA_BUS_FMT_SGRBG12_1X12, { 12, "SGRBG12_1X12", PixelFormatInfo::ColourEncodingRAW } }, > > { MEDIA_BUS_FMT_SRGGB12_1X12, { 12, "SRGGB12_1X12", PixelFormatInfo::ColourEncodingRAW } }, > > + { MEDIA_BUS_FMT_SBGGR16_1X16, { 16, "SBGGR16_1x16", PixelFormatInfo::ColourEncodingRAW } }, > > + { MEDIA_BUS_FMT_SGBRG16_1X16, { 16, "SGBRG16_1x16", PixelFormatInfo::ColourEncodingRAW } }, > > + { MEDIA_BUS_FMT_SGRBG16_1X16, { 16, "SGRBG16_1x16", PixelFormatInfo::ColourEncodingRAW } }, > > + { MEDIA_BUS_FMT_SRGGB16_1X16, { 16, "SRGGB16_1x16", PixelFormatInfo::ColourEncodingRAW } }, > > /* \todo Clarify colour encoding for HSV formats */ > > { MEDIA_BUS_FMT_AHSV8888_1X32, { 32, "AHSV8888_1X32", PixelFormatInfo::ColourEncodingRGB } }, > > { MEDIA_BUS_FMT_JPEG_1X8, { 8, "JPEG_1X8", PixelFormatInfo::ColourEncodingYUV } }, > > -- > > 2.34.1 > >
Hi Naush On Tue, Feb 27, 2024 at 10:20:40AM +0000, Naushir Patuck wrote: > Hi Jacopo, > > Thanks for the feedback, and sending the updates to the DRM mailing list! > As usual, nothing's easy :) > On Mon, 26 Feb 2024 at 13:49, Jacopo Mondi > <jacopo.mondi@ideasonboard.com> wrote: > > > > Hi Naush > > > > On Thu, Feb 15, 2024 at 01:27:10PM +0000, Naushir Patuck wrote: > > > Add the Raspberry Pi 5 PiSP specific compressed Bayer format types 1/2: > > > - V4L2_PIX_FMT_PISP_COMP1_xxx > > > - V4L2_PIX_FMT_PISP_COMP2_xxx > > > > > > Add the Raspberry Pi 5 PiSP Backend config format: > > > - V4L2_META_FMT_RPI_BE_CFG > > > > > > Additionally, we also extend libcamera format handlers to support 16-bit > > > Bayer formats across the media bus. > > > > > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > > > --- > > > include/libcamera/internal/bayer_format.h | 2 + > > > include/linux/drm_fourcc.h | 4 ++ > > > include/linux/videodev2.h | 15 +++++++ > > > src/libcamera/bayer_format.cpp | 18 ++++++++ > > > src/libcamera/formats.cpp | 51 ++++++++++++++++++++++- > > > src/libcamera/formats.yaml | 16 +++++++ > > > src/libcamera/v4l2_pixelformat.cpp | 10 +++++ > > > src/libcamera/v4l2_subdevice.cpp | 4 ++ > > > 8 files changed, 119 insertions(+), 1 deletion(-) > > > > > > diff --git a/include/libcamera/internal/bayer_format.h b/include/libcamera/internal/bayer_format.h > > > index 78ba3969913d..164743f7e9f6 100644 > > > --- a/include/libcamera/internal/bayer_format.h > > > +++ b/include/libcamera/internal/bayer_format.h > > > @@ -34,6 +34,8 @@ public: > > > None = 0, > > > CSI2 = 1, > > > IPU3 = 2, > > > + PISP1 = 3, > > > + PISP2 = 4, > > > }; > > > > > > constexpr BayerFormat() > > > diff --git a/include/linux/drm_fourcc.h b/include/linux/drm_fourcc.h > > > index 4ee421b95730..eff27fbe5a1e 100644 > > > --- a/include/linux/drm_fourcc.h > > > +++ b/include/linux/drm_fourcc.h > > > @@ -490,6 +490,7 @@ extern "C" { > > > #define DRM_FORMAT_MOD_VENDOR_ALLWINNER 0x09 > > > #define DRM_FORMAT_MOD_VENDOR_AMLOGIC 0x0a > > > #define DRM_FORMAT_MOD_VENDOR_MIPI 0x0b > > > +#define DRM_FORMAT_MOD_VENDOR_RPI 0x0c > > > > > > /* add more to the end as needed */ > > > > > > @@ -1670,6 +1671,9 @@ drm_fourcc_canonicalize_nvidia_format_mod(__u64 modifier) > > > */ > > > #define MIPI_FORMAT_MOD_CSI2_PACKED fourcc_mod_code(MIPI, 1) > > > > > > +#define PISP_FORMAT_MOD_COMPRESS_MODE1 fourcc_mod_code(RPI, 1) > > > +#define PISP_FORMAT_MOD_COMPRESS_MODE2 fourcc_mod_code(RPI, 2) > > > + > > > > I'll send a patch to DRM/KMS for these formats as well > > > > > #if defined(__cplusplus) > > > } > > > #endif > > > diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h > > > index ba48d2c89726..59af6f794680 100644 > > > --- a/include/linux/videodev2.h > > > +++ b/include/linux/videodev2.h > > > @@ -789,6 +789,18 @@ struct v4l2_pix_format { > > > #define V4L2_PIX_FMT_IPU3_SGRBG10 v4l2_fourcc('i', 'p', '3', 'G') /* IPU3 packed 10-bit GRBG bayer */ > > > #define V4L2_PIX_FMT_IPU3_SRGGB10 v4l2_fourcc('i', 'p', '3', 'r') /* IPU3 packed 10-bit RGGB bayer */ > > > > > > +/* The pixel format for all our buffers (the precise format is found in the config buffer). */ > > > +#define V4L2_PIX_FMT_PISP_COMP1_RGGB v4l2_fourcc('P', 'C', '1', 'R') > > > +#define V4L2_PIX_FMT_PISP_COMP1_GRBG v4l2_fourcc('P', 'C', '1', 'G') > > > +#define V4L2_PIX_FMT_PISP_COMP1_GBRG v4l2_fourcc('P', 'C', '1', 'g') > > > +#define V4L2_PIX_FMT_PISP_COMP1_BGGR v4l2_fourcc('P', 'C', '1', 'B') > > > +#define V4L2_PIX_FMT_PISP_COMP1_MONO v4l2_fourcc('P', 'C', '1', 'M') > > > +#define V4L2_PIX_FMT_PISP_COMP2_RGGB v4l2_fourcc('P', 'C', '2', 'R') > > > +#define V4L2_PIX_FMT_PISP_COMP2_GRBG v4l2_fourcc('P', 'C', '2', 'G') > > > +#define V4L2_PIX_FMT_PISP_COMP2_GBRG v4l2_fourcc('P', 'C', '2', 'g') > > > +#define V4L2_PIX_FMT_PISP_COMP2_BGGR v4l2_fourcc('P', 'C', '2', 'B') > > > +#define V4L2_PIX_FMT_PISP_COMP2_MONO v4l2_fourcc('P', 'C', '2', 'M') > > > + > > > > I would use here what has been sent upstream for the BE support > > > > /* Raspberry Pi PiSP compressed formats. */ > > #define V4L2_PIX_FMT_PISP_COMP1_RGGB v4l2_fourcc('P', 'C', '1', 'R') /* PiSP 8-bit mode 1 compressed RGGB bayer */ > > #define V4L2_PIX_FMT_PISP_COMP1_GRBG v4l2_fourcc('P', 'C', '1', 'G') /* PiSP 8-bit mode 1 compressed GRBG bayer */ > > #define V4L2_PIX_FMT_PISP_COMP1_GBRG v4l2_fourcc('P', 'C', '1', 'g') /* PiSP 8-bit mode 1 compressed GBRG bayer */ > > #define V4L2_PIX_FMT_PISP_COMP1_BGGR v4l2_fourcc('P', 'C', '1', 'B') /* PiSP 8-bit mode 1 compressed BGGR bayer */ > > #define V4L2_PIX_FMT_PISP_COMP1_MONO v4l2_fourcc('P', 'C', '1', 'M') /* PiSP 8-bit mode 1 compressed monochrome */ > > #define V4L2_PIX_FMT_PISP_COMP2_RGGB v4l2_fourcc('P', 'C', '2', 'R') /* PiSP 8-bit mode 2 compressed RGGB bayer */ > > #define V4L2_PIX_FMT_PISP_COMP2_GRBG v4l2_fourcc('P', 'C', '2', 'G') /* PiSP 8-bit mode 2 compressed GRBG bayer */ > > #define V4L2_PIX_FMT_PISP_COMP2_GBRG v4l2_fourcc('P', 'C', '2', 'g') /* PiSP 8-bit mode 2 compressed GBRG bayer */ > > #define V4L2_PIX_FMT_PISP_COMP2_BGGR v4l2_fourcc('P', 'C', '2', 'B') /* PiSP 8-bit mode 2 compressed BGGR bayer */ > > #define V4L2_PIX_FMT_PISP_COMP2_MONO v4l2_fourcc('P', 'C', '2', 'M') /* PiSP 8-bit mode 2 compressed monochrome */ > > > > To make it easier to update it when these will land in upstream > > Ack. > > > > > > /* SDR formats - used only for Software Defined Radio devices */ > > > #define V4L2_SDR_FMT_CU8 v4l2_fourcc('C', 'U', '0', '8') /* IQ u8 */ > > > #define V4L2_SDR_FMT_CU16LE v4l2_fourcc('C', 'U', '1', '6') /* IQ u16le */ > > > @@ -818,6 +830,9 @@ struct v4l2_pix_format { > > > #define V4L2_META_FMT_RK_ISP1_PARAMS v4l2_fourcc('R', 'K', '1', 'P') /* Rockchip ISP1 3A Parameters */ > > > #define V4L2_META_FMT_RK_ISP1_STAT_3A v4l2_fourcc('R', 'K', '1', 'S') /* Rockchip ISP1 3A Statistics */ > > > > > > +/* Vendor specific - used for RaspberryPi PiSP */ > > > +#define V4L2_META_FMT_RPI_BE_CFG v4l2_fourcc('R', 'P', 'B', 'C') /* PiSP BE configuration */ > > > /* priv field value to indicates that subsequent fields are valid. */ > > > #define V4L2_PIX_FMT_PRIV_MAGIC 0xfeedcafe > > > > > > diff --git a/src/libcamera/bayer_format.cpp b/src/libcamera/bayer_format.cpp > > > index 20aedfa6d925..333b1117f531 100644 > > > --- a/src/libcamera/bayer_format.cpp > > > +++ b/src/libcamera/bayer_format.cpp > > > @@ -164,6 +164,14 @@ const std::map<BayerFormat, Formats, BayerFormatComparator> bayerToFormat{ > > > { formats::SGRBG16, V4L2PixelFormat(V4L2_PIX_FMT_SGRBG16) } }, > > > { { BayerFormat::RGGB, 16, BayerFormat::Packing::None }, > > > { formats::SRGGB16, V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16) } }, > > > + { { BayerFormat::BGGR, 16, BayerFormat::Packing::PISP1 }, > > > + { formats::BGGR16_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_BGGR) } }, > > > + { { BayerFormat::GBRG, 16, BayerFormat::Packing::PISP1 }, > > > + { formats::GBRG16_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GBRG) } }, > > > + { { BayerFormat::GRBG, 16, BayerFormat::Packing::PISP1 }, > > > + { formats::GRBG16_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GRBG) } }, > > > + { { BayerFormat::RGGB, 16, BayerFormat::Packing::PISP1 }, > > > + { formats::RGGB16_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_RGGB) } }, > > > { { BayerFormat::MONO, 8, BayerFormat::Packing::None }, > > > { formats::R8, V4L2PixelFormat(V4L2_PIX_FMT_GREY) } }, > > > { { BayerFormat::MONO, 10, BayerFormat::Packing::None }, > > > @@ -174,6 +182,8 @@ const std::map<BayerFormat, Formats, BayerFormatComparator> bayerToFormat{ > > > { formats::R12, V4L2PixelFormat(V4L2_PIX_FMT_Y12) } }, > > > { { BayerFormat::MONO, 16, BayerFormat::Packing::None }, > > > { formats::R16, V4L2PixelFormat(V4L2_PIX_FMT_Y16) } }, > > > + { { BayerFormat::MONO, 16, BayerFormat::Packing::PISP1 }, > > > + { formats::MONO_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_MONO) } }, > > > > Should the COMP2 versions be added as well ? > > To be honest, I doubt we will ever use COMP2, COMP1 is the better > option almost always. But I'll add it for completeness. > If you don't think they're useful feel free to skip them or add them later > > > > > > }; > > > > > > const std::unordered_map<unsigned int, BayerFormat> mbusCodeToBayer{ > > > @@ -209,6 +219,10 @@ const std::unordered_map<unsigned int, BayerFormat> mbusCodeToBayer{ > > > { MEDIA_BUS_FMT_SGBRG16_1X16, { BayerFormat::GBRG, 16, BayerFormat::Packing::None } }, > > > { MEDIA_BUS_FMT_SGRBG16_1X16, { BayerFormat::GRBG, 16, BayerFormat::Packing::None } }, > > > { MEDIA_BUS_FMT_SRGGB16_1X16, { BayerFormat::RGGB, 16, BayerFormat::Packing::None } }, > > > + { MEDIA_BUS_FMT_SBGGR16_1X16, { BayerFormat::BGGR, 16, BayerFormat::Packing::PISP1 } }, > > > + { MEDIA_BUS_FMT_SGBRG16_1X16, { BayerFormat::GBRG, 16, BayerFormat::Packing::PISP1 } }, > > > + { MEDIA_BUS_FMT_SGRBG16_1X16, { BayerFormat::GRBG, 16, BayerFormat::Packing::PISP1 } }, > > > + { MEDIA_BUS_FMT_SRGGB16_1X16, { BayerFormat::RGGB, 16, BayerFormat::Packing::PISP1 } }, > > > > mbusCodeToBayer is an unordered_map which > > "is an associative container that contains key-value pairs with unique > > keys." > > > > I'm afraid adding a new entry with the same key overwrite the existing > > one ? > > Oops you are right! I think this came about from when we originally > had MBUS codes for the PISP comp formats, and those subsequently got > removed. > > > > > We already had issues with this map, as we had similar problems with the > > _LE and _BE version of RGB565 which resolv to the same BayerFormat > > > > { MEDIA_BUS_FMT_SBGGR10_2X8_PADHI_BE, { BayerFormat::BGGR, 10, BayerFormat::Packing::None } }, > > { MEDIA_BUS_FMT_SBGGR10_2X8_PADHI_LE, { BayerFormat::BGGR, 10, BayerFormat::Packing::None } }, > > { MEDIA_BUS_FMT_SBGGR10_2X8_PADLO_BE, { BayerFormat::BGGR, 10, BayerFormat::Packing::None } }, > > { MEDIA_BUS_FMT_SBGGR10_2X8_PADLO_LE, { BayerFormat::BGGR, 10, BayerFormat::Packing::None } }, > > > > In general, we know that resolving mbus_codes to a unique format is > > ill-defined. How are you using these entries in the pi5 support ? > > As luck would have it, the Pi 5 pipeline handler never calls > BayerFormat::fromMbusCode() with a PISP_COMP type format so the code > never gets exercised and goes wrong! > > I think the right solution is to remove MEDIA_BUS_FMT_* for the > PISP_COMP formats as they genuinely don't have MBUS codes associated > with them. > Not 100% sure I get this. Are you proposing not to add the PISP1 compressed formats to the mbusCodeToBayer map ? > > > > > { MEDIA_BUS_FMT_Y8_1X8, { BayerFormat::MONO, 8, BayerFormat::Packing::None } }, > > > { MEDIA_BUS_FMT_Y10_1X10, { BayerFormat::MONO, 10, BayerFormat::Packing::None } }, > > > { MEDIA_BUS_FMT_Y12_1X12, { BayerFormat::MONO, 12, BayerFormat::Packing::None } }, > > > @@ -303,6 +317,10 @@ std::ostream &operator<<(std::ostream &out, const BayerFormat &f) > > > out << "-CSI2P"; > > > else if (f.packing == BayerFormat::Packing::IPU3) > > > out << "-IPU3P"; > > > + else if (f.packing == BayerFormat::Packing::PISP1) > > > + out << "-PISP1"; > > > + else if (f.packing == BayerFormat::Packing::PISP2) > > > + out << "-PISP2"; > > > > > > return out; > > > } > > > diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp > > > index a674f4179cc8..e603a7eda579 100644 > > > --- a/src/libcamera/formats.cpp > > > +++ b/src/libcamera/formats.cpp > > > @@ -547,6 +547,16 @@ const std::map<PixelFormat, PixelFormatInfo> pixelFormatInfo{ > > > .pixelsPerGroup = 1, > > > .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, > > > } }, > > > + { formats::MONO_PISP_COMP1, { > > > + .name = "MONO_PISP_COMP1", > > > + .format = formats::MONO_PISP_COMP1, > > > + .v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_MONO), }, > > > + .bitsPerPixel = 16, > > > + .colourEncoding = PixelFormatInfo::ColourEncodingYUV, > > > + .packed = true, > > > + .pixelsPerGroup = 1, > > > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, > > > + } }, > > > > > > /* Bayer formats. */ > > > { formats::SBGGR8, { > > > @@ -910,7 +920,46 @@ const std::map<PixelFormat, PixelFormatInfo> pixelFormatInfo{ > > > .pixelsPerGroup = 25, > > > .planes = {{ { 32, 1 }, { 0, 0 }, { 0, 0 } }}, > > > } }, > > > - > > > + { formats::BGGR16_PISP_COMP1, { > > > + .name = "BGGR16_PISP_COMP1", > > > + .format = formats::BGGR16_PISP_COMP1, > > > + .v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_BGGR), }, > > > + .bitsPerPixel = 16, > > > + .colourEncoding = PixelFormatInfo::ColourEncodingRAW, > > > + .packed = true, > > > + .pixelsPerGroup = 2, > > > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, > > > + } }, > > > > For regular 16-bit formats this should be > > > > .pixelsPerGroup = 2, > > .planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }}, > > > > As 2 pixels of 16 bits each consume 4 bytes. Is this {2, 1} > > intentional and due to compression ? > > Yes, I used {2, 1} to denote the 8-bit/sample after compression. Do > you think I should change to {4, 1}? Depends on how many bytes the format actually consumes :) For 16bpp RAW formats, we have pixelsPerGroup = 2 and each sample consumes 2 bytes. According to the pixelsPerGroup definition: * A pixel group is defined as the minimum number of pixels (including padding) * necessary in a row when the image has only one column of effective pixels. So for a RAW Bayer formats, to get a complete full-color "superpixel" we need at least two pixels per row. RG GB Each "RG" or "GB" pair is composed by 2 16-bit samples, for a total of 32bits of data, from which the {4, 1} value in planes[0] According to "Table 2", Chapter 5 of the PiSP datasheet, the compressed formats are described as 8-bits per pixel single channel compressed with mode n, for n = 1, 2, 3 Which suggests each "RG" or "GB" pair is actually 2 samples of 8 bits each, for a plane size of {2, 1}. Is this correct ? Is the actual .bitsPerPixel value for the compressed formats 8 instead of 16 then ? Thanks j > > Regards, > Naush > > > > > > > + { formats::GBRG16_PISP_COMP1, { > > > + .name = "GBRG16_PISP_COMP1", > > > + .format = formats::GBRG16_PISP_COMP1, > > > + .v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GBRG), }, > > > + .bitsPerPixel = 16, > > > + .colourEncoding = PixelFormatInfo::ColourEncodingRAW, > > > + .packed = true, > > > + .pixelsPerGroup = 2, > > > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, > > > + } }, > > > + { formats::GRBG16_PISP_COMP1, { > > > + .name = "GRBG16_PISP_COMP1", > > > + .format = formats::GRBG16_PISP_COMP1, > > > + .v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GRBG), }, > > > + .bitsPerPixel = 16, > > > + .colourEncoding = PixelFormatInfo::ColourEncodingRAW, > > > + .packed = true, > > > + .pixelsPerGroup = 2, > > > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, > > > + } }, > > > + { formats::RGGB16_PISP_COMP1, { > > > + .name = "RGGB16_PISP_COMP1", > > > + .format = formats::RGGB16_PISP_COMP1, > > > + .v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_RGGB), }, > > > + .bitsPerPixel = 16, > > > + .colourEncoding = PixelFormatInfo::ColourEncodingRAW, > > > + .packed = true, > > > + .pixelsPerGroup = 2, > > > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, > > > + } }, > > > /* Compressed formats. */ > > > { formats::MJPEG, { > > > .name = "MJPEG", > > > diff --git a/src/libcamera/formats.yaml b/src/libcamera/formats.yaml > > > index bde2cc803b98..f6df721243d0 100644 > > > --- a/src/libcamera/formats.yaml > > > +++ b/src/libcamera/formats.yaml > > > @@ -190,4 +190,20 @@ formats: > > > - SBGGR10_IPU3: > > > fourcc: DRM_FORMAT_SBGGR10 > > > mod: IPU3_FORMAT_MOD_PACKED > > > + > > > + - RGGB16_PISP_COMP1: > > > + fourcc: DRM_FORMAT_SRGGB16 > > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1 > > > + - GRBG16_PISP_COMP1: > > > + fourcc: DRM_FORMAT_SGRBG16 > > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1 > > > + - GBRG16_PISP_COMP1: > > > + fourcc: DRM_FORMAT_SGBRG16 > > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1 > > > + - BGGR16_PISP_COMP1: > > > + fourcc: DRM_FORMAT_SBGGR16 > > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1 > > > + - MONO_PISP_COMP1: > > > + fourcc: DRM_FORMAT_R16 > > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1 > > > ... > > > diff --git a/src/libcamera/v4l2_pixelformat.cpp b/src/libcamera/v4l2_pixelformat.cpp > > > index efb6f2940235..47baaf60199d 100644 > > > --- a/src/libcamera/v4l2_pixelformat.cpp > > > +++ b/src/libcamera/v4l2_pixelformat.cpp > > > @@ -207,6 +207,16 @@ const std::map<V4L2PixelFormat, V4L2PixelFormat::Info> vpf2pf{ > > > { formats::SGRBG16, "16-bit Bayer GRGR/BGBG" } }, > > > { V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16), > > > { formats::SRGGB16, "16-bit Bayer RGRG/GBGB" } }, > > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_BGGR), > > > + { formats::BGGR16_PISP_COMP1, "16-bit Bayer BGBG/GRGR PiSP Compress Mode 1" } }, > > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GBRG), > > > + { formats::GBRG16_PISP_COMP1, "16-bit Bayer GBGB/RGRG PiSP Compress Mode 1" } }, > > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GRBG), > > > + { formats::GRBG16_PISP_COMP1, "16-bit Bayer GRGR/BGBG PiSP Compress Mode 1" } }, > > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_RGGB), > > > + { formats::RGGB16_PISP_COMP1, "16-bit Bayer RGRG/GBGB PiSP Compress Mode 1" } }, > > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_MONO), > > > + { formats::MONO_PISP_COMP1, "16-bit Mono PiSP Compress Mode 1" } }, > > > > > > /* Compressed formats. */ > > > { V4L2PixelFormat(V4L2_PIX_FMT_MJPEG), > > > diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp > > > index 6d0785b7b484..aea90abaf9ef 100644 > > > --- a/src/libcamera/v4l2_subdevice.cpp > > > +++ b/src/libcamera/v4l2_subdevice.cpp > > > @@ -134,6 +134,10 @@ const std::map<uint32_t, V4L2SubdeviceFormatInfo> formatInfoMap = { > > > { MEDIA_BUS_FMT_SGBRG12_1X12, { 12, "SGBRG12_1X12", PixelFormatInfo::ColourEncodingRAW } }, > > > { MEDIA_BUS_FMT_SGRBG12_1X12, { 12, "SGRBG12_1X12", PixelFormatInfo::ColourEncodingRAW } }, > > > { MEDIA_BUS_FMT_SRGGB12_1X12, { 12, "SRGGB12_1X12", PixelFormatInfo::ColourEncodingRAW } }, > > > + { MEDIA_BUS_FMT_SBGGR16_1X16, { 16, "SBGGR16_1x16", PixelFormatInfo::ColourEncodingRAW } }, > > > + { MEDIA_BUS_FMT_SGBRG16_1X16, { 16, "SGBRG16_1x16", PixelFormatInfo::ColourEncodingRAW } }, > > > + { MEDIA_BUS_FMT_SGRBG16_1X16, { 16, "SGRBG16_1x16", PixelFormatInfo::ColourEncodingRAW } }, > > > + { MEDIA_BUS_FMT_SRGGB16_1X16, { 16, "SRGGB16_1x16", PixelFormatInfo::ColourEncodingRAW } }, > > > /* \todo Clarify colour encoding for HSV formats */ > > > { MEDIA_BUS_FMT_AHSV8888_1X32, { 32, "AHSV8888_1X32", PixelFormatInfo::ColourEncodingRGB } }, > > > { MEDIA_BUS_FMT_JPEG_1X8, { 8, "JPEG_1X8", PixelFormatInfo::ColourEncodingYUV } }, > > > -- > > > 2.34.1 > > >
Hi Jacopo, On Wed, 28 Feb 2024 at 11:14, Jacopo Mondi <jacopo.mondi@ideasonboard.com> wrote: > > Hi Naush > > On Tue, Feb 27, 2024 at 10:20:40AM +0000, Naushir Patuck wrote: > > Hi Jacopo, > > > > Thanks for the feedback, and sending the updates to the DRM mailing list! > > > > As usual, nothing's easy :) > > > On Mon, 26 Feb 2024 at 13:49, Jacopo Mondi > > <jacopo.mondi@ideasonboard.com> wrote: > > > > > > Hi Naush > > > > > > On Thu, Feb 15, 2024 at 01:27:10PM +0000, Naushir Patuck wrote: > > > > Add the Raspberry Pi 5 PiSP specific compressed Bayer format types 1/2: > > > > - V4L2_PIX_FMT_PISP_COMP1_xxx > > > > - V4L2_PIX_FMT_PISP_COMP2_xxx > > > > > > > > Add the Raspberry Pi 5 PiSP Backend config format: > > > > - V4L2_META_FMT_RPI_BE_CFG > > > > > > > > Additionally, we also extend libcamera format handlers to support 16-bit > > > > Bayer formats across the media bus. > > > > > > > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > > > > --- > > > > include/libcamera/internal/bayer_format.h | 2 + > > > > include/linux/drm_fourcc.h | 4 ++ > > > > include/linux/videodev2.h | 15 +++++++ > > > > src/libcamera/bayer_format.cpp | 18 ++++++++ > > > > src/libcamera/formats.cpp | 51 ++++++++++++++++++++++- > > > > src/libcamera/formats.yaml | 16 +++++++ > > > > src/libcamera/v4l2_pixelformat.cpp | 10 +++++ > > > > src/libcamera/v4l2_subdevice.cpp | 4 ++ > > > > 8 files changed, 119 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/include/libcamera/internal/bayer_format.h b/include/libcamera/internal/bayer_format.h > > > > index 78ba3969913d..164743f7e9f6 100644 > > > > --- a/include/libcamera/internal/bayer_format.h > > > > +++ b/include/libcamera/internal/bayer_format.h > > > > @@ -34,6 +34,8 @@ public: > > > > None = 0, > > > > CSI2 = 1, > > > > IPU3 = 2, > > > > + PISP1 = 3, > > > > + PISP2 = 4, > > > > }; > > > > > > > > constexpr BayerFormat() > > > > diff --git a/include/linux/drm_fourcc.h b/include/linux/drm_fourcc.h > > > > index 4ee421b95730..eff27fbe5a1e 100644 > > > > --- a/include/linux/drm_fourcc.h > > > > +++ b/include/linux/drm_fourcc.h > > > > @@ -490,6 +490,7 @@ extern "C" { > > > > #define DRM_FORMAT_MOD_VENDOR_ALLWINNER 0x09 > > > > #define DRM_FORMAT_MOD_VENDOR_AMLOGIC 0x0a > > > > #define DRM_FORMAT_MOD_VENDOR_MIPI 0x0b > > > > +#define DRM_FORMAT_MOD_VENDOR_RPI 0x0c > > > > > > > > /* add more to the end as needed */ > > > > > > > > @@ -1670,6 +1671,9 @@ drm_fourcc_canonicalize_nvidia_format_mod(__u64 modifier) > > > > */ > > > > #define MIPI_FORMAT_MOD_CSI2_PACKED fourcc_mod_code(MIPI, 1) > > > > > > > > +#define PISP_FORMAT_MOD_COMPRESS_MODE1 fourcc_mod_code(RPI, 1) > > > > +#define PISP_FORMAT_MOD_COMPRESS_MODE2 fourcc_mod_code(RPI, 2) > > > > + > > > > > > I'll send a patch to DRM/KMS for these formats as well > > > > > > > #if defined(__cplusplus) > > > > } > > > > #endif > > > > diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h > > > > index ba48d2c89726..59af6f794680 100644 > > > > --- a/include/linux/videodev2.h > > > > +++ b/include/linux/videodev2.h > > > > @@ -789,6 +789,18 @@ struct v4l2_pix_format { > > > > #define V4L2_PIX_FMT_IPU3_SGRBG10 v4l2_fourcc('i', 'p', '3', 'G') /* IPU3 packed 10-bit GRBG bayer */ > > > > #define V4L2_PIX_FMT_IPU3_SRGGB10 v4l2_fourcc('i', 'p', '3', 'r') /* IPU3 packed 10-bit RGGB bayer */ > > > > > > > > +/* The pixel format for all our buffers (the precise format is found in the config buffer). */ > > > > +#define V4L2_PIX_FMT_PISP_COMP1_RGGB v4l2_fourcc('P', 'C', '1', 'R') > > > > +#define V4L2_PIX_FMT_PISP_COMP1_GRBG v4l2_fourcc('P', 'C', '1', 'G') > > > > +#define V4L2_PIX_FMT_PISP_COMP1_GBRG v4l2_fourcc('P', 'C', '1', 'g') > > > > +#define V4L2_PIX_FMT_PISP_COMP1_BGGR v4l2_fourcc('P', 'C', '1', 'B') > > > > +#define V4L2_PIX_FMT_PISP_COMP1_MONO v4l2_fourcc('P', 'C', '1', 'M') > > > > +#define V4L2_PIX_FMT_PISP_COMP2_RGGB v4l2_fourcc('P', 'C', '2', 'R') > > > > +#define V4L2_PIX_FMT_PISP_COMP2_GRBG v4l2_fourcc('P', 'C', '2', 'G') > > > > +#define V4L2_PIX_FMT_PISP_COMP2_GBRG v4l2_fourcc('P', 'C', '2', 'g') > > > > +#define V4L2_PIX_FMT_PISP_COMP2_BGGR v4l2_fourcc('P', 'C', '2', 'B') > > > > +#define V4L2_PIX_FMT_PISP_COMP2_MONO v4l2_fourcc('P', 'C', '2', 'M') > > > > + > > > > > > I would use here what has been sent upstream for the BE support > > > > > > /* Raspberry Pi PiSP compressed formats. */ > > > #define V4L2_PIX_FMT_PISP_COMP1_RGGB v4l2_fourcc('P', 'C', '1', 'R') /* PiSP 8-bit mode 1 compressed RGGB bayer */ > > > #define V4L2_PIX_FMT_PISP_COMP1_GRBG v4l2_fourcc('P', 'C', '1', 'G') /* PiSP 8-bit mode 1 compressed GRBG bayer */ > > > #define V4L2_PIX_FMT_PISP_COMP1_GBRG v4l2_fourcc('P', 'C', '1', 'g') /* PiSP 8-bit mode 1 compressed GBRG bayer */ > > > #define V4L2_PIX_FMT_PISP_COMP1_BGGR v4l2_fourcc('P', 'C', '1', 'B') /* PiSP 8-bit mode 1 compressed BGGR bayer */ > > > #define V4L2_PIX_FMT_PISP_COMP1_MONO v4l2_fourcc('P', 'C', '1', 'M') /* PiSP 8-bit mode 1 compressed monochrome */ > > > #define V4L2_PIX_FMT_PISP_COMP2_RGGB v4l2_fourcc('P', 'C', '2', 'R') /* PiSP 8-bit mode 2 compressed RGGB bayer */ > > > #define V4L2_PIX_FMT_PISP_COMP2_GRBG v4l2_fourcc('P', 'C', '2', 'G') /* PiSP 8-bit mode 2 compressed GRBG bayer */ > > > #define V4L2_PIX_FMT_PISP_COMP2_GBRG v4l2_fourcc('P', 'C', '2', 'g') /* PiSP 8-bit mode 2 compressed GBRG bayer */ > > > #define V4L2_PIX_FMT_PISP_COMP2_BGGR v4l2_fourcc('P', 'C', '2', 'B') /* PiSP 8-bit mode 2 compressed BGGR bayer */ > > > #define V4L2_PIX_FMT_PISP_COMP2_MONO v4l2_fourcc('P', 'C', '2', 'M') /* PiSP 8-bit mode 2 compressed monochrome */ > > > > > > To make it easier to update it when these will land in upstream > > > > Ack. > > > > > > > > > /* SDR formats - used only for Software Defined Radio devices */ > > > > #define V4L2_SDR_FMT_CU8 v4l2_fourcc('C', 'U', '0', '8') /* IQ u8 */ > > > > #define V4L2_SDR_FMT_CU16LE v4l2_fourcc('C', 'U', '1', '6') /* IQ u16le */ > > > > @@ -818,6 +830,9 @@ struct v4l2_pix_format { > > > > #define V4L2_META_FMT_RK_ISP1_PARAMS v4l2_fourcc('R', 'K', '1', 'P') /* Rockchip ISP1 3A Parameters */ > > > > #define V4L2_META_FMT_RK_ISP1_STAT_3A v4l2_fourcc('R', 'K', '1', 'S') /* Rockchip ISP1 3A Statistics */ > > > > > > > > +/* Vendor specific - used for RaspberryPi PiSP */ > > > > +#define V4L2_META_FMT_RPI_BE_CFG v4l2_fourcc('R', 'P', 'B', 'C') /* PiSP BE configuration */ > > > > /* priv field value to indicates that subsequent fields are valid. */ > > > > #define V4L2_PIX_FMT_PRIV_MAGIC 0xfeedcafe > > > > > > > > diff --git a/src/libcamera/bayer_format.cpp b/src/libcamera/bayer_format.cpp > > > > index 20aedfa6d925..333b1117f531 100644 > > > > --- a/src/libcamera/bayer_format.cpp > > > > +++ b/src/libcamera/bayer_format.cpp > > > > @@ -164,6 +164,14 @@ const std::map<BayerFormat, Formats, BayerFormatComparator> bayerToFormat{ > > > > { formats::SGRBG16, V4L2PixelFormat(V4L2_PIX_FMT_SGRBG16) } }, > > > > { { BayerFormat::RGGB, 16, BayerFormat::Packing::None }, > > > > { formats::SRGGB16, V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16) } }, > > > > + { { BayerFormat::BGGR, 16, BayerFormat::Packing::PISP1 }, > > > > + { formats::BGGR16_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_BGGR) } }, > > > > + { { BayerFormat::GBRG, 16, BayerFormat::Packing::PISP1 }, > > > > + { formats::GBRG16_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GBRG) } }, > > > > + { { BayerFormat::GRBG, 16, BayerFormat::Packing::PISP1 }, > > > > + { formats::GRBG16_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GRBG) } }, > > > > + { { BayerFormat::RGGB, 16, BayerFormat::Packing::PISP1 }, > > > > + { formats::RGGB16_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_RGGB) } }, > > > > { { BayerFormat::MONO, 8, BayerFormat::Packing::None }, > > > > { formats::R8, V4L2PixelFormat(V4L2_PIX_FMT_GREY) } }, > > > > { { BayerFormat::MONO, 10, BayerFormat::Packing::None }, > > > > @@ -174,6 +182,8 @@ const std::map<BayerFormat, Formats, BayerFormatComparator> bayerToFormat{ > > > > { formats::R12, V4L2PixelFormat(V4L2_PIX_FMT_Y12) } }, > > > > { { BayerFormat::MONO, 16, BayerFormat::Packing::None }, > > > > { formats::R16, V4L2PixelFormat(V4L2_PIX_FMT_Y16) } }, > > > > + { { BayerFormat::MONO, 16, BayerFormat::Packing::PISP1 }, > > > > + { formats::MONO_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_MONO) } }, > > > > > > Should the COMP2 versions be added as well ? > > > > To be honest, I doubt we will ever use COMP2, COMP1 is the better > > option almost always. But I'll add it for completeness. > > > > If you don't think they're useful feel free to skip them or add them > later I think that's best. > > > > > > > > > > }; > > > > > > > > const std::unordered_map<unsigned int, BayerFormat> mbusCodeToBayer{ > > > > @@ -209,6 +219,10 @@ const std::unordered_map<unsigned int, BayerFormat> mbusCodeToBayer{ > > > > { MEDIA_BUS_FMT_SGBRG16_1X16, { BayerFormat::GBRG, 16, BayerFormat::Packing::None } }, > > > > { MEDIA_BUS_FMT_SGRBG16_1X16, { BayerFormat::GRBG, 16, BayerFormat::Packing::None } }, > > > > { MEDIA_BUS_FMT_SRGGB16_1X16, { BayerFormat::RGGB, 16, BayerFormat::Packing::None } }, > > > > + { MEDIA_BUS_FMT_SBGGR16_1X16, { BayerFormat::BGGR, 16, BayerFormat::Packing::PISP1 } }, > > > > + { MEDIA_BUS_FMT_SGBRG16_1X16, { BayerFormat::GBRG, 16, BayerFormat::Packing::PISP1 } }, > > > > + { MEDIA_BUS_FMT_SGRBG16_1X16, { BayerFormat::GRBG, 16, BayerFormat::Packing::PISP1 } }, > > > > + { MEDIA_BUS_FMT_SRGGB16_1X16, { BayerFormat::RGGB, 16, BayerFormat::Packing::PISP1 } }, > > > > > > mbusCodeToBayer is an unordered_map which > > > "is an associative container that contains key-value pairs with unique > > > keys." > > > > > > I'm afraid adding a new entry with the same key overwrite the existing > > > one ? > > > > Oops you are right! I think this came about from when we originally > > had MBUS codes for the PISP comp formats, and those subsequently got > > removed. > > > > > > > > We already had issues with this map, as we had similar problems with the > > > _LE and _BE version of RGB565 which resolv to the same BayerFormat > > > > > > { MEDIA_BUS_FMT_SBGGR10_2X8_PADHI_BE, { BayerFormat::BGGR, 10, BayerFormat::Packing::None } }, > > > { MEDIA_BUS_FMT_SBGGR10_2X8_PADHI_LE, { BayerFormat::BGGR, 10, BayerFormat::Packing::None } }, > > > { MEDIA_BUS_FMT_SBGGR10_2X8_PADLO_BE, { BayerFormat::BGGR, 10, BayerFormat::Packing::None } }, > > > { MEDIA_BUS_FMT_SBGGR10_2X8_PADLO_LE, { BayerFormat::BGGR, 10, BayerFormat::Packing::None } }, > > > > > > In general, we know that resolving mbus_codes to a unique format is > > > ill-defined. How are you using these entries in the pi5 support ? > > > > As luck would have it, the Pi 5 pipeline handler never calls > > BayerFormat::fromMbusCode() with a PISP_COMP type format so the code > > never gets exercised and goes wrong! > > > > I think the right solution is to remove MEDIA_BUS_FMT_* for the > > PISP_COMP formats as they genuinely don't have MBUS codes associated > > with them. > > > > Not 100% sure I get this. Are you proposing not to add the PISP1 > compressed formats to the mbusCodeToBayer map ? That's correct. PISP_COMP formats do not actually have mubs codes as they are not strictly formats used across a bus, only on the output. So we remove them from this table. The Pi 5 pipeline handler will never need to access an mbus code for a PISP_COMP format. > > > > > > > > { MEDIA_BUS_FMT_Y8_1X8, { BayerFormat::MONO, 8, BayerFormat::Packing::None } }, > > > > { MEDIA_BUS_FMT_Y10_1X10, { BayerFormat::MONO, 10, BayerFormat::Packing::None } }, > > > > { MEDIA_BUS_FMT_Y12_1X12, { BayerFormat::MONO, 12, BayerFormat::Packing::None } }, > > > > @@ -303,6 +317,10 @@ std::ostream &operator<<(std::ostream &out, const BayerFormat &f) > > > > out << "-CSI2P"; > > > > else if (f.packing == BayerFormat::Packing::IPU3) > > > > out << "-IPU3P"; > > > > + else if (f.packing == BayerFormat::Packing::PISP1) > > > > + out << "-PISP1"; > > > > + else if (f.packing == BayerFormat::Packing::PISP2) > > > > + out << "-PISP2"; > > > > > > > > return out; > > > > } > > > > diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp > > > > index a674f4179cc8..e603a7eda579 100644 > > > > --- a/src/libcamera/formats.cpp > > > > +++ b/src/libcamera/formats.cpp > > > > @@ -547,6 +547,16 @@ const std::map<PixelFormat, PixelFormatInfo> pixelFormatInfo{ > > > > .pixelsPerGroup = 1, > > > > .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, > > > > } }, > > > > + { formats::MONO_PISP_COMP1, { > > > > + .name = "MONO_PISP_COMP1", > > > > + .format = formats::MONO_PISP_COMP1, > > > > + .v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_MONO), }, > > > > + .bitsPerPixel = 16, > > > > + .colourEncoding = PixelFormatInfo::ColourEncodingYUV, > > > > + .packed = true, > > > > + .pixelsPerGroup = 1, > > > > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, > > > > + } }, > > > > > > > > /* Bayer formats. */ > > > > { formats::SBGGR8, { > > > > @@ -910,7 +920,46 @@ const std::map<PixelFormat, PixelFormatInfo> pixelFormatInfo{ > > > > .pixelsPerGroup = 25, > > > > .planes = {{ { 32, 1 }, { 0, 0 }, { 0, 0 } }}, > > > > } }, > > > > - > > > > + { formats::BGGR16_PISP_COMP1, { > > > > + .name = "BGGR16_PISP_COMP1", > > > > + .format = formats::BGGR16_PISP_COMP1, > > > > + .v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_BGGR), }, > > > > + .bitsPerPixel = 16, > > > > + .colourEncoding = PixelFormatInfo::ColourEncodingRAW, > > > > + .packed = true, > > > > + .pixelsPerGroup = 2, > > > > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, > > > > + } }, > > > > > > For regular 16-bit formats this should be > > > > > > .pixelsPerGroup = 2, > > > .planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }}, > > > > > > As 2 pixels of 16 bits each consume 4 bytes. Is this {2, 1} > > > intentional and due to compression ? > > > > Yes, I used {2, 1} to denote the 8-bit/sample after compression. Do > > you think I should change to {4, 1}? > > Depends on how many bytes the format actually consumes :) > > For 16bpp RAW formats, we have pixelsPerGroup = 2 and each sample > consumes 2 bytes. > > According to the pixelsPerGroup definition: > > * A pixel group is defined as the minimum number of pixels (including padding) > * necessary in a row when the image has only one column of effective pixels. > > So for a RAW Bayer formats, to get a complete full-color "superpixel" > we need at least two pixels per row. > > RG > GB > > Each "RG" or "GB" pair is composed by 2 16-bit samples, for a total of > 32bits of data, from which the {4, 1} value in planes[0] > > According to "Table 2", Chapter 5 of the PiSP datasheet, the > compressed formats are described as > > 8-bits per pixel single channel compressed with mode n, for n = 1, 2, 3 > > Which suggests each "RG" or "GB" pair is actually 2 samples of 8 bits > each, for a plane size of {2, 1}. > > Is this correct ? Is the actual .bitsPerPixel value for the compressed > formats 8 instead of 16 then ? This is correct, each sample is 8-bits so "RG"/"GB" pairs are 16-bits each. So it sounds like I need to use .planes = {2, 1}, but .bitsPerPixel = 8 maybe? Naush > > Thanks > j > > > > > Regards, > > Naush > > > > > > > > > > > + { formats::GBRG16_PISP_COMP1, { > > > > + .name = "GBRG16_PISP_COMP1", > > > > + .format = formats::GBRG16_PISP_COMP1, > > > > + .v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GBRG), }, > > > > + .bitsPerPixel = 16, > > > > + .colourEncoding = PixelFormatInfo::ColourEncodingRAW, > > > > + .packed = true, > > > > + .pixelsPerGroup = 2, > > > > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, > > > > + } }, > > > > + { formats::GRBG16_PISP_COMP1, { > > > > + .name = "GRBG16_PISP_COMP1", > > > > + .format = formats::GRBG16_PISP_COMP1, > > > > + .v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GRBG), }, > > > > + .bitsPerPixel = 16, > > > > + .colourEncoding = PixelFormatInfo::ColourEncodingRAW, > > > > + .packed = true, > > > > + .pixelsPerGroup = 2, > > > > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, > > > > + } }, > > > > + { formats::RGGB16_PISP_COMP1, { > > > > + .name = "RGGB16_PISP_COMP1", > > > > + .format = formats::RGGB16_PISP_COMP1, > > > > + .v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_RGGB), }, > > > > + .bitsPerPixel = 16, > > > > + .colourEncoding = PixelFormatInfo::ColourEncodingRAW, > > > > + .packed = true, > > > > + .pixelsPerGroup = 2, > > > > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, > > > > + } }, > > > > /* Compressed formats. */ > > > > { formats::MJPEG, { > > > > .name = "MJPEG", > > > > diff --git a/src/libcamera/formats.yaml b/src/libcamera/formats.yaml > > > > index bde2cc803b98..f6df721243d0 100644 > > > > --- a/src/libcamera/formats.yaml > > > > +++ b/src/libcamera/formats.yaml > > > > @@ -190,4 +190,20 @@ formats: > > > > - SBGGR10_IPU3: > > > > fourcc: DRM_FORMAT_SBGGR10 > > > > mod: IPU3_FORMAT_MOD_PACKED > > > > + > > > > + - RGGB16_PISP_COMP1: > > > > + fourcc: DRM_FORMAT_SRGGB16 > > > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1 > > > > + - GRBG16_PISP_COMP1: > > > > + fourcc: DRM_FORMAT_SGRBG16 > > > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1 > > > > + - GBRG16_PISP_COMP1: > > > > + fourcc: DRM_FORMAT_SGBRG16 > > > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1 > > > > + - BGGR16_PISP_COMP1: > > > > + fourcc: DRM_FORMAT_SBGGR16 > > > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1 > > > > + - MONO_PISP_COMP1: > > > > + fourcc: DRM_FORMAT_R16 > > > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1 > > > > ... > > > > diff --git a/src/libcamera/v4l2_pixelformat.cpp b/src/libcamera/v4l2_pixelformat.cpp > > > > index efb6f2940235..47baaf60199d 100644 > > > > --- a/src/libcamera/v4l2_pixelformat.cpp > > > > +++ b/src/libcamera/v4l2_pixelformat.cpp > > > > @@ -207,6 +207,16 @@ const std::map<V4L2PixelFormat, V4L2PixelFormat::Info> vpf2pf{ > > > > { formats::SGRBG16, "16-bit Bayer GRGR/BGBG" } }, > > > > { V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16), > > > > { formats::SRGGB16, "16-bit Bayer RGRG/GBGB" } }, > > > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_BGGR), > > > > + { formats::BGGR16_PISP_COMP1, "16-bit Bayer BGBG/GRGR PiSP Compress Mode 1" } }, > > > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GBRG), > > > > + { formats::GBRG16_PISP_COMP1, "16-bit Bayer GBGB/RGRG PiSP Compress Mode 1" } }, > > > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GRBG), > > > > + { formats::GRBG16_PISP_COMP1, "16-bit Bayer GRGR/BGBG PiSP Compress Mode 1" } }, > > > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_RGGB), > > > > + { formats::RGGB16_PISP_COMP1, "16-bit Bayer RGRG/GBGB PiSP Compress Mode 1" } }, > > > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_MONO), > > > > + { formats::MONO_PISP_COMP1, "16-bit Mono PiSP Compress Mode 1" } }, > > > > > > > > /* Compressed formats. */ > > > > { V4L2PixelFormat(V4L2_PIX_FMT_MJPEG), > > > > diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp > > > > index 6d0785b7b484..aea90abaf9ef 100644 > > > > --- a/src/libcamera/v4l2_subdevice.cpp > > > > +++ b/src/libcamera/v4l2_subdevice.cpp > > > > @@ -134,6 +134,10 @@ const std::map<uint32_t, V4L2SubdeviceFormatInfo> formatInfoMap = { > > > > { MEDIA_BUS_FMT_SGBRG12_1X12, { 12, "SGBRG12_1X12", PixelFormatInfo::ColourEncodingRAW } }, > > > > { MEDIA_BUS_FMT_SGRBG12_1X12, { 12, "SGRBG12_1X12", PixelFormatInfo::ColourEncodingRAW } }, > > > > { MEDIA_BUS_FMT_SRGGB12_1X12, { 12, "SRGGB12_1X12", PixelFormatInfo::ColourEncodingRAW } }, > > > > + { MEDIA_BUS_FMT_SBGGR16_1X16, { 16, "SBGGR16_1x16", PixelFormatInfo::ColourEncodingRAW } }, > > > > + { MEDIA_BUS_FMT_SGBRG16_1X16, { 16, "SGBRG16_1x16", PixelFormatInfo::ColourEncodingRAW } }, > > > > + { MEDIA_BUS_FMT_SGRBG16_1X16, { 16, "SGRBG16_1x16", PixelFormatInfo::ColourEncodingRAW } }, > > > > + { MEDIA_BUS_FMT_SRGGB16_1X16, { 16, "SRGGB16_1x16", PixelFormatInfo::ColourEncodingRAW } }, > > > > /* \todo Clarify colour encoding for HSV formats */ > > > > { MEDIA_BUS_FMT_AHSV8888_1X32, { 32, "AHSV8888_1X32", PixelFormatInfo::ColourEncodingRGB } }, > > > > { MEDIA_BUS_FMT_JPEG_1X8, { 8, "JPEG_1X8", PixelFormatInfo::ColourEncodingYUV } }, > > > > -- > > > > 2.34.1 > > > >
Hi Naush On Wed, Feb 28, 2024 at 04:45:54PM +0000, Naushir Patuck wrote: > Hi Jacopo, > > > > > > - > > > > > + { formats::BGGR16_PISP_COMP1, { > > > > > + .name = "BGGR16_PISP_COMP1", > > > > > + .format = formats::BGGR16_PISP_COMP1, > > > > > + .v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_BGGR), }, > > > > > + .bitsPerPixel = 16, > > > > > + .colourEncoding = PixelFormatInfo::ColourEncodingRAW, > > > > > + .packed = true, > > > > > + .pixelsPerGroup = 2, > > > > > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, > > > > > + } }, > > > > > > > > For regular 16-bit formats this should be > > > > > > > > .pixelsPerGroup = 2, > > > > .planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }}, > > > > > > > > As 2 pixels of 16 bits each consume 4 bytes. Is this {2, 1} > > > > intentional and due to compression ? > > > > > > Yes, I used {2, 1} to denote the 8-bit/sample after compression. Do > > > you think I should change to {4, 1}? > > > > Depends on how many bytes the format actually consumes :) > > > > For 16bpp RAW formats, we have pixelsPerGroup = 2 and each sample > > consumes 2 bytes. > > > > According to the pixelsPerGroup definition: > > > > * A pixel group is defined as the minimum number of pixels (including padding) > > * necessary in a row when the image has only one column of effective pixels. > > > > So for a RAW Bayer formats, to get a complete full-color "superpixel" > > we need at least two pixels per row. > > > > RG > > GB > > > > Each "RG" or "GB" pair is composed by 2 16-bit samples, for a total of > > 32bits of data, from which the {4, 1} value in planes[0] > > > > According to "Table 2", Chapter 5 of the PiSP datasheet, the > > compressed formats are described as > > > > 8-bits per pixel single channel compressed with mode n, for n = 1, 2, 3 > > > > Which suggests each "RG" or "GB" pair is actually 2 samples of 8 bits > > each, for a plane size of {2, 1}. > > > > Is this correct ? Is the actual .bitsPerPixel value for the compressed > > formats 8 instead of 16 then ? > > This is correct, each sample is 8-bits so "RG"/"GB" pairs are 16-bits each. > So it sounds like I need to use .planes = {2, 1}, but .bitsPerPixel = 8 maybe? I now wonder if the '16' in the format names is correct then :) > > Naush > > > > > Thanks > > j > > > > > > > > Regards, > > > Naush > > > > > > > > > > > > > > > + { formats::GBRG16_PISP_COMP1, { > > > > > + .name = "GBRG16_PISP_COMP1", > > > > > + .format = formats::GBRG16_PISP_COMP1, > > > > > + .v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GBRG), }, > > > > > + .bitsPerPixel = 16, > > > > > + .colourEncoding = PixelFormatInfo::ColourEncodingRAW, > > > > > + .packed = true, > > > > > + .pixelsPerGroup = 2, > > > > > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, > > > > > + } }, > > > > > + { formats::GRBG16_PISP_COMP1, { > > > > > + .name = "GRBG16_PISP_COMP1", > > > > > + .format = formats::GRBG16_PISP_COMP1, > > > > > + .v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GRBG), }, > > > > > + .bitsPerPixel = 16, > > > > > + .colourEncoding = PixelFormatInfo::ColourEncodingRAW, > > > > > + .packed = true, > > > > > + .pixelsPerGroup = 2, > > > > > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, > > > > > + } }, > > > > > + { formats::RGGB16_PISP_COMP1, { > > > > > + .name = "RGGB16_PISP_COMP1", > > > > > + .format = formats::RGGB16_PISP_COMP1, > > > > > + .v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_RGGB), }, > > > > > + .bitsPerPixel = 16, > > > > > + .colourEncoding = PixelFormatInfo::ColourEncodingRAW, > > > > > + .packed = true, > > > > > + .pixelsPerGroup = 2, > > > > > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, > > > > > + } }, > > > > > /* Compressed formats. */ > > > > > { formats::MJPEG, { > > > > > .name = "MJPEG", > > > > > diff --git a/src/libcamera/formats.yaml b/src/libcamera/formats.yaml > > > > > index bde2cc803b98..f6df721243d0 100644 > > > > > --- a/src/libcamera/formats.yaml > > > > > +++ b/src/libcamera/formats.yaml > > > > > @@ -190,4 +190,20 @@ formats: > > > > > - SBGGR10_IPU3: > > > > > fourcc: DRM_FORMAT_SBGGR10 > > > > > mod: IPU3_FORMAT_MOD_PACKED > > > > > + > > > > > + - RGGB16_PISP_COMP1: > > > > > + fourcc: DRM_FORMAT_SRGGB16 > > > > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1 > > > > > + - GRBG16_PISP_COMP1: > > > > > + fourcc: DRM_FORMAT_SGRBG16 > > > > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1 > > > > > + - GBRG16_PISP_COMP1: > > > > > + fourcc: DRM_FORMAT_SGBRG16 > > > > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1 > > > > > + - BGGR16_PISP_COMP1: > > > > > + fourcc: DRM_FORMAT_SBGGR16 > > > > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1 > > > > > + - MONO_PISP_COMP1: > > > > > + fourcc: DRM_FORMAT_R16 > > > > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1 > > > > > ... > > > > > diff --git a/src/libcamera/v4l2_pixelformat.cpp b/src/libcamera/v4l2_pixelformat.cpp > > > > > index efb6f2940235..47baaf60199d 100644 > > > > > --- a/src/libcamera/v4l2_pixelformat.cpp > > > > > +++ b/src/libcamera/v4l2_pixelformat.cpp > > > > > @@ -207,6 +207,16 @@ const std::map<V4L2PixelFormat, V4L2PixelFormat::Info> vpf2pf{ > > > > > { formats::SGRBG16, "16-bit Bayer GRGR/BGBG" } }, > > > > > { V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16), > > > > > { formats::SRGGB16, "16-bit Bayer RGRG/GBGB" } }, > > > > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_BGGR), > > > > > + { formats::BGGR16_PISP_COMP1, "16-bit Bayer BGBG/GRGR PiSP Compress Mode 1" } }, > > > > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GBRG), > > > > > + { formats::GBRG16_PISP_COMP1, "16-bit Bayer GBGB/RGRG PiSP Compress Mode 1" } }, > > > > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GRBG), > > > > > + { formats::GRBG16_PISP_COMP1, "16-bit Bayer GRGR/BGBG PiSP Compress Mode 1" } }, > > > > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_RGGB), > > > > > + { formats::RGGB16_PISP_COMP1, "16-bit Bayer RGRG/GBGB PiSP Compress Mode 1" } }, > > > > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_MONO), > > > > > + { formats::MONO_PISP_COMP1, "16-bit Mono PiSP Compress Mode 1" } }, > > > > > > > > > > /* Compressed formats. */ > > > > > { V4L2PixelFormat(V4L2_PIX_FMT_MJPEG), > > > > > diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp > > > > > index 6d0785b7b484..aea90abaf9ef 100644 > > > > > --- a/src/libcamera/v4l2_subdevice.cpp > > > > > +++ b/src/libcamera/v4l2_subdevice.cpp > > > > > @@ -134,6 +134,10 @@ const std::map<uint32_t, V4L2SubdeviceFormatInfo> formatInfoMap = { > > > > > { MEDIA_BUS_FMT_SGBRG12_1X12, { 12, "SGBRG12_1X12", PixelFormatInfo::ColourEncodingRAW } }, > > > > > { MEDIA_BUS_FMT_SGRBG12_1X12, { 12, "SGRBG12_1X12", PixelFormatInfo::ColourEncodingRAW } }, > > > > > { MEDIA_BUS_FMT_SRGGB12_1X12, { 12, "SRGGB12_1X12", PixelFormatInfo::ColourEncodingRAW } }, > > > > > + { MEDIA_BUS_FMT_SBGGR16_1X16, { 16, "SBGGR16_1x16", PixelFormatInfo::ColourEncodingRAW } }, > > > > > + { MEDIA_BUS_FMT_SGBRG16_1X16, { 16, "SGBRG16_1x16", PixelFormatInfo::ColourEncodingRAW } }, > > > > > + { MEDIA_BUS_FMT_SGRBG16_1X16, { 16, "SGRBG16_1x16", PixelFormatInfo::ColourEncodingRAW } }, > > > > > + { MEDIA_BUS_FMT_SRGGB16_1X16, { 16, "SRGGB16_1x16", PixelFormatInfo::ColourEncodingRAW } }, > > > > > /* \todo Clarify colour encoding for HSV formats */ > > > > > { MEDIA_BUS_FMT_AHSV8888_1X32, { 32, "AHSV8888_1X32", PixelFormatInfo::ColourEncodingRGB } }, > > > > > { MEDIA_BUS_FMT_JPEG_1X8, { 8, "JPEG_1X8", PixelFormatInfo::ColourEncodingYUV } }, > > > > > -- > > > > > 2.34.1 > > > > >
On Wed, 28 Feb 2024, 5:16 pm Jacopo Mondi, <jacopo.mondi@ideasonboard.com> wrote: > Hi Naush > > On Wed, Feb 28, 2024 at 04:45:54PM +0000, Naushir Patuck wrote: > > Hi Jacopo, > > > > > > > > - > > > > > > + { formats::BGGR16_PISP_COMP1, { > > > > > > + .name = "BGGR16_PISP_COMP1", > > > > > > + .format = formats::BGGR16_PISP_COMP1, > > > > > > + .v4l2Formats = { > V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_BGGR), }, > > > > > > + .bitsPerPixel = 16, > > > > > > + .colourEncoding = > PixelFormatInfo::ColourEncodingRAW, > > > > > > + .packed = true, > > > > > > + .pixelsPerGroup = 2, > > > > > > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, > > > > > > + } }, > > > > > > > > > > For regular 16-bit formats this should be > > > > > > > > > > .pixelsPerGroup = 2, > > > > > .planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }}, > > > > > > > > > > As 2 pixels of 16 bits each consume 4 bytes. Is this {2, 1} > > > > > intentional and due to compression ? > > > > > > > > Yes, I used {2, 1} to denote the 8-bit/sample after compression. Do > > > > you think I should change to {4, 1}? > > > > > > Depends on how many bytes the format actually consumes :) > > > > > > For 16bpp RAW formats, we have pixelsPerGroup = 2 and each sample > > > consumes 2 bytes. > > > > > > According to the pixelsPerGroup definition: > > > > > > * A pixel group is defined as the minimum number of pixels (including > padding) > > > * necessary in a row when the image has only one column of effective > pixels. > > > > > > So for a RAW Bayer formats, to get a complete full-color "superpixel" > > > we need at least two pixels per row. > > > > > > RG > > > GB > > > > > > Each "RG" or "GB" pair is composed by 2 16-bit samples, for a total of > > > 32bits of data, from which the {4, 1} value in planes[0] > > > > > > According to "Table 2", Chapter 5 of the PiSP datasheet, the > > > compressed formats are described as > > > > > > 8-bits per pixel single channel compressed with mode n, for n = > 1, 2, 3 > > > > > > Which suggests each "RG" or "GB" pair is actually 2 samples of 8 bits > > > each, for a plane size of {2, 1}. > > > > > > Is this correct ? Is the actual .bitsPerPixel value for the compressed > > > formats 8 instead of 16 then ? > > > > This is correct, each sample is 8-bits so "RG"/"GB" pairs are 16-bits > each. > > So it sounds like I need to use .planes = {2, 1}, but .bitsPerPixel = 8 > maybe? > > I now wonder if the '16' in the format names is correct then :) > It's meant to signify 16-bits compressed to 8-bits (in this case). Maybe a defined naming scheme is needed for such cases? > > > > Naush > > > > > > > > Thanks > > > j > > > > > > > > > > > Regards, > > > > Naush > > > > > > > > > > > > > > > > > > > + { formats::GBRG16_PISP_COMP1, { > > > > > > + .name = "GBRG16_PISP_COMP1", > > > > > > + .format = formats::GBRG16_PISP_COMP1, > > > > > > + .v4l2Formats = { > V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GBRG), }, > > > > > > + .bitsPerPixel = 16, > > > > > > + .colourEncoding = > PixelFormatInfo::ColourEncodingRAW, > > > > > > + .packed = true, > > > > > > + .pixelsPerGroup = 2, > > > > > > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, > > > > > > + } }, > > > > > > + { formats::GRBG16_PISP_COMP1, { > > > > > > + .name = "GRBG16_PISP_COMP1", > > > > > > + .format = formats::GRBG16_PISP_COMP1, > > > > > > + .v4l2Formats = { > V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GRBG), }, > > > > > > + .bitsPerPixel = 16, > > > > > > + .colourEncoding = > PixelFormatInfo::ColourEncodingRAW, > > > > > > + .packed = true, > > > > > > + .pixelsPerGroup = 2, > > > > > > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, > > > > > > + } }, > > > > > > + { formats::RGGB16_PISP_COMP1, { > > > > > > + .name = "RGGB16_PISP_COMP1", > > > > > > + .format = formats::RGGB16_PISP_COMP1, > > > > > > + .v4l2Formats = { > V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_RGGB), }, > > > > > > + .bitsPerPixel = 16, > > > > > > + .colourEncoding = > PixelFormatInfo::ColourEncodingRAW, > > > > > > + .packed = true, > > > > > > + .pixelsPerGroup = 2, > > > > > > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, > > > > > > + } }, > > > > > > /* Compressed formats. */ > > > > > > { formats::MJPEG, { > > > > > > .name = "MJPEG", > > > > > > diff --git a/src/libcamera/formats.yaml > b/src/libcamera/formats.yaml > > > > > > index bde2cc803b98..f6df721243d0 100644 > > > > > > --- a/src/libcamera/formats.yaml > > > > > > +++ b/src/libcamera/formats.yaml > > > > > > @@ -190,4 +190,20 @@ formats: > > > > > > - SBGGR10_IPU3: > > > > > > fourcc: DRM_FORMAT_SBGGR10 > > > > > > mod: IPU3_FORMAT_MOD_PACKED > > > > > > + > > > > > > + - RGGB16_PISP_COMP1: > > > > > > + fourcc: DRM_FORMAT_SRGGB16 > > > > > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1 > > > > > > + - GRBG16_PISP_COMP1: > > > > > > + fourcc: DRM_FORMAT_SGRBG16 > > > > > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1 > > > > > > + - GBRG16_PISP_COMP1: > > > > > > + fourcc: DRM_FORMAT_SGBRG16 > > > > > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1 > > > > > > + - BGGR16_PISP_COMP1: > > > > > > + fourcc: DRM_FORMAT_SBGGR16 > > > > > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1 > > > > > > + - MONO_PISP_COMP1: > > > > > > + fourcc: DRM_FORMAT_R16 > > > > > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1 > > > > > > ... > > > > > > diff --git a/src/libcamera/v4l2_pixelformat.cpp > b/src/libcamera/v4l2_pixelformat.cpp > > > > > > index efb6f2940235..47baaf60199d 100644 > > > > > > --- a/src/libcamera/v4l2_pixelformat.cpp > > > > > > +++ b/src/libcamera/v4l2_pixelformat.cpp > > > > > > @@ -207,6 +207,16 @@ const std::map<V4L2PixelFormat, > V4L2PixelFormat::Info> vpf2pf{ > > > > > > { formats::SGRBG16, "16-bit Bayer GRGR/BGBG" } }, > > > > > > { V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16), > > > > > > { formats::SRGGB16, "16-bit Bayer RGRG/GBGB" } }, > > > > > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_BGGR), > > > > > > + { formats::BGGR16_PISP_COMP1, "16-bit Bayer > BGBG/GRGR PiSP Compress Mode 1" } }, > > > > > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GBRG), > > > > > > + { formats::GBRG16_PISP_COMP1, "16-bit Bayer > GBGB/RGRG PiSP Compress Mode 1" } }, > > > > > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GRBG), > > > > > > + { formats::GRBG16_PISP_COMP1, "16-bit Bayer > GRGR/BGBG PiSP Compress Mode 1" } }, > > > > > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_RGGB), > > > > > > + { formats::RGGB16_PISP_COMP1, "16-bit Bayer > RGRG/GBGB PiSP Compress Mode 1" } }, > > > > > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_MONO), > > > > > > + { formats::MONO_PISP_COMP1, "16-bit Mono PiSP > Compress Mode 1" } }, > > > > > > > > > > > > /* Compressed formats. */ > > > > > > { V4L2PixelFormat(V4L2_PIX_FMT_MJPEG), > > > > > > diff --git a/src/libcamera/v4l2_subdevice.cpp > b/src/libcamera/v4l2_subdevice.cpp > > > > > > index 6d0785b7b484..aea90abaf9ef 100644 > > > > > > --- a/src/libcamera/v4l2_subdevice.cpp > > > > > > +++ b/src/libcamera/v4l2_subdevice.cpp > > > > > > @@ -134,6 +134,10 @@ const std::map<uint32_t, > V4L2SubdeviceFormatInfo> formatInfoMap = { > > > > > > { MEDIA_BUS_FMT_SGBRG12_1X12, { 12, "SGBRG12_1X12", > PixelFormatInfo::ColourEncodingRAW } }, > > > > > > { MEDIA_BUS_FMT_SGRBG12_1X12, { 12, "SGRBG12_1X12", > PixelFormatInfo::ColourEncodingRAW } }, > > > > > > { MEDIA_BUS_FMT_SRGGB12_1X12, { 12, "SRGGB12_1X12", > PixelFormatInfo::ColourEncodingRAW } }, > > > > > > + { MEDIA_BUS_FMT_SBGGR16_1X16, { 16, "SBGGR16_1x16", > PixelFormatInfo::ColourEncodingRAW } }, > > > > > > + { MEDIA_BUS_FMT_SGBRG16_1X16, { 16, "SGBRG16_1x16", > PixelFormatInfo::ColourEncodingRAW } }, > > > > > > + { MEDIA_BUS_FMT_SGRBG16_1X16, { 16, "SGRBG16_1x16", > PixelFormatInfo::ColourEncodingRAW } }, > > > > > > + { MEDIA_BUS_FMT_SRGGB16_1X16, { 16, "SRGGB16_1x16", > PixelFormatInfo::ColourEncodingRAW } }, > > > > > > /* \todo Clarify colour encoding for HSV formats */ > > > > > > { MEDIA_BUS_FMT_AHSV8888_1X32, { 32, "AHSV8888_1X32", > PixelFormatInfo::ColourEncodingRGB } }, > > > > > > { MEDIA_BUS_FMT_JPEG_1X8, { 8, "JPEG_1X8", > PixelFormatInfo::ColourEncodingYUV } }, > > > > > > -- > > > > > > 2.34.1 > > > > > > >
Hi Naush On Wed, Feb 28, 2024 at 05:19:47PM +0000, Naushir Patuck wrote: > On Wed, 28 Feb 2024, 5:16 pm Jacopo Mondi, <jacopo.mondi@ideasonboard.com> > wrote: > > > Hi Naush > > > > On Wed, Feb 28, 2024 at 04:45:54PM +0000, Naushir Patuck wrote: > > > Hi Jacopo, > > > > > > > > > > - > > > > > > > + { formats::BGGR16_PISP_COMP1, { > > > > > > > + .name = "BGGR16_PISP_COMP1", > > > > > > > + .format = formats::BGGR16_PISP_COMP1, > > > > > > > + .v4l2Formats = { > > V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_BGGR), }, > > > > > > > + .bitsPerPixel = 16, > > > > > > > + .colourEncoding = > > PixelFormatInfo::ColourEncodingRAW, > > > > > > > + .packed = true, > > > > > > > + .pixelsPerGroup = 2, > > > > > > > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, > > > > > > > + } }, > > > > > > > > > > > > For regular 16-bit formats this should be > > > > > > > > > > > > .pixelsPerGroup = 2, > > > > > > .planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }}, > > > > > > > > > > > > As 2 pixels of 16 bits each consume 4 bytes. Is this {2, 1} > > > > > > intentional and due to compression ? > > > > > > > > > > Yes, I used {2, 1} to denote the 8-bit/sample after compression. Do > > > > > you think I should change to {4, 1}? > > > > > > > > Depends on how many bytes the format actually consumes :) > > > > > > > > For 16bpp RAW formats, we have pixelsPerGroup = 2 and each sample > > > > consumes 2 bytes. > > > > > > > > According to the pixelsPerGroup definition: > > > > > > > > * A pixel group is defined as the minimum number of pixels (including > > padding) > > > > * necessary in a row when the image has only one column of effective > > pixels. > > > > > > > > So for a RAW Bayer formats, to get a complete full-color "superpixel" > > > > we need at least two pixels per row. > > > > > > > > RG > > > > GB > > > > > > > > Each "RG" or "GB" pair is composed by 2 16-bit samples, for a total of > > > > 32bits of data, from which the {4, 1} value in planes[0] > > > > > > > > According to "Table 2", Chapter 5 of the PiSP datasheet, the > > > > compressed formats are described as > > > > > > > > 8-bits per pixel single channel compressed with mode n, for n = > > 1, 2, 3 > > > > > > > > Which suggests each "RG" or "GB" pair is actually 2 samples of 8 bits > > > > each, for a plane size of {2, 1}. > > > > > > > > Is this correct ? Is the actual .bitsPerPixel value for the compressed > > > > formats 8 instead of 16 then ? > > > > > > This is correct, each sample is 8-bits so "RG"/"GB" pairs are 16-bits > > each. > > > So it sounds like I need to use .planes = {2, 1}, but .bitsPerPixel = 8 > > maybe? > > > > I now wonder if the '16' in the format names is correct then :) > > > > It's meant to signify 16-bits compressed to 8-bits (in this case). Maybe a > defined naming scheme is needed for such cases? > What about simply leaving the bitdpeth out from the format name like it is done for the V4L2 pixel formats ? #define V4L2_PIX_FMT_PISP_COMP1_RGGB v4l2_fourcc('P', 'C', '1', 'R') /* PiSP 8-bit mode 1 compressed RGGB bayer */ Something like { formats::BGGR_PISP_COMP1, { .name = "BGGR_PISP_COMP1", .format = formats::BGGR_PISP_COMP1, .v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_BGGR), }, .bitsPerPixel = 8, .colourEncoding = PixelFormatInfo::ColourEncodingRAW, .packed = true, .pixelsPerGroup = 2, .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, } }, ? > > > > > > > Naush > > > > > > > > > > > Thanks > > > > j > > > > > > > > > > > > > > Regards, > > > > > Naush > > > > > > > > > > > > > > > > > > > > > > > + { formats::GBRG16_PISP_COMP1, { > > > > > > > + .name = "GBRG16_PISP_COMP1", > > > > > > > + .format = formats::GBRG16_PISP_COMP1, > > > > > > > + .v4l2Formats = { > > V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GBRG), }, > > > > > > > + .bitsPerPixel = 16, > > > > > > > + .colourEncoding = > > PixelFormatInfo::ColourEncodingRAW, > > > > > > > + .packed = true, > > > > > > > + .pixelsPerGroup = 2, > > > > > > > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, > > > > > > > + } }, > > > > > > > + { formats::GRBG16_PISP_COMP1, { > > > > > > > + .name = "GRBG16_PISP_COMP1", > > > > > > > + .format = formats::GRBG16_PISP_COMP1, > > > > > > > + .v4l2Formats = { > > V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GRBG), }, > > > > > > > + .bitsPerPixel = 16, > > > > > > > + .colourEncoding = > > PixelFormatInfo::ColourEncodingRAW, > > > > > > > + .packed = true, > > > > > > > + .pixelsPerGroup = 2, > > > > > > > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, > > > > > > > + } }, > > > > > > > + { formats::RGGB16_PISP_COMP1, { > > > > > > > + .name = "RGGB16_PISP_COMP1", > > > > > > > + .format = formats::RGGB16_PISP_COMP1, > > > > > > > + .v4l2Formats = { > > V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_RGGB), }, > > > > > > > + .bitsPerPixel = 16, > > > > > > > + .colourEncoding = > > PixelFormatInfo::ColourEncodingRAW, > > > > > > > + .packed = true, > > > > > > > + .pixelsPerGroup = 2, > > > > > > > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, > > > > > > > + } }, > > > > > > > /* Compressed formats. */ > > > > > > > { formats::MJPEG, { > > > > > > > .name = "MJPEG", > > > > > > > diff --git a/src/libcamera/formats.yaml > > b/src/libcamera/formats.yaml > > > > > > > index bde2cc803b98..f6df721243d0 100644 > > > > > > > --- a/src/libcamera/formats.yaml > > > > > > > +++ b/src/libcamera/formats.yaml > > > > > > > @@ -190,4 +190,20 @@ formats: > > > > > > > - SBGGR10_IPU3: > > > > > > > fourcc: DRM_FORMAT_SBGGR10 > > > > > > > mod: IPU3_FORMAT_MOD_PACKED > > > > > > > + > > > > > > > + - RGGB16_PISP_COMP1: > > > > > > > + fourcc: DRM_FORMAT_SRGGB16 > > > > > > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1 > > > > > > > + - GRBG16_PISP_COMP1: > > > > > > > + fourcc: DRM_FORMAT_SGRBG16 > > > > > > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1 > > > > > > > + - GBRG16_PISP_COMP1: > > > > > > > + fourcc: DRM_FORMAT_SGBRG16 > > > > > > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1 > > > > > > > + - BGGR16_PISP_COMP1: > > > > > > > + fourcc: DRM_FORMAT_SBGGR16 > > > > > > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1 > > > > > > > + - MONO_PISP_COMP1: > > > > > > > + fourcc: DRM_FORMAT_R16 > > > > > > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1 > > > > > > > ... > > > > > > > diff --git a/src/libcamera/v4l2_pixelformat.cpp > > b/src/libcamera/v4l2_pixelformat.cpp > > > > > > > index efb6f2940235..47baaf60199d 100644 > > > > > > > --- a/src/libcamera/v4l2_pixelformat.cpp > > > > > > > +++ b/src/libcamera/v4l2_pixelformat.cpp > > > > > > > @@ -207,6 +207,16 @@ const std::map<V4L2PixelFormat, > > V4L2PixelFormat::Info> vpf2pf{ > > > > > > > { formats::SGRBG16, "16-bit Bayer GRGR/BGBG" } }, > > > > > > > { V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16), > > > > > > > { formats::SRGGB16, "16-bit Bayer RGRG/GBGB" } }, > > > > > > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_BGGR), > > > > > > > + { formats::BGGR16_PISP_COMP1, "16-bit Bayer > > BGBG/GRGR PiSP Compress Mode 1" } }, > > > > > > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GBRG), > > > > > > > + { formats::GBRG16_PISP_COMP1, "16-bit Bayer > > GBGB/RGRG PiSP Compress Mode 1" } }, > > > > > > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GRBG), > > > > > > > + { formats::GRBG16_PISP_COMP1, "16-bit Bayer > > GRGR/BGBG PiSP Compress Mode 1" } }, > > > > > > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_RGGB), > > > > > > > + { formats::RGGB16_PISP_COMP1, "16-bit Bayer > > RGRG/GBGB PiSP Compress Mode 1" } }, > > > > > > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_MONO), > > > > > > > + { formats::MONO_PISP_COMP1, "16-bit Mono PiSP > > Compress Mode 1" } }, > > > > > > > > > > > > > > /* Compressed formats. */ > > > > > > > { V4L2PixelFormat(V4L2_PIX_FMT_MJPEG), > > > > > > > diff --git a/src/libcamera/v4l2_subdevice.cpp > > b/src/libcamera/v4l2_subdevice.cpp > > > > > > > index 6d0785b7b484..aea90abaf9ef 100644 > > > > > > > --- a/src/libcamera/v4l2_subdevice.cpp > > > > > > > +++ b/src/libcamera/v4l2_subdevice.cpp > > > > > > > @@ -134,6 +134,10 @@ const std::map<uint32_t, > > V4L2SubdeviceFormatInfo> formatInfoMap = { > > > > > > > { MEDIA_BUS_FMT_SGBRG12_1X12, { 12, "SGBRG12_1X12", > > PixelFormatInfo::ColourEncodingRAW } }, > > > > > > > { MEDIA_BUS_FMT_SGRBG12_1X12, { 12, "SGRBG12_1X12", > > PixelFormatInfo::ColourEncodingRAW } }, > > > > > > > { MEDIA_BUS_FMT_SRGGB12_1X12, { 12, "SRGGB12_1X12", > > PixelFormatInfo::ColourEncodingRAW } }, > > > > > > > + { MEDIA_BUS_FMT_SBGGR16_1X16, { 16, "SBGGR16_1x16", > > PixelFormatInfo::ColourEncodingRAW } }, > > > > > > > + { MEDIA_BUS_FMT_SGBRG16_1X16, { 16, "SGBRG16_1x16", > > PixelFormatInfo::ColourEncodingRAW } }, > > > > > > > + { MEDIA_BUS_FMT_SGRBG16_1X16, { 16, "SGRBG16_1x16", > > PixelFormatInfo::ColourEncodingRAW } }, > > > > > > > + { MEDIA_BUS_FMT_SRGGB16_1X16, { 16, "SRGGB16_1x16", > > PixelFormatInfo::ColourEncodingRAW } }, > > > > > > > /* \todo Clarify colour encoding for HSV formats */ > > > > > > > { MEDIA_BUS_FMT_AHSV8888_1X32, { 32, "AHSV8888_1X32", > > PixelFormatInfo::ColourEncodingRGB } }, > > > > > > > { MEDIA_BUS_FMT_JPEG_1X8, { 8, "JPEG_1X8", > > PixelFormatInfo::ColourEncodingYUV } }, > > > > > > > -- > > > > > > > 2.34.1 > > > > > > > > >
Hey Jacopo, On Thu, 29 Feb 2024 at 07:40, Jacopo Mondi <jacopo.mondi@ideasonboard.com> wrote: > > Hi Naush > > On Wed, Feb 28, 2024 at 05:19:47PM +0000, Naushir Patuck wrote: > > On Wed, 28 Feb 2024, 5:16 pm Jacopo Mondi, <jacopo.mondi@ideasonboard.com> > > wrote: > > > > > Hi Naush > > > > > > On Wed, Feb 28, 2024 at 04:45:54PM +0000, Naushir Patuck wrote: > > > > Hi Jacopo, > > > > > > > > > > > > - > > > > > > > > + { formats::BGGR16_PISP_COMP1, { > > > > > > > > + .name = "BGGR16_PISP_COMP1", > > > > > > > > + .format = formats::BGGR16_PISP_COMP1, > > > > > > > > + .v4l2Formats = { > > > V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_BGGR), }, > > > > > > > > + .bitsPerPixel = 16, > > > > > > > > + .colourEncoding = > > > PixelFormatInfo::ColourEncodingRAW, > > > > > > > > + .packed = true, > > > > > > > > + .pixelsPerGroup = 2, > > > > > > > > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, > > > > > > > > + } }, > > > > > > > > > > > > > > For regular 16-bit formats this should be > > > > > > > > > > > > > > .pixelsPerGroup = 2, > > > > > > > .planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }}, > > > > > > > > > > > > > > As 2 pixels of 16 bits each consume 4 bytes. Is this {2, 1} > > > > > > > intentional and due to compression ? > > > > > > > > > > > > Yes, I used {2, 1} to denote the 8-bit/sample after compression. Do > > > > > > you think I should change to {4, 1}? > > > > > > > > > > Depends on how many bytes the format actually consumes :) > > > > > > > > > > For 16bpp RAW formats, we have pixelsPerGroup = 2 and each sample > > > > > consumes 2 bytes. > > > > > > > > > > According to the pixelsPerGroup definition: > > > > > > > > > > * A pixel group is defined as the minimum number of pixels (including > > > padding) > > > > > * necessary in a row when the image has only one column of effective > > > pixels. > > > > > > > > > > So for a RAW Bayer formats, to get a complete full-color "superpixel" > > > > > we need at least two pixels per row. > > > > > > > > > > RG > > > > > GB > > > > > > > > > > Each "RG" or "GB" pair is composed by 2 16-bit samples, for a total of > > > > > 32bits of data, from which the {4, 1} value in planes[0] > > > > > > > > > > According to "Table 2", Chapter 5 of the PiSP datasheet, the > > > > > compressed formats are described as > > > > > > > > > > 8-bits per pixel single channel compressed with mode n, for n = > > > 1, 2, 3 > > > > > > > > > > Which suggests each "RG" or "GB" pair is actually 2 samples of 8 bits > > > > > each, for a plane size of {2, 1}. > > > > > > > > > > Is this correct ? Is the actual .bitsPerPixel value for the compressed > > > > > formats 8 instead of 16 then ? > > > > > > > > This is correct, each sample is 8-bits so "RG"/"GB" pairs are 16-bits > > > each. > > > > So it sounds like I need to use .planes = {2, 1}, but .bitsPerPixel = 8 > > > maybe? > > > > > > I now wonder if the '16' in the format names is correct then :) > > > > > > > It's meant to signify 16-bits compressed to 8-bits (in this case). Maybe a > > defined naming scheme is needed for such cases? > > > > What about simply leaving the bitdpeth out from the format name like > it is done for the V4L2 pixel formats ? That sounds good to me! I'll make the change in the next patch. > > #define V4L2_PIX_FMT_PISP_COMP1_RGGB v4l2_fourcc('P', 'C', '1', 'R') /* PiSP 8-bit mode 1 compressed RGGB bayer */ > > Something like > > { formats::BGGR_PISP_COMP1, { > .name = "BGGR_PISP_COMP1", > .format = formats::BGGR_PISP_COMP1, > .v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_BGGR), }, > .bitsPerPixel = 8, > .colourEncoding = PixelFormatInfo::ColourEncodingRAW, > .packed = true, > .pixelsPerGroup = 2, > .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, > } }, > > ? > > > > > > > > > > > Naush > > > > > > > > > > > > > > Thanks > > > > > j > > > > > > > > > > > > > > > > > Regards, > > > > > > Naush > > > > > > > > > > > > > > > > > > > > > > > > > > > + { formats::GBRG16_PISP_COMP1, { > > > > > > > > + .name = "GBRG16_PISP_COMP1", > > > > > > > > + .format = formats::GBRG16_PISP_COMP1, > > > > > > > > + .v4l2Formats = { > > > V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GBRG), }, > > > > > > > > + .bitsPerPixel = 16, > > > > > > > > + .colourEncoding = > > > PixelFormatInfo::ColourEncodingRAW, > > > > > > > > + .packed = true, > > > > > > > > + .pixelsPerGroup = 2, > > > > > > > > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, > > > > > > > > + } }, > > > > > > > > + { formats::GRBG16_PISP_COMP1, { > > > > > > > > + .name = "GRBG16_PISP_COMP1", > > > > > > > > + .format = formats::GRBG16_PISP_COMP1, > > > > > > > > + .v4l2Formats = { > > > V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GRBG), }, > > > > > > > > + .bitsPerPixel = 16, > > > > > > > > + .colourEncoding = > > > PixelFormatInfo::ColourEncodingRAW, > > > > > > > > + .packed = true, > > > > > > > > + .pixelsPerGroup = 2, > > > > > > > > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, > > > > > > > > + } }, > > > > > > > > + { formats::RGGB16_PISP_COMP1, { > > > > > > > > + .name = "RGGB16_PISP_COMP1", > > > > > > > > + .format = formats::RGGB16_PISP_COMP1, > > > > > > > > + .v4l2Formats = { > > > V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_RGGB), }, > > > > > > > > + .bitsPerPixel = 16, > > > > > > > > + .colourEncoding = > > > PixelFormatInfo::ColourEncodingRAW, > > > > > > > > + .packed = true, > > > > > > > > + .pixelsPerGroup = 2, > > > > > > > > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, > > > > > > > > + } }, > > > > > > > > /* Compressed formats. */ > > > > > > > > { formats::MJPEG, { > > > > > > > > .name = "MJPEG", > > > > > > > > diff --git a/src/libcamera/formats.yaml > > > b/src/libcamera/formats.yaml > > > > > > > > index bde2cc803b98..f6df721243d0 100644 > > > > > > > > --- a/src/libcamera/formats.yaml > > > > > > > > +++ b/src/libcamera/formats.yaml > > > > > > > > @@ -190,4 +190,20 @@ formats: > > > > > > > > - SBGGR10_IPU3: > > > > > > > > fourcc: DRM_FORMAT_SBGGR10 > > > > > > > > mod: IPU3_FORMAT_MOD_PACKED > > > > > > > > + > > > > > > > > + - RGGB16_PISP_COMP1: > > > > > > > > + fourcc: DRM_FORMAT_SRGGB16 > > > > > > > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1 > > > > > > > > + - GRBG16_PISP_COMP1: > > > > > > > > + fourcc: DRM_FORMAT_SGRBG16 > > > > > > > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1 > > > > > > > > + - GBRG16_PISP_COMP1: > > > > > > > > + fourcc: DRM_FORMAT_SGBRG16 > > > > > > > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1 > > > > > > > > + - BGGR16_PISP_COMP1: > > > > > > > > + fourcc: DRM_FORMAT_SBGGR16 > > > > > > > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1 > > > > > > > > + - MONO_PISP_COMP1: > > > > > > > > + fourcc: DRM_FORMAT_R16 > > > > > > > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1 > > > > > > > > ... > > > > > > > > diff --git a/src/libcamera/v4l2_pixelformat.cpp > > > b/src/libcamera/v4l2_pixelformat.cpp > > > > > > > > index efb6f2940235..47baaf60199d 100644 > > > > > > > > --- a/src/libcamera/v4l2_pixelformat.cpp > > > > > > > > +++ b/src/libcamera/v4l2_pixelformat.cpp > > > > > > > > @@ -207,6 +207,16 @@ const std::map<V4L2PixelFormat, > > > V4L2PixelFormat::Info> vpf2pf{ > > > > > > > > { formats::SGRBG16, "16-bit Bayer GRGR/BGBG" } }, > > > > > > > > { V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16), > > > > > > > > { formats::SRGGB16, "16-bit Bayer RGRG/GBGB" } }, > > > > > > > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_BGGR), > > > > > > > > + { formats::BGGR16_PISP_COMP1, "16-bit Bayer > > > BGBG/GRGR PiSP Compress Mode 1" } }, > > > > > > > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GBRG), > > > > > > > > + { formats::GBRG16_PISP_COMP1, "16-bit Bayer > > > GBGB/RGRG PiSP Compress Mode 1" } }, > > > > > > > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GRBG), > > > > > > > > + { formats::GRBG16_PISP_COMP1, "16-bit Bayer > > > GRGR/BGBG PiSP Compress Mode 1" } }, > > > > > > > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_RGGB), > > > > > > > > + { formats::RGGB16_PISP_COMP1, "16-bit Bayer > > > RGRG/GBGB PiSP Compress Mode 1" } }, > > > > > > > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_MONO), > > > > > > > > + { formats::MONO_PISP_COMP1, "16-bit Mono PiSP > > > Compress Mode 1" } }, > > > > > > > > > > > > > > > > /* Compressed formats. */ > > > > > > > > { V4L2PixelFormat(V4L2_PIX_FMT_MJPEG), > > > > > > > > diff --git a/src/libcamera/v4l2_subdevice.cpp > > > b/src/libcamera/v4l2_subdevice.cpp > > > > > > > > index 6d0785b7b484..aea90abaf9ef 100644 > > > > > > > > --- a/src/libcamera/v4l2_subdevice.cpp > > > > > > > > +++ b/src/libcamera/v4l2_subdevice.cpp > > > > > > > > @@ -134,6 +134,10 @@ const std::map<uint32_t, > > > V4L2SubdeviceFormatInfo> formatInfoMap = { > > > > > > > > { MEDIA_BUS_FMT_SGBRG12_1X12, { 12, "SGBRG12_1X12", > > > PixelFormatInfo::ColourEncodingRAW } }, > > > > > > > > { MEDIA_BUS_FMT_SGRBG12_1X12, { 12, "SGRBG12_1X12", > > > PixelFormatInfo::ColourEncodingRAW } }, > > > > > > > > { MEDIA_BUS_FMT_SRGGB12_1X12, { 12, "SRGGB12_1X12", > > > PixelFormatInfo::ColourEncodingRAW } }, > > > > > > > > + { MEDIA_BUS_FMT_SBGGR16_1X16, { 16, "SBGGR16_1x16", > > > PixelFormatInfo::ColourEncodingRAW } }, > > > > > > > > + { MEDIA_BUS_FMT_SGBRG16_1X16, { 16, "SGBRG16_1x16", > > > PixelFormatInfo::ColourEncodingRAW } }, > > > > > > > > + { MEDIA_BUS_FMT_SGRBG16_1X16, { 16, "SGRBG16_1x16", > > > PixelFormatInfo::ColourEncodingRAW } }, > > > > > > > > + { MEDIA_BUS_FMT_SRGGB16_1X16, { 16, "SRGGB16_1x16", > > > PixelFormatInfo::ColourEncodingRAW } }, > > > > > > > > /* \todo Clarify colour encoding for HSV formats */ > > > > > > > > { MEDIA_BUS_FMT_AHSV8888_1X32, { 32, "AHSV8888_1X32", > > > PixelFormatInfo::ColourEncodingRGB } }, > > > > > > > > { MEDIA_BUS_FMT_JPEG_1X8, { 8, "JPEG_1X8", > > > PixelFormatInfo::ColourEncodingYUV } }, > > > > > > > > -- > > > > > > > > 2.34.1 > > > > > > > > > > >
diff --git a/include/libcamera/internal/bayer_format.h b/include/libcamera/internal/bayer_format.h index 78ba3969913d..164743f7e9f6 100644 --- a/include/libcamera/internal/bayer_format.h +++ b/include/libcamera/internal/bayer_format.h @@ -34,6 +34,8 @@ public: None = 0, CSI2 = 1, IPU3 = 2, + PISP1 = 3, + PISP2 = 4, }; constexpr BayerFormat() diff --git a/include/linux/drm_fourcc.h b/include/linux/drm_fourcc.h index 4ee421b95730..eff27fbe5a1e 100644 --- a/include/linux/drm_fourcc.h +++ b/include/linux/drm_fourcc.h @@ -490,6 +490,7 @@ extern "C" { #define DRM_FORMAT_MOD_VENDOR_ALLWINNER 0x09 #define DRM_FORMAT_MOD_VENDOR_AMLOGIC 0x0a #define DRM_FORMAT_MOD_VENDOR_MIPI 0x0b +#define DRM_FORMAT_MOD_VENDOR_RPI 0x0c /* add more to the end as needed */ @@ -1670,6 +1671,9 @@ drm_fourcc_canonicalize_nvidia_format_mod(__u64 modifier) */ #define MIPI_FORMAT_MOD_CSI2_PACKED fourcc_mod_code(MIPI, 1) +#define PISP_FORMAT_MOD_COMPRESS_MODE1 fourcc_mod_code(RPI, 1) +#define PISP_FORMAT_MOD_COMPRESS_MODE2 fourcc_mod_code(RPI, 2) + #if defined(__cplusplus) } #endif diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h index ba48d2c89726..59af6f794680 100644 --- a/include/linux/videodev2.h +++ b/include/linux/videodev2.h @@ -789,6 +789,18 @@ struct v4l2_pix_format { #define V4L2_PIX_FMT_IPU3_SGRBG10 v4l2_fourcc('i', 'p', '3', 'G') /* IPU3 packed 10-bit GRBG bayer */ #define V4L2_PIX_FMT_IPU3_SRGGB10 v4l2_fourcc('i', 'p', '3', 'r') /* IPU3 packed 10-bit RGGB bayer */ +/* The pixel format for all our buffers (the precise format is found in the config buffer). */ +#define V4L2_PIX_FMT_PISP_COMP1_RGGB v4l2_fourcc('P', 'C', '1', 'R') +#define V4L2_PIX_FMT_PISP_COMP1_GRBG v4l2_fourcc('P', 'C', '1', 'G') +#define V4L2_PIX_FMT_PISP_COMP1_GBRG v4l2_fourcc('P', 'C', '1', 'g') +#define V4L2_PIX_FMT_PISP_COMP1_BGGR v4l2_fourcc('P', 'C', '1', 'B') +#define V4L2_PIX_FMT_PISP_COMP1_MONO v4l2_fourcc('P', 'C', '1', 'M') +#define V4L2_PIX_FMT_PISP_COMP2_RGGB v4l2_fourcc('P', 'C', '2', 'R') +#define V4L2_PIX_FMT_PISP_COMP2_GRBG v4l2_fourcc('P', 'C', '2', 'G') +#define V4L2_PIX_FMT_PISP_COMP2_GBRG v4l2_fourcc('P', 'C', '2', 'g') +#define V4L2_PIX_FMT_PISP_COMP2_BGGR v4l2_fourcc('P', 'C', '2', 'B') +#define V4L2_PIX_FMT_PISP_COMP2_MONO v4l2_fourcc('P', 'C', '2', 'M') + /* SDR formats - used only for Software Defined Radio devices */ #define V4L2_SDR_FMT_CU8 v4l2_fourcc('C', 'U', '0', '8') /* IQ u8 */ #define V4L2_SDR_FMT_CU16LE v4l2_fourcc('C', 'U', '1', '6') /* IQ u16le */ @@ -818,6 +830,9 @@ struct v4l2_pix_format { #define V4L2_META_FMT_RK_ISP1_PARAMS v4l2_fourcc('R', 'K', '1', 'P') /* Rockchip ISP1 3A Parameters */ #define V4L2_META_FMT_RK_ISP1_STAT_3A v4l2_fourcc('R', 'K', '1', 'S') /* Rockchip ISP1 3A Statistics */ +/* Vendor specific - used for RaspberryPi PiSP */ +#define V4L2_META_FMT_RPI_BE_CFG v4l2_fourcc('R', 'P', 'B', 'C') /* PiSP BE configuration */ + /* priv field value to indicates that subsequent fields are valid. */ #define V4L2_PIX_FMT_PRIV_MAGIC 0xfeedcafe diff --git a/src/libcamera/bayer_format.cpp b/src/libcamera/bayer_format.cpp index 20aedfa6d925..333b1117f531 100644 --- a/src/libcamera/bayer_format.cpp +++ b/src/libcamera/bayer_format.cpp @@ -164,6 +164,14 @@ const std::map<BayerFormat, Formats, BayerFormatComparator> bayerToFormat{ { formats::SGRBG16, V4L2PixelFormat(V4L2_PIX_FMT_SGRBG16) } }, { { BayerFormat::RGGB, 16, BayerFormat::Packing::None }, { formats::SRGGB16, V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16) } }, + { { BayerFormat::BGGR, 16, BayerFormat::Packing::PISP1 }, + { formats::BGGR16_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_BGGR) } }, + { { BayerFormat::GBRG, 16, BayerFormat::Packing::PISP1 }, + { formats::GBRG16_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GBRG) } }, + { { BayerFormat::GRBG, 16, BayerFormat::Packing::PISP1 }, + { formats::GRBG16_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GRBG) } }, + { { BayerFormat::RGGB, 16, BayerFormat::Packing::PISP1 }, + { formats::RGGB16_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_RGGB) } }, { { BayerFormat::MONO, 8, BayerFormat::Packing::None }, { formats::R8, V4L2PixelFormat(V4L2_PIX_FMT_GREY) } }, { { BayerFormat::MONO, 10, BayerFormat::Packing::None }, @@ -174,6 +182,8 @@ const std::map<BayerFormat, Formats, BayerFormatComparator> bayerToFormat{ { formats::R12, V4L2PixelFormat(V4L2_PIX_FMT_Y12) } }, { { BayerFormat::MONO, 16, BayerFormat::Packing::None }, { formats::R16, V4L2PixelFormat(V4L2_PIX_FMT_Y16) } }, + { { BayerFormat::MONO, 16, BayerFormat::Packing::PISP1 }, + { formats::MONO_PISP_COMP1, V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_MONO) } }, }; const std::unordered_map<unsigned int, BayerFormat> mbusCodeToBayer{ @@ -209,6 +219,10 @@ const std::unordered_map<unsigned int, BayerFormat> mbusCodeToBayer{ { MEDIA_BUS_FMT_SGBRG16_1X16, { BayerFormat::GBRG, 16, BayerFormat::Packing::None } }, { MEDIA_BUS_FMT_SGRBG16_1X16, { BayerFormat::GRBG, 16, BayerFormat::Packing::None } }, { MEDIA_BUS_FMT_SRGGB16_1X16, { BayerFormat::RGGB, 16, BayerFormat::Packing::None } }, + { MEDIA_BUS_FMT_SBGGR16_1X16, { BayerFormat::BGGR, 16, BayerFormat::Packing::PISP1 } }, + { MEDIA_BUS_FMT_SGBRG16_1X16, { BayerFormat::GBRG, 16, BayerFormat::Packing::PISP1 } }, + { MEDIA_BUS_FMT_SGRBG16_1X16, { BayerFormat::GRBG, 16, BayerFormat::Packing::PISP1 } }, + { MEDIA_BUS_FMT_SRGGB16_1X16, { BayerFormat::RGGB, 16, BayerFormat::Packing::PISP1 } }, { MEDIA_BUS_FMT_Y8_1X8, { BayerFormat::MONO, 8, BayerFormat::Packing::None } }, { MEDIA_BUS_FMT_Y10_1X10, { BayerFormat::MONO, 10, BayerFormat::Packing::None } }, { MEDIA_BUS_FMT_Y12_1X12, { BayerFormat::MONO, 12, BayerFormat::Packing::None } }, @@ -303,6 +317,10 @@ std::ostream &operator<<(std::ostream &out, const BayerFormat &f) out << "-CSI2P"; else if (f.packing == BayerFormat::Packing::IPU3) out << "-IPU3P"; + else if (f.packing == BayerFormat::Packing::PISP1) + out << "-PISP1"; + else if (f.packing == BayerFormat::Packing::PISP2) + out << "-PISP2"; return out; } diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp index a674f4179cc8..e603a7eda579 100644 --- a/src/libcamera/formats.cpp +++ b/src/libcamera/formats.cpp @@ -547,6 +547,16 @@ const std::map<PixelFormat, PixelFormatInfo> pixelFormatInfo{ .pixelsPerGroup = 1, .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, } }, + { formats::MONO_PISP_COMP1, { + .name = "MONO_PISP_COMP1", + .format = formats::MONO_PISP_COMP1, + .v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_MONO), }, + .bitsPerPixel = 16, + .colourEncoding = PixelFormatInfo::ColourEncodingYUV, + .packed = true, + .pixelsPerGroup = 1, + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, + } }, /* Bayer formats. */ { formats::SBGGR8, { @@ -910,7 +920,46 @@ const std::map<PixelFormat, PixelFormatInfo> pixelFormatInfo{ .pixelsPerGroup = 25, .planes = {{ { 32, 1 }, { 0, 0 }, { 0, 0 } }}, } }, - + { formats::BGGR16_PISP_COMP1, { + .name = "BGGR16_PISP_COMP1", + .format = formats::BGGR16_PISP_COMP1, + .v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_BGGR), }, + .bitsPerPixel = 16, + .colourEncoding = PixelFormatInfo::ColourEncodingRAW, + .packed = true, + .pixelsPerGroup = 2, + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, + } }, + { formats::GBRG16_PISP_COMP1, { + .name = "GBRG16_PISP_COMP1", + .format = formats::GBRG16_PISP_COMP1, + .v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GBRG), }, + .bitsPerPixel = 16, + .colourEncoding = PixelFormatInfo::ColourEncodingRAW, + .packed = true, + .pixelsPerGroup = 2, + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, + } }, + { formats::GRBG16_PISP_COMP1, { + .name = "GRBG16_PISP_COMP1", + .format = formats::GRBG16_PISP_COMP1, + .v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GRBG), }, + .bitsPerPixel = 16, + .colourEncoding = PixelFormatInfo::ColourEncodingRAW, + .packed = true, + .pixelsPerGroup = 2, + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, + } }, + { formats::RGGB16_PISP_COMP1, { + .name = "RGGB16_PISP_COMP1", + .format = formats::RGGB16_PISP_COMP1, + .v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_RGGB), }, + .bitsPerPixel = 16, + .colourEncoding = PixelFormatInfo::ColourEncodingRAW, + .packed = true, + .pixelsPerGroup = 2, + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }}, + } }, /* Compressed formats. */ { formats::MJPEG, { .name = "MJPEG", diff --git a/src/libcamera/formats.yaml b/src/libcamera/formats.yaml index bde2cc803b98..f6df721243d0 100644 --- a/src/libcamera/formats.yaml +++ b/src/libcamera/formats.yaml @@ -190,4 +190,20 @@ formats: - SBGGR10_IPU3: fourcc: DRM_FORMAT_SBGGR10 mod: IPU3_FORMAT_MOD_PACKED + + - RGGB16_PISP_COMP1: + fourcc: DRM_FORMAT_SRGGB16 + mod: PISP_FORMAT_MOD_COMPRESS_MODE1 + - GRBG16_PISP_COMP1: + fourcc: DRM_FORMAT_SGRBG16 + mod: PISP_FORMAT_MOD_COMPRESS_MODE1 + - GBRG16_PISP_COMP1: + fourcc: DRM_FORMAT_SGBRG16 + mod: PISP_FORMAT_MOD_COMPRESS_MODE1 + - BGGR16_PISP_COMP1: + fourcc: DRM_FORMAT_SBGGR16 + mod: PISP_FORMAT_MOD_COMPRESS_MODE1 + - MONO_PISP_COMP1: + fourcc: DRM_FORMAT_R16 + mod: PISP_FORMAT_MOD_COMPRESS_MODE1 ... diff --git a/src/libcamera/v4l2_pixelformat.cpp b/src/libcamera/v4l2_pixelformat.cpp index efb6f2940235..47baaf60199d 100644 --- a/src/libcamera/v4l2_pixelformat.cpp +++ b/src/libcamera/v4l2_pixelformat.cpp @@ -207,6 +207,16 @@ const std::map<V4L2PixelFormat, V4L2PixelFormat::Info> vpf2pf{ { formats::SGRBG16, "16-bit Bayer GRGR/BGBG" } }, { V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16), { formats::SRGGB16, "16-bit Bayer RGRG/GBGB" } }, + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_BGGR), + { formats::BGGR16_PISP_COMP1, "16-bit Bayer BGBG/GRGR PiSP Compress Mode 1" } }, + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GBRG), + { formats::GBRG16_PISP_COMP1, "16-bit Bayer GBGB/RGRG PiSP Compress Mode 1" } }, + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GRBG), + { formats::GRBG16_PISP_COMP1, "16-bit Bayer GRGR/BGBG PiSP Compress Mode 1" } }, + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_RGGB), + { formats::RGGB16_PISP_COMP1, "16-bit Bayer RGRG/GBGB PiSP Compress Mode 1" } }, + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_MONO), + { formats::MONO_PISP_COMP1, "16-bit Mono PiSP Compress Mode 1" } }, /* Compressed formats. */ { V4L2PixelFormat(V4L2_PIX_FMT_MJPEG), diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp index 6d0785b7b484..aea90abaf9ef 100644 --- a/src/libcamera/v4l2_subdevice.cpp +++ b/src/libcamera/v4l2_subdevice.cpp @@ -134,6 +134,10 @@ const std::map<uint32_t, V4L2SubdeviceFormatInfo> formatInfoMap = { { MEDIA_BUS_FMT_SGBRG12_1X12, { 12, "SGBRG12_1X12", PixelFormatInfo::ColourEncodingRAW } }, { MEDIA_BUS_FMT_SGRBG12_1X12, { 12, "SGRBG12_1X12", PixelFormatInfo::ColourEncodingRAW } }, { MEDIA_BUS_FMT_SRGGB12_1X12, { 12, "SRGGB12_1X12", PixelFormatInfo::ColourEncodingRAW } }, + { MEDIA_BUS_FMT_SBGGR16_1X16, { 16, "SBGGR16_1x16", PixelFormatInfo::ColourEncodingRAW } }, + { MEDIA_BUS_FMT_SGBRG16_1X16, { 16, "SGBRG16_1x16", PixelFormatInfo::ColourEncodingRAW } }, + { MEDIA_BUS_FMT_SGRBG16_1X16, { 16, "SGRBG16_1x16", PixelFormatInfo::ColourEncodingRAW } }, + { MEDIA_BUS_FMT_SRGGB16_1X16, { 16, "SRGGB16_1x16", PixelFormatInfo::ColourEncodingRAW } }, /* \todo Clarify colour encoding for HSV formats */ { MEDIA_BUS_FMT_AHSV8888_1X32, { 32, "AHSV8888_1X32", PixelFormatInfo::ColourEncodingRGB } }, { MEDIA_BUS_FMT_JPEG_1X8, { 8, "JPEG_1X8", PixelFormatInfo::ColourEncodingYUV } },
Add the Raspberry Pi 5 PiSP specific compressed Bayer format types 1/2: - V4L2_PIX_FMT_PISP_COMP1_xxx - V4L2_PIX_FMT_PISP_COMP2_xxx Add the Raspberry Pi 5 PiSP Backend config format: - V4L2_META_FMT_RPI_BE_CFG Additionally, we also extend libcamera format handlers to support 16-bit Bayer formats across the media bus. Signed-off-by: Naushir Patuck <naush@raspberrypi.com> --- include/libcamera/internal/bayer_format.h | 2 + include/linux/drm_fourcc.h | 4 ++ include/linux/videodev2.h | 15 +++++++ src/libcamera/bayer_format.cpp | 18 ++++++++ src/libcamera/formats.cpp | 51 ++++++++++++++++++++++- src/libcamera/formats.yaml | 16 +++++++ src/libcamera/v4l2_pixelformat.cpp | 10 +++++ src/libcamera/v4l2_subdevice.cpp | 4 ++ 8 files changed, 119 insertions(+), 1 deletion(-)