Linux-Samsung-soc Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 0/2] Exynos5422 DMC: adjust to new devfreq monitoring mechanism
@ 2020-07-08 15:34 Lukasz Luba
  2020-07-08 15:34 ` [PATCH 1/2] memory: samsung: exynos5422-dmc: Adjust polling interval and uptreshold Lukasz Luba
  2020-07-08 15:34 ` [PATCH 2/2] ARM: dts: exynos: Remove interrupts from DMC controller in Exynos5422 Lukasz Luba
  0 siblings, 2 replies; 16+ messages in thread
From: Lukasz Luba @ 2020-07-08 15:34 UTC (permalink / raw)
  To: linux-kernel, linux-pm, linux-arm-kernel, linux-samsung-soc
  Cc: lukasz.luba, willy.mh.wolff.ml, k.konieczny, cw00.choi,
	b.zolnierkie, krzk, chanwoo, myungjoo.ham, kyungmin.park,
	s.nawrocki, kgene

Hi all,

There have been discussions around devfreq, it's related drivers
and governors, reporting some issues [1][2]. There are a patch sets
trying to address these issues [2][3], discussion is still ongoing.

This patch set is a follow up for changes presented in [2]. It moves
from internal interrupts to devfreq monitoring mechanism with fixed
polling interval in the memory controller. The value of polling and
uptreshold have been adjusted in order to get better reaction in the
benchmark workload [4]. The test workload is specific and I still
have to spend some more time to understand impact of different CPUs
or different utilization and governor's decisions. Now, with the patches
it looks better.
For other future adjustments, not specific to a particular
driver's code, I would rather see a different mechanism, which I'll post
shortly as an RFC.

The patch 1/2 needs [2] series, while the patch 2/2 is on top of Krzysztof's
today next/dt branch [5] (I assume patch 2/2 would go through his tree).

Here are test results for the benchmark from [4], showing some more accurate
reaction for the benchmark workload (gentle ask to Willy to confirm this):
simple_ondlemand:
littles: 1.40374 s (was 4.74308 s)
bigs: 1.44519 s (was 4.76556 s)
performance:
littles: 1.13635 s
bigs: 1.17349 s

-------------------------------8<--------------------------------
oller/trans_stat /odroid/benchmark# cat /sys/class/devfreq/10c20000.memory-contro
     From  :   To
           : 165000000 206000000 275000000 413000000 543000000 633000000 728000000 825000000   time(ms)
* 165000000:         0        15        24        13        12         2         0        11   1128580
  206000000:        12         0         8         7         3         1         0         0      3760
  275000000:        17         8         0        12        10         2         0         5      6340
  413000000:        19         5         9         0         7         5         0         9     10290
  543000000:        18         0         6        13         0        31         1         8      7510
  633000000:         4         3         3         6        27         0         0         1     11440
  728000000:         1         0         0         0         0         0         0         0       110
  825000000:         7         0         4         3        18         3         0         0     23490
Total transition : 373
root@odroid:/home/odroid/benchmark# bash source/scripts/test_dvfs_mem.sh


mem_gov = simple_ondemand at 413000000

     From  :   To
           : 165000000 206000000 275000000 413000000 543000000 633000000 728000000 825000000   time(ms)
  165000000:         0        15        26        14        12         2         0        11   1134220
  206000000:        12         0         8         7         3         1         0         0      3760
  275000000:        18         8         0        13        10         2         0         5      6670
* 413000000:        20         5         9         0         7         5         0         9     10460
  543000000:        18         0         6        13         0        31         1         8      7510
  633000000:         4         3         3         6        27         0         0         1     11440
  728000000:         1         0         0         0         0         0         0         0       110
  825000000:         7         0         4         3        18         3         0         0     23490
Total transition : 379


 Running on the LITTLE cluster

array_size rounded to fit a CACHE_LINE_SIZE from 33554431 to 33554432
sizeof(size_t) = 4
array_size = 33554432
==> 134217728 b; 131072 Kb; 128 Mb
stride = 0
nr_iter = 9722222
nr_iter_2 = 1
effective_nr_iter = 9722222
cpu_freq = 1400000
==> 1.4 GHz; 1400 MHz; 1400000 KHz
total time = 1403736317 ns; 1.40374 s
time per iter 144.384 ns
estimated cycles per iter 202.138 c


 Running on the big cluster

array_size rounded to fit a CACHE_LINE_SIZE from 33554431 to 33554432
sizeof(size_t) = 4
array_size = 33554432
==> 134217728 b; 131072 Kb; 128 Mb
stride = 0
nr_iter = 9722222
nr_iter_2 = 1
effective_nr_iter = 9722222
cpu_freq = 2000000
==> 2 GHz; 2000 MHz; 2000000 KHz
total time = 1445186489 ns; 1.44519 s
time per iter 148.648 ns
estimated cycles per iter 297.296 c
     From  :   To
           : 165000000 206000000 275000000 413000000 543000000 633000000 728000000 825000000   time(ms)
  165000000:         0        15        26        14        12         2         0        11   1134220
  206000000:        12         0         8         7         3         1         0         0      3760
  275000000:        18         8         0        13        15         2         0         5      7220
  413000000:        20         5        10         0         7         5         0         9     10510
  543000000:        18         0        10        13         0        32         1         9      8280
* 633000000:         4         3         3         6        28         0         0         1     14160
  728000000:         1         0         0         0         0         0         0         0       110
  825000000:         7         0         4         3        18         4         0         0     23930
Total transition : 393


mem_gov = performance at 825000000

     From  :   To
           : 165000000 206000000 275000000 413000000 543000000 633000000 728000000 825000000   time(ms)
  165000000:         0        15        26        14        12         2         0        11   1134220
  206000000:        12         0         8         7         3         1         0         0      3760
  275000000:        18         8         0        13        15         2         0         6      7300
  413000000:        20         5        10         0         7         5         0         9     10510
  543000000:        18         0        10        13         0        32         1         9      8280
  633000000:         4         3         4         6        28         0         0         1     14190
  728000000:         1         0         0         0         0         0         0         0       110
* 825000000:         7         0         4         3        18         4         0         0     23980
Total transition : 395


 Running on the LITTLE cluster

array_size rounded to fit a CACHE_LINE_SIZE from 33554431 to 33554432
sizeof(size_t) = 4
array_size = 33554432
==> 134217728 b; 131072 Kb; 128 Mb
stride = 0
nr_iter = 9722222
nr_iter_2 = 1
effective_nr_iter = 9722222
cpu_freq = 1400000
==> 1.4 GHz; 1400 MHz; 1400000 KHz
total time = 1136350257 ns; 1.13635 s
time per iter 116.882 ns
estimated cycles per iter 163.634 c


 Running on the big cluster

array_size rounded to fit a CACHE_LINE_SIZE from 33554431 to 33554432
sizeof(size_t) = 4
array_size = 33554432
==> 134217728 b; 131072 Kb; 128 Mb
stride = 0
nr_iter = 9722222
nr_iter_2 = 1
effective_nr_iter = 9722222
cpu_freq = 2000000
==> 2 GHz; 2000 MHz; 2000000 KHz
total time = 1173486470 ns; 1.17349 s
time per iter 120.701 ns
estimated cycles per iter 241.403 c
     From  :   To
           : 165000000 206000000 275000000 413000000 543000000 633000000 728000000 825000000   time(ms)
  165000000:         0        15        26        14        12         2         0        11   1134220
  206000000:        12         0         8         7         3         1         0         0      3760
  275000000:        18         8         0        13        15         2         0         6      7300
  413000000:        20         5        10         0         7         5         0         9     10510
  543000000:        18         0        10        13         0        32         1         9      8280
  633000000:         4         3         4         6        28         0         0         1     14190
  728000000:         1         0         0         0         0         0         0         0       110
* 825000000:         7         0         4         3        18         4         0         0     27760
Total transition : 395
root@odroid:/home/odroid/benchmark#
oller/trans_stat /odroid/benchmark# cat /sys/class/devfreq/10c20000.memory-contro
     From  :   To
           : 165000000 206000000 275000000 413000000 543000000 633000000 728000000 825000000   time(ms)
* 165000000:         0        15        26        14        12         2         0        11   1138930
  206000000:        12         0         8         7         3         1         0         0      3760
  275000000:        18         8         0        13        15         2         0         6      7300
  413000000:        20         5        10         0         7         5         0         9     10510
  543000000:        19         0        10        13         0        32         1         9      8390
  633000000:         4         3         4         6        28         0         0         1     14190
  728000000:         1         0         0         0         0         0         0         0       110
  825000000:         7         0         4         3        19         4         0         0     28280
Total transition : 397
root@odroid:/home/odroid/benchmark#
----------------------------------------->8--------------------------------------------

Regards,
Lukasz Luba

[1] https://lore.kernel.org/linux-pm/20200623164733.qbhua7b6cg2umafj@macmini.local/
[2] https://lore.kernel.org/linux-pm/20200703062622.11773-1-cw00.choi@samsung.com/
[3] https://lore.kernel.org/linux-pm/20200703100810.12304-1-cw00.choi@samsung.com/
[4] https://github.com/wwilly/benchmark.git
[5] https://git.kernel.org/pub/scm/linux/kernel/git/krzk/linux.git/log/?h=next/dt




Lukasz Luba (2):
  memory: samsung: exynos5422-dmc: Adjust polling interval and
    uptreshold
  ARM: dts: exynos: Remove interrupts from DMC controller in Exynos5422

 arch/arm/boot/dts/exynos5420.dtsi       | 3 ---
 drivers/memory/samsung/exynos5422-dmc.c | 4 ++--
 2 files changed, 2 insertions(+), 5 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 1/2] memory: samsung: exynos5422-dmc: Adjust polling interval and uptreshold
  2020-07-08 15:34 [PATCH 0/2] Exynos5422 DMC: adjust to new devfreq monitoring mechanism Lukasz Luba
@ 2020-07-08 15:34 ` Lukasz Luba
  2020-07-09  4:08   ` Chanwoo Choi
  2020-07-08 15:34 ` [PATCH 2/2] ARM: dts: exynos: Remove interrupts from DMC controller in Exynos5422 Lukasz Luba
  1 sibling, 1 reply; 16+ messages in thread
From: Lukasz Luba @ 2020-07-08 15:34 UTC (permalink / raw)
  To: linux-kernel, linux-pm, linux-arm-kernel, linux-samsung-soc
  Cc: lukasz.luba, willy.mh.wolff.ml, k.konieczny, cw00.choi,
	b.zolnierkie, krzk, chanwoo, myungjoo.ham, kyungmin.park,
	s.nawrocki, kgene

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


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 2/2] ARM: dts: exynos: Remove interrupts from DMC controller in Exynos5422
  2020-07-08 15:34 [PATCH 0/2] Exynos5422 DMC: adjust to new devfreq monitoring mechanism Lukasz Luba
  2020-07-08 15:34 ` [PATCH 1/2] memory: samsung: exynos5422-dmc: Adjust polling interval and uptreshold Lukasz Luba
@ 2020-07-08 15:34 ` Lukasz Luba
  1 sibling, 0 replies; 16+ messages in thread
From: Lukasz Luba @ 2020-07-08 15:34 UTC (permalink / raw)
  To: linux-kernel, linux-pm, linux-arm-kernel, linux-samsung-soc
  Cc: lukasz.luba, willy.mh.wolff.ml, k.konieczny, cw00.choi,
	b.zolnierkie, krzk, chanwoo, myungjoo.ham, kyungmin.park,
	s.nawrocki, kgene

The interrupts in Dynamic Memory Controller in Exynos5422 and Odroid
XU3-family boards are no longer needed. They have been used in order
to workaround some issues in scheduled work in devfreq. Now when the
devfreq framework design is improved, remove the interrupt driven
approach and rely on devfreq monitoring mechanism with fixed intervals.

Reported-by: Willy Wolff <willy.mh.wolff.ml@gmail.com>
Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
---
 arch/arm/boot/dts/exynos5420.dtsi | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/arch/arm/boot/dts/exynos5420.dtsi b/arch/arm/boot/dts/exynos5420.dtsi
index c76460b70532..967868cc8211 100644
--- a/arch/arm/boot/dts/exynos5420.dtsi
+++ b/arch/arm/boot/dts/exynos5420.dtsi
@@ -240,9 +240,6 @@
 		dmc: memory-controller@10c20000 {
 			compatible = "samsung,exynos5422-dmc";
 			reg = <0x10c20000 0x10000>, <0x10c30000 0x10000>;
-			interrupt-parent = <&combiner>;
-			interrupts = <16 0>, <16 1>;
-			interrupt-names = "drex_0", "drex_1";
 			clocks = <&clock CLK_FOUT_SPLL>,
 				 <&clock CLK_MOUT_SCLK_SPLL>,
 				 <&clock CLK_FF_DOUT_SPLL2>,
-- 
2.17.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] memory: samsung: exynos5422-dmc: Adjust polling interval and uptreshold
  2020-07-08 15:34 ` [PATCH 1/2] memory: samsung: exynos5422-dmc: Adjust polling interval and uptreshold Lukasz Luba
@ 2020-07-09  4:08   ` Chanwoo Choi
  2020-07-10  8:34     ` Lukasz Luba
  0 siblings, 1 reply; 16+ messages in thread
From: Chanwoo Choi @ 2020-07-09  4:08 UTC (permalink / raw)
  To: Lukasz Luba, linux-kernel, linux-pm, linux-arm-kernel, linux-samsung-soc
  Cc: willy.mh.wolff.ml, k.konieczny, b.zolnierkie, krzk, chanwoo,
	myungjoo.ham, kyungmin.park, s.nawrocki, kgene

Hi Lukasz,

On 7/9/20 12:34 AM, 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>
> 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;
>  	}
>  
>  
> 

Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>

-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] memory: samsung: exynos5422-dmc: Adjust polling interval and uptreshold
  2020-07-09  4:08   ` Chanwoo Choi
@ 2020-07-10  8:34     ` Lukasz Luba
  2020-07-10 12:45       ` Krzysztof Kozlowski
  2020-07-10 13:13       ` Marek Szyprowski
  0 siblings, 2 replies; 16+ messages in thread
From: Lukasz Luba @ 2020-07-10  8:34 UTC (permalink / raw)
  To: Chanwoo Choi, linux-kernel, linux-pm, linux-arm-kernel,
	linux-samsung-soc
  Cc: willy.mh.wolff.ml, k.konieczny, b.zolnierkie, krzk, chanwoo,
	myungjoo.ham, kyungmin.park, s.nawrocki, kgene

Hi Chanwoo,

On 7/9/20 5:08 AM, Chanwoo Choi wrote:
> Hi Lukasz,
> 
> On 7/9/20 12:34 AM, 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>
>> 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;
>>   	}
>>   
>>   
>>
> 
> Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
> 

Thank you for the review. Do you think this patch could go through
your tree together with your patches?

I don't know Krzysztof's opinion about the patch 2/2, but
I would expect, assuming the patch itself is correct, he would
like to take it into his next/dt branch.

Regards,
Lukasz

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] memory: samsung: exynos5422-dmc: Adjust polling interval and uptreshold
  2020-07-10  8:34     ` Lukasz Luba
@ 2020-07-10 12:45       ` Krzysztof Kozlowski
  2020-07-10 12:56         ` Lukasz Luba
  2020-07-10 13:13       ` Marek Szyprowski
  1 sibling, 1 reply; 16+ messages in thread
From: Krzysztof Kozlowski @ 2020-07-10 12:45 UTC (permalink / raw)
  To: Lukasz Luba
  Cc: Chanwoo Choi, linux-kernel, linux-pm, linux-arm-kernel,
	linux-samsung-soc, willy.mh.wolff.ml, k.konieczny, b.zolnierkie,
	chanwoo, myungjoo.ham, kyungmin.park, s.nawrocki, kgene

On Fri, Jul 10, 2020 at 09:34:45AM +0100, Lukasz Luba wrote:
> Hi Chanwoo,
> 
> On 7/9/20 5:08 AM, Chanwoo Choi wrote:
> > Hi Lukasz,
> > 
> > On 7/9/20 12:34 AM, 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>
> > > 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;
> > >   	}
> > > 
> > 
> > Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
> > 
> 
> Thank you for the review. Do you think this patch could go through
> your tree together with your patches?
> 
> I don't know Krzysztof's opinion about the patch 2/2, but
> I would expect, assuming the patch itself is correct, he would
> like to take it into his next/dt branch.

In the cover letter you mentioned that this is a follow up for the
Chanwoo's patchset. But are these patches really depending on it? Can
they be picked up independently?

The DTS patch must go through arm soc, so I will take it. If it really
depends on driver changes, then it has to wait for next release.

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] memory: samsung: exynos5422-dmc: Adjust polling interval and uptreshold
  2020-07-10 12:45       ` Krzysztof Kozlowski
@ 2020-07-10 12:56         ` Lukasz Luba
  2020-07-10 13:07           ` Krzysztof Kozlowski
  2020-07-10 13:49           ` Bartlomiej Zolnierkiewicz
  0 siblings, 2 replies; 16+ messages in thread
From: Lukasz Luba @ 2020-07-10 12:56 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Chanwoo Choi, linux-kernel, linux-pm, linux-arm-kernel,
	linux-samsung-soc, willy.mh.wolff.ml, k.konieczny, b.zolnierkie,
	chanwoo, myungjoo.ham, kyungmin.park, s.nawrocki, kgene



On 7/10/20 1:45 PM, Krzysztof Kozlowski wrote:
> On Fri, Jul 10, 2020 at 09:34:45AM +0100, Lukasz Luba wrote:
>> Hi Chanwoo,
>>
>> On 7/9/20 5:08 AM, Chanwoo Choi wrote:
>>> Hi Lukasz,
>>>
>>> On 7/9/20 12:34 AM, 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>
>>>> 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;
>>>>    	}
>>>>
>>>
>>> Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
>>>
>>
>> Thank you for the review. Do you think this patch could go through
>> your tree together with your patches?
>>
>> I don't know Krzysztof's opinion about the patch 2/2, but
>> I would expect, assuming the patch itself is correct, he would
>> like to take it into his next/dt branch.
> 
> In the cover letter you mentioned that this is a follow up for the
> Chanwoo's patchset. But are these patches really depending on it? Can
> they be picked up independently?


They are not heavily dependent on Chanwoo's patches.
Yes, they can be picked up independently.
I just wanted to mention that the patch 1/2 was produced on the
code base which had already applied Chanwoo's patch for DMC.
If you like to take both 1/2 and 2/2 into your tree, it's good.

Thank you for having a look on this.

Regards,
Lukasz


> 
> The DTS patch must go through arm soc, so I will take it. If it really
> depends on driver changes, then it has to wait for next release.
> 
> Best regards,
> Krzysztof
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] memory: samsung: exynos5422-dmc: Adjust polling interval and uptreshold
  2020-07-10 12:56         ` Lukasz Luba
@ 2020-07-10 13:07           ` Krzysztof Kozlowski
  2020-07-10 13:49           ` Bartlomiej Zolnierkiewicz
  1 sibling, 0 replies; 16+ messages in thread
From: Krzysztof Kozlowski @ 2020-07-10 13:07 UTC (permalink / raw)
  To: Lukasz Luba
  Cc: Chanwoo Choi, linux-kernel, linux-pm, linux-arm-kernel,
	linux-samsung-soc, willy.mh.wolff.ml, k.konieczny, b.zolnierkie,
	chanwoo, myungjoo.ham, kyungmin.park, s.nawrocki, kgene

On Fri, Jul 10, 2020 at 01:56:33PM +0100, Lukasz Luba wrote:
> 
> 
> On 7/10/20 1:45 PM, Krzysztof Kozlowski wrote:
> > On Fri, Jul 10, 2020 at 09:34:45AM +0100, Lukasz Luba wrote:
> > > Hi Chanwoo,
> > > 
> > > On 7/9/20 5:08 AM, Chanwoo Choi wrote:
> > > > Hi Lukasz,
> > > > 
> > > > On 7/9/20 12:34 AM, 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>
> > > > > 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;
> > > > >    	}
> > > > > 
> > > > 
> > > > Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
> > > > 
> > > 
> > > Thank you for the review. Do you think this patch could go through
> > > your tree together with your patches?
> > > 
> > > I don't know Krzysztof's opinion about the patch 2/2, but
> > > I would expect, assuming the patch itself is correct, he would
> > > like to take it into his next/dt branch.
> > 
> > In the cover letter you mentioned that this is a follow up for the
> > Chanwoo's patchset. But are these patches really depending on it? Can
> > they be picked up independently?
> 
> 
> They are not heavily dependent on Chanwoo's patches.
> Yes, they can be picked up independently.
> I just wanted to mention that the patch 1/2 was produced on the
> code base which had already applied Chanwoo's patch for DMC.
> If you like to take both 1/2 and 2/2 into your tree, it's good.

Then let me take them. If there is more development ongoing, it would
create unnecessary merges between trees.

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] memory: samsung: exynos5422-dmc: Adjust polling interval and uptreshold
  2020-07-10  8:34     ` Lukasz Luba
  2020-07-10 12:45       ` Krzysztof Kozlowski
@ 2020-07-10 13:13       ` Marek Szyprowski
  2020-07-10 13:19         ` Krzysztof Kozlowski
  1 sibling, 1 reply; 16+ messages in thread
From: Marek Szyprowski @ 2020-07-10 13:13 UTC (permalink / raw)
  To: Lukasz Luba, Chanwoo Choi, linux-kernel, linux-pm,
	linux-arm-kernel, linux-samsung-soc
  Cc: willy.mh.wolff.ml, k.konieczny, b.zolnierkie, krzk, chanwoo,
	myungjoo.ham, kyungmin.park, s.nawrocki, kgene

Hi Lukasz,

On 10.07.2020 10:34, Lukasz Luba wrote:
> Hi Chanwoo,
>
> On 7/9/20 5:08 AM, Chanwoo Choi wrote:
>> Hi Lukasz,
>>
>> On 7/9/20 12:34 AM, 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>
>>> 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;
>>>       }
>>>
>>
>> Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
>>
>
> Thank you for the review. Do you think this patch could go through
> your tree together with your patches?
>
> I don't know Krzysztof's opinion about the patch 2/2, but
> I would expect, assuming the patch itself is correct, he would
> like to take it into his next/dt branch.

Is there really a need to remove the interrupts property? imho they are 
correct hw description, it just a matter of the driver to use or not to 
use them.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] memory: samsung: exynos5422-dmc: Adjust polling interval and uptreshold
  2020-07-10 13:13       ` Marek Szyprowski
@ 2020-07-10 13:19         ` Krzysztof Kozlowski
  2020-07-10 13:41           ` Lukasz Luba
  0 siblings, 1 reply; 16+ messages in thread
From: Krzysztof Kozlowski @ 2020-07-10 13:19 UTC (permalink / raw)
  To: Marek Szyprowski
  Cc: Lukasz Luba, Chanwoo Choi, linux-kernel, linux-pm,
	linux-arm-kernel, linux-samsung-soc, willy.mh.wolff.ml,
	k.konieczny, b.zolnierkie, chanwoo, myungjoo.ham, kyungmin.park,
	s.nawrocki, kgene

On Fri, Jul 10, 2020 at 03:13:18PM +0200, Marek Szyprowski wrote:
> Hi Lukasz,
> 
> On 10.07.2020 10:34, Lukasz Luba wrote:
> > Hi Chanwoo,
> >
> > On 7/9/20 5:08 AM, Chanwoo Choi wrote:
> >> Hi Lukasz,
> >>
> >> On 7/9/20 12:34 AM, 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>
> >>> 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;
> >>>       }
> >>>
> >>
> >> Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
> >>
> >
> > Thank you for the review. Do you think this patch could go through
> > your tree together with your patches?
> >
> > I don't know Krzysztof's opinion about the patch 2/2, but
> > I would expect, assuming the patch itself is correct, he would
> > like to take it into his next/dt branch.
> 
> Is there really a need to remove the interrupts property? imho they are 
> correct hw description, it just a matter of the driver to use or not to 
> use them.

That's actually very good point. I would also prefer to leave them.
However I understood that driver chooses mode depending on the property.

In such case, maybe as you said, let's switch to polling mode
unconditionally?

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] memory: samsung: exynos5422-dmc: Adjust polling interval and uptreshold
  2020-07-10 13:19         ` Krzysztof Kozlowski
@ 2020-07-10 13:41           ` Lukasz Luba
  2020-07-10 14:49             ` Krzysztof Kozlowski
  0 siblings, 1 reply; 16+ messages in thread
From: Lukasz Luba @ 2020-07-10 13:41 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Marek Szyprowski
  Cc: Chanwoo Choi, linux-kernel, linux-pm, linux-arm-kernel,
	linux-samsung-soc, willy.mh.wolff.ml, k.konieczny, b.zolnierkie,
	chanwoo, myungjoo.ham, kyungmin.park, s.nawrocki, kgene



On 7/10/20 2:19 PM, Krzysztof Kozlowski wrote:
> On Fri, Jul 10, 2020 at 03:13:18PM +0200, Marek Szyprowski wrote:
>> Hi Lukasz,
>>
>> On 10.07.2020 10:34, Lukasz Luba wrote:
>>> Hi Chanwoo,
>>>
>>> On 7/9/20 5:08 AM, Chanwoo Choi wrote:
>>>> Hi Lukasz,
>>>>
>>>> On 7/9/20 12:34 AM, 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>
>>>>> 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;
>>>>>        }
>>>>>
>>>>
>>>> Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
>>>>
>>>
>>> Thank you for the review. Do you think this patch could go through
>>> your tree together with your patches?
>>>
>>> I don't know Krzysztof's opinion about the patch 2/2, but
>>> I would expect, assuming the patch itself is correct, he would
>>> like to take it into his next/dt branch.
>>
>> Is there really a need to remove the interrupts property? imho they are
>> correct hw description, it just a matter of the driver to use or not to
>> use them.

Marek, I agree with you, they are correct hw description. Unfortunately,
I don't have TRM to experiment and maybe fix the interrupt mode code.

> 
> That's actually very good point. I would also prefer to leave them.
> However I understood that driver chooses mode depending on the property.

Correct

> 
> In such case, maybe as you said, let's switch to polling mode
> unconditionally?

I can make happen that the polling mode will be unconditionally
set as default.

Do you think that the interrupt mode code can still stay in the
driver, because maybe in future could be fixed?


> 
> Best regards,
> Krzysztof
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] memory: samsung: exynos5422-dmc: Adjust polling interval and uptreshold
  2020-07-10 12:56         ` Lukasz Luba
  2020-07-10 13:07           ` Krzysztof Kozlowski
@ 2020-07-10 13:49           ` Bartlomiej Zolnierkiewicz
  2020-07-10 14:00             ` Lukasz Luba
  1 sibling, 1 reply; 16+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2020-07-10 13:49 UTC (permalink / raw)
  To: Lukasz Luba
  Cc: Krzysztof Kozlowski, Chanwoo Choi, linux-kernel, linux-pm,
	linux-arm-kernel, linux-samsung-soc, willy.mh.wolff.ml,
	k.konieczny, chanwoo, myungjoo.ham, kyungmin.park, s.nawrocki,
	kgene


On 7/10/20 2:56 PM, Lukasz Luba wrote:
> 
> 
> On 7/10/20 1:45 PM, Krzysztof Kozlowski wrote:
>> On Fri, Jul 10, 2020 at 09:34:45AM +0100, Lukasz Luba wrote:
>>> Hi Chanwoo,
>>>
>>> On 7/9/20 5:08 AM, Chanwoo Choi wrote:
>>>> Hi Lukasz,
>>>>
>>>> On 7/9/20 12:34 AM, 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>
>>>>> 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;
>>>>>        }
>>>>>
>>>>
>>>> Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
>>>>
>>>
>>> Thank you for the review. Do you think this patch could go through
>>> your tree together with your patches?
>>>
>>> I don't know Krzysztof's opinion about the patch 2/2, but
>>> I would expect, assuming the patch itself is correct, he would
>>> like to take it into his next/dt branch.
>>
>> In the cover letter you mentioned that this is a follow up for the
>> Chanwoo's patchset. But are these patches really depending on it? Can
>> they be picked up independently?
> 
> 
> They are not heavily dependent on Chanwoo's patches.
> Yes, they can be picked up independently.

Hmmm, are you sure?

Sure, they will apply fine but without Chanwoo's patches won't they
cause the dmc driver to use using polling mode with deferred timer
(unintended/bad behavior) instead of IRQs (current behavior) or
polling mode with delayed timer (future behavior)?

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

> I just wanted to mention that the patch 1/2 was produced on the
> code base which had already applied Chanwoo's patch for DMC.
> If you like to take both 1/2 and 2/2 into your tree, it's good.
> 
> Thank you for having a look on this.
> 
> Regards,
> Lukasz
> 
> 
>>
>> The DTS patch must go through arm soc, so I will take it. If it really
>> depends on driver changes, then it has to wait for next release.
>>
>> Best regards,
>> Krzysztof
>>


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] memory: samsung: exynos5422-dmc: Adjust polling interval and uptreshold
  2020-07-10 13:49           ` Bartlomiej Zolnierkiewicz
@ 2020-07-10 14:00             ` Lukasz Luba
  2020-07-10 14:47               ` Krzysztof Kozlowski
  0 siblings, 1 reply; 16+ messages in thread
From: Lukasz Luba @ 2020-07-10 14:00 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz
  Cc: Krzysztof Kozlowski, Chanwoo Choi, linux-kernel, linux-pm,
	linux-arm-kernel, linux-samsung-soc, willy.mh.wolff.ml,
	k.konieczny, chanwoo, myungjoo.ham, kyungmin.park, s.nawrocki,
	kgene



On 7/10/20 2:49 PM, Bartlomiej Zolnierkiewicz wrote:
> 
> On 7/10/20 2:56 PM, Lukasz Luba wrote:
>>
>>
>> On 7/10/20 1:45 PM, Krzysztof Kozlowski wrote:
>>> On Fri, Jul 10, 2020 at 09:34:45AM +0100, Lukasz Luba wrote:
>>>> Hi Chanwoo,
>>>>
>>>> On 7/9/20 5:08 AM, Chanwoo Choi wrote:
>>>>> Hi Lukasz,
>>>>>
>>>>> On 7/9/20 12:34 AM, 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>
>>>>>> 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;
>>>>>>         }
>>>>>>
>>>>>
>>>>> Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
>>>>>
>>>>
>>>> Thank you for the review. Do you think this patch could go through
>>>> your tree together with your patches?
>>>>
>>>> I don't know Krzysztof's opinion about the patch 2/2, but
>>>> I would expect, assuming the patch itself is correct, he would
>>>> like to take it into his next/dt branch.
>>>
>>> In the cover letter you mentioned that this is a follow up for the
>>> Chanwoo's patchset. But are these patches really depending on it? Can
>>> they be picked up independently?
>>
>>
>> They are not heavily dependent on Chanwoo's patches.
>> Yes, they can be picked up independently.
> 
> Hmmm, are you sure?

In a sense: in two phases (first the Chanwoo's changes land into
devfreq, then when Krzysztof prepares his topic branches for
arm soc, I assumed Chanwoo's patches are mainline and will be there
already).

> 
> Sure, they will apply fine but without Chanwoo's patches won't they
> cause the dmc driver to use using polling mode with deferred timer
> (unintended/bad behavior) instead of IRQs (current behavior) or
> polling mode with delayed timer (future behavior)?

I was assuming that it will take longer, when Krzysztof is going to pick
patch 2/2, definitely after a while (and it could be also the case for
patch 1/1 if Krzysztof was going to take it).

I think there is no rush and it can go in two phases.

Good point Bartek for clarifying this. I wasn't clear in the messages.
Thank you for keeping eye on this.

Regards,
Lukasz


> 
> Best regards,
> --
> Bartlomiej Zolnierkiewicz
> Samsung R&D Institute Poland
> Samsung Electronics
> 
>> I just wanted to mention that the patch 1/2 was produced on the
>> code base which had already applied Chanwoo's patch for DMC.
>> If you like to take both 1/2 and 2/2 into your tree, it's good.
>>
>> Thank you for having a look on this.
>>
>> Regards,
>> Lukasz
>>
>>
>>>
>>> The DTS patch must go through arm soc, so I will take it. If it really
>>> depends on driver changes, then it has to wait for next release.
>>>
>>> Best regards,
>>> Krzysztof
>>>
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] memory: samsung: exynos5422-dmc: Adjust polling interval and uptreshold
  2020-07-10 14:00             ` Lukasz Luba
@ 2020-07-10 14:47               ` Krzysztof Kozlowski
  0 siblings, 0 replies; 16+ messages in thread
From: Krzysztof Kozlowski @ 2020-07-10 14:47 UTC (permalink / raw)
  To: Lukasz Luba
  Cc: Bartlomiej Zolnierkiewicz, Chanwoo Choi, linux-kernel, linux-pm,
	linux-arm-kernel, linux-samsung-soc, willy.mh.wolff.ml,
	k.konieczny, chanwoo, myungjoo.ham, kyungmin.park, s.nawrocki,
	kgene

On Fri, Jul 10, 2020 at 03:00:37PM +0100, Lukasz Luba wrote:
> 
> 
> On 7/10/20 2:49 PM, Bartlomiej Zolnierkiewicz wrote:
> > 
> > On 7/10/20 2:56 PM, Lukasz Luba wrote:
> > > 
> > > 
> > > On 7/10/20 1:45 PM, Krzysztof Kozlowski wrote:
> > > > On Fri, Jul 10, 2020 at 09:34:45AM +0100, Lukasz Luba wrote:
> > > > > Hi Chanwoo,
> > > > > 
> > > > > On 7/9/20 5:08 AM, Chanwoo Choi wrote:
> > > > > > Hi Lukasz,
> > > > > > 
> > > > > > On 7/9/20 12:34 AM, 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>
> > > > > > > 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;
> > > > > > >         }
> > > > > > > 
> > > > > > 
> > > > > > Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
> > > > > > 
> > > > > 
> > > > > Thank you for the review. Do you think this patch could go through
> > > > > your tree together with your patches?
> > > > > 
> > > > > I don't know Krzysztof's opinion about the patch 2/2, but
> > > > > I would expect, assuming the patch itself is correct, he would
> > > > > like to take it into his next/dt branch.
> > > > 
> > > > In the cover letter you mentioned that this is a follow up for the
> > > > Chanwoo's patchset. But are these patches really depending on it? Can
> > > > they be picked up independently?
> > > 
> > > 
> > > They are not heavily dependent on Chanwoo's patches.
> > > Yes, they can be picked up independently.
> > 
> > Hmmm, are you sure?
> 
> In a sense: in two phases (first the Chanwoo's changes land into
> devfreq, then when Krzysztof prepares his topic branches for
> arm soc, I assumed Chanwoo's patches are mainline and will be there
> already).
> 
> > 
> > Sure, they will apply fine but without Chanwoo's patches won't they
> > cause the dmc driver to use using polling mode with deferred timer
> > (unintended/bad behavior) instead of IRQs (current behavior) or
> > polling mode with delayed timer (future behavior)?
> 
> I was assuming that it will take longer, when Krzysztof is going to pick
> patch 2/2, definitely after a while (and it could be also the case for
> patch 1/1 if Krzysztof was going to take it).
> 
> I think there is no rush and it can go in two phases.
> 
> Good point Bartek for clarifying this. I wasn't clear in the messages.
> Thank you for keeping eye on this.

... which means they are dependent on Chanwoo's patchset. They cannot go
in parallel, they cannot be picked up independently.

You know, when someone's patches are in mainline, it is never a
dependency anymore...

The patches then could wait for next cycle if there is no rush. Maybe
there will be some further revisions of this approach?

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] memory: samsung: exynos5422-dmc: Adjust polling interval and uptreshold
  2020-07-10 13:41           ` Lukasz Luba
@ 2020-07-10 14:49             ` Krzysztof Kozlowski
  2020-07-10 15:45               ` Lukasz Luba
  0 siblings, 1 reply; 16+ messages in thread
From: Krzysztof Kozlowski @ 2020-07-10 14:49 UTC (permalink / raw)
  To: Lukasz Luba
  Cc: Marek Szyprowski, Chanwoo Choi, linux-kernel, linux-pm,
	linux-arm-kernel, linux-samsung-soc, willy.mh.wolff.ml,
	k.konieczny, b.zolnierkie, chanwoo, myungjoo.ham, kyungmin.park,
	s.nawrocki, kgene

On Fri, Jul 10, 2020 at 02:41:28PM +0100, Lukasz Luba wrote:
> 
> 
> On 7/10/20 2:19 PM, Krzysztof Kozlowski wrote:
> > On Fri, Jul 10, 2020 at 03:13:18PM +0200, Marek Szyprowski wrote:
 > In such case, maybe as you said, let's switch to polling mode
> > unconditionally?
> 
> I can make happen that the polling mode will be unconditionally
> set as default.
> 
> Do you think that the interrupt mode code can still stay in the
> driver, because maybe in future could be fixed?

How interrupt mode would exist in such case? Or rather: how would it be
used? There is no point to keep dead code and code once removed, can be
easily brought back.

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] memory: samsung: exynos5422-dmc: Adjust polling interval and uptreshold
  2020-07-10 14:49             ` Krzysztof Kozlowski
@ 2020-07-10 15:45               ` Lukasz Luba
  0 siblings, 0 replies; 16+ messages in thread
From: Lukasz Luba @ 2020-07-10 15:45 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Marek Szyprowski, Chanwoo Choi, linux-kernel, linux-pm,
	linux-arm-kernel, linux-samsung-soc, willy.mh.wolff.ml,
	k.konieczny, b.zolnierkie, chanwoo, myungjoo.ham, kyungmin.park,
	s.nawrocki, kgene



On 7/10/20 3:49 PM, Krzysztof Kozlowski wrote:
> On Fri, Jul 10, 2020 at 02:41:28PM +0100, Lukasz Luba wrote:
>>
>>
>> On 7/10/20 2:19 PM, Krzysztof Kozlowski wrote:
>>> On Fri, Jul 10, 2020 at 03:13:18PM +0200, Marek Szyprowski wrote:
>   > In such case, maybe as you said, let's switch to polling mode
>>> unconditionally?
>>
>> I can make happen that the polling mode will be unconditionally
>> set as default.
>>
>> Do you think that the interrupt mode code can still stay in the
>> driver, because maybe in future could be fixed?
> 
> How interrupt mode would exist in such case? Or rather: how would it be
> used? There is no point to keep dead code and code once removed, can be
> easily brought back.

I can make a module param i.e. irq-mode=1, while in default where
the user don't provide param, we use polling mode. Then I don't have to
remove DT interrupts and the related code from the driver.

Regards,
Lukasz

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, back to index

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-08 15:34 [PATCH 0/2] Exynos5422 DMC: adjust to new devfreq monitoring mechanism Lukasz Luba
2020-07-08 15:34 ` [PATCH 1/2] memory: samsung: exynos5422-dmc: Adjust polling interval and uptreshold Lukasz Luba
2020-07-09  4:08   ` Chanwoo Choi
2020-07-10  8:34     ` Lukasz Luba
2020-07-10 12:45       ` Krzysztof Kozlowski
2020-07-10 12:56         ` Lukasz Luba
2020-07-10 13:07           ` Krzysztof Kozlowski
2020-07-10 13:49           ` Bartlomiej Zolnierkiewicz
2020-07-10 14:00             ` Lukasz Luba
2020-07-10 14:47               ` Krzysztof Kozlowski
2020-07-10 13:13       ` Marek Szyprowski
2020-07-10 13:19         ` Krzysztof Kozlowski
2020-07-10 13:41           ` Lukasz Luba
2020-07-10 14:49             ` Krzysztof Kozlowski
2020-07-10 15:45               ` Lukasz Luba
2020-07-08 15:34 ` [PATCH 2/2] ARM: dts: exynos: Remove interrupts from DMC controller in Exynos5422 Lukasz Luba

Linux-Samsung-soc Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-samsung-soc/0 linux-samsung-soc/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-samsung-soc linux-samsung-soc/ https://lore.kernel.org/linux-samsung-soc \
		linux-samsung-soc@vger.kernel.org
	public-inbox-index linux-samsung-soc

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-samsung-soc


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git