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

zhoucm1 zhoucm1 at amd.com
Thu Aug 29 05:55:31 UTC 2019


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?

-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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20190829/663f67d1/attachment-0001.html>


More information about the amd-gfx mailing list