[PATCH v2] drm/amdgpu: Default disable GDS for compute+gfx
zhoucm1
zhoucm1 at amd.com
Thu Aug 29 09:06:00 UTC 2019
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 at amd.com
>>> <mailto:zhoucm1 at amd.com>> 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 at amd.com
>>> <mailto:Joseph.Greathouse at amd.com>>
>>> >
>>> > Reviewed-by: Christian König <christian.koenig at amd.com
>>> <mailto:christian.koenig at 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 at lists.freedesktop.org
>>> <mailto:amd-gfx at lists.freedesktop.org>
>>> > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>> _______________________________________________
>>> amd-gfx mailing list
>>> amd-gfx at lists.freedesktop.org <mailto:amd-gfx at lists.freedesktop.org>
>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>>
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20190829/226f7123/attachment-0001.html>
More information about the amd-gfx
mailing list