All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/amdgpu: Default disable GDS for compute+gfx
@ 2019-07-19 23:46 Greathouse, Joseph
       [not found] ` <20190719234612.8198-1-Joseph.Greathouse-5C7GfCeVMHo@public.gmane.org>
  0 siblings, 1 reply; 10+ messages in thread
From: Greathouse, Joseph @ 2019-07-19 23:46 UTC (permalink / raw)
  To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
  Cc: Olsak, Marek, Koenig, Christian, Greathouse, Joseph

Units in the GDS block default to allowing all VMIDs access
to all entries. Disable shader access to the GDS, GWS, and OA
blocks from all compute and gfx VMIDs by default. For compute,
HWS firmware will set up the access bits for the appropriate
VMID when a compute queue requires access to these blocks.
The driver will handle enabling access on-demand for graphics
VMIDs.

Leaving VMID0 with full access because otherwise HWS cannot save
or restore values during task switch.

Signed-off-by: Joseph Greathouse <Joseph.Greathouse@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 22 +++++++++++++++-------
 drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c  | 21 ++++++++++++++-------
 drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c  | 21 ++++++++++++++-------
 drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c  | 21 ++++++++++++++-------
 4 files changed, 57 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
index 73dcb632a3ce..615838a55e8f 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
@@ -1516,17 +1516,24 @@ static void gfx_v10_0_init_compute_vmid(struct amdgpu_device *adev)
 	}
 	nv_grbm_select(adev, 0, 0, 0, 0);
 	mutex_unlock(&adev->srbm_mutex);
+}
 
-	/* Initialize all compute VMIDs to have no GDS, GWS, or OA
-	   acccess. These should be enabled by FW for target VMIDs. */
-	for (i = FIRST_COMPUTE_VMID; i < LAST_COMPUTE_VMID; i++) {
-		WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_BASE, 2 * i, 0);
-		WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_SIZE, 2 * i, 0);
-		WREG32_SOC15_OFFSET(GC, 0, mmGDS_GWS_VMID0, i, 0);
-		WREG32_SOC15_OFFSET(GC, 0, mmGDS_OA_VMID0, i, 0);
+static void gfx_v10_0_init_gds_vmid(struct amdgpu_device *adev)
+{
+	int vmid;
+	/* Initialize all compute and user-gfx VMIDs to have no GDS, GWS, or OA
+	   acccess. Compute VMIDs should be enabled by FW for target VMIDs,
+	   the driver can enable them for graphics. VMID0 should maintain
+	   access so that HWS firmware can save/restore entries. */
+	for (vmid = 1; vmid < 16; vmid++) {
+		WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_BASE, 2 * vmid, 0);
+		WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_SIZE, 2 * vmid, 0);
+		WREG32_SOC15_OFFSET(GC, 0, mmGDS_GWS_VMID0, vmid, 0);
+		WREG32_SOC15_OFFSET(GC, 0, mmGDS_OA_VMID0, vmid, 0);
 	}
 }
 
+
 static void gfx_v10_0_tcp_harvest(struct amdgpu_device *adev)
 {
 	int i, j, k;
@@ -1629,6 +1636,7 @@ static void gfx_v10_0_constants_init(struct amdgpu_device *adev)
 	mutex_unlock(&adev->srbm_mutex);
 
 	gfx_v10_0_init_compute_vmid(adev);
+	gfx_v10_0_init_gds_vmid(adev);
 
 }
 
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
index 3f98624772a4..04e52f7e04ae 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
@@ -1877,14 +1877,20 @@ static void gfx_v7_0_init_compute_vmid(struct amdgpu_device *adev)
 	}
 	cik_srbm_select(adev, 0, 0, 0, 0);
 	mutex_unlock(&adev->srbm_mutex);
+}
 
-	/* Initialize all compute VMIDs to have no GDS, GWS, or OA
-	   acccess. These should be enabled by FW for target VMIDs. */
-	for (i = FIRST_COMPUTE_VMID; i < LAST_COMPUTE_VMID; i++) {
-		WREG32(amdgpu_gds_reg_offset[i].mem_base, 0);
-		WREG32(amdgpu_gds_reg_offset[i].mem_size, 0);
-		WREG32(amdgpu_gds_reg_offset[i].gws, 0);
-		WREG32(amdgpu_gds_reg_offset[i].oa, 0);
+static void gfx_v7_0_init_gds_vmid(struct amdgpu_device *adev)
+{
+	int vmid;
+	/* Initialize all compute and user-gfx VMIDs to have no GDS, GWS, or OA
+	   acccess. Compute VMIDs should be enabled by FW for target VMIDs,
+	   the driver can enable them for graphics. VMID0 should maintain
+	   access so that HWS firmware can save/restore entries. */
+	for (vmid = 1; vmid < 16; vmid++) {
+		WREG32(amdgpu_gds_reg_offset[vmid].mem_base, 0);
+		WREG32(amdgpu_gds_reg_offset[vmid].mem_size, 0);
+		WREG32(amdgpu_gds_reg_offset[vmid].gws, 0);
+		WREG32(amdgpu_gds_reg_offset[vmid].oa, 0);
 	}
 }
 
@@ -1966,6 +1972,7 @@ static void gfx_v7_0_constants_init(struct amdgpu_device *adev)
 	mutex_unlock(&adev->srbm_mutex);
 
 	gfx_v7_0_init_compute_vmid(adev);
+	gfx_v7_0_init_gds_vmid(adev);
 
 	WREG32(mmSX_DEBUG_1, 0x20);
 
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
index e4028d54f8f7..8dc03df48e04 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
@@ -3702,14 +3702,20 @@ static void gfx_v8_0_init_compute_vmid(struct amdgpu_device *adev)
 	}
 	vi_srbm_select(adev, 0, 0, 0, 0);
 	mutex_unlock(&adev->srbm_mutex);
+}
 
-	/* Initialize all compute VMIDs to have no GDS, GWS, or OA
-	   acccess. These should be enabled by FW for target VMIDs. */
-	for (i = FIRST_COMPUTE_VMID; i < LAST_COMPUTE_VMID; i++) {
-		WREG32(amdgpu_gds_reg_offset[i].mem_base, 0);
-		WREG32(amdgpu_gds_reg_offset[i].mem_size, 0);
-		WREG32(amdgpu_gds_reg_offset[i].gws, 0);
-		WREG32(amdgpu_gds_reg_offset[i].oa, 0);
+static void gfx_v8_0_init_gds_vmid(struct amdgpu_device *adev)
+{
+	int vmid;
+	/* Initialize all compute and user-gfx VMIDs to have no GDS, GWS, or OA
+	   acccess. Compute VMIDs should be enabled by FW for target VMIDs,
+	   the driver can enable them for graphics. VMID0 should maintain
+	   access so that HWS firmware can save/restore entries. */
+	for (vmid = 1; vmid < 16; vmid++) {
+                WREG32(amdgpu_gds_reg_offset[vmid].mem_base, 0);
+                WREG32(amdgpu_gds_reg_offset[vmid].mem_size, 0);
+                WREG32(amdgpu_gds_reg_offset[vmid].gws, 0);
+                WREG32(amdgpu_gds_reg_offset[vmid].oa, 0);
 	}
 }
 
@@ -3779,6 +3785,7 @@ static void gfx_v8_0_constants_init(struct amdgpu_device *adev)
 	mutex_unlock(&adev->srbm_mutex);
 
 	gfx_v8_0_init_compute_vmid(adev);
+	gfx_v8_0_init_gds_vmid(adev);
 
 	mutex_lock(&adev->grbm_idx_mutex);
 	/*
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
index 259a35395fec..e0f145ffd597 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
@@ -2025,14 +2025,20 @@ static void gfx_v9_0_init_compute_vmid(struct amdgpu_device *adev)
 	}
 	soc15_grbm_select(adev, 0, 0, 0, 0);
 	mutex_unlock(&adev->srbm_mutex);
+}
 
-	/* Initialize all compute VMIDs to have no GDS, GWS, or OA
-	   acccess. These should be enabled by FW for target VMIDs. */
-	for (i = FIRST_COMPUTE_VMID; i < LAST_COMPUTE_VMID; i++) {
-		WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_BASE, 2 * i, 0);
-		WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_SIZE, 2 * i, 0);
-		WREG32_SOC15_OFFSET(GC, 0, mmGDS_GWS_VMID0, i, 0);
-		WREG32_SOC15_OFFSET(GC, 0, mmGDS_OA_VMID0, i, 0);
+static void gfx_v9_0_init_gds_vmid(struct amdgpu_device *adev)
+{
+	int vmid;
+	/* Initialize all compute and user-gfx VMIDs to have no GDS, GWS, or OA
+	   acccess. Compute VMIDs should be enabled by FW for target VMIDs,
+	   the driver can enable them for graphics. VMID0 should maintain
+	   access so that HWS firmware can save/restore entries. */
+	for (vmid = 1; vmid < 16; vmid++) {
+		WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_BASE, 2 * vmid, 0);
+		WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_SIZE, 2 * vmid, 0);
+		WREG32_SOC15_OFFSET(GC, 0, mmGDS_GWS_VMID0, vmid, 0);
+		WREG32_SOC15_OFFSET(GC, 0, mmGDS_OA_VMID0, vmid, 0);
 	}
 }
 
@@ -2080,6 +2086,7 @@ static void gfx_v9_0_constants_init(struct amdgpu_device *adev)
 	mutex_unlock(&adev->srbm_mutex);
 
 	gfx_v9_0_init_compute_vmid(adev);
+	gfx_v9_0_init_gds_vmid(adev);
 }
 
 static void gfx_v9_0_wait_for_rlc_serdes(struct amdgpu_device *adev)
-- 
2.19.1

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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/amdgpu: Default disable GDS for compute+gfx
       [not found] ` <20190719234612.8198-1-Joseph.Greathouse-5C7GfCeVMHo@public.gmane.org>
@ 2019-07-21 14:51   ` Koenig, Christian
       [not found]     ` <7002f783-2223-a524-2c60-03447377bd28-5C7GfCeVMHo@public.gmane.org>
  0 siblings, 1 reply; 10+ messages in thread
From: Koenig, Christian @ 2019-07-21 14:51 UTC (permalink / raw)
  To: Greathouse, Joseph, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW; +Cc: Olsak, Marek

Am 20.07.19 um 01:46 schrieb Greathouse, Joseph:
> Units in the GDS block default to allowing all VMIDs access
> to all entries. Disable shader access to the GDS, GWS, and OA
> blocks from all compute and gfx VMIDs by default. For compute,
> HWS firmware will set up the access bits for the appropriate
> VMID when a compute queue requires access to these blocks.
> The driver will handle enabling access on-demand for graphics
> VMIDs.
>
> Leaving VMID0 with full access because otherwise HWS cannot save
> or restore values during task switch.
>
> Signed-off-by: Joseph Greathouse <Joseph.Greathouse@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 22 +++++++++++++++-------
>   drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c  | 21 ++++++++++++++-------
>   drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c  | 21 ++++++++++++++-------
>   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c  | 21 ++++++++++++++-------
>   4 files changed, 57 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> index 73dcb632a3ce..615838a55e8f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> @@ -1516,17 +1516,24 @@ static void gfx_v10_0_init_compute_vmid(struct amdgpu_device *adev)
>   	}
>   	nv_grbm_select(adev, 0, 0, 0, 0);
>   	mutex_unlock(&adev->srbm_mutex);
> +}
>   
> -	/* Initialize all compute VMIDs to have no GDS, GWS, or OA
> -	   acccess. These should be enabled by FW for target VMIDs. */
> -	for (i = FIRST_COMPUTE_VMID; i < LAST_COMPUTE_VMID; i++) {
> -		WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_BASE, 2 * i, 0);
> -		WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_SIZE, 2 * i, 0);
> -		WREG32_SOC15_OFFSET(GC, 0, mmGDS_GWS_VMID0, i, 0);
> -		WREG32_SOC15_OFFSET(GC, 0, mmGDS_OA_VMID0, i, 0);
> +static void gfx_v10_0_init_gds_vmid(struct amdgpu_device *adev)
> +{
> +	int vmid;
> +	/* Initialize all compute and user-gfx VMIDs to have no GDS, GWS, or OA
> +	   acccess. Compute VMIDs should be enabled by FW for target VMIDs,
> +	   the driver can enable them for graphics. VMID0 should maintain
> +	   access so that HWS firmware can save/restore entries. */

Please double check the coding style here and all other similar locations.

There should be a blank line between declaration and code (or in this 
case comments). And multi line comments have a specific format.

When you run checkpatch.pl it should complain about that as well.

Apart from that the patch looks good to me,
Christian.

> +	for (vmid = 1; vmid < 16; vmid++) {
> +		WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_BASE, 2 * vmid, 0);
> +		WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_SIZE, 2 * vmid, 0);
> +		WREG32_SOC15_OFFSET(GC, 0, mmGDS_GWS_VMID0, vmid, 0);
> +		WREG32_SOC15_OFFSET(GC, 0, mmGDS_OA_VMID0, vmid, 0);
>   	}
>   }
>   
> +
>   static void gfx_v10_0_tcp_harvest(struct amdgpu_device *adev)
>   {
>   	int i, j, k;
> @@ -1629,6 +1636,7 @@ static void gfx_v10_0_constants_init(struct amdgpu_device *adev)
>   	mutex_unlock(&adev->srbm_mutex);
>   
>   	gfx_v10_0_init_compute_vmid(adev);
> +	gfx_v10_0_init_gds_vmid(adev);
>   
>   }
>   
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> index 3f98624772a4..04e52f7e04ae 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> @@ -1877,14 +1877,20 @@ static void gfx_v7_0_init_compute_vmid(struct amdgpu_device *adev)
>   	}
>   	cik_srbm_select(adev, 0, 0, 0, 0);
>   	mutex_unlock(&adev->srbm_mutex);
> +}
>   
> -	/* Initialize all compute VMIDs to have no GDS, GWS, or OA
> -	   acccess. These should be enabled by FW for target VMIDs. */
> -	for (i = FIRST_COMPUTE_VMID; i < LAST_COMPUTE_VMID; i++) {
> -		WREG32(amdgpu_gds_reg_offset[i].mem_base, 0);
> -		WREG32(amdgpu_gds_reg_offset[i].mem_size, 0);
> -		WREG32(amdgpu_gds_reg_offset[i].gws, 0);
> -		WREG32(amdgpu_gds_reg_offset[i].oa, 0);
> +static void gfx_v7_0_init_gds_vmid(struct amdgpu_device *adev)
> +{
> +	int vmid;
> +	/* Initialize all compute and user-gfx VMIDs to have no GDS, GWS, or OA
> +	   acccess. Compute VMIDs should be enabled by FW for target VMIDs,
> +	   the driver can enable them for graphics. VMID0 should maintain
> +	   access so that HWS firmware can save/restore entries. */
> +	for (vmid = 1; vmid < 16; vmid++) {
> +		WREG32(amdgpu_gds_reg_offset[vmid].mem_base, 0);
> +		WREG32(amdgpu_gds_reg_offset[vmid].mem_size, 0);
> +		WREG32(amdgpu_gds_reg_offset[vmid].gws, 0);
> +		WREG32(amdgpu_gds_reg_offset[vmid].oa, 0);
>   	}
>   }
>   
> @@ -1966,6 +1972,7 @@ static void gfx_v7_0_constants_init(struct amdgpu_device *adev)
>   	mutex_unlock(&adev->srbm_mutex);
>   
>   	gfx_v7_0_init_compute_vmid(adev);
> +	gfx_v7_0_init_gds_vmid(adev);
>   
>   	WREG32(mmSX_DEBUG_1, 0x20);
>   
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> index e4028d54f8f7..8dc03df48e04 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> @@ -3702,14 +3702,20 @@ static void gfx_v8_0_init_compute_vmid(struct amdgpu_device *adev)
>   	}
>   	vi_srbm_select(adev, 0, 0, 0, 0);
>   	mutex_unlock(&adev->srbm_mutex);
> +}
>   
> -	/* Initialize all compute VMIDs to have no GDS, GWS, or OA
> -	   acccess. These should be enabled by FW for target VMIDs. */
> -	for (i = FIRST_COMPUTE_VMID; i < LAST_COMPUTE_VMID; i++) {
> -		WREG32(amdgpu_gds_reg_offset[i].mem_base, 0);
> -		WREG32(amdgpu_gds_reg_offset[i].mem_size, 0);
> -		WREG32(amdgpu_gds_reg_offset[i].gws, 0);
> -		WREG32(amdgpu_gds_reg_offset[i].oa, 0);
> +static void gfx_v8_0_init_gds_vmid(struct amdgpu_device *adev)
> +{
> +	int vmid;
> +	/* Initialize all compute and user-gfx VMIDs to have no GDS, GWS, or OA
> +	   acccess. Compute VMIDs should be enabled by FW for target VMIDs,
> +	   the driver can enable them for graphics. VMID0 should maintain
> +	   access so that HWS firmware can save/restore entries. */
> +	for (vmid = 1; vmid < 16; vmid++) {
> +                WREG32(amdgpu_gds_reg_offset[vmid].mem_base, 0);
> +                WREG32(amdgpu_gds_reg_offset[vmid].mem_size, 0);
> +                WREG32(amdgpu_gds_reg_offset[vmid].gws, 0);
> +                WREG32(amdgpu_gds_reg_offset[vmid].oa, 0);
>   	}
>   }
>   
> @@ -3779,6 +3785,7 @@ static void gfx_v8_0_constants_init(struct amdgpu_device *adev)
>   	mutex_unlock(&adev->srbm_mutex);
>   
>   	gfx_v8_0_init_compute_vmid(adev);
> +	gfx_v8_0_init_gds_vmid(adev);
>   
>   	mutex_lock(&adev->grbm_idx_mutex);
>   	/*
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> index 259a35395fec..e0f145ffd597 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> @@ -2025,14 +2025,20 @@ static void gfx_v9_0_init_compute_vmid(struct amdgpu_device *adev)
>   	}
>   	soc15_grbm_select(adev, 0, 0, 0, 0);
>   	mutex_unlock(&adev->srbm_mutex);
> +}
>   
> -	/* Initialize all compute VMIDs to have no GDS, GWS, or OA
> -	   acccess. These should be enabled by FW for target VMIDs. */
> -	for (i = FIRST_COMPUTE_VMID; i < LAST_COMPUTE_VMID; i++) {
> -		WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_BASE, 2 * i, 0);
> -		WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_SIZE, 2 * i, 0);
> -		WREG32_SOC15_OFFSET(GC, 0, mmGDS_GWS_VMID0, i, 0);
> -		WREG32_SOC15_OFFSET(GC, 0, mmGDS_OA_VMID0, i, 0);
> +static void gfx_v9_0_init_gds_vmid(struct amdgpu_device *adev)
> +{
> +	int vmid;
> +	/* Initialize all compute and user-gfx VMIDs to have no GDS, GWS, or OA
> +	   acccess. Compute VMIDs should be enabled by FW for target VMIDs,
> +	   the driver can enable them for graphics. VMID0 should maintain
> +	   access so that HWS firmware can save/restore entries. */
> +	for (vmid = 1; vmid < 16; vmid++) {
> +		WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_BASE, 2 * vmid, 0);
> +		WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_SIZE, 2 * vmid, 0);
> +		WREG32_SOC15_OFFSET(GC, 0, mmGDS_GWS_VMID0, vmid, 0);
> +		WREG32_SOC15_OFFSET(GC, 0, mmGDS_OA_VMID0, vmid, 0);
>   	}
>   }
>   
> @@ -2080,6 +2086,7 @@ static void gfx_v9_0_constants_init(struct amdgpu_device *adev)
>   	mutex_unlock(&adev->srbm_mutex);
>   
>   	gfx_v9_0_init_compute_vmid(adev);
> +	gfx_v9_0_init_gds_vmid(adev);
>   }
>   
>   static void gfx_v9_0_wait_for_rlc_serdes(struct amdgpu_device *adev)

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2] drm/amdgpu: Default disable GDS for compute+gfx
       [not found]     ` <7002f783-2223-a524-2c60-03447377bd28-5C7GfCeVMHo@public.gmane.org>
@ 2019-07-22 15:34       ` Greathouse, Joseph
       [not found]         ` <20190722153350.29339-1-Joseph.Greathouse-5C7GfCeVMHo@public.gmane.org>
  0 siblings, 1 reply; 10+ messages in thread
From: Greathouse, Joseph @ 2019-07-22 15:34 UTC (permalink / raw)
  To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
  Cc: Olsak, Marek, Koenig, Christian, Greathouse, Joseph

Units in the GDS block default to allowing all VMIDs access to all
entries. Disable shader access to the GDS, GWS, and OA blocks from all
compute and gfx VMIDs by default. For compute, HWS firmware will set
up the access bits for the appropriate VMID when a compute queue
requires access to these blocks.
The driver will handle enabling access on-demand for graphics VMIDs.

Leaving VMID0 with full access because otherwise HWS cannot save or
restore values during task switch.

v2: Fixed code and comment styling.

Change-Id: I3d768a96935d2ed1dff09b02c995090f4fbfa539
Signed-off-by: Joseph Greathouse <Joseph.Greathouse@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 25 ++++++++++++++++++-------
 drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c  | 24 +++++++++++++++++-------
 drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c  | 24 +++++++++++++++++-------
 drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c  | 24 +++++++++++++++++-------
 4 files changed, 69 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
index 73dcb632a3ce..2a9692bc34b4 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
@@ -1516,17 +1516,27 @@ static void gfx_v10_0_init_compute_vmid(struct amdgpu_device *adev)
 	}
 	nv_grbm_select(adev, 0, 0, 0, 0);
 	mutex_unlock(&adev->srbm_mutex);
+}
 
-	/* Initialize all compute VMIDs to have no GDS, GWS, or OA
-	   acccess. These should be enabled by FW for target VMIDs. */
-	for (i = FIRST_COMPUTE_VMID; i < LAST_COMPUTE_VMID; i++) {
-		WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_BASE, 2 * i, 0);
-		WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_SIZE, 2 * i, 0);
-		WREG32_SOC15_OFFSET(GC, 0, mmGDS_GWS_VMID0, i, 0);
-		WREG32_SOC15_OFFSET(GC, 0, mmGDS_OA_VMID0, i, 0);
+static void gfx_v10_0_init_gds_vmid(struct amdgpu_device *adev)
+{
+	int vmid;
+
+	/*
+	 * Initialize all compute and user-gfx VMIDs to have no GDS, GWS, or OA
+	 * access. Compute VMIDs should be enabled by FW for target VMIDs,
+	 * the driver can enable them for graphics. VMID0 should maintain
+	 * access so that HWS firmware can save/restore entries.
+	 */
+	for (vmid = 1; vmid < 16; vmid++) {
+		WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_BASE, 2 * vmid, 0);
+		WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_SIZE, 2 * vmid, 0);
+		WREG32_SOC15_OFFSET(GC, 0, mmGDS_GWS_VMID0, vmid, 0);
+		WREG32_SOC15_OFFSET(GC, 0, mmGDS_OA_VMID0, vmid, 0);
 	}
 }
 
+
 static void gfx_v10_0_tcp_harvest(struct amdgpu_device *adev)
 {
 	int i, j, k;
@@ -1629,6 +1639,7 @@ static void gfx_v10_0_constants_init(struct amdgpu_device *adev)
 	mutex_unlock(&adev->srbm_mutex);
 
 	gfx_v10_0_init_compute_vmid(adev);
+	gfx_v10_0_init_gds_vmid(adev);
 
 }
 
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
index 3f98624772a4..48796b6824cf 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
@@ -1877,14 +1877,23 @@ static void gfx_v7_0_init_compute_vmid(struct amdgpu_device *adev)
 	}
 	cik_srbm_select(adev, 0, 0, 0, 0);
 	mutex_unlock(&adev->srbm_mutex);
+}
 
-	/* Initialize all compute VMIDs to have no GDS, GWS, or OA
-	   acccess. These should be enabled by FW for target VMIDs. */
-	for (i = FIRST_COMPUTE_VMID; i < LAST_COMPUTE_VMID; i++) {
-		WREG32(amdgpu_gds_reg_offset[i].mem_base, 0);
-		WREG32(amdgpu_gds_reg_offset[i].mem_size, 0);
-		WREG32(amdgpu_gds_reg_offset[i].gws, 0);
-		WREG32(amdgpu_gds_reg_offset[i].oa, 0);
+static void gfx_v7_0_init_gds_vmid(struct amdgpu_device *adev)
+{
+	int vmid;
+
+	/*
+	 * Initialize all compute and user-gfx VMIDs to have no GDS, GWS, or OA
+	 * access. Compute VMIDs should be enabled by FW for target VMIDs,
+	 * the driver can enable them for graphics. VMID0 should maintain
+	 * access so that HWS firmware can save/restore entries.
+	 */
+	for (vmid = 1; vmid < 16; vmid++) {
+		WREG32(amdgpu_gds_reg_offset[vmid].mem_base, 0);
+		WREG32(amdgpu_gds_reg_offset[vmid].mem_size, 0);
+		WREG32(amdgpu_gds_reg_offset[vmid].gws, 0);
+		WREG32(amdgpu_gds_reg_offset[vmid].oa, 0);
 	}
 }
 
@@ -1966,6 +1975,7 @@ static void gfx_v7_0_constants_init(struct amdgpu_device *adev)
 	mutex_unlock(&adev->srbm_mutex);
 
 	gfx_v7_0_init_compute_vmid(adev);
+	gfx_v7_0_init_gds_vmid(adev);
 
 	WREG32(mmSX_DEBUG_1, 0x20);
 
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
index e4028d54f8f7..d2907186bb24 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
@@ -3702,14 +3702,23 @@ static void gfx_v8_0_init_compute_vmid(struct amdgpu_device *adev)
 	}
 	vi_srbm_select(adev, 0, 0, 0, 0);
 	mutex_unlock(&adev->srbm_mutex);
+}
 
-	/* Initialize all compute VMIDs to have no GDS, GWS, or OA
-	   acccess. These should be enabled by FW for target VMIDs. */
-	for (i = FIRST_COMPUTE_VMID; i < LAST_COMPUTE_VMID; i++) {
-		WREG32(amdgpu_gds_reg_offset[i].mem_base, 0);
-		WREG32(amdgpu_gds_reg_offset[i].mem_size, 0);
-		WREG32(amdgpu_gds_reg_offset[i].gws, 0);
-		WREG32(amdgpu_gds_reg_offset[i].oa, 0);
+static void gfx_v8_0_init_gds_vmid(struct amdgpu_device *adev)
+{
+	int vmid;
+
+	/*
+	 * Initialize all compute and user-gfx VMIDs to have no GDS, GWS, or OA
+	 * access. Compute VMIDs should be enabled by FW for target VMIDs,
+	 * the driver can enable them for graphics. VMID0 should maintain
+	 * access so that HWS firmware can save/restore entries.
+	 */
+	for (vmid = 1; vmid < 16; vmid++) {
+		WREG32(amdgpu_gds_reg_offset[vmid].mem_base, 0);
+		WREG32(amdgpu_gds_reg_offset[vmid].mem_size, 0);
+		WREG32(amdgpu_gds_reg_offset[vmid].gws, 0);
+		WREG32(amdgpu_gds_reg_offset[vmid].oa, 0);
 	}
 }
 
@@ -3779,6 +3788,7 @@ static void gfx_v8_0_constants_init(struct amdgpu_device *adev)
 	mutex_unlock(&adev->srbm_mutex);
 
 	gfx_v8_0_init_compute_vmid(adev);
+	gfx_v8_0_init_gds_vmid(adev);
 
 	mutex_lock(&adev->grbm_idx_mutex);
 	/*
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
index 259a35395fec..bd7f431a24d9 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
@@ -2025,14 +2025,23 @@ static void gfx_v9_0_init_compute_vmid(struct amdgpu_device *adev)
 	}
 	soc15_grbm_select(adev, 0, 0, 0, 0);
 	mutex_unlock(&adev->srbm_mutex);
+}
 
-	/* Initialize all compute VMIDs to have no GDS, GWS, or OA
-	   acccess. These should be enabled by FW for target VMIDs. */
-	for (i = FIRST_COMPUTE_VMID; i < LAST_COMPUTE_VMID; i++) {
-		WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_BASE, 2 * i, 0);
-		WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_SIZE, 2 * i, 0);
-		WREG32_SOC15_OFFSET(GC, 0, mmGDS_GWS_VMID0, i, 0);
-		WREG32_SOC15_OFFSET(GC, 0, mmGDS_OA_VMID0, i, 0);
+static void gfx_v9_0_init_gds_vmid(struct amdgpu_device *adev)
+{
+	int vmid;
+
+	/*
+	 * Initialize all compute and user-gfx VMIDs to have no GDS, GWS, or OA
+	 * access. Compute VMIDs should be enabled by FW for target VMIDs,
+	 * the driver can enable them for graphics. VMID0 should maintain
+	 * access so that HWS firmware can save/restore entries.
+	 */
+	for (vmid = 1; vmid < 16; vmid++) {
+		WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_BASE, 2 * vmid, 0);
+		WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_SIZE, 2 * vmid, 0);
+		WREG32_SOC15_OFFSET(GC, 0, mmGDS_GWS_VMID0, vmid, 0);
+		WREG32_SOC15_OFFSET(GC, 0, mmGDS_OA_VMID0, vmid, 0);
 	}
 }
 
@@ -2080,6 +2089,7 @@ static void gfx_v9_0_constants_init(struct amdgpu_device *adev)
 	mutex_unlock(&adev->srbm_mutex);
 
 	gfx_v9_0_init_compute_vmid(adev);
+	gfx_v9_0_init_gds_vmid(adev);
 }
 
 static void gfx_v9_0_wait_for_rlc_serdes(struct amdgpu_device *adev)
-- 
2.19.1

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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] drm/amdgpu: Default disable GDS for compute+gfx
       [not found]         ` <20190722153350.29339-1-Joseph.Greathouse-5C7GfCeVMHo@public.gmane.org>
@ 2019-07-22 19:08           ` Christian König
       [not found]             ` <c4856ea0-3cd3-1628-54e9-9660be334054-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  0 siblings, 1 reply; 10+ messages in thread
From: Christian König @ 2019-07-22 19:08 UTC (permalink / raw)
  To: Greathouse, Joseph, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
  Cc: Olsak, Marek, Koenig, Christian

Am 22.07.19 um 17:34 schrieb Greathouse, Joseph:
> Units in the GDS block default to allowing all VMIDs access to all
> entries. Disable shader access to the GDS, GWS, and OA blocks from all
> compute and gfx VMIDs by default. For compute, HWS firmware will set
> up the access bits for the appropriate VMID when a compute queue
> requires access to these blocks.
> The driver will handle enabling access on-demand for graphics VMIDs.
>
> Leaving VMID0 with full access because otherwise HWS cannot save or
> restore values during task switch.
>
> v2: Fixed code and comment styling.
>
> Change-Id: I3d768a96935d2ed1dff09b02c995090f4fbfa539
> Signed-off-by: Joseph Greathouse <Joseph.Greathouse@amd.com>

Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>   drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 25 ++++++++++++++++++-------
>   drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c  | 24 +++++++++++++++++-------
>   drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c  | 24 +++++++++++++++++-------
>   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c  | 24 +++++++++++++++++-------
>   4 files changed, 69 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> index 73dcb632a3ce..2a9692bc34b4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> @@ -1516,17 +1516,27 @@ static void gfx_v10_0_init_compute_vmid(struct amdgpu_device *adev)
>   	}
>   	nv_grbm_select(adev, 0, 0, 0, 0);
>   	mutex_unlock(&adev->srbm_mutex);
> +}
>   
> -	/* Initialize all compute VMIDs to have no GDS, GWS, or OA
> -	   acccess. These should be enabled by FW for target VMIDs. */
> -	for (i = FIRST_COMPUTE_VMID; i < LAST_COMPUTE_VMID; i++) {
> -		WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_BASE, 2 * i, 0);
> -		WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_SIZE, 2 * i, 0);
> -		WREG32_SOC15_OFFSET(GC, 0, mmGDS_GWS_VMID0, i, 0);
> -		WREG32_SOC15_OFFSET(GC, 0, mmGDS_OA_VMID0, i, 0);
> +static void gfx_v10_0_init_gds_vmid(struct amdgpu_device *adev)
> +{
> +	int vmid;
> +
> +	/*
> +	 * Initialize all compute and user-gfx VMIDs to have no GDS, GWS, or OA
> +	 * access. Compute VMIDs should be enabled by FW for target VMIDs,
> +	 * the driver can enable them for graphics. VMID0 should maintain
> +	 * access so that HWS firmware can save/restore entries.
> +	 */
> +	for (vmid = 1; vmid < 16; vmid++) {
> +		WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_BASE, 2 * vmid, 0);
> +		WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_SIZE, 2 * vmid, 0);
> +		WREG32_SOC15_OFFSET(GC, 0, mmGDS_GWS_VMID0, vmid, 0);
> +		WREG32_SOC15_OFFSET(GC, 0, mmGDS_OA_VMID0, vmid, 0);
>   	}
>   }
>   
> +
>   static void gfx_v10_0_tcp_harvest(struct amdgpu_device *adev)
>   {
>   	int i, j, k;
> @@ -1629,6 +1639,7 @@ static void gfx_v10_0_constants_init(struct amdgpu_device *adev)
>   	mutex_unlock(&adev->srbm_mutex);
>   
>   	gfx_v10_0_init_compute_vmid(adev);
> +	gfx_v10_0_init_gds_vmid(adev);
>   
>   }
>   
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> index 3f98624772a4..48796b6824cf 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> @@ -1877,14 +1877,23 @@ static void gfx_v7_0_init_compute_vmid(struct amdgpu_device *adev)
>   	}
>   	cik_srbm_select(adev, 0, 0, 0, 0);
>   	mutex_unlock(&adev->srbm_mutex);
> +}
>   
> -	/* Initialize all compute VMIDs to have no GDS, GWS, or OA
> -	   acccess. These should be enabled by FW for target VMIDs. */
> -	for (i = FIRST_COMPUTE_VMID; i < LAST_COMPUTE_VMID; i++) {
> -		WREG32(amdgpu_gds_reg_offset[i].mem_base, 0);
> -		WREG32(amdgpu_gds_reg_offset[i].mem_size, 0);
> -		WREG32(amdgpu_gds_reg_offset[i].gws, 0);
> -		WREG32(amdgpu_gds_reg_offset[i].oa, 0);
> +static void gfx_v7_0_init_gds_vmid(struct amdgpu_device *adev)
> +{
> +	int vmid;
> +
> +	/*
> +	 * Initialize all compute and user-gfx VMIDs to have no GDS, GWS, or OA
> +	 * access. Compute VMIDs should be enabled by FW for target VMIDs,
> +	 * the driver can enable them for graphics. VMID0 should maintain
> +	 * access so that HWS firmware can save/restore entries.
> +	 */
> +	for (vmid = 1; vmid < 16; vmid++) {
> +		WREG32(amdgpu_gds_reg_offset[vmid].mem_base, 0);
> +		WREG32(amdgpu_gds_reg_offset[vmid].mem_size, 0);
> +		WREG32(amdgpu_gds_reg_offset[vmid].gws, 0);
> +		WREG32(amdgpu_gds_reg_offset[vmid].oa, 0);
>   	}
>   }
>   
> @@ -1966,6 +1975,7 @@ static void gfx_v7_0_constants_init(struct amdgpu_device *adev)
>   	mutex_unlock(&adev->srbm_mutex);
>   
>   	gfx_v7_0_init_compute_vmid(adev);
> +	gfx_v7_0_init_gds_vmid(adev);
>   
>   	WREG32(mmSX_DEBUG_1, 0x20);
>   
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> index e4028d54f8f7..d2907186bb24 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> @@ -3702,14 +3702,23 @@ static void gfx_v8_0_init_compute_vmid(struct amdgpu_device *adev)
>   	}
>   	vi_srbm_select(adev, 0, 0, 0, 0);
>   	mutex_unlock(&adev->srbm_mutex);
> +}
>   
> -	/* Initialize all compute VMIDs to have no GDS, GWS, or OA
> -	   acccess. These should be enabled by FW for target VMIDs. */
> -	for (i = FIRST_COMPUTE_VMID; i < LAST_COMPUTE_VMID; i++) {
> -		WREG32(amdgpu_gds_reg_offset[i].mem_base, 0);
> -		WREG32(amdgpu_gds_reg_offset[i].mem_size, 0);
> -		WREG32(amdgpu_gds_reg_offset[i].gws, 0);
> -		WREG32(amdgpu_gds_reg_offset[i].oa, 0);
> +static void gfx_v8_0_init_gds_vmid(struct amdgpu_device *adev)
> +{
> +	int vmid;
> +
> +	/*
> +	 * Initialize all compute and user-gfx VMIDs to have no GDS, GWS, or OA
> +	 * access. Compute VMIDs should be enabled by FW for target VMIDs,
> +	 * the driver can enable them for graphics. VMID0 should maintain
> +	 * access so that HWS firmware can save/restore entries.
> +	 */
> +	for (vmid = 1; vmid < 16; vmid++) {
> +		WREG32(amdgpu_gds_reg_offset[vmid].mem_base, 0);
> +		WREG32(amdgpu_gds_reg_offset[vmid].mem_size, 0);
> +		WREG32(amdgpu_gds_reg_offset[vmid].gws, 0);
> +		WREG32(amdgpu_gds_reg_offset[vmid].oa, 0);
>   	}
>   }
>   
> @@ -3779,6 +3788,7 @@ static void gfx_v8_0_constants_init(struct amdgpu_device *adev)
>   	mutex_unlock(&adev->srbm_mutex);
>   
>   	gfx_v8_0_init_compute_vmid(adev);
> +	gfx_v8_0_init_gds_vmid(adev);
>   
>   	mutex_lock(&adev->grbm_idx_mutex);
>   	/*
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> index 259a35395fec..bd7f431a24d9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> @@ -2025,14 +2025,23 @@ static void gfx_v9_0_init_compute_vmid(struct amdgpu_device *adev)
>   	}
>   	soc15_grbm_select(adev, 0, 0, 0, 0);
>   	mutex_unlock(&adev->srbm_mutex);
> +}
>   
> -	/* Initialize all compute VMIDs to have no GDS, GWS, or OA
> -	   acccess. These should be enabled by FW for target VMIDs. */
> -	for (i = FIRST_COMPUTE_VMID; i < LAST_COMPUTE_VMID; i++) {
> -		WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_BASE, 2 * i, 0);
> -		WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_SIZE, 2 * i, 0);
> -		WREG32_SOC15_OFFSET(GC, 0, mmGDS_GWS_VMID0, i, 0);
> -		WREG32_SOC15_OFFSET(GC, 0, mmGDS_OA_VMID0, i, 0);
> +static void gfx_v9_0_init_gds_vmid(struct amdgpu_device *adev)
> +{
> +	int vmid;
> +
> +	/*
> +	 * Initialize all compute and user-gfx VMIDs to have no GDS, GWS, or OA
> +	 * access. Compute VMIDs should be enabled by FW for target VMIDs,
> +	 * the driver can enable them for graphics. VMID0 should maintain
> +	 * access so that HWS firmware can save/restore entries.
> +	 */
> +	for (vmid = 1; vmid < 16; vmid++) {
> +		WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_BASE, 2 * vmid, 0);
> +		WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_SIZE, 2 * vmid, 0);
> +		WREG32_SOC15_OFFSET(GC, 0, mmGDS_GWS_VMID0, vmid, 0);
> +		WREG32_SOC15_OFFSET(GC, 0, mmGDS_OA_VMID0, vmid, 0);
>   	}
>   }
>   
> @@ -2080,6 +2089,7 @@ static void gfx_v9_0_constants_init(struct amdgpu_device *adev)
>   	mutex_unlock(&adev->srbm_mutex);
>   
>   	gfx_v9_0_init_compute_vmid(adev);
> +	gfx_v9_0_init_gds_vmid(adev);
>   }
>   
>   static void gfx_v9_0_wait_for_rlc_serdes(struct amdgpu_device *adev)

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] drm/amdgpu: Default disable GDS for compute+gfx
       [not found]             ` <c4856ea0-3cd3-1628-54e9-9660be334054-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2019-08-28  6:41               ` zhoucm1
       [not found]                 ` <36885047-8915-fb90-fcaa-04f98b4519b1-5C7GfCeVMHo@public.gmane.org>
  0 siblings, 1 reply; 10+ messages in thread
From: zhoucm1 @ 2019-08-28  6:41 UTC (permalink / raw)
  To: christian.koenig-5C7GfCeVMHo, Greathouse, Joseph,
	amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
  Cc: Olsak, Marek


On 2019/7/23 上午3:08, Christian König wrote:
> Am 22.07.19 um 17:34 schrieb Greathouse, Joseph:
>> Units in the GDS block default to allowing all VMIDs access to all
>> entries. Disable shader access to the GDS, GWS, and OA blocks from all
>> compute and gfx VMIDs by default. For compute, HWS firmware will set
>> up the access bits for the appropriate VMID when a compute queue
>> requires access to these blocks.
>> The driver will handle enabling access on-demand for graphics VMIDs.

gds_switch is depending on job->gds/gws/oa/_base/size.

"[PATCH] drm/amdgpu: remove static GDS, GWS and OA allocation", the 
default allocations in kernel were removed. If some UMD stacks don't 
pass gds/gws/oa allocation to bo_list, then kernel will not enable 
access of them, that will break previous driver.

do we need revert "[PATCH] drm/amdgpu: remove static GDS, GWS and OA 
allocation" ?

-David

>>
>> Leaving VMID0 with full access because otherwise HWS cannot save or
>> restore values during task switch.
>>
>> v2: Fixed code and comment styling.
>>
>> Change-Id: I3d768a96935d2ed1dff09b02c995090f4fbfa539
>> Signed-off-by: Joseph Greathouse <Joseph.Greathouse@amd.com>
>
> Reviewed-by: Christian König <christian.koenig@amd.com>
>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 25 ++++++++++++++++++-------
>>   drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c  | 24 +++++++++++++++++-------
>>   drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c  | 24 +++++++++++++++++-------
>>   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c  | 24 +++++++++++++++++-------
>>   4 files changed, 69 insertions(+), 28 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c 
>> b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
>> index 73dcb632a3ce..2a9692bc34b4 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
>> @@ -1516,17 +1516,27 @@ static void 
>> gfx_v10_0_init_compute_vmid(struct amdgpu_device *adev)
>>       }
>>       nv_grbm_select(adev, 0, 0, 0, 0);
>>       mutex_unlock(&adev->srbm_mutex);
>> +}
>>   -    /* Initialize all compute VMIDs to have no GDS, GWS, or OA
>> -       acccess. These should be enabled by FW for target VMIDs. */
>> -    for (i = FIRST_COMPUTE_VMID; i < LAST_COMPUTE_VMID; i++) {
>> -        WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_BASE, 2 * i, 0);
>> -        WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_SIZE, 2 * i, 0);
>> -        WREG32_SOC15_OFFSET(GC, 0, mmGDS_GWS_VMID0, i, 0);
>> -        WREG32_SOC15_OFFSET(GC, 0, mmGDS_OA_VMID0, i, 0);
>> +static void gfx_v10_0_init_gds_vmid(struct amdgpu_device *adev)
>> +{
>> +    int vmid;
>> +
>> +    /*
>> +     * Initialize all compute and user-gfx VMIDs to have no GDS, 
>> GWS, or OA
>> +     * access. Compute VMIDs should be enabled by FW for target VMIDs,
>> +     * the driver can enable them for graphics. VMID0 should maintain
>> +     * access so that HWS firmware can save/restore entries.
>> +     */
>> +    for (vmid = 1; vmid < 16; vmid++) {
>> +        WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_BASE, 2 * vmid, 0);
>> +        WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_SIZE, 2 * vmid, 0);
>> +        WREG32_SOC15_OFFSET(GC, 0, mmGDS_GWS_VMID0, vmid, 0);
>> +        WREG32_SOC15_OFFSET(GC, 0, mmGDS_OA_VMID0, vmid, 0);
>>       }
>>   }
>>   +
>>   static void gfx_v10_0_tcp_harvest(struct amdgpu_device *adev)
>>   {
>>       int i, j, k;
>> @@ -1629,6 +1639,7 @@ static void gfx_v10_0_constants_init(struct 
>> amdgpu_device *adev)
>>       mutex_unlock(&adev->srbm_mutex);
>>         gfx_v10_0_init_compute_vmid(adev);
>> +    gfx_v10_0_init_gds_vmid(adev);
>>     }
>>   diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c 
>> b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
>> index 3f98624772a4..48796b6824cf 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
>> @@ -1877,14 +1877,23 @@ static void gfx_v7_0_init_compute_vmid(struct 
>> amdgpu_device *adev)
>>       }
>>       cik_srbm_select(adev, 0, 0, 0, 0);
>>       mutex_unlock(&adev->srbm_mutex);
>> +}
>>   -    /* Initialize all compute VMIDs to have no GDS, GWS, or OA
>> -       acccess. These should be enabled by FW for target VMIDs. */
>> -    for (i = FIRST_COMPUTE_VMID; i < LAST_COMPUTE_VMID; i++) {
>> -        WREG32(amdgpu_gds_reg_offset[i].mem_base, 0);
>> -        WREG32(amdgpu_gds_reg_offset[i].mem_size, 0);
>> -        WREG32(amdgpu_gds_reg_offset[i].gws, 0);
>> -        WREG32(amdgpu_gds_reg_offset[i].oa, 0);
>> +static void gfx_v7_0_init_gds_vmid(struct amdgpu_device *adev)
>> +{
>> +    int vmid;
>> +
>> +    /*
>> +     * Initialize all compute and user-gfx VMIDs to have no GDS, 
>> GWS, or OA
>> +     * access. Compute VMIDs should be enabled by FW for target VMIDs,
>> +     * the driver can enable them for graphics. VMID0 should maintain
>> +     * access so that HWS firmware can save/restore entries.
>> +     */
>> +    for (vmid = 1; vmid < 16; vmid++) {
>> +        WREG32(amdgpu_gds_reg_offset[vmid].mem_base, 0);
>> +        WREG32(amdgpu_gds_reg_offset[vmid].mem_size, 0);
>> +        WREG32(amdgpu_gds_reg_offset[vmid].gws, 0);
>> +        WREG32(amdgpu_gds_reg_offset[vmid].oa, 0);
>>       }
>>   }
>>   @@ -1966,6 +1975,7 @@ static void gfx_v7_0_constants_init(struct 
>> amdgpu_device *adev)
>>       mutex_unlock(&adev->srbm_mutex);
>>         gfx_v7_0_init_compute_vmid(adev);
>> +    gfx_v7_0_init_gds_vmid(adev);
>>         WREG32(mmSX_DEBUG_1, 0x20);
>>   diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c 
>> b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
>> index e4028d54f8f7..d2907186bb24 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
>> @@ -3702,14 +3702,23 @@ static void gfx_v8_0_init_compute_vmid(struct 
>> amdgpu_device *adev)
>>       }
>>       vi_srbm_select(adev, 0, 0, 0, 0);
>>       mutex_unlock(&adev->srbm_mutex);
>> +}
>>   -    /* Initialize all compute VMIDs to have no GDS, GWS, or OA
>> -       acccess. These should be enabled by FW for target VMIDs. */
>> -    for (i = FIRST_COMPUTE_VMID; i < LAST_COMPUTE_VMID; i++) {
>> -        WREG32(amdgpu_gds_reg_offset[i].mem_base, 0);
>> -        WREG32(amdgpu_gds_reg_offset[i].mem_size, 0);
>> -        WREG32(amdgpu_gds_reg_offset[i].gws, 0);
>> -        WREG32(amdgpu_gds_reg_offset[i].oa, 0);
>> +static void gfx_v8_0_init_gds_vmid(struct amdgpu_device *adev)
>> +{
>> +    int vmid;
>> +
>> +    /*
>> +     * Initialize all compute and user-gfx VMIDs to have no GDS, 
>> GWS, or OA
>> +     * access. Compute VMIDs should be enabled by FW for target VMIDs,
>> +     * the driver can enable them for graphics. VMID0 should maintain
>> +     * access so that HWS firmware can save/restore entries.
>> +     */
>> +    for (vmid = 1; vmid < 16; vmid++) {
>> +        WREG32(amdgpu_gds_reg_offset[vmid].mem_base, 0);
>> +        WREG32(amdgpu_gds_reg_offset[vmid].mem_size, 0);
>> +        WREG32(amdgpu_gds_reg_offset[vmid].gws, 0);
>> +        WREG32(amdgpu_gds_reg_offset[vmid].oa, 0);
>>       }
>>   }
>>   @@ -3779,6 +3788,7 @@ static void gfx_v8_0_constants_init(struct 
>> amdgpu_device *adev)
>>       mutex_unlock(&adev->srbm_mutex);
>>         gfx_v8_0_init_compute_vmid(adev);
>> +    gfx_v8_0_init_gds_vmid(adev);
>>         mutex_lock(&adev->grbm_idx_mutex);
>>       /*
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c 
>> b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>> index 259a35395fec..bd7f431a24d9 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>> @@ -2025,14 +2025,23 @@ static void gfx_v9_0_init_compute_vmid(struct 
>> amdgpu_device *adev)
>>       }
>>       soc15_grbm_select(adev, 0, 0, 0, 0);
>>       mutex_unlock(&adev->srbm_mutex);
>> +}
>>   -    /* Initialize all compute VMIDs to have no GDS, GWS, or OA
>> -       acccess. These should be enabled by FW for target VMIDs. */
>> -    for (i = FIRST_COMPUTE_VMID; i < LAST_COMPUTE_VMID; i++) {
>> -        WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_BASE, 2 * i, 0);
>> -        WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_SIZE, 2 * i, 0);
>> -        WREG32_SOC15_OFFSET(GC, 0, mmGDS_GWS_VMID0, i, 0);
>> -        WREG32_SOC15_OFFSET(GC, 0, mmGDS_OA_VMID0, i, 0);
>> +static void gfx_v9_0_init_gds_vmid(struct amdgpu_device *adev)
>> +{
>> +    int vmid;
>> +
>> +    /*
>> +     * Initialize all compute and user-gfx VMIDs to have no GDS, 
>> GWS, or OA
>> +     * access. Compute VMIDs should be enabled by FW for target VMIDs,
>> +     * the driver can enable them for graphics. VMID0 should maintain
>> +     * access so that HWS firmware can save/restore entries.
>> +     */
>> +    for (vmid = 1; vmid < 16; vmid++) {
>> +        WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_BASE, 2 * vmid, 0);
>> +        WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_SIZE, 2 * vmid, 0);
>> +        WREG32_SOC15_OFFSET(GC, 0, mmGDS_GWS_VMID0, vmid, 0);
>> +        WREG32_SOC15_OFFSET(GC, 0, mmGDS_OA_VMID0, vmid, 0);
>>       }
>>   }
>>   @@ -2080,6 +2089,7 @@ static void gfx_v9_0_constants_init(struct 
>> amdgpu_device *adev)
>>       mutex_unlock(&adev->srbm_mutex);
>>         gfx_v9_0_init_compute_vmid(adev);
>> +    gfx_v9_0_init_gds_vmid(adev);
>>   }
>>     static void gfx_v9_0_wait_for_rlc_serdes(struct amdgpu_device *adev)
>
> _______________________________________________
> 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] 10+ messages in thread

* Re: [PATCH v2] drm/amdgpu: Default disable GDS for compute+gfx
       [not found]                 ` <36885047-8915-fb90-fcaa-04f98b4519b1-5C7GfCeVMHo@public.gmane.org>
@ 2019-08-28 17:08                   ` Marek Olšák
       [not found]                     ` <CAAxE2A5MgKswdeHg1YrDs_G7hCuoZAVFuPyoHoA+DbUjA32eHg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 10+ messages in thread
From: Marek Olšák @ 2019-08-28 17:08 UTC (permalink / raw)
  To: zhoucm1
  Cc: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Christian König,
	Greathouse, Joseph


[-- Attachment #1.1: Type: text/plain, Size: 10610 bytes --]

It can't break an older driver, because there is no older driver that
requires the static allocation.

Note that closed source drivers don't count, because they don't need
backward compatibility.

Marek

On Wed, Aug 28, 2019 at 2:44 AM zhoucm1 <zhoucm1-5C7GfCeVMHo@public.gmane.org> wrote:

>
> On 2019/7/23 上午3:08, Christian König wrote:
> > Am 22.07.19 um 17:34 schrieb Greathouse, Joseph:
> >> Units in the GDS block default to allowing all VMIDs access to all
> >> entries. Disable shader access to the GDS, GWS, and OA blocks from all
> >> compute and gfx VMIDs by default. For compute, HWS firmware will set
> >> up the access bits for the appropriate VMID when a compute queue
> >> requires access to these blocks.
> >> The driver will handle enabling access on-demand for graphics VMIDs.
>
> gds_switch is depending on job->gds/gws/oa/_base/size.
>
> "[PATCH] drm/amdgpu: remove static GDS, GWS and OA allocation", the
> default allocations in kernel were removed. If some UMD stacks don't
> pass gds/gws/oa allocation to bo_list, then kernel will not enable
> access of them, that will break previous driver.
>
> do we need revert "[PATCH] drm/amdgpu: remove static GDS, GWS and OA
> allocation" ?
>
> -David
>
> >>
> >> Leaving VMID0 with full access because otherwise HWS cannot save or
> >> restore values during task switch.
> >>
> >> v2: Fixed code and comment styling.
> >>
> >> Change-Id: I3d768a96935d2ed1dff09b02c995090f4fbfa539
> >> Signed-off-by: Joseph Greathouse <Joseph.Greathouse-5C7GfCeVMHo@public.gmane.org>
> >
> > Reviewed-by: Christian König <christian.koenig-5C7GfCeVMHo@public.gmane.org>
> >
> >> ---
> >>   drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 25 ++++++++++++++++++-------
> >>   drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c  | 24 +++++++++++++++++-------
> >>   drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c  | 24 +++++++++++++++++-------
> >>   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c  | 24 +++++++++++++++++-------
> >>   4 files changed, 69 insertions(+), 28 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> >> b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> >> index 73dcb632a3ce..2a9692bc34b4 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> >> @@ -1516,17 +1516,27 @@ static void
> >> gfx_v10_0_init_compute_vmid(struct amdgpu_device *adev)
> >>       }
> >>       nv_grbm_select(adev, 0, 0, 0, 0);
> >>       mutex_unlock(&adev->srbm_mutex);
> >> +}
> >>   -    /* Initialize all compute VMIDs to have no GDS, GWS, or OA
> >> -       acccess. These should be enabled by FW for target VMIDs. */
> >> -    for (i = FIRST_COMPUTE_VMID; i < LAST_COMPUTE_VMID; i++) {
> >> -        WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_BASE, 2 * i, 0);
> >> -        WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_SIZE, 2 * i, 0);
> >> -        WREG32_SOC15_OFFSET(GC, 0, mmGDS_GWS_VMID0, i, 0);
> >> -        WREG32_SOC15_OFFSET(GC, 0, mmGDS_OA_VMID0, i, 0);
> >> +static void gfx_v10_0_init_gds_vmid(struct amdgpu_device *adev)
> >> +{
> >> +    int vmid;
> >> +
> >> +    /*
> >> +     * Initialize all compute and user-gfx VMIDs to have no GDS,
> >> GWS, or OA
> >> +     * access. Compute VMIDs should be enabled by FW for target VMIDs,
> >> +     * the driver can enable them for graphics. VMID0 should maintain
> >> +     * access so that HWS firmware can save/restore entries.
> >> +     */
> >> +    for (vmid = 1; vmid < 16; vmid++) {
> >> +        WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_BASE, 2 * vmid, 0);
> >> +        WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_SIZE, 2 * vmid, 0);
> >> +        WREG32_SOC15_OFFSET(GC, 0, mmGDS_GWS_VMID0, vmid, 0);
> >> +        WREG32_SOC15_OFFSET(GC, 0, mmGDS_OA_VMID0, vmid, 0);
> >>       }
> >>   }
> >>   +
> >>   static void gfx_v10_0_tcp_harvest(struct amdgpu_device *adev)
> >>   {
> >>       int i, j, k;
> >> @@ -1629,6 +1639,7 @@ static void gfx_v10_0_constants_init(struct
> >> amdgpu_device *adev)
> >>       mutex_unlock(&adev->srbm_mutex);
> >>         gfx_v10_0_init_compute_vmid(adev);
> >> +    gfx_v10_0_init_gds_vmid(adev);
> >>     }
> >>   diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> >> b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> >> index 3f98624772a4..48796b6824cf 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> >> @@ -1877,14 +1877,23 @@ static void gfx_v7_0_init_compute_vmid(struct
> >> amdgpu_device *adev)
> >>       }
> >>       cik_srbm_select(adev, 0, 0, 0, 0);
> >>       mutex_unlock(&adev->srbm_mutex);
> >> +}
> >>   -    /* Initialize all compute VMIDs to have no GDS, GWS, or OA
> >> -       acccess. These should be enabled by FW for target VMIDs. */
> >> -    for (i = FIRST_COMPUTE_VMID; i < LAST_COMPUTE_VMID; i++) {
> >> -        WREG32(amdgpu_gds_reg_offset[i].mem_base, 0);
> >> -        WREG32(amdgpu_gds_reg_offset[i].mem_size, 0);
> >> -        WREG32(amdgpu_gds_reg_offset[i].gws, 0);
> >> -        WREG32(amdgpu_gds_reg_offset[i].oa, 0);
> >> +static void gfx_v7_0_init_gds_vmid(struct amdgpu_device *adev)
> >> +{
> >> +    int vmid;
> >> +
> >> +    /*
> >> +     * Initialize all compute and user-gfx VMIDs to have no GDS,
> >> GWS, or OA
> >> +     * access. Compute VMIDs should be enabled by FW for target VMIDs,
> >> +     * the driver can enable them for graphics. VMID0 should maintain
> >> +     * access so that HWS firmware can save/restore entries.
> >> +     */
> >> +    for (vmid = 1; vmid < 16; vmid++) {
> >> +        WREG32(amdgpu_gds_reg_offset[vmid].mem_base, 0);
> >> +        WREG32(amdgpu_gds_reg_offset[vmid].mem_size, 0);
> >> +        WREG32(amdgpu_gds_reg_offset[vmid].gws, 0);
> >> +        WREG32(amdgpu_gds_reg_offset[vmid].oa, 0);
> >>       }
> >>   }
> >>   @@ -1966,6 +1975,7 @@ static void gfx_v7_0_constants_init(struct
> >> amdgpu_device *adev)
> >>       mutex_unlock(&adev->srbm_mutex);
> >>         gfx_v7_0_init_compute_vmid(adev);
> >> +    gfx_v7_0_init_gds_vmid(adev);
> >>         WREG32(mmSX_DEBUG_1, 0x20);
> >>   diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> >> b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> >> index e4028d54f8f7..d2907186bb24 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> >> @@ -3702,14 +3702,23 @@ static void gfx_v8_0_init_compute_vmid(struct
> >> amdgpu_device *adev)
> >>       }
> >>       vi_srbm_select(adev, 0, 0, 0, 0);
> >>       mutex_unlock(&adev->srbm_mutex);
> >> +}
> >>   -    /* Initialize all compute VMIDs to have no GDS, GWS, or OA
> >> -       acccess. These should be enabled by FW for target VMIDs. */
> >> -    for (i = FIRST_COMPUTE_VMID; i < LAST_COMPUTE_VMID; i++) {
> >> -        WREG32(amdgpu_gds_reg_offset[i].mem_base, 0);
> >> -        WREG32(amdgpu_gds_reg_offset[i].mem_size, 0);
> >> -        WREG32(amdgpu_gds_reg_offset[i].gws, 0);
> >> -        WREG32(amdgpu_gds_reg_offset[i].oa, 0);
> >> +static void gfx_v8_0_init_gds_vmid(struct amdgpu_device *adev)
> >> +{
> >> +    int vmid;
> >> +
> >> +    /*
> >> +     * Initialize all compute and user-gfx VMIDs to have no GDS,
> >> GWS, or OA
> >> +     * access. Compute VMIDs should be enabled by FW for target VMIDs,
> >> +     * the driver can enable them for graphics. VMID0 should maintain
> >> +     * access so that HWS firmware can save/restore entries.
> >> +     */
> >> +    for (vmid = 1; vmid < 16; vmid++) {
> >> +        WREG32(amdgpu_gds_reg_offset[vmid].mem_base, 0);
> >> +        WREG32(amdgpu_gds_reg_offset[vmid].mem_size, 0);
> >> +        WREG32(amdgpu_gds_reg_offset[vmid].gws, 0);
> >> +        WREG32(amdgpu_gds_reg_offset[vmid].oa, 0);
> >>       }
> >>   }
> >>   @@ -3779,6 +3788,7 @@ static void gfx_v8_0_constants_init(struct
> >> amdgpu_device *adev)
> >>       mutex_unlock(&adev->srbm_mutex);
> >>         gfx_v8_0_init_compute_vmid(adev);
> >> +    gfx_v8_0_init_gds_vmid(adev);
> >>         mutex_lock(&adev->grbm_idx_mutex);
> >>       /*
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> >> b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> >> index 259a35395fec..bd7f431a24d9 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> >> @@ -2025,14 +2025,23 @@ static void gfx_v9_0_init_compute_vmid(struct
> >> amdgpu_device *adev)
> >>       }
> >>       soc15_grbm_select(adev, 0, 0, 0, 0);
> >>       mutex_unlock(&adev->srbm_mutex);
> >> +}
> >>   -    /* Initialize all compute VMIDs to have no GDS, GWS, or OA
> >> -       acccess. These should be enabled by FW for target VMIDs. */
> >> -    for (i = FIRST_COMPUTE_VMID; i < LAST_COMPUTE_VMID; i++) {
> >> -        WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_BASE, 2 * i, 0);
> >> -        WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_SIZE, 2 * i, 0);
> >> -        WREG32_SOC15_OFFSET(GC, 0, mmGDS_GWS_VMID0, i, 0);
> >> -        WREG32_SOC15_OFFSET(GC, 0, mmGDS_OA_VMID0, i, 0);
> >> +static void gfx_v9_0_init_gds_vmid(struct amdgpu_device *adev)
> >> +{
> >> +    int vmid;
> >> +
> >> +    /*
> >> +     * Initialize all compute and user-gfx VMIDs to have no GDS,
> >> GWS, or OA
> >> +     * access. Compute VMIDs should be enabled by FW for target VMIDs,
> >> +     * the driver can enable them for graphics. VMID0 should maintain
> >> +     * access so that HWS firmware can save/restore entries.
> >> +     */
> >> +    for (vmid = 1; vmid < 16; vmid++) {
> >> +        WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_BASE, 2 * vmid, 0);
> >> +        WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_SIZE, 2 * vmid, 0);
> >> +        WREG32_SOC15_OFFSET(GC, 0, mmGDS_GWS_VMID0, vmid, 0);
> >> +        WREG32_SOC15_OFFSET(GC, 0, mmGDS_OA_VMID0, vmid, 0);
> >>       }
> >>   }
> >>   @@ -2080,6 +2089,7 @@ static void gfx_v9_0_constants_init(struct
> >> amdgpu_device *adev)
> >>       mutex_unlock(&adev->srbm_mutex);
> >>         gfx_v9_0_init_compute_vmid(adev);
> >> +    gfx_v9_0_init_gds_vmid(adev);
> >>   }
> >>     static void gfx_v9_0_wait_for_rlc_serdes(struct amdgpu_device *adev)
> >
> > _______________________________________________
> > amd-gfx mailing list
> > amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> _______________________________________________
> amd-gfx mailing list
> amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[-- Attachment #1.2: Type: text/html, Size: 13868 bytes --]

[-- Attachment #2: Type: text/plain, Size: 153 bytes --]

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] drm/amdgpu: Default disable GDS for compute+gfx
       [not found]                     ` <CAAxE2A5MgKswdeHg1YrDs_G7hCuoZAVFuPyoHoA+DbUjA32eHg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2019-08-29  5:55                       ` zhoucm1
       [not found]                         ` <bdb61ac5-e1ab-725c-56cf-475d7b41ebc2-5C7GfCeVMHo@public.gmane.org>
  0 siblings, 1 reply; 10+ messages in thread
From: zhoucm1 @ 2019-08-29  5:55 UTC (permalink / raw)
  To: Marek Olšák
  Cc: Qiao, Ken, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	Christian König, Greathouse, Joseph


[-- Attachment #1.1: Type: text/plain, Size: 12490 bytes --]


On 2019/8/29 上午1:08, Marek Olšák wrote:
> It can't break an older driver, because there is no older driver that 
> requires the static allocation.
>
> Note that closed source drivers don't count, because they don't need 
> backward compatibility.

Yes, I agree, we don't need take care of closed source stack.

But AMDVLK is indeed an open source stack, many fans are using it, we 
need keep its compatibility, don't we?

-David

>
> Marek
>
> On Wed, Aug 28, 2019 at 2:44 AM zhoucm1 <zhoucm1-5C7GfCeVMHo@public.gmane.org 
> <mailto:zhoucm1-5C7GfCeVMHo@public.gmane.org>> wrote:
>
>
>     On 2019/7/23 上午3:08, Christian König wrote:
>     > Am 22.07.19 um 17:34 schrieb Greathouse, Joseph:
>     >> Units in the GDS block default to allowing all VMIDs access to all
>     >> entries. Disable shader access to the GDS, GWS, and OA blocks
>     from all
>     >> compute and gfx VMIDs by default. For compute, HWS firmware
>     will set
>     >> up the access bits for the appropriate VMID when a compute queue
>     >> requires access to these blocks.
>     >> The driver will handle enabling access on-demand for graphics
>     VMIDs.
>
>     gds_switch is depending on job->gds/gws/oa/_base/size.
>
>     "[PATCH] drm/amdgpu: remove static GDS, GWS and OA allocation", the
>     default allocations in kernel were removed. If some UMD stacks don't
>     pass gds/gws/oa allocation to bo_list, then kernel will not enable
>     access of them, that will break previous driver.
>
>     do we need revert "[PATCH] drm/amdgpu: remove static GDS, GWS and OA
>     allocation" ?
>
>     -David
>
>     >>
>     >> Leaving VMID0 with full access because otherwise HWS cannot save or
>     >> restore values during task switch.
>     >>
>     >> v2: Fixed code and comment styling.
>     >>
>     >> Change-Id: I3d768a96935d2ed1dff09b02c995090f4fbfa539
>     >> Signed-off-by: Joseph Greathouse <Joseph.Greathouse-5C7GfCeVMHo@public.gmane.org
>     <mailto:Joseph.Greathouse-5C7GfCeVMHo@public.gmane.org>>
>     >
>     > Reviewed-by: Christian König <christian.koenig-5C7GfCeVMHo@public.gmane.org
>     <mailto:christian.koenig-5C7GfCeVMHo@public.gmane.org>>
>     >
>     >> ---
>     >>   drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 25
>     ++++++++++++++++++-------
>     >>   drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c  | 24
>     +++++++++++++++++-------
>     >>   drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c  | 24
>     +++++++++++++++++-------
>     >>   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c  | 24
>     +++++++++++++++++-------
>     >>   4 files changed, 69 insertions(+), 28 deletions(-)
>     >>
>     >> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
>     >> b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
>     >> index 73dcb632a3ce..2a9692bc34b4 100644
>     >> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
>     >> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
>     >> @@ -1516,17 +1516,27 @@ static void
>     >> gfx_v10_0_init_compute_vmid(struct amdgpu_device *adev)
>     >>       }
>     >>       nv_grbm_select(adev, 0, 0, 0, 0);
>     >>       mutex_unlock(&adev->srbm_mutex);
>     >> +}
>     >>   -    /* Initialize all compute VMIDs to have no GDS, GWS, or OA
>     >> -       acccess. These should be enabled by FW for target VMIDs. */
>     >> -    for (i = FIRST_COMPUTE_VMID; i < LAST_COMPUTE_VMID; i++) {
>     >> -        WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_BASE, 2 * i, 0);
>     >> -        WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_SIZE, 2 * i, 0);
>     >> -        WREG32_SOC15_OFFSET(GC, 0, mmGDS_GWS_VMID0, i, 0);
>     >> -        WREG32_SOC15_OFFSET(GC, 0, mmGDS_OA_VMID0, i, 0);
>     >> +static void gfx_v10_0_init_gds_vmid(struct amdgpu_device *adev)
>     >> +{
>     >> +    int vmid;
>     >> +
>     >> +    /*
>     >> +     * Initialize all compute and user-gfx VMIDs to have no GDS,
>     >> GWS, or OA
>     >> +     * access. Compute VMIDs should be enabled by FW for
>     target VMIDs,
>     >> +     * the driver can enable them for graphics. VMID0 should
>     maintain
>     >> +     * access so that HWS firmware can save/restore entries.
>     >> +     */
>     >> +    for (vmid = 1; vmid < 16; vmid++) {
>     >> +        WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_BASE, 2 * vmid, 0);
>     >> +        WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_SIZE, 2 * vmid, 0);
>     >> +        WREG32_SOC15_OFFSET(GC, 0, mmGDS_GWS_VMID0, vmid, 0);
>     >> +        WREG32_SOC15_OFFSET(GC, 0, mmGDS_OA_VMID0, vmid, 0);
>     >>       }
>     >>   }
>     >>   +
>     >>   static void gfx_v10_0_tcp_harvest(struct amdgpu_device *adev)
>     >>   {
>     >>       int i, j, k;
>     >> @@ -1629,6 +1639,7 @@ static void gfx_v10_0_constants_init(struct
>     >> amdgpu_device *adev)
>     >>       mutex_unlock(&adev->srbm_mutex);
>     >>         gfx_v10_0_init_compute_vmid(adev);
>     >> +    gfx_v10_0_init_gds_vmid(adev);
>     >>     }
>     >>   diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
>     >> b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
>     >> index 3f98624772a4..48796b6824cf 100644
>     >> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
>     >> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
>     >> @@ -1877,14 +1877,23 @@ static void
>     gfx_v7_0_init_compute_vmid(struct
>     >> amdgpu_device *adev)
>     >>       }
>     >>       cik_srbm_select(adev, 0, 0, 0, 0);
>     >>       mutex_unlock(&adev->srbm_mutex);
>     >> +}
>     >>   -    /* Initialize all compute VMIDs to have no GDS, GWS, or OA
>     >> -       acccess. These should be enabled by FW for target VMIDs. */
>     >> -    for (i = FIRST_COMPUTE_VMID; i < LAST_COMPUTE_VMID; i++) {
>     >> -        WREG32(amdgpu_gds_reg_offset[i].mem_base, 0);
>     >> -        WREG32(amdgpu_gds_reg_offset[i].mem_size, 0);
>     >> -        WREG32(amdgpu_gds_reg_offset[i].gws, 0);
>     >> -        WREG32(amdgpu_gds_reg_offset[i].oa, 0);
>     >> +static void gfx_v7_0_init_gds_vmid(struct amdgpu_device *adev)
>     >> +{
>     >> +    int vmid;
>     >> +
>     >> +    /*
>     >> +     * Initialize all compute and user-gfx VMIDs to have no GDS,
>     >> GWS, or OA
>     >> +     * access. Compute VMIDs should be enabled by FW for
>     target VMIDs,
>     >> +     * the driver can enable them for graphics. VMID0 should
>     maintain
>     >> +     * access so that HWS firmware can save/restore entries.
>     >> +     */
>     >> +    for (vmid = 1; vmid < 16; vmid++) {
>     >> +        WREG32(amdgpu_gds_reg_offset[vmid].mem_base, 0);
>     >> +        WREG32(amdgpu_gds_reg_offset[vmid].mem_size, 0);
>     >> +        WREG32(amdgpu_gds_reg_offset[vmid].gws, 0);
>     >> +        WREG32(amdgpu_gds_reg_offset[vmid].oa, 0);
>     >>       }
>     >>   }
>     >>   @@ -1966,6 +1975,7 @@ static void gfx_v7_0_constants_init(struct
>     >> amdgpu_device *adev)
>     >>       mutex_unlock(&adev->srbm_mutex);
>     >>         gfx_v7_0_init_compute_vmid(adev);
>     >> +    gfx_v7_0_init_gds_vmid(adev);
>     >>         WREG32(mmSX_DEBUG_1, 0x20);
>     >>   diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
>     >> b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
>     >> index e4028d54f8f7..d2907186bb24 100644
>     >> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
>     >> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
>     >> @@ -3702,14 +3702,23 @@ static void
>     gfx_v8_0_init_compute_vmid(struct
>     >> amdgpu_device *adev)
>     >>       }
>     >>       vi_srbm_select(adev, 0, 0, 0, 0);
>     >>       mutex_unlock(&adev->srbm_mutex);
>     >> +}
>     >>   -    /* Initialize all compute VMIDs to have no GDS, GWS, or OA
>     >> -       acccess. These should be enabled by FW for target VMIDs. */
>     >> -    for (i = FIRST_COMPUTE_VMID; i < LAST_COMPUTE_VMID; i++) {
>     >> -        WREG32(amdgpu_gds_reg_offset[i].mem_base, 0);
>     >> -        WREG32(amdgpu_gds_reg_offset[i].mem_size, 0);
>     >> -        WREG32(amdgpu_gds_reg_offset[i].gws, 0);
>     >> -        WREG32(amdgpu_gds_reg_offset[i].oa, 0);
>     >> +static void gfx_v8_0_init_gds_vmid(struct amdgpu_device *adev)
>     >> +{
>     >> +    int vmid;
>     >> +
>     >> +    /*
>     >> +     * Initialize all compute and user-gfx VMIDs to have no GDS,
>     >> GWS, or OA
>     >> +     * access. Compute VMIDs should be enabled by FW for
>     target VMIDs,
>     >> +     * the driver can enable them for graphics. VMID0 should
>     maintain
>     >> +     * access so that HWS firmware can save/restore entries.
>     >> +     */
>     >> +    for (vmid = 1; vmid < 16; vmid++) {
>     >> +        WREG32(amdgpu_gds_reg_offset[vmid].mem_base, 0);
>     >> +        WREG32(amdgpu_gds_reg_offset[vmid].mem_size, 0);
>     >> +        WREG32(amdgpu_gds_reg_offset[vmid].gws, 0);
>     >> +        WREG32(amdgpu_gds_reg_offset[vmid].oa, 0);
>     >>       }
>     >>   }
>     >>   @@ -3779,6 +3788,7 @@ static void gfx_v8_0_constants_init(struct
>     >> amdgpu_device *adev)
>     >>       mutex_unlock(&adev->srbm_mutex);
>     >>         gfx_v8_0_init_compute_vmid(adev);
>     >> +    gfx_v8_0_init_gds_vmid(adev);
>     >>         mutex_lock(&adev->grbm_idx_mutex);
>     >>       /*
>     >> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>     >> b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>     >> index 259a35395fec..bd7f431a24d9 100644
>     >> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>     >> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>     >> @@ -2025,14 +2025,23 @@ static void
>     gfx_v9_0_init_compute_vmid(struct
>     >> amdgpu_device *adev)
>     >>       }
>     >>       soc15_grbm_select(adev, 0, 0, 0, 0);
>     >>       mutex_unlock(&adev->srbm_mutex);
>     >> +}
>     >>   -    /* Initialize all compute VMIDs to have no GDS, GWS, or OA
>     >> -       acccess. These should be enabled by FW for target VMIDs. */
>     >> -    for (i = FIRST_COMPUTE_VMID; i < LAST_COMPUTE_VMID; i++) {
>     >> -        WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_BASE, 2 * i, 0);
>     >> -        WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_SIZE, 2 * i, 0);
>     >> -        WREG32_SOC15_OFFSET(GC, 0, mmGDS_GWS_VMID0, i, 0);
>     >> -        WREG32_SOC15_OFFSET(GC, 0, mmGDS_OA_VMID0, i, 0);
>     >> +static void gfx_v9_0_init_gds_vmid(struct amdgpu_device *adev)
>     >> +{
>     >> +    int vmid;
>     >> +
>     >> +    /*
>     >> +     * Initialize all compute and user-gfx VMIDs to have no GDS,
>     >> GWS, or OA
>     >> +     * access. Compute VMIDs should be enabled by FW for
>     target VMIDs,
>     >> +     * the driver can enable them for graphics. VMID0 should
>     maintain
>     >> +     * access so that HWS firmware can save/restore entries.
>     >> +     */
>     >> +    for (vmid = 1; vmid < 16; vmid++) {
>     >> +        WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_BASE, 2 * vmid, 0);
>     >> +        WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_SIZE, 2 * vmid, 0);
>     >> +        WREG32_SOC15_OFFSET(GC, 0, mmGDS_GWS_VMID0, vmid, 0);
>     >> +        WREG32_SOC15_OFFSET(GC, 0, mmGDS_OA_VMID0, vmid, 0);
>     >>       }
>     >>   }
>     >>   @@ -2080,6 +2089,7 @@ static void gfx_v9_0_constants_init(struct
>     >> amdgpu_device *adev)
>     >>       mutex_unlock(&adev->srbm_mutex);
>     >>         gfx_v9_0_init_compute_vmid(adev);
>     >> +    gfx_v9_0_init_gds_vmid(adev);
>     >>   }
>     >>     static void gfx_v9_0_wait_for_rlc_serdes(struct
>     amdgpu_device *adev)
>     >
>     > _______________________________________________
>     > amd-gfx mailing list
>     > amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org <mailto:amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
>     > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>     _______________________________________________
>     amd-gfx mailing list
>     amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org <mailto:amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
>     https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>

[-- Attachment #1.2: Type: text/html, Size: 18051 bytes --]

[-- Attachment #2: Type: text/plain, Size: 153 bytes --]

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] drm/amdgpu: Default disable GDS for compute+gfx
       [not found]                         ` <bdb61ac5-e1ab-725c-56cf-475d7b41ebc2-5C7GfCeVMHo@public.gmane.org>
@ 2019-08-29  7:22                           ` Christian König
       [not found]                             ` <293feae7-bda5-d8af-2dca-6f2025dc3fba-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  0 siblings, 1 reply; 10+ messages in thread
From: Christian König @ 2019-08-29  7:22 UTC (permalink / raw)
  To: zhoucm1, Marek Olšák
  Cc: Greathouse, Joseph, Qiao, Ken, Christian König,
	amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW


[-- Attachment #1.1: Type: text/plain, Size: 13646 bytes --]

Am 29.08.19 um 07:55 schrieb zhoucm1:
>
>
> On 2019/8/29 上午1:08, Marek Olšák wrote:
>> It can't break an older driver, because there is no older driver that 
>> requires the static allocation.
>>
>> Note that closed source drivers don't count, because they don't need 
>> backward compatibility.
>
> Yes, I agree, we don't need take care of closed source stack.
>
> But AMDVLK is indeed an open source stack, many fans are using it, we 
> need keep its compatibility, don't we?
>

Actually that is still under discussion.

But AMDVLK should have never ever used the static GDS space in the first 
place. We only added that for a transition time for old OpenGL and it 
shouldn't have leaked into the upstream driver.

Not sure what's the best approach here. We could revert "[PATCH] 
drm/amdgpu: remove static GDS, GWS and OA", but that would break KFD. So 
we can only choose between two evils here.

Only alternative I can see which would work for both would be to still 
allocate the static GDS, GWS and OA space, but make it somehow dynamic 
so that the KFD can swap it out again.

Christian.

> -David
>
>>
>> Marek
>>
>> On Wed, Aug 28, 2019 at 2:44 AM zhoucm1 <zhoucm1-5C7GfCeVMHo@public.gmane.org 
>> <mailto:zhoucm1-5C7GfCeVMHo@public.gmane.org>> wrote:
>>
>>
>>     On 2019/7/23 上午3:08, Christian König wrote:
>>     > Am 22.07.19 um 17:34 schrieb Greathouse, Joseph:
>>     >> Units in the GDS block default to allowing all VMIDs access to all
>>     >> entries. Disable shader access to the GDS, GWS, and OA blocks
>>     from all
>>     >> compute and gfx VMIDs by default. For compute, HWS firmware
>>     will set
>>     >> up the access bits for the appropriate VMID when a compute queue
>>     >> requires access to these blocks.
>>     >> The driver will handle enabling access on-demand for graphics
>>     VMIDs.
>>
>>     gds_switch is depending on job->gds/gws/oa/_base/size.
>>
>>     "[PATCH] drm/amdgpu: remove static GDS, GWS and OA allocation", the
>>     default allocations in kernel were removed. If some UMD stacks don't
>>     pass gds/gws/oa allocation to bo_list, then kernel will not enable
>>     access of them, that will break previous driver.
>>
>>     do we need revert "[PATCH] drm/amdgpu: remove static GDS, GWS and OA
>>     allocation" ?
>>
>>     -David
>>
>>     >>
>>     >> Leaving VMID0 with full access because otherwise HWS cannot
>>     save or
>>     >> restore values during task switch.
>>     >>
>>     >> v2: Fixed code and comment styling.
>>     >>
>>     >> Change-Id: I3d768a96935d2ed1dff09b02c995090f4fbfa539
>>     >> Signed-off-by: Joseph Greathouse <Joseph.Greathouse-5C7GfCeVMHo@public.gmane.org
>>     <mailto:Joseph.Greathouse-5C7GfCeVMHo@public.gmane.org>>
>>     >
>>     > Reviewed-by: Christian König <christian.koenig-5C7GfCeVMHo@public.gmane.org
>>     <mailto:christian.koenig-5C7GfCeVMHo@public.gmane.org>>
>>     >
>>     >> ---
>>     >>   drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 25
>>     ++++++++++++++++++-------
>>     >>   drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c  | 24
>>     +++++++++++++++++-------
>>     >>   drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c  | 24
>>     +++++++++++++++++-------
>>     >>   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c  | 24
>>     +++++++++++++++++-------
>>     >>   4 files changed, 69 insertions(+), 28 deletions(-)
>>     >>
>>     >> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
>>     >> b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
>>     >> index 73dcb632a3ce..2a9692bc34b4 100644
>>     >> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
>>     >> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
>>     >> @@ -1516,17 +1516,27 @@ static void
>>     >> gfx_v10_0_init_compute_vmid(struct amdgpu_device *adev)
>>     >>       }
>>     >>       nv_grbm_select(adev, 0, 0, 0, 0);
>>     >>       mutex_unlock(&adev->srbm_mutex);
>>     >> +}
>>     >>   -    /* Initialize all compute VMIDs to have no GDS, GWS, or OA
>>     >> -       acccess. These should be enabled by FW for target
>>     VMIDs. */
>>     >> -    for (i = FIRST_COMPUTE_VMID; i < LAST_COMPUTE_VMID; i++) {
>>     >> -        WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_BASE, 2 * i, 0);
>>     >> -        WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_SIZE, 2 * i, 0);
>>     >> -        WREG32_SOC15_OFFSET(GC, 0, mmGDS_GWS_VMID0, i, 0);
>>     >> -        WREG32_SOC15_OFFSET(GC, 0, mmGDS_OA_VMID0, i, 0);
>>     >> +static void gfx_v10_0_init_gds_vmid(struct amdgpu_device *adev)
>>     >> +{
>>     >> +    int vmid;
>>     >> +
>>     >> +    /*
>>     >> +     * Initialize all compute and user-gfx VMIDs to have no GDS,
>>     >> GWS, or OA
>>     >> +     * access. Compute VMIDs should be enabled by FW for
>>     target VMIDs,
>>     >> +     * the driver can enable them for graphics. VMID0 should
>>     maintain
>>     >> +     * access so that HWS firmware can save/restore entries.
>>     >> +     */
>>     >> +    for (vmid = 1; vmid < 16; vmid++) {
>>     >> +        WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_BASE, 2 *
>>     vmid, 0);
>>     >> +        WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_SIZE, 2 *
>>     vmid, 0);
>>     >> +        WREG32_SOC15_OFFSET(GC, 0, mmGDS_GWS_VMID0, vmid, 0);
>>     >> +        WREG32_SOC15_OFFSET(GC, 0, mmGDS_OA_VMID0, vmid, 0);
>>     >>       }
>>     >>   }
>>     >>   +
>>     >>   static void gfx_v10_0_tcp_harvest(struct amdgpu_device *adev)
>>     >>   {
>>     >>       int i, j, k;
>>     >> @@ -1629,6 +1639,7 @@ static void gfx_v10_0_constants_init(struct
>>     >> amdgpu_device *adev)
>>     >>       mutex_unlock(&adev->srbm_mutex);
>>     >>         gfx_v10_0_init_compute_vmid(adev);
>>     >> +    gfx_v10_0_init_gds_vmid(adev);
>>     >>     }
>>     >>   diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
>>     >> b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
>>     >> index 3f98624772a4..48796b6824cf 100644
>>     >> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
>>     >> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
>>     >> @@ -1877,14 +1877,23 @@ static void
>>     gfx_v7_0_init_compute_vmid(struct
>>     >> amdgpu_device *adev)
>>     >>       }
>>     >>       cik_srbm_select(adev, 0, 0, 0, 0);
>>     >>       mutex_unlock(&adev->srbm_mutex);
>>     >> +}
>>     >>   -    /* Initialize all compute VMIDs to have no GDS, GWS, or OA
>>     >> -       acccess. These should be enabled by FW for target
>>     VMIDs. */
>>     >> -    for (i = FIRST_COMPUTE_VMID; i < LAST_COMPUTE_VMID; i++) {
>>     >> -        WREG32(amdgpu_gds_reg_offset[i].mem_base, 0);
>>     >> -        WREG32(amdgpu_gds_reg_offset[i].mem_size, 0);
>>     >> -        WREG32(amdgpu_gds_reg_offset[i].gws, 0);
>>     >> -        WREG32(amdgpu_gds_reg_offset[i].oa, 0);
>>     >> +static void gfx_v7_0_init_gds_vmid(struct amdgpu_device *adev)
>>     >> +{
>>     >> +    int vmid;
>>     >> +
>>     >> +    /*
>>     >> +     * Initialize all compute and user-gfx VMIDs to have no GDS,
>>     >> GWS, or OA
>>     >> +     * access. Compute VMIDs should be enabled by FW for
>>     target VMIDs,
>>     >> +     * the driver can enable them for graphics. VMID0 should
>>     maintain
>>     >> +     * access so that HWS firmware can save/restore entries.
>>     >> +     */
>>     >> +    for (vmid = 1; vmid < 16; vmid++) {
>>     >> + WREG32(amdgpu_gds_reg_offset[vmid].mem_base, 0);
>>     >> + WREG32(amdgpu_gds_reg_offset[vmid].mem_size, 0);
>>     >> +        WREG32(amdgpu_gds_reg_offset[vmid].gws, 0);
>>     >> +        WREG32(amdgpu_gds_reg_offset[vmid].oa, 0);
>>     >>       }
>>     >>   }
>>     >>   @@ -1966,6 +1975,7 @@ static void
>>     gfx_v7_0_constants_init(struct
>>     >> amdgpu_device *adev)
>>     >>       mutex_unlock(&adev->srbm_mutex);
>>     >>         gfx_v7_0_init_compute_vmid(adev);
>>     >> +    gfx_v7_0_init_gds_vmid(adev);
>>     >>         WREG32(mmSX_DEBUG_1, 0x20);
>>     >>   diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
>>     >> b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
>>     >> index e4028d54f8f7..d2907186bb24 100644
>>     >> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
>>     >> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
>>     >> @@ -3702,14 +3702,23 @@ static void
>>     gfx_v8_0_init_compute_vmid(struct
>>     >> amdgpu_device *adev)
>>     >>       }
>>     >>       vi_srbm_select(adev, 0, 0, 0, 0);
>>     >>       mutex_unlock(&adev->srbm_mutex);
>>     >> +}
>>     >>   -    /* Initialize all compute VMIDs to have no GDS, GWS, or OA
>>     >> -       acccess. These should be enabled by FW for target
>>     VMIDs. */
>>     >> -    for (i = FIRST_COMPUTE_VMID; i < LAST_COMPUTE_VMID; i++) {
>>     >> -        WREG32(amdgpu_gds_reg_offset[i].mem_base, 0);
>>     >> -        WREG32(amdgpu_gds_reg_offset[i].mem_size, 0);
>>     >> -        WREG32(amdgpu_gds_reg_offset[i].gws, 0);
>>     >> -        WREG32(amdgpu_gds_reg_offset[i].oa, 0);
>>     >> +static void gfx_v8_0_init_gds_vmid(struct amdgpu_device *adev)
>>     >> +{
>>     >> +    int vmid;
>>     >> +
>>     >> +    /*
>>     >> +     * Initialize all compute and user-gfx VMIDs to have no GDS,
>>     >> GWS, or OA
>>     >> +     * access. Compute VMIDs should be enabled by FW for
>>     target VMIDs,
>>     >> +     * the driver can enable them for graphics. VMID0 should
>>     maintain
>>     >> +     * access so that HWS firmware can save/restore entries.
>>     >> +     */
>>     >> +    for (vmid = 1; vmid < 16; vmid++) {
>>     >> + WREG32(amdgpu_gds_reg_offset[vmid].mem_base, 0);
>>     >> + WREG32(amdgpu_gds_reg_offset[vmid].mem_size, 0);
>>     >> +        WREG32(amdgpu_gds_reg_offset[vmid].gws, 0);
>>     >> +        WREG32(amdgpu_gds_reg_offset[vmid].oa, 0);
>>     >>       }
>>     >>   }
>>     >>   @@ -3779,6 +3788,7 @@ static void
>>     gfx_v8_0_constants_init(struct
>>     >> amdgpu_device *adev)
>>     >>       mutex_unlock(&adev->srbm_mutex);
>>     >>         gfx_v8_0_init_compute_vmid(adev);
>>     >> +    gfx_v8_0_init_gds_vmid(adev);
>>     >>         mutex_lock(&adev->grbm_idx_mutex);
>>     >>       /*
>>     >> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>>     >> b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>>     >> index 259a35395fec..bd7f431a24d9 100644
>>     >> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>>     >> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>>     >> @@ -2025,14 +2025,23 @@ static void
>>     gfx_v9_0_init_compute_vmid(struct
>>     >> amdgpu_device *adev)
>>     >>       }
>>     >>       soc15_grbm_select(adev, 0, 0, 0, 0);
>>     >>       mutex_unlock(&adev->srbm_mutex);
>>     >> +}
>>     >>   -    /* Initialize all compute VMIDs to have no GDS, GWS, or OA
>>     >> -       acccess. These should be enabled by FW for target
>>     VMIDs. */
>>     >> -    for (i = FIRST_COMPUTE_VMID; i < LAST_COMPUTE_VMID; i++) {
>>     >> -        WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_BASE, 2 * i, 0);
>>     >> -        WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_SIZE, 2 * i, 0);
>>     >> -        WREG32_SOC15_OFFSET(GC, 0, mmGDS_GWS_VMID0, i, 0);
>>     >> -        WREG32_SOC15_OFFSET(GC, 0, mmGDS_OA_VMID0, i, 0);
>>     >> +static void gfx_v9_0_init_gds_vmid(struct amdgpu_device *adev)
>>     >> +{
>>     >> +    int vmid;
>>     >> +
>>     >> +    /*
>>     >> +     * Initialize all compute and user-gfx VMIDs to have no GDS,
>>     >> GWS, or OA
>>     >> +     * access. Compute VMIDs should be enabled by FW for
>>     target VMIDs,
>>     >> +     * the driver can enable them for graphics. VMID0 should
>>     maintain
>>     >> +     * access so that HWS firmware can save/restore entries.
>>     >> +     */
>>     >> +    for (vmid = 1; vmid < 16; vmid++) {
>>     >> +        WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_BASE, 2 *
>>     vmid, 0);
>>     >> +        WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_SIZE, 2 *
>>     vmid, 0);
>>     >> +        WREG32_SOC15_OFFSET(GC, 0, mmGDS_GWS_VMID0, vmid, 0);
>>     >> +        WREG32_SOC15_OFFSET(GC, 0, mmGDS_OA_VMID0, vmid, 0);
>>     >>       }
>>     >>   }
>>     >>   @@ -2080,6 +2089,7 @@ static void
>>     gfx_v9_0_constants_init(struct
>>     >> amdgpu_device *adev)
>>     >>       mutex_unlock(&adev->srbm_mutex);
>>     >>         gfx_v9_0_init_compute_vmid(adev);
>>     >> +    gfx_v9_0_init_gds_vmid(adev);
>>     >>   }
>>     >>     static void gfx_v9_0_wait_for_rlc_serdes(struct
>>     amdgpu_device *adev)
>>     >
>>     > _______________________________________________
>>     > amd-gfx mailing list
>>     > amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
>>     <mailto:amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
>>     > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>     _______________________________________________
>>     amd-gfx mailing list
>>     amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org <mailto:amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
>>     https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx


[-- Attachment #1.2: Type: text/html, Size: 20515 bytes --]

[-- Attachment #2: Type: text/plain, Size: 153 bytes --]

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] drm/amdgpu: Default disable GDS for compute+gfx
       [not found]                             ` <293feae7-bda5-d8af-2dca-6f2025dc3fba-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2019-08-29  9:06                               ` zhoucm1
       [not found]                                 ` <148e2050-558c-eee6-bb74-3f7f9a341f98-5C7GfCeVMHo@public.gmane.org>
  0 siblings, 1 reply; 10+ messages in thread
From: zhoucm1 @ 2019-08-29  9:06 UTC (permalink / raw)
  To: christian.koenig-5C7GfCeVMHo, Marek Olšák
  Cc: Qiao, Ken, Greathouse, Joseph, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW


[-- Attachment #1.1: Type: text/plain, Size: 14046 bytes --]


On 2019/8/29 下午3:22, Christian König wrote:
> Am 29.08.19 um 07:55 schrieb zhoucm1:
>>
>>
>> On 2019/8/29 上午1:08, Marek Olšák wrote:
>>> It can't break an older driver, because there is no older driver 
>>> that requires the static allocation.
>>>
>>> Note that closed source drivers don't count, because they don't need 
>>> backward compatibility.
>>
>> Yes, I agree, we don't need take care of closed source stack.
>>
>> But AMDVLK is indeed an open source stack, many fans are using it, we 
>> need keep its compatibility, don't we?
>>
>
> Actually that is still under discussion.
>
> But AMDVLK should have never ever used the static GDS space in the 
> first place. We only added that for a transition time for old OpenGL 
> and it shouldn't have leaked into the upstream driver.
>
> Not sure what's the best approach here. We could revert "[PATCH] 
> drm/amdgpu: remove static GDS, GWS and OA", but that would break KFD. 
> So we can only choose between two evils here.
>
> Only alternative I can see which would work for both would be to still 
> allocate the static GDS, GWS and OA space, but make it somehow dynamic 
> so that the KFD can swap it out again.

Agree with you.

-David

>
> Christian.
>
>> -David
>>
>>>
>>> Marek
>>>
>>> On Wed, Aug 28, 2019 at 2:44 AM zhoucm1 <zhoucm1-5C7GfCeVMHo@public.gmane.org 
>>> <mailto:zhoucm1-5C7GfCeVMHo@public.gmane.org>> wrote:
>>>
>>>
>>>     On 2019/7/23 上午3:08, Christian König wrote:
>>>     > Am 22.07.19 um 17:34 schrieb Greathouse, Joseph:
>>>     >> Units in the GDS block default to allowing all VMIDs access
>>>     to all
>>>     >> entries. Disable shader access to the GDS, GWS, and OA blocks
>>>     from all
>>>     >> compute and gfx VMIDs by default. For compute, HWS firmware
>>>     will set
>>>     >> up the access bits for the appropriate VMID when a compute queue
>>>     >> requires access to these blocks.
>>>     >> The driver will handle enabling access on-demand for graphics
>>>     VMIDs.
>>>
>>>     gds_switch is depending on job->gds/gws/oa/_base/size.
>>>
>>>     "[PATCH] drm/amdgpu: remove static GDS, GWS and OA allocation", the
>>>     default allocations in kernel were removed. If some UMD stacks
>>>     don't
>>>     pass gds/gws/oa allocation to bo_list, then kernel will not enable
>>>     access of them, that will break previous driver.
>>>
>>>     do we need revert "[PATCH] drm/amdgpu: remove static GDS, GWS
>>>     and OA
>>>     allocation" ?
>>>
>>>     -David
>>>
>>>     >>
>>>     >> Leaving VMID0 with full access because otherwise HWS cannot
>>>     save or
>>>     >> restore values during task switch.
>>>     >>
>>>     >> v2: Fixed code and comment styling.
>>>     >>
>>>     >> Change-Id: I3d768a96935d2ed1dff09b02c995090f4fbfa539
>>>     >> Signed-off-by: Joseph Greathouse <Joseph.Greathouse-5C7GfCeVMHo@public.gmane.org
>>>     <mailto:Joseph.Greathouse-5C7GfCeVMHo@public.gmane.org>>
>>>     >
>>>     > Reviewed-by: Christian König <christian.koenig-5C7GfCeVMHo@public.gmane.org
>>>     <mailto:christian.koenig-5C7GfCeVMHo@public.gmane.org>>
>>>     >
>>>     >> ---
>>>     >>   drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 25
>>>     ++++++++++++++++++-------
>>>     >>   drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c  | 24
>>>     +++++++++++++++++-------
>>>     >>   drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c  | 24
>>>     +++++++++++++++++-------
>>>     >>   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c  | 24
>>>     +++++++++++++++++-------
>>>     >>   4 files changed, 69 insertions(+), 28 deletions(-)
>>>     >>
>>>     >> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
>>>     >> b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
>>>     >> index 73dcb632a3ce..2a9692bc34b4 100644
>>>     >> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
>>>     >> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
>>>     >> @@ -1516,17 +1516,27 @@ static void
>>>     >> gfx_v10_0_init_compute_vmid(struct amdgpu_device *adev)
>>>     >>       }
>>>     >>       nv_grbm_select(adev, 0, 0, 0, 0);
>>>     >>       mutex_unlock(&adev->srbm_mutex);
>>>     >> +}
>>>     >>   -    /* Initialize all compute VMIDs to have no GDS, GWS, or OA
>>>     >> -       acccess. These should be enabled by FW for target
>>>     VMIDs. */
>>>     >> -    for (i = FIRST_COMPUTE_VMID; i < LAST_COMPUTE_VMID; i++) {
>>>     >> -        WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_BASE, 2 * i, 0);
>>>     >> -        WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_SIZE, 2 * i, 0);
>>>     >> -        WREG32_SOC15_OFFSET(GC, 0, mmGDS_GWS_VMID0, i, 0);
>>>     >> -        WREG32_SOC15_OFFSET(GC, 0, mmGDS_OA_VMID0, i, 0);
>>>     >> +static void gfx_v10_0_init_gds_vmid(struct amdgpu_device *adev)
>>>     >> +{
>>>     >> +    int vmid;
>>>     >> +
>>>     >> +    /*
>>>     >> +     * Initialize all compute and user-gfx VMIDs to have no
>>>     GDS,
>>>     >> GWS, or OA
>>>     >> +     * access. Compute VMIDs should be enabled by FW for
>>>     target VMIDs,
>>>     >> +     * the driver can enable them for graphics. VMID0 should
>>>     maintain
>>>     >> +     * access so that HWS firmware can save/restore entries.
>>>     >> +     */
>>>     >> +    for (vmid = 1; vmid < 16; vmid++) {
>>>     >> +        WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_BASE, 2 *
>>>     vmid, 0);
>>>     >> +        WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_SIZE, 2 *
>>>     vmid, 0);
>>>     >> +        WREG32_SOC15_OFFSET(GC, 0, mmGDS_GWS_VMID0, vmid, 0);
>>>     >> +        WREG32_SOC15_OFFSET(GC, 0, mmGDS_OA_VMID0, vmid, 0);
>>>     >>       }
>>>     >>   }
>>>     >>   +
>>>     >>   static void gfx_v10_0_tcp_harvest(struct amdgpu_device *adev)
>>>     >>   {
>>>     >>       int i, j, k;
>>>     >> @@ -1629,6 +1639,7 @@ static void
>>>     gfx_v10_0_constants_init(struct
>>>     >> amdgpu_device *adev)
>>>     >>       mutex_unlock(&adev->srbm_mutex);
>>>     >>         gfx_v10_0_init_compute_vmid(adev);
>>>     >> +    gfx_v10_0_init_gds_vmid(adev);
>>>     >>     }
>>>     >>   diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
>>>     >> b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
>>>     >> index 3f98624772a4..48796b6824cf 100644
>>>     >> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
>>>     >> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
>>>     >> @@ -1877,14 +1877,23 @@ static void
>>>     gfx_v7_0_init_compute_vmid(struct
>>>     >> amdgpu_device *adev)
>>>     >>       }
>>>     >>       cik_srbm_select(adev, 0, 0, 0, 0);
>>>     >>       mutex_unlock(&adev->srbm_mutex);
>>>     >> +}
>>>     >>   -    /* Initialize all compute VMIDs to have no GDS, GWS, or OA
>>>     >> -       acccess. These should be enabled by FW for target
>>>     VMIDs. */
>>>     >> -    for (i = FIRST_COMPUTE_VMID; i < LAST_COMPUTE_VMID; i++) {
>>>     >> - WREG32(amdgpu_gds_reg_offset[i].mem_base, 0);
>>>     >> - WREG32(amdgpu_gds_reg_offset[i].mem_size, 0);
>>>     >> -        WREG32(amdgpu_gds_reg_offset[i].gws, 0);
>>>     >> -        WREG32(amdgpu_gds_reg_offset[i].oa, 0);
>>>     >> +static void gfx_v7_0_init_gds_vmid(struct amdgpu_device *adev)
>>>     >> +{
>>>     >> +    int vmid;
>>>     >> +
>>>     >> +    /*
>>>     >> +     * Initialize all compute and user-gfx VMIDs to have no
>>>     GDS,
>>>     >> GWS, or OA
>>>     >> +     * access. Compute VMIDs should be enabled by FW for
>>>     target VMIDs,
>>>     >> +     * the driver can enable them for graphics. VMID0 should
>>>     maintain
>>>     >> +     * access so that HWS firmware can save/restore entries.
>>>     >> +     */
>>>     >> +    for (vmid = 1; vmid < 16; vmid++) {
>>>     >> + WREG32(amdgpu_gds_reg_offset[vmid].mem_base, 0);
>>>     >> + WREG32(amdgpu_gds_reg_offset[vmid].mem_size, 0);
>>>     >> +        WREG32(amdgpu_gds_reg_offset[vmid].gws, 0);
>>>     >> +        WREG32(amdgpu_gds_reg_offset[vmid].oa, 0);
>>>     >>       }
>>>     >>   }
>>>     >>   @@ -1966,6 +1975,7 @@ static void
>>>     gfx_v7_0_constants_init(struct
>>>     >> amdgpu_device *adev)
>>>     >>       mutex_unlock(&adev->srbm_mutex);
>>>     >>         gfx_v7_0_init_compute_vmid(adev);
>>>     >> +    gfx_v7_0_init_gds_vmid(adev);
>>>     >>         WREG32(mmSX_DEBUG_1, 0x20);
>>>     >>   diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
>>>     >> b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
>>>     >> index e4028d54f8f7..d2907186bb24 100644
>>>     >> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
>>>     >> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
>>>     >> @@ -3702,14 +3702,23 @@ static void
>>>     gfx_v8_0_init_compute_vmid(struct
>>>     >> amdgpu_device *adev)
>>>     >>       }
>>>     >>       vi_srbm_select(adev, 0, 0, 0, 0);
>>>     >>       mutex_unlock(&adev->srbm_mutex);
>>>     >> +}
>>>     >>   -    /* Initialize all compute VMIDs to have no GDS, GWS, or OA
>>>     >> -       acccess. These should be enabled by FW for target
>>>     VMIDs. */
>>>     >> -    for (i = FIRST_COMPUTE_VMID; i < LAST_COMPUTE_VMID; i++) {
>>>     >> - WREG32(amdgpu_gds_reg_offset[i].mem_base, 0);
>>>     >> - WREG32(amdgpu_gds_reg_offset[i].mem_size, 0);
>>>     >> -        WREG32(amdgpu_gds_reg_offset[i].gws, 0);
>>>     >> -        WREG32(amdgpu_gds_reg_offset[i].oa, 0);
>>>     >> +static void gfx_v8_0_init_gds_vmid(struct amdgpu_device *adev)
>>>     >> +{
>>>     >> +    int vmid;
>>>     >> +
>>>     >> +    /*
>>>     >> +     * Initialize all compute and user-gfx VMIDs to have no
>>>     GDS,
>>>     >> GWS, or OA
>>>     >> +     * access. Compute VMIDs should be enabled by FW for
>>>     target VMIDs,
>>>     >> +     * the driver can enable them for graphics. VMID0 should
>>>     maintain
>>>     >> +     * access so that HWS firmware can save/restore entries.
>>>     >> +     */
>>>     >> +    for (vmid = 1; vmid < 16; vmid++) {
>>>     >> + WREG32(amdgpu_gds_reg_offset[vmid].mem_base, 0);
>>>     >> + WREG32(amdgpu_gds_reg_offset[vmid].mem_size, 0);
>>>     >> +        WREG32(amdgpu_gds_reg_offset[vmid].gws, 0);
>>>     >> +        WREG32(amdgpu_gds_reg_offset[vmid].oa, 0);
>>>     >>       }
>>>     >>   }
>>>     >>   @@ -3779,6 +3788,7 @@ static void
>>>     gfx_v8_0_constants_init(struct
>>>     >> amdgpu_device *adev)
>>>     >>       mutex_unlock(&adev->srbm_mutex);
>>>     >>         gfx_v8_0_init_compute_vmid(adev);
>>>     >> +    gfx_v8_0_init_gds_vmid(adev);
>>>     >>         mutex_lock(&adev->grbm_idx_mutex);
>>>     >>       /*
>>>     >> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>>>     >> b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>>>     >> index 259a35395fec..bd7f431a24d9 100644
>>>     >> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>>>     >> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>>>     >> @@ -2025,14 +2025,23 @@ static void
>>>     gfx_v9_0_init_compute_vmid(struct
>>>     >> amdgpu_device *adev)
>>>     >>       }
>>>     >>       soc15_grbm_select(adev, 0, 0, 0, 0);
>>>     >>       mutex_unlock(&adev->srbm_mutex);
>>>     >> +}
>>>     >>   -    /* Initialize all compute VMIDs to have no GDS, GWS, or OA
>>>     >> -       acccess. These should be enabled by FW for target
>>>     VMIDs. */
>>>     >> -    for (i = FIRST_COMPUTE_VMID; i < LAST_COMPUTE_VMID; i++) {
>>>     >> -        WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_BASE, 2 * i, 0);
>>>     >> -        WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_SIZE, 2 * i, 0);
>>>     >> -        WREG32_SOC15_OFFSET(GC, 0, mmGDS_GWS_VMID0, i, 0);
>>>     >> -        WREG32_SOC15_OFFSET(GC, 0, mmGDS_OA_VMID0, i, 0);
>>>     >> +static void gfx_v9_0_init_gds_vmid(struct amdgpu_device *adev)
>>>     >> +{
>>>     >> +    int vmid;
>>>     >> +
>>>     >> +    /*
>>>     >> +     * Initialize all compute and user-gfx VMIDs to have no
>>>     GDS,
>>>     >> GWS, or OA
>>>     >> +     * access. Compute VMIDs should be enabled by FW for
>>>     target VMIDs,
>>>     >> +     * the driver can enable them for graphics. VMID0 should
>>>     maintain
>>>     >> +     * access so that HWS firmware can save/restore entries.
>>>     >> +     */
>>>     >> +    for (vmid = 1; vmid < 16; vmid++) {
>>>     >> +        WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_BASE, 2 *
>>>     vmid, 0);
>>>     >> +        WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_SIZE, 2 *
>>>     vmid, 0);
>>>     >> +        WREG32_SOC15_OFFSET(GC, 0, mmGDS_GWS_VMID0, vmid, 0);
>>>     >> +        WREG32_SOC15_OFFSET(GC, 0, mmGDS_OA_VMID0, vmid, 0);
>>>     >>       }
>>>     >>   }
>>>     >>   @@ -2080,6 +2089,7 @@ static void
>>>     gfx_v9_0_constants_init(struct
>>>     >> amdgpu_device *adev)
>>>     >>       mutex_unlock(&adev->srbm_mutex);
>>>     >>         gfx_v9_0_init_compute_vmid(adev);
>>>     >> +    gfx_v9_0_init_gds_vmid(adev);
>>>     >>   }
>>>     >>     static void gfx_v9_0_wait_for_rlc_serdes(struct
>>>     amdgpu_device *adev)
>>>     >
>>>     > _______________________________________________
>>>     > amd-gfx mailing list
>>>     > amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
>>>     <mailto:amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
>>>     > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>>     _______________________________________________
>>>     amd-gfx mailing list
>>>     amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org <mailto:amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
>>>     https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>>
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>

[-- Attachment #1.2: Type: text/html, Size: 21731 bytes --]

[-- Attachment #2: Type: text/plain, Size: 153 bytes --]

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] drm/amdgpu: Default disable GDS for compute+gfx
       [not found]                                 ` <148e2050-558c-eee6-bb74-3f7f9a341f98-5C7GfCeVMHo@public.gmane.org>
@ 2019-08-29 20:30                                   ` Marek Olšák
  0 siblings, 0 replies; 10+ messages in thread
From: Marek Olšák @ 2019-08-29 20:30 UTC (permalink / raw)
  To: zhoucm1
  Cc: Greathouse, Joseph, Qiao, Ken, Christian König,
	amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW


[-- Attachment #1.1: Type: text/plain, Size: 12397 bytes --]

If you decide to add it back, use this instead, it's simpler:
https://patchwork.freedesktop.org/patch/318391/?series=63775&rev=1

Maybe remove OA reservation if you don't need it.

Marek

On Thu, Aug 29, 2019 at 5:06 AM zhoucm1 <zhoucm1-5C7GfCeVMHo@public.gmane.org> wrote:

>
> On 2019/8/29 下午3:22, Christian König wrote:
>
> Am 29.08.19 um 07:55 schrieb zhoucm1:
>
>
> On 2019/8/29 上午1:08, Marek Olšák wrote:
>
> It can't break an older driver, because there is no older driver that
> requires the static allocation.
>
> Note that closed source drivers don't count, because they don't need
> backward compatibility.
>
> Yes, I agree, we don't need take care of closed source stack.
>
> But AMDVLK is indeed an open source stack, many fans are using it, we need
> keep its compatibility, don't we?
>
>
> Actually that is still under discussion.
>
> But AMDVLK should have never ever used the static GDS space in the first
> place. We only added that for a transition time for old OpenGL and it
> shouldn't have leaked into the upstream driver.
>
> Not sure what's the best approach here. We could revert "[PATCH]
> drm/amdgpu: remove static GDS, GWS and OA", but that would break KFD. So we
> can only choose between two evils here.
>
> Only alternative I can see which would work for both would be to still
> allocate the static GDS, GWS and OA space, but make it somehow dynamic so
> that the KFD can swap it out again.
>
> Agree with you.
>
> -David
>
>
> Christian.
>
> -David
>
>
> Marek
>
> On Wed, Aug 28, 2019 at 2:44 AM zhoucm1 <zhoucm1-5C7GfCeVMHo@public.gmane.org> wrote:
>
>>
>> On 2019/7/23 上午3:08, Christian König wrote:
>> > Am 22.07.19 um 17:34 schrieb Greathouse, Joseph:
>> >> Units in the GDS block default to allowing all VMIDs access to all
>> >> entries. Disable shader access to the GDS, GWS, and OA blocks from all
>> >> compute and gfx VMIDs by default. For compute, HWS firmware will set
>> >> up the access bits for the appropriate VMID when a compute queue
>> >> requires access to these blocks.
>> >> The driver will handle enabling access on-demand for graphics VMIDs.
>>
>> gds_switch is depending on job->gds/gws/oa/_base/size.
>>
>> "[PATCH] drm/amdgpu: remove static GDS, GWS and OA allocation", the
>> default allocations in kernel were removed. If some UMD stacks don't
>> pass gds/gws/oa allocation to bo_list, then kernel will not enable
>> access of them, that will break previous driver.
>>
>> do we need revert "[PATCH] drm/amdgpu: remove static GDS, GWS and OA
>> allocation" ?
>>
>> -David
>>
>> >>
>> >> Leaving VMID0 with full access because otherwise HWS cannot save or
>> >> restore values during task switch.
>> >>
>> >> v2: Fixed code and comment styling.
>> >>
>> >> Change-Id: I3d768a96935d2ed1dff09b02c995090f4fbfa539
>> >> Signed-off-by: Joseph Greathouse <Joseph.Greathouse-5C7GfCeVMHo@public.gmane.org>
>> >
>> > Reviewed-by: Christian König <christian.koenig-5C7GfCeVMHo@public.gmane.org>
>> >
>> >> ---
>> >>   drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 25 ++++++++++++++++++-------
>> >>   drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c  | 24 +++++++++++++++++-------
>> >>   drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c  | 24 +++++++++++++++++-------
>> >>   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c  | 24 +++++++++++++++++-------
>> >>   4 files changed, 69 insertions(+), 28 deletions(-)
>> >>
>> >> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
>> >> b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
>> >> index 73dcb632a3ce..2a9692bc34b4 100644
>> >> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
>> >> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
>> >> @@ -1516,17 +1516,27 @@ static void
>> >> gfx_v10_0_init_compute_vmid(struct amdgpu_device *adev)
>> >>       }
>> >>       nv_grbm_select(adev, 0, 0, 0, 0);
>> >>       mutex_unlock(&adev->srbm_mutex);
>> >> +}
>> >>   -    /* Initialize all compute VMIDs to have no GDS, GWS, or OA
>> >> -       acccess. These should be enabled by FW for target VMIDs. */
>> >> -    for (i = FIRST_COMPUTE_VMID; i < LAST_COMPUTE_VMID; i++) {
>> >> -        WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_BASE, 2 * i, 0);
>> >> -        WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_SIZE, 2 * i, 0);
>> >> -        WREG32_SOC15_OFFSET(GC, 0, mmGDS_GWS_VMID0, i, 0);
>> >> -        WREG32_SOC15_OFFSET(GC, 0, mmGDS_OA_VMID0, i, 0);
>> >> +static void gfx_v10_0_init_gds_vmid(struct amdgpu_device *adev)
>> >> +{
>> >> +    int vmid;
>> >> +
>> >> +    /*
>> >> +     * Initialize all compute and user-gfx VMIDs to have no GDS,
>> >> GWS, or OA
>> >> +     * access. Compute VMIDs should be enabled by FW for target VMIDs,
>> >> +     * the driver can enable them for graphics. VMID0 should maintain
>> >> +     * access so that HWS firmware can save/restore entries.
>> >> +     */
>> >> +    for (vmid = 1; vmid < 16; vmid++) {
>> >> +        WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_BASE, 2 * vmid, 0);
>> >> +        WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_SIZE, 2 * vmid, 0);
>> >> +        WREG32_SOC15_OFFSET(GC, 0, mmGDS_GWS_VMID0, vmid, 0);
>> >> +        WREG32_SOC15_OFFSET(GC, 0, mmGDS_OA_VMID0, vmid, 0);
>> >>       }
>> >>   }
>> >>   +
>> >>   static void gfx_v10_0_tcp_harvest(struct amdgpu_device *adev)
>> >>   {
>> >>       int i, j, k;
>> >> @@ -1629,6 +1639,7 @@ static void gfx_v10_0_constants_init(struct
>> >> amdgpu_device *adev)
>> >>       mutex_unlock(&adev->srbm_mutex);
>> >>         gfx_v10_0_init_compute_vmid(adev);
>> >> +    gfx_v10_0_init_gds_vmid(adev);
>> >>     }
>> >>   diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
>> >> b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
>> >> index 3f98624772a4..48796b6824cf 100644
>> >> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
>> >> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
>> >> @@ -1877,14 +1877,23 @@ static void gfx_v7_0_init_compute_vmid(struct
>> >> amdgpu_device *adev)
>> >>       }
>> >>       cik_srbm_select(adev, 0, 0, 0, 0);
>> >>       mutex_unlock(&adev->srbm_mutex);
>> >> +}
>> >>   -    /* Initialize all compute VMIDs to have no GDS, GWS, or OA
>> >> -       acccess. These should be enabled by FW for target VMIDs. */
>> >> -    for (i = FIRST_COMPUTE_VMID; i < LAST_COMPUTE_VMID; i++) {
>> >> -        WREG32(amdgpu_gds_reg_offset[i].mem_base, 0);
>> >> -        WREG32(amdgpu_gds_reg_offset[i].mem_size, 0);
>> >> -        WREG32(amdgpu_gds_reg_offset[i].gws, 0);
>> >> -        WREG32(amdgpu_gds_reg_offset[i].oa, 0);
>> >> +static void gfx_v7_0_init_gds_vmid(struct amdgpu_device *adev)
>> >> +{
>> >> +    int vmid;
>> >> +
>> >> +    /*
>> >> +     * Initialize all compute and user-gfx VMIDs to have no GDS,
>> >> GWS, or OA
>> >> +     * access. Compute VMIDs should be enabled by FW for target VMIDs,
>> >> +     * the driver can enable them for graphics. VMID0 should maintain
>> >> +     * access so that HWS firmware can save/restore entries.
>> >> +     */
>> >> +    for (vmid = 1; vmid < 16; vmid++) {
>> >> +        WREG32(amdgpu_gds_reg_offset[vmid].mem_base, 0);
>> >> +        WREG32(amdgpu_gds_reg_offset[vmid].mem_size, 0);
>> >> +        WREG32(amdgpu_gds_reg_offset[vmid].gws, 0);
>> >> +        WREG32(amdgpu_gds_reg_offset[vmid].oa, 0);
>> >>       }
>> >>   }
>> >>   @@ -1966,6 +1975,7 @@ static void gfx_v7_0_constants_init(struct
>> >> amdgpu_device *adev)
>> >>       mutex_unlock(&adev->srbm_mutex);
>> >>         gfx_v7_0_init_compute_vmid(adev);
>> >> +    gfx_v7_0_init_gds_vmid(adev);
>> >>         WREG32(mmSX_DEBUG_1, 0x20);
>> >>   diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
>> >> b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
>> >> index e4028d54f8f7..d2907186bb24 100644
>> >> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
>> >> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
>> >> @@ -3702,14 +3702,23 @@ static void gfx_v8_0_init_compute_vmid(struct
>> >> amdgpu_device *adev)
>> >>       }
>> >>       vi_srbm_select(adev, 0, 0, 0, 0);
>> >>       mutex_unlock(&adev->srbm_mutex);
>> >> +}
>> >>   -    /* Initialize all compute VMIDs to have no GDS, GWS, or OA
>> >> -       acccess. These should be enabled by FW for target VMIDs. */
>> >> -    for (i = FIRST_COMPUTE_VMID; i < LAST_COMPUTE_VMID; i++) {
>> >> -        WREG32(amdgpu_gds_reg_offset[i].mem_base, 0);
>> >> -        WREG32(amdgpu_gds_reg_offset[i].mem_size, 0);
>> >> -        WREG32(amdgpu_gds_reg_offset[i].gws, 0);
>> >> -        WREG32(amdgpu_gds_reg_offset[i].oa, 0);
>> >> +static void gfx_v8_0_init_gds_vmid(struct amdgpu_device *adev)
>> >> +{
>> >> +    int vmid;
>> >> +
>> >> +    /*
>> >> +     * Initialize all compute and user-gfx VMIDs to have no GDS,
>> >> GWS, or OA
>> >> +     * access. Compute VMIDs should be enabled by FW for target VMIDs,
>> >> +     * the driver can enable them for graphics. VMID0 should maintain
>> >> +     * access so that HWS firmware can save/restore entries.
>> >> +     */
>> >> +    for (vmid = 1; vmid < 16; vmid++) {
>> >> +        WREG32(amdgpu_gds_reg_offset[vmid].mem_base, 0);
>> >> +        WREG32(amdgpu_gds_reg_offset[vmid].mem_size, 0);
>> >> +        WREG32(amdgpu_gds_reg_offset[vmid].gws, 0);
>> >> +        WREG32(amdgpu_gds_reg_offset[vmid].oa, 0);
>> >>       }
>> >>   }
>> >>   @@ -3779,6 +3788,7 @@ static void gfx_v8_0_constants_init(struct
>> >> amdgpu_device *adev)
>> >>       mutex_unlock(&adev->srbm_mutex);
>> >>         gfx_v8_0_init_compute_vmid(adev);
>> >> +    gfx_v8_0_init_gds_vmid(adev);
>> >>         mutex_lock(&adev->grbm_idx_mutex);
>> >>       /*
>> >> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>> >> b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>> >> index 259a35395fec..bd7f431a24d9 100644
>> >> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>> >> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>> >> @@ -2025,14 +2025,23 @@ static void gfx_v9_0_init_compute_vmid(struct
>> >> amdgpu_device *adev)
>> >>       }
>> >>       soc15_grbm_select(adev, 0, 0, 0, 0);
>> >>       mutex_unlock(&adev->srbm_mutex);
>> >> +}
>> >>   -    /* Initialize all compute VMIDs to have no GDS, GWS, or OA
>> >> -       acccess. These should be enabled by FW for target VMIDs. */
>> >> -    for (i = FIRST_COMPUTE_VMID; i < LAST_COMPUTE_VMID; i++) {
>> >> -        WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_BASE, 2 * i, 0);
>> >> -        WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_SIZE, 2 * i, 0);
>> >> -        WREG32_SOC15_OFFSET(GC, 0, mmGDS_GWS_VMID0, i, 0);
>> >> -        WREG32_SOC15_OFFSET(GC, 0, mmGDS_OA_VMID0, i, 0);
>> >> +static void gfx_v9_0_init_gds_vmid(struct amdgpu_device *adev)
>> >> +{
>> >> +    int vmid;
>> >> +
>> >> +    /*
>> >> +     * Initialize all compute and user-gfx VMIDs to have no GDS,
>> >> GWS, or OA
>> >> +     * access. Compute VMIDs should be enabled by FW for target VMIDs,
>> >> +     * the driver can enable them for graphics. VMID0 should maintain
>> >> +     * access so that HWS firmware can save/restore entries.
>> >> +     */
>> >> +    for (vmid = 1; vmid < 16; vmid++) {
>> >> +        WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_BASE, 2 * vmid, 0);
>> >> +        WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_SIZE, 2 * vmid, 0);
>> >> +        WREG32_SOC15_OFFSET(GC, 0, mmGDS_GWS_VMID0, vmid, 0);
>> >> +        WREG32_SOC15_OFFSET(GC, 0, mmGDS_OA_VMID0, vmid, 0);
>> >>       }
>> >>   }
>> >>   @@ -2080,6 +2089,7 @@ static void gfx_v9_0_constants_init(struct
>> >> amdgpu_device *adev)
>> >>       mutex_unlock(&adev->srbm_mutex);
>> >>         gfx_v9_0_init_compute_vmid(adev);
>> >> +    gfx_v9_0_init_gds_vmid(adev);
>> >>   }
>> >>     static void gfx_v9_0_wait_for_rlc_serdes(struct amdgpu_device
>> *adev)
>> >
>> > _______________________________________________
>> > amd-gfx mailing list
>> > amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
>> > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
>
> _______________________________________________
> amd-gfx mailing listamd-gfx-PD4FTy7X32lNgt0PjOBp934avgP/u3fG0wdF1cv0I5s@public.gmane.org://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
>
>

[-- Attachment #1.2: Type: text/html, Size: 22017 bytes --]

[-- Attachment #2: Type: text/plain, Size: 153 bytes --]

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2019-08-29 20:30 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-19 23:46 [PATCH] drm/amdgpu: Default disable GDS for compute+gfx Greathouse, Joseph
     [not found] ` <20190719234612.8198-1-Joseph.Greathouse-5C7GfCeVMHo@public.gmane.org>
2019-07-21 14:51   ` Koenig, Christian
     [not found]     ` <7002f783-2223-a524-2c60-03447377bd28-5C7GfCeVMHo@public.gmane.org>
2019-07-22 15:34       ` [PATCH v2] " Greathouse, Joseph
     [not found]         ` <20190722153350.29339-1-Joseph.Greathouse-5C7GfCeVMHo@public.gmane.org>
2019-07-22 19:08           ` Christian König
     [not found]             ` <c4856ea0-3cd3-1628-54e9-9660be334054-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2019-08-28  6:41               ` zhoucm1
     [not found]                 ` <36885047-8915-fb90-fcaa-04f98b4519b1-5C7GfCeVMHo@public.gmane.org>
2019-08-28 17:08                   ` Marek Olšák
     [not found]                     ` <CAAxE2A5MgKswdeHg1YrDs_G7hCuoZAVFuPyoHoA+DbUjA32eHg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2019-08-29  5:55                       ` zhoucm1
     [not found]                         ` <bdb61ac5-e1ab-725c-56cf-475d7b41ebc2-5C7GfCeVMHo@public.gmane.org>
2019-08-29  7:22                           ` Christian König
     [not found]                             ` <293feae7-bda5-d8af-2dca-6f2025dc3fba-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2019-08-29  9:06                               ` zhoucm1
     [not found]                                 ` <148e2050-558c-eee6-bb74-3f7f9a341f98-5C7GfCeVMHo@public.gmane.org>
2019-08-29 20:30                                   ` Marek Olšák

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.