* Re: Alternatives to /sys/kernel/debug/wakeup_sources
From: Joel Fernandes @ 2019-06-19 20:09 UTC (permalink / raw)
To: Saravana Kannan
Cc: Greg Kroah-Hartman, Rafael J. Wysocki, Tri Vo, Rafael J. Wysocki,
Sandeep Patil, Viresh Kumar, Hridya Valsaraju, Linux PM,
Cc: Android Kernel, LKML, Alexei Starovoitov, Steven Rostedt,
Alexei Starovoitov
In-Reply-To: <CAGETcx-ZZRc_jtBws2cFTe1wjiWeBowdqfqOhcCJV_7AUyBEVw@mail.gmail.com>
On Wed, Jun 19, 2019 at 3:59 PM 'Saravana Kannan' via kernel-team
<kernel-team@android.com> wrote:
>
>
>
> On Wed, Jun 19, 2019, 11:55 AM 'Joel Fernandes' via kernel-team <kernel-team@android.com> wrote:
>>
>> On Wed, Jun 19, 2019 at 2:35 PM Greg Kroah-Hartman
>> <gregkh@linuxfoundation.org> wrote:
>> >
>> > On Wed, Jun 19, 2019 at 02:01:36PM -0400, Joel Fernandes wrote:
>> > > On Wed, Jun 19, 2019 at 1:07 PM Greg Kroah-Hartman
>> > > <gregkh@linuxfoundation.org> wrote:
>> > > >
>> > > > On Wed, Jun 19, 2019 at 12:53:12PM -0400, Joel Fernandes wrote:
>> > > > > > It is conceivable to have a "wakeup_sources" directory under
>> > > > > > /sys/power/ and sysfs nodes for all wakeup sources in there.
>> > > > >
>> > > > > One of the "issues" with this is, now if you have say 100 wake up
>> > > > > sources, with 10 entries each, then we're talking about a 1000 sysfs
>> > > > > files. Each one has to be opened, and read individually. This adds
>> > > > > overhead and it is more convenient to read from a single file. The
>> > > > > problem is this single file is not ABI. So the question I guess is,
>> > > > > how do we solve this in both an ABI friendly way while keeping the
>> > > > > overhead low.
>> > > >
>> > > > How much overhead? Have you measured it, reading from virtual files is
>> > > > fast :)
>> > >
>> > > I measured, and it is definitely not free. If you create and read a
>> > > 1000 files and just return a string back, it can take up to 11-13
>> > > milliseconds (did not lock CPU frequencies, was just looking for
>> > > average ball park). This is assuming that the counter reading is just
>> > > doing that, and nothing else is being done to return the sysfs data
>> > > which is probably not always true in practice.
>> > >
>> > > Our display pipeline deadline is around 16ms at 60Hz. Conceivably, any
>> > > CPU scheduling competion reading sysfs can hurt the deadline. There's
>> > > also the question of power - we definitely have spent time in the past
>> > > optimizing other virtual files such as /proc/pid/smaps for this reason
>> > > where it spent lots of CPU time.
>> >
>> > smaps was "odd", but that was done after measurements were actually made
>> > to prove it was needed. That hasn't happened yet :)
>> >
>> > And is there a reason you have to do this every 16ms?
>>
>> Not every, I was just saying whenever it happens and a frame delivery
>> deadline is missed, then a frame drop can occur which can result in a
>> poor user experience.
>
>
> But this is not done in the UI thread context. So some thread running for more than 16ms shouldn't cause a frame drop. If it does, we have bigger problems.
>
Not really. That depends on the priority of the other thread and other
things. It can obviously time share the same CPU as the UI thread if
it is not configured correctly. Even with CFS it can reduce the time
consumed by other "real-time" CFS threads. I am not sure what you are
proposing, there are also (obviously) power issues with things running
for long times pointlessly. We should try to do better if we can. As
Greg said, some study/research can be done on the use case before
settling for a solution (sysfs or other).
^ permalink raw reply
* Re: [GIT PULL] Power management fix for v5.2-rc6
From: pr-tracker-bot @ 2019-06-19 19:00 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Linus Torvalds, Linux PM, Linux PCI, Linux Kernel Mailing List
In-Reply-To: <CAJZ5v0hbF4i-mJ6thnu8HRNcAXBrw4=vqQG+ikufB2z3q82F4g@mail.gmail.com>
The pull request you sent on Wed, 19 Jun 2019 11:51:19 +0200:
> git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git pm-5.2-rc6
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/abf02e2964b3aaf1d5197c5d7c671e771b0e109e
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.wiki.kernel.org/userdoc/prtracker
^ permalink raw reply
* Re: Alternatives to /sys/kernel/debug/wakeup_sources
From: Joel Fernandes @ 2019-06-19 18:55 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Rafael J. Wysocki, Tri Vo, Rafael J. Wysocki, Sandeep Patil,
Viresh Kumar, Hridya Valsaraju, Linux PM, Cc: Android Kernel,
LKML, Alexei Starovoitov, Steven Rostedt, Alexei Starovoitov
In-Reply-To: <20190619183523.GA7018@kroah.com>
On Wed, Jun 19, 2019 at 2:35 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Wed, Jun 19, 2019 at 02:01:36PM -0400, Joel Fernandes wrote:
> > On Wed, Jun 19, 2019 at 1:07 PM Greg Kroah-Hartman
> > <gregkh@linuxfoundation.org> wrote:
> > >
> > > On Wed, Jun 19, 2019 at 12:53:12PM -0400, Joel Fernandes wrote:
> > > > > It is conceivable to have a "wakeup_sources" directory under
> > > > > /sys/power/ and sysfs nodes for all wakeup sources in there.
> > > >
> > > > One of the "issues" with this is, now if you have say 100 wake up
> > > > sources, with 10 entries each, then we're talking about a 1000 sysfs
> > > > files. Each one has to be opened, and read individually. This adds
> > > > overhead and it is more convenient to read from a single file. The
> > > > problem is this single file is not ABI. So the question I guess is,
> > > > how do we solve this in both an ABI friendly way while keeping the
> > > > overhead low.
> > >
> > > How much overhead? Have you measured it, reading from virtual files is
> > > fast :)
> >
> > I measured, and it is definitely not free. If you create and read a
> > 1000 files and just return a string back, it can take up to 11-13
> > milliseconds (did not lock CPU frequencies, was just looking for
> > average ball park). This is assuming that the counter reading is just
> > doing that, and nothing else is being done to return the sysfs data
> > which is probably not always true in practice.
> >
> > Our display pipeline deadline is around 16ms at 60Hz. Conceivably, any
> > CPU scheduling competion reading sysfs can hurt the deadline. There's
> > also the question of power - we definitely have spent time in the past
> > optimizing other virtual files such as /proc/pid/smaps for this reason
> > where it spent lots of CPU time.
>
> smaps was "odd", but that was done after measurements were actually made
> to prove it was needed. That hasn't happened yet :)
>
> And is there a reason you have to do this every 16ms?
Not every, I was just saying whenever it happens and a frame delivery
deadline is missed, then a frame drop can occur which can result in a
poor user experience.
^ permalink raw reply
* Re: Alternatives to /sys/kernel/debug/wakeup_sources
From: Greg Kroah-Hartman @ 2019-06-19 18:35 UTC (permalink / raw)
To: Joel Fernandes
Cc: Rafael J. Wysocki, Tri Vo, Rafael J. Wysocki, Sandeep Patil,
Viresh Kumar, Hridya Valsaraju, Linux PM, Cc: Android Kernel,
LKML, Alexei Starovoitov, Steven Rostedt, Alexei Starovoitov
In-Reply-To: <CAJWu+ookFTYGfSvJ3otpFQixG2kbkJGOqf7HHUeYNQAQv2Cskw@mail.gmail.com>
On Wed, Jun 19, 2019 at 02:01:36PM -0400, Joel Fernandes wrote:
> On Wed, Jun 19, 2019 at 1:07 PM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > On Wed, Jun 19, 2019 at 12:53:12PM -0400, Joel Fernandes wrote:
> > > > It is conceivable to have a "wakeup_sources" directory under
> > > > /sys/power/ and sysfs nodes for all wakeup sources in there.
> > >
> > > One of the "issues" with this is, now if you have say 100 wake up
> > > sources, with 10 entries each, then we're talking about a 1000 sysfs
> > > files. Each one has to be opened, and read individually. This adds
> > > overhead and it is more convenient to read from a single file. The
> > > problem is this single file is not ABI. So the question I guess is,
> > > how do we solve this in both an ABI friendly way while keeping the
> > > overhead low.
> >
> > How much overhead? Have you measured it, reading from virtual files is
> > fast :)
>
> I measured, and it is definitely not free. If you create and read a
> 1000 files and just return a string back, it can take up to 11-13
> milliseconds (did not lock CPU frequencies, was just looking for
> average ball park). This is assuming that the counter reading is just
> doing that, and nothing else is being done to return the sysfs data
> which is probably not always true in practice.
>
> Our display pipeline deadline is around 16ms at 60Hz. Conceivably, any
> CPU scheduling competion reading sysfs can hurt the deadline. There's
> also the question of power - we definitely have spent time in the past
> optimizing other virtual files such as /proc/pid/smaps for this reason
> where it spent lots of CPU time.
smaps was "odd", but that was done after measurements were actually made
to prove it was needed. That hasn't happened yet :)
And is there a reason you have to do this every 16ms?
> > And how often does this happen? Does it _need_ to happen?
>
> These are good questions and we should find out. I am not saying that
> sysfs will not work, I am just saying we need to consider all the
> things. I will let Tri look into this since he is working on it, I
> don't know off the top.
>
> > Parsing files is also hard, and not for sysfs files, you can't have it
> > both ways.
>
> I don't think we are concerned with a parsing issue here, or are
> discussing it in this thread.
>
> > So try it this way, and if there really is a performance issue, we can
> > then talk about it...
>
> Sure that sounds good to me, again I am not saying we should do sysfs.
> But we should consider all the issues and chose the right solution.
Figure out who needs this, how often it needs it, and how many files
really are going to be involved before we try to optimize anything.
thanks,
greg k-h
^ permalink raw reply
* Re: Alternatives to /sys/kernel/debug/wakeup_sources
From: Tri Vo @ 2019-06-19 18:31 UTC (permalink / raw)
To: Joel Fernandes
Cc: Greg Kroah-Hartman, Rafael J. Wysocki, Rafael J. Wysocki,
Sandeep Patil, Viresh Kumar, Hridya Valsaraju, Linux PM,
Cc: Android Kernel, LKML, Alexei Starovoitov, Steven Rostedt,
Alexei Starovoitov
In-Reply-To: <CAJWu+ookFTYGfSvJ3otpFQixG2kbkJGOqf7HHUeYNQAQv2Cskw@mail.gmail.com>
On Wed, Jun 19, 2019 at 11:01 AM Joel Fernandes <joelaf@google.com> wrote:
>
> On Wed, Jun 19, 2019 at 1:07 PM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > On Wed, Jun 19, 2019 at 12:53:12PM -0400, Joel Fernandes wrote:
> > > > It is conceivable to have a "wakeup_sources" directory under
> > > > /sys/power/ and sysfs nodes for all wakeup sources in there.
> > >
> > > One of the "issues" with this is, now if you have say 100 wake up
> > > sources, with 10 entries each, then we're talking about a 1000 sysfs
> > > files. Each one has to be opened, and read individually. This adds
> > > overhead and it is more convenient to read from a single file. The
> > > problem is this single file is not ABI. So the question I guess is,
> > > how do we solve this in both an ABI friendly way while keeping the
> > > overhead low.
> >
> > How much overhead? Have you measured it, reading from virtual files is
> > fast :)
>
> I measured, and it is definitely not free. If you create and read a
> 1000 files and just return a string back, it can take up to 11-13
> milliseconds (did not lock CPU frequencies, was just looking for
> average ball park). This is assuming that the counter reading is just
> doing that, and nothing else is being done to return the sysfs data
> which is probably not always true in practice.
>
> Our display pipeline deadline is around 16ms at 60Hz. Conceivably, any
> CPU scheduling competion reading sysfs can hurt the deadline. There's
> also the question of power - we definitely have spent time in the past
> optimizing other virtual files such as /proc/pid/smaps for this reason
> where it spent lots of CPU time.
>
> > And how often does this happen? Does it _need_ to happen?
>
> These are good questions and we should find out. I am not saying that
> sysfs will not work, I am just saying we need to consider all the
> things. I will let Tri look into this since he is working on it, I
> don't know off the top.
There are really two use cases for wakeup_sources in Android:
(1) As Sandeep pointed out, it's used to plot history of wakeup
sources. This use case might actually be performance sensitive. I'll
check with our internal Android teams and loop back here.
(2) wakeup_sources information is included in bugreports.
Reading/parsing wakeup_sources in this case only happens when
generating a bugreport, so not particularly sensitive to overhead.
^ permalink raw reply
* Re: Alternatives to /sys/kernel/debug/wakeup_sources
From: Joel Fernandes @ 2019-06-19 18:01 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Rafael J. Wysocki, Tri Vo, Rafael J. Wysocki, Sandeep Patil,
Viresh Kumar, Hridya Valsaraju, Linux PM, Cc: Android Kernel,
LKML, Alexei Starovoitov, Steven Rostedt, Alexei Starovoitov
In-Reply-To: <20190619170750.GB10107@kroah.com>
On Wed, Jun 19, 2019 at 1:07 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Wed, Jun 19, 2019 at 12:53:12PM -0400, Joel Fernandes wrote:
> > > It is conceivable to have a "wakeup_sources" directory under
> > > /sys/power/ and sysfs nodes for all wakeup sources in there.
> >
> > One of the "issues" with this is, now if you have say 100 wake up
> > sources, with 10 entries each, then we're talking about a 1000 sysfs
> > files. Each one has to be opened, and read individually. This adds
> > overhead and it is more convenient to read from a single file. The
> > problem is this single file is not ABI. So the question I guess is,
> > how do we solve this in both an ABI friendly way while keeping the
> > overhead low.
>
> How much overhead? Have you measured it, reading from virtual files is
> fast :)
I measured, and it is definitely not free. If you create and read a
1000 files and just return a string back, it can take up to 11-13
milliseconds (did not lock CPU frequencies, was just looking for
average ball park). This is assuming that the counter reading is just
doing that, and nothing else is being done to return the sysfs data
which is probably not always true in practice.
Our display pipeline deadline is around 16ms at 60Hz. Conceivably, any
CPU scheduling competion reading sysfs can hurt the deadline. There's
also the question of power - we definitely have spent time in the past
optimizing other virtual files such as /proc/pid/smaps for this reason
where it spent lots of CPU time.
> And how often does this happen? Does it _need_ to happen?
These are good questions and we should find out. I am not saying that
sysfs will not work, I am just saying we need to consider all the
things. I will let Tri look into this since he is working on it, I
don't know off the top.
> Parsing files is also hard, and not for sysfs files, you can't have it
> both ways.
I don't think we are concerned with a parsing issue here, or are
discussing it in this thread.
> So try it this way, and if there really is a performance issue, we can
> then talk about it...
Sure that sounds good to me, again I am not saying we should do sysfs.
But we should consider all the issues and chose the right solution.
thanks!
- Joel
^ permalink raw reply
* [PATCH] power: Do not clear events_check_enabled in pm_wakeup_pending()
From: Ravi Chandra Sadineni @ 2019-06-19 17:51 UTC (permalink / raw)
To: rjw, len.brown, pavel, gregkh, linux-pm, linux-kernel, tbroch,
ravisadineni, rajatja
events_check_enabled bool is set when wakeup_count sysfs attribute
is written. User level daemon is expected to write this attribute
just before suspend.
When this boolean is set, calls to pm_wakeup_event() will result in
increment of per device and global wakeup count that helps in
identifying the wake source. global wakeup count is also used by
pm_wakeup_pending() to identify if there are any pending events that
should result in an suspend abort.
Currently calls to pm_wakeup_pending() also clears events_check_enabled.
This can be a problem when there are multiple wake events or when the
suspend is aborted due to an interrupt on a shared interrupt line.
For example an Mfd device can create several platform devices which
might fetch the state on resume in the driver resume method and increment
the wakeup count if needed. But if events_check_enabled is cleared before
resume methods get to execute, wakeup count will not be incremented. Thus
let us not reset the bool here.
Note that events_check_enabled is also cleared in suspend.c/enter_state()
on every resume at the end.
Signed-off-by: Ravi Chandra Sadineni <ravisadineni@chromium.org>
---
drivers/base/power/wakeup.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c
index 5b2b6a05a4f3..88aade871589 100644
--- a/drivers/base/power/wakeup.c
+++ b/drivers/base/power/wakeup.c
@@ -838,7 +838,6 @@ bool pm_wakeup_pending(void)
split_counters(&cnt, &inpr);
ret = (cnt != saved_count || inpr > 0);
- events_check_enabled = !ret;
}
raw_spin_unlock_irqrestore(&events_lock, flags);
--
2.20.1
^ permalink raw reply related
* Re: [PATCH] power: Do not clear events_check_enabled in pm_wakeup_pending()
From: Greg KH @ 2019-06-19 17:41 UTC (permalink / raw)
To: Ravi Chandra Sadineni
Cc: len.brown, pavel, linux-pm, linux-kernel, Todd Broch, Rajat Jain
In-Reply-To: <CAEZbON5oRSnOHQrzPcapd=XrK4_ngAY-7hTzxMOyO+=rWJB1bA@mail.gmail.com>
On Fri, Jun 14, 2019 at 11:23:31AM -0700, Ravi Chandra Sadineni wrote:
> Hi,
>
> Just wanted to check if this o.k.
You seem to have forgoten to send this to the maintainer of this file :(
Please use scripts/get_maintainer.pl and resend it, remembering to send
it to Rafael next time :)
thanks,
greg k-h
^ permalink raw reply
* Re: Alternatives to /sys/kernel/debug/wakeup_sources
From: Greg Kroah-Hartman @ 2019-06-19 17:07 UTC (permalink / raw)
To: Joel Fernandes
Cc: Rafael J. Wysocki, Tri Vo, Rafael J. Wysocki, Sandeep Patil,
Viresh Kumar, Hridya Valsaraju, Linux PM, Cc: Android Kernel,
LKML, Alexei Starovoitov, Steven Rostedt, Alexei Starovoitov
In-Reply-To: <CAJWu+oqSgcBVhDY7CjWpNQrK=XiKAb5S-YSp=6-UM--UFmKvGQ@mail.gmail.com>
On Wed, Jun 19, 2019 at 12:53:12PM -0400, Joel Fernandes wrote:
> > It is conceivable to have a "wakeup_sources" directory under
> > /sys/power/ and sysfs nodes for all wakeup sources in there.
>
> One of the "issues" with this is, now if you have say 100 wake up
> sources, with 10 entries each, then we're talking about a 1000 sysfs
> files. Each one has to be opened, and read individually. This adds
> overhead and it is more convenient to read from a single file. The
> problem is this single file is not ABI. So the question I guess is,
> how do we solve this in both an ABI friendly way while keeping the
> overhead low.
How much overhead? Have you measured it, reading from virtual files is
fast :)
And how often does this happen? Does it _need_ to happen?
Parsing files is also hard, and not for sysfs files, you can't have it
both ways.
So try it this way, and if there really is a performance issue, we can
then talk about it...
thanks,
greg k-h
^ permalink raw reply
* Re: Alternatives to /sys/kernel/debug/wakeup_sources
From: Joel Fernandes @ 2019-06-19 16:53 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Tri Vo, Rafael J. Wysocki, Sandeep Patil, Viresh Kumar,
Hridya Valsaraju, Linux PM, Cc: Android Kernel,
Greg Kroah-Hartman, LKML, Alexei Starovoitov, Steven Rostedt,
Alexei Starovoitov
In-Reply-To: <CAJZ5v0gvzCx-7qS9qkxB=sGKjQJKMR7yCc21f=_vqrbZxMSWNg@mail.gmail.com>
On Wed, Jun 19, 2019 at 4:35 AM Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> On Wed, Jun 19, 2019 at 1:52 AM Joel Fernandes <joelaf@google.com> wrote:
> >
> > On Tue, Jun 18, 2019 at 7:15 PM Tri Vo <trong@android.com> wrote:
[snip]
> > > > > If no, then we would love to hear suggestions for any changes that need to be
> > > > > made or we simply just move the debugfs entry into somewhere like
> > > > > /sys/power/ ?
> > > >
> > > > No, moving that entire file from debugfs into sysfs is not an option either.
> > > >
> > > > The statistics for the wakeup sources associated with devices are already there
> > > > under /sys/devices/.../power/ , but I guess you want all wakeup sources?
> > > >
> > > > That would require adding a kobject to struct wakeup_source and exposing
> > > > all of the statistics as separate attributes under it. In which case it would be
> > > > good to replace the existing wakeup statistics under /sys/devices/.../power/
> > > > with symbolic links to the attributes under the wakeup_source kobject.
> > >
> > > Thanks for your input, Rafael! Your suggestion makes sense. I'll work
> > > on a patch for this.
> >
> > Does that entail making each wake up source, a new sysfs node under a
> > particular device, and then adding stats under that new node?
>
> Not under a device, because there are wakeup source objects without
> associated devices.
>
> It is conceivable to have a "wakeup_sources" directory under
> /sys/power/ and sysfs nodes for all wakeup sources in there.
One of the "issues" with this is, now if you have say 100 wake up
sources, with 10 entries each, then we're talking about a 1000 sysfs
files. Each one has to be opened, and read individually. This adds
overhead and it is more convenient to read from a single file. The
problem is this single file is not ABI. So the question I guess is,
how do we solve this in both an ABI friendly way while keeping the
overhead low.
Thanks,
- Joel
^ permalink raw reply
* Re: Alternatives to /sys/kernel/debug/wakeup_sources
From: Sandeep Patil @ 2019-06-19 16:51 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Joel Fernandes, Tri Vo, Rafael J. Wysocki, Viresh Kumar,
Hridya Valsaraju, Linux PM, Cc: Android Kernel,
Greg Kroah-Hartman, LKML
In-Reply-To: <CAJZ5v0gvzCx-7qS9qkxB=sGKjQJKMR7yCc21f=_vqrbZxMSWNg@mail.gmail.com>
On Wed, Jun 19, 2019 at 10:35:17AM +0200, Rafael J. Wysocki wrote:
> On Wed, Jun 19, 2019 at 1:52 AM Joel Fernandes <joelaf@google.com> wrote:
> >
> > On Tue, Jun 18, 2019 at 7:15 PM Tri Vo <trong@android.com> wrote:
> > [snip]
> > > > > > >
> > > > > > > Android userspace reading wakeup_sources is not ideal because:
> > > > > > > - Debugfs API is not stable, i.e. Android tools built on top of it are
> > > > > > > not guaranteed to be backward/forward compatible.
> > > > > > > - This file requires debugfs to be mounted, which itself is
> > > > > > > undesirable for security reasons.
> > > > > > >
> > > > > > > To address these problems, we want to contribute a way to expose these
> > > > > > > statistics that doesn't depend on debugfs.
> > > > > > >
> > > > > > > Some initial thoughts/questions: Should we expose the stats in sysfs?
> > > > > > > Or maybe implement eBPF-based solution? What do you think?
> > > > >
> > > > > We are going through Android's out-of-tree kernel dependencies along with
> > > > > userspace APIs that are not necessarily considered "stable and forever
> > > > > supported" upstream. The debugfs dependencies showed up on our radar as a
> > > > > result and so we are wondering if we should worry about changes in debugfs
> > > > > interface and hence the question(s) below.
> > > > >
> > > > > So, can we rely on /d/wakeup_sources to be considered a userspace API and
> > > > > hence maintained stable as we do for other /proc and /sys entries?
> > > > >
> > > > > If yes, then we will go ahead and add tests for this in LTP or
> > > > > somewhere else suitable.
> > > >
> > > > No, debugfs is not ABI.
> > > >
> > > > > If no, then we would love to hear suggestions for any changes that need to be
> > > > > made or we simply just move the debugfs entry into somewhere like
> > > > > /sys/power/ ?
> > > >
> > > > No, moving that entire file from debugfs into sysfs is not an option either.
> > > >
> > > > The statistics for the wakeup sources associated with devices are already there
> > > > under /sys/devices/.../power/ , but I guess you want all wakeup sources?
> > > >
> > > > That would require adding a kobject to struct wakeup_source and exposing
> > > > all of the statistics as separate attributes under it. In which case it would be
> > > > good to replace the existing wakeup statistics under /sys/devices/.../power/
> > > > with symbolic links to the attributes under the wakeup_source kobject.
> > >
> > > Thanks for your input, Rafael! Your suggestion makes sense. I'll work
> > > on a patch for this.
> >
> > Does that entail making each wake up source, a new sysfs node under a
> > particular device, and then adding stats under that new node?
>
> Not under a device, because there are wakeup source objects without
> associated devices.
>
> It is conceivable to have a "wakeup_sources" directory under
> /sys/power/ and sysfs nodes for all wakeup sources in there.
This is what I understood from your initial reply and I think it makes sense.
Thanks again, Rafael.
- ssp
>
> Then, instead of exposing wakeup statistics directly under
> /sys/devices/.../power/, there can be symbolic links from there to the
> new wakeup source nodes under "wakeup_sources" (so as to avoid
> exposing the same data in two different places in sysfs, which may be
> confusing).
>
> Cheers!
^ permalink raw reply
* Re: [IMX] [DRM]: suspend/resume support
From: Valdis Klētnieks @ 2019-06-19 16:26 UTC (permalink / raw)
To: Pintu Agarwal
Cc: Greg KH, Fabio Estevam, linux-pm, open list,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
Kernelnewbies
In-Reply-To: <CAOuPNLgYN3FDvNsaWqom45h7aFz=HczDdL-QoHOc_Sreqf8T2g@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1346 bytes --]
On Wed, 19 Jun 2019 20:47:34 +0530, Pintu Agarwal said:
> No I mean to say, there are lots of features and customization already
> done on this version and stabilized.
> Upgrading again may require months of effort.
This is what happens when you don't upstream your local changes.
And no, saying "But we're a small company and nobody cares" isn't an
excuse - Linux carried the entire Voyager architecture around for several years
for 2 machines. Not two models, 2 physical machines, the last 2 operational
systems of the product line.
(Not the Xubuntu-based Voyage distribution either - the Voyager was a mid-80s
SMP fault-tolerant system from NCR with up to 32 486/586 cores and 4G of
memory, which was a honking big system for the day...)
https://kernel.googlesource.com/pub/scm/linux/kernel/git/rzhang/linux/+/v2.6.20-rc1/Documentation/voyager.txt
The architecture was finally dropped in 2009 when enough hardware failures
had happened that James Bottomley was unable to create a bootable
system from the parts from both...
So if your production run is several thousand systems, that's *plenty* big
enough for patches and drivers (especially since drivers for hardware you
included in your several-thousand system run are also likely applicable to
a half dozen other vendors who made several thousand systems using the
same chipset....
[-- Attachment #2: Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply
* Re: [RFC PATCH 6/7] sched/cpufreq: Improve sugov_cpu_is_busy accuracy
From: Douglas Raillard @ 2019-06-19 16:19 UTC (permalink / raw)
To: Patrick Bellasi
Cc: linux-kernel, linux-pm, mingo, peterz, quentin.perret,
dietmar.eggemann
In-Reply-To: <20190516125552.hol3rasllhveekxq@e110439-lin>
Hi Patrick,
On 5/16/19 1:55 PM, Patrick Bellasi wrote:
> On 08-May 18:43, douglas.raillard@arm.com wrote:
>> From: Douglas RAILLARD <douglas.raillard@arm.com>
>>
>> Avoid assuming a CPU is busy when it has begun being idle before
>> get_next_freq() is called. This is achieved by making sure the CPU will
>> not be detected as busy by other CPUs whenever its utilization is
>> decreasing.
>
> If I understand it correctly, what you are after here is a "metric"
> which tells you (in a shared performance domain) if a CPU has been
> busy for a certain amount of time.
> You do that by reworking the way idle_calls are accounted for the
> sugov_update_single() case.
>
> That approach could work but it looks a bit convoluted in the way it's
> coded and it's also difficult to exclude there could be corner cases
> with wired behaviors.
> Isn't that why you "fix" the saved_idle_calls counter after all?
>
> What about a different approach where we:
>
> 1. we annotate the timestamp a CPU wakes up from IDLE (last_wakeup_time)
>
> 2. we use that annotated last_wake_time and the rq->nr_running to
> define the "cpu is busy" heuristic.
>
> Looking at a sibling CPU, I think we can end up with two main cases:
>
> 1. CPU's nr_running is == 0
> then we don't consider busy that CPU
>
> 2. CPU's nr_running is > 0
> then the CPU is busy iff
> (current_time - last_wakeup_tim) >= busy_threshold
>
> Notice that, when a CPU is active, its rq clock is periodically
> updated, at least once per tick. Thus, provided a tick time is not too
> long to declare busy a CPU, then the above logic should work.
>
> Perhaps the busy_threshold can also be defined considering the PELT
> dynamics and starting from an expected utilization increase converted
> in time.
After experimenting with quite a few combinations, I managed to get a heuristic
based on util_avg and util_est_enqueued that seems to work better in my case.
Key differences are:
* this new heuristic only really takes into account CFS signals
(current one based on idle calls takes into account everything that executes
on a given CPU.)
* it will mark a CPU as busy less often, since it should only trigger when
there is a change in the utilization of a currently enqueued tasks.
Util changes due to enqueue/dequeue will not trigger it, which IMHO
is desirable, since we only want to bias frequency selection
when we know that we don't have precise utilization values for the
enqueued tasks (because the task has changed its behavior).
That change will be part of v2 posting of this series.
> Could something like to above work or am I missing something?
>
>> Signed-off-by: Douglas RAILLARD <douglas.raillard@arm.com>
>> ---
>> kernel/sched/cpufreq_schedutil.c | 42 ++++++++++++++++++++++++++++----
>> 1 file changed, 37 insertions(+), 5 deletions(-)
>>
>> diff --git a/kernel/sched/cpufreq_schedutil.c b/kernel/sched/cpufreq_schedutil.c
>> index a12b7e5bc028..ce4b90cafbb5 100644
>> --- a/kernel/sched/cpufreq_schedutil.c
>> +++ b/kernel/sched/cpufreq_schedutil.c
>> @@ -62,6 +62,7 @@ struct sugov_cpu {
>> /* The field below is for single-CPU policies only: */
>> #ifdef CONFIG_NO_HZ_COMMON
>> unsigned long saved_idle_calls;
>> + unsigned long previous_util;
>> #endif
>> };
>>
>> @@ -181,14 +182,35 @@ static bool sugov_cpu_is_busy(struct sugov_cpu *sg_cpu)
>> return ret;
>> }
>>
>> -static void sugov_cpu_is_busy_update(struct sugov_cpu *sg_cpu)
>> +static void sugov_cpu_is_busy_update(struct sugov_cpu *sg_cpu,
>> + unsigned long util)
>> {
>> unsigned long idle_calls = tick_nohz_get_idle_calls_cpu(sg_cpu->cpu);
>> sg_cpu->saved_idle_calls = idle_calls;
>> +
>> + /*
>> + * Make sure that this CPU will not be immediately considered as busy in
>> + * cases where the CPU has already entered an idle state. In that case,
>> + * the number of idle_calls will not vary anymore until it exits idle,
>> + * which would lead sugov_cpu_is_busy() to say that this CPU is busy,
>> + * because it has not (re)entered idle since the last time we looked at
>> + * it.
>> + * Assuming cpu0 and cpu1 are in the same policy, that will make sure
>> + * this sequence of events leads to right cpu1 business status from
>> + * get_next_freq(cpu=1)
>> + * cpu0: [enter idle] -> [get_next_freq] -> [doing nothing] -> [wakeup]
>> + * cpu1: ... -> [get_next_freq] -> ...
>> + */
>> + if (util <= sg_cpu->previous_util)
>> + sg_cpu->saved_idle_calls--;
>> +
>> + sg_cpu->previous_util = util;
>> }
>> #else
>> static inline bool sugov_cpu_is_busy(struct sugov_cpu *sg_cpu) { return false; }
>> -static void sugov_cpu_is_busy_update(struct sugov_cpu *sg_cpu) {}
>> +static void sugov_cpu_is_busy_update(struct sugov_cpu *sg_cpu
>> + unsigned long util)
>> +{}
>> #endif /* CONFIG_NO_HZ_COMMON */
>>
>> /**
>> @@ -507,10 +529,9 @@ static void sugov_update_single(struct update_util_data *hook, u64 time,
>> if (!sugov_should_update_freq(sg_policy, time))
>> return;
>>
>> - busy = sugov_cpu_is_busy(sg_cpu);
>> - sugov_cpu_is_busy_update(sg_cpu);
>> -
>> util = sugov_get_util(sg_cpu);
>> + busy = sugov_cpu_is_busy(sg_cpu);
>> + sugov_cpu_is_busy_update(sg_cpu, util);
>> max = sg_cpu->max;
>> util = sugov_iowait_apply(sg_cpu, time, util, max);
>> next_f = get_next_freq(sg_policy, util, max);
>> @@ -545,12 +566,15 @@ static unsigned int sugov_next_freq_shared(struct sugov_cpu *sg_cpu, u64 time)
>> struct cpufreq_policy *policy = sg_policy->policy;
>> unsigned long util = 0, max = 1;
>> unsigned int j;
>> + unsigned long sg_cpu_util = 0;
>>
>> for_each_cpu(j, policy->cpus) {
>> struct sugov_cpu *j_sg_cpu = &per_cpu(sugov_cpu, j);
>> unsigned long j_util, j_max;
>>
>> j_util = sugov_get_util(j_sg_cpu);
>> + if (j_sg_cpu == sg_cpu)
>> + sg_cpu_util = j_util;
>> j_max = j_sg_cpu->max;
>> j_util = sugov_iowait_apply(j_sg_cpu, time, j_util, j_max);
>>
>> @@ -560,6 +584,14 @@ static unsigned int sugov_next_freq_shared(struct sugov_cpu *sg_cpu, u64 time)
>> }
>> }
>>
>> + /*
>> + * Only update the business status if we are looking at the CPU for
>> + * which a utilization change triggered a call to get_next_freq(). This
>> + * way, we don't affect the "busy" status of CPUs that don't have any
>> + * change in utilization.
>> + */
>> + sugov_cpu_is_busy_update(sg_cpu, sg_cpu_util);
>> +
>> return get_next_freq(sg_policy, util, max);
>> }
>>
>> --
>> 2.21.0
>>
>
Thanks,
Douglas
^ permalink raw reply
* Re: [RFC PATCH 1/7] PM: Introduce em_pd_get_higher_freq()
From: Douglas Raillard @ 2019-06-19 16:08 UTC (permalink / raw)
To: Patrick Bellasi, Quentin Perret
Cc: linux-kernel, linux-pm, mingo, peterz, dietmar.eggemann
In-Reply-To: <20190516132250.hedtianse7rnk3wq@e110439-lin>
Hi Patrick,
On 5/16/19 2:22 PM, Patrick Bellasi wrote:
> On 16-May 14:01, Quentin Perret wrote:
>> On Thursday 16 May 2019 at 13:42:00 (+0100), Patrick Bellasi wrote:
>>>> +static inline unsigned long em_pd_get_higher_freq(struct em_perf_domain *pd,
>>>> + unsigned long min_freq, unsigned long cost_margin)
>>>> +{
>>>> + unsigned long max_cost = 0;
>>>> + struct em_cap_state *cs;
>>>> + int i;
>>>> +
>>>> + if (!pd)
>>>> + return min_freq;
>>>> +
>>>> + /* Compute the maximum allowed cost */
>>>> + for (i = 0; i < pd->nr_cap_states; i++) {
>>>> + cs = &pd->table[i];
>>>> + if (cs->frequency >= min_freq) {
>>>> + max_cost = cs->cost + (cs->cost * cost_margin) / 1024;
>>> ^^^^
>>> ... end here we should probably better use SCHED_CAPACITY_SCALE
>>> instead of hard-coding in values, isn't it?
>>
>> I'm not sure to agree. This isn't part of the scheduler per se, and the
>> cost thing isn't in units of capacity, but in units of power, so I don't
>> think SCHED_CAPACITY_SCALE is correct here.
>
> Right, I get the units do not match and it would not be elegant to use
> it here...
>
>> But I agree these hard coded values (that one, and the 512 in one of the
>> following patches) could use some motivation :-)
>
> ... ultimately SCHED_CAPACITY_SCALE is just SCHED_FIXEDPOINT_SCALE,
> which is adimensional. Perhaps we should use that or yet another alias
> for the same.
Would it be a good idea to use SCHED_FIXEDPOINT_SCALE in energy.c ?
Since it's not part of the scheduler, maybe there is a scale covering a wider scope,
or we can introduce a similar ENERGY_FIXEDPOINT_SCALE in energy_model.h.
>> Thanks,
>> Quentin
>
Thanks,
Douglas
^ permalink raw reply
* Re: [IMX] [DRM]: suspend/resume support
From: Pintu Agarwal @ 2019-06-19 15:29 UTC (permalink / raw)
To: Fabio Estevam
Cc: open list,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
Kernelnewbies, linux-pm
In-Reply-To: <CAOMZO5BsJWTw0nCeUboam4kuKyCO3N_Ch5ZW8k5Y9KFtQBanhQ@mail.gmail.com>
On Wed, Jun 19, 2019 at 8:41 PM Fabio Estevam <festevam@gmail.com> wrote:
>
> > > Then you can test suspend/resume and see if HDMI hangs or not.
> > >
> > By this suspend/resume, you mean "suspend-to-ram" or "suspend-to-disk" ?
>
> I tested with "echo mem > /sys/power/state"
>
hmm. That's what I said. This is runtime suspend, and it works for me also.
I am talking about hibernation:
# echo disk > /sys/power/state (with some customization)
> > This scenario is not with suspend/resume.
> > This hang is, when we make hdmi as a loadable module (.ko) and trying
> > to install it after resume.
> > In this case, suspend/resume will not come into picture. Not sure why
> > it still hangs.
> > Do you have any clue for this scenario?
>
> I haven't tried this one.
>
> Please test it with 5.1.11 and if it fails, please report.
Okay will check this on latest kernel. Thanks.
^ permalink raw reply
* Re: [PATCH] PM: sleep: Update struct wakeup_source documentation
From: Greg Kroah-Hartman @ 2019-06-19 15:15 UTC (permalink / raw)
To: Rafael J. Wysocki; +Cc: Linux PM, LKML
In-Reply-To: <9413171.SRfamDnp5r@kreacher>
On Wed, Jun 19, 2019 at 12:48:15AM +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> The kerneldoc comment for struct wakeup_source has become outdated,
> so fix that.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
^ permalink raw reply
* Re: [IMX] [DRM]: suspend/resume support
From: Pintu Agarwal @ 2019-06-19 15:17 UTC (permalink / raw)
To: Greg KH
Cc: Fabio Estevam, linux-pm, open list,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
Kernelnewbies
In-Reply-To: <20190619150406.GB19346@kroah.com>
On Wed, Jun 19, 2019 at 8:34 PM Greg KH <greg@kroah.com> wrote:
>
> On Wed, Jun 19, 2019 at 07:03:39PM +0530, Pintu Agarwal wrote:
> > You can think that the version I am using right now is almost 4.9 itself.
> > Upgrading again to higher kernel version is again difficult, also
> > customer does not want it.
>
> They don't want a secure and supported kernel over the lifetime of their
> device? That sounds really odd.
No I mean to say, there are lots of features and customization already
done on this version and stabilized.
Upgrading again may require months of effort.
Of course upgrading is planned for later products.
> Never create a new device using the 4.9.y kernel tree unless you have someone else doing the support for it
> (i.e. a SoC vendor that you are paying support from).
>
Actually, this is not a new device. Its an older device itself.
This is an experimental activity.
^ permalink raw reply
* Re: [IMX] [DRM]: suspend/resume support
From: Fabio Estevam @ 2019-06-19 15:11 UTC (permalink / raw)
To: Pintu Agarwal
Cc: open list,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
Kernelnewbies, linux-pm
In-Reply-To: <CAOuPNLhZhgN26rquLQq9zHBct1QxK-7hXAza0xk-0QooPGYLNw@mail.gmail.com>
On Wed, Jun 19, 2019 at 12:05 PM Pintu Agarwal <pintu.ping@gmail.com> wrote:
> Okay. I will surely try it on another IMX6 fresh board.
> With 5.1.11 you mean the kernel from mainline kernel.org
> Or, is there a separate kernel repo for imx6. If yes, please let me know.
5.1.11 from kernel.org
> > Then you can test suspend/resume and see if HDMI hangs or not.
> >
> By this suspend/resume, you mean "suspend-to-ram" or "suspend-to-disk" ?
I tested with "echo mem > /sys/power/state"
> This scenario is not with suspend/resume.
> This hang is, when we make hdmi as a loadable module (.ko) and trying
> to install it after resume.
> In this case, suspend/resume will not come into picture. Not sure why
> it still hangs.
> Do you have any clue for this scenario?
I haven't tried this one.
Please test it with 5.1.11 and if it fails, please report.
^ permalink raw reply
* Re: [IMX] [DRM]: suspend/resume support
From: Pintu Agarwal @ 2019-06-19 15:04 UTC (permalink / raw)
To: Fabio Estevam
Cc: open list,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
Kernelnewbies, linux-pm
In-Reply-To: <CAOMZO5ADK1L5UMM9XZetHvmjTvmvUg99G7VPdeXitgpctGLCkw@mail.gmail.com>
On Wed, Jun 19, 2019 at 7:14 PM Fabio Estevam <festevam@gmail.com> wrote:
>
> On Wed, Jun 19, 2019 at 10:33 AM Pintu Agarwal <pintu.ping@gmail.com> wrote:
>
> > You can think that the version I am using right now is almost 4.9 itself.
>
> "almost 4.9" does not help, sorry.
>
> > Upgrading again to higher kernel version is again difficult, also
> > customer does not want it.
>
> dw-hdmi is well supported on i.MX6 with mainline, so you could simply try that.
>
> Try booting 5.1.11 on your custom board with a very minimal dts with
> HDMI support.
>
Okay. I will surely try it on another IMX6 fresh board.
With 5.1.11 you mean the kernel from mainline kernel.org
Or, is there a separate kernel repo for imx6. If yes, please let me know.
> Then you can test suspend/resume and see if HDMI hangs or not.
>
By this suspend/resume, you mean "suspend-to-ram" or "suspend-to-disk" ?
> > Another approach I tried is, installing hdmi {dw-hdmi.ko,
> > dw_hdmi-imx.ko} as a module, just after the resume.
> > With this also system hangs during installation of dw_hdmi-imx.ko at
> > below location:
> >
> > imx_drm_driver_load(..)
> > |
> > |--> component_bind_all(...)
> > |
> > |-> component_bind(ipu)
> > |
> > |
> > ipu_drm_bind(..)
> > |
> > |-> ipu_crtc_init(..)
> > |
> > |-> ipu_plane_irq(..)
> > |
> > | ----> << __HANGS HERE__ >>
>
> I am not able to reproduce this hang. As I mentioned yesterday
> suspend/resume is working just fine.
>
This scenario is not with suspend/resume.
This hang is, when we make hdmi as a loadable module (.ko) and trying
to install it after resume.
In this case, suspend/resume will not come into picture. Not sure why
it still hangs.
Do you have any clue for this scenario?
^ permalink raw reply
* Re: [IMX] [DRM]: suspend/resume support
From: Greg KH @ 2019-06-19 15:04 UTC (permalink / raw)
To: Pintu Agarwal
Cc: Fabio Estevam, linux-pm, open list,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
Kernelnewbies
In-Reply-To: <CAOuPNLjYhkP_kL+q-ZpiDZMMpOHrU88BFBc2agtnCzXt8dihOg@mail.gmail.com>
On Wed, Jun 19, 2019 at 07:03:39PM +0530, Pintu Agarwal wrote:
> You can think that the version I am using right now is almost 4.9 itself.
> Upgrading again to higher kernel version is again difficult, also
> customer does not want it.
They don't want a secure and supported kernel over the lifetime of their
device? That sounds really odd. Never create a new device using the
4.9.y kernel tree unless you have someone else doing the support for it
(i.e. a SoC vendor that you are paying support from).
good luck!
greg k-h
^ permalink raw reply
* Re: [PATCH v15 0/7] support ROHM BD70528 PMIC
From: Alexandre Belloni @ 2019-06-19 14:31 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Lee Jones, mazziesaccount, Rob Herring, Mark Rutland,
Michael Turquette, Stephen Boyd, Linus Walleij,
Bartosz Golaszewski, Liam Girdwood, Mark Brown, Alessandro Zummo,
devicetree, linux-kernel, linux-clk, linux-gpio, linux-pm,
linux-rtc
In-Reply-To: <20190613130529.kgswgbuszb24itxz@earth.universe>
On 13/06/2019 15:05:29+0200, Sebastian Reichel wrote:
> Hi,
>
> On Wed, Jun 12, 2019 at 07:03:28AM +0100, Lee Jones wrote:
> > On Tue, 11 Jun 2019, Sebastian Reichel wrote:
> > > On Mon, Jun 03, 2019 at 10:23:37AM +0300, Matti Vaittinen wrote:
> > > > Patch series introducing support for ROHM BD70528 PMIC
> > > > [...]
> > >
> > > I think all patches have been reviewed by the respective subsystem
> > > maintainers. Lee, can you provide an immutable branch with the MFD
> > > patches (1, 2, 4)? Looks like the other patches only depend on those
> > > and can go through their respective subsystems.
> >
> > Yes. It's on my TODO list.
>
> Thanks.
>
> > Would you prefer this method over me just taking them all and sending
> > out a PR? The latter is my usual flow, but I'm happy with either.
>
> Both methods are fine with me. I usually go with "my" method, since
> that results in less shared commits. It simplifies potential
> follow-up bug fixes, since there is only one tree to fix.
>
Personally, I'm fine with Lee taking everything in his tree as it is
very unlikely to require follow up patches this cycle (i.e before rc1).
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [PATCH 4/6] cpufreq: Use has_target() instead of !setpolicy
From: Viresh Kumar @ 2019-06-19 14:20 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Rafael Wysocki, Linux PM, Vincent Guittot,
Linux Kernel Mailing List
In-Reply-To: <CAJZ5v0gmkAS-A2eT5VUyuBMD9+FfsM0HL-HPeUYUV24_oMTvVw@mail.gmail.com>
On 19-06-19, 14:28, Rafael J. Wysocki wrote:
> On Wed, Jun 19, 2019 at 1:36 PM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> >
> > For code consistency, use has_target() instead of !setpolicy everywhere,
> > as it is already done at several places.
>
> That's OK
>
> > Maybe we should also use !has_target() for setpolicy case to use only one expression
> > for this differentiation.
>
> But I'm not sure what you mean here?
At many places in code we are doing tests like:
if (cpufreq_driver->setpolicy) {
xxx
}
Maybe we can write them as well like:
if (!has_target()) {
xxx
}
--
viresh
^ permalink raw reply
* Re: [PATCH 3/6] cpufreq: Remove the has_target() check from notifier handler
From: Viresh Kumar @ 2019-06-19 14:19 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Rafael Wysocki, Linux PM, Vincent Guittot,
Linux Kernel Mailing List
In-Reply-To: <CAJZ5v0iR8oBZ2mM2vdHzTBNcjsHa6QtJmN+Q17Q8ZD-ny3qy3w@mail.gmail.com>
On 19-06-19, 14:25, Rafael J. Wysocki wrote:
> On Wed, Jun 19, 2019 at 1:35 PM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> >
> > cpufreq_notify_transition() is only called for has_target() type driver
> > and not for set_policy type, and the check is simply redundant. Remove
> > it.
>
> Ah, OK
>
> So this patch removes a check introduced by the previous one. :-)
>
> Please merge them together.
It made sense to keep them separate because there are two different
issues I am fixing here. But if that is what you want, I will merge
them.
--
viresh
^ permalink raw reply
* Re: [PATCH 2/6] cpufreq: Replace few CPUFREQ_CONST_LOOPS checks with has_target()
From: Viresh Kumar @ 2019-06-19 14:18 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Rafael Wysocki, Linux PM, Vincent Guittot,
Linux Kernel Mailing List
In-Reply-To: <CAJZ5v0g1avBwjezWpMimGbs1NHOchib9pmTKoxaixKtpw_CGJw@mail.gmail.com>
On 19-06-19, 14:20, Rafael J. Wysocki wrote:
> On Wed, Jun 19, 2019 at 1:36 PM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> > diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> > index 54befd775bd6..e59194c2c613 100644
> > --- a/drivers/cpufreq/cpufreq.c
> > +++ b/drivers/cpufreq/cpufreq.c
> > @@ -359,12 +359,10 @@ static void cpufreq_notify_transition(struct cpufreq_policy *policy,
> > * which is not equal to what the cpufreq core thinks is
> > * "old frequency".
> > */
> > - if (!(cpufreq_driver->flags & CPUFREQ_CONST_LOOPS)) {
> > - if (policy->cur && (policy->cur != freqs->old)) {
> > - pr_debug("Warning: CPU frequency is %u, cpufreq assumed %u kHz\n",
> > - freqs->old, policy->cur);
> > - freqs->old = policy->cur;
> > - }
> > + if (has_target() && policy->cur && policy->cur != freqs->old) {
> > + pr_debug("Warning: CPU frequency is %u, cpufreq assumed %u kHz\n",
> > + freqs->old, policy->cur);
> > + freqs->old = policy->cur;
>
> Is cpufreq_notify_transition() ever called if ->setpolicy drivers are in use?
I tried to find it, but I couldn't find any driver from where we can
get this called for setpolicy drivers.
--
viresh
^ permalink raw reply
* Re: [IMX] [DRM]: suspend/resume support
From: Fabio Estevam @ 2019-06-19 13:44 UTC (permalink / raw)
To: Pintu Agarwal
Cc: open list,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
Kernelnewbies, linux-pm
In-Reply-To: <CAOuPNLjYhkP_kL+q-ZpiDZMMpOHrU88BFBc2agtnCzXt8dihOg@mail.gmail.com>
On Wed, Jun 19, 2019 at 10:33 AM Pintu Agarwal <pintu.ping@gmail.com> wrote:
> You can think that the version I am using right now is almost 4.9 itself.
"almost 4.9" does not help, sorry.
> Upgrading again to higher kernel version is again difficult, also
> customer does not want it.
dw-hdmi is well supported on i.MX6 with mainline, so you could simply try that.
Try booting 5.1.11 on your custom board with a very minimal dts with
HDMI support.
Then you can test suspend/resume and see if HDMI hangs or not.
> Another approach I tried is, installing hdmi {dw-hdmi.ko,
> dw_hdmi-imx.ko} as a module, just after the resume.
> With this also system hangs during installation of dw_hdmi-imx.ko at
> below location:
>
> imx_drm_driver_load(..)
> |
> |--> component_bind_all(...)
> |
> |-> component_bind(ipu)
> |
> |
> ipu_drm_bind(..)
> |
> |-> ipu_crtc_init(..)
> |
> |-> ipu_plane_irq(..)
> |
> | ----> << __HANGS HERE__ >>
I am not able to reproduce this hang. As I mentioned yesterday
suspend/resume is working just fine.
We would be glad to help fixing the hang if you could reproduce it
with a mainline kernel.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox