|
From: Huang, Ray
Sent: Tuesday, June 4, 2019 1:37 PM To: Xiao, Jack; amd-gfx@xxxxxxxxxxxxxxxxxxxxx; Deucher, Alexander; Zhang, Hawking Cc: Xiao, Jack; Wang, Kevin(Yang); Quan, Evan; Gui, Jack; Gao, Likun Subject: RE: [PATCH] drm/amd/powerplay: add smu message mutex > -----Original Message-----
> From: amd-gfx <amd-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of Xiao, > Jack > Sent: Monday, June 03, 2019 3:02 PM > To: amd-gfx@xxxxxxxxxxxxxxxxxxxxx; Deucher, Alexander > <Alexander.Deucher@xxxxxxx>; Zhang, Hawking > <Hawking.Zhang@xxxxxxx> > Cc: Xiao, Jack <Jack.Xiao@xxxxxxx> > Subject: [PATCH] drm/amd/powerplay: add smu message mutex > > Add smu message mutex preventing against race condition issue. > > Signed-off-by: Jack Xiao <Jack.Xiao@xxxxxxx> > --- > drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 1 + > drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h | 1 + > drivers/gpu/drm/amd/powerplay/smu_v11_0.c | 7 ++++++- > 3 files changed, 8 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c > b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c > index 3026c7e..db2bbec 100644 > --- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c > +++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c > @@ -350,6 +350,7 @@ static int smu_early_init(void *handle) > smu->adev = adev; > smu->pm_enabled = !!amdgpu_dpm; > mutex_init(&smu->mutex); > + mutex_init(&smu->msg_mutex); 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. + Linux power folks, Kevin, could you please use the smu->mutex to protect below callbacks which will be called from gpu_info ioctl. amdgpu_dpm_get_sclk amdgpu_dpm_get_mclk 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. Thanks, Ray [Kevin]
Hi Ray,
i think we should add msg_mutex lock to protect smu message function and regsiter change.
more elaborate locks should be used to protect this critical resource and reduce the probability of mutual exclusion.
Hi Jack,
The patch is
Reviewed-by: Kevin Wang <kevin1.wang@xxxxxxx>
Thanks,
Kevin
>
> return smu_set_funcs(adev); > } > diff --git a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h > b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h > index 3eb1de9..735233e 100644 > --- a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h > +++ b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h > @@ -374,6 +374,7 @@ struct smu_context > const struct smu_funcs *funcs; > const struct pptable_funcs *ppt_funcs; > struct mutex mutex; > + struct mutex msg_mutex; > uint64_t pool_size; > > struct smu_table_context smu_table; > diff --git a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c > b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c > index d2eeb62..de737a0 100644 > --- a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c > +++ b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c > @@ -104,6 +104,8 @@ static int smu_v11_0_send_msg(struct smu_context > *smu, uint16_t msg) > if (index < 0) > return index; > > + mutex_lock(&smu->msg_mutex); > + > smu_v11_0_wait_for_response(smu); > > WREG32_SOC15(MP1, 0, mmMP1_SMN_C2PMSG_90, 0); @@ - > 111,11 +113,11 @@ static int smu_v11_0_send_msg(struct smu_context > *smu, uint16_t msg) > smu_v11_0_send_msg_without_waiting(smu, (uint16_t)index); > > ret = smu_v11_0_wait_for_response(smu); > - > if (ret) > pr_err("Failed to send message 0x%x, response 0x%x\n", > index, > ret); > > + mutex_unlock(&smu->msg_mutex); > return ret; > > } > @@ -132,6 +134,8 @@ static int smu_v11_0_send_msg(struct smu_context > *smu, uint16_t msg) > if (index < 0) > return index; > > + mutex_lock(&smu->msg_mutex); > + > ret = smu_v11_0_wait_for_response(smu); > if (ret) > pr_err("Failed to send message 0x%x, response 0x%x, param > 0x%x\n", @@ -148,6 +152,7 @@ static int smu_v11_0_send_msg(struct > smu_context *smu, uint16_t msg) > pr_err("Failed to send message 0x%x, response 0x%x param > 0x%x\n", > index, ret, param); > > + mutex_unlock(&smu->msg_mutex); > return ret; > } > > -- > 1.9.1 > > _______________________________________________ > amd-gfx mailing list > amd-gfx@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/amd-gfx |
_______________________________________________ amd-gfx mailing list amd-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/amd-gfx
