From: Lukasz Luba <l.luba@partner.samsung.com>
To: myungjoo.ham@samsung.com,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"linux-pm@vger.kernel.org" <linux-pm@vger.kernel.org>
Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>,
Chanwoo Choi <cw00.choi@samsung.com>,
Kyungmin Park <kyungmin.park@samsung.com>,
Marek Szyprowski <m.szyprowski@samsung.com>,
Sylwester Nawrocki <s.nawrocki@samsung.com>,
"tkjos@google.com" <tkjos@google.com>,
"joel@joelfernandes.org" <joel@joelfernandes.org>,
"chris.diamand@arm.com" <chris.diamand@arm.com>,
"mka@chromium.org" <mka@chromium.org>,
"rostedt@goodmis.org" <rostedt@goodmis.org>,
"mingo@redhat.com" <mingo@redhat.com>
Subject: Re: [PATCH v3 5/7] drivers: devfreq: add longer polling interval in idle
Date: Tue, 19 Feb 2019 09:33:46 +0100 [thread overview]
Message-ID: <e5e9c6f0-5a1e-8ba1-556a-74edb1782980@partner.samsung.com> (raw)
In-Reply-To: <20190218043311epcms1p33ccb2c312e871294025171d97bb87ac6@epcms1p3>
Hi MyungJoo,
Thank you for taking part in the discussion.
Please check my comments below.
On 2/18/19 5:33 AM, MyungJoo Ham wrote:
>> This patch adds new mechanism for devfreq devices which changes polling
>> interval. The system should sleep longer when the devfreq device is almost
>> not used. The devfreq framework will not schedule the work too often.
>> This low-load state is recognised when the device is operating at the lowest
>> frequency and has low busy_time/total_time factor (< 30%).
>> When the frequency is different then min, the device is under normal polling
>> which is the value defined in driver's 'polling_ms'.
>> When the device is getting more pressure, the framework is able to catch it
>> based on 'load' in lowest frequency and will start polling more frequently.
>> The second scenario is when the governor recognised heavy load at minimum
>> frequency and increases the frequency. The devfreq framework will start
>> polling in shorter intervals.
>> The polling interval, when the device is not heavily, can also be changed
>> from userspace of defined by the driver's author.
>>
>> Signed-off-by: Lukasz Luba <l.luba@partner.samsung.com>
>> ---
>> drivers/devfreq/devfreq.c | 151 +++++++++++++++++++++++++++---
>> drivers/devfreq/governor.h | 3 +-
>> drivers/devfreq/governor_simpleondemand.c | 6 +-
>> 3 files changed, 145 insertions(+), 15 deletions(-)
>>
>
> There are some requirements that you need to consider:
>
> Is 30% really applicable to ALL devfreq devices?
The 30% load while the device is on lowest OPP is to filter some noise.
It might be tunable over sysfs for each device if you like.
> - What if some devices do not want such behaviors?
They can set polling_idle_ms and polling_ms the same value.
> - What if some devices want different values (change behavors)?
Need of sysfs tunable here.
> - What if some manufactures want different default values?
Like above (sysfs).
> - What if some devices want to let the framework know that it's in idle?
There might be a filed in devfreq->state which could handle this.
> - What if some other kernel context, device (drivers),
> or userspace process want to notify that it's no more idling?This issue is more related to the new movement in the 'interconnect'
development. They have a goal for this kind of interactions and QoS
between devices or their clients. In devfreq it would be possible
to tackle this, but would require a lot of changes (notification chain,
state machines in devices,
>
> As mentioned in the internal thread (tizen.org),
> I'm not convinced by the idea of assuming that a device can be considered "idling"
> if it has simply "low" utilization.
>
> You are going to deteriorate the UI response time of mobile devices significantly.
Current devfreq wake-up also does not guarantee that, maybe on a single
CPU platform does.
I will try to address your and Chanwoo's comments that the devfreq still
needs deferred polling in some platforms.
Would it be OK if we have two options: deferred and delayed work while
registering a wakeup for a device?
That would be a function like: polling_mode_init(devfreq) instead of
simple INIT_DEFERRED_WORK(), which will check the device's preference.
The device driver could set a filed in 'polling_mode' to enum:
POWER_EFFICIENT or RELIABLE_INTERVAL. For compatibility with old drivers
where the polling_mode = 0, SYSTEM_DEFAULT_POLLING_MODE (which is one
of these two) would be used.
Then the two-phase-polling-interval from this patch could only be used
for the RELIABLE_INTERVAL configuration or even dropped.
Regards,
Lukasz
>
> Cheers,
> MyungJoo.
>
>
next prev parent reply other threads:[~2019-02-19 8:33 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <CGME20190212222422eucas1p1624203db4db3e495035820dea542e23a@eucas1p1.samsung.com>
2019-02-12 22:23 ` [PATCH v3 0/7] drivers: devfreq: fix and optimize workqueue mechanism Lukasz Luba
[not found] ` <CGME20190212222430eucas1p1ad7992e29d224790c1e20ef7442e62fe@eucas1p1.samsung.com>
2019-02-12 22:23 ` [PATCH v3 1/7] drivers: devfreq: change deferred work into delayed Lukasz Luba
2019-02-14 4:10 ` Chanwoo Choi
[not found] ` <CGME20190212222431eucas1p1697607e6536a90283cf7dad37fa74dbb@eucas1p1.samsung.com>
2019-02-12 22:23 ` [PATCH v3 2/7] drivers: devfreq: change devfreq workqueue mechanism Lukasz Luba
2019-02-14 4:11 ` Chanwoo Choi
[not found] ` <CGME20190212222433eucas1p264602d67a916c644c7eb5012932fc17a@eucas1p2.samsung.com>
2019-02-12 22:23 ` [PATCH v3 3/7] Kconfig: drivers: devfreq: add default idle polling Lukasz Luba
[not found] ` <CGME20190212222434eucas1p134dcdce827df19704c698fd6452b0a06@eucas1p1.samsung.com>
2019-02-12 22:23 ` [PATCH v3 4/7] include: devfreq: add polling_idle_ms to 'profile' Lukasz Luba
2019-02-14 4:51 ` Chanwoo Choi
[not found] ` <CGME20190212222436eucas1p21eebc80796406787a2ebf9a84ee5b868@eucas1p2.samsung.com>
2019-02-12 22:23 ` [PATCH v3 5/7] drivers: devfreq: add longer polling interval in idle Lukasz Luba
[not found] ` <CGME20190212222437eucas1p198db6fca1f1ba3056d93c57327dd48ed@eucas1p1.samsung.com>
2019-02-12 22:23 ` [PATCH v3 6/7] trace: events: add devfreq trace event file Lukasz Luba
2019-02-12 23:14 ` Steven Rostedt
2019-02-13 13:35 ` Lukasz Luba
2019-02-14 5:01 ` Chanwoo Choi
2019-02-13 13:56 ` Steven Rostedt
2019-02-13 14:37 ` Lukasz Luba
[not found] ` <CGME20190212222438eucas1p27e020c2b36f2e5a2188e4df6fb18488b@eucas1p2.samsung.com>
2019-02-12 22:23 ` [PATCH v3 7/7] drivers: devfreq: add tracing for scheduling work Lukasz Luba
2019-02-14 4:57 ` Chanwoo Choi
2019-02-13 0:18 ` [PATCH v3 0/7] drivers: devfreq: fix and optimize workqueue mechanism Chanwoo Choi
2019-02-13 11:14 ` Lukasz Luba
2019-02-13 14:52 ` Lukasz Luba
2019-02-14 0:41 ` Chanwoo Choi
[not found] ` <CGME20190212222436eucas1p21eebc80796406787a2ebf9a84ee5b868@epcms1p3>
2019-02-18 4:33 ` [PATCH v3 5/7] drivers: devfreq: add longer polling interval in idle MyungJoo Ham
2019-02-19 8:33 ` Lukasz Luba [this message]
[not found] ` <CGME20190212222436eucas1p21eebc80796406787a2ebf9a84ee5b868@epcms1p7>
2019-02-21 5:56 ` MyungJoo Ham
2019-02-22 16:03 ` Lukasz Luba
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=e5e9c6f0-5a1e-8ba1-556a-74edb1782980@partner.samsung.com \
--to=l.luba@partner.samsung.com \
--cc=b.zolnierkie@samsung.com \
--cc=chris.diamand@arm.com \
--cc=cw00.choi@samsung.com \
--cc=joel@joelfernandes.org \
--cc=kyungmin.park@samsung.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=m.szyprowski@samsung.com \
--cc=mingo@redhat.com \
--cc=mka@chromium.org \
--cc=myungjoo.ham@samsung.com \
--cc=rostedt@goodmis.org \
--cc=s.nawrocki@samsung.com \
--cc=tkjos@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).