* [PATCH 5.15.y 2/3] media: atmel: atmel-isc-base: use mutex to lock awb workq from streaming
2026-11-11 2:07 ` [PATCH 5.15.y 1/3] media: atmel: atmel-isc: remove redundant comments Sasha Levin
@ 2026-11-11 2:07 ` Sasha Levin
2026-11-11 2:07 ` [PATCH 5.15.y 3/3] media: microchip-isc: do not touch WB registers when not streaming Sasha Levin
1 sibling, 0 replies; 4+ messages in thread
From: Sasha Levin @ 2026-11-11 2:07 UTC (permalink / raw)
To: stable
Cc: Eugen Hristev, Jacopo Mondi, Hans Verkuil, Mauro Carvalho Chehab,
Sasha Levin
From: Eugen Hristev <eugen.hristev@microchip.com>
[ Upstream commit 314c96e5203de1bac744e8536ef20b195f460674 ]
The AWB workqueue runs in a kernel thread and needs to be synchronized
w.r.t. the streaming status.
It is possible that streaming is stopped while the AWB workq is running.
In this case it is likely that the check for vb2_start_streaming_called is
done at one point in time, but the AWB computations are done later,
including a call to isc_update_profile, which requires streaming to be
started.
Thus , isc_update_profile will fail if during this operation sequence the
streaming was stopped.
To solve this issue, a mutex is added, that will serialize the awb work and
streaming stopping, with the mention that either streaming is stopped
completely including termination of the last frame is done, and after that
the AWB work can check stream status and stop; either first AWB work is
completed and after that the streaming can stop correctly.
The awb spin lock cannot be used since this spinlock is taken in the same
context and using it in the stop streaming will result in a recursion BUG.
Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
Stable-dep-of: d8983c47d4d9 ("media: microchip-isc: do not touch WB registers when not streaming")
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/media/platform/atmel/atmel-isc-base.c | 30 ++++++++++++++++---
drivers/media/platform/atmel/atmel-isc.h | 2 ++
2 files changed, 28 insertions(+), 4 deletions(-)
diff --git a/drivers/media/platform/atmel/atmel-isc-base.c b/drivers/media/platform/atmel/atmel-isc-base.c
index eeaf91d80acd7..565719fda13c3 100644
--- a/drivers/media/platform/atmel/atmel-isc-base.c
+++ b/drivers/media/platform/atmel/atmel-isc-base.c
@@ -696,6 +696,7 @@ static void isc_stop_streaming(struct vb2_queue *vq)
struct isc_buffer *buf;
int ret;
+ mutex_lock(&isc->awb_mutex);
v4l2_ctrl_activate(isc->do_wb_ctrl, false);
isc->stop = true;
@@ -705,6 +706,8 @@ static void isc_stop_streaming(struct vb2_queue *vq)
v4l2_err(&isc->v4l2_dev,
"Timeout waiting for end of the capture\n");
+ mutex_unlock(&isc->awb_mutex);
+
/* Disable DMA interrupt */
regmap_write(isc->regmap, ISC_INTDIS, ISC_INT_DDONE);
@@ -1696,10 +1699,6 @@ static void isc_awb_work(struct work_struct *w)
u32 min, max;
int ret;
- /* streaming is not active anymore */
- if (isc->stop)
- return;
-
if (ctrls->hist_stat != HIST_ENABLED)
return;
@@ -1750,7 +1749,24 @@ static void isc_awb_work(struct work_struct *w)
}
regmap_write(regmap, ISC_HIS_CFG + isc->offsets.his,
hist_id | baysel | ISC_HIS_CFG_RAR);
+
+ /*
+ * We have to make sure the streaming has not stopped meanwhile.
+ * ISC requires a frame to clock the internal profile update.
+ * To avoid issues, lock the sequence with a mutex
+ */
+ mutex_lock(&isc->awb_mutex);
+
+ /* streaming is not active anymore */
+ if (isc->stop) {
+ mutex_unlock(&isc->awb_mutex);
+ return;
+ };
+
isc_update_profile(isc);
+
+ mutex_unlock(&isc->awb_mutex);
+
/* if awb has been disabled, we don't need to start another histogram */
if (ctrls->awb)
regmap_write(regmap, ISC_CTRLEN, ISC_CTRL_HISREQ);
@@ -1829,6 +1845,7 @@ static int isc_s_awb_ctrl(struct v4l2_ctrl *ctrl)
isc_update_awb_ctrls(isc);
+ mutex_lock(&isc->awb_mutex);
if (vb2_is_streaming(&isc->vb2_vidq)) {
/*
* If we are streaming, we can update profile to
@@ -1843,6 +1860,7 @@ static int isc_s_awb_ctrl(struct v4l2_ctrl *ctrl)
*/
v4l2_ctrl_activate(isc->do_wb_ctrl, false);
}
+ mutex_unlock(&isc->awb_mutex);
/* if we have autowhitebalance on, start histogram procedure */
if (ctrls->awb == ISC_WB_AUTO &&
@@ -2024,6 +2042,7 @@ static void isc_async_unbind(struct v4l2_async_notifier *notifier,
{
struct isc_device *isc = container_of(notifier->v4l2_dev,
struct isc_device, v4l2_dev);
+ mutex_destroy(&isc->awb_mutex);
cancel_work_sync(&isc->awb_work);
video_unregister_device(&isc->video_dev);
v4l2_ctrl_handler_free(&isc->ctrls.handler);
@@ -2133,6 +2152,8 @@ static int isc_async_complete(struct v4l2_async_notifier *notifier)
isc->current_subdev = container_of(notifier,
struct isc_subdev_entity, notifier);
mutex_init(&isc->lock);
+ mutex_init(&isc->awb_mutex);
+
init_completion(&isc->comp);
/* Initialize videobuf2 queue */
@@ -2201,6 +2222,7 @@ static int isc_async_complete(struct v4l2_async_notifier *notifier)
return 0;
isc_async_complete_err:
+ mutex_destroy(&isc->awb_mutex);
mutex_destroy(&isc->lock);
return ret;
}
diff --git a/drivers/media/platform/atmel/atmel-isc.h b/drivers/media/platform/atmel/atmel-isc.h
index b0211100a5f23..0c3a452dafe33 100644
--- a/drivers/media/platform/atmel/atmel-isc.h
+++ b/drivers/media/platform/atmel/atmel-isc.h
@@ -207,6 +207,7 @@ struct isc_reg_offsets {
*
* @lock: lock for serializing userspace file operations
* with ISC operations
+ * @awb_mutex: serialize access to streaming status from awb work queue
* @awb_lock: lock for serializing awb work queue operations
* with DMA/buffer operations
*
@@ -279,6 +280,7 @@ struct isc_device {
struct work_struct awb_work;
struct mutex lock;
+ struct mutex awb_mutex;
spinlock_t awb_lock;
struct regmap_field *pipeline[ISC_PIPE_LINE_NODE_NUM];
--
2.53.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH 5.15.y 3/3] media: microchip-isc: do not touch WB registers when not streaming
2026-11-11 2:07 ` [PATCH 5.15.y 1/3] media: atmel: atmel-isc: remove redundant comments Sasha Levin
2026-11-11 2:07 ` [PATCH 5.15.y 2/3] media: atmel: atmel-isc-base: use mutex to lock awb workq from streaming Sasha Levin
@ 2026-11-11 2:07 ` Sasha Levin
1 sibling, 0 replies; 4+ messages in thread
From: Sasha Levin @ 2026-11-11 2:07 UTC (permalink / raw)
To: stable; +Cc: Balakrishnan Sambath, Eugen Hristev, Hans Verkuil, Sasha Levin
From: Balakrishnan Sambath <balakrishnan.s@microchip.com>
[ Upstream commit d8983c47d4d95eb1ed889dbb7516d3738fa854d8 ]
isc_s_awb_ctrl() called isc_update_awb_ctrls() unconditionally, writing
the white balance registers even when the device is runtime suspended;
on many ARM platforms accessing the unclocked registers is an external
abort. The write was also done without awb_lock, racing isc_awb_work(),
which holds it so the DMA done IRQ cannot latch a half-updated pipeline.
Write the registers only while streaming and not stopping, under
awb_lock, and update the profile there. The isc->stop check covers the
window where isc_stop_streaming() has gated the clocks but vb2 still
reports streaming. Otherwise the new values stay cached and
isc_configure() programs them at the next stream start.
Fixes: 4e52889f48fe ("media: atmel: atmel-isc-base: expose white balance as v4l2 controls")
Cc: stable@vger.kernel.org
Reviewed-by: Eugen Hristev <ehristev@kernel.org>
Signed-off-by: Balakrishnan Sambath <balakrishnan.s@microchip.com>
Signed-off-by: Hans Verkuil <hverkuil+cisco@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/media/platform/atmel/atmel-isc-base.c | 20 +++++++++++--------
1 file changed, 12 insertions(+), 8 deletions(-)
diff --git a/drivers/media/platform/atmel/atmel-isc-base.c b/drivers/media/platform/atmel/atmel-isc-base.c
index 565719fda13c3..dc12424965f3e 100644
--- a/drivers/media/platform/atmel/atmel-isc-base.c
+++ b/drivers/media/platform/atmel/atmel-isc-base.c
@@ -1843,20 +1843,24 @@ static int isc_s_awb_ctrl(struct v4l2_ctrl *ctrl)
if (ctrl->cluster[ISC_CTRL_GB_OFF]->is_new)
ctrls->offset[ISC_HIS_CFG_MODE_GB] = isc->gb_off_ctrl->val;
- isc_update_awb_ctrls(isc);
-
mutex_lock(&isc->awb_mutex);
- if (vb2_is_streaming(&isc->vb2_vidq)) {
+ if (vb2_is_streaming(&isc->vb2_vidq) && !isc->stop) {
+ unsigned long flags;
+
/*
- * If we are streaming, we can update profile to
- * have the new settings in place.
+ * awb_lock keeps the DMA done IRQ from latching a
+ * partially written WB pipeline.
*/
+ spin_lock_irqsave(&isc->awb_lock, flags);
+ isc_update_awb_ctrls(isc);
+ spin_unlock_irqrestore(&isc->awb_lock, flags);
+
isc_update_profile(isc);
} else {
/*
- * The auto cluster will activate automatically this
- * control. This has to be deactivated when not
- * streaming.
+ * Not streaming: keep the cached values for the next
+ * stream start and deactivate the cluster-activated
+ * do_white_balance button.
*/
v4l2_ctrl_activate(isc->do_wb_ctrl, false);
}
--
2.53.0
^ permalink raw reply related [flat|nested] 4+ messages in thread