On 3/29/2024 1:58 PM, Ma Jun wrote:
> refactor the code of runtime pm mode detection to support
> amdgpu_runtime_pm =2 and 1 two cases
>
> Signed-off-by: Ma Jun <Jun.Ma2@xxxxxxx>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 +
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 68 ++++++++++++++++++++++
> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 48 +--------------
> 3 files changed, 70 insertions(+), 47 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 3723235f5818..4358d8c630b2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -1408,6 +1408,7 @@ bool amdgpu_device_supports_px(struct drm_device *dev);
> bool amdgpu_device_supports_boco(struct drm_device *dev);
> bool amdgpu_device_supports_smart_shift(struct drm_device *dev);
> int amdgpu_device_supports_baco(struct drm_device *dev);
> +void amdgpu_device_detect_runtime_pm_mode(struct amdgpu_device *adev);
> bool amdgpu_device_is_peer_accessible(struct amdgpu_device *adev,
> struct amdgpu_device *peer_adev);
> int amdgpu_device_baco_enter(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 410f878462bc..ca117f2666bc 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -350,6 +350,74 @@ int amdgpu_device_supports_baco(struct drm_device *dev)
> return amdgpu_asic_supports_baco(adev);
> }
>
> +void amdgpu_device_detect_runtime_pm_mode(struct amdgpu_device *adev)
> +{
> + struct drm_device *dev;
> + int bamaco_support = 0;
This initialization is not required.
> +
> + dev = adev_to_drm(adev);
> +
> + adev->pm.rpm_mode = AMDGPU_RUNPM_NONE;
> + bamaco_support = amdgpu_device_supports_baco(dev);
> +
> + if (amdgpu_runtime_pm == 2) {
> + if (bamaco_support == (BACO_SUPPORT | MACO_SUPPORT)) {
Better to check only if MACO is supported as that's the user preference.
> + adev->pm.rpm_mode = AMDGPU_RUNPM_BAMACO;
> + dev_info(adev->dev, "Forcing BAMACO for runtime pm\n");
> + } else if (bamaco_support == BACO_SUPPORT) {
> + adev->pm.rpm_mode = AMDGPU_RUNPM_BACO;
> + dev_info(adev->dev, "Forcing BACO for runtime pm\n");
Maybe change this so that user doesn't get confused - "Requested mode
BAMACO not available, fallback to use BACO"
> + }
> + } else if (amdgpu_runtime_pm == 1) {
> + if (bamaco_support == BACO_SUPPORT) {
Use & instead of ==. This could also mean SOC supports both MACO and
BACO, but user prefers BACO for runpm.
> + adev->pm.rpm_mode = AMDGPU_RUNPM_BACO;
> + dev_info(adev->dev, "Forcing BACO for runtime pm\n");
> + }
> + } else if (amdgpu_runtime_pm == -1 || amdgpu_runtime_pm == -2) {
> + 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 {
> + if (!bamaco_support)
> + goto no_runtime_pm;
> +
> + switch (adev->asic_type) {
> + case CHIP_VEGA20:
> + case CHIP_ARCTURUS:
> + /* BACO are not supported on vega20 and arctrus */
> + 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;
> + }
> +
> + 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");
> + }
> + }
> + }
> +
> + } else {
> + dev_info(adev->dev, "runtime pm is manually disabled\n");
> + }
> +
> +no_runtime_pm:
> + if (adev->pm.rpm_mode == AMDGPU_RUNPM_NONE)
> + dev_info(adev->dev, "NO pm mode for runtime pm\n");
Maybe put this differently - "Runtime PM not available"
Thanks,
Lijo
> +}
> /**
> * amdgpu_device_supports_smart_shift - Is the device dGPU with
> * smart shift support
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> index 5d1b084eb631..924baf58e322 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> @@ -133,7 +133,6 @@ 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,52 +149,7 @@ int amdgpu_driver_load_kms(struct amdgpu_device *adev, unsigned long flags)
> goto out;
> }
>
> - 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_runtime_pm != 0) {
> - bamaco_support = amdgpu_device_supports_baco(dev);
> -
> - if (!bamaco_support)
> - goto no_runtime_pm;
> -
> - 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)
> - adev->pm.rpm_mode = AMDGPU_RUNPM_BACO;
> - break;
> - default:
> - /* enable BACO as runpm mode on CI+ */
> - adev->pm.rpm_mode = AMDGPU_RUNPM_BACO;
> - 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");
> - }
> - }
> - }
> -
> -no_runtime_pm:
> - if (adev->pm.rpm_mode == AMDGPU_RUNPM_NONE)
> - dev_info(adev->dev, "NO pm mode for runtime pm\n");
> + amdgpu_device_detect_runtime_pm_mode(adev);
>
> /* Call ACPI methods: require modeset init
> * but failure is not fatal
