[PATCH 06/16] media: rkisp1: Modernise the locking
Daniel Scally
dan.scally at ideasonboard.com
Mon Nov 2 23:40:42 PST 2026
Modernise the locking routines within the rkisp1 driver to use the
guard() macros from cleanup.h. This simplifies unlocking spin_locks
through the driver and will help ensure correct locking as the driver
is updated.
Signed-off-by: Daniel Scally <dan.scally at ideasonboard.com>
---
.../platform/rockchip/rkisp1/rkisp1-capture.c | 90 +++++++++++-----------
.../media/platform/rockchip/rkisp1/rkisp1-params.c | 31 +++-----
.../media/platform/rockchip/rkisp1/rkisp1-stats.c | 13 ++--
3 files changed, 62 insertions(+), 72 deletions(-)
diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
index 20d88865081c33a9bd6a4adc1036841505e2a87f..757fd35e248a89ff3a6af4e07491239e14dda9c4 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c
@@ -8,6 +8,7 @@
* Copyright (C) 2017 Rockchip Electronics Co., Ltd.
*/
+#include <linux/cleanup.h>
#include <linux/delay.h>
#include <linux/pm_runtime.h>
#include <media/v4l2-common.h>
@@ -796,7 +797,7 @@ static void rkisp1_handle_buffer(struct rkisp1_capture *cap)
struct rkisp1_isp *isp = &cap->rkisp1->isp;
struct rkisp1_buffer *curr_buf;
- spin_lock(&cap->buf.lock);
+ guard(spinlock)(&cap->buf.lock);
curr_buf = cap->buf.curr;
if (curr_buf) {
@@ -809,7 +810,6 @@ static void rkisp1_handle_buffer(struct rkisp1_capture *cap)
}
rkisp1_set_next_buf(cap);
- spin_unlock(&cap->buf.lock);
}
irqreturn_t rkisp1_capture_isr(int irq, void *ctx)
@@ -928,9 +928,9 @@ static void rkisp1_vb2_buf_queue(struct vb2_buffer *vb)
container_of(vbuf, struct rkisp1_buffer, vb);
struct rkisp1_capture *cap = vb->vb2_queue->drv_priv;
- spin_lock_irq(&cap->buf.lock);
- list_add_tail(&ispbuf->queue, &cap->buf.queue);
- spin_unlock_irq(&cap->buf.lock);
+ scoped_guard(spinlock_irq, &cap->buf.lock) {
+ list_add_tail(&ispbuf->queue, &cap->buf.queue);
+ }
}
static int rkisp1_vb2_buf_prepare(struct vb2_buffer *vb)
@@ -958,7 +958,8 @@ static void rkisp1_return_all_buffers(struct rkisp1_capture *cap,
{
struct rkisp1_buffer *buf;
- spin_lock_irq(&cap->buf.lock);
+ guard(spinlock_irq)(&cap->buf.lock);
+
if (cap->buf.curr) {
vb2_buffer_done(&cap->buf.curr->vb.vb2_buf, state);
cap->buf.curr = NULL;
@@ -973,7 +974,6 @@ static void rkisp1_return_all_buffers(struct rkisp1_capture *cap,
list_del(&buf->queue);
vb2_buffer_done(&buf->vb.vb2_buf, state);
}
- spin_unlock_irq(&cap->buf.lock);
}
/*
@@ -992,46 +992,48 @@ static void rkisp1_cap_stream_enable(struct rkisp1_capture *cap)
cap->ops->config(cap);
/* Setup a buffer for the next frame */
- spin_lock_irq(&cap->buf.lock);
- rkisp1_set_next_buf(cap);
- cap->ops->enable(cap);
-
- /*
- * It's safe to configure ACTIVE and SHADOW registers for the first
- * stream. While when the second is starting, do NOT force update
- * because it also updates the first one.
- *
- * The latter case would drop one more buffer(that is 2) since there's
- * no buffer in a shadow register when the second FE received. This's
- * also required because the second FE maybe corrupt especially when
- * run at 120fps.
- */
- if (!has_self_path || !other->is_streaming) {
- u32 reg;
+ scoped_guard(spinlock_irq, &cap->buf.lock) {
+ rkisp1_set_next_buf(cap);
+ cap->ops->enable(cap);
/*
- * Force cfg update.
- *
- * The ISP8000 (implementing the MAIN_STRIDE feature) as a
- * mp_output_format field in the CIF_MI_INIT register that must
- * be preserved. It can be read back, but it is not clear what
- * other register bits will return. Mask them out.
- *
- * On Rockchip platforms, the CIF_MI_INIT register is marked as
- * write-only and reads as zeros. We can skip reading it.
- */
- if (rkisp1_has_feature(rkisp1, MAIN_STRIDE))
- reg = rkisp1_read(rkisp1, RKISP1_CIF_MI_INIT)
- & RKISP1_CIF_MI_INIT_MP_OUTPUT_MASK;
- else
- reg = 0;
-
- reg |= RKISP1_CIF_MI_INIT_SOFT_UPD;
- rkisp1_write(rkisp1, RKISP1_CIF_MI_INIT, reg);
-
- rkisp1_set_next_buf(cap);
+ * It's safe to configure ACTIVE and SHADOW registers for the
+ * first stream. While when the second is starting, do NOT force
+ * update because it also updates the first one.
+ *
+ * The latter case would drop one more buffer(that is 2) since
+ * there's no buffer in a shadow register when the second FE
+ * received. This's also required because the second FE maybe
+ * corrupt especially when run at 120fps.
+ */
+ if (!has_self_path || !other->is_streaming) {
+ u32 reg;
+
+ /*
+ * Force cfg update.
+ *
+ * The ISP8000 (implementing the MAIN_STRIDE feature) as
+ * a mp_output_format field in the CIF_MI_INIT register
+ * that must be preserved. It can be read back, but it is
+ * not clear what other register bits will return. Mask
+ * them out.
+ *
+ * On Rockchip platforms, the CIF_MI_INIT register is
+ * marked as write-only and reads as zeros. We can skip
+ * reading it.
+ */
+ if (rkisp1_has_feature(rkisp1, MAIN_STRIDE))
+ reg = rkisp1_read(rkisp1, RKISP1_CIF_MI_INIT)
+ & RKISP1_CIF_MI_INIT_MP_OUTPUT_MASK;
+ else
+ reg = 0;
+
+ reg |= RKISP1_CIF_MI_INIT_SOFT_UPD;
+ rkisp1_write(rkisp1, RKISP1_CIF_MI_INIT, reg);
+
+ rkisp1_set_next_buf(cap);
+ }
}
- spin_unlock_irq(&cap->buf.lock);
cap->is_streaming = true;
}
diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c
index e5913f8b73726d16362e429960e251b48b62d9aa..cc42b5afbf65a9a992323c40cff1a6fe2d8b61e4 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-params.c
@@ -7,6 +7,7 @@
#include <linux/bitfield.h>
#include <linux/build_bug.h>
+#include <linux/cleanup.h>
#include <linux/math.h>
#include <linux/string.h>
@@ -2339,13 +2340,13 @@ void rkisp1_params_isr(struct rkisp1_device *rkisp1)
{
struct rkisp1_params *params = &rkisp1->params;
- spin_lock(¶ms->buf.lock);
+ guard(spinlock)(¶ms->buf.lock);
params->buf.curr = list_first_entry_or_null(¶ms->buf.queue,
struct rkisp1_params_buffer,
queue);
if (!params->buf.curr)
- goto unlock;
+ return;
if (params->metafmt->dataformat == V4L2_META_FMT_RK_ISP1_PARAMS) {
rkisp1_isp_isr_other_config(params, params->buf.curr->cfg);
@@ -2371,9 +2372,6 @@ void rkisp1_params_isr(struct rkisp1_device *rkisp1)
*/
rkisp1_params_complete_buffer(params, params->buf.curr,
rkisp1->isp.frame_sequence + 1);
-
-unlock:
- spin_unlock(¶ms->buf.lock);
}
static const struct rkisp1_cif_isp_awb_meas_config rkisp1_awb_params_default_config = {
@@ -2445,7 +2443,7 @@ void rkisp1_params_pre_configure(struct rkisp1_params *params,
rkisp1_csm_config(params);
- spin_lock_irq(¶ms->buf.lock);
+ guard(spinlock_irq)(¶ms->buf.lock);
/* apply the first buffer if there is one already */
@@ -2453,7 +2451,7 @@ void rkisp1_params_pre_configure(struct rkisp1_params *params,
struct rkisp1_params_buffer,
queue);
if (!params->buf.curr)
- goto unlock;
+ return;
if (params->metafmt->dataformat == V4L2_META_FMT_RK_ISP1_PARAMS) {
rkisp1_isp_isr_other_config(params, params->buf.curr->cfg);
@@ -2466,14 +2464,11 @@ void rkisp1_params_pre_configure(struct rkisp1_params *params,
/* update shadow register immediately */
rkisp1_param_set_bits(params, RKISP1_CIF_ISP_CTRL,
RKISP1_CIF_ISP_CTRL_ISP_CFG_UPD);
-
-unlock:
- spin_unlock_irq(¶ms->buf.lock);
}
void rkisp1_params_post_configure(struct rkisp1_params *params)
{
- spin_lock_irq(¶ms->buf.lock);
+ guard(spinlock_irq)(¶ms->buf.lock);
/*
* Apply LSC parameters from the first buffer (if any is already
@@ -2487,7 +2482,7 @@ void rkisp1_params_post_configure(struct rkisp1_params *params)
struct rkisp1_params_buffer,
queue);
if (!params->buf.curr)
- goto unlock;
+ return;
if (params->metafmt->dataformat == V4L2_META_FMT_RK_ISP1_PARAMS)
rkisp1_isp_isr_lsc_config(params, params->buf.curr->cfg);
@@ -2500,9 +2495,6 @@ void rkisp1_params_post_configure(struct rkisp1_params *params)
RKISP1_CIF_ISP_CTRL_ISP_CFG_UPD);
rkisp1_params_complete_buffer(params, params->buf.curr, 0);
-
-unlock:
- spin_unlock_irq(¶ms->buf.lock);
}
/*
@@ -2710,9 +2702,8 @@ static void rkisp1_params_vb2_buf_queue(struct vb2_buffer *vb)
struct vb2_queue *vq = vb->vb2_queue;
struct rkisp1_params *params = vq->drv_priv;
- spin_lock_irq(¶ms->buf.lock);
+ guard(spinlock_irq)(¶ms->buf.lock);
list_add_tail(¶ms_buf->queue, ¶ms->buf.queue);
- spin_unlock_irq(¶ms->buf.lock);
}
static int rkisp1_params_prepare_ext_params(struct rkisp1_params *params,
@@ -2781,9 +2772,9 @@ static void rkisp1_params_vb2_stop_streaming(struct vb2_queue *vq)
* and then we can iterate it and call vb2_buffer_done
* without holding the lock
*/
- spin_lock_irq(¶ms->buf.lock);
- list_splice_init(¶ms->buf.queue, &tmp_list);
- spin_unlock_irq(¶ms->buf.lock);
+ scoped_guard(spinlock_irq, ¶ms->buf.lock) {
+ list_splice_init(¶ms->buf.queue, &tmp_list);
+ }
list_for_each_entry(buf, &tmp_list, queue)
vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_ERROR);
diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-stats.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-stats.c
index d5fdb8f82dc78b0143f71d76f36817db389921b7..fba95a1786be0d6782829a5604d41893dda8e826 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-stats.c
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-stats.c
@@ -5,6 +5,8 @@
* Copyright (C) 2017 Rockchip Electronics Co., Ltd.
*/
+#include <linux/cleanup.h>
+
#include <media/v4l2-common.h>
#include <media/v4l2-event.h>
#include <media/v4l2-ioctl.h>
@@ -112,10 +114,8 @@ static void rkisp1_stats_vb2_buf_queue(struct vb2_buffer *vb)
struct vb2_queue *vq = vb->vb2_queue;
struct rkisp1_stats *stats_dev = vq->drv_priv;
-
- spin_lock_irq(&stats_dev->lock);
+ guard(spinlock_irq)(&stats_dev->lock);
list_add_tail(&stats_buf->queue, &stats_dev->stat);
- spin_unlock_irq(&stats_dev->lock);
}
static int rkisp1_stats_vb2_buf_prepare(struct vb2_buffer *vb)
@@ -134,7 +134,7 @@ static void rkisp1_stats_vb2_stop_streaming(struct vb2_queue *vq)
struct rkisp1_buffer *buf;
unsigned int i;
- spin_lock_irq(&stats->lock);
+ guard(spinlock_irq)(&stats->lock);
for (i = 0; i < RKISP1_ISP_STATS_REQ_BUFS_MAX; i++) {
if (list_empty(&stats->stat))
break;
@@ -143,7 +143,6 @@ static void rkisp1_stats_vb2_stop_streaming(struct vb2_queue *vq)
list_del(&buf->queue);
vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_ERROR);
}
- spin_unlock_irq(&stats->lock);
}
static const struct vb2_ops rkisp1_stats_vb2_ops = {
@@ -381,7 +380,7 @@ void rkisp1_stats_isr(struct rkisp1_stats *stats, u32 isp_ris)
struct rkisp1_device *rkisp1 = stats->rkisp1;
unsigned int isp_mis_tmp = 0;
- spin_lock(&stats->lock);
+ guard(spinlock)(&stats->lock);
rkisp1_write(rkisp1, RKISP1_CIF_ISP_ICR, RKISP1_STATS_MEAS_MASK);
@@ -391,8 +390,6 @@ void rkisp1_stats_isr(struct rkisp1_stats *stats, u32 isp_ris)
if (isp_ris & RKISP1_STATS_MEAS_MASK)
rkisp1_stats_send_measurement(stats, isp_ris);
-
- spin_unlock(&stats->lock);
}
static void rkisp1_init_stats(struct rkisp1_stats *stats)
--
2.43.0
More information about the linux-arm-kernel
mailing list