Message-ID: <58351176.20800@codeaurora.org>
Date:   Wed, 23 Nov 2016 09:18:06 +0530
From:   Rajendra Nayak <rnayak@...eaurora.org>
To:     Ulf Hansson <ulf.hansson@...aro.org>,
        Stanimir Varbanov <stanimir.varbanov@...aro.org>
CC:     Jon Hunter <jonathanh@...dia.com>,
        "Rafael J. Wysocki" <rjw@...ysocki.net>,
        Kevin Hilman <khilman@...nel.org>,
        "linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-tegra@...r.kernel.org" <linux-tegra@...r.kernel.org>,
        Sricharan <sricharan@...eaurora.org>
Subject: Re: [RFC PATCH 0/3] PM / Domains: Add support for devices that require
 multiple domains


On 11/22/2016 06:35 PM, Ulf Hansson wrote:
> On 17 November 2016 at 16:39, Stanimir Varbanov
> <stanimir.varbanov@...aro.org> wrote:
>> Hi,
>>
>> On 11/17/2016 04:31 AM, Rajendra Nayak wrote:
>>>
>>>
>>> On 11/16/2016 06:41 PM, Ulf Hansson wrote:
>>>> On 2 November 2016 at 09:56, Rajendra Nayak <rnayak@...eaurora.org> wrote:
>>>>> Hi Jon,
>>>>>
>>>>> On 10/31/2016 04:14 PM, Jon Hunter wrote:
>>>>>> Hi Rajendra,
>>>>>>
>>>>>> On 06/10/16 09:43, Rajendra Nayak wrote:
>>>>>>>
>>>>>>> On 10/06/2016 01:55 PM, Jon Hunter wrote:
>>>>>>>> Hi Rajendra,
>>>>>>>>
>>>>>>>> On 06/10/16 07:04, Rajendra Nayak wrote:
>>>>>>>>>
>>>>>>>>> On 09/20/2016 03:58 PM, Jon Hunter wrote:
>>>>>>>>>> The Tegra124/210 XUSB subsystem (that consists of both host and device
>>>>>>>>>> controllers) is partitioned across 3 PM domains which are:
>>>>>>>>>> - XUSBA: Superspeed logic (for USB 3.0)
>>>>>>>>>> - XUSBB: Device controller
>>>>>>>>>> - XUSBC: Host controller
>>>>>>>>>>
>>>>>>>>>> These power domains are not nested and can be powered-up and down
>>>>>>>>>> independently of one another. In practice different scenarios require
>>>>>>>>>> different combinations of the power domains, for example:
>>>>>>>>>> - Superspeed host: XUSBA and XUSBC
>>>>>>>>>> - Superspeed device: XUSBA and XUSBB
>>>>>>>>>>
>>>>>>>>>> Although it could be possible to logically nest both the XUSBB and XUSBC
>>>>>>>>>> domains under the XUSBA, superspeed may not always be used/required and
>>>>>>>>>> so this would keep it on unnecessarily.
>>>>>>>>>
>>>>>>>>> Hey Jon, so does this RFC provide a way to just specify multiple Powerdomains
>>>>>>>>> for a device (which then will *all* be powered on/off together) or does
>>>>>>>>> it also provide for more granular control of these powerdomains?
>>>>>>>>
>>>>>>>> Only to specify multiple power-domains for a device and not the later.
>>>>>>>>
>>>>>>>>> The above statement seems to suggest you would need more granular control
>>>>>>>>> of these powerdomains (like keeping XUSBA off in case superspeed it not
>>>>>>>>> needed) but I can't seem to figure out how you achieve it with this series.
>>>>>>>>
>>>>>>>> It is an interesting point but today we have always kept the superspeed
>>>>>>>> partition on if the device is configured for superspeed regardless of
>>>>>>>> what is actually connected. I will check to see if the h/w would allow
>>>>>>>> us to turn it off if a non-superspeed device is in use but I did not
>>>>>>>> think so.
>>>>>>>>
>>>>>>>> Do you have any interesting use-cases that would make use of this or
>>>>>>>> require other such enhancements?
>>>>>>>
>>>>>>> We do have atleast a few devices which need to control multiple power domains,
>>>>>>> I will need to look more to see if any of them can be controlled individually.
>>>>>>> The downstream code we have models these (powerdomains) as regulators and
>>>>>>> the drivers hence have individual control on each (specifying multiple -supply's
>>>>>>> in DT)
>>>>>>
>>>>>> Were you able to check to see if you need to have individual control for the power-domains?
>>>>>
>>>>> I had a look at the Video decode block (for msm8996), which seems to be powered using 3 different
>>>>> powerdomains, mainly venus, venus_core0 and venus_core1. The venus PD powers the ARM core
>>>>> which runs the firmware, while the venus_core0 and venus_core1 power the encode/decode logic,
>>>>> so for things like firmware image loading you ideally need only venus PD to be ON, but during
>>>>> an encode/decode operation you would need all 3 to be ON.
>>>>
>>>> Isn't there a scenario when encoding *or* decoding happens, not always both?
>>>>
>>>> If so, doesn't that mean you may have venus + venus_core0 powered and
>>>> in some other case venus + venus_core1 powered?
>>>>
>>>>> The downstream driver turns *all* of them together, and does not control them individually.
>>>>> For upstream, the way we have it working (the driver is not merged) is by having venus be the parent
>>>>> of venus_core0 and venus_core0 as the parent of venus_core1, and having venus_core1 mentioned as
>>>>> the powerdomain for the video decode block in DT.
>>>>>
>>>>> So in summary, there is still no need to control them individually, but given there is no way to
>>>>> specify more than one powerdomain for a given device, we are ending up hooking up some
>>>>> parent/child relations in the powerdomain code.
>>>>>
>>>>
>>>> I think a better solution would be to model the video decode block as
>>>> three struct devices.
>>>>
>>>> 1) The main ARM device, attached to the venus PM domain.
>>>> 2) The encoder device, having the main device assigned as its parent
>>>> and being attached to the venus_core0 PM domain.
>>>> 3) The decoder device, having the main device assigned as its parent
>>>> and being attached to the venus_core1 PM domain.
>>>>
>>>> Then there is no need to specific a  PM domain hierarchy (which seems
>>>> to be the issue here), but instead only the parent/child relationships
>>>> between the struct devices.
>>>>
>>>> Moreover, as you deploy runtime PM for these devices, you can more
>>>> easily distinguish which device you need to operate on
>>>> (pm_runtime_get|put*()) depending on what particular operations you
>>>> want to do (encode, decode etc).
>>>
>>> Stan, is this something you think is possible to do, given the way the
>>> vidc driver is designed? This is mainly for 8996 which has 3 different
>>> powerdomains associated with the video decode block.
>>
>> Even if it is possible it will be difficult for many reasons.
>>
>> On the other side, current design (firmware) doesn't expect kernel
>> driver to have control over venus_core0 and venus_core1 pm domains. The
>> firmware manages those two pm domains internally and the only thing
>> which we need to do is to prepare those domains (and follow the power up
>> sequence) to be in hardware control mode. So I think the best we could
>> do is to model those two power domains as genpd subdomains of the parent
>> venus pm domain.
> 
> Okay, so that was easy then. Why all the fuzz? :-)

Not as easy as it sounds :-), so Venus has 3 powerdomains, core, subcore0 and subcore1.
So the ideal way of representing the parent/child relation in this case would be

         Core
         /  \
        /    \
       /      \
 subcore0    subcore1

So what powerdomain would you associate with the venus device in such case so a runtime
call from the venus driver would turn all 3 of them?

So, instead we end up with something like..

         Core
          |
          |
       subcore0
          |
          |
       subcore1

..and associate subcore1 as the powerdomain for Venus.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

Powered by blists - more mailing lists