* [PATCH v2 00/79] Address some issues with PM runtime at media subsystem
@ 2021-04-27 10:12 Mauro Carvalho Chehab
2021-04-27 10:12 ` [PATCH v2 20/79] staging: media: rkvdec: fix pm_runtime_get_sync() usage count Mauro Carvalho Chehab
` (8 more replies)
0 siblings, 9 replies; 10+ messages in thread
From: Mauro Carvalho Chehab @ 2021-04-27 10:12 UTC (permalink / raw)
Cc: Shawn Tu, Ricardo Ribalda, Dafna Hirschfeld, Heiko Stuebner,
linuxarm, Todor Tomov, Bjorn Andersson, Andrzej Hajda, Lad,
Prabhakar, Thierry Reding, Pengutronix Kernel Team,
Dmitry Osipenko, linux-stm32, Andrzej Pietrasiewicz, Leon Luo,
Paul Kocialkowski, Mauro Carvalho Chehab, Dave Stevenson,
Mauro Carvalho Chehab, Matt Ranostay, Krzysztof Kozlowski,
Jonathan Hunter, linux-rockchip, Chen-Yu Tsai, Andy Gross,
Matthias Brugger, Dongchun Zhu, Sakari Ailus, Bingbu Cao,
Marek Szyprowski, Shunqian Zheng, Tianshu Qiu, NXP Linux Team,
Philipp Zabel, devel, Jacopo Mondi, Sylwester Nawrocki,
linux-tegra, Alexandre Torgue, Wenyou Yang,
Manivannan Sadhasivam, linux-arm-msm, Sascha Hauer,
Steve Longerbeam, linux-media, Maxime Ripard, Stanimir Varbanov,
Benoit Parrot, Helen Koike, linux-samsung-soc, linux-mediatek,
Jacek Anaszewski, mauro.chehab, Sylwester Nawrocki,
Paul J. Murphy, Ezequiel Garcia, Daniele Alessandrelli,
Chiranjeevi Rapolu, linux-arm-kernel, Jacob Chen, Jernej Skrabec,
Hyungwoo Yang, linux-kernel, Robert Foss, Dan Scally,
Sowjanya Komatineni, Maxime Coquelin, linux-renesas-soc,
Yong Zhi, Shawn Guo
During the review of the patches from unm.edu, one of the patterns
I noticed is the amount of patches trying to fix pm_runtime_get_sync()
calls.
After analyzing the feedback from version 1 of this series, I noticed
a few other weird behaviors at the PM runtime resume code. So, this
series start addressing some bugs and issues at the current code.
Then, it gets rid of pm_runtime_get_sync() at the media subsystem
(with 2 exceptions).
It should be noticed that
Commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
added a new method to does a pm_runtime get, which increments
the usage count only on success.
The rationale of getting rid of pm_runtime_get_sync() is:
1. despite its name, this is actually a PM runtime resume call,
but some developers didn't seem to realize that, as I got this
pattern on some drivers:
pm_runtime_get_sync(&client->dev);
pm_runtime_disable(&client->dev);
pm_runtime_set_suspended(&client->dev);
pm_runtime_put_noidle(&client->dev);
It makes no sense to resume PM just to suspend it again ;-)
2. Usual *_get() methods only increment their use count on success,
but pm_runtime_get_sync() increments it unconditionally. Due to
that, several drivers were mistakenly not calling
pm_runtime_put_noidle() when it fails;
3. The name of the new variant is a lot clearer:
pm_runtime_resume_and_get()
As its same clearly says that this is a PM runtime resume function,
that also increments the usage counter on success;
4. Consistency: we did similar changes subsystem wide with
for instance strlcpy() and strcpy() that got replaced by
strscpy(). Having all drivers using the same known-to-be-safe
methods is a good thing;
5. Prevent newer drivers to copy-and-paste a code that it would
be easier to break if they don't truly understand what's behind
the scenes.
This series replace places pm_runtime_get_sync(), by calling
pm_runtime_resume_and_get() instead.
This should help to avoid future mistakes like that, as people
tend to use the existing drivers as examples for newer ones.
compile-tested only.
Patches 1 to 7 fix some issues that already exists at the current
PM runtime code;
patches 8 to 20 fix some usage_count problems that still exists
at the media subsystem;
patches 21 to 78 repaces pm_runtime_get_sync() by
pm_runtime_resume_and_get();
Patch 79 (and a hunk on patch 78) documents the two exceptions
where pm_runtime_get_sync() will still be used for now.
Mauro Carvalho Chehab (79):
media: venus: fix PM runtime logic at venus_sys_error_handler()
media: i2c: ccs-core: return the right error code at suspend
media: i2c: mt9m001: don't resume at remove time
media: i2c: ov7740: don't resume at remove time
media: i2c: video-i2c: don't resume at remove time
media: exynos-gsc: don't resume at remove time
media: atmel: properly get pm_runtime
media: marvel-ccic: fix some issues when getting pm_runtime
media: mdk-mdp: fix pm_runtime_get_sync() usage count
media: rcar_fdp1: fix pm_runtime_get_sync() usage count
media: rga-buf: use pm_runtime_resume_and_get()
media: renesas-ceu: Properly check for PM errors
media: s5p: fix pm_runtime_get_sync() usage count
media: am437x: fix pm_runtime_get_sync() usage count
media: sh_vou: fix pm_runtime_get_sync() usage count
media: mtk-vcodec: fix pm_runtime_get_sync() usage count
media: s5p-jpeg: fix pm_runtime_get_sync() usage count
media: delta-v4l2: fix pm_runtime_get_sync() usage count
media: sun8i_rotate: fix pm_runtime_get_sync() usage count
staging: media: rkvdec: fix pm_runtime_get_sync() usage count
staging: media: atomisp_fops: use pm_runtime_resume_and_get()
staging: media: imx7-mipi-csis: 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: vde: use pm_runtime_resume_and_get()
staging: media: csi: use pm_runtime_resume_and_get()
staging: media: vi: use pm_runtime_resume_and_get()
media: i2c: ak7375: use pm_runtime_resume_and_get()
media: i2c: ccs-core: use pm_runtime_resume_and_get()
media: i2c: dw9714: use pm_runtime_resume_and_get()
media: i2c: dw9768: use pm_runtime_resume_and_get()
media: i2c: dw9807-vcm: use pm_runtime_resume_and_get()
media: i2c: hi556: use pm_runtime_resume_and_get()
media: i2c: imx214: use pm_runtime_resume_and_get()
media: i2c: imx219: use pm_runtime_resume_and_get()
media: i2c: imx258: use pm_runtime_resume_and_get()
media: i2c: imx274: use pm_runtime_resume_and_get()
media: i2c: imx290: use pm_runtime_resume_and_get()
media: i2c: imx319: use pm_runtime_resume_and_get()
media: i2c: imx334: use pm_runtime_resume_and_get()
media: i2c: imx355: use pm_runtime_resume_and_get()
media: i2c: mt9m001: use pm_runtime_resume_and_get()
media: i2c: ov02a10: use pm_runtime_resume_and_get()
media: i2c: ov13858: use pm_runtime_resume_and_get()
media: i2c: ov2659: use pm_runtime_resume_and_get()
media: i2c: ov2685: use pm_runtime_resume_and_get()
media: i2c: ov2740: use pm_runtime_resume_and_get()
media: i2c: ov5647: use pm_runtime_resume_and_get()
media: i2c: ov5648: use pm_runtime_resume_and_get()
media: i2c: ov5670: use pm_runtime_resume_and_get()
media: i2c: ov5675: use pm_runtime_resume_and_get()
media: i2c: ov5695: use pm_runtime_resume_and_get()
media: i2c: ov7740: use pm_runtime_resume_and_get()
media: i2c: ov8856: use pm_runtime_resume_and_get()
media: i2c: ov8865: use pm_runtime_resume_and_get()
media: i2c: ov9734: use pm_runtime_resume_and_get()
media: i2c: tvp5150: use pm_runtime_resume_and_get()
media: i2c: video-i2c: 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: exynos4-is: use pm_runtime_resume_and_get()
media: exynos-gsc: 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: 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: bdisp-v4l2: 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: document the usage of pm_runtime_get_sync()
drivers/media/cec/platform/s5p/s5p_cec.c | 5 +++-
drivers/media/i2c/ak7375.c | 10 +------
drivers/media/i2c/ccs/ccs-core.c | 18 +++++-------
drivers/media/i2c/dw9714.c | 10 +------
drivers/media/i2c/dw9768.c | 10 +------
drivers/media/i2c/dw9807-vcm.c | 10 +------
drivers/media/i2c/hi556.c | 3 +-
drivers/media/i2c/imx214.c | 6 ++--
drivers/media/i2c/imx219.c | 6 ++--
drivers/media/i2c/imx258.c | 6 ++--
drivers/media/i2c/imx274.c | 3 +-
drivers/media/i2c/imx290.c | 6 ++--
drivers/media/i2c/imx319.c | 6 ++--
drivers/media/i2c/imx334.c | 5 ++--
drivers/media/i2c/imx355.c | 6 ++--
drivers/media/i2c/mt9m001.c | 8 ++----
drivers/media/i2c/ov02a10.c | 6 ++--
drivers/media/i2c/ov13858.c | 6 ++--
drivers/media/i2c/ov2659.c | 6 ++--
drivers/media/i2c/ov2685.c | 7 ++---
drivers/media/i2c/ov2740.c | 6 ++--
drivers/media/i2c/ov5647.c | 9 +++---
drivers/media/i2c/ov5648.c | 6 ++--
drivers/media/i2c/ov5670.c | 6 ++--
drivers/media/i2c/ov5675.c | 3 +-
drivers/media/i2c/ov5695.c | 6 ++--
drivers/media/i2c/ov7740.c | 8 ++----
drivers/media/i2c/ov8856.c | 3 +-
drivers/media/i2c/ov8865.c | 6 ++--
drivers/media/i2c/ov9734.c | 3 +-
drivers/media/i2c/tvp5150.c | 16 ++---------
drivers/media/i2c/video-i2c.c | 14 +++-------
drivers/media/pci/intel/ipu3/ipu3-cio2-main.c | 3 +-
drivers/media/platform/am437x/am437x-vpfe.c | 22 +++++++++++----
drivers/media/platform/atmel/atmel-isc-base.c | 27 +++++++++++++-----
drivers/media/platform/atmel/atmel-isi.c | 19 ++++++++++---
drivers/media/platform/coda/coda-common.c | 5 ++--
drivers/media/platform/exynos-gsc/gsc-core.c | 3 --
drivers/media/platform/exynos-gsc/gsc-m2m.c | 2 +-
.../media/platform/exynos4-is/fimc-capture.c | 6 ++--
drivers/media/platform/exynos4-is/fimc-is.c | 4 +--
.../platform/exynos4-is/fimc-isp-video.c | 3 +-
drivers/media/platform/exynos4-is/fimc-isp.c | 7 ++---
drivers/media/platform/exynos4-is/fimc-lite.c | 5 ++--
drivers/media/platform/exynos4-is/fimc-m2m.c | 2 +-
drivers/media/platform/exynos4-is/media-dev.c | 8 ++----
drivers/media/platform/exynos4-is/mipi-csis.c | 8 ++----
.../media/platform/marvell-ccic/mcam-core.c | 9 ++++--
.../media/platform/mtk-jpeg/mtk_jpeg_core.c | 4 +--
drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c | 6 ++--
.../platform/mtk-vcodec/mtk_vcodec_dec_pm.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 ++--
drivers/media/platform/qcom/venus/core.c | 28 +++++++++++--------
.../media/platform/qcom/venus/pm_helpers.c | 10 +++----
drivers/media/platform/qcom/venus/vdec.c | 4 +--
drivers/media/platform/qcom/venus/venc.c | 5 ++--
drivers/media/platform/rcar-fcp.c | 6 ++--
drivers/media/platform/rcar-vin/rcar-csi2.c | 6 ++++
drivers/media/platform/rcar-vin/rcar-dma.c | 6 ++--
drivers/media/platform/rcar-vin/rcar-v4l2.c | 6 ++--
drivers/media/platform/rcar_fdp1.c | 12 ++++++--
drivers/media/platform/renesas-ceu.c | 4 +--
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-jpeg/jpeg-core.c | 2 +-
drivers/media/platform/s5p-mfc/s5p_mfc_pm.c | 6 ++--
drivers/media/platform/sh_vou.c | 6 +++-
drivers/media/platform/sti/bdisp/bdisp-v4l2.c | 7 +++--
drivers/media/platform/sti/delta/delta-v4l2.c | 4 +--
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 ++--
.../sunxi/sun8i-rotate/sun8i_rotate.c | 2 +-
drivers/media/platform/ti-vpe/cal-video.c | 4 ++-
drivers/media/platform/ti-vpe/cal.c | 8 ++++--
drivers/media/platform/ti-vpe/vpe.c | 4 +--
drivers/media/platform/vsp1/vsp1_drv.c | 6 ++--
.../staging/media/atomisp/pci/atomisp_fops.c | 6 ++--
drivers/staging/media/hantro/hantro_drv.c | 7 +++++
drivers/staging/media/imx/imx7-mipi-csis.c | 7 ++---
drivers/staging/media/ipu3/ipu3.c | 3 +-
drivers/staging/media/rkvdec/rkvdec.c | 2 +-
.../staging/media/sunxi/cedrus/cedrus_video.c | 6 ++--
drivers/staging/media/tegra-vde/vde.c | 16 +++++++----
drivers/staging/media/tegra-video/csi.c | 3 +-
drivers/staging/media/tegra-video/vi.c | 3 +-
92 files changed, 298 insertions(+), 335 deletions(-)
--
2.30.2
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 20/79] staging: media: rkvdec: fix pm_runtime_get_sync() usage count
2021-04-27 10:12 [PATCH v2 00/79] Address some issues with PM runtime at media subsystem Mauro Carvalho Chehab
@ 2021-04-27 10:12 ` Mauro Carvalho Chehab
2021-04-27 10:12 ` [PATCH v2 21/79] staging: media: atomisp_fops: use pm_runtime_resume_and_get() Mauro Carvalho Chehab
` (7 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Mauro Carvalho Chehab @ 2021-04-27 10:12 UTC (permalink / raw)
Cc: devel, Mauro Carvalho Chehab, Greg Kroah-Hartman, linuxarm,
linux-kernel, linux-rockchip, mauro.chehab,
Mauro Carvalho Chehab, Ezequiel Garcia, linux-media
The pm_runtime_get_sync() internally increments the
dev->power.usage_count without decrementing it, even on errors.
Replace it by the new pm_runtime_resume_and_get(), introduced by:
commit dd8088d5a896 ("PM: runtime: Add pm_runtime_resume_and_get to deal with usage counter")
in order to properly decrement the usage counter and avoid memory
leaks.
Reviewed-by: Ezequiel Garcia <ezequiel@collabora.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
drivers/staging/media/rkvdec/rkvdec.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/media/rkvdec/rkvdec.c b/drivers/staging/media/rkvdec/rkvdec.c
index d821661d30f3..8c17615f3a7a 100644
--- a/drivers/staging/media/rkvdec/rkvdec.c
+++ b/drivers/staging/media/rkvdec/rkvdec.c
@@ -658,7 +658,7 @@ static void rkvdec_device_run(void *priv)
if (WARN_ON(!desc))
return;
- ret = pm_runtime_get_sync(rkvdec->dev);
+ ret = pm_runtime_resume_and_get(rkvdec->dev);
if (ret < 0) {
rkvdec_job_finish_no_pm(ctx, VB2_BUF_STATE_ERROR);
return;
--
2.30.2
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 21/79] staging: media: atomisp_fops: use pm_runtime_resume_and_get()
2021-04-27 10:12 [PATCH v2 00/79] Address some issues with PM runtime at media subsystem Mauro Carvalho Chehab
2021-04-27 10:12 ` [PATCH v2 20/79] staging: media: rkvdec: fix pm_runtime_get_sync() usage count Mauro Carvalho Chehab
@ 2021-04-27 10:12 ` Mauro Carvalho Chehab
2021-04-27 10:12 ` [PATCH v2 22/79] staging: media: imx7-mipi-csis: " Mauro Carvalho Chehab
` (6 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Mauro Carvalho Chehab @ 2021-04-27 10:12 UTC (permalink / raw)
Cc: devel, Mauro Carvalho Chehab, Greg Kroah-Hartman, linuxarm,
linux-kernel, Sakari Ailus, mauro.chehab, Mauro Carvalho Chehab,
linux-media
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 <mchehab+huawei@kernel.org>
---
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
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 22/79] staging: media: imx7-mipi-csis: use pm_runtime_resume_and_get()
2021-04-27 10:12 [PATCH v2 00/79] Address some issues with PM runtime at media subsystem Mauro Carvalho Chehab
2021-04-27 10:12 ` [PATCH v2 20/79] staging: media: rkvdec: fix pm_runtime_get_sync() usage count Mauro Carvalho Chehab
2021-04-27 10:12 ` [PATCH v2 21/79] staging: media: atomisp_fops: use pm_runtime_resume_and_get() Mauro Carvalho Chehab
@ 2021-04-27 10:12 ` Mauro Carvalho Chehab
2021-04-27 10:12 ` [PATCH v2 23/79] staging: media: ipu3: " Mauro Carvalho Chehab
` (5 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Mauro Carvalho Chehab @ 2021-04-27 10:12 UTC (permalink / raw)
Cc: devel, Philipp Zabel, Mauro Carvalho Chehab, Greg Kroah-Hartman,
Sascha Hauer, linuxarm, linux-kernel, NXP Linux Team,
Pengutronix Kernel Team, Steve Longerbeam, mauro.chehab,
Shawn Guo, Mauro Carvalho Chehab, linux-arm-kernel, linux-media
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 <rmfrfs@gmail.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
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
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 23/79] staging: media: ipu3: use pm_runtime_resume_and_get()
2021-04-27 10:12 [PATCH v2 00/79] Address some issues with PM runtime at media subsystem Mauro Carvalho Chehab
` (2 preceding siblings ...)
2021-04-27 10:12 ` [PATCH v2 22/79] staging: media: imx7-mipi-csis: " Mauro Carvalho Chehab
@ 2021-04-27 10:12 ` Mauro Carvalho Chehab
2021-04-27 10:12 ` [PATCH v2 24/79] staging: media: cedrus_video: " Mauro Carvalho Chehab
` (4 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Mauro Carvalho Chehab @ 2021-04-27 10:12 UTC (permalink / raw)
Cc: devel, Mauro Carvalho Chehab, Greg Kroah-Hartman, linuxarm,
linux-kernel, Sakari Ailus, mauro.chehab, Bingbu Cao,
Mauro Carvalho Chehab, Tianshu Qiu, linux-media
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 <mchehab+huawei@kernel.org>
---
drivers/staging/media/ipu3/ipu3.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/staging/media/ipu3/ipu3.c b/drivers/staging/media/ipu3/ipu3.c
index ee1bba6bdcac..8e1e9e46e604 100644
--- a/drivers/staging/media/ipu3/ipu3.c
+++ b/drivers/staging/media/ipu3/ipu3.c
@@ -392,10 +392,9 @@ int imgu_s_stream(struct imgu_device *imgu, int enable)
}
/* Set Power */
- r = pm_runtime_get_sync(dev);
+ r = pm_runtime_resume_and_get(dev);
if (r < 0) {
dev_err(dev, "failed to set imgu power\n");
- pm_runtime_put(dev);
return r;
}
--
2.30.2
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 24/79] staging: media: cedrus_video: use pm_runtime_resume_and_get()
2021-04-27 10:12 [PATCH v2 00/79] Address some issues with PM runtime at media subsystem Mauro Carvalho Chehab
` (3 preceding siblings ...)
2021-04-27 10:12 ` [PATCH v2 23/79] staging: media: ipu3: " Mauro Carvalho Chehab
@ 2021-04-27 10:12 ` Mauro Carvalho Chehab
2021-04-27 10:12 ` [PATCH v2 25/79] staging: media: vde: " Mauro Carvalho Chehab
` (3 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Mauro Carvalho Chehab @ 2021-04-27 10:12 UTC (permalink / raw)
Cc: devel, Jernej Skrabec, Mauro Carvalho Chehab, Greg Kroah-Hartman,
linuxarm, Maxime Ripard, linux-kernel, Paul Kocialkowski,
Chen-Yu Tsai, mauro.chehab, Mauro Carvalho Chehab,
linux-arm-kernel, linux-media
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 <mchehab+huawei@kernel.org>
---
drivers/staging/media/sunxi/cedrus/cedrus_video.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_video.c b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
index b62eb8e84057..9ddd789d0b1f 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_video.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
@@ -490,11 +490,9 @@ static int cedrus_start_streaming(struct vb2_queue *vq, unsigned int count)
}
if (V4L2_TYPE_IS_OUTPUT(vq->type)) {
- ret = pm_runtime_get_sync(dev->dev);
- if (ret < 0) {
- pm_runtime_put_noidle(dev->dev);
+ ret = pm_runtime_resume_and_get(dev->dev);
+ if (ret < 0)
goto err_cleanup;
- }
if (dev->dec_ops[ctx->current_codec]->start) {
ret = dev->dec_ops[ctx->current_codec]->start(ctx);
--
2.30.2
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 25/79] staging: media: vde: use pm_runtime_resume_and_get()
2021-04-27 10:12 [PATCH v2 00/79] Address some issues with PM runtime at media subsystem Mauro Carvalho Chehab
` (4 preceding siblings ...)
2021-04-27 10:12 ` [PATCH v2 24/79] staging: media: cedrus_video: " Mauro Carvalho Chehab
@ 2021-04-27 10:12 ` Mauro Carvalho Chehab
2021-04-27 10:12 ` [PATCH v2 26/79] staging: media: csi: " Mauro Carvalho Chehab
` (2 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Mauro Carvalho Chehab @ 2021-04-27 10:12 UTC (permalink / raw)
Cc: devel, Mauro Carvalho Chehab, Greg Kroah-Hartman, linuxarm,
Jonathan Hunter, linux-tegra, Thierry Reding, mauro.chehab,
Dmitry Osipenko, Mauro Carvalho Chehab, linux-kernel,
linux-media
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 <mchehab+huawei@kernel.org>
---
drivers/staging/media/tegra-vde/vde.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/media/tegra-vde/vde.c b/drivers/staging/media/tegra-vde/vde.c
index 28845b5bafaf..8936f140a246 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,8 +1071,8 @@ 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);
- pm_runtime_put(dev);
+ if (pm_runtime_resume_and_get(dev) >= 0)
+ pm_runtime_put(dev);
return 0;
@@ -1088,8 +1090,9 @@ static int tegra_vde_remove(struct platform_device *pdev)
{
struct tegra_vde *vde = platform_get_drvdata(pdev);
struct device *dev = &pdev->dev;
+ int ret;
- pm_runtime_get_sync(dev);
+ ret = pm_runtime_resume_and_get(dev);
pm_runtime_dont_use_autosuspend(dev);
pm_runtime_disable(dev);
@@ -1097,7 +1100,8 @@ static int tegra_vde_remove(struct platform_device *pdev)
* Balance RPM state, the VDE power domain is left ON and hardware
* is clock-gated. It's safe to reboot machine now.
*/
- pm_runtime_put_noidle(dev);
+ if (ret >= 0)
+ pm_runtime_put_noidle(dev);
clk_disable_unprepare(vde->clk);
misc_deregister(&vde->miscdev);
--
2.30.2
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 26/79] staging: media: csi: use pm_runtime_resume_and_get()
2021-04-27 10:12 [PATCH v2 00/79] Address some issues with PM runtime at media subsystem Mauro Carvalho Chehab
` (5 preceding siblings ...)
2021-04-27 10:12 ` [PATCH v2 25/79] staging: media: vde: " Mauro Carvalho Chehab
@ 2021-04-27 10:12 ` Mauro Carvalho Chehab
2021-04-27 10:12 ` [PATCH v2 27/79] staging: media: vi: " Mauro Carvalho Chehab
2021-04-27 10:13 ` [PATCH v2 79/79] media: hantro: document the usage of pm_runtime_get_sync() Mauro Carvalho Chehab
8 siblings, 0 replies; 10+ messages in thread
From: Mauro Carvalho Chehab @ 2021-04-27 10:12 UTC (permalink / raw)
Cc: devel, Mauro Carvalho Chehab, Greg Kroah-Hartman, linuxarm,
Jonathan Hunter, linux-tegra, Thierry Reding,
Sowjanya Komatineni, mauro.chehab, Mauro Carvalho Chehab,
linux-kernel, linux-media
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 <mchehab+huawei@kernel.org>
---
drivers/staging/media/tegra-video/csi.c | 3 +--
1 file changed, 1 insertion(+), 2 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;
}
--
2.30.2
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 27/79] staging: media: vi: use pm_runtime_resume_and_get()
2021-04-27 10:12 [PATCH v2 00/79] Address some issues with PM runtime at media subsystem Mauro Carvalho Chehab
` (6 preceding siblings ...)
2021-04-27 10:12 ` [PATCH v2 26/79] staging: media: csi: " Mauro Carvalho Chehab
@ 2021-04-27 10:12 ` Mauro Carvalho Chehab
2021-04-27 10:13 ` [PATCH v2 79/79] media: hantro: document the usage of pm_runtime_get_sync() Mauro Carvalho Chehab
8 siblings, 0 replies; 10+ messages in thread
From: Mauro Carvalho Chehab @ 2021-04-27 10:12 UTC (permalink / raw)
Cc: devel, Mauro Carvalho Chehab, Greg Kroah-Hartman, linuxarm,
Jonathan Hunter, linux-tegra, Thierry Reding,
Sowjanya Komatineni, mauro.chehab, Mauro Carvalho Chehab,
linux-kernel, linux-media
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 <mchehab+huawei@kernel.org>
---
drivers/staging/media/tegra-video/vi.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
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
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 79/79] media: hantro: document the usage of pm_runtime_get_sync()
2021-04-27 10:12 [PATCH v2 00/79] Address some issues with PM runtime at media subsystem Mauro Carvalho Chehab
` (7 preceding siblings ...)
2021-04-27 10:12 ` [PATCH v2 27/79] staging: media: vi: " Mauro Carvalho Chehab
@ 2021-04-27 10:13 ` Mauro Carvalho Chehab
8 siblings, 0 replies; 10+ messages in thread
From: Mauro Carvalho Chehab @ 2021-04-27 10:13 UTC (permalink / raw)
Cc: devel, Mauro Carvalho Chehab, Greg Kroah-Hartman, linuxarm,
linux-kernel, linux-rockchip, Philipp Zabel, mauro.chehab,
Mauro Carvalho Chehab, Ezequiel Garcia, linux-media
Despite other *_get()/*_put() functions, where usage count is
incremented only if not errors, the pm_runtime_get_sync() has
a different behavior, incrementing the counter *even* on
errors.
That's an error prone behavior, as people often forget to
decrement the usage counter.
However, the hantro driver depends on this behavior, as it
will decrement the usage_count unconditionally at the m2m
job finish time, which makes sense.
So, intead of using the pm_runtime_resume_and_get() that
would decrement the counter on error, keep the current
API, but add a documentation explaining the rationale for
keep using pm_runtime_get_sync().
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
drivers/staging/media/hantro/hantro_drv.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/staging/media/hantro/hantro_drv.c b/drivers/staging/media/hantro/hantro_drv.c
index 595e82a82728..96f940c1c85c 100644
--- a/drivers/staging/media/hantro/hantro_drv.c
+++ b/drivers/staging/media/hantro/hantro_drv.c
@@ -155,6 +155,13 @@ static void device_run(void *priv)
ret = clk_bulk_enable(ctx->dev->variant->num_clocks, ctx->dev->clocks);
if (ret)
goto err_cancel_job;
+
+ /*
+ * The pm_runtime_get_sync() will increment dev->power.usage_count,
+ * even on errors. That's the expected behavior here, since the
+ * hantro_job_finish() function at the error handling code
+ * will internally call pm_runtime_put_autosuspend().
+ */
ret = pm_runtime_get_sync(ctx->dev->dev);
if (ret < 0)
goto err_cancel_job;
--
2.30.2
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply related [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-04-27 10:15 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-27 10:12 [PATCH v2 00/79] Address some issues with PM runtime at media subsystem Mauro Carvalho Chehab
2021-04-27 10:12 ` [PATCH v2 20/79] staging: media: rkvdec: fix pm_runtime_get_sync() usage count Mauro Carvalho Chehab
2021-04-27 10:12 ` [PATCH v2 21/79] staging: media: atomisp_fops: use pm_runtime_resume_and_get() Mauro Carvalho Chehab
2021-04-27 10:12 ` [PATCH v2 22/79] staging: media: imx7-mipi-csis: " Mauro Carvalho Chehab
2021-04-27 10:12 ` [PATCH v2 23/79] staging: media: ipu3: " Mauro Carvalho Chehab
2021-04-27 10:12 ` [PATCH v2 24/79] staging: media: cedrus_video: " Mauro Carvalho Chehab
2021-04-27 10:12 ` [PATCH v2 25/79] staging: media: vde: " Mauro Carvalho Chehab
2021-04-27 10:12 ` [PATCH v2 26/79] staging: media: csi: " Mauro Carvalho Chehab
2021-04-27 10:12 ` [PATCH v2 27/79] staging: media: vi: " Mauro Carvalho Chehab
2021-04-27 10:13 ` [PATCH v2 79/79] media: hantro: document the usage of pm_runtime_get_sync() Mauro Carvalho Chehab
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).