On 12/01/2024 22:26, Andrzej Pietrasiewicz wrote:
> Hi Tomasz,
>
> W dniu 10.01.2024 o 13:41, Tomasz Figa pisze:
>> On Fri, Jan 5, 2024 at 10:40 PM Andrzej Pietrasiewicz
>> <andrzej.p@xxxxxxxxxxxxx> wrote:
>>>
>>> While at it rearrange other comments to match the order of struct members.
>>>
>>> Fixes: d65842f7126a ("media: vb2: add waiting_in_dqbuf flag")
>>> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@xxxxxxxxxxxxx>
>>> ---
>>> include/media/videobuf2-core.h | 12 +++++++-----
>>> 1 file changed, 7 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
>>> index e41204df19f0..5020e052eda0 100644
>>> --- a/include/media/videobuf2-core.h
>>> +++ b/include/media/videobuf2-core.h
>>> @@ -72,6 +72,10 @@ struct vb2_buffer;
>>> * argument to other ops in this structure.
>>> * @put_userptr: inform the allocator that a USERPTR buffer will no longer
>>> * be used.
>>> + * @prepare: called every time the buffer is passed from userspace to the
>>> + * driver, useful for cache synchronisation, optional.
>>> + * @finish: called every time the buffer is passed back from the driver
>>> + * to the userspace, also optional.
>>> * @attach_dmabuf: attach a shared &struct dma_buf for a hardware operation;
>>> * used for DMABUF memory types; dev is the alloc device
>>> * dbuf is the shared dma_buf; returns ERR_PTR() on failure;
>>> @@ -86,10 +90,6 @@ struct vb2_buffer;
>>> * dmabuf.
>>> * @unmap_dmabuf: releases access control to the dmabuf - allocator is notified
>>> * that this driver is done using the dmabuf for now.
>>> - * @prepare: called every time the buffer is passed from userspace to the
>>> - * driver, useful for cache synchronisation, optional.
>>> - * @finish: called every time the buffer is passed back from the driver
>>> - * to the userspace, also optional.
>>> * @vaddr: return a kernel virtual address to a given memory buffer
>>> * associated with the passed private structure or NULL if no
>>> * such mapping exists.
>>> @@ -484,7 +484,6 @@ struct vb2_buf_ops {
>>> * caller. For example, for V4L2, it should match
>>> * the types defined on &enum v4l2_buf_type.
>>> * @io_modes: supported io methods (see &enum vb2_io_modes).
>>> - * @alloc_devs: &struct device memory type/allocator-specific per-plane device
>>> * @dev: device to use for the default allocation context if the driver
>>> * doesn't fill in the @alloc_devs array.
>>> * @dma_attrs: DMA attributes to use for the DMA.
>>> @@ -550,6 +549,7 @@ struct vb2_buf_ops {
>>> * @start_streaming can be called. Used when a DMA engine
>>> * cannot be started unless at least this number of buffers
>>> * have been queued into the driver.
>>> + * @alloc_devs: &struct device memory type/allocator-specific per-plane device
>>> */
>>> /*
>>> * Private elements (won't appear at the uAPI book):
>>> @@ -571,6 +571,8 @@ struct vb2_buf_ops {
>>> * @waiting_for_buffers: used in poll() to check if vb2 is still waiting for
>>> * buffers. Only set for capture queues if qbuf has not yet been
>>> * called since poll() needs to return %EPOLLERR in that situation.
>>> + * @waiting_in_dqbuf: set whenever vb2_queue->lock is released while waiting for
>>> + * a buffer to arrive so that -EBUSY can be returned when appropriate
>>
>> Appreciate documentation improvements. Thanks!
>>
>
> I haven't been hunting for opportunities to improve the documentation,
> the opportunity has found me ;P
>
>> Just one comment: Could we make it more clear who sets it? For example > Set by the core for the duration of a blocking DQBUF, when it has
>> to wait for
>> a buffer to become available with vb2_queue->lock released. Used to prevent
>> destroying the queue by other threads.
>
> Makes sense for me.
>
> @Nicolas are you ok with changing the text and retaining your R-b?
>
> Andrzej
Since this patch no longer applies, I think it is best if you make a v2.
I'll mark this one as "Changes Requested" in patchwork.
Regards,
Hans
>>
>> WDYT?
>>
>> Best regards,
>> Tomasz
>>
>
>
