Received: by 2002:a6b:500f:0:0:0:0:0 with SMTP id e15csp237228iob; Thu, 28 Apr 2022 00:54:24 -0700 (PDT) X-Google-Smtp-Source: ABdhPJzt6LIG8IV88Dg32CENoLR5vv0hgBIREtD0uDIAWe9aGi2Qbp4aizudpKDg2ODeRm5WThCV X-Received: by 2002:a17:907:62a6:b0:6ef:8118:d3e2 with SMTP id nd38-20020a17090762a600b006ef8118d3e2mr29886946ejc.605.1651132464598; Thu, 28 Apr 2022 00:54:24 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1651132464; cv=none; d=google.com; s=arc-20160816; b=YVoN+ksYBoX4z4cbFgPM5MK17WnqzNYhAul+fGrOAOSiYTHd7RI5hMxrYV7xiHAbyM pgSEXlJ66VcXGITlzsexk94nQ84eO/7BzB0RWT8xiFK03Jg8IBnIsUD2wzy2pk5rTbPN gGA5MWXWmoQIRlSevkXcVkPgYP3fnQ9ZdX/YstyRv1S6QPxCh25/wKGN/ksFeMKyTCXQ aQ1WdTN6w4Tl/955KQYUx+PMOGkJOePJ/aRkCN1hWTxN9Us3ODVzNaoZOKlJM8+2GxX6 Gc6gD1fTOqnSP8YQ/NokPEwlXwtaWayKTkN5tL9zTeo7UiQdWjzHG86c+lOL8ItrUtkg iCUw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:in-reply-to:from :references:cc:to:content-language:subject:user-agent:mime-version :date:message-id; bh=iwccNq6llGK2vratRI/20Rb2MKJSTp0WPLGzvUONmZU=; b=bC4FtnCOZL4AC21vRIlxK/rknaR6QKCjnPZa8L7gkVC4BMOhITmO6rKo1215LdLUpV UdF0gYiypUb+oTHkqphNnxnxUjPX10fk7CQfJQ/banTF2c71OL4KnNSL8T9SZvi/aGNP XAZ/r7GIy4DlGhR2Z0Gw4BLYhOhW9FoVbthcq5nWWVLVmz+4ijHq1W6eNkPTV6sTThh/ 0e/SBd3gdn0pyf6toQ1y+ZRLH8SLvoOhcN/zqM0ZxxWgBOQjiudupSQhlEcTtfl0d/c8 iREPUNVW9m4c4mGflrGXrwgt6ex9z7Xfacb81N5yeMHzn8l3HnMG7Kv9T7WhrRIDD7yB zYyQ== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=xs4all.nl Return-Path: Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id s13-20020a1709067b8d00b006e8925846cesi3263480ejo.471.2022.04.28.00.54.00; Thu, 28 Apr 2022 00:54:24 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=xs4all.nl Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S243478AbiD1GP6 (ORCPT + 99 others); Thu, 28 Apr 2022 02:15:58 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:34574 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233488AbiD1GP6 (ORCPT ); Thu, 28 Apr 2022 02:15:58 -0400 Received: from ams.source.kernel.org (ams.source.kernel.org [145.40.68.75]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 506FD20F7F; Wed, 27 Apr 2022 23:12:44 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id 04ABFB82B2B; Thu, 28 Apr 2022 06:12:43 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 0A50BC385A9; Thu, 28 Apr 2022 06:12:39 +0000 (UTC) Message-ID: <4b7a3d71-629b-56d6-fdc7-d07682390fd2@xs4all.nl> Date: Thu, 28 Apr 2022 08:12:37 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.7.0 Subject: Re: [PATCH v4 03/24] media: videobuf2-v4l2: Warn on holding buffers without support Content-Language: en-US To: Nicolas Dufresne , Tomasz Figa , Marek Szyprowski , Mauro Carvalho Chehab Cc: Sebastian Fricke , linux-media@vger.kernel.org, Ezequiel Garcia , linux-kernel@vger.kernel.org References: <20220426125751.108293-1-nicolas.dufresne@collabora.com> <20220426125751.108293-4-nicolas.dufresne@collabora.com> From: Hans Verkuil In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-8.5 required=5.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,NICE_REPLY_A,RCVD_IN_DNSWL_HI, SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 27/04/2022 17:08, Nicolas Dufresne wrote: > Le mercredi 27 avril 2022 à 13:31 +0900, Tomasz Figa a écrit : >> Hi Nicolas, Sebastian, >> >> On Tue, Apr 26, 2022 at 9:58 PM Nicolas Dufresne >> wrote: >>> >>> From: Sebastian Fricke >>> >>> Using V4L2_BUF_FLAG_M2M_HOLD_CAPTURE_BUF flag without specifying the >>> subsystem flag VB2_V4L2_FL_SUPPORTS_M2M_HOLD_CAPTURE_BUF, results in >>> silently ignoring it. >>> Warn the user via a debug print when the flag is requested but ignored >>> by the videobuf2 framework. >>> >>> Signed-off-by: Sebastian Fricke >>> Signed-off-by: Nicolas Dufresne >>> Reviewed-by: Ezequiel Garcia >>> --- >>> drivers/media/common/videobuf2/videobuf2-v4l2.c | 7 ++++++- >>> 1 file changed, 6 insertions(+), 1 deletion(-) >>> >> >> Thanks for the patch. Please see my comments inline. >> >>> diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c >>> index 6edf4508c636..812c8d1962e0 100644 >>> --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c >>> +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c >>> @@ -329,8 +329,13 @@ static int vb2_fill_vb2_v4l2_buffer(struct vb2_buffer *vb, struct v4l2_buffer *b >>> */ >>> vbuf->flags &= ~V4L2_BUF_FLAG_TIMECODE; >>> vbuf->field = b->field; >>> - if (!(q->subsystem_flags & VB2_V4L2_FL_SUPPORTS_M2M_HOLD_CAPTURE_BUF)) >>> + if (!(q->subsystem_flags & VB2_V4L2_FL_SUPPORTS_M2M_HOLD_CAPTURE_BUF)) { >>> + if (vbuf->flags & V4L2_BUF_FLAG_M2M_HOLD_CAPTURE_BUF) >>> + dprintk(q, 1, >>> + "Request holding buffer (%d), unsupported on output queue\n", >>> + b->index); >> >> I wonder if we shouldn't just fail such a QBUF operation. Otherwise >> the application would get unexpected behavior from the kernel. >> Although it might be too late to do it now if there are applications >> that rely on this implicit ignore... > > In the context of this patchset, the statu quo seems to be the logical thing to > do. We can raise this up in a separate thread. The side effect is of course > confusing for developers, but it is hard for me to tell if a hard failure may > break an existing software. I am leaning towards returning an error as well. It makes no sense to try to hold on to a buffer when this is not supported. I also thought that it should be enough to rely on the core to clear the flag upon return if it isn't supported, but looking through the vb2 core code it looks like we're not clearing unknown flags at all, so running this for older kernels that do not support holding at all will not clear the flag either. The handling for flags in vb2 can be improved, I think I'll take a look at that myself. I plan to merge this series soon, but will skip this patch for now. Regards, Hans > > regards, > Nicolas > >> >> Best regards, >> Tomasz >