* [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; 22+ 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] 22+ 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; 22+ 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 related [flat|nested] 22+ 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; 22+ 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] 22+ 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; 22+ 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] 22+ 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; 22+ 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] 22+ 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; 22+ 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] 22+ 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; 22+ 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] 22+ 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; 22+ 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] 22+ 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; 22+ 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] 22+ 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; 22+ 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] 22+ 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; 22+ 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] 22+ 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; 22+ 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] 22+ 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; 22+ 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] 22+ 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; 22+ 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] 22+ 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; 22+ 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] 22+ 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 2020-08-17 15:50 ` Krzysztof Kozlowski 2020-10-26 19:57 ` Krzysztof Kozlowski 1 sibling, 2 replies; 22+ 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 related [flat|nested] 22+ messages in thread
* Re: [PATCH 2/2] ARM: dts: exynos: Remove interrupts from DMC controller in Exynos5422 2020-07-08 15:34 ` [PATCH 2/2] ARM: dts: exynos: Remove interrupts from DMC controller in Exynos5422 Lukasz Luba @ 2020-08-17 15:50 ` Krzysztof Kozlowski 2020-08-17 17:17 ` Lukasz Luba 2020-10-26 19:57 ` Krzysztof Kozlowski 1 sibling, 1 reply; 22+ messages in thread From: Krzysztof Kozlowski @ 2020-08-17 15:50 UTC (permalink / raw) To: Lukasz Luba Cc: linux-kernel, linux-pm, linux-arm-kernel, linux-samsung-soc, willy.mh.wolff.ml, k.konieczny, cw00.choi, b.zolnierkie, chanwoo, myungjoo.ham, kyungmin.park, s.nawrocki, kgene On Wed, Jul 08, 2020 at 04:34:20PM +0100, Lukasz Luba wrote: > 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(-) I think the dependencies were merged so this can be safely applied without bisectability problems? Best regards, Krzysztof ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/2] ARM: dts: exynos: Remove interrupts from DMC controller in Exynos5422 2020-08-17 15:50 ` Krzysztof Kozlowski @ 2020-08-17 17:17 ` Lukasz Luba 2020-08-21 6:31 ` Krzysztof Kozlowski 0 siblings, 1 reply; 22+ messages in thread From: Lukasz Luba @ 2020-08-17 17:17 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: linux-kernel, linux-pm, linux-arm-kernel, linux-samsung-soc, willy.mh.wolff.ml, k.konieczny, cw00.choi, b.zolnierkie, chanwoo, myungjoo.ham, kyungmin.park, s.nawrocki, kgene Hi Krzysztof, On 8/17/20 4:50 PM, Krzysztof Kozlowski wrote: > On Wed, Jul 08, 2020 at 04:34:20PM +0100, Lukasz Luba wrote: >> 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(-) > > I think the dependencies were merged so this can be safely applied > without bisectability problems? I have created v2 of that fix and it got merged via Chanwoo's tree, the commit 4fc9a0470d2dc37028 https://lkml.org/lkml/2020/7/10/1048 That commit switched the driver default mode from 'irq driven' to new devfreq monitoring mechanism. Furthermore, when the driver is built as a module, you can try to use the 'irq mode', but for this you would need the DT IRQs description (this $subject tries to remove). I would like to keep this IRQ mode for experimentation, as I described in answers to Bartek's questions: https://lkml.org/lkml/2020/7/14/315 Unfortunately, I am quite busy and won't make any progress before the LPC. Regards, Lukasz ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/2] ARM: dts: exynos: Remove interrupts from DMC controller in Exynos5422 2020-08-17 17:17 ` Lukasz Luba @ 2020-08-21 6:31 ` Krzysztof Kozlowski 2020-08-21 6:32 ` Krzysztof Kozlowski 0 siblings, 1 reply; 22+ messages in thread From: Krzysztof Kozlowski @ 2020-08-21 6:31 UTC (permalink / raw) To: Lukasz Luba Cc: linux-kernel, linux-pm, linux-arm-kernel, linux-samsung-soc, willy.mh.wolff.ml, k.konieczny, Chanwoo Choi, Bartłomiej Żołnierkiewicz, chanwoo, myungjoo.ham, kyungmin.park, s.nawrocki, kgene On Mon, 17 Aug 2020 at 19:17, Lukasz Luba <lukasz.luba@arm.com> wrote: > > Hi Krzysztof, > > On 8/17/20 4:50 PM, Krzysztof Kozlowski wrote: > > On Wed, Jul 08, 2020 at 04:34:20PM +0100, Lukasz Luba wrote: > >> 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(-) > > > > I think the dependencies were merged so this can be safely applied > > without bisectability problems? > > I have created v2 of that fix and it got merged > via Chanwoo's tree, the commit 4fc9a0470d2dc37028 > https://lkml.org/lkml/2020/7/10/1048 > > That commit switched the driver default mode from 'irq driven' to > new devfreq monitoring mechanism. Furthermore, when the driver is > built as a module, you can try to use the 'irq mode', but for this > you would need the DT IRQs description (this $subject tries to remove). > > I would like to keep this IRQ mode for experimentation, as I > described in answers to Bartek's questions: > https://lkml.org/lkml/2020/7/14/315 > > Unfortunately, I am quite busy and won't make any progress before the > LPC. None of these were the actual answer to my question, unless by "v2 of that fix and it got merged" means the dependencies? I'll drop it from the queue. Please resend if it is valid. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/2] ARM: dts: exynos: Remove interrupts from DMC controller in Exynos5422 2020-08-21 6:31 ` Krzysztof Kozlowski @ 2020-08-21 6:32 ` Krzysztof Kozlowski 2020-08-24 11:34 ` Lukasz Luba 0 siblings, 1 reply; 22+ messages in thread From: Krzysztof Kozlowski @ 2020-08-21 6:32 UTC (permalink / raw) To: Lukasz Luba Cc: linux-kernel, linux-pm, linux-arm-kernel, linux-samsung-soc, willy.mh.wolff.ml, k.konieczny, Chanwoo Choi, Bartłomiej Żołnierkiewicz, chanwoo, myungjoo.ham, kyungmin.park, s.nawrocki, kgene On Fri, 21 Aug 2020 at 08:31, Krzysztof Kozlowski <krzk@kernel.org> wrote: > > On Mon, 17 Aug 2020 at 19:17, Lukasz Luba <lukasz.luba@arm.com> wrote: > > > > Hi Krzysztof, > > > > On 8/17/20 4:50 PM, Krzysztof Kozlowski wrote: > > > On Wed, Jul 08, 2020 at 04:34:20PM +0100, Lukasz Luba wrote: > > >> 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(-) > > > > > > I think the dependencies were merged so this can be safely applied > > > without bisectability problems? > > > > I have created v2 of that fix and it got merged > > via Chanwoo's tree, the commit 4fc9a0470d2dc37028 > > https://lkml.org/lkml/2020/7/10/1048 > > > > That commit switched the driver default mode from 'irq driven' to > > new devfreq monitoring mechanism. Furthermore, when the driver is > > built as a module, you can try to use the 'irq mode', but for this > > you would need the DT IRQs description (this $subject tries to remove). > > > > I would like to keep this IRQ mode for experimentation, as I > > described in answers to Bartek's questions: > > https://lkml.org/lkml/2020/7/14/315 > > > > Unfortunately, I am quite busy and won't make any progress before the > > LPC. > > None of these were the actual answer to my question, unless by "v2 of > that fix and it got merged" means the dependencies? > > I'll drop it from the queue. Please resend if it is valid. Agh, not drop, I mean postpone for the next cycle (this is v5.11). Best regards, Krzysztof ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/2] ARM: dts: exynos: Remove interrupts from DMC controller in Exynos5422 2020-08-21 6:32 ` Krzysztof Kozlowski @ 2020-08-24 11:34 ` Lukasz Luba 0 siblings, 0 replies; 22+ messages in thread From: Lukasz Luba @ 2020-08-24 11:34 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: linux-kernel, linux-pm, linux-arm-kernel, linux-samsung-soc, willy.mh.wolff.ml, k.konieczny, Chanwoo Choi, Bartłomiej Żołnierkiewicz, chanwoo, myungjoo.ham, kyungmin.park, s.nawrocki, kgene On 8/21/20 7:32 AM, Krzysztof Kozlowski wrote: > On Fri, 21 Aug 2020 at 08:31, Krzysztof Kozlowski <krzk@kernel.org> wrote: >> >> On Mon, 17 Aug 2020 at 19:17, Lukasz Luba <lukasz.luba@arm.com> wrote: >>> >>> Hi Krzysztof, >>> >>> On 8/17/20 4:50 PM, Krzysztof Kozlowski wrote: >>>> On Wed, Jul 08, 2020 at 04:34:20PM +0100, Lukasz Luba wrote: >>>>> 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(-) >>>> >>>> I think the dependencies were merged so this can be safely applied >>>> without bisectability problems? >>> >>> I have created v2 of that fix and it got merged >>> via Chanwoo's tree, the commit 4fc9a0470d2dc37028 >>> https://lkml.org/lkml/2020/7/10/1048 >>> >>> That commit switched the driver default mode from 'irq driven' to >>> new devfreq monitoring mechanism. Furthermore, when the driver is >>> built as a module, you can try to use the 'irq mode', but for this >>> you would need the DT IRQs description (this $subject tries to remove). >>> >>> I would like to keep this IRQ mode for experimentation, as I >>> described in answers to Bartek's questions: >>> https://lkml.org/lkml/2020/7/14/315 >>> >>> Unfortunately, I am quite busy and won't make any progress before the >>> LPC. >> >> None of these were the actual answer to my question, unless by "v2 of >> that fix and it got merged" means the dependencies? >> >> I'll drop it from the queue. Please resend if it is valid. > > Agh, not drop, I mean postpone for the next cycle (this is v5.11). > Fair enough Regards, Lukasz ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/2] ARM: dts: exynos: Remove interrupts from DMC controller in Exynos5422 2020-07-08 15:34 ` [PATCH 2/2] ARM: dts: exynos: Remove interrupts from DMC controller in Exynos5422 Lukasz Luba 2020-08-17 15:50 ` Krzysztof Kozlowski @ 2020-10-26 19:57 ` Krzysztof Kozlowski 1 sibling, 0 replies; 22+ messages in thread From: Krzysztof Kozlowski @ 2020-10-26 19:57 UTC (permalink / raw) To: Lukasz Luba Cc: linux-kernel, linux-pm, linux-arm-kernel, linux-samsung-soc, willy.mh.wolff.ml, k.konieczny, cw00.choi, b.zolnierkie, chanwoo, myungjoo.ham, kyungmin.park, s.nawrocki, kgene On Wed, Jul 08, 2020 at 04:34:20PM +0100, Lukasz Luba wrote: > 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 --- Thanks, applied. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2020-10-26 19:57 UTC | newest] Thread overview: 22+ 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 2020-08-17 15:50 ` Krzysztof Kozlowski 2020-08-17 17:17 ` Lukasz Luba 2020-08-21 6:31 ` Krzysztof Kozlowski 2020-08-21 6:32 ` Krzysztof Kozlowski 2020-08-24 11:34 ` Lukasz Luba 2020-10-26 19:57 ` Krzysztof Kozlowski
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).