Hi Pawel!
Thanks for doing this work, but I have a few comments...
On Tuesday, November 22, 2011 06:26:37 Pawel Osciak wrote:
> The app_offset can only be set by userspace and will be passed by vb2 to
> the driver.
>
> Signed-off-by: Pawel Osciak <pa...@osciak.com>
> CC: Marek Szyprowski <m.szyprow...@samsung.com>
> ---
> drivers/media/video/videobuf2-core.c | 5 +++++
> 1 files changed, 5 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/media/video/videobuf2-core.c
> b/drivers/media/video/videobuf2-core.c
> index 979e544..41cc8e9 100644
> --- a/drivers/media/video/videobuf2-core.c
> +++ b/drivers/media/video/videobuf2-core.c
> @@ -830,6 +830,11 @@ static int __fill_vb2_buffer(struct vb2_buffer *vb,
> const struct v4l2_buffer *b,
> }
> }
>
> + /* App offset can only be set by userspace, for all types */
> + for (plane = 0; plane < vb->num_planes; ++plane)
> + v4l2_planes[plane].app_offset =
> + b->m.planes[plane].app_offset;
> +
> if (b->memory == V4L2_MEMORY_USERPTR) {
> for (plane = 0; plane < vb->num_planes; ++plane) {
> v4l2_planes[plane].m.userptr =
>
I'd like to see this implemented in vivi and preferably one other driver.
What also needs to be clarified is how this affects queue_setup (should the
sizes include the app_offset or not?) and e.g. vb2_plane_size (again, is the
size with or without app_offset?).
Should app_offset handling be enforced (i.e. should all vb2 drivers support
it?) or should it be optional? If optional, then app_offset should be set to
0 somehow.
This code in __qbuf_userptr should probably also be modified as this
currently does not take app_offset into account.
/* Check if the provided plane buffer is large enough */
if (planes[plane].length < q->plane_sizes[plane]) {
ret = -EINVAL;
goto err;
}
I think there are some subtleties that we don't know about yet, so implementing
this in a real driver would hopefully bring those subtleties out in the open.
Regards,
Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
