* [PATCH 1/5] drm/amdgpu: Split amdgpu_ucode_init/fini_bo into two functions
@ 2018-10-09 12:43 Rex Zhu
[not found] ` <1539089023-2360-1-git-send-email-Rex.Zhu-5C7GfCeVMHo@public.gmane.org>
0 siblings, 1 reply; 13+ messages in thread
From: Rex Zhu @ 2018-10-09 12:43 UTC (permalink / raw)
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW; +Cc: Rex Zhu
1. one is for create/free bo when init/fini
2. one is for fill the bo before fw loading
the ucode bo only need to be created when load driver
and free when driver unload.
when resume/reset, driver only need to re-fill the bo
if the bo is allocated in vram.
Suggested by Christian.
Signed-off-by: Rex Zhu <Rex.Zhu@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 3 ++
drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c | 57 +++++++++++++++---------------
drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h | 3 ++
3 files changed, 34 insertions(+), 29 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 94c92f5..4787571 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -1581,6 +1581,8 @@ static int amdgpu_device_ip_init(struct amdgpu_device *adev)
}
}
+ amdgpu_ucode_create_bo(adev); /* create ucode bo when sw_init complete*/
+
for (i = 0; i < adev->num_ip_blocks; i++) {
if (!adev->ip_blocks[i].status.sw)
continue;
@@ -1803,6 +1805,7 @@ static int amdgpu_device_ip_fini(struct amdgpu_device *adev)
continue;
if (adev->ip_blocks[i].version->type == AMD_IP_BLOCK_TYPE_GMC) {
+ amdgpu_ucode_free_bo(adev);
amdgpu_free_static_csa(adev);
amdgpu_device_wb_fini(adev);
amdgpu_device_vram_scratch_fini(adev);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
index adfeb93..7b6b2f4c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
@@ -422,32 +422,41 @@ static int amdgpu_ucode_patch_jt(struct amdgpu_firmware_info *ucode,
return 0;
}
+void amdgpu_ucode_create_bo(struct amdgpu_device *adev)
+{
+ if (adev->firmware.load_type != AMDGPU_FW_LOAD_DIRECT) {
+ amdgpu_bo_create_kernel(adev, adev->firmware.fw_size, PAGE_SIZE,
+ amdgpu_sriov_vf(adev) ? AMDGPU_GEM_DOMAIN_VRAM : AMDGPU_GEM_DOMAIN_GTT,
+ &adev->firmware.fw_buf,
+ &adev->firmware.fw_buf_mc,
+ &adev->firmware.fw_buf_ptr);
+ if (!adev->firmware.fw_buf) {
+ dev_err(adev->dev, "failed to create kernel buffer for firmware.fw_buf\n");
+ adev->firmware.load_type = AMDGPU_FW_LOAD_DIRECT;
+ } else if (amdgpu_sriov_vf(adev)) {
+ memset(adev->firmware.fw_buf_ptr, 0, adev->firmware.fw_size);
+ }
+ }
+}
+
+void amdgpu_ucode_free_bo(struct amdgpu_device *adev)
+{
+ if (adev->firmware.load_type != AMDGPU_FW_LOAD_DIRECT)
+ amdgpu_bo_free_kernel(&adev->firmware.fw_buf,
+ &adev->firmware.fw_buf_mc,
+ &adev->firmware.fw_buf_ptr);
+}
+
int amdgpu_ucode_init_bo(struct amdgpu_device *adev)
{
uint64_t fw_offset = 0;
- int i, err;
+ int i;
struct amdgpu_firmware_info *ucode = NULL;
const struct common_firmware_header *header = NULL;
- if (!adev->firmware.fw_size) {
- dev_warn(adev->dev, "No ip firmware need to load\n");
+ /* for baremetal, the ucode is allocated in gtt, so don't need to fill the bo when reset/suspend */
+ if (!amdgpu_sriov_vf(adev) && (adev->in_gpu_reset || adev->in_suspend))
return 0;
- }
-
- if (!adev->in_gpu_reset && !adev->in_suspend) {
- err = amdgpu_bo_create_kernel(adev, adev->firmware.fw_size, PAGE_SIZE,
- amdgpu_sriov_vf(adev) ? AMDGPU_GEM_DOMAIN_VRAM : AMDGPU_GEM_DOMAIN_GTT,
- &adev->firmware.fw_buf,
- &adev->firmware.fw_buf_mc,
- &adev->firmware.fw_buf_ptr);
- if (err) {
- dev_err(adev->dev, "failed to create kernel buffer for firmware.fw_buf\n");
- goto failed;
- }
- }
-
- memset(adev->firmware.fw_buf_ptr, 0, adev->firmware.fw_size);
-
/*
* if SMU loaded firmware, it needn't add SMC, UVD, and VCE
* ucode info here
@@ -479,12 +488,6 @@ int amdgpu_ucode_init_bo(struct amdgpu_device *adev)
}
}
return 0;
-
-failed:
- if (err)
- adev->firmware.load_type = AMDGPU_FW_LOAD_DIRECT;
-
- return err;
}
int amdgpu_ucode_fini_bo(struct amdgpu_device *adev)
@@ -503,9 +506,5 @@ int amdgpu_ucode_fini_bo(struct amdgpu_device *adev)
}
}
- amdgpu_bo_free_kernel(&adev->firmware.fw_buf,
- &adev->firmware.fw_buf_mc,
- &adev->firmware.fw_buf_ptr);
-
return 0;
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
index 8f3f111..4c0e5be 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
@@ -279,6 +279,9 @@ bool amdgpu_ucode_hdr_version(union amdgpu_firmware_header *hdr,
int amdgpu_ucode_init_bo(struct amdgpu_device *adev);
int amdgpu_ucode_fini_bo(struct amdgpu_device *adev);
+void amdgpu_ucode_create_bo(struct amdgpu_device *adev);
+void amdgpu_ucode_free_bo(struct amdgpu_device *adev);
+
enum amdgpu_firmware_load_type
amdgpu_ucode_get_load_type(struct amdgpu_device *adev, int load_type);
--
1.9.1
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 2/5] drm/amdgpu: Remove amdgpu_ucode_fini_bo
[not found] ` <1539089023-2360-1-git-send-email-Rex.Zhu-5C7GfCeVMHo@public.gmane.org>
@ 2018-10-09 12:43 ` Rex Zhu
[not found] ` <1539089023-2360-2-git-send-email-Rex.Zhu-5C7GfCeVMHo@public.gmane.org>
2018-10-09 12:43 ` [PATCH 3/5] drm/amdgpu: Extract the function of fw loading out of powerplay Rex Zhu
` (3 subsequent siblings)
4 siblings, 1 reply; 13+ messages in thread
From: Rex Zhu @ 2018-10-09 12:43 UTC (permalink / raw)
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW; +Cc: Rex Zhu
The variable clean is unnecessary.
Signed-off-by: Rex Zhu <Rex.Zhu@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 2 --
drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c | 19 -------------------
drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h | 3 +--
drivers/gpu/drm/amd/powerplay/amd_powerplay.c | 3 ---
4 files changed, 1 insertion(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
index bd397d2..25d2f3e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
@@ -452,8 +452,6 @@ static int psp_hw_fini(void *handle)
if (adev->firmware.load_type != AMDGPU_FW_LOAD_PSP)
return 0;
- amdgpu_ucode_fini_bo(adev);
-
psp_ring_destroy(psp, PSP_RING_TYPE__KM);
amdgpu_bo_free_kernel(&psp->tmr_bo, &psp->tmr_mc_addr, &psp->tmr_buf);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
index 7b6b2f4c..f2604ac 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
@@ -489,22 +489,3 @@ int amdgpu_ucode_init_bo(struct amdgpu_device *adev)
}
return 0;
}
-
-int amdgpu_ucode_fini_bo(struct amdgpu_device *adev)
-{
- int i;
- struct amdgpu_firmware_info *ucode = NULL;
-
- if (!adev->firmware.fw_size)
- return 0;
-
- for (i = 0; i < adev->firmware.max_ucodes; i++) {
- ucode = &adev->firmware.ucode[i];
- if (ucode->fw) {
- ucode->mc_addr = 0;
- ucode->kaddr = NULL;
- }
- }
-
- return 0;
-}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
index 4c0e5be..05a2c46 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
@@ -276,9 +276,8 @@ struct amdgpu_firmware {
int amdgpu_ucode_validate(const struct firmware *fw);
bool amdgpu_ucode_hdr_version(union amdgpu_firmware_header *hdr,
uint16_t hdr_major, uint16_t hdr_minor);
-int amdgpu_ucode_init_bo(struct amdgpu_device *adev);
-int amdgpu_ucode_fini_bo(struct amdgpu_device *adev);
+int amdgpu_ucode_init_bo(struct amdgpu_device *adev);
void amdgpu_ucode_create_bo(struct amdgpu_device *adev);
void amdgpu_ucode_free_bo(struct amdgpu_device *adev);
diff --git a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
index 6bc8e9c..75b56ae 100644
--- a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
+++ b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
@@ -109,9 +109,6 @@ static int pp_sw_fini(void *handle)
hwmgr_sw_fini(hwmgr);
- if (adev->firmware.load_type == AMDGPU_FW_LOAD_SMU)
- amdgpu_ucode_fini_bo(adev);
-
release_firmware(adev->pm.fw);
adev->pm.fw = NULL;
--
1.9.1
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 3/5] drm/amdgpu: Extract the function of fw loading out of powerplay
[not found] ` <1539089023-2360-1-git-send-email-Rex.Zhu-5C7GfCeVMHo@public.gmane.org>
2018-10-09 12:43 ` [PATCH 2/5] drm/amdgpu: Remove amdgpu_ucode_fini_bo Rex Zhu
@ 2018-10-09 12:43 ` Rex Zhu
[not found] ` <1539089023-2360-3-git-send-email-Rex.Zhu-5C7GfCeVMHo@public.gmane.org>
2018-10-09 12:43 ` [PATCH 4/5] drm/amdgpu: Fix unnecessary warning in dmesg Rex Zhu
` (2 subsequent siblings)
4 siblings, 1 reply; 13+ messages in thread
From: Rex Zhu @ 2018-10-09 12:43 UTC (permalink / raw)
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW; +Cc: Rex Zhu
So there is no dependence between gfx/sdma/smu.
and for Vi, after IH hw_init, driver load all the smu/gfx/sdma
fw. for AI, fw loading is controlled by PSP, after psp hw init,
we call the function to check smu fw version.
Signed-off-by: Rex Zhu <Rex.Zhu@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 30 ++++++++++++++++++++++
drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 11 --------
drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c | 8 ------
drivers/gpu/drm/amd/powerplay/hwmgr/hwmgr.c | 20 ---------------
drivers/gpu/drm/amd/powerplay/inc/hwmgr.h | 1 -
drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.c | 8 ++----
drivers/gpu/drm/amd/powerplay/smumgr/smu8_smumgr.c | 5 ----
7 files changed, 32 insertions(+), 51 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 4787571..a6766b3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -1525,6 +1525,24 @@ static int amdgpu_device_ip_early_init(struct amdgpu_device *adev)
return 0;
}
+static int amdgpu_device_fw_loading(struct amdgpu_device *adev, uint32_t index)
+{
+ int r = 0;
+
+ if ((adev->asic_type < CHIP_VEGA10
+ && (adev->ip_blocks[index].version->type == AMD_IP_BLOCK_TYPE_IH))
+ || (adev->asic_type >= CHIP_VEGA10
+ && (adev->ip_blocks[index].version->type == AMD_IP_BLOCK_TYPE_PSP))) {
+ if (adev->powerplay.pp_funcs->load_firmware) {
+ r = adev->powerplay.pp_funcs->load_firmware(adev->powerplay.pp_handle);
+ if (r) {
+ pr_err("firmware loading failed\n");
+ return r;
+ }
+ }
+ }
+ return 0;
+}
/**
* amdgpu_device_ip_init - run init for hardware IPs
*
@@ -1595,6 +1613,9 @@ static int amdgpu_device_ip_init(struct amdgpu_device *adev)
return r;
}
adev->ip_blocks[i].status.hw = true;
+ r = amdgpu_device_fw_loading(adev, i);
+ if (r)
+ return r;
}
amdgpu_xgmi_add_device(adev);
@@ -2030,6 +2051,9 @@ static int amdgpu_device_ip_reinit_early_sriov(struct amdgpu_device *adev)
DRM_INFO("RE-INIT: %s %s\n", block->version->funcs->name, r?"failed":"succeeded");
if (r)
return r;
+ r = amdgpu_device_fw_loading(adev, i);
+ if (r)
+ return r;
}
}
@@ -2098,6 +2122,9 @@ static int amdgpu_device_ip_resume_phase1(struct amdgpu_device *adev)
adev->ip_blocks[i].version->funcs->name, r);
return r;
}
+ r = amdgpu_device_fw_loading(adev, i);
+ if (r)
+ return r;
}
}
@@ -2134,6 +2161,9 @@ static int amdgpu_device_ip_resume_phase2(struct amdgpu_device *adev)
adev->ip_blocks[i].version->funcs->name, r);
return r;
}
+ r = amdgpu_device_fw_loading(adev, i);
+ if (r)
+ return r;
}
return 0;
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
index 8439f9a..3d0f277 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
@@ -4175,20 +4175,9 @@ static void gfx_v8_0_rlc_start(struct amdgpu_device *adev)
static int gfx_v8_0_rlc_resume(struct amdgpu_device *adev)
{
- int r;
-
gfx_v8_0_rlc_stop(adev);
gfx_v8_0_rlc_reset(adev);
gfx_v8_0_init_pg(adev);
-
- if (adev->powerplay.pp_funcs->load_firmware) {
- r = adev->powerplay.pp_funcs->load_firmware(adev->powerplay.pp_handle);
- if (r) {
- pr_err("firmware loading failed\n");
- return r;
- }
- }
-
gfx_v8_0_rlc_start(adev);
return 0;
diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
index 0bdde7f..6fb3eda 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
@@ -788,14 +788,6 @@ static int sdma_v3_0_start(struct amdgpu_device *adev)
{
int r;
- if (adev->powerplay.pp_funcs->load_firmware) {
- r = adev->powerplay.pp_funcs->load_firmware(adev->powerplay.pp_handle);
- if (r) {
- pr_err("firmware loading failed\n");
- return r;
- }
- }
-
/* disable sdma engine before programing it */
sdma_v3_0_ctx_switch_enable(adev, false);
sdma_v3_0_enable(adev, false);
diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/hwmgr.c b/drivers/gpu/drm/amd/powerplay/hwmgr/hwmgr.c
index d552af2..47ac923 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/hwmgr.c
@@ -89,7 +89,6 @@ int hwmgr_early_init(struct pp_hwmgr *hwmgr)
hwmgr_init_default_caps(hwmgr);
hwmgr_set_user_specify_caps(hwmgr);
hwmgr->fan_ctrl_is_in_default_mode = true;
- hwmgr->reload_fw = 1;
hwmgr_init_workload_prority(hwmgr);
switch (hwmgr->chip_family) {
@@ -209,17 +208,6 @@ int hwmgr_hw_init(struct pp_hwmgr *hwmgr)
{
int ret = 0;
- if (!hwmgr || !hwmgr->smumgr_funcs)
- return -EINVAL;
-
- if (hwmgr->smumgr_funcs->start_smu) {
- ret = hwmgr->smumgr_funcs->start_smu(hwmgr);
- if (ret) {
- pr_err("smc start failed\n");
- return -EINVAL;
- }
- }
-
if (!hwmgr->pm_en)
return 0;
@@ -301,7 +289,6 @@ int hwmgr_suspend(struct pp_hwmgr *hwmgr)
if (!hwmgr || !hwmgr->pm_en)
return 0;
- hwmgr->reload_fw = true;
phm_disable_smc_firmware_ctf(hwmgr);
ret = psm_set_boot_states(hwmgr);
if (ret)
@@ -321,13 +308,6 @@ int hwmgr_resume(struct pp_hwmgr *hwmgr)
if (!hwmgr)
return -EINVAL;
- if (hwmgr->smumgr_funcs && hwmgr->smumgr_funcs->start_smu) {
- if (hwmgr->smumgr_funcs->start_smu(hwmgr)) {
- pr_err("smc start failed\n");
- return -EINVAL;
- }
- }
-
if (!hwmgr->pm_en)
return 0;
diff --git a/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h b/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
index 35f2272..e5a60aa 100644
--- a/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
+++ b/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
@@ -734,7 +734,6 @@ struct pp_hwmgr {
void *smu_backend;
const struct pp_smumgr_func *smumgr_funcs;
bool is_kicker;
- bool reload_fw;
enum PP_DAL_POWERLEVEL dal_power_level;
struct phm_dynamic_state_info dyn_state;
diff --git a/drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.c b/drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.c
index 99b4e4f..3f51d54 100644
--- a/drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.c
+++ b/drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.c
@@ -343,9 +343,6 @@ int smu7_request_smu_load_fw(struct pp_hwmgr *hwmgr)
uint32_t fw_to_load;
int r = 0;
- if (!hwmgr->reload_fw)
- return 0;
-
amdgpu_ucode_init_bo(hwmgr->adev);
if (smu_data->soft_regs_start)
@@ -432,10 +429,9 @@ int smu7_request_smu_load_fw(struct pp_hwmgr *hwmgr)
smu7_send_msg_to_smc_with_parameter(hwmgr, PPSMC_MSG_LoadUcodes, fw_to_load);
r = smu7_check_fw_load_finish(hwmgr, fw_to_load);
- if (!r) {
- hwmgr->reload_fw = 0;
+ if (!r)
return 0;
- }
+
pr_err("SMU load firmware failed\n");
failed:
diff --git a/drivers/gpu/drm/amd/powerplay/smumgr/smu8_smumgr.c b/drivers/gpu/drm/amd/powerplay/smumgr/smu8_smumgr.c
index abbf2f2..f836d30 100644
--- a/drivers/gpu/drm/amd/powerplay/smumgr/smu8_smumgr.c
+++ b/drivers/gpu/drm/amd/powerplay/smumgr/smu8_smumgr.c
@@ -661,9 +661,6 @@ static int smu8_request_smu_load_fw(struct pp_hwmgr *hwmgr)
uint32_t fw_to_check = 0;
int ret;
- if (!hwmgr->reload_fw)
- return 0;
-
amdgpu_ucode_init_bo(hwmgr->adev);
smu8_smu_populate_firmware_entries(hwmgr);
@@ -719,8 +716,6 @@ static int smu8_request_smu_load_fw(struct pp_hwmgr *hwmgr)
return ret;
}
- hwmgr->reload_fw = 0;
-
return 0;
}
--
1.9.1
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 4/5] drm/amdgpu: Fix unnecessary warning in dmesg
[not found] ` <1539089023-2360-1-git-send-email-Rex.Zhu-5C7GfCeVMHo@public.gmane.org>
2018-10-09 12:43 ` [PATCH 2/5] drm/amdgpu: Remove amdgpu_ucode_fini_bo Rex Zhu
2018-10-09 12:43 ` [PATCH 3/5] drm/amdgpu: Extract the function of fw loading out of powerplay Rex Zhu
@ 2018-10-09 12:43 ` Rex Zhu
[not found] ` <1539089023-2360-4-git-send-email-Rex.Zhu-5C7GfCeVMHo@public.gmane.org>
2018-10-09 12:43 ` [PATCH 5/5] drm/amdgpu: Remove the direct fw loading support for sdma2.4 Rex Zhu
2018-10-09 13:06 ` [PATCH 1/5] drm/amdgpu: Split amdgpu_ucode_init/fini_bo into two functions Christian König
4 siblings, 1 reply; 13+ messages in thread
From: Rex Zhu @ 2018-10-09 12:43 UTC (permalink / raw)
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW; +Cc: Rex Zhu
Fix the warning message:
"-1 is not supported on VI"
the -1 is the default fw load type, mean auto.
Signed-off-by: Rex Zhu <Rex.Zhu@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
index f2604ac..e5b13b2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
@@ -297,7 +297,7 @@ enum amdgpu_firmware_load_type
case CHIP_POLARIS11:
case CHIP_POLARIS12:
case CHIP_VEGAM:
- if (load_type != AMDGPU_FW_LOAD_SMU)
+ if (load_type != AMDGPU_FW_LOAD_DIRECT || load_type == AMDGPU_FW_LOAD_PSP)
pr_warning("%d is not supported on VI\n", load_type);
return AMDGPU_FW_LOAD_SMU;
case CHIP_VEGA10:
--
1.9.1
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 5/5] drm/amdgpu: Remove the direct fw loading support for sdma2.4
[not found] ` <1539089023-2360-1-git-send-email-Rex.Zhu-5C7GfCeVMHo@public.gmane.org>
` (2 preceding siblings ...)
2018-10-09 12:43 ` [PATCH 4/5] drm/amdgpu: Fix unnecessary warning in dmesg Rex Zhu
@ 2018-10-09 12:43 ` Rex Zhu
[not found] ` <1539089023-2360-5-git-send-email-Rex.Zhu-5C7GfCeVMHo@public.gmane.org>
2018-10-09 13:06 ` [PATCH 1/5] drm/amdgpu: Split amdgpu_ucode_init/fini_bo into two functions Christian König
4 siblings, 1 reply; 13+ messages in thread
From: Rex Zhu @ 2018-10-09 12:43 UTC (permalink / raw)
To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW; +Cc: Rex Zhu
sdma2.4 is only for iceland. For Vi, we don't maintain the
direct fw loading.
Signed-off-by: Rex Zhu <Rex.Zhu@amd.com>
---
drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c | 42 ----------------------------------
1 file changed, 42 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c b/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
index cd781ab..2d4770e 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
@@ -504,41 +504,6 @@ static int sdma_v2_4_rlc_resume(struct amdgpu_device *adev)
return 0;
}
-/**
- * sdma_v2_4_load_microcode - load the sDMA ME ucode
- *
- * @adev: amdgpu_device pointer
- *
- * Loads the sDMA0/1 ucode.
- * Returns 0 for success, -EINVAL if the ucode is not available.
- */
-static int sdma_v2_4_load_microcode(struct amdgpu_device *adev)
-{
- const struct sdma_firmware_header_v1_0 *hdr;
- const __le32 *fw_data;
- u32 fw_size;
- int i, j;
-
- /* halt the MEs */
- sdma_v2_4_enable(adev, false);
-
- for (i = 0; i < adev->sdma.num_instances; i++) {
- if (!adev->sdma.instance[i].fw)
- return -EINVAL;
- hdr = (const struct sdma_firmware_header_v1_0 *)adev->sdma.instance[i].fw->data;
- amdgpu_ucode_print_sdma_hdr(&hdr->header);
- fw_size = le32_to_cpu(hdr->header.ucode_size_bytes) / 4;
- fw_data = (const __le32 *)
- (adev->sdma.instance[i].fw->data +
- le32_to_cpu(hdr->header.ucode_array_offset_bytes));
- WREG32(mmSDMA0_UCODE_ADDR + sdma_offsets[i], 0);
- for (j = 0; j < fw_size; j++)
- WREG32(mmSDMA0_UCODE_DATA + sdma_offsets[i], le32_to_cpup(fw_data++));
- WREG32(mmSDMA0_UCODE_ADDR + sdma_offsets[i], adev->sdma.instance[i].fw_version);
- }
-
- return 0;
-}
/**
* sdma_v2_4_start - setup and start the async dma engines
@@ -552,13 +517,6 @@ static int sdma_v2_4_start(struct amdgpu_device *adev)
{
int r;
-
- if (adev->firmware.load_type == AMDGPU_FW_LOAD_DIRECT) {
- r = sdma_v2_4_load_microcode(adev);
- if (r)
- return r;
- }
-
/* halt the engine before programing */
sdma_v2_4_enable(adev, false);
--
1.9.1
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 1/5] drm/amdgpu: Split amdgpu_ucode_init/fini_bo into two functions
[not found] ` <1539089023-2360-1-git-send-email-Rex.Zhu-5C7GfCeVMHo@public.gmane.org>
` (3 preceding siblings ...)
2018-10-09 12:43 ` [PATCH 5/5] drm/amdgpu: Remove the direct fw loading support for sdma2.4 Rex Zhu
@ 2018-10-09 13:06 ` Christian König
4 siblings, 0 replies; 13+ messages in thread
From: Christian König @ 2018-10-09 13:06 UTC (permalink / raw)
To: Rex Zhu, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Am 09.10.2018 um 14:43 schrieb Rex Zhu:
> 1. one is for create/free bo when init/fini
> 2. one is for fill the bo before fw loading
>
> the ucode bo only need to be created when load driver
> and free when driver unload.
>
> when resume/reset, driver only need to re-fill the bo
> if the bo is allocated in vram.
>
> Suggested by Christian.
>
> Signed-off-by: Rex Zhu <Rex.Zhu@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 3 ++
> drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c | 57 +++++++++++++++---------------
> drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h | 3 ++
> 3 files changed, 34 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 94c92f5..4787571 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -1581,6 +1581,8 @@ static int amdgpu_device_ip_init(struct amdgpu_device *adev)
> }
> }
>
> + amdgpu_ucode_create_bo(adev); /* create ucode bo when sw_init complete*/
> +
> for (i = 0; i < adev->num_ip_blocks; i++) {
> if (!adev->ip_blocks[i].status.sw)
> continue;
> @@ -1803,6 +1805,7 @@ static int amdgpu_device_ip_fini(struct amdgpu_device *adev)
> continue;
>
> if (adev->ip_blocks[i].version->type == AMD_IP_BLOCK_TYPE_GMC) {
> + amdgpu_ucode_free_bo(adev);
> amdgpu_free_static_csa(adev);
> amdgpu_device_wb_fini(adev);
> amdgpu_device_vram_scratch_fini(adev);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
> index adfeb93..7b6b2f4c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
> @@ -422,32 +422,41 @@ static int amdgpu_ucode_patch_jt(struct amdgpu_firmware_info *ucode,
> return 0;
> }
>
> +void amdgpu_ucode_create_bo(struct amdgpu_device *adev)
> +{
> + if (adev->firmware.load_type != AMDGPU_FW_LOAD_DIRECT) {
Better use early bailout here, e.g. in this case a return statement.
Apart from that looks good to me,
Christian.
> + amdgpu_bo_create_kernel(adev, adev->firmware.fw_size, PAGE_SIZE,
> + amdgpu_sriov_vf(adev) ? AMDGPU_GEM_DOMAIN_VRAM : AMDGPU_GEM_DOMAIN_GTT,
> + &adev->firmware.fw_buf,
> + &adev->firmware.fw_buf_mc,
> + &adev->firmware.fw_buf_ptr);
> + if (!adev->firmware.fw_buf) {
> + dev_err(adev->dev, "failed to create kernel buffer for firmware.fw_buf\n");
> + adev->firmware.load_type = AMDGPU_FW_LOAD_DIRECT;
> + } else if (amdgpu_sriov_vf(adev)) {
> + memset(adev->firmware.fw_buf_ptr, 0, adev->firmware.fw_size);
> + }
> + }
> +}
> +
> +void amdgpu_ucode_free_bo(struct amdgpu_device *adev)
> +{
> + if (adev->firmware.load_type != AMDGPU_FW_LOAD_DIRECT)
> + amdgpu_bo_free_kernel(&adev->firmware.fw_buf,
> + &adev->firmware.fw_buf_mc,
> + &adev->firmware.fw_buf_ptr);
> +}
> +
> int amdgpu_ucode_init_bo(struct amdgpu_device *adev)
> {
> uint64_t fw_offset = 0;
> - int i, err;
> + int i;
> struct amdgpu_firmware_info *ucode = NULL;
> const struct common_firmware_header *header = NULL;
>
> - if (!adev->firmware.fw_size) {
> - dev_warn(adev->dev, "No ip firmware need to load\n");
> + /* for baremetal, the ucode is allocated in gtt, so don't need to fill the bo when reset/suspend */
> + if (!amdgpu_sriov_vf(adev) && (adev->in_gpu_reset || adev->in_suspend))
> return 0;
> - }
> -
> - if (!adev->in_gpu_reset && !adev->in_suspend) {
> - err = amdgpu_bo_create_kernel(adev, adev->firmware.fw_size, PAGE_SIZE,
> - amdgpu_sriov_vf(adev) ? AMDGPU_GEM_DOMAIN_VRAM : AMDGPU_GEM_DOMAIN_GTT,
> - &adev->firmware.fw_buf,
> - &adev->firmware.fw_buf_mc,
> - &adev->firmware.fw_buf_ptr);
> - if (err) {
> - dev_err(adev->dev, "failed to create kernel buffer for firmware.fw_buf\n");
> - goto failed;
> - }
> - }
> -
> - memset(adev->firmware.fw_buf_ptr, 0, adev->firmware.fw_size);
> -
> /*
> * if SMU loaded firmware, it needn't add SMC, UVD, and VCE
> * ucode info here
> @@ -479,12 +488,6 @@ int amdgpu_ucode_init_bo(struct amdgpu_device *adev)
> }
> }
> return 0;
> -
> -failed:
> - if (err)
> - adev->firmware.load_type = AMDGPU_FW_LOAD_DIRECT;
> -
> - return err;
> }
>
> int amdgpu_ucode_fini_bo(struct amdgpu_device *adev)
> @@ -503,9 +506,5 @@ int amdgpu_ucode_fini_bo(struct amdgpu_device *adev)
> }
> }
>
> - amdgpu_bo_free_kernel(&adev->firmware.fw_buf,
> - &adev->firmware.fw_buf_mc,
> - &adev->firmware.fw_buf_ptr);
> -
> return 0;
> }
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
> index 8f3f111..4c0e5be 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
> @@ -279,6 +279,9 @@ bool amdgpu_ucode_hdr_version(union amdgpu_firmware_header *hdr,
> int amdgpu_ucode_init_bo(struct amdgpu_device *adev);
> int amdgpu_ucode_fini_bo(struct amdgpu_device *adev);
>
> +void amdgpu_ucode_create_bo(struct amdgpu_device *adev);
> +void amdgpu_ucode_free_bo(struct amdgpu_device *adev);
> +
> enum amdgpu_firmware_load_type
> amdgpu_ucode_get_load_type(struct amdgpu_device *adev, int load_type);
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 4/5] drm/amdgpu: Fix unnecessary warning in dmesg
[not found] ` <1539089023-2360-4-git-send-email-Rex.Zhu-5C7GfCeVMHo@public.gmane.org>
@ 2018-10-09 13:18 ` Michel Dänzer
[not found] ` <15af5a1b-ea73-c779-5f30-e6da0192d504-otUistvHUpPR7s880joybQ@public.gmane.org>
0 siblings, 1 reply; 13+ messages in thread
From: Michel Dänzer @ 2018-10-09 13:18 UTC (permalink / raw)
To: Rex Zhu; +Cc: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
On 2018-10-09 2:43 p.m., Rex Zhu wrote:
> Fix the warning message:
> "-1 is not supported on VI"
> the -1 is the default fw load type, mean auto.
>
> Signed-off-by: Rex Zhu <Rex.Zhu@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
> index f2604ac..e5b13b2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
> @@ -297,7 +297,7 @@ enum amdgpu_firmware_load_type
> case CHIP_POLARIS11:
> case CHIP_POLARIS12:
> case CHIP_VEGAM:
> - if (load_type != AMDGPU_FW_LOAD_SMU)
> + if (load_type != AMDGPU_FW_LOAD_DIRECT || load_type == AMDGPU_FW_LOAD_PSP)
AMDGPU_FW_LOAD_PSP == 2 != AMDGPU_FW_LOAD_DIRECT == 0
I suspect you meant something else here.
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH 4/5] drm/amdgpu: Fix unnecessary warning in dmesg
[not found] ` <15af5a1b-ea73-c779-5f30-e6da0192d504-otUistvHUpPR7s880joybQ@public.gmane.org>
@ 2018-10-09 14:42 ` Zhu, Rex
0 siblings, 0 replies; 13+ messages in thread
From: Zhu, Rex @ 2018-10-09 14:42 UTC (permalink / raw)
To: Michel Dänzer; +Cc: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
> -----Original Message-----
> From: Michel Dänzer <michel@daenzer.net>
> Sent: Tuesday, October 9, 2018 9:19 PM
> To: Zhu, Rex <Rex.Zhu@amd.com>
> Cc: amd-gfx@lists.freedesktop.org
> Subject: Re: [PATCH 4/5] drm/amdgpu: Fix unnecessary warning in dmesg
>
> On 2018-10-09 2:43 p.m., Rex Zhu wrote:
> > Fix the warning message:
> > "-1 is not supported on VI"
> > the -1 is the default fw load type, mean auto.
> >
> > Signed-off-by: Rex Zhu <Rex.Zhu@amd.com>
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
> > index f2604ac..e5b13b2 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
> > @@ -297,7 +297,7 @@ enum amdgpu_firmware_load_type
> > case CHIP_POLARIS11:
> > case CHIP_POLARIS12:
> > case CHIP_VEGAM:
> > - if (load_type != AMDGPU_FW_LOAD_SMU)
> > + if (load_type != AMDGPU_FW_LOAD_DIRECT || load_type ==
> AMDGPU_FW_LOAD_PSP)
>
> AMDGPU_FW_LOAD_PSP == 2 != AMDGPU_FW_LOAD_DIRECT == 0
>
> I suspect you meant something else here.
Yes, it should be
- if (load_type != AMDGPU_FW_LOAD_SMU)
+ if (load_type == AMDGPU_FW_LOAD_DIRECT || load_type == AMDGPU_FW_LOAD_PSP)
Typo when split the patches.
Thanks for pointing it out.
Best Regards
Rex
>
> --
> Earthling Michel Dänzer | http://www.amd.com
> Libre software enthusiast | Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/5] drm/amdgpu: Remove amdgpu_ucode_fini_bo
[not found] ` <1539089023-2360-2-git-send-email-Rex.Zhu-5C7GfCeVMHo@public.gmane.org>
@ 2018-10-09 18:54 ` Alex Deucher
0 siblings, 0 replies; 13+ messages in thread
From: Alex Deucher @ 2018-10-09 18:54 UTC (permalink / raw)
To: Rex Zhu; +Cc: amd-gfx list
On Tue, Oct 9, 2018 at 8:44 AM Rex Zhu <Rex.Zhu@amd.com> wrote:
>
> The variable clean is unnecessary.
>
> Signed-off-by: Rex Zhu <Rex.Zhu@amd.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 2 --
> drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c | 19 -------------------
> drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h | 3 +--
> drivers/gpu/drm/amd/powerplay/amd_powerplay.c | 3 ---
> 4 files changed, 1 insertion(+), 26 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> index bd397d2..25d2f3e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> @@ -452,8 +452,6 @@ static int psp_hw_fini(void *handle)
> if (adev->firmware.load_type != AMDGPU_FW_LOAD_PSP)
> return 0;
>
> - amdgpu_ucode_fini_bo(adev);
> -
> psp_ring_destroy(psp, PSP_RING_TYPE__KM);
>
> amdgpu_bo_free_kernel(&psp->tmr_bo, &psp->tmr_mc_addr, &psp->tmr_buf);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
> index 7b6b2f4c..f2604ac 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
> @@ -489,22 +489,3 @@ int amdgpu_ucode_init_bo(struct amdgpu_device *adev)
> }
> return 0;
> }
> -
> -int amdgpu_ucode_fini_bo(struct amdgpu_device *adev)
> -{
> - int i;
> - struct amdgpu_firmware_info *ucode = NULL;
> -
> - if (!adev->firmware.fw_size)
> - return 0;
> -
> - for (i = 0; i < adev->firmware.max_ucodes; i++) {
> - ucode = &adev->firmware.ucode[i];
> - if (ucode->fw) {
> - ucode->mc_addr = 0;
> - ucode->kaddr = NULL;
> - }
> - }
> -
> - return 0;
> -}
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
> index 4c0e5be..05a2c46 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
> @@ -276,9 +276,8 @@ struct amdgpu_firmware {
> int amdgpu_ucode_validate(const struct firmware *fw);
> bool amdgpu_ucode_hdr_version(union amdgpu_firmware_header *hdr,
> uint16_t hdr_major, uint16_t hdr_minor);
> -int amdgpu_ucode_init_bo(struct amdgpu_device *adev);
> -int amdgpu_ucode_fini_bo(struct amdgpu_device *adev);
>
> +int amdgpu_ucode_init_bo(struct amdgpu_device *adev);
> void amdgpu_ucode_create_bo(struct amdgpu_device *adev);
> void amdgpu_ucode_free_bo(struct amdgpu_device *adev);
>
> diff --git a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
> index 6bc8e9c..75b56ae 100644
> --- a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
> +++ b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
> @@ -109,9 +109,6 @@ static int pp_sw_fini(void *handle)
>
> hwmgr_sw_fini(hwmgr);
>
> - if (adev->firmware.load_type == AMDGPU_FW_LOAD_SMU)
> - amdgpu_ucode_fini_bo(adev);
> -
> release_firmware(adev->pm.fw);
> adev->pm.fw = NULL;
>
> --
> 1.9.1
>
> _______________________________________________
> 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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/5] drm/amdgpu: Extract the function of fw loading out of powerplay
[not found] ` <1539089023-2360-3-git-send-email-Rex.Zhu-5C7GfCeVMHo@public.gmane.org>
@ 2018-10-09 19:27 ` Alex Deucher
[not found] ` <CADnq5_NwMRLpqr12KtYt5j67vQfd-5pKN=b8UAU9=G5Nsc_mbA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
0 siblings, 1 reply; 13+ messages in thread
From: Alex Deucher @ 2018-10-09 19:27 UTC (permalink / raw)
To: Rex Zhu; +Cc: amd-gfx list
On Tue, Oct 9, 2018 at 8:45 AM Rex Zhu <Rex.Zhu@amd.com> wrote:
>
> So there is no dependence between gfx/sdma/smu.
> and for Vi, after IH hw_init, driver load all the smu/gfx/sdma
> fw. for AI, fw loading is controlled by PSP, after psp hw init,
> we call the function to check smu fw version.
>
> Signed-off-by: Rex Zhu <Rex.Zhu@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 30 ++++++++++++++++++++++
> drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 11 --------
> drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c | 8 ------
> drivers/gpu/drm/amd/powerplay/hwmgr/hwmgr.c | 20 ---------------
> drivers/gpu/drm/amd/powerplay/inc/hwmgr.h | 1 -
> drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.c | 8 ++----
> drivers/gpu/drm/amd/powerplay/smumgr/smu8_smumgr.c | 5 ----
> 7 files changed, 32 insertions(+), 51 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 4787571..a6766b3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -1525,6 +1525,24 @@ static int amdgpu_device_ip_early_init(struct amdgpu_device *adev)
> return 0;
> }
>
> +static int amdgpu_device_fw_loading(struct amdgpu_device *adev, uint32_t index)
> +{
> + int r = 0;
> +
> + if ((adev->asic_type < CHIP_VEGA10
> + && (adev->ip_blocks[index].version->type == AMD_IP_BLOCK_TYPE_IH))
> + || (adev->asic_type >= CHIP_VEGA10
> + && (adev->ip_blocks[index].version->type == AMD_IP_BLOCK_TYPE_PSP))) {
This seems kind of fragile. If we change the order again at some
point, it will break. How about we check whether hw_init/resume is
done or not on the blocks we care about or move the checks into the
callers and only call when we need it?
> + if (adev->powerplay.pp_funcs->load_firmware) {
> + r = adev->powerplay.pp_funcs->load_firmware(adev->powerplay.pp_handle);
> + if (r) {
> + pr_err("firmware loading failed\n");
> + return r;
> + }
> + }
> + }
> + return 0;
> +}
> /**
> * amdgpu_device_ip_init - run init for hardware IPs
> *
> @@ -1595,6 +1613,9 @@ static int amdgpu_device_ip_init(struct amdgpu_device *adev)
> return r;
> }
> adev->ip_blocks[i].status.hw = true;
> + r = amdgpu_device_fw_loading(adev, i);
> + if (r)
> + return r;
> }
>
> amdgpu_xgmi_add_device(adev);
> @@ -2030,6 +2051,9 @@ static int amdgpu_device_ip_reinit_early_sriov(struct amdgpu_device *adev)
> DRM_INFO("RE-INIT: %s %s\n", block->version->funcs->name, r?"failed":"succeeded");
> if (r)
> return r;
> + r = amdgpu_device_fw_loading(adev, i);
> + if (r)
> + return r;
> }
> }
>
> @@ -2098,6 +2122,9 @@ static int amdgpu_device_ip_resume_phase1(struct amdgpu_device *adev)
> adev->ip_blocks[i].version->funcs->name, r);
> return r;
> }
> + r = amdgpu_device_fw_loading(adev, i);
> + if (r)
> + return r;
> }
> }
>
> @@ -2134,6 +2161,9 @@ static int amdgpu_device_ip_resume_phase2(struct amdgpu_device *adev)
> adev->ip_blocks[i].version->funcs->name, r);
> return r;
> }
> + r = amdgpu_device_fw_loading(adev, i);
> + if (r)
> + return r;
> }
>
> return 0;
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> index 8439f9a..3d0f277 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> @@ -4175,20 +4175,9 @@ static void gfx_v8_0_rlc_start(struct amdgpu_device *adev)
>
> static int gfx_v8_0_rlc_resume(struct amdgpu_device *adev)
> {
> - int r;
> -
> gfx_v8_0_rlc_stop(adev);
> gfx_v8_0_rlc_reset(adev);
> gfx_v8_0_init_pg(adev);
> -
> - if (adev->powerplay.pp_funcs->load_firmware) {
> - r = adev->powerplay.pp_funcs->load_firmware(adev->powerplay.pp_handle);
> - if (r) {
> - pr_err("firmware loading failed\n");
> - return r;
> - }
> - }
> -
> gfx_v8_0_rlc_start(adev);
>
> return 0;
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
> index 0bdde7f..6fb3eda 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
> @@ -788,14 +788,6 @@ static int sdma_v3_0_start(struct amdgpu_device *adev)
> {
> int r;
>
> - if (adev->powerplay.pp_funcs->load_firmware) {
> - r = adev->powerplay.pp_funcs->load_firmware(adev->powerplay.pp_handle);
> - if (r) {
> - pr_err("firmware loading failed\n");
> - return r;
> - }
> - }
> -
> /* disable sdma engine before programing it */
> sdma_v3_0_ctx_switch_enable(adev, false);
> sdma_v3_0_enable(adev, false);
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/hwmgr.c b/drivers/gpu/drm/amd/powerplay/hwmgr/hwmgr.c
> index d552af2..47ac923 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/hwmgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/hwmgr.c
> @@ -89,7 +89,6 @@ int hwmgr_early_init(struct pp_hwmgr *hwmgr)
> hwmgr_init_default_caps(hwmgr);
> hwmgr_set_user_specify_caps(hwmgr);
> hwmgr->fan_ctrl_is_in_default_mode = true;
> - hwmgr->reload_fw = 1;
> hwmgr_init_workload_prority(hwmgr);
>
> switch (hwmgr->chip_family) {
> @@ -209,17 +208,6 @@ int hwmgr_hw_init(struct pp_hwmgr *hwmgr)
> {
> int ret = 0;
>
> - if (!hwmgr || !hwmgr->smumgr_funcs)
> - return -EINVAL;
> -
> - if (hwmgr->smumgr_funcs->start_smu) {
> - ret = hwmgr->smumgr_funcs->start_smu(hwmgr);
> - if (ret) {
> - pr_err("smc start failed\n");
> - return -EINVAL;
> - }
> - }
> -
> if (!hwmgr->pm_en)
> return 0;
>
> @@ -301,7 +289,6 @@ int hwmgr_suspend(struct pp_hwmgr *hwmgr)
> if (!hwmgr || !hwmgr->pm_en)
> return 0;
>
> - hwmgr->reload_fw = true;
> phm_disable_smc_firmware_ctf(hwmgr);
> ret = psm_set_boot_states(hwmgr);
> if (ret)
> @@ -321,13 +308,6 @@ int hwmgr_resume(struct pp_hwmgr *hwmgr)
> if (!hwmgr)
> return -EINVAL;
>
> - if (hwmgr->smumgr_funcs && hwmgr->smumgr_funcs->start_smu) {
> - if (hwmgr->smumgr_funcs->start_smu(hwmgr)) {
> - pr_err("smc start failed\n");
> - return -EINVAL;
> - }
> - }
> -
> if (!hwmgr->pm_en)
> return 0;
>
> diff --git a/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h b/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
> index 35f2272..e5a60aa 100644
> --- a/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
> +++ b/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
> @@ -734,7 +734,6 @@ struct pp_hwmgr {
> void *smu_backend;
> const struct pp_smumgr_func *smumgr_funcs;
> bool is_kicker;
> - bool reload_fw;
>
> enum PP_DAL_POWERLEVEL dal_power_level;
> struct phm_dynamic_state_info dyn_state;
> diff --git a/drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.c b/drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.c
> index 99b4e4f..3f51d54 100644
> --- a/drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.c
> @@ -343,9 +343,6 @@ int smu7_request_smu_load_fw(struct pp_hwmgr *hwmgr)
> uint32_t fw_to_load;
> int r = 0;
>
> - if (!hwmgr->reload_fw)
> - return 0;
> -
> amdgpu_ucode_init_bo(hwmgr->adev);
>
> if (smu_data->soft_regs_start)
> @@ -432,10 +429,9 @@ int smu7_request_smu_load_fw(struct pp_hwmgr *hwmgr)
> smu7_send_msg_to_smc_with_parameter(hwmgr, PPSMC_MSG_LoadUcodes, fw_to_load);
>
> r = smu7_check_fw_load_finish(hwmgr, fw_to_load);
> - if (!r) {
> - hwmgr->reload_fw = 0;
> + if (!r)
> return 0;
> - }
> +
> pr_err("SMU load firmware failed\n");
>
> failed:
> diff --git a/drivers/gpu/drm/amd/powerplay/smumgr/smu8_smumgr.c b/drivers/gpu/drm/amd/powerplay/smumgr/smu8_smumgr.c
> index abbf2f2..f836d30 100644
> --- a/drivers/gpu/drm/amd/powerplay/smumgr/smu8_smumgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/smumgr/smu8_smumgr.c
> @@ -661,9 +661,6 @@ static int smu8_request_smu_load_fw(struct pp_hwmgr *hwmgr)
> uint32_t fw_to_check = 0;
> int ret;
>
> - if (!hwmgr->reload_fw)
> - return 0;
> -
> amdgpu_ucode_init_bo(hwmgr->adev);
>
> smu8_smu_populate_firmware_entries(hwmgr);
> @@ -719,8 +716,6 @@ static int smu8_request_smu_load_fw(struct pp_hwmgr *hwmgr)
> return ret;
> }
>
> - hwmgr->reload_fw = 0;
> -
> return 0;
> }
>
> --
> 1.9.1
>
> _______________________________________________
> 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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 5/5] drm/amdgpu: Remove the direct fw loading support for sdma2.4
[not found] ` <1539089023-2360-5-git-send-email-Rex.Zhu-5C7GfCeVMHo@public.gmane.org>
@ 2018-10-09 19:28 ` Alex Deucher
0 siblings, 0 replies; 13+ messages in thread
From: Alex Deucher @ 2018-10-09 19:28 UTC (permalink / raw)
To: Rex Zhu; +Cc: amd-gfx list
On Tue, Oct 9, 2018 at 8:45 AM Rex Zhu <Rex.Zhu@amd.com> wrote:
>
> sdma2.4 is only for iceland. For Vi, we don't maintain the
> direct fw loading.
>
> Signed-off-by: Rex Zhu <Rex.Zhu@amd.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c | 42 ----------------------------------
> 1 file changed, 42 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c b/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
> index cd781ab..2d4770e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
> @@ -504,41 +504,6 @@ static int sdma_v2_4_rlc_resume(struct amdgpu_device *adev)
> return 0;
> }
>
> -/**
> - * sdma_v2_4_load_microcode - load the sDMA ME ucode
> - *
> - * @adev: amdgpu_device pointer
> - *
> - * Loads the sDMA0/1 ucode.
> - * Returns 0 for success, -EINVAL if the ucode is not available.
> - */
> -static int sdma_v2_4_load_microcode(struct amdgpu_device *adev)
> -{
> - const struct sdma_firmware_header_v1_0 *hdr;
> - const __le32 *fw_data;
> - u32 fw_size;
> - int i, j;
> -
> - /* halt the MEs */
> - sdma_v2_4_enable(adev, false);
> -
> - for (i = 0; i < adev->sdma.num_instances; i++) {
> - if (!adev->sdma.instance[i].fw)
> - return -EINVAL;
> - hdr = (const struct sdma_firmware_header_v1_0 *)adev->sdma.instance[i].fw->data;
> - amdgpu_ucode_print_sdma_hdr(&hdr->header);
> - fw_size = le32_to_cpu(hdr->header.ucode_size_bytes) / 4;
> - fw_data = (const __le32 *)
> - (adev->sdma.instance[i].fw->data +
> - le32_to_cpu(hdr->header.ucode_array_offset_bytes));
> - WREG32(mmSDMA0_UCODE_ADDR + sdma_offsets[i], 0);
> - for (j = 0; j < fw_size; j++)
> - WREG32(mmSDMA0_UCODE_DATA + sdma_offsets[i], le32_to_cpup(fw_data++));
> - WREG32(mmSDMA0_UCODE_ADDR + sdma_offsets[i], adev->sdma.instance[i].fw_version);
> - }
> -
> - return 0;
> -}
>
> /**
> * sdma_v2_4_start - setup and start the async dma engines
> @@ -552,13 +517,6 @@ static int sdma_v2_4_start(struct amdgpu_device *adev)
> {
> int r;
>
> -
> - if (adev->firmware.load_type == AMDGPU_FW_LOAD_DIRECT) {
> - r = sdma_v2_4_load_microcode(adev);
> - if (r)
> - return r;
> - }
> -
> /* halt the engine before programing */
> sdma_v2_4_enable(adev, false);
>
> --
> 1.9.1
>
> _______________________________________________
> 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
^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH 3/5] drm/amdgpu: Extract the function of fw loading out of powerplay
[not found] ` <CADnq5_NwMRLpqr12KtYt5j67vQfd-5pKN=b8UAU9=G5Nsc_mbA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2018-10-10 1:44 ` Zhu, Rex
[not found] ` <BYAPR12MB277578DE7CB42DA32FBC190CFBE00-ZGDeBxoHBPmJeBUhB162ZQdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
0 siblings, 1 reply; 13+ messages in thread
From: Zhu, Rex @ 2018-10-10 1:44 UTC (permalink / raw)
To: Alex Deucher; +Cc: amd-gfx list
> -----Original Message-----
> From: Alex Deucher <alexdeucher@gmail.com>
> Sent: Wednesday, October 10, 2018 3:28 AM
> To: Zhu, Rex <Rex.Zhu@amd.com>
> Cc: amd-gfx list <amd-gfx@lists.freedesktop.org>
> Subject: Re: [PATCH 3/5] drm/amdgpu: Extract the function of fw loading out
> of powerplay
>
> On Tue, Oct 9, 2018 at 8:45 AM Rex Zhu <Rex.Zhu@amd.com> wrote:
> >
> > So there is no dependence between gfx/sdma/smu.
> > and for Vi, after IH hw_init, driver load all the smu/gfx/sdma fw. for
> > AI, fw loading is controlled by PSP, after psp hw init, we call the
> > function to check smu fw version.
> >
> > Signed-off-by: Rex Zhu <Rex.Zhu@amd.com>
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 30
> ++++++++++++++++++++++
> > drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 11 --------
> > drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c | 8 ------
> > drivers/gpu/drm/amd/powerplay/hwmgr/hwmgr.c | 20 ---------------
> > drivers/gpu/drm/amd/powerplay/inc/hwmgr.h | 1 -
> > drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.c | 8 ++----
> > drivers/gpu/drm/amd/powerplay/smumgr/smu8_smumgr.c | 5 ----
> > 7 files changed, 32 insertions(+), 51 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > index 4787571..a6766b3 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > @@ -1525,6 +1525,24 @@ static int amdgpu_device_ip_early_init(struct
> amdgpu_device *adev)
> > return 0;
> > }
> >
> > +static int amdgpu_device_fw_loading(struct amdgpu_device *adev,
> > +uint32_t index) {
> > + int r = 0;
> > +
> > + if ((adev->asic_type < CHIP_VEGA10
> > + && (adev->ip_blocks[index].version->type ==
> AMD_IP_BLOCK_TYPE_IH))
> > + || (adev->asic_type >= CHIP_VEGA10
> > + && (adev->ip_blocks[index].version->type ==
> > + AMD_IP_BLOCK_TYPE_PSP))) {
>
> This seems kind of fragile. If we change the order again at some point, it will
> break. How about we check whether hw_init/resume is done or not on the
> blocks we care about or move the checks into the callers and only call when
> we need it?
Hi Alex,
How about split hw_init to hw_init_phase1 and hw_init_phase2 as resume?
We loaded fw(call psp_hw_init and start_smu) between phase1 and phase2.
Regards
Rex
> > + if (adev->powerplay.pp_funcs->load_firmware) {
> > + r = adev->powerplay.pp_funcs->load_firmware(adev-
> >powerplay.pp_handle);
> > + if (r) {
> > + pr_err("firmware loading failed\n");
> > + return r;
> > + }
> > + }
> > + }
> > + return 0;
> > +}
> > /**
> > * amdgpu_device_ip_init - run init for hardware IPs
> > *
> > @@ -1595,6 +1613,9 @@ static int amdgpu_device_ip_init(struct
> amdgpu_device *adev)
> > return r;
> > }
> > adev->ip_blocks[i].status.hw = true;
> > + r = amdgpu_device_fw_loading(adev, i);
> > + if (r)
> > + return r;
> > }
> >
> > amdgpu_xgmi_add_device(adev);
> > @@ -2030,6 +2051,9 @@ static int
> amdgpu_device_ip_reinit_early_sriov(struct amdgpu_device *adev)
> > DRM_INFO("RE-INIT: %s %s\n", block->version->funcs->name,
> r?"failed":"succeeded");
> > if (r)
> > return r;
> > + r = amdgpu_device_fw_loading(adev, i);
> > + if (r)
> > + return r;
> > }
> > }
> >
> > @@ -2098,6 +2122,9 @@ static int
> amdgpu_device_ip_resume_phase1(struct amdgpu_device *adev)
> > adev->ip_blocks[i].version->funcs->name, r);
> > return r;
> > }
> > + r = amdgpu_device_fw_loading(adev, i);
> > + if (r)
> > + return r;
> > }
> > }
> >
> > @@ -2134,6 +2161,9 @@ static int
> amdgpu_device_ip_resume_phase2(struct amdgpu_device *adev)
> > adev->ip_blocks[i].version->funcs->name, r);
> > return r;
> > }
> > + r = amdgpu_device_fw_loading(adev, i);
> > + if (r)
> > + return r;
> > }
> >
> > return 0;
> > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> > b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> > index 8439f9a..3d0f277 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> > @@ -4175,20 +4175,9 @@ static void gfx_v8_0_rlc_start(struct
> > amdgpu_device *adev)
> >
> > static int gfx_v8_0_rlc_resume(struct amdgpu_device *adev) {
> > - int r;
> > -
> > gfx_v8_0_rlc_stop(adev);
> > gfx_v8_0_rlc_reset(adev);
> > gfx_v8_0_init_pg(adev);
> > -
> > - if (adev->powerplay.pp_funcs->load_firmware) {
> > - r = adev->powerplay.pp_funcs->load_firmware(adev-
> >powerplay.pp_handle);
> > - if (r) {
> > - pr_err("firmware loading failed\n");
> > - return r;
> > - }
> > - }
> > -
> > gfx_v8_0_rlc_start(adev);
> >
> > return 0;
> > diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
> > b/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
> > index 0bdde7f..6fb3eda 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
> > @@ -788,14 +788,6 @@ static int sdma_v3_0_start(struct amdgpu_device
> > *adev) {
> > int r;
> >
> > - if (adev->powerplay.pp_funcs->load_firmware) {
> > - r = adev->powerplay.pp_funcs->load_firmware(adev-
> >powerplay.pp_handle);
> > - if (r) {
> > - pr_err("firmware loading failed\n");
> > - return r;
> > - }
> > - }
> > -
> > /* disable sdma engine before programing it */
> > sdma_v3_0_ctx_switch_enable(adev, false);
> > sdma_v3_0_enable(adev, false); diff --git
> > a/drivers/gpu/drm/amd/powerplay/hwmgr/hwmgr.c
> > b/drivers/gpu/drm/amd/powerplay/hwmgr/hwmgr.c
> > index d552af2..47ac923 100644
> > --- a/drivers/gpu/drm/amd/powerplay/hwmgr/hwmgr.c
> > +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/hwmgr.c
> > @@ -89,7 +89,6 @@ int hwmgr_early_init(struct pp_hwmgr *hwmgr)
> > hwmgr_init_default_caps(hwmgr);
> > hwmgr_set_user_specify_caps(hwmgr);
> > hwmgr->fan_ctrl_is_in_default_mode = true;
> > - hwmgr->reload_fw = 1;
> > hwmgr_init_workload_prority(hwmgr);
> >
> > switch (hwmgr->chip_family) {
> > @@ -209,17 +208,6 @@ int hwmgr_hw_init(struct pp_hwmgr *hwmgr) {
> > int ret = 0;
> >
> > - if (!hwmgr || !hwmgr->smumgr_funcs)
> > - return -EINVAL;
> > -
> > - if (hwmgr->smumgr_funcs->start_smu) {
> > - ret = hwmgr->smumgr_funcs->start_smu(hwmgr);
> > - if (ret) {
> > - pr_err("smc start failed\n");
> > - return -EINVAL;
> > - }
> > - }
> > -
> > if (!hwmgr->pm_en)
> > return 0;
> >
> > @@ -301,7 +289,6 @@ int hwmgr_suspend(struct pp_hwmgr *hwmgr)
> > if (!hwmgr || !hwmgr->pm_en)
> > return 0;
> >
> > - hwmgr->reload_fw = true;
> > phm_disable_smc_firmware_ctf(hwmgr);
> > ret = psm_set_boot_states(hwmgr);
> > if (ret)
> > @@ -321,13 +308,6 @@ int hwmgr_resume(struct pp_hwmgr *hwmgr)
> > if (!hwmgr)
> > return -EINVAL;
> >
> > - if (hwmgr->smumgr_funcs && hwmgr->smumgr_funcs->start_smu) {
> > - if (hwmgr->smumgr_funcs->start_smu(hwmgr)) {
> > - pr_err("smc start failed\n");
> > - return -EINVAL;
> > - }
> > - }
> > -
> > if (!hwmgr->pm_en)
> > return 0;
> >
> > diff --git a/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
> > b/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
> > index 35f2272..e5a60aa 100644
> > --- a/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
> > +++ b/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
> > @@ -734,7 +734,6 @@ struct pp_hwmgr {
> > void *smu_backend;
> > const struct pp_smumgr_func *smumgr_funcs;
> > bool is_kicker;
> > - bool reload_fw;
> >
> > enum PP_DAL_POWERLEVEL dal_power_level;
> > struct phm_dynamic_state_info dyn_state; diff --git
> > a/drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.c
> > b/drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.c
> > index 99b4e4f..3f51d54 100644
> > --- a/drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.c
> > +++ b/drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.c
> > @@ -343,9 +343,6 @@ int smu7_request_smu_load_fw(struct pp_hwmgr
> *hwmgr)
> > uint32_t fw_to_load;
> > int r = 0;
> >
> > - if (!hwmgr->reload_fw)
> > - return 0;
> > -
> > amdgpu_ucode_init_bo(hwmgr->adev);
> >
> > if (smu_data->soft_regs_start) @@ -432,10 +429,9 @@ int
> > smu7_request_smu_load_fw(struct pp_hwmgr *hwmgr)
> > smu7_send_msg_to_smc_with_parameter(hwmgr,
> > PPSMC_MSG_LoadUcodes, fw_to_load);
> >
> > r = smu7_check_fw_load_finish(hwmgr, fw_to_load);
> > - if (!r) {
> > - hwmgr->reload_fw = 0;
> > + if (!r)
> > return 0;
> > - }
> > +
> > pr_err("SMU load firmware failed\n");
> >
> > failed:
> > diff --git a/drivers/gpu/drm/amd/powerplay/smumgr/smu8_smumgr.c
> > b/drivers/gpu/drm/amd/powerplay/smumgr/smu8_smumgr.c
> > index abbf2f2..f836d30 100644
> > --- a/drivers/gpu/drm/amd/powerplay/smumgr/smu8_smumgr.c
> > +++ b/drivers/gpu/drm/amd/powerplay/smumgr/smu8_smumgr.c
> > @@ -661,9 +661,6 @@ static int smu8_request_smu_load_fw(struct
> pp_hwmgr *hwmgr)
> > uint32_t fw_to_check = 0;
> > int ret;
> >
> > - if (!hwmgr->reload_fw)
> > - return 0;
> > -
> > amdgpu_ucode_init_bo(hwmgr->adev);
> >
> > smu8_smu_populate_firmware_entries(hwmgr);
> > @@ -719,8 +716,6 @@ static int smu8_request_smu_load_fw(struct
> pp_hwmgr *hwmgr)
> > return ret;
> > }
> >
> > - hwmgr->reload_fw = 0;
> > -
> > return 0;
> > }
> >
> > --
> > 1.9.1
> >
> > _______________________________________________
> > 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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/5] drm/amdgpu: Extract the function of fw loading out of powerplay
[not found] ` <BYAPR12MB277578DE7CB42DA32FBC190CFBE00-ZGDeBxoHBPmJeBUhB162ZQdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
@ 2018-10-10 14:44 ` Deucher, Alexander
0 siblings, 0 replies; 13+ messages in thread
From: Deucher, Alexander @ 2018-10-10 14:44 UTC (permalink / raw)
To: Zhu, Rex, Alex Deucher; +Cc: amd-gfx list
[-- Attachment #1.1: Type: text/plain, Size: 11892 bytes --]
Sounds good to me.
Alex
________________________________
From: amd-gfx <amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org> on behalf of Zhu, Rex <Rex.Zhu-5C7GfCeVMHo@public.gmane.org>
Sent: Tuesday, October 9, 2018 9:44 PM
To: Alex Deucher
Cc: amd-gfx list
Subject: RE: [PATCH 3/5] drm/amdgpu: Extract the function of fw loading out of powerplay
> -----Original Message-----
> From: Alex Deucher <alexdeucher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Sent: Wednesday, October 10, 2018 3:28 AM
> To: Zhu, Rex <Rex.Zhu-5C7GfCeVMHo@public.gmane.org>
> Cc: amd-gfx list <amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
> Subject: Re: [PATCH 3/5] drm/amdgpu: Extract the function of fw loading out
> of powerplay
>
> On Tue, Oct 9, 2018 at 8:45 AM Rex Zhu <Rex.Zhu-5C7GfCeVMHo@public.gmane.org> wrote:
> >
> > So there is no dependence between gfx/sdma/smu.
> > and for Vi, after IH hw_init, driver load all the smu/gfx/sdma fw. for
> > AI, fw loading is controlled by PSP, after psp hw init, we call the
> > function to check smu fw version.
> >
> > Signed-off-by: Rex Zhu <Rex.Zhu-5C7GfCeVMHo@public.gmane.org>
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 30
> ++++++++++++++++++++++
> > drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 11 --------
> > drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c | 8 ------
> > drivers/gpu/drm/amd/powerplay/hwmgr/hwmgr.c | 20 ---------------
> > drivers/gpu/drm/amd/powerplay/inc/hwmgr.h | 1 -
> > drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.c | 8 ++----
> > drivers/gpu/drm/amd/powerplay/smumgr/smu8_smumgr.c | 5 ----
> > 7 files changed, 32 insertions(+), 51 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > index 4787571..a6766b3 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > @@ -1525,6 +1525,24 @@ static int amdgpu_device_ip_early_init(struct
> amdgpu_device *adev)
> > return 0;
> > }
> >
> > +static int amdgpu_device_fw_loading(struct amdgpu_device *adev,
> > +uint32_t index) {
> > + int r = 0;
> > +
> > + if ((adev->asic_type < CHIP_VEGA10
> > + && (adev->ip_blocks[index].version->type ==
> AMD_IP_BLOCK_TYPE_IH))
> > + || (adev->asic_type >= CHIP_VEGA10
> > + && (adev->ip_blocks[index].version->type ==
> > + AMD_IP_BLOCK_TYPE_PSP))) {
>
> This seems kind of fragile. If we change the order again at some point, it will
> break. How about we check whether hw_init/resume is done or not on the
> blocks we care about or move the checks into the callers and only call when
> we need it?
Hi Alex,
How about split hw_init to hw_init_phase1 and hw_init_phase2 as resume?
We loaded fw(call psp_hw_init and start_smu) between phase1 and phase2.
Regards
Rex
> > + if (adev->powerplay.pp_funcs->load_firmware) {
> > + r = adev->powerplay.pp_funcs->load_firmware(adev-
> >powerplay.pp_handle);
> > + if (r) {
> > + pr_err("firmware loading failed\n");
> > + return r;
> > + }
> > + }
> > + }
> > + return 0;
> > +}
> > /**
> > * amdgpu_device_ip_init - run init for hardware IPs
> > *
> > @@ -1595,6 +1613,9 @@ static int amdgpu_device_ip_init(struct
> amdgpu_device *adev)
> > return r;
> > }
> > adev->ip_blocks[i].status.hw = true;
> > + r = amdgpu_device_fw_loading(adev, i);
> > + if (r)
> > + return r;
> > }
> >
> > amdgpu_xgmi_add_device(adev);
> > @@ -2030,6 +2051,9 @@ static int
> amdgpu_device_ip_reinit_early_sriov(struct amdgpu_device *adev)
> > DRM_INFO("RE-INIT: %s %s\n", block->version->funcs->name,
> r?"failed":"succeeded");
> > if (r)
> > return r;
> > + r = amdgpu_device_fw_loading(adev, i);
> > + if (r)
> > + return r;
> > }
> > }
> >
> > @@ -2098,6 +2122,9 @@ static int
> amdgpu_device_ip_resume_phase1(struct amdgpu_device *adev)
> > adev->ip_blocks[i].version->funcs->name, r);
> > return r;
> > }
> > + r = amdgpu_device_fw_loading(adev, i);
> > + if (r)
> > + return r;
> > }
> > }
> >
> > @@ -2134,6 +2161,9 @@ static int
> amdgpu_device_ip_resume_phase2(struct amdgpu_device *adev)
> > adev->ip_blocks[i].version->funcs->name, r);
> > return r;
> > }
> > + r = amdgpu_device_fw_loading(adev, i);
> > + if (r)
> > + return r;
> > }
> >
> > return 0;
> > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> > b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> > index 8439f9a..3d0f277 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> > @@ -4175,20 +4175,9 @@ static void gfx_v8_0_rlc_start(struct
> > amdgpu_device *adev)
> >
> > static int gfx_v8_0_rlc_resume(struct amdgpu_device *adev) {
> > - int r;
> > -
> > gfx_v8_0_rlc_stop(adev);
> > gfx_v8_0_rlc_reset(adev);
> > gfx_v8_0_init_pg(adev);
> > -
> > - if (adev->powerplay.pp_funcs->load_firmware) {
> > - r = adev->powerplay.pp_funcs->load_firmware(adev-
> >powerplay.pp_handle);
> > - if (r) {
> > - pr_err("firmware loading failed\n");
> > - return r;
> > - }
> > - }
> > -
> > gfx_v8_0_rlc_start(adev);
> >
> > return 0;
> > diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
> > b/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
> > index 0bdde7f..6fb3eda 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
> > @@ -788,14 +788,6 @@ static int sdma_v3_0_start(struct amdgpu_device
> > *adev) {
> > int r;
> >
> > - if (adev->powerplay.pp_funcs->load_firmware) {
> > - r = adev->powerplay.pp_funcs->load_firmware(adev-
> >powerplay.pp_handle);
> > - if (r) {
> > - pr_err("firmware loading failed\n");
> > - return r;
> > - }
> > - }
> > -
> > /* disable sdma engine before programing it */
> > sdma_v3_0_ctx_switch_enable(adev, false);
> > sdma_v3_0_enable(adev, false); diff --git
> > a/drivers/gpu/drm/amd/powerplay/hwmgr/hwmgr.c
> > b/drivers/gpu/drm/amd/powerplay/hwmgr/hwmgr.c
> > index d552af2..47ac923 100644
> > --- a/drivers/gpu/drm/amd/powerplay/hwmgr/hwmgr.c
> > +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/hwmgr.c
> > @@ -89,7 +89,6 @@ int hwmgr_early_init(struct pp_hwmgr *hwmgr)
> > hwmgr_init_default_caps(hwmgr);
> > hwmgr_set_user_specify_caps(hwmgr);
> > hwmgr->fan_ctrl_is_in_default_mode = true;
> > - hwmgr->reload_fw = 1;
> > hwmgr_init_workload_prority(hwmgr);
> >
> > switch (hwmgr->chip_family) {
> > @@ -209,17 +208,6 @@ int hwmgr_hw_init(struct pp_hwmgr *hwmgr) {
> > int ret = 0;
> >
> > - if (!hwmgr || !hwmgr->smumgr_funcs)
> > - return -EINVAL;
> > -
> > - if (hwmgr->smumgr_funcs->start_smu) {
> > - ret = hwmgr->smumgr_funcs->start_smu(hwmgr);
> > - if (ret) {
> > - pr_err("smc start failed\n");
> > - return -EINVAL;
> > - }
> > - }
> > -
> > if (!hwmgr->pm_en)
> > return 0;
> >
> > @@ -301,7 +289,6 @@ int hwmgr_suspend(struct pp_hwmgr *hwmgr)
> > if (!hwmgr || !hwmgr->pm_en)
> > return 0;
> >
> > - hwmgr->reload_fw = true;
> > phm_disable_smc_firmware_ctf(hwmgr);
> > ret = psm_set_boot_states(hwmgr);
> > if (ret)
> > @@ -321,13 +308,6 @@ int hwmgr_resume(struct pp_hwmgr *hwmgr)
> > if (!hwmgr)
> > return -EINVAL;
> >
> > - if (hwmgr->smumgr_funcs && hwmgr->smumgr_funcs->start_smu) {
> > - if (hwmgr->smumgr_funcs->start_smu(hwmgr)) {
> > - pr_err("smc start failed\n");
> > - return -EINVAL;
> > - }
> > - }
> > -
> > if (!hwmgr->pm_en)
> > return 0;
> >
> > diff --git a/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
> > b/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
> > index 35f2272..e5a60aa 100644
> > --- a/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
> > +++ b/drivers/gpu/drm/amd/powerplay/inc/hwmgr.h
> > @@ -734,7 +734,6 @@ struct pp_hwmgr {
> > void *smu_backend;
> > const struct pp_smumgr_func *smumgr_funcs;
> > bool is_kicker;
> > - bool reload_fw;
> >
> > enum PP_DAL_POWERLEVEL dal_power_level;
> > struct phm_dynamic_state_info dyn_state; diff --git
> > a/drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.c
> > b/drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.c
> > index 99b4e4f..3f51d54 100644
> > --- a/drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.c
> > +++ b/drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.c
> > @@ -343,9 +343,6 @@ int smu7_request_smu_load_fw(struct pp_hwmgr
> *hwmgr)
> > uint32_t fw_to_load;
> > int r = 0;
> >
> > - if (!hwmgr->reload_fw)
> > - return 0;
> > -
> > amdgpu_ucode_init_bo(hwmgr->adev);
> >
> > if (smu_data->soft_regs_start) @@ -432,10 +429,9 @@ int
> > smu7_request_smu_load_fw(struct pp_hwmgr *hwmgr)
> > smu7_send_msg_to_smc_with_parameter(hwmgr,
> > PPSMC_MSG_LoadUcodes, fw_to_load);
> >
> > r = smu7_check_fw_load_finish(hwmgr, fw_to_load);
> > - if (!r) {
> > - hwmgr->reload_fw = 0;
> > + if (!r)
> > return 0;
> > - }
> > +
> > pr_err("SMU load firmware failed\n");
> >
> > failed:
> > diff --git a/drivers/gpu/drm/amd/powerplay/smumgr/smu8_smumgr.c
> > b/drivers/gpu/drm/amd/powerplay/smumgr/smu8_smumgr.c
> > index abbf2f2..f836d30 100644
> > --- a/drivers/gpu/drm/amd/powerplay/smumgr/smu8_smumgr.c
> > +++ b/drivers/gpu/drm/amd/powerplay/smumgr/smu8_smumgr.c
> > @@ -661,9 +661,6 @@ static int smu8_request_smu_load_fw(struct
> pp_hwmgr *hwmgr)
> > uint32_t fw_to_check = 0;
> > int ret;
> >
> > - if (!hwmgr->reload_fw)
> > - return 0;
> > -
> > amdgpu_ucode_init_bo(hwmgr->adev);
> >
> > smu8_smu_populate_firmware_entries(hwmgr);
> > @@ -719,8 +716,6 @@ static int smu8_request_smu_load_fw(struct
> pp_hwmgr *hwmgr)
> > return ret;
> > }
> >
> > - hwmgr->reload_fw = 0;
> > -
> > return 0;
> > }
> >
> > --
> > 1.9.1
> >
> > _______________________________________________
> > amd-gfx mailing list
> > amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[-- Attachment #1.2: Type: text/html, Size: 26011 bytes --]
[-- Attachment #2: Type: text/plain, Size: 154 bytes --]
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2018-10-10 14:44 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-09 12:43 [PATCH 1/5] drm/amdgpu: Split amdgpu_ucode_init/fini_bo into two functions Rex Zhu
[not found] ` <1539089023-2360-1-git-send-email-Rex.Zhu-5C7GfCeVMHo@public.gmane.org>
2018-10-09 12:43 ` [PATCH 2/5] drm/amdgpu: Remove amdgpu_ucode_fini_bo Rex Zhu
[not found] ` <1539089023-2360-2-git-send-email-Rex.Zhu-5C7GfCeVMHo@public.gmane.org>
2018-10-09 18:54 ` Alex Deucher
2018-10-09 12:43 ` [PATCH 3/5] drm/amdgpu: Extract the function of fw loading out of powerplay Rex Zhu
[not found] ` <1539089023-2360-3-git-send-email-Rex.Zhu-5C7GfCeVMHo@public.gmane.org>
2018-10-09 19:27 ` Alex Deucher
[not found] ` <CADnq5_NwMRLpqr12KtYt5j67vQfd-5pKN=b8UAU9=G5Nsc_mbA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2018-10-10 1:44 ` Zhu, Rex
[not found] ` <BYAPR12MB277578DE7CB42DA32FBC190CFBE00-ZGDeBxoHBPmJeBUhB162ZQdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
2018-10-10 14:44 ` Deucher, Alexander
2018-10-09 12:43 ` [PATCH 4/5] drm/amdgpu: Fix unnecessary warning in dmesg Rex Zhu
[not found] ` <1539089023-2360-4-git-send-email-Rex.Zhu-5C7GfCeVMHo@public.gmane.org>
2018-10-09 13:18 ` Michel Dänzer
[not found] ` <15af5a1b-ea73-c779-5f30-e6da0192d504-otUistvHUpPR7s880joybQ@public.gmane.org>
2018-10-09 14:42 ` Zhu, Rex
2018-10-09 12:43 ` [PATCH 5/5] drm/amdgpu: Remove the direct fw loading support for sdma2.4 Rex Zhu
[not found] ` <1539089023-2360-5-git-send-email-Rex.Zhu-5C7GfCeVMHo@public.gmane.org>
2018-10-09 19:28 ` Alex Deucher
2018-10-09 13:06 ` [PATCH 1/5] drm/amdgpu: Split amdgpu_ucode_init/fini_bo into two functions Christian König
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.