Re: [PATCH v5] PCI: Allow PCI bridges to go to D3Hot on all Devicetree based platforms

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Feb 28, 2025 at 10:39:57AM -0800, Brian Norris wrote:
> Hi Bjorn,
> 
> On Fri, Feb 28, 2025 at 11:45:09AM -0600, Bjorn Helgaas wrote:
> > On Tue, Nov 26, 2024 at 03:17:11PM -0800, Brian Norris wrote:
> > > From: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxx>
> > > 
> > > Unlike ACPI based platforms, there are no known issues with D3Hot for
> > > the PCI bridges in Device Tree based platforms. 
> > 
> > Can we elaborate on this a little bit?  Referring to "known issues
> > with ACPI-based platforms" depends on a lot of domain-specific history
> > that most readers (including me) don't know.
> 
> Well, to me, the background here is simply the surrounding code context,
> and the past discussions that I linked:
> 
> https://lore.kernel.org/linux-pci/20240227225442.GA249898@bhelgaas/
> 
> The whole reason we need this patch is that:
> (a) there's some vaguely specified reason this function (which prevents
>     standard-specified behavior) exists; and
> (b) that function includes a condition that allows all systems with a
>     DMI/BIOS newer than year 2015 to use this feature.
> 
> Digging a bit further, it seems like maybe the only reason this feature
> is prevented on DT systems is from commit ("9d26d3a8f1b0 PCI: Put PCIe
> ports into D3 during suspend"), where the subtext is that it was written
> by and for Intel in 2016, with an arbitrary time-based cutoff ("year
> this was being developed") that only works for DMI systems. DT systems
> do not tend to support DMI.
> 

I would say 'Majority of the DT systems' since there are exceptions like Qcom
compute platforms where developers put devicetree in an ACPI based laptop with a
BIOS/DMI (for a reason).

> If any of this is what you're looking for, I can try to
> copy/paste/summarize a few more of those bits, if it helps.
> 
> > I don't think "ACPI-based" or "devicetree-based" are good
> > justifications for changing the behavior because they don't identify
> > any specific reasons.  It's like saying "we can enable this feature
> > because the platform spec is written in French."
> 
> AIUI, It's involved because of the general strategy of this function
> (per its comments, "recent enough PCIe ports"). So far, it sounds like
> that reason (presumably, old BIOS with poor power management code)
> doesn't really apply to a system based on device tree, where the power
> management code is mostly/entirely in the OS.
> 
> But really, the original commit doesn't actually state reasons, so maybe
> the "known issues" phrasing could be weakened a bit, to avoid implying
> there were any stated reasons.
> 

Right. I guess the commit tried to be less invasive so the author decided to
limit it to DMI based systems. I couldn't think of any other reasons.

> > > Past discussions (Link [1]) determined the restrictions around D3
> > > should be relaxed for all Device Tree systems. 
> > 
> > This is far too generic a statement for me to sign up to, especially
> > since "all Device Tree systems" doesn't say anything at all about how
> > any particular hardware works or what behavior we're relying on.
> > 
> > We need to say something about what D3hot means (i.e., only message
> > and type 0 config requests accepted) and that we know anything below
> > the bridge is inaccessible in D3hot and why that's OK.  E.g., maybe we
> > only care about wakeup requests and we know those still work with the
> > bridge in D3hot because XYZ.
> 

This patch's main motive is to enable D3hot for bridges that was disabled for no
good reasons, other than the one mentioned above. Maybe adding a bit more about
bridge D3hot handling could be of help, but your comment sounds like you are
questioning why allowing D3hot is OK? But we already enable it for newer DMI
based systems. I think the question should be asked is, why PCI core decided to
not allow D3hot for DT based platforms without any user reported issues.

- Mani

-- 
மணிவண்ணன் சதாசிவம்




[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux