[RESEND,v7,2/4] media: chips-media: wave5: Support runtime suspend/resume

Commit Message

jackson.lee Aug. 12, 2024, 7:08 a.m. UTC

Add support for runtime suspend/resume in the encoder and decoder. This is
achieved by saving the VPU state and powering it off while the VPU idle.

Signed-off-by: Jackson.lee <[e-mail]>
Signed-off-by: Nas Chung <[e-mail]>
Reviewed-by: Nicolas Dufresne <[e-mail]>
---
 .../platform/chips-media/wave5/wave5-helper.c | 13 -----
 .../platform/chips-media/wave5/wave5-hw.c     |  4 +-
 .../chips-media/wave5/wave5-vpu-dec.c         | 21 ++++++--
 .../chips-media/wave5/wave5-vpu-enc.c         | 20 ++++++--
 .../platform/chips-media/wave5/wave5-vpu.c    | 50 +++++++++++++++++++
 .../platform/chips-media/wave5/wave5-vpuapi.c | 33 ++++++++++--
 .../media/platform/chips-media/wave5/wave5.h  |  3 ++
 7 files changed, 118 insertions(+), 26 deletions(-)

Comments

Nicolas Dufresne Sept. 6, 2024, 5:08 p.m. UTC | #1

Hi,

Le lundi 12 août 2024 à 16:08 +0900, Jackson.lee a écrit :
> Add support for runtime suspend/resume in the encoder and decoder. This is
> achieved by saving the VPU state and powering it off while the VPU idle.

If you don't, I'll edit to "while the VPU *is* idle".

regards,
Nicolas

> 
> Signed-off-by: Jackson.lee <[e-mail]>
> Signed-off-by: Nas Chung <[e-mail]>
> Reviewed-by: Nicolas Dufresne <[e-mail]>
> ---
>  .../platform/chips-media/wave5/wave5-helper.c | 13 -----
>  .../platform/chips-media/wave5/wave5-hw.c     |  4 +-
>  .../chips-media/wave5/wave5-vpu-dec.c         | 21 ++++++--
>  .../chips-media/wave5/wave5-vpu-enc.c         | 20 ++++++--
>  .../platform/chips-media/wave5/wave5-vpu.c    | 50 +++++++++++++++++++
>  .../platform/chips-media/wave5/wave5-vpuapi.c | 33 ++++++++++--
>  .../media/platform/chips-media/wave5/wave5.h  |  3 ++
>  7 files changed, 118 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/media/platform/chips-media/wave5/wave5-helper.c b/drivers/media/platform/chips-media/wave5/wave5-helper.c
> index d60841c54a80..a20d84dbe3aa 100644
> --- a/drivers/media/platform/chips-media/wave5/wave5-helper.c
> +++ b/drivers/media/platform/chips-media/wave5/wave5-helper.c
> @@ -58,7 +58,6 @@ int wave5_vpu_release_device(struct file *filp,
>  			     char *name)
>  {
>  	struct vpu_instance *inst = wave5_to_vpu_inst(filp->private_data);
> -	struct vpu_device *dev = inst->dev;
>  	int ret = 0;
>  
>  	v4l2_m2m_ctx_release(inst->v4l2_fh.m2m_ctx);
> @@ -78,18 +77,6 @@ int wave5_vpu_release_device(struct file *filp,
>  	}
>  
>  	wave5_cleanup_instance(inst);
> -	if (dev->irq < 0) {
> -		ret = mutex_lock_interruptible(&dev->dev_lock);
> -		if (ret)
> -			return ret;
> -
> -		if (list_empty(&dev->instances)) {
> -			dev_dbg(dev->dev, "Disabling the hrtimer\n");
> -			hrtimer_cancel(&dev->hrtimer);
> -		}
> -
> -		mutex_unlock(&dev->dev_lock);
> -	}
>  
>  	return ret;
>  }
> diff --git a/drivers/media/platform/chips-media/wave5/wave5-hw.c b/drivers/media/platform/chips-media/wave5/wave5-hw.c
> index 2a98bab446d0..c8a905994109 100644
> --- a/drivers/media/platform/chips-media/wave5/wave5-hw.c
> +++ b/drivers/media/platform/chips-media/wave5/wave5-hw.c
> @@ -1228,8 +1228,8 @@ int wave5_vpu_re_init(struct device *dev, u8 *fw, size_t size)
>  	return setup_wave5_properties(dev);
>  }
>  
> -static int wave5_vpu_sleep_wake(struct device *dev, bool i_sleep_wake, const uint16_t *code,
> -				size_t size)
> +int wave5_vpu_sleep_wake(struct device *dev, bool i_sleep_wake, const uint16_t *code,
> +			 size_t size)
>  {
>  	u32 reg_val;
>  	struct vpu_buf *common_vb;
> diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
> index 0c5c9a8de91f..698c83e3082e 100644
> --- a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
> +++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
> @@ -5,6 +5,7 @@
>   * Copyright (C) 2021-2023 CHIPS&MEDIA INC
>   */
>  
> +#include <linux/pm_runtime.h>
>  #include "wave5-helper.h"
>  
>  #define VPU_DEC_DEV_NAME "C&M Wave5 VPU decoder"
> @@ -518,6 +519,8 @@ static void wave5_vpu_dec_finish_decode(struct vpu_instance *inst)
>  	if (q_status.report_queue_count == 0 &&
>  	    (q_status.instance_queue_count == 0 || dec_info.sequence_changed)) {
>  		dev_dbg(inst->dev->dev, "%s: finishing job.\n", __func__);
> +		pm_runtime_mark_last_busy(inst->dev->dev);
> +		pm_runtime_put_autosuspend(inst->dev->dev);
>  		v4l2_m2m_job_finish(inst->v4l2_m2m_dev, m2m_ctx);
>  	}
>  }
> @@ -1398,6 +1401,7 @@ static int wave5_vpu_dec_start_streaming(struct vb2_queue *q, unsigned int count
>  	int ret = 0;
>  
>  	dev_dbg(inst->dev->dev, "%s: type: %u\n", __func__, q->type);
> +	pm_runtime_resume_and_get(inst->dev->dev);
>  
>  	v4l2_m2m_update_start_streaming_state(m2m_ctx, q);
>  
> @@ -1429,13 +1433,15 @@ static int wave5_vpu_dec_start_streaming(struct vb2_queue *q, unsigned int count
>  		if (ret)
>  			goto return_buffers;
>  	}
> -
> +	pm_runtime_mark_last_busy(inst->dev->dev);
> +	pm_runtime_put_autosuspend(inst->dev->dev);
>  	return ret;
>  
>  free_bitstream_vbuf:
>  	wave5_vdi_free_dma_memory(inst->dev, &inst->bitstream_vbuf);
>  return_buffers:
>  	wave5_return_bufs(q, VB2_BUF_STATE_QUEUED);
> +	pm_runtime_put_autosuspend(inst->dev->dev);
>  	return ret;
>  }
>  
> @@ -1521,6 +1527,7 @@ static void wave5_vpu_dec_stop_streaming(struct vb2_queue *q)
>  	bool check_cmd = TRUE;
>  
>  	dev_dbg(inst->dev->dev, "%s: type: %u\n", __func__, q->type);
> +	pm_runtime_resume_and_get(inst->dev->dev);
>  
>  	while (check_cmd) {
>  		struct queue_status_info q_status;
> @@ -1544,6 +1551,9 @@ static void wave5_vpu_dec_stop_streaming(struct vb2_queue *q)
>  		streamoff_output(q);
>  	else
>  		streamoff_capture(q);
> +
> +	pm_runtime_mark_last_busy(inst->dev->dev);
> +	pm_runtime_put_autosuspend(inst->dev->dev);
>  }
>  
>  static const struct vb2_ops wave5_vpu_dec_vb2_ops = {
> @@ -1630,7 +1640,7 @@ static void wave5_vpu_dec_device_run(void *priv)
>  	int ret = 0;
>  
>  	dev_dbg(inst->dev->dev, "%s: Fill the ring buffer with new bitstream data", __func__);
> -
> +	pm_runtime_resume_and_get(inst->dev->dev);
>  	ret = fill_ringbuffer(inst);
>  	if (ret) {
>  		dev_warn(inst->dev->dev, "Filling ring buffer failed\n");
> @@ -1713,6 +1723,8 @@ static void wave5_vpu_dec_device_run(void *priv)
>  
>  finish_job_and_return:
>  	dev_dbg(inst->dev->dev, "%s: leave and finish job", __func__);
> +	pm_runtime_mark_last_busy(inst->dev->dev);
> +	pm_runtime_put_autosuspend(inst->dev->dev);
>  	v4l2_m2m_job_finish(inst->v4l2_m2m_dev, m2m_ctx);
>  }
>  
> @@ -1879,9 +1891,8 @@ static int wave5_vpu_open_dec(struct file *filp)
>  	if (ret)
>  		goto cleanup_inst;
>  
> -	if (dev->irq < 0 && !hrtimer_active(&dev->hrtimer) && list_empty(&dev->instances))
> -		hrtimer_start(&dev->hrtimer, ns_to_ktime(dev->vpu_poll_interval * NSEC_PER_MSEC),
> -			      HRTIMER_MODE_REL_PINNED);
> +	if (list_empty(&dev->instances))
> +		pm_runtime_use_autosuspend(inst->dev->dev);
>  
>  	list_add_tail(&inst->list, &dev->instances);
>  
> diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c b/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c
> index b731decbfda6..985de0c293e2 100644
> --- a/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c
> +++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c
> @@ -5,6 +5,7 @@
>   * Copyright (C) 2021-2023 CHIPS&MEDIA INC
>   */
>  
> +#include <linux/pm_runtime.h>
>  #include "wave5-helper.h"
>  
>  #define VPU_ENC_DEV_NAME "C&M Wave5 VPU encoder"
> @@ -1310,6 +1311,7 @@ static int wave5_vpu_enc_start_streaming(struct vb2_queue *q, unsigned int count
>  	struct v4l2_m2m_ctx *m2m_ctx = inst->v4l2_fh.m2m_ctx;
>  	int ret = 0;
>  
> +	pm_runtime_resume_and_get(inst->dev->dev);
>  	v4l2_m2m_update_start_streaming_state(m2m_ctx, q);
>  
>  	if (inst->state == VPU_INST_STATE_NONE && q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) {
> @@ -1364,9 +1366,13 @@ static int wave5_vpu_enc_start_streaming(struct vb2_queue *q, unsigned int count
>  	if (ret)
>  		goto return_buffers;
>  
> +	pm_runtime_mark_last_busy(inst->dev->dev);
> +	pm_runtime_put_autosuspend(inst->dev->dev);
>  	return 0;
>  return_buffers:
>  	wave5_return_bufs(q, VB2_BUF_STATE_QUEUED);
> +	pm_runtime_mark_last_busy(inst->dev->dev);
> +	pm_runtime_put_autosuspend(inst->dev->dev);
>  	return ret;
>  }
>  
> @@ -1408,6 +1414,7 @@ static void wave5_vpu_enc_stop_streaming(struct vb2_queue *q)
>  	 */
>  
>  	dev_dbg(inst->dev->dev, "%s: type: %u\n", __func__, q->type);
> +	pm_runtime_resume_and_get(inst->dev->dev);
>  
>  	if (wave5_vpu_both_queues_are_streaming(inst))
>  		switch_state(inst, VPU_INST_STATE_STOP);
> @@ -1432,6 +1439,9 @@ static void wave5_vpu_enc_stop_streaming(struct vb2_queue *q)
>  		streamoff_output(inst, q);
>  	else
>  		streamoff_capture(inst, q);
> +
> +	pm_runtime_mark_last_busy(inst->dev->dev);
> +	pm_runtime_put_autosuspend(inst->dev->dev);
>  }
>  
>  static const struct vb2_ops wave5_vpu_enc_vb2_ops = {
> @@ -1478,6 +1488,7 @@ static void wave5_vpu_enc_device_run(void *priv)
>  	u32 fail_res = 0;
>  	int ret = 0;
>  
> +	pm_runtime_resume_and_get(inst->dev->dev);
>  	switch (inst->state) {
>  	case VPU_INST_STATE_PIC_RUN:
>  		ret = start_encode(inst, &fail_res);
> @@ -1491,6 +1502,8 @@ static void wave5_vpu_enc_device_run(void *priv)
>  			break;
>  		}
>  		dev_dbg(inst->dev->dev, "%s: leave with active job", __func__);
> +		pm_runtime_mark_last_busy(inst->dev->dev);
> +		pm_runtime_put_autosuspend(inst->dev->dev);
>  		return;
>  	default:
>  		WARN(1, "Execution of a job in state %s is invalid.\n",
> @@ -1498,6 +1511,8 @@ static void wave5_vpu_enc_device_run(void *priv)
>  		break;
>  	}
>  	dev_dbg(inst->dev->dev, "%s: leave and finish job", __func__);
> +	pm_runtime_mark_last_busy(inst->dev->dev);
> +	pm_runtime_put_autosuspend(inst->dev->dev);
>  	v4l2_m2m_job_finish(inst->v4l2_m2m_dev, m2m_ctx);
>  }
>  
> @@ -1739,9 +1754,8 @@ static int wave5_vpu_open_enc(struct file *filp)
>  	if (ret)
>  		goto cleanup_inst;
>  
> -	if (dev->irq < 0 && !hrtimer_active(&dev->hrtimer) && list_empty(&dev->instances))
> -		hrtimer_start(&dev->hrtimer, ns_to_ktime(dev->vpu_poll_interval * NSEC_PER_MSEC),
> -			      HRTIMER_MODE_REL_PINNED);
> +	if (list_empty(&dev->instances))
> +		pm_runtime_use_autosuspend(inst->dev->dev);
>  
>  	list_add_tail(&inst->list, &dev->instances);
>  
> diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu.c b/drivers/media/platform/chips-media/wave5/wave5-vpu.c
> index 7273254ecb03..41c4bf64f27d 100644
> --- a/drivers/media/platform/chips-media/wave5/wave5-vpu.c
> +++ b/drivers/media/platform/chips-media/wave5/wave5-vpu.c
> @@ -10,6 +10,7 @@
>  #include <linux/clk.h>
>  #include <linux/firmware.h>
>  #include <linux/interrupt.h>
> +#include <linux/pm_runtime.h>
>  #include <linux/reset.h>
>  #include "wave5-vpu.h"
>  #include "wave5-regdefine.h"
> @@ -153,6 +154,45 @@ static int wave5_vpu_load_firmware(struct device *dev, const char *fw_name,
>  	return 0;
>  }
>  
> +static __maybe_unused int wave5_pm_suspend(struct device *dev)
> +{
> +	struct vpu_device *vpu = dev_get_drvdata(dev);
> +
> +	if (pm_runtime_suspended(dev))
> +		return 0;
> +
> +	if (vpu->irq < 0)
> +		hrtimer_cancel(&vpu->hrtimer);
> +
> +	wave5_vpu_sleep_wake(dev, true, NULL, 0);
> +	clk_bulk_disable_unprepare(vpu->num_clks, vpu->clks);
> +
> +	return 0;
> +}
> +
> +static __maybe_unused int wave5_pm_resume(struct device *dev)
> +{
> +	struct vpu_device *vpu = dev_get_drvdata(dev);
> +	int ret = 0;
> +
> +	wave5_vpu_sleep_wake(dev, false, NULL, 0);
> +	ret = clk_bulk_prepare_enable(vpu->num_clks, vpu->clks);
> +	if (ret) {
> +		dev_err(dev, "Enabling clocks, fail: %d\n", ret);
> +		return ret;
> +	}
> +
> +	if (vpu->irq < 0 && !hrtimer_active(&vpu->hrtimer))
> +		hrtimer_start(&vpu->hrtimer, ns_to_ktime(vpu->vpu_poll_interval * NSEC_PER_MSEC),
> +				HRTIMER_MODE_REL_PINNED);
> +
> +	return ret;
> +}
> +
> +static const struct dev_pm_ops wave5_pm_ops = {
> +	SET_RUNTIME_PM_OPS(wave5_pm_suspend, wave5_pm_resume, NULL)
> +};
> +
>  static int wave5_vpu_probe(struct platform_device *pdev)
>  {
>  	int ret;
> @@ -281,6 +321,12 @@ static int wave5_vpu_probe(struct platform_device *pdev)
>  		 (match_data->flags & WAVE5_IS_DEC) ? "'DECODE'" : "");
>  	dev_info(&pdev->dev, "Product Code:      0x%x\n", dev->product_code);
>  	dev_info(&pdev->dev, "Firmware Revision: %u\n", fw_revision);
> +
> +	pm_runtime_set_autosuspend_delay(&pdev->dev, 100);
> +	pm_runtime_use_autosuspend(&pdev->dev);
> +	pm_runtime_enable(&pdev->dev);
> +	wave5_vpu_sleep_wake(&pdev->dev, true, NULL, 0);
> +
>  	return 0;
>  
>  err_enc_unreg:
> @@ -310,6 +356,9 @@ static void wave5_vpu_remove(struct platform_device *pdev)
>  		hrtimer_cancel(&dev->hrtimer);
>  	}
>  
> +	pm_runtime_put_sync(&pdev->dev);
> +	pm_runtime_disable(&pdev->dev);
> +
>  	mutex_destroy(&dev->dev_lock);
>  	mutex_destroy(&dev->hw_lock);
>  	reset_control_assert(dev->resets);
> @@ -337,6 +386,7 @@ static struct platform_driver wave5_vpu_driver = {
>  	.driver = {
>  		.name = VPU_PLATFORM_DEVICE_NAME,
>  		.of_match_table = of_match_ptr(wave5_dt_ids),
> +		.pm = &wave5_pm_ops,
>  		},
>  	.probe = wave5_vpu_probe,
>  	.remove_new = wave5_vpu_remove,
> diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.c b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.c
> index 1a3efb638dde..e16b990041c2 100644
> --- a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.c
> +++ b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.c
> @@ -6,6 +6,8 @@
>   */
>  
>  #include <linux/bug.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/delay.h>
>  #include "wave5-vpuapi.h"
>  #include "wave5-regdefine.h"
>  #include "wave5.h"
> @@ -195,14 +197,20 @@ int wave5_vpu_dec_close(struct vpu_instance *inst, u32 *fail_res)
>  	int retry = 0;
>  	struct vpu_device *vpu_dev = inst->dev;
>  	int i;
> +	int inst_count = 0;
> +	struct vpu_instance *inst_elm;
>  
>  	*fail_res = 0;
>  	if (!inst->codec_info)
>  		return -EINVAL;
>  
> +	pm_runtime_resume_and_get(inst->dev->dev);
> +
>  	ret = mutex_lock_interruptible(&vpu_dev->hw_lock);
> -	if (ret)
> +	if (ret) {
> +		pm_runtime_put_sync(inst->dev->dev);
>  		return ret;
> +	}
>  
>  	do {
>  		ret = wave5_vpu_dec_finish_seq(inst, fail_res);
> @@ -232,9 +240,14 @@ int wave5_vpu_dec_close(struct vpu_instance *inst, u32 *fail_res)
>  
>  	wave5_vdi_free_dma_memory(vpu_dev, &p_dec_info->vb_task);
>  
> +	list_for_each_entry(inst_elm, &vpu_dev->instances, list)
> +		inst_count++;
> +	if (inst_count == 1)
> +		pm_runtime_dont_use_autosuspend(vpu_dev->dev);
> +
>  unlock_and_return:
>  	mutex_unlock(&vpu_dev->hw_lock);
> -
> +	pm_runtime_put_sync(inst->dev->dev);
>  	return ret;
>  }
>  
> @@ -697,25 +710,33 @@ int wave5_vpu_enc_close(struct vpu_instance *inst, u32 *fail_res)
>  	int ret;
>  	int retry = 0;
>  	struct vpu_device *vpu_dev = inst->dev;
> +	int inst_count = 0;
> +	struct vpu_instance *inst_elm;
>  
>  	*fail_res = 0;
>  	if (!inst->codec_info)
>  		return -EINVAL;
>  
> +	pm_runtime_resume_and_get(inst->dev->dev);
> +
>  	ret = mutex_lock_interruptible(&vpu_dev->hw_lock);
> -	if (ret)
> +	if (ret) {
> +		pm_runtime_resume_and_get(inst->dev->dev);
>  		return ret;
> +	}
>  
>  	do {
>  		ret = wave5_vpu_enc_finish_seq(inst, fail_res);
>  		if (ret < 0 && *fail_res != WAVE5_SYSERR_VPU_STILL_RUNNING) {
>  			dev_warn(inst->dev->dev, "enc_finish_seq timed out\n");
> +			pm_runtime_resume_and_get(inst->dev->dev);
>  			mutex_unlock(&vpu_dev->hw_lock);
>  			return ret;
>  		}
>  
>  		if (*fail_res == WAVE5_SYSERR_VPU_STILL_RUNNING &&
>  		    retry++ >= MAX_FIRMWARE_CALL_RETRY) {
> +			pm_runtime_resume_and_get(inst->dev->dev);
>  			mutex_unlock(&vpu_dev->hw_lock);
>  			return -ETIMEDOUT;
>  		}
> @@ -734,7 +755,13 @@ int wave5_vpu_enc_close(struct vpu_instance *inst, u32 *fail_res)
>  
>  	wave5_vdi_free_dma_memory(vpu_dev, &p_enc_info->vb_task);
>  
> +	list_for_each_entry(inst_elm, &vpu_dev->instances, list)
> +		inst_count++;
> +	if (inst_count == 1)
> +		pm_runtime_dont_use_autosuspend(vpu_dev->dev);
> +
>  	mutex_unlock(&vpu_dev->hw_lock);
> +	pm_runtime_put_sync(inst->dev->dev);
>  
>  	return 0;
>  }
> diff --git a/drivers/media/platform/chips-media/wave5/wave5.h b/drivers/media/platform/chips-media/wave5/wave5.h
> index 2a29b9164f97..2caab356f3e1 100644
> --- a/drivers/media/platform/chips-media/wave5/wave5.h
> +++ b/drivers/media/platform/chips-media/wave5/wave5.h
> @@ -62,6 +62,9 @@ int wave5_vpu_get_version(struct vpu_device *vpu_dev, u32 *revision);
>  
>  int wave5_vpu_init(struct device *dev, u8 *fw, size_t size);
>  
> +int wave5_vpu_sleep_wake(struct device *dev, bool i_sleep_wake, const uint16_t *code,
> +			 size_t size);
> +
>  int wave5_vpu_reset(struct device *dev, enum sw_reset_mode reset_mode);
>  
>  int wave5_vpu_build_up_dec_param(struct vpu_instance *inst, struct dec_open_param *param);

Nicolas Dufresne Sept. 6, 2024, 7:26 p.m. UTC | #2

Hi Again,

Le vendredi 06 septembre 2024 à 13:08 -0400, Nicolas Dufresne a écrit :
> Hi,
> 
> Le lundi 12 août 2024 à 16:08 +0900, Jackson.lee a écrit :
> > Add support for runtime suspend/resume in the encoder and decoder. This is
> > achieved by saving the VPU state and powering it off while the VPU idle.
> 
> If you don't, I'll edit to "while the VPU *is* idle".
> 
> regards,
> Nicolas
> 
> > 
> > Signed-off-by: Jackson.lee <[e-mail]>
> > Signed-off-by: Nas Chung <[e-mail]>
> > Reviewed-by: Nicolas Dufresne <[e-mail]>
> > ---
> >  .../platform/chips-media/wave5/wave5-helper.c | 13 -----
> >  .../platform/chips-media/wave5/wave5-hw.c     |  4 +-
> >  .../chips-media/wave5/wave5-vpu-dec.c         | 21 ++++++--
> >  .../chips-media/wave5/wave5-vpu-enc.c         | 20 ++++++--
> >  .../platform/chips-media/wave5/wave5-vpu.c    | 50 +++++++++++++++++++
> >  .../platform/chips-media/wave5/wave5-vpuapi.c | 33 ++++++++++--
> >  .../media/platform/chips-media/wave5/wave5.h  |  3 ++
> >  7 files changed, 118 insertions(+), 26 deletions(-)
> > 
> > diff --git a/drivers/media/platform/chips-media/wave5/wave5-helper.c b/drivers/media/platform/chips-media/wave5/wave5-helper.c
> > index d60841c54a80..a20d84dbe3aa 100644
> > --- a/drivers/media/platform/chips-media/wave5/wave5-helper.c
> > +++ b/drivers/media/platform/chips-media/wave5/wave5-helper.c
> > @@ -58,7 +58,6 @@ int wave5_vpu_release_device(struct file *filp,
> >  			     char *name)
> >  {
> >  	struct vpu_instance *inst = wave5_to_vpu_inst(filp->private_data);
> > -	struct vpu_device *dev = inst->dev;
> >  	int ret = 0;
> >  
> >  	v4l2_m2m_ctx_release(inst->v4l2_fh.m2m_ctx);
> > @@ -78,18 +77,6 @@ int wave5_vpu_release_device(struct file *filp,
> >  	}
> >  
> >  	wave5_cleanup_instance(inst);
> > -	if (dev->irq < 0) {
> > -		ret = mutex_lock_interruptible(&dev->dev_lock);
> > -		if (ret)
> > -			return ret;
> > -
> > -		if (list_empty(&dev->instances)) {
> > -			dev_dbg(dev->dev, "Disabling the hrtimer\n");
> > -			hrtimer_cancel(&dev->hrtimer);
> > -		}
> > -
> > -		mutex_unlock(&dev->dev_lock);
> > -	}
> >  
> >  	return ret;
> >  }
> > diff --git a/drivers/media/platform/chips-media/wave5/wave5-hw.c b/drivers/media/platform/chips-media/wave5/wave5-hw.c
> > index 2a98bab446d0..c8a905994109 100644
> > --- a/drivers/media/platform/chips-media/wave5/wave5-hw.c
> > +++ b/drivers/media/platform/chips-media/wave5/wave5-hw.c
> > @@ -1228,8 +1228,8 @@ int wave5_vpu_re_init(struct device *dev, u8 *fw, size_t size)
> >  	return setup_wave5_properties(dev);
> >  }
> >  
> > -static int wave5_vpu_sleep_wake(struct device *dev, bool i_sleep_wake, const uint16_t *code,
> > -				size_t size)
> > +int wave5_vpu_sleep_wake(struct device *dev, bool i_sleep_wake, const uint16_t *code,
> > +			 size_t size)
> >  {
> >  	u32 reg_val;
> >  	struct vpu_buf *common_vb;
> > diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
> > index 0c5c9a8de91f..698c83e3082e 100644
> > --- a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
> > +++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
> > @@ -5,6 +5,7 @@
> >   * Copyright (C) 2021-2023 CHIPS&MEDIA INC
> >   */
> >  
> > +#include <linux/pm_runtime.h>
> >  #include "wave5-helper.h"
> >  
> >  #define VPU_DEC_DEV_NAME "C&M Wave5 VPU decoder"
> > @@ -518,6 +519,8 @@ static void wave5_vpu_dec_finish_decode(struct vpu_instance *inst)
> >  	if (q_status.report_queue_count == 0 &&
> >  	    (q_status.instance_queue_count == 0 || dec_info.sequence_changed)) {
> >  		dev_dbg(inst->dev->dev, "%s: finishing job.\n", __func__);
> > +		pm_runtime_mark_last_busy(inst->dev->dev);
> > +		pm_runtime_put_autosuspend(inst->dev->dev);
> >  		v4l2_m2m_job_finish(inst->v4l2_m2m_dev, m2m_ctx);
> >  	}
> >  }
> > @@ -1398,6 +1401,7 @@ static int wave5_vpu_dec_start_streaming(struct vb2_queue *q, unsigned int count
> >  	int ret = 0;
> >  
> >  	dev_dbg(inst->dev->dev, "%s: type: %u\n", __func__, q->type);
> > +	pm_runtime_resume_and_get(inst->dev->dev);
> >  
> >  	v4l2_m2m_update_start_streaming_state(m2m_ctx, q);
> >  
> > @@ -1429,13 +1433,15 @@ static int wave5_vpu_dec_start_streaming(struct vb2_queue *q, unsigned int count
> >  		if (ret)
> >  			goto return_buffers;
> >  	}
> > -
> > +	pm_runtime_mark_last_busy(inst->dev->dev);
> > +	pm_runtime_put_autosuspend(inst->dev->dev);
> >  	return ret;
> >  
> >  free_bitstream_vbuf:
> >  	wave5_vdi_free_dma_memory(inst->dev, &inst->bitstream_vbuf);
> >  return_buffers:
> >  	wave5_return_bufs(q, VB2_BUF_STATE_QUEUED);
> > +	pm_runtime_put_autosuspend(inst->dev->dev);
> >  	return ret;
> >  }
> >  
> > @@ -1521,6 +1527,7 @@ static void wave5_vpu_dec_stop_streaming(struct vb2_queue *q)
> >  	bool check_cmd = TRUE;
> >  
> >  	dev_dbg(inst->dev->dev, "%s: type: %u\n", __func__, q->type);
> > +	pm_runtime_resume_and_get(inst->dev->dev);
> >  
> >  	while (check_cmd) {
> >  		struct queue_status_info q_status;
> > @@ -1544,6 +1551,9 @@ static void wave5_vpu_dec_stop_streaming(struct vb2_queue *q)
> >  		streamoff_output(q);
> >  	else
> >  		streamoff_capture(q);
> > +
> > +	pm_runtime_mark_last_busy(inst->dev->dev);
> > +	pm_runtime_put_autosuspend(inst->dev->dev);
> >  }
> >  
> >  static const struct vb2_ops wave5_vpu_dec_vb2_ops = {
> > @@ -1630,7 +1640,7 @@ static void wave5_vpu_dec_device_run(void *priv)
> >  	int ret = 0;
> >  
> >  	dev_dbg(inst->dev->dev, "%s: Fill the ring buffer with new bitstream data", __func__);
> > -
> > +	pm_runtime_resume_and_get(inst->dev->dev);
> >  	ret = fill_ringbuffer(inst);
> >  	if (ret) {
> >  		dev_warn(inst->dev->dev, "Filling ring buffer failed\n");
> > @@ -1713,6 +1723,8 @@ static void wave5_vpu_dec_device_run(void *priv)
> >  
> >  finish_job_and_return:
> >  	dev_dbg(inst->dev->dev, "%s: leave and finish job", __func__);
> > +	pm_runtime_mark_last_busy(inst->dev->dev);
> > +	pm_runtime_put_autosuspend(inst->dev->dev);
> >  	v4l2_m2m_job_finish(inst->v4l2_m2m_dev, m2m_ctx);
> >  }
> >  
> > @@ -1879,9 +1891,8 @@ static int wave5_vpu_open_dec(struct file *filp)
> >  	if (ret)
> >  		goto cleanup_inst;
> >  
> > -	if (dev->irq < 0 && !hrtimer_active(&dev->hrtimer) && list_empty(&dev->instances))
> > -		hrtimer_start(&dev->hrtimer, ns_to_ktime(dev->vpu_poll_interval * NSEC_PER_MSEC),
> > -			      HRTIMER_MODE_REL_PINNED);
> > +	if (list_empty(&dev->instances))
> > +		pm_runtime_use_autosuspend(inst->dev->dev);
> >  
> >  	list_add_tail(&inst->list, &dev->instances);
> >  
> > diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c b/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c
> > index b731decbfda6..985de0c293e2 100644
> > --- a/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c
> > +++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c
> > @@ -5,6 +5,7 @@
> >   * Copyright (C) 2021-2023 CHIPS&MEDIA INC
> >   */
> >  
> > +#include <linux/pm_runtime.h>
> >  #include "wave5-helper.h"
> >  
> >  #define VPU_ENC_DEV_NAME "C&M Wave5 VPU encoder"
> > @@ -1310,6 +1311,7 @@ static int wave5_vpu_enc_start_streaming(struct vb2_queue *q, unsigned int count
> >  	struct v4l2_m2m_ctx *m2m_ctx = inst->v4l2_fh.m2m_ctx;
> >  	int ret = 0;
> >  
> > +	pm_runtime_resume_and_get(inst->dev->dev);
> >  	v4l2_m2m_update_start_streaming_state(m2m_ctx, q);
> >  
> >  	if (inst->state == VPU_INST_STATE_NONE && q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) {
> > @@ -1364,9 +1366,13 @@ static int wave5_vpu_enc_start_streaming(struct vb2_queue *q, unsigned int count
> >  	if (ret)
> >  		goto return_buffers;
> >  
> > +	pm_runtime_mark_last_busy(inst->dev->dev);
> > +	pm_runtime_put_autosuspend(inst->dev->dev);
> >  	return 0;
> >  return_buffers:
> >  	wave5_return_bufs(q, VB2_BUF_STATE_QUEUED);
> > +	pm_runtime_mark_last_busy(inst->dev->dev);
> > +	pm_runtime_put_autosuspend(inst->dev->dev);
> >  	return ret;
> >  }
> >  
> > @@ -1408,6 +1414,7 @@ static void wave5_vpu_enc_stop_streaming(struct vb2_queue *q)
> >  	 */
> >  
> >  	dev_dbg(inst->dev->dev, "%s: type: %u\n", __func__, q->type);
> > +	pm_runtime_resume_and_get(inst->dev->dev);
> >  
> >  	if (wave5_vpu_both_queues_are_streaming(inst))
> >  		switch_state(inst, VPU_INST_STATE_STOP);
> > @@ -1432,6 +1439,9 @@ static void wave5_vpu_enc_stop_streaming(struct vb2_queue *q)
> >  		streamoff_output(inst, q);
> >  	else
> >  		streamoff_capture(inst, q);
> > +
> > +	pm_runtime_mark_last_busy(inst->dev->dev);
> > +	pm_runtime_put_autosuspend(inst->dev->dev);
> >  }
> >  
> >  static const struct vb2_ops wave5_vpu_enc_vb2_ops = {
> > @@ -1478,6 +1488,7 @@ static void wave5_vpu_enc_device_run(void *priv)
> >  	u32 fail_res = 0;
> >  	int ret = 0;
> >  
> > +	pm_runtime_resume_and_get(inst->dev->dev);
> >  	switch (inst->state) {
> >  	case VPU_INST_STATE_PIC_RUN:
> >  		ret = start_encode(inst, &fail_res);
> > @@ -1491,6 +1502,8 @@ static void wave5_vpu_enc_device_run(void *priv)
> >  			break;
> >  		}
> >  		dev_dbg(inst->dev->dev, "%s: leave with active job", __func__);
> > +		pm_runtime_mark_last_busy(inst->dev->dev);
> > +		pm_runtime_put_autosuspend(inst->dev->dev);
> >  		return;
> >  	default:
> >  		WARN(1, "Execution of a job in state %s is invalid.\n",
> > @@ -1498,6 +1511,8 @@ static void wave5_vpu_enc_device_run(void *priv)
> >  		break;
> >  	}
> >  	dev_dbg(inst->dev->dev, "%s: leave and finish job", __func__);
> > +	pm_runtime_mark_last_busy(inst->dev->dev);
> > +	pm_runtime_put_autosuspend(inst->dev->dev);
> >  	v4l2_m2m_job_finish(inst->v4l2_m2m_dev, m2m_ctx);
> >  }
> >  
> > @@ -1739,9 +1754,8 @@ static int wave5_vpu_open_enc(struct file *filp)
> >  	if (ret)
> >  		goto cleanup_inst;
> >  
> > -	if (dev->irq < 0 && !hrtimer_active(&dev->hrtimer) && list_empty(&dev->instances))
> > -		hrtimer_start(&dev->hrtimer, ns_to_ktime(dev->vpu_poll_interval * NSEC_PER_MSEC),
> > -			      HRTIMER_MODE_REL_PINNED);
> > +	if (list_empty(&dev->instances))
> > +		pm_runtime_use_autosuspend(inst->dev->dev);
> >  
> >  	list_add_tail(&inst->list, &dev->instances);
> >  
> > diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu.c b/drivers/media/platform/chips-media/wave5/wave5-vpu.c
> > index 7273254ecb03..41c4bf64f27d 100644
> > --- a/drivers/media/platform/chips-media/wave5/wave5-vpu.c
> > +++ b/drivers/media/platform/chips-media/wave5/wave5-vpu.c
> > @@ -10,6 +10,7 @@
> >  #include <linux/clk.h>
> >  #include <linux/firmware.h>
> >  #include <linux/interrupt.h>
> > +#include <linux/pm_runtime.h>
> >  #include <linux/reset.h>
> >  #include "wave5-vpu.h"
> >  #include "wave5-regdefine.h"
> > @@ -153,6 +154,45 @@ static int wave5_vpu_load_firmware(struct device *dev, const char *fw_name,
> >  	return 0;
> >  }
> >  
> > +static __maybe_unused int wave5_pm_suspend(struct device *dev)
> > +{
> > +	struct vpu_device *vpu = dev_get_drvdata(dev);
> > +
> > +	if (pm_runtime_suspended(dev))
> > +		return 0;
> > +
> > +	if (vpu->irq < 0)
> > +		hrtimer_cancel(&vpu->hrtimer);
> > +
> > +	wave5_vpu_sleep_wake(dev, true, NULL, 0);
> > +	clk_bulk_disable_unprepare(vpu->num_clks, vpu->clks);
> > +
> > +	return 0;
> > +}
> > +
> > +static __maybe_unused int wave5_pm_resume(struct device *dev)
> > +{
> > +	struct vpu_device *vpu = dev_get_drvdata(dev);
> > +	int ret = 0;
> > +
> > +	wave5_vpu_sleep_wake(dev, false, NULL, 0);
> > +	ret = clk_bulk_prepare_enable(vpu->num_clks, vpu->clks);
> > +	if (ret) {
> > +		dev_err(dev, "Enabling clocks, fail: %d\n", ret);
> > +		return ret;
> > +	}
> > +
> > +	if (vpu->irq < 0 && !hrtimer_active(&vpu->hrtimer))
> > +		hrtimer_start(&vpu->hrtimer, ns_to_ktime(vpu->vpu_poll_interval * NSEC_PER_MSEC),
> > +				HRTIMER_MODE_REL_PINNED);

I have fixed locally this style error. Alignment should match open parenthesis.
Checkpatch caught that, it saves times if you run checkpatch before your
submissions.

https://gitlab.freedesktop.org/ndufresne/media-staging/-/jobs/63283654

regards,
Nicolas

> > +
> > +	return ret;
> > +}
> > +
> > +static const struct dev_pm_ops wave5_pm_ops = {
> > +	SET_RUNTIME_PM_OPS(wave5_pm_suspend, wave5_pm_resume, NULL)
> > +};
> > +
> >  static int wave5_vpu_probe(struct platform_device *pdev)
> >  {
> >  	int ret;
> > @@ -281,6 +321,12 @@ static int wave5_vpu_probe(struct platform_device *pdev)
> >  		 (match_data->flags & WAVE5_IS_DEC) ? "'DECODE'" : "");
> >  	dev_info(&pdev->dev, "Product Code:      0x%x\n", dev->product_code);
> >  	dev_info(&pdev->dev, "Firmware Revision: %u\n", fw_revision);
> > +
> > +	pm_runtime_set_autosuspend_delay(&pdev->dev, 100);
> > +	pm_runtime_use_autosuspend(&pdev->dev);
> > +	pm_runtime_enable(&pdev->dev);
> > +	wave5_vpu_sleep_wake(&pdev->dev, true, NULL, 0);
> > +
> >  	return 0;
> >  
> >  err_enc_unreg:
> > @@ -310,6 +356,9 @@ static void wave5_vpu_remove(struct platform_device *pdev)
> >  		hrtimer_cancel(&dev->hrtimer);
> >  	}
> >  
> > +	pm_runtime_put_sync(&pdev->dev);
> > +	pm_runtime_disable(&pdev->dev);
> > +
> >  	mutex_destroy(&dev->dev_lock);
> >  	mutex_destroy(&dev->hw_lock);
> >  	reset_control_assert(dev->resets);
> > @@ -337,6 +386,7 @@ static struct platform_driver wave5_vpu_driver = {
> >  	.driver = {
> >  		.name = VPU_PLATFORM_DEVICE_NAME,
> >  		.of_match_table = of_match_ptr(wave5_dt_ids),
> > +		.pm = &wave5_pm_ops,
> >  		},
> >  	.probe = wave5_vpu_probe,
> >  	.remove_new = wave5_vpu_remove,
> > diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.c b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.c
> > index 1a3efb638dde..e16b990041c2 100644
> > --- a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.c
> > +++ b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.c
> > @@ -6,6 +6,8 @@
> >   */
> >  
> >  #include <linux/bug.h>
> > +#include <linux/pm_runtime.h>
> > +#include <linux/delay.h>
> >  #include "wave5-vpuapi.h"
> >  #include "wave5-regdefine.h"
> >  #include "wave5.h"
> > @@ -195,14 +197,20 @@ int wave5_vpu_dec_close(struct vpu_instance *inst, u32 *fail_res)
> >  	int retry = 0;
> >  	struct vpu_device *vpu_dev = inst->dev;
> >  	int i;
> > +	int inst_count = 0;
> > +	struct vpu_instance *inst_elm;
> >  
> >  	*fail_res = 0;
> >  	if (!inst->codec_info)
> >  		return -EINVAL;
> >  
> > +	pm_runtime_resume_and_get(inst->dev->dev);
> > +
> >  	ret = mutex_lock_interruptible(&vpu_dev->hw_lock);
> > -	if (ret)
> > +	if (ret) {
> > +		pm_runtime_put_sync(inst->dev->dev);
> >  		return ret;
> > +	}
> >  
> >  	do {
> >  		ret = wave5_vpu_dec_finish_seq(inst, fail_res);
> > @@ -232,9 +240,14 @@ int wave5_vpu_dec_close(struct vpu_instance *inst, u32 *fail_res)
> >  
> >  	wave5_vdi_free_dma_memory(vpu_dev, &p_dec_info->vb_task);
> >  
> > +	list_for_each_entry(inst_elm, &vpu_dev->instances, list)
> > +		inst_count++;
> > +	if (inst_count == 1)
> > +		pm_runtime_dont_use_autosuspend(vpu_dev->dev);
> > +
> >  unlock_and_return:
> >  	mutex_unlock(&vpu_dev->hw_lock);
> > -
> > +	pm_runtime_put_sync(inst->dev->dev);
> >  	return ret;
> >  }
> >  
> > @@ -697,25 +710,33 @@ int wave5_vpu_enc_close(struct vpu_instance *inst, u32 *fail_res)
> >  	int ret;
> >  	int retry = 0;
> >  	struct vpu_device *vpu_dev = inst->dev;
> > +	int inst_count = 0;
> > +	struct vpu_instance *inst_elm;
> >  
> >  	*fail_res = 0;
> >  	if (!inst->codec_info)
> >  		return -EINVAL;
> >  
> > +	pm_runtime_resume_and_get(inst->dev->dev);
> > +
> >  	ret = mutex_lock_interruptible(&vpu_dev->hw_lock);
> > -	if (ret)
> > +	if (ret) {
> > +		pm_runtime_resume_and_get(inst->dev->dev);
> >  		return ret;
> > +	}
> >  
> >  	do {
> >  		ret = wave5_vpu_enc_finish_seq(inst, fail_res);
> >  		if (ret < 0 && *fail_res != WAVE5_SYSERR_VPU_STILL_RUNNING) {
> >  			dev_warn(inst->dev->dev, "enc_finish_seq timed out\n");
> > +			pm_runtime_resume_and_get(inst->dev->dev);
> >  			mutex_unlock(&vpu_dev->hw_lock);
> >  			return ret;
> >  		}
> >  
> >  		if (*fail_res == WAVE5_SYSERR_VPU_STILL_RUNNING &&
> >  		    retry++ >= MAX_FIRMWARE_CALL_RETRY) {
> > +			pm_runtime_resume_and_get(inst->dev->dev);
> >  			mutex_unlock(&vpu_dev->hw_lock);
> >  			return -ETIMEDOUT;
> >  		}
> > @@ -734,7 +755,13 @@ int wave5_vpu_enc_close(struct vpu_instance *inst, u32 *fail_res)
> >  
> >  	wave5_vdi_free_dma_memory(vpu_dev, &p_enc_info->vb_task);
> >  
> > +	list_for_each_entry(inst_elm, &vpu_dev->instances, list)
> > +		inst_count++;
> > +	if (inst_count == 1)
> > +		pm_runtime_dont_use_autosuspend(vpu_dev->dev);
> > +
> >  	mutex_unlock(&vpu_dev->hw_lock);
> > +	pm_runtime_put_sync(inst->dev->dev);
> >  
> >  	return 0;
> >  }
> > diff --git a/drivers/media/platform/chips-media/wave5/wave5.h b/drivers/media/platform/chips-media/wave5/wave5.h
> > index 2a29b9164f97..2caab356f3e1 100644
> > --- a/drivers/media/platform/chips-media/wave5/wave5.h
> > +++ b/drivers/media/platform/chips-media/wave5/wave5.h
> > @@ -62,6 +62,9 @@ int wave5_vpu_get_version(struct vpu_device *vpu_dev, u32 *revision);
> >  
> >  int wave5_vpu_init(struct device *dev, u8 *fw, size_t size);
> >  
> > +int wave5_vpu_sleep_wake(struct device *dev, bool i_sleep_wake, const uint16_t *code,
> > +			 size_t size);
> > +
> >  int wave5_vpu_reset(struct device *dev, enum sw_reset_mode reset_mode);
> >  
> >  int wave5_vpu_build_up_dec_param(struct vpu_instance *inst, struct dec_open_param *param);
>

jackson.lee Sept. 9, 2024, 1:14 a.m. UTC | #3

Hi Nicolas

I agree it, thanks for your feedback.

> -----Original Message-----
> From: Nicolas Dufresne <[e-mail]>
> Sent: Saturday, September 7, 2024 2:08 AM
> To: jackson.lee <[e-mail]>; [e-mail];
> [e-mail]
> Cc: [e-mail]; [e-mail];
> [e-mail]; Nas Chung <[e-mail]>; lafley.kim
> <[e-mail]>; [e-mail]
> Subject: Re: [RESEND PATCH v7 2/4] media: chips-media: wave5: Support
> runtime suspend/resume
> 
> Hi,
> 
> Le lundi 12 août 2024 à 16:08 +0900, Jackson.lee a écrit :
> > Add support for runtime suspend/resume in the encoder and decoder.
> > This is achieved by saving the VPU state and powering it off while the
> VPU idle.
> 
> If you don't, I'll edit to "while the VPU *is* idle".
> 
> regards,
> Nicolas
> 
> >
> > Signed-off-by: Jackson.lee <[e-mail]>
> > Signed-off-by: Nas Chung <[e-mail]>
> > Reviewed-by: Nicolas Dufresne <[e-mail]>
> > ---
> >  .../platform/chips-media/wave5/wave5-helper.c | 13 -----
> >  .../platform/chips-media/wave5/wave5-hw.c     |  4 +-
> >  .../chips-media/wave5/wave5-vpu-dec.c         | 21 ++++++--
> >  .../chips-media/wave5/wave5-vpu-enc.c         | 20 ++++++--
> >  .../platform/chips-media/wave5/wave5-vpu.c    | 50 +++++++++++++++++++
> >  .../platform/chips-media/wave5/wave5-vpuapi.c | 33 ++++++++++--
> > .../media/platform/chips-media/wave5/wave5.h  |  3 ++
> >  7 files changed, 118 insertions(+), 26 deletions(-)
> >
> > diff --git a/drivers/media/platform/chips-media/wave5/wave5-helper.c
> > b/drivers/media/platform/chips-media/wave5/wave5-helper.c
> > index d60841c54a80..a20d84dbe3aa 100644
> > --- a/drivers/media/platform/chips-media/wave5/wave5-helper.c
> > +++ b/drivers/media/platform/chips-media/wave5/wave5-helper.c
> > @@ -58,7 +58,6 @@ int wave5_vpu_release_device(struct file *filp,
> >  			     char *name)
> >  {
> >  	struct vpu_instance *inst = wave5_to_vpu_inst(filp->private_data);
> > -	struct vpu_device *dev = inst->dev;
> >  	int ret = 0;
> >
> >  	v4l2_m2m_ctx_release(inst->v4l2_fh.m2m_ctx);
> > @@ -78,18 +77,6 @@ int wave5_vpu_release_device(struct file *filp,
> >  	}
> >
> >  	wave5_cleanup_instance(inst);
> > -	if (dev->irq < 0) {
> > -		ret = mutex_lock_interruptible(&dev->dev_lock);
> > -		if (ret)
> > -			return ret;
> > -
> > -		if (list_empty(&dev->instances)) {
> > -			dev_dbg(dev->dev, "Disabling the hrtimer\n");
> > -			hrtimer_cancel(&dev->hrtimer);
> > -		}
> > -
> > -		mutex_unlock(&dev->dev_lock);
> > -	}
> >
> >  	return ret;
> >  }
> > diff --git a/drivers/media/platform/chips-media/wave5/wave5-hw.c
> > b/drivers/media/platform/chips-media/wave5/wave5-hw.c
> > index 2a98bab446d0..c8a905994109 100644
> > --- a/drivers/media/platform/chips-media/wave5/wave5-hw.c
> > +++ b/drivers/media/platform/chips-media/wave5/wave5-hw.c
> > @@ -1228,8 +1228,8 @@ int wave5_vpu_re_init(struct device *dev, u8 *fw,
> size_t size)
> >  	return setup_wave5_properties(dev);
> >  }
> >
> > -static int wave5_vpu_sleep_wake(struct device *dev, bool i_sleep_wake,
> const uint16_t *code,
> > -				size_t size)
> > +int wave5_vpu_sleep_wake(struct device *dev, bool i_sleep_wake, const
> uint16_t *code,
> > +			 size_t size)
> >  {
> >  	u32 reg_val;
> >  	struct vpu_buf *common_vb;
> > diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
> > b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
> > index 0c5c9a8de91f..698c83e3082e 100644
> > --- a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
> > +++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
> > @@ -5,6 +5,7 @@
> >   * Copyright (C) 2021-2023 CHIPS&MEDIA INC
> >   */
> >
> > +#include <linux/pm_runtime.h>
> >  #include "wave5-helper.h"
> >
> >  #define VPU_DEC_DEV_NAME "C&M Wave5 VPU decoder"
> > @@ -518,6 +519,8 @@ static void wave5_vpu_dec_finish_decode(struct
> vpu_instance *inst)
> >  	if (q_status.report_queue_count == 0 &&
> >  	    (q_status.instance_queue_count == 0 ||
> dec_info.sequence_changed)) {
> >  		dev_dbg(inst->dev->dev, "%s: finishing job.\n", __func__);
> > +		pm_runtime_mark_last_busy(inst->dev->dev);
> > +		pm_runtime_put_autosuspend(inst->dev->dev);
> >  		v4l2_m2m_job_finish(inst->v4l2_m2m_dev, m2m_ctx);
> >  	}
> >  }
> > @@ -1398,6 +1401,7 @@ static int wave5_vpu_dec_start_streaming(struct
> vb2_queue *q, unsigned int count
> >  	int ret = 0;
> >
> >  	dev_dbg(inst->dev->dev, "%s: type: %u\n", __func__, q->type);
> > +	pm_runtime_resume_and_get(inst->dev->dev);
> >
> >  	v4l2_m2m_update_start_streaming_state(m2m_ctx, q);
> >
> > @@ -1429,13 +1433,15 @@ static int wave5_vpu_dec_start_streaming(struct
> vb2_queue *q, unsigned int count
> >  		if (ret)
> >  			goto return_buffers;
> >  	}
> > -
> > +	pm_runtime_mark_last_busy(inst->dev->dev);
> > +	pm_runtime_put_autosuspend(inst->dev->dev);
> >  	return ret;
> >
> >  free_bitstream_vbuf:
> >  	wave5_vdi_free_dma_memory(inst->dev, &inst->bitstream_vbuf);
> >  return_buffers:
> >  	wave5_return_bufs(q, VB2_BUF_STATE_QUEUED);
> > +	pm_runtime_put_autosuspend(inst->dev->dev);
> >  	return ret;
> >  }
> >
> > @@ -1521,6 +1527,7 @@ static void wave5_vpu_dec_stop_streaming(struct
> vb2_queue *q)
> >  	bool check_cmd = TRUE;
> >
> >  	dev_dbg(inst->dev->dev, "%s: type: %u\n", __func__, q->type);
> > +	pm_runtime_resume_and_get(inst->dev->dev);
> >
> >  	while (check_cmd) {
> >  		struct queue_status_info q_status;
> > @@ -1544,6 +1551,9 @@ static void wave5_vpu_dec_stop_streaming(struct
> vb2_queue *q)
> >  		streamoff_output(q);
> >  	else
> >  		streamoff_capture(q);
> > +
> > +	pm_runtime_mark_last_busy(inst->dev->dev);
> > +	pm_runtime_put_autosuspend(inst->dev->dev);
> >  }
> >
> >  static const struct vb2_ops wave5_vpu_dec_vb2_ops = { @@ -1630,7
> > +1640,7 @@ static void wave5_vpu_dec_device_run(void *priv)
> >  	int ret = 0;
> >
> >  	dev_dbg(inst->dev->dev, "%s: Fill the ring buffer with new
> bitstream
> > data", __func__);
> > -
> > +	pm_runtime_resume_and_get(inst->dev->dev);
> >  	ret = fill_ringbuffer(inst);
> >  	if (ret) {
> >  		dev_warn(inst->dev->dev, "Filling ring buffer failed\n"); @@
> > -1713,6 +1723,8 @@ static void wave5_vpu_dec_device_run(void *priv)
> >
> >  finish_job_and_return:
> >  	dev_dbg(inst->dev->dev, "%s: leave and finish job", __func__);
> > +	pm_runtime_mark_last_busy(inst->dev->dev);
> > +	pm_runtime_put_autosuspend(inst->dev->dev);
> >  	v4l2_m2m_job_finish(inst->v4l2_m2m_dev, m2m_ctx);  }
> >
> > @@ -1879,9 +1891,8 @@ static int wave5_vpu_open_dec(struct file *filp)
> >  	if (ret)
> >  		goto cleanup_inst;
> >
> > -	if (dev->irq < 0 && !hrtimer_active(&dev->hrtimer) &&
> list_empty(&dev->instances))
> > -		hrtimer_start(&dev->hrtimer, ns_to_ktime(dev-
> >vpu_poll_interval * NSEC_PER_MSEC),
> > -			      HRTIMER_MODE_REL_PINNED);
> > +	if (list_empty(&dev->instances))
> > +		pm_runtime_use_autosuspend(inst->dev->dev);
> >
> >  	list_add_tail(&inst->list, &dev->instances);
> >
> > diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c
> > b/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c
> > index b731decbfda6..985de0c293e2 100644
> > --- a/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c
> > +++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c
> > @@ -5,6 +5,7 @@
> >   * Copyright (C) 2021-2023 CHIPS&MEDIA INC
> >   */
> >
> > +#include <linux/pm_runtime.h>
> >  #include "wave5-helper.h"
> >
> >  #define VPU_ENC_DEV_NAME "C&M Wave5 VPU encoder"
> > @@ -1310,6 +1311,7 @@ static int wave5_vpu_enc_start_streaming(struct
> vb2_queue *q, unsigned int count
> >  	struct v4l2_m2m_ctx *m2m_ctx = inst->v4l2_fh.m2m_ctx;
> >  	int ret = 0;
> >
> > +	pm_runtime_resume_and_get(inst->dev->dev);
> >  	v4l2_m2m_update_start_streaming_state(m2m_ctx, q);
> >
> >  	if (inst->state == VPU_INST_STATE_NONE && q->type ==
> > V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) { @@ -1364,9 +1366,13 @@ static int
> wave5_vpu_enc_start_streaming(struct vb2_queue *q, unsigned int count
> >  	if (ret)
> >  		goto return_buffers;
> >
> > +	pm_runtime_mark_last_busy(inst->dev->dev);
> > +	pm_runtime_put_autosuspend(inst->dev->dev);
> >  	return 0;
> >  return_buffers:
> >  	wave5_return_bufs(q, VB2_BUF_STATE_QUEUED);
> > +	pm_runtime_mark_last_busy(inst->dev->dev);
> > +	pm_runtime_put_autosuspend(inst->dev->dev);
> >  	return ret;
> >  }
> >
> > @@ -1408,6 +1414,7 @@ static void wave5_vpu_enc_stop_streaming(struct
> vb2_queue *q)
> >  	 */
> >
> >  	dev_dbg(inst->dev->dev, "%s: type: %u\n", __func__, q->type);
> > +	pm_runtime_resume_and_get(inst->dev->dev);
> >
> >  	if (wave5_vpu_both_queues_are_streaming(inst))
> >  		switch_state(inst, VPU_INST_STATE_STOP); @@ -1432,6 +1439,9
> @@
> > static void wave5_vpu_enc_stop_streaming(struct vb2_queue *q)
> >  		streamoff_output(inst, q);
> >  	else
> >  		streamoff_capture(inst, q);
> > +
> > +	pm_runtime_mark_last_busy(inst->dev->dev);
> > +	pm_runtime_put_autosuspend(inst->dev->dev);
> >  }
> >
> >  static const struct vb2_ops wave5_vpu_enc_vb2_ops = { @@ -1478,6
> > +1488,7 @@ static void wave5_vpu_enc_device_run(void *priv)
> >  	u32 fail_res = 0;
> >  	int ret = 0;
> >
> > +	pm_runtime_resume_and_get(inst->dev->dev);
> >  	switch (inst->state) {
> >  	case VPU_INST_STATE_PIC_RUN:
> >  		ret = start_encode(inst, &fail_res); @@ -1491,6 +1502,8 @@
> static
> > void wave5_vpu_enc_device_run(void *priv)
> >  			break;
> >  		}
> >  		dev_dbg(inst->dev->dev, "%s: leave with active job",
> __func__);
> > +		pm_runtime_mark_last_busy(inst->dev->dev);
> > +		pm_runtime_put_autosuspend(inst->dev->dev);
> >  		return;
> >  	default:
> >  		WARN(1, "Execution of a job in state %s is invalid.\n", @@ -
> 1498,6
> > +1511,8 @@ static void wave5_vpu_enc_device_run(void *priv)
> >  		break;
> >  	}
> >  	dev_dbg(inst->dev->dev, "%s: leave and finish job", __func__);
> > +	pm_runtime_mark_last_busy(inst->dev->dev);
> > +	pm_runtime_put_autosuspend(inst->dev->dev);
> >  	v4l2_m2m_job_finish(inst->v4l2_m2m_dev, m2m_ctx);  }
> >
> > @@ -1739,9 +1754,8 @@ static int wave5_vpu_open_enc(struct file *filp)
> >  	if (ret)
> >  		goto cleanup_inst;
> >
> > -	if (dev->irq < 0 && !hrtimer_active(&dev->hrtimer) &&
> list_empty(&dev->instances))
> > -		hrtimer_start(&dev->hrtimer, ns_to_ktime(dev-
> >vpu_poll_interval * NSEC_PER_MSEC),
> > -			      HRTIMER_MODE_REL_PINNED);
> > +	if (list_empty(&dev->instances))
> > +		pm_runtime_use_autosuspend(inst->dev->dev);
> >
> >  	list_add_tail(&inst->list, &dev->instances);
> >
> > diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu.c
> > b/drivers/media/platform/chips-media/wave5/wave5-vpu.c
> > index 7273254ecb03..41c4bf64f27d 100644
> > --- a/drivers/media/platform/chips-media/wave5/wave5-vpu.c
> > +++ b/drivers/media/platform/chips-media/wave5/wave5-vpu.c
> > @@ -10,6 +10,7 @@
> >  #include <linux/clk.h>
> >  #include <linux/firmware.h>
> >  #include <linux/interrupt.h>
> > +#include <linux/pm_runtime.h>
> >  #include <linux/reset.h>
> >  #include "wave5-vpu.h"
> >  #include "wave5-regdefine.h"
> > @@ -153,6 +154,45 @@ static int wave5_vpu_load_firmware(struct device
> *dev, const char *fw_name,
> >  	return 0;
> >  }
> >
> > +static __maybe_unused int wave5_pm_suspend(struct device *dev) {
> > +	struct vpu_device *vpu = dev_get_drvdata(dev);
> > +
> > +	if (pm_runtime_suspended(dev))
> > +		return 0;
> > +
> > +	if (vpu->irq < 0)
> > +		hrtimer_cancel(&vpu->hrtimer);
> > +
> > +	wave5_vpu_sleep_wake(dev, true, NULL, 0);
> > +	clk_bulk_disable_unprepare(vpu->num_clks, vpu->clks);
> > +
> > +	return 0;
> > +}
> > +
> > +static __maybe_unused int wave5_pm_resume(struct device *dev) {
> > +	struct vpu_device *vpu = dev_get_drvdata(dev);
> > +	int ret = 0;
> > +
> > +	wave5_vpu_sleep_wake(dev, false, NULL, 0);
> > +	ret = clk_bulk_prepare_enable(vpu->num_clks, vpu->clks);
> > +	if (ret) {
> > +		dev_err(dev, "Enabling clocks, fail: %d\n", ret);
> > +		return ret;
> > +	}
> > +
> > +	if (vpu->irq < 0 && !hrtimer_active(&vpu->hrtimer))
> > +		hrtimer_start(&vpu->hrtimer, ns_to_ktime(vpu-
> >vpu_poll_interval * NSEC_PER_MSEC),
> > +				HRTIMER_MODE_REL_PINNED);
> > +
> > +	return ret;
> > +}
> > +
> > +static const struct dev_pm_ops wave5_pm_ops = {
> > +	SET_RUNTIME_PM_OPS(wave5_pm_suspend, wave5_pm_resume, NULL) };
> > +
> >  static int wave5_vpu_probe(struct platform_device *pdev)  {
> >  	int ret;
> > @@ -281,6 +321,12 @@ static int wave5_vpu_probe(struct platform_device
> *pdev)
> >  		 (match_data->flags & WAVE5_IS_DEC) ? "'DECODE'" : "");
> >  	dev_info(&pdev->dev, "Product Code:      0x%x\n", dev->product_code);
> >  	dev_info(&pdev->dev, "Firmware Revision: %u\n", fw_revision);
> > +
> > +	pm_runtime_set_autosuspend_delay(&pdev->dev, 100);
> > +	pm_runtime_use_autosuspend(&pdev->dev);
> > +	pm_runtime_enable(&pdev->dev);
> > +	wave5_vpu_sleep_wake(&pdev->dev, true, NULL, 0);
> > +
> >  	return 0;
> >
> >  err_enc_unreg:
> > @@ -310,6 +356,9 @@ static void wave5_vpu_remove(struct platform_device
> *pdev)
> >  		hrtimer_cancel(&dev->hrtimer);
> >  	}
> >
> > +	pm_runtime_put_sync(&pdev->dev);
> > +	pm_runtime_disable(&pdev->dev);
> > +
> >  	mutex_destroy(&dev->dev_lock);
> >  	mutex_destroy(&dev->hw_lock);
> >  	reset_control_assert(dev->resets);
> > @@ -337,6 +386,7 @@ static struct platform_driver wave5_vpu_driver = {
> >  	.driver = {
> >  		.name = VPU_PLATFORM_DEVICE_NAME,
> >  		.of_match_table = of_match_ptr(wave5_dt_ids),
> > +		.pm = &wave5_pm_ops,
> >  		},
> >  	.probe = wave5_vpu_probe,
> >  	.remove_new = wave5_vpu_remove,
> > diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.c
> > b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.c
> > index 1a3efb638dde..e16b990041c2 100644
> > --- a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.c
> > +++ b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.c
> > @@ -6,6 +6,8 @@
> >   */
> >
> >  #include <linux/bug.h>
> > +#include <linux/pm_runtime.h>
> > +#include <linux/delay.h>
> >  #include "wave5-vpuapi.h"
> >  #include "wave5-regdefine.h"
> >  #include "wave5.h"
> > @@ -195,14 +197,20 @@ int wave5_vpu_dec_close(struct vpu_instance *inst,
> u32 *fail_res)
> >  	int retry = 0;
> >  	struct vpu_device *vpu_dev = inst->dev;
> >  	int i;
> > +	int inst_count = 0;
> > +	struct vpu_instance *inst_elm;
> >
> >  	*fail_res = 0;
> >  	if (!inst->codec_info)
> >  		return -EINVAL;
> >
> > +	pm_runtime_resume_and_get(inst->dev->dev);
> > +
> >  	ret = mutex_lock_interruptible(&vpu_dev->hw_lock);
> > -	if (ret)
> > +	if (ret) {
> > +		pm_runtime_put_sync(inst->dev->dev);
> >  		return ret;
> > +	}
> >
> >  	do {
> >  		ret = wave5_vpu_dec_finish_seq(inst, fail_res); @@ -232,9
> +240,14
> > @@ int wave5_vpu_dec_close(struct vpu_instance *inst, u32 *fail_res)
> >
> >  	wave5_vdi_free_dma_memory(vpu_dev, &p_dec_info->vb_task);
> >
> > +	list_for_each_entry(inst_elm, &vpu_dev->instances, list)
> > +		inst_count++;
> > +	if (inst_count == 1)
> > +		pm_runtime_dont_use_autosuspend(vpu_dev->dev);
> > +
> >  unlock_and_return:
> >  	mutex_unlock(&vpu_dev->hw_lock);
> > -
> > +	pm_runtime_put_sync(inst->dev->dev);
> >  	return ret;
> >  }
> >
> > @@ -697,25 +710,33 @@ int wave5_vpu_enc_close(struct vpu_instance *inst,
> u32 *fail_res)
> >  	int ret;
> >  	int retry = 0;
> >  	struct vpu_device *vpu_dev = inst->dev;
> > +	int inst_count = 0;
> > +	struct vpu_instance *inst_elm;
> >
> >  	*fail_res = 0;
> >  	if (!inst->codec_info)
> >  		return -EINVAL;
> >
> > +	pm_runtime_resume_and_get(inst->dev->dev);
> > +
> >  	ret = mutex_lock_interruptible(&vpu_dev->hw_lock);
> > -	if (ret)
> > +	if (ret) {
> > +		pm_runtime_resume_and_get(inst->dev->dev);
> >  		return ret;
> > +	}
> >
> >  	do {
> >  		ret = wave5_vpu_enc_finish_seq(inst, fail_res);
> >  		if (ret < 0 && *fail_res != WAVE5_SYSERR_VPU_STILL_RUNNING)
> {
> >  			dev_warn(inst->dev->dev, "enc_finish_seq timed out\n");
> > +			pm_runtime_resume_and_get(inst->dev->dev);
> >  			mutex_unlock(&vpu_dev->hw_lock);
> >  			return ret;
> >  		}
> >
> >  		if (*fail_res == WAVE5_SYSERR_VPU_STILL_RUNNING &&
> >  		    retry++ >= MAX_FIRMWARE_CALL_RETRY) {
> > +			pm_runtime_resume_and_get(inst->dev->dev);
> >  			mutex_unlock(&vpu_dev->hw_lock);
> >  			return -ETIMEDOUT;
> >  		}
> > @@ -734,7 +755,13 @@ int wave5_vpu_enc_close(struct vpu_instance
> > *inst, u32 *fail_res)
> >
> >  	wave5_vdi_free_dma_memory(vpu_dev, &p_enc_info->vb_task);
> >
> > +	list_for_each_entry(inst_elm, &vpu_dev->instances, list)
> > +		inst_count++;
> > +	if (inst_count == 1)
> > +		pm_runtime_dont_use_autosuspend(vpu_dev->dev);
> > +
> >  	mutex_unlock(&vpu_dev->hw_lock);
> > +	pm_runtime_put_sync(inst->dev->dev);
> >
> >  	return 0;
> >  }
> > diff --git a/drivers/media/platform/chips-media/wave5/wave5.h
> > b/drivers/media/platform/chips-media/wave5/wave5.h
> > index 2a29b9164f97..2caab356f3e1 100644
> > --- a/drivers/media/platform/chips-media/wave5/wave5.h
> > +++ b/drivers/media/platform/chips-media/wave5/wave5.h
> > @@ -62,6 +62,9 @@ int wave5_vpu_get_version(struct vpu_device
> > *vpu_dev, u32 *revision);
> >
> >  int wave5_vpu_init(struct device *dev, u8 *fw, size_t size);
> >
> > +int wave5_vpu_sleep_wake(struct device *dev, bool i_sleep_wake, const
> uint16_t *code,
> > +			 size_t size);
> > +
> >  int wave5_vpu_reset(struct device *dev, enum sw_reset_mode
> > reset_mode);
> >
> >  int wave5_vpu_build_up_dec_param(struct vpu_instance *inst, struct
> > dec_open_param *param);

jackson.lee Sept. 9, 2024, 1:21 a.m. UTC | #4

Hi Nicolas

> -----Original Message-----
> From: Nicolas Dufresne <[e-mail]>
> Sent: Saturday, September 7, 2024 4:27 AM
> To: jackson.lee <[e-mail]>; [e-mail];
> [e-mail]
> Cc: [e-mail]; [e-mail];
> [e-mail]; Nas Chung <[e-mail]>; lafley.kim
> <[e-mail]>; [e-mail]
> Subject: Re: [RESEND PATCH v7 2/4] media: chips-media: wave5: Support
> runtime suspend/resume
> 
> Hi Again,
> 
> Le vendredi 06 septembre 2024 à 13:08 -0400, Nicolas Dufresne a écrit :
> > Hi,
> >
> > Le lundi 12 août 2024 à 16:08 +0900, Jackson.lee a écrit :
> > > Add support for runtime suspend/resume in the encoder and decoder.
> > > This is achieved by saving the VPU state and powering it off while the
> VPU idle.
> >
> > If you don't, I'll edit to "while the VPU *is* idle".
> >
> > regards,
> > Nicolas
> >
> > >
> > > Signed-off-by: Jackson.lee <[e-mail]>
> > > Signed-off-by: Nas Chung <[e-mail]>
> > > Reviewed-by: Nicolas Dufresne <[e-mail]>
> > > ---
> > >  .../platform/chips-media/wave5/wave5-helper.c | 13 -----
> > >  .../platform/chips-media/wave5/wave5-hw.c     |  4 +-
> > >  .../chips-media/wave5/wave5-vpu-dec.c         | 21 ++++++--
> > >  .../chips-media/wave5/wave5-vpu-enc.c         | 20 ++++++--
> > >  .../platform/chips-media/wave5/wave5-vpu.c    | 50 +++++++++++++++++++
> > >  .../platform/chips-media/wave5/wave5-vpuapi.c | 33 ++++++++++--
> > > .../media/platform/chips-media/wave5/wave5.h  |  3 ++
> > >  7 files changed, 118 insertions(+), 26 deletions(-)
> > >
> > > diff --git a/drivers/media/platform/chips-media/wave5/wave5-helper.c
> > > b/drivers/media/platform/chips-media/wave5/wave5-helper.c
> > > index d60841c54a80..a20d84dbe3aa 100644
> > > --- a/drivers/media/platform/chips-media/wave5/wave5-helper.c
> > > +++ b/drivers/media/platform/chips-media/wave5/wave5-helper.c
> > > @@ -58,7 +58,6 @@ int wave5_vpu_release_device(struct file *filp,
> > >  			     char *name)
> > >  {
> > >  	struct vpu_instance *inst = wave5_to_vpu_inst(filp->private_data);
> > > -	struct vpu_device *dev = inst->dev;
> > >  	int ret = 0;
> > >
> > >  	v4l2_m2m_ctx_release(inst->v4l2_fh.m2m_ctx);
> > > @@ -78,18 +77,6 @@ int wave5_vpu_release_device(struct file *filp,
> > >  	}
> > >
> > >  	wave5_cleanup_instance(inst);
> > > -	if (dev->irq < 0) {
> > > -		ret = mutex_lock_interruptible(&dev->dev_lock);
> > > -		if (ret)
> > > -			return ret;
> > > -
> > > -		if (list_empty(&dev->instances)) {
> > > -			dev_dbg(dev->dev, "Disabling the hrtimer\n");
> > > -			hrtimer_cancel(&dev->hrtimer);
> > > -		}
> > > -
> > > -		mutex_unlock(&dev->dev_lock);
> > > -	}
> > >
> > >  	return ret;
> > >  }
> > > diff --git a/drivers/media/platform/chips-media/wave5/wave5-hw.c
> > > b/drivers/media/platform/chips-media/wave5/wave5-hw.c
> > > index 2a98bab446d0..c8a905994109 100644
> > > --- a/drivers/media/platform/chips-media/wave5/wave5-hw.c
> > > +++ b/drivers/media/platform/chips-media/wave5/wave5-hw.c
> > > @@ -1228,8 +1228,8 @@ int wave5_vpu_re_init(struct device *dev, u8 *fw,
> size_t size)
> > >  	return setup_wave5_properties(dev);  }
> > >
> > > -static int wave5_vpu_sleep_wake(struct device *dev, bool i_sleep_wake,
> const uint16_t *code,
> > > -				size_t size)
> > > +int wave5_vpu_sleep_wake(struct device *dev, bool i_sleep_wake, const
> uint16_t *code,
> > > +			 size_t size)
> > >  {
> > >  	u32 reg_val;
> > >  	struct vpu_buf *common_vb;
> > > diff --git
> > > a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
> > > b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
> > > index 0c5c9a8de91f..698c83e3082e 100644
> > > --- a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
> > > +++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
> > > @@ -5,6 +5,7 @@
> > >   * Copyright (C) 2021-2023 CHIPS&MEDIA INC
> > >   */
> > >
> > > +#include <linux/pm_runtime.h>
> > >  #include "wave5-helper.h"
> > >
> > >  #define VPU_DEC_DEV_NAME "C&M Wave5 VPU decoder"
> > > @@ -518,6 +519,8 @@ static void wave5_vpu_dec_finish_decode(struct
> vpu_instance *inst)
> > >  	if (q_status.report_queue_count == 0 &&
> > >  	    (q_status.instance_queue_count == 0 ||
> dec_info.sequence_changed)) {
> > >  		dev_dbg(inst->dev->dev, "%s: finishing job.\n", __func__);
> > > +		pm_runtime_mark_last_busy(inst->dev->dev);
> > > +		pm_runtime_put_autosuspend(inst->dev->dev);
> > >  		v4l2_m2m_job_finish(inst->v4l2_m2m_dev, m2m_ctx);
> > >  	}
> > >  }
> > > @@ -1398,6 +1401,7 @@ static int wave5_vpu_dec_start_streaming(struct
> vb2_queue *q, unsigned int count
> > >  	int ret = 0;
> > >
> > >  	dev_dbg(inst->dev->dev, "%s: type: %u\n", __func__, q->type);
> > > +	pm_runtime_resume_and_get(inst->dev->dev);
> > >
> > >  	v4l2_m2m_update_start_streaming_state(m2m_ctx, q);
> > >
> > > @@ -1429,13 +1433,15 @@ static int
> wave5_vpu_dec_start_streaming(struct vb2_queue *q, unsigned int count
> > >  		if (ret)
> > >  			goto return_buffers;
> > >  	}
> > > -
> > > +	pm_runtime_mark_last_busy(inst->dev->dev);
> > > +	pm_runtime_put_autosuspend(inst->dev->dev);
> > >  	return ret;
> > >
> > >  free_bitstream_vbuf:
> > >  	wave5_vdi_free_dma_memory(inst->dev, &inst->bitstream_vbuf);
> > >  return_buffers:
> > >  	wave5_return_bufs(q, VB2_BUF_STATE_QUEUED);
> > > +	pm_runtime_put_autosuspend(inst->dev->dev);
> > >  	return ret;
> > >  }
> > >
> > > @@ -1521,6 +1527,7 @@ static void wave5_vpu_dec_stop_streaming(struct
> vb2_queue *q)
> > >  	bool check_cmd = TRUE;
> > >
> > >  	dev_dbg(inst->dev->dev, "%s: type: %u\n", __func__, q->type);
> > > +	pm_runtime_resume_and_get(inst->dev->dev);
> > >
> > >  	while (check_cmd) {
> > >  		struct queue_status_info q_status; @@ -1544,6 +1551,9 @@
> static
> > > void wave5_vpu_dec_stop_streaming(struct vb2_queue *q)
> > >  		streamoff_output(q);
> > >  	else
> > >  		streamoff_capture(q);
> > > +
> > > +	pm_runtime_mark_last_busy(inst->dev->dev);
> > > +	pm_runtime_put_autosuspend(inst->dev->dev);
> > >  }
> > >
> > >  static const struct vb2_ops wave5_vpu_dec_vb2_ops = { @@ -1630,7
> > > +1640,7 @@ static void wave5_vpu_dec_device_run(void *priv)
> > >  	int ret = 0;
> > >
> > >  	dev_dbg(inst->dev->dev, "%s: Fill the ring buffer with new
> > > bitstream data", __func__);
> > > -
> > > +	pm_runtime_resume_and_get(inst->dev->dev);
> > >  	ret = fill_ringbuffer(inst);
> > >  	if (ret) {
> > >  		dev_warn(inst->dev->dev, "Filling ring buffer failed\n"); @@
> > > -1713,6 +1723,8 @@ static void wave5_vpu_dec_device_run(void *priv)
> > >
> > >  finish_job_and_return:
> > >  	dev_dbg(inst->dev->dev, "%s: leave and finish job", __func__);
> > > +	pm_runtime_mark_last_busy(inst->dev->dev);
> > > +	pm_runtime_put_autosuspend(inst->dev->dev);
> > >  	v4l2_m2m_job_finish(inst->v4l2_m2m_dev, m2m_ctx);  }
> > >
> > > @@ -1879,9 +1891,8 @@ static int wave5_vpu_open_dec(struct file *filp)
> > >  	if (ret)
> > >  		goto cleanup_inst;
> > >
> > > -	if (dev->irq < 0 && !hrtimer_active(&dev->hrtimer) &&
> list_empty(&dev->instances))
> > > -		hrtimer_start(&dev->hrtimer, ns_to_ktime(dev-
> >vpu_poll_interval * NSEC_PER_MSEC),
> > > -			      HRTIMER_MODE_REL_PINNED);
> > > +	if (list_empty(&dev->instances))
> > > +		pm_runtime_use_autosuspend(inst->dev->dev);
> > >
> > >  	list_add_tail(&inst->list, &dev->instances);
> > >
> > > diff --git
> > > a/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c
> > > b/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c
> > > index b731decbfda6..985de0c293e2 100644
> > > --- a/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c
> > > +++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c
> > > @@ -5,6 +5,7 @@
> > >   * Copyright (C) 2021-2023 CHIPS&MEDIA INC
> > >   */
> > >
> > > +#include <linux/pm_runtime.h>
> > >  #include "wave5-helper.h"
> > >
> > >  #define VPU_ENC_DEV_NAME "C&M Wave5 VPU encoder"
> > > @@ -1310,6 +1311,7 @@ static int wave5_vpu_enc_start_streaming(struct
> vb2_queue *q, unsigned int count
> > >  	struct v4l2_m2m_ctx *m2m_ctx = inst->v4l2_fh.m2m_ctx;
> > >  	int ret = 0;
> > >
> > > +	pm_runtime_resume_and_get(inst->dev->dev);
> > >  	v4l2_m2m_update_start_streaming_state(m2m_ctx, q);
> > >
> > >  	if (inst->state == VPU_INST_STATE_NONE && q->type ==
> > > V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) { @@ -1364,9 +1366,13 @@ static int
> wave5_vpu_enc_start_streaming(struct vb2_queue *q, unsigned int count
> > >  	if (ret)
> > >  		goto return_buffers;
> > >
> > > +	pm_runtime_mark_last_busy(inst->dev->dev);
> > > +	pm_runtime_put_autosuspend(inst->dev->dev);
> > >  	return 0;
> > >  return_buffers:
> > >  	wave5_return_bufs(q, VB2_BUF_STATE_QUEUED);
> > > +	pm_runtime_mark_last_busy(inst->dev->dev);
> > > +	pm_runtime_put_autosuspend(inst->dev->dev);
> > >  	return ret;
> > >  }
> > >
> > > @@ -1408,6 +1414,7 @@ static void wave5_vpu_enc_stop_streaming(struct
> vb2_queue *q)
> > >  	 */
> > >
> > >  	dev_dbg(inst->dev->dev, "%s: type: %u\n", __func__, q->type);
> > > +	pm_runtime_resume_and_get(inst->dev->dev);
> > >
> > >  	if (wave5_vpu_both_queues_are_streaming(inst))
> > >  		switch_state(inst, VPU_INST_STATE_STOP); @@ -1432,6 +1439,9
> @@
> > > static void wave5_vpu_enc_stop_streaming(struct vb2_queue *q)
> > >  		streamoff_output(inst, q);
> > >  	else
> > >  		streamoff_capture(inst, q);
> > > +
> > > +	pm_runtime_mark_last_busy(inst->dev->dev);
> > > +	pm_runtime_put_autosuspend(inst->dev->dev);
> > >  }
> > >
> > >  static const struct vb2_ops wave5_vpu_enc_vb2_ops = { @@ -1478,6
> > > +1488,7 @@ static void wave5_vpu_enc_device_run(void *priv)
> > >  	u32 fail_res = 0;
> > >  	int ret = 0;
> > >
> > > +	pm_runtime_resume_and_get(inst->dev->dev);
> > >  	switch (inst->state) {
> > >  	case VPU_INST_STATE_PIC_RUN:
> > >  		ret = start_encode(inst, &fail_res); @@ -1491,6 +1502,8 @@
> static
> > > void wave5_vpu_enc_device_run(void *priv)
> > >  			break;
> > >  		}
> > >  		dev_dbg(inst->dev->dev, "%s: leave with active job",
> __func__);
> > > +		pm_runtime_mark_last_busy(inst->dev->dev);
> > > +		pm_runtime_put_autosuspend(inst->dev->dev);
> > >  		return;
> > >  	default:
> > >  		WARN(1, "Execution of a job in state %s is invalid.\n", @@
> > > -1498,6 +1511,8 @@ static void wave5_vpu_enc_device_run(void *priv)
> > >  		break;
> > >  	}
> > >  	dev_dbg(inst->dev->dev, "%s: leave and finish job", __func__);
> > > +	pm_runtime_mark_last_busy(inst->dev->dev);
> > > +	pm_runtime_put_autosuspend(inst->dev->dev);
> > >  	v4l2_m2m_job_finish(inst->v4l2_m2m_dev, m2m_ctx);  }
> > >
> > > @@ -1739,9 +1754,8 @@ static int wave5_vpu_open_enc(struct file *filp)
> > >  	if (ret)
> > >  		goto cleanup_inst;
> > >
> > > -	if (dev->irq < 0 && !hrtimer_active(&dev->hrtimer) &&
> list_empty(&dev->instances))
> > > -		hrtimer_start(&dev->hrtimer, ns_to_ktime(dev-
> >vpu_poll_interval * NSEC_PER_MSEC),
> > > -			      HRTIMER_MODE_REL_PINNED);
> > > +	if (list_empty(&dev->instances))
> > > +		pm_runtime_use_autosuspend(inst->dev->dev);
> > >
> > >  	list_add_tail(&inst->list, &dev->instances);
> > >
> > > diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu.c
> > > b/drivers/media/platform/chips-media/wave5/wave5-vpu.c
> > > index 7273254ecb03..41c4bf64f27d 100644
> > > --- a/drivers/media/platform/chips-media/wave5/wave5-vpu.c
> > > +++ b/drivers/media/platform/chips-media/wave5/wave5-vpu.c
> > > @@ -10,6 +10,7 @@
> > >  #include <linux/clk.h>
> > >  #include <linux/firmware.h>
> > >  #include <linux/interrupt.h>
> > > +#include <linux/pm_runtime.h>
> > >  #include <linux/reset.h>
> > >  #include "wave5-vpu.h"
> > >  #include "wave5-regdefine.h"
> > > @@ -153,6 +154,45 @@ static int wave5_vpu_load_firmware(struct device
> *dev, const char *fw_name,
> > >  	return 0;
> > >  }
> > >
> > > +static __maybe_unused int wave5_pm_suspend(struct device *dev) {
> > > +	struct vpu_device *vpu = dev_get_drvdata(dev);
> > > +
> > > +	if (pm_runtime_suspended(dev))
> > > +		return 0;
> > > +
> > > +	if (vpu->irq < 0)
> > > +		hrtimer_cancel(&vpu->hrtimer);
> > > +
> > > +	wave5_vpu_sleep_wake(dev, true, NULL, 0);
> > > +	clk_bulk_disable_unprepare(vpu->num_clks, vpu->clks);
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +static __maybe_unused int wave5_pm_resume(struct device *dev) {
> > > +	struct vpu_device *vpu = dev_get_drvdata(dev);
> > > +	int ret = 0;
> > > +
> > > +	wave5_vpu_sleep_wake(dev, false, NULL, 0);
> > > +	ret = clk_bulk_prepare_enable(vpu->num_clks, vpu->clks);
> > > +	if (ret) {
> > > +		dev_err(dev, "Enabling clocks, fail: %d\n", ret);
> > > +		return ret;
> > > +	}
> > > +
> > > +	if (vpu->irq < 0 && !hrtimer_active(&vpu->hrtimer))
> > > +		hrtimer_start(&vpu->hrtimer, ns_to_ktime(vpu-
> >vpu_poll_interval * NSEC_PER_MSEC),
> > > +				HRTIMER_MODE_REL_PINNED);
> 
> I have fixed locally this style error. Alignment should match open
> parenthesis.
> Checkpatch caught that, it saves times if you run checkpatch before your
> submissions.
> 
> https://gitlab.freedesktop.org/ndufresne/media-staging/-/jobs/63283654
> 


Sorry for that, thanks for your feedback.
Should I make v8 for that ?





> regards,
> Nicolas
> 
> > > +
> > > +	return ret;
> > > +}
> > > +
> > > +static const struct dev_pm_ops wave5_pm_ops = {
> > > +	SET_RUNTIME_PM_OPS(wave5_pm_suspend, wave5_pm_resume, NULL) };
> > > +
> > >  static int wave5_vpu_probe(struct platform_device *pdev)  {
> > >  	int ret;
> > > @@ -281,6 +321,12 @@ static int wave5_vpu_probe(struct platform_device
> *pdev)
> > >  		 (match_data->flags & WAVE5_IS_DEC) ? "'DECODE'" : "");
> > >  	dev_info(&pdev->dev, "Product Code:      0x%x\n", dev->product_code);
> > >  	dev_info(&pdev->dev, "Firmware Revision: %u\n", fw_revision);
> > > +
> > > +	pm_runtime_set_autosuspend_delay(&pdev->dev, 100);
> > > +	pm_runtime_use_autosuspend(&pdev->dev);
> > > +	pm_runtime_enable(&pdev->dev);
> > > +	wave5_vpu_sleep_wake(&pdev->dev, true, NULL, 0);
> > > +
> > >  	return 0;
> > >
> > >  err_enc_unreg:
> > > @@ -310,6 +356,9 @@ static void wave5_vpu_remove(struct
> platform_device *pdev)
> > >  		hrtimer_cancel(&dev->hrtimer);
> > >  	}
> > >
> > > +	pm_runtime_put_sync(&pdev->dev);
> > > +	pm_runtime_disable(&pdev->dev);
> > > +
> > >  	mutex_destroy(&dev->dev_lock);
> > >  	mutex_destroy(&dev->hw_lock);
> > >  	reset_control_assert(dev->resets);
> > > @@ -337,6 +386,7 @@ static struct platform_driver wave5_vpu_driver = {
> > >  	.driver = {
> > >  		.name = VPU_PLATFORM_DEVICE_NAME,
> > >  		.of_match_table = of_match_ptr(wave5_dt_ids),
> > > +		.pm = &wave5_pm_ops,
> > >  		},
> > >  	.probe = wave5_vpu_probe,
> > >  	.remove_new = wave5_vpu_remove,
> > > diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.c
> > > b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.c
> > > index 1a3efb638dde..e16b990041c2 100644
> > > --- a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.c
> > > +++ b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.c
> > > @@ -6,6 +6,8 @@
> > >   */
> > >
> > >  #include <linux/bug.h>
> > > +#include <linux/pm_runtime.h>
> > > +#include <linux/delay.h>
> > >  #include "wave5-vpuapi.h"
> > >  #include "wave5-regdefine.h"
> > >  #include "wave5.h"
> > > @@ -195,14 +197,20 @@ int wave5_vpu_dec_close(struct vpu_instance
> *inst, u32 *fail_res)
> > >  	int retry = 0;
> > >  	struct vpu_device *vpu_dev = inst->dev;
> > >  	int i;
> > > +	int inst_count = 0;
> > > +	struct vpu_instance *inst_elm;
> > >
> > >  	*fail_res = 0;
> > >  	if (!inst->codec_info)
> > >  		return -EINVAL;
> > >
> > > +	pm_runtime_resume_and_get(inst->dev->dev);
> > > +
> > >  	ret = mutex_lock_interruptible(&vpu_dev->hw_lock);
> > > -	if (ret)
> > > +	if (ret) {
> > > +		pm_runtime_put_sync(inst->dev->dev);
> > >  		return ret;
> > > +	}
> > >
> > >  	do {
> > >  		ret = wave5_vpu_dec_finish_seq(inst, fail_res); @@ -232,9
> +240,14
> > > @@ int wave5_vpu_dec_close(struct vpu_instance *inst, u32 *fail_res)
> > >
> > >  	wave5_vdi_free_dma_memory(vpu_dev, &p_dec_info->vb_task);
> > >
> > > +	list_for_each_entry(inst_elm, &vpu_dev->instances, list)
> > > +		inst_count++;
> > > +	if (inst_count == 1)
> > > +		pm_runtime_dont_use_autosuspend(vpu_dev->dev);
> > > +
> > >  unlock_and_return:
> > >  	mutex_unlock(&vpu_dev->hw_lock);
> > > -
> > > +	pm_runtime_put_sync(inst->dev->dev);
> > >  	return ret;
> > >  }
> > >
> > > @@ -697,25 +710,33 @@ int wave5_vpu_enc_close(struct vpu_instance
> *inst, u32 *fail_res)
> > >  	int ret;
> > >  	int retry = 0;
> > >  	struct vpu_device *vpu_dev = inst->dev;
> > > +	int inst_count = 0;
> > > +	struct vpu_instance *inst_elm;
> > >
> > >  	*fail_res = 0;
> > >  	if (!inst->codec_info)
> > >  		return -EINVAL;
> > >
> > > +	pm_runtime_resume_and_get(inst->dev->dev);
> > > +
> > >  	ret = mutex_lock_interruptible(&vpu_dev->hw_lock);
> > > -	if (ret)
> > > +	if (ret) {
> > > +		pm_runtime_resume_and_get(inst->dev->dev);
> > >  		return ret;
> > > +	}
> > >
> > >  	do {
> > >  		ret = wave5_vpu_enc_finish_seq(inst, fail_res);
> > >  		if (ret < 0 && *fail_res != WAVE5_SYSERR_VPU_STILL_RUNNING)
> {
> > >  			dev_warn(inst->dev->dev, "enc_finish_seq timed out\n");
> > > +			pm_runtime_resume_and_get(inst->dev->dev);
> > >  			mutex_unlock(&vpu_dev->hw_lock);
> > >  			return ret;
> > >  		}
> > >
> > >  		if (*fail_res == WAVE5_SYSERR_VPU_STILL_RUNNING &&
> > >  		    retry++ >= MAX_FIRMWARE_CALL_RETRY) {
> > > +			pm_runtime_resume_and_get(inst->dev->dev);
> > >  			mutex_unlock(&vpu_dev->hw_lock);
> > >  			return -ETIMEDOUT;
> > >  		}
> > > @@ -734,7 +755,13 @@ int wave5_vpu_enc_close(struct vpu_instance
> > > *inst, u32 *fail_res)
> > >
> > >  	wave5_vdi_free_dma_memory(vpu_dev, &p_enc_info->vb_task);
> > >
> > > +	list_for_each_entry(inst_elm, &vpu_dev->instances, list)
> > > +		inst_count++;
> > > +	if (inst_count == 1)
> > > +		pm_runtime_dont_use_autosuspend(vpu_dev->dev);
> > > +
> > >  	mutex_unlock(&vpu_dev->hw_lock);
> > > +	pm_runtime_put_sync(inst->dev->dev);
> > >
> > >  	return 0;
> > >  }
> > > diff --git a/drivers/media/platform/chips-media/wave5/wave5.h
> > > b/drivers/media/platform/chips-media/wave5/wave5.h
> > > index 2a29b9164f97..2caab356f3e1 100644
> > > --- a/drivers/media/platform/chips-media/wave5/wave5.h
> > > +++ b/drivers/media/platform/chips-media/wave5/wave5.h
> > > @@ -62,6 +62,9 @@ int wave5_vpu_get_version(struct vpu_device
> > > *vpu_dev, u32 *revision);
> > >
> > >  int wave5_vpu_init(struct device *dev, u8 *fw, size_t size);
> > >
> > > +int wave5_vpu_sleep_wake(struct device *dev, bool i_sleep_wake, const
> uint16_t *code,
> > > +			 size_t size);
> > > +
> > >  int wave5_vpu_reset(struct device *dev, enum sw_reset_mode
> > > reset_mode);
> > >
> > >  int wave5_vpu_build_up_dec_param(struct vpu_instance *inst, struct
> > > dec_open_param *param);
> >

Sebastian Fricke Sept. 9, 2024, 5:46 a.m. UTC | #5

Hey Jackson,

On 09.09.2024 01:21, jackson.lee wrote:
>Hi Nicolas
>
>> -----Original Message-----
>> From: Nicolas Dufresne <[e-mail]>
>> Sent: Saturday, September 7, 2024 4:27 AM
>> To: jackson.lee <[e-mail]>; [e-mail];
>> [e-mail]
>> Cc: [e-mail]; [e-mail];
>> [e-mail]; Nas Chung <[e-mail]>; lafley.kim
>> <[e-mail]>; [e-mail]
>> Subject: Re: [RESEND PATCH v7 2/4] media: chips-media: wave5: Support
>> runtime suspend/resume
>>
>> Hi Again,
>>
>> Le vendredi 06 septembre 2024 à 13:08 -0400, Nicolas Dufresne a écrit :
>> > Hi,
>> >
>> > Le lundi 12 août 2024 à 16:08 +0900, Jackson.lee a écrit :
>> > > Add support for runtime suspend/resume in the encoder and decoder.
>> > > This is achieved by saving the VPU state and powering it off while the
>> VPU idle.
>> >
>> > If you don't, I'll edit to "while the VPU *is* idle".
>> >
>> > regards,
>> > Nicolas
>> >
>> > >
>> > > Signed-off-by: Jackson.lee <[e-mail]>
>> > > Signed-off-by: Nas Chung <[e-mail]>
>> > > Reviewed-by: Nicolas Dufresne <[e-mail]>
>> > > ---
>> > >  .../platform/chips-media/wave5/wave5-helper.c | 13 -----
>> > >  .../platform/chips-media/wave5/wave5-hw.c     |  4 +-
>> > >  .../chips-media/wave5/wave5-vpu-dec.c         | 21 ++++++--
>> > >  .../chips-media/wave5/wave5-vpu-enc.c         | 20 ++++++--
>> > >  .../platform/chips-media/wave5/wave5-vpu.c    | 50 +++++++++++++++++++
>> > >  .../platform/chips-media/wave5/wave5-vpuapi.c | 33 ++++++++++--
>> > > .../media/platform/chips-media/wave5/wave5.h  |  3 ++
>> > >  7 files changed, 118 insertions(+), 26 deletions(-)
>> > >
>> > > diff --git a/drivers/media/platform/chips-media/wave5/wave5-helper.c
>> > > b/drivers/media/platform/chips-media/wave5/wave5-helper.c
>> > > index d60841c54a80..a20d84dbe3aa 100644
>> > > --- a/drivers/media/platform/chips-media/wave5/wave5-helper.c
>> > > +++ b/drivers/media/platform/chips-media/wave5/wave5-helper.c
>> > > @@ -58,7 +58,6 @@ int wave5_vpu_release_device(struct file *filp,
>> > >  			     char *name)
>> > >  {
>> > >  	struct vpu_instance *inst = wave5_to_vpu_inst(filp->private_data);
>> > > -	struct vpu_device *dev = inst->dev;
>> > >  	int ret = 0;
>> > >
>> > >  	v4l2_m2m_ctx_release(inst->v4l2_fh.m2m_ctx);
>> > > @@ -78,18 +77,6 @@ int wave5_vpu_release_device(struct file *filp,
>> > >  	}
>> > >
>> > >  	wave5_cleanup_instance(inst);
>> > > -	if (dev->irq < 0) {
>> > > -		ret = mutex_lock_interruptible(&dev->dev_lock);
>> > > -		if (ret)
>> > > -			return ret;
>> > > -
>> > > -		if (list_empty(&dev->instances)) {
>> > > -			dev_dbg(dev->dev, "Disabling the hrtimer\n");
>> > > -			hrtimer_cancel(&dev->hrtimer);
>> > > -		}
>> > > -
>> > > -		mutex_unlock(&dev->dev_lock);
>> > > -	}
>> > >
>> > >  	return ret;
>> > >  }
>> > > diff --git a/drivers/media/platform/chips-media/wave5/wave5-hw.c
>> > > b/drivers/media/platform/chips-media/wave5/wave5-hw.c
>> > > index 2a98bab446d0..c8a905994109 100644
>> > > --- a/drivers/media/platform/chips-media/wave5/wave5-hw.c
>> > > +++ b/drivers/media/platform/chips-media/wave5/wave5-hw.c
>> > > @@ -1228,8 +1228,8 @@ int wave5_vpu_re_init(struct device *dev, u8 *fw,
>> size_t size)
>> > >  	return setup_wave5_properties(dev);  }
>> > >
>> > > -static int wave5_vpu_sleep_wake(struct device *dev, bool i_sleep_wake,
>> const uint16_t *code,
>> > > -				size_t size)
>> > > +int wave5_vpu_sleep_wake(struct device *dev, bool i_sleep_wake, const
>> uint16_t *code,
>> > > +			 size_t size)
>> > >  {
>> > >  	u32 reg_val;
>> > >  	struct vpu_buf *common_vb;
>> > > diff --git
>> > > a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
>> > > b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
>> > > index 0c5c9a8de91f..698c83e3082e 100644
>> > > --- a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
>> > > +++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
>> > > @@ -5,6 +5,7 @@
>> > >   * Copyright (C) 2021-2023 CHIPS&MEDIA INC
>> > >   */
>> > >
>> > > +#include <linux/pm_runtime.h>
>> > >  #include "wave5-helper.h"
>> > >
>> > >  #define VPU_DEC_DEV_NAME "C&M Wave5 VPU decoder"
>> > > @@ -518,6 +519,8 @@ static void wave5_vpu_dec_finish_decode(struct
>> vpu_instance *inst)
>> > >  	if (q_status.report_queue_count == 0 &&
>> > >  	    (q_status.instance_queue_count == 0 ||
>> dec_info.sequence_changed)) {
>> > >  		dev_dbg(inst->dev->dev, "%s: finishing job.\n", __func__);
>> > > +		pm_runtime_mark_last_busy(inst->dev->dev);
>> > > +		pm_runtime_put_autosuspend(inst->dev->dev);
>> > >  		v4l2_m2m_job_finish(inst->v4l2_m2m_dev, m2m_ctx);
>> > >  	}
>> > >  }
>> > > @@ -1398,6 +1401,7 @@ static int wave5_vpu_dec_start_streaming(struct
>> vb2_queue *q, unsigned int count
>> > >  	int ret = 0;
>> > >
>> > >  	dev_dbg(inst->dev->dev, "%s: type: %u\n", __func__, q->type);
>> > > +	pm_runtime_resume_and_get(inst->dev->dev);
>> > >
>> > >  	v4l2_m2m_update_start_streaming_state(m2m_ctx, q);
>> > >
>> > > @@ -1429,13 +1433,15 @@ static int
>> wave5_vpu_dec_start_streaming(struct vb2_queue *q, unsigned int count
>> > >  		if (ret)
>> > >  			goto return_buffers;
>> > >  	}
>> > > -
>> > > +	pm_runtime_mark_last_busy(inst->dev->dev);
>> > > +	pm_runtime_put_autosuspend(inst->dev->dev);
>> > >  	return ret;
>> > >
>> > >  free_bitstream_vbuf:
>> > >  	wave5_vdi_free_dma_memory(inst->dev, &inst->bitstream_vbuf);
>> > >  return_buffers:
>> > >  	wave5_return_bufs(q, VB2_BUF_STATE_QUEUED);
>> > > +	pm_runtime_put_autosuspend(inst->dev->dev);
>> > >  	return ret;
>> > >  }
>> > >
>> > > @@ -1521,6 +1527,7 @@ static void wave5_vpu_dec_stop_streaming(struct
>> vb2_queue *q)
>> > >  	bool check_cmd = TRUE;
>> > >
>> > >  	dev_dbg(inst->dev->dev, "%s: type: %u\n", __func__, q->type);
>> > > +	pm_runtime_resume_and_get(inst->dev->dev);
>> > >
>> > >  	while (check_cmd) {
>> > >  		struct queue_status_info q_status; @@ -1544,6 +1551,9 @@
>> static
>> > > void wave5_vpu_dec_stop_streaming(struct vb2_queue *q)
>> > >  		streamoff_output(q);
>> > >  	else
>> > >  		streamoff_capture(q);
>> > > +
>> > > +	pm_runtime_mark_last_busy(inst->dev->dev);
>> > > +	pm_runtime_put_autosuspend(inst->dev->dev);
>> > >  }
>> > >
>> > >  static const struct vb2_ops wave5_vpu_dec_vb2_ops = { @@ -1630,7
>> > > +1640,7 @@ static void wave5_vpu_dec_device_run(void *priv)
>> > >  	int ret = 0;
>> > >
>> > >  	dev_dbg(inst->dev->dev, "%s: Fill the ring buffer with new
>> > > bitstream data", __func__);
>> > > -
>> > > +	pm_runtime_resume_and_get(inst->dev->dev);
>> > >  	ret = fill_ringbuffer(inst);
>> > >  	if (ret) {
>> > >  		dev_warn(inst->dev->dev, "Filling ring buffer failed\n"); @@
>> > > -1713,6 +1723,8 @@ static void wave5_vpu_dec_device_run(void *priv)
>> > >
>> > >  finish_job_and_return:
>> > >  	dev_dbg(inst->dev->dev, "%s: leave and finish job", __func__);
>> > > +	pm_runtime_mark_last_busy(inst->dev->dev);
>> > > +	pm_runtime_put_autosuspend(inst->dev->dev);
>> > >  	v4l2_m2m_job_finish(inst->v4l2_m2m_dev, m2m_ctx);  }
>> > >
>> > > @@ -1879,9 +1891,8 @@ static int wave5_vpu_open_dec(struct file *filp)
>> > >  	if (ret)
>> > >  		goto cleanup_inst;
>> > >
>> > > -	if (dev->irq < 0 && !hrtimer_active(&dev->hrtimer) &&
>> list_empty(&dev->instances))
>> > > -		hrtimer_start(&dev->hrtimer, ns_to_ktime(dev-
>> >vpu_poll_interval * NSEC_PER_MSEC),
>> > > -			      HRTIMER_MODE_REL_PINNED);
>> > > +	if (list_empty(&dev->instances))
>> > > +		pm_runtime_use_autosuspend(inst->dev->dev);
>> > >
>> > >  	list_add_tail(&inst->list, &dev->instances);
>> > >
>> > > diff --git
>> > > a/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c
>> > > b/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c
>> > > index b731decbfda6..985de0c293e2 100644
>> > > --- a/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c
>> > > +++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c
>> > > @@ -5,6 +5,7 @@
>> > >   * Copyright (C) 2021-2023 CHIPS&MEDIA INC
>> > >   */
>> > >
>> > > +#include <linux/pm_runtime.h>
>> > >  #include "wave5-helper.h"
>> > >
>> > >  #define VPU_ENC_DEV_NAME "C&M Wave5 VPU encoder"
>> > > @@ -1310,6 +1311,7 @@ static int wave5_vpu_enc_start_streaming(struct
>> vb2_queue *q, unsigned int count
>> > >  	struct v4l2_m2m_ctx *m2m_ctx = inst->v4l2_fh.m2m_ctx;
>> > >  	int ret = 0;
>> > >
>> > > +	pm_runtime_resume_and_get(inst->dev->dev);
>> > >  	v4l2_m2m_update_start_streaming_state(m2m_ctx, q);
>> > >
>> > >  	if (inst->state == VPU_INST_STATE_NONE && q->type ==
>> > > V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) { @@ -1364,9 +1366,13 @@ static int
>> wave5_vpu_enc_start_streaming(struct vb2_queue *q, unsigned int count
>> > >  	if (ret)
>> > >  		goto return_buffers;
>> > >
>> > > +	pm_runtime_mark_last_busy(inst->dev->dev);
>> > > +	pm_runtime_put_autosuspend(inst->dev->dev);
>> > >  	return 0;
>> > >  return_buffers:
>> > >  	wave5_return_bufs(q, VB2_BUF_STATE_QUEUED);
>> > > +	pm_runtime_mark_last_busy(inst->dev->dev);
>> > > +	pm_runtime_put_autosuspend(inst->dev->dev);
>> > >  	return ret;
>> > >  }
>> > >
>> > > @@ -1408,6 +1414,7 @@ static void wave5_vpu_enc_stop_streaming(struct
>> vb2_queue *q)
>> > >  	 */
>> > >
>> > >  	dev_dbg(inst->dev->dev, "%s: type: %u\n", __func__, q->type);
>> > > +	pm_runtime_resume_and_get(inst->dev->dev);
>> > >
>> > >  	if (wave5_vpu_both_queues_are_streaming(inst))
>> > >  		switch_state(inst, VPU_INST_STATE_STOP); @@ -1432,6 +1439,9
>> @@
>> > > static void wave5_vpu_enc_stop_streaming(struct vb2_queue *q)
>> > >  		streamoff_output(inst, q);
>> > >  	else
>> > >  		streamoff_capture(inst, q);
>> > > +
>> > > +	pm_runtime_mark_last_busy(inst->dev->dev);
>> > > +	pm_runtime_put_autosuspend(inst->dev->dev);
>> > >  }
>> > >
>> > >  static const struct vb2_ops wave5_vpu_enc_vb2_ops = { @@ -1478,6
>> > > +1488,7 @@ static void wave5_vpu_enc_device_run(void *priv)
>> > >  	u32 fail_res = 0;
>> > >  	int ret = 0;
>> > >
>> > > +	pm_runtime_resume_and_get(inst->dev->dev);
>> > >  	switch (inst->state) {
>> > >  	case VPU_INST_STATE_PIC_RUN:
>> > >  		ret = start_encode(inst, &fail_res); @@ -1491,6 +1502,8 @@
>> static
>> > > void wave5_vpu_enc_device_run(void *priv)
>> > >  			break;
>> > >  		}
>> > >  		dev_dbg(inst->dev->dev, "%s: leave with active job",
>> __func__);
>> > > +		pm_runtime_mark_last_busy(inst->dev->dev);
>> > > +		pm_runtime_put_autosuspend(inst->dev->dev);
>> > >  		return;
>> > >  	default:
>> > >  		WARN(1, "Execution of a job in state %s is invalid.\n", @@
>> > > -1498,6 +1511,8 @@ static void wave5_vpu_enc_device_run(void *priv)
>> > >  		break;
>> > >  	}
>> > >  	dev_dbg(inst->dev->dev, "%s: leave and finish job", __func__);
>> > > +	pm_runtime_mark_last_busy(inst->dev->dev);
>> > > +	pm_runtime_put_autosuspend(inst->dev->dev);
>> > >  	v4l2_m2m_job_finish(inst->v4l2_m2m_dev, m2m_ctx);  }
>> > >
>> > > @@ -1739,9 +1754,8 @@ static int wave5_vpu_open_enc(struct file *filp)
>> > >  	if (ret)
>> > >  		goto cleanup_inst;
>> > >
>> > > -	if (dev->irq < 0 && !hrtimer_active(&dev->hrtimer) &&
>> list_empty(&dev->instances))
>> > > -		hrtimer_start(&dev->hrtimer, ns_to_ktime(dev-
>> >vpu_poll_interval * NSEC_PER_MSEC),
>> > > -			      HRTIMER_MODE_REL_PINNED);
>> > > +	if (list_empty(&dev->instances))
>> > > +		pm_runtime_use_autosuspend(inst->dev->dev);
>> > >
>> > >  	list_add_tail(&inst->list, &dev->instances);
>> > >
>> > > diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu.c
>> > > b/drivers/media/platform/chips-media/wave5/wave5-vpu.c
>> > > index 7273254ecb03..41c4bf64f27d 100644
>> > > --- a/drivers/media/platform/chips-media/wave5/wave5-vpu.c
>> > > +++ b/drivers/media/platform/chips-media/wave5/wave5-vpu.c
>> > > @@ -10,6 +10,7 @@
>> > >  #include <linux/clk.h>
>> > >  #include <linux/firmware.h>
>> > >  #include <linux/interrupt.h>
>> > > +#include <linux/pm_runtime.h>
>> > >  #include <linux/reset.h>
>> > >  #include "wave5-vpu.h"
>> > >  #include "wave5-regdefine.h"
>> > > @@ -153,6 +154,45 @@ static int wave5_vpu_load_firmware(struct device
>> *dev, const char *fw_name,
>> > >  	return 0;
>> > >  }
>> > >
>> > > +static __maybe_unused int wave5_pm_suspend(struct device *dev) {
>> > > +	struct vpu_device *vpu = dev_get_drvdata(dev);
>> > > +
>> > > +	if (pm_runtime_suspended(dev))
>> > > +		return 0;
>> > > +
>> > > +	if (vpu->irq < 0)
>> > > +		hrtimer_cancel(&vpu->hrtimer);
>> > > +
>> > > +	wave5_vpu_sleep_wake(dev, true, NULL, 0);
>> > > +	clk_bulk_disable_unprepare(vpu->num_clks, vpu->clks);
>> > > +
>> > > +	return 0;
>> > > +}
>> > > +
>> > > +static __maybe_unused int wave5_pm_resume(struct device *dev) {
>> > > +	struct vpu_device *vpu = dev_get_drvdata(dev);
>> > > +	int ret = 0;
>> > > +
>> > > +	wave5_vpu_sleep_wake(dev, false, NULL, 0);
>> > > +	ret = clk_bulk_prepare_enable(vpu->num_clks, vpu->clks);
>> > > +	if (ret) {
>> > > +		dev_err(dev, "Enabling clocks, fail: %d\n", ret);
>> > > +		return ret;
>> > > +	}
>> > > +
>> > > +	if (vpu->irq < 0 && !hrtimer_active(&vpu->hrtimer))
>> > > +		hrtimer_start(&vpu->hrtimer, ns_to_ktime(vpu-
>> >vpu_poll_interval * NSEC_PER_MSEC),
>> > > +				HRTIMER_MODE_REL_PINNED);
>>
>> I have fixed locally this style error. Alignment should match open
>> parenthesis.
>> Checkpatch caught that, it saves times if you run checkpatch before your
>> submissions.
>>
>> https://gitlab.freedesktop.org/ndufresne/media-staging/-/jobs/63283654
>>
>
>
>Sorry for that, thanks for your feedback.
>Should I make v8 for that ?

That is not required in this case, when he says "I have fixed locally",
that means that the required change is applied by the maintainer before
merging the changes upstream.

Regards,
Sebastian

jackson.lee Sept. 9, 2024, 7:55 a.m. UTC | #6

Hi Sebastian

Many thanks

Jackson

> -----Original Message-----
> From: [e-mail] <[e-mail]>
> Sent: Monday, September 9, 2024 2:47 PM
> To: jackson.lee <[e-mail]>
> Cc: Nicolas Dufresne <[e-mail]>; [e-mail]; linux-
> [e-mail]; [e-mail]; [e-mail];
> Nas Chung <[e-mail]>; lafley.kim
> <[e-mail]>; [e-mail]
> Subject: Re: [RESEND PATCH v7 2/4] media: chips-media: wave5: Support
> runtime suspend/resume
> 
> Hey Jackson,
> 
> On 09.09.2024 01:21, jackson.lee wrote:
> >Hi Nicolas
> >
> >> -----Original Message-----
> >> From: Nicolas Dufresne <[e-mail]>
> >> Sent: Saturday, September 7, 2024 4:27 AM
> >> To: jackson.lee <[e-mail]>; [e-mail];
> >> [e-mail]
> >> Cc: [e-mail]; [e-mail];
> >> [e-mail]; Nas Chung <[e-mail]>; lafley.kim
> >> <[e-mail]>; [e-mail]
> >> Subject: Re: [RESEND PATCH v7 2/4] media: chips-media: wave5: Support
> >> runtime suspend/resume
> >>
> >> Hi Again,
> >>
> >> Le vendredi 06 septembre 2024 à 13:08 -0400, Nicolas Dufresne a écrit :
> >> > Hi,
> >> >
> >> > Le lundi 12 août 2024 à 16:08 +0900, Jackson.lee a écrit :
> >> > > Add support for runtime suspend/resume in the encoder and decoder.
> >> > > This is achieved by saving the VPU state and powering it off
> >> > > while the
> >> VPU idle.
> >> >
> >> > If you don't, I'll edit to "while the VPU *is* idle".
> >> >
> >> > regards,
> >> > Nicolas
> >> >
> >> > >
> >> > > Signed-off-by: Jackson.lee <[e-mail]>
> >> > > Signed-off-by: Nas Chung <[e-mail]>
> >> > > Reviewed-by: Nicolas Dufresne <[e-mail]>
> >> > > ---
> >> > >  .../platform/chips-media/wave5/wave5-helper.c | 13 -----
> >> > >  .../platform/chips-media/wave5/wave5-hw.c     |  4 +-
> >> > >  .../chips-media/wave5/wave5-vpu-dec.c         | 21 ++++++--
> >> > >  .../chips-media/wave5/wave5-vpu-enc.c         | 20 ++++++--
> >> > >  .../platform/chips-media/wave5/wave5-vpu.c    | 50
> +++++++++++++++++++
> >> > >  .../platform/chips-media/wave5/wave5-vpuapi.c | 33 ++++++++++--
> >> > > .../media/platform/chips-media/wave5/wave5.h  |  3 ++
> >> > >  7 files changed, 118 insertions(+), 26 deletions(-)
> >> > >
> >> > > diff --git
> >> > > a/drivers/media/platform/chips-media/wave5/wave5-helper.c
> >> > > b/drivers/media/platform/chips-media/wave5/wave5-helper.c
> >> > > index d60841c54a80..a20d84dbe3aa 100644
> >> > > --- a/drivers/media/platform/chips-media/wave5/wave5-helper.c
> >> > > +++ b/drivers/media/platform/chips-media/wave5/wave5-helper.c
> >> > > @@ -58,7 +58,6 @@ int wave5_vpu_release_device(struct file *filp,
> >> > >  			     char *name)
> >> > >  {
> >> > >  	struct vpu_instance *inst = wave5_to_vpu_inst(filp-
> >private_data);
> >> > > -	struct vpu_device *dev = inst->dev;
> >> > >  	int ret = 0;
> >> > >
> >> > >  	v4l2_m2m_ctx_release(inst->v4l2_fh.m2m_ctx);
> >> > > @@ -78,18 +77,6 @@ int wave5_vpu_release_device(struct file *filp,
> >> > >  	}
> >> > >
> >> > >  	wave5_cleanup_instance(inst);
> >> > > -	if (dev->irq < 0) {
> >> > > -		ret = mutex_lock_interruptible(&dev->dev_lock);
> >> > > -		if (ret)
> >> > > -			return ret;
> >> > > -
> >> > > -		if (list_empty(&dev->instances)) {
> >> > > -			dev_dbg(dev->dev, "Disabling the hrtimer\n");
> >> > > -			hrtimer_cancel(&dev->hrtimer);
> >> > > -		}
> >> > > -
> >> > > -		mutex_unlock(&dev->dev_lock);
> >> > > -	}
> >> > >
> >> > >  	return ret;
> >> > >  }
> >> > > diff --git a/drivers/media/platform/chips-media/wave5/wave5-hw.c
> >> > > b/drivers/media/platform/chips-media/wave5/wave5-hw.c
> >> > > index 2a98bab446d0..c8a905994109 100644
> >> > > --- a/drivers/media/platform/chips-media/wave5/wave5-hw.c
> >> > > +++ b/drivers/media/platform/chips-media/wave5/wave5-hw.c
> >> > > @@ -1228,8 +1228,8 @@ int wave5_vpu_re_init(struct device *dev,
> >> > > u8 *fw,
> >> size_t size)
> >> > >  	return setup_wave5_properties(dev);  }
> >> > >
> >> > > -static int wave5_vpu_sleep_wake(struct device *dev, bool
> >> > > i_sleep_wake,
> >> const uint16_t *code,
> >> > > -				size_t size)
> >> > > +int wave5_vpu_sleep_wake(struct device *dev, bool i_sleep_wake,
> >> > > +const
> >> uint16_t *code,
> >> > > +			 size_t size)
> >> > >  {
> >> > >  	u32 reg_val;
> >> > >  	struct vpu_buf *common_vb;
> >> > > diff --git
> >> > > a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
> >> > > b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
> >> > > index 0c5c9a8de91f..698c83e3082e 100644
> >> > > --- a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
> >> > > +++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
> >> > > @@ -5,6 +5,7 @@
> >> > >   * Copyright (C) 2021-2023 CHIPS&MEDIA INC
> >> > >   */
> >> > >
> >> > > +#include <linux/pm_runtime.h>
> >> > >  #include "wave5-helper.h"
> >> > >
> >> > >  #define VPU_DEC_DEV_NAME "C&M Wave5 VPU decoder"
> >> > > @@ -518,6 +519,8 @@ static void
> >> > > wave5_vpu_dec_finish_decode(struct
> >> vpu_instance *inst)
> >> > >  	if (q_status.report_queue_count == 0 &&
> >> > >  	    (q_status.instance_queue_count == 0 ||
> >> dec_info.sequence_changed)) {
> >> > >  		dev_dbg(inst->dev->dev, "%s: finishing job.\n",
> __func__);
> >> > > +		pm_runtime_mark_last_busy(inst->dev->dev);
> >> > > +		pm_runtime_put_autosuspend(inst->dev->dev);
> >> > >  		v4l2_m2m_job_finish(inst->v4l2_m2m_dev, m2m_ctx);
> >> > >  	}
> >> > >  }
> >> > > @@ -1398,6 +1401,7 @@ static int
> >> > > wave5_vpu_dec_start_streaming(struct
> >> vb2_queue *q, unsigned int count
> >> > >  	int ret = 0;
> >> > >
> >> > >  	dev_dbg(inst->dev->dev, "%s: type: %u\n", __func__, q->type);
> >> > > +	pm_runtime_resume_and_get(inst->dev->dev);
> >> > >
> >> > >  	v4l2_m2m_update_start_streaming_state(m2m_ctx, q);
> >> > >
> >> > > @@ -1429,13 +1433,15 @@ static int
> >> wave5_vpu_dec_start_streaming(struct vb2_queue *q, unsigned int count
> >> > >  		if (ret)
> >> > >  			goto return_buffers;
> >> > >  	}
> >> > > -
> >> > > +	pm_runtime_mark_last_busy(inst->dev->dev);
> >> > > +	pm_runtime_put_autosuspend(inst->dev->dev);
> >> > >  	return ret;
> >> > >
> >> > >  free_bitstream_vbuf:
> >> > >  	wave5_vdi_free_dma_memory(inst->dev, &inst->bitstream_vbuf);
> >> > >  return_buffers:
> >> > >  	wave5_return_bufs(q, VB2_BUF_STATE_QUEUED);
> >> > > +	pm_runtime_put_autosuspend(inst->dev->dev);
> >> > >  	return ret;
> >> > >  }
> >> > >
> >> > > @@ -1521,6 +1527,7 @@ static void
> >> > > wave5_vpu_dec_stop_streaming(struct
> >> vb2_queue *q)
> >> > >  	bool check_cmd = TRUE;
> >> > >
> >> > >  	dev_dbg(inst->dev->dev, "%s: type: %u\n", __func__, q->type);
> >> > > +	pm_runtime_resume_and_get(inst->dev->dev);
> >> > >
> >> > >  	while (check_cmd) {
> >> > >  		struct queue_status_info q_status; @@ -1544,6 +1551,9
> @@
> >> static
> >> > > void wave5_vpu_dec_stop_streaming(struct vb2_queue *q)
> >> > >  		streamoff_output(q);
> >> > >  	else
> >> > >  		streamoff_capture(q);
> >> > > +
> >> > > +	pm_runtime_mark_last_busy(inst->dev->dev);
> >> > > +	pm_runtime_put_autosuspend(inst->dev->dev);
> >> > >  }
> >> > >
> >> > >  static const struct vb2_ops wave5_vpu_dec_vb2_ops = { @@ -1630,7
> >> > > +1640,7 @@ static void wave5_vpu_dec_device_run(void *priv)
> >> > >  	int ret = 0;
> >> > >
> >> > >  	dev_dbg(inst->dev->dev, "%s: Fill the ring buffer with new
> >> > > bitstream data", __func__);
> >> > > -
> >> > > +	pm_runtime_resume_and_get(inst->dev->dev);
> >> > >  	ret = fill_ringbuffer(inst);
> >> > >  	if (ret) {
> >> > >  		dev_warn(inst->dev->dev, "Filling ring buffer
> failed\n"); @@
> >> > > -1713,6 +1723,8 @@ static void wave5_vpu_dec_device_run(void
> >> > > *priv)
> >> > >
> >> > >  finish_job_and_return:
> >> > >  	dev_dbg(inst->dev->dev, "%s: leave and finish job",
> __func__);
> >> > > +	pm_runtime_mark_last_busy(inst->dev->dev);
> >> > > +	pm_runtime_put_autosuspend(inst->dev->dev);
> >> > >  	v4l2_m2m_job_finish(inst->v4l2_m2m_dev, m2m_ctx);  }
> >> > >
> >> > > @@ -1879,9 +1891,8 @@ static int wave5_vpu_open_dec(struct file
> *filp)
> >> > >  	if (ret)
> >> > >  		goto cleanup_inst;
> >> > >
> >> > > -	if (dev->irq < 0 && !hrtimer_active(&dev->hrtimer) &&
> >> list_empty(&dev->instances))
> >> > > -		hrtimer_start(&dev->hrtimer, ns_to_ktime(dev-
> >> >vpu_poll_interval * NSEC_PER_MSEC),
> >> > > -			      HRTIMER_MODE_REL_PINNED);
> >> > > +	if (list_empty(&dev->instances))
> >> > > +		pm_runtime_use_autosuspend(inst->dev->dev);
> >> > >
> >> > >  	list_add_tail(&inst->list, &dev->instances);
> >> > >
> >> > > diff --git
> >> > > a/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c
> >> > > b/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c
> >> > > index b731decbfda6..985de0c293e2 100644
> >> > > --- a/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c
> >> > > +++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c
> >> > > @@ -5,6 +5,7 @@
> >> > >   * Copyright (C) 2021-2023 CHIPS&MEDIA INC
> >> > >   */
> >> > >
> >> > > +#include <linux/pm_runtime.h>
> >> > >  #include "wave5-helper.h"
> >> > >
> >> > >  #define VPU_ENC_DEV_NAME "C&M Wave5 VPU encoder"
> >> > > @@ -1310,6 +1311,7 @@ static int
> >> > > wave5_vpu_enc_start_streaming(struct
> >> vb2_queue *q, unsigned int count
> >> > >  	struct v4l2_m2m_ctx *m2m_ctx = inst->v4l2_fh.m2m_ctx;
> >> > >  	int ret = 0;
> >> > >
> >> > > +	pm_runtime_resume_and_get(inst->dev->dev);
> >> > >  	v4l2_m2m_update_start_streaming_state(m2m_ctx, q);
> >> > >
> >> > >  	if (inst->state == VPU_INST_STATE_NONE && q->type ==
> >> > > V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) { @@ -1364,9 +1366,13 @@
> >> > > static int
> >> wave5_vpu_enc_start_streaming(struct vb2_queue *q, unsigned int count
> >> > >  	if (ret)
> >> > >  		goto return_buffers;
> >> > >
> >> > > +	pm_runtime_mark_last_busy(inst->dev->dev);
> >> > > +	pm_runtime_put_autosuspend(inst->dev->dev);
> >> > >  	return 0;
> >> > >  return_buffers:
> >> > >  	wave5_return_bufs(q, VB2_BUF_STATE_QUEUED);
> >> > > +	pm_runtime_mark_last_busy(inst->dev->dev);
> >> > > +	pm_runtime_put_autosuspend(inst->dev->dev);
> >> > >  	return ret;
> >> > >  }
> >> > >
> >> > > @@ -1408,6 +1414,7 @@ static void
> >> > > wave5_vpu_enc_stop_streaming(struct
> >> vb2_queue *q)
> >> > >  	 */
> >> > >
> >> > >  	dev_dbg(inst->dev->dev, "%s: type: %u\n", __func__, q->type);
> >> > > +	pm_runtime_resume_and_get(inst->dev->dev);
> >> > >
> >> > >  	if (wave5_vpu_both_queues_are_streaming(inst))
> >> > >  		switch_state(inst, VPU_INST_STATE_STOP); @@ -1432,6
> +1439,9
> >> @@
> >> > > static void wave5_vpu_enc_stop_streaming(struct vb2_queue *q)
> >> > >  		streamoff_output(inst, q);
> >> > >  	else
> >> > >  		streamoff_capture(inst, q);
> >> > > +
> >> > > +	pm_runtime_mark_last_busy(inst->dev->dev);
> >> > > +	pm_runtime_put_autosuspend(inst->dev->dev);
> >> > >  }
> >> > >
> >> > >  static const struct vb2_ops wave5_vpu_enc_vb2_ops = { @@ -1478,6
> >> > > +1488,7 @@ static void wave5_vpu_enc_device_run(void *priv)
> >> > >  	u32 fail_res = 0;
> >> > >  	int ret = 0;
> >> > >
> >> > > +	pm_runtime_resume_and_get(inst->dev->dev);
> >> > >  	switch (inst->state) {
> >> > >  	case VPU_INST_STATE_PIC_RUN:
> >> > >  		ret = start_encode(inst, &fail_res); @@ -1491,6
> +1502,8 @@
> >> static
> >> > > void wave5_vpu_enc_device_run(void *priv)
> >> > >  			break;
> >> > >  		}
> >> > >  		dev_dbg(inst->dev->dev, "%s: leave with active job",
> >> __func__);
> >> > > +		pm_runtime_mark_last_busy(inst->dev->dev);
> >> > > +		pm_runtime_put_autosuspend(inst->dev->dev);
> >> > >  		return;
> >> > >  	default:
> >> > >  		WARN(1, "Execution of a job in state %s is invalid.\n",
> @@
> >> > > -1498,6 +1511,8 @@ static void wave5_vpu_enc_device_run(void *priv)
> >> > >  		break;
> >> > >  	}
> >> > >  	dev_dbg(inst->dev->dev, "%s: leave and finish job",
> __func__);
> >> > > +	pm_runtime_mark_last_busy(inst->dev->dev);
> >> > > +	pm_runtime_put_autosuspend(inst->dev->dev);
> >> > >  	v4l2_m2m_job_finish(inst->v4l2_m2m_dev, m2m_ctx);  }
> >> > >
> >> > > @@ -1739,9 +1754,8 @@ static int wave5_vpu_open_enc(struct file
> *filp)
> >> > >  	if (ret)
> >> > >  		goto cleanup_inst;
> >> > >
> >> > > -	if (dev->irq < 0 && !hrtimer_active(&dev->hrtimer) &&
> >> list_empty(&dev->instances))
> >> > > -		hrtimer_start(&dev->hrtimer, ns_to_ktime(dev-
> >> >vpu_poll_interval * NSEC_PER_MSEC),
> >> > > -			      HRTIMER_MODE_REL_PINNED);
> >> > > +	if (list_empty(&dev->instances))
> >> > > +		pm_runtime_use_autosuspend(inst->dev->dev);
> >> > >
> >> > >  	list_add_tail(&inst->list, &dev->instances);
> >> > >
> >> > > diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu.c
> >> > > b/drivers/media/platform/chips-media/wave5/wave5-vpu.c
> >> > > index 7273254ecb03..41c4bf64f27d 100644
> >> > > --- a/drivers/media/platform/chips-media/wave5/wave5-vpu.c
> >> > > +++ b/drivers/media/platform/chips-media/wave5/wave5-vpu.c
> >> > > @@ -10,6 +10,7 @@
> >> > >  #include <linux/clk.h>
> >> > >  #include <linux/firmware.h>
> >> > >  #include <linux/interrupt.h>
> >> > > +#include <linux/pm_runtime.h>
> >> > >  #include <linux/reset.h>
> >> > >  #include "wave5-vpu.h"
> >> > >  #include "wave5-regdefine.h"
> >> > > @@ -153,6 +154,45 @@ static int wave5_vpu_load_firmware(struct
> >> > > device
> >> *dev, const char *fw_name,
> >> > >  	return 0;
> >> > >  }
> >> > >
> >> > > +static __maybe_unused int wave5_pm_suspend(struct device *dev) {
> >> > > +	struct vpu_device *vpu = dev_get_drvdata(dev);
> >> > > +
> >> > > +	if (pm_runtime_suspended(dev))
> >> > > +		return 0;
> >> > > +
> >> > > +	if (vpu->irq < 0)
> >> > > +		hrtimer_cancel(&vpu->hrtimer);
> >> > > +
> >> > > +	wave5_vpu_sleep_wake(dev, true, NULL, 0);
> >> > > +	clk_bulk_disable_unprepare(vpu->num_clks, vpu->clks);
> >> > > +
> >> > > +	return 0;
> >> > > +}
> >> > > +
> >> > > +static __maybe_unused int wave5_pm_resume(struct device *dev) {
> >> > > +	struct vpu_device *vpu = dev_get_drvdata(dev);
> >> > > +	int ret = 0;
> >> > > +
> >> > > +	wave5_vpu_sleep_wake(dev, false, NULL, 0);
> >> > > +	ret = clk_bulk_prepare_enable(vpu->num_clks, vpu->clks);
> >> > > +	if (ret) {
> >> > > +		dev_err(dev, "Enabling clocks, fail: %d\n", ret);
> >> > > +		return ret;
> >> > > +	}
> >> > > +
> >> > > +	if (vpu->irq < 0 && !hrtimer_active(&vpu->hrtimer))
> >> > > +		hrtimer_start(&vpu->hrtimer, ns_to_ktime(vpu-
> >> >vpu_poll_interval * NSEC_PER_MSEC),
> >> > > +				HRTIMER_MODE_REL_PINNED);
> >>
> >> I have fixed locally this style error. Alignment should match open
> >> parenthesis.
> >> Checkpatch caught that, it saves times if you run checkpatch before
> >> your submissions.
> >>
> >> https://gitlab.freedesktop.org/ndufresne/media-staging/-/jobs/6328365
> >> 4
> >>
> >
> >
> >Sorry for that, thanks for your feedback.
> >Should I make v8 for that ?
> 
> That is not required in this case, when he says "I have fixed locally",
> that means that the required change is applied by the maintainer before
> merging the changes upstream.
> 
> Regards,
> Sebastian

jackson.lee Sept. 25, 2024, 1:22 a.m. UTC | #7

Hi Sebastian and Nicolas

Can you please pull our patch?

Thanks
Jackson

> -----Original Message-----
> From: jackson.lee
> Sent: Monday, September 9, 2024 4:56 PM
> To: [e-mail]
> Cc: Nicolas Dufresne <[e-mail]>; [e-mail]; linux-
> [e-mail]; [e-mail]; [e-mail];
> Nas Chung <[e-mail]>; lafley.kim
> <[e-mail]>; [e-mail]
> Subject: RE: [RESEND PATCH v7 2/4] media: chips-media: wave5: Support
> runtime suspend/resume
> 
> Hi Sebastian
> 
> Many thanks
> 
> Jackson
> 
> > -----Original Message-----
> > From: [e-mail] <[e-mail]>
> > Sent: Monday, September 9, 2024 2:47 PM
> > To: jackson.lee <[e-mail]>
> > Cc: Nicolas Dufresne <[e-mail]>; [e-mail];
> > linux- [e-mail]; [e-mail];
> > [e-mail]; Nas Chung <[e-mail]>; lafley.kim
> > <[e-mail]>; [e-mail]
> > Subject: Re: [RESEND PATCH v7 2/4] media: chips-media: wave5: Support
> > runtime suspend/resume
> >
> > Hey Jackson,
> >
> > On 09.09.2024 01:21, jackson.lee wrote:
> > >Hi Nicolas
> > >
> > >> -----Original Message-----
> > >> From: Nicolas Dufresne <[e-mail]>
> > >> Sent: Saturday, September 7, 2024 4:27 AM
> > >> To: jackson.lee <[e-mail]>; [e-mail];
> > >> [e-mail]
> > >> Cc: [e-mail]; [e-mail];
> > >> [e-mail]; Nas Chung <[e-mail]>;
> > >> lafley.kim <[e-mail]>; [e-mail]
> > >> Subject: Re: [RESEND PATCH v7 2/4] media: chips-media: wave5:
> > >> Support runtime suspend/resume
> > >>
> > >> Hi Again,
> > >>
> > >> Le vendredi 06 septembre 2024 à 13:08 -0400, Nicolas Dufresne a
> écrit :
> > >> > Hi,
> > >> >
> > >> > Le lundi 12 août 2024 à 16:08 +0900, Jackson.lee a écrit :
> > >> > > Add support for runtime suspend/resume in the encoder and decoder.
> > >> > > This is achieved by saving the VPU state and powering it off
> > >> > > while the
> > >> VPU idle.
> > >> >
> > >> > If you don't, I'll edit to "while the VPU *is* idle".
> > >> >
> > >> > regards,
> > >> > Nicolas
> > >> >
> > >> > >
> > >> > > Signed-off-by: Jackson.lee <[e-mail]>
> > >> > > Signed-off-by: Nas Chung <[e-mail]>
> > >> > > Reviewed-by: Nicolas Dufresne <[e-mail]>
> > >> > > ---
> > >> > >  .../platform/chips-media/wave5/wave5-helper.c | 13 -----
> > >> > >  .../platform/chips-media/wave5/wave5-hw.c     |  4 +-
> > >> > >  .../chips-media/wave5/wave5-vpu-dec.c         | 21 ++++++--
> > >> > >  .../chips-media/wave5/wave5-vpu-enc.c         | 20 ++++++--
> > >> > >  .../platform/chips-media/wave5/wave5-vpu.c    | 50
> > +++++++++++++++++++
> > >> > >  .../platform/chips-media/wave5/wave5-vpuapi.c | 33
> > >> > > ++++++++++-- .../media/platform/chips-media/wave5/wave5.h  |  3
> > >> > > ++
> > >> > >  7 files changed, 118 insertions(+), 26 deletions(-)
> > >> > >
> > >> > > diff --git
> > >> > > a/drivers/media/platform/chips-media/wave5/wave5-helper.c
> > >> > > b/drivers/media/platform/chips-media/wave5/wave5-helper.c
> > >> > > index d60841c54a80..a20d84dbe3aa 100644
> > >> > > --- a/drivers/media/platform/chips-media/wave5/wave5-helper.c
> > >> > > +++ b/drivers/media/platform/chips-media/wave5/wave5-helper.c
> > >> > > @@ -58,7 +58,6 @@ int wave5_vpu_release_device(struct file *filp,
> > >> > >  			     char *name)
> > >> > >  {
> > >> > >  	struct vpu_instance *inst = wave5_to_vpu_inst(filp-
> > >private_data);
> > >> > > -	struct vpu_device *dev = inst->dev;
> > >> > >  	int ret = 0;
> > >> > >
> > >> > >  	v4l2_m2m_ctx_release(inst->v4l2_fh.m2m_ctx);
> > >> > > @@ -78,18 +77,6 @@ int wave5_vpu_release_device(struct file *filp,
> > >> > >  	}
> > >> > >
> > >> > >  	wave5_cleanup_instance(inst);
> > >> > > -	if (dev->irq < 0) {
> > >> > > -		ret = mutex_lock_interruptible(&dev->dev_lock);
> > >> > > -		if (ret)
> > >> > > -			return ret;
> > >> > > -
> > >> > > -		if (list_empty(&dev->instances)) {
> > >> > > -			dev_dbg(dev->dev, "Disabling the hrtimer\n");
> > >> > > -			hrtimer_cancel(&dev->hrtimer);
> > >> > > -		}
> > >> > > -
> > >> > > -		mutex_unlock(&dev->dev_lock);
> > >> > > -	}
> > >> > >
> > >> > >  	return ret;
> > >> > >  }
> > >> > > diff --git
> > >> > > a/drivers/media/platform/chips-media/wave5/wave5-hw.c
> > >> > > b/drivers/media/platform/chips-media/wave5/wave5-hw.c
> > >> > > index 2a98bab446d0..c8a905994109 100644
> > >> > > --- a/drivers/media/platform/chips-media/wave5/wave5-hw.c
> > >> > > +++ b/drivers/media/platform/chips-media/wave5/wave5-hw.c
> > >> > > @@ -1228,8 +1228,8 @@ int wave5_vpu_re_init(struct device *dev,
> > >> > > u8 *fw,
> > >> size_t size)
> > >> > >  	return setup_wave5_properties(dev);  }
> > >> > >
> > >> > > -static int wave5_vpu_sleep_wake(struct device *dev, bool
> > >> > > i_sleep_wake,
> > >> const uint16_t *code,
> > >> > > -				size_t size)
> > >> > > +int wave5_vpu_sleep_wake(struct device *dev, bool
> > >> > > +i_sleep_wake, const
> > >> uint16_t *code,
> > >> > > +			 size_t size)
> > >> > >  {
> > >> > >  	u32 reg_val;
> > >> > >  	struct vpu_buf *common_vb;
> > >> > > diff --git
> > >> > > a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
> > >> > > b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
> > >> > > index 0c5c9a8de91f..698c83e3082e 100644
> > >> > > --- a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
> > >> > > +++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
> > >> > > @@ -5,6 +5,7 @@
> > >> > >   * Copyright (C) 2021-2023 CHIPS&MEDIA INC
> > >> > >   */
> > >> > >
> > >> > > +#include <linux/pm_runtime.h>
> > >> > >  #include "wave5-helper.h"
> > >> > >
> > >> > >  #define VPU_DEC_DEV_NAME "C&M Wave5 VPU decoder"
> > >> > > @@ -518,6 +519,8 @@ static void
> > >> > > wave5_vpu_dec_finish_decode(struct
> > >> vpu_instance *inst)
> > >> > >  	if (q_status.report_queue_count == 0 &&
> > >> > >  	    (q_status.instance_queue_count == 0 ||
> > >> dec_info.sequence_changed)) {
> > >> > >  		dev_dbg(inst->dev->dev, "%s: finishing job.\n",
> > __func__);
> > >> > > +		pm_runtime_mark_last_busy(inst->dev->dev);
> > >> > > +		pm_runtime_put_autosuspend(inst->dev->dev);
> > >> > >  		v4l2_m2m_job_finish(inst->v4l2_m2m_dev, m2m_ctx);
> > >> > >  	}
> > >> > >  }
> > >> > > @@ -1398,6 +1401,7 @@ static int
> > >> > > wave5_vpu_dec_start_streaming(struct
> > >> vb2_queue *q, unsigned int count
> > >> > >  	int ret = 0;
> > >> > >
> > >> > >  	dev_dbg(inst->dev->dev, "%s: type: %u\n", __func__, q->type);
> > >> > > +	pm_runtime_resume_and_get(inst->dev->dev);
> > >> > >
> > >> > >  	v4l2_m2m_update_start_streaming_state(m2m_ctx, q);
> > >> > >
> > >> > > @@ -1429,13 +1433,15 @@ static int
> > >> wave5_vpu_dec_start_streaming(struct vb2_queue *q, unsigned int
> > >> count
> > >> > >  		if (ret)
> > >> > >  			goto return_buffers;
> > >> > >  	}
> > >> > > -
> > >> > > +	pm_runtime_mark_last_busy(inst->dev->dev);
> > >> > > +	pm_runtime_put_autosuspend(inst->dev->dev);
> > >> > >  	return ret;
> > >> > >
> > >> > >  free_bitstream_vbuf:
> > >> > >  	wave5_vdi_free_dma_memory(inst->dev, &inst->bitstream_vbuf);
> > >> > >  return_buffers:
> > >> > >  	wave5_return_bufs(q, VB2_BUF_STATE_QUEUED);
> > >> > > +	pm_runtime_put_autosuspend(inst->dev->dev);
> > >> > >  	return ret;
> > >> > >  }
> > >> > >
> > >> > > @@ -1521,6 +1527,7 @@ static void
> > >> > > wave5_vpu_dec_stop_streaming(struct
> > >> vb2_queue *q)
> > >> > >  	bool check_cmd = TRUE;
> > >> > >
> > >> > >  	dev_dbg(inst->dev->dev, "%s: type: %u\n", __func__, q->type);
> > >> > > +	pm_runtime_resume_and_get(inst->dev->dev);
> > >> > >
> > >> > >  	while (check_cmd) {
> > >> > >  		struct queue_status_info q_status; @@ -1544,6 +1551,9
> > @@
> > >> static
> > >> > > void wave5_vpu_dec_stop_streaming(struct vb2_queue *q)
> > >> > >  		streamoff_output(q);
> > >> > >  	else
> > >> > >  		streamoff_capture(q);
> > >> > > +
> > >> > > +	pm_runtime_mark_last_busy(inst->dev->dev);
> > >> > > +	pm_runtime_put_autosuspend(inst->dev->dev);
> > >> > >  }
> > >> > >
> > >> > >  static const struct vb2_ops wave5_vpu_dec_vb2_ops = { @@
> > >> > > -1630,7
> > >> > > +1640,7 @@ static void wave5_vpu_dec_device_run(void *priv)
> > >> > >  	int ret = 0;
> > >> > >
> > >> > >  	dev_dbg(inst->dev->dev, "%s: Fill the ring buffer with new
> > >> > > bitstream data", __func__);
> > >> > > -
> > >> > > +	pm_runtime_resume_and_get(inst->dev->dev);
> > >> > >  	ret = fill_ringbuffer(inst);
> > >> > >  	if (ret) {
> > >> > >  		dev_warn(inst->dev->dev, "Filling ring buffer
> > failed\n"); @@
> > >> > > -1713,6 +1723,8 @@ static void wave5_vpu_dec_device_run(void
> > >> > > *priv)
> > >> > >
> > >> > >  finish_job_and_return:
> > >> > >  	dev_dbg(inst->dev->dev, "%s: leave and finish job",
> > __func__);
> > >> > > +	pm_runtime_mark_last_busy(inst->dev->dev);
> > >> > > +	pm_runtime_put_autosuspend(inst->dev->dev);
> > >> > >  	v4l2_m2m_job_finish(inst->v4l2_m2m_dev, m2m_ctx);  }
> > >> > >
> > >> > > @@ -1879,9 +1891,8 @@ static int wave5_vpu_open_dec(struct file
> > *filp)
> > >> > >  	if (ret)
> > >> > >  		goto cleanup_inst;
> > >> > >
> > >> > > -	if (dev->irq < 0 && !hrtimer_active(&dev->hrtimer) &&
> > >> list_empty(&dev->instances))
> > >> > > -		hrtimer_start(&dev->hrtimer, ns_to_ktime(dev-
> > >> >vpu_poll_interval * NSEC_PER_MSEC),
> > >> > > -			      HRTIMER_MODE_REL_PINNED);
> > >> > > +	if (list_empty(&dev->instances))
> > >> > > +		pm_runtime_use_autosuspend(inst->dev->dev);
> > >> > >
> > >> > >  	list_add_tail(&inst->list, &dev->instances);
> > >> > >
> > >> > > diff --git
> > >> > > a/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c
> > >> > > b/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c
> > >> > > index b731decbfda6..985de0c293e2 100644
> > >> > > --- a/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c
> > >> > > +++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c
> > >> > > @@ -5,6 +5,7 @@
> > >> > >   * Copyright (C) 2021-2023 CHIPS&MEDIA INC
> > >> > >   */
> > >> > >
> > >> > > +#include <linux/pm_runtime.h>
> > >> > >  #include "wave5-helper.h"
> > >> > >
> > >> > >  #define VPU_ENC_DEV_NAME "C&M Wave5 VPU encoder"
> > >> > > @@ -1310,6 +1311,7 @@ static int
> > >> > > wave5_vpu_enc_start_streaming(struct
> > >> vb2_queue *q, unsigned int count
> > >> > >  	struct v4l2_m2m_ctx *m2m_ctx = inst->v4l2_fh.m2m_ctx;
> > >> > >  	int ret = 0;
> > >> > >
> > >> > > +	pm_runtime_resume_and_get(inst->dev->dev);
> > >> > >  	v4l2_m2m_update_start_streaming_state(m2m_ctx, q);
> > >> > >
> > >> > >  	if (inst->state == VPU_INST_STATE_NONE && q->type ==
> > >> > > V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) { @@ -1364,9 +1366,13 @@
> > >> > > static int
> > >> wave5_vpu_enc_start_streaming(struct vb2_queue *q, unsigned int
> > >> count
> > >> > >  	if (ret)
> > >> > >  		goto return_buffers;
> > >> > >
> > >> > > +	pm_runtime_mark_last_busy(inst->dev->dev);
> > >> > > +	pm_runtime_put_autosuspend(inst->dev->dev);
> > >> > >  	return 0;
> > >> > >  return_buffers:
> > >> > >  	wave5_return_bufs(q, VB2_BUF_STATE_QUEUED);
> > >> > > +	pm_runtime_mark_last_busy(inst->dev->dev);
> > >> > > +	pm_runtime_put_autosuspend(inst->dev->dev);
> > >> > >  	return ret;
> > >> > >  }
> > >> > >
> > >> > > @@ -1408,6 +1414,7 @@ static void
> > >> > > wave5_vpu_enc_stop_streaming(struct
> > >> vb2_queue *q)
> > >> > >  	 */
> > >> > >
> > >> > >  	dev_dbg(inst->dev->dev, "%s: type: %u\n", __func__, q->type);
> > >> > > +	pm_runtime_resume_and_get(inst->dev->dev);
> > >> > >
> > >> > >  	if (wave5_vpu_both_queues_are_streaming(inst))
> > >> > >  		switch_state(inst, VPU_INST_STATE_STOP); @@ -1432,6
> > +1439,9
> > >> @@
> > >> > > static void wave5_vpu_enc_stop_streaming(struct vb2_queue *q)
> > >> > >  		streamoff_output(inst, q);
> > >> > >  	else
> > >> > >  		streamoff_capture(inst, q);
> > >> > > +
> > >> > > +	pm_runtime_mark_last_busy(inst->dev->dev);
> > >> > > +	pm_runtime_put_autosuspend(inst->dev->dev);
> > >> > >  }
> > >> > >
> > >> > >  static const struct vb2_ops wave5_vpu_enc_vb2_ops = { @@
> > >> > > -1478,6
> > >> > > +1488,7 @@ static void wave5_vpu_enc_device_run(void *priv)
> > >> > >  	u32 fail_res = 0;
> > >> > >  	int ret = 0;
> > >> > >
> > >> > > +	pm_runtime_resume_and_get(inst->dev->dev);
> > >> > >  	switch (inst->state) {
> > >> > >  	case VPU_INST_STATE_PIC_RUN:
> > >> > >  		ret = start_encode(inst, &fail_res); @@ -1491,6
> > +1502,8 @@
> > >> static
> > >> > > void wave5_vpu_enc_device_run(void *priv)
> > >> > >  			break;
> > >> > >  		}
> > >> > >  		dev_dbg(inst->dev->dev, "%s: leave with active job",
> > >> __func__);
> > >> > > +		pm_runtime_mark_last_busy(inst->dev->dev);
> > >> > > +		pm_runtime_put_autosuspend(inst->dev->dev);
> > >> > >  		return;
> > >> > >  	default:
> > >> > >  		WARN(1, "Execution of a job in state %s is invalid.\n",
> > @@
> > >> > > -1498,6 +1511,8 @@ static void wave5_vpu_enc_device_run(void
> *priv)
> > >> > >  		break;
> > >> > >  	}
> > >> > >  	dev_dbg(inst->dev->dev, "%s: leave and finish job",
> > __func__);
> > >> > > +	pm_runtime_mark_last_busy(inst->dev->dev);
> > >> > > +	pm_runtime_put_autosuspend(inst->dev->dev);
> > >> > >  	v4l2_m2m_job_finish(inst->v4l2_m2m_dev, m2m_ctx);  }
> > >> > >
> > >> > > @@ -1739,9 +1754,8 @@ static int wave5_vpu_open_enc(struct file
> > *filp)
> > >> > >  	if (ret)
> > >> > >  		goto cleanup_inst;
> > >> > >
> > >> > > -	if (dev->irq < 0 && !hrtimer_active(&dev->hrtimer) &&
> > >> list_empty(&dev->instances))
> > >> > > -		hrtimer_start(&dev->hrtimer, ns_to_ktime(dev-
> > >> >vpu_poll_interval * NSEC_PER_MSEC),
> > >> > > -			      HRTIMER_MODE_REL_PINNED);
> > >> > > +	if (list_empty(&dev->instances))
> > >> > > +		pm_runtime_use_autosuspend(inst->dev->dev);
> > >> > >
> > >> > >  	list_add_tail(&inst->list, &dev->instances);
> > >> > >
> > >> > > diff --git
> > >> > > a/drivers/media/platform/chips-media/wave5/wave5-vpu.c
> > >> > > b/drivers/media/platform/chips-media/wave5/wave5-vpu.c
> > >> > > index 7273254ecb03..41c4bf64f27d 100644
> > >> > > --- a/drivers/media/platform/chips-media/wave5/wave5-vpu.c
> > >> > > +++ b/drivers/media/platform/chips-media/wave5/wave5-vpu.c
> > >> > > @@ -10,6 +10,7 @@
> > >> > >  #include <linux/clk.h>
> > >> > >  #include <linux/firmware.h>
> > >> > >  #include <linux/interrupt.h>
> > >> > > +#include <linux/pm_runtime.h>
> > >> > >  #include <linux/reset.h>
> > >> > >  #include "wave5-vpu.h"
> > >> > >  #include "wave5-regdefine.h"
> > >> > > @@ -153,6 +154,45 @@ static int wave5_vpu_load_firmware(struct
> > >> > > device
> > >> *dev, const char *fw_name,
> > >> > >  	return 0;
> > >> > >  }
> > >> > >
> > >> > > +static __maybe_unused int wave5_pm_suspend(struct device *dev) {
> > >> > > +	struct vpu_device *vpu = dev_get_drvdata(dev);
> > >> > > +
> > >> > > +	if (pm_runtime_suspended(dev))
> > >> > > +		return 0;
> > >> > > +
> > >> > > +	if (vpu->irq < 0)
> > >> > > +		hrtimer_cancel(&vpu->hrtimer);
> > >> > > +
> > >> > > +	wave5_vpu_sleep_wake(dev, true, NULL, 0);
> > >> > > +	clk_bulk_disable_unprepare(vpu->num_clks, vpu->clks);
> > >> > > +
> > >> > > +	return 0;
> > >> > > +}
> > >> > > +
> > >> > > +static __maybe_unused int wave5_pm_resume(struct device *dev) {
> > >> > > +	struct vpu_device *vpu = dev_get_drvdata(dev);
> > >> > > +	int ret = 0;
> > >> > > +
> > >> > > +	wave5_vpu_sleep_wake(dev, false, NULL, 0);
> > >> > > +	ret = clk_bulk_prepare_enable(vpu->num_clks, vpu->clks);
> > >> > > +	if (ret) {
> > >> > > +		dev_err(dev, "Enabling clocks, fail: %d\n", ret);
> > >> > > +		return ret;
> > >> > > +	}
> > >> > > +
> > >> > > +	if (vpu->irq < 0 && !hrtimer_active(&vpu->hrtimer))
> > >> > > +		hrtimer_start(&vpu->hrtimer, ns_to_ktime(vpu-
> > >> >vpu_poll_interval * NSEC_PER_MSEC),
> > >> > > +				HRTIMER_MODE_REL_PINNED);
> > >>
> > >> I have fixed locally this style error. Alignment should match open
> > >> parenthesis.
> > >> Checkpatch caught that, it saves times if you run checkpatch before
> > >> your submissions.
> > >>
> > >> https://gitlab.freedesktop.org/ndufresne/media-staging/-/jobs/63283
> > >> 65
> > >> 4
> > >>
> > >
> > >
> > >Sorry for that, thanks for your feedback.
> > >Should I make v8 for that ?
> >
> > That is not required in this case, when he says "I have fixed
> > locally", that means that the required change is applied by the
> > maintainer before merging the changes upstream.
> >
> > Regards,
> > Sebastian

Patch

diff --git a/drivers/media/platform/chips-media/wave5/wave5-helper.c b/drivers/media/platform/chips-media/wave5/wave5-helper.c
index d60841c54a80..a20d84dbe3aa 100644
--- a/drivers/media/platform/chips-media/wave5/wave5-helper.c
+++ b/drivers/media/platform/chips-media/wave5/wave5-helper.c
@@ -58,7 +58,6 @@  int wave5_vpu_release_device(struct file *filp,
 			     char *name)
 {
 	struct vpu_instance *inst = wave5_to_vpu_inst(filp->private_data);
-	struct vpu_device *dev = inst->dev;
 	int ret = 0;
 
 	v4l2_m2m_ctx_release(inst->v4l2_fh.m2m_ctx);
@@ -78,18 +77,6 @@  int wave5_vpu_release_device(struct file *filp,
 	}
 
 	wave5_cleanup_instance(inst);
-	if (dev->irq < 0) {
-		ret = mutex_lock_interruptible(&dev->dev_lock);
-		if (ret)
-			return ret;
-
-		if (list_empty(&dev->instances)) {
-			dev_dbg(dev->dev, "Disabling the hrtimer\n");
-			hrtimer_cancel(&dev->hrtimer);
-		}
-
-		mutex_unlock(&dev->dev_lock);
-	}
 
 	return ret;
 }
diff --git a/drivers/media/platform/chips-media/wave5/wave5-hw.c b/drivers/media/platform/chips-media/wave5/wave5-hw.c
index 2a98bab446d0..c8a905994109 100644
--- a/drivers/media/platform/chips-media/wave5/wave5-hw.c
+++ b/drivers/media/platform/chips-media/wave5/wave5-hw.c
@@ -1228,8 +1228,8 @@  int wave5_vpu_re_init(struct device *dev, u8 *fw, size_t size)
 	return setup_wave5_properties(dev);
 }
 
-static int wave5_vpu_sleep_wake(struct device *dev, bool i_sleep_wake, const uint16_t *code,
-				size_t size)
+int wave5_vpu_sleep_wake(struct device *dev, bool i_sleep_wake, const uint16_t *code,
+			 size_t size)
 {
 	u32 reg_val;
 	struct vpu_buf *common_vb;
diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
index 0c5c9a8de91f..698c83e3082e 100644
--- a/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
+++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-dec.c
@@ -5,6 +5,7 @@ 
  * Copyright (C) 2021-2023 CHIPS&MEDIA INC
  */
 
+#include <linux/pm_runtime.h>
 #include "wave5-helper.h"
 
 #define VPU_DEC_DEV_NAME "C&M Wave5 VPU decoder"
@@ -518,6 +519,8 @@  static void wave5_vpu_dec_finish_decode(struct vpu_instance *inst)
 	if (q_status.report_queue_count == 0 &&
 	    (q_status.instance_queue_count == 0 || dec_info.sequence_changed)) {
 		dev_dbg(inst->dev->dev, "%s: finishing job.\n", __func__);
+		pm_runtime_mark_last_busy(inst->dev->dev);
+		pm_runtime_put_autosuspend(inst->dev->dev);
 		v4l2_m2m_job_finish(inst->v4l2_m2m_dev, m2m_ctx);
 	}
 }
@@ -1398,6 +1401,7 @@  static int wave5_vpu_dec_start_streaming(struct vb2_queue *q, unsigned int count
 	int ret = 0;
 
 	dev_dbg(inst->dev->dev, "%s: type: %u\n", __func__, q->type);
+	pm_runtime_resume_and_get(inst->dev->dev);
 
 	v4l2_m2m_update_start_streaming_state(m2m_ctx, q);
 
@@ -1429,13 +1433,15 @@  static int wave5_vpu_dec_start_streaming(struct vb2_queue *q, unsigned int count
 		if (ret)
 			goto return_buffers;
 	}
-
+	pm_runtime_mark_last_busy(inst->dev->dev);
+	pm_runtime_put_autosuspend(inst->dev->dev);
 	return ret;
 
 free_bitstream_vbuf:
 	wave5_vdi_free_dma_memory(inst->dev, &inst->bitstream_vbuf);
 return_buffers:
 	wave5_return_bufs(q, VB2_BUF_STATE_QUEUED);
+	pm_runtime_put_autosuspend(inst->dev->dev);
 	return ret;
 }
 
@@ -1521,6 +1527,7 @@  static void wave5_vpu_dec_stop_streaming(struct vb2_queue *q)
 	bool check_cmd = TRUE;
 
 	dev_dbg(inst->dev->dev, "%s: type: %u\n", __func__, q->type);
+	pm_runtime_resume_and_get(inst->dev->dev);
 
 	while (check_cmd) {
 		struct queue_status_info q_status;
@@ -1544,6 +1551,9 @@  static void wave5_vpu_dec_stop_streaming(struct vb2_queue *q)
 		streamoff_output(q);
 	else
 		streamoff_capture(q);
+
+	pm_runtime_mark_last_busy(inst->dev->dev);
+	pm_runtime_put_autosuspend(inst->dev->dev);
 }
 
 static const struct vb2_ops wave5_vpu_dec_vb2_ops = {
@@ -1630,7 +1640,7 @@  static void wave5_vpu_dec_device_run(void *priv)
 	int ret = 0;
 
 	dev_dbg(inst->dev->dev, "%s: Fill the ring buffer with new bitstream data", __func__);
-
+	pm_runtime_resume_and_get(inst->dev->dev);
 	ret = fill_ringbuffer(inst);
 	if (ret) {
 		dev_warn(inst->dev->dev, "Filling ring buffer failed\n");
@@ -1713,6 +1723,8 @@  static void wave5_vpu_dec_device_run(void *priv)
 
 finish_job_and_return:
 	dev_dbg(inst->dev->dev, "%s: leave and finish job", __func__);
+	pm_runtime_mark_last_busy(inst->dev->dev);
+	pm_runtime_put_autosuspend(inst->dev->dev);
 	v4l2_m2m_job_finish(inst->v4l2_m2m_dev, m2m_ctx);
 }
 
@@ -1879,9 +1891,8 @@  static int wave5_vpu_open_dec(struct file *filp)
 	if (ret)
 		goto cleanup_inst;
 
-	if (dev->irq < 0 && !hrtimer_active(&dev->hrtimer) && list_empty(&dev->instances))
-		hrtimer_start(&dev->hrtimer, ns_to_ktime(dev->vpu_poll_interval * NSEC_PER_MSEC),
-			      HRTIMER_MODE_REL_PINNED);
+	if (list_empty(&dev->instances))
+		pm_runtime_use_autosuspend(inst->dev->dev);
 
 	list_add_tail(&inst->list, &dev->instances);
 
diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c b/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c
index b731decbfda6..985de0c293e2 100644
--- a/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c
+++ b/drivers/media/platform/chips-media/wave5/wave5-vpu-enc.c
@@ -5,6 +5,7 @@ 
  * Copyright (C) 2021-2023 CHIPS&MEDIA INC
  */
 
+#include <linux/pm_runtime.h>
 #include "wave5-helper.h"
 
 #define VPU_ENC_DEV_NAME "C&M Wave5 VPU encoder"
@@ -1310,6 +1311,7 @@  static int wave5_vpu_enc_start_streaming(struct vb2_queue *q, unsigned int count
 	struct v4l2_m2m_ctx *m2m_ctx = inst->v4l2_fh.m2m_ctx;
 	int ret = 0;
 
+	pm_runtime_resume_and_get(inst->dev->dev);
 	v4l2_m2m_update_start_streaming_state(m2m_ctx, q);
 
 	if (inst->state == VPU_INST_STATE_NONE && q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) {
@@ -1364,9 +1366,13 @@  static int wave5_vpu_enc_start_streaming(struct vb2_queue *q, unsigned int count
 	if (ret)
 		goto return_buffers;
 
+	pm_runtime_mark_last_busy(inst->dev->dev);
+	pm_runtime_put_autosuspend(inst->dev->dev);
 	return 0;
 return_buffers:
 	wave5_return_bufs(q, VB2_BUF_STATE_QUEUED);
+	pm_runtime_mark_last_busy(inst->dev->dev);
+	pm_runtime_put_autosuspend(inst->dev->dev);
 	return ret;
 }
 
@@ -1408,6 +1414,7 @@  static void wave5_vpu_enc_stop_streaming(struct vb2_queue *q)
 	 */
 
 	dev_dbg(inst->dev->dev, "%s: type: %u\n", __func__, q->type);
+	pm_runtime_resume_and_get(inst->dev->dev);
 
 	if (wave5_vpu_both_queues_are_streaming(inst))
 		switch_state(inst, VPU_INST_STATE_STOP);
@@ -1432,6 +1439,9 @@  static void wave5_vpu_enc_stop_streaming(struct vb2_queue *q)
 		streamoff_output(inst, q);
 	else
 		streamoff_capture(inst, q);
+
+	pm_runtime_mark_last_busy(inst->dev->dev);
+	pm_runtime_put_autosuspend(inst->dev->dev);
 }
 
 static const struct vb2_ops wave5_vpu_enc_vb2_ops = {
@@ -1478,6 +1488,7 @@  static void wave5_vpu_enc_device_run(void *priv)
 	u32 fail_res = 0;
 	int ret = 0;
 
+	pm_runtime_resume_and_get(inst->dev->dev);
 	switch (inst->state) {
 	case VPU_INST_STATE_PIC_RUN:
 		ret = start_encode(inst, &fail_res);
@@ -1491,6 +1502,8 @@  static void wave5_vpu_enc_device_run(void *priv)
 			break;
 		}
 		dev_dbg(inst->dev->dev, "%s: leave with active job", __func__);
+		pm_runtime_mark_last_busy(inst->dev->dev);
+		pm_runtime_put_autosuspend(inst->dev->dev);
 		return;
 	default:
 		WARN(1, "Execution of a job in state %s is invalid.\n",
@@ -1498,6 +1511,8 @@  static void wave5_vpu_enc_device_run(void *priv)
 		break;
 	}
 	dev_dbg(inst->dev->dev, "%s: leave and finish job", __func__);
+	pm_runtime_mark_last_busy(inst->dev->dev);
+	pm_runtime_put_autosuspend(inst->dev->dev);
 	v4l2_m2m_job_finish(inst->v4l2_m2m_dev, m2m_ctx);
 }
 
@@ -1739,9 +1754,8 @@  static int wave5_vpu_open_enc(struct file *filp)
 	if (ret)
 		goto cleanup_inst;
 
-	if (dev->irq < 0 && !hrtimer_active(&dev->hrtimer) && list_empty(&dev->instances))
-		hrtimer_start(&dev->hrtimer, ns_to_ktime(dev->vpu_poll_interval * NSEC_PER_MSEC),
-			      HRTIMER_MODE_REL_PINNED);
+	if (list_empty(&dev->instances))
+		pm_runtime_use_autosuspend(inst->dev->dev);
 
 	list_add_tail(&inst->list, &dev->instances);
 
diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpu.c b/drivers/media/platform/chips-media/wave5/wave5-vpu.c
index 7273254ecb03..41c4bf64f27d 100644
--- a/drivers/media/platform/chips-media/wave5/wave5-vpu.c
+++ b/drivers/media/platform/chips-media/wave5/wave5-vpu.c
@@ -10,6 +10,7 @@ 
 #include <linux/clk.h>
 #include <linux/firmware.h>
 #include <linux/interrupt.h>
+#include <linux/pm_runtime.h>
 #include <linux/reset.h>
 #include "wave5-vpu.h"
 #include "wave5-regdefine.h"
@@ -153,6 +154,45 @@  static int wave5_vpu_load_firmware(struct device *dev, const char *fw_name,
 	return 0;
 }
 
+static __maybe_unused int wave5_pm_suspend(struct device *dev)
+{
+	struct vpu_device *vpu = dev_get_drvdata(dev);
+
+	if (pm_runtime_suspended(dev))
+		return 0;
+
+	if (vpu->irq < 0)
+		hrtimer_cancel(&vpu->hrtimer);
+
+	wave5_vpu_sleep_wake(dev, true, NULL, 0);
+	clk_bulk_disable_unprepare(vpu->num_clks, vpu->clks);
+
+	return 0;
+}
+
+static __maybe_unused int wave5_pm_resume(struct device *dev)
+{
+	struct vpu_device *vpu = dev_get_drvdata(dev);
+	int ret = 0;
+
+	wave5_vpu_sleep_wake(dev, false, NULL, 0);
+	ret = clk_bulk_prepare_enable(vpu->num_clks, vpu->clks);
+	if (ret) {
+		dev_err(dev, "Enabling clocks, fail: %d\n", ret);
+		return ret;
+	}
+
+	if (vpu->irq < 0 && !hrtimer_active(&vpu->hrtimer))
+		hrtimer_start(&vpu->hrtimer, ns_to_ktime(vpu->vpu_poll_interval * NSEC_PER_MSEC),
+				HRTIMER_MODE_REL_PINNED);
+
+	return ret;
+}
+
+static const struct dev_pm_ops wave5_pm_ops = {
+	SET_RUNTIME_PM_OPS(wave5_pm_suspend, wave5_pm_resume, NULL)
+};
+
 static int wave5_vpu_probe(struct platform_device *pdev)
 {
 	int ret;
@@ -281,6 +321,12 @@  static int wave5_vpu_probe(struct platform_device *pdev)
 		 (match_data->flags & WAVE5_IS_DEC) ? "'DECODE'" : "");
 	dev_info(&pdev->dev, "Product Code:      0x%x\n", dev->product_code);
 	dev_info(&pdev->dev, "Firmware Revision: %u\n", fw_revision);
+
+	pm_runtime_set_autosuspend_delay(&pdev->dev, 100);
+	pm_runtime_use_autosuspend(&pdev->dev);
+	pm_runtime_enable(&pdev->dev);
+	wave5_vpu_sleep_wake(&pdev->dev, true, NULL, 0);
+
 	return 0;
 
 err_enc_unreg:
@@ -310,6 +356,9 @@  static void wave5_vpu_remove(struct platform_device *pdev)
 		hrtimer_cancel(&dev->hrtimer);
 	}
 
+	pm_runtime_put_sync(&pdev->dev);
+	pm_runtime_disable(&pdev->dev);
+
 	mutex_destroy(&dev->dev_lock);
 	mutex_destroy(&dev->hw_lock);
 	reset_control_assert(dev->resets);
@@ -337,6 +386,7 @@  static struct platform_driver wave5_vpu_driver = {
 	.driver = {
 		.name = VPU_PLATFORM_DEVICE_NAME,
 		.of_match_table = of_match_ptr(wave5_dt_ids),
+		.pm = &wave5_pm_ops,
 		},
 	.probe = wave5_vpu_probe,
 	.remove_new = wave5_vpu_remove,
diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.c b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.c
index 1a3efb638dde..e16b990041c2 100644
--- a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.c
+++ b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.c
@@ -6,6 +6,8 @@ 
  */
 
 #include <linux/bug.h>
+#include <linux/pm_runtime.h>
+#include <linux/delay.h>
 #include "wave5-vpuapi.h"
 #include "wave5-regdefine.h"
 #include "wave5.h"
@@ -195,14 +197,20 @@  int wave5_vpu_dec_close(struct vpu_instance *inst, u32 *fail_res)
 	int retry = 0;
 	struct vpu_device *vpu_dev = inst->dev;
 	int i;
+	int inst_count = 0;
+	struct vpu_instance *inst_elm;
 
 	*fail_res = 0;
 	if (!inst->codec_info)
 		return -EINVAL;
 
+	pm_runtime_resume_and_get(inst->dev->dev);
+
 	ret = mutex_lock_interruptible(&vpu_dev->hw_lock);
-	if (ret)
+	if (ret) {
+		pm_runtime_put_sync(inst->dev->dev);
 		return ret;
+	}
 
 	do {
 		ret = wave5_vpu_dec_finish_seq(inst, fail_res);
@@ -232,9 +240,14 @@  int wave5_vpu_dec_close(struct vpu_instance *inst, u32 *fail_res)
 
 	wave5_vdi_free_dma_memory(vpu_dev, &p_dec_info->vb_task);
 
+	list_for_each_entry(inst_elm, &vpu_dev->instances, list)
+		inst_count++;
+	if (inst_count == 1)
+		pm_runtime_dont_use_autosuspend(vpu_dev->dev);
+
 unlock_and_return:
 	mutex_unlock(&vpu_dev->hw_lock);
-
+	pm_runtime_put_sync(inst->dev->dev);
 	return ret;
 }
 
@@ -697,25 +710,33 @@  int wave5_vpu_enc_close(struct vpu_instance *inst, u32 *fail_res)
 	int ret;
 	int retry = 0;
 	struct vpu_device *vpu_dev = inst->dev;
+	int inst_count = 0;
+	struct vpu_instance *inst_elm;
 
 	*fail_res = 0;
 	if (!inst->codec_info)
 		return -EINVAL;
 
+	pm_runtime_resume_and_get(inst->dev->dev);
+
 	ret = mutex_lock_interruptible(&vpu_dev->hw_lock);
-	if (ret)
+	if (ret) {
+		pm_runtime_resume_and_get(inst->dev->dev);
 		return ret;
+	}
 
 	do {
 		ret = wave5_vpu_enc_finish_seq(inst, fail_res);
 		if (ret < 0 && *fail_res != WAVE5_SYSERR_VPU_STILL_RUNNING) {
 			dev_warn(inst->dev->dev, "enc_finish_seq timed out\n");
+			pm_runtime_resume_and_get(inst->dev->dev);
 			mutex_unlock(&vpu_dev->hw_lock);
 			return ret;
 		}
 
 		if (*fail_res == WAVE5_SYSERR_VPU_STILL_RUNNING &&
 		    retry++ >= MAX_FIRMWARE_CALL_RETRY) {
+			pm_runtime_resume_and_get(inst->dev->dev);
 			mutex_unlock(&vpu_dev->hw_lock);
 			return -ETIMEDOUT;
 		}
@@ -734,7 +755,13 @@  int wave5_vpu_enc_close(struct vpu_instance *inst, u32 *fail_res)
 
 	wave5_vdi_free_dma_memory(vpu_dev, &p_enc_info->vb_task);
 
+	list_for_each_entry(inst_elm, &vpu_dev->instances, list)
+		inst_count++;
+	if (inst_count == 1)
+		pm_runtime_dont_use_autosuspend(vpu_dev->dev);
+
 	mutex_unlock(&vpu_dev->hw_lock);
+	pm_runtime_put_sync(inst->dev->dev);
 
 	return 0;
 }
diff --git a/drivers/media/platform/chips-media/wave5/wave5.h b/drivers/media/platform/chips-media/wave5/wave5.h
index 2a29b9164f97..2caab356f3e1 100644
--- a/drivers/media/platform/chips-media/wave5/wave5.h
+++ b/drivers/media/platform/chips-media/wave5/wave5.h
@@ -62,6 +62,9 @@  int wave5_vpu_get_version(struct vpu_device *vpu_dev, u32 *revision);
 
 int wave5_vpu_init(struct device *dev, u8 *fw, size_t size);
 
+int wave5_vpu_sleep_wake(struct device *dev, bool i_sleep_wake, const uint16_t *code,
+			 size_t size);
+
 int wave5_vpu_reset(struct device *dev, enum sw_reset_mode reset_mode);
 
 int wave5_vpu_build_up_dec_param(struct vpu_instance *inst, struct dec_open_param *param);