* [PATCH v3 00/79] Address some issues with PM runtime at media subsystem
@ 2021-04-27 10:25 Mauro Carvalho Chehab
2021-04-27 10:26 ` [PATCH v3 20/79] staging: media: rkvdec: fix pm_runtime_get_sync() usage count Mauro Carvalho Chehab
` (8 more replies)
0 siblings, 9 replies; 18+ messages in thread
From: Mauro Carvalho Chehab @ 2021-04-27 10:25 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.
---
v3: - fix a compilation error;
v2: - addressed pointed issues and fixed a few other PM issues.
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] 18+ messages in thread
* [PATCH v3 20/79] staging: media: rkvdec: fix pm_runtime_get_sync() usage count
2021-04-27 10:25 [PATCH v3 00/79] Address some issues with PM runtime at media subsystem Mauro Carvalho Chehab
@ 2021-04-27 10:26 ` Mauro Carvalho Chehab
2021-04-27 10:26 ` [PATCH v3 21/79] staging: media: atomisp_fops: use pm_runtime_resume_and_get() Mauro Carvalho Chehab
` (7 subsequent siblings)
8 siblings, 0 replies; 18+ messages in thread
From: Mauro Carvalho Chehab @ 2021-04-27 10:26 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] 18+ messages in thread
* [PATCH v3 21/79] staging: media: atomisp_fops: use pm_runtime_resume_and_get()
2021-04-27 10:25 [PATCH v3 00/79] Address some issues with PM runtime at media subsystem Mauro Carvalho Chehab
2021-04-27 10:26 ` [PATCH v3 20/79] staging: media: rkvdec: fix pm_runtime_get_sync() usage count Mauro Carvalho Chehab
@ 2021-04-27 10:26 ` Mauro Carvalho Chehab
2021-04-27 10:26 ` [PATCH v3 22/79] staging: media: imx7-mipi-csis: " Mauro Carvalho Chehab
` (6 subsequent siblings)
8 siblings, 0 replies; 18+ messages in thread
From: Mauro Carvalho Chehab @ 2021-04-27 10:26 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] 18+ messages in thread
* [PATCH v3 22/79] staging: media: imx7-mipi-csis: use pm_runtime_resume_and_get()
2021-04-27 10:25 [PATCH v3 00/79] Address some issues with PM runtime at media subsystem Mauro Carvalho Chehab
2021-04-27 10:26 ` [PATCH v3 20/79] staging: media: rkvdec: fix pm_runtime_get_sync() usage count Mauro Carvalho Chehab
2021-04-27 10:26 ` [PATCH v3 21/79] staging: media: atomisp_fops: use pm_runtime_resume_and_get() Mauro Carvalho Chehab
@ 2021-04-27 10:26 ` Mauro Carvalho Chehab
2021-04-27 10:26 ` [PATCH v3 23/79] staging: media: ipu3: " Mauro Carvalho Chehab
` (5 subsequent siblings)
8 siblings, 0 replies; 18+ messages in thread
From: Mauro Carvalho Chehab @ 2021-04-27 10:26 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] 18+ messages in thread
* [PATCH v3 23/79] staging: media: ipu3: use pm_runtime_resume_and_get()
2021-04-27 10:25 [PATCH v3 00/79] Address some issues with PM runtime at media subsystem Mauro Carvalho Chehab
` (2 preceding siblings ...)
2021-04-27 10:26 ` [PATCH v3 22/79] staging: media: imx7-mipi-csis: " Mauro Carvalho Chehab
@ 2021-04-27 10:26 ` Mauro Carvalho Chehab
2021-04-27 10:26 ` [PATCH v3 24/79] staging: media: cedrus_video: " Mauro Carvalho Chehab
` (4 subsequent siblings)
8 siblings, 0 replies; 18+ messages in thread
From: Mauro Carvalho Chehab @ 2021-04-27 10:26 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] 18+ messages in thread
* [PATCH v3 24/79] staging: media: cedrus_video: use pm_runtime_resume_and_get()
2021-04-27 10:25 [PATCH v3 00/79] Address some issues with PM runtime at media subsystem Mauro Carvalho Chehab
` (3 preceding siblings ...)
2021-04-27 10:26 ` [PATCH v3 23/79] staging: media: ipu3: " Mauro Carvalho Chehab
@ 2021-04-27 10:26 ` Mauro Carvalho Chehab
2021-04-27 10:26 ` [PATCH v3 25/79] staging: media: vde: " Mauro Carvalho Chehab
` (3 subsequent siblings)
8 siblings, 0 replies; 18+ messages in thread
From: Mauro Carvalho Chehab @ 2021-04-27 10:26 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] 18+ messages in thread
* [PATCH v3 25/79] staging: media: vde: use pm_runtime_resume_and_get()
2021-04-27 10:25 [PATCH v3 00/79] Address some issues with PM runtime at media subsystem Mauro Carvalho Chehab
` (4 preceding siblings ...)
2021-04-27 10:26 ` [PATCH v3 24/79] staging: media: cedrus_video: " Mauro Carvalho Chehab
@ 2021-04-27 10:26 ` Mauro Carvalho Chehab
2021-04-27 11:47 ` Dmitry Osipenko
2021-04-27 10:26 ` [PATCH v3 26/79] staging: media: csi: " Mauro Carvalho Chehab
` (2 subsequent siblings)
8 siblings, 1 reply; 18+ messages in thread
From: Mauro Carvalho Chehab @ 2021-04-27 10:26 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] 18+ messages in thread
* [PATCH v3 26/79] staging: media: csi: use pm_runtime_resume_and_get()
2021-04-27 10:25 [PATCH v3 00/79] Address some issues with PM runtime at media subsystem Mauro Carvalho Chehab
` (5 preceding siblings ...)
2021-04-27 10:26 ` [PATCH v3 25/79] staging: media: vde: " Mauro Carvalho Chehab
@ 2021-04-27 10:26 ` Mauro Carvalho Chehab
2021-04-27 10:26 ` [PATCH v3 27/79] staging: media: vi: " Mauro Carvalho Chehab
2021-04-27 10:27 ` [PATCH v3 79/79] media: hantro: document the usage of pm_runtime_get_sync() Mauro Carvalho Chehab
8 siblings, 0 replies; 18+ messages in thread
From: Mauro Carvalho Chehab @ 2021-04-27 10:26 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] 18+ messages in thread
* [PATCH v3 27/79] staging: media: vi: use pm_runtime_resume_and_get()
2021-04-27 10:25 [PATCH v3 00/79] Address some issues with PM runtime at media subsystem Mauro Carvalho Chehab
` (6 preceding siblings ...)
2021-04-27 10:26 ` [PATCH v3 26/79] staging: media: csi: " Mauro Carvalho Chehab
@ 2021-04-27 10:26 ` Mauro Carvalho Chehab
2021-04-27 10:27 ` [PATCH v3 79/79] media: hantro: document the usage of pm_runtime_get_sync() Mauro Carvalho Chehab
8 siblings, 0 replies; 18+ messages in thread
From: Mauro Carvalho Chehab @ 2021-04-27 10:26 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] 18+ messages in thread
* [PATCH v3 79/79] media: hantro: document the usage of pm_runtime_get_sync()
2021-04-27 10:25 [PATCH v3 00/79] Address some issues with PM runtime at media subsystem Mauro Carvalho Chehab
` (7 preceding siblings ...)
2021-04-27 10:26 ` [PATCH v3 27/79] staging: media: vi: " Mauro Carvalho Chehab
@ 2021-04-27 10:27 ` Mauro Carvalho Chehab
2021-04-27 15:08 ` Robin Murphy
8 siblings, 1 reply; 18+ messages in thread
From: Mauro Carvalho Chehab @ 2021-04-27 10:27 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] 18+ messages in thread
* Re: [PATCH v3 25/79] staging: media: vde: use pm_runtime_resume_and_get()
2021-04-27 10:26 ` [PATCH v3 25/79] staging: media: vde: " Mauro Carvalho Chehab
@ 2021-04-27 11:47 ` Dmitry Osipenko
2021-04-28 7:20 ` Mauro Carvalho Chehab
0 siblings, 1 reply; 18+ messages in thread
From: Dmitry Osipenko @ 2021-04-27 11:47 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: devel, Greg Kroah-Hartman, linuxarm, Jonathan Hunter,
linux-tegra, Thierry Reding, mauro.chehab, Mauro Carvalho Chehab,
linux-kernel, linux-media
27.04.2021 13:26, Mauro Carvalho Chehab пишет:
> @@ -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);
Should be cleaner to return error directly here, IMO.
> 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);
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 79/79] media: hantro: document the usage of pm_runtime_get_sync()
2021-04-27 10:27 ` [PATCH v3 79/79] media: hantro: document the usage of pm_runtime_get_sync() Mauro Carvalho Chehab
@ 2021-04-27 15:08 ` Robin Murphy
2021-04-27 15:18 ` Ezequiel Garcia
0 siblings, 1 reply; 18+ messages in thread
From: Robin Murphy @ 2021-04-27 15:08 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: devel, Greg Kroah-Hartman, linuxarm, linux-kernel,
linux-rockchip, Philipp Zabel, mauro.chehab,
Mauro Carvalho Chehab, Ezequiel Garcia, linux-media
On 2021-04-27 11:27, Mauro Carvalho Chehab wrote:
> 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;
..except this can also cause the same pm_runtime_put_autosuspend() call
without even reaching the "matching" get below, so rather than some kind
of cleverness it seems more like it's just broken :/
Robin.
> +
> + /*
> + * 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;
>
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 79/79] media: hantro: document the usage of pm_runtime_get_sync()
2021-04-27 15:08 ` Robin Murphy
@ 2021-04-27 15:18 ` Ezequiel Garcia
2021-04-28 6:27 ` Mauro Carvalho Chehab
0 siblings, 1 reply; 18+ messages in thread
From: Ezequiel Garcia @ 2021-04-27 15:18 UTC (permalink / raw)
To: Robin Murphy, Mauro Carvalho Chehab
Cc: devel, Greg Kroah-Hartman, linuxarm, linux-kernel,
linux-rockchip, Philipp Zabel, mauro.chehab,
Mauro Carvalho Chehab, linux-media
On Tue, 2021-04-27 at 16:08 +0100, Robin Murphy wrote:
> On 2021-04-27 11:27, Mauro Carvalho Chehab wrote:
> > 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;
>
> ..except this can also cause the same pm_runtime_put_autosuspend() call
> without even reaching the "matching" get below, so rather than some kind
> of cleverness it seems more like it's just broken :/
>
Indeed, I was trying to find time to cook a quick patch, but kept
getting preempted.
Feel free to submit a fix for this, otherwise, I'll try to find
time later this week.
Thanks,
Ezequiel
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 79/79] media: hantro: document the usage of pm_runtime_get_sync()
2021-04-27 15:18 ` Ezequiel Garcia
@ 2021-04-28 6:27 ` Mauro Carvalho Chehab
2021-04-28 6:44 ` Mauro Carvalho Chehab
0 siblings, 1 reply; 18+ messages in thread
From: Mauro Carvalho Chehab @ 2021-04-28 6:27 UTC (permalink / raw)
To: Ezequiel Garcia
Cc: devel, Greg Kroah-Hartman, linuxarm, linux-kernel,
linux-rockchip, Philipp Zabel, mauro.chehab,
Mauro Carvalho Chehab, Robin Murphy, linux-media
Em Tue, 27 Apr 2021 12:18:32 -0300
Ezequiel Garcia <ezequiel@collabora.com> escreveu:
> On Tue, 2021-04-27 at 16:08 +0100, Robin Murphy wrote:
> > On 2021-04-27 11:27, Mauro Carvalho Chehab wrote:
> > > 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;
> >
> > ..except this can also cause the same pm_runtime_put_autosuspend() call
> > without even reaching the "matching" get below, so rather than some kind
> > of cleverness it seems more like it's just broken :/
> >
>
> Indeed, I was trying to find time to cook a quick patch, but kept
> getting preempted.
>
> Feel free to submit a fix for this, otherwise, I'll try to find
> time later this week.
What about doing this instead:
diff --git a/drivers/staging/media/hantro/hantro_drv.c b/drivers/staging/media/hantro/hantro_drv.c
index 595e82a82728..67de6b15236d 100644
--- a/drivers/staging/media/hantro/hantro_drv.c
+++ b/drivers/staging/media/hantro/hantro_drv.c
@@ -56,14 +56,12 @@ dma_addr_t hantro_get_ref(struct hantro_ctx *ctx, u64 ts)
return hantro_get_dec_buf_addr(ctx, buf);
}
-static void hantro_job_finish(struct hantro_dev *vpu,
- struct hantro_ctx *ctx,
- enum vb2_buffer_state result)
+static void hantro_job_finish_no_pm(struct hantro_dev *vpu,
+ struct hantro_ctx *ctx,
+ enum vb2_buffer_state result)
{
struct vb2_v4l2_buffer *src, *dst;
- pm_runtime_mark_last_busy(vpu->dev);
- pm_runtime_put_autosuspend(vpu->dev);
clk_bulk_disable(vpu->variant->num_clocks, vpu->clocks);
src = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
@@ -81,6 +79,16 @@ static void hantro_job_finish(struct hantro_dev *vpu,
result);
}
+static void hantro_job_finish(struct hantro_dev *vpu,
+ struct hantro_ctx *ctx,
+ enum vb2_buffer_state result)
+{
+ pm_runtime_mark_last_busy(vpu->dev);
+ pm_runtime_put_autosuspend(vpu->dev);
+
+ hantro_job_finish_no_pm(vpu, ctx, result);
+}
+
void hantro_irq_done(struct hantro_dev *vpu,
enum vb2_buffer_state result)
{
@@ -152,12 +160,13 @@ static void device_run(void *priv)
src = hantro_get_src_buf(ctx);
dst = hantro_get_dst_buf(ctx);
+ ret = pm_runtime_resume_and_get(ctx->dev->dev);
+ if (ret < 0)
+ goto err_cancel_job;
+
ret = clk_bulk_enable(ctx->dev->variant->num_clocks, ctx->dev->clocks);
if (ret)
goto err_cancel_job;
- ret = pm_runtime_get_sync(ctx->dev->dev);
- if (ret < 0)
- goto err_cancel_job;
v4l2_m2m_buf_copy_metadata(src, dst, true);
@@ -165,7 +174,7 @@ static void device_run(void *priv)
return;
err_cancel_job:
- hantro_job_finish(ctx->dev, ctx, VB2_BUF_STATE_ERROR);
+ hantro_job_finish_no_pm(ctx->dev, ctx, VB2_BUF_STATE_ERROR);
}
static struct v4l2_m2m_ops vpu_m2m_ops = {
Thanks,
Mauro
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v3 79/79] media: hantro: document the usage of pm_runtime_get_sync()
2021-04-28 6:27 ` Mauro Carvalho Chehab
@ 2021-04-28 6:44 ` Mauro Carvalho Chehab
2021-04-28 14:14 ` Ezequiel Garcia
0 siblings, 1 reply; 18+ messages in thread
From: Mauro Carvalho Chehab @ 2021-04-28 6:44 UTC (permalink / raw)
To: Ezequiel Garcia
Cc: devel, Greg Kroah-Hartman, linuxarm, linux-kernel,
linux-rockchip, Philipp Zabel, mauro.chehab,
Mauro Carvalho Chehab, Robin Murphy, linux-media
Em Wed, 28 Apr 2021 08:27:42 +0200
Mauro Carvalho Chehab <mchehab+huawei@kernel.org> escreveu:
> Em Tue, 27 Apr 2021 12:18:32 -0300
> Ezequiel Garcia <ezequiel@collabora.com> escreveu:
>
> > On Tue, 2021-04-27 at 16:08 +0100, Robin Murphy wrote:
> > > On 2021-04-27 11:27, Mauro Carvalho Chehab wrote:
> > > > 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;
> > >
> > > ..except this can also cause the same pm_runtime_put_autosuspend() call
> > > without even reaching the "matching" get below, so rather than some kind
> > > of cleverness it seems more like it's just broken :/
> > >
> >
> > Indeed, I was trying to find time to cook a quick patch, but kept
> > getting preempted.
> >
> > Feel free to submit a fix for this, otherwise, I'll try to find
> > time later this week.
>
> What about doing this instead:
>
> diff --git a/drivers/staging/media/hantro/hantro_drv.c b/drivers/staging/media/hantro/hantro_drv.c
> index 595e82a82728..67de6b15236d 100644
> --- a/drivers/staging/media/hantro/hantro_drv.c
> +++ b/drivers/staging/media/hantro/hantro_drv.c
> @@ -56,14 +56,12 @@ dma_addr_t hantro_get_ref(struct hantro_ctx *ctx, u64 ts)
> return hantro_get_dec_buf_addr(ctx, buf);
> }
>
> -static void hantro_job_finish(struct hantro_dev *vpu,
> - struct hantro_ctx *ctx,
> - enum vb2_buffer_state result)
> +static void hantro_job_finish_no_pm(struct hantro_dev *vpu,
> + struct hantro_ctx *ctx,
> + enum vb2_buffer_state result)
> {
> struct vb2_v4l2_buffer *src, *dst;
>
> - pm_runtime_mark_last_busy(vpu->dev);
> - pm_runtime_put_autosuspend(vpu->dev);
> clk_bulk_disable(vpu->variant->num_clocks, vpu->clocks);
>
> src = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
> @@ -81,6 +79,16 @@ static void hantro_job_finish(struct hantro_dev *vpu,
> result);
> }
>
> +static void hantro_job_finish(struct hantro_dev *vpu,
> + struct hantro_ctx *ctx,
> + enum vb2_buffer_state result)
> +{
> + pm_runtime_mark_last_busy(vpu->dev);
> + pm_runtime_put_autosuspend(vpu->dev);
> +
> + hantro_job_finish_no_pm(vpu, ctx, result);
> +}
> +
> void hantro_irq_done(struct hantro_dev *vpu,
> enum vb2_buffer_state result)
> {
> @@ -152,12 +160,13 @@ static void device_run(void *priv)
> src = hantro_get_src_buf(ctx);
> dst = hantro_get_dst_buf(ctx);
>
> + ret = pm_runtime_resume_and_get(ctx->dev->dev);
> + if (ret < 0)
> + goto err_cancel_job;
> +
> ret = clk_bulk_enable(ctx->dev->variant->num_clocks, ctx->dev->clocks);
> if (ret)
> goto err_cancel_job;
> - ret = pm_runtime_get_sync(ctx->dev->dev);
> - if (ret < 0)
> - goto err_cancel_job;
>
> v4l2_m2m_buf_copy_metadata(src, dst, true);
>
> @@ -165,7 +174,7 @@ static void device_run(void *priv)
> return;
>
> err_cancel_job:
> - hantro_job_finish(ctx->dev, ctx, VB2_BUF_STATE_ERROR);
> + hantro_job_finish_no_pm(ctx->dev, ctx, VB2_BUF_STATE_ERROR);
> }
>
> static struct v4l2_m2m_ops vpu_m2m_ops = {
>
> Thanks,
> Mauro
Actually, the order at the finish logic should change as well.
Maybe like this:
<snip>
static void hantro_job_finish_no_pm(struct hantro_dev *vpu,
struct hantro_ctx *ctx,
enum vb2_buffer_state result)
{
struct vb2_v4l2_buffer *src, *dst;
src = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
dst = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
if (WARN_ON(!src))
return;
if (WARN_ON(!dst))
return;
src->sequence = ctx->sequence_out++;
dst->sequence = ctx->sequence_cap++;
v4l2_m2m_buf_done_and_job_finish(ctx->dev->m2m_dev, ctx->fh.m2m_ctx,
result);
}
static void hantro_job_finish(struct hantro_dev *vpu,
struct hantro_ctx *ctx,
enum vb2_buffer_state result)
{
hantro_job_finish_no_pm(vpu, ctx, result);
clk_bulk_disable(vpu->variant->num_clocks, vpu->clocks);
pm_runtime_mark_last_busy(vpu->dev);
pm_runtime_put_autosuspend(vpu->dev);
}
static void device_run(void *priv)
{
struct hantro_ctx *ctx = priv;
struct vb2_v4l2_buffer *src, *dst;
int ret;
src = hantro_get_src_buf(ctx);
dst = hantro_get_dst_buf(ctx);
ret = pm_runtime_resume_and_get(ctx->dev->dev);
if (ret < 0)
goto err_cancel_job;
ret = clk_bulk_enable(ctx->dev->variant->num_clocks, ctx->dev->clocks);
if (ret)
goto err_cancel_job;
v4l2_m2m_buf_copy_metadata(src, dst, true);
ctx->codec_ops->run(ctx);
return;
err_cancel_job:
hantro_job_finish_no_pm(ctx->dev, ctx, VB2_BUF_STATE_ERROR);
}
</snip>
Thanks,
Mauro
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 25/79] staging: media: vde: use pm_runtime_resume_and_get()
2021-04-27 11:47 ` Dmitry Osipenko
@ 2021-04-28 7:20 ` Mauro Carvalho Chehab
2021-04-28 8:05 ` Dmitry Osipenko
0 siblings, 1 reply; 18+ messages in thread
From: Mauro Carvalho Chehab @ 2021-04-28 7:20 UTC (permalink / raw)
To: Dmitry Osipenko
Cc: devel, Greg Kroah-Hartman, linuxarm, Jonathan Hunter,
linux-tegra, Thierry Reding, mauro.chehab, Mauro Carvalho Chehab,
linux-kernel, linux-media
Em Tue, 27 Apr 2021 14:47:01 +0300
Dmitry Osipenko <digetx@gmail.com> escreveu:
> 27.04.2021 13:26, Mauro Carvalho Chehab пишет:
> > @@ -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);
>
> Should be cleaner to return error directly here, IMO.
I double-checked how drivers/base/platform.c deals with non-zero
returns at the .remove method:
static int platform_remove(struct device *_dev)
{
struct platform_driver *drv = to_platform_driver(_dev->driver);
struct platform_device *dev = to_platform_device(_dev);
if (drv->remove) {
int ret = drv->remove(dev);
if (ret)
dev_warn(_dev, "remove callback returned a non-zero value. This will be ignored.\n");
}
dev_pm_domain_detach(_dev, true);
return 0;
}
Basically, it will print a message but will ignore whatever happens
afterwards.
So, if the driver is changed to return an error there, it will leak
resources.
Thanks,
Mauro
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 25/79] staging: media: vde: use pm_runtime_resume_and_get()
2021-04-28 7:20 ` Mauro Carvalho Chehab
@ 2021-04-28 8:05 ` Dmitry Osipenko
0 siblings, 0 replies; 18+ messages in thread
From: Dmitry Osipenko @ 2021-04-28 8:05 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: devel, Greg Kroah-Hartman, linuxarm, Jonathan Hunter,
linux-tegra, Thierry Reding, mauro.chehab, Mauro Carvalho Chehab,
linux-kernel, linux-media
28.04.2021 10:20, Mauro Carvalho Chehab пишет:
> Em Tue, 27 Apr 2021 14:47:01 +0300
> Dmitry Osipenko <digetx@gmail.com> escreveu:
>
>> 27.04.2021 13:26, Mauro Carvalho Chehab пишет:
>>> @@ -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);
>>
>> Should be cleaner to return error directly here, IMO.
>
> I double-checked how drivers/base/platform.c deals with non-zero
> returns at the .remove method:
>
> static int platform_remove(struct device *_dev)
> {
> struct platform_driver *drv = to_platform_driver(_dev->driver);
> struct platform_device *dev = to_platform_device(_dev);
>
> if (drv->remove) {
> int ret = drv->remove(dev);
>
> if (ret)
> dev_warn(_dev, "remove callback returned a non-zero value. This will be ignored.\n");
> }
> dev_pm_domain_detach(_dev, true);
>
> return 0;
> }
>
> Basically, it will print a message but will ignore whatever happens
> afterwards.
>
> So, if the driver is changed to return an error there, it will leak
> resources.
Indeed, thank you. But then the pm_runtime_get_sync() should be more
appropriate since this function is specifically made for such cases
where returned value is ignored.
A better option could be better to add a clarifying comment to the code
rather than to change it to a variant which introduces confusion, IMO.
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3 79/79] media: hantro: document the usage of pm_runtime_get_sync()
2021-04-28 6:44 ` Mauro Carvalho Chehab
@ 2021-04-28 14:14 ` Ezequiel Garcia
0 siblings, 0 replies; 18+ messages in thread
From: Ezequiel Garcia @ 2021-04-28 14:14 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: devel, Greg Kroah-Hartman, linuxarm, linux-kernel,
linux-rockchip, Philipp Zabel, mauro.chehab,
Mauro Carvalho Chehab, Robin Murphy, linux-media
Hi Mauro,
On Wed, 2021-04-28 at 08:44 +0200, Mauro Carvalho Chehab wrote:
> Em Wed, 28 Apr 2021 08:27:42 +0200
> Mauro Carvalho Chehab <mchehab+huawei@kernel.org> escreveu:
>
> > Em Tue, 27 Apr 2021 12:18:32 -0300
> > Ezequiel Garcia <ezequiel@collabora.com> escreveu:
> >
> > > On Tue, 2021-04-27 at 16:08 +0100, Robin Murphy wrote:
> > > > On 2021-04-27 11:27, Mauro Carvalho Chehab wrote:
> > > > > 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;
> > > >
> > > > ..except this can also cause the same pm_runtime_put_autosuspend() call
> > > > without even reaching the "matching" get below, so rather than some kind
> > > > of cleverness it seems more like it's just broken :/
> > > >
> > >
> > > Indeed, I was trying to find time to cook a quick patch, but kept
> > > getting preempted.
> > >
> > > Feel free to submit a fix for this, otherwise, I'll try to find
> > > time later this week.
> >
> > What about doing this instead:
> >
> > diff --git a/drivers/staging/media/hantro/hantro_drv.c b/drivers/staging/media/hantro/hantro_drv.c
> > index 595e82a82728..67de6b15236d 100644
> > --- a/drivers/staging/media/hantro/hantro_drv.c
> > +++ b/drivers/staging/media/hantro/hantro_drv.c
> > @@ -56,14 +56,12 @@ dma_addr_t hantro_get_ref(struct hantro_ctx *ctx, u64 ts)
> > return hantro_get_dec_buf_addr(ctx, buf);
> > }
> >
> > -static void hantro_job_finish(struct hantro_dev *vpu,
> > - struct hantro_ctx *ctx,
> > - enum vb2_buffer_state result)
> > +static void hantro_job_finish_no_pm(struct hantro_dev *vpu,
> > + struct hantro_ctx *ctx,
> > + enum vb2_buffer_state result)
> > {
> > struct vb2_v4l2_buffer *src, *dst;
> >
> > - pm_runtime_mark_last_busy(vpu->dev);
> > - pm_runtime_put_autosuspend(vpu->dev);
> > clk_bulk_disable(vpu->variant->num_clocks, vpu->clocks);
> >
> > src = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
> > @@ -81,6 +79,16 @@ static void hantro_job_finish(struct hantro_dev *vpu,
> > result);
> > }
> >
> > +static void hantro_job_finish(struct hantro_dev *vpu,
> > + struct hantro_ctx *ctx,
> > + enum vb2_buffer_state result)
> > +{
> > + pm_runtime_mark_last_busy(vpu->dev);
> > + pm_runtime_put_autosuspend(vpu->dev);
> > +
> > + hantro_job_finish_no_pm(vpu, ctx, result);
> > +}
> > +
> > void hantro_irq_done(struct hantro_dev *vpu,
> > enum vb2_buffer_state result)
> > {
> > @@ -152,12 +160,13 @@ static void device_run(void *priv)
> > src = hantro_get_src_buf(ctx);
> > dst = hantro_get_dst_buf(ctx);
> >
> > + ret = pm_runtime_resume_and_get(ctx->dev->dev);
> > + if (ret < 0)
> > + goto err_cancel_job;
> > +
> > ret = clk_bulk_enable(ctx->dev->variant->num_clocks, ctx->dev->clocks);
> > if (ret)
> > goto err_cancel_job;
> > - ret = pm_runtime_get_sync(ctx->dev->dev);
> > - if (ret < 0)
> > - goto err_cancel_job;
> >
> > v4l2_m2m_buf_copy_metadata(src, dst, true);
> >
> > @@ -165,7 +174,7 @@ static void device_run(void *priv)
> > return;
> >
> > err_cancel_job:
> > - hantro_job_finish(ctx->dev, ctx, VB2_BUF_STATE_ERROR);
> > + hantro_job_finish_no_pm(ctx->dev, ctx, VB2_BUF_STATE_ERROR);
> > }
> >
> > static struct v4l2_m2m_ops vpu_m2m_ops = {
> >
> > Thanks,
> > Mauro
>
> Actually, the order at the finish logic should change as well.
> Maybe like this:
>
This one looks good.
Thanks!
Ezequiel
> <snip>
> static void hantro_job_finish_no_pm(struct hantro_dev *vpu,
> struct hantro_ctx *ctx,
> enum vb2_buffer_state result)
> {
> struct vb2_v4l2_buffer *src, *dst;
>
> src = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
> dst = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
>
> if (WARN_ON(!src))
> return;
> if (WARN_ON(!dst))
> return;
>
> src->sequence = ctx->sequence_out++;
> dst->sequence = ctx->sequence_cap++;
>
> v4l2_m2m_buf_done_and_job_finish(ctx->dev->m2m_dev, ctx->fh.m2m_ctx,
> result);
> }
>
> static void hantro_job_finish(struct hantro_dev *vpu,
> struct hantro_ctx *ctx,
> enum vb2_buffer_state result)
> {
>
> hantro_job_finish_no_pm(vpu, ctx, result);
>
> clk_bulk_disable(vpu->variant->num_clocks, vpu->clocks);
>
> pm_runtime_mark_last_busy(vpu->dev);
> pm_runtime_put_autosuspend(vpu->dev);
> }
>
> static void device_run(void *priv)
> {
> struct hantro_ctx *ctx = priv;
> struct vb2_v4l2_buffer *src, *dst;
> int ret;
>
> src = hantro_get_src_buf(ctx);
> dst = hantro_get_dst_buf(ctx);
>
> ret = pm_runtime_resume_and_get(ctx->dev->dev);
> if (ret < 0)
> goto err_cancel_job;
>
> ret = clk_bulk_enable(ctx->dev->variant->num_clocks, ctx->dev->clocks);
> if (ret)
> goto err_cancel_job;
>
> v4l2_m2m_buf_copy_metadata(src, dst, true);
>
> ctx->codec_ops->run(ctx);
> return;
>
> err_cancel_job:
> hantro_job_finish_no_pm(ctx->dev, ctx, VB2_BUF_STATE_ERROR);
> }
> </snip>
>
>
> Thanks,
> Mauro
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2021-04-28 14:15 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-27 10:25 [PATCH v3 00/79] Address some issues with PM runtime at media subsystem Mauro Carvalho Chehab
2021-04-27 10:26 ` [PATCH v3 20/79] staging: media: rkvdec: fix pm_runtime_get_sync() usage count Mauro Carvalho Chehab
2021-04-27 10:26 ` [PATCH v3 21/79] staging: media: atomisp_fops: use pm_runtime_resume_and_get() Mauro Carvalho Chehab
2021-04-27 10:26 ` [PATCH v3 22/79] staging: media: imx7-mipi-csis: " Mauro Carvalho Chehab
2021-04-27 10:26 ` [PATCH v3 23/79] staging: media: ipu3: " Mauro Carvalho Chehab
2021-04-27 10:26 ` [PATCH v3 24/79] staging: media: cedrus_video: " Mauro Carvalho Chehab
2021-04-27 10:26 ` [PATCH v3 25/79] staging: media: vde: " Mauro Carvalho Chehab
2021-04-27 11:47 ` Dmitry Osipenko
2021-04-28 7:20 ` Mauro Carvalho Chehab
2021-04-28 8:05 ` Dmitry Osipenko
2021-04-27 10:26 ` [PATCH v3 26/79] staging: media: csi: " Mauro Carvalho Chehab
2021-04-27 10:26 ` [PATCH v3 27/79] staging: media: vi: " Mauro Carvalho Chehab
2021-04-27 10:27 ` [PATCH v3 79/79] media: hantro: document the usage of pm_runtime_get_sync() Mauro Carvalho Chehab
2021-04-27 15:08 ` Robin Murphy
2021-04-27 15:18 ` Ezequiel Garcia
2021-04-28 6:27 ` Mauro Carvalho Chehab
2021-04-28 6:44 ` Mauro Carvalho Chehab
2021-04-28 14:14 ` Ezequiel Garcia
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).