[PATCH v3 5/8] drm/amdgpu: Fix consecutive DPC recovery failures.

Luben Tuikov luben.tuikov at amd.com
Mon Aug 31 21:05:50 UTC 2020


On 2020-08-31 11:50 a.m., Andrey Grodzovsky wrote:
> Cache the PCI state on boot and before each case were we might
> loose it.

Typo: "were" --> "where".

> 
> v2: Add pci_restore_state while caching the PCI state to avoid
> breaking PCI core logic for stuff like suspend/resume.
> 
> v3: Extract pci_restore_state from amdgpu_device_cache_pci_state
> to avoid superflous restores during GPU resets and suspend/resumes.
> 
> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky at amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu.h        |  6 +++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 62 ++++++++++++++++++++++++++++--
>  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c    |  4 +-
>  drivers/gpu/drm/amd/amdgpu/nv.c            |  4 +-
>  drivers/gpu/drm/amd/amdgpu/soc15.c         |  4 +-
>  5 files changed, 71 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index cac51e8..5e74db6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -992,7 +992,9 @@ struct amdgpu_device {
>  	atomic_t			throttling_logging_enabled;
>  	struct ratelimit_state		throttling_logging_rs;
>  	uint32_t			ras_features;
> +
>  	bool                            in_pci_err_recovery;
> +	struct pci_saved_state          *pci_state;
>  };
>  
>  static inline struct amdgpu_device *drm_to_adev(struct drm_device *ddev)
> @@ -1272,6 +1274,10 @@ pci_ers_result_t amdgpu_pci_mmio_enabled(struct pci_dev *pdev);
>  pci_ers_result_t amdgpu_pci_slot_reset(struct pci_dev *pdev);
>  void amdgpu_pci_resume(struct pci_dev *pdev);
>  
> +bool amdgpu_device_cache_pci_state(struct pci_dev *pdev);
> +bool amdgpu_device_load_pci_state(struct pci_dev *pdev);
> +
> +

Unnecessary extra line added.

Regards,
Luben

>  
>  #include "amdgpu_object.h"
>  
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 4a009ca..0329b43 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -1282,7 +1282,7 @@ static void amdgpu_switcheroo_set_state(struct pci_dev *pdev,
>  		dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
>  
>  		pci_set_power_state(dev->pdev, PCI_D0);
> -		pci_restore_state(dev->pdev);
> +		amdgpu_device_load_pci_state(dev->pdev);
>  		r = pci_enable_device(dev->pdev);
>  		if (r)
>  			DRM_WARN("pci_enable_device failed (%d)\n", r);
> @@ -1295,7 +1295,7 @@ static void amdgpu_switcheroo_set_state(struct pci_dev *pdev,
>  		drm_kms_helper_poll_disable(dev);
>  		dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
>  		amdgpu_device_suspend(dev, true);
> -		pci_save_state(dev->pdev);
> +		amdgpu_device_cache_pci_state(dev->pdev);
>  		/* Shut down the device */
>  		pci_disable_device(dev->pdev);
>  		pci_set_power_state(dev->pdev, PCI_D3cold);
> @@ -3400,6 +3400,10 @@ int amdgpu_device_init(struct amdgpu_device *adev,
>  	if (r)
>  		dev_err(adev->dev, "amdgpu_pmu_init failed\n");
>  
> +	/* Have stored pci confspace at hand for restore in sudden PCI error */
> +	if (amdgpu_device_cache_pci_state(adev->pdev))
> +		pci_restore_state(pdev);
> +
>  	return 0;
>  
>  failed:
> @@ -3426,6 +3430,8 @@ void amdgpu_device_fini(struct amdgpu_device *adev)
>  	flush_delayed_work(&adev->delayed_init_work);
>  	adev->shutdown = true;
>  
> +	kfree(adev->pci_state);
> +
>  	/* make sure IB test finished before entering exclusive mode
>  	 * to avoid preemption on IB test
>  	 * */
> @@ -4850,7 +4856,7 @@ pci_ers_result_t amdgpu_pci_slot_reset(struct pci_dev *pdev)
>  	/* wait for asic to come out of reset */
>  	msleep(500);
>  
> -	pci_restore_state(pdev);
> +	amdgpu_device_load_pci_state(pdev);
>  
>  	/* confirm  ASIC came out of reset */
>  	for (i = 0; i < adev->usec_timeout; i++) {
> @@ -4930,6 +4936,9 @@ pci_ers_result_t amdgpu_pci_slot_reset(struct pci_dev *pdev)
>  out:
>  
>  	if (!r) {
> +		if (amdgpu_device_cache_pci_state(adev->pdev))
> +			pci_restore_state(adev->pdev);
> +
>  		DRM_INFO("PCIe error recovery succeeded\n");
>  	} else {
>  		DRM_ERROR("PCIe error recovery failed, err:%d", r);
> @@ -4969,3 +4978,50 @@ void amdgpu_pci_resume(struct pci_dev *pdev)
>  
>  	amdgpu_device_unlock_adev(adev);
>  }
> +
> +bool amdgpu_device_cache_pci_state(struct pci_dev *pdev)
> +{
> +	struct drm_device *dev = pci_get_drvdata(pdev);
> +	struct amdgpu_device *adev = drm_to_adev(dev);
> +	int r;
> +
> +	r = pci_save_state(pdev);
> +	if (!r) {
> +		kfree(adev->pci_state);
> +
> +		adev->pci_state = pci_store_saved_state(pdev);
> +
> +		if (!adev->pci_state) {
> +			DRM_ERROR("Failed to store PCI saved state");
> +			return false;
> +		}
> +	} else {
> +		DRM_WARN("Failed to save PCI state, err:%d\n", r);
> +		return false;
> +	}
> +
> +	return true;
> +}
> +
> +bool amdgpu_device_load_pci_state(struct pci_dev *pdev)
> +{
> +	struct drm_device *dev = pci_get_drvdata(pdev);
> +	struct amdgpu_device *adev = drm_to_adev(dev);
> +	int r;
> +
> +	if (!adev->pci_state)
> +		return false;
> +
> +	r = pci_load_saved_state(pdev, adev->pci_state);
> +
> +	if (!r) {
> +		pci_restore_state(pdev);
> +	} else {
> +		DRM_WARN("Failed to load PCI state, err:%d\n", r);
> +		return false;
> +	}
> +
> +	return true;
> +}
> +
> +
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 4bbcc70..7a6482a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -1320,7 +1320,7 @@ static int amdgpu_pmops_runtime_suspend(struct device *dev)
>  		if (amdgpu_is_atpx_hybrid()) {
>  			pci_ignore_hotplug(pdev);
>  		} else {
> -			pci_save_state(pdev);
> +			amdgpu_device_cache_pci_state(pdev);
>  			pci_disable_device(pdev);
>  			pci_ignore_hotplug(pdev);
>  			pci_set_power_state(pdev, PCI_D3cold);
> @@ -1353,7 +1353,7 @@ static int amdgpu_pmops_runtime_resume(struct device *dev)
>  			pci_set_master(pdev);
>  		} else {
>  			pci_set_power_state(pdev, PCI_D0);
> -			pci_restore_state(pdev);
> +			amdgpu_device_load_pci_state(pdev);
>  			ret = pci_enable_device(pdev);
>  			if (ret)
>  				return ret;
> diff --git a/drivers/gpu/drm/amd/amdgpu/nv.c b/drivers/gpu/drm/amd/amdgpu/nv.c
> index 4d14023..0ec6603 100644
> --- a/drivers/gpu/drm/amd/amdgpu/nv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/nv.c
> @@ -311,7 +311,7 @@ static int nv_asic_mode1_reset(struct amdgpu_device *adev)
>  	/* disable BM */
>  	pci_clear_master(adev->pdev);
>  
> -	pci_save_state(adev->pdev);
> +	amdgpu_device_cache_pci_state(adev->pdev);
>  
>  	if (amdgpu_dpm_is_mode1_reset_supported(adev)) {
>  		dev_info(adev->dev, "GPU smu mode1 reset\n");
> @@ -323,7 +323,7 @@ static int nv_asic_mode1_reset(struct amdgpu_device *adev)
>  
>  	if (ret)
>  		dev_err(adev->dev, "GPU mode1 reset failed\n");
> -	pci_restore_state(adev->pdev);
> +	amdgpu_device_load_pci_state(adev->pdev);
>  
>  	/* wait for asic to come out of reset */
>  	for (i = 0; i < adev->usec_timeout; i++) {
> diff --git a/drivers/gpu/drm/amd/amdgpu/soc15.c b/drivers/gpu/drm/amd/amdgpu/soc15.c
> index 2f93c47..ddd55e3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/soc15.c
> +++ b/drivers/gpu/drm/amd/amdgpu/soc15.c
> @@ -484,13 +484,13 @@ static int soc15_asic_mode1_reset(struct amdgpu_device *adev)
>  	/* disable BM */
>  	pci_clear_master(adev->pdev);
>  
> -	pci_save_state(adev->pdev);
> +	amdgpu_device_cache_pci_state(adev->pdev);
>  
>  	ret = psp_gpu_reset(adev);
>  	if (ret)
>  		dev_err(adev->dev, "GPU mode1 reset failed\n");
>  
> -	pci_restore_state(adev->pdev);
> +	amdgpu_device_load_pci_state(adev->pdev);
>  
>  	/* wait for asic to come out of reset */
>  	for (i = 0; i < adev->usec_timeout; i++) {
> 



More information about the amd-gfx mailing list