From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pf1-f182.google.com (mail-pf1-f182.google.com [209.85.210.182]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 6018B86348 for ; Tue, 3 Feb 2026 23:19:59 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.210.182 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1770160801; cv=none; b=kyLKSahJDfrIGQ9o37XuVvi2VFhfJSmVoRbf6jv/gDUceo+ZoMHR65+BWHGFABuXKkNnDE9DK0smVqJ+agGj6LFQm5mB2KNmFtf6F9l3krMJyxPhnhyiUzkUuQfFjk61XbrdHTVlocdgKniL1KEivSnr+vPhwBsyD39Zun9/iQ8= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1770160801; c=relaxed/simple; bh=G81U0UDHby4RrLSchNdEF9/UbbKHjneIt8BYzC8lVwE=; h=From:To:Cc:Subject:In-Reply-To:References:Date:Message-ID: MIME-Version:Content-Type; b=X4qOZQruGUf4U63Od6WagyDCzAnw0xH1JFyqfmKqFJjv+hZLQsvAJWy4fel/lWtzU5sZ1lSDALAl+HMPtNgUnbQ5/DxPZDEsVmomzqW5LP8Uo9e+slRCYCl1/eH1D+ifkUqEVEgU6o3T7dz5YEy+XMFFtz2ttvcuTXT9oap/3Pk= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=baylibre.com; spf=pass smtp.mailfrom=baylibre.com; dkim=pass (2048-bit key) header.d=baylibre-com.20230601.gappssmtp.com header.i=@baylibre-com.20230601.gappssmtp.com header.b=gMSMm8Xl; arc=none smtp.client-ip=209.85.210.182 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=baylibre.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=baylibre.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=baylibre-com.20230601.gappssmtp.com header.i=@baylibre-com.20230601.gappssmtp.com header.b="gMSMm8Xl" Received: by mail-pf1-f182.google.com with SMTP id d2e1a72fcca58-81e9d0cd082so4869353b3a.0 for ; Tue, 03 Feb 2026 15:19:59 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=baylibre-com.20230601.gappssmtp.com; s=20230601; t=1770160799; x=1770765599; darn=vger.kernel.org; h=mime-version:message-id:date:references:in-reply-to:subject:cc:to :from:from:to:cc:subject:date:message-id:reply-to; bh=Jm9DxbIWuku09f2xOOYS1JETDqxEFrbJpaIM7ZnpNuQ=; b=gMSMm8XlJisiE4sMaLxNss5ense9pzIpKEDgT88qjSqzZd7jnEvACvbHEcBHnMl6WK RMfaeWCOnWYhsW/kRerPlcPK5HUUFt/s7XVInX8h5z53Ji31PnLHK9x2D6NMJKlEylCu gJ28ZRWyc+BlFE+9QYVxKte1ZX+Br/Jukf6yVT3GVMMa7C6KpvOx2wvoVDh2ciyVTg+U M1MVCjwOIprtfKspSLm6nUDqgiQLlydQfidiA6HXDA3Vb3iGgr0ebT2eGupSXIhoewbn /dsLyiGsIN5pDyRII/DstcrD7Epstd7kBCTHAkHfQmTzQYQrRXigtYAr0ddYAttXuvcw eh2w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1770160799; x=1770765599; h=mime-version:message-id:date:references:in-reply-to:subject:cc:to :from:x-gm-gg:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=Jm9DxbIWuku09f2xOOYS1JETDqxEFrbJpaIM7ZnpNuQ=; b=rXiAK2uGiypCy9/iQ30NHetrd58b5PtRVkPcgCH57j30Nq9ULuyB46gIscHGTFwHPP SnEPCxIocKxvy4gAe7KoyQ/3up8838Hw+b//LysdqV53E6AvGs8yZbnmZf2ENFlS3Eun FZMb7atoGcOW1XN+iqKA8m6L8Ixv8SxBVdtwA70puH/WsGN+PWzZCFX5U0kgSpkc3UWj NPnLbu4vg0lqtQxhewjs/UPl3bn/dod3GTtuhzEJstZzYgTGc461ZOTWRWn4SmnItoqU 5qXOAtG+xBhC3BvyvqkB+BVY42i4zAjv5o5u91roV25KK+z7pScnEi6KpqI3ogJpzfIB y3HA== X-Forwarded-Encrypted: i=1; AJvYcCVgEnRkPPE7ull9wwRGYli3mUVXOvzSyT7GxUwt0jw3gtmUmaaKejZngYGZn+VKz5oeFR1cTlIAnf0KDWA=@vger.kernel.org X-Gm-Message-State: AOJu0Yx34OP3RGbYtwxYvyQvjoty+Ek9A4XA3f5W8Tx9KVs/Sy9VvW/q tHU/YGQsRlalPigoDEBC5BCY5bVQWl4prciRLz0h9cYz48QGyq974A7C2RDRjASEZQ4= X-Gm-Gg: AZuq6aKzyFYvY124Yuh10tw4eKaRTYov0ioRWiPubgrPL8Y4XbOpnvveCbMJ0u7LhZ3 he47ELTn/fvUocgv3BHqZXBPnWwkCl/WsCoWYTu3uyJL45ZKfoeMBT0MA4k/Ek1POn/44iATxIQ PBQKUzKCNDET5eDHWV5SkNCZXMZIlmDfR0xcNRfk39NOXxritKidStw55LdpwlJKRMwAJZkAcXf UMDouZjJi0RtZYGNfQ+6+Ll7YGyweKlsft1MXq4CZ+1ztbShcohLDipjyGFD56ny9+DJySoH2gd Yc4ESBeIf8/FSYBiPc2WGSu7cyPx9V/NfB4BDwSR4QI1O7rtKQYOn8KNhPwjO7E3Npjutppt+a3 srR5W5Btr53VDSiYtErCClnZ3uTEGb1R+rUmnfIMNgZkvyLGQsdBup74uiBoqxUbn6kGbNRTznO p5XLthyklPLBGlnqPSPdo= X-Received: by 2002:a05:6a21:618b:b0:393:38a3:8975 with SMTP id adf61e73a8af0-393725d9c10mr968502637.70.1770160798635; Tue, 03 Feb 2026 15:19:58 -0800 (PST) Received: from localhost ([71.212.200.220]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-2a933851685sm4579565ad.13.2026.02.03.15.19.57 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 03 Feb 2026 15:19:58 -0800 (PST) From: Kevin Hilman To: Ulf Hansson Cc: "Rafael J. Wysocki" , linux-pm@vger.kernel.org, Dhruva Gole , linux-kernel@vger.kernel.org Subject: Re: [PATCH 2/2] pmdommain: add support system-wide resume latency constraints In-Reply-To: References: <20260120-topic-lpm-pmdomain-device-constraints-v1-0-108fc4cfafce@baylibre.com> <20260120-topic-lpm-pmdomain-device-constraints-v1-2-108fc4cfafce@baylibre.com> Date: Tue, 03 Feb 2026 15:19:57 -0800 Message-ID: <7hbji5za76.fsf@baylibre.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain Ulf Hansson writes: > On Wed, 21 Jan 2026 at 02:54, Kevin Hilman (TI) wrote: >> >> In addition to checking for CPU latency constraints when checking if >> OK to power down a domain, also check for QoS latency constraints in >> all devices of a domain and use that in determining the final latency >> constraint to use for the domain. >> >> Since cpu_system_power_down_ok() is used for system-wide suspend, the >> per-device constratints are only relevant if the LATENCY_SYS QoS flag >> is set. >> >> Because this flag implies the latency constraint only applies to >> system-wide suspend, also check the flag in >> dev_update_qos_constraint(). If it is set, then the constraint is not >> relevant for runtime PM decisions. >> >> Signed-off-by: Kevin Hilman (TI) >> --- >> drivers/pmdomain/governor.c | 42 +++++++++++++++++++++++++++++++++++++++++- >> 1 file changed, 41 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/pmdomain/governor.c b/drivers/pmdomain/governor.c >> index 96737abbb496..03802a859a78 100644 >> --- a/drivers/pmdomain/governor.c >> +++ b/drivers/pmdomain/governor.c >> @@ -31,6 +31,8 @@ static int dev_update_qos_constraint(struct device *dev, void *data) >> constraint_ns = td ? td->effective_constraint_ns : >> PM_QOS_RESUME_LATENCY_NO_CONSTRAINT_NS; >> } else { >> + enum pm_qos_flags_status flag_status; >> + >> /* >> * The child is not in a domain and there's no info on its >> * suspend/resume latencies, so assume them to be negligible and >> @@ -38,7 +40,14 @@ static int dev_update_qos_constraint(struct device *dev, void *data) >> * known at this point anyway). >> */ >> constraint_ns = dev_pm_qos_read_value(dev, DEV_PM_QOS_RESUME_LATENCY); >> - constraint_ns *= NSEC_PER_USEC; >> + flag_status = dev_pm_qos_flags(dev, PM_QOS_FLAG_LATENCY_SYS); >> + if ((constraint_ns != PM_QOS_RESUME_LATENCY_NO_CONSTRAINT) && >> + (flag_status == PM_QOS_FLAGS_ALL)) { >> + dev_dbg_once(dev, "resume-latency only for system-wide. Ignoring.\n"); >> + constraint_ns = PM_QOS_RESUME_LATENCY_NO_CONSTRAINT_NS; >> + } else { >> + constraint_ns *= NSEC_PER_USEC; >> + } >> } > > dev_update_qos_constraint() is called only to take into account the > QoS constraints for the device's *children*. > > It looks like we should also be checking the PM_QOS_FLAG_LATENCY_SYS > flag in default_suspend_ok() for the device in question. > > That said, there seems to be more places in the kernel where we should > check the PM_QOS_FLAG_LATENCY_SYS flag, like in cpu_power_down_ok(), > cpuidle_governor_latency_req(), etc. OK. But now that we've agreed to drop the userspace interface for this, I wonder if the better approach is now to consider the flag to mean that the latency applies to runtime PM *and* system-wide PM. Then, without the flag set, the latency applies *only* to runtime PM. That approach would allow the current default behavior to stay the same, and not require adding checks for this flag throughout the runtime code, and only require checking for the flag in the system-wide PM paths. >> if (constraint_ns < *constraint_ns_p) >> @@ -430,12 +439,43 @@ static bool cpu_system_power_down_ok(struct dev_pm_domain *pd) >> s64 constraint_ns = cpu_wakeup_latency_qos_limit() * NSEC_PER_USEC; >> struct generic_pm_domain *genpd = pd_to_genpd(pd); >> int state_idx = genpd->state_count - 1; >> + struct pm_domain_data *pdd; >> + s32 min_dev_latency = PM_QOS_RESUME_LATENCY_NO_CONSTRAINT; >> + s64 min_dev_latency_ns = PM_QOS_RESUME_LATENCY_NO_CONSTRAINT_NS; >> + struct gpd_link *link; >> >> if (!(genpd->flags & GENPD_FLAG_CPU_DOMAIN)) { >> genpd->state_idx = state_idx; >> return true; >> } >> >> + list_for_each_entry(link, &genpd->parent_links, parent_node) { >> + struct generic_pm_domain *child_pd = link->child; >> + >> + list_for_each_entry(pdd, &child_pd->dev_list, list_node) { >> + enum pm_qos_flags_status flag_status; >> + s32 dev_latency; >> + >> + dev_latency = dev_pm_qos_read_value(pdd->dev, DEV_PM_QOS_RESUME_LATENCY); >> + flag_status = dev_pm_qos_flags(pdd->dev, PM_QOS_FLAG_LATENCY_SYS); >> + if ((dev_latency != PM_QOS_RESUME_LATENCY_NO_CONSTRAINT) && >> + (flag_status == PM_QOS_FLAGS_ALL)) { >> + dev_dbg(pdd->dev, >> + "in domain %s, has QoS system-wide resume latency=%d\n", >> + child_pd->name, dev_latency); >> + if (dev_latency < min_dev_latency) >> + min_dev_latency = dev_latency; >> + } >> + } > > cpu_system_power_down_ok() is at the moment only used for CPU PM > domains. If the intent is to take into account QoS constraints for > CPUs, we should check the QoS value for CPU-devices as well (by using > get_cpu_device(), see cpu_power_down_ok(). For non-CPU devices > something along the lines of the above makes sense to me. > > Although, please note, the above code is just walking through the > devices in the child-domains, there is nothing checking the devices > that belong to the current/parent-domain. Oops, yeah. Good catch. > Nor are we taking child-devices into account. Indeed... double oops. This makes me wonder if we have any helpers to iterate over every device (and children) in a domain (and subdomains.) Kevin