Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757671AbcLOLjg (ORCPT ); Thu, 15 Dec 2016 06:39:36 -0500 Received: from hqemgate14.nvidia.com ([216.228.121.143]:16771 "EHLO hqemgate14.nvidia.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757606AbcLOLje (ORCPT ); Thu, 15 Dec 2016 06:39:34 -0500 X-PGP-Universal: processed; by hqnvupgp07.nvidia.com on Wed, 14 Dec 2016 15:36:56 -0800 Subject: Re: [RFC PATCH 0/3] PM / Domains: Add support for devices that require multiple domains To: Marek Szyprowski , Stephen Boyd References: <90faea7d-65b6-590a-83f1-24fcdffa0569@nvidia.com> <63670abf-1d58-a7e3-6927-0c815d44d8a1@nvidia.com> <20161124023014.GK6095@codeaurora.org> <52af1977-8ca3-40d1-43bb-920c5b933f94@samsung.com> CC: Kevin Hilman , "Rafael J. Wysocki" , Ulf Hansson , "Rafael J. Wysocki" , "linux-pm@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "linux-tegra@vger.kernel.org" , Rajendra Nayak From: Jon Hunter Message-ID: Date: Thu, 15 Dec 2016 11:38:03 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.5.1 MIME-Version: 1.0 In-Reply-To: <52af1977-8ca3-40d1-43bb-920c5b933f94@samsung.com> X-Originating-IP: [10.21.132.139] X-ClientProxiedBy: DRUKMAIL101.nvidia.com (10.25.59.19) To UKMAIL101.nvidia.com (10.26.138.13) Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5790 Lines: 129 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 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