[PATCH 2/2] drm/amdgpu: Use the default reset when loading amdgpu driver

[Xu, Feifei]

[AMD Official Use Only - General]

Most asic using mode1 as default, some not. Could check here: soc21_asic_reset_method()/nv_asic_reset_method()
Patch is using default reset method. 

Thanks,
Feifei

-----Original Message-----
From: Li, Lyndon <Lyndon.Li at amd.com> 
Sent: Monday, April 24, 2023 4:01 PM
To: Xu, Feifei <Feifei.Xu at amd.com>; amd-gfx at lists.freedesktop.org
Cc: Liu, Shaoyun <Shaoyun.Liu at amd.com>; Zhao, Victor <Victor.Zhao at amd.com>; Feng, Kenneth <Kenneth.Feng at amd.com>; Li, Yunxiang (Teddy) <Yunxiang.Li at amd.com>
Subject: RE: [PATCH 2/2] drm/amdgpu: Use the default reset when loading amdgpu driver

[AMD Official Use Only - General]

Hi Feifei,

Thanks for your feedback. Will add comments inside and modify commit messages.
I think you are a little misunderstood.

It should do a mode1 reset when loading or reloading the driver, regardless of the module parameter reset_method. 
It will call amdgpu_device_mode1_reset in amdgpu_asic_reset if amdgpu_reset_method is set to AMD_RESET_METHOD_NONE. 
Here's an example,
modprobe amdgpu
modprobe -r amdgpu
modprobe amdgpu reset_method=3 //The real reset method should be mode1 reset, since it is initialization.

Regards,
Lyndon

> -----Original Message-----
> From: Xu, Feifei <Feifei.Xu at amd.com>
> Sent: Monday, April 24, 2023 2:00 PM
> To: Li, Lyndon <Lyndon.Li at amd.com>; amd-gfx at lists.freedesktop.org
> Cc: Liu, Shaoyun <Shaoyun.Liu at amd.com>; Zhao, Victor 
> <Victor.Zhao at amd.com>; Feng, Kenneth <Kenneth.Feng at amd.com>; Li, 
> Yunxiang (Teddy) <Yunxiang.Li at amd.com>; Li, Lyndon <Lyndon.Li at amd.com>
> Subject: RE: [PATCH 2/2] drm/amdgpu: Use the default reset when 
> loading amdgpu driver
> 
> [AMD Official Use Only - General]
> 
> I think you might be refer to : mod parameter reset_method will not 
> affect the loading driver code path. If loading driver, it should use 
> the default reset which might be mode1/mode2 or BACO instead of the specific mode2.
> 
> With the confusing commit msg corrected. And adding comment before the 
> code " r = amdgpu_asic_reset(adev);"
> 
> 
> 
> Reviewed-by: Feifei Xu <Feifei.Xu at amd.com>
> 
> -----Original Message-----
> From: lyndonli <Lyndon.Li at amd.com>
> Sent: Monday, April 24, 2023 9:58 AM
> To: amd-gfx at lists.freedesktop.org
> Cc: Liu, Shaoyun <Shaoyun.Liu at amd.com>; Zhao, Victor 
> <Victor.Zhao at amd.com>; Feng, Kenneth <Kenneth.Feng at amd.com>; Xu, 
> Feifei <Feifei.Xu at amd.com>; Li, Yunxiang (Teddy) 
> <Yunxiang.Li at amd.com>; Li, Lyndon <Lyndon.Li at amd.com>
> Subject: [PATCH 2/2] drm/amdgpu: Use the default reset when loading 
> amdgpu driver
> 
> Below call trace and errors are observed when reloading amdgpu driver 
> with the module parameter reset_method=3.
> 
> It should do a mode1 reset when loading the driver.
> 
> [  +2.180243] [drm] psp gfx command ID_LOAD_TOC(0x20) failed and 
> response status is (0x0) [  +0.000011] [drm:psp_hw_start [amdgpu]]
> *ERROR* Failed to load toc [  +0.000890] [drm:psp_hw_start [amdgpu]]
> *ERROR* PSP tmr init failed!
> [  +0.020683] [drm:amdgpu_fill_buffer [amdgpu]] *ERROR* Trying to 
> clear memory with ring turned off.
> [  +0.000003] RIP: 0010:amdgpu_bo_release_notify+0x1ef/0x210 [amdgpu] 
> [  +0.000004] Call Trace:
> [  +0.000003]  <TASK>
> [  +0.000008]  ttm_bo_release+0x2c4/0x330 [amdttm] [  +0.000026]
> amdttm_bo_put+0x3c/0x70 [amdttm] [  +0.000020]
> amdgpu_bo_free_kernel+0xe6/0x140 [amdgpu] [  +0.000728]
> psp_v11_0_ring_destroy+0x34/0x60 [amdgpu] [  +0.000826]
> psp_hw_init+0xe7/0x2f0 [amdgpu] [  +0.000813]
> amdgpu_device_fw_loading+0x1ad/0x2d0 [amdgpu] [  +0.000731]
> amdgpu_device_init.cold+0x108e/0x2002 [amdgpu] [  +0.001071]  ?
> do_pci_enable_device+0xe1/0x110 [  +0.000011]
> amdgpu_driver_load_kms+0x1a/0x160 [amdgpu] [  +0.000729]
> amdgpu_pci_probe+0x179/0x3a0 [amdgpu]
> 
> Signed-off-by: lyndonli <Lyndon.Li at amd.com>
> Signed-off-by: Yunxiang Li <Yunxiang.Li at amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index e536886f6d42..9738e3660cf1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -3578,6 +3578,7 @@ int amdgpu_device_init(struct amdgpu_device 
> *adev,
>  	int r, i;
>  	bool px = false;
>  	u32 max_MBps;
> +	int tmp;
> 
>  	adev->shutdown = false;
>  	adev->flags = flags;
> @@ -3799,7 +3800,10 @@ int amdgpu_device_init(struct amdgpu_device 
> *adev,
>  				}
>  			}
>  		} else {
> +			tmp = amdgpu_reset_method;
> +			amdgpu_reset_method =
> AMD_RESET_METHOD_NONE;
>  			r = amdgpu_asic_reset(adev);
> +			amdgpu_reset_method = tmp;
>  			if (r) {
>  				dev_err(adev->dev, "asic reset on init failed\n");
>  				goto failed;
> --
> 2.34.1