Re: [PATCH v2] PCI/portdrv: Do not require an interrupt for all AER capable ports

[Mika Westerberg <mika.westerberg@linux.intel.com>]

Hi,

On Thu, Dec 08, 2022 at 06:23:49AM -0600, Bjorn Helgaas wrote:
> On Thu, Dec 08, 2022 at 07:58:42AM +0200, Mika Westerberg wrote:
> > On Wed, Dec 07, 2022 at 04:35:37PM -0600, Bjorn Helgaas wrote:
> > > On Wed, Dec 07, 2022 at 10:41:05AM +0200, Mika Westerberg wrote:
> > > > Only Root Ports and Event Collectors use MSI for AER. PCIe Switch ports
> > > > or endpoints on the other hand only send messages (that get collected by
> > > > the former). For this reason do not require PCIe switch ports and
> > > > endpoints to use interrupt if they support AER.
> > > > 
> > > > This allows portdrv to attach PCIe switch ports of Intel DG1 and DG2
> > > > discrete graphics cards. These do not declare MSI or legacy interrupts.
> > > 
> > > Help me understand more about this situation.  I guess we want portdrv
> > > to attach not to a GPU itself, but to a switch port on the card that
> > > *leads* to the GPU?
> > 
> > Yes correct.
> > 
> > > From the patch, it looks like the only PCIe port service this switch
> > > port advertises is AER (not PME, DPC, hotplug, etc), and it doesn't
> > > have MSI or MSI-X.
> > 
> > Correct.
> > 
> > > So aerdriver should be able to register for PCIE_PORT_SERVICE_AER, but
> > > aer_probe() ignores everything except Root Ports and RCECs.  What's
> > > the benefit then?  I must be missing something.
> > 
> > The portdrv is needed for power management and everything else PCI even
> > if there is no actual "service" attached.
> 
> Thanks!  I'm trying to connect the dots to get to the specific bug fix
> or improvement made by this patch.

Sure :)

> The pcie_pme_driver itself doesn't seem involved because it registers
> for PCIE_PORT_SERVICE_PME, and that's only set for Root Ports and
> RCECs.
> 
> The pcie_portdrv_pm_ops would be another possibility, but with the
> exception of pcie_port_runtime_idle(), all those ops just iterate over
> service drivers, which aren't involved.
> 
> So I'm guessing this has to do with setting
> DPM_FLAG_NO_DIRECT_COMPLETE and DPM_FLAG_SMART_SUSPEND in
> pcie_portdrv_probe().  Does the lack of portdrv mean the GPU can't
> suspend?  Does this reduce power consumption?  How would a user know
> this change would help their system?

If there is no driver attached to PCI device (this case upstream port)
it will not enter low power states. It allows GPU to suspend but it
does not allow the whole topology to enter low power states.