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

[Dani Liberman]

On 17/01/2024 1:13, Lucas De Marchi wrote:
> 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".

Will fix.

>
>> 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?
I've replied in another mail.
>
>> +        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.

Will fix.

Dani Liberman

>
> 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
>>