[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] pcie: Add more ASPM support
From: |
Michael S. Tsirkin |
Subject: |
Re: [Qemu-devel] [PATCH] pcie: Add more ASPM support |
Date: |
Sun, 19 May 2013 14:54:18 +0300 |
On Wed, May 15, 2013 at 04:33:08PM -0600, Bjorn Helgaas wrote:
> Indicate ASPM L0s and L1 support in Link Capabilities and make the ASPM
> bits in Link Control writable. These Link Control bits don't do anything
> in qemu, but having them writable means the BIOS or OS can write them as
> on real hardware.
>
> Signed-off-by: Bjorn Helgaas <address@hidden>
Out of curiousity - L1 is optional on real hardware, isn't it?
Anyway, went over spec and L1 is just like L0s
but with higher latency, so I think this is a safe change.
A minor comment below - would appreciate if you can address it.
> ---
> hw/pci/pcie.c | 4 +++-
> include/hw/pci/pcie_regs.h | 5 ++++-
> 2 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
> index 62bd0b8..7336054 100644
> --- a/hw/pci/pcie.c
> +++ b/hw/pci/pcie.c
> @@ -73,13 +73,15 @@ int pcie_cap_init(PCIDevice *dev, uint8_t offset, uint8_t
> type, uint8_t port)
>
> pci_set_long(exp_cap + PCI_EXP_LNKCAP,
> (port << PCI_EXP_LNKCAP_PN_SHIFT) |
> - PCI_EXP_LNKCAP_ASPMS_0S |
> + PCI_EXP_LNKCAP_ASPMS_0S | PCI_EXP_LNKCAP_ASPMS_1 |
> PCI_EXP_LNK_MLW_1 |
> PCI_EXP_LNK_LS_25);
>
> pci_set_word(exp_cap + PCI_EXP_LNKSTA,
> PCI_EXP_LNK_MLW_1 | PCI_EXP_LNK_LS_25);
>
> + pci_set_word(dev->wmask + pos + PCI_EXP_LNKCTL, PCI_EXP_LNKCTL_ASPM);
> +
> pci_set_long(exp_cap + PCI_EXP_DEVCAP2,
> PCI_EXP_DEVCAP2_EFF | PCI_EXP_DEVCAP2_EETLPP);
>
> diff --git a/include/hw/pci/pcie_regs.h b/include/hw/pci/pcie_regs.h
> index 4d123d9..6b80a8d 100644
> --- a/include/hw/pci/pcie_regs.h
> +++ b/include/hw/pci/pcie_regs.h
> @@ -31,7 +31,7 @@
> #define PCI_EXP_FLAGS_TYPE_SHIFT (ffs(PCI_EXP_FLAGS_TYPE) - 1)
>
>
> -/* PCI_EXP_LINK{CAP, STA} */
> +/* PCI_EXP_LINK{CAP, STA, CTL} */
> /* link speed */
> #define PCI_EXP_LNK_LS_25 1
>
> @@ -41,9 +41,12 @@
> /* PCI_EXP_LINKCAP */
> #define PCI_EXP_LNKCAP_ASPMS_SHIFT (ffs(PCI_EXP_LNKCAP_ASPMS) - 1)
> #define PCI_EXP_LNKCAP_ASPMS_0S (1 << PCI_EXP_LNKCAP_ASPMS_SHIFT)
> +#define PCI_EXP_LNKCAP_ASPMS_1 (2 << PCI_EXP_LNKCAP_ASPMS_SHIFT)
>
> #define PCI_EXP_LNKCAP_PN_SHIFT (ffs(PCI_EXP_LNKCAP_PN) - 1)
>
> +#define PCI_EXP_LNKCTL_ASPM 0x0003
> +
> #define PCI_EXP_SLTCAP_PSN_SHIFT (ffs(PCI_EXP_SLTCAP_PSN) - 1)
>
> #define PCI_EXP_SLTCTL_IND_RESERVED 0x0
I'd like to name them the same as you did in linux/pci_regs.h
Let's rename PCI_EXP_LNKCAP_ASPMS_0S/PCI_EXP_LNKCAP_ASPMS_1 as well.
Thanks!