>-Original Message-
>From: Alex Deucher
>Sent: Saturday, May 25, 2019 12:59 AM
>To: Deng, Emily
>Cc: amd-gfx list
>Subject: Re: [PATCH] drm/amdgpu: Need to set the baco cap before baco
>reset
>
>[CAUTION: External Email]
>
>On Thu, May 23, 2019 at 10:29 PM Deng, Emily wrote:
>>
>>
>>
>> >-Original Message-
>> >From: Alex Deucher
>> >Sent: Friday, May 24, 2019 12:09 AM
>> >To: Deng, Emily
>> >Cc: amd-gfx list
>> >Subject: Re: [PATCH] drm/amdgpu: Need to set the baco cap before baco
>> >reset
>> >
>> >[CAUTION: External Email]
>> >
>> >On Thu, May 23, 2019 at 6:22 AM Emily Deng
>wrote:
>> >>
>> >> For passthrough, after rebooted the VM, driver will do a baco reset
>> >> before doing other driver initialization during loading driver.
>> >> For doing the baco reset, it will first check the baco reset capability.
>> >> So first need to set the cap from the vbios information or baco
>> >> reset won't be enabled.
>> >>
>> >> Signed-off-by: Emily Deng
>> >> ---
>> >> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 8
>> >> drivers/gpu/drm/amd/amdgpu/soc15.c | 3 ++-
>> >> drivers/gpu/drm/amd/include/kgd_pp_interface.h | 1 +
>> >> drivers/gpu/drm/amd/powerplay/amd_powerplay.c | 16
>> >+++
>> >> drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c | 1 +
>> >> .../amd/powerplay/hwmgr/vega10_processpptables.c | 24
>> >++
>> >> .../amd/powerplay/hwmgr/vega10_processpptables.h | 1 +
>> >> drivers/gpu/drm/amd/powerplay/inc/hwmgr.h | 1 +
>> >> 8 files changed, 54 insertions(+), 1 deletion(-)
>> >>
>> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> >> index bdd1fe73..2dde672 100644
>> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> >> @@ -2611,6 +2611,14 @@ int amdgpu_device_init(struct
>amdgpu_device
>> >*adev,
>> >> * E.g., driver was not cleanly unloaded previously, etc.
>> >> */
>> >> if (!amdgpu_sriov_vf(adev) &&
>> >> amdgpu_asic_need_reset_on_init(adev)) {
>> >> + if (amdgpu_passthrough(adev) &&
>> >> + adev->powerplay.pp_funcs &&
>> >adev->powerplay.pp_funcs->set_asic_baco_cap) {
>> >> + r =
>> >> + adev->powerplay.pp_funcs->set_asic_baco_cap(adev-
>> >>powerplay.pp_handle);
>> >> + if (r) {
>> >> + dev_err(adev->dev, "set baco capability
>> >> failed\n");
>> >> + goto failed;
>> >> + }
>> >> + }
>> >> +
>> >
>> >I think it would be cleaner to add this to hwmgr_early_init() or
>> >something called from early init for powerplay.
>> I also preferred to put it in the hwmgr_early_init, but as the function
>set_asic_baco_cap need to get the vbios info, so need to put the
>amdgpu_get_bios before early init. If so the code changes too big.
>
>I think this change is all you need:
>diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>index bdd1fe73f14b..952f61e28d42 100644
>--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>@@ -2564,6 +2564,12 @@ int amdgpu_device_init(struct amdgpu_device
>*adev,
>
>amdgpu_device_get_pcie_info(adev);
>
>+ /* Read BIOS */
>+ if (!amdgpu_get_bios(adev)) {
>+ r = -EINVAL;
>+ goto failed;
>+ }
>+
>/* early init functions */
>r = amdgpu_device_ip_early_init(adev);
>if (r)
>@@ -2591,12 +2597,6 @@ int amdgpu_device_init(struct amdgpu_device
>*adev,
>goto fence_driver_init;
>}
>
>- /* Read BIOS */
>- if (!amdgpu_get_bios(adev)) {
>- r = -EINVAL;
>- goto failed;
>- }
>-
>r = amdgpu_atombios_init(adev);
>if (r) {
>dev_err(adev->dev, "amdgpu_atombios_init failed\n");
>
>I guess that could be a follow up change if you want.
>
>Alex
Thanks, will modify the change as your suggestion.
Best wishes
Emily Deng
>
>> >
>> >Alex
>> >
>> >> r = amdgpu_asic_reset(adev);
>> >> if (r) {
>> >> dev_err(adev->dev, "asic reset on init
>> >> failed\n"); diff --git a/drivers/gpu/drm/amd/amdgpu/soc15.c
>> >> b/drivers/gpu/drm/amd/amdgpu/soc15.c
>> >> index 78bd4fc..d9fdd95 100644
>> >> --- a/drivers/gpu/drm/amd/amdgpu/soc15.c
>> >> +++ b/drivers/gpu/drm/amd/amdgpu/soc15.c
>> >> @@ -764,7 +764,8 @@ static bool soc15_need_reset_on_init(struct
>> >amdgpu_device *adev)
>> >> /* Just return false for soc15 GPUs. Reset does not seem to
>> >> * be necessary.
>> >> */
>> >> - return false;
>> >> + if (!amdgpu_passthrough(adev))
>> >> + return false;
>> >>
>> >> if (adev->flags & AMD_IS_APU)
>> >>
