[COMMON] media: scaler: wait device stop running locklessly

Since "fec3dab [8895] media: scaler: add mutex lock" patch has been
applied, the following problem is reported:

BUG: sleeping function called from invalid context at kernel/locking/mutex.c:617
in_atomic(): 1, irqs_disabled(): 128, pid: 0, name: swapper/0
CPU: 0 MPIDR: 80000100 PID: 0 Comm: swapper/0 Tainted: G        W       4.4.13-10228462-eng #1
Hardware name: Samsung SM-G955F rev00 board based on EXYNOS8895 (DT)
Call trace:
[<ffffffc00008a254>] dump_backtrace+0x0/0x110
[<ffffffc00008a378>] show_stack+0x14/0x20
[<ffffffc0003892b4>] dump_stack+0xa8/0xe4
[<ffffffc0000d5220>] ___might_sleep+0x244/0x258
[<ffffffc0000d52ac>] __might_sleep+0x78/0x8c
[<ffffffc000c240ec>] mutex_lock_nested+0x3c/0x3bc
[<ffffffc00075490c>] sc_m2m1shot_device_run+0x30/0xd4
[<ffffffc000746b74>] m2m1shot_task_schedule+0x90/0xd8
[<ffffffc00074735c>] m2m1shot_task_finish+0x78/0x90
[<ffffffc000752338>] sc_irq_handler+0x4a4/0x4f4
[<ffffffc00010b84c>] handle_irq_event_percpu+0x15c/0x418
[<ffffffc00010bb4c>] handle_irq_event+0x44/0x78
[<ffffffc00010ef04>] handle_fasteoi_irq+0xc4/0x138
[<ffffffc00010af80>] generic_handle_irq+0x1c/0x30
[<ffffffc00010b060>] __handle_domain_irq+0xcc/0x11c
[<ffffffc0000814f8>] gic_handle_irq+0x70/0xcc

It is caused by the mutex lock introduced by the above patch and the
mutex is required for modifying DEV_SUSPEND and testing DEV_RUN
atomically.

However, the problem that a new task starts running between testing
DEV_SUSPEND and testing DEV_RUN in sc_suspend() and sc_shutdown() is
basically caused by the sc_irq_handler() that clears DEV_RUN before
testing DEV_SUSPEND. Therefore, the problem of running an unexpected
task is simply solved by moving clearing DEV_RUN after testing
DEV_SUSPEND.

Change-Id: I7deed59fc636868d062a63ba8c5b2bd5b9f3c753
Signed-off-by: Cho KyongHo <pullip.cho@samsung.com>

diff --git a/drivers/media/platform/exynos/scaler/scaler-core.c b/drivers/media/platform/exynos/scaler/scaler-core.c
index a15cc81ad745f1de73db073ecb5136ec63bef332..0a2fb06959ea95ff977a2b418df63cea81e2404f 100644 (file)
--- a/drivers/media/platform/exynos/scaler/scaler-core.c
+++ b/drivers/media/platform/exynos/scaler/scaler-core.c
@@ -2468,37 +2468,46 @@ static int sc_run_next_job(struct sc_dev *sc)
        unsigned int h_shift, v_shift;
        int ret;
 
+       ret = sc_power_clk_enable(sc);
+       if (ret)
+               return ret;
+
        spin_lock_irqsave(&sc->ctxlist_lock, flags);
 
        if (sc->current_ctx || list_empty(&sc->context_list)) {
                /* a job is currently being processed or no job is to run */
+               sc_clk_power_disable(sc);
+               spin_unlock_irqrestore(&sc->ctxlist_lock, flags);
+               return 0;
+       }
+
+       /*
+        * sc_run_next_job() must not reenter while sc->state is DEV_RUN.
+        * DEV_RUN is cleared when an operation is finished.
+        */
+       BUG_ON(test_bit(DEV_RUN, &sc->state));
+
+       set_bit(DEV_RUN, &sc->state);
+
+       if (test_bit(DEV_SUSPEND, &sc->state)) {
+               clear_bit(DEV_RUN, &sc->state);
+               sc_clk_power_disable(sc);
                spin_unlock_irqrestore(&sc->ctxlist_lock, flags);
                return 0;
        }
 
        ctx = list_first_entry(&sc->context_list, struct sc_ctx, node);
 
+       set_bit(CTX_RUN, &ctx->flags);
        list_del_init(&ctx->node);
 
        sc->current_ctx = ctx;
 
        spin_unlock_irqrestore(&sc->ctxlist_lock, flags);
 
-       /*
-        * sc_run_next_job() must not reenter while sc->state is DEV_RUN.
-        * DEV_RUN is cleared when an operation is finished.
-        */
-       BUG_ON(test_bit(DEV_RUN, &sc->state));
-
        s_frame = &ctx->s_frame;
        d_frame = &ctx->d_frame;
 
-       ret = sc_power_clk_enable(sc);
-       if (ret) {
-               pm_runtime_put(sc->dev);
-               return ret;
-       }
-
        sc_hwset_init(sc);
 
        if (ctx->i_frame) {
@@ -2588,9 +2597,6 @@ static int sc_run_next_job(struct sc_dev *sc)
 
        sc_hwset_int_en(sc);
 
-       set_bit(DEV_RUN, &sc->state);
-       set_bit(CTX_RUN, &ctx->flags);
-
        sc_set_prefetch_buffers(sc->dev, ctx);
 
        mod_timer(&sc->wdt.timer, jiffies + SC_TIMEOUT);
@@ -2625,6 +2631,11 @@ static int sc_add_context_and_run(struct sc_dev *sc, struct sc_ctx *ctx)
 {
        unsigned long flags;
 
+       if (test_bit(CTX_ABORT, &ctx->flags)) {
+               dev_err(sc->dev, "aborted scaler device run\n");
+               return -EAGAIN;
+       }
+
        spin_lock_irqsave(&sc->ctxlist_lock, flags);
        list_add_tail(&ctx->node, &sc->context_list);
        spin_unlock_irqrestore(&sc->ctxlist_lock, flags);
@@ -2641,8 +2652,6 @@ static irqreturn_t sc_irq_handler(int irq, void *priv)
 
        spin_lock(&sc->slock);
 
-       clear_bit(DEV_RUN, &sc->state);
-
        /*
         * ok to access sc->current_ctx withot ctxlist_lock held
         * because it is not modified until sc_run_next_job() is called.
@@ -2667,10 +2676,11 @@ static irqreturn_t sc_irq_handler(int irq, void *priv)
        if (!SCALER_INT_OK(irq_status))
                sc_hwset_soft_reset(sc);
 
-       sc_clk_power_disable(sc);
-
+       clear_bit(DEV_RUN, &sc->state);
        clear_bit(CTX_RUN, &ctx->flags);
 
+       sc_clk_power_disable(sc);
+
        if (ctx->context_type == SC_CTX_V4L2_TYPE) {
                BUG_ON(ctx != v4l2_m2m_get_curr_priv(sc->m2m.m2m_dev));
 
@@ -2696,16 +2706,10 @@ static irqreturn_t sc_irq_handler(int irq, void *priv)
                        SCALER_INT_OK(irq_status) ?
                                VB2_BUF_STATE_DONE : VB2_BUF_STATE_ERROR);
 
-               if (test_bit(DEV_SUSPEND, &sc->state)) {
-                       sc_dbg("wake up blocked process by suspend\n");
-                       wake_up(&sc->wait);
-               } else {
-                       v4l2_m2m_job_finish(sc->m2m.m2m_dev, ctx->m2m_ctx);
-               }
+               v4l2_m2m_job_finish(sc->m2m.m2m_dev, ctx->m2m_ctx);
 
                /* Wake up from CTX_ABORT state */
-               if (test_and_clear_bit(CTX_ABORT, &ctx->flags))
-                       wake_up(&sc->wait);
+               clear_bit(CTX_ABORT, &ctx->flags);
        } else {
                struct m2m1shot_task *task =
                                        m2m1shot_get_current_task(sc->m21dev);
@@ -2725,6 +2729,8 @@ static irqreturn_t sc_irq_handler(int irq, void *priv)
        sc->current_ctx = NULL;
        spin_unlock(&sc->ctxlist_lock);
 
+       wake_up(&sc->wait);
+
        sc_run_next_job(sc);
 
 isr_unlock:
@@ -2847,16 +2853,6 @@ static void sc_m2m_device_run(void *priv)
        struct sc_dev *sc = ctx->sc_dev;
        struct sc_frame *s_frame, *d_frame;
 
-       if (test_bit(DEV_SUSPEND, &sc->state)) {
-               dev_err(sc->dev, "Scaler is in suspend state\n");
-               return;
-       }
-
-       if (test_bit(CTX_ABORT, &ctx->flags)) {
-               dev_err(sc->dev, "aborted scaler device run\n");
-               return;
-       }
-
        s_frame = &ctx->s_frame;
        d_frame = &ctx->d_frame;
 
@@ -3277,13 +3273,6 @@ static int sc_m2m1shot_device_run(struct m2m1shot_context *m21ctx,
        struct sc_dev *sc = ctx->sc_dev;
        struct sc_frame *s_frame, *d_frame;
 
-       if (test_bit(DEV_SUSPEND, &sc->state)) {
-               dev_err(sc->dev, "Scaler is in suspend state\n");
-               return -EAGAIN;
-       }
-
-       /* no aborted state is required for m2m1shot */
-
        s_frame = &ctx->s_frame;
        d_frame = &ctx->d_frame;
 
@@ -3424,6 +3413,8 @@ static int sc_resume(struct device *dev)
 
        clear_bit(DEV_SUSPEND, &sc->state);
 
+       sc_run_next_job(sc);
+
        return 0;
 }
 #endif
@@ -3650,11 +3641,8 @@ static int sc_remove(struct platform_device *pdev)
 static void sc_shutdown(struct platform_device *pdev)
 {
        struct sc_dev *sc = platform_get_drvdata(pdev);
-       unsigned long flags;
 
-       spin_lock_irqsave(&sc->slock, flags);
        set_bit(DEV_SUSPEND, &sc->state);
-       spin_unlock_irqrestore(&sc->slock, flags);
 
        wait_event(sc->wait,
                        !test_bit(DEV_RUN, &sc->state));