| Message ID | 1436456621-29839-3-git-send-email-b.zolnierkie@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 10.07.2015 00:43, Bartlomiej Zolnierkiewicz wrote: > Turbo modes should be marked with CPUFREQ_BOOST_FREQ flag in > the frequency table entry. > > Cc: Tomasz Figa <tomasz.figa@gmail.com> > Cc: Michael Turquette <mturquette@baylibre.com> > Cc: Javier Martinez Canillas <javier@dowhile0.org> > Cc: Thomas Abraham <thomas.ab@samsung.com> > Cc: Viresh Kumar <viresh.kumar@linaro.org> > Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> > --- > drivers/cpufreq/cpufreq_opp.c | 2 ++ > 1 file changed, 2 insertions(+) Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> Best regards, Krzysztof
$subject is a bit wrong, we aren't fixing any issue here. We are supporting a new feature and so it should be like: cpufreq: Mark boost frequencies based on OPP's turbo mode On 09-07-15, 17:43, Bartlomiej Zolnierkiewicz wrote: > Turbo modes should be marked with CPUFREQ_BOOST_FREQ flag in > the frequency table entry. > > Cc: Tomasz Figa <tomasz.figa@gmail.com> > Cc: Michael Turquette <mturquette@baylibre.com> > Cc: Javier Martinez Canillas <javier@dowhile0.org> > Cc: Thomas Abraham <thomas.ab@samsung.com> > Cc: Viresh Kumar <viresh.kumar@linaro.org> > Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> > --- > drivers/cpufreq/cpufreq_opp.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/cpufreq/cpufreq_opp.c b/drivers/cpufreq/cpufreq_opp.c > index 773bcde..f0cf502 100644 > --- a/drivers/cpufreq/cpufreq_opp.c > +++ b/drivers/cpufreq/cpufreq_opp.c > @@ -75,6 +75,8 @@ int dev_pm_opp_init_cpufreq_table(struct device *dev, > } > freq_table[i].driver_data = i; > freq_table[i].frequency = rate / 1000; > + if (dev_pm_opp_get_turbo_mode_setting(opp) == true) > + freq_table[i].flags |= CPUFREQ_BOOST_FREQ; > } > > freq_table[i].driver_data = i; Rest look fine.
Hi, On Monday, July 27, 2015 02:05:31 PM Viresh Kumar wrote: > $subject is a bit wrong, we aren't fixing any issue here. We are > supporting a new feature and so it should be like: Have you read my explanation of the issue? With your OPP-v2 patches cpufreq-dt picks turbo frequencies and uses them as normal ones. (More at: http://www.spinics.net/lists/arm-kernel/msg430397.html) Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics > cpufreq: Mark boost frequencies based on OPP's turbo mode > > On 09-07-15, 17:43, Bartlomiej Zolnierkiewicz wrote: > > Turbo modes should be marked with CPUFREQ_BOOST_FREQ flag in > > the frequency table entry. > > > > Cc: Tomasz Figa <tomasz.figa@gmail.com> > > Cc: Michael Turquette <mturquette@baylibre.com> > > Cc: Javier Martinez Canillas <javier@dowhile0.org> > > Cc: Thomas Abraham <thomas.ab@samsung.com> > > Cc: Viresh Kumar <viresh.kumar@linaro.org> > > Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> > > --- > > drivers/cpufreq/cpufreq_opp.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/drivers/cpufreq/cpufreq_opp.c b/drivers/cpufreq/cpufreq_opp.c > > index 773bcde..f0cf502 100644 > > --- a/drivers/cpufreq/cpufreq_opp.c > > +++ b/drivers/cpufreq/cpufreq_opp.c > > @@ -75,6 +75,8 @@ int dev_pm_opp_init_cpufreq_table(struct device *dev, > > } > > freq_table[i].driver_data = i; > > freq_table[i].frequency = rate / 1000; > > + if (dev_pm_opp_get_turbo_mode_setting(opp) == true) > > + freq_table[i].flags |= CPUFREQ_BOOST_FREQ; > > } > > > > freq_table[i].driver_data = i; > > Rest look fine.
On 27-07-15, 12:24, Bartlomiej Zolnierkiewicz wrote: > Have you read my explanation of the issue? > > With your OPP-v2 patches cpufreq-dt picks turbo frequencies and uses > them as normal ones. > > (More at: http://www.spinics.net/lists/arm-kernel/msg430397.html) Yes I did. I understand that the turbo frequencies are not considered as such by OPP/cpufreq core and it requires your changes to get it working. But its not an issue or bug we are fixing, the problem is that the code for opp-v2 isn't complete yet and your patches is putting things in place. So, we are still doing the bring up here and not fixing a bug really.
Hi, On Monday, July 27, 2015 04:05:21 PM Viresh Kumar wrote: > On 27-07-15, 12:24, Bartlomiej Zolnierkiewicz wrote: > > Have you read my explanation of the issue? > > > > With your OPP-v2 patches cpufreq-dt picks turbo frequencies and uses > > them as normal ones. > > > > (More at: http://www.spinics.net/lists/arm-kernel/msg430397.html) > > Yes I did. I understand that the turbo frequencies are not considered > as such by OPP/cpufreq core and it requires your changes to get it > working. Sorry but you don't seem to understand the issue. The problem is that without my patch and with your OPP-v2 patches turbo frequencies get picked by OPP/cpufreq core and then by cpufreq-dt. This happens without enabling any boost & thermal etc. support for turbo frequencies. > But its not an issue or bug we are fixing, the problem is that the > code for opp-v2 isn't complete yet and your patches is putting things > in place. So, we are still doing the bring up here and not fixing a > bug really. I consider the possibility to use turbo frequencies without explicitly enabling boost support to be a buggy behavior. While it is unlikely that somebody defines turbo frequencies in their dts file in the near future (except Exynos ones) it costs as nearly nothing to prevent such behavior now. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics
On 27-07-15, 13:14, Bartlomiej Zolnierkiewicz wrote:
> Sorry but you don't seem to understand the issue.
:)
No, I did. I understand that if someone uses opp bindings today with
some entries as turbo OPPs, cpufreq will use them as normal
frequencies. And that may harm the board.
BUT, opp-v2 code isn't ready to be used yet. And platforms should see
what all is implemented before trying to use them.
All I was saying is, this isn't a FIX as we haven't introduced the
feature yet. Otherwise I had no issues with the patch.
On Monday, July 27, 2015 05:06:41 PM Viresh Kumar wrote: > On 27-07-15, 13:14, Bartlomiej Zolnierkiewicz wrote: > > Sorry but you don't seem to understand the issue. > > :) > > No, I did. I understand that if someone uses opp bindings today with > some entries as turbo OPPs, cpufreq will use them as normal > frequencies. And that may harm the board. > > BUT, opp-v2 code isn't ready to be used yet. And platforms should see > what all is implemented before trying to use them. OK. > All I was saying is, this isn't a FIX as we haven't introduced the > feature yet. Otherwise I had no issues with the patch. I will update the description for the next patchset revision. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics
On 07/27/15 20:47, Bartlomiej Zolnierkiewicz wrote: > On Monday, July 27, 2015 05:06:41 PM Viresh Kumar wrote: >> On 27-07-15, 13:14, Bartlomiej Zolnierkiewicz wrote: >>> Sorry but you don't seem to understand the issue. >> >> :) >> >> No, I did. I understand that if someone uses opp bindings today with >> some entries as turbo OPPs, cpufreq will use them as normal >> frequencies. And that may harm the board. >> >> BUT, opp-v2 code isn't ready to be used yet. And platforms should see >> what all is implemented before trying to use them. > > OK. > >> All I was saying is, this isn't a FIX as we haven't introduced the >> feature yet. Otherwise I had no issues with the patch. > > I will update the description for the next patchset revision. > Hi Bart, When will you re-post v3? Because I have a plan to send a pull-request to arm-soc until this weekend... - Kukjin
On Thursday, July 30, 2015 11:37:27 PM Kukjin Kim wrote: > On 07/27/15 20:47, Bartlomiej Zolnierkiewicz wrote: > > On Monday, July 27, 2015 05:06:41 PM Viresh Kumar wrote: > >> On 27-07-15, 13:14, Bartlomiej Zolnierkiewicz wrote: > >>> Sorry but you don't seem to understand the issue. > >> > >> :) > >> > >> No, I did. I understand that if someone uses opp bindings today with > >> some entries as turbo OPPs, cpufreq will use them as normal > >> frequencies. And that may harm the board. > >> > >> BUT, opp-v2 code isn't ready to be used yet. And platforms should see > >> what all is implemented before trying to use them. > > > > OK. > > > >> All I was saying is, this isn't a FIX as we haven't introduced the > >> feature yet. Otherwise I had no issues with the patch. > > > > I will update the description for the next patchset revision. > > > Hi Bart, Hi, > When will you re-post v3? Because I have a plan to send a pull-request > to arm-soc until this weekend... I have just posted v3. I hope that it is not too late.. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics
On 30.07.2015 23:37, Kukjin Kim wrote: > On 07/27/15 20:47, Bartlomiej Zolnierkiewicz wrote: >> On Monday, July 27, 2015 05:06:41 PM Viresh Kumar wrote: >>> On 27-07-15, 13:14, Bartlomiej Zolnierkiewicz wrote: >>>> Sorry but you don't seem to understand the issue. >>> >>> :) >>> >>> No, I did. I understand that if someone uses opp bindings today with >>> some entries as turbo OPPs, cpufreq will use them as normal >>> frequencies. And that may harm the board. >>> >>> BUT, opp-v2 code isn't ready to be used yet. And platforms should see >>> what all is implemented before trying to use them. >> >> OK. >> >>> All I was saying is, this isn't a FIX as we haven't introduced the >>> feature yet. Otherwise I had no issues with the patch. >> >> I will update the description for the next patchset revision. >> > Hi Bart, > > When will you re-post v3? Because I have a plan to send a pull-request > to arm-soc until this weekend... Dear Kukjin, We are already at 4.2-rc5 and you did not send the pull request before the weekend as you said. It is really late and there is no special reason for delaying the request. What happened? Best regards, Krzysztof
diff --git a/drivers/cpufreq/cpufreq_opp.c b/drivers/cpufreq/cpufreq_opp.c index 773bcde..f0cf502 100644 --- a/drivers/cpufreq/cpufreq_opp.c +++ b/drivers/cpufreq/cpufreq_opp.c @@ -75,6 +75,8 @@ int dev_pm_opp_init_cpufreq_table(struct device *dev, } freq_table[i].driver_data = i; freq_table[i].frequency = rate / 1000; + if (dev_pm_opp_get_turbo_mode_setting(opp) == true) + freq_table[i].flags |= CPUFREQ_BOOST_FREQ; } freq_table[i].driver_data = i;
Turbo modes should be marked with CPUFREQ_BOOST_FREQ flag in the frequency table entry. Cc: Tomasz Figa <tomasz.figa@gmail.com> Cc: Michael Turquette <mturquette@baylibre.com> Cc: Javier Martinez Canillas <javier@dowhile0.org> Cc: Thomas Abraham <thomas.ab@samsung.com> Cc: Viresh Kumar <viresh.kumar@linaro.org> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> --- drivers/cpufreq/cpufreq_opp.c | 2 ++ 1 file changed, 2 insertions(+)