Re: [PATCHv2 0/9] vb2/cedrus: add tag support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 11/14/2018 02:47 PM, Hans Verkuil wrote:
> From: Hans Verkuil <hansverk@xxxxxxxxx>
> 
> As was discussed here (among other places):
> 
> https://lkml.org/lkml/2018/10/19/440
> 
> using capture queue buffer indices to refer to reference frames is
> not a good idea. A better idea is to use a 'tag' where the
> application can assign a u64 tag to an output buffer, which is then 
> copied to the capture buffer(s) derived from the output buffer.
> 
> A u64 is chosen since this allows userspace to also use pointers to
> internal structures as 'tag'.
> 
> The first three patches add core tag support, the next patch document
> the tag support, then a new helper function is added to v4l2-mem2mem.c
> to easily copy data from a source to a destination buffer that drivers
> can use.
> 
> Next a new supports_tags vb2_queue flag is added to indicate that
> the driver supports tags. Ideally this should not be necessary, but
> that would require that all m2m drivers are converted to using the
> new helper function introduced in the previous patch. That takes more
> time then I have now since we want to get this in for 4.20.
> 
> Finally the vim2m, vicodec and cedrus drivers are converted to support
> tags.
> 
> I also removed the 'pad' fields from the mpeg2 control structs (it
> should never been added in the first place) and aligned the structs
> to a u32 boundary (u64 for the tag values).
> 
> Note that this might change further (Paul suggested using bitfields).
> 
> Also note that the cedrus code doesn't set the sequence counter, that's
> something that should still be added before this driver can be moved
> out of staging.
> 
> Note: if no buffer is found for a certain tag, then the dma address
> is just set to 0. That happened before as well with invalid buffer
> indices. This should be checked in the driver!
> 
> The previous RFC series was tested successfully with the cedrus driver.
> 
> Regards,
> 
>         Hans

I'd like to get some Acked-by or Reviewed-by replies for this series.
Or comments if you don't like something.

I would really like to get this in for 4.20 so the cedrus API is stable
(hopefully), since this is a last outstanding API item.

Tomasz: you commented that the text still referred to the tag as a u64,
but that was only in the cover letter, not the patches themselves. So
I don't plan to post a v3 just for that.

Regards,

	Hans

> 
> Changes since v1:
> 
> - changed to a u32 tag. Using a 64 bit tag was overly complicated due
>   to the bad layout of the v4l2_buffer struct, and there is no real
>   need for it by applications.
> 
> Main changes since the RFC:
> 
> - Added new buffer capability flag
> - Added m2m helper to copy data between buffers
> - Added documentation
> - Added tag logging in v4l2-ioctl.c
> 
> Hans Verkuil (9):
>   videodev2.h: add tag support
>   vb2: add tag support
>   v4l2-ioctl.c: log v4l2_buffer tag
>   buffer.rst: document the new buffer tag feature.
>   v4l2-mem2mem: add v4l2_m2m_buf_copy_data helper function
>   vb2: add new supports_tags queue flag
>   vim2m: add tag support
>   vicodec: add tag support
>   cedrus: add tag support
> 
>  Documentation/media/uapi/v4l/buffer.rst       | 32 +++++++++----
>  .../media/uapi/v4l/vidioc-reqbufs.rst         |  4 ++
>  .../media/common/videobuf2/videobuf2-v4l2.c   | 45 ++++++++++++++++---
>  drivers/media/platform/vicodec/vicodec-core.c | 14 ++----
>  drivers/media/platform/vim2m.c                | 14 ++----
>  drivers/media/v4l2-core/v4l2-ctrls.c          |  9 ----
>  drivers/media/v4l2-core/v4l2-ioctl.c          |  9 ++--
>  drivers/media/v4l2-core/v4l2-mem2mem.c        | 23 ++++++++++
>  drivers/staging/media/sunxi/cedrus/cedrus.h   |  9 ++--
>  .../staging/media/sunxi/cedrus/cedrus_dec.c   |  2 +
>  .../staging/media/sunxi/cedrus/cedrus_mpeg2.c | 21 ++++-----
>  .../staging/media/sunxi/cedrus/cedrus_video.c |  2 +
>  include/media/v4l2-mem2mem.h                  | 21 +++++++++
>  include/media/videobuf2-core.h                |  2 +
>  include/media/videobuf2-v4l2.h                | 21 ++++++++-
>  include/uapi/linux/v4l2-controls.h            | 14 +++---
>  include/uapi/linux/videodev2.h                |  9 +++-
>  17 files changed, 178 insertions(+), 73 deletions(-)
> 




[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux