RE: [PATCH 6/7] drm/amd/pp: Export load_firmware interface
> -Original Message-
> From: Alex Deucher
> Sent: Wednesday, September 26, 2018 11:39 PM
> To: Zhu, Rex
> Cc: amd-gfx list
> Subject: Re: [PATCH 6/7] drm/amd/pp: Export load_firmware interface
>
> On Wed, Sep 26, 2018 at 11:21 AM Zhu, Rex wrote:
> >
> >
> >
> > > -Original Message-
> > > From: Alex Deucher
> > > Sent: Wednesday, September 26, 2018 10:15 PM
> > > To: Zhu, Rex
> > > Cc: amd-gfx list
> > > Subject: Re: [PATCH 6/7] drm/amd/pp: Export load_firmware interface
> > >
> > > On Wed, Sep 26, 2018 at 8:53 AM Rex Zhu wrote:
> > > >
> > > > Export this interface for the AMDGPU_FW_LOAD_SMU type.
> > > > gfx/sdma can request smu to load firmware.
> > > >
> > > > Split the smu7/8_start_smu function into two functions 1.
> > > > start_smu, used for load smu firmware in smu7/8 and
> > > >check smu firmware version.
> > > > 2. request_smu_load_fw, used for load other ip's firmware
> > > >on smu7/8 and add firmware loading staus check.
> > > >
> > > > Signed-off-by: Rex Zhu
> > >
> > > Don't we need to convert sdma to use this interface as well?
> >
> > When load firmware via smu, I load all the
> firmwares(rlc/ce/pfp/me/mec/sdma) at one time.
> > So just need to call the interface when load rlc firmware.
> > We also can load the firmware separately with ucode mask.
>
> I see. So we rely on the call from the gfx IP to load all the FWs.
> It might be better to be more explicit. E.g., if someone disables the gfx IP
> via
> the ip_block_mask parameter, that would break sdma.
I didn't consider that aspect.
We can load firmware separately with ucode_id.
Or just call this function again in sdma to load all the firmware.
When firmware is loaded , clean the reload_fw in Patch5.
Best Regards
Rex
> Alex
>
> >
> >
> > > Alex
> > >
> > > > ---
> > > > drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 11 +++-
> > > > drivers/gpu/drm/amd/powerplay/amd_powerplay.c | 14 +++--
> > > > drivers/gpu/drm/amd/powerplay/smumgr/fiji_smumgr.c | 4 +-
> > > > .../gpu/drm/amd/powerplay/smumgr/iceland_smumgr.c | 25 -
> > > > .../drm/amd/powerplay/smumgr/polaris10_smumgr.c| 4 +-
> > > > drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.c | 59
> > > > +-
> > > > drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.h | 3 +-
> > > drivers/gpu/drm/amd/powerplay/smumgr/smu8_smumgr.c | 46
> --
> > > ---
> > > > .../gpu/drm/amd/powerplay/smumgr/tonga_smumgr.c| 12 -
> > > > .../gpu/drm/amd/powerplay/smumgr/vegam_smumgr.c| 4 +-
> > > > 10 files changed, 79 insertions(+), 103 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> > > > b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> > > > index 2aeef2b..f020f6f 100644
> > > > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> > > > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> > > > @@ -4216,10 +4216,17 @@ static int gfx_v8_0_rlc_resume(struct
> > > amdgpu_device *adev)
> > > > if (adev->firmware.load_type == AMDGPU_FW_LOAD_DIRECT) {
> > > > /* legacy rlc firmware loading */
> > > > r = gfx_v8_0_rlc_load_microcode(adev);
> > > > - if (r)
> > > > - return r;
> > > > + } else if (adev->firmware.load_type == AMDGPU_FW_LOAD_SMU
> > > > + &&
> > > adev->powerplay.pp_funcs->load_firmware) {
> > > > + amdgpu_ucode_init_bo(adev);
> > > > + r = adev->powerplay.pp_funcs->load_firmware(adev-
> > > >powerplay.pp_handle);
> > > > + } else {
> > > > + r = -EINVAL;
> > > > }
> > > >
> > > > + 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/powerplay/amd_powerplay.c
> > > > b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
> > > > index aff7c14..3bc825c 100644
> > > > --- a/drivers/gpu/drm/amd/powerplay/
Re: [PATCH 6/7] drm/amd/pp: Export load_firmware interface
On Wed, Sep 26, 2018 at 11:21 AM Zhu, Rex wrote:
>
>
>
> > -Original Message-
> > From: Alex Deucher
> > Sent: Wednesday, September 26, 2018 10:15 PM
> > To: Zhu, Rex
> > Cc: amd-gfx list
> > Subject: Re: [PATCH 6/7] drm/amd/pp: Export load_firmware interface
> >
> > On Wed, Sep 26, 2018 at 8:53 AM Rex Zhu wrote:
> > >
> > > Export this interface for the AMDGPU_FW_LOAD_SMU type.
> > > gfx/sdma can request smu to load firmware.
> > >
> > > Split the smu7/8_start_smu function into two functions 1. start_smu,
> > > used for load smu firmware in smu7/8 and
> > >check smu firmware version.
> > > 2. request_smu_load_fw, used for load other ip's firmware
> > >on smu7/8 and add firmware loading staus check.
> > >
> > > Signed-off-by: Rex Zhu
> >
> > Don't we need to convert sdma to use this interface as well?
>
> When load firmware via smu, I load all the firmwares(rlc/ce/pfp/me/mec/sdma)
> at one time.
> So just need to call the interface when load rlc firmware.
> We also can load the firmware separately with ucode mask.
I see. So we rely on the call from the gfx IP to load all the FWs.
It might be better to be more explicit. E.g., if someone disables the
gfx IP via the ip_block_mask parameter, that would break sdma.
Alex
>
>
> > Alex
> >
> > > ---
> > > drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 11 +++-
> > > drivers/gpu/drm/amd/powerplay/amd_powerplay.c | 14 +++--
> > > drivers/gpu/drm/amd/powerplay/smumgr/fiji_smumgr.c | 4 +-
> > > .../gpu/drm/amd/powerplay/smumgr/iceland_smumgr.c | 25 -
> > > .../drm/amd/powerplay/smumgr/polaris10_smumgr.c| 4 +-
> > > drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.c | 59
> > > +-
> > > drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.h | 3 +-
> > drivers/gpu/drm/amd/powerplay/smumgr/smu8_smumgr.c | 46 --
> > ---
> > > .../gpu/drm/amd/powerplay/smumgr/tonga_smumgr.c| 12 -
> > > .../gpu/drm/amd/powerplay/smumgr/vegam_smumgr.c| 4 +-
> > > 10 files changed, 79 insertions(+), 103 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> > > b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> > > index 2aeef2b..f020f6f 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> > > @@ -4216,10 +4216,17 @@ static int gfx_v8_0_rlc_resume(struct
> > amdgpu_device *adev)
> > > if (adev->firmware.load_type == AMDGPU_FW_LOAD_DIRECT) {
> > > /* legacy rlc firmware loading */
> > > r = gfx_v8_0_rlc_load_microcode(adev);
> > > - if (r)
> > > - return r;
> > > + } else if (adev->firmware.load_type == AMDGPU_FW_LOAD_SMU &&
> > adev->powerplay.pp_funcs->load_firmware) {
> > > + amdgpu_ucode_init_bo(adev);
> > > + r = adev->powerplay.pp_funcs->load_firmware(adev-
> > >powerplay.pp_handle);
> > > + } else {
> > > + r = -EINVAL;
> > > }
> > >
> > > + 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/powerplay/amd_powerplay.c
> > > b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
> > > index aff7c14..3bc825c 100644
> > > --- a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
> > > +++ b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
> > > @@ -124,9 +124,6 @@ static int pp_hw_init(void *handle)
> > > struct amdgpu_device *adev = handle;
> > > struct pp_hwmgr *hwmgr = adev->powerplay.pp_handle;
> > >
> > > - if (adev->firmware.load_type == AMDGPU_FW_LOAD_SMU)
> > > - amdgpu_ucode_init_bo(adev);
> > > -
> > > ret = hwmgr_hw_init(hwmgr);
> > >
> > > if (ret)
> > > @@ -275,7 +272,16 @@ static int pp_set_clockgating_state(void *handle,
> > >
> > > static int pp_dpm_load_fw(void *handle) {
> > > - return 0;
> > > + struct pp_hwmgr *hwmgr = handle;
> > > + int ret = 0;
> > > +
> > > + if (!hwmgr || !hwmgr->smumgr_funcs)
RE: [PATCH 6/7] drm/amd/pp: Export load_firmware interface
> -Original Message-
> From: Alex Deucher
> Sent: Wednesday, September 26, 2018 10:15 PM
> To: Zhu, Rex
> Cc: amd-gfx list
> Subject: Re: [PATCH 6/7] drm/amd/pp: Export load_firmware interface
>
> On Wed, Sep 26, 2018 at 8:53 AM Rex Zhu wrote:
> >
> > Export this interface for the AMDGPU_FW_LOAD_SMU type.
> > gfx/sdma can request smu to load firmware.
> >
> > Split the smu7/8_start_smu function into two functions 1. start_smu,
> > used for load smu firmware in smu7/8 and
> >check smu firmware version.
> > 2. request_smu_load_fw, used for load other ip's firmware
> >on smu7/8 and add firmware loading staus check.
> >
> > Signed-off-by: Rex Zhu
>
> Don't we need to convert sdma to use this interface as well?
When load firmware via smu, I load all the firmwares(rlc/ce/pfp/me/mec/sdma) at
one time.
So just need to call the interface when load rlc firmware.
We also can load the firmware separately with ucode mask.
> Alex
>
> > ---
> > drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 11 +++-
> > drivers/gpu/drm/amd/powerplay/amd_powerplay.c | 14 +++--
> > drivers/gpu/drm/amd/powerplay/smumgr/fiji_smumgr.c | 4 +-
> > .../gpu/drm/amd/powerplay/smumgr/iceland_smumgr.c | 25 -
> > .../drm/amd/powerplay/smumgr/polaris10_smumgr.c| 4 +-
> > drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.c | 59
> > +-
> > drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.h | 3 +-
> drivers/gpu/drm/amd/powerplay/smumgr/smu8_smumgr.c | 46 --
> ---
> > .../gpu/drm/amd/powerplay/smumgr/tonga_smumgr.c| 12 -
> > .../gpu/drm/amd/powerplay/smumgr/vegam_smumgr.c| 4 +-
> > 10 files changed, 79 insertions(+), 103 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> > b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> > index 2aeef2b..f020f6f 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> > @@ -4216,10 +4216,17 @@ static int gfx_v8_0_rlc_resume(struct
> amdgpu_device *adev)
> > if (adev->firmware.load_type == AMDGPU_FW_LOAD_DIRECT) {
> > /* legacy rlc firmware loading */
> > r = gfx_v8_0_rlc_load_microcode(adev);
> > - if (r)
> > - return r;
> > + } else if (adev->firmware.load_type == AMDGPU_FW_LOAD_SMU &&
> adev->powerplay.pp_funcs->load_firmware) {
> > + amdgpu_ucode_init_bo(adev);
> > + r = adev->powerplay.pp_funcs->load_firmware(adev-
> >powerplay.pp_handle);
> > + } else {
> > + r = -EINVAL;
> > }
> >
> > + 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/powerplay/amd_powerplay.c
> > b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
> > index aff7c14..3bc825c 100644
> > --- a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
> > +++ b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
> > @@ -124,9 +124,6 @@ static int pp_hw_init(void *handle)
> > struct amdgpu_device *adev = handle;
> > struct pp_hwmgr *hwmgr = adev->powerplay.pp_handle;
> >
> > - if (adev->firmware.load_type == AMDGPU_FW_LOAD_SMU)
> > - amdgpu_ucode_init_bo(adev);
> > -
> > ret = hwmgr_hw_init(hwmgr);
> >
> > if (ret)
> > @@ -275,7 +272,16 @@ static int pp_set_clockgating_state(void *handle,
> >
> > static int pp_dpm_load_fw(void *handle) {
> > - return 0;
> > + struct pp_hwmgr *hwmgr = handle;
> > + int ret = 0;
> > +
> > + if (!hwmgr || !hwmgr->smumgr_funcs)
> > + return -EINVAL;
> > +
> > + if (hwmgr->smumgr_funcs->request_smu_load_fw)
> > + ret = hwmgr->smumgr_funcs->request_smu_load_fw(hwmgr);
> > +
> > + return ret;
> > }
> >
> > static int pp_dpm_fw_loading_complete(void *handle) diff --git
> > a/drivers/gpu/drm/amd/powerplay/smumgr/fiji_smumgr.c
> > b/drivers/gpu/drm/amd/powerplay/smumgr/fiji_smumgr.c
> > index b6b62a7..ffd7d78 100644
> > --- a/drivers/gpu/drm/amd/powerplay/smumgr/fiji_smumgr.c
> > +++ b/drivers/gpu/drm/amd/powerplay/smumgr/fiji_smumgr.c
> > @@ -310,8 +310,6 @@ static int fiji_start_smu(struct pp_hwmgr *hwmgr)
> >
Re: [PATCH 6/7] drm/amd/pp: Export load_firmware interface
On Wed, Sep 26, 2018 at 8:53 AM Rex Zhu wrote:
>
> Export this interface for the AMDGPU_FW_LOAD_SMU type.
> gfx/sdma can request smu to load firmware.
>
> Split the smu7/8_start_smu function into two functions
> 1. start_smu, used for load smu firmware in smu7/8 and
>check smu firmware version.
> 2. request_smu_load_fw, used for load other ip's firmware
>on smu7/8 and add firmware loading staus check.
>
> Signed-off-by: Rex Zhu
Don't we need to convert sdma to use this interface as well?
Alex
> ---
> drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 11 +++-
> drivers/gpu/drm/amd/powerplay/amd_powerplay.c | 14 +++--
> drivers/gpu/drm/amd/powerplay/smumgr/fiji_smumgr.c | 4 +-
> .../gpu/drm/amd/powerplay/smumgr/iceland_smumgr.c | 25 -
> .../drm/amd/powerplay/smumgr/polaris10_smumgr.c| 4 +-
> drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.c | 59
> +-
> drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.h | 3 +-
> drivers/gpu/drm/amd/powerplay/smumgr/smu8_smumgr.c | 46 -
> .../gpu/drm/amd/powerplay/smumgr/tonga_smumgr.c| 12 -
> .../gpu/drm/amd/powerplay/smumgr/vegam_smumgr.c| 4 +-
> 10 files changed, 79 insertions(+), 103 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> index 2aeef2b..f020f6f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> @@ -4216,10 +4216,17 @@ static int gfx_v8_0_rlc_resume(struct amdgpu_device
> *adev)
> if (adev->firmware.load_type == AMDGPU_FW_LOAD_DIRECT) {
> /* legacy rlc firmware loading */
> r = gfx_v8_0_rlc_load_microcode(adev);
> - if (r)
> - return r;
> + } else if (adev->firmware.load_type == AMDGPU_FW_LOAD_SMU &&
> adev->powerplay.pp_funcs->load_firmware) {
> + amdgpu_ucode_init_bo(adev);
> + r =
> adev->powerplay.pp_funcs->load_firmware(adev->powerplay.pp_handle);
> + } else {
> + r = -EINVAL;
> }
>
> + 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/powerplay/amd_powerplay.c
> b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
> index aff7c14..3bc825c 100644
> --- a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
> +++ b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
> @@ -124,9 +124,6 @@ static int pp_hw_init(void *handle)
> struct amdgpu_device *adev = handle;
> struct pp_hwmgr *hwmgr = adev->powerplay.pp_handle;
>
> - if (adev->firmware.load_type == AMDGPU_FW_LOAD_SMU)
> - amdgpu_ucode_init_bo(adev);
> -
> ret = hwmgr_hw_init(hwmgr);
>
> if (ret)
> @@ -275,7 +272,16 @@ static int pp_set_clockgating_state(void *handle,
>
> static int pp_dpm_load_fw(void *handle)
> {
> - return 0;
> + struct pp_hwmgr *hwmgr = handle;
> + int ret = 0;
> +
> + if (!hwmgr || !hwmgr->smumgr_funcs)
> + return -EINVAL;
> +
> + if (hwmgr->smumgr_funcs->request_smu_load_fw)
> + ret = hwmgr->smumgr_funcs->request_smu_load_fw(hwmgr);
> +
> + return ret;
> }
>
> static int pp_dpm_fw_loading_complete(void *handle)
> diff --git a/drivers/gpu/drm/amd/powerplay/smumgr/fiji_smumgr.c
> b/drivers/gpu/drm/amd/powerplay/smumgr/fiji_smumgr.c
> index b6b62a7..ffd7d78 100644
> --- a/drivers/gpu/drm/amd/powerplay/smumgr/fiji_smumgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/smumgr/fiji_smumgr.c
> @@ -310,8 +310,6 @@ static int fiji_start_smu(struct pp_hwmgr *hwmgr)
> offsetof(SMU73_Firmware_Header, SoftRegisters),
> &(priv->smu7_data.soft_regs_start), 0x4);
>
> - result = smu7_request_smu_load_fw(hwmgr);
> -
> return result;
> }
>
> @@ -2643,7 +2641,7 @@ static int fiji_update_dpm_settings(struct pp_hwmgr
> *hwmgr,
> .smu_fini = &smu7_smu_fini,
> .start_smu = &fiji_start_smu,
> .check_fw_load_finish = &smu7_check_fw_load_finish,
> - .request_smu_load_fw = &smu7_reload_firmware,
> + .request_smu_load_fw = &smu7_request_smu_load_fw,
> .request_smu_load_specific_fw = NULL,
> .send_msg_to_smc = &smu7_send_msg_to_smc,
> .send_msg_to_smc_with_parameter =
> &smu7_send_msg_to_smc_with_parameter,
> diff --git a/drivers/gpu/drm/amd/powerplay/smumgr/iceland_smumgr.c
> b/drivers/gpu/drm/amd/powerplay/smumgr/iceland_smumgr.c
> index 73aa368..68a4836 100644
> --- a/drivers/gpu/drm/amd/powerplay/smumgr/iceland_smumgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/smumgr/iceland_smumgr.c
> @@ -232,27 +232,24 @@ static int iceland_request_smu_load_specific_fw(struct
> pp_hwmgr *hwmgr,
>
> static int iceland_start_smu(struct pp_hwmgr *hwmgr)
> {
> - int result;
> -
> - result = iceland_s
