Re: [RFC PATCH v1 3/5] media: tegra-video: Move PM runtime handle to streaming

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 4/28/20 8:15 AM, Dmitry Osipenko wrote:
External email: Use caution opening links or attachments


28.04.2020 18:08, Sowjanya Komatineni пишет:
On 4/28/20 7:59 AM, Dmitry Osipenko wrote:
External email: Use caution opening links or attachments


28.04.2020 17:51, Sowjanya Komatineni пишет:
On 4/28/20 6:59 AM, Dmitry Osipenko wrote:
External email: Use caution opening links or attachments


28.04.2020 07:20, Sowjanya Komatineni пишет:
diff --git a/drivers/staging/media/tegra-video/csi.c
b/drivers/staging/media/tegra-video/csi.c
index b3dd0c3..29ccdae 100644
--- a/drivers/staging/media/tegra-video/csi.c
+++ b/drivers/staging/media/tegra-video/csi.c
@@ -272,8 +272,25 @@ static int tegra_csi_s_stream(struct v4l2_subdev
*subdev, int enable)
         struct tegra_vi_channel *chan =
v4l2_get_subdev_hostdata(subdev);
         struct tegra_csi_channel *csi_chan = to_csi_chan(subdev);
         struct tegra_csi *csi = csi_chan->csi;
+     int ret;
+
+     if (enable && atomic_add_return(1, &csi->clk_refcnt) == 1) {
+             ret = pm_runtime_get_sync(csi->dev);
+             if (ret < 0) {
+                     dev_err(csi->dev,
+                             "failed to get runtime PM: %d\n", ret);
+                     pm_runtime_put_noidle(csi->dev);
Why this pm_runtime_put_noidle() is needed? This should be wrong, please
remove it.

pm_runtime_get_sync() increments power.usage_count prior to rpm_resume

if rpm_resume fails it does not decrement usage_count.

So to balance count on failure, calling pm_runtime_put_noidle()


+                     atomic_dec(&csi->clk_refcnt);
+                     return ret;
+             }
+     }
+
+     ret = csi->ops->csi_streaming(csi_chan, chan->pg_mode, enable);

-     return csi->ops->csi_streaming(csi_chan, chan->pg_mode, enable);
+     if ((ret < 0 || !enable) &&
atomic_dec_and_test(&csi->clk_refcnt))
+             pm_runtime_put_sync(csi->dev);
Runtime PM maintains its own refcount, why these
clk_refcnt/power_on_refcnt are needed?
Streaming is per channel and we can't turn power/clocks off while other
channels may still be running.

All channels use the same CSI device. You should remove the custom
refcounting.

BTW, next time you'll really need to do refcounting, use the generic
kref.
Before channel stream we enable power/clocks and after streaming we stop.

So without refcount, on stream stop of any of the channel RPM put turns
power/clock but other channels will still be streaming.

Runtime PM uses its own refcounting. Please consult the RPM code and
documentation.

https://elixir.bootlin.com/linux/v5.7-rc3/source/include/linux/pm_runtime.h#L78
https://elixir.bootlin.com/linux/v5.7-rc3/source/drivers/base/power/runtime.c#L1079
Got it. Will remove refcount for RPM put/get




[Index of Archives]     [ARM Kernel]     [Linux ARM]     [Linux ARM MSM]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux