On 16/07/2024 12:55, Jacopo Mondi wrote:
Hi Jai
On Tue, Jul 16, 2024 at 03:04:55PM GMT, Jai Luthra wrote:
Hi Jacopo,
Thanks for the review.
On Jul 12, 2024 at 18:09:48 +0200, Jacopo Mondi wrote:
Hi Jai
On Thu, Jun 27, 2024 at 06:40:06PM GMT, Jai Luthra wrote:
Cadence CSI-2 bridge IP supports capturing multiple virtual "streams"
of data over the same physical interface using MIPI Virtual Channels.
The V4L2 subdev APIs should reflect this capability and allow per-stream
routing and controls.
While the hardware IP supports usecases where streams coming in the sink
pad can be broadcasted to multiple source pads, the driver will need
significant re-architecture to make that possible. The two users of this
IP in mainline linux are TI Shim and StarFive JH7110 CAMSS, and both
have only integrated the first source pad i.e stream0 of this IP. So for
now keep it simple and only allow 1-to-1 mapping of streams from sink to
source, without any broadcasting.
With stream routing now supported in the driver, implement the
enable_stream and disable_stream hooks in place of the stream-unaware
s_stream hook.
This allows consumer devices like a DMA bridge or ISP, to enable
particular streams on a source pad, which in turn can be used to enable
only particular streams on the CSI-TX device connected on the sink pad.
Implement a fallback s_stream hook that internally calls enable_stream
on each source pad, for consumer drivers that don't use multi-stream
APIs to still work.
Signed-off-by: Jai Luthra <j-luthra@xxxxxx>
---
drivers/media/platform/cadence/cdns-csi2rx.c | 407 ++++++++++++++++++++-------
1 file changed, 313 insertions(+), 94 deletions(-)
diff --git a/drivers/media/platform/cadence/cdns-csi2rx.c b/drivers/media/platform/cadence/cdns-csi2rx.c
index 2ec34fc9c524..b0c91a9c65e8 100644
--- a/drivers/media/platform/cadence/cdns-csi2rx.c
+++ b/drivers/media/platform/cadence/cdns-csi2rx.c
@@ -90,6 +90,7 @@ struct csi2rx_priv {
struct reset_control *pixel_rst[CSI2RX_STREAMS_MAX];
struct phy *dphy;
+ u32 vc_select[CSI2RX_STREAMS_MAX];
u8 lanes[CSI2RX_LANES_MAX];
u8 num_lanes;
u8 max_lanes;
@@ -179,27 +180,43 @@ static void csi2rx_reset(struct csi2rx_priv *csi2rx)
static int csi2rx_configure_ext_dphy(struct csi2rx_priv *csi2rx)
{
+ struct v4l2_ctrl_handler *handler = csi2rx->source_subdev->ctrl_handler;
union phy_configure_opts opts = { };
struct phy_configure_opts_mipi_dphy *cfg = &opts.mipi_dphy;
- struct v4l2_subdev_format sd_fmt = {
- .which = V4L2_SUBDEV_FORMAT_ACTIVE,
- .pad = CSI2RX_PAD_SINK,
- };
+ struct v4l2_mbus_framefmt *framefmt;
+ struct v4l2_subdev_state *state;
const struct csi2rx_fmt *fmt;
s64 link_freq;
int ret;
- ret = v4l2_subdev_call_state_active(&csi2rx->subdev, pad, get_fmt,
- &sd_fmt);
- if (ret < 0)
- return ret;
+ if (v4l2_ctrl_find(handler, V4L2_CID_LINK_FREQ)) {
Do you need to do this by yourself ? afaict v4l2_get_link_freq()
already checks if V4L2_CID_LINK_FREQ is available, and if not,
fallsback to use PIXEL_RATE.
+ link_freq = v4l2_get_link_freq(handler, 0, 0);
+ } else {
+ state = v4l2_subdev_get_locked_active_state(&csi2rx->subdev);
+ framefmt = v4l2_subdev_state_get_format(state, CSI2RX_PAD_SINK,
+ 0);
+
+ if (framefmt) {
+ fmt = csi2rx_get_fmt_by_code(framefmt->code);
+ } else {
+ dev_err(csi2rx->dev,
+ "Did not find active sink format\n");
+ return -EINVAL;
Is this possibile ?
+ }
- fmt = csi2rx_get_fmt_by_code(sd_fmt.format.code);
+ link_freq = v4l2_get_link_freq(handler, fmt->bpp,
+ 2 * csi2rx->num_lanes);
Do we want to allow falling back on PIXEL_RATE for multiplexed
transmitters ? I presume this will give you invalid results anyway.
This is mostly done to avoid breaking any single stream sensor that does
not have the LINK_FREQ control, and was working with this bridge before.
Thus the warning below for multi-format sources.
Is it possible to allow usage of PIXEL_LINK only for non-multiplexed
transmitters ?
I would simply call v4l2_get_link_freq(handler, 0, 0) to force the
usage of LINK_FREQ which will become mandatory for transmitters
Ah I did not know LINK_FREQ will be mandatory soon. Any threads I can
look at where this was discussed?
I meant mandatory for multiplexed transmitters, which will have to be
'forced' to use LINK_FREQ as PIXEL_RATE doesn't make much sense for
them
In CAL driver's multiplexed streams patch I have:
/*
* 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.
*/
state = v4l2_subdev_get_locked_active_state(&phy->subdev);
if (state->routing.num_routes > 1) {
bpp = 0;
} else {
struct v4l2_subdev_route *route = &state->routing.routes[0];
const struct cal_format_info *fmtinfo;
struct v4l2_mbus_framefmt *fmt;
fmt = v4l2_subdev_state_get_format(state,
route->sink_pad, route->sink_stream);
fmtinfo = cal_format_by_code(fmt->code);
if (!fmtinfo)
return -EINVAL;
bpp = fmtinfo->bpp;
}
freq = v4l2_get_link_freq(phy->source->ctrl_handler, bpp, 2 * num_lanes);
if (freq < 0) {
phy_err(phy, "failed to get link freq for subdev '%s'\n",
phy->source->name);
return freq;
}
Tomi
