[v4,3/4] drm/amd/powerplay: Add interface to lock SMU HW I2C.

Submitted by Andrey Grodzovsky on Aug. 21, 2019, 8:01 p.m.

Details

Message ID 1566417719-1528-4-git-send-email-andrey.grodzovsky@amd.com
State New
Headers show
Series "Add RAS EEPROM table and I2C driver implementation." ( rev: 4 3 2 1 ) in AMD X.Org drivers

Not browsing as part of any series.

Commit Message

Andrey Grodzovsky Aug. 21, 2019, 8:01 p.m.
v2:
PPSMC_MSG_RequestI2CBus seems not to work and so to avoid conflict
over I2C bus and engine disable thermal control access to
force SMU stop using the I2C bus until the issue is reslolved.

Expose and call vega20_is_smc_ram_running to skip locking when SMU
FW is not yet loaded.

v3:
Remove the prevoius hack as the SMU found the bug.

Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
---
 drivers/gpu/drm/amd/include/kgd_pp_interface.h       |  1 +
 drivers/gpu/drm/amd/powerplay/amd_powerplay.c        | 16 ++++++++++++++++
 drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c   | 19 +++++++++++++++++++
 drivers/gpu/drm/amd/powerplay/inc/hwmgr.h            |  1 +
 drivers/gpu/drm/amd/powerplay/smumgr/vega20_smumgr.c |  2 +-
 drivers/gpu/drm/amd/powerplay/smumgr/vega20_smumgr.h |  2 ++
 6 files changed, 40 insertions(+), 1 deletion(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/amd/include/kgd_pp_interface.h b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
index 0de4e37..021ce466 100644
--- a/drivers/gpu/drm/amd/include/kgd_pp_interface.h
+++ b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
@@ -275,6 +275,7 @@  struct amd_pm_funcs {
 	int (*set_power_profile_mode)(void *handle, long *input, uint32_t size);
 	int (*odn_edit_dpm_table)(void *handle, uint32_t type, long *input, uint32_t size);
 	int (*set_mp1_state)(void *handle, enum pp_mp1_state mp1_state);
+	int (*smu_i2c_bus_access)(void *handle, bool aquire);
 /* export to DC */
 	u32 (*get_sclk)(void *handle, bool low);
 	u32 (*get_mclk)(void *handle, bool low);
diff --git a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
index 7ef2027..bef5cd1 100644
--- a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
+++ b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
@@ -1528,6 +1528,21 @@  static int pp_asic_reset_mode_2(void *handle)
 	return ret;
 }
 
+static int pp_smu_i2c_bus_access(void *handle, bool aquire)
+{
+	struct pp_hwmgr *hwmgr = handle;
+
+	if (!hwmgr || !hwmgr->pm_en)
+		return -EINVAL;
+
+	if (hwmgr->hwmgr_func->smu_i2c_bus_access == NULL) {
+		pr_info_ratelimited("%s was not implemented.\n", __func__);
+		return -EINVAL;
+	}
+
+	return hwmgr->hwmgr_func->smu_i2c_bus_access(hwmgr, aquire);
+}
+
 static const struct amd_pm_funcs pp_dpm_funcs = {
 	.load_firmware = pp_dpm_load_fw,
 	.wait_for_fw_loading_complete = pp_dpm_fw_loading_complete,
@@ -1585,4 +1600,5 @@  static const struct amd_pm_funcs pp_dpm_funcs = {
 	.get_ppfeature_status = pp_get_ppfeature_status,
 	.set_ppfeature_status = pp_set_ppfeature_status,
 	.asic_reset_mode_2 = pp_asic_reset_mode_2,
+	.smu_i2c_bus_access = pp_smu_i2c_bus_access,
 };
diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c b/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
index 0516c29..f26b14f 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
@@ -4085,6 +4085,24 @@  static int vega20_get_thermal_temperature_range(struct pp_hwmgr *hwmgr,
 	return 0;
 }
 
+static int vega20_smu_i2c_bus_access(struct pp_hwmgr *hwmgr, bool aquire)
+{
+	int res;
+
+	/* I2C bus access can happen very early, when SMU not loaded yet */
+	if (!vega20_is_smc_ram_running(hwmgr))
+		return 0;
+
+	res = smum_send_msg_to_smc_with_parameter(hwmgr,
+						  (aquire ?
+						  PPSMC_MSG_RequestI2CBus :
+						  PPSMC_MSG_ReleaseI2CBus),
+						  0);
+
+	PP_ASSERT_WITH_CODE(!res, "[SmuI2CAccessBus] Failed to access bus!", return res);
+	return res;
+}
+
 static const struct pp_hwmgr_func vega20_hwmgr_funcs = {
 	/* init/fini related */
 	.backend_init = vega20_hwmgr_backend_init,
@@ -4152,6 +4170,7 @@  static const struct pp_hwmgr_func vega20_hwmgr_funcs = {
 	.get_asic_baco_state = vega20_baco_get_state,
 	.set_asic_baco_state = vega20_baco_set_state,
 	.set_mp1_state = vega20_set_mp1_state,
+	.smu_i2c_bus_access = vega20_smu_i2c_bus_access,
 };
 
 int vega20_hwmgr_init(struct pp_hwmgr *hwmgr)
diff --git a/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h b/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
index abeff15..7bf9a14 100644
--- a/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
+++ b/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
@@ -354,6 +354,7 @@  struct pp_hwmgr_func {
 	int (*set_ppfeature_status)(struct pp_hwmgr *hwmgr, uint64_t ppfeature_masks);
 	int (*set_mp1_state)(struct pp_hwmgr *hwmgr, enum pp_mp1_state mp1_state);
 	int (*asic_reset)(struct pp_hwmgr *hwmgr, enum SMU_ASIC_RESET_MODE mode);
+	int (*smu_i2c_bus_access)(struct pp_hwmgr *hwmgr, bool aquire);
 };
 
 struct pp_table_func {
diff --git a/drivers/gpu/drm/amd/powerplay/smumgr/vega20_smumgr.c b/drivers/gpu/drm/amd/powerplay/smumgr/vega20_smumgr.c
index 3e97b83..b9089c6 100644
--- a/drivers/gpu/drm/amd/powerplay/smumgr/vega20_smumgr.c
+++ b/drivers/gpu/drm/amd/powerplay/smumgr/vega20_smumgr.c
@@ -44,7 +44,7 @@ 
 #define smnMP0_FW_INTF			0x30101c0
 #define smnMP1_PUB_CTRL			0x3010b14
 
-static bool vega20_is_smc_ram_running(struct pp_hwmgr *hwmgr)
+bool vega20_is_smc_ram_running(struct pp_hwmgr *hwmgr)
 {
 	struct amdgpu_device *adev = hwmgr->adev;
 	uint32_t mp1_fw_flags;
diff --git a/drivers/gpu/drm/amd/powerplay/smumgr/vega20_smumgr.h b/drivers/gpu/drm/amd/powerplay/smumgr/vega20_smumgr.h
index ec953ab..62ebbfd 100644
--- a/drivers/gpu/drm/amd/powerplay/smumgr/vega20_smumgr.h
+++ b/drivers/gpu/drm/amd/powerplay/smumgr/vega20_smumgr.h
@@ -57,5 +57,7 @@  int vega20_get_activity_monitor_coeff(struct pp_hwmgr *hwmgr,
 		uint8_t *table, uint16_t workload_type);
 int vega20_set_pptable_driver_address(struct pp_hwmgr *hwmgr);
 
+bool vega20_is_smc_ram_running(struct pp_hwmgr *hwmgr);
+
 #endif
 

Comments

On Wed, Aug 21, 2019 at 4:02 PM Andrey Grodzovsky
<andrey.grodzovsky@amd.com> wrote:
>
> v2:
> PPSMC_MSG_RequestI2CBus seems not to work and so to avoid conflict
> over I2C bus and engine disable thermal control access to
> force SMU stop using the I2C bus until the issue is reslolved.
>
> Expose and call vega20_is_smc_ram_running to skip locking when SMU
> FW is not yet loaded.
>
> v3:
> Remove the prevoius hack as the SMU found the bug.
>
> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>

Minor spelling typo noted below.  With that fixed:
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

> ---
>  drivers/gpu/drm/amd/include/kgd_pp_interface.h       |  1 +
>  drivers/gpu/drm/amd/powerplay/amd_powerplay.c        | 16 ++++++++++++++++
>  drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c   | 19 +++++++++++++++++++
>  drivers/gpu/drm/amd/powerplay/inc/hwmgr.h            |  1 +
>  drivers/gpu/drm/amd/powerplay/smumgr/vega20_smumgr.c |  2 +-
>  drivers/gpu/drm/amd/powerplay/smumgr/vega20_smumgr.h |  2 ++
>  6 files changed, 40 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/include/kgd_pp_interface.h b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
> index 0de4e37..021ce466 100644
> --- a/drivers/gpu/drm/amd/include/kgd_pp_interface.h
> +++ b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
> @@ -275,6 +275,7 @@ struct amd_pm_funcs {
>         int (*set_power_profile_mode)(void *handle, long *input, uint32_t size);
>         int (*odn_edit_dpm_table)(void *handle, uint32_t type, long *input, uint32_t size);
>         int (*set_mp1_state)(void *handle, enum pp_mp1_state mp1_state);
> +       int (*smu_i2c_bus_access)(void *handle, bool aquire);

typo: aquire -> acquire

>  /* export to DC */
>         u32 (*get_sclk)(void *handle, bool low);
>         u32 (*get_mclk)(void *handle, bool low);
> diff --git a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
> index 7ef2027..bef5cd1 100644
> --- a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
> +++ b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
> @@ -1528,6 +1528,21 @@ static int pp_asic_reset_mode_2(void *handle)
>         return ret;
>  }
>
> +static int pp_smu_i2c_bus_access(void *handle, bool aquire)

and here.

> +{
> +       struct pp_hwmgr *hwmgr = handle;
> +
> +       if (!hwmgr || !hwmgr->pm_en)
> +               return -EINVAL;
> +
> +       if (hwmgr->hwmgr_func->smu_i2c_bus_access == NULL) {
> +               pr_info_ratelimited("%s was not implemented.\n", __func__);
> +               return -EINVAL;
> +       }
> +
> +       return hwmgr->hwmgr_func->smu_i2c_bus_access(hwmgr, aquire);
> +}
> +
>  static const struct amd_pm_funcs pp_dpm_funcs = {
>         .load_firmware = pp_dpm_load_fw,
>         .wait_for_fw_loading_complete = pp_dpm_fw_loading_complete,
> @@ -1585,4 +1600,5 @@ static const struct amd_pm_funcs pp_dpm_funcs = {
>         .get_ppfeature_status = pp_get_ppfeature_status,
>         .set_ppfeature_status = pp_set_ppfeature_status,
>         .asic_reset_mode_2 = pp_asic_reset_mode_2,
> +       .smu_i2c_bus_access = pp_smu_i2c_bus_access,
>  };
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c b/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
> index 0516c29..f26b14f 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
> @@ -4085,6 +4085,24 @@ static int vega20_get_thermal_temperature_range(struct pp_hwmgr *hwmgr,
>         return 0;
>  }
>
> +static int vega20_smu_i2c_bus_access(struct pp_hwmgr *hwmgr, bool aquire)

Same here.

> +{
> +       int res;
> +
> +       /* I2C bus access can happen very early, when SMU not loaded yet */
> +       if (!vega20_is_smc_ram_running(hwmgr))
> +               return 0;
> +
> +       res = smum_send_msg_to_smc_with_parameter(hwmgr,
> +                                                 (aquire ?
> +                                                 PPSMC_MSG_RequestI2CBus :
> +                                                 PPSMC_MSG_ReleaseI2CBus),
> +                                                 0);
> +
> +       PP_ASSERT_WITH_CODE(!res, "[SmuI2CAccessBus] Failed to access bus!", return res);
> +       return res;
> +}
> +
>  static const struct pp_hwmgr_func vega20_hwmgr_funcs = {
>         /* init/fini related */
>         .backend_init = vega20_hwmgr_backend_init,
> @@ -4152,6 +4170,7 @@ static const struct pp_hwmgr_func vega20_hwmgr_funcs = {
>         .get_asic_baco_state = vega20_baco_get_state,
>         .set_asic_baco_state = vega20_baco_set_state,
>         .set_mp1_state = vega20_set_mp1_state,
> +       .smu_i2c_bus_access = vega20_smu_i2c_bus_access,
>  };
>
>  int vega20_hwmgr_init(struct pp_hwmgr *hwmgr)
> diff --git a/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h b/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
> index abeff15..7bf9a14 100644
> --- a/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
> +++ b/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
> @@ -354,6 +354,7 @@ struct pp_hwmgr_func {
>         int (*set_ppfeature_status)(struct pp_hwmgr *hwmgr, uint64_t ppfeature_masks);
>         int (*set_mp1_state)(struct pp_hwmgr *hwmgr, enum pp_mp1_state mp1_state);
>         int (*asic_reset)(struct pp_hwmgr *hwmgr, enum SMU_ASIC_RESET_MODE mode);
> +       int (*smu_i2c_bus_access)(struct pp_hwmgr *hwmgr, bool aquire);
>  };
>
>  struct pp_table_func {
> diff --git a/drivers/gpu/drm/amd/powerplay/smumgr/vega20_smumgr.c b/drivers/gpu/drm/amd/powerplay/smumgr/vega20_smumgr.c
> index 3e97b83..b9089c6 100644
> --- a/drivers/gpu/drm/amd/powerplay/smumgr/vega20_smumgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/smumgr/vega20_smumgr.c
> @@ -44,7 +44,7 @@
>  #define smnMP0_FW_INTF                 0x30101c0
>  #define smnMP1_PUB_CTRL                        0x3010b14
>
> -static bool vega20_is_smc_ram_running(struct pp_hwmgr *hwmgr)
> +bool vega20_is_smc_ram_running(struct pp_hwmgr *hwmgr)
>  {
>         struct amdgpu_device *adev = hwmgr->adev;
>         uint32_t mp1_fw_flags;
> diff --git a/drivers/gpu/drm/amd/powerplay/smumgr/vega20_smumgr.h b/drivers/gpu/drm/amd/powerplay/smumgr/vega20_smumgr.h
> index ec953ab..62ebbfd 100644
> --- a/drivers/gpu/drm/amd/powerplay/smumgr/vega20_smumgr.h
> +++ b/drivers/gpu/drm/amd/powerplay/smumgr/vega20_smumgr.h
> @@ -57,5 +57,7 @@ int vega20_get_activity_monitor_coeff(struct pp_hwmgr *hwmgr,
>                 uint8_t *table, uint16_t workload_type);
>  int vega20_set_pptable_driver_address(struct pp_hwmgr *hwmgr);
>
> +bool vega20_is_smc_ram_running(struct pp_hwmgr *hwmgr);
> +
>  #endif
>
> --
> 2.7.4
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx