From patchwork Wed May 11 08:00:09 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Ulf Hansson X-Patchwork-Id: 9067011 Return-Path: X-Original-To: patchwork-dri-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 8D7709F372 for ; Wed, 11 May 2016 09:56:47 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 07D7220172 for ; Wed, 11 May 2016 09:56:46 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 4A834201C0 for ; Wed, 11 May 2016 09:56:44 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 62E668932A; Wed, 11 May 2016 09:56:39 +0000 (UTC) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mail-lf0-x22c.google.com (mail-lf0-x22c.google.com [IPv6:2a00:1450:4010:c07::22c]) by gabe.freedesktop.org (Postfix) with ESMTPS id 4E86A6E578 for ; Wed, 11 May 2016 08:00:40 +0000 (UTC) Received: by mail-lf0-x22c.google.com with SMTP id u64so40749139lff.3 for ; Wed, 11 May 2016 01:00:40 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=KFkK7MiOCwhkDIBNSOCQP0fPBFQOl2K5i6zQu6/J0f4=; b=Nzzg5KL6/tOkcQv2GYPNpldGou2tAIVdcFS9EZ9/ndE2yBwP2HvkuJfHz1EqtCUqwS D4NaSOHdu8+5XhO8AIIEH1r7RJ6HZ96zDqq46BoxWNmxH4XWO76KyOGF1Cl9PRkLvu6P S/7K7ycDFiHV/FR1TW2oUgNS4K+8gOy1FidjQ= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=KFkK7MiOCwhkDIBNSOCQP0fPBFQOl2K5i6zQu6/J0f4=; b=jbbZ2nTVbT9rRTu6SqPq7ayiRncvz4oyqzwYmvzEUNZVDVT8FtTsuhVyNSDA9uqSO1 i1SQ3OUg1014HGyEvqx9E1vM6++3+HccUwX1OBKtF/jl4xigC8eM9pUipIBGVSTaH1pv 3qRHlSc4usuJYHNmgWtyz7GbTXpuaiy/2sevTIp0/Jl6U7JJr8BEZDJ82JLn1hZXsyCz Wp6/B7bqG6Nn3yf+FQzYV4I5KkQBg3hkXM4nyA+BOZ8tXR8QuvGdZBz6/oAHiC3FrR/m TgN8+HbUZ4SBwIGyFtSEy4ib1wkJEmBFEGg4cK23oWxMM5vLN1BbxdiO5qHJdx97HJYI uQJA== X-Gm-Message-State: AOPr4FWXMOLn/aUUYbjaLYYKb0zOdg2utM3Ffr2kWOrAk617s+x1rxS+skEIoRT9vzGI7/Mn X-Received: by 10.112.154.5 with SMTP id vk5mr872506lbb.126.1462953638597; Wed, 11 May 2016 01:00:38 -0700 (PDT) Received: from localhost.localdomain (h-155-4-128-67.na.cust.bahnhof.se. [155.4.128.67]) by smtp.gmail.com with ESMTPSA id sg8sm1064894lbb.28.2016.05.11.01.00.36 (version=TLSv1/SSLv3 cipher=OTHER); Wed, 11 May 2016 01:00:37 -0700 (PDT) From: Ulf Hansson To: "Rafael J. Wysocki" , Kevin Hilman , Ulf Hansson , linux-pm@vger.kernel.org Subject: [PATCH v2 1/2] PM / Domains: Allow genpd to power on during the system PM phase Date: Wed, 11 May 2016 10:00:09 +0200 Message-Id: <1462953610-20824-2-git-send-email-ulf.hansson@linaro.org> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1462953610-20824-1-git-send-email-ulf.hansson@linaro.org> References: <1462953610-20824-1-git-send-email-ulf.hansson@linaro.org> MIME-Version: 1.0 X-Mailman-Approved-At: Wed, 11 May 2016 09:56:38 +0000 Cc: Len Brown , Maruthi Srinivas Bayyavarapu , Geert Uytterhoeven , Axel Haslam , dri-devel@lists.freedesktop.org, Jon Hunter , Alex Deucher , Laurent Pinchart , Pavel Machek , Andy Gross , =?UTF-8?q?Christian=20K=C3=B6nig?= , Lina Iyer , Marek Szyprowski X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Spam-Status: No, score=-5.5 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_MED,RP_MATCHES_RCVD,T_DKIM_INVALID,UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP If the PM domain is powered off when the first device in the domain starts its system PM prepare phase, genpd prevents any further attempts to power on the PM domain during the system PM phase. This constraint affects not only the current device which is being prepared, but all devices within the same PM domain. This behaviour needs to be changed, as a subsystem/driver for a device in the same PM domain may still need to be able to serve requests in the system PM phase. Accordingly, it may need to runtime resume its device and thus also request the corresponding PM domain to be powered on. To deal with these scenarios, let's make the device operational in the system PM prepare phase by runtime resuming it, no matter if the PM domain is powered on or off. Changing this also enables us to remove genpd's suspend_power_off flag, as it's being used to track this condition. Additionally, we must allow the PM domain to be powered on via runtime PM during the system PM phase. This change also requires a fix in the AMD ACP (Audio CoProcessor) drm driver. It registers a genpd to model the ACP as a PM domain, but unfortunately it's also abuses genpd's "internal" suspend_power_off flag to deal with a corner case at system PM resume. More precisely, the so called SMU block powers on the ACP at system PM resume, unconditionally if it's being used or not. This may lead to that genpd's internal status of the power state, may not correctly reflect the power state of the HW after a system PM resume. Because of changing the behaviour of genpd, by runtime resuming devices in the prepare phase, the AMD ACP drm driver no longer have to deal with this corner case. So let's just drop the related code in this driver. Cc: David Airlie Cc: Alex Deucher Cc: Christian König Cc: Maruthi Srinivas Bayyavarapu Cc: dri-devel@lists.freedesktop.org Signed-off-by: Ulf Hansson --- Changes in v2: - Updated changelog. - Added a fix in the AMD ACP (Audio CoProcessor) drm driver, which registers a genpd. The fix removes the usage of genpd's internal suspend_power_off flag as it's not needed after this change. Because of this change I am also requesting an ack from the drm driver maintainer. --- drivers/base/power/domain.c | 84 ++++++++++++--------------------- drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c | 23 --------- include/linux/pm_domain.h | 1 - 3 files changed, 31 insertions(+), 77 deletions(-) diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index de23b64..a608f52 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -187,8 +187,7 @@ static int genpd_poweron(struct generic_pm_domain *genpd, unsigned int depth) struct gpd_link *link; int ret = 0; - if (genpd->status == GPD_STATE_ACTIVE - || (genpd->prepared_count > 0 && genpd->suspend_power_off)) + if (genpd->status == GPD_STATE_ACTIVE) return 0; /* @@ -735,21 +734,22 @@ static int pm_genpd_prepare(struct device *dev) mutex_lock(&genpd->lock); - if (genpd->prepared_count++ == 0) { + if (genpd->prepared_count++ == 0) genpd->suspended_count = 0; - genpd->suspend_power_off = genpd->status == GPD_STATE_POWER_OFF; - } mutex_unlock(&genpd->lock); - if (genpd->suspend_power_off) - return 0; - /* - * The PM domain must be in the GPD_STATE_ACTIVE state at this point, - * so genpd_poweron() will return immediately, but if the device - * is suspended (e.g. it's been stopped by genpd_stop_dev()), we need - * to make it operational. + * Even if the PM domain is powered off at this point, we can't expect + * it to remain in that state during the entire system PM suspend + * phase. Any subsystem/driver for a device in the PM domain, may still + * need to serve a request which may require the device to be runtime + * resumed and its PM domain to be powered. + * + * As we are disabling runtime PM at this point, we are preventing the + * subsystem/driver to decide themselves. For that reason, we need to + * make sure the device is operational as it may be required in some + * cases. */ pm_runtime_resume(dev); __pm_runtime_disable(dev, false); @@ -758,8 +758,7 @@ static int pm_genpd_prepare(struct device *dev) if (ret) { mutex_lock(&genpd->lock); - if (--genpd->prepared_count == 0) - genpd->suspend_power_off = false; + genpd->prepared_count--; mutex_unlock(&genpd->lock); pm_runtime_enable(dev); @@ -786,7 +785,7 @@ static int pm_genpd_suspend(struct device *dev) if (IS_ERR(genpd)) return -EINVAL; - return genpd->suspend_power_off ? 0 : pm_generic_suspend(dev); + return pm_generic_suspend(dev); } /** @@ -807,7 +806,7 @@ static int pm_genpd_suspend_late(struct device *dev) if (IS_ERR(genpd)) return -EINVAL; - return genpd->suspend_power_off ? 0 : pm_generic_suspend_late(dev); + return pm_generic_suspend_late(dev); } /** @@ -827,8 +826,7 @@ static int pm_genpd_suspend_noirq(struct device *dev) if (IS_ERR(genpd)) return -EINVAL; - if (genpd->suspend_power_off - || (dev->power.wakeup_path && genpd_dev_active_wakeup(genpd, dev))) + if (dev->power.wakeup_path && genpd_dev_active_wakeup(genpd, dev)) return 0; genpd_stop_dev(genpd, dev); @@ -860,8 +858,7 @@ static int pm_genpd_resume_noirq(struct device *dev) if (IS_ERR(genpd)) return -EINVAL; - if (genpd->suspend_power_off - || (dev->power.wakeup_path && genpd_dev_active_wakeup(genpd, dev))) + if (dev->power.wakeup_path && genpd_dev_active_wakeup(genpd, dev)) return 0; /* @@ -894,7 +891,7 @@ static int pm_genpd_resume_early(struct device *dev) if (IS_ERR(genpd)) return -EINVAL; - return genpd->suspend_power_off ? 0 : pm_generic_resume_early(dev); + return pm_generic_resume_early(dev); } /** @@ -915,7 +912,7 @@ static int pm_genpd_resume(struct device *dev) if (IS_ERR(genpd)) return -EINVAL; - return genpd->suspend_power_off ? 0 : pm_generic_resume(dev); + return pm_generic_resume(dev); } /** @@ -936,7 +933,7 @@ static int pm_genpd_freeze(struct device *dev) if (IS_ERR(genpd)) return -EINVAL; - return genpd->suspend_power_off ? 0 : pm_generic_freeze(dev); + return pm_generic_freeze(dev); } /** @@ -958,7 +955,7 @@ static int pm_genpd_freeze_late(struct device *dev) if (IS_ERR(genpd)) return -EINVAL; - return genpd->suspend_power_off ? 0 : pm_generic_freeze_late(dev); + return pm_generic_freeze_late(dev); } /** @@ -980,7 +977,7 @@ static int pm_genpd_freeze_noirq(struct device *dev) if (IS_ERR(genpd)) return -EINVAL; - return genpd->suspend_power_off ? 0 : genpd_stop_dev(genpd, dev); + return genpd_stop_dev(genpd, dev); } /** @@ -1000,8 +997,7 @@ static int pm_genpd_thaw_noirq(struct device *dev) if (IS_ERR(genpd)) return -EINVAL; - return genpd->suspend_power_off ? - 0 : genpd_start_dev(genpd, dev); + return genpd_start_dev(genpd, dev); } /** @@ -1023,7 +1019,7 @@ static int pm_genpd_thaw_early(struct device *dev) if (IS_ERR(genpd)) return -EINVAL; - return genpd->suspend_power_off ? 0 : pm_generic_thaw_early(dev); + return pm_generic_thaw_early(dev); } /** @@ -1044,7 +1040,7 @@ static int pm_genpd_thaw(struct device *dev) if (IS_ERR(genpd)) return -EINVAL; - return genpd->suspend_power_off ? 0 : pm_generic_thaw(dev); + return pm_generic_thaw(dev); } /** @@ -1072,26 +1068,13 @@ static int pm_genpd_restore_noirq(struct device *dev) * At this point suspended_count == 0 means we are being run for the * first time for the given domain in the present cycle. */ - if (genpd->suspended_count++ == 0) { + if (genpd->suspended_count++ == 0) /* * The boot kernel might put the domain into arbitrary state, * so make it appear as powered off to pm_genpd_sync_poweron(), * so that it tries to power it on in case it was really off. */ genpd->status = GPD_STATE_POWER_OFF; - if (genpd->suspend_power_off) { - /* - * If the domain was off before the hibernation, make - * sure it will be off going forward. - */ - genpd_power_off(genpd, true); - - return 0; - } - } - - if (genpd->suspend_power_off) - return 0; pm_genpd_sync_poweron(genpd, true); @@ -1110,7 +1093,6 @@ static int pm_genpd_restore_noirq(struct device *dev) static void pm_genpd_complete(struct device *dev) { struct generic_pm_domain *genpd; - bool run_complete; dev_dbg(dev, "%s()\n", __func__); @@ -1120,18 +1102,14 @@ static void pm_genpd_complete(struct device *dev) mutex_lock(&genpd->lock); - run_complete = !genpd->suspend_power_off; - if (--genpd->prepared_count == 0) - genpd->suspend_power_off = false; + genpd->prepared_count--; mutex_unlock(&genpd->lock); - if (run_complete) { - pm_generic_complete(dev); - pm_runtime_set_active(dev); - pm_runtime_enable(dev); - pm_request_idle(dev); - } + pm_generic_complete(dev); + pm_runtime_set_active(dev); + pm_runtime_enable(dev); + pm_request_idle(dev); } /** diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c index b7b583c..a905f1f 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c @@ -421,29 +421,6 @@ static int acp_suspend(void *handle) static int acp_resume(void *handle) { - int i, ret; - struct acp_pm_domain *apd; - struct amdgpu_device *adev = (struct amdgpu_device *)handle; - - /* return early if no ACP */ - if (!adev->acp.acp_genpd) - return 0; - - /* SMU block will power on ACP irrespective of ACP runtime status. - * Power off explicitly based on genpd ACP runtime status so that ACP - * hw and ACP-genpd status are in sync. - * 'suspend_power_off' represents "Power status before system suspend" - */ - if (adev->acp.acp_genpd->gpd.suspend_power_off == true) { - apd = container_of(&adev->acp.acp_genpd->gpd, - struct acp_pm_domain, gpd); - - for (i = 4; i >= 0 ; i--) { - ret = acp_suspend_tile(apd->cgs_dev, ACP_TILE_P1 + i); - if (ret) - pr_err("ACP tile %d tile suspend failed\n", i); - } - } return 0; } diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h index 39285c7..dd5b044 100644 --- a/include/linux/pm_domain.h +++ b/include/linux/pm_domain.h @@ -57,7 +57,6 @@ struct generic_pm_domain { unsigned int device_count; /* Number of devices */ unsigned int suspended_count; /* System suspend device counter */ unsigned int prepared_count; /* Suspend counter of prepared devices */ - bool suspend_power_off; /* Power status before system suspend */ int (*power_off)(struct generic_pm_domain *domain); int (*power_on)(struct generic_pm_domain *domain); struct gpd_dev_ops dev_ops;