diff mbox

[v7,1/6] base: power: runtime: Export pm_runtime_get/put_suppliers

Message ID 1517999482-17317-2-git-send-email-vivek.gautam@codeaurora.org (mailing list archive)
State Not Applicable, archived
Delegated to: Andy Gross
Headers show

Commit Message

Vivek Gautam Feb. 7, 2018, 10:31 a.m. UTC
The device link allows the pm framework to tie the supplier and
consumer. So, whenever the consumer is powered-on the supplier
is powered-on first.

There are however cases in which the consumer wants to power-on
the supplier, but not itself.
E.g., A Graphics or multimedia driver wants to power-on the SMMU
to unmap a buffer and finish the TLB operations without powering
on itself. Some of these unmap requests are coming from the
user space when the controller itself is not powered-up, and it
can be huge penalty in terms of power and latency to power-up
the graphics/mm controllers.
There can be an argument that the supplier should handle this case
on its own and there should not be a need for the consumer to
power-on the supplier. But as discussed on the thread [1] about
ARM-SMMU runtime pm, we don't want to introduce runtime pm calls
in atomic path in arm_smmu_unmap.

[1] https://patchwork.kernel.org/patch/9827825/

Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org>
Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/base/power/runtime.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Tomasz Figa Feb. 13, 2018, 7:44 a.m. UTC | #1
Hi Vivek,

On Wed, Feb 7, 2018 at 7:31 PM, Vivek Gautam
<vivek.gautam@codeaurora.org> wrote:
> The device link allows the pm framework to tie the supplier and
> consumer. So, whenever the consumer is powered-on the supplier
> is powered-on first.
>
> There are however cases in which the consumer wants to power-on
> the supplier, but not itself.
> E.g., A Graphics or multimedia driver wants to power-on the SMMU
> to unmap a buffer and finish the TLB operations without powering
> on itself.

This sounds strange to me. If the SMMU is powered down, wouldn't the
TLB lose its contents as well (and so no flushing needed)?

Other than that, what kind of hardware operations would be needed
besides just updating the page tables from the CPU?

Best regards,
Tomasz
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Robin Murphy Feb. 13, 2018, noon UTC | #2
On 13/02/18 07:44, Tomasz Figa wrote:
> Hi Vivek,
> 
> On Wed, Feb 7, 2018 at 7:31 PM, Vivek Gautam
> <vivek.gautam@codeaurora.org> wrote:
>> The device link allows the pm framework to tie the supplier and
>> consumer. So, whenever the consumer is powered-on the supplier
>> is powered-on first.
>>
>> There are however cases in which the consumer wants to power-on
>> the supplier, but not itself.
>> E.g., A Graphics or multimedia driver wants to power-on the SMMU
>> to unmap a buffer and finish the TLB operations without powering
>> on itself.
> 
> This sounds strange to me. If the SMMU is powered down, wouldn't the
> TLB lose its contents as well (and so no flushing needed)?

Depends on implementation details - if runtime PM is actually 
implemented via external clock gating (in the absence of fine-grained 
power domains), then "suspended" TLBs might both retain state and not 
receive invalidation requests, which is really the worst case.

> Other than that, what kind of hardware operations would be needed
> besides just updating the page tables from the CPU?

Domain attach/detach also require updating SMMU hardware state (and 
possibly TLB maintenance), but don't logically require the master device 
itself to be active at the time.

Robin.
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tomasz Figa Feb. 13, 2018, 12:54 p.m. UTC | #3
On Tue, Feb 13, 2018 at 9:00 PM, Robin Murphy <robin.murphy@arm.com> wrote:
> On 13/02/18 07:44, Tomasz Figa wrote:
>>
>> Hi Vivek,
>>
>> On Wed, Feb 7, 2018 at 7:31 PM, Vivek Gautam
>> <vivek.gautam@codeaurora.org> wrote:
>>>
>>> The device link allows the pm framework to tie the supplier and
>>> consumer. So, whenever the consumer is powered-on the supplier
>>> is powered-on first.
>>>
>>> There are however cases in which the consumer wants to power-on
>>> the supplier, but not itself.
>>> E.g., A Graphics or multimedia driver wants to power-on the SMMU
>>> to unmap a buffer and finish the TLB operations without powering
>>> on itself.
>>
>>
>> This sounds strange to me. If the SMMU is powered down, wouldn't the
>> TLB lose its contents as well (and so no flushing needed)?
>
>
> Depends on implementation details - if runtime PM is actually implemented
> via external clock gating (in the absence of fine-grained power domains),
> then "suspended" TLBs might both retain state and not receive invalidation
> requests, which is really the worst case.

Agreed. That's why in "[PATCH v7 3/6] iommu/arm-smmu: Invoke
pm_runtime during probe, add/remove device" I actually suggested
managing clocks separately from runtime PM. At least until runtime PM
framework arrives at a state, where multiple power states can be
managed, i.e. full power state, clock-gated state, domain-off state.
(I think I might have seen some ongoing work on this on LWN though...)

>
>> Other than that, what kind of hardware operations would be needed
>> besides just updating the page tables from the CPU?
>
>
> Domain attach/detach also require updating SMMU hardware state (and possibly
> TLB maintenance), but don't logically require the master device itself to be
> active at the time.

Wouldn't this hardware state need to be reinitialized anyway after
respective power domain power cycles? (In other words, hardware would
only need programming if it's powered on at the moment.)

Best regards,
Tomasz
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Robin Murphy Feb. 13, 2018, 1:37 p.m. UTC | #4
On 13/02/18 12:54, Tomasz Figa wrote:
> On Tue, Feb 13, 2018 at 9:00 PM, Robin Murphy <robin.murphy@arm.com> wrote:
>> On 13/02/18 07:44, Tomasz Figa wrote:
>>>
>>> Hi Vivek,
>>>
>>> On Wed, Feb 7, 2018 at 7:31 PM, Vivek Gautam
>>> <vivek.gautam@codeaurora.org> wrote:
>>>>
>>>> The device link allows the pm framework to tie the supplier and
>>>> consumer. So, whenever the consumer is powered-on the supplier
>>>> is powered-on first.
>>>>
>>>> There are however cases in which the consumer wants to power-on
>>>> the supplier, but not itself.
>>>> E.g., A Graphics or multimedia driver wants to power-on the SMMU
>>>> to unmap a buffer and finish the TLB operations without powering
>>>> on itself.
>>>
>>>
>>> This sounds strange to me. If the SMMU is powered down, wouldn't the
>>> TLB lose its contents as well (and so no flushing needed)?
>>
>>
>> Depends on implementation details - if runtime PM is actually implemented
>> via external clock gating (in the absence of fine-grained power domains),
>> then "suspended" TLBs might both retain state and not receive invalidation
>> requests, which is really the worst case.
> 
> Agreed. That's why in "[PATCH v7 3/6] iommu/arm-smmu: Invoke
> pm_runtime during probe, add/remove device" I actually suggested
> managing clocks separately from runtime PM. At least until runtime PM
> framework arrives at a state, where multiple power states can be
> managed, i.e. full power state, clock-gated state, domain-off state.
> (I think I might have seen some ongoing work on this on LWN though...)
> 
>>
>>> Other than that, what kind of hardware operations would be needed
>>> besides just updating the page tables from the CPU?
>>
>>
>> Domain attach/detach also require updating SMMU hardware state (and possibly
>> TLB maintenance), but don't logically require the master device itself to be
>> active at the time.
> 
> Wouldn't this hardware state need to be reinitialized anyway after
> respective power domain power cycles? (In other words, hardware would
> only need programming if it's powered on at the moment.)

Yes, if the entire SMMU was fully powered down because all masters were 
inactive, then all that should need to be done is to update the software 
shadow state in the expectation that arm_smmu_reset() would re-sync it 
upon TCU powerup. If at least some part of the internal logic remains 
active, though, then you may or may not need to fiddle with zero or more 
clocks and/or power domains (depending on microarchitecture and 
integration) in order to be sure that everything from the programming 
slave interface through to wherever that state is kept works correctly 
so that it can be changed.

The main motivation here is that the Qualcomm SMMU microarchitecture 
apparently allows the programming interface to be shut down separately 
from the TCU core (context banks, page table walker, etc.), and they get 
an appreciable power saving from doing so. This is different from, say, 
the Arm Ltd. implementations, where the entire TCU is a single 
clock/power domain internally (although you could maybe still gate the 
external APB interface clock).

As the previous discussions have shown, this is really, really hard to 
do properly in a generic manner.

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

Patch

diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
index 8bef3cb2424d..5b8226c8af19 100644
--- a/drivers/base/power/runtime.c
+++ b/drivers/base/power/runtime.c
@@ -1579,6 +1579,7 @@  void pm_runtime_get_suppliers(struct device *dev)
 
 	device_links_read_unlock(idx);
 }
+EXPORT_SYMBOL_GPL(pm_runtime_get_suppliers);
 
 /**
  * pm_runtime_put_suppliers - Drop references to supplier devices.
@@ -1597,6 +1598,7 @@  void pm_runtime_put_suppliers(struct device *dev)
 
 	device_links_read_unlock(idx);
 }
+EXPORT_SYMBOL_GPL(pm_runtime_put_suppliers);
 
 void pm_runtime_new_link(struct device *dev)
 {