All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC 0/8] Preparing the request API
@ 2018-03-09 23:48 Sakari Ailus
  2018-03-09 23:48 ` [RFC 1/8] media: Support variable size IOCTL arguments Sakari Ailus
                   ` (7 more replies)
  0 siblings, 8 replies; 9+ messages in thread
From: Sakari Ailus @ 2018-03-09 23:48 UTC (permalink / raw)
  To: linux-media; +Cc: acourbot

Hi folks,

This preliminary RFC patchset prepares for the request API. What's new
here is support for binding arbitrary configuration or resources to
requests.

There are a few new concepts here:

Class --- a type of configuration or resource a driver (or e.g. the V4L2
framework) can attach to a resource. E.g. a video buffer queue would be a
class.

Object --- an instance of the class. This may be either configuration (in
which case the setting will stay until changed, e.g. V4L2 format on a
video node) or a resource (such as a video buffer).

Reference --- a reference to an object. If a configuration is not changed
in a request, instead of allocating a new object, a reference to an
existing object is used. This saves time and memory.

I expect Laurent to comment on aligning the concept names between the
request API and DRM. As far as I understand, the respective DRM names
would be "object" and "state".

The drivers will need to interact with the requests in three ways:

- Allocate new configurations or resources. Drivers are free to store
  their own data into request objects as well. These callbacks are
  specific to classes.

- Validate and queue callbacks. These callbacks are used to try requests
  (validate only) as well as queue them (validate and queue). These
  callbacks are media device wide, at least for now.

The lifetime of the objects related to requests is based on refcounting
both requests and request objects. This fits well for existing use cases
whether or not based on refcounting; what still needs most of the
attention is likely that the number of gets and puts matches once the
object is no longer needed.

Configuration can be bound to the request the usual way (V4L2 IOCTLs with
the request_fd field set to the request). Once queued, request completion
is signalled through polling the request file handle (POLLPRI).

I'm posting this as an RFC because it's not complete yet; I'll continue
working on it next week.

Todo list:

- Better separation of request support between media-device.c and
  media-request.c. The intent is that mostly IOCTL handling would be in
  media-device.c.

- Request support in a few drivers as well as the control framework.

- Request support for V4L2 formats.

- Provide means to iterate over references in a request. This would be
  useful for drivers. The exact needs will be clearer once support for a
  few drivers has been added.

- Reinit support.

In the future, support for changing e.g. Media controller link state or
V4L2 sub-device formats will need to be added. Those are not in my
immediate plans though.

Comments and questions are welcome.


Open questions:

- How to tell at complete time whether a request failed? Return error code
  on release? What's the behaviour with reinit then --- fail on error? Add
  another request command to ask for status?


Alexandre Courbot (1):
  videodev2.h: add request_fd field to v4l2_ext_controls

Hans Verkuil (1):
  videodev2.h: Add request_fd field to v4l2_buffer

Laurent Pinchart (1):
  media: Add request API

Sakari Ailus (5):
  media: Support variable size IOCTL arguments
  media: Support polling for completed requests
  media: Add support for request classes and objects
  staging: media: atomisp: Remove v4l2_buffer.reserved2 field hack
  vb2: Add support for requests

 drivers/media/Makefile                             |   3 +-
 drivers/media/common/videobuf2/videobuf2-core.c    |  43 +-
 drivers/media/common/videobuf2/videobuf2-v4l2.c    |  24 +-
 drivers/media/media-device.c                       | 483 ++++++++++++++++++++-
 drivers/media/media-request.c                      | 287 ++++++++++++
 drivers/media/usb/cpia2/cpia2_v4l.c                |   2 +-
 drivers/media/v4l2-core/v4l2-compat-ioctl32.c      |  16 +-
 drivers/media/v4l2-core/v4l2-ioctl.c               |   6 +-
 .../media/atomisp/pci/atomisp2/atomisp_ioctl.c     |  17 +-
 include/media/media-device.h                       |  80 +++-
 include/media/media-request.h                      | 229 ++++++++++
 include/media/videobuf2-core.h                     |  19 +
 include/media/videobuf2-v4l2.h                     |   2 +
 include/uapi/linux/media.h                         |  10 +
 include/uapi/linux/videodev2.h                     |   6 +-
 15 files changed, 1186 insertions(+), 41 deletions(-)
 create mode 100644 drivers/media/media-request.c
 create mode 100644 include/media/media-request.h

-- 
2.7.4

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

* [RFC 1/8] media: Support variable size IOCTL arguments
  2018-03-09 23:48 [RFC 0/8] Preparing the request API Sakari Ailus
@ 2018-03-09 23:48 ` Sakari Ailus
  2018-03-09 23:48 ` [RFC 2/8] media: Add request API Sakari Ailus
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Sakari Ailus @ 2018-03-09 23:48 UTC (permalink / raw)
  To: linux-media; +Cc: acourbot

Maintain a list of supported IOCTL argument sizes and allow only those in
the list.

As an additional bonus, IOCTL handlers will be able to check whether the
caller actually set (using the argument size) the field vs. assigning it
to zero. Separate macro can be provided for that.

This will be easier for applications as well since there is no longer the
problem of setting the reserved fields zero, or at least it is a lesser
problem.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Acked-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/media-device.c | 65 ++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 59 insertions(+), 6 deletions(-)

diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index 35e81f7..da63da1 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -387,22 +387,36 @@ static long copy_arg_to_user(void __user *uarg, void *karg, unsigned int cmd)
 /* Do acquire the graph mutex */
 #define MEDIA_IOC_FL_GRAPH_MUTEX	BIT(0)
 
-#define MEDIA_IOC_ARG(__cmd, func, fl, from_user, to_user)		\
+#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,	\
 		.flags = fl,						\
+		.alt_arg_sizes = alt_sz,				\
 		.arg_from_user = from_user,				\
 		.arg_to_user = to_user,					\
 	}
 
-#define MEDIA_IOC(__cmd, func, fl)					\
-	MEDIA_IOC_ARG(__cmd, func, fl, copy_arg_from_user, copy_arg_to_user)
+#define MEDIA_IOC_ARG(__cmd, func, fl, from_user, to_user)		\
+	MEDIA_IOC_SZ_ARG(__cmd, func, fl, NULL, from_user, to_user)
+
+#define MEDIA_IOC_SZ(__cmd, func, fl, alt_sz)			\
+	MEDIA_IOC_SZ_ARG(__cmd, func, fl, alt_sz,		\
+			 copy_arg_from_user, copy_arg_to_user)
+
+#define MEDIA_IOC(__cmd, func, fl)				\
+	MEDIA_IOC_ARG(__cmd, func, fl,				\
+		      copy_arg_from_user, copy_arg_to_user)
 
 /* the table is indexed by _IOC_NR(cmd) */
 struct media_ioctl_info {
 	unsigned int cmd;
 	unsigned short flags;
+	/*
+	 * Sizes of the alternative arguments. If there are none, this
+	 * pointer is NULL.
+	 */
+	const unsigned short *alt_arg_sizes;
 	long (*fn)(struct media_device *dev, void *arg);
 	long (*arg_from_user)(void *karg, void __user *uarg, unsigned int cmd);
 	long (*arg_to_user)(void __user *uarg, void *karg, unsigned int cmd);
@@ -416,6 +430,42 @@ static const struct media_ioctl_info ioctl_info[] = {
 	MEDIA_IOC(G_TOPOLOGY, media_device_get_topology, MEDIA_IOC_FL_GRAPH_MUTEX),
 };
 
+#define MASK_IOC_SIZE(cmd) \
+	((cmd) & ~(_IOC_SIZEMASK << _IOC_SIZESHIFT))
+
+static inline long is_valid_ioctl(unsigned int cmd)
+{
+	const struct media_ioctl_info *info = ioctl_info;
+	const unsigned short *alt_arg_sizes;
+
+	if (_IOC_NR(cmd) >= ARRAY_SIZE(ioctl_info))
+		return -ENOIOCTLCMD;
+
+	info += _IOC_NR(cmd);
+
+	if (info->cmd == cmd)
+		return 0;
+
+	/*
+	 * Verify that the size-dependent patch of the IOCTL command
+	 * matches and that the size does not exceed the principal
+	 * argument size.
+	 */
+	if (MASK_IOC_SIZE(info->cmd) != MASK_IOC_SIZE(cmd)
+	    || _IOC_SIZE(info->cmd) < _IOC_SIZE(cmd))
+		return -ENOIOCTLCMD;
+
+	alt_arg_sizes = info->alt_arg_sizes;
+	if (!alt_arg_sizes)
+		return -ENOIOCTLCMD;
+
+	for (; *alt_arg_sizes; alt_arg_sizes++)
+		if (_IOC_SIZE(cmd) == *alt_arg_sizes)
+			return 0;
+
+	return -ENOIOCTLCMD;
+}
+
 static long media_device_ioctl(struct file *filp, unsigned int cmd,
 			       unsigned long __arg)
 {
@@ -426,9 +476,9 @@ static long media_device_ioctl(struct file *filp, unsigned int cmd,
 	char __karg[256], *karg = __karg;
 	long ret;
 
-	if (_IOC_NR(cmd) >= ARRAY_SIZE(ioctl_info)
-	    || ioctl_info[_IOC_NR(cmd)].cmd != cmd)
-		return -ENOIOCTLCMD;
+	ret = is_valid_ioctl(cmd);
+	if (ret)
+		return ret;
 
 	info = &ioctl_info[_IOC_NR(cmd)];
 
@@ -444,6 +494,9 @@ static long media_device_ioctl(struct file *filp, unsigned int cmd,
 			goto out_free;
 	}
 
+	/* Set the rest of the argument struct to zero */
+	memset(karg + _IOC_SIZE(cmd), 0, _IOC_SIZE(info->cmd) - _IOC_SIZE(cmd));
+
 	if (info->flags & MEDIA_IOC_FL_GRAPH_MUTEX)
 		mutex_lock(&dev->graph_mutex);
 
-- 
2.7.4

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

* [RFC 2/8] media: Add request API
  2018-03-09 23:48 [RFC 0/8] Preparing the request API Sakari Ailus
  2018-03-09 23:48 ` [RFC 1/8] media: Support variable size IOCTL arguments Sakari Ailus
@ 2018-03-09 23:48 ` Sakari Ailus
  2018-03-09 23:48 ` [RFC 3/8] media: Support polling for completed requests Sakari Ailus
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Sakari Ailus @ 2018-03-09 23:48 UTC (permalink / raw)
  To: linux-media; +Cc: acourbot, 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>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/media-device.c | 366 ++++++++++++++++++++++++++++++++++++++++++-
 include/media/media-device.h |  71 ++++++++-
 include/uapi/linux/media.h   |  10 ++
 3 files changed, 441 insertions(+), 6 deletions(-)

diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index da63da1..41ec5ac 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -19,11 +19,13 @@
 /* We need to access legacy defines from linux/media.h */
 #define __NEED_MEDIA_LEGACY_API
 
+#include <linux/anon_inodes.h>
 #include <linux/compat.h>
 #include <linux/export.h>
-#include <linux/idr.h>
+#include <linux/file.h>
 #include <linux/ioctl.h>
 #include <linux/media.h>
+#include <linux/sched.h>
 #include <linux/slab.h>
 #include <linux/types.h>
 #include <linux/pci.h>
@@ -35,6 +37,349 @@
 
 #ifdef CONFIG_MEDIA_CONTROLLER
 
+static const char *__request_state[] = {
+	"IDLE",
+	"QUEUED",
+	"COMPLETED",
+};
+
+const char *
+media_device_request_state_str(enum media_device_request_state state)
+{
+	if (state < ARRAY_SIZE(__request_state))
+		return __request_state[state];
+
+	return "UNKNOWN";
+}
+
+/* -----------------------------------------------------------------------------
+ * Requests
+ */
+
+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;
+	unsigned long flags;
+
+	dev_dbg(mdev->dev, "request: release %s\n", req->debug_str);
+
+	spin_lock_irqsave(&mdev->req_lock, flags);
+	list_del(&req->list);
+	spin_unlock_irqrestore(&mdev->req_lock, flags);
+
+	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_close(struct inode *inode, struct file *filp)
+{
+	struct media_device_request *req = filp->private_data;
+
+	media_device_request_put(req);
+
+	return 0;
+}
+
+static const struct file_operations request_fops = {
+	.owner = THIS_MODULE,
+	.release = media_device_request_close,
+};
+
+/**
+ * media_device_request_find - Find a request based on the file descriptor
+ * @mdev: The media device
+ * @request: The request file handle
+ *
+ * Find and return the request associated with the given file descriptor, or
+ * an error if no such request exists.
+ *
+ * When the function returns a 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, int request)
+{
+	struct file *filp;
+	struct media_device_request *req;
+
+	filp = fget(request);
+	if (!filp)
+		return ERR_PTR(-ENOENT);
+
+	if (filp->f_op != &request_fops)
+		goto err_fput;
+	req = filp->private_data;
+	media_device_request_get(req);
+
+	if (req->mdev != mdev)
+		goto err_kref_put;
+
+	fput(filp);
+
+	return req;
+
+err_kref_put:
+	media_device_request_put(req);
+
+err_fput:
+	fput(filp);
+
+	return ERR_PTR(-EBADF);
+}
+EXPORT_SYMBOL_GPL(media_device_request_find);
+
+static int media_device_request_alloc(struct media_device *mdev,
+				      struct file *filp,
+				      struct media_request_cmd *cmd)
+{
+	struct media_device_request *req;
+#ifdef CONFIG_DYNAMIC_DEBUG
+	char comm[TASK_COMM_LEN];
+#endif
+	unsigned long flags;
+	int fd;
+	int ret;
+
+	fd = get_unused_fd_flags(O_CLOEXEC);
+	if (fd < 0)
+		return fd;
+
+	filp = anon_inode_getfile("request", &request_fops, NULL, O_CLOEXEC);
+	if (IS_ERR(filp)) {
+		ret = PTR_ERR(filp);
+		goto err_put_fd;
+	}
+
+	req = mdev->ops->req_alloc(mdev);
+	if (!req) {
+		ret = -ENOMEM;
+		goto err_fput;
+	}
+
+	filp->private_data = req;
+	req->mdev = mdev;
+	req->state = MEDIA_DEVICE_REQUEST_STATE_IDLE;
+	kref_init(&req->kref);
+
+	spin_lock_irqsave(&mdev->req_lock, flags);
+	list_add_tail(&req->req_list, &mdev->req_idle);
+	spin_unlock_irqrestore(&mdev->req_lock, flags);
+
+	cmd->request = fd;
+
+#ifdef CONFIG_DYNAMIC_DEBUG
+	get_task_comm(comm, current);
+	snprintf(req->debug_str, sizeof(req->debug_str), "%s:%d",
+		 comm, fd);
+#endif
+
+	dev_dbg(mdev->dev, "request: allocated %s\n", req->debug_str);
+	fd_install(fd, filp);
+
+	return 0;
+
+err_fput:
+	fput(filp);
+
+err_put_fd:
+	put_unused_fd(fd);
+
+	return ret;
+}
+
+void media_device_request_complete(struct media_device *mdev,
+				   struct media_device_request *req)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&mdev->req_lock, flags);
+
+	if (req->state == MEDIA_DEVICE_REQUEST_STATE_IDLE) {
+		dev_dbg(mdev->dev,
+			"request: not completing an idle request %s\n",
+			req->debug_str);
+		spin_unlock_irqrestore(&mdev->req_lock, flags);
+		return;
+	}
+
+	if (WARN_ON(req->state != MEDIA_DEVICE_REQUEST_STATE_QUEUED)) {
+		dev_dbg(mdev->dev, "request: can't delete %s, state %s\n",
+			req->debug_str,
+			media_device_request_state_str(req->state));
+		spin_unlock_irqrestore(&mdev->req_lock, flags);
+		return;
+	}
+
+	req->state = MEDIA_DEVICE_REQUEST_STATE_COMPLETE;
+
+	spin_unlock_irqrestore(&mdev->req_lock, flags);
+
+	media_device_request_put(req);
+}
+EXPORT_SYMBOL_GPL(media_device_request_complete);
+
+static int media_device_request_queue(
+	struct media_device *mdev, struct media_device_request *req, bool try)
+{
+	char *str = try ? "try" : "queue";
+	unsigned long flags;
+	int ret = 0;
+
+	dev_dbg(mdev->dev, "request: %s %s\n", str, req->debug_str);
+
+	/*
+	 * Ensure the request that was just validated is the one that gets
+	 * queued next.
+	 */
+	mutex_lock(&mdev->req_queue_mutex);
+
+	spin_lock_irqsave(&mdev->req_lock, flags);
+	if (req->state != MEDIA_DEVICE_REQUEST_STATE_IDLE) {
+		ret = -EINVAL;
+		dev_dbg(mdev->dev,
+			"request: unable to %s %s, request in state %s\n",
+			str, req->debug_str,
+			media_device_request_state_str(req->state));
+	} else {
+		req->state = MEDIA_DEVICE_REQUEST_STATE_QUEUED;
+	}
+	spin_unlock_irqrestore(&mdev->req_lock, flags);
+
+	if (ret)
+		goto err_unlock;
+
+	ret = mdev->ops->req_validate(mdev, req);
+	if (ret || try) {
+		if (ret)
+			dev_dbg(mdev->dev,
+				"request: %s failed validation (%d)\n",
+				req->debug_str, ret);
+		goto err_set_idle;
+	}
+
+	/*
+	 * Obtain a reference to the request, release once complete (or there's
+	 * an error).
+	 */
+	media_device_request_get(req);
+	spin_lock_irqsave(&mdev->req_lock, flags);
+	list_move(&req->list, &mdev->req_queued);
+	spin_unlock_irqrestore(&mdev->req_lock, flags);
+
+	ret = mdev->ops->req_queue(mdev, req);
+	if (ret) {
+		dev_dbg(mdev->dev, "request: can't queue %s (%d)\n",
+			req->debug_str, ret);
+		goto err_put;
+	}
+
+	mutex_unlock(&mdev->req_queue_mutex);
+
+	return 0;
+
+err_put:
+	media_device_request_put(req);
+
+err_set_idle:
+	spin_lock_irqsave(&mdev->req_lock, flags);
+	req->state = MEDIA_DEVICE_REQUEST_STATE_IDLE;
+	spin_unlock_irqrestore(&mdev->req_lock, flags);
+
+err_unlock:
+	mutex_unlock(&mdev->req_queue_mutex);
+
+	return ret;
+
+}
+
+static long media_device_request_cmd(struct media_device *mdev,
+				     struct file *filp,
+				     struct media_request_cmd *cmd)
+{
+	struct media_device_request *req = NULL;
+	int ret;
+
+	if (!mdev->ops || !mdev->ops->req_alloc || !mdev->ops->req_free ||
+	    !mdev->ops->req_validate || !mdev->ops->req_queue)
+		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, filp, cmd);
+		break;
+
+	case MEDIA_REQ_CMD_QUEUE:
+		ret = media_device_request_queue(mdev, req, false);
+		break;
+
+	case MEDIA_REQ_CMD_TRY:
+		ret = media_device_request_queue(mdev, req, true);
+		break;
+
+	default:
+		ret = -EINVAL;
+		break;
+	}
+
+	if (req)
+		media_device_request_put(req);
+
+	return ret < 0 ? ret : 0;
+}
+
+/**
+ * media_device_request_alloc_simple - Callback to allocate memory for media
+ *				       request
+ *
+ * @mdev: The media device for which the request is to be allocated
+ *
+ * Allocate memory for a media request. Drivers are free to implement their own
+ * callbacks if more than just the size of the @struct media_device_request
+ * itself is needed. Driver's own data comes after @struct media_device_request.
+ */
+struct media_device_request *
+media_device_request_alloc_simple(struct media_device *mdev)
+{
+	return kzalloc(sizeof(struct media_device_request), GFP_KERNEL);
+}
+EXPORT_SYMBOL_GPL(media_device_request_alloc_simple);
+
+/**
+ * media_device_request_alloc_free - Callback to release media request memory
+ *
+ * @mdev: The media device
+ * @req: The request which is to be released
+ *
+ * Release the media request memory. If more than this is needed to clean up a
+ * media request, drivers are free to implement their own callbacks.
+ */
+void media_device_request_free_simple(struct media_device *mdev,
+				      struct media_device_request *req)
+{
+	kfree(req);
+}
+EXPORT_SYMBOL_GPL(media_device_request_free_simple);
+
 /* -----------------------------------------------------------------------------
  * Userspace API
  */
@@ -55,6 +400,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));
@@ -94,6 +440,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;
@@ -147,6 +494,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;
@@ -196,6 +544,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;
@@ -226,6 +575,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;
@@ -390,7 +740,8 @@ static long copy_arg_to_user(void __user *uarg, void *karg, unsigned int cmd)
 #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,				\
@@ -417,7 +768,7 @@ struct media_ioctl_info {
 	 * pointer is NULL.
 	 */
 	const unsigned short *alt_arg_sizes;
-	long (*fn)(struct media_device *dev, void *arg);
+	long (*fn)(struct media_device *dev, struct file *file, void *arg);
 	long (*arg_from_user)(void *karg, void __user *uarg, unsigned int cmd);
 	long (*arg_to_user)(void __user *uarg, void *karg, unsigned int cmd);
 };
@@ -428,6 +779,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),
 };
 
 #define MASK_IOC_SIZE(cmd) \
@@ -500,7 +852,7 @@ static long media_device_ioctl(struct file *filp, unsigned int cmd,
 	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);
@@ -540,7 +892,7 @@ static long media_device_enum_links32(struct media_device *mdev,
 	links.pads = compat_ptr(pads_ptr);
 	links.links = compat_ptr(links_ptr);
 
-	return media_device_enum_links(mdev, &links);
+	return media_device_enum_links(mdev, NULL, &links);
 }
 
 #define MEDIA_IOC_ENUM_LINKS32		_IOWR('|', 0x02, struct media_links_enum32)
@@ -765,6 +1117,10 @@ int __must_check __media_device_register(struct media_device *mdev,
 	if (!devnode)
 		return -ENOMEM;
 
+	spin_lock_init(&mdev->req_lock);
+	INIT_LIST_HEAD(&mdev->req_idle);
+	INIT_LIST_HEAD(&mdev->req_queued);
+
 	/* Register the device node. */
 	mdev->devnode = devnode;
 	devnode->fops = &media_device_fops;
diff --git a/include/media/media-device.h b/include/media/media-device.h
index bcc6ec4..0f32ceb 100644
--- a/include/media/media-device.h
+++ b/include/media/media-device.h
@@ -19,14 +19,51 @@
 #ifndef _MEDIA_DEVICE_H
 #define _MEDIA_DEVICE_H
 
+#include <linux/kref.h>
+#include <linux/idr.h>
 #include <linux/list.h>
 #include <linux/mutex.h>
 
 #include <media/media-devnode.h>
 #include <media/media-entity.h>
 
-struct ida;
 struct device;
+struct ida;
+struct media_device;
+
+enum media_device_request_state {
+	MEDIA_DEVICE_REQUEST_STATE_IDLE,
+	MEDIA_DEVICE_REQUEST_STATE_QUEUED,
+	MEDIA_DEVICE_REQUEST_STATE_COMPLETE,
+};
+
+/**
+ * media_device_request_state_str - Convert media request state to a string
+ *
+ * @state: The state of the media request
+ *
+ * Returns the string corresponding to a media request state.
+ */
+const char *
+media_device_request_state_str(enum media_device_request_state state);
+
+/**
+ * struct media_device_request - Media device request
+ * @mdev: Media device this request belongs to
+ * @kref: Reference count
+ * @list: List entry in the media device requests list
+ * @debug_prefix: Prefix for debug messages (process name:fd)
+ * @state: The state of the request
+ */
+struct media_device_request {
+	struct media_device *mdev;
+	struct kref kref;
+	struct list_head list;
+#ifdef CONFIG_DYNAMIC_DEBUG
+	char debug_str[TASK_COMM_LEN + 6];
+#endif
+	enum media_device_request_state state;
+};
 
 /**
  * struct media_entity_notify - Media Entity Notify
@@ -50,10 +87,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_validate: Validate 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_validate)(struct media_device *mdev,
+			 struct media_device_request *req);
+	int (*req_queue)(struct media_device *mdev,
+			 struct media_device_request *req);
 };
 
 /**
@@ -88,6 +136,11 @@ struct media_device_ops {
  * @disable_source: Disable Source Handler function pointer
  *
  * @ops:	Operation handler callbacks
+ * @req_lock:	Serialise access to @requests
+ * @req_queue_mutex: Serialise validating and queueing requests
+ * @req_idle:	List of idle requests (@struct media_request.list)
+ * @req_queued:	List of queued (and completed) requests (@struct
+ *		media_request.list)
  *
  * This structure represents an abstract high-level media device. It allows easy
  * access to entities and provides basic media device-level support. The
@@ -158,6 +211,11 @@ struct media_device {
 	void (*disable_source)(struct media_entity *entity);
 
 	const struct media_device_ops *ops;
+
+	spinlock_t req_lock;
+	struct mutex req_queue_mutex;
+	struct list_head req_queued;
+	struct list_head req_idle;
 };
 
 /* We don't need to include pci.h or usb.h here */
@@ -475,4 +533,15 @@ 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, int request);
+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_device_request *
+media_device_request_alloc_simple(struct media_device *mdev);
+void media_device_request_free_simple(struct media_device *mdev,
+				      struct media_device_request *req);
 #endif
diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
index c7e9a5c..9bc397b 100644
--- a/include/uapi/linux/media.h
+++ b/include/uapi/linux/media.h
@@ -342,11 +342,21 @@ struct media_v2_topology {
 
 /* ioctls */
 
+#define MEDIA_REQ_CMD_ALLOC		0
+#define MEDIA_REQ_CMD_TRY		1
+#define MEDIA_REQ_CMD_QUEUE		2
+
+struct __attribute__ ((packed)) media_request_cmd {
+	__u32 cmd;
+	__s32 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)
 
 #if !defined(__KERNEL__) || defined(__NEED_MEDIA_LEGACY_API)
 
-- 
2.7.4

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

* [RFC 3/8] media: Support polling for completed requests
  2018-03-09 23:48 [RFC 0/8] Preparing the request API Sakari Ailus
  2018-03-09 23:48 ` [RFC 1/8] media: Support variable size IOCTL arguments Sakari Ailus
  2018-03-09 23:48 ` [RFC 2/8] media: Add request API Sakari Ailus
@ 2018-03-09 23:48 ` Sakari Ailus
  2018-03-09 23:48 ` [RFC 4/8] media: Add support for request classes and objects Sakari Ailus
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Sakari Ailus @ 2018-03-09 23:48 UTC (permalink / raw)
  To: linux-media; +Cc: acourbot

Implement poll for events. POLLPRI is used to notify the user an event is
complete.

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

diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index 41ec5ac..a4d3884 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -84,6 +84,34 @@ void media_device_request_put(struct media_device_request *req)
 }
 EXPORT_SYMBOL_GPL(media_device_request_put);
 
+static unsigned int media_device_request_poll(struct file *filp,
+					      struct poll_table_struct *wait)
+{
+	struct media_device_request *req = filp->private_data;
+	struct media_device *mdev = req->mdev;
+	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 complete;
+
+		spin_lock_irqsave(&mdev->req_lock, flags);
+		complete = req->state == MEDIA_DEVICE_REQUEST_STATE_COMPLETE;
+		spin_unlock_irqrestore(&mdev->req_lock, flags);
+
+		if (complete)
+			poll_wait(filp, &req->poll_wait, wait);
+		else
+			ret |= POLLPRI;
+	}
+
+	return ret;
+}
+
 static int media_device_request_close(struct inode *inode, struct file *filp)
 {
 	struct media_device_request *req = filp->private_data;
@@ -95,6 +123,7 @@ static int media_device_request_close(struct inode *inode, struct file *filp)
 
 static const struct file_operations request_fops = {
 	.owner = THIS_MODULE,
+	.poll = media_device_request_poll,
 	.release = media_device_request_close,
 };
 
-- 
2.7.4

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

* [RFC 4/8] media: Add support for request classes and objects
  2018-03-09 23:48 [RFC 0/8] Preparing the request API Sakari Ailus
                   ` (2 preceding siblings ...)
  2018-03-09 23:48 ` [RFC 3/8] media: Support polling for completed requests Sakari Ailus
@ 2018-03-09 23:48 ` Sakari Ailus
  2018-03-09 23:48 ` [RFC 5/8] videodev2.h: Add request_fd field to v4l2_buffer Sakari Ailus
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Sakari Ailus @ 2018-03-09 23:48 UTC (permalink / raw)
  To: linux-media; +Cc: acourbot

Media requests may contain multiple objects (e.g. V4L2 format or video
buffers). The media request objects support managing these in a generic
way. The classes are effectively a helper for managing certain kinds of
objects.

Objects may be sticky (the previous configuration matters, e.g. V4L2
format) or non-sticky (whatever was before is irrelevant, e.g. video
buffers).

The drivers are responsible for initialising the classes for types of
objects they support with requests. Some of this could possibly be
implemented in the media and V4L2 frameworks.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/Makefile        |   3 +-
 drivers/media/media-device.c  |  27 +++-
 drivers/media/media-request.c | 287 ++++++++++++++++++++++++++++++++++++++++++
 include/media/media-device.h  |  13 +-
 include/media/media-request.h | 229 +++++++++++++++++++++++++++++++++
 5 files changed, 554 insertions(+), 5 deletions(-)
 create mode 100644 drivers/media/media-request.c
 create mode 100644 include/media/media-request.h

diff --git a/drivers/media/Makefile b/drivers/media/Makefile
index 594b462..985d35e 100644
--- a/drivers/media/Makefile
+++ b/drivers/media/Makefile
@@ -3,7 +3,8 @@
 # Makefile for the kernel multimedia device drivers.
 #
 
-media-objs	:= media-device.o media-devnode.o media-entity.o
+media-objs	:= media-device.o media-devnode.o media-entity.o \
+		   media-request.o
 
 #
 # I2C drivers should come before other drivers, otherwise they'll fail
diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index a4d3884..873d83c 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -34,6 +34,7 @@
 #include <media/media-device.h>
 #include <media/media-devnode.h>
 #include <media/media-entity.h>
+#include <media/media-request.h>
 
 #ifdef CONFIG_MEDIA_CONTROLLER
 
@@ -66,15 +67,21 @@ static void media_device_request_release(struct kref *kref)
 {
 	struct media_device_request *req =
 		container_of(kref, struct media_device_request, kref);
+	struct media_request_ref *ref, *ref_safe;
 	struct media_device *mdev = req->mdev;
 	unsigned long flags;
 
 	dev_dbg(mdev->dev, "request: release %s\n", req->debug_str);
 
 	spin_lock_irqsave(&mdev->req_lock, flags);
-	list_del(&req->list);
+	list_del(&req->req_list);
 	spin_unlock_irqrestore(&mdev->req_lock, flags);
 
+	list_for_each_entry_safe(ref, ref_safe, &req->obj_refs, req_list) {
+		__media_request_ref_put(ref);
+		kfree(ref);
+	}
+
 	mdev->ops->req_free(mdev, req);
 }
 
@@ -203,6 +210,7 @@ static int media_device_request_alloc(struct media_device *mdev,
 	req->mdev = mdev;
 	req->state = MEDIA_DEVICE_REQUEST_STATE_IDLE;
 	kref_init(&req->kref);
+	INIT_LIST_HEAD(&req->obj_refs);
 
 	spin_lock_irqsave(&mdev->req_lock, flags);
 	list_add_tail(&req->req_list, &mdev->req_idle);
@@ -237,6 +245,13 @@ void media_device_request_complete(struct media_device *mdev,
 
 	spin_lock_irqsave(&mdev->req_lock, flags);
 
+	if (!media_request_is_complete(req)) {
+		spin_unlock_irqrestore(&mdev->req_lock, flags);
+		dev_dbg(mdev->dev, "request: %s is not complete yet\n",
+			req->debug_str);
+		return;
+	}
+
 	if (req->state == MEDIA_DEVICE_REQUEST_STATE_IDLE) {
 		dev_dbg(mdev->dev,
 			"request: not completing an idle request %s\n",
@@ -285,7 +300,9 @@ static int media_device_request_queue(
 			media_device_request_state_str(req->state));
 	} else {
 		req->state = MEDIA_DEVICE_REQUEST_STATE_QUEUED;
+		media_request_sticky_to_old(req);
 	}
+
 	spin_unlock_irqrestore(&mdev->req_lock, flags);
 
 	if (ret)
@@ -306,7 +323,7 @@ static int media_device_request_queue(
 	 */
 	media_device_request_get(req);
 	spin_lock_irqsave(&mdev->req_lock, flags);
-	list_move(&req->list, &mdev->req_queued);
+	list_move(&req->req_list, &mdev->req_queued);
 	spin_unlock_irqrestore(&mdev->req_lock, flags);
 
 	ret = mdev->ops->req_queue(mdev, req);
@@ -316,6 +333,10 @@ static int media_device_request_queue(
 		goto err_put;
 	}
 
+	spin_lock_irqsave(&mdev->req_lock, flags);
+	media_request_new_to_sticky(req);
+	spin_unlock_irqrestore(&mdev->req_lock, flags);
+
 	mutex_unlock(&mdev->req_queue_mutex);
 
 	return 0;
@@ -325,6 +346,7 @@ static int media_device_request_queue(
 
 err_set_idle:
 	spin_lock_irqsave(&mdev->req_lock, flags);
+	media_request_detach_old(req);
 	req->state = MEDIA_DEVICE_REQUEST_STATE_IDLE;
 	spin_unlock_irqrestore(&mdev->req_lock, flags);
 
@@ -1149,6 +1171,7 @@ int __must_check __media_device_register(struct media_device *mdev,
 	spin_lock_init(&mdev->req_lock);
 	INIT_LIST_HEAD(&mdev->req_idle);
 	INIT_LIST_HEAD(&mdev->req_queued);
+	INIT_LIST_HEAD(&mdev->classes);
 
 	/* Register the device node. */
 	mdev->devnode = devnode;
diff --git a/drivers/media/media-request.c b/drivers/media/media-request.c
new file mode 100644
index 0000000..f9f8b41
--- /dev/null
+++ b/drivers/media/media-request.c
@@ -0,0 +1,287 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Media device request objects
+ *
+ * Copyright (C) 2018 Intel Corporation
+ *
+ * Author: Sakari Ailus <sakari.ailus@linux.intel.com>
+ */
+
+#include <linux/mm.h>
+#include <linux/string.h>
+
+#include <media/media-device.h>
+#include <media/media-request.h>
+
+static void media_request_object_release(struct kref *kref)
+{
+	struct media_request_object *obj =
+		container_of(kref, struct media_request_object, kref);
+	struct media_device *mdev = obj->class->mdev;
+	unsigned long flags;
+
+	spin_lock_irqsave(&mdev->req_lock, flags);
+	list_del(&obj->object_list);
+	spin_unlock_irqrestore(&mdev->req_lock, flags);
+
+	obj->class->release(obj);
+}
+
+void media_request_object_put(struct media_request_object *obj)
+{
+	if (obj)
+		kref_put(&obj->kref, media_request_object_release);
+}
+EXPORT_SYMBOL_GPL(media_request_object_put);
+
+static struct media_request_object *
+media_request_object_get(struct media_request_object *obj)
+{
+	kref_get(&obj->kref);
+
+	return obj;
+}
+
+void
+media_request_class_register(struct media_device *mdev,
+			     struct media_request_class *class,
+			     void (*release)(struct media_request_object *object),
+			     bool completeable)
+{
+	unsigned long flags;
+
+	INIT_LIST_HEAD(&class->objects);
+	class->release = release;
+	class->completeable = completeable;
+	class->mdev = mdev;
+
+	spin_lock_irqsave(&mdev->req_lock, flags);
+	list_add(&class->mdev_list, &mdev->classes);
+	spin_unlock_irqrestore(&mdev->req_lock, flags);
+}
+EXPORT_SYMBOL_GPL(media_request_class_register);
+
+void media_request_class_unregister(struct media_request_class *class)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&class->mdev->req_lock, flags);
+	list_del(&class->mdev_list);
+	spin_unlock_irqrestore(&class->mdev->req_lock, flags);
+	media_request_object_put(class->sticky);
+}
+EXPORT_SYMBOL_GPL(media_request_class_unregister);
+
+void media_request_class_set_sticky(struct media_request_class *class,
+				    struct media_request_object *sticky)
+{
+	if (WARN_ON(class->sticky))
+		return;
+
+	class->sticky = media_request_object_get(sticky);
+}
+
+void media_request_object_init(struct media_request_class *class,
+			       struct media_request_object *obj)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&class->mdev->req_lock, flags);
+	list_add(&obj->object_list, &class->objects);
+	spin_unlock_irqrestore(&class->mdev->req_lock, flags);
+	obj->class = class;
+	kref_init(&obj->kref);
+}
+EXPORT_SYMBOL_GPL(media_request_object_init);
+
+void media_request_ref_put(struct media_request_ref *ref)
+{
+	if (!ref)
+		return;
+
+	media_request_object_put(ref->new);
+	media_request_object_put(ref->old);
+}
+EXPORT_SYMBOL_GPL(media_request_ref_put);
+
+static struct media_request_ref *
+media_request_ref_find(struct media_device_request *req,
+		       struct media_request_class *class)
+{
+	struct media_request_ref *ref;
+
+	lockdep_assert_held(&class->mdev->req_lock);
+
+	list_for_each_entry(ref, &req->obj_refs, req_list)
+		if (ref->new->class == class)
+			return ref;
+
+	return NULL;
+}
+
+static int __media_request_object_bind(struct media_device_request *req,
+				       struct media_request_ref *ref,
+				       struct media_request_object *obj)
+{
+	if (req->state != MEDIA_DEVICE_REQUEST_STATE_IDLE) {
+		dev_dbg(req->mdev->dev, "request: %s not idle but %s\n",
+			req->debug_str,
+			media_device_request_state_str(req->state));
+		return -EBUSY;
+	}
+
+	media_request_object_put(ref->new);
+	ref->new = media_request_object_get(obj);
+
+	return 0;
+}
+
+struct media_request_ref *
+media_request_object_bind(struct media_device_request *req,
+			  struct media_request_object *obj)
+{
+	struct media_request_class *class = obj->class;
+	struct media_device *mdev = class->mdev;
+	struct media_request_ref *ref, *ref_new;
+	unsigned long flags;
+	int ret;
+
+	ref_new = kzalloc(sizeof(*ref_new), GFP_KERNEL);
+
+	spin_lock_irqsave(&mdev->req_lock, flags);
+
+	ref = media_request_ref_find(req, obj->class);
+	if (!ref) {
+		if (!ref_new) {
+			ret = -ENOMEM;
+			goto err;
+		}
+
+		ref = ref_new;
+	}
+
+	ret = __media_request_object_bind(req, ref, obj);
+	if (ret)
+		goto err;
+
+	/* Newly created reference? */
+	if (ref == ref_new) {
+		list_add(&ref->req_list, &req->obj_refs);
+		if (class->completeable)
+			req->incomplete++;
+		ref->req = req;
+	}
+
+	spin_unlock_irqrestore(&req->mdev->req_lock, flags);
+
+	/* Release unused reference */
+	if (ref != ref_new)
+		kfree(ref_new);
+
+	return ref;
+
+err:
+	spin_unlock_irqrestore(&req->mdev->req_lock, flags);
+
+	kfree(ref_new);
+
+	return ERR_PTR(ret);
+}
+EXPORT_SYMBOL_GPL(media_request_object_bind);
+
+void media_request_ref_unbind(struct media_request_ref *ref)
+{
+	struct media_request_object *obj = ref->new;
+	struct media_device *mdev = obj->class->mdev;
+	unsigned long flags;
+
+	if (WARN_ON(!obj->class->completeable))
+		return;
+
+	spin_lock_irqsave(&mdev->req_lock, flags);
+	if (!ref->complete) {
+		ref->req->incomplete--;
+		WARN_ON(ref->req->incomplete < 0);
+	}
+	list_del(&ref->req_list);
+	spin_unlock_irqrestore(&mdev->req_lock, flags);
+	media_request_ref_put(ref);
+}
+EXPORT_SYMBOL_GPL(media_request_ref_unbind);
+
+/* Tip of the queue state is the state previous to the request. */
+void media_request_sticky_to_old(struct media_device_request *req)
+{
+	struct media_request_ref *ref;
+
+	lockdep_assert_held(&req->mdev->req_lock);
+
+	list_for_each_entry(ref, &req->obj_refs, req_list) {
+		struct media_request_class *class = ref->new->class;
+
+		if (!class->sticky)
+			continue;
+
+		ref->old = media_request_object_get(class->sticky);
+	}
+}
+EXPORT_SYMBOL_GPL(media_request_sticky_to_old);
+
+void media_request_new_to_sticky(struct media_device_request *req)
+{
+	struct media_request_ref *ref;
+
+	lockdep_assert_held(&req->mdev->req_lock);
+
+	list_for_each_entry(ref, &req->obj_refs, req_list) {
+		struct media_request_class *class = ref->new->class;
+
+		if (!class->sticky)
+			continue;
+
+		media_request_object_put(class->sticky);
+		class->sticky = media_request_object_get(ref->new);
+	}
+}
+EXPORT_SYMBOL_GPL(media_request_new_to_sticky);
+
+void media_request_detach_old(struct media_device_request *req)
+{
+	struct media_request_ref *ref;
+
+	lockdep_assert_held(&req->mdev->req_lock);
+
+	list_for_each_entry(ref, &req->obj_refs, req_list) {
+		media_request_object_put(ref->old);
+		ref->old = NULL;
+	}
+}
+EXPORT_SYMBOL_GPL(media_request_detach_old);
+
+bool media_request_is_complete(struct media_device_request *req)
+{
+	lockdep_assert_held(&req->mdev->req_lock);
+
+	return !req->incomplete;
+}
+EXPORT_SYMBOL_GPL(media_request_is_complete);
+
+void media_request_ref_complete(struct media_request_ref *ref)
+{
+	struct media_request_object *obj = ref->new;
+	struct media_device *mdev = obj->class->mdev;
+	unsigned long flags;
+
+	if (WARN_ON(!obj->class->completeable))
+		return;
+
+	spin_lock_irqsave(&mdev->req_lock, flags);
+	if (!ref->complete) {
+		ref->complete = true;
+		ref->req->incomplete--;
+		WARN_ON(ref->req->incomplete < 0);
+	}
+	spin_unlock_irqrestore(&mdev->req_lock, flags);
+	media_request_ref_put(ref);
+}
+EXPORT_SYMBOL_GPL(media_request_ref_complete);
diff --git a/include/media/media-device.h b/include/media/media-device.h
index 0f32ceb..01543f8 100644
--- a/include/media/media-device.h
+++ b/include/media/media-device.h
@@ -51,18 +51,24 @@ media_device_request_state_str(enum media_device_request_state state);
  * struct media_device_request - Media device request
  * @mdev: Media device this request belongs to
  * @kref: Reference count
- * @list: List entry in the media device requests list
+ * @req_list: List entry in the media device requests list
  * @debug_prefix: Prefix for debug messages (process name:fd)
  * @state: The state of the request
+ * @obj_refs: List of @struct media_request_object_ref req_list field
+ * @incomplete: The number of unsticky objects that have not been completed
+ * @poll_wait: Wait queue for poll
  */
 struct media_device_request {
 	struct media_device *mdev;
 	struct kref kref;
-	struct list_head list;
+	struct list_head req_list;
 #ifdef CONFIG_DYNAMIC_DEBUG
 	char debug_str[TASK_COMM_LEN + 6];
 #endif
 	enum media_device_request_state state;
+	struct list_head obj_refs;
+	unsigned int incomplete;
+	struct wait_queue_head poll_wait;
 };
 
 /**
@@ -141,6 +147,8 @@ struct media_device_ops {
  * @req_idle:	List of idle requests (@struct media_request.list)
  * @req_queued:	List of queued (and completed) requests (@struct
  *		media_request.list)
+ * @classes:	List of request classes, i.e. which objects may be contained in
+ *		media requests (@struct media_request_class.mdev_list)
  *
  * This structure represents an abstract high-level media device. It allows easy
  * access to entities and provides basic media device-level support. The
@@ -216,6 +224,7 @@ struct media_device {
 	struct mutex req_queue_mutex;
 	struct list_head req_queued;
 	struct list_head req_idle;
+	struct list_head classes;
 };
 
 /* We don't need to include pci.h or usb.h here */
diff --git a/include/media/media-request.h b/include/media/media-request.h
new file mode 100644
index 0000000..74037c8
--- /dev/null
+++ b/include/media/media-request.h
@@ -0,0 +1,229 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Media device request objects
+ *
+ * Copyright (C) 2018 Intel Corporation
+ *
+ * Author: Sakari Ailus <sakari.ailus@linux.intel.com>
+ */
+
+#include <linux/list.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+
+#include <media/media-device.h>
+
+struct media_request_object;
+
+/**
+ * struct media_request_class - Class of object that may be part of a media
+ *				request
+ *
+ * @objects: List of objects belonging to a class (@struct media_request_object
+ *	     object_list field)
+ * @sticky: Configuration as of the latest request queued; also indicates that a
+ *	    class is sticky
+ * @mdev_list: List entry of the media device's class list
+ * @mdev: The media device
+ * @release: A callback function to release a previously initialised
+ *	     @struct media_request_object
+ */
+struct media_request_class {
+	bool completeable;
+	struct list_head objects;
+	struct media_request_object *sticky;
+	struct list_head mdev_list;
+	struct media_device *mdev;
+	void (*release)(struct media_request_object *object);
+};
+
+/**
+ * struct media_request_object - An opaque object that belongs to a media
+ *				 request
+ *
+ * @class: The class which the object is related to
+ * @object_list: List entry of the object list of the class
+ * @kref: Reference to the object, acquire before releasing mdev->req_lock
+ *
+ * An object related to the request. The object data follows this struct.
+ */
+struct media_request_object {
+	struct media_request_class *class;
+	struct list_head object_list;
+	struct kref kref;
+};
+
+/**
+ * struct media_request_ref - Reference to a media request object
+ *
+ * @new: The new object
+ * @old: The old object
+ * @req_list: List entry of in the request's object list
+ * @req: The request the reference is related to
+ * @complete: A reference has been marked complete
+ *
+ * Represents a reference to a media request object; object references are bound
+ * to requests.
+ */
+struct media_request_ref {
+	struct media_request_object *new;
+	struct media_request_object *old;
+	struct list_head req_list;
+	struct media_device_request *req;
+	bool complete;
+};
+
+#define media_request_for_each_ref(ref, req)	     \
+	lockdep_assert_held(&(req)->mdev->req_lock); \
+	list_for_each_entry(ref, req, obj_refs)
+
+
+/**
+ * media_request_class_register - Register a media device request class
+ *
+ * @mdev: The media device
+ * @class: The class to be registered
+ * @size: The size (in bytes) of an object in a class
+ * @completeable: Whether objects in this class must complete for the request to
+ *		  be completed
+ * @name: Name of the class for debug prints, may be NULL
+ *
+ * Registers a media device class for request objects. Objects are allocated by
+ * the framework. Sticky objects are kept after the request has been completed;
+ * they are configuration rather than a resource (such as buffers).
+ */
+void
+media_request_class_register(struct media_device *mdev,
+			     struct media_request_class *class,
+			     void (*release)(struct media_request_object *object),
+			     bool completeable);
+
+/**
+ * media_request_class_set_sticky - Make a class sticky
+ *
+ * @class: The request object class
+ * @sticky: The sticky object
+ *
+ * Makes a class sticky as well as sets the sticky object to a class. Sticky
+ * objects represent configuration which may be changed by a request but will
+ * prevail until changed again.
+ */
+void media_request_class_set_sticky(struct media_request_class *class,
+				    struct media_request_object *sticky);
+
+/**
+ * media_request_class_unregister - Unregister a media device request class
+ *
+ * @class: The class to unregister
+ */
+void media_request_class_unregister(struct media_request_class *class);
+
+/**
+ * media_request_object_put - Put a media request object
+ *
+ * @obj: The object
+ *
+ * Put a reference to a media request object. Once all references are gone, the
+ * object's memory is released.
+ */
+void media_request_object_put(struct media_request_object *obj);
+
+/**
+ * media_request_object_init - Initialise an allocated media request object
+ *
+ * @class: The class the object belongs to
+ *
+ * Initialise a media request object. The object will be released using the
+ * release function of the class once it has no references (this function
+ * initialises references to one).
+ */
+void media_request_object_init(struct media_request_class *class,
+			       struct media_request_object *obj);
+
+/**
+ * __media_request_ref_put - Put a reference to a request object
+ *
+ * @ref: The reference
+ *
+ * Put a reference to a media request object. The caller must be holding @struct
+ * media_device.req_lock.
+ */
+void __media_request_ref_put(struct media_request_ref *ref);
+
+/**
+ * media_request_ref_put - Put a reference to a request object
+ *
+ * @ref: The reference
+ *
+ * Put a reference to a media request object.
+ */
+void media_request_ref_put(struct media_request_ref *ref);
+
+/**
+ * media_request_object_bind - Bind an object to a request
+ *
+ * @req: The request where the object is to be added
+ * @obj: The object
+ *
+ * Bind an object to a request.
+ *
+ * Returns a reference to the bound object.
+ */
+struct media_request_ref *
+media_request_object_bind(struct media_device_request *req,
+			  struct media_request_object *obj);
+
+/**
+ * media_request_sticky_to_old - Move sticky configuration to request
+ *
+ * @req: The request
+ *
+ * Move the current configuration to the request's old configuration.
+ */
+void media_request_sticky_to_old(struct media_device_request *req);
+
+/**
+ * media_request_new_to_sticky - Make the request configuration stick
+ *
+ * @req: The request
+ *
+ * Make the configuration in the request the current configuration.
+ */
+void media_request_new_to_sticky(struct media_device_request *req);
+
+/**
+ * media_request_detach_old - Detach old configuration
+ *
+ * @req: The request
+ *
+ * Detach the previous (old) configuration from the request.
+ */
+void media_request_detach_old(struct media_device_request *req);
+
+/*
+ * media_device_ref_unbind - Unbind an object reference from a request
+ *
+ * @ref: The reference to be unbound.
+ *
+ * Unbind a previously bound reference from a request. The object is put by this
+ * function as well. Only references to completeable objects may be unbound.
+ */
+void media_request_ref_unbind(struct media_request_ref *ref);
+
+/*
+ * media_request_is_complete - Tell whether the media request is complete
+ *
+ * @req: The request
+ *
+ * Return true if all unsticky objects have been completed in a request.
+ */
+bool media_request_is_complete(struct media_device_request *req);
+
+/**
+ * media_request_ref_complete - Mark a reference complete
+ *
+ * @ref: The reference to the request
+ *
+ * Mark a part of the request as completed. Also puts the ref.
+ */
+void media_request_ref_complete(struct media_request_ref *ref);
-- 
2.7.4

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

* [RFC 5/8] videodev2.h: Add request_fd field to v4l2_buffer
  2018-03-09 23:48 [RFC 0/8] Preparing the request API Sakari Ailus
                   ` (3 preceding siblings ...)
  2018-03-09 23:48 ` [RFC 4/8] media: Add support for request classes and objects Sakari Ailus
@ 2018-03-09 23:48 ` Sakari Ailus
  2018-03-09 23:48 ` [RFC 6/8] videodev2.h: add request_fd field to v4l2_ext_controls Sakari Ailus
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Sakari Ailus @ 2018-03-09 23:48 UTC (permalink / raw)
  To: linux-media; +Cc: acourbot, Hans Verkuil

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

When queuing buffers allow for passing the request that should
be associated with this buffer.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
[acourbot@chromium.org: make request ID 32-bit]
Signed-off-by: Alexandre Courbot <acourbot@chromium.org>
[Sakari Ailus: requests fds are int; use assign_in_user in get_v4l2_buffer]
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/common/videobuf2/videobuf2-v4l2.c | 2 +-
 drivers/media/usb/cpia2/cpia2_v4l.c             | 2 +-
 drivers/media/v4l2-core/v4l2-compat-ioctl32.c   | 7 ++++---
 drivers/media/v4l2-core/v4l2-ioctl.c            | 4 ++--
 include/uapi/linux/videodev2.h                  | 3 ++-
 5 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c
index 886a2d8..6d4d184 100644
--- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
+++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
@@ -203,7 +203,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_fd = 0;
 	b->reserved = 0;
 
 	if (q->is_multiplanar) {
diff --git a/drivers/media/usb/cpia2/cpia2_v4l.c b/drivers/media/usb/cpia2/cpia2_v4l.c
index 99f106b..af42ce3 100644
--- a/drivers/media/usb/cpia2/cpia2_v4l.c
+++ b/drivers/media/usb/cpia2/cpia2_v4l.c
@@ -948,7 +948,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->reserved2 = 0;
+	buf->request_fd = 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 5198c9e..61a8bd4 100644
--- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
+++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
@@ -386,7 +386,7 @@ struct v4l2_buffer32 {
 		__s32		fd;
 	} m;
 	__u32			length;
-	__u32			reserved2;
+	__s32			request_fd;
 	__u32			reserved;
 };
 
@@ -500,7 +500,8 @@ static int get_v4l2_buffer32(struct v4l2_buffer __user *kp,
 	    get_user(memory, &up->memory) ||
 	    put_user(memory, &kp->memory) ||
 	    get_user(length, &up->length) ||
-	    put_user(length, &kp->length))
+	    put_user(length, &kp->length) ||
+	    assign_in_user(&kp->request_fd, &up->request_fd))
 		return -EFAULT;
 
 	if (V4L2_TYPE_IS_OUTPUT(type))
@@ -604,7 +605,7 @@ static int put_v4l2_buffer32(struct v4l2_buffer __user *kp,
 	    assign_in_user(&up->timestamp.tv_usec, &kp->timestamp.tv_usec) ||
 	    copy_in_user(&up->timecode, &kp->timecode, sizeof(kp->timecode)) ||
 	    assign_in_user(&up->sequence, &kp->sequence) ||
-	    assign_in_user(&up->reserved2, &kp->reserved2) ||
+	    assign_in_user(&up->request_fd, &kp->request_fd) ||
 	    assign_in_user(&up->reserved, &kp->reserved) ||
 	    get_user(length, &kp->length) ||
 	    put_user(length, &up->length))
diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
index f697c23..6c623e5 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -437,13 +437,13 @@ static void v4l_print_buffer(const void *arg, bool write_only)
 	const struct v4l2_plane *plane;
 	int i;
 
-	pr_cont("%02ld:%02d:%02d.%08ld index=%d, type=%s, flags=0x%08x, field=%s, sequence=%d, memory=%s",
+	pr_cont("%02ld:%02d:%02d.%08ld index=%d, type=%s, request_fd=%d, 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_fd,
 			p->flags, prt_names(p->field, v4l2_field_names),
 			p->sequence, prt_names(p->memory, v4l2_memory_names));
 
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 9827189..4fd46ae 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -909,6 +909,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_fd: fd of the request that this buffer should use
  *
  * Contains data exchanged by application and driver using one of the Streaming
  * I/O methods.
@@ -932,7 +933,7 @@ struct v4l2_buffer {
 		__s32		fd;
 	} m;
 	__u32			length;
-	__u32			reserved2;
+	__s32			request_fd;
 	__u32			reserved;
 };
 
-- 
2.7.4

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

* [RFC 6/8] videodev2.h: add request_fd field to v4l2_ext_controls
  2018-03-09 23:48 [RFC 0/8] Preparing the request API Sakari Ailus
                   ` (4 preceding siblings ...)
  2018-03-09 23:48 ` [RFC 5/8] videodev2.h: Add request_fd field to v4l2_buffer Sakari Ailus
@ 2018-03-09 23:48 ` Sakari Ailus
  2018-03-09 23:48 ` [RFC 7/8] staging: media: atomisp: Remove v4l2_buffer.reserved2 field hack Sakari Ailus
  2018-03-09 23:48 ` [RFC 8/8] vb2: Add support for requests Sakari Ailus
  7 siblings, 0 replies; 9+ messages in thread
From: Sakari Ailus @ 2018-03-09 23:48 UTC (permalink / raw)
  To: linux-media; +Cc: acourbot

From: Alexandre Courbot <acourbot@chromium.org>

Allow to specify a request to be used with the S_EXT_CTRLS and
G_EXT_CTRLS operations.

Signed-off-by: Alexandre Courbot <acourbot@chromium.org>
[Sakari Ailus: reserved no longer an array, add compat32 code]
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 9 ++++++---
 drivers/media/v4l2-core/v4l2-ioctl.c          | 2 +-
 include/uapi/linux/videodev2.h                | 3 ++-
 3 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
index 61a8bd4..9adb367 100644
--- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
+++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
@@ -733,7 +733,8 @@ struct v4l2_ext_controls32 {
 	__u32 which;
 	__u32 count;
 	__u32 error_idx;
-	__u32 reserved[2];
+	__s32 request_fd;
+	__u32 reserved;
 	compat_caddr_t controls; /* actually struct v4l2_ext_control32 * */
 };
 
@@ -808,7 +809,8 @@ static int get_v4l2_ext_controls32(struct file *file,
 	    get_user(count, &up->count) ||
 	    put_user(count, &kp->count) ||
 	    assign_in_user(&kp->error_idx, &up->error_idx) ||
-	    copy_in_user(kp->reserved, up->reserved, sizeof(kp->reserved)))
+	    assign_in_user(&kp->request_fd, &up->request_fd) ||
+	    assign_in_user(&kp->reserved, &up->reserved))
 		return -EFAULT;
 
 	if (count == 0)
@@ -866,7 +868,8 @@ static int put_v4l2_ext_controls32(struct file *file,
 	    get_user(count, &kp->count) ||
 	    put_user(count, &up->count) ||
 	    assign_in_user(&up->error_idx, &kp->error_idx) ||
-	    copy_in_user(up->reserved, kp->reserved, sizeof(up->reserved)) ||
+	    assign_in_user(&up->request_fd, &kp->request_fd) ||
+	    assign_in_user(&up->reserved, &kp->reserved) ||
 	    get_user(kcontrols, &kp->controls))
 		return -EFAULT;
 
diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
index 6c623e5..b58018b 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -870,7 +870,7 @@ static int check_ext_ctrls(struct v4l2_ext_controls *c, int allow_priv)
 	__u32 i;
 
 	/* zero the reserved fields */
-	c->reserved[0] = c->reserved[1] = 0;
+	c->reserved = 0;
 	for (i = 0; i < c->count; i++)
 		c->controls[i].reserved2[0] = 0;
 
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 4fd46ae..ac502ad 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -1592,7 +1592,8 @@ struct v4l2_ext_controls {
 	};
 	__u32 count;
 	__u32 error_idx;
-	__u32 reserved[2];
+	__s32 request_fd;
+	__u32 reserved;
 	struct v4l2_ext_control *controls;
 };
 
-- 
2.7.4

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

* [RFC 7/8] staging: media: atomisp: Remove v4l2_buffer.reserved2 field hack
  2018-03-09 23:48 [RFC 0/8] Preparing the request API Sakari Ailus
                   ` (5 preceding siblings ...)
  2018-03-09 23:48 ` [RFC 6/8] videodev2.h: add request_fd field to v4l2_ext_controls Sakari Ailus
@ 2018-03-09 23:48 ` Sakari Ailus
  2018-03-09 23:48 ` [RFC 8/8] vb2: Add support for requests Sakari Ailus
  7 siblings, 0 replies; 9+ messages in thread
From: Sakari Ailus @ 2018-03-09 23:48 UTC (permalink / raw)
  To: linux-media; +Cc: acourbot

The atomisp driver used to use the reserved2 field in struct v4l2_buffer
for picking a particular ISP configuration for the buffer. As reserved2
field will have new use soon, remove this hack.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 .../staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c  | 17 +++--------------
 1 file changed, 3 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c
index 5c84dd6..182bb70 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_ioctl.c
@@ -1283,16 +1283,7 @@ static int atomisp_qbuf(struct file *file, void *fh, struct v4l2_buffer *buf)
 	if (!((buf->flags & NOFLUSH_FLAGS) == NOFLUSH_FLAGS))
 		wbinvd();
 
-	if (!atomisp_is_vf_pipe(pipe) &&
-	    (buf->reserved2 & ATOMISP_BUFFER_HAS_PER_FRAME_SETTING)) {
-		/* this buffer will have a per-frame parameter */
-		pipe->frame_request_config_id[buf->index] = buf->reserved2 &
-					~ATOMISP_BUFFER_HAS_PER_FRAME_SETTING;
-		dev_dbg(isp->dev, "This buffer requires per_frame setting which has isp_config_id %d\n",
-			pipe->frame_request_config_id[buf->index]);
-	} else {
-		pipe->frame_request_config_id[buf->index] = 0;
-	}
+	pipe->frame_request_config_id[buf->index] = 0;
 
 	pipe->frame_params[buf->index] = NULL;
 
@@ -1470,12 +1461,10 @@ static int atomisp_dqbuf(struct file *file, void *fh, struct v4l2_buffer *buf)
 	buf->reserved &= 0x0000ffff;
 	if (!(buf->flags & V4L2_BUF_FLAG_ERROR))
 		buf->reserved |= __get_frame_exp_id(pipe, buf) << 16;
-	buf->reserved2 = pipe->frame_config_id[buf->index];
 	rt_mutex_unlock(&isp->mutex);
 
-	dev_dbg(isp->dev, "dqbuf buffer %d (%s) for asd%d with exp_id %d, isp_config_id %d\n",
-		buf->index, vdev->name, asd->index, buf->reserved >> 16,
-		buf->reserved2);
+	dev_dbg(isp->dev, "dqbuf buffer %d (%s) for asd%d with exp_id %d\n",
+		buf->index, vdev->name, asd->index, buf->reserved >> 16);
 	return 0;
 }
 
-- 
2.7.4

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

* [RFC 8/8] vb2: Add support for requests
  2018-03-09 23:48 [RFC 0/8] Preparing the request API Sakari Ailus
                   ` (6 preceding siblings ...)
  2018-03-09 23:48 ` [RFC 7/8] staging: media: atomisp: Remove v4l2_buffer.reserved2 field hack Sakari Ailus
@ 2018-03-09 23:48 ` Sakari Ailus
  7 siblings, 0 replies; 9+ messages in thread
From: Sakari Ailus @ 2018-03-09 23:48 UTC (permalink / raw)
  To: linux-media; +Cc: acourbot

Associate a buffer to a request when it is queued and disassociate when it
is done.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/common/videobuf2/videobuf2-core.c | 43 ++++++++++++++++++++++++-
 drivers/media/common/videobuf2/videobuf2-v4l2.c | 24 +++++++++++++-
 include/media/videobuf2-core.h                  | 19 +++++++++++
 include/media/videobuf2-v4l2.h                  |  2 ++
 4 files changed, 86 insertions(+), 2 deletions(-)

diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
index d3f7bb3..0e8d555 100644
--- a/drivers/media/common/videobuf2/videobuf2-core.c
+++ b/drivers/media/common/videobuf2/videobuf2-core.c
@@ -346,6 +346,9 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum vb2_memory memory,
 			break;
 		}
 
+		if (q->class)
+			media_request_object_init(q->class, &vb->req_obj);
+
 		vb->state = VB2_BUF_STATE_DEQUEUED;
 		vb->vb2_queue = q;
 		vb->num_planes = num_planes;
@@ -520,7 +523,10 @@ static int __vb2_queue_free(struct vb2_queue *q, unsigned int buffers)
 	/* Free videobuf buffers */
 	for (buffer = q->num_buffers - buffers; buffer < q->num_buffers;
 	     ++buffer) {
-		kfree(q->bufs[buffer]);
+		if (q->class)
+			media_request_object_put(&q->bufs[buffer]->req_obj);
+		else
+			kfree(q->bufs[buffer]);
 		q->bufs[buffer] = NULL;
 	}
 
@@ -944,6 +950,10 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum vb2_buffer_state state)
 	default:
 		/* Inform any processes that may be waiting for buffers */
 		wake_up(&q->done_wq);
+		if (vb->req_ref) {
+			media_request_ref_complete(vb->req_ref);
+			vb->req_ref = NULL;
+		}
 		break;
 	}
 }
@@ -1249,6 +1259,32 @@ static int __buf_prepare(struct vb2_buffer *vb, const void *pb)
 		return -EIO;
 	}
 
+	if (vb->request_fd) {
+		struct media_device_request *req;
+		struct media_request_ref *ref;
+
+		if (!q->class) {
+			dprintk(1, "requests not enabled for the queue\n");
+			return -EINVAL;
+		}
+
+		req = media_device_request_find(q->class->mdev, vb->request_fd);
+		if (IS_ERR(req)) {
+			dprintk(1, "no request found for fd %d (%ld)\n",
+				vb->request_fd, PTR_ERR(req));
+			return PTR_ERR(req);
+		}
+
+		ref = media_request_object_bind(req,
+						&q->bufs[vb->index]->req_obj);
+		media_device_request_put(req);
+
+		if (IS_ERR(ref))
+			return PTR_ERR(ref);
+
+		vb->req_ref = ref;
+	}
+
 	vb->state = VB2_BUF_STATE_PREPARING;
 
 	switch (q->memory) {
@@ -1269,6 +1305,8 @@ static int __buf_prepare(struct vb2_buffer *vb, const void *pb)
 	if (ret) {
 		dprintk(1, "buffer preparation failed: %d\n", ret);
 		vb->state = VB2_BUF_STATE_DEQUEUED;
+		media_request_ref_unbind(vb->req_ref);
+		vb->req_ref = NULL;
 		return ret;
 	}
 
@@ -2037,6 +2075,9 @@ void vb2_core_queue_release(struct vb2_queue *q)
 	mutex_lock(&q->mmap_lock);
 	__vb2_queue_free(q, q->num_buffers);
 	mutex_unlock(&q->mmap_lock);
+	media_request_class_unregister(q->class);
+	kfree(q->class);
+	q->class = NULL;
 }
 EXPORT_SYMBOL_GPL(vb2_core_queue_release);
 
diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c
index 6d4d184..ca36109 100644
--- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
+++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
@@ -196,6 +196,7 @@ static void __fill_v4l2_buffer(struct vb2_buffer *vb, void *pb)
 	b->index = vb->index;
 	b->type = vb->type;
 	b->memory = vb->memory;
+	b->request_fd = vb->request_fd;
 	b->bytesused = 0;
 
 	b->flags = vbuf->flags;
@@ -203,7 +204,6 @@ 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->request_fd = 0;
 	b->reserved = 0;
 
 	if (q->is_multiplanar) {
@@ -319,6 +319,7 @@ static int __fill_vb2_buffer(struct vb2_buffer *vb,
 		return -EINVAL;
 	}
 	vb->timestamp = 0;
+	vb->request_fd = b->request_fd;
 	vbuf->sequence = 0;
 
 	if (V4L2_TYPE_IS_MULTIPLANAR(b->type)) {
@@ -667,6 +668,27 @@ int vb2_queue_init(struct vb2_queue *q)
 }
 EXPORT_SYMBOL_GPL(vb2_queue_init);
 
+void vb2_media_req_class_release(struct media_request_object *obj)
+{
+	struct vb2_v4l2_buffer *vbuf =
+		to_vb2_v4l2_buffer(media_request_object_to_vb2_buffer(obj));
+
+	kfree(vbuf);
+}
+
+int vb2_queue_allow_requests(struct vb2_queue *q, struct media_device *mdev)
+{
+	q->class = kzalloc(sizeof(*q->class), GFP_KERNEL);
+	if (!q->class)
+		return -ENOMEM;
+
+	media_request_class_register(mdev, q->class,
+				     vb2_media_req_class_release, true);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(vb2_queue_allow_requests);
+
 void vb2_queue_release(struct vb2_queue *q)
 {
 	vb2_core_queue_release(q);
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index 5b6c541..7aab811 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -18,6 +18,8 @@
 #include <linux/dma-buf.h>
 #include <linux/bitops.h>
 
+#include <media/media-request.h>
+
 #define VB2_MAX_FRAME	(32)
 #define VB2_MAX_PLANES	(8)
 
@@ -42,6 +44,8 @@ enum vb2_memory {
 	VB2_MEMORY_DMABUF	= 4,
 };
 
+struct media_request_class;
+struct media_request_ref;
 struct vb2_fileio_data;
 struct vb2_threadio_data;
 
@@ -255,12 +259,19 @@ struct vb2_buffer {
 	 * done_entry:		entry on the list that stores all buffers ready
 	 *			to be dequeued to userspace
 	 * vb2_plane:		per-plane information; do not change
+	 * req_obj:		media request object
+	 * req_ref:		media request reference (stored between qbuf --
+	 *			dqbuf)
+	 * request_fd		file descriptor of the request
 	 */
 	enum vb2_buffer_state	state;
 
 	struct vb2_plane	planes[VB2_MAX_PLANES];
 	struct list_head	queued_entry;
 	struct list_head	done_entry;
+	struct media_request_object req_obj;
+	struct media_request_ref *req_ref;
+	int			request_fd;
 #ifdef CONFIG_VIDEO_ADV_DEBUG
 	/*
 	 * Counters for how often these buffer-related ops are
@@ -293,6 +304,12 @@ struct vb2_buffer {
 #endif
 };
 
+static inline struct vb2_buffer *
+media_request_object_to_vb2_buffer(struct media_request_object *obj)
+{
+	return container_of(obj, struct vb2_buffer, req_obj);
+}
+
 /**
  * struct vb2_ops - driver-specific callbacks.
  *
@@ -502,6 +519,7 @@ struct vb2_buf_ops {
  *		when a buffer with the %V4L2_BUF_FLAG_LAST is dequeued.
  * @fileio:	file io emulator internal data, used only if emulator is active
  * @threadio:	thread io internal data, used only if thread is active
+ * @req_class:	The media request class for buffers in this queue
  */
 struct vb2_queue {
 	unsigned int			type;
@@ -555,6 +573,7 @@ struct vb2_queue {
 
 	struct vb2_fileio_data		*fileio;
 	struct vb2_threadio_data	*threadio;
+	struct media_request_class	*class;
 
 #ifdef CONFIG_VIDEO_ADV_DEBUG
 	/*
diff --git a/include/media/videobuf2-v4l2.h b/include/media/videobuf2-v4l2.h
index 3d5e2d7..5d20e2e 100644
--- a/include/media/videobuf2-v4l2.h
+++ b/include/media/videobuf2-v4l2.h
@@ -32,6 +32,7 @@
  *		&enum v4l2_field.
  * @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``.
@@ -43,6 +44,7 @@ struct vb2_v4l2_buffer {
 	__u32			field;
 	struct v4l2_timecode	timecode;
 	__u32			sequence;
+	__u32			request;
 };
 
 /*
-- 
2.7.4

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

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

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-09 23:48 [RFC 0/8] Preparing the request API Sakari Ailus
2018-03-09 23:48 ` [RFC 1/8] media: Support variable size IOCTL arguments Sakari Ailus
2018-03-09 23:48 ` [RFC 2/8] media: Add request API Sakari Ailus
2018-03-09 23:48 ` [RFC 3/8] media: Support polling for completed requests Sakari Ailus
2018-03-09 23:48 ` [RFC 4/8] media: Add support for request classes and objects Sakari Ailus
2018-03-09 23:48 ` [RFC 5/8] videodev2.h: Add request_fd field to v4l2_buffer Sakari Ailus
2018-03-09 23:48 ` [RFC 6/8] videodev2.h: add request_fd field to v4l2_ext_controls Sakari Ailus
2018-03-09 23:48 ` [RFC 7/8] staging: media: atomisp: Remove v4l2_buffer.reserved2 field hack Sakari Ailus
2018-03-09 23:48 ` [RFC 8/8] vb2: Add support for requests Sakari Ailus

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