[...]

quoted

quoted

quoted

quoted

quoted

quoted

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.

quoted

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?

quoted

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.

quoted

quoted

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. :-)

quoted

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?

quoted

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel