Re: [PATCH 3/3] drm/amd: Drop calls for checking "support" for BACO/BOCO/PX

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Nov 29, 2023 at 6:17 AM Lazar, Lijo <lijo.lazar@xxxxxxx> wrote:
>
>
>
> 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.

Right.  There are cases where we use BACO for things besides runtime
pm.  E.g., GPU reset.

Alex

>
> 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@xxxxxxx>
> > ---
> >   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)




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux