[PATCH 1/3] drm/amdgpu: add infrastructure for soft IH ring

[PATCH 1/3] drm/amdgpu: add infrastructure for soft IH ring

[Christian König]

Add a soft IH ring implementation similar to the hardware IH1/2.

This can be used if the hardware delegation of interrupts to IH1/2
doesn't work for some reason.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c  | 29 ++++++++++++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h  |  2 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c | 35 +++++++++++++++++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h |  8 ++++--
 4 files changed, 72 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c
index 111a301ce878..dcd9b4a8e20b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.c
@@ -131,6 +131,35 @@ void amdgpu_ih_ring_fini(struct amdgpu_device *adev, struct amdgpu_ih_ring *ih)
 	}
 }
 
+/**
+ * amdgpu_ih_ring_write - write IV to the ring buffer
+ *
+ * @ih: ih ring to write to
+ * @iv: the iv to write
+ * @num_dw: size of the iv in dw
+ *
+ * Writes an IV to the ring buffer using the CPU and increment the wptr.
+ * Used for testing and delegating IVs to a software ring.
+ */
+void amdgpu_ih_ring_write(struct amdgpu_ih_ring *ih, const uint32_t *iv,
+			  unsigned int num_dw)
+{
+	uint32_t wptr = le32_to_cpu(*ih->wptr_cpu) >> 2;
+	unsigned int i;
+
+	for (i = 0; i < num_dw; ++i)
+	        ih->ring[wptr++] = cpu_to_le32(iv[i]);
+
+	wptr <<= 2;
+	wptr &= ih->ptr_mask;
+
+	/* Only commit the new wptr if we don't overflow */
+	if (wptr != READ_ONCE(ih->rptr)) {
+		wmb();
+		WRITE_ONCE(*ih->wptr_cpu, cpu_to_le32(wptr));
+	}
+}
+
 /**
  * amdgpu_ih_process - interrupt handler
  *
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h
index 4e0bb645176d..3c9cfe7eecff 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ih.h
@@ -72,6 +72,8 @@ struct amdgpu_ih_funcs {
 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);
+void amdgpu_ih_ring_write(struct amdgpu_ih_ring *ih, const uint32_t *iv,
+			  unsigned int num_dw);
 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 300ac73b4738..bea57e8e793f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
@@ -206,6 +206,21 @@ static void amdgpu_irq_handle_ih2(struct work_struct *work)
 	amdgpu_ih_process(adev, &adev->irq.ih2);
 }
 
+/**
+ * amdgpu_irq_handle_ih_soft - kick of processing for ih_soft
+ *
+ * @work: work structure in struct amdgpu_irq
+ *
+ * Kick of processing IH soft ring.
+ */
+static void amdgpu_irq_handle_ih_soft(struct work_struct *work)
+{
+	struct amdgpu_device *adev = container_of(work, struct amdgpu_device,
+						  irq.ih_soft_work);
+
+	amdgpu_ih_process(adev, &adev->irq.ih_soft);
+}
+
 /**
  * amdgpu_msi_ok - check whether MSI functionality is enabled
  *
@@ -281,6 +296,7 @@ int amdgpu_irq_init(struct amdgpu_device *adev)
 
 	INIT_WORK(&adev->irq.ih1_work, amdgpu_irq_handle_ih1);
 	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 */
@@ -413,6 +429,7 @@ void amdgpu_irq_dispatch(struct amdgpu_device *adev,
 	bool handled = false;
 	int r;
 
+	entry.ih = ih;
 	entry.iv_entry = (const uint32_t *)&ih->ring[ring_index];
 	amdgpu_ih_decode_iv(adev, &entry);
 
@@ -450,6 +467,24 @@ void amdgpu_irq_dispatch(struct amdgpu_device *adev,
 		amdgpu_amdkfd_interrupt(adev, entry.iv_entry);
 }
 
+/**
+ * amdgpu_irq_delegate - delegate IV to soft IH ring
+ *
+ * @adev: amdgpu device pointer
+ * @entry: IV entry
+ * @num_dw: size of IV
+ *
+ * Delegate the IV to the soft IH ring and schedule processing of it. Used
+ * if the hardware delegation to IH1 or IH2 doesn't work for some reason.
+ */
+void amdgpu_irq_delegate(struct amdgpu_device *adev,
+			 struct amdgpu_iv_entry *entry,
+			 unsigned int num_dw)
+{
+	amdgpu_ih_ring_write(&adev->irq.ih_soft, entry->iv_entry, num_dw);
+	schedule_work(&adev->irq.ih_soft_work);
+}
+
 /**
  * amdgpu_irq_update - update hardware interrupt state
  *
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h
index c718e94a55c9..ac527e5deae6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.h
@@ -44,6 +44,7 @@ enum amdgpu_interrupt_state {
 };
 
 struct amdgpu_iv_entry {
+	struct amdgpu_ih_ring *ih;
 	unsigned client_id;
 	unsigned src_id;
 	unsigned ring_id;
@@ -88,9 +89,9 @@ struct amdgpu_irq {
 	bool				msi_enabled; /* msi enabled */
 
 	/* interrupt rings */
-	struct amdgpu_ih_ring		ih, ih1, ih2;
+	struct amdgpu_ih_ring		ih, ih1, ih2, ih_soft;
 	const struct amdgpu_ih_funcs    *ih_funcs;
-	struct work_struct		ih1_work, ih2_work;
+	struct work_struct		ih1_work, ih2_work, ih_soft_work;
 	struct amdgpu_irq_src		self_irq;
 
 	/* gen irq stuff */
@@ -109,6 +110,9 @@ int amdgpu_irq_add_id(struct amdgpu_device *adev,
 		      struct amdgpu_irq_src *source);
 void amdgpu_irq_dispatch(struct amdgpu_device *adev,
 			 struct amdgpu_ih_ring *ih);
+void amdgpu_irq_delegate(struct amdgpu_device *adev,
+			 struct amdgpu_iv_entry *entry,
+			 unsigned int num_dw);
 int amdgpu_irq_update(struct amdgpu_device *adev, struct amdgpu_irq_src *src,
 		      unsigned type);
 int amdgpu_irq_get(struct amdgpu_device *adev, struct amdgpu_irq_src *src,
-- 
2.25.1

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

[PATCH 2/3] drm/amdgpu: enabled software IH ring for Vega

[Christian König]

Seems like we won't get the hardware IH1/2 rings on Vega20 working.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/vega10_ih.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/vega10_ih.c b/drivers/gpu/drm/amd/amdgpu/vega10_ih.c
index 407c6093c2ec..cef61dd46a37 100644
--- a/drivers/gpu/drm/amd/amdgpu/vega10_ih.c
+++ b/drivers/gpu/drm/amd/amdgpu/vega10_ih.c
@@ -91,6 +91,9 @@ static void vega10_ih_enable_interrupts(struct amdgpu_device *adev)
 		}
 		adev->irq.ih2.enabled = true;
 	}
+
+	if (adev->irq.ih_soft.ring_size)
+		adev->irq.ih_soft.enabled = true;
 }
 
 /**
@@ -606,6 +609,10 @@ static int vega10_ih_sw_init(void *handle)
 	adev->irq.ih2.use_doorbell = true;
 	adev->irq.ih2.doorbell_index = (adev->doorbell_index.ih + 2) << 1;
 
+	r = amdgpu_ih_ring_init(adev, &adev->irq.ih_soft, PAGE_SIZE, true);
+	if (r)
+		return r;
+
 	r = amdgpu_irq_init(adev);
 
 	return r;
-- 
2.25.1

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

[PATCH 3/3] drm/amdgpu: make sure retry faults are handled in a work item on Vega

[Christian König]

Looks like we can't enabled the IH1/IH2 feature for Vega20, make sure
retry faults are handled on a separate ring anyway.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 31 ++++++++++++++++++++-------
 1 file changed, 23 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
index 0c3421d587e8..32a6c15c2e08 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -524,14 +524,29 @@ static int gmc_v9_0_process_interrupt(struct amdgpu_device *adev,
 	addr = (u64)entry->src_data[0] << 12;
 	addr |= ((u64)entry->src_data[1] & 0xf) << 44;
 
-	if (retry_fault && amdgpu_gmc_filter_faults(adev, addr, entry->pasid,
-						    entry->timestamp))
-		return 1; /* This also prevents sending it to KFD */
-
-	/* If it's the first fault for this address, process it normally */
-	if (retry_fault && !in_interrupt() &&
-	    amdgpu_vm_handle_fault(adev, entry->pasid, addr))
-		return 1; /* This also prevents sending it to KFD */
+	if (retry_fault) {
+		/* Returning 1 here also prevents sending the IV to the KFD */
+
+		/* Process it onyl if it's the first fault for this address */
+		if (entry->ih != &adev->irq.ih_soft &&
+		    amdgpu_gmc_filter_faults(adev, addr, entry->pasid,
+					     entry->timestamp))
+			return 1;
+
+		/* Delegate it to a different ring if the hardware hasn't
+		 * already done it.
+		 */
+		if (in_interrupt()) {
+			amdgpu_irq_delegate(adev, entry, 8);
+			return 1;
+		}
+
+		/* Try to handle the recoverable page faults by filling page
+		 * tables
+		 */
+		if (amdgpu_vm_handle_fault(adev, entry->pasid, addr))
+			return 1;
+	}
 
 	if (!printk_ratelimit())
 		return 0;
-- 
2.25.1

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

Re: [PATCH 2/3] drm/amdgpu: enabled software IH ring for Vega

[Alex Deucher]

On Mon, Nov 2, 2020 at 6:34 AM Christian König
<ckoenig.leichtzumerken@gmail.com> wrote:
>
> Seems like we won't get the hardware IH1/2 rings on Vega20 working.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/vega10_ih.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/vega10_ih.c b/drivers/gpu/drm/amd/amdgpu/vega10_ih.c
> index 407c6093c2ec..cef61dd46a37 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vega10_ih.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vega10_ih.c
> @@ -91,6 +91,9 @@ static void vega10_ih_enable_interrupts(struct amdgpu_device *adev)
>                 }
>                 adev->irq.ih2.enabled = true;
>         }
> +
> +       if (adev->irq.ih_soft.ring_size)
> +               adev->irq.ih_soft.enabled = true;
>  }
>
>  /**
> @@ -606,6 +609,10 @@ static int vega10_ih_sw_init(void *handle)
>         adev->irq.ih2.use_doorbell = true;
>         adev->irq.ih2.doorbell_index = (adev->doorbell_index.ih + 2) << 1;
>
> +       r = amdgpu_ih_ring_init(adev, &adev->irq.ih_soft, PAGE_SIZE, true);
> +       if (r)
> +               return r;
> +

Should we only enable this on vega20?

Alex


>         r = amdgpu_irq_init(adev);
>
>         return r;
> --
> 2.25.1
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/3] drm/amdgpu: enabled software IH ring for Vega

[Felix Kuehling]

Am 2020-11-02 um 1:53 p.m. schrieb Alex Deucher:
> On Mon, Nov 2, 2020 at 6:34 AM Christian König
> <ckoenig.leichtzumerken@gmail.com> wrote:
>> Seems like we won't get the hardware IH1/2 rings on Vega20 working.
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
>> ---
>>  drivers/gpu/drm/amd/amdgpu/vega10_ih.c | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/vega10_ih.c b/drivers/gpu/drm/amd/amdgpu/vega10_ih.c
>> index 407c6093c2ec..cef61dd46a37 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/vega10_ih.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/vega10_ih.c
>> @@ -91,6 +91,9 @@ static void vega10_ih_enable_interrupts(struct amdgpu_device *adev)
>>                 }
>>                 adev->irq.ih2.enabled = true;
>>         }
>> +
>> +       if (adev->irq.ih_soft.ring_size)
>> +               adev->irq.ih_soft.enabled = true;
>>  }
>>
>>  /**
>> @@ -606,6 +609,10 @@ static int vega10_ih_sw_init(void *handle)
>>         adev->irq.ih2.use_doorbell = true;
>>         adev->irq.ih2.doorbell_index = (adev->doorbell_index.ih + 2) << 1;
>>
>> +       r = amdgpu_ih_ring_init(adev, &adev->irq.ih_soft, PAGE_SIZE, true);
>> +       if (r)
>> +               return r;
>> +
> Should we only enable this on vega20?

It affects other GPUs as well. Including probably some Navi GPUs. We'll
probably need a similar change in navi10_ih.c.

Is there a way to reliably detect whether IH redirection works. Or do we
need to allocate the soft IH ring unconditionally?

Regards,
  Felix


>
> Alex
>
>
>>         r = amdgpu_irq_init(adev);
>>
>>         return r;
>> --
>> 2.25.1
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/3] drm/amdgpu: enabled software IH ring for Vega

[Christian König]

Am 02.11.20 um 19:59 schrieb Felix Kuehling:
> Am 2020-11-02 um 1:53 p.m. schrieb Alex Deucher:
>> On Mon, Nov 2, 2020 at 6:34 AM Christian König
>> <ckoenig.leichtzumerken@gmail.com> wrote:
>>> Seems like we won't get the hardware IH1/2 rings on Vega20 working.
>>>
>>> Signed-off-by: Christian König <christian.koenig@amd.com>
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/vega10_ih.c | 7 +++++++
>>>   1 file changed, 7 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/vega10_ih.c b/drivers/gpu/drm/amd/amdgpu/vega10_ih.c
>>> index 407c6093c2ec..cef61dd46a37 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/vega10_ih.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/vega10_ih.c
>>> @@ -91,6 +91,9 @@ static void vega10_ih_enable_interrupts(struct amdgpu_device *adev)
>>>                  }
>>>                  adev->irq.ih2.enabled = true;
>>>          }
>>> +
>>> +       if (adev->irq.ih_soft.ring_size)
>>> +               adev->irq.ih_soft.enabled = true;
>>>   }
>>>
>>>   /**
>>> @@ -606,6 +609,10 @@ static int vega10_ih_sw_init(void *handle)
>>>          adev->irq.ih2.use_doorbell = true;
>>>          adev->irq.ih2.doorbell_index = (adev->doorbell_index.ih + 2) << 1;
>>>
>>> +       r = amdgpu_ih_ring_init(adev, &adev->irq.ih_soft, PAGE_SIZE, true);
>>> +       if (r)
>>> +               return r;
>>> +
>> Should we only enable this on vega20?
> It affects other GPUs as well. Including probably some Navi GPUs. We'll
> probably need a similar change in navi10_ih.c.
>
> Is there a way to reliably detect whether IH redirection works. Or do we
> need to allocate the soft IH ring unconditionally?

We can allocate it unconditionally on Vega and Navi, it's just a single 
page ring buffer which is only used when needed.

What worries me more is that testing shows that I can't even enable IV 
tracing or risk that a single CPU becomes so busy with processing IVs 
that I get "stuck for 23 seconds" warnings.

We *really* need the hardware to work correctly either by using the CAM 
for filtering page faults or by redirecting them to the different IH ring.

Regards,
Christian.

>
> Regards,
>    Felix
>
>
>> Alex
>>
>>
>>>          r = amdgpu_irq_init(adev);
>>>
>>>          return r;
>>> --
>>> 2.25.1
>>>
>>> _______________________________________________
>>> amd-gfx mailing list
>>> amd-gfx@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

Re: [PATCH 2/3] drm/amdgpu: enabled software IH ring for Vega

[Christian König]

Am 02.11.20 um 20:58 schrieb Christian König:
> Am 02.11.20 um 19:59 schrieb Felix Kuehling:
>> Am 2020-11-02 um 1:53 p.m. schrieb Alex Deucher:
>>> On Mon, Nov 2, 2020 at 6:34 AM Christian König
>>> <ckoenig.leichtzumerken@gmail.com> wrote:
>>>> Seems like we won't get the hardware IH1/2 rings on Vega20 working.
>>>>
>>>> Signed-off-by: Christian König <christian.koenig@amd.com>
>>>> ---
>>>>   drivers/gpu/drm/amd/amdgpu/vega10_ih.c | 7 +++++++
>>>>   1 file changed, 7 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/vega10_ih.c 
>>>> b/drivers/gpu/drm/amd/amdgpu/vega10_ih.c
>>>> index 407c6093c2ec..cef61dd46a37 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/vega10_ih.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/vega10_ih.c
>>>> @@ -91,6 +91,9 @@ static void vega10_ih_enable_interrupts(struct 
>>>> amdgpu_device *adev)
>>>>                  }
>>>>                  adev->irq.ih2.enabled = true;
>>>>          }
>>>> +
>>>> +       if (adev->irq.ih_soft.ring_size)
>>>> +               adev->irq.ih_soft.enabled = true;
>>>>   }
>>>>
>>>>   /**
>>>> @@ -606,6 +609,10 @@ static int vega10_ih_sw_init(void *handle)
>>>>          adev->irq.ih2.use_doorbell = true;
>>>>          adev->irq.ih2.doorbell_index = (adev->doorbell_index.ih + 
>>>> 2) << 1;
>>>>
>>>> +       r = amdgpu_ih_ring_init(adev, &adev->irq.ih_soft, 
>>>> PAGE_SIZE, true);
>>>> +       if (r)
>>>> +               return r;
>>>> +
>>> Should we only enable this on vega20?
>> It affects other GPUs as well. Including probably some Navi GPUs. We'll
>> probably need a similar change in navi10_ih.c.

And gmc_v10.c doesn't even have the recoverable page fault handling yet.

Going to add that as well.

Christian.

>> Is there a way to reliably detect whether IH redirection works. Or do we
>> need to allocate the soft IH ring unconditionally?
>
> We can allocate it unconditionally on Vega and Navi, it's just a 
> single page ring buffer which is only used when needed.
>
> What worries me more is that testing shows that I can't even enable IV 
> tracing or risk that a single CPU becomes so busy with processing IVs 
> that I get "stuck for 23 seconds" warnings.
>
> We *really* need the hardware to work correctly either by using the 
> CAM for filtering page faults or by redirecting them to the different 
> IH ring.
>
> Regards,
> Christian.
>
>>
>> Regards,
>>    Felix
>>
>>
>>> Alex
>>>
>>>
>>>>          r = amdgpu_irq_init(adev);
>>>>
>>>>          return r;
>>>> -- 
>>>> 2.25.1
>>>>
>>>> _______________________________________________
>>>> amd-gfx mailing list
>>>> amd-gfx@lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>

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