On 3/26/2024 5:29 PM, Lazar, Lijo wrote:
> 
> 
> On 3/25/2024 3:45 PM, Ma Jun wrote:
>> Optimize the code to add support for BAMACO mode checking
>>
>> Signed-off-by: Ma Jun <jun....@amd.com>
>> ---
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c |  4 +-
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 74 +++++++++++++++----------
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c |  4 +-
>>  3 files changed, 50 insertions(+), 32 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>> index 80b9642f2bc4..e267ac032a1c 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>> @@ -2734,7 +2734,7 @@ static int amdgpu_pmops_runtime_suspend(struct device 
>> *dev)
>>              drm_dev->switch_power_state = DRM_SWITCH_POWER_DYNAMIC_OFF;
>>      } else if (adev->pm.rpm_mode == AMDGPU_RUNPM_BOCO) {
>>              /* nothing to do */
>> -    } else if (adev->pm.rpm_mode == AMDGPU_RUNPM_BACO) {
>> +    } else if (adev->pm.rpm_mode >= AMDGPU_RUNPM_BACO) {
> 
> This kind of checking doesn't work well if we have to add new RPM modes.
> Instead use || or a wrapper.

Thanks, will fix in v2.

Regards,
Ma Jun
> 
> Thanks,
> Lijo
> 
>>              amdgpu_device_baco_enter(drm_dev);
>>      }
>>  
>> @@ -2774,7 +2774,7 @@ static int amdgpu_pmops_runtime_resume(struct device 
>> *dev)
>>               * PCI core handles it for _PR3.
>>               */
>>              pci_set_master(pdev);
>> -    } else if (adev->pm.rpm_mode == AMDGPU_RUNPM_BACO) {
>> +    } else if (adev->pm.rpm_mode >= AMDGPU_RUNPM_BACO) {
>>              amdgpu_device_baco_exit(drm_dev);
>>      }
>>      ret = amdgpu_device_resume(drm_dev, false);
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>> index a66d47865e3b..81bb0a2c8227 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>> @@ -133,6 +133,7 @@ void amdgpu_register_gpu_instance(struct amdgpu_device 
>> *adev)
>>  int amdgpu_driver_load_kms(struct amdgpu_device *adev, unsigned long flags)
>>  {
>>      struct drm_device *dev;
>> +    int bamaco_support = 0;
>>      int r, acpi_status;
>>  
>>      dev = adev_to_drm(adev);
>> @@ -150,38 +151,55 @@ int amdgpu_driver_load_kms(struct amdgpu_device *adev, 
>> unsigned long flags)
>>      }
>>  
>>      adev->pm.rpm_mode = AMDGPU_RUNPM_NONE;
>> -    if (amdgpu_device_supports_px(dev) &&
>> -        (amdgpu_runtime_pm != 0)) { /* enable PX as runtime mode */
>> -            adev->pm.rpm_mode = AMDGPU_RUNPM_PX;
>> -            dev_info(adev->dev, "Using ATPX for runtime pm\n");
>> -    } else if (amdgpu_device_supports_boco(dev) &&
>> -               (amdgpu_runtime_pm != 0)) { /* enable boco as runtime mode */
>> -            adev->pm.rpm_mode = AMDGPU_RUNPM_BOCO;
>> -            dev_info(adev->dev, "Using BOCO for runtime pm\n");
>> -    } else if (amdgpu_device_supports_baco(dev) &&
>> -               (amdgpu_runtime_pm != 0)) {
>> -            switch (adev->asic_type) {
>> -            case CHIP_VEGA20:
>> -            case CHIP_ARCTURUS:
>> -                    /* enable BACO as runpm mode if runpm=1 */
>> -                    if (amdgpu_runtime_pm > 0)
>> -                            adev->pm.rpm_mode = AMDGPU_RUNPM_BACO;
>> -                    break;
>> -            case CHIP_VEGA10:
>> -                    /* enable BACO as runpm mode if noretry=0 */
>> -                    if (!adev->gmc.noretry)
>> +    if (amdgpu_runtime_pm == 2) {
>> +            adev->pm.rpm_mode = AMDGPU_RUNPM_BAMACO;
>> +            dev_info(adev->dev, "Forcing BAMACO for runtime pm\n");
>> +    } else if (amdgpu_runtime_pm == 1) {
>> +            adev->pm.rpm_mode = AMDGPU_RUNPM_BACO;
>> +            dev_info(adev->dev, "Forcing BACO for runtime pm\n");
>> +    } else if (amdgpu_runtime_pm != 0) {
>> +            if (amdgpu_device_supports_px(dev)) { /* enable PX as runtime 
>> mode */
>> +                    adev->pm.rpm_mode = AMDGPU_RUNPM_PX;
>> +                    dev_info(adev->dev, "Using ATPX for runtime pm\n");
>> +            } else if (amdgpu_device_supports_boco(dev)) { /* enable boco 
>> as runtime mode */
>> +                    adev->pm.rpm_mode = AMDGPU_RUNPM_BOCO;
>> +                    dev_info(adev->dev, "Using BOCO for runtime pm\n");
>> +            } else {
>> +                    bamaco_support = amdgpu_device_supports_baco(dev);
>> +
>> +                    if (!bamaco_support)
>> +                            goto no_runtime_pm;
>> +
>> +                    switch (adev->asic_type) {
>> +                    case CHIP_VEGA20:
>> +                    case CHIP_ARCTURUS:
>> +                            /* vega20 and arcturus don't support runtime pm 
>> */
>> +                            break;
>> +                    case CHIP_VEGA10:
>> +                            /* enable BACO as runpm mode if noretry=0 */
>> +                            if (!adev->gmc.noretry)
>> +                                    adev->pm.rpm_mode = AMDGPU_RUNPM_BACO;
>> +                            break;
>> +                    default:
>> +                            /* enable BACO as runpm mode on CI+ */
>>                              adev->pm.rpm_mode = AMDGPU_RUNPM_BACO;
>> -                    break;
>> -            default:
>> -                    /* enable BACO as runpm mode on CI+ */
>> -                    adev->pm.rpm_mode = AMDGPU_RUNPM_BACO;
>> -                    break;
>> +                            break;
>> +                    }
>> +                    if (adev->pm.rpm_mode == AMDGPU_RUNPM_BACO) {
>> +                            if (bamaco_support & MACO_SUPPORT) {
>> +                                    adev->pm.rpm_mode = AMDGPU_RUNPM_BAMACO;
>> +                                    dev_info(adev->dev, "Using BAMACO for 
>> runtime pm\n");
>> +                            } else {
>> +                                    dev_info(adev->dev, "Using BACO for 
>> runtime pm\n");
>> +                            }
>> +                    }
>>              }
>> -
>> -            if (adev->pm.rpm_mode == AMDGPU_RUNPM_BACO)
>> -                    dev_info(adev->dev, "Using BACO for runtime pm\n");
>>      }
>>  
>> +no_runtime_pm:
>> +    if (adev->pm.rpm_mode == AMDGPU_RUNPM_NONE)
>> +            dev_info(adev->dev, "NO pm mode for runtime pm\n");
>> +
>>      /* Call ACPI methods: require modeset init
>>       * but failure is not fatal
>>       */
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
>> index 94b310fdb719..b4702a7961ec 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
>> @@ -2617,7 +2617,7 @@ static int psp_load_p2s_table(struct psp_context *psp)
>>      struct amdgpu_firmware_info *ucode =
>>              &adev->firmware.ucode[AMDGPU_UCODE_ID_P2S_TABLE];
>>  
>> -    if (adev->in_runpm && (adev->pm.rpm_mode == AMDGPU_RUNPM_BACO))
>> +    if (adev->in_runpm && (adev->pm.rpm_mode >= AMDGPU_RUNPM_BACO))
>>              return 0;
>>  
>>      if (amdgpu_ip_version(adev, MP0_HWIP, 0) == IP_VERSION(13, 0, 6)) {
>> @@ -2647,7 +2647,7 @@ static int psp_load_smu_fw(struct psp_context *psp)
>>       * Skip SMU FW reloading in case of using BACO for runpm only,
>>       * as SMU is always alive.
>>       */
>> -    if (adev->in_runpm && (adev->pm.rpm_mode == AMDGPU_RUNPM_BACO))
>> +    if (adev->in_runpm && (adev->pm.rpm_mode >= AMDGPU_RUNPM_BACO))
>>              return 0;
>>  
>>      if (!ucode->fw || amdgpu_sriov_vf(psp->adev))

Reply via email to