[PATCH 0/4] media: ti-vpe: cal: multiplexed streams support

[PATCH 0/4] media: ti-vpe: cal: multiplexed streams support

[Tomi Valkeinen]

Hi,

This series adds embedded data and multiplexed streams support to TI CAL
driver. These patches depend on the subdev routing series and CAL preparation
series:

https://lore.kernel.org/linux-media/20210427124523.990938-1-tomi.valkeinen@ideasonboard.com/

https://lore.kernel.org/linux-media/20210420120433.902394-1-tomi.valkeinen@ideasonboard.com/

One detail I'm not too happy about is adding a new kernel module parameter
'cal_streams_api'. Setting this parameter enables the multiplexed streams
support for the CAL sink pads. I'd like the driver to be able to figure this
out itself, but there's no simple answer to it.

A pad in a subdev has to be either in non-multiplexed or multiplexed mode. If
the subdev cannot support multiplexed streams, then the answer is clear. But if
it can, then the question becomes "what's connected to the pad, and what does
it want". Afaik, the idea with subdev configuration is that each subdev can be
configured independently, so the driver trying to figure out the answer to that
question breaks this idea. Also, two connected subdevs that both can support
both modes wouldn't even give the answer, but the answer might be found further
away, probably from the sensor subdev.

So, maybe it should be the userspace that can set a subdev pad to either of the
modes. There's currently no such uAPI, but I think this option makes the most
sense.

Another detail to note is the embedded data support. I don't have hardware with
real embedded data, so I have not added any embedded data formats. Also, the
embedded data is captured with the video capture ioctls, not the metadata
capture ioctls. The reason for this is that in the case of CSI-2 embedded data
(at least for this hardware) the embedded data is very much like pixel data:
the data is sent in lines just before or after the pixel content and each
subdev needs a set_fmt call with the width, height, and mbus-code.

Using the metadata ioctls is possible, but it only results in much more complex
driver code, and, I think, more confusing userspace.

 Tomi

Tomi Valkeinen (4):
  media: ti-vpe: cal: add embedded data support
  media: ti-vpe: cal: use frame desc to get vc and dt
  media: ti-vpe: cal: add routing support
  media: ti-vpe: cal: add multiplexed streams support

 drivers/media/platform/ti-vpe/cal-camerarx.c | 234 ++++++++++++++++---
 drivers/media/platform/ti-vpe/cal-video.c    |  95 +++++++-
 drivers/media/platform/ti-vpe/cal.c          | 102 ++++++--
 drivers/media/platform/ti-vpe/cal.h          |  10 +-
 4 files changed, 383 insertions(+), 58 deletions(-)

-- 
2.25.1

[PATCH 1/4] media: ti-vpe: cal: add embedded data support

[Tomi Valkeinen]

Add support for capturing embedded data from the sensor. The only
difference with capturing pixel data and embedded data is that we need
to ensure the PIX PROC is disabled for embedded data so that CAL doesn't
repack the data.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
 drivers/media/platform/ti-vpe/cal.c | 17 ++++++++++-------
 drivers/media/platform/ti-vpe/cal.h |  1 +
 2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/media/platform/ti-vpe/cal.c b/drivers/media/platform/ti-vpe/cal.c
index 62c45add4efe..fcc81024ae18 100644
--- a/drivers/media/platform/ti-vpe/cal.c
+++ b/drivers/media/platform/ti-vpe/cal.c
@@ -473,14 +473,17 @@ int cal_ctx_prepare(struct cal_ctx *ctx)
 {
 	int ret;
 
-	ret = cal_reserve_pix_proc(ctx->cal);
-	if (ret < 0) {
-		ctx_err(ctx, "Failed to reserve pix proc: %d\n", ret);
-		return ret;
-	}
+	ctx->use_pix_proc = !ctx->fmtinfo->meta;
+
+	if (ctx->use_pix_proc) {
+		ret = cal_reserve_pix_proc(ctx->cal);
+		if (ret < 0) {
+			ctx_err(ctx, "Failed to reserve pix proc: %d\n", ret);
+			return ret;
+		}
 
-	ctx->pix_proc = ret;
-	ctx->use_pix_proc = true;
+		ctx->pix_proc = ret;
+	}
 
 	return 0;
 }
diff --git a/drivers/media/platform/ti-vpe/cal.h b/drivers/media/platform/ti-vpe/cal.h
index 42a3f8004077..29b865d1a238 100644
--- a/drivers/media/platform/ti-vpe/cal.h
+++ b/drivers/media/platform/ti-vpe/cal.h
@@ -88,6 +88,7 @@ struct cal_format_info {
 	u32	code;
 	/* Bits per pixel */
 	u8	bpp;
+	bool	meta;
 };
 
 /* buffer for one video frame */
-- 
2.25.1

[PATCH 2/4] media: ti-vpe: cal: use frame desc to get vc and dt

[Tomi Valkeinen]

Use get_frame_desc() to get the frame desc from the connected source,
and use the provided virtual channel and datatype instead of hardcoded
ones.

get_frame_desc() works per stream, but as we don't support multiple
streams yet, we will just always use stream 0.

If the source doesn't support get_frame_desc(), fall back to the
previous method of always capturing virtual channel 0 and any datatype.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
 drivers/media/platform/ti-vpe/cal-camerarx.c | 26 +++++++++++
 drivers/media/platform/ti-vpe/cal.c          | 49 +++++++++++++++++++-
 drivers/media/platform/ti-vpe/cal.h          |  3 ++
 3 files changed, 76 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/ti-vpe/cal-camerarx.c b/drivers/media/platform/ti-vpe/cal-camerarx.c
index a4b783e038b5..36103f5af6e9 100644
--- a/drivers/media/platform/ti-vpe/cal-camerarx.c
+++ b/drivers/media/platform/ti-vpe/cal-camerarx.c
@@ -583,6 +583,32 @@ static int cal_camerarx_parse_dt(struct cal_camerarx *phy)
 	return ret;
 }
 
+int cal_camerarx_get_remote_frame_desc(struct cal_camerarx *phy,
+				       struct v4l2_mbus_frame_desc *fd)
+{
+	struct media_pad *pad;
+	int ret;
+
+	if (!phy->source)
+		return -ENODEV;
+
+	pad = media_entity_remote_pad(&phy->pads[CAL_CAMERARX_PAD_SINK]);
+	if (!pad)
+		return -ENODEV;
+
+	ret = v4l2_subdev_call(phy->source, pad, get_frame_desc, pad->index,
+			       fd);
+	if (ret)
+		return ret;
+
+	if (fd->type != V4L2_MBUS_FRAME_DESC_TYPE_CSI2) {
+		dev_err(phy->cal->dev, "Frame desc do not describe CSI-2 link");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 /* ------------------------------------------------------------------
  *	V4L2 Subdev Operations
  * ------------------------------------------------------------------
diff --git a/drivers/media/platform/ti-vpe/cal.c b/drivers/media/platform/ti-vpe/cal.c
index fcc81024ae18..7975bb449acd 100644
--- a/drivers/media/platform/ti-vpe/cal.c
+++ b/drivers/media/platform/ti-vpe/cal.c
@@ -469,10 +469,56 @@ static bool cal_ctx_wr_dma_stopped(struct cal_ctx *ctx)
 	return stopped;
 }
 
+static int
+cal_get_remote_frame_desc_entry(struct cal_camerarx *phy, u32 stream,
+				struct v4l2_mbus_frame_desc_entry *entry)
+{
+	struct v4l2_mbus_frame_desc fd;
+	unsigned int i;
+	int ret;
+
+	ret = cal_camerarx_get_remote_frame_desc(phy, &fd);
+	if (ret) {
+		if (ret != -ENOIOCTLCMD)
+			dev_err(phy->cal->dev,
+				"Failed to get remote frame desc: %d\n", ret);
+		return ret;
+	}
+
+	for (i = 0; i < fd.num_entries; i++) {
+		if (stream == fd.entry[i].stream) {
+			*entry = fd.entry[i];
+			return 0;
+		}
+	}
+
+	return -ENODEV;
+}
+
 int cal_ctx_prepare(struct cal_ctx *ctx)
 {
+	struct v4l2_mbus_frame_desc_entry entry;
 	int ret;
 
+	ret = cal_get_remote_frame_desc_entry(ctx->phy, ctx->stream, &entry);
+
+	if (ret == -ENOIOCTLCMD) {
+		ctx->vc = 0;
+		ctx->datatype = CAL_CSI2_CTX_DT_ANY;
+	} else if (!ret) {
+		ctx_dbg(2, ctx, "Framedesc: stream %u, len %u, vc %u, dt %#x\n",
+		       entry.stream,
+		       entry.length,
+		       entry.bus.csi2.vc,
+		       entry.bus.csi2.dt);
+
+		ctx->vc = entry.bus.csi2.vc;
+		ctx->datatype = entry.bus.csi2.dt;
+	} else {
+		ctx_err(ctx, "Failed to get remote frame desc: %d\n", ret);
+		return ret;
+	}
+
 	ctx->use_pix_proc = !ctx->fmtinfo->meta;
 
 	if (ctx->use_pix_proc) {
@@ -925,8 +971,7 @@ static struct cal_ctx *cal_ctx_create(struct cal_dev *cal, int inst)
 	ctx->dma_ctx = inst;
 	ctx->csi2_ctx = inst;
 	ctx->cport = inst;
-	ctx->vc = 0;
-	ctx->datatype = CAL_CSI2_CTX_DT_ANY;
+	ctx->stream = 0;
 
 	ret = cal_ctx_v4l2_init(ctx);
 	if (ret)
diff --git a/drivers/media/platform/ti-vpe/cal.h b/drivers/media/platform/ti-vpe/cal.h
index 29b865d1a238..3aea444f8bf8 100644
--- a/drivers/media/platform/ti-vpe/cal.h
+++ b/drivers/media/platform/ti-vpe/cal.h
@@ -245,6 +245,7 @@ struct cal_ctx {
 	u8			pix_proc;
 	u8			vc;
 	u8			datatype;
+	u32			stream;
 
 	bool			use_pix_proc;
 };
@@ -318,6 +319,8 @@ const struct cal_format_info *cal_format_by_code(u32 code);
 
 void cal_quickdump_regs(struct cal_dev *cal);
 
+int cal_camerarx_get_remote_frame_desc(struct cal_camerarx *phy,
+				       struct v4l2_mbus_frame_desc *fd);
 void cal_camerarx_disable(struct cal_camerarx *phy);
 void cal_camerarx_i913_errata(struct cal_camerarx *phy);
 struct cal_camerarx *cal_camerarx_create(struct cal_dev *cal,
-- 
2.25.1

[PATCH 3/4] media: ti-vpe: cal: add routing support

[Tomi Valkeinen]

Add routing support. As we still only support a single stream (i.e. a
single route), the routing is not really used for anything yet.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
 drivers/media/platform/ti-vpe/cal-camerarx.c | 64 ++++++++++++++++++++
 drivers/media/platform/ti-vpe/cal.h          |  2 +
 2 files changed, 66 insertions(+)

diff --git a/drivers/media/platform/ti-vpe/cal-camerarx.c b/drivers/media/platform/ti-vpe/cal-camerarx.c
index 36103f5af6e9..3470f6dc0ec1 100644
--- a/drivers/media/platform/ti-vpe/cal-camerarx.c
+++ b/drivers/media/platform/ti-vpe/cal-camerarx.c
@@ -805,6 +805,37 @@ static int cal_camerarx_sd_set_fmt(struct v4l2_subdev *sd,
 	return 0;
 }
 
+static int cal_camerarx_sd_get_routing(struct v4l2_subdev *sd,
+				       struct v4l2_subdev_krouting *routing)
+{
+	struct cal_camerarx *phy = to_cal_camerarx(sd);
+	struct v4l2_subdev_krouting *src;
+
+	src = &phy->routing;
+
+	if (routing->num_routes < src->num_routes) {
+		routing->num_routes = src->num_routes;
+		return -ENOSPC;
+	}
+
+	v4l2_subdev_cpy_routing(routing, src);
+
+	return 0;
+}
+
+static int cal_camerarx_sd_set_routing(struct v4l2_subdev *sd,
+				       struct v4l2_subdev_krouting *routing)
+{
+	struct cal_camerarx *phy = to_cal_camerarx(sd);
+	int ret;
+
+	ret = v4l2_subdev_dup_routing(&phy->routing, routing);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
 static int cal_camerarx_sd_init_cfg(struct v4l2_subdev *sd,
 				    struct v4l2_subdev_pad_config *cfg)
 {
@@ -837,6 +868,8 @@ static const struct v4l2_subdev_pad_ops cal_camerarx_pad_ops = {
 	.enum_frame_size = cal_camerarx_sd_enum_frame_size,
 	.get_fmt = cal_camerarx_sd_get_fmt,
 	.set_fmt = cal_camerarx_sd_set_fmt,
+	.get_routing = cal_camerarx_sd_get_routing,
+	.set_routing = cal_camerarx_sd_set_routing,
 };
 
 static const struct v4l2_subdev_ops cal_camerarx_subdev_ops = {
@@ -844,8 +877,18 @@ static const struct v4l2_subdev_ops cal_camerarx_subdev_ops = {
 	.pad = &cal_camerarx_pad_ops,
 };
 
+static bool cal_camerarx_has_route(struct media_entity *entity, unsigned int pad0,
+			  unsigned int pad1)
+{
+	struct v4l2_subdev *sd = media_entity_to_v4l2_subdev(entity);
+	struct cal_camerarx *phy = to_cal_camerarx(sd);
+
+	return v4l2_subdev_has_route(&phy->routing, pad0, pad1);
+}
+
 static struct media_entity_operations cal_camerarx_media_ops = {
 	.link_validate = v4l2_subdev_link_validate,
+	.has_route = cal_camerarx_has_route,
 };
 
 /* ------------------------------------------------------------------
@@ -862,6 +905,20 @@ struct cal_camerarx *cal_camerarx_create(struct cal_dev *cal,
 	int ret;
 	unsigned int i;
 
+	struct v4l2_subdev_route routes[] = { {
+		.sink_pad = 0,
+		.sink_stream = 0,
+		.source_pad = 1,
+		.source_stream = 0,
+		.flags = V4L2_SUBDEV_ROUTE_FL_ACTIVE,
+	} };
+
+	struct v4l2_subdev_krouting routing = {
+		.which = V4L2_SUBDEV_FORMAT_ACTIVE,
+		.num_routes = 1,
+		.routes = routes,
+	};
+
 	phy = kzalloc(sizeof(*phy), GFP_KERNEL);
 	if (!phy)
 		return ERR_PTR(-ENOMEM);
@@ -914,6 +971,11 @@ struct cal_camerarx *cal_camerarx_create(struct cal_dev *cal,
 	if (ret)
 		goto error;
 
+	/* Initialize routing to single route to the fist source pad */
+	ret = cal_camerarx_sd_set_routing(sd, &routing);
+	if (ret)
+		goto error;
+
 	ret = v4l2_device_register_subdev(&cal->v4l2_dev, sd);
 	if (ret)
 		goto error;
@@ -921,6 +983,7 @@ struct cal_camerarx *cal_camerarx_create(struct cal_dev *cal,
 	return phy;
 
 error:
+	v4l2_subdev_free_routing(&phy->routing);
 	media_entity_cleanup(&phy->subdev.entity);
 	kfree(phy);
 	return ERR_PTR(ret);
@@ -932,6 +995,7 @@ void cal_camerarx_destroy(struct cal_camerarx *phy)
 		return;
 
 	v4l2_device_unregister_subdev(&phy->subdev);
+	v4l2_subdev_free_routing(&phy->routing);
 	media_entity_cleanup(&phy->subdev.entity);
 	of_node_put(phy->source_ep_node);
 	of_node_put(phy->source_node);
diff --git a/drivers/media/platform/ti-vpe/cal.h b/drivers/media/platform/ti-vpe/cal.h
index 3aea444f8bf8..c96364eb0930 100644
--- a/drivers/media/platform/ti-vpe/cal.h
+++ b/drivers/media/platform/ti-vpe/cal.h
@@ -184,6 +184,8 @@ struct cal_camerarx {
 	struct mutex		mutex;
 
 	unsigned int enable_count;
+
+	struct v4l2_subdev_krouting routing;
 };
 
 struct cal_dev {
-- 
2.25.1

[PATCH 4/4] media: ti-vpe: cal: add multiplexed streams support

[Tomi Valkeinen]

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.

As multiplexed streams changes how the media entity pads behave, and
there's no simple way to decide if the driver should use the multiplexed
streams behavior or the non-multiplexed streams behavior, a new
'cal_streams_api' module parameter is added, which needs to be set to
use the multiplexed streams.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
 drivers/media/platform/ti-vpe/cal-camerarx.c | 144 +++++++++++++++----
 drivers/media/platform/ti-vpe/cal-video.c    |  95 ++++++++++--
 drivers/media/platform/ti-vpe/cal.c          |  40 ++++--
 drivers/media/platform/ti-vpe/cal.h          |   4 +-
 4 files changed, 232 insertions(+), 51 deletions(-)

diff --git a/drivers/media/platform/ti-vpe/cal-camerarx.c b/drivers/media/platform/ti-vpe/cal-camerarx.c
index 3470f6dc0ec1..40adf0b1f37a 100644
--- a/drivers/media/platform/ti-vpe/cal-camerarx.c
+++ b/drivers/media/platform/ti-vpe/cal-camerarx.c
@@ -49,15 +49,25 @@ static s64 cal_camerarx_get_ext_link_freq(struct cal_camerarx *phy)
 {
 	struct v4l2_fwnode_bus_mipi_csi2 *mipi_csi2 = &phy->endpoint.bus.mipi_csi2;
 	u32 num_lanes = mipi_csi2->num_data_lanes;
-	const struct cal_format_info *fmtinfo;
 	u32 bpp;
 	s64 freq;
 
-	fmtinfo = cal_format_by_code(phy->formats[CAL_CAMERARX_PAD_SINK].code);
-	if (!fmtinfo)
-		return -EINVAL;
+	/*
+	 * With multistream input we don't have bpp, and cannot use
+	 * V4L2_CID_PIXEL_RATE. Passing 0 as bpp causes v4l2_get_link_freq()
+	 * to return an error if it falls back to V4L2_CID_PIXEL_RATE.
+	 */
+	if (cal_streams_api) {
+		bpp = 0;
+	} else {
+		const struct cal_format_info *fmtinfo;
 
-	bpp = fmtinfo->bpp;
+		fmtinfo = cal_format_by_code(phy->formats[CAL_CAMERARX_PAD_SINK].code);
+		if (!fmtinfo)
+			return -EINVAL;
+
+		bpp = fmtinfo->bpp;
+	}
 
 	freq = v4l2_get_link_freq(phy->source->ctrl_handler, bpp, 2 * num_lanes);
 	if (freq < 0) {
@@ -619,6 +629,17 @@ static inline struct cal_camerarx *to_cal_camerarx(struct v4l2_subdev *sd)
 	return container_of(sd, struct cal_camerarx, subdev);
 }
 
+struct cal_camerarx *cal_camerarx_get_phy_from_entity(struct media_entity *entity)
+{
+	struct v4l2_subdev *sd;
+
+	sd = media_entity_to_v4l2_subdev(entity);
+	if (!sd)
+		return NULL;
+
+	return to_cal_camerarx(sd);
+}
+
 static struct v4l2_mbus_framefmt *
 cal_camerarx_get_pad_format(struct cal_camerarx *phy,
 			    struct v4l2_subdev_pad_config *cfg,
@@ -658,6 +679,18 @@ static int cal_camerarx_sd_enum_mbus_code(struct v4l2_subdev *sd,
 	struct cal_camerarx *phy = to_cal_camerarx(sd);
 	int r = 0;
 
+	if (cal_streams_api) {
+		if (cal_rx_pad_is_sink(code->pad))
+			return -ENOIOCTLCMD;
+
+		if (code->index >= cal_num_formats)
+			return -EINVAL;
+
+		code->code = cal_formats[code->index].code;
+
+		return 0;
+	}
+
 	mutex_lock(&phy->mutex);
 
 	/* No transcoding, source and sink codes must match. */
@@ -699,6 +732,22 @@ static int cal_camerarx_sd_enum_frame_size(struct v4l2_subdev *sd,
 	if (fse->index > 0)
 		return -EINVAL;
 
+	if (cal_streams_api) {
+		if (cal_rx_pad_is_sink(fse->pad))
+			return -ENOIOCTLCMD;
+
+		fmtinfo = cal_format_by_code(fse->code);
+		if (!fmtinfo)
+			return -EINVAL;
+
+		fse->min_width = CAL_MIN_WIDTH_BYTES * 8 / ALIGN(fmtinfo->bpp, 8);
+		fse->max_width = CAL_MAX_WIDTH_BYTES * 8 / ALIGN(fmtinfo->bpp, 8);
+		fse->min_height = CAL_MIN_HEIGHT_LINES;
+		fse->max_height = CAL_MAX_HEIGHT_LINES;
+
+		return 0;
+	}
+
 	mutex_lock(&phy->mutex);
 
 	/* No transcoding, source and sink formats must match. */
@@ -745,6 +794,9 @@ static int cal_camerarx_sd_get_fmt(struct v4l2_subdev *sd,
 	struct cal_camerarx *phy = to_cal_camerarx(sd);
 	struct v4l2_mbus_framefmt *fmt;
 
+	if (cal_streams_api && cal_rx_pad_is_sink(format->pad))
+		return -ENOIOCTLCMD;
+
 	mutex_lock(&phy->mutex);
 
 	fmt = cal_camerarx_get_pad_format(phy, cfg, format->pad, format->which);
@@ -764,8 +816,11 @@ static int cal_camerarx_sd_set_fmt(struct v4l2_subdev *sd,
 	struct v4l2_mbus_framefmt *fmt;
 	unsigned int bpp;
 
+	if (cal_streams_api && cal_rx_pad_is_sink(format->pad))
+		return -ENOIOCTLCMD;
+
 	/* No transcoding, source and sink formats must match. */
-	if (cal_rx_pad_is_source(format->pad))
+	if (!cal_streams_api && cal_rx_pad_is_source(format->pad))
 		return cal_camerarx_sd_get_fmt(sd, cfg, format);
 
 	/*
@@ -788,17 +843,27 @@ static int cal_camerarx_sd_set_fmt(struct v4l2_subdev *sd,
 	format->format.code = fmtinfo->code;
 	format->format.field = V4L2_FIELD_NONE;
 
-	/* Store the format and propagate it to the source pad. */
-
 	mutex_lock(&phy->mutex);
 
-	fmt = cal_camerarx_get_pad_format(phy, cfg, CAL_CAMERARX_PAD_SINK,
-					  format->which);
-	*fmt = format->format;
+	if (!cal_streams_api) {
+		unsigned int i;
+
+		/* Store the format and propagate it to the source pads. */
 
-	fmt = cal_camerarx_get_pad_format(phy, cfg, CAL_CAMERARX_PAD_FIRST_SOURCE,
-					  format->which);
-	*fmt = format->format;
+		fmt = cal_camerarx_get_pad_format(phy, cfg, CAL_CAMERARX_PAD_SINK,
+						  format->which);
+		*fmt = format->format;
+
+		for (i = CAL_CAMERARX_PAD_FIRST_SOURCE; i < CAL_CAMERARX_NUM_PADS; ++i) {
+			fmt = cal_camerarx_get_pad_format(phy, cfg, i, format->which);
+			*fmt = format->format;
+		}
+	} else {
+		/* Store the format */
+
+		fmt = cal_camerarx_get_pad_format(phy, cfg, format->pad, format->which);
+		*fmt = format->format;
+	}
 
 	mutex_unlock(&phy->mutex);
 
@@ -839,23 +904,38 @@ static int cal_camerarx_sd_set_routing(struct v4l2_subdev *sd,
 static int cal_camerarx_sd_init_cfg(struct v4l2_subdev *sd,
 				    struct v4l2_subdev_pad_config *cfg)
 {
-	struct v4l2_subdev_format format = {
-		.which = cfg ? V4L2_SUBDEV_FORMAT_TRY
-		       : V4L2_SUBDEV_FORMAT_ACTIVE,
-		.pad = CAL_CAMERARX_PAD_SINK,
-		.format = {
-			.width = 640,
-			.height = 480,
-			.code = MEDIA_BUS_FMT_UYVY8_2X8,
-			.field = V4L2_FIELD_NONE,
-			.colorspace = V4L2_COLORSPACE_SRGB,
-			.ycbcr_enc = V4L2_YCBCR_ENC_601,
-			.quantization = V4L2_QUANTIZATION_LIM_RANGE,
-			.xfer_func = V4L2_XFER_FUNC_SRGB,
-		},
-	};
+	int i;
+	int ret;
+	u32 which = cfg ? V4L2_SUBDEV_FORMAT_TRY : V4L2_SUBDEV_FORMAT_ACTIVE;
+
+	for (i = 0; i < CAL_CAMERARX_NUM_PADS; ++i) {
+		struct v4l2_subdev_format format = {
+			.which = which,
+			.pad = i,
+			.format = {
+				.width = 640,
+				.height = 480,
+				.code = MEDIA_BUS_FMT_UYVY8_2X8,
+				.field = V4L2_FIELD_NONE,
+				.colorspace = V4L2_COLORSPACE_SRGB,
+				.ycbcr_enc = V4L2_YCBCR_ENC_601,
+				.quantization = V4L2_QUANTIZATION_LIM_RANGE,
+				.xfer_func = V4L2_XFER_FUNC_SRGB,
+			},
+		};
+
+		if (!cal_streams_api && cal_rx_pad_is_source(i))
+			continue;
 
-	return cal_camerarx_sd_set_fmt(sd, cfg, &format);
+		if (cal_streams_api && cal_rx_pad_is_sink(i))
+			continue;
+
+		ret = cal_camerarx_sd_set_fmt(sd, cfg, &format);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
 }
 
 static const struct v4l2_subdev_video_ops cal_camerarx_video_ops = {
@@ -959,6 +1039,10 @@ struct cal_camerarx *cal_camerarx_create(struct cal_dev *cal,
 	sd->dev = cal->dev;
 
 	phy->pads[CAL_CAMERARX_PAD_SINK].flags = MEDIA_PAD_FL_SINK;
+
+	if (cal_streams_api)
+		phy->pads[CAL_CAMERARX_PAD_SINK].flags |= MEDIA_PAD_FL_MULTIPLEXED;
+
 	for (i = CAL_CAMERARX_PAD_FIRST_SOURCE; i < CAL_CAMERARX_NUM_PADS; ++i)
 		phy->pads[i].flags = MEDIA_PAD_FL_SOURCE;
 	sd->entity.ops = &cal_camerarx_media_ops;
diff --git a/drivers/media/platform/ti-vpe/cal-video.c b/drivers/media/platform/ti-vpe/cal-video.c
index 8ecae7dc2774..097ebd1bfb2f 100644
--- a/drivers/media/platform/ti-vpe/cal-video.c
+++ b/drivers/media/platform/ti-vpe/cal-video.c
@@ -711,6 +711,46 @@ static int cal_start_streaming(struct vb2_queue *vq, unsigned int count)
 	dma_addr_t addr;
 	int ret;
 
+	if (cal_mc_api) {
+		struct v4l2_subdev_route *route = NULL;
+		struct media_pad *remote_pad;
+		unsigned int i;
+
+		/* Find the PHY connected to this video device */
+
+		remote_pad = media_entity_remote_pad(&ctx->pad);
+		if (!remote_pad) {
+			ctx_err(ctx, "Context not connected\n");
+			return -ENODEV;
+		}
+
+		ctx->phy = cal_camerarx_get_phy_from_entity(remote_pad->entity);
+
+		/* Find the stream */
+
+		for (i = 0; i < ctx->phy->routing.num_routes; ++i) {
+			struct v4l2_subdev_route *r =
+				&ctx->phy->routing.routes[i];
+
+			if (!(r->flags & V4L2_SUBDEV_ROUTE_FL_ACTIVE))
+				continue;
+
+			if (r->source_pad != remote_pad->index)
+				continue;
+
+			route = r;
+
+			break;
+		}
+
+		if (!route) {
+			ctx_err(ctx, "Failed to find route\n");
+			return -ENODEV;
+		}
+
+		ctx->stream = route->sink_stream;
+	}
+
 	ret = media_pipeline_start(ctx->vdev.entity.pads, &ctx->phy->pipe);
 	if (ret < 0) {
 		ctx_err(ctx, "Failed to start media pipeline: %d\n", ret);
@@ -784,6 +824,9 @@ static void cal_stop_streaming(struct vb2_queue *vq)
 	cal_release_buffers(ctx, VB2_BUF_STATE_ERROR);
 
 	media_pipeline_stop(ctx->vdev.entity.pads);
+
+	if (cal_mc_api)
+		ctx->phy = NULL;
 }
 
 static const struct vb2_ops cal_video_qops = {
@@ -945,16 +988,48 @@ int cal_ctx_v4l2_register(struct cal_ctx *ctx)
 		return ret;
 	}
 
-	ret = media_create_pad_link(&ctx->phy->subdev.entity,
-				    CAL_CAMERARX_PAD_FIRST_SOURCE,
-				    &vfd->entity, 0,
-				    MEDIA_LNK_FL_IMMUTABLE |
-				    MEDIA_LNK_FL_ENABLED);
-	if (ret) {
-		ctx_err(ctx, "Failed to create media link for context %u\n",
-			ctx->dma_ctx);
-		video_unregister_device(vfd);
-		return ret;
+	if (cal_mc_api) {
+		u16 phy_idx;
+		u16 pad_idx;
+
+		/* Create links from all video nodes to all PHYs */
+
+		for (phy_idx = 0; phy_idx < ctx->cal->data->num_csi2_phy; ++phy_idx) {
+			for (pad_idx = 1; pad_idx < CAL_CAMERARX_NUM_PADS; ++pad_idx) {
+				/*
+				 * Enable only links from video0 to PHY0 pad 1, and
+				 * video1 to PHY1 pad 1.
+				 */
+				bool enable = (ctx->dma_ctx == 0 &&
+					       phy_idx == 0 && pad_idx == 1) ||
+					      (ctx->dma_ctx == 1 &&
+					       phy_idx == 1 && pad_idx == 1);
+
+				ret = media_create_pad_link(
+					&ctx->cal->phy[phy_idx]->subdev.entity,
+					pad_idx, &vfd->entity, 0,
+					enable ? MEDIA_LNK_FL_ENABLED : 0);
+				if (ret) {
+					ctx_err(ctx,
+						"Failed to create media link for context %u\n",
+						ctx->dma_ctx);
+					video_unregister_device(vfd);
+					return ret;
+				}
+			}
+		}
+	} else {
+		ret = media_create_pad_link(
+			&ctx->phy->subdev.entity, CAL_CAMERARX_PAD_FIRST_SOURCE,
+			&vfd->entity, 0,
+			MEDIA_LNK_FL_IMMUTABLE | MEDIA_LNK_FL_ENABLED);
+		if (ret) {
+			ctx_err(ctx,
+				"Failed to create media link for context %u\n",
+				ctx->dma_ctx);
+			video_unregister_device(vfd);
+			return ret;
+		}
 	}
 
 	ctx_info(ctx, "V4L2 device registered as %s\n",
diff --git a/drivers/media/platform/ti-vpe/cal.c b/drivers/media/platform/ti-vpe/cal.c
index 7975bb449acd..42ad3ade64c1 100644
--- a/drivers/media/platform/ti-vpe/cal.c
+++ b/drivers/media/platform/ti-vpe/cal.c
@@ -53,6 +53,12 @@ bool cal_mc_api = CAL_MC_API_DEFAULT;
 module_param_named(mc_api, cal_mc_api, bool, 0444);
 MODULE_PARM_DESC(mc_api, "activates the MC API");
 
+bool cal_streams_api;
+module_param_named(streams_api, cal_streams_api, bool, 0444);
+MODULE_PARM_DESC(streams_api, "activates the multiplexed streams API");
+
+static struct cal_ctx *cal_ctx_create(struct cal_dev *cal, int inst);
+
 /* ------------------------------------------------------------------
  *	Format Handling
  * ------------------------------------------------------------------
@@ -967,7 +973,6 @@ static struct cal_ctx *cal_ctx_create(struct cal_dev *cal, int inst)
 		return NULL;
 
 	ctx->cal = cal;
-	ctx->phy = cal->phy[inst];
 	ctx->dma_ctx = inst;
 	ctx->csi2_ctx = inst;
 	ctx->cport = inst;
@@ -1178,18 +1183,33 @@ static int cal_probe(struct platform_device *pdev)
 	}
 
 	/* Create contexts. */
-	for (i = 0; i < cal->data->num_csi2_phy; ++i) {
-		if (!cal->phy[i]->source_node)
-			continue;
+	if (!cal_mc_api) {
+		for (i = 0; i < cal->data->num_csi2_phy; ++i) {
+			if (!cal->phy[i]->source_node)
+				continue;
+
+			cal->ctx[i] = cal_ctx_create(cal, i);
+			if (!cal->ctx[i]) {
+				cal_err(cal, "Failed to create context %u\n", i);
+				ret = -ENODEV;
+				goto error_context;
+			}
+
+			cal->ctx[i]->phy = cal->phy[i];
 
-		cal->ctx[i] = cal_ctx_create(cal, i);
-		if (!cal->ctx[i]) {
-			cal_err(cal, "Failed to create context %u\n", i);
-			ret = -ENODEV;
-			goto error_context;
+			cal->num_contexts++;
 		}
+	} else {
+		for (i = 0; i < ARRAY_SIZE(cal->ctx); ++i) {
+			cal->ctx[i] = cal_ctx_create(cal, i);
+			if (!cal->ctx[i]) {
+				cal_err(cal, "Failed to create context %u\n", i);
+				ret = -ENODEV;
+				goto error_context;
+			}
 
-		cal->num_contexts++;
+			cal->num_contexts++;
+		}
 	}
 
 	/* Register the media device. */
diff --git a/drivers/media/platform/ti-vpe/cal.h b/drivers/media/platform/ti-vpe/cal.h
index c96364eb0930..fc6d68195367 100644
--- a/drivers/media/platform/ti-vpe/cal.h
+++ b/drivers/media/platform/ti-vpe/cal.h
@@ -45,7 +45,7 @@
 
 #define CAL_CAMERARX_PAD_SINK		0
 #define CAL_CAMERARX_PAD_FIRST_SOURCE	1
-#define CAL_CAMERARX_NUM_SOURCE_PADS	1
+#define CAL_CAMERARX_NUM_SOURCE_PADS	8
 #define CAL_CAMERARX_NUM_PADS		(1 + CAL_CAMERARX_NUM_SOURCE_PADS)
 
 static inline bool cal_rx_pad_is_sink(u32 pad)
@@ -255,6 +255,7 @@ struct cal_ctx {
 extern unsigned int cal_debug;
 extern int cal_video_nr;
 extern bool cal_mc_api;
+extern bool cal_streams_api;
 
 #define cal_dbg(level, cal, fmt, arg...)				\
 	do {								\
@@ -323,6 +324,7 @@ void cal_quickdump_regs(struct cal_dev *cal);
 
 int cal_camerarx_get_remote_frame_desc(struct cal_camerarx *phy,
 				       struct v4l2_mbus_frame_desc *fd);
+struct cal_camerarx *cal_camerarx_get_phy_from_entity(struct media_entity *entity);
 void cal_camerarx_disable(struct cal_camerarx *phy);
 void cal_camerarx_i913_errata(struct cal_camerarx *phy);
 struct cal_camerarx *cal_camerarx_create(struct cal_dev *cal,
-- 
2.25.1

Re: [PATCH 1/4] media: ti-vpe: cal: add embedded data support

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Tue, Apr 27, 2021 at 04:20:25PM +0300, Tomi Valkeinen wrote:
> Add support for capturing embedded data from the sensor. The only
> difference with capturing pixel data and embedded data is that we need
> to ensure the PIX PROC is disabled for embedded data so that CAL doesn't
> repack the data.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/media/platform/ti-vpe/cal.c | 17 ++++++++++-------
>  drivers/media/platform/ti-vpe/cal.h |  1 +
>  2 files changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/media/platform/ti-vpe/cal.c b/drivers/media/platform/ti-vpe/cal.c
> index 62c45add4efe..fcc81024ae18 100644
> --- a/drivers/media/platform/ti-vpe/cal.c
> +++ b/drivers/media/platform/ti-vpe/cal.c
> @@ -473,14 +473,17 @@ int cal_ctx_prepare(struct cal_ctx *ctx)
>  {
>  	int ret;
>  
> -	ret = cal_reserve_pix_proc(ctx->cal);
> -	if (ret < 0) {
> -		ctx_err(ctx, "Failed to reserve pix proc: %d\n", ret);
> -		return ret;
> -	}
> +	ctx->use_pix_proc = !ctx->fmtinfo->meta;
> +
> +	if (ctx->use_pix_proc) {
> +		ret = cal_reserve_pix_proc(ctx->cal);
> +		if (ret < 0) {
> +			ctx_err(ctx, "Failed to reserve pix proc: %d\n", ret);
> +			return ret;
> +		}
>  
> -	ctx->pix_proc = ret;
> -	ctx->use_pix_proc = true;
> +		ctx->pix_proc = ret;
> +	}
>  
>  	return 0;
>  }
> diff --git a/drivers/media/platform/ti-vpe/cal.h b/drivers/media/platform/ti-vpe/cal.h
> index 42a3f8004077..29b865d1a238 100644
> --- a/drivers/media/platform/ti-vpe/cal.h
> +++ b/drivers/media/platform/ti-vpe/cal.h
> @@ -88,6 +88,7 @@ struct cal_format_info {
>  	u32	code;
>  	/* Bits per pixel */
>  	u8	bpp;
> +	bool	meta;
>  };
>  
>  /* buffer for one video frame */

-- 
Regards,

Laurent Pinchart

Re: [PATCH 2/4] media: ti-vpe: cal: use frame desc to get vc and dt

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Tue, Apr 27, 2021 at 04:20:26PM +0300, Tomi Valkeinen wrote:
> Use get_frame_desc() to get the frame desc from the connected source,
> and use the provided virtual channel and datatype instead of hardcoded
> ones.
> 
> get_frame_desc() works per stream, but as we don't support multiple
> streams yet, we will just always use stream 0.
> 
> If the source doesn't support get_frame_desc(), fall back to the
> previous method of always capturing virtual channel 0 and any datatype.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> ---
>  drivers/media/platform/ti-vpe/cal-camerarx.c | 26 +++++++++++
>  drivers/media/platform/ti-vpe/cal.c          | 49 +++++++++++++++++++-
>  drivers/media/platform/ti-vpe/cal.h          |  3 ++
>  3 files changed, 76 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/platform/ti-vpe/cal-camerarx.c b/drivers/media/platform/ti-vpe/cal-camerarx.c
> index a4b783e038b5..36103f5af6e9 100644
> --- a/drivers/media/platform/ti-vpe/cal-camerarx.c
> +++ b/drivers/media/platform/ti-vpe/cal-camerarx.c
> @@ -583,6 +583,32 @@ static int cal_camerarx_parse_dt(struct cal_camerarx *phy)
>  	return ret;
>  }
>  
> +int cal_camerarx_get_remote_frame_desc(struct cal_camerarx *phy,
> +				       struct v4l2_mbus_frame_desc *fd)
> +{
> +	struct media_pad *pad;
> +	int ret;
> +
> +	if (!phy->source)
> +		return -ENODEV;
> +
> +	pad = media_entity_remote_pad(&phy->pads[CAL_CAMERARX_PAD_SINK]);
> +	if (!pad)
> +		return -ENODEV;
> +
> +	ret = v4l2_subdev_call(phy->source, pad, get_frame_desc, pad->index,
> +			       fd);
> +	if (ret)
> +		return ret;
> +
> +	if (fd->type != V4L2_MBUS_FRAME_DESC_TYPE_CSI2) {
> +		dev_err(phy->cal->dev, "Frame desc do not describe CSI-2 link");
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
>  /* ------------------------------------------------------------------
>   *	V4L2 Subdev Operations
>   * ------------------------------------------------------------------
> diff --git a/drivers/media/platform/ti-vpe/cal.c b/drivers/media/platform/ti-vpe/cal.c
> index fcc81024ae18..7975bb449acd 100644
> --- a/drivers/media/platform/ti-vpe/cal.c
> +++ b/drivers/media/platform/ti-vpe/cal.c
> @@ -469,10 +469,56 @@ static bool cal_ctx_wr_dma_stopped(struct cal_ctx *ctx)
>  	return stopped;
>  }
>  
> +static int
> +cal_get_remote_frame_desc_entry(struct cal_camerarx *phy, u32 stream,

As this isn't a camerarx function, would it be better to pass the
cal_ctx pointer instead ?

> +				struct v4l2_mbus_frame_desc_entry *entry)
> +{
> +	struct v4l2_mbus_frame_desc fd;
> +	unsigned int i;
> +	int ret;
> +
> +	ret = cal_camerarx_get_remote_frame_desc(phy, &fd);
> +	if (ret) {
> +		if (ret != -ENOIOCTLCMD)
> +			dev_err(phy->cal->dev,
> +				"Failed to get remote frame desc: %d\n", ret);

Should we drop this message as the caller also prints one ?

> +		return ret;
> +	}
> +
> +	for (i = 0; i < fd.num_entries; i++) {
> +		if (stream == fd.entry[i].stream) {
> +			*entry = fd.entry[i];
> +			return 0;
> +		}
> +	}
> +
> +	return -ENODEV;
> +}
> +
>  int cal_ctx_prepare(struct cal_ctx *ctx)
>  {
> +	struct v4l2_mbus_frame_desc_entry entry;
>  	int ret;
>  
> +	ret = cal_get_remote_frame_desc_entry(ctx->phy, ctx->stream, &entry);
> +
> +	if (ret == -ENOIOCTLCMD) {
> +		ctx->vc = 0;
> +		ctx->datatype = CAL_CSI2_CTX_DT_ANY;

I'd have moved this after the !ret case, but that's very personal.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> +	} else if (!ret) {
> +		ctx_dbg(2, ctx, "Framedesc: stream %u, len %u, vc %u, dt %#x\n",
> +		       entry.stream,
> +		       entry.length,
> +		       entry.bus.csi2.vc,
> +		       entry.bus.csi2.dt);
> +
> +		ctx->vc = entry.bus.csi2.vc;
> +		ctx->datatype = entry.bus.csi2.dt;
> +	} else {
> +		ctx_err(ctx, "Failed to get remote frame desc: %d\n", ret);
> +		return ret;
> +	}
> +
>  	ctx->use_pix_proc = !ctx->fmtinfo->meta;
>  
>  	if (ctx->use_pix_proc) {
> @@ -925,8 +971,7 @@ static struct cal_ctx *cal_ctx_create(struct cal_dev *cal, int inst)
>  	ctx->dma_ctx = inst;
>  	ctx->csi2_ctx = inst;
>  	ctx->cport = inst;
> -	ctx->vc = 0;
> -	ctx->datatype = CAL_CSI2_CTX_DT_ANY;
> +	ctx->stream = 0;
>  
>  	ret = cal_ctx_v4l2_init(ctx);
>  	if (ret)
> diff --git a/drivers/media/platform/ti-vpe/cal.h b/drivers/media/platform/ti-vpe/cal.h
> index 29b865d1a238..3aea444f8bf8 100644
> --- a/drivers/media/platform/ti-vpe/cal.h
> +++ b/drivers/media/platform/ti-vpe/cal.h
> @@ -245,6 +245,7 @@ struct cal_ctx {
>  	u8			pix_proc;
>  	u8			vc;
>  	u8			datatype;
> +	u32			stream;
>  
>  	bool			use_pix_proc;
>  };
> @@ -318,6 +319,8 @@ const struct cal_format_info *cal_format_by_code(u32 code);
>  
>  void cal_quickdump_regs(struct cal_dev *cal);
>  
> +int cal_camerarx_get_remote_frame_desc(struct cal_camerarx *phy,
> +				       struct v4l2_mbus_frame_desc *fd);
>  void cal_camerarx_disable(struct cal_camerarx *phy);
>  void cal_camerarx_i913_errata(struct cal_camerarx *phy);
>  struct cal_camerarx *cal_camerarx_create(struct cal_dev *cal,

-- 
Regards,

Laurent Pinchart

Re: [PATCH 3/4] media: ti-vpe: cal: add routing support

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Tue, Apr 27, 2021 at 04:20:27PM +0300, Tomi Valkeinen wrote:
> Add routing support. As we still only support a single stream (i.e. a
> single route), the routing is not really used for anything yet.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> ---
>  drivers/media/platform/ti-vpe/cal-camerarx.c | 64 ++++++++++++++++++++
>  drivers/media/platform/ti-vpe/cal.h          |  2 +
>  2 files changed, 66 insertions(+)
> 
> diff --git a/drivers/media/platform/ti-vpe/cal-camerarx.c b/drivers/media/platform/ti-vpe/cal-camerarx.c
> index 36103f5af6e9..3470f6dc0ec1 100644
> --- a/drivers/media/platform/ti-vpe/cal-camerarx.c
> +++ b/drivers/media/platform/ti-vpe/cal-camerarx.c
> @@ -805,6 +805,37 @@ static int cal_camerarx_sd_set_fmt(struct v4l2_subdev *sd,
>  	return 0;
>  }
>  
> +static int cal_camerarx_sd_get_routing(struct v4l2_subdev *sd,
> +				       struct v4l2_subdev_krouting *routing)
> +{
> +	struct cal_camerarx *phy = to_cal_camerarx(sd);
> +	struct v4l2_subdev_krouting *src;

const, as src isn't modified.

> +
> +	src = &phy->routing;

That could be written

	const struct v4l2_subdev_krouting *src = &phy->routing;

> +
> +	if (routing->num_routes < src->num_routes) {
> +		routing->num_routes = src->num_routes;
> +		return -ENOSPC;
> +	}
> +
> +	v4l2_subdev_cpy_routing(routing, src);
> +
> +	return 0;
> +}
> +
> +static int cal_camerarx_sd_set_routing(struct v4l2_subdev *sd,
> +				       struct v4l2_subdev_krouting *routing)
> +{
> +	struct cal_camerarx *phy = to_cal_camerarx(sd);
> +	int ret;

Shouldn't you reject !ACTIVE configs ? Same in
cal_camerarx_sd_get_routing() I suppose.

> +
> +	ret = v4l2_subdev_dup_routing(&phy->routing, routing);
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +
>  static int cal_camerarx_sd_init_cfg(struct v4l2_subdev *sd,
>  				    struct v4l2_subdev_pad_config *cfg)
>  {
> @@ -837,6 +868,8 @@ static const struct v4l2_subdev_pad_ops cal_camerarx_pad_ops = {
>  	.enum_frame_size = cal_camerarx_sd_enum_frame_size,
>  	.get_fmt = cal_camerarx_sd_get_fmt,
>  	.set_fmt = cal_camerarx_sd_set_fmt,
> +	.get_routing = cal_camerarx_sd_get_routing,
> +	.set_routing = cal_camerarx_sd_set_routing,
>  };
>  
>  static const struct v4l2_subdev_ops cal_camerarx_subdev_ops = {
> @@ -844,8 +877,18 @@ static const struct v4l2_subdev_ops cal_camerarx_subdev_ops = {
>  	.pad = &cal_camerarx_pad_ops,
>  };
>  
> +static bool cal_camerarx_has_route(struct media_entity *entity, unsigned int pad0,
> +			  unsigned int pad1)
> +{
> +	struct v4l2_subdev *sd = media_entity_to_v4l2_subdev(entity);
> +	struct cal_camerarx *phy = to_cal_camerarx(sd);
> +
> +	return v4l2_subdev_has_route(&phy->routing, pad0, pad1);
> +}
> +
>  static struct media_entity_operations cal_camerarx_media_ops = {
>  	.link_validate = v4l2_subdev_link_validate,
> +	.has_route = cal_camerarx_has_route,
>  };
>  
>  /* ------------------------------------------------------------------
> @@ -862,6 +905,20 @@ struct cal_camerarx *cal_camerarx_create(struct cal_dev *cal,
>  	int ret;
>  	unsigned int i;
>  
> +	struct v4l2_subdev_route routes[] = { {
> +		.sink_pad = 0,
> +		.sink_stream = 0,
> +		.source_pad = 1,
> +		.source_stream = 0,
> +		.flags = V4L2_SUBDEV_ROUTE_FL_ACTIVE,
> +	} };
> +
> +	struct v4l2_subdev_krouting routing = {
> +		.which = V4L2_SUBDEV_FORMAT_ACTIVE,
> +		.num_routes = 1,
> +		.routes = routes,
> +	};

I'd move this above the other variables. Blank lines are not customary.

> +
>  	phy = kzalloc(sizeof(*phy), GFP_KERNEL);
>  	if (!phy)
>  		return ERR_PTR(-ENOMEM);
> @@ -914,6 +971,11 @@ struct cal_camerarx *cal_camerarx_create(struct cal_dev *cal,
>  	if (ret)
>  		goto error;
>  
> +	/* Initialize routing to single route to the fist source pad */

s/fist/first/
s/pad/pad./

> +	ret = cal_camerarx_sd_set_routing(sd, &routing);
> +	if (ret)
> +		goto error;
> +
>  	ret = v4l2_device_register_subdev(&cal->v4l2_dev, sd);
>  	if (ret)
>  		goto error;
> @@ -921,6 +983,7 @@ struct cal_camerarx *cal_camerarx_create(struct cal_dev *cal,
>  	return phy;
>  
>  error:
> +	v4l2_subdev_free_routing(&phy->routing);
>  	media_entity_cleanup(&phy->subdev.entity);
>  	kfree(phy);
>  	return ERR_PTR(ret);
> @@ -932,6 +995,7 @@ void cal_camerarx_destroy(struct cal_camerarx *phy)
>  		return;
>  
>  	v4l2_device_unregister_subdev(&phy->subdev);
> +	v4l2_subdev_free_routing(&phy->routing);
>  	media_entity_cleanup(&phy->subdev.entity);
>  	of_node_put(phy->source_ep_node);
>  	of_node_put(phy->source_node);
> diff --git a/drivers/media/platform/ti-vpe/cal.h b/drivers/media/platform/ti-vpe/cal.h
> index 3aea444f8bf8..c96364eb0930 100644
> --- a/drivers/media/platform/ti-vpe/cal.h
> +++ b/drivers/media/platform/ti-vpe/cal.h
> @@ -184,6 +184,8 @@ struct cal_camerarx {
>  	struct mutex		mutex;
>  
>  	unsigned int enable_count;
> +
> +	struct v4l2_subdev_krouting routing;

Looking forward to storing this in v4l2_subdev_config, and embedding an
instance of v4l2_subdev_config in v4l2_subdev :-)
cal_camerarx_has_route() could then be replaced by a generic V4L2 subdev
helper, and so could cal_camerarx_sd_get_routing().

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

>  };
>  
>  struct cal_dev {

-- 
Regards,

Laurent Pinchart

Re: [PATCH 3/4] media: ti-vpe: cal: add routing support

[Laurent Pinchart]

Hi Tomi,

One more comment.

On Thu, Apr 29, 2021 at 06:27:01AM +0300, Laurent Pinchart wrote:
> On Tue, Apr 27, 2021 at 04:20:27PM +0300, Tomi Valkeinen wrote:
> > Add routing support. As we still only support a single stream (i.e. a
> > single route), the routing is not really used for anything yet.
> > 
> > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> > ---
> >  drivers/media/platform/ti-vpe/cal-camerarx.c | 64 ++++++++++++++++++++
> >  drivers/media/platform/ti-vpe/cal.h          |  2 +
> >  2 files changed, 66 insertions(+)
> > 
> > diff --git a/drivers/media/platform/ti-vpe/cal-camerarx.c b/drivers/media/platform/ti-vpe/cal-camerarx.c
> > index 36103f5af6e9..3470f6dc0ec1 100644
> > --- a/drivers/media/platform/ti-vpe/cal-camerarx.c
> > +++ b/drivers/media/platform/ti-vpe/cal-camerarx.c
> > @@ -805,6 +805,37 @@ static int cal_camerarx_sd_set_fmt(struct v4l2_subdev *sd,
> >  	return 0;
> >  }
> >  
> > +static int cal_camerarx_sd_get_routing(struct v4l2_subdev *sd,
> > +				       struct v4l2_subdev_krouting *routing)
> > +{
> > +	struct cal_camerarx *phy = to_cal_camerarx(sd);
> > +	struct v4l2_subdev_krouting *src;
> 
> const, as src isn't modified.
> 
> > +
> > +	src = &phy->routing;
> 
> That could be written
> 
> 	const struct v4l2_subdev_krouting *src = &phy->routing;
> 
> > +
> > +	if (routing->num_routes < src->num_routes) {
> > +		routing->num_routes = src->num_routes;
> > +		return -ENOSPC;
> > +	}
> > +
> > +	v4l2_subdev_cpy_routing(routing, src);
> > +
> > +	return 0;
> > +}
> > +
> > +static int cal_camerarx_sd_set_routing(struct v4l2_subdev *sd,
> > +				       struct v4l2_subdev_krouting *routing)
> > +{
> > +	struct cal_camerarx *phy = to_cal_camerarx(sd);
> > +	int ret;
> 
> Shouldn't you reject !ACTIVE configs ? Same in
> cal_camerarx_sd_get_routing() I suppose.
> 
> > +
> > +	ret = v4l2_subdev_dup_routing(&phy->routing, routing);
> > +	if (ret)
> > +		return ret;

Do we need locking here ? The get and set should be serialized, and I
also think we don't want to allow routing configuration during
streaming, at least to start with.

> > +
> > +	return 0;
> > +}
> > +
> >  static int cal_camerarx_sd_init_cfg(struct v4l2_subdev *sd,
> >  				    struct v4l2_subdev_pad_config *cfg)
> >  {
> > @@ -837,6 +868,8 @@ static const struct v4l2_subdev_pad_ops cal_camerarx_pad_ops = {
> >  	.enum_frame_size = cal_camerarx_sd_enum_frame_size,
> >  	.get_fmt = cal_camerarx_sd_get_fmt,
> >  	.set_fmt = cal_camerarx_sd_set_fmt,
> > +	.get_routing = cal_camerarx_sd_get_routing,
> > +	.set_routing = cal_camerarx_sd_set_routing,
> >  };
> >  
> >  static const struct v4l2_subdev_ops cal_camerarx_subdev_ops = {
> > @@ -844,8 +877,18 @@ static const struct v4l2_subdev_ops cal_camerarx_subdev_ops = {
> >  	.pad = &cal_camerarx_pad_ops,
> >  };
> >  
> > +static bool cal_camerarx_has_route(struct media_entity *entity, unsigned int pad0,
> > +			  unsigned int pad1)
> > +{
> > +	struct v4l2_subdev *sd = media_entity_to_v4l2_subdev(entity);
> > +	struct cal_camerarx *phy = to_cal_camerarx(sd);
> > +
> > +	return v4l2_subdev_has_route(&phy->routing, pad0, pad1);
> > +}
> > +
> >  static struct media_entity_operations cal_camerarx_media_ops = {
> >  	.link_validate = v4l2_subdev_link_validate,
> > +	.has_route = cal_camerarx_has_route,
> >  };
> >  
> >  /* ------------------------------------------------------------------
> > @@ -862,6 +905,20 @@ struct cal_camerarx *cal_camerarx_create(struct cal_dev *cal,
> >  	int ret;
> >  	unsigned int i;
> >  
> > +	struct v4l2_subdev_route routes[] = { {
> > +		.sink_pad = 0,
> > +		.sink_stream = 0,
> > +		.source_pad = 1,
> > +		.source_stream = 0,
> > +		.flags = V4L2_SUBDEV_ROUTE_FL_ACTIVE,
> > +	} };
> > +
> > +	struct v4l2_subdev_krouting routing = {
> > +		.which = V4L2_SUBDEV_FORMAT_ACTIVE,
> > +		.num_routes = 1,
> > +		.routes = routes,
> > +	};
> 
> I'd move this above the other variables. Blank lines are not customary.
> 
> > +
> >  	phy = kzalloc(sizeof(*phy), GFP_KERNEL);
> >  	if (!phy)
> >  		return ERR_PTR(-ENOMEM);
> > @@ -914,6 +971,11 @@ struct cal_camerarx *cal_camerarx_create(struct cal_dev *cal,
> >  	if (ret)
> >  		goto error;
> >  
> > +	/* Initialize routing to single route to the fist source pad */
> 
> s/fist/first/
> s/pad/pad./
> 
> > +	ret = cal_camerarx_sd_set_routing(sd, &routing);
> > +	if (ret)
> > +		goto error;
> > +
> >  	ret = v4l2_device_register_subdev(&cal->v4l2_dev, sd);
> >  	if (ret)
> >  		goto error;
> > @@ -921,6 +983,7 @@ struct cal_camerarx *cal_camerarx_create(struct cal_dev *cal,
> >  	return phy;
> >  
> >  error:
> > +	v4l2_subdev_free_routing(&phy->routing);
> >  	media_entity_cleanup(&phy->subdev.entity);
> >  	kfree(phy);
> >  	return ERR_PTR(ret);
> > @@ -932,6 +995,7 @@ void cal_camerarx_destroy(struct cal_camerarx *phy)
> >  		return;
> >  
> >  	v4l2_device_unregister_subdev(&phy->subdev);
> > +	v4l2_subdev_free_routing(&phy->routing);
> >  	media_entity_cleanup(&phy->subdev.entity);
> >  	of_node_put(phy->source_ep_node);
> >  	of_node_put(phy->source_node);
> > diff --git a/drivers/media/platform/ti-vpe/cal.h b/drivers/media/platform/ti-vpe/cal.h
> > index 3aea444f8bf8..c96364eb0930 100644
> > --- a/drivers/media/platform/ti-vpe/cal.h
> > +++ b/drivers/media/platform/ti-vpe/cal.h
> > @@ -184,6 +184,8 @@ struct cal_camerarx {
> >  	struct mutex		mutex;
> >  
> >  	unsigned int enable_count;
> > +
> > +	struct v4l2_subdev_krouting routing;
> 
> Looking forward to storing this in v4l2_subdev_config, and embedding an
> instance of v4l2_subdev_config in v4l2_subdev :-)
> cal_camerarx_has_route() could then be replaced by a generic V4L2 subdev
> helper, and so could cal_camerarx_sd_get_routing().
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> >  };
> >  
> >  struct cal_dev {

-- 
Regards,

Laurent Pinchart

Re: [PATCH 0/4] media: ti-vpe: cal: multiplexed streams support

[Laurent Pinchart]

Hi Tomi,

Thank you for the series. Nice to see the multiplexed streams API in
operation.

On Tue, Apr 27, 2021 at 04:20:24PM +0300, Tomi Valkeinen wrote:
> Hi,
> 
> This series adds embedded data and multiplexed streams support to TI CAL
> driver. These patches depend on the subdev routing series and CAL preparation
> series:
> 
> https://lore.kernel.org/linux-media/20210427124523.990938-1-tomi.valkeinen@ideasonboard.com/
> 
> https://lore.kernel.org/linux-media/20210420120433.902394-1-tomi.valkeinen@ideasonboard.com/
> 
> One detail I'm not too happy about is adding a new kernel module parameter
> 'cal_streams_api'. Setting this parameter enables the multiplexed streams
> support for the CAL sink pads. I'd like the driver to be able to figure this
> out itself, but there's no simple answer to it.
> 
> A pad in a subdev has to be either in non-multiplexed or multiplexed mode. If
> the subdev cannot support multiplexed streams, then the answer is clear. But if
> it can, then the question becomes "what's connected to the pad, and what does
> it want". Afaik, the idea with subdev configuration is that each subdev can be
> configured independently, so the driver trying to figure out the answer to that
> question breaks this idea. Also, two connected subdevs that both can support
> both modes wouldn't even give the answer, but the answer might be found further
> away, probably from the sensor subdev.
> 
> So, maybe it should be the userspace that can set a subdev pad to either of the
> modes. There's currently no such uAPI, but I think this option makes the most
> sense.

We'll need to brainstorm this a bit. I agree that the module parameter
isn't a good solution.

> Another detail to note is the embedded data support. I don't have hardware with
> real embedded data, so I have not added any embedded data formats. Also, the
> embedded data is captured with the video capture ioctls, not the metadata
> capture ioctls. The reason for this is that in the case of CSI-2 embedded data
> (at least for this hardware) the embedded data is very much like pixel data:
> the data is sent in lines just before or after the pixel content and each
> subdev needs a set_fmt call with the width, height, and mbus-code.
> 
> Using the metadata ioctls is possible, but it only results in much more complex
> driver code, and, I think, more confusing userspace.
> 
>  Tomi
> 
> Tomi Valkeinen (4):
>   media: ti-vpe: cal: add embedded data support
>   media: ti-vpe: cal: use frame desc to get vc and dt
>   media: ti-vpe: cal: add routing support
>   media: ti-vpe: cal: add multiplexed streams support
> 
>  drivers/media/platform/ti-vpe/cal-camerarx.c | 234 ++++++++++++++++---
>  drivers/media/platform/ti-vpe/cal-video.c    |  95 +++++++-
>  drivers/media/platform/ti-vpe/cal.c          | 102 ++++++--
>  drivers/media/platform/ti-vpe/cal.h          |  10 +-
>  4 files changed, 383 insertions(+), 58 deletions(-)

-- 
Regards,

Laurent Pinchart