All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFCv2] Request API
@ 2018-03-23  8:46 Hans Verkuil
  2018-03-23 15:23 ` Tomasz Figa
  2018-03-23 23:06 ` Sakari Ailus
  0 siblings, 2 replies; 3+ messages in thread
From: Hans Verkuil @ 2018-03-23  8:46 UTC (permalink / raw)
  To: Linux Media Mailing List, Sakari Ailus, Alexandre Courbot,
	Tomasz Figa, Pawel Osciak

RFC Request API, version 2
--------------------------

This document proposes the public API for handling requests.

There has been some confusion about how to do this, so this summarizes the
current approach based on conversations with the various stakeholders today
(Sakari, Alexandre Courbot, Thomasz Figa and myself).

1) Additions to the media API

   Allocate an empty request object:

   #define MEDIA_IOC_REQUEST_ALLOC _IOW('|', 0x05, __s32 *)

   This will return a file descriptor representing the request or an error
   if it can't allocate the request.

   If the pointer argument is NULL, then this will just return 0 (if this ioctl
   is implemented) or -ENOTTY otherwise. This can be used to test whether this
   ioctl is supported or not without actually having to allocate a request.

2) Operations on the request fd

   You can queue or reinit a request by calling these ioctls on the request fd:

   #define MEDIA_REQUEST_IOC_QUEUE   _IO('|',  128)
   #define MEDIA_REQUEST_IOC_REINIT  _IO('|',  129)

   With REINIT you reset the state of the request as if you had just allocated
   it.

   You can poll the request fd to wait for it to complete.

   To free a request you close the request fd. Note that it may still be in
   use internally, so the internal datastructures have to be refcounted.

   For this initial implementation only buffers and controls are contained
   in a request. This is needed to implement stateless codecs. Supporting
   complex camera pipelines will require more work.

   Requests only contain the changes to the state at request queue time
   relative to the previously queued request(s) or the current hardware state
   if no other requests were queued.

   Once a request is completed it will retain the state at completion
   time.

3) To associate a v4l2 buffer with a request the 'reserved' field in struct
   v4l2_buffer is used to store the request fd. Buffers won't be 'prepared'
   until the request is queued since the request may contain information that
   is needed to prepare the buffer.

   To indicate that request_fd should be used this flag should be set by
   userspace at QBUF time:

#define V4L2_BUF_FLAG_REQUEST			0x00800000

   This flag will also be returned by the driver to indicate that the buffer
   is associated with a request.

   TBD: what should vb2 return as request_fd value if this flag is set?
   This should act the same as the fence patch series and this is still
   being tweaked so let's wait for that to be merged first, then we can
   finalize this.

4) To associate v4l2 controls with a request we take the first of the
   'reserved[2]' array elements in struct v4l2_ext_controls and use it to store
   the request fd.

   We also add a new WHICH value:

#define V4L2_CTRL_WHICH_REQUEST   0x0f010000

   This tells the control framework to get/set controls from the given
   request fd.

   When querying a control value from a request it will return the newest
   value in the list of pending requests, or the current hardware value if
   is not set in any of the pending requests.

   When a request is completed the controls will no longer change. A copy
   will be made of volatile controls at the time of completion (actually
   it will be up to the driver to decide when to do that).

   Volatile controls and requests:

   - If you set a volatile control in a request, then that will be ignored,
     unless the V4L2_CTRL_FLAG_EXECUTE_ON_WRITE flag is set as well.

   - If you get a volatile control from a request then:
     1) If the request is completed it will return the value of the volatile
        control at completion time.
     2) Otherwise: if the V4L2_CTRL_FLAG_EXECUTE_ON_WRITE is set and it was
        set in a request, then that value is returned.
     3) Otherwise: return the current value from the hardware (i.e. normal
        behavior).

   Read-only controls and requests:

   - If you get a read-only control from a request then:
     1) If the request is completed it will return the value of the read-only
        control at completion time.
     2) Otherwise it will get the current value from the driver (i.e. normal
        behavior).

   Open issue: should we receive control events if a control in a request is
   added/changed? Currently there are no plans to support control events for
   requests. I don't see a clear use-case and neither do I see an easy way
   of implementing this (which fd would receive those events?).

Notes:

- Earlier versions of this API had a TRY command as well to validate the
  request. I'm not sure that is useful so I dropped it, but it can easily
  be added if there is a good use-case for it. Traditionally within V4L the
  TRY ioctl will also update wrong values to something that works, but that
  is not the intention here as far as I understand it. So the validation
  step can also be done when the request is queued and, if it fails, it will
  just return an error.

- If due to performance reasons we will have to allocate/queue/reinit multiple
  requests with a single ioctl, then we will have to add new ioctls to the
  media device. At this moment in time it is not clear that this is really
  needed and it certainly isn't needed for the stateless codec support that
  we are looking at now.

- The behavior of VIDIOC_G_EXT_CTRLS with which == V4L2_CTRL_WHICH_CUR_VAL
  and VIDIOC_G_CTRL remains the same (i.e. it returns the current driver/HW
  values). However, when combined with requests the documentation should make
  clear that this returns a snapshot only and is racy w.r.t. applying values
  from a request.

- There is a discussion whether there should be a VIDIOC_REQUEST_ALLOC ioctl
  for V4L2 in addition to the media ioctl. The reason is that stateless codecs
  do not need the media controller except for allocating requests. So a V4L2
  ioctl would avoid applications from having to deal with a media device.
  This would also add additional hassle w.r.t. SELinux as I understand it.

  Support for this can be added for now as a final patch in the Request API
  patch series and we'll postpone the decision on this.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [RFCv2] Request API
  2018-03-23  8:46 [RFCv2] Request API Hans Verkuil
@ 2018-03-23 15:23 ` Tomasz Figa
  2018-03-23 23:06 ` Sakari Ailus
  1 sibling, 0 replies; 3+ messages in thread
From: Tomasz Figa @ 2018-03-23 15:23 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Linux Media Mailing List, Sakari Ailus, Alexandre Courbot, Pawel Osciak

Hi Hans,

On Fri, Mar 23, 2018 at 5:46 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> RFC Request API, version 2
> --------------------------
>
> This document proposes the public API for handling requests.
>
> There has been some confusion about how to do this, so this summarizes the
> current approach based on conversations with the various stakeholders today
> (Sakari, Alexandre Courbot, Thomasz Figa and myself).
>
> 1) Additions to the media API
>
>    Allocate an empty request object:
>
>    #define MEDIA_IOC_REQUEST_ALLOC _IOW('|', 0x05, __s32 *)
>
>    This will return a file descriptor representing the request or an error
>    if it can't allocate the request.
>
>    If the pointer argument is NULL, then this will just return 0 (if this ioctl
>    is implemented) or -ENOTTY otherwise. This can be used to test whether this
>    ioctl is supported or not without actually having to allocate a request.
>
> 2) Operations on the request fd
>
>    You can queue or reinit a request by calling these ioctls on the request fd:
>
>    #define MEDIA_REQUEST_IOC_QUEUE   _IO('|',  128)
>    #define MEDIA_REQUEST_IOC_REINIT  _IO('|',  129)
>
>    With REINIT you reset the state of the request as if you had just allocated
>    it.
>
>    You can poll the request fd to wait for it to complete.
>
>    To free a request you close the request fd. Note that it may still be in
>    use internally, so the internal datastructures have to be refcounted.
>
>    For this initial implementation only buffers and controls are contained
>    in a request. This is needed to implement stateless codecs. Supporting
>    complex camera pipelines will require more work.
>
>    Requests only contain the changes to the state at request queue time
>    relative to the previously queued request(s) or the current hardware state
>    if no other requests were queued.
>
>    Once a request is completed it will retain the state at completion
>    time.
>
> 3) To associate a v4l2 buffer with a request the 'reserved' field in struct
>    v4l2_buffer is used to store the request fd. Buffers won't be 'prepared'
>    until the request is queued since the request may contain information that
>    is needed to prepare the buffer.
>
>    To indicate that request_fd should be used this flag should be set by
>    userspace at QBUF time:
>
> #define V4L2_BUF_FLAG_REQUEST                   0x00800000
>
>    This flag will also be returned by the driver to indicate that the buffer
>    is associated with a request.
>
>    TBD: what should vb2 return as request_fd value if this flag is set?
>    This should act the same as the fence patch series and this is still
>    being tweaked so let's wait for that to be merged first, then we can
>    finalize this.
>
> 4) To associate v4l2 controls with a request we take the first of the
>    'reserved[2]' array elements in struct v4l2_ext_controls and use it to store
>    the request fd.
>
>    We also add a new WHICH value:
>
> #define V4L2_CTRL_WHICH_REQUEST   0x0f010000
>
>    This tells the control framework to get/set controls from the given
>    request fd.
>
>    When querying a control value from a request it will return the newest
>    value in the list of pending requests, or the current hardware value if
>    is not set in any of the pending requests.
>
>    When a request is completed the controls will no longer change. A copy
>    will be made of volatile controls at the time of completion (actually
>    it will be up to the driver to decide when to do that).
>
>    Volatile controls and requests:
>
>    - If you set a volatile control in a request, then that will be ignored,
>      unless the V4L2_CTRL_FLAG_EXECUTE_ON_WRITE flag is set as well.
>
>    - If you get a volatile control from a request then:
>      1) If the request is completed it will return the value of the volatile
>         control at completion time.
>      2) Otherwise: if the V4L2_CTRL_FLAG_EXECUTE_ON_WRITE is set and it was
>         set in a request, then that value is returned.
>      3) Otherwise: return the current value from the hardware (i.e. normal
>         behavior).
>
>    Read-only controls and requests:
>
>    - If you get a read-only control from a request then:
>      1) If the request is completed it will return the value of the read-only
>         control at completion time.
>      2) Otherwise it will get the current value from the driver (i.e. normal
>         behavior).
>
>    Open issue: should we receive control events if a control in a request is
>    added/changed? Currently there are no plans to support control events for
>    requests. I don't see a clear use-case and neither do I see an easy way
>    of implementing this (which fd would receive those events?).
>
> Notes:
>
> - Earlier versions of this API had a TRY command as well to validate the
>   request. I'm not sure that is useful so I dropped it, but it can easily
>   be added if there is a good use-case for it. Traditionally within V4L the
>   TRY ioctl will also update wrong values to something that works, but that
>   is not the intention here as far as I understand it. So the validation
>   step can also be done when the request is queued and, if it fails, it will
>   just return an error.
>
> - If due to performance reasons we will have to allocate/queue/reinit multiple
>   requests with a single ioctl, then we will have to add new ioctls to the
>   media device. At this moment in time it is not clear that this is really
>   needed and it certainly isn't needed for the stateless codec support that
>   we are looking at now.
>
> - The behavior of VIDIOC_G_EXT_CTRLS with which == V4L2_CTRL_WHICH_CUR_VAL
>   and VIDIOC_G_CTRL remains the same (i.e. it returns the current driver/HW
>   values). However, when combined with requests the documentation should make
>   clear that this returns a snapshot only and is racy w.r.t. applying values
>   from a request.
>
> - There is a discussion whether there should be a VIDIOC_REQUEST_ALLOC ioctl
>   for V4L2 in addition to the media ioctl. The reason is that stateless codecs
>   do not need the media controller except for allocating requests. So a V4L2
>   ioctl would avoid applications from having to deal with a media device.
>   This would also add additional hassle w.r.t. SELinux as I understand it.
>
>   Support for this can be added for now as a final patch in the Request API
>   patch series and we'll postpone the decision on this.

FWIW:

Acked-by: Tomasz Figa <tfiga@chromium.org>

Best regards,
Tomasz

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [RFCv2] Request API
  2018-03-23  8:46 [RFCv2] Request API Hans Verkuil
  2018-03-23 15:23 ` Tomasz Figa
@ 2018-03-23 23:06 ` Sakari Ailus
  1 sibling, 0 replies; 3+ messages in thread
From: Sakari Ailus @ 2018-03-23 23:06 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Linux Media Mailing List, Alexandre Courbot, Tomasz Figa, Pawel Osciak

Hi Hans,

Thanks for writing this. I generally agree with the RFC to the level of
detail available here; a few comments below.

On Fri, Mar 23, 2018 at 09:46:00AM +0100, Hans Verkuil wrote:
> RFC Request API, version 2
> --------------------------
> 
> This document proposes the public API for handling requests.
> 
> There has been some confusion about how to do this, so this summarizes the
> current approach based on conversations with the various stakeholders today
> (Sakari, Alexandre Courbot, Thomasz Figa and myself).
> 
> 1) Additions to the media API
> 
>    Allocate an empty request object:
> 
>    #define MEDIA_IOC_REQUEST_ALLOC _IOW('|', 0x05, __s32 *)
> 
>    This will return a file descriptor representing the request or an error
>    if it can't allocate the request.
> 
>    If the pointer argument is NULL, then this will just return 0 (if this ioctl
>    is implemented) or -ENOTTY otherwise. This can be used to test whether this
>    ioctl is supported or not without actually having to allocate a request.
> 
> 2) Operations on the request fd
> 
>    You can queue or reinit a request by calling these ioctls on the request fd:
> 
>    #define MEDIA_REQUEST_IOC_QUEUE   _IO('|',  128)
>    #define MEDIA_REQUEST_IOC_REINIT  _IO('|',  129)
> 
>    With REINIT you reset the state of the request as if you had just allocated
>    it.
> 
>    You can poll the request fd to wait for it to complete.

I'm not sure I like having to release a reference (fd) to a request to know
whether or not it succeeded. I put this as an open question on my RFC
patchset.

> 
>    To free a request you close the request fd. Note that it may still be in
>    use internally, so the internal datastructures have to be refcounted.
> 
>    For this initial implementation only buffers and controls are contained
>    in a request. This is needed to implement stateless codecs. Supporting
>    complex camera pipelines will require more work.
> 
>    Requests only contain the changes to the state at request queue time
>    relative to the previously queued request(s) or the current hardware state
>    if no other requests were queued.
> 
>    Once a request is completed it will retain the state at completion
>    time.
> 
> 3) To associate a v4l2 buffer with a request the 'reserved' field in struct
>    v4l2_buffer is used to store the request fd. Buffers won't be 'prepared'
>    until the request is queued since the request may contain information that
>    is needed to prepare the buffer.
> 
>    To indicate that request_fd should be used this flag should be set by
>    userspace at QBUF time:
> 
> #define V4L2_BUF_FLAG_REQUEST			0x00800000
> 
>    This flag will also be returned by the driver to indicate that the buffer
>    is associated with a request.
> 
>    TBD: what should vb2 return as request_fd value if this flag is set?
>    This should act the same as the fence patch series and this is still
>    being tweaked so let's wait for that to be merged first, then we can
>    finalize this.
> 
> 4) To associate v4l2 controls with a request we take the first of the
>    'reserved[2]' array elements in struct v4l2_ext_controls and use it to store
>    the request fd.
> 
>    We also add a new WHICH value:
> 
> #define V4L2_CTRL_WHICH_REQUEST   0x0f010000
> 
>    This tells the control framework to get/set controls from the given
>    request fd.
> 
>    When querying a control value from a request it will return the newest
>    value in the list of pending requests, or the current hardware value if
>    is not set in any of the pending requests.
> 
>    When a request is completed the controls will no longer change. A copy
>    will be made of volatile controls at the time of completion (actually
>    it will be up to the driver to decide when to do that).
> 
>    Volatile controls and requests:
> 
>    - If you set a volatile control in a request, then that will be ignored,
>      unless the V4L2_CTRL_FLAG_EXECUTE_ON_WRITE flag is set as well.
> 
>    - If you get a volatile control from a request then:
>      1) If the request is completed it will return the value of the volatile
>         control at completion time.
>      2) Otherwise: if the V4L2_CTRL_FLAG_EXECUTE_ON_WRITE is set and it was
>         set in a request, then that value is returned.
>      3) Otherwise: return the current value from the hardware (i.e. normal
>         behavior).
> 
>    Read-only controls and requests:
> 
>    - If you get a read-only control from a request then:
>      1) If the request is completed it will return the value of the read-only
>         control at completion time.
>      2) Otherwise it will get the current value from the driver (i.e. normal
>         behavior).
> 
>    Open issue: should we receive control events if a control in a request is
>    added/changed? Currently there are no plans to support control events for
>    requests. I don't see a clear use-case and neither do I see an easy way
>    of implementing this (which fd would receive those events?).
> 
> Notes:
> 
> - Earlier versions of this API had a TRY command as well to validate the
>   request. I'm not sure that is useful so I dropped it, but it can easily

I'd think this would be mostly useful on complex pipelines where you'll
have interdependencies in configuration across entities. One such case is
the available V4L2 formats given a choice of a media bus format towards the
DMA engine: you can't use the current APIs to try that (dirty one-off fix
could be to provide the mbus code on the same IOCTL) but there are more
complex cases, too.

Currently this is a niche use case, but moving all IOCTLs to the media
device, depending a bit on how that would be done, is what I'd expect to
change this completely.

Because this is not very relevant right now, I left it out.

>   be added if there is a good use-case for it. Traditionally within V4L the
>   TRY ioctl will also update wrong values to something that works, but that
>   is not the intention here as far as I understand it. So the validation
>   step can also be done when the request is queued and, if it fails, it will
>   just return an error.
> 
> - If due to performance reasons we will have to allocate/queue/reinit multiple
>   requests with a single ioctl, then we will have to add new ioctls to the
>   media device. At this moment in time it is not clear that this is really
>   needed and it certainly isn't needed for the stateless codec support that
>   we are looking at now.

I think the time for this is later indeed, likely much later.

> 
> - The behavior of VIDIOC_G_EXT_CTRLS with which == V4L2_CTRL_WHICH_CUR_VAL
>   and VIDIOC_G_CTRL remains the same (i.e. it returns the current driver/HW
>   values). However, when combined with requests the documentation should make
>   clear that this returns a snapshot only and is racy w.r.t. applying values
>   from a request.
> 
> - There is a discussion whether there should be a VIDIOC_REQUEST_ALLOC ioctl
>   for V4L2 in addition to the media ioctl. The reason is that stateless codecs
>   do not need the media controller except for allocating requests. So a V4L2
>   ioctl would avoid applications from having to deal with a media device.
>   This would also add additional hassle w.r.t. SELinux as I understand it.
> 
>   Support for this can be added for now as a final patch in the Request API
>   patch series and we'll postpone the decision on this.

Works for me.

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ailus@iki.fi

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2018-03-23 23:06 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-23  8:46 [RFCv2] Request API Hans Verkuil
2018-03-23 15:23 ` Tomasz Figa
2018-03-23 23:06 ` Sakari Ailus

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.