[PATCH] drm/amd: Fix the flag setting code for interrupt request

[Christian König]

Am 04.09.23 um 08:05 schrieb Ma Jun:
> [1] Remove the irq flags setting code since pci_alloc_irq_vectors()
> handles these flags.
> [2] Free the msi vectors in case of error.
>
> Signed-off-by: Ma Jun <Jun.Ma2 at amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c | 43 ++++++++++++++-----------
>   1 file changed, 25 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
> index fa6d0adcec20..17043a1e37a5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
> @@ -271,28 +271,28 @@ static void amdgpu_restore_msix(struct amdgpu_device *adev)
>   int amdgpu_irq_init(struct amdgpu_device *adev)
>   {
>   	int r = 0;

While at it please remove the assignment here.

Unless really necessary initializing local variables is rather frowned upon.

Apart from that Alex needs to take a look at this, I'm not that familiar 
with this code.

Christian.

> -	unsigned int irq;
> +	unsigned int irq, flags;
>   
>   	spin_lock_init(&adev->irq.lock);
>   
>   	/* Enable MSI if not disabled by module parameter */
>   	adev->irq.msi_enabled = false;
>   
> +	if (amdgpu_msi_ok(adev))
> +		flags = PCI_IRQ_MSI | PCI_IRQ_MSIX;
> +	else
> +		flags = PCI_IRQ_LEGACY;
> +
> +	/* we only need one vector */
> +	r = pci_alloc_irq_vectors(adev->pdev, 1, 1, flags);
> +	if (r < 0) {
> +		dev_err(adev->dev, "Failed to alloc msi vectors\n");
> +		return r;
> +	}
> +
>   	if (amdgpu_msi_ok(adev)) {
> -		int nvec = pci_msix_vec_count(adev->pdev);
> -		unsigned int flags;
> -
> -		if (nvec <= 0)
> -			flags = PCI_IRQ_MSI;
> -		else
> -			flags = PCI_IRQ_MSI | PCI_IRQ_MSIX;
> -
> -		/* we only need one vector */
> -		nvec = pci_alloc_irq_vectors(adev->pdev, 1, 1, flags);
> -		if (nvec > 0) {
> -			adev->irq.msi_enabled = true;
> -			dev_dbg(adev->dev, "using MSI/MSI-X.\n");
> -		}
> +		adev->irq.msi_enabled = true;
> +		dev_dbg(adev->dev, "using MSI/MSI-X.\n");
>   	}
>   
>   	INIT_WORK(&adev->irq.ih1_work, amdgpu_irq_handle_ih1);
> @@ -302,22 +302,29 @@ int amdgpu_irq_init(struct amdgpu_device *adev)
>   	/* Use vector 0 for MSI-X. */
>   	r = pci_irq_vector(adev->pdev, 0);
>   	if (r < 0)
> -		return r;
> +		goto free_vectors;
>   	irq = r;
>   
>   	/* PCI devices require shared interrupts. */
>   	r = request_irq(irq, amdgpu_irq_handler, IRQF_SHARED, adev_to_drm(adev)->driver->name,
>   			adev_to_drm(adev));
>   	if (r)
> -		return r;
> +		goto free_vectors;
> +
>   	adev->irq.installed = true;
>   	adev->irq.irq = irq;
>   	adev_to_drm(adev)->max_vblank_count = 0x00ffffff;
>   
>   	DRM_DEBUG("amdgpu: irq initialized.\n");
>   	return 0;
> -}
>   
> +free_vectors:
> +	if (adev->irq.msi_enabled)
> +		pci_free_irq_vectors(adev->pdev);
> +
> +	adev->irq.msi_enabled = false;
> +	return r;
> +}
>   
>   void amdgpu_irq_fini_hw(struct amdgpu_device *adev)
>   {