[PATCH 3/3] drm/amd: Drop calls for checking "support" for BACO/BOCO/PX
Lazar, Lijo
lijo.lazar at amd.com
Wed Nov 29 10:59:19 UTC 2023
On 11/29/2023 12:22 AM, Mario Limonciello wrote:
> As the module parameter can be used to control behavior, all parts
> of the driver should obey what has been programmed by user or
> detected by auto mode rather than what "can" be supported.
>
This is also not correct. You can very well disable runpm mode and the
rest of the logic will still apply. A substitution like this doesn't
work when runpm mode is disabled. Only in cases where the support check
is used for runpm related logic, this replacement can be applied.
Thanks,
Lijo
> Drop calls to all functions that check for BACO/BOCO/PX runpm modes
> and instead use the variable that is programmed when device is probed.
>
> Signed-off-by: Mario Limonciello <mario.limonciello at amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 7 +++--
> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 34 ++++++++++++----------
> drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 3 +-
> 3 files changed, 24 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 1181fe4baf0f..8f7377b37f2f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -1822,9 +1822,10 @@ static void amdgpu_switcheroo_set_state(struct pci_dev *pdev,
> enum vga_switcheroo_state state)
> {
> struct drm_device *dev = pci_get_drvdata(pdev);
> + struct amdgpu_device *adev = drm_to_adev(dev);
> int r;
>
> - if (amdgpu_device_supports_px(dev) && state == VGA_SWITCHEROO_OFF)
> + if (adev->pm.rpm_mode == AMDGPU_RUNPM_PX && state == VGA_SWITCHEROO_OFF)
> return;
>
> if (state == VGA_SWITCHEROO_ON) {
> @@ -4244,7 +4245,7 @@ int amdgpu_device_init(struct amdgpu_device *adev,
> if ((adev->pdev->class >> 8) == PCI_CLASS_DISPLAY_VGA)
> vga_client_register(adev->pdev, amdgpu_device_vga_set_decode);
>
> - px = amdgpu_device_supports_px(ddev);
> + px = (adev->pm.rpm_mode == AMDGPU_RUNPM_PX);
>
> if (px || (!dev_is_removable(&adev->pdev->dev) &&
> apple_gmux_detect(NULL, NULL)))
> @@ -4392,7 +4393,7 @@ void amdgpu_device_fini_sw(struct amdgpu_device *adev)
> kfree(adev->fru_info);
> adev->fru_info = NULL;
>
> - px = amdgpu_device_supports_px(adev_to_drm(adev));
> + px = (adev->pm.rpm_mode == AMDGPU_RUNPM_PX);
>
> if (px || (!dev_is_removable(&adev->pdev->dev) &&
> apple_gmux_detect(NULL, NULL)))
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index e39f3a334c9d..12fb8398fb45 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -2248,10 +2248,10 @@ static int amdgpu_pci_probe(struct pci_dev *pdev,
>
> if (adev->pm.rpm_mode != AMDGPU_RUNPM_NONE) {
> /* only need to skip on ATPX */
> - if (amdgpu_device_supports_px(ddev))
> + if (adev->pm.rpm_mode == AMDGPU_RUNPM_PX)
> dev_pm_set_driver_flags(ddev->dev, DPM_FLAG_NO_DIRECT_COMPLETE);
> /* we want direct complete for BOCO */
> - if (amdgpu_device_supports_boco(ddev))
> + if (adev->pm.rpm_mode == AMDGPU_RUNPM_BOCO)
> dev_pm_set_driver_flags(ddev->dev, DPM_FLAG_SMART_PREPARE |
> DPM_FLAG_SMART_SUSPEND |
> DPM_FLAG_MAY_SKIP_RESUME);
> @@ -2284,7 +2284,8 @@ static int amdgpu_pci_probe(struct pci_dev *pdev,
> * into D0 state. Then there will be a PMFW-aware D-state
> * transition(D0->D3) on runpm suspend.
> */
> - if (amdgpu_device_supports_baco(ddev) &&
> + if ((adev->pm.rpm_mode == AMDGPU_RUNPM_BACO ||
> + adev->pm.rpm_mode == AMDGPU_RUNPM_BAMACO) &&
> !(adev->flags & AMD_IS_APU) &&
> (adev->asic_type >= CHIP_NAVI10))
> amdgpu_get_secondary_funcs(adev);
> @@ -2466,7 +2467,7 @@ static int amdgpu_pmops_prepare(struct device *dev)
> /* Return a positive number here so
> * DPM_FLAG_SMART_SUSPEND works properly
> */
> - if (amdgpu_device_supports_boco(drm_dev) &&
> + if (adev->pm.rpm_mode == AMDGPU_RUNPM_BOCO &&
> pm_runtime_suspended(dev))
> return 1;
>
> @@ -2664,7 +2665,7 @@ static int amdgpu_pmops_runtime_suspend(struct device *dev)
> }
>
> adev->in_runpm = true;
> - if (amdgpu_device_supports_px(drm_dev))
> + if (adev->pm.rpm_mode == AMDGPU_RUNPM_PX)
> drm_dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
>
> /*
> @@ -2674,7 +2675,7 @@ static int amdgpu_pmops_runtime_suspend(struct device *dev)
> * platforms.
> * TODO: this may be also needed for PX capable platform.
> */
> - if (amdgpu_device_supports_boco(drm_dev))
> + if (adev->pm.rpm_mode == AMDGPU_RUNPM_BOCO)
> adev->mp1_state = PP_MP1_STATE_UNLOAD;
>
> ret = amdgpu_device_prepare(drm_dev);
> @@ -2683,15 +2684,15 @@ static int amdgpu_pmops_runtime_suspend(struct device *dev)
> ret = amdgpu_device_suspend(drm_dev, false);
> if (ret) {
> adev->in_runpm = false;
> - if (amdgpu_device_supports_boco(drm_dev))
> + if (adev->pm.rpm_mode == AMDGPU_RUNPM_BOCO)
> adev->mp1_state = PP_MP1_STATE_NONE;
> return ret;
> }
>
> - if (amdgpu_device_supports_boco(drm_dev))
> + if (adev->pm.rpm_mode == AMDGPU_RUNPM_BOCO)
> adev->mp1_state = PP_MP1_STATE_NONE;
>
> - if (amdgpu_device_supports_px(drm_dev)) {
> + if (adev->pm.rpm_mode == AMDGPU_RUNPM_PX) {
> /* Only need to handle PCI state in the driver for ATPX
> * PCI core handles it for _PR3.
> */
> @@ -2700,9 +2701,10 @@ static int amdgpu_pmops_runtime_suspend(struct device *dev)
> pci_ignore_hotplug(pdev);
> pci_set_power_state(pdev, PCI_D3cold);
> drm_dev->switch_power_state = DRM_SWITCH_POWER_DYNAMIC_OFF;
> - } else if (amdgpu_device_supports_boco(drm_dev)) {
> + } else if (adev->pm.rpm_mode == AMDGPU_RUNPM_BOCO) {
> /* nothing to do */
> - } else if (amdgpu_device_supports_baco(drm_dev)) {
> + } else if (adev->pm.rpm_mode == AMDGPU_RUNPM_BACO ||
> + adev->pm.rpm_mode == AMDGPU_RUNPM_BAMACO) {
> amdgpu_device_baco_enter(drm_dev);
> }
>
> @@ -2725,7 +2727,7 @@ static int amdgpu_pmops_runtime_resume(struct device *dev)
> if (!pci_device_is_present(adev->pdev))
> adev->no_hw_access = true;
>
> - if (amdgpu_device_supports_px(drm_dev)) {
> + if (adev->pm.rpm_mode == AMDGPU_RUNPM_PX) {
> drm_dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
>
> /* Only need to handle PCI state in the driver for ATPX
> @@ -2737,22 +2739,22 @@ static int amdgpu_pmops_runtime_resume(struct device *dev)
> if (ret)
> return ret;
> pci_set_master(pdev);
> - } else if (amdgpu_device_supports_boco(drm_dev)) {
> + } else if (adev->pm.rpm_mode == AMDGPU_RUNPM_BOCO) {
> /* Only need to handle PCI state in the driver for ATPX
> * PCI core handles it for _PR3.
> */
> pci_set_master(pdev);
> - } else if (amdgpu_device_supports_baco(drm_dev)) {
> + } else if (adev->pm.rpm_mode == AMDGPU_RUNPM_BACO) {
> amdgpu_device_baco_exit(drm_dev);
> }
> ret = amdgpu_device_resume(drm_dev, false);
> if (ret) {
> - if (amdgpu_device_supports_px(drm_dev))
> + if (adev->pm.rpm_mode == AMDGPU_RUNPM_PX)
> pci_disable_device(pdev);
> return ret;
> }
>
> - if (amdgpu_device_supports_px(drm_dev))
> + if (adev->pm.rpm_mode == AMDGPU_RUNPM_PX)
> drm_dev->switch_power_state = DRM_SWITCH_POWER_ON;
> adev->in_runpm = false;
> return 0;
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> index f464610a959f..d7977185f4e2 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> @@ -1618,7 +1618,8 @@ static int smu_disable_dpms(struct smu_context *smu)
> bool use_baco = !smu->is_apu &&
> ((amdgpu_in_reset(adev) &&
> (amdgpu_asic_reset_method(adev) == AMD_RESET_METHOD_BACO)) ||
> - ((adev->in_runpm || adev->in_s4) && amdgpu_asic_supports_baco(adev)));
> + ((adev->in_runpm || adev->in_s4) &&
> + (adev->pm.rpm_mode == AMDGPU_RUNPM_BACO || adev->pm.rpm_mode == AMDGPU_RUNPM_BAMACO)));
>
> /*
> * For SMU 13.0.0 and 13.0.7, PMFW will handle the DPM features(disablement or others)
More information about the amd-gfx
mailing list