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:35 UTC|newest]
Thread overview: 30+ 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 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
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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®