All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/amdgpu: Don't flush HDP on A+A
@ 2021-05-25 17:56 Eric Huang
  2021-05-26 13:55 ` Felix Kuehling
  2021-05-26 13:58 ` Christian König
  0 siblings, 2 replies; 6+ messages in thread
From: Eric Huang @ 2021-05-25 17:56 UTC (permalink / raw)
  To: amd-gfx

With XGMI connection flushing HDP on PCIe is unnecessary,
it is also to optimize memory allocation latency.

Signed-off-by: Eric Huang <jinhuieric.huang@amd.com>

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c
index ac45d9c7a4e9..aefb3d2927d5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c
@@ -108,6 +108,8 @@ static int amdgpu_vm_cpu_update(struct 
amdgpu_vm_update_params *p,
  static int amdgpu_vm_cpu_commit(struct amdgpu_vm_update_params *p,
                                 struct dma_fence **fence)
  {
+       if (p->adev->gmc.xgmi.connected_to_cpu)
+               return 0;
         /* Flush HDP */
         mb();
         amdgpu_asic_flush_hdp(p->adev, NULL);
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH] drm/amdgpu: Don't flush HDP on A+A
  2021-05-25 17:56 [PATCH] drm/amdgpu: Don't flush HDP on A+A Eric Huang
@ 2021-05-26 13:55 ` Felix Kuehling
  2021-05-27 12:14   ` Christian König
  2021-05-26 13:58 ` Christian König
  1 sibling, 1 reply; 6+ messages in thread
From: Felix Kuehling @ 2021-05-26 13:55 UTC (permalink / raw)
  To: Eric Huang, amd-gfx

Am 2021-05-25 um 1:56 p.m. schrieb Eric Huang:
> With XGMI connection flushing HDP on PCIe is unnecessary,
> it is also to optimize memory allocation latency.
>
> Signed-off-by: Eric Huang <jinhuieric.huang@amd.com>
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c
> index ac45d9c7a4e9..aefb3d2927d5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c
> @@ -108,6 +108,8 @@ static int amdgpu_vm_cpu_update(struct
> amdgpu_vm_update_params *p,
>  static int amdgpu_vm_cpu_commit(struct amdgpu_vm_update_params *p,
>                                 struct dma_fence **fence)
>  {
> +       if (p->adev->gmc.xgmi.connected_to_cpu)
> +               return 0;

I wonder if this check should be inside hdp_v4_0_flush_hdp instead so it
catches all unnecessary HDP flushing. On the other hand, that would
still leave the mb(). But that mb() is probably needed anyway to ensure
that the GPU will see any previous memory writes.

Regards,
  Felix


>         /* Flush HDP */
>         mb();
>         amdgpu_asic_flush_hdp(p->adev, NULL);
> _______________________________________________
> 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

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] drm/amdgpu: Don't flush HDP on A+A
  2021-05-25 17:56 [PATCH] drm/amdgpu: Don't flush HDP on A+A Eric Huang
  2021-05-26 13:55 ` Felix Kuehling
@ 2021-05-26 13:58 ` Christian König
  1 sibling, 0 replies; 6+ messages in thread
From: Christian König @ 2021-05-26 13:58 UTC (permalink / raw)
  To: Eric Huang, amd-gfx

Am 25.05.21 um 19:56 schrieb Eric Huang:
> With XGMI connection flushing HDP on PCIe is unnecessary,
> it is also to optimize memory allocation latency.
>
> Signed-off-by: Eric Huang <jinhuieric.huang@amd.com>
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c
> index ac45d9c7a4e9..aefb3d2927d5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c
> @@ -108,6 +108,8 @@ static int amdgpu_vm_cpu_update(struct 
> amdgpu_vm_update_params *p,
>  static int amdgpu_vm_cpu_commit(struct amdgpu_vm_update_params *p,
>                                 struct dma_fence **fence)
>  {
> +       if (p->adev->gmc.xgmi.connected_to_cpu)
> +               return 0;

You still need at least the memory barrier and I suggest to add a common 
flag/function to check if HDP flushing is needed or not.

We also have a bunch of cases in the IB submission code as well.

Christian.

>         /* Flush HDP */
>         mb();
>         amdgpu_asic_flush_hdp(p->adev, NULL);
> _______________________________________________
> 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

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] drm/amdgpu: Don't flush HDP on A+A
  2021-05-26 13:55 ` Felix Kuehling
@ 2021-05-27 12:14   ` Christian König
  0 siblings, 0 replies; 6+ messages in thread
From: Christian König @ 2021-05-27 12:14 UTC (permalink / raw)
  To: Felix Kuehling, Eric Huang, amd-gfx



Am 26.05.21 um 15:55 schrieb Felix Kuehling:
> Am 2021-05-25 um 1:56 p.m. schrieb Eric Huang:
>> With XGMI connection flushing HDP on PCIe is unnecessary,
>> it is also to optimize memory allocation latency.
>>
>> Signed-off-by: Eric Huang <jinhuieric.huang@amd.com>
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c
>> index ac45d9c7a4e9..aefb3d2927d5 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c
>> @@ -108,6 +108,8 @@ static int amdgpu_vm_cpu_update(struct
>> amdgpu_vm_update_params *p,
>>   static int amdgpu_vm_cpu_commit(struct amdgpu_vm_update_params *p,
>>                                  struct dma_fence **fence)
>>   {
>> +       if (p->adev->gmc.xgmi.connected_to_cpu)
>> +               return 0;
> I wonder if this check should be inside hdp_v4_0_flush_hdp instead so it
> catches all unnecessary HDP flushing. On the other hand, that would
> still leave the mb(). But that mb() is probably needed anyway to ensure
> that the GPU will see any previous memory writes.

Yes, that was also my concern. But it looks like my reply never made it 
to the mailing list.

I think we should have a function in amdgpu_device.c to decide if HDP 
flush/Invalidate are necessary or not.

This should then be used at all the different places where we trigger 
this (CS, GART and here).

Regards,
Christian.

>
> Regards,
>    Felix
>
>
>>          /* Flush HDP */
>>          mb();
>>          amdgpu_asic_flush_hdp(p->adev, NULL);
>> _______________________________________________
>> 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

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

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] drm/amdgpu: Don't flush HDP on A+A
  2021-05-31 15:51 Eric Huang
@ 2021-05-31 19:40 ` Christian König
  0 siblings, 0 replies; 6+ messages in thread
From: Christian König @ 2021-05-31 19:40 UTC (permalink / raw)
  To: Eric Huang, amd-gfx

Am 31.05.21 um 17:51 schrieb Eric Huang:
> With XGMI connection flushing HDP on PCIe is unnecessary,
> it is also to optimize memory allocation latency.

Well that's closer to what I had in mind, but not 100% correct.

See the code in amdgpu_ib_schedule() as well:

> #ifdef CONFIG_X86_64
>         if (!(adev->flags & AMD_IS_APU))
> #endif
>         {
>                 if (ring->funcs->emit_hdp_flush)
>                         amdgpu_ring_emit_hdp_flush(ring);
>                 else
>                         amdgpu_asic_flush_hdp(adev, ring);
>         }

I suggest to unify the code here as well.

And in general stuff like that should be in the common code, so that we 
can have the same handling independent of the hardware generation.

Christian.

>
> Signed-off-by: Eric Huang <jinhuieric.huang@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.h | 1 +
>   drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c   | 1 +
>   drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c   | 3 +++
>   3 files changed, 5 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.h
> index 7ec99d591584..1ca23f2f51d2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.h
> @@ -44,6 +44,7 @@ struct amdgpu_hdp {
>   	struct ras_common_if			*ras_if;
>   	const struct amdgpu_hdp_funcs		*funcs;
>   	const struct amdgpu_hdp_ras_funcs	*ras_funcs;
> +	bool	no_flush;
>   };
>   
>   int amdgpu_hdp_ras_late_init(struct amdgpu_device *adev);
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> index 2749621d5f63..6e1eab615914 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> @@ -1223,6 +1223,7 @@ static int gmc_v9_0_early_init(void *handle)
>   		adev->gmc.xgmi.supported = true;
>   		adev->gmc.xgmi.connected_to_cpu =
>   			adev->smuio.funcs->is_host_gpu_xgmi_supported(adev);
> +		adev->hdp.no_flush = adev->gmc.xgmi.connected_to_cpu;
>   	}
>   
>   	gmc_v9_0_set_gmc_funcs(adev);
> diff --git a/drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c b/drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c
> index 74b90cc2bf48..e1b2face8656 100644
> --- a/drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c
> @@ -40,6 +40,9 @@
>   static void hdp_v4_0_flush_hdp(struct amdgpu_device *adev,
>   				struct amdgpu_ring *ring)
>   {
> +	if (adev->hdp.no_flush)
> +		return;
> +
>   	if (!ring || !ring->funcs->emit_wreg)
>   		WREG32_NO_KIQ((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
>   	else

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

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH] drm/amdgpu: Don't flush HDP on A+A
@ 2021-05-31 15:51 Eric Huang
  2021-05-31 19:40 ` Christian König
  0 siblings, 1 reply; 6+ messages in thread
From: Eric Huang @ 2021-05-31 15:51 UTC (permalink / raw)
  To: amd-gfx; +Cc: Eric Huang

With XGMI connection flushing HDP on PCIe is unnecessary,
it is also to optimize memory allocation latency.

Signed-off-by: Eric Huang <jinhuieric.huang@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.h | 1 +
 drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c   | 1 +
 drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c   | 3 +++
 3 files changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.h
index 7ec99d591584..1ca23f2f51d2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_hdp.h
@@ -44,6 +44,7 @@ struct amdgpu_hdp {
 	struct ras_common_if			*ras_if;
 	const struct amdgpu_hdp_funcs		*funcs;
 	const struct amdgpu_hdp_ras_funcs	*ras_funcs;
+	bool	no_flush;
 };
 
 int amdgpu_hdp_ras_late_init(struct amdgpu_device *adev);
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
index 2749621d5f63..6e1eab615914 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -1223,6 +1223,7 @@ static int gmc_v9_0_early_init(void *handle)
 		adev->gmc.xgmi.supported = true;
 		adev->gmc.xgmi.connected_to_cpu =
 			adev->smuio.funcs->is_host_gpu_xgmi_supported(adev);
+		adev->hdp.no_flush = adev->gmc.xgmi.connected_to_cpu;
 	}
 
 	gmc_v9_0_set_gmc_funcs(adev);
diff --git a/drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c b/drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c
index 74b90cc2bf48..e1b2face8656 100644
--- a/drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/hdp_v4_0.c
@@ -40,6 +40,9 @@
 static void hdp_v4_0_flush_hdp(struct amdgpu_device *adev,
 				struct amdgpu_ring *ring)
 {
+	if (adev->hdp.no_flush)
+		return;
+
 	if (!ring || !ring->funcs->emit_wreg)
 		WREG32_NO_KIQ((adev->rmmio_remap.reg_offset + KFD_MMIO_REMAP_HDP_MEM_FLUSH_CNTL) >> 2, 0);
 	else
-- 
2.25.1

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-05-31 19:40 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-25 17:56 [PATCH] drm/amdgpu: Don't flush HDP on A+A Eric Huang
2021-05-26 13:55 ` Felix Kuehling
2021-05-27 12:14   ` Christian König
2021-05-26 13:58 ` Christian König
2021-05-31 15:51 Eric Huang
2021-05-31 19:40 ` Christian König

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.