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

[zhoucm1 <zhoucm1@xxxxxxx>]

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@xxxxxxx>
>
> Reviewed-by: Christian König <christian.koenig@xxxxxxx>
>
> > ---
> >   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@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx