linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC 00/22] Media request API
@ 2016-05-06 10:53 Sakari Ailus
  2016-05-06 10:53 ` [RFC 01/22] media: Add " Sakari Ailus
                   ` (23 more replies)
  0 siblings, 24 replies; 28+ messages in thread
From: Sakari Ailus @ 2016-05-06 10:53 UTC (permalink / raw)
  To: linux-media; +Cc: laurent.pinchart, hverkuil, mchehab

Hello all,

Here's a set of patches to implement support for the request API that has
been discussed many, many times over the past few years. Some of the
patches are my own while some have been written by Laurent and Hans.

I've made a few changes to these patches and added more, mainly:

- Implement request state in the framework in order to prevent performing
  invalid actions on requests, e.g. deleting a queued request or queueing
  a request which is already queued.

- Use 32 bits for the request ID. As requests are global, a 16-bit space
  could become a limitation. The IDs are reserved using ida (as in
  linux/idr.h) instead of a counter. This way we can ensure that once the
  counter wraps around, we will continue to allocate unique IDs for new
  requests. 

- Add media events to signal applications on completed events. Only the
  file handle which queued the request is notified. I've chosen this
  instead of a request specific means to dequeue events as there isn't
  really anything to obtain other than the information that the request
  has been completed: there's no data related to it, no buffers as in
  V4L2. Some applications may not be interested in receiving such events
  so the events are optional, specific to the queued request.

- Add poll support to tell the user there are dequeueable events.

My own use case for this is a little bit more limited than for some
others, i.e. I only need to bind video buffers to requests. That is
currently functional with these patches.

My open questions and to-do items I'm aware of:

- Global ID space vs. file handles. Should requests be referred to by
  global IDs or file handles specific to a process? V4L2 uses IDs but V4L2
  devices (other than m2m) can meaningfully be used only by a single
  program (or co-operative programs) at a time whereas the media device
  could conceivably be accessed and used for different purposes by
  multiple programs at the same time as the resources accessible through
  the media device are numerous and often independent of each other. A
  badly behaving application could disturb other applications using the
  same media device even if no resource conflict exist by accessing the
  same request IDs than other applications.

- Request completion and deletion. Should completed requests be deleted
  automatically, or should the request return to an "empty" state after
  completion? Applications typically have to create a new request right
  after a completion of an earlier one, and sparing one additional IOCTL
  call would be nice. (In current implementation the requests are deleted
  in completion, but this would be a trivial change.)

- Video buffers vs. requests. In the current implementation, I'm using
  VIDIOC_QBUF() from user space to associate buffers with requests. This
  is convenient in drivers since the only extra steps to support requests
  (vs. operation without requests) is to find the request and not really
  queueing the buffer yet. What's noteworthy as well that the VB2 buffer
  is in correct state after this when the request is queued.

- Subsystem framework specific request information. The requests are about
  the media device, and struct media_device_request is free of e.g. V4L2
  related information. Adding a pointer to V4L2 related data would make it
  easier to add request handling related functionality to the V4L2
  framework.

- MEDIA_IOC_REQUEST_CMD + (ALLOC/DELETE/QUEUE/APPLY) vs.
  MEDIA_IOC_REQUEST_(ALLOC/DELETE/QUEUE/APPLY). Should we continue to have
  a single IOCTL on the media device for requests, or should each
  operation on a request have a distinct IOCTL? The current implementation
  has a single IOCTL.

- VB2 queues are quite self-sufficient at the moment. Starting start in a
  queue requires at least one queued buffer whereas a stream containing
  multiple queues using requests could start e.g. by having a single
  buffer in a request for three streaming queues. The functionality would
  need to be relaxed to properly support requests.

- Limit number of allocated requests and dequeueable events to prevent
  unintentional or intentional system memory exhaustion.

The patchset is dependent on two patchsets (Media device IOCTL handling
refactoring and Media device file handle support) I've posted lately:

<URL:http://www.spinics.net/lists/linux-media/msg100194.html>
<URL:http://www.spinics.net/lists/linux-media/msg100188.html>

The code can be found here in the branch called "request":

<URL:http://git.retiisi.org.uk/?p=~sailus/linux.git;a=summary>

I intend to provide a test program for testing requests in the near
future.

Questions and comments are the most welcome!

-- 
Kind regards,
Sakari


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

* [RFC 01/22] media: Add request API
  2016-05-06 10:53 [RFC 00/22] Media request API Sakari Ailus
@ 2016-05-06 10:53 ` Sakari Ailus
  2016-05-06 10:53 ` [RFC 02/22] media: Add media device request state Sakari Ailus
                   ` (22 subsequent siblings)
  23 siblings, 0 replies; 28+ messages in thread
From: Sakari Ailus @ 2016-05-06 10:53 UTC (permalink / raw)
  To: linux-media; +Cc: laurent.pinchart, hverkuil, mchehab, Laurent Pinchart

From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

The request API allows bundling media device parameters with request
objects and applying them atomically, either synchronously or
asynchronously.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

---

Changes since v0:

- Make the request ID 32 bits wide internally

---

- Strip off the reserved fields

- Use __attribute__ ((packed)) for the IOCTL argument struct.

- Manage request ID space using ida_simple

- Fix compat handling for requests

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/media-device.c | 194 ++++++++++++++++++++++++++++++++++++++++++-
 include/media/media-device.h |  41 +++++++++
 include/uapi/linux/media.h   |  11 +++
 3 files changed, 243 insertions(+), 3 deletions(-)

diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index 3b9a1bf..4fc1eb1 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -41,6 +41,7 @@
 
 struct media_device_fh {
 	struct media_devnode_fh fh;
+	struct list_head requests;
 };
 
 static inline struct media_device_fh *media_device_fh(struct file *filp)
@@ -49,6 +50,170 @@ static inline struct media_device_fh *media_device_fh(struct file *filp)
 }
 
 /* -----------------------------------------------------------------------------
+ * Requests
+ */
+
+/**
+ * media_device_request_find - Find a request based from its ID
+ * @mdev: The media device
+ * @reqid: The request ID
+ *
+ * Find and return the request associated with the given ID, or NULL if no such
+ * request exists.
+ *
+ * When the function returns a non-NULL request it increases its reference
+ * count. The caller is responsible for releasing the reference by calling
+ * media_device_request_put() on the request.
+ */
+struct media_device_request *
+media_device_request_find(struct media_device *mdev, u16 reqid)
+{
+	struct media_device_request *req;
+	unsigned long flags;
+	bool found = false;
+
+	spin_lock_irqsave(&mdev->req_lock, flags);
+	list_for_each_entry(req, &mdev->requests, list) {
+		if (req->id == reqid) {
+			kref_get(&req->kref);
+			found = true;
+			break;
+		}
+	}
+	spin_unlock_irqrestore(&mdev->req_lock, flags);
+
+	return found ? req : NULL;
+}
+EXPORT_SYMBOL_GPL(media_device_request_find);
+
+void media_device_request_get(struct media_device_request *req)
+{
+	kref_get(&req->kref);
+}
+EXPORT_SYMBOL_GPL(media_device_request_get);
+
+static void media_device_request_release(struct kref *kref)
+{
+	struct media_device_request *req =
+		container_of(kref, struct media_device_request, kref);
+	struct media_device *mdev = req->mdev;
+
+	ida_simple_remove(&mdev->req_ids, req->id);
+
+	mdev->ops->req_free(mdev, req);
+}
+
+void media_device_request_put(struct media_device_request *req)
+{
+	kref_put(&req->kref, media_device_request_release);
+}
+EXPORT_SYMBOL_GPL(media_device_request_put);
+
+static int media_device_request_alloc(struct media_device *mdev,
+				      struct media_device_fh *fh,
+				      struct media_request_cmd *cmd)
+{
+	struct media_device_request *req;
+	unsigned long flags;
+	int id = ida_simple_get(&mdev->req_ids, 1, 0, GFP_KERNEL);
+	int ret;
+
+	if (id < 0)
+		return id;
+
+	req = mdev->ops->req_alloc(mdev);
+	if (!req) {
+		ret = -ENOMEM;
+		goto out_ida_simple_remove;
+	}
+
+	req->mdev = mdev;
+	req->id = id;
+	kref_init(&req->kref);
+
+	spin_lock_irqsave(&mdev->req_lock, flags);
+	list_add_tail(&req->list, &mdev->requests);
+	list_add_tail(&req->fh_list, &fh->requests);
+	spin_unlock_irqrestore(&mdev->req_lock, flags);
+
+	cmd->request = req->id;
+
+	return 0;
+
+out_ida_simple_remove:
+	ida_simple_remove(&mdev->req_ids, id);
+
+	return ret;
+}
+
+static void media_device_request_delete(struct media_device *mdev,
+					struct media_device_request *req)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&mdev->req_lock, flags);
+	list_del(&req->list);
+	spin_unlock_irqrestore(&mdev->req_lock, flags);
+
+	media_device_request_put(req);
+}
+
+static long media_device_request_cmd(struct media_device *mdev,
+				     struct file *filp,
+				     struct media_request_cmd *cmd)
+{
+	struct media_device_fh *fh = media_device_fh(filp);
+	struct media_device_request *req = NULL;
+	int ret;
+
+	if (!mdev->ops || !mdev->ops->req_alloc || !mdev->ops->req_free)
+		return -ENOTTY;
+
+	if (cmd->cmd != MEDIA_REQ_CMD_ALLOC) {
+		req = media_device_request_find(mdev, cmd->request);
+		if (!req)
+			return -EINVAL;
+	}
+
+	switch (cmd->cmd) {
+	case MEDIA_REQ_CMD_ALLOC:
+		ret = media_device_request_alloc(mdev, fh, cmd);
+		break;
+
+	case MEDIA_REQ_CMD_DELETE:
+		media_device_request_delete(mdev, req);
+		ret = 0;
+		break;
+
+	case MEDIA_REQ_CMD_APPLY:
+		if (!mdev->ops->req_apply)
+			return -ENOSYS;
+
+		ret = mdev->ops->req_apply(mdev, req);
+		break;
+
+	case MEDIA_REQ_CMD_QUEUE:
+		if (!mdev->ops->req_queue)
+			return -ENOSYS;
+
+		ret = mdev->ops->req_queue(mdev, req);
+		break;
+
+	default:
+		ret = -EINVAL;
+		break;
+	}
+
+	if (req)
+		media_device_request_put(req);
+
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+/* -----------------------------------------------------------------------------
  * Userspace API
  */
 
@@ -65,6 +230,7 @@ static int media_device_open(struct file *filp)
 	if (!fh)
 		return -ENOMEM;
 
+	INIT_LIST_HEAD(&fh->requests);
 	filp->private_data = &fh->fh;
 
 	return 0;
@@ -73,6 +239,15 @@ static int media_device_open(struct file *filp)
 static int media_device_close(struct file *filp)
 {
 	struct media_device_fh *fh = media_device_fh(filp);
+	struct media_device *mdev = to_media_device(fh->fh.devnode);
+	struct media_device_request *req, *next;
+
+	spin_lock_irq(&mdev->req_lock);
+	list_for_each_entry_safe(req, next, &fh->requests, fh_list) {
+		list_del(&req->fh_list);
+		media_device_request_put(req);
+	}
+	spin_unlock_irq(&mdev->req_lock);
 
 	kfree(fh);
 
@@ -80,6 +255,7 @@ static int media_device_close(struct file *filp)
 }
 
 static int media_device_get_info(struct media_device *dev,
+				 struct file *filp,
 				 struct media_device_info *info)
 {
 	memset(info, 0, sizeof(*info));
@@ -119,6 +295,7 @@ static struct media_entity *find_entity(struct media_device *mdev, u32 id)
 }
 
 static long media_device_enum_entities(struct media_device *mdev,
+				       struct file *filp,
 				       struct media_entity_desc *entd)
 {
 	struct media_entity *ent;
@@ -172,6 +349,7 @@ static void media_device_kpad_to_upad(const struct media_pad *kpad,
 }
 
 static long media_device_enum_links(struct media_device *mdev,
+				    struct file *filp,
 				    struct media_links_enum *links)
 {
 	struct media_entity *entity;
@@ -220,6 +398,7 @@ static long media_device_enum_links(struct media_device *mdev,
 }
 
 static long media_device_setup_link(struct media_device *mdev,
+				    struct file *filp,
 				    struct media_link_desc *linkd)
 {
 	struct media_link *link = NULL;
@@ -248,6 +427,7 @@ static long media_device_setup_link(struct media_device *mdev,
 }
 
 static long media_device_get_topology(struct media_device *mdev,
+				      struct file *filp,
 				      struct media_v2_topology *topo)
 {
 	struct media_entity *entity;
@@ -417,7 +597,8 @@ static long copy_arg_to_user_nop(void __user *uarg, void *karg,
 #define MEDIA_IOC_SZ_ARG(__cmd, func, fl, alt_sz, from_user, to_user)	\
 	[_IOC_NR(MEDIA_IOC_##__cmd)] = {				\
 		.cmd = MEDIA_IOC_##__cmd,				\
-		.fn = (long (*)(struct media_device *, void *))func,	\
+		.fn = (long (*)(struct media_device *,			\
+				struct file *, void *))func,		\
 		.flags = fl,						\
 		.alt_arg_sizes = alt_sz,				\
 		.arg_from_user = from_user,				\
@@ -438,7 +619,7 @@ static long copy_arg_to_user_nop(void __user *uarg, void *karg,
 /* the table is indexed by _IOC_NR(cmd) */
 struct media_ioctl_info {
 	unsigned int cmd;
-	long (*fn)(struct media_device *dev, void *arg);
+	long (*fn)(struct media_device *dev, struct file *file, void *arg);
 	unsigned short flags;
 	const unsigned short *alt_arg_sizes;
 	long (*arg_from_user)(void *karg, void __user *uarg, unsigned int cmd);
@@ -511,7 +692,7 @@ static long __media_device_ioctl(
 	if (info->flags & MEDIA_IOC_FL_GRAPH_MUTEX)
 		mutex_lock(&dev->graph_mutex);
 
-	ret = info->fn(dev, karg);
+	ret = info->fn(dev, filp, karg);
 
 	if (info->flags & MEDIA_IOC_FL_GRAPH_MUTEX)
 		mutex_unlock(&dev->graph_mutex);
@@ -531,6 +712,7 @@ static const struct media_ioctl_info ioctl_info[] = {
 	MEDIA_IOC(ENUM_LINKS, media_device_enum_links, MEDIA_IOC_FL_GRAPH_MUTEX),
 	MEDIA_IOC(SETUP_LINK, media_device_setup_link, MEDIA_IOC_FL_GRAPH_MUTEX),
 	MEDIA_IOC(G_TOPOLOGY, media_device_get_topology, MEDIA_IOC_FL_GRAPH_MUTEX),
+	MEDIA_IOC(REQUEST_CMD, media_device_request_cmd, 0),
 };
 
 static long media_device_ioctl(struct file *filp, unsigned int cmd,
@@ -578,6 +760,7 @@ static const struct media_ioctl_info compat_ioctl_info[] = {
 	MEDIA_IOC_ARG(ENUM_LINKS32, media_device_enum_links, MEDIA_IOC_FL_GRAPH_MUTEX, from_user_enum_links32, copy_arg_to_user_nop),
 	MEDIA_IOC(SETUP_LINK, media_device_setup_link, MEDIA_IOC_FL_GRAPH_MUTEX),
 	MEDIA_IOC(G_TOPOLOGY, media_device_get_topology, MEDIA_IOC_FL_GRAPH_MUTEX),
+	MEDIA_IOC(REQUEST_CMD, media_device_request_cmd, 0),
 };
 
 static long media_device_compat_ioctl(struct file *filp, unsigned int cmd,
@@ -762,6 +945,7 @@ void media_device_init(struct media_device *mdev)
 	INIT_LIST_HEAD(&mdev->entity_notify);
 	mutex_init(&mdev->graph_mutex);
 	ida_init(&mdev->entity_internal_idx);
+	ida_init(&mdev->req_ids);
 
 	dev_dbg(mdev->dev, "Media device initialized\n");
 }
@@ -769,6 +953,7 @@ EXPORT_SYMBOL_GPL(media_device_init);
 
 void media_device_cleanup(struct media_device *mdev)
 {
+	ida_destroy(&mdev->req_ids);
 	ida_destroy(&mdev->entity_internal_idx);
 	mdev->entity_internal_idx_max = 0;
 	media_entity_graph_walk_cleanup(&mdev->pm_count_walk);
@@ -781,6 +966,9 @@ int __must_check __media_device_register(struct media_device *mdev,
 {
 	int ret;
 
+	spin_lock_init(&mdev->req_lock);
+	INIT_LIST_HEAD(&mdev->requests);
+
 	/* Register the device node. */
 	mdev->devnode.fops = &media_device_fops;
 	mdev->devnode.parent = mdev->dev;
diff --git a/include/media/media-device.h b/include/media/media-device.h
index 19c8ed4..49c3367a 100644
--- a/include/media/media-device.h
+++ b/include/media/media-device.h
@@ -23,6 +23,7 @@
 #ifndef _MEDIA_DEVICE_H
 #define _MEDIA_DEVICE_H
 
+#include <linux/kref.h>
 #include <linux/list.h>
 #include <linux/mutex.h>
 
@@ -262,6 +263,23 @@
 
 struct ida;
 struct device;
+struct media_device;
+
+/**
+ * struct media_device_request - Media device request
+ * @id: Request ID
+ * @mdev: Media device this request belongs to
+ * @kref: Reference count
+ * @list: List entry in the media device requests list
+ * @fh_list: List entry in the media file handle requests list
+ */
+struct media_device_request {
+	u32 id;
+	struct media_device *mdev;
+	struct kref kref;
+	struct list_head list;
+	struct list_head fh_list;
+};
 
 /**
  * struct media_entity_notify - Media Entity Notify
@@ -283,10 +301,21 @@ struct media_entity_notify {
  * struct media_device_ops - Media device operations
  * @link_notify: Link state change notification callback. This callback is
  *		 called with the graph_mutex held.
+ * @req_alloc: Allocate a request
+ * @req_free: Free a request
+ * @req_apply: Apply a request
+ * @req_queue: Queue a request
  */
 struct media_device_ops {
 	int (*link_notify)(struct media_link *link, u32 flags,
 			   unsigned int notification);
+	struct media_device_request *(*req_alloc)(struct media_device *mdev);
+	void (*req_free)(struct media_device *mdev,
+			 struct media_device_request *req);
+	int (*req_apply)(struct media_device *mdev,
+			 struct media_device_request *req);
+	int (*req_queue)(struct media_device *mdev,
+			 struct media_device_request *req);
 };
 
 /**
@@ -322,6 +351,9 @@ struct media_device_ops {
  * @disable_source: Disable Source Handler function pointer
  *
  * @ops:	Operation handler callbacks
+ * @req_ids:	Allocated request IDs
+ * @req_lock:	Serialise access to requests list
+ * @requests:	List of allocated requests
  *
  * This structure represents an abstract high-level media device. It allows easy
  * access to entities and provides basic media device-level support. The
@@ -389,6 +421,10 @@ struct media_device {
 	void (*disable_source)(struct media_entity *entity);
 
 	const struct media_device_ops *ops;
+
+	struct ida req_ids;
+	spinlock_t req_lock;
+	struct list_head requests;
 };
 
 /* We don't need to include pci.h or usb.h here */
@@ -715,4 +751,9 @@ static inline void __media_device_usb_init(struct media_device *mdev,
 #define media_device_usb_init(mdev, udev, name) \
 	__media_device_usb_init(mdev, udev, name, KBUILD_MODNAME)
 
+struct media_device_request *
+media_device_request_find(struct media_device *mdev, u16 reqid);
+void media_device_request_get(struct media_device_request *req);
+void media_device_request_put(struct media_device_request *req);
+
 #endif
diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
index df59ede..e8922ea 100644
--- a/include/uapi/linux/media.h
+++ b/include/uapi/linux/media.h
@@ -389,10 +389,21 @@ struct media_v2_topology {
 
 /* ioctls */
 
+#define MEDIA_REQ_CMD_ALLOC		0
+#define MEDIA_REQ_CMD_DELETE		1
+#define MEDIA_REQ_CMD_APPLY		2
+#define MEDIA_REQ_CMD_QUEUE		3
+
+struct __attribute__ ((packed)) media_request_cmd {
+	__u32 cmd;
+	__u32 request;
+};
+
 #define MEDIA_IOC_DEVICE_INFO		_IOWR('|', 0x00, struct media_device_info)
 #define MEDIA_IOC_ENUM_ENTITIES		_IOWR('|', 0x01, struct media_entity_desc)
 #define MEDIA_IOC_ENUM_LINKS		_IOWR('|', 0x02, struct media_links_enum)
 #define MEDIA_IOC_SETUP_LINK		_IOWR('|', 0x03, struct media_link_desc)
 #define MEDIA_IOC_G_TOPOLOGY		_IOWR('|', 0x04, struct media_v2_topology)
+#define MEDIA_IOC_REQUEST_CMD		_IOWR('|', 0x05, struct media_request_cmd)
 
 #endif /* __LINUX_MEDIA_H */
-- 
1.9.1


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

* [RFC 02/22] media: Add media device request state
  2016-05-06 10:53 [RFC 00/22] Media request API Sakari Ailus
  2016-05-06 10:53 ` [RFC 01/22] media: Add " Sakari Ailus
@ 2016-05-06 10:53 ` Sakari Ailus
  2016-05-06 10:53 ` [RFC 03/22] media: Prevent queueing queued requests Sakari Ailus
                   ` (21 subsequent siblings)
  23 siblings, 0 replies; 28+ messages in thread
From: Sakari Ailus @ 2016-05-06 10:53 UTC (permalink / raw)
  To: linux-media; +Cc: laurent.pinchart, hverkuil, mchehab

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 include/media/media-device.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/include/media/media-device.h b/include/media/media-device.h
index 49c3367a..acb2481 100644
--- a/include/media/media-device.h
+++ b/include/media/media-device.h
@@ -265,6 +265,11 @@ struct ida;
 struct device;
 struct media_device;
 
+enum media_device_request_state {
+	MEDIA_DEVICE_REQUEST_STATE_IDLE,
+	MEDIA_DEVICE_REQUEST_STATE_QUEUED,
+};
+
 /**
  * struct media_device_request - Media device request
  * @id: Request ID
@@ -272,6 +277,8 @@ struct media_device;
  * @kref: Reference count
  * @list: List entry in the media device requests list
  * @fh_list: List entry in the media file handle requests list
+ * @state: The state of the request, MEDIA_DEVICE_REQUEST_STATE_*,
+ *	   access to state serialised by mdev->req_lock
  */
 struct media_device_request {
 	u32 id;
@@ -279,6 +286,7 @@ struct media_device_request {
 	struct kref kref;
 	struct list_head list;
 	struct list_head fh_list;
+	enum media_device_request_state state;
 };
 
 /**
-- 
1.9.1


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

* [RFC 03/22] media: Prevent queueing queued requests
  2016-05-06 10:53 [RFC 00/22] Media request API Sakari Ailus
  2016-05-06 10:53 ` [RFC 01/22] media: Add " Sakari Ailus
  2016-05-06 10:53 ` [RFC 02/22] media: Add media device request state Sakari Ailus
@ 2016-05-06 10:53 ` Sakari Ailus
  2016-05-06 10:53 ` [RFC 04/22] media: Only requests in IDLE state may be deleted Sakari Ailus
                   ` (20 subsequent siblings)
  23 siblings, 0 replies; 28+ messages in thread
From: Sakari Ailus @ 2016-05-06 10:53 UTC (permalink / raw)
  To: linux-media; +Cc: laurent.pinchart, hverkuil, mchehab

Verify that the request state is IDLE before queueing it. Also mark
requests queued when they're queued, and return the request to IDLE if
queueing it failed.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/media-device.c | 43 +++++++++++++++++++++++++++++++++++--------
 1 file changed, 35 insertions(+), 8 deletions(-)

diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index 4fc1eb1..481e8e4 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -158,6 +158,37 @@ static void media_device_request_delete(struct media_device *mdev,
 	media_device_request_put(req);
 }
 
+static int media_device_request_queue_apply(
+	struct media_device *mdev, struct media_device_request *req,
+	int (*fn)(struct media_device *mdev,
+		  struct media_device_request *req))
+{
+	unsigned long flags;
+	int rval = 0;
+
+	if (!fn)
+		return -ENOSYS;
+
+	spin_lock_irqsave(&mdev->req_lock, flags);
+	if (req->state != MEDIA_DEVICE_REQUEST_STATE_IDLE)
+		rval = -EINVAL;
+	else
+		req->state = MEDIA_DEVICE_REQUEST_STATE_QUEUED;
+	spin_unlock_irqrestore(&mdev->req_lock, flags);
+
+	if (rval)
+		return rval;
+
+	rval = fn(mdev, req);
+	if (rval) {
+		spin_lock_irqsave(&mdev->req_lock, flags);
+		req->state = MEDIA_DEVICE_REQUEST_STATE_IDLE;
+		spin_unlock_irqrestore(&mdev->req_lock, flags);
+	}
+
+	return rval;
+}
+
 static long media_device_request_cmd(struct media_device *mdev,
 				     struct file *filp,
 				     struct media_request_cmd *cmd)
@@ -186,17 +217,13 @@ static long media_device_request_cmd(struct media_device *mdev,
 		break;
 
 	case MEDIA_REQ_CMD_APPLY:
-		if (!mdev->ops->req_apply)
-			return -ENOSYS;
-
-		ret = mdev->ops->req_apply(mdev, req);
+		ret = media_device_request_queue_apply(mdev, req,
+						       mdev->ops->req_apply);
 		break;
 
 	case MEDIA_REQ_CMD_QUEUE:
-		if (!mdev->ops->req_queue)
-			return -ENOSYS;
-
-		ret = mdev->ops->req_queue(mdev, req);
+		ret = media_device_request_queue_apply(mdev, req,
+						       mdev->ops->req_queue);
 		break;
 
 	default:
-- 
1.9.1


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

* [RFC 04/22] media: Only requests in IDLE state may be deleted
  2016-05-06 10:53 [RFC 00/22] Media request API Sakari Ailus
                   ` (2 preceding siblings ...)
  2016-05-06 10:53 ` [RFC 03/22] media: Prevent queueing queued requests Sakari Ailus
@ 2016-05-06 10:53 ` Sakari Ailus
  2016-05-06 10:53 ` [RFC 05/22] media: Add media_device_request_complete() to mark requests complete Sakari Ailus
                   ` (19 subsequent siblings)
  23 siblings, 0 replies; 28+ messages in thread
From: Sakari Ailus @ 2016-05-06 10:53 UTC (permalink / raw)
  To: linux-media; +Cc: laurent.pinchart, hverkuil, mchehab

Prevent deleting queued requests. Also mark deleted requests as such by
adding a new state for them.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/media-device.c | 14 ++++++++++++--
 include/media/media-device.h |  1 +
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index 481e8e4..5b7bfcf 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -195,6 +195,7 @@ static long media_device_request_cmd(struct media_device *mdev,
 {
 	struct media_device_fh *fh = media_device_fh(filp);
 	struct media_device_request *req = NULL;
+	unsigned long flags;
 	int ret;
 
 	if (!mdev->ops || !mdev->ops->req_alloc || !mdev->ops->req_free)
@@ -212,8 +213,17 @@ static long media_device_request_cmd(struct media_device *mdev,
 		break;
 
 	case MEDIA_REQ_CMD_DELETE:
-		media_device_request_delete(mdev, req);
-		ret = 0;
+		spin_lock_irqsave(&mdev->req_lock, flags);
+		if (req->state == MEDIA_DEVICE_REQUEST_STATE_IDLE) {
+			ret = 0;
+			req->state = MEDIA_DEVICE_REQUEST_STATE_DELETED;
+		} else {
+			ret = -EINVAL;
+		}
+		spin_unlock_irqrestore(&mdev->req_lock, flags);
+
+		if (!ret)
+			media_device_request_delete(mdev, req);
 		break;
 
 	case MEDIA_REQ_CMD_APPLY:
diff --git a/include/media/media-device.h b/include/media/media-device.h
index acb2481..3167c52 100644
--- a/include/media/media-device.h
+++ b/include/media/media-device.h
@@ -268,6 +268,7 @@ struct media_device;
 enum media_device_request_state {
 	MEDIA_DEVICE_REQUEST_STATE_IDLE,
 	MEDIA_DEVICE_REQUEST_STATE_QUEUED,
+	MEDIA_DEVICE_REQUEST_STATE_DELETED,
 };
 
 /**
-- 
1.9.1


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

* [RFC 05/22] media: Add media_device_request_complete() to mark requests complete
  2016-05-06 10:53 [RFC 00/22] Media request API Sakari Ailus
                   ` (3 preceding siblings ...)
  2016-05-06 10:53 ` [RFC 04/22] media: Only requests in IDLE state may be deleted Sakari Ailus
@ 2016-05-06 10:53 ` Sakari Ailus
  2016-05-06 10:53 ` [RFC 06/22] media: Add per-entity request data support Sakari Ailus
                   ` (18 subsequent siblings)
  23 siblings, 0 replies; 28+ messages in thread
From: Sakari Ailus @ 2016-05-06 10:53 UTC (permalink / raw)
  To: linux-media; +Cc: laurent.pinchart, hverkuil, mchehab

Once the request has been queued and later on completed, a driver will
mark the request complete by calling media_device_request_complete().

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/media-device.c | 7 +++++++
 include/media/media-device.h | 3 +++
 2 files changed, 10 insertions(+)

diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index 5b7bfcf..cbd3b8b 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -158,6 +158,13 @@ static void media_device_request_delete(struct media_device *mdev,
 	media_device_request_put(req);
 }
 
+void media_device_request_complete(struct media_device *mdev,
+				   struct media_device_request *req)
+{
+	media_device_request_delete(mdev, req);
+}
+EXPORT_SYMBOL_GPL(media_device_request_complete);
+
 static int media_device_request_queue_apply(
 	struct media_device *mdev, struct media_device_request *req,
 	int (*fn)(struct media_device *mdev,
diff --git a/include/media/media-device.h b/include/media/media-device.h
index 3167c52..d86fb8a 100644
--- a/include/media/media-device.h
+++ b/include/media/media-device.h
@@ -269,6 +269,7 @@ enum media_device_request_state {
 	MEDIA_DEVICE_REQUEST_STATE_IDLE,
 	MEDIA_DEVICE_REQUEST_STATE_QUEUED,
 	MEDIA_DEVICE_REQUEST_STATE_DELETED,
+	MEDIA_DEVICE_REQUEST_STATE_COMPLETE,
 };
 
 /**
@@ -764,5 +765,7 @@ struct media_device_request *
 media_device_request_find(struct media_device *mdev, u16 reqid);
 void media_device_request_get(struct media_device_request *req);
 void media_device_request_put(struct media_device_request *req);
+void media_device_request_complete(struct media_device *mdev,
+				   struct media_device_request *req);
 
 #endif
-- 
1.9.1


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

* [RFC 06/22] media: Add per-entity request data support
  2016-05-06 10:53 [RFC 00/22] Media request API Sakari Ailus
                   ` (4 preceding siblings ...)
  2016-05-06 10:53 ` [RFC 05/22] media: Add media_device_request_complete() to mark requests complete Sakari Ailus
@ 2016-05-06 10:53 ` Sakari Ailus
  2016-05-06 10:53 ` [RFC 07/22] videodev2.h: Add request field to v4l2_buffer Sakari Ailus
                   ` (17 subsequent siblings)
  23 siblings, 0 replies; 28+ messages in thread
From: Sakari Ailus @ 2016-05-06 10:53 UTC (permalink / raw)
  To: linux-media; +Cc: laurent.pinchart, hverkuil, mchehab, Laurent Pinchart

From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

Allow subsystems to associate data with entities in each request. This
will be used by the V4L2 subdev core to store pad formats in requests.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

---

Changes since v0:

- Dereference requests without holding the list lock
- Remove requests from global list when closing the fh
---
 drivers/media/media-device.c | 82 ++++++++++++++++++++++++++++++++++++++++++--
 include/media/media-device.h |  7 ++++
 include/media/media-entity.h | 12 +++++++
 3 files changed, 99 insertions(+), 2 deletions(-)

diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index cbd3b8b..10b9a4a 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -94,10 +94,16 @@ EXPORT_SYMBOL_GPL(media_device_request_get);
 
 static void media_device_request_release(struct kref *kref)
 {
+	struct media_entity_request_data *data, *next;
 	struct media_device_request *req =
 		container_of(kref, struct media_device_request, kref);
 	struct media_device *mdev = req->mdev;
 
+	list_for_each_entry_safe(data, next, &req->data, list) {
+		list_del(&data->list);
+		data->release(data);
+	}
+
 	ida_simple_remove(&mdev->req_ids, req->id);
 
 	mdev->ops->req_free(mdev, req);
@@ -109,6 +115,70 @@ void media_device_request_put(struct media_device_request *req)
 }
 EXPORT_SYMBOL_GPL(media_device_request_put);
 
+/**
+ * media_device_request_get_entity_data - Get per-entity data
+ * @req: The request
+ * @entity: The entity
+ *
+ * Search and return per-entity data (as a struct media_entity_request_data
+ * instance) associated with the given entity for the request, as previously
+ * registered by a call to media_device_request_set_entity_data().
+ *
+ * The caller is expected to hold a reference to the request. Per-entity data is
+ * not reference counted, the returned pointer will be valid only as long as the
+ * reference to the request is held.
+ *
+ * Return the data instance pointer or NULL if no data could be found.
+ */
+struct media_entity_request_data *
+media_device_request_get_entity_data(struct media_device_request *req,
+				     struct media_entity *entity)
+{
+	struct media_entity_request_data *data;
+	unsigned long flags;
+
+	spin_lock_irqsave(&req->mdev->req_lock, flags);
+
+	list_for_each_entry(data, &req->data, list) {
+		if (data->entity == entity)
+			goto done;
+	}
+
+	data = NULL;
+
+done:
+	spin_unlock_irqrestore(&req->mdev->req_lock, flags);
+	return data;
+}
+EXPORT_SYMBOL_GPL(media_device_request_get_entity_data);
+
+/**
+ * media_device_request_set_entity_data - Set per-entity data
+ * @req: The request
+ * @entity: The entity
+ * @data: The data
+ *
+ * Record the given per-entity data as being associated with the entity for the
+ * request.
+ *
+ * Only one per-entity data instance can be associated with a request. The
+ * caller is responsible for enforcing this requirement.
+ *
+ * Ownership of the per-entity data is transferred to the request when calling
+ * this function. The data will be freed automatically when the last reference
+ * to the request is released.
+ */
+void media_device_request_set_entity_data(struct media_device_request *req,
+	struct media_entity *entity, struct media_entity_request_data *data)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&req->mdev->req_lock, flags);
+	list_add_tail(&data->list, &req->data);
+	spin_unlock_irqrestore(&req->mdev->req_lock, flags);
+}
+EXPORT_SYMBOL_GPL(media_device_request_set_entity_data);
+
 static int media_device_request_alloc(struct media_device *mdev,
 				      struct media_device_fh *fh,
 				      struct media_request_cmd *cmd)
@@ -130,6 +200,7 @@ static int media_device_request_alloc(struct media_device *mdev,
 	req->mdev = mdev;
 	req->id = id;
 	kref_init(&req->kref);
+	INIT_LIST_HEAD(&req->data);
 
 	spin_lock_irqsave(&mdev->req_lock, flags);
 	list_add_tail(&req->list, &mdev->requests);
@@ -153,6 +224,7 @@ static void media_device_request_delete(struct media_device *mdev,
 
 	spin_lock_irqsave(&mdev->req_lock, flags);
 	list_del(&req->list);
+	list_del(&req->fh_list);
 	spin_unlock_irqrestore(&mdev->req_lock, flags);
 
 	media_device_request_put(req);
@@ -284,12 +356,18 @@ static int media_device_close(struct file *filp)
 {
 	struct media_device_fh *fh = media_device_fh(filp);
 	struct media_device *mdev = to_media_device(fh->fh.devnode);
-	struct media_device_request *req, *next;
 
 	spin_lock_irq(&mdev->req_lock);
-	list_for_each_entry_safe(req, next, &fh->requests, fh_list) {
+	while (!list_empty(&fh->requests)) {
+		struct media_device_request *req;
+
+		req = list_first_entry(&fh->requests, typeof(*req), fh_list);
+		list_del(&req->list);
 		list_del(&req->fh_list);
+
+		spin_unlock_irq(&mdev->req_lock);
 		media_device_request_put(req);
+		spin_lock_irq(&mdev->req_lock);
 	}
 	spin_unlock_irq(&mdev->req_lock);
 
diff --git a/include/media/media-device.h b/include/media/media-device.h
index d86fb8a..2082108 100644
--- a/include/media/media-device.h
+++ b/include/media/media-device.h
@@ -281,6 +281,7 @@ enum media_device_request_state {
  * @fh_list: List entry in the media file handle requests list
  * @state: The state of the request, MEDIA_DEVICE_REQUEST_STATE_*,
  *	   access to state serialised by mdev->req_lock
+ * @data: Per-entity data list
  */
 struct media_device_request {
 	u32 id;
@@ -289,6 +290,7 @@ struct media_device_request {
 	struct list_head list;
 	struct list_head fh_list;
 	enum media_device_request_state state;
+	struct list_head data;
 };
 
 /**
@@ -767,5 +769,10 @@ void media_device_request_get(struct media_device_request *req);
 void media_device_request_put(struct media_device_request *req);
 void media_device_request_complete(struct media_device *mdev,
 				   struct media_device_request *req);
+struct media_entity_request_data *
+media_device_request_get_entity_data(struct media_device_request *req,
+				     struct media_entity *entity);
+void media_device_request_set_entity_data(struct media_device_request *req,
+	struct media_entity *entity, struct media_entity_request_data *data);
 
 #endif
diff --git a/include/media/media-entity.h b/include/media/media-entity.h
index cbb266f..cdc2e6c 100644
--- a/include/media/media-entity.h
+++ b/include/media/media-entity.h
@@ -557,6 +557,18 @@ void media_gobj_create(struct media_device *mdev,
  */
 void media_gobj_destroy(struct media_gobj *gobj);
 
+/*
+ * struct media_entity_request_data - Per-entity request data
+ * @entity: Entity this data belongs to
+ * @release: Release operation to free the data
+ * @list: List entry in the media_device_request data list
+ */
+struct media_entity_request_data {
+	struct media_entity *entity;
+	void (*release)(struct media_entity_request_data *data);
+	struct list_head list;
+};
+
 /**
  * media_entity_pads_init() - Initialize the entity pads
  *
-- 
1.9.1


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

* [RFC 07/22] videodev2.h: Add request field to v4l2_buffer
  2016-05-06 10:53 [RFC 00/22] Media request API Sakari Ailus
                   ` (5 preceding siblings ...)
  2016-05-06 10:53 ` [RFC 06/22] media: Add per-entity request data support Sakari Ailus
@ 2016-05-06 10:53 ` Sakari Ailus
  2016-05-06 10:53 ` [RFC 08/22] videodev2.h: Add request field to v4l2_pix_format_mplane Sakari Ailus
                   ` (16 subsequent siblings)
  23 siblings, 0 replies; 28+ messages in thread
From: Sakari Ailus @ 2016-05-06 10:53 UTC (permalink / raw)
  To: linux-media; +Cc: laurent.pinchart, hverkuil, mchehab, Hans Verkuil

From: Hans Verkuil <hans.verkuil@cisco.com>

When queuing buffers allow for passing the request ID that
should be associated with this buffer. Split the u32 reserved2 field
into two u16 fields, one for request, one with the old reserved2 name.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>

Use full 32 bits for the request ID.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/usb/cpia2/cpia2_v4l.c           | 1 +
 drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 4 ++--
 drivers/media/v4l2-core/v4l2-ioctl.c          | 4 ++--
 drivers/media/v4l2-core/videobuf2-v4l2.c      | 3 ++-
 include/media/videobuf2-v4l2.h                | 2 ++
 include/uapi/linux/videodev2.h                | 3 ++-
 6 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/media/usb/cpia2/cpia2_v4l.c b/drivers/media/usb/cpia2/cpia2_v4l.c
index 9caea83..01c596a 100644
--- a/drivers/media/usb/cpia2/cpia2_v4l.c
+++ b/drivers/media/usb/cpia2/cpia2_v4l.c
@@ -952,6 +952,7 @@ static int cpia2_dqbuf(struct file *file, void *fh, struct v4l2_buffer *buf)
 	buf->sequence = cam->buffers[buf->index].seq;
 	buf->m.offset = cam->buffers[buf->index].data - cam->frame_buffer;
 	buf->length = cam->frame_size;
+	buf->request = 0;
 	buf->reserved2 = 0;
 	buf->reserved = 0;
 	memset(&buf->timecode, 0, sizeof(buf->timecode));
diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
index bacecbd..0315e0e 100644
--- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
+++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
@@ -348,7 +348,7 @@ struct v4l2_buffer32 {
 		__s32		fd;
 	} m;
 	__u32			length;
-	__u32			reserved2;
+	__u32			request;
 	__u32			reserved;
 };
 
@@ -509,7 +509,7 @@ static int put_v4l2_buffer32(struct v4l2_buffer *kp, struct v4l2_buffer32 __user
 		put_user(kp->timestamp.tv_usec, &up->timestamp.tv_usec) ||
 		copy_to_user(&up->timecode, &kp->timecode, sizeof(struct v4l2_timecode)) ||
 		put_user(kp->sequence, &up->sequence) ||
-		put_user(kp->reserved2, &up->reserved2) ||
+		put_user(kp->request, &up->request) ||
 		put_user(kp->reserved, &up->reserved) ||
 		put_user(kp->length, &up->length))
 			return -EFAULT;
diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
index 6bf5a3e..bf15580 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -443,13 +443,13 @@ static void v4l_print_buffer(const void *arg, bool write_only)
 	int i;
 
 	pr_cont("%02ld:%02d:%02d.%08ld index=%d, type=%s, "
-		"flags=0x%08x, field=%s, sequence=%d, memory=%s",
+		"request %u, flags=0x%08x, field=%s, sequence=%d, memory=%s",
 			p->timestamp.tv_sec / 3600,
 			(int)(p->timestamp.tv_sec / 60) % 60,
 			(int)(p->timestamp.tv_sec % 60),
 			(long)p->timestamp.tv_usec,
 			p->index,
-			prt_names(p->type, v4l2_type_names),
+			prt_names(p->type, v4l2_type_names), p->request,
 			p->flags, prt_names(p->field, v4l2_field_names),
 			p->sequence, prt_names(p->memory, v4l2_memory_names));
 
diff --git a/drivers/media/v4l2-core/videobuf2-v4l2.c b/drivers/media/v4l2-core/videobuf2-v4l2.c
index 91f5521..0721258 100644
--- a/drivers/media/v4l2-core/videobuf2-v4l2.c
+++ b/drivers/media/v4l2-core/videobuf2-v4l2.c
@@ -199,7 +199,7 @@ static void __fill_v4l2_buffer(struct vb2_buffer *vb, void *pb)
 	b->timestamp = ns_to_timeval(vb->timestamp);
 	b->timecode = vbuf->timecode;
 	b->sequence = vbuf->sequence;
-	b->reserved2 = 0;
+	b->request = vbuf->request;
 	b->reserved = 0;
 
 	if (q->is_multiplanar) {
@@ -317,6 +317,7 @@ static int __fill_vb2_buffer(struct vb2_buffer *vb,
 	}
 	vb->timestamp = 0;
 	vbuf->sequence = 0;
+	vbuf->request = b->request;
 
 	if (V4L2_TYPE_IS_MULTIPLANAR(b->type)) {
 		if (b->memory == VB2_MEMORY_USERPTR) {
diff --git a/include/media/videobuf2-v4l2.h b/include/media/videobuf2-v4l2.h
index 3cc836f..b1ee91c 100644
--- a/include/media/videobuf2-v4l2.h
+++ b/include/media/videobuf2-v4l2.h
@@ -30,6 +30,7 @@
  * @field:	enum v4l2_field; field order of the image in the buffer
  * @timecode:	frame timecode
  * @sequence:	sequence count of this frame
+ * @request:	request used by the buffer
  * Should contain enough information to be able to cover all the fields
  * of struct v4l2_buffer at videodev2.h
  */
@@ -40,6 +41,7 @@ struct vb2_v4l2_buffer {
 	__u32			field;
 	struct v4l2_timecode	timecode;
 	__u32			sequence;
+	__u32			request;
 };
 
 /*
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 8f95191..ac28299 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -846,6 +846,7 @@ struct v4l2_plane {
  * @length:	size in bytes of the buffer (NOT its payload) for single-plane
  *		buffers (when type != *_MPLANE); number of elements in the
  *		planes array for multi-plane buffers
+ * @request: this buffer should use this request
  *
  * Contains data exchanged by application and driver using one of the Streaming
  * I/O methods.
@@ -869,7 +870,7 @@ struct v4l2_buffer {
 		__s32		fd;
 	} m;
 	__u32			length;
-	__u32			reserved2;
+	__u32			request;
 	__u32			reserved;
 };
 
-- 
1.9.1


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

* [RFC 08/22] videodev2.h: Add request field to v4l2_pix_format_mplane
  2016-05-06 10:53 [RFC 00/22] Media request API Sakari Ailus
                   ` (6 preceding siblings ...)
  2016-05-06 10:53 ` [RFC 07/22] videodev2.h: Add request field to v4l2_buffer Sakari Ailus
@ 2016-05-06 10:53 ` Sakari Ailus
  2016-05-06 16:33   ` Nicolas Dufresne
  2016-05-06 10:53 ` [RFC 09/22] v4l2-subdev.h: Add request field to format and selection structures Sakari Ailus
                   ` (15 subsequent siblings)
  23 siblings, 1 reply; 28+ messages in thread
From: Sakari Ailus @ 2016-05-06 10:53 UTC (permalink / raw)
  To: linux-media; +Cc: laurent.pinchart, hverkuil, mchehab, Laurent Pinchart

From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

Let userspace specify a request ID when getting or setting formats. The
support is limited to the multi-planar API at the moment, extending it
to the single-planar API is possible if needed.

>From a userspace point of view the API change is also minimized and
doesn't require any new ioctl.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 include/uapi/linux/videodev2.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index ac28299..6260d0e 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -1972,6 +1972,7 @@ struct v4l2_plane_pix_format {
  * @ycbcr_enc:		enum v4l2_ycbcr_encoding, Y'CbCr encoding
  * @quantization:	enum v4l2_quantization, colorspace quantization
  * @xfer_func:		enum v4l2_xfer_func, colorspace transfer function
+ * @request:		request ID
  */
 struct v4l2_pix_format_mplane {
 	__u32				width;
@@ -1986,7 +1987,8 @@ struct v4l2_pix_format_mplane {
 	__u8				ycbcr_enc;
 	__u8				quantization;
 	__u8				xfer_func;
-	__u8				reserved[7];
+	__u8				reserved[3];
+	__u32				request;
 } __attribute__ ((packed));
 
 /**
-- 
1.9.1


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

* [RFC 09/22] v4l2-subdev.h: Add request field to format and selection structures
  2016-05-06 10:53 [RFC 00/22] Media request API Sakari Ailus
                   ` (7 preceding siblings ...)
  2016-05-06 10:53 ` [RFC 08/22] videodev2.h: Add request field to v4l2_pix_format_mplane Sakari Ailus
@ 2016-05-06 10:53 ` Sakari Ailus
  2016-05-06 10:53 ` [RFC 10/22] v4l: Support the request API in format operations Sakari Ailus
                   ` (14 subsequent siblings)
  23 siblings, 0 replies; 28+ messages in thread
From: Sakari Ailus @ 2016-05-06 10:53 UTC (permalink / raw)
  To: linux-media; +Cc: laurent.pinchart, hverkuil, mchehab, Laurent Pinchart

From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

Let userspace specify a request ID when getting or setting formats or
selection rectangles.

>From a userspace point of view the API change is minimized and doesn't
require any new ioctl.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 include/uapi/linux/v4l2-subdev.h | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/include/uapi/linux/v4l2-subdev.h b/include/uapi/linux/v4l2-subdev.h
index dbce2b554..dbb7c1d 100644
--- a/include/uapi/linux/v4l2-subdev.h
+++ b/include/uapi/linux/v4l2-subdev.h
@@ -32,10 +32,12 @@
  * enum v4l2_subdev_format_whence - Media bus format type
  * @V4L2_SUBDEV_FORMAT_TRY: try format, for negotiation only
  * @V4L2_SUBDEV_FORMAT_ACTIVE: active format, applied to the device
+ * @V4L2_SUBDEV_FORMAT_REQUEST: format stored in request
  */
 enum v4l2_subdev_format_whence {
 	V4L2_SUBDEV_FORMAT_TRY = 0,
 	V4L2_SUBDEV_FORMAT_ACTIVE = 1,
+	V4L2_SUBDEV_FORMAT_REQUEST = 2,
 };
 
 /**
@@ -43,12 +45,15 @@ enum v4l2_subdev_format_whence {
  * @which: format type (from enum v4l2_subdev_format_whence)
  * @pad: pad number, as reported by the media API
  * @format: media bus format (format code and frame size)
+ * @request: request ID (when which is set to V4L2_SUBDEV_FORMAT_REQUEST)
+ * @reserved: for future use, set to zero for now
  */
 struct v4l2_subdev_format {
 	__u32 which;
 	__u32 pad;
 	struct v4l2_mbus_framefmt format;
-	__u32 reserved[8];
+	__u32 request;
+	__u32 reserved[7];
 };
 
 /**
@@ -139,6 +144,7 @@ struct v4l2_subdev_frame_interval_enum {
  *	    defined in v4l2-common.h; V4L2_SEL_TGT_* .
  * @flags: constraint flags, defined in v4l2-common.h; V4L2_SEL_FLAG_*.
  * @r: coordinates of the selection window
+ * @request: request ID (when which is set to V4L2_SUBDEV_FORMAT_REQUEST)
  * @reserved: for future use, set to zero for now
  *
  * Hardware may use multiple helper windows to process a video stream.
@@ -151,7 +157,8 @@ struct v4l2_subdev_selection {
 	__u32 target;
 	__u32 flags;
 	struct v4l2_rect r;
-	__u32 reserved[8];
+	__u32 request;
+	__u32 reserved[7];
 };
 
 /* Backwards compatibility define --- to be removed */
-- 
1.9.1


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

* [RFC 10/22] v4l: Support the request API in format operations
  2016-05-06 10:53 [RFC 00/22] Media request API Sakari Ailus
                   ` (8 preceding siblings ...)
  2016-05-06 10:53 ` [RFC 09/22] v4l2-subdev.h: Add request field to format and selection structures Sakari Ailus
@ 2016-05-06 10:53 ` Sakari Ailus
  2016-05-06 10:53 ` [RFC 11/22] v4l: subdev: Support the request API in format and selection operations Sakari Ailus
                   ` (13 subsequent siblings)
  23 siblings, 0 replies; 28+ messages in thread
From: Sakari Ailus @ 2016-05-06 10:53 UTC (permalink / raw)
  To: linux-media; +Cc: laurent.pinchart, hverkuil, mchehab, Laurent Pinchart

From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

Store the formats in per-entity request data. The get and set format
operations are completely handled by the V4L2 core with help of the try
format driver operation.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/media/v4l2-core/v4l2-ioctl.c | 121 +++++++++++++++++++++++++++++++++++
 include/media/v4l2-dev.h             |  13 ++++
 2 files changed, 134 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
index bf15580..533fac6 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -1355,6 +1355,119 @@ static int v4l_enum_fmt(const struct v4l2_ioctl_ops *ops,
 	return ret;
 }
 
+#if defined(CONFIG_MEDIA_CONTROLLER)
+static void vdev_request_data_release(struct media_entity_request_data *data)
+{
+	struct video_device_request_data *vdata =
+		to_video_device_request_data(data);
+
+	kfree(vdata);
+}
+
+static int vdev_request_format(struct video_device *vdev, unsigned int req_id,
+			       struct media_device_request **_req,
+			       struct v4l2_pix_format_mplane **_fmt)
+{
+	struct media_entity_request_data *data;
+	struct video_device_request_data *vdata;
+	struct media_device_request *req;
+
+	if (!vdev->v4l2_dev || !vdev->v4l2_dev->mdev)
+		return -EINVAL;
+
+	req = media_device_request_find(vdev->v4l2_dev->mdev, req_id);
+	if (!req)
+		return -EINVAL;
+
+	*_req = req;
+
+	data = media_device_request_get_entity_data(req, &vdev->entity);
+	if (data) {
+		vdata = to_video_device_request_data(data);
+		*_fmt = &vdata->format;
+		return 0;
+	}
+
+	vdata = kzalloc(sizeof(*vdata), GFP_KERNEL);
+	if (!vdata) {
+		media_device_request_put(req);
+		return -ENOMEM;
+	}
+
+	vdata->data.release = vdev_request_data_release;
+
+	media_device_request_set_entity_data(req, &vdev->entity, &vdata->data);
+
+	*_fmt = &vdata->format;
+	return 0;
+}
+
+static int v4l_g_req_mplane_fmt(const struct v4l2_ioctl_ops *ops,
+				struct file *file, void *fh,
+				struct v4l2_format *fmt)
+{
+	struct video_device *vdev = video_devdata(file);
+	struct v4l2_pix_format_mplane *format;
+	struct media_device_request *req;
+	int ret;
+
+	ret = vdev_request_format(vdev, fmt->fmt.pix_mp.request,
+				  &req, &format);
+	if (ret < 0)
+		return ret;
+
+	fmt->fmt.pix_mp = *format;
+	media_device_request_put(req);
+	return 0;
+}
+
+static int v4l_s_req_mplane_fmt(const struct v4l2_ioctl_ops *ops,
+				struct file *file, void *fh,
+				struct v4l2_format *fmt)
+{
+	int (*try_op)(struct file *file, void *fh, struct v4l2_format *fmt);
+	struct video_device *vdev = video_devdata(file);
+	struct v4l2_pix_format_mplane *format;
+	struct media_device_request *req;
+	int ret;
+
+	if (fmt->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
+		try_op = ops->vidioc_try_fmt_vid_cap_mplane;
+	else
+		try_op = ops->vidioc_try_fmt_vid_out_mplane;
+
+	if (unlikely(!try_op))
+		return -ENOSYS;
+
+	ret = try_op(file, fh, fmt);
+	if (ret < 0)
+		return ret;
+
+	ret = vdev_request_format(vdev, fmt->fmt.pix_mp.request,
+				  &req, &format);
+	if (ret < 0)
+		return ret;
+
+	*format = fmt->fmt.pix_mp;
+	media_device_request_put(req);
+	return 0;
+}
+#else
+static int v4l_g_req_mplane_fmt(const struct v4l2_ioctl_ops *ops,
+				struct file *file, void *fh,
+				struct v4l2_format *fmt)
+{
+	return -ENOSYS;
+}
+
+static int v4l_s_req_mplane_fmt(const struct v4l2_ioctl_ops *ops,
+				struct file *file, void *fh,
+				struct v4l2_format *fmt)
+{
+	return -ENOSYS;
+}
+#endif
+
 static int v4l_g_fmt(const struct v4l2_ioctl_ops *ops,
 				struct file *file, void *fh, void *arg)
 {
@@ -1402,6 +1515,8 @@ static int v4l_g_fmt(const struct v4l2_ioctl_ops *ops,
 	case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
 		if (unlikely(!is_rx || !is_vid || !ops->vidioc_g_fmt_vid_cap_mplane))
 			break;
+		if (p->fmt.pix_mp.request)
+			return v4l_g_req_mplane_fmt(ops, file, fh, p);
 		return ops->vidioc_g_fmt_vid_cap_mplane(file, fh, arg);
 	case V4L2_BUF_TYPE_VIDEO_OVERLAY:
 		if (unlikely(!is_rx || !is_vid || !ops->vidioc_g_fmt_vid_overlay))
@@ -1426,6 +1541,8 @@ static int v4l_g_fmt(const struct v4l2_ioctl_ops *ops,
 	case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
 		if (unlikely(!is_tx || !is_vid || !ops->vidioc_g_fmt_vid_out_mplane))
 			break;
+		if (p->fmt.pix_mp.request)
+			return v4l_g_req_mplane_fmt(ops, file, fh, p);
 		return ops->vidioc_g_fmt_vid_out_mplane(file, fh, arg);
 	case V4L2_BUF_TYPE_VIDEO_OUTPUT_OVERLAY:
 		if (unlikely(!is_tx || !is_vid || !ops->vidioc_g_fmt_vid_out_overlay))
@@ -1480,6 +1597,8 @@ static int v4l_s_fmt(const struct v4l2_ioctl_ops *ops,
 		if (unlikely(!is_rx || !is_vid || !ops->vidioc_s_fmt_vid_cap_mplane))
 			break;
 		CLEAR_AFTER_FIELD(p, fmt.pix_mp);
+		if (p->fmt.pix_mp.request)
+			return v4l_s_req_mplane_fmt(ops, file, fh, p);
 		return ops->vidioc_s_fmt_vid_cap_mplane(file, fh, arg);
 	case V4L2_BUF_TYPE_VIDEO_OVERLAY:
 		if (unlikely(!is_rx || !is_vid || !ops->vidioc_s_fmt_vid_overlay))
@@ -1508,6 +1627,8 @@ static int v4l_s_fmt(const struct v4l2_ioctl_ops *ops,
 		if (unlikely(!is_tx || !is_vid || !ops->vidioc_s_fmt_vid_out_mplane))
 			break;
 		CLEAR_AFTER_FIELD(p, fmt.pix_mp);
+		if (p->fmt.pix_mp.request)
+			return v4l_s_req_mplane_fmt(ops, file, fh, p);
 		return ops->vidioc_s_fmt_vid_out_mplane(file, fh, arg);
 	case V4L2_BUF_TYPE_VIDEO_OUTPUT_OVERLAY:
 		if (unlikely(!is_tx || !is_vid || !ops->vidioc_s_fmt_vid_out_overlay))
diff --git a/include/media/v4l2-dev.h b/include/media/v4l2-dev.h
index 25a3190..19c7702 100644
--- a/include/media/v4l2-dev.h
+++ b/include/media/v4l2-dev.h
@@ -238,4 +238,17 @@ static inline int video_is_registered(struct video_device *vdev)
 	return test_bit(V4L2_FL_REGISTERED, &vdev->flags);
 }
 
+#if defined(CONFIG_MEDIA_CONTROLLER)
+struct video_device_request_data {
+	struct media_entity_request_data data;
+	struct v4l2_pix_format_mplane format;
+};
+
+static inline struct video_device_request_data *
+to_video_device_request_data(struct media_entity_request_data *data)
+{
+	return container_of(data, struct video_device_request_data, data);
+}
+#endif
+
 #endif /* _V4L2_DEV_H */
-- 
1.9.1


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

* [RFC 11/22] v4l: subdev: Support the request API in format and selection operations
  2016-05-06 10:53 [RFC 00/22] Media request API Sakari Ailus
                   ` (9 preceding siblings ...)
  2016-05-06 10:53 ` [RFC 10/22] v4l: Support the request API in format operations Sakari Ailus
@ 2016-05-06 10:53 ` Sakari Ailus
  2016-05-06 10:53 ` [RFC 12/22] vb2: Add allow_requests flag Sakari Ailus
                   ` (12 subsequent siblings)
  23 siblings, 0 replies; 28+ messages in thread
From: Sakari Ailus @ 2016-05-06 10:53 UTC (permalink / raw)
  To: linux-media; +Cc: laurent.pinchart, hverkuil, mchehab, Laurent Pinchart

From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

Store the formats and selection rectangles in per-entity request data.
This minimizes changes to drivers by reusing the v4l2_subdev_pad_config
infrastructure.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

Replace three if's for testing the same variable by a switch.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/v4l2-core/v4l2-subdev.c | 225 +++++++++++++++++++++++++---------
 include/media/v4l2-subdev.h           |  11 ++
 2 files changed, 180 insertions(+), 56 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index 9cbd011..461c2ac 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -130,39 +130,182 @@ static int subdev_close(struct file *file)
 }
 
 #if defined(CONFIG_VIDEO_V4L2_SUBDEV_API)
-static int check_format(struct v4l2_subdev *sd,
-			struct v4l2_subdev_format *format)
+static void subdev_request_data_release(struct media_entity_request_data *data)
 {
-	if (format->which != V4L2_SUBDEV_FORMAT_TRY &&
-	    format->which != V4L2_SUBDEV_FORMAT_ACTIVE)
-		return -EINVAL;
+	struct v4l2_subdev_request_data *sddata =
+		to_v4l2_subdev_request_data(data);
 
-	if (format->pad >= sd->entity.num_pads)
-		return -EINVAL;
+	kfree(sddata->pad);
+	kfree(sddata);
+}
 
-	return 0;
+static struct v4l2_subdev_pad_config *
+subdev_request_pad_config(struct v4l2_subdev *sd,
+			  struct media_device_request *req)
+{
+	struct media_entity_request_data *data;
+	struct v4l2_subdev_request_data *sddata;
+
+	data = media_device_request_get_entity_data(req, &sd->entity);
+	if (data) {
+		sddata = to_v4l2_subdev_request_data(data);
+		return sddata->pad;
+	}
+
+	sddata = kzalloc(sizeof(*sddata), GFP_KERNEL);
+	if (!sddata)
+		return ERR_PTR(-ENOMEM);
+
+	sddata->data.release = subdev_request_data_release;
+
+	sddata->pad = v4l2_subdev_alloc_pad_config(sd);
+	if (sddata->pad == NULL) {
+		kfree(sddata);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	media_device_request_set_entity_data(req, &sd->entity, &sddata->data);
+
+	return sddata->pad;
 }
 
-static int check_crop(struct v4l2_subdev *sd, struct v4l2_subdev_crop *crop)
+static int subdev_prepare_pad_config(struct v4l2_subdev *sd,
+				     struct v4l2_subdev_fh *fh,
+				     enum v4l2_subdev_format_whence which,
+				     unsigned int pad, unsigned int req_id,
+				     struct media_device_request **_req,
+				     struct v4l2_subdev_pad_config **_cfg)
 {
-	if (crop->which != V4L2_SUBDEV_FORMAT_TRY &&
-	    crop->which != V4L2_SUBDEV_FORMAT_ACTIVE)
+	struct v4l2_subdev_pad_config *cfg;
+	struct media_device_request *req;
+
+	if (pad >= sd->entity.num_pads)
 		return -EINVAL;
 
-	if (crop->pad >= sd->entity.num_pads)
+	switch (which) {
+	case V4L2_SUBDEV_FORMAT_ACTIVE:
+		*_req = NULL;
+		*_cfg = NULL;
+		return 0;
+	case V4L2_SUBDEV_FORMAT_TRY:
+		*_req = NULL;
+		*_cfg = fh->pad;
+		return 0;
+	case V4L2_SUBDEV_FORMAT_REQUEST:
+		if (!sd->v4l2_dev->mdev)
+			return -EINVAL;
+
+		req = media_device_request_find(sd->v4l2_dev->mdev, req_id);
+		if (!req)
+			return -EINVAL;
+
+		cfg = subdev_request_pad_config(sd, req);
+		if (IS_ERR(cfg)) {
+			media_device_request_put(req);
+			return PTR_ERR(cfg);
+		}
+
+		*_req = req;
+		*_cfg = cfg;
+
+		return 0;
+	default:
 		return -EINVAL;
+	}
 
-	return 0;
 }
 
-static int check_selection(struct v4l2_subdev *sd,
-			   struct v4l2_subdev_selection *sel)
+static int subdev_get_format(struct v4l2_subdev *sd,
+			     struct v4l2_subdev_fh *fh,
+			     struct v4l2_subdev_format *format)
+{
+	struct v4l2_subdev_pad_config *cfg;
+	struct media_device_request *req;
+	int ret;
+
+	ret = subdev_prepare_pad_config(sd, fh, format->which, format->pad,
+					format->request, &req, &cfg);
+	if (ret < 0)
+		return ret;
+
+	ret = v4l2_subdev_call(sd, pad, get_fmt, cfg, format);
+
+	if (req)
+		media_device_request_put(req);
+
+	return ret;
+}
+
+static int subdev_set_format(struct v4l2_subdev *sd,
+			     struct v4l2_subdev_fh *fh,
+			     struct v4l2_subdev_format *format)
+{
+	struct v4l2_subdev_pad_config *cfg;
+	struct media_device_request *req;
+	int ret;
+
+	ret = subdev_prepare_pad_config(sd, fh, format->which, format->pad,
+					format->request, &req, &cfg);
+	if (ret < 0)
+		return ret;
+
+	ret = v4l2_subdev_call(sd, pad, set_fmt, cfg, format);
+
+	if (req)
+		media_device_request_put(req);
+
+	return ret;
+}
+
+static int subdev_get_selection(struct v4l2_subdev *sd,
+				struct v4l2_subdev_fh *fh,
+				struct v4l2_subdev_selection *sel)
+{
+	struct v4l2_subdev_pad_config *cfg;
+	struct media_device_request *req;
+	int ret;
+
+	ret = subdev_prepare_pad_config(sd, fh, sel->which, sel->pad,
+					sel->request, &req, &cfg);
+	if (ret < 0)
+		return ret;
+
+	ret = v4l2_subdev_call(sd, pad, get_selection, cfg, sel);
+
+	if (req)
+		media_device_request_put(req);
+
+	return ret;
+}
+
+static int subdev_set_selection(struct v4l2_subdev *sd,
+				struct v4l2_subdev_fh *fh,
+				struct v4l2_subdev_selection *sel)
+{
+	struct v4l2_subdev_pad_config *cfg;
+	struct media_device_request *req;
+	int ret;
+
+	ret = subdev_prepare_pad_config(sd, fh, sel->which, sel->pad,
+					sel->request, &req, &cfg);
+	if (ret < 0)
+		return ret;
+
+	ret = v4l2_subdev_call(sd, pad, set_selection, cfg, sel);
+
+	if (req)
+		media_device_request_put(req);
+
+	return ret;
+}
+
+static int check_crop(struct v4l2_subdev *sd, struct v4l2_subdev_crop *crop)
 {
-	if (sel->which != V4L2_SUBDEV_FORMAT_TRY &&
-	    sel->which != V4L2_SUBDEV_FORMAT_ACTIVE)
+	if (crop->which != V4L2_SUBDEV_FORMAT_TRY &&
+	    crop->which != V4L2_SUBDEV_FORMAT_ACTIVE)
 		return -EINVAL;
 
-	if (sel->pad >= sd->entity.num_pads)
+	if (crop->pad >= sd->entity.num_pads)
 		return -EINVAL;
 
 	return 0;
@@ -258,25 +401,11 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
 	}
 
 #if defined(CONFIG_VIDEO_V4L2_SUBDEV_API)
-	case VIDIOC_SUBDEV_G_FMT: {
-		struct v4l2_subdev_format *format = arg;
+	case VIDIOC_SUBDEV_G_FMT:
+		return subdev_get_format(sd, subdev_fh, arg);
 
-		rval = check_format(sd, format);
-		if (rval)
-			return rval;
-
-		return v4l2_subdev_call(sd, pad, get_fmt, subdev_fh->pad, format);
-	}
-
-	case VIDIOC_SUBDEV_S_FMT: {
-		struct v4l2_subdev_format *format = arg;
-
-		rval = check_format(sd, format);
-		if (rval)
-			return rval;
-
-		return v4l2_subdev_call(sd, pad, set_fmt, subdev_fh->pad, format);
-	}
+	case VIDIOC_SUBDEV_S_FMT:
+		return subdev_set_format(sd, subdev_fh, arg);
 
 	case VIDIOC_SUBDEV_G_CROP: {
 		struct v4l2_subdev_crop *crop = arg;
@@ -381,27 +510,11 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
 					fie);
 	}
 
-	case VIDIOC_SUBDEV_G_SELECTION: {
-		struct v4l2_subdev_selection *sel = arg;
-
-		rval = check_selection(sd, sel);
-		if (rval)
-			return rval;
-
-		return v4l2_subdev_call(
-			sd, pad, get_selection, subdev_fh->pad, sel);
-	}
-
-	case VIDIOC_SUBDEV_S_SELECTION: {
-		struct v4l2_subdev_selection *sel = arg;
-
-		rval = check_selection(sd, sel);
-		if (rval)
-			return rval;
+	case VIDIOC_SUBDEV_G_SELECTION:
+		return subdev_get_selection(sd, subdev_fh, arg);
 
-		return v4l2_subdev_call(
-			sd, pad, set_selection, subdev_fh->pad, sel);
-	}
+	case VIDIOC_SUBDEV_S_SELECTION:
+		return subdev_set_selection(sd, subdev_fh, arg);
 
 	case VIDIOC_G_EDID: {
 		struct v4l2_subdev_edid *edid = arg;
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index 32fc7a4..ca92e2c 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -808,6 +808,17 @@ int v4l2_subdev_link_validate(struct media_link *link);
 struct v4l2_subdev_pad_config *
 v4l2_subdev_alloc_pad_config(struct v4l2_subdev *sd);
 void v4l2_subdev_free_pad_config(struct v4l2_subdev_pad_config *cfg);
+
+struct v4l2_subdev_request_data {
+	struct media_entity_request_data data;
+	struct v4l2_subdev_pad_config *pad;
+};
+
+static inline struct v4l2_subdev_request_data *
+to_v4l2_subdev_request_data(struct media_entity_request_data *data)
+{
+	return container_of(data, struct v4l2_subdev_request_data, data);
+}
 #endif /* CONFIG_MEDIA_CONTROLLER */
 
 void v4l2_subdev_init(struct v4l2_subdev *sd,
-- 
1.9.1


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

* [RFC 12/22] vb2: Add allow_requests flag
  2016-05-06 10:53 [RFC 00/22] Media request API Sakari Ailus
                   ` (10 preceding siblings ...)
  2016-05-06 10:53 ` [RFC 11/22] v4l: subdev: Support the request API in format and selection operations Sakari Ailus
@ 2016-05-06 10:53 ` Sakari Ailus
  2016-05-06 10:53 ` [RFC 13/22] vb2: Add helper function to check for request buffers Sakari Ailus
                   ` (11 subsequent siblings)
  23 siblings, 0 replies; 28+ messages in thread
From: Sakari Ailus @ 2016-05-06 10:53 UTC (permalink / raw)
  To: linux-media; +Cc: laurent.pinchart, hverkuil, mchehab, Hans Verkuil

From: Hans Verkuil <hans.verkuil@cisco.com>

The driver has to set allow_requests explicitly in order to allow
queuing or preparing buffers for a specific request ID.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/v4l2-core/videobuf2-v4l2.c | 5 +++++
 include/media/videobuf2-core.h           | 2 ++
 2 files changed, 7 insertions(+)

diff --git a/drivers/media/v4l2-core/videobuf2-v4l2.c b/drivers/media/v4l2-core/videobuf2-v4l2.c
index 0721258..bb135fc 100644
--- a/drivers/media/v4l2-core/videobuf2-v4l2.c
+++ b/drivers/media/v4l2-core/videobuf2-v4l2.c
@@ -174,6 +174,11 @@ static int vb2_queue_or_prepare_buf(struct vb2_queue *q, struct v4l2_buffer *b,
 		return -EINVAL;
 	}
 
+	if (!q->allow_requests && b->request) {
+		dprintk(1, "%s: unsupported request ID\n", opname);
+		return -EINVAL;
+	}
+
 	return __verify_planes_array(q->bufs[b->index], b);
 }
 
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index 8a0f55b..41dab63 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -400,6 +400,7 @@ struct vb2_buf_ops {
  * @fileio_read_once:		report EOF after reading the first buffer
  * @fileio_write_immediately:	queue buffer after each write() call
  * @allow_zero_bytesused:	allow bytesused == 0 to be passed to the driver
+ * @allow_requests:		allow request != 0 to be passed to the driver
  * @lock:	pointer to a mutex that protects the vb2_queue struct. The
  *		driver can set this to a mutex to let the v4l2 core serialize
  *		the queuing ioctls. If the driver wants to handle locking
@@ -463,6 +464,7 @@ struct vb2_queue {
 	unsigned			fileio_read_once:1;
 	unsigned			fileio_write_immediately:1;
 	unsigned			allow_zero_bytesused:1;
+	unsigned			allow_requests:1;
 
 	struct mutex			*lock;
 	void				*owner;
-- 
1.9.1


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

* [RFC 13/22] vb2: Add helper function to check for request buffers
  2016-05-06 10:53 [RFC 00/22] Media request API Sakari Ailus
                   ` (11 preceding siblings ...)
  2016-05-06 10:53 ` [RFC 12/22] vb2: Add allow_requests flag Sakari Ailus
@ 2016-05-06 10:53 ` Sakari Ailus
  2016-05-06 10:53 ` [RFC 14/22] media: Add MEDIA_IOC_DQEVENT Sakari Ailus
                   ` (10 subsequent siblings)
  23 siblings, 0 replies; 28+ messages in thread
From: Sakari Ailus @ 2016-05-06 10:53 UTC (permalink / raw)
  To: linux-media; +Cc: laurent.pinchart, hverkuil, mchehab, Laurent Pinchart

From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

The vb2_queue_has_request() function will check whether a buffer has
been prepared for the given request ID.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 drivers/media/v4l2-core/videobuf2-v4l2.c | 17 +++++++++++++++++
 include/media/videobuf2-v4l2.h           |  2 ++
 2 files changed, 19 insertions(+)

diff --git a/drivers/media/v4l2-core/videobuf2-v4l2.c b/drivers/media/v4l2-core/videobuf2-v4l2.c
index bb135fc..6c1c4bf 100644
--- a/drivers/media/v4l2-core/videobuf2-v4l2.c
+++ b/drivers/media/v4l2-core/videobuf2-v4l2.c
@@ -790,6 +790,23 @@ void vb2_queue_release(struct vb2_queue *q)
 }
 EXPORT_SYMBOL_GPL(vb2_queue_release);
 
+bool vb2_queue_has_request(struct vb2_queue *q, unsigned int request)
+{
+	unsigned int i;
+
+	for (i = 0; i < q->num_buffers; i++) {
+		struct vb2_buffer *vb = q->bufs[i];
+		struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
+
+		if (vb->state == VB2_BUF_STATE_PREPARED &&
+		    vbuf->request == request)
+			return true;
+	}
+
+	return false;
+}
+EXPORT_SYMBOL_GPL(vb2_queue_has_request);
+
 /**
  * vb2_poll() - implements poll userspace operation
  * @q:		videobuf2 queue
diff --git a/include/media/videobuf2-v4l2.h b/include/media/videobuf2-v4l2.h
index b1ee91c..7727c52 100644
--- a/include/media/videobuf2-v4l2.h
+++ b/include/media/videobuf2-v4l2.h
@@ -68,6 +68,8 @@ void vb2_queue_release(struct vb2_queue *q);
 unsigned int vb2_poll(struct vb2_queue *q, struct file *file,
 		poll_table *wait);
 
+bool vb2_queue_has_request(struct vb2_queue *q, unsigned int request);
+
 /*
  * The following functions are not part of the vb2 core API, but are simple
  * helper functions that you can use in your struct v4l2_file_operations,
-- 
1.9.1


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

* [RFC 14/22] media: Add MEDIA_IOC_DQEVENT
  2016-05-06 10:53 [RFC 00/22] Media request API Sakari Ailus
                   ` (12 preceding siblings ...)
  2016-05-06 10:53 ` [RFC 13/22] vb2: Add helper function to check for request buffers Sakari Ailus
@ 2016-05-06 10:53 ` Sakari Ailus
  2016-05-06 10:53 ` [RFC 15/22] media: Make events on request completion optional, disabled by default Sakari Ailus
                   ` (9 subsequent siblings)
  23 siblings, 0 replies; 28+ messages in thread
From: Sakari Ailus @ 2016-05-06 10:53 UTC (permalink / raw)
  To: linux-media; +Cc: laurent.pinchart, hverkuil, mchehab

Events on a media device tell about completion of requests. Blocking and
non-blocking operation is supported.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/media-device.c | 114 +++++++++++++++++++++++++++++++++++++++++--
 include/media/media-device.h |  11 +++++
 include/uapi/linux/media.h   |  17 +++++++
 3 files changed, 137 insertions(+), 5 deletions(-)

diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index 10b9a4a..357c3cb 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -23,6 +23,7 @@
 /* We need to access legacy defines from linux/media.h */
 #define __NEED_MEDIA_LEGACY_API
 
+#include <linux/atomic.h>
 #include <linux/compat.h>
 #include <linux/export.h>
 #include <linux/idr.h>
@@ -32,6 +33,7 @@
 #include <linux/types.h>
 #include <linux/pci.h>
 #include <linux/usb.h>
+#include <linux/wait.h>
 
 #include <media/media-device.h>
 #include <media/media-devnode.h>
@@ -42,6 +44,12 @@
 struct media_device_fh {
 	struct media_devnode_fh fh;
 	struct list_head requests;
+	struct {
+		spinlock_t lock;
+		struct list_head head;
+		wait_queue_head_t wait;
+		atomic_t sequence;
+	} kevents;
 };
 
 static inline struct media_device_fh *media_device_fh(struct file *filp)
@@ -92,6 +100,33 @@ void media_device_request_get(struct media_device_request *req)
 }
 EXPORT_SYMBOL_GPL(media_device_request_get);
 
+static void media_device_request_queue_event(struct media_device *mdev,
+					     struct media_device_request *req)
+{
+	struct media_kevent *kev = req->kev;
+	struct media_event *ev = &kev->ev;
+	struct media_device_fh *fh;
+	unsigned long flags;
+
+	spin_lock_irqsave(&mdev->req_lock, flags);
+	if (!req->filp) {
+		spin_unlock_irqrestore(&mdev->req_lock, flags);
+		return;
+	}
+	fh = media_device_fh(req->filp);
+
+	ev->sequence = atomic_inc_return(&fh->kevents.sequence);
+	ev->type = MEDIA_EVENT_TYPE_REQUEST_COMPLETE;
+	ev->req_complete.id = req->id;
+	ev->req_complete.status = 0;
+
+	list_add(&kev->list, &fh->kevents.head);
+	req->kev = NULL;
+	req->state = MEDIA_DEVICE_REQUEST_STATE_COMPLETE;
+	wake_up(&fh->kevents.wait);
+	spin_unlock_irqrestore(&mdev->req_lock, flags);
+}
+
 static void media_device_request_release(struct kref *kref)
 {
 	struct media_entity_request_data *data, *next;
@@ -106,6 +141,9 @@ static void media_device_request_release(struct kref *kref)
 
 	ida_simple_remove(&mdev->req_ids, req->id);
 
+	kfree(req->kev);
+	req->kev = NULL;
+
 	mdev->ops->req_free(mdev, req);
 }
 
@@ -180,10 +218,12 @@ void media_device_request_set_entity_data(struct media_device_request *req,
 EXPORT_SYMBOL_GPL(media_device_request_set_entity_data);
 
 static int media_device_request_alloc(struct media_device *mdev,
-				      struct media_device_fh *fh,
+				      struct file *filp,
 				      struct media_request_cmd *cmd)
 {
+	struct media_device_fh *fh = media_device_fh(filp);
 	struct media_device_request *req;
+	struct media_kevent *kev;
 	unsigned long flags;
 	int id = ida_simple_get(&mdev->req_ids, 1, 0, GFP_KERNEL);
 	int ret;
@@ -191,14 +231,22 @@ static int media_device_request_alloc(struct media_device *mdev,
 	if (id < 0)
 		return id;
 
+	kev = kzalloc(sizeof(*kev), GFP_KERNEL);
+	if (!kev) {
+		ret = -ENOMEM;
+		goto out_ida_simple_remove;
+	}
+
 	req = mdev->ops->req_alloc(mdev);
 	if (!req) {
 		ret = -ENOMEM;
-		goto out_ida_simple_remove;
+		goto out_kev_free;
 	}
 
 	req->mdev = mdev;
 	req->id = id;
+	req->filp = filp;
+	req->kev = kev;
 	kref_init(&req->kref);
 	INIT_LIST_HEAD(&req->data);
 
@@ -211,6 +259,9 @@ static int media_device_request_alloc(struct media_device *mdev,
 
 	return 0;
 
+out_kev_free:
+	kfree(kev);
+
 out_ida_simple_remove:
 	ida_simple_remove(&mdev->req_ids, id);
 
@@ -233,6 +284,8 @@ static void media_device_request_delete(struct media_device *mdev,
 void media_device_request_complete(struct media_device *mdev,
 				   struct media_device_request *req)
 {
+	media_device_request_queue_event(mdev, req);
+
 	media_device_request_delete(mdev, req);
 }
 EXPORT_SYMBOL_GPL(media_device_request_complete);
@@ -272,7 +325,6 @@ static long media_device_request_cmd(struct media_device *mdev,
 				     struct file *filp,
 				     struct media_request_cmd *cmd)
 {
-	struct media_device_fh *fh = media_device_fh(filp);
 	struct media_device_request *req = NULL;
 	unsigned long flags;
 	int ret;
@@ -288,7 +340,7 @@ static long media_device_request_cmd(struct media_device *mdev,
 
 	switch (cmd->cmd) {
 	case MEDIA_REQ_CMD_ALLOC:
-		ret = media_device_request_alloc(mdev, fh, cmd);
+		ret = media_device_request_alloc(mdev, filp, cmd);
 		break;
 
 	case MEDIA_REQ_CMD_DELETE:
@@ -347,6 +399,10 @@ static int media_device_open(struct file *filp)
 		return -ENOMEM;
 
 	INIT_LIST_HEAD(&fh->requests);
+	INIT_LIST_HEAD(&fh->kevents.head);
+	spin_lock_init(&fh->kevents.lock);
+	init_waitqueue_head(&fh->kevents.wait);
+	atomic_set(&fh->kevents.sequence, -1);
 	filp->private_data = &fh->fh;
 
 	return 0;
@@ -356,6 +412,7 @@ static int media_device_close(struct file *filp)
 {
 	struct media_device_fh *fh = media_device_fh(filp);
 	struct media_device *mdev = to_media_device(fh->fh.devnode);
+	struct media_kevent *kev, *kev_safe;
 
 	spin_lock_irq(&mdev->req_lock);
 	while (!list_empty(&fh->requests)) {
@@ -364,13 +421,17 @@ static int media_device_close(struct file *filp)
 		req = list_first_entry(&fh->requests, typeof(*req), fh_list);
 		list_del(&req->list);
 		list_del(&req->fh_list);
-
+		req->filp = NULL;
 		spin_unlock_irq(&mdev->req_lock);
 		media_device_request_put(req);
 		spin_lock_irq(&mdev->req_lock);
 	}
 	spin_unlock_irq(&mdev->req_lock);
 
+	/* No other users around --- no lock needed. */
+	list_for_each_entry_safe(kev, kev_safe, &fh->kevents.head, list)
+		list_del(&kev->list);
+
 	kfree(fh);
 
 	return 0;
@@ -686,6 +747,47 @@ static long media_device_get_topology(struct media_device *mdev,
 	return ret;
 }
 
+static struct media_kevent *opportunistic_dqevent(struct file *filp)
+{
+	struct media_device_fh *fh = media_device_fh(filp);
+	struct media_kevent *kev = NULL;
+	unsigned long flags;
+
+	spin_lock_irqsave(&fh->kevents.lock, flags);
+	if (!list_empty(&fh->kevents.head)) {
+		kev = list_last_entry(&fh->kevents.head,
+				      struct media_kevent, list);
+		list_del(&kev->list);
+	}
+	spin_unlock_irqrestore(&fh->kevents.lock, flags);
+
+	return kev;
+}
+
+static int media_device_dqevent(struct media_device *mdev,
+				struct file *filp,
+				struct media_event *ev)
+{
+	struct media_device_fh *fh = media_device_fh(filp);
+	struct media_kevent *kev;
+
+	if (filp->f_flags & O_NONBLOCK) {
+		kev = opportunistic_dqevent(filp);
+		if (!kev)
+			return -ENODATA;
+	} else {
+		int ret = wait_event_interruptible(
+			fh->kevents.wait, (kev = opportunistic_dqevent(filp)));
+		if (ret == -ERESTARTSYS)
+			return ret;
+	}
+
+	*ev = kev->ev;
+	kfree(kev);
+
+	return 0;
+}
+
 static long copy_arg_from_user(void *karg, void __user *uarg, unsigned int cmd)
 {
 	/* All media IOCTLs are _IOWR() */
@@ -835,6 +937,7 @@ static const struct media_ioctl_info ioctl_info[] = {
 	MEDIA_IOC(SETUP_LINK, media_device_setup_link, MEDIA_IOC_FL_GRAPH_MUTEX),
 	MEDIA_IOC(G_TOPOLOGY, media_device_get_topology, MEDIA_IOC_FL_GRAPH_MUTEX),
 	MEDIA_IOC(REQUEST_CMD, media_device_request_cmd, 0),
+	MEDIA_IOC(DQEVENT, media_device_dqevent, 0),
 };
 
 static long media_device_ioctl(struct file *filp, unsigned int cmd,
@@ -883,6 +986,7 @@ static const struct media_ioctl_info compat_ioctl_info[] = {
 	MEDIA_IOC(SETUP_LINK, media_device_setup_link, MEDIA_IOC_FL_GRAPH_MUTEX),
 	MEDIA_IOC(G_TOPOLOGY, media_device_get_topology, MEDIA_IOC_FL_GRAPH_MUTEX),
 	MEDIA_IOC(REQUEST_CMD, media_device_request_cmd, 0),
+	MEDIA_IOC(DQEVENT, media_device_dqevent, 0),
 };
 
 static long media_device_compat_ioctl(struct file *filp, unsigned int cmd,
diff --git a/include/media/media-device.h b/include/media/media-device.h
index 2082108..e62ad13 100644
--- a/include/media/media-device.h
+++ b/include/media/media-device.h
@@ -30,6 +30,8 @@
 #include <media/media-devnode.h>
 #include <media/media-entity.h>
 
+#include <uapi/linux/media.h>
+
 /**
  * DOC: Media Controller
  *
@@ -263,7 +265,9 @@
 
 struct ida;
 struct device;
+struct file;
 struct media_device;
+struct media_device_fh;
 
 enum media_device_request_state {
 	MEDIA_DEVICE_REQUEST_STATE_IDLE,
@@ -272,6 +276,11 @@ enum media_device_request_state {
 	MEDIA_DEVICE_REQUEST_STATE_COMPLETE,
 };
 
+struct media_kevent {
+	struct list_head list;
+	struct media_event ev;
+};
+
 /**
  * struct media_device_request - Media device request
  * @id: Request ID
@@ -286,6 +295,8 @@ enum media_device_request_state {
 struct media_device_request {
 	u32 id;
 	struct media_device *mdev;
+	struct file *filp;
+	struct media_kevent *kev;
 	struct kref kref;
 	struct list_head list;
 	struct list_head fh_list;
diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
index e8922ea..f6e1efe 100644
--- a/include/uapi/linux/media.h
+++ b/include/uapi/linux/media.h
@@ -399,11 +399,28 @@ struct __attribute__ ((packed)) media_request_cmd {
 	__u32 request;
 };
 
+#define MEDIA_EVENT_TYPE_REQUEST_COMPLETE	1
+
+struct media_event_request_complete {
+	__u32 id;
+	__s32 status;
+};
+
+struct media_event {
+	__u32 type;
+	__u32 sequence;
+
+	union {
+		struct media_event_request_complete req_complete;
+	};
+} __attribute__ ((packed));
+
 #define MEDIA_IOC_DEVICE_INFO		_IOWR('|', 0x00, struct media_device_info)
 #define MEDIA_IOC_ENUM_ENTITIES		_IOWR('|', 0x01, struct media_entity_desc)
 #define MEDIA_IOC_ENUM_LINKS		_IOWR('|', 0x02, struct media_links_enum)
 #define MEDIA_IOC_SETUP_LINK		_IOWR('|', 0x03, struct media_link_desc)
 #define MEDIA_IOC_G_TOPOLOGY		_IOWR('|', 0x04, struct media_v2_topology)
 #define MEDIA_IOC_REQUEST_CMD		_IOWR('|', 0x05, struct media_request_cmd)
+#define MEDIA_IOC_DQEVENT		_IOWR('|', 0x06, struct media_event)
 
 #endif /* __LINUX_MEDIA_H */
-- 
1.9.1


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

* [RFC 15/22] media: Make events on request completion optional, disabled by default
  2016-05-06 10:53 [RFC 00/22] Media request API Sakari Ailus
                   ` (13 preceding siblings ...)
  2016-05-06 10:53 ` [RFC 14/22] media: Add MEDIA_IOC_DQEVENT Sakari Ailus
@ 2016-05-06 10:53 ` Sakari Ailus
  2016-05-06 11:05   ` Sakari Ailus
  2016-05-06 10:53 ` [RFC 16/22] media: Add poll support Sakari Ailus
                   ` (8 subsequent siblings)
  23 siblings, 1 reply; 28+ messages in thread
From: Sakari Ailus @ 2016-05-06 10:53 UTC (permalink / raw)
  To: linux-media; +Cc: laurent.pinchart, hverkuil, mchehab

Add flags to requests. The first defined flag, MEDIA_REQ_FL_COMPLETE_EVENT
is used to tell whether to queue an event on request completion. Unless
the flag is set, no event is generated when a request completes.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/media-device.c | 26 +++++++++++++++++---------
 include/media/media-device.h |  2 ++
 include/uapi/linux/media.h   | 16 +++++++++++++---
 3 files changed, 32 insertions(+), 12 deletions(-)

diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index 357c3cb..0b1ab88 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -284,7 +284,8 @@ static void media_device_request_delete(struct media_device *mdev,
 void media_device_request_complete(struct media_device *mdev,
 				   struct media_device_request *req)
 {
-	media_device_request_queue_event(mdev, req);
+	if (req->flags & MEDIA_REQ_FL_COMPLETE_EVENT)
+		media_device_request_queue_event(mdev, req);
 
 	media_device_request_delete(mdev, req);
 }
@@ -292,8 +293,8 @@ EXPORT_SYMBOL_GPL(media_device_request_complete);
 
 static int media_device_request_queue_apply(
 	struct media_device *mdev, struct media_device_request *req,
-	int (*fn)(struct media_device *mdev,
-		  struct media_device_request *req))
+	u32 req_flags, int (*fn)(struct media_device *mdev,
+				 struct media_device_request *req))
 {
 	unsigned long flags;
 	int rval = 0;
@@ -302,10 +303,12 @@ static int media_device_request_queue_apply(
 		return -ENOSYS;
 
 	spin_lock_irqsave(&mdev->req_lock, flags);
-	if (req->state != MEDIA_DEVICE_REQUEST_STATE_IDLE)
+	if (req->state != MEDIA_DEVICE_REQUEST_STATE_IDLE) {
 		rval = -EINVAL;
-	else
+	} else {
 		req->state = MEDIA_DEVICE_REQUEST_STATE_QUEUED;
+		req->flags = req_flags;
+	}
 	spin_unlock_irqrestore(&mdev->req_lock, flags);
 
 	if (rval)
@@ -358,12 +361,12 @@ static long media_device_request_cmd(struct media_device *mdev,
 		break;
 
 	case MEDIA_REQ_CMD_APPLY:
-		ret = media_device_request_queue_apply(mdev, req,
+		ret = media_device_request_queue_apply(mdev, req, cmd->flags,
 						       mdev->ops->req_apply);
 		break;
 
 	case MEDIA_REQ_CMD_QUEUE:
-		ret = media_device_request_queue_apply(mdev, req,
+		ret = media_device_request_queue_apply(mdev, req, cmd->flags,
 						       mdev->ops->req_queue);
 		break;
 
@@ -930,13 +933,18 @@ static long __media_device_ioctl(
 	return ret;
 }
 
+static const unsigned short media_request_cmd_sizes[] = {
+	sizeof(struct media_request_cmd_0),
+	0
+};
+
 static const struct media_ioctl_info ioctl_info[] = {
 	MEDIA_IOC(DEVICE_INFO, media_device_get_info, MEDIA_IOC_FL_GRAPH_MUTEX),
 	MEDIA_IOC(ENUM_ENTITIES, media_device_enum_entities, MEDIA_IOC_FL_GRAPH_MUTEX),
 	MEDIA_IOC(ENUM_LINKS, media_device_enum_links, MEDIA_IOC_FL_GRAPH_MUTEX),
 	MEDIA_IOC(SETUP_LINK, media_device_setup_link, MEDIA_IOC_FL_GRAPH_MUTEX),
 	MEDIA_IOC(G_TOPOLOGY, media_device_get_topology, MEDIA_IOC_FL_GRAPH_MUTEX),
-	MEDIA_IOC(REQUEST_CMD, media_device_request_cmd, 0),
+	MEDIA_IOC_SZ(REQUEST_CMD, media_device_request_cmd, 0, media_request_cmd_sizes),
 	MEDIA_IOC(DQEVENT, media_device_dqevent, 0),
 };
 
@@ -985,7 +993,7 @@ static const struct media_ioctl_info compat_ioctl_info[] = {
 	MEDIA_IOC_ARG(ENUM_LINKS32, media_device_enum_links, MEDIA_IOC_FL_GRAPH_MUTEX, from_user_enum_links32, copy_arg_to_user_nop),
 	MEDIA_IOC(SETUP_LINK, media_device_setup_link, MEDIA_IOC_FL_GRAPH_MUTEX),
 	MEDIA_IOC(G_TOPOLOGY, media_device_get_topology, MEDIA_IOC_FL_GRAPH_MUTEX),
-	MEDIA_IOC(REQUEST_CMD, media_device_request_cmd, 0),
+	MEDIA_IOC_SZ(REQUEST_CMD, media_device_request_cmd, 0, media_request_cmd_sizes),
 	MEDIA_IOC(DQEVENT, media_device_dqevent, 0),
 };
 
diff --git a/include/media/media-device.h b/include/media/media-device.h
index e62ad13..fc91d2d 100644
--- a/include/media/media-device.h
+++ b/include/media/media-device.h
@@ -291,6 +291,7 @@ struct media_kevent {
  * @state: The state of the request, MEDIA_DEVICE_REQUEST_STATE_*,
  *	   access to state serialised by mdev->req_lock
  * @data: Per-entity data list
+ * @flags: Request specific flags, MEDIA_REQ_FL_*
  */
 struct media_device_request {
 	u32 id;
@@ -302,6 +303,7 @@ struct media_device_request {
 	struct list_head fh_list;
 	enum media_device_request_state state;
 	struct list_head data;
+	u32 flags;
 };
 
 /**
diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
index f6e1efe..4fab54d 100644
--- a/include/uapi/linux/media.h
+++ b/include/uapi/linux/media.h
@@ -394,26 +394,36 @@ struct media_v2_topology {
 #define MEDIA_REQ_CMD_APPLY		2
 #define MEDIA_REQ_CMD_QUEUE		3
 
+#define MEDIA_REQ_FL_COMPLETE_EVENT	(1 << 0)
+
+#ifdef __KERNEL__
+struct __attribute__ ((packed)) media_request_cmd_0 {
+	__u32 cmd;
+	__u32 request;
+};
+#endif
+
 struct __attribute__ ((packed)) media_request_cmd {
 	__u32 cmd;
 	__u32 request;
+	__u32 flags;
 };
 
 #define MEDIA_EVENT_TYPE_REQUEST_COMPLETE	1
 
-struct media_event_request_complete {
+struct __attribute__ ((packed)) media_event_request_complete {
 	__u32 id;
 	__s32 status;
 };
 
-struct media_event {
+struct __attribute__ ((packed)) media_event {
 	__u32 type;
 	__u32 sequence;
 
 	union {
 		struct media_event_request_complete req_complete;
 	};
-} __attribute__ ((packed));
+};
 
 #define MEDIA_IOC_DEVICE_INFO		_IOWR('|', 0x00, struct media_device_info)
 #define MEDIA_IOC_ENUM_ENTITIES		_IOWR('|', 0x01, struct media_entity_desc)
-- 
1.9.1


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

* [RFC 16/22] media: Add poll support
  2016-05-06 10:53 [RFC 00/22] Media request API Sakari Ailus
                   ` (14 preceding siblings ...)
  2016-05-06 10:53 ` [RFC 15/22] media: Make events on request completion optional, disabled by default Sakari Ailus
@ 2016-05-06 10:53 ` Sakari Ailus
  2016-05-06 10:53 ` [RFC 17/22] DocBook: media: Document the media request API Sakari Ailus
                   ` (7 subsequent siblings)
  23 siblings, 0 replies; 28+ messages in thread
From: Sakari Ailus @ 2016-05-06 10:53 UTC (permalink / raw)
  To: linux-media; +Cc: laurent.pinchart, hverkuil, mchehab

Implement poll for events. POLLPRI is used to notify users on incoming
events.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/media-device.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index 0b1ab88..2a9bf80 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -956,6 +956,32 @@ static long media_device_ioctl(struct file *filp, unsigned int cmd,
 		ioctl_info, ARRAY_SIZE(ioctl_info));
 }
 
+unsigned int media_device_poll(struct file *filp, struct poll_table_struct *wait)
+{
+	struct media_device_fh *fh = media_device_fh(filp);
+	unsigned int poll_events = poll_requested_events(wait);
+	int ret = 0;
+
+	if (poll_events & (POLLIN | POLLOUT))
+		return POLLERR;
+
+	if (poll_events & POLLPRI) {
+		unsigned long flags;
+		bool empty;
+
+		spin_lock_irqsave(&fh->kevents.lock, flags);
+		empty = list_empty(&fh->kevents.head);
+		spin_unlock_irqrestore(&fh->kevents.lock, flags);
+
+		if (empty)
+			poll_wait(filp, &fh->kevents.wait, wait);
+		else
+			ret |= POLLPRI;
+	}
+
+	return ret;
+}
+
 #ifdef CONFIG_COMPAT
 
 struct media_links_enum32 {
@@ -1010,6 +1036,7 @@ static const struct media_file_operations media_device_fops = {
 	.owner = THIS_MODULE,
 	.open = media_device_open,
 	.ioctl = media_device_ioctl,
+	.poll = media_device_poll,
 #ifdef CONFIG_COMPAT
 	.compat_ioctl = media_device_compat_ioctl,
 #endif /* CONFIG_COMPAT */
-- 
1.9.1


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

* [RFC 17/22] DocBook: media: Document the media request API
  2016-05-06 10:53 [RFC 00/22] Media request API Sakari Ailus
                   ` (15 preceding siblings ...)
  2016-05-06 10:53 ` [RFC 16/22] media: Add poll support Sakari Ailus
@ 2016-05-06 10:53 ` Sakari Ailus
  2016-05-06 10:53 ` [RFC 18/22] DocBook: media: Document the V4L2 " Sakari Ailus
                   ` (6 subsequent siblings)
  23 siblings, 0 replies; 28+ messages in thread
From: Sakari Ailus @ 2016-05-06 10:53 UTC (permalink / raw)
  To: linux-media; +Cc: laurent.pinchart, hverkuil, mchehab, Laurent Pinchart

From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

The media request API is made of a new ioctl to implement request
management. Document it.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

Strip off the reserved fields.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 .../DocBook/media/v4l/media-controller.xml         |   1 +
 .../DocBook/media/v4l/media-ioc-request-cmd.xml    | 188 +++++++++++++++++++++
 2 files changed, 189 insertions(+)
 create mode 100644 Documentation/DocBook/media/v4l/media-ioc-request-cmd.xml

diff --git a/Documentation/DocBook/media/v4l/media-controller.xml b/Documentation/DocBook/media/v4l/media-controller.xml
index 5f2fc07..2a5a5d0 100644
--- a/Documentation/DocBook/media/v4l/media-controller.xml
+++ b/Documentation/DocBook/media/v4l/media-controller.xml
@@ -101,5 +101,6 @@
   &sub-media-ioc-g-topology;
   &sub-media-ioc-enum-entities;
   &sub-media-ioc-enum-links;
+  &sub-media-ioc-request-cmd;
   &sub-media-ioc-setup-link;
 </appendix>
diff --git a/Documentation/DocBook/media/v4l/media-ioc-request-cmd.xml b/Documentation/DocBook/media/v4l/media-ioc-request-cmd.xml
new file mode 100644
index 0000000..4f4acea
--- /dev/null
+++ b/Documentation/DocBook/media/v4l/media-ioc-request-cmd.xml
@@ -0,0 +1,188 @@
+<refentry id="media-ioc-request-cmd">
+  <refmeta>
+    <refentrytitle>ioctl MEDIA_IOC_REQUEST_CMD</refentrytitle>
+    &manvol;
+  </refmeta>
+
+  <refnamediv>
+    <refname>MEDIA_IOC_REQUEST_CMD</refname>
+    <refpurpose>Manage media device requests</refpurpose>
+  </refnamediv>
+
+  <refsynopsisdiv>
+    <funcsynopsis>
+      <funcprototype>
+	<funcdef>int <function>ioctl</function></funcdef>
+	<paramdef>int <parameter>fd</parameter></paramdef>
+	<paramdef>int <parameter>request</parameter></paramdef>
+	<paramdef>struct media_request_cmd *<parameter>argp</parameter></paramdef>
+      </funcprototype>
+    </funcsynopsis>
+  </refsynopsisdiv>
+
+  <refsect1>
+    <title>Arguments</title>
+
+    <variablelist>
+      <varlistentry>
+	<term><parameter>fd</parameter></term>
+	<listitem>
+	  <para>File descriptor returned by
+	  <link linkend='media-func-open'><function>open()</function></link>.</para>
+	</listitem>
+      </varlistentry>
+      <varlistentry>
+	<term><parameter>request</parameter></term>
+	<listitem>
+	  <para>MEDIA_IOC_REQUEST_CMD</para>
+	</listitem>
+      </varlistentry>
+      <varlistentry>
+	<term><parameter>argp</parameter></term>
+	<listitem>
+	  <para></para>
+	</listitem>
+      </varlistentry>
+    </variablelist>
+  </refsect1>
+
+  <refsect1>
+    <title>Description</title>
+
+    <para>The MEDIA_IOC_REQUEST_CMD ioctl allow applications to manage media
+    device requests. A request is an object that can group media device
+    configuration parameters, including subsystem-specific parameters, in order
+    to apply all the parameters atomically. Applications are responsible for
+    allocating and deleting requests, filling them with configuration parameters
+    and (synchronously) applying or (asynchronously) queuing them.</para>
+
+    <para>Request operations are performed by calling the MEDIA_IOC_REQUEST_CMD
+    ioctl with a pointer to a &media-request-cmd; with the
+    <structfield>cmd</structfield> set to the appropriate command.
+    <xref linkend="media-request-commands" /> lists the commands supported by
+    the ioctl.</para>
+
+    <para>The &media-request-cmd; <structfield>request</structfield> field
+    contains the ID of the request on which the command operates. For the
+    <constant>MEDIA_REQ_CMD_ALLOC</constant> command the field is set to zero
+    by applications and filled by the driver. For all other commands the field
+    is set by applications and left untouched by the driver.</para>
+
+    <para>To allocate a new request applications use the
+    <constant>MEDIA_REQ_CMD_ALLOC</constant>. The driver will allocate a new
+    request and return its ID in the <structfield>request</structfield> field.
+    </para>
+
+    <para>Requests are reference-counted. A newly allocate request is referenced
+    by the media device file handled on which it has been created, and can be
+    later referenced by subsystem-specific operations using the request ID.
+    Requests will thus be automatically deleted when they're not longer used
+    after the media device file handle is closed.</para>
+
+    <para>If a request isn't needed applications can delete it using the
+    <constant>MEDIA_REQ_CMD_DELETE</constant> command. The driver will drop the
+    reference to the request stored in the media device file handle. The request
+    will not be usable through the MEDIA_IOC_REQUEST_CMD ioctl anymore, but will
+    only be deleted when the last reference is released. If no other reference
+    exists when the delete command is invoked the request will be deleted
+    immediately.</para>
+
+    <para>After creating a request applications should fill it with
+    configuration parameters. This is performed through subsystem-specific
+    request APIs outside the scope of the media controller API. See the
+    appropriate subsystem APIs for more information, including how they interact
+    with the MEDIA_IOC_REQUEST_CMD ioctl.</para>
+
+    <para>Once a request contains all the desired configuration parameters it
+    can be applied synchronously or queued asynchronously. To apply a request
+    applications use the <constant>MEDIA_REQ_CMD_APPLY</constant> command. The
+    driver will apply all configuration parameters stored in the request to the
+    device atomically. The ioctl will return once all parameters have been
+    applied, but it should be noted that they might not have fully taken effect
+    yet.</para>
+
+    <para>To queue a request applications use the
+    <constant>MEDIA_REQ_CMD_QUEUE</constant> command. The driver will queue the
+    request for processing and return immediately. The request will then be
+    processed and applied after all previously queued requests.</para>
+
+    <para>Once a request has been queued applications are not allowed to modify
+    its configuration parameters until the request has been fully processed.
+    Any attempt to do so will result in the related subsystem API returning
+    an error. The media device request API doesn't notify applications of
+    request processing completion, this is left to the other subsystems APIs to
+    implement.</para>
+
+    <table pgwide="1" frame="none" id="media-request-cmd">
+      <title>struct <structname>media_request_cmd</structname></title>
+      <tgroup cols="3">
+        &cs-str;
+	<tbody valign="top">
+	  <row>
+	    <entry>__u32</entry>
+	    <entry><structfield>cmd</structfield></entry>
+	    <entry>Command, set by the application. See
+	    <xref linkend="media-request-commands" /> for the list of supported
+	    commands.</entry>
+	  </row>
+	  <row>
+	    <entry>__u32</entry>
+	    <entry><structfield>request</structfield></entry>
+	    <entry>Request ID, set by the driver for the
+	    <constant>MEDIA_REQ_CMD_ALLOC</constant> and by the application
+	    for all other commands.</entry>
+	  </row>
+	</tbody>
+      </tgroup>
+    </table>
+
+    <table frame="none" pgwide="1" id="media-request-commands">
+      <title>Media request commands</title>
+      <tgroup cols="2">
+        <colspec colname="c1"/>
+        <colspec colname="c2"/>
+	<tbody valign="top">
+	  <row>
+	    <entry><constant>MEDIA_REQ_CMD_ALLOC</constant></entry>
+	    <entry>Allocate a new request.</entry>
+	  </row>
+	  <row>
+	    <entry><constant>MEDIA_REQ_CMD_DELETE</constant></entry>
+	    <entry>Delete a request.</entry>
+	  </row>
+	  <row>
+	    <entry><constant>MEDIA_REQ_CMD_APPLY</constant></entry>
+	    <entry>Apply all settings from a request.</entry>
+	  </row>
+	  <row>
+	    <entry><constant>MEDIA_REQ_CMD_QUEUE</constant></entry>
+	    <entry>Queue a request for processing.</entry>
+	  </row>
+	</tbody>
+      </tgroup>
+    </table>
+  </refsect1>
+
+  <refsect1>
+    &return-value;
+
+    <variablelist>
+      <varlistentry>
+	<term><errorcode>EINVAL</errorcode></term>
+	<listitem>
+	  <para>The &media-request-cmd; specifies an invalid command or
+	  references a non-existing request.
+	  </para>
+	</listitem>
+	<term><errorcode>ENOSYS</errorcode></term>
+	<listitem>
+	  <para>The &media-request-cmd; specifies the
+	  <constant>MEDIA_REQ_CMD_QUEUE</constant> or
+	  <constant>MEDIA_REQ_CMD_APPLY</constant> and that command isn't
+	  implemented by the device.
+	  </para>
+	</listitem>
+      </varlistentry>
+    </variablelist>
+  </refsect1>
+</refentry>
-- 
1.9.1


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

* [RFC 18/22] DocBook: media: Document the V4L2 request API
  2016-05-06 10:53 [RFC 00/22] Media request API Sakari Ailus
                   ` (16 preceding siblings ...)
  2016-05-06 10:53 ` [RFC 17/22] DocBook: media: Document the media request API Sakari Ailus
@ 2016-05-06 10:53 ` Sakari Ailus
  2016-05-06 10:53 ` [RFC 19/22] DocBook: media: Document the subdev selection API Sakari Ailus
                   ` (5 subsequent siblings)
  23 siblings, 0 replies; 28+ messages in thread
From: Sakari Ailus @ 2016-05-06 10:53 UTC (permalink / raw)
  To: linux-media; +Cc: laurent.pinchart, hverkuil, mchehab, Laurent Pinchart

From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

The V4L2 request API consists in extensions to existing V4L2 ioctls.
Document it.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 Documentation/DocBook/media/v4l/common.xml         |  2 +
 Documentation/DocBook/media/v4l/io.xml             |  9 ++-
 Documentation/DocBook/media/v4l/request-api.xml    | 90 ++++++++++++++++++++++
 .../DocBook/media/v4l/vidioc-prepare-buf.xml       |  9 +++
 Documentation/DocBook/media/v4l/vidioc-qbuf.xml    |  6 ++
 5 files changed, 113 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/DocBook/media/v4l/request-api.xml

diff --git a/Documentation/DocBook/media/v4l/common.xml b/Documentation/DocBook/media/v4l/common.xml
index 8b5e014..30cb0f2 100644
--- a/Documentation/DocBook/media/v4l/common.xml
+++ b/Documentation/DocBook/media/v4l/common.xml
@@ -1073,6 +1073,8 @@ dheight = format.fmt.pix.height;
 
   &sub-selection-api;
 
+  &sub-request-api;
+
   <section id="streaming-par">
     <title>Streaming Parameters</title>
 
diff --git a/Documentation/DocBook/media/v4l/io.xml b/Documentation/DocBook/media/v4l/io.xml
index e09025d..5695bc8 100644
--- a/Documentation/DocBook/media/v4l/io.xml
+++ b/Documentation/DocBook/media/v4l/io.xml
@@ -833,10 +833,13 @@ is the file descriptor associated with a DMABUF buffer.</entry>
 	  </row>
 	  <row>
 	    <entry>__u32</entry>
-	    <entry><structfield>reserved2</structfield></entry>
+	    <entry><structfield>request</structfield></entry>
 	    <entry></entry>
-	    <entry>A place holder for future extensions. Drivers and applications
-must set this to 0.</entry>
+	    <entry>ID of the request to associate the buffer to. Set by the
+	    application for &VIDIOC-QBUF; and &VIDIOC-PREPARE-BUF;. Set to zero
+	    by the application and the driver in all other cases. See
+	    <xref linkend="v4l2-requests" /> for more information.
+	    </entry>
 	  </row>
 	  <row>
 	    <entry>__u32</entry>
diff --git a/Documentation/DocBook/media/v4l/request-api.xml b/Documentation/DocBook/media/v4l/request-api.xml
new file mode 100644
index 0000000..992f25a
--- /dev/null
+++ b/Documentation/DocBook/media/v4l/request-api.xml
@@ -0,0 +1,90 @@
+<section id="v4l2-requests">
+
+  <title>Experimental API for request handling</title>
+
+  <note>
+    <title>Experimental</title>
+    <para>This is an <link linkend="experimental">experimental</link>
+    interface and may change in the future.</para>
+  </note>
+
+  <section>
+    <title>Introduction</title>
+
+<para>It is often useful to apply certain settings when a buffer is about to be
+filled by the DMA capture of a video capture device, ensuring that those
+settings are applied in time for them to be used with that buffer.</para>
+
+<para>One of the prime use-cases of this is Android's CameraHAL v3 which
+requires per-frame configuration support. Other use-cases are possible as well:
+changing codec settings (bit rate, etc.) starting with a specific buffer,
+preparing a configuration to be applied at a certain time, etc.</para>
+
+<para>The request API is the way V4L2 solves this problem.</para>
+
+  </section>
+
+  <section>
+    <title>Request Objects</title>
+
+<para>At the core of the request API is the request object. Applications store
+configuration parameters such as V4L2 controls, formats and selection rectangles
+in request objects and then associate those request objects for processing with
+specific buffers.</para>
+
+<para>Request objects are created and managed through the media controller
+device node. Details on request object management can be found in the
+<link linkend="media-ioc-request-cmd">media controller request API</link>
+documentation and are outside the scope of the V4L2 request API. Once a request
+object is created it can be referenced by its ID in the V4L2 ioctls that support
+requests.</para>
+
+<para>Applications can store controls, subdev formats and subdev selection
+rectangles in requests. To do so they use the usual V4L2 ioctls
+&VIDIOC-S-EXT-CTRLS;, &VIDIOC-SUBDEV-S-FMT; and &VIDIOC-SUBDEV-S-SELECTION; with
+the <structfield>request</structfield> field of the associated structure set to
+the request ID (for subdev formats and selection rectangles the
+<structfield>which</structfield> field need to be additionally set to
+<constant>V4L2_SUBDEV_FORMAT_REQUEST</constant>). Controls, formats and
+selection rectangles will be processed as usual but will be stored in the
+request instead of applied to the device.
+</para>
+
+<para>Parameters stored in requests can further be retrieved by calling the
+&VIDIOC-G-EXT-CTRLS;, &VIDIOC-SUBDEV-G-FMT; or &VIDIOC-SUBDEV-G-SELECTION;
+ioctls similarly with the <structfield>request</structfield> field of the
+associated structure set to the request ID.
+</para>
+
+  </section>
+
+  <section>
+    <title>Applying Requests</title>
+
+<para>The simplest way to apply a request is to associated it with a buffer.
+This is done by setting the <structfield>request</structfield> field of the
+&v4l2-buffer; to the request ID when queuing the buffer with the &VIDIOC-QBUF;
+ioctl.
+</para>
+
+<para>Once a buffer is queued with a non-zero request ID the driver will apply
+all parameters stored in the request atomically. The parameters are guaranteed
+to come in effect before the buffer starts being transferred and after all
+previous buffers have been complete.
+</para>
+
+<para>For devices with multiple video nodes requests might need to be applied
+synchronously with several buffers. This is achieved by first preparing (but not
+queuing) all the related buffers using the &VIDIOC-PREPARE-BUF; ioctl with the
+<structfield>request</structfield> field of the &v4l2-buffer; set to the request
+ID. Once this is done the request is queued using the
+<constant>MEDIA_REQ_CMD_QUEUE</constant> command of the &MEDIA-IOC-REQUEST-CMD;
+ioctl on the media controller device node. The driver will then queue all
+buffers prepared for the request as if the &VIDIOC-QBUF; ioctl was called on all
+of them and will apply the request parameters atomically and synchronously with
+the transfer of the buffers.
+</para>
+
+  </section>
+
+</section>
diff --git a/Documentation/DocBook/media/v4l/vidioc-prepare-buf.xml b/Documentation/DocBook/media/v4l/vidioc-prepare-buf.xml
index 7bde698..2c5b72c1 100644
--- a/Documentation/DocBook/media/v4l/vidioc-prepare-buf.xml
+++ b/Documentation/DocBook/media/v4l/vidioc-prepare-buf.xml
@@ -59,6 +59,15 @@ not required, the application can use one of
 <constant>V4L2_BUF_FLAG_NO_CACHE_CLEAN</constant> flags to skip the respective
 step.</para>
 
+    <para>Applications can prepare a buffer to be processed for a specific
+request. To do so they set the <structfield>request</structfield> field of the
+struct <structname>v4l2_buffer</structname> to the request ID. The buffer will
+then be automatically queued when the request is processed as if the
+<constant>VIDIOC_QBUF</constant> ioctl was called at that time by the
+application. For more information about requests see
+<xref linkend="v4l2-requests" />.
+</para>
+
     <para>The <structname>v4l2_buffer</structname> structure is
 specified in <xref linkend="buffer" />.</para>
   </refsect1>
diff --git a/Documentation/DocBook/media/v4l/vidioc-qbuf.xml b/Documentation/DocBook/media/v4l/vidioc-qbuf.xml
index 8b98a0e..742f1dd 100644
--- a/Documentation/DocBook/media/v4l/vidioc-qbuf.xml
+++ b/Documentation/DocBook/media/v4l/vidioc-qbuf.xml
@@ -80,6 +80,12 @@ to a filled-in array of &v4l2-plane; and the <structfield>length</structfield>
 field must be set to the number of elements in that array.
 </para>
 
+    <para>Applications can reference a request to be applied when the buffer is
+processed. To do so they set the <structfield>request</structfield> field of the
+struct <structname>v4l2_buffer</structname> to the request ID. For more
+information about requests see <xref linkend="v4l2-requests" />.
+</para>
+
     <para>To enqueue a <link linkend="mmap">memory mapped</link>
 buffer applications set the <structfield>memory</structfield>
 field to <constant>V4L2_MEMORY_MMAP</constant>. When
-- 
1.9.1


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

* [RFC 19/22] DocBook: media: Document the subdev selection API
  2016-05-06 10:53 [RFC 00/22] Media request API Sakari Ailus
                   ` (17 preceding siblings ...)
  2016-05-06 10:53 ` [RFC 18/22] DocBook: media: Document the V4L2 " Sakari Ailus
@ 2016-05-06 10:53 ` Sakari Ailus
  2016-05-06 10:53 ` [RFC 20/22] DocBook: media: Document the V4L2 subdev request API Sakari Ailus
                   ` (4 subsequent siblings)
  23 siblings, 0 replies; 28+ messages in thread
From: Sakari Ailus @ 2016-05-06 10:53 UTC (permalink / raw)
  To: linux-media; +Cc: laurent.pinchart, hverkuil, mchehab, Laurent Pinchart

From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

Now that the subdev crop API is considered obsolete, the selection API
that replaces it deserves a full documentation instead of referring to
the crop API documentation.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 .../media/v4l/vidioc-subdev-g-selection.xml        | 37 ++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/Documentation/DocBook/media/v4l/vidioc-subdev-g-selection.xml b/Documentation/DocBook/media/v4l/vidioc-subdev-g-selection.xml
index 8346b2e..faac955 100644
--- a/Documentation/DocBook/media/v4l/vidioc-subdev-g-selection.xml
+++ b/Documentation/DocBook/media/v4l/vidioc-subdev-g-selection.xml
@@ -63,6 +63,43 @@
     more information on how each selection target affects the image
     processing pipeline inside the subdevice.</para>
 
+    <para>To retrieve a current selection rectangle applications set the
+    <structfield>pad</structfield> field of a &v4l2-subdev-selection; to the
+    desired pad number as reported by the media API, the
+    <structfield>which</structfield> field to
+    <constant>V4L2_SUBDEV_FORMAT_ACTIVE</constant> and the
+    <structfield>target</structfield> to the target selection rectangle. They
+    then call the <constant>VIDIOC_SUBDEV_G_SELECTION</constant> ioctl with a
+    pointer to this structure. The driver fills the members of the
+    <structfield>r</structfield> field or returns &EINVAL; if the input
+    arguments are invalid, or if selection is not supported on the given pad.
+    </para>
+
+    <para>To change a current selection rectangle applications set the
+    <structfield>pad</structfield>, <structfield>which</structfield> and
+    <structfield>target</structfield> fields and all members of the
+    <structfield>r</structfield> field. They then call the
+    <constant>VIDIOC_SUBDEV_S_SELECTION</constant> ioctl with a pointer to this
+    structure. The driver verifies the requested selection rectangle, adjusts it
+    based on the hardware capabilities and configures the device. Upon return
+    the &v4l2-subdev-selection; contains the current selection rectangle as
+    would be returned by a <constant>VIDIOC_SUBDEV_G_SELECTION</constant> call.
+    </para>
+
+    <para>Applications can query the device capabilities by setting the
+    <structfield>which</structfield> to
+    <constant>V4L2_SUBDEV_FORMAT_TRY</constant>. When set, 'try' selection
+    rectangles are not applied to the device by the driver, but are mangled
+    exactly as active selection rectangles and stored in the sub-device file
+    handle. Two applications querying the same sub-device would thus not
+    interfere with each other.</para>
+
+    <para>Drivers must not return an error solely because the requested
+    selection rectangle doesn't match the device capabilities. They must instead
+    modify the rectangle to match what the hardware can provide. The modified
+    selection rectangle should be as close as possible to the original request.
+    </para>
+
     <refsect2>
       <title>Types of selection targets</title>
 
-- 
1.9.1


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

* [RFC 20/22] DocBook: media: Document the V4L2 subdev request API
  2016-05-06 10:53 [RFC 00/22] Media request API Sakari Ailus
                   ` (18 preceding siblings ...)
  2016-05-06 10:53 ` [RFC 19/22] DocBook: media: Document the subdev selection API Sakari Ailus
@ 2016-05-06 10:53 ` Sakari Ailus
  2016-05-06 10:53 ` [RFC 21/22] DocBook: media: Document media events (MEDIA_IOC_DQEVENT IOCTL) Sakari Ailus
                   ` (3 subsequent siblings)
  23 siblings, 0 replies; 28+ messages in thread
From: Sakari Ailus @ 2016-05-06 10:53 UTC (permalink / raw)
  To: linux-media; +Cc: laurent.pinchart, hverkuil, mchehab, Laurent Pinchart

From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

The V4L2 subdev request API consists in extensions to existing V4L2
subdev ioctls. Document it.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 .../DocBook/media/v4l/vidioc-subdev-g-fmt.xml      | 27 +++++++++++++++++++---
 .../media/v4l/vidioc-subdev-g-selection.xml        | 24 +++++++++++++++----
 2 files changed, 44 insertions(+), 7 deletions(-)

diff --git a/Documentation/DocBook/media/v4l/vidioc-subdev-g-fmt.xml b/Documentation/DocBook/media/v4l/vidioc-subdev-g-fmt.xml
index 781089c..5cf6d89 100644
--- a/Documentation/DocBook/media/v4l/vidioc-subdev-g-fmt.xml
+++ b/Documentation/DocBook/media/v4l/vidioc-subdev-g-fmt.xml
@@ -91,6 +91,13 @@
     low-pass noise filter might crop pixels at the frame boundaries, modifying
     its output frame size.</para>
 
+    <para>Applications can get and set formats stored in a request by setting
+    the <structfield>which</structfield> field to
+    <constant>V4L2_SUBDEV_FORMAT_REQUEST</constant> and the
+    <structfield>request</structfield> to the request ID. See
+    <xref linkend="v4l2-requests" /> for more information about the request
+    API.</para>
+
     <para>Drivers must not return an error solely because the requested format
     doesn't match the device capabilities. They must instead modify the format
     to match what the hardware can provide. The modified format should be as
@@ -119,7 +126,15 @@
 	  </row>
 	  <row>
 	    <entry>__u32</entry>
-	    <entry><structfield>reserved</structfield>[8]</entry>
+	    <entry><structfield>request</structfield></entry>
+	    <entry>Request ID, only valid when the <structfield>which</structfield>
+	    field is set to <constant>V4L2_SUBDEV_FORMAT_REQUEST</constant>.
+	    Applications and drivers must set the field to zero in all other
+	    cases.</entry>
+	  </row>
+	  <row>
+	    <entry>__u32</entry>
+	    <entry><structfield>reserved</structfield>[7]</entry>
 	    <entry>Reserved for future extensions. Applications and drivers must
 	    set the array to zero.</entry>
 	  </row>
@@ -142,6 +157,11 @@
 	    <entry>1</entry>
 	    <entry>Active formats, applied to the hardware.</entry>
 	  </row>
+	  <row>
+	    <entry>V4L2_SUBDEV_FORMAT_REQUEST</entry>
+	    <entry>2</entry>
+	    <entry>Request formats, used with the requests API.</entry>
+	  </row>
 	</tbody>
       </tgroup>
     </table>
@@ -165,8 +185,9 @@
 	<term><errorcode>EINVAL</errorcode></term>
 	<listitem>
 	  <para>The &v4l2-subdev-format; <structfield>pad</structfield>
-	  references a non-existing pad, or the <structfield>which</structfield>
-	  field references a non-existing format.</para>
+	  references a non-existing pad, the <structfield>which</structfield>
+	  field references a non-existing format or the request ID references
+	  a nonexistant request.</para>
 	</listitem>
       </varlistentry>
     </variablelist>
diff --git a/Documentation/DocBook/media/v4l/vidioc-subdev-g-selection.xml b/Documentation/DocBook/media/v4l/vidioc-subdev-g-selection.xml
index faac955..c0fbfbe 100644
--- a/Documentation/DocBook/media/v4l/vidioc-subdev-g-selection.xml
+++ b/Documentation/DocBook/media/v4l/vidioc-subdev-g-selection.xml
@@ -94,6 +94,13 @@
     handle. Two applications querying the same sub-device would thus not
     interfere with each other.</para>
 
+    <para>Applications can get and set selection rectangles stored in a request
+    by setting the <structfield>which</structfield> field to
+    <constant>V4L2_SUBDEV_FORMAT_REQUEST</constant> and the
+    <structfield>request</structfield> to the request ID. See
+    <xref linkend="v4l2-requests" /> for more information about the request
+    API.</para>
+
     <para>Drivers must not return an error solely because the requested
     selection rectangle doesn't match the device capabilities. They must instead
     modify the rectangle to match what the hardware can provide. The modified
@@ -128,7 +135,7 @@
 	  <row>
 	    <entry>__u32</entry>
 	    <entry><structfield>which</structfield></entry>
-	    <entry>Active or try selection, from
+	    <entry>Selection to be modified, from
 	    &v4l2-subdev-format-whence;.</entry>
 	  </row>
 	  <row>
@@ -155,7 +162,15 @@
 	  </row>
 	  <row>
 	    <entry>__u32</entry>
-	    <entry><structfield>reserved</structfield>[8]</entry>
+	    <entry><structfield>request</structfield></entry>
+	    <entry>Request ID, only valid when the <structfield>which</structfield>
+	    field is set to <constant>V4L2_SUBDEV_FORMAT_REQUEST</constant>.
+	    Applications and drivers must set the field to zero in all other
+	    cases.</entry>
+	  </row>
+	  <row>
+	    <entry>__u32</entry>
+	    <entry><structfield>reserved</structfield>[7]</entry>
 	    <entry>Reserved for future extensions. Applications and drivers must
 	    set the array to zero.</entry>
 	  </row>
@@ -187,8 +202,9 @@
 	  <para>The &v4l2-subdev-selection;
 	  <structfield>pad</structfield> references a non-existing
 	  pad, the <structfield>which</structfield> field references a
-	  non-existing format, or the selection target is not
-	  supported on the given subdev pad.</para>
+	  non-existing format, the selection target is not supported on
+	  the given subdev pad or the request ID references a nonexistant
+	  request.</para>
 	</listitem>
       </varlistentry>
     </variablelist>
-- 
1.9.1


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

* [RFC 21/22] DocBook: media: Document media events (MEDIA_IOC_DQEVENT IOCTL)
  2016-05-06 10:53 [RFC 00/22] Media request API Sakari Ailus
                   ` (19 preceding siblings ...)
  2016-05-06 10:53 ` [RFC 20/22] DocBook: media: Document the V4L2 subdev request API Sakari Ailus
@ 2016-05-06 10:53 ` Sakari Ailus
  2016-05-06 10:53 ` [RFC 22/22] DocBook: media: Document request flags Sakari Ailus
                   ` (2 subsequent siblings)
  23 siblings, 0 replies; 28+ messages in thread
From: Sakari Ailus @ 2016-05-06 10:53 UTC (permalink / raw)
  To: linux-media; +Cc: laurent.pinchart, hverkuil, mchehab

Add DocBook documentation for media events.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 .../DocBook/media/v4l/media-controller.xml         |   1 +
 .../DocBook/media/v4l/media-ioc-dqevent.xml        | 155 +++++++++++++++++++++
 2 files changed, 156 insertions(+)
 create mode 100644 Documentation/DocBook/media/v4l/media-ioc-dqevent.xml

diff --git a/Documentation/DocBook/media/v4l/media-controller.xml b/Documentation/DocBook/media/v4l/media-controller.xml
index 2a5a5d0..aa78fcb 100644
--- a/Documentation/DocBook/media/v4l/media-controller.xml
+++ b/Documentation/DocBook/media/v4l/media-controller.xml
@@ -98,6 +98,7 @@
   &sub-media-func-ioctl;
   <!-- All ioctls go here. -->
   &sub-media-ioc-device-info;
+  &sub-media-ioc-dqevent;
   &sub-media-ioc-g-topology;
   &sub-media-ioc-enum-entities;
   &sub-media-ioc-enum-links;
diff --git a/Documentation/DocBook/media/v4l/media-ioc-dqevent.xml b/Documentation/DocBook/media/v4l/media-ioc-dqevent.xml
new file mode 100644
index 0000000..7d58491
--- /dev/null
+++ b/Documentation/DocBook/media/v4l/media-ioc-dqevent.xml
@@ -0,0 +1,155 @@
+<refentry id="media-ioc-dqevent">
+  <refmeta>
+    <refentrytitle>ioctl MEDIA_IOC_DQEVENT</refentrytitle>
+    &manvol;
+  </refmeta>
+
+  <refnamediv>
+    <refname>MEDIA_IOC_DQEVENT</refname>
+    <refpurpose>Dequeue a media event</refpurpose>
+  </refnamediv>
+
+  <refsynopsisdiv>
+    <funcsynopsis>
+      <funcprototype>
+	<funcdef>int <function>ioctl</function></funcdef>
+	<paramdef>int <parameter>fd</parameter></paramdef>
+	<paramdef>int <parameter>request</parameter></paramdef>
+	<paramdef>struct media_event *<parameter>argp</parameter></paramdef>
+      </funcprototype>
+    </funcsynopsis>
+  </refsynopsisdiv>
+
+  <refsect1>
+    <title>Arguments</title>
+
+    <variablelist>
+      <varlistentry>
+	<term><parameter>fd</parameter></term>
+	<listitem>
+	  <para>File descriptor returned by
+	  <link linkend='media-func-open'><function>open()</function></link>.</para>
+	</listitem>
+      </varlistentry>
+      <varlistentry>
+	<term><parameter>request</parameter></term>
+	<listitem>
+	  <para>MEDIA_IOC_DQEVENT</para>
+	</listitem>
+      </varlistentry>
+      <varlistentry>
+	<term><parameter>argp</parameter></term>
+	<listitem>
+	  <para></para>
+	</listitem>
+      </varlistentry>
+    </variablelist>
+  </refsect1>
+
+  <refsect1>
+    <title>Description</title>
+
+    <para>Dequeue a media event from a media device. Both non-blocking
+    and blocking access is supported. <constant>poll</constant>(2)
+    IOCTL may be used with poll event type
+    <constant>POLLPRI</constant> to learn about dequeueable
+    events.</para>
+
+    <para>Media events are specific to file handle: they are delivered
+    to and dequeued from each file handle separately.</para>
+
+    <table pgwide="1" frame="none" id="media-event">
+      <title>struct <structname>media_event</structname></title>
+      <tgroup cols="4">
+        &cs-str;
+	<tbody valign="top">
+	  <row>
+	    <entry>__u32</entry>
+	    <entry><structfield>type</structfield></entry>
+	    <entry></entry>
+	    <entry>Type of the media event. Set by the driver. See
+	    <xref linkend="media-event-type" /> for available media
+	    event types.</entry>
+	  </row>
+	  <row>
+	    <entry>__u32</entry>
+	    <entry><structfield>sequence</structfield></entry>
+	    <entry></entry>
+	    <entry>Event sequence number. The sequence number is file
+	    handle specific and counts from zero until it wraps around
+	    after reaching 32^2-1.</entry>
+	  </row>
+	  <row>
+	    <entry>union</entry>
+	    <entry><structfield></structfield></entry>
+	    <entry></entry>
+	    <entry>Anonymous union for event type specific data.</entry>
+	  </row>
+	  <row>
+	    <entry></entry>
+	    <entry>struct &media_event_request_complete;</entry>
+	    <entry><structfield>req_complete</structfield></entry>
+	    <entry>Event data for
+	    <constant>MEDIA_EVENT_REQUEST_COMPLETE</constant> event.
+	    </entry>
+	  </row>
+	</tbody>
+      </tgroup>
+    </table>
+
+    <table frame="none" pgwide="1" id="media-event-type">
+      <title>Media event types</title>
+      <tgroup cols="3">
+	&cs-def;
+	<tbody valign="top">
+	  <row>
+	    <entry><constant>MEDIA_EVENT_REQUEST_COMPLETE</constant></entry>
+	    <entry>1</entry>
+	    <entry>A request has been completed. This media event type
+	    has &media-event-request-complete; associated with it. The
+	    event is only queued to the file handle from which the
+	    event was queued.
+	    </entry>
+	  </row>
+	</tbody>
+      </tgroup>
+    </table>
+
+    <table pgwide="1" frame="none" id="media-event-request-complete">
+      <title>struct <structname>media_event_request_complete</structname></title>
+      <tgroup cols="3">
+        &cs-str;
+	<tbody valign="top">
+	  <row>
+	    <entry>__u32</entry>
+	    <entry><structfield>id</structfield></entry>
+	    <entry>Request ID. The identifier of the request which has
+	    been completed.</entry>
+	  </row>
+	  <row>
+	    <entry>__s32</entry>
+	    <entry><structfield>status</structfield></entry>
+	    <entry>Negative error code of the completed request. See errno(2)
+	    for possible error codes.</entry>
+	  </row>
+	</tbody>
+      </tgroup>
+    </table>
+
+  </refsect1>
+
+  <refsect1>
+    &return-value;
+
+    <variablelist>
+      <varlistentry>
+	<term><errorcode>ENOENT</errorcode></term>
+	<listitem>
+	  <para>No events are available for dequeueing. This is returned
+	  only when non-blocking I/O is used.
+	  </para>
+	</listitem>
+      </varlistentry>
+    </variablelist>
+  </refsect1>
+</refentry>
-- 
1.9.1


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

* [RFC 22/22] DocBook: media: Document request flags
  2016-05-06 10:53 [RFC 00/22] Media request API Sakari Ailus
                   ` (20 preceding siblings ...)
  2016-05-06 10:53 ` [RFC 21/22] DocBook: media: Document media events (MEDIA_IOC_DQEVENT IOCTL) Sakari Ailus
@ 2016-05-06 10:53 ` Sakari Ailus
  2016-05-06 15:25 ` [RFC 00/22] Media request API Sakari Ailus
  2016-05-17  8:05 ` Sakari Ailus
  23 siblings, 0 replies; 28+ messages in thread
From: Sakari Ailus @ 2016-05-06 10:53 UTC (permalink / raw)
  To: linux-media; +Cc: laurent.pinchart, hverkuil, mchehab

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 .../DocBook/media/v4l/media-ioc-request-cmd.xml    | 24 ++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/Documentation/DocBook/media/v4l/media-ioc-request-cmd.xml b/Documentation/DocBook/media/v4l/media-ioc-request-cmd.xml
index 4f4acea..14c0068 100644
--- a/Documentation/DocBook/media/v4l/media-ioc-request-cmd.xml
+++ b/Documentation/DocBook/media/v4l/media-ioc-request-cmd.xml
@@ -132,6 +132,13 @@
 	    <constant>MEDIA_REQ_CMD_ALLOC</constant> and by the application
 	    for all other commands.</entry>
 	  </row>
+	  <row>
+	    <entry>__u32</entry>
+	    <entry><structfield>flags</structfield></entry>
+	    <entry>Flags related to a request. See <xref
+	    linkend="media-request-flags" /> for the list of
+	    flags.</entry>
+	  </row>
 	</tbody>
       </tgroup>
     </table>
@@ -161,6 +168,23 @@
 	</tbody>
       </tgroup>
     </table>
+
+    <table frame="none" pgwide="1" id="media-request-flags">
+      <title>Media request flags</title>
+      <tgroup cols="2">
+        <colspec colname="c1"/>
+        <colspec colname="c2"/>
+	<tbody valign="top">
+	  <row>
+	    <entry><constant>MEDIA_REQ_FL_COMPLETE_EVENT</constant></entry>
+	    <entry>Queue and event to the file handle on request
+	    completion. This flag is relevant for
+	    <constant>MEDIA_REQ_CMD_APPLY</constant> and
+	    <constant>MEDIA_REQ_CMD_QUEUE</constant> commands.</entry>
+	  </row>
+	</tbody>
+      </tgroup>
+    </table>
   </refsect1>
 
   <refsect1>
-- 
1.9.1


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

* Re: [RFC 15/22] media: Make events on request completion optional, disabled by default
  2016-05-06 10:53 ` [RFC 15/22] media: Make events on request completion optional, disabled by default Sakari Ailus
@ 2016-05-06 11:05   ` Sakari Ailus
  0 siblings, 0 replies; 28+ messages in thread
From: Sakari Ailus @ 2016-05-06 11:05 UTC (permalink / raw)
  To: Sakari Ailus; +Cc: linux-media, laurent.pinchart, hverkuil, mchehab

On Fri, May 06, 2016 at 01:53:24PM +0300, Sakari Ailus wrote:
> Add flags to requests. The first defined flag, MEDIA_REQ_FL_COMPLETE_EVENT
> is used to tell whether to queue an event on request completion. Unless
> the flag is set, no event is generated when a request completes.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>

This one nicely demonstrates variable size IOCTL argument support so I left
it there for the time being. To be merged with the previous patch in the
future, with struct media_request_cmd_0 removed.

> ---
>  drivers/media/media-device.c | 26 +++++++++++++++++---------
>  include/media/media-device.h |  2 ++
>  include/uapi/linux/media.h   | 16 +++++++++++++---
>  3 files changed, 32 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
> index 357c3cb..0b1ab88 100644
> --- a/drivers/media/media-device.c
> +++ b/drivers/media/media-device.c
> @@ -284,7 +284,8 @@ static void media_device_request_delete(struct media_device *mdev,
>  void media_device_request_complete(struct media_device *mdev,
>  				   struct media_device_request *req)
>  {
> -	media_device_request_queue_event(mdev, req);
> +	if (req->flags & MEDIA_REQ_FL_COMPLETE_EVENT)
> +		media_device_request_queue_event(mdev, req);
>  
>  	media_device_request_delete(mdev, req);
>  }
> @@ -292,8 +293,8 @@ EXPORT_SYMBOL_GPL(media_device_request_complete);
>  
>  static int media_device_request_queue_apply(
>  	struct media_device *mdev, struct media_device_request *req,
> -	int (*fn)(struct media_device *mdev,
> -		  struct media_device_request *req))
> +	u32 req_flags, int (*fn)(struct media_device *mdev,
> +				 struct media_device_request *req))
>  {
>  	unsigned long flags;
>  	int rval = 0;
> @@ -302,10 +303,12 @@ static int media_device_request_queue_apply(
>  		return -ENOSYS;
>  
>  	spin_lock_irqsave(&mdev->req_lock, flags);
> -	if (req->state != MEDIA_DEVICE_REQUEST_STATE_IDLE)
> +	if (req->state != MEDIA_DEVICE_REQUEST_STATE_IDLE) {
>  		rval = -EINVAL;
> -	else
> +	} else {
>  		req->state = MEDIA_DEVICE_REQUEST_STATE_QUEUED;
> +		req->flags = req_flags;
> +	}
>  	spin_unlock_irqrestore(&mdev->req_lock, flags);
>  
>  	if (rval)
> @@ -358,12 +361,12 @@ static long media_device_request_cmd(struct media_device *mdev,
>  		break;
>  
>  	case MEDIA_REQ_CMD_APPLY:
> -		ret = media_device_request_queue_apply(mdev, req,
> +		ret = media_device_request_queue_apply(mdev, req, cmd->flags,
>  						       mdev->ops->req_apply);
>  		break;
>  
>  	case MEDIA_REQ_CMD_QUEUE:
> -		ret = media_device_request_queue_apply(mdev, req,
> +		ret = media_device_request_queue_apply(mdev, req, cmd->flags,
>  						       mdev->ops->req_queue);
>  		break;
>  
> @@ -930,13 +933,18 @@ static long __media_device_ioctl(
>  	return ret;
>  }
>  
> +static const unsigned short media_request_cmd_sizes[] = {
> +	sizeof(struct media_request_cmd_0),
> +	0
> +};
> +
>  static const struct media_ioctl_info ioctl_info[] = {
>  	MEDIA_IOC(DEVICE_INFO, media_device_get_info, MEDIA_IOC_FL_GRAPH_MUTEX),
>  	MEDIA_IOC(ENUM_ENTITIES, media_device_enum_entities, MEDIA_IOC_FL_GRAPH_MUTEX),
>  	MEDIA_IOC(ENUM_LINKS, media_device_enum_links, MEDIA_IOC_FL_GRAPH_MUTEX),
>  	MEDIA_IOC(SETUP_LINK, media_device_setup_link, MEDIA_IOC_FL_GRAPH_MUTEX),
>  	MEDIA_IOC(G_TOPOLOGY, media_device_get_topology, MEDIA_IOC_FL_GRAPH_MUTEX),
> -	MEDIA_IOC(REQUEST_CMD, media_device_request_cmd, 0),
> +	MEDIA_IOC_SZ(REQUEST_CMD, media_device_request_cmd, 0, media_request_cmd_sizes),
>  	MEDIA_IOC(DQEVENT, media_device_dqevent, 0),
>  };
>  
> @@ -985,7 +993,7 @@ static const struct media_ioctl_info compat_ioctl_info[] = {
>  	MEDIA_IOC_ARG(ENUM_LINKS32, media_device_enum_links, MEDIA_IOC_FL_GRAPH_MUTEX, from_user_enum_links32, copy_arg_to_user_nop),
>  	MEDIA_IOC(SETUP_LINK, media_device_setup_link, MEDIA_IOC_FL_GRAPH_MUTEX),
>  	MEDIA_IOC(G_TOPOLOGY, media_device_get_topology, MEDIA_IOC_FL_GRAPH_MUTEX),
> -	MEDIA_IOC(REQUEST_CMD, media_device_request_cmd, 0),
> +	MEDIA_IOC_SZ(REQUEST_CMD, media_device_request_cmd, 0, media_request_cmd_sizes),
>  	MEDIA_IOC(DQEVENT, media_device_dqevent, 0),
>  };
>  
> diff --git a/include/media/media-device.h b/include/media/media-device.h
> index e62ad13..fc91d2d 100644
> --- a/include/media/media-device.h
> +++ b/include/media/media-device.h
> @@ -291,6 +291,7 @@ struct media_kevent {
>   * @state: The state of the request, MEDIA_DEVICE_REQUEST_STATE_*,
>   *	   access to state serialised by mdev->req_lock
>   * @data: Per-entity data list
> + * @flags: Request specific flags, MEDIA_REQ_FL_*
>   */
>  struct media_device_request {
>  	u32 id;
> @@ -302,6 +303,7 @@ struct media_device_request {
>  	struct list_head fh_list;
>  	enum media_device_request_state state;
>  	struct list_head data;
> +	u32 flags;
>  };
>  
>  /**
> diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
> index f6e1efe..4fab54d 100644
> --- a/include/uapi/linux/media.h
> +++ b/include/uapi/linux/media.h
> @@ -394,26 +394,36 @@ struct media_v2_topology {
>  #define MEDIA_REQ_CMD_APPLY		2
>  #define MEDIA_REQ_CMD_QUEUE		3
>  
> +#define MEDIA_REQ_FL_COMPLETE_EVENT	(1 << 0)
> +
> +#ifdef __KERNEL__
> +struct __attribute__ ((packed)) media_request_cmd_0 {
> +	__u32 cmd;
> +	__u32 request;
> +};
> +#endif
> +
>  struct __attribute__ ((packed)) media_request_cmd {
>  	__u32 cmd;
>  	__u32 request;
> +	__u32 flags;
>  };
>  
>  #define MEDIA_EVENT_TYPE_REQUEST_COMPLETE	1
>  
> -struct media_event_request_complete {
> +struct __attribute__ ((packed)) media_event_request_complete {
>  	__u32 id;
>  	__s32 status;
>  };
>  
> -struct media_event {
> +struct __attribute__ ((packed)) media_event {
>  	__u32 type;
>  	__u32 sequence;
>  
>  	union {
>  		struct media_event_request_complete req_complete;
>  	};
> -} __attribute__ ((packed));
> +};
>  
>  #define MEDIA_IOC_DEVICE_INFO		_IOWR('|', 0x00, struct media_device_info)
>  #define MEDIA_IOC_ENUM_ENTITIES		_IOWR('|', 0x01, struct media_entity_desc)
> -- 
> 1.9.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

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

* Re: [RFC 00/22] Media request API
  2016-05-06 10:53 [RFC 00/22] Media request API Sakari Ailus
                   ` (21 preceding siblings ...)
  2016-05-06 10:53 ` [RFC 22/22] DocBook: media: Document request flags Sakari Ailus
@ 2016-05-06 15:25 ` Sakari Ailus
  2016-05-17  8:05 ` Sakari Ailus
  23 siblings, 0 replies; 28+ messages in thread
From: Sakari Ailus @ 2016-05-06 15:25 UTC (permalink / raw)
  To: Sakari Ailus; +Cc: linux-media, laurent.pinchart, hverkuil, mchehab

On Fri, May 06, 2016 at 01:53:09PM +0300, Sakari Ailus wrote:
> My open questions and to-do items I'm aware of:
> 
> - Global ID space vs. file handles. Should requests be referred to by
>   global IDs or file handles specific to a process? V4L2 uses IDs but V4L2
>   devices (other than m2m) can meaningfully be used only by a single
>   program (or co-operative programs) at a time whereas the media device
>   could conceivably be accessed and used for different purposes by
>   multiple programs at the same time as the resources accessible through
>   the media device are numerous and often independent of each other. A
>   badly behaving application could disturb other applications using the
>   same media device even if no resource conflict exist by accessing the
>   same request IDs than other applications.
> 
> - Request completion and deletion. Should completed requests be deleted
>   automatically, or should the request return to an "empty" state after
>   completion? Applications typically have to create a new request right
>   after a completion of an earlier one, and sparing one additional IOCTL
>   call would be nice. (In current implementation the requests are deleted
>   in completion, but this would be a trivial change.)
> 
> - Video buffers vs. requests. In the current implementation, I'm using
>   VIDIOC_QBUF() from user space to associate buffers with requests. This
>   is convenient in drivers since the only extra steps to support requests
>   (vs. operation without requests) is to find the request and not really
>   queueing the buffer yet. What's noteworthy as well that the VB2 buffer
>   is in correct state after this when the request is queued.
> 
> - Subsystem framework specific request information. The requests are about
>   the media device, and struct media_device_request is free of e.g. V4L2
>   related information. Adding a pointer to V4L2 related data would make it
>   easier to add request handling related functionality to the V4L2
>   framework.
> 
> - MEDIA_IOC_REQUEST_CMD + (ALLOC/DELETE/QUEUE/APPLY) vs.
>   MEDIA_IOC_REQUEST_(ALLOC/DELETE/QUEUE/APPLY). Should we continue to have
>   a single IOCTL on the media device for requests, or should each
>   operation on a request have a distinct IOCTL? The current implementation
>   has a single IOCTL.
> 
> - VB2 queues are quite self-sufficient at the moment. Starting start in a
>   queue requires at least one queued buffer whereas a stream containing
>   multiple queues using requests could start e.g. by having a single
>   buffer in a request for three streaming queues. The functionality would
>   need to be relaxed to properly support requests.
> 
> - Limit number of allocated requests and dequeueable events to prevent
>   unintentional or intentional system memory exhaustion.

One matter to add here:

- Serialisation. How should access to various objects be handled? There
  might not be bugs in the current code, but supporting requests changing
  the Media device (graph) state would definitely benefit from abolishing
  the big graph_mutex.

-- 
Regards,

Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

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

* Re: [RFC 08/22] videodev2.h: Add request field to v4l2_pix_format_mplane
  2016-05-06 10:53 ` [RFC 08/22] videodev2.h: Add request field to v4l2_pix_format_mplane Sakari Ailus
@ 2016-05-06 16:33   ` Nicolas Dufresne
  2016-05-06 21:48     ` Sakari Ailus
  0 siblings, 1 reply; 28+ messages in thread
From: Nicolas Dufresne @ 2016-05-06 16:33 UTC (permalink / raw)
  To: Sakari Ailus, linux-media
  Cc: laurent.pinchart, hverkuil, mchehab, Laurent Pinchart

Le vendredi 06 mai 2016 à 13:53 +0300, Sakari Ailus a écrit :
> From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> 
> Let userspace specify a request ID when getting or setting formats.
> The
> support is limited to the multi-planar API at the moment, extending
> it
> to the single-planar API is possible if needed.
> 
> From a userspace point of view the API change is also minimized and
> doesn't require any new ioctl.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboar
> d.com>
> ---
>  include/uapi/linux/videodev2.h | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/include/uapi/linux/videodev2.h
> b/include/uapi/linux/videodev2.h
> index ac28299..6260d0e 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -1972,6 +1972,7 @@ struct v4l2_plane_pix_format {
>   * @ycbcr_enc:		enum v4l2_ycbcr_encoding, Y'CbCr
> encoding
>   * @quantization:	enum v4l2_quantization, colorspace
> quantization
>   * @xfer_func:		enum v4l2_xfer_func, colorspace
> transfer function
> + * @request:		request ID
>   */
>  struct v4l2_pix_format_mplane {
>  	__u32				width;
> @@ -1986,7 +1987,8 @@ struct v4l2_pix_format_mplane {
>  	__u8				ycbcr_enc;
>  	__u8				quantization;
>  	__u8				xfer_func;
> -	__u8				reserved[7];
> +	__u8				reserved[3];
> +	__u32				request;

Shouldn't the request member be added before the padding ?

>  } __attribute__ ((packed));
>  
>  /**

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

* Re: [RFC 08/22] videodev2.h: Add request field to v4l2_pix_format_mplane
  2016-05-06 16:33   ` Nicolas Dufresne
@ 2016-05-06 21:48     ` Sakari Ailus
  0 siblings, 0 replies; 28+ messages in thread
From: Sakari Ailus @ 2016-05-06 21:48 UTC (permalink / raw)
  To: nicolas
  Cc: Sakari Ailus, linux-media, laurent.pinchart, hverkuil, mchehab,
	Laurent Pinchart

Hi Nicolas,

On Fri, May 06, 2016 at 12:33:11PM -0400, Nicolas Dufresne wrote:
> Le vendredi 06 mai 2016 à 13:53 +0300, Sakari Ailus a écrit :
> > From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > 
> > Let userspace specify a request ID when getting or setting formats.
> > The
> > support is limited to the multi-planar API at the moment, extending
> > it
> > to the single-planar API is possible if needed.
> > 
> > From a userspace point of view the API change is also minimized and
> > doesn't require any new ioctl.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboar
> > d.com>
> > ---
> >  include/uapi/linux/videodev2.h | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/include/uapi/linux/videodev2.h
> > b/include/uapi/linux/videodev2.h
> > index ac28299..6260d0e 100644
> > --- a/include/uapi/linux/videodev2.h
> > +++ b/include/uapi/linux/videodev2.h
> > @@ -1972,6 +1972,7 @@ struct v4l2_plane_pix_format {
> >   * @ycbcr_enc:		enum v4l2_ycbcr_encoding, Y'CbCr
> > encoding
> >   * @quantization:	enum v4l2_quantization, colorspace
> > quantization
> >   * @xfer_func:		enum v4l2_xfer_func, colorspace
> > transfer function
> > + * @request:		request ID
> >   */
> >  struct v4l2_pix_format_mplane {
> >  	__u32				width;
> > @@ -1986,7 +1987,8 @@ struct v4l2_pix_format_mplane {
> >  	__u8				ycbcr_enc;
> >  	__u8				quantization;
> >  	__u8				xfer_func;
> > -	__u8				reserved[7];
> > +	__u8				reserved[3];
> > +	__u32				request;
> 
> Shouldn't the request member be added before the padding ?

In this case, no. The reason is that struct fields are aligned in most ABIs
(and to my knowledge, all ABIs Linux supports by default work that way, some
have looser limitations) to the size of the field. The struct is packed so
unaligned access would work, too, but it'd be more expensive.

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

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

* Re: [RFC 00/22] Media request API
  2016-05-06 10:53 [RFC 00/22] Media request API Sakari Ailus
                   ` (22 preceding siblings ...)
  2016-05-06 15:25 ` [RFC 00/22] Media request API Sakari Ailus
@ 2016-05-17  8:05 ` Sakari Ailus
  23 siblings, 0 replies; 28+ messages in thread
From: Sakari Ailus @ 2016-05-17  8:05 UTC (permalink / raw)
  To: Sakari Ailus; +Cc: linux-media, laurent.pinchart, hverkuil, mchehab

On Fri, May 06, 2016 at 01:53:09PM +0300, Sakari Ailus wrote:
> Hello all,
> 
> Here's a set of patches to implement support for the request API that has
> been discussed many, many times over the past few years. Some of the
> patches are my own while some have been written by Laurent and Hans.

In case someone is interested in these patches, I have updated ones here:

<URL:http://git.retiisi.org.uk/?p=~sailus/linux.git;a=shortlog;h=refs/heads/request>

I will post them again at a later time. For now, there are bugfixes in list
handling and more.

-- 
Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

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

end of thread, other threads:[~2016-05-17  8:05 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-06 10:53 [RFC 00/22] Media request API Sakari Ailus
2016-05-06 10:53 ` [RFC 01/22] media: Add " Sakari Ailus
2016-05-06 10:53 ` [RFC 02/22] media: Add media device request state Sakari Ailus
2016-05-06 10:53 ` [RFC 03/22] media: Prevent queueing queued requests Sakari Ailus
2016-05-06 10:53 ` [RFC 04/22] media: Only requests in IDLE state may be deleted Sakari Ailus
2016-05-06 10:53 ` [RFC 05/22] media: Add media_device_request_complete() to mark requests complete Sakari Ailus
2016-05-06 10:53 ` [RFC 06/22] media: Add per-entity request data support Sakari Ailus
2016-05-06 10:53 ` [RFC 07/22] videodev2.h: Add request field to v4l2_buffer Sakari Ailus
2016-05-06 10:53 ` [RFC 08/22] videodev2.h: Add request field to v4l2_pix_format_mplane Sakari Ailus
2016-05-06 16:33   ` Nicolas Dufresne
2016-05-06 21:48     ` Sakari Ailus
2016-05-06 10:53 ` [RFC 09/22] v4l2-subdev.h: Add request field to format and selection structures Sakari Ailus
2016-05-06 10:53 ` [RFC 10/22] v4l: Support the request API in format operations Sakari Ailus
2016-05-06 10:53 ` [RFC 11/22] v4l: subdev: Support the request API in format and selection operations Sakari Ailus
2016-05-06 10:53 ` [RFC 12/22] vb2: Add allow_requests flag Sakari Ailus
2016-05-06 10:53 ` [RFC 13/22] vb2: Add helper function to check for request buffers Sakari Ailus
2016-05-06 10:53 ` [RFC 14/22] media: Add MEDIA_IOC_DQEVENT Sakari Ailus
2016-05-06 10:53 ` [RFC 15/22] media: Make events on request completion optional, disabled by default Sakari Ailus
2016-05-06 11:05   ` Sakari Ailus
2016-05-06 10:53 ` [RFC 16/22] media: Add poll support Sakari Ailus
2016-05-06 10:53 ` [RFC 17/22] DocBook: media: Document the media request API Sakari Ailus
2016-05-06 10:53 ` [RFC 18/22] DocBook: media: Document the V4L2 " Sakari Ailus
2016-05-06 10:53 ` [RFC 19/22] DocBook: media: Document the subdev selection API Sakari Ailus
2016-05-06 10:53 ` [RFC 20/22] DocBook: media: Document the V4L2 subdev request API Sakari Ailus
2016-05-06 10:53 ` [RFC 21/22] DocBook: media: Document media events (MEDIA_IOC_DQEVENT IOCTL) Sakari Ailus
2016-05-06 10:53 ` [RFC 22/22] DocBook: media: Document request flags Sakari Ailus
2016-05-06 15:25 ` [RFC 00/22] Media request API Sakari Ailus
2016-05-17  8:05 ` Sakari Ailus

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).