Re: [PATCH 0/8] interconnect: Add imx support via devfreq

[Leonard Crestez <leonard.crestez@nxp.com>]

On 2020-03-30 11:52 AM, Martin Kepplinger wrote:
> On 27.03.20 12:36, Guido Günther wrote:
>> Hi Martin,
>> On Thu, Mar 26, 2020 at 02:55:27PM +0100, Martin Kepplinger wrote:
>>> On 26.03.20 03:16, Leonard Crestez wrote:
>>>> This series adds interconnect scaling support for imx8m series chips. It uses a
>>>> per-SOC interconnect provider layered on top of multiple instances of devfreq
>>>> for scalable nodes along the interconnect.
>>>>
>>>> Existing qcom interconnect providers mostly translate bandwidth requests into
>>>> firmware calls but equivalent firmware on imx8m is much thinner. Scaling
>>>> support for individual nodes is implemented as distinct devfreq drivers
>>>> instead.
>>>>
>>>> The imx interconnect provider doesn't communicate with devfreq directly
>>>> but rather computes "minimum frequencies" for nodes along the path and
>>>> creates dev_pm_qos requests.
>>>>
>>>> Since there is no single devicetree node that can represent the
>>>> "interconnect" the main NOC is picked as the "interconnect provider" and
>>>> will probe the interconnect platform device if #interconnect-cells is
>>>> present. This avoids introducing "virtual" devices but it means that DT
>>>> bindings of main NOC includes properties for both devfreq and
>>>> interconnect.
>>>>
>>>> Only the ddrc and main noc are scalable right now but more can be added.
>>>>
>>>> Also available on a github branch (with various unrelated changes):
>>>> 	https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fcdleonard%2Flinux%2Ftree%2Fnext&data=02%7C01%7Cleonard.crestez%40nxp.com%7C52eb55181dce469eb85608d7d487bb93%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637211551737394247&sdata=A0l5FBF%2BT3k7H5HMtRMfX8WfVSqQm9jijgr8aexCoUA%3D&reserved=0
>>>> Testing currently requires NXP branch of atf+uboot
>>>>
>>>> Martin: I believe you should be able to use this to control DRAM
>>>> frequency from video by just adding interconnect consumer code to
>>>> nwl-dsi. Sample code:
>>>> 	https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fcdleonard%2Flinux%2Fcommit%2F43772762aa5045f1ce5623740f9a4baef988d083&data=02%7C01%7Cleonard.crestez%40nxp.com%7C52eb55181dce469eb85608d7d487bb93%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637211551737394247&sdata=%2B%2BGWQTaQLLk98yFRHJ0o3Sks9DHGuKv7twBvn89f1Tg%3D&reserved=0
>>>> 	https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fcdleonard%2Flinux%2Fcommit%2F7b601e981b1f517b5d98b43bde292972ded13086&data=02%7C01%7Cleonard.crestez%40nxp.com%7C52eb55181dce469eb85608d7d487bb93%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637211551737394247&sdata=Jy2%2FI3CE1H3ilmLvZAVhjlPHO3KRNK6%2F9dX%2BS124ROA%3D&reserved=0
>>>>
>>>
>>> Thanks for updating this series Leonard! A few questions for my
>>> understanding before trying to test:
>>>
>>> Isn't the ddrc_opp_table missing in these additions to the DT? That's
>>> what I want to scale after all.
DDRC OPP table belongs in board file because it can vary with RAM type 
on boards.

>>> If I want to keep calling the "request", now icc_set_bw(), in nwl-dsi:
>>> I'd add an "interconnects" property to the node, but what would be my
>>> interconnect master? i.e.: interconnects = <&noc master? &noc
>>> IMX8MQ_ICS_DRAM>; At least it's not obvious to me from
>>> interconnect/imx/imx8mq.c
>>
>> The NWL DSI host controller is fed by DCSS or mxsfb so any bandwidth
>> requirements should (as far as I understand things) go into the display
>> controller driver since that's what fetches from RAM.
>> Cheers,
>>   -- Guido

This is correct.

> Thanks a lot Leonard and Guido! Here's the tree I'm running, which is
> your patches based on Linus' tree, with icc request in mxsfb:
> 
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fsource.puri.sm%2Fmartin.kepplinger%2Flinux-next%2Fcommits%2F5.6-rc7%2Flibrem5__integration_devfreq1&amp;data=02%7C01%7Cleonard.crestez%40nxp.com%7C52eb55181dce469eb85608d7d487bb93%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637211551737394247&amp;sdata=FM8JoOuNa2gg09XVJ%2FLTLqK9rlL4hAwig2iM9cMYFhg%3D&amp;reserved=0
> 
> The path from icc via pm_qos to devfreq does work (which is great) -
> however only after setting the minimum frequencies via a governor - I
> set the "powersave" governor.
> 
> After that, frequencies are both set to high / low correctly.
> 
> My impression was that I should be able to use the "passive" governor (a
> passive devfreq device?). What am I missing with using devfreq
> correctly? Or do I already?

The devfreq governor is something else: it's used to make one devfreq 
device match frequencies with another devfreq device.

Setting the default governor to "powersave" is correct and roughly 
matches behavior in NXP kernel.

> Tested-by: Martin Kepplinger <martin.kepplinger@puri.sm>

Thanks!

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel