From: Arun Mankuzhi <arun.m@xxxxxxxxxxx> In MFC's dynamic clock gating, we turn on the clock in try_run and turn off the clock upon receiving an interrupt. But this leads to excessive gating/ungating of the clocks in the case of multi-instance video playback. The clock gets disabled in ctx1's irq and then immediately turned on for ctx2's try_run. A better solution is to turn off the clocks only when there are no new frames to be processed in any context. This is done by conditionally clocking on/off calls in try-run. clock-off is done outside try-run only for suspend and error scenarios. Signed-off-by: Prathyush K <prathyush.k@xxxxxxxxxxx> Signed-off-by: Arun Mankuzhi <arun.m@xxxxxxxxxxx> Signed-off-by: Kiran AVND <avnd.kiran@xxxxxxxxxxx> --- drivers/media/platform/s5p-mfc/s5p_mfc.c | 26 ++++++++-------------- drivers/media/platform/s5p-mfc/s5p_mfc_common.h | 2 + drivers/media/platform/s5p-mfc/s5p_mfc_opr_v5.c | 11 ++++++++- drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c | 16 ++++++++++++- 4 files changed, 35 insertions(+), 20 deletions(-) diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c b/drivers/media/platform/s5p-mfc/s5p_mfc.c index e37fb99..9df130b 100644 --- a/drivers/media/platform/s5p-mfc/s5p_mfc.c +++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c @@ -150,7 +150,8 @@ static void s5p_mfc_watchdog_worker(struct work_struct *work) mfc_err("Error: some instance may be closing/opening\n"); spin_lock_irqsave(&dev->irqlock, flags); - s5p_mfc_clock_off(); + if (test_and_clear_bit(0, &dev->clk_flag)) + s5p_mfc_clock_off(); for (i = 0; i < MFC_NUM_CONTEXTS; i++) { ctx = dev->ctx[i]; @@ -174,7 +175,6 @@ static void s5p_mfc_watchdog_worker(struct work_struct *work) mfc_err("Failed to reload FW\n"); goto unlock; } - s5p_mfc_clock_on(); ret = s5p_mfc_init_hw(dev); if (ret) mfc_err("Failed to reinit FW\n"); @@ -338,7 +338,6 @@ static void s5p_mfc_handle_frame(struct s5p_mfc_ctx *ctx, wake_up_ctx(ctx, reason, err); if (test_and_clear_bit(0, &dev->hw_lock) == 0) BUG(); - s5p_mfc_clock_off(); s5p_mfc_hw_call(dev->mfc_ops, try_run, dev); return; } @@ -411,12 +410,11 @@ leave_handle_frame: wake_up_ctx(ctx, reason, err); if (test_and_clear_bit(0, &dev->hw_lock) == 0) BUG(); - s5p_mfc_clock_off(); - /* if suspending, wake up device and do not try_run again*/ + /* if suspending, wake up device*/ if (test_bit(0, &dev->enter_suspend)) wake_up_dev(dev, reason, err); - else - s5p_mfc_hw_call(dev->mfc_ops, try_run, dev); + + s5p_mfc_hw_call(dev->mfc_ops, try_run, dev); } /* Error handling for interrupt */ @@ -460,7 +458,8 @@ static void s5p_mfc_handle_error(struct s5p_mfc_dev *dev, if (test_and_clear_bit(0, &dev->hw_lock) == 0) BUG(); s5p_mfc_hw_call(dev->mfc_ops, clear_int_flags, dev); - s5p_mfc_clock_off(); + if (test_and_clear_bit(0, &dev->clk_flag)) + s5p_mfc_clock_off(); wake_up_dev(dev, reason, err); return; } @@ -514,7 +513,6 @@ static void s5p_mfc_handle_seq_done(struct s5p_mfc_ctx *ctx, clear_work_bit(ctx); if (test_and_clear_bit(0, &dev->hw_lock) == 0) BUG(); - s5p_mfc_clock_off(); s5p_mfc_hw_call(dev->mfc_ops, try_run, dev); wake_up_ctx(ctx, reason, err); } @@ -554,15 +552,14 @@ static void s5p_mfc_handle_init_buffers(struct s5p_mfc_ctx *ctx, if (test_and_clear_bit(0, &dev->hw_lock) == 0) BUG(); - s5p_mfc_clock_off(); - wake_up(&ctx->queue); s5p_mfc_hw_call(dev->mfc_ops, try_run, dev); } else { if (test_and_clear_bit(0, &dev->hw_lock) == 0) BUG(); - s5p_mfc_clock_off(); + if (test_and_clear_bit(0, &dev->clk_flag)) + s5p_mfc_clock_off(); wake_up(&ctx->queue); } @@ -596,7 +593,6 @@ static void s5p_mfc_handle_stream_complete(struct s5p_mfc_ctx *ctx, WARN_ON(test_and_clear_bit(0, &dev->hw_lock) == 0); - s5p_mfc_clock_off(); wake_up(&ctx->queue); s5p_mfc_hw_call(dev->mfc_ops, try_run, dev); } @@ -639,7 +635,6 @@ static irqreturn_t s5p_mfc_irq(int irq, void *priv) wake_up_ctx(ctx, reason, err); if (test_and_clear_bit(0, &dev->hw_lock) == 0) BUG(); - s5p_mfc_clock_off(); s5p_mfc_hw_call(dev->mfc_ops, try_run, dev); } else { s5p_mfc_handle_frame(ctx, reason, err); @@ -704,8 +699,6 @@ irq_cleanup_hw: if (test_and_clear_bit(0, &dev->hw_lock) == 0) mfc_err("Failed to unlock hw\n"); - s5p_mfc_clock_off(); - s5p_mfc_hw_call(dev->mfc_ops, try_run, dev); mfc_debug(2, "Exit via irq_cleanup_hw\n"); return IRQ_HANDLED; @@ -1216,6 +1209,7 @@ static int s5p_mfc_probe(struct platform_device *pdev) platform_set_drvdata(pdev, dev); dev->hw_lock = 0; + dev->clk_flag = 0; dev->watchdog_workqueue = create_singlethread_workqueue(S5P_MFC_NAME); INIT_WORK(&dev->watchdog_work, s5p_mfc_watchdog_worker); atomic_set(&dev->watchdog_cnt, 0); diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h index 01816ff..92f596e 100644 --- a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h @@ -289,6 +289,7 @@ struct s5p_mfc_priv_buf { * @watchdog_workqueue: workqueue for the watchdog * @watchdog_work: worker for the watchdog * @alloc_ctx: videobuf2 allocator contexts for two memory banks + * @clk_flag: flag used for dynamic control of mfc clock * @enter_suspend: flag set when entering suspend * @ctx_buf: common context memory (MFCv6) * @warn_start: hardware error code from which warnings start @@ -332,6 +333,7 @@ struct s5p_mfc_dev { struct workqueue_struct *watchdog_workqueue; struct work_struct watchdog_work; void *alloc_ctx[2]; + unsigned long clk_flag; unsigned long enter_suspend; struct s5p_mfc_priv_buf ctx_buf; diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v5.c b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v5.c index 58ec7bb..e2b2f31 100644 --- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v5.c +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v5.c @@ -1371,6 +1371,8 @@ static void s5p_mfc_try_run_v5(struct s5p_mfc_dev *dev) if (test_bit(0, &dev->enter_suspend)) { mfc_debug(1, "Entering suspend so do not schedule any jobs\n"); + if (test_and_clear_bit(0, &dev->clk_flag)) + s5p_mfc_clock_off(); return; } /* Check whether hardware is not running */ @@ -1383,6 +1385,8 @@ static void s5p_mfc_try_run_v5(struct s5p_mfc_dev *dev) new_ctx = s5p_mfc_get_new_ctx(dev); if (new_ctx < 0) { /* No contexts to run */ + if (test_and_clear_bit(0, &dev->clk_flag)) + s5p_mfc_clock_off(); if (test_and_clear_bit(0, &dev->hw_lock) == 0) { mfc_err("Failed to unlock hardware\n"); return; @@ -1396,7 +1400,9 @@ static void s5p_mfc_try_run_v5(struct s5p_mfc_dev *dev) * Last frame has already been sent to MFC. * Now obtaining frames from MFC buffer */ - s5p_mfc_clock_on(); + if (test_and_set_bit(0, &dev->clk_flag) == 0) + s5p_mfc_clock_on(); + if (ctx->type == MFCINST_DECODER) { s5p_mfc_set_dec_desc_buffer(ctx); switch (ctx->state) { @@ -1474,7 +1480,8 @@ static void s5p_mfc_try_run_v5(struct s5p_mfc_dev *dev) * scheduled, reduce the clock count as no one will * ever do this, because no interrupt related to this try_run * will ever come from hardware. */ - s5p_mfc_clock_off(); + if (test_and_clear_bit(0, &dev->clk_flag)) + s5p_mfc_clock_off(); } } diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c index 85600f2..8cf1c6f 100644 --- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c @@ -1745,6 +1745,13 @@ static void s5p_mfc_try_run_v6(struct s5p_mfc_dev *dev) mfc_debug(1, "Try run dev: %p\n", dev); + if (test_bit(0, &dev->enter_suspend)) { + mfc_debug(1, "Entering suspend so do not schedule any jobs\n"); + if (test_and_clear_bit(0, &dev->clk_flag)) + s5p_mfc_clock_off(); + return; + } + /* Check whether hardware is not running */ if (test_and_set_bit(0, &dev->hw_lock) != 0) { /* This is perfectly ok, the scheduled ctx should wait */ @@ -1756,6 +1763,8 @@ static void s5p_mfc_try_run_v6(struct s5p_mfc_dev *dev) new_ctx = s5p_mfc_get_new_ctx(dev); if (new_ctx < 0) { /* No contexts to run */ + if (test_and_clear_bit(0, &dev->clk_flag)) + s5p_mfc_clock_off(); if (test_and_clear_bit(0, &dev->hw_lock) == 0) { mfc_err("Failed to unlock hardware.\n"); return; @@ -1775,7 +1784,9 @@ static void s5p_mfc_try_run_v6(struct s5p_mfc_dev *dev) /* Last frame has already been sent to MFC * Now obtaining frames from MFC buffer */ - s5p_mfc_clock_on(); + if (test_and_set_bit(0, &dev->clk_flag) == 0) + s5p_mfc_clock_on(); + if (ctx->type == MFCINST_DECODER) { switch (ctx->state) { case MFCINST_FINISHING: @@ -1855,7 +1866,8 @@ static void s5p_mfc_try_run_v6(struct s5p_mfc_dev *dev) * scheduled, reduce the clock count as no one will * ever do this, because no interrupt related to this try_run * will ever come from hardware. */ - s5p_mfc_clock_off(); + if (test_and_clear_bit(0, &dev->clk_flag)) + s5p_mfc_clock_off(); } } -- 1.7.3.rc2 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html