| Message ID | 20240301212121.9072-1-laurent.pinchart@ideasonboard.com |
|---|---|
| Headers | show |
| Series |
|
| Related | show |
On Fri, Mar 01, 2024 at 11:20:49PM +0200, Laurent Pinchart wrote: > Hello everybody, > > This large(-ish) patch series is the first complete implementation of > support for the Unicam upstream kernel driver for libcamera. > > Or, it would be, if the Unicam driver was upstream. While I have also > completed a version of the Unicam driver for the upstream kernel, > there's a bit of a chicken-and-egg issue here as I would like to > cross-reference the cover letters. To break the loop, I'm posting this > series first, and will reply with a link to the Unicam patches ([1]). > > Support for Raspberry Pi 4 in libcamera currently relies on downstream > drivers that live in the Raspberry Pi kernel tree, for Unicam (the CSI-2 > receiver) and the ISP. The Unicam driver, in particular, includes a few > workarounds to implement support for sensor embedded data, to compensate > for features that are or were until recently missing in the V4L2 > in-kernel and userspace APIs. The new Unicam submission for the kernel > reworks the driver extensively to use the recently merged V4L2 streams > API, as well as the under development V4L2 generic metadata and internal > pads APIs ([2]). > > The series starts with nine patches that clean up, rework and improve > the V4L2Subdevice class (01/32 to 09/32). They have been posted to the > list before, and mostly reviewed. Patches 10/32 is where the fun starts, > as it pulls the APIs from [2] in the kernel headers. The next three > patches, 11/32 to 13/32, then update the V4L2Subdevice and > V4L2VideoDevice classes to support those APIs. > > The next ten patches rework the CameraSensor class to support sensor > drivers that use the new APIs. Patches 14/32 to to 21/32 contain various > reworks, including moving the camera sensor support to the new > src/libcamera/sensor/ directory, and turning CameraSensor into an > abstract base class with multiple implementations, using a factory > pattern to pick the right match. Patches 22/32 introduces a new > CameraSensor subclass for the new APIs, and patch 23/32 then extends the > CameraSensor interface to support embedded data. > > With all the building blocks in place, the next seven patches focus on > the Raspberry Pi VC4 pipeline handler to use the new APIs, when > interacting with the sensor (24/32), or with the mainline Unicam driver > (27/32 to 30/32). Once done, patch 31/32 drops the > V4L2_META_FMT_SENSOR_DATA format from the videodev2.h kernel header, as > the format is not used anymore and has never been present in the > upstream kernel. > > The last patch, 32/32, is a hack that unconditionally enables embedded > data for the IMX219 sensor, as that is the platform I've been using to > develop and test this series. > > Worthy of a note, this series breaks compatibility with the downstream > Raspberry Pi kernel. I have decided not to preserve backward > compatibiliy to ease development, and to make the new code easier to > review. We still need to discuss how to integrate the various components > (libcamera and kernel space) in a way that will not generate lots of > anger among users. This may lead to some temporary changes to the code > to help with the transition. > > For convenience, the patches can be found at [3]. > > [1] TBD due to lack of a time machine [1] https://lore.kernel.org/linux-media/20240301213231.10340-1-laurent.pinchart@ideasonboard.com > [2] https://lore.kernel.org/linux-media/20231106122539.1268265-1-sakari.ailus@linux.intel.com > [3] https://git.libcamera.org/libcamera/pinchartl/libcamera.git/log/?h=rpi/streams/next > > Jacopo Mondi (1): > libcamera: camera_sensor: Introduce CameraSensorFactory > > Laurent Pinchart (31): > libcamera: v4l2_subdevice: Rename V4L2SubdeviceFormatInfo > libcamera: v4l2_subdevice: Add code member to MediaBusFormatInfo > libcamera: v4l2_subdevice: Expose media bus format info as internal > API > libcamera: v4l2_subdevice: Extend MediaBusFormatInfo with metadata > formats > libcamera: v4l2_subdevice: Drop V4L2SubdeviceFormat::bitsPerPixel() > libcamera: v4l2_subdevice: Rename V4L2SubdeviceFormat::mbus_code to > code > libcamera: v4l2_subdevice: Add stream support to get/set functions > libcamera: v4l2_subdevice: Replace Routing::toString() with > operator<<() > libcamera: v4l2_subdevice: Add V4L2Subdevice::Route structure > [DNI] include: linux: Update kernel headers to metadata API > libcamera: v4l2_subdevice: Update to the new kernel routing API > libcamera: v4l2_subdevice: Add new metadata formats > libcamera: v4l2_videodevice: Update to the new kernel metadata API > libcamera: camera_sensor: Move related classes to subdirectory > libcamera: camera_sensor: Drop updateControlInfo() function > libcamera: camera_sensor: Reorder functions > libcamera: camera_sensor: Test for read-only HBLANK with READ_ONLY > flag > libcamera: camera_sensor: Expose the Bayer order > libcamera: camera_sensor: Create abstract base class > libcamera: camera_sensor: Sort factories by priority > libcamera: Add CameraSensor implementation for raw V4L2 sensors > libcamera: camera_sensor: Add support for embedded data > pipeline: raspberrypi: common: Configure sensor embedded data > pipeline: raspberrypi: vc4: Use operator<<(V4L2VideoFormat) > pipeline: raspberrypi: vc4: Reorganize platformConfigure() > pipeline: raspberrypi: vc4: Use the CameraSensor embedded data API > pipeline: raspberrypi: vc4: Unconditionally create embedded data > stream > pipeline: raspberrypi: vc4: Configure format on Unicam subdev > pipeline: raspberrypi: vc4: Fix configuration of the embedded data > node > include: linux: Drop V4L2_META_FMT_SENSOR_DATA > [HACK]: ipa: rpi: cam_helper_imx219: Enable embedded data > > Documentation/Doxyfile.in | 2 + > include/libcamera/internal/camera_sensor.h | 162 +-- > include/libcamera/internal/v4l2_subdevice.h | 103 +- > include/linux/README | 2 +- > include/linux/media-bus-format.h | 13 + > include/linux/media.h | 1 + > include/linux/v4l2-controls.h | 16 +- > include/linux/v4l2-mediabus.h | 18 +- > include/linux/v4l2-subdev.h | 23 +- > include/linux/videodev2.h | 30 +- > src/ipa/rpi/cam_helper/cam_helper_imx219.cpp | 2 +- > src/libcamera/meson.build | 3 +- > src/libcamera/pipeline/imx8-isi/imx8-isi.cpp | 19 +- > src/libcamera/pipeline/ipu3/cio2.cpp | 15 +- > src/libcamera/pipeline/ipu3/imgu.cpp | 4 +- > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 9 +- > src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 2 +- > .../pipeline/rpi/common/pipeline_base.cpp | 85 +- > .../pipeline/rpi/common/pipeline_base.h | 6 +- > src/libcamera/pipeline/rpi/vc4/vc4.cpp | 174 ++- > src/libcamera/pipeline/simple/simple.cpp | 29 +- > src/libcamera/pipeline/vimc/vimc.cpp | 11 +- > src/libcamera/sensor/camera_sensor.cpp | 557 ++++++++ > .../camera_sensor_legacy.cpp} | 717 ++++------ > .../{ => sensor}/camera_sensor_properties.cpp | 0 > src/libcamera/sensor/camera_sensor_raw.cpp | 1135 +++++++++++++++ > src/libcamera/sensor/meson.build | 8 + > src/libcamera/v4l2_subdevice.cpp | 1239 ++++++++++++++--- > src/libcamera/v4l2_videodevice.cpp | 31 +- > test/camera-sensor.cpp | 9 +- > .../v4l2_videodevice_test.cpp | 7 +- > test/v4l2_videodevice/v4l2_videodevice_test.h | 2 +- > 32 files changed, 3471 insertions(+), 963 deletions(-) > create mode 100644 src/libcamera/sensor/camera_sensor.cpp > rename src/libcamera/{camera_sensor.cpp => sensor/camera_sensor_legacy.cpp} (60%) > rename src/libcamera/{ => sensor}/camera_sensor_properties.cpp (100%) > create mode 100644 src/libcamera/sensor/camera_sensor_raw.cpp > create mode 100644 src/libcamera/sensor/meson.build > > > base-commit: c64446c226d4e629884d2f5b148a01969e8ee84a
Hi Laurent, Very exciting series! I've had a brief attempt at getting this running with the Pi 5 pipeline handler with Tomi's CFE driver changes. I do have it working with the IMX219 but run into some very minor issues. I'll comment on these in the relevant patches, but it is very possible (likey) that I may not have the latest IMX219 kernel code, so some of these may be redundant. Not a full review yet, that will come later :) but I thought I should note my findings before I forget them. Regards, Naush On Fri, 1 Mar 2024 at 21:21, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > > Hello everybody, > > This large(-ish) patch series is the first complete implementation of > support for the Unicam upstream kernel driver for libcamera. > > Or, it would be, if the Unicam driver was upstream. While I have also > completed a version of the Unicam driver for the upstream kernel, > there's a bit of a chicken-and-egg issue here as I would like to > cross-reference the cover letters. To break the loop, I'm posting this > series first, and will reply with a link to the Unicam patches ([1]). > > Support for Raspberry Pi 4 in libcamera currently relies on downstream > drivers that live in the Raspberry Pi kernel tree, for Unicam (the CSI-2 > receiver) and the ISP. The Unicam driver, in particular, includes a few > workarounds to implement support for sensor embedded data, to compensate > for features that are or were until recently missing in the V4L2 > in-kernel and userspace APIs. The new Unicam submission for the kernel > reworks the driver extensively to use the recently merged V4L2 streams > API, as well as the under development V4L2 generic metadata and internal > pads APIs ([2]). > > The series starts with nine patches that clean up, rework and improve > the V4L2Subdevice class (01/32 to 09/32). They have been posted to the > list before, and mostly reviewed. Patches 10/32 is where the fun starts, > as it pulls the APIs from [2] in the kernel headers. The next three > patches, 11/32 to 13/32, then update the V4L2Subdevice and > V4L2VideoDevice classes to support those APIs. > > The next ten patches rework the CameraSensor class to support sensor > drivers that use the new APIs. Patches 14/32 to to 21/32 contain various > reworks, including moving the camera sensor support to the new > src/libcamera/sensor/ directory, and turning CameraSensor into an > abstract base class with multiple implementations, using a factory > pattern to pick the right match. Patches 22/32 introduces a new > CameraSensor subclass for the new APIs, and patch 23/32 then extends the > CameraSensor interface to support embedded data. > > With all the building blocks in place, the next seven patches focus on > the Raspberry Pi VC4 pipeline handler to use the new APIs, when > interacting with the sensor (24/32), or with the mainline Unicam driver > (27/32 to 30/32). Once done, patch 31/32 drops the > V4L2_META_FMT_SENSOR_DATA format from the videodev2.h kernel header, as > the format is not used anymore and has never been present in the > upstream kernel. > > The last patch, 32/32, is a hack that unconditionally enables embedded > data for the IMX219 sensor, as that is the platform I've been using to > develop and test this series. > > Worthy of a note, this series breaks compatibility with the downstream > Raspberry Pi kernel. I have decided not to preserve backward > compatibiliy to ease development, and to make the new code easier to > review. We still need to discuss how to integrate the various components > (libcamera and kernel space) in a way that will not generate lots of > anger among users. This may lead to some temporary changes to the code > to help with the transition. > > For convenience, the patches can be found at [3]. > > [1] TBD due to lack of a time machine > [2] https://lore.kernel.org/linux-media/20231106122539.1268265-1-sakari.ailus@linux.intel.com > [3] https://git.libcamera.org/libcamera/pinchartl/libcamera.git/log/?h=rpi/streams/next > > Jacopo Mondi (1): > libcamera: camera_sensor: Introduce CameraSensorFactory > > Laurent Pinchart (31): > libcamera: v4l2_subdevice: Rename V4L2SubdeviceFormatInfo > libcamera: v4l2_subdevice: Add code member to MediaBusFormatInfo > libcamera: v4l2_subdevice: Expose media bus format info as internal > API > libcamera: v4l2_subdevice: Extend MediaBusFormatInfo with metadata > formats > libcamera: v4l2_subdevice: Drop V4L2SubdeviceFormat::bitsPerPixel() > libcamera: v4l2_subdevice: Rename V4L2SubdeviceFormat::mbus_code to > code > libcamera: v4l2_subdevice: Add stream support to get/set functions > libcamera: v4l2_subdevice: Replace Routing::toString() with > operator<<() > libcamera: v4l2_subdevice: Add V4L2Subdevice::Route structure > [DNI] include: linux: Update kernel headers to metadata API > libcamera: v4l2_subdevice: Update to the new kernel routing API > libcamera: v4l2_subdevice: Add new metadata formats > libcamera: v4l2_videodevice: Update to the new kernel metadata API > libcamera: camera_sensor: Move related classes to subdirectory > libcamera: camera_sensor: Drop updateControlInfo() function > libcamera: camera_sensor: Reorder functions > libcamera: camera_sensor: Test for read-only HBLANK with READ_ONLY > flag > libcamera: camera_sensor: Expose the Bayer order > libcamera: camera_sensor: Create abstract base class > libcamera: camera_sensor: Sort factories by priority > libcamera: Add CameraSensor implementation for raw V4L2 sensors > libcamera: camera_sensor: Add support for embedded data > pipeline: raspberrypi: common: Configure sensor embedded data > pipeline: raspberrypi: vc4: Use operator<<(V4L2VideoFormat) > pipeline: raspberrypi: vc4: Reorganize platformConfigure() > pipeline: raspberrypi: vc4: Use the CameraSensor embedded data API > pipeline: raspberrypi: vc4: Unconditionally create embedded data > stream > pipeline: raspberrypi: vc4: Configure format on Unicam subdev > pipeline: raspberrypi: vc4: Fix configuration of the embedded data > node > include: linux: Drop V4L2_META_FMT_SENSOR_DATA > [HACK]: ipa: rpi: cam_helper_imx219: Enable embedded data > > Documentation/Doxyfile.in | 2 + > include/libcamera/internal/camera_sensor.h | 162 +-- > include/libcamera/internal/v4l2_subdevice.h | 103 +- > include/linux/README | 2 +- > include/linux/media-bus-format.h | 13 + > include/linux/media.h | 1 + > include/linux/v4l2-controls.h | 16 +- > include/linux/v4l2-mediabus.h | 18 +- > include/linux/v4l2-subdev.h | 23 +- > include/linux/videodev2.h | 30 +- > src/ipa/rpi/cam_helper/cam_helper_imx219.cpp | 2 +- > src/libcamera/meson.build | 3 +- > src/libcamera/pipeline/imx8-isi/imx8-isi.cpp | 19 +- > src/libcamera/pipeline/ipu3/cio2.cpp | 15 +- > src/libcamera/pipeline/ipu3/imgu.cpp | 4 +- > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 9 +- > src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 2 +- > .../pipeline/rpi/common/pipeline_base.cpp | 85 +- > .../pipeline/rpi/common/pipeline_base.h | 6 +- > src/libcamera/pipeline/rpi/vc4/vc4.cpp | 174 ++- > src/libcamera/pipeline/simple/simple.cpp | 29 +- > src/libcamera/pipeline/vimc/vimc.cpp | 11 +- > src/libcamera/sensor/camera_sensor.cpp | 557 ++++++++ > .../camera_sensor_legacy.cpp} | 717 ++++------ > .../{ => sensor}/camera_sensor_properties.cpp | 0 > src/libcamera/sensor/camera_sensor_raw.cpp | 1135 +++++++++++++++ > src/libcamera/sensor/meson.build | 8 + > src/libcamera/v4l2_subdevice.cpp | 1239 ++++++++++++++--- > src/libcamera/v4l2_videodevice.cpp | 31 +- > test/camera-sensor.cpp | 9 +- > .../v4l2_videodevice_test.cpp | 7 +- > test/v4l2_videodevice/v4l2_videodevice_test.h | 2 +- > 32 files changed, 3471 insertions(+), 963 deletions(-) > create mode 100644 src/libcamera/sensor/camera_sensor.cpp > rename src/libcamera/{camera_sensor.cpp => sensor/camera_sensor_legacy.cpp} (60%) > rename src/libcamera/{ => sensor}/camera_sensor_properties.cpp (100%) > create mode 100644 src/libcamera/sensor/camera_sensor_raw.cpp > create mode 100644 src/libcamera/sensor/meson.build > > > base-commit: c64446c226d4e629884d2f5b148a01969e8ee84a > -- > Regards, > > Laurent Pinchart >
Hello everybody, This large(-ish) patch series is the first complete implementation of support for the Unicam upstream kernel driver for libcamera. Or, it would be, if the Unicam driver was upstream. While I have also completed a version of the Unicam driver for the upstream kernel, there's a bit of a chicken-and-egg issue here as I would like to cross-reference the cover letters. To break the loop, I'm posting this series first, and will reply with a link to the Unicam patches ([1]). Support for Raspberry Pi 4 in libcamera currently relies on downstream drivers that live in the Raspberry Pi kernel tree, for Unicam (the CSI-2 receiver) and the ISP. The Unicam driver, in particular, includes a few workarounds to implement support for sensor embedded data, to compensate for features that are or were until recently missing in the V4L2 in-kernel and userspace APIs. The new Unicam submission for the kernel reworks the driver extensively to use the recently merged V4L2 streams API, as well as the under development V4L2 generic metadata and internal pads APIs ([2]). The series starts with nine patches that clean up, rework and improve the V4L2Subdevice class (01/32 to 09/32). They have been posted to the list before, and mostly reviewed. Patches 10/32 is where the fun starts, as it pulls the APIs from [2] in the kernel headers. The next three patches, 11/32 to 13/32, then update the V4L2Subdevice and V4L2VideoDevice classes to support those APIs. The next ten patches rework the CameraSensor class to support sensor drivers that use the new APIs. Patches 14/32 to to 21/32 contain various reworks, including moving the camera sensor support to the new src/libcamera/sensor/ directory, and turning CameraSensor into an abstract base class with multiple implementations, using a factory pattern to pick the right match. Patches 22/32 introduces a new CameraSensor subclass for the new APIs, and patch 23/32 then extends the CameraSensor interface to support embedded data. With all the building blocks in place, the next seven patches focus on the Raspberry Pi VC4 pipeline handler to use the new APIs, when interacting with the sensor (24/32), or with the mainline Unicam driver (27/32 to 30/32). Once done, patch 31/32 drops the V4L2_META_FMT_SENSOR_DATA format from the videodev2.h kernel header, as the format is not used anymore and has never been present in the upstream kernel. The last patch, 32/32, is a hack that unconditionally enables embedded data for the IMX219 sensor, as that is the platform I've been using to develop and test this series. Worthy of a note, this series breaks compatibility with the downstream Raspberry Pi kernel. I have decided not to preserve backward compatibiliy to ease development, and to make the new code easier to review. We still need to discuss how to integrate the various components (libcamera and kernel space) in a way that will not generate lots of anger among users. This may lead to some temporary changes to the code to help with the transition. For convenience, the patches can be found at [3]. [1] TBD due to lack of a time machine [2] https://lore.kernel.org/linux-media/20231106122539.1268265-1-sakari.ailus@linux.intel.com [3] https://git.libcamera.org/libcamera/pinchartl/libcamera.git/log/?h=rpi/streams/next Jacopo Mondi (1): libcamera: camera_sensor: Introduce CameraSensorFactory Laurent Pinchart (31): libcamera: v4l2_subdevice: Rename V4L2SubdeviceFormatInfo libcamera: v4l2_subdevice: Add code member to MediaBusFormatInfo libcamera: v4l2_subdevice: Expose media bus format info as internal API libcamera: v4l2_subdevice: Extend MediaBusFormatInfo with metadata formats libcamera: v4l2_subdevice: Drop V4L2SubdeviceFormat::bitsPerPixel() libcamera: v4l2_subdevice: Rename V4L2SubdeviceFormat::mbus_code to code libcamera: v4l2_subdevice: Add stream support to get/set functions libcamera: v4l2_subdevice: Replace Routing::toString() with operator<<() libcamera: v4l2_subdevice: Add V4L2Subdevice::Route structure [DNI] include: linux: Update kernel headers to metadata API libcamera: v4l2_subdevice: Update to the new kernel routing API libcamera: v4l2_subdevice: Add new metadata formats libcamera: v4l2_videodevice: Update to the new kernel metadata API libcamera: camera_sensor: Move related classes to subdirectory libcamera: camera_sensor: Drop updateControlInfo() function libcamera: camera_sensor: Reorder functions libcamera: camera_sensor: Test for read-only HBLANK with READ_ONLY flag libcamera: camera_sensor: Expose the Bayer order libcamera: camera_sensor: Create abstract base class libcamera: camera_sensor: Sort factories by priority libcamera: Add CameraSensor implementation for raw V4L2 sensors libcamera: camera_sensor: Add support for embedded data pipeline: raspberrypi: common: Configure sensor embedded data pipeline: raspberrypi: vc4: Use operator<<(V4L2VideoFormat) pipeline: raspberrypi: vc4: Reorganize platformConfigure() pipeline: raspberrypi: vc4: Use the CameraSensor embedded data API pipeline: raspberrypi: vc4: Unconditionally create embedded data stream pipeline: raspberrypi: vc4: Configure format on Unicam subdev pipeline: raspberrypi: vc4: Fix configuration of the embedded data node include: linux: Drop V4L2_META_FMT_SENSOR_DATA [HACK]: ipa: rpi: cam_helper_imx219: Enable embedded data Documentation/Doxyfile.in | 2 + include/libcamera/internal/camera_sensor.h | 162 +-- include/libcamera/internal/v4l2_subdevice.h | 103 +- include/linux/README | 2 +- include/linux/media-bus-format.h | 13 + include/linux/media.h | 1 + include/linux/v4l2-controls.h | 16 +- include/linux/v4l2-mediabus.h | 18 +- include/linux/v4l2-subdev.h | 23 +- include/linux/videodev2.h | 30 +- src/ipa/rpi/cam_helper/cam_helper_imx219.cpp | 2 +- src/libcamera/meson.build | 3 +- src/libcamera/pipeline/imx8-isi/imx8-isi.cpp | 19 +- src/libcamera/pipeline/ipu3/cio2.cpp | 15 +- src/libcamera/pipeline/ipu3/imgu.cpp | 4 +- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 9 +- src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 2 +- .../pipeline/rpi/common/pipeline_base.cpp | 85 +- .../pipeline/rpi/common/pipeline_base.h | 6 +- src/libcamera/pipeline/rpi/vc4/vc4.cpp | 174 ++- src/libcamera/pipeline/simple/simple.cpp | 29 +- src/libcamera/pipeline/vimc/vimc.cpp | 11 +- src/libcamera/sensor/camera_sensor.cpp | 557 ++++++++ .../camera_sensor_legacy.cpp} | 717 ++++------ .../{ => sensor}/camera_sensor_properties.cpp | 0 src/libcamera/sensor/camera_sensor_raw.cpp | 1135 +++++++++++++++ src/libcamera/sensor/meson.build | 8 + src/libcamera/v4l2_subdevice.cpp | 1239 ++++++++++++++--- src/libcamera/v4l2_videodevice.cpp | 31 +- test/camera-sensor.cpp | 9 +- .../v4l2_videodevice_test.cpp | 7 +- test/v4l2_videodevice/v4l2_videodevice_test.h | 2 +- 32 files changed, 3471 insertions(+), 963 deletions(-) create mode 100644 src/libcamera/sensor/camera_sensor.cpp rename src/libcamera/{camera_sensor.cpp => sensor/camera_sensor_legacy.cpp} (60%) rename src/libcamera/{ => sensor}/camera_sensor_properties.cpp (100%) create mode 100644 src/libcamera/sensor/camera_sensor_raw.cpp create mode 100644 src/libcamera/sensor/meson.build base-commit: c64446c226d4e629884d2f5b148a01969e8ee84a