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

[Lucas De Marchi]

On Tue, Jan 16, 2024 at 03:07:36PM +0200, Dani Liberman wrote:
>For future platforms which will support msix, need to allocate all
			    ^^^^^

I think this is too broad. IMO better rephrased as:

"If platform supports MSIX, driver needs to allocate all possible
interrupts".

>possible interrupts.
>
>Signed-off-by: Dani Liberman <dliberman at habana.ai>
>Cc: Ohad Sharabi <osharabi 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..62722809a01d 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);
>+	if (pdev->msix_cap) {
>+		nvec = pci_msix_vec_count(pdev);

 From Documentation/PCI/msi-howto.rst:
	The following old APIs to enable and disable MSI or MSI-X interrupts should
	not be used in new code::

	  pci_enable_msi()              /* deprecated */
	  pci_disable_msi()             /* deprecated */
	  pci_enable_msix_range()       /* deprecated */
	  pci_enable_msix_exact()       /* deprecated */
	  pci_disable_msix()            /* deprecated */

	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.


 From pci_alloc_irq_vectors() documentation
it seems you only want to pass something other than 1 to
max_vecs?

>+		if (nvec <= 0) {
>+			drm_err(&xe->drm, "MSIX: Failed getting count\n");
>+			return -EINVAL;

error doesn't seem very appropriate here. Only reason I see for this
failing after checking msix_cap would be if kernel was built without
CONFIG_PCI_MSI, which would then be ENOSYS.

Lucas De Marchi

>+		}
>+	} else {
>+		/* device supports only msi */
>+		nvec = 1;
>+	}
>+
>+	err = pci_alloc_irq_vectors(pdev, nvec, nvec, PCI_IRQ_MSI | PCI_IRQ_MSIX);
> 	if (err < 0) {
> 		drm_err(&xe->drm, "MSI/MSIX: Failed to enable support %d\n", err);
> 		return err;
>-- 
>2.34.1
>