[PATCH v2 00/17] media: atomisp: Convert to videobuf2

[PATCH v2 00/17] media: atomisp: Convert to videobuf2

[Hans de Goede]

Hi All,

Here is v2 of my patch series converting the staging atomisp driver to use
the videobuf2 framework, fixing MMAP mode not working.

New in v2 is that the "media: atomisp: Convert to videobuf2" patch
now also contains moving over to using a per vb_queue lock as is
standard for v4l2 drivers. This removes a bunch of FIXME + checkpatch
warnings (due to commented out prep for this) from v1 of this patch.
This v2 also fixes the 1 new warning pointed out by the lkp test robot.

For some more background info here are the still relevant bits of
the v1 cover-letter:

This also contains an important fix to try_fmt handling, so that
the various supported output formats can actually be used by userspace
which calls try_fmt before doing s_fmt.

So slowly this is starting to look more and more like a standard
v4l2 device (with all the complex pipeline handling hidden in the driver,
moving that to userspace seems to be impossible with this design since
multiple pipeline steps are handled by a single firmware program).

Regards,

Hans


Hans de Goede (17):
  media: atomisp: Add hmm_create_from_vmalloc_buf() function
  media: atomisp: Add ia_css_frame_init_from_info() function
  media: atomisp: Make atomisp_q_video_buffers_to_css() static
  media: atomisp: On streamoff wait for buffers owned by the CSS to be
    given back
  media: atomisp: Remove unused atomisp_buffers_queued[_pipe] functions
  media: atomisp: Also track buffers in a list when submitted to the ISP
  media: atomisp: Add an index helper variable to atomisp_buf_done()
  media: atomisp: Use new atomisp_flush_video_pipe() helper in
    atomisp_streamoff()
  media: atomisp: Add ia_css_frame_get_info() helper
  media: atomisp: Convert to videobuf2
  media: atomisp: Make it possible to call atomisp_set_fmt() without a
    file handle
  media: atomisp: Fix VIDIOC_REQBUFS failing when VIDIOC_S_FMT has not
    been called yet
  media: atomisp: Refactor atomisp_adjust_fmt()
  media: atomisp: Fix atomisp_try_fmt_cap() always returning YUV420
    pixelformat
  media: atomisp: Make atomisp_g_fmt_cap() default to YUV420
  media: atomisp: Remove __atomisp_get_pipe() helper
  media: atomisp: gc0310: Power on sensor from set_fmt() callback

 .../media/atomisp/i2c/atomisp-gc0310.c        |  14 +-
 drivers/staging/media/atomisp/i2c/gc0310.h    |   1 +
 .../staging/media/atomisp/include/hmm/hmm.h   |   2 +
 .../media/atomisp/include/hmm/hmm_bo.h        |   4 +-
 .../staging/media/atomisp/pci/atomisp_cmd.c   | 353 ++++--------
 .../staging/media/atomisp/pci/atomisp_cmd.h   |   9 +-
 .../media/atomisp/pci/atomisp_common.h        |   6 +-
 .../media/atomisp/pci/atomisp_compat.h        |   3 +-
 .../media/atomisp/pci/atomisp_compat_css20.c  |   6 +-
 .../staging/media/atomisp/pci/atomisp_fops.c  | 528 ++++++-----------
 .../staging/media/atomisp/pci/atomisp_fops.h  |  13 -
 .../staging/media/atomisp/pci/atomisp_ioctl.c | 536 ++++--------------
 .../staging/media/atomisp/pci/atomisp_ioctl.h |  10 +-
 .../media/atomisp/pci/atomisp_subdev.c        |   2 +
 .../media/atomisp/pci/atomisp_subdev.h        |  22 +-
 drivers/staging/media/atomisp/pci/hmm/hmm.c   |  15 +-
 .../staging/media/atomisp/pci/hmm/hmm_bo.c    |  32 +-
 .../media/atomisp/pci/ia_css_frame_public.h   |  36 +-
 .../bayer_io_ls/ia_css_bayer_io.host.c        |  10 +-
 .../yuv444_io_ls/ia_css_yuv444_io.host.c      |  10 +-
 .../isp/kernels/ref/ref_1.0/ia_css_ref.host.c |   2 +-
 .../isp/kernels/tnr/tnr_1.0/ia_css_tnr.host.c |   4 +-
 .../pci/runtime/debug/src/ia_css_debug.c      |  26 +-
 .../atomisp/pci/runtime/frame/src/frame.c     |  82 +--
 drivers/staging/media/atomisp/pci/sh_css.c    |  38 +-
 .../staging/media/atomisp/pci/sh_css_params.c |  17 +-
 drivers/staging/media/atomisp/pci/sh_css_sp.c |  52 +-
 27 files changed, 640 insertions(+), 1193 deletions(-)

-- 
2.37.3

[PATCH v2 01/17] media: atomisp: Add hmm_create_from_vmalloc_buf() function

[Hans de Goede]

Add a new hmm creating function to create a vmm object from a vmalloc-ed
kernel buffer. This is a preparation patch for adding videobuf2 (and
working MMAP mode) support.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 .../staging/media/atomisp/include/hmm/hmm.h   |  2 ++
 .../media/atomisp/include/hmm/hmm_bo.h        |  4 ++-
 drivers/staging/media/atomisp/pci/hmm/hmm.c   | 15 ++++++---
 .../staging/media/atomisp/pci/hmm/hmm_bo.c    | 32 +++++++++++++++----
 4 files changed, 42 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/media/atomisp/include/hmm/hmm.h b/drivers/staging/media/atomisp/include/hmm/hmm.h
index c0384bb0a762..b81b8580d405 100644
--- a/drivers/staging/media/atomisp/include/hmm/hmm.h
+++ b/drivers/staging/media/atomisp/include/hmm/hmm.h
@@ -38,6 +38,8 @@ void hmm_cleanup(void);
 
 ia_css_ptr hmm_alloc(size_t bytes);
 ia_css_ptr hmm_create_from_userdata(size_t bytes, const void __user *userptr);
+ia_css_ptr hmm_create_from_vmalloc_buf(size_t bytes, void *vmalloc_addr);
+
 void hmm_free(ia_css_ptr ptr);
 int hmm_load(ia_css_ptr virt, void *data, unsigned int bytes);
 int hmm_store(ia_css_ptr virt, const void *data, unsigned int bytes);
diff --git a/drivers/staging/media/atomisp/include/hmm/hmm_bo.h b/drivers/staging/media/atomisp/include/hmm/hmm_bo.h
index c5cbae1d9cf9..a51d89f0b5cc 100644
--- a/drivers/staging/media/atomisp/include/hmm/hmm_bo.h
+++ b/drivers/staging/media/atomisp/include/hmm/hmm_bo.h
@@ -73,6 +73,7 @@
 
 enum hmm_bo_type {
 	HMM_BO_PRIVATE,
+	HMM_BO_VMALLOC,
 	HMM_BO_USER,
 	HMM_BO_LAST,
 };
@@ -207,7 +208,8 @@ int hmm_bo_allocated(struct hmm_buffer_object *bo);
  */
 int hmm_bo_alloc_pages(struct hmm_buffer_object *bo,
 		       enum hmm_bo_type type,
-		       const void __user *userptr);
+		       const void __user *userptr,
+		       void *vmalloc_addr);
 void hmm_bo_free_pages(struct hmm_buffer_object *bo);
 int hmm_bo_page_allocated(struct hmm_buffer_object *bo);
 
diff --git a/drivers/staging/media/atomisp/pci/hmm/hmm.c b/drivers/staging/media/atomisp/pci/hmm/hmm.c
index fc6cfe9f7744..207a834e37bf 100644
--- a/drivers/staging/media/atomisp/pci/hmm/hmm.c
+++ b/drivers/staging/media/atomisp/pci/hmm/hmm.c
@@ -168,7 +168,9 @@ void hmm_cleanup(void)
 	hmm_initialized = false;
 }
 
-static ia_css_ptr __hmm_alloc(size_t bytes, enum hmm_bo_type type, const void __user *userptr)
+static ia_css_ptr __hmm_alloc(size_t bytes, enum hmm_bo_type type,
+			      const void __user *userptr,
+			      void *vmalloc_addr)
 {
 	unsigned int pgnr;
 	struct hmm_buffer_object *bo;
@@ -192,7 +194,7 @@ static ia_css_ptr __hmm_alloc(size_t bytes, enum hmm_bo_type type, const void __
 	}
 
 	/* Allocate pages for memory */
-	ret = hmm_bo_alloc_pages(bo, type, userptr);
+	ret = hmm_bo_alloc_pages(bo, type, userptr, vmalloc_addr);
 	if (ret) {
 		dev_err(atomisp_dev, "hmm_bo_alloc_pages failed.\n");
 		goto alloc_page_err;
@@ -221,12 +223,17 @@ static ia_css_ptr __hmm_alloc(size_t bytes, enum hmm_bo_type type, const void __
 
 ia_css_ptr hmm_alloc(size_t bytes)
 {
-	return __hmm_alloc(bytes, HMM_BO_PRIVATE, NULL);
+	return __hmm_alloc(bytes, HMM_BO_PRIVATE, NULL, NULL);
+}
+
+ia_css_ptr hmm_create_from_vmalloc_buf(size_t bytes, void *vmalloc_addr)
+{
+	return __hmm_alloc(bytes, HMM_BO_VMALLOC, NULL, vmalloc_addr);
 }
 
 ia_css_ptr hmm_create_from_userdata(size_t bytes, const void __user *userptr)
 {
-	return __hmm_alloc(bytes, HMM_BO_USER, userptr);
+	return __hmm_alloc(bytes, HMM_BO_USER, userptr, NULL);
 }
 
 void hmm_free(ia_css_ptr virt)
diff --git a/drivers/staging/media/atomisp/pci/hmm/hmm_bo.c b/drivers/staging/media/atomisp/pci/hmm/hmm_bo.c
index a5fd6d38d3c4..465ba837f2ed 100644
--- a/drivers/staging/media/atomisp/pci/hmm/hmm_bo.c
+++ b/drivers/staging/media/atomisp/pci/hmm/hmm_bo.c
@@ -694,18 +694,38 @@ static int alloc_user_pages(struct hmm_buffer_object *bo,
 	return -ENOMEM;
 }
 
+static int alloc_vmalloc_pages(struct hmm_buffer_object *bo, void *vmalloc_addr)
+{
+	void *vaddr = vmalloc_addr;
+	int i;
+
+	for (i = 0; i < bo->pgnr; i++) {
+		bo->pages[i] = vmalloc_to_page(vaddr);
+		if (!bo->pages[i]) {
+			dev_err(atomisp_dev, "Error could not get page %d of vmalloc buf\n", i);
+			return -ENOMEM;
+		}
+		vaddr += PAGE_SIZE;
+	}
+
+	return 0;
+}
+
 /*
  * allocate/free physical pages for the bo.
  *
  * type indicate where are the pages from. currently we have 3 types
- * of memory: HMM_BO_PRIVATE, HMM_BO_USER.
+ * of memory: HMM_BO_PRIVATE, HMM_BO_VMALLOC, HMM_BO_USER.
+ *
+ * vmalloc_addr is only valid when type is HMM_BO_VMALLOC.
  *
  * userptr is only valid when type is HMM_BO_USER, it indicates
  * the start address from user space task.
  */
 int hmm_bo_alloc_pages(struct hmm_buffer_object *bo,
 		       enum hmm_bo_type type,
-		       const void __user *userptr)
+		       const void __user *userptr,
+		       void *vmalloc_addr)
 {
 	int ret = -EINVAL;
 
@@ -720,12 +740,10 @@ int hmm_bo_alloc_pages(struct hmm_buffer_object *bo,
 		goto alloc_err;
 	}
 
-	/*
-	 * TO DO:
-	 * add HMM_BO_USER type
-	 */
 	if (type == HMM_BO_PRIVATE) {
 		ret = alloc_private_pages(bo);
+	} else if (type == HMM_BO_VMALLOC) {
+		ret = alloc_vmalloc_pages(bo, vmalloc_addr);
 	} else if (type == HMM_BO_USER) {
 		ret = alloc_user_pages(bo, userptr);
 	} else {
@@ -771,6 +789,8 @@ void hmm_bo_free_pages(struct hmm_buffer_object *bo)
 
 	if (bo->type == HMM_BO_PRIVATE)
 		free_private_bo_pages(bo);
+	else if (bo->type == HMM_BO_VMALLOC)
+		; /* No-op, nothing to do */
 	else if (bo->type == HMM_BO_USER)
 		free_user_pages(bo, bo->pgnr);
 	else
-- 
2.37.3

[PATCH v2 02/17] media: atomisp: Add ia_css_frame_init_from_info() function

[Hans de Goede]

Add a function to initialize (rather then alloc/create) a
ia_css_frame struct based on an ia_css_frame_info struct.

This is a preparation patch for adding videobuf2 support.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 .../media/atomisp/pci/ia_css_frame_public.h      | 11 +++++++++++
 .../media/atomisp/pci/runtime/frame/src/frame.c  | 16 ++++++++++++++++
 2 files changed, 27 insertions(+)

diff --git a/drivers/staging/media/atomisp/pci/ia_css_frame_public.h b/drivers/staging/media/atomisp/pci/ia_css_frame_public.h
index 514d933f934d..5727bd175330 100644
--- a/drivers/staging/media/atomisp/pci/ia_css_frame_public.h
+++ b/drivers/staging/media/atomisp/pci/ia_css_frame_public.h
@@ -220,6 +220,17 @@ ia_css_frame_allocate(struct ia_css_frame **frame,
 		      unsigned int stride,
 		      unsigned int raw_bit_depth);
 
+/* @brief Initialize a CSS frame structure using a frame info structure.
+ *
+ * @param	frame	The allocated frame.
+ * @param[in]	info	The frame info structure.
+ * @return		The error code.
+ *
+ * Initialize a frame using the resolution and format from a frame info struct.
+ */
+int ia_css_frame_init_from_info(struct ia_css_frame *frame,
+				const struct ia_css_frame_info *info);
+
 /* @brief Allocate a CSS frame structure using a frame info structure.
  *
  * @param	frame	The allocated frame.
diff --git a/drivers/staging/media/atomisp/pci/runtime/frame/src/frame.c b/drivers/staging/media/atomisp/pci/runtime/frame/src/frame.c
index 5a7058320ee6..c20a4527c842 100644
--- a/drivers/staging/media/atomisp/pci/runtime/frame/src/frame.c
+++ b/drivers/staging/media/atomisp/pci/runtime/frame/src/frame.c
@@ -847,3 +847,19 @@ void ia_css_resolution_to_sp_resolution(
 	to->width  = (uint16_t)from->width;
 	to->height = (uint16_t)from->height;
 }
+
+int ia_css_frame_init_from_info(struct ia_css_frame *frame,
+				const struct ia_css_frame_info *frame_info)
+{
+	frame->info.res.width = frame_info->res.width;
+	frame->info.res.height = frame_info->res.height;
+	frame->info.format = frame_info->format;
+	frame->info.padded_width = frame_info->padded_width;
+	frame->info.raw_bit_depth = frame_info->raw_bit_depth;
+	frame->valid = true;
+	/* To indicate it is not valid frame. */
+	frame->dynamic_queue_id = SH_CSS_INVALID_QUEUE_ID;
+	frame->buf_type = IA_CSS_BUFFER_TYPE_INVALID;
+
+	return ia_css_frame_init_planes(frame);
+}
-- 
2.37.3

[PATCH v2 03/17] media: atomisp: Make atomisp_q_video_buffers_to_css() static

[Hans de Goede]

atomisp_q_video_buffers_to_css() is only used insidd atomisp_fops.c, make
it static.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/staging/media/atomisp/pci/atomisp_fops.c | 10 +++++-----
 drivers/staging/media/atomisp/pci/atomisp_fops.h |  6 ------
 2 files changed, 5 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp_fops.c b/drivers/staging/media/atomisp/pci/atomisp_fops.c
index 84a84e0cdeef..d47b7f19125f 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_fops.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_fops.c
@@ -206,11 +206,11 @@ static int atomisp_q_one_dis_buffer(struct atomisp_sub_device *asd,
 	return 0;
 }
 
-int atomisp_q_video_buffers_to_css(struct atomisp_sub_device *asd,
-				   struct atomisp_video_pipe *pipe,
-				   enum atomisp_input_stream_id stream_id,
-				   enum ia_css_buffer_type css_buf_type,
-				   enum ia_css_pipe_id css_pipe_id)
+static int atomisp_q_video_buffers_to_css(struct atomisp_sub_device *asd,
+					  struct atomisp_video_pipe *pipe,
+					  enum atomisp_input_stream_id stream_id,
+					  enum ia_css_buffer_type css_buf_type,
+					  enum ia_css_pipe_id css_pipe_id)
 {
 	struct videobuf_vmalloc_memory *vm_mem;
 	struct atomisp_css_params_with_list *param;
diff --git a/drivers/staging/media/atomisp/pci/atomisp_fops.h b/drivers/staging/media/atomisp/pci/atomisp_fops.h
index 3f1e442ba782..8d3ea33a7d9a 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_fops.h
+++ b/drivers/staging/media/atomisp/pci/atomisp_fops.h
@@ -22,12 +22,6 @@
 #define	__ATOMISP_FOPS_H__
 #include "atomisp_subdev.h"
 
-int atomisp_q_video_buffers_to_css(struct atomisp_sub_device *asd,
-				   struct atomisp_video_pipe *pipe,
-				   enum atomisp_input_stream_id stream_id,
-				   enum ia_css_buffer_type css_buf_type,
-				   enum ia_css_pipe_id css_pipe_id);
-
 unsigned int atomisp_dev_users(struct atomisp_device *isp);
 unsigned int atomisp_sub_dev_users(struct atomisp_sub_device *asd);
 
-- 
2.37.3

[PATCH v2 04/17] media: atomisp: On streamoff wait for buffers owned by the CSS to be given back

[Hans de Goede]

There is no guarantee that when we stop the pipeline all buffers owned
by the CSS are cleanly returned to the videobuf queue.

This is a problem with videobuf2 which will complain loudly when not
all buffers have been returned after the streamoff() queue op has
returned.

And this also allows moving a WARN() in the continuous mode path.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 .../staging/media/atomisp/pci/atomisp_fops.c  |  3 ++
 .../staging/media/atomisp/pci/atomisp_ioctl.c | 39 ++++++++-----------
 .../media/atomisp/pci/atomisp_subdev.h        |  3 +-
 3 files changed, 21 insertions(+), 24 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp_fops.c b/drivers/staging/media/atomisp/pci/atomisp_fops.c
index d47b7f19125f..3b833cd5b423 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_fops.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_fops.c
@@ -222,6 +222,9 @@ static int atomisp_q_video_buffers_to_css(struct atomisp_sub_device *asd,
 	if (WARN_ON(css_pipe_id >= IA_CSS_PIPE_ID_NUM))
 		return -EINVAL;
 
+	if (pipe->stopping)
+		return -EINVAL;
+
 	while (pipe->buffers_in_css < ATOMISP_CSS_Q_DEPTH) {
 		struct videobuf_buffer *vb;
 
diff --git a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
index aefa7c07242a..bf5249b0d3bd 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
@@ -1754,6 +1754,22 @@ int atomisp_streamoff(struct file *file, void *fh, enum v4l2_buf_type type)
 		return -EINVAL;
 	}
 
+	/*
+	 * There is no guarantee that the buffers queued to / owned by the ISP
+	 * will properly be returned to the queue when stopping. Set a flag to
+	 * avoid new buffers getting queued and then wait for all the current
+	 * buffers to finish.
+	 */
+	pipe->stopping = true;
+	mutex_unlock(&isp->mutex);
+	/* wait max 1 second */
+	ret = wait_event_interruptible_timeout(pipe->capq.wait,
+					       pipe->buffers_in_css == 0, HZ);
+	mutex_lock(&isp->mutex);
+	pipe->stopping = false;
+	if (ret <= 0)
+		return ret ? ret : -ETIME;
+
 	/*
 	 * do only videobuf_streamoff for capture & vf pipes in
 	 * case of continuous capture
@@ -1768,29 +1784,6 @@ int atomisp_streamoff(struct file *file, void *fh, enum v4l2_buf_type type)
 								      0, 0, 0);
 			atomisp_freq_scaling(isp, ATOMISP_DFS_MODE_AUTO, false);
 		}
-		/*
-		 * Currently there is no way to flush buffers queued to css.
-		 * When doing videobuf_streamoff, active buffers will be
-		 * marked as VIDEOBUF_NEEDS_INIT. HAL will be able to use
-		 * these buffers again, and these buffers might be queued to
-		 * css more than once! Warn here, if HAL has not dequeued all
-		 * buffers back before calling streamoff.
-		 */
-		if (pipe->buffers_in_css != 0) {
-			WARN(1, "%s: buffers of vdev %s still in CSS!\n",
-			     __func__, pipe->vdev.name);
-
-			/*
-			 * Buffers remained in css maybe dequeued out in the
-			 * next stream on, while this will causes serious
-			 * issues as buffers already get invalid after
-			 * previous stream off.
-			 *
-			 * No way to flush buffers but to reset the whole css
-			 */
-			dev_warn(isp->dev, "Reset CSS to clean up css buffers.\n");
-			atomisp_css_flush(isp);
-		}
 
 		return videobuf_streamoff(&pipe->capq);
 	}
diff --git a/drivers/staging/media/atomisp/pci/atomisp_subdev.h b/drivers/staging/media/atomisp/pci/atomisp_subdev.h
index a1f4da35235d..65c2f8664f9d 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_subdev.h
+++ b/drivers/staging/media/atomisp/pci/atomisp_subdev.h
@@ -81,7 +81,8 @@ struct atomisp_video_pipe {
 
 	/* Store here the initial run mode */
 	unsigned int default_run_mode;
-
+	/* Set from streamoff to disallow queuing further buffers in CSS */
+	bool stopping;
 	unsigned int buffers_in_css;
 
 	/*
-- 
2.37.3

[PATCH v2 05/17] media: atomisp: Remove unused atomisp_buffers_queued[_pipe] functions

[Hans de Goede]

The atomisp_buffers_queued[_pipe] functions are not used anywhere,
remove them.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/staging/media/atomisp/pci/atomisp_cmd.c | 15 ---------------
 drivers/staging/media/atomisp/pci/atomisp_cmd.h |  4 ----
 2 files changed, 19 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp_cmd.c b/drivers/staging/media/atomisp/pci/atomisp_cmd.c
index c72d0e344671..0d3a4ff99730 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_cmd.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_cmd.c
@@ -640,21 +640,6 @@ void atomisp_clear_css_buffer_counters(struct atomisp_sub_device *asd)
 	asd->video_out_video_capture.buffers_in_css = 0;
 }
 
-/* ISP2400 */
-bool atomisp_buffers_queued(struct atomisp_sub_device *asd)
-{
-	return asd->video_out_capture.buffers_in_css ||
-	       asd->video_out_vf.buffers_in_css ||
-	       asd->video_out_preview.buffers_in_css ||
-	       asd->video_out_video_capture.buffers_in_css;
-}
-
-/* ISP2401 */
-bool atomisp_buffers_queued_pipe(struct atomisp_video_pipe *pipe)
-{
-	return pipe->buffers_in_css ? true : false;
-}
-
 /* 0x100000 is the start of dmem inside SP */
 #define SP_DMEM_BASE	0x100000
 
diff --git a/drivers/staging/media/atomisp/pci/atomisp_cmd.h b/drivers/staging/media/atomisp/pci/atomisp_cmd.h
index c9f92f1326b6..fc1cfda718e1 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_cmd.h
+++ b/drivers/staging/media/atomisp/pci/atomisp_cmd.h
@@ -57,10 +57,6 @@ struct atomisp_video_pipe *atomisp_to_video_pipe(struct video_device *dev);
 int atomisp_reset(struct atomisp_device *isp);
 void atomisp_flush_bufs_and_wakeup(struct atomisp_sub_device *asd);
 void atomisp_clear_css_buffer_counters(struct atomisp_sub_device *asd);
-/* ISP2400 */
-bool atomisp_buffers_queued(struct atomisp_sub_device *asd);
-/* ISP2401 */
-bool atomisp_buffers_queued_pipe(struct atomisp_video_pipe *pipe);
 
 /* Interrupt functions */
 void atomisp_msi_irq_init(struct atomisp_device *isp);
-- 
2.37.3

[PATCH v2 06/17] media: atomisp: Also track buffers in a list when submitted to the ISP

[Hans de Goede]

Instead of using an integer to keep count of how many buffers have
been handed over to the ISP (buffers_in_css) move buffers handed
over to the ISP to a new buffers_in_css list_head so that we can
easily loop over them.

This removes the need for atomisp_flush_video_pipe() to loop over
all buffers and then (ab)use the state to figure out if they
were handed over to the ISP.

Since the buffers are now always on a list when owned by the driver
this also allows the buffer_done path on flush vs normal completion
to be unified (both now need a list_del()) and this common code can
now be factored out into a new atomisp_buffer_done() helper.

This is a preparation patch for moving the driver over to
the videobuf2 framework.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 .../staging/media/atomisp/pci/atomisp_cmd.c   | 91 ++++++++++---------
 .../staging/media/atomisp/pci/atomisp_cmd.h   |  4 +
 .../staging/media/atomisp/pci/atomisp_fops.c  | 24 ++---
 .../staging/media/atomisp/pci/atomisp_ioctl.c |  2 +-
 .../media/atomisp/pci/atomisp_subdev.c        |  1 +
 .../media/atomisp/pci/atomisp_subdev.h        |  4 +-
 6 files changed, 72 insertions(+), 54 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp_cmd.c b/drivers/staging/media/atomisp/pci/atomisp_cmd.c
index 0d3a4ff99730..5f0bebefcadd 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_cmd.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_cmd.c
@@ -634,10 +634,6 @@ void atomisp_clear_css_buffer_counters(struct atomisp_sub_device *asd)
 		memset(asd->metadata_bufs_in_css[i], 0,
 		       sizeof(asd->metadata_bufs_in_css[i]));
 	asd->dis_bufs_in_css = 0;
-	asd->video_out_capture.buffers_in_css = 0;
-	asd->video_out_vf.buffers_in_css = 0;
-	asd->video_out_preview.buffers_in_css = 0;
-	asd->video_out_video_capture.buffers_in_css = 0;
 }
 
 /* 0x100000 is the start of dmem inside SP */
@@ -683,40 +679,65 @@ static struct videobuf_buffer *atomisp_css_frame_to_vbuf(
 	return NULL;
 }
 
-static void atomisp_flush_video_pipe(struct atomisp_sub_device *asd,
-				     struct atomisp_video_pipe *pipe)
+int atomisp_buffers_in_css(struct atomisp_video_pipe *pipe)
 {
 	unsigned long irqflags;
-	int i;
+	struct list_head *pos;
+	int buffers_in_css = 0;
 
-	if (!pipe->users)
-		return;
+	spin_lock_irqsave(&pipe->irq_lock, irqflags);
 
-	for (i = 0; pipe->capq.bufs[i]; i++) {
-		spin_lock_irqsave(&pipe->irq_lock, irqflags);
-		if (pipe->capq.bufs[i]->state == VIDEOBUF_ACTIVE ||
-		    pipe->capq.bufs[i]->state == VIDEOBUF_QUEUED) {
-			pipe->capq.bufs[i]->ts = ktime_get_ns();
-			pipe->capq.bufs[i]->field_count =
-			    atomic_read(&asd->sequence) << 1;
-			dev_dbg(asd->isp->dev, "release buffers on device %s\n",
-				pipe->vdev.name);
-			if (pipe->capq.bufs[i]->state == VIDEOBUF_QUEUED)
-				list_del_init(&pipe->capq.bufs[i]->queue);
-			pipe->capq.bufs[i]->state = VIDEOBUF_ERROR;
-			wake_up(&pipe->capq.bufs[i]->done);
-		}
-		spin_unlock_irqrestore(&pipe->irq_lock, irqflags);
+	list_for_each(pos, &pipe->buffers_in_css)
+		buffers_in_css++;
+
+	spin_unlock_irqrestore(&pipe->irq_lock, irqflags);
+
+	return buffers_in_css;
+}
+
+void atomisp_buffer_done(struct atomisp_video_pipe *pipe, struct videobuf_buffer *vb,
+			 int state)
+{
+	lockdep_assert_held(&pipe->irq_lock);
+
+	vb->ts = ktime_get_ns();
+	vb->field_count = atomic_read(&pipe->asd->sequence) << 1;
+	vb->state = state;
+	list_del(&vb->queue);
+	wake_up(&vb->done);
+}
+
+void atomisp_flush_video_pipe(struct atomisp_video_pipe *pipe, bool warn_on_css_frames)
+{
+	struct videobuf_buffer *_vb, *vb;
+	unsigned long irqflags;
+
+	spin_lock_irqsave(&pipe->irq_lock, irqflags);
+
+	list_for_each_entry_safe(vb, _vb, &pipe->buffers_in_css, queue) {
+		if (warn_on_css_frames)
+			dev_warn(pipe->isp->dev, "Warning: CSS frames queued on flush\n");
+		atomisp_buffer_done(pipe, vb, VIDEOBUF_ERROR);
+	}
+
+	list_for_each_entry_safe(vb, _vb, &pipe->activeq, queue)
+		atomisp_buffer_done(pipe, vb, VIDEOBUF_ERROR);
+
+	list_for_each_entry_safe(vb, _vb, &pipe->buffers_waiting_for_param, queue) {
+		pipe->frame_request_config_id[vb->i] = 0;
+		atomisp_buffer_done(pipe, vb, VIDEOBUF_ERROR);
 	}
+
+	spin_unlock_irqrestore(&pipe->irq_lock, irqflags);
 }
 
 /* Returns queued buffers back to video-core */
 void atomisp_flush_bufs_and_wakeup(struct atomisp_sub_device *asd)
 {
-	atomisp_flush_video_pipe(asd, &asd->video_out_capture);
-	atomisp_flush_video_pipe(asd, &asd->video_out_vf);
-	atomisp_flush_video_pipe(asd, &asd->video_out_preview);
-	atomisp_flush_video_pipe(asd, &asd->video_out_video_capture);
+	atomisp_flush_video_pipe(&asd->video_out_capture, false);
+	atomisp_flush_video_pipe(&asd->video_out_vf, false);
+	atomisp_flush_video_pipe(&asd->video_out_preview, false);
+	atomisp_flush_video_pipe(&asd->video_out_video_capture, false);
 }
 
 /* clean out the parameters that did not apply */
@@ -974,7 +995,6 @@ void atomisp_buf_done(struct atomisp_sub_device *asd, int error,
 		break;
 	case IA_CSS_BUFFER_TYPE_VF_OUTPUT_FRAME:
 	case IA_CSS_BUFFER_TYPE_SEC_VF_OUTPUT_FRAME:
-		pipe->buffers_in_css--;
 		frame = buffer.css_buffer.data.frame;
 		if (!frame) {
 			WARN_ON(1);
@@ -1026,7 +1046,6 @@ void atomisp_buf_done(struct atomisp_sub_device *asd, int error,
 		break;
 	case IA_CSS_BUFFER_TYPE_OUTPUT_FRAME:
 	case IA_CSS_BUFFER_TYPE_SEC_OUTPUT_FRAME:
-		pipe->buffers_in_css--;
 		frame = buffer.css_buffer.data.frame;
 		if (!frame) {
 			WARN_ON(1);
@@ -1179,19 +1198,9 @@ void atomisp_buf_done(struct atomisp_sub_device *asd, int error,
 		break;
 	}
 	if (vb) {
-		vb->ts = ktime_get_ns();
-		vb->field_count = atomic_read(&asd->sequence) << 1;
-		/*mark videobuffer done for dequeue*/
 		spin_lock_irqsave(&pipe->irq_lock, irqflags);
-		vb->state = !error ? VIDEOBUF_DONE : VIDEOBUF_ERROR;
+		atomisp_buffer_done(pipe, vb, error ? VIDEOBUF_ERROR : VIDEOBUF_DONE);
 		spin_unlock_irqrestore(&pipe->irq_lock, irqflags);
-
-		/*
-		 * Frame capture done, wake up any process block on
-		 * current active buffer
-		 * possibly hold by videobuf_dqbuf()
-		 */
-		wake_up(&vb->done);
 	}
 
 	/*
diff --git a/drivers/staging/media/atomisp/pci/atomisp_cmd.h b/drivers/staging/media/atomisp/pci/atomisp_cmd.h
index fc1cfda718e1..1b46f4e60924 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_cmd.h
+++ b/drivers/staging/media/atomisp/pci/atomisp_cmd.h
@@ -55,6 +55,10 @@ void dump_sp_dmem(struct atomisp_device *isp, unsigned int addr,
 struct camera_mipi_info *atomisp_to_sensor_mipi_info(struct v4l2_subdev *sd);
 struct atomisp_video_pipe *atomisp_to_video_pipe(struct video_device *dev);
 int atomisp_reset(struct atomisp_device *isp);
+int atomisp_buffers_in_css(struct atomisp_video_pipe *pipe);
+void atomisp_buffer_done(struct atomisp_video_pipe *pipe, struct videobuf_buffer *buf,
+			 int state);
+void atomisp_flush_video_pipe(struct atomisp_video_pipe *pipe, bool warn_on_css_frames);
 void atomisp_flush_bufs_and_wakeup(struct atomisp_sub_device *asd);
 void atomisp_clear_css_buffer_counters(struct atomisp_sub_device *asd);
 
diff --git a/drivers/staging/media/atomisp/pci/atomisp_fops.c b/drivers/staging/media/atomisp/pci/atomisp_fops.c
index 3b833cd5b423..ac9aa8649635 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_fops.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_fops.c
@@ -217,7 +217,9 @@ static int atomisp_q_video_buffers_to_css(struct atomisp_sub_device *asd,
 	struct ia_css_dvs_grid_info *dvs_grid =
 	    atomisp_css_get_dvs_grid_info(&asd->params.curr_grid_info);
 	unsigned long irqflags;
-	int err = 0;
+	int space, err = 0;
+
+	lockdep_assert_held(&asd->isp->mutex);
 
 	if (WARN_ON(css_pipe_id >= IA_CSS_PIPE_ID_NUM))
 		return -EINVAL;
@@ -225,20 +227,21 @@ static int atomisp_q_video_buffers_to_css(struct atomisp_sub_device *asd,
 	if (pipe->stopping)
 		return -EINVAL;
 
-	while (pipe->buffers_in_css < ATOMISP_CSS_Q_DEPTH) {
+	space = ATOMISP_CSS_Q_DEPTH - atomisp_buffers_in_css(pipe);
+	while (space--) {
 		struct videobuf_buffer *vb;
 
 		spin_lock_irqsave(&pipe->irq_lock, irqflags);
-		if (list_empty(&pipe->activeq)) {
-			spin_unlock_irqrestore(&pipe->irq_lock, irqflags);
-			return -EINVAL;
+		vb = list_first_entry_or_null(&pipe->activeq, struct videobuf_buffer, queue);
+		if (vb) {
+			list_move_tail(&vb->queue, &pipe->buffers_in_css);
+			vb->state = VIDEOBUF_ACTIVE;
 		}
-		vb = list_entry(pipe->activeq.next,
-				struct videobuf_buffer, queue);
-		list_del_init(&vb->queue);
-		vb->state = VIDEOBUF_ACTIVE;
 		spin_unlock_irqrestore(&pipe->irq_lock, irqflags);
 
+		if (!vb)
+			return -EINVAL;
+
 		/*
 		 * If there is a per_frame setting to apply on the buffer,
 		 * do it before buffer en-queueing.
@@ -291,14 +294,13 @@ static int atomisp_q_video_buffers_to_css(struct atomisp_sub_device *asd,
 						    css_buf_type, css_pipe_id);
 		if (err) {
 			spin_lock_irqsave(&pipe->irq_lock, irqflags);
-			list_add_tail(&vb->queue, &pipe->activeq);
+			list_move_tail(&vb->queue, &pipe->activeq);
 			vb->state = VIDEOBUF_QUEUED;
 			spin_unlock_irqrestore(&pipe->irq_lock, irqflags);
 			dev_err(asd->isp->dev, "%s, css q fails: %d\n",
 				__func__, err);
 			return -EINVAL;
 		}
-		pipe->buffers_in_css++;
 
 		/* enqueue 3A/DIS/metadata buffers */
 		if (asd->params.curr_grid_info.s3a_grid.enable &&
diff --git a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
index bf5249b0d3bd..1fffe49cf578 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
@@ -1764,7 +1764,7 @@ int atomisp_streamoff(struct file *file, void *fh, enum v4l2_buf_type type)
 	mutex_unlock(&isp->mutex);
 	/* wait max 1 second */
 	ret = wait_event_interruptible_timeout(pipe->capq.wait,
-					       pipe->buffers_in_css == 0, HZ);
+					       atomisp_buffers_in_css(pipe) == 0, HZ);
 	mutex_lock(&isp->mutex);
 	pipe->stopping = false;
 	if (ret <= 0)
diff --git a/drivers/staging/media/atomisp/pci/atomisp_subdev.c b/drivers/staging/media/atomisp/pci/atomisp_subdev.c
index 847dfee6ad78..95dc4188309b 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_subdev.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_subdev.c
@@ -1064,6 +1064,7 @@ static void atomisp_init_subdev_pipe(struct atomisp_sub_device *asd,
 	pipe->asd = asd;
 	pipe->isp = asd->isp;
 	spin_lock_init(&pipe->irq_lock);
+	INIT_LIST_HEAD(&pipe->buffers_in_css);
 	INIT_LIST_HEAD(&pipe->activeq);
 	INIT_LIST_HEAD(&pipe->buffers_waiting_for_param);
 	INIT_LIST_HEAD(&pipe->per_frame_params);
diff --git a/drivers/staging/media/atomisp/pci/atomisp_subdev.h b/drivers/staging/media/atomisp/pci/atomisp_subdev.h
index 65c2f8664f9d..45b0c7341e84 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_subdev.h
+++ b/drivers/staging/media/atomisp/pci/atomisp_subdev.h
@@ -70,6 +70,9 @@ struct atomisp_video_pipe {
 	enum v4l2_buf_type type;
 	struct media_pad pad;
 	struct videobuf_queue capq;
+	/* List of video-buffers handed over to the CSS  */
+	struct list_head buffers_in_css;
+	/* List of video-buffers handed over to the driver, but not yet to the CSS */
 	struct list_head activeq;
 	/*
 	 * the buffers waiting for per-frame parameters, this is only valid
@@ -83,7 +86,6 @@ struct atomisp_video_pipe {
 	unsigned int default_run_mode;
 	/* Set from streamoff to disallow queuing further buffers in CSS */
 	bool stopping;
-	unsigned int buffers_in_css;
 
 	/*
 	 * irq_lock is used to protect video buffer state change operations and
-- 
2.37.3

[PATCH v2 07/17] media: atomisp: Add an index helper variable to atomisp_buf_done()

[Hans de Goede]

With the videobuf2 conversion accessing the index of a frame is going
to become more involved and writing this out each time is undesired.

Add an 'i' helper variable for the index and assing this once as
preparation for the videobuf2 conversion.

This also makes use of the new rules wrt max line-lengths in the kernel
to avoid breaking up a bunch of lines. Not breaking these lines results
in better readable code (IMHO).

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 .../staging/media/atomisp/pci/atomisp_cmd.c   | 70 ++++++++-----------
 1 file changed, 28 insertions(+), 42 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp_cmd.c b/drivers/staging/media/atomisp/pci/atomisp_cmd.c
index 5f0bebefcadd..a97c54615ba4 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_cmd.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_cmd.c
@@ -878,7 +878,6 @@ void atomisp_buf_done(struct atomisp_sub_device *asd, int error,
 	struct atomisp_video_pipe *pipe = NULL;
 	struct atomisp_css_buffer buffer;
 	bool requeue = false;
-	int err;
 	unsigned long irqflags;
 	struct ia_css_frame *frame = NULL;
 	struct atomisp_s3a_buf *s3a_buf = NULL, *_s3a_buf_tmp, *s3a_iter;
@@ -887,6 +886,7 @@ void atomisp_buf_done(struct atomisp_sub_device *asd, int error,
 	enum atomisp_metadata_type md_type;
 	struct atomisp_device *isp = asd->isp;
 	struct v4l2_control ctrl;
+	int i, err;
 
 	lockdep_assert_held(&isp->mutex);
 
@@ -1072,66 +1072,52 @@ void atomisp_buf_done(struct atomisp_sub_device *asd, int error,
 			break;
 		}
 
+		i = vb->i;
+
 		/* free the parameters */
-		if (pipe->frame_params[vb->i]) {
-			if (asd->params.dvs_6axis ==
-			    pipe->frame_params[vb->i]->params.dvs_6axis)
+		if (pipe->frame_params[i]) {
+			if (asd->params.dvs_6axis == pipe->frame_params[i]->params.dvs_6axis)
 				asd->params.dvs_6axis = NULL;
-			atomisp_free_css_parameters(
-			    &pipe->frame_params[vb->i]->params);
-			kvfree(pipe->frame_params[vb->i]);
-			pipe->frame_params[vb->i] = NULL;
+			atomisp_free_css_parameters(&pipe->frame_params[i]->params);
+			kvfree(pipe->frame_params[i]);
+			pipe->frame_params[i] = NULL;
 		}
 
-		pipe->frame_config_id[vb->i] = frame->isp_config_id;
+		pipe->frame_config_id[i] = frame->isp_config_id;
 		ctrl.id = V4L2_CID_FLASH_MODE;
 		if (asd->params.flash_state == ATOMISP_FLASH_ONGOING) {
-			if (frame->flash_state
-			    == IA_CSS_FRAME_FLASH_STATE_PARTIAL) {
-				asd->frame_status[vb->i] =
-				    ATOMISP_FRAME_STATUS_FLASH_PARTIAL;
-				dev_dbg(isp->dev, "%s partially flashed\n",
-					__func__);
-			} else if (frame->flash_state
-				   == IA_CSS_FRAME_FLASH_STATE_FULL) {
-				asd->frame_status[vb->i] =
-				    ATOMISP_FRAME_STATUS_FLASH_EXPOSED;
+			if (frame->flash_state == IA_CSS_FRAME_FLASH_STATE_PARTIAL) {
+				asd->frame_status[i] = ATOMISP_FRAME_STATUS_FLASH_PARTIAL;
+				dev_dbg(isp->dev, "%s partially flashed\n", __func__);
+			} else if (frame->flash_state == IA_CSS_FRAME_FLASH_STATE_FULL) {
+				asd->frame_status[i] = ATOMISP_FRAME_STATUS_FLASH_EXPOSED;
 				asd->params.num_flash_frames--;
-				dev_dbg(isp->dev, "%s completely flashed\n",
-					__func__);
+				dev_dbg(isp->dev, "%s completely flashed\n", __func__);
 			} else {
-				asd->frame_status[vb->i] =
-				    ATOMISP_FRAME_STATUS_OK;
-				dev_dbg(isp->dev,
-					"%s no flash in this frame\n",
-					__func__);
+				asd->frame_status[i] = ATOMISP_FRAME_STATUS_OK;
+				dev_dbg(isp->dev, "%s no flash in this frame\n", __func__);
 			}
 
 			/* Check if flashing sequence is done */
-			if (asd->frame_status[vb->i] ==
-			    ATOMISP_FRAME_STATUS_FLASH_EXPOSED)
+			if (asd->frame_status[i] == ATOMISP_FRAME_STATUS_FLASH_EXPOSED)
 				asd->params.flash_state = ATOMISP_FLASH_DONE;
 		} else if (isp->flash) {
-			if (v4l2_g_ctrl(isp->flash->ctrl_handler, &ctrl) ==
-			    0 && ctrl.value == ATOMISP_FLASH_MODE_TORCH) {
+			if (v4l2_g_ctrl(isp->flash->ctrl_handler, &ctrl) == 0 &&
+			    ctrl.value == ATOMISP_FLASH_MODE_TORCH) {
 				ctrl.id = V4L2_CID_FLASH_TORCH_INTENSITY;
-				if (v4l2_g_ctrl(isp->flash->ctrl_handler, &ctrl)
-				    == 0 && ctrl.value > 0) {
-					asd->frame_status[vb->i] =
-					    ATOMISP_FRAME_STATUS_FLASH_EXPOSED;
-				} else {
-					asd->frame_status[vb->i] =
-					    ATOMISP_FRAME_STATUS_OK;
-				}
+				if (v4l2_g_ctrl(isp->flash->ctrl_handler, &ctrl) == 0 &&
+				    ctrl.value > 0)
+					asd->frame_status[i] = ATOMISP_FRAME_STATUS_FLASH_EXPOSED;
+				else
+					asd->frame_status[i] = ATOMISP_FRAME_STATUS_OK;
 			} else {
-				asd->frame_status[vb->i] =
-				    ATOMISP_FRAME_STATUS_OK;
+				asd->frame_status[i] = ATOMISP_FRAME_STATUS_OK;
 			}
 		} else {
-			asd->frame_status[vb->i] = ATOMISP_FRAME_STATUS_OK;
+			asd->frame_status[i] = ATOMISP_FRAME_STATUS_OK;
 		}
 
-		asd->params.last_frame_status = asd->frame_status[vb->i];
+		asd->params.last_frame_status = asd->frame_status[i];
 
 		if (asd->continuous_mode->val) {
 			if (css_pipe_id == IA_CSS_PIPE_ID_PREVIEW ||
-- 
2.37.3

[PATCH v2 08/17] media: atomisp: Use new atomisp_flush_video_pipe() helper in atomisp_streamoff()

[Hans de Goede]

Use the new atomisp_flush_video_pipe() helper instead of open-coding it.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 .../staging/media/atomisp/pci/atomisp_ioctl.c | 40 +------------------
 1 file changed, 2 insertions(+), 38 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
index 1fffe49cf578..7d5413981714 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
@@ -1734,11 +1734,6 @@ int atomisp_streamoff(struct file *file, void *fh, enum v4l2_buf_type type)
 	struct pci_dev *pdev = to_pci_dev(isp->dev);
 	struct atomisp_video_pipe *pipe = atomisp_to_video_pipe(vdev);
 	struct atomisp_sub_device *asd = pipe->asd;
-	struct atomisp_video_pipe *capture_pipe = NULL;
-	struct atomisp_video_pipe *vf_pipe = NULL;
-	struct atomisp_video_pipe *preview_pipe = NULL;
-	struct atomisp_video_pipe *video_pipe = NULL;
-	struct videobuf_buffer *vb, *_vb;
 	enum ia_css_pipe_id css_pipe_id;
 	int ret;
 	unsigned long flags;
@@ -1819,43 +1814,12 @@ int atomisp_streamoff(struct file *file, void *fh, enum v4l2_buf_type type)
 	css_pipe_id = atomisp_get_css_pipe_id(asd);
 	atomisp_css_stop(asd, css_pipe_id, false);
 
-	/* cancel work queue*/
-	if (asd->video_out_capture.users) {
-		capture_pipe = &asd->video_out_capture;
-		wake_up_interruptible(&capture_pipe->capq.wait);
-	}
-	if (asd->video_out_vf.users) {
-		vf_pipe = &asd->video_out_vf;
-		wake_up_interruptible(&vf_pipe->capq.wait);
-	}
-	if (asd->video_out_preview.users) {
-		preview_pipe = &asd->video_out_preview;
-		wake_up_interruptible(&preview_pipe->capq.wait);
-	}
-	if (asd->video_out_video_capture.users) {
-		video_pipe = &asd->video_out_video_capture;
-		wake_up_interruptible(&video_pipe->capq.wait);
-	}
+	atomisp_flush_video_pipe(pipe, true);
+
 	ret = videobuf_streamoff(&pipe->capq);
 	if (ret)
 		return ret;
 
-	/* cleanup css here */
-	/* no need for this, as ISP will be reset anyway */
-	/*atomisp_flush_bufs_in_css(isp);*/
-
-	spin_lock_irqsave(&pipe->irq_lock, flags);
-	list_for_each_entry_safe(vb, _vb, &pipe->activeq, queue) {
-		vb->state = VIDEOBUF_PREPARED;
-		list_del(&vb->queue);
-	}
-	list_for_each_entry_safe(vb, _vb, &pipe->buffers_waiting_for_param, queue) {
-		vb->state = VIDEOBUF_PREPARED;
-		list_del(&vb->queue);
-		pipe->frame_request_config_id[vb->i] = 0;
-	}
-	spin_unlock_irqrestore(&pipe->irq_lock, flags);
-
 	atomisp_subdev_cleanup_pending_events(asd);
 stopsensor:
 	if (atomisp_subdev_streaming_count(asd) + 1
-- 
2.37.3

[PATCH v2 09/17] media: atomisp: Add ia_css_frame_get_info() helper

[Hans de Goede]

Several places rely on the [frame_]info member being the first member of
struct ia_css_frame, so that &frame->info will yield NULL when frame is
NULL (some places already explicitly check for a NULL frame pointer but
not nearly all).

For videobuf2 support the vb2_v4l2_buffer struct needs to be embedded
in the frame struct and it needs to be the first member. Breaking the
assumption that &frame->info will yield NULL when frame is NULL.

Add a ia_css_frame_get_info() helper to return either the ia_css_frame_info
struct embedded in the frame, or NULL when the frame pointer is NULL and
use this in places where a ia_css_frame_info ptr or NULL is expected.

To make sure that we catch all uses of the info field this patch also
renames the info field to frame_info.

This is a preparation patch for converting the driver to videobuf2.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 .../staging/media/atomisp/pci/atomisp_fops.c  |  2 +-
 .../media/atomisp/pci/ia_css_frame_public.h   | 10 ++-
 .../bayer_io_ls/ia_css_bayer_io.host.c        | 10 +--
 .../yuv444_io_ls/ia_css_yuv444_io.host.c      | 10 +--
 .../isp/kernels/ref/ref_1.0/ia_css_ref.host.c |  2 +-
 .../isp/kernels/tnr/tnr_1.0/ia_css_tnr.host.c |  4 +-
 .../pci/runtime/debug/src/ia_css_debug.c      | 26 +++----
 .../atomisp/pci/runtime/frame/src/frame.c     | 76 +++++++++----------
 drivers/staging/media/atomisp/pci/sh_css.c    | 38 +++++-----
 .../staging/media/atomisp/pci/sh_css_params.c | 17 ++---
 drivers/staging/media/atomisp/pci/sh_css_sp.c | 52 ++++++-------
 11 files changed, 124 insertions(+), 123 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp_fops.c b/drivers/staging/media/atomisp/pci/atomisp_fops.c
index ac9aa8649635..aefe1c56c262 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_fops.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_fops.c
@@ -992,7 +992,7 @@ static int remove_pad_from_frame(struct atomisp_device *isp,
 		if (ret < 0)
 			goto remove_pad_error;
 
-		load  += in_frame->info.padded_width;
+		load  += in_frame->frame_info.padded_width;
 		store += width;
 	}
 
diff --git a/drivers/staging/media/atomisp/pci/ia_css_frame_public.h b/drivers/staging/media/atomisp/pci/ia_css_frame_public.h
index 5727bd175330..d787c4054bb1 100644
--- a/drivers/staging/media/atomisp/pci/ia_css_frame_public.h
+++ b/drivers/staging/media/atomisp/pci/ia_css_frame_public.h
@@ -146,7 +146,7 @@ enum ia_css_frame_flash_state {
  *  This is the main structure used for all input and output images.
  */
 struct ia_css_frame {
-	struct ia_css_frame_info info; /** info struct describing the frame */
+	struct ia_css_frame_info frame_info; /** info struct describing the frame */
 	ia_css_ptr   data;	       /** pointer to start of image data */
 	unsigned int data_bytes;       /** size of image data in bytes */
 	/* LA: move this to ia_css_buffer */
@@ -184,7 +184,7 @@ struct ia_css_frame {
 };
 
 #define DEFAULT_FRAME { \
-	.info			= IA_CSS_BINARY_DEFAULT_FRAME_INFO, \
+	.frame_info		= IA_CSS_BINARY_DEFAULT_FRAME_INFO, \
 	.dynamic_queue_id	= SH_CSS_INVALID_QUEUE_ID, \
 	.buf_type		= IA_CSS_BUFFER_TYPE_INVALID, \
 	.flash_state		= IA_CSS_FRAME_FLASH_STATE_NONE, \
@@ -320,4 +320,10 @@ ia_css_frame_map(struct ia_css_frame **frame,
 void
 ia_css_frame_unmap(struct ia_css_frame *frame);
 
+static inline const struct ia_css_frame_info *
+ia_css_frame_get_info(const struct ia_css_frame *frame)
+{
+	return frame ? &frame->frame_info : NULL;
+}
+
 #endif /* __IA_CSS_FRAME_PUBLIC_H */
diff --git a/drivers/staging/media/atomisp/pci/isp/kernels/ipu2_io_ls/bayer_io_ls/ia_css_bayer_io.host.c b/drivers/staging/media/atomisp/pci/isp/kernels/ipu2_io_ls/bayer_io_ls/ia_css_bayer_io.host.c
index c7d88552dfde..0091e2a3da52 100644
--- a/drivers/staging/media/atomisp/pci/isp/kernels/ipu2_io_ls/bayer_io_ls/ia_css_bayer_io.host.c
+++ b/drivers/staging/media/atomisp/pci/isp/kernels/ipu2_io_ls/bayer_io_ls/ia_css_bayer_io.host.c
@@ -28,9 +28,7 @@ int ia_css_bayer_io_config(const struct ia_css_binary      *binary,
 	const struct ia_css_frame *in_frame = args->in_frame;
 	const struct ia_css_frame **out_frames = (const struct ia_css_frame **)
 		&args->out_frame;
-	const struct ia_css_frame_info *in_frame_info = (in_frame) ? &in_frame->info :
-		&binary->in_frame_info;
-
+	const struct ia_css_frame_info *in_frame_info = ia_css_frame_get_info(in_frame);
 	const unsigned int ddr_bits_per_element = sizeof(short) * 8;
 	const unsigned int ddr_elems_per_word = ceil_div(HIVE_ISP_DDR_WORD_BITS,
 						ddr_bits_per_element);
@@ -80,12 +78,12 @@ int ia_css_bayer_io_config(const struct ia_css_binary      *binary,
 				    "ia_css_bayer_io_config() put part enter:\n");
 #endif
 
-		ret = ia_css_dma_configure_from_info(&config, &out_frames[0]->info);
+		ret = ia_css_dma_configure_from_info(&config, &out_frames[0]->frame_info);
 		if (ret)
 			return ret;
 		to->base_address = out_frames[0]->data;
-		to->width = out_frames[0]->info.res.width;
-		to->height = out_frames[0]->info.res.height;
+		to->width = out_frames[0]->frame_info.res.width;
+		to->height = out_frames[0]->frame_info.res.height;
 		to->stride = config.stride;
 		to->ddr_elems_per_word = ddr_elems_per_word;
 
diff --git a/drivers/staging/media/atomisp/pci/isp/kernels/ipu2_io_ls/yuv444_io_ls/ia_css_yuv444_io.host.c b/drivers/staging/media/atomisp/pci/isp/kernels/ipu2_io_ls/yuv444_io_ls/ia_css_yuv444_io.host.c
index 7d2ef6e26ee6..32c504a950ce 100644
--- a/drivers/staging/media/atomisp/pci/isp/kernels/ipu2_io_ls/yuv444_io_ls/ia_css_yuv444_io.host.c
+++ b/drivers/staging/media/atomisp/pci/isp/kernels/ipu2_io_ls/yuv444_io_ls/ia_css_yuv444_io.host.c
@@ -28,9 +28,7 @@ int ia_css_yuv444_io_config(const struct ia_css_binary      *binary,
 	const struct ia_css_frame *in_frame = args->in_frame;
 	const struct ia_css_frame **out_frames = (const struct ia_css_frame **)
 		&args->out_frame;
-	const struct ia_css_frame_info *in_frame_info = (in_frame) ? &in_frame->info :
-		&binary->in_frame_info;
-
+	const struct ia_css_frame_info *in_frame_info = ia_css_frame_get_info(in_frame);
 	const unsigned int ddr_bits_per_element = sizeof(short) * 8;
 	const unsigned int ddr_elems_per_word = ceil_div(HIVE_ISP_DDR_WORD_BITS,
 						ddr_bits_per_element);
@@ -81,13 +79,13 @@ int ia_css_yuv444_io_config(const struct ia_css_binary      *binary,
 				    "ia_css_yuv444_io_config() put part enter:\n");
 #endif
 
-		ret = ia_css_dma_configure_from_info(&config, &out_frames[0]->info);
+		ret = ia_css_dma_configure_from_info(&config, &out_frames[0]->frame_info);
 		if (ret)
 			return ret;
 
 		to->base_address = out_frames[0]->data;
-		to->width = out_frames[0]->info.res.width;
-		to->height = out_frames[0]->info.res.height;
+		to->width = out_frames[0]->frame_info.res.width;
+		to->height = out_frames[0]->frame_info.res.height;
 		to->stride = config.stride;
 		to->ddr_elems_per_word = ddr_elems_per_word;
 
diff --git a/drivers/staging/media/atomisp/pci/isp/kernels/ref/ref_1.0/ia_css_ref.host.c b/drivers/staging/media/atomisp/pci/isp/kernels/ref/ref_1.0/ia_css_ref.host.c
index 08ed916a7eb8..9288a7a37b37 100644
--- a/drivers/staging/media/atomisp/pci/isp/kernels/ref/ref_1.0/ia_css_ref.host.c
+++ b/drivers/staging/media/atomisp/pci/isp/kernels/ref/ref_1.0/ia_css_ref.host.c
@@ -30,7 +30,7 @@ int ia_css_ref_config(struct sh_css_isp_ref_isp_config *to,
 	int ret;
 
 	if (from->ref_frames[0]) {
-		ret = ia_css_dma_configure_from_info(&to->port_b, &from->ref_frames[0]->info);
+		ret = ia_css_dma_configure_from_info(&to->port_b, &from->ref_frames[0]->frame_info);
 		if (ret)
 			return ret;
 		to->width_a_over_b = elems_a / to->port_b.elems;
diff --git a/drivers/staging/media/atomisp/pci/isp/kernels/tnr/tnr_1.0/ia_css_tnr.host.c b/drivers/staging/media/atomisp/pci/isp/kernels/tnr/tnr_1.0/ia_css_tnr.host.c
index 53050c0c49fc..a5fea753ec64 100644
--- a/drivers/staging/media/atomisp/pci/isp/kernels/tnr/tnr_1.0/ia_css_tnr.host.c
+++ b/drivers/staging/media/atomisp/pci/isp/kernels/tnr/tnr_1.0/ia_css_tnr.host.c
@@ -79,11 +79,11 @@ int ia_css_tnr_config(struct sh_css_isp_tnr_isp_config *to,
 	unsigned int i;
 	int ret;
 
-	ret = ia_css_dma_configure_from_info(&to->port_b, &from->tnr_frames[0]->info);
+	ret = ia_css_dma_configure_from_info(&to->port_b, &from->tnr_frames[0]->frame_info);
 	if (ret)
 		return ret;
 	to->width_a_over_b = elems_a / to->port_b.elems;
-	to->frame_height = from->tnr_frames[0]->info.res.height;
+	to->frame_height = from->tnr_frames[0]->frame_info.res.height;
 	for (i = 0; i < NUM_VIDEO_TNR_FRAMES; i++) {
 		to->tnr_frame_addr[i] = from->tnr_frames[i]->data +
 					from->tnr_frames[i]->planes.yuyv.offset;
diff --git a/drivers/staging/media/atomisp/pci/runtime/debug/src/ia_css_debug.c b/drivers/staging/media/atomisp/pci/runtime/debug/src/ia_css_debug.c
index 3d269bd23207..c10c2c598179 100644
--- a/drivers/staging/media/atomisp/pci/runtime/debug/src/ia_css_debug.c
+++ b/drivers/staging/media/atomisp/pci/runtime/debug/src/ia_css_debug.c
@@ -1301,11 +1301,11 @@ void ia_css_debug_frame_print(const struct ia_css_frame *frame,
 	data = (char *)HOST_ADDRESS(frame->data);
 	ia_css_debug_dtrace(2, "frame %s (%p):\n", descr, frame);
 	ia_css_debug_dtrace(2, "  resolution    = %dx%d\n",
-			    frame->info.res.width, frame->info.res.height);
+			    frame->frame_info.res.width, frame->frame_info.res.height);
 	ia_css_debug_dtrace(2, "  padded width  = %d\n",
-			    frame->info.padded_width);
-	ia_css_debug_dtrace(2, "  format        = %d\n", frame->info.format);
-	switch (frame->info.format) {
+			    frame->frame_info.padded_width);
+	ia_css_debug_dtrace(2, "  format        = %d\n", frame->frame_info.format);
+	switch (frame->frame_info.format) {
 	case IA_CSS_FRAME_FORMAT_NV12:
 	case IA_CSS_FRAME_FORMAT_NV16:
 	case IA_CSS_FRAME_FORMAT_NV21:
@@ -2565,11 +2565,11 @@ ia_css_debug_pipe_graph_dump_frame(
 	dtrace_dot(
 	    "node [shape = box, fixedsize=true, width=2, height=0.7]; \"%p\" [label = \"%s\\n%d(%d) x %d, %dbpp\\n%s\"];",
 	    frame,
-	    debug_frame_format2str(frame->info.format),
-	    frame->info.res.width,
-	    frame->info.padded_width,
-	    frame->info.res.height,
-	    frame->info.raw_bit_depth,
+	    debug_frame_format2str(frame->frame_info.format),
+	    frame->frame_info.res.width,
+	    frame->frame_info.padded_width,
+	    frame->frame_info.res.height,
+	    frame->frame_info.raw_bit_depth,
 	    bufinfo);
 
 	if (in_frame) {
@@ -2866,10 +2866,10 @@ ia_css_debug_pipe_graph_dump_sp_raw_copy(
 	snprintf(ring_buffer, sizeof(ring_buffer),
 		 "node [shape = box, fixedsize=true, width=2, height=0.7]; \"%p\" [label = \"%s\\n%d(%d) x %d\\nRingbuffer\"];",
 		 out_frame,
-		 debug_frame_format2str(out_frame->info.format),
-		 out_frame->info.res.width,
-		 out_frame->info.padded_width,
-		 out_frame->info.res.height);
+		 debug_frame_format2str(out_frame->frame_info.format),
+		 out_frame->frame_info.res.width,
+		 out_frame->frame_info.padded_width,
+		 out_frame->frame_info.res.height);
 
 	dtrace_dot(ring_buffer);
 
diff --git a/drivers/staging/media/atomisp/pci/runtime/frame/src/frame.c b/drivers/staging/media/atomisp/pci/runtime/frame/src/frame.c
index c20a4527c842..332b4a39e74d 100644
--- a/drivers/staging/media/atomisp/pci/runtime/frame/src/frame.c
+++ b/drivers/staging/media/atomisp/pci/runtime/frame/src/frame.c
@@ -286,32 +286,32 @@ int ia_css_frame_init_planes(struct ia_css_frame *frame)
 {
 	assert(frame);
 
-	switch (frame->info.format) {
+	switch (frame->frame_info.format) {
 	case IA_CSS_FRAME_FORMAT_MIPI:
 		dev_err(atomisp_dev,
 			"%s: unexpected use of IA_CSS_FRAME_FORMAT_MIPI\n", __func__);
 		return -EINVAL;
 	case IA_CSS_FRAME_FORMAT_RAW_PACKED:
 		frame_init_raw_single_plane(frame, &frame->planes.raw,
-					    frame->info.res.height,
-					    frame->info.padded_width,
-					    frame->info.raw_bit_depth);
+					    frame->frame_info.res.height,
+					    frame->frame_info.padded_width,
+					    frame->frame_info.raw_bit_depth);
 		break;
 	case IA_CSS_FRAME_FORMAT_RAW:
 		frame_init_single_plane(frame, &frame->planes.raw,
-					frame->info.res.height,
-					frame->info.padded_width,
-					frame->info.raw_bit_depth <= 8 ? 1 : 2);
+					frame->frame_info.res.height,
+					frame->frame_info.padded_width,
+					frame->frame_info.raw_bit_depth <= 8 ? 1 : 2);
 		break;
 	case IA_CSS_FRAME_FORMAT_RGB565:
 		frame_init_single_plane(frame, &frame->planes.rgb,
-					frame->info.res.height,
-					frame->info.padded_width, 2);
+					frame->frame_info.res.height,
+					frame->frame_info.padded_width, 2);
 		break;
 	case IA_CSS_FRAME_FORMAT_RGBA888:
 		frame_init_single_plane(frame, &frame->planes.rgb,
-					frame->info.res.height,
-					frame->info.padded_width * 4, 1);
+					frame->frame_info.res.height,
+					frame->frame_info.padded_width * 4, 1);
 		break;
 	case IA_CSS_FRAME_FORMAT_PLANAR_RGB888:
 		frame_init_rgb_planes(frame, 1);
@@ -324,14 +324,14 @@ int ia_css_frame_init_planes(struct ia_css_frame *frame)
 	case IA_CSS_FRAME_FORMAT_CSI_MIPI_YUV420_8:
 	case IA_CSS_FRAME_FORMAT_CSI_MIPI_LEGACY_YUV420_8:
 		frame_init_single_plane(frame, &frame->planes.yuyv,
-					frame->info.res.height,
-					frame->info.padded_width * 2, 1);
+					frame->frame_info.res.height,
+					frame->frame_info.padded_width * 2, 1);
 		break;
 	case IA_CSS_FRAME_FORMAT_YUV_LINE:
 		/* Needs 3 extra lines to allow vf_pp prefetching */
 		frame_init_single_plane(frame, &frame->planes.yuyv,
-					frame->info.res.height * 3 / 2 + 3,
-					frame->info.padded_width, 1);
+					frame->frame_info.res.height * 3 / 2 + 3,
+					frame->frame_info.padded_width, 1);
 		break;
 	case IA_CSS_FRAME_FORMAT_NV11:
 		frame_init_nv_planes(frame, 4, 1, 1);
@@ -380,8 +380,8 @@ int ia_css_frame_init_planes(struct ia_css_frame *frame)
 		break;
 	case IA_CSS_FRAME_FORMAT_BINARY_8:
 		frame_init_single_plane(frame, &frame->planes.binary.data,
-					frame->info.res.height,
-					frame->info.padded_width, 1);
+					frame->frame_info.res.height,
+					frame->frame_info.padded_width, 1);
 		frame->planes.binary.size = 0;
 		break;
 	default:
@@ -510,8 +510,8 @@ bool ia_css_frame_is_same_type(const struct ia_css_frame *frame_a,
 			       const struct ia_css_frame *frame_b)
 {
 	bool is_equal = false;
-	const struct ia_css_frame_info *info_a = &frame_a->info,
-						*info_b = &frame_b->info;
+	const struct ia_css_frame_info *info_a = &frame_a->frame_info;
+	const struct ia_css_frame_info *info_b = &frame_b->frame_info;
 
 	ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
 			    "ia_css_frame_is_same_type() enter:\n");
@@ -613,8 +613,8 @@ static void frame_init_nv_planes(struct ia_css_frame *frame,
 				 unsigned int vertical_decimation,
 				 unsigned int bytes_per_element)
 {
-	unsigned int y_width = frame->info.padded_width;
-	unsigned int y_height = frame->info.res.height;
+	unsigned int y_width = frame->frame_info.padded_width;
+	unsigned int y_height = frame->frame_info.res.height;
 	unsigned int uv_width;
 	unsigned int uv_height;
 	unsigned int y_bytes;
@@ -627,7 +627,7 @@ static void frame_init_nv_planes(struct ia_css_frame *frame,
 	uv_width = 2 * (y_width / horizontal_decimation);
 	uv_height = y_height / vertical_decimation;
 
-	if (frame->info.format == IA_CSS_FRAME_FORMAT_NV12_TILEY) {
+	if (frame->frame_info.format == IA_CSS_FRAME_FORMAT_NV12_TILEY) {
 		y_width   = CEIL_MUL(y_width,   NV12_TILEY_TILE_WIDTH);
 		uv_width  = CEIL_MUL(uv_width,  NV12_TILEY_TILE_WIDTH);
 		y_height  = CEIL_MUL(y_height,  NV12_TILEY_TILE_HEIGHT);
@@ -652,8 +652,8 @@ static void frame_init_yuv_planes(struct ia_css_frame *frame,
 				  bool swap_uv,
 				  unsigned int bytes_per_element)
 {
-	unsigned int y_width = frame->info.padded_width,
-		     y_height = frame->info.res.height,
+	unsigned int y_width = frame->frame_info.padded_width,
+		     y_height = frame->frame_info.res.height,
 		     uv_width = y_width / horizontal_decimation,
 		     uv_height = y_height / vertical_decimation,
 		     y_stride, y_bytes, uv_bytes, uv_stride;
@@ -682,8 +682,8 @@ static void frame_init_yuv_planes(struct ia_css_frame *frame,
 static void frame_init_rgb_planes(struct ia_css_frame *frame,
 				  unsigned int bytes_per_element)
 {
-	unsigned int width = frame->info.res.width,
-		     height = frame->info.res.height, stride, bytes;
+	unsigned int width = frame->frame_info.res.width,
+		     height = frame->frame_info.res.height, stride, bytes;
 
 	stride = width * bytes_per_element;
 	bytes = stride * height;
@@ -698,8 +698,8 @@ static void frame_init_rgb_planes(struct ia_css_frame *frame,
 
 static void frame_init_qplane6_planes(struct ia_css_frame *frame)
 {
-	unsigned int width = frame->info.padded_width / 2,
-		     height = frame->info.res.height / 2, bytes, stride;
+	unsigned int width = frame->frame_info.padded_width / 2,
+		     height = frame->frame_info.res.height / 2, bytes, stride;
 
 	stride = width * 2;
 	bytes = stride * height;
@@ -781,11 +781,11 @@ static struct ia_css_frame *frame_create(unsigned int width,
 		return NULL;
 
 	memset(me, 0, sizeof(*me));
-	me->info.res.width = width;
-	me->info.res.height = height;
-	me->info.format = format;
-	me->info.padded_width = padded_width;
-	me->info.raw_bit_depth = raw_bit_depth;
+	me->frame_info.res.width = width;
+	me->frame_info.res.height = height;
+	me->frame_info.format = format;
+	me->frame_info.padded_width = padded_width;
+	me->frame_info.raw_bit_depth = raw_bit_depth;
 	me->valid = valid;
 	me->data_bytes = 0;
 	me->data = mmgr_NULL;
@@ -851,11 +851,11 @@ void ia_css_resolution_to_sp_resolution(
 int ia_css_frame_init_from_info(struct ia_css_frame *frame,
 				const struct ia_css_frame_info *frame_info)
 {
-	frame->info.res.width = frame_info->res.width;
-	frame->info.res.height = frame_info->res.height;
-	frame->info.format = frame_info->format;
-	frame->info.padded_width = frame_info->padded_width;
-	frame->info.raw_bit_depth = frame_info->raw_bit_depth;
+	frame->frame_info.res.width = frame_info->res.width;
+	frame->frame_info.res.height = frame_info->res.height;
+	frame->frame_info.format = frame_info->format;
+	frame->frame_info.padded_width = frame_info->padded_width;
+	frame->frame_info.raw_bit_depth = frame_info->raw_bit_depth;
 	frame->valid = true;
 	/* To indicate it is not valid frame. */
 	frame->dynamic_queue_id = SH_CSS_INVALID_QUEUE_ID;
diff --git a/drivers/staging/media/atomisp/pci/sh_css.c b/drivers/staging/media/atomisp/pci/sh_css.c
index da96aaffebc1..5fae96bf447d 100644
--- a/drivers/staging/media/atomisp/pci/sh_css.c
+++ b/drivers/staging/media/atomisp/pci/sh_css.c
@@ -3060,7 +3060,7 @@ init_vf_frameinfo_defaults(struct ia_css_pipe *pipe,
 
 	assert(vf_frame);
 
-	sh_css_pipe_get_viewfinder_frame_info(pipe, &vf_frame->info, idx);
+	sh_css_pipe_get_viewfinder_frame_info(pipe, &vf_frame->frame_info, idx);
 	vf_frame->flash_state = IA_CSS_FRAME_FLASH_STATE_NONE;
 	ia_css_pipeline_get_sp_thread_id(ia_css_pipe_get_pipe_num(pipe), &thread_id);
 	ia_css_query_internal_queue_id(IA_CSS_BUFFER_TYPE_VF_OUTPUT_FRAME + idx, thread_id, &queue_id);
@@ -3229,31 +3229,31 @@ init_in_frameinfo_memory_defaults(struct ia_css_pipe *pipe,
 	assert(frame);
 	in_frame = frame;
 
-	in_frame->info.format = format;
+	in_frame->frame_info.format = format;
 
 #ifdef ISP2401
 	if (format == IA_CSS_FRAME_FORMAT_RAW)
-		in_frame->info.format = (pipe->stream->config.pack_raw_pixels) ?
+		in_frame->frame_info.format = (pipe->stream->config.pack_raw_pixels) ?
 		IA_CSS_FRAME_FORMAT_RAW_PACKED : IA_CSS_FRAME_FORMAT_RAW;
 #endif
 
-	in_frame->info.res.width = pipe->stream->config.input_config.input_res.width;
-	in_frame->info.res.height = pipe->stream->config.input_config.input_res.height;
-	in_frame->info.raw_bit_depth =
-	ia_css_pipe_util_pipe_input_format_bpp(pipe);
-	ia_css_frame_info_set_width(&in_frame->info, pipe->stream->config.input_config.input_res.width, 0);
+	in_frame->frame_info.res.width = pipe->stream->config.input_config.input_res.width;
+	in_frame->frame_info.res.height = pipe->stream->config.input_config.input_res.height;
+	in_frame->frame_info.raw_bit_depth = ia_css_pipe_util_pipe_input_format_bpp(pipe);
+	ia_css_frame_info_set_width(&in_frame->frame_info,
+				    pipe->stream->config.input_config.input_res.width, 0);
 	in_frame->flash_state = IA_CSS_FRAME_FLASH_STATE_NONE;
 	ia_css_pipeline_get_sp_thread_id(ia_css_pipe_get_pipe_num(pipe), &thread_id);
 	ia_css_query_internal_queue_id(IA_CSS_BUFFER_TYPE_INPUT_FRAME, thread_id, &queue_id);
 	in_frame->dynamic_queue_id = queue_id;
 	in_frame->buf_type = IA_CSS_BUFFER_TYPE_INPUT_FRAME;
 #ifdef ISP2401
-	ia_css_get_crop_offsets(pipe, &in_frame->info);
+	ia_css_get_crop_offsets(pipe, &in_frame->frame_info);
 #endif
 	err = ia_css_frame_init_planes(in_frame);
 
-	ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE_PRIVATE,
-			    "init_in_frameinfo_memory_defaults() bayer_order = %d:\n", in_frame->info.raw_bayer_order);
+	ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE_PRIVATE, "%s() bayer_order = %d\n",
+			    __func__, in_frame->frame_info.raw_bayer_order);
 
 	return err;
 }
@@ -3268,7 +3268,7 @@ init_out_frameinfo_defaults(struct ia_css_pipe *pipe,
 
 	assert(out_frame);
 
-	sh_css_pipe_get_output_frame_info(pipe, &out_frame->info, idx);
+	sh_css_pipe_get_output_frame_info(pipe, &out_frame->frame_info, idx);
 	out_frame->flash_state = IA_CSS_FRAME_FLASH_STATE_NONE;
 	ia_css_pipeline_get_sp_thread_id(ia_css_pipe_get_pipe_num(pipe), &thread_id);
 	ia_css_query_internal_queue_id(IA_CSS_BUFFER_TYPE_OUTPUT_FRAME + idx, thread_id, &queue_id);
@@ -4146,7 +4146,7 @@ ia_css_pipe_dequeue_buffer(struct ia_css_pipe *pipe,
 				if (!frame->valid)
 					pipe->num_invalid_frames--;
 
-				if (frame->info.format == IA_CSS_FRAME_FORMAT_BINARY_8) {
+				if (frame->frame_info.format == IA_CSS_FRAME_FORMAT_BINARY_8) {
 #ifdef ISP2401
 					frame->planes.binary.size = frame->data_bytes;
 #else
@@ -7102,7 +7102,7 @@ create_host_yuvpp_pipeline(struct ia_css_pipe *pipe)
 			/* we use output port 1 as internal output port */
 			tmp_in_frame = yuv_scaler_stage->args.out_frame[1];
 			if (pipe->pipe_settings.yuvpp.is_output_stage[i]) {
-				if (tmp_vf_frame && (tmp_vf_frame->info.res.width != 0)) {
+				if (tmp_vf_frame && (tmp_vf_frame->frame_info.res.width != 0)) {
 					in_frame = yuv_scaler_stage->args.out_vf_frame;
 					err = add_vf_pp_stage(pipe, in_frame,
 							      tmp_vf_frame,
@@ -7118,7 +7118,7 @@ create_host_yuvpp_pipeline(struct ia_css_pipe *pipe)
 			}
 		}
 	} else if (copy_stage) {
-		if (vf_frame[0] && vf_frame[0]->info.res.width != 0) {
+		if (vf_frame[0] && vf_frame[0]->frame_info.res.width != 0) {
 			in_frame = copy_stage->args.out_vf_frame;
 			err = add_vf_pp_stage(pipe, in_frame, vf_frame[0],
 					      &vf_pp_binary[0], &vf_pp_stage);
@@ -7158,10 +7158,10 @@ create_host_copy_pipeline(struct ia_css_pipe *pipe,
 
 	if (copy_on_sp(pipe) &&
 	    pipe->stream->config.input_config.format == ATOMISP_INPUT_FORMAT_BINARY_8) {
-		ia_css_frame_info_init(&out_frame->info, JPEG_BYTES, 1,
+		ia_css_frame_info_init(&out_frame->frame_info, JPEG_BYTES, 1,
 				       IA_CSS_FRAME_FORMAT_BINARY_8, 0);
-	} else if (out_frame->info.format == IA_CSS_FRAME_FORMAT_RAW) {
-		out_frame->info.raw_bit_depth =
+	} else if (out_frame->frame_info.format == IA_CSS_FRAME_FORMAT_RAW) {
+		out_frame->frame_info.raw_bit_depth =
 		ia_css_pipe_util_pipe_input_format_bpp(pipe);
 	}
 
@@ -7200,7 +7200,7 @@ create_host_isyscopy_capture_pipeline(struct ia_css_pipe *pipe)
 	ia_css_pipeline_clean(me);
 
 	/* Construct out_frame info */
-	err = sh_css_pipe_get_output_frame_info(pipe, &out_frame->info, 0);
+	err = sh_css_pipe_get_output_frame_info(pipe, &out_frame->frame_info, 0);
 	if (err)
 		return err;
 	out_frame->flash_state = IA_CSS_FRAME_FLASH_STATE_NONE;
diff --git a/drivers/staging/media/atomisp/pci/sh_css_params.c b/drivers/staging/media/atomisp/pci/sh_css_params.c
index 67915d76a87f..f08564f58242 100644
--- a/drivers/staging/media/atomisp/pci/sh_css_params.c
+++ b/drivers/staging/media/atomisp/pci/sh_css_params.c
@@ -936,8 +936,8 @@ sh_css_set_black_frame(struct ia_css_stream *stream,
 	assert(raw_black_frame);
 
 	params = stream->isp_params_configs;
-	height = raw_black_frame->info.res.height;
-	width = raw_black_frame->info.padded_width;
+	height = raw_black_frame->frame_info.res.height;
+	width = raw_black_frame->frame_info.padded_width;
 
 	ptr = raw_black_frame->data
 	+ raw_black_frame->planes.raw.offset;
@@ -1187,16 +1187,15 @@ ia_css_process_zoom_and_motion(
 			const struct sh_css_binary_args *args = &stage->args;
 			const struct ia_css_frame_info *out_infos[IA_CSS_BINARY_MAX_OUTPUT_PORTS] = {NULL};
 
-			if (args->out_frame[0])
-				out_infos[0] = &args->out_frame[0]->info;
+			out_infos[0] = ia_css_frame_get_info(args->out_frame[0]);
+
 			info = &stage->firmware->info.isp;
 			ia_css_binary_fill_info(info, false, false,
 						ATOMISP_INPUT_FORMAT_RAW_10,
-						args->in_frame  ? &args->in_frame->info  : NULL,
+						ia_css_frame_get_info(args->in_frame),
 						NULL,
 						out_infos,
-						args->out_vf_frame ? &args->out_vf_frame->info
-						: NULL,
+						ia_css_frame_get_info(args->out_vf_frame),
 						&tmp_binary,
 						NULL,
 						-1, true);
@@ -3461,10 +3460,10 @@ sh_css_params_write_to_ddr_internal(
 			if (stage->args.delay_frames[0]) {
 				/*When delay frames are present(as in case of video),
 				they are used for dvs. Configure DVS using those params*/
-				dvs_in_frame_info = &stage->args.delay_frames[0]->info;
+				dvs_in_frame_info = &stage->args.delay_frames[0]->frame_info;
 			} else {
 				/*Otherwise, use input frame to configure DVS*/
-				dvs_in_frame_info = &stage->args.in_frame->info;
+				dvs_in_frame_info = &stage->args.in_frame->frame_info;
 			}
 
 			/* Generate default DVS unity table on start up*/
diff --git a/drivers/staging/media/atomisp/pci/sh_css_sp.c b/drivers/staging/media/atomisp/pci/sh_css_sp.c
index 615500a7d3c4..c301298b8ee4 100644
--- a/drivers/staging/media/atomisp/pci/sh_css_sp.c
+++ b/drivers/staging/media/atomisp/pci/sh_css_sp.c
@@ -277,10 +277,10 @@ sh_css_sp_start_raw_copy(struct ia_css_frame *out_frame,
 	ia_css_pipeline_get_sp_thread_id(pipe_num, &thread_id);
 	pipe = &sh_css_sp_group.pipe[thread_id];
 
-	pipe->copy.raw.height	    = out_frame->info.res.height;
-	pipe->copy.raw.width	    = out_frame->info.res.width;
-	pipe->copy.raw.padded_width  = out_frame->info.padded_width;
-	pipe->copy.raw.raw_bit_depth = out_frame->info.raw_bit_depth;
+	pipe->copy.raw.height	    = out_frame->frame_info.res.height;
+	pipe->copy.raw.width	    = out_frame->frame_info.res.width;
+	pipe->copy.raw.padded_width  = out_frame->frame_info.padded_width;
+	pipe->copy.raw.raw_bit_depth = out_frame->frame_info.raw_bit_depth;
 	pipe->copy.raw.max_input_width = max_input_width;
 	pipe->num_stages = 1;
 	pipe->pipe_id = pipe_id;
@@ -351,10 +351,10 @@ sh_css_sp_start_isys_copy(struct ia_css_frame *out_frame,
 	ia_css_pipeline_get_sp_thread_id(pipe_num, &thread_id);
 	pipe = &sh_css_sp_group.pipe[thread_id];
 
-	pipe->copy.raw.height		= out_frame->info.res.height;
-	pipe->copy.raw.width		= out_frame->info.res.width;
-	pipe->copy.raw.padded_width	= out_frame->info.padded_width;
-	pipe->copy.raw.raw_bit_depth	= out_frame->info.raw_bit_depth;
+	pipe->copy.raw.height		= out_frame->frame_info.res.height;
+	pipe->copy.raw.width		= out_frame->frame_info.res.width;
+	pipe->copy.raw.padded_width	= out_frame->frame_info.padded_width;
+	pipe->copy.raw.raw_bit_depth	= out_frame->frame_info.raw_bit_depth;
 	pipe->copy.raw.max_input_width	= max_input_width;
 	pipe->num_stages		= 1;
 	pipe->pipe_id			= pipe_id;
@@ -451,9 +451,9 @@ sh_css_copy_frame_to_spframe(struct ia_css_frame_sp *sp_frame_out,
 					    frame_in->data,
 					    frame_in->buf_type);
 
-	ia_css_frame_info_to_frame_sp_info(&sp_frame_out->info, &frame_in->info);
+	ia_css_frame_info_to_frame_sp_info(&sp_frame_out->info, &frame_in->frame_info);
 
-	switch (frame_in->info.format) {
+	switch (frame_in->frame_info.format) {
 	case IA_CSS_FRAME_FORMAT_RAW_PACKED:
 	case IA_CSS_FRAME_FORMAT_RAW:
 		sp_frame_out->planes.raw.offset = frame_in->planes.raw.offset;
@@ -536,7 +536,7 @@ set_input_frame_buffer(const struct ia_css_frame *frame)
 	if (!frame)
 		return -EINVAL;
 
-	switch (frame->info.format) {
+	switch (frame->frame_info.format) {
 	case IA_CSS_FRAME_FORMAT_QPLANE6:
 	case IA_CSS_FRAME_FORMAT_YUV420_16:
 	case IA_CSS_FRAME_FORMAT_RAW_PACKED:
@@ -567,7 +567,7 @@ set_output_frame_buffer(const struct ia_css_frame *frame,
 	if (!frame)
 		return -EINVAL;
 
-	switch (frame->info.format) {
+	switch (frame->frame_info.format) {
 	case IA_CSS_FRAME_FORMAT_YUV420:
 	case IA_CSS_FRAME_FORMAT_YUV422:
 	case IA_CSS_FRAME_FORMAT_YUV444:
@@ -608,7 +608,7 @@ set_view_finder_buffer(const struct ia_css_frame *frame)
 	if (!frame)
 		return -EINVAL;
 
-	switch (frame->info.format) {
+	switch (frame->frame_info.format) {
 	/* the dual output pin */
 	case IA_CSS_FRAME_FORMAT_NV12:
 	case IA_CSS_FRAME_FORMAT_NV12_16:
@@ -819,34 +819,35 @@ static int configure_isp_from_args(const struct sh_css_sp_pipeline *pipeline,
 	ret = ia_css_fpn_configure(binary,  &binary->in_frame_info);
 	if (ret)
 		return ret;
-	ret = ia_css_crop_configure(binary, &args->delay_frames[0]->info);
+	ret = ia_css_crop_configure(binary, ia_css_frame_get_info(args->delay_frames[0]));
 	if (ret)
 		return ret;
 	ret = ia_css_qplane_configure(pipeline, binary, &binary->in_frame_info);
 	if (ret)
 		return ret;
-	ret = ia_css_output0_configure(binary, &args->out_frame[0]->info);
+	ret = ia_css_output0_configure(binary, ia_css_frame_get_info(args->out_frame[0]));
 	if (ret)
 		return ret;
-	ret = ia_css_output1_configure(binary, &args->out_vf_frame->info);
+	ret = ia_css_output1_configure(binary, ia_css_frame_get_info(args->out_vf_frame));
 	if (ret)
 		return ret;
 	ret = ia_css_copy_output_configure(binary, args->copy_output);
 	if (ret)
 		return ret;
-	ret = ia_css_output0_configure(binary, &args->out_frame[0]->info);
+	ret = ia_css_output0_configure(binary, ia_css_frame_get_info(args->out_frame[0]));
 	if (ret)
 		return ret;
-	ret = ia_css_iterator_configure(binary, &args->in_frame->info);
+	ret = ia_css_iterator_configure(binary, ia_css_frame_get_info(args->in_frame));
 	if (ret)
 		return ret;
-	ret = ia_css_dvs_configure(binary, &args->out_frame[0]->info);
+	ret = ia_css_dvs_configure(binary, ia_css_frame_get_info(args->out_frame[0]));
 	if (ret)
 		return ret;
-	ret = ia_css_output_configure(binary, &args->out_frame[0]->info);
+	ret = ia_css_output_configure(binary, ia_css_frame_get_info(args->out_frame[0]));
 	if (ret)
 		return ret;
-	ret = ia_css_raw_configure(pipeline, binary, &args->in_frame->info, &binary->in_frame_info, two_ppc, deinterleaved);
+	ret = ia_css_raw_configure(pipeline, binary, ia_css_frame_get_info(args->in_frame),
+				   &binary->in_frame_info, two_ppc, deinterleaved);
 	if (ret)
 		return ret;
 
@@ -1037,7 +1038,7 @@ sh_css_sp_init_stage(struct ia_css_binary *binary,
 		return -EINVAL;
 
 	if (args->in_frame)
-		ia_css_get_crop_offsets(pipe, &args->in_frame->info);
+		ia_css_get_crop_offsets(pipe, &args->in_frame->frame_info);
 	else
 		ia_css_get_crop_offsets(pipe, &binary->in_frame_info);
 #else
@@ -1124,15 +1125,14 @@ sp_init_stage(struct ia_css_pipeline_stage *stage,
 		const struct ia_css_frame_info *out_infos[IA_CSS_BINARY_MAX_OUTPUT_PORTS] = {NULL};
 
 		if (args->out_frame[0])
-			out_infos[0] = &args->out_frame[0]->info;
+			out_infos[0] = &args->out_frame[0]->frame_info;
 		info = &firmware->info.isp;
 		ia_css_binary_fill_info(info, false, false,
 					ATOMISP_INPUT_FORMAT_RAW_10,
-					args->in_frame  ? &args->in_frame->info  : NULL,
+					ia_css_frame_get_info(args->in_frame),
 					NULL,
 					out_infos,
-					args->out_vf_frame ? &args->out_vf_frame->info
-					: NULL,
+					ia_css_frame_get_info(args->out_vf_frame),
 					&tmp_binary,
 					NULL,
 					-1, true);
-- 
2.37.3

[PATCH v2 10/17] media: atomisp: Convert to videobuf2

[Hans de Goede]

Convert atomisp to use videobuf2.

This fixes mmap not working and in general moving over to
the more modern videobuf2 is a good plan.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 .../staging/media/atomisp/pci/atomisp_cmd.c   | 120 ++---
 .../staging/media/atomisp/pci/atomisp_cmd.h   |   3 +-
 .../media/atomisp/pci/atomisp_common.h        |   6 +-
 .../media/atomisp/pci/atomisp_compat.h        |   3 +-
 .../media/atomisp/pci/atomisp_compat_css20.c  |   4 +-
 .../staging/media/atomisp/pci/atomisp_fops.c  | 482 ++++++------------
 .../staging/media/atomisp/pci/atomisp_fops.h  |   7 -
 .../staging/media/atomisp/pci/atomisp_ioctl.c | 427 +++-------------
 .../staging/media/atomisp/pci/atomisp_ioctl.h |  10 +-
 .../media/atomisp/pci/atomisp_subdev.c        |   1 +
 .../media/atomisp/pci/atomisp_subdev.h        |  15 +-
 .../media/atomisp/pci/ia_css_frame_public.h   |  15 +
 12 files changed, 303 insertions(+), 790 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp_cmd.c b/drivers/staging/media/atomisp/pci/atomisp_cmd.c
index a97c54615ba4..0f8d683382fd 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_cmd.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_cmd.c
@@ -30,7 +30,6 @@
 #include <asm/iosf_mbi.h>
 
 #include <media/v4l2-event.h>
-#include <media/videobuf-vmalloc.h>
 
 #define CREATE_TRACE_POINTS
 #include "atomisp_trace_event.h"
@@ -662,23 +661,6 @@ void dump_sp_dmem(struct atomisp_device *isp, unsigned int addr,
 	} while (--size32);
 }
 
-static struct videobuf_buffer *atomisp_css_frame_to_vbuf(
-    struct atomisp_video_pipe *pipe, struct ia_css_frame *frame)
-{
-	struct videobuf_vmalloc_memory *vm_mem;
-	struct ia_css_frame *handle;
-	int i;
-
-	for (i = 0; pipe->capq.bufs[i]; i++) {
-		vm_mem = pipe->capq.bufs[i]->priv;
-		handle = vm_mem->vaddr;
-		if (handle && handle->data == frame->data)
-			return pipe->capq.bufs[i];
-	}
-
-	return NULL;
-}
-
 int atomisp_buffers_in_css(struct atomisp_video_pipe *pipe)
 {
 	unsigned long irqflags;
@@ -695,37 +677,40 @@ int atomisp_buffers_in_css(struct atomisp_video_pipe *pipe)
 	return buffers_in_css;
 }
 
-void atomisp_buffer_done(struct atomisp_video_pipe *pipe, struct videobuf_buffer *vb,
-			 int state)
+void atomisp_buffer_done(struct ia_css_frame *frame, enum vb2_buffer_state state)
 {
+	struct atomisp_video_pipe *pipe = vb_to_pipe(&frame->vb.vb2_buf);
+
 	lockdep_assert_held(&pipe->irq_lock);
 
-	vb->ts = ktime_get_ns();
-	vb->field_count = atomic_read(&pipe->asd->sequence) << 1;
-	vb->state = state;
-	list_del(&vb->queue);
-	wake_up(&vb->done);
+	frame->vb.vb2_buf.timestamp = ktime_get_ns();
+	frame->vb.field = pipe->pix.field;
+	frame->vb.sequence = atomic_read(&pipe->asd->sequence);
+	list_del(&frame->queue);
+	if (state == VB2_BUF_STATE_DONE)
+		vb2_set_plane_payload(&frame->vb.vb2_buf, 0, pipe->pix.sizeimage);
+	vb2_buffer_done(&frame->vb.vb2_buf, state);
 }
 
 void atomisp_flush_video_pipe(struct atomisp_video_pipe *pipe, bool warn_on_css_frames)
 {
-	struct videobuf_buffer *_vb, *vb;
+	struct ia_css_frame *frame, *_frame;
 	unsigned long irqflags;
 
 	spin_lock_irqsave(&pipe->irq_lock, irqflags);
 
-	list_for_each_entry_safe(vb, _vb, &pipe->buffers_in_css, queue) {
+	list_for_each_entry_safe(frame, _frame, &pipe->buffers_in_css, queue) {
 		if (warn_on_css_frames)
 			dev_warn(pipe->isp->dev, "Warning: CSS frames queued on flush\n");
-		atomisp_buffer_done(pipe, vb, VIDEOBUF_ERROR);
+		atomisp_buffer_done(frame, VB2_BUF_STATE_ERROR);
 	}
 
-	list_for_each_entry_safe(vb, _vb, &pipe->activeq, queue)
-		atomisp_buffer_done(pipe, vb, VIDEOBUF_ERROR);
+	list_for_each_entry_safe(frame, _frame, &pipe->activeq, queue)
+		atomisp_buffer_done(frame, VB2_BUF_STATE_ERROR);
 
-	list_for_each_entry_safe(vb, _vb, &pipe->buffers_waiting_for_param, queue) {
-		pipe->frame_request_config_id[vb->i] = 0;
-		atomisp_buffer_done(pipe, vb, VIDEOBUF_ERROR);
+	list_for_each_entry_safe(frame, _frame, &pipe->buffers_waiting_for_param, queue) {
+		pipe->frame_request_config_id[frame->vb.vb2_buf.index] = 0;
+		atomisp_buffer_done(frame, VB2_BUF_STATE_ERROR);
 	}
 
 	spin_unlock_irqrestore(&pipe->irq_lock, irqflags);
@@ -874,7 +859,6 @@ void atomisp_buf_done(struct atomisp_sub_device *asd, int error,
 		      enum ia_css_pipe_id css_pipe_id,
 		      bool q_buffers, enum atomisp_input_stream_id stream_id)
 {
-	struct videobuf_buffer *vb = NULL;
 	struct atomisp_video_pipe *pipe = NULL;
 	struct atomisp_css_buffer buffer;
 	bool requeue = false;
@@ -1027,10 +1011,7 @@ void atomisp_buf_done(struct atomisp_sub_device *asd, int error,
 				dev_dbg(isp->dev, "%s thumb no flash in this frame\n",
 					__func__);
 		}
-		vb = atomisp_css_frame_to_vbuf(pipe, frame);
-		WARN_ON(!vb);
-		if (vb)
-			pipe->frame_config_id[vb->i] = frame->isp_config_id;
+		pipe->frame_config_id[frame->vb.vb2_buf.index] = frame->isp_config_id;
 		if (css_pipe_id == IA_CSS_PIPE_ID_CAPTURE &&
 		    asd->pending_capture_request > 0) {
 			err = atomisp_css_offline_capture_configure(asd,
@@ -1066,13 +1047,8 @@ void atomisp_buf_done(struct atomisp_sub_device *asd, int error,
 
 		dev_dbg(isp->dev, "%s: main frame with exp_id %d is ready\n",
 			__func__, frame->exp_id);
-		vb = atomisp_css_frame_to_vbuf(pipe, frame);
-		if (!vb) {
-			WARN_ON(1);
-			break;
-		}
 
-		i = vb->i;
+		i = frame->vb.vb2_buf.index;
 
 		/* free the parameters */
 		if (pipe->frame_params[i]) {
@@ -1183,9 +1159,9 @@ void atomisp_buf_done(struct atomisp_sub_device *asd, int error,
 	default:
 		break;
 	}
-	if (vb) {
+	if (frame) {
 		spin_lock_irqsave(&pipe->irq_lock, irqflags);
-		atomisp_buffer_done(pipe, vb, error ? VIDEOBUF_ERROR : VIDEOBUF_DONE);
+		atomisp_buffer_done(frame, error ? VB2_BUF_STATE_ERROR : VB2_BUF_STATE_DONE);
 		spin_unlock_irqrestore(&pipe->irq_lock, irqflags);
 	}
 
@@ -3656,6 +3632,18 @@ void atomisp_free_css_parameters(struct atomisp_css_params *css_param)
 	}
 }
 
+static void atomisp_move_frame_to_activeq(struct ia_css_frame *frame,
+					  struct atomisp_css_params_with_list *param)
+{
+	struct atomisp_video_pipe *pipe = vb_to_pipe(&frame->vb.vb2_buf);
+	unsigned long irqflags;
+
+	pipe->frame_params[frame->vb.vb2_buf.index] = param;
+	spin_lock_irqsave(&pipe->irq_lock, irqflags);
+	list_move_tail(&frame->queue, &pipe->activeq);
+	spin_unlock_irqrestore(&pipe->irq_lock, irqflags);
+}
+
 /*
  * Check parameter queue list and buffer queue list to find out if matched items
  * and then set parameter to CSS and enqueue buffer to CSS.
@@ -3666,11 +3654,10 @@ void atomisp_free_css_parameters(struct atomisp_css_params *css_param)
 void atomisp_handle_parameter_and_buffer(struct atomisp_video_pipe *pipe)
 {
 	struct atomisp_sub_device *asd = pipe->asd;
-	struct videobuf_buffer *vb = NULL, *vb_tmp;
+	struct ia_css_frame *frame = NULL, *frame_tmp;
 	struct atomisp_css_params_with_list *param = NULL, *param_tmp;
-	struct videobuf_vmalloc_memory *vm_mem = NULL;
-	unsigned long irqflags;
 	bool need_to_enqueue_buffer = false;
+	int i;
 
 	lockdep_assert_held(&asd->isp->mutex);
 
@@ -3694,44 +3681,33 @@ void atomisp_handle_parameter_and_buffer(struct atomisp_video_pipe *pipe)
 	    list_empty(&pipe->buffers_waiting_for_param))
 		return;
 
-	list_for_each_entry_safe(vb, vb_tmp,
+	list_for_each_entry_safe(frame, frame_tmp,
 				 &pipe->buffers_waiting_for_param, queue) {
-		if (pipe->frame_request_config_id[vb->i]) {
+		i = frame->vb.vb2_buf.index;
+		if (pipe->frame_request_config_id[i]) {
 			list_for_each_entry_safe(param, param_tmp,
 						 &pipe->per_frame_params, list) {
-				if (pipe->frame_request_config_id[vb->i] !=
-				    param->params.isp_config_id)
+				if (pipe->frame_request_config_id[i] != param->params.isp_config_id)
 					continue;
 
 				list_del(&param->list);
-				list_del(&vb->queue);
+
 				/*
 				 * clear the request config id as the buffer
 				 * will be handled and enqueued into CSS soon
 				 */
-				pipe->frame_request_config_id[vb->i] = 0;
-				pipe->frame_params[vb->i] = param;
-				vm_mem = vb->priv;
-				BUG_ON(!vm_mem);
+				pipe->frame_request_config_id[i] = 0;
+				atomisp_move_frame_to_activeq(frame, param);
+				need_to_enqueue_buffer = true;
 				break;
 			}
 
-			if (vm_mem) {
-				spin_lock_irqsave(&pipe->irq_lock, irqflags);
-				list_add_tail(&vb->queue, &pipe->activeq);
-				spin_unlock_irqrestore(&pipe->irq_lock, irqflags);
-				vm_mem = NULL;
-				need_to_enqueue_buffer = true;
-			} else {
-				/* The is the end, stop further loop */
+			if (list_entry_is_head(param, &pipe->per_frame_params, list)) {
+				/* The is the end, stop outer loop */
 				break;
 			}
 		} else {
-			list_del(&vb->queue);
-			pipe->frame_params[vb->i] = NULL;
-			spin_lock_irqsave(&pipe->irq_lock, irqflags);
-			list_add_tail(&vb->queue, &pipe->activeq);
-			spin_unlock_irqrestore(&pipe->irq_lock, irqflags);
+			atomisp_move_frame_to_activeq(frame, NULL);
 			need_to_enqueue_buffer = true;
 		}
 	}
@@ -5538,8 +5514,6 @@ int atomisp_set_fmt(struct file *file, void *unused, struct v4l2_format *f)
 	f->fmt.pix.priv = PAGE_ALIGN(pipe->pix.width *
 				     pipe->pix.height * 2);
 
-	pipe->capq.field = f->fmt.pix.field;
-
 	/*
 	 * If in video 480P case, no GFX throttle
 	 */
diff --git a/drivers/staging/media/atomisp/pci/atomisp_cmd.h b/drivers/staging/media/atomisp/pci/atomisp_cmd.h
index 1b46f4e60924..4c965d17c9a3 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_cmd.h
+++ b/drivers/staging/media/atomisp/pci/atomisp_cmd.h
@@ -56,8 +56,7 @@ struct camera_mipi_info *atomisp_to_sensor_mipi_info(struct v4l2_subdev *sd);
 struct atomisp_video_pipe *atomisp_to_video_pipe(struct video_device *dev);
 int atomisp_reset(struct atomisp_device *isp);
 int atomisp_buffers_in_css(struct atomisp_video_pipe *pipe);
-void atomisp_buffer_done(struct atomisp_video_pipe *pipe, struct videobuf_buffer *buf,
-			 int state);
+void atomisp_buffer_done(struct ia_css_frame *frame, enum vb2_buffer_state state);
 void atomisp_flush_video_pipe(struct atomisp_video_pipe *pipe, bool warn_on_css_frames);
 void atomisp_flush_bufs_and_wakeup(struct atomisp_sub_device *asd);
 void atomisp_clear_css_buffer_counters(struct atomisp_sub_device *asd);
diff --git a/drivers/staging/media/atomisp/pci/atomisp_common.h b/drivers/staging/media/atomisp/pci/atomisp_common.h
index b29874f2bc0f..07c38e487a66 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_common.h
+++ b/drivers/staging/media/atomisp/pci/atomisp_common.h
@@ -25,7 +25,7 @@
 
 #include <linux/v4l2-mediabus.h>
 
-#include <media/videobuf-core.h>
+#include <media/videobuf2-v4l2.h>
 
 #include "atomisp_compat.h"
 
@@ -64,8 +64,4 @@ struct atomisp_fmt {
 	u32 bayer_order;
 };
 
-struct atomisp_buffer {
-	struct videobuf_buffer	vb;
-};
-
 #endif
diff --git a/drivers/staging/media/atomisp/pci/atomisp_compat.h b/drivers/staging/media/atomisp/pci/atomisp_compat.h
index a6d85d0f9ae5..d1893a0deec1 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_compat.h
+++ b/drivers/staging/media/atomisp/pci/atomisp_compat.h
@@ -22,7 +22,6 @@
 #include "atomisp_compat_css20.h"
 
 #include "../../include/linux/atomisp.h"
-#include <media/videobuf-vmalloc.h>
 
 struct atomisp_device;
 struct atomisp_sub_device;
@@ -61,7 +60,7 @@ int atomisp_css_irq_enable(struct atomisp_device *isp,
 			   enum ia_css_irq_info info, bool enable);
 
 int atomisp_q_video_buffer_to_css(struct atomisp_sub_device *asd,
-				  struct videobuf_vmalloc_memory *vm_mem,
+				  struct ia_css_frame *frame,
 				  enum atomisp_input_stream_id stream_id,
 				  enum ia_css_buffer_type css_buf_type,
 				  enum ia_css_pipe_id css_pipe_id);
diff --git a/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c b/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
index fdc05548d972..ea6114679c53 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
@@ -996,7 +996,7 @@ void atomisp_css_init_struct(struct atomisp_sub_device *asd)
 }
 
 int atomisp_q_video_buffer_to_css(struct atomisp_sub_device *asd,
-				  struct videobuf_vmalloc_memory *vm_mem,
+				  struct ia_css_frame *frame,
 				  enum atomisp_input_stream_id stream_id,
 				  enum ia_css_buffer_type css_buf_type,
 				  enum ia_css_pipe_id css_pipe_id)
@@ -1006,7 +1006,7 @@ int atomisp_q_video_buffer_to_css(struct atomisp_sub_device *asd,
 	int err;
 
 	css_buf.type = css_buf_type;
-	css_buf.data.frame = vm_mem->vaddr;
+	css_buf.data.frame = frame;
 
 	err = ia_css_pipe_enqueue_buffer(
 		  stream_env->pipes[css_pipe_id], &css_buf);
diff --git a/drivers/staging/media/atomisp/pci/atomisp_fops.c b/drivers/staging/media/atomisp/pci/atomisp_fops.c
index aefe1c56c262..bc47a092a8af 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_fops.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_fops.c
@@ -22,7 +22,7 @@
 #include <linux/pm_runtime.h>
 
 #include <media/v4l2-ioctl.h>
-#include <media/videobuf-vmalloc.h>
+#include <media/videobuf2-vmalloc.h>
 
 #include "atomisp_cmd.h"
 #include "atomisp_common.h"
@@ -35,49 +35,53 @@
 #include "atomisp-regs.h"
 #include "hmm/hmm.h"
 
+#include "ia_css_frame.h"
 #include "type_support.h"
 #include "device_access/device_access.h"
 
-#define ISP_LEFT_PAD			128	/* equal to 2*NWAY */
-
 /*
- * input image data, and current frame resolution for test
+ * Videobuf2 ops
  */
-#define	ISP_PARAM_MMAP_OFFSET	0xfffff000
+static int atomisp_queue_setup(struct vb2_queue *vq,
+			       unsigned int *nbuffers, unsigned int *nplanes,
+			       unsigned int sizes[], struct device *alloc_devs[])
+{
+	struct atomisp_video_pipe *pipe = container_of(vq, struct atomisp_video_pipe, vb_queue);
+	u16 source_pad = atomisp_subdev_source_pad(&pipe->vdev);
+	int ret;
 
-#define MAGIC_CHECK(is, should)	\
-	do { \
-		if (unlikely((is) != (should))) { \
-			pr_err("magic mismatch: %x (expected %x)\n", \
-				is, should); \
-			BUG(); \
-		} \
-	} while (0)
+	ret = atomisp_get_css_frame_info(pipe->asd, source_pad, &pipe->frame_info);
+	if (ret)
+		return ret;
 
-/*
- * Videobuf ops
- */
-static int atomisp_buf_setup(struct videobuf_queue *vq, unsigned int *count,
-			     unsigned int *size)
-{
-	struct atomisp_video_pipe *pipe = vq->priv_data;
+	atomisp_alloc_css_stat_bufs(pipe->asd, ATOMISP_INPUT_STREAM_GENERAL);
 
-	*size = pipe->pix.sizeimage;
+	*nplanes = 1;
+	sizes[0] = PAGE_ALIGN(pipe->pix.sizeimage);
 
 	return 0;
 }
 
-static int atomisp_buf_prepare(struct videobuf_queue *vq,
-			       struct videobuf_buffer *vb,
-			       enum v4l2_field field)
+static int atomisp_buf_init(struct vb2_buffer *vb)
 {
-	struct atomisp_video_pipe *pipe = vq->priv_data;
+	struct atomisp_video_pipe *pipe = vb_to_pipe(vb);
+	struct ia_css_frame *frame = vb_to_frame(vb);
+	int ret;
 
-	vb->size = pipe->pix.sizeimage;
-	vb->width = pipe->pix.width;
-	vb->height = pipe->pix.height;
-	vb->field = field;
-	vb->state = VIDEOBUF_PREPARED;
+	ret = ia_css_frame_init_from_info(frame, &pipe->frame_info);
+	if (ret)
+		return ret;
+
+	if (frame->data_bytes > vb2_plane_size(vb, 0)) {
+		dev_err(pipe->asd->isp->dev, "Internal error frame.data_bytes(%u) > vb.length(%lu)\n",
+			frame->data_bytes, vb2_plane_size(vb, 0));
+		return -EIO;
+	}
+
+	frame->data = hmm_create_from_vmalloc_buf(vb2_plane_size(vb, 0),
+						  vb2_plane_vaddr(vb, 0));
+	if (frame->data == mmgr_NULL)
+		return -ENOMEM;
 
 	return 0;
 }
@@ -212,7 +216,6 @@ static int atomisp_q_video_buffers_to_css(struct atomisp_sub_device *asd,
 					  enum ia_css_buffer_type css_buf_type,
 					  enum ia_css_pipe_id css_pipe_id)
 {
-	struct videobuf_vmalloc_memory *vm_mem;
 	struct atomisp_css_params_with_list *param;
 	struct ia_css_dvs_grid_info *dvs_grid =
 	    atomisp_css_get_dvs_grid_info(&asd->params.curr_grid_info);
@@ -229,26 +232,22 @@ static int atomisp_q_video_buffers_to_css(struct atomisp_sub_device *asd,
 
 	space = ATOMISP_CSS_Q_DEPTH - atomisp_buffers_in_css(pipe);
 	while (space--) {
-		struct videobuf_buffer *vb;
+		struct ia_css_frame *frame;
 
 		spin_lock_irqsave(&pipe->irq_lock, irqflags);
-		vb = list_first_entry_or_null(&pipe->activeq, struct videobuf_buffer, queue);
-		if (vb) {
-			list_move_tail(&vb->queue, &pipe->buffers_in_css);
-			vb->state = VIDEOBUF_ACTIVE;
-		}
+		frame = list_first_entry_or_null(&pipe->activeq, struct ia_css_frame, queue);
+		if (frame)
+			list_move_tail(&frame->queue, &pipe->buffers_in_css);
 		spin_unlock_irqrestore(&pipe->irq_lock, irqflags);
 
-		if (!vb)
+		if (!frame)
 			return -EINVAL;
 
 		/*
 		 * If there is a per_frame setting to apply on the buffer,
 		 * do it before buffer en-queueing.
 		 */
-		vm_mem = vb->priv;
-
-		param = pipe->frame_params[vb->i];
+		param = pipe->frame_params[frame->vb.vb2_buf.index];
 		if (param) {
 			atomisp_makeup_css_parameters(asd,
 						      &asd->params.css_param.update_flag,
@@ -262,8 +261,7 @@ static int atomisp_q_video_buffers_to_css(struct atomisp_sub_device *asd,
 				if (!err)
 					asd->params.config.dz_config = &param->params.dz_config;
 			}
-			atomisp_css_set_isp_config_applied_frame(asd,
-				vm_mem->vaddr);
+			atomisp_css_set_isp_config_applied_frame(asd, frame);
 			atomisp_css_update_isp_params_on_pipe(asd,
 							      asd->stream_env[stream_id].pipes[css_pipe_id]);
 			asd->params.dvs_6axis = (struct ia_css_dvs_6axis_config *)
@@ -288,14 +286,14 @@ static int atomisp_q_video_buffers_to_css(struct atomisp_sub_device *asd,
 				    &asd->params.css_param.dz_config;
 				asd->params.css_update_params_needed = true;
 			}
+			pipe->frame_params[frame->vb.vb2_buf.index] = NULL;
 		}
 		/* Enqueue buffer */
-		err = atomisp_q_video_buffer_to_css(asd, vm_mem, stream_id,
+		err = atomisp_q_video_buffer_to_css(asd, frame, stream_id,
 						    css_buf_type, css_pipe_id);
 		if (err) {
 			spin_lock_irqsave(&pipe->irq_lock, irqflags);
-			list_move_tail(&vb->queue, &pipe->activeq);
-			vb->state = VIDEOBUF_QUEUED;
+			list_move_tail(&frame->queue, &pipe->activeq);
 			spin_unlock_irqrestore(&pipe->irq_lock, irqflags);
 			dev_err(asd->isp->dev, "%s, css q fails: %d\n",
 				__func__, err);
@@ -522,11 +520,32 @@ int atomisp_qbuffers_to_css(struct atomisp_sub_device *asd)
 	return 0;
 }
 
-static void atomisp_buf_queue(struct videobuf_queue *vq,
-			      struct videobuf_buffer *vb)
+static void atomisp_buf_queue(struct vb2_buffer *vb)
 {
-	struct atomisp_video_pipe *pipe = vq->priv_data;
+	struct atomisp_video_pipe *pipe = vb_to_pipe(vb);
+	struct ia_css_frame *frame = vb_to_frame(vb);
+	struct atomisp_sub_device *asd = pipe->asd;
+	u16 source_pad = atomisp_subdev_source_pad(&pipe->vdev);
+	unsigned long irqflags;
+	int ret;
+
+	mutex_lock(&asd->isp->mutex);
 
+	ret = atomisp_pipe_check(pipe, false);
+	if (ret || pipe->stopping) {
+		spin_lock_irqsave(&pipe->irq_lock, irqflags);
+		atomisp_buffer_done(frame, VB2_BUF_STATE_ERROR);
+		spin_unlock_irqrestore(&pipe->irq_lock, irqflags);
+		goto unlock;
+	}
+
+	/* FIXME this ugliness comes from the original atomisp buffer handling */
+	if (!(vb->skip_cache_sync_on_finish && vb->skip_cache_sync_on_prepare))
+		wbinvd();
+
+	pipe->frame_params[vb->index] = NULL;
+
+	spin_lock_irqsave(&pipe->irq_lock, irqflags);
 	/*
 	 * when a frame buffer meets following conditions, it should be put into
 	 * the waiting list:
@@ -538,40 +557,83 @@ static void atomisp_buf_queue(struct videobuf_queue *vq,
 	 *     get enqueued.
 	 */
 	if (!atomisp_is_vf_pipe(pipe) &&
-	    (pipe->frame_request_config_id[vb->i] ||
+	    (pipe->frame_request_config_id[vb->index] ||
 	     !list_empty(&pipe->buffers_waiting_for_param)))
-		list_add_tail(&vb->queue, &pipe->buffers_waiting_for_param);
+		list_add_tail(&frame->queue, &pipe->buffers_waiting_for_param);
 	else
-		list_add_tail(&vb->queue, &pipe->activeq);
+		list_add_tail(&frame->queue, &pipe->activeq);
+
+	spin_unlock_irqrestore(&pipe->irq_lock, irqflags);
+
+	/* TODO: do this better, not best way to queue to css */
+	if (asd->streaming == ATOMISP_DEVICE_STREAMING_ENABLED) {
+		if (!list_empty(&pipe->buffers_waiting_for_param))
+			atomisp_handle_parameter_and_buffer(pipe);
+		else
+			atomisp_qbuffers_to_css(asd);
+	}
 
-	vb->state = VIDEOBUF_QUEUED;
+	/*
+	 * Workaround: Due to the design of HALv3,
+	 * sometimes in ZSL or SDV mode HAL needs to
+	 * capture multiple images within one streaming cycle.
+	 * But the capture number cannot be determined by HAL.
+	 * So HAL only sets the capture number to be 1 and queue multiple
+	 * buffers. Atomisp driver needs to check this case and re-trigger
+	 * CSS to do capture when new buffer is queued.
+	 */
+	if (asd->continuous_mode->val && source_pad == ATOMISP_SUBDEV_PAD_SOURCE_CAPTURE &&
+	    !asd->enable_raw_buffer_lock->val && asd->params.offline_parm.num_captures == 1) {
+		asd->pending_capture_request++;
+		dev_dbg(asd->isp->dev, "Add one pending capture request.\n");
+	}
+
+unlock:
+	mutex_unlock(&asd->isp->mutex);
 }
 
-static void atomisp_buf_release(struct videobuf_queue *vq,
-				struct videobuf_buffer *vb)
+static void atomisp_buf_cleanup(struct vb2_buffer *vb)
 {
-	vb->state = VIDEOBUF_NEEDS_INIT;
-	atomisp_videobuf_free_buf(vb);
+	struct atomisp_video_pipe *pipe = vb_to_pipe(vb);
+	struct ia_css_frame *frame = vb_to_frame(vb);
+	int index = frame->vb.vb2_buf.index;
+
+	pipe->frame_request_config_id[index] = 0;
+	pipe->frame_params[index] = NULL;
+
+	hmm_free(frame->data);
 }
 
-static const struct videobuf_queue_ops videobuf_qops = {
-	.buf_setup	= atomisp_buf_setup,
-	.buf_prepare	= atomisp_buf_prepare,
-	.buf_queue	= atomisp_buf_queue,
-	.buf_release	= atomisp_buf_release,
+static const struct vb2_ops atomisp_vb2_ops = {
+	.queue_setup		= atomisp_queue_setup,
+	.buf_init		= atomisp_buf_init,
+	.buf_cleanup		= atomisp_buf_cleanup,
+	.buf_queue		= atomisp_buf_queue,
+	.start_streaming	= atomisp_start_streaming,
+	.stop_streaming		= atomisp_stop_streaming,
 };
 
 static int atomisp_init_pipe(struct atomisp_video_pipe *pipe)
 {
+	int ret;
+
 	/* init locks */
 	spin_lock_init(&pipe->irq_lock);
+	mutex_init(&pipe->vb_queue_mutex);
+
+	/* Init videobuf2 queue structure */
+	pipe->vb_queue.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
+	pipe->vb_queue.io_modes = VB2_MMAP | VB2_USERPTR;
+	pipe->vb_queue.buf_struct_size = sizeof(struct ia_css_frame);
+	pipe->vb_queue.ops = &atomisp_vb2_ops;
+	pipe->vb_queue.mem_ops = &vb2_vmalloc_memops;
+	pipe->vb_queue.timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
+	ret = vb2_queue_init(&pipe->vb_queue);
+	if (ret)
+		return ret;
 
-	videobuf_queue_vmalloc_init(&pipe->capq, &videobuf_qops, NULL,
-				    &pipe->irq_lock,
-				    V4L2_BUF_TYPE_VIDEO_CAPTURE,
-				    V4L2_FIELD_NONE,
-				    sizeof(struct atomisp_buffer), pipe,
-				    NULL);	/* ext_lock: NULL */
+	pipe->vdev.queue = &pipe->vb_queue;
+	pipe->vdev.queue->lock = &pipe->vb_queue_mutex;
 
 	INIT_LIST_HEAD(&pipe->activeq);
 	INIT_LIST_HEAD(&pipe->buffers_waiting_for_param);
@@ -771,45 +833,35 @@ static int atomisp_release(struct file *file)
 	struct atomisp_device *isp = video_get_drvdata(vdev);
 	struct atomisp_video_pipe *pipe = atomisp_to_video_pipe(vdev);
 	struct atomisp_sub_device *asd = pipe->asd;
-	struct v4l2_requestbuffers req;
 	struct v4l2_subdev_fh fh;
 	struct v4l2_rect clear_compose = {0};
 	unsigned long flags;
-	int ret = 0;
+	int ret;
 
 	v4l2_fh_init(&fh.vfh, vdev);
 
-	req.count = 0;
-	if (!isp)
-		return -EBADF;
-
+	mutex_lock(&pipe->vb_queue_mutex);
 	mutex_lock(&isp->mutex);
 
 	dev_dbg(isp->dev, "release device %s\n", vdev->name);
 
 	asd->subdev.devnode = vdev;
 
-	pipe->users--;
-
-	if (pipe->capq.streaming)
-		dev_warn(isp->dev,
-			 "%s: ISP still streaming while closing!",
-			 __func__);
-
-	if (pipe->capq.streaming &&
-	    atomisp_streamoff(file, NULL, V4L2_BUF_TYPE_VIDEO_CAPTURE)) {
-		dev_err(isp->dev, "atomisp_streamoff failed on release, driver bug");
-		goto done;
+	/*
+	 * FIXME This if is copied from _vb2_fop_release, this cannot use that
+	 * because that calls v4l2_fh_release() earlier then this function.
+	 * Maybe we can release the fh earlier though, it does not look like
+	 * anything needs it after this.
+	 */
+	if (file->private_data == vdev->queue->owner) {
+		vb2_queue_release(vdev->queue);
+		vdev->queue->owner = NULL;
 	}
 
+	pipe->users--;
 	if (pipe->users)
 		goto done;
 
-	if (atomisp_reqbufs(file, NULL, &req)) {
-		dev_err(isp->dev, "atomisp_reqbufs failed on release, driver bug");
-		goto done;
-	}
-
 	/*
 	 * A little trick here:
 	 * file injection input resolution is recorded in the sink pad,
@@ -867,260 +919,17 @@ static int atomisp_release(struct file *file)
 				     V4L2_SEL_TGT_COMPOSE, 0,
 				     &clear_compose);
 	mutex_unlock(&isp->mutex);
+	mutex_unlock(&pipe->vb_queue_mutex);
 
 	return v4l2_fh_release(file);
 }
 
-/*
- * Memory help functions for image frame and private parameters
- */
-static int do_isp_mm_remap(struct atomisp_device *isp,
-			   struct vm_area_struct *vma,
-			   ia_css_ptr isp_virt, u32 host_virt, u32 pgnr)
-{
-	u32 pfn;
-
-	while (pgnr) {
-		pfn = hmm_virt_to_phys(isp_virt) >> PAGE_SHIFT;
-		if (remap_pfn_range(vma, host_virt, pfn,
-				    PAGE_SIZE, PAGE_SHARED)) {
-			dev_err(isp->dev, "remap_pfn_range err.\n");
-			return -EAGAIN;
-		}
-
-		isp_virt += PAGE_SIZE;
-		host_virt += PAGE_SIZE;
-		pgnr--;
-	}
-
-	return 0;
-}
-
-static int frame_mmap(struct atomisp_device *isp,
-		      const struct ia_css_frame *frame, struct vm_area_struct *vma)
-{
-	ia_css_ptr isp_virt;
-	u32 host_virt;
-	u32 pgnr;
-
-	if (!frame) {
-		dev_err(isp->dev, "%s: NULL frame pointer.\n", __func__);
-		return -EINVAL;
-	}
-
-	host_virt = vma->vm_start;
-	isp_virt = frame->data;
-	pgnr = DIV_ROUND_UP(frame->data_bytes, PAGE_SIZE);
-
-	if (do_isp_mm_remap(isp, vma, isp_virt, host_virt, pgnr))
-		return -EAGAIN;
-
-	return 0;
-}
-
-int atomisp_videobuf_mmap_mapper(struct videobuf_queue *q,
-				 struct vm_area_struct *vma)
-{
-	u32 offset = vma->vm_pgoff << PAGE_SHIFT;
-	int ret = -EINVAL, i;
-	struct atomisp_device *isp =
-	    ((struct atomisp_video_pipe *)(q->priv_data))->isp;
-	struct videobuf_vmalloc_memory *vm_mem;
-	struct videobuf_mapping *map;
-
-	MAGIC_CHECK(q->int_ops->magic, MAGIC_QTYPE_OPS);
-	if (!(vma->vm_flags & VM_WRITE) || !(vma->vm_flags & VM_SHARED)) {
-		dev_err(isp->dev, "map appl bug: PROT_WRITE and MAP_SHARED are required\n");
-		return -EINVAL;
-	}
-
-	mutex_lock(&q->vb_lock);
-	for (i = 0; i < VIDEO_MAX_FRAME; i++) {
-		struct videobuf_buffer *buf = q->bufs[i];
-
-		if (!buf)
-			continue;
-
-		map = kzalloc(sizeof(struct videobuf_mapping), GFP_KERNEL);
-		if (!map) {
-			mutex_unlock(&q->vb_lock);
-			return -ENOMEM;
-		}
-
-		buf->map = map;
-		map->q = q;
-
-		buf->baddr = vma->vm_start;
-
-		if (buf && buf->memory == V4L2_MEMORY_MMAP &&
-		    buf->boff == offset) {
-			vm_mem = buf->priv;
-			ret = frame_mmap(isp, vm_mem->vaddr, vma);
-			vma->vm_flags |= VM_IO | VM_DONTEXPAND | VM_DONTDUMP;
-			break;
-		}
-	}
-	mutex_unlock(&q->vb_lock);
-
-	return ret;
-}
-
-/* The input frame contains left and right padding that need to be removed.
- * There is always ISP_LEFT_PAD padding on the left side.
- * There is also padding on the right (padded_width - width).
- */
-static int remove_pad_from_frame(struct atomisp_device *isp,
-				 struct ia_css_frame *in_frame, __u32 width, __u32 height)
-{
-	unsigned int i;
-	unsigned short *buffer;
-	int ret = 0;
-	ia_css_ptr load = in_frame->data;
-	ia_css_ptr store = load;
-
-	buffer = kmalloc_array(width, sizeof(load), GFP_KERNEL);
-	if (!buffer)
-		return -ENOMEM;
-
-	load += ISP_LEFT_PAD;
-	for (i = 0; i < height; i++) {
-		ret = hmm_load(load, buffer, width * sizeof(load));
-		if (ret < 0)
-			goto remove_pad_error;
-
-		ret = hmm_store(store, buffer, width * sizeof(store));
-		if (ret < 0)
-			goto remove_pad_error;
-
-		load  += in_frame->frame_info.padded_width;
-		store += width;
-	}
-
-remove_pad_error:
-	kfree(buffer);
-	return ret;
-}
-
-static int atomisp_mmap(struct file *file, struct vm_area_struct *vma)
-{
-	struct video_device *vdev = video_devdata(file);
-	struct atomisp_device *isp = video_get_drvdata(vdev);
-	struct atomisp_video_pipe *pipe = atomisp_to_video_pipe(vdev);
-	struct atomisp_sub_device *asd = pipe->asd;
-	struct ia_css_frame *raw_virt_addr;
-	u32 start = vma->vm_start;
-	u32 end = vma->vm_end;
-	u32 size = end - start;
-	u32 origin_size, new_size;
-	int ret;
-
-	if (!asd) {
-		dev_err(isp->dev, "%s(): asd is NULL, device is %s\n",
-			__func__, vdev->name);
-		return -EINVAL;
-	}
-
-	if (!(vma->vm_flags & (VM_WRITE | VM_READ)))
-		return -EACCES;
-
-	mutex_lock(&isp->mutex);
-
-	if (!(vma->vm_flags & VM_SHARED)) {
-		/* Map private buffer.
-		 * Set VM_SHARED to the flags since we need
-		 * to map the buffer page by page.
-		 * Without VM_SHARED, remap_pfn_range() treats
-		 * this kind of mapping as invalid.
-		 */
-		vma->vm_flags |= VM_SHARED;
-		ret = hmm_mmap(vma, vma->vm_pgoff << PAGE_SHIFT);
-		mutex_unlock(&isp->mutex);
-		return ret;
-	}
-
-	/* mmap for ISP offline raw data */
-	if (atomisp_subdev_source_pad(vdev)
-	    == ATOMISP_SUBDEV_PAD_SOURCE_CAPTURE &&
-	    vma->vm_pgoff == (ISP_PARAM_MMAP_OFFSET >> PAGE_SHIFT)) {
-		new_size = pipe->pix.width * pipe->pix.height * 2;
-		if (asd->params.online_process != 0) {
-			ret = -EINVAL;
-			goto error;
-		}
-		raw_virt_addr = asd->raw_output_frame;
-		if (!raw_virt_addr) {
-			dev_err(isp->dev, "Failed to request RAW frame\n");
-			ret = -EINVAL;
-			goto error;
-		}
-
-		ret = remove_pad_from_frame(isp, raw_virt_addr,
-					    pipe->pix.width, pipe->pix.height);
-		if (ret < 0) {
-			dev_err(isp->dev, "remove pad failed.\n");
-			goto error;
-		}
-		origin_size = raw_virt_addr->data_bytes;
-		raw_virt_addr->data_bytes = new_size;
-
-		if (size != PAGE_ALIGN(new_size)) {
-			dev_err(isp->dev, "incorrect size for mmap ISP  Raw Frame\n");
-			ret = -EINVAL;
-			goto error;
-		}
-
-		if (frame_mmap(isp, raw_virt_addr, vma)) {
-			dev_err(isp->dev, "frame_mmap failed.\n");
-			raw_virt_addr->data_bytes = origin_size;
-			ret = -EAGAIN;
-			goto error;
-		}
-		raw_virt_addr->data_bytes = origin_size;
-		vma->vm_flags |= VM_IO | VM_DONTEXPAND | VM_DONTDUMP;
-		mutex_unlock(&isp->mutex);
-		return 0;
-	}
-
-	/*
-	 * mmap for normal frames
-	 */
-	if (size != pipe->pix.sizeimage) {
-		dev_err(isp->dev, "incorrect size for mmap ISP frames\n");
-		ret = -EINVAL;
-		goto error;
-	}
-	mutex_unlock(&isp->mutex);
-
-	return atomisp_videobuf_mmap_mapper(&pipe->capq, vma);
-
-error:
-	mutex_unlock(&isp->mutex);
-
-	return ret;
-}
-
-static __poll_t atomisp_poll(struct file *file,
-			     struct poll_table_struct *pt)
-{
-	struct video_device *vdev = video_devdata(file);
-	struct atomisp_device *isp = video_get_drvdata(vdev);
-	struct atomisp_video_pipe *pipe = atomisp_to_video_pipe(vdev);
-
-	mutex_lock(&isp->mutex);
-	if (pipe->capq.streaming != 1) {
-		mutex_unlock(&isp->mutex);
-		return EPOLLERR;
-	}
-	mutex_unlock(&isp->mutex);
-
-	return videobuf_poll_stream(file, &pipe->capq, pt);
-}
-
 const struct v4l2_file_operations atomisp_fops = {
 	.owner = THIS_MODULE,
 	.open = atomisp_open,
 	.release = atomisp_release,
-	.mmap = atomisp_mmap,
+	.mmap = vb2_fop_mmap,
+	.poll = vb2_fop_poll,
 	.unlocked_ioctl = video_ioctl2,
 #ifdef CONFIG_COMPAT
 	/*
@@ -1129,5 +938,4 @@ const struct v4l2_file_operations atomisp_fops = {
 	.compat_ioctl32 = atomisp_compat_ioctl32,
 	 */
 #endif
-	.poll = atomisp_poll,
 };
diff --git a/drivers/staging/media/atomisp/pci/atomisp_fops.h b/drivers/staging/media/atomisp/pci/atomisp_fops.h
index 8d3ea33a7d9a..10e43126b693 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_fops.h
+++ b/drivers/staging/media/atomisp/pci/atomisp_fops.h
@@ -29,13 +29,6 @@ unsigned int atomisp_sub_dev_users(struct atomisp_sub_device *asd);
  * Memory help functions for image frame and private parameters
  */
 
-int atomisp_videobuf_mmap_mapper(struct videobuf_queue *q,
-				 struct vm_area_struct *vma);
-
-int atomisp_qbuf_to_css(struct atomisp_device *isp,
-			struct atomisp_video_pipe *pipe,
-			struct videobuf_buffer *vb);
-
 int atomisp_qbuffers_to_css(struct atomisp_sub_device *asd);
 
 extern const struct v4l2_file_operations atomisp_fops;
diff --git a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
index 7d5413981714..e850de069f8f 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
@@ -23,7 +23,6 @@
 
 #include <media/v4l2-ioctl.h>
 #include <media/v4l2-event.h>
-#include <media/videobuf-vmalloc.h>
 
 #include "atomisp_cmd.h"
 #include "atomisp_common.h"
@@ -542,6 +541,11 @@ int atomisp_pipe_check(struct atomisp_video_pipe *pipe, bool settings_change)
 	if (pipe->isp->isp_fatal_error)
 		return -EIO;
 
+	if (settings_change && vb2_is_busy(&pipe->vb_queue)) {
+		dev_err(pipe->isp->dev, "Set fmt/input IOCTL while streaming\n");
+		return -EBUSY;
+	}
+
 	switch (pipe->asd->streaming) {
 	case ATOMISP_DEVICE_STREAMING_DISABLED:
 		break;
@@ -632,10 +636,10 @@ static int atomisp_enum_input(struct file *file, void *fh,
 static unsigned int
 atomisp_subdev_streaming_count(struct atomisp_sub_device *asd)
 {
-	return asd->video_out_preview.capq.streaming
-	       + asd->video_out_capture.capq.streaming
-	       + asd->video_out_video_capture.capq.streaming
-	       + asd->video_out_vf.capq.streaming;
+	return asd->video_out_preview.vb_queue.start_streaming_called
+	       + asd->video_out_capture.vb_queue.start_streaming_called
+	       + asd->video_out_video_capture.vb_queue.start_streaming_called
+	       + asd->video_out_vf.vb_queue.start_streaming_called;
 }
 
 unsigned int atomisp_streaming_count(struct atomisp_device *isp)
@@ -968,37 +972,6 @@ static int atomisp_g_fmt_cap(struct file *file, void *fh,
 	return atomisp_try_fmt_cap(file, fh, f);
 }
 
-/*
- * Free videobuffer buffer priv data
- */
-void atomisp_videobuf_free_buf(struct videobuf_buffer *vb)
-{
-	struct videobuf_vmalloc_memory *vm_mem;
-
-	if (!vb)
-		return;
-
-	vm_mem = vb->priv;
-	if (vm_mem && vm_mem->vaddr) {
-		ia_css_frame_free(vm_mem->vaddr);
-		vm_mem->vaddr = NULL;
-	}
-}
-
-/*
- * this function is used to free video buffer queue
- */
-static void atomisp_videobuf_free_queue(struct videobuf_queue *q)
-{
-	int i;
-
-	for (i = 0; i < VIDEO_MAX_FRAME; i++) {
-		atomisp_videobuf_free_buf(q->bufs[i]);
-		kfree(q->bufs[i]);
-		q->bufs[i] = NULL;
-	}
-}
-
 int atomisp_alloc_css_stat_bufs(struct atomisp_sub_device *asd,
 				uint16_t stream_id)
 {
@@ -1100,178 +1073,13 @@ int atomisp_alloc_css_stat_bufs(struct atomisp_sub_device *asd,
 	return -ENOMEM;
 }
 
-/*
- * Initiate Memory Mapping or User Pointer I/O
- */
-int atomisp_reqbufs(struct file *file, void *fh, struct v4l2_requestbuffers *req)
-{
-	struct video_device *vdev = video_devdata(file);
-	struct atomisp_video_pipe *pipe = atomisp_to_video_pipe(vdev);
-	struct atomisp_sub_device *asd = pipe->asd;
-	struct ia_css_frame_info frame_info;
-	struct ia_css_frame *frame;
-	struct videobuf_vmalloc_memory *vm_mem;
-	u16 source_pad = atomisp_subdev_source_pad(vdev);
-	int ret = 0, i = 0;
-
-	if (req->count == 0) {
-		mutex_lock(&pipe->capq.vb_lock);
-		if (!list_empty(&pipe->capq.stream))
-			videobuf_queue_cancel(&pipe->capq);
-
-		atomisp_videobuf_free_queue(&pipe->capq);
-		mutex_unlock(&pipe->capq.vb_lock);
-		/* clear request config id */
-		memset(pipe->frame_request_config_id, 0,
-		       VIDEO_MAX_FRAME * sizeof(unsigned int));
-		memset(pipe->frame_params, 0,
-		       VIDEO_MAX_FRAME *
-		       sizeof(struct atomisp_css_params_with_list *));
-		return 0;
-	}
-
-	ret = videobuf_reqbufs(&pipe->capq, req);
-	if (ret)
-		return ret;
-
-	atomisp_alloc_css_stat_bufs(asd, ATOMISP_INPUT_STREAM_GENERAL);
-
-	/*
-	 * for user pointer type, buffers are not really allocated here,
-	 * buffers are setup in QBUF operation through v4l2_buffer structure
-	 */
-	if (req->memory == V4L2_MEMORY_USERPTR)
-		return 0;
-
-	ret = atomisp_get_css_frame_info(asd, source_pad, &frame_info);
-	if (ret)
-		return ret;
-
-	/*
-	 * Allocate the real frame here for selected node using our
-	 * memory management function
-	 */
-	for (i = 0; i < req->count; i++) {
-		if (ia_css_frame_allocate_from_info(&frame, &frame_info))
-			goto error;
-		vm_mem = pipe->capq.bufs[i]->priv;
-		vm_mem->vaddr = frame;
-	}
-
-	return ret;
-
-error:
-	while (i--) {
-		vm_mem = pipe->capq.bufs[i]->priv;
-		ia_css_frame_free(vm_mem->vaddr);
-	}
-
-	if (asd->vf_frame)
-		ia_css_frame_free(asd->vf_frame);
-
-	return -ENOMEM;
-}
-
-/* application query the status of a buffer */
-static int atomisp_querybuf(struct file *file, void *fh,
-			    struct v4l2_buffer *buf)
+static int atomisp_qbuf_wrapper(struct file *file, void *fh, struct v4l2_buffer *buf)
 {
 	struct video_device *vdev = video_devdata(file);
-	struct atomisp_video_pipe *pipe = atomisp_to_video_pipe(vdev);
-
-	return videobuf_querybuf(&pipe->capq, buf);
-}
-
-/*
- * Applications call the VIDIOC_QBUF ioctl to enqueue an empty (capturing) or
- * filled (output) buffer in the drivers incoming queue.
- */
-static int atomisp_qbuf(struct file *file, void *fh, struct v4l2_buffer *buf)
-{
-	static const int NOFLUSH_FLAGS = V4L2_BUF_FLAG_NO_CACHE_INVALIDATE |
-					 V4L2_BUF_FLAG_NO_CACHE_CLEAN;
-	struct video_device *vdev = video_devdata(file);
 	struct atomisp_device *isp = video_get_drvdata(vdev);
 	struct atomisp_video_pipe *pipe = atomisp_to_video_pipe(vdev);
-	struct atomisp_sub_device *asd = pipe->asd;
-	struct videobuf_buffer *vb;
-	struct videobuf_vmalloc_memory *vm_mem;
-	struct ia_css_frame_info frame_info;
-	struct ia_css_frame *handle = NULL;
-	u32 length;
-	u32 pgnr;
-	int ret;
-
-	ret = atomisp_pipe_check(pipe, false);
-	if (ret)
-		return ret;
-
-	if (!buf || buf->index >= VIDEO_MAX_FRAME ||
-	    !pipe->capq.bufs[buf->index]) {
-		dev_err(isp->dev, "Invalid index for qbuf.\n");
-		return -EINVAL;
-	}
-
-	/*
-	 * For userptr type frame, we convert user space address to physic
-	 * address and reprograme out page table properly
-	 */
-	if (buf->memory == V4L2_MEMORY_USERPTR) {
-		if (offset_in_page(buf->m.userptr)) {
-			dev_err(isp->dev, "Error userptr is not page aligned.\n");
-			return -EINVAL;
-		}
-
-		vb = pipe->capq.bufs[buf->index];
-		vm_mem = vb->priv;
-		if (!vm_mem)
-			return -EINVAL;
-
-		length = vb->bsize;
-		pgnr = (length + (PAGE_SIZE - 1)) >> PAGE_SHIFT;
-
-		if (vb->baddr == buf->m.userptr && vm_mem->vaddr)
-			goto done;
-
-		if (atomisp_get_css_frame_info(asd,
-					       atomisp_subdev_source_pad(vdev), &frame_info))
-			return -EIO;
-
-		ret = ia_css_frame_map(&handle, &frame_info,
-					    (void __user *)buf->m.userptr,
-					    pgnr);
-		if (ret) {
-			dev_err(isp->dev, "Failed to map user buffer\n");
-			return ret;
-		}
-
-		if (vm_mem->vaddr) {
-			mutex_lock(&pipe->capq.vb_lock);
-			ia_css_frame_free(vm_mem->vaddr);
-			vm_mem->vaddr = NULL;
-			vb->state = VIDEOBUF_NEEDS_INIT;
-			mutex_unlock(&pipe->capq.vb_lock);
-		}
-
-		vm_mem->vaddr = handle;
-
-		buf->flags &= ~V4L2_BUF_FLAG_MAPPED;
-		buf->flags |= V4L2_BUF_FLAG_QUEUED;
-		buf->flags &= ~V4L2_BUF_FLAG_DONE;
-	} else if (buf->memory == V4L2_MEMORY_MMAP) {
-		buf->flags |= V4L2_BUF_FLAG_MAPPED;
-		buf->flags |= V4L2_BUF_FLAG_QUEUED;
-		buf->flags &= ~V4L2_BUF_FLAG_DONE;
-
-		/*
-		 * For mmap, frames were allocated at request buffers
-		 */
-	}
-
-done:
-	if (!((buf->flags & NOFLUSH_FLAGS) == NOFLUSH_FLAGS))
-		wbinvd();
 
+	/* FIXME this abuse of buf->reserved2 comes from the original atomisp buffer handling */
 	if (!atomisp_is_vf_pipe(pipe) &&
 	    (buf->reserved2 & ATOMISP_BUFFER_HAS_PER_FRAME_SETTING)) {
 		/* this buffer will have a per-frame parameter */
@@ -1284,90 +1092,27 @@ static int atomisp_qbuf(struct file *file, void *fh, struct v4l2_buffer *buf)
 		pipe->frame_request_config_id[buf->index] = 0;
 	}
 
-	pipe->frame_params[buf->index] = NULL;
-
-	mutex_unlock(&isp->mutex);
-	ret = videobuf_qbuf(&pipe->capq, buf);
-	mutex_lock(&isp->mutex);
-	if (ret)
-		return ret;
-
-	/* TODO: do this better, not best way to queue to css */
-	if (asd->streaming == ATOMISP_DEVICE_STREAMING_ENABLED) {
-		if (!list_empty(&pipe->buffers_waiting_for_param)) {
-			atomisp_handle_parameter_and_buffer(pipe);
-		} else {
-			atomisp_qbuffers_to_css(asd);
-		}
-	}
-
-	/*
-	 * Workaround: Due to the design of HALv3,
-	 * sometimes in ZSL or SDV mode HAL needs to
-	 * capture multiple images within one streaming cycle.
-	 * But the capture number cannot be determined by HAL.
-	 * So HAL only sets the capture number to be 1 and queue multiple
-	 * buffers. Atomisp driver needs to check this case and re-trigger
-	 * CSS to do capture when new buffer is queued.
-	 */
-	if (asd->continuous_mode->val &&
-	    atomisp_subdev_source_pad(vdev)
-	    == ATOMISP_SUBDEV_PAD_SOURCE_CAPTURE &&
-	    pipe->capq.streaming &&
-	    !asd->enable_raw_buffer_lock->val &&
-	    asd->params.offline_parm.num_captures == 1) {
-			asd->pending_capture_request++;
-			dev_dbg(isp->dev, "Add one pending capture request.\n");
-	}
-
-	dev_dbg(isp->dev, "qbuf buffer %d (%s) for asd%d\n", buf->index,
-		vdev->name, asd->index);
-
-	return 0;
+	return vb2_ioctl_qbuf(file, fh, buf);
 }
 
-static int __get_frame_exp_id(struct atomisp_video_pipe *pipe,
-			      struct v4l2_buffer *buf)
-{
-	struct videobuf_vmalloc_memory *vm_mem;
-	struct ia_css_frame *handle;
-	int i;
-
-	for (i = 0; pipe->capq.bufs[i]; i++) {
-		vm_mem = pipe->capq.bufs[i]->priv;
-		handle = vm_mem->vaddr;
-		if (buf->index == pipe->capq.bufs[i]->i && handle)
-			return handle->exp_id;
-	}
-	return -EINVAL;
-}
-
-/*
- * Applications call the VIDIOC_DQBUF ioctl to dequeue a filled (capturing) or
- * displayed (output buffer)from the driver's outgoing queue
- */
-static int atomisp_dqbuf(struct file *file, void *fh, struct v4l2_buffer *buf)
+static int atomisp_dqbuf_wrapper(struct file *file, void *fh, struct v4l2_buffer *buf)
 {
 	struct video_device *vdev = video_devdata(file);
 	struct atomisp_video_pipe *pipe = atomisp_to_video_pipe(vdev);
 	struct atomisp_sub_device *asd = pipe->asd;
 	struct atomisp_device *isp = video_get_drvdata(vdev);
+	struct ia_css_frame *frame;
+	struct vb2_buffer *vb;
 	int ret;
 
-	ret = atomisp_pipe_check(pipe, false);
+	ret = vb2_ioctl_dqbuf(file, fh, buf);
 	if (ret)
 		return ret;
 
-	mutex_unlock(&isp->mutex);
-	ret = videobuf_dqbuf(&pipe->capq, buf, file->f_flags & O_NONBLOCK);
-	mutex_lock(&isp->mutex);
-	if (ret) {
-		if (ret != -EAGAIN)
-			dev_dbg(isp->dev, "<%s: %d\n", __func__, ret);
-		return ret;
-	}
+	vb = pipe->vb_queue.bufs[buf->index];
+	frame = vb_to_frame(vb);
 
-	buf->bytesused = pipe->pix.sizeimage;
+	/* FIXME this abuse of buf->reserved* comes from the original atomisp buffer handling */
 	buf->reserved = asd->frame_status[buf->index];
 
 	/*
@@ -1378,7 +1123,7 @@ static int atomisp_dqbuf(struct file *file, void *fh, struct v4l2_buffer *buf)
 	 */
 	buf->reserved &= 0x0000ffff;
 	if (!(buf->flags & V4L2_BUF_FLAG_ERROR))
-		buf->reserved |= __get_frame_exp_id(pipe, buf) << 16;
+		buf->reserved |= frame->exp_id;
 	buf->reserved2 = pipe->frame_config_id[buf->index];
 
 	dev_dbg(isp->dev,
@@ -1506,36 +1251,26 @@ static void atomisp_dma_burst_len_cfg(struct atomisp_sub_device *asd)
 		atomisp_css2_hw_store_32(DMA_BURST_SIZE_REG, 0x00);
 }
 
-/*
- * This ioctl start the capture during streaming I/O.
- */
-static int atomisp_streamon(struct file *file, void *fh,
-			    enum v4l2_buf_type type)
+int atomisp_start_streaming(struct vb2_queue *vq, unsigned int count)
 {
-	struct video_device *vdev = video_devdata(file);
-	struct atomisp_video_pipe *pipe = atomisp_to_video_pipe(vdev);
+	struct atomisp_video_pipe *pipe = vq_to_pipe(vq);
 	struct atomisp_sub_device *asd = pipe->asd;
-	struct atomisp_device *isp = video_get_drvdata(vdev);
+	struct video_device *vdev = &pipe->vdev;
+	struct atomisp_device *isp = asd->isp;
 	struct pci_dev *pdev = to_pci_dev(isp->dev);
 	enum ia_css_pipe_id css_pipe_id;
 	unsigned int sensor_start_stream;
 	unsigned long irqflags;
 	int ret;
 
+	mutex_lock(&isp->mutex);
+
 	dev_dbg(isp->dev, "Start stream on pad %d for asd%d\n",
 		atomisp_subdev_source_pad(vdev), asd->index);
 
-	if (type != V4L2_BUF_TYPE_VIDEO_CAPTURE) {
-		dev_dbg(isp->dev, "unsupported v4l2 buf type\n");
-		return -EINVAL;
-	}
-
 	ret = atomisp_pipe_check(pipe, false);
 	if (ret)
-		return ret;
-
-	if (pipe->capq.streaming)
-		return 0;
+		goto out;
 
 	/* Input system HW workaround */
 	atomisp_dma_burst_len_cfg(asd);
@@ -1546,18 +1281,6 @@ static int atomisp_streamon(struct file *file, void *fh,
 	 */
 	sensor_start_stream = atomisp_sensor_start_stream(asd);
 
-	spin_lock_irqsave(&pipe->irq_lock, irqflags);
-	if (list_empty(&pipe->capq.stream)) {
-		spin_unlock_irqrestore(&pipe->irq_lock, irqflags);
-		dev_dbg(isp->dev, "no buffer in the queue\n");
-		return -EINVAL;
-	}
-	spin_unlock_irqrestore(&pipe->irq_lock, irqflags);
-
-	ret = videobuf_streamon(&pipe->capq);
-	if (ret)
-		return ret;
-
 	/* Reset pending capture request count. */
 	asd->pending_capture_request = 0;
 
@@ -1578,8 +1301,10 @@ static int atomisp_streamon(struct file *file, void *fh,
 				mutex_unlock(&isp->mutex);
 				ret = wait_for_completion_interruptible(&asd->init_done);
 				mutex_lock(&isp->mutex);
-				if (ret != 0)
-					return -ERESTARTSYS;
+				if (ret) {
+					ret = -ERESTARTSYS;
+					goto out;
+				}
 			}
 
 			/* handle per_frame_setting parameter and buffers */
@@ -1601,12 +1326,15 @@ static int atomisp_streamon(struct file *file, void *fh,
 					asd->params.offline_parm.num_captures,
 					asd->params.offline_parm.skip_frames,
 					asd->params.offline_parm.offset);
-				if (ret)
-					return -EINVAL;
+				if (ret) {
+					ret = -EINVAL;
+					goto out;
+				}
 			}
 		}
 		atomisp_qbuffers_to_css(asd);
-		return 0;
+		ret = 0;
+		goto out;
 	}
 
 	if (asd->streaming == ATOMISP_DEVICE_STREAMING_ENABLED) {
@@ -1632,7 +1360,7 @@ static int atomisp_streamon(struct file *file, void *fh,
 
 	ret = atomisp_css_start(asd, css_pipe_id, false);
 	if (ret)
-		return ret;
+		goto out;
 
 	spin_lock_irqsave(&isp->lock, irqflags);
 	asd->streaming = ATOMISP_DEVICE_STREAMING_ENABLED;
@@ -1652,8 +1380,10 @@ static int atomisp_streamon(struct file *file, void *fh,
 	atomisp_qbuffers_to_css(asd);
 
 	/* Only start sensor when the last streaming instance started */
-	if (atomisp_subdev_streaming_count(asd) < sensor_start_stream)
-		return 0;
+	if (atomisp_subdev_streaming_count(asd) < sensor_start_stream) {
+		ret = 0;
+		goto out;
+	}
 
 start_sensor:
 	if (isp->flash) {
@@ -1684,7 +1414,7 @@ static int atomisp_streamon(struct file *file, void *fh,
 		ret = atomisp_stream_on_master_slave_sensor(isp, false);
 		if (ret) {
 			dev_err(isp->dev, "master slave sensor stream on failed!\n");
-			return ret;
+			goto out;
 		}
 		goto start_delay_wq;
 	} else if (asd->depth_mode->val && (atomisp_streaming_count(isp) <
@@ -1706,7 +1436,8 @@ static int atomisp_streamon(struct file *file, void *fh,
 		spin_lock_irqsave(&isp->lock, irqflags);
 		asd->streaming = ATOMISP_DEVICE_STREAMING_DISABLED;
 		spin_unlock_irqrestore(&isp->lock, irqflags);
-		return -EINVAL;
+		ret = -EINVAL;
+		goto out;
 	}
 
 start_delay_wq:
@@ -1724,31 +1455,28 @@ static int atomisp_streamon(struct file *file, void *fh,
 		asd->delayed_init = ATOMISP_DELAYED_INIT_NOT_QUEUED;
 	}
 
-	return 0;
+out:
+	mutex_unlock(&isp->mutex);
+	return ret;
 }
 
-int atomisp_streamoff(struct file *file, void *fh, enum v4l2_buf_type type)
+void atomisp_stop_streaming(struct vb2_queue *vq)
 {
-	struct video_device *vdev = video_devdata(file);
-	struct atomisp_device *isp = video_get_drvdata(vdev);
-	struct pci_dev *pdev = to_pci_dev(isp->dev);
-	struct atomisp_video_pipe *pipe = atomisp_to_video_pipe(vdev);
+	struct atomisp_video_pipe *pipe = vq_to_pipe(vq);
 	struct atomisp_sub_device *asd = pipe->asd;
+	struct video_device *vdev = &pipe->vdev;
+	struct atomisp_device *isp = asd->isp;
+	struct pci_dev *pdev = to_pci_dev(isp->dev);
 	enum ia_css_pipe_id css_pipe_id;
 	int ret;
 	unsigned long flags;
 	bool first_streamoff = false;
 
+	mutex_lock(&isp->mutex);
+
 	dev_dbg(isp->dev, "Stop stream on pad %d for asd%d\n",
 		atomisp_subdev_source_pad(vdev), asd->index);
 
-	lockdep_assert_held(&isp->mutex);
-
-	if (type != V4L2_BUF_TYPE_VIDEO_CAPTURE) {
-		dev_dbg(isp->dev, "unsupported v4l2 buf type\n");
-		return -EINVAL;
-	}
-
 	/*
 	 * There is no guarantee that the buffers queued to / owned by the ISP
 	 * will properly be returned to the queue when stopping. Set a flag to
@@ -1758,12 +1486,12 @@ int atomisp_streamoff(struct file *file, void *fh, enum v4l2_buf_type type)
 	pipe->stopping = true;
 	mutex_unlock(&isp->mutex);
 	/* wait max 1 second */
-	ret = wait_event_interruptible_timeout(pipe->capq.wait,
-					       atomisp_buffers_in_css(pipe) == 0, HZ);
+	ret = wait_event_timeout(pipe->vb_queue.done_wq,
+				 atomisp_buffers_in_css(pipe) == 0, HZ);
 	mutex_lock(&isp->mutex);
 	pipe->stopping = false;
-	if (ret <= 0)
-		return ret ? ret : -ETIME;
+	if (ret == 0)
+		dev_warn(isp->dev, "Warning timeout waiting for CSS to return buffers\n");
 
 	/*
 	 * do only videobuf_streamoff for capture & vf pipes in
@@ -1780,12 +1508,9 @@ int atomisp_streamoff(struct file *file, void *fh, enum v4l2_buf_type type)
 			atomisp_freq_scaling(isp, ATOMISP_DFS_MODE_AUTO, false);
 		}
 
-		return videobuf_streamoff(&pipe->capq);
+		goto out;
 	}
 
-	if (!pipe->capq.streaming)
-		return 0;
-
 	if (asd->streaming == ATOMISP_DEVICE_STREAMING_ENABLED)
 		first_streamoff = true;
 
@@ -1796,12 +1521,8 @@ int atomisp_streamoff(struct file *file, void *fh, enum v4l2_buf_type type)
 		asd->streaming = ATOMISP_DEVICE_STREAMING_STOPPING;
 	spin_unlock_irqrestore(&isp->lock, flags);
 
-	if (!first_streamoff) {
-		ret = videobuf_streamoff(&pipe->capq);
-		if (ret)
-			return ret;
+	if (!first_streamoff)
 		goto stopsensor;
-	}
 
 	atomisp_clear_css_buffer_counters(asd);
 	atomisp_css_irq_enable(isp, IA_CSS_IRQ_INFO_CSS_RECEIVER_SOF, false);
@@ -1816,15 +1537,10 @@ int atomisp_streamoff(struct file *file, void *fh, enum v4l2_buf_type type)
 
 	atomisp_flush_video_pipe(pipe, true);
 
-	ret = videobuf_streamoff(&pipe->capq);
-	if (ret)
-		return ret;
-
 	atomisp_subdev_cleanup_pending_events(asd);
 stopsensor:
-	if (atomisp_subdev_streaming_count(asd) + 1
-	    != atomisp_sensor_start_stream(asd))
-		return 0;
+	if (atomisp_subdev_streaming_count(asd) != atomisp_sensor_start_stream(asd))
+		goto out;
 
 	ret = v4l2_subdev_call(isp->inputs[asd->input_curr].camera,
 			       video, s_stream, 0);
@@ -1837,7 +1553,7 @@ int atomisp_streamoff(struct file *file, void *fh, enum v4l2_buf_type type)
 	/* if other streams are running, isp should not be powered off */
 	if (atomisp_streaming_count(isp)) {
 		atomisp_css_flush(isp);
-		return 0;
+		goto out;
 	}
 
 	/* Disable the CSI interface on ANN B0/K0 */
@@ -1896,7 +1612,8 @@ int atomisp_streamoff(struct file *file, void *fh, enum v4l2_buf_type type)
 		}
 		isp->isp_timeout = false;
 	}
-	return ret;
+out:
+	mutex_unlock(&isp->mutex);
 }
 
 /*
@@ -2685,12 +2402,12 @@ const struct v4l2_ioctl_ops atomisp_ioctl_ops = {
 	.vidioc_try_fmt_vid_cap = atomisp_try_fmt_cap,
 	.vidioc_g_fmt_vid_cap = atomisp_g_fmt_cap,
 	.vidioc_s_fmt_vid_cap = atomisp_set_fmt,
-	.vidioc_reqbufs = atomisp_reqbufs,
-	.vidioc_querybuf = atomisp_querybuf,
-	.vidioc_qbuf = atomisp_qbuf,
-	.vidioc_dqbuf = atomisp_dqbuf,
-	.vidioc_streamon = atomisp_streamon,
-	.vidioc_streamoff = atomisp_streamoff,
+	.vidioc_reqbufs = vb2_ioctl_reqbufs,
+	.vidioc_querybuf = vb2_ioctl_querybuf,
+	.vidioc_qbuf = atomisp_qbuf_wrapper,
+	.vidioc_dqbuf = atomisp_dqbuf_wrapper,
+	.vidioc_streamon = vb2_ioctl_streamon,
+	.vidioc_streamoff = vb2_ioctl_streamoff,
 	.vidioc_default = atomisp_vidioc_default,
 	.vidioc_s_parm = atomisp_s_parm,
 	.vidioc_g_parm = atomisp_g_parm,
diff --git a/drivers/staging/media/atomisp/pci/atomisp_ioctl.h b/drivers/staging/media/atomisp/pci/atomisp_ioctl.h
index c660f631d371..59e071f035f9 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_ioctl.h
+++ b/drivers/staging/media/atomisp/pci/atomisp_ioctl.h
@@ -39,14 +39,12 @@ int atomisp_pipe_check(struct atomisp_video_pipe *pipe, bool streaming_ok);
 int atomisp_alloc_css_stat_bufs(struct atomisp_sub_device *asd,
 				uint16_t stream_id);
 
-int atomisp_streamoff(struct file *file, void *fh, enum v4l2_buf_type type);
-int atomisp_reqbufs(struct file *file, void *fh, struct v4l2_requestbuffers *req);
+int atomisp_start_streaming(struct vb2_queue *vq, unsigned int count);
+void atomisp_stop_streaming(struct vb2_queue *vq);
 
 enum ia_css_pipe_id atomisp_get_css_pipe_id(struct atomisp_sub_device
 	*asd);
 
-void atomisp_videobuf_free_buf(struct videobuf_buffer *vb);
-
 extern const struct v4l2_ioctl_ops atomisp_ioctl_ops;
 
 unsigned int atomisp_streaming_count(struct atomisp_device *isp);
@@ -57,4 +55,8 @@ long atomisp_compat_ioctl32(struct file *file,
 
 int atomisp_stream_on_master_slave_sensor(struct atomisp_device *isp,
 	bool isp_timeout);
+
+int atomisp_start_streaming(struct vb2_queue *vq, unsigned int count);
+void atomisp_stop_streaming(struct vb2_queue *vq);
+
 #endif /* __ATOMISP_IOCTL_H__ */
diff --git a/drivers/staging/media/atomisp/pci/atomisp_subdev.c b/drivers/staging/media/atomisp/pci/atomisp_subdev.c
index 95dc4188309b..cadc468b4c2f 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_subdev.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_subdev.c
@@ -1064,6 +1064,7 @@ static void atomisp_init_subdev_pipe(struct atomisp_sub_device *asd,
 	pipe->asd = asd;
 	pipe->isp = asd->isp;
 	spin_lock_init(&pipe->irq_lock);
+	mutex_init(&pipe->vb_queue_mutex);
 	INIT_LIST_HEAD(&pipe->buffers_in_css);
 	INIT_LIST_HEAD(&pipe->activeq);
 	INIT_LIST_HEAD(&pipe->buffers_waiting_for_param);
diff --git a/drivers/staging/media/atomisp/pci/atomisp_subdev.h b/drivers/staging/media/atomisp/pci/atomisp_subdev.h
index 45b0c7341e84..bd2872cbb50c 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_subdev.h
+++ b/drivers/staging/media/atomisp/pci/atomisp_subdev.h
@@ -21,8 +21,7 @@
 #include <media/v4l2-ctrls.h>
 #include <media/v4l2-device.h>
 #include <media/v4l2-subdev.h>
-#include <media/videobuf-core.h>
-
+#include <media/videobuf2-v4l2.h>
 #include "atomisp_common.h"
 #include "atomisp_compat.h"
 #include "atomisp_v4l2.h"
@@ -69,7 +68,9 @@ struct atomisp_video_pipe {
 	struct video_device vdev;
 	enum v4l2_buf_type type;
 	struct media_pad pad;
-	struct videobuf_queue capq;
+	struct vb2_queue vb_queue;
+	/* Lock for vb_queue, when also taking isp->mutex this must be taken first! */
+	struct mutex vb_queue_mutex;
 	/* List of video-buffers handed over to the CSS  */
 	struct list_head buffers_in_css;
 	/* List of video-buffers handed over to the driver, but not yet to the CSS */
@@ -82,6 +83,9 @@ struct atomisp_video_pipe {
 	/* the link list to store per_frame parameters */
 	struct list_head per_frame_params;
 
+	/* Filled through atomisp_get_css_frame_info() on queue setup */
+	struct ia_css_frame_info frame_info;
+
 	/* Store here the initial run mode */
 	unsigned int default_run_mode;
 	/* Set from streamoff to disallow queuing further buffers in CSS */
@@ -113,6 +117,11 @@ struct atomisp_video_pipe {
 	struct atomisp_css_params_with_list *frame_params[VIDEO_MAX_FRAME];
 };
 
+#define vq_to_pipe(queue) \
+	container_of(queue, struct atomisp_video_pipe, vb_queue)
+
+#define vb_to_pipe(vb) vq_to_pipe((vb)->vb2_queue)
+
 struct atomisp_pad_format {
 	struct v4l2_mbus_framefmt fmt;
 	struct v4l2_rect crop;
diff --git a/drivers/staging/media/atomisp/pci/ia_css_frame_public.h b/drivers/staging/media/atomisp/pci/ia_css_frame_public.h
index d787c4054bb1..32d6d9699c37 100644
--- a/drivers/staging/media/atomisp/pci/ia_css_frame_public.h
+++ b/drivers/staging/media/atomisp/pci/ia_css_frame_public.h
@@ -20,6 +20,7 @@
  * This file contains structs to describe various frame-formats supported by the ISP.
  */
 
+#include <media/videobuf2-v4l2.h>
 #include <type_support.h>
 #include "ia_css_err.h"
 #include "ia_css_types.h"
@@ -146,6 +147,17 @@ enum ia_css_frame_flash_state {
  *  This is the main structure used for all input and output images.
  */
 struct ia_css_frame {
+	/*
+	 * The videobuf2 core will allocate buffers including room for private
+	 * data (the rest of struct ia_css_frame). The vb2_v4l2_buffer must
+	 * be the first member for this to work!
+	 * Note the atomisp code also uses ia_css_frame-s which are not used
+	 * as v4l2-buffers in some places. In this case the vb2 member will
+	 * be unused.
+	 */
+	struct vb2_v4l2_buffer vb;
+	/* List-head for linking into the activeq or buffers_waiting_for_param list */
+	struct list_head queue;
 	struct ia_css_frame_info frame_info; /** info struct describing the frame */
 	ia_css_ptr   data;	       /** pointer to start of image data */
 	unsigned int data_bytes;       /** size of image data in bytes */
@@ -183,6 +195,9 @@ struct ia_css_frame {
 		       info.format */
 };
 
+#define vb_to_frame(vb2) \
+	container_of(to_vb2_v4l2_buffer(vb2), struct ia_css_frame, vb)
+
 #define DEFAULT_FRAME { \
 	.frame_info		= IA_CSS_BINARY_DEFAULT_FRAME_INFO, \
 	.dynamic_queue_id	= SH_CSS_INVALID_QUEUE_ID, \
-- 
2.37.3

[PATCH v2 11/17] media: atomisp: Make it possible to call atomisp_set_fmt() without a file handle

[Hans de Goede]

To fix atomisp_queue_setup() sometimes failing it needs to be able to call
atomisp_set_fmt(), but atomisp_queue_setup() (VIDIOC_REQBUFS) does not get
passed a file handle by the videobuf2 core.

Partly revert commit b3be98f984d4 ("media: atomisp: Remove a couple of not
useful function wrappers") so that atomisp_set_fmt() can be used
without a file handle.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/staging/media/atomisp/pci/atomisp_cmd.c   |  3 +--
 drivers/staging/media/atomisp/pci/atomisp_cmd.h   |  2 +-
 drivers/staging/media/atomisp/pci/atomisp_ioctl.c | 10 +++++++++-
 3 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp_cmd.c b/drivers/staging/media/atomisp/pci/atomisp_cmd.c
index 0f8d683382fd..b52c8e010752 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_cmd.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_cmd.c
@@ -5096,9 +5096,8 @@ static int atomisp_set_fmt_to_snr(struct video_device *vdev,
 	return css_input_resolution_changed(asd, ffmt);
 }
 
-int atomisp_set_fmt(struct file *file, void *unused, struct v4l2_format *f)
+int atomisp_set_fmt(struct video_device *vdev, struct v4l2_format *f)
 {
-	struct video_device *vdev = video_devdata(file);
 	struct atomisp_device *isp = video_get_drvdata(vdev);
 	struct atomisp_video_pipe *pipe = atomisp_to_video_pipe(vdev);
 	struct atomisp_sub_device *asd = pipe->asd;
diff --git a/drivers/staging/media/atomisp/pci/atomisp_cmd.h b/drivers/staging/media/atomisp/pci/atomisp_cmd.h
index 4c965d17c9a3..d9110bba8c28 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_cmd.h
+++ b/drivers/staging/media/atomisp/pci/atomisp_cmd.h
@@ -265,7 +265,7 @@ int atomisp_get_sensor_mode_data(struct atomisp_sub_device *asd,
 int atomisp_try_fmt(struct video_device *vdev, struct v4l2_pix_format *f,
 		    bool *res_overflow);
 
-int atomisp_set_fmt(struct file *file, void *fh, struct v4l2_format *f);
+int atomisp_set_fmt(struct video_device *vdev, struct v4l2_format *f);
 
 int atomisp_set_shading_table(struct atomisp_sub_device *asd,
 			      struct atomisp_shading_table *shading_table);
diff --git a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
index e850de069f8f..b5d42257a87d 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
@@ -665,6 +665,14 @@ static int atomisp_g_input(struct file *file, void *fh, unsigned int *input)
 	return 0;
 }
 
+static int atomisp_s_fmt_cap(struct file *file, void *fh,
+			     struct v4l2_format *f)
+{
+	struct video_device *vdev = video_devdata(file);
+
+	return atomisp_set_fmt(vdev, f);
+}
+
 /*
  * set input are used to set current primary/secondary camera
  */
@@ -2401,7 +2409,7 @@ const struct v4l2_ioctl_ops atomisp_ioctl_ops = {
 	.vidioc_enum_fmt_vid_cap = atomisp_enum_fmt_cap,
 	.vidioc_try_fmt_vid_cap = atomisp_try_fmt_cap,
 	.vidioc_g_fmt_vid_cap = atomisp_g_fmt_cap,
-	.vidioc_s_fmt_vid_cap = atomisp_set_fmt,
+	.vidioc_s_fmt_vid_cap = atomisp_s_fmt_cap,
 	.vidioc_reqbufs = vb2_ioctl_reqbufs,
 	.vidioc_querybuf = vb2_ioctl_querybuf,
 	.vidioc_qbuf = atomisp_qbuf_wrapper,
-- 
2.37.3

[PATCH v2 12/17] media: atomisp: Fix VIDIOC_REQBUFS failing when VIDIOC_S_FMT has not been called yet

[Hans de Goede]

camorama calls VIDIOC_REQBUFS(V4L2_MEMORY_MMAP) to test if MMAP support
works (this was added specifically because of the previously broken
MMAP support in atomisp).

Currently this fails because atomisp_get_css_frame_info() fails in this
case. Although it is weird to call VIDIOC_REQBUFS before VIDIOC_S_FMT,
it is allowed to do this.

Fix this not working by doing a S_FMT to V4L2_PIX_FMT_YUV420 + the highest
supported resolution.

Note this will cause camorama to use mmap mode, which means it will also
use libv4l2 to do format conversion. libv4l2 will pick V4L2_PIX_FMT_RGB565
as input format and this will lead to a garbled video display. This is
a libv4lconvert bug, the RGB565 -> RGB24 path in libv4lconvert assumes
that stride == width which is not true on the atomisp.

I've already send out a libv4lconvert fix for this. Also this can be worked
around by passing --dont-use-libv4l2 as argument to camorama.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 .../media/atomisp/pci/atomisp_compat_css20.c  |  2 +-
 .../staging/media/atomisp/pci/atomisp_fops.c  | 25 +++++++++++++++++--
 2 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c b/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
index ea6114679c53..f282572d69da 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
@@ -2688,7 +2688,7 @@ int atomisp_get_css_frame_info(struct atomisp_sub_device *asd,
 
 	if (0 != ia_css_pipe_get_info(asd->stream_env[stream_index]
 		.pipes[pipe_index], &info)) {
-		dev_err(isp->dev, "ia_css_pipe_get_info FAILED");
+		dev_dbg(isp->dev, "ia_css_pipe_get_info FAILED");
 		return -EINVAL;
 	}
 
diff --git a/drivers/staging/media/atomisp/pci/atomisp_fops.c b/drivers/staging/media/atomisp/pci/atomisp_fops.c
index bc47a092a8af..f539df09ebd9 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_fops.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_fops.c
@@ -50,15 +50,36 @@ static int atomisp_queue_setup(struct vb2_queue *vq,
 	u16 source_pad = atomisp_subdev_source_pad(&pipe->vdev);
 	int ret;
 
+	mutex_lock(&pipe->asd->isp->mutex); /* for get_css_frame_info() / set_fmt() */
+
+	/*
+	 * When VIDIOC_S_FMT has not been called before VIDIOC_REQBUFS, then
+	 * this will fail. Call atomisp_set_fmt() ourselves and try again.
+	 */
 	ret = atomisp_get_css_frame_info(pipe->asd, source_pad, &pipe->frame_info);
-	if (ret)
-		return ret;
+	if (ret) {
+		struct v4l2_format f = {
+			.fmt.pix.pixelformat = V4L2_PIX_FMT_YUV420,
+			.fmt.pix.width = 10000,
+			.fmt.pix.height = 10000,
+		};
+
+		ret = atomisp_set_fmt(&pipe->vdev, &f);
+		if (ret)
+			goto out;
+
+		ret = atomisp_get_css_frame_info(pipe->asd, source_pad, &pipe->frame_info);
+		if (ret)
+			goto out;
+	}
 
 	atomisp_alloc_css_stat_bufs(pipe->asd, ATOMISP_INPUT_STREAM_GENERAL);
 
 	*nplanes = 1;
 	sizes[0] = PAGE_ALIGN(pipe->pix.sizeimage);
 
+out:
+	mutex_unlock(&pipe->asd->isp->mutex);
 	return 0;
 }
 
-- 
2.37.3

[PATCH v2 13/17] media: atomisp: Refactor atomisp_adjust_fmt()

[Hans de Goede]

Refactor atomisp_adjust_fmt():

1. The block starting at "format_bridge = atomisp_get_format_bridge(...)"
and ending with "if (field == V4L2_FIELD_ANY) field = V4L2_FIELD_NONE;"
is duplicated. With only the second block:
a) Properly checking that format_bridge is not NULL; amd
b) Having the special handling for IA_CSS_FRAME_FORMAT_RAW

Remove the first block.

2. On a NULL return from atomisp_get_format_bridge(f->fmt.pix.pixelformat)
fall back to V4L2_PIX_FMT_YUV420 just like in the IA_CSS_FRAME_FORMAT_RAW
case. atomisp_adjust_fmt() is used in VIDIOC_TRY_FMT handling and that
should jusy pick a fmt rather then returning -EINVAL.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 .../staging/media/atomisp/pci/atomisp_ioctl.c | 23 +------------------
 1 file changed, 1 insertion(+), 22 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
index b5d42257a87d..5034a5509a3b 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
@@ -878,29 +878,8 @@ static int atomisp_adjust_fmt(struct v4l2_format *f)
 	u32 padded_width;
 
 	format_bridge = atomisp_get_format_bridge(f->fmt.pix.pixelformat);
-
-	padded_width = f->fmt.pix.width + pad_w;
-
-	if (format_bridge->planar) {
-		f->fmt.pix.bytesperline = padded_width;
-		f->fmt.pix.sizeimage = PAGE_ALIGN(f->fmt.pix.height *
-						  DIV_ROUND_UP(format_bridge->depth *
-						  padded_width, 8));
-	} else {
-		f->fmt.pix.bytesperline = DIV_ROUND_UP(format_bridge->depth *
-						      padded_width, 8);
-		f->fmt.pix.sizeimage = PAGE_ALIGN(f->fmt.pix.height * f->fmt.pix.bytesperline);
-	}
-
-	if (f->fmt.pix.field == V4L2_FIELD_ANY)
-		f->fmt.pix.field = V4L2_FIELD_NONE;
-
-	format_bridge = atomisp_get_format_bridge(f->fmt.pix.pixelformat);
-	if (!format_bridge)
-		return -EINVAL;
-
 	/* Currently, raw formats are broken!!! */
-	if (format_bridge->sh_fmt == IA_CSS_FRAME_FORMAT_RAW) {
+	if (!format_bridge || format_bridge->sh_fmt == IA_CSS_FRAME_FORMAT_RAW) {
 		f->fmt.pix.pixelformat = V4L2_PIX_FMT_YUV420;
 
 		format_bridge = atomisp_get_format_bridge(f->fmt.pix.pixelformat);
-- 
2.37.3

[PATCH v2 14/17] media: atomisp: Fix atomisp_try_fmt_cap() always returning YUV420 pixelformat

[Hans de Goede]

The atomisp_try_fmt() call in atomisp_try_fmt_cap() replaces
the pixelformat passed by userspace with the sensors native pixelformat.

Which always gets replaced by V4L2_PIX_FMT_YUV420 by atomisp_adjust_fmt()
because raw sensor formats are not supported.

This needs to be fixed so that userspace which does a try_fmt call before
s_fmt does not end up always getting YUV420 even if it passed something
else into the try_fmt call.

To fix this restore the userspace requested pixelformat before
the atomisp_adjust_fmt() call. atomisp_adjust_fmt() will replace this
with V4L2_PIX_FMT_YUV420 in case an unsupported format is requested.

Note this relies on the "media: atomisp: Refactor atomisp_adjust_fmt()"
change, before that atomisp_adjust_fmt() would return -EINVAL for
unsupported pixelformats.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/staging/media/atomisp/pci/atomisp_ioctl.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
index 5034a5509a3b..384b231de8bc 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
@@ -922,6 +922,7 @@ static int atomisp_try_fmt_cap(struct file *file, void *fh,
 			       struct v4l2_format *f)
 {
 	struct video_device *vdev = video_devdata(file);
+	u32 pixfmt = f->fmt.pix.pixelformat;
 	int ret;
 
 	/*
@@ -935,6 +936,12 @@ static int atomisp_try_fmt_cap(struct file *file, void *fh,
 	if (ret)
 		return ret;
 
+	/*
+	 * atomisp_try_fmt() replaces pixelformat with the sensors native
+	 * format, restore the actual format requested by userspace.
+	 */
+	f->fmt.pix.pixelformat = pixfmt;
+
 	return atomisp_adjust_fmt(f);
 }
 
-- 
2.37.3

[PATCH v2 15/17] media: atomisp: Make atomisp_g_fmt_cap() default to YUV420

[Hans de Goede]

Make atomisp_g_fmt_cap() default to YUV420 so that it matches with what
atomisp_try_fmt_cap() and atomisp_queue_setup() do when they need to
pick a default pixelformat.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/staging/media/atomisp/pci/atomisp_ioctl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
index 384b231de8bc..c8f6f0c2f368 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
@@ -959,7 +959,7 @@ static int atomisp_g_fmt_cap(struct file *file, void *fh,
 	if (f->fmt.pix.sizeimage)
 		return 0;
 
-	f->fmt.pix.pixelformat = V4L2_PIX_FMT_YUYV;
+	f->fmt.pix.pixelformat = V4L2_PIX_FMT_YUV420;
 	f->fmt.pix.width = 10000;
 	f->fmt.pix.height = 10000;
 
-- 
2.37.3

[PATCH v2 16/17] media: atomisp: Remove __atomisp_get_pipe() helper

[Hans de Goede]

Remove the complicated __atomisp_get_pipe() helper, atomisp_buf_done()
only needs the pipe pointer in cases where it has a frame, so we can
simply get the pipe from the frame using the vb_to_pipe() helper.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 .../staging/media/atomisp/pci/atomisp_cmd.c   | 98 +------------------
 1 file changed, 4 insertions(+), 94 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp_cmd.c b/drivers/staging/media/atomisp/pci/atomisp_cmd.c
index b52c8e010752..82d936819ed6 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_cmd.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_cmd.c
@@ -754,93 +754,6 @@ static void atomisp_recover_params_queue(struct atomisp_video_pipe *pipe)
 	atomisp_handle_parameter_and_buffer(pipe);
 }
 
-/* find atomisp_video_pipe with css pipe id, buffer type and atomisp run_mode */
-static struct atomisp_video_pipe *__atomisp_get_pipe(
-    struct atomisp_sub_device *asd,
-    enum atomisp_input_stream_id stream_id,
-    enum ia_css_pipe_id css_pipe_id,
-    enum ia_css_buffer_type buf_type)
-{
-	/* video is same in online as in continuouscapture mode */
-	if (asd->vfpp->val == ATOMISP_VFPP_DISABLE_LOWLAT) {
-		/*
-		 * Disable vf_pp and run CSS in still capture mode. In this
-		 * mode, CSS does not cause extra latency with buffering, but
-		 * scaling is not available.
-		 */
-		return &asd->video_out_capture;
-	} else if (asd->vfpp->val == ATOMISP_VFPP_DISABLE_SCALER) {
-		/*
-		 * Disable vf_pp and run CSS in video mode. This allows using
-		 * ISP scaling but it has one frame delay due to CSS internal
-		 * buffering.
-		 */
-		return &asd->video_out_video_capture;
-	} else if (css_pipe_id == IA_CSS_PIPE_ID_YUVPP) {
-		/*
-		 * to SOC camera, yuvpp pipe is run for capture/video/SDV/ZSL.
-		 */
-		if (asd->continuous_mode->val) {
-			if (asd->run_mode->val == ATOMISP_RUN_MODE_VIDEO) {
-				/* SDV case */
-				switch (buf_type) {
-				case IA_CSS_BUFFER_TYPE_SEC_OUTPUT_FRAME:
-					return &asd->video_out_video_capture;
-				case IA_CSS_BUFFER_TYPE_SEC_VF_OUTPUT_FRAME:
-					return &asd->video_out_preview;
-				case IA_CSS_BUFFER_TYPE_OUTPUT_FRAME:
-					return &asd->video_out_capture;
-				default:
-					return &asd->video_out_vf;
-				}
-			} else if (asd->run_mode->val == ATOMISP_RUN_MODE_PREVIEW) {
-				/* ZSL case */
-				switch (buf_type) {
-				case IA_CSS_BUFFER_TYPE_SEC_OUTPUT_FRAME:
-					return &asd->video_out_preview;
-				case IA_CSS_BUFFER_TYPE_OUTPUT_FRAME:
-					return &asd->video_out_capture;
-				default:
-					return &asd->video_out_vf;
-				}
-			}
-		} else if (buf_type == IA_CSS_BUFFER_TYPE_OUTPUT_FRAME) {
-			switch (asd->run_mode->val) {
-			case ATOMISP_RUN_MODE_VIDEO:
-				return &asd->video_out_video_capture;
-			case ATOMISP_RUN_MODE_PREVIEW:
-				return &asd->video_out_preview;
-			default:
-				return &asd->video_out_capture;
-			}
-		} else if (buf_type == IA_CSS_BUFFER_TYPE_VF_OUTPUT_FRAME) {
-			if (asd->run_mode->val == ATOMISP_RUN_MODE_VIDEO)
-				return &asd->video_out_preview;
-			else
-				return &asd->video_out_vf;
-		}
-	} else if (asd->run_mode->val == ATOMISP_RUN_MODE_VIDEO) {
-		/* For online video or SDV video pipe. */
-		if (css_pipe_id == IA_CSS_PIPE_ID_VIDEO ||
-		    css_pipe_id == IA_CSS_PIPE_ID_COPY ||
-		    css_pipe_id == IA_CSS_PIPE_ID_YUVPP) {
-			if (buf_type == IA_CSS_BUFFER_TYPE_OUTPUT_FRAME)
-				return &asd->video_out_video_capture;
-			return &asd->video_out_preview;
-		}
-	} else if (asd->run_mode->val == ATOMISP_RUN_MODE_PREVIEW) {
-		/* For online preview or ZSL preview pipe. */
-		if (css_pipe_id == IA_CSS_PIPE_ID_PREVIEW ||
-		    css_pipe_id == IA_CSS_PIPE_ID_COPY ||
-		    css_pipe_id == IA_CSS_PIPE_ID_YUVPP)
-			return &asd->video_out_preview;
-	}
-	/* For capture pipe. */
-	if (buf_type == IA_CSS_BUFFER_TYPE_VF_OUTPUT_FRAME)
-		return &asd->video_out_vf;
-	return &asd->video_out_capture;
-}
-
 enum atomisp_metadata_type
 atomisp_get_metadata_type(struct atomisp_sub_device *asd,
 			  enum ia_css_pipe_id pipe_id)
@@ -898,13 +811,6 @@ void atomisp_buf_done(struct atomisp_sub_device *asd, int error,
 		return;
 	}
 
-	/* need to know the atomisp pipe for frame buffers */
-	pipe = __atomisp_get_pipe(asd, stream_id, css_pipe_id, buf_type);
-	if (!pipe) {
-		dev_err(isp->dev, "error getting atomisp pipe\n");
-		return;
-	}
-
 	switch (buf_type) {
 	case IA_CSS_BUFFER_TYPE_3A_STATISTICS:
 		list_for_each_entry_safe(s3a_iter, _s3a_buf_tmp,
@@ -987,6 +893,8 @@ void atomisp_buf_done(struct atomisp_sub_device *asd, int error,
 		if (!frame->valid)
 			error = true;
 
+		pipe = vb_to_pipe(&frame->vb.vb2_buf);
+
 		/* FIXME:
 		 * YUVPP doesn't set postview exp_id correctlly in SDV mode.
 		 * This is a WORKAROUND to set exp_id. see HSDES-1503911606.
@@ -1036,6 +944,8 @@ void atomisp_buf_done(struct atomisp_sub_device *asd, int error,
 		if (!frame->valid)
 			error = true;
 
+		pipe = vb_to_pipe(&frame->vb.vb2_buf);
+
 		/* FIXME:
 		 * YUVPP doesn't set preview exp_id correctlly in ZSL mode.
 		 * This is a WORKAROUND to set exp_id. see HSDES-1503911606.
-- 
2.37.3

[PATCH v2 17/17] media: atomisp: gc0310: Power on sensor from set_fmt() callback

[Hans de Goede]

Depending on which order userspace makes various v4l2 calls, the sensor
might still be powered down when set_fmt is called.

What should really happen here is delay the writing of the mode-related
registers till streaming is started, but for now use the same quick fix
as the atomisp_ov2680 code and call power_up() from set_fmt() in
combination with keeping track of the power-state to avoid doing the
power-up sequence twice.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/staging/media/atomisp/i2c/atomisp-gc0310.c | 14 ++++++++++++--
 drivers/staging/media/atomisp/i2c/gc0310.h         |  1 +
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
index 783f1b88ebf2..87a634bf9ff5 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
@@ -786,8 +786,6 @@ static int gpio_ctrl(struct v4l2_subdev *sd, bool flag)
 	return ret;
 }
 
-static int power_down(struct v4l2_subdev *sd);
-
 static int power_up(struct v4l2_subdev *sd)
 {
 	struct gc0310_device *dev = to_gc0310_sensor(sd);
@@ -800,6 +798,9 @@ static int power_up(struct v4l2_subdev *sd)
 		return -ENODEV;
 	}
 
+	if (dev->power_on)
+		return 0; /* Already on */
+
 	/* power control */
 	ret = power_ctrl(sd, 1);
 	if (ret)
@@ -820,6 +821,7 @@ static int power_up(struct v4l2_subdev *sd)
 
 	msleep(100);
 
+	dev->power_on = true;
 	return 0;
 
 fail_gpio:
@@ -844,6 +846,9 @@ static int power_down(struct v4l2_subdev *sd)
 		return -ENODEV;
 	}
 
+	if (!dev->power_on)
+		return 0; /* Already off */
+
 	/* gpio ctrl */
 	ret = gpio_ctrl(sd, 0);
 	if (ret) {
@@ -861,6 +866,7 @@ static int power_down(struct v4l2_subdev *sd)
 	if (ret)
 		dev_err(&client->dev, "vprog failed.\n");
 
+	dev->power_on = false;
 	return ret;
 }
 
@@ -935,6 +941,9 @@ static int gc0310_set_fmt(struct v4l2_subdev *sd,
 		return 0;
 	}
 
+	/* s_power has not been called yet for std v4l2 clients (camorama) */
+	power_up(sd);
+
 	dev_dbg(&client->dev, "%s: before gc0310_write_reg_array %s\n",
 		__func__, dev->res->desc);
 	ret = startup(sd);
@@ -1073,6 +1082,7 @@ static int gc0310_s_config(struct v4l2_subdev *sd,
 	 * as first power on by board may not fulfill the
 	 * power on sequqence needed by the module
 	 */
+	dev->power_on = true; /* force power_down() to run */
 	ret = power_down(sd);
 	if (ret) {
 		dev_err(&client->dev, "gc0310 power-off err.\n");
diff --git a/drivers/staging/media/atomisp/i2c/gc0310.h b/drivers/staging/media/atomisp/i2c/gc0310.h
index db643ebc3909..4b9ce681bd93 100644
--- a/drivers/staging/media/atomisp/i2c/gc0310.h
+++ b/drivers/staging/media/atomisp/i2c/gc0310.h
@@ -152,6 +152,7 @@ struct gc0310_device {
 	int vt_pix_clk_freq_mhz;
 	struct gc0310_resolution *res;
 	u8 type;
+	bool power_on;
 };
 
 enum gc0310_tok_type {
-- 
2.37.3

Re: [PATCH v2 04/17] media: atomisp: On streamoff wait for buffers owned by the CSS to be given back

[Andy Shevchenko]

On Thu, Oct 20, 2022 at 09:55:20PM +0200, Hans de Goede wrote:
> There is no guarantee that when we stop the pipeline all buffers owned
> by the CSS are cleanly returned to the videobuf queue.
> 
> This is a problem with videobuf2 which will complain loudly when not
> all buffers have been returned after the streamoff() queue op has
> returned.
> 
> And this also allows moving a WARN() in the continuous mode path.

...

> +	if (ret <= 0)
> +		return ret ? ret : -ETIME;

You can use Elvis and ETIME is not correct AFAIU, should be -ETIMEDOUT.


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 06/17] media: atomisp: Also track buffers in a list when submitted to the ISP

[Andy Shevchenko]

On Thu, Oct 20, 2022 at 09:55:22PM +0200, Hans de Goede wrote:
> Instead of using an integer to keep count of how many buffers have
> been handed over to the ISP (buffers_in_css) move buffers handed
> over to the ISP to a new buffers_in_css list_head so that we can
> easily loop over them.
> 
> This removes the need for atomisp_flush_video_pipe() to loop over
> all buffers and then (ab)use the state to figure out if they
> were handed over to the ISP.
> 
> Since the buffers are now always on a list when owned by the driver
> this also allows the buffer_done path on flush vs normal completion
> to be unified (both now need a list_del()) and this common code can
> now be factored out into a new atomisp_buffer_done() helper.
> 
> This is a preparation patch for moving the driver over to
> the videobuf2 framework.

...

> +int atomisp_buffers_in_css(struct atomisp_video_pipe *pipe)
>  {
>  	unsigned long irqflags;
> +	struct list_head *pos;
> +	int buffers_in_css = 0;
>  
> +	spin_lock_irqsave(&pipe->irq_lock, irqflags);
>  
> +	list_for_each(pos, &pipe->buffers_in_css)
> +		buffers_in_css++;
> +
> +	spin_unlock_irqrestore(&pipe->irq_lock, irqflags);
> +
> +	return buffers_in_css;
> +}

Looking at this I come up with the
https://lore.kernel.org/r/20221114112842.38565-1-andriy.shevchenko@linux.intel.com

But I think your stuff will be earlier in upstream, so feel free to create
a followup later on.

...

> +		vb = list_first_entry_or_null(&pipe->activeq, struct videobuf_buffer, queue);
> +		if (vb) {

Wouldn't simply list_empty() work here? (Yes, you would need to have else
branch under spin lock, but codewise seems better to me).

> +			list_move_tail(&vb->queue, &pipe->buffers_in_css);
> +			vb->state = VIDEOBUF_ACTIVE;
>  		}

>  		spin_unlock_irqrestore(&pipe->irq_lock, irqflags);
>  
> +		if (!vb)
> +			return -EINVAL;


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 09/17] media: atomisp: Add ia_css_frame_get_info() helper

[Andy Shevchenko]

On Thu, Oct 20, 2022 at 09:55:25PM +0200, Hans de Goede wrote:
> Several places rely on the [frame_]info member being the first member of
> struct ia_css_frame, so that &frame->info will yield NULL when frame is
> NULL (some places already explicitly check for a NULL frame pointer but
> not nearly all).
> 
> For videobuf2 support the vb2_v4l2_buffer struct needs to be embedded
> in the frame struct and it needs to be the first member. Breaking the
> assumption that &frame->info will yield NULL when frame is NULL.
> 
> Add a ia_css_frame_get_info() helper to return either the ia_css_frame_info
> struct embedded in the frame, or NULL when the frame pointer is NULL and
> use this in places where a ia_css_frame_info ptr or NULL is expected.
> 
> To make sure that we catch all uses of the info field this patch also
> renames the info field to frame_info.
> 
> This is a preparation patch for converting the driver to videobuf2.

...

> +	const struct ia_css_frame_info *in_frame_info = ia_css_frame_get_info(in_frame);
>  	const unsigned int ddr_bits_per_element = sizeof(short) * 8;

Side note: BITS_PER_TYPE().

...

> +	const struct ia_css_frame_info *in_frame_info = ia_css_frame_get_info(in_frame);
>  	const unsigned int ddr_bits_per_element = sizeof(short) * 8;

Ditto.

>  	const unsigned int ddr_elems_per_word = ceil_div(HIVE_ISP_DDR_WORD_BITS,
>  						ddr_bits_per_element);

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 10/17] media: atomisp: Convert to videobuf2

[Andy Shevchenko]

On Thu, Oct 20, 2022 at 09:55:26PM +0200, Hans de Goede wrote:
> Convert atomisp to use videobuf2.
> 
> This fixes mmap not working and in general moving over to
> the more modern videobuf2 is a good plan.

...

> -				/* The is the end, stop further loop */
> +			if (list_entry_is_head(param, &pipe->per_frame_params, list)) {
> +				/* The is the end, stop outer loop */
>  				break;
>  			}

You can drop {} by writing this as

			/* If this is the end, stop further loop */
			if (list_entry_is_head(param, &pipe->per_frame_params, list))
				break;

...

> +		if (!list_empty(&pipe->buffers_waiting_for_param))

Why not positive conditional?

> +			atomisp_handle_parameter_and_buffer(pipe);
> +		else
> +			atomisp_qbuffers_to_css(asd);

...

> +	/*
> +	 * Workaround: Due to the design of HALv3,
> +	 * sometimes in ZSL or SDV mode HAL needs to
> +	 * capture multiple images within one streaming cycle.
> +	 * But the capture number cannot be determined by HAL.
> +	 * So HAL only sets the capture number to be 1 and queue multiple
> +	 * buffers. Atomisp driver needs to check this case and re-trigger
> +	 * CSS to do capture when new buffer is queued.

Indentation of the above seems arbitrary.

> +	 */

...

> +	/*
> +	 * FIXME This if is copied from _vb2_fop_release, this cannot use that

 _vb2_fop_release() ?

> +	 * because that calls v4l2_fh_release() earlier then this function.
> +	 * Maybe we can release the fh earlier though, it does not look like
> +	 * anything needs it after this.
> +	 */

...

> +out:

In some cases you use 'unlock' in some 'out' for the same, I would suggest to
unify as

out_unlock:

in all affected locations.

> +	mutex_unlock(&isp->mutex);
> +	return ret;

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 12/17] media: atomisp: Fix VIDIOC_REQBUFS failing when VIDIOC_S_FMT has not been called yet

[Andy Shevchenko]

On Thu, Oct 20, 2022 at 09:55:28PM +0200, Hans de Goede wrote:
> camorama calls VIDIOC_REQBUFS(V4L2_MEMORY_MMAP) to test if MMAP support
> works (this was added specifically because of the previously broken
> MMAP support in atomisp).
> 
> Currently this fails because atomisp_get_css_frame_info() fails in this
> case. Although it is weird to call VIDIOC_REQBUFS before VIDIOC_S_FMT,
> it is allowed to do this.
> 
> Fix this not working by doing a S_FMT to V4L2_PIX_FMT_YUV420 + the highest
> supported resolution.
> 
> Note this will cause camorama to use mmap mode, which means it will also
> use libv4l2 to do format conversion. libv4l2 will pick V4L2_PIX_FMT_RGB565
> as input format and this will lead to a garbled video display. This is
> a libv4lconvert bug, the RGB565 -> RGB24 path in libv4lconvert assumes
> that stride == width which is not true on the atomisp.
> 
> I've already send out a libv4lconvert fix for this. Also this can be worked

Link: ?

> around by passing --dont-use-libv4l2 as argument to camorama.

...

> +		struct v4l2_format f = {
> +			.fmt.pix.pixelformat = V4L2_PIX_FMT_YUV420,

> +			.fmt.pix.width = 10000,
> +			.fmt.pix.height = 10000,

If it's just an arbitrary maximum, I would suggest to use definitions from
limits.h, so it will better show the intention.

> +		};

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 15/17] media: atomisp: Make atomisp_g_fmt_cap() default to YUV420

[Andy Shevchenko]

On Thu, Oct 20, 2022 at 09:55:31PM +0200, Hans de Goede wrote:
> Make atomisp_g_fmt_cap() default to YUV420 so that it matches with what
> atomisp_try_fmt_cap() and atomisp_queue_setup() do when they need to
> pick a default pixelformat.

...

>  	f->fmt.pix.width = 10000;
>  	f->fmt.pix.height = 10000;

Side note: Same as before, perhaps better to use limits.h instead.
Unless it's defined by hardware limitations, in such case perhaps
define them with explanatory naming.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 17/17] media: atomisp: gc0310: Power on sensor from set_fmt() callback

[Andy Shevchenko]

On Thu, Oct 20, 2022 at 09:55:33PM +0200, Hans de Goede wrote:
> Depending on which order userspace makes various v4l2 calls, the sensor
> might still be powered down when set_fmt is called.
> 
> What should really happen here is delay the writing of the mode-related
> registers till streaming is started, but for now use the same quick fix
> as the atomisp_ov2680 code and call power_up() from set_fmt() in
> combination with keeping track of the power-state to avoid doing the
> power-up sequence twice.

...

> +	/* s_power has not been called yet for std v4l2 clients (camorama) */
> +	power_up(sd);

if camorama is fixed, will this become a problem?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 00/17] media: atomisp: Convert to videobuf2

[Andy Shevchenko]

On Thu, Oct 20, 2022 at 09:55:16PM +0200, Hans de Goede wrote:
> Hi All,
> 
> Here is v2 of my patch series converting the staging atomisp driver to use
> the videobuf2 framework, fixing MMAP mode not working.
> 
> New in v2 is that the "media: atomisp: Convert to videobuf2" patch
> now also contains moving over to using a per vb_queue lock as is
> standard for v4l2 drivers. This removes a bunch of FIXME + checkpatch
> warnings (due to commented out prep for this) from v1 of this patch.
> This v2 also fixes the 1 new warning pointed out by the lkp test robot.
> 
> For some more background info here are the still relevant bits of
> the v1 cover-letter:
> 
> This also contains an important fix to try_fmt handling, so that
> the various supported output formats can actually be used by userspace
> which calls try_fmt before doing s_fmt.
> 
> So slowly this is starting to look more and more like a standard
> v4l2 device (with all the complex pipeline handling hidden in the driver,
> moving that to userspace seems to be impossible with this design since
> multiple pipeline steps are handled by a single firmware program).

I completely in favour of the series, so
Reviewed-by: Andy Shevchenko <andy@kernel.org>
for non-commented patches, for the commented, where comment is a "side note".

For the rest depends on your action, if you are going to address as suggested,
feel free to add my tag.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 12/17] media: atomisp: Fix VIDIOC_REQBUFS failing when VIDIOC_S_FMT has not been called yet

[Mauro Carvalho Chehab]

Em Thu, 20 Oct 2022 21:55:28 +0200
Hans de Goede <hdegoede@redhat.com> escreveu:

> camorama calls VIDIOC_REQBUFS(V4L2_MEMORY_MMAP) to test if MMAP support
> works (this was added specifically because of the previously broken
> MMAP support in atomisp).
> 
> Currently this fails because atomisp_get_css_frame_info() fails in this
> case. Although it is weird to call VIDIOC_REQBUFS before VIDIOC_S_FMT,
> it is allowed to do this.
> 
> Fix this not working by doing a S_FMT to V4L2_PIX_FMT_YUV420 + the highest
> supported resolution.
> 
> Note this will cause camorama to use mmap mode, which means it will also
> use libv4l2 to do format conversion. libv4l2 will pick V4L2_PIX_FMT_RGB565
> as input format and this will lead to a garbled video display. This is
> a libv4lconvert bug, the RGB565 -> RGB24 path in libv4lconvert assumes
> that stride == width which is not true on the atomisp.

Hmm... should we add a patch on Camorama for it to not use libv4l2 if
format == V4L2_PIX_FMT_RGB565?

> 
> I've already send out a libv4lconvert fix for this. Also this can be worked
> around by passing --dont-use-libv4l2 as argument to camorama.

Was the patch already applied? The better would be to apply it at
libv4l2 upstream, as a fix, porting it to old versions, and mentioning
on what versions this got fixed on this changeset.

> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  .../media/atomisp/pci/atomisp_compat_css20.c  |  2 +-
>  .../staging/media/atomisp/pci/atomisp_fops.c  | 25 +++++++++++++++++--
>  2 files changed, 24 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c b/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
> index ea6114679c53..f282572d69da 100644
> --- a/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
> +++ b/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
> @@ -2688,7 +2688,7 @@ int atomisp_get_css_frame_info(struct atomisp_sub_device *asd,
>  
>  	if (0 != ia_css_pipe_get_info(asd->stream_env[stream_index]
>  		.pipes[pipe_index], &info)) {
> -		dev_err(isp->dev, "ia_css_pipe_get_info FAILED");
> +		dev_dbg(isp->dev, "ia_css_pipe_get_info FAILED");
>  		return -EINVAL;
>  	}
>  
> diff --git a/drivers/staging/media/atomisp/pci/atomisp_fops.c b/drivers/staging/media/atomisp/pci/atomisp_fops.c
> index bc47a092a8af..f539df09ebd9 100644
> --- a/drivers/staging/media/atomisp/pci/atomisp_fops.c
> +++ b/drivers/staging/media/atomisp/pci/atomisp_fops.c
> @@ -50,15 +50,36 @@ static int atomisp_queue_setup(struct vb2_queue *vq,
>  	u16 source_pad = atomisp_subdev_source_pad(&pipe->vdev);
>  	int ret;
>  
> +	mutex_lock(&pipe->asd->isp->mutex); /* for get_css_frame_info() / set_fmt() */
> +
> +	/*
> +	 * When VIDIOC_S_FMT has not been called before VIDIOC_REQBUFS, then
> +	 * this will fail. Call atomisp_set_fmt() ourselves and try again.
> +	 */
>  	ret = atomisp_get_css_frame_info(pipe->asd, source_pad, &pipe->frame_info);
> -	if (ret)
> -		return ret;
> +	if (ret) {
> +		struct v4l2_format f = {
> +			.fmt.pix.pixelformat = V4L2_PIX_FMT_YUV420,
> +			.fmt.pix.width = 10000,
> +			.fmt.pix.height = 10000,
> +		};
> +
> +		ret = atomisp_set_fmt(&pipe->vdev, &f);
> +		if (ret)
> +			goto out;
> +
> +		ret = atomisp_get_css_frame_info(pipe->asd, source_pad, &pipe->frame_info);
> +		if (ret)
> +			goto out;
> +	}
>  
>  	atomisp_alloc_css_stat_bufs(pipe->asd, ATOMISP_INPUT_STREAM_GENERAL);
>  
>  	*nplanes = 1;
>  	sizes[0] = PAGE_ALIGN(pipe->pix.sizeimage);
>  
> +out:
> +	mutex_unlock(&pipe->asd->isp->mutex);
>  	return 0;
>  }
>  

Re: [PATCH v2 17/17] media: atomisp: gc0310: Power on sensor from set_fmt() callback

[Mauro Carvalho Chehab]

Em Thu, 20 Oct 2022 21:55:33 +0200
Hans de Goede <hdegoede@redhat.com> escreveu:

> Depending on which order userspace makes various v4l2 calls, the sensor
> might still be powered down when set_fmt is called.
> 
> What should really happen here is delay the writing of the mode-related
> registers till streaming is started, but for now use the same quick fix
> as the atomisp_ov2680 code and call power_up() from set_fmt() in
> combination with keeping track of the power-state to avoid doing the
> power-up sequence twice.

Yes, that seems to be the right solution: to store the mode and use
it during stream on. I was thinking on doing this at ov2680, but I
opted not to.

The rationale is that we need first to ensure that atomisp will use the 
standard API for the sensor drivers (this is currently not the case).
As far as I recall, the try_fmt logic requires some things to come
from the driver on a non-standard way, and for some, the device needs
to be in power up state. It also uses it at atomisp/sensor's probe time.

Regards,
Mauro

Re: [PATCH v2 04/17] media: atomisp: On streamoff wait for buffers owned by the CSS to be given back

[Hans de Goede]

Hi,

On 11/14/22 11:38, Andy Shevchenko wrote:
> On Thu, Oct 20, 2022 at 09:55:20PM +0200, Hans de Goede wrote:
>> There is no guarantee that when we stop the pipeline all buffers owned
>> by the CSS are cleanly returned to the videobuf queue.
>>
>> This is a problem with videobuf2 which will complain loudly when not
>> all buffers have been returned after the streamoff() queue op has
>> returned.
>>
>> And this also allows moving a WARN() in the continuous mode path.
> 
> ...
> 
>> +	if (ret <= 0)
>> +		return ret ? ret : -ETIME;
> 
> You can use Elvis and ETIME is not correct AFAIU, should be -ETIMEDOUT.

Both fixed in the version which I'm about to push to:
https://git.kernel.org/pub/scm/linux/kernel/git/hansg/linux.git/log/?h=media-atomisp

Regards,

Hans

Re: [PATCH v2 06/17] media: atomisp: Also track buffers in a list when submitted to the ISP

[Hans de Goede]

Hi,

On 11/14/22 12:53, Andy Shevchenko wrote:
> On Thu, Oct 20, 2022 at 09:55:22PM +0200, Hans de Goede wrote:
>> Instead of using an integer to keep count of how many buffers have
>> been handed over to the ISP (buffers_in_css) move buffers handed
>> over to the ISP to a new buffers_in_css list_head so that we can
>> easily loop over them.
>>
>> This removes the need for atomisp_flush_video_pipe() to loop over
>> all buffers and then (ab)use the state to figure out if they
>> were handed over to the ISP.
>>
>> Since the buffers are now always on a list when owned by the driver
>> this also allows the buffer_done path on flush vs normal completion
>> to be unified (both now need a list_del()) and this common code can
>> now be factored out into a new atomisp_buffer_done() helper.
>>
>> This is a preparation patch for moving the driver over to
>> the videobuf2 framework.
> 
> ...
> 
>> +int atomisp_buffers_in_css(struct atomisp_video_pipe *pipe)
>>  {
>>  	unsigned long irqflags;
>> +	struct list_head *pos;
>> +	int buffers_in_css = 0;
>>  
>> +	spin_lock_irqsave(&pipe->irq_lock, irqflags);
>>  
>> +	list_for_each(pos, &pipe->buffers_in_css)
>> +		buffers_in_css++;
>> +
>> +	spin_unlock_irqrestore(&pipe->irq_lock, irqflags);
>> +
>> +	return buffers_in_css;
>> +}
> 
> Looking at this I come up with the
> https://lore.kernel.org/r/20221114112842.38565-1-andriy.shevchenko@linux.intel.com
> 
> But I think your stuff will be earlier in upstream, so feel free to create
> a followup later on.

That is super useful, thanks. But as you mention it is probably best to
just conver the code here to this alter. I've added this to my atomisp
TODO list.


> 
> ...
> 
>> +		vb = list_first_entry_or_null(&pipe->activeq, struct videobuf_buffer, queue);
>> +		if (vb) {
> 
> Wouldn't simply list_empty() work here? (Yes, you would need to have else
> branch under spin lock, but codewise seems better to me).

The problem with that is that the else branch does a "return -EINVAL;"
so then I would need a separate spin_unlock_irqrestore() for the else
branch and I really dislike not having my locks / unlocks cleanly
balanced 1:1.

Regards,

Hans



> 
>> +			list_move_tail(&vb->queue, &pipe->buffers_in_css);
>> +			vb->state = VIDEOBUF_ACTIVE;
>>  		}
> 
>>  		spin_unlock_irqrestore(&pipe->irq_lock, irqflags);
>>  
>> +		if (!vb)
>> +			return -EINVAL;
> 
> 

Re: [PATCH v2 10/17] media: atomisp: Convert to videobuf2

[Hans de Goede]

Hi,

On 11/14/22 13:14, Andy Shevchenko wrote:
> On Thu, Oct 20, 2022 at 09:55:26PM +0200, Hans de Goede wrote:
>> Convert atomisp to use videobuf2.
>>
>> This fixes mmap not working and in general moving over to
>> the more modern videobuf2 is a good plan.
> 
> ...
> 
>> -				/* The is the end, stop further loop */
>> +			if (list_entry_is_head(param, &pipe->per_frame_params, list)) {
>> +				/* The is the end, stop outer loop */
>>  				break;
>>  			}
> 
> You can drop {} by writing this as
> 
> 			/* If this is the end, stop further loop */
> 			if (list_entry_is_head(param, &pipe->per_frame_params, list))
> 				break;
> 

Fixed in my media-atomisp branch.

> ...
> 
>> +		if (!list_empty(&pipe->buffers_waiting_for_param))
> 
> Why not positive conditional?

This is existing code which is moved around, I prefer to keep this
the same to make it clear when looking at the entire diff that
this is just moved around and not changed.

> 
>> +			atomisp_handle_parameter_and_buffer(pipe);
>> +		else
>> +			atomisp_qbuffers_to_css(asd);
> 
> ...
> 
>> +	/*
>> +	 * Workaround: Due to the design of HALv3,
>> +	 * sometimes in ZSL or SDV mode HAL needs to
>> +	 * capture multiple images within one streaming cycle.
>> +	 * But the capture number cannot be determined by HAL.
>> +	 * So HAL only sets the capture number to be 1 and queue multiple
>> +	 * buffers. Atomisp driver needs to check this case and re-trigger
>> +	 * CSS to do capture when new buffer is queued.
> 
> Indentation of the above seems arbitrary.

Yeah it is, again this is existing code (existing comment) which
is just moved around.

> 
>> +	 */
> 
> ...
> 
>> +	/*
>> +	 * FIXME This if is copied from _vb2_fop_release, this cannot use that
> 
>  _vb2_fop_release() ?
> 
>> +	 * because that calls v4l2_fh_release() earlier then this function.
>> +	 * Maybe we can release the fh earlier though, it does not look like
>> +	 * anything needs it after this.
>> +	 */
> 
> ...
> 
>> +out:
> 
> In some cases you use 'unlock' in some 'out' for the same, I would suggest to
> unify as
> 
> out_unlock:
> 
> in all affected locations.

Fixed in my media-atomisp branch.

Regards,

Hans




> 
>> +	mutex_unlock(&isp->mutex);
>> +	return ret;
> 

Re: [PATCH v2 12/17] media: atomisp: Fix VIDIOC_REQBUFS failing when VIDIOC_S_FMT has not been called yet

[Hans de Goede]

Hi,

On 11/14/22 21:38, Mauro Carvalho Chehab wrote:
> Em Thu, 20 Oct 2022 21:55:28 +0200
> Hans de Goede <hdegoede@redhat.com> escreveu:
> 
>> camorama calls VIDIOC_REQBUFS(V4L2_MEMORY_MMAP) to test if MMAP support
>> works (this was added specifically because of the previously broken
>> MMAP support in atomisp).
>>
>> Currently this fails because atomisp_get_css_frame_info() fails in this
>> case. Although it is weird to call VIDIOC_REQBUFS before VIDIOC_S_FMT,
>> it is allowed to do this.
>>
>> Fix this not working by doing a S_FMT to V4L2_PIX_FMT_YUV420 + the highest
>> supported resolution.
>>
>> Note this will cause camorama to use mmap mode, which means it will also
>> use libv4l2 to do format conversion. libv4l2 will pick V4L2_PIX_FMT_RGB565
>> as input format and this will lead to a garbled video display. This is
>> a libv4lconvert bug, the RGB565 -> RGB24 path in libv4lconvert assumes
>> that stride == width which is not true on the atomisp.
> 
> Hmm... should we add a patch on Camorama for it to not use libv4l2 if
> format == V4L2_PIX_FMT_RGB565?

This is not controlled by Camorama but by libv4lconvert, AFAICT there are
no atomisp2 native formats which are supported in Camorama without libv4l  ?


> 
>>
>> I've already send out a libv4lconvert fix for this. Also this can be worked
>> around by passing --dont-use-libv4l2 as argument to camorama.
> 
> Was the patch already applied? The better would be to apply it at
> libv4l2 upstream, as a fix, porting it to old versions, and mentioning
> on what versions this got fixed on this changeset.

I see that you have already found the patches. I will add a comment
to the commit msg pointing to the now applied patch.

Regards,

Hans



> 
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>  .../media/atomisp/pci/atomisp_compat_css20.c  |  2 +-
>>  .../staging/media/atomisp/pci/atomisp_fops.c  | 25 +++++++++++++++++--
>>  2 files changed, 24 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c b/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
>> index ea6114679c53..f282572d69da 100644
>> --- a/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
>> +++ b/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
>> @@ -2688,7 +2688,7 @@ int atomisp_get_css_frame_info(struct atomisp_sub_device *asd,
>>  
>>  	if (0 != ia_css_pipe_get_info(asd->stream_env[stream_index]
>>  		.pipes[pipe_index], &info)) {
>> -		dev_err(isp->dev, "ia_css_pipe_get_info FAILED");
>> +		dev_dbg(isp->dev, "ia_css_pipe_get_info FAILED");
>>  		return -EINVAL;
>>  	}
>>  
>> diff --git a/drivers/staging/media/atomisp/pci/atomisp_fops.c b/drivers/staging/media/atomisp/pci/atomisp_fops.c
>> index bc47a092a8af..f539df09ebd9 100644
>> --- a/drivers/staging/media/atomisp/pci/atomisp_fops.c
>> +++ b/drivers/staging/media/atomisp/pci/atomisp_fops.c
>> @@ -50,15 +50,36 @@ static int atomisp_queue_setup(struct vb2_queue *vq,
>>  	u16 source_pad = atomisp_subdev_source_pad(&pipe->vdev);
>>  	int ret;
>>  
>> +	mutex_lock(&pipe->asd->isp->mutex); /* for get_css_frame_info() / set_fmt() */
>> +
>> +	/*
>> +	 * When VIDIOC_S_FMT has not been called before VIDIOC_REQBUFS, then
>> +	 * this will fail. Call atomisp_set_fmt() ourselves and try again.
>> +	 */
>>  	ret = atomisp_get_css_frame_info(pipe->asd, source_pad, &pipe->frame_info);
>> -	if (ret)
>> -		return ret;
>> +	if (ret) {
>> +		struct v4l2_format f = {
>> +			.fmt.pix.pixelformat = V4L2_PIX_FMT_YUV420,
>> +			.fmt.pix.width = 10000,
>> +			.fmt.pix.height = 10000,
>> +		};
>> +
>> +		ret = atomisp_set_fmt(&pipe->vdev, &f);
>> +		if (ret)
>> +			goto out;
>> +
>> +		ret = atomisp_get_css_frame_info(pipe->asd, source_pad, &pipe->frame_info);
>> +		if (ret)
>> +			goto out;
>> +	}
>>  
>>  	atomisp_alloc_css_stat_bufs(pipe->asd, ATOMISP_INPUT_STREAM_GENERAL);
>>  
>>  	*nplanes = 1;
>>  	sizes[0] = PAGE_ALIGN(pipe->pix.sizeimage);
>>  
>> +out:
>> +	mutex_unlock(&pipe->asd->isp->mutex);
>>  	return 0;
>>  }
>>  
> 

Re: [PATCH v2 12/17] media: atomisp: Fix VIDIOC_REQBUFS failing when VIDIOC_S_FMT has not been called yet

[Hans de Goede]

Hi,

On 11/14/22 13:17, Andy Shevchenko wrote:
> On Thu, Oct 20, 2022 at 09:55:28PM +0200, Hans de Goede wrote:
>> camorama calls VIDIOC_REQBUFS(V4L2_MEMORY_MMAP) to test if MMAP support
>> works (this was added specifically because of the previously broken
>> MMAP support in atomisp).
>>
>> Currently this fails because atomisp_get_css_frame_info() fails in this
>> case. Although it is weird to call VIDIOC_REQBUFS before VIDIOC_S_FMT,
>> it is allowed to do this.
>>
>> Fix this not working by doing a S_FMT to V4L2_PIX_FMT_YUV420 + the highest
>> supported resolution.
>>
>> Note this will cause camorama to use mmap mode, which means it will also
>> use libv4l2 to do format conversion. libv4l2 will pick V4L2_PIX_FMT_RGB565
>> as input format and this will lead to a garbled video display. This is
>> a libv4lconvert bug, the RGB565 -> RGB24 path in libv4lconvert assumes
>> that stride == width which is not true on the atomisp.
>>
>> I've already send out a libv4lconvert fix for this. Also this can be worked
> 
> Link: ?
> 

Fixed in my media-atomisp branch.

>> around by passing --dont-use-libv4l2 as argument to camorama.
> 
> ...
> 
>> +		struct v4l2_format f = {
>> +			.fmt.pix.pixelformat = V4L2_PIX_FMT_YUV420,
> 
>> +			.fmt.pix.width = 10000,
>> +			.fmt.pix.height = 10000,
> 
> If it's just an arbitrary maximum, I would suggest to use definitions from
> limits.h, so it will better show the intention.

This is duplicating existing code in another path in atomisp2 where it
does this when it needs to pick a fmt because it needs one and userspace
has not set one yet.

I have added an entry to my atomisp TODO list to writer a helper for
this so that we only have these magic 10000 values only once.

As for if 10000 really is necessary, or if e.g. 65535 might work
as well, that is unclear.

Regards,

Hans



> 
>> +		};
> 

Re: [PATCH v2 17/17] media: atomisp: gc0310: Power on sensor from set_fmt() callback

[Hans de Goede]

Hi,

On 11/14/22 13:20, Andy Shevchenko wrote:
> On Thu, Oct 20, 2022 at 09:55:33PM +0200, Hans de Goede wrote:
>> Depending on which order userspace makes various v4l2 calls, the sensor
>> might still be powered down when set_fmt is called.
>>
>> What should really happen here is delay the writing of the mode-related
>> registers till streaming is started, but for now use the same quick fix
>> as the atomisp_ov2680 code and call power_up() from set_fmt() in
>> combination with keeping track of the power-state to avoid doing the
>> power-up sequence twice.
> 
> ...
> 
>> +	/* s_power has not been called yet for std v4l2 clients (camorama) */
>> +	power_up(sd);
> 
> if camorama is fixed, will this become a problem?

This is not a camorama issue but an issue with the atomisp2 + sensor driver
combination. camorama uses a slightly different order in which various
v4l2 ioctls are done so it needs the power_up() here. Note that power_up()
checks + sets a flag so that if it gets called multiple times it only does
the actual power-up once.

Regards,

Hans

Re: [PATCH v2 00/17] media: atomisp: Convert to videobuf2

[Hans de Goede]

Hi,

On 11/14/22 13:22, Andy Shevchenko wrote:
> On Thu, Oct 20, 2022 at 09:55:16PM +0200, Hans de Goede wrote:
>> Hi All,
>>
>> Here is v2 of my patch series converting the staging atomisp driver to use
>> the videobuf2 framework, fixing MMAP mode not working.
>>
>> New in v2 is that the "media: atomisp: Convert to videobuf2" patch
>> now also contains moving over to using a per vb_queue lock as is
>> standard for v4l2 drivers. This removes a bunch of FIXME + checkpatch
>> warnings (due to commented out prep for this) from v1 of this patch.
>> This v2 also fixes the 1 new warning pointed out by the lkp test robot.
>>
>> For some more background info here are the still relevant bits of
>> the v1 cover-letter:
>>
>> This also contains an important fix to try_fmt handling, so that
>> the various supported output formats can actually be used by userspace
>> which calls try_fmt before doing s_fmt.
>>
>> So slowly this is starting to look more and more like a standard
>> v4l2 device (with all the complex pipeline handling hidden in the driver,
>> moving that to userspace seems to be impossible with this design since
>> multiple pipeline steps are handled by a single firmware program).
> 
> I completely in favour of the series, so
> Reviewed-by: Andy Shevchenko <andy@kernel.org>
> for non-commented patches, for the commented, where comment is a "side note".
> 
> For the rest depends on your action, if you are going to address as suggested,
> feel free to add my tag.

Thanks, I've pushed an updated version with your suggested changes and
your Reviewed-by added to my media-atomisp branch in preparation for
sending a pull-req to Mauro with these changes.

I'll also add your patch "[PATCH v1 1/1] atomisp: Make bds_factors_list be type
of struct u32_fract" there once I have had the time to review it.

Note since I send out this series I've been working on a bunch of further
cleanups / on preparation work for getting rid of the ugly PCI power-management
errors on (runtime)suspend/resume.

I will send these out as a patch series now and I've also pushed these to my
media-atomisp branch.

Regards,

Hans

Re: [PATCH v2 12/17] media: atomisp: Fix VIDIOC_REQBUFS failing when VIDIOC_S_FMT has not been called yet

[Mauro Carvalho Chehab]

Em Sun, 20 Nov 2022 23:17:56 +0100
Hans de Goede <hdegoede@redhat.com> escreveu:

> Hi,
> 
> On 11/14/22 21:38, Mauro Carvalho Chehab wrote:
> > Em Thu, 20 Oct 2022 21:55:28 +0200
> > Hans de Goede <hdegoede@redhat.com> escreveu:
> >   
> >> camorama calls VIDIOC_REQBUFS(V4L2_MEMORY_MMAP) to test if MMAP support
> >> works (this was added specifically because of the previously broken
> >> MMAP support in atomisp).
> >>
> >> Currently this fails because atomisp_get_css_frame_info() fails in this
> >> case. Although it is weird to call VIDIOC_REQBUFS before VIDIOC_S_FMT,
> >> it is allowed to do this.
> >>
> >> Fix this not working by doing a S_FMT to V4L2_PIX_FMT_YUV420 + the highest
> >> supported resolution.
> >>
> >> Note this will cause camorama to use mmap mode, which means it will also
> >> use libv4l2 to do format conversion. libv4l2 will pick V4L2_PIX_FMT_RGB565
> >> as input format and this will lead to a garbled video display. This is
> >> a libv4lconvert bug, the RGB565 -> RGB24 path in libv4lconvert assumes
> >> that stride == width which is not true on the atomisp.  
> > 
> > Hmm... should we add a patch on Camorama for it to not use libv4l2 if
> > format == V4L2_PIX_FMT_RGB565?  
> 
> This is not controlled by Camorama but by libv4lconvert, AFAICT there are
> no atomisp2 native formats which are supported in Camorama without libv4l  ?

Actually, I added support on Camorama to decode it, plus other formats.
It currently supports:

	V4L2_PIX_FMT_ABGR32
	V4L2_PIX_FMT_ARGB32
	V4L2_PIX_FMT_BGR24
	V4L2_PIX_FMT_BGR32
	V4L2_PIX_FMT_NV12
	V4L2_PIX_FMT_NV21
	V4L2_PIX_FMT_RGB24
	V4L2_PIX_FMT_RGB32
	V4L2_PIX_FMT_RGB565
	V4L2_PIX_FMT_RGB565X
	V4L2_PIX_FMT_UYVY
	V4L2_PIX_FMT_VYUY
	V4L2_PIX_FMT_YUYV
	V4L2_PIX_FMT_YVYU
	V4L2_PIX_FMT_XBGR32
	V4L2_PIX_FMT_XRGB32
	V4L2_PIX_FMT_YUV420
	V4L2_PIX_FMT_YVU420

It is just that it defaults to request RGB24 when not in userptr mode
or when --dont-use-libv4l2 is not used.

I guess it makes sense to make it smarter by using a native format,
using its internal logic if the format is directly supported on it.

It would also make sense to add Bayer and MPEG formats to it
as well.

> >> I've already send out a libv4lconvert fix for this. Also this can be worked
> >> around by passing --dont-use-libv4l2 as argument to camorama.  
> > 
> > Was the patch already applied? The better would be to apply it at
> > libv4l2 upstream, as a fix, porting it to old versions, and mentioning
> > on what versions this got fixed on this changeset.  
> 
> I see that you have already found the patches. I will add a comment
> to the commit msg pointing to the now applied patch.

Ok.

Regards,
Mauro