Hi Sakari, Thank you for the patches. This is progressing nicely, and I think we can merge part of this series for v6.10. Patches from 01/46 to 08/46 and from 11/46 to 17/46 are nearly there, just a few of them will need a new (and hopefully final) version. You could additionally merge the CCS-specific patches 18/46 to 24/46 if desired. Some ov2740 rework could go in too. The rest of the patches (and in particular 09/46 and 10/46) depend on internal pads support, which still needs discussions and additional documentation for raw sensors. When posting v10, please move 09/46 and 10/46 after 24/46. On Tue, Apr 16, 2024 at 10:32:33PM +0300, Sakari Ailus wrote: > Hi folks, > > Here are a few patches to add support generic, line based metadata as well > as internal pads. While the amount of code is not very large, to the > contrary it is quite small actually IMO, I presume what this is about and > why it is being proposed requires some explaining. > > Metadata mbus codes and formats have existed for some time in V4L2. They > however have been only used by drivers that produce the data itself and > effectively this metadata has always been statistics of some sort (at > least when it comes to ISPs). What is different here is that we intend to > add support for metadata originating from camera sensors. > > Camera sensors produce different kinds of metadata, embedded data (usually > register address--value pairs used to capture the frame, in a more or less > sensor specific format), histograms (in a very sensor specific format), > dark pixels etc. The number of these formats is probably going to be about > as large as image data formats if not larger, as the image data formats > are much better standardised but a smaller subset of them will be > supported by V4L2, at least initially but possibly much more in the long > run. > > Having this many device specific formats would be a major problem for all > the other drivers along that pipeline (not to mention the users of those > drivers), including bridge (e.g. CSI-2 to parallel) but especially CSI-2 > receiver drivers that have DMA: the poor driver developer would not only > need to know camera sensor specific formats but to choose the specific > packing of that format suitable for the DMA used by the hardware. It is > unlikely many of these would ever get tested while being present on the > driver API. Also adding new sensors with new embedded data formats would > involve updating all bridge and CSI-2 receiver drivers. I don't expect > this to be a workable approach. > > Instead what I'm proposing is to use specific metadata formats on the > sensor devices only, on internal pads (more about those soon) of the > sensors, only visible in the UAPI, and then generic mbus formats along the > pipeline and finally generic V4L2 metadata formats on the DMAs (specific > to bit depth and packing). This would unsnarl the two, defining what data > there is (specific mbus code) and how that is transported and packed > (generic mbus codes and V4L2 formats). > > The user space would be required to "know" the path of that data from the > sensor's internal pad to the V4L2 video node. I do not see this as these > devices require at least some knowledge of the pipeline, i.e. hardware at > hand. Separating what the data means and how it is packed may even be > beneficial: it allows separating code that interprets the data (sensor > internal mbus code) from the code that accesses it (packing). > > These formats are in practice line based, meaning that there may be > padding at the end of the line, depending on the bus as well as the DMA. > If non-line based formats are needed, it is always possible to set the > "height" field to 1. > > The internal sink pads are an alternative to source routes [1]. The source > routes were not universally liked and I do have to say I like re-using > existing interface concepts (pads and everything you can do with pads, > including access format, selections etc.) wherever it makes sense, instead > of duplicating functionality. > > Effectively internal sink pads behave mostly just like sink pads, but they > describe a flow of data that originates from a sub-device instead of > arriving to a sub-device. The SUBDEV_S_ROUTING IOCTLs are used to enable > and disable routes from internal sink pads to sub-device's source pads. > The subdev format IOCTLs are usable, too, so one can find which subdev > format is available on given internal sink pad. > > I've also pushed these here and I'll keep updating the branch, I've also > included untested OV2740 patches: > > <URL:https://git.linuxtv.org/sailus/media_tree.git/log/?h=metadata> > > Questions and comments are most welcome. > > Preliminary media-ctl and yavta patches can be found here: > > <URL:https://git.retiisi.eu/?p=~sailus/yavta.git;a=shortlog;h=refs/heads/metadata> > <URL:https://git.retiisi.eu/?p=~sailus/v4l-utils.git;a=shortlog;h=refs/heads/metadata> > > I have used IMX219 as an example on routing in a sensor driver in this > version. > > The patches are on my master branch > <URL:https://git.linuxtv.org/sailus/media_tree.git/log/>. > > [1] <URL:https://lore.kernel.org/linux-media/20220831141357.1396081-20-tomi.valkeinen@xxxxxxxxxxxxxxxx/> > > since v8: > > - Move the patch adding internal pad flag past the routing API reworks, as > well as a few other patches, in order to separate the patches to those > that could still be merged for v6.10 (routing changes) and those that > couldn't (sensor API related). The patch on the edge is "media: uapi: > v4l: subdev: Enable streams API". > > - Include Laurent's two patches to address crop API issues wrt. streams. > > - Add two patches to prepare for CCS driver rework (media: ccs: Move > ccs_pm_get_init function up and media: ccs: Rename out label of > ccs_start_streaming). > > - Address issues in the ov2740 driver patches (as well as the driver > itself), 4 more patches towards the end of the set. > > - Improved generic metadata format names, align with other existing > formats. > > - Improved ov2740 embedded data documentation. > > - Reworked streams and camera sensor documentation based on Laurent's > comments mainly. In particular, the contradictory concept of internal > source pads no longer should exist in the patches. > > - Fixed pad numbering in the CCS example. > > - Fixed S_ROUTING behaviour when len_routes is too small and when > S_ROUTING isn't implemented by the driver. > > - Reorder sections in meta-formats.rst alphabetically. > > - Add a note per struct fields that certain struct v4l2_subdev_format are > zero for metadata mbus codes. > > - CCS driver patch cleanups. > > - CCS driver metadata width fix for space-efficient embedded data at 16 > bpp and over. > > - Postpone CCS frame descriptor quirk for now. > > - Use MIPI_CSI2_DT_USER_DEFINED(0) instead of a numerical value for > compressed data datatype. > > since v7: > > - Add embedded data support for the ov2740 driver. > > - Add three patches on top, to add an IMMUTABLE flag to source streams > when they cannot be disabled. > > - Improved documentation of len_routes and num_routes arguments of > [GS]_ROUTING. > > - Remove one inclusion of twice-included media/v4l2-fwnode.h in > drivers/media/i2c/ccs/ccs-core.c . > > - Add missing forward declaration of ccs_internal_ops in > drivers/media/i2c/ccs/ccs-core.c . > > since v6: > > - Improve embedded data UAPI documentation on camera sensors. > > - Improve wording of stream glossary entry. > > - Improve internal pad flag documentation. > > - Fix definition of "data unit" and remove an extra "only" in INTERNAL pad > flag description (1st patch). > > - Use IMX219 driver in examples consistently. > > - Remove the CSI-2 to parallel bridge from the example to simplify the > example. > > - Minor rewording of some parts of the routing examples. > > - Rebase on unified sub-device state information access functions: > <URL:https://lore.kernel.org/linux-media/20231027095913.1010187-1-sakari.ailus@xxxxxxxxxxxxxxx/T/#t> > > - In CCS driver, do not maintain current active configuration in driver's > device context struct (apart from mbus codes). Rely on sub-device state > locking and clean up the code. (Multiple patches towards the end of the > set.) > > - Arrange the CCS patches early in the set towards the end of the set. > > - Move the patch enabling streams API to the end of the set. > > - Rework IOCTL argument copying condition for [GS]_ROUTING). > > - Handle copying back routes in S_ROUTING, do not rely on G_ROUTING > IOCTL implementation. > > - Rebase on metadata preparation patchset v6: > <URL:https://lore.kernel.org/linux-media/20231106121805.1266696-1-sakari.ailus@xxxxxxxxxxxxxxx/T/#t>. > > since v5: > > - Rebase on new set of preparation patches. > > - Switch CCS driver from s_stream to enable_streams/disable_streams. Keep > streaming state information --- the sensor remains in streaming state if > any of the streams is enabled. > > - Fix setting mbus code on embedded data in get_frame_desc() op in the CCS > driver. > > since v4: > > - Add a patch to acquire two sub-device states that may use the same lock. > > - Add a patch for CCS driver to remove ccs_get_crop_compose() helper. > > - Add a patch for CCS driver moving acquiring and releasing the mutex to > the s_stream callback. > > - Add a patch for CCS driver to rely on sub-device state locking using a > single driver-provided lock. > > - Fixed calculating minimum number of routes in copying the routes > (thanks, Laurent). > > - Moved a label in S_ROUTING handling to make Clang happy (hopefully). > > - Fixed setting emb_data_ctrl register for CCS embedded data support. > > - Rebase on Laurent's cleanup patches. > > - Wrap a few long lines. > > - Write in embedded data documentation sensor drivers generally don't > allow configuring it. > > since v3: > > - Separate preparation patches from this set. > > - Add a definition for "Data unit", a pixel that is not image data and use > it instead in format documentation. > > - Fix more numbered lists in dev-subdev.rst. > > - Remove a redundant definition for V4L2_META_FMT_GENERIC_CSI2_2_24 --- > V4L2_META_FMT_GENERIC_CSI2_12 can be used instead. > > - Use "X" instead of "p" to denote padding in format documentation. > > - Use IMX219 in examples instead of CCS. > > - Document that the generic V4L2 CSI-2 metadata formats use padding > defined in CSI-2 spec and packing defined in CCS spec. > > - Add patches to align [GS]_ROUTING behaviour with V4L2. This means mainly > returning configured routes as part of S_ROUTING as well. "len_routes" > field is added to denote the length of the array and having more routes > than fits in the array is no longer an error. Also added more reserved > fields. > > - Added trivial support for S_ROUTING (via G_ROUTING implementation) for > use in drivers with static-only routes. > > - Added helper functions to obtain mbus format as well as crop and compose > rectangles that are streams-independent. > > - Added a patch to define generic CSI-2 long packet types. > > - Removed MEDIA_BUS_FMT_IS_META() macro. It didn't seem useful in the end. > > - Use a single CCS embedded data format. The bit depth can be selected > using the meta stream on the source pad. > > - Fix mbus code numbers (there were holes due to removed redundant > formats). > > - Fix generic mbus code documentation (byte was being used instead of > bit). > > - Fix spelling of "length". > > - Added a patch to remove v4l2_subdev_enable_streams_api that disables > streams API. This should be merged once libcamera support for streams > works nicely. > > - Don't use strings in printing frame descriptor flags. > > - Warn on string truncation in printing frame descriptor. > > since v2: > > - Add a better example, with formats. > > - Add CCS static data media bus codes. > > - Added an example demonstrating the use of internal pads. --- Is the > level of detail enough for the purpose? > > - Improved documentation. > > - Added a macro to tell whether a format is a metadata format. > (Documentation could be added.) > > - A small ReST syntax fix in the same section. > > - Drop leftovers of a patch checking for the INTERNAL_SOURCE flag. > > since v1: > > - Make the new pad flag just "INTERNAL", requiring either SINK or SOURCE > pad flag to accompany it. Removed the union in struct v4l2_subdev_route. > > - Add the term "stream" to MC glossary. > > - Improved and fixed documentation (according to comments). > > - Note these formats are little endian. > > - Remove 1X8 from the names of the mbus codes. These formats have generally > 8 bits per pixel. > > - Fix mbus code numbering (had holes in RFC). > > - Add new metadata fields to debug prints. > > - Fix a minor documentation build issue. > > Laurent Pinchart (2): > media: v4l2-subdev: Fix stream handling for crop API > media: v4l2-subdev: Clearly document that the crop API won't be > extended > > Sakari Ailus (44): > media: Documentation: Add "stream" into glossary > media: uapi: Add generic serial metadata mbus formats > media: uapi: Document which mbus format fields are valid for metadata > media: uapi: v4l: Add generic 8-bit metadata format definitions > media: v4l: Support line-based metadata capture > media: Documentation: Additional streams generally don't harm capture > media: Documentation: Document embedded data guidelines for camera > sensors > media: Documentation: v4l: Document internal sink pads > media: Documentation: Document S_ROUTING behaviour > media: v4l: subdev: Add a function to lock two sub-device states, use > it > media: v4l: subdev: Move G_ROUTING handling below S_ROUTING > media: v4l: subdev: Copy argument back to user also for S_ROUTING > media: v4l: subdev: Add len_routes field to struct v4l2_subdev_routing > media: v4l: subdev: Return routes set using S_ROUTING > media: v4l: subdev: Add trivial set_routing support > media: ccs: No need to set streaming to false in power off > media: ccs: Move ccs_pm_get_init function up > media: ccs: Rename out label of ccs_start_streaming > media: ccs: Use {enable,disable}_streams operations > media: ccs: Track streaming state > media: ccs: Move ccs_validate_csi_data_format up > media: ccs: Support frame descriptors > media: uapi: v4l: subdev: Enable streams API > media: mc: Add INTERNAL pad flag > media: uapi: ccs: Add media bus code for MIPI CCS embedded data > media: Documentation: Document non-CCS use of CCS embedded data format > media: Documentation: ccs: Document routing > media: ccs: Add support for embedded data stream > media: ccs: Remove ccs_get_crop_compose helper > media: ccs: Rely on sub-device state locking > media: ccs: Compute binning configuration from sub-device state > media: ccs: Compute scaling configuration from sub-device state > media: ccs: Remove which parameter from ccs_propagate > media: uapi: Add media bus code for ov2740 embedded data > media: ov2740: Fix LINK_FREQ and PIXEL_RATE control value reporting > media: ov2740: Remove shorthand variables > media: ov2740: Switch to {enable,disable}_streams > media: ov2740: Track streaming state > media: ov2740: Add support for embedded data > media: ov2740: Add generic sensor fwnode properties as controls > media: ov2740: Add support for G_SELECTION IOCTL > media: v4l: Add V4L2_SUBDEV_ROUTE_FL_IMMUTABLE sub-device routing flag > media: ccs: Add IMMUTABLE route flag > media: ov2740: Add IMMUTABLE route flag > > .../media/drivers/camera-sensor.rst | 32 + > .../userspace-api/media/drivers/ccs.rst | 38 +- > .../userspace-api/media/glossary.rst | 15 + > .../media/mediactl/media-types.rst | 9 + > .../userspace-api/media/v4l/dev-meta.rst | 21 + > .../userspace-api/media/v4l/dev-subdev.rst | 179 ++- > .../userspace-api/media/v4l/meta-formats.rst | 3 +- > .../media/v4l/metafmt-generic.rst | 328 +++++ > .../media/v4l/subdev-formats.rst | 374 +++++- > .../media/v4l/vidioc-enum-fmt.rst | 7 + > .../media/v4l/vidioc-subdev-g-crop.rst | 6 +- > .../media/v4l/vidioc-subdev-g-routing.rst | 60 +- > .../media/videodev2.h.rst.exceptions | 1 + > drivers/media/i2c/ccs/ccs-core.c | 1050 +++++++++++------ > drivers/media/i2c/ccs/ccs.h | 27 +- > drivers/media/i2c/ov2740.c | 304 +++-- > drivers/media/mc/mc-entity.c | 15 +- > drivers/media/v4l2-core/v4l2-ioctl.c | 25 +- > drivers/media/v4l2-core/v4l2-subdev.c | 118 +- > include/media/v4l2-subdev.h | 42 + > include/uapi/linux/media-bus-format.h | 13 + > include/uapi/linux/media.h | 1 + > include/uapi/linux/v4l2-mediabus.h | 18 +- > include/uapi/linux/v4l2-subdev.h | 18 +- > include/uapi/linux/videodev2.h | 18 + > 25 files changed, 2183 insertions(+), 539 deletions(-) > create mode 100644 Documentation/userspace-api/media/v4l/metafmt-generic.rst -- Regards, Laurent Pinchart
