From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752212AbdLETlH (ORCPT ); Tue, 5 Dec 2017 14:41:07 -0500 Received: from mail.kernel.org ([198.145.29.99]:58274 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751746AbdLETkw (ORCPT ); Tue, 5 Dec 2017 14:40:52 -0500 DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org C60BD21878 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=helgaas@kernel.org Date: Tue, 5 Dec 2017 13:40:50 -0600 From: Bjorn Helgaas To: linux-pci@vger.kernel.org Cc: Kenji Chen , Thierry Reding , Manikanta Maddireddy , David Daney , Krishna Kishore , linux-kernel@vger.kernel.org, Vidya Sagar , Julia Lawall , linux-tegra@vger.kernel.org, Patrick Georgi , Rajat Jain , Yinghai Lu , Stefan Reinauer , Duncan Laurie Subject: Re: [PATCH v1 0/2] PCI/ASPM: Refine L1 PM Substates support Message-ID: <20171205194050.GD23510@bhelgaas-glaptop.roam.corp.google.com> References: <20171202211833.23146.5744.stgit@bhelgaas-glaptop.roam.corp.google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20171202211833.23146.5744.stgit@bhelgaas-glaptop.roam.corp.google.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sat, Dec 02, 2017 at 03:45:38PM -0600, Bjorn Helgaas wrote: > The PCIe active link power state is L0. ASPM defines two low-power > states: L0s and L1. The L1 PM Substates feature defines two > additional low-power states: L1.1 and L2.2. > > The L1.2 state may have substantial entry/exit latency. Downstream > devices can use the Latency Tolerance Reporting (LTR) feature to > report how much latency they can tolerate. The L1 PM Substates > capability includes a programmable L1.2 threshold register. If the > downstream devices can tolerate the latency indicated by the > threshold, L1.2 may be used. > > If LTR is not enabled, the L1.2 threshold is ignored and L1.2 can be > used whenever it is enabled and CLKREQ# is de-asserted. Linux > currently never enables LTR, but firmware may have done so. > > The current L1 PM substates support in Linux sets the L1.2 threshold > to a fixed value (163.84us) copied from coreboot [1]. I don't know > where that value came from, and it is incorrect for some platforms, > e.g., Tegra [2]. > > These patches change that so we calculate the L1.2 threshold based on > values reported by the hardware, and we enable LTR whenever possible. > > This is all just my understanding from reading the spec. I'd be glad > to be corrected if I'm going wrong. > > I'm particularly puzzled about the coreboot code in > pciexp_L1_substate_commit() that sets LTR_L1.2_THRESHOLD: > > + pcie_update_cfg(root, root_cap + 0x08, ~0xe3ff0000, > + (1 << 21) | (1 << 23) | (1 << 30)); > > This is writing the L1 PM Substates Control 1 register, but the shifts > don't match up with any of the fields in the register, so this is an > awfully funny way to write the threshold. > > [1] https://mail.coreboot.org/pipermail/coreboot-gerrit/2015-March/021134.html > [2] https://lkml.kernel.org/r/1510492674-12786-1-git-send-email-vidyas@nvidia.com > > --- > > Bjorn Helgaas (2): > PCI/ASPM: Calculate LTR_L1.2_THRESHOLD from device characteristics > PCI/ASPM: Enable Latency Tolerance Reporting when supported > > > drivers/pci/pcie/aspm.c | 71 +++++++++++++++++++++++++++++++---------------- > drivers/pci/probe.c | 33 ++++++++++++++++++++++ > include/linux/pci.h | 2 + > 3 files changed, 82 insertions(+), 24 deletions(-) I applied these with Vidya's Reviewed-by to pci/aspm for v4.16. I'd still really like to hear from the coreboot folks about the rationale for the current coreboot code. You folks are in a much better position to actually understand the hardware details. Bjorn