On 2 October 2014 14:00, Sylwester Nawrocki [off-list ref] wrote:

Hello Ulf,

On 02/10/14 11:09, Ulf Hansson wrote:

quoted

On 1 October 2014 18:36, Sylwester Nawrocki [off-list ref] wrote:

quoted

On 01/10/14 16:41, Ulf Hansson wrote:

quoted

There are currently no users of this API, let's remove it.

Hi Sylwester,

Thanks for your reply!

quoted

The sad fact is that removal of pm_genpd_dev_need_restore() calls
from arch/arm/mach-exynos/pm_domains.c introduces regressions in
multiple exynos drivers (I'm sure it breaks media drivers).

The fact that you need pm_genpd_dev_need_restore() is really worrying.
That indicates that exynos are suffering from this race condition I am
trying to fix in this patchset.

quoted

I think before doing such changes all relevant drivers should be
updated first. I need to take a closer look again, but it seems
after dropping the pm_genpd_dev_need_restore() calls, client
driver's runtime_resume() callback is not being called in response
to first pm_runtime_get(_sync) call, even if a device is runtime
pm active.

Sorry, I meant to say "inactive" here.

Ahh, now I see the approach.

Correct me if I am wrong, but I think in principle these exynos
drivers don't use pm_runtime_set_active() during ->probe() and are
instead relying on CONFIG_PM_RUNTIME to be enabled.

That's not a good behaviour. If these drivers are build without
CONFIG_PM_RUNTIME - they won't work.

I guess I have pointed out this in earlier discussions around runtime
PM, I have seen quite some driver's getting their support for runtime
PM wrong.

quoted

Why would runtime PM callbacks be invoked when the device are runtime
PM active? That's prevented by the runtime PM core and is the expected
behaviour.

pm_genpd_dev_need_restore() is not the solution, besides that it gives
you the option to lie about device's runtime PM state to genpd. Thus,
if you are really lucky, that might workaround your issues. :-)

Might be, nevertheless so far it more or less worked for us. At least
I'm sure without it everything breaks right away.

I see. Obviously we must not break exynos, let's try to figure out the
best way forward.

quoted

I will happily help out in fixing drivers for exynos. Would be nice if
you could provide me with a list of which driver/subsystems that seems
broken. HW, wise I have a exynos 5250 and exynos 5422 on my desk, so
those I can test.

For exynos5 I would check exynos-gsc and s5p-mfc (see kernel logs below).

Thanks, I will have look and run some tests on exynos 5. Let you know soon.

I could take care of other, exynos4 drivers. But I believe the fix
belongs in the PM core, I doubt we can solve in the driver a problem
which only occurs to one instance (first probed) of a device from a set
of devices in the power domain.

quoted

quoted

More details can be found in commit ebc35c726298ba3fdebba316a
'ARM: EXYNOS: register devices in 'need_restore' state for pm_domains'.

The above only happens when devices are added to an inactive power
domain, then I guess patch 2/4 is also an attempt to address this
issue ?

Yes.

I would really appreciate if you could run a test with the complete
patchset and see if that resolves the issues.

Sure, is there a git tree I could fetch all the relevant patches from ?
I'm not sure what the base tree for this series, I could only apply
first 2 patches from this thread, on top of the generic OF PM domain
patches. And that didn't solve issues which are seen in the logs below.

You have two options:

1) Use/build Rafaels tree. Apply my patches on the bleeding edge
branch.That's also what I use a work base.
http://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git

2) Use/build Stephen Rothwell's linux-next tree. I can confirm the
patches applies on the master branch as of today.
http://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git

I really appreciate your help here, thanks!

Nevertheless, as Rafael pointed out, enabling all power domains
unconditionally seems a step backwards. It would be nice to have
resolved the race in a away the power domains remain in state left
by the firmware and are being enabled only before any client device
actually needs it.

You are right, but I am not sure we can do this in one go - since it
depends on if we can fix this on PM core of if we need need
adaptations in drivers/subsystems.

Kind regards
Uffe