Hi Arash, On Wed 22 Apr 26, 08:23, arash golgol wrote: > I was hoping this patch would make its way into media-tree, but it seems > it hasn't been applied yet. > > Is there anything missing or any modifications needed from my side? > I'd appreciate any feedback. I would like to test this patch before it gets in, but didn't manage to find the time. The same should probably also apply to the a83t mipi csi-2 driver, which is structured the same way. Thanks for your work, Paul > Regards, > Arash Golgol > > On Sat, Feb 14, 2026 at 8:40 AM 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. > > > > 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 > > Reviewed-by: Laurent Pinchart > > --- > > 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..682bdd82098c 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 > > -- 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.