drm/amdgpu: Default disable GDS for compute+gfx

Submitted by Greathouse, Joseph on July 19, 2019, 11:46 p.m.

Details

Message ID 20190719234612.8198-1-Joseph.Greathouse@amd.com
State New
Headers show
Series "drm/amdgpu: Default disable GDS for compute+gfx" ( rev: 1 ) in AMD X.Org drivers

Not browsing as part of any series.

Commit Message

Greathouse, Joseph July 19, 2019, 11:46 p.m.
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(-)

Patch hide | download patch | download mbox

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)

Comments

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)