From: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
To: Gjorgji.Rosikopulos.gjorgji.rosikopulos@oss.qualcomm.com,
Mauro Carvalho Chehab <mchehab@kernel.org>
Cc: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>,
Loic Poulain <loic.poulain@oss.qualcomm.com>,
Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>,
Atanas Filipov <atanas.filipov@oss.qualcomm.com>,
Jigarkumar Zala <jigarkumar.zala@oss.qualcomm.com>,
linux-media@vger.kernel.org, linux-arm-msm@vger.kernel.org,
linux-kernel@vger.kernel.org,
Gjorgji Rosikopulos <gjorgji.rosikopulos@oss.qualcomm.com>
Subject: Re: [PATCH 4/8] media: qcom: camss: Add streams API support in CSID subdevice
Date: Fri, 11 Sep 2026 12:35:01 +0100 [thread overview]
Message-ID: <f1570f7e-c895-433e-996d-973a96a9afef@linaro.org> (raw)
In-Reply-To: <20260911062213.195007-5-gjorgji.rosikopulos@oss.qualcomm.com>
On 11/09/2026 07:22,
Gjorgji.Rosikopulos.gjorgji.rosikopulos@oss.qualcomm.com wrote:
> From: Gjorgji Rosikopulos <gjorgji.rosikopulos@oss.qualcomm.com>
>
> Add the V4L2 subdev streams API to the CSID driver: per-source-pad
> enable/disable_streams pad ops, VC/DT discovery via get_frame_desc on
> the remote sink pad, and a routing table that routes a single incoming
> sink stream to every source pad by default (remappable via
> set_routing for multi-VC sensors).
>
> Active streams are tracked per pad via a per-pad enabled_streams[]
> bitmask, so that enable/disable_streams correctly propagates to the
> CSIPHY only on first arrival/last departure of a sink stream, and
> multiple source-pad consumers can share a single propagated sink
> stream without redundant or colliding propagation.
>
> csid_init_state() caps the number of default routes to
> MSM_CSID_MAX_SRC_STREAMS - 1 for non-lite CSIDs, matching the 3 usable
> RDI pads on full-IFE CSIDs (the 4th/pix pad is non-functional).
That makes sense.
> msm_csid_register_entity() mirrors the same is_lite check for the pad
> count itself, so non-lite CSIDs no longer register a pix source pad
> that no route ever targets.
Nope. I'll send a different solution. Fixing the non-functional and
incorrectly mapped pix is a Fixes: level thing not a workaround it
inline thing.
Just drop the workaround and assume any CSID you are working with
actually works and is valid.
> csid_pad_enable_streams() rejects enabling with -ENOLINK when there is
> no remote sink link and the test generator is disabled, matching the
> equivalent check the legacy csid_set_stream() already performs.
>
> msm_csid_register_entity() assigns the ctrl handler's lock as the
> subdev's state_lock before v4l2_subdev_init_finalize(), when the test
> pattern control is present. Without this, the test-pattern S_CTRL
> handler and the streams API's active-state accessors serialize on two
> independent locks despite both touching csid->testgen.enabled,
> allowing a concurrent S_CTRL(TEST_PATTERN) and stream enable/disable to
> race.
Actually this reminds me of the first go at VCs in CAMSS which ended up
getting rolled back.
How will this be tested ? Is the TPG capable of generating different VCs ?
_That_ would be very useful.
>
> This is opt-in per CSID instance via the new streams_enable resource
> flag; no platform sets it yet, so CSIDs continue to use the legacy
> non-streams subdev ops unchanged.
>
> Signed-off-by: Gjorgji Rosikopulos <gjorgji.rosikopulos@oss.qualcomm.com>
> ---
> .../media/platform/qcom/camss/camss-csid.c | 494 +++++++++++++++++-
> .../media/platform/qcom/camss/camss-csid.h | 2 +
> 2 files changed, 491 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/camss/camss-csid.c b/drivers/media/platform/qcom/camss/camss-csid.c
> index 48459b46a981..ce4b07c0c1c2 100644
> --- a/drivers/media/platform/qcom/camss/camss-csid.c
> +++ b/drivers/media/platform/qcom/camss/camss-csid.c
> @@ -842,7 +842,7 @@ static void csid_try_format(struct csid_device *csid,
>
> break;
>
> - case MSM_CSID_PAD_SRC:
> + default:
Why subtract the case ?
Just add the default:
Also I don't think that code change is anything todo with adding streams
support to CSID.
Separate patches for separate things.
> if (csid->testgen.nmodes == CSID_PAYLOAD_MODE_DISABLED ||
> csid->testgen_mode->cur.val == 0) {
> /* Test generator is disabled, */
> @@ -1338,10 +1338,476 @@ static const struct v4l2_subdev_ops csid_v4l2_ops = {
> .pad = &csid_pad_ops,
> };
>
> +/*
> + * csid_get_stream_csi2_desc - Discover the virtual channel and data type
> + * used by a given sink stream, from an
> + * already-fetched frame descriptor
> + * @frame_desc: Frame descriptor fetched via .get_frame_desc from the remote
> + * subdev linked on the sink pad
> + * @sink_stream: Sink-side stream number to look up
> + * @desc_csi2: Returns the discovered virtual channel/data type on success
> + *
> + * A frame descriptor with a single entry means the remote only exposes one
> + * stream (e.g. a single-VC sensor), which feeds every CSID source pad, so
> + * that entry is used regardless of @sink_stream.
> + *
> + * Return true if a matching entry was found, false otherwise
> + */
> +static bool csid_get_stream_csi2_desc(struct v4l2_mbus_frame_desc *frame_desc,
> + u32 sink_stream,
> + struct v4l2_mbus_frame_desc_entry_csi2 *desc_csi2)
> +{
> + unsigned int i;
> +
> + if (frame_desc->type != V4L2_MBUS_FRAME_DESC_TYPE_CSI2 || !frame_desc->num_entries)
> + return false;
> +
> + if (frame_desc->num_entries == 1) {
> + *desc_csi2 = frame_desc->entry[0].bus.csi2;
> + return true;
> + }
You can drop that check entirely, the below loop will do exactly the
same thing for num_entires == 1..
> +
> + for (i = 0; i < frame_desc->num_entries; i++) {
> + if (frame_desc->entry[i].stream == sink_stream) {
> + *desc_csi2 = frame_desc->entry[i].bus.csi2;
> + return true;
> + }
> + }
> +
> + return false;
> +}
> +
> +/*
> + * csid_get_stream_vc_dt - Discover the virtual channel and data type to
> + * program for a given sink pad/stream, falling back
> + * to @format_dt when no frame descriptor is available
> + * @csid: CSID device
> + * @state: V4L2 subdevice state
> + * @remote_pad: Remote pad linked on the CSID sink pad, or NULL if unlinked
> + * @pad: Source pad number the caller is enabling a stream on
> + * @format_dt: Data type derived from the sink format, used as a fallback
> + * and sanity-checked against the discovered data type
> + *
> + * Return the discovered virtual channel/data type, or {0, @format_dt} if
> + * not discovered
> + */
> +static struct v4l2_mbus_frame_desc_entry_csi2
> +csid_get_stream_vc_dt(struct csid_device *csid, struct v4l2_subdev_state *state,
> + struct media_pad *remote_pad, u32 pad, u8 format_dt)
> +{
> + struct v4l2_mbus_frame_desc_entry_csi2 desc_csi2 = { .dt = format_dt };
> + struct v4l2_mbus_frame_desc fd = { };
> + u32 sink_stream;
> +
> + if (!remote_pad ||
> + v4l2_subdev_call(media_entity_to_v4l2_subdev(remote_pad->entity),
> + pad, get_frame_desc, remote_pad->index, &fd))
> + return desc_csi2;
> +
if (thing ||
some_other_thing) {
return desc_csi2;
}
> + if (v4l2_subdev_routing_find_opposite_end(&state->routing, pad, 0, NULL, &sink_stream))
> + return desc_csi2;
> +
> + if (!csid_get_stream_csi2_desc(&fd, sink_stream, &desc_csi2)) {
> + dev_warn(csid->camss->dev,
> + "Failed to find CSI2 descriptor for sink stream %u, using vc=%u dt=%u\n",
> + sink_stream, desc_csi2.vc, desc_csi2.dt);
> + return desc_csi2;
Is this an error it seems like it should be ?
> + }
> +
> + if (desc_csi2.dt != format_dt)
> + dev_warn(csid->camss->dev,
> + "Sink stream %u frame desc dt=%u differs from format dt=%u, using dt=%u\n",
> + sink_stream, desc_csi2.dt, format_dt, desc_csi2.dt);
> +
> + return desc_csi2;
I'd return a pointer here.
> +}
> +
> +/*
> + * csid_pad_enable_streams - Enable one or more streams on a source pad
> + * @sd: CSID V4L2 subdevice
> + * @state: V4L2 subdevice state
> + * @pad: Pad number
> + * @streams_mask: Bitmask of v4l2 streams to enable
> + *
> + * The v4l2 core only calls this on a source pad (v4l2_subdev_enable_streams()
> + * rejects sink pads with -EOPNOTSUPP before reaching the driver), so @pad is
> + * not checked here. Each source pad only ever carries stream 0.
> + *
> + * The shared sink stream(s) are propagated upstream only once, on the
> + * transition from no active sink streams to at least one, so that a second
> + * consumer of the same shared sink stream never triggers a second, redundant
> + * propagation to the sensor. The Rx front-end is likewise only configured
> + * once, on that same transition.
> + *
> + * Return 0 on success, -ENOLINK if there is no remote sink link and the test
> + * generator is disabled, or another negative error code otherwise
> + */
> +static int csid_pad_enable_streams(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *state,
> + u32 pad, u64 streams_mask)
> +{
> + struct csid_device *csid = v4l2_get_subdevdata(sd);
> + const struct csid_hw_ops *hw_ops = csid->res->hw_ops;
> + struct media_pad *remote_pad = media_pad_remote_pad_first(&csid->pads[MSM_CSID_PAD_SINK]);
> + unsigned int hw_port = pad - MSM_CSID_PAD_FIRST_SRC;
> + const struct csid_format_info *format;
> + struct v4l2_mbus_frame_desc_entry_csi2 desc_csi2;
> + u64 sink_streams, propagate_mask;
> + int ret;
> +
> + if (!csid->testgen.enabled && !remote_pad)
> + return -ENOLINK;
> +
> + sink_streams = v4l2_subdev_state_xlate_streams(state, pad, MSM_CSID_PAD_SINK,
> + &streams_mask);
> +
> + if (!csid->enabled_streams[MSM_CSID_PAD_SINK]) {
> + if (csid->testgen.nmodes != CSID_PAYLOAD_MODE_DISABLED) {
> + /*
> + * sd->state_lock is aliased to csid->ctrls.lock, and is
> + * already held here by the v4l2_subdev_enable_streams()
> + * caller, so use the lock-free variant to avoid
> + * self-deadlocking on the same mutex.
> + */
> + ret = __v4l2_ctrl_handler_setup(&csid->ctrls);
> + if (ret < 0) {
> + dev_err(csid->camss->dev,
> + "could not sync v4l2 controls: %d\n", ret);
> + return ret;
> + }
> + }
> +
> + hw_ops->configure_rx(csid);
> + }
> +
> + /* Sink streams already active elsewhere don't need re-propagating. */
> + propagate_mask = sink_streams & ~csid->enabled_streams[MSM_CSID_PAD_SINK];
> + csid->enabled_streams[MSM_CSID_PAD_SINK] |= sink_streams;
> + csid->enabled_streams[pad] |= streams_mask;
> +
> + format = csid_get_fmt_entry(csid->res->formats->formats,
> + csid->res->formats->nformats,
> + csid->fmt[pad].code);
> + desc_csi2 = csid_get_stream_vc_dt(csid, state, remote_pad, pad, format->data_type);
You're doing an implict memcpy() here - just return a pointer.
And I really wonder based on the activity within the method why
returning an error isn't part of this ?
> +
> + hw_ops->enable_stream(csid, hw_port, desc_csi2.vc, desc_csi2.dt);
I commented elsewhere should this be void or int ?
I'm not suggesting either more asking rhetorically.
> +
> + if (propagate_mask && remote_pad) {
> + ret = v4l2_subdev_enable_streams(media_entity_to_v4l2_subdev(remote_pad->entity),
> + remote_pad->index, propagate_mask);
> + if (ret) {
> + csid->enabled_streams[MSM_CSID_PAD_SINK] &= ~propagate_mask;
> + csid->enabled_streams[pad] &= ~streams_mask;
> +
> + hw_ops->disable_stream(csid, hw_port);
> +
> + return ret;
> + }
> + }
> +
> + return 0;
> +}
> +
> +/*
> + * csid_sink_streams_in_use - Compute the subset of sink streams still
> + * referenced by a source pad other than @pad
> + * @csid: CSID device
> + * @state: V4L2 subdevice state
> + * @pad: Source pad to exclude from the check
> + * @sink_streams: Candidate sink streams to check
> + *
> + * Return the subset of @sink_streams still referenced by some other source
> + * pad
> + */
> +static u64 csid_sink_streams_in_use(struct csid_device *csid, struct v4l2_subdev_state *state,
> + u32 pad, u64 sink_streams)
> +{
> + u64 in_use = 0;
> + unsigned int i;
> +
> + for (i = MSM_CSID_PAD_FIRST_SRC; i < MSM_CSID_PADS_NUM; i++) {
> + u64 other_streams = csid->enabled_streams[i];
> + u64 other_sink_streams;
> +
> + if (i == pad)
> + continue;
> +
> + other_sink_streams = v4l2_subdev_state_xlate_streams(state, i, MSM_CSID_PAD_SINK,
> + &other_streams);
> + in_use |= sink_streams & other_sink_streams;
> + }
> +
> + return in_use;
> +}
> +
> +/*
> + * csid_pad_disable_streams - Disable one or more streams on a source pad
> + * @sd: CSID V4L2 subdevice
> + * @state: V4L2 subdevice state
> + * @pad: Pad number
> + * @streams_mask: Bitmask of v4l2 streams to disable
> + *
> + * The v4l2 core only calls this on a source pad (v4l2_subdev_disable_streams()
> + * rejects sink pads with -EOPNOTSUPP before reaching the driver), so @pad is
> + * not checked here. Each source pad only ever carries stream 0.
> + *
> + * A sink stream is only disabled, and propagated upstream to disable it there
> + * too, once no source pad references it any more.
> + *
> + * Return 0 on success or a negative error code otherwise
> + */
> +static int csid_pad_disable_streams(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *state,
> + u32 pad, u64 streams_mask)
> +{
> + struct csid_device *csid = v4l2_get_subdevdata(sd);
> + const struct csid_hw_ops *hw_ops = csid->res->hw_ops;
> + struct media_pad *remote_pad = media_pad_remote_pad_first(&csid->pads[MSM_CSID_PAD_SINK]);
> + unsigned int hw_port = pad - MSM_CSID_PAD_FIRST_SRC;
> + u64 sink_streams, disable_sink_streams;
> + int ret = 0;
> +
> + sink_streams = v4l2_subdev_state_xlate_streams(state, pad, MSM_CSID_PAD_SINK,
> + &streams_mask);
> +
> + /* Keep a sink stream active as long as any other source pad still uses it. */
> + disable_sink_streams = sink_streams &
> + ~csid_sink_streams_in_use(csid, state, pad, sink_streams);
> +
> + if (disable_sink_streams && remote_pad) {
> + ret = v4l2_subdev_disable_streams(media_entity_to_v4l2_subdev(remote_pad->entity),
> + remote_pad->index, disable_sink_streams);
> + if (ret)
> + dev_err(csid->camss->dev,
> + "Failed to disable stream on remote pad: %d\n", ret);
> + }
> +
> + hw_ops->disable_stream(csid, hw_port);
> +
> + csid->enabled_streams[pad] &= ~streams_mask;
> + csid->enabled_streams[MSM_CSID_PAD_SINK] &= ~disable_sink_streams;
> +
> + return ret;
> +}
> +
> +static const struct v4l2_mbus_framefmt csid_default_format = {
> + .code = MEDIA_BUS_FMT_UYVY8_1X16,
> + .width = 1920,
> + .height = 1080,
> + .field = V4L2_FIELD_NONE,
> + .colorspace = V4L2_COLORSPACE_SRGB,
> +};
> +
> +/*
> + * csid_set_routing - Handle setting of routing table
> + * @sd: CSID V4L2 subdevice
> + * @state: V4L2 subdevice state
> + * @which: TRY or ACTIVE routing
> + * @routing: Routing table to set
> + *
> + * Return 0 on success or a negative error code otherwise
> + */
> +static int csid_set_routing(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *state,
> + enum v4l2_subdev_format_whence which,
> + struct v4l2_subdev_krouting *routing)
> +{
> + struct csid_device *csid = v4l2_get_subdevdata(sd);
> + unsigned int i;
> + int ret;
> +
> + if (which == V4L2_SUBDEV_FORMAT_ACTIVE && csid->enabled_streams[MSM_CSID_PAD_SINK])
> + return -EBUSY;
> +
> + for (i = 0; i < routing->num_routes; i++)
> + if (routing->routes[i].source_stream != 0)
> + return -EINVAL;
> +
> + ret = v4l2_subdev_routing_validate(sd, routing,
> + V4L2_SUBDEV_ROUTING_NO_SOURCE_STREAM_MIX |
> + V4L2_SUBDEV_ROUTING_NO_SOURCE_MULTIPLEXING |
> + V4L2_SUBDEV_ROUTING_NO_N_TO_1);
> + if (ret)
> + return ret;
> +
> + return v4l2_subdev_set_routing_with_fmt(sd, state, routing, &csid_default_format);
> +}
> +
> +/*
> + * __csid_get_stream_format - Get pointer to per-stream format structure
> + * @csid: CSID device
> + * @sd_state: V4L2 subdev state
> + * @pad: pad from which format is requested
> + * @stream: stream from which format is requested
> + * @which: TRY or ACTIVE format
> + *
> + * Same as __csid_get_format(), but honors @stream for TRY-state lookups.
> + * For ACTIVE state, csid->fmt[] is indexed by pad + stream. @stream is
> + * always 0 and @pad selects the RDI channel (0-3).
> + *
> + * Return pointer to TRY or ACTIVE format structure
> + */
> +static struct v4l2_mbus_framefmt *
> +__csid_get_stream_format(struct csid_device *csid,
> + struct v4l2_subdev_state *sd_state,
> + unsigned int pad, u32 stream,
> + enum v4l2_subdev_format_whence which)
> +{
> + if (which == V4L2_SUBDEV_FORMAT_TRY)
> + return v4l2_subdev_state_get_format(sd_state, pad, stream);
> +
> + if (pad == MSM_CSID_PAD_SINK)
> + return &csid->fmt[MSM_CSID_PAD_SINK];
> +
> + return &csid->fmt[pad + stream];
> +}
> +
> +/*
> + * csid_streams_get_format - Handle get format by pads subdev method
> + * @sd: CSID V4L2 subdevice
> + * @sd_state: V4L2 subdev state
> + * @fmt: pointer to v4l2 subdev format structure
> + *
> + * Return -EINVAL or zero on success
> + */
> +static int csid_streams_get_format(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *sd_state,
> + struct v4l2_subdev_format *fmt)
> +{
> + struct csid_device *csid = v4l2_get_subdevdata(sd);
> + struct v4l2_mbus_framefmt *format;
> +
> + format = __csid_get_stream_format(csid, sd_state, fmt->pad, fmt->stream, fmt->which);
> + if (!format)
> + return -EINVAL;
> +
> + fmt->format = *format;
> +
> + return 0;
> +}
> +
> +/*
> + * csid_streams_set_format - Handle set format by pads subdev method
> + * @sd: CSID V4L2 subdevice
> + * @sd_state: V4L2 subdev state
> + * @fmt: pointer to v4l2 subdev format structure
> + *
> + * Return -EINVAL or zero on success
> + */
> +static int csid_streams_set_format(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *sd_state,
> + struct v4l2_subdev_format *fmt)
> +{
> + struct csid_device *csid = v4l2_get_subdevdata(sd);
> + struct v4l2_mbus_framefmt *format;
> + struct v4l2_subdev_route *route;
> +
> + if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE && csid->enabled_streams[MSM_CSID_PAD_SINK])
> + return -EBUSY;
> +
> + format = __csid_get_stream_format(csid, sd_state, fmt->pad, fmt->stream, fmt->which);
> + if (!format)
> + return -EINVAL;
> +
> + csid_try_format(csid, sd_state, fmt->pad, &fmt->format, fmt->which);
> + *format = fmt->format;
> +
> + /* Propagate the format from the sink stream to every source stream it feeds */
> + for_each_active_route(&sd_state->routing, route) {
> + struct v4l2_mbus_framefmt *src_format;
> +
> + if (route->sink_pad != fmt->pad || route->sink_stream != fmt->stream)
> + continue;
> +
> + src_format = __csid_get_stream_format(csid, sd_state, route->source_pad,
> + route->source_stream, fmt->which);
> + if (!src_format)
> + continue;
> +
> + *src_format = fmt->format;
> + csid_try_format(csid, sd_state, route->source_pad, src_format, fmt->which);
> + }
> +
> + return 0;
> +}
Why do we need a full new set of get-format and set-format ?
"Feels" like this could wrapper the existing code.
> +
> +static const struct v4l2_subdev_pad_ops csid_streams_pad_ops = {
> + .enum_mbus_code = csid_enum_mbus_code,
> + .enum_frame_size = csid_enum_frame_size,
> + .get_fmt = csid_streams_get_format,
> + .set_fmt = csid_streams_set_format,
> + .set_routing = csid_set_routing,
> + .enable_streams = csid_pad_enable_streams,
> + .disable_streams = csid_pad_disable_streams,
> +};
> +
> +static const struct v4l2_subdev_video_ops csid_streams_video_ops = {
> + .s_stream = v4l2_subdev_s_stream_helper,
> +};
> +
> +static const struct v4l2_subdev_ops csid_streams_v4l2_ops = {
> + .core = &csid_core_ops,
> + .pad = &csid_streams_pad_ops,
> + .video = &csid_streams_video_ops,
> +};
> +
> +/*
> + * csid_init_state - Initialize the routing table for the streams API subdev
> + * @sd: CSID V4L2 subdevice
> + * @state: V4L2 subdev state
> + *
> + * source_stream is always 0: each source pad MSM_CSID_PAD_FIRST_SRC + i
> + * links to its own independent downstream subdev, and a link's sink side is
> + * validated against the implicit stream 0 exposed by any subdev without
> + * V4L2_SUBDEV_FL_STREAMS (see v4l2_link_validate_get_streams()) — every
> + * downstream VFE line is such a subdev.
> + *
> + * All source pads route from sink_stream 0 by default, fanning the single
> + * incoming stream out to every port; a multi-VC source is supported by
> + * remapping each route's sink_stream via .set_routing, leaving
> + * source_pad/source_stream untouched.
> + *
> + * Return 0 on success or a negative error code otherwise
> + */
> +static int csid_init_state(struct v4l2_subdev *sd, struct v4l2_subdev_state *state)
> +{
> + struct csid_device *csid = v4l2_get_subdevdata(sd);
> + struct v4l2_subdev_route routes[MSM_CSID_MAX_SRC_STREAMS];
> + struct v4l2_subdev_krouting routing = { };
> + unsigned int num_routes;
> + int i, ret;
> +
> + /* The full IFE has only 3 rdi's and pix output is not functional */
> + if (csid_is_lite(csid))
> + num_routes = MSM_CSID_MAX_SRC_STREAMS;
> + else
> + num_routes = MSM_CSID_MAX_SRC_STREAMS - 1;
No. Don't code around this here.
I'll make a Fixes: patch for the pix stuff - I have it in tree. I don't
want to add work-arounds in code.
> +
> + for (i = 0; i < num_routes; i++) {
> + routes[i].sink_pad = MSM_CSID_PAD_SINK;
> + routes[i].sink_stream = 0;
> + routes[i].source_pad = MSM_CSID_PAD_FIRST_SRC + i;
> + routes[i].source_stream = 0;
> + routes[i].flags = V4L2_SUBDEV_ROUTE_FL_ACTIVE;
> + }
> +
> + routing.num_routes = num_routes;
> + routing.routes = routes;
> + ret = v4l2_subdev_set_routing_with_fmt(sd, state, &routing, &csid_default_format);
> + if (ret)
> + dev_err(csid->camss->dev, "Failed to set routing: %d\n", ret);
> +
> + return ret;
> +}
> +
> static const struct v4l2_subdev_internal_ops csid_v4l2_internal_ops = {
> .open = csid_init_formats,
> };
>
> +static const struct v4l2_subdev_internal_ops csid_streams_internal_ops = {
> + .init_state = csid_init_state,
> +};
> +
> static const struct media_entity_operations csid_media_ops = {
> .link_setup = csid_link_setup,
> .link_validate = v4l2_subdev_link_validate,
> @@ -1360,13 +1826,18 @@ int msm_csid_register_entity(struct csid_device *csid,
> struct v4l2_subdev *sd = &csid->subdev;
> struct media_pad *pads = csid->pads;
> struct device *dev = csid->camss->dev;
> + bool streams_api = csid->res->streams_enable;
As I stated elsewhere there's no need to have this flag copy/pasted.
Just move it one level up does this SoC support streams, in
camss->supports_streams;
> + unsigned int num_pads = csid_is_lite(csid) ? MSM_CSID_PADS_NUM : MSM_CSID_PADS_NUM - 1;
> int i;
> int ret;
>
> - v4l2_subdev_init(sd, &csid_v4l2_ops);
> - sd->internal_ops = &csid_v4l2_internal_ops;
> + v4l2_subdev_init(sd, streams_api ? &csid_streams_v4l2_ops : &csid_v4l2_ops);
> + sd->internal_ops = streams_api ? &csid_streams_internal_ops
> + : &csid_v4l2_internal_ops;
> sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE |
> V4L2_SUBDEV_FL_HAS_EVENTS;
> + if (streams_api)
> + sd->flags |= V4L2_SUBDEV_FL_STREAMS;
> snprintf(sd->name, ARRAY_SIZE(sd->name), "%s%d",
> MSM_CSID_NAME, csid->id);
> v4l2_set_subdevdata(sd, csid);
> @@ -1400,17 +1871,28 @@ int msm_csid_register_entity(struct csid_device *csid,
> }
>
> pads[MSM_CSID_PAD_SINK].flags = MEDIA_PAD_FL_SINK;
> - for (i = MSM_CSID_PAD_FIRST_SRC; i < MSM_CSID_PADS_NUM; ++i)
> + for (i = MSM_CSID_PAD_FIRST_SRC; i < num_pads; ++i)
> pads[i].flags = MEDIA_PAD_FL_SOURCE;
>
> sd->entity.function = MEDIA_ENT_F_PROC_VIDEO_PIXEL_FORMATTER;
> sd->entity.ops = &csid_media_ops;
> - ret = media_entity_pads_init(&sd->entity, MSM_CSID_PADS_NUM, pads);
> + ret = media_entity_pads_init(&sd->entity, num_pads, pads);
> if (ret < 0) {
> dev_err(dev, "Failed to init media entity: %d\n", ret);
> goto free_ctrl;
> }
>
> + if (streams_api) {
> + if (csid->testgen.nmodes != CSID_PAYLOAD_MODE_DISABLED)
> + sd->state_lock = csid->ctrls.lock;
> +
> + ret = v4l2_subdev_init_finalize(sd);
> + if (ret) {
> + dev_err(dev, "Failed to finalize subdev: %d\n", ret);
> + goto media_cleanup;
> + }
> + }
> +
> ret = v4l2_device_register_subdev(v4l2_dev, sd);
> if (ret < 0) {
> dev_err(dev, "Failed to register subdev: %d\n", ret);
> @@ -1420,6 +1902,7 @@ int msm_csid_register_entity(struct csid_device *csid,
> return 0;
>
> media_cleanup:
> + v4l2_subdev_cleanup(sd);
> media_entity_cleanup(&sd->entity);
> free_ctrl:
> if (csid->testgen.nmodes != CSID_PAYLOAD_MODE_DISABLED)
> @@ -1435,6 +1918,7 @@ int msm_csid_register_entity(struct csid_device *csid,
> void msm_csid_unregister_entity(struct csid_device *csid)
> {
> v4l2_device_unregister_subdev(&csid->subdev);
> + v4l2_subdev_cleanup(&csid->subdev);
> media_entity_cleanup(&csid->subdev.entity);
> if (csid->testgen.nmodes != CSID_PAYLOAD_MODE_DISABLED)
> v4l2_ctrl_handler_free(&csid->ctrls);
> diff --git a/drivers/media/platform/qcom/camss/camss-csid.h b/drivers/media/platform/qcom/camss/camss-csid.h
> index 90ee611b9092..a312103cf86d 100644
> --- a/drivers/media/platform/qcom/camss/camss-csid.h
> +++ b/drivers/media/platform/qcom/camss/camss-csid.h
> @@ -184,6 +184,7 @@ struct csid_hw_ops {
>
> struct csid_subdev_resources {
> bool is_lite;
> + bool streams_enable;
> const struct csid_hw_ops *hw_ops;
> const struct parent_dev_ops *parent_dev_ops;
> const struct csid_formats *formats;
> @@ -209,6 +210,7 @@ struct csid_device {
> struct v4l2_mbus_framefmt fmt[MSM_CSID_PADS_NUM];
> struct v4l2_ctrl_handler ctrls;
> struct v4l2_ctrl *testgen_mode;
> + u64 enabled_streams[MSM_CSID_PADS_NUM];
> const struct csid_subdev_resources *res;
> };
>
next prev parent reply other threads:[~2026-09-11 11:40 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-11 6:22 [PATCH 0/8] media: qcom: camss: add V4L2 subdev streams API support Gjorgji.Rosikopulos.gjorgji.rosikopulos
2026-09-11 6:22 ` [PATCH 1/8] media: qcom: camss: Add streams API support for CSIPHY Gjorgji.Rosikopulos.gjorgji.rosikopulos
2026-09-11 10:37 ` Bryan O'Donoghue
2026-09-11 13:00 ` Gjorgji Rosikopulos (Consultant)
2026-09-11 6:22 ` [PATCH 2/8] media: qcom: camss: Add streams API hw_ops to CSID interface Gjorgji.Rosikopulos.gjorgji.rosikopulos
2026-09-11 10:43 ` Bryan O'Donoghue
2026-09-11 14:05 ` Gjorgji Rosikopulos (Consultant)
2026-09-11 6:22 ` [PATCH 3/8] media: qcom: camss: Implement CSID streams API hw_ops for gen2 Gjorgji.Rosikopulos.gjorgji.rosikopulos
2026-09-11 10:46 ` Bryan O'Donoghue
2026-09-11 14:08 ` Gjorgji Rosikopulos (Consultant)
2026-09-11 13:30 ` Loic Poulain
2026-09-11 14:17 ` Gjorgji Rosikopulos (Consultant)
2026-09-12 5:34 ` Gjorgji Rosikopulos (Consultant)
2026-09-11 6:22 ` [PATCH 4/8] media: qcom: camss: Add streams API support in CSID subdevice Gjorgji.Rosikopulos.gjorgji.rosikopulos
2026-09-11 11:35 ` Bryan O'Donoghue [this message]
2026-09-11 14:33 ` Gjorgji Rosikopulos (Consultant)
2026-09-11 6:22 ` [PATCH 5/8] media: qcom: camss: Fix CSID-to-VFE all-to-all link crossbar on sm8250 Gjorgji.Rosikopulos.gjorgji.rosikopulos
2026-09-11 11:37 ` Bryan O'Donoghue
2026-09-11 14:37 ` Gjorgji Rosikopulos (Consultant)
2026-09-11 6:22 ` [PATCH 6/8] media: qcom: camss: add streams API support for VFE Gjorgji.Rosikopulos.gjorgji.rosikopulos
2026-09-11 6:22 ` [PATCH 7/8] media: qcom: camss: add streams API support in camss-video Gjorgji.Rosikopulos.gjorgji.rosikopulos
2026-09-11 6:22 ` [PATCH 8/8] media: qcom: camss: enable streams API on SM8250 Gjorgji.Rosikopulos.gjorgji.rosikopulos
2026-09-11 10:19 ` [PATCH 0/8] media: qcom: camss: add V4L2 subdev streams API support Bryan O'Donoghue
2026-09-11 12:55 ` Gjorgji Rosikopulos (Consultant)
2026-09-15 12:15 ` Hitesh Patel
2026-09-15 12:15 ` [PATCH 1/2] media: qcom: camss: Do not link CSID source pads the CSID does not have Hitesh Patel
2026-09-15 12:15 ` [PATCH 2/2] media: qcom: camss: Enable the streams API on SC7280 Hitesh Patel
2026-09-16 8:08 ` Bryan O'Donoghue
2026-09-16 8:31 ` Hitesh Patel
2026-09-16 5:53 ` [PATCH v2 0/2] media: qcom: camss: SC7280 fixes for the streams API series Hitesh Patel
2026-09-16 5:53 ` [PATCH v2 1/2] media: qcom: camss: Do not link CSID source pads the CSID does not have Hitesh Patel
2026-09-16 5:53 ` [PATCH v2 2/2] media: qcom: camss: Enable the streams API on SC7280 Hitesh Patel
2026-09-16 6:54 ` [PATCH v3 0/2] media: qcom: camss: SC7280 fixes for the streams API series Hitesh Patel
2026-09-16 6:54 ` [PATCH v3 1/2] media: qcom: camss: Do not link CSID source pads the CSID does not have Hitesh Patel
2026-09-16 6:54 ` [PATCH v3 2/2] media: qcom: camss: Enable the streams API on SC7280 Hitesh Patel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=f1570f7e-c895-433e-996d-973a96a9afef@linaro.org \
--to=bryan.odonoghue@linaro.org \
--cc=Gjorgji.Rosikopulos.gjorgji.rosikopulos@oss.qualcomm.com \
--cc=atanas.filipov@oss.qualcomm.com \
--cc=dmitry.baryshkov@oss.qualcomm.com \
--cc=gjorgji.rosikopulos@oss.qualcomm.com \
--cc=jigarkumar.zala@oss.qualcomm.com \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=loic.poulain@oss.qualcomm.com \
--cc=mchehab@kernel.org \
--cc=vladimir.zapolskiy@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.