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 <zhou...@amd.com> 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 <zhou...@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.greatho...@amd.com>
>> >
>> > Reviewed-by: Christian König <christian.koe...@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
>
>
> _______________________________________________
> amd-gfx mailing
> listamd-gfx@lists.freedesktop.orghttps://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
>
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
