linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Rafael J. Wysocki" <rafael@kernel.org>
To: Lukas Wunner <lukas@wunner.de>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>,
	"Luis R. Rodriguez" <mcgrof@kernel.org>,
	Linux PM <linux-pm@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	"open list:AMD IOMMU (AMD-VI)" <iommu@lists.linux-foundation.org>,
	Linux Samsung SoC <linux-samsung-soc@vger.kernel.org>,
	Joerg Roedel <joro@8bytes.org>, Inki Dae <inki.dae@samsung.com>,
	Kukjin Kim <kgene@kernel.org>,
	Krzysztof Kozlowski <krzk@kernel.org>,
	Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>,
	"Rafael J. Wysocki" <rjw@rjwysocki.net>,
	Mark Brown <broonie@kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Tomeu Vizoso <tomeu.vizoso@collabora.com>,
	Kevin Hilman <khilman@kernel.org>,
	Tobias Jakobi <tjakobi@math.uni-bielefeld.de>,
	Tomasz Figa <tomasz.figa@gmail.com>,
	Grant Likely <grant.likely@secretlab.ca>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	Lars-Peter Clausen <lars@metafoo.de>,
	Andrzej Hajda <a.hajda@samsung.com>,
	Mauro Carvalho Chehab <mchehab@osg.samsung.com>,
	Dmitry Torokhov <dmitry.torokhov@gmail.com>
Subject: Re: [PATCH v5 7/7] iommu/exynos: Use device dependency links to control runtime pm
Date: Thu, 10 Nov 2016 00:56:14 +0100	[thread overview]
Message-ID: <CAJZ5v0i3_8AE_o7vx=5kyuc8+P0jFOvjTC3YB9gcJVyQ+pTSkA@mail.gmail.com> (raw)
In-Reply-To: <20161108153044.GA6983@wunner.de>

On Tue, Nov 8, 2016 at 4:30 PM, Lukas Wunner <lukas@wunner.de> wrote:
> On Tue, Nov 08, 2016 at 08:27:12AM +0100, Marek Szyprowski wrote:
>> On 2016-11-07 22:47, Luis R. Rodriguez wrote:
>> > Has there been any review of the existing similar solutions out there
>> > such as the DRM / audio component framework? Would that help ?
>>
>> Nope, none of that solution deals with runtime pm.
>
> Well, they do.  Hybrid graphics laptops often have an HDA controller
> on the discrete GPU and I assume that's what Luis meant. There's code
> in drivers/gpu/vga/vga_switcheroo.c to make this (only sort of) work:
>
> * When the GPU is powered up/down, the HDA controller's driver is
>   instructed to pm_runtime_get/put the HDA device (see call to
>   set_audio_state() in vga_switcheroo_set_dynamic_switch()).
>
> * When a runtime PM ref is acquired on the HDA device, the
>   GPU is powered up (see vga_switcheroo_runtime_resume_hdmi_audio()).
>
>
> Unfortunately this is all fairly broken, e.g.:
>
> * If a runtime PM ref on the HDA device is held for more than 5 sec
>   (autosuspend delay of the GPU), the GPU will be powered down and
>   the HDA device will become inaccessible, regardless of the runtime
>   PM ref still being held (because vga_switcheroo_set_dynamic_switch()
>   doesn't check the refcount of the HDA device).
>
> * The DRM device is afforded direct-complete but the HDA device is not.
>   If the GPU is handled earlier by dpm_suspend(), then runtime PM will
>   have been disabled on the GPU and thus the HDA device will fail to
>   runtime resume before system sleep.
>
> Rafael's series allows representation of such inter-device dependencies
> in the PM core and can thus replace kludgy and broken "solutions" like
> the one above.
>
> There's one thing that I haven't understood myself though:  In an e-mail
> exchange in September Rafael has argued that the above-mentioned hybrid
> graphics use case "isn't a good [example] IMO.  That clearly is a case
> when two (or more) devices share power resources controlled by a single
> on/off switch.  Which is a clear use case for a PM domain."
>
> The same seems to apply to Marek's SYSMMU use case.  When applying device
> links to SYSMMU or hybrid graphics, we select one of the devices in the
> PM domain as master and have the other one depend on it as slave, i.e.
> a synthetic hierarchical relationship is established.
>
> I've responded to Rafael on September 18 that this can't be solved with
> a struct dev_pm_domain, but haven't received a reply since:
> https://lkml.org/lkml/2016/9/18/103

Well, that clearly fell off my radar, sorry about that.

The idea, roughly, is that if there is a single on/off switch acting
on multiple devices, you can (a) set up a PM domain tracking all of
those device's runtime PM invocations and (b) maintaining a reference
counter of devices still not suspended.  This way it would only turn
the switch off when all of the devices in question had been suspended.
Analogously, it would turn the switch on before resuming the first
device in the domain.  Of course, that code isn't available as a
library, you would need to implement it (or use genpd, but chances are
it is too heavy weight for the job).

In theory, it shouldn't really matter where the switch is and how it
is operated as long as the PM domain "driver" knows how to access it.
However, for that to work something would need to bind that "driver"
to the domain and something would need to know which devices needed to
be added to the PM domains during enumeration and the ordering of
things bringup matters a lot here, which is a problem.

So in short, you are right that for the GPU/HDA thing device links
would likely to be a simpler to use and still get the job done.

Thanks,
Rafael

  parent reply	other threads:[~2016-11-09 23:56 UTC|newest]

Thread overview: 27+ 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
     [not found]   ` <CGME20161020072331eucas1p1af7dc7270b0b19168b949f3416eda474@eucas1p1.samsung.com>
2016-10-20  7:22     ` [PATCH v5 1/7] iommu/exynos: Remove excessive, useless debug Marek Szyprowski
     [not found]   ` <CGME20161020072332eucas1p1d980c1659979bd5bc2918bfc9d40a415@eucas1p1.samsung.com>
2016-10-20  7:22     ` [PATCH v5 2/7] iommu/exynos: Remove dead code 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
     [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
     [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-22  5:50       ` Sricharan
2016-10-24  5:19         ` Marek Szyprowski
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-24  5:30         ` Marek Szyprowski
2016-10-24 12:29           ` Sricharan
2016-10-24 12:39             ` Marek Szyprowski
2016-10-25  6:53               ` Sricharan
2016-11-07 21:47       ` Luis R. Rodriguez
2016-11-08  7:27         ` Marek Szyprowski
2016-11-08 15:30           ` Lukas Wunner
2016-11-09 23:55             ` Luis R. Rodriguez
2016-11-10  0:05               ` Rafael J. Wysocki
2016-11-10  0:12                 ` Luis R. Rodriguez
2016-11-10  0:20                   ` Rafael J. Wysocki
2016-11-09 23:56             ` Rafael J. Wysocki [this message]
2016-11-16  9:30               ` Lukas Wunner
2016-11-19 11:11           ` Lukas Wunner
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='CAJZ5v0i3_8AE_o7vx=5kyuc8+P0jFOvjTC3YB9gcJVyQ+pTSkA@mail.gmail.com' \
    --to=rafael@kernel.org \
    --cc=a.hajda@samsung.com \
    --cc=b.zolnierkie@samsung.com \
    --cc=broonie@kernel.org \
    --cc=dmitry.torokhov@gmail.com \
    --cc=grant.likely@secretlab.ca \
    --cc=gregkh@linuxfoundation.org \
    --cc=inki.dae@samsung.com \
    --cc=iommu@lists.linux-foundation.org \
    --cc=joro@8bytes.org \
    --cc=kgene@kernel.org \
    --cc=khilman@kernel.org \
    --cc=krzk@kernel.org \
    --cc=lars@metafoo.de \
    --cc=laurent.pinchart@ideasonboard.com \
    --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=mchehab@osg.samsung.com \
    --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: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).