[PATCH v2] drm/xe/irq: allocate all possible msix interrupts

[Lucas De Marchi]

On Sun, Jan 21, 2024 at 11:02:14AM +0200, Dani Liberman wrote:
>If platform supports MSIX, driver needs to allocate all possible
>interrupts.
>
>v2:
>  - drop msix_cap and use the api return code instead.
>  - fix commit message.
>
>Cc: Ohad Sharabi <osharabi at habana.ai>
>Cc: Lucas De Marchi <lucas.demarchi at intel.com>
>Signed-off-by: Dani Liberman <dliberman at habana.ai>
>---
> drivers/gpu/drm/xe/xe_irq.c | 15 +++++++++++++--
> 1 file changed, 13 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/gpu/drm/xe/xe_irq.c b/drivers/gpu/drm/xe/xe_irq.c
>index 907c8ff0fa21..7a23d25c1062 100644
>--- a/drivers/gpu/drm/xe/xe_irq.c
>+++ b/drivers/gpu/drm/xe/xe_irq.c
>@@ -662,7 +662,7 @@ int xe_irq_install(struct xe_device *xe)
> {
> 	struct pci_dev *pdev = to_pci_dev(xe->drm.dev);
> 	irq_handler_t irq_handler;
>-	int err, irq;
>+	int err, irq, nvec;
>
> 	irq_handler = xe_irq_handler(xe);
> 	if (!irq_handler) {
>@@ -672,7 +672,18 @@ int xe_irq_install(struct xe_device *xe)
>
> 	xe_irq_reset(xe);
>
>-	err = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_MSI | PCI_IRQ_MSIX);
>+	nvec = pci_msix_vec_count(pdev);

 From docs:
	Additionally there are APIs to provide the number of supported MSI or MSI-X
	vectors: pci_msi_vec_count() and pci_msix_vec_count().  In general these
	should be avoided in favor of letting pci_alloc_irq_vectors() cap the
	number of vectors.  If you have a legitimate special use case for the count
	of vectors we might have to revisit that decision and add a
	pci_nr_irq_vectors() helper that handles MSI and MSI-X transparently.



>+	if (nvec <= 0) {
>+		if (nvec == -EINVAL) {
>+			/* MSIX capability is not supported in the device, using MSI */
>+			nvec = 1;
>+		} else {
>+			drm_err(&xe->drm, "MSIX: Failed getting count\n");
>+			return nvec;
>+		}
>+	}
>+
>+	err = pci_alloc_irq_vectors(pdev, nvec, nvec, PCI_IRQ_MSI | PCI_IRQ_MSIX);

these are just possible flags, so what you did above was for nothing? 

If platform doesn't support MSIX and we pass e.g. 16, we will still have
just 1 allocated. So, as I said in the other version, this is just a
maximum the **driver** wants to deal with. Still not clear why we need
this rather than just pass a value for max that covers all the
platforms.

Lucas De Marchi

> 	if (err < 0) {
> 		drm_err(&xe->drm, "MSI/MSIX: Failed to enable support %d\n", err);
> 		return err;
>-- 
>2.34.1
>