drm/amd/powerplay: add smu message mutex

Submitted by Xiao, Jack on June 3, 2019, 7:02 a.m.

Details

Message ID 1559545315-25728-1-git-send-email-Jack.Xiao@amd.com
State New
Headers show
Series "drm/amd/powerplay: add smu message mutex" ( rev: 2 ) in AMD X.Org drivers

Not browsing as part of any series.

Commit Message

Xiao, Jack June 3, 2019, 7:02 a.m.
Add smu message mutex preventing against race condition issue.

Signed-off-by: Jack Xiao <Jack.Xiao@amd.com>

---
 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(-)

-- 
1.9.1

Patch hide | download patch | download mbox

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);
 
 	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;
 }
 

Comments

> -----Original Message-----

> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Xiao,

> Jack

> Sent: Monday, June 03, 2019 3:02 PM

> To: amd-gfx@lists.freedesktop.org; Deucher, Alexander

> <Alexander.Deucher@amd.com>; Zhang, Hawking

> <Hawking.Zhang@amd.com>

> Cc: Xiao, Jack <Jack.Xiao@amd.com>

> 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@amd.com>

> ---

>  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

> 

>  	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@lists.freedesktop.org

> https://lists.freedesktop.org/mailman/listinfo/amd-gfx