lkml.org 
[lkml]   [2021]   [Oct]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH 2/2] PM: sleep: Fix runtime PM based cpuidle support
    [...]

    > > > > > >
    > > > > > > > Additionally, since find_deepest_state() is being called for
    > > > > > > > cpuidle_enter_s2idle() too, we would need to treat the new
    > > > > > > > CPUIDLE_STATE_DISABLED_ flag in a special way, right?
    > > > > > >
    > > > > > > No, it already checks "disabled".
    > > > > >
    > > > > > Yes, but that would be wrong.
    > > > >
    > > > > Hmmm.
    > > > >
    > > > > > The use case I want to support, for cpuidle-psci, is to allow all idle
    > > > > > states in suspend-to-idle,
    > > > >
    > > > > So does PM-runtime work in suspend-to-idle? How?
    > > > >
    > > > > > but prevent those that rely on runtime PM
    > > > > > (after it has been disabled) for the regular idle path.
    > > > >
    > > > > Do you have a special suspend-to-idle handling of those states that
    > > > > doesn't require PM-runtime?
    > > >
    > > > Regardless, pausing cpuidle in the suspend-to-idle path simply doesn't
    > > > make sense at all, so this needs to be taken care of in the first
    > > > place.
    > >
    > > Right, I do agree, don't get me wrong. But, do we really want to treat
    > > s2-to-idle differently, compared to s2-to-ram in regards to this?
    > >
    > > Wouldn't it be a lot easier to let cpuidle drivers to opt-out for
    > > cpuidle_pause|resume(), no matter whether it's for s2-to-idle or
    > > s2-to-ram?
    >
    > I don't think so.
    >
    > Suspend-to-idle resume cpuidle after pausing it which is just plain
    > confusing and waste of energy and the fact that the system-wide
    > suspend flow interferes with using PM-runtime for implementing cpuidle
    > callbacks at the low level really is an orthogonal problem.

    It's certainly an orthogonal problem, I agree. However, trying to
    solve it in two different ways, may not really be worth the effort, in
    my opinion.

    As I kind of pointed out in the earlier reply, I am not sure there are
    any other relatively easy solutions available, to fix the problem for
    runtime PM based cpuidle drivers. We probably need to call
    cpuidle_pause() (or similar) in some way.

    >
    > > >
    > > > The problem with PM-runtime being unavailable after dpm_suspend()
    > > > needs to be addressed in a different way IMO, because it only affects
    > > > one specific use case.
    > >
    > > It's one specific case so far, but we have the riscv driver on its
    > > way, which would suffer from the same problem.
    >
    > So perhaps they should be advised about this issue.

    Yes, I will let them know - and hopefully I will soon also be able to
    provide them with a fix. :-)

    >
    > > Anyway, an option is to figure out what platforms and cpuidle drivers,
    > > that really needs cpuidle_pause|resume() at this point and make an
    > > opt-in solution instead.
    >
    > None of them need to pause cpuidle for suspend-to-idle AFAICS.

    I assume so too, otherwise things would have been broken when
    cpuidle_resume() is called in s2idle_enter(). But, it's still a bit
    unclear.

    >
    > Some may want it in the non-s2idle suspend path, but I'm not sure
    > about the exact point where cpuidle needs to be paused in this case.
    > Possibly before offlining the nonboot CPUs.

    Okay.

    Note that, I assume it would be okay to also pause cpuidle a bit
    earlier in these cases, like in dpm_suspend() for example. The point
    is, it's really a limited short period of time for when cpuidle would
    be paused, so I doubt it would have any impact on the consumed energy.
    Right?

    >
    > > This could then be used by runtime PM based
    > > cpuidle drivers as well. Would that be a way forward?
    >
    > The PM-runtime case should be addressed directly IMO, we only need to
    > figure out how to do that.

    If you have any other suggestions, I am listening. :-)

    >
    > I'm wondering how you are dealing with the case when user space
    > prevents pd_dev from suspending via sysfs, for that matter.

    That should work fine during runtime - because runtime PM is enabled
    for the device.

    Kind regards
    Uffe

    \
     
     \ /
      Last update: 2021-10-21 20:37    [W:2.588 / U:0.000 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site