From: Kai-Heng Feng <kai.heng.feng@canonical.com>
To: Bjorn Helgaas <helgaas@kernel.org>
Cc: Linux PCI <linux-pci@vger.kernel.org>,
alsa-devel@alsa-project.org, tiwai@suse.com,
LKML <linux-kernel@vger.kernel.org>
Subject: Re: [alsa-devel] [PATCH 1/2] PCI: Add a helper to check Power Resource Requirements _PR3 existence
Date: Fri, 20 Sep 2019 13:23:20 +0200 [thread overview]
Message-ID: <CAAd53p4mc0tgCBiwfZRowr4os_bqDP+7Ko=d+do8OW2aH1Whzg@mail.gmail.com> (raw)
In-Reply-To: <20190909114129.GT103977@google.com>
Hi Bjorn,
I didn't find your reply in my mailbox earlier.
On Mon, Sep 9, 2019 at 1:41 PM Bjorn Helgaas <helgaas@kernel.org> 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@canonical.com>
> > ---
> > 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@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
next prev parent reply other threads:[~2019-09-20 11:24 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-08-27 13:47 [PATCH 1/2] PCI: Add a helper to check Power Resource Requirements _PR3 existence Kai-Heng Feng
2019-08-27 13:47 ` [PATCH 2/2] ALSA: hda: Allow HDA to be runtime suspended when dGPU is not bound Kai-Heng Feng
2019-08-27 14:50 ` Kai-Heng Feng
2019-08-27 15:24 ` Takashi Iwai
2019-08-27 22:31 ` Bjorn Helgaas
2019-08-28 8:25 ` Kai-Heng Feng
2019-08-28 18:01 ` [PATCH v2 " Kai-Heng Feng
2019-09-05 21:35 ` [alsa-devel] " Bjorn Helgaas
2019-09-17 9:36 ` Kai-Heng Feng
2019-09-18 12:42 ` [alsa-devel] [PATCH v3 2/2] ALSA: hda: Allow HDA to be runtime suspended when dGPU is not bound to a driver Kai-Heng Feng
2019-08-27 15:25 ` [PATCH 1/2] PCI: Add a helper to check Power Resource Requirements _PR3 existence Takashi Iwai
2019-08-27 16:58 ` Kai-Heng Feng
2019-08-27 22:13 ` Bjorn Helgaas
2019-08-27 22:39 ` Peter Wu
2019-09-09 11:41 ` [alsa-devel] " Bjorn Helgaas
2019-09-20 11:23 ` Kai-Heng Feng [this message]
2019-09-20 13:26 ` Bjorn Helgaas
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAAd53p4mc0tgCBiwfZRowr4os_bqDP+7Ko=d+do8OW2aH1Whzg@mail.gmail.com' \
--to=kai.heng.feng@canonical.com \
--cc=alsa-devel@alsa-project.org \
--cc=helgaas@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=tiwai@suse.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).