<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
  </head>
  <body text="#000000" bgcolor="#FFFFFF">
    <p><br>
    </p>
    <div class="moz-cite-prefix">On 2019/8/29 下午3:22, Christian König
      wrote:<br>
    </div>
    <blockquote type="cite"
      cite="mid:293feae7-bda5-d8af-2dca-6f2025dc3fba@gmail.com">
      <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
      <div class="moz-cite-prefix">Am 29.08.19 um 07:55 schrieb zhoucm1:<br>
      </div>
      <blockquote type="cite"
        cite="mid:bdb61ac5-e1ab-725c-56cf-475d7b41ebc2@amd.com">
        <p><br>
        </p>
        <div class="moz-cite-prefix">On 2019/8/29 上午1:08, Marek Olšák
          wrote:<br>
        </div>
        <blockquote type="cite"
cite="mid:CAAxE2A5MgKswdeHg1YrDs_G7hCuoZAVFuPyoHoA+DbUjA32eHg@mail.gmail.com">
          <div dir="ltr">
            <div>It can't break an older driver, because there is no
              older driver that requires the static allocation.</div>
            <div><br>
            </div>
            <div>Note that closed source drivers don't count, because
              they don't need backward compatibility.</div>
          </div>
        </blockquote>
        <p>Yes, I agree, we don't need take care of closed source stack.</p>
        <p>But AMDVLK is indeed an open source stack, many fans are
          using it, we need keep its compatibility, don't we?</p>
      </blockquote>
      <br>
      Actually that is still under discussion.<br>
      <br>
      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.<br>
      <br>
      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.<br>
      <br>
      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.<br>
    </blockquote>
    <p>Agree with you.</p>
    <p>-David<br>
    </p>
    <blockquote type="cite"
      cite="mid:293feae7-bda5-d8af-2dca-6f2025dc3fba@gmail.com"> <br>
      Christian.<br>
      <br>
      <blockquote type="cite"
        cite="mid:bdb61ac5-e1ab-725c-56cf-475d7b41ebc2@amd.com">
        <p>-David<br>
        </p>
        <blockquote type="cite"
cite="mid:CAAxE2A5MgKswdeHg1YrDs_G7hCuoZAVFuPyoHoA+DbUjA32eHg@mail.gmail.com">
          <div dir="ltr">
            <div><br>
            </div>
            <div>Marek<br>
            </div>
          </div>
          <br>
          <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"
                moz-do-not-send="true">zhoucm1@amd.com</a>> wrote:<br>
            </div>
            <blockquote class="gmail_quote" style="margin:0px 0px 0px
              0.8ex;border-left:1px solid
              rgb(204,204,204);padding-left:1ex"><br>
              On 2019/7/23 上午3:08, Christian König wrote:<br>
              > Am 22.07.19 um 17:34 schrieb Greathouse, Joseph:<br>
              >> Units in the GDS block default to allowing all
              VMIDs access to all<br>
              >> entries. Disable shader access to the GDS, GWS,
              and OA blocks from all<br>
              >> compute and gfx VMIDs by default. For compute,
              HWS firmware will set<br>
              >> up the access bits for the appropriate VMID when
              a compute queue<br>
              >> requires access to these blocks.<br>
              >> The driver will handle enabling access on-demand
              for graphics VMIDs.<br>
              <br>
              gds_switch is depending on job->gds/gws/oa/_base/size.<br>
              <br>
              "[PATCH] drm/amdgpu: remove static GDS, GWS and OA
              allocation", the <br>
              default allocations in kernel were removed. If some UMD
              stacks don't <br>
              pass gds/gws/oa allocation to bo_list, then kernel will
              not enable <br>
              access of them, that will break previous driver.<br>
              <br>
              do we need revert "[PATCH] drm/amdgpu: remove static GDS,
              GWS and OA <br>
              allocation" ?<br>
              <br>
              -David<br>
              <br>
              >><br>
              >> Leaving VMID0 with full access because otherwise
              HWS cannot save or<br>
              >> restore values during task switch.<br>
              >><br>
              >> v2: Fixed code and comment styling.<br>
              >><br>
              >> Change-Id:
              I3d768a96935d2ed1dff09b02c995090f4fbfa539<br>
              >> Signed-off-by: Joseph Greathouse <<a
                href="mailto:Joseph.Greathouse@amd.com" target="_blank"
                moz-do-not-send="true">Joseph.Greathouse@amd.com</a>><br>
              ><br>
              > Reviewed-by: Christian König <<a
                href="mailto:christian.koenig@amd.com" target="_blank"
                moz-do-not-send="true">christian.koenig@amd.com</a>><br>
              ><br>
              >> ---<br>
              >>   drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 25
              ++++++++++++++++++-------<br>
              >>   drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c  | 24
              +++++++++++++++++-------<br>
              >>   drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c  | 24
              +++++++++++++++++-------<br>
              >>   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c  | 24
              +++++++++++++++++-------<br>
              >>   4 files changed, 69 insertions(+), 28
              deletions(-)<br>
              >><br>
              >> diff --git
              a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c <br>
              >> b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c<br>
              >> index 73dcb632a3ce..2a9692bc34b4 100644<br>
              >> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c<br>
              >> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c<br>
              >> @@ -1516,17 +1516,27 @@ static void <br>
              >> gfx_v10_0_init_compute_vmid(struct amdgpu_device
              *adev)<br>
              >>       }<br>
              >>       nv_grbm_select(adev, 0, 0, 0, 0);<br>
              >>       mutex_unlock(&adev->srbm_mutex);<br>
              >> +}<br>
              >>   -    /* Initialize all compute VMIDs to have no
              GDS, GWS, or OA<br>
              >> -       acccess. These should be enabled by FW
              for target VMIDs. */<br>
              >> -    for (i = FIRST_COMPUTE_VMID; i <
              LAST_COMPUTE_VMID; i++) {<br>
              >> -        WREG32_SOC15_OFFSET(GC, 0,
              mmGDS_VMID0_BASE, 2 * i, 0);<br>
              >> -        WREG32_SOC15_OFFSET(GC, 0,
              mmGDS_VMID0_SIZE, 2 * i, 0);<br>
              >> -        WREG32_SOC15_OFFSET(GC, 0,
              mmGDS_GWS_VMID0, i, 0);<br>
              >> -        WREG32_SOC15_OFFSET(GC, 0,
              mmGDS_OA_VMID0, i, 0);<br>
              >> +static void gfx_v10_0_init_gds_vmid(struct
              amdgpu_device *adev)<br>
              >> +{<br>
              >> +    int vmid;<br>
              >> +<br>
              >> +    /*<br>
              >> +     * Initialize all compute and user-gfx VMIDs
              to have no GDS, <br>
              >> GWS, or OA<br>
              >> +     * access. Compute VMIDs should be enabled
              by FW for target VMIDs,<br>
              >> +     * the driver can enable them for graphics.
              VMID0 should maintain<br>
              >> +     * access so that HWS firmware can
              save/restore entries.<br>
              >> +     */<br>
              >> +    for (vmid = 1; vmid < 16; vmid++) {<br>
              >> +        WREG32_SOC15_OFFSET(GC, 0,
              mmGDS_VMID0_BASE, 2 * vmid, 0);<br>
              >> +        WREG32_SOC15_OFFSET(GC, 0,
              mmGDS_VMID0_SIZE, 2 * vmid, 0);<br>
              >> +        WREG32_SOC15_OFFSET(GC, 0,
              mmGDS_GWS_VMID0, vmid, 0);<br>
              >> +        WREG32_SOC15_OFFSET(GC, 0,
              mmGDS_OA_VMID0, vmid, 0);<br>
              >>       }<br>
              >>   }<br>
              >>   +<br>
              >>   static void gfx_v10_0_tcp_harvest(struct
              amdgpu_device *adev)<br>
              >>   {<br>
              >>       int i, j, k;<br>
              >> @@ -1629,6 +1639,7 @@ static void
              gfx_v10_0_constants_init(struct <br>
              >> amdgpu_device *adev)<br>
              >>       mutex_unlock(&adev->srbm_mutex);<br>
              >>         gfx_v10_0_init_compute_vmid(adev);<br>
              >> +    gfx_v10_0_init_gds_vmid(adev);<br>
              >>     }<br>
              >>   diff --git
              a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c <br>
              >> b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c<br>
              >> index 3f98624772a4..48796b6824cf 100644<br>
              >> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c<br>
              >> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c<br>
              >> @@ -1877,14 +1877,23 @@ static void
              gfx_v7_0_init_compute_vmid(struct <br>
              >> amdgpu_device *adev)<br>
              >>       }<br>
              >>       cik_srbm_select(adev, 0, 0, 0, 0);<br>
              >>       mutex_unlock(&adev->srbm_mutex);<br>
              >> +}<br>
              >>   -    /* Initialize all compute VMIDs to have no
              GDS, GWS, or OA<br>
              >> -       acccess. These should be enabled by FW
              for target VMIDs. */<br>
              >> -    for (i = FIRST_COMPUTE_VMID; i <
              LAST_COMPUTE_VMID; i++) {<br>
              >> -       
              WREG32(amdgpu_gds_reg_offset[i].mem_base, 0);<br>
              >> -       
              WREG32(amdgpu_gds_reg_offset[i].mem_size, 0);<br>
              >> -        WREG32(amdgpu_gds_reg_offset[i].gws, 0);<br>
              >> -        WREG32(amdgpu_gds_reg_offset[i].oa, 0);<br>
              >> +static void gfx_v7_0_init_gds_vmid(struct
              amdgpu_device *adev)<br>
              >> +{<br>
              >> +    int vmid;<br>
              >> +<br>
              >> +    /*<br>
              >> +     * Initialize all compute and user-gfx VMIDs
              to have no GDS, <br>
              >> GWS, or OA<br>
              >> +     * access. Compute VMIDs should be enabled
              by FW for target VMIDs,<br>
              >> +     * the driver can enable them for graphics.
              VMID0 should maintain<br>
              >> +     * access so that HWS firmware can
              save/restore entries.<br>
              >> +     */<br>
              >> +    for (vmid = 1; vmid < 16; vmid++) {<br>
              >> +       
              WREG32(amdgpu_gds_reg_offset[vmid].mem_base, 0);<br>
              >> +       
              WREG32(amdgpu_gds_reg_offset[vmid].mem_size, 0);<br>
              >> +        WREG32(amdgpu_gds_reg_offset[vmid].gws,
              0);<br>
              >> +        WREG32(amdgpu_gds_reg_offset[vmid].oa,
              0);<br>
              >>       }<br>
              >>   }<br>
              >>   @@ -1966,6 +1975,7 @@ static void
              gfx_v7_0_constants_init(struct <br>
              >> amdgpu_device *adev)<br>
              >>       mutex_unlock(&adev->srbm_mutex);<br>
              >>         gfx_v7_0_init_compute_vmid(adev);<br>
              >> +    gfx_v7_0_init_gds_vmid(adev);<br>
              >>         WREG32(mmSX_DEBUG_1, 0x20);<br>
              >>   diff --git
              a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c <br>
              >> b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c<br>
              >> index e4028d54f8f7..d2907186bb24 100644<br>
              >> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c<br>
              >> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c<br>
              >> @@ -3702,14 +3702,23 @@ static void
              gfx_v8_0_init_compute_vmid(struct <br>
              >> amdgpu_device *adev)<br>
              >>       }<br>
              >>       vi_srbm_select(adev, 0, 0, 0, 0);<br>
              >>       mutex_unlock(&adev->srbm_mutex);<br>
              >> +}<br>
              >>   -    /* Initialize all compute VMIDs to have no
              GDS, GWS, or OA<br>
              >> -       acccess. These should be enabled by FW
              for target VMIDs. */<br>
              >> -    for (i = FIRST_COMPUTE_VMID; i <
              LAST_COMPUTE_VMID; i++) {<br>
              >> -       
              WREG32(amdgpu_gds_reg_offset[i].mem_base, 0);<br>
              >> -       
              WREG32(amdgpu_gds_reg_offset[i].mem_size, 0);<br>
              >> -        WREG32(amdgpu_gds_reg_offset[i].gws, 0);<br>
              >> -        WREG32(amdgpu_gds_reg_offset[i].oa, 0);<br>
              >> +static void gfx_v8_0_init_gds_vmid(struct
              amdgpu_device *adev)<br>
              >> +{<br>
              >> +    int vmid;<br>
              >> +<br>
              >> +    /*<br>
              >> +     * Initialize all compute and user-gfx VMIDs
              to have no GDS, <br>
              >> GWS, or OA<br>
              >> +     * access. Compute VMIDs should be enabled
              by FW for target VMIDs,<br>
              >> +     * the driver can enable them for graphics.
              VMID0 should maintain<br>
              >> +     * access so that HWS firmware can
              save/restore entries.<br>
              >> +     */<br>
              >> +    for (vmid = 1; vmid < 16; vmid++) {<br>
              >> +       
              WREG32(amdgpu_gds_reg_offset[vmid].mem_base, 0);<br>
              >> +       
              WREG32(amdgpu_gds_reg_offset[vmid].mem_size, 0);<br>
              >> +        WREG32(amdgpu_gds_reg_offset[vmid].gws,
              0);<br>
              >> +        WREG32(amdgpu_gds_reg_offset[vmid].oa,
              0);<br>
              >>       }<br>
              >>   }<br>
              >>   @@ -3779,6 +3788,7 @@ static void
              gfx_v8_0_constants_init(struct <br>
              >> amdgpu_device *adev)<br>
              >>       mutex_unlock(&adev->srbm_mutex);<br>
              >>         gfx_v8_0_init_compute_vmid(adev);<br>
              >> +    gfx_v8_0_init_gds_vmid(adev);<br>
              >>         mutex_lock(&adev->grbm_idx_mutex);<br>
              >>       /*<br>
              >> diff --git
              a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c <br>
              >> b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c<br>
              >> index 259a35395fec..bd7f431a24d9 100644<br>
              >> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c<br>
              >> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c<br>
              >> @@ -2025,14 +2025,23 @@ static void
              gfx_v9_0_init_compute_vmid(struct <br>
              >> amdgpu_device *adev)<br>
              >>       }<br>
              >>       soc15_grbm_select(adev, 0, 0, 0, 0);<br>
              >>       mutex_unlock(&adev->srbm_mutex);<br>
              >> +}<br>
              >>   -    /* Initialize all compute VMIDs to have no
              GDS, GWS, or OA<br>
              >> -       acccess. These should be enabled by FW
              for target VMIDs. */<br>
              >> -    for (i = FIRST_COMPUTE_VMID; i <
              LAST_COMPUTE_VMID; i++) {<br>
              >> -        WREG32_SOC15_OFFSET(GC, 0,
              mmGDS_VMID0_BASE, 2 * i, 0);<br>
              >> -        WREG32_SOC15_OFFSET(GC, 0,
              mmGDS_VMID0_SIZE, 2 * i, 0);<br>
              >> -        WREG32_SOC15_OFFSET(GC, 0,
              mmGDS_GWS_VMID0, i, 0);<br>
              >> -        WREG32_SOC15_OFFSET(GC, 0,
              mmGDS_OA_VMID0, i, 0);<br>
              >> +static void gfx_v9_0_init_gds_vmid(struct
              amdgpu_device *adev)<br>
              >> +{<br>
              >> +    int vmid;<br>
              >> +<br>
              >> +    /*<br>
              >> +     * Initialize all compute and user-gfx VMIDs
              to have no GDS, <br>
              >> GWS, or OA<br>
              >> +     * access. Compute VMIDs should be enabled
              by FW for target VMIDs,<br>
              >> +     * the driver can enable them for graphics.
              VMID0 should maintain<br>
              >> +     * access so that HWS firmware can
              save/restore entries.<br>
              >> +     */<br>
              >> +    for (vmid = 1; vmid < 16; vmid++) {<br>
              >> +        WREG32_SOC15_OFFSET(GC, 0,
              mmGDS_VMID0_BASE, 2 * vmid, 0);<br>
              >> +        WREG32_SOC15_OFFSET(GC, 0,
              mmGDS_VMID0_SIZE, 2 * vmid, 0);<br>
              >> +        WREG32_SOC15_OFFSET(GC, 0,
              mmGDS_GWS_VMID0, vmid, 0);<br>
              >> +        WREG32_SOC15_OFFSET(GC, 0,
              mmGDS_OA_VMID0, vmid, 0);<br>
              >>       }<br>
              >>   }<br>
              >>   @@ -2080,6 +2089,7 @@ static void
              gfx_v9_0_constants_init(struct <br>
              >> amdgpu_device *adev)<br>
              >>       mutex_unlock(&adev->srbm_mutex);<br>
              >>         gfx_v9_0_init_compute_vmid(adev);<br>
              >> +    gfx_v9_0_init_gds_vmid(adev);<br>
              >>   }<br>
              >>     static void
              gfx_v9_0_wait_for_rlc_serdes(struct amdgpu_device *adev)<br>
              ><br>
              > _______________________________________________<br>
              > amd-gfx mailing list<br>
              > <a href="mailto:amd-gfx@lists.freedesktop.org"
                target="_blank" moz-do-not-send="true">amd-gfx@lists.freedesktop.org</a><br>
              > <a
                href="https://lists.freedesktop.org/mailman/listinfo/amd-gfx"
                rel="noreferrer" target="_blank" moz-do-not-send="true">https://lists.freedesktop.org/mailman/listinfo/amd-gfx</a><br>
              _______________________________________________<br>
              amd-gfx mailing list<br>
              <a href="mailto:amd-gfx@lists.freedesktop.org"
                target="_blank" moz-do-not-send="true">amd-gfx@lists.freedesktop.org</a><br>
              <a
                href="https://lists.freedesktop.org/mailman/listinfo/amd-gfx"
                rel="noreferrer" target="_blank" moz-do-not-send="true">https://lists.freedesktop.org/mailman/listinfo/amd-gfx</a></blockquote>
          </div>
        </blockquote>
        <br>
        <fieldset class="mimeAttachmentHeader"></fieldset>
        <pre class="moz-quote-pre" wrap="">_______________________________________________
amd-gfx mailing list
<a class="moz-txt-link-abbreviated" href="mailto:amd-gfx@lists.freedesktop.org" moz-do-not-send="true">amd-gfx@lists.freedesktop.org</a>
<a class="moz-txt-link-freetext" href="https://lists.freedesktop.org/mailman/listinfo/amd-gfx" moz-do-not-send="true">https://lists.freedesktop.org/mailman/listinfo/amd-gfx</a></pre>
      </blockquote>
      <br>
    </blockquote>
  </body>
</html>