Re: [Linux-stable-mirror] FAILED: patch "[PATCH] drm/nouveau: Move irq setup/teardown to pci ctor/dtor" failed to apply to 4.14-stable tree

Oh sweet, looks like this managed to make it in on the final pull! I had cc'd stable in case it didn't manage to. You can ignore backporting this patch; the problem shouldn't be present on 4.14 and earlier

On Sat, 2018-01-27 at 11:02 +0100, gregkh@linuxfoundation.org wrote:

The patch below does not apply to the 4.14-stable tree. If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to stable@vger.kernel.org.

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From 0fd189a95fdbc631737df5f27a0fc0a3dd31b75e Mon Sep 17 00:00:00 2001 From: Lyude Paul lyude@redhat.com Date: Thu, 25 Jan 2018 18:29:53 -0500 Subject: [PATCH] drm/nouveau: Move irq setup/teardown to pci ctor/dtor

For a while we've been having issues with seemingly random interrupts coming from nvidia cards when resuming them. Originally the fix for this was thought to be just re-arming the MSI interrupt registers right after re-allocating our IRQs, however it seems a lot of what we do is both wrong and not even nessecary.

This was made apparent by what appeared to be a regression in the mainline kernel that started introducing suspend/resume issues for nouveau:

    a0c9259dc4e1 (irq/matrix: Spread interrupts on allocation)

After this commit was introduced, we started getting interrupts from the GPU before we actually re-allocated our own IRQ (see references below) and assigned the IRQ handler. Investigating this turned out that the problem was not with the commit, but the fact that nouveau even free/allocates it's irqs before and after suspend/resume.

For starters: drivers in the linux kernel haven't had to handle freeing/re-allocating their IRQs during suspend/resume cycles for quite a while now. Nouveau seems to be one of the few drivers left that still does this, despite the fact there's no reason we actually need to since disabling interrupts from the device side should be enough, as the kernel is already smart enough to know to disable host-side interrupts for us before going into suspend. Since we were tearing down our IRQs by hand however, that means there was a short period during resume where interrupts could be received before we re-allocated our IRQ which would lead to us getting an unhandled IRQ. Since we never handle said IRQ and re-arm the interrupt registers, this would cause us to miss all of the interrupts from the GPU and cause our init process to start timing out on anything requiring interrupts.

So, since this whole setup/teardown every suspend/resume cycle is useless anyway, move irq setup/teardown into the pci subdev's ctor/dtor functions instead so they're only called at driver load and driver unload. This should fix most of the issues with pending interrupts on resume, along with getting suspend/resume for nouveau to work again.

As well, this probably means we can also just remove the msi rearm call inside nvkm_pci_init(). But since our main focus here is to fix suspend/resume before 4.15, we'll save that for a later patch.

Signed-off-by: Lyude Paul lyude@redhat.com Cc: Karol Herbst kherbst@redhat.com Cc: Thomas Gleixner tglx@linutronix.de Cc: Mike Galbraith efault@gmx.de Cc: stable@vger.kernel.org Signed-off-by: Ben Skeggs bskeggs@redhat.com

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/pci/base.c b/drivers/gpu/drm/nouveau/nvkm/subdev/pci/base.c index deb96de54b00..ee2431a7804e 100644 --- a/drivers/gpu/drm/nouveau/nvkm/subdev/pci/base.c +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/pci/base.c @@ -71,6 +71,10 @@ nvkm_pci_intr(int irq, void *arg) struct nvkm_pci *pci = arg; struct nvkm_device *device = pci->subdev.device; bool handled = false;

• if (pci->irq < 0)

• return IRQ_HANDLED;
  

• nvkm_mc_intr_unarm(device); if (pci->msi) pci->func->msi_rearm(pci);

@@ -84,11 +88,6 @@ nvkm_pci_fini(struct nvkm_subdev *subdev, bool suspend) { struct nvkm_pci *pci = nvkm_pci(subdev);

• if (pci->irq >= 0) {

• free_irq(pci->irq, pci);
  

• pci->irq = -1;
  

• }
• if (pci->agp.bridge) nvkm_agp_fini(pci);

@@ -108,8 +107,20 @@ static int nvkm_pci_oneinit(struct nvkm_subdev *subdev) { struct nvkm_pci *pci = nvkm_pci(subdev);

• if (pci_is_pcie(pci->pdev))

• return nvkm_pcie_oneinit(pci);
  

• struct pci_dev *pdev = pci->pdev;
• int ret;
• if (pci_is_pcie(pci->pdev)) {

• ret = nvkm_pcie_oneinit(pci);
  

• if (ret)
  

• 	return ret;
  

• }
• ret = request_irq(pdev->irq, nvkm_pci_intr, IRQF_SHARED, "nvkm",

pci);

• if (ret)

• return ret;
  

• pci->irq = pdev->irq; return 0;

} @@ -117,7 +128,6 @@ static int nvkm_pci_init(struct nvkm_subdev *subdev) { struct nvkm_pci *pci = nvkm_pci(subdev);

• struct pci_dev *pdev = pci->pdev; int ret;

if (pci->agp.bridge) { @@ -131,28 +141,34 @@ nvkm_pci_init(struct nvkm_subdev *subdev) if (pci->func->init) pci->func->init(pci);

• ret = request_irq(pdev->irq, nvkm_pci_intr, IRQF_SHARED, "nvkm",

pci);

• if (ret)

• return ret;
  

• pci->irq = pdev->irq;
• /* Ensure MSI interrupts are armed, for the case where there are
  • already interrupts pending (for whatever reason) at load time.
  */ if (pci->msi) pci->func->msi_rearm(pci);

• return ret;

• return 0;

} static void * nvkm_pci_dtor(struct nvkm_subdev *subdev) { struct nvkm_pci *pci = nvkm_pci(subdev);

• nvkm_agp_dtor(pci);
• if (pci->irq >= 0) {

• /* freq_irq() will call the handler, we use pci->irq == -1
  

•  * to signal that it's been torn down and should be a noop.
  

•  */
  

• int irq = pci->irq;
  

• pci->irq = -1;
  

• free_irq(irq, pci);
  

• }
• if (pci->msi) pci_disable_msi(pci->pdev);
• return nvkm_pci(subdev);

}