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 9C8A8E7543D for ; Tue, 3 Oct 2023 09:46:22 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=iz7IB1i3pLrpXWFDbM7kCV6zTnQui/8dcMByidTFcrs=; b=M5ob66eFuh+fmJ FATztoPPdFjzX9yf/VDm57w24uqDo1WgXwVLgKDHD190sua06lpFQvzk21q0MQQv3HiUfj77rFLH6 OZ1e3Mie2xHUnnA5wriXl0RMxfG5dLyhzKMxgw3aNUHAAFaJk9kxTdfiY5AJKu46Exd5EW0F00Od4 XcmGuaHIaATHtqlGR1QOm0UTKPnNY6ZN/UBMvlPNPsO5LYmpFzPzuhy54/2iRw8wOZj5iqJAiKqhP JPcRR+AjUSU+Ov90ZfjEAmOsydfPha9uvyZe8Qw+A3mgf8Q6mQqVmzv4RF1iYeZsdm1AYyNzvIWwR Z+ujJX9VcANwK6HSyZng==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1qnbyR-00EFyB-1W; Tue, 03 Oct 2023 09:45:51 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1qnbyP-00EFxs-0H for linux-arm-kernel@lists.infradead.org; Tue, 03 Oct 2023 09:45:50 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 2B7AFC15; Tue, 3 Oct 2023 02:46:25 -0700 (PDT) Received: from bogus (e103737-lin.cambridge.arm.com [10.1.197.49]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 7FB2D3F5A1; Tue, 3 Oct 2023 02:45:45 -0700 (PDT) Date: Tue, 3 Oct 2023 10:45:43 +0100 From: Sudeep Holla To: "Pawandeep Oza (QUIC)" Cc: 'Will Deacon' , Sudeep Holla , "catalin.marinas@arm.com" , "rafael@kernel.org" , "lenb@kernel.org" , "linux-arm-kernel@lists.infradead.org" , "linux-kernel@vger.kernel.org" , "linux-acpi@vger.kernel.org" Subject: Re: [PATCH v8] cpuidle, ACPI: Evaluate LPI arch_flags for broadcast timer Message-ID: <20231003094543.oinq2onehefxdrbw@bogus> References: <20230918172140.2825357-1-quic_poza@quicinc.com> <20230929150459.GA30623@willie-the-truck> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20231003_024549_237419_658152EF X-CRM114-Status: GOOD ( 30.53 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Mon, Oct 02, 2023 at 07:22:57PM +0000, Pawandeep Oza (QUIC) wrote: > > > -----Original Message----- > From: Will Deacon > Sent: Friday, September 29, 2023 8:05 AM > To: Pawandeep Oza (QUIC) > Cc: sudeep.holla@arm.com; catalin.marinas@arm.com; rafael@kernel.org; lenb@kernel.org; linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; linux-acpi@vger.kernel.org > Subject: Re: [PATCH v8] cpuidle, ACPI: Evaluate LPI arch_flags for broadcast timer > > On Mon, Sep 18, 2023 at 10:21:40AM -0700, Oza Pawandeep wrote: > > Arm(r) Functional Fixed Hardware Specification defines LPI states, which > > provide an architectural context loss flags field that can be used to > > describe the context that might be lost when an LPI state is entered. > > > > - Core context Lost > > - General purpose registers. > > - Floating point and SIMD registers. > > - System registers, include the System register based > > - generic timer for the core. > > - Debug register in the core power domain. > > - PMU registers in the core power domain. > > - Trace register in the core power domain. > > - Trace context loss > > - GICR > > - GICD > > > > Qualcomm's custom CPUs preserves the architectural state, including > > keeping the power domain for local timers active. > > when core is power gated, the local timers are sufficient to wake the > > core up without needing broadcast timer. > > > > The patch fixes the evaluation of cpuidle arch_flags, and moves only > > to broadcast timer if core context lost is defined in ACPI LPI. > > > > Fixes: a36a7fecfe607 ("Add support for Low Power Idle(LPI) states") > > Reviewed-by: Sudeep Holla > > Acked-by: Rafael J. Wysocki > > Signed-off-by: Oza Pawandeep > > --- > > diff --git a/drivers/acpi/processor_idle.c > > b/drivers/acpi/processor_idle.c index dc615ef6550a..5c1d13eecdd1 > > 100644 > > --- a/drivers/acpi/processor_idle.c > > +++ b/drivers/acpi/processor_idle.c > > @@ -1217,8 +1217,7 @@ static int acpi_processor_setup_lpi_states(struct acpi_processor *pr) > > strscpy(state->desc, lpi->desc, CPUIDLE_DESC_LEN); > > state->exit_latency = lpi->wake_latency; > > state->target_residency = lpi->min_residency; > > - if (lpi->arch_flags) > > - state->flags |= CPUIDLE_FLAG_TIMER_STOP; > > + arch_update_idle_state_flags(lpi->arch_flags, &state->flags); > > Hmm, I know Rafael has Acked this, but I think this is pretending to be more > generic than it really is. While passing in a pointer to the flags field > allows the arch code to set and clear arbitrary flags, we're calling this > before we've set CPUIDLE_FLAG_RCU_IDLE, so that cannot be changed. > > Why not just name it like it is and return the arch flags directly: > > state->flags |= arch_get_idle_state_flags(lpi->arch_flags); > > Oza: > > ? Not sure if this "?" is by mistake or you didn't follow Will's comment. The point made was that it is cleaner for arch code to just provide the flags that needs to be set via some helper like 'arch_get_idle_state_flags()' rather than set/update itself via 'arch_update_idle_state_flags()' like you have in this patch. I completely agree with Will as this is much cleaner and arch code just returns the requested flags and the core code is still in charge to update the flags. -- Regards, Sudeep _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel