| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <176839508123.9154.16324392708272572564@freya>
Date: Wed, 14 Jan 2026 18:21:21 +0530
From: Jai Luthra <jai.luthra@...asonboard.com>
To: Rishikesh Donadkar <r-donadkar@...com>, jai.luthra@...ux.dev, laurent.pinchart@...asonboard.com, mripard@...nel.org
Cc: r-donadkar@...com, y-abhilashchandra@...com, devarsht@...com, s-jain1@...com, vigneshr@...com, mchehab@...nel.org, robh@...nel.org, krzk+dt@...nel.org, p.zabel@...gutronix.de, conor+dt@...nel.org, sakari.ailus@...ux.intel.com, hverkuil-cisco@...all.nl, tomi.valkeinen@...asonboard.com, changhuang.liang@...rfivetech.com, jack.zhu@...rfivetech.com, sjoerd@...labora.com, dan.carpenter@...aro.org, hverkuil+cisco@...nel.org, linux-kernel@...r.kernel.org, linux-media@...r.kernel.org, devicetree@...r.kernel.org
Subject: Re: [PATCH v9 08/19] media: staging: starfive: Move to enabel-disable streams in starfive drivers
Hi Rishikesh,
Thanks for the patch.
> Subject: [PATCH v9 08/19] media: staging: starfive: Move to enabel-disable streams in starfive drivers
s/enabel/enable
Quoting Rishikesh Donadkar (2025-12-30 14:02:09)
> The enable_streams() API in v4l2 supports passing a bitmask to enable
> each pad/stream combination individually on any media subdev. Use this
> API instead of s_stream() API in the starfive drivers
nit: I think the description can be explicit that this driver does not
support "multiple streams" (at least right now), but just switching to the
new API while ignoring the passed streams mask.
>
> Signed-off-by: Rishikesh Donadkar <r-donadkar@...com>
> ---
> .../staging/media/starfive/camss/stf-isp.c | 43 ++++++++++++-------
> .../staging/media/starfive/camss/stf-video.c | 4 +-
> 2 files changed, 30 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/staging/media/starfive/camss/stf-isp.c b/drivers/staging/media/starfive/camss/stf-isp.c
> index df7a903fbb1b0..4930ffb0e07a6 100644
> --- a/drivers/staging/media/starfive/camss/stf-isp.c
> +++ b/drivers/staging/media/starfive/camss/stf-isp.c
> @@ -55,27 +55,43 @@ int stf_isp_init(struct stfcamss *stfcamss)
> return 0;
> }
>
> -static int isp_set_stream(struct v4l2_subdev *sd, int enable)
> +static int isp_sd_enable_stream(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *state,
> + u32 pad, u64 streams_mask)
> {
> struct stf_isp_dev *isp_dev = v4l2_get_subdevdata(sd);
> struct v4l2_subdev_state *sd_state;
> struct v4l2_mbus_framefmt *fmt;
> struct v4l2_rect *crop;
> + int ret;
>
> - sd_state = v4l2_subdev_lock_and_get_active_state(sd);
> + sd_state = v4l2_subdev_get_locked_active_state(sd);
> fmt = v4l2_subdev_state_get_format(sd_state, STF_ISP_PAD_SINK);
> crop = v4l2_subdev_state_get_crop(sd_state, STF_ISP_PAD_SRC);
>
> - if (enable) {
> - stf_isp_reset(isp_dev);
> - stf_isp_init_cfg(isp_dev);
> - stf_isp_settings(isp_dev, crop, fmt->code);
> - stf_isp_stream_set(isp_dev);
> - }
> + stf_isp_reset(isp_dev);
> + stf_isp_init_cfg(isp_dev);
> + stf_isp_settings(isp_dev, crop, fmt->code);
> + stf_isp_stream_set(isp_dev);
> +
> + ret = v4l2_subdev_enable_streams(isp_dev->source_subdev, 1, BIT(0));
Given you have a streams_mask argument in this function now, it might be
cleaner to use it here (and let stf-video populate it with BIT(0)).
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
>
> - v4l2_subdev_call(isp_dev->source_subdev, video, s_stream, enable);
> +static int isp_sd_disable_stream(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *state,
> + u32 pad, u64 streams_mask)
> +{
> + struct stf_isp_dev *isp_dev = v4l2_get_subdevdata(sd);
> + int ret;
> +
> + ret = v4l2_subdev_disable_streams(isp_dev->source_subdev, 1, BIT(0));
Same here.
> + if (ret)
> + return ret;
>
> - v4l2_subdev_unlock_state(sd_state);
> return 0;
> }
>
> @@ -300,20 +316,17 @@ static int isp_init_formats(struct v4l2_subdev *sd,
> return isp_set_format(sd, sd_state, &format);
> }
>
> -static const struct v4l2_subdev_video_ops isp_video_ops = {
> - .s_stream = isp_set_stream,
> -};
> -
> static const struct v4l2_subdev_pad_ops isp_pad_ops = {
> .enum_mbus_code = isp_enum_mbus_code,
> .get_fmt = v4l2_subdev_get_fmt,
> .set_fmt = isp_set_format,
> .get_selection = isp_get_selection,
> .set_selection = isp_set_selection,
> + .enable_streams = isp_sd_enable_stream,
> + .disable_streams = isp_sd_disable_stream,
> };
>
> static const struct v4l2_subdev_ops isp_v4l2_ops = {
> - .video = &isp_video_ops,
> .pad = &isp_pad_ops,
> };
>
> diff --git a/drivers/staging/media/starfive/camss/stf-video.c b/drivers/staging/media/starfive/camss/stf-video.c
> index a0420eb6a0aa0..2db29bf8bdef8 100644
> --- a/drivers/staging/media/starfive/camss/stf-video.c
> +++ b/drivers/staging/media/starfive/camss/stf-video.c
> @@ -287,7 +287,7 @@ static int video_start_streaming(struct vb2_queue *q, unsigned int count)
>
> video->ops->start_streaming(video);
>
> - ret = v4l2_subdev_call(video->source_subdev, video, s_stream, true);
> + ret = v4l2_subdev_enable_streams(video->source_subdev, 1, BIT(0));
Now that I think of it, it was not necessary to implement enable / disable
API for the ISP subdev driver given v4l2_subdev_*_streams falls back on
s_stream. But it's anyway good to move drivers, so I guess it's alright.
> if (ret) {
> dev_err(video->stfcamss->dev, "stream on failed\n");
> goto err_pm_put;
> @@ -311,7 +311,7 @@ static void video_stop_streaming(struct vb2_queue *q)
>
> video->ops->stop_streaming(video);
>
> - v4l2_subdev_call(video->source_subdev, video, s_stream, false);
> + v4l2_subdev_disable_streams(video->source_subdev, 1, BIT(0));
>
> pm_runtime_put(video->stfcamss->dev);
>
> --
> 2.34.1
>
>
Thanks,
Jai
Powered by blists - more mailing lists