driverdev-devel.linuxdriverproject.org archive mirror
 help / color / mirror / Atom feed
* [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).