RE: [PATCH 02/11] drm/amdgpu: send IVs to the KFD only after processing them v2

["Zeng, Oak" <Oak.Zeng@xxxxxxx>]

See comment [Oak]

Thanks,
Oak

-----Original Message-----
From: amd-gfx <amd-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of Christian König
Sent: Friday, November 30, 2018 7:36 AM
To: amd-gfx@xxxxxxxxxxxxxxxxxxxxx
Subject: [PATCH 02/11] drm/amdgpu: send IVs to the KFD only after processing them v2

This allows us to filter out VM faults in the GMC code.

v2: don't filter out all faults

Signed-off-by: Christian König <christian.koenig@xxxxxxx>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c | 29 +++++++++++++++----------
 1 file changed, 17 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
index 6b6524f04ce0..6db4c58ddc13 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
@@ -149,9 +149,6 @@ static void amdgpu_irq_callback(struct amdgpu_device *adev,
 	if (!amdgpu_ih_prescreen_iv(adev))
 		return;
 
-	/* Before dispatching irq to IP blocks, send it to amdkfd */
-	amdgpu_amdkfd_interrupt(adev, (const void *) &ih->ring[ring_index]);
-
 	entry.iv_entry = (const uint32_t *)&ih->ring[ring_index];
 	amdgpu_ih_decode_iv(adev, &entry);
 
@@ -371,29 +368,31 @@ void amdgpu_irq_dispatch(struct amdgpu_device *adev,
 	unsigned client_id = entry->client_id;
 	unsigned src_id = entry->src_id;
 	struct amdgpu_irq_src *src;
+	bool handled = false;
 	int r;
 
 	trace_amdgpu_iv(entry);
 
 	if (client_id >= AMDGPU_IRQ_CLIENTID_MAX) {
-		DRM_DEBUG("Invalid client_id in IV: %d\n", client_id);
+		DRM_ERROR("Invalid client_id in IV: %d\n", client_id);
 		return;
 	}
 
 	if (src_id >= AMDGPU_MAX_IRQ_SRC_ID) {
-		DRM_DEBUG("Invalid src_id in IV: %d\n", src_id);
+		DRM_ERROR("Invalid src_id in IV: %d\n", src_id);
 		return;
 	}
 
 	if (adev->irq.virq[src_id]) {
 		generic_handle_irq(irq_find_mapping(adev->irq.domain, src_id));
-	} else {
-		if (!adev->irq.client[client_id].sources) {
-			DRM_DEBUG("Unregistered interrupt client_id: %d src_id: %d\n",
-				  client_id, src_id);
-			return;
-		}
+		return;
+	}
 
+	if (!adev->irq.client[client_id].sources) {
+		DRM_DEBUG("Unregistered interrupt client_id: %d src_id: %d\n",
+			  client_id, src_id);
+		return;
+	} else {
[Oak]: you probably don't need a else because the return in above if. 
 		src = adev->irq.client[client_id].sources[src_id];
 		if (!src) {
 			DRM_DEBUG("Unhandled interrupt src_id: %d\n", src_id); @@ -401,9 +400,15 @@ void amdgpu_irq_dispatch(struct amdgpu_device *adev,
 		}
 
 		r = src->funcs->process(adev, src, entry);
-		if (r)
+		if (r < 0)
 			DRM_ERROR("error processing interrupt (%d)\n", r);
+		else if (r)
+			handled = true;
 	}
+
+	/* Send it to amdkfd as well if it isn't already handled */
+	if (!handled)
[Oak]: Kfd cares about followed interrupt source/client. How can we guarantee that those are not handled by registered interrupt handler at amdgpu level?
		source_id == SOC15_INTSRC_CP_END_OF_PIPE ||
		source_id == SOC15_INTSRC_SDMA_TRAP ||
		source_id == SOC15_INTSRC_SQ_INTERRUPT_MSG ||
		source_id == SOC15_INTSRC_CP_BAD_OPCODE ||
		client_id == SOC15_IH_CLIENTID_VMC ||
		client_id == SOC15_IH_CLIENTID_UTCL2;

To completely avoid this, we should remove the amdgpu_amdkfd_interrupt function. Instead, kfd interrupt handling functions should be splitted and registered to adev->irq.client[].sources[]

+		amdgpu_amdkfd_interrupt(adev, entry->iv_entry);
 }
 
 /**
--
2.17.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