atomisp: add spinlock to solve race condition

BZ: 34525

issue: kernel panicking and system hang when doing MTBF
root cause:
	In failed case, atomisp_isr() is running on CPU0,
	atomisp_acc_load() is on another CPU1 (hyper-thread / dual-core).
	Atomisp_acc_load() sets  my_css.invalidate  between
	sh_css_preview_next_stage_needs_alloc(), and atomisp_start_binary().
	It causes kernel panicing when CSS API sh_css_preview_start()
	tries to free memory. Because the memory (de)allocation
	might trigger a sleep operation and it's in the ISR of ISP.
solution:
	added a spinlock to protect the CSS flag my_css.invalidate in
	sh_css_load_acceleration(), meanwhile the memory allocation for
        header->sp_arg in sh_css_load_acc() should be moved out of the
	path spinlock protection. Moved it into the function
	sh_css_acc_set_argument() which is the first place to use
	the variable header->sp_arg.

	For digital zoom/xnr, also add spinlock/mutex to protect
	the CSS flag my_css.invalidate.

Change-Id: Icec8b3d2e1b513290fcbabcd19210710a962ecb8
Signed-off-by: Jianxu Zheng <jian.xu.zheng@intel.com>
Reviewed-on: http://android.intel.com:8080/53298
Reviewed-by: Augusteijn, Lex <lex.augusteijn@intel.com>
Reviewed-by: Govindapillai, Vinod <vinod.govindapillai@intel.com>
Reviewed-by: Kruger, Jozef <jozef.kruger@intel.com>
Reviewed-by: Tuominen, TeemuX <teemux.tuominen@intel.com>
Reviewed-by: Toivonen, Tuukka <tuukka.toivonen@intel.com>
Reviewed-by: Laakso, Antti <antti.laakso@intel.com>
Reviewed-by: Koski, Anttu <anttu.koski@intel.com>
Tested-by: Koski, Anttu <anttu.koski@intel.com>
Reviewed-by: buildbot <buildbot@intel.com>
Tested-by: buildbot <buildbot@intel.com>

diff --git a/drivers/media/video/atomisp/atomisp_cmd.c b/drivers/media/video/atomisp/atomisp_cmd.c
index d339019..f1718d0 100644 (file)
--- a/drivers/media/video/atomisp/atomisp_cmd.c
+++ b/drivers/media/video/atomisp/atomisp_cmd.c
@@ -1394,13 +1394,18 @@ int atomisp_low_light(struct atomisp_device *isp, int flag, __s32 * value)
 int atomisp_xnr(struct atomisp_device *isp, int flag, int *arg)
 {
        int xnr_enable = (*arg == 0) ? 0 : 1;
+       unsigned long irq_flag;
 
        if (flag == 0) {
                *arg = isp->params.xnr_en;
                return 0;
        }
 
+       mutex_lock(&isp->isp_lock);
+       spin_lock_irqsave(&isp->irq_lock, irq_flag);
        sh_css_capture_enable_xnr(xnr_enable);
+       spin_unlock_irqrestore(&isp->irq_lock, irq_flag);
+       mutex_unlock(&isp->isp_lock);
 
        return 0;
 }
@@ -2686,6 +2691,7 @@ int atomisp_shading_correction(struct atomisp_device *isp, int flag,
 int atomisp_digital_zoom(struct atomisp_device *isp, int flag, __s32 *value)
 {
        int zoom, zoom_max;
+       unsigned long irq_flag;
 
        if (isp->params.yuv_us_en && isp->main_format) {
                /* If upscaling is enabled, we have to ensure minimum zoom */
@@ -2725,7 +2731,11 @@ int atomisp_digital_zoom(struct atomisp_device *isp, int flag, __s32 *value)
                /* Scale control value in reverse between CSS allowed range */
                zoom = 64 - zoom;
                zoom = zoom * zoom_max / 64;
+               mutex_lock(&isp->isp_lock);
+               spin_lock_irqsave(&isp->irq_lock, irq_flag);
                sh_css_set_zoom_factor(zoom, zoom);
+               spin_unlock_irqrestore(&isp->irq_lock, irq_flag);
+               mutex_unlock(&isp->isp_lock);
        }
 
        return 0;
@@ -3780,6 +3790,7 @@ int atomisp_acc_load(struct atomisp_device *isp,
 {
        struct sh_css_acc_fw *fw;
        int ret;
+       unsigned long irq_flag;
 
        mutex_lock(&isp->input_lock);
        mutex_lock(&isp->isp_lock);
@@ -3791,7 +3802,9 @@ int atomisp_acc_load(struct atomisp_device *isp,
                goto out;
        }
 
+       spin_lock_irqsave(&isp->irq_lock, irq_flag);
        ret = sh_css_load_acceleration(fw);
+       spin_unlock_irqrestore(&isp->irq_lock, irq_flag);
        if (ret) {
                __acc_fw_free(isp, fw);
                v4l2_err(&atomisp_dev, "%s: Failed to load acceleration "

diff --git a/drivers/media/video/atomisp/css/sh_css_accelerate.c b/drivers/media/video/atomisp/css/sh_css_accelerate.c
index 831c782..e3d3a02 100644 (file)
--- a/drivers/media/video/atomisp/css/sh_css_accelerate.c
+++ b/drivers/media/video/atomisp/css/sh_css_accelerate.c
@@ -74,14 +74,14 @@ upload_int(unsigned *sp_address, unsigned *val)
 enum sh_css_err
 sh_css_acc_load(const struct sh_css_acc_fw *firmware)
 {
+       /*
+        * moving memory alloc out of spinlock criteria.
+        * placing it in sh_css_acc_set_argument.
+        */
        struct sh_css_acc_fw_hdr *header
-               = (struct sh_css_acc_fw_hdr *)&firmware->header;
-       header->sp_args =
-               sh_css_malloc(sizeof(*header->sp_args) *
-                             header->sp.args_cnt);
-       if (!header->sp_args)
-               return sh_css_err_cannot_allocate_memory;
-       header->loaded = true;
+           = (struct sh_css_acc_fw_hdr *)&firmware->header;
+       if (header->sp.args_cnt == 0)
+               header->loaded = true;
        return sh_css_success;
 }
 
@@ -101,11 +101,33 @@ sh_css_acc_unload(const struct sh_css_acc_fw *firmware)
        header->loaded   = false;
 }
 
+static enum sh_css_err
+mem_allocation_for_sp_args(struct sh_css_acc_fw *firmware)
+{
+       struct sh_css_acc_fw_hdr *header
+           = (struct sh_css_acc_fw_hdr *)&firmware->header;
+       if (!header->loaded) {
+               if (!header->sp_args)
+                       header->sp_args =
+                           sh_css_malloc(sizeof(*header->sp_args) *
+                                         header->sp.args_cnt);
+               if (!header->sp_args)
+                       return sh_css_err_cannot_allocate_memory;
+               header->loaded = true;
+       }
+       return sh_css_success;
+}
 /* Set argument <num> of size <size> to value <val> */
 enum sh_css_err
 sh_css_acc_set_argument(struct sh_css_acc_fw *firmware,
                        unsigned num, void *val, size_t size)
 {
+       enum sh_css_err ret;
+       ret = mem_allocation_for_sp_args(firmware);
+
+       if (ret != sh_css_success)
+               return ret;
+
        if (num >= firmware->header.sp.args_cnt)
                return sh_css_err_invalid_arguments;
 
@@ -247,6 +269,7 @@ sh_css_acc_start(struct sh_css_acc_fw *firmware,
        bool is_extension = (header->type != SH_CSS_ACC_STANDALONE);
        const struct sh_css_sp_fw *sp_fw = &header->sp.fw;
        const unsigned char *sp_program;
+       enum sh_css_err ret;
 #if !defined(C_RUN) && !defined(HRT_UNSCHED)
        const unsigned char *isp_program;
 #endif
@@ -254,8 +277,11 @@ sh_css_acc_start(struct sh_css_acc_fw *firmware,
        *(const void **)&sp_fw->text = SH_CSS_ACC_SP_CODE(firmware);
        *(const void **)&sp_fw->data = SH_CSS_ACC_SP_DATA(firmware);
 
-       if (!header->loaded)
-               return sh_css_err_invalid_arguments;
+       ret = mem_allocation_for_sp_args(firmware);
+
+       if (ret != sh_css_success)
+               return ret;
+
        if (has_extension_args != is_extension)
                return sh_css_err_invalid_arguments;