2021-05-06 15:31:18

by Mauro Carvalho Chehab

[permalink] [raw]
Subject: [PATCH v5 00/25] media: use pm_runtime_resume_and_get() on non-i2c drivers

Dealing with PM runtime (RPM) is different than dealing with other kAPIs used
on media, as most pm_runtime_get_*() functions won't return to the the state
before the call if an error rises. They, instead, increment an usage_count.

Due to that, there were several bugs on media. Just on this review, we found
24 such errors.

So, let's use pm_runtime_resume_and_get() whenever possible, as it
has two advantages over :

1. On errors, it decrements the usage count;
2. It always return zero on success or an error code. This prevents a
common error pattern of checking if ret is not zero to identify
for errors.

There are however a few places where calls to pm_runtime_get_sync()
are kept. On several of those, a comment was added, in order to
help preventing trivial patches that could try to change them.

PS.: This series was submitted already together with the fix patches
at:

https://lore.kernel.org/linux-media/[email protected]/

I opted to break it on 3 parts, in order to make easier to review.

This is the third (and final) part.

Mauro Carvalho Chehab (25):
staging: media: imx7-mipi-csis: use pm_runtime_resume_and_get()
staging: media: atomisp: use pm_runtime_resume_and_get()
staging: media: ipu3: use pm_runtime_resume_and_get()
staging: media: cedrus_video: use pm_runtime_resume_and_get()
staging: media: tegra-vde: use pm_runtime_resume_and_get()
staging: media: tegra-video: use pm_runtime_resume_and_get()
media: rockchip/rga: use pm_runtime_resume_and_get()
media: sti/hva: use pm_runtime_resume_and_get()
media: ipu3: use pm_runtime_resume_and_get()
media: coda: use pm_runtime_resume_and_get()
media: mtk-jpeg: use pm_runtime_resume_and_get()
media: camss: use pm_runtime_resume_and_get()
media: venus: core: use pm_runtime_resume_and_get()
media: venus: vdec: use pm_runtime_resume_and_get()
media: venus: venc: use pm_runtime_resume_and_get()
media: rcar-fcp: use pm_runtime_resume_and_get()
media: rkisp1: use pm_runtime_resume_and_get()
media: s3c-camif: use pm_runtime_resume_and_get()
media: s5p-mfc: use pm_runtime_resume_and_get()
media: stm32: use pm_runtime_resume_and_get()
media: sunxi: use pm_runtime_resume_and_get()
media: ti-vpe: use pm_runtime_resume_and_get()
media: vsp1: use pm_runtime_resume_and_get()
media: rcar-vin: use pm_runtime_resume_and_get()
media: hantro: use pm_runtime_resume_and_get()

drivers/media/pci/intel/ipu3/ipu3-cio2-main.c | 3 +--
drivers/media/platform/coda/coda-common.c | 7 ++++---
.../media/platform/mtk-jpeg/mtk_jpeg_core.c | 4 ++--
.../media/platform/qcom/camss/camss-csid.c | 6 ++----
.../media/platform/qcom/camss/camss-csiphy.c | 6 ++----
.../media/platform/qcom/camss/camss-ispif.c | 6 ++----
drivers/media/platform/qcom/camss/camss-vfe.c | 5 +++--
.../media/platform/qcom/venus/pm_helpers.c | 3 +--
drivers/media/platform/qcom/venus/vdec.c | 6 +++---
drivers/media/platform/qcom/venus/venc.c | 5 +++--
drivers/media/platform/rcar-fcp.c | 8 +------
drivers/media/platform/rcar-vin/rcar-csi2.c | 15 ++++++++++---
drivers/media/platform/rcar-vin/rcar-dma.c | 6 ++----
drivers/media/platform/rcar-vin/rcar-v4l2.c | 6 ++----
drivers/media/platform/rockchip/rga/rga-buf.c | 3 +--
drivers/media/platform/rockchip/rga/rga.c | 4 +++-
.../platform/rockchip/rkisp1/rkisp1-capture.c | 3 +--
.../media/platform/s3c-camif/camif-capture.c | 2 +-
drivers/media/platform/s3c-camif/camif-core.c | 5 +++--
drivers/media/platform/s5p-mfc/s5p_mfc_pm.c | 6 ++----
drivers/media/platform/sti/hva/hva-hw.c | 17 ++++++++-------
drivers/media/platform/stm32/stm32-dcmi.c | 5 +++--
.../platform/sunxi/sun4i-csi/sun4i_v4l2.c | 6 ++++--
drivers/media/platform/ti-vpe/cal-video.c | 4 +++-
drivers/media/platform/ti-vpe/cal.c | 8 ++++---
drivers/media/platform/ti-vpe/vpe.c | 8 +++----
drivers/media/platform/vsp1/vsp1_drv.c | 10 +--------
.../staging/media/atomisp/pci/atomisp_fops.c | 6 +++---
drivers/staging/media/hantro/hantro_drv.c | 5 ++---
drivers/staging/media/imx/imx7-mipi-csis.c | 7 +++----
drivers/staging/media/ipu3/ipu3.c | 3 +--
.../staging/media/sunxi/cedrus/cedrus_video.c | 6 ++----
drivers/staging/media/tegra-vde/vde.c | 21 ++++++++++++++++---
drivers/staging/media/tegra-video/csi.c | 3 +--
drivers/staging/media/tegra-video/vi.c | 3 +--
35 files changed, 110 insertions(+), 111 deletions(-)

--
2.30.2



2021-05-06 15:32:07

by Mauro Carvalho Chehab

[permalink] [raw]
Subject: [PATCH v5 02/25] staging: media: atomisp: use pm_runtime_resume_and_get()

Commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
added pm_runtime_resume_and_get() in order to automatically handle
dev->power.usage_count decrement on errors.

Use the new API, in order to cleanup the error check logic.

Besides that, the de-init order in case of css error was wrong.
This change should also fix that.

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/staging/media/atomisp/pci/atomisp_fops.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp_fops.c b/drivers/staging/media/atomisp/pci/atomisp_fops.c
index f1e6b2597853..26d05474a035 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_fops.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_fops.c
@@ -837,7 +837,7 @@ static int atomisp_open(struct file *file)
}

/* runtime power management, turn on ISP */
- ret = pm_runtime_get_sync(vdev->v4l2_dev->dev);
+ ret = pm_runtime_resume_and_get(vdev->v4l2_dev->dev);
if (ret < 0) {
dev_err(isp->dev, "Failed to power on device\n");
goto error;
@@ -881,9 +881,9 @@ static int atomisp_open(struct file *file)

css_error:
atomisp_css_uninit(isp);
-error:
- hmm_pool_unregister(HMM_POOL_TYPE_DYNAMIC);
pm_runtime_put(vdev->v4l2_dev->dev);
+error:
+ hmm_pool_unregister(HMM_POOL_TYPE_DYNAMIC);
rt_mutex_unlock(&isp->mutex);
return ret;
}
--
2.30.2

2021-05-06 15:32:07

by Mauro Carvalho Chehab

[permalink] [raw]
Subject: [PATCH v5 21/25] media: sunxi: use pm_runtime_resume_and_get()

Commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
added pm_runtime_resume_and_get() in order to automatically handle
dev->power.usage_count decrement on errors.

Use the new API, in order to cleanup the error check logic.

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/media/platform/sunxi/sun4i-csi/sun4i_v4l2.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/sunxi/sun4i-csi/sun4i_v4l2.c b/drivers/media/platform/sunxi/sun4i-csi/sun4i_v4l2.c
index 4785faddf630..54b909987caa 100644
--- a/drivers/media/platform/sunxi/sun4i-csi/sun4i_v4l2.c
+++ b/drivers/media/platform/sunxi/sun4i-csi/sun4i_v4l2.c
@@ -206,9 +206,9 @@ static int sun4i_csi_open(struct file *file)
if (ret)
return ret;

- ret = pm_runtime_get_sync(csi->dev);
+ ret = pm_runtime_resume_and_get(csi->dev);
if (ret < 0)
- goto err_pm_put;
+ goto err_unlock;

ret = v4l2_pipeline_pm_get(&csi->vdev.entity);
if (ret)
@@ -227,6 +227,8 @@ static int sun4i_csi_open(struct file *file)

err_pm_put:
pm_runtime_put(csi->dev);
+
+err_unlock:
mutex_unlock(&csi->lock);

return ret;
--
2.30.2

2021-05-06 19:08:32

by Mauro Carvalho Chehab

[permalink] [raw]
Subject: [PATCH v5 08/25] media: sti/hva: use pm_runtime_resume_and_get()

Commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
added pm_runtime_resume_and_get() in order to automatically handle
dev->power.usage_count decrement on errors.

While the hva driver does it right, there are lots of errors
on other drivers due to its misusage. So, let's change
this driver to also use pm_runtime_resume_and_get(), as we're
doing similar changes all over the media subsystem.

Reviewed-by: Jonathan Cameron <[email protected]>
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/media/platform/sti/hva/hva-hw.c | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/media/platform/sti/hva/hva-hw.c b/drivers/media/platform/sti/hva/hva-hw.c
index f59811e27f51..77b8bfa5e0c5 100644
--- a/drivers/media/platform/sti/hva/hva-hw.c
+++ b/drivers/media/platform/sti/hva/hva-hw.c
@@ -270,9 +270,8 @@ static unsigned long int hva_hw_get_ip_version(struct hva_dev *hva)
struct device *dev = hva_to_dev(hva);
unsigned long int version;

- if (pm_runtime_get_sync(dev) < 0) {
+ if (pm_runtime_resume_and_get(dev) < 0) {
dev_err(dev, "%s failed to get pm_runtime\n", HVA_PREFIX);
- pm_runtime_put_noidle(dev);
mutex_unlock(&hva->protect_mutex);
return -EFAULT;
}
@@ -386,10 +385,10 @@ int hva_hw_probe(struct platform_device *pdev, struct hva_dev *hva)
pm_runtime_set_suspended(dev);
pm_runtime_enable(dev);

- ret = pm_runtime_get_sync(dev);
+ ret = pm_runtime_resume_and_get(dev);
if (ret < 0) {
dev_err(dev, "%s failed to set PM\n", HVA_PREFIX);
- goto err_pm;
+ goto err_clk;
}

/* check IP hardware version */
@@ -462,6 +461,7 @@ int hva_hw_execute_task(struct hva_ctx *ctx, enum hva_hw_cmd_type cmd,
u8 client_id = ctx->id;
int ret;
u32 reg = 0;
+ bool got_pm = false;

mutex_lock(&hva->protect_mutex);

@@ -469,12 +469,13 @@ int hva_hw_execute_task(struct hva_ctx *ctx, enum hva_hw_cmd_type cmd,
enable_irq(hva->irq_its);
enable_irq(hva->irq_err);

- if (pm_runtime_get_sync(dev) < 0) {
+ if (pm_runtime_resume_and_get(dev) < 0) {
dev_err(dev, "%s failed to get pm_runtime\n", ctx->name);
ctx->sys_errors++;
ret = -EFAULT;
goto out;
}
+ got_pm = true;

reg = readl_relaxed(hva->regs + HVA_HIF_REG_CLK_GATING);
switch (cmd) {
@@ -537,7 +538,8 @@ int hva_hw_execute_task(struct hva_ctx *ctx, enum hva_hw_cmd_type cmd,
dev_dbg(dev, "%s unknown command 0x%x\n", ctx->name, cmd);
}

- pm_runtime_put_autosuspend(dev);
+ if (got_pm)
+ pm_runtime_put_autosuspend(dev);
mutex_unlock(&hva->protect_mutex);

return ret;
@@ -553,9 +555,8 @@ void hva_hw_dump_regs(struct hva_dev *hva, struct seq_file *s)

mutex_lock(&hva->protect_mutex);

- if (pm_runtime_get_sync(dev) < 0) {
+ if (pm_runtime_resume_and_get(dev) < 0) {
seq_puts(s, "Cannot wake up IP\n");
- pm_runtime_put_noidle(dev);
mutex_unlock(&hva->protect_mutex);
return;
}
--
2.30.2

2021-05-06 19:08:43

by Mauro Carvalho Chehab

[permalink] [raw]
Subject: [PATCH v5 12/25] media: camss: use pm_runtime_resume_and_get()

Commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
added pm_runtime_resume_and_get() in order to automatically handle
dev->power.usage_count decrement on errors.

Use the new API, in order to cleanup the error check logic.

Reviewed-by: Robert Foss <[email protected]>
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/media/platform/qcom/camss/camss-csid.c | 6 ++----
drivers/media/platform/qcom/camss/camss-csiphy.c | 6 ++----
drivers/media/platform/qcom/camss/camss-ispif.c | 6 ++----
drivers/media/platform/qcom/camss/camss-vfe.c | 5 +++--
4 files changed, 9 insertions(+), 14 deletions(-)

diff --git a/drivers/media/platform/qcom/camss/camss-csid.c b/drivers/media/platform/qcom/camss/camss-csid.c
index cc11fbfdae13..d2a7f2a64f26 100644
--- a/drivers/media/platform/qcom/camss/camss-csid.c
+++ b/drivers/media/platform/qcom/camss/camss-csid.c
@@ -156,11 +156,9 @@ static int csid_set_power(struct v4l2_subdev *sd, int on)
int ret;

if (on) {
- ret = pm_runtime_get_sync(dev);
- if (ret < 0) {
- pm_runtime_put_sync(dev);
+ ret = pm_runtime_resume_and_get(dev);
+ if (ret < 0)
return ret;
- }

ret = regulator_enable(csid->vdda);
if (ret < 0) {
diff --git a/drivers/media/platform/qcom/camss/camss-csiphy.c b/drivers/media/platform/qcom/camss/camss-csiphy.c
index b3c3bf19e522..8e18b8e668cf 100644
--- a/drivers/media/platform/qcom/camss/camss-csiphy.c
+++ b/drivers/media/platform/qcom/camss/camss-csiphy.c
@@ -197,11 +197,9 @@ static int csiphy_set_power(struct v4l2_subdev *sd, int on)
if (on) {
int ret;

- ret = pm_runtime_get_sync(dev);
- if (ret < 0) {
- pm_runtime_put_sync(dev);
+ ret = pm_runtime_resume_and_get(dev);
+ if (ret < 0)
return ret;
- }

ret = csiphy_set_clock_rates(csiphy);
if (ret < 0) {
diff --git a/drivers/media/platform/qcom/camss/camss-ispif.c b/drivers/media/platform/qcom/camss/camss-ispif.c
index 37611c8861da..d9907742ba79 100644
--- a/drivers/media/platform/qcom/camss/camss-ispif.c
+++ b/drivers/media/platform/qcom/camss/camss-ispif.c
@@ -372,11 +372,9 @@ static int ispif_set_power(struct v4l2_subdev *sd, int on)
goto exit;
}

- ret = pm_runtime_get_sync(dev);
- if (ret < 0) {
- pm_runtime_put_sync(dev);
+ ret = pm_runtime_resume_and_get(dev);
+ if (ret < 0)
goto exit;
- }

ret = camss_enable_clocks(ispif->nclocks, ispif->clock, dev);
if (ret < 0) {
diff --git a/drivers/media/platform/qcom/camss/camss-vfe.c b/drivers/media/platform/qcom/camss/camss-vfe.c
index 15695fd466c4..cf743e61f798 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe.c
+++ b/drivers/media/platform/qcom/camss/camss-vfe.c
@@ -584,9 +584,9 @@ static int vfe_get(struct vfe_device *vfe)
if (ret < 0)
goto error_pm_domain;

- ret = pm_runtime_get_sync(vfe->camss->dev);
+ ret = pm_runtime_resume_and_get(vfe->camss->dev);
if (ret < 0)
- goto error_pm_runtime_get;
+ goto error_domain_off;

ret = vfe_set_clock_rates(vfe);
if (ret < 0)
@@ -620,6 +620,7 @@ static int vfe_get(struct vfe_device *vfe)

error_pm_runtime_get:
pm_runtime_put_sync(vfe->camss->dev);
+error_domain_off:
vfe->ops->pm_domain_off(vfe);

error_pm_domain:
--
2.30.2

2021-05-06 19:08:51

by Mauro Carvalho Chehab

[permalink] [raw]
Subject: [PATCH v5 23/25] media: vsp1: use pm_runtime_resume_and_get()

Commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
added pm_runtime_resume_and_get() in order to automatically handle
dev->power.usage_count decrement on errors.

Use the new API, in order to cleanup the error check logic.

As a bonus, pm_runtime_resume_and_get() always return 0 on success.
So, the code can be simplified.

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/media/platform/vsp1/vsp1_drv.c | 10 +---------
1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1_drv.c b/drivers/media/platform/vsp1/vsp1_drv.c
index aa66e4f5f3f3..de442d6c9926 100644
--- a/drivers/media/platform/vsp1/vsp1_drv.c
+++ b/drivers/media/platform/vsp1/vsp1_drv.c
@@ -559,15 +559,7 @@ static int vsp1_device_init(struct vsp1_device *vsp1)
*/
int vsp1_device_get(struct vsp1_device *vsp1)
{
- int ret;
-
- ret = pm_runtime_get_sync(vsp1->dev);
- if (ret < 0) {
- pm_runtime_put_noidle(vsp1->dev);
- return ret;
- }
-
- return 0;
+ return pm_runtime_resume_and_get(vsp1->dev);
}

/*
--
2.30.2

2021-05-06 19:10:33

by Mauro Carvalho Chehab

[permalink] [raw]
Subject: [PATCH v5 05/25] staging: media: tegra-vde: use pm_runtime_resume_and_get()

Commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
added pm_runtime_resume_and_get() in order to automatically handle
dev->power.usage_count decrement on errors.

Use the new API, in order to cleanup the error check logic.

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/staging/media/tegra-vde/vde.c | 21 ++++++++++++++++++---
1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/media/tegra-vde/vde.c b/drivers/staging/media/tegra-vde/vde.c
index 28845b5bafaf..e025b69776f2 100644
--- a/drivers/staging/media/tegra-vde/vde.c
+++ b/drivers/staging/media/tegra-vde/vde.c
@@ -775,9 +775,9 @@ static int tegra_vde_ioctl_decode_h264(struct tegra_vde *vde,
if (ret)
goto release_dpb_frames;

- ret = pm_runtime_get_sync(dev);
+ ret = pm_runtime_resume_and_get(dev);
if (ret < 0)
- goto put_runtime_pm;
+ goto unlock;

/*
* We rely on the VDE registers reset value, otherwise VDE
@@ -843,6 +843,8 @@ static int tegra_vde_ioctl_decode_h264(struct tegra_vde *vde,
put_runtime_pm:
pm_runtime_mark_last_busy(dev);
pm_runtime_put_autosuspend(dev);
+
+unlock:
mutex_unlock(&vde->lock);

release_dpb_frames:
@@ -1069,11 +1071,19 @@ static int tegra_vde_probe(struct platform_device *pdev)
* power-cycle it in order to put hardware into a predictable lower
* power state.
*/
- pm_runtime_get_sync(dev);
+ if (pm_runtime_resume_and_get(dev) < 0)
+ goto err_pm_runtime;
+
pm_runtime_put(dev);

return 0;

+err_pm_runtime:
+ misc_deregister(&vde->miscdev);
+
+ pm_runtime_dont_use_autosuspend(dev);
+ pm_runtime_disable(dev);
+
err_deinit_iommu:
tegra_vde_iommu_deinit(vde);

@@ -1089,7 +1099,12 @@ static int tegra_vde_remove(struct platform_device *pdev)
struct tegra_vde *vde = platform_get_drvdata(pdev);
struct device *dev = &pdev->dev;

+ /*
+ * As it increments RPM usage_count even on errors, we don't need to
+ * check the returned code here.
+ */
pm_runtime_get_sync(dev);
+
pm_runtime_dont_use_autosuspend(dev);
pm_runtime_disable(dev);

--
2.30.2

2021-05-06 19:12:14

by Mauro Carvalho Chehab

[permalink] [raw]
Subject: [PATCH v5 01/25] staging: media: imx7-mipi-csis: use pm_runtime_resume_and_get()

Commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
added pm_runtime_resume_and_get() in order to automatically handle
dev->power.usage_count decrement on errors.

Use the new API, in order to cleanup the error check logic.

Acked-by: Rui Miguel Silva <[email protected]>
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/staging/media/imx/imx7-mipi-csis.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c b/drivers/staging/media/imx/imx7-mipi-csis.c
index 025fdc488bd6..1dc680d94a46 100644
--- a/drivers/staging/media/imx/imx7-mipi-csis.c
+++ b/drivers/staging/media/imx/imx7-mipi-csis.c
@@ -695,11 +695,10 @@ static int mipi_csis_s_stream(struct v4l2_subdev *mipi_sd, int enable)

mipi_csis_clear_counters(state);

- ret = pm_runtime_get_sync(&state->pdev->dev);
- if (ret < 0) {
- pm_runtime_put_noidle(&state->pdev->dev);
+ ret = pm_runtime_resume_and_get(&state->pdev->dev);
+ if (ret < 0)
return ret;
- }
+
ret = v4l2_subdev_call(state->src_sd, core, s_power, 1);
if (ret < 0 && ret != -ENOIOCTLCMD)
goto done;
--
2.30.2

2021-05-06 19:19:51

by Mauro Carvalho Chehab

[permalink] [raw]
Subject: [PATCH v5 09/25] media: ipu3: use pm_runtime_resume_and_get()

Commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
added pm_runtime_resume_and_get() in order to automatically handle
dev->power.usage_count decrement on errors.

Use the new API, in order to cleanup the error check logic.

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/media/pci/intel/ipu3/ipu3-cio2-main.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2-main.c b/drivers/media/pci/intel/ipu3/ipu3-cio2-main.c
index fecef85bd62e..ca8040d1a725 100644
--- a/drivers/media/pci/intel/ipu3/ipu3-cio2-main.c
+++ b/drivers/media/pci/intel/ipu3/ipu3-cio2-main.c
@@ -975,10 +975,9 @@ static int cio2_vb2_start_streaming(struct vb2_queue *vq, unsigned int count)
cio2->cur_queue = q;
atomic_set(&q->frame_sequence, 0);

- r = pm_runtime_get_sync(&cio2->pci_dev->dev);
+ r = pm_runtime_resume_and_get(&cio2->pci_dev->dev);
if (r < 0) {
dev_info(&cio2->pci_dev->dev, "failed to set power %d\n", r);
- pm_runtime_put_noidle(&cio2->pci_dev->dev);
return r;
}

--
2.30.2

2021-05-06 19:19:51

by Mauro Carvalho Chehab

[permalink] [raw]
Subject: [PATCH v5 18/25] media: s3c-camif: use pm_runtime_resume_and_get()

Commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
added pm_runtime_resume_and_get() in order to automatically handle
dev->power.usage_count decrement on errors.

Use the new API, in order to cleanup the error check logic.

Reviewed-by: Sylwester Nawrocki <[email protected]>
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/media/platform/s3c-camif/camif-capture.c | 2 +-
drivers/media/platform/s3c-camif/camif-core.c | 5 +++--
2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/s3c-camif/camif-capture.c b/drivers/media/platform/s3c-camif/camif-capture.c
index 9ca49af29542..62241ec3b978 100644
--- a/drivers/media/platform/s3c-camif/camif-capture.c
+++ b/drivers/media/platform/s3c-camif/camif-capture.c
@@ -547,7 +547,7 @@ static int s3c_camif_open(struct file *file)
if (ret < 0)
goto unlock;

- ret = pm_runtime_get_sync(camif->dev);
+ ret = pm_runtime_resume_and_get(camif->dev);
if (ret < 0)
goto err_pm;

diff --git a/drivers/media/platform/s3c-camif/camif-core.c b/drivers/media/platform/s3c-camif/camif-core.c
index 4c3c00d59c92..e1d51fd3e700 100644
--- a/drivers/media/platform/s3c-camif/camif-core.c
+++ b/drivers/media/platform/s3c-camif/camif-core.c
@@ -460,9 +460,9 @@ static int s3c_camif_probe(struct platform_device *pdev)

pm_runtime_enable(dev);

- ret = pm_runtime_get_sync(dev);
+ ret = pm_runtime_resume_and_get(dev);
if (ret < 0)
- goto err_pm;
+ goto err_disable;

ret = camif_media_dev_init(camif);
if (ret < 0)
@@ -502,6 +502,7 @@ static int s3c_camif_probe(struct platform_device *pdev)
camif_unregister_media_entities(camif);
err_pm:
pm_runtime_put(dev);
+err_disable:
pm_runtime_disable(dev);
camif_clk_put(camif);
err_clk:
--
2.30.2

2021-05-06 19:19:54

by Mauro Carvalho Chehab

[permalink] [raw]
Subject: [PATCH v5 25/25] media: hantro: use pm_runtime_resume_and_get()

Commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
added pm_runtime_resume_and_get() in order to automatically handle
dev->power.usage_count decrement on errors.

While there's nothing wrong with the current usage on this driver,
as we're getting rid of the pm_runtime_get_sync() call all over
the media subsystem, let's remove the last occurrence on this
driver.

Reviewed-by: Ezequiel Garcia <[email protected]>
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/staging/media/hantro/hantro_drv.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/media/hantro/hantro_drv.c b/drivers/staging/media/hantro/hantro_drv.c
index eea2009fa17b..74d6545395f9 100644
--- a/drivers/staging/media/hantro/hantro_drv.c
+++ b/drivers/staging/media/hantro/hantro_drv.c
@@ -160,9 +160,8 @@ static void device_run(void *priv)
src = hantro_get_src_buf(ctx);
dst = hantro_get_dst_buf(ctx);

- ret = pm_runtime_get_sync(ctx->dev->dev);
- if (ret < 0) {
- pm_runtime_put_noidle(ctx->dev->dev);
+ ret = pm_runtime_resume_and_get(ctx->dev->dev);
+ if (ret < 0)
goto err_cancel_job;
}

--
2.30.2

2021-05-06 19:19:55

by Mauro Carvalho Chehab

[permalink] [raw]
Subject: [PATCH v5 06/25] staging: media: tegra-video: use pm_runtime_resume_and_get()

Commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
added pm_runtime_resume_and_get() in order to automatically handle
dev->power.usage_count decrement on errors.

Use the new API, in order to cleanup the error check logic.

Reviewed-by: Jonathan Cameron <[email protected]>
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/staging/media/tegra-video/csi.c | 3 +--
drivers/staging/media/tegra-video/vi.c | 3 +--
2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/media/tegra-video/csi.c b/drivers/staging/media/tegra-video/csi.c
index 033a6935c26d..e938bf4c48b6 100644
--- a/drivers/staging/media/tegra-video/csi.c
+++ b/drivers/staging/media/tegra-video/csi.c
@@ -298,10 +298,9 @@ static int tegra_csi_enable_stream(struct v4l2_subdev *subdev)
struct tegra_csi *csi = csi_chan->csi;
int ret, err;

- ret = pm_runtime_get_sync(csi->dev);
+ ret = pm_runtime_resume_and_get(csi->dev);
if (ret < 0) {
dev_err(csi->dev, "failed to get runtime PM: %d\n", ret);
- pm_runtime_put_noidle(csi->dev);
return ret;
}

diff --git a/drivers/staging/media/tegra-video/vi.c b/drivers/staging/media/tegra-video/vi.c
index 7a09061cda57..1298740a9c6c 100644
--- a/drivers/staging/media/tegra-video/vi.c
+++ b/drivers/staging/media/tegra-video/vi.c
@@ -297,10 +297,9 @@ static int tegra_channel_start_streaming(struct vb2_queue *vq, u32 count)
struct tegra_vi_channel *chan = vb2_get_drv_priv(vq);
int ret;

- ret = pm_runtime_get_sync(chan->vi->dev);
+ ret = pm_runtime_resume_and_get(chan->vi->dev);
if (ret < 0) {
dev_err(chan->vi->dev, "failed to get runtime PM: %d\n", ret);
- pm_runtime_put_noidle(chan->vi->dev);
return ret;
}

--
2.30.2

2021-05-06 19:20:33

by Mauro Carvalho Chehab

[permalink] [raw]
Subject: [PATCH v5 11/25] media: mtk-jpeg: use pm_runtime_resume_and_get()

Commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
added pm_runtime_resume_and_get() in order to automatically handle
dev->power.usage_count decrement on errors.

Use the new API, in order to cleanup the error check logic.

Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
index 88a23bce569d..a89c7b206eef 100644
--- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
+++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
@@ -920,7 +920,7 @@ static void mtk_jpeg_enc_device_run(void *priv)
src_buf = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
dst_buf = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);

- ret = pm_runtime_get_sync(jpeg->dev);
+ ret = pm_runtime_resume_and_get(jpeg->dev);
if (ret < 0)
goto enc_end;

@@ -973,7 +973,7 @@ static void mtk_jpeg_dec_device_run(void *priv)
return;
}

- ret = pm_runtime_get_sync(jpeg->dev);
+ ret = pm_runtime_resume_and_get(jpeg->dev);
if (ret < 0)
goto dec_end;

--
2.30.2

2021-05-06 19:34:17

by Laurent Pinchart

[permalink] [raw]
Subject: Re: [PATCH v5 23/25] media: vsp1: use pm_runtime_resume_and_get()

Hi Mauro,

Thank you for the patch.

On Thu, May 06, 2021 at 05:26:01PM +0200, Mauro Carvalho Chehab wrote:
> Commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
> added pm_runtime_resume_and_get() in order to automatically handle
> dev->power.usage_count decrement on errors.
>
> Use the new API, in order to cleanup the error check logic.
>
> As a bonus, pm_runtime_resume_and_get() always return 0 on success.
> So, the code can be simplified.
>
> Signed-off-by: Mauro Carvalho Chehab <[email protected]>

Reviewed-by: Laurent Pinchart <[email protected]>

> ---
> drivers/media/platform/vsp1/vsp1_drv.c | 10 +---------
> 1 file changed, 1 insertion(+), 9 deletions(-)
>
> diff --git a/drivers/media/platform/vsp1/vsp1_drv.c b/drivers/media/platform/vsp1/vsp1_drv.c
> index aa66e4f5f3f3..de442d6c9926 100644
> --- a/drivers/media/platform/vsp1/vsp1_drv.c
> +++ b/drivers/media/platform/vsp1/vsp1_drv.c
> @@ -559,15 +559,7 @@ static int vsp1_device_init(struct vsp1_device *vsp1)
> */
> int vsp1_device_get(struct vsp1_device *vsp1)
> {
> - int ret;
> -
> - ret = pm_runtime_get_sync(vsp1->dev);
> - if (ret < 0) {
> - pm_runtime_put_noidle(vsp1->dev);
> - return ret;
> - }
> -
> - return 0;
> + return pm_runtime_resume_and_get(vsp1->dev);
> }
>
> /*

--
Regards,

Laurent Pinchart

2021-05-07 06:37:10

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH v5 05/25] staging: media: tegra-vde: use pm_runtime_resume_and_get()

On Thu, May 06, 2021 at 05:25:43PM +0200, Mauro Carvalho Chehab wrote:
> @@ -1069,11 +1071,19 @@ static int tegra_vde_probe(struct platform_device *pdev)
> * power-cycle it in order to put hardware into a predictable lower
> * power state.
> */
> - pm_runtime_get_sync(dev);
> + if (pm_runtime_resume_and_get(dev) < 0)
> + goto err_pm_runtime;

Needs an error code on this path. These days the kbuild bot will send
a warning for this eventually.

> +
> pm_runtime_put(dev);
>
> return 0;
>
> +err_pm_runtime:
> + misc_deregister(&vde->miscdev);
> +
> + pm_runtime_dont_use_autosuspend(dev);
> + pm_runtime_disable(dev);
> +
> err_deinit_iommu:
> tegra_vde_iommu_deinit(vde);
>

regards,
dan carpenter

2021-05-10 13:03:43

by Dmitry Osipenko

[permalink] [raw]
Subject: Re: [PATCH v5 05/25] staging: media: tegra-vde: use pm_runtime_resume_and_get()

07.05.2021 08:13, Dan Carpenter пишет:
> On Thu, May 06, 2021 at 05:25:43PM +0200, Mauro Carvalho Chehab wrote:
>> @@ -1069,11 +1071,19 @@ static int tegra_vde_probe(struct platform_device *pdev)
>> * power-cycle it in order to put hardware into a predictable lower
>> * power state.
>> */
>> - pm_runtime_get_sync(dev);
>> + if (pm_runtime_resume_and_get(dev) < 0)
>> + goto err_pm_runtime;
>
> Needs an error code on this path. These days the kbuild bot will send
> a warning for this eventually.

Good catch, thank you.