From: "Sricharan" <sricharan@codeaurora.org> To: "'Marek Szyprowski'" <m.szyprowski@samsung.com>, <linux-pm@vger.kernel.org>, <linux-kernel@vger.kernel.org>, <iommu@lists.linux-foundation.org>, <linux-samsung-soc@vger.kernel.org> Cc: "'Tomeu Vizoso'" <tomeu.vizoso@collabora.com>, "'Bartlomiej Zolnierkiewicz'" <b.zolnierkie@samsung.com>, "'Greg Kroah-Hartman'" <gregkh@linuxfoundation.org>, "'Kevin Hilman'" <khilman@kernel.org>, "'Rafael J. Wysocki'" <rjw@rjwysocki.net>, "'Tomasz Figa'" <tomasz.figa@gmail.com>, "'Krzysztof Kozlowski'" <krzk@kernel.org>, "'Inki Dae'" <inki.dae@samsung.com>, "'Tobias Jakobi'" <tjakobi@math.uni-bielefeld.de>, "'Luis R. Rodriguez'" <mcgrof@kernel.org>, "'Kukjin Kim'" <kgene@kernel.org>, "'Mark Brown'" <broonie@kernel.org>, "'Lukas Wunner'" <lukas@wunner.de> Subject: RE: [PATCH v5 6/7] iommu/exynos: Add runtime pm support Date: Sat, 22 Oct 2016 11:20:12 +0530 [thread overview] Message-ID: <000701d22c28$2da381d0$88ea8570$@codeaurora.org> (raw) In-Reply-To: <1476948173-21093-7-git-send-email-m.szyprowski@samsung.com> Hi Marek, >This patch adds runtime pm implementation, which is based on previous >suspend/resume code. SYSMMU controller is now being enabled/disabled mainly >from the runtime pm callbacks. System sleep callbacks relies on generic >pm_runtime_force_suspend/pm_runtime_force_resume helpers. To ensure >internal state consistency, additional lock for runtime pm transitions >was introduced. > >Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> >--- > drivers/iommu/exynos-iommu.c | 45 +++++++++++++++++++++++++++++++++++--------- > 1 file changed, 36 insertions(+), 9 deletions(-) > >diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c >index a959443e6f33..5e6d7bbf9b70 100644 >--- a/drivers/iommu/exynos-iommu.c >+++ b/drivers/iommu/exynos-iommu.c >@@ -206,6 +206,7 @@ struct sysmmu_fault_info { > struct exynos_iommu_owner { > struct list_head controllers; /* list of sysmmu_drvdata.owner_node */ > struct iommu_domain *domain; /* domain this device is attached */ >+ struct mutex rpm_lock; /* for runtime pm of all sysmmus */ > }; > > /* >@@ -594,40 +595,46 @@ static int __init exynos_sysmmu_probe(struct platform_device *pdev) > return 0; > } > >-#ifdef CONFIG_PM_SLEEP >-static int exynos_sysmmu_suspend(struct device *dev) >+static int __maybe_unused exynos_sysmmu_suspend(struct device *dev) > { > struct sysmmu_drvdata *data = dev_get_drvdata(dev); > struct device *master = data->master; > > if (master) { >- pm_runtime_put(dev); >+ struct exynos_iommu_owner *owner = master->archdata.iommu; >+ >+ mutex_lock(&owner->rpm_lock); More of a device link question, To understand, i see that with device link + runtime, the supplier callbacks are not called for irqsafe clients, even if supplier is irqsafe. Why so ? > if (data->domain) { > dev_dbg(data->sysmmu, "saving state\n"); > __sysmmu_disable(data); > } >+ mutex_unlock(&owner->rpm_lock); > } > return 0; > } > >-static int exynos_sysmmu_resume(struct device *dev) >+static int __maybe_unused exynos_sysmmu_resume(struct device *dev) > { > struct sysmmu_drvdata *data = dev_get_drvdata(dev); > struct device *master = data->master; > > if (master) { >- pm_runtime_get_sync(dev); >+ struct exynos_iommu_owner *owner = master->archdata.iommu; >+ >+ mutex_lock(&owner->rpm_lock); > if (data->domain) { > dev_dbg(data->sysmmu, "restoring state\n"); > __sysmmu_enable(data); > } >+ mutex_unlock(&owner->rpm_lock); > } > return 0; > } >-#endif > > static const struct dev_pm_ops sysmmu_pm_ops = { >- SET_LATE_SYSTEM_SLEEP_PM_OPS(exynos_sysmmu_suspend, exynos_sysmmu_resume) >+ SET_RUNTIME_PM_OPS(exynos_sysmmu_suspend, exynos_sysmmu_resume, NULL) >+ SET_LATE_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend, >+ pm_runtime_force_resume) > }; Is this needed to be LATE_SYSTEM_SLEEP_PM_OPS with device links to take care of the order ? Regards, Sricharan
WARNING: multiple messages have this Message-ID (diff)
From: "Sricharan" <sricharan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> To: 'Marek Szyprowski' <m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>, linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org, linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org Cc: 'Tomeu Vizoso' <tomeu.vizoso-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>, 'Kevin Hilman' <khilman-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>, 'Greg Kroah-Hartman' <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>, 'Mark Brown' <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>, 'Bartlomiej Zolnierkiewicz' <b.zolnierkie-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>, "'Rafael J. Wysocki'" <rjw-LthD3rsA81gm4RdzfppkhA@public.gmane.org>, 'Tomasz Figa' <tomasz.figa-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>, 'Krzysztof Kozlowski' <krzk-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>, 'Inki Dae' <inki.dae-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>, 'Tobias Jakobi' <tjakobi-o02PS0xoJP9W0yFyLvAVXMxlOr/tl8fh@public.gmane.org>, "'Luis R. Rodriguez'" <mcgrof-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>, 'Kukjin Kim' <kgene-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>, 'Lukas Wunner' <lukas-JFq808J9C/izQB+pC5nmwQ@public.gmane.org> Subject: RE: [PATCH v5 6/7] iommu/exynos: Add runtime pm support Date: Sat, 22 Oct 2016 11:20:12 +0530 [thread overview] Message-ID: <000701d22c28$2da381d0$88ea8570$@codeaurora.org> (raw) In-Reply-To: <1476948173-21093-7-git-send-email-m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> Hi Marek, >This patch adds runtime pm implementation, which is based on previous >suspend/resume code. SYSMMU controller is now being enabled/disabled mainly >from the runtime pm callbacks. System sleep callbacks relies on generic >pm_runtime_force_suspend/pm_runtime_force_resume helpers. To ensure >internal state consistency, additional lock for runtime pm transitions >was introduced. > >Signed-off-by: Marek Szyprowski <m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> >--- > drivers/iommu/exynos-iommu.c | 45 +++++++++++++++++++++++++++++++++++--------- > 1 file changed, 36 insertions(+), 9 deletions(-) > >diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c >index a959443e6f33..5e6d7bbf9b70 100644 >--- a/drivers/iommu/exynos-iommu.c >+++ b/drivers/iommu/exynos-iommu.c >@@ -206,6 +206,7 @@ struct sysmmu_fault_info { > struct exynos_iommu_owner { > struct list_head controllers; /* list of sysmmu_drvdata.owner_node */ > struct iommu_domain *domain; /* domain this device is attached */ >+ struct mutex rpm_lock; /* for runtime pm of all sysmmus */ > }; > > /* >@@ -594,40 +595,46 @@ static int __init exynos_sysmmu_probe(struct platform_device *pdev) > return 0; > } > >-#ifdef CONFIG_PM_SLEEP >-static int exynos_sysmmu_suspend(struct device *dev) >+static int __maybe_unused exynos_sysmmu_suspend(struct device *dev) > { > struct sysmmu_drvdata *data = dev_get_drvdata(dev); > struct device *master = data->master; > > if (master) { >- pm_runtime_put(dev); >+ struct exynos_iommu_owner *owner = master->archdata.iommu; >+ >+ mutex_lock(&owner->rpm_lock); More of a device link question, To understand, i see that with device link + runtime, the supplier callbacks are not called for irqsafe clients, even if supplier is irqsafe. Why so ? > if (data->domain) { > dev_dbg(data->sysmmu, "saving state\n"); > __sysmmu_disable(data); > } >+ mutex_unlock(&owner->rpm_lock); > } > return 0; > } > >-static int exynos_sysmmu_resume(struct device *dev) >+static int __maybe_unused exynos_sysmmu_resume(struct device *dev) > { > struct sysmmu_drvdata *data = dev_get_drvdata(dev); > struct device *master = data->master; > > if (master) { >- pm_runtime_get_sync(dev); >+ struct exynos_iommu_owner *owner = master->archdata.iommu; >+ >+ mutex_lock(&owner->rpm_lock); > if (data->domain) { > dev_dbg(data->sysmmu, "restoring state\n"); > __sysmmu_enable(data); > } >+ mutex_unlock(&owner->rpm_lock); > } > return 0; > } >-#endif > > static const struct dev_pm_ops sysmmu_pm_ops = { >- SET_LATE_SYSTEM_SLEEP_PM_OPS(exynos_sysmmu_suspend, exynos_sysmmu_resume) >+ SET_RUNTIME_PM_OPS(exynos_sysmmu_suspend, exynos_sysmmu_resume, NULL) >+ SET_LATE_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend, >+ pm_runtime_force_resume) > }; Is this needed to be LATE_SYSTEM_SLEEP_PM_OPS with device links to take care of the order ? Regards, Sricharan
next prev parent reply other threads:[~2016-10-22 5:50 UTC|newest] Thread overview: 50+ messages / expand[flat|nested] mbox.gz Atom feed top [not found] <CGME20161020072330eucas1p2b09ad8d091171edbac9449815fdc0fb7@eucas1p2.samsung.com> 2016-10-20 7:22 ` [PATCH v5 0/7] Exynos IOMMU: proper runtime PM support (use device dependencies) Marek Szyprowski 2016-10-20 7:22 ` Marek Szyprowski [not found] ` <CGME20161020072331eucas1p1af7dc7270b0b19168b949f3416eda474@eucas1p1.samsung.com> 2016-10-20 7:22 ` [PATCH v5 1/7] iommu/exynos: Remove excessive, useless debug Marek Szyprowski 2016-10-20 7:22 ` Marek Szyprowski [not found] ` <CGME20161020072332eucas1p1d980c1659979bd5bc2918bfc9d40a415@eucas1p1.samsung.com> 2016-10-20 7:22 ` [PATCH v5 2/7] iommu/exynos: Remove dead code Marek Szyprowski 2016-10-20 7:22 ` Marek Szyprowski [not found] ` <CGME20161020072332eucas1p26960035de3007724498d59057329683d@eucas1p2.samsung.com> 2016-10-20 7:22 ` [PATCH v5 3/7] iommu/exynos: Simplify internal enable/disable functions Marek Szyprowski 2016-10-20 7:22 ` Marek Szyprowski [not found] ` <CGME20161020072333eucas1p25b638379091939f10b3c9eb5d89a031e@eucas1p2.samsung.com> 2016-10-20 7:22 ` [PATCH v5 4/7] iommu/exynos: Set master device once on boot Marek Szyprowski [not found] ` <CGME20161020072334eucas1p2a159a25a2875611eff208381ebdb2e84@eucas1p2.samsung.com> 2016-10-20 7:22 ` [PATCH v5 5/7] iommu/exynos: Rework and fix internal locking Marek Szyprowski 2016-10-20 7:22 ` Marek Szyprowski [not found] ` <CGME20161020072335eucas1p209675d6fbf39e5045281e8023fa9d234@eucas1p2.samsung.com> 2016-10-20 7:22 ` [PATCH v5 6/7] iommu/exynos: Add runtime pm support Marek Szyprowski 2016-10-20 7:22 ` Marek Szyprowski 2016-10-22 5:50 ` Sricharan [this message] 2016-10-22 5:50 ` Sricharan 2016-10-24 5:19 ` Marek Szyprowski 2016-10-24 12:15 ` Sricharan 2016-10-24 12:15 ` Sricharan [not found] ` <CGME20161020072336eucas1p24a2b020f69b6ae1f55e1760e6e0e94f9@eucas1p2.samsung.com> 2016-10-20 7:22 ` [PATCH v5 7/7] iommu/exynos: Use device dependency links to control runtime pm Marek Szyprowski 2016-10-23 9:49 ` Sricharan 2016-10-23 9:49 ` Sricharan 2016-10-24 5:30 ` Marek Szyprowski 2016-10-24 5:30 ` Marek Szyprowski 2016-10-24 12:29 ` Sricharan 2016-10-24 12:29 ` Sricharan 2016-10-24 12:39 ` Marek Szyprowski 2016-10-25 6:53 ` Sricharan 2016-10-25 6:53 ` Sricharan 2016-11-07 21:47 ` Luis R. Rodriguez 2016-11-07 21:47 ` Luis R. Rodriguez 2016-11-08 7:27 ` Marek Szyprowski 2016-11-08 7:27 ` Marek Szyprowski 2016-11-08 15:30 ` Lukas Wunner 2016-11-08 15:30 ` Lukas Wunner 2016-11-09 23:55 ` Luis R. Rodriguez 2016-11-09 23:55 ` Luis R. Rodriguez 2016-11-10 0:05 ` Rafael J. Wysocki 2016-11-10 0:05 ` Rafael J. Wysocki 2016-11-10 0:12 ` Luis R. Rodriguez 2016-11-10 0:12 ` Luis R. Rodriguez 2016-11-10 0:20 ` Rafael J. Wysocki 2016-11-10 0:20 ` Rafael J. Wysocki 2016-11-09 23:56 ` Rafael J. Wysocki 2016-11-09 23:56 ` Rafael J. Wysocki 2016-11-16 9:30 ` Lukas Wunner 2016-11-16 9:30 ` Lukas Wunner 2016-11-19 11:11 ` Lukas Wunner 2016-11-19 11:11 ` Lukas Wunner 2016-11-21 13:11 ` Marek Szyprowski 2016-11-21 13:11 ` Marek Szyprowski
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to='000701d22c28$2da381d0$88ea8570$@codeaurora.org' \ --to=sricharan@codeaurora.org \ --cc=b.zolnierkie@samsung.com \ --cc=broonie@kernel.org \ --cc=gregkh@linuxfoundation.org \ --cc=inki.dae@samsung.com \ --cc=iommu@lists.linux-foundation.org \ --cc=kgene@kernel.org \ --cc=khilman@kernel.org \ --cc=krzk@kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-pm@vger.kernel.org \ --cc=linux-samsung-soc@vger.kernel.org \ --cc=lukas@wunner.de \ --cc=m.szyprowski@samsung.com \ --cc=mcgrof@kernel.org \ --cc=rjw@rjwysocki.net \ --cc=tjakobi@math.uni-bielefeld.de \ --cc=tomasz.figa@gmail.com \ --cc=tomeu.vizoso@collabora.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.