From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751605AbdLSQyU (ORCPT ); Tue, 19 Dec 2017 11:54:20 -0500 Received: from mail-ot0-f196.google.com ([74.125.82.196]:38068 "EHLO mail-ot0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750877AbdLSQyQ (ORCPT ); Tue, 19 Dec 2017 11:54:16 -0500 X-Google-Smtp-Source: ACJfBov8r6m1jjd89mXLj8i2kCW+8ZP4p/YXYilPyzO/j5oHHVN3KG/5cD8ceFgTZfLjPBcZ1igh7wB2BdG4qIXAsow= MIME-Version: 1.0 In-Reply-To: References: <7742130.AaJQIxeI1n@aspire.rjw.lan> <252167445.OQUXi2Ca9Y@aspire.rjw.lan> From: "Rafael J. Wysocki" Date: Tue, 19 Dec 2017 17:54:15 +0100 X-Google-Sender-Auth: pCKByJ4eP_PhLpYndhfvgiIyF28 Message-ID: Subject: Re: [PATCH 3/4] PM / core: Direct DPM_FLAG_SMART_SUSPEND optimization To: Ulf Hansson Cc: "Rafael J. Wysocki" , Linux PM , Greg Kroah-Hartman , Alan Stern , Kevin Hilman , LKML , Mika Westerberg Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Dec 19, 2017 at 5:29 PM, Rafael J. Wysocki wrote: > On Tue, Dec 19, 2017 at 2:10 PM, Ulf Hansson wrote: >> On 19 December 2017 at 12:13, Rafael J. Wysocki wrote: >>> On Tue, Dec 19, 2017 at 8:38 AM, Ulf Hansson wrote: >>>> On 10 December 2017 at 01:00, Rafael J. Wysocki wrote: >>>>> From: Rafael J. Wysocki >>>>> >>>>> Make the PM core avoid invoking the "late" and "noirq" system-wide >>>>> suspend (or analogous) callbacks provided by device drivers directly >>>>> for devices with DPM_FLAG_SMART_SUSPEND set that are in runtime >>>>> suspend during the "late" and "noirq" phases of system-wide suspend >>>>> (or analogous) transitions. That is only done for devices without >>>>> any middle-layer "late" and "noirq" suspend callbacks (to avoid >>>>> confusing the middle layer if there is one). >>>>> >>>>> The underlying observation is that runtime PM is disabled for devices >>>>> during the "late" and "noirq" system-wide suspend phases, so if they >>>>> remain in runtime suspend from the "late" phase forward, it doesn't >>>>> make sense to invoke the "late" and "noirq" callbacks provided by >>>>> the drivers for them (arguably, the device is already suspended and >>>>> in the right state). Thus, if the remaining driver suspend callbacks >>>>> are to be invoked directly by the core, they can be skipped. >>>>> >>> >>> It looks like I'm consistently failing to explain my point clearly enough. :-) >>> >>>> As I have stated earlier, this isn't going to solve the general case, >>>> as the above change log seems to state. > > No, it doesn't, as long as drivers follow the documentation. > > So your concern seems to be "What if I don't follow the > documentation?" which, honestly, is not something I can address. :-) I'm not sure why this ended up in this particular place ... I must have messed up something. >>> Well, it doesn't say that or anything similar, at least to my eyes. >>> >>> The observation is that if you have set DPM_FLAG_SMART_SUSPEND, then >>> you need to be prepared for your ->suspend_late and ->suspend_noirq to >>> be skipped (because the ACPI PM domain does that and you may happen to >>> work with it, for example) if the device is already suspended at the >>> beginning of the "late suspend" phase. That's already documented. >>> >>> Given the above, and the fact that there is not much to be done for a >>> suspended device in ->suspend_late and ->suspend_noirq, why can't the >>> core skip these callbacks too if there's no middle layer? >>> >>> But the reason why I really need this is because >>> i2c-designware-platdrv can work both with the ACPI PM domain and >>> standalone and I need it to be handled consistently in both cases. >> >> Yeah, I understand that. >> >>> >>>> I think we really need to do that, before adding yet another system suspend/resume optimization >>>> path in the PM core. >>> >>> So what exactly is the technical argument in the above? >> >> As stated, when the driver needs additional operations to be done, it >> all falls a part. So I wanted to say the above thing here: + No, it doesn't, as long as drivers follow the documentation. + + So your concern seems to be "What if I don't follow the + documentation?" which, honestly, is not something I can address. :-) > If the driver *needs* such operations to be done, then it *should* > *not* set DPM_FLAG_SMART_SUSPEND as per the existing documentation. Thanks, Rafael