[PATCH 0/8] psp code clean up

[PATCH 0/8] psp code clean up

[Hawking Zhang]

the seires make cleanup in current software psp
support, including:
1). retire unnecessary ip callback function like
support_vmr_ring, check_fw_loading.
2). remove unnecessary tOS version check
3). create common helper functions to avoid duplicate
code per IP generation

Hawking Zhang (8):
  drm/amdgpu: retire support_vmr_ring interface
  drm/amdgpu: remove unnecessary tOS version check
  drm/amdgpu: retire unused check_fw_loading status
  drm/amdgpu: add helper function to init asd ucode
  drm/amdgpu: switch to helper function to init asd ucode
  drm/amdgpu: add helper function to init sos ucode
  drm/amdgpu: switch to helper function to init sos ucode
  drm/amdgpu: retire legacy vega10 sos version check

 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 130 ++++++++++++----
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h |  14 +-
 drivers/gpu/drm/amd/amdgpu/psp_v10_0.c  | 141 +----------------
 drivers/gpu/drm/amd/amdgpu/psp_v11_0.c  | 235 ++--------------------------
 drivers/gpu/drm/amd/amdgpu/psp_v12_0.c  | 172 +--------------------
 drivers/gpu/drm/amd/amdgpu/psp_v3_1.c   | 262 +++-----------------------------
 6 files changed, 149 insertions(+), 805 deletions(-)

-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 1/8] drm/amdgpu: retire support_vmr_ring interface

[Hawking Zhang]

vmr ring is dedicated for sriov vf (i.e.guest driver
in sriov), which is general communication interface
between driver and psp fw accross all ip version.
it is not correct to make it as ip specific callback.
it is even worse to check specific tOS version per IP
version (like psp_v11/v12).

Signed-off-by: Hawking Zhang <Hawking.Zhang@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h |  3 --
 drivers/gpu/drm/amd/amdgpu/psp_v11_0.c  | 18 +++--------
 drivers/gpu/drm/amd/amdgpu/psp_v12_0.c  | 17 +++-------
 drivers/gpu/drm/amd/amdgpu/psp_v3_1.c   | 56 +++++++++++----------------------
 5 files changed, 30 insertions(+), 66 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
index 8020f18..901ee79 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
@@ -274,7 +274,7 @@ static void psp_prep_tmr_cmd_buf(struct psp_context *psp,
 				 struct psp_gfx_cmd_resp *cmd,
 				 uint64_t tmr_mc, uint32_t size)
 {
-	if (psp_support_vmr_ring(psp))
+	if (amdgpu_sriov_vf(psp->adev))
 		cmd->cmd_id = GFX_CMD_ID_SETUP_VMR;
 	else
 		cmd->cmd_id = GFX_CMD_ID_SETUP_TMR;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
index 6a717fd..65a7d0a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
@@ -104,7 +104,6 @@ struct psp_funcs
 				      struct psp_xgmi_topology_info *topology);
 	int (*xgmi_set_topology_info)(struct psp_context *psp, int number_devices,
 				      struct psp_xgmi_topology_info *topology);
-	bool (*support_vmr_ring)(struct psp_context *psp);
 	int (*ras_trigger_error)(struct psp_context *psp,
 			struct ta_ras_trigger_error_input *info);
 	int (*ras_cure_posion)(struct psp_context *psp, uint64_t *mode_ptr);
@@ -320,8 +319,6 @@ struct amdgpu_psp_funcs {
 		((psp)->funcs->bootloader_load_sos ? (psp)->funcs->bootloader_load_sos((psp)) : 0)
 #define psp_smu_reload_quirk(psp) \
 		((psp)->funcs->smu_reload_quirk ? (psp)->funcs->smu_reload_quirk((psp)) : false)
-#define psp_support_vmr_ring(psp) \
-		((psp)->funcs->support_vmr_ring ? (psp)->funcs->support_vmr_ring((psp)) : false)
 #define psp_mode1_reset(psp) \
 		((psp)->funcs->mode1_reset ? (psp)->funcs->mode1_reset((psp)) : false)
 #define psp_xgmi_get_node_id(psp, node_id) \
diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
index 0afd610..46ef008 100644
--- a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
@@ -446,13 +446,6 @@ static int psp_v11_0_ring_init(struct psp_context *psp,
 	return 0;
 }
 
-static bool psp_v11_0_support_vmr_ring(struct psp_context *psp)
-{
-	if (amdgpu_sriov_vf(psp->adev) && psp->sos_fw_version > 0x80045)
-		return true;
-	return false;
-}
-
 static int psp_v11_0_ring_stop(struct psp_context *psp,
 			      enum psp_ring_type ring_type)
 {
@@ -460,7 +453,7 @@ static int psp_v11_0_ring_stop(struct psp_context *psp,
 	struct amdgpu_device *adev = psp->adev;
 
 	/* Write the ring destroy command*/
-	if (psp_v11_0_support_vmr_ring(psp))
+	if (amdgpu_sriov_vf(adev))
 		WREG32_SOC15(MP0, 0, mmMP0_SMN_C2PMSG_101,
 				     GFX_CTRL_CMD_ID_DESTROY_GPCOM_RING);
 	else
@@ -471,7 +464,7 @@ static int psp_v11_0_ring_stop(struct psp_context *psp,
 	mdelay(20);
 
 	/* Wait for response flag (bit 31) */
-	if (psp_v11_0_support_vmr_ring(psp))
+	if (amdgpu_sriov_vf(adev))
 		ret = psp_wait_for(psp, SOC15_REG_OFFSET(MP0, 0, mmMP0_SMN_C2PMSG_101),
 				   0x80000000, 0x80000000, false);
 	else
@@ -489,7 +482,7 @@ static int psp_v11_0_ring_create(struct psp_context *psp,
 	struct psp_ring *ring = &psp->km_ring;
 	struct amdgpu_device *adev = psp->adev;
 
-	if (psp_v11_0_support_vmr_ring(psp)) {
+	if (amdgpu_sriov_vf(adev)) {
 		ret = psp_v11_0_ring_stop(psp, ring_type);
 		if (ret) {
 			DRM_ERROR("psp_v11_0_ring_stop_sriov failed!\n");
@@ -1099,7 +1092,7 @@ static uint32_t psp_v11_0_ring_get_wptr(struct psp_context *psp)
 	uint32_t data;
 	struct amdgpu_device *adev = psp->adev;
 
-	if (psp_v11_0_support_vmr_ring(psp))
+	if (amdgpu_sriov_vf(adev))
 		data = RREG32_SOC15(MP0, 0, mmMP0_SMN_C2PMSG_102);
 	else
 		data = RREG32_SOC15(MP0, 0, mmMP0_SMN_C2PMSG_67);
@@ -1111,7 +1104,7 @@ static void psp_v11_0_ring_set_wptr(struct psp_context *psp, uint32_t value)
 {
 	struct amdgpu_device *adev = psp->adev;
 
-	if (psp_v11_0_support_vmr_ring(psp)) {
+	if (amdgpu_sriov_vf(adev)) {
 		WREG32_SOC15(MP0, 0, mmMP0_SMN_C2PMSG_102, value);
 		WREG32_SOC15(MP0, 0, mmMP0_SMN_C2PMSG_101, GFX_CTRL_CMD_ID_CONSUME_CMD);
 	} else
@@ -1209,7 +1202,6 @@ static const struct psp_funcs psp_v11_0_funcs = {
 	.xgmi_set_topology_info = psp_v11_0_xgmi_set_topology_info,
 	.xgmi_get_hive_id = psp_v11_0_xgmi_get_hive_id,
 	.xgmi_get_node_id = psp_v11_0_xgmi_get_node_id,
-	.support_vmr_ring = psp_v11_0_support_vmr_ring,
 	.ras_trigger_error = psp_v11_0_ras_trigger_error,
 	.ras_cure_posion = psp_v11_0_ras_cure_posion,
 	.rlc_autoload_start = psp_v11_0_rlc_autoload_start,
diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v12_0.c b/drivers/gpu/drm/amd/amdgpu/psp_v12_0.c
index 58d8b6d..17e4475 100644
--- a/drivers/gpu/drm/amd/amdgpu/psp_v12_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/psp_v12_0.c
@@ -228,13 +228,6 @@ static int psp_v12_0_ring_init(struct psp_context *psp,
 	return 0;
 }
 
-static bool psp_v12_0_support_vmr_ring(struct psp_context *psp)
-{
-	if (amdgpu_sriov_vf(psp->adev) && psp->sos_fw_version > 0x80045)
-		return true;
-	return false;
-}
-
 static int psp_v12_0_ring_create(struct psp_context *psp,
 				enum psp_ring_type ring_type)
 {
@@ -243,7 +236,7 @@ static int psp_v12_0_ring_create(struct psp_context *psp,
 	struct psp_ring *ring = &psp->km_ring;
 	struct amdgpu_device *adev = psp->adev;
 
-	if (psp_v12_0_support_vmr_ring(psp)) {
+	if (amdgpu_sriov_vf(psp->adev)) {
 		/* Write low address of the ring to C2PMSG_102 */
 		psp_ring_reg = lower_32_bits(ring->ring_mem_mc_addr);
 		WREG32_SOC15(MP0, 0, mmMP0_SMN_C2PMSG_102, psp_ring_reg);
@@ -295,7 +288,7 @@ static int psp_v12_0_ring_stop(struct psp_context *psp,
 	struct amdgpu_device *adev = psp->adev;
 
 	/* Write the ring destroy command*/
-	if (psp_v12_0_support_vmr_ring(psp))
+	if (amdgpu_sriov_vf(adev))
 		WREG32_SOC15(MP0, 0, mmMP0_SMN_C2PMSG_101,
 				     GFX_CTRL_CMD_ID_DESTROY_GPCOM_RING);
 	else
@@ -306,7 +299,7 @@ static int psp_v12_0_ring_stop(struct psp_context *psp,
 	mdelay(20);
 
 	/* Wait for response flag (bit 31) */
-	if (psp_v12_0_support_vmr_ring(psp))
+	if (amdgpu_sriov_vf(adev))
 		ret = psp_wait_for(psp, SOC15_REG_OFFSET(MP0, 0, mmMP0_SMN_C2PMSG_101),
 				   0x80000000, 0x80000000, false);
 	else
@@ -495,7 +488,7 @@ static uint32_t psp_v12_0_ring_get_wptr(struct psp_context *psp)
 	uint32_t data;
 	struct amdgpu_device *adev = psp->adev;
 
-	if (psp_v12_0_support_vmr_ring(psp))
+	if (amdgpu_sriov_vf(adev))
 		data = RREG32_SOC15(MP0, 0, mmMP0_SMN_C2PMSG_102);
 	else
 		data = RREG32_SOC15(MP0, 0, mmMP0_SMN_C2PMSG_67);
@@ -507,7 +500,7 @@ static void psp_v12_0_ring_set_wptr(struct psp_context *psp, uint32_t value)
 {
 	struct amdgpu_device *adev = psp->adev;
 
-	if (psp_v12_0_support_vmr_ring(psp)) {
+	if (amdgpu_sriov_vf(adev)) {
 		WREG32_SOC15(MP0, 0, mmMP0_SMN_C2PMSG_102, value);
 		WREG32_SOC15(MP0, 0, mmMP0_SMN_C2PMSG_101, GFX_CTRL_CMD_ID_CONSUME_CMD);
 	} else
diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c b/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c
index 735c43c..bd13625 100644
--- a/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c
+++ b/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c
@@ -52,7 +52,6 @@ MODULE_FIRMWARE("amdgpu/vega12_asd.bin");
 
 static uint32_t sos_old_versions[] = {1517616, 1510592, 1448594, 1446554};
 
-static bool psp_v3_1_support_vmr_ring(struct psp_context *psp);
 static int psp_v3_1_ring_stop(struct psp_context *psp,
 			      enum psp_ring_type ring_type);
 
@@ -302,7 +301,7 @@ static int psp_v3_1_ring_create(struct psp_context *psp,
 
 	psp_v3_1_reroute_ih(psp);
 
-	if (psp_v3_1_support_vmr_ring(psp)) {
+	if (amdgpu_sriov_vf(adev)) {
 		ret = psp_v3_1_ring_stop(psp, ring_type);
 		if (ret) {
 			DRM_ERROR("psp_v3_1_ring_stop_sriov failed!\n");
@@ -360,34 +359,26 @@ static int psp_v3_1_ring_stop(struct psp_context *psp,
 			      enum psp_ring_type ring_type)
 {
 	int ret = 0;
-	unsigned int psp_ring_reg = 0;
 	struct amdgpu_device *adev = psp->adev;
 
-	if (psp_v3_1_support_vmr_ring(psp)) {
-		/* Write the Destroy GPCOM ring command to C2PMSG_101 */
-		psp_ring_reg = GFX_CTRL_CMD_ID_DESTROY_GPCOM_RING;
-		WREG32_SOC15(MP0, 0, mmMP0_SMN_C2PMSG_101, psp_ring_reg);
-
-		/* there might be handshake issue which needs delay */
-		mdelay(20);
-
-		/* Wait for response flag (bit 31) in C2PMSG_101 */
-		ret = psp_wait_for(psp,
-				SOC15_REG_OFFSET(MP0, 0, mmMP0_SMN_C2PMSG_101),
-				0x80000000, 0x80000000, false);
-	} else {
-		/* Write the ring destroy command to C2PMSG_64 */
-		psp_ring_reg = 3 << 16;
-		WREG32_SOC15(MP0, 0, mmMP0_SMN_C2PMSG_64, psp_ring_reg);
+	/* Write the ring destroy command*/
+	if (amdgpu_sriov_vf(adev))
+		WREG32_SOC15(MP0, 0, mmMP0_SMN_C2PMSG_101,
+				     GFX_CTRL_CMD_ID_DESTROY_GPCOM_RING);
+	else
+		WREG32_SOC15(MP0, 0, mmMP0_SMN_C2PMSG_64,
+				     GFX_CTRL_CMD_ID_DESTROY_RINGS);
 
-		/* there might be handshake issue which needs delay */
-		mdelay(20);
+	/* there might be handshake issue with hardware which needs delay */
+	mdelay(20);
 
-		/* Wait for response flag (bit 31) in C2PMSG_64 */
-		ret = psp_wait_for(psp,
-				SOC15_REG_OFFSET(MP0, 0, mmMP0_SMN_C2PMSG_64),
-				0x80000000, 0x80000000, false);
-	}
+	/* Wait for response flag (bit 31) */
+	if (amdgpu_sriov_vf(adev))
+		ret = psp_wait_for(psp, SOC15_REG_OFFSET(MP0, 0, mmMP0_SMN_C2PMSG_101),
+				   0x80000000, 0x80000000, false);
+	else
+		ret = psp_wait_for(psp, SOC15_REG_OFFSET(MP0, 0, mmMP0_SMN_C2PMSG_64),
+				   0x80000000, 0x80000000, false);
 
 	return ret;
 }
@@ -575,20 +566,12 @@ static int psp_v3_1_mode1_reset(struct psp_context *psp)
 	return 0;
 }
 
-static bool psp_v3_1_support_vmr_ring(struct psp_context *psp)
-{
-	if (amdgpu_sriov_vf(psp->adev))
-		return true;
-
-	return false;
-}
-
 static uint32_t psp_v3_1_ring_get_wptr(struct psp_context *psp)
 {
 	uint32_t data;
 	struct amdgpu_device *adev = psp->adev;
 
-	if (psp_v3_1_support_vmr_ring(psp))
+	if (amdgpu_sriov_vf(adev))
 		data = RREG32_SOC15(MP0, 0, mmMP0_SMN_C2PMSG_102);
 	else
 		data = RREG32_SOC15(MP0, 0, mmMP0_SMN_C2PMSG_67);
@@ -599,7 +582,7 @@ static void psp_v3_1_ring_set_wptr(struct psp_context *psp, uint32_t value)
 {
 	struct amdgpu_device *adev = psp->adev;
 
-	if (psp_v3_1_support_vmr_ring(psp)) {
+	if (amdgpu_sriov_vf(adev)) {
 		WREG32_SOC15(MP0, 0, mmMP0_SMN_C2PMSG_102, value);
 		/* send interrupt to PSP for SRIOV ring write pointer update */
 		WREG32_SOC15(MP0, 0, mmMP0_SMN_C2PMSG_101,
@@ -619,7 +602,6 @@ static const struct psp_funcs psp_v3_1_funcs = {
 	.compare_sram_data = psp_v3_1_compare_sram_data,
 	.smu_reload_quirk = psp_v3_1_smu_reload_quirk,
 	.mode1_reset = psp_v3_1_mode1_reset,
-	.support_vmr_ring = psp_v3_1_support_vmr_ring,
 	.ring_get_wptr = psp_v3_1_ring_get_wptr,
 	.ring_set_wptr = psp_v3_1_ring_set_wptr,
 };
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 2/8] drm/amdgpu: remove unnecessary tOS version check

[Hawking Zhang]

tOS version is available through debugfs interface

Signed-off-by: Hawking Zhang <Hawking.Zhang@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/psp_v11_0.c | 10 ++--------
 drivers/gpu/drm/amd/amdgpu/psp_v12_0.c |  5 +----
 drivers/gpu/drm/amd/amdgpu/psp_v3_1.c  |  5 +----
 3 files changed, 4 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
index 46ef008..20fbd43 100644
--- a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
@@ -283,11 +283,8 @@ static int psp_v11_0_bootloader_load_kdb(struct psp_context *psp)
 	/* Check tOS sign of life register to confirm sys driver and sOS
 	 * are already been loaded.
 	 */
-	if (psp_v11_0_is_sos_alive(psp)) {
-		psp->sos_fw_version = RREG32_SOC15(MP0, 0, mmMP0_SMN_C2PMSG_58);
-		dev_info(adev->dev, "sos fw version = 0x%x.\n", psp->sos_fw_version);
+	if (psp_v11_0_is_sos_alive(psp))
 		return 0;
-	}
 
 	ret = psp_v11_0_wait_for_bootloader(psp);
 	if (ret)
@@ -319,11 +316,8 @@ static int psp_v11_0_bootloader_load_sysdrv(struct psp_context *psp)
 	/* Check sOS sign of life register to confirm sys driver and sOS
 	 * are already been loaded.
 	 */
-	if (psp_v11_0_is_sos_alive(psp)) {
-		psp->sos_fw_version = RREG32_SOC15(MP0, 0, mmMP0_SMN_C2PMSG_58);
-		dev_info(adev->dev, "sos fw version = 0x%x.\n", psp->sos_fw_version);
+	if (psp_v11_0_is_sos_alive(psp))
 		return 0;
-	}
 
 	ret = psp_v11_0_wait_for_bootloader(psp);
 	if (ret)
diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v12_0.c b/drivers/gpu/drm/amd/amdgpu/psp_v12_0.c
index 17e4475..d3c86a0 100644
--- a/drivers/gpu/drm/amd/amdgpu/psp_v12_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/psp_v12_0.c
@@ -95,11 +95,8 @@ static int psp_v12_0_bootloader_load_sysdrv(struct psp_context *psp)
 	 * are already been loaded.
 	 */
 	sol_reg = RREG32_SOC15(MP0, 0, mmMP0_SMN_C2PMSG_81);
-	if (sol_reg) {
-		psp->sos_fw_version = RREG32_SOC15(MP0, 0, mmMP0_SMN_C2PMSG_58);
-		printk("sos fw version = 0x%x.\n", psp->sos_fw_version);
+	if (sol_reg)
 		return 0;
-	}
 
 	/* Wait for bootloader to signify that is ready having bit 31 of C2PMSG_35 set to 1 */
 	ret = psp_wait_for(psp, SOC15_REG_OFFSET(MP0, 0, mmMP0_SMN_C2PMSG_35),
diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c b/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c
index bd13625..ab03190 100644
--- a/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c
+++ b/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c
@@ -197,11 +197,8 @@ static int psp_v3_1_bootloader_load_sos(struct psp_context *psp)
 	 * are already been loaded.
 	 */
 	sol_reg = RREG32_SOC15(MP0, 0, mmMP0_SMN_C2PMSG_81);
-	if (sol_reg) {
-		psp->sos_fw_version = RREG32_SOC15(MP0, 0, mmMP0_SMN_C2PMSG_58);
-		printk("sos fw version = 0x%x.\n", psp->sos_fw_version);
+	if (sol_reg)
 		return 0;
-	}
 
 	/* Wait for bootloader to signify that is ready having bit 31 of C2PMSG_35 set to 1 */
 	ret = psp_wait_for(psp, SOC15_REG_OFFSET(MP0, 0, mmMP0_SMN_C2PMSG_35),
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 3/8] drm/amdgpu: retire unused check_fw_loading status

[Hawking Zhang]

It is not allowed to read out engine sram via registers
to check the fw loading status.

Signed-off-by: Hawking Zhang <Hawking.Zhang@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c |  34 --------
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h |   7 --
 drivers/gpu/drm/amd/amdgpu/psp_v10_0.c  | 124 -----------------------------
 drivers/gpu/drm/amd/amdgpu/psp_v11_0.c  | 133 --------------------------------
 drivers/gpu/drm/amd/amdgpu/psp_v12_0.c  | 123 -----------------------------
 drivers/gpu/drm/amd/amdgpu/psp_v3_1.c   | 123 -----------------------------
 6 files changed, 544 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
index 901ee79..7797065 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
@@ -37,8 +37,6 @@
 
 #include "amdgpu_ras.h"
 
-static void psp_set_funcs(struct amdgpu_device *adev);
-
 static int psp_sysfs_init(struct amdgpu_device *adev);
 static void psp_sysfs_fini(struct amdgpu_device *adev);
 
@@ -82,8 +80,6 @@ static int psp_early_init(void *handle)
 	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
 	struct psp_context *psp = &adev->psp;
 
-	psp_set_funcs(adev);
-
 	switch (adev->asic_type) {
 	case CHIP_VEGA10:
 	case CHIP_VEGA12:
@@ -1487,11 +1483,6 @@ static int psp_np_fw_load(struct psp_context *psp)
 				return ret;
 			}
 		}
-#if 0
-		/* check if firmware loaded sucessfully */
-		if (!amdgpu_psp_check_fw_loading_status(adev, i))
-			return -EINVAL;
-#endif
 	}
 
 	return 0;
@@ -1849,21 +1840,6 @@ int psp_ring_cmd_submit(struct psp_context *psp,
 	return 0;
 }
 
-static bool psp_check_fw_loading_status(struct amdgpu_device *adev,
-					enum AMDGPU_UCODE_ID ucode_type)
-{
-	struct amdgpu_firmware_info *ucode = NULL;
-
-	if (!adev->firmware.fw_size)
-		return false;
-
-	ucode = &adev->firmware.ucode[ucode_type];
-	if (!ucode->fw || !ucode->ucode_size)
-		return false;
-
-	return psp_compare_sram_data(&adev->psp, ucode, ucode_type);
-}
-
 static int psp_set_clockgating_state(void *handle,
 				     enum amd_clockgating_state state)
 {
@@ -2000,16 +1976,6 @@ static void psp_sysfs_fini(struct amdgpu_device *adev)
 	device_remove_file(adev->dev, &dev_attr_usbc_pd_fw);
 }
 
-static const struct amdgpu_psp_funcs psp_funcs = {
-	.check_fw_loading_status = psp_check_fw_loading_status,
-};
-
-static void psp_set_funcs(struct amdgpu_device *adev)
-{
-	if (NULL == adev->firmware.funcs)
-		adev->firmware.funcs = &psp_funcs;
-}
-
 const struct amdgpu_ip_block_version psp_v3_1_ip_block =
 {
 	.type = AMD_IP_BLOCK_TYPE_PSP,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
index 65a7d0a..f8b1f03 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
@@ -93,9 +93,6 @@ struct psp_funcs
 			    enum psp_ring_type ring_type);
 	int (*ring_destroy)(struct psp_context *psp,
 			    enum psp_ring_type ring_type);
-	bool (*compare_sram_data)(struct psp_context *psp,
-				  struct amdgpu_firmware_info *ucode,
-				  enum AMDGPU_UCODE_ID ucode_type);
 	bool (*smu_reload_quirk)(struct psp_context *psp);
 	int (*mode1_reset)(struct psp_context *psp);
 	int (*xgmi_get_node_id)(struct psp_context *psp, uint64_t *node_id);
@@ -307,8 +304,6 @@ struct amdgpu_psp_funcs {
 #define psp_ring_create(psp, type) (psp)->funcs->ring_create((psp), (type))
 #define psp_ring_stop(psp, type) (psp)->funcs->ring_stop((psp), (type))
 #define psp_ring_destroy(psp, type) ((psp)->funcs->ring_destroy((psp), (type)))
-#define psp_compare_sram_data(psp, ucode, type) \
-		(psp)->funcs->compare_sram_data((psp), (ucode), (type))
 #define psp_init_microcode(psp) \
 		((psp)->funcs->init_microcode ? (psp)->funcs->init_microcode((psp)) : 0)
 #define psp_bootloader_load_kdb(psp) \
@@ -340,8 +335,6 @@ struct amdgpu_psp_funcs {
 #define psp_mem_training(psp, ops) \
 	((psp)->funcs->mem_training ? (psp)->funcs->mem_training((psp), (ops)) : 0)
 
-#define amdgpu_psp_check_fw_loading_status(adev, i) (adev)->firmware.funcs->check_fw_loading_status((adev), (i))
-
 #define psp_ras_trigger_error(psp, info) \
 	((psp)->funcs->ras_trigger_error ? \
 	(psp)->funcs->ras_trigger_error((psp), (info)) : -EINVAL)
diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c b/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
index 7539104..6e041b7 100644
--- a/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
@@ -230,129 +230,6 @@ static int psp_v10_0_ring_destroy(struct psp_context *psp,
 	return ret;
 }
 
-static int
-psp_v10_0_sram_map(struct amdgpu_device *adev,
-		   unsigned int *sram_offset, unsigned int *sram_addr_reg_offset,
-		   unsigned int *sram_data_reg_offset,
-		   enum AMDGPU_UCODE_ID ucode_id)
-{
-	int ret = 0;
-
-	switch(ucode_id) {
-/* TODO: needs to confirm */
-#if 0
-	case AMDGPU_UCODE_ID_SMC:
-		*sram_offset = 0;
-		*sram_addr_reg_offset = 0;
-		*sram_data_reg_offset = 0;
-		break;
-#endif
-
-	case AMDGPU_UCODE_ID_CP_CE:
-		*sram_offset = 0x0;
-		*sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_CE_UCODE_ADDR);
-		*sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_CE_UCODE_DATA);
-		break;
-
-	case AMDGPU_UCODE_ID_CP_PFP:
-		*sram_offset = 0x0;
-		*sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_PFP_UCODE_ADDR);
-		*sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_PFP_UCODE_DATA);
-		break;
-
-	case AMDGPU_UCODE_ID_CP_ME:
-		*sram_offset = 0x0;
-		*sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_HYP_ME_UCODE_ADDR);
-		*sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_HYP_ME_UCODE_DATA);
-		break;
-
-	case AMDGPU_UCODE_ID_CP_MEC1:
-		*sram_offset = 0x10000;
-		*sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_MEC_ME1_UCODE_ADDR);
-		*sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_MEC_ME1_UCODE_DATA);
-		break;
-
-	case AMDGPU_UCODE_ID_CP_MEC2:
-		*sram_offset = 0x10000;
-		*sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_HYP_MEC2_UCODE_ADDR);
-		*sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_HYP_MEC2_UCODE_DATA);
-		break;
-
-	case AMDGPU_UCODE_ID_RLC_G:
-		*sram_offset = 0x2000;
-		*sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmRLC_GPM_UCODE_ADDR);
-		*sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmRLC_GPM_UCODE_DATA);
-		break;
-
-	case AMDGPU_UCODE_ID_SDMA0:
-		*sram_offset = 0x0;
-		*sram_addr_reg_offset = SOC15_REG_OFFSET(SDMA0, 0, mmSDMA0_UCODE_ADDR);
-		*sram_data_reg_offset = SOC15_REG_OFFSET(SDMA0, 0, mmSDMA0_UCODE_DATA);
-		break;
-
-/* TODO: needs to confirm */
-#if 0
-	case AMDGPU_UCODE_ID_SDMA1:
-		*sram_offset = ;
-		*sram_addr_reg_offset = ;
-		break;
-
-	case AMDGPU_UCODE_ID_UVD:
-		*sram_offset = ;
-		*sram_addr_reg_offset = ;
-		break;
-
-	case AMDGPU_UCODE_ID_VCE:
-		*sram_offset = ;
-		*sram_addr_reg_offset = ;
-		break;
-#endif
-
-	case AMDGPU_UCODE_ID_MAXIMUM:
-	default:
-		ret = -EINVAL;
-		break;
-	}
-
-	return ret;
-}
-
-static bool psp_v10_0_compare_sram_data(struct psp_context *psp,
-					struct amdgpu_firmware_info *ucode,
-					enum AMDGPU_UCODE_ID ucode_type)
-{
-	int err = 0;
-	unsigned int fw_sram_reg_val = 0;
-	unsigned int fw_sram_addr_reg_offset = 0;
-	unsigned int fw_sram_data_reg_offset = 0;
-	unsigned int ucode_size;
-	uint32_t *ucode_mem = NULL;
-	struct amdgpu_device *adev = psp->adev;
-
-	err = psp_v10_0_sram_map(adev, &fw_sram_reg_val, &fw_sram_addr_reg_offset,
-				&fw_sram_data_reg_offset, ucode_type);
-	if (err)
-		return false;
-
-	WREG32(fw_sram_addr_reg_offset, fw_sram_reg_val);
-
-	ucode_size = ucode->ucode_size;
-	ucode_mem = (uint32_t *)ucode->kaddr;
-	while (!ucode_size) {
-		fw_sram_reg_val = RREG32(fw_sram_data_reg_offset);
-
-		if (*ucode_mem != fw_sram_reg_val)
-			return false;
-
-		ucode_mem++;
-		/* 4 bytes */
-		ucode_size -= 4;
-	}
-
-	return true;
-}
-
-
 static int psp_v10_0_mode1_reset(struct psp_context *psp)
 {
 	DRM_INFO("psp mode 1 reset not supported now! \n");
@@ -379,7 +256,6 @@ static const struct psp_funcs psp_v10_0_funcs = {
 	.ring_create = psp_v10_0_ring_create,
 	.ring_stop = psp_v10_0_ring_stop,
 	.ring_destroy = psp_v10_0_ring_destroy,
-	.compare_sram_data = psp_v10_0_compare_sram_data,
 	.mode1_reset = psp_v10_0_mode1_reset,
 	.ring_get_wptr = psp_v10_0_ring_get_wptr,
 	.ring_set_wptr = psp_v10_0_ring_set_wptr,
diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
index 20fbd43..f633577 100644
--- a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
@@ -554,138 +554,6 @@ static int psp_v11_0_ring_destroy(struct psp_context *psp,
 	return ret;
 }
 
-static int
-psp_v11_0_sram_map(struct amdgpu_device *adev,
-		  unsigned int *sram_offset, unsigned int *sram_addr_reg_offset,
-		  unsigned int *sram_data_reg_offset,
-		  enum AMDGPU_UCODE_ID ucode_id)
-{
-	int ret = 0;
-
-	switch (ucode_id) {
-/* TODO: needs to confirm */
-#if 0
-	case AMDGPU_UCODE_ID_SMC:
-		*sram_offset = 0;
-		*sram_addr_reg_offset = 0;
-		*sram_data_reg_offset = 0;
-		break;
-#endif
-
-	case AMDGPU_UCODE_ID_CP_CE:
-		*sram_offset = 0x0;
-		*sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_CE_UCODE_ADDR);
-		*sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_CE_UCODE_DATA);
-		break;
-
-	case AMDGPU_UCODE_ID_CP_PFP:
-		*sram_offset = 0x0;
-		*sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_PFP_UCODE_ADDR);
-		*sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_PFP_UCODE_DATA);
-		break;
-
-	case AMDGPU_UCODE_ID_CP_ME:
-		*sram_offset = 0x0;
-		*sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_HYP_ME_UCODE_ADDR);
-		*sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_HYP_ME_UCODE_DATA);
-		break;
-
-	case AMDGPU_UCODE_ID_CP_MEC1:
-		*sram_offset = 0x10000;
-		*sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_MEC_ME1_UCODE_ADDR);
-		*sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_MEC_ME1_UCODE_DATA);
-		break;
-
-	case AMDGPU_UCODE_ID_CP_MEC2:
-		*sram_offset = 0x10000;
-		*sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_HYP_MEC2_UCODE_ADDR);
-		*sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_HYP_MEC2_UCODE_DATA);
-		break;
-
-	case AMDGPU_UCODE_ID_RLC_G:
-		*sram_offset = 0x2000;
-		if (adev->asic_type < CHIP_NAVI10) {
-			*sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmRLC_GPM_UCODE_ADDR);
-			*sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmRLC_GPM_UCODE_DATA);
-		} else {
-			*sram_addr_reg_offset = adev->reg_offset[GC_HWIP][0][1] + mmRLC_GPM_UCODE_ADDR_NV10;
-			*sram_data_reg_offset = adev->reg_offset[GC_HWIP][0][1] + mmRLC_GPM_UCODE_DATA_NV10;
-		}
-		break;
-
-	case AMDGPU_UCODE_ID_SDMA0:
-		*sram_offset = 0x0;
-		if (adev->asic_type < CHIP_NAVI10) {
-			*sram_addr_reg_offset = SOC15_REG_OFFSET(SDMA0, 0, mmSDMA0_UCODE_ADDR);
-			*sram_data_reg_offset = SOC15_REG_OFFSET(SDMA0, 0, mmSDMA0_UCODE_DATA);
-		} else {
-			*sram_addr_reg_offset = adev->reg_offset[GC_HWIP][0][1] + mmSDMA0_UCODE_ADDR_NV10;
-			*sram_data_reg_offset = adev->reg_offset[GC_HWIP][0][1] + mmSDMA0_UCODE_DATA_NV10;
-		}
-		break;
-
-/* TODO: needs to confirm */
-#if 0
-	case AMDGPU_UCODE_ID_SDMA1:
-		*sram_offset = ;
-		*sram_addr_reg_offset = ;
-		break;
-
-	case AMDGPU_UCODE_ID_UVD:
-		*sram_offset = ;
-		*sram_addr_reg_offset = ;
-		break;
-
-	case AMDGPU_UCODE_ID_VCE:
-		*sram_offset = ;
-		*sram_addr_reg_offset = ;
-		break;
-#endif
-
-	case AMDGPU_UCODE_ID_MAXIMUM:
-	default:
-		ret = -EINVAL;
-		break;
-	}
-
-	return ret;
-}
-
-static bool psp_v11_0_compare_sram_data(struct psp_context *psp,
-				       struct amdgpu_firmware_info *ucode,
-				       enum AMDGPU_UCODE_ID ucode_type)
-{
-	int err = 0;
-	unsigned int fw_sram_reg_val = 0;
-	unsigned int fw_sram_addr_reg_offset = 0;
-	unsigned int fw_sram_data_reg_offset = 0;
-	unsigned int ucode_size;
-	uint32_t *ucode_mem = NULL;
-	struct amdgpu_device *adev = psp->adev;
-
-	err = psp_v11_0_sram_map(adev, &fw_sram_reg_val, &fw_sram_addr_reg_offset,
-				&fw_sram_data_reg_offset, ucode_type);
-	if (err)
-		return false;
-
-	WREG32(fw_sram_addr_reg_offset, fw_sram_reg_val);
-
-	ucode_size = ucode->ucode_size;
-	ucode_mem = (uint32_t *)ucode->kaddr;
-	while (ucode_size) {
-		fw_sram_reg_val = RREG32(fw_sram_data_reg_offset);
-
-		if (*ucode_mem != fw_sram_reg_val)
-			return false;
-
-		ucode_mem++;
-		/* 4 bytes */
-		ucode_size -= 4;
-	}
-
-	return true;
-}
-
 static int psp_v11_0_mode1_reset(struct psp_context *psp)
 {
 	int ret;
@@ -1190,7 +1058,6 @@ static const struct psp_funcs psp_v11_0_funcs = {
 	.ring_create = psp_v11_0_ring_create,
 	.ring_stop = psp_v11_0_ring_stop,
 	.ring_destroy = psp_v11_0_ring_destroy,
-	.compare_sram_data = psp_v11_0_compare_sram_data,
 	.mode1_reset = psp_v11_0_mode1_reset,
 	.xgmi_get_topology_info = psp_v11_0_xgmi_get_topology_info,
 	.xgmi_set_topology_info = psp_v11_0_xgmi_set_topology_info,
diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v12_0.c b/drivers/gpu/drm/amd/amdgpu/psp_v12_0.c
index d3c86a0..42c485b 100644
--- a/drivers/gpu/drm/amd/amdgpu/psp_v12_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/psp_v12_0.c
@@ -324,128 +324,6 @@ static int psp_v12_0_ring_destroy(struct psp_context *psp,
 	return ret;
 }
 
-static int
-psp_v12_0_sram_map(struct amdgpu_device *adev,
-		  unsigned int *sram_offset, unsigned int *sram_addr_reg_offset,
-		  unsigned int *sram_data_reg_offset,
-		  enum AMDGPU_UCODE_ID ucode_id)
-{
-	int ret = 0;
-
-	switch (ucode_id) {
-/* TODO: needs to confirm */
-#if 0
-	case AMDGPU_UCODE_ID_SMC:
-		*sram_offset = 0;
-		*sram_addr_reg_offset = 0;
-		*sram_data_reg_offset = 0;
-		break;
-#endif
-
-	case AMDGPU_UCODE_ID_CP_CE:
-		*sram_offset = 0x0;
-		*sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_CE_UCODE_ADDR);
-		*sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_CE_UCODE_DATA);
-		break;
-
-	case AMDGPU_UCODE_ID_CP_PFP:
-		*sram_offset = 0x0;
-		*sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_PFP_UCODE_ADDR);
-		*sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_PFP_UCODE_DATA);
-		break;
-
-	case AMDGPU_UCODE_ID_CP_ME:
-		*sram_offset = 0x0;
-		*sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_HYP_ME_UCODE_ADDR);
-		*sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_HYP_ME_UCODE_DATA);
-		break;
-
-	case AMDGPU_UCODE_ID_CP_MEC1:
-		*sram_offset = 0x10000;
-		*sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_MEC_ME1_UCODE_ADDR);
-		*sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_MEC_ME1_UCODE_DATA);
-		break;
-
-	case AMDGPU_UCODE_ID_CP_MEC2:
-		*sram_offset = 0x10000;
-		*sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_HYP_MEC2_UCODE_ADDR);
-		*sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_HYP_MEC2_UCODE_DATA);
-		break;
-
-	case AMDGPU_UCODE_ID_RLC_G:
-		*sram_offset = 0x2000;
-		*sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmRLC_GPM_UCODE_ADDR);
-		*sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmRLC_GPM_UCODE_DATA);
-		break;
-
-	case AMDGPU_UCODE_ID_SDMA0:
-		*sram_offset = 0x0;
-		*sram_addr_reg_offset = SOC15_REG_OFFSET(SDMA0, 0, mmSDMA0_UCODE_ADDR);
-		*sram_data_reg_offset = SOC15_REG_OFFSET(SDMA0, 0, mmSDMA0_UCODE_DATA);
-		break;
-
-/* TODO: needs to confirm */
-#if 0
-	case AMDGPU_UCODE_ID_SDMA1:
-		*sram_offset = ;
-		*sram_addr_reg_offset = ;
-		break;
-
-	case AMDGPU_UCODE_ID_UVD:
-		*sram_offset = ;
-		*sram_addr_reg_offset = ;
-		break;
-
-	case AMDGPU_UCODE_ID_VCE:
-		*sram_offset = ;
-		*sram_addr_reg_offset = ;
-		break;
-#endif
-
-	case AMDGPU_UCODE_ID_MAXIMUM:
-	default:
-		ret = -EINVAL;
-		break;
-	}
-
-	return ret;
-}
-
-static bool psp_v12_0_compare_sram_data(struct psp_context *psp,
-				       struct amdgpu_firmware_info *ucode,
-				       enum AMDGPU_UCODE_ID ucode_type)
-{
-	int err = 0;
-	unsigned int fw_sram_reg_val = 0;
-	unsigned int fw_sram_addr_reg_offset = 0;
-	unsigned int fw_sram_data_reg_offset = 0;
-	unsigned int ucode_size;
-	uint32_t *ucode_mem = NULL;
-	struct amdgpu_device *adev = psp->adev;
-
-	err = psp_v12_0_sram_map(adev, &fw_sram_reg_val, &fw_sram_addr_reg_offset,
-				&fw_sram_data_reg_offset, ucode_type);
-	if (err)
-		return false;
-
-	WREG32(fw_sram_addr_reg_offset, fw_sram_reg_val);
-
-	ucode_size = ucode->ucode_size;
-	ucode_mem = (uint32_t *)ucode->kaddr;
-	while (ucode_size) {
-		fw_sram_reg_val = RREG32(fw_sram_data_reg_offset);
-
-		if (*ucode_mem != fw_sram_reg_val)
-			return false;
-
-		ucode_mem++;
-		/* 4 bytes */
-		ucode_size -= 4;
-	}
-
-	return true;
-}
-
 static int psp_v12_0_mode1_reset(struct psp_context *psp)
 {
 	int ret;
@@ -512,7 +390,6 @@ static const struct psp_funcs psp_v12_0_funcs = {
 	.ring_create = psp_v12_0_ring_create,
 	.ring_stop = psp_v12_0_ring_stop,
 	.ring_destroy = psp_v12_0_ring_destroy,
-	.compare_sram_data = psp_v12_0_compare_sram_data,
 	.mode1_reset = psp_v12_0_mode1_reset,
 	.ring_get_wptr = psp_v12_0_ring_get_wptr,
 	.ring_set_wptr = psp_v12_0_ring_set_wptr,
diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c b/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c
index ab03190..9ca37d0 100644
--- a/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c
+++ b/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c
@@ -398,128 +398,6 @@ static int psp_v3_1_ring_destroy(struct psp_context *psp,
 	return ret;
 }
 
-static int
-psp_v3_1_sram_map(struct amdgpu_device *adev,
-		  unsigned int *sram_offset, unsigned int *sram_addr_reg_offset,
-		  unsigned int *sram_data_reg_offset,
-		  enum AMDGPU_UCODE_ID ucode_id)
-{
-	int ret = 0;
-
-	switch(ucode_id) {
-/* TODO: needs to confirm */
-#if 0
-	case AMDGPU_UCODE_ID_SMC:
-		*sram_offset = 0;
-		*sram_addr_reg_offset = 0;
-		*sram_data_reg_offset = 0;
-		break;
-#endif
-
-	case AMDGPU_UCODE_ID_CP_CE:
-		*sram_offset = 0x0;
-		*sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_CE_UCODE_ADDR);
-		*sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_CE_UCODE_DATA);
-		break;
-
-	case AMDGPU_UCODE_ID_CP_PFP:
-		*sram_offset = 0x0;
-		*sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_PFP_UCODE_ADDR);
-		*sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_PFP_UCODE_DATA);
-		break;
-
-	case AMDGPU_UCODE_ID_CP_ME:
-		*sram_offset = 0x0;
-		*sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_HYP_ME_UCODE_ADDR);
-		*sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_HYP_ME_UCODE_DATA);
-		break;
-
-	case AMDGPU_UCODE_ID_CP_MEC1:
-		*sram_offset = 0x10000;
-		*sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_MEC_ME1_UCODE_ADDR);
-		*sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_MEC_ME1_UCODE_DATA);
-		break;
-
-	case AMDGPU_UCODE_ID_CP_MEC2:
-		*sram_offset = 0x10000;
-		*sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_HYP_MEC2_UCODE_ADDR);
-		*sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_HYP_MEC2_UCODE_DATA);
-		break;
-
-	case AMDGPU_UCODE_ID_RLC_G:
-		*sram_offset = 0x2000;
-		*sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmRLC_GPM_UCODE_ADDR);
-		*sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmRLC_GPM_UCODE_DATA);
-		break;
-
-	case AMDGPU_UCODE_ID_SDMA0:
-		*sram_offset = 0x0;
-		*sram_addr_reg_offset = SOC15_REG_OFFSET(SDMA0, 0, mmSDMA0_UCODE_ADDR);
-		*sram_data_reg_offset = SOC15_REG_OFFSET(SDMA0, 0, mmSDMA0_UCODE_DATA);
-		break;
-
-/* TODO: needs to confirm */
-#if 0
-	case AMDGPU_UCODE_ID_SDMA1:
-		*sram_offset = ;
-		*sram_addr_reg_offset = ;
-		break;
-
-	case AMDGPU_UCODE_ID_UVD:
-		*sram_offset = ;
-		*sram_addr_reg_offset = ;
-		break;
-
-	case AMDGPU_UCODE_ID_VCE:
-		*sram_offset = ;
-		*sram_addr_reg_offset = ;
-		break;
-#endif
-
-	case AMDGPU_UCODE_ID_MAXIMUM:
-	default:
-		ret = -EINVAL;
-		break;
-	}
-
-	return ret;
-}
-
-static bool psp_v3_1_compare_sram_data(struct psp_context *psp,
-				       struct amdgpu_firmware_info *ucode,
-				       enum AMDGPU_UCODE_ID ucode_type)
-{
-	int err = 0;
-	unsigned int fw_sram_reg_val = 0;
-	unsigned int fw_sram_addr_reg_offset = 0;
-	unsigned int fw_sram_data_reg_offset = 0;
-	unsigned int ucode_size;
-	uint32_t *ucode_mem = NULL;
-	struct amdgpu_device *adev = psp->adev;
-
-	err = psp_v3_1_sram_map(adev, &fw_sram_reg_val, &fw_sram_addr_reg_offset,
-				&fw_sram_data_reg_offset, ucode_type);
-	if (err)
-		return false;
-
-	WREG32(fw_sram_addr_reg_offset, fw_sram_reg_val);
-
-	ucode_size = ucode->ucode_size;
-	ucode_mem = (uint32_t *)ucode->kaddr;
-	while (ucode_size) {
-		fw_sram_reg_val = RREG32(fw_sram_data_reg_offset);
-
-		if (*ucode_mem != fw_sram_reg_val)
-			return false;
-
-		ucode_mem++;
-		/* 4 bytes */
-		ucode_size -= 4;
-	}
-
-	return true;
-}
-
 static bool psp_v3_1_smu_reload_quirk(struct psp_context *psp)
 {
 	struct amdgpu_device *adev = psp->adev;
@@ -596,7 +474,6 @@ static const struct psp_funcs psp_v3_1_funcs = {
 	.ring_create = psp_v3_1_ring_create,
 	.ring_stop = psp_v3_1_ring_stop,
 	.ring_destroy = psp_v3_1_ring_destroy,
-	.compare_sram_data = psp_v3_1_compare_sram_data,
 	.smu_reload_quirk = psp_v3_1_smu_reload_quirk,
 	.mode1_reset = psp_v3_1_mode1_reset,
 	.ring_get_wptr = psp_v3_1_ring_get_wptr,
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 3/8] drm/amdgpu: retire unused check_fw_loading status

[Luben Tuikov]

On 2020-04-20 6:16 a.m., Hawking Zhang wrote:
> It is not allowed to read out engine sram via registers
> to check the fw loading status.
> 

Who or what is "It"--do you mean "The driver"?

Abbreviations should be capitalized: SRAM, ASIC, etc.

Regards,
Luben

> Signed-off-by: Hawking Zhang <Hawking.Zhang@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c |  34 --------
>  drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h |   7 --
>  drivers/gpu/drm/amd/amdgpu/psp_v10_0.c  | 124 -----------------------------
>  drivers/gpu/drm/amd/amdgpu/psp_v11_0.c  | 133 --------------------------------
>  drivers/gpu/drm/amd/amdgpu/psp_v12_0.c  | 123 -----------------------------
>  drivers/gpu/drm/amd/amdgpu/psp_v3_1.c   | 123 -----------------------------
>  6 files changed, 544 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> index 901ee79..7797065 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> @@ -37,8 +37,6 @@
>  
>  #include "amdgpu_ras.h"
>  
> -static void psp_set_funcs(struct amdgpu_device *adev);
> -
>  static int psp_sysfs_init(struct amdgpu_device *adev);
>  static void psp_sysfs_fini(struct amdgpu_device *adev);
>  
> @@ -82,8 +80,6 @@ static int psp_early_init(void *handle)
>  	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
>  	struct psp_context *psp = &adev->psp;
>  
> -	psp_set_funcs(adev);
> -
>  	switch (adev->asic_type) {
>  	case CHIP_VEGA10:
>  	case CHIP_VEGA12:
> @@ -1487,11 +1483,6 @@ static int psp_np_fw_load(struct psp_context *psp)
>  				return ret;
>  			}
>  		}
> -#if 0
> -		/* check if firmware loaded sucessfully */
> -		if (!amdgpu_psp_check_fw_loading_status(adev, i))
> -			return -EINVAL;
> -#endif
>  	}
>  
>  	return 0;
> @@ -1849,21 +1840,6 @@ int psp_ring_cmd_submit(struct psp_context *psp,
>  	return 0;
>  }
>  
> -static bool psp_check_fw_loading_status(struct amdgpu_device *adev,
> -					enum AMDGPU_UCODE_ID ucode_type)
> -{
> -	struct amdgpu_firmware_info *ucode = NULL;
> -
> -	if (!adev->firmware.fw_size)
> -		return false;
> -
> -	ucode = &adev->firmware.ucode[ucode_type];
> -	if (!ucode->fw || !ucode->ucode_size)
> -		return false;
> -
> -	return psp_compare_sram_data(&adev->psp, ucode, ucode_type);
> -}
> -
>  static int psp_set_clockgating_state(void *handle,
>  				     enum amd_clockgating_state state)
>  {
> @@ -2000,16 +1976,6 @@ static void psp_sysfs_fini(struct amdgpu_device *adev)
>  	device_remove_file(adev->dev, &dev_attr_usbc_pd_fw);
>  }
>  
> -static const struct amdgpu_psp_funcs psp_funcs = {
> -	.check_fw_loading_status = psp_check_fw_loading_status,
> -};
> -
> -static void psp_set_funcs(struct amdgpu_device *adev)
> -{
> -	if (NULL == adev->firmware.funcs)
> -		adev->firmware.funcs = &psp_funcs;
> -}
> -
>  const struct amdgpu_ip_block_version psp_v3_1_ip_block =
>  {
>  	.type = AMD_IP_BLOCK_TYPE_PSP,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
> index 65a7d0a..f8b1f03 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
> @@ -93,9 +93,6 @@ struct psp_funcs
>  			    enum psp_ring_type ring_type);
>  	int (*ring_destroy)(struct psp_context *psp,
>  			    enum psp_ring_type ring_type);
> -	bool (*compare_sram_data)(struct psp_context *psp,
> -				  struct amdgpu_firmware_info *ucode,
> -				  enum AMDGPU_UCODE_ID ucode_type);
>  	bool (*smu_reload_quirk)(struct psp_context *psp);
>  	int (*mode1_reset)(struct psp_context *psp);
>  	int (*xgmi_get_node_id)(struct psp_context *psp, uint64_t *node_id);
> @@ -307,8 +304,6 @@ struct amdgpu_psp_funcs {
>  #define psp_ring_create(psp, type) (psp)->funcs->ring_create((psp), (type))
>  #define psp_ring_stop(psp, type) (psp)->funcs->ring_stop((psp), (type))
>  #define psp_ring_destroy(psp, type) ((psp)->funcs->ring_destroy((psp), (type)))
> -#define psp_compare_sram_data(psp, ucode, type) \
> -		(psp)->funcs->compare_sram_data((psp), (ucode), (type))
>  #define psp_init_microcode(psp) \
>  		((psp)->funcs->init_microcode ? (psp)->funcs->init_microcode((psp)) : 0)
>  #define psp_bootloader_load_kdb(psp) \
> @@ -340,8 +335,6 @@ struct amdgpu_psp_funcs {
>  #define psp_mem_training(psp, ops) \
>  	((psp)->funcs->mem_training ? (psp)->funcs->mem_training((psp), (ops)) : 0)
>  
> -#define amdgpu_psp_check_fw_loading_status(adev, i) (adev)->firmware.funcs->check_fw_loading_status((adev), (i))
> -
>  #define psp_ras_trigger_error(psp, info) \
>  	((psp)->funcs->ras_trigger_error ? \
>  	(psp)->funcs->ras_trigger_error((psp), (info)) : -EINVAL)
> diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c b/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
> index 7539104..6e041b7 100644
> --- a/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
> @@ -230,129 +230,6 @@ static int psp_v10_0_ring_destroy(struct psp_context *psp,
>  	return ret;
>  }
>  
> -static int
> -psp_v10_0_sram_map(struct amdgpu_device *adev,
> -		   unsigned int *sram_offset, unsigned int *sram_addr_reg_offset,
> -		   unsigned int *sram_data_reg_offset,
> -		   enum AMDGPU_UCODE_ID ucode_id)
> -{
> -	int ret = 0;
> -
> -	switch(ucode_id) {
> -/* TODO: needs to confirm */
> -#if 0
> -	case AMDGPU_UCODE_ID_SMC:
> -		*sram_offset = 0;
> -		*sram_addr_reg_offset = 0;
> -		*sram_data_reg_offset = 0;
> -		break;
> -#endif
> -
> -	case AMDGPU_UCODE_ID_CP_CE:
> -		*sram_offset = 0x0;
> -		*sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_CE_UCODE_ADDR);
> -		*sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_CE_UCODE_DATA);
> -		break;
> -
> -	case AMDGPU_UCODE_ID_CP_PFP:
> -		*sram_offset = 0x0;
> -		*sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_PFP_UCODE_ADDR);
> -		*sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_PFP_UCODE_DATA);
> -		break;
> -
> -	case AMDGPU_UCODE_ID_CP_ME:
> -		*sram_offset = 0x0;
> -		*sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_HYP_ME_UCODE_ADDR);
> -		*sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_HYP_ME_UCODE_DATA);
> -		break;
> -
> -	case AMDGPU_UCODE_ID_CP_MEC1:
> -		*sram_offset = 0x10000;
> -		*sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_MEC_ME1_UCODE_ADDR);
> -		*sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_MEC_ME1_UCODE_DATA);
> -		break;
> -
> -	case AMDGPU_UCODE_ID_CP_MEC2:
> -		*sram_offset = 0x10000;
> -		*sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_HYP_MEC2_UCODE_ADDR);
> -		*sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_HYP_MEC2_UCODE_DATA);
> -		break;
> -
> -	case AMDGPU_UCODE_ID_RLC_G:
> -		*sram_offset = 0x2000;
> -		*sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmRLC_GPM_UCODE_ADDR);
> -		*sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmRLC_GPM_UCODE_DATA);
> -		break;
> -
> -	case AMDGPU_UCODE_ID_SDMA0:
> -		*sram_offset = 0x0;
> -		*sram_addr_reg_offset = SOC15_REG_OFFSET(SDMA0, 0, mmSDMA0_UCODE_ADDR);
> -		*sram_data_reg_offset = SOC15_REG_OFFSET(SDMA0, 0, mmSDMA0_UCODE_DATA);
> -		break;
> -
> -/* TODO: needs to confirm */
> -#if 0
> -	case AMDGPU_UCODE_ID_SDMA1:
> -		*sram_offset = ;
> -		*sram_addr_reg_offset = ;
> -		break;
> -
> -	case AMDGPU_UCODE_ID_UVD:
> -		*sram_offset = ;
> -		*sram_addr_reg_offset = ;
> -		break;
> -
> -	case AMDGPU_UCODE_ID_VCE:
> -		*sram_offset = ;
> -		*sram_addr_reg_offset = ;
> -		break;
> -#endif
> -
> -	case AMDGPU_UCODE_ID_MAXIMUM:
> -	default:
> -		ret = -EINVAL;
> -		break;
> -	}
> -
> -	return ret;
> -}
> -
> -static bool psp_v10_0_compare_sram_data(struct psp_context *psp,
> -					struct amdgpu_firmware_info *ucode,
> -					enum AMDGPU_UCODE_ID ucode_type)
> -{
> -	int err = 0;
> -	unsigned int fw_sram_reg_val = 0;
> -	unsigned int fw_sram_addr_reg_offset = 0;
> -	unsigned int fw_sram_data_reg_offset = 0;
> -	unsigned int ucode_size;
> -	uint32_t *ucode_mem = NULL;
> -	struct amdgpu_device *adev = psp->adev;
> -
> -	err = psp_v10_0_sram_map(adev, &fw_sram_reg_val, &fw_sram_addr_reg_offset,
> -				&fw_sram_data_reg_offset, ucode_type);
> -	if (err)
> -		return false;
> -
> -	WREG32(fw_sram_addr_reg_offset, fw_sram_reg_val);
> -
> -	ucode_size = ucode->ucode_size;
> -	ucode_mem = (uint32_t *)ucode->kaddr;
> -	while (!ucode_size) {
> -		fw_sram_reg_val = RREG32(fw_sram_data_reg_offset);
> -
> -		if (*ucode_mem != fw_sram_reg_val)
> -			return false;
> -
> -		ucode_mem++;
> -		/* 4 bytes */
> -		ucode_size -= 4;
> -	}
> -
> -	return true;
> -}
> -
> -
>  static int psp_v10_0_mode1_reset(struct psp_context *psp)
>  {
>  	DRM_INFO("psp mode 1 reset not supported now! \n");
> @@ -379,7 +256,6 @@ static const struct psp_funcs psp_v10_0_funcs = {
>  	.ring_create = psp_v10_0_ring_create,
>  	.ring_stop = psp_v10_0_ring_stop,
>  	.ring_destroy = psp_v10_0_ring_destroy,
> -	.compare_sram_data = psp_v10_0_compare_sram_data,
>  	.mode1_reset = psp_v10_0_mode1_reset,
>  	.ring_get_wptr = psp_v10_0_ring_get_wptr,
>  	.ring_set_wptr = psp_v10_0_ring_set_wptr,
> diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
> index 20fbd43..f633577 100644
> --- a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
> @@ -554,138 +554,6 @@ static int psp_v11_0_ring_destroy(struct psp_context *psp,
>  	return ret;
>  }
>  
> -static int
> -psp_v11_0_sram_map(struct amdgpu_device *adev,
> -		  unsigned int *sram_offset, unsigned int *sram_addr_reg_offset,
> -		  unsigned int *sram_data_reg_offset,
> -		  enum AMDGPU_UCODE_ID ucode_id)
> -{
> -	int ret = 0;
> -
> -	switch (ucode_id) {
> -/* TODO: needs to confirm */
> -#if 0
> -	case AMDGPU_UCODE_ID_SMC:
> -		*sram_offset = 0;
> -		*sram_addr_reg_offset = 0;
> -		*sram_data_reg_offset = 0;
> -		break;
> -#endif
> -
> -	case AMDGPU_UCODE_ID_CP_CE:
> -		*sram_offset = 0x0;
> -		*sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_CE_UCODE_ADDR);
> -		*sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_CE_UCODE_DATA);
> -		break;
> -
> -	case AMDGPU_UCODE_ID_CP_PFP:
> -		*sram_offset = 0x0;
> -		*sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_PFP_UCODE_ADDR);
> -		*sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_PFP_UCODE_DATA);
> -		break;
> -
> -	case AMDGPU_UCODE_ID_CP_ME:
> -		*sram_offset = 0x0;
> -		*sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_HYP_ME_UCODE_ADDR);
> -		*sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_HYP_ME_UCODE_DATA);
> -		break;
> -
> -	case AMDGPU_UCODE_ID_CP_MEC1:
> -		*sram_offset = 0x10000;
> -		*sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_MEC_ME1_UCODE_ADDR);
> -		*sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_MEC_ME1_UCODE_DATA);
> -		break;
> -
> -	case AMDGPU_UCODE_ID_CP_MEC2:
> -		*sram_offset = 0x10000;
> -		*sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_HYP_MEC2_UCODE_ADDR);
> -		*sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_HYP_MEC2_UCODE_DATA);
> -		break;
> -
> -	case AMDGPU_UCODE_ID_RLC_G:
> -		*sram_offset = 0x2000;
> -		if (adev->asic_type < CHIP_NAVI10) {
> -			*sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmRLC_GPM_UCODE_ADDR);
> -			*sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmRLC_GPM_UCODE_DATA);
> -		} else {
> -			*sram_addr_reg_offset = adev->reg_offset[GC_HWIP][0][1] + mmRLC_GPM_UCODE_ADDR_NV10;
> -			*sram_data_reg_offset = adev->reg_offset[GC_HWIP][0][1] + mmRLC_GPM_UCODE_DATA_NV10;
> -		}
> -		break;
> -
> -	case AMDGPU_UCODE_ID_SDMA0:
> -		*sram_offset = 0x0;
> -		if (adev->asic_type < CHIP_NAVI10) {
> -			*sram_addr_reg_offset = SOC15_REG_OFFSET(SDMA0, 0, mmSDMA0_UCODE_ADDR);
> -			*sram_data_reg_offset = SOC15_REG_OFFSET(SDMA0, 0, mmSDMA0_UCODE_DATA);
> -		} else {
> -			*sram_addr_reg_offset = adev->reg_offset[GC_HWIP][0][1] + mmSDMA0_UCODE_ADDR_NV10;
> -			*sram_data_reg_offset = adev->reg_offset[GC_HWIP][0][1] + mmSDMA0_UCODE_DATA_NV10;
> -		}
> -		break;
> -
> -/* TODO: needs to confirm */
> -#if 0
> -	case AMDGPU_UCODE_ID_SDMA1:
> -		*sram_offset = ;
> -		*sram_addr_reg_offset = ;
> -		break;
> -
> -	case AMDGPU_UCODE_ID_UVD:
> -		*sram_offset = ;
> -		*sram_addr_reg_offset = ;
> -		break;
> -
> -	case AMDGPU_UCODE_ID_VCE:
> -		*sram_offset = ;
> -		*sram_addr_reg_offset = ;
> -		break;
> -#endif
> -
> -	case AMDGPU_UCODE_ID_MAXIMUM:
> -	default:
> -		ret = -EINVAL;
> -		break;
> -	}
> -
> -	return ret;
> -}
> -
> -static bool psp_v11_0_compare_sram_data(struct psp_context *psp,
> -				       struct amdgpu_firmware_info *ucode,
> -				       enum AMDGPU_UCODE_ID ucode_type)
> -{
> -	int err = 0;
> -	unsigned int fw_sram_reg_val = 0;
> -	unsigned int fw_sram_addr_reg_offset = 0;
> -	unsigned int fw_sram_data_reg_offset = 0;
> -	unsigned int ucode_size;
> -	uint32_t *ucode_mem = NULL;
> -	struct amdgpu_device *adev = psp->adev;
> -
> -	err = psp_v11_0_sram_map(adev, &fw_sram_reg_val, &fw_sram_addr_reg_offset,
> -				&fw_sram_data_reg_offset, ucode_type);
> -	if (err)
> -		return false;
> -
> -	WREG32(fw_sram_addr_reg_offset, fw_sram_reg_val);
> -
> -	ucode_size = ucode->ucode_size;
> -	ucode_mem = (uint32_t *)ucode->kaddr;
> -	while (ucode_size) {
> -		fw_sram_reg_val = RREG32(fw_sram_data_reg_offset);
> -
> -		if (*ucode_mem != fw_sram_reg_val)
> -			return false;
> -
> -		ucode_mem++;
> -		/* 4 bytes */
> -		ucode_size -= 4;
> -	}
> -
> -	return true;
> -}
> -
>  static int psp_v11_0_mode1_reset(struct psp_context *psp)
>  {
>  	int ret;
> @@ -1190,7 +1058,6 @@ static const struct psp_funcs psp_v11_0_funcs = {
>  	.ring_create = psp_v11_0_ring_create,
>  	.ring_stop = psp_v11_0_ring_stop,
>  	.ring_destroy = psp_v11_0_ring_destroy,
> -	.compare_sram_data = psp_v11_0_compare_sram_data,
>  	.mode1_reset = psp_v11_0_mode1_reset,
>  	.xgmi_get_topology_info = psp_v11_0_xgmi_get_topology_info,
>  	.xgmi_set_topology_info = psp_v11_0_xgmi_set_topology_info,
> diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v12_0.c b/drivers/gpu/drm/amd/amdgpu/psp_v12_0.c
> index d3c86a0..42c485b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/psp_v12_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/psp_v12_0.c
> @@ -324,128 +324,6 @@ static int psp_v12_0_ring_destroy(struct psp_context *psp,
>  	return ret;
>  }
>  
> -static int
> -psp_v12_0_sram_map(struct amdgpu_device *adev,
> -		  unsigned int *sram_offset, unsigned int *sram_addr_reg_offset,
> -		  unsigned int *sram_data_reg_offset,
> -		  enum AMDGPU_UCODE_ID ucode_id)
> -{
> -	int ret = 0;
> -
> -	switch (ucode_id) {
> -/* TODO: needs to confirm */
> -#if 0
> -	case AMDGPU_UCODE_ID_SMC:
> -		*sram_offset = 0;
> -		*sram_addr_reg_offset = 0;
> -		*sram_data_reg_offset = 0;
> -		break;
> -#endif
> -
> -	case AMDGPU_UCODE_ID_CP_CE:
> -		*sram_offset = 0x0;
> -		*sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_CE_UCODE_ADDR);
> -		*sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_CE_UCODE_DATA);
> -		break;
> -
> -	case AMDGPU_UCODE_ID_CP_PFP:
> -		*sram_offset = 0x0;
> -		*sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_PFP_UCODE_ADDR);
> -		*sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_PFP_UCODE_DATA);
> -		break;
> -
> -	case AMDGPU_UCODE_ID_CP_ME:
> -		*sram_offset = 0x0;
> -		*sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_HYP_ME_UCODE_ADDR);
> -		*sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_HYP_ME_UCODE_DATA);
> -		break;
> -
> -	case AMDGPU_UCODE_ID_CP_MEC1:
> -		*sram_offset = 0x10000;
> -		*sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_MEC_ME1_UCODE_ADDR);
> -		*sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_MEC_ME1_UCODE_DATA);
> -		break;
> -
> -	case AMDGPU_UCODE_ID_CP_MEC2:
> -		*sram_offset = 0x10000;
> -		*sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_HYP_MEC2_UCODE_ADDR);
> -		*sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_HYP_MEC2_UCODE_DATA);
> -		break;
> -
> -	case AMDGPU_UCODE_ID_RLC_G:
> -		*sram_offset = 0x2000;
> -		*sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmRLC_GPM_UCODE_ADDR);
> -		*sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmRLC_GPM_UCODE_DATA);
> -		break;
> -
> -	case AMDGPU_UCODE_ID_SDMA0:
> -		*sram_offset = 0x0;
> -		*sram_addr_reg_offset = SOC15_REG_OFFSET(SDMA0, 0, mmSDMA0_UCODE_ADDR);
> -		*sram_data_reg_offset = SOC15_REG_OFFSET(SDMA0, 0, mmSDMA0_UCODE_DATA);
> -		break;
> -
> -/* TODO: needs to confirm */
> -#if 0
> -	case AMDGPU_UCODE_ID_SDMA1:
> -		*sram_offset = ;
> -		*sram_addr_reg_offset = ;
> -		break;
> -
> -	case AMDGPU_UCODE_ID_UVD:
> -		*sram_offset = ;
> -		*sram_addr_reg_offset = ;
> -		break;
> -
> -	case AMDGPU_UCODE_ID_VCE:
> -		*sram_offset = ;
> -		*sram_addr_reg_offset = ;
> -		break;
> -#endif
> -
> -	case AMDGPU_UCODE_ID_MAXIMUM:
> -	default:
> -		ret = -EINVAL;
> -		break;
> -	}
> -
> -	return ret;
> -}
> -
> -static bool psp_v12_0_compare_sram_data(struct psp_context *psp,
> -				       struct amdgpu_firmware_info *ucode,
> -				       enum AMDGPU_UCODE_ID ucode_type)
> -{
> -	int err = 0;
> -	unsigned int fw_sram_reg_val = 0;
> -	unsigned int fw_sram_addr_reg_offset = 0;
> -	unsigned int fw_sram_data_reg_offset = 0;
> -	unsigned int ucode_size;
> -	uint32_t *ucode_mem = NULL;
> -	struct amdgpu_device *adev = psp->adev;
> -
> -	err = psp_v12_0_sram_map(adev, &fw_sram_reg_val, &fw_sram_addr_reg_offset,
> -				&fw_sram_data_reg_offset, ucode_type);
> -	if (err)
> -		return false;
> -
> -	WREG32(fw_sram_addr_reg_offset, fw_sram_reg_val);
> -
> -	ucode_size = ucode->ucode_size;
> -	ucode_mem = (uint32_t *)ucode->kaddr;
> -	while (ucode_size) {
> -		fw_sram_reg_val = RREG32(fw_sram_data_reg_offset);
> -
> -		if (*ucode_mem != fw_sram_reg_val)
> -			return false;
> -
> -		ucode_mem++;
> -		/* 4 bytes */
> -		ucode_size -= 4;
> -	}
> -
> -	return true;
> -}
> -
>  static int psp_v12_0_mode1_reset(struct psp_context *psp)
>  {
>  	int ret;
> @@ -512,7 +390,6 @@ static const struct psp_funcs psp_v12_0_funcs = {
>  	.ring_create = psp_v12_0_ring_create,
>  	.ring_stop = psp_v12_0_ring_stop,
>  	.ring_destroy = psp_v12_0_ring_destroy,
> -	.compare_sram_data = psp_v12_0_compare_sram_data,
>  	.mode1_reset = psp_v12_0_mode1_reset,
>  	.ring_get_wptr = psp_v12_0_ring_get_wptr,
>  	.ring_set_wptr = psp_v12_0_ring_set_wptr,
> diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c b/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c
> index ab03190..9ca37d0 100644
> --- a/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c
> +++ b/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c
> @@ -398,128 +398,6 @@ static int psp_v3_1_ring_destroy(struct psp_context *psp,
>  	return ret;
>  }
>  
> -static int
> -psp_v3_1_sram_map(struct amdgpu_device *adev,
> -		  unsigned int *sram_offset, unsigned int *sram_addr_reg_offset,
> -		  unsigned int *sram_data_reg_offset,
> -		  enum AMDGPU_UCODE_ID ucode_id)
> -{
> -	int ret = 0;
> -
> -	switch(ucode_id) {
> -/* TODO: needs to confirm */
> -#if 0
> -	case AMDGPU_UCODE_ID_SMC:
> -		*sram_offset = 0;
> -		*sram_addr_reg_offset = 0;
> -		*sram_data_reg_offset = 0;
> -		break;
> -#endif
> -
> -	case AMDGPU_UCODE_ID_CP_CE:
> -		*sram_offset = 0x0;
> -		*sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_CE_UCODE_ADDR);
> -		*sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_CE_UCODE_DATA);
> -		break;
> -
> -	case AMDGPU_UCODE_ID_CP_PFP:
> -		*sram_offset = 0x0;
> -		*sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_PFP_UCODE_ADDR);
> -		*sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_PFP_UCODE_DATA);
> -		break;
> -
> -	case AMDGPU_UCODE_ID_CP_ME:
> -		*sram_offset = 0x0;
> -		*sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_HYP_ME_UCODE_ADDR);
> -		*sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_HYP_ME_UCODE_DATA);
> -		break;
> -
> -	case AMDGPU_UCODE_ID_CP_MEC1:
> -		*sram_offset = 0x10000;
> -		*sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_MEC_ME1_UCODE_ADDR);
> -		*sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_MEC_ME1_UCODE_DATA);
> -		break;
> -
> -	case AMDGPU_UCODE_ID_CP_MEC2:
> -		*sram_offset = 0x10000;
> -		*sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_HYP_MEC2_UCODE_ADDR);
> -		*sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_HYP_MEC2_UCODE_DATA);
> -		break;
> -
> -	case AMDGPU_UCODE_ID_RLC_G:
> -		*sram_offset = 0x2000;
> -		*sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmRLC_GPM_UCODE_ADDR);
> -		*sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmRLC_GPM_UCODE_DATA);
> -		break;
> -
> -	case AMDGPU_UCODE_ID_SDMA0:
> -		*sram_offset = 0x0;
> -		*sram_addr_reg_offset = SOC15_REG_OFFSET(SDMA0, 0, mmSDMA0_UCODE_ADDR);
> -		*sram_data_reg_offset = SOC15_REG_OFFSET(SDMA0, 0, mmSDMA0_UCODE_DATA);
> -		break;
> -
> -/* TODO: needs to confirm */
> -#if 0
> -	case AMDGPU_UCODE_ID_SDMA1:
> -		*sram_offset = ;
> -		*sram_addr_reg_offset = ;
> -		break;
> -
> -	case AMDGPU_UCODE_ID_UVD:
> -		*sram_offset = ;
> -		*sram_addr_reg_offset = ;
> -		break;
> -
> -	case AMDGPU_UCODE_ID_VCE:
> -		*sram_offset = ;
> -		*sram_addr_reg_offset = ;
> -		break;
> -#endif
> -
> -	case AMDGPU_UCODE_ID_MAXIMUM:
> -	default:
> -		ret = -EINVAL;
> -		break;
> -	}
> -
> -	return ret;
> -}
> -
> -static bool psp_v3_1_compare_sram_data(struct psp_context *psp,
> -				       struct amdgpu_firmware_info *ucode,
> -				       enum AMDGPU_UCODE_ID ucode_type)
> -{
> -	int err = 0;
> -	unsigned int fw_sram_reg_val = 0;
> -	unsigned int fw_sram_addr_reg_offset = 0;
> -	unsigned int fw_sram_data_reg_offset = 0;
> -	unsigned int ucode_size;
> -	uint32_t *ucode_mem = NULL;
> -	struct amdgpu_device *adev = psp->adev;
> -
> -	err = psp_v3_1_sram_map(adev, &fw_sram_reg_val, &fw_sram_addr_reg_offset,
> -				&fw_sram_data_reg_offset, ucode_type);
> -	if (err)
> -		return false;
> -
> -	WREG32(fw_sram_addr_reg_offset, fw_sram_reg_val);
> -
> -	ucode_size = ucode->ucode_size;
> -	ucode_mem = (uint32_t *)ucode->kaddr;
> -	while (ucode_size) {
> -		fw_sram_reg_val = RREG32(fw_sram_data_reg_offset);
> -
> -		if (*ucode_mem != fw_sram_reg_val)
> -			return false;
> -
> -		ucode_mem++;
> -		/* 4 bytes */
> -		ucode_size -= 4;
> -	}
> -
> -	return true;
> -}
> -
>  static bool psp_v3_1_smu_reload_quirk(struct psp_context *psp)
>  {
>  	struct amdgpu_device *adev = psp->adev;
> @@ -596,7 +474,6 @@ static const struct psp_funcs psp_v3_1_funcs = {
>  	.ring_create = psp_v3_1_ring_create,
>  	.ring_stop = psp_v3_1_ring_stop,
>  	.ring_destroy = psp_v3_1_ring_destroy,
> -	.compare_sram_data = psp_v3_1_compare_sram_data,
>  	.smu_reload_quirk = psp_v3_1_smu_reload_quirk,
>  	.mode1_reset = psp_v3_1_mode1_reset,
>  	.ring_get_wptr = psp_v3_1_ring_get_wptr,
> 

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 3/8] drm/amdgpu: retire unused check_fw_loading status

[Alex Deucher]

On Mon, Apr 20, 2020 at 10:45 AM Luben Tuikov <luben.tuikov@amd.com> wrote:
>
> On 2020-04-20 6:16 a.m., Hawking Zhang wrote:
> > It is not allowed to read out engine sram via registers
> > to check the fw loading status.
> >
>
> Who or what is "It"--do you mean "The driver"?
>

The driver can't access these registers on production boards.  These
are leftover from bring up.  Adding a sentence like that should help
clarify.

Alex

> Abbreviations should be capitalized: SRAM, ASIC, etc.
>
> Regards,
> Luben
>
> > Signed-off-by: Hawking Zhang <Hawking.Zhang@amd.com>
> > ---
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c |  34 --------
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h |   7 --
> >  drivers/gpu/drm/amd/amdgpu/psp_v10_0.c  | 124 -----------------------------
> >  drivers/gpu/drm/amd/amdgpu/psp_v11_0.c  | 133 --------------------------------
> >  drivers/gpu/drm/amd/amdgpu/psp_v12_0.c  | 123 -----------------------------
> >  drivers/gpu/drm/amd/amdgpu/psp_v3_1.c   | 123 -----------------------------
> >  6 files changed, 544 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> > index 901ee79..7797065 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> > @@ -37,8 +37,6 @@
> >
> >  #include "amdgpu_ras.h"
> >
> > -static void psp_set_funcs(struct amdgpu_device *adev);
> > -
> >  static int psp_sysfs_init(struct amdgpu_device *adev);
> >  static void psp_sysfs_fini(struct amdgpu_device *adev);
> >
> > @@ -82,8 +80,6 @@ static int psp_early_init(void *handle)
> >       struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> >       struct psp_context *psp = &adev->psp;
> >
> > -     psp_set_funcs(adev);
> > -
> >       switch (adev->asic_type) {
> >       case CHIP_VEGA10:
> >       case CHIP_VEGA12:
> > @@ -1487,11 +1483,6 @@ static int psp_np_fw_load(struct psp_context *psp)
> >                               return ret;
> >                       }
> >               }
> > -#if 0
> > -             /* check if firmware loaded sucessfully */
> > -             if (!amdgpu_psp_check_fw_loading_status(adev, i))
> > -                     return -EINVAL;
> > -#endif
> >       }
> >
> >       return 0;
> > @@ -1849,21 +1840,6 @@ int psp_ring_cmd_submit(struct psp_context *psp,
> >       return 0;
> >  }
> >
> > -static bool psp_check_fw_loading_status(struct amdgpu_device *adev,
> > -                                     enum AMDGPU_UCODE_ID ucode_type)
> > -{
> > -     struct amdgpu_firmware_info *ucode = NULL;
> > -
> > -     if (!adev->firmware.fw_size)
> > -             return false;
> > -
> > -     ucode = &adev->firmware.ucode[ucode_type];
> > -     if (!ucode->fw || !ucode->ucode_size)
> > -             return false;
> > -
> > -     return psp_compare_sram_data(&adev->psp, ucode, ucode_type);
> > -}
> > -
> >  static int psp_set_clockgating_state(void *handle,
> >                                    enum amd_clockgating_state state)
> >  {
> > @@ -2000,16 +1976,6 @@ static void psp_sysfs_fini(struct amdgpu_device *adev)
> >       device_remove_file(adev->dev, &dev_attr_usbc_pd_fw);
> >  }
> >
> > -static const struct amdgpu_psp_funcs psp_funcs = {
> > -     .check_fw_loading_status = psp_check_fw_loading_status,
> > -};
> > -
> > -static void psp_set_funcs(struct amdgpu_device *adev)
> > -{
> > -     if (NULL == adev->firmware.funcs)
> > -             adev->firmware.funcs = &psp_funcs;
> > -}
> > -
> >  const struct amdgpu_ip_block_version psp_v3_1_ip_block =
> >  {
> >       .type = AMD_IP_BLOCK_TYPE_PSP,
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
> > index 65a7d0a..f8b1f03 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
> > @@ -93,9 +93,6 @@ struct psp_funcs
> >                           enum psp_ring_type ring_type);
> >       int (*ring_destroy)(struct psp_context *psp,
> >                           enum psp_ring_type ring_type);
> > -     bool (*compare_sram_data)(struct psp_context *psp,
> > -                               struct amdgpu_firmware_info *ucode,
> > -                               enum AMDGPU_UCODE_ID ucode_type);
> >       bool (*smu_reload_quirk)(struct psp_context *psp);
> >       int (*mode1_reset)(struct psp_context *psp);
> >       int (*xgmi_get_node_id)(struct psp_context *psp, uint64_t *node_id);
> > @@ -307,8 +304,6 @@ struct amdgpu_psp_funcs {
> >  #define psp_ring_create(psp, type) (psp)->funcs->ring_create((psp), (type))
> >  #define psp_ring_stop(psp, type) (psp)->funcs->ring_stop((psp), (type))
> >  #define psp_ring_destroy(psp, type) ((psp)->funcs->ring_destroy((psp), (type)))
> > -#define psp_compare_sram_data(psp, ucode, type) \
> > -             (psp)->funcs->compare_sram_data((psp), (ucode), (type))
> >  #define psp_init_microcode(psp) \
> >               ((psp)->funcs->init_microcode ? (psp)->funcs->init_microcode((psp)) : 0)
> >  #define psp_bootloader_load_kdb(psp) \
> > @@ -340,8 +335,6 @@ struct amdgpu_psp_funcs {
> >  #define psp_mem_training(psp, ops) \
> >       ((psp)->funcs->mem_training ? (psp)->funcs->mem_training((psp), (ops)) : 0)
> >
> > -#define amdgpu_psp_check_fw_loading_status(adev, i) (adev)->firmware.funcs->check_fw_loading_status((adev), (i))
> > -
> >  #define psp_ras_trigger_error(psp, info) \
> >       ((psp)->funcs->ras_trigger_error ? \
> >       (psp)->funcs->ras_trigger_error((psp), (info)) : -EINVAL)
> > diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c b/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
> > index 7539104..6e041b7 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
> > @@ -230,129 +230,6 @@ static int psp_v10_0_ring_destroy(struct psp_context *psp,
> >       return ret;
> >  }
> >
> > -static int
> > -psp_v10_0_sram_map(struct amdgpu_device *adev,
> > -                unsigned int *sram_offset, unsigned int *sram_addr_reg_offset,
> > -                unsigned int *sram_data_reg_offset,
> > -                enum AMDGPU_UCODE_ID ucode_id)
> > -{
> > -     int ret = 0;
> > -
> > -     switch(ucode_id) {
> > -/* TODO: needs to confirm */
> > -#if 0
> > -     case AMDGPU_UCODE_ID_SMC:
> > -             *sram_offset = 0;
> > -             *sram_addr_reg_offset = 0;
> > -             *sram_data_reg_offset = 0;
> > -             break;
> > -#endif
> > -
> > -     case AMDGPU_UCODE_ID_CP_CE:
> > -             *sram_offset = 0x0;
> > -             *sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_CE_UCODE_ADDR);
> > -             *sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_CE_UCODE_DATA);
> > -             break;
> > -
> > -     case AMDGPU_UCODE_ID_CP_PFP:
> > -             *sram_offset = 0x0;
> > -             *sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_PFP_UCODE_ADDR);
> > -             *sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_PFP_UCODE_DATA);
> > -             break;
> > -
> > -     case AMDGPU_UCODE_ID_CP_ME:
> > -             *sram_offset = 0x0;
> > -             *sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_HYP_ME_UCODE_ADDR);
> > -             *sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_HYP_ME_UCODE_DATA);
> > -             break;
> > -
> > -     case AMDGPU_UCODE_ID_CP_MEC1:
> > -             *sram_offset = 0x10000;
> > -             *sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_MEC_ME1_UCODE_ADDR);
> > -             *sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_MEC_ME1_UCODE_DATA);
> > -             break;
> > -
> > -     case AMDGPU_UCODE_ID_CP_MEC2:
> > -             *sram_offset = 0x10000;
> > -             *sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_HYP_MEC2_UCODE_ADDR);
> > -             *sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_HYP_MEC2_UCODE_DATA);
> > -             break;
> > -
> > -     case AMDGPU_UCODE_ID_RLC_G:
> > -             *sram_offset = 0x2000;
> > -             *sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmRLC_GPM_UCODE_ADDR);
> > -             *sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmRLC_GPM_UCODE_DATA);
> > -             break;
> > -
> > -     case AMDGPU_UCODE_ID_SDMA0:
> > -             *sram_offset = 0x0;
> > -             *sram_addr_reg_offset = SOC15_REG_OFFSET(SDMA0, 0, mmSDMA0_UCODE_ADDR);
> > -             *sram_data_reg_offset = SOC15_REG_OFFSET(SDMA0, 0, mmSDMA0_UCODE_DATA);
> > -             break;
> > -
> > -/* TODO: needs to confirm */
> > -#if 0
> > -     case AMDGPU_UCODE_ID_SDMA1:
> > -             *sram_offset = ;
> > -             *sram_addr_reg_offset = ;
> > -             break;
> > -
> > -     case AMDGPU_UCODE_ID_UVD:
> > -             *sram_offset = ;
> > -             *sram_addr_reg_offset = ;
> > -             break;
> > -
> > -     case AMDGPU_UCODE_ID_VCE:
> > -             *sram_offset = ;
> > -             *sram_addr_reg_offset = ;
> > -             break;
> > -#endif
> > -
> > -     case AMDGPU_UCODE_ID_MAXIMUM:
> > -     default:
> > -             ret = -EINVAL;
> > -             break;
> > -     }
> > -
> > -     return ret;
> > -}
> > -
> > -static bool psp_v10_0_compare_sram_data(struct psp_context *psp,
> > -                                     struct amdgpu_firmware_info *ucode,
> > -                                     enum AMDGPU_UCODE_ID ucode_type)
> > -{
> > -     int err = 0;
> > -     unsigned int fw_sram_reg_val = 0;
> > -     unsigned int fw_sram_addr_reg_offset = 0;
> > -     unsigned int fw_sram_data_reg_offset = 0;
> > -     unsigned int ucode_size;
> > -     uint32_t *ucode_mem = NULL;
> > -     struct amdgpu_device *adev = psp->adev;
> > -
> > -     err = psp_v10_0_sram_map(adev, &fw_sram_reg_val, &fw_sram_addr_reg_offset,
> > -                             &fw_sram_data_reg_offset, ucode_type);
> > -     if (err)
> > -             return false;
> > -
> > -     WREG32(fw_sram_addr_reg_offset, fw_sram_reg_val);
> > -
> > -     ucode_size = ucode->ucode_size;
> > -     ucode_mem = (uint32_t *)ucode->kaddr;
> > -     while (!ucode_size) {
> > -             fw_sram_reg_val = RREG32(fw_sram_data_reg_offset);
> > -
> > -             if (*ucode_mem != fw_sram_reg_val)
> > -                     return false;
> > -
> > -             ucode_mem++;
> > -             /* 4 bytes */
> > -             ucode_size -= 4;
> > -     }
> > -
> > -     return true;
> > -}
> > -
> > -
> >  static int psp_v10_0_mode1_reset(struct psp_context *psp)
> >  {
> >       DRM_INFO("psp mode 1 reset not supported now! \n");
> > @@ -379,7 +256,6 @@ static const struct psp_funcs psp_v10_0_funcs = {
> >       .ring_create = psp_v10_0_ring_create,
> >       .ring_stop = psp_v10_0_ring_stop,
> >       .ring_destroy = psp_v10_0_ring_destroy,
> > -     .compare_sram_data = psp_v10_0_compare_sram_data,
> >       .mode1_reset = psp_v10_0_mode1_reset,
> >       .ring_get_wptr = psp_v10_0_ring_get_wptr,
> >       .ring_set_wptr = psp_v10_0_ring_set_wptr,
> > diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
> > index 20fbd43..f633577 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
> > @@ -554,138 +554,6 @@ static int psp_v11_0_ring_destroy(struct psp_context *psp,
> >       return ret;
> >  }
> >
> > -static int
> > -psp_v11_0_sram_map(struct amdgpu_device *adev,
> > -               unsigned int *sram_offset, unsigned int *sram_addr_reg_offset,
> > -               unsigned int *sram_data_reg_offset,
> > -               enum AMDGPU_UCODE_ID ucode_id)
> > -{
> > -     int ret = 0;
> > -
> > -     switch (ucode_id) {
> > -/* TODO: needs to confirm */
> > -#if 0
> > -     case AMDGPU_UCODE_ID_SMC:
> > -             *sram_offset = 0;
> > -             *sram_addr_reg_offset = 0;
> > -             *sram_data_reg_offset = 0;
> > -             break;
> > -#endif
> > -
> > -     case AMDGPU_UCODE_ID_CP_CE:
> > -             *sram_offset = 0x0;
> > -             *sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_CE_UCODE_ADDR);
> > -             *sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_CE_UCODE_DATA);
> > -             break;
> > -
> > -     case AMDGPU_UCODE_ID_CP_PFP:
> > -             *sram_offset = 0x0;
> > -             *sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_PFP_UCODE_ADDR);
> > -             *sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_PFP_UCODE_DATA);
> > -             break;
> > -
> > -     case AMDGPU_UCODE_ID_CP_ME:
> > -             *sram_offset = 0x0;
> > -             *sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_HYP_ME_UCODE_ADDR);
> > -             *sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_HYP_ME_UCODE_DATA);
> > -             break;
> > -
> > -     case AMDGPU_UCODE_ID_CP_MEC1:
> > -             *sram_offset = 0x10000;
> > -             *sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_MEC_ME1_UCODE_ADDR);
> > -             *sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_MEC_ME1_UCODE_DATA);
> > -             break;
> > -
> > -     case AMDGPU_UCODE_ID_CP_MEC2:
> > -             *sram_offset = 0x10000;
> > -             *sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_HYP_MEC2_UCODE_ADDR);
> > -             *sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_HYP_MEC2_UCODE_DATA);
> > -             break;
> > -
> > -     case AMDGPU_UCODE_ID_RLC_G:
> > -             *sram_offset = 0x2000;
> > -             if (adev->asic_type < CHIP_NAVI10) {
> > -                     *sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmRLC_GPM_UCODE_ADDR);
> > -                     *sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmRLC_GPM_UCODE_DATA);
> > -             } else {
> > -                     *sram_addr_reg_offset = adev->reg_offset[GC_HWIP][0][1] + mmRLC_GPM_UCODE_ADDR_NV10;
> > -                     *sram_data_reg_offset = adev->reg_offset[GC_HWIP][0][1] + mmRLC_GPM_UCODE_DATA_NV10;
> > -             }
> > -             break;
> > -
> > -     case AMDGPU_UCODE_ID_SDMA0:
> > -             *sram_offset = 0x0;
> > -             if (adev->asic_type < CHIP_NAVI10) {
> > -                     *sram_addr_reg_offset = SOC15_REG_OFFSET(SDMA0, 0, mmSDMA0_UCODE_ADDR);
> > -                     *sram_data_reg_offset = SOC15_REG_OFFSET(SDMA0, 0, mmSDMA0_UCODE_DATA);
> > -             } else {
> > -                     *sram_addr_reg_offset = adev->reg_offset[GC_HWIP][0][1] + mmSDMA0_UCODE_ADDR_NV10;
> > -                     *sram_data_reg_offset = adev->reg_offset[GC_HWIP][0][1] + mmSDMA0_UCODE_DATA_NV10;
> > -             }
> > -             break;
> > -
> > -/* TODO: needs to confirm */
> > -#if 0
> > -     case AMDGPU_UCODE_ID_SDMA1:
> > -             *sram_offset = ;
> > -             *sram_addr_reg_offset = ;
> > -             break;
> > -
> > -     case AMDGPU_UCODE_ID_UVD:
> > -             *sram_offset = ;
> > -             *sram_addr_reg_offset = ;
> > -             break;
> > -
> > -     case AMDGPU_UCODE_ID_VCE:
> > -             *sram_offset = ;
> > -             *sram_addr_reg_offset = ;
> > -             break;
> > -#endif
> > -
> > -     case AMDGPU_UCODE_ID_MAXIMUM:
> > -     default:
> > -             ret = -EINVAL;
> > -             break;
> > -     }
> > -
> > -     return ret;
> > -}
> > -
> > -static bool psp_v11_0_compare_sram_data(struct psp_context *psp,
> > -                                    struct amdgpu_firmware_info *ucode,
> > -                                    enum AMDGPU_UCODE_ID ucode_type)
> > -{
> > -     int err = 0;
> > -     unsigned int fw_sram_reg_val = 0;
> > -     unsigned int fw_sram_addr_reg_offset = 0;
> > -     unsigned int fw_sram_data_reg_offset = 0;
> > -     unsigned int ucode_size;
> > -     uint32_t *ucode_mem = NULL;
> > -     struct amdgpu_device *adev = psp->adev;
> > -
> > -     err = psp_v11_0_sram_map(adev, &fw_sram_reg_val, &fw_sram_addr_reg_offset,
> > -                             &fw_sram_data_reg_offset, ucode_type);
> > -     if (err)
> > -             return false;
> > -
> > -     WREG32(fw_sram_addr_reg_offset, fw_sram_reg_val);
> > -
> > -     ucode_size = ucode->ucode_size;
> > -     ucode_mem = (uint32_t *)ucode->kaddr;
> > -     while (ucode_size) {
> > -             fw_sram_reg_val = RREG32(fw_sram_data_reg_offset);
> > -
> > -             if (*ucode_mem != fw_sram_reg_val)
> > -                     return false;
> > -
> > -             ucode_mem++;
> > -             /* 4 bytes */
> > -             ucode_size -= 4;
> > -     }
> > -
> > -     return true;
> > -}
> > -
> >  static int psp_v11_0_mode1_reset(struct psp_context *psp)
> >  {
> >       int ret;
> > @@ -1190,7 +1058,6 @@ static const struct psp_funcs psp_v11_0_funcs = {
> >       .ring_create = psp_v11_0_ring_create,
> >       .ring_stop = psp_v11_0_ring_stop,
> >       .ring_destroy = psp_v11_0_ring_destroy,
> > -     .compare_sram_data = psp_v11_0_compare_sram_data,
> >       .mode1_reset = psp_v11_0_mode1_reset,
> >       .xgmi_get_topology_info = psp_v11_0_xgmi_get_topology_info,
> >       .xgmi_set_topology_info = psp_v11_0_xgmi_set_topology_info,
> > diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v12_0.c b/drivers/gpu/drm/amd/amdgpu/psp_v12_0.c
> > index d3c86a0..42c485b 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/psp_v12_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/psp_v12_0.c
> > @@ -324,128 +324,6 @@ static int psp_v12_0_ring_destroy(struct psp_context *psp,
> >       return ret;
> >  }
> >
> > -static int
> > -psp_v12_0_sram_map(struct amdgpu_device *adev,
> > -               unsigned int *sram_offset, unsigned int *sram_addr_reg_offset,
> > -               unsigned int *sram_data_reg_offset,
> > -               enum AMDGPU_UCODE_ID ucode_id)
> > -{
> > -     int ret = 0;
> > -
> > -     switch (ucode_id) {
> > -/* TODO: needs to confirm */
> > -#if 0
> > -     case AMDGPU_UCODE_ID_SMC:
> > -             *sram_offset = 0;
> > -             *sram_addr_reg_offset = 0;
> > -             *sram_data_reg_offset = 0;
> > -             break;
> > -#endif
> > -
> > -     case AMDGPU_UCODE_ID_CP_CE:
> > -             *sram_offset = 0x0;
> > -             *sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_CE_UCODE_ADDR);
> > -             *sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_CE_UCODE_DATA);
> > -             break;
> > -
> > -     case AMDGPU_UCODE_ID_CP_PFP:
> > -             *sram_offset = 0x0;
> > -             *sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_PFP_UCODE_ADDR);
> > -             *sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_PFP_UCODE_DATA);
> > -             break;
> > -
> > -     case AMDGPU_UCODE_ID_CP_ME:
> > -             *sram_offset = 0x0;
> > -             *sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_HYP_ME_UCODE_ADDR);
> > -             *sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_HYP_ME_UCODE_DATA);
> > -             break;
> > -
> > -     case AMDGPU_UCODE_ID_CP_MEC1:
> > -             *sram_offset = 0x10000;
> > -             *sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_MEC_ME1_UCODE_ADDR);
> > -             *sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_MEC_ME1_UCODE_DATA);
> > -             break;
> > -
> > -     case AMDGPU_UCODE_ID_CP_MEC2:
> > -             *sram_offset = 0x10000;
> > -             *sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_HYP_MEC2_UCODE_ADDR);
> > -             *sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_HYP_MEC2_UCODE_DATA);
> > -             break;
> > -
> > -     case AMDGPU_UCODE_ID_RLC_G:
> > -             *sram_offset = 0x2000;
> > -             *sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmRLC_GPM_UCODE_ADDR);
> > -             *sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmRLC_GPM_UCODE_DATA);
> > -             break;
> > -
> > -     case AMDGPU_UCODE_ID_SDMA0:
> > -             *sram_offset = 0x0;
> > -             *sram_addr_reg_offset = SOC15_REG_OFFSET(SDMA0, 0, mmSDMA0_UCODE_ADDR);
> > -             *sram_data_reg_offset = SOC15_REG_OFFSET(SDMA0, 0, mmSDMA0_UCODE_DATA);
> > -             break;
> > -
> > -/* TODO: needs to confirm */
> > -#if 0
> > -     case AMDGPU_UCODE_ID_SDMA1:
> > -             *sram_offset = ;
> > -             *sram_addr_reg_offset = ;
> > -             break;
> > -
> > -     case AMDGPU_UCODE_ID_UVD:
> > -             *sram_offset = ;
> > -             *sram_addr_reg_offset = ;
> > -             break;
> > -
> > -     case AMDGPU_UCODE_ID_VCE:
> > -             *sram_offset = ;
> > -             *sram_addr_reg_offset = ;
> > -             break;
> > -#endif
> > -
> > -     case AMDGPU_UCODE_ID_MAXIMUM:
> > -     default:
> > -             ret = -EINVAL;
> > -             break;
> > -     }
> > -
> > -     return ret;
> > -}
> > -
> > -static bool psp_v12_0_compare_sram_data(struct psp_context *psp,
> > -                                    struct amdgpu_firmware_info *ucode,
> > -                                    enum AMDGPU_UCODE_ID ucode_type)
> > -{
> > -     int err = 0;
> > -     unsigned int fw_sram_reg_val = 0;
> > -     unsigned int fw_sram_addr_reg_offset = 0;
> > -     unsigned int fw_sram_data_reg_offset = 0;
> > -     unsigned int ucode_size;
> > -     uint32_t *ucode_mem = NULL;
> > -     struct amdgpu_device *adev = psp->adev;
> > -
> > -     err = psp_v12_0_sram_map(adev, &fw_sram_reg_val, &fw_sram_addr_reg_offset,
> > -                             &fw_sram_data_reg_offset, ucode_type);
> > -     if (err)
> > -             return false;
> > -
> > -     WREG32(fw_sram_addr_reg_offset, fw_sram_reg_val);
> > -
> > -     ucode_size = ucode->ucode_size;
> > -     ucode_mem = (uint32_t *)ucode->kaddr;
> > -     while (ucode_size) {
> > -             fw_sram_reg_val = RREG32(fw_sram_data_reg_offset);
> > -
> > -             if (*ucode_mem != fw_sram_reg_val)
> > -                     return false;
> > -
> > -             ucode_mem++;
> > -             /* 4 bytes */
> > -             ucode_size -= 4;
> > -     }
> > -
> > -     return true;
> > -}
> > -
> >  static int psp_v12_0_mode1_reset(struct psp_context *psp)
> >  {
> >       int ret;
> > @@ -512,7 +390,6 @@ static const struct psp_funcs psp_v12_0_funcs = {
> >       .ring_create = psp_v12_0_ring_create,
> >       .ring_stop = psp_v12_0_ring_stop,
> >       .ring_destroy = psp_v12_0_ring_destroy,
> > -     .compare_sram_data = psp_v12_0_compare_sram_data,
> >       .mode1_reset = psp_v12_0_mode1_reset,
> >       .ring_get_wptr = psp_v12_0_ring_get_wptr,
> >       .ring_set_wptr = psp_v12_0_ring_set_wptr,
> > diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c b/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c
> > index ab03190..9ca37d0 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c
> > @@ -398,128 +398,6 @@ static int psp_v3_1_ring_destroy(struct psp_context *psp,
> >       return ret;
> >  }
> >
> > -static int
> > -psp_v3_1_sram_map(struct amdgpu_device *adev,
> > -               unsigned int *sram_offset, unsigned int *sram_addr_reg_offset,
> > -               unsigned int *sram_data_reg_offset,
> > -               enum AMDGPU_UCODE_ID ucode_id)
> > -{
> > -     int ret = 0;
> > -
> > -     switch(ucode_id) {
> > -/* TODO: needs to confirm */
> > -#if 0
> > -     case AMDGPU_UCODE_ID_SMC:
> > -             *sram_offset = 0;
> > -             *sram_addr_reg_offset = 0;
> > -             *sram_data_reg_offset = 0;
> > -             break;
> > -#endif
> > -
> > -     case AMDGPU_UCODE_ID_CP_CE:
> > -             *sram_offset = 0x0;
> > -             *sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_CE_UCODE_ADDR);
> > -             *sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_CE_UCODE_DATA);
> > -             break;
> > -
> > -     case AMDGPU_UCODE_ID_CP_PFP:
> > -             *sram_offset = 0x0;
> > -             *sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_PFP_UCODE_ADDR);
> > -             *sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_PFP_UCODE_DATA);
> > -             break;
> > -
> > -     case AMDGPU_UCODE_ID_CP_ME:
> > -             *sram_offset = 0x0;
> > -             *sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_HYP_ME_UCODE_ADDR);
> > -             *sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_HYP_ME_UCODE_DATA);
> > -             break;
> > -
> > -     case AMDGPU_UCODE_ID_CP_MEC1:
> > -             *sram_offset = 0x10000;
> > -             *sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_MEC_ME1_UCODE_ADDR);
> > -             *sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_MEC_ME1_UCODE_DATA);
> > -             break;
> > -
> > -     case AMDGPU_UCODE_ID_CP_MEC2:
> > -             *sram_offset = 0x10000;
> > -             *sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_HYP_MEC2_UCODE_ADDR);
> > -             *sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmCP_HYP_MEC2_UCODE_DATA);
> > -             break;
> > -
> > -     case AMDGPU_UCODE_ID_RLC_G:
> > -             *sram_offset = 0x2000;
> > -             *sram_addr_reg_offset = SOC15_REG_OFFSET(GC, 0, mmRLC_GPM_UCODE_ADDR);
> > -             *sram_data_reg_offset = SOC15_REG_OFFSET(GC, 0, mmRLC_GPM_UCODE_DATA);
> > -             break;
> > -
> > -     case AMDGPU_UCODE_ID_SDMA0:
> > -             *sram_offset = 0x0;
> > -             *sram_addr_reg_offset = SOC15_REG_OFFSET(SDMA0, 0, mmSDMA0_UCODE_ADDR);
> > -             *sram_data_reg_offset = SOC15_REG_OFFSET(SDMA0, 0, mmSDMA0_UCODE_DATA);
> > -             break;
> > -
> > -/* TODO: needs to confirm */
> > -#if 0
> > -     case AMDGPU_UCODE_ID_SDMA1:
> > -             *sram_offset = ;
> > -             *sram_addr_reg_offset = ;
> > -             break;
> > -
> > -     case AMDGPU_UCODE_ID_UVD:
> > -             *sram_offset = ;
> > -             *sram_addr_reg_offset = ;
> > -             break;
> > -
> > -     case AMDGPU_UCODE_ID_VCE:
> > -             *sram_offset = ;
> > -             *sram_addr_reg_offset = ;
> > -             break;
> > -#endif
> > -
> > -     case AMDGPU_UCODE_ID_MAXIMUM:
> > -     default:
> > -             ret = -EINVAL;
> > -             break;
> > -     }
> > -
> > -     return ret;
> > -}
> > -
> > -static bool psp_v3_1_compare_sram_data(struct psp_context *psp,
> > -                                    struct amdgpu_firmware_info *ucode,
> > -                                    enum AMDGPU_UCODE_ID ucode_type)
> > -{
> > -     int err = 0;
> > -     unsigned int fw_sram_reg_val = 0;
> > -     unsigned int fw_sram_addr_reg_offset = 0;
> > -     unsigned int fw_sram_data_reg_offset = 0;
> > -     unsigned int ucode_size;
> > -     uint32_t *ucode_mem = NULL;
> > -     struct amdgpu_device *adev = psp->adev;
> > -
> > -     err = psp_v3_1_sram_map(adev, &fw_sram_reg_val, &fw_sram_addr_reg_offset,
> > -                             &fw_sram_data_reg_offset, ucode_type);
> > -     if (err)
> > -             return false;
> > -
> > -     WREG32(fw_sram_addr_reg_offset, fw_sram_reg_val);
> > -
> > -     ucode_size = ucode->ucode_size;
> > -     ucode_mem = (uint32_t *)ucode->kaddr;
> > -     while (ucode_size) {
> > -             fw_sram_reg_val = RREG32(fw_sram_data_reg_offset);
> > -
> > -             if (*ucode_mem != fw_sram_reg_val)
> > -                     return false;
> > -
> > -             ucode_mem++;
> > -             /* 4 bytes */
> > -             ucode_size -= 4;
> > -     }
> > -
> > -     return true;
> > -}
> > -
> >  static bool psp_v3_1_smu_reload_quirk(struct psp_context *psp)
> >  {
> >       struct amdgpu_device *adev = psp->adev;
> > @@ -596,7 +474,6 @@ static const struct psp_funcs psp_v3_1_funcs = {
> >       .ring_create = psp_v3_1_ring_create,
> >       .ring_stop = psp_v3_1_ring_stop,
> >       .ring_destroy = psp_v3_1_ring_destroy,
> > -     .compare_sram_data = psp_v3_1_compare_sram_data,
> >       .smu_reload_quirk = psp_v3_1_smu_reload_quirk,
> >       .mode1_reset = psp_v3_1_mode1_reset,
> >       .ring_get_wptr = psp_v3_1_ring_get_wptr,
> >
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 4/8] drm/amdgpu: add helper function to init asd ucode

[Hawking Zhang]

asd is unified ucode across asic. it is not necessary
to keep its software structure to be ip specific one

Signed-off-by: Hawking Zhang <Hawking.Zhang@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 36 +++++++++++++++++++++++++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h |  2 ++
 2 files changed, 38 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
index 7797065..3656068 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
@@ -1840,6 +1840,42 @@ int psp_ring_cmd_submit(struct psp_context *psp,
 	return 0;
 }
 
+int psp_init_asd_microcode(struct psp_context *psp,
+			   const char *chip_name)
+{
+	struct amdgpu_device *adev = psp->adev;
+	char fw_name[30];
+	const struct psp_firmware_header_v1_0 *asd_hdr;
+	int err = 0;
+
+	if (!chip_name) {
+		dev_err(adev->dev, "invalid chip name for asd microcode\n");
+		return -EINVAL;
+	}
+
+	snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_asd.bin", chip_name);
+	err = request_firmware(&adev->psp.asd_fw, fw_name, adev->dev);
+	if (err)
+		goto out;
+
+	err = amdgpu_ucode_validate(adev->psp.asd_fw);
+	if (err)
+		goto out;
+
+	asd_hdr = (const struct psp_firmware_header_v1_0 *)adev->psp.asd_fw->data;
+	adev->psp.asd_fw_version = le32_to_cpu(asd_hdr->header.ucode_version);
+	adev->psp.asd_feature_version = le32_to_cpu(asd_hdr->ucode_feature_version);
+	adev->psp.asd_ucode_size = le32_to_cpu(asd_hdr->header.ucode_size_bytes);
+	adev->psp.asd_start_addr = (uint8_t *)asd_hdr +
+				le32_to_cpu(asd_hdr->header.ucode_array_offset_bytes);
+	return 0;
+out:
+	dev_err(adev->dev, "fail to initialize asd microcode\n");
+	release_firmware(adev->psp.asd_fw);
+	adev->psp.asd_fw = NULL;
+	return err;
+}
+
 static int psp_set_clockgating_state(void *handle,
 				     enum amd_clockgating_state state)
 {
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
index f8b1f03..a763148 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
@@ -385,4 +385,6 @@ int psp_ring_cmd_submit(struct psp_context *psp,
 			uint64_t cmd_buf_mc_addr,
 			uint64_t fence_mc_addr,
 			int index);
+int psp_init_asd_microcode(struct psp_context *psp,
+			   const char *chip_name);
 #endif
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 4/8] drm/amdgpu: add helper function to init asd ucode

[Luben Tuikov]

On 2020-04-20 6:16 a.m., Hawking Zhang wrote:
> asd is unified ucode across asic. it is not necessary
> to keep its software structure to be ip specific one

Sentences usually start with a capital letter.
"ASD is unified uCode across ASICs."
The second sentence uses "it" twice and it is not clear
whose software structure?
ip --> IP and the sentence should end with a period.

> 
> Signed-off-by: Hawking Zhang <Hawking.Zhang@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 36 +++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h |  2 ++
>  2 files changed, 38 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> index 7797065..3656068 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> @@ -1840,6 +1840,42 @@ int psp_ring_cmd_submit(struct psp_context *psp,
>  	return 0;
>  }
>  
> +int psp_init_asd_microcode(struct psp_context *psp,
> +			   const char *chip_name)
> +{
> +	struct amdgpu_device *adev = psp->adev;
> +	char fw_name[30];

I'm not sure that that length is going to be enough in all contingencies.
/lib/firmware and /usr/lib/firmware are indeed the same inode (hard link),
but if/when using /usr/lib/firmware as opposed to /lib/firmware, some names
for ASD firmware are at 40 or more characters:

$for F in /usr/lib/firmware/amdgpu/*asd*; do LL=`echo $F | wc -c`; echo $LL : $F ; done
42 : /usr/lib/firmware/amdgpu/arcturus_asd.bin
40 : /usr/lib/firmware/amdgpu/navi10_asd.bin
40 : /usr/lib/firmware/amdgpu/navi12_asd.bin
40 : /usr/lib/firmware/amdgpu/navi14_asd.bin
41 : /usr/lib/firmware/amdgpu/picasso_asd.bin
40 : /usr/lib/firmware/amdgpu/raven2_asd.bin
39 : /usr/lib/firmware/amdgpu/raven_asd.bin
40 : /usr/lib/firmware/amdgpu/renoir_asd.bin
40 : /usr/lib/firmware/amdgpu/vega10_asd.bin
40 : /usr/lib/firmware/amdgpu/vega12_asd.bin
40 : /usr/lib/firmware/amdgpu/vega20_asd.bin

And when using /lib/firmware, the above line lengths are less by 4 characters,
which leaves it too close for comfort given that ASIC names could vary.

So, I'd rather see the size of the path name be something larger,
say 50, or more.

> +	const struct psp_firmware_header_v1_0 *asd_hdr;
> +	int err = 0;
> +
> +	if (!chip_name) {
> +		dev_err(adev->dev, "invalid chip name for asd microcode\n");

Here, "chip_name" is not "invalid"--it's NULL. The message
printed in the kernel logs should be more clear:

"No chip name given for ASD microcode."

> +		return -EINVAL;
> +	}
> +
> +	snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_asd.bin", chip_name);
> +	err = request_firmware(&adev->psp.asd_fw, fw_name, adev->dev);
> +	if (err)
> +		goto out;
> +
> +	err = amdgpu_ucode_validate(adev->psp.asd_fw);
> +	if (err)
> +		goto out;
> +
> +	asd_hdr = (const struct psp_firmware_header_v1_0 *)adev->psp.asd_fw->data;
> +	adev->psp.asd_fw_version = le32_to_cpu(asd_hdr->header.ucode_version);
> +	adev->psp.asd_feature_version = le32_to_cpu(asd_hdr->ucode_feature_version);
> +	adev->psp.asd_ucode_size = le32_to_cpu(asd_hdr->header.ucode_size_bytes);
> +	adev->psp.asd_start_addr = (uint8_t *)asd_hdr +
> +				le32_to_cpu(asd_hdr->header.ucode_array_offset_bytes);
> +	return 0;
> +out:
> +	dev_err(adev->dev, "fail to initialize asd microcode\n");

This message is vague, in both counts: load and validate.
The rest of the driver prints something like "failed to load %s firmware", fw_name,
which is more descriptive an_ it shows the file which failed to be loaded
or validated (giving the user a visual check of the name).

For instance, see gfx_v10_0.v at the bottom of "..._init_microcode()" function.

Print something like,

	dev_err(adev->dev, "psp: Failed to load firmware \"%s\"\n", fw_name);

Regards,
Luben

> +	release_firmware(adev->psp.asd_fw);
> +	adev->psp.asd_fw = NULL;
> +	return err;
> +}
> +
>  static int psp_set_clockgating_state(void *handle,
>  				     enum amd_clockgating_state state)
>  {
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
> index f8b1f03..a763148 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
> @@ -385,4 +385,6 @@ int psp_ring_cmd_submit(struct psp_context *psp,
>  			uint64_t cmd_buf_mc_addr,
>  			uint64_t fence_mc_addr,
>  			int index);
> +int psp_init_asd_microcode(struct psp_context *psp,
> +			   const char *chip_name);
>  #endif
> 

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 4/8] drm/amdgpu: add helper function to init asd ucode

[Luben Tuikov]

On 2020-04-20 11:29 a.m., Luben Tuikov wrote:
> On 2020-04-20 6:16 a.m., Hawking Zhang wrote:
>> asd is unified ucode across asic. it is not necessary
>> to keep its software structure to be ip specific one
> 
> Sentences usually start with a capital letter.
> "ASD is unified uCode across ASICs."
> The second sentence uses "it" twice and it is not clear
> whose software structure?
> ip --> IP and the sentence should end with a period.
> 
>>
>> Signed-off-by: Hawking Zhang <Hawking.Zhang@amd.com>
>> ---
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 36 +++++++++++++++++++++++++++++++++
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h |  2 ++
>>  2 files changed, 38 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
>> index 7797065..3656068 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
>> @@ -1840,6 +1840,42 @@ int psp_ring_cmd_submit(struct psp_context *psp,
>>  	return 0;
>>  }
>>  
>> +int psp_init_asd_microcode(struct psp_context *psp,
>> +			   const char *chip_name)
>> +{
>> +	struct amdgpu_device *adev = psp->adev;
>> +	char fw_name[30];
> 
> I'm not sure that that length is going to be enough in all contingencies.
> /lib/firmware and /usr/lib/firmware are indeed the same inode (hard link),
> but if/when using /usr/lib/firmware as opposed to /lib/firmware, some names
> for ASD firmware are at 40 or more characters:
> 
> $for F in /usr/lib/firmware/amdgpu/*asd*; do LL=`echo $F | wc -c`; echo $LL : $F ; done
> 42 : /usr/lib/firmware/amdgpu/arcturus_asd.bin
> 40 : /usr/lib/firmware/amdgpu/navi10_asd.bin
> 40 : /usr/lib/firmware/amdgpu/navi12_asd.bin
> 40 : /usr/lib/firmware/amdgpu/navi14_asd.bin
> 41 : /usr/lib/firmware/amdgpu/picasso_asd.bin
> 40 : /usr/lib/firmware/amdgpu/raven2_asd.bin
> 39 : /usr/lib/firmware/amdgpu/raven_asd.bin
> 40 : /usr/lib/firmware/amdgpu/renoir_asd.bin
> 40 : /usr/lib/firmware/amdgpu/vega10_asd.bin
> 40 : /usr/lib/firmware/amdgpu/vega12_asd.bin
> 40 : /usr/lib/firmware/amdgpu/vega20_asd.bin
> 
> And when using /lib/firmware, the above line lengths are less by 4 characters,
> which leaves it too close for comfort given that ASIC names could vary.
> 
> So, I'd rather see the size of the path name be something larger,
> say 50, or more.
> 
>> +	const struct psp_firmware_header_v1_0 *asd_hdr;
>> +	int err = 0;
>> +
>> +	if (!chip_name) {
>> +		dev_err(adev->dev, "invalid chip name for asd microcode\n");
> 
> Here, "chip_name" is not "invalid"--it's NULL. The message
> printed in the kernel logs should be more clear:
> 
> "No chip name given for ASD microcode."
> 
>> +		return -EINVAL;
>> +	}
>> +
>> +	snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_asd.bin", chip_name);

Ah, I see you're only printing "amdgpu/%s_asd.bin", so I guess 30 chars
should fit.

Regards,
Luben

>> +	err = request_firmware(&adev->psp.asd_fw, fw_name, adev->dev);
>> +	if (err)
>> +		goto out;
>> +
>> +	err = amdgpu_ucode_validate(adev->psp.asd_fw);
>> +	if (err)
>> +		goto out;
>> +
>> +	asd_hdr = (const struct psp_firmware_header_v1_0 *)adev->psp.asd_fw->data;
>> +	adev->psp.asd_fw_version = le32_to_cpu(asd_hdr->header.ucode_version);
>> +	adev->psp.asd_feature_version = le32_to_cpu(asd_hdr->ucode_feature_version);
>> +	adev->psp.asd_ucode_size = le32_to_cpu(asd_hdr->header.ucode_size_bytes);
>> +	adev->psp.asd_start_addr = (uint8_t *)asd_hdr +
>> +				le32_to_cpu(asd_hdr->header.ucode_array_offset_bytes);
>> +	return 0;
>> +out:
>> +	dev_err(adev->dev, "fail to initialize asd microcode\n");
> 
> This message is vague, in both counts: load and validate.
> The rest of the driver prints something like "failed to load %s firmware", fw_name,
> which is more descriptive an_ it shows the file which failed to be loaded
> or validated (giving the user a visual check of the name).
> 
> For instance, see gfx_v10_0.v at the bottom of "..._init_microcode()" function.
> 
> Print something like,
> 
> 	dev_err(adev->dev, "psp: Failed to load firmware \"%s\"\n", fw_name);
> 
> Regards,
> Luben
> 
>> +	release_firmware(adev->psp.asd_fw);
>> +	adev->psp.asd_fw = NULL;
>> +	return err;
>> +}
>> +
>>  static int psp_set_clockgating_state(void *handle,
>>  				     enum amd_clockgating_state state)
>>  {
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
>> index f8b1f03..a763148 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
>> @@ -385,4 +385,6 @@ int psp_ring_cmd_submit(struct psp_context *psp,
>>  			uint64_t cmd_buf_mc_addr,
>>  			uint64_t fence_mc_addr,
>>  			int index);
>> +int psp_init_asd_microcode(struct psp_context *psp,
>> +			   const char *chip_name);
>>  #endif
>>
> 
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=02%7C01%7Cluben.tuikov%40amd.com%7C3bf4ce6df8a348e735a608d7e53fada3%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637229933975152085&amp;sdata=Vlq1qsHb8ygT46RJgf9fHamzUGAFuWB3%2B1xKaI06H2o%3D&amp;reserved=0
> 

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 5/8] drm/amdgpu: switch to helper function to init asd ucode

[Hawking Zhang]

call common helper function to initialize asd ucode

Signed-off-by: Hawking Zhang <Hawking.Zhang@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/psp_v10_0.c | 17 +----------------
 drivers/gpu/drm/amd/amdgpu/psp_v11_0.c | 20 ++------------------
 drivers/gpu/drm/amd/amdgpu/psp_v12_0.c | 27 +--------------------------
 drivers/gpu/drm/amd/amdgpu/psp_v3_1.c  | 16 +---------------
 4 files changed, 5 insertions(+), 75 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c b/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
index 6e041b7..90727cf 100644
--- a/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
@@ -50,7 +50,6 @@ static int psp_v10_0_init_microcode(struct psp_context *psp)
 	const char *chip_name;
 	char fw_name[30];
 	int err = 0;
-	const struct psp_firmware_header_v1_0 *hdr;
 	const struct ta_firmware_header_v1_0 *ta_hdr;
 	DRM_DEBUG("\n");
 
@@ -66,22 +65,10 @@ static int psp_v10_0_init_microcode(struct psp_context *psp)
 	default: BUG();
 	}
 
-	snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_asd.bin", chip_name);
-	err = request_firmware(&adev->psp.asd_fw, fw_name, adev->dev);
+	err = psp_init_asd_microcode(psp, chip_name);
 	if (err)
 		goto out;
 
-	err = amdgpu_ucode_validate(adev->psp.asd_fw);
-	if (err)
-		goto out;
-
-	hdr = (const struct psp_firmware_header_v1_0 *)adev->psp.asd_fw->data;
-	adev->psp.asd_fw_version = le32_to_cpu(hdr->header.ucode_version);
-	adev->psp.asd_feature_version = le32_to_cpu(hdr->ucode_feature_version);
-	adev->psp.asd_ucode_size = le32_to_cpu(hdr->header.ucode_size_bytes);
-	adev->psp.asd_start_addr = (uint8_t *)hdr +
-				le32_to_cpu(hdr->header.ucode_array_offset_bytes);
-
 	snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_ta.bin", chip_name);
 	err = request_firmware(&adev->psp.ta_fw, fw_name, adev->dev);
 	if (err) {
@@ -126,8 +113,6 @@ static int psp_v10_0_init_microcode(struct psp_context *psp)
 		dev_err(adev->dev,
 			"psp v10.0: Failed to load firmware \"%s\"\n",
 			fw_name);
-		release_firmware(adev->psp.asd_fw);
-		adev->psp.asd_fw = NULL;
 	}
 
 	return err;
diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
index f633577..6d50da0 100644
--- a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
@@ -78,7 +78,6 @@ static int psp_v11_0_init_microcode(struct psp_context *psp)
 	const struct psp_firmware_header_v1_0 *sos_hdr;
 	const struct psp_firmware_header_v1_1 *sos_hdr_v1_1;
 	const struct psp_firmware_header_v1_2 *sos_hdr_v1_2;
-	const struct psp_firmware_header_v1_0 *asd_hdr;
 	const struct ta_firmware_header_v1_0 *ta_hdr;
 
 	DRM_DEBUG("\n");
@@ -148,21 +147,9 @@ static int psp_v11_0_init_microcode(struct psp_context *psp)
 		goto out;
 	}
 
-	snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_asd.bin", chip_name);
-	err = request_firmware(&adev->psp.asd_fw, fw_name, adev->dev);
+	err = psp_init_asd_microcode(psp, chip_name);
 	if (err)
-		goto out1;
-
-	err = amdgpu_ucode_validate(adev->psp.asd_fw);
-	if (err)
-		goto out1;
-
-	asd_hdr = (const struct psp_firmware_header_v1_0 *)adev->psp.asd_fw->data;
-	adev->psp.asd_fw_version = le32_to_cpu(asd_hdr->header.ucode_version);
-	adev->psp.asd_feature_version = le32_to_cpu(asd_hdr->ucode_feature_version);
-	adev->psp.asd_ucode_size = le32_to_cpu(asd_hdr->header.ucode_size_bytes);
-	adev->psp.asd_start_addr = (uint8_t *)asd_hdr +
-				le32_to_cpu(asd_hdr->header.ucode_array_offset_bytes);
+		goto out;
 
 	switch (adev->asic_type) {
 	case CHIP_VEGA20:
@@ -229,9 +216,6 @@ static int psp_v11_0_init_microcode(struct psp_context *psp)
 out2:
 	release_firmware(adev->psp.ta_fw);
 	adev->psp.ta_fw = NULL;
-out1:
-	release_firmware(adev->psp.asd_fw);
-	adev->psp.asd_fw = NULL;
 out:
 	dev_err(adev->dev,
 		"psp v11.0: Failed to load firmware \"%s\"\n", fw_name);
diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v12_0.c b/drivers/gpu/drm/amd/amdgpu/psp_v12_0.c
index 42c485b..6c9614f 100644
--- a/drivers/gpu/drm/amd/amdgpu/psp_v12_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/psp_v12_0.c
@@ -45,11 +45,7 @@ static int psp_v12_0_init_microcode(struct psp_context *psp)
 {
 	struct amdgpu_device *adev = psp->adev;
 	const char *chip_name;
-	char fw_name[30];
 	int err = 0;
-	const struct psp_firmware_header_v1_0 *asd_hdr;
-
-	DRM_DEBUG("\n");
 
 	switch (adev->asic_type) {
 	case CHIP_RENOIR:
@@ -59,28 +55,7 @@ static int psp_v12_0_init_microcode(struct psp_context *psp)
 		BUG();
 	}
 
-	snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_asd.bin", chip_name);
-	err = request_firmware(&adev->psp.asd_fw, fw_name, adev->dev);
-	if (err)
-		goto out1;
-
-	err = amdgpu_ucode_validate(adev->psp.asd_fw);
-	if (err)
-		goto out1;
-
-	asd_hdr = (const struct psp_firmware_header_v1_0 *)adev->psp.asd_fw->data;
-	adev->psp.asd_fw_version = le32_to_cpu(asd_hdr->header.ucode_version);
-	adev->psp.asd_feature_version = le32_to_cpu(asd_hdr->ucode_feature_version);
-	adev->psp.asd_ucode_size = le32_to_cpu(asd_hdr->header.ucode_size_bytes);
-	adev->psp.asd_start_addr = (uint8_t *)asd_hdr +
-				le32_to_cpu(asd_hdr->header.ucode_array_offset_bytes);
-
-	return 0;
-
-out1:
-	release_firmware(adev->psp.asd_fw);
-	adev->psp.asd_fw = NULL;
-
+	err = psp_init_asd_microcode(psp, chip_name);
 	return err;
 }
 
diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c b/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c
index 9ca37d0..14d17b1 100644
--- a/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c
+++ b/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c
@@ -95,22 +95,10 @@ static int psp_v3_1_init_microcode(struct psp_context *psp)
 	adev->psp.sos_start_addr = (uint8_t *)adev->psp.sys_start_addr +
 				le32_to_cpu(hdr->sos_offset_bytes);
 
-	snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_asd.bin", chip_name);
-	err = request_firmware(&adev->psp.asd_fw, fw_name, adev->dev);
+	err = psp_init_asd_microcode(psp, chip_name);
 	if (err)
 		goto out;
 
-	err = amdgpu_ucode_validate(adev->psp.asd_fw);
-	if (err)
-		goto out;
-
-	hdr = (const struct psp_firmware_header_v1_0 *)adev->psp.asd_fw->data;
-	adev->psp.asd_fw_version = le32_to_cpu(hdr->header.ucode_version);
-	adev->psp.asd_feature_version = le32_to_cpu(hdr->ucode_feature_version);
-	adev->psp.asd_ucode_size = le32_to_cpu(hdr->header.ucode_size_bytes);
-	adev->psp.asd_start_addr = (uint8_t *)hdr +
-				le32_to_cpu(hdr->header.ucode_array_offset_bytes);
-
 	return 0;
 out:
 	if (err) {
@@ -119,8 +107,6 @@ static int psp_v3_1_init_microcode(struct psp_context *psp)
 			fw_name);
 		release_firmware(adev->psp.sos_fw);
 		adev->psp.sos_fw = NULL;
-		release_firmware(adev->psp.asd_fw);
-		adev->psp.asd_fw = NULL;
 	}
 
 	return err;
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 6/8] drm/amdgpu: add helper function to init sos ucode

[Hawking Zhang]

driver already had psp_firmware_header struture to
deal with different layout of sos ucode. the sos
micorcode initialization could be common one.

Signed-off-by: Hawking Zhang <Hawking.Zhang@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 70 +++++++++++++++++++++++++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h |  2 +
 2 files changed, 72 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
index 3656068..730f98a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
@@ -1876,6 +1876,76 @@ int psp_init_asd_microcode(struct psp_context *psp,
 	return err;
 }
 
+int psp_init_sos_microcode(struct psp_context *psp,
+			   const char *chip_name)
+{
+	struct amdgpu_device *adev = psp->adev;
+	char fw_name[30];
+	const struct psp_firmware_header_v1_0 *sos_hdr;
+	const struct psp_firmware_header_v1_1 *sos_hdr_v1_1;
+	const struct psp_firmware_header_v1_2 *sos_hdr_v1_2;
+	int err = 0;
+
+	if (!chip_name) {
+		dev_err(adev->dev, "invalid chip name for sos microcode\n");
+		return -EINVAL;
+	}
+
+	snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_sos.bin", chip_name);
+	err = request_firmware(&adev->psp.sos_fw, fw_name, adev->dev);
+	if (err)
+		goto out;
+
+	err = amdgpu_ucode_validate(adev->psp.sos_fw);
+	if (err)
+		goto out;
+
+	sos_hdr = (const struct psp_firmware_header_v1_0 *)adev->psp.sos_fw->data;
+	amdgpu_ucode_print_psp_hdr(&sos_hdr->header);
+
+	switch (sos_hdr->header.header_version_major) {
+	case 1:
+		adev->psp.sos_fw_version = le32_to_cpu(sos_hdr->header.ucode_version);
+		adev->psp.sos_feature_version = le32_to_cpu(sos_hdr->ucode_feature_version);
+		adev->psp.sos_bin_size = le32_to_cpu(sos_hdr->sos_size_bytes);
+		adev->psp.sys_bin_size = le32_to_cpu(sos_hdr->sos_offset_bytes);
+		adev->psp.sys_start_addr = (uint8_t *)sos_hdr +
+				le32_to_cpu(sos_hdr->header.ucode_array_offset_bytes);
+		adev->psp.sos_start_addr = (uint8_t *)adev->psp.sys_start_addr +
+				le32_to_cpu(sos_hdr->sos_offset_bytes);
+		if (sos_hdr->header.header_version_minor == 1) {
+			sos_hdr_v1_1 = (const struct psp_firmware_header_v1_1 *)adev->psp.sos_fw->data;
+			adev->psp.toc_bin_size = le32_to_cpu(sos_hdr_v1_1->toc_size_bytes);
+			adev->psp.toc_start_addr = (uint8_t *)adev->psp.sys_start_addr +
+					le32_to_cpu(sos_hdr_v1_1->toc_offset_bytes);
+			adev->psp.kdb_bin_size = le32_to_cpu(sos_hdr_v1_1->kdb_size_bytes);
+			adev->psp.kdb_start_addr = (uint8_t *)adev->psp.sys_start_addr +
+					le32_to_cpu(sos_hdr_v1_1->kdb_offset_bytes);
+		}
+		if (sos_hdr->header.header_version_minor == 2) {
+			sos_hdr_v1_2 = (const struct psp_firmware_header_v1_2 *)adev->psp.sos_fw->data;
+			adev->psp.kdb_bin_size = le32_to_cpu(sos_hdr_v1_2->kdb_size_bytes);
+			adev->psp.kdb_start_addr = (uint8_t *)adev->psp.sys_start_addr +
+						    le32_to_cpu(sos_hdr_v1_2->kdb_offset_bytes);
+		}
+		break;
+	default:
+		dev_err(adev->dev,
+			"unsupported psp sos firmware\n");
+		err = -EINVAL;
+		goto out;
+	}
+
+	return 0;
+out:
+	dev_err(adev->dev,
+		"failed to init sos firmware\n");
+	release_firmware(adev->psp.sos_fw);
+	adev->psp.sos_fw = NULL;
+
+	return err;
+}
+
 static int psp_set_clockgating_state(void *handle,
 				     enum amd_clockgating_state state)
 {
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
index a763148..7fcd63d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
@@ -387,4 +387,6 @@ int psp_ring_cmd_submit(struct psp_context *psp,
 			int index);
 int psp_init_asd_microcode(struct psp_context *psp,
 			   const char *chip_name);
+int psp_init_sos_microcode(struct psp_context *psp,
+			   const char *chip_name);
 #endif
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 6/8] drm/amdgpu: add helper function to init sos ucode

[Luben Tuikov]

On 2020-04-20 6:16 a.m., Hawking Zhang wrote:
> driver already had psp_firmware_header struture to
> deal with different layout of sos ucode. the sos
> micorcode initialization could be common one.
> 
> Signed-off-by: Hawking Zhang <Hawking.Zhang@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 70 +++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h |  2 +
>  2 files changed, 72 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> index 3656068..730f98a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> @@ -1876,6 +1876,76 @@ int psp_init_asd_microcode(struct psp_context *psp,
>  	return err;
>  }
>  
> +int psp_init_sos_microcode(struct psp_context *psp,
> +			   const char *chip_name)
> +{
> +	struct amdgpu_device *adev = psp->adev;
> +	char fw_name[30];
> +	const struct psp_firmware_header_v1_0 *sos_hdr;
> +	const struct psp_firmware_header_v1_1 *sos_hdr_v1_1;
> +	const struct psp_firmware_header_v1_2 *sos_hdr_v1_2;
> +	int err = 0;
> +
> +	if (!chip_name) {
> +		dev_err(adev->dev, "invalid chip name for sos microcode\n");
> +		return -EINVAL;
> +	}
> +
> +	snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_sos.bin", chip_name);
> +	err = request_firmware(&adev->psp.sos_fw, fw_name, adev->dev);
> +	if (err)
> +		goto out;
> +
> +	err = amdgpu_ucode_validate(adev->psp.sos_fw);
> +	if (err)
> +		goto out;
> +
> +	sos_hdr = (const struct psp_firmware_header_v1_0 *)adev->psp.sos_fw->data;
> +	amdgpu_ucode_print_psp_hdr(&sos_hdr->header);
> +
> +	switch (sos_hdr->header.header_version_major) {
> +	case 1:
> +		adev->psp.sos_fw_version = le32_to_cpu(sos_hdr->header.ucode_version);
> +		adev->psp.sos_feature_version = le32_to_cpu(sos_hdr->ucode_feature_version);
> +		adev->psp.sos_bin_size = le32_to_cpu(sos_hdr->sos_size_bytes);
> +		adev->psp.sys_bin_size = le32_to_cpu(sos_hdr->sos_offset_bytes);
> +		adev->psp.sys_start_addr = (uint8_t *)sos_hdr +
> +				le32_to_cpu(sos_hdr->header.ucode_array_offset_bytes);
> +		adev->psp.sos_start_addr = (uint8_t *)adev->psp.sys_start_addr +
> +				le32_to_cpu(sos_hdr->sos_offset_bytes);
> +		if (sos_hdr->header.header_version_minor == 1) {
> +			sos_hdr_v1_1 = (const struct psp_firmware_header_v1_1 *)adev->psp.sos_fw->data;
> +			adev->psp.toc_bin_size = le32_to_cpu(sos_hdr_v1_1->toc_size_bytes);
> +			adev->psp.toc_start_addr = (uint8_t *)adev->psp.sys_start_addr +
> +					le32_to_cpu(sos_hdr_v1_1->toc_offset_bytes);
> +			adev->psp.kdb_bin_size = le32_to_cpu(sos_hdr_v1_1->kdb_size_bytes);
> +			adev->psp.kdb_start_addr = (uint8_t *)adev->psp.sys_start_addr +
> +					le32_to_cpu(sos_hdr_v1_1->kdb_offset_bytes);
> +		}
> +		if (sos_hdr->header.header_version_minor == 2) {
> +			sos_hdr_v1_2 = (const struct psp_firmware_header_v1_2 *)adev->psp.sos_fw->data;
> +			adev->psp.kdb_bin_size = le32_to_cpu(sos_hdr_v1_2->kdb_size_bytes);
> +			adev->psp.kdb_start_addr = (uint8_t *)adev->psp.sys_start_addr +
> +						    le32_to_cpu(sos_hdr_v1_2->kdb_offset_bytes);
> +		}
> +		break;
> +	default:
> +		dev_err(adev->dev,
> +			"unsupported psp sos firmware\n");
> +		err = -EINVAL;
> +		goto out;
> +	}
> +
> +	return 0;
> +out:
> +	dev_err(adev->dev,
> +		"failed to init sos firmware\n");

The message is vague. Print the name of the firmware which
the driver wasn't able to load, don't use "failed to init"
as it doesn't give specific information, use this instead:

	dev_err(adev->dev, "Failed to load firmware \"%s\"\n", fw_name);

Like the rest of the driver does.

Regards,
Luben

> +	release_firmware(adev->psp.sos_fw);
> +	adev->psp.sos_fw = NULL;
> +
> +	return err;
> +}
> +
>  static int psp_set_clockgating_state(void *handle,
>  				     enum amd_clockgating_state state)
>  {
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
> index a763148..7fcd63d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
> @@ -387,4 +387,6 @@ int psp_ring_cmd_submit(struct psp_context *psp,
>  			int index);
>  int psp_init_asd_microcode(struct psp_context *psp,
>  			   const char *chip_name);
> +int psp_init_sos_microcode(struct psp_context *psp,
> +			   const char *chip_name);
>  #endif
> 

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 7/8] drm/amdgpu: switch to helper function to init sos ucode

[Hawking Zhang]

call common helper function to init sos ucode, instead
of duplicate codes per ip version

Signed-off-by: Hawking Zhang <Hawking.Zhang@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/psp_v11_0.c | 56 ++--------------------------------
 drivers/gpu/drm/amd/amdgpu/psp_v3_1.c  | 34 ++-------------------
 2 files changed, 6 insertions(+), 84 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
index 6d50da0..d2d2363 100644
--- a/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/psp_v11_0.c
@@ -75,9 +75,6 @@ static int psp_v11_0_init_microcode(struct psp_context *psp)
 	const char *chip_name;
 	char fw_name[30];
 	int err = 0;
-	const struct psp_firmware_header_v1_0 *sos_hdr;
-	const struct psp_firmware_header_v1_1 *sos_hdr_v1_1;
-	const struct psp_firmware_header_v1_2 *sos_hdr_v1_2;
 	const struct ta_firmware_header_v1_0 *ta_hdr;
 
 	DRM_DEBUG("\n");
@@ -102,54 +99,13 @@ static int psp_v11_0_init_microcode(struct psp_context *psp)
 		BUG();
 	}
 
-	snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_sos.bin", chip_name);
-	err = request_firmware(&adev->psp.sos_fw, fw_name, adev->dev);
+	err = psp_init_sos_microcode(psp, chip_name);
 	if (err)
-		goto out;
-
-	err = amdgpu_ucode_validate(adev->psp.sos_fw);
-	if (err)
-		goto out;
-
-	sos_hdr = (const struct psp_firmware_header_v1_0 *)adev->psp.sos_fw->data;
-	amdgpu_ucode_print_psp_hdr(&sos_hdr->header);
-
-	switch (sos_hdr->header.header_version_major) {
-	case 1:
-		adev->psp.sos_fw_version = le32_to_cpu(sos_hdr->header.ucode_version);
-		adev->psp.sos_feature_version = le32_to_cpu(sos_hdr->ucode_feature_version);
-		adev->psp.sos_bin_size = le32_to_cpu(sos_hdr->sos_size_bytes);
-		adev->psp.sys_bin_size = le32_to_cpu(sos_hdr->sos_offset_bytes);
-		adev->psp.sys_start_addr = (uint8_t *)sos_hdr +
-				le32_to_cpu(sos_hdr->header.ucode_array_offset_bytes);
-		adev->psp.sos_start_addr = (uint8_t *)adev->psp.sys_start_addr +
-				le32_to_cpu(sos_hdr->sos_offset_bytes);
-		if (sos_hdr->header.header_version_minor == 1) {
-			sos_hdr_v1_1 = (const struct psp_firmware_header_v1_1 *)adev->psp.sos_fw->data;
-			adev->psp.toc_bin_size = le32_to_cpu(sos_hdr_v1_1->toc_size_bytes);
-			adev->psp.toc_start_addr = (uint8_t *)adev->psp.sys_start_addr +
-					le32_to_cpu(sos_hdr_v1_1->toc_offset_bytes);
-			adev->psp.kdb_bin_size = le32_to_cpu(sos_hdr_v1_1->kdb_size_bytes);
-			adev->psp.kdb_start_addr = (uint8_t *)adev->psp.sys_start_addr +
-					le32_to_cpu(sos_hdr_v1_1->kdb_offset_bytes);
-		}
-		if (sos_hdr->header.header_version_minor == 2) {
-			sos_hdr_v1_2 = (const struct psp_firmware_header_v1_2 *)adev->psp.sos_fw->data;
-			adev->psp.kdb_bin_size = le32_to_cpu(sos_hdr_v1_2->kdb_size_bytes);
-			adev->psp.kdb_start_addr = (uint8_t *)adev->psp.sys_start_addr +
-						    le32_to_cpu(sos_hdr_v1_2->kdb_offset_bytes);
-		}
-		break;
-	default:
-		dev_err(adev->dev,
-			"Unsupported psp sos firmware\n");
-		err = -EINVAL;
-		goto out;
-	}
+		return err;
 
 	err = psp_init_asd_microcode(psp, chip_name);
 	if (err)
-		goto out;
+		return err;
 
 	switch (adev->asic_type) {
 	case CHIP_VEGA20:
@@ -216,12 +172,6 @@ static int psp_v11_0_init_microcode(struct psp_context *psp)
 out2:
 	release_firmware(adev->psp.ta_fw);
 	adev->psp.ta_fw = NULL;
-out:
-	dev_err(adev->dev,
-		"psp v11.0: Failed to load firmware \"%s\"\n", fw_name);
-	release_firmware(adev->psp.sos_fw);
-	adev->psp.sos_fw = NULL;
-
 	return err;
 }
 
diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c b/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c
index 14d17b1..7e3a2f2 100644
--- a/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c
+++ b/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c
@@ -59,9 +59,7 @@ static int psp_v3_1_init_microcode(struct psp_context *psp)
 {
 	struct amdgpu_device *adev = psp->adev;
 	const char *chip_name;
-	char fw_name[30];
 	int err = 0;
-	const struct psp_firmware_header_v1_0 *hdr;
 
 	DRM_DEBUG("\n");
 
@@ -75,41 +73,15 @@ static int psp_v3_1_init_microcode(struct psp_context *psp)
 	default: BUG();
 	}
 
-	snprintf(fw_name, sizeof(fw_name), "amdgpu/%s_sos.bin", chip_name);
-	err = request_firmware(&adev->psp.sos_fw, fw_name, adev->dev);
+	err = psp_init_sos_microcode(psp, chip_name);
 	if (err)
-		goto out;
-
-	err = amdgpu_ucode_validate(adev->psp.sos_fw);
-	if (err)
-		goto out;
-
-	hdr = (const struct psp_firmware_header_v1_0 *)adev->psp.sos_fw->data;
-	adev->psp.sos_fw_version = le32_to_cpu(hdr->header.ucode_version);
-	adev->psp.sos_feature_version = le32_to_cpu(hdr->ucode_feature_version);
-	adev->psp.sos_bin_size = le32_to_cpu(hdr->sos_size_bytes);
-	adev->psp.sys_bin_size = le32_to_cpu(hdr->header.ucode_size_bytes) -
-					le32_to_cpu(hdr->sos_size_bytes);
-	adev->psp.sys_start_addr = (uint8_t *)hdr +
-				le32_to_cpu(hdr->header.ucode_array_offset_bytes);
-	adev->psp.sos_start_addr = (uint8_t *)adev->psp.sys_start_addr +
-				le32_to_cpu(hdr->sos_offset_bytes);
+		return err;
 
 	err = psp_init_asd_microcode(psp, chip_name);
 	if (err)
-		goto out;
+		return err;
 
 	return 0;
-out:
-	if (err) {
-		dev_err(adev->dev,
-			"psp v3.1: Failed to load firmware \"%s\"\n",
-			fw_name);
-		release_firmware(adev->psp.sos_fw);
-		adev->psp.sos_fw = NULL;
-	}
-
-	return err;
 }
 
 static int psp_v3_1_bootloader_load_sysdrv(struct psp_context *psp)
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 8/8] drm/amdgpu: retire legacy vega10 sos version check

[Hawking Zhang]

retired those early sos version used in vega10 bring up
phase

Signed-off-by: Hawking Zhang <Hawking.Zhang@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/psp_v3_1.c | 28 +---------------------------
 1 file changed, 1 insertion(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c b/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c
index 7e3a2f2..f2e725f 100644
--- a/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c
+++ b/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c
@@ -50,8 +50,6 @@ MODULE_FIRMWARE("amdgpu/vega12_asd.bin");
 
 #define smnMP1_FIRMWARE_FLAGS 0x3010028
 
-static uint32_t sos_old_versions[] = {1517616, 1510592, 1448594, 1446554};
-
 static int psp_v3_1_ring_stop(struct psp_context *psp,
 			      enum psp_ring_type ring_type);
 
@@ -125,31 +123,12 @@ static int psp_v3_1_bootloader_load_sysdrv(struct psp_context *psp)
 	return ret;
 }
 
-static bool psp_v3_1_match_version(struct amdgpu_device *adev, uint32_t ver)
-{
-	int i;
-
-	if (ver == adev->psp.sos_fw_version)
-		return true;
-
-	/*
-	 * Double check if the latest four legacy versions.
-	 * If yes, it is still the right version.
-	 */
-	for (i = 0; i < ARRAY_SIZE(sos_old_versions); i++) {
-		if (sos_old_versions[i] == adev->psp.sos_fw_version)
-			return true;
-	}
-
-	return false;
-}
-
 static int psp_v3_1_bootloader_load_sos(struct psp_context *psp)
 {
 	int ret;
 	unsigned int psp_gfxdrv_command_reg = 0;
 	struct amdgpu_device *adev = psp->adev;
-	uint32_t sol_reg, ver;
+	uint32_t sol_reg;
 
 	/* Check sOS sign of life register to confirm sys driver and sOS
 	 * are already been loaded.
@@ -181,11 +160,6 @@ static int psp_v3_1_bootloader_load_sos(struct psp_context *psp)
 	ret = psp_wait_for(psp, SOC15_REG_OFFSET(MP0, 0, mmMP0_SMN_C2PMSG_81),
 			   RREG32_SOC15(MP0, 0, mmMP0_SMN_C2PMSG_81),
 			   0, true);
-
-	ver = RREG32_SOC15(MP0, 0, mmMP0_SMN_C2PMSG_58);
-	if (!psp_v3_1_match_version(adev, ver))
-		DRM_WARN("SOS version doesn't match\n");
-
 	return ret;
 }
 
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH 0/8] psp code clean up

[Chen, Guchun]

[AMD Public Use]

The series is: Reviewed-by: Guchun Chen <guchun.chen@amd.com>

Regards,
Guchun

-----Original Message-----
From: Zhang, Hawking <Hawking.Zhang@amd.com> 
Sent: Monday, April 20, 2020 6:17 PM
To: amd-gfx@lists.freedesktop.org; Deucher, Alexander <Alexander.Deucher@amd.com>; Clements, John <John.Clements@amd.com>; Chen, Guchun <Guchun.Chen@amd.com>
Cc: Zhang, Hawking <Hawking.Zhang@amd.com>
Subject: [PATCH 0/8] psp code clean up

the seires make cleanup in current software psp support, including:
1). retire unnecessary ip callback function like support_vmr_ring, check_fw_loading.
2). remove unnecessary tOS version check 3). create common helper functions to avoid duplicate code per IP generation

Hawking Zhang (8):
  drm/amdgpu: retire support_vmr_ring interface
  drm/amdgpu: remove unnecessary tOS version check
  drm/amdgpu: retire unused check_fw_loading status
  drm/amdgpu: add helper function to init asd ucode
  drm/amdgpu: switch to helper function to init asd ucode
  drm/amdgpu: add helper function to init sos ucode
  drm/amdgpu: switch to helper function to init sos ucode
  drm/amdgpu: retire legacy vega10 sos version check

 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 130 ++++++++++++----  drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h |  14 +-  drivers/gpu/drm/amd/amdgpu/psp_v10_0.c  | 141 +----------------  drivers/gpu/drm/amd/amdgpu/psp_v11_0.c  | 235 ++--------------------------  drivers/gpu/drm/amd/amdgpu/psp_v12_0.c  | 172 +--------------------
 drivers/gpu/drm/amd/amdgpu/psp_v3_1.c   | 262 +++-----------------------------
 6 files changed, 149 insertions(+), 805 deletions(-)

--
2.7.4
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH 0/8] psp code clean up

[Clements, John]

[AMD Public Use]

The series is: Reviewed-by: John Clements <john.clements@amd.com>

-----Original Message-----
From: Chen, Guchun <Guchun.Chen@amd.com> 
Sent: Monday, April 20, 2020 8:56 PM
To: Zhang, Hawking <Hawking.Zhang@amd.com>; amd-gfx@lists.freedesktop.org; Deucher, Alexander <Alexander.Deucher@amd.com>; Clements, John <John.Clements@amd.com>
Subject: RE: [PATCH 0/8] psp code clean up

[AMD Public Use]

The series is: Reviewed-by: Guchun Chen <guchun.chen@amd.com>

Regards,
Guchun

-----Original Message-----
From: Zhang, Hawking <Hawking.Zhang@amd.com> 
Sent: Monday, April 20, 2020 6:17 PM
To: amd-gfx@lists.freedesktop.org; Deucher, Alexander <Alexander.Deucher@amd.com>; Clements, John <John.Clements@amd.com>; Chen, Guchun <Guchun.Chen@amd.com>
Cc: Zhang, Hawking <Hawking.Zhang@amd.com>
Subject: [PATCH 0/8] psp code clean up

the seires make cleanup in current software psp support, including:
1). retire unnecessary ip callback function like support_vmr_ring, check_fw_loading.
2). remove unnecessary tOS version check 3). create common helper functions to avoid duplicate code per IP generation

Hawking Zhang (8):
  drm/amdgpu: retire support_vmr_ring interface
  drm/amdgpu: remove unnecessary tOS version check
  drm/amdgpu: retire unused check_fw_loading status
  drm/amdgpu: add helper function to init asd ucode
  drm/amdgpu: switch to helper function to init asd ucode
  drm/amdgpu: add helper function to init sos ucode
  drm/amdgpu: switch to helper function to init sos ucode
  drm/amdgpu: retire legacy vega10 sos version check

 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 130 ++++++++++++----  drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h |  14 +-  drivers/gpu/drm/amd/amdgpu/psp_v10_0.c  | 141 +----------------  drivers/gpu/drm/amd/amdgpu/psp_v11_0.c  | 235 ++--------------------------  drivers/gpu/drm/amd/amdgpu/psp_v12_0.c  | 172 +--------------------
 drivers/gpu/drm/amd/amdgpu/psp_v3_1.c   | 262 +++-----------------------------
 6 files changed, 149 insertions(+), 805 deletions(-)

--
2.7.4
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx