* [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).