question: interconnect: changes in 5.10 / imx8mq ?

question: interconnect: changes in 5.10 / imx8mq ?

[Martin Kepplinger]

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

* hook up the driver
https://source.puri.sm/martin.kepplinger/linux-next/-/commit/63679bcfa586578fc4345010a30ff7ac1af4c3c6
(I know this doesn't use devm_* and has to be cleaned up)


on v5.9 this would nicely have the driver's value set:

  node                                  tag          avg         peak
--------------------------------------------------------------------
NOC                                                   0       678900
   30320000.lcd-controller                0            0       678900
DRAM                                                  0       678900
   30320000.lcd-controller                0            0       678900
(...)

now on 5.10, I get dram always running at the highest speed because the 
*node* "requests" a lot, see the interconnect_summary:

  node                                  tag          avg         peak
--------------------------------------------------------------------
NOC                                          2147483647   2147483647
   30320000.lcd-controller                0            0       678900
DRAM                                         2147483647   2147483647
   30320000.lcd-controller                0            0       678900
(...)

the above is with screen on of course, as can be seen by the value from 
the driver. That *is* set to 0 as expected, but still, the "2147483647" 
remains:

  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?

thanks a lot!

                                   martin

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

Re: question: interconnect: changes in 5.10 / imx8mq ?

[Georgi Djakov]

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.

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,
  	},
  };

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

Re: question: interconnect: changes in 5.10 / imx8mq ?

[Martin Kepplinger]

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

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

Re: question: interconnect: changes in 5.10 / imx8mq ?

[Martin Kepplinger]

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)

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.

I don't find obvious changes since v5.9 that could cause the error. Do you?

thanks,

                             martin

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

Re: 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

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

Re: 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

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

Re: question: interconnect: changes in 5.10 / imx8mq ?

[Georgi Djakov]

On 1.12.20 13:35, Martin Kepplinger wrote:
> 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:
[..]
>>>
>>> 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 could be a DT change or something else.. Maybe try running git bisect
and see if it gives any clue.

Thanks,
Georgi

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

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