[prev in list] [next in list] [prev in thread] [next in thread] 

List:       linux-tegra
Subject:    Re: [RFC PATCH 0/3] PM / Domains: Add support for devices that require multiple domains
From:       Jon Hunter <jonathanh () nvidia ! com>
Date:       2016-12-15 11:38:03
Message-ID: c06d8619-0d2b-7b1c-37d9-da64a15c6db1 () nvidia ! com
[Download RAW message or body]


On 29/11/16 11:33, Marek Szyprowski wrote:
> Hi Stephen,
> 
> Thanks for pointing to my patches, but I would like to clarify a few
> things.
> 
> On 2016-11-24 03:30, Stephen Boyd wrote:
> > On 11/22, Jon Hunter wrote:
> > > On 22/11/16 18:26, Kevin Hilman wrote:
> > > > Jon Hunter <jonathanh@nvidia.com> writes:
> > > > > However, I would rather the client of
> > > > > the power-domains specify which power-domains they require and
> > > > > dynamically nested the power-domains at runtime. This is slightly
> > > > > different to what I proposed in this RFC, but it is not really beyond
> > > > > the bounds of what we support today IMO. What is missing is a means to
> > > > > do this dynamically and not statically.
> > > > > 
> > > > > By the way, I am not sure if you are suggesting that for devices that
> > > > > may need multiple power-domains we should architect the driver
> > > > > differently and split it up in some way such that we have a
> > > > > power-domain
> > > > > per device. But for the case of the Tegra XHCI it is quite complex
> > > > > because the driver loads firmware which runs on a micro-controller and
> > > > > we need to manage the various power-domains that are used.
> > > > IMO, constructing a network of new struct devices just to workaround
> > > > limitations in the framework doesn't sound quite right either.
> > > I agree.
> > > 
> > Marek is attempting to do this for the samsung clock
> > controller[1] (patch #5 is informative).
> 
> You probably meant patch #3 / #4, which is a patch for Exynos 4412
> ( https://marc.info/?l=linux-arm-kernel&m=147731142926053&w=2 ).

Sorry for the delay, I have been meaning to get back to this thread. Yes
patch #3 is definitely interesting and highlights power-domain
complexities with various SoCs ...

https://marc.info/?l=linux-clk&m=147731116925951&w=2


> Patch #5 is for Exynos 5433, which already has separate nodes for
> each clock sub-controller, so there is no problem to add generic
> power domains there (see multiple CMU nodes):
> 
> https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/tree/arch/arm64/boot/dts/exynos/exynos5433.dtsi#n261
>  
> 
> > From what I can tell
> > they have one DT node for their clock controller because it's one
> > register address space to control clocks. But, certain clocks
> > exposed by that driver only work when certain power domains are
> > enabled. For example, they have a clock controller that exposes
> > clock signals for multimedia hardware blocks like video
> > accelerators, gpus, and cameras. The clocks seem to have been
> > placed inside different power domains for the multimedia hardware
> > they're associated with, so there may be 10 or so power domains
> > that need to be enabled at different times for different clocks
> > to work. If the GPU power domain isn't enabled when the GPU
> > clocks are touched by the driver, things break, etc.
> > 
> > In the proposed patchset, we have the top-level clock controller
> > node with subnodes for each power domain that needs to be
> > associated with clocks inside these different multimedia blocks.
> 
> This is valid only for the Exynos4412 case (and not-yet-posted
> Exynos5422), which has a single clock controller node and patch #4
> added a sub-node for ISP clocks part (the only one which in fact
> is in the other power domain).
> 
> > So we end up with one parent device and attached driver for the
> > clock driver, and then that driver populates child nodes as
> > devices and matches up clocks with child nodes while registering
> > clks with clk_register(). Because we pass a dev pointer to
> > clk_register, we associate different devices with different
> > clocks all from the same top-level clock controller device
> > driver. Then clk framework calls runtime_pm APIs with devices
> > used during clk registration.
> 
> Right, this is how I did it for Exynos4412 case.
> 
> > Some of those devices don't have
> > any driver bound to them, which feels odd.
> 
> Well, I don't get this. In the proposed patches each sub-node has
> a separate driver, none is left without a driver.

So it seems in this solution you are creating a pseudo device in order
to control the pm-domain.

> > This seems like a case where we really want a better way to
> > explicitly control power domains without making up subnodes and
> > registering struct devices just to work around the one device to
> > one genpd construct we have today. Maybe power domains just don't
> > map to genpd though and that's the disconnect.
> 
> Having an API for full control over multiple power domain assigned to
> a single device node might indeed solve somehow this problem, but as
> long as runtime pm is tied to struct device, this will again end in
> creating virtual sub-devices per each power domain to fit runtime pm
> principles. However we might be able to avoid creating sub-nodes in
> the device tree.

Right, but what if the device is not bound to the pm-domain? What if due
to pm-domain complexities we need to have finer granularity over the
control of pm-domains?

It seems to me that because we don't always have foresight into the
creativeness of the SoC h/w designers, may be the pm-domain framework
needs to expose APIs to allow direct control of the pm-domains without
binding the pm-domain to the device, so that these APIs can be called in
the context on the runtime-pm callbacks as we do for clocks, etc.

Do you see any benefit to being able to directly control the pm-domains?

That said, I did recently add a patch to prevent the expose of the
pm-domain struct to clients in order to be able to remove them [0].
However, ref-counting can obviously be used to allow client to request
handles to pm-domains.

Cheers
Jon

[0]
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/base/power/domain.c?id=f58d4e5ab0ca3453f091eab514474e9fdbfc539f


-- 
nvpublic
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[prev in list] [next in list] [prev in thread] [next in thread]