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 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 61724CEDD8F for ; Wed, 9 Oct 2024 12:48:42 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:Content-Transfer-Encoding: Content-Type:In-Reply-To:From:References:CC:To:Subject:MIME-Version:Date: Message-ID:Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=b9CbWLDMZti2ojgRTihH3RA0WYLUzEcG7aX6pdihoQY=; b=dQvTMGmXDOanSaM7DRunzpxbDE P96WEucNxPvIp03vUEthVzo8sxtIfXmrRJB1qD6Vf7MCRI7KSMtWi9xlg7+6ANigOdpJrCjm347aW DsBuwO52fOlo8wIU5tIg6PSxLZntlIRrTD98qxY+0CtJY9BJZwZwfTPypgVklSfjOmd7KzjVr5SEa dB0hPH/tsvNqaxr2s26wY+6zAoTVd+d7SNwmeuLFP2mkZWPWbcXM+JVjey8MxifYDhrukCCTkfwBS hvgLOMMsl1BbK42Yu/clYUsewQ2uOhFkyWA0hCNrN2R6ojL/eog2i3zbRi1n9VQJ0jslCX8Fe6A44 P+G0O2rA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98 #2 (Red Hat Linux)) id 1syW7M-00000009HAb-2Vit; Wed, 09 Oct 2024 12:48:40 +0000 Received: from mx0a-001ae601.pphosted.com ([67.231.149.25] helo=mx0b-001ae601.pphosted.com) by bombadil.infradead.org with esmtps (Exim 4.98 #2 (Red Hat Linux)) id 1syW7G-00000009H6w-3tJ9; Wed, 09 Oct 2024 12:48:36 +0000 Received: from pps.filterd (m0077473.ppops.net [127.0.0.1]) by mx0a-001ae601.pphosted.com (8.18.1.2/8.18.1.2) with ESMTP id 4994xc0x003245; Wed, 9 Oct 2024 07:48:18 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cirrus.com; h=cc :content-transfer-encoding:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to; s= PODMain02222019; bh=b9CbWLDMZti2ojgRTihH3RA0WYLUzEcG7aX6pdihoQY=; b= WI0sVaycTZYkwMgmtYcXXMdvigsiua8Ix4Qh02mNCGH2fBXcoCmYBNNsEO5BXVde 1Bz9WSIl3H9aytV4MjeLGq5iNgl4nMnifXKD7gZBG7Nh0M0RDNAmEax34ZlRrQpc BqxwXp6rQsj7d8HBI3neZNecBDiZiUXrqk9SjO/XH7GSP3KBa0vc3lo3Nh2evgTs vMpU/eouIhXwHQgJkPlVYadHgMk2W3tKDA7WeoPiKLk9i2DcqwNiAeCbgFtNS1E3 D/03y3ryRn8KcZ7U0bo3WMxdHCmB9tG9qfIm/V0gyX7oGE/Hh+fOgbX3EkFod4UH WWLlkgIPTShr+Vylj7CyYA== Received: from ediex02.ad.cirrus.com ([84.19.233.68]) by mx0a-001ae601.pphosted.com (PPS) with ESMTPS id 4232uy5xfs-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 09 Oct 2024 07:48:17 -0500 (CDT) Received: from ediex01.ad.cirrus.com (198.61.84.80) by ediex02.ad.cirrus.com (198.61.84.81) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.1544.9; Wed, 9 Oct 2024 13:48:15 +0100 Received: from ediswmail9.ad.cirrus.com (198.61.86.93) by anon-ediex01.ad.cirrus.com (198.61.84.80) with Microsoft SMTP Server id 15.2.1544.9 via Frontend Transport; Wed, 9 Oct 2024 13:48:15 +0100 Received: from [198.90.208.18] (ediswws06.ad.cirrus.com [198.90.208.18]) by ediswmail9.ad.cirrus.com (Postfix) with ESMTP id 33C5E82024A; Wed, 9 Oct 2024 12:48:15 +0000 (UTC) Message-ID: <41a0ad69-912b-4eb3-84f7-fb385433c056@opensource.cirrus.com> Date: Wed, 9 Oct 2024 13:48:15 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH 00/51] treewide: Switch to __pm_runtime_put_autosuspend() To: "Rafael J. Wysocki" , Ulf Hansson , Laurent Pinchart , Sakari Ailus CC: , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , Andy Shevchenko References: <20241004094101.113349-1-sakari.ailus@linux.intel.com> <20241007184924.GH14766@pendragon.ideasonboard.com> <20241007222502.GG30699@pendragon.ideasonboard.com> Content-Language: en-GB From: Richard Fitzgerald In-Reply-To: Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit X-Proofpoint-GUID: ltWPFp1gnPUjzPmaRQ9EaXCq91PhMVjU X-Proofpoint-ORIG-GUID: ltWPFp1gnPUjzPmaRQ9EaXCq91PhMVjU X-Proofpoint-Spam-Reason: safe X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20241009_054835_021753_A188963B X-CRM114-Status: GOOD ( 22.19 ) X-BeenThere: linux-mediatek@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "Linux-mediatek" Errors-To: linux-mediatek-bounces+linux-mediatek=archiver.kernel.org@lists.infradead.org On 08/10/2024 7:24 pm, Rafael J. Wysocki wrote: > On Tue, Oct 8, 2024 at 12:35 AM Ulf Hansson wrote: >> >> On Tue, 8 Oct 2024 at 00:25, Laurent Pinchart >> wrote: >>> >>> Hi Ulf, >>> >>> On Tue, Oct 08, 2024 at 12:08:24AM +0200, Ulf Hansson wrote: >>>> On Mon, 7 Oct 2024 at 20:49, Laurent Pinchart wrote: >>>>> On Fri, Oct 04, 2024 at 04:38:36PM +0200, Ulf Hansson wrote: >>>>>> On Fri, 4 Oct 2024 at 11:41, Sakari Ailus wrote: >>>>>>> >>>>>>> Hello everyone, >>>>>>> >>>>>>> This set will switch the users of pm_runtime_put_autosuspend() to >>>>>>> __pm_runtime_put_autosuspend() while the former will soon be re-purposed >>>>>>> to include a call to pm_runtime_mark_last_busy(). The two are almost >>>>>>> always used together, apart from bugs which are likely common. Going >>>>>>> forward, most new users should be using pm_runtime_put_autosuspend(). >>>>>>> >>>>>>> Once this conversion is done and pm_runtime_put_autosuspend() re-purposed, >>>>>>> I'll post another set to merge the calls to __pm_runtime_put_autosuspend() >>>>>>> and pm_runtime_mark_last_busy(). >>>>>> >>>>>> That sounds like it could cause a lot of churns. >>>>>> >>>>>> Why not add a new helper function that does the >>>>>> pm_runtime_put_autosuspend() and the pm_runtime_mark_last_busy() >>>>>> things? Then we can start moving users over to this new interface, >>>>>> rather than having this intermediate step? >>>>> >>>>> I think the API would be nicer if we used the shortest and simplest >>>>> function names for the most common use cases. Following >>>>> pm_runtime_put_autosuspend() with pm_runtime_mark_last_busy() is that >>>>> most common use case. That's why I like Sakari's approach of repurposing >>>>> pm_runtime_put_autosuspend(), and introducing >>>>> __pm_runtime_put_autosuspend() for the odd cases where >>>>> pm_runtime_mark_last_busy() shouldn't be called. >>>> >>>> Okay, so the reason for this approach is because we couldn't find a >>>> short and descriptive name that could be used in favor of >>>> pm_runtime_put_autosuspend(). Let me throw some ideas at it and maybe >>>> you like it - or not. :-) >>> >>> I like the idea at least :-) >>> >>>> I don't know what options you guys discussed, but to me the entire >>>> "autosuspend"-suffix isn't really that necessary in my opinion. There >>>> are more ways than calling pm_runtime_put_autosuspend() that triggers >>>> us to use the RPM_AUTO flag for rpm_suspend(). For example, just >>>> calling pm_runtime_put() has the similar effect. >>> >>> To be honest, I'm lost there. pm_runtime_put() calls >>> __pm_runtime_idle(RPM_GET_PUT | RPM_ASYNC), while >>> pm_runtime_put_autosuspend() calls __pm_runtime_suspend(RPM_GET_PUT | >>> RPM_ASYNC | RPM_AUTO). >> >> __pm_runtime_idle() ends up calling rpm_idle(), which may call >> rpm_suspend() - if it succeeds to idle the device. In that case, it >> tags on the RPM_AUTO flag in the call to rpm_suspend(). Quite similar >> to what is happening when calling pm_runtime_put_autosuspend(). > > Right. > > For almost everybody, except for a small bunch of drivers that > actually have a .runtime_idle() callback, pm_runtime_put() is > literally equivalent to pm_runtime_put_autosuspend(). > > So really the question is why anyone who doesn't provide a > .runtime_idle() callback bothers with using this special > pm_runtime_put_autosuspend() thing, Because they are following the documentation? It says: "Drivers should call pm_runtime_mark_last_busy() to update this field after carrying out I/O, typically just before calling pm_runtime_put_autosuspend()." and "In order to use autosuspend, subsystems or drivers must call pm_runtime_use_autosuspend() (...), and thereafter they should use the various `*_autosuspend()` helper functions instead of the non# autosuspend counterparts" So the documentation says I should be using pm_runtime_put_autosuspend() instead of pm_runtime_put(). Seems unfair to criticise people for following the documentation.