Hi Bjorn,
I didn't find your reply in my mailbox earlier.
On Mon, Sep 9, 2019 at 1:41 PM Bjorn Helgaas <helgaas@xxxxxxxxxx> wrote:
>
> Maybe:
>
> PCI: Add pci_pr3_present() to check for Power Resources for D3hot
Ok, this is a good title.
>
> On Tue, Aug 27, 2019 at 09:47:55PM +0800, Kai-Heng Feng wrote:
> > A driver may want to know the existence of _PR3, to choose different
> > runtime suspend behavior. A user will be add in next patch.
>
> Maybe include something like this in the commit lot?
>
> Add pci_pr3_present() to check whether the platform supplies _PR3 to
> tell us which power resources the device depends on when in D3hot.
Ok.
>
> > This is mostly the same as nouveau_pr3_present().
> >
> > Signed-off-by: Kai-Heng Feng <kai.heng.feng@xxxxxxxxxxxxx>
> > ---
> > drivers/pci/pci.c | 20 ++++++++++++++++++++
> > include/linux/pci.h | 2 ++
> > 2 files changed, 22 insertions(+)
> >
> > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > index 1b27b5af3d55..776af15b92c2 100644
> > --- a/drivers/pci/pci.c
> > +++ b/drivers/pci/pci.c
> > @@ -5856,6 +5856,26 @@ int pci_set_vga_state(struct pci_dev *dev, bool decode,
> > return 0;
> > }
> >
> > +bool pci_pr3_present(struct pci_dev *pdev)
> > +{
> > + struct pci_dev *parent_pdev = pci_upstream_bridge(pdev);
> > + struct acpi_device *parent_adev;
> > +
> > + if (acpi_disabled)
> > + return false;
> > +
> > + if (!parent_pdev)
> > + return false;
> > +
> > + parent_adev = ACPI_COMPANION(&parent_pdev->dev);
> > + if (!parent_adev)
> > + return false;
> > +
> > + return parent_adev->power.flags.power_resources &&
> > + acpi_has_method(parent_adev->handle, "_PR3");
>
> I think this is generally OK, but it doesn't actually check whether
> *pdev* has a _PR3; it checks whether pdev's *parent* does. So does
> that mean this is dependent on the GPU topology, i.e., does it assume
> that there is an upstream bridge and that power for everything under
> that bridge can be managed together?
Yes, the power resource is managed by its upstream port.
>
> I'm wondering whether the "parent_pdev = pci_upstream_bridge()" part
> should be in the caller rather than in pci_pr3_present()?
This will make the function more align to its name, but needs more
work from caller side.
How about rename the function to pci_upstream_pr3_present()?
>
> I can't connect any of the dots from _PR3 through to
> "need_eld_notify_link" (whatever "eld" is :)) and the uses of
> hda_intel.need_eld_notify_link (and needs_eld_notify_link()).
>
> But that's beyond the scope of *this* patch and it makes sense that
> you do want to discover the _PR3 existence, so I'm fine with this once
> we figure out the pdev vs parent question.
Thanks for your review.
Kai-Heng
>
> > +}
> > +EXPORT_SYMBOL_GPL(pci_pr3_present);
> > +
> > /**
> > * pci_add_dma_alias - Add a DMA devfn alias for a device
> > * @dev: the PCI device for which alias is added
> > diff --git a/include/linux/pci.h b/include/linux/pci.h
> > index 82e4cd1b7ac3..9b6f7b67fac9 100644
> > --- a/include/linux/pci.h
> > +++ b/include/linux/pci.h
> > @@ -2348,9 +2348,11 @@ struct irq_domain *pci_host_bridge_acpi_msi_domain(struct pci_bus *bus);
> >
> > void
> > pci_msi_register_fwnode_provider(struct fwnode_handle *(*fn)(struct device *));
> > +bool pci_pr3_present(struct pci_dev *pdev);
> > #else
> > static inline struct irq_domain *
> > pci_host_bridge_acpi_msi_domain(struct pci_bus *bus) { return NULL; }
> > +static bool pci_pr3_present(struct pci_dev *pdev) { return false; }
> > #endif
> >
> > #ifdef CONFIG_EEH
> > --
> > 2.17.1
> >
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
