Re: [PATCH v8 3/3] PM / AVS: SVS: Introduce SVS engine

From: Matthias Brugger
Date: Tue May 26 2020 - 11:30:03 EST




On 26/05/2020 11:12, Roger Lu wrote:
> Hi Matthias,
>
> Thanks for the feedback.
>
> On Fri, 2020-05-22 at 17:38 +0200, Matthias Brugger wrote:
>>
>> On 22/05/2020 11:40, Roger Lu wrote:
>>>
>>> Hi Enric,
>>>
>>> On Tue, 2020-05-19 at 17:30 +0200, Enric Balletbo Serra wrote:
>>>> Hi Roger,
>>>>
>>>> Thank you for your patch. I have the feeling that this driver is
>>>> complex and difficult to follow and I am wondering if it wouldn't be
>>>> better if you can send a version that simply adds basic functionality
>>>> for now. Some comments below.
>>>
>>> Thanks for the advices. I'll submit SVS v9 with basic functionality
>>> patch + step by step functionalities' patches.
>>>
>>>>
>>>> Missatge de Roger Lu <roger.lu@xxxxxxxxxxxx> del dia dl., 18 de maig
>>>> 2020 a les 11:25:
>>>>>
>>>>> The SVS (Smart Voltage Scaling) engine is a piece
>>>>> of hardware which is used to calculate optimized
>>>>> voltage values of several power domains,
>>>>> e.g. CPU/GPU/CCI, according to chip process corner,
>>>>> temperatures, and other factors. Then DVFS driver
>>>>> could apply those optimized voltage values to reduce
>>>>> power consumption.
>>>>>
>>>>> Signed-off-by: Roger Lu <roger.lu@xxxxxxxxxxxx>
>>>>> ---
>>>>> drivers/power/avs/Kconfig | 10 +
>>>>> drivers/power/avs/Makefile | 1 +
>>>>> drivers/power/avs/mtk_svs.c | 2119 +++++++++++++++++++++++++++++++++
>>>>> include/linux/power/mtk_svs.h | 23 +
>>>>> 4 files changed, 2153 insertions(+)
>>>>> create mode 100644 drivers/power/avs/mtk_svs.c
>>>>> create mode 100644 include/linux/power/mtk_svs.h
>>>>>
>>>>> diff --git a/drivers/power/avs/Kconfig b/drivers/power/avs/Kconfig
>>>>> index cdb4237bfd02..67089ac6040e 100644
>>>>> --- a/drivers/power/avs/Kconfig
>>>>> +++ b/drivers/power/avs/Kconfig
>>>>> @@ -35,3 +35,13 @@ config ROCKCHIP_IODOMAIN
>>>>> Say y here to enable support io domains on Rockchip SoCs. It is
>>>>> necessary for the io domain setting of the SoC to match the
>>>>> voltage supplied by the regulators.
>>>>> +
>>>>> +config MTK_SVS
>>>>> + bool "MediaTek Smart Voltage Scaling(SVS)"
>>>>
>>>> Can't be this a module? Why? In such case, you should use tristate option
>>>
>>> Generally, MTK_SVS is needed in MTK SoC(mt81xx) products. So, we don't provide
>>> module option in config. If, somehow, SVS isn't needed, we suggest
>>> CONFIG_MTK_SVS=n to be set.
>>>
>>
>> The question here is if it needs to be probed before we probe the modules. If
>> not, we should add a Kconfig option for MT81xx SoCs to select MTK_SVS.
>
> Excuse me to make you confuse. MT81xx SoCs is the subset MTK ICs that
> will use CONFIG_MTK_SVS. In other words, CONFIG_MTK_SVS will be used
> with other MTK ICs as well. So, MTK_SVS is the general naming for MTK IC
> to enable SVS power feature. Anyway, back to Enric's question, I'll make
> MTK_SVS become a tristate feature in the next patch. Thanks.
>
>>
[...]
>>>>> +
>>>>> +static const u32 svs_regs_v2[] = {
>>>>
>>>> Is this SoC specific or shared between SoCs?
>>>
>>> Shared between SoCs. Some SVS in MTK SoCs use v2 register map.
>>>
>>
>> And which silicon uses v1 then? Is v2 a MediaTek internal naming you want to keep?
>
> 1. MT8173 IC uses v1 register map.
> 2. Yes, I'll keep v2 postfix.
>

Sounds good, thanks for clarification.

Regards,
Matthias