Re: [PATCH 3/9] drm/amdgpu: cleanup amdgpu_ih.c

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 

Am 26.09.2018 um 10:52 schrieb Huang, Ray:

-----Original Message-----
From: Christian König [mailto:ckoenig.leichtzumerken@xxxxxxxxx]
Sent: Tuesday, September 25, 2018 7:01 PM
To: Huang, Ray <Ray.Huang@xxxxxxx>
Cc: amd-gfx@xxxxxxxxxxxxxxxxxxxxx
Subject: Re: [PATCH 3/9] drm/amdgpu: cleanup amdgpu_ih.c

Am 25.09.2018 um 12:28 schrieb Huang Rui:

On Mon, Sep 24, 2018 at 02:38:14PM +0200, Christian König wrote:

Cleanup amdgpu_ih.c to be able to handle multiple interrupt rings.

Signed-off-by: Christian König <christian.koenig@xxxxxxx>

Reviewed-by: Huang Rui <ray.huang@xxxxxxx>

Will we have multiple interrupt rings in new asic?

Vega already has 3 of them, we just haven't activated the other yet.


Good to know. I just took a look at the spec. IH ring 1 is used for request log and IH ring 2 is used for translation & invalidation log.
They are to support the page migration feature. Do you plan to work on it?
Yes, actually the IH is able to route any IV to any ring based on the client ID, e.g. who is sending the IV.
So I'm currently looking into routing the VM faults to ring 2 as well. That should allow us to better handle them and avoid overwhelming the CPU with an interrupt storm in case of page faults. 

Regards,
Christian.



Thanks,
Ray


Christian.


Thanks,
Ray


---
   drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c  | 152 ++++++++++++++---

---------------

   drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h  |   8 +-
   drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c |   2 +-
   drivers/gpu/drm/amd/amdgpu/cik_ih.c     |   4 +-
   drivers/gpu/drm/amd/amdgpu/cz_ih.c      |   4 +-
   drivers/gpu/drm/amd/amdgpu/iceland_ih.c |   4 +-
   drivers/gpu/drm/amd/amdgpu/si_ih.c      |   4 +-
   drivers/gpu/drm/amd/amdgpu/tonga_ih.c   |   4 +-
   drivers/gpu/drm/amd/amdgpu/vega10_ih.c  |   4 +-
   9 files changed, 84 insertions(+), 102 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c
index 4ed86218cef3..15fb0f9738ab 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c
@@ -26,44 +26,20 @@
   #include "amdgpu_ih.h"
   #include "amdgpu_amdkfd.h"

-/**
- * amdgpu_ih_ring_alloc - allocate memory for the IH ring
- *
- * @adev: amdgpu_device pointer
- *
- * Allocate a ring buffer for the interrupt controller.
- * Returns 0 for success, errors for failure.
- */
-static int amdgpu_ih_ring_alloc(struct amdgpu_device *adev) -{
-	int r;
-
-	/* Allocate ring buffer */
-	if (adev->irq.ih.ring_obj == NULL) {
-		r = amdgpu_bo_create_kernel(adev, adev->irq.ih.ring_size,
-					    PAGE_SIZE,

AMDGPU_GEM_DOMAIN_GTT,

-					    &adev->irq.ih.ring_obj,
-					    &adev->irq.ih.gpu_addr,
-					    (void **)&adev->irq.ih.ring);
-		if (r) {
-			DRM_ERROR("amdgpu: failed to create ih ring buffer

(%d).\n", r);

-			return r;
-		}
-	}
-	return 0;
-}
-
   /**
    * amdgpu_ih_ring_init - initialize the IH state
    *
    * @adev: amdgpu_device pointer
+ * @ih: ih ring to initialize
+ * @ring_size: ring size to allocate
+ * @use_bus_addr: true when we can use dma_alloc_coherent
    *
    * Initializes the IH state and allocates a buffer
    * for the IH ring buffer.
    * Returns 0 for success, errors for failure.
    */
-int amdgpu_ih_ring_init(struct amdgpu_device *adev, unsigned

ring_size,

-			bool use_bus_addr)
+int amdgpu_ih_ring_init(struct amdgpu_device *adev, struct

amdgpu_ih_ring *ih,

+			unsigned ring_size, bool use_bus_addr)
   {
   	u32 rb_bufsz;
   	int r;
@@ -71,70 +47,76 @@ int amdgpu_ih_ring_init(struct amdgpu_device

*adev, unsigned ring_size,

   	/* Align ring size */
   	rb_bufsz = order_base_2(ring_size / 4);
   	ring_size = (1 << rb_bufsz) * 4;
-	adev->irq.ih.ring_size = ring_size;
-	adev->irq.ih.ptr_mask = adev->irq.ih.ring_size - 1;
-	adev->irq.ih.rptr = 0;
-	adev->irq.ih.use_bus_addr = use_bus_addr;
-
-	if (adev->irq.ih.use_bus_addr) {
-		if (!adev->irq.ih.ring) {
-			/* add 8 bytes for the rptr/wptr shadows and
-			 * add them to the end of the ring allocation.
-			 */
-			adev->irq.ih.ring = pci_alloc_consistent(adev->pdev,
-								 adev-

irq.ih.ring_size + 8,

-								 &adev-

irq.ih.rb_dma_addr);

-			if (adev->irq.ih.ring == NULL)
-				return -ENOMEM;
-			memset((void *)adev->irq.ih.ring, 0, adev-

irq.ih.ring_size + 8);

-			adev->irq.ih.wptr_offs = (adev->irq.ih.ring_size / 4) +

0;

-			adev->irq.ih.rptr_offs = (adev->irq.ih.ring_size / 4) +

1;

-		}
-		return 0;
+	ih->ring_size = ring_size;
+	ih->ptr_mask = ih->ring_size - 1;
+	ih->rptr = 0;
+	ih->use_bus_addr = use_bus_addr;
+
+	if (use_bus_addr) {
+		if (ih->ring)
+			return 0;
+
+		/* add 8 bytes for the rptr/wptr shadows and
+		 * add them to the end of the ring allocation.
+		 */
+		ih->ring = dma_alloc_coherent(adev->dev, ih->ring_size + 8,
+					      &ih->rb_dma_addr, GFP_KERNEL);
+		if (ih->ring == NULL)
+			return -ENOMEM;
+
+		memset((void *)ih->ring, 0, ih->ring_size + 8);
+		ih->wptr_offs = (ih->ring_size / 4) + 0;
+		ih->rptr_offs = (ih->ring_size / 4) + 1;
   	} else {
-		r = amdgpu_device_wb_get(adev, &adev->irq.ih.wptr_offs);
+		r = amdgpu_device_wb_get(adev, &ih->wptr_offs);
+		if (r)
+			return r;
+
+		r = amdgpu_device_wb_get(adev, &ih->rptr_offs);
   		if (r) {
-			dev_err(adev->dev, "(%d) ih wptr_offs wb alloc

failed\n", r);

+			amdgpu_device_wb_free(adev, ih->wptr_offs);
   			return r;
   		}

-		r = amdgpu_device_wb_get(adev, &adev->irq.ih.rptr_offs);
+		r = amdgpu_bo_create_kernel(adev, ih->ring_size,

PAGE_SIZE,

+					    AMDGPU_GEM_DOMAIN_GTT,
+					    &ih->ring_obj, &ih->gpu_addr,
+					    (void **)&ih->ring);
   		if (r) {
-			amdgpu_device_wb_free(adev, adev-

irq.ih.wptr_offs);

-			dev_err(adev->dev, "(%d) ih rptr_offs wb alloc

failed\n", r);

+			amdgpu_device_wb_free(adev, ih->rptr_offs);
+			amdgpu_device_wb_free(adev, ih->wptr_offs);
   			return r;
   		}
-
-		return amdgpu_ih_ring_alloc(adev);
   	}
+	return 0;
   }

   /**
    * amdgpu_ih_ring_fini - tear down the IH state
    *
    * @adev: amdgpu_device pointer
+ * @ih: ih ring to tear down
    *
    * Tears down the IH state and frees buffer
    * used for the IH ring buffer.
    */
-void amdgpu_ih_ring_fini(struct amdgpu_device *adev)
+void amdgpu_ih_ring_fini(struct amdgpu_device *adev, struct
+amdgpu_ih_ring *ih)
   {
-	if (adev->irq.ih.use_bus_addr) {
-		if (adev->irq.ih.ring) {
-			/* add 8 bytes for the rptr/wptr shadows and
-			 * add them to the end of the ring allocation.
-			 */
-			pci_free_consistent(adev->pdev, adev-

irq.ih.ring_size + 8,

-					    (void *)adev->irq.ih.ring,
-					    adev->irq.ih.rb_dma_addr);
-			adev->irq.ih.ring = NULL;
-		}
+	if (ih->use_bus_addr) {
+		if (!ih->ring)
+			return;
+
+		/* add 8 bytes for the rptr/wptr shadows and
+		 * add them to the end of the ring allocation.
+		 */
+		dma_free_coherent(adev->dev, ih->ring_size + 8,
+				  (void *)ih->ring, ih->rb_dma_addr);
+		ih->ring = NULL;
   	} else {
-		amdgpu_bo_free_kernel(&adev->irq.ih.ring_obj,
-				      &adev->irq.ih.gpu_addr,
-				      (void **)&adev->irq.ih.ring);
-		amdgpu_device_wb_free(adev, adev->irq.ih.wptr_offs);
-		amdgpu_device_wb_free(adev, adev->irq.ih.rptr_offs);
+		amdgpu_bo_free_kernel(&ih->ring_obj, &ih->gpu_addr,
+				      (void **)&ih->ring);
+		amdgpu_device_wb_free(adev, ih->wptr_offs);
+		amdgpu_device_wb_free(adev, ih->rptr_offs);
   	}
   }

@@ -142,56 +124,56 @@ void amdgpu_ih_ring_fini(struct

amdgpu_device *adev)

    * amdgpu_ih_process - interrupt handler
    *
    * @adev: amdgpu_device pointer
+ * @ih: ih ring to process
    *
    * Interrupt hander (VI), walk the IH ring.
    * Returns irq process return code.
    */
-int amdgpu_ih_process(struct amdgpu_device *adev)
+int amdgpu_ih_process(struct amdgpu_device *adev, struct
+amdgpu_ih_ring *ih)
   {
   	struct amdgpu_iv_entry entry;
   	u32 wptr;

-	if (!adev->irq.ih.enabled || adev->shutdown)
+	if (!ih->enabled || adev->shutdown)
   		return IRQ_NONE;

   	wptr = amdgpu_ih_get_wptr(adev);

   restart_ih:
   	/* is somebody else already processing irqs? */
-	if (atomic_xchg(&adev->irq.ih.lock, 1))
+	if (atomic_xchg(&ih->lock, 1))
   		return IRQ_NONE;

-	DRM_DEBUG("%s: rptr %d, wptr %d\n", __func__, adev->irq.ih.rptr,

wptr);

+	DRM_DEBUG("%s: rptr %d, wptr %d\n", __func__, ih->rptr, wptr);

   	/* Order reading of wptr vs. reading of IH ring data */
   	rmb();

-	while (adev->irq.ih.rptr != wptr) {
-		u32 ring_index = adev->irq.ih.rptr >> 2;
+	while (ih->rptr != wptr) {
+		u32 ring_index = ih->rptr >> 2;

   		/* Prescreening of high-frequency interrupts */
   		if (!amdgpu_ih_prescreen_iv(adev)) {
-			adev->irq.ih.rptr &= adev->irq.ih.ptr_mask;
+			ih->rptr &= ih->ptr_mask;
   			continue;
   		}

   		/* Before dispatching irq to IP blocks, send it to amdkfd */
   		amdgpu_amdkfd_interrupt(adev,
-				(const void *) &adev->irq.ih.ring[ring_index]);
+					(const void *) &ih->ring[ring_index]);

-		entry.iv_entry = (const uint32_t *)
-			&adev->irq.ih.ring[ring_index];
+		entry.iv_entry = (const uint32_t *)&ih->ring[ring_index];
   		amdgpu_ih_decode_iv(adev, &entry);
-		adev->irq.ih.rptr &= adev->irq.ih.ptr_mask;
+		ih->rptr &= ih->ptr_mask;

   		amdgpu_irq_dispatch(adev, &entry);
   	}
   	amdgpu_ih_set_rptr(adev);
-	atomic_set(&adev->irq.ih.lock, 0);
+	atomic_set(&ih->lock, 0);

   	/* make sure wptr hasn't changed while processing */
   	wptr = amdgpu_ih_get_wptr(adev);
-	if (wptr != adev->irq.ih.rptr)
+	if (wptr != ih->rptr)
   		goto restart_ih;

   	return IRQ_HANDLED;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h
index 0d5b3f5201d2..3e55f985005c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h
@@ -82,9 +82,9 @@ struct amdgpu_ih_funcs {
   #define amdgpu_ih_decode_iv(adev, iv) (adev)->irq.ih_funcs-

decode_iv((adev), (iv))

   #define amdgpu_ih_set_rptr(adev)
(adev)->irq.ih_funcs->set_rptr((adev))

-int amdgpu_ih_ring_init(struct amdgpu_device *adev, unsigned

ring_size,

-			bool use_bus_addr);
-void amdgpu_ih_ring_fini(struct amdgpu_device *adev); -int
amdgpu_ih_process(struct amdgpu_device *adev);
+int amdgpu_ih_ring_init(struct amdgpu_device *adev, struct

amdgpu_ih_ring *ih,

+			unsigned ring_size, bool use_bus_addr); void
+amdgpu_ih_ring_fini(struct amdgpu_device *adev, struct
+amdgpu_ih_ring *ih); int amdgpu_ih_process(struct amdgpu_device
+*adev, struct amdgpu_ih_ring *ih);

   #endif
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
index b927e8798534..aaa8545e458a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
@@ -163,7 +163,7 @@ irqreturn_t amdgpu_irq_handler(int irq, void *arg)
   	struct amdgpu_device *adev = dev->dev_private;
   	irqreturn_t ret;

-	ret = amdgpu_ih_process(adev);
+	ret = amdgpu_ih_process(adev, &adev->irq.ih);
   	if (ret == IRQ_HANDLED)
   		pm_runtime_mark_last_busy(dev->dev);
   	return ret;
diff --git a/drivers/gpu/drm/amd/amdgpu/cik_ih.c
b/drivers/gpu/drm/amd/amdgpu/cik_ih.c
index e75183e09820..c37c4b76e7e9 100644
--- a/drivers/gpu/drm/amd/amdgpu/cik_ih.c
+++ b/drivers/gpu/drm/amd/amdgpu/cik_ih.c
@@ -318,7 +318,7 @@ static int cik_ih_sw_init(void *handle)
   	int r;
   	struct amdgpu_device *adev = (struct amdgpu_device *)handle;

-	r = amdgpu_ih_ring_init(adev, 64 * 1024, false);
+	r = amdgpu_ih_ring_init(adev, &adev->irq.ih, 64 * 1024, false);
   	if (r)
   		return r;

@@ -332,7 +332,7 @@ static int cik_ih_sw_fini(void *handle)
   	struct amdgpu_device *adev = (struct amdgpu_device *)handle;

   	amdgpu_irq_fini(adev);
-	amdgpu_ih_ring_fini(adev);
+	amdgpu_ih_ring_fini(adev, &adev->irq.ih);
   	amdgpu_irq_remove_domain(adev);

   	return 0;
diff --git a/drivers/gpu/drm/amd/amdgpu/cz_ih.c
b/drivers/gpu/drm/amd/amdgpu/cz_ih.c
index 9385da1e1e40..306e0bd154fa 100644
--- a/drivers/gpu/drm/amd/amdgpu/cz_ih.c
+++ b/drivers/gpu/drm/amd/amdgpu/cz_ih.c
@@ -297,7 +297,7 @@ static int cz_ih_sw_init(void *handle)
   	int r;
   	struct amdgpu_device *adev = (struct amdgpu_device *)handle;

-	r = amdgpu_ih_ring_init(adev, 64 * 1024, false);
+	r = amdgpu_ih_ring_init(adev, &adev->irq.ih, 64 * 1024, false);
   	if (r)
   		return r;

@@ -311,7 +311,7 @@ static int cz_ih_sw_fini(void *handle)
   	struct amdgpu_device *adev = (struct amdgpu_device *)handle;

   	amdgpu_irq_fini(adev);
-	amdgpu_ih_ring_fini(adev);
+	amdgpu_ih_ring_fini(adev, &adev->irq.ih);
   	amdgpu_irq_remove_domain(adev);

   	return 0;
diff --git a/drivers/gpu/drm/amd/amdgpu/iceland_ih.c
b/drivers/gpu/drm/amd/amdgpu/iceland_ih.c
index 45ef0a818e11..9005deeec612 100644
--- a/drivers/gpu/drm/amd/amdgpu/iceland_ih.c
+++ b/drivers/gpu/drm/amd/amdgpu/iceland_ih.c
@@ -297,7 +297,7 @@ static int iceland_ih_sw_init(void *handle)
   	int r;
   	struct amdgpu_device *adev = (struct amdgpu_device *)handle;

-	r = amdgpu_ih_ring_init(adev, 64 * 1024, false);
+	r = amdgpu_ih_ring_init(adev, &adev->irq.ih, 64 * 1024, false);
   	if (r)
   		return r;

@@ -311,7 +311,7 @@ static int iceland_ih_sw_fini(void *handle)
   	struct amdgpu_device *adev = (struct amdgpu_device *)handle;

   	amdgpu_irq_fini(adev);
-	amdgpu_ih_ring_fini(adev);
+	amdgpu_ih_ring_fini(adev, &adev->irq.ih);
   	amdgpu_irq_remove_domain(adev);

   	return 0;
diff --git a/drivers/gpu/drm/amd/amdgpu/si_ih.c
b/drivers/gpu/drm/amd/amdgpu/si_ih.c
index 97711d327527..acdf6075957a 100644
--- a/drivers/gpu/drm/amd/amdgpu/si_ih.c
+++ b/drivers/gpu/drm/amd/amdgpu/si_ih.c
@@ -170,7 +170,7 @@ static int si_ih_sw_init(void *handle)
   	int r;
   	struct amdgpu_device *adev = (struct amdgpu_device *)handle;

-	r = amdgpu_ih_ring_init(adev, 64 * 1024, false);
+	r = amdgpu_ih_ring_init(adev, &adev->irq.ih, 64 * 1024, false);
   	if (r)
   		return r;

@@ -182,7 +182,7 @@ static int si_ih_sw_fini(void *handle)
   	struct amdgpu_device *adev = (struct amdgpu_device *)handle;

   	amdgpu_irq_fini(adev);
-	amdgpu_ih_ring_fini(adev);
+	amdgpu_ih_ring_fini(adev, &adev->irq.ih);

   	return 0;
   }
diff --git a/drivers/gpu/drm/amd/amdgpu/tonga_ih.c
b/drivers/gpu/drm/amd/amdgpu/tonga_ih.c
index a79a3776888a..83fdf810ffc7 100644
--- a/drivers/gpu/drm/amd/amdgpu/tonga_ih.c
+++ b/drivers/gpu/drm/amd/amdgpu/tonga_ih.c
@@ -317,7 +317,7 @@ static int tonga_ih_sw_init(void *handle)
   	int r;
   	struct amdgpu_device *adev = (struct amdgpu_device *)handle;

-	r = amdgpu_ih_ring_init(adev, 64 * 1024, true);
+	r = amdgpu_ih_ring_init(adev, &adev->irq.ih, 64 * 1024, true);
   	if (r)
   		return r;

@@ -334,7 +334,7 @@ static int tonga_ih_sw_fini(void *handle)
   	struct amdgpu_device *adev = (struct amdgpu_device *)handle;

   	amdgpu_irq_fini(adev);
-	amdgpu_ih_ring_fini(adev);
+	amdgpu_ih_ring_fini(adev, &adev->irq.ih);
   	amdgpu_irq_remove_domain(adev);

   	return 0;
diff --git a/drivers/gpu/drm/amd/amdgpu/vega10_ih.c
b/drivers/gpu/drm/amd/amdgpu/vega10_ih.c
index 37487b4cbd6e..a99f71797aa3 100644
--- a/drivers/gpu/drm/amd/amdgpu/vega10_ih.c
+++ b/drivers/gpu/drm/amd/amdgpu/vega10_ih.c
@@ -380,7 +380,7 @@ static int vega10_ih_sw_init(void *handle)
   	int r;
   	struct amdgpu_device *adev = (struct amdgpu_device *)handle;

-	r = amdgpu_ih_ring_init(adev, 256 * 1024, true);
+	r = amdgpu_ih_ring_init(adev, &adev->irq.ih, 256 * 1024, true);
   	if (r)
   		return r;

@@ -397,7 +397,7 @@ static int vega10_ih_sw_fini(void *handle)
   	struct amdgpu_device *adev = (struct amdgpu_device *)handle;

   	amdgpu_irq_fini(adev);
-	amdgpu_ih_ring_fini(adev);
+	amdgpu_ih_ring_fini(adev, &adev->irq.ih);

   	return 0;
   }
--
2.14.1

_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux