[PATCH v2 0/2] Exynos5422 DMC: adjust to new devfreq monitoring mechanism

[PATCH v2 0/2] Exynos5422 DMC: adjust to new devfreq monitoring mechanism

[Lukasz Luba]

Hi all,

This is a v2 patch set adjusting Exynos5422 DMC to the new devfreq monitoring
mechanism. This time the IRQ mode is explicitly controlled using module
parameter (in default the driver uses polling mode = devfreq monitoring).

The detailed cover letter describing the topic can be found here [1].

The patches should apply on top of Chanwoo's devfreq-next branch, where
the new devfreq mechanism is queued [2]. If there is no objections
I think they can go via this tree, since they logically use it.

Changes:
v2:
- added Reviewed-by from Chanwoo for patch 1/2
- added module_param which controls the mode in which the driver operates
- switched in default to devfreq monitoring mechanism instead of interrupts

Regards,
Lukasz Luba

[1] https://lore.kernel.org/linux-pm/20200708153420.29484-1-lukasz.luba@arm.com/
[2] https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/log/?h=devfreq-next

Lukasz Luba (2):
  memory: samsung: exynos5422-dmc: Adjust polling interval and
    uptreshold
  memory: samsung: exynos5422-dmc: Add module param to control IRQ mode

 drivers/memory/samsung/exynos5422-dmc.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

-- 
2.17.1

[PATCH v2 0/2] Exynos5422 DMC: adjust to new devfreq monitoring mechanism

[Lukasz Luba]

Hi all,

This is a v2 patch set adjusting Exynos5422 DMC to the new devfreq monitoring
mechanism. This time the IRQ mode is explicitly controlled using module
parameter (in default the driver uses polling mode = devfreq monitoring).

The detailed cover letter describing the topic can be found here [1].

The patches should apply on top of Chanwoo's devfreq-next branch, where
the new devfreq mechanism is queued [2]. If there is no objections
I think they can go via this tree, since they logically use it.

Changes:
v2:
- added Reviewed-by from Chanwoo for patch 1/2
- added module_param which controls the mode in which the driver operates
- switched in default to devfreq monitoring mechanism instead of interrupts

Regards,
Lukasz Luba

[1] https://lore.kernel.org/linux-pm/20200708153420.29484-1-lukasz.luba@arm.com/
[2] https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/log/?h=devfreq-next

Lukasz Luba (2):
  memory: samsung: exynos5422-dmc: Adjust polling interval and
    uptreshold
  memory: samsung: exynos5422-dmc: Add module param to control IRQ mode

 drivers/memory/samsung/exynos5422-dmc.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

-- 
2.17.1


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

[PATCH v2 1/2] memory: samsung: exynos5422-dmc: Adjust polling interval and uptreshold

[Lukasz Luba]

In order to react faster and make better decisions under some workloads,
benchmarking the memory subsystem behavior, adjust the polling interval
and upthreshold value used by the simple_ondemand governor.

Reported-by: Willy Wolff <willy.mh.wolff.ml@gmail.com>
Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
---
 drivers/memory/samsung/exynos5422-dmc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/memory/samsung/exynos5422-dmc.c b/drivers/memory/samsung/exynos5422-dmc.c
index 93e9c2429c0d..e03ee35f0ab5 100644
--- a/drivers/memory/samsung/exynos5422-dmc.c
+++ b/drivers/memory/samsung/exynos5422-dmc.c
@@ -1466,10 +1466,10 @@ static int exynos5_dmc_probe(struct platform_device *pdev)
 		 * Setup default thresholds for the devfreq governor.
 		 * The values are chosen based on experiments.
 		 */
-		dmc->gov_data.upthreshold = 30;
+		dmc->gov_data.upthreshold = 10;
 		dmc->gov_data.downdifferential = 5;
 
-		exynos5_dmc_df_profile.polling_ms = 500;
+		exynos5_dmc_df_profile.polling_ms = 100;
 	}
 
 
-- 
2.17.1

[PATCH v2 1/2] memory: samsung: exynos5422-dmc: Adjust polling interval and uptreshold

[Lukasz Luba]

In order to react faster and make better decisions under some workloads,
benchmarking the memory subsystem behavior, adjust the polling interval
and upthreshold value used by the simple_ondemand governor.

Reported-by: Willy Wolff <willy.mh.wolff.ml@gmail.com>
Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
---
 drivers/memory/samsung/exynos5422-dmc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/memory/samsung/exynos5422-dmc.c b/drivers/memory/samsung/exynos5422-dmc.c
index 93e9c2429c0d..e03ee35f0ab5 100644
--- a/drivers/memory/samsung/exynos5422-dmc.c
+++ b/drivers/memory/samsung/exynos5422-dmc.c
@@ -1466,10 +1466,10 @@ static int exynos5_dmc_probe(struct platform_device *pdev)
 		 * Setup default thresholds for the devfreq governor.
 		 * The values are chosen based on experiments.
 		 */
-		dmc->gov_data.upthreshold = 30;
+		dmc->gov_data.upthreshold = 10;
 		dmc->gov_data.downdifferential = 5;
 
-		exynos5_dmc_df_profile.polling_ms = 500;
+		exynos5_dmc_df_profile.polling_ms = 100;
 	}
 
 
-- 
2.17.1


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

Re: [PATCH v2 1/2] memory: samsung: exynos5422-dmc: Adjust polling interval and uptreshold

[Krzysztof Kozlowski]

On Fri, Jul 10, 2020 at 08:11:21PM +0100, Lukasz Luba wrote:
> In order to react faster and make better decisions under some workloads,
> benchmarking the memory subsystem behavior, adjust the polling interval
> and upthreshold value used by the simple_ondemand governor.
> 
> Reported-by: Willy Wolff <willy.mh.wolff.ml@gmail.com>
> Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
> Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
> ---
>  drivers/memory/samsung/exynos5422-dmc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>

Feel free to take it via devfreq tree.

Best regards,
Krzysztof

Re: [PATCH v2 1/2] memory: samsung: exynos5422-dmc: Adjust polling interval and uptreshold

[Krzysztof Kozlowski]

On Fri, Jul 10, 2020 at 08:11:21PM +0100, Lukasz Luba wrote:
> In order to react faster and make better decisions under some workloads,
> benchmarking the memory subsystem behavior, adjust the polling interval
> and upthreshold value used by the simple_ondemand governor.
> 
> Reported-by: Willy Wolff <willy.mh.wolff.ml@gmail.com>
> Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
> Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
> ---
>  drivers/memory/samsung/exynos5422-dmc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>

Feel free to take it via devfreq tree.

Best regards,
Krzysztof

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

Re: [PATCH v2 1/2] memory: samsung: exynos5422-dmc: Adjust polling interval and uptreshold

[Lukasz Luba]

On 7/20/20 3:46 PM, Krzysztof Kozlowski wrote:
> On Fri, Jul 10, 2020 at 08:11:21PM +0100, Lukasz Luba wrote:
>> In order to react faster and make better decisions under some workloads,
>> benchmarking the memory subsystem behavior, adjust the polling interval
>> and upthreshold value used by the simple_ondemand governor.
>>
>> Reported-by: Willy Wolff <willy.mh.wolff.ml@gmail.com>
>> Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
>> Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
>> ---
>>   drivers/memory/samsung/exynos5422-dmc.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
> 
> Feel free to take it via devfreq tree.
> 
> Best regards,
> Krzysztof
> 

Thank you Krzysztof for the review.

Regards,
Lukasz

Re: [PATCH v2 1/2] memory: samsung: exynos5422-dmc: Adjust polling interval and uptreshold

[Lukasz Luba]

On 7/20/20 3:46 PM, Krzysztof Kozlowski wrote:
> On Fri, Jul 10, 2020 at 08:11:21PM +0100, Lukasz Luba wrote:
>> In order to react faster and make better decisions under some workloads,
>> benchmarking the memory subsystem behavior, adjust the polling interval
>> and upthreshold value used by the simple_ondemand governor.
>>
>> Reported-by: Willy Wolff <willy.mh.wolff.ml@gmail.com>
>> Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
>> Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
>> ---
>>   drivers/memory/samsung/exynos5422-dmc.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
> 
> Feel free to take it via devfreq tree.
> 
> Best regards,
> Krzysztof
> 

Thank you Krzysztof for the review.

Regards,
Lukasz

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

[PATCH v2 2/2] memory: samsung: exynos5422-dmc: Add module param to control IRQ mode

[Lukasz Luba]

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 <willy.mh.wolff.ml@gmail.com>
Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
---
 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

[PATCH v2 2/2] memory: samsung: exynos5422-dmc: Add module param to control IRQ mode

[Lukasz Luba]

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 <willy.mh.wolff.ml@gmail.com>
Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
---
 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


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

Re: [PATCH v2 2/2] memory: samsung: exynos5422-dmc: Add module param to control IRQ mode

[Bartlomiej Zolnierkiewicz]

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 <willy.mh.wolff.ml@gmail.com>
> Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
> ---
>  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;
>  
> 

Re: [PATCH v2 2/2] memory: samsung: exynos5422-dmc: Add module param to control IRQ mode

[Bartlomiej Zolnierkiewicz]

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 <willy.mh.wolff.ml@gmail.com>
> Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
> ---
>  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;
>  
> 


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

Re: [PATCH v2 2/2] memory: samsung: exynos5422-dmc: Add module param to control IRQ mode

[Lukasz Luba]

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 <willy.mh.wolff.ml@gmail.com>
>> Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
>> ---
>>   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;
>>   
>>
> 

Re: [PATCH v2 2/2] memory: samsung: exynos5422-dmc: Add module param to control IRQ mode

[Lukasz Luba]

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 <willy.mh.wolff.ml@gmail.com>
>> Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
>> ---
>>   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;
>>   
>>
> 

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

Re: [PATCH v2 2/2] memory: samsung: exynos5422-dmc: Add module param to control IRQ mode

[Willy Wolff]

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 <willy.mh.wolff.ml@gmail.com>
>>>Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
>>>---
>>>  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;
>>>
>>

Re: [PATCH v2 2/2] memory: samsung: exynos5422-dmc: Add module param to control IRQ mode

[Willy Wolff]

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 <willy.mh.wolff.ml@gmail.com>
>>>Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
>>>---
>>>  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;
>>>
>>

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

Re: [PATCH v2 2/2] memory: samsung: exynos5422-dmc: Add module param to control IRQ mode

[Lukasz Luba]

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 <willy.mh.wolff.ml@gmail.com>
>>>> Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
>>>> ---
>>>>  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;
>>>>
>>>

Re: [PATCH v2 2/2] memory: samsung: exynos5422-dmc: Add module param to control IRQ mode

[Lukasz Luba]

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 <willy.mh.wolff.ml@gmail.com>
>>>> Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
>>>> ---
>>>>  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;
>>>>
>>>

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

Re: [PATCH v2 2/2] memory: samsung: exynos5422-dmc: Add module param to control IRQ mode

[Lukasz Luba]

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

Re: [PATCH v2 2/2] memory: samsung: exynos5422-dmc: Add module param to control IRQ mode

[Lukasz Luba]

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

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

Re: [PATCH v2 2/2] memory: samsung: exynos5422-dmc: Add module param to control IRQ mode

[Bartlomiej Zolnierkiewicz]

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

Re: [PATCH v2 2/2] memory: samsung: exynos5422-dmc: Add module param to control IRQ mode

[Bartlomiej Zolnierkiewicz]

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

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

Re: [PATCH v2 2/2] memory: samsung: exynos5422-dmc: Add module param to control IRQ mode

[Krzysztof Kozlowski]

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 <willy.mh.wolff.ml@gmail.com>
> Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
> ---
>  drivers/memory/samsung/exynos5422-dmc.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)

Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof

Re: [PATCH v2 2/2] memory: samsung: exynos5422-dmc: Add module param to control IRQ mode

[Krzysztof Kozlowski]

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 <willy.mh.wolff.ml@gmail.com>
> Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
> ---
>  drivers/memory/samsung/exynos5422-dmc.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)

Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof

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

Re: [PATCH v2 2/2] memory: samsung: exynos5422-dmc: Add module param to control IRQ mode

[Marek Szyprowski]

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 <willy.mh.wolff.ml@gmail.com>
> Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>

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

Re: [PATCH v2 2/2] memory: samsung: exynos5422-dmc: Add module param to control IRQ mode

[Marek Szyprowski]

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 <willy.mh.wolff.ml@gmail.com>
> Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>

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


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

Re: [PATCH v2 2/2] memory: samsung: exynos5422-dmc: Add module param to control IRQ mode

[Marek Szyprowski]

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 <willy.mh.wolff.ml@gmail.com>
>> Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
>
> 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

Re: [PATCH v2 2/2] memory: samsung: exynos5422-dmc: Add module param to control IRQ mode

[Marek Szyprowski]

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 <willy.mh.wolff.ml@gmail.com>
>> Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
>
> 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


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

Re: [PATCH v2 2/2] memory: samsung: exynos5422-dmc: Add module param to control IRQ mode

[Lukasz Luba]

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 <willy.mh.wolff.ml@gmail.com>
>>> Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
>>
>> 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

Re: [PATCH v2 2/2] memory: samsung: exynos5422-dmc: Add module param to control IRQ mode

[Lukasz Luba]

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 <willy.mh.wolff.ml@gmail.com>
>>> Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
>>
>> 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

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

Re: [PATCH v2 0/2] Exynos5422 DMC: adjust to new devfreq monitoring mechanism

[Chanwoo Choi]

Hi Lukasz,

On 7/11/20 4:11 AM, Lukasz Luba wrote:
> Hi all,
> 
> This is a v2 patch set adjusting Exynos5422 DMC to the new devfreq monitoring
> mechanism. This time the IRQ mode is explicitly controlled using module
> parameter (in default the driver uses polling mode = devfreq monitoring).
> 
> The detailed cover letter describing the topic can be found here [1].
> 
> The patches should apply on top of Chanwoo's devfreq-next branch, where
> the new devfreq mechanism is queued [2]. If there is no objections
> I think they can go via this tree, since they logically use it.
> 
> Changes:
> v2:
> - added Reviewed-by from Chanwoo for patch 1/2
> - added module_param which controls the mode in which the driver operates
> - switched in default to devfreq monitoring mechanism instead of interrupts
> 
> Regards,
> Lukasz Luba
> 
> [1] https://lore.kernel.org/linux-pm/20200708153420.29484-1-lukasz.luba@arm.com/
> [2] https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/log/?h=devfreq-next
> 
> Lukasz Luba (2):
>   memory: samsung: exynos5422-dmc: Adjust polling interval and
>     uptreshold
>   memory: samsung: exynos5422-dmc: Add module param to control IRQ mode
> 
>  drivers/memory/samsung/exynos5422-dmc.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 

Applied them to devfreq-next branch. Thanks.

-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

Re: [PATCH v2 0/2] Exynos5422 DMC: adjust to new devfreq monitoring mechanism

[Chanwoo Choi]

Hi Lukasz,

On 7/11/20 4:11 AM, Lukasz Luba wrote:
> Hi all,
> 
> This is a v2 patch set adjusting Exynos5422 DMC to the new devfreq monitoring
> mechanism. This time the IRQ mode is explicitly controlled using module
> parameter (in default the driver uses polling mode = devfreq monitoring).
> 
> The detailed cover letter describing the topic can be found here [1].
> 
> The patches should apply on top of Chanwoo's devfreq-next branch, where
> the new devfreq mechanism is queued [2]. If there is no objections
> I think they can go via this tree, since they logically use it.
> 
> Changes:
> v2:
> - added Reviewed-by from Chanwoo for patch 1/2
> - added module_param which controls the mode in which the driver operates
> - switched in default to devfreq monitoring mechanism instead of interrupts
> 
> Regards,
> Lukasz Luba
> 
> [1] https://lore.kernel.org/linux-pm/20200708153420.29484-1-lukasz.luba@arm.com/
> [2] https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/log/?h=devfreq-next
> 
> Lukasz Luba (2):
>   memory: samsung: exynos5422-dmc: Adjust polling interval and
>     uptreshold
>   memory: samsung: exynos5422-dmc: Add module param to control IRQ mode
> 
>  drivers/memory/samsung/exynos5422-dmc.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 

Applied them to devfreq-next branch. Thanks.

-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

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

Re: [PATCH v2 0/2] Exynos5422 DMC: adjust to new devfreq monitoring mechanism

[Lukasz Luba]

On 7/24/20 2:40 AM, Chanwoo Choi wrote:
> Hi Lukasz,
> 
> On 7/11/20 4:11 AM, Lukasz Luba wrote:
>> Hi all,
>>
>> This is a v2 patch set adjusting Exynos5422 DMC to the new devfreq monitoring
>> mechanism. This time the IRQ mode is explicitly controlled using module
>> parameter (in default the driver uses polling mode = devfreq monitoring).
>>
>> The detailed cover letter describing the topic can be found here [1].
>>
>> The patches should apply on top of Chanwoo's devfreq-next branch, where
>> the new devfreq mechanism is queued [2]. If there is no objections
>> I think they can go via this tree, since they logically use it.
>>
>> Changes:
>> v2:
>> - added Reviewed-by from Chanwoo for patch 1/2
>> - added module_param which controls the mode in which the driver operates
>> - switched in default to devfreq monitoring mechanism instead of interrupts
>>
>> Regards,
>> Lukasz Luba
>>
>> [1] https://lore.kernel.org/linux-pm/20200708153420.29484-1-lukasz.luba@arm.com/
>> [2] https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/log/?h=devfreq-next
>>
>> Lukasz Luba (2):
>>    memory: samsung: exynos5422-dmc: Adjust polling interval and
>>      uptreshold
>>    memory: samsung: exynos5422-dmc: Add module param to control IRQ mode
>>
>>   drivers/memory/samsung/exynos5422-dmc.c | 13 +++++++++----
>>   1 file changed, 9 insertions(+), 4 deletions(-)
>>
> 
> Applied them to devfreq-next branch. Thanks.
> 

Thank you Chanwoo

Regards,
Lukasz

Re: [PATCH v2 0/2] Exynos5422 DMC: adjust to new devfreq monitoring mechanism

[Lukasz Luba]

On 7/24/20 2:40 AM, Chanwoo Choi wrote:
> Hi Lukasz,
> 
> On 7/11/20 4:11 AM, Lukasz Luba wrote:
>> Hi all,
>>
>> This is a v2 patch set adjusting Exynos5422 DMC to the new devfreq monitoring
>> mechanism. This time the IRQ mode is explicitly controlled using module
>> parameter (in default the driver uses polling mode = devfreq monitoring).
>>
>> The detailed cover letter describing the topic can be found here [1].
>>
>> The patches should apply on top of Chanwoo's devfreq-next branch, where
>> the new devfreq mechanism is queued [2]. If there is no objections
>> I think they can go via this tree, since they logically use it.
>>
>> Changes:
>> v2:
>> - added Reviewed-by from Chanwoo for patch 1/2
>> - added module_param which controls the mode in which the driver operates
>> - switched in default to devfreq monitoring mechanism instead of interrupts
>>
>> Regards,
>> Lukasz Luba
>>
>> [1] https://lore.kernel.org/linux-pm/20200708153420.29484-1-lukasz.luba@arm.com/
>> [2] https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/log/?h=devfreq-next
>>
>> Lukasz Luba (2):
>>    memory: samsung: exynos5422-dmc: Adjust polling interval and
>>      uptreshold
>>    memory: samsung: exynos5422-dmc: Add module param to control IRQ mode
>>
>>   drivers/memory/samsung/exynos5422-dmc.c | 13 +++++++++----
>>   1 file changed, 9 insertions(+), 4 deletions(-)
>>
> 
> Applied them to devfreq-next branch. Thanks.
> 

Thank you Chanwoo

Regards,
Lukasz

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