On 03/12/2019 03:43 PM, Tomasz Figa wrote:
On Tue, Mar 12, 2019 at 3:48 PM Bingbu Cao <bingbu.cao@xxxxxxxxxxxxxxx> wrote:
On 03/12/2019 01:33 PM, Tomasz Figa wrote:
Hi Bingbu,
On Fri, Feb 15, 2019 at 6:02 PM <bingbu.cao@xxxxxxxxx> wrote:
From: Bingbu Cao <bingbu.cao@xxxxxxxxx>
Current ImgU driver processes and releases the parameter buffer
immediately after queued from user. This does not align with other
image buffers which are grouped in sets and used for the same frame.
If user queues multiple parameter buffers continuously, only the last
one will take effect.
To make consistent buffers usage, this patch changes the parameter
buffer handling and group parameter buffer with other image buffers
for each frame.
Thanks for the patch. Please see my comments inline.
Signed-off-by: Tianshu Qiu <tian.shu.qiu@xxxxxxxxx>
Signed-off-by: Bingbu Cao <bingbu.cao@xxxxxxxxx>
---
drivers/staging/media/ipu3/ipu3-css.c | 5 -----
drivers/staging/media/ipu3/ipu3-v4l2.c | 41 ++++++++--------------------------
drivers/staging/media/ipu3/ipu3.c | 24 ++++++++++++++++++++
3 files changed, 33 insertions(+), 37 deletions(-)
diff --git a/drivers/staging/media/ipu3/ipu3-css.c b/drivers/staging/media/ipu3/ipu3-css.c
index b9354d2bb692..bcb1d436bc98 100644
--- a/drivers/staging/media/ipu3/ipu3-css.c
+++ b/drivers/staging/media/ipu3/ipu3-css.c
@@ -2160,11 +2160,6 @@ int ipu3_css_set_parameters(struct ipu3_css *css, unsigned int pipe,
obgrid_size = ipu3_css_fw_obgrid_size(bi);
stripes = bi->info.isp.sp.iterator.num_stripes ? : 1;
- /*
- * TODO(b/118782861): If userspace queues more than 4 buffers, the
- * parameters from previous buffers will be overwritten. Fix the driver
- * not to allow this.
- */
Wouldn't this still happen even with current patch?
imgu_queue_buffers() supposedly queues "as many buffers to CSS as
possible". This means that if the userspace queues more than 4
complete frames, we still end up overwriting the parameter buffers in
the pool. Please correct me if I'm wrong.
The parameter buffers are queued to CSS sequentially and queue one
parameter along with one input buffer once ready, all the data and
parameter buffers are tied together to queue to the CSS. If userspace
queue more parameter buffers then input buffer, they are pending on the
buffer list.
It doesn't seem to be what the code does. I'm talking about the
following example:
Queue OUT buffer 1
Queue PARAM buffer 1
Queue IN buffer 1
Queue OUT buffer 2
Queue PARAM buffer 2
Queue IN buffer 2
Queue OUT buffer 3
Queue PARAM buffer 3
Queue IN buffer 3
Queue OUT buffer 4
Queue PARAM buffer 4
Queue IN buffer 4
Queue OUT buffer 5
Queue PARAM buffer 5
Queue IN buffer 5
All the operations happening exactly one after each other. How would
the code prevent the 5th PARAM buffer to be queued to the IMGU, after
the 5th IN buffer is queued? As I said, imgu_queue_buffers() just
queues as many buffers of each type as there are IN buffers available.
So the parameter pool now is only used as record last valid parameter not
used as a list or cached, all the parameters will be queued to CSS as soon as
possible(if queue for CSS is not full).
As the size of pool now is a bit confusing, I think we can shrink the its value
for each pipe to 2.
The buffer queue size is limited for CSS, if massive buffers from
user pass down and the css queue is full, driver will get -EBUSY
and return the buffers to user with error.
Best regards,
Tomasz
