* [PATCH v2] drm/amd: Fix the flag setting code for interrupt request
@ 2023-09-06 6:55 Ma Jun
2023-09-06 7:23 ` Christian König
0 siblings, 1 reply; 3+ messages in thread
From: Ma Jun @ 2023-09-06 6:55 UTC (permalink / raw)
To: amd-gfx, christian.koenig, Alexander.Deucher; +Cc: 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.
v2:
- Remove local variable initializing code (Christian)
- Use PCI_IRQ_ALL_TYPES (Alex)
Signed-off-by: Ma Jun <Jun.Ma2@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c | 45 ++++++++++++++-----------
1 file changed, 26 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
index fa6d0adcec20..64c245015e17 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
@@ -270,29 +270,29 @@ static void amdgpu_restore_msix(struct amdgpu_device *adev)
*/
int amdgpu_irq_init(struct amdgpu_device *adev)
{
- int r = 0;
- unsigned int irq;
+ int r;
+ 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_LEGACY;
+ else
+ flags = PCI_IRQ_ALL_TYPES;
+
+ /* 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)
{
--
2.34.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2] drm/amd: Fix the flag setting code for interrupt request
2023-09-06 6:55 [PATCH v2] drm/amd: Fix the flag setting code for interrupt request Ma Jun
@ 2023-09-06 7:23 ` Christian König
2023-09-06 11:22 ` Ma, Jun
0 siblings, 1 reply; 3+ messages in thread
From: Christian König @ 2023-09-06 7:23 UTC (permalink / raw)
To: Ma Jun, amd-gfx, Alexander.Deucher
Am 06.09.23 um 08:55 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.
>
> v2:
> - Remove local variable initializing code (Christian)
> - Use PCI_IRQ_ALL_TYPES (Alex)
>
> Signed-off-by: Ma Jun <Jun.Ma2@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c | 45 ++++++++++++++-----------
> 1 file changed, 26 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
> index fa6d0adcec20..64c245015e17 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
> @@ -270,29 +270,29 @@ static void amdgpu_restore_msix(struct amdgpu_device *adev)
> */
> int amdgpu_irq_init(struct amdgpu_device *adev)
> {
> - int r = 0;
> - unsigned int irq;
> + int r;
> + unsigned int irq, flags;
It's also good style to define variables like "r" and "i" last. Some
upstream maintainers even require reverse xmas tree style defines (e.g.
longest first, shortest last).
With that changed the patch is Acked-by: Christian König
<christian.koenig@amd.com>
Regards,
Christian.
>
> 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_LEGACY;
> + else
> + flags = PCI_IRQ_ALL_TYPES;
> +
> + /* 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)
> {
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] drm/amd: Fix the flag setting code for interrupt request
2023-09-06 7:23 ` Christian König
@ 2023-09-06 11:22 ` Ma, Jun
0 siblings, 0 replies; 3+ messages in thread
From: Ma, Jun @ 2023-09-06 11:22 UTC (permalink / raw)
To: Christian König, Ma Jun, amd-gfx, Alexander.Deucher
On 9/6/2023 3:23 PM, Christian König wrote:
> Am 06.09.23 um 08:55 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.
>>
>> v2:
>> - Remove local variable initializing code (Christian)
>> - Use PCI_IRQ_ALL_TYPES (Alex)
>>
>> Signed-off-by: Ma Jun <Jun.Ma2@amd.com>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c | 45 ++++++++++++++-----------
>> 1 file changed, 26 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
>> index fa6d0adcec20..64c245015e17 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
>> @@ -270,29 +270,29 @@ static void amdgpu_restore_msix(struct amdgpu_device *adev)
>> */
>> int amdgpu_irq_init(struct amdgpu_device *adev)
>> {
>> - int r = 0;
>> - unsigned int irq;
>> + int r;
>> + unsigned int irq, flags;
>
> It's also good style to define variables like "r" and "i" last. Some
> upstream maintainers even require reverse xmas tree style defines (e.g.
> longest first, shortest last).
>
> With that changed the patch is Acked-by: Christian König
> <christian.koenig@amd.com>
>
Thanks, I will update it when push.
Regards,
Ma Jun
> Regards,
> Christian.
>
>>
>> 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_LEGACY;
>> + else
>> + flags = PCI_IRQ_ALL_TYPES;
>> +
>> + /* 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)
>> {
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-09-06 11:22 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-06 6:55 [PATCH v2] drm/amd: Fix the flag setting code for interrupt request Ma Jun
2023-09-06 7:23 ` Christian König
2023-09-06 11:22 ` Ma, Jun
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.