All of lore.kernel.org
 help / color / mirror / Atom feed
From: Marek Szyprowski <m.szyprowski@samsung.com>
To: Sricharan <sricharan@codeaurora.org>,
	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 7/7] iommu/exynos: Use device dependency links to control runtime pm
Date: Mon, 24 Oct 2016 07:30:12 +0200	[thread overview]
Message-ID: <7a69ca4e-e276-1bdc-edf2-b881a91f820a@samsung.com> (raw)
In-Reply-To: <001e01d22d12$d2a4e990$77eebcb0$@codeaurora.org>

Hi Sricharan,


On 2016-10-23 11:49, Sricharan wrote:
> Hi Marek,
>> This patch uses recently introduced device dependency links to track the
>> runtime pm state of the master's device. This way each SYSMMU controller
>> is set to runtime active only when its master's device is active and can
>> restore or save its state instead of being activated all the time when
>> attached to the given master device. This way SYSMMU controllers no longer
>> prevents respective power domains to be turned off when master's device
>> is not being used.
>>
>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> ---
>> drivers/iommu/exynos-iommu.c | 16 ++++++++--------
>> 1 file changed, 8 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
>> index 5e6d7bbf9b70..59b4f2ce4f5f 100644
>> --- a/drivers/iommu/exynos-iommu.c
>> +++ b/drivers/iommu/exynos-iommu.c
>> @@ -781,10 +781,6 @@ static void exynos_iommu_detach_device(struct iommu_domain *iommu_domain,
>> 	if (!has_sysmmu(dev) || owner->domain != iommu_domain)
>> 		return;
>>
>> -	list_for_each_entry(data, &owner->controllers, owner_node) {
>> -		pm_runtime_put_sync(data->sysmmu);
>> -	}
>> -
>> 	mutex_lock(&owner->rpm_lock);
>>
>> 	list_for_each_entry(data, &owner->controllers, owner_node) {
>> @@ -848,10 +844,6 @@ static int exynos_iommu_attach_device(struct iommu_domain *iommu_domain,
>>
>> 	mutex_unlock(&owner->rpm_lock);
>>
>> -	list_for_each_entry(data, &owner->controllers, owner_node) {
>> -		pm_runtime_get_sync(data->sysmmu);
>> -	}
>> -
>> 	dev_dbg(dev, "%s: Attached IOMMU with pgtable %pa\n", __func__,
>> 		&pagetable);
>>
>> @@ -1232,6 +1224,14 @@ static int exynos_iommu_of_xlate(struct device *dev,
>>
>> 	list_add_tail(&data->owner_node, &owner->controllers);
>> 	data->master = dev;
>> +
>> +	/*
>> +	 * SYSMMU will be runtime activated via device link (dependency) to its
>> +	 * master device, so there are no direct calls to pm_runtime_get/put
>> +	 * in this driver.
>> +	 */
>> +	device_link_add(dev, data->sysmmu, DL_FLAG_PM_RUNTIME);
>> +
>    So in the case of master with multiple sids, this would be called multiple times
>    for the same master ?

I don't know what is "multiple sids" case, but if given SYSMMU master 
device (supplier)
has multiple SYSMMU controllers (consumers), then links will be created 
for each SYSMMU
controller. Please note that this code is based on vanilla v4.9-rc1, 
which calls
of_xlate() callback only once for every iommu for given master device. 
Your IOMMU
deferred probe patches change this, but I already posted a fix for 
Exynos IOMMU driver
to handle such case.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

WARNING: multiple messages have this Message-ID (diff)
From: Marek Szyprowski <m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
To: Sricharan <sricharan-sgV2jX0FEOL9JmXXK+q4OQ@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 7/7] iommu/exynos: Use device dependency links to control runtime pm
Date: Mon, 24 Oct 2016 07:30:12 +0200	[thread overview]
Message-ID: <7a69ca4e-e276-1bdc-edf2-b881a91f820a@samsung.com> (raw)
In-Reply-To: <001e01d22d12$d2a4e990$77eebcb0$@codeaurora.org>

Hi Sricharan,


On 2016-10-23 11:49, Sricharan wrote:
> Hi Marek,
>> This patch uses recently introduced device dependency links to track the
>> runtime pm state of the master's device. This way each SYSMMU controller
>> is set to runtime active only when its master's device is active and can
>> restore or save its state instead of being activated all the time when
>> attached to the given master device. This way SYSMMU controllers no longer
>> prevents respective power domains to be turned off when master's device
>> is not being used.
>>
>> Signed-off-by: Marek Szyprowski <m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>> ---
>> drivers/iommu/exynos-iommu.c | 16 ++++++++--------
>> 1 file changed, 8 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
>> index 5e6d7bbf9b70..59b4f2ce4f5f 100644
>> --- a/drivers/iommu/exynos-iommu.c
>> +++ b/drivers/iommu/exynos-iommu.c
>> @@ -781,10 +781,6 @@ static void exynos_iommu_detach_device(struct iommu_domain *iommu_domain,
>> 	if (!has_sysmmu(dev) || owner->domain != iommu_domain)
>> 		return;
>>
>> -	list_for_each_entry(data, &owner->controllers, owner_node) {
>> -		pm_runtime_put_sync(data->sysmmu);
>> -	}
>> -
>> 	mutex_lock(&owner->rpm_lock);
>>
>> 	list_for_each_entry(data, &owner->controllers, owner_node) {
>> @@ -848,10 +844,6 @@ static int exynos_iommu_attach_device(struct iommu_domain *iommu_domain,
>>
>> 	mutex_unlock(&owner->rpm_lock);
>>
>> -	list_for_each_entry(data, &owner->controllers, owner_node) {
>> -		pm_runtime_get_sync(data->sysmmu);
>> -	}
>> -
>> 	dev_dbg(dev, "%s: Attached IOMMU with pgtable %pa\n", __func__,
>> 		&pagetable);
>>
>> @@ -1232,6 +1224,14 @@ static int exynos_iommu_of_xlate(struct device *dev,
>>
>> 	list_add_tail(&data->owner_node, &owner->controllers);
>> 	data->master = dev;
>> +
>> +	/*
>> +	 * SYSMMU will be runtime activated via device link (dependency) to its
>> +	 * master device, so there are no direct calls to pm_runtime_get/put
>> +	 * in this driver.
>> +	 */
>> +	device_link_add(dev, data->sysmmu, DL_FLAG_PM_RUNTIME);
>> +
>    So in the case of master with multiple sids, this would be called multiple times
>    for the same master ?

I don't know what is "multiple sids" case, but if given SYSMMU master 
device (supplier)
has multiple SYSMMU controllers (consumers), then links will be created 
for each SYSMMU
controller. Please note that this code is based on vanilla v4.9-rc1, 
which calls
of_xlate() callback only once for every iommu for given master device. 
Your IOMMU
deferred probe patches change this, but I already posted a fix for 
Exynos IOMMU driver
to handle such case.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

  reply	other threads:[~2016-10-24  5:30 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
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 [this message]
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=7a69ca4e-e276-1bdc-edf2-b881a91f820a@samsung.com \
    --to=m.szyprowski@samsung.com \
    --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=mcgrof@kernel.org \
    --cc=rjw@rjwysocki.net \
    --cc=sricharan@codeaurora.org \
    --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: link
Be 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.