Re: [PATCH 01/14] drm/amdgpu: Convert to Linux IRQ interfaces

[Christian König <christian.koenig@xxxxxxx>]

Am 27.07.21 um 20:27 schrieb Thomas Zimmermann:
> Drop the DRM IRQ midlayer in favor of Linux IRQ interfaces. DRM's
> IRQ helpers are mostly useful for UMS drivers. Modern KMS drivers
> don't benefit from using it.
>
> DRM IRQ callbacks are now being called directly or inlined.
>
> The interrupt number returned by pci_msi_vector() is now stored
> in struct amdgpu_irq. Calls to pci_msi_vector() can fail and return
> a negative errno code. Abort initlaizaton in thi case. The DRM IRQ
> midlayer does not handle this correctly.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx>

Alex needs to take a look at this as well, but of hand the patch is 
Acked-by: Christian König <christian.koenig@xxxxxxx>.

Thanks,
Christian.

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c |  1 -
>   drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c | 21 ++++++++++++++-------
>   drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h |  2 +-
>   3 files changed, 15 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 2bd13fc2541a..1e05b5aa94e7 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -1775,7 +1775,6 @@ static const struct drm_driver amdgpu_kms_driver = {
>   	.open = amdgpu_driver_open_kms,
>   	.postclose = amdgpu_driver_postclose_kms,
>   	.lastclose = amdgpu_driver_lastclose_kms,
> -	.irq_handler = amdgpu_irq_handler,
>   	.ioctls = amdgpu_ioctls_kms,
>   	.num_ioctls = ARRAY_SIZE(amdgpu_ioctls_kms),
>   	.dumb_create = amdgpu_mode_dumb_create,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
> index 0d01cfaca77e..a36cdc7323f4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
> @@ -46,7 +46,6 @@
>   #include <linux/pci.h>
>   
>   #include <drm/drm_crtc_helper.h>
> -#include <drm/drm_irq.h>
>   #include <drm/drm_vblank.h>
>   #include <drm/amdgpu_drm.h>
>   #include <drm/drm_drv.h>
> @@ -184,7 +183,7 @@ void amdgpu_irq_disable_all(struct amdgpu_device *adev)
>    * Returns:
>    * result of handling the IRQ, as defined by &irqreturn_t
>    */
> -irqreturn_t amdgpu_irq_handler(int irq, void *arg)
> +static irqreturn_t amdgpu_irq_handler(int irq, void *arg)
>   {
>   	struct drm_device *dev = (struct drm_device *) arg;
>   	struct amdgpu_device *adev = drm_to_adev(dev);
> @@ -307,6 +306,7 @@ static void amdgpu_restore_msix(struct amdgpu_device *adev)
>   int amdgpu_irq_init(struct amdgpu_device *adev)
>   {
>   	int r = 0;
> +	unsigned int irq;
>   
>   	spin_lock_init(&adev->irq.lock);
>   
> @@ -349,15 +349,22 @@ int amdgpu_irq_init(struct amdgpu_device *adev)
>   	INIT_WORK(&adev->irq.ih2_work, amdgpu_irq_handle_ih2);
>   	INIT_WORK(&adev->irq.ih_soft_work, amdgpu_irq_handle_ih_soft);
>   
> -	adev->irq.installed = true;
> -	/* Use vector 0 for MSI-X */
> -	r = drm_irq_install(adev_to_drm(adev), pci_irq_vector(adev->pdev, 0));
> +	/* Use vector 0 for MSI-X. */
> +	r = pci_irq_vector(adev->pdev, 0);
> +	if (r < 0)
> +		return r;
> +	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) {
> -		adev->irq.installed = false;
>   		if (!amdgpu_device_has_dc_support(adev))
>   			flush_work(&adev->hotplug_work);
>   		return r;
>   	}
> +	adev->irq.installed = true;
> +	adev->irq.irq = irq;
>   	adev_to_drm(adev)->max_vblank_count = 0x00ffffff;
>   
>   	DRM_DEBUG("amdgpu: irq initialized.\n");
> @@ -368,7 +375,7 @@ int amdgpu_irq_init(struct amdgpu_device *adev)
>   void amdgpu_irq_fini_hw(struct amdgpu_device *adev)
>   {
>   	if (adev->irq.installed) {
> -		drm_irq_uninstall(&adev->ddev);
> +		free_irq(adev->irq.irq, adev_to_drm(adev));
>   		adev->irq.installed = false;
>   		if (adev->irq.msi_enabled)
>   			pci_free_irq_vectors(adev->pdev);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h
> index 78ad4784cc74..e9f2c11ea416 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h
> @@ -80,6 +80,7 @@ struct amdgpu_irq_src_funcs {
>   
>   struct amdgpu_irq {
>   	bool				installed;
> +	unsigned int			irq;
>   	spinlock_t			lock;
>   	/* interrupt sources */
>   	struct amdgpu_irq_client	client[AMDGPU_IRQ_CLIENTID_MAX];
> @@ -100,7 +101,6 @@ struct amdgpu_irq {
>   };
>   
>   void amdgpu_irq_disable_all(struct amdgpu_device *adev);
> -irqreturn_t amdgpu_irq_handler(int irq, void *arg);
>   
>   int amdgpu_irq_init(struct amdgpu_device *adev);
>   void amdgpu_irq_fini_sw(struct amdgpu_device *adev);