Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Message-ID: <3e7b5acc-a3f8-09e2-7d83-bd0b58ba031a@amd.com>
Date:   Tue, 12 Oct 2021 16:10:16 -0500
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.2.0
Subject: Re: [PATCH] ACPI: scan: Release PM resources blocked by unused
 objects
Content-Language: en-US
To:     "Rafael J. Wysocki" <rjw@rjwysocki.net>,
        Linux ACPI <linux-acpi@vger.kernel.org>
Cc:     Linux PCI <linux-pci@vger.kernel.org>,
        Linux PM <linux-pm@vger.kernel.org>,
        LKML <linux-kernel@vger.kernel.org>
References: <5507158.DvuYhMxLoT@kreacher>
From:   "Limonciello, Mario" <mario.limonciello@amd.com>
In-Reply-To: <5507158.DvuYhMxLoT@kreacher>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?bWdENXBEejM5SHBlRko1RTRIQXdSMzhUU3FYVUFoSlpjai9URjgvZzFhTlBE?=
 =?utf-8?B?SG0yTFFlY0ttazEwYk9mZFNHY0o0SWJma0lyN016RENoazlsUWxOaDBEYm0y?=
 =?utf-8?B?eXAxM2hwVFhxanVpSG0yMCtqakt2QlB2SUVOYWYwcEYxV2ZqRXBwWUtwaElJ?=
 =?utf-8?B?NTA2dGNuWWRncTZMQTE5bWhFOUFFakViNVBzRm5obGZEeWt4TTE5MlYweVZV?=
 =?utf-8?B?OG5KNzNicWlTZUw5V2JVaERnM0ZncVJHdEJhSTdxUVAwanpaUGJBQVNYM0Ry?=
 =?utf-8?B?OHR2SkIwUndUTC9mc0dTVVlRbWZSellUMHFYYUszQ3d0b3lSbVcreTBQUzUz?=
 =?utf-8?B?UjR1YUU2UUhvZ1BvZmpIQnZPQnRwMDkxQ2gzQ1hOWEhiTTg5Nnk1Y0JtcHhG?=
 =?utf-8?B?N1l1eTZqb3RHUGpENFRmWHozSitFWk9nVThyWEpSRWRoVURRWXVvMityWDRQ?=
 =?utf-8?B?Y0tsMkRIbEd4b0xna3BIWTlueEZ6b2R6RGVqOWZDd1ppUDJsOGJKbERoSkcr?=
 =?utf-8?B?dXdxeEVGZFA1Z2UvR2R5emY1UmVZUU1LVFNqbWlRZ1lMMVE2V2Y5N3ZrYmlx?=
 =?utf-8?B?K0FLd0FKaDcyeHB2MCtiZEh3SWdYMGhuM0hJNXZPK2lTNDVFS1BINENCRWdJ?=
 =?utf-8?B?YUc5a0hndXdNNzZldEJuNHFvK3ZOalFjd2JLTTUwMlJ6YmdTL2NYRU1uWGVv?=
 =?utf-8?B?VkZjc2NqZGJiYlpBU2dadXhPS0RsY1YzcTY2dnlQdlFMdzFxVHVzalNsT3pl?=
 =?utf-8?B?UXA5MnpSa1lsbFpDKzArazRHZnM2TnU3RUdyUFI1SzEvbWg4TTBKUlNDMzVE?=
 =?utf-8?B?ZkhMMnpiNExTU0xoWDV2d1dLTTBOWEVJcmxtZy9SZ0tud1dtYzYyR2hGRis2?=
 =?utf-8?B?bC8rRlVodTM3c2JweVBRUE93WXRBMFNpRHRBQmZzSUpJSjBHRVpIQ1BwNElv?=
 =?utf-8?B?UEU5MVFGam1LVDkrOXVEazdNR0I4REZ4bDlsMXdRd1ZFOEJFRHdBcEYyVitU?=
 =?utf-8?B?RlZUVm00ZE9LY1ZFSXErMTRNVGNNNEx4SWhiQXhCV0tOd1VMWXl4MkY2Skpi?=
 =?utf-8?B?VU9qV0lUWFQ0bGJTZWx4L1NFYUFrNTR2NHZOam16cDFYRml3RmIxSVV3dkV6?=
 =?utf-8?B?MllRazZJUnIxb1BFRlBzUGc5R3hYYVcvRy9qTVRLNTcxWEY2Q3VXcytzKzFM?=
 =?utf-8?B?OWxUT1gwYTlJNlVLMWxmZWxKTXBBQm54cHozK0dmQXJzT3dCN21DaW9mOGRN?=
 =?utf-8?B?Z0JXeTVScHcvbTNmWGFaTVhQM3N1R2FnNG4rck9Ca3krYTRibWhFUHN3ZkIy?=
 =?utf-8?B?UWZHR3hYR0xJZ1AxNTRCaXlaMkZBMEU3RFViUitPS05NRnJGSXdsa2VoeE9O?=
 =?utf-8?B?Qy9XRUFGbzZ3M25ML2UraWU0VDduUUpGdTl6WnI1TFp3UkpNM3R0Y3VMTER5?=
 =?utf-8?B?QTV0aVcyTkRtMGFURTJlY1MvTjJPZHJTcTBzYmg5Q3NyQ0FlcEJta1BhZm5L?=
 =?utf-8?B?TVMxMTNhYlZYTFVvYURDUkVjL1dxQkM3Q3E4Ykpub2ZEUzBBdlNHRktrbzh1?=
 =?utf-8?B?cmhrcFlGdGlJQktiOVZsRVErUXQxbC9uWHJtUmhHYjNVVDdQL1V1RkZWV1p0?=
 =?utf-8?B?WHpUNGpoaEVWeUN0bm1HQzVsOGhKZjEramt6dFp6MVIzb1lWaXh0VUk0OEVo?=
 =?utf-8?B?SU5uUDRaTU5NK0RKakVaLy93MFRxaEQzK0lBMGhSbFZ4YjljWERUMmpIa3lx?=
 =?utf-8?Q?1BHXxwS0tt1xU8BBY/YjJxHftu/Y28NWZRyzHx1?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 4f05de9d-1ed7-44fe-60c3-08d98dc4b1ee
X-MS-Exchange-CrossTenant-AuthSource: SA0PR12MB4510.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 12 Oct 2021 21:10:19.0490
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 0fxLKgfq7eKIqARllxLs/1SveMpiegEMSjwCtNse1OD8zAEA9KyGDGu41NVNlCxsvd+TS4QGqWF1W+TmzxAOfA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN6PR12MB4670
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 10/9/2021 09:22, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> On some systems the ACPI namespace contains device objects that are
> not used in certain configurations of the system.  If they start off
> in the D0 power state configuration, they will stay in it until the
> system reboots, because of the lack of any mechanism possibly causing
> their configuration to change.  If that happens, they may prevent
> some power resources from being turned off or generally they may
> prevent the platform from getting into the deepest low-power states
> thus causing some energy to be wasted.
> 
> Address this issue by changing the configuration of unused ACPI
> device objects to the D3cold power state one after carrying out
> the ACPI-based enumeration of devices.
> 
> BugLink: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.kernel.org%2Fshow_bug.cgi%3Fid%3D214091&amp;data=04%7C01%7Cmario.limonciello%40amd.com%7C6e87f8a1465c424d721c08d98b303310%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637693861879301335%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=zCknM%2FyK8unP%2BI0fstH8OoB5UUVfwOKqeERZOVu9lQY%3D&amp;reserved=0
> Link: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Flinux-acpi%2F20211007205126.11769-1-mario.limonciello%40amd.com%2F&amp;data=04%7C01%7Cmario.limonciello%40amd.com%7C6e87f8a1465c424d721c08d98b303310%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637693861879301335%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=4yE1GvAineqku3yjQyfXv6nifmx1uqoEAAbf57eDiFY%3D&amp;reserved=0
> Reported-by: Mario Limonciello <mario.limonciello@amd.com>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Tested-by: Mario Limonciello <mario.limonciello@amd.com>

> ---
> 
> Mario,
> 
> I've changed acpi_dev_turn_off_if_unused() to check pnp.type.hardware_id
> instead of pnp.type.bus_address, because on some systems there are objects
> with both _ADR and _HID (which is against the spec) and the former is bogus.
> 
> Please test this one, even though for PCI devices there should be no difference.
> 

Yes, Thanks.  I had a try and this new version is also working correctly 
on the system this was found.

> Thanks!
> 
> ---
>   drivers/acpi/glue.c     |   25 +++++++++++++++++++++++++
>   drivers/acpi/internal.h |    1 +
>   drivers/acpi/scan.c     |    6 ++++++
>   3 files changed, 32 insertions(+)
> 
> Index: linux-pm/drivers/acpi/scan.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/scan.c
> +++ linux-pm/drivers/acpi/scan.c
> @@ -2559,6 +2559,12 @@ int __init acpi_scan_init(void)
>   		}
>   	}
>   
> +	/*
> +	 * Make sure that power management resources are not blocked by ACPI
> +	 * device objects with no users.
> +	 */
> +	bus_for_each_dev(&acpi_bus_type, NULL, NULL, acpi_dev_turn_off_if_unused);
> +
>   	acpi_turn_off_unused_power_resources();
>   
>   	acpi_scan_initialized = true;
> Index: linux-pm/drivers/acpi/glue.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/glue.c
> +++ linux-pm/drivers/acpi/glue.c
> @@ -350,3 +350,28 @@ void acpi_device_notify_remove(struct de
>   
>   	acpi_unbind_one(dev);
>   }
> +
> +int acpi_dev_turn_off_if_unused(struct device *dev, void *not_used)
> +{
> +	struct acpi_device *adev = to_acpi_device(dev);
> +
> +	/*
> +	 * Skip device objects with device IDs, because they may be in use even
> +	 * if they are not companions of any physical device objects.
> +	 */
> +	if (adev->pnp.type.hardware_id)
> +		return 0;
> +
> +	mutex_lock(&adev->physical_node_lock);
> +
> +	/*
> +	 * Device objects without device IDs are not in use if they have no
> +	 * corresponding physical device objects.
> +	 */
> +	if (list_empty(&adev->physical_node_list))
> +		acpi_device_set_power(adev, ACPI_STATE_D3_COLD);
> +
> +	mutex_unlock(&adev->physical_node_lock);
> +
> +	return 0;
> +}
> Index: linux-pm/drivers/acpi/internal.h
> ===================================================================
> --- linux-pm.orig/drivers/acpi/internal.h
> +++ linux-pm/drivers/acpi/internal.h
> @@ -117,6 +117,7 @@ bool acpi_device_is_battery(struct acpi_
>   bool acpi_device_is_first_physical_node(struct acpi_device *adev,
>   					const struct device *dev);
>   int acpi_bus_register_early_device(int type);
> +int acpi_dev_turn_off_if_unused(struct device *dev, void *not_used);
>   
>   /* --------------------------------------------------------------------------
>                        Device Matching and Notification
> 
> 
>