Re: [PATCH v7 1/6] [media] vb2: add is_unordered callback for drivers
On Mon, Jan 15, 2018 at 9:01 PM, Gustavo Padovan wrote: > 2018-01-15 Alexandre Courbot : > >> On Thu, Jan 11, 2018 at 1:07 AM, Gustavo Padovan wrote: >> > From: Gustavo Padovan >> > >> > Explicit synchronization benefits a lot from ordered queues, they fit >> > better in a pipeline with DRM for example so create a opt-in way for >> > drivers notify videobuf2 that the queue is unordered. >> > >> > Drivers don't need implement it if the queue is ordered. >> >> This is going to make user-space believe that *all* vb2 drivers use >> ordered queues by default, at least until non-ordered drivers catch up >> with this change. Wouldn't it be less dangerous to do the opposite >> (make queues non-ordered by default)? > > The rational behind this decision was because most formats/drivers are > ordered so only a small amount of drivers need to changed. I think this > was proposed by Hans on the Media Summit. As long as all concerned drivers are updated we should be on the safe side. At first I was surprised that we expose the ordering feature in a negative tense, but if the vast majority of devices are ordered this probably makes sense.
Re: [PATCH v7 1/6] [media] vb2: add is_unordered callback for drivers
2018-01-15 Hans Verkuil : > On 01/15/2018 01:01 PM, Gustavo Padovan wrote: > > 2018-01-15 Alexandre Courbot : > > > >> On Thu, Jan 11, 2018 at 1:07 AM, Gustavo Padovan > >> wrote: > >>> From: Gustavo Padovan > >>> > >>> Explicit synchronization benefits a lot from ordered queues, they fit > >>> better in a pipeline with DRM for example so create a opt-in way for > >>> drivers notify videobuf2 that the queue is unordered. > >>> > >>> Drivers don't need implement it if the queue is ordered. > >> > >> This is going to make user-space believe that *all* vb2 drivers use > >> ordered queues by default, at least until non-ordered drivers catch up > >> with this change. Wouldn't it be less dangerous to do the opposite > >> (make queues non-ordered by default)? > > > > The rational behind this decision was because most formats/drivers are > > ordered so only a small amount of drivers need to changed. I think this > > was proposed by Hans on the Media Summit. > > > > I understand your concern. My question is how dangerous will it be. If > > you are building a product you will make the changes in the driver if > > they are not there yet, or if it is a distribution you'd never know > > which driver/format you are using so you should be prepared for > > everything. > > > > AFAIK all Capture drivers are ordered and that is where I think fences > > is most useful. > > Right. What could be done is to mark all codec drivers as unordered initially > ask the driver authors to verify this. All capture drivers using vb2 and not > using REQUEUE are ordered. That is a good way out. > > One thing we haven't looked at is what to do with drivers that do not use vb2. > Those won't support fences, but how will userspace know that fences are not > supported? I'm not sure what the best method is for that. > > I am leaning towards a new capability since this has to be advertised clearly. The capability flag makes sense to me, I'll incorporate it as part of my next patchset. Gustavo
Re: [PATCH v7 1/6] [media] vb2: add is_unordered callback for drivers
On 01/15/2018 01:01 PM, Gustavo Padovan wrote: > 2018-01-15 Alexandre Courbot : > >> On Thu, Jan 11, 2018 at 1:07 AM, Gustavo Padovan wrote: >>> From: Gustavo Padovan >>> >>> Explicit synchronization benefits a lot from ordered queues, they fit >>> better in a pipeline with DRM for example so create a opt-in way for >>> drivers notify videobuf2 that the queue is unordered. >>> >>> Drivers don't need implement it if the queue is ordered. >> >> This is going to make user-space believe that *all* vb2 drivers use >> ordered queues by default, at least until non-ordered drivers catch up >> with this change. Wouldn't it be less dangerous to do the opposite >> (make queues non-ordered by default)? > > The rational behind this decision was because most formats/drivers are > ordered so only a small amount of drivers need to changed. I think this > was proposed by Hans on the Media Summit. > > I understand your concern. My question is how dangerous will it be. If > you are building a product you will make the changes in the driver if > they are not there yet, or if it is a distribution you'd never know > which driver/format you are using so you should be prepared for > everything. > > AFAIK all Capture drivers are ordered and that is where I think fences > is most useful. Right. What could be done is to mark all codec drivers as unordered initially ask the driver authors to verify this. All capture drivers using vb2 and not using REQUEUE are ordered. One thing we haven't looked at is what to do with drivers that do not use vb2. Those won't support fences, but how will userspace know that fences are not supported? I'm not sure what the best method is for that. I am leaning towards a new capability since this has to be advertised clearly. Regards, Hans
Re: [PATCH v7 1/6] [media] vb2: add is_unordered callback for drivers
2018-01-15 Alexandre Courbot : > On Thu, Jan 11, 2018 at 1:07 AM, Gustavo Padovan wrote: > > From: Gustavo Padovan > > > > Explicit synchronization benefits a lot from ordered queues, they fit > > better in a pipeline with DRM for example so create a opt-in way for > > drivers notify videobuf2 that the queue is unordered. > > > > Drivers don't need implement it if the queue is ordered. > > This is going to make user-space believe that *all* vb2 drivers use > ordered queues by default, at least until non-ordered drivers catch up > with this change. Wouldn't it be less dangerous to do the opposite > (make queues non-ordered by default)? The rational behind this decision was because most formats/drivers are ordered so only a small amount of drivers need to changed. I think this was proposed by Hans on the Media Summit. I understand your concern. My question is how dangerous will it be. If you are building a product you will make the changes in the driver if they are not there yet, or if it is a distribution you'd never know which driver/format you are using so you should be prepared for everything. AFAIK all Capture drivers are ordered and that is where I think fences is most useful. Gustavo
Re: [PATCH v7 1/6] [media] vb2: add is_unordered callback for drivers
On Thu, Jan 11, 2018 at 1:07 AM, Gustavo Padovan wrote: > From: Gustavo Padovan > > Explicit synchronization benefits a lot from ordered queues, they fit > better in a pipeline with DRM for example so create a opt-in way for > drivers notify videobuf2 that the queue is unordered. > > Drivers don't need implement it if the queue is ordered. This is going to make user-space believe that *all* vb2 drivers use ordered queues by default, at least until non-ordered drivers catch up with this change. Wouldn't it be less dangerous to do the opposite (make queues non-ordered by default)?
Re: [PATCH v7 1/6] [media] vb2: add is_unordered callback for drivers
On 01/10/18 17:07, Gustavo Padovan wrote:
> From: Gustavo Padovan
>
> Explicit synchronization benefits a lot from ordered queues, they fit
> better in a pipeline with DRM for example so create a opt-in way for
> drivers notify videobuf2 that the queue is unordered.
>
> Drivers don't need implement it if the queue is ordered.
>
> Signed-off-by: Gustavo Padovan
> ---
> include/media/videobuf2-core.h | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
> index f3ee4c7c2fb3..583cdc06de79 100644
> --- a/include/media/videobuf2-core.h
> +++ b/include/media/videobuf2-core.h
> @@ -370,6 +370,9 @@ struct vb2_buffer {
> * callback by calling vb2_buffer_done() with either
> * %VB2_BUF_STATE_DONE or %VB2_BUF_STATE_ERROR; may use
> * vb2_wait_for_all_buffers() function
> + * @is_unordered:tell if the queue format is unordered. The default is
I'd replace the first sentence by this:
"tell if the queue is unordered, i.e. buffers can be dequeued in a different
order from how they were queued."
Regards,
Hans
> + * assumed to be ordered and this function only needs to
> + * be implemented for unordered queues.
> * @buf_queue: passes buffer vb to the driver; driver may start
> * hardware operation on this buffer; driver should give
> * the buffer back by calling vb2_buffer_done() function;
> @@ -393,6 +396,7 @@ struct vb2_ops {
>
> int (*start_streaming)(struct vb2_queue *q, unsigned int count);
> void (*stop_streaming)(struct vb2_queue *q);
> + int (*is_unordered)(struct vb2_queue *q);
>
> void (*buf_queue)(struct vb2_buffer *vb);
> };
> @@ -566,6 +570,7 @@ struct vb2_queue {
> u32 cnt_wait_finish;
> u32 cnt_start_streaming;
> u32 cnt_stop_streaming;
> + u32 cnt_is_unordered;
> #endif
> };
>
>
[PATCH v7 1/6] [media] vb2: add is_unordered callback for drivers
From: Gustavo Padovan
Explicit synchronization benefits a lot from ordered queues, they fit
better in a pipeline with DRM for example so create a opt-in way for
drivers notify videobuf2 that the queue is unordered.
Drivers don't need implement it if the queue is ordered.
Signed-off-by: Gustavo Padovan
---
include/media/videobuf2-core.h | 5 +
1 file changed, 5 insertions(+)
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index f3ee4c7c2fb3..583cdc06de79 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -370,6 +370,9 @@ struct vb2_buffer {
* callback by calling vb2_buffer_done() with either
* %VB2_BUF_STATE_DONE or %VB2_BUF_STATE_ERROR; may use
* vb2_wait_for_all_buffers() function
+ * @is_unordered: tell if the queue format is unordered. The default is
+ * assumed to be ordered and this function only needs to
+ * be implemented for unordered queues.
* @buf_queue: passes buffer vb to the driver; driver may start
* hardware operation on this buffer; driver should give
* the buffer back by calling vb2_buffer_done() function;
@@ -393,6 +396,7 @@ struct vb2_ops {
int (*start_streaming)(struct vb2_queue *q, unsigned int count);
void (*stop_streaming)(struct vb2_queue *q);
+ int (*is_unordered)(struct vb2_queue *q);
void (*buf_queue)(struct vb2_buffer *vb);
};
@@ -566,6 +570,7 @@ struct vb2_queue {
u32 cnt_wait_finish;
u32 cnt_start_streaming;
u32 cnt_stop_streaming;
+ u32 cnt_is_unordered;
#endif
};
--
2.14.3
