[PATCH] media: sun6i-mipi-csi2: Propagate format to source pad in TRY mode

[PATCH] media: sun6i-mipi-csi2: Propagate format to source pad in TRY mode

[Arash Golgol]

sun6i-mipi-csi2 does not support any format conversion. So the format
on sink and source pad must always match. This limitation is handled for
ACTIVE state via mbus_format member of bridge private structure.

To enforce this limitation in TRY state, user space should only be able
to set format on sink pad. The sink format must be propagated to source
pad to ensure consistent behavior.

This also aligns the driver with userspace relying on media controller
based format negotiation.

Signed-off-by: Arash Golgol <arash.golgol@gmail.com>
---
 .../sunxi/sun6i-mipi-csi2/sun6i_mipi_csi2.c   | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 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..6ee0f6685663 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
@@ -369,15 +369,26 @@ static int sun6i_mipi_csi2_set_fmt(struct v4l2_subdev *subdev,
 	struct v4l2_mbus_framefmt *mbus_format = &format->format;
 	struct mutex *lock = &csi2_dev->bridge.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);
+
 	mutex_lock(lock);
 
 	sun6i_mipi_csi2_mbus_format_prepare(mbus_format);
 
-	if (format->which == V4L2_SUBDEV_FORMAT_TRY)
-		*v4l2_subdev_state_get_format(state, format->pad) =
-			*mbus_format;
-	else
+	if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
+		struct v4l2_mbus_framefmt *fmt;
+
+		fmt = v4l2_subdev_state_get_format(state, format->pad);
+		*fmt = *mbus_format;
+
+		/* Propagate the format to the source pad. */
+		fmt = v4l2_subdev_state_get_format(state, SUN6I_MIPI_CSI2_PAD_SOURCE);
+		*fmt = *mbus_format;
+	} else {
 		csi2_dev->bridge.mbus_format = *mbus_format;
+	}
 
 	mutex_unlock(lock);
 
-- 
2.34.1

Re: [PATCH] media: sun6i-mipi-csi2: Propagate format to source pad in TRY mode

[Laurent Pinchart]

Hello Arash,

Thank you for the patch.

On Mon, Jan 26, 2026 at 01:51:49PM +0330, Arash Golgol wrote:
> sun6i-mipi-csi2 does not support any format conversion. So the format
> on sink and source pad must always match. This limitation is handled for
> ACTIVE state via mbus_format member of bridge private structure.

It's not even handled correctly though, the sun6i_mipi_csi2_set_fmt()
function will accept setting the ACTIVE format on the source pad, when
it shouldn't. You also fix this in your patch. This should be explained
in the commit message.

> To enforce this limitation in TRY state, user space should only be able
> to set format on sink pad. The sink format must be propagated to source
> pad to ensure consistent behavior.
> 
> This also aligns the driver with userspace relying on media controller
> based format negotiation.
> 
> Signed-off-by: Arash Golgol <arash.golgol@gmail.com>
> ---
>  .../sunxi/sun6i-mipi-csi2/sun6i_mipi_csi2.c   | 19 +++++++++++++++----
>  1 file changed, 15 insertions(+), 4 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..6ee0f6685663 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
> @@ -369,15 +369,26 @@ static int sun6i_mipi_csi2_set_fmt(struct v4l2_subdev *subdev,
>  	struct v4l2_mbus_framefmt *mbus_format = &format->format;
>  	struct mutex *lock = &csi2_dev->bridge.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);
> +
>  	mutex_lock(lock);
>  
>  	sun6i_mipi_csi2_mbus_format_prepare(mbus_format);
>  
> -	if (format->which == V4L2_SUBDEV_FORMAT_TRY)
> -		*v4l2_subdev_state_get_format(state, format->pad) =
> -			*mbus_format;
> -	else
> +	if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
> +		struct v4l2_mbus_framefmt *fmt;
> +
> +		fmt = v4l2_subdev_state_get_format(state, format->pad);
> +		*fmt = *mbus_format;
> +
> +		/* Propagate the format to the source pad. */
> +		fmt = v4l2_subdev_state_get_format(state, SUN6I_MIPI_CSI2_PAD_SOURCE);
> +		*fmt = *mbus_format;
> +	} else {
>  		csi2_dev->bridge.mbus_format = *mbus_format;
> +	}

If you have a bit more time, could I ask you to take it one step further
and convert the driver to framework-managed V4L2 subdev active ? This
will simplify the implementation by dropping bridge.mbus_format.

You can find an example of the conversion in commit a2514b9a634a
("media: i2c: imx290: Use V4L2 subdev active state"). In a nutshell, you
need to

- Call v4l2_subdev_init_finalize() at probe time and
  v4l2_subdev_cleanup() at remove time
- Drop the mutex (locking is now handled by the framework)
- Use v4l2_subdev_get_fmt() as the .get_fmt() handler
- Drop bridge.mbus_format and always use v4l2_subdev_get_pad_format() on
  the state passed to the subdev functions (which will be the ACTIVE or
  TRY state depending on the .which argument passed by userspace)
- In .s_stream(), access the state with
  v4l2_subdev_lock_and_get_active_state() as it's the only subdev
  operation that doesn't get the state as a parameter

You can ignore the control handler changes in commit a2514b9a634a, and
you can leave subdev.state_lock NULL (the framework will then use an
internal lock).

>  
>  	mutex_unlock(lock);
>  

-- 
Regards,

Laurent Pinchart

Re: [PATCH] media: sun6i-mipi-csi2: Propagate format to source pad in TRY mode

[arash golgol]

Hi Laurent,

Thanks for the review and the detailed feedback.

On Wed, Feb 4, 2026 at 3:32 AM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hello Arash,
>
> Thank you for the patch.
>
> On Mon, Jan 26, 2026 at 01:51:49PM +0330, Arash Golgol wrote:
> > sun6i-mipi-csi2 does not support any format conversion. So the format
> > on sink and source pad must always match. This limitation is handled for
> > ACTIVE state via mbus_format member of bridge private structure.
>
> It's not even handled correctly though, the sun6i_mipi_csi2_set_fmt()
> function will accept setting the ACTIVE format on the source pad, when
> it shouldn't. You also fix this in your patch. This should be explained
> in the commit message.
>

Yes, you're right.

> > To enforce this limitation in TRY state, user space should only be able
> > to set format on sink pad. The sink format must be propagated to source
> > pad to ensure consistent behavior.
> >
> > This also aligns the driver with userspace relying on media controller
> > based format negotiation.
> >
> > Signed-off-by: Arash Golgol <arash.golgol@gmail.com>
> > ---
> >  .../sunxi/sun6i-mipi-csi2/sun6i_mipi_csi2.c   | 19 +++++++++++++++----
> >  1 file changed, 15 insertions(+), 4 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..6ee0f6685663 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
> > @@ -369,15 +369,26 @@ static int sun6i_mipi_csi2_set_fmt(struct v4l2_subdev *subdev,
> >       struct v4l2_mbus_framefmt *mbus_format = &format->format;
> >       struct mutex *lock = &csi2_dev->bridge.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);
> > +
> >       mutex_lock(lock);
> >
> >       sun6i_mipi_csi2_mbus_format_prepare(mbus_format);
> >
> > -     if (format->which == V4L2_SUBDEV_FORMAT_TRY)
> > -             *v4l2_subdev_state_get_format(state, format->pad) =
> > -                     *mbus_format;
> > -     else
> > +     if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
> > +             struct v4l2_mbus_framefmt *fmt;
> > +
> > +             fmt = v4l2_subdev_state_get_format(state, format->pad);
> > +             *fmt = *mbus_format;
> > +
> > +             /* Propagate the format to the source pad. */
> > +             fmt = v4l2_subdev_state_get_format(state, SUN6I_MIPI_CSI2_PAD_SOURCE);
> > +             *fmt = *mbus_format;
> > +     } else {
> >               csi2_dev->bridge.mbus_format = *mbus_format;
> > +     }
>
> If you have a bit more time, could I ask you to take it one step further
> and convert the driver to framework-managed V4L2 subdev active ? This
> will simplify the implementation by dropping bridge.mbus_format.
>

Sure, I'll do that and submit a new patch converting the driver
to framework-managed V4L2 subdev.

> You can find an example of the conversion in commit a2514b9a634a
> ("media: i2c: imx290: Use V4L2 subdev active state"). In a nutshell, you
> need to
>
> - Call v4l2_subdev_init_finalize() at probe time and
>   v4l2_subdev_cleanup() at remove time
> - Drop the mutex (locking is now handled by the framework)
> - Use v4l2_subdev_get_fmt() as the .get_fmt() handler
> - Drop bridge.mbus_format and always use v4l2_subdev_get_pad_format() on
>   the state passed to the subdev functions (which will be the ACTIVE or
>   TRY state depending on the .which argument passed by userspace)
> - In .s_stream(), access the state with
>   v4l2_subdev_lock_and_get_active_state() as it's the only subdev
>   operation that doesn't get the state as a parameter
>
> You can ignore the control handler changes in commit a2514b9a634a, and
> you can leave subdev.state_lock NULL (the framework will then use an
> internal lock).
>

Thanks a lot for the example and roadmap of the changes.

> >
> >       mutex_unlock(lock);
> >
>
> --
> Regards,
>
> Laurent Pinchart

--
Regards,

Arash Golgol