<div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div>Right. we have the max gen and width supported by the card and the platform in already in the driver thanks to amdgpu_device_get_pcie_info() so we just need to use the platform gen caps to limit the max pcie gen in dpm1. You don't want to use the current status because that might have been changed by sw at some point. The asic caps are defined by CAIL_ASIC_PCIE_LINK_SPEED_SUPPORT_GEN* while the platform caps are defined by CAIL_PCIE_LINK_SPEED_SUPPORT_GEN*. I think the original code was correct. What you may have been running into was a bug in pcie_get_speed_cap() that recently got fixed:</div><div><br></div><div>commit f1f90e254e46e0a14220e4090041f68256fbe297<br>Author: Mikulas Patocka <<a href="mailto:mpatocka@redhat.com">mpatocka@redhat.com</a>><br>Date: Mon Nov 26 10:37:13 2018 -0600<br><br> PCI: Fix incorrect value returned from pcie_get_speed_cap()<br> <br> The macros PCI_EXP_LNKCAP_SLS_*GB are values, not bit masks. We must mask<br> the register and compare it against them.<br> <br> This fixes errors like this:<br> <br> amdgpu: [powerplay] failed to send message 261 ret is 0<br> <br> when a PCIe-v3 card is plugged into a PCIe-v1 slot, because the slot is<br> being incorrectly reported as PCIe-v3 capable.<br> <br> 6cf57be0f78e, which appeared in v4.17, added pcie_get_speed_cap() with the<br> incorrect test of PCI_EXP_LNKCAP_SLS as a bitmask. 5d9a63304032, which<br> appeared in v4.19, changed amdgpu to use pcie_get_speed_cap(), so the<br> amdgpu bug reports below are regressions in v4.19.<br> <br> Fixes: 6cf57be0f78e ("PCI: Add pcie_get_speed_cap() to find max supported link speed")<br> Fixes: 5d9a63304032 ("drm/amdgpu: use pcie functions for link width and speed")<br> Link: <a href="https://bugs.freedesktop.org/show_bug.cgi?id=108704">https://bugs.freedesktop.org/show_bug.cgi?id=108704</a><br> Link: <a href="https://bugs.freedesktop.org/show_bug.cgi?id=108778">https://bugs.freedesktop.org/show_bug.cgi?id=108778</a><br> Signed-off-by: Mikulas Patocka <<a href="mailto:mpatocka@redhat.com">mpatocka@redhat.com</a>><br> [bhelgaas: update comment, remove use of PCI_EXP_LNKCAP_SLS_8_0GB and<br> PCI_EXP_LNKCAP_SLS_16_0GB since those should be covered by PCI_EXP_LNKCAP2,<br> remove test of PCI_EXP_LNKCAP for zero, since that register is required]<br> Signed-off-by: Bjorn Helgaas <<a href="mailto:bhelgaas@google.com">bhelgaas@google.com</a>><br> Acked-by: Alex Deucher <<a href="mailto:alexander.deucher@amd.com">alexander.deucher@amd.com</a>><br> Cc: <a href="mailto:stable@vger.kernel.org">stable@vger.kernel.org</a> # v4.17+<br><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Feb 1, 2019 at 5:43 PM Kasiviswanathan, Harish <<a href="mailto:Harish.Kasiviswanathan@amd.com">Harish.Kasiviswanathan@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"> <div dir="ltr"> <div id="gmail-m_3154020805083033663divtagdefaultwrapper" style="font-size:12pt;color:rgb(0,0,0);font-family:Calibri,Helvetica,sans-serif" dir="ltr"> <p style="margin-top:0px;margin-bottom:0px">Let me explain the problem. I also adding Colin & Sheldon from the SMU team.</p> <p style="margin-top:0px;margin-bottom:0px"><br> </p> <p style="margin-top:0px;margin-bottom:0px">Currently, <font size="2"><span style="font-size:11pt">PPTable entries (inside VBIOS) reflects ASIC capabilities and not the system capabilities.</span></font> So if we put Gen4 capable Vega20 in a system that supports only Gen3, the PPTable has Gen4 for DPM 1 (higher DPM state). Later on when SMU wants to switch to DPM1 it fails since Gen4 is not supported. SMU however, doesn't fall back to Gen3 but instead to Gen0. This is causing a performance issue.</p> <p style="margin-top:0px;margin-bottom:0px"><br> </p> <p style="margin-top:0px;margin-bottom:0px">As I understand, this override is required only for the higher DPM state.<br> </p> <div><br> </div> <div>Best Regards,</div> <div>Harish</div> <div><br> </div> <div style="color:rgb(0,0,0)"> <hr style="display:inline-block;width:98%"> <div id="gmail-m_3154020805083033663divRplyFwdMsg" dir="ltr"><font style="font-size:11pt" face="Calibri, sans-serif" color="#000000"><b>From:</b> Alex Deucher <<a href="mailto:alexdeucher@gmail.com" target="_blank">alexdeucher@gmail.com</a>><br> <b>Sent:</b> Friday, February 1, 2019 5:34 PM<br> <b>To:</b> Kasiviswanathan, Harish<br> <b>Cc:</b> <a href="mailto:amd-gfx@lists.freedesktop.org" target="_blank">amd-gfx@lists.freedesktop.org</a><br> <b>Subject:</b> Re: [PATCH] drm/amd/powerplay: add override pcie parameters for Vega20 (v2)</font> <div> </div> </div> <div> <div dir="ltr"> <div>Do we actually need the current status? We have the caps of the platform and the device. For the lowest dpm state, we generally want the lowest common gen and then the highest common gen for the highest dpm state. If we use the current status, we will mostly likely end up with the highest gen in the lowest dpm state because generally the highest gen is negotiated at power up.</div> <div><br> </div> <div>Alex<br> </div> <br> <div class="gmail-m_3154020805083033663x_gmail_quote"> <div dir="ltr" class="gmail-m_3154020805083033663x_gmail_attr">On Fri, Feb 1, 2019 at 5:28 PM Kasiviswanathan, Harish <<a href="mailto:Harish.Kasiviswanathan@amd.com" id="gmail-m_3154020805083033663LPlnk390754" class="gmail-m_3154020805083033663OWAAutoLink" target="_blank">Harish.Kasiviswanathan@amd.com</a>> wrote:<br> </div> <blockquote class="gmail-m_3154020805083033663x_gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> <div dir="ltr"> <div id="gmail-m_3154020805083033663x_gmail-m_329555177500593286divtagdefaultwrapper" dir="ltr" style="font-size:12pt;color:rgb(0,0,0);font-family:Calibri,Helvetica,sans-serif"> <p style="margin-top:0px;margin-bottom:0px">Thanks Alex. I saw that function. It gets caps. The function requires link status (<font size="2"><span style="font-size:11pt">PCI_EXP_LNKSTA</span></font>), the value negotiated. <br> </p> <p style="margin-top:0px;margin-bottom:0px"><br> </p> <p style="margin-top:0px;margin-bottom:0px"><font size="2"><span style="font-size:11pt">> I'd rather not access pci config space from the powerplay code because it doesn't work under virtualization.</span></font><br> </p> <div>[HK] I will then modify <font size="2"><span style="font-size:11pt">amdgpu_device_get_pcie_info()</span></font> to read "PCI_EXP_LINKSTA" and store that information for future use.</div> <div><br> </div> <div>Best Regards,</div> <div>Harish</div> <div><br> </div> <div><br> </div> <div style="color:rgb(0,0,0)"> <hr style="display:inline-block;width:98%"> <div id="gmail-m_3154020805083033663x_gmail-m_329555177500593286divRplyFwdMsg" dir="ltr"><font style="font-size:11pt" face="Calibri, sans-serif" color="#000000"><b>From:</b> Alex Deucher <<a href="mailto:alexdeucher@gmail.com" id="gmail-m_3154020805083033663LPlnk97574" class="gmail-m_3154020805083033663OWAAutoLink" target="_blank">alexdeucher@gmail.com</a>><br> <b>Sent:</b> Friday, February 1, 2019 5:09 PM<br> <b>To:</b> Kasiviswanathan, Harish<br> <b>Cc:</b> <a href="mailto:amd-gfx@lists.freedesktop.org" id="gmail-m_3154020805083033663LPlnk857262" class="gmail-m_3154020805083033663OWAAutoLink" target="_blank"> amd-gfx@lists.freedesktop.org</a><br> <b>Subject:</b> Re: [PATCH] drm/amd/powerplay: add override pcie parameters for Vega20 (v2)</font> <div> </div> </div> <div class="gmail-m_3154020805083033663x_gmail-m_329555177500593286BodyFragment"><font size="2"><span style="font-size:11pt"> <div class="gmail-m_3154020805083033663x_gmail-m_329555177500593286PlainText">On Fri, Feb 1, 2019 at 4:17 PM Kasiviswanathan, Harish<br> <<a href="mailto:Harish.Kasiviswanathan@amd.com" id="gmail-m_3154020805083033663LPlnk596173" class="gmail-m_3154020805083033663OWAAutoLink" target="_blank">Harish.Kasiviswanathan@amd.com</a>> wrote:<br> ><br> > v2: Use PCIe link status instead of link capability<br> > Send override message after SMU enable features<br> ><br> > Change-Id: Iea2a1ac595cf63a9528ff1e9a6955d14d8c3a6d5<br> > Signed-off-by: Harish Kasiviswanathan <<a href="mailto:Harish.Kasiviswanathan@amd.com" id="gmail-m_3154020805083033663LPlnk870253" class="gmail-m_3154020805083033663OWAAutoLink" target="_blank">Harish.Kasiviswanathan@amd.com</a>><br> > ---<br> > drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c | 93 +++++++++++++++-------<br> > 1 file changed, 64 insertions(+), 29 deletions(-)<br> ><br> > diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c b/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c<br> > index da022ca..d166f8c 100644<br> > --- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c<br> > +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c<br> > @@ -771,40 +771,75 @@ static int vega20_init_smc_table(struct pp_hwmgr *hwmgr)<br> > return 0;<br> > }<br> ><br> > +/*<br> > + * Override PCIe link speed and link width for DPM Level 1. PPTable entries<br> > + * reflect the ASIC capabilities and not the system capabilities. For e.g.<br> > + * Vega20 board in a PCI Gen3 system. In this case, when SMU's tries to switch<br> > + * to DPM1, it fails as system doesn't support Gen4.<br> > + */<br> > static int vega20_override_pcie_parameters(struct pp_hwmgr *hwmgr)<br> > {<br> > struct amdgpu_device *adev = (struct amdgpu_device *)(hwmgr->adev);<br> > - uint32_t pcie_speed = 0, pcie_width = 0, pcie_arg;<br> > + uint32_t pcie_gen = 0, pcie_width = 0, smu_pcie_arg;<br> > int ret;<br> > + uint16_t lnkstat;<br> > +<br> > + pcie_capability_read_word(adev->pdev, PCI_EXP_LNKSTA, &lnkstat);<br> ><br> > - if (adev->pm.pcie_gen_mask & CAIL_PCIE_LINK_SPEED_SUPPORT_GEN4)<br> > - pcie_speed = 16;<br> > - else if (adev->pm.pcie_gen_mask & CAIL_PCIE_LINK_SPEED_SUPPORT_GEN3)<br> > - pcie_speed = 8;<br> > - else if (adev->pm.pcie_gen_mask & CAIL_PCIE_LINK_SPEED_SUPPORT_GEN2)<br> > - pcie_speed = 5;<br> > - else if (adev->pm.pcie_gen_mask & CAIL_PCIE_LINK_SPEED_SUPPORT_GEN1)<br> > - pcie_speed = 2;<br> > -<br> > - if (adev->pm.pcie_mlw_mask & CAIL_PCIE_LINK_WIDTH_SUPPORT_X32)<br> > - pcie_width = 32;<br> > - else if (adev->pm.pcie_mlw_mask & CAIL_PCIE_LINK_WIDTH_SUPPORT_X16)<br> > - pcie_width = 16;<br> > - else if (adev->pm.pcie_mlw_mask & CAIL_PCIE_LINK_WIDTH_SUPPORT_X12)<br> > - pcie_width = 12;<br> > - else if (adev->pm.pcie_mlw_mask & CAIL_PCIE_LINK_WIDTH_SUPPORT_X8)<br> > - pcie_width = 8;<br> > - else if (adev->pm.pcie_mlw_mask & CAIL_PCIE_LINK_WIDTH_SUPPORT_X4)<br> > + switch (lnkstat & PCI_EXP_LNKSTA_CLS) {<br> > + case PCI_EXP_LNKSTA_CLS_2_5GB:<br> > + pcie_gen = 0;<br> > + break;<br> > + case PCI_EXP_LNKSTA_CLS_5_0GB:<br> > + pcie_gen = 1;<br> > + break;<br> > + case PCI_EXP_LNKSTA_CLS_8_0GB:<br> > + pcie_gen = 2;<br> > + break;<br> > + case PCI_EXP_LNKSTA_CLS_16_0GB:<br> > + pcie_gen = 3;<br> > + break;<br> > + default:<br> > + pr_warn("Unknown PCI link speed %x\n",<br> > + lnkstat & PCI_EXP_LNKSTA_CLS);<br> > + }<br> > +<br> > +<br> > + switch ((lnkstat & PCI_EXP_LNKSTA_NLW) >> PCI_EXP_LNKSTA_NLW_SHIFT) {<br> > + case 32:<br> > + pcie_width = 7;<br> > + break;<br> > + case 16:<br> > + pcie_width = 6;<br> > + break;<br> > + case 12:<br> > + pcie_width = 5;<br> > + break;<br> > + case 8:<br> > pcie_width = 4;<br> > - else if (adev->pm.pcie_mlw_mask & CAIL_PCIE_LINK_WIDTH_SUPPORT_X2)<br> > + break;<br> > + case 4:<br> > + pcie_width = 3;<br> > + break;<br> > + case 2:<br> > pcie_width = 2;<br> > - else if (adev->pm.pcie_mlw_mask & CAIL_PCIE_LINK_WIDTH_SUPPORT_X1)<br> > + break;<br> > + case 1:<br> > pcie_width = 1;<br> > + break;<br> > + default:<br> > + pr_warn("Unknown PCI link width %x\n",<br> > + lnkstat & PCI_EXP_LNKSTA_CLS);<br> > + }<br> ><br> <br> We already get the link caps for both the device and the platform in<br> amdgpu_device_get_pcie_info(). Is that info not adequate? I'd rather<br> not access pci config space from the powerplay code because it doesn't<br> work under virtualization.<br> <br> Alex<br> <br> > - pcie_arg = pcie_width | (pcie_speed << 8);<br> > -<br> > + /* Bit 31:16: LCLK DPM level. 0 is DPM0, and 1 is DPM1<br> > + * Bit 15:8: PCIE GEN, 0 to 3 corresponds to GEN1 to GEN4<br> > + * Bit 7:0: PCIE lane width, 1 to 7 corresponds is x1 to x32<br> > + */<br> > + smu_pcie_arg = (1 << 16) | (pcie_gen << 8) | pcie_width;<br> > ret = smum_send_msg_to_smc_with_parameter(hwmgr,<br> > - PPSMC_MSG_OverridePcieParameters, pcie_arg);<br> > + PPSMC_MSG_OverridePcieParameters, smu_pcie_arg);<br> > +<br> > PP_ASSERT_WITH_CODE(!ret,<br> > "[OverridePcieParameters] Attempt to override pcie params failed!",<br> > return ret);<br> > @@ -1611,11 +1646,6 @@ static int vega20_enable_dpm_tasks(struct pp_hwmgr *hwmgr)<br> > "[EnableDPMTasks] Failed to initialize SMC table!",<br> > return result);<br> ><br> > - result = vega20_override_pcie_parameters(hwmgr);<br> > - PP_ASSERT_WITH_CODE(!result,<br> > - "[EnableDPMTasks] Failed to override pcie parameters!",<br> > - return result);<br> > -<br> > result = vega20_run_btc(hwmgr);<br> > PP_ASSERT_WITH_CODE(!result,<br> > "[EnableDPMTasks] Failed to run btc!",<br> > @@ -1631,6 +1661,11 @@ static int vega20_enable_dpm_tasks(struct pp_hwmgr *hwmgr)<br> > "[EnableDPMTasks] Failed to enable all smu features!",<br> > return result);<br> ><br> > + result = vega20_override_pcie_parameters(hwmgr);<br> > + PP_ASSERT_WITH_CODE(!result,<br> > + "[EnableDPMTasks] Failed to override pcie parameters!",<br> > + return result);<br> > +<br> > result = vega20_notify_smc_display_change(hwmgr);<br> > PP_ASSERT_WITH_CODE(!result,<br> > "[EnableDPMTasks] Failed to notify smc display change!",<br> > --<br> > 2.7.4<br> ><br> > _______________________________________________<br> > amd-gfx mailing list<br> > <a href="mailto:amd-gfx@lists.freedesktop.org" id="gmail-m_3154020805083033663LPlnk901207" class="gmail-m_3154020805083033663OWAAutoLink" target="_blank"> amd-gfx@lists.freedesktop.org</a><br> > <a href="https://lists.freedesktop.org/mailman/listinfo/amd-gfx" id="gmail-m_3154020805083033663LPlnk961930" class="gmail-m_3154020805083033663x_gmail-m_329555177500593286OWAAutoLink gmail-m_3154020805083033663OWAAutoLink" target="_blank"> https://lists.freedesktop.org/mailman/listinfo/amd-gfx</a> <div id="gmail-m_3154020805083033663x_gmail-m_329555177500593286LPBorder_GT_15490596973030.8109052536501797" style="margin-bottom:20px;overflow:auto;width:100%;text-indent:0px"> <table id="gmail-m_3154020805083033663x_gmail-m_329555177500593286LPContainer_15490596973000.9760548684631369" style="width:90%;background-color:rgb(255,255,255);overflow:auto;padding-top:20px;padding-bottom:20px;margin-top:20px;border-top:1px dotted rgb(200,200,200);border-bottom:1px dotted rgb(200,200,200)" cellspacing="0"> <tbody> <tr style="border-spacing:0px" valign="top"> <td id="gmail-m_3154020805083033663x_gmail-m_329555177500593286TextCell_15490596973010.8841707630863909" colspan="2" style="vertical-align:top;padding:0px;display:table-cell"> <div id="gmail-m_3154020805083033663x_gmail-m_329555177500593286LPRemovePreviewContainer_15490596973010.5097834543649797"> </div> <div id="gmail-m_3154020805083033663x_gmail-m_329555177500593286LPTitle_15490596973010.6766157293160253" style="color:rgb(0,120,215);font-weight:400;font-size:21px;font-family:"wf_segoe-ui_light","Segoe UI Light","Segoe WP Light","Segoe UI","Segoe WP",Tahoma,Arial,sans-serif;line-height:21px"> <a id="gmail-m_3154020805083033663LPlnk508294" href="https://lists.freedesktop.org/mailman/listinfo/amd-gfx" style="text-decoration:none" class="gmail-m_3154020805083033663OWAAutoLink" target="_blank">amd-gfx Info Page - freedesktop.org</a></div> <div id="gmail-m_3154020805083033663x_gmail-m_329555177500593286LPMetadata_15490596973020.9257698477379372" style="margin:10px 0px 16px;color:rgb(102,102,102);font-weight:400;font-family:"wf_segoe-ui_normal","Segoe UI","Segoe WP",Tahoma,Arial,sans-serif;font-size:14px;line-height:14px"> <a href="http://lists.freedesktop.org" id="gmail-m_3154020805083033663LPlnk237919" class="gmail-m_3154020805083033663OWAAutoLink" target="_blank">lists.freedesktop.org</a></div> <div id="gmail-m_3154020805083033663x_gmail-m_329555177500593286LPDescription_15490596973020.8919956792568361" style="display:block;color:rgb(102,102,102);font-weight:400;font-family:"wf_segoe-ui_normal","Segoe UI","Segoe WP",Tahoma,Arial,sans-serif;font-size:14px;line-height:20px;max-height:100px;overflow:hidden"> To see the collection of prior postings to the list, visit the amd-gfx Archives.. Using amd-gfx: To post a message to all the list members, send email to <a href="mailto:amd-gfx@lists.freedesktop.org" id="gmail-m_3154020805083033663LPlnk852842" class="gmail-m_3154020805083033663OWAAutoLink" target="_blank"> amd-gfx@lists.freedesktop.org</a>. You can subscribe to the list, or change your existing subscription, in the sections below.</div> </td> </tr> </tbody> </table> </div> <br> </div> </span></font></div> </div> </div> </div> </blockquote> </div> </div> </div> </div> </div> </div> </blockquote></div></div></div></div></div></div></div></div>