diff mbox

amdkfd: Drop interrupt SW ring buffer

Message ID 1420603176-1520-1-git-send-email-michel@daenzer.net (mailing list archive)
State New, archived
Headers show

Commit Message

Michel Dänzer Jan. 7, 2015, 3:59 a.m. UTC
From: Michel Dänzer <michel.daenzer@amd.com>

The SW ring buffer was smaller than the corresponding hardware ring, so
dmesg could get spammed by

 kfd kfd: Interrupt ring overflow, dropping interrupt.

messages when running graphics apps.

Since the SW ring buffer doesn't actually do anything at this point, just
remove it for now. When actual interrupt processing code is added to
amdkfd, it should try to do things immediately and only defer to work
queues when necessary.

Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
---
 drivers/gpu/drm/amd/amdkfd/Makefile        |   3 +-
 drivers/gpu/drm/amd/amdkfd/kfd_device.c    |  20 +---
 drivers/gpu/drm/amd/amdkfd/kfd_interrupt.c | 176 -----------------------------
 drivers/gpu/drm/amd/amdkfd/kfd_priv.h      |  15 ---
 4 files changed, 2 insertions(+), 212 deletions(-)
 delete mode 100644 drivers/gpu/drm/amd/amdkfd/kfd_interrupt.c

Comments

Oded Gabbay Jan. 7, 2015, 12:24 p.m. UTC | #1
Hi Michel,
So your patch is quite, hmm, *drastic* :)

Instead, could I suggest to only remove the calls to kfd_interrupt_init()
and kfd_interrupt_exit() ? It will also require a minor modification to the
logic in kgd2kfd_interrupt() but it is much less intrusive than what you are
suggesting.

Alternatively, we could take just this hunk:

> @@ -296,13 +286,5 @@ int kgd2kfd_resume(struct kfd_dev *kfd)
>  /* This is called directly from KGD at ISR. */
>  void kgd2kfd_interrupt(struct kfd_dev *kfd, const void *ih_ring_entry)
>  {
> -	if (kfd->init_complete) {
> -		spin_lock(&kfd->interrupt_lock);
> -
> -		if (kfd->interrupts_active
> -		    && enqueue_ih_ring_entry(kfd, ih_ring_entry))
> -			schedule_work(&kfd->interrupt_work);
> -
> -		spin_unlock(&kfd->interrupt_lock);
> -	}
> +	/* Process interrupts / schedule work as necessary */
>  }


After all, we do need this feature eventually and most of it is fine, so I
don't want to take it all out. As I said, it is *drastic*.

	Oded

On 01/07/2015 05:59 AM, Michel Dänzer wrote:
> From: Michel Dänzer <michel.daenzer@amd.com>
> 
> The SW ring buffer was smaller than the corresponding hardware ring, so
> dmesg could get spammed by
> 
>  kfd kfd: Interrupt ring overflow, dropping interrupt.
> 
> messages when running graphics apps.
> 
> Since the SW ring buffer doesn't actually do anything at this point, just
> remove it for now. When actual interrupt processing code is added to
> amdkfd, it should try to do things immediately and only defer to work
> queues when necessary.
> 
> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
> ---
>  drivers/gpu/drm/amd/amdkfd/Makefile        |   3 +-
>  drivers/gpu/drm/amd/amdkfd/kfd_device.c    |  20 +---
>  drivers/gpu/drm/amd/amdkfd/kfd_interrupt.c | 176 -----------------------------
>  drivers/gpu/drm/amd/amdkfd/kfd_priv.h      |  15 ---
>  4 files changed, 2 insertions(+), 212 deletions(-)
>  delete mode 100644 drivers/gpu/drm/amd/amdkfd/kfd_interrupt.c
> 
> diff --git a/drivers/gpu/drm/amd/amdkfd/Makefile b/drivers/gpu/drm/amd/amdkfd/Makefile
> index be6246d..307a309 100644
> --- a/drivers/gpu/drm/amd/amdkfd/Makefile
> +++ b/drivers/gpu/drm/amd/amdkfd/Makefile
> @@ -8,7 +8,6 @@ amdkfd-y	:= kfd_module.o kfd_device.o kfd_chardev.o kfd_topology.o \
>  		kfd_pasid.o kfd_doorbell.o kfd_flat_memory.o \
>  		kfd_process.o kfd_queue.o kfd_mqd_manager.o \
>  		kfd_kernel_queue.o kfd_packet_manager.o \
> -		kfd_process_queue_manager.o kfd_device_queue_manager.o \
> -		kfd_interrupt.o
> +		kfd_process_queue_manager.o kfd_device_queue_manager.o
>  
>  obj-$(CONFIG_HSA_AMD)	+= amdkfd.o
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device.c b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> index 43884eb..633532a 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> @@ -192,13 +192,6 @@ bool kgd2kfd_device_init(struct kfd_dev *kfd,
>  		goto kfd_topology_add_device_error;
>  	}
>  
> -	if (kfd_interrupt_init(kfd)) {
> -		dev_err(kfd_device,
> -			"Error initializing interrupts for device (%x:%x)\n",
> -			kfd->pdev->vendor, kfd->pdev->device);
> -		goto kfd_interrupt_error;
> -	}
> -
>  	if (!device_iommu_pasid_init(kfd)) {
>  		dev_err(kfd_device,
>  			"Error initializing iommuv2 for device (%x:%x)\n",
> @@ -237,8 +230,6 @@ dqm_start_error:
>  device_queue_manager_error:
>  	amd_iommu_free_device(kfd->pdev);
>  device_iommu_pasid_error:
> -	kfd_interrupt_exit(kfd);
> -kfd_interrupt_error:
>  	kfd_topology_remove_device(kfd);
>  kfd_topology_add_device_error:
>  	kfd2kgd->fini_sa_manager(kfd->kgd);
> @@ -254,7 +245,6 @@ void kgd2kfd_device_exit(struct kfd_dev *kfd)
>  	if (kfd->init_complete) {
>  		device_queue_manager_uninit(kfd->dqm);
>  		amd_iommu_free_device(kfd->pdev);
> -		kfd_interrupt_exit(kfd);
>  		kfd_topology_remove_device(kfd);
>  	}
>  
> @@ -296,13 +286,5 @@ int kgd2kfd_resume(struct kfd_dev *kfd)
>  /* This is called directly from KGD at ISR. */
>  void kgd2kfd_interrupt(struct kfd_dev *kfd, const void *ih_ring_entry)
>  {
> -	if (kfd->init_complete) {
> -		spin_lock(&kfd->interrupt_lock);
> -
> -		if (kfd->interrupts_active
> -		    && enqueue_ih_ring_entry(kfd, ih_ring_entry))
> -			schedule_work(&kfd->interrupt_work);
> -
> -		spin_unlock(&kfd->interrupt_lock);
> -	}
> +	/* Process interrupts / schedule work as necessary */
>  }
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_interrupt.c b/drivers/gpu/drm/amd/amdkfd/kfd_interrupt.c
> deleted file mode 100644
> index 5b99909..0000000
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_interrupt.c
> +++ /dev/null
> @@ -1,176 +0,0 @@
> -/*
> - * Copyright 2014 Advanced Micro Devices, Inc.
> - *
> - * Permission is hereby granted, free of charge, to any person obtaining a
> - * copy of this software and associated documentation files (the "Software"),
> - * to deal in the Software without restriction, including without limitation
> - * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> - * and/or sell copies of the Software, and to permit persons to whom the
> - * Software is furnished to do so, subject to the following conditions:
> - *
> - * The above copyright notice and this permission notice shall be included in
> - * all copies or substantial portions of the Software.
> - *
> - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> - * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
> - * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> - * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> - * OTHER DEALINGS IN THE SOFTWARE.
> - */
> -
> -/*
> - * KFD Interrupts.
> - *
> - * AMD GPUs deliver interrupts by pushing an interrupt description onto the
> - * interrupt ring and then sending an interrupt. KGD receives the interrupt
> - * in ISR and sends us a pointer to each new entry on the interrupt ring.
> - *
> - * We generally can't process interrupt-signaled events from ISR, so we call
> - * out to each interrupt client module (currently only the scheduler) to ask if
> - * each interrupt is interesting. If they return true, then it requires further
> - * processing so we copy it to an internal interrupt ring and call each
> - * interrupt client again from a work-queue.
> - *
> - * There's no acknowledgment for the interrupts we use. The hardware simply
> - * queues a new interrupt each time without waiting.
> - *
> - * The fixed-size internal queue means that it's possible for us to lose
> - * interrupts because we have no back-pressure to the hardware.
> - */
> -
> -#include <linux/slab.h>
> -#include <linux/device.h>
> -#include "kfd_priv.h"
> -
> -#define KFD_INTERRUPT_RING_SIZE 256
> -
> -static void interrupt_wq(struct work_struct *);
> -
> -int kfd_interrupt_init(struct kfd_dev *kfd)
> -{
> -	void *interrupt_ring = kmalloc_array(KFD_INTERRUPT_RING_SIZE,
> -					kfd->device_info->ih_ring_entry_size,
> -					GFP_KERNEL);
> -	if (!interrupt_ring)
> -		return -ENOMEM;
> -
> -	kfd->interrupt_ring = interrupt_ring;
> -	kfd->interrupt_ring_size =
> -		KFD_INTERRUPT_RING_SIZE * kfd->device_info->ih_ring_entry_size;
> -	atomic_set(&kfd->interrupt_ring_wptr, 0);
> -	atomic_set(&kfd->interrupt_ring_rptr, 0);
> -
> -	spin_lock_init(&kfd->interrupt_lock);
> -
> -	INIT_WORK(&kfd->interrupt_work, interrupt_wq);
> -
> -	kfd->interrupts_active = true;
> -
> -	/*
> -	 * After this function returns, the interrupt will be enabled. This
> -	 * barrier ensures that the interrupt running on a different processor
> -	 * sees all the above writes.
> -	 */
> -	smp_wmb();
> -
> -	return 0;
> -}
> -
> -void kfd_interrupt_exit(struct kfd_dev *kfd)
> -{
> -	/*
> -	 * Stop the interrupt handler from writing to the ring and scheduling
> -	 * workqueue items. The spinlock ensures that any interrupt running
> -	 * after we have unlocked sees interrupts_active = false.
> -	 */
> -	unsigned long flags;
> -
> -	spin_lock_irqsave(&kfd->interrupt_lock, flags);
> -	kfd->interrupts_active = false;
> -	spin_unlock_irqrestore(&kfd->interrupt_lock, flags);
> -
> -	/*
> -	 * Flush_scheduled_work ensures that there are no outstanding
> -	 * work-queue items that will access interrupt_ring. New work items
> -	 * can't be created because we stopped interrupt handling above.
> -	 */
> -	flush_scheduled_work();
> -
> -	kfree(kfd->interrupt_ring);
> -}
> -
> -/*
> - * This assumes that it can't be called concurrently with itself
> - * but only with dequeue_ih_ring_entry.
> - */
> -bool enqueue_ih_ring_entry(struct kfd_dev *kfd,	const void *ih_ring_entry)
> -{
> -	unsigned int rptr = atomic_read(&kfd->interrupt_ring_rptr);
> -	unsigned int wptr = atomic_read(&kfd->interrupt_ring_wptr);
> -
> -	if ((rptr - wptr) % kfd->interrupt_ring_size ==
> -					kfd->device_info->ih_ring_entry_size) {
> -		/* This is very bad, the system is likely to hang. */
> -		dev_err_ratelimited(kfd_chardev(),
> -			"Interrupt ring overflow, dropping interrupt.\n");
> -		return false;
> -	}
> -
> -	memcpy(kfd->interrupt_ring + wptr, ih_ring_entry,
> -			kfd->device_info->ih_ring_entry_size);
> -
> -	wptr = (wptr + kfd->device_info->ih_ring_entry_size) %
> -			kfd->interrupt_ring_size;
> -	smp_wmb(); /* Ensure memcpy'd data is visible before wptr update. */
> -	atomic_set(&kfd->interrupt_ring_wptr, wptr);
> -
> -	return true;
> -}
> -
> -/*
> - * This assumes that it can't be called concurrently with itself
> - * but only with enqueue_ih_ring_entry.
> - */
> -static bool dequeue_ih_ring_entry(struct kfd_dev *kfd, void *ih_ring_entry)
> -{
> -	/*
> -	 * Assume that wait queues have an implicit barrier, i.e. anything that
> -	 * happened in the ISR before it queued work is visible.
> -	 */
> -
> -	unsigned int wptr = atomic_read(&kfd->interrupt_ring_wptr);
> -	unsigned int rptr = atomic_read(&kfd->interrupt_ring_rptr);
> -
> -	if (rptr == wptr)
> -		return false;
> -
> -	memcpy(ih_ring_entry, kfd->interrupt_ring + rptr,
> -			kfd->device_info->ih_ring_entry_size);
> -
> -	rptr = (rptr + kfd->device_info->ih_ring_entry_size) %
> -			kfd->interrupt_ring_size;
> -
> -	/*
> -	 * Ensure the rptr write update is not visible until
> -	 * memcpy has finished reading.
> -	 */
> -	smp_mb();
> -	atomic_set(&kfd->interrupt_ring_rptr, rptr);
> -
> -	return true;
> -}
> -
> -static void interrupt_wq(struct work_struct *work)
> -{
> -	struct kfd_dev *dev = container_of(work, struct kfd_dev,
> -						interrupt_work);
> -
> -	uint32_t ih_ring_entry[DIV_ROUND_UP(
> -				dev->device_info->ih_ring_entry_size,
> -				sizeof(uint32_t))];
> -
> -	while (dequeue_ih_ring_entry(dev, ih_ring_entry))
> -		;
> -}
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> index f9fb81e..3a6ac90 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> @@ -135,22 +135,10 @@ struct kfd_dev {
>  
>  	struct kgd2kfd_shared_resources shared_resources;
>  
> -	void *interrupt_ring;
> -	size_t interrupt_ring_size;
> -	atomic_t interrupt_ring_rptr;
> -	atomic_t interrupt_ring_wptr;
> -	struct work_struct interrupt_work;
> -	spinlock_t interrupt_lock;
> -
>  	/* QCM Device instance */
>  	struct device_queue_manager *dqm;
>  
>  	bool init_complete;
> -	/*
> -	 * Interrupts of interest to KFD are copied
> -	 * from the HW ring into a SW ring.
> -	 */
> -	bool interrupts_active;
>  };
>  
>  /* KGD2KFD callbacks */
> @@ -513,10 +501,7 @@ struct kfd_dev *kfd_device_by_pci_dev(const struct pci_dev *pdev);
>  struct kfd_dev *kfd_topology_enum_kfd_devices(uint8_t idx);
>  
>  /* Interrupts */
> -int kfd_interrupt_init(struct kfd_dev *dev);
> -void kfd_interrupt_exit(struct kfd_dev *dev);
>  void kgd2kfd_interrupt(struct kfd_dev *kfd, const void *ih_ring_entry);
> -bool enqueue_ih_ring_entry(struct kfd_dev *kfd,	const void *ih_ring_entry);
>  
>  /* Power Management */
>  void kgd2kfd_suspend(struct kfd_dev *kfd);
>
Michel Dänzer Jan. 8, 2015, 4:02 a.m. UTC | #2
On 07.01.2015 21:24, Oded Gabbay wrote:
> Hi Michel,
> So your patch is quite, hmm, *drastic* :)
> 
> Instead, could I suggest to only remove the calls to kfd_interrupt_init()
> and kfd_interrupt_exit() ? It will also require a minor modification to the
> logic in kgd2kfd_interrupt() but it is much less intrusive than what you are
> suggesting.
> 
> Alternatively, we could take just this hunk:
> 
>> @@ -296,13 +286,5 @@ int kgd2kfd_resume(struct kfd_dev *kfd)
>>  /* This is called directly from KGD at ISR. */
>>  void kgd2kfd_interrupt(struct kfd_dev *kfd, const void *ih_ring_entry)
>>  {
>> -	if (kfd->init_complete) {
>> -		spin_lock(&kfd->interrupt_lock);
>> -
>> -		if (kfd->interrupts_active
>> -		    && enqueue_ih_ring_entry(kfd, ih_ring_entry))
>> -			schedule_work(&kfd->interrupt_work);
>> -
>> -		spin_unlock(&kfd->interrupt_lock);
>> -	}
>> +	/* Process interrupts / schedule work as necessary */
>>  }
> 
> 
> After all, we do need this feature eventually and most of it is fine,

Actually, I'm not sure I can agree with 'most of it is fine'. The whole
design of the SW ring buffer which is processed by a work queue seems
rather flawed, I'm not sure it's possible to reliably prevent the ring
buffer from overflowing like that, even while the hardware interrupt
ring is being processed without problems.


> so I don't want to take it all out. As I said, it is *drastic*.

Why do you think so? It's removing code which is currently essentially
dead but which is causing user visible problems. If it does turn out to
be useful later and the problems can be fixed, it'll be easy to
reinstate it from the Git history.
diff mbox

Patch

diff --git a/drivers/gpu/drm/amd/amdkfd/Makefile b/drivers/gpu/drm/amd/amdkfd/Makefile
index be6246d..307a309 100644
--- a/drivers/gpu/drm/amd/amdkfd/Makefile
+++ b/drivers/gpu/drm/amd/amdkfd/Makefile
@@ -8,7 +8,6 @@  amdkfd-y	:= kfd_module.o kfd_device.o kfd_chardev.o kfd_topology.o \
 		kfd_pasid.o kfd_doorbell.o kfd_flat_memory.o \
 		kfd_process.o kfd_queue.o kfd_mqd_manager.o \
 		kfd_kernel_queue.o kfd_packet_manager.o \
-		kfd_process_queue_manager.o kfd_device_queue_manager.o \
-		kfd_interrupt.o
+		kfd_process_queue_manager.o kfd_device_queue_manager.o
 
 obj-$(CONFIG_HSA_AMD)	+= amdkfd.o
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device.c b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
index 43884eb..633532a 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
@@ -192,13 +192,6 @@  bool kgd2kfd_device_init(struct kfd_dev *kfd,
 		goto kfd_topology_add_device_error;
 	}
 
-	if (kfd_interrupt_init(kfd)) {
-		dev_err(kfd_device,
-			"Error initializing interrupts for device (%x:%x)\n",
-			kfd->pdev->vendor, kfd->pdev->device);
-		goto kfd_interrupt_error;
-	}
-
 	if (!device_iommu_pasid_init(kfd)) {
 		dev_err(kfd_device,
 			"Error initializing iommuv2 for device (%x:%x)\n",
@@ -237,8 +230,6 @@  dqm_start_error:
 device_queue_manager_error:
 	amd_iommu_free_device(kfd->pdev);
 device_iommu_pasid_error:
-	kfd_interrupt_exit(kfd);
-kfd_interrupt_error:
 	kfd_topology_remove_device(kfd);
 kfd_topology_add_device_error:
 	kfd2kgd->fini_sa_manager(kfd->kgd);
@@ -254,7 +245,6 @@  void kgd2kfd_device_exit(struct kfd_dev *kfd)
 	if (kfd->init_complete) {
 		device_queue_manager_uninit(kfd->dqm);
 		amd_iommu_free_device(kfd->pdev);
-		kfd_interrupt_exit(kfd);
 		kfd_topology_remove_device(kfd);
 	}
 
@@ -296,13 +286,5 @@  int kgd2kfd_resume(struct kfd_dev *kfd)
 /* This is called directly from KGD at ISR. */
 void kgd2kfd_interrupt(struct kfd_dev *kfd, const void *ih_ring_entry)
 {
-	if (kfd->init_complete) {
-		spin_lock(&kfd->interrupt_lock);
-
-		if (kfd->interrupts_active
-		    && enqueue_ih_ring_entry(kfd, ih_ring_entry))
-			schedule_work(&kfd->interrupt_work);
-
-		spin_unlock(&kfd->interrupt_lock);
-	}
+	/* Process interrupts / schedule work as necessary */
 }
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_interrupt.c b/drivers/gpu/drm/amd/amdkfd/kfd_interrupt.c
deleted file mode 100644
index 5b99909..0000000
--- a/drivers/gpu/drm/amd/amdkfd/kfd_interrupt.c
+++ /dev/null
@@ -1,176 +0,0 @@ 
-/*
- * Copyright 2014 Advanced Micro Devices, Inc.
- *
- * Permission is hereby granted, free of charge, to any person obtaining a
- * copy of this software and associated documentation files (the "Software"),
- * to deal in the Software without restriction, including without limitation
- * the rights to use, copy, modify, merge, publish, distribute, sublicense,
- * and/or sell copies of the Software, and to permit persons to whom the
- * Software is furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice shall be included in
- * all copies or substantial portions of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
- * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
- * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
- * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
- * OTHER DEALINGS IN THE SOFTWARE.
- */
-
-/*
- * KFD Interrupts.
- *
- * AMD GPUs deliver interrupts by pushing an interrupt description onto the
- * interrupt ring and then sending an interrupt. KGD receives the interrupt
- * in ISR and sends us a pointer to each new entry on the interrupt ring.
- *
- * We generally can't process interrupt-signaled events from ISR, so we call
- * out to each interrupt client module (currently only the scheduler) to ask if
- * each interrupt is interesting. If they return true, then it requires further
- * processing so we copy it to an internal interrupt ring and call each
- * interrupt client again from a work-queue.
- *
- * There's no acknowledgment for the interrupts we use. The hardware simply
- * queues a new interrupt each time without waiting.
- *
- * The fixed-size internal queue means that it's possible for us to lose
- * interrupts because we have no back-pressure to the hardware.
- */
-
-#include <linux/slab.h>
-#include <linux/device.h>
-#include "kfd_priv.h"
-
-#define KFD_INTERRUPT_RING_SIZE 256
-
-static void interrupt_wq(struct work_struct *);
-
-int kfd_interrupt_init(struct kfd_dev *kfd)
-{
-	void *interrupt_ring = kmalloc_array(KFD_INTERRUPT_RING_SIZE,
-					kfd->device_info->ih_ring_entry_size,
-					GFP_KERNEL);
-	if (!interrupt_ring)
-		return -ENOMEM;
-
-	kfd->interrupt_ring = interrupt_ring;
-	kfd->interrupt_ring_size =
-		KFD_INTERRUPT_RING_SIZE * kfd->device_info->ih_ring_entry_size;
-	atomic_set(&kfd->interrupt_ring_wptr, 0);
-	atomic_set(&kfd->interrupt_ring_rptr, 0);
-
-	spin_lock_init(&kfd->interrupt_lock);
-
-	INIT_WORK(&kfd->interrupt_work, interrupt_wq);
-
-	kfd->interrupts_active = true;
-
-	/*
-	 * After this function returns, the interrupt will be enabled. This
-	 * barrier ensures that the interrupt running on a different processor
-	 * sees all the above writes.
-	 */
-	smp_wmb();
-
-	return 0;
-}
-
-void kfd_interrupt_exit(struct kfd_dev *kfd)
-{
-	/*
-	 * Stop the interrupt handler from writing to the ring and scheduling
-	 * workqueue items. The spinlock ensures that any interrupt running
-	 * after we have unlocked sees interrupts_active = false.
-	 */
-	unsigned long flags;
-
-	spin_lock_irqsave(&kfd->interrupt_lock, flags);
-	kfd->interrupts_active = false;
-	spin_unlock_irqrestore(&kfd->interrupt_lock, flags);
-
-	/*
-	 * Flush_scheduled_work ensures that there are no outstanding
-	 * work-queue items that will access interrupt_ring. New work items
-	 * can't be created because we stopped interrupt handling above.
-	 */
-	flush_scheduled_work();
-
-	kfree(kfd->interrupt_ring);
-}
-
-/*
- * This assumes that it can't be called concurrently with itself
- * but only with dequeue_ih_ring_entry.
- */
-bool enqueue_ih_ring_entry(struct kfd_dev *kfd,	const void *ih_ring_entry)
-{
-	unsigned int rptr = atomic_read(&kfd->interrupt_ring_rptr);
-	unsigned int wptr = atomic_read(&kfd->interrupt_ring_wptr);
-
-	if ((rptr - wptr) % kfd->interrupt_ring_size ==
-					kfd->device_info->ih_ring_entry_size) {
-		/* This is very bad, the system is likely to hang. */
-		dev_err_ratelimited(kfd_chardev(),
-			"Interrupt ring overflow, dropping interrupt.\n");
-		return false;
-	}
-
-	memcpy(kfd->interrupt_ring + wptr, ih_ring_entry,
-			kfd->device_info->ih_ring_entry_size);
-
-	wptr = (wptr + kfd->device_info->ih_ring_entry_size) %
-			kfd->interrupt_ring_size;
-	smp_wmb(); /* Ensure memcpy'd data is visible before wptr update. */
-	atomic_set(&kfd->interrupt_ring_wptr, wptr);
-
-	return true;
-}
-
-/*
- * This assumes that it can't be called concurrently with itself
- * but only with enqueue_ih_ring_entry.
- */
-static bool dequeue_ih_ring_entry(struct kfd_dev *kfd, void *ih_ring_entry)
-{
-	/*
-	 * Assume that wait queues have an implicit barrier, i.e. anything that
-	 * happened in the ISR before it queued work is visible.
-	 */
-
-	unsigned int wptr = atomic_read(&kfd->interrupt_ring_wptr);
-	unsigned int rptr = atomic_read(&kfd->interrupt_ring_rptr);
-
-	if (rptr == wptr)
-		return false;
-
-	memcpy(ih_ring_entry, kfd->interrupt_ring + rptr,
-			kfd->device_info->ih_ring_entry_size);
-
-	rptr = (rptr + kfd->device_info->ih_ring_entry_size) %
-			kfd->interrupt_ring_size;
-
-	/*
-	 * Ensure the rptr write update is not visible until
-	 * memcpy has finished reading.
-	 */
-	smp_mb();
-	atomic_set(&kfd->interrupt_ring_rptr, rptr);
-
-	return true;
-}
-
-static void interrupt_wq(struct work_struct *work)
-{
-	struct kfd_dev *dev = container_of(work, struct kfd_dev,
-						interrupt_work);
-
-	uint32_t ih_ring_entry[DIV_ROUND_UP(
-				dev->device_info->ih_ring_entry_size,
-				sizeof(uint32_t))];
-
-	while (dequeue_ih_ring_entry(dev, ih_ring_entry))
-		;
-}
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
index f9fb81e..3a6ac90 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
@@ -135,22 +135,10 @@  struct kfd_dev {
 
 	struct kgd2kfd_shared_resources shared_resources;
 
-	void *interrupt_ring;
-	size_t interrupt_ring_size;
-	atomic_t interrupt_ring_rptr;
-	atomic_t interrupt_ring_wptr;
-	struct work_struct interrupt_work;
-	spinlock_t interrupt_lock;
-
 	/* QCM Device instance */
 	struct device_queue_manager *dqm;
 
 	bool init_complete;
-	/*
-	 * Interrupts of interest to KFD are copied
-	 * from the HW ring into a SW ring.
-	 */
-	bool interrupts_active;
 };
 
 /* KGD2KFD callbacks */
@@ -513,10 +501,7 @@  struct kfd_dev *kfd_device_by_pci_dev(const struct pci_dev *pdev);
 struct kfd_dev *kfd_topology_enum_kfd_devices(uint8_t idx);
 
 /* Interrupts */
-int kfd_interrupt_init(struct kfd_dev *dev);
-void kfd_interrupt_exit(struct kfd_dev *dev);
 void kgd2kfd_interrupt(struct kfd_dev *kfd, const void *ih_ring_entry);
-bool enqueue_ih_ring_entry(struct kfd_dev *kfd,	const void *ih_ring_entry);
 
 /* Power Management */
 void kgd2kfd_suspend(struct kfd_dev *kfd);