Re: [PATCHv16 01/34] Documentation: v4l: document request API

2018-08-10 Thread Hans Verkuil 
On 08/10/18 12:46, Pavel Machek wrote:
> Hi!
>> From: Alexandre Courbot 
>>
>> Document the request API for V4L2 devices, and amend the documentation
>> of system calls influenced by it.
>>
>> Signed-off-by: Alexandre Courbot 
>> Signed-off-by: Hans Verkuil 
> 
>> --- a/Documentation/media/uapi/v4l/buffer.rst
>> +++ b/Documentation/media/uapi/v4l/buffer.rst
>> @@ -306,10 +306,15 @@ struct v4l2_buffer
>>- A place holder for future extensions. Drivers and applications
>>  must set this to 0.
>>  * - __u32
>> -  - ``reserved``
>> +  - ``request_fd``
>>-
>> -  - A place holder for future extensions. Drivers and applications
>> -must set this to 0.
>> +  - The file descriptor of the request to queue the buffer to. If 
>> specified
>> +and flag ``V4L2_BUF_FLAG_REQUEST_FD`` is set, then the buffer will 
>> be
> 
> Delete "specified and" -- 0 is valid fd?

Good catch!

> 
>> +queued to that request. This is set by the user when calling
>> +:ref:`ioctl VIDIOC_QBUF ` and ignored by other ioctls.
>> +If the device does not support requests, then ``EPERM`` will be 
>> returned.
>> +If requests are supported but an invalid request FD is given, then
>> +``ENOENT`` will be returned.
> 
> Should this still specify that this should be 0 if
> V4L2_BUF_FLAG_REQUEST_FD is not set?

I don't think so. But I can mentioned with request_fd that it is ignored if
V4L2_BUF_FLAG_REQUEST_FD is not set.

Regards,

Hans

> 
>   Pavel
>

Re: [PATCHv16 01/34] Documentation: v4l: document request API

2018-08-10 Thread Pavel Machek 
Hi!
> From: Alexandre Courbot 
> 
> Document the request API for V4L2 devices, and amend the documentation
> of system calls influenced by it.
> 
> Signed-off-by: Alexandre Courbot 
> Signed-off-by: Hans Verkuil 

> --- a/Documentation/media/uapi/v4l/buffer.rst
> +++ b/Documentation/media/uapi/v4l/buffer.rst
> @@ -306,10 +306,15 @@ struct v4l2_buffer
>- A place holder for future extensions. Drivers and applications
>   must set this to 0.
>  * - __u32
> -  - ``reserved``
> +  - ``request_fd``
>-
> -  - A place holder for future extensions. Drivers and applications
> - must set this to 0.
> +  - The file descriptor of the request to queue the buffer to. If 
> specified
> +and flag ``V4L2_BUF_FLAG_REQUEST_FD`` is set, then the buffer will be

Delete "specified and" -- 0 is valid fd?

> + queued to that request. This is set by the user when calling
> + :ref:`ioctl VIDIOC_QBUF ` and ignored by other ioctls.
> + If the device does not support requests, then ``EPERM`` will be 
> returned.
> + If requests are supported but an invalid request FD is given, then
> + ``ENOENT`` will be returned.

Should this still specify that this should be 0 if
V4L2_BUF_FLAG_REQUEST_FD is not set?

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: [PATCHv16 01/34] Documentation: v4l: document request API

2018-08-04 Thread Hans Verkuil 
Hi Tomasz,

Thank you for your review. Unless stated otherwise I have incorporated your
suggestions.

On 08/03/2018 05:00 PM, Tomasz Figa wrote:
> Hi Hans,
> 
> On Fri, Jul 6, 2018 at 1:04 AM Hans Verkuil  wrote:
>>
>> From: Alexandre Courbot 
>>
>> Document the request API for V4L2 devices, and amend the documentation
>> of system calls influenced by it.
>>
>> Signed-off-by: Alexandre Courbot 
>> Signed-off-by: Hans Verkuil 
>> ---
>>  .../media/uapi/mediactl/media-controller.rst  |   1 +
>>  .../media/uapi/mediactl/media-funcs.rst   |   6 +
>>  .../uapi/mediactl/media-ioc-request-alloc.rst |  77 ++
>>  .../uapi/mediactl/media-request-ioc-queue.rst |  81 ++
>>  .../mediactl/media-request-ioc-reinit.rst |  51 
>>  .../media/uapi/mediactl/request-api.rst   | 247 ++
>>  .../uapi/mediactl/request-func-close.rst  |  49 
>>  .../uapi/mediactl/request-func-ioctl.rst  |  68 +
>>  .../media/uapi/mediactl/request-func-poll.rst |  74 ++
>>  Documentation/media/uapi/v4l/buffer.rst   |  21 +-
>>  .../media/uapi/v4l/vidioc-g-ext-ctrls.rst |  94 ---
>>  Documentation/media/uapi/v4l/vidioc-qbuf.rst  |  32 ++-
>>  .../media/videodev2.h.rst.exceptions  |   1 +
>>  13 files changed, 766 insertions(+), 36 deletions(-)
>>  create mode 100644 
>> Documentation/media/uapi/mediactl/media-ioc-request-alloc.rst
>>  create mode 100644 
>> Documentation/media/uapi/mediactl/media-request-ioc-queue.rst
>>  create mode 100644 
>> Documentation/media/uapi/mediactl/media-request-ioc-reinit.rst
>>  create mode 100644 Documentation/media/uapi/mediactl/request-api.rst
>>  create mode 100644 Documentation/media/uapi/mediactl/request-func-close.rst
>>  create mode 100644 Documentation/media/uapi/mediactl/request-func-ioctl.rst
>>  create mode 100644 Documentation/media/uapi/mediactl/request-func-poll.rst
>>
> 
> Thanks a lot for working on this and sorry for being late to the
> party. Please see some comments inline. (Mostly wording nits, though.)
> 



>> diff --git a/Documentation/media/uapi/mediactl/request-api.rst 
>> b/Documentation/media/uapi/mediactl/request-api.rst
>> new file mode 100644
>> index ..3c49627acb72
>> --- /dev/null
>> +++ b/Documentation/media/uapi/mediactl/request-api.rst
>> @@ -0,0 +1,247 @@
>> +.. -*- coding: utf-8; mode: rst -*-
>> +
>> +.. _media-request-api:
>> +
>> +Request API
>> +===
>> +
>> +The Request API has been designed to allow V4L2 to deal with requirements of
>> +modern devices (stateless codecs, complex camera pipelines, ...) and APIs
>> +(Android Codec v2). One such requirement is the ability for devices 
>> belonging to
>> +the same pipeline to reconfigure and collaborate closely on a per-frame 
>> basis.
>> +Another is efficient support of stateless codecs, which need per-frame 
>> controls
>> +to be set synchronously in order to be used efficiently.
> 
> I think "synchronously" would match what we could do without Request
> API (wait for 1 frame to be dequeued, set control, queue next frame,
> etc.) and it would be inefficient. Perhaps "Another is efficient
> support of stateless codecs, which need controls to be set for exact
> frames beforehand to be used efficiently."?

I went with this:

"Another is support of stateless codecs, which require controls to be applied
to specific frames (aka 'per-frame controls') in order to be used efficiently."



>> +Once the request is fully prepared, it can be queued to the driver:
>> +
>> +.. code-block:: c
>> +
>> +   if (ioctl(req_fd, MEDIA_REQUEST_IOC_QUEUE))
>> +   return some_error;
> 
> This may suggest that it's okay to ignore the error code and just
> handle all the errors the same way. Perhaps "return some_error; /*
> check errno for exact error code */"?

I've just renamed 'some_error' to 'errno'. That makes much more sense.



>> diff --git a/Documentation/media/uapi/mediactl/request-func-close.rst 
>> b/Documentation/media/uapi/mediactl/request-func-close.rst
>> new file mode 100644
>> index ..1195f493bf05
>> --- /dev/null
>> +++ b/Documentation/media/uapi/mediactl/request-func-close.rst
>> @@ -0,0 +1,49 @@
>> +.. SPDX-License-Identifier: GPL-2.0
>> +.. -*- coding: utf-8; mode: rst -*-
>> +
>> +.. _request-func-close:
>> +
>> +***
>> +request close()
>> +***
>> +
>> +Name
>> +
>> +
>> +request-close - Close a request file descriptor
>> +
>> +
>> +Synopsis
>> +
>> +
>> +.. code-block:: c
>> +
>> +#include 
>> +
>> +
>> +.. c:function:: int close( int fd )
>> +:name: req-close
>> +
>> +Arguments
>> +=
>> +
>> +``fd``
>> +File descriptor returned by :ref:`MEDIA_IOC_REQUEST_ALLOC`.
>> +
> 
> I guess for now we're not considering exchanging those between
> processes, but even then, this could be a dup()ed descriptor too.

I've kept this as-is. We never mention dup() specifically for other
APIs either and I think it is overkill to do so.

> 
>> +
>> +Description
>>

Re: [PATCHv16 01/34] Documentation: v4l: document request API

2018-08-03 Thread Tomasz Figa 
Hi Hans,

On Fri, Jul 6, 2018 at 1:04 AM Hans Verkuil  wrote:
>
> From: Alexandre Courbot 
>
> Document the request API for V4L2 devices, and amend the documentation
> of system calls influenced by it.
>
> Signed-off-by: Alexandre Courbot 
> Signed-off-by: Hans Verkuil 
> ---
>  .../media/uapi/mediactl/media-controller.rst  |   1 +
>  .../media/uapi/mediactl/media-funcs.rst   |   6 +
>  .../uapi/mediactl/media-ioc-request-alloc.rst |  77 ++
>  .../uapi/mediactl/media-request-ioc-queue.rst |  81 ++
>  .../mediactl/media-request-ioc-reinit.rst |  51 
>  .../media/uapi/mediactl/request-api.rst   | 247 ++
>  .../uapi/mediactl/request-func-close.rst  |  49 
>  .../uapi/mediactl/request-func-ioctl.rst  |  68 +
>  .../media/uapi/mediactl/request-func-poll.rst |  74 ++
>  Documentation/media/uapi/v4l/buffer.rst   |  21 +-
>  .../media/uapi/v4l/vidioc-g-ext-ctrls.rst |  94 ---
>  Documentation/media/uapi/v4l/vidioc-qbuf.rst  |  32 ++-
>  .../media/videodev2.h.rst.exceptions  |   1 +
>  13 files changed, 766 insertions(+), 36 deletions(-)
>  create mode 100644 
> Documentation/media/uapi/mediactl/media-ioc-request-alloc.rst
>  create mode 100644 
> Documentation/media/uapi/mediactl/media-request-ioc-queue.rst
>  create mode 100644 
> Documentation/media/uapi/mediactl/media-request-ioc-reinit.rst
>  create mode 100644 Documentation/media/uapi/mediactl/request-api.rst
>  create mode 100644 Documentation/media/uapi/mediactl/request-func-close.rst
>  create mode 100644 Documentation/media/uapi/mediactl/request-func-ioctl.rst
>  create mode 100644 Documentation/media/uapi/mediactl/request-func-poll.rst
>

Thanks a lot for working on this and sorry for being late to the
party. Please see some comments inline. (Mostly wording nits, though.)

> diff --git a/Documentation/media/uapi/mediactl/media-controller.rst 
> b/Documentation/media/uapi/mediactl/media-controller.rst
> index 0eea4f9a07d5..66aff38cd499 100644
> --- a/Documentation/media/uapi/mediactl/media-controller.rst
> +++ b/Documentation/media/uapi/mediactl/media-controller.rst
> @@ -21,6 +21,7 @@ Part IV - Media Controller API
>  media-controller-intro
>  media-controller-model
>  media-types
> +request-api
>  media-funcs
>  media-header
>
> diff --git a/Documentation/media/uapi/mediactl/media-funcs.rst 
> b/Documentation/media/uapi/mediactl/media-funcs.rst
> index 076856501cdb..260f9dcadcde 100644
> --- a/Documentation/media/uapi/mediactl/media-funcs.rst
> +++ b/Documentation/media/uapi/mediactl/media-funcs.rst
> @@ -16,3 +16,9 @@ Function Reference
>  media-ioc-enum-entities
>  media-ioc-enum-links
>  media-ioc-setup-link
> +media-ioc-request-alloc
> +request-func-close
> +request-func-ioctl
> +request-func-poll
> +media-request-ioc-queue
> +media-request-ioc-reinit
> diff --git a/Documentation/media/uapi/mediactl/media-ioc-request-alloc.rst 
> b/Documentation/media/uapi/mediactl/media-ioc-request-alloc.rst
> new file mode 100644
> index ..8f8ecf6c63b6
> --- /dev/null
> +++ b/Documentation/media/uapi/mediactl/media-ioc-request-alloc.rst
> @@ -0,0 +1,77 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +.. _media_ioc_request_alloc:
> +
> +*
> +ioctl MEDIA_IOC_REQUEST_ALLOC
> +*
> +
> +Name
> +
> +
> +MEDIA_IOC_REQUEST_ALLOC - Allocate a request
> +
> +
> +Synopsis
> +
> +
> +.. c:function:: int ioctl( int fd, MEDIA_IOC_REQUEST_ALLOC, struct 
> media_request_alloc *argp )
> +:name: MEDIA_IOC_REQUEST_ALLOC
> +
> +
> +Arguments
> +=
> +
> +``fd``
> +File descriptor returned by :ref:`open() `.
> +
> +``argp``
> +

Missing description.

> +
> +Description
> +===
> +
> +If the media device supports :ref:`requests `, then
> +this ioctl can be used to allocate a request. If it is not supported, then
> +``errno`` is set to ``ENOTTY``. A request is accessed through a file 
> descriptor
> +that is returned in struct :c:type:`media_request_alloc`.
> +
> +If the request was successfully allocated, then the request file descriptor
> +can be passed to the :ref:`VIDIOC_QBUF `,
> +:ref:`VIDIOC_G_EXT_CTRLS `,
> +:ref:`VIDIOC_S_EXT_CTRLS ` and
> +:ref:`VIDIOC_TRY_EXT_CTRLS ` ioctls.
> +
> +In addition, the request can be queued by calling
> +:ref:`MEDIA_REQUEST_IOC_QUEUE` and re-initialized by calling
> +:ref:`MEDIA_REQUEST_IOC_REINIT`.
> +
> +Finally, the file descriptor can be :ref:`polled ` to wait
> +for the request to complete.
> +
> +The request will remain allocated until the associated file descriptor is
> +closed by :ref:`close() ` and the driver no longer uses
> +the request internally.

I suppose that would be "until all the file descriptors associated
with it are closed and", since one could dup() the original file
descriptor and even close it, keeping just the copy.

> +
> +.. c:type:: media_request_alloc
> +
> +.. tabularcolumns::

[PATCHv16 01/34] Documentation: v4l: document request API

2018-07-05 Thread Hans Verkuil 
From: Alexandre Courbot 

Document the request API for V4L2 devices, and amend the documentation
of system calls influenced by it.

Signed-off-by: Alexandre Courbot 
Signed-off-by: Hans Verkuil 
---
 .../media/uapi/mediactl/media-controller.rst  |   1 +
 .../media/uapi/mediactl/media-funcs.rst   |   6 +
 .../uapi/mediactl/media-ioc-request-alloc.rst |  77 ++
 .../uapi/mediactl/media-request-ioc-queue.rst |  81 ++
 .../mediactl/media-request-ioc-reinit.rst |  51 
 .../media/uapi/mediactl/request-api.rst   | 247 ++
 .../uapi/mediactl/request-func-close.rst  |  49 
 .../uapi/mediactl/request-func-ioctl.rst  |  68 +
 .../media/uapi/mediactl/request-func-poll.rst |  74 ++
 Documentation/media/uapi/v4l/buffer.rst   |  21 +-
 .../media/uapi/v4l/vidioc-g-ext-ctrls.rst |  94 ---
 Documentation/media/uapi/v4l/vidioc-qbuf.rst  |  32 ++-
 .../media/videodev2.h.rst.exceptions  |   1 +
 13 files changed, 766 insertions(+), 36 deletions(-)
 create mode 100644 
Documentation/media/uapi/mediactl/media-ioc-request-alloc.rst
 create mode 100644 
Documentation/media/uapi/mediactl/media-request-ioc-queue.rst
 create mode 100644 
Documentation/media/uapi/mediactl/media-request-ioc-reinit.rst
 create mode 100644 Documentation/media/uapi/mediactl/request-api.rst
 create mode 100644 Documentation/media/uapi/mediactl/request-func-close.rst
 create mode 100644 Documentation/media/uapi/mediactl/request-func-ioctl.rst
 create mode 100644 Documentation/media/uapi/mediactl/request-func-poll.rst

diff --git a/Documentation/media/uapi/mediactl/media-controller.rst 
b/Documentation/media/uapi/mediactl/media-controller.rst
index 0eea4f9a07d5..66aff38cd499 100644
--- a/Documentation/media/uapi/mediactl/media-controller.rst
+++ b/Documentation/media/uapi/mediactl/media-controller.rst
@@ -21,6 +21,7 @@ Part IV - Media Controller API
 media-controller-intro
 media-controller-model
 media-types
+request-api
 media-funcs
 media-header
 
diff --git a/Documentation/media/uapi/mediactl/media-funcs.rst 
b/Documentation/media/uapi/mediactl/media-funcs.rst
index 076856501cdb..260f9dcadcde 100644
--- a/Documentation/media/uapi/mediactl/media-funcs.rst
+++ b/Documentation/media/uapi/mediactl/media-funcs.rst
@@ -16,3 +16,9 @@ Function Reference
 media-ioc-enum-entities
 media-ioc-enum-links
 media-ioc-setup-link
+media-ioc-request-alloc
+request-func-close
+request-func-ioctl
+request-func-poll
+media-request-ioc-queue
+media-request-ioc-reinit
diff --git a/Documentation/media/uapi/mediactl/media-ioc-request-alloc.rst 
b/Documentation/media/uapi/mediactl/media-ioc-request-alloc.rst
new file mode 100644
index ..8f8ecf6c63b6
--- /dev/null
+++ b/Documentation/media/uapi/mediactl/media-ioc-request-alloc.rst
@@ -0,0 +1,77 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+.. _media_ioc_request_alloc:
+
+*
+ioctl MEDIA_IOC_REQUEST_ALLOC
+*
+
+Name
+
+
+MEDIA_IOC_REQUEST_ALLOC - Allocate a request
+
+
+Synopsis
+
+
+.. c:function:: int ioctl( int fd, MEDIA_IOC_REQUEST_ALLOC, struct 
media_request_alloc *argp )
+:name: MEDIA_IOC_REQUEST_ALLOC
+
+
+Arguments
+=
+
+``fd``
+File descriptor returned by :ref:`open() `.
+
+``argp``
+
+
+Description
+===
+
+If the media device supports :ref:`requests `, then
+this ioctl can be used to allocate a request. If it is not supported, then
+``errno`` is set to ``ENOTTY``. A request is accessed through a file descriptor
+that is returned in struct :c:type:`media_request_alloc`.
+
+If the request was successfully allocated, then the request file descriptor
+can be passed to the :ref:`VIDIOC_QBUF `,
+:ref:`VIDIOC_G_EXT_CTRLS `,
+:ref:`VIDIOC_S_EXT_CTRLS ` and
+:ref:`VIDIOC_TRY_EXT_CTRLS ` ioctls.
+
+In addition, the request can be queued by calling
+:ref:`MEDIA_REQUEST_IOC_QUEUE` and re-initialized by calling
+:ref:`MEDIA_REQUEST_IOC_REINIT`.
+
+Finally, the file descriptor can be :ref:`polled ` to wait
+for the request to complete.
+
+The request will remain allocated until the associated file descriptor is
+closed by :ref:`close() ` and the driver no longer uses
+the request internally.
+
+.. c:type:: media_request_alloc
+
+.. tabularcolumns:: |p{4.4cm}|p{4.4cm}|p{8.7cm}|
+
+.. flat-table:: struct media_request_alloc
+:header-rows:  0
+:stub-columns: 0
+:widths:   1 1 2
+
+*  -  __s32
+   -  ``fd``
+   -  The file descriptor of the request.
+
+Return Value
+
+
+On success 0 is returned, on error -1 and the ``errno`` variable is set
+appropriately. The generic error codes are described at the
+:ref:`Generic Error Codes ` chapter.
+
+ENOTTY
+The driver has no support for requests.
diff --git a/Documentation/media/uapi/mediactl/media-request-ioc-queue.rst 
b/Documentation/media/uapi/mediactl/media-request-ioc-queue.rst
new file mode 100644
index