[PATCH] drm/amdgpu: Default disable GDS for compute+gfx

Koenig, Christian Christian.Koenig at amd.com
Sun Jul 21 14:51:28 UTC 2019


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 at 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)



More information about the amd-gfx mailing list