[PATCH v6 4/7] drm/amd: Capture errors in amdgpu_switcheroo_set_state()

[Mario Limonciello]

On 10/9/2023 13:40, Alex Deucher wrote:
> On Mon, Oct 9, 2023 at 12:52 PM Mario Limonciello
> <mario.limonciello at amd.com> wrote:
>>
>> amdgpu_switcheroo_set_state() calls lots of functions that could
>> fail under memory pressure or for other reasons.  Don't assume
>> everything can successfully run sequentially, and check return codes
>> for everything that returns one.
> 
> I think the reason we do this rather than returning errors was not
> because we assumed they would be successful, but that it seemed better
> to try and restore the hardware than to bail early.

OK.  If no other feedback for the series I guess for now I'll drop this 
patch from the series when it's committed.

> 
> Alex
> 
>>
>> Acked-by: Christian König <christian.koenig at amd.com>
>> Signed-off-by: Mario Limonciello <mario.limonciello at amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 36 +++++++++++++++++-----
>>   1 file changed, 29 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> index 0f98f720d9ca..65a4537ee6f3 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> @@ -1749,23 +1749,45 @@ static void amdgpu_switcheroo_set_state(struct pci_dev *pdev,
>>                  /* don't suspend or resume card normally */
>>                  dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
>>
>> -               pci_set_power_state(pdev, PCI_D0);
>> -               amdgpu_device_load_pci_state(pdev);
>> +               r = pci_set_power_state(pdev, PCI_D0);
>> +               if (r) {
>> +                       DRM_WARN("pci_set_power_state failed (%d)\n", r);
>> +                       return;
>> +               }
>> +               if (!amdgpu_device_load_pci_state(pdev))
>> +                       return;
>>                  r = pci_enable_device(pdev);
>>                  if (r)
>>                          DRM_WARN("pci_enable_device failed (%d)\n", r);
>> -               amdgpu_device_resume(dev, true);
>> +               r = amdgpu_device_resume(dev, true);
>> +               if (r) {
>> +                       DRM_WARN("amdgpu_device_resume failed (%d)\n", r);
>> +                       return;
>> +               }
>>
>>                  dev->switch_power_state = DRM_SWITCH_POWER_ON;
>>          } else {
>>                  pr_info("switched off\n");
>>                  dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
>> -               amdgpu_device_prepare(dev);
>> -               amdgpu_device_suspend(dev, true);
>> -               amdgpu_device_cache_pci_state(pdev);
>> +               r = amdgpu_device_prepare(dev);
>> +               if (r) {
>> +                       DRM_WARN("amdgpu_device_prepare failed (%d)\n", r);
>> +                       return;
>> +               }
>> +               r = amdgpu_device_suspend(dev, true);
>> +               if (r) {
>> +                       DRM_WARN("amdgpu_device_suspend failed (%d)\n", r);
>> +                       return;
>> +               }
>> +               if (!amdgpu_device_cache_pci_state(pdev))
>> +                       return;
>>                  /* Shut down the device */
>>                  pci_disable_device(pdev);
>> -               pci_set_power_state(pdev, PCI_D3cold);
>> +               r = pci_set_power_state(pdev, PCI_D3cold);
>> +               if (r) {
>> +                       DRM_WARN("pci_set_power_state failed (%d)\n", r);
>> +                       return;
>> +               }
>>                  dev->switch_power_state = DRM_SWITCH_POWER_OFF;
>>          }
>>   }
>> --
>> 2.34.1
>>