Hello,
On 2023-02-24 16:48:55 +0100, Jacopo Mondi wrote:
> Hi Tomi
>
> On Wed, Feb 22, 2023 at 02:56:30PM +0200, Tomi Valkeinen wrote:
> > Add routing and stream_config support to CAL driver.
> >
> > Add multiplexed streams support. CAL has 8 dma-engines and can capture 8
> > separate streams at the same time.
> >
> > Add 8 video device nodes, each representing a single dma-engine, and set
> > the number of source pads on camerarx to 8. Each video node can be
> > connected to any of the source pads on either of the camerarx instances
> > using media links. Camerarx internal routing is used to route the
> > incoming CSI-2 streams to one of the 8 source pads.
> >
> > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@xxxxxxxxxxxxxxxx>
> > ---
> > drivers/media/platform/ti/cal/cal-camerarx.c | 233 ++++++++++++++-----
> > drivers/media/platform/ti/cal/cal-video.c | 146 +++++++++---
> > drivers/media/platform/ti/cal/cal.c | 65 ++++--
> > drivers/media/platform/ti/cal/cal.h | 4 +-
> > 4 files changed, 342 insertions(+), 106 deletions(-)
> >
> > diff --git a/drivers/media/platform/ti/cal/cal-camerarx.c b/drivers/media/platform/ti/cal/cal-camerarx.c
> > index faafbd0e9240..49ae29065cd1 100644
> > --- a/drivers/media/platform/ti/cal/cal-camerarx.c
> > +++ b/drivers/media/platform/ti/cal/cal-camerarx.c
> > @@ -49,21 +49,41 @@ static s64 cal_camerarx_get_ext_link_freq(struct cal_camerarx *phy)
> > {
> > struct v4l2_mbus_config_mipi_csi2 *mipi_csi2 = &phy->endpoint.bus.mipi_csi2;
> > u32 num_lanes = mipi_csi2->num_data_lanes;
> > - const struct cal_format_info *fmtinfo;
> > struct v4l2_subdev_state *state;
> > - struct v4l2_mbus_framefmt *fmt;
> > u32 bpp;
> > s64 freq;
> >
> > - state = v4l2_subdev_get_locked_active_state(&phy->subdev);
> > + /*
> > + * v4l2_get_link_freq() uses V4L2_CID_LINK_FREQ first, and falls back
> > + * to V4L2_CID_PIXEL_RATE if V4L2_CID_LINK_FREQ is not available.
> > + *
> > + * With multistream input there is no single pixel rate, and thus we
> > + * cannot use V4L2_CID_PIXEL_RATE, so we pass 0 as the bpp which
> > + * causes v4l2_get_link_freq() to return an error if it falls back to
> > + * V4L2_CID_PIXEL_RATE.
> > + */
>
> To recap a bit of our offline discussion:
> - max9286 GMSL deserializer (as a comparison for a multiplexed
> transmitter) use PIXEL_RATE to report the cumulative pixel rate of
> enabled transmitters. This is because the R-Car CSI-2 receiver on
> which use PIXEL_RATE to compute the link freq [1]
>
> - according to [2]
> pixel_rate = link_freq * 2 * nr_of_lanes / bits_per_sample (on D-PHY)
>
> from which:
> link_freq = pixel_rate * bits_per_sample / (2 * nr_of_lanes)
>
> This works as long the reported pixel rate includes visible and
> blankings, something I'm not sure how many transmitters handle
> correctly as PIXEL_RATE control is meant to report the visible pixel
> sampling rate on the pixel array.
>
> I guess we should go towards mandating LINK_FREQ for transmitters.
>
> cc-Niklas for opinions on R-Car CSI-2 rcsi2_calc_mbps()
Thanks for the ping.
The choice to use the PIXEL_RATE instead of the LINK_FREQ control for
the R-Car CSI-2 was originally because the ADV748x which was the first
CSI-2 transmitter used during development.
AFIK the ADV748x adjusts the CSI-2 TX link frequency to match the pixel
clock. This results in quiet a big range of possible values that need to
be communicated between the two sub devices. The V4L2_CID_LINK_FREQ
control is a V4L2_CTRL_TYPE_INTEGER_MENU which do not render itself to
report the large range of values needed.
When we added MAX9286 and friends to the mix, we built on-top of this by
reporting the total pixel rate of all streams being transmitted on the
CSI-2 link. IMHO the v4l2_get_link_freq() was an OK middle ground on how
to align the two use-cases.
I agree that situation is not ideal. And in a perfect world a control
other then PIXEL_RATE would be used for the R-Car CSI-2 driver, but no
such control exists. And chancing the control type of LINK_FREQ is not a
good idea as that is usually specified in as a list in DT.
Adding a new control DYNAMIC_LINK_FREQ and wire that into
v4l2_get_link_freq() ?
>
> [1] https://elixir.bootlin.com/linux/v6.2/source/drivers/media/platform/renesas/rcar-vin/rcar-csi2.c#L608
> [2] https://www.kernel.org/doc/html/latest/driver-api/media/tx-rx.html#csi-2-transmitter-drivers
>
--
Kind Regards,
Niklas Söderlund
