* [PATCH v4 1/3] media: sun6i-mipi-csi2: Use V4L2 subdev active state
2026-08-04 7:26 [PATCH v1 0/3] media: sunxi: Resend active-state conversions Arash Golgol
@ 2026-08-04 7:26 ` Arash Golgol
2026-08-04 7:26 ` [PATCH v4 2/3] media: sun8i-a83t-mipi-csi2: " Arash Golgol
2026-08-04 7:26 ` [PATCH v5 3/3] media: sun6i-csi: bridge: " Arash Golgol
2 siblings, 0 replies; 4+ messages in thread
From: Arash Golgol @ 2026-08-04 7:26 UTC (permalink / raw)
To: linux-media, linux-arm-kernel, linux-sunxi
Cc: yong.deng, paulk, laurent.pinchart, mchehab, wens, jernej.skrabec,
samuel, 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.
The sun6i-mipi-csi2 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>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Paul Kocialkowski <paulk@sys-base.io>
Tested-by: Paul Kocialkowski <paulk@sys-base.io>
---
No changes in v4, just resend
- Link to v3: https://patchwork.linuxtv.org/project/linux-media/patch/20260214050943.6306-1-arash.golgol@gmail.com/
- Link to v3: https://lore.kernel.org/linux-media/20260214050943.6306-1-arash.golgol@gmail.com/
Changes in v3:
- link to v2: https://patchwork.kernel.org/project/linux-media/patch/20260209055529.16644-1-arash.golgol@gmail.com/
- Keep error path jumping to error_v4l2_notifier_cleanup on
bridge setup failure
Changes in v2:
- link to v1: https://patchwork.kernel.org/project/linux-media/patch/20260206123455.46476-1-arash.golgol@gmail.com/
- Simplify control flow by dropping the else at end of s_stream()
- Call v4l2_subdev_cleanup() on bridge setup failure before
notifier registration
.../sunxi/sun6i-mipi-csi2/sun6i_mipi_csi2.c | 107 +++++++++---------
.../sunxi/sun6i-mipi-csi2/sun6i_mipi_csi2.h | 2 -
2 files changed, 53 insertions(+), 56 deletions(-)
diff --git a/drivers/media/platform/sunxi/sun6i-mipi-csi2/sun6i_mipi_csi2.c b/drivers/media/platform/sunxi/sun6i-mipi-csi2/sun6i_mipi_csi2.c
index b06cb73015cd..17a9a215a98a 100644
--- a/drivers/media/platform/sunxi/sun6i-mipi-csi2/sun6i_mipi_csi2.c
+++ b/drivers/media/platform/sunxi/sun6i-mipi-csi2/sun6i_mipi_csi2.c
@@ -95,12 +95,12 @@ static void sun6i_mipi_csi2_disable(struct sun6i_mipi_csi2_device *csi2_dev)
SUN6I_MIPI_CSI2_CTL_EN, 0);
}
-static void sun6i_mipi_csi2_configure(struct sun6i_mipi_csi2_device *csi2_dev)
+static void sun6i_mipi_csi2_configure(struct sun6i_mipi_csi2_device *csi2_dev,
+ const struct v4l2_mbus_framefmt *mbus_format)
{
struct regmap *regmap = csi2_dev->regmap;
unsigned int lanes_count =
csi2_dev->bridge.endpoint.bus.mipi_csi2.num_data_lanes;
- struct v4l2_mbus_framefmt *mbus_format = &csi2_dev->bridge.mbus_format;
const struct sun6i_mipi_csi2_format *format;
struct device *dev = csi2_dev->dev;
u32 version = 0;
@@ -173,7 +173,8 @@ static int sun6i_mipi_csi2_s_stream(struct v4l2_subdev *subdev, int on)
struct v4l2_subdev *source_subdev = csi2_dev->bridge.source_subdev;
union phy_configure_opts dphy_opts = { 0 };
struct phy_configure_opts_mipi_dphy *dphy_cfg = &dphy_opts.mipi_dphy;
- struct v4l2_mbus_framefmt *mbus_format = &csi2_dev->bridge.mbus_format;
+ struct v4l2_subdev_state *state;
+ const struct v4l2_mbus_framefmt *mbus_format;
const struct sun6i_mipi_csi2_format *format;
struct phy *dphy = csi2_dev->dphy;
struct device *dev = csi2_dev->dev;
@@ -183,8 +184,12 @@ static int sun6i_mipi_csi2_s_stream(struct v4l2_subdev *subdev, int on)
unsigned long pixel_rate;
int ret;
- if (!source_subdev)
- return -ENODEV;
+ state = v4l2_subdev_lock_and_get_active_state(subdev);
+
+ if (!source_subdev) {
+ ret = -ENODEV;
+ goto unlock;
+ }
if (!on) {
v4l2_subdev_call(source_subdev, video, s_stream, 0);
@@ -196,7 +201,7 @@ static int sun6i_mipi_csi2_s_stream(struct v4l2_subdev *subdev, int on)
ret = pm_runtime_resume_and_get(dev);
if (ret < 0)
- return ret;
+ goto unlock;
/* Sensor Pixel Rate */
@@ -222,6 +227,8 @@ static int sun6i_mipi_csi2_s_stream(struct v4l2_subdev *subdev, int on)
goto error_pm;
}
+ mbus_format = v4l2_subdev_state_get_format(state,
+ SUN6I_MIPI_CSI2_PAD_SINK);
format = sun6i_mipi_csi2_format_find(mbus_format->code);
if (WARN_ON(!format)) {
ret = -ENODEV;
@@ -260,7 +267,7 @@ static int sun6i_mipi_csi2_s_stream(struct v4l2_subdev *subdev, int on)
/* Controller */
- sun6i_mipi_csi2_configure(csi2_dev);
+ sun6i_mipi_csi2_configure(csi2_dev, mbus_format);
sun6i_mipi_csi2_enable(csi2_dev);
/* D-PHY */
@@ -277,7 +284,8 @@ static int sun6i_mipi_csi2_s_stream(struct v4l2_subdev *subdev, int on)
if (ret && ret != -ENOIOCTLCMD)
goto disable;
- return 0;
+ ret = 0;
+ goto unlock;
disable:
phy_power_off(dphy);
@@ -286,6 +294,8 @@ static int sun6i_mipi_csi2_s_stream(struct v4l2_subdev *subdev, int on)
error_pm:
pm_runtime_put(dev);
+unlock:
+ v4l2_subdev_unlock_state(state);
return ret;
}
@@ -308,21 +318,23 @@ sun6i_mipi_csi2_mbus_format_prepare(struct v4l2_mbus_framefmt *mbus_format)
static int sun6i_mipi_csi2_init_state(struct v4l2_subdev *subdev,
struct v4l2_subdev_state *state)
{
- struct sun6i_mipi_csi2_device *csi2_dev = v4l2_get_subdevdata(subdev);
- unsigned int pad = SUN6I_MIPI_CSI2_PAD_SINK;
- struct v4l2_mbus_framefmt *mbus_format =
- v4l2_subdev_state_get_format(state, pad);
- struct mutex *lock = &csi2_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_mipi_csi2_formats[0].mbus_code;
- mbus_format->width = 640;
- mbus_format->height = 480;
+ mbus_format = v4l2_subdev_state_get_format(state, pad);
- sun6i_mipi_csi2_mbus_format_prepare(mbus_format);
+ mbus_format->code = sun6i_mipi_csi2_formats[0].mbus_code;
+ mbus_format->width = 640;
+ mbus_format->height = 480;
- mutex_unlock(lock);
+ sun6i_mipi_csi2_mbus_format_prepare(mbus_format);
+ }
return 0;
}
@@ -340,53 +352,32 @@ sun6i_mipi_csi2_enum_mbus_code(struct v4l2_subdev *subdev,
return 0;
}
-static int sun6i_mipi_csi2_get_fmt(struct v4l2_subdev *subdev,
- struct v4l2_subdev_state *state,
- struct v4l2_subdev_format *format)
-{
- struct sun6i_mipi_csi2_device *csi2_dev = v4l2_get_subdevdata(subdev);
- struct v4l2_mbus_framefmt *mbus_format = &format->format;
- struct mutex *lock = &csi2_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 = csi2_dev->bridge.mbus_format;
-
- mutex_unlock(lock);
-
- return 0;
-}
-
static int sun6i_mipi_csi2_set_fmt(struct v4l2_subdev *subdev,
struct v4l2_subdev_state *state,
struct v4l2_subdev_format *format)
{
- struct sun6i_mipi_csi2_device *csi2_dev = v4l2_get_subdevdata(subdev);
- struct v4l2_mbus_framefmt *mbus_format = &format->format;
- struct mutex *lock = &csi2_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_MIPI_CSI2_PAD_SINK)
+ return v4l2_subdev_get_fmt(subdev, state, format);
- sun6i_mipi_csi2_mbus_format_prepare(mbus_format);
+ sun6i_mipi_csi2_mbus_format_prepare(&format->format);
- if (format->which == V4L2_SUBDEV_FORMAT_TRY)
- *v4l2_subdev_state_get_format(state, format->pad) =
- *mbus_format;
- else
- csi2_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_MIPI_CSI2_PAD_SOURCE);
+ *fmt = format->format;
return 0;
}
static const struct v4l2_subdev_pad_ops sun6i_mipi_csi2_pad_ops = {
.enum_mbus_code = sun6i_mipi_csi2_enum_mbus_code,
- .get_fmt = sun6i_mipi_csi2_get_fmt,
+ .get_fmt = v4l2_subdev_get_fmt,
.set_fmt = sun6i_mipi_csi2_set_fmt,
};
@@ -502,8 +493,6 @@ static int sun6i_mipi_csi2_bridge_setup(struct sun6i_mipi_csi2_device *csi2_dev)
bool notifier_registered = false;
int ret;
- mutex_init(&bridge->lock);
-
/* V4L2 Subdev */
v4l2_subdev_init(subdev, &sun6i_mipi_csi2_subdev_ops);
@@ -532,6 +521,12 @@ static int sun6i_mipi_csi2_bridge_setup(struct sun6i_mipi_csi2_device *csi2_dev)
if (ret)
return ret;
+ /* V4L2 Subdev finalize */
+
+ ret = v4l2_subdev_init_finalize(subdev);
+ if (ret < 0)
+ goto error_media_entity_cleanup;
+
/* V4L2 Async */
v4l2_async_subdev_nf_init(notifier, subdev);
@@ -565,6 +560,9 @@ static int sun6i_mipi_csi2_bridge_setup(struct sun6i_mipi_csi2_device *csi2_dev)
error_v4l2_notifier_cleanup:
v4l2_async_nf_cleanup(notifier);
+ v4l2_subdev_cleanup(subdev);
+
+error_media_entity_cleanup:
media_entity_cleanup(&subdev->entity);
return ret;
@@ -579,6 +577,7 @@ sun6i_mipi_csi2_bridge_cleanup(struct sun6i_mipi_csi2_device *csi2_dev)
v4l2_async_unregister_subdev(subdev);
v4l2_async_nf_unregister(notifier);
v4l2_async_nf_cleanup(notifier);
+ v4l2_subdev_cleanup(subdev);
media_entity_cleanup(&subdev->entity);
}
diff --git a/drivers/media/platform/sunxi/sun6i-mipi-csi2/sun6i_mipi_csi2.h b/drivers/media/platform/sunxi/sun6i-mipi-csi2/sun6i_mipi_csi2.h
index 24b15e34b5e8..d72dfbd6a993 100644
--- a/drivers/media/platform/sunxi/sun6i-mipi-csi2/sun6i_mipi_csi2.h
+++ b/drivers/media/platform/sunxi/sun6i-mipi-csi2/sun6i_mipi_csi2.h
@@ -32,8 +32,6 @@ struct sun6i_mipi_csi2_bridge {
struct media_pad pads[SUN6I_MIPI_CSI2_PAD_COUNT];
struct v4l2_fwnode_endpoint endpoint;
struct v4l2_async_notifier notifier;
- struct v4l2_mbus_framefmt mbus_format;
- struct mutex lock; /* Mbus format lock. */
struct v4l2_subdev *source_subdev;
};
--
2.34.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH v4 2/3] media: sun8i-a83t-mipi-csi2: Use V4L2 subdev active state
2026-08-04 7:26 [PATCH v1 0/3] media: sunxi: Resend active-state conversions Arash Golgol
2026-08-04 7:26 ` [PATCH v4 1/3] media: sun6i-mipi-csi2: Use V4L2 subdev active state Arash Golgol
@ 2026-08-04 7:26 ` Arash Golgol
2026-08-04 7:26 ` [PATCH v5 3/3] media: sun6i-csi: bridge: " Arash Golgol
2 siblings, 0 replies; 4+ messages in thread
From: Arash Golgol @ 2026-08-04 7:26 UTC (permalink / raw)
To: linux-media, linux-arm-kernel, linux-sunxi
Cc: yong.deng, paulk, laurent.pinchart, mchehab, wens, jernej.skrabec,
samuel, 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.
The sun8i-a83t-mipi-csi2 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>
Reviewed-by: Paul Kocialkowski <paulk@sys-base.io>
Tested-by: Paul Kocialkowski <paulk@sys-base.io>
---
No changes in v4, just resend
- Link to v3: https://patchwork.linuxtv.org/project/linux-media/patch/20260515173101.8978-1-arash.golgol@gmail.com/
- Link to v3: https://lore.kernel.org/linux-media/20260515173101.8978-1-arash.golgol@gmail.com/
Changes in v3:
- Fix active state lock leak on runtime PM error path
Changes in v2:
- Initialize active state before calling v4l2_subdev_state_get_format()
- Fix line wrapping reported by checkpatch
- Link to media-ci report: https://linux-media.pages.freedesktop.org/-/users/patchwork/-/jobs/99865145/artifacts/report.htm
.../sun8i_a83t_mipi_csi2.c | 112 +++++++++---------
.../sun8i_a83t_mipi_csi2.h | 2 -
2 files changed, 55 insertions(+), 59 deletions(-)
diff --git a/drivers/media/platform/sunxi/sun8i-a83t-mipi-csi2/sun8i_a83t_mipi_csi2.c b/drivers/media/platform/sunxi/sun8i-a83t-mipi-csi2/sun8i_a83t_mipi_csi2.c
index dbc51daa4fe3..38a6ef19f4ff 100644
--- a/drivers/media/platform/sunxi/sun8i-a83t-mipi-csi2/sun8i_a83t_mipi_csi2.c
+++ b/drivers/media/platform/sunxi/sun8i-a83t-mipi-csi2/sun8i_a83t_mipi_csi2.c
@@ -144,12 +144,12 @@ sun8i_a83t_mipi_csi2_disable(struct sun8i_a83t_mipi_csi2_device *csi2_dev)
}
static void
-sun8i_a83t_mipi_csi2_configure(struct sun8i_a83t_mipi_csi2_device *csi2_dev)
+sun8i_a83t_mipi_csi2_configure(struct sun8i_a83t_mipi_csi2_device *csi2_dev,
+ const struct v4l2_mbus_framefmt *mbus_format)
{
struct regmap *regmap = csi2_dev->regmap;
unsigned int lanes_count =
csi2_dev->bridge.endpoint.bus.mipi_csi2.num_data_lanes;
- struct v4l2_mbus_framefmt *mbus_format = &csi2_dev->bridge.mbus_format;
const struct sun8i_a83t_mipi_csi2_format *format;
struct device *dev = csi2_dev->dev;
u32 version = 0;
@@ -205,7 +205,8 @@ static int sun8i_a83t_mipi_csi2_s_stream(struct v4l2_subdev *subdev, int on)
struct v4l2_subdev *source_subdev = csi2_dev->bridge.source_subdev;
union phy_configure_opts dphy_opts = { 0 };
struct phy_configure_opts_mipi_dphy *dphy_cfg = &dphy_opts.mipi_dphy;
- struct v4l2_mbus_framefmt *mbus_format = &csi2_dev->bridge.mbus_format;
+ struct v4l2_subdev_state *state;
+ const struct v4l2_mbus_framefmt *mbus_format;
const struct sun8i_a83t_mipi_csi2_format *format;
struct phy *dphy = csi2_dev->dphy;
struct device *dev = csi2_dev->dev;
@@ -215,8 +216,12 @@ static int sun8i_a83t_mipi_csi2_s_stream(struct v4l2_subdev *subdev, int on)
unsigned long pixel_rate;
int ret;
- if (!source_subdev)
- return -ENODEV;
+ state = v4l2_subdev_lock_and_get_active_state(subdev);
+
+ if (!source_subdev) {
+ ret = -ENODEV;
+ goto unlock;
+ }
if (!on) {
v4l2_subdev_call(source_subdev, video, s_stream, 0);
@@ -228,7 +233,7 @@ static int sun8i_a83t_mipi_csi2_s_stream(struct v4l2_subdev *subdev, int on)
ret = pm_runtime_resume_and_get(dev);
if (ret < 0)
- return ret;
+ goto unlock;
/* Sensor pixel rate */
@@ -254,6 +259,9 @@ static int sun8i_a83t_mipi_csi2_s_stream(struct v4l2_subdev *subdev, int on)
goto error_pm;
}
+ mbus_format =
+ v4l2_subdev_state_get_format(state,
+ SUN8I_A83T_MIPI_CSI2_PAD_SINK);
format = sun8i_a83t_mipi_csi2_format_find(mbus_format->code);
if (WARN_ON(!format)) {
ret = -ENODEV;
@@ -292,7 +300,7 @@ static int sun8i_a83t_mipi_csi2_s_stream(struct v4l2_subdev *subdev, int on)
/* Controller */
- sun8i_a83t_mipi_csi2_configure(csi2_dev);
+ sun8i_a83t_mipi_csi2_configure(csi2_dev, mbus_format);
sun8i_a83t_mipi_csi2_enable(csi2_dev);
/* D-PHY */
@@ -309,7 +317,8 @@ static int sun8i_a83t_mipi_csi2_s_stream(struct v4l2_subdev *subdev, int on)
if (ret && ret != -ENOIOCTLCMD)
goto disable;
- return 0;
+ ret = 0;
+ goto unlock;
disable:
phy_power_off(dphy);
@@ -318,6 +327,8 @@ static int sun8i_a83t_mipi_csi2_s_stream(struct v4l2_subdev *subdev, int on)
error_pm:
pm_runtime_put(dev);
+unlock:
+ v4l2_subdev_unlock_state(state);
return ret;
}
@@ -341,22 +352,23 @@ sun8i_a83t_mipi_csi2_mbus_format_prepare(struct v4l2_mbus_framefmt *mbus_format)
static int sun8i_a83t_mipi_csi2_init_state(struct v4l2_subdev *subdev,
struct v4l2_subdev_state *state)
{
- struct sun8i_a83t_mipi_csi2_device *csi2_dev =
- v4l2_get_subdevdata(subdev);
- unsigned int pad = SUN8I_A83T_MIPI_CSI2_PAD_SINK;
- struct v4l2_mbus_framefmt *mbus_format =
- v4l2_subdev_state_get_format(state, pad);
- struct mutex *lock = &csi2_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 = sun8i_a83t_mipi_csi2_formats[0].mbus_code;
- mbus_format->width = 640;
- mbus_format->height = 480;
+ mbus_format = v4l2_subdev_state_get_format(state, pad);
- sun8i_a83t_mipi_csi2_mbus_format_prepare(mbus_format);
+ mbus_format->code = sun8i_a83t_mipi_csi2_formats[0].mbus_code;
+ mbus_format->width = 640;
+ mbus_format->height = 480;
- mutex_unlock(lock);
+ sun8i_a83t_mipi_csi2_mbus_format_prepare(mbus_format);
+ }
return 0;
}
@@ -375,55 +387,33 @@ sun8i_a83t_mipi_csi2_enum_mbus_code(struct v4l2_subdev *subdev,
return 0;
}
-static int sun8i_a83t_mipi_csi2_get_fmt(struct v4l2_subdev *subdev,
- struct v4l2_subdev_state *state,
- struct v4l2_subdev_format *format)
-{
- struct sun8i_a83t_mipi_csi2_device *csi2_dev =
- v4l2_get_subdevdata(subdev);
- struct v4l2_mbus_framefmt *mbus_format = &format->format;
- struct mutex *lock = &csi2_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 = csi2_dev->bridge.mbus_format;
-
- mutex_unlock(lock);
-
- return 0;
-}
-
static int sun8i_a83t_mipi_csi2_set_fmt(struct v4l2_subdev *subdev,
struct v4l2_subdev_state *state,
struct v4l2_subdev_format *format)
{
- struct sun8i_a83t_mipi_csi2_device *csi2_dev =
- v4l2_get_subdevdata(subdev);
- struct v4l2_mbus_framefmt *mbus_format = &format->format;
- struct mutex *lock = &csi2_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 != SUN8I_A83T_MIPI_CSI2_PAD_SINK)
+ return v4l2_subdev_get_fmt(subdev, state, format);
- sun8i_a83t_mipi_csi2_mbus_format_prepare(mbus_format);
+ sun8i_a83t_mipi_csi2_mbus_format_prepare(&format->format);
- if (format->which == V4L2_SUBDEV_FORMAT_TRY)
- *v4l2_subdev_state_get_format(state, format->pad) =
- *mbus_format;
- else
- csi2_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,
+ SUN8I_A83T_MIPI_CSI2_PAD_SOURCE);
+ *fmt = format->format;
return 0;
}
static const struct v4l2_subdev_pad_ops sun8i_a83t_mipi_csi2_pad_ops = {
.enum_mbus_code = sun8i_a83t_mipi_csi2_enum_mbus_code,
- .get_fmt = sun8i_a83t_mipi_csi2_get_fmt,
+ .get_fmt = v4l2_subdev_get_fmt,
.set_fmt = sun8i_a83t_mipi_csi2_set_fmt,
};
@@ -540,8 +530,6 @@ sun8i_a83t_mipi_csi2_bridge_setup(struct sun8i_a83t_mipi_csi2_device *csi2_dev)
bool notifier_registered = false;
int ret;
- mutex_init(&bridge->lock);
-
/* V4L2 Subdev */
v4l2_subdev_init(subdev, &sun8i_a83t_mipi_csi2_subdev_ops);
@@ -570,6 +558,12 @@ sun8i_a83t_mipi_csi2_bridge_setup(struct sun8i_a83t_mipi_csi2_device *csi2_dev)
if (ret)
return ret;
+ /* V4L2 Subdev finalize */
+
+ ret = v4l2_subdev_init_finalize(subdev);
+ if (ret < 0)
+ goto error_media_entity_cleanup;
+
/* V4L2 Async */
v4l2_async_subdev_nf_init(notifier, subdev);
@@ -603,6 +597,9 @@ sun8i_a83t_mipi_csi2_bridge_setup(struct sun8i_a83t_mipi_csi2_device *csi2_dev)
error_v4l2_notifier_cleanup:
v4l2_async_nf_cleanup(notifier);
+ v4l2_subdev_cleanup(subdev);
+
+error_media_entity_cleanup:
media_entity_cleanup(&subdev->entity);
return ret;
@@ -617,6 +614,7 @@ sun8i_a83t_mipi_csi2_bridge_cleanup(struct sun8i_a83t_mipi_csi2_device *csi2_dev
v4l2_async_unregister_subdev(subdev);
v4l2_async_nf_unregister(notifier);
v4l2_async_nf_cleanup(notifier);
+ v4l2_subdev_cleanup(subdev);
media_entity_cleanup(&subdev->entity);
}
diff --git a/drivers/media/platform/sunxi/sun8i-a83t-mipi-csi2/sun8i_a83t_mipi_csi2.h b/drivers/media/platform/sunxi/sun8i-a83t-mipi-csi2/sun8i_a83t_mipi_csi2.h
index f1e64c53434c..819527bcd64d 100644
--- a/drivers/media/platform/sunxi/sun8i-a83t-mipi-csi2/sun8i_a83t_mipi_csi2.h
+++ b/drivers/media/platform/sunxi/sun8i-a83t-mipi-csi2/sun8i_a83t_mipi_csi2.h
@@ -33,8 +33,6 @@ struct sun8i_a83t_mipi_csi2_bridge {
struct media_pad pads[SUN8I_A83T_MIPI_CSI2_PAD_COUNT];
struct v4l2_fwnode_endpoint endpoint;
struct v4l2_async_notifier notifier;
- struct v4l2_mbus_framefmt mbus_format;
- struct mutex lock; /* Mbus format lock. */
struct v4l2_subdev *source_subdev;
};
--
2.34.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH v5 3/3] media: sun6i-csi: bridge: Use V4L2 subdev active state
2026-08-04 7:26 [PATCH v1 0/3] media: sunxi: Resend active-state conversions Arash Golgol
2026-08-04 7:26 ` [PATCH v4 1/3] media: sun6i-mipi-csi2: Use V4L2 subdev active state Arash Golgol
2026-08-04 7:26 ` [PATCH v4 2/3] media: sun8i-a83t-mipi-csi2: " Arash Golgol
@ 2026-08-04 7:26 ` Arash Golgol
2 siblings, 0 replies; 4+ messages in thread
From: Arash Golgol @ 2026-08-04 7:26 UTC (permalink / raw)
To: linux-media, linux-arm-kernel, linux-sunxi
Cc: yong.deng, paulk, laurent.pinchart, mchehab, wens, jernej.skrabec,
samuel, 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>
Reviewed-by: Paul Kocialkowski <paulk@sys-base.io>
Tested-by: Paul Kocialkowski <paulk@sys-base.io>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
No changes in v4 and v5, just resend
- Link to v5: https://patchwork.linuxtv.org/project/linux-media/patch/20260719112714.72802-2-arash.golgol@gmail.com/
- Link to v5: https://lore.kernel.org/linux-media/20260719112714.72802-2-arash.golgol@gmail.com/
- Link to v3: https://patchwork.linuxtv.org/project/linux-media/patch/20260509050921.22158-2-arash.golgol@gmail.com/
Changes in v3:
- Fix Media CI robot warnings about open parenthesis
- Link to report: https://linux-media.pages.freedesktop.org/-/users/patchwork/-/jobs/99380724/artifacts/report.htm
- Link to v2: https://patchwork.kernel.org/project/linux-media/patch/20260508161721.94285-2-arash.golgol@gmail.com/
Changes in v2:
- Fix indentation in link validation path
- link to v1: https://patchwork.kernel.org/project/linux-media/patch/20260217064050.18388-2-arash.golgol@gmail.com/
.../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.34.1
^ permalink raw reply related [flat|nested] 4+ messages in thread