Re: [PATCH v12 07/29] media: v4l2-mem2mem: add v4l2_m2m_suspend, v4l2_m2m_resume

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

I'm having a problem with this patch which landed in linux-next.

On Fri, 14 Aug 2020, Xia Jiang <xia.jiang@xxxxxxxxxxxx> wrote:
From: Pi-Hsun Shih <pihsun@xxxxxxxxxxxx> Add two functions that can be used to stop new jobs from being queued / continue running queued job. This can be used while a driver using m2m helper is going to suspend / wake up from resume, and can ensure that there's no job running in suspend process. BUG=b:143046833 TEST=build Signed-off-by: Pi-Hsun Shih <pihsun@xxxxxxxxxxxx> Signed-off-by: Jerry-ch Chen <jerry-ch.chen@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx> Reviewed-by: Tomasz Figa <tfiga@xxxxxxxxxxxx> --- v12: add this relied patch to the series --- drivers/media/v4l2-core/v4l2-mem2mem.c | 41 ++++++++++++++++++++++++++ include/media/v4l2-mem2mem.h | 22 ++++++++++++++ 2 files changed, 63 insertions(+) diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c b/drivers/media/v4l2-core/v4l2-mem2mem.c index 62ac9424c92a..ddfdb6375064 100644 --- a/drivers/media/v4l2-core/v4l2-mem2mem.c +++ b/drivers/media/v4l2-core/v4l2-mem2mem.c @@ -43,6 +43,10 @@ module_param(debug, bool, 0644); #define TRANS_ABORT (1 << 2) +/* The job queue is not running new jobs */ +#define QUEUE_PAUSED (1 << 0) + + /* Offset base for buffers on the destination queue - used to distinguish * between source and destination buffers when mmapping - they receive the same * offsets but for different queues */ @@ -84,6 +88,7 @@ static const char * const m2m_entity_name[] = { * @job_queue: instances queued to run * @job_spinlock: protects job_queue * @job_work: worker to run queued jobs. + * @job_queue_flags: flags of the queue status, %QUEUE_PAUSED. * @m2m_ops: driver callbacks */ struct v4l2_m2m_dev { @@ -101,6 +106,7 @@ struct v4l2_m2m_dev { struct list_head job_queue; spinlock_t job_spinlock; struct work_struct job_work; + unsigned long job_queue_flags; const struct v4l2_m2m_ops *m2m_ops; }; @@ -263,6 +269,12 @@ static void v4l2_m2m_try_run(struct v4l2_m2m_dev *m2m_dev) return; } + if (m2m_dev->job_queue_flags & QUEUE_PAUSED) { + spin_unlock_irqrestore(&m2m_dev->job_spinlock, flags); + dprintk("Running new jobs is paused\n"); + return; + } + m2m_dev->curr_ctx = list_first_entry(&m2m_dev->job_queue, struct v4l2_m2m_ctx, queue); m2m_dev->curr_ctx->job_flags |= TRANS_RUNNING; @@ -504,6 +516,7 @@ void v4l2_m2m_buf_done_and_job_finish(struct v4l2_m2m_dev *m2m_dev, if (WARN_ON(!src_buf || !dst_buf)) goto unlock; + v4l2_m2m_buf_done(src_buf, state);

This line looks out of place in this commit and is causing a lot of warnings (1 per frame). Any reason in particular why we need this?

[ 87.825061] ------------[ cut here ]------------ [ 87.829695] WARNING: CPU: 0 PID: 0 at drivers/media/common/videobuf2/videobuf2-core.c:986 vb2_buffer_done+0x208/0x2a0 [ 87.840302] Modules linked in: [ 87.843364] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G W 5.9.0-rc6-next-20200924+ #472 [ 87.852407] Hardware name: NXP i.MX8MQ EVK (DT) [ 87.856942] pstate: 20000085 (nzCv daIf -PAN -UAO -TCO BTYPE=--) [ 87.862953] pc : vb2_buffer_done+0x208/0x2a0 [ 87.867224] lr : v4l2_m2m_buf_done_and_job_finish+0x94/0x140 [ 87.872882] sp : ffff80001183bd50 [ 87.876195] x29: ffff80001183bd50 x28: ffff8000115d1500 [ 87.881512] x27: ffff80001128e018 x26: ffff00009fb4a828 [ 87.886828] x25: ffff0000a4e13a08 x24: 0000000000000080 [ 87.892143] x23: 0000000000000005 x22: ffff0000a253bc00 [ 87.897457] x21: ffff00009fb4aa98 x20: ffff00009fb4a800 [ 87.902772] x19: ffff0000a24f0000 x18: 0000000000000000 [ 87.908086] x17: 0000000000000000 x16: 0000000000000000 [ 87.913400] x15: 0000000000000000 x14: 0000000000000500 [ 87.918714] x13: 0000000000000003 x12: 0000000000000000 [ 87.924028] x11: 0000000000000040 x10: ffff800011658520 [ 87.929340] x9 : ffff800010998464 x8 : ffff0000a5800270 [ 87.934655] x7 : 0000000000000000 x6 : ffff00009fb4aa20 [ 87.939969] x5 : ffff80001183bd10 x4 : 0000000000000000 [ 87.945285] x3 : 0000000000000000 x2 : 0000000000000000 [ 87.950599] x1 : 0000000000000005 x0 : 0000000000000005 [ 87.955914] Call trace: [ 87.958364] vb2_buffer_done+0x208/0x2a0 [ 87.962288] v4l2_m2m_buf_done_and_job_finish+0x94/0x140 [ 87.967601] hantro_job_finish+0xa8/0xe0 [ 87.971524] hantro_irq_done+0x58/0x90 [ 87.975275] imx8m_vpu_g1_irq+0x8c/0x160 [ 87.979201] __handle_irq_event_percpu+0x68/0x2a0 [ 87.983905] handle_irq_event_percpu+0x3c/0xa0 [ 87.988347] handle_irq_event+0x50/0xf0 [ 87.992185] handle_fasteoi_irq+0xc0/0x180 [ 87.996283] generic_handle_irq+0x38/0x50 [ 88.000296] __handle_domain_irq+0x6c/0xd0 [ 88.004393] gic_handle_irq+0x60/0x12c [ 88.008143] el1_irq+0xbc/0x180 [ 88.011287] arch_cpu_idle+0x1c/0x30 [ 88.014864] do_idle+0x220/0x270 [ 88.018093] cpu_startup_entry+0x30/0x70 [ 88.022019] rest_init+0xe0/0xf0 [ 88.025250] arch_call_rest_init+0x18/0x24 [ 88.029347] start_kernel+0x7a4/0x7e0 [ 88.033013] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G W 5.9.0-rc6-next-20200924+ #472 [ 88.042056] Hardware name: NXP i.MX8MQ EVK (DT) [ 88.046585] Call trace: [ 88.049034] dump_backtrace+0x0/0x1b0 [ 88.052697] show_stack+0x20/0x70 [ 88.056014] dump_stack+0xd0/0x12c [ 88.059418] __warn+0xfc/0x180 [ 88.062474] report_bug+0xfc/0x170 [ 88.065875] bug_handler+0x28/0x70 [ 88.069276] brk_handler+0x70/0xe0 [ 88.072681] do_debug_exception+0xcc/0x1e0 [ 88.076776] el1_sync_handler+0xd8/0x140 [ 88.080697] el1_sync+0x80/0x100 [ 88.083926] vb2_buffer_done+0x208/0x2a0 [ 88.087852] v4l2_m2m_buf_done_and_job_finish+0x94/0x140 [ 88.093162] hantro_job_finish+0xa8/0xe0 [ 88.097083] hantro_irq_done+0x58/0x90 [ 88.100831] imx8m_vpu_g1_irq+0x8c/0x160 [ 88.104753] __handle_irq_event_percpu+0x68/0x2a0 [ 88.109456] handle_irq_event_percpu+0x3c/0xa0 [ 88.113899] handle_irq_event+0x50/0xf0 [ 88.117734] handle_fasteoi_irq+0xc0/0x180 [ 88.121832] generic_handle_irq+0x38/0x50 [ 88.125841] __handle_domain_irq+0x6c/0xd0 [ 88.129937] gic_handle_irq+0x60/0x12c [ 88.133686] el1_irq+0xbc/0x180 [ 88.136827] arch_cpu_idle+0x1c/0x30 [ 88.140403] do_idle+0x220/0x270 [ 88.143630] cpu_startup_entry+0x30/0x70 [ 88.147553] rest_init+0xe0/0xf0 [ 88.150782] arch_call_rest_init+0x18/0x24 [ 88.154879] start_kernel+0x7a4/0x7e0 [ 88.158542] ---[ end trace 847145312866dff5 ]---

Regards,
Adrian

 	dst_buf->is_held = src_buf->flags & V4L2_BUF_FLAG_M2M_HOLD_CAPTURE_BUF;
 	if (!dst_buf->is_held) {
 		v4l2_m2m_dst_buf_remove(m2m_ctx);
@@ -528,6 +541,34 @@ void v4l2_m2m_buf_done_and_job_finish(struct v4l2_m2m_dev *m2m_dev,
 }
 EXPORT_SYMBOL(v4l2_m2m_buf_done_and_job_finish);
+void v4l2_m2m_suspend(struct v4l2_m2m_dev *m2m_dev)
+{
+	unsigned long flags;
+	struct v4l2_m2m_ctx *curr_ctx;
+
+	spin_lock_irqsave(&m2m_dev->job_spinlock, flags);
+	m2m_dev->job_queue_flags |= QUEUE_PAUSED;
+	curr_ctx = m2m_dev->curr_ctx;
+	spin_unlock_irqrestore(&m2m_dev->job_spinlock, flags);
+
+	if (curr_ctx)
+		wait_event(curr_ctx->finished,
+			   !(curr_ctx->job_flags & TRANS_RUNNING));
+}
+EXPORT_SYMBOL(v4l2_m2m_suspend);
+
+void v4l2_m2m_resume(struct v4l2_m2m_dev *m2m_dev)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&m2m_dev->job_spinlock, flags);
+	m2m_dev->job_queue_flags &= ~QUEUE_PAUSED;
+	spin_unlock_irqrestore(&m2m_dev->job_spinlock, flags);
+
+	v4l2_m2m_try_run(m2m_dev);
+}
+EXPORT_SYMBOL(v4l2_m2m_resume);
+
 int v4l2_m2m_reqbufs(struct file *file, struct v4l2_m2m_ctx *m2m_ctx,
 		     struct v4l2_requestbuffers *reqbufs)
 {
diff --git a/include/media/v4l2-mem2mem.h b/include/media/v4l2-mem2mem.h
index 98753f00df7e..5a91b548ecc0 100644
--- a/include/media/v4l2-mem2mem.h
+++ b/include/media/v4l2-mem2mem.h
@@ -304,6 +304,28 @@ v4l2_m2m_is_last_draining_src_buf(struct v4l2_m2m_ctx *m2m_ctx,
 void v4l2_m2m_last_buffer_done(struct v4l2_m2m_ctx *m2m_ctx,
 			       struct vb2_v4l2_buffer *vbuf);
+/**
+ * v4l2_m2m_suspend() - stop new jobs from being run and wait for current job
+ * to finish
+ *
+ * @m2m_dev: opaque pointer to the internal data to handle M2M context
+ *
+ * Called by a driver in the suspend hook. Stop new jobs from being run, and
+ * wait for current running job to finish.
+ */
+void v4l2_m2m_suspend(struct v4l2_m2m_dev *m2m_dev);
+
+/**
+ * v4l2_m2m_resume() - resume job running and try to run a queued job
+ *
+ * @m2m_dev: opaque pointer to the internal data to handle M2M context
+ *
+ * Called by a driver in the resume hook. This reverts the operation of
+ * v4l2_m2m_suspend() and allows job to be run. Also try to run a queued job if
+ * there is any.
+ */
+void v4l2_m2m_resume(struct v4l2_m2m_dev *m2m_dev);
+
 /**
  * v4l2_m2m_reqbufs() - multi-queue-aware REQBUFS multiplexer
  *
--
2.18.0



[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux