<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1"> <style type="text/css" style="display:none;"></style> </head> <body dir="ltr"> <div id="divtagdefaultwrapper" dir="ltr" style="font-size:12pt; color:rgb(0,0,0); font-family:Calibri,Helvetica,sans-serif,EmojiFont,"Apple Color Emoji","Segoe UI Emoji",NotoColorEmoji,"Segoe UI Symbol","Android Emoji",EmojiSymbols"> <p style="margin-top:0; margin-bottom:0"><br> </p> <div style="color:rgb(0,0,0)"> <hr tabindex="-1" style="display:inline-block; width:98%"> <div id="divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" color="#000000" style="font-size:11pt"><b>From:</b> Huang, Ray<br> <b>Sent:</b> Tuesday, June 4, 2019 1:37 PM<br> <b>To:</b> Xiao, Jack; amd-gfx@lists.freedesktop.org; Deucher, Alexander; Zhang, Hawking<br> <b>Cc:</b> Xiao, Jack; Wang, Kevin(Yang); Quan, Evan; Gui, Jack; Gao, Likun<br> <b>Subject:</b> RE: [PATCH] drm/amd/powerplay: add smu message mutex</font> <div> </div> </div> <div class="BodyFragment"><font size="2"><span style="font-size:11pt"> <div class="PlainText">> -----Original Message-----<br> > From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Xiao,<br> > Jack<br> > Sent: Monday, June 03, 2019 3:02 PM<br> > To: amd-gfx@lists.freedesktop.org; Deucher, Alexander<br> > <Alexander.Deucher@amd.com>; Zhang, Hawking<br> > <Hawking.Zhang@amd.com><br> > Cc: Xiao, Jack <Jack.Xiao@amd.com><br> > Subject: [PATCH] drm/amd/powerplay: add smu message mutex<br> > <br> > Add smu message mutex preventing against race condition issue.<br> > <br> > Signed-off-by: Jack Xiao <Jack.Xiao@amd.com><br> > ---<br> > drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 1 +<br> > drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h | 1 +<br> > drivers/gpu/drm/amd/powerplay/smu_v11_0.c | 7 ++++++-<br> > 3 files changed, 8 insertions(+), 1 deletion(-)<br> > <br> > diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c<br> > b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c<br> > index 3026c7e..db2bbec 100644<br> > --- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c<br> > +++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c<br> > @@ -350,6 +350,7 @@ static int smu_early_init(void *handle)<br> > smu->adev = adev;<br> > smu->pm_enabled = !!amdgpu_dpm;<br> > mutex_init(&smu->mutex);<br> > + mutex_init(&smu->msg_mutex);<br> <br> As talked with you, we need use smu->mutex to protect the context in the thread instead of introducing the specific mutex of messages. Because msg_mutex cannot protect the case of multi-message pairing. And yes, this is the key issue of swSMU so far.<br> <br> + Linux power folks, <br> Kevin, could you please use the smu->mutex to protect below callbacks which will be called from gpu_info ioctl. <br> <br> amdgpu_dpm_get_sclk<br> amdgpu_dpm_get_mclk<br> <br> And we need smu->mutex to protect the smu_dpm_set_uvd_enable/ smu_dpm_set_vce_enable as well, because they will be called during VCN command submissions. We should look over all ioctl/sysfs interface in the driver, they all need the mutex.<br> <br> Thanks,<br> Ray</div> <div class="PlainText"><br> </div> <div class="PlainText">[Kevin]</div> <div class="PlainText"><br> </div> <div class="PlainText">Hi Ray,</div> <div class="PlainText"><br> </div> <div class="PlainText">i think we should add msg_mutex lock to protect smu message function and regsiter change.</div> <div class="PlainText"></div> <div>more elaborate locks should be used to protect this critical resource and reduce the probability of mutual exclusion.</div> <div><br> </div> <div>Hi Jack,</div> <div><br> </div> <div>The patch is</div> <div>Reviewed-by: Kevin Wang <kevin1.wang@amd.com></div> <div><br> </div> <div>Thanks,</div> <div>Kevin</div> <div class="PlainText"><br> </div> <div class="PlainText">> <br> > return smu_set_funcs(adev);<br> > }<br> > diff --git a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h<br> > b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h<br> > index 3eb1de9..735233e 100644<br> > --- a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h<br> > +++ b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h<br> > @@ -374,6 +374,7 @@ struct smu_context<br> > const struct smu_funcs *funcs;<br> > const struct pptable_funcs *ppt_funcs;<br> > struct mutex mutex;<br> > + struct mutex msg_mutex;<br> > uint64_t pool_size;<br> > <br> > struct smu_table_context smu_table;<br> > diff --git a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c<br> > b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c<br> > index d2eeb62..de737a0 100644<br> > --- a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c<br> > +++ b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c<br> > @@ -104,6 +104,8 @@ static int smu_v11_0_send_msg(struct smu_context<br> > *smu, uint16_t msg)<br> > if (index < 0)<br> > return index;<br> > <br> > + mutex_lock(&smu->msg_mutex);<br> > +<br> > smu_v11_0_wait_for_response(smu);<br> > <br> > WREG32_SOC15(MP1, 0, mmMP1_SMN_C2PMSG_90, 0); @@ -<br> > 111,11 +113,11 @@ static int smu_v11_0_send_msg(struct smu_context<br> > *smu, uint16_t msg)<br> > smu_v11_0_send_msg_without_waiting(smu, (uint16_t)index);<br> > <br> > ret = smu_v11_0_wait_for_response(smu);<br> > -<br> > if (ret)<br> > pr_err("Failed to send message 0x%x, response 0x%x\n",<br> > index,<br> > ret);<br> > <br> > + mutex_unlock(&smu->msg_mutex);<br> > return ret;<br> > <br> > }<br> > @@ -132,6 +134,8 @@ static int smu_v11_0_send_msg(struct smu_context<br> > *smu, uint16_t msg)<br> > if (index < 0)<br> > return index;<br> > <br> > + mutex_lock(&smu->msg_mutex);<br> > +<br> > ret = smu_v11_0_wait_for_response(smu);<br> > if (ret)<br> > pr_err("Failed to send message 0x%x, response 0x%x, param<br> > 0x%x\n", @@ -148,6 +152,7 @@ static int smu_v11_0_send_msg(struct<br> > smu_context *smu, uint16_t msg)<br> > pr_err("Failed to send message 0x%x, response 0x%x param<br> > 0x%x\n",<br> > index, ret, param);<br> > <br> > + mutex_unlock(&smu->msg_mutex);<br> > return ret;<br> > }<br> > <br> > --<br> > 1.9.1<br> > <br> > _______________________________________________<br> > amd-gfx mailing list<br> > amd-gfx@lists.freedesktop.org<br> > <a href="https://lists.freedesktop.org/mailman/listinfo/amd-gfx" id="LPlnk554601" class="OWAAutoLink" previewremoved="true"> https://lists.freedesktop.org/mailman/listinfo/amd-gfx</a><br> </div> </span></font></div> </div> </div> </body> </html>