[PATCH 1/2] drm/amdgpu: change vm_inv_engs array based on new GFXHUB/MMHUB layout

[PATCH 1/2] drm/amdgpu: change vm_inv_engs array based on new GFXHUB/MMHUB layout

[Yifan Zhang]

vm_inv_engs should be changd accordingly.

Signed-off-by: Yifan Zhang <yifan1.zhang@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
index 8c8a2f37ba33..eeaa8d215801 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
@@ -539,12 +539,14 @@ void amdgpu_gmc_ras_fini(struct amdgpu_device *adev)
 int amdgpu_gmc_allocate_vm_inv_eng(struct amdgpu_device *adev)
 {
 	struct amdgpu_ring *ring;
-	unsigned vm_inv_engs[AMDGPU_MAX_VMHUBS] =
-		{GFXHUB_FREE_VM_INV_ENGS_BITMAP, MMHUB_FREE_VM_INV_ENGS_BITMAP,
-		GFXHUB_FREE_VM_INV_ENGS_BITMAP};
+	unsigned vm_inv_engs[AMDGPU_MAX_VMHUBS];
 	unsigned i;
 	unsigned vmhub, inv_eng;
 
+	vm_inv_engs[AMDGPU_GFXHUB(0)] = GFXHUB_FREE_VM_INV_ENGS_BITMAP;
+	vm_inv_engs[AMDGPU_GFXHUB(1)] = GFXHUB_FREE_VM_INV_ENGS_BITMAP;
+	vm_inv_engs[AMDGPU_MMHUB0(0)] = MMHUB_FREE_VM_INV_ENGS_BITMAP;
+
 	if (adev->enable_mes) {
 		/* reserve engine 5 for firmware */
 		for (vmhub = 0; vmhub < AMDGPU_MAX_VMHUBS; vmhub++)
-- 
2.37.3

[PATCH 2/2] drm/amdgpu: adjust vmhub flush tlb iteration to fit the new GFXHUB/MMHUB layout

[Yifan Zhang]

tlb flush has to be changed for the new mmhub layout

Signed-off-by: Yifan Zhang <yifan1.zhang@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c | 16 ++++++++++++----
 drivers/gpu/drm/amd/amdgpu/gmc_v11_0.c   | 10 +++++++---
 2 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
index 01cb89ffbd56..2383db399c95 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
@@ -160,6 +160,7 @@ void amdgpu_gart_unbind(struct amdgpu_device *adev, uint64_t offset,
 	/* Starting from VEGA10, system bit must be 0 to mean invalid. */
 	uint64_t flags = 0;
 	int idx;
+	struct amdgpu_vmhub *hub;
 
 	if (!adev->gart.ptr)
 		return;
@@ -182,8 +183,11 @@ void amdgpu_gart_unbind(struct amdgpu_device *adev, uint64_t offset,
 	}
 	mb();
 	amdgpu_device_flush_hdp(adev, NULL);
-	for (i = 0; i < adev->num_vmhubs; i++)
-		amdgpu_gmc_flush_gpu_tlb(adev, 0, i, 0);
+	for (i = 0; i < AMDGPU_MAX_VMHUBS; i++) {
+		hub = &adev->vmhub[i];
+		if (hub->vmhub_funcs)
+			amdgpu_gmc_flush_gpu_tlb(adev, 0, i, 0);
+	}
 
 	drm_dev_exit(idx);
 }
@@ -258,14 +262,18 @@ void amdgpu_gart_bind(struct amdgpu_device *adev, uint64_t offset,
 void amdgpu_gart_invalidate_tlb(struct amdgpu_device *adev)
 {
 	int i;
+	struct amdgpu_vmhub *hub;
 
 	if (!adev->gart.ptr)
 		return;
 
 	mb();
 	amdgpu_device_flush_hdp(adev, NULL);
-	for (i = 0; i < adev->num_vmhubs; i++)
-		amdgpu_gmc_flush_gpu_tlb(adev, 0, i, 0);
+	for (i = 0; i < AMDGPU_MAX_VMHUBS; i++) {
+		hub = &adev->vmhub[i];
+		if (hub->vmhub_funcs)
+			amdgpu_gmc_flush_gpu_tlb(adev, 0, i, 0);
+	}
 }
 
 /**
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v11_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v11_0.c
index 90cf79f8ddde..3ee7f5e067fb 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v11_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v11_0.c
@@ -331,6 +331,7 @@ static int gmc_v11_0_flush_gpu_tlb_pasid(struct amdgpu_device *adev,
 	bool ret;
 	struct amdgpu_ring *ring = &adev->gfx.kiq[0].ring;
 	struct amdgpu_kiq *kiq = &adev->gfx.kiq[0];
+	struct amdgpu_vmhub *hub;
 
 	if (amdgpu_emu_mode == 0 && ring->sched.ready) {
 		spin_lock(&adev->gfx.kiq[0].ring_lock);
@@ -362,9 +363,12 @@ static int gmc_v11_0_flush_gpu_tlb_pasid(struct amdgpu_device *adev,
 				&queried_pasid);
 		if (ret	&& queried_pasid == pasid) {
 			if (all_hub) {
-				for (i = 0; i < adev->num_vmhubs; i++)
-					gmc_v11_0_flush_gpu_tlb(adev, vmid,
-							i, flush_type);
+				for (i = 0; i < AMDGPU_MAX_VMHUBS; i++) {
+					hub = &adev->vmhub[i];
+					if (hub->vmhub_funcs)
+						gmc_v11_0_flush_gpu_tlb(adev, vmid,
+								i, flush_type);
+				}
 			} else {
 				gmc_v11_0_flush_gpu_tlb(adev, vmid,
 						AMDGPU_GFXHUB(0), flush_type);
-- 
2.37.3

Re: [PATCH 2/2] drm/amdgpu: adjust vmhub flush tlb iteration to fit the new GFXHUB/MMHUB layout

[Alex Deucher]

Maybe add:
Fixes: dc267018dac1 ("drm/amdgpu: introduce vmhub definition for
multi-partition cases (v3)")
to the series.  With that, the series is:
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

Alex

On Thu, May 4, 2023 at 11:51 AM Yifan Zhang <yifan1.zhang@amd.com> wrote:
>
> tlb flush has to be changed for the new mmhub layout
>
> Signed-off-by: Yifan Zhang <yifan1.zhang@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c | 16 ++++++++++++----
>  drivers/gpu/drm/amd/amdgpu/gmc_v11_0.c   | 10 +++++++---
>  2 files changed, 19 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
> index 01cb89ffbd56..2383db399c95 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
> @@ -160,6 +160,7 @@ void amdgpu_gart_unbind(struct amdgpu_device *adev, uint64_t offset,
>         /* Starting from VEGA10, system bit must be 0 to mean invalid. */
>         uint64_t flags = 0;
>         int idx;
> +       struct amdgpu_vmhub *hub;
>
>         if (!adev->gart.ptr)
>                 return;
> @@ -182,8 +183,11 @@ void amdgpu_gart_unbind(struct amdgpu_device *adev, uint64_t offset,
>         }
>         mb();
>         amdgpu_device_flush_hdp(adev, NULL);
> -       for (i = 0; i < adev->num_vmhubs; i++)
> -               amdgpu_gmc_flush_gpu_tlb(adev, 0, i, 0);
> +       for (i = 0; i < AMDGPU_MAX_VMHUBS; i++) {
> +               hub = &adev->vmhub[i];
> +               if (hub->vmhub_funcs)
> +                       amdgpu_gmc_flush_gpu_tlb(adev, 0, i, 0);
> +       }
>
>         drm_dev_exit(idx);
>  }
> @@ -258,14 +262,18 @@ void amdgpu_gart_bind(struct amdgpu_device *adev, uint64_t offset,
>  void amdgpu_gart_invalidate_tlb(struct amdgpu_device *adev)
>  {
>         int i;
> +       struct amdgpu_vmhub *hub;
>
>         if (!adev->gart.ptr)
>                 return;
>
>         mb();
>         amdgpu_device_flush_hdp(adev, NULL);
> -       for (i = 0; i < adev->num_vmhubs; i++)
> -               amdgpu_gmc_flush_gpu_tlb(adev, 0, i, 0);
> +       for (i = 0; i < AMDGPU_MAX_VMHUBS; i++) {
> +               hub = &adev->vmhub[i];
> +               if (hub->vmhub_funcs)
> +                       amdgpu_gmc_flush_gpu_tlb(adev, 0, i, 0);
> +       }
>  }
>
>  /**
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v11_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v11_0.c
> index 90cf79f8ddde..3ee7f5e067fb 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v11_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v11_0.c
> @@ -331,6 +331,7 @@ static int gmc_v11_0_flush_gpu_tlb_pasid(struct amdgpu_device *adev,
>         bool ret;
>         struct amdgpu_ring *ring = &adev->gfx.kiq[0].ring;
>         struct amdgpu_kiq *kiq = &adev->gfx.kiq[0];
> +       struct amdgpu_vmhub *hub;
>
>         if (amdgpu_emu_mode == 0 && ring->sched.ready) {
>                 spin_lock(&adev->gfx.kiq[0].ring_lock);
> @@ -362,9 +363,12 @@ static int gmc_v11_0_flush_gpu_tlb_pasid(struct amdgpu_device *adev,
>                                 &queried_pasid);
>                 if (ret && queried_pasid == pasid) {
>                         if (all_hub) {
> -                               for (i = 0; i < adev->num_vmhubs; i++)
> -                                       gmc_v11_0_flush_gpu_tlb(adev, vmid,
> -                                                       i, flush_type);
> +                               for (i = 0; i < AMDGPU_MAX_VMHUBS; i++) {
> +                                       hub = &adev->vmhub[i];
> +                                       if (hub->vmhub_funcs)
> +                                               gmc_v11_0_flush_gpu_tlb(adev, vmid,
> +                                                               i, flush_type);
> +                               }
>                         } else {
>                                 gmc_v11_0_flush_gpu_tlb(adev, vmid,
>                                                 AMDGPU_GFXHUB(0), flush_type);
> --
> 2.37.3
>

Re: [PATCH 2/2] drm/amdgpu: adjust vmhub flush tlb iteration to fit the new GFXHUB/MMHUB layout

[Christian König]

Am 04.05.23 um 17:50 schrieb Yifan Zhang:
> tlb flush has to be changed for the new mmhub layout
>
> Signed-off-by: Yifan Zhang <yifan1.zhang@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c | 16 ++++++++++++----
>   drivers/gpu/drm/amd/amdgpu/gmc_v11_0.c   | 10 +++++++---
>   2 files changed, 19 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
> index 01cb89ffbd56..2383db399c95 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
> @@ -160,6 +160,7 @@ void amdgpu_gart_unbind(struct amdgpu_device *adev, uint64_t offset,
>   	/* Starting from VEGA10, system bit must be 0 to mean invalid. */
>   	uint64_t flags = 0;
>   	int idx;
> +	struct amdgpu_vmhub *hub;

Please always define variables as local as possible. E.g. in this case 
not in the function but rather in the loop.

Apart from that it's good practice to keep defines like i, r, idx etc.. 
last. Some upstream maintainer even enforce reverse xmas tree ordering.

Regards,
Christian.


>   
>   	if (!adev->gart.ptr)
>   		return;
> @@ -182,8 +183,11 @@ void amdgpu_gart_unbind(struct amdgpu_device *adev, uint64_t offset,
>   	}
>   	mb();
>   	amdgpu_device_flush_hdp(adev, NULL);
> -	for (i = 0; i < adev->num_vmhubs; i++)
> -		amdgpu_gmc_flush_gpu_tlb(adev, 0, i, 0);
> +	for (i = 0; i < AMDGPU_MAX_VMHUBS; i++) {
> +		hub = &adev->vmhub[i];
> +		if (hub->vmhub_funcs)
> +			amdgpu_gmc_flush_gpu_tlb(adev, 0, i, 0);
> +	}
>   
>   	drm_dev_exit(idx);
>   }
> @@ -258,14 +262,18 @@ void amdgpu_gart_bind(struct amdgpu_device *adev, uint64_t offset,
>   void amdgpu_gart_invalidate_tlb(struct amdgpu_device *adev)
>   {
>   	int i;
> +	struct amdgpu_vmhub *hub;
>   
>   	if (!adev->gart.ptr)
>   		return;
>   
>   	mb();
>   	amdgpu_device_flush_hdp(adev, NULL);
> -	for (i = 0; i < adev->num_vmhubs; i++)
> -		amdgpu_gmc_flush_gpu_tlb(adev, 0, i, 0);
> +	for (i = 0; i < AMDGPU_MAX_VMHUBS; i++) {
> +		hub = &adev->vmhub[i];
> +		if (hub->vmhub_funcs)
> +			amdgpu_gmc_flush_gpu_tlb(adev, 0, i, 0);
> +	}
>   }
>   
>   /**
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v11_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v11_0.c
> index 90cf79f8ddde..3ee7f5e067fb 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v11_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v11_0.c
> @@ -331,6 +331,7 @@ static int gmc_v11_0_flush_gpu_tlb_pasid(struct amdgpu_device *adev,
>   	bool ret;
>   	struct amdgpu_ring *ring = &adev->gfx.kiq[0].ring;
>   	struct amdgpu_kiq *kiq = &adev->gfx.kiq[0];
> +	struct amdgpu_vmhub *hub;
>   
>   	if (amdgpu_emu_mode == 0 && ring->sched.ready) {
>   		spin_lock(&adev->gfx.kiq[0].ring_lock);
> @@ -362,9 +363,12 @@ static int gmc_v11_0_flush_gpu_tlb_pasid(struct amdgpu_device *adev,
>   				&queried_pasid);
>   		if (ret	&& queried_pasid == pasid) {
>   			if (all_hub) {
> -				for (i = 0; i < adev->num_vmhubs; i++)
> -					gmc_v11_0_flush_gpu_tlb(adev, vmid,
> -							i, flush_type);
> +				for (i = 0; i < AMDGPU_MAX_VMHUBS; i++) {
> +					hub = &adev->vmhub[i];
> +					if (hub->vmhub_funcs)
> +						gmc_v11_0_flush_gpu_tlb(adev, vmid,
> +								i, flush_type);
> +				}
>   			} else {
>   				gmc_v11_0_flush_gpu_tlb(adev, vmid,
>   						AMDGPU_GFXHUB(0), flush_type);