<div dir="ltr"><div>It can't break an older driver, because there is no older driver that requires the static allocation.</div><div> </div><div>Note that closed source drivers don't count, because they don't need backward compatibility.</div><div> </div><div>Marek </div></div> <div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Aug 28, 2019 at 2:44 AM zhoucm1 <<a href="mailto:zhoucm1@amd.com">zhoucm1@amd.com</a>> wrote: </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> 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 <<a href="mailto:Joseph.Greathouse@amd.com" target="_blank">Joseph.Greathouse@amd.com</a>> > > Reviewed-by: Christian König <<a href="mailto:christian.koenig@amd.com" target="_blank">christian.koenig@amd.com</a>> > >> --- >> 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 > <a href="mailto:amd-gfx@lists.freedesktop.org" target="_blank">amd-gfx@lists.freedesktop.org</a> > <a href="https://lists.freedesktop.org/mailman/listinfo/amd-gfx" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/amd-gfx</a> _______________________________________________ amd-gfx mailing list <a href="mailto:amd-gfx@lists.freedesktop.org" target="_blank">amd-gfx@lists.freedesktop.org</a> <a href="https://lists.freedesktop.org/mailman/listinfo/amd-gfx" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/amd-gfx</a></blockquote></div>