[PATCH v1 0/3] media: sun6i-csi: Convert to active state and improve MC support

[PATCH v1 0/3] media: sun6i-csi: Convert to active state and improve MC support

[Arash Golgol]

Hi,

This series modernizes the sun6i-csi driver by aligning the bridge and
capture components with the V4L2 subdev active state API and improving
media controller integration.

The first patch migrates the bridge driver to the V4L2 subdev active
state framework. The private mbus_format field and its associated lock
are removed, with the framework-managed state becoming the single
source of truth. As part of this change, the capture driver no longer
accesses bridge-private data directly and instead resolves the active
format through the subdev pad API. Since the hardware does not perform
format conversion, identical formats are enforced on both pads.

The second patch implements vidioc_enum_framesizes for the capture
device, reporting a stepwise range of even frame sizes within the
hardware limits.

The final patch adds MC-centric format enumeration support by filtering
pixel formats based on the provided mbus code and advertises
V4L2_CAP_IO_MC to reflect the intended usage within a media graph.

Hardware testing performed on LicheePi Zero Dock (Allwinner V3s):

  - Parallel pipeline (csi1):
    ov7670 -> sun6i-csi-bridge -> sun6i-csi-capture

  - MIPI pipeline (csi0):
    ov5647 -> sun6i-mipi-csi2 -> sun6i-csi-bridge -> sun6i-csi-capture


Arash Golgol (3):
  media: sun6i-csi: bridge: Use V4L2 subdev active state
  media: sun6i-csi: capture: Implement vidioc_enum_framesizes
  media: sun6i-csi: capture: Support MC-centric format enumeration

 .../sunxi/sun6i-csi/sun6i_csi_bridge.c        | 154 ++++++++----------
 .../sunxi/sun6i-csi/sun6i_csi_bridge.h        |   9 -
 .../sunxi/sun6i-csi/sun6i_csi_capture.c       |  87 ++++++++--
 3 files changed, 142 insertions(+), 108 deletions(-)

-- 
2.34.1

[PATCH v1 1/3] media: sun6i-csi: bridge: Use V4L2 subdev active state

[Arash Golgol]

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@gmail.com>
---
 .../sunxi/sun6i-csi/sun6i_csi_bridge.c        | 154 ++++++++----------
 .../sunxi/sun6i-csi/sun6i_csi_bridge.h        |   9 -
 .../sunxi/sun6i-csi/sun6i_csi_capture.c       |  27 ++-
 3 files changed, 85 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..4406b0f8c839 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,19 @@ 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 +369,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 +394,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 +414,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 +428,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 +436,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 +459,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 +470,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 +494,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 +528,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 +751,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 +778,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 +793,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 +827,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 +846,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..a21a146fb02a 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.34.1

Re: [PATCH v1 1/3] media: sun6i-csi: bridge: Use V4L2 subdev active state

[Paul Kocialkowski]

[-- Attachment #1: Type: text/plain, Size: 16545 bytes --]

Hi Arash,

Le Tue 17 Feb 26, 10:10, Arash Golgol a écrit :
> 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@gmail.com>

This looks very good to me, thanks for your great work!

Reviewed-by: Paul Kocialkowski <paulk@sys-base.io>
Tested-by: Paul Kocialkowski <paulk@sys-base.io>

All the best,

Paul

> ---
>  .../sunxi/sun6i-csi/sun6i_csi_bridge.c        | 154 ++++++++----------
>  .../sunxi/sun6i-csi/sun6i_csi_bridge.h        |   9 -
>  .../sunxi/sun6i-csi/sun6i_csi_capture.c       |  27 ++-
>  3 files changed, 85 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..4406b0f8c839 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,19 @@ 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 +369,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 +394,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 +414,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 +428,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 +436,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 +459,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 +470,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 +494,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 +528,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 +751,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 +778,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 +793,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 +827,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 +846,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..a21a146fb02a 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.34.1
> 

-- 
Paul Kocialkowski,

Independent contractor - sys-base - https://www.sys-base.io/
Free software developer - https://www.paulk.fr/

Expert in multimedia, graphics and embedded hardware support with Linux.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v1 1/3] media: sun6i-csi: bridge: Use V4L2 subdev active state

[Laurent Pinchart]

Hi Arash,

Thank you for the patch.

On Tue, Feb 17, 2026 at 10:10:48AM +0330, Arash Golgol wrote:
> 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@gmail.com>
> ---
>  .../sunxi/sun6i-csi/sun6i_csi_bridge.c        | 154 ++++++++----------
>  .../sunxi/sun6i-csi/sun6i_csi_bridge.h        |   9 -
>  .../sunxi/sun6i-csi/sun6i_csi_capture.c       |  27 ++-
>  3 files changed, 85 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..4406b0f8c839 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,19 @@ 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 +369,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 +394,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 +414,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 +428,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 +436,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 +459,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 +470,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 +494,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 +528,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 +751,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 +778,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 +793,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 +827,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 +846,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..a21a146fb02a 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) {

	if (capture_width != src_fmt.format.width ||
	    capture_height != src_fmt.format.height) {

With that,

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

>  		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;
>  	}

-- 
Regards,

Laurent Pinchart

[PATCH v1 2/3] media: sun6i-csi: capture: Implement vidioc_enum_framesizes

[Arash Golgol]

Report the stepwise frame size range supported by the CSI capture
hardware for the pixel formats exposed by the driver.

The hardware does not perform scaling and accepts any even width and
height within the reported limits.

Signed-off-by: Arash Golgol <arash.golgol@gmail.com>
---
 .../sunxi/sun6i-csi/sun6i_csi_capture.c       | 22 +++++++++++++++++++
 1 file changed, 22 insertions(+)

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 a21a146fb02a..dd06d4c116e0 100644
--- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c
+++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c
@@ -738,6 +738,27 @@ static int sun6i_csi_capture_enum_fmt(struct file *file, void *priv,
 	return 0;
 }
 
+static int sun6i_csi_capture_enum_framesize(struct file *file, void *fh,
+					     struct v4l2_frmsizeenum *fsize)
+{
+	if (fsize->index)
+		return -EINVAL;
+
+	/* Only accept format in map table. */
+	if (!sun6i_csi_capture_format_find(fsize->pixel_format))
+		return -EINVAL;
+
+	fsize->type = V4L2_FRMSIZE_TYPE_STEPWISE;
+	fsize->stepwise.min_width = SUN6I_CSI_CAPTURE_WIDTH_MIN;
+	fsize->stepwise.max_width = SUN6I_CSI_CAPTURE_WIDTH_MAX;
+	fsize->stepwise.min_height = SUN6I_CSI_CAPTURE_HEIGHT_MIN;
+	fsize->stepwise.max_height = SUN6I_CSI_CAPTURE_HEIGHT_MAX;
+	fsize->stepwise.step_width = 2;
+	fsize->stepwise.step_height = 2;
+
+	return 0;
+}
+
 static int sun6i_csi_capture_g_fmt(struct file *file, void *priv,
 				   struct v4l2_format *format)
 {
@@ -805,6 +826,7 @@ static const struct v4l2_ioctl_ops sun6i_csi_capture_ioctl_ops = {
 	.vidioc_querycap		= sun6i_csi_capture_querycap,
 
 	.vidioc_enum_fmt_vid_cap	= sun6i_csi_capture_enum_fmt,
+	.vidioc_enum_framesizes		= sun6i_csi_capture_enum_framesize,
 	.vidioc_g_fmt_vid_cap		= sun6i_csi_capture_g_fmt,
 	.vidioc_s_fmt_vid_cap		= sun6i_csi_capture_s_fmt,
 	.vidioc_try_fmt_vid_cap		= sun6i_csi_capture_try_fmt,
-- 
2.34.1

Re: [PATCH v1 2/3] media: sun6i-csi: capture: Implement vidioc_enum_framesizes

[Paul Kocialkowski]

[-- Attachment #1: Type: text/plain, Size: 2578 bytes --]

Hi,

Le Tue 17 Feb 26, 10:10, Arash Golgol a écrit :
> Report the stepwise frame size range supported by the CSI capture
> hardware for the pixel formats exposed by the driver.
> 
> The hardware does not perform scaling and accepts any even width and
> height within the reported limits.
> 
> Signed-off-by: Arash Golgol <arash.golgol@gmail.com>

Looks good to me.

Reviewed-by: Paul Kocialkowski <paulk@sys-base.io>
Tested-by: Paul Kocialkowski <paulk@sys-base.io>

All the best,

Paul

> ---
>  .../sunxi/sun6i-csi/sun6i_csi_capture.c       | 22 +++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> 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 a21a146fb02a..dd06d4c116e0 100644
> --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c
> +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c
> @@ -738,6 +738,27 @@ static int sun6i_csi_capture_enum_fmt(struct file *file, void *priv,
>  	return 0;
>  }
>  
> +static int sun6i_csi_capture_enum_framesize(struct file *file, void *fh,
> +					     struct v4l2_frmsizeenum *fsize)
> +{
> +	if (fsize->index)
> +		return -EINVAL;
> +
> +	/* Only accept format in map table. */
> +	if (!sun6i_csi_capture_format_find(fsize->pixel_format))
> +		return -EINVAL;
> +
> +	fsize->type = V4L2_FRMSIZE_TYPE_STEPWISE;
> +	fsize->stepwise.min_width = SUN6I_CSI_CAPTURE_WIDTH_MIN;
> +	fsize->stepwise.max_width = SUN6I_CSI_CAPTURE_WIDTH_MAX;
> +	fsize->stepwise.min_height = SUN6I_CSI_CAPTURE_HEIGHT_MIN;
> +	fsize->stepwise.max_height = SUN6I_CSI_CAPTURE_HEIGHT_MAX;
> +	fsize->stepwise.step_width = 2;
> +	fsize->stepwise.step_height = 2;
> +
> +	return 0;
> +}
> +
>  static int sun6i_csi_capture_g_fmt(struct file *file, void *priv,
>  				   struct v4l2_format *format)
>  {
> @@ -805,6 +826,7 @@ static const struct v4l2_ioctl_ops sun6i_csi_capture_ioctl_ops = {
>  	.vidioc_querycap		= sun6i_csi_capture_querycap,
>  
>  	.vidioc_enum_fmt_vid_cap	= sun6i_csi_capture_enum_fmt,
> +	.vidioc_enum_framesizes		= sun6i_csi_capture_enum_framesize,
>  	.vidioc_g_fmt_vid_cap		= sun6i_csi_capture_g_fmt,
>  	.vidioc_s_fmt_vid_cap		= sun6i_csi_capture_s_fmt,
>  	.vidioc_try_fmt_vid_cap		= sun6i_csi_capture_try_fmt,
> -- 
> 2.34.1
> 

-- 
Paul Kocialkowski,

Independent contractor - sys-base - https://www.sys-base.io/
Free software developer - https://www.paulk.fr/

Expert in multimedia, graphics and embedded hardware support with Linux.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v1 2/3] media: sun6i-csi: capture: Implement vidioc_enum_framesizes

[Laurent Pinchart]

On Tue, Feb 17, 2026 at 10:10:49AM +0330, Arash Golgol wrote:
> Report the stepwise frame size range supported by the CSI capture
> hardware for the pixel formats exposed by the driver.
> 
> The hardware does not perform scaling and accepts any even width and
> height within the reported limits.
> 
> Signed-off-by: Arash Golgol <arash.golgol@gmail.com>

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

> ---
>  .../sunxi/sun6i-csi/sun6i_csi_capture.c       | 22 +++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> 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 a21a146fb02a..dd06d4c116e0 100644
> --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c
> +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c
> @@ -738,6 +738,27 @@ static int sun6i_csi_capture_enum_fmt(struct file *file, void *priv,
>  	return 0;
>  }
>  
> +static int sun6i_csi_capture_enum_framesize(struct file *file, void *fh,
> +					     struct v4l2_frmsizeenum *fsize)
> +{
> +	if (fsize->index)
> +		return -EINVAL;
> +
> +	/* Only accept format in map table. */
> +	if (!sun6i_csi_capture_format_find(fsize->pixel_format))
> +		return -EINVAL;
> +
> +	fsize->type = V4L2_FRMSIZE_TYPE_STEPWISE;
> +	fsize->stepwise.min_width = SUN6I_CSI_CAPTURE_WIDTH_MIN;
> +	fsize->stepwise.max_width = SUN6I_CSI_CAPTURE_WIDTH_MAX;
> +	fsize->stepwise.min_height = SUN6I_CSI_CAPTURE_HEIGHT_MIN;
> +	fsize->stepwise.max_height = SUN6I_CSI_CAPTURE_HEIGHT_MAX;
> +	fsize->stepwise.step_width = 2;
> +	fsize->stepwise.step_height = 2;
> +
> +	return 0;
> +}
> +
>  static int sun6i_csi_capture_g_fmt(struct file *file, void *priv,
>  				   struct v4l2_format *format)
>  {
> @@ -805,6 +826,7 @@ static const struct v4l2_ioctl_ops sun6i_csi_capture_ioctl_ops = {
>  	.vidioc_querycap		= sun6i_csi_capture_querycap,
>  
>  	.vidioc_enum_fmt_vid_cap	= sun6i_csi_capture_enum_fmt,
> +	.vidioc_enum_framesizes		= sun6i_csi_capture_enum_framesize,
>  	.vidioc_g_fmt_vid_cap		= sun6i_csi_capture_g_fmt,
>  	.vidioc_s_fmt_vid_cap		= sun6i_csi_capture_s_fmt,
>  	.vidioc_try_fmt_vid_cap		= sun6i_csi_capture_try_fmt,

-- 
Regards,

Laurent Pinchart

[PATCH v1 3/3] media: sun6i-csi: capture: Support MC-centric format enumeration

[Arash Golgol]

Extend vidioc_enum_fmt to support MC-centric enumeration by filtering
pixel formats based on the provided mbus code. Advertise MC I/O support
on the video device to reflect its intended usage within a media graph.

Signed-off-by: Arash Golgol <arash.golgol@gmail.com>
---
 .../sunxi/sun6i-csi/sun6i_csi_capture.c       | 38 +++++++++++++++++--
 1 file changed, 35 insertions(+), 3 deletions(-)

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 dd06d4c116e0..f54d7a6397be 100644
--- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c
+++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c
@@ -327,6 +327,21 @@ static bool sun6i_csi_capture_format_match(u32 pixelformat, u32 mbus_code)
 	return false;
 }
 
+static const u32 *sun6i_csi_capture_pixelformat_find(u32 mbus_code)
+{
+	unsigned int i;
+
+	for (i = 0; i < ARRAY_SIZE(sun6i_csi_capture_format_matches); i++) {
+		const struct sun6i_csi_capture_format_match *match =
+			&sun6i_csi_capture_format_matches[i];
+
+		if (match->mbus_code == mbus_code)
+			return &match->pixelformat;
+	}
+
+	return NULL;
+}
+
 /* Capture */
 
 static void
@@ -729,11 +744,27 @@ static int sun6i_csi_capture_enum_fmt(struct file *file, void *priv,
 				      struct v4l2_fmtdesc *fmtdesc)
 {
 	u32 index = fmtdesc->index;
+	u32 mbus_code = fmtdesc->mbus_code;
+	const u32 *pixelformat;
+
+	/* MC-centric or Video-node-centric */
+	if (mbus_code) {
+		/* There is only one pixelformat for a mbus_code. */
+		if (index)
+			return -EINVAL;
+
+		pixelformat = sun6i_csi_capture_pixelformat_find(mbus_code);
+	} else {
+		if (index >= ARRAY_SIZE(sun6i_csi_capture_formats))
+			return -EINVAL;
+
+		pixelformat = &sun6i_csi_capture_formats[index].pixelformat;
+	}
 
-	if (index >= ARRAY_SIZE(sun6i_csi_capture_formats))
+	if (!pixelformat)
 		return -EINVAL;
 
-	fmtdesc->pixelformat = sun6i_csi_capture_formats[index].pixelformat;
+	fmtdesc->pixelformat = *pixelformat;
 
 	return 0;
 }
@@ -1065,7 +1096,8 @@ int sun6i_csi_capture_setup(struct sun6i_csi_device *csi_dev)
 
 	strscpy(video_dev->name, SUN6I_CSI_CAPTURE_NAME,
 		sizeof(video_dev->name));
-	video_dev->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING;
+	video_dev->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING |
+				 V4L2_CAP_IO_MC;
 	video_dev->vfl_dir = VFL_DIR_RX;
 	video_dev->release = video_device_release_empty;
 	video_dev->fops = &sun6i_csi_capture_fops;
-- 
2.34.1

Re: [PATCH v1 3/3] media: sun6i-csi: capture: Support MC-centric format enumeration

[Paul Kocialkowski]

[-- Attachment #1: Type: text/plain, Size: 3254 bytes --]

Hi Arash,

Le Tue 17 Feb 26, 10:10, Arash Golgol a écrit :
> Extend vidioc_enum_fmt to support MC-centric enumeration by filtering
> pixel formats based on the provided mbus code. Advertise MC I/O support
> on the video device to reflect its intended usage within a media graph.
> 
> Signed-off-by: Arash Golgol <arash.golgol@gmail.com>
> ---
>  .../sunxi/sun6i-csi/sun6i_csi_capture.c       | 38 +++++++++++++++++--
>  1 file changed, 35 insertions(+), 3 deletions(-)
> 
> 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 dd06d4c116e0..f54d7a6397be 100644
> --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c
> +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c
> @@ -327,6 +327,21 @@ static bool sun6i_csi_capture_format_match(u32 pixelformat, u32 mbus_code)
>  	return false;
>  }
>  
> +static const u32 *sun6i_csi_capture_pixelformat_find(u32 mbus_code)

I am not a big fan of returning a pointer here. We can safely assume
that a valid pixel format will be non-zero (it's a fourcc), so I would
just return the pixelformat directly and 0 for invalid cases.

All the best,

Paul

> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(sun6i_csi_capture_format_matches); i++) {
> +		const struct sun6i_csi_capture_format_match *match =
> +			&sun6i_csi_capture_format_matches[i];
> +
> +		if (match->mbus_code == mbus_code)
> +			return &match->pixelformat;
> +	}
> +
> +	return NULL;
> +}
> +
>  /* Capture */
>  
>  static void
> @@ -729,11 +744,27 @@ static int sun6i_csi_capture_enum_fmt(struct file *file, void *priv,
>  				      struct v4l2_fmtdesc *fmtdesc)
>  {
>  	u32 index = fmtdesc->index;
> +	u32 mbus_code = fmtdesc->mbus_code;
> +	const u32 *pixelformat;
> +
> +	/* MC-centric or Video-node-centric */
> +	if (mbus_code) {
> +		/* There is only one pixelformat for a mbus_code. */
> +		if (index)
> +			return -EINVAL;
> +
> +		pixelformat = sun6i_csi_capture_pixelformat_find(mbus_code);
> +	} else {
> +		if (index >= ARRAY_SIZE(sun6i_csi_capture_formats))
> +			return -EINVAL;
> +
> +		pixelformat = &sun6i_csi_capture_formats[index].pixelformat;
> +	}
>  
> -	if (index >= ARRAY_SIZE(sun6i_csi_capture_formats))
> +	if (!pixelformat)
>  		return -EINVAL;
>  
> -	fmtdesc->pixelformat = sun6i_csi_capture_formats[index].pixelformat;
> +	fmtdesc->pixelformat = *pixelformat;
>  
>  	return 0;
>  }
> @@ -1065,7 +1096,8 @@ int sun6i_csi_capture_setup(struct sun6i_csi_device *csi_dev)
>  
>  	strscpy(video_dev->name, SUN6I_CSI_CAPTURE_NAME,
>  		sizeof(video_dev->name));
> -	video_dev->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING;
> +	video_dev->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING |
> +				 V4L2_CAP_IO_MC;
>  	video_dev->vfl_dir = VFL_DIR_RX;
>  	video_dev->release = video_device_release_empty;
>  	video_dev->fops = &sun6i_csi_capture_fops;
> -- 
> 2.34.1
> 

-- 
Paul Kocialkowski,

Independent contractor - sys-base - https://www.sys-base.io/
Free software developer - https://www.paulk.fr/

Expert in multimedia, graphics and embedded hardware support with Linux.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v1 3/3] media: sun6i-csi: capture: Support MC-centric format enumeration

[Laurent Pinchart]

On Thu, May 07, 2026 at 01:58:08PM +0200, Paul Kocialkowski wrote:
> Le Tue 17 Feb 26, 10:10, Arash Golgol a écrit :
> > Extend vidioc_enum_fmt to support MC-centric enumeration by filtering
> > pixel formats based on the provided mbus code. Advertise MC I/O support
> > on the video device to reflect its intended usage within a media graph.
> > 
> > Signed-off-by: Arash Golgol <arash.golgol@gmail.com>
> > ---
> >  .../sunxi/sun6i-csi/sun6i_csi_capture.c       | 38 +++++++++++++++++--
> >  1 file changed, 35 insertions(+), 3 deletions(-)
> > 
> > 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 dd06d4c116e0..f54d7a6397be 100644
> > --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c
> > +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c
> > @@ -327,6 +327,21 @@ static bool sun6i_csi_capture_format_match(u32 pixelformat, u32 mbus_code)
> >  	return false;
> >  }
> >  
> > +static const u32 *sun6i_csi_capture_pixelformat_find(u32 mbus_code)
> 
> I am not a big fan of returning a pointer here. We can safely assume
> that a valid pixel format will be non-zero (it's a fourcc), so I would
> just return the pixelformat directly and 0 for invalid cases.

I agree. The rest of the patch looks fine. With this addressed,

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

> > +{
> > +	unsigned int i;
> > +
> > +	for (i = 0; i < ARRAY_SIZE(sun6i_csi_capture_format_matches); i++) {
> > +		const struct sun6i_csi_capture_format_match *match =
> > +			&sun6i_csi_capture_format_matches[i];
> > +
> > +		if (match->mbus_code == mbus_code)
> > +			return &match->pixelformat;
> > +	}
> > +
> > +	return NULL;
> > +}
> > +
> >  /* Capture */
> >  
> >  static void
> > @@ -729,11 +744,27 @@ static int sun6i_csi_capture_enum_fmt(struct file *file, void *priv,
> >  				      struct v4l2_fmtdesc *fmtdesc)
> >  {
> >  	u32 index = fmtdesc->index;
> > +	u32 mbus_code = fmtdesc->mbus_code;
> > +	const u32 *pixelformat;
> > +
> > +	/* MC-centric or Video-node-centric */
> > +	if (mbus_code) {
> > +		/* There is only one pixelformat for a mbus_code. */
> > +		if (index)
> > +			return -EINVAL;
> > +
> > +		pixelformat = sun6i_csi_capture_pixelformat_find(mbus_code);
> > +	} else {
> > +		if (index >= ARRAY_SIZE(sun6i_csi_capture_formats))
> > +			return -EINVAL;
> > +
> > +		pixelformat = &sun6i_csi_capture_formats[index].pixelformat;
> > +	}
> >  
> > -	if (index >= ARRAY_SIZE(sun6i_csi_capture_formats))
> > +	if (!pixelformat)
> >  		return -EINVAL;
> >  
> > -	fmtdesc->pixelformat = sun6i_csi_capture_formats[index].pixelformat;
> > +	fmtdesc->pixelformat = *pixelformat;
> >  
> >  	return 0;
> >  }
> > @@ -1065,7 +1096,8 @@ int sun6i_csi_capture_setup(struct sun6i_csi_device *csi_dev)
> >  
> >  	strscpy(video_dev->name, SUN6I_CSI_CAPTURE_NAME,
> >  		sizeof(video_dev->name));
> > -	video_dev->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING;
> > +	video_dev->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING |
> > +				 V4L2_CAP_IO_MC;
> >  	video_dev->vfl_dir = VFL_DIR_RX;
> >  	video_dev->release = video_device_release_empty;
> >  	video_dev->fops = &sun6i_csi_capture_fops;

-- 
Regards,

Laurent Pinchart

Re: [PATCH v1 0/3] media: sun6i-csi: Convert to active state and improve MC support

[arash golgol]

Hi,

This is a gentle ping on the series.
Link to series:
https://patchwork.kernel.org/project/linux-media/list/?series=1054768

-- 
Regards,
Arash Golgol

On Tue, Feb 17, 2026 at 10:11 AM Arash Golgol <arash.golgol@gmail.com> wrote:
>
> Hi,
>
> This series modernizes the sun6i-csi driver by aligning the bridge and
> capture components with the V4L2 subdev active state API and improving
> media controller integration.
>
> The first patch migrates the bridge driver to the V4L2 subdev active
> state framework. The private mbus_format field and its associated lock
> are removed, with the framework-managed state becoming the single
> source of truth. As part of this change, the capture driver no longer
> accesses bridge-private data directly and instead resolves the active
> format through the subdev pad API. Since the hardware does not perform
> format conversion, identical formats are enforced on both pads.
>
> The second patch implements vidioc_enum_framesizes for the capture
> device, reporting a stepwise range of even frame sizes within the
> hardware limits.
>
> The final patch adds MC-centric format enumeration support by filtering
> pixel formats based on the provided mbus code and advertises
> V4L2_CAP_IO_MC to reflect the intended usage within a media graph.
>
> Hardware testing performed on LicheePi Zero Dock (Allwinner V3s):
>
>   - Parallel pipeline (csi1):
>     ov7670 -> sun6i-csi-bridge -> sun6i-csi-capture
>
>   - MIPI pipeline (csi0):
>     ov5647 -> sun6i-mipi-csi2 -> sun6i-csi-bridge -> sun6i-csi-capture
>
>
> Arash Golgol (3):
>   media: sun6i-csi: bridge: Use V4L2 subdev active state
>   media: sun6i-csi: capture: Implement vidioc_enum_framesizes
>   media: sun6i-csi: capture: Support MC-centric format enumeration
>
>  .../sunxi/sun6i-csi/sun6i_csi_bridge.c        | 154 ++++++++----------
>  .../sunxi/sun6i-csi/sun6i_csi_bridge.h        |   9 -
>  .../sunxi/sun6i-csi/sun6i_csi_capture.c       |  87 ++++++++--
>  3 files changed, 142 insertions(+), 108 deletions(-)
>
> --
> 2.34.1
>