From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 662DF20E31C for ; Fri, 18 Oct 2024 04:14:29 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1729224869; cv=none; b=FGyhfli3CJ61294g4M0TIRlTaZ/Jsd6UHmUSGj/460lHQPduHodMQ9xM1QgJrRga9n0MJAZlZdhHOz61m/Wiaz3KPUhS2Pt6aAj77Vy35/Y2SR4BEDg+/Cc3rPBS9Egu2l/zK2nPc3HhUH7cr6AxDjxBf8Bp1Zlyz21z/AQrmVg= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1729224869; c=relaxed/simple; bh=Jl6DjAAGFcrwb6awcc4A5zUZF2UJ7AtYBzoQg+9Q6lI=; h=Date:From:To:Cc:Subject:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=HR0Kof61a/Zy6oagRlONjHcTP/OOBYmjjztjT98IvgMvJPGIODWkc9kF7czxygCdnwtHBxLLi2tKjB9F0IpbzopCvBsf/RdQT/ie3C48wzHRPCFF+3W96cPtcqpMGDlrfF4iQV3FtWSl10t8crXHz4o5EaaqpQiYGKYDZLb2gkk= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=em4fbnJH; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="em4fbnJH" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 8CC84C4CEC3; Fri, 18 Oct 2024 04:14:08 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1729224868; bh=Jl6DjAAGFcrwb6awcc4A5zUZF2UJ7AtYBzoQg+9Q6lI=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=em4fbnJHUErUZk7r+JVypHFpmM/vGNmqN/uA13nU1QGTYSgb2a2dnxvmHlRCTst3/ 9pC/qvFqdCIjBv05mQsqgfZMzrjjf1UkmXCWlDmSJNc2/odX9QLJ+3V4/ZiNyF6cAW VUOY2XVgwOnhjQem6shf+TyingO+eRQuc2ZBhcl4gq4eoognvp75zPMOBKDw2PfMTX SPDto9dLzJLuf/FNtGzZYcrQ8kji/zKBuZKwxhTZQWvUnJneF/TTfsm03gVd/rnNDo MwXKL4xNDsIpPPI4/4Lq/0aQ+CfkNC4dB1Ad5i8s7Vr6FHrtUQqxQSXWzrhagAUOTv MArAhOZVSNRIQ== Date: Fri, 18 Oct 2024 06:14:03 +0200 From: Mauro Carvalho Chehab To: Hans Verkuil Cc: Tomasz Figa , Marek Szyprowski , Mauro Carvalho Chehab , Shuah Khan , Kieran Bingham , Daniel Almeida , Andy Walls , Yong Zhi , Sakari Ailus , Bingbu Cao , Dan Scally , Tianshu Qiu , Martin Tuma , Bluecherry Maintainers , Andrey Utkin , Ismael Luceno , Ezequiel Garcia , Corentin Labbe , Michael Krufky , Laurent Pinchart , Matt Ranostay , Michael Tretter , Pengutronix Kernel Team , Neil Armstrong , Kevin Hilman , Jerome Brunet , Martin Blumenstingl , Ming Qian , Zhou Peng , Eddie James , Joel Stanley , Andrew Jeffery , Eugen Hristev , Nicolas Ferre , Alexandre Belloni , Claudiu Beznea , Raspberry Pi Kernel Maintenance , Florian Fainelli , Broadcom internal kernel review list , Ray Jui , Scott Branden , Philipp Zabel , Nas Chung , Jackson Lee , Devarsh Thakkar , Bin Liu , Matthias Brugger , AngeloGioacchino Del Regno , Minghsiu Tsai , Houlong Wei , Andrew-CT Chen , Tiffany Lin , Yunfei Dong , Joseph Liu , Marvin Lin , Dmitry Osipenko , Thierry Reding , Jonathan Hunter , Xavier Roumegue , Mirela Rabulea , Shawn Guo , Sascha Hauer , Fabio Estevam , Rui Miguel Silva , Martin Kepplinger , Purism Kernel Team , Robert Foss , Todor Tomov , Bryan O'Donoghue , Stanimir Varbanov , Vikash Garodia , Jacopo Mondi , Niklas =?UTF-8?B?U8O2ZGVybHVuZA==?= , Fabrizio Castro , Kieran Bingham , Mikhail Ulyanov , Jacob Chen , Heiko Stuebner , Dafna Hirschfeld , Krzysztof Kozlowski , Alim Akhtar , Sylwester Nawrocki , =?UTF-8?B?xYF1a2Fzeg==?= Stelmach , Andrzej Pietrasiewicz , Jacek Anaszewski , Andrzej Hajda , Fabien Dessenne , Hugues Fruchet , Jean-Christophe Trotin , Maxime Coquelin , Alexandre Torgue , Alain Volmat , Maxime Ripard , Chen-Yu Tsai , Jernej Skrabec , Samuel Holland , Yong Deng , Paul Kocialkowski , Benoit Parrot , Jai Luthra , Michal Simek , Andy Shevchenko , Hans de Goede , Greg Kroah-Hartman , Steve Longerbeam , Jack Zhu , Changhuang Liang , Sowjanya Komatineni , Luca Ceresoli , linux-media@vger.kernel.org Subject: Re: [PATCHv2 01/10] media: videobuf2-core: update vb2_thread if wait_finish/prepare are NULL Message-ID: <20241018061403.318ce951@foz.lan> In-Reply-To: <8ec79f05-b4a4-4a60-b10f-9ce2dd55bde2@xs4all.nl> References: <20241014-vb2-wait-v1-0-8c3ee25c618c@xs4all.nl> <20241014-vb2-wait-v1-1-8c3ee25c618c@xs4all.nl> <8ec79f05-b4a4-4a60-b10f-9ce2dd55bde2@xs4all.nl> X-Mailer: Claws Mail 4.3.0 (GTK 3.24.43; x86_64-redhat-linux-gnu) Precedence: bulk X-Mailing-List: linux-media@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Em Thu, 17 Oct 2024 17:09:23 +0200 Hans Verkuil escreveu: > The vb2_thread is used for DVB support. This will queue and dequeue buffers > automatically. > > It calls wait_finish/prepare around vb2_core_dqbuf() and vb2_core_qbuf(), > but that assumes all drivers have these ops set. But that will change > due to commit 88785982a19d ("media: vb2: use lock if wait_prepare/finish > are NULL"). > > So instead just check if the callback is available, and if not, use > q->lock, just as __vb2_wait_for_done_vb() does. > > Signed-off-by: Hans Verkuil LGTM. Reviewed-by: Mauro Carvalho Chehab > --- > I'm just updating this patch, not the others in this series. > > Changes since v1: > - move the locking code inside the 'if (!threadio->stop)' > - do the same for vb2_core_qbuf() > --- > .../media/common/videobuf2/videobuf2-core.c | 26 ++++++++++++++----- > 1 file changed, 20 insertions(+), 6 deletions(-) > > diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c > index d064e0664851..d2275c878ea9 100644 > --- a/drivers/media/common/videobuf2/videobuf2-core.c > +++ b/drivers/media/common/videobuf2/videobuf2-core.c > @@ -3218,10 +3218,17 @@ static int vb2_thread(void *data) > continue; > prequeue--; > } else { > - call_void_qop(q, wait_finish, q); > - if (!threadio->stop) > + if (!threadio->stop) { > + if (q->ops->wait_finish) > + call_void_qop(q, wait_finish, q); > + else if (q->lock) > + mutex_lock(q->lock); > ret = vb2_core_dqbuf(q, &index, NULL, 0); > - call_void_qop(q, wait_prepare, q); > + if (q->ops->wait_prepare) > + call_void_qop(q, wait_prepare, q); > + else if (q->lock) > + mutex_unlock(q->lock); > + } > dprintk(q, 5, "file io: vb2_dqbuf result: %d\n", ret); > if (!ret) > vb = vb2_get_buffer(q, index); > @@ -3233,12 +3240,19 @@ static int vb2_thread(void *data) > if (vb->state != VB2_BUF_STATE_ERROR) > if (threadio->fnc(vb, threadio->priv)) > break; > - call_void_qop(q, wait_finish, q); > if (copy_timestamp) > vb->timestamp = ktime_get_ns(); > - if (!threadio->stop) > + if (!threadio->stop) { > + if (q->ops->wait_finish) > + call_void_qop(q, wait_finish, q); > + else if (q->lock) > + mutex_lock(q->lock); > ret = vb2_core_qbuf(q, vb, NULL, NULL); > - call_void_qop(q, wait_prepare, q); > + if (q->ops->wait_prepare) > + call_void_qop(q, wait_prepare, q); > + else if (q->lock) > + mutex_unlock(q->lock); > + } > if (ret || threadio->stop) > break; > } Thanks, Mauro