From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id BDB2CC11F68 for ; Tue, 13 Jul 2021 09:42:23 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id A5D3461152 for ; Tue, 13 Jul 2021 09:42:23 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S235322AbhGMJpM (ORCPT ); Tue, 13 Jul 2021 05:45:12 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:34944 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S235311AbhGMJpK (ORCPT ); Tue, 13 Jul 2021 05:45:10 -0400 Received: from mail-lf1-x12c.google.com (mail-lf1-x12c.google.com [IPv6:2a00:1450:4864:20::12c]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 69AB6C0613DD for ; Tue, 13 Jul 2021 02:42:20 -0700 (PDT) Received: by mail-lf1-x12c.google.com with SMTP id n14so49177623lfu.8 for ; Tue, 13 Jul 2021 02:42:20 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ragnatech-se.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to; bh=S/4UHmbEjPWzGrsdhXiNmY0edAGnL7qyMx4WNkWK7gA=; b=YHv1s+lBSQwJExSVYUiO5xYWvJsvmOVY4nUYnPZ7lXW5OllcjqcHFA5BqGMGzzlCoD UYK7+N4S0sGqz2RtN0lRHsjy9jyaHV/SLhi/xHAXVGu+5dzbi9HYm+diurjx3H5RMAIh mB4Q+/plyEFQFayjrKM6jiGSEI3nct7cQPfS+AE/o/FODl8KqQzlyGMorTns0hkWjta7 ytgryitzTPhsLZ5ISx0IsssWOrv+5OW9T9Wj3SNC+dNHLAg+UZgkVVINSHg3QF97mBLL v5rtIDKQIxfZsBMyYdopAo9BZd2YTtImYrZgrCgPX3ZlbmWzQgFWub8E6JQH8Tb9n8/G PvbA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to; bh=S/4UHmbEjPWzGrsdhXiNmY0edAGnL7qyMx4WNkWK7gA=; b=dwfKplHxq6Ty1cqm4LAzaHYvn9obGFMlexoenZ2klrQEUCDsae5XD6KZGPkeIuvwFW WzrMvwLKpSAzGkuaQh/npjJKOXKgG7DfK0S+tPVlvgnjUajdOYRjE0yGTeC24Wui914F RYAW3FhLKkNg5xDwWgLo0pyHOFNZVtnrLulmmAUy3h7fBaMk8Tn423pCYg+/83N1YGo4 ViuhfP69BLjIxh6DoFzn7jxR1Z0FytynA1Sh/DMpHisZRvEL3DaxX0bcC4qe2UXhwUKE 57p9WvyfUxumPXY5nbD9ToUTXMLSdlfjfFtlNLtmQ6DbvNgbaRQSilWqELqZ3bZGGJB0 vjUQ== X-Gm-Message-State: AOAM5326vVDBfHvJnWPSlZ0dDJTsG8E8SrKfTxjqcOt1uCV/CIcdWhHy 74sk0Bcu3sSnGV6/Q309pSfFaQ== X-Google-Smtp-Source: ABdhPJyK9ndywXiDK63yOL2qMNJerVSS3hwDtdMZfd5UuuN6A/qjH7WKjBKaGNeAEzHte4WIg3NQ6A== X-Received: by 2002:ac2:48a3:: with SMTP id u3mr886842lfg.151.1626169338798; Tue, 13 Jul 2021 02:42:18 -0700 (PDT) Received: from localhost (h-46-59-88-219.A463.priv.bahnhof.se. [46.59.88.219]) by smtp.gmail.com with ESMTPSA id s13sm1840013ljp.8.2021.07.13.02.42.17 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 13 Jul 2021 02:42:18 -0700 (PDT) Date: Tue, 13 Jul 2021 11:42:17 +0200 From: Niklas =?iso-8859-1?Q?S=F6derlund?= To: Laurent Pinchart Cc: Dennis Rachui , Steve Longerbeam , Mauro Carvalho Chehab , Maxime Ripard , Sakari Ailus , linux-media@vger.kernel.org, linux-renesas-soc@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] media: rcar-csi2: do not update format while streaming Message-ID: References: <1625750578-108454-1-git-send-email-drachui@de.adit-jv.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Laurent, On 2021-07-11 01:42:44 +0300, Laurent Pinchart wrote: > Hi Niklas, > > On Fri, Jul 09, 2021 at 04:20:40PM +0200, Niklas Söderlund wrote: > > 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? > > Some drivers may support format changes during streaming (that's allowed > by the V4L2 API, I'm not sure if it's used anywhere though). While I'd > favour not duplicating the same logic in different (and differently > buggy) ways in drivers, I'm not sure how this could be implemented in a > sane way in the V4L2 core in its current state. I understand it's possible from some devices to support to format changes during streaming, but as you point out it's the exception and not the rule, if used at all. So my point is if we start to enforce this in drivers we are headed down a road where this will be messier to clean up. Would it not make more sens to default the V4L2 core to disallow format changes while streaming and add a new flag to V4L2_SUBDEV_CAP_ to signal that the subdevice supports format changes while streaming? We already have V4L2_SUBDEV_CAP_RO_SUBDEV to signal that a subdevice only supports read-only operations so I think it would not be too hard to move this functionality into the core? > > > > 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 > > > Signed-off-by: Dennis Rachui > > > --- > > > 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, > > -- > Regards, > > Laurent Pinchart -- Regards, Niklas Söderlund