| Message ID | 20170616073915.5027-9-gustavo@padovan.org (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers |
Received: from mail.tu-berlin.de ([130.149.7.33]) by www.linuxtv.org with esmtp (Exim 4.84_2) (envelope-from <linux-media-owner@vger.kernel.org>) id 1dLlrE-0005oD-AC; Fri, 16 Jun 2017 07:39:52 +0000 X-tubIT-Incoming-IP: 209.132.180.67 Received: from vger.kernel.org ([209.132.180.67]) by mail.tu-berlin.de (exim-4.89/mailfrontend-8) with esmtp id 1dLlrC-0004vQ-kd; Fri, 16 Jun 2017 09:39:52 +0200 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752692AbdFPHjp (ORCPT <rfc822;mkrufky@linuxtv.org> + 1 other); Fri, 16 Jun 2017 03:39:45 -0400 Received: from mail-pf0-f194.google.com ([209.85.192.194]:35390 "EHLO mail-pf0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752684AbdFPHjo (ORCPT <rfc822;linux-media@vger.kernel.org>); Fri, 16 Jun 2017 03:39:44 -0400 Received: by mail-pf0-f194.google.com with SMTP id s66so4709651pfs.2 for <linux-media@vger.kernel.org>; Fri, 16 Jun 2017 00:39:43 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=Y38R3aX5aE4CD8znTGxX/vSrALy4escJyIBBedVjZOU=; b=HKhxdQHTgNxHcg4qNXZ0ZbH7hOMN+a/eSzCKmTOR5VxQE45LA9UpTK72nr7a6bCcJ4 EyPE+KiD4jwoLYRZaAwzsa/VYtZoS29A99VcnaHZR7nQb8v3vNF0LLHxwHFeSr8FvM8w JMN7URhuK+FRFAcz9MkphSVzcAWpdnfacte11WrUMKxMAXwKKog8BaUQYvvmsv4TiJ0K OfPnKL2vJpcruf5Ica8r5qFX/iY/plh0OHJNx3m1klr8NceJ6AJyKLfwYl+VWYejrsw9 NrDDyHRX1KieXaGY6J2QcyTDdHcPYrMf8Jv0hmWRUzQa424IrjvnMKKdrz2SItP2X5KN ZgKQ== X-Gm-Message-State: AKS2vOzfJc0Vj/A4leqP4CYwKi/KxSoPi+uMzxTYEeX12d2EgiW0imqP uNBWU9rDhm1Ak6L4tRc= X-Received: by 10.84.233.129 with SMTP id l1mr11034251plk.169.1497598783163; Fri, 16 Jun 2017 00:39:43 -0700 (PDT) Received: from jade.nodan1.kt.home.ne.jp (202-72-64-244.koalanet.ne.jp. [202.72.64.244]) by smtp.gmail.com with ESMTPSA id a2sm2760568pfj.8.2017.06.16.00.39.41 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 16 Jun 2017 00:39:42 -0700 (PDT) From: Gustavo Padovan <gustavo@padovan.org> To: linux-media@vger.kernel.org Cc: Hans Verkuil <hverkuil@xs4all.nl>, Javier Martinez Canillas <javier@osg.samsung.com>, Mauro Carvalho Chehab <mchehab@osg.samsung.com>, Shuah Khan <shuahkh@osg.samsung.com>, Gustavo Padovan <gustavo.padovan@collabora.com> Subject: [PATCH 08/12] [media] vb2: add 'ordered' property to queues Date: Fri, 16 Jun 2017 16:39:11 +0900 Message-Id: <20170616073915.5027-9-gustavo@padovan.org> X-Mailer: git-send-email 2.9.4 In-Reply-To: <20170616073915.5027-1-gustavo@padovan.org> References: <20170616073915.5027-1-gustavo@padovan.org> Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: <linux-media.vger.kernel.org> X-Mailing-List: linux-media@vger.kernel.org X-PMX-Version: 6.0.0.2142326, Antispam-Engine: 2.7.2.2107409, Antispam-Data: 2017.6.16.73017 X-PMX-Spam: Gauge=IIIIIIII, Probability=8%, Report=' MULTIPLE_RCPTS 0.1, HTML_00_01 0.05, HTML_00_10 0.05, BODYTEXTP_SIZE_3000_LESS 0, BODY_SIZE_1400_1499 0, BODY_SIZE_2000_LESS 0, BODY_SIZE_5000_LESS 0, BODY_SIZE_7000_LESS 0, IN_REP_TO 0, LEGITIMATE_SIGNS 0, MSG_THREAD 0, MULTIPLE_REAL_RCPTS 0, NO_URI_HTTPS 0, REFERENCES 0, __ANY_URI 0, __CC_NAME 0, __CC_NAME_DIFF_FROM_ACC 0, __CC_REAL_NAMES 0, __HAS_CC_HDR 0, __HAS_FROM 0, __HAS_LIST_ID 0, __HAS_MSGID 0, __HAS_X_MAILER 0, __HAS_X_MAILING_LIST 0, __IN_REP_TO 0, __MIME_TEXT_ONLY 0, __MIME_TEXT_P 0, __MIME_TEXT_P1 0, __MULTIPLE_RCPTS_CC_X2 0, __NO_HTML_TAG_RAW 0, __REFERENCES 0, __SANE_MSGID 0, __SUBJ_ALPHA_END 0, __TO_MALFORMED_2 0, __TO_NO_NAME 0, __URI_NO_WWW 0, __URI_NS , __YOUTUBE_RCVD 0' |
Commit Message
Gustavo F. Padovan
June 16, 2017, 7:39 a.m. UTC
From: Gustavo Padovan <gustavo.padovan@collabora.com> For explicit synchronization (and soon for HAL3/Request API) we need the v4l2-driver to guarantee the ordering which the buffer were queued by userspace. This is already true for many drivers, but we never had the need to say it. Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.com> --- include/media/videobuf2-core.h | 4 ++++ 1 file changed, 4 insertions(+)
Comments
Le vendredi 16 juin 2017 à 16:39 +0900, Gustavo Padovan a écrit : > > From: Gustavo Padovan <gustavo.padovan@collabora.com> > > For explicit synchronization (and soon for HAL3/Request API) we need > the v4l2-driver to guarantee the ordering which the buffer were queued > by userspace. This is already true for many drivers, but we never had > the need to say it. Phrased this way, that sound like a statement that a m2m decoder handling b-frame will just never be supported. I think decoders are a very important use case for explicit synchronization. What I believe happens with decoders is simply that the allocation order (the order in which empty buffers are retrieved from the queue) will be different then the actual presentation order. Also, multiple buffers endup being filled at the same time. Some firmware may inform of the new order at the last minute, making indeed the fence useless, but these are firmware and the information can be known earlier. Also, this information would be known by userspace for the case (up-coming, see STM patches and Rockchip comments [0]) or state-less decoder, because it is available while parsing the bitstream. For this last scenarios, the fact that ordering is not the same should disable the fences since userspace can know which fences to wait for first. Those drivers would need to set "ordered" to 0, which would be counter intuitive. I think this use case is too important to just ignore it. I would expect that we at least have a todo with something sensible as a plan to cover this. > > > Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.com> > --- > include/media/videobuf2-core.h | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h > index aa43e43..a8b800e 100644 > --- a/include/media/videobuf2-core.h > +++ b/include/media/videobuf2-core.h > @@ -491,6 +491,9 @@ struct vb2_buf_ops { > * @last_buffer_dequeued: used in poll() and DQBUF to immediately return if the > > * last decoded buffer was already dequeued. Set for capture queues > > * when a buffer with the V4L2_BUF_FLAG_LAST is dequeued. > + * @ordered: if the driver can guarantee that the queue will be ordered or not. > > + * The default is not ordered unless the driver sets this flag. It > > + * is mandatory for using explicit fences. > > * @fileio: file io emulator internal data, used only if emulator is active > > * @threadio: thread io internal data, used only if thread is active > */ > @@ -541,6 +544,7 @@ struct vb2_queue { > > > unsigned int is_output:1; > > > unsigned int copy_timestamp:1; > > > unsigned int last_buffer_dequeued:1; > > > + unsigned int ordered:1; > > > > struct vb2_fileio_data *fileio; > > > struct vb2_threadio_data *threadio;
Hi Nicolas, 2017-06-16 Nicolas Dufresne <nicolas@ndufresne.ca>: > Le vendredi 16 juin 2017 à 16:39 +0900, Gustavo Padovan a écrit : > > > From: Gustavo Padovan <gustavo.padovan@collabora.com> > > > > For explicit synchronization (and soon for HAL3/Request API) we need > > the v4l2-driver to guarantee the ordering which the buffer were queued > > by userspace. This is already true for many drivers, but we never had > > the need to say it. > > Phrased this way, that sound like a statement that a m2m decoder > handling b-frame will just never be supported. I think decoders are a > very important use case for explicit synchronization. > > What I believe happens with decoders is simply that the allocation > order (the order in which empty buffers are retrieved from the queue) > will be different then the actual presentation order. Also, multiple > buffers endup being filled at the same time. Some firmware may inform > of the new order at the last minute, making indeed the fence useless, > but these are firmware and the information can be known earlier. Also, > this information would be known by userspace for the case (up-coming, > see STM patches and Rockchip comments [0]) or state-less decoder, > because it is available while parsing the bitstream. For this last > scenarios, the fact that ordering is not the same should disable the > fences since userspace can know which fences to wait for first. Those > drivers would need to set "ordered" to 0, which would be counter > intuitive. > > I think this use case is too important to just ignore it. I would > expect that we at least have a todo with something sensible as a plan > to cover this. We definitely need to cover these usecases, I sent the patchset in a hurry just before going on vacation and forget to lay down any plan for other things. But for now, I believe we need refine the implementation of the most common case and then look at expanding it. Gustavo
On 06/16/17 09:39, Gustavo Padovan wrote: > From: Gustavo Padovan <gustavo.padovan@collabora.com> > > For explicit synchronization (and soon for HAL3/Request API) we need > the v4l2-driver to guarantee the ordering which the buffer were queued > by userspace. This is already true for many drivers, but we never had > the need to say it. > > Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.com> > --- > include/media/videobuf2-core.h | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h > index aa43e43..a8b800e 100644 > --- a/include/media/videobuf2-core.h > +++ b/include/media/videobuf2-core.h > @@ -491,6 +491,9 @@ struct vb2_buf_ops { > * @last_buffer_dequeued: used in poll() and DQBUF to immediately return if the > * last decoded buffer was already dequeued. Set for capture queues > * when a buffer with the V4L2_BUF_FLAG_LAST is dequeued. > + * @ordered: if the driver can guarantee that the queue will be ordered or not. > + * The default is not ordered unless the driver sets this flag. It > + * is mandatory for using explicit fences. This needs to be clarified both here and in the commit log. 1) what is meant with 'ordered'? I assume FIFO is meant. 2) is it the order in which buffers are queued in the driver, or the order in which buffers are queued in userspace? With in-fences it is ordered in the driver but not in userspace since the in-fence will block a buffer from being queued to the driver until the fence callback is called. 3) does this apply to in-fences or out-fences or both? It appears that this only applies to out-fences. > * @fileio: file io emulator internal data, used only if emulator is active > * @threadio: thread io internal data, used only if thread is active > */ > @@ -541,6 +544,7 @@ struct vb2_queue { > unsigned int is_output:1; > unsigned int copy_timestamp:1; > unsigned int last_buffer_dequeued:1; > + unsigned int ordered:1; > > struct vb2_fileio_data *fileio; > struct vb2_threadio_data *threadio; > Regards, Hans
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h index aa43e43..a8b800e 100644 --- a/include/media/videobuf2-core.h +++ b/include/media/videobuf2-core.h @@ -491,6 +491,9 @@ struct vb2_buf_ops { * @last_buffer_dequeued: used in poll() and DQBUF to immediately return if the * last decoded buffer was already dequeued. Set for capture queues * when a buffer with the V4L2_BUF_FLAG_LAST is dequeued. + * @ordered: if the driver can guarantee that the queue will be ordered or not. + * The default is not ordered unless the driver sets this flag. It + * is mandatory for using explicit fences. * @fileio: file io emulator internal data, used only if emulator is active * @threadio: thread io internal data, used only if thread is active */ @@ -541,6 +544,7 @@ struct vb2_queue { unsigned int is_output:1; unsigned int copy_timestamp:1; unsigned int last_buffer_dequeued:1; + unsigned int ordered:1; struct vb2_fileio_data *fileio; struct vb2_threadio_data *threadio;