Re: [PATCH 00/19] media: renesas: rcar-csi2: Support multiple streams

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

On Tue, Apr 30, 2024 at 01:51:13PM +0200, Jacopo Mondi wrote:
> Hi Niklas
>   thanks for the quick reply
> 
> On Tue, Apr 30, 2024 at 01:17:18PM +0200, Niklas Söderlund wrote:
> > Hi Jacopo,
> >
> > Thanks for this work! I'm happy that we finally can get this upstream. I
> > will test and review this in the coming days. As there appears to be a
> > few dependencies to get this to work do you have a branch covering
> > everything needed?
> 
> Sure
> https://git.kernel.org/pub/scm/linux/kernel/git/jmondi/linux.git/
> jmondi/renesas-drivers-2024-04-23-v6.9-rc5/multistream
> 
> I should have added it to the cover letter
> 
> > On 2024-04-30 12:39:36 +0200, Jacopo Mondi wrote:
> > > Hello this series implements multi-stream support for R-Car CSI-2 and the
> > > two CSI-2 devices that are available on Renesas dev-boards which are mainline
> > > supported (the ADV748x HDMI decoder and the MAX9286 GMSL deserializer).
> >
> > You also need to update the MAX96712 driver in staging else V3U and Gen4
> > will break.
> >
> 
> Ouch, right. I can do that but won't be able to test. Would you ?
> 
> > > The net gain in feature is:
> > > - ADV748x: Select on which MIPI CSI-2 VC to send the stream on
> > > - MAX9286: Paves the way for run-time selection of active links
> > >
> > > Both features are realized by controlling the devices routing tables.
> > > The max9286 routing support is not implemented, but these patches are needed
> > > in order to maintain the current version compatible with R-Car CSI-2.
> > >
> > > The R-Car CSI-2 driver needs to be updated in order to support multiplexed
> > > transmitters, in particular:
> > >
> > > - Use LINK_FREQ to compute the lane bandwidth
> > > - Support subdev_active state
> > > - Configure DT and VC handling by using the remote's frame_desc
> > >
> > > A separate fix for VIN sits at the top of the series to re-enable YUYV capture
> > > operations.
> > >
> > > The series needs to activate streams in v4l2-subdev to work correctly:
> > >
> > > --- a/drivers/media/v4l2-core/v4l2-subdev.c
> > > +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> > > @@ -32,7 +32,7 @@
> > >   * 'v4l2_subdev_enable_streams_api' to 1 below.
> > >   */
> > >
> > > -static bool v4l2_subdev_enable_streams_api;
> > > +static bool v4l2_subdev_enable_streams_api = 1;
> > >  #endif
> >
> > I'm not up to date with the latest state of the multiple streams API.
> > Can this be runtime configured or will this break capture if the kernel
> > is not patched to enable this?
> 
> I'm afraid the latter. I really hope in the next kernel release we can
> get rid of this protection and enable streams by default.

I hope it will be done for v6.12 or possibly v6.11.

> > >
> > > Test scripts, based on vin-tests are available at:
> > > https://git.uk.ideasonboard.com/jmondi/vin-tests
> > >
> > > A note for Sakari: patch
> > > "media: max9286: Implement support for LINK_FREQ"
> > >
> > > Could actually use your new
> > > "media: v4l: Support passing sub-device argument to v4l2_get_link_freq()"
> > >
> > > I kept it as it is for this version waiting for your patch to get in.
> >
> > I like this idea.
> >
> > >
> > > A note for Niklas:
> > > I think this series paves the way to actually moving the control of which
> > > VC goes to which VIN using a routing table instead of using link enablement.
> > >
> > > Currently the setup is as it follows: routing within R-Car CSI-2 is fixed, sink
> > > stream 0/X goes to source stream X+1/0 and each source pad represents a VC.
> > > Which VC gets sent to which VIN is controlled by link enablement.
> > >
> > > A more natural way of handling this would be to make the routing table
> > > within CSI-2 configurable and have media-links immutable between the CSI-2
> > > source pad. A CSI-2 source pad would then represent a 'channel' between
> > > CSI-2 and VIN and not the VC which is sent on it.
> > >
> > > This setup would better represent the current implementation, with VIN's VCSEL
> > > fixed channel routing and with the CSI-2 routing table controlling which VC gets
> > > sent on which channel. What do you think ?
> >
> > That could be useful. Maybe we can even switch to use immutable links
> > and use the routing instead greatly simplifying the VIN driver as this
> > together with the other VIN series on the list we can drop the ugly
> > group concept ;-)
> 
> I had the same idea, but I'm not yet sure how this would work when a
> CSI-2 can be routed to multiple VIN groups...
> 
> > But this series is already quiet large, I think we can work on this
> > on-top in a new series.
> 
> Exactly, I didn't want to pile too many things. max9286 routing would
> be next in line, I kept it out to avoid sending 30 patches in one go
> (and to send this out earlier to get feedbacks)
> 
> > > Jacopo Mondi (19):
> > >   media: adv748x: Add support for active state
> > >   media: adv748x: Add flags to adv748x_subdev_init()
> > >   media: adv748x: Use V4L2 streams
> > >   media: adv748x: Propagate format to opposite stream
> > >   media: adv748x: Implement set_routing()
> > >   media: adv748x: Use routes to configure VC
> > >   media: adv748x: Implement .get_frame_desc()
> > >   media: max9286: Add support for subdev active state
> > >   media: max9286: Fix enum_mbus_code
> > >   media: max9286: Use frame interval from subdev state
> > >   media: max9286: Use V4L2 Streams
> > >   media: max9286: Implement .get_frame_desc()
> > >   media: max9286: Implement support for LINK_FREQ
> > >   media: max9286: Implement .get_mbus_config()
> > >   media: rcar-csi2: Add support for multiplexed streams
> > >   media: rcar-csi2: Support multiplexed transmitters
> > >   media: rcar-csi2: Store format in the subdev state
> > >   media: rcar-csi2: Implement set_routing
> > >   media: rcar-vin: Fix YUYV8_1X16 handling for CSI-2
> > >
> > >  drivers/media/i2c/adv748x/adv748x-afe.c       |   2 +-
> > >  drivers/media/i2c/adv748x/adv748x-core.c      |  12 +-
> > >  drivers/media/i2c/adv748x/adv748x-csi2.c      | 251 +++++++---
> > >  drivers/media/i2c/adv748x/adv748x-hdmi.c      |   2 +-
> > >  drivers/media/i2c/adv748x/adv748x.h           |   4 +-
> > >  drivers/media/i2c/max9286.c                   | 457 ++++++++++++------
> > >  drivers/media/platform/renesas/rcar-csi2.c    | 442 ++++++++++++-----
> > >  .../platform/renesas/rcar-vin/rcar-dma.c      |  16 +-
> > >  8 files changed, 834 insertions(+), 352 deletions(-)

-- 
Regards,

Laurent Pinchart