From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Rafael J. Wysocki" Subject: Re: [PATCH v12 1/4] iommu/arm-smmu: Add pm_runtime/sleep ops Date: Mon, 16 Jul 2018 12:23:12 +0200 Message-ID: References: <20180708173413.1965-1-vivek.gautam@codeaurora.org> <20180708173413.1965-2-vivek.gautam@codeaurora.org> <17407514.unFVTGoGrn@aspire.rjw.lan> <010cb56a-36e8-e729-1fe7-738048eb551d@codeaurora.org> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <010cb56a-36e8-e729-1fe7-738048eb551d-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: freedreno-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org Sender: "Freedreno" To: Vivek Gautam Cc: Mark Rutland , "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" , Linux PM , Stephen Boyd , "Rafael J. Wysocki" , Joerg Roedel , "Rafael J. Wysocki" , Will Deacon , Linux Kernel Mailing List , Tomasz Figa , "list-Y9sIeH5OGRo@public.gmane.org:IOMMU DRIVERS" , Rob Herring , linux-arm-msm , freedreno List-Id: linux-arm-msm@vger.kernel.org SGksCgpPbiBNb24sIEp1bCAxNiwgMjAxOCBhdCAxMjoxMSBQTSwgVml2ZWsgR2F1dGFtCjx2aXZl ay5nYXV0YW1AY29kZWF1cm9yYS5vcmc+IHdyb3RlOgo+IEhJIFJhZmFlbCwKPgo+Cj4KPiBPbiA3 LzE2LzIwMTggMjoyMSBQTSwgUmFmYWVsIEouIFd5c29ja2kgd3JvdGU6Cj4+Cj4+IE9uIFRodSwg SnVsIDEyLCAyMDE4IGF0IDEyOjU3IFBNLCBWaXZlayBHYXV0YW0KPj4gPHZpdmVrLmdhdXRhbUBj b2RlYXVyb3JhLm9yZz4gd3JvdGU6CgpbY3V0XQoKPj4+PiBBbHRob3VnaCwgZ2l2ZW4gdGhlIFBN Cj4+Pj4gc3Vic3lzdGVtIGludGVybmFscywgdGhlIHN1c3BlbmQgZnVuY3Rpb24gd291bGRuJ3Qg YmUgY2FsbGVkIG9uIFNNTVUKPj4+PiBpbXBsZW1lbnRhdGlvbiBuZWVkZWQgcG93ZXIgY29udHJv bCAoc2luY2UgdGhleSB3b3VsZCBoYXZlIHJ1bnRpbWUgUE0KPj4+PiBlbmFibGVkKSBhbmQgb24g b3RoZXJzLCBpdCB3b3VsZCBiZSBjYWxsZWQgYnV0IGRvIG5vdGhpbmcgKHNpbmNlIG5vCj4+Pj4g Y2xvY2tzKS4KPj4+Pgo+Pj4+PiBIb25lc3RseSwgSSBqdXN0IGRvbid0IGtub3cuIDotKQo+Pj4+ Pgo+Pj4+PiBJdCBqdXN0IGxvb2tzIG9kZCB0aGUgd2F5IGl0IGlzIGRvbmUuICBJIHRoaW5rIHRo ZSBjbG9jayBzaG91bGQgYmUKPj4+Pj4gZ2F0ZWQgZHVyaW5nIHN5c3RlbS13aWRlIHN1c3BlbmQg dG9vLCBiZWNhdXNlIHRoZSBzeXN0ZW0gY2FuIHNwZW5kCj4+Pj4+IG11Y2ggbW9yZSB0aW1lIGlu IGEgc2xlZXAgc3RhdGUgdGhhbiBpbiB0aGUgd29ya2luZyBzdGF0ZSwgb24gYXZlcmFnZS4KPj4+ Pj4KPj4+Pj4gQW5kIG5vdGUgdGhhdCB5b3UgY2Fubm90IHJlbHkgb24gcnVudGltZSBQTSB0byBh bHdheXMgZG8gaXQgZm9yIHlvdSwKPj4+Pj4gYmVjYXVzZSBpdCBtYXkgYmUgZGlzYWJsZWQgYXQg YSBjbGllbnQgZGV2aWNlIG9yIGV2ZW4gYmxvY2tlZCBieSB1c2VyCj4+Pj4+IHNwYWNlIHZpYSBw b3dlci9jb250cm9sIGluIHN5c2ZzIGFuZCB0aGF0IHNob3VsZG4ndCBtYXR0ZXIgZm9yCj4+Pj4+ IHN5c3RlbS13aWRlIFBNLgo+Pj4+Cj4+Pj4gVXNlciBzcGFjZSBibG9ja2luZyBydW50aW1lIFBN IHRocm91Z2ggc3lzZnMgaXMgYSBnb29kIHBvaW50LiBJJ20gbm90Cj4+Pj4gMTAwJSBzdXJlIGhv dyB0aGUgUE0gc3Vic3lzdGVtIGRlYWxzIHdpdGggdGhhdCBpbiBjYXNlIG9mIHN5c3RlbS13aWRl Cj4+Pj4gc3VzcGVuZC4gSSBndWVzcyBmb3IgY29uc2lzdGVuY3kgYW5kIHNhZmV0eSwgd2Ugc2hv dWxkIGhhdmUgdGhlCj4+Pj4gc3VzcGVuZCBjYWxsYmFjay4KPj4+Cj4+PiBXaWxsIGFkZCB0aGUg Zm9sbG93aW5nIHN1c3BlbmQgY2FsbGJhY2sgKHNhbWUgYXMKPj4+IGFybV9zbW11X3J1bnRpbWVf c3VzcGVuZCk6Cj4+Pgo+Pj4gICBzdGF0aWMgaW50IF9fbWF5YmVfdW51c2VkIGFybV9zbW11X3Bt X3N1c3BlbmQoc3RydWN0IGRldmljZSAqZGV2KQo+Pj4gICB7Cj4+PiAgICAgICAgICAgc3RydWN0 IGFybV9zbW11X2RldmljZSAqc21tdSA9IGRldl9nZXRfZHJ2ZGF0YShkZXYpOwo+Pj4KPj4+ICAg ICAgICAgICBjbGtfYnVsa19kaXNhYmxlKHNtbXUtPm51bV9jbGtzLCBzbW11LT5jbGtzKTsKPj4+ Cj4+PiAgICAgICAgICAgcmV0dXJuIDA7Cj4+PiAgIH0KPj4KPj4gSSB0aGluayB5b3UgYWxzbyBu ZWVkIHRvIGNoZWNrIGlmIHRoZSBjbG9jayBoYXMgYWxyZWFkeSBiZWVuIGRpc2FibGVkCj4+IGJ5 IHJ1bnRpbWUgUE0uICBPdGhlcndpc2UgeW91IG1heSBlbmQgdXAgZGlzYWJsaW5nIGl0IHR3aWNl IGluIGEgcm93Lgo+Cj4KPiBTaG91bGQgSSByYXRoZXIgY2FsbCBhIHBtX3J1bnRpbWVfcHV0KCkg aW4gc3VzcGVuZCBjYWxsYmFjaz8KClRoYXQgd291bGRuJ3Qgd29yayBhcyBydW50aW1lIFBNIG1h eSBiZSBlZmZlY3RpdmVseSBkaXNhYmxlZCBieSB1c2VyCnNwYWNlIHZpYSBzeXNmcy4gIFRoYXQn cyBvbmUgb2YgdGhlIHJlYXNvbnMgd2h5IHlvdSBuZWVkIHRoZSBleHRyYQpzeXN0ZW0td2lkZSBz dXNwZW5kIGNhbGxiYWNrIGluIHRoZSBmaXJzdCBwbGFjZS4gOi0pCgo+IE9yIGFuIGV4cGFuZGVk IGZvcm0gc29tZXRoaW5nIHNpbWlsYXIgdG86Cj4gaHR0cHM6Ly9lbGl4aXIuYm9vdGxpbi5jb20v bGludXgvdjQuMTgtcmM1L3NvdXJjZS9kcml2ZXJzL3NsaW1idXMvcWNvbS1jdHJsLmMjTDY5NQoK WWVzLCB5b3UgY2FuIGRvIHNvbWV0aGluZyBsaWtlIHRoYXQsIGJ1dCBiZSBjYXJlZnVsIHRvIG1h a2Ugc3VyZSB0aGF0CnRoZSBzdGF0ZSBvZiB0aGUgZGV2aWNlIGFmdGVyIHN5c3RlbS13aWRlIHJl c3VtZSBpcyBjb25zaXN0ZW50IHdpdGgKaXRzIHJ1bnRpbWUgUE0gc3RhdHVzIGluIGFsbCBjYXNl cy4KX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KRnJlZWRy ZW5vIG1haWxpbmcgbGlzdApGcmVlZHJlbm9AbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8v bGlzdHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZnJlZWRyZW5vCg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-0.9 required=3.0 tests=DKIM_SIGNED, MAILING_LIST_MULTI,SPF_PASS,T_DKIM_INVALID,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 21F93ECDFAA for ; Mon, 16 Jul 2018 10:23:16 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id C6B6E208E3 for ; Mon, 16 Jul 2018 10:23:15 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="UH1eaVM5" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org C6B6E208E3 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728450AbeGPKt5 (ORCPT ); Mon, 16 Jul 2018 06:49:57 -0400 Received: from mail-oi0-f68.google.com ([209.85.218.68]:34632 "EHLO mail-oi0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727128AbeGPKt5 (ORCPT ); Mon, 16 Jul 2018 06:49:57 -0400 Received: by mail-oi0-f68.google.com with SMTP id 13-v6so73768726ois.1; Mon, 16 Jul 2018 03:23:13 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:sender:in-reply-to:references:from:date:message-id :subject:to:cc; bh=nTqpGmU8kuqxTbhRDqaClGdv9ylIhoy/Qnv+PsLP8mo=; b=UH1eaVM5YseVeZn9rl0afpt8BmONiUZPi7QcVja6rvpL7jpC/wP2e60VO658PSdjFI ZgieDUqOTnIvtVt1Jl4Z5TQGRyHmyy0wYHbe9WKDOQSsE2KPxViOHJ0mhaamuh/ljLk0 rFCPO8+MdoJLNdo6SJzoU9KZH818j9ShRRT8mGfH4rDuAVpvGXOEeXUsZamLwjZ4Sf7R /T2gAxsS/GaWIuKkSZZEJe6etW3hE8JeOp8IGYt8HMF0pfIAS02fuX6yh6Fc3AHjZ158 Mc1q7vThjTxD7D9mw8lI2k1muZurLyHspWHA+envqfBAcv2VjND24/FQ61bO3hQI30hv AsiQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:sender:in-reply-to:references:from :date:message-id:subject:to:cc; bh=nTqpGmU8kuqxTbhRDqaClGdv9ylIhoy/Qnv+PsLP8mo=; b=sZwQTWco09uPvQBB3kEi8TMmI/MsZjWL6diGatN0+IGcD6+mo7EDwQWggi1khNGWX3 cr4Yx5m70+1PPR+fHK3X1iOC9if5/5mn6yZAwci6Ab8Pn7jCbCZcWLl0jGOGEn6jltuf UC3wxWFzywijhn8ND+kDrBDH8i5MCwxZ+o/k6Uk7wVkDss2LaT74GQlk3HRUqIcbW1Ov mtjsHVRSD2W6TpZwVuMjqE7smo74n8AsuFKuxxvXKJMOBJ/FNQQpcWlWGhXDtjNyVJjZ QnQWALCT0msEypfaymEL/wLJCGMZx7KBgUXIgfIQqzgrw2DJCJ285C1LNGLV2SM8utNx 0hWw== X-Gm-Message-State: AOUpUlFRuc8DVa7DPkMJo1GnGzpWuRNE/1omxglm8CL/5B+isdsSdkfr Ltmwocvi/GmFB3NnnXxaEzOYmknORnJKwvNppnM= X-Google-Smtp-Source: AAOMgpe2aWR36y6x+FgCv9Vww9BQb/6+3vW5yp2Uab3xFtlQ6Zt85DiC7VAdI1mlZuMyvMfV0CJM7HQ0p1a/4N/uhAE= X-Received: by 2002:aca:ef44:: with SMTP id n65-v6mr16501435oih.120.1531736592651; Mon, 16 Jul 2018 03:23:12 -0700 (PDT) MIME-Version: 1.0 Received: by 2002:a9d:63d2:0:0:0:0:0 with HTTP; Mon, 16 Jul 2018 03:23:12 -0700 (PDT) In-Reply-To: <010cb56a-36e8-e729-1fe7-738048eb551d@codeaurora.org> References: <20180708173413.1965-1-vivek.gautam@codeaurora.org> <20180708173413.1965-2-vivek.gautam@codeaurora.org> <17407514.unFVTGoGrn@aspire.rjw.lan> <010cb56a-36e8-e729-1fe7-738048eb551d@codeaurora.org> From: "Rafael J. Wysocki" Date: Mon, 16 Jul 2018 12:23:12 +0200 X-Google-Sender-Auth: tuPiBTnbA989LSn13gfPaXiVI70 Message-ID: Subject: Re: [PATCH v12 1/4] iommu/arm-smmu: Add pm_runtime/sleep ops To: Vivek Gautam Cc: "Rafael J. Wysocki" , Tomasz Figa , Mark Rutland , "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" , Linux PM , Stephen Boyd , Will Deacon , "Rafael J. Wysocki" , Linux Kernel Mailing List , "list@263.net:IOMMU DRIVERS" , Joerg Roedel , Rob Herring , linux-arm-msm , freedreno Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, On Mon, Jul 16, 2018 at 12:11 PM, Vivek Gautam wrote: > HI Rafael, > > > > On 7/16/2018 2:21 PM, Rafael J. Wysocki wrote: >> >> On Thu, Jul 12, 2018 at 12:57 PM, Vivek Gautam >> wrote: [cut] >>>> 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). >>>> >>>>> 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.