On 19. 7. 19. 오전 10:15, Chanwoo Choi wrote: > On 19. 7. 19. 오전 9:40, Dmitry Osipenko wrote: >> В Thu, 18 Jul 2019 18:51:02 +0900 >> Chanwoo Choi <cw00.choi@xxxxxxxxxxx> пишет: >> >>> On 19. 7. 8. 오전 7:32, Dmitry Osipenko wrote: >>>> The memory activity counter may get a bit higher than a watermark >>>> which is selected based on OPP that corresponds to a highest EMC >>>> rate, in this case watermark is lower than the actual memory >>>> activity is and thus results in unwanted "upper" interrupts. >>>> >>>> Signed-off-by: Dmitry Osipenko <digetx@xxxxxxxxx> >>>> --- >>>> drivers/devfreq/tegra30-devfreq.c | 13 ++++++++++++- >>>> 1 file changed, 12 insertions(+), 1 deletion(-) >>> >>> It seems that you can combine patch19 with patch20. >> >> No, consecutive and average watermarks are different things that have >> different purposes. Consecutive are used for boosting, while average >> are for significant memory bandwidth changes. >> >>>> >>>> diff --git a/drivers/devfreq/tegra30-devfreq.c >>>> b/drivers/devfreq/tegra30-devfreq.c index >>>> 8d6bf6e9f1ae..c3cf87231d25 100644 --- >>>> a/drivers/devfreq/tegra30-devfreq.c +++ >>>> b/drivers/devfreq/tegra30-devfreq.c @@ -363,7 +363,18 @@ static >>>> void tegra_devfreq_update_wmark(struct tegra_devfreq *tegra, >>>> tegra_actmon_get_lower_upper(tegra, dev, freq - 1, &lower, &upper); >>>> delta = do_percent(upper - lower, >>>> dev->config->boost_up_threshold); >>>> - device_writel(dev, lower + delta, ACTMON_DEV_UPPER_WMARK); > > This line was added on patch5 and then this line is removed on this patch. > It is wrong method to make the patch in the same patchset. > > It is enough to reduce the inefficient add/remove code in the same patchset. I'm missing some expression. So, I update my comment as following: It is possible to reduce the inefficient add/remove code in the same patchset if you combine the patches. > > Have to merge this patch to patch5. > >>>> + >>>> + /* >>>> + * The memory events count could go a bit higher than the >>>> maximum >>>> + * defined by the OPPs, hence make the upper watermark >>>> infinitely >>>> + * high to avoid unnecessary upper interrupts in that case. >>>> + */ >>>> + if (freq == tegra->max_freq) >>>> + upper = ULONG_MAX; >>>> + else >>>> + upper = lower + delta; >>>> + >>>> + device_writel(dev, upper, ACTMON_DEV_UPPER_WMARK); >>>> >>>> /* >>>> * Meanwhile the lower mark is based on the average value >>>> >>> >>> >> >> >> > > -- Best Regards, Chanwoo Choi Samsung Electronics
