[PATCH 1/2] drm/amdgpu: skip SMU FW reloading in runpm BACO case (v2)

[Deucher, Alexander]

[AMD Official Use Only - General]

> -----Original Message-----
> From: Chen, Guchun <Guchun.Chen at amd.com>
> Sent: Wednesday, July 13, 2022 11:53 PM
> To: Alex Deucher <alexdeucher at gmail.com>
> Cc: amd-gfx list <amd-gfx at lists.freedesktop.org>; Deucher, Alexander
> <Alexander.Deucher at amd.com>; Zhang, Hawking
> <Hawking.Zhang at amd.com>; Lazar, Lijo <Lijo.Lazar at amd.com>; Quan, Evan
> <Evan.Quan at amd.com>; Feng, Kenneth <Kenneth.Feng at amd.com>
> Subject: RE: [PATCH 1/2] drm/amdgpu: skip SMU FW reloading in runpm
> BACO case (v2)
> 
> Sounds good, Alex. Let me update it soon.
> 
> Also, after a discussion with Lijo, once we introduce rpm mode, we think it's
> fine to drop adev->runpm indicator by rpm mode, as it's a bit overlapped with
> latter one.
> So for the check like adev->runpm, we can use 'rpm_mode !=
> AMDGPU_RUNPM_NONE' instead.
> If it makes sense, I will provide a follow up patch as well to optimize it.

Sure.  Sounds good.

Alex

> 
> Regards,
> Guchun
> 
> -----Original Message-----
> From: Alex Deucher <alexdeucher at gmail.com>
> Sent: Thursday, July 14, 2022 11:44 AM
> To: Chen, Guchun <Guchun.Chen at amd.com>
> Cc: amd-gfx list <amd-gfx at lists.freedesktop.org>; Deucher, Alexander
> <Alexander.Deucher at amd.com>; Zhang, Hawking
> <Hawking.Zhang at amd.com>; Lazar, Lijo <Lijo.Lazar at amd.com>; Quan, Evan
> <Evan.Quan at amd.com>; Feng, Kenneth <Kenneth.Feng at amd.com>
> Subject: Re: [PATCH 1/2] drm/amdgpu: skip SMU FW reloading in runpm
> BACO case (v2)
> 
> On Wed, Jul 13, 2022 at 11:15 PM Chen, Guchun <Guchun.Chen at amd.com>
> wrote:
> >
> > Re: I think this would be better as:
> > if (adev->in_runpm && (adev->pm.rpm_mode != AMDGPU_RUNPM_BOCO))
> or something like that.
> >
> > Yes, patch 2 in this series addresses it. Patch 1 intends to fix SMU reloading,
> while patch 2 focus on fixing race issue when getting feature mask during
> runtime cycle.
> 
> I get that, but I think it would be better to switch the order of the patches and
> then use the rpm_mode in this patch as well.  That way we are consistent and
> we don't miss some case if we change the BACO or BOCO logic in the future.
> 
> Alex
> 
> >
> > Regards,
> > Guchun
> >
> > -----Original Message-----
> > From: Alex Deucher <alexdeucher at gmail.com>
> > Sent: Wednesday, July 13, 2022 11:31 PM
> > To: Chen, Guchun <Guchun.Chen at amd.com>
> > Cc: amd-gfx list <amd-gfx at lists.freedesktop.org>; Deucher, Alexander
> > <Alexander.Deucher at amd.com>; Zhang, Hawking
> <Hawking.Zhang at amd.com>;
> > Lazar, Lijo <Lijo.Lazar at amd.com>; Quan, Evan <Evan.Quan at amd.com>;
> > Feng, Kenneth <Kenneth.Feng at amd.com>
> > Subject: Re: [PATCH 1/2] drm/amdgpu: skip SMU FW reloading in runpm
> > BACO case (v2)
> >
> > On Tue, Jul 12, 2022 at 11:18 PM Guchun Chen <guchun.chen at amd.com>
> wrote:
> > >
> > > SMU is always alive, so it's fine to skip SMU FW reloading when
> > > runpm resumed from BACO, this can avoid some race issues when
> > > resuming SMU FW.
> > >
> > > v2: Exclude boco case if an ASIC supports both boco and baco
> > >
> > > Suggested-by: Evan Quan <evan.quan at amd.com>
> > > Signed-off-by: Guchun Chen <guchun.chen at amd.com>
> > > ---
> > >  drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 8 ++++++++
> > >  1 file changed, 8 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> > > index e9411c28d88b..de59dc051340 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> > > @@ -2348,6 +2348,14 @@ static int psp_load_smu_fw(struct
> psp_context *psp)
> > >                         &adev->firmware.ucode[AMDGPU_UCODE_ID_SMC];
> > >         struct amdgpu_ras *ras = psp->ras_context.ras;
> > >
> > > +       /* Skip SMU FW reloading in case of using BACO for runpm only,
> > > +        * as SMU is always alive.
> > > +        */
> > > +       if (adev->in_runpm &&
> > > +           !amdgpu_device_supports_boco(adev_to_drm(adev)) &&
> > > +           amdgpu_device_supports_baco(adev_to_drm(adev)))
> >
> > I think this would be better as:
> > if (adev->in_runpm && (adev->pm.rpm_mode != AMDGPU_RUNPM_BOCO))
> or something like that.
> >
> > Alex
> >
> > > +               return 0;
> > > +
> > >         if (!ucode->fw || amdgpu_sriov_vf(psp->adev))
> > >                 return 0;
> > >
> > > --
> > > 2.17.1
> > >