question: interconnect: changes in 5.10 / imx8mq ?

[Martin Kepplinger]

On 01.12.20 10:10, Georgi Djakov wrote:
> 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 for taking the time here. I don't see notable changes to the 
clock tree compared to 5.9. Specifically, "dram_apb" where reparenting 
fails, is running on 5.10 too.

It's strange that I see this error on imx8mq-librem5 but not on 
imx8mq-librem5-devkit.

thanks,
                              martin