Message-ID: <a2712f66-f4fe-4753-bfec-dedd60c8210f@ti.com>
Date: Tue, 24 Jun 2025 15:22:39 +0530
From: Yemike Abhilash Chandra <y-abhilashchandra@...com>
To: Rishikesh Donadkar <r-donadkar@...com>, <jai.luthra@...ux.dev>,
        <laurent.pinchart@...asonboard.com>, <mripard@...nel.org>
CC: <devarsht@...com>, <vaishnav.a@...com>, <s-jain1@...com>,
        <vigneshr@...com>, <mchehab@...nel.org>, <robh@...nel.org>,
        <krzk+dt@...nel.org>, <conor+dt@...nel.org>,
        <sakari.ailus@...ux.intel.com>, <hverkuil-cisco@...all.nl>,
        <tomi.valkeinen@...asonboard.com>, <jai.luthra@...asonboard.com>,
        <changhuang.liang@...rfivetech.com>, <jack.zhu@...rfivetech.com>,
        <linux-kernel@...r.kernel.org>, <linux-media@...r.kernel.org>,
        <devicetree@...r.kernel.org>
Subject: Re: [PATCH v4 00/12] media: cadence,ti: CSI2RX Multistream Support

Hi Rishikesh,

Thank you for the patch series.

On 14/05/25 16:55, Rishikesh Donadkar wrote:
> This series adds multi-stream support for Cadence CSI2RX and TI CSI2RX
> SHIM drivers.
> 
> PATCH 1-6:  Support multiple DMA contexts/video nodes in TI CSI2RX
> PATCH 7-8:  Use get_frame_desc to propagate virtual channel
>              information across Cadence and TI CSI-RX subdevs
> PATCH 9-10: Use new multi-stream APIs across the drivers to support
>              multiplexed cameras from sources like UB960 (FPDLink)
> PATCH 11:   Optimize stream on by submitting all queued buffers to DMA
> PATCH 12:   Change the drain architecture to support multi-stream
> 
> Testing for this series has been done on top of media tree with 4x IMX219
> camera modules connected to TI's AM62A using V3 Link fusion mini board.
> 
> Overlay and defconfig changes for the same can be found below:
> https://github.com/RISHI27-dot/linux/commits/u/multistream_v4
> 
> v4l2-compliance results:
> https://gist.github.com/Rishikesh-D/4ddf957da2f62191eefe7a9e5ff212ab
> 

For the entire series,

Reviewed-by: Yemike Abhilash Chandra <y-abhilashchandra@...com>

> ---
> Changes in v4:
> 
> [PATCH 01/13] dt-bindings: media: ti,j721e-csi2rx-shim: Support 32 dma chans
>    - No change
> [PATCH 02/13] media: ti: j721e-csi2rx: separate out device and context
>    - Add ctx identifier in the dev_err() message
>    - No change
> [PATCH 04/13] media: ti: j721e-csi2rx: prepare SHIM code for multiple contexts
>    - Reduced the name string lenght from 32 chars to 5 chars
> [PATCH 05/13] media: ti: j721e-csi2rx: add a subdev for the core device
>    - Add .enum_mbus_code callback
>    - Replace statically allocated struct with a global static const struct
>      v4l2_mbus_framefmt and used that in the _init_state() function
> [PATCH 06/13] media: ti: j721e-csi2rx: get number of contexts from device tree
>    - Fix the drain buffer being leaked
>    - If the shows more number of ctx than the TI_CSI2RX_MAX_CTX, return an error
>      instead of warning
> [PATCH 07/13] media: cadence: csi2rx: add get_frame_desc wrapper
>    - No change
> [PATCH 08/13] media: ti: j721e-csi2rx: add support for processing virtual channels
>    - Call ti_csi2rx_get_vc() only once on first stream start and cache the VC data in
>      the driver, use the corresponding VC in all subsequent stream starts.
> [PATCH 09/13] media: cadence: csi2rx: Use new enable stream APIs
> [PATCH 10/13] media: cadence: csi2rx: Enable multi-stream support
>    - Squash the above two patches into
>      [PATCH v4 09/12] media: cadence: csi2rx: add multistream support
>    - Use already obtained csi2rx->source_pad in enable_streams() and
>      disable_streams() call
>    - Update commit message with the reason for using a custom helper for s_stream
>      instead of v4l2_subdev_s_stream_helper()
>    - Use v4l2_get_link_freq() variant that takes pad of the source as its first
>      argument instead of the one that takes v4l2_ctrl_handler
>    - Call v4l2_get_link_freq() with bpp = 0 to prevent fallback to V4L2_CID_PIXEL_RATE
>      in multi-stream case
>    - Use lock guards to simplify error handling
>    - Call csi2rx_update_vc_select() at first stream start before enabling the controller
> [PATCH 11/13] media: ti: j721e-csi2rx: add multistream support
>    - No change
> [PATCH 12/13] media: ti: j721e-csi2rx: Submit all available buffers
>    - No change
> [PATCH 13/13] media: ti: j721e-csi2rx: Change the drain architecture for multistream
>    - Fix checkpatch warning
>    - Change commit message to give a better description of the patch
> 
> Link to (v3):
>    https://lore.kernel.org/all/20250417065554.437541-1-r-donadkar@ti.com/
> 
> Changes in v3:
> 
> - Drop [PATCH v2 01/13] media: cadence: csi2rx: Support runtime PM from
>    v2, support for runtime PM will be added in a separate series:
>    https://lore.kernel.org/all/20250224-ti_csi_pm-v1-0-8f8c29ef646d@ideasonboard.com/
> - Change the drain architecture to prevent FIFO overflow in multistream
>    usecases.
> - With the new drain architecture, we don't need the the driver to wait
>    for userspace to start streaming on all "actively routed" video nodes
>    before starting streaming on the source. So, revert back to the capture
>    architecture where streams can be started and stopped independent
>    to each other.
> 
> Link to (v2):
>    https://lore.kernel.org/r/20240627-multistream-v2-0-6ae96c54c1c3@ti.com
> 
> Changes in v2:
> 
> - Change the multi-camera capture architecture to be similar to that of
>    Tomi's RPi5 FE series, where the driver will wait for userspace to
>    start streaming on all "actively routed" video nodes before starting
>    streaming on the source. This simplifies things a lot from the HW
>    perspective, which might run into deadlocks due to a shared FIFO
>    between multiple DMA channels.
> 
> - Drop a few fixes that were posted separately and are already merged
> - Fix dtschema warnings reported by Rob on [02/13]
> - Fix warnings for uninitialized `used_vc` variable in cdns-csi2rx.c
> - Return -EBUSY if someone updates routes for j721e-csi2rx subdev while
>    streaming
> - Only allow single-streams to be routed to the source pads (linked to
>    video nodes) of the j721e-csi2rx device
> - Squash the patches marked "SQUASH" in the v1 RFC series
> 
> Link to RFC (v1):
> https://lore.kernel.org/r/20240222-multistream-v1-0-1837ed916eeb@ti.com
> 
> 
> Jai Luthra (7):
>    dt-bindings: media: ti,j721e-csi2rx-shim: Support 32 dma chans
>    media: ti: j721e-csi2rx: separate out device and context
>    media: ti: j721e-csi2rx: add a subdev for the core device
>    media: ti: j721e-csi2rx: add support for processing virtual channels
>    media: cadence: csi2rx: add multistream support
>    media: ti: j721e-csi2rx: add multistream support
>    media: ti: j721e-csi2rx: Submit all available buffers
> 
> Pratyush Yadav (4):
>    media: ti: j721e-csi2rx: prepare SHIM code for multiple contexts
>    media: ti: j721e-csi2rx: allocate DMA channel based on context index
>    media: ti: j721e-csi2rx: get number of contexts from device tree
>    media: cadence: csi2rx: add get_frame_desc wrapper
> 
> Rishikesh Donadkar (1):
>    media: ti: j721e-csi2rx: Change the drain architecture for multistream
> 
>   .../bindings/media/ti,j721e-csi2rx-shim.yaml  |  39 +-
>   drivers/media/platform/cadence/cdns-csi2rx.c  | 372 +++++--
>   .../platform/ti/j721e-csi2rx/j721e-csi2rx.c   | 958 +++++++++++++-----
>   3 files changed, 1025 insertions(+), 344 deletions(-)
> 

Powered by blists - more mailing lists