<html>
  <head>
    <meta content="text/html; charset=utf-8" http-equiv="Content-Type">
  </head>
  <body text="#000000" bgcolor="#FFFFFF">
    <div class="moz-cite-prefix">
      <blockquote type="cite">It's my understanding that kmalloc
        allocates entire pages (and ideally contiguous ones) so it's a
        bit more precious.</blockquote>
      That's not correct. kmalloc uses the slab, slub or whatever
      allocator you have for small memory objects.<br>
      <br>
      It's vmalloc which allocates only entire pages and maps them into
      the virtual kernel address space.<br>
      <br>
      This has the advantage of not needing continuous pages for larger
      allocations, but the large disadvantage that it is much slower to
      allocate because of the page table manipulation.<br>
      <br>
      So for temporary and small things try to use kmalloc, for larger
      and long living allocations use vmalloc and if you aren't sure use
      drm_calloc_large.<br>
      <br>
      Regards,<br>
      Christian.<br>
      <br>
      Am 29.06.2016 um 18:37 schrieb StDenis, Tom:<br>
    </div>
    <blockquote
cite="mid:SN1PR12MB0784CC5571DD8B905759AAC2F7230@SN1PR12MB0784.namprd12.prod.outlook.com"
      type="cite">
      <meta http-equiv="Content-Type" content="text/html; charset=utf-8">
      <style type="text/css" style="display:none;"><!-- P {margin-top:0;margin-bottom:0;} --></style>
      <div id="divtagdefaultwrapper"
style="font-size:12pt;color:#000000;background-color:#FFFFFF;font-family:Calibri,Arial,Helvetica,sans-serif;">
        <p>It's my understanding that kmalloc allocates entire pages
          (and ideally contiguous ones) so it's a bit more precious.
           From my experience on the embedded space that was definitely
          the case.  Trying to allocate [say] multiple 64KB chunks could
          result in failures in short order.</p>
        <p><br>
        </p>
        <p>Ultimately I'm not against either approach since the memory
          is so short lived and the function is not performance
          sensitive.  I'd just like to get these pushed today so I can
          get the debug updates pushed out too.</p>
        <p><br>
        </p>
        <p>If you'd (or anyone) will slap a RB on it with kmalloc I'll
          gladly change it :-)</p>
        <p><br>
        </p>
        <p>Tom</p>
        <br>
        <br>
        <div style="color: rgb(0, 0, 0);">
          <div>
            <hr tabindex="-1" style="display:inline-block; width:98%">
            <div id="x_divRplyFwdMsg" dir="ltr"><font
                style="font-size:11pt" face="Calibri, sans-serif"
                color="#000000"><b>From:</b> amd-gfx
                <a class="moz-txt-link-rfc2396E" href="mailto:amd-gfx-bounces@lists.freedesktop.org"><amd-gfx-bounces@lists.freedesktop.org></a> on behalf
                of Nicolai Hähnle <a class="moz-txt-link-rfc2396E" href="mailto:nhaehnle@gmail.com"><nhaehnle@gmail.com></a><br>
                <b>Sent:</b> Wednesday, June 29, 2016 11:30<br>
                <b>To:</b> StDenis, Tom; Tom St Denis;
                <a class="moz-txt-link-abbreviated" href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a><br>
                <b>Subject:</b> Re: [PATCH 3/3] drm/amd/amdgpu: Add bank
                selection for MMIO debugfs (v3)</font>
              <div> </div>
            </div>
          </div>
          <font size="2"><span style="font-size:10pt;">
              <div class="PlainText">On 29.06.2016 16:22, StDenis, Tom
                wrote:<br>
                > Thanks.  I use vmalloc first to avoid putting the
                array on the stack (as<br>
                > good practice) then I chose vmalloc because there
                really isn't a need<br>
                > for contiguous memory and in theory it should be
                just as fast to<br>
                > allocate.  Wouldn't you typically use kmalloc when
                you need contiguous<br>
                > memory?  Though this all boils down to a single
                page since it's so small<br>
                > anyways.<br>
                <br>
                If it's potentially non-contiguous memory, isn't there
                the overhead of <br>
                page table manipulation?<br>
                <br>
                Maybe somebody who's more familiar with the kernel has
                an opinion...<br>
                <br>
                Cheers,<br>
                Nicolai<br>
                <br>
                ><br>
                ><br>
                > Tom<br>
                ><br>
                ><br>
                ><br>
                ><br>
                ><br>
                >
                ------------------------------------------------------------------------<br>
                > *From:* Nicolai Hähnle <a class="moz-txt-link-rfc2396E" href="mailto:nhaehnle@gmail.com"><nhaehnle@gmail.com></a><br>
                > *Sent:* Wednesday, June 29, 2016 10:20<br>
                > *To:* Tom St Denis; <a class="moz-txt-link-abbreviated" href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a><br>
                > *Cc:* StDenis, Tom<br>
                > *Subject:* Re: [PATCH 3/3] drm/amd/amdgpu: Add bank
                selection for MMIO<br>
                > debugfs (v3)<br>
                > Patch 1 & 3 are<br>
                ><br>
                > Reviewed-by: Nicolai Hähnle
                <a class="moz-txt-link-rfc2396E" href="mailto:nicolai.haehnle@amd.com"><nicolai.haehnle@amd.com></a><br>
                ><br>
                > Patch 2 looks mostly fine to me, too, but I'm not
                sure about the<br>
                > vmalloc/vfree vs. kmalloc/kfree.<br>
                ><br>
                > Cheers,<br>
                > Nicolai<br>
                ><br>
                > On 29.06.2016 15:56, Tom St Denis wrote:<br>
                >> (v2) Added INSTANCE selector<br>
                >> (v3) Changed order of bank selectors<br>
                >><br>
                >> Signed-off-by: Tom St Denis
                <a class="moz-txt-link-rfc2396E" href="mailto:tom.stdenis@amd.com"><tom.stdenis@amd.com></a><br>
                >> ---<br>
                >>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |
                35 +++++++++++++++++++++++++++---<br>
                >>   1 file changed, 32 insertions(+), 3
                deletions(-)<br>
                >><br>
                >> diff --git
                a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
                b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c<br>
                >> index 7f08ac02f9de..4a8f66c5ff43 100644<br>
                >> ---
                a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c<br>
                >> +++
                b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c<br>
                >> @@ -2244,20 +2244,43 @@ static ssize_t
                amdgpu_debugfs_regs_read(struct file *f, char __user
                *buf,<br>
                >>        struct amdgpu_device *adev =
                f->f_inode->i_private;<br>
                >>        ssize_t result = 0;<br>
                >>        int r;<br>
                >> +     bool use_bank;<br>
                >> +     unsigned instance_bank, sh_bank, se_bank;<br>
                >><br>
                >>        if (size & 0x3 || *pos & 0x3)<br>
                >>                return -EINVAL;<br>
                >><br>
                >> +     if (*pos & (1ULL << 62)) {<br>
                >> +             se_bank = (*pos >> 24)
                & 0x3FF;<br>
                >> +             sh_bank = (*pos >> 34)
                & 0x3FF;<br>
                >> +             instance_bank = (*pos >>
                44) & 0x3FF;<br>
                >> +             use_bank = 1;<br>
                >> +             *pos &= 0xFFFFFF;<br>
                >> +     } else {<br>
                >> +             use_bank = 0;<br>
                >> +     }<br>
                >> +<br>
                >> +     if (use_bank) {<br>
                >> +             if (sh_bank >=
                adev->gfx.config.max_sh_per_se ||<br>
                >> +                 se_bank >=
                adev->gfx.config.max_shader_engines)<br>
                >> +                     return -EINVAL;<br>
                >> +            
                mutex_lock(&adev->grbm_idx_mutex);<br>
                >> +             amdgpu_gfx_select_se_sh(adev,
                se_bank,<br>
                >> +                                     sh_bank,
                instance_bank);<br>
                >> +     }<br>
                >> +<br>
                >>        while (size) {<br>
                >>                uint32_t value;<br>
                >><br>
                >>                if (*pos >
                adev->rmmio_size)<br>
                >> -                     return result;<br>
                >> +                     goto end;<br>
                >><br>
                >>                value = RREG32(*pos >> 2);<br>
                >>                r = put_user(value, (uint32_t
                *)buf);<br>
                >> -             if (r)<br>
                >> -                     return r;<br>
                >> +             if (r) {<br>
                >> +                     result = r;<br>
                >> +                     goto end;<br>
                >> +             }<br>
                >><br>
                >>                result += 4;<br>
                >>                buf += 4;<br>
                >> @@ -2265,6 +2288,12 @@ static ssize_t
                amdgpu_debugfs_regs_read(struct file *f, char __user
                *buf,<br>
                >>                size -= 4;<br>
                >>        }<br>
                >><br>
                >> +end:<br>
                >> +     if (use_bank) {<br>
                >> +             amdgpu_gfx_select_se_sh(adev,
                0xffffffff, 0xffffffff, 0xffffffff);<br>
                >> +            
                mutex_unlock(&adev->grbm_idx_mutex);<br>
                >> +     }<br>
                >> +<br>
                >>        return result;<br>
                >>   }<br>
                >><br>
                >><br>
                _______________________________________________<br>
                amd-gfx mailing list<br>
                <a class="moz-txt-link-abbreviated" href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a><br>
                <a moz-do-not-send="true"
                  href="https://lists.freedesktop.org/mailman/listinfo/amd-gfx"
                  id="LPlnk479643">https://lists.freedesktop.org/mailman/listinfo/amd-gfx</a><br>
              </div>
            </span></font></div>
      </div>
      <br>
      <fieldset class="mimeAttachmentHeader"></fieldset>
      <br>
      <pre wrap="">_______________________________________________
amd-gfx mailing list
<a class="moz-txt-link-abbreviated" href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a>
<a class="moz-txt-link-freetext" href="https://lists.freedesktop.org/mailman/listinfo/amd-gfx">https://lists.freedesktop.org/mailman/listinfo/amd-gfx</a>
</pre>
    </blockquote>
    <br>
  </body>
</html>