Re: [PATCH v2 1/4] PM / Domains: Remove pm_genpd_dev_need_restore() API

[Sylwester Nawrocki <s.nawrocki@samsung.com>]

On 02/10/14 15:30, Ulf Hansson wrote:
[...]
> 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.

Yes, pm_runtime_set_active() is not used in probe(), I believe this
is not required. In case of those IP blocks there is no use of activating
them during probe(). Instead we check if PM_RUNTIME is enabled through
pm_runtime_enabled() helper and enable the device clock(s) if not.
I agree it all doesn't quite work in current mainline for !PM_RUNTIME,
since there is nothing ensuring that the power domains are enabled
in such kernel configuration.

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

They wouldn't similarly work with pm_runtime_set_active() call in probe()
with CONFIG_PM_RUNTIME disabled, would they ?

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

Especially it might be interesting look at gscaler, where there are 
multiple instances of same device handled by one driver.

[...]
>> 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

Thanks, I will be able to take a look at this the earliest on Monday. 

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

I see, fortunately the genpd API appears not to be wildly used yet.

-- 
Regards,
Sylwester