[PATCH 01/16] media: sun6i-csi: bridge: Use V4L2 subdev active state
arash golgol
arash.golgol at gmail.com
Thu Jul 2 00:38:23 PDT 2026
Hi Paul,
On Mon, May 18, 2026 at 1:55 PM Paul Kocialkowski <paulk at sys-base.io> wrote:
>
> From: Arash Golgol <arash.golgol at gmail.com>
>
> Use the V4L2 subdev active state API to store the active format.
> This simplifies the driver not only by dropping the bridge mbus_format
> field, but it also allows dropping the bridge lock, replaced with
> the state lock.
>
> Previously, capture accessed bridge private state directly. After
> moving to framework-managed state, resolve the format through the
> subdev pad API.
>
> The sun6i-csi-bridge hardware does not perform any format conversion.
> Enforce identical formats on the sink and source pads in the set_fmt()
> and init_state() callbacks.
>
> Signed-off-by: Arash Golgol <arash.golgol at gmail.com>
> Reviewed-by: Paul Kocialkowski <paulk at sys-base.io>
> Tested-by: Paul Kocialkowski <paulk at sys-base.io>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
I noticed this series didn't make it into the v7.2 merge window.
Is there anything still pending on my side before it can be picked up,
or is it just a matter of timing for the next cycle?
Happy to help however I can.
> ---
> .../sunxi/sun6i-csi/sun6i_csi_bridge.c | 155 ++++++++----------
> .../sunxi/sun6i-csi/sun6i_csi_bridge.h | 9 -
> .../sunxi/sun6i-csi/sun6i_csi_capture.c | 27 ++-
> 3 files changed, 86 insertions(+), 105 deletions(-)
>
> diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_bridge.c b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_bridge.c
> index d006d9dd0170..43a85bcc2ba2 100644
> --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_bridge.c
> +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_bridge.c
> @@ -13,26 +13,6 @@
> #include "sun6i_csi_bridge.h"
> #include "sun6i_csi_reg.h"
>
> -/* Helpers */
> -
> -void sun6i_csi_bridge_dimensions(struct sun6i_csi_device *csi_dev,
> - unsigned int *width, unsigned int *height)
> -{
> - if (width)
> - *width = csi_dev->bridge.mbus_format.width;
> - if (height)
> - *height = csi_dev->bridge.mbus_format.height;
> -}
> -
> -void sun6i_csi_bridge_format(struct sun6i_csi_device *csi_dev,
> - u32 *mbus_code, u32 *field)
> -{
> - if (mbus_code)
> - *mbus_code = csi_dev->bridge.mbus_format.code;
> - if (field)
> - *field = csi_dev->bridge.mbus_format.field;
> -}
> -
> /* Format */
>
> static const struct sun6i_csi_bridge_format sun6i_csi_bridge_formats[] = {
> @@ -226,7 +206,8 @@ static void sun6i_csi_bridge_disable(struct sun6i_csi_device *csi_dev)
> }
>
> static void
> -sun6i_csi_bridge_configure_parallel(struct sun6i_csi_device *csi_dev)
> +sun6i_csi_bridge_configure_parallel(struct sun6i_csi_device *csi_dev,
> + const struct v4l2_mbus_framefmt *mbus_format)
> {
> struct device *dev = csi_dev->dev;
> struct regmap *regmap = csi_dev->regmap;
> @@ -234,11 +215,9 @@ sun6i_csi_bridge_configure_parallel(struct sun6i_csi_device *csi_dev)
> &csi_dev->bridge.source_parallel.endpoint;
> unsigned char bus_width = endpoint->bus.parallel.bus_width;
> unsigned int flags = endpoint->bus.parallel.flags;
> - u32 field;
> + u32 field = mbus_format->field;
> u32 value = SUN6I_CSI_IF_CFG_IF_CSI;
>
> - sun6i_csi_bridge_format(csi_dev, NULL, &field);
> -
> if (field == V4L2_FIELD_INTERLACED ||
> field == V4L2_FIELD_INTERLACED_TB ||
> field == V4L2_FIELD_INTERLACED_BT)
> @@ -317,13 +296,12 @@ sun6i_csi_bridge_configure_parallel(struct sun6i_csi_device *csi_dev)
> }
>
> static void
> -sun6i_csi_bridge_configure_mipi_csi2(struct sun6i_csi_device *csi_dev)
> +sun6i_csi_bridge_configure_mipi_csi2(struct sun6i_csi_device *csi_dev,
> + const struct v4l2_mbus_framefmt *mbus_format)
> {
> struct regmap *regmap = csi_dev->regmap;
> u32 value = SUN6I_CSI_IF_CFG_IF_MIPI;
> - u32 field;
> -
> - sun6i_csi_bridge_format(csi_dev, NULL, &field);
> + u32 field = mbus_format->field;
>
> if (field == V4L2_FIELD_INTERLACED ||
> field == V4L2_FIELD_INTERLACED_TB ||
> @@ -335,19 +313,20 @@ sun6i_csi_bridge_configure_mipi_csi2(struct sun6i_csi_device *csi_dev)
> regmap_write(regmap, SUN6I_CSI_IF_CFG_REG, value);
> }
>
> -static void sun6i_csi_bridge_configure_format(struct sun6i_csi_device *csi_dev)
> +static void
> +sun6i_csi_bridge_configure_format(struct sun6i_csi_device *csi_dev,
> + const struct v4l2_mbus_framefmt *mbus_format)
> {
> struct regmap *regmap = csi_dev->regmap;
> bool capture_streaming = csi_dev->capture.state.streaming;
> const struct sun6i_csi_bridge_format *bridge_format;
> const struct sun6i_csi_capture_format *capture_format;
> - u32 mbus_code, field, pixelformat;
> + u32 pixelformat;
> + u32 field = mbus_format->field;
> u8 input_format, input_yuv_seq, output_format;
> u32 value = 0;
>
> - sun6i_csi_bridge_format(csi_dev, &mbus_code, &field);
> -
> - bridge_format = sun6i_csi_bridge_format_find(mbus_code);
> + bridge_format = sun6i_csi_bridge_format_find(mbus_format->code);
> if (WARN_ON(!bridge_format))
> return;
>
> @@ -391,16 +370,17 @@ static void sun6i_csi_bridge_configure_format(struct sun6i_csi_device *csi_dev)
> }
>
> static void sun6i_csi_bridge_configure(struct sun6i_csi_device *csi_dev,
> - struct sun6i_csi_bridge_source *source)
> + struct sun6i_csi_bridge_source *source,
> + const struct v4l2_mbus_framefmt *mbus_format)
> {
> struct sun6i_csi_bridge *bridge = &csi_dev->bridge;
>
> if (source == &bridge->source_parallel)
> - sun6i_csi_bridge_configure_parallel(csi_dev);
> + sun6i_csi_bridge_configure_parallel(csi_dev, mbus_format);
> else
> - sun6i_csi_bridge_configure_mipi_csi2(csi_dev);
> + sun6i_csi_bridge_configure_mipi_csi2(csi_dev, mbus_format);
>
> - sun6i_csi_bridge_configure_format(csi_dev);
> + sun6i_csi_bridge_configure_format(csi_dev, mbus_format);
> }
>
> /* V4L2 Subdev */
> @@ -415,6 +395,8 @@ static int sun6i_csi_bridge_s_stream(struct v4l2_subdev *subdev, int on)
> struct sun6i_csi_bridge_source *source;
> struct v4l2_subdev *source_subdev;
> struct media_pad *remote_pad;
> + struct v4l2_subdev_state *state;
> + const struct v4l2_mbus_framefmt *mbus_format;
> int ret;
>
> /* Source */
> @@ -433,6 +415,10 @@ static int sun6i_csi_bridge_s_stream(struct v4l2_subdev *subdev, int on)
> else
> source = &bridge->source_mipi_csi2;
>
> + /* Active State */
> +
> + state = v4l2_subdev_lock_and_get_active_state(subdev);
> +
> if (!on) {
> v4l2_subdev_call(source_subdev, video, s_stream, 0);
> ret = 0;
> @@ -443,7 +429,7 @@ static int sun6i_csi_bridge_s_stream(struct v4l2_subdev *subdev, int on)
>
> ret = pm_runtime_resume_and_get(dev);
> if (ret < 0)
> - return ret;
> + goto unlock;
>
> /* Clear */
>
> @@ -451,7 +437,9 @@ static int sun6i_csi_bridge_s_stream(struct v4l2_subdev *subdev, int on)
>
> /* Configure */
>
> - sun6i_csi_bridge_configure(csi_dev, source);
> + mbus_format = v4l2_subdev_state_get_format(state,
> + SUN6I_CSI_BRIDGE_PAD_SINK);
> + sun6i_csi_bridge_configure(csi_dev, source, mbus_format);
>
> if (capture_streaming)
> sun6i_csi_capture_configure(csi_dev);
> @@ -472,7 +460,8 @@ static int sun6i_csi_bridge_s_stream(struct v4l2_subdev *subdev, int on)
> if (ret && ret != -ENOIOCTLCMD)
> goto disable;
>
> - return 0;
> + ret = 0;
> + goto unlock;
>
> disable:
> if (capture_streaming)
> @@ -482,6 +471,8 @@ static int sun6i_csi_bridge_s_stream(struct v4l2_subdev *subdev, int on)
>
> pm_runtime_put(dev);
>
> +unlock:
> + v4l2_subdev_unlock_state(state);
> return ret;
> }
>
> @@ -504,21 +495,23 @@ sun6i_csi_bridge_mbus_format_prepare(struct v4l2_mbus_framefmt *mbus_format)
> static int sun6i_csi_bridge_init_state(struct v4l2_subdev *subdev,
> struct v4l2_subdev_state *state)
> {
> - struct sun6i_csi_device *csi_dev = v4l2_get_subdevdata(subdev);
> - unsigned int pad = SUN6I_CSI_BRIDGE_PAD_SINK;
> - struct v4l2_mbus_framefmt *mbus_format =
> - v4l2_subdev_state_get_format(state, pad);
> - struct mutex *lock = &csi_dev->bridge.lock;
> + unsigned int pad;
>
> - mutex_lock(lock);
> + /*
> + * This subdev does not perform format conversion,
> + * initialize both pads identically.
> + */
> + for (pad = 0; pad < subdev->entity.num_pads; pad++) {
> + struct v4l2_mbus_framefmt *mbus_format;
>
> - mbus_format->code = sun6i_csi_bridge_formats[0].mbus_code;
> - mbus_format->width = 1280;
> - mbus_format->height = 720;
> + mbus_format = v4l2_subdev_state_get_format(state, pad);
>
> - sun6i_csi_bridge_mbus_format_prepare(mbus_format);
> + mbus_format->code = sun6i_csi_bridge_formats[0].mbus_code;
> + mbus_format->width = 1280;
> + mbus_format->height = 720;
>
> - mutex_unlock(lock);
> + sun6i_csi_bridge_mbus_format_prepare(mbus_format);
> + }
>
> return 0;
> }
> @@ -536,53 +529,32 @@ sun6i_csi_bridge_enum_mbus_code(struct v4l2_subdev *subdev,
> return 0;
> }
>
> -static int sun6i_csi_bridge_get_fmt(struct v4l2_subdev *subdev,
> - struct v4l2_subdev_state *state,
> - struct v4l2_subdev_format *format)
> -{
> - struct sun6i_csi_device *csi_dev = v4l2_get_subdevdata(subdev);
> - struct v4l2_mbus_framefmt *mbus_format = &format->format;
> - struct mutex *lock = &csi_dev->bridge.lock;
> -
> - mutex_lock(lock);
> -
> - if (format->which == V4L2_SUBDEV_FORMAT_TRY)
> - *mbus_format = *v4l2_subdev_state_get_format(state,
> - format->pad);
> - else
> - *mbus_format = csi_dev->bridge.mbus_format;
> -
> - mutex_unlock(lock);
> -
> - return 0;
> -}
> -
> static int sun6i_csi_bridge_set_fmt(struct v4l2_subdev *subdev,
> struct v4l2_subdev_state *state,
> struct v4l2_subdev_format *format)
> {
> - struct sun6i_csi_device *csi_dev = v4l2_get_subdevdata(subdev);
> - struct v4l2_mbus_framefmt *mbus_format = &format->format;
> - struct mutex *lock = &csi_dev->bridge.lock;
> + struct v4l2_mbus_framefmt *fmt;
>
> - mutex_lock(lock);
> + /* The format on the source pad always matches the sink pad. */
> + if (format->pad != SUN6I_CSI_BRIDGE_PAD_SINK)
> + return v4l2_subdev_get_fmt(subdev, state, format);
>
> - sun6i_csi_bridge_mbus_format_prepare(mbus_format);
> + sun6i_csi_bridge_mbus_format_prepare(&format->format);
>
> - if (format->which == V4L2_SUBDEV_FORMAT_TRY)
> - *v4l2_subdev_state_get_format(state, format->pad) =
> - *mbus_format;
> - else
> - csi_dev->bridge.mbus_format = *mbus_format;
> + /* Set the format on the sink pad. */
> + fmt = v4l2_subdev_state_get_format(state, format->pad);
> + *fmt = format->format;
>
> - mutex_unlock(lock);
> + /* Propagate the format to the source pad. */
> + fmt = v4l2_subdev_state_get_format(state, SUN6I_CSI_BRIDGE_PAD_SOURCE);
> + *fmt = format->format;
>
> return 0;
> }
>
> static const struct v4l2_subdev_pad_ops sun6i_csi_bridge_pad_ops = {
> .enum_mbus_code = sun6i_csi_bridge_enum_mbus_code,
> - .get_fmt = sun6i_csi_bridge_get_fmt,
> + .get_fmt = v4l2_subdev_get_fmt,
> .set_fmt = sun6i_csi_bridge_set_fmt,
> };
>
> @@ -780,8 +752,6 @@ int sun6i_csi_bridge_setup(struct sun6i_csi_device *csi_dev)
> };
> int ret;
>
> - mutex_init(&bridge->lock);
> -
> /* V4L2 Subdev */
>
> v4l2_subdev_init(subdev, &sun6i_csi_bridge_subdev_ops);
> @@ -809,6 +779,12 @@ int sun6i_csi_bridge_setup(struct sun6i_csi_device *csi_dev)
> if (ret < 0)
> return ret;
>
> + /* V4L2 Subdev finalize */
> +
> + ret = v4l2_subdev_init_finalize(subdev);
> + if (ret < 0)
> + goto error_media_entity;
> +
> /* V4L2 Subdev */
>
> if (csi_dev->isp_available)
> @@ -818,7 +794,7 @@ int sun6i_csi_bridge_setup(struct sun6i_csi_device *csi_dev)
>
> if (ret) {
> dev_err(dev, "failed to register v4l2 subdev: %d\n", ret);
> - goto error_media_entity;
> + goto error_subdev_finalize;
> }
>
> /* V4L2 Async */
> @@ -852,6 +828,9 @@ int sun6i_csi_bridge_setup(struct sun6i_csi_device *csi_dev)
> else
> v4l2_device_unregister_subdev(subdev);
>
> +error_subdev_finalize:
> + v4l2_subdev_cleanup(subdev);
> +
> error_media_entity:
> media_entity_cleanup(&subdev->entity);
>
> @@ -868,5 +847,7 @@ void sun6i_csi_bridge_cleanup(struct sun6i_csi_device *csi_dev)
>
> v4l2_device_unregister_subdev(subdev);
>
> + v4l2_subdev_cleanup(subdev);
> +
> media_entity_cleanup(&subdev->entity);
> }
> diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_bridge.h b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_bridge.h
> index 44653b38f722..a5b0a6f064dd 100644
> --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_bridge.h
> +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_bridge.h
> @@ -42,20 +42,11 @@ struct sun6i_csi_bridge {
> struct v4l2_subdev subdev;
> struct v4l2_async_notifier notifier;
> struct media_pad pads[2];
> - struct v4l2_mbus_framefmt mbus_format;
> - struct mutex lock; /* Mbus format lock. */
>
> struct sun6i_csi_bridge_source source_parallel;
> struct sun6i_csi_bridge_source source_mipi_csi2;
> };
>
> -/* Helpers */
> -
> -void sun6i_csi_bridge_dimensions(struct sun6i_csi_device *csi_dev,
> - unsigned int *width, unsigned int *height);
> -void sun6i_csi_bridge_format(struct sun6i_csi_device *csi_dev,
> - u32 *mbus_code, u32 *field);
> -
> /* Format */
>
> const struct sun6i_csi_bridge_format *
> diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c
> index 65879f4802c0..d90abba21309 100644
> --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c
> +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c
> @@ -888,14 +888,19 @@ static int sun6i_csi_capture_link_validate(struct media_link *link)
> media_entity_to_video_device(link->sink->entity);
> struct sun6i_csi_device *csi_dev = video_get_drvdata(video_dev);
> struct v4l2_device *v4l2_dev = csi_dev->v4l2_dev;
> + struct v4l2_subdev *src_subdev =
> + media_entity_to_v4l2_subdev(link->source->entity);
> const struct sun6i_csi_capture_format *capture_format;
> const struct sun6i_csi_bridge_format *bridge_format;
> unsigned int capture_width, capture_height;
> - unsigned int bridge_width, bridge_height;
> const struct v4l2_format_info *format_info;
> + struct v4l2_subdev_format src_fmt = {
> + .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> + .pad = link->source->index
> + };
> u32 pixelformat, capture_field;
> - u32 mbus_code, bridge_field;
> bool match;
> + int ret;
>
> sun6i_csi_capture_dimensions(csi_dev, &capture_width, &capture_height);
>
> @@ -904,19 +909,22 @@ static int sun6i_csi_capture_link_validate(struct media_link *link)
> if (WARN_ON(!capture_format))
> return -EINVAL;
>
> - sun6i_csi_bridge_dimensions(csi_dev, &bridge_width, &bridge_height);
> + /* Resolve csi bridge format. */
> + ret = v4l2_subdev_call(src_subdev, pad, get_fmt, NULL, &src_fmt);
> + if (ret)
> + return ret;
>
> - sun6i_csi_bridge_format(csi_dev, &mbus_code, &bridge_field);
> - bridge_format = sun6i_csi_bridge_format_find(mbus_code);
> + bridge_format = sun6i_csi_bridge_format_find(src_fmt.format.code);
> if (WARN_ON(!bridge_format))
> return -EINVAL;
>
> /* No cropping/scaling is supported. */
> - if (capture_width != bridge_width || capture_height != bridge_height) {
> + if (capture_width != src_fmt.format.width ||
> + capture_height != src_fmt.format.height) {
> v4l2_err(v4l2_dev,
> "invalid input/output dimensions: %ux%u/%ux%u\n",
> - bridge_width, bridge_height, capture_width,
> - capture_height);
> + src_fmt.format.width, src_fmt.format.height,
> + capture_width, capture_height);
> return -EINVAL;
> }
>
> @@ -947,7 +955,8 @@ static int sun6i_csi_capture_link_validate(struct media_link *link)
> /* With raw input mode, we need a 1:1 match between input and output. */
> if (bridge_format->input_format == SUN6I_CSI_INPUT_FMT_RAW ||
> capture_format->input_format_raw) {
> - match = sun6i_csi_capture_format_match(pixelformat, mbus_code);
> + match = sun6i_csi_capture_format_match(pixelformat,
> + src_fmt.format.code);
> if (!match)
> goto invalid;
> }
> --
> 2.54.0
>
--
Regards,
Arash Golgol
More information about the linux-arm-kernel
mailing list