On Thu, Mar 21, 2024 at 2:52 AM Ma, Jun <majun@xxxxxxx> wrote:
>
>
>
> On 3/20/2024 9:24 PM, Alex Deucher wrote:
> > On Wed, Mar 20, 2024 at 6:17 AM Ma Jun <Jun.Ma2@xxxxxxx> wrote:
> >>
> >> Because of the logic error, Arcturus and vega20 currently
> >> use the AMDGPU_RUNPM_NONE for runtime pm even though they
> >> support BACO. So, the code is optimized to fix this error.
> >>
> >> Signed-off-by: Ma Jun <Jun.Ma2@xxxxxxx>
> >> ---
> >> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 56 ++++++++++++-------------
> >> 1 file changed, 27 insertions(+), 29 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> >> index 1f92fb1e7421..70cf2d0c7683 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> >> @@ -150,42 +150,40 @@ 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 > 0) {
> >> + adev->pm.rpm_mode = AMDGPU_RUNPM_BACO;
> >> + dev_info(adev->dev, "Forcing BACO for runtime pm\n");
> >
> > Does this need special handling for BAMACO? Setting
> > amdgpu_runtime_pm=2 is supposed to set BAMACO and 1 is supposed to
> > force BACO.
> >
>
> BAMACO currently is detected in pptable_funcs->baco_enter() and baco_exit function.
> We just need to set BACO flag here.
>
> How about adding a new mode AMDGPU_RUNPM_BAMACO, so we only have to check
> amdgpu_runtime_pm and detect the PM mode in initialization stage and don't
> need to handle it later.
Sure that works, although we still need to verify that it is supported
on the platform or not so I think we still need to check if BAMACO is
supported in the baco_enter function and then fall back to BACO if
BAMACO is not supported.
Alex
>
> Regards,
> Ma Jun
>
> > Alex
> >
> >> + } 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 if (amdgpu_device_supports_baco(dev)) {
> >> + switch (adev->asic_type) {
> >> + 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)
> >> - 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");
> >> + }
> >> }
> >>
> >> + 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
> >> */
> >> -
> >> acpi_status = amdgpu_acpi_init(adev);
> >> if (acpi_status)
> >> dev_dbg(dev->dev, "Error during ACPI methods call\n");
> >> --
> >> 2.34.1
> >>
