Skip to content

Commit 178aa33

Browse files
Guoniu ZhouHans Verkuil
Guoniu Zhou
authored and
Hans Verkuil
committed
media: nxp: imx8-isi: m2m: Fix streaming cleanup on release
If streamon/streamoff calls are imbalanced, such as when exiting an application with Ctrl+C when streaming, the m2m usage_count will never reach zero and the ISI channel won't be freed. Besides from that, if the input line width is more than 2K, it will trigger a WARN_ON(): [ 59.222120] ------------[ cut here ]------------ [ 59.226758] WARNING: drivers/media/platform/nxp/imx8-isi/imx8-isi-hw.c:631 at mxc_isi_channel_chain+0xa4/0x120, CPU#4: v4l2-ctl/654 [ 59.238569] Modules linked in: ap1302 [ 59.242231] CPU: 4 UID: 0 PID: 654 Comm: v4l2-ctl Not tainted 6.16.0-rc4-next-20250704-06511-gff0e002d480a-dirty #258 PREEMPT [ 59.253597] Hardware name: NXP i.MX95 15X15 board (DT) [ 59.258720] pstate: 80400009 (Nzcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--) [ 59.265669] pc : mxc_isi_channel_chain+0xa4/0x120 [ 59.270358] lr : mxc_isi_channel_chain+0x44/0x120 [ 59.275047] sp : ffff8000848c3b40 [ 59.278348] x29: ffff8000848c3b40 x28: ffff0000859b4c98 x27: ffff800081939f00 [ 59.285472] x26: 000000000000000a x25: ffff0000859b4cb8 x24: 0000000000000001 [ 59.292597] x23: ffff0000816f4760 x22: ffff0000816f4258 x21: ffff000084ceb780 [ 59.299720] x20: ffff000084342ff8 x19: ffff000084340000 x18: 0000000000000000 [ 59.306845] x17: 0000000000000000 x16: 0000000000000000 x15: 0000ffffdb369e1c [ 59.313969] x14: 0000000000000000 x13: 0000000000000000 x12: 0000000000000000 [ 59.321093] x11: 0000000000000000 x10: 0000000000000000 x9 : 0000000000000000 [ 59.328217] x8 : ffff8000848c3d48 x7 : ffff800081930b30 x6 : ffff800081930b30 [ 59.335340] x5 : ffff0000859b6000 x4 : ffff80008193ae80 x3 : ffff800081022420 [ 59.342464] x2 : ffff0000852f6900 x1 : 0000000000000001 x0 : ffff000084341000 [ 59.349590] Call trace: [ 59.352025] mxc_isi_channel_chain+0xa4/0x120 (P) [ 59.356722] mxc_isi_m2m_streamon+0x160/0x20c [ 59.361072] v4l_streamon+0x24/0x30 [ 59.364556] __video_do_ioctl+0x40c/0x4a0 [ 59.368560] video_usercopy+0x2bc/0x690 [ 59.372382] video_ioctl2+0x18/0x24 [ 59.375857] v4l2_ioctl+0x40/0x60 [ 59.379168] __arm64_sys_ioctl+0xac/0x104 [ 59.383172] invoke_syscall+0x48/0x104 [ 59.386916] el0_svc_common.constprop.0+0xc0/0xe0 [ 59.391613] do_el0_svc+0x1c/0x28 [ 59.394915] el0_svc+0x34/0xf4 [ 59.397966] el0t_64_sync_handler+0xa0/0xe4 [ 59.402143] el0t_64_sync+0x198/0x19c [ 59.405801] ---[ end trace 0000000000000000 ]--- Address this issue by moving the streaming preparation and cleanup to the vb2 .prepare_streaming() and .unprepare_streaming() operations. This also simplifies the driver by allowing direct usage of the v4l2_m2m_ioctl_streamon() and v4l2_m2m_ioctl_streamoff() helpers. Fixes: cf21f32 ("media: nxp: Add i.MX8 ISI driver") Cc: stable@vger.kernel.org Link: https://lore.kernel.org/r/20250821135123.29462-1-laurent.pinchart@ideasonboard.com Signed-off-by: Guoniu Zhou <guoniu.zhou@nxp.com> Co-developed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Tested-by: Guoniu Zhou <guoniu.zhou@nxp.com> Reviewed-by: Frank Li <Frank.Li@nxp.com> Signed-off-by: Hans Verkuil <hverkuil+cisco@kernel.org>
1 parent 45854b1 commit 178aa33

File tree

1 file changed

+92
-132
lines changed

1 file changed

+92
-132
lines changed

‎drivers/media/platform/nxp/imx8-isi/imx8-isi-m2m.c‎

Lines changed: 92 additions & 132 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@ struct mxc_isi_m2m_ctx_queue_data {
4343
struct v4l2_pix_format_mplane format;
4444
const struct mxc_isi_format_info *info;
4545
u32 sequence;
46-
bool streaming;
4746
};
4847

4948
struct mxc_isi_m2m_ctx {
@@ -236,6 +235,65 @@ static void mxc_isi_m2m_vb2_buffer_queue(struct vb2_buffer *vb2)
236235
v4l2_m2m_buf_queue(ctx->fh.m2m_ctx, vbuf);
237236
}
238237

238+
static int mxc_isi_m2m_vb2_prepare_streaming(struct vb2_queue *q)
239+
{
240+
struct mxc_isi_m2m_ctx *ctx = vb2_get_drv_priv(q);
241+
const struct v4l2_pix_format_mplane *out_pix = &ctx->queues.out.format;
242+
const struct v4l2_pix_format_mplane *cap_pix = &ctx->queues.cap.format;
243+
const struct mxc_isi_format_info *cap_info = ctx->queues.cap.info;
244+
const struct mxc_isi_format_info *out_info = ctx->queues.out.info;
245+
struct mxc_isi_m2m *m2m = ctx->m2m;
246+
int ret;
247+
248+
guard(mutex)(&m2m->lock);
249+
250+
if (m2m->usage_count == INT_MAX)
251+
return -EOVERFLOW;
252+
253+
/*
254+
* Acquire the pipe and initialize the channel with the first user of
255+
* the M2M device.
256+
*/
257+
if (m2m->usage_count == 0) {
258+
bool bypass = cap_pix->width == out_pix->width &&
259+
cap_pix->height == out_pix->height &&
260+
cap_info->encoding == out_info->encoding;
261+
262+
ret = mxc_isi_channel_acquire(m2m->pipe,
263+
&mxc_isi_m2m_frame_write_done,
264+
bypass);
265+
if (ret)
266+
return ret;
267+
268+
mxc_isi_channel_get(m2m->pipe);
269+
}
270+
271+
m2m->usage_count++;
272+
273+
/*
274+
* Allocate resources for the channel, counting how many users require
275+
* buffer chaining.
276+
*/
277+
if (!ctx->chained && out_pix->width > MXC_ISI_MAX_WIDTH_UNCHAINED) {
278+
ret = mxc_isi_channel_chain(m2m->pipe);
279+
if (ret)
280+
goto err_deinit;
281+
282+
m2m->chained_count++;
283+
ctx->chained = true;
284+
}
285+
286+
return 0;
287+
288+
err_deinit:
289+
if (--m2m->usage_count == 0) {
290+
mxc_isi_channel_put(m2m->pipe);
291+
mxc_isi_channel_release(m2m->pipe);
292+
}
293+
294+
return ret;
295+
}
296+
239297
static int mxc_isi_m2m_vb2_start_streaming(struct vb2_queue *q,
240298
unsigned int count)
241299
{
@@ -265,13 +323,44 @@ static void mxc_isi_m2m_vb2_stop_streaming(struct vb2_queue *q)
265323
}
266324
}
267325

326+
static void mxc_isi_m2m_vb2_unprepare_streaming(struct vb2_queue *q)
327+
{
328+
struct mxc_isi_m2m_ctx *ctx = vb2_get_drv_priv(q);
329+
struct mxc_isi_m2m *m2m = ctx->m2m;
330+
331+
guard(mutex)(&m2m->lock);
332+
333+
/*
334+
* If the last context is this one, reset it to make sure the device
335+
* will be reconfigured when streaming is restarted.
336+
*/
337+
if (m2m->last_ctx == ctx)
338+
m2m->last_ctx = NULL;
339+
340+
/* Free the channel resources if this is the last chained context. */
341+
if (ctx->chained && --m2m->chained_count == 0)
342+
mxc_isi_channel_unchain(m2m->pipe);
343+
ctx->chained = false;
344+
345+
/* Turn off the light with the last user. */
346+
if (--m2m->usage_count == 0) {
347+
mxc_isi_channel_disable(m2m->pipe);
348+
mxc_isi_channel_put(m2m->pipe);
349+
mxc_isi_channel_release(m2m->pipe);
350+
}
351+
352+
WARN_ON(m2m->usage_count < 0);
353+
}
354+
268355
static const struct vb2_ops mxc_isi_m2m_vb2_qops = {
269356
.queue_setup = mxc_isi_m2m_vb2_queue_setup,
270357
.buf_init = mxc_isi_m2m_vb2_buffer_init,
271358
.buf_prepare = mxc_isi_m2m_vb2_buffer_prepare,
272359
.buf_queue = mxc_isi_m2m_vb2_buffer_queue,
360+
.prepare_streaming = mxc_isi_m2m_vb2_prepare_streaming,
273361
.start_streaming = mxc_isi_m2m_vb2_start_streaming,
274362
.stop_streaming = mxc_isi_m2m_vb2_stop_streaming,
363+
.unprepare_streaming = mxc_isi_m2m_vb2_unprepare_streaming,
275364
};
276365

277366
static int mxc_isi_m2m_queue_init(void *priv, struct vb2_queue *src_vq,
@@ -481,135 +570,6 @@ static int mxc_isi_m2m_s_fmt_vid(struct file *file, void *fh,
481570
return 0;
482571
}
483572

484-
static int mxc_isi_m2m_streamon(struct file *file, void *fh,
485-
enum v4l2_buf_type type)
486-
{
487-
struct mxc_isi_m2m_ctx *ctx = file_to_isi_m2m_ctx(file);
488-
struct mxc_isi_m2m_ctx_queue_data *q = mxc_isi_m2m_ctx_qdata(ctx, type);
489-
const struct v4l2_pix_format_mplane *out_pix = &ctx->queues.out.format;
490-
const struct v4l2_pix_format_mplane *cap_pix = &ctx->queues.cap.format;
491-
const struct mxc_isi_format_info *cap_info = ctx->queues.cap.info;
492-
const struct mxc_isi_format_info *out_info = ctx->queues.out.info;
493-
struct mxc_isi_m2m *m2m = ctx->m2m;
494-
int ret;
495-
496-
if (q->streaming)
497-
return 0;
498-
499-
mutex_lock(&m2m->lock);
500-
501-
if (m2m->usage_count == INT_MAX) {
502-
ret = -EOVERFLOW;
503-
goto unlock;
504-
}
505-
506-
/*
507-
* Acquire the pipe and initialize the channel with the first user of
508-
* the M2M device.
509-
*/
510-
if (m2m->usage_count == 0) {
511-
bool bypass = cap_pix->width == out_pix->width &&
512-
cap_pix->height == out_pix->height &&
513-
cap_info->encoding == out_info->encoding;
514-
515-
ret = mxc_isi_channel_acquire(m2m->pipe,
516-
&mxc_isi_m2m_frame_write_done,
517-
bypass);
518-
if (ret)
519-
goto unlock;
520-
521-
mxc_isi_channel_get(m2m->pipe);
522-
}
523-
524-
m2m->usage_count++;
525-
526-
/*
527-
* Allocate resources for the channel, counting how many users require
528-
* buffer chaining.
529-
*/
530-
if (!ctx->chained && out_pix->width > MXC_ISI_MAX_WIDTH_UNCHAINED) {
531-
ret = mxc_isi_channel_chain(m2m->pipe);
532-
if (ret)
533-
goto deinit;
534-
535-
m2m->chained_count++;
536-
ctx->chained = true;
537-
}
538-
539-
/*
540-
* Drop the lock to start the stream, as the .device_run() operation
541-
* needs to acquire it.
542-
*/
543-
mutex_unlock(&m2m->lock);
544-
ret = v4l2_m2m_ioctl_streamon(file, fh, type);
545-
if (ret) {
546-
/* Reacquire the lock for the cleanup path. */
547-
mutex_lock(&m2m->lock);
548-
goto unchain;
549-
}
550-
551-
q->streaming = true;
552-
553-
return 0;
554-
555-
unchain:
556-
if (ctx->chained && --m2m->chained_count == 0)
557-
mxc_isi_channel_unchain(m2m->pipe);
558-
ctx->chained = false;
559-
560-
deinit:
561-
if (--m2m->usage_count == 0) {
562-
mxc_isi_channel_put(m2m->pipe);
563-
mxc_isi_channel_release(m2m->pipe);
564-
}
565-
566-
unlock:
567-
mutex_unlock(&m2m->lock);
568-
return ret;
569-
}
570-
571-
static int mxc_isi_m2m_streamoff(struct file *file, void *fh,
572-
enum v4l2_buf_type type)
573-
{
574-
struct mxc_isi_m2m_ctx *ctx = file_to_isi_m2m_ctx(file);
575-
struct mxc_isi_m2m_ctx_queue_data *q = mxc_isi_m2m_ctx_qdata(ctx, type);
576-
struct mxc_isi_m2m *m2m = ctx->m2m;
577-
578-
v4l2_m2m_ioctl_streamoff(file, fh, type);
579-
580-
if (!q->streaming)
581-
return 0;
582-
583-
mutex_lock(&m2m->lock);
584-
585-
/*
586-
* If the last context is this one, reset it to make sure the device
587-
* will be reconfigured when streaming is restarted.
588-
*/
589-
if (m2m->last_ctx == ctx)
590-
m2m->last_ctx = NULL;
591-
592-
/* Free the channel resources if this is the last chained context. */
593-
if (ctx->chained && --m2m->chained_count == 0)
594-
mxc_isi_channel_unchain(m2m->pipe);
595-
ctx->chained = false;
596-
597-
/* Turn off the light with the last user. */
598-
if (--m2m->usage_count == 0) {
599-
mxc_isi_channel_disable(m2m->pipe);
600-
mxc_isi_channel_put(m2m->pipe);
601-
mxc_isi_channel_release(m2m->pipe);
602-
}
603-
604-
WARN_ON(m2m->usage_count < 0);
605-
606-
mutex_unlock(&m2m->lock);
607-
608-
q->streaming = false;
609-
610-
return 0;
611-
}
612-
613573
static const struct v4l2_ioctl_ops mxc_isi_m2m_ioctl_ops = {
614574
.vidioc_querycap = mxc_isi_m2m_querycap,
615575

@@ -630,8 +590,8 @@ static const struct v4l2_ioctl_ops mxc_isi_m2m_ioctl_ops = {
630590
.vidioc_prepare_buf = v4l2_m2m_ioctl_prepare_buf,
631591
.vidioc_create_bufs = v4l2_m2m_ioctl_create_bufs,
632592

633-
.vidioc_streamon = mxc_isi_m2m_streamon,
634-
.vidioc_streamoff = mxc_isi_m2m_streamoff,
593+
.vidioc_streamon = v4l2_m2m_ioctl_streamon,
594+
.vidioc_streamoff = v4l2_m2m_ioctl_streamoff,
635595

636596
.vidioc_subscribe_event = v4l2_ctrl_subscribe_event,
637597
.vidioc_unsubscribe_event = v4l2_event_unsubscribe,

0 commit comments

Comments
 (0)