| Message ID | 20220127025418.1989642-2-kai.heng.feng@canonical.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | [v2,1/2] PCI/AER: Disable AER service when link is in L2/L3 ready, L2 and L3 state | expand |
On Thu, Jan 27, 2022 at 10:54:18AM +0800, Kai-Heng Feng wrote: > Since TLP and DLLP transmission is disabled for a Link in L2/L3 Ready, > L2 and L3 (i.e. device in D3hot and D3cold), and DPC depends on AER, so > also disable DPC here. > > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
On 1/26/22 6:54 PM, Kai-Heng Feng wrote: > Since TLP and DLLP transmission is disabled for a Link in L2/L3 Ready, > L2 and L3 (i.e. device in D3hot and D3cold), and DPC depends on AER, so Better description about the problem would be helpful. I know you have included a log in AER patch. But a quick summary of the problem in this patch will make it easier to read the patch. > also disable DPC here. > > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> > --- > v2: > - Wording change. > - Empty line dropped. > > drivers/pci/pcie/dpc.c | 60 +++++++++++++++++++++++++++++++----------- > 1 file changed, 44 insertions(+), 16 deletions(-) > > diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c > index 3e9afee02e8d1..414258967f08e 100644 > --- a/drivers/pci/pcie/dpc.c > +++ b/drivers/pci/pcie/dpc.c > @@ -343,13 +343,33 @@ void pci_dpc_init(struct pci_dev *pdev) > } > } > > +static void dpc_enable(struct pcie_device *dev) > +{ > + struct pci_dev *pdev = dev->port; > + u16 ctl; > + > + pci_read_config_word(pdev, pdev->dpc_cap + PCI_EXP_DPC_CTL, &ctl); > + ctl = (ctl & 0xfff4) | PCI_EXP_DPC_CTL_EN_FATAL | PCI_EXP_DPC_CTL_INT_EN; > + pci_write_config_word(pdev, pdev->dpc_cap + PCI_EXP_DPC_CTL, ctl); > +} > + > +static void dpc_disable(struct pcie_device *dev) > +{ > + struct pci_dev *pdev = dev->port; > + u16 ctl; > + > + pci_read_config_word(pdev, pdev->dpc_cap + PCI_EXP_DPC_CTL, &ctl); > + ctl &= ~(PCI_EXP_DPC_CTL_EN_FATAL | PCI_EXP_DPC_CTL_INT_EN); > + pci_write_config_word(pdev, pdev->dpc_cap + PCI_EXP_DPC_CTL, ctl); > +} > + > #define FLAG(x, y) (((x) & (y)) ? '+' : '-') > static int dpc_probe(struct pcie_device *dev) > { > struct pci_dev *pdev = dev->port; > struct device *device = &dev->device; > int status; > - u16 ctl, cap; > + u16 cap; > > if (!pcie_aer_is_native(pdev) && !pcie_ports_dpc_native) > return -ENOTSUPP; > @@ -364,10 +384,7 @@ static int dpc_probe(struct pcie_device *dev) > } > > pci_read_config_word(pdev, pdev->dpc_cap + PCI_EXP_DPC_CAP, &cap); > - pci_read_config_word(pdev, pdev->dpc_cap + PCI_EXP_DPC_CTL, &ctl); > - > - ctl = (ctl & 0xfff4) | PCI_EXP_DPC_CTL_EN_FATAL | PCI_EXP_DPC_CTL_INT_EN; > - pci_write_config_word(pdev, pdev->dpc_cap + PCI_EXP_DPC_CTL, ctl); > + dpc_enable(dev); > pci_info(pdev, "enabled with IRQ %d\n", dev->irq); > > pci_info(pdev, "error containment capabilities: Int Msg #%d, RPExt%c PoisonedTLP%c SwTrigger%c RP PIO Log %d, DL_ActiveErr%c\n", > @@ -380,22 +397,33 @@ static int dpc_probe(struct pcie_device *dev) > return status; > } > > -static void dpc_remove(struct pcie_device *dev) > +static int dpc_suspend(struct pcie_device *dev) > { > - struct pci_dev *pdev = dev->port; > - u16 ctl; > + dpc_disable(dev); > + return 0; > +} > > - pci_read_config_word(pdev, pdev->dpc_cap + PCI_EXP_DPC_CTL, &ctl); > - ctl &= ~(PCI_EXP_DPC_CTL_EN_FATAL | PCI_EXP_DPC_CTL_INT_EN); > - pci_write_config_word(pdev, pdev->dpc_cap + PCI_EXP_DPC_CTL, ctl); > +static int dpc_resume(struct pcie_device *dev) > +{ > + dpc_enable(dev); > + return 0; > +} > + > +static void dpc_remove(struct pcie_device *dev) > +{ > + dpc_disable(dev); > } > > static struct pcie_port_service_driver dpcdriver = { > - .name = "dpc", > - .port_type = PCIE_ANY_PORT, > - .service = PCIE_PORT_SERVICE_DPC, > - .probe = dpc_probe, > - .remove = dpc_remove, > + .name = "dpc", > + .port_type = PCIE_ANY_PORT, > + .service = PCIE_PORT_SERVICE_DPC, > + .probe = dpc_probe, > + .suspend = dpc_suspend, > + .resume = dpc_resume, > + .runtime_suspend = dpc_suspend, > + .runtime_resume = dpc_resume, > + .remove = dpc_remove, > }; > > int __init pcie_dpc_init(void)
diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c index 3e9afee02e8d1..414258967f08e 100644 --- a/drivers/pci/pcie/dpc.c +++ b/drivers/pci/pcie/dpc.c @@ -343,13 +343,33 @@ void pci_dpc_init(struct pci_dev *pdev) } } +static void dpc_enable(struct pcie_device *dev) +{ + struct pci_dev *pdev = dev->port; + u16 ctl; + + pci_read_config_word(pdev, pdev->dpc_cap + PCI_EXP_DPC_CTL, &ctl); + ctl = (ctl & 0xfff4) | PCI_EXP_DPC_CTL_EN_FATAL | PCI_EXP_DPC_CTL_INT_EN; + pci_write_config_word(pdev, pdev->dpc_cap + PCI_EXP_DPC_CTL, ctl); +} + +static void dpc_disable(struct pcie_device *dev) +{ + struct pci_dev *pdev = dev->port; + u16 ctl; + + pci_read_config_word(pdev, pdev->dpc_cap + PCI_EXP_DPC_CTL, &ctl); + ctl &= ~(PCI_EXP_DPC_CTL_EN_FATAL | PCI_EXP_DPC_CTL_INT_EN); + pci_write_config_word(pdev, pdev->dpc_cap + PCI_EXP_DPC_CTL, ctl); +} + #define FLAG(x, y) (((x) & (y)) ? '+' : '-') static int dpc_probe(struct pcie_device *dev) { struct pci_dev *pdev = dev->port; struct device *device = &dev->device; int status; - u16 ctl, cap; + u16 cap; if (!pcie_aer_is_native(pdev) && !pcie_ports_dpc_native) return -ENOTSUPP; @@ -364,10 +384,7 @@ static int dpc_probe(struct pcie_device *dev) } pci_read_config_word(pdev, pdev->dpc_cap + PCI_EXP_DPC_CAP, &cap); - pci_read_config_word(pdev, pdev->dpc_cap + PCI_EXP_DPC_CTL, &ctl); - - ctl = (ctl & 0xfff4) | PCI_EXP_DPC_CTL_EN_FATAL | PCI_EXP_DPC_CTL_INT_EN; - pci_write_config_word(pdev, pdev->dpc_cap + PCI_EXP_DPC_CTL, ctl); + dpc_enable(dev); pci_info(pdev, "enabled with IRQ %d\n", dev->irq); pci_info(pdev, "error containment capabilities: Int Msg #%d, RPExt%c PoisonedTLP%c SwTrigger%c RP PIO Log %d, DL_ActiveErr%c\n", @@ -380,22 +397,33 @@ static int dpc_probe(struct pcie_device *dev) return status; } -static void dpc_remove(struct pcie_device *dev) +static int dpc_suspend(struct pcie_device *dev) { - struct pci_dev *pdev = dev->port; - u16 ctl; + dpc_disable(dev); + return 0; +} - pci_read_config_word(pdev, pdev->dpc_cap + PCI_EXP_DPC_CTL, &ctl); - ctl &= ~(PCI_EXP_DPC_CTL_EN_FATAL | PCI_EXP_DPC_CTL_INT_EN); - pci_write_config_word(pdev, pdev->dpc_cap + PCI_EXP_DPC_CTL, ctl); +static int dpc_resume(struct pcie_device *dev) +{ + dpc_enable(dev); + return 0; +} + +static void dpc_remove(struct pcie_device *dev) +{ + dpc_disable(dev); } static struct pcie_port_service_driver dpcdriver = { - .name = "dpc", - .port_type = PCIE_ANY_PORT, - .service = PCIE_PORT_SERVICE_DPC, - .probe = dpc_probe, - .remove = dpc_remove, + .name = "dpc", + .port_type = PCIE_ANY_PORT, + .service = PCIE_PORT_SERVICE_DPC, + .probe = dpc_probe, + .suspend = dpc_suspend, + .resume = dpc_resume, + .runtime_suspend = dpc_suspend, + .runtime_resume = dpc_resume, + .remove = dpc_remove, }; int __init pcie_dpc_init(void)
Since TLP and DLLP transmission is disabled for a Link in L2/L3 Ready, L2 and L3 (i.e. device in D3hot and D3cold), and DPC depends on AER, so also disable DPC here. Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> --- v2: - Wording change. - Empty line dropped. drivers/pci/pcie/dpc.c | 60 +++++++++++++++++++++++++++++++----------- 1 file changed, 44 insertions(+), 16 deletions(-)