Hi,

On Mon, Jul 16, 2018 at 12:11 PM, Vivek Gautam
[off-list ref] wrote:

HI Rafael,



On 7/16/2018 2:21 PM, Rafael J. Wysocki wrote:

quoted

On Thu, Jul 12, 2018 at 12:57 PM, Vivek Gautam
[off-list ref] wrote:

[cut]

quoted

quoted

quoted

Although, given the PM
subsystem internals, the suspend function wouldn't be called on SMMU
implementation needed power control (since they would have runtime PM
enabled) and on others, it would be called but do nothing (since no
clocks).

quoted

Honestly, I just don't know. :-)

It just looks odd the way it is done.  I think the clock should be
gated during system-wide suspend too, because the system can spend
much more time in a sleep state than in the working state, on average.

And note that you cannot rely on runtime PM to always do it for you,
because it may be disabled at a client device or even blocked by user
space via power/control in sysfs and that shouldn't matter for
system-wide PM.

User space blocking runtime PM through sysfs is a good point. I'm not
100% sure how the PM subsystem deals with that in case of system-wide
suspend. I guess for consistency and safety, we should have the
suspend callback.

Will add the following suspend callback (same as
arm_smmu_runtime_suspend):

  static int __maybe_unused arm_smmu_pm_suspend(struct device *dev)
  {
          struct arm_smmu_device *smmu = dev_get_drvdata(dev);

          clk_bulk_disable(smmu->num_clks, smmu->clks);

          return 0;
  }

I think you also need to check if the clock has already been disabled
by runtime PM.  Otherwise you may end up disabling it twice in a row.

Should I rather call a pm_runtime_put() in suspend callback?

That wouldn't work as runtime PM may be effectively disabled by user
space via sysfs.  That's one of the reasons why you need the extra
system-wide suspend callback in the first place. :-)

Or an expanded form something similar to:
https://elixir.bootlin.com/linux/v4.18-rc5/source/drivers/slimbus/qcom-ctrl.c#L695

Yes, you can do something like that, but be careful to make sure that
the state of the device after system-wide resume is consistent with
its runtime PM status in all cases.
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno