The driver can operate in two modes relaying on devfreq monitoring
mechanism which periodically checks the device status or it can use
interrupts when they are provided by loaded Device Tree. The newly
introduced module parameter can be used to choose between devfreq
monitoring and internal interrupts without modifying the Device Tree.
It also sets devfreq monitoring as default when the parameter is not set
(also the case for default when the driver is not built as a module).
Reported-by: Willy Wolff <[email protected]>
Signed-off-by: Lukasz Luba <[email protected]>
---
drivers/memory/samsung/exynos5422-dmc.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/memory/samsung/exynos5422-dmc.c b/drivers/memory/samsung/exynos5422-dmc.c
index e03ee35f0ab5..53bfe6b7b703 100644
--- a/drivers/memory/samsung/exynos5422-dmc.c
+++ b/drivers/memory/samsung/exynos5422-dmc.c
@@ -12,6 +12,7 @@
#include <linux/io.h>
#include <linux/mfd/syscon.h>
#include <linux/module.h>
+#include <linux/moduleparam.h>
#include <linux/of_device.h>
#include <linux/pm_opp.h>
#include <linux/platform_device.h>
@@ -21,6 +22,10 @@
#include "../jedec_ddr.h"
#include "../of_memory.h"
+static int irqmode;
+module_param(irqmode, int, 0644);
+MODULE_PARM_DESC(irqmode, "Enable IRQ mode (0=off [default], 1=on)");
+
#define EXYNOS5_DREXI_TIMINGAREF (0x0030)
#define EXYNOS5_DREXI_TIMINGROW0 (0x0034)
#define EXYNOS5_DREXI_TIMINGDATA0 (0x0038)
@@ -1428,7 +1433,7 @@ static int exynos5_dmc_probe(struct platform_device *pdev)
/* There is two modes in which the driver works: polling or IRQ */
irq[0] = platform_get_irq_byname(pdev, "drex_0");
irq[1] = platform_get_irq_byname(pdev, "drex_1");
- if (irq[0] > 0 && irq[1] > 0) {
+ if (irq[0] > 0 && irq[1] > 0 && irqmode) {
ret = devm_request_threaded_irq(dev, irq[0], NULL,
dmc_irq_thread, IRQF_ONESHOT,
dev_name(dev), dmc);
@@ -1485,7 +1490,7 @@ static int exynos5_dmc_probe(struct platform_device *pdev)
if (dmc->in_irq_mode)
exynos5_dmc_start_perf_events(dmc, PERF_COUNTER_START_VALUE);
- dev_info(dev, "DMC initialized\n");
+ dev_info(dev, "DMC initialized, in irq mode: %d\n", dmc->in_irq_mode);
return 0;
--
2.17.1
Hi,
On 7/10/20 9:11 PM, Lukasz Luba wrote:
> The driver can operate in two modes relaying on devfreq monitoring
> mechanism which periodically checks the device status or it can use
> interrupts when they are provided by loaded Device Tree. The newly
> introduced module parameter can be used to choose between devfreq
> monitoring and internal interrupts without modifying the Device Tree.
> It also sets devfreq monitoring as default when the parameter is not set
> (also the case for default when the driver is not built as a module).
Could you please explain why should we leave the IRQ mode
support in the dmc driver?
What are the advantages over the polling mode?
In what scenarios it should be used?
[ If this is only for documentation purposes then it should be
removed as it would stay in (easily accessible) git history
anyway.. ]
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
> Reported-by: Willy Wolff <[email protected]>
> Signed-off-by: Lukasz Luba <[email protected]>
> ---
> drivers/memory/samsung/exynos5422-dmc.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/memory/samsung/exynos5422-dmc.c b/drivers/memory/samsung/exynos5422-dmc.c
> index e03ee35f0ab5..53bfe6b7b703 100644
> --- a/drivers/memory/samsung/exynos5422-dmc.c
> +++ b/drivers/memory/samsung/exynos5422-dmc.c
> @@ -12,6 +12,7 @@
> #include <linux/io.h>
> #include <linux/mfd/syscon.h>
> #include <linux/module.h>
> +#include <linux/moduleparam.h>
> #include <linux/of_device.h>
> #include <linux/pm_opp.h>
> #include <linux/platform_device.h>
> @@ -21,6 +22,10 @@
> #include "../jedec_ddr.h"
> #include "../of_memory.h"
>
> +static int irqmode;
> +module_param(irqmode, int, 0644);
> +MODULE_PARM_DESC(irqmode, "Enable IRQ mode (0=off [default], 1=on)");
> +
> #define EXYNOS5_DREXI_TIMINGAREF (0x0030)
> #define EXYNOS5_DREXI_TIMINGROW0 (0x0034)
> #define EXYNOS5_DREXI_TIMINGDATA0 (0x0038)
> @@ -1428,7 +1433,7 @@ static int exynos5_dmc_probe(struct platform_device *pdev)
> /* There is two modes in which the driver works: polling or IRQ */
> irq[0] = platform_get_irq_byname(pdev, "drex_0");
> irq[1] = platform_get_irq_byname(pdev, "drex_1");
> - if (irq[0] > 0 && irq[1] > 0) {
> + if (irq[0] > 0 && irq[1] > 0 && irqmode) {
> ret = devm_request_threaded_irq(dev, irq[0], NULL,
> dmc_irq_thread, IRQF_ONESHOT,
> dev_name(dev), dmc);
> @@ -1485,7 +1490,7 @@ static int exynos5_dmc_probe(struct platform_device *pdev)
> if (dmc->in_irq_mode)
> exynos5_dmc_start_perf_events(dmc, PERF_COUNTER_START_VALUE);
>
> - dev_info(dev, "DMC initialized\n");
> + dev_info(dev, "DMC initialized, in irq mode: %d\n", dmc->in_irq_mode);
>
> return 0;
>
>
Hi Bartek,
On 7/14/20 8:42 AM, Bartlomiej Zolnierkiewicz wrote:
>
> Hi,
>
> On 7/10/20 9:11 PM, Lukasz Luba wrote:
>> The driver can operate in two modes relaying on devfreq monitoring
>> mechanism which periodically checks the device status or it can use
>> interrupts when they are provided by loaded Device Tree. The newly
>> introduced module parameter can be used to choose between devfreq
>> monitoring and internal interrupts without modifying the Device Tree.
>> It also sets devfreq monitoring as default when the parameter is not set
>> (also the case for default when the driver is not built as a module).
>
> Could you please explain why should we leave the IRQ mode
> support in the dmc driver?
I am still experimenting with the IRQ mode in DMC, but have limited time
for it and no TRM.
The IRQ mode in memory controller or bus controller has one major
advantage: is more interactive. In polling we have fixed period, i.e.
100ms - that's a lot when we have a sudden, latency sensitive workload.
There might be no check of the device load for i.e. 99ms, but the tasks
with such workload started running. That's a long period of a few frames
which are likely to be junked. Should we adjust polling interval to i.e.
10ms, I don't think so. There is no easy way to address all of the
scenarios.
>
> What are the advantages over the polling mode?
As described above: more reactive to sudden workload, which might be
latency sensitive and cause junk frames.
Drawback: not best in benchmarks which are randomly jumping
over the data set, causing low traffic on memory.
It could be mitigated as Sylwester described with not only one type
of interrupt, but another, which could 'observe' also other information
type in the counters and fire.
>
> In what scenarios it should be used?
System like Android with GUI, when there is this sudden workload
quite often.
I think the interconnect could help here and would adjust the DMC
freq upfront. Although I don't know if interconnect on Exynos5422 is in
your scope in near future. Of course the interconnect will not cover
all scenarios either.
>
> [ If this is only for documentation purposes then it should be
> removed as it would stay in (easily accessible) git history
> anyway.. ]
The current interrupt mode is definitely not perfect and switching
to devfreq monitoring mode has more sense. On the other hand, it
still has potential, until there is no interconnect for this SoC.
I will continue experimenting with irq mode, so I would like to
still have the code in the driver.
Regards,
Lukasz
>
> Best regards,
> --
> Bartlomiej Zolnierkiewicz
> Samsung R&D Institute Poland
> Samsung Electronics
>
>> Reported-by: Willy Wolff <[email protected]>
>> Signed-off-by: Lukasz Luba <[email protected]>
>> ---
>> drivers/memory/samsung/exynos5422-dmc.c | 9 +++++++--
>> 1 file changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/memory/samsung/exynos5422-dmc.c b/drivers/memory/samsung/exynos5422-dmc.c
>> index e03ee35f0ab5..53bfe6b7b703 100644
>> --- a/drivers/memory/samsung/exynos5422-dmc.c
>> +++ b/drivers/memory/samsung/exynos5422-dmc.c
>> @@ -12,6 +12,7 @@
>> #include <linux/io.h>
>> #include <linux/mfd/syscon.h>
>> #include <linux/module.h>
>> +#include <linux/moduleparam.h>
>> #include <linux/of_device.h>
>> #include <linux/pm_opp.h>
>> #include <linux/platform_device.h>
>> @@ -21,6 +22,10 @@
>> #include "../jedec_ddr.h"
>> #include "../of_memory.h"
>>
>> +static int irqmode;
>> +module_param(irqmode, int, 0644);
>> +MODULE_PARM_DESC(irqmode, "Enable IRQ mode (0=off [default], 1=on)");
>> +
>> #define EXYNOS5_DREXI_TIMINGAREF (0x0030)
>> #define EXYNOS5_DREXI_TIMINGROW0 (0x0034)
>> #define EXYNOS5_DREXI_TIMINGDATA0 (0x0038)
>> @@ -1428,7 +1433,7 @@ static int exynos5_dmc_probe(struct platform_device *pdev)
>> /* There is two modes in which the driver works: polling or IRQ */
>> irq[0] = platform_get_irq_byname(pdev, "drex_0");
>> irq[1] = platform_get_irq_byname(pdev, "drex_1");
>> - if (irq[0] > 0 && irq[1] > 0) {
>> + if (irq[0] > 0 && irq[1] > 0 && irqmode) {
>> ret = devm_request_threaded_irq(dev, irq[0], NULL,
>> dmc_irq_thread, IRQF_ONESHOT,
>> dev_name(dev), dmc);
>> @@ -1485,7 +1490,7 @@ static int exynos5_dmc_probe(struct platform_device *pdev)
>> if (dmc->in_irq_mode)
>> exynos5_dmc_start_perf_events(dmc, PERF_COUNTER_START_VALUE);
>>
>> - dev_info(dev, "DMC initialized\n");
>> + dev_info(dev, "DMC initialized, in irq mode: %d\n", dmc->in_irq_mode);
>>
>> return 0;
>>
>>
>
Hi Lukasz and Bartek,
On 2020-07-14-10-01-16, Lukasz Luba wrote:
>Hi Bartek,
>
>On 7/14/20 8:42 AM, Bartlomiej Zolnierkiewicz wrote:
>>
>>Hi,
>>
>>On 7/10/20 9:11 PM, Lukasz Luba wrote:
>>>The driver can operate in two modes relaying on devfreq monitoring
>>>mechanism which periodically checks the device status or it can use
>>>interrupts when they are provided by loaded Device Tree. The newly
>>>introduced module parameter can be used to choose between devfreq
>>>monitoring and internal interrupts without modifying the Device Tree.
>>>It also sets devfreq monitoring as default when the parameter is not set
>>>(also the case for default when the driver is not built as a module).
>>
>>Could you please explain why should we leave the IRQ mode
>>support in the dmc driver?
>
>I am still experimenting with the IRQ mode in DMC, but have limited time
>for it and no TRM.
>The IRQ mode in memory controller or bus controller has one major
>advantage: is more interactive. In polling we have fixed period, i.e.
>100ms - that's a lot when we have a sudden, latency sensitive workload.
>There might be no check of the device load for i.e. 99ms, but the tasks
>with such workload started running. That's a long period of a few frames
>which are likely to be junked. Should we adjust polling interval to i.e.
>10ms, I don't think so. There is no easy way to address all of the
>scenarios.
>
>>
>>What are the advantages over the polling mode?
>
>As described above: more reactive to sudden workload, which might be
>latency sensitive and cause junk frames.
>Drawback: not best in benchmarks which are randomly jumping
>over the data set, causing low traffic on memory.
>It could be mitigated as Sylwester described with not only one type
>of interrupt, but another, which could 'observe' also other information
>type in the counters and fire.
>
>>
>>In what scenarios it should be used?
>
>System like Android with GUI, when there is this sudden workload
>quite often.
>
>I think the interconnect could help here and would adjust the DMC
>freq upfront. Although I don't know if interconnect on Exynos5422 is in
>your scope in near future. Of course the interconnect will not cover
>all scenarios either.
>
The interconnect (CCI-400) will not help much, you still have the same problem
of setting interrupts at the right threshold, or to poll it to see any activity
through it.
>
>>
>>[ If this is only for documentation purposes then it should be
>> removed as it would stay in (easily accessible) git history
>> anyway.. ]
>
>The current interrupt mode is definitely not perfect and switching
>to devfreq monitoring mode has more sense. On the other hand, it
>still has potential, until there is no interconnect for this SoC.
>I will continue experimenting with irq mode, so I would like to
>still have the code in the driver.
>
>Regards,
>Lukasz
>
>>
>>Best regards,
>>--
>>Bartlomiej Zolnierkiewicz
>>Samsung R&D Institute Poland
>>Samsung Electronics
>>
>>>Reported-by: Willy Wolff <[email protected]>
>>>Signed-off-by: Lukasz Luba <[email protected]>
>>>---
>>> drivers/memory/samsung/exynos5422-dmc.c | 9 +++++++--
>>> 1 file changed, 7 insertions(+), 2 deletions(-)
>>>
>>>diff --git a/drivers/memory/samsung/exynos5422-dmc.c b/drivers/memory/samsung/exynos5422-dmc.c
>>>index e03ee35f0ab5..53bfe6b7b703 100644
>>>--- a/drivers/memory/samsung/exynos5422-dmc.c
>>>+++ b/drivers/memory/samsung/exynos5422-dmc.c
>>>@@ -12,6 +12,7 @@
>>> #include <linux/io.h>
>>> #include <linux/mfd/syscon.h>
>>> #include <linux/module.h>
>>>+#include <linux/moduleparam.h>
>>> #include <linux/of_device.h>
>>> #include <linux/pm_opp.h>
>>> #include <linux/platform_device.h>
>>>@@ -21,6 +22,10 @@
>>> #include "../jedec_ddr.h"
>>> #include "../of_memory.h"
>>>+static int irqmode;
>>>+module_param(irqmode, int, 0644);
>>>+MODULE_PARM_DESC(irqmode, "Enable IRQ mode (0=off [default], 1=on)");
>>>+
>>> #define EXYNOS5_DREXI_TIMINGAREF (0x0030)
>>> #define EXYNOS5_DREXI_TIMINGROW0 (0x0034)
>>> #define EXYNOS5_DREXI_TIMINGDATA0 (0x0038)
>>>@@ -1428,7 +1433,7 @@ static int exynos5_dmc_probe(struct platform_device *pdev)
>>> /* There is two modes in which the driver works: polling or IRQ */
>>> irq[0] = platform_get_irq_byname(pdev, "drex_0");
>>> irq[1] = platform_get_irq_byname(pdev, "drex_1");
>>>- if (irq[0] > 0 && irq[1] > 0) {
>>>+ if (irq[0] > 0 && irq[1] > 0 && irqmode) {
>>> ret = devm_request_threaded_irq(dev, irq[0], NULL,
>>> dmc_irq_thread, IRQF_ONESHOT,
>>> dev_name(dev), dmc);
>>>@@ -1485,7 +1490,7 @@ static int exynos5_dmc_probe(struct platform_device *pdev)
>>> if (dmc->in_irq_mode)
>>> exynos5_dmc_start_perf_events(dmc, PERF_COUNTER_START_VALUE);
>>>- dev_info(dev, "DMC initialized\n");
>>>+ dev_info(dev, "DMC initialized, in irq mode: %d\n", dmc->in_irq_mode);
>>> return 0;
>>>
>>
Hi Willy,
On 7/14/20 10:32 AM, Willy Wolff wrote:
> Hi Lukasz and Bartek,
>
> On 2020-07-14-10-01-16, Lukasz Luba wrote:
>> Hi Bartek,
>>
>> On 7/14/20 8:42 AM, Bartlomiej Zolnierkiewicz wrote:
>>>
>>> Hi,
>>>
>>> On 7/10/20 9:11 PM, Lukasz Luba wrote:
>>>> The driver can operate in two modes relaying on devfreq monitoring
>>>> mechanism which periodically checks the device status or it can use
>>>> interrupts when they are provided by loaded Device Tree. The newly
>>>> introduced module parameter can be used to choose between devfreq
>>>> monitoring and internal interrupts without modifying the Device Tree.
>>>> It also sets devfreq monitoring as default when the parameter is not
>>>> set
>>>> (also the case for default when the driver is not built as a module).
>>>
>>> Could you please explain why should we leave the IRQ mode
>>> support in the dmc driver?
>>
>> I am still experimenting with the IRQ mode in DMC, but have limited time
>> for it and no TRM.
>> The IRQ mode in memory controller or bus controller has one major
>> advantage: is more interactive. In polling we have fixed period, i.e.
>> 100ms - that's a lot when we have a sudden, latency sensitive workload.
>> There might be no check of the device load for i.e. 99ms, but the tasks
>> with such workload started running. That's a long period of a few frames
>> which are likely to be junked. Should we adjust polling interval to i.e.
>> 10ms, I don't think so. There is no easy way to address all of the
>> scenarios.
>>
>>>
>>> What are the advantages over the polling mode?
>>
>> As described above: more reactive to sudden workload, which might be
>> latency sensitive and cause junk frames.
>> Drawback: not best in benchmarks which are randomly jumping
>> over the data set, causing low traffic on memory.
>> It could be mitigated as Sylwester described with not only one type
>> of interrupt, but another, which could 'observe' also other information
>> type in the counters and fire.
>>
>>>
>>> In what scenarios it should be used?
>>
>> System like Android with GUI, when there is this sudden workload
>> quite often.
>>
>> I think the interconnect could help here and would adjust the DMC
>> freq upfront. Although I don't know if interconnect on Exynos5422 is in
>> your scope in near future. Of course the interconnect will not cover
>> all scenarios either.
>>
>
> The interconnect (CCI-400) will not help much, you still have the same
> problem
> of setting interrupts at the right threshold, or to poll it to see any
> activity
> through it.
I was referring to 'interconnect' framework, the work Artur and now
Sylwester is doing [1]. Together with devfreq passive governors, proper
description of device dependencies and required bandwidth, should be
able to address the typical scenarios in the system.
My bad, I haven't explained which interconnect I have in mind.
I agree with you that HW interconnect won't solve this.
Regards,
Lukasz
[1] https://lkml.org/lkml/2020/7/2/861
>
>>
>>>
>>> [ If this is only for documentation purposes then it should be
>>> removed as it would stay in (easily accessible) git history
>>> anyway.. ]
>>
>> The current interrupt mode is definitely not perfect and switching
>> to devfreq monitoring mode has more sense. On the other hand, it
>> still has potential, until there is no interconnect for this SoC.
>> I will continue experimenting with irq mode, so I would like to
>> still have the code in the driver.
>>
>> Regards,
>> Lukasz
>>
>>>
>>> Best regards,
>>> --
>>> Bartlomiej Zolnierkiewicz
>>> Samsung R&D Institute Poland
>>> Samsung Electronics
>>>
>>>> Reported-by: Willy Wolff <[email protected]>
>>>> Signed-off-by: Lukasz Luba <[email protected]>
>>>> ---
>>>> drivers/memory/samsung/exynos5422-dmc.c | 9 +++++++--
>>>> 1 file changed, 7 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/memory/samsung/exynos5422-dmc.c
>>>> b/drivers/memory/samsung/exynos5422-dmc.c
>>>> index e03ee35f0ab5..53bfe6b7b703 100644
>>>> --- a/drivers/memory/samsung/exynos5422-dmc.c
>>>> +++ b/drivers/memory/samsung/exynos5422-dmc.c
>>>> @@ -12,6 +12,7 @@
>>>> #include <linux/io.h>
>>>> #include <linux/mfd/syscon.h>
>>>> #include <linux/module.h>
>>>> +#include <linux/moduleparam.h>
>>>> #include <linux/of_device.h>
>>>> #include <linux/pm_opp.h>
>>>> #include <linux/platform_device.h>
>>>> @@ -21,6 +22,10 @@
>>>> #include "../jedec_ddr.h"
>>>> #include "../of_memory.h"
>>>> +static int irqmode;
>>>> +module_param(irqmode, int, 0644);
>>>> +MODULE_PARM_DESC(irqmode, "Enable IRQ mode (0=off [default], 1=on)");
>>>> +
>>>> #define EXYNOS5_DREXI_TIMINGAREF (0x0030)
>>>> #define EXYNOS5_DREXI_TIMINGROW0 (0x0034)
>>>> #define EXYNOS5_DREXI_TIMINGDATA0 (0x0038)
>>>> @@ -1428,7 +1433,7 @@ static int exynos5_dmc_probe(struct
>>>> platform_device *pdev)
>>>> /* There is two modes in which the driver works: polling or IRQ */
>>>> irq[0] = platform_get_irq_byname(pdev, "drex_0");
>>>> irq[1] = platform_get_irq_byname(pdev, "drex_1");
>>>> - if (irq[0] > 0 && irq[1] > 0) {
>>>> + if (irq[0] > 0 && irq[1] > 0 && irqmode) {
>>>> ret = devm_request_threaded_irq(dev, irq[0], NULL,
>>>> dmc_irq_thread, IRQF_ONESHOT,
>>>> dev_name(dev), dmc);
>>>> @@ -1485,7 +1490,7 @@ static int exynos5_dmc_probe(struct
>>>> platform_device *pdev)
>>>> if (dmc->in_irq_mode)
>>>> exynos5_dmc_start_perf_events(dmc, PERF_COUNTER_START_VALUE);
>>>> - dev_info(dev, "DMC initialized\n");
>>>> + dev_info(dev, "DMC initialized, in irq mode: %d\n",
>>>> dmc->in_irq_mode);
>>>> return 0;
>>>>
>>>
On 7/14/20 10:01 AM, Lukasz Luba wrote:
> Hi Bartek,
>
> On 7/14/20 8:42 AM, Bartlomiej Zolnierkiewicz wrote:
>>
>> Hi,
>>
>> On 7/10/20 9:11 PM, Lukasz Luba wrote:
>>> The driver can operate in two modes relaying on devfreq monitoring
>>> mechanism which periodically checks the device status or it can use
>>> interrupts when they are provided by loaded Device Tree. The newly
>>> introduced module parameter can be used to choose between devfreq
>>> monitoring and internal interrupts without modifying the Device Tree.
>>> It also sets devfreq monitoring as default when the parameter is not set
>>> (also the case for default when the driver is not built as a module).
>>
>> Could you please explain why should we leave the IRQ mode
>> support in the dmc driver?
>
> I am still experimenting with the IRQ mode in DMC, but have limited time
> for it and no TRM.
> The IRQ mode in memory controller or bus controller has one major
> advantage: is more interactive. In polling we have fixed period, i.e.
> 100ms - that's a lot when we have a sudden, latency sensitive workload.
> There might be no check of the device load for i.e. 99ms, but the tasks
> with such workload started running. That's a long period of a few frames
> which are likely to be junked. Should we adjust polling interval to i.e.
> 10ms, I don't think so. There is no easy way to address all of the
> scenarios.
>
>>
>> What are the advantages over the polling mode?
>
> As described above: more reactive to sudden workload, which might be
> latency sensitive and cause junk frames.
> Drawback: not best in benchmarks which are randomly jumping
> over the data set, causing low traffic on memory.
> It could be mitigated as Sylwester described with not only one type
> of interrupt, but another, which could 'observe' also other information
> type in the counters and fire.
>
>>
>> In what scenarios it should be used?
>
> System like Android with GUI, when there is this sudden workload
> quite often.
>
> I think the interconnect could help here and would adjust the DMC
> freq upfront. Although I don't know if interconnect on Exynos5422 is in
> your scope in near future. Of course the interconnect will not cover
> all scenarios either.
>
>
>>
>> [ If this is only for documentation purposes then it should be
>> removed as it would stay in (easily accessible) git history
>> anyway.. ]
>
> The current interrupt mode is definitely not perfect and switching
> to devfreq monitoring mode has more sense. On the other hand, it
> still has potential, until there is no interconnect for this SoC.
> I will continue experimenting with irq mode, so I would like to
> still have the code in the driver.
>
> Regards,
> Lukasz
>
>>
>> Best regards,
>> --
>> Bartlomiej Zolnierkiewicz
>> Samsung R&D Institute Poland
>> Samsung Electronics
>>
Bartek, do you have some objections to the patches or you think
they can be taken via devfreq-next?
Cheers,
Lukasz
On 7/17/20 1:53 PM, Lukasz Luba wrote:
>
>
> On 7/14/20 10:01 AM, Lukasz Luba wrote:
>> Hi Bartek,
>>
>> On 7/14/20 8:42 AM, Bartlomiej Zolnierkiewicz wrote:
>>>
>>> Hi,
>>>
>>> On 7/10/20 9:11 PM, Lukasz Luba wrote:
>>>> The driver can operate in two modes relaying on devfreq monitoring
>>>> mechanism which periodically checks the device status or it can use
>>>> interrupts when they are provided by loaded Device Tree. The newly
>>>> introduced module parameter can be used to choose between devfreq
>>>> monitoring and internal interrupts without modifying the Device Tree.
>>>> It also sets devfreq monitoring as default when the parameter is not set
>>>> (also the case for default when the driver is not built as a module).
>>>
>>> Could you please explain why should we leave the IRQ mode
>>> support in the dmc driver?
>>
>> I am still experimenting with the IRQ mode in DMC, but have limited time
>> for it and no TRM.
>> The IRQ mode in memory controller or bus controller has one major
>> advantage: is more interactive. In polling we have fixed period, i.e.
>> 100ms - that's a lot when we have a sudden, latency sensitive workload.
>> There might be no check of the device load for i.e. 99ms, but the tasks
>> with such workload started running. That's a long period of a few frames
>> which are likely to be junked. Should we adjust polling interval to i.e.
>> 10ms, I don't think so. There is no easy way to address all of the
>> scenarios.
>>
>>>
>>> What are the advantages over the polling mode?
>>
>> As described above: more reactive to sudden workload, which might be
>> latency sensitive and cause junk frames.
>> Drawback: not best in benchmarks which are randomly jumping
>> over the data set, causing low traffic on memory.
>> It could be mitigated as Sylwester described with not only one type
>> of interrupt, but another, which could 'observe' also other information
>> type in the counters and fire.
>>
>>>
>>> In what scenarios it should be used?
>>
>> System like Android with GUI, when there is this sudden workload
>> quite often.
>>
>> I think the interconnect could help here and would adjust the DMC
>> freq upfront. Although I don't know if interconnect on Exynos5422 is in
>> your scope in near future. Of course the interconnect will not cover
>> all scenarios either.
>>
>>
>>>
>>> [ If this is only for documentation purposes then it should be
>>> removed as it would stay in (easily accessible) git history
>>> anyway.. ]
>>
>> The current interrupt mode is definitely not perfect and switching
>> to devfreq monitoring mode has more sense. On the other hand, it
>> still has potential, until there is no interconnect for this SoC.
>> I will continue experimenting with irq mode, so I would like to
>> still have the code in the driver.
>>
>> Regards,
>> Lukasz
>>
>>>
>>> Best regards,
>>> --
>>> Bartlomiej Zolnierkiewicz
>>> Samsung R&D Institute Poland
>>> Samsung Electronics
>>>
>
> Bartek, do you have some objections to the patches or you think
> they can be taken via devfreq-next?
No objections from me, thank you for the IRQ mode explanation.
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
On Fri, Jul 10, 2020 at 08:11:22PM +0100, Lukasz Luba wrote:
> The driver can operate in two modes relaying on devfreq monitoring
> mechanism which periodically checks the device status or it can use
> interrupts when they are provided by loaded Device Tree. The newly
> introduced module parameter can be used to choose between devfreq
> monitoring and internal interrupts without modifying the Device Tree.
> It also sets devfreq monitoring as default when the parameter is not set
> (also the case for default when the driver is not built as a module).
>
> Reported-by: Willy Wolff <[email protected]>
> Signed-off-by: Lukasz Luba <[email protected]>
> ---
> drivers/memory/samsung/exynos5422-dmc.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
Reviewed-by: Krzysztof Kozlowski <[email protected]>
Best regards,
Krzysztof
Hi Lukasz,
On 10.07.2020 21:11, Lukasz Luba wrote:
> The driver can operate in two modes relaying on devfreq monitoring
> mechanism which periodically checks the device status or it can use
> interrupts when they are provided by loaded Device Tree. The newly
> introduced module parameter can be used to choose between devfreq
> monitoring and internal interrupts without modifying the Device Tree.
> It also sets devfreq monitoring as default when the parameter is not set
> (also the case for default when the driver is not built as a module).
>
> Reported-by: Willy Wolff <[email protected]>
> Signed-off-by: Lukasz Luba <[email protected]>
I've got back from my holidays and noticed that in meantime this commit
got merged as commit 4fc9a0470d2d. It revealed that there is a race
between registering exynos5422-dmc driver and exynos-ppmu driver, which
can be observed sometimes as the following message and freeze on Odroid
XU3 with multi_v7_defconfig:
[ 8.767708] exynos5-dmc 10c20000.memory-controller: couldn't probe
performance counters
I will check this later why the EPROBE_DEFER error is not properly
propagated and why it causes a freeze.
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
Hi All,
On 03.08.2020 17:30, Marek Szyprowski wrote:
> On 10.07.2020 21:11, Lukasz Luba wrote:
>> The driver can operate in two modes relaying on devfreq monitoring
>> mechanism which periodically checks the device status or it can use
>> interrupts when they are provided by loaded Device Tree. The newly
>> introduced module parameter can be used to choose between devfreq
>> monitoring and internal interrupts without modifying the Device Tree.
>> It also sets devfreq monitoring as default when the parameter is not set
>> (also the case for default when the driver is not built as a module).
>>
>> Reported-by: Willy Wolff <[email protected]>
>> Signed-off-by: Lukasz Luba <[email protected]>
>
> I've got back from my holidays and noticed that in meantime this
> commit got merged as commit 4fc9a0470d2d. It revealed that there is a
> race between registering exynos5422-dmc driver and exynos-ppmu driver,
> which can be observed sometimes as the following message and freeze on
> Odroid XU3 with multi_v7_defconfig:
>
> [ 8.767708] exynos5-dmc 10c20000.memory-controller: couldn't probe
> performance counters
>
> I will check this later why the EPROBE_DEFER error is not properly
> propagated and why it causes a freeze.
It looks that simply propagating return value from
exynos5_counters_get() in exynos5_dmc_get_status() fixes the boot:
# dmesg | grep dmc
[ 8.838754] exynos-ppmu: new PPMU device registered 10d00000.ppmu
(ppmu-event3-dmc0_0)
[ 8.861344] exynos-ppmu: new PPMU device registered 10d10000.ppmu
(ppmu-event3-dmc0_1)
[ 8.868488] exynos5-dmc 10c20000.memory-controller: couldn't probe
performance counters
[ 8.874417] exynos-ppmu: new PPMU device registered 10d60000.ppmu
(ppmu-event3-dmc1_0)
[ 8.886612] exynos-ppmu: new PPMU device registered 10d70000.ppmu
(ppmu-event3-dmc1_1)
[ 9.396769] exynos5-dmc 10c20000.memory-controller: DMC initialized,
in irq mode: 0
I'm still curious why it freezes if getting performance counters is not
possible.
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
Hi Marek,
On 8/3/20 4:35 PM, Marek Szyprowski wrote:
> Hi All,
>
> On 03.08.2020 17:30, Marek Szyprowski wrote:
>> On 10.07.2020 21:11, Lukasz Luba wrote:
>>> The driver can operate in two modes relaying on devfreq monitoring
>>> mechanism which periodically checks the device status or it can use
>>> interrupts when they are provided by loaded Device Tree. The newly
>>> introduced module parameter can be used to choose between devfreq
>>> monitoring and internal interrupts without modifying the Device Tree.
>>> It also sets devfreq monitoring as default when the parameter is not set
>>> (also the case for default when the driver is not built as a module).
>>>
>>> Reported-by: Willy Wolff <[email protected]>
>>> Signed-off-by: Lukasz Luba <[email protected]>
>>
>> I've got back from my holidays and noticed that in meantime this
>> commit got merged as commit 4fc9a0470d2d. It revealed that there is a
>> race between registering exynos5422-dmc driver and exynos-ppmu driver,
>> which can be observed sometimes as the following message and freeze on
>> Odroid XU3 with multi_v7_defconfig:
>>
>> [ 8.767708] exynos5-dmc 10c20000.memory-controller: couldn't probe
>> performance counters
>>
>> I will check this later why the EPROBE_DEFER error is not properly
>> propagated and why it causes a freeze.
>
> It looks that simply propagating return value from
> exynos5_counters_get() in exynos5_dmc_get_status() fixes the boot:
>
> # dmesg | grep dmc
> [ 8.838754] exynos-ppmu: new PPMU device registered 10d00000.ppmu
> (ppmu-event3-dmc0_0)
> [ 8.861344] exynos-ppmu: new PPMU device registered 10d10000.ppmu
> (ppmu-event3-dmc0_1)
> [ 8.868488] exynos5-dmc 10c20000.memory-controller: couldn't probe
> performance counters
> [ 8.874417] exynos-ppmu: new PPMU device registered 10d60000.ppmu
> (ppmu-event3-dmc1_0)
> [ 8.886612] exynos-ppmu: new PPMU device registered 10d70000.ppmu
> (ppmu-event3-dmc1_1)
> [ 9.396769] exynos5-dmc 10c20000.memory-controller: DMC initialized,
> in irq mode: 0
>
> I'm still curious why it freezes if getting performance counters is not
> possible.
>
> Best regards
>
Thank you for investigating this issue. Indeed, it's odd why it freezes.
I've seen you patch with the fix.
Regards,
Lukasz