[RFC PATCH 0/9] V4L2 Jobs API WIP

[RFC PATCH 0/9] V4L2 Jobs API WIP

[Alexandre Courbot]

Hi everyone,

Here is a new attempt at the "request" (which I propose to rename "jobs") API
for V4L2, hopefully in a manner that can converge to something that will be
merged. The core ideas should be easy to grasp for those familiar with the
previous attemps, yet there are a few important differences.

Most notably, user-space does not need to explicitly allocate and manage
requests/jobs (but still can if this makes sense). We noticed that only specific
use-cases require such an explicit management, and opted for a jobs queue that
controls the flow of work over a set of opened devices. This should simplify
user-space code quite a bit, while still retaining the ability to manage states
explicitly like the previous request API proposals allowed to do.

The jobs API defines a few new concepts that user-space can use to control the
workflow on a set of opened V4L2 devices:

A JOB QUEUE can be created from a set of opened FDs that are part of a pipeline
and need to cooperate (be it capture, m2m, or media controller devices).

A JOB can then be set up with regular (if slightly modified) V4L2 ioctls, and
then submitted to the job queue. Once the job queue schedules the job, its
parameters (controls, etc) are applied to the devices of the queue, and itsd
buffers are processed. Immediately after a job is submitted, the next job is
ready to be set up without further user action.

Once a job completes, it must be dequeued and user-space can then read back its
properties (notably controls) at completion time.

Internally, the state of jobs is managed through STATE HANDLERS. Each driver
supporting the jobs API needs to specify an implementation of a state handler.
Fortunately, most drivers can rely on the generic state handler implementation
that simply records and replays a job's parameter using standard V4L2 functions.
Thanks to this, adding jobs API support to a driver relying on the control
framework and vb2 only requires a dozen lines of codes.

Drivers with specific needs or opportunities for optimization can however
provide their own implementation of a state handler. This may in particular be
beneficial for hardware that supports configuration or command buffers (thinking
about VSP1 here).

This is still very early work, and focus has been on the following points:

* Provide something that anybody can test (currently using vim2m and vivid),
* Reuse the current V4L2 APIs as much as possible,
* Remain flexible enough to accomodate the inevitable changes that will be
  requested,
* Keep line count low, even if functionality is missing at the moment.

Please keep this in mind while going through the patches. In particular, at the
moment the parameters of a job are limited to integer controls. I know that much
more is expected, but V4L2 has quite a learning curve and I preferred to focus
on the general concepts for now. More is coming though! :)

I have written two small example programs that demonstrate the use of this API:

- With a codec device (vim2m): https://gist.github.com/Gnurou/34c35f1f8e278dad454b51578d239a42

- With a capture device (vivid): https://gist.github.com/Gnurou/5052e6ab41e7c55164b75d2970bc5a04

Considering the history with the request API, I don't expect everything proposed
here to be welcome or understood immediately. In particular I apologize for not
reusing any of the previous attempts - I was just more comfortable laying down
my ideas from scratch.

If this proposal is not dismissed as complete garbage I will also be happy to
discuss it in-person at the mini-summit in Prague. :)

Cheers,
Alex.

Alexandre Courbot (9):
  [media] v4l2-core: add v4l2_is_v4l2_file function
  [media] v4l2-core: add core jobs API support
  [media] videobuf2: add support for jobs API
  [media] v4l2-ctrls: add support for jobs API
  [media] v4l2-job: add generic jobs ops
  [media] m2m: add generic support for jobs API
  [media] vim2m: add jobs API support
  [media] vivid: add jobs API support for capture device
  [media] document jobs API

 Documentation/media/intro.rst                      |   2 +
 Documentation/media/media_uapi.rst                 |   1 +
 Documentation/media/uapi/jobs/jobs-api.rst         |  23 +
 Documentation/media/uapi/jobs/jobs-example.rst     |  69 ++
 Documentation/media/uapi/jobs/jobs-intro.rst       |  61 ++
 Documentation/media/uapi/jobs/jobs-queue.rst       |  73 ++
 Documentation/media/uapi/jobs/jobs-queue.svg       | 192 ++++++
 .../media/uapi/v4l/vidioc-g-ext-ctrls.rst          |   6 +
 drivers/media/platform/vim2m.c                     |  24 +
 drivers/media/platform/vivid/vivid-core.c          |  16 +
 drivers/media/platform/vivid/vivid-core.h          |   2 +
 drivers/media/platform/vivid/vivid-kthread-cap.c   |   5 +
 drivers/media/v4l2-core/Makefile                   |   4 +-
 drivers/media/v4l2-core/v4l2-ctrls.c               |  50 +-
 drivers/media/v4l2-core/v4l2-dev.c                 |  12 +
 drivers/media/v4l2-core/v4l2-job-generic.c         | 394 +++++++++++
 drivers/media/v4l2-core/v4l2-jobqueue-dev.c        | 173 +++++
 drivers/media/v4l2-core/v4l2-jobqueue.c            | 764 +++++++++++++++++++++
 drivers/media/v4l2-core/v4l2-mem2mem.c             |  19 +
 drivers/media/v4l2-core/videobuf2-core.c           |  33 +-
 include/media/v4l2-ctrls.h                         |   6 +
 include/media/v4l2-dev.h                           |  13 +
 include/media/v4l2-fh.h                            |   4 +
 include/media/v4l2-job-generic.h                   |  47 ++
 include/media/v4l2-job-state.h                     |  75 ++
 include/media/v4l2-jobqueue-dev.h                  |  24 +
 include/media/v4l2-jobqueue.h                      |  54 ++
 include/media/v4l2-mem2mem.h                       |  11 +
 include/media/videobuf2-core.h                     |  16 +
 include/uapi/linux/v4l2-jobs.h                     |  40 ++
 include/uapi/linux/videodev2.h                     |   2 +
 31 files changed, 2205 insertions(+), 10 deletions(-)
 create mode 100644 Documentation/media/uapi/jobs/jobs-api.rst
 create mode 100644 Documentation/media/uapi/jobs/jobs-example.rst
 create mode 100644 Documentation/media/uapi/jobs/jobs-intro.rst
 create mode 100644 Documentation/media/uapi/jobs/jobs-queue.rst
 create mode 100644 Documentation/media/uapi/jobs/jobs-queue.svg
 create mode 100644 drivers/media/v4l2-core/v4l2-job-generic.c
 create mode 100644 drivers/media/v4l2-core/v4l2-jobqueue-dev.c
 create mode 100644 drivers/media/v4l2-core/v4l2-jobqueue.c
 create mode 100644 include/media/v4l2-job-generic.h
 create mode 100644 include/media/v4l2-job-state.h
 create mode 100644 include/media/v4l2-jobqueue-dev.h
 create mode 100644 include/media/v4l2-jobqueue.h
 create mode 100644 include/uapi/linux/v4l2-jobs.h

-- 
2.14.2.822.g60be5d43e6-goog

[RFC PATCH 1/9] [media] v4l2-core: add v4l2_is_v4l2_file function

[Alexandre Courbot]

Add a function that checks whether a given open file is a v4l2 device
instance. This will be useful for job queue creation as we are passed a
set of FDs and we need to make this check.

Signed-off-by: Alexandre Courbot <acourbot@chromium.org>
---
 drivers/media/v4l2-core/v4l2-dev.c | 6 ++++++
 include/media/v4l2-dev.h           | 9 +++++++++
 2 files changed, 15 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
index c647ba648805..5a7063886c93 100644
--- a/drivers/media/v4l2-core/v4l2-dev.c
+++ b/drivers/media/v4l2-core/v4l2-dev.c
@@ -470,6 +470,12 @@ static const struct file_operations v4l2_fops = {
 	.llseek = no_llseek,
 };
 
+bool v4l2_is_v4l2_file(struct file *filp)
+{
+	return filp->f_op == &v4l2_fops;
+}
+EXPORT_SYMBOL(v4l2_is_v4l2_file);
+
 /**
  * get_index - assign stream index number based on v4l2_dev
  * @vdev: video_device to assign index number to, vdev->v4l2_dev should be assigned
diff --git a/include/media/v4l2-dev.h b/include/media/v4l2-dev.h
index e657614521e3..b73d646980da 100644
--- a/include/media/v4l2-dev.h
+++ b/include/media/v4l2-dev.h
@@ -395,6 +395,15 @@ void video_device_release(struct video_device *vdev);
  */
 void video_device_release_empty(struct video_device *vdev);
 
+/**
+ * v4l2_is_v4l2_file - Check whether a file describes a V4L2 device
+ *
+ * @filp: opened file to check
+ *
+ * Returns true of the file is a V4L2 device, false otherwise.
+ */
+bool v4l2_is_v4l2_file(struct file *filp);
+
 /**
  * v4l2_is_known_ioctl - Checks if a given cmd is a known V4L ioctl
  *
-- 
2.14.2.822.g60be5d43e6-goog

[RFC PATCH 2/9] [media] v4l2-core: add core jobs API support

[Alexandre Courbot]

Add core support code for jobs API. This manages the life cycle of jobs
and creation of a jobs queue, as well as the interface for job states.

It also exposes the user-space jobs API.

Signed-off-by: Alexandre Courbot <acourbot@chromium.org>
---
 drivers/media/v4l2-core/Makefile            |   3 +-
 drivers/media/v4l2-core/v4l2-dev.c          |   6 +
 drivers/media/v4l2-core/v4l2-jobqueue-dev.c | 173 +++++++
 drivers/media/v4l2-core/v4l2-jobqueue.c     | 764 ++++++++++++++++++++++++++++
 include/media/v4l2-dev.h                    |   4 +
 include/media/v4l2-fh.h                     |   4 +
 include/media/v4l2-job-state.h              |  75 +++
 include/media/v4l2-jobqueue-dev.h           |  24 +
 include/media/v4l2-jobqueue.h               |  54 ++
 include/uapi/linux/v4l2-jobs.h              |  40 ++
 include/uapi/linux/videodev2.h              |   2 +
 11 files changed, 1148 insertions(+), 1 deletion(-)
 create mode 100644 drivers/media/v4l2-core/v4l2-jobqueue-dev.c
 create mode 100644 drivers/media/v4l2-core/v4l2-jobqueue.c
 create mode 100644 include/media/v4l2-job-state.h
 create mode 100644 include/media/v4l2-jobqueue-dev.h
 create mode 100644 include/media/v4l2-jobqueue.h
 create mode 100644 include/uapi/linux/v4l2-jobs.h

diff --git a/drivers/media/v4l2-core/Makefile b/drivers/media/v4l2-core/Makefile
index 098ad5fd5231..a717bb8f1a25 100644
--- a/drivers/media/v4l2-core/Makefile
+++ b/drivers/media/v4l2-core/Makefile
@@ -6,7 +6,8 @@ tuner-objs	:=	tuner-core.o
 
 videodev-objs	:=	v4l2-dev.o v4l2-ioctl.o v4l2-device.o v4l2-fh.o \
 			v4l2-event.o v4l2-ctrls.o v4l2-subdev.o v4l2-clk.o \
-			v4l2-async.o
+			v4l2-async.o v4l2-jobqueue.o v4l2-jobqueue-dev.o
+
 ifeq ($(CONFIG_COMPAT),y)
   videodev-objs += v4l2-compat-ioctl32.o
 endif
diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
index 5a7063886c93..fb229b671b9d 100644
--- a/drivers/media/v4l2-core/v4l2-dev.c
+++ b/drivers/media/v4l2-core/v4l2-dev.c
@@ -30,6 +30,7 @@
 #include <media/v4l2-common.h>
 #include <media/v4l2-device.h>
 #include <media/v4l2-ioctl.h>
+#include <media/v4l2-jobqueue-dev.h>
 
 #define VIDEO_NUM_DEVICES	256
 #define VIDEO_NAME              "video4linux"
@@ -1058,6 +1059,10 @@ static int __init videodev_init(void)
 		return -EIO;
 	}
 
+	ret = v4l2_jobqueue_device_init();
+	if (ret < 0)
+		printk(KERN_WARNING "video_dev: channel initialization failed\n");
+
 	return 0;
 }
 
@@ -1065,6 +1070,7 @@ static void __exit videodev_exit(void)
 {
 	dev_t dev = MKDEV(VIDEO_MAJOR, 0);
 
+	v4l2_jobqueue_device_exit();
 	class_unregister(&video_class);
 	unregister_chrdev_region(dev, VIDEO_NUM_DEVICES);
 }
diff --git a/drivers/media/v4l2-core/v4l2-jobqueue-dev.c b/drivers/media/v4l2-core/v4l2-jobqueue-dev.c
new file mode 100644
index 000000000000..688c4ba275a6
--- /dev/null
+++ b/drivers/media/v4l2-core/v4l2-jobqueue-dev.c
@@ -0,0 +1,173 @@
+/*
+    V4L2 job queue device
+
+    Copyright (C) 2017  The Chromium project
+
+    This program is free software; you can redistribute it and/or modify
+    it under the terms of the GNU General Public License as published by
+    the Free Software Foundation; either version 2 of the License, or
+    (at your option) any later version.
+
+    This program is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+
+ */
+
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/fs.h>
+#include <media/v4l2-ioctl.h>
+#include <media/v4l2-jobqueue.h>
+#include <uapi/linux/v4l2-jobs.h>
+
+#define CLASS_NAME "v4l2_jobqueue"
+#define DEVICE_NAME "v4l2_jobqueue"
+
+static int major;
+static struct class *jobqueue_class;
+static struct device *jobqueue_device;
+
+static int v4l2_jobqueue_device_open(struct inode *inode, struct file *filp)
+{
+	struct v4l2_jobqueue *jq;
+
+	jq = v4l2_jobqueue_new();
+	if (IS_ERR(jq))
+		return PTR_ERR(jq);
+
+	filp->private_data = jq;
+
+	return 0;
+}
+
+static int v4l2_jobqueue_device_release(struct inode *inode, struct file *filp)
+{
+	struct v4l2_jobqueue *jq = filp->private_data;
+
+	return v4l2_jobqueue_del(jq);
+}
+
+static long v4l2_jobqueue_ioctl_init(struct file *filp, void *arg)
+{
+	struct v4l2_jobqueue *jq = filp->private_data;
+	struct v4l2_jobqueue_init *cinit = arg;
+
+	return v4l2_jobqueue_init(jq, cinit);
+}
+
+static long v4l2_jobqueue_device_ioctl_qjob(struct file *filp, void *arg)
+{
+	struct v4l2_jobqueue *jq = filp->private_data;
+
+	return v4l2_jobqueue_qjob(jq);
+}
+
+static long v4l2_jobqueue_device_ioctl_dqjob(struct file *filp, void *arg)
+{
+	struct v4l2_jobqueue *jq = filp->private_data;
+
+	return v4l2_jobqueue_dqjob(jq);
+}
+
+static long v4l2_jobqueue_device_ioctl_export_job(struct file *filp, void *arg)
+{
+	struct v4l2_jobqueue *jq = filp->private_data;
+	struct v4l2_jobqueue_job *job = arg;
+
+	return v4l2_jobqueue_export_job(jq, job);
+}
+
+static long v4l2_jobqueue_device_ioctl_import_job(struct file *filp, void *arg)
+{
+	struct v4l2_jobqueue *jq = filp->private_data;
+	struct v4l2_jobqueue_job *job = arg;
+
+	return v4l2_jobqueue_import_job(jq, job);
+}
+
+static long v4l2_jobqueue_device_do_ioctl(struct file *filp, unsigned int cmd,
+					  void *arg)
+{
+	switch (cmd) {
+		case VIDIOC_JOBQUEUE_INIT:
+			return v4l2_jobqueue_ioctl_init(filp, arg);
+
+		case VIDIOC_JOBQUEUE_QJOB:
+			return v4l2_jobqueue_device_ioctl_qjob(filp, arg);
+
+		case VIDIOC_JOBQUEUE_DQJOB:
+			return v4l2_jobqueue_device_ioctl_dqjob(filp, arg);
+
+		case VIDIOC_JOBQUEUE_EXPORT_JOB:
+			return v4l2_jobqueue_device_ioctl_export_job(filp, arg);
+
+		case VIDIOC_JOBQUEUE_IMPORT_JOB:
+			return v4l2_jobqueue_device_ioctl_import_job(filp, arg);
+
+		default:
+			pr_err("Invalid ioctl!\n");
+			return -EINVAL;
+	}
+
+	return 0;
+}
+
+static long v4l2_jobqueue_device_ioctl(struct file *filp, unsigned int cmd,
+				       unsigned long arg)
+{
+	return video_usercopy(filp, cmd, arg, v4l2_jobqueue_device_do_ioctl);
+}
+
+static const struct file_operations v4l2_jobqueue_devnode_fops = {
+	.owner = THIS_MODULE,
+	.open = v4l2_jobqueue_device_open,
+	.unlocked_ioctl = v4l2_jobqueue_device_ioctl,
+#ifdef CONFIG_COMPAT
+	/* TODO */
+	/* .compat_ioctl = jobqueue_compat_ioctl, */
+#endif
+	.release = v4l2_jobqueue_device_release,
+};
+
+int __init v4l2_jobqueue_device_init(void)
+{
+	/* Set to error value so v4l2_jobqueue_device_exit does nothing if we
+	 * don't initialize properly */
+	jobqueue_device = ERR_PTR(-EINVAL);
+
+	major = register_chrdev(0, DEVICE_NAME, &v4l2_jobqueue_devnode_fops);
+	if (major < 0) {
+		pr_err("unable to allocate major\n");
+		return major;
+	}
+
+	jobqueue_class = class_create(THIS_MODULE, CLASS_NAME);
+	if (IS_ERR(jobqueue_class)) {
+		pr_err("cannot create class\n");
+		unregister_chrdev(major, DEVICE_NAME);
+		return PTR_ERR(jobqueue_class);
+	}
+
+	jobqueue_device = device_create(jobqueue_class, NULL, MKDEV(major, 0),
+					NULL, DEVICE_NAME);
+	if (IS_ERR(jobqueue_device)) {
+		pr_err("cannot create device\n");
+		class_destroy(jobqueue_class);
+		unregister_chrdev(major, DEVICE_NAME);
+		return PTR_ERR(jobqueue_device);
+	}
+
+	return 0;
+}
+
+void __exit v4l2_jobqueue_device_exit(void)
+{
+	if (IS_ERR(jobqueue_device))
+		return;
+
+	device_destroy(jobqueue_class, MKDEV(major, 0));
+	class_destroy(jobqueue_class);
+	unregister_chrdev(major, DEVICE_NAME);
+}
diff --git a/drivers/media/v4l2-core/v4l2-jobqueue.c b/drivers/media/v4l2-core/v4l2-jobqueue.c
new file mode 100644
index 000000000000..36d2dd48b086
--- /dev/null
+++ b/drivers/media/v4l2-core/v4l2-jobqueue.c
@@ -0,0 +1,764 @@
+/*
+    V4L2 job queue implementation
+
+    Copyright (C) 2017  The Chromium project
+
+    This program is free software; you can redistribute it and/or modify
+    it under the terms of the GNU General Public License as published by
+    the Free Software Foundation; either version 2 of the License, or
+    (at your option) any later version.
+
+    This program is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+
+ */
+
+#include <linux/compat.h>
+#include <linux/export.h>
+#include <linux/string.h>
+#include <linux/file.h>
+#include <linux/list.h>
+#include <linux/kref.h>
+#include <linux/anon_inodes.h>
+#include <linux/slab.h>
+#include <linux/mutex.h>
+#include <linux/workqueue.h>
+#include <media/v4l2-dev.h>
+#include <media/v4l2-fh.h>
+#include <media/v4l2-ctrls.h>
+#include <media/v4l2-jobqueue.h>
+#include <media/v4l2-job-state.h>
+#include <uapi/linux/v4l2-jobs.h>
+
+/* Limited by the size of atomic_t to track devices that completed a job */
+#define V4L2_JOBQUEUE_MAX_DEVICES sizeof(atomic_t)
+
+/*
+ * State of all managed devices for a given job
+ */
+struct v4l2_job {
+	struct kref refcount;
+	struct v4l2_jobqueue *jq;
+	/* node in v4l2_jobqueue's queued_jobs or completed_jobs */
+	struct list_head node;
+	/* global list of existing jobs for this queue */
+	struct list_head jobs_list;
+	/* mask of devices that completed this job */
+	atomic_t completed;
+	/* fd exported to user-space */
+	int fd;
+	enum v4l2_job_status status;
+
+	/* per-device states */
+	struct v4l2_job_state *state[0];
+};
+
+/*
+ * A job queue manages the job flow for a given set of devices, applies their
+ * state, and activates them in lockstep.
+ *
+ * A job goes through the following stages through its life:
+ *
+ * * current_job: the job has been created and is waiting to be queued. S_CTRL
+ *   will apply to it. Once queued, it is pushed into
+ * * queued_jobs: a queue of jobs to be processed in sequential order. The head
+ *   of this list becomes the
+ * * active_job: the job currently being processed by the hardware. Once
+ *   completed, the next job in queued_job becomes active, and the previous
+ *   active job goes into
+ * * completed_jobs: a list of completed jobs waiting to be dequeued by
+ *   user-space. As user-space called the DQJOB ioctl, the head becomes the
+ * * dequeued_job: the job on which G_CTRL will be performed on. A job stays
+ *   in this state until another one is dequeued, at which point it is deleted.
+ */
+struct v4l2_jobqueue {
+	/* List of all jobs created for this queue, regardless of state */
+	struct list_head jobs_list;
+	/*
+	 * Job that user-space is currently preparing, to be added to
+	 * queued_jobs upon QJOB ioctl.
+	 */
+	struct v4l2_job *current_job;
+
+	/* List of jobs that are ready to be processed */
+	struct list_head queued_jobs;
+
+	/* Job that is currently processed by the devices */
+	struct v4l2_job *active_job;
+
+	/* List of completed jobs, ready to be dequeued */
+	struct list_head completed_jobs;
+
+	/* Job that has last been dequeued and can be queried by user-space */
+	struct v4l2_job *dequeued_job;
+
+	/* Projects the *_job[s] lists/pointers above */
+	struct mutex lock;
+	struct work_struct job_complete_work;
+
+	wait_queue_head_t done_wq;
+
+	unsigned int nb_devs;
+	struct {
+		struct file *f;
+		struct v4l2_job_state_handler *state_handler;
+	} *devs;
+};
+
+static bool v4l2_jobqueue_is_locked(struct v4l2_jobqueue *jq)
+{
+	return mutex_is_locked(&jq->lock);
+}
+
+static void v4l2_jobqueue_free_job(struct v4l2_job *job)
+{
+	struct v4l2_jobqueue *jq = job->jq;
+	int i;
+
+	for (i = 0; i < jq->nb_devs; i++) {
+		struct v4l2_job_state_handler *hdl = jq->devs[i].state_handler;
+		if (job->state[i])
+			hdl->ops->job_free(hdl, job->state[i]);
+	}
+	kfree(job);
+}
+
+/*
+ * Must be called with jobqueue lock held
+ */
+static void v4l2_jobqueue_delete_job(struct kref *ref)
+{
+	struct v4l2_job *job = container_of(ref, struct v4l2_job, refcount);
+
+	list_del(&job->jobs_list);
+
+	   v4l2_jobqueue_free_job(job);
+}
+
+/*
+ * Must be called with the jobqueue lock acquired
+ */
+static void job_get(struct v4l2_job *job)
+{
+	kref_get(&job->refcount);
+}
+
+/*
+ * Must be called with the jobqueue lock acquired
+ */
+static int job_put(struct v4l2_job *job)
+{
+	return kref_put(&job->refcount, v4l2_jobqueue_delete_job);
+}
+
+/*
+ * Move a job from one state to another in the jobqueue state machine, making
+ * extensive sanity tests.
+ *
+ * jobqueue lock must be held when this function is called.
+ */
+static void jobqueue_set_job_state(struct v4l2_job *job,
+				   enum v4l2_job_status status)
+{
+	struct v4l2_jobqueue *jq = job->jq;
+	int i;
+
+	BUG_ON(!v4l2_jobqueue_is_locked(jq));
+
+	/* Sanity checks & jobqueue state update */
+	switch (status) {
+	case CURRENT:
+		BUG_ON(job->status != OUT_OF_QUEUE);
+		BUG_ON(jq->current_job != NULL);
+		jq->current_job = job;
+		break;
+	case QUEUED:
+		BUG_ON(job->status != CURRENT);
+		BUG_ON(jq->current_job != job);
+		jq->current_job = NULL;
+		list_add_tail(&job->node, &jq->queued_jobs);
+		break;
+	case ACTIVE:
+		BUG_ON(job->status != QUEUED);
+		BUG_ON(jq->active_job != NULL);
+		BUG_ON(list_first_entry_or_null(&jq->queued_jobs,
+						struct v4l2_job, node) != job);
+		list_del(&job->node);
+		jq->active_job = job;
+		break;
+	case COMPLETED:
+		BUG_ON(job->status != ACTIVE);
+		BUG_ON(jq->active_job != job);
+		jq->active_job = NULL;
+		list_add_tail(&job->node, &jq->completed_jobs);
+		break;
+	case DEQUEUED:
+		BUG_ON(job->status != COMPLETED);
+		BUG_ON(jq->dequeued_job != NULL);
+		BUG_ON(list_first_entry_or_null(&jq->completed_jobs,
+						struct v4l2_job, node) != job);
+		list_del(&job->node);
+		jq->dequeued_job = job;
+		break;
+	case OUT_OF_QUEUE:
+		BUG_ON(job->status != DEQUEUED);
+		BUG_ON(jq->dequeued_job != job);
+		jq->dequeued_job = NULL;
+		break;
+	};
+
+	job->status = status;
+
+	for (i = 0; i < jq->nb_devs; i++) {
+		struct v4l2_job_state_handler *hdl = jq->devs[i].state_handler;
+		struct v4l2_job_state *state = job->state[i];
+
+		switch (status) {
+		case CURRENT:
+			hdl->current_state = state;
+			break;
+		case ACTIVE:
+			hdl->active_state = state;
+			break;
+		case DEQUEUED:
+			hdl->dequeued_state = state;
+			break;
+		default:
+			break;
+		}
+
+		if (hdl->ops->state_changed)
+			hdl->ops->state_changed(hdl, state, status);
+	}
+}
+
+/*
+ * jobqueue lock must be held by caller
+ */
+static int v4l2_jobqueue_new_job(struct v4l2_jobqueue *jq)
+{
+	struct v4l2_job *job;
+	int i;
+
+	BUG_ON(!v4l2_jobqueue_is_locked(jq));
+
+	if (jq->current_job)
+		return -EBUSY;
+
+	job = kzalloc(sizeof(*job) + sizeof(job->state[0]) * jq->nb_devs,
+		      GFP_KERNEL);
+	if (job == NULL)
+		return -ENOMEM;
+
+	list_add(&job->jobs_list, &jq->jobs_list);
+	kref_init(&job->refcount);
+	job->jq = jq;
+	job->status = OUT_OF_QUEUE;
+	job->fd = -1;
+	atomic_set(&job->completed, 0);
+	for (i = 0; i < jq->nb_devs; i++) {
+		struct v4l2_job_state_handler *hdl = jq->devs[i].state_handler;
+		struct v4l2_job_state *state;
+
+		state = hdl->ops->job_new(hdl);
+		job->state[i] = state;
+		if (IS_ERR(state)) {
+			         v4l2_jobqueue_free_job(job);
+			return PTR_ERR(state);
+		}
+		state->job = job;
+	}
+
+	jobqueue_set_job_state(job, CURRENT);
+
+	return 0;
+}
+
+/*
+ * Prepare the next queued job for streaming.
+ *
+ * jobqueue lock must be held by the caller.
+ */
+static void v4l2_jobqueue_prepare_streaming(struct v4l2_jobqueue *jq)
+{
+	struct v4l2_job *job;
+
+	BUG_ON(!v4l2_jobqueue_is_locked(jq));
+
+	/* Already streaming */
+	if (jq->active_job)
+		return;
+
+	job = list_first_entry_or_null(&jq->queued_jobs, struct v4l2_job, node);
+	/* No job ready to be streamed */
+	if (!job)
+		return;
+	jobqueue_set_job_state(job, ACTIVE);
+}
+
+/*
+ * Asks all devices to start processing the prepared job
+ *
+ */
+static void v4l2_jobqueue_start_streaming(struct v4l2_jobqueue *jq)
+{
+	int ret;
+	int i;
+
+	/* No job ready to be streamed */
+	if (!jq->active_job)
+		return;
+
+	/*
+	 * TODO move what follows into a worker?
+	 */
+
+	/* Set the desired state on all devices */
+	for (i = 0; i < jq->nb_devs; i++) {
+		struct v4l2_job_state_handler *handler;
+
+		handler = jq->devs[i].state_handler;
+		ret = handler->ops->job_apply(handler);
+		/* TODO proper cleanup and reporting after error */
+		if (ret < 0) {
+			pr_err("error while setting job queue parameters!\n");
+			return;
+		}
+	}
+
+	/* Start streaming on all devices */
+	for (i = 0; i < jq->nb_devs; i++) {
+		struct v4l2_job_state_handler *handler;
+
+		handler = jq->devs[i].state_handler;
+
+		if (handler->process_active_job)
+			handler->process_active_job(jq->devs[i].state_handler);
+	}
+}
+
+static void v4l2_jobqueue_job_complete(struct work_struct *work)
+{
+	struct v4l2_jobqueue *jq = container_of(work, struct v4l2_jobqueue,
+						job_complete_work);
+
+	v4l2_jobqueue_lock(jq);
+
+	jobqueue_set_job_state(jq->active_job, COMPLETED);
+	wake_up(&jq->done_wq);
+
+	v4l2_jobqueue_prepare_streaming(jq);
+
+	v4l2_jobqueue_unlock(jq);
+
+	/* see if we can perform the next job */
+	v4l2_jobqueue_start_streaming(jq);
+}
+
+struct v4l2_jobqueue *v4l2_jobqueue_new(void)
+{
+	struct v4l2_jobqueue *jq;
+
+	jq = kzalloc(sizeof(*jq), GFP_KERNEL);
+	if (jq == NULL)
+		return ERR_PTR(-ENOMEM);
+
+	INIT_LIST_HEAD(&jq->jobs_list);
+	INIT_LIST_HEAD(&jq->queued_jobs);
+	INIT_LIST_HEAD(&jq->completed_jobs);
+	mutex_init(&jq->lock);
+	init_waitqueue_head(&jq->done_wq);
+	INIT_WORK(&jq->job_complete_work, v4l2_jobqueue_job_complete);
+
+	return jq;
+}
+EXPORT_SYMBOL(v4l2_jobqueue_new);
+
+int v4l2_jobqueue_del(struct v4l2_jobqueue *jq)
+{
+	struct v4l2_job *job, *_job_t;
+	int i;
+
+	v4l2_jobqueue_lock(jq);
+
+	if (jq->current_job != NULL) {
+		job_put(jq->current_job);
+		jq->current_job = NULL;
+	}
+
+	/* Clean all pending jobs */
+	list_for_each_entry_safe(job, _job_t, &jq->queued_jobs, node) {
+		pr_warn("Deleting pending queued job\n");
+		list_del(&job->node);
+		job_put(job);
+	}
+
+	/* Wait for active job to complete, if any */
+	while (jq->active_job != NULL) {
+		v4l2_jobqueue_unlock(jq);
+		wait_event(jq->done_wq, jq->active_job == NULL);
+		cancel_work_sync(&jq->job_complete_work);
+		v4l2_jobqueue_lock(jq);
+	}
+
+	/* Clean all completed jobs */
+	list_for_each_entry_safe(job, _job_t, &jq->completed_jobs, node) {
+		pr_warn("Deleting pending completed job\n");
+		list_del(&job->node);
+		job_put(job);
+	}
+
+	/* Delete currently dequeued job, if any */
+	if (jq->dequeued_job != NULL) {
+		job = jq->dequeued_job;
+		jobqueue_set_job_state(job, OUT_OF_QUEUE);
+		job_put(job);
+	}
+
+	/* No job should exist anymore */
+	WARN_ON(jq->dequeued_job);
+	WARN_ON(!list_empty(&jq->completed_jobs));
+	WARN_ON(jq->active_job);
+	WARN_ON(!list_empty(&jq->queued_jobs));
+	WARN_ON(jq->current_job);
+
+	/*
+	 * must invalidate all fds exported to user-space, otherwise users
+	 * may do funny things with them, or they will be automatically closed
+	 * when the process exits
+	 *
+	 * TODO improve this. We can still enter a race if jobqueue_release_job
+	 * if called right before we set private_data to NULL. Unfortunately
+	 * we also cannot use fput() here the call to release is asynchronous.
+	 */
+	if (!list_empty(&jq->jobs_list)) {
+		pr_warn("Exported jobs still exist, removing them\n");
+		list_for_each_entry_safe(job, _job_t, &jq->jobs_list, jobs_list)
+			v4l2_jobqueue_free_job(job);
+	}
+
+	v4l2_jobqueue_unlock(jq);
+
+	for (i = 0; i < jq->nb_devs; i++) {
+		jq->devs[i].state_handler->jobqueue = NULL;
+		fput(jq->devs[i].f);
+	}
+	kfree(jq->devs);
+	kfree(jq);
+
+	return 0;
+}
+EXPORT_SYMBOL(v4l2_jobqueue_del);
+
+int v4l2_jobqueue_init(struct v4l2_jobqueue *jq,
+		       struct v4l2_jobqueue_init *cinit)
+{
+	int ret;
+	int i;
+
+	if (cinit->nb_devs > V4L2_JOBQUEUE_MAX_DEVICES)
+		return -ENOSPC;
+
+	jq->devs = kzalloc(sizeof(jq->devs[0]) * cinit->nb_devs, GFP_KERNEL);
+	if (!jq->devs) {
+		pr_err("error: out of memory\n");
+		return -ENOMEM;
+	}
+
+	for (i = 0; i < cinit->nb_devs; i++) {
+		struct file *f;
+		struct video_device *vdev;
+		struct v4l2_fh *fh;
+		struct v4l2_job_state_handler *handler;
+		struct v4l2_ctrl_handler *ctrl_handler;
+
+		f = fget(cinit->fd[i]);
+		jq->devs[i].f = f;
+		if (!v4l2_is_v4l2_file(f)) {
+			pr_err("error: passed fd is not v4l2 device!\n");
+			ret = -EINVAL;
+			goto error;
+		}
+
+		fh = f->private_data;
+		vdev = video_devdata(f);
+
+		ctrl_handler = fh ? fh->ctrl_handler : vdev->ctrl_handler;
+		if (!ctrl_handler) {
+			pr_err("error: no control handler in device!\n");
+			ret = -EINVAL;
+			goto error;
+		}
+
+		if (fh)
+			handler = fh->state_handler;
+
+		if (!handler && vdev)
+			handler = vdev->state_handler;
+
+		if (!handler) {
+			pr_err("error: no state handler in device!\n");
+			ret = -EINVAL;
+			goto error;
+		}
+		handler->jobqueue = jq;
+		jq->devs[0].state_handler = handler;
+	}
+
+	jq->nb_devs = cinit->nb_devs;
+
+	/* Create first job */
+	v4l2_jobqueue_lock(jq);
+	ret = v4l2_jobqueue_new_job(jq);
+	v4l2_jobqueue_unlock(jq);
+	if (ret < 0)
+		goto error;
+
+	return 0;
+
+error:
+	jq->nb_devs = 0;
+	for (i = 0; i < cinit->nb_devs; i++)
+		if (jq->devs[i].f)
+			fput(jq->devs[i].f);
+	kfree(jq->devs);
+	jq->devs = NULL;
+	return ret;
+}
+EXPORT_SYMBOL(v4l2_jobqueue_init);
+
+void v4l2_jobqueue_lock(struct v4l2_jobqueue *jq)
+{
+	mutex_lock(&jq->lock);
+}
+EXPORT_SYMBOL(v4l2_jobqueue_lock);
+
+void v4l2_jobqueue_unlock(struct v4l2_jobqueue *jq)
+{
+	mutex_unlock(&jq->lock);
+}
+EXPORT_SYMBOL(v4l2_jobqueue_unlock);
+
+void v4l2_jobqueue_job_finish(struct v4l2_job_state_handler *handler)
+{
+	struct v4l2_job_state *state = handler->active_state;
+	struct v4l2_job *job;
+	struct v4l2_jobqueue *jq;
+	unsigned int finished_mask;
+	int pos;
+
+	BUG_ON(!state);
+
+	job = state->job;
+	jq = job->jq;
+	pos = state - job->state[0];
+	finished_mask = BIT(jq->nb_devs) - 1;
+
+	handler->ops->job_complete(handler);
+	handler->active_state = NULL;
+
+	/* have all devices completed? */
+	if (!atomic_add_return(BIT(pos), &job->completed) != finished_mask)
+		schedule_work(&jq->job_complete_work);
+}
+EXPORT_SYMBOL(v4l2_jobqueue_job_finish);
+
+int v4l2_jobqueue_qjob(struct v4l2_jobqueue *jq)
+{
+	bool start_streaming;
+	int ret = 0;
+
+	v4l2_jobqueue_lock(jq);
+
+	if (jq->current_job == NULL) {
+		/* This should never happen */
+		pr_err("no current job at the moment!\n");
+		ret = -EINVAL;
+		goto out;
+	}
+
+	jobqueue_set_job_state(jq->current_job, QUEUED);
+
+	start_streaming = (jq->active_job == NULL);
+
+	v4l2_jobqueue_prepare_streaming(jq);
+
+	v4l2_jobqueue_unlock(jq);
+
+	v4l2_jobqueue_lock(jq);
+
+	ret = v4l2_jobqueue_new_job(jq);
+	if (ret < 0)
+		goto out;
+
+out:
+	v4l2_jobqueue_unlock(jq);
+
+	if (ret == 0 && start_streaming)
+		v4l2_jobqueue_start_streaming(jq);
+
+	return ret;
+}
+EXPORT_SYMBOL(v4l2_jobqueue_qjob);
+
+int v4l2_jobqueue_dqjob(struct v4l2_jobqueue *jq)
+{
+	struct v4l2_job *job = NULL;
+	struct v4l2_job *old_job;
+	int ret;
+
+	v4l2_jobqueue_lock(jq);
+
+	while (true) {
+		job = list_first_entry_or_null(&jq->completed_jobs,
+					       struct v4l2_job, node);
+		if (job)
+			break;
+
+		v4l2_jobqueue_unlock(jq);
+		ret = wait_event_interruptible(jq->done_wq,
+					      !list_empty(&jq->completed_jobs));
+		if (ret)
+			return ret;
+		v4l2_jobqueue_lock(jq);
+	}
+
+	old_job = jq->dequeued_job;
+	if (old_job)
+		jobqueue_set_job_state(old_job, OUT_OF_QUEUE);
+
+	jobqueue_set_job_state(job, DEQUEUED);
+
+	v4l2_jobqueue_unlock(jq);
+
+	/* dispose of reference to previous job */
+	if (old_job)
+		job_put(old_job);
+
+	return 0;
+}
+EXPORT_SYMBOL(v4l2_jobqueue_dqjob);
+
+static int v4l2_jobqueue_release_job(struct inode *inode, struct file *filp)
+{
+	struct v4l2_job *job = filp->private_data;
+	struct v4l2_jobqueue *jq = job->jq;
+
+	/* This can happen if a job queue is closed before all its exported
+	 * jobs. In that case the job becomes inactive until its fd finally
+	 * gets closed */
+	if (job == NULL)
+		return 0;
+
+	v4l2_jobqueue_lock(jq);
+
+	job->fd = -1;
+	job_put(job);
+
+	v4l2_jobqueue_unlock(jq);
+
+	return 0;
+}
+
+static const struct file_operations v4l2_jobqueue_job_ops = {
+	.owner = THIS_MODULE,
+	.release = v4l2_jobqueue_release_job,
+};
+
+int v4l2_jobqueue_export_job(struct v4l2_jobqueue *jq,
+			     struct v4l2_jobqueue_job *export_job)
+{
+	struct v4l2_job *job = jq->current_job;
+	int fd;
+	int i;
+
+	v4l2_jobqueue_lock(jq);
+
+	if (job->fd >= 0) {
+		v4l2_jobqueue_unlock(jq);
+		pr_warn("job already exported!\n");
+		return -EBUSY;
+	}
+
+	/* Sanity check that all devices support export */
+	for (i = 0; i < jq->nb_devs; i++) {
+		if (!jq->devs[i].state_handler->ops->job_export) {
+			v4l2_jobqueue_unlock(jq);
+			pr_warn("some devices do not support job export\n");
+			return -ENOTSUPP;
+		}
+	}
+
+	for (i = 0; i < jq->nb_devs; i++) {
+		struct v4l2_job_state_handler *hdl = jq->devs[i].state_handler;
+		int ret;
+
+		ret = hdl->ops->job_export(hdl);
+		if (ret < 0) {
+			v4l2_jobqueue_unlock(jq);
+			pr_warn("job export failed\n");
+			return ret;
+		}
+	}
+
+	fd = anon_inode_getfd("v4l2", &v4l2_jobqueue_job_ops, job, O_CLOEXEC);
+	if (fd < 0) {
+		v4l2_jobqueue_unlock(jq);
+		return fd;
+	}
+
+	export_job->fd = job->fd = fd;
+	job_get(job);
+
+	v4l2_jobqueue_unlock(jq);
+
+	return 0;
+}
+EXPORT_SYMBOL(v4l2_jobqueue_export_job);
+
+int v4l2_jobqueue_import_job(struct v4l2_jobqueue *jq,
+			     struct v4l2_jobqueue_job *import_job)
+{
+	struct v4l2_job *current_job = NULL;
+	struct v4l2_job *job;
+	struct file *f = fget(import_job->fd);
+
+	if (!f)
+		return -EINVAL;
+
+	/* Is this fd legit? */
+	if (f->f_op != &v4l2_jobqueue_job_ops)
+		return -EINVAL;
+
+	job = f->private_data;
+	fput(f);
+
+	/* Does the job actually belong to us? */
+	if (job->jq != jq) {
+		pr_err("job belongs to a different queue!\n");
+		return -EINVAL;
+	}
+
+	v4l2_jobqueue_lock(jq);
+
+	/* Cannot import a job that is not out of queue */
+	if (job->status != OUT_OF_QUEUE) {
+		pr_err("job is already in the queue!\n");
+		v4l2_jobqueue_unlock(jq);
+		return -EBUSY;
+	}
+
+	job_get(job);
+	current_job = jq->current_job;
+	jq->current_job = NULL;
+	jobqueue_set_job_state(job, CURRENT);
+
+	v4l2_jobqueue_unlock(jq);
+	if (current_job)
+		job_put(current_job);
+
+	return 0;
+}
+EXPORT_SYMBOL(v4l2_jobqueue_import_job);
diff --git a/include/media/v4l2-dev.h b/include/media/v4l2-dev.h
index b73d646980da..a1558d9bf309 100644
--- a/include/media/v4l2-dev.h
+++ b/include/media/v4l2-dev.h
@@ -38,6 +38,7 @@ struct v4l2_ioctl_callbacks;
 struct video_device;
 struct v4l2_device;
 struct v4l2_ctrl_handler;
+struct v4l2_job_state_handler;
 
 /* Flag to mark the video_device struct as registered.
    Drivers can clear this flag if they want to block all future
@@ -184,6 +185,8 @@ struct v4l2_file_operations {
  * @dev_parent: pointer to &struct device parent
  * @ctrl_handler: Control handler associated with this device node.
  *	 May be NULL.
+ * @state_handler: State handler associated with this device node.
+ *	 May be NULL.
  * @queue: &struct vb2_queue associated with this device node. May be NULL.
  * @prio: pointer to &struct v4l2_prio_state with device's Priority state.
  *	 If NULL, then v4l2_dev->prio will be used.
@@ -229,6 +232,7 @@ struct video_device
 	struct device *dev_parent;
 
 	struct v4l2_ctrl_handler *ctrl_handler;
+	struct v4l2_job_state_handler *state_handler;
 
 	struct vb2_queue *queue;
 
diff --git a/include/media/v4l2-fh.h b/include/media/v4l2-fh.h
index 0b0757090c04..ea86d713a59a 100644
--- a/include/media/v4l2-fh.h
+++ b/include/media/v4l2-fh.h
@@ -28,6 +28,7 @@
 
 struct video_device;
 struct v4l2_ctrl_handler;
+struct v4l2_job_state_handler;
 
 /**
  * struct v4l2_fh - Describes a V4L2 file handler
@@ -35,6 +36,8 @@ struct v4l2_ctrl_handler;
  * @list: list of file handlers
  * @vdev: pointer to &struct video_device
  * @ctrl_handler: pointer to &struct v4l2_ctrl_handler
+ * @state_handler: pointer to &struct v4l2_job_state_handler, may be NULL if
+ *                 jobs API not supported by this device.
  * @prio: priority of the file handler, as defined by &enum v4l2_priority
  *
  * @wait: event' s wait queue
@@ -48,6 +51,7 @@ struct v4l2_fh {
 	struct list_head	list;
 	struct video_device	*vdev;
 	struct v4l2_ctrl_handler *ctrl_handler;
+	struct v4l2_job_state_handler *state_handler;
 	enum v4l2_priority	prio;
 
 	/* Events */
diff --git a/include/media/v4l2-job-state.h b/include/media/v4l2-job-state.h
new file mode 100644
index 000000000000..42048e8d11a8
--- /dev/null
+++ b/include/media/v4l2-job-state.h
@@ -0,0 +1,75 @@
+/*
+    V4L2 job states interface
+
+    Copyright (C) 2017  The Chromium project
+
+    This program is free software; you can redistribute it and/or modify
+    it under the terms of the GNU General Public License as published by
+    the Free Software Foundation; either version 2 of the License, or
+    (at your option) any later version.
+
+    This program is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+
+ */
+
+#ifndef _V4L2_JOB_STATE_H
+#define _V4L2_JOB_STATE_H
+
+struct v4l2_jobqueue;
+struct v4l2_job_state_handler;
+struct v4l2_ext_control;
+struct v4l2_job;
+struct v4l2_ctrl;
+
+enum v4l2_job_status;
+
+struct v4l2_job_state {
+	struct v4l2_job *job;
+};
+
+struct v4l2_job_state_handler_ops {
+	/* Allocate a new job state for this device */
+	struct v4l2_job_state *(*job_new)(struct v4l2_job_state_handler *hdl);
+	/* Free a previously allocated job state */
+	void (*job_free)(struct v4l2_job_state_handler *hdl,
+			 struct v4l2_job_state *state);
+	/* Apply current job state */
+	int (*job_apply)(struct v4l2_job_state_handler *hdl);
+	/* Signal that the device has completed its part of the job */
+	void (*job_complete)(struct v4l2_job_state_handler *hdl);
+	/* Prepare the current job for being exported */
+	int (*job_export)(struct v4l2_job_state_handler *hdl);
+
+	/* Set control the the current state */
+	int (*s_ctrl)(struct v4l2_job_state_handler *hdl,
+		      struct v4l2_ext_control *c);
+	/* Get control from the dequeued state */
+	int (*g_ctrl)(struct v4l2_job_state_handler *hdl, u32 ctrl_id,
+		      struct v4l2_ext_control *c, u32 which);
+
+	/* Called whenever the HW value of a non-volatile control is changed */
+	void (*ctrl_changed)(struct v4l2_job_state_handler *hdl,
+			     struct v4l2_ctrl *ctrl);
+	/* Called whenever a job has moved in the job queue */
+	void (*state_changed)(struct v4l2_job_state_handler *hdl,
+			      struct v4l2_job_state *state,
+		       enum v4l2_job_status status);
+};
+
+/*
+ * Manages the state of one particular device. Contains pointers to the
+ * current, active and dequeued state that are updated by the jobs framework
+ */
+struct v4l2_job_state_handler {
+	struct v4l2_jobqueue *jobqueue;
+	const struct v4l2_job_state_handler_ops *ops;
+	void (*process_active_job)(struct v4l2_job_state_handler *hdl);
+	struct v4l2_job_state *current_state;
+	struct v4l2_job_state *active_state;
+	struct v4l2_job_state *dequeued_state;
+};
+
+#endif
diff --git a/include/media/v4l2-jobqueue-dev.h b/include/media/v4l2-jobqueue-dev.h
new file mode 100644
index 000000000000..9c3dcff72563
--- /dev/null
+++ b/include/media/v4l2-jobqueue-dev.h
@@ -0,0 +1,24 @@
+/*
+    V4L2 jobqueue device
+
+    Copyright (C) 2017  The Chromium project
+
+    This program is free software; you can redistribute it and/or modify
+    it under the terms of the GNU General Public License as published by
+    the Free Software Foundation; either version 2 of the License, or
+    (at your option) any later version.
+
+    This program is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+
+ */
+
+#ifndef _V4L2_JOBQUEUE_DEV_H_
+#define _V4L2_JOBQUEUE_DEV_H
+
+int v4l2_jobqueue_device_init(void);
+void v4l2_jobqueue_device_exit(void);
+
+#endif
diff --git a/include/media/v4l2-jobqueue.h b/include/media/v4l2-jobqueue.h
new file mode 100644
index 000000000000..a294042b7c13
--- /dev/null
+++ b/include/media/v4l2-jobqueue.h
@@ -0,0 +1,54 @@
+/*
+    V4L2 jobqueue support header.
+
+    Copyright (C) 2017  The Chromium project
+
+    This program is free software; you can redistribute it and/or modify
+    it under the terms of the GNU General Public License as published by
+    the Free Software Foundation; either version 2 of the License, or
+    (at your option) any later version.
+
+    This program is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+
+ */
+
+#ifndef _V4L2_JOBQUEUE_H
+#define _V4L2_JOBQUEUE_H
+
+struct v4l2_jobqueue;
+struct v4l2_job_state_handler;
+
+#include <uapi/linux/v4l2-jobs.h>
+
+enum v4l2_job_status {
+	CURRENT,
+	QUEUED,
+	ACTIVE,
+	COMPLETED,
+	DEQUEUED,
+	OUT_OF_QUEUE,
+};
+
+/* Jobqueue device interface */
+struct v4l2_jobqueue *v4l2_jobqueue_new(void);
+int v4l2_jobqueue_del(struct v4l2_jobqueue *jq);
+
+/* Ioctls support */
+int v4l2_jobqueue_init(struct v4l2_jobqueue *jq,
+		       struct v4l2_jobqueue_init *init);
+int v4l2_jobqueue_qjob(struct v4l2_jobqueue *jq);
+int v4l2_jobqueue_dqjob(struct v4l2_jobqueue *jq);
+int v4l2_jobqueue_export_job(struct v4l2_jobqueue *jq,
+			     struct v4l2_jobqueue_job *job);
+int v4l2_jobqueue_import_job(struct v4l2_jobqueue *jq,
+			     struct v4l2_jobqueue_job *job);
+
+/* Driver/state handler interface */
+void v4l2_jobqueue_job_finish(struct v4l2_job_state_handler *hdl);
+void v4l2_jobqueue_lock(struct v4l2_jobqueue *jq);
+void v4l2_jobqueue_unlock(struct v4l2_jobqueue *jq);
+
+#endif
diff --git a/include/uapi/linux/v4l2-jobs.h b/include/uapi/linux/v4l2-jobs.h
new file mode 100644
index 000000000000..2cba4d20e62f
--- /dev/null
+++ b/include/uapi/linux/v4l2-jobs.h
@@ -0,0 +1,40 @@
+/*
+ * V4L2 jobs API
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef __LINUX_V4L2_JOBS_H
+#define __LINUX_V4L2_JOBS_H
+
+#ifndef __KERNEL__
+#include <stdint.h>
+#endif
+#include <linux/ioctl.h>
+#include <linux/types.h>
+
+struct v4l2_jobqueue_init {
+	__u32 nb_devs;
+	__s32 *fd;
+};
+
+struct v4l2_jobqueue_job {
+	__s32 fd;
+};
+
+#define VIDIOC_JOBQUEUE_IOCTL_START	0x80
+
+#define VIDIOC_JOBQUEUE_INIT		_IOW('|', VIDIOC_JOBQUEUE_IOCTL_START + 0x00, struct v4l2_jobqueue_init)
+#define VIDIOC_JOBQUEUE_QJOB		_IO('|', VIDIOC_JOBQUEUE_IOCTL_START + 0x01)
+#define VIDIOC_JOBQUEUE_DQJOB		_IO('|', VIDIOC_JOBQUEUE_IOCTL_START + 0x02)
+#define VIDIOC_JOBQUEUE_EXPORT_JOB	_IOR('|', VIDIOC_JOBQUEUE_IOCTL_START + 0x03, struct v4l2_jobqueue_job)
+#define VIDIOC_JOBQUEUE_IMPORT_JOB	_IOW('|', VIDIOC_JOBQUEUE_IOCTL_START + 0x03, struct v4l2_jobqueue_job)
+
+#endif
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 45cf7359822c..7f43e97cf461 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -1591,6 +1591,8 @@ struct v4l2_ext_controls {
 #define V4L2_CTRL_MAX_DIMS	  (4)
 #define V4L2_CTRL_WHICH_CUR_VAL   0
 #define V4L2_CTRL_WHICH_DEF_VAL   0x0f000000
+#define V4L2_CTRL_WHICH_CURJOB_VAL   0x0e000000
+#define V4L2_CTRL_WHICH_DEQJOB_VAL   0x0d000000
 
 enum v4l2_ctrl_type {
 	V4L2_CTRL_TYPE_INTEGER	     = 1,
-- 
2.14.2.822.g60be5d43e6-goog

[RFC PATCH 3/9] [media] videobuf2: add support for jobs API

[Alexandre Courbot]

Add generic support for jobs in videobuf2. When the jobs API is active,
the passing of buffers to the driver is delayed until their job is
submitted. Drivers need to call the vb2_queue_active_job_buffers()
function in order to receive the buffers corresponding to the active
job.

Signed-off-by: Alexandre Courbot <acourbot@chromium.org>
---
 drivers/media/v4l2-core/videobuf2-core.c | 33 ++++++++++++++++++++++++++++----
 include/media/videobuf2-core.h           | 16 ++++++++++++++++
 2 files changed, 45 insertions(+), 4 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
index 14f83cecfa92..2ac880ebe192 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -26,6 +26,7 @@
 
 #include <media/videobuf2-core.h>
 #include <media/v4l2-mc.h>
+#include <media/v4l2-job-state.h>
 
 #include <trace/events/vb2.h>
 
@@ -1304,6 +1305,22 @@ int vb2_core_prepare_buf(struct vb2_queue *q, unsigned int index, void *pb)
 }
 EXPORT_SYMBOL_GPL(vb2_core_prepare_buf);
 
+void vb2_queue_active_job_buffers(struct vb2_queue *q)
+{
+	const struct v4l2_job_state_handler *hdl = q->state_handler;
+	struct vb2_buffer *vb;
+
+	if (!q->start_streaming_called)
+		return;
+
+	list_for_each_entry(vb, &q->queued_list, queued_entry) {
+		if (!hdl || hdl->active_state == vb->job)
+			__enqueue_in_driver(vb);
+	}
+}
+EXPORT_SYMBOL_GPL(vb2_queue_active_job_buffers);
+
+
 /**
  * vb2_start_streaming() - Attempt to start streaming.
  * @q:		videobuf2 queue
@@ -1320,15 +1337,15 @@ static int vb2_start_streaming(struct vb2_queue *q)
 	struct vb2_buffer *vb;
 	int ret;
 
+	q->start_streaming_called = 1;
+
 	/*
 	 * If any buffers were queued before streamon,
 	 * we can now pass them to driver for processing.
 	 */
-	list_for_each_entry(vb, &q->queued_list, queued_entry)
-		__enqueue_in_driver(vb);
+	vb2_queue_active_job_buffers(q);
 
 	/* Tell the driver to start streaming */
-	q->start_streaming_called = 1;
 	ret = call_qop(q, start_streaming, q,
 		       atomic_read(&q->owned_by_drv_count));
 	if (!ret)
@@ -1398,6 +1415,7 @@ int vb2_core_qbuf(struct vb2_queue *q, unsigned int index, void *pb)
 	q->queued_count++;
 	q->waiting_for_buffers = false;
 	vb->state = VB2_BUF_STATE_QUEUED;
+	vb->job = q->state_handler ? q->state_handler->current_state : NULL;
 
 	if (pb)
 		call_void_bufop(q, copy_timestamp, vb, pb);
@@ -1407,8 +1425,11 @@ int vb2_core_qbuf(struct vb2_queue *q, unsigned int index, void *pb)
 	/*
 	 * If already streaming, give the buffer to driver for processing.
 	 * If not, the buffer will be given to driver on next streamon.
+	 *
+	 * If using the jobs API, we will give the buffer to the driver when
+	 * its job becomes active.
 	 */
-	if (q->start_streaming_called)
+	if (q->start_streaming_called && !vb->job)
 		__enqueue_in_driver(vb);
 
 	/* Fill buffer information for the userspace */
@@ -1422,6 +1443,8 @@ int vb2_core_qbuf(struct vb2_queue *q, unsigned int index, void *pb)
 	 * then we can finally call start_streaming().
 	 */
 	if (q->streaming && !q->start_streaming_called &&
+	/* TODO potential issue: what if we have less than min_buffers_needed
+	 * in the next job? */
 	    q->queued_count >= q->min_buffers_needed) {
 		ret = vb2_start_streaming(q);
 		if (ret)
@@ -1728,6 +1751,8 @@ int vb2_core_streamon(struct vb2_queue *q, unsigned int type)
 	 * Tell driver to start streaming provided sufficient buffers
 	 * are available.
 	 */
+	/* TODO potential issue: what if we have less than min_buffers_needed
+	 * in the next job? */
 	if (q->queued_count >= q->min_buffers_needed) {
 		ret = v4l_vb2q_enable_media_source(q);
 		if (ret)
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index cb97c224be73..9e172168e011 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -246,6 +246,7 @@ struct vb2_buffer {
 	unsigned int		num_planes;
 	struct vb2_plane	planes[VB2_MAX_PLANES];
 	u64			timestamp;
+	struct v4l2_job_state	*job;
 
 	/* private: internal use only
 	 *
@@ -506,6 +507,7 @@ struct vb2_queue {
 	const struct vb2_ops		*ops;
 	const struct vb2_mem_ops	*mem_ops;
 	const struct vb2_buf_ops	*buf_ops;
+	const struct v4l2_job_state_handler *state_handler;
 
 	void				*drv_priv;
 	unsigned int			buf_struct_size;
@@ -625,6 +627,20 @@ void vb2_discard_done(struct vb2_queue *q);
  */
 int vb2_wait_for_all_buffers(struct vb2_queue *q);
 
+/**
+ * vb2_queue_active_job_buffers() - Pass all buffers for the active job to the
+ *                                  driver
+ *
+ * @q:		videobuf2 queue
+ *
+ * When using the jobs API, buffers are not passed to the driver until their
+ * job becomes active. Drivers using the jobs API are thus expected to call
+ * this function whenever a new job becomes active, so all buffers assigned
+ * to this job are passed to them.
+ */
+void vb2_queue_active_job_buffers(struct vb2_queue *q);
+
+
 /**
  * vb2_core_querybuf() - query video buffer information
  * @q:		videobuf queue
-- 
2.14.2.822.g60be5d43e6-goog

[RFC PATCH 4/9] [media] v4l2-ctrls: add support for jobs API

[Alexandre Courbot]

Add generic support for jobs in the control framework by handling the
new V4L2_CTRL_WHICH_*JOB_VAL which values. This calls the state handler
callbacks in such a case and takes care of control management for
drivers with jobs API support.

Note: this is a very simple and naive way to manage controls in jobs.
Doing this properly will probably require more changes to the control
framework, but the current form is enough to demonstrate the general
ideas.

Signed-off-by: Alexandre Courbot <acourbot@chromium.org>
---
 drivers/media/v4l2-core/v4l2-ctrls.c | 50 ++++++++++++++++++++++++++++++++----
 include/media/v4l2-ctrls.h           |  6 +++++
 2 files changed, 51 insertions(+), 5 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
index dd1db678718c..fcc644a83cf0 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -27,6 +27,7 @@
 #include <media/v4l2-ctrls.h>
 #include <media/v4l2-event.h>
 #include <media/v4l2-dev.h>
+#include <media/v4l2-job-state.h>
 
 #define has_op(master, op) \
 	(master->ops && master->ops->op)
@@ -1603,8 +1604,14 @@ static void new_to_cur(struct v4l2_fh *fh, struct v4l2_ctrl *ctrl, u32 ch_flags)
 
 	/* has_changed is set by cluster_changed */
 	changed = ctrl->has_changed;
-	if (changed)
+	if (changed) {
+		struct v4l2_job_state_handler *state = ctrl->handler->state_handler;
+
+		if (state && state->ops->ctrl_changed)
+			state->ops->ctrl_changed(state, ctrl);
+
 		ptr_to_ptr(ctrl, ctrl->p_new, ctrl->p_cur);
+	}
 
 	if (ch_flags & V4L2_EVENT_CTRL_CH_FLAGS) {
 		/* Note: CH_FLAGS is only set for auto clusters. */
@@ -2738,6 +2745,8 @@ static int prepare_ext_ctrls(struct v4l2_ctrl_handler *hdl,
 
 		if (cs->which &&
 		    cs->which != V4L2_CTRL_WHICH_DEF_VAL &&
+		    cs->which != V4L2_CTRL_WHICH_CURJOB_VAL &&
+		    cs->which != V4L2_CTRL_WHICH_DEQJOB_VAL &&
 		    V4L2_CTRL_ID2WHICH(id) != cs->which)
 			return -EINVAL;
 
@@ -2817,7 +2826,9 @@ static int prepare_ext_ctrls(struct v4l2_ctrl_handler *hdl,
    whether there are any controls at all. */
 static int class_check(struct v4l2_ctrl_handler *hdl, u32 which)
 {
-	if (which == 0 || which == V4L2_CTRL_WHICH_DEF_VAL)
+	if (which == 0 || which == V4L2_CTRL_WHICH_DEF_VAL ||
+	    which == V4L2_CTRL_WHICH_CURJOB_VAL ||
+	    which == V4L2_CTRL_WHICH_DEQJOB_VAL)
 		return list_empty(&hdl->ctrl_refs) ? -EINVAL : 0;
 	return find_ref_lock(hdl, which | 1) ? 0 : -EINVAL;
 }
@@ -2829,11 +2840,14 @@ int v4l2_g_ext_ctrls(struct v4l2_ctrl_handler *hdl, struct v4l2_ext_controls *cs
 {
 	struct v4l2_ctrl_helper helper[4];
 	struct v4l2_ctrl_helper *helpers = helper;
+	struct v4l2_job_state_handler *state;
 	int ret;
 	int i, j;
-	bool def_value;
+	bool def_value, job_value;
 
 	def_value = (cs->which == V4L2_CTRL_WHICH_DEF_VAL);
+	job_value = (cs->which == V4L2_CTRL_WHICH_DEQJOB_VAL ||
+		     cs->which == V4L2_CTRL_WHICH_CURJOB_VAL);
 
 	cs->error_idx = cs->count;
 	cs->which = V4L2_CTRL_ID2WHICH(cs->which);
@@ -2841,6 +2855,10 @@ int v4l2_g_ext_ctrls(struct v4l2_ctrl_handler *hdl, struct v4l2_ext_controls *cs
 	if (hdl == NULL)
 		return -EINVAL;
 
+	state = hdl->state_handler;
+	if (!state && job_value)
+		return -EINVAL;
+
 	if (cs->count == 0)
 		return class_check(hdl, cs->which);
 
@@ -2874,18 +2892,20 @@ int v4l2_g_ext_ctrls(struct v4l2_ctrl_handler *hdl, struct v4l2_ext_controls *cs
 		v4l2_ctrl_lock(master);
 
 		/* g_volatile_ctrl will update the new control values */
-		if (!def_value &&
+		if (!def_value && !job_value &&
 		    ((master->flags & V4L2_CTRL_FLAG_VOLATILE) ||
 		    (master->has_volatiles && !is_cur_manual(master)))) {
 			for (j = 0; j < master->ncontrols; j++)
 				cur_to_new(master->cluster[j]);
 			ret = call_op(master, g_volatile_ctrl);
 			ctrl_to_user = new_to_user;
+		} else if (job_value) {
+			ret = state->ops->g_ctrl(state, master->id, cs->controls + i, cs->which);
 		}
 		/* If OK, then copy the current (for non-volatile controls)
 		   or the new (for volatile controls) control values to the
 		   caller */
-		if (!ret) {
+		if (!ret && !job_value) {
 			u32 idx = i;
 
 			do {
@@ -3008,6 +3028,7 @@ static int try_or_set_cluster(struct v4l2_fh *fh, struct v4l2_ctrl *master,
 	/* Don't set if there is no change */
 	if (ret || !set || !cluster_changed(master))
 		return ret;
+
 	ret = call_op(master, s_ctrl);
 	if (ret)
 		return ret;
@@ -3082,6 +3103,8 @@ static int try_set_ext_ctrls(struct v4l2_fh *fh, struct v4l2_ctrl_handler *hdl,
 {
 	struct v4l2_ctrl_helper helper[4];
 	struct v4l2_ctrl_helper *helpers = helper;
+	struct v4l2_job_state_handler *state;
+	bool job_value;
 	unsigned i, j;
 	int ret;
 
@@ -3096,6 +3119,14 @@ static int try_set_ext_ctrls(struct v4l2_fh *fh, struct v4l2_ctrl_handler *hdl,
 	if (hdl == NULL)
 		return -EINVAL;
 
+	if (cs->which == V4L2_CTRL_WHICH_DEQJOB_VAL)
+		return -EINVAL;
+
+	job_value = (cs->which == V4L2_CTRL_WHICH_CURJOB_VAL);
+	state = hdl->state_handler;
+	if (!state && job_value)
+		return -EINVAL;
+
 	if (cs->count == 0)
 		return class_check(hdl, cs->which);
 
@@ -3121,6 +3152,15 @@ static int try_set_ext_ctrls(struct v4l2_fh *fh, struct v4l2_ctrl_handler *hdl,
 		master = helpers[i].mref->ctrl;
 		v4l2_ctrl_lock(master);
 
+		if (job_value) {
+			ret = state->ops->s_ctrl(state, cs->controls + idx);
+			v4l2_ctrl_unlock(master);
+			if (ret)
+				return ret;
+			continue;
+		}
+
+
 		/* Reset the 'is_new' flags of the cluster */
 		for (j = 0; j < master->ncontrols; j++)
 			if (master->cluster[j])
diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h
index 2d2aed56922f..da3eb69d1af1 100644
--- a/include/media/v4l2-ctrls.h
+++ b/include/media/v4l2-ctrls.h
@@ -21,8 +21,11 @@
 #include <linux/mutex.h>
 #include <linux/videodev2.h>
 
+#include <media/v4l2-jobqueue.h>
+
 /* forward references */
 struct file;
+struct v4l2_job_state_handler;
 struct v4l2_ctrl_handler;
 struct v4l2_ctrl_helper;
 struct v4l2_ctrl;
@@ -72,6 +75,7 @@ struct v4l2_ctrl_ops {
 	int (*s_ctrl)(struct v4l2_ctrl *ctrl);
 };
 
+
 /**
  * struct v4l2_ctrl_type_ops - The control type operations that the driver
  *			       has to provide.
@@ -257,6 +261,7 @@ struct v4l2_ctrl_ref {
  *	controls: both the controls owned by the handler and those inherited
  *	from other handlers.
  *
+ * @state_handler: State handler to use when jobs API is in use.
  * @_lock:	Default for "lock".
  * @lock:	Lock to control access to this handler and its controls.
  *		May be replaced by the user right after init.
@@ -275,6 +280,7 @@ struct v4l2_ctrl_ref {
  * @error:	The error code of the first failed control addition.
  */
 struct v4l2_ctrl_handler {
+	struct v4l2_job_state_handler *state_handler;
 	struct mutex _lock;
 	struct mutex *lock;
 	struct list_head ctrls;
-- 
2.14.2.822.g60be5d43e6-goog

[RFC PATCH 5/9] [media] v4l2-job: add generic jobs ops

[Alexandre Courbot]

Add a generic state handler that records controls to be set for a given
job and applies them once the job is scheduled, while also allowing said
controls to be read back once the job is dequeued.

This implementation can be used as-is for most drivers, with only drivers
with specific needs needing to provide an alternative implementation.

Note: this is still very early, but should do the job to demonstrate the
jobs API feasibility. Amongst the current limitations:

- We use v4l2_ext_control to store controls, which expects user-space
pointers. As a consequence only integer controls are supported at the
moment.
- No support for try_ctrl yet.

Signed-off-by: Alexandre Courbot <acourbot@chromium.org>
---
 drivers/media/v4l2-core/Makefile           |   3 +-
 drivers/media/v4l2-core/v4l2-job-generic.c | 394 +++++++++++++++++++++++++++++
 include/media/v4l2-job-generic.h           |  47 ++++
 3 files changed, 443 insertions(+), 1 deletion(-)
 create mode 100644 drivers/media/v4l2-core/v4l2-job-generic.c
 create mode 100644 include/media/v4l2-job-generic.h

diff --git a/drivers/media/v4l2-core/Makefile b/drivers/media/v4l2-core/Makefile
index a717bb8f1a25..ee09e1f29129 100644
--- a/drivers/media/v4l2-core/Makefile
+++ b/drivers/media/v4l2-core/Makefile
@@ -6,7 +6,8 @@ tuner-objs	:=	tuner-core.o
 
 videodev-objs	:=	v4l2-dev.o v4l2-ioctl.o v4l2-device.o v4l2-fh.o \
 			v4l2-event.o v4l2-ctrls.o v4l2-subdev.o v4l2-clk.o \
-			v4l2-async.o v4l2-jobqueue.o v4l2-jobqueue-dev.o
+			v4l2-async.o v4l2-jobqueue.o v4l2-jobqueue-dev.o \
+			v4l2-job-generic.o
 
 ifeq ($(CONFIG_COMPAT),y)
   videodev-objs += v4l2-compat-ioctl32.o
diff --git a/drivers/media/v4l2-core/v4l2-job-generic.c b/drivers/media/v4l2-core/v4l2-job-generic.c
new file mode 100644
index 000000000000..ded6464e723a
--- /dev/null
+++ b/drivers/media/v4l2-core/v4l2-job-generic.c
@@ -0,0 +1,394 @@
+/*
+    V4L2 generic jobs implementation
+
+    Copyright (C) 2017  The Chromium project
+
+    This program is free software; you can redistribute it and/or modify
+    it under the terms of the GNU General Public License as published by
+    the Free Software Foundation; either version 2 of the License, or
+    (at your option) any later version.
+
+    This program is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+
+ */
+
+#include <linux/slab.h>
+#include <linux/mutex.h>
+#include <linux/list.h>
+
+#include <media/v4l2-job-generic.h>
+#include <media/v4l2-jobqueue.h>
+#include <media/v4l2-ctrls.h>
+#include <media/v4l2-fh.h>
+#include <media/v4l2-dev.h>
+#include <linux/videodev2.h>
+
+#define to_generic_state_handler(hdl) \
+	container_of(hdl, struct v4l2_generic_state_handler, base)
+
+struct v4l2_generic_job_state {
+	struct v4l2_job_state base;
+	struct list_head node;
+
+	int nr_ctrls;
+	struct v4l2_ext_control ctrls[0];
+};
+#define to_generic_job_state(job) \
+	container_of(job, struct v4l2_generic_job_state, base)
+
+/* TODO this is O(n). Find a better way to store/lookup controls */
+static struct v4l2_ext_control *
+v4l2_generic_job_find_control(struct v4l2_generic_job_state *job, u32 id)
+{
+	int i;
+
+	for (i = 0; i < job->nr_ctrls; i++)
+		if (job->ctrls[i].id == id)
+			return &job->ctrls[i];
+
+	return NULL;
+}
+
+static struct v4l2_job_state *
+v4l2_job_generic_job_new(struct v4l2_job_state_handler *_hdl)
+{
+	struct v4l2_generic_state_handler *hdl = to_generic_state_handler(_hdl);
+	struct v4l2_generic_job_state *ret;
+
+	ret = kzalloc(sizeof(*ret) + sizeof(ret->ctrls[0]) * hdl->nr_ctrls,
+		      GFP_KERNEL);
+	if (ret == NULL)
+		return ERR_PTR(-ENOMEM);
+
+	return &ret->base;
+}
+
+static void v4l2_job_generic_job_free(struct v4l2_job_state_handler *hdl,
+					struct v4l2_job_state *_job)
+{
+	struct v4l2_generic_job_state *job = to_generic_job_state(_job);
+
+	kfree(job);
+}
+
+static int v4l2_job_generic_job_apply(struct v4l2_job_state_handler *_hdl)
+{
+	struct v4l2_generic_state_handler *hdl = to_generic_state_handler(_hdl);
+	struct v4l2_generic_job_state *job;
+	struct v4l2_ext_controls ctrls;
+	int ret;
+
+	job = to_generic_job_state(_hdl->active_state);
+
+	if (job->nr_ctrls == 0)
+		return 0;
+
+	ctrls.which = V4L2_CTRL_WHICH_CUR_VAL;
+	ctrls.count = job->nr_ctrls;
+	ctrls.controls = job->ctrls;
+
+	ret = v4l2_s_ext_ctrls(hdl->fh, hdl->ctrl_hdl, &ctrls);
+	if (ret) {
+		pr_err("Cannot set job controls: %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int v4l2_job_generic_s_ctrl(struct v4l2_job_state_handler *_hdl,
+				   struct v4l2_ext_control *c)
+{
+	struct v4l2_generic_state_handler *hdl = to_generic_state_handler(_hdl);
+	struct v4l2_generic_job_state *job;
+	struct v4l2_ext_control *ctrl = NULL;
+
+	job = to_generic_job_state(_hdl->current_state);
+
+	ctrl = v4l2_generic_job_find_control(job, c->id);
+	if (!ctrl)
+		ctrl = &job->ctrls[job->nr_ctrls++];
+
+	/* This should never happen as we allocate exactly enough space for
+	 * all the controls of our device */
+	BUG_ON(job->nr_ctrls > hdl->nr_ctrls);
+
+	/* TODO manage pointers, do a try, etc */
+	ctrl->id = c->id;
+	ctrl->value = c->value;
+
+	return 0;
+}
+
+/*
+ * Try to read the control value from the passed job, then its parents, then
+ * the hardware if none has defined a state.
+ */
+static int
+v4l2_job_generic_g_ctrl_locked(struct v4l2_generic_state_handler *hdl,
+			       struct v4l2_generic_job_state *job, u32 ctrl_id,
+			       struct v4l2_ext_control *c, bool upward)
+{
+	struct v4l2_ext_control *ctrl;
+	struct list_head *node;
+
+	if (job == NULL || &job->base == hdl->base.active_state) {
+		struct v4l2_control ctrl = {
+			.id = ctrl_id,
+		};
+		int ret;
+
+		/* TODO terrible! */
+		mutex_unlock(hdl->ctrl_hdl->lock);
+		ret = v4l2_g_ctrl(hdl->ctrl_hdl, &ctrl);
+		mutex_lock(hdl->ctrl_hdl->lock);
+		if (ret < 0)
+			return ret;
+
+		c->value = ctrl.value;
+
+		return 0;
+	}
+
+	ctrl = v4l2_generic_job_find_control(job, ctrl_id);
+	if (ctrl) {
+		/* TODO handle pointers, etc. */
+		c->value = ctrl->value;
+		return 0;
+	}
+
+	if (upward)
+		node = job->node.prev;
+	else
+		node = job->node.next;
+
+	/* That was our last job, request hardware state */
+	if (node == &hdl->jobs)
+		job = NULL;
+	else
+		job = container_of(node, struct v4l2_generic_job_state, node);
+	return v4l2_job_generic_g_ctrl_locked(hdl, job, ctrl_id, c, upward);
+}
+
+static int
+v4l2_job_generic_g_ctrl(struct v4l2_job_state_handler *_hdl, u32 ctrl_id,
+			struct v4l2_ext_control *c, u32 which)
+{
+	struct v4l2_jobqueue *jq = _hdl->jobqueue;
+	struct v4l2_generic_state_handler *hdl = to_generic_state_handler(_hdl);
+	struct v4l2_job_state *_job;
+	struct v4l2_generic_job_state *job;
+	bool upward;
+	int ret;
+
+	if (which != V4L2_CTRL_WHICH_CURJOB_VAL &&
+	    which != V4L2_CTRL_WHICH_DEQJOB_VAL)
+		return -EINVAL;
+
+	v4l2_jobqueue_lock(jq);
+
+	if (which == V4L2_CTRL_WHICH_DEQJOB_VAL) {
+		_job = hdl->base.dequeued_state;
+		upward = true;
+	} else {
+		_job = hdl->base.current_state;
+		upward = false;
+	}
+	if (!_job) {
+		pr_err("No state to query controls from!\n");
+		return -EINVAL;
+	}
+
+	job = to_generic_job_state(_job);
+
+	ret = v4l2_job_generic_g_ctrl_locked(hdl, job, ctrl_id, c, upward);
+
+	v4l2_jobqueue_unlock(jq);
+
+	return ret;
+}
+
+static void v4l2_job_generic_job_complete(struct v4l2_job_state_handler *_hdl)
+{
+	struct v4l2_generic_state_handler *hdl = to_generic_state_handler(_hdl);
+	struct v4l2_generic_job_state *job;
+	struct v4l2_ext_controls cs;
+	struct v4l2_ext_control ctrls[hdl->nr_vol_ctrls];
+	int ret;
+	int i;
+
+	/* We need to update all volatile controls, if any */
+	if (hdl->nr_vol_ctrls == 0)
+		return;
+
+	job = to_generic_job_state(_hdl->active_state);
+
+	memset(job->ctrls, 0, sizeof(job->ctrls[0]) * hdl->nr_ctrls);
+
+	for (i = 0; i < hdl->nr_vol_ctrls; i++)
+		job->ctrls[i].id = hdl->ctrls_ids[i];
+
+	cs.which = V4L2_CTRL_WHICH_CUR_VAL;
+	cs.count = hdl->nr_vol_ctrls;
+	cs.controls = ctrls;
+
+	ret = v4l2_g_ext_ctrls(hdl->ctrl_hdl, &cs);
+	if (ret < 0) {
+		pr_err("Cannot read output controls: %d\n", ret);
+		return;
+	}
+
+	for (i = 0; i < cs.count; i++) {
+		ret = v4l2_job_generic_s_ctrl(_hdl, &ctrls[i]);
+		if (ret < 0)
+			pr_err("Cannot update volatile control value!\n");
+	}
+}
+
+static void v4l2_job_generic_state_changed(struct v4l2_job_state_handler *_hdl,
+					   struct v4l2_job_state *_job,
+					   enum v4l2_job_status status)
+{
+	struct v4l2_generic_state_handler *hdl = to_generic_state_handler(_hdl);
+	struct v4l2_generic_job_state *job = to_generic_job_state(_job);
+
+	switch (status) {
+	case CURRENT:
+		list_add(&job->node, &hdl->jobs);
+		break;
+	case COMPLETED:
+		hdl->last_completed = job;
+		break;
+	case OUT_OF_QUEUE:
+		list_del(&job->node);
+		if (hdl->last_completed == job)
+			hdl->last_completed = NULL;
+		break;
+	default:
+		break;
+	}
+}
+
+static void v4l2_job_generic_ctrl_changed(struct v4l2_job_state_handler *_hdl,
+					  struct v4l2_ctrl *ctrl)
+{
+	struct v4l2_jobqueue *jq = _hdl->jobqueue;
+	struct v4l2_generic_state_handler *hdl = to_generic_state_handler(_hdl);
+	struct v4l2_generic_job_state *job;
+	struct v4l2_ext_control *ext_ctrl;
+
+	/* prevent the job queue from changing state */
+	v4l2_jobqueue_lock(jq);
+
+	job = hdl->last_completed;
+
+	if (!job)
+		goto out;
+
+	/* store the current value of the control into the job so it reflects
+	 * the state at the time it completed */
+	ext_ctrl = v4l2_generic_job_find_control(job, ctrl->id);
+	/* we already have a completion value stored, nothing to do */
+	if (ext_ctrl)
+		goto out;
+
+	ext_ctrl = &job->ctrls[job->nr_ctrls++];
+	ext_ctrl->id = ctrl->id;
+	ext_ctrl->value = ctrl->cur.val;
+
+out:
+	v4l2_jobqueue_unlock(jq);
+}
+
+static int v4l2_job_generic_job_export(struct v4l2_job_state_handler *_hdl)
+{
+	struct v4l2_generic_state_handler *hdl = to_generic_state_handler(_hdl);
+	struct v4l2_generic_job_state *job;
+	struct v4l2_ctrl_handler *ctrl_hdl = hdl->ctrl_hdl;
+	struct v4l2_ctrl *ctrl;
+
+	job = to_generic_job_state(_hdl->current_state);
+
+	/*
+	 * Read and store all controls, so the full state can be reapplied
+	 * when we reuse this state
+	 */
+	mutex_lock(ctrl_hdl->lock);
+
+	list_for_each_entry(ctrl, &ctrl_hdl->ctrls, node) {
+		/* dummy */
+		struct v4l2_ext_control c;
+
+		if (ctrl->flags & V4L2_CTRL_FLAG_WRITE_ONLY)
+			continue;
+
+		c.id = ctrl->id;
+		/* get the current value either from the HW or a parent state */
+		v4l2_job_generic_g_ctrl_locked(hdl, job, ctrl->id, &c, false);
+		/* ... and store it */
+		v4l2_job_generic_s_ctrl(&hdl->base, &c);
+	}
+
+	mutex_unlock(ctrl_hdl->lock);
+
+	return 0;
+}
+
+static const struct v4l2_job_state_handler_ops v4l2_generic_job_ops = {
+	.job_new = v4l2_job_generic_job_new,
+	.job_free = v4l2_job_generic_job_free,
+	.job_apply = v4l2_job_generic_job_apply,
+	.job_complete = v4l2_job_generic_job_complete,
+	.job_export = v4l2_job_generic_job_export,
+
+	.s_ctrl = v4l2_job_generic_s_ctrl,
+	.g_ctrl = v4l2_job_generic_g_ctrl,
+
+	.state_changed = v4l2_job_generic_state_changed,
+	.ctrl_changed = v4l2_job_generic_ctrl_changed,
+};
+
+int v4l2_job_generic_init(struct v4l2_generic_state_handler *hdl,
+		    void (*process_active_job)(struct v4l2_job_state_handler *),
+		    struct v4l2_fh *fh, struct video_device *vdev)
+{
+	struct v4l2_ctrl *ctrl;
+	struct v4l2_ctrl_handler *ctrl_hdl;
+
+	hdl->base.process_active_job = process_active_job;
+
+	ctrl_hdl = fh ? fh->ctrl_handler : vdev->ctrl_handler;
+	ctrl_hdl->state_handler = &hdl->base;
+
+	if (fh)
+		fh->state_handler = &hdl->base;
+	else
+		vdev->state_handler = &hdl->base;
+
+	hdl->ctrl_hdl = ctrl_hdl;
+	hdl->fh = fh;
+	INIT_LIST_HEAD(&hdl->jobs);
+	hdl->base.ops = &v4l2_generic_job_ops;
+
+	mutex_lock(ctrl_hdl->lock);
+
+	/* Count how many controls we have to manage */
+	list_for_each_entry(ctrl, &ctrl_hdl->ctrls, node) {
+		/* Reserve permanent space for volatile controls */
+		if (ctrl->flags & V4L2_CTRL_FLAG_VOLATILE) {
+			if (hdl->nr_vol_ctrls >= V4L2_GENERIC_JOB_MAX_CTRLS)
+				return -ENOSPC;
+			hdl->ctrls_ids[hdl->nr_vol_ctrls++] = ctrl->id;
+		}
+
+		hdl->nr_ctrls++;
+	}
+
+	mutex_unlock(ctrl_hdl->lock);
+
+	return 0;
+}
+EXPORT_SYMBOL(v4l2_job_generic_init);
diff --git a/include/media/v4l2-job-generic.h b/include/media/v4l2-job-generic.h
new file mode 100644
index 000000000000..5f6ee55d68e4
--- /dev/null
+++ b/include/media/v4l2-job-generic.h
@@ -0,0 +1,47 @@
+/*
+    V4L2 generic jobs support header.
+
+    Copyright (C) 2017  The Chromium project
+
+    This program is free software; you can redistribute it and/or modify
+    it under the terms of the GNU General Public License as published by
+    the Free Software Foundation; either version 2 of the License, or
+    (at your option) any later version.
+
+    This program is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+
+ */
+
+#ifndef _V4L2_JOB_GENERIC_H
+#define _V4L2_JOB_GENERIC_H
+
+#include <media/v4l2-job-state.h>
+#include <linux/videodev2.h>
+#include <linux/list.h>
+
+struct video_device;
+struct v4l2_fh;
+struct v4l2_ctrl;
+struct v4l2_ctrl_handler;
+struct v4l2_generic_job_state;
+
+#define V4L2_GENERIC_JOB_MAX_CTRLS 32
+struct v4l2_generic_state_handler {
+	struct v4l2_job_state_handler base;
+	struct list_head jobs;
+	struct v4l2_ctrl_handler *ctrl_hdl;
+	struct v4l2_fh *fh;
+	struct v4l2_generic_job_state *last_completed;
+	unsigned int nr_ctrls;
+	unsigned int nr_vol_ctrls;
+	u32 ctrls_ids[V4L2_GENERIC_JOB_MAX_CTRLS];
+};
+
+int v4l2_job_generic_init(struct v4l2_generic_state_handler *handler,
+		    void (*process_active_job)(struct v4l2_job_state_handler *),
+		    struct v4l2_fh *fh, struct video_device *vdev);
+
+#endif
-- 
2.14.2.822.g60be5d43e6-goog

[RFC PATCH 6/9] [media] m2m: add generic support for jobs API

[Alexandre Courbot]

Add a v4l2_mem_ctx_job_init() function that drivers using m2m can call
at init time in order to set the state handler.

Also make sure to call v4l2_jobqueue_job_finish() when the jobs API is
used and all buffers for the current job have been processed.

Signed-off-by: Alexandre Courbot <acourbot@chromium.org>
---
 drivers/media/v4l2-core/v4l2-mem2mem.c | 19 +++++++++++++++++++
 include/media/v4l2-mem2mem.h           | 11 +++++++++++
 2 files changed, 30 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c b/drivers/media/v4l2-core/v4l2-mem2mem.c
index f62e68aa04c4..dc4d8b16c0e2 100644
--- a/drivers/media/v4l2-core/v4l2-mem2mem.c
+++ b/drivers/media/v4l2-core/v4l2-mem2mem.c
@@ -22,6 +22,8 @@
 #include <media/v4l2-dev.h>
 #include <media/v4l2-fh.h>
 #include <media/v4l2-event.h>
+#include <media/v4l2-job-state.h>
+#include <media/v4l2-jobqueue.h>
 
 MODULE_DESCRIPTION("Mem to mem device framework for videobuf");
 MODULE_AUTHOR("Pawel Osciak, <pawel@osciak.com>");
@@ -333,6 +335,13 @@ void v4l2_m2m_job_finish(struct v4l2_m2m_dev *m2m_dev,
 
 	spin_unlock_irqrestore(&m2m_dev->job_spinlock, flags);
 
+	/* Jobs mode: if we have no more buffers for the current job,
+	 * consider it completed */
+	if (m2m_ctx->state && m2m_ctx->state->active_state &&
+	    v4l2_m2m_next_src_buf(m2m_ctx) == NULL &&
+	    v4l2_m2m_next_dst_buf(m2m_ctx) == NULL)
+		v4l2_jobqueue_job_finish(m2m_ctx->state);
+
 	/* This instance might have more buffers ready, but since we do not
 	 * allow more than one job on the job_queue per instance, each has
 	 * to be scheduled separately after the previous one finishes. */
@@ -665,6 +674,16 @@ struct v4l2_m2m_ctx *v4l2_m2m_ctx_init(struct v4l2_m2m_dev *m2m_dev,
 }
 EXPORT_SYMBOL_GPL(v4l2_m2m_ctx_init);
 
+void v4l2_mem_ctx_job_init(struct v4l2_m2m_ctx *ctx,
+			   struct v4l2_job_state_handler *hdl)
+{
+	ctx->state = hdl;
+
+	ctx->out_q_ctx.q.state_handler = hdl;
+	ctx->cap_q_ctx.q.state_handler = hdl;
+}
+EXPORT_SYMBOL_GPL(v4l2_mem_ctx_job_init);
+
 void v4l2_m2m_ctx_release(struct v4l2_m2m_ctx *m2m_ctx)
 {
 	/* wait until the current context is dequeued from job_queue */
diff --git a/include/media/v4l2-mem2mem.h b/include/media/v4l2-mem2mem.h
index e157d5c9b224..4ba18a32c9b0 100644
--- a/include/media/v4l2-mem2mem.h
+++ b/include/media/v4l2-mem2mem.h
@@ -83,6 +83,7 @@ struct v4l2_m2m_queue_ctx {
  *
  * @q_lock: struct &mutex lock
  * @m2m_dev: opaque pointer to the internal data to handle M2M context
+ * @state: Pointer to state handler for channel support
  * @cap_q_ctx: Capture (output to memory) queue context
  * @out_q_ctx: Output (input from memory) queue context
  * @queue: List of memory to memory contexts
@@ -100,6 +101,7 @@ struct v4l2_m2m_ctx {
 
 	/* internal use only */
 	struct v4l2_m2m_dev		*m2m_dev;
+	struct v4l2_job_state_handler	*state;
 
 	struct v4l2_m2m_queue_ctx	cap_q_ctx;
 
@@ -351,6 +353,15 @@ struct v4l2_m2m_ctx *v4l2_m2m_ctx_init(struct v4l2_m2m_dev *m2m_dev,
 		void *drv_priv,
 		int (*queue_init)(void *priv, struct vb2_queue *src_vq, struct vb2_queue *dst_vq));
 
+/**
+ * v4l2_mem_ctx_channel_init() - enable a context to be used in a channel
+ *
+ * @ctx: context to potentially use within a channel
+ * @hal: state handler that will be used with this context
+ */
+void v4l2_mem_ctx_job_init(struct v4l2_m2m_ctx *ctx,
+			       struct v4l2_job_state_handler *hdl);
+
 static inline void v4l2_m2m_set_src_buffered(struct v4l2_m2m_ctx *m2m_ctx,
 					     bool buffered)
 {
-- 
2.14.2.822.g60be5d43e6-goog

[RFC PATCH 7/9] [media] vim2m: add jobs API support

[Alexandre Courbot]

Add support for jobs in vim2m, using the generic state handler.

Signed-off-by: Alexandre Courbot <acourbot@chromium.org>
---
 drivers/media/platform/vim2m.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/media/platform/vim2m.c b/drivers/media/platform/vim2m.c
index 970b9b6dab25..aff21d53d898 100644
--- a/drivers/media/platform/vim2m.c
+++ b/drivers/media/platform/vim2m.c
@@ -31,6 +31,8 @@
 #include <media/v4l2-event.h>
 #include <media/videobuf2-vmalloc.h>
 
+#include <media/v4l2-job-generic.h>
+
 MODULE_DESCRIPTION("Virtual device for mem2mem framework testing");
 MODULE_AUTHOR("Pawel Osciak, <pawel@osciak.com>");
 MODULE_LICENSE("GPL");
@@ -155,6 +157,7 @@ struct vim2m_ctx {
 	struct vim2m_dev	*dev;
 
 	struct v4l2_ctrl_handler hdl;
+	struct v4l2_generic_state_handler state;
 
 	/* Processed buffers in this transaction */
 	u8			num_processed;
@@ -877,6 +880,15 @@ static const struct v4l2_ctrl_config vim2m_ctrl_trans_num_bufs = {
 	.step = 1,
 };
 
+static void vim2m_process_active_job(struct v4l2_job_state_handler *hdl)
+{
+	struct vim2m_ctx *ctx = container_of(hdl, struct vim2m_ctx, state.base);
+
+	vb2_queue_active_job_buffers(&ctx->fh.m2m_ctx->cap_q_ctx.q);
+	vb2_queue_active_job_buffers(&ctx->fh.m2m_ctx->out_q_ctx.q);
+	v4l2_m2m_try_schedule(ctx->fh.m2m_ctx);
+}
+
 /*
  * File operations
  */
@@ -886,6 +898,7 @@ static int vim2m_open(struct file *file)
 	struct vim2m_ctx *ctx = NULL;
 	struct v4l2_ctrl_handler *hdl;
 	int rc = 0;
+	int ret;
 
 	if (mutex_lock_interruptible(&dev->dev_mutex))
 		return -ERESTARTSYS;
@@ -913,6 +926,15 @@ static int vim2m_open(struct file *file)
 	ctx->fh.ctrl_handler = hdl;
 	v4l2_ctrl_handler_setup(hdl);
 
+	ret = v4l2_job_generic_init(&ctx->state, vim2m_process_active_job,
+				    &ctx->fh, NULL);
+	if (ret) {
+		v4l2_ctrl_handler_free(hdl);
+		v4l2_fh_exit(&ctx->fh);
+		kfree(ctx);
+		goto open_unlock;
+	}
+
 	ctx->q_data[V4L2_M2M_SRC].fmt = &formats[0];
 	ctx->q_data[V4L2_M2M_SRC].width = 640;
 	ctx->q_data[V4L2_M2M_SRC].height = 480;
@@ -934,6 +956,8 @@ static int vim2m_open(struct file *file)
 		goto open_unlock;
 	}
 
+	v4l2_mem_ctx_job_init(ctx->fh.m2m_ctx, &ctx->state.base);
+
 	v4l2_fh_add(&ctx->fh);
 	atomic_inc(&dev->num_inst);
 
-- 
2.14.2.822.g60be5d43e6-goog

[RFC PATCH 8/9] [media] vivid: add jobs API support for capture device

[Alexandre Courbot]

Add support for jobs in the vivid capture device, using the generic
state handler.

Signed-off-by: Alexandre Courbot <acourbot@chromium.org>
---
 drivers/media/platform/vivid/vivid-core.c        | 16 ++++++++++++++++
 drivers/media/platform/vivid/vivid-core.h        |  2 ++
 drivers/media/platform/vivid/vivid-kthread-cap.c |  5 +++++
 3 files changed, 23 insertions(+)

diff --git a/drivers/media/platform/vivid/vivid-core.c b/drivers/media/platform/vivid/vivid-core.c
index ef344b9a48af..161c2199d2aa 100644
--- a/drivers/media/platform/vivid/vivid-core.c
+++ b/drivers/media/platform/vivid/vivid-core.c
@@ -35,6 +35,7 @@
 #include <media/v4l2-ioctl.h>
 #include <media/v4l2-fh.h>
 #include <media/v4l2-event.h>
+#include <media/v4l2-job-generic.h>
 
 #include "vivid-core.h"
 #include "vivid-vid-common.h"
@@ -639,6 +640,14 @@ static void vivid_dev_release(struct v4l2_device *v4l2_dev)
 	kfree(dev);
 }
 
+void vivid_process_active_job(struct v4l2_job_state_handler *hdl)
+{
+	struct vivid_dev *dev = container_of(hdl, struct vivid_dev,
+					     state_hdl_vid_cap.base);
+
+	vb2_queue_active_job_buffers(&dev->vb_vid_cap_q);
+}
+
 static int vivid_create_instance(struct platform_device *pdev, int inst)
 {
 	static const struct v4l2_dv_timings def_dv_timings =
@@ -1071,6 +1080,7 @@ static int vivid_create_instance(struct platform_device *pdev, int inst)
 		q->min_buffers_needed = 2;
 		q->lock = &dev->mutex;
 		q->dev = dev->v4l2_dev.dev;
+		q->state_handler = &dev->state_hdl_vid_cap.base;
 
 		ret = vb2_queue_init(q);
 		if (ret)
@@ -1185,6 +1195,12 @@ static int vivid_create_instance(struct platform_device *pdev, int inst)
 		vfd->lock = &dev->mutex;
 		video_set_drvdata(vfd, dev);
 
+		ret = v4l2_job_generic_init(&dev->state_hdl_vid_cap,
+					   vivid_process_active_job, NULL, vfd);
+		if (ret)
+			return ret;
+
+
 #ifdef CONFIG_VIDEO_VIVID_CEC
 		if (in_type_counter[HDMI]) {
 			struct cec_adapter *adap;
diff --git a/drivers/media/platform/vivid/vivid-core.h b/drivers/media/platform/vivid/vivid-core.h
index 5cdf95bdc4d1..c4014816beb7 100644
--- a/drivers/media/platform/vivid/vivid-core.h
+++ b/drivers/media/platform/vivid/vivid-core.h
@@ -28,6 +28,7 @@
 #include <media/v4l2-dev.h>
 #include <media/v4l2-ctrls.h>
 #include <media/v4l2-tpg.h>
+#include <media/v4l2-job-generic.h>
 #include "vivid-rds-gen.h"
 #include "vivid-vbi-gen.h"
 
@@ -156,6 +157,7 @@ struct vivid_dev {
 	struct v4l2_ctrl_handler	ctrl_hdl_loop_cap;
 	struct video_device		vid_cap_dev;
 	struct v4l2_ctrl_handler	ctrl_hdl_vid_cap;
+	struct v4l2_generic_state_handler state_hdl_vid_cap;
 	struct video_device		vid_out_dev;
 	struct v4l2_ctrl_handler	ctrl_hdl_vid_out;
 	struct video_device		vbi_cap_dev;
diff --git a/drivers/media/platform/vivid/vivid-kthread-cap.c b/drivers/media/platform/vivid/vivid-kthread-cap.c
index 6ca71aabb576..b31ebfa55eca 100644
--- a/drivers/media/platform/vivid/vivid-kthread-cap.c
+++ b/drivers/media/platform/vivid/vivid-kthread-cap.c
@@ -683,6 +683,7 @@ static void vivid_thread_vid_cap_tick(struct vivid_dev *dev, int dropped_bufs)
 {
 	struct vivid_buffer *vid_cap_buf = NULL;
 	struct vivid_buffer *vbi_cap_buf = NULL;
+	bool vid_job_complete;
 
 	dprintk(dev, 1, "Video Capture Thread Tick\n");
 
@@ -700,6 +701,7 @@ static void vivid_thread_vid_cap_tick(struct vivid_dev *dev, int dropped_bufs)
 	if (!list_empty(&dev->vid_cap_active)) {
 		vid_cap_buf = list_entry(dev->vid_cap_active.next, struct vivid_buffer, list);
 		list_del(&vid_cap_buf->list);
+		vid_job_complete = list_empty(&dev->vid_cap_active);
 	}
 	if (!list_empty(&dev->vbi_cap_active)) {
 		if (dev->field_cap != V4L2_FIELD_ALTERNATE ||
@@ -729,6 +731,9 @@ static void vivid_thread_vid_cap_tick(struct vivid_dev *dev, int dropped_bufs)
 				VB2_BUF_STATE_ERROR : VB2_BUF_STATE_DONE);
 		dprintk(dev, 2, "vid_cap buffer %d done\n",
 				vid_cap_buf->vb.vb2_buf.index);
+
+		if (vid_job_complete)
+			v4l2_jobqueue_job_finish(&dev->state_hdl_vid_cap.base);
 	}
 
 	if (vbi_cap_buf) {
-- 
2.14.2.822.g60be5d43e6-goog

[RFC PATCH 9/9] [media] document jobs API

[Alexandre Courbot]

Still a work-in-progress, but hopefully conveys the general idea.

Signed-off-by: Alexandre Courbot <acourbot@chromium.org>
---
 Documentation/media/intro.rst                      |   2 +
 Documentation/media/media_uapi.rst                 |   1 +
 Documentation/media/uapi/jobs/jobs-api.rst         |  23 +++
 Documentation/media/uapi/jobs/jobs-example.rst     |  69 ++++++++
 Documentation/media/uapi/jobs/jobs-intro.rst       |  61 +++++++
 Documentation/media/uapi/jobs/jobs-queue.rst       |  73 ++++++++
 Documentation/media/uapi/jobs/jobs-queue.svg       | 192 +++++++++++++++++++++
 .../media/uapi/v4l/vidioc-g-ext-ctrls.rst          |   6 +
 8 files changed, 427 insertions(+)
 create mode 100644 Documentation/media/uapi/jobs/jobs-api.rst
 create mode 100644 Documentation/media/uapi/jobs/jobs-example.rst
 create mode 100644 Documentation/media/uapi/jobs/jobs-intro.rst
 create mode 100644 Documentation/media/uapi/jobs/jobs-queue.rst
 create mode 100644 Documentation/media/uapi/jobs/jobs-queue.svg

diff --git a/Documentation/media/intro.rst b/Documentation/media/intro.rst
index 9ce2e23a0236..e39a9dd3444a 100644
--- a/Documentation/media/intro.rst
+++ b/Documentation/media/intro.rst
@@ -38,6 +38,8 @@ divided into five parts.
 
 5. The :ref:`fifth part <cec>` covers the CEC (Consumer Electronics Control) API.
 
+6. The :ref:`sixth part <jobsapi>` covers the jobs API.
+
 It should also be noted that a media device may also have audio components, like
 mixers, PCM capture, PCM playback, etc, which are controlled via ALSA API.  For
 additional information and for the latest development code, see:
diff --git a/Documentation/media/media_uapi.rst b/Documentation/media/media_uapi.rst
index fd8ebe002cd2..254e3d085abc 100644
--- a/Documentation/media/media_uapi.rst
+++ b/Documentation/media/media_uapi.rst
@@ -27,5 +27,6 @@ License".
     uapi/rc/remote_controllers
     uapi/mediactl/media-controller
     uapi/cec/cec-api
+    uapi/jobs/jobs-api
     uapi/gen-errors
     uapi/fdl-appendix
diff --git a/Documentation/media/uapi/jobs/jobs-api.rst b/Documentation/media/uapi/jobs/jobs-api.rst
new file mode 100644
index 000000000000..3a7aa8568e93
--- /dev/null
+++ b/Documentation/media/uapi/jobs/jobs-api.rst
@@ -0,0 +1,23 @@
+.. -*- coding: utf-8; mode: rst -*-
+
+.. include:: <isonum.txt>
+
+.. _jobsapi:
+
+##################
+Part VI - Jobs API
+##################
+
+This part describes the V4L2 Jobs API.
+
+.. class:: toc-title
+
+        Table of Contents
+
+.. toctree::
+    :maxdepth: 5
+    :numbered:
+
+    jobs-intro
+    jobs-queue
+    jobs-example
diff --git a/Documentation/media/uapi/jobs/jobs-example.rst b/Documentation/media/uapi/jobs/jobs-example.rst
new file mode 100644
index 000000000000..0b725dfb58bd
--- /dev/null
+++ b/Documentation/media/uapi/jobs/jobs-example.rst
@@ -0,0 +1,69 @@
+.. -*- coding: utf-8; mode: rst -*-
+
+.. _jobs-example:
+
+===========================================
+Example: Using the Jobs API for HDR capture
+===========================================
+
+HDR capturing involves taking two shots of the same image with different
+exposure settings. When using the V4L2 API, one must wait for the first shot to
+be completed before updating the exposure and taking the second one, introducing
+synchronization with user-space and potential delays for the second image.
+
+The Jobs API allows us to simply submit two jobs with different exposure
+parameters, and to dequeue their buffers to get the result.
+
+The following example shows how to do this on a capture device with the exposure
+control. It assumes that the capture device has already been opened and its
+format set. To keep the code simple this code does not handle errors.
+
+.. code-block:: c
+
+    struct v4l2_ext_control ctrl[1];
+    struct v4l2_ext_controls ctrls;
+    struct v4l2_jobqueue_init jq_init;
+    struct v4l2_buffer buf[2];
+    int jq_fd;
+
+    /* FD for the jobs queue */
+    jq_fd = open("/dev/v4l2_jobqueue");
+
+    /* Initialize the jobs queue */
+    jq_init.nb_devs = 1;
+    jq_init.fd[0] = capture_fd;
+    ioctl(jq_fd, VIDIOC_JOBQUEUE_INIT, &jq_init);
+
+    /* Prepare and submit the first capture job, low exposure */
+    ctrl[0].id = V4L2_CID_EXPOSURE;
+    ctrl[0].value = 32;
+    ctrls.which = V4L2_CTRL_WHICH_CURJOB_VAL;
+    ctrls.count = 1;
+    ctrsl.controls = ctrl;
+    ioctl(capture_fd, VICIOC_S_EXT_CTRLS, &ctrls);
+    ioctl(capture_fd, VIDIOC_QBUF, &buf[0]);
+    ioctl(jq_fd, VIDIOC_JOBQUEUE_QJOB, NULL);
+
+    /* Prepare and submit the second capture job, high exposure */
+    ctrl[0].id = V4L2_CID_EXPOSURE;
+    ctrl[0].value = 192;
+    ctrls.which = V4L2_CTRL_WHICH_CURJOB_VAL;
+    ctrls.count = 1;
+    ctrsl.controls = ctrl;
+    ioctl(capture_fd, VICIOC_S_EXT_CTRLS, &ctrls);
+    ioctl(capture_fd, VIDIOC_QBUF, &buf[1]);
+    ioctl(jq_fd, VIDIOC_JOBQUEUE_QJOB, NULL);
+
+    /* Dequeue buffers with the captured data */
+    ioctl(capture_fd, VIDIOC_DQBUF, &buf[0]);
+    ioctl(capture_fd, VIDIOC_DQBUF, &buf[1]);
+
+    /* Dequeue jobs and confirm exposure parameters */
+    ioctl(jq_fd, VIDIOC_JOBQUEUE_DQJOB, NULL);
+    ctrls.which = V4L2_CTRL_WHICH_DEQJOB_VAL
+    ioctl(fd, VIDIOC_G_EXT_CTRLS, &ctrls);
+    printf("Exposure for first capture: %d\n", ctrl[0].value);
+
+    ioctl(jq_fd, VIDIOC_JOBQUEUE_DQJOB, NULL);
+    ioctl(fd, VIDIOC_G_EXT_CTRLS, &ctrls);
+    printf("Exposure for second capture: %d\n", ctrl[0].value);
diff --git a/Documentation/media/uapi/jobs/jobs-intro.rst b/Documentation/media/uapi/jobs/jobs-intro.rst
new file mode 100644
index 000000000000..34d7efe5bcfb
--- /dev/null
+++ b/Documentation/media/uapi/jobs/jobs-intro.rst
@@ -0,0 +1,61 @@
+.. -*- coding: utf-8; mode: rst -*-
+
+.. _jobs-intro:
+
+============
+Introduction
+============
+
+The Jobs API allows to submit units of work ("jobs") to be processed
+cooperatively by a set of opened devices. The state of jobs can be set and
+queried independently from the actual hardware state, which means that
+user-space does not need to explicitly wait for a job to finish before
+submitting a new one with different parameters.
+
+The Jobs API is suitable for several workflows:
+
+- A set of devices need to cooperate on a given work, e.g. MIPI capture devices.
+In this case the Jobs API ensures that all devices are properly set up for
+processing the work.
+
+- Asynchronous submission of works with different parameters. Especially useful
+for stateless codecs, but can also facilitate capture use-cases like HDR where
+two frames with different exposures need to be captured as quickly as possible.
+
+- Quick switch between several states without having to explicitly reprogram
+them. A job can be kept in memory and be reapplied at a future time.
+
+A job's will usually be used as follows:
+
+1. The state of the job is defined using the standard V4L2 API: controls,
+formats, and other parameters are set, and buffers are queued on all devices
+that take part in the job.
+
+2. The job is queued. All set parameters are applied, and the queued buffers are
+processed.
+
+3. The job is dequeued. Output controls at the time of job completion can be
+read back. If another job is queued, it is processed.
+
+Jobs are submitted to a jobs queue which processes them in sequential order.
+Similarly to buffers, jobs must be dequeued and the state of the devices at the
+time of job completion can then be read back.
+
+The Jobs API extends the existing V4L2 API and slightly changes the behavior of
+some existing commands when in use. This part will describe the new APIs and how
+to use them.
+
+WARNING: This is a work-in-progress. Comments are welcome. Many pieces are
+missing, while some are subject to change. The list of missing pieces and open
+questions include notably:
+
+- No format or crop setting
+- No support for non-integer controls (this requires some rework of the control
+framework)
+- No support for media controller devices yet
+- No error reporting when something unexpected happens while a job is being
+processed
+- It is probably not desirable to have a /dev/v4l2_jobsqueue device node for
+managing jobs, but this makes testing easier at the moment.
+- Jobs should have identifiers to make them easier to identify by user-space
+- Dequeued buffers should carry the id of the job they were associated to
diff --git a/Documentation/media/uapi/jobs/jobs-queue.rst b/Documentation/media/uapi/jobs/jobs-queue.rst
new file mode 100644
index 000000000000..ecfc8dd46302
--- /dev/null
+++ b/Documentation/media/uapi/jobs/jobs-queue.rst
@@ -0,0 +1,73 @@
+.. -*- coding: utf-8; mode: rst -*-
+
+.. _jobs-queue:
+
+==============
+The Jobs Queue
+==============
+
+A jobs queue is instanciated from a set of already opened V4L2 devices, and
+controls their behavior with atomic (from the user perspective) units of work
+called jobs. User-space prepares a job by setting its desired state through
+controls and formats, queuing the buffers to be processed for the job, and
+finally queuing the job itself once it is ready. The hardware state will not be
+affected until the job is effectively processed.
+
+Figure :ref:`jobs-queue` shows the life cycle of jobs within the queue:
+
+.. kernel-figure:: jobs-queue.svg
+    :alt:   Life of a job inside the jobs queue
+    :align: center
+
+User-space creates a jobs queue by opening the jobs queue device and invoking
+the :c:func:`VIDIOC_JOBQUEUE_INIT` ioctl with the list of opened file
+descriptors of the devices being part of the new queue. After this, all the
+passed devices are ready to perform according to the jobs API semantics, and the
+opened jobs queue can accept queue and dequeue ioctls.
+
+Upon job queue creation, the first job is instanciated and ready to be prepared
+and queued (we refer to this as the "current job"). User-space prepares the job
+using variants of the regular V4L2 ioctls: for instance, in order to set
+controls on a job, one will invoke :c:func:`VIDIOC_S_EXT_CTRLS` with
+V4L2_CTRL_WHICH_CURJOB_VAL.
+
+The current job can be submitted using the :c:func:`VIDIOC_JOBQUEUE_QJOB` ioctl
+when it is ready. A new job is immediately created and becomes the new active
+job.
+
+Queued jobs are processed sequentially. As a job is processed, its parameters
+for every device that is part of the queue are applied, and all its buffers are
+processed. Once completed, the job is ready to be dequeued.
+
+Buffers can be dequeued independently of jobs. This allows a buffer that is
+produced in the middle of the pipeline to be dequeued and used before the entire
+job completes. However jobs **must** be dequeued at some point to avoid
+accumulation of completed jobs. This is done using the
+:c:func:`VIDIOC_JOBQUEUE_DQJOB` ioctl. User-space can then read the state of the
+dequeued job at the time of its completion. For instance, in order to read the
+values of controls at the time the dequeued job completed, one will invoke
+:c:func:`VIDIOC_QUERY_EXT_CTRL` with V4L2_CTRL_WHICH_DEQJOB_VAL.
+
+Note that regular, unmodified ioctls will continue working as usual: for
+instance, invoking :c:func:`VIDIOC_S_EXT_CTRLS` with V4L2_CTRL_WHICH_CUR_VAL
+will immediately change the value of submitted controls. The only difference
+when the jobs API is used is that buffers queued on devices that are part of a
+jobs queue are not processed until the current job of that queue is submitted.
+
+By default, jobs are purely sequential: a new job is created as soon as the
+current job is queued, and becomes the new current job. This new current job
+initially has an empty state, meaning that its expected state is the state of
+the previously queued job after it completes. Once a dequeued job is replaced by
+the next one, it is deleted.
+
+For some use-cases however, it may make sense to reapply the exact state of a
+previous job. The jobs API allows to do so using the
+:c:func:`VIDIOC_JOBQUEUE_EXPORT_JOB` and :c:func:`VIDIOC_JOBQUEUE_IMPORT_JOB`
+ioctls.
+
+:c:func:`VIDIOC_JOBQUEUE_EXPORT_JOB` returns a file descriptor that represents
+the current job, which will be preserved in memory until that file descriptor is
+closed. By passing that file descriptor to the
+:c:func:`VIDIOC_JOBQUEUE_IMPORT_JOB`, the current job is replaced by the job
+represented by the file descriptor. User-space can then tune the state of the
+job, queue new buffers, and submit the job to be processed again.
diff --git a/Documentation/media/uapi/jobs/jobs-queue.svg b/Documentation/media/uapi/jobs/jobs-queue.svg
new file mode 100644
index 000000000000..e6d9408fd1df
--- /dev/null
+++ b/Documentation/media/uapi/jobs/jobs-queue.svg
@@ -0,0 +1,192 @@
+<?xml version="1.0" encoding="UTF-8" standalone="no"?>
+<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.0//EN" "http://www.w3.org/TR/2001/PR-SVG-20010719/DTD/svg10.dtd">
+<svg width="33cm" height="23cm" viewBox="96 61 644 457" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
+  <text font-size="12.7998" style="fill: #000000;text-anchor:start;font-family:sans-serif;font-style:normal;font-weight:normal" x="198.6" y="319.8">
+    <tspan x="198.6" y="319.8"></tspan>
+  </text>
+  <g>
+    <rect style="fill: #a3d5eb" x="129.6" y="404" width="138" height="53"/>
+    <rect style="fill: none; fill-opacity:0; stroke-width: 2; stroke: #000000" x="129.6" y="404" width="138" height="53"/>
+    <text font-size="15.8042" style="fill: #000000;text-anchor:middle;font-family:sans-serif;font-style:normal;font-weight:normal" x="198.6" y="435.322">
+      <tspan x="198.6" y="435.322">Current</tspan>
+    </text>
+  </g>
+  <text font-size="12.7998" style="fill: #000000;text-anchor:start;font-family:sans-serif;font-style:normal;font-weight:normal" x="198.6" y="304.4">
+    <tspan x="198.6" y="304.4"></tspan>
+  </text>
+  <g>
+    <rect style="fill: #a3d5eb" x="129.6" y="262.6" width="138" height="53"/>
+    <rect style="fill: none; fill-opacity:0; stroke-width: 2; stroke: #000000" x="129.6" y="262.6" width="138" height="53"/>
+    <text font-size="15.8042" style="fill: #000000;text-anchor:middle;font-family:sans-serif;font-style:normal;font-weight:normal" x="198.6" y="293.922">
+      <tspan x="198.6" y="293.922"></tspan>
+    </text>
+  </g>
+  <text font-size="12.7998" style="fill: #000000;text-anchor:start;font-family:sans-serif;font-style:normal;font-weight:normal" x="198.6" y="289.1">
+    <tspan x="198.6" y="289.1"></tspan>
+  </text>
+  <g>
+    <rect style="fill: #a3d5eb" x="129.6" y="277.9" width="138" height="53"/>
+    <rect style="fill: none; fill-opacity:0; stroke-width: 2; stroke: #000000" x="129.6" y="277.9" width="138" height="53"/>
+    <text font-size="15.8042" style="fill: #000000;text-anchor:middle;font-family:sans-serif;font-style:normal;font-weight:normal" x="198.6" y="309.222">
+      <tspan x="198.6" y="309.222"></tspan>
+    </text>
+  </g>
+  <g>
+    <rect style="fill: #a3d5eb" x="129.6" y="293.3" width="138" height="53"/>
+    <rect style="fill: none; fill-opacity:0; stroke-width: 2; stroke: #000000" x="129.6" y="293.3" width="138" height="53"/>
+    <text font-size="15.8042" style="fill: #000000;text-anchor:middle;font-family:sans-serif;font-style:normal;font-weight:normal" x="198.6" y="324.622">
+      <tspan x="198.6" y="324.622">Queued</tspan>
+    </text>
+  </g>
+  <g>
+    <line style="fill: none; fill-opacity:0; stroke-width: 2; stroke: #000000" x1="198.6" y1="404" x2="198.6" y2="357.3"/>
+    <polygon style="fill: #000000" points="203.6,357.3 198.6,347.3 193.6,357.3 "/>
+    <polygon style="fill: none; fill-opacity:0; stroke-width: 2; stroke: #000000" points="203.6,357.3 198.6,347.3 193.6,357.3 "/>
+  </g>
+  <text font-size="12.7998" style="fill: #000000;text-anchor:start;font-family:sans-serif;font-style:normal;font-weight:normal" x="613.6" y="320.5">
+    <tspan x="613.6" y="320.5"></tspan>
+  </text>
+  <g>
+    <rect style="fill: #a3d5eb" x="544.6" y="404.7" width="138" height="53"/>
+    <rect style="fill: none; fill-opacity:0; stroke-width: 2; stroke: #000000" x="544.6" y="404.7" width="138" height="53"/>
+    <text font-size="15.8042" style="fill: #000000;text-anchor:middle;font-family:sans-serif;font-style:normal;font-weight:normal" x="613.6" y="436.022">
+      <tspan x="613.6" y="436.022">Dequeued</tspan>
+    </text>
+  </g>
+  <text font-size="12.7998" style="fill: #000000;text-anchor:start;font-family:sans-serif;font-style:normal;font-weight:normal" x="613.6" y="305.1">
+    <tspan x="613.6" y="305.1"></tspan>
+  </text>
+  <g>
+    <rect style="fill: #a3d5eb" x="544.6" y="263.3" width="138" height="53"/>
+    <rect style="fill: none; fill-opacity:0; stroke-width: 2; stroke: #000000" x="544.6" y="263.3" width="138" height="53"/>
+    <text font-size="15.8042" style="fill: #000000;text-anchor:middle;font-family:sans-serif;font-style:normal;font-weight:normal" x="613.6" y="294.622">
+      <tspan x="613.6" y="294.622"></tspan>
+    </text>
+  </g>
+  <text font-size="12.7998" style="fill: #000000;text-anchor:start;font-family:sans-serif;font-style:normal;font-weight:normal" x="613.6" y="289.8">
+    <tspan x="613.6" y="289.8"></tspan>
+  </text>
+  <g>
+    <rect style="fill: #a3d5eb" x="544.6" y="278.6" width="138" height="53"/>
+    <rect style="fill: none; fill-opacity:0; stroke-width: 2; stroke: #000000" x="544.6" y="278.6" width="138" height="53"/>
+    <text font-size="15.8042" style="fill: #000000;text-anchor:middle;font-family:sans-serif;font-style:normal;font-weight:normal" x="613.6" y="309.922">
+      <tspan x="613.6" y="309.922"></tspan>
+    </text>
+  </g>
+  <g>
+    <rect style="fill: #a3d5eb" x="544.6" y="294" width="138" height="53"/>
+    <rect style="fill: none; fill-opacity:0; stroke-width: 2; stroke: #000000" x="544.6" y="294" width="138" height="53"/>
+    <text font-size="15.8042" style="fill: #000000;text-anchor:middle;font-family:sans-serif;font-style:normal;font-weight:normal" x="613.6" y="325.322">
+      <tspan x="613.6" y="325.322">Completed</tspan>
+    </text>
+  </g>
+  <text font-size="12.7998" style="fill: #000000;text-anchor:start;font-family:sans-serif;font-style:normal;font-weight:normal" x="648.1" y="316.3">
+    <tspan x="648.1" y="316.3"></tspan>
+  </text>
+  <line style="fill: none; fill-opacity:0; stroke-width: 2; stroke: #000000" x1="98" y1="480" x2="724.125" y2="480.75"/>
+  <g>
+    <line style="fill: none; fill-opacity:0; stroke-width: 2; stroke: #000000" x1="613.6" y1="347" x2="613.6" y2="393.7"/>
+    <polygon style="fill: #000000" points="608.6,393.7 613.6,403.7 618.6,393.7 "/>
+    <polygon style="fill: none; fill-opacity:0; stroke-width: 2; stroke: #000000" points="608.6,393.7 613.6,403.7 618.6,393.7 "/>
+  </g>
+  <text font-size="12.7998" style="fill: #000000;text-anchor:start;font-family:sans-serif;font-style:normal;font-weight:normal" x="101" y="515">
+    <tspan x="101" y="515">S_EXT_CTRL(WHICH_JOB_CUR_VAL)</tspan>
+  </text>
+  <text font-size="12.7998" style="fill: #000000;text-anchor:start;font-family:sans-serif;font-style:normal;font-weight:normal" x="518" y="515.187">
+    <tspan x="518" y="515.187">G_EXT_CTRL(WHICH_DEQJOB_VAL)</tspan>
+  </text>
+  <g>
+    <line style="fill: none; fill-opacity:0; stroke-width: 2; stroke: #000000" x1="151.125" y1="495.293" x2="150.778" y2="470.292"/>
+    <polygon style="fill: #000000" points="155.777,470.222 150.639,460.293 145.778,470.361 "/>
+    <polygon style="fill: none; fill-opacity:0; stroke-width: 2; stroke: #000000" points="155.777,470.222 150.639,460.293 145.778,470.361 "/>
+  </g>
+  <g>
+    <line style="fill: none; fill-opacity:0; stroke-width: 2; stroke: #000000" x1="175.991" y1="495.293" x2="175.644" y2="470.292"/>
+    <polygon style="fill: #000000" points="180.643,470.222 175.505,460.293 170.644,470.361 "/>
+    <polygon style="fill: none; fill-opacity:0; stroke-width: 2; stroke: #000000" points="180.643,470.222 175.505,460.293 170.644,470.361 "/>
+  </g>
+  <g>
+    <line style="fill: none; fill-opacity:0; stroke-width: 2; stroke: #000000" x1="200.857" y1="495.293" x2="200.51" y2="470.292"/>
+    <polygon style="fill: #000000" points="205.51,470.222 200.371,460.293 195.511,470.361 "/>
+    <polygon style="fill: none; fill-opacity:0; stroke-width: 2; stroke: #000000" points="205.51,470.222 200.371,460.293 195.511,470.361 "/>
+  </g>
+  <g>
+    <line style="fill: none; fill-opacity:0; stroke-width: 2; stroke: #000000" x1="225.723" y1="495.293" x2="225.376" y2="470.292"/>
+    <polygon style="fill: #000000" points="230.376,470.222 225.237,460.293 220.377,470.361 "/>
+    <polygon style="fill: none; fill-opacity:0; stroke-width: 2; stroke: #000000" points="230.376,470.222 225.237,460.293 220.377,470.361 "/>
+  </g>
+  <g>
+    <line style="fill: none; fill-opacity:0; stroke-width: 2; stroke: #000000" x1="250.59" y1="495.293" x2="250.242" y2="470.292"/>
+    <polygon style="fill: #000000" points="255.242,470.222 250.104,460.293 245.243,470.361 "/>
+    <polygon style="fill: none; fill-opacity:0; stroke-width: 2; stroke: #000000" points="255.242,470.222 250.104,460.293 245.243,470.361 "/>
+  </g>
+  <g>
+    <line style="fill: none; fill-opacity:0; stroke-width: 2; stroke: #000000" x1="565.187" y1="485.337" x2="564.84" y2="460.336"/>
+    <polygon style="fill: #000000" points="560.187,485.406 565.326,495.336 570.186,485.267 "/>
+    <polygon style="fill: none; fill-opacity:0; stroke-width: 2; stroke: #000000" points="560.187,485.406 565.326,495.336 570.186,485.267 "/>
+  </g>
+  <g>
+    <line style="fill: none; fill-opacity:0; stroke-width: 2; stroke: #000000" x1="590.053" y1="485.337" x2="589.706" y2="460.336"/>
+    <polygon style="fill: #000000" points="585.054,485.406 590.192,495.336 595.053,485.267 "/>
+    <polygon style="fill: none; fill-opacity:0; stroke-width: 2; stroke: #000000" points="585.054,485.406 590.192,495.336 595.053,485.267 "/>
+  </g>
+  <g>
+    <line style="fill: none; fill-opacity:0; stroke-width: 2; stroke: #000000" x1="614.919" y1="485.337" x2="614.572" y2="460.336"/>
+    <polygon style="fill: #000000" points="609.92,485.406 615.058,495.336 619.919,485.267 "/>
+    <polygon style="fill: none; fill-opacity:0; stroke-width: 2; stroke: #000000" points="609.92,485.406 615.058,495.336 619.919,485.267 "/>
+  </g>
+  <g>
+    <line style="fill: none; fill-opacity:0; stroke-width: 2; stroke: #000000" x1="639.785" y1="485.337" x2="639.438" y2="460.336"/>
+    <polygon style="fill: #000000" points="634.786,485.406 639.924,495.336 644.785,485.267 "/>
+    <polygon style="fill: none; fill-opacity:0; stroke-width: 2; stroke: #000000" points="634.786,485.406 639.924,495.336 644.785,485.267 "/>
+  </g>
+  <g>
+    <line style="fill: none; fill-opacity:0; stroke-width: 2; stroke: #000000" x1="664.652" y1="485.337" x2="664.304" y2="460.336"/>
+    <polygon style="fill: #000000" points="659.652,485.406 664.79,495.336 669.651,485.267 "/>
+    <polygon style="fill: none; fill-opacity:0; stroke-width: 2; stroke: #000000" points="659.652,485.406 664.79,495.336 669.651,485.267 "/>
+  </g>
+  <g>
+    <rect style="fill: #a3d5eb" x="344.875" y="155.6" width="138" height="53"/>
+    <rect style="fill: none; fill-opacity:0; stroke-width: 2; stroke: #000000" x="344.875" y="155.6" width="138" height="53"/>
+    <text font-size="15.8042" style="fill: #000000;text-anchor:middle;font-family:sans-serif;font-style:normal;font-weight:normal" x="413.875" y="186.922">
+      <tspan x="413.875" y="186.922">Active</tspan>
+    </text>
+  </g>
+  <g>
+    <polyline style="fill: none; fill-opacity:0; stroke-width: 2; stroke: #000000" points="198.6,262.6 198.6,182.1 333.875,182.1 "/>
+    <polygon style="fill: #000000" points="333.875,187.1 343.875,182.1 333.875,177.1 "/>
+    <polygon style="fill: none; fill-opacity:0; stroke-width: 2; stroke: #000000" points="333.875,187.1 343.875,182.1 333.875,177.1 "/>
+  </g>
+  <g>
+    <polyline style="fill: none; fill-opacity:0; stroke-width: 2; stroke: #000000" points="482.875,182.1 613.6,182.1 613.6,252.3 "/>
+    <polygon style="fill: #000000" points="608.6,252.3 613.6,262.3 618.6,252.3 "/>
+    <polygon style="fill: none; fill-opacity:0; stroke-width: 2; stroke: #000000" points="608.6,252.3 613.6,262.3 618.6,252.3 "/>
+  </g>
+  <g>
+    <rect style="fill: #ffe283" x="289.125" y="62.6" width="249.5" height="53"/>
+    <rect style="fill: none; fill-opacity:0; stroke-width: 2; stroke: #000000" x="289.125" y="62.6" width="249.5" height="53"/>
+    <text font-size="15.8042" style="fill: #000000;text-anchor:middle;font-family:sans-serif;font-style:normal;font-weight:normal" x="413.875" y="93.9222">
+      <tspan x="413.875" y="93.9222">Devices</tspan>
+    </text>
+  </g>
+  <g>
+    <line style="fill: none; fill-opacity:0; stroke-width: 2; stroke: #000000" x1="369.34" y1="154.011" x2="368.992" y2="129.01"/>
+    <polygon style="fill: #000000" points="373.992,128.94 368.854,119.011 363.993,129.079 "/>
+    <polygon style="fill: none; fill-opacity:0; stroke-width: 2; stroke: #000000" points="373.992,128.94 368.854,119.011 363.993,129.079 "/>
+  </g>
+  <g>
+    <line style="fill: none; fill-opacity:0; stroke-width: 2; stroke: #000000" x1="454.59" y1="154.011" x2="454.242" y2="129.01"/>
+    <polygon style="fill: #000000" points="459.242,128.94 454.104,119.011 449.243,129.079 "/>
+    <polygon style="fill: none; fill-opacity:0; stroke-width: 2; stroke: #000000" points="459.242,128.94 454.104,119.011 449.243,129.079 "/>
+  </g>
+  <g>
+    <line style="fill: none; fill-opacity:0; stroke-width: 2; stroke: #000000" x1="397.423" y1="142.29" x2="397.076" y2="117.289"/>
+    <polygon style="fill: #000000" points="392.423,142.359 397.562,152.289 402.422,142.22 "/>
+    <polygon style="fill: none; fill-opacity:0; stroke-width: 2; stroke: #000000" points="392.423,142.359 397.562,152.289 402.422,142.22 "/>
+  </g>
+  <g>
+    <line style="fill: none; fill-opacity:0; stroke-width: 2; stroke: #000000" x1="425.84" y1="142.29" x2="425.492" y2="117.289"/>
+    <polygon style="fill: #000000" points="420.84,142.359 425.978,152.289 430.839,142.22 "/>
+    <polygon style="fill: none; fill-opacity:0; stroke-width: 2; stroke: #000000" points="420.84,142.359 425.978,152.289 430.839,142.22 "/>
+  </g>
+</svg>
diff --git a/Documentation/media/uapi/v4l/vidioc-g-ext-ctrls.rst b/Documentation/media/uapi/v4l/vidioc-g-ext-ctrls.rst
index 5ab8d2ac27b9..035d5e29bd2e 100644
--- a/Documentation/media/uapi/v4l/vidioc-g-ext-ctrls.rst
+++ b/Documentation/media/uapi/v4l/vidioc-g-ext-ctrls.rst
@@ -216,6 +216,12 @@ still cause this situation.
 	   You can only get the default value of the control,
 	   you cannot set or try it.
 
+	When the Jobs API is active ``V4L2_CTRL_WHICH_CURJOB_VAL`` will return or set the value of the current job, while ``V4L2_CTRL_WHICH_DEQJOB_VAL`` returns the value of the currently dequeued job.
+
+	.. note::
+
+	   It is invalid to try and set the value of the dequeued job.
+
 	For backwards compatibility you can also use a control class here
 	(see :ref:`ctrl-class`). In that case all controls have to
 	belong to that control class. This usage is deprecated, instead
-- 
2.14.2.822.g60be5d43e6-goog

Re: [RFC PATCH 2/9] [media] v4l2-core: add core jobs API support

[Hans Verkuil]

On 09/28/2017 11:50 AM, Alexandre Courbot wrote:
> Add core support code for jobs API. This manages the life cycle of jobs
> and creation of a jobs queue, as well as the interface for job states.
> 
> It also exposes the user-space jobs API.
> 
> Signed-off-by: Alexandre Courbot <acourbot@chromium.org>
> ---
>  drivers/media/v4l2-core/Makefile            |   3 +-
>  drivers/media/v4l2-core/v4l2-dev.c          |   6 +
>  drivers/media/v4l2-core/v4l2-jobqueue-dev.c | 173 +++++++
>  drivers/media/v4l2-core/v4l2-jobqueue.c     | 764 ++++++++++++++++++++++++++++
>  include/media/v4l2-dev.h                    |   4 +
>  include/media/v4l2-fh.h                     |   4 +
>  include/media/v4l2-job-state.h              |  75 +++
>  include/media/v4l2-jobqueue-dev.h           |  24 +
>  include/media/v4l2-jobqueue.h               |  54 ++
>  include/uapi/linux/v4l2-jobs.h              |  40 ++
>  include/uapi/linux/videodev2.h              |   2 +
>  11 files changed, 1148 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/media/v4l2-core/v4l2-jobqueue-dev.c
>  create mode 100644 drivers/media/v4l2-core/v4l2-jobqueue.c
>  create mode 100644 include/media/v4l2-job-state.h
>  create mode 100644 include/media/v4l2-jobqueue-dev.h
>  create mode 100644 include/media/v4l2-jobqueue.h
>  create mode 100644 include/uapi/linux/v4l2-jobs.h
> 
> diff --git a/drivers/media/v4l2-core/Makefile b/drivers/media/v4l2-core/Makefile
> index 098ad5fd5231..a717bb8f1a25 100644
> --- a/drivers/media/v4l2-core/Makefile
> +++ b/drivers/media/v4l2-core/Makefile
> @@ -6,7 +6,8 @@ tuner-objs	:=	tuner-core.o
>  
>  videodev-objs	:=	v4l2-dev.o v4l2-ioctl.o v4l2-device.o v4l2-fh.o \
>  			v4l2-event.o v4l2-ctrls.o v4l2-subdev.o v4l2-clk.o \
> -			v4l2-async.o
> +			v4l2-async.o v4l2-jobqueue.o v4l2-jobqueue-dev.o
> +
>  ifeq ($(CONFIG_COMPAT),y)
>    videodev-objs += v4l2-compat-ioctl32.o
>  endif
> diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
> index 5a7063886c93..fb229b671b9d 100644
> --- a/drivers/media/v4l2-core/v4l2-dev.c
> +++ b/drivers/media/v4l2-core/v4l2-dev.c
> @@ -30,6 +30,7 @@
>  #include <media/v4l2-common.h>
>  #include <media/v4l2-device.h>
>  #include <media/v4l2-ioctl.h>
> +#include <media/v4l2-jobqueue-dev.h>
>  
>  #define VIDEO_NUM_DEVICES	256
>  #define VIDEO_NAME              "video4linux"
> @@ -1058,6 +1059,10 @@ static int __init videodev_init(void)
>  		return -EIO;
>  	}
>  
> +	ret = v4l2_jobqueue_device_init();
> +	if (ret < 0)
> +		printk(KERN_WARNING "video_dev: channel initialization failed\n");
> +
>  	return 0;
>  }
>  
> @@ -1065,6 +1070,7 @@ static void __exit videodev_exit(void)
>  {
>  	dev_t dev = MKDEV(VIDEO_MAJOR, 0);
>  
> +	v4l2_jobqueue_device_exit();
>  	class_unregister(&video_class);
>  	unregister_chrdev_region(dev, VIDEO_NUM_DEVICES);
>  }
> diff --git a/drivers/media/v4l2-core/v4l2-jobqueue-dev.c b/drivers/media/v4l2-core/v4l2-jobqueue-dev.c
> new file mode 100644
> index 000000000000..688c4ba275a6
> --- /dev/null
> +++ b/drivers/media/v4l2-core/v4l2-jobqueue-dev.c
> @@ -0,0 +1,173 @@
> +/*
> +    V4L2 job queue device
> +
> +    Copyright (C) 2017  The Chromium project
> +
> +    This program is free software; you can redistribute it and/or modify
> +    it under the terms of the GNU General Public License as published by
> +    the Free Software Foundation; either version 2 of the License, or
> +    (at your option) any later version.
> +
> +    This program is distributed in the hope that it will be useful,
> +    but WITHOUT ANY WARRANTY; without even the implied warranty of
> +    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +    GNU General Public License for more details.
> +
> + */
> +
> +#include <linux/device.h>
> +#include <linux/module.h>
> +#include <linux/fs.h>
> +#include <media/v4l2-ioctl.h>
> +#include <media/v4l2-jobqueue.h>
> +#include <uapi/linux/v4l2-jobs.h>
> +
> +#define CLASS_NAME "v4l2_jobqueue"
> +#define DEVICE_NAME "v4l2_jobqueue"
> +
> +static int major;
> +static struct class *jobqueue_class;
> +static struct device *jobqueue_device;
> +
> +static int v4l2_jobqueue_device_open(struct inode *inode, struct file *filp)
> +{
> +	struct v4l2_jobqueue *jq;
> +
> +	jq = v4l2_jobqueue_new();
> +	if (IS_ERR(jq))
> +		return PTR_ERR(jq);
> +
> +	filp->private_data = jq;
> +
> +	return 0;
> +}
> +
> +static int v4l2_jobqueue_device_release(struct inode *inode, struct file *filp)
> +{
> +	struct v4l2_jobqueue *jq = filp->private_data;
> +
> +	return v4l2_jobqueue_del(jq);
> +}
> +
> +static long v4l2_jobqueue_ioctl_init(struct file *filp, void *arg)
> +{
> +	struct v4l2_jobqueue *jq = filp->private_data;
> +	struct v4l2_jobqueue_init *cinit = arg;
> +
> +	return v4l2_jobqueue_init(jq, cinit);
> +}
> +
> +static long v4l2_jobqueue_device_ioctl_qjob(struct file *filp, void *arg)
> +{
> +	struct v4l2_jobqueue *jq = filp->private_data;
> +
> +	return v4l2_jobqueue_qjob(jq);
> +}
> +
> +static long v4l2_jobqueue_device_ioctl_dqjob(struct file *filp, void *arg)
> +{
> +	struct v4l2_jobqueue *jq = filp->private_data;
> +
> +	return v4l2_jobqueue_dqjob(jq);
> +}
> +
> +static long v4l2_jobqueue_device_ioctl_export_job(struct file *filp, void *arg)
> +{
> +	struct v4l2_jobqueue *jq = filp->private_data;
> +	struct v4l2_jobqueue_job *job = arg;
> +
> +	return v4l2_jobqueue_export_job(jq, job);
> +}
> +
> +static long v4l2_jobqueue_device_ioctl_import_job(struct file *filp, void *arg)
> +{
> +	struct v4l2_jobqueue *jq = filp->private_data;
> +	struct v4l2_jobqueue_job *job = arg;
> +
> +	return v4l2_jobqueue_import_job(jq, job);
> +}
> +
> +static long v4l2_jobqueue_device_do_ioctl(struct file *filp, unsigned int cmd,
> +					  void *arg)
> +{
> +	switch (cmd) {
> +		case VIDIOC_JOBQUEUE_INIT:
> +			return v4l2_jobqueue_ioctl_init(filp, arg);
> +
> +		case VIDIOC_JOBQUEUE_QJOB:
> +			return v4l2_jobqueue_device_ioctl_qjob(filp, arg);
> +
> +		case VIDIOC_JOBQUEUE_DQJOB:
> +			return v4l2_jobqueue_device_ioctl_dqjob(filp, arg);
> +
> +		case VIDIOC_JOBQUEUE_EXPORT_JOB:
> +			return v4l2_jobqueue_device_ioctl_export_job(filp, arg);
> +
> +		case VIDIOC_JOBQUEUE_IMPORT_JOB:
> +			return v4l2_jobqueue_device_ioctl_import_job(filp, arg);

There really is no need for these stub functions, just inline them for each
case.

> +
> +		default:
> +			pr_err("Invalid ioctl!\n");
> +			return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static long v4l2_jobqueue_device_ioctl(struct file *filp, unsigned int cmd,
> +				       unsigned long arg)
> +{
> +	return video_usercopy(filp, cmd, arg, v4l2_jobqueue_device_do_ioctl);
> +}
> +
> +static const struct file_operations v4l2_jobqueue_devnode_fops = {
> +	.owner = THIS_MODULE,
> +	.open = v4l2_jobqueue_device_open,
> +	.unlocked_ioctl = v4l2_jobqueue_device_ioctl,
> +#ifdef CONFIG_COMPAT
> +	/* TODO */
> +	/* .compat_ioctl = jobqueue_compat_ioctl, */
> +#endif
> +	.release = v4l2_jobqueue_device_release,
> +};
> +
> +int __init v4l2_jobqueue_device_init(void)
> +{
> +	/* Set to error value so v4l2_jobqueue_device_exit does nothing if we
> +	 * don't initialize properly */
> +	jobqueue_device = ERR_PTR(-EINVAL);
> +
> +	major = register_chrdev(0, DEVICE_NAME, &v4l2_jobqueue_devnode_fops);
> +	if (major < 0) {
> +		pr_err("unable to allocate major\n");
> +		return major;
> +	}
> +
> +	jobqueue_class = class_create(THIS_MODULE, CLASS_NAME);
> +	if (IS_ERR(jobqueue_class)) {
> +		pr_err("cannot create class\n");
> +		unregister_chrdev(major, DEVICE_NAME);
> +		return PTR_ERR(jobqueue_class);
> +	}
> +
> +	jobqueue_device = device_create(jobqueue_class, NULL, MKDEV(major, 0),
> +					NULL, DEVICE_NAME);
> +	if (IS_ERR(jobqueue_device)) {
> +		pr_err("cannot create device\n");
> +		class_destroy(jobqueue_class);
> +		unregister_chrdev(major, DEVICE_NAME);
> +		return PTR_ERR(jobqueue_device);
> +	}
> +
> +	return 0;
> +}
> +
> +void __exit v4l2_jobqueue_device_exit(void)
> +{
> +	if (IS_ERR(jobqueue_device))
> +		return;
> +
> +	device_destroy(jobqueue_class, MKDEV(major, 0));
> +	class_destroy(jobqueue_class);
> +	unregister_chrdev(major, DEVICE_NAME);
> +}

<snip>

> diff --git a/drivers/media/v4l2-core/v4l2-jobqueue.c b/drivers/media/v4l2-core/v4l2-jobqueue.c
> new file mode 100644
> index 000000000000..36d2dd48b086
> --- /dev/null
> +++ b/drivers/media/v4l2-core/v4l2-jobqueue.c
> @@ -0,0 +1,764 @@
> +/*
> +    V4L2 job queue implementation
> +
> +    Copyright (C) 2017  The Chromium project
> +
> +    This program is free software; you can redistribute it and/or modify
> +    it under the terms of the GNU General Public License as published by
> +    the Free Software Foundation; either version 2 of the License, or
> +    (at your option) any later version.
> +
> +    This program is distributed in the hope that it will be useful,
> +    but WITHOUT ANY WARRANTY; without even the implied warranty of
> +    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +    GNU General Public License for more details.
> +
> + */
> +
> +#include <linux/compat.h>
> +#include <linux/export.h>
> +#include <linux/string.h>
> +#include <linux/file.h>
> +#include <linux/list.h>
> +#include <linux/kref.h>
> +#include <linux/anon_inodes.h>
> +#include <linux/slab.h>
> +#include <linux/mutex.h>
> +#include <linux/workqueue.h>
> +#include <media/v4l2-dev.h>
> +#include <media/v4l2-fh.h>
> +#include <media/v4l2-ctrls.h>
> +#include <media/v4l2-jobqueue.h>
> +#include <media/v4l2-job-state.h>
> +#include <uapi/linux/v4l2-jobs.h>
> +
> +/* Limited by the size of atomic_t to track devices that completed a job */
> +#define V4L2_JOBQUEUE_MAX_DEVICES sizeof(atomic_t)
> +
> +/*
> + * State of all managed devices for a given job
> + */
> +struct v4l2_job {
> +	struct kref refcount;
> +	struct v4l2_jobqueue *jq;
> +	/* node in v4l2_jobqueue's queued_jobs or completed_jobs */
> +	struct list_head node;
> +	/* global list of existing jobs for this queue */
> +	struct list_head jobs_list;
> +	/* mask of devices that completed this job */
> +	atomic_t completed;
> +	/* fd exported to user-space */
> +	int fd;
> +	enum v4l2_job_status status;
> +
> +	/* per-device states */
> +	struct v4l2_job_state *state[0];
> +};
> +
> +/*
> + * A job queue manages the job flow for a given set of devices, applies their
> + * state, and activates them in lockstep.
> + *
> + * A job goes through the following stages through its life:
> + *
> + * * current_job: the job has been created and is waiting to be queued. S_CTRL
> + *   will apply to it. Once queued, it is pushed into
> + * * queued_jobs: a queue of jobs to be processed in sequential order. The head
> + *   of this list becomes the
> + * * active_job: the job currently being processed by the hardware. Once
> + *   completed, the next job in queued_job becomes active, and the previous
> + *   active job goes into
> + * * completed_jobs: a list of completed jobs waiting to be dequeued by
> + *   user-space. As user-space called the DQJOB ioctl, the head becomes the
> + * * dequeued_job: the job on which G_CTRL will be performed on. A job stays
> + *   in this state until another one is dequeued, at which point it is deleted.
> + */
> +struct v4l2_jobqueue {
> +	/* List of all jobs created for this queue, regardless of state */
> +	struct list_head jobs_list;
> +	/*
> +	 * Job that user-space is currently preparing, to be added to
> +	 * queued_jobs upon QJOB ioctl.
> +	 */
> +	struct v4l2_job *current_job;
> +
> +	/* List of jobs that are ready to be processed */
> +	struct list_head queued_jobs;
> +
> +	/* Job that is currently processed by the devices */
> +	struct v4l2_job *active_job;

Shouldn't this be a list as well? I interpret 'active job' as being a
job that is passed to the various drivers that need to process it.

Just as with video buffers where the hardware may need a minimum of
buffers before it can start the DMA (min_buffers_needed), so the same
is true for jobs. E.g. a driver may have to look ahead by a few frames
to see what changes are requested. Some changes (esp. sensor related)
can take a few frames before they take effect and the hardware has to
be programmed ahead of time.

It would make more sense if this was a list of active jobs. It would
likely also solve the TODOs you have in the code w.r.t. min_buffers_needed:
after STREAMON you'd just queue the jobs to the active list, and also
queue any associated buffers. Once the minimum number of buffers has been
reached the DMA is started.

> +
> +	/* List of completed jobs, ready to be dequeued */
> +	struct list_head completed_jobs;
> +
> +	/* Job that has last been dequeued and can be queried by user-space */
> +	struct v4l2_job *dequeued_job;
> +
> +	/* Projects the *_job[s] lists/pointers above */
> +	struct mutex lock;
> +	struct work_struct job_complete_work;
> +
> +	wait_queue_head_t done_wq;
> +
> +	unsigned int nb_devs;
> +	struct {
> +		struct file *f;
> +		struct v4l2_job_state_handler *state_handler;
> +	} *devs;
> +};

<snip>

> diff --git a/include/uapi/linux/v4l2-jobs.h b/include/uapi/linux/v4l2-jobs.h
> new file mode 100644
> index 000000000000..2cba4d20e62f
> --- /dev/null
> +++ b/include/uapi/linux/v4l2-jobs.h
> @@ -0,0 +1,40 @@
> +/*
> + * V4L2 jobs API
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#ifndef __LINUX_V4L2_JOBS_H
> +#define __LINUX_V4L2_JOBS_H
> +
> +#ifndef __KERNEL__
> +#include <stdint.h>
> +#endif
> +#include <linux/ioctl.h>
> +#include <linux/types.h>
> +
> +struct v4l2_jobqueue_init {
> +	__u32 nb_devs;
> +	__s32 *fd;
> +};
> +
> +struct v4l2_jobqueue_job {
> +	__s32 fd;
> +};
> +
> +#define VIDIOC_JOBQUEUE_IOCTL_START	0x80

Why this offset?

> +
> +#define VIDIOC_JOBQUEUE_INIT		_IOW('|', VIDIOC_JOBQUEUE_IOCTL_START + 0x00, struct v4l2_jobqueue_init)
> +#define VIDIOC_JOBQUEUE_QJOB		_IO('|', VIDIOC_JOBQUEUE_IOCTL_START + 0x01)
> +#define VIDIOC_JOBQUEUE_DQJOB		_IO('|', VIDIOC_JOBQUEUE_IOCTL_START + 0x02)
> +#define VIDIOC_JOBQUEUE_EXPORT_JOB	_IOR('|', VIDIOC_JOBQUEUE_IOCTL_START + 0x03, struct v4l2_jobqueue_job)
> +#define VIDIOC_JOBQUEUE_IMPORT_JOB	_IOW('|', VIDIOC_JOBQUEUE_IOCTL_START + 0x03, struct v4l2_jobqueue_job)
> +
> +#endif
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index 45cf7359822c..7f43e97cf461 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -1591,6 +1591,8 @@ struct v4l2_ext_controls {
>  #define V4L2_CTRL_MAX_DIMS	  (4)
>  #define V4L2_CTRL_WHICH_CUR_VAL   0
>  #define V4L2_CTRL_WHICH_DEF_VAL   0x0f000000
> +#define V4L2_CTRL_WHICH_CURJOB_VAL   0x0e000000
> +#define V4L2_CTRL_WHICH_DEQJOB_VAL   0x0d000000
>  
>  enum v4l2_ctrl_type {
>  	V4L2_CTRL_TYPE_INTEGER	     = 1,
> 

Regards,

	Hans

Re: [RFC PATCH 0/9] V4L2 Jobs API WIP

[Hans Verkuil]

Hi Alexandre,

Thank you very much for working on this. Much appreciated!

I only did a very high-level review of the patch series: there is not much
point IMHO of doing a detailed review given the upcoming discussions in
Prague. It's better to wait until we agree with the high-level API.

Regarding the public API: the ioctls seem sane. It's all very similar to the
other implementations we've seen.

I'm still not sure about the name 'job', but this is 'just' a naming issue.

The part where I have more doubts is the need to create a new device node.

For the upcoming meeting I would like to discuss whether this cannot be added
to the media API.

Originally the plan was that the media API would be subsystem-agnostic and could
also be used by ALSA/DRM/etc. This never happened and I also am not aware of any
movement in that area.

I am wondering whether we should just be realistic and abandon the 'subsystem
agnostic' part and be willing to add e.g. the job support to the media API.

We can also consider controlling sub-devices via the media device node instead
of creating separate v4l-subdev device nodes.

This does not necessarily preclude the use of the media device by other
subsystems, but it certainly ties it much closer to the media subsystem. On the
other hand, it does make life easier for us (and I like easy!).

This is just a brainstorm at the moment, but I am interested what others think
of making /dev/mediaX specific to the media subsystem (at least we chose the
right name for that device node!).

Obviously, if we go into that direction, then that will have an obvious impact
on the jobs API, especially if we want to be able to control subdevs through the
media device instead of through the v4l-subdev devices.

Regards,

	Hans

On 09/28/2017 11:50 AM, Alexandre Courbot wrote:
> Hi everyone,
> 
> Here is a new attempt at the "request" (which I propose to rename "jobs") API
> for V4L2, hopefully in a manner that can converge to something that will be
> merged. The core ideas should be easy to grasp for those familiar with the
> previous attemps, yet there are a few important differences.
> 
> Most notably, user-space does not need to explicitly allocate and manage
> requests/jobs (but still can if this makes sense). We noticed that only specific
> use-cases require such an explicit management, and opted for a jobs queue that
> controls the flow of work over a set of opened devices. This should simplify
> user-space code quite a bit, while still retaining the ability to manage states
> explicitly like the previous request API proposals allowed to do.
> 
> The jobs API defines a few new concepts that user-space can use to control the
> workflow on a set of opened V4L2 devices:
> 
> A JOB QUEUE can be created from a set of opened FDs that are part of a pipeline
> and need to cooperate (be it capture, m2m, or media controller devices).
> 
> A JOB can then be set up with regular (if slightly modified) V4L2 ioctls, and
> then submitted to the job queue. Once the job queue schedules the job, its
> parameters (controls, etc) are applied to the devices of the queue, and itsd
> buffers are processed. Immediately after a job is submitted, the next job is
> ready to be set up without further user action.
> 
> Once a job completes, it must be dequeued and user-space can then read back its
> properties (notably controls) at completion time.
> 
> Internally, the state of jobs is managed through STATE HANDLERS. Each driver
> supporting the jobs API needs to specify an implementation of a state handler.
> Fortunately, most drivers can rely on the generic state handler implementation
> that simply records and replays a job's parameter using standard V4L2 functions.
> Thanks to this, adding jobs API support to a driver relying on the control
> framework and vb2 only requires a dozen lines of codes.
> 
> Drivers with specific needs or opportunities for optimization can however
> provide their own implementation of a state handler. This may in particular be
> beneficial for hardware that supports configuration or command buffers (thinking
> about VSP1 here).
> 
> This is still very early work, and focus has been on the following points:
> 
> * Provide something that anybody can test (currently using vim2m and vivid),
> * Reuse the current V4L2 APIs as much as possible,
> * Remain flexible enough to accomodate the inevitable changes that will be
>   requested,
> * Keep line count low, even if functionality is missing at the moment.
> 
> Please keep this in mind while going through the patches. In particular, at the
> moment the parameters of a job are limited to integer controls. I know that much
> more is expected, but V4L2 has quite a learning curve and I preferred to focus
> on the general concepts for now. More is coming though! :)
> 
> I have written two small example programs that demonstrate the use of this API:
> 
> - With a codec device (vim2m): https://gist.github.com/Gnurou/34c35f1f8e278dad454b51578d239a42
> 
> - With a capture device (vivid): https://gist.github.com/Gnurou/5052e6ab41e7c55164b75d2970bc5a04
> 
> Considering the history with the request API, I don't expect everything proposed
> here to be welcome or understood immediately. In particular I apologize for not
> reusing any of the previous attempts - I was just more comfortable laying down
> my ideas from scratch.
> 
> If this proposal is not dismissed as complete garbage I will also be happy to
> discuss it in-person at the mini-summit in Prague. :)
> 
> Cheers,
> Alex.
> 
> Alexandre Courbot (9):
>   [media] v4l2-core: add v4l2_is_v4l2_file function
>   [media] v4l2-core: add core jobs API support
>   [media] videobuf2: add support for jobs API
>   [media] v4l2-ctrls: add support for jobs API
>   [media] v4l2-job: add generic jobs ops
>   [media] m2m: add generic support for jobs API
>   [media] vim2m: add jobs API support
>   [media] vivid: add jobs API support for capture device
>   [media] document jobs API
> 
>  Documentation/media/intro.rst                      |   2 +
>  Documentation/media/media_uapi.rst                 |   1 +
>  Documentation/media/uapi/jobs/jobs-api.rst         |  23 +
>  Documentation/media/uapi/jobs/jobs-example.rst     |  69 ++
>  Documentation/media/uapi/jobs/jobs-intro.rst       |  61 ++
>  Documentation/media/uapi/jobs/jobs-queue.rst       |  73 ++
>  Documentation/media/uapi/jobs/jobs-queue.svg       | 192 ++++++
>  .../media/uapi/v4l/vidioc-g-ext-ctrls.rst          |   6 +
>  drivers/media/platform/vim2m.c                     |  24 +
>  drivers/media/platform/vivid/vivid-core.c          |  16 +
>  drivers/media/platform/vivid/vivid-core.h          |   2 +
>  drivers/media/platform/vivid/vivid-kthread-cap.c   |   5 +
>  drivers/media/v4l2-core/Makefile                   |   4 +-
>  drivers/media/v4l2-core/v4l2-ctrls.c               |  50 +-
>  drivers/media/v4l2-core/v4l2-dev.c                 |  12 +
>  drivers/media/v4l2-core/v4l2-job-generic.c         | 394 +++++++++++
>  drivers/media/v4l2-core/v4l2-jobqueue-dev.c        | 173 +++++
>  drivers/media/v4l2-core/v4l2-jobqueue.c            | 764 +++++++++++++++++++++
>  drivers/media/v4l2-core/v4l2-mem2mem.c             |  19 +
>  drivers/media/v4l2-core/videobuf2-core.c           |  33 +-
>  include/media/v4l2-ctrls.h                         |   6 +
>  include/media/v4l2-dev.h                           |  13 +
>  include/media/v4l2-fh.h                            |   4 +
>  include/media/v4l2-job-generic.h                   |  47 ++
>  include/media/v4l2-job-state.h                     |  75 ++
>  include/media/v4l2-jobqueue-dev.h                  |  24 +
>  include/media/v4l2-jobqueue.h                      |  54 ++
>  include/media/v4l2-mem2mem.h                       |  11 +
>  include/media/videobuf2-core.h                     |  16 +
>  include/uapi/linux/v4l2-jobs.h                     |  40 ++
>  include/uapi/linux/videodev2.h                     |   2 +
>  31 files changed, 2205 insertions(+), 10 deletions(-)
>  create mode 100644 Documentation/media/uapi/jobs/jobs-api.rst
>  create mode 100644 Documentation/media/uapi/jobs/jobs-example.rst
>  create mode 100644 Documentation/media/uapi/jobs/jobs-intro.rst
>  create mode 100644 Documentation/media/uapi/jobs/jobs-queue.rst
>  create mode 100644 Documentation/media/uapi/jobs/jobs-queue.svg
>  create mode 100644 drivers/media/v4l2-core/v4l2-job-generic.c
>  create mode 100644 drivers/media/v4l2-core/v4l2-jobqueue-dev.c
>  create mode 100644 drivers/media/v4l2-core/v4l2-jobqueue.c
>  create mode 100644 include/media/v4l2-job-generic.h
>  create mode 100644 include/media/v4l2-job-state.h
>  create mode 100644 include/media/v4l2-jobqueue-dev.h
>  create mode 100644 include/media/v4l2-jobqueue.h
>  create mode 100644 include/uapi/linux/v4l2-jobs.h
> 

Re: [RFC PATCH 0/9] V4L2 Jobs API WIP

[Sakari Ailus]

Hi Alexandre,

On Thu, Sep 28, 2017 at 06:50:18PM +0900, Alexandre Courbot wrote:
> Hi everyone,
> 
> Here is a new attempt at the "request" (which I propose to rename "jobs") API
> for V4L2, hopefully in a manner that can converge to something that will be
> merged. The core ideas should be easy to grasp for those familiar with the
> previous attemps, yet there are a few important differences.
> 
> Most notably, user-space does not need to explicitly allocate and manage
> requests/jobs (but still can if this makes sense). We noticed that only specific
> use-cases require such an explicit management, and opted for a jobs queue that
> controls the flow of work over a set of opened devices. This should simplify
> user-space code quite a bit, while still retaining the ability to manage states
> explicitly like the previous request API proposals allowed to do.
> 
> The jobs API defines a few new concepts that user-space can use to control the
> workflow on a set of opened V4L2 devices:
> 
> A JOB QUEUE can be created from a set of opened FDs that are part of a pipeline
> and need to cooperate (be it capture, m2m, or media controller devices).
> 
> A JOB can then be set up with regular (if slightly modified) V4L2 ioctls, and
> then submitted to the job queue. Once the job queue schedules the job, its
> parameters (controls, etc) are applied to the devices of the queue, and itsd
> buffers are processed. Immediately after a job is submitted, the next job is
> ready to be set up without further user action.
> 
> Once a job completes, it must be dequeued and user-space can then read back its
> properties (notably controls) at completion time.
> 
> Internally, the state of jobs is managed through STATE HANDLERS. Each driver
> supporting the jobs API needs to specify an implementation of a state handler.
> Fortunately, most drivers can rely on the generic state handler implementation
> that simply records and replays a job's parameter using standard V4L2 functions.
> Thanks to this, adding jobs API support to a driver relying on the control
> framework and vb2 only requires a dozen lines of codes.
> 
> Drivers with specific needs or opportunities for optimization can however
> provide their own implementation of a state handler. This may in particular be
> beneficial for hardware that supports configuration or command buffers (thinking
> about VSP1 here).
> 
> This is still very early work, and focus has been on the following points:
> 
> * Provide something that anybody can test (currently using vim2m and vivid),
> * Reuse the current V4L2 APIs as much as possible,
> * Remain flexible enough to accomodate the inevitable changes that will be
>   requested,
> * Keep line count low, even if functionality is missing at the moment.
> 
> Please keep this in mind while going through the patches. In particular, at the
> moment the parameters of a job are limited to integer controls. I know that much
> more is expected, but V4L2 has quite a learning curve and I preferred to focus
> on the general concepts for now. More is coming though! :)
> 
> I have written two small example programs that demonstrate the use of this API:
> 
> - With a codec device (vim2m): https://gist.github.com/Gnurou/34c35f1f8e278dad454b51578d239a42
> 
> - With a capture device (vivid): https://gist.github.com/Gnurou/5052e6ab41e7c55164b75d2970bc5a04
> 
> Considering the history with the request API, I don't expect everything proposed
> here to be welcome or understood immediately. In particular I apologize for not
> reusing any of the previous attempts - I was just more comfortable laying down
> my ideas from scratch.
> 
> If this proposal is not dismissed as complete garbage I will also be happy to
> discuss it in-person at the mini-summit in Prague. :)

Thank you for the initiative and the patchset.

While reviewing this patchset, I'm concentrating primarily on the approach
taken and the design, not so much in the actual implementation which I
don't think matters much at this moment.

It's difficult to avoid seeing many similarities with the Request API
patches posted earlier on. And not only that, rather you have to start
looking for the differences in what I could call details, while important
design decisions could sometimes be only visible in what appear details at
this point.

Both request and jobs APIs have the concept of a request, or a job, which
is created by the user and then different buffers or controls can be bound
to that request. (Other configuration isn't really excluded but it's
non-trivial to implement in practice.) This is common for both.

The differences begin however how the functionality within the scope is
actieved, in particular:

- A new user space interface (character device) is created for the jobs API
  whereas requests use existing Media controller interface. Therefore the
  jobs API is specific to V4L2. Consequently, the V4L2 jobs API may not
  support Media controller link state changes through requests.
  
  I don't see a reason why it should be this way, and I also see no reason
  why there should be yet another user space interface for this purpose
  alone: this is a clear drawback compared to handling this through the
  Media device.

- Controls and buffers are always bound to requests explicitly whereas for
  jobs, this seems to be implicit based on associating the file handle
  related to the relevant video device with the request.

  There are advantages in the approach, but currently I'd see that they're
  primarily related to not having to change the existing IOCTL argument
  structs.
  
  There lie problems, too, because of this: with requests (or jobs) it is
  vitally important that the user will always have the information if a
  buffer, control event etc. was related to a request (or not), and in
  particular which request it was.

  You could say that the user must keep track of this information but I'd
  suppose it won't make it easier for the user no having an important piece
  of information. Having this information as part of the IOCTL argument
  struct is mandatory for e.g. events that the user doesn't have an ID to
  compare with in order to find the related request.

  Also --- when an association with a video devnode file descriptor and a
  job with a request is made, when does it cease to exist? When the job is
  released? When the job is done?

There are smaller differences, not very important IMO:

- Requests are removed once complete. This was done to simplify the
  implementation and it could be added if it is seen reasonable to
  implement and useful, neither of which is known.

These are differences, I'd say, in the parts that are somewhat manageable
in any way they're designed, and with rather easily understandable
consequences from these design decisions. I still prefer the design choices
made for the Request API (regarding device nodes and request association
especially).

The hard stuff, e.g. how do you implement including non-buffer and
non-control configuration into the request in a meaningful way in an actual
driver I haven't seen yet. We'd need one driver to implement that, and in
general case it likely requires locking scheme changes in MC, for instance.
There are still use cases where this all isn't needed so there is a
motivation to have less than full implementation in the mainline kernel.

Another matter is making videobuf2 helpful here: we should have, if not in
the videobuf2 framework itself, then around it helper function(s) to manage
the submission of buffers to a driver. You can get things working pretty
easily but the error handling is very painful: what do you do, for
instance, with buffers queued with a request if queueing the request itself
fails, possibly because the user hasn't provided enough buffers with the
request? Mark the buffers errorneous and return them to the user? Probably
so, but that requires the user to dequeue the buffers and gather the
request again. I presume this would only happen in special circumstances
though, and not typically in an application using requests. This, and many
other special cases still must be handled by the kernel.

Finally, I want to say that I like some aspects of the patchset, such as
moving more things to the V4L2 framework from the driver. This would be
very useful in helping driver implementation: V4L2 is very stream-centric
and the configurations and buffers across device nodes have been
essentially independent from API point of view. Associating the pieces of
information together in requests would be painful to do in drivers. What
the framework can do, still controlled by drivers, could help a lot here.
This aspect wasn't much considered where the Request API work was left.

Still it shouldn't be forgotten that if the framework is geared towards
helping drivers "running one job at a time" the scope will be limited to
memory-to-memory devices; streaming devices, e.g. all kinds of cameras have
multiple requests being processed simultaneously (or frames are bound to be
lost, something we can't allow to happen due to framework design). And I
believe the memory-to-memory devices are generally the easy case. That
could be one option to start with, but at the same time we have to make it
absolutely certain we will not paint ourselves to the corner: the V4L2 UAPI
(or even KAPI) paint will take much longer to dry than the regular one.

-- 
Kind regards,

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

Re: [RFC PATCH 2/9] [media] v4l2-core: add core jobs API support

[Alexandre Courbot]

Hi Hans,

On Mon, Oct 16, 2017 at 7:01 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
>> +static long v4l2_jobqueue_device_do_ioctl(struct file *filp, unsigned int cmd,
>> +                                       void *arg)
>> +{
>> +     switch (cmd) {
>> +             case VIDIOC_JOBQUEUE_INIT:
>> +                     return v4l2_jobqueue_ioctl_init(filp, arg);
>> +
>> +             case VIDIOC_JOBQUEUE_QJOB:
>> +                     return v4l2_jobqueue_device_ioctl_qjob(filp, arg);
>> +
>> +             case VIDIOC_JOBQUEUE_DQJOB:
>> +                     return v4l2_jobqueue_device_ioctl_dqjob(filp, arg);
>> +
>> +             case VIDIOC_JOBQUEUE_EXPORT_JOB:
>> +                     return v4l2_jobqueue_device_ioctl_export_job(filp, arg);
>> +
>> +             case VIDIOC_JOBQUEUE_IMPORT_JOB:
>> +                     return v4l2_jobqueue_device_ioctl_import_job(filp, arg);
>
> There really is no need for these stub functions, just inline them for each
> case.

Indeed!

>> diff --git a/drivers/media/v4l2-core/v4l2-jobqueue.c b/drivers/media/v4l2-core/v4l2-jobqueue.c
>> new file mode 100644
>> index 000000000000..36d2dd48b086
>> --- /dev/null
>> +++ b/drivers/media/v4l2-core/v4l2-jobqueue.c
>> @@ -0,0 +1,764 @@
>> +/*
>> +    V4L2 job queue implementation
>> +
>> +    Copyright (C) 2017  The Chromium project
>> +
>> +    This program is free software; you can redistribute it and/or modify
>> +    it under the terms of the GNU General Public License as published by
>> +    the Free Software Foundation; either version 2 of the License, or
>> +    (at your option) any later version.
>> +
>> +    This program is distributed in the hope that it will be useful,
>> +    but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> +    GNU General Public License for more details.
>> +
>> + */
>> +
>> +#include <linux/compat.h>
>> +#include <linux/export.h>
>> +#include <linux/string.h>
>> +#include <linux/file.h>
>> +#include <linux/list.h>
>> +#include <linux/kref.h>
>> +#include <linux/anon_inodes.h>
>> +#include <linux/slab.h>
>> +#include <linux/mutex.h>
>> +#include <linux/workqueue.h>
>> +#include <media/v4l2-dev.h>
>> +#include <media/v4l2-fh.h>
>> +#include <media/v4l2-ctrls.h>
>> +#include <media/v4l2-jobqueue.h>
>> +#include <media/v4l2-job-state.h>
>> +#include <uapi/linux/v4l2-jobs.h>
>> +
>> +/* Limited by the size of atomic_t to track devices that completed a job */
>> +#define V4L2_JOBQUEUE_MAX_DEVICES sizeof(atomic_t)
>> +
>> +/*
>> + * State of all managed devices for a given job
>> + */
>> +struct v4l2_job {
>> +     struct kref refcount;
>> +     struct v4l2_jobqueue *jq;
>> +     /* node in v4l2_jobqueue's queued_jobs or completed_jobs */
>> +     struct list_head node;
>> +     /* global list of existing jobs for this queue */
>> +     struct list_head jobs_list;
>> +     /* mask of devices that completed this job */
>> +     atomic_t completed;
>> +     /* fd exported to user-space */
>> +     int fd;
>> +     enum v4l2_job_status status;
>> +
>> +     /* per-device states */
>> +     struct v4l2_job_state *state[0];
>> +};
>> +
>> +/*
>> + * A job queue manages the job flow for a given set of devices, applies their
>> + * state, and activates them in lockstep.
>> + *
>> + * A job goes through the following stages through its life:
>> + *
>> + * * current_job: the job has been created and is waiting to be queued. S_CTRL
>> + *   will apply to it. Once queued, it is pushed into
>> + * * queued_jobs: a queue of jobs to be processed in sequential order. The head
>> + *   of this list becomes the
>> + * * active_job: the job currently being processed by the hardware. Once
>> + *   completed, the next job in queued_job becomes active, and the previous
>> + *   active job goes into
>> + * * completed_jobs: a list of completed jobs waiting to be dequeued by
>> + *   user-space. As user-space called the DQJOB ioctl, the head becomes the
>> + * * dequeued_job: the job on which G_CTRL will be performed on. A job stays
>> + *   in this state until another one is dequeued, at which point it is deleted.
>> + */
>> +struct v4l2_jobqueue {
>> +     /* List of all jobs created for this queue, regardless of state */
>> +     struct list_head jobs_list;
>> +     /*
>> +      * Job that user-space is currently preparing, to be added to
>> +      * queued_jobs upon QJOB ioctl.
>> +      */
>> +     struct v4l2_job *current_job;
>> +
>> +     /* List of jobs that are ready to be processed */
>> +     struct list_head queued_jobs;
>> +
>> +     /* Job that is currently processed by the devices */
>> +     struct v4l2_job *active_job;
>
> Shouldn't this be a list as well? I interpret 'active job' as being a
> job that is passed to the various drivers that need to process it.
>
> Just as with video buffers where the hardware may need a minimum of
> buffers before it can start the DMA (min_buffers_needed), so the same
> is true for jobs. E.g. a driver may have to look ahead by a few frames
> to see what changes are requested. Some changes (esp. sensor related)
> can take a few frames before they take effect and the hardware has to
> be programmed ahead of time.

Regarding the number of queued frames, frames end up being queued into
the VB2 queue even if they are not passed to the driver, so I guess
the driver can still peek at forward frames through that mechanism.
Same can be done for the list of jobs themselves with the state change
listener.

> It would make more sense if this was a list of active jobs. It would
> likely also solve the TODOs you have in the code w.r.t. min_buffers_needed:
> after STREAMON you'd just queue the jobs to the active list, and also
> queue any associated buffers. Once the minimum number of buffers has been
> reached the DMA is started.

Mmm, but wouldn't that kind of defeat the idea of jobs as a single
unit of work if they needed to be grouped before being processed? I
need to think a bit more about that. Maybe we could pass the buffers
to the drivers prior to their job being scheduled, but I like the idea
of having VB2 managing the buffer's flow as it is easy to screw the
flow otherwise.

>> diff --git a/include/uapi/linux/v4l2-jobs.h b/include/uapi/linux/v4l2-jobs.h
>> new file mode 100644
>> index 000000000000..2cba4d20e62f
>> --- /dev/null
>> +++ b/include/uapi/linux/v4l2-jobs.h
>> @@ -0,0 +1,40 @@
>> +/*
>> + * V4L2 jobs API
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + */
>> +
>> +#ifndef __LINUX_V4L2_JOBS_H
>> +#define __LINUX_V4L2_JOBS_H
>> +
>> +#ifndef __KERNEL__
>> +#include <stdint.h>
>> +#endif
>> +#include <linux/ioctl.h>
>> +#include <linux/types.h>
>> +
>> +struct v4l2_jobqueue_init {
>> +     __u32 nb_devs;
>> +     __s32 *fd;
>> +};
>> +
>> +struct v4l2_jobqueue_job {
>> +     __s32 fd;
>> +};
>> +
>> +#define VIDIOC_JOBQUEUE_IOCTL_START  0x80
>
> Why this offset?

IIRC this is to avoid collision with other V4L2 ioctls that start at
offset 0. I have not thought this thoroughly however, just wanted to
have something that works for experimenting.

Thanks,
Alex.

Re: [RFC PATCH 0/9] V4L2 Jobs API WIP

[Alexandre Courbot]

Hi Hans,

On Mon, Oct 16, 2017 at 8:06 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> Hi Alexandre,
>
> Thank you very much for working on this. Much appreciated!

Thanks! I feel like there is still a long way before we get this right
though, so your guidance is highly appreciated! :)

> I only did a very high-level review of the patch series: there is not much
> point IMHO of doing a detailed review given the upcoming discussions in
> Prague. It's better to wait until we agree with the high-level API.

Agree on this.

> Regarding the public API: the ioctls seem sane. It's all very similar to the
> other implementations we've seen.
>
> I'm still not sure about the name 'job', but this is 'just' a naming issue.
>
> The part where I have more doubts is the need to create a new device node.
>
> For the upcoming meeting I would like to discuss whether this cannot be added
> to the media API.

Yeah, this device node is not exactly well-received. Although I wanted
to see what people would think about it (and am taking note of
everyone's doubts), its main purpose for now is to make my life easier
and the job API available no matter which driver I am working on. If
we move this to the media API, we need said media API activated, and
of course a media node, which is not a guarantee on every setup.

My point here is that although the current patchset does not reflect
this, the jobs API is also supposed to control media devices. In fact
it is supposed to allow control on a whole range of V4L2 features,
which is why I am hesitating tying it to an existing subsystem at the
moment. Also device and media nodes are currently created by
individual drivers. Jobs allow to control said devices together, thus
in my (maybe flawed!) logic it made more sense if they came from a
global node created by the framework.

Note that I am willing to be flexible on that, and the current setup
is mostly for my convenience as I develop this.

>
> Originally the plan was that the media API would be subsystem-agnostic and could
> also be used by ALSA/DRM/etc. This never happened and I also am not aware of any
> movement in that area.
>
> I am wondering whether we should just be realistic and abandon the 'subsystem
> agnostic' part and be willing to add e.g. the job support to the media API.
>
> We can also consider controlling sub-devices via the media device node instead
> of creating separate v4l-subdev device nodes.
>
> This does not necessarily preclude the use of the media device by other
> subsystems, but it certainly ties it much closer to the media subsystem. On the
> other hand, it does make life easier for us (and I like easy!).
>
> This is just a brainstorm at the moment, but I am interested what others think
> of making /dev/mediaX specific to the media subsystem (at least we chose the
> right name for that device node!).

V4L2 is still fairly new to me, so I am probably still confused, but
aren't /dev/mediaX nodes created by the media subsystem currently?

Cheers,
Alex.

>
> Obviously, if we go into that direction, then that will have an obvious impact
> on the jobs API, especially if we want to be able to control subdevs through the
> media device instead of through the v4l-subdev devices.
>
> Regards,
>
>         Hans
>
> On 09/28/2017 11:50 AM, Alexandre Courbot wrote:
>> Hi everyone,
>>
>> Here is a new attempt at the "request" (which I propose to rename "jobs") API
>> for V4L2, hopefully in a manner that can converge to something that will be
>> merged. The core ideas should be easy to grasp for those familiar with the
>> previous attemps, yet there are a few important differences.
>>
>> Most notably, user-space does not need to explicitly allocate and manage
>> requests/jobs (but still can if this makes sense). We noticed that only specific
>> use-cases require such an explicit management, and opted for a jobs queue that
>> controls the flow of work over a set of opened devices. This should simplify
>> user-space code quite a bit, while still retaining the ability to manage states
>> explicitly like the previous request API proposals allowed to do.
>>
>> The jobs API defines a few new concepts that user-space can use to control the
>> workflow on a set of opened V4L2 devices:
>>
>> A JOB QUEUE can be created from a set of opened FDs that are part of a pipeline
>> and need to cooperate (be it capture, m2m, or media controller devices).
>>
>> A JOB can then be set up with regular (if slightly modified) V4L2 ioctls, and
>> then submitted to the job queue. Once the job queue schedules the job, its
>> parameters (controls, etc) are applied to the devices of the queue, and itsd
>> buffers are processed. Immediately after a job is submitted, the next job is
>> ready to be set up without further user action.
>>
>> Once a job completes, it must be dequeued and user-space can then read back its
>> properties (notably controls) at completion time.
>>
>> Internally, the state of jobs is managed through STATE HANDLERS. Each driver
>> supporting the jobs API needs to specify an implementation of a state handler.
>> Fortunately, most drivers can rely on the generic state handler implementation
>> that simply records and replays a job's parameter using standard V4L2 functions.
>> Thanks to this, adding jobs API support to a driver relying on the control
>> framework and vb2 only requires a dozen lines of codes.
>>
>> Drivers with specific needs or opportunities for optimization can however
>> provide their own implementation of a state handler. This may in particular be
>> beneficial for hardware that supports configuration or command buffers (thinking
>> about VSP1 here).
>>
>> This is still very early work, and focus has been on the following points:
>>
>> * Provide something that anybody can test (currently using vim2m and vivid),
>> * Reuse the current V4L2 APIs as much as possible,
>> * Remain flexible enough to accomodate the inevitable changes that will be
>>   requested,
>> * Keep line count low, even if functionality is missing at the moment.
>>
>> Please keep this in mind while going through the patches. In particular, at the
>> moment the parameters of a job are limited to integer controls. I know that much
>> more is expected, but V4L2 has quite a learning curve and I preferred to focus
>> on the general concepts for now. More is coming though! :)
>>
>> I have written two small example programs that demonstrate the use of this API:
>>
>> - With a codec device (vim2m): https://gist.github.com/Gnurou/34c35f1f8e278dad454b51578d239a42
>>
>> - With a capture device (vivid): https://gist.github.com/Gnurou/5052e6ab41e7c55164b75d2970bc5a04
>>
>> Considering the history with the request API, I don't expect everything proposed
>> here to be welcome or understood immediately. In particular I apologize for not
>> reusing any of the previous attempts - I was just more comfortable laying down
>> my ideas from scratch.
>>
>> If this proposal is not dismissed as complete garbage I will also be happy to
>> discuss it in-person at the mini-summit in Prague. :)
>>
>> Cheers,
>> Alex.
>>
>> Alexandre Courbot (9):
>>   [media] v4l2-core: add v4l2_is_v4l2_file function
>>   [media] v4l2-core: add core jobs API support
>>   [media] videobuf2: add support for jobs API
>>   [media] v4l2-ctrls: add support for jobs API
>>   [media] v4l2-job: add generic jobs ops
>>   [media] m2m: add generic support for jobs API
>>   [media] vim2m: add jobs API support
>>   [media] vivid: add jobs API support for capture device
>>   [media] document jobs API
>>
>>  Documentation/media/intro.rst                      |   2 +
>>  Documentation/media/media_uapi.rst                 |   1 +
>>  Documentation/media/uapi/jobs/jobs-api.rst         |  23 +
>>  Documentation/media/uapi/jobs/jobs-example.rst     |  69 ++
>>  Documentation/media/uapi/jobs/jobs-intro.rst       |  61 ++
>>  Documentation/media/uapi/jobs/jobs-queue.rst       |  73 ++
>>  Documentation/media/uapi/jobs/jobs-queue.svg       | 192 ++++++
>>  .../media/uapi/v4l/vidioc-g-ext-ctrls.rst          |   6 +
>>  drivers/media/platform/vim2m.c                     |  24 +
>>  drivers/media/platform/vivid/vivid-core.c          |  16 +
>>  drivers/media/platform/vivid/vivid-core.h          |   2 +
>>  drivers/media/platform/vivid/vivid-kthread-cap.c   |   5 +
>>  drivers/media/v4l2-core/Makefile                   |   4 +-
>>  drivers/media/v4l2-core/v4l2-ctrls.c               |  50 +-
>>  drivers/media/v4l2-core/v4l2-dev.c                 |  12 +
>>  drivers/media/v4l2-core/v4l2-job-generic.c         | 394 +++++++++++
>>  drivers/media/v4l2-core/v4l2-jobqueue-dev.c        | 173 +++++
>>  drivers/media/v4l2-core/v4l2-jobqueue.c            | 764 +++++++++++++++++++++
>>  drivers/media/v4l2-core/v4l2-mem2mem.c             |  19 +
>>  drivers/media/v4l2-core/videobuf2-core.c           |  33 +-
>>  include/media/v4l2-ctrls.h                         |   6 +
>>  include/media/v4l2-dev.h                           |  13 +
>>  include/media/v4l2-fh.h                            |   4 +
>>  include/media/v4l2-job-generic.h                   |  47 ++
>>  include/media/v4l2-job-state.h                     |  75 ++
>>  include/media/v4l2-jobqueue-dev.h                  |  24 +
>>  include/media/v4l2-jobqueue.h                      |  54 ++
>>  include/media/v4l2-mem2mem.h                       |  11 +
>>  include/media/videobuf2-core.h                     |  16 +
>>  include/uapi/linux/v4l2-jobs.h                     |  40 ++
>>  include/uapi/linux/videodev2.h                     |   2 +
>>  31 files changed, 2205 insertions(+), 10 deletions(-)
>>  create mode 100644 Documentation/media/uapi/jobs/jobs-api.rst
>>  create mode 100644 Documentation/media/uapi/jobs/jobs-example.rst
>>  create mode 100644 Documentation/media/uapi/jobs/jobs-intro.rst
>>  create mode 100644 Documentation/media/uapi/jobs/jobs-queue.rst
>>  create mode 100644 Documentation/media/uapi/jobs/jobs-queue.svg
>>  create mode 100644 drivers/media/v4l2-core/v4l2-job-generic.c
>>  create mode 100644 drivers/media/v4l2-core/v4l2-jobqueue-dev.c
>>  create mode 100644 drivers/media/v4l2-core/v4l2-jobqueue.c
>>  create mode 100644 include/media/v4l2-job-generic.h
>>  create mode 100644 include/media/v4l2-job-state.h
>>  create mode 100644 include/media/v4l2-jobqueue-dev.h
>>  create mode 100644 include/media/v4l2-jobqueue.h
>>  create mode 100644 include/uapi/linux/v4l2-jobs.h
>>
>

Re: [RFC PATCH 0/9] V4L2 Jobs API WIP

[Alexandre Courbot]

Hi Sakari, thanks for the feedback!

On Thu, Oct 19, 2017 at 11:43 PM, Sakari Ailus <sakari.ailus@iki.fi> wrote:
> Hi Alexandre,
>
> On Thu, Sep 28, 2017 at 06:50:18PM +0900, Alexandre Courbot wrote:
>> Hi everyone,
>>
>> Here is a new attempt at the "request" (which I propose to rename "jobs") API
>> for V4L2, hopefully in a manner that can converge to something that will be
>> merged. The core ideas should be easy to grasp for those familiar with the
>> previous attemps, yet there are a few important differences.
>>
>> Most notably, user-space does not need to explicitly allocate and manage
>> requests/jobs (but still can if this makes sense). We noticed that only specific
>> use-cases require such an explicit management, and opted for a jobs queue that
>> controls the flow of work over a set of opened devices. This should simplify
>> user-space code quite a bit, while still retaining the ability to manage states
>> explicitly like the previous request API proposals allowed to do.
>>
>> The jobs API defines a few new concepts that user-space can use to control the
>> workflow on a set of opened V4L2 devices:
>>
>> A JOB QUEUE can be created from a set of opened FDs that are part of a pipeline
>> and need to cooperate (be it capture, m2m, or media controller devices).
>>
>> A JOB can then be set up with regular (if slightly modified) V4L2 ioctls, and
>> then submitted to the job queue. Once the job queue schedules the job, its
>> parameters (controls, etc) are applied to the devices of the queue, and itsd
>> buffers are processed. Immediately after a job is submitted, the next job is
>> ready to be set up without further user action.
>>
>> Once a job completes, it must be dequeued and user-space can then read back its
>> properties (notably controls) at completion time.
>>
>> Internally, the state of jobs is managed through STATE HANDLERS. Each driver
>> supporting the jobs API needs to specify an implementation of a state handler.
>> Fortunately, most drivers can rely on the generic state handler implementation
>> that simply records and replays a job's parameter using standard V4L2 functions.
>> Thanks to this, adding jobs API support to a driver relying on the control
>> framework and vb2 only requires a dozen lines of codes.
>>
>> Drivers with specific needs or opportunities for optimization can however
>> provide their own implementation of a state handler. This may in particular be
>> beneficial for hardware that supports configuration or command buffers (thinking
>> about VSP1 here).
>>
>> This is still very early work, and focus has been on the following points:
>>
>> * Provide something that anybody can test (currently using vim2m and vivid),
>> * Reuse the current V4L2 APIs as much as possible,
>> * Remain flexible enough to accomodate the inevitable changes that will be
>>   requested,
>> * Keep line count low, even if functionality is missing at the moment.
>>
>> Please keep this in mind while going through the patches. In particular, at the
>> moment the parameters of a job are limited to integer controls. I know that much
>> more is expected, but V4L2 has quite a learning curve and I preferred to focus
>> on the general concepts for now. More is coming though! :)
>>
>> I have written two small example programs that demonstrate the use of this API:
>>
>> - With a codec device (vim2m): https://gist.github.com/Gnurou/34c35f1f8e278dad454b51578d239a42
>>
>> - With a capture device (vivid): https://gist.github.com/Gnurou/5052e6ab41e7c55164b75d2970bc5a04
>>
>> Considering the history with the request API, I don't expect everything proposed
>> here to be welcome or understood immediately. In particular I apologize for not
>> reusing any of the previous attempts - I was just more comfortable laying down
>> my ideas from scratch.
>>
>> If this proposal is not dismissed as complete garbage I will also be happy to
>> discuss it in-person at the mini-summit in Prague. :)
>
> Thank you for the initiative and the patchset.
>
> While reviewing this patchset, I'm concentrating primarily on the approach
> taken and the design, not so much in the actual implementation which I
> don't think matters much at this moment.

Thanks, that is exactly how I hoped things would go for the moment.

>
> It's difficult to avoid seeing many similarities with the Request API
> patches posted earlier on. And not only that, rather you have to start
> looking for the differences in what I could call details, while important
> design decisions could sometimes be only visible in what appear details at
> this point.

I was not quite sure whether I should base this work on one of the
existing patchsets (and in this case, which one) or start from
scratch. This being my first contribution to a new area of the kernel
for me, I decided to start from scratch as it would yield more
educative value.

>
> Both request and jobs APIs have the concept of a request, or a job, which
> is created by the user and then different buffers or controls can be bound
> to that request. (Other configuration isn't really excluded but it's
> non-trivial to implement in practice.) This is common for both.

Yes, the main difference being that the current proposal manages the
jobs flow implicitly by default, to ease the most common uses of this
API (codecs & camera). It still maintains the ability to control them
more finely similarly to the previous request API proposals.

>
> The differences begin however how the functionality within the scope is
> actieved, in particular:
>
> - A new user space interface (character device) is created for the jobs API
>   whereas requests use existing Media controller interface. Therefore the
>   jobs API is specific to V4L2. Consequently, the V4L2 jobs API may not
>   support Media controller link state changes through requests.

It is not clear to me why that is the case - could you elaborate on that a bit?

>
>   I don't see a reason why it should be this way, and I also see no reason
>   why there should be yet another user space interface for this purpose
>   alone: this is a clear drawback compared to handling this through the
>   Media device.

Yeah, as I discussed in my reply to Hans, this node is mostly here for
convenience reasons and I don't feel too strongly about it.

>
> - Controls and buffers are always bound to requests explicitly whereas for
>   jobs, this seems to be implicit based on associating the file handle
>   related to the relevant video device with the request.
>
>   There are advantages in the approach, but currently I'd see that they're
>   primarily related to not having to change the existing IOCTL argument
>   structs.

For controls, this is definitely not the case (see the newly
introduced V4L2_CTRL_WHICH_CURJOB_VAL flag for instance). Even when
using the jobs API, you can change controls on-the-fly without waiting
for the current job to be processed.

For other parameters, we need to decide whether it can make sense to
decouple them from jobs. In particular, allowing buffers to be
processed both from and out of the jobs queue seems difficult to
achieve in a consistent way, and may not make any sense semantically
speaking. This is why the meaning of QBUF/QDBUF depends on whether the
jobs API is in use for a particular opened node.

>
>   There lie problems, too, because of this: with requests (or jobs) it is
>   vitally important that the user will always have the information if a
>   buffer, control event etc. was related to a request (or not), and in
>   particular which request it was.
>
>   You could say that the user must keep track of this information but I'd
>   suppose it won't make it easier for the user no having an important piece
>   of information. Having this information as part of the IOCTL argument
>   struct is mandatory for e.g. events that the user doesn't have an ID to
>   compare with in order to find the related request.

When queuing buffers, it is quite obvious which job they will be
linked too, as it is the current one. We can return a job ID as an
output argument of the QJOB ioctl to make this easier to handle.

I need to look at the ioctl structures too see what is possible, but I
also see no drawbacks to associating a job ID to dequeued buffers if
there is space remaining.

>
>   Also --- when an association with a video devnode file descriptor and a
>   job with a request is made, when does it cease to exist? When the job is
>   released? When the job is done?

Association is made between a job queue (to which an undefined number
of jobs can be queued) and a set of device nodes. A job queue remains
active as long as its file descriptor is not closed. So the short
answer to your question is that the devnode remains part of the queue
until the file descriptor obtained by opening /dev/v4l2_jobqueue (and
initialized using VIDIOC_JOBQUEUE_INIT) is opened. This is of course
subject to change if /dev/v4l2_jobqueue disappears, but I would like
to retain the idea of managing the jobs queue via its own file
descriptor.

>
> There are smaller differences, not very important IMO:
>
> - Requests are removed once complete. This was done to simplify the
>   implementation and it could be added if it is seen reasonable to
>   implement and useful, neither of which is known.
>
> These are differences, I'd say, in the parts that are somewhat manageable
> in any way they're designed, and with rather easily understandable
> consequences from these design decisions. I still prefer the design choices
> made for the Request API (regarding device nodes and request association
> especially).

I suppose this is something we need to discuss thoroughly in Prague to
make sure we understand why we made different design decisions. I am
rather fond of the idea of associating a set of opened devices into a
jobs queue and think that this is necessary for some advanced cases
(MIPI camera notably).

>
> The hard stuff, e.g. how do you implement including non-buffer and
> non-control configuration into the request in a meaningful way in an actual
> driver I haven't seen yet. We'd need one driver to implement that, and in
> general case it likely requires locking scheme changes in MC, for instance.
> There are still use cases where this all isn't needed so there is a
> motivation to have less than full implementation in the mainline kernel.

You're right - I wanted to give a go at the easiest part first and
receive feedback on this. Also it is easiest for us (Google) to
evaluate this first step as it allows us to replace the config store
currently used in Chrome OS.

>
> Another matter is making videobuf2 helpful here: we should have, if not in
> the videobuf2 framework itself, then around it helper function(s) to manage
> the submission of buffers to a driver. You can get things working pretty
> easily but the error handling is very painful: what do you do, for
> instance, with buffers queued with a request if queueing the request itself
> fails, possibly because the user hasn't provided enough buffers with the
> request? Mark the buffers errorneous and return them to the user? Probably
> so, but that requires the user to dequeue the buffers and gather the
> request again. I presume this would only happen in special circumstances
> though, and not typically in an application using requests. This, and many
> other special cases still must be handled by the kernel.

Error handling is still pretty weak in that version. I would like to
get an overall agreement on the general direction before looking at
this more closely though, as I suppose getting things right will take
some time.

>
> Finally, I want to say that I like some aspects of the patchset, such as
> moving more things to the V4L2 framework from the driver. This would be
> very useful in helping driver implementation: V4L2 is very stream-centric
> and the configurations and buffers across device nodes have been
> essentially independent from API point of view. Associating the pieces of
> information together in requests would be painful to do in drivers. What
> the framework can do, still controlled by drivers, could help a lot here.
> This aspect wasn't much considered where the Request API work was left.

I am not sure whether this is obvious in this patchset, but the idea
is that while there is a default implementation that can easily be
used as-is by most drivers (which allows to keep the lines count in
vivid and vim2m low), a driver with more specific needs could still
write its own state handler, tailored to its needs. I am especially
thinking of VSP1 here, Laurent's implementation was designed with the
perspective of writing and reusing command buffers, and the same
result should be attainable with a custom state handler (while still
benefiting from some of the framework code).

But whether you write a custom state handler or use the generic one,
the current implementation still depends on VB2/control framework,
which means that older drivers (and notably uvc) could not make use of
the jobs API. I am not sure whether this would be considered a
problem.

>
> Still it shouldn't be forgotten that if the framework is geared towards
> helping drivers "running one job at a time" the scope will be limited to
> memory-to-memory devices; streaming devices, e.g. all kinds of cameras have
> multiple requests being processed simultaneously (or frames are bound to be
> lost, something we can't allow to happen due to framework design). And I
> believe the memory-to-memory devices are generally the easy case. That
> could be one option to start with, but at the same time we have to make it
> absolutely certain we will not paint ourselves to the corner: the V4L2 UAPI
> (or even KAPI) paint will take much longer to dry than the regular one.

There are several reasons why the current patchset is focused on m2m
devices (although vivid can be considered a regular camera case):
- As you said, it is the easiest use-case to implement,
- It is also the use-case we are the most interested in for Chrome OS,
so I am clearly biased towards it, :)
- My exposure to V4L2 is still limited, so I may not be able to see
the whole picture yet.

It is important though, that we consider all the cases that need to be
supported by the jobs API and I want to make it very clear that I am
not attempting to direct it towards our specific use. It is not clear
to me why some cameras would need multiple requests to be processed
simultaneously (neither is it clear how I could implement that using
the current design), so we definitely need to discuss this in Prague.
The current patchset is just to try and validate the general
direction, and get confidence that this scheme can support all that
needs to be supported.

Oh, I have also updated it to complete controls support, and now all
kinds of controls should work. Besides that the other changes are
minor improvements on things that were very clumsy, so not resending
another patchset here, but in case someone wants to have a look it is
on https://github.com/Gnurou/linux/commits/v4l2_jobs.

Thanks and see you all in Prague! I should be there from Thursday.

Alex.

Re: [RFC PATCH 0/9] V4L2 Jobs API WIP

[Gustavo Padovan]

2017-10-16 Hans Verkuil <hverkuil@xs4all.nl>:

> Hi Alexandre,
> 
> Thank you very much for working on this. Much appreciated!
> 
> I only did a very high-level review of the patch series: there is not much
> point IMHO of doing a detailed review given the upcoming discussions in
> Prague. It's better to wait until we agree with the high-level API.
> 
> Regarding the public API: the ioctls seem sane. It's all very similar to the
> other implementations we've seen.
> 
> I'm still not sure about the name 'job', but this is 'just' a naming issue.
> 
> The part where I have more doubts is the need to create a new device node.
> 
> For the upcoming meeting I would like to discuss whether this cannot be added
> to the media API.
> 
> Originally the plan was that the media API would be subsystem-agnostic and could
> also be used by ALSA/DRM/etc. This never happened and I also am not aware of any
> movement in that area.
> 
> I am wondering whether we should just be realistic and abandon the 'subsystem
> agnostic' part and be willing to add e.g. the job support to the media API.

Stupid question here: is there any techinically possible way to support
it through the media API while being subsystem agnostic?

Gustavo

Re: [RFC PATCH 0/9] V4L2 Jobs API WIP

[Laurent Pinchart]

Hello,

On Monday, 23 October 2017 11:45:01 EEST Alexandre Courbot wrote:
> On Thu, Oct 19, 2017 at 11:43 PM, Sakari Ailus <sakari.ailus@iki.fi> wrote:
> > On Thu, Sep 28, 2017 at 06:50:18PM +0900, Alexandre Courbot wrote:
> >> Hi everyone,
> >> 
> >> Here is a new attempt at the "request" (which I propose to rename "jobs")
> >> API for V4L2, hopefully in a manner that can converge to something that
> >> will be merged. The core ideas should be easy to grasp for those
> >> familiar with the previous attemps, yet there are a few important
> >> differences.
> >> 
> >> Most notably, user-space does not need to explicitly allocate and manage
> >> requests/jobs (but still can if this makes sense). We noticed that only
> >> specific use-cases require such an explicit management, and opted for a
> >> jobs queue that controls the flow of work over a set of opened devices.
> >> This should simplify user-space code quite a bit, while still retaining
> >> the ability to manage states explicitly like the previous request API
> >> proposals allowed to do.
> >> 
> >> The jobs API defines a few new concepts that user-space can use to
> >> control the workflow on a set of opened V4L2 devices:
> >> 
> >> A JOB QUEUE can be created from a set of opened FDs that are part of a
> >> pipeline and need to cooperate (be it capture, m2m, or media controller
> >> devices).
> >> 
> >> A JOB can then be set up with regular (if slightly modified) V4L2 ioctls,
> >> and then submitted to the job queue. Once the job queue schedules the
> >> job, its parameters (controls, etc) are applied to the devices of the
> >> queue, and itsd buffers are processed. Immediately after a job is
> >> submitted, the next job is ready to be set up without further user
> >> action.
> >> 
> >> Once a job completes, it must be dequeued and user-space can then read
> >> back its properties (notably controls) at completion time.
> >> 
> >> Internally, the state of jobs is managed through STATE HANDLERS. Each
> >> driver supporting the jobs API needs to specify an implementation of a
> >> state handler. Fortunately, most drivers can rely on the generic state
> >> handler implementation that simply records and replays a job's parameter
> >> using standard V4L2 functions. Thanks to this, adding jobs API support
> >> to a driver relying on the control framework and vb2 only requires a
> >> dozen lines of codes.
> >> 
> >> Drivers with specific needs or opportunities for optimization can however
> >> provide their own implementation of a state handler. This may in
> >> particular be beneficial for hardware that supports configuration or
> >> command buffers (thinking about VSP1 here).
> >> 
> >> This is still very early work, and focus has been on the following
> >> points:
> >> 
> >> * Provide something that anybody can test (currently using vim2m and
> >> vivid),
> >> * Reuse the current V4L2 APIs as much as possible,
> >> * Remain flexible enough to accomodate the inevitable changes that will
> >> be requested,
> >> * Keep line count low, even if functionality is missing at the moment.
> >> 
> >> Please keep this in mind while going through the patches. In particular,
> >> at the moment the parameters of a job are limited to integer controls. I
> >> know that much more is expected, but V4L2 has quite a learning curve and
> >> I preferred to focus on the general concepts for now. More is coming
> >> though! :)
> >> 
> >> I have written two small example programs that demonstrate the use of
> >> this API:
> >> 
> >> - With a codec device (vim2m):
> >> https://gist.github.com/Gnurou/34c35f1f8e278dad454b51578d239a42
> >> 
> >> - With a capture device (vivid):
> >> https://gist.github.com/Gnurou/5052e6ab41e7c55164b75d2970bc5a04
> >> 
> >> Considering the history with the request API, I don't expect everything
> >> proposed here to be welcome or understood immediately. In particular I
> >> apologize for not reusing any of the previous attempts - I was just more
> >> comfortable laying down my ideas from scratch.
> >> 
> >> If this proposal is not dismissed as complete garbage I will also be
> >> happy to discuss it in-person at the mini-summit in Prague. :)
> > 
> > Thank you for the initiative and the patchset.
> > 
> > While reviewing this patchset, I'm concentrating primarily on the approach
> > taken and the design, not so much in the actual implementation which I
> > don't think matters much at this moment.
> 
> Thanks, that is exactly how I hoped things would go for the moment.
> 
> > It's difficult to avoid seeing many similarities with the Request API
> > patches posted earlier on. And not only that, rather you have to start
> > looking for the differences in what I could call details, while important
> > design decisions could sometimes be only visible in what appear details at
> > this point.
> 
> I was not quite sure whether I should base this work on one of the
> existing patchsets (and in this case, which one) or start from
> scratch. This being my first contribution to a new area of the kernel
> for me, I decided to start from scratch as it would yield more
> educative value.

What bothers me here is that we had a full day face to face meeting in Tokyo 
back in June about this, where you presented this idea of how the userspace 
API could look like. We went through the proposal point by point to discuss 
potential issues, and for most points I recall agreeing that the changes 
proposed compared to the previous request API RFC were introducing problems 
that couldn't be easily solved. I walked out of the meeting understanding we 
had an agreement to go back to an API quite similar to the previous RFC, in 
particular with explicit request object management from userspace, and I now 
see several months later an RFC that ignores all the conclusions of our 
meeting.

> > Both request and jobs APIs have the concept of a request, or a job, which
> > is created by the user and then different buffers or controls can be bound
> > to that request. (Other configuration isn't really excluded but it's
> > non-trivial to implement in practice.) This is common for both.
> 
> Yes, the main difference being that the current proposal manages the
> jobs flow implicitly by default, to ease the most common uses of this
> API (codecs & camera). It still maintains the ability to control them
> more finely similarly to the previous request API proposals.

Implicit job handling might make your codec use-cases simpler, but it does 
*not* ease camera support.

> > The differences begin however how the functionality within the scope is
> > actieved, in particular:
> > 
> > - A new user space interface (character device) is created for the jobs
> >   API whereas requests use existing Media controller interface. Therefore
> >   the jobs API is specific to V4L2. Consequently, the V4L2 jobs API may
> >   not support Media controller link state changes through requests.
> 
> It is not clear to me why that is the case - could you elaborate on that a
> bit?

I suppose it would be possible to support changing link state in a request if 
we add the MC fd to the job queue, at the price of more implicit state 
handling and I believe a complex implementation on the kernel side. 
Regardless, I see no reason at all to create a new device node for this when 
all operations can be performed on the media device node.

> >   I don't see a reason why it should be this way, and I also see no reason
> >   why there should be yet another user space interface for this purpose
> >   alone: this is a clear drawback compared to handling this through the
> >   Media device.
> 
> Yeah, as I discussed in my reply to Hans, this node is mostly here for
> convenience reasons and I don't feel too strongly about it.

To be honest I don't really understand how this device node increases 
convenience compared to using the media controller device node.

> > - Controls and buffers are always bound to requests explicitly whereas for
> >   jobs, this seems to be implicit based on associating the file handle
> >   related to the relevant video device with the request.
> >   
> >   There are advantages in the approach, but currently I'd see that they're
> >   primarily related to not having to change the existing IOCTL argument
> >   structs.
> 
> For controls, this is definitely not the case (see the newly
> introduced V4L2_CTRL_WHICH_CURJOB_VAL flag for instance). Even when
> using the jobs API, you can change controls on-the-fly without waiting
> for the current job to be processed.

Good, if we're not reluctant to add arguments to existing ioctls, then we can 
certainly pass a request ID :-)

> For other parameters, we need to decide whether it can make sense to
> decouple them from jobs. In particular, allowing buffers to be
> processed both from and out of the jobs queue seems difficult to
> achieve in a consistent way, and may not make any sense semantically
> speaking. This is why the meaning of QBUF/QDBUF depends on whether the
> jobs API is in use for a particular opened node.

I'll have to go through an in-depth analysis of the problem again to come up 
with useful examples (unfortunately we don't have notes of the Tokyo meeting), 
but I remember having discussed cases where buffers are queued as part of a 
request but dequeued separately. This doesn't seem to be supported with the 
proposed implicit API.

> >   There lie problems, too, because of this: with requests (or jobs) it is
> >   vitally important that the user will always have the information if a
> >   buffer, control event etc. was related to a request (or not), and in
> >   particular which request it was.
> >   
> >   You could say that the user must keep track of this information but I'd
> >   suppose it won't make it easier for the user no having an important
> >   piece of information.

I very much agree here. Userspace needs to maintain relationships between 
requests/jobs and parameters in any case. With an explicit API more hints will 
be give to userspace, easing the implementation and reducing the risk of 
getting it wrong.

> >   Having this information as part of the IOCTL argument struct is
> >   mandatory for e.g. events that the user doesn't have an ID to compare
> >   with in order to find the related request.
> 
> When queuing buffers, it is quite obvious which job they will be
> linked too, as it is the current one. We can return a job ID as an
> output argument of the QJOB ioctl to make this easier to handle.

I strongly believe we should expose job IDs to userspace, yes. And if we do, I 
see little reason to use an implicit API.

> I need to look at the ioctl structures too see what is possible, but I
> also see no drawbacks to associating a job ID to dequeued buffers if
> there is space remaining.

I believe there's space in most structures, as this is what my previous RFC 
was doing. We can also extend existing structures, for example v4l2_buffer 
will soon run out of reserved fields and needs to be reworked to support Y2038 
anyway.

> >   Also --- when an association with a video devnode file descriptor and a
> >   job with a request is made, when does it cease to exist? When the job is
> >   released? When the job is done?
> 
> Association is made between a job queue (to which an undefined number
> of jobs can be queued) and a set of device nodes. A job queue remains
> active as long as its file descriptor is not closed. So the short
> answer to your question is that the devnode remains part of the queue
> until the file descriptor obtained by opening /dev/v4l2_jobqueue (and
> initialized using VIDIOC_JOBQUEUE_INIT) is opened. This is of course
> subject to change if /dev/v4l2_jobqueue disappears, but I would like
> to retain the idea of managing the jobs queue via its own file
> descriptor.

This makes it quite inconvenient to change controls both through a request and 
directly, a userspace application would need to open subdevs and video nodes 
twice, keeping one "direct" fd and associating the other one with the queue. 
That's a complexity increase for userspace without any advantage in my 
opinion.

> > There are smaller differences, not very important IMO:
> > 
> > - Requests are removed once complete. This was done to simplify the
> >   implementation and it could be added if it is seen reasonable to
> >   implement and useful, neither of which is known.
> > 
> > These are differences, I'd say, in the parts that are somewhat manageable
> > in any way they're designed, and with rather easily understandable
> > consequences from these design decisions. I still prefer the design
> > choices made for the Request API (regarding device nodes and request
> > association especially).
> 
> I suppose this is something we need to discuss thoroughly in Prague to
> make sure we understand why we made different design decisions. I am
> rather fond of the idea of associating a set of opened devices into a
> jobs queue and think that this is necessary for some advanced cases
> (MIPI camera notably).

And I think it hinders that use case, for the reasons I've explained in Tokyo 
(if you manage to extract them from your meeting notes I'd be grateful ;-)). I 
don't see any advantage in an implicit API for requests/jobs. I would rather 
like to go fully the other way around and pass *all* configuration parameters 
(controls, formats, links, ...) through a single ioctl call when queueing (or 
possibly preparing, if we end up needed to split the preparation from queuing 
for a performance reason, but I don't think at the moment that there's a need 
for it) the request. Configuring a pipeline through subdev nodes is painful, 
regardless of whether we use requests/jobs or not, and some operations are 
just not possible (such a propagating the test configuration of multiplexed 
streams for instance, which would in my opinion require a device-wide test 
context to be implemented properly, but that's not upstream yet).

> > The hard stuff, e.g. how do you implement including non-buffer and
> > non-control configuration into the request in a meaningful way in an
> > actual driver I haven't seen yet. We'd need one driver to implement that,
> > and in general case it likely requires locking scheme changes in MC, for
> > instance. There are still use cases where this all isn't needed so there
> > is a motivation to have less than full implementation in the mainline
> > kernel.
> 
> You're right - I wanted to give a go at the easiest part first and
> receive feedback on this. Also it is easiest for us (Google) to
> evaluate this first step as it allows us to replace the config store
> currently used in Chrome OS.

I'm not completely opposed to getting a partial implementation upstream as a 
first step, but only if we can guarantee it won't hinder the full 
implementation. I haven't seen any such guarantee at the moment, quite the 
contrary.

> > Another matter is making videobuf2 helpful here: we should have, if not in
> > the videobuf2 framework itself, then around it helper function(s) to
> > manage the submission of buffers to a driver. You can get things working
> > pretty easily but the error handling is very painful: what do you do, for
> > instance, with buffers queued with a request if queueing the request
> > itself fails, possibly because the user hasn't provided enough buffers
> > with the request? Mark the buffers errorneous and return them to the user?
> > Probably so, but that requires the user to dequeue the buffers and gather
> > the request again. I presume this would only happen in special
> > circumstances though, and not typically in an application using requests.
> > This, and many other special cases still must be handled by the kernel.
> 
> Error handling is still pretty weak in that version. I would like to
> get an overall agreement on the general direction before looking at
> this more closely though, as I suppose getting things right will take
> some time.

I believe error handling would be much simpler if we passed all request 
parameters through a single ioctl, as we would have one clear point where to 
perform validation with all required information available.

> > Finally, I want to say that I like some aspects of the patchset, such as
> > moving more things to the V4L2 framework from the driver. This would be
> > very useful in helping driver implementation: V4L2 is very stream-centric
> > and the configurations and buffers across device nodes have been
> > essentially independent from API point of view. Associating the pieces of
> > information together in requests would be painful to do in drivers. What
> > the framework can do, still controlled by drivers, could help a lot here.
> > This aspect wasn't much considered where the Request API work was left.
> 
> I am not sure whether this is obvious in this patchset, but the idea
> is that while there is a default implementation that can easily be
> used as-is by most drivers (which allows to keep the lines count in
> vivid and vim2m low), a driver with more specific needs could still
> write its own state handler, tailored to its needs. I am especially
> thinking of VSP1 here, Laurent's implementation was designed with the
> perspective of writing and reusing command buffers, and the same
> result should be attainable with a custom state handler (while still
> benefiting from some of the framework code).

To make my opinion as clear as possible, I would like to see the main entry 
point for the request API being implemented by all drivers. We could implement 
helper functions that translate a request to individual ioctl calls, but I'm 
more and more convinced that we shouldn't. Instead we should do it the other 
way around, make the request API a first class citizen, and provide helper 
functions to translate the V4L2 API to requests (how complicated that will be 
remains to be seen). Implementing legacy APIs on top of new ones is akin to 
V4L1 support on top of V4L2 driver that we have implemented in libv4l, and 
userspace is where I believe such a compatibility layer belongs (assuming we 
need one at all).

> But whether you write a custom state handler or use the generic one,
> the current implementation still depends on VB2/control framework,
> which means that older drivers (and notably uvc) could not make use of
> the jobs API. I am not sure whether this would be considered a
> problem.

UVC isn't the main target of this API, as the UVC protocol doesn't provide a 
way to apply changes atomically.

As for dependencies on the control framework, I believe I've already exposed 
my opinion that the control framework should be refactored to split control 
values from control static data. The former would be used as part of the 
device state, and the latter could be used as part of control validation 
(possibly with some parts written specifically for requests).

> > Still it shouldn't be forgotten that if the framework is geared towards
> > helping drivers "running one job at a time" the scope will be limited to
> > memory-to-memory devices; streaming devices, e.g. all kinds of cameras
> > have multiple requests being processed simultaneously (or frames are bound
> > to be lost, something we can't allow to happen due to framework design).
> > And I believe the memory-to-memory devices are generally the easy case.
> > That could be one option to start with, but at the same time we have to
> > make it absolutely certain we will not paint ourselves to the corner: the
> > V4L2 UAPI (or even KAPI) paint will take much longer to dry than the
> > regular one.
> 
> There are several reasons why the current patchset is focused on m2m
> devices (although vivid can be considered a regular camera case):
> - As you said, it is the easiest use-case to implement,
> - It is also the use-case we are the most interested in for Chrome OS,
> so I am clearly biased towards it, :)
> - My exposure to V4L2 is still limited, so I may not be able to see
> the whole picture yet.

It's good we'll meet to discuss this then :-)

> It is important though, that we consider all the cases that need to be
> supported by the jobs API and I want to make it very clear that I am
> not attempting to direct it towards our specific use. It is not clear
> to me why some cameras would need multiple requests to be processed
> simultaneously (neither is it clear how I could implement that using
> the current design), so we definitely need to discuss this in Prague.
> The current patchset is just to try and validate the general
> direction, and get confidence that this scheme can support all that
> needs to be supported.
> 
> Oh, I have also updated it to complete controls support, and now all
> kinds of controls should work. Besides that the other changes are
> minor improvements on things that were very clumsy, so not resending
> another patchset here, but in case someone wants to have a look it is
> on https://github.com/Gnurou/linux/commits/v4l2_jobs.
> 
> Thanks and see you all in Prague! I should be there from Thursday.

-- 
Regards,

Laurent Pinchart

Re: [RFC PATCH 0/9] V4L2 Jobs API WIP

[Hans Verkuil]

On 10/25/2017 05:48 PM, Laurent Pinchart wrote:
> Hello,
> 
> On Monday, 23 October 2017 11:45:01 EEST Alexandre Courbot wrote:
>> On Thu, Oct 19, 2017 at 11:43 PM, Sakari Ailus <sakari.ailus@iki.fi> wrote:
>>> On Thu, Sep 28, 2017 at 06:50:18PM +0900, Alexandre Courbot wrote:
>>>> Hi everyone,
>>>>
>>>> Here is a new attempt at the "request" (which I propose to rename "jobs")
>>>> API for V4L2, hopefully in a manner that can converge to something that
>>>> will be merged. The core ideas should be easy to grasp for those
>>>> familiar with the previous attemps, yet there are a few important
>>>> differences.
>>>>
>>>> Most notably, user-space does not need to explicitly allocate and manage
>>>> requests/jobs (but still can if this makes sense). We noticed that only
>>>> specific use-cases require such an explicit management, and opted for a
>>>> jobs queue that controls the flow of work over a set of opened devices.
>>>> This should simplify user-space code quite a bit, while still retaining
>>>> the ability to manage states explicitly like the previous request API
>>>> proposals allowed to do.
>>>>
>>>> The jobs API defines a few new concepts that user-space can use to
>>>> control the workflow on a set of opened V4L2 devices:
>>>>
>>>> A JOB QUEUE can be created from a set of opened FDs that are part of a
>>>> pipeline and need to cooperate (be it capture, m2m, or media controller
>>>> devices).
>>>>
>>>> A JOB can then be set up with regular (if slightly modified) V4L2 ioctls,
>>>> and then submitted to the job queue. Once the job queue schedules the
>>>> job, its parameters (controls, etc) are applied to the devices of the
>>>> queue, and itsd buffers are processed. Immediately after a job is
>>>> submitted, the next job is ready to be set up without further user
>>>> action.
>>>>
>>>> Once a job completes, it must be dequeued and user-space can then read
>>>> back its properties (notably controls) at completion time.
>>>>
>>>> Internally, the state of jobs is managed through STATE HANDLERS. Each
>>>> driver supporting the jobs API needs to specify an implementation of a
>>>> state handler. Fortunately, most drivers can rely on the generic state
>>>> handler implementation that simply records and replays a job's parameter
>>>> using standard V4L2 functions. Thanks to this, adding jobs API support
>>>> to a driver relying on the control framework and vb2 only requires a
>>>> dozen lines of codes.
>>>>
>>>> Drivers with specific needs or opportunities for optimization can however
>>>> provide their own implementation of a state handler. This may in
>>>> particular be beneficial for hardware that supports configuration or
>>>> command buffers (thinking about VSP1 here).
>>>>
>>>> This is still very early work, and focus has been on the following
>>>> points:
>>>>
>>>> * Provide something that anybody can test (currently using vim2m and
>>>> vivid),
>>>> * Reuse the current V4L2 APIs as much as possible,
>>>> * Remain flexible enough to accomodate the inevitable changes that will
>>>> be requested,
>>>> * Keep line count low, even if functionality is missing at the moment.
>>>>
>>>> Please keep this in mind while going through the patches. In particular,
>>>> at the moment the parameters of a job are limited to integer controls. I
>>>> know that much more is expected, but V4L2 has quite a learning curve and
>>>> I preferred to focus on the general concepts for now. More is coming
>>>> though! :)
>>>>
>>>> I have written two small example programs that demonstrate the use of
>>>> this API:
>>>>
>>>> - With a codec device (vim2m):
>>>> https://gist.github.com/Gnurou/34c35f1f8e278dad454b51578d239a42
>>>>
>>>> - With a capture device (vivid):
>>>> https://gist.github.com/Gnurou/5052e6ab41e7c55164b75d2970bc5a04
>>>>
>>>> Considering the history with the request API, I don't expect everything
>>>> proposed here to be welcome or understood immediately. In particular I
>>>> apologize for not reusing any of the previous attempts - I was just more
>>>> comfortable laying down my ideas from scratch.
>>>>
>>>> If this proposal is not dismissed as complete garbage I will also be
>>>> happy to discuss it in-person at the mini-summit in Prague. :)
>>>
>>> Thank you for the initiative and the patchset.
>>>
>>> While reviewing this patchset, I'm concentrating primarily on the approach
>>> taken and the design, not so much in the actual implementation which I
>>> don't think matters much at this moment.
>>
>> Thanks, that is exactly how I hoped things would go for the moment.
>>
>>> It's difficult to avoid seeing many similarities with the Request API
>>> patches posted earlier on. And not only that, rather you have to start
>>> looking for the differences in what I could call details, while important
>>> design decisions could sometimes be only visible in what appear details at
>>> this point.
>>
>> I was not quite sure whether I should base this work on one of the
>> existing patchsets (and in this case, which one) or start from
>> scratch. This being my first contribution to a new area of the kernel
>> for me, I decided to start from scratch as it would yield more
>> educative value.
> 
> What bothers me here is that we had a full day face to face meeting in Tokyo 
> back in June about this, where you presented this idea of how the userspace 
> API could look like. We went through the proposal point by point to discuss 
> potential issues, and for most points I recall agreeing that the changes 
> proposed compared to the previous request API RFC were introducing problems 
> that couldn't be easily solved. I walked out of the meeting understanding we 
> had an agreement to go back to an API quite similar to the previous RFC, in 
> particular with explicit request object management from userspace, and I now 
> see several months later an RFC that ignores all the conclusions of our 
> meeting.

Laurent, can you give a link to that RFC? Just so we all refer to the same
request API RFC. That will help with the discussion tomorrow.

Two notes: my hope is that by the end of Friday we have a public API that is
good enough for the codec use case and can be extended for the more generic
use case. We have codec drivers waiting to be mainlined for years now that are
blocked because of this. It's getting ridiculous. That also means that I don't
care all that much about the kernel API: ideally it will be suitable or
extensible for the generic use case, but if it isn't and needs to be reworked
substantially for the generic use case, then so be it. Obviously this is something
I hope we can avoid, but it would be much worse if the codec vendors would move
away from V4L2 and start using other suboptimal solutions just because we
can't reach an agreement.

As I said in the beginning: we don't have that option for the public API: that
does need some careful thought as we cannot change that later, we can only extend it.

Second note (just to throw it in the discussion): I've always thought that
controls are a good way to store state, including things like formats.

Having a single framework taking care of that would simplify things substantially.
With VIDIOC_S_EXT_CTRLS you can already set a large number of controls in one
ioctl.

This does require some (or a lot?) refactoring of the control framework as you rightly
mentioned in your email, but I'm willing to do that work.

Regards,

	Hans

Re: [RFC PATCH 0/9] V4L2 Jobs API WIP

[Laurent Pinchart]

Hi Hans,

On Wednesday, 25 October 2017 19:19:27 EEST Hans Verkuil wrote:
> On 10/25/2017 05:48 PM, Laurent Pinchart wrote:
> > Hello,
> > 
> > On Monday, 23 October 2017 11:45:01 EEST Alexandre Courbot wrote:
> >> On Thu, Oct 19, 2017 at 11:43 PM, Sakari Ailus <sakari.ailus@iki.fi> 
wrote:
> >>> On Thu, Sep 28, 2017 at 06:50:18PM +0900, Alexandre Courbot wrote:
> >>>> Hi everyone,
> >>>> 
> >>>> Here is a new attempt at the "request" (which I propose to rename
> >>>> "jobs")
> >>>> API for V4L2, hopefully in a manner that can converge to something that
> >>>> will be merged. The core ideas should be easy to grasp for those
> >>>> familiar with the previous attemps, yet there are a few important
> >>>> differences.
> >>>> 
> >>>> Most notably, user-space does not need to explicitly allocate and
> >>>> manage
> >>>> requests/jobs (but still can if this makes sense). We noticed that only
> >>>> specific use-cases require such an explicit management, and opted for a
> >>>> jobs queue that controls the flow of work over a set of opened devices.
> >>>> This should simplify user-space code quite a bit, while still retaining
> >>>> the ability to manage states explicitly like the previous request API
> >>>> proposals allowed to do.
> >>>> 
> >>>> The jobs API defines a few new concepts that user-space can use to
> >>>> control the workflow on a set of opened V4L2 devices:
> >>>> 
> >>>> A JOB QUEUE can be created from a set of opened FDs that are part of a
> >>>> pipeline and need to cooperate (be it capture, m2m, or media controller
> >>>> devices).
> >>>> 
> >>>> A JOB can then be set up with regular (if slightly modified) V4L2
> >>>> ioctls,
> >>>> and then submitted to the job queue. Once the job queue schedules the
> >>>> job, its parameters (controls, etc) are applied to the devices of the
> >>>> queue, and itsd buffers are processed. Immediately after a job is
> >>>> submitted, the next job is ready to be set up without further user
> >>>> action.
> >>>> 
> >>>> Once a job completes, it must be dequeued and user-space can then read
> >>>> back its properties (notably controls) at completion time.
> >>>> 
> >>>> Internally, the state of jobs is managed through STATE HANDLERS. Each
> >>>> driver supporting the jobs API needs to specify an implementation of a
> >>>> state handler. Fortunately, most drivers can rely on the generic state
> >>>> handler implementation that simply records and replays a job's
> >>>> parameter
> >>>> using standard V4L2 functions. Thanks to this, adding jobs API support
> >>>> to a driver relying on the control framework and vb2 only requires a
> >>>> dozen lines of codes.
> >>>> 
> >>>> Drivers with specific needs or opportunities for optimization can
> >>>> however
> >>>> provide their own implementation of a state handler. This may in
> >>>> particular be beneficial for hardware that supports configuration or
> >>>> command buffers (thinking about VSP1 here).
> >>>> 
> >>>> This is still very early work, and focus has been on the following
> >>>> points:
> >>>> 
> >>>> * Provide something that anybody can test (currently using vim2m and
> >>>> vivid),
> >>>> * Reuse the current V4L2 APIs as much as possible,
> >>>> * Remain flexible enough to accomodate the inevitable changes that will
> >>>> be requested,
> >>>> * Keep line count low, even if functionality is missing at the moment.
> >>>> 
> >>>> Please keep this in mind while going through the patches. In
> >>>> particular,
> >>>> at the moment the parameters of a job are limited to integer controls.
> >>>> I
> >>>> know that much more is expected, but V4L2 has quite a learning curve
> >>>> and
> >>>> I preferred to focus on the general concepts for now. More is coming
> >>>> though! :)
> >>>> 
> >>>> I have written two small example programs that demonstrate the use of
> >>>> this API:
> >>>> 
> >>>> - With a codec device (vim2m):
> >>>> https://gist.github.com/Gnurou/34c35f1f8e278dad454b51578d239a42
> >>>> 
> >>>> - With a capture device (vivid):
> >>>> https://gist.github.com/Gnurou/5052e6ab41e7c55164b75d2970bc5a04
> >>>> 
> >>>> Considering the history with the request API, I don't expect everything
> >>>> proposed here to be welcome or understood immediately. In particular I
> >>>> apologize for not reusing any of the previous attempts - I was just
> >>>> more
> >>>> comfortable laying down my ideas from scratch.
> >>>> 
> >>>> If this proposal is not dismissed as complete garbage I will also be
> >>>> happy to discuss it in-person at the mini-summit in Prague. :)
> >>> 
> >>> Thank you for the initiative and the patchset.
> >>> 
> >>> While reviewing this patchset, I'm concentrating primarily on the
> >>> approach
> >>> taken and the design, not so much in the actual implementation which I
> >>> don't think matters much at this moment.
> >> 
> >> Thanks, that is exactly how I hoped things would go for the moment.
> >> 
> >>> It's difficult to avoid seeing many similarities with the Request API
> >>> patches posted earlier on. And not only that, rather you have to start
> >>> looking for the differences in what I could call details, while
> >>> important
> >>> design decisions could sometimes be only visible in what appear details
> >>> at
> >>> this point.
> >> 
> >> I was not quite sure whether I should base this work on one of the
> >> existing patchsets (and in this case, which one) or start from
> >> scratch. This being my first contribution to a new area of the kernel
> >> for me, I decided to start from scratch as it would yield more
> >> educative value.
> > 
> > What bothers me here is that we had a full day face to face meeting in
> > Tokyo back in June about this, where you presented this idea of how the
> > userspace API could look like. We went through the proposal point by
> > point to discuss potential issues, and for most points I recall agreeing
> > that the changes proposed compared to the previous request API RFC were
> > introducing problems that couldn't be easily solved. I walked out of the
> > meeting understanding we had an agreement to go back to an API quite
> > similar to the previous RFC, in particular with explicit request object
> > management from userspace, and I now see several months later an RFC that
> > ignores all the conclusions of our meeting.
> 
> Laurent, can you give a link to that RFC? Just so we all refer to the same
> request API RFC. That will help with the discussion tomorrow.

Sure, it's available at https://www.spinics.net/lists/linux-sh/msg48289.html. 
Sakari has posted two newer versions based on that original series.

> Two notes: my hope is that by the end of Friday we have a public API that is
> good enough for the codec use case and can be extended for the more generic
> use case. We have codec drivers waiting to be mainlined for years now that
> are blocked because of this. It's getting ridiculous. That also means that
> I don't care all that much about the kernel API: ideally it will be
> suitable or extensible for the generic use case, but if it isn't and needs
> to be reworked substantially for the generic use case, then so be it.
> Obviously this is something I hope we can avoid, but it would be much worse
> if the codec vendors would move away from V4L2 and start using other
> suboptimal solutions just because we can't reach an agreement.
> 
> As I said in the beginning: we don't have that option for the public API:
> that does need some careful thought as we cannot change that later, we can
> only extend it.

I agree with your overall, we need to move forward on this. Focussing on the 
userspace API first is the way to go according to me as well. I wouldn't say 
that I don't care about the kernel API, but that's indeed easier to change. 
I'd like to share your optimism about what we will achieve by the end of 
Friday, and I would certainly love to be proven wrong when I'm pessimistic :) 
Let's try our best.

> Second note (just to throw it in the discussion): I've always thought that
> controls are a good way to store state, including things like formats.
> 
> Having a single framework taking care of that would simplify things
> substantially. With VIDIOC_S_EXT_CTRLS you can already set a large number
> of controls in one ioctl.

To some extent I agree with you, but I don't think we should base the API on 
VIDIOC_S_EXT_CTRLS, nor the implementation on the control framework as it 
exists today. That's a topic to be discussed tomorrow :-)

> This does require some (or a lot?) refactoring of the control framework as
> you rightly mentioned in your email, but I'm willing to do that work.

Thank you.

-- 
Regards,

Laurent Pinchart

Re: [RFC PATCH 0/9] V4L2 Jobs API WIP

[Tomasz Figa]

Hi Sakari,

On Thu, Oct 19, 2017 at 11:43 PM, Sakari Ailus <sakari.ailus@iki.fi> wrote:
> Hi Alexandre,
>
> On Thu, Sep 28, 2017 at 06:50:18PM +0900, Alexandre Courbot wrote:
>> Hi everyone,
>>
[snip]
>
> Still it shouldn't be forgotten that if the framework is geared towards
> helping drivers "running one job at a time" the scope will be limited to
> memory-to-memory devices; streaming devices, e.g. all kinds of cameras have
> multiple requests being processed simultaneously (or frames are bound to be
> lost, something we can't allow to happen due to framework design).

I'd be interested in some further explanation of these multiple
requests being processed simultaneously and why they couldn't be
included in a single job.

Best regards,
Tomasz

Re: [RFC PATCH 0/9] V4L2 Jobs API WIP

[Tomasz Figa]

Hi Laurent,

On Thu, Oct 26, 2017 at 12:48 AM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hello,
>
> On Monday, 23 October 2017 11:45:01 EEST Alexandre Courbot wrote:
>> On Thu, Oct 19, 2017 at 11:43 PM, Sakari Ailus <sakari.ailus@iki.fi> wrote:
>> > On Thu, Sep 28, 2017 at 06:50:18PM +0900, Alexandre Courbot wrote:
[snip]
>> > Both request and jobs APIs have the concept of a request, or a job, which
>> > is created by the user and then different buffers or controls can be bound
>> > to that request. (Other configuration isn't really excluded but it's
>> > non-trivial to implement in practice.) This is common for both.
>>
>> Yes, the main difference being that the current proposal manages the
>> jobs flow implicitly by default, to ease the most common uses of this
>> API (codecs & camera). It still maintains the ability to control them
>> more finely similarly to the previous request API proposals.
>
> Implicit job handling might make your codec use-cases simpler, but it does
> *not* ease camera support.

I'd appreciate some example use cases for explicit job handling that
would benefit the camera.

[snip]
>> >   Also --- when an association with a video devnode file descriptor and a
>> >   job with a request is made, when does it cease to exist? When the job is
>> >   released? When the job is done?
>>
>> Association is made between a job queue (to which an undefined number
>> of jobs can be queued) and a set of device nodes. A job queue remains
>> active as long as its file descriptor is not closed. So the short
>> answer to your question is that the devnode remains part of the queue
>> until the file descriptor obtained by opening /dev/v4l2_jobqueue (and
>> initialized using VIDIOC_JOBQUEUE_INIT) is opened. This is of course
>> subject to change if /dev/v4l2_jobqueue disappears, but I would like
>> to retain the idea of managing the jobs queue via its own file
>> descriptor.
>
> This makes it quite inconvenient to change controls both through a request and
> directly, a userspace application would need to open subdevs and video nodes
> twice, keeping one "direct" fd and associating the other one with the queue.
> That's a complexity increase for userspace without any advantage in my
> opinion.

There is the .which field within v4l2_ext_controls struct, which could
be used to determine whether the operation applies to a request or
directly.

[snip]
>> > Another matter is making videobuf2 helpful here: we should have, if not in
>> > the videobuf2 framework itself, then around it helper function(s) to
>> > manage the submission of buffers to a driver. You can get things working
>> > pretty easily but the error handling is very painful: what do you do, for
>> > instance, with buffers queued with a request if queueing the request
>> > itself fails, possibly because the user hasn't provided enough buffers
>> > with the request? Mark the buffers errorneous and return them to the user?
>> > Probably so, but that requires the user to dequeue the buffers and gather
>> > the request again. I presume this would only happen in special
>> > circumstances though, and not typically in an application using requests.
>> > This, and many other special cases still must be handled by the kernel.
>>
>> Error handling is still pretty weak in that version. I would like to
>> get an overall agreement on the general direction before looking at
>> this more closely though, as I suppose getting things right will take
>> some time.
>
> I believe error handling would be much simpler if we passed all request
> parameters through a single ioctl, as we would have one clear point where to
> perform validation with all required information available.

Compared to fine granularity of particular ioctls, this would make
determining the error cause more difficult or at least would require
providing a special interface to userspace just to communicate the
exact reason of the failure.

Best regards,
Tomasz

Re: [RFC PATCH 0/9] V4L2 Jobs API WIP

[Tomasz Figa]