<!DOCTYPE html><html><head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
  </head>
  <body>
    <p><br>
    </p>
    <div class="moz-cite-prefix">On 2024-03-04 19:20, Rehman, Ahmad
      wrote:<br>
    </div>
    <blockquote type="cite" cite="mid:CY5PR12MB60344BA3CFBEAA1BDD9D66648F222@CY5PR12MB6034.namprd12.prod.outlook.com">
      
      <style type="text/css" style="display:none;">P {margin-top:0;margin-bottom:0;}</style>
      <p style="font-family:Arial;font-size:10pt;color:#0000FF;margin:5pt;font-style:normal;font-weight:normal;text-decoration:none;" align="Left">
        [AMD Official Use Only - General]<br>
      </p>
      <br>
      <div>
        <div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">
          Hey,</div>
        <div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">
          <br>
        </div>
        <div class="elementToProof"><span style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);"><br>
          </span></div>
        <div class="elementToProof"><span style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">Due
            to mode-1 reset (pending_reset), the
            amdgpu_amdkfd_device_init will not be called and hence
            adev->kfd.init_complete will not be set. The function </span><span style="font-family: "Segoe UI Web (West European)", "Segoe UI", -apple-system, BlinkMacSystemFont, Roboto, "Helvetica Neue", sans-serif; font-size: 14.6667px; color: rgb(0, 0, 0); background-color: rgb(255, 255, 255);">amdgpu_amdkfd_drm_client_create</span><span style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);"> has
            condition:</span></div>
        <div class="elementToProof"><span style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">if
            (!adev->kfd.init_complete)</span></div>
        <div class="elementToProof"><span style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);"> 
                          return 0;</span></div>
        <div class="elementToProof"><span style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">So,
            in probe function, when we return from device_init the KFD
            is not initialized and
          </span><span style="font-family: "Segoe UI Web (West European)", "Segoe UI", -apple-system, BlinkMacSystemFont, Roboto, "Helvetica Neue", sans-serif; font-size: 14.6667px; color: rgb(0, 0, 0); background-color: rgb(255, 255, 255);">amdgpu_amdkfd_drm_client_create</span><span style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0); background-color: rgb(255, 255, 255);"> returns
            without doing anything.</span></div>
      </div>
    </blockquote>
    <p>I think your change could result in calling
      amdgpu_amdkfd_drm_client_create multiple times. IIRC, one purpose
      of moving the call to amdgpu_pci_probe was to ensure that it is
      only called once, because it only gets unregistered once when the
      driver is unloaded. Maybe it would be better to remove the if
      (!adev->kfd.init_complete) condition from
      amdgpu_amdkfd_drm_client_create. That way we would always create
      the client at probe and it would be ready when it's needed after
      the GPU reset.</p>
    <p><br>
    </p>
    <p>There is a chance that the client would get created unnecessarily
      if KFD init never succeeds. But that should be rare, and it's not
      a big resource waste.</p>
    <p><br>
    </p>
    <p>There were some comments on a previous code review, that creating
      the DRM client too early could cause problems. But I don't
      understand what that problem could be. As I understand it, the
      adev->kfd.client is just a place to put GEM handles for KFD BOs
      that we don't want to expose to user mode. I see no harm in
      creating this client too early or when it's not needed.</p>
    <p><br>
    </p>
    <p>Regards,<br>
        Felix<br>
    </p>
    <p><br>
    </p>
    <blockquote type="cite" cite="mid:CY5PR12MB60344BA3CFBEAA1BDD9D66648F222@CY5PR12MB6034.namprd12.prod.outlook.com">
      <div>
        <div class="elementToProof"><span style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);"><br>
          </span></div>
        <div class="elementToProof"><span style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">Thanks,</span></div>
        <div class="elementToProof"><span style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">Ahmad</span></div>
        <div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">
          <br>
        </div>
        <hr style="display: inline-block; width: 98%;">
        <div dir="ltr" id="divRplyFwdMsg"><span style="font-family: Calibri, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);"><b>From:</b> Kuehling,
            Felix <a class="moz-txt-link-rfc2396E" href="mailto:Felix.Kuehling@amd.com"><Felix.Kuehling@amd.com></a><br>
            <b>Sent:</b> Monday, March 4, 2024 6:39 PM<br>
            <b>To:</b> Rehman, Ahmad <a class="moz-txt-link-rfc2396E" href="mailto:Ahmad.Rehman@amd.com"><Ahmad.Rehman@amd.com></a>;
            <a class="moz-txt-link-abbreviated" href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a>
            <a class="moz-txt-link-rfc2396E" href="mailto:amd-gfx@lists.freedesktop.org"><amd-gfx@lists.freedesktop.org></a><br>
            <b>Cc:</b> Wan, Gavin <a class="moz-txt-link-rfc2396E" href="mailto:Gavin.Wan@amd.com"><Gavin.Wan@amd.com></a><br>
            <b>Subject:</b> Re: [PATCH] drm/amdgpu: Init zone device and
            drm client after mode-1 reset on reload</span>
          <div> </div>
        </div>
        <div><span style="font-size: 11pt;"><br>
            On 2024-03-04 17:05, Ahmad Rehman wrote:<br>
            > In passthrough environment, when amdgpu is reloaded
            after unload, mode-1<br>
            > is triggered after initializing the necessary IPs, That
            init does not<br>
            > include KFD, and KFD init waits until the reset is
            completed. KFD init<br>
            > is called in the reset handler, but in this case, the
            zone device and<br>
            > drm client is not initialized, causing app to create
            kernel panic.<br>
            ><br>
            > Signed-off-by: Ahmad Rehman
            <a class="moz-txt-link-rfc2396E" href="mailto:Ahmad.Rehman@amd.com"><Ahmad.Rehman@amd.com></a><br>
            > ---<br>
            >   drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 5 ++++-<br>
            >   1 file changed, 4 insertions(+), 1 deletion(-)<br>
            ><br>
            > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
            b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c<br>
            > index 15b188aaf681..80b9642f2bc4 100644<br>
            > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c<br>
            > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c<br>
            > @@ -2479,8 +2479,11 @@ static void
            amdgpu_drv_delayed_reset_work_handler(struct work_struct
            *work)<br>
            >        }<br>
            >        for (i = 0; i < mgpu_info.num_dgpu; i++) {<br>
            >                adev = mgpu_info.gpu_ins[i].adev;<br>
            > -             if (!adev->kfd.init_complete)<br>
            > +             if (!adev->kfd.init_complete) {<br>
            > +                     kgd2kfd_init_zone_device(adev);<br>
            >                        amdgpu_amdkfd_device_init(adev);<br>
            > +                    
            amdgpu_amdkfd_drm_client_create(adev);<br>
            I don't see what's preventing the DRM client initialization
            in the<br>
            reset-on-driver-load case. It only needs to be created once
            and that<br>
            happens in amdgpu_pci_probe. Am I missing anything?<br>
            <br>
            Regards,<br>
               Felix<br>
            <br>
            <br>
            > +             }<br>
            >                amdgpu_ttm_set_buffer_funcs_status(adev,
            true);<br>
            >        }<br>
            >   }</span></div>
      </div>
    </blockquote>
  </body>
</html>