Re: [PATCH RFC 08/27] PM / Domains: Support IRQ safe PM domains

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Jan 15 2016 at 01:55 -0700, Ulf Hansson wrote:
[...]

*) Non-IRQ safe parent + IRQ-safe subdomain

Let's name this case 1.

  Attach subdomain:
        - parent mutex held and subdomain spinlocked.
  Power-on:
        - subdomain assumes parent is always ON or powered on before the
        subdomain is powered ON.

As stated above, what do you think of *not* dealing with this case as
I think it's not a solution but a workaround!

  Power-off:
        - parent is not powered off even if the subdomain was the last
        active domain.

**) IRQ-safe parent + IRQ-safe subdomain

Let's name this case 2.

   Attach subdomain:
        - parent spinlock held and subdomain spinlocked.
   Power-on:
        - subdomain may power on the parent.
   Power-off:
        - last active sub-domain will power off the parent in the same
          context.

From a genpd locking point of view, there should be no reason to power
off in same context. We should be able to deal with this via a
scheduled work for this case as well, right!?

I agree it need not be a requirement. May be we can have an option to
schedule the work. In the case of CPU power management, scheduling a
work is not an option, especially when the domain and the CPUs are
collapsing.

Although, I realize that it's very useful to power off in the same
context to minimize latencies in the full blown CPU Cluster Power
Management solution.
Perhaps a separate genpd configuration flag could be added to enable
this method, as I imagine that not *all* IRQ safe domains wants to use
it.

What do you think?

May be we can provide the is_async option as a genpd flag.


***) IRQ-safe parent + non-IRQ-safe subdomain

Let's name this case 3.

   Attach subdomain:
        - parent spinlock held and subdomain **cannot** be mutex locked.

First, attaching shouldn't occur from atomic context.

Second, the important part is that we pick the locks in same order as
elsewhere in genpd, and of course we must not pick a mutex while
holding a spinlock.

In all cases when adding subdomains, we should start by fetching the
subdomains lock *then* the lock for the parent. The current genpd code
doesn't do that and additionally it uses nested locks.
I am going to send a patch the changes this behaviour, as I think it's
wrong and may cause deadlocks!

Ah. Makes sense. I wish I had thought of it.

Following this approach means that case 1 can't be supported, as that
would mean holding a spinlock while fetching a mutex.

   Power-on:
        - subdomain may power on the parent.
   Power-off:
        - last active subdomain will be able to power off the domain

Except for the last case, we can support the others in addition to the
currently available, with the restriction that
- IRQ-safe parents can only have IRQ-safe subdomains and devices

- Non-IRQ safe parents may have both IRQ-safe and non-IRQ-safe
 subdomains and devices and the assumption that
        -- IRQ-safe subdomains and devices will not bother to power
        on/off their non-IRQ-safe parent.


I think case 2 and case 3 should be okay to support, but I am really
questioning case 1.

With you other patch, it does make sense to have 2 and 3 instead of 1
and 2.
My only concern is as I view the SoC as a whole, there would be cases
where the parent domain could only be non-IRQ safe (turning off a big
regulator may require some sleep) and the subdomains are quicker and
faster IRQ-safe domains. Putting up this restriction, chops up the
domain hierarchy. Imagine, if you could represent the whole power domain
organization in DT, but because of this restriction, we may not be able
to do that.

Thanks,
Lina
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux