Hi Dennis,
Thanks for your patch.
On 2021-07-08 15:22:58 +0200, Dennis Rachui wrote:
> Verify that streaming is not active before setting the pad format.
>
> According to the VIDIOC documentation [1] changes to the active
> format of a media pad via the VIDIOC_SUBDEV_S_FMT ioctl are
> applied to the underlying hardware.
> In rcar-csi2 a format change only applies to hardware, when the
> pipeline is started. While the device is not in use, it is therefore
> okay to update the format.
>
> However, when the pipeline is active, this leads to a format
> mismatch between driver and device.
> Other applications can query the format with
> VIDIOC_SUBDEV_G_FMT at any time and would be reported
> a format that does not fit the current stream.
>
> This commit prevents format update while streaming is active
> and returns -EBUSY to user space, as suggested by [1].
>
> [1] Documentation/userspace-api/media/v4l/vidioc-subdev-g-fmt.rst
I like that this is addressed, but I wonder is this not something that
should be fixed in the V4L2 core and not in drivers?
>
> Note: after creation of this commit, it was noticed that Steve
> Longerbeam has a very similar solution in his fork.
>
> Fixes: 769afd212b16 ("media: rcar-csi2: add Renesas R-Car MIPI CSI-2 receiver driver")
> Cc: Steve Longerbeam <slongerbeam@xxxxxxxxx>
> Signed-off-by: Dennis Rachui <drachui@xxxxxxxxxxxxxx>
> ---
> drivers/media/platform/rcar-vin/rcar-csi2.c | 21 ++++++++++++++++++++-
> 1 file changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c b/drivers/media/platform/rcar-vin/rcar-csi2.c
> index e28eff0..98152e1 100644
> --- a/drivers/media/platform/rcar-vin/rcar-csi2.c
> +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
> @@ -724,18 +724,37 @@ static int rcsi2_set_pad_format(struct v4l2_subdev *sd,
> {
> struct rcar_csi2 *priv = sd_to_csi2(sd);
> struct v4l2_mbus_framefmt *framefmt;
> + int ret = 0;
> +
> + mutex_lock(&priv->lock);
>
> if (!rcsi2_code_to_fmt(format->format.code))
> format->format.code = rcar_csi2_formats[0].code;
>
> if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE) {
> +
> + /*
> + * Do not apply changes to active format while streaming.
> + *
> + * Since video streams could be forwarded from sink pad to any
> + * source pad (depending on CSI-2 channel routing), all
> + * media pads are effected by this rule.
> + */
> + if (priv->stream_count > 0) {
> + ret = -EBUSY;
> + goto out;
> + }
> +
> priv->mf = format->format;
> } else {
> framefmt = v4l2_subdev_get_try_format(sd, sd_state, 0);
> *framefmt = format->format;
> }
>
> - return 0;
> +out:
> + mutex_unlock(&priv->lock);
> +
> + return ret;
> }
>
> static int rcsi2_get_pad_format(struct v4l2_subdev *sd,
> --
> 2.7.4
>
--
Regards,
Niklas Söderlund
