Hi Sakari, Laurent,
On 26/04/2024 20:13, Sakari Ailus wrote:
> Hi Hans, Mauro,
>
> Here are drivers for Unicam and IPU6 ISYS, prepended by the first part of
> the metadata series. Both drivers can be used without the ROUTING IOCTL
> with partial functionality. The ROUTING IOCTL enablement patch will enable
> the rest of the functionality when it is merged later on.
Running this PR through my build scripts produces these smatch warnings (one
for bcm2835-unicam, 6 for IPU6):
drivers/media/platform/broadcom/bcm2835-unicam.c:2547 unicam_async_nf_init() warn: missing error code 'ret'
drivers/media/pci/intel/ipu6/ipu6-dma.c:67 __dma_alloc_buffer() warn: use 'gfp' here instead of GFP_KERNEL?
drivers/media/pci/intel/ipu6/ipu6-dma.c:164 ipu6_dma_alloc() warn: use 'gfp' here instead of GFP_KERNEL?
drivers/media/pci/intel/ipu6/ipu6.c:402 ipu6_isys_init() warn: passing a valid pointer to 'PTR_ERR'
drivers/media/pci/intel/ipu6/ipu6.c:448 ipu6_psys_init() warn: passing a valid pointer to 'PTR_ERR'
drivers/media/pci/intel/ipu6/ipu6-isys-video.c:833 ipu6_isys_put_stream() warn: variable dereferenced before check 'stream' (see line 829)
drivers/media/pci/intel/ipu6/ipu6-isys-mcd-phy.c:641 ipu6_isys_mcd_phy_config() warn: unsigned 'phy_port' is never less than zero.
I also get a lot of kerneldoc warnings for ipu6-fw-isys.h:
drivers/media/pci/intel/ipu6/ipu6-fw-isys.h:169: warning: Enum value 'IPU6_FW_ISYS_MIPI_VC_0' not described in enum 'ipu6_fw_isys_mipi_vc'
drivers/media/pci/intel/ipu6/ipu6-fw-isys.h:169: warning: Enum value 'IPU6_FW_ISYS_MIPI_VC_1' not described in enum 'ipu6_fw_isys_mipi_vc'
drivers/media/pci/intel/ipu6/ipu6-fw-isys.h:169: warning: Enum value 'IPU6_FW_ISYS_MIPI_VC_2' not described in enum 'ipu6_fw_isys_mipi_vc'
drivers/media/pci/intel/ipu6/ipu6-fw-isys.h:169: warning: Enum value 'IPU6_FW_ISYS_MIPI_VC_3' not described in enum 'ipu6_fw_isys_mipi_vc'
drivers/media/pci/intel/ipu6/ipu6-fw-isys.h:169: warning: Enum value 'N_IPU6_FW_ISYS_MIPI_VC' not described in enum 'ipu6_fw_isys_mipi_vc'
drivers/media/pci/intel/ipu6/ipu6-fw-isys.h:228: warning: wrong kernel-doc identifier on line:
drivers/media/pci/intel/ipu6/ipu6-fw-isys.h:283: warning: Function parameter or struct member 'width' not described in 'ipu6_fw_isys_resolution_abi'
drivers/media/pci/intel/ipu6/ipu6-fw-isys.h:283: warning: Function parameter or struct member 'height' not described in 'ipu6_fw_isys_resolution_abi'
drivers/media/pci/intel/ipu6/ipu6-fw-isys.h:327: warning: Function parameter or struct member 'ts_offsets' not described in 'ipu6_fw_isys_output_pin_info_abi'
drivers/media/pci/intel/ipu6/ipu6-fw-isys.h:327: warning: Function parameter or struct member 's2m_pixel_soc_pixel_remapping' not described in 'ipu6_fw_isys_output_pin_info_abi'
drivers/media/pci/intel/ipu6/ipu6-fw-isys.h:327: warning: Function parameter or struct member 'csi_be_soc_pixel_remapping' not described in 'ipu6_fw_isys_output_pin_info_abi'
drivers/media/pci/intel/ipu6/ipu6-fw-isys.h:327: warning: Function parameter or struct member 'reserved' not described in 'ipu6_fw_isys_output_pin_info_abi'
drivers/media/pci/intel/ipu6/ipu6-fw-isys.h:327: warning: Function parameter or struct member 'snoopable' not described in 'ipu6_fw_isys_output_pin_info_abi'
drivers/media/pci/intel/ipu6/ipu6-fw-isys.h:327: warning: Function parameter or struct member 'error_handling_enable' not described in 'ipu6_fw_isys_output_pin_info_abi'
drivers/media/pci/intel/ipu6/ipu6-fw-isys.h:327: warning: Function parameter or struct member 'sensor_type' not described in 'ipu6_fw_isys_output_pin_info_abi'
drivers/media/pci/intel/ipu6/ipu6-fw-isys.h:356: warning: Function parameter or struct member 'crop_first_and_last_lines' not described in 'ipu6_fw_isys_input_pin_info_abi'
drivers/media/pci/intel/ipu6/ipu6-fw-isys.h:356: warning: Function parameter or struct member 'reserved' not described in 'ipu6_fw_isys_input_pin_info_abi'
drivers/media/pci/intel/ipu6/ipu6-fw-isys.h:366: warning: Function parameter or struct member 'top_offset' not described in 'ipu6_fw_isys_cropping_abi'
drivers/media/pci/intel/ipu6/ipu6-fw-isys.h:366: warning: Function parameter or struct member 'left_offset' not described in 'ipu6_fw_isys_cropping_abi'
drivers/media/pci/intel/ipu6/ipu6-fw-isys.h:366: warning: Function parameter or struct member 'bottom_offset' not described in 'ipu6_fw_isys_cropping_abi'
drivers/media/pci/intel/ipu6/ipu6-fw-isys.h:366: warning: Function parameter or struct member 'right_offset' not described in 'ipu6_fw_isys_cropping_abi'
drivers/media/pci/intel/ipu6/ipu6-fw-isys.h:413: warning: Function parameter or struct member 'reserved' not described in 'ipu6_fw_isys_stream_cfg_data_abi'
drivers/media/pci/intel/ipu6/ipu6-fw-isys.h:413: warning: Function parameter or struct member 'reserved2' not described in 'ipu6_fw_isys_stream_cfg_data_abi'
drivers/media/pci/intel/ipu6/ipu6-fw-isys.h:446: warning: expecting prototype for struct ipu6_fw_isys_frame_buff_set. Prototype was for struct ipu6_fw_isys_frame_buff_set_abi instead
drivers/media/pci/intel/ipu6/ipu6-fw-isys.h:478: warning: expecting prototype for struct ipu6_fw_isys_resp_info_comm. Prototype was for struct ipu6_fw_isys_resp_info_abi instead
drivers/media/pci/intel/ipu6/ipu6-fw-isys.h:489: warning: expecting prototype for struct ipu6_fw_isys_proxy_error_info_comm. Prototype was for struct ipu6_fw_isys_proxy_error_info_abi instead
drivers/media/pci/intel/ipu6/ipu6-fw-isys.h:516: warning: Function parameter or struct member 'resp_info' not described in 'ipu6_fw_resp_queue_token'
drivers/media/pci/intel/ipu6/ipu6-fw-isys.h:526: warning: Function parameter or struct member 'buf_handle' not described in 'ipu6_fw_send_queue_token'
drivers/media/pci/intel/ipu6/ipu6-fw-isys.h:526: warning: Function parameter or struct member 'payload' not described in 'ipu6_fw_send_queue_token'
drivers/media/pci/intel/ipu6/ipu6-fw-isys.h:526: warning: Function parameter or struct member 'send_type' not described in 'ipu6_fw_send_queue_token'
drivers/media/pci/intel/ipu6/ipu6-fw-isys.h:526: warning: Function parameter or struct member 'stream_id' not described in 'ipu6_fw_send_queue_token'
drivers/media/pci/intel/ipu6/ipu6-fw-isys.h:533: warning: Function parameter or struct member 'proxy_resp_info' not described in 'ipu6_fw_proxy_resp_queue_token'
drivers/media/pci/intel/ipu6/ipu6-fw-isys.h:543: warning: Function parameter or struct member 'request_id' not described in 'ipu6_fw_proxy_send_queue_token'
drivers/media/pci/intel/ipu6/ipu6-fw-isys.h:543: warning: Function parameter or struct member 'region_index' not described in 'ipu6_fw_proxy_send_queue_token'
drivers/media/pci/intel/ipu6/ipu6-fw-isys.h:543: warning: Function parameter or struct member 'offset' not described in 'ipu6_fw_proxy_send_queue_token'
drivers/media/pci/intel/ipu6/ipu6-fw-isys.h:543: warning: Function parameter or struct member 'value' not described in 'ipu6_fw_proxy_send_queue_token'
Regards,
Hans
>
> since v2:
>
> - Toss Laurent's patch to disable the metadata format definitions in
> UAPI, on the top.
>
> - Squash Laurent's patch to fix ret assignment in the Unicam driver.
>
> since v1:
>
> - Fix a compile issue in an intermediate patch. The result is unaffected by
> this (no diff).
>
> The diff to v2 is:
>
> diff --git a/drivers/media/platform/broadcom/bcm2835-unicam.c b/drivers/media/platform/broadcom/bcm2835-unicam.c
> index e91d02a64770..6ac4ea8efb1a 100644
> --- a/drivers/media/platform/broadcom/bcm2835-unicam.c
> +++ b/drivers/media/platform/broadcom/bcm2835-unicam.c
> @@ -2519,6 +2519,7 @@ static int unicam_async_nf_init(struct unicam_device *unicam)
> num_data_lanes != 4) {
> dev_err(unicam->dev, "%u data lanes not supported\n",
> num_data_lanes);
> + ret = -EINVAL;
> goto error;
> }
>
> @@ -2526,6 +2527,7 @@ static int unicam_async_nf_init(struct unicam_device *unicam)
> dev_err(unicam->dev,
> "Endpoint uses %u data lanes when %u are supported\n",
> num_data_lanes, unicam->max_data_lanes);
> + ret = -EINVAL;
> goto error;
> }
>
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index 2f9762842313..fe6b67e83751 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -841,6 +841,7 @@ 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 */
>
> +#ifdef __KERNEL__
> /*
> * Line-based metadata formats. Remember to update v4l_fill_fmtdesc() when
> * adding new ones!
> @@ -852,6 +853,7 @@ struct v4l2_pix_format {
> #define V4L2_META_FMT_GENERIC_CSI2_16 v4l2_fourcc('M', 'C', '1', 'G') /* 16-bit CSI-2 packed 8-bit metadata */
> #define V4L2_META_FMT_GENERIC_CSI2_20 v4l2_fourcc('M', 'C', '1', 'K') /* 20-bit CSI-2 packed 8-bit metadata */
> #define V4L2_META_FMT_GENERIC_CSI2_24 v4l2_fourcc('M', 'C', '1', 'O') /* 24-bit CSI-2 packed 8-bit metadata */
> +#endif
>
> /* priv field value to indicates that subsequent fields are valid. */
> #define V4L2_PIX_FMT_PRIV_MAGIC 0xfeedcafe
>
> Please pull.
>
>
> The following changes since commit faa4364bef2ec0060de381ff028d1d836600a381:
>
> media: stk1160: fix bounds checking in stk1160_copy_video() (2024-04-24 13:49:56 +0200)
>
> are available in the Git repository at:
>
> git://linuxtv.org/sailus/media_tree.git tags/for-6.10-metadata-5-no-fmts-signed
>
> for you to fetch changes up to 58e783fc4885b2f04e7ae201a6f854dd4dfde06d:
>
> media: uapi: v4l: Don't expose generic metadata formats to userspace (2024-04-26 21:09:31 +0300)
>
> ----------------------------------------------------------------
> Unicam and IPU6 ISYS drivers, with metadata formats but in-kernel only
>
> ----------------------------------------------------------------
> Bingbu Cao (16):
> media: intel/ipu6: add Intel IPU6 PCI device driver
> media: intel/ipu6: add IPU auxiliary devices
> media: intel/ipu6: add IPU6 buttress interface driver
> media: intel/ipu6: CPD parsing for get firmware components
> media: intel/ipu6: add IPU6 DMA mapping API and MMU table
> media: intel/ipu6: add syscom interfaces between firmware and driver
> media: intel/ipu6: input system ABI between firmware and driver
> media: intel/ipu6: add IPU6 CSI2 receiver v4l2 sub-device
> media: intel/ipu6: add the CSI2 DPHY implementation
> media: intel/ipu6: input system video nodes and buffer queues
> media: intel/ipu6: add the main input system driver
> media: intel/ipu6: add Kconfig and Makefile
> media: MAINTAINERS: add maintainers for Intel IPU6 input system driver
> media: intel/ipu6: support line-based metadata capture support
> media: Documentation: add Intel IPU6 ISYS driver admin-guide doc
> media: Documentation: add documentation of Intel IPU6 driver and hardware overview
>
> Dave Stevenson (2):
> dt-bindings: media: Add bindings for bcm2835-unicam
> media: bcm2835-unicam: Add support for CCP2/CSI2 camera interface
>
> Jean-Michel Hautbois (2):
> media: v4l: Add V4L2-PIX-FMT-Y12P format
> media: v4l: Add V4L2-PIX-FMT-Y14P format
>
> Laurent Pinchart (2):
> media: v4l2-subdev: Clearly document that the crop API won't be extended
> media: uapi: v4l: Don't expose generic metadata formats to userspace
>
> Sakari Ailus (15):
> 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: v4l: Set line based metadata flag in V4L2 core
> media: Documentation: Additional streams generally don't harm capture
> media: Documentation: Document S_ROUTING behaviour
> media: v4l: subdev: Add a function to lock two sub-device states, use it
> 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: ipu6: Add PCI device table header
> media: ivsc: csi: Use IPU bridge
>
> Documentation/admin-guide/media/ipu6-isys.rst | 161 ++
> .../admin-guide/media/ipu6_isys_graph.svg | 548 ++++
> Documentation/admin-guide/media/v4l-drivers.rst | 1 +
> .../bindings/media/brcm,bcm2835-unicam.yaml | 127 +
> Documentation/driver-api/media/drivers/index.rst | 1 +
> Documentation/driver-api/media/drivers/ipu6.rst | 205 ++
> Documentation/userspace-api/media/glossary.rst | 12 +
> Documentation/userspace-api/media/v4l/dev-meta.rst | 21 +
> .../userspace-api/media/v4l/dev-subdev.rst | 31 +-
> .../userspace-api/media/v4l/meta-formats.rst | 3 +-
> .../userspace-api/media/v4l/metafmt-generic.rst | 340 +++
> .../userspace-api/media/v4l/pixfmt-yuv-luma.rst | 48 +
> .../userspace-api/media/v4l/subdev-formats.rst | 269 +-
> .../userspace-api/media/v4l/vidioc-enum-fmt.rst | 7 +
> .../media/v4l/vidioc-subdev-g-crop.rst | 6 +-
> .../media/v4l/vidioc-subdev-g-routing.rst | 51 +-
> .../userspace-api/media/videodev2.h.rst.exceptions | 1 +
> MAINTAINERS | 17 +
> drivers/media/pci/intel/Kconfig | 1 +
> drivers/media/pci/intel/Makefile | 1 +
> drivers/media/pci/intel/ipu6/Kconfig | 18 +
> drivers/media/pci/intel/ipu6/Makefile | 23 +
> drivers/media/pci/intel/ipu6/ipu6-bus.c | 165 ++
> drivers/media/pci/intel/ipu6/ipu6-bus.h | 58 +
> drivers/media/pci/intel/ipu6/ipu6-buttress.c | 912 +++++++
> drivers/media/pci/intel/ipu6/ipu6-buttress.h | 92 +
> drivers/media/pci/intel/ipu6/ipu6-cpd.c | 362 +++
> drivers/media/pci/intel/ipu6/ipu6-cpd.h | 105 +
> drivers/media/pci/intel/ipu6/ipu6-dma.c | 502 ++++
> drivers/media/pci/intel/ipu6/ipu6-dma.h | 19 +
> drivers/media/pci/intel/ipu6/ipu6-fw-com.c | 413 +++
> drivers/media/pci/intel/ipu6/ipu6-fw-com.h | 47 +
> drivers/media/pci/intel/ipu6/ipu6-fw-isys.c | 487 ++++
> drivers/media/pci/intel/ipu6/ipu6-fw-isys.h | 568 ++++
> drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c | 663 +++++
> drivers/media/pci/intel/ipu6/ipu6-isys-csi2.h | 82 +
> drivers/media/pci/intel/ipu6/ipu6-isys-dwc-phy.c | 536 ++++
> drivers/media/pci/intel/ipu6/ipu6-isys-jsl-phy.c | 242 ++
> drivers/media/pci/intel/ipu6/ipu6-isys-mcd-phy.c | 720 +++++
> drivers/media/pci/intel/ipu6/ipu6-isys-queue.c | 811 ++++++
> drivers/media/pci/intel/ipu6/ipu6-isys-queue.h | 78 +
> drivers/media/pci/intel/ipu6/ipu6-isys-subdev.c | 403 +++
> drivers/media/pci/intel/ipu6/ipu6-isys-subdev.h | 59 +
> drivers/media/pci/intel/ipu6/ipu6-isys-video.c | 1399 ++++++++++
> drivers/media/pci/intel/ipu6/ipu6-isys-video.h | 141 +
> drivers/media/pci/intel/ipu6/ipu6-isys.c | 1367 ++++++++++
> drivers/media/pci/intel/ipu6/ipu6-isys.h | 206 ++
> drivers/media/pci/intel/ipu6/ipu6-mmu.c | 845 ++++++
> drivers/media/pci/intel/ipu6/ipu6-mmu.h | 73 +
> .../pci/intel/ipu6/ipu6-platform-buttress-regs.h | 226 ++
> .../pci/intel/ipu6/ipu6-platform-isys-csi2-reg.h | 172 ++
> drivers/media/pci/intel/ipu6/ipu6-platform-regs.h | 179 ++
> drivers/media/pci/intel/ipu6/ipu6.c | 895 +++++++
> drivers/media/pci/intel/ipu6/ipu6.h | 342 +++
> drivers/media/pci/intel/ivsc/mei_csi.c | 20 +-
> drivers/media/platform/Kconfig | 1 +
> drivers/media/platform/Makefile | 1 +
> drivers/media/platform/broadcom/Kconfig | 23 +
> drivers/media/platform/broadcom/Makefile | 3 +
> .../media/platform/broadcom/bcm2835-unicam-regs.h | 246 ++
> drivers/media/platform/broadcom/bcm2835-unicam.c | 2743 ++++++++++++++++++++
> drivers/media/v4l2-core/v4l2-ioctl.c | 43 +-
> drivers/media/v4l2-core/v4l2-subdev.c | 50 +-
> include/media/ipu6-pci-table.h | 28 +
> include/media/v4l2-subdev.h | 42 +
> include/uapi/linux/media-bus-format.h | 9 +
> include/uapi/linux/v4l2-mediabus.h | 18 +-
> include/uapi/linux/v4l2-subdev.h | 14 +-
> include/uapi/linux/videodev2.h | 26 +
> 69 files changed, 18263 insertions(+), 65 deletions(-)
> create mode 100644 Documentation/admin-guide/media/ipu6-isys.rst
> create mode 100644 Documentation/admin-guide/media/ipu6_isys_graph.svg
> create mode 100644 Documentation/devicetree/bindings/media/brcm,bcm2835-unicam.yaml
> create mode 100644 Documentation/driver-api/media/drivers/ipu6.rst
> create mode 100644 Documentation/userspace-api/media/v4l/metafmt-generic.rst
> create mode 100644 drivers/media/pci/intel/ipu6/Kconfig
> create mode 100644 drivers/media/pci/intel/ipu6/Makefile
> create mode 100644 drivers/media/pci/intel/ipu6/ipu6-bus.c
> create mode 100644 drivers/media/pci/intel/ipu6/ipu6-bus.h
> create mode 100644 drivers/media/pci/intel/ipu6/ipu6-buttress.c
> create mode 100644 drivers/media/pci/intel/ipu6/ipu6-buttress.h
> create mode 100644 drivers/media/pci/intel/ipu6/ipu6-cpd.c
> create mode 100644 drivers/media/pci/intel/ipu6/ipu6-cpd.h
> create mode 100644 drivers/media/pci/intel/ipu6/ipu6-dma.c
> create mode 100644 drivers/media/pci/intel/ipu6/ipu6-dma.h
> create mode 100644 drivers/media/pci/intel/ipu6/ipu6-fw-com.c
> create mode 100644 drivers/media/pci/intel/ipu6/ipu6-fw-com.h
> create mode 100644 drivers/media/pci/intel/ipu6/ipu6-fw-isys.c
> create mode 100644 drivers/media/pci/intel/ipu6/ipu6-fw-isys.h
> create mode 100644 drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c
> create mode 100644 drivers/media/pci/intel/ipu6/ipu6-isys-csi2.h
> create mode 100644 drivers/media/pci/intel/ipu6/ipu6-isys-dwc-phy.c
> create mode 100644 drivers/media/pci/intel/ipu6/ipu6-isys-jsl-phy.c
> create mode 100644 drivers/media/pci/intel/ipu6/ipu6-isys-mcd-phy.c
> create mode 100644 drivers/media/pci/intel/ipu6/ipu6-isys-queue.c
> create mode 100644 drivers/media/pci/intel/ipu6/ipu6-isys-queue.h
> create mode 100644 drivers/media/pci/intel/ipu6/ipu6-isys-subdev.c
> create mode 100644 drivers/media/pci/intel/ipu6/ipu6-isys-subdev.h
> create mode 100644 drivers/media/pci/intel/ipu6/ipu6-isys-video.c
> create mode 100644 drivers/media/pci/intel/ipu6/ipu6-isys-video.h
> create mode 100644 drivers/media/pci/intel/ipu6/ipu6-isys.c
> create mode 100644 drivers/media/pci/intel/ipu6/ipu6-isys.h
> create mode 100644 drivers/media/pci/intel/ipu6/ipu6-mmu.c
> create mode 100644 drivers/media/pci/intel/ipu6/ipu6-mmu.h
> create mode 100644 drivers/media/pci/intel/ipu6/ipu6-platform-buttress-regs.h
> create mode 100644 drivers/media/pci/intel/ipu6/ipu6-platform-isys-csi2-reg.h
> create mode 100644 drivers/media/pci/intel/ipu6/ipu6-platform-regs.h
> create mode 100644 drivers/media/pci/intel/ipu6/ipu6.c
> create mode 100644 drivers/media/pci/intel/ipu6/ipu6.h
> create mode 100644 drivers/media/platform/broadcom/Kconfig
> create mode 100644 drivers/media/platform/broadcom/Makefile
> create mode 100644 drivers/media/platform/broadcom/bcm2835-unicam-regs.h
> create mode 100644 drivers/media/platform/broadcom/bcm2835-unicam.c
> create mode 100644 include/media/ipu6-pci-table.h
>
