question: interconnect: changes in 5.10 / imx8mq ?

[Georgi Djakov]

On 12/1/20 02:36, Martin Kepplinger wrote:
> On 30.11.20 23:10, Martin Kepplinger wrote:
>> On 30.11.20 22:18, Georgi Djakov wrote:
>>> On 30.11.20 22:34, Martin Kepplinger wrote:
>>>> hi,
>>>>
>>>> what I've used on v5.9 on imx8mq in order to hook up dram frequency to 
>>>> interconnect (via mxsfb/lcdif) - and has worked fine - is:
>>>>
>>>> * add the NOC node description with "#interconnect-cells = <1>;"
>>>> https://source.puri.sm/martin.kepplinger/linux-next/-/commit/8a6b8486a3e94e2886bde01000f9532e03d243a4 
>>>>
>>>> (original author is Leonard. I'll preserve authorship when submitting)
>>>>
>>>> * add "interconnects = <&noc IMX8MQ_ICM_LCDIF &noc IMX8MQ_ICS_DRAM>;
>>>> " to lcdif:
>>>> https://source.puri.sm/martin.kepplinger/linux-next/-/commit/6c4bbcdc315da01a9dc8bbda36290587ce1ed33a 
>>>
>>>
>>>
>>> [..]
>>>>
>>>>   node                                  tag          avg         peak
>>>> --------------------------------------------------------------------
>>>> NOC                                          2147483647   2147483647
>>>>    30320000.lcd-controller                0            0            0
>>>> DRAM                                         2147483647   2147483647
>>>>    30320000.lcd-controller                0            0            0
>>>> (...)
>>>>
>>>>
>>>>
>>>> what am I doing wrong on recent kernels?
>>>
>>> Hi Martin,
>>> This looks related to sync_state. Please try the change below.
>>> It would be nice to get these DT patches merged into mainline.
>>
>> that's the plan. I'll send them soon.
>>
>>>
>>> Thanks,
>>> Georgi
>>>
>>> diff --git a/drivers/interconnect/imx/imx8mq.c 
>>> b/drivers/interconnect/imx/imx8mq.c
>>> index ba43a15aefec..9bb951b075e9 100644
>>> --- a/drivers/interconnect/imx/imx8mq.c
>>> +++ b/drivers/interconnect/imx/imx8mq.c
>>> @@ -94,6 +94,7 @@ static struct platform_driver imx8mq_icc_driver = {
>>>       .remove = imx8mq_icc_remove,
>>>       .driver = {
>>>           .name = "imx8mq-interconnect",
>>> +        .sync_state = icc_sync_state,
>>>       },
>>>   };
>>
>> that's exactly it. thanks a lot!
>>
>>                             martin
> 
> but there follows the next problem. it looks imx8m specific:
> 
> On the librem5-devkit where I initially tested, switching works. FYI we have the 
> 2 frequencies:
> https://source.puri.sm/martin.kepplinger/linux-next/-/blob/5.10-rc5/librem5__integration/arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts#L283 
> 
> (the opp table also to be submitted to mainline soon)
> 
> On the Librem5 itself (different SoC revision, different frequencies available) 
> it fails:
> https://source.puri.sm/martin.kepplinger/linux-next/-/blob/5.10-rc5/librem5__integration/arch/arm64/boot/dts/freescale/imx8mq-librem5.dtsi#L387 
> 
> 
> When I "request 0" (or disable the icc path) in order to switch to 25Mhz I now get:
> 
> [  129.391755] imx8m-ddrc-devfreq 3d400000.memory-controller: failed to set 
> dram_apb parent: -16
> [  129.391959] imx8m-ddrc-devfreq 3d400000.memory-controller: ddrc failed freq 
> switch to 25000000 from 800000000: error -16. now at 25000000
> [  129.406133] imx8m-ddrc-devfreq 3d400000.memory-controller: failed to update 
> frequency from PM QoS (-16)

I am not familiar with the clock tree of this platform, but it looks like -EBUSY 
is returned when the we are trying to change the parent of the clock.

> and the system hangs at this point.
> 
> I'm not aware of any changes we do in our tree in that area to mainline.
> 
> Only removing all but one frequency in the opp node, leaving only opp-800M, 
> "works around" (not really) the error (just mentioning as a data point if that 
> helps). I hope that's not misleading - no idea where exactly the problem lies.

When there is only a single frequency, then probably we do not try to change the
mux settings and that's why it does not hang. Maybe check the clock tree and if
all needed clocks and branches are enabled.

Thanks,
Georgi