This is an automated email from the ASF dual-hosted git repository.

acassis pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/nuttx.git

commit c2efa960020f1a6eb80855ae50f202f9a8a0bdae
Author: yaojingwei <[email protected]>
AuthorDate: Tue Jun 10 14:13:58 2025 +0800

    drivers/video: optimize critical section.
    
    use spin/mutex lock to replace critial section.
    
    Signed-off-by: yaojingwei <[email protected]>
---
 drivers/video/v4l2_cap.c | 137 +++++++++++++++++++++++++++++------------------
 drivers/video/v4l2_m2m.c |  32 +++++------
 2 files changed, 98 insertions(+), 71 deletions(-)

diff --git a/drivers/video/v4l2_cap.c b/drivers/video/v4l2_cap.c
index 39f48a0a154..0c54dbe219a 100644
--- a/drivers/video/v4l2_cap.c
+++ b/drivers/video/v4l2_cap.c
@@ -31,7 +31,9 @@
 #include <errno.h>
 #include <poll.h>
 
+#include <nuttx/kmalloc.h>
 #include <nuttx/mutex.h>
+#include <nuttx/spinlock.h>
 #include <nuttx/video/v4l2_cap.h>
 #include <nuttx/video/video.h>
 
@@ -172,7 +174,6 @@ struct capture_mng_s
 
   /* Parameter of capture_initialize() */
 
-  mutex_t                lock_open_num;
   uint8_t                open_num;
   capture_type_inf_t     capture_inf;
   capture_type_inf_t     still_inf;
@@ -181,6 +182,8 @@ struct capture_mng_s
   enum v4l2_scene_mode   capture_scene_mode;
   uint8_t                capture_scence_num;
   FAR capture_scene_params_t *capture_scene_param[V4L2_SCENE_MODE_MAX];
+  spinlock_t             lock;
+  mutex_t                mutex;
 #ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
   bool                   unlinked;
 #endif
@@ -782,9 +785,21 @@ static int start_capture(FAR struct capture_mng_s *cmng,
                          FAR struct v4l2_fract *interval,
                          uintptr_t bufaddr, uint32_t bufsize)
 {
-  video_format_t c_fmt[MAX_CAPTURE_FMT];
-  imgdata_format_t df[MAX_CAPTURE_FMT];
-  imgsensor_format_t sf[MAX_CAPTURE_FMT];
+  video_format_t c_fmt[MAX_CAPTURE_FMT] =
+    {
+      0
+    };
+
+  imgdata_format_t df[MAX_CAPTURE_FMT] =
+    {
+      0
+    };
+
+  imgsensor_format_t sf[MAX_CAPTURE_FMT] =
+    {
+      0
+    };
+
   imgdata_interval_t di;
   imgsensor_interval_t si;
 
@@ -1263,9 +1278,21 @@ static int validate_frame_setting(FAR capture_mng_t 
*cmng,
                                   FAR struct v4l2_rect *clip,
                                   FAR struct v4l2_fract *interval)
 {
-  video_format_t c_fmt[MAX_CAPTURE_FMT];
-  imgdata_format_t df[MAX_CAPTURE_FMT];
-  imgsensor_format_t sf[MAX_CAPTURE_FMT];
+  video_format_t c_fmt[MAX_CAPTURE_FMT] =
+    {
+      0
+    };
+
+  imgdata_format_t df[MAX_CAPTURE_FMT] =
+    {
+      0
+    };
+
+  imgsensor_format_t sf[MAX_CAPTURE_FMT] =
+    {
+      0
+    };
+
   imgdata_interval_t di;
   imgsensor_interval_t si;
   int ret;
@@ -1947,10 +1974,17 @@ static int complete_capture(uint8_t err_code,
   FAR vbuf_container_t *container = NULL;
   enum v4l2_buf_type buf_type;
   irqstate_t           flags;
-  imgdata_format_t df[MAX_CAPTURE_FMT];
-  video_format_t c_fmt[MAX_CAPTURE_FMT];
+  imgdata_format_t df[MAX_CAPTURE_FMT] =
+    {
+      0
+    };
+
+  video_format_t c_fmt[MAX_CAPTURE_FMT] =
+    {
+      0
+    };
 
-  flags = enter_critical_section();
+  flags = spin_lock_irqsave_nopreempt(&cmng->lock);
 
   buf_type = cmng->still_inf.state == CAPTURE_STATE_CAPTURE ?
                V4L2_BUF_TYPE_STILL_CAPTURE : V4L2_BUF_TYPE_VIDEO_CAPTURE;
@@ -1958,7 +1992,7 @@ static int complete_capture(uint8_t err_code,
   type_inf = get_capture_type_inf(cmng, buf_type);
   if (type_inf == NULL)
     {
-      leave_critical_section(flags);
+      spin_unlock_irqrestore_nopreempt(&cmng->lock, flags);
       return -EINVAL;
     }
 
@@ -2041,7 +2075,7 @@ static int complete_capture(uint8_t err_code,
         }
     }
 
-  leave_critical_section(flags);
+  spin_unlock_irqrestore_nopreempt(&cmng->lock, flags);
   return OK;
 }
 
@@ -2139,7 +2173,6 @@ static int capture_reqbufs(FAR struct file *filep,
   FAR capture_mng_t *cmng = inode->i_private;
   FAR capture_type_inf_t *type_inf;
   struct imgdata_s *imgdata;
-  irqstate_t flags;
   int ret = OK;
 
   if (cmng == NULL || reqbufs == NULL)
@@ -2154,7 +2187,7 @@ static int capture_reqbufs(FAR struct file *filep,
       return -EINVAL;
     }
 
-  flags = enter_critical_section();
+  nxmutex_lock(&cmng->mutex);
 
   if (type_inf->state == CAPTURE_STATE_CAPTURE)
     {
@@ -2205,7 +2238,7 @@ static int capture_reqbufs(FAR struct file *filep,
         }
     }
 
-  leave_critical_section(flags);
+  nxmutex_unlock(&cmng->mutex);
   return ret;
 }
 
@@ -2246,7 +2279,6 @@ static int capture_qbuf(FAR struct file *filep,
   FAR capture_type_inf_t *type_inf;
   FAR vbuf_container_t *container;
   enum capture_state_e next_capture_state;
-  irqstate_t flags;
 
   if (cmng == NULL || buf == NULL)
     {
@@ -2283,11 +2315,8 @@ static int capture_qbuf(FAR struct file *filep,
   video_framebuff_queue_container(&type_inf->bufinf, container);
 
   nxmutex_lock(&type_inf->lock_state);
-  flags = enter_critical_section();
   if (type_inf->state == CAPTURE_STATE_STREAMON)
     {
-      leave_critical_section(flags);
-
       if (buf->type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
         {
           nxmutex_lock(&cmng->still_inf.lock_state);
@@ -2315,10 +2344,6 @@ static int capture_qbuf(FAR struct file *filep,
             }
         }
     }
-  else
-    {
-      leave_critical_section(flags);
-    }
 
   nxmutex_unlock(&type_inf->lock_state);
   return OK;
@@ -2333,7 +2358,6 @@ static int capture_dqbuf(FAR struct file *filep,
   FAR vbuf_container_t *container;
   FAR sem_t *dqbuf_wait_flg;
   enum capture_state_e next_capture_state;
-  irqstate_t flags;
 
   if (cmng == NULL || buf == NULL)
     {
@@ -2366,11 +2390,11 @@ static int capture_dqbuf(FAR struct file *filep,
             {
               /* If start capture condition is satisfied, start capture */
 
-              flags = enter_critical_section();
+              nxmutex_lock(&cmng->mutex);
               next_capture_state =
                 estimate_next_capture_state(cmng, CAUSE_CAPTURE_DQBUF);
               change_capture_state(cmng, next_capture_state);
-              leave_critical_section(flags);
+              nxmutex_unlock(&cmng->mutex);
             }
 
           nxsem_wait_uninterruptible(dqbuf_wait_flg);
@@ -2523,7 +2547,11 @@ static int capture_try_fmt(FAR struct file *filep,
   FAR struct inode *inode = filep->f_inode;
   FAR capture_mng_t *cmng = inode->i_private;
   FAR capture_type_inf_t *type_inf;
-  video_format_t vf[MAX_CAPTURE_FMT];
+  video_format_t vf[MAX_CAPTURE_FMT] =
+    {
+      0
+    };
+
   uint8_t nr_fmt;
 
   if (cmng == NULL || fmt == NULL)
@@ -2727,7 +2755,6 @@ static int capture_streamoff(FAR struct file *filep,
   FAR capture_mng_t *cmng = inode->i_private;
   FAR capture_type_inf_t *type_inf;
   enum capture_state_e next_capture_state;
-  irqstate_t flags;
   int ret = OK;
 
   if (cmng == NULL || type == NULL)
@@ -2748,7 +2775,7 @@ static int capture_streamoff(FAR struct file *filep,
       return OK;
     }
 
-  flags = enter_critical_section();
+  nxmutex_lock(&cmng->mutex);
 
   if (type_inf->state == CAPTURE_STATE_STREAMOFF)
     {
@@ -2761,7 +2788,7 @@ static int capture_streamoff(FAR struct file *filep,
       change_capture_state(cmng, next_capture_state);
     }
 
-  leave_critical_section(flags);
+  nxmutex_unlock(&cmng->mutex);
 
   return ret;
 }
@@ -2795,7 +2822,6 @@ static int capture_takepict_start(FAR struct file *filep,
   FAR capture_mng_t *cmng = inode->i_private;
   enum capture_state_e next_capture_state;
   FAR vbuf_container_t *container;
-  irqstate_t flags;
   int ret = OK;
 
   if (cmng == NULL)
@@ -2822,13 +2848,13 @@ static int capture_takepict_start(FAR struct file 
*filep,
 
       /* Control video stream prior to still stream */
 
-      flags = enter_critical_section();
+      nxmutex_lock(&cmng->mutex);
 
       next_capture_state = estimate_next_capture_state(cmng,
                                                    CAUSE_STILL_START);
       change_capture_state(cmng, next_capture_state);
 
-      leave_critical_section(flags);
+      nxmutex_unlock(&cmng->mutex);
 
       container =
         video_framebuff_get_vacant_container(&cmng->still_inf.bufinf);
@@ -2863,7 +2889,6 @@ static int capture_takepict_stop(FAR struct file *filep,
   FAR struct inode *inode = filep->f_inode;
   FAR capture_mng_t *cmng = inode->i_private;
   enum capture_state_e next_capture_state;
-  irqstate_t flags;
   int ret = OK;
 
   if (cmng == NULL)
@@ -2880,13 +2905,13 @@ static int capture_takepict_stop(FAR struct file *filep,
     }
   else
     {
-      flags = enter_critical_section();
+      nxmutex_lock(&cmng->mutex);
       if (cmng->still_inf.state == CAPTURE_STATE_CAPTURE)
         {
           stop_capture(cmng, V4L2_BUF_TYPE_STILL_CAPTURE);
         }
 
-      leave_critical_section(flags);
+      nxmutex_unlock(&cmng->mutex);
 
       cmng->still_inf.state = CAPTURE_STATE_STREAMOFF;
       cmng->still_inf.remaining_capnum = REMAINING_CAPNUM_INFINITY;
@@ -2910,7 +2935,11 @@ static int capture_s_selection(FAR struct file *filep,
   FAR struct inode *inode = filep->f_inode;
   FAR capture_mng_t *cmng = inode->i_private;
   FAR capture_type_inf_t *type_inf;
-  uint32_t p_u32[IMGSENSOR_CLIP_NELEM];
+  uint32_t p_u32[IMGSENSOR_CLIP_NELEM] =
+    {
+      0
+    };
+
   imgsensor_value_t val;
   uint32_t id;
   int ret;
@@ -3552,7 +3581,7 @@ static int capture_open(FAR struct file *filep)
       return -EINVAL;
     }
 
-  nxmutex_lock(&cmng->lock_open_num);
+  nxmutex_lock(&cmng->mutex);
   if (cmng->open_num == 0)
     {
       /* Only in first execution, open device */
@@ -3579,7 +3608,7 @@ static int capture_open(FAR struct file *filep)
       cmng->open_num++;
     }
 
-  nxmutex_unlock(&cmng->lock_open_num);
+  nxmutex_unlock(&cmng->mutex);
   return ret;
 }
 
@@ -3593,7 +3622,7 @@ static int capture_close(FAR struct file *filep)
       return -EINVAL;
     }
 
-  nxmutex_lock(&cmng->lock_open_num);
+  nxmutex_lock(&cmng->mutex);
 
   if (--cmng->open_num == 0)
     {
@@ -3603,8 +3632,8 @@ static int capture_close(FAR struct file *filep)
 #ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
       if (cmng->unlinked)
         {
-          nxmutex_unlock(&cmng->lock_open_num);
-          nxmutex_destroy(&cmng->lock_open_num);
+          nxmutex_unlock(&cmng->mutex);
+          nxmutex_destroy(&cmng->mutex);
           kmm_free(cmng);
           inode->i_private = NULL;
           return OK;
@@ -3613,7 +3642,7 @@ static int capture_close(FAR struct file *filep)
 #endif
     }
 
-  nxmutex_unlock(&cmng->lock_open_num);
+  nxmutex_unlock(&cmng->mutex);
   return OK;
 }
 
@@ -3667,7 +3696,7 @@ static int capture_poll(FAR struct file *filep,
       return -EINVAL;
     }
 
-  flags = enter_critical_section();
+  flags = spin_lock_irqsave_nopreempt(&cmng->lock);
 
   if (setup)
     {
@@ -3682,7 +3711,7 @@ static int capture_poll(FAR struct file *filep,
         }
       else
         {
-          leave_critical_section(flags);
+          spin_unlock_irqrestore_nopreempt(&cmng->lock, flags);
           return -EBUSY;
         }
     }
@@ -3692,7 +3721,7 @@ static int capture_poll(FAR struct file *filep,
       fds->priv     = NULL;
     }
 
-  leave_critical_section(flags);
+  spin_unlock_irqrestore_nopreempt(&cmng->lock, flags);
 
   return OK;
 }
@@ -3707,18 +3736,18 @@ static int capture_unlink(FAR struct inode *inode)
       return -EINVAL;
     }
 
-  nxmutex_lock(&cmng->lock_open_num);
+  nxmutex_lock(&cmng->mutex);
   if (cmng->open_num == 0)
     {
-      nxmutex_unlock(&cmng->lock_open_num);
-      nxmutex_destroy(&cmng->lock_open_num);
+      nxmutex_unlock(&cmng->mutex);
+      nxmutex_destroy(&cmng->mutex);
       kmm_free(cmng);
       inode->i_private = NULL;
     }
   else
     {
       cmng->unlinked = true;
-      nxmutex_unlock(&cmng->lock_open_num);
+      nxmutex_unlock(&cmng->mutex);
     }
 
   return OK;
@@ -3779,7 +3808,11 @@ int capture_register(FAR const char *devpath,
 
   /* Initialize mutex */
 
-  nxmutex_init(&cmng->lock_open_num);
+  nxmutex_init(&cmng->mutex);
+
+  /* Initialize spinlock */
+
+  spin_lock_init(&cmng->lock);
 
   /* Register the character driver */
 
@@ -3787,7 +3820,7 @@ int capture_register(FAR const char *devpath,
   if (ret < 0)
     {
       verr("Failed to register driver: %d\n", ret);
-      nxmutex_destroy(&cmng->lock_open_num);
+      nxmutex_destroy(&cmng->mutex);
       kmm_free(cmng);
       return ret;
     }
diff --git a/drivers/video/v4l2_m2m.c b/drivers/video/v4l2_m2m.c
index 1e1efa5e607..69aa3cbdb54 100644
--- a/drivers/video/v4l2_m2m.c
+++ b/drivers/video/v4l2_m2m.c
@@ -29,7 +29,9 @@
 #include <fcntl.h>
 #include <poll.h>
 
+#include <nuttx/kmalloc.h>
 #include <nuttx/sched.h>
+#include <nuttx/spinlock.h>
 #include <nuttx/video/v4l2_m2m.h>
 #include <nuttx/video/video.h>
 
@@ -75,6 +77,7 @@ struct codec_file_s
   sq_queue_t        event_avail;
   sq_queue_t        event_free;
   codec_event_t     event_pool[CODEC_EVENT_COUNT];
+  spinlock_t        lock;
   FAR struct pollfd *fds;
   FAR void          *priv;
 };
@@ -270,7 +273,6 @@ static int codec_reqbufs(FAR struct file *filep,
   FAR codec_mng_t *cmng = inode->i_private;
   FAR codec_file_t *cfile = filep->f_priv;
   FAR codec_type_inf_t *type_inf;
-  irqstate_t flags;
   uint32_t buf_size;
   int ret = OK;
 
@@ -299,8 +301,6 @@ static int codec_reqbufs(FAR struct file *filep,
       return -EINVAL;
     }
 
-  flags = enter_critical_section();
-
   type_inf = codec_get_type_inf(cfile, reqbufs->type);
   video_framebuff_change_mode(&type_inf->bufinf, reqbufs->mode);
   ret = video_framebuff_realloc_container(&type_inf->bufinf,
@@ -315,7 +315,6 @@ static int codec_reqbufs(FAR struct file *filep,
         }
     }
 
-  leave_critical_section(flags);
   return ret;
 }
 
@@ -432,7 +431,6 @@ static int codec_dqbuf(FAR struct file *filep,
   FAR codec_file_t *cfile = filep->f_priv;
   FAR codec_type_inf_t *type_inf;
   FAR vbuf_container_t *container;
-  irqstate_t flags;
 
   if (buf == NULL)
     {
@@ -445,18 +443,14 @@ static int codec_dqbuf(FAR struct file *filep,
       return -EINVAL;
     }
 
-  flags = enter_critical_section();
-
   if (video_framebuff_is_empty(&type_inf->bufinf))
     {
-      leave_critical_section(flags);
       return -EAGAIN;
     }
 
   container = video_framebuff_dq_valid_container(&type_inf->bufinf);
   if (container == NULL)
     {
-      leave_critical_section(flags);
       return -EAGAIN;
     }
 
@@ -466,7 +460,6 @@ static int codec_dqbuf(FAR struct file *filep,
   vinfo("%s dequeue done\n", V4L2_TYPE_IS_OUTPUT(buf->type) ?
                              "output" : "capture");
 
-  leave_critical_section(flags);
   return OK;
 }
 
@@ -748,11 +741,11 @@ int codec_dqevent(FAR struct file *filep,
       return -EINVAL;
     }
 
-  flags = enter_critical_section();
+  flags = spin_lock_irqsave(&cfile->lock);
 
   if (sq_empty(&cfile->event_avail))
     {
-      leave_critical_section(flags);
+      spin_unlock_irqrestore(&cfile->lock, flags);
       return -ENOENT;
     }
 
@@ -760,7 +753,7 @@ int codec_dqevent(FAR struct file *filep,
   memcpy(event, &cevt->event, sizeof(struct v4l2_event));
   sq_addlast((FAR sq_entry_t *)cevt, &cfile->event_free);
 
-  leave_critical_section(flags);
+  spin_unlock_irqrestore(&cfile->lock, flags);
   return OK;
 }
 
@@ -843,6 +836,7 @@ static int codec_open(FAR struct file *filep)
                  &cfile->event_free);
     }
 
+  spin_lock_init(&cfile->lock);
   video_framebuff_init(&cfile->capture_inf.bufinf);
   video_framebuff_init(&cfile->output_inf.bufinf);
 
@@ -925,7 +919,7 @@ static int codec_poll(FAR struct file *filep,
   pollevent_t eventset = 0;
   irqstate_t flags;
 
-  flags = enter_critical_section();
+  flags = spin_lock_irqsave_nopreempt(&cfile->lock);
 
   if (setup)
     {
@@ -957,7 +951,7 @@ static int codec_poll(FAR struct file *filep,
         }
       else
         {
-          leave_critical_section(flags);
+          spin_unlock_irqrestore_nopreempt(&cfile->lock, flags);
           return -EBUSY;
         }
     }
@@ -967,7 +961,7 @@ static int codec_poll(FAR struct file *filep,
       fds->priv  = NULL;
     }
 
-  leave_critical_section(flags);
+  spin_unlock_irqrestore_nopreempt(&cfile->lock, flags);
   return OK;
 }
 
@@ -1083,12 +1077,12 @@ int codec_queue_event(FAR void *cookie, FAR struct 
v4l2_event *evt)
   FAR codec_event_t *cevt;
   irqstate_t flags;
 
-  flags = enter_critical_section();
+  flags = spin_lock_irqsave_nopreempt(&cfile->lock);
 
   cevt = (FAR codec_event_t *)sq_remfirst(&cfile->event_free);
   if (cevt == NULL)
     {
-      leave_critical_section(flags);
+      spin_unlock_irqrestore_nopreempt(&cfile->lock, flags);
       return -EINVAL;
     }
 
@@ -1096,7 +1090,7 @@ int codec_queue_event(FAR void *cookie, FAR struct 
v4l2_event *evt)
   sq_addlast((FAR sq_entry_t *)cevt, &cfile->event_avail);
 
   poll_notify(&cfile->fds, 1, POLLPRI);
-  leave_critical_section(flags);
+  spin_unlock_irqrestore_nopreempt(&cfile->lock, flags);
 
   return OK;
 }

Reply via email to