From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932336AbcINHMI (ORCPT ); Wed, 14 Sep 2016 03:12:08 -0400 Received: from mailout4.w1.samsung.com ([210.118.77.14]:10662 "EHLO mailout4.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753872AbcINHMF (ORCPT ); Wed, 14 Sep 2016 03:12:05 -0400 X-AuditID: cbfec7f5-f79ce6d000004c54-63-57d8f83ffd01 Subject: Re: [PATCH v3 2/2] iommu/exynos: Add proper runtime pm support To: Ulf Hansson Cc: "linux-pm@vger.kernel.org" , "linux-kernel@vger.kernel.org" , iommu@lists.linux-foundation.org, linux-samsung-soc , Joerg Roedel , Inki Dae , Kukjin Kim , Krzysztof Kozlowski , Bartlomiej Zolnierkiewicz , "Rafael J. Wysocki" , Mark Brown , "Luis R. Rodriguez" , Greg Kroah-Hartman , Tomeu Vizoso , Lukas Wunner , Kevin Hilman From: Marek Szyprowski Message-id: Date: Wed, 14 Sep 2016 09:11:57 +0200 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-version: 1.0 In-reply-to: Content-type: text/plain; charset=utf-8; format=flowed Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA02Se0hTURzHObuP3Y2u3Kbmr6cwMKNIkwxPLiwj6uYfERUNoqhRN7U2k92s lAjTLDctX0liDyU1UTRrLrNcvjKl0maaNUsN8dELhRBRIVdu18D/Pr/z+57zO5/DYQjVJ2oZ Ex1zVjDG6PRqWknWtM7Y12+bdmg3XLYj/DivisK5g8M0Ti6qonH210wSFzZosOn2Izn+9T0I Zwz9IvBI9ZAMdz+/Q+OJ6y0I59nrZfj7t+XYkTmCcPvbLgrfqOyicVuldttifrjpnoyv7S9G vKXcRPN9H20033C3Qs7nOEoRb+25RvI3rOWIn7Cs4vOu1VB7lYeUW04I+uhzgjEw7JgyynS5 jI59EXjhWZFXImpZbUYMA1wwNCZiM1LM4RLoHKiizUjJqLgSBNbpNEoqJhAkpf6RS6lgsHb2 yqXGAwRZvztIqfiG4MrUc9qV8uR2gam9h3KxF7cGbIN291EEZ6PgbnIv4WrQXBCYx8zuDSwX BunZk6SLSc4PZl853ezNHYZ+R8d8ZjFM5wy41xXcfhjtLXZfieBCYdSZQknsC9UVY4RrGHBZ DOSWvpdJoivB0khICjtgIM1BS+wJP9us82orwJTaJJM4Y845ZZ3EeQjejbESa+Bl2/v5WR6Q XXOLkI5nIfWqSorwUFCQTkocDiN19fPPWCiDpOQ3KBP55i/QyV+gkL9AoRAR5chLiBMNkYK4 KUDUGcS4mMiA42cMFjT33d462yZrUUlraDPiGKRexO6d/qRVUbpzYryhGQFDqL3Y1kmHVsWe 0MUnCMYzR41xekFsRssZUu3D2go/aFVcpO6scFoQYgXj/66MUSxLRB7+u/27f0TUHnnorJwp 6PMY36zxbjLc2Tg7WKcZPz5cHcEkfMH94qWy+551r/12fu7aXhS3lW/6eTLw4t91kTedIR4f LxH69Cl9svxgltkyK6bkKN4cOOpTkNa+5fyOcPORqJg9+4ZD+sJDJp+8Vvj2sN1l48Gncmcz SlvYp+xSu5oUo3RBawmjqPsH5brMUGoDAAA= X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFvrBIsWRmVeSWpSXmKPExsVy+t/xq7pCP26EG0z6JmCxccZ6VoupD5+w WTQvXs9mMen+BBaLBfutLTpnb2C3eP3C0KL/8Wtmi6ebHzNZXN41h83ic+8RRosZ5/cxWbx4 Lm1xY8JTRoszpy+xWvStvcRmcXxtuIOgx5OD85g8dtxdwuixaVUnm8eda3vYPPbPXcPuMfnG ckaPLVfbWTz6tqxi9Pi8Sc5jRvs21gCuKDebjNTElNQihdS85PyUzLx0W6XQEDddCyWFvMTc VFulCF3fkCAlhbLEnFIgz8gADTg4B7gHK+nbJbhldDauZCvYq1+xc7FIA+MRtS5GTg4JAROJ LRduskPYYhIX7q1n62Lk4hASWMIo0fh0IpTznFHi98ErjCBVwgLuEp1nrrKC2CICGhJ7Hp5n hShaxCQxc8VcZhCHWWAfq8T5J0vB5rIJGEp0ve1iA7F5BewkeiZ9ZQGxWQRUJf4e/Qdkc3CI CsRIrO9LgCgRlPgx+R5YCadAsMSHxltgrcwCZhJfXh5mhbDlJTavecs8gVFgFpKWWUjKZiEp W8DIvIpRJLW0ODc9t9hQrzgxt7g0L10vOT93EyMw1rcd+7l5B+OljcGHGAU4GJV4eAN+XA8X Yk0sK67MPcQowcGsJMJ77OuNcCHelMTKqtSi/Pii0pzU4kOMpkA/TGSWEk3OB6ahvJJ4QxND c0tDI2MLC3MjIyVx3pIPV8KFBNITS1KzU1MLUotg+pg4OKUaGFOucMw/F2a4s23nPL+3wkdq bDa7ui2u5DVYFtCyxOfsrhV232buFz13O/DSW4Fb5j/zo5L3uPl2v5ly84BDwtPrX6bdLll+ 9BzjWx7jfzfbXhffdDgzbX6437cZH1atnGKySjIggqsmvNf026TnO/KSJGU0J3ErtLCv/bEg MD/Kcq6Eq/PTnXFKLMUZiYZazEXFiQB3FLhmCwMAAA== X-MTR: 20000000000000000@CPGS X-CMS-MailID: 20160914071159eucas1p2530d8459957c4b962c55575b4c562e82 X-Msg-Generator: CA X-Sender-IP: 182.198.249.179 X-Local-Sender: =?UTF-8?B?TWFyZWsgU3p5cHJvd3NraRtTUlBPTC1LZXJuZWwgKFRQKRs=?= =?UTF-8?B?7IK87ISx7KCE7J6QG1NlbmlvciBTb2Z0d2FyZSBFbmdpbmVlcg==?= X-Global-Sender: =?UTF-8?B?TWFyZWsgU3p5cHJvd3NraRtTUlBPTC1LZXJuZWwgKFRQKRtT?= =?UTF-8?B?YW1zdW5nIEVsZWN0cm9uaWNzG1NlbmlvciBTb2Z0d2FyZSBFbmdpbmVlcg==?= X-Sender-Code: =?UTF-8?B?QzEwG0VIURtDMTBDRDAyQ0QwMjczOTI=?= CMS-TYPE: 201P X-HopCount: 7 X-CMS-RootMailID: 20160913124918eucas1p29f49efbb142d416215482c29b53daff8 X-RootMTR: 20160913124918eucas1p29f49efbb142d416215482c29b53daff8 References: <1473770941-8337-1-git-send-email-m.szyprowski@samsung.com> <1473770941-8337-3-git-send-email-m.szyprowski@samsung.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Ulf, On 2016-09-13 16:20, Ulf Hansson wrote: > On 13 September 2016 at 14:49, Marek Szyprowski > wrote: >> This patch uses recently introduced device links to track the runtime pm >> state of the master's device. This way each SYSMMU controller is runtime >> activated when its master's device is active and can save/restore its state >> instead of being enabled all the time. This way SYSMMU controllers no >> longer prevents respective power domains to be turned off when master's >> device is not used. > Apologize for not reviewing earlier and if you find my > questions/suggestions being silly. You may ignore them, if you don't > think they deserves a proper answer. :-) No problem. There are no silly questions, but there might be some silly answers ;) > I am not so familiar with the IOMMU subsystem, but I am wondering > whether the issue you are solving, is similar to what can be observed > for DMA and serial drivers. And of course also for other IOMMU > drivers. > > In general the DMA/serial drivers requires to use the > pm_runtime_irq_safe() option, to be able to easily deploy runtime PM > support (of course there are some other workarounds as well). There are some similarities between IOMMU and DMA engine devices (serial drivers are imho completely different case). Both hw blocks do their work on behalf of some other hardware block, which I will call master device. DMA engine performs some DMA transaction on master's device request, while IOMMU usually sits between system memory and master's device memory interface, remapping addresses of each DMA transaction according to its configuration and provided mapping tables (master device has some kind of internal DMA controller and performs DMA transactions on their own). IOMMU is usually used for a) mapping physically discontinuous memory into contiguous DMA addresses and b) isolating devices, so they can access only memory, which is dedicated or allocated for them. DMA engine devices provide explicit API for their master's device drivers, while IOMMU drivers are usually hidden behind DMA-mapping API (for most use cases, although it would be possible for master's device driver to call IOMMU API directly and some GPU/DRM drivers do that). However from runtime pm perspective the DMA engine and IOMMU devices are a bit different. DMA engine drivers have well defined start and end of operation (queuing dma request and irq from hw about having it finished). During that time the device has to be runtime active all the time. The problem with using current implementation of runtime pm is the fact that both start and end of operation can be triggered from atomic context, what is not really suitable for runtime pm. So the problem is mainly about API incompatibility and lack of something like dma_engine_prepare()/unprepare() (as an analogy to clocks api). In case of IOMMU the main problem is determining weather IOMMU controller has to be activated. There is no calls in IOMMU and DMA-mapping API, which would bracket all DMA transactions performed by the master device. Someone proposed to keep IOMMU runtime active when there exist at least one mapping created by the IOMMU/DMA-mapping layers. This however does not cover all the cases. In case of our IOMMU, when it is disabled or runtime suspended, it enters "pass-thought" mode, so master device can still perform DMA operations with identity mappings (so DMA address equals to physical memory address). Till now Exynos IOMMU called pm_runtime_get() on attaching to the iommu domain (what happens during initialization of dma-mapping structures for given master device) and kept it active all the time. This patch series tries to address Exynos IOMMU runtime pm issue by forcing IOMMU controller to follow runtime pm status of its master device. This way we ensure that anytime when master's device is runtime activated, the iommu will be also active and master device won't be able to bypass during its DMA transactions mappings created by the IOMMU layer. Quite long answer, but I hope I managed to give you a bit more background on this topic. > As we know, using the pm_runtime_irq_safe() option comes with some > limitations, such as the runtime PM callbacks is not allowed to sleep. > For a PM domain (genpd) that is attached to the device, this also > means it must not be powered off. Right, if possible I would like to avoid using pm_runtime_irq_safe() option, because it is really impractical. > To solve this problem, I was thinking we could convert to use the > asynchronous pm_runtime_get() API, when trying to runtime resume the > device from atomic contexts. I'm not sure if this will work for DMA engine devices. If I understand correctly some client's of DMA engine device might rely on the DMA engine being configured and operational after queuing a request and they might lock up if the DMA engine device activation if postponed because of async runtime pm activation. > Of course when it turns out that the device isn't yet runtime resumed > immediately after calling pm_runtime_get(), the request needs to be > put on a request queue to be managed shortly after instead. Doing it > like this, would remove the need to use the pm_runtime_irq_safe() > option. > > I realize that such change needs to be implemented in common code for > IOMMU drivers, if at all possible. > > Anyway, I hope you at least get the idea and I just wanted to mention > that I have been exploring this option for DMA and serial drivers. I also have runtime pm for serial driver on my todo list, but it doesn't have high priority. The other runtime pm integration subsystem that I want to work on first is pinctrl. It is needed to fully support Exynos 5433 SoCs, because registers of some audio related pins are in the audio power domain, what now prevent us from enabling support for audio power domain. Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland