[Public]
> -----Original Message-----
> From: Lazar, Lijo <Lijo.Lazar@xxxxxxx>
> Sent: Monday, June 7, 2021 4:19 PM
> To: Quan, Evan <Evan.Quan@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx
> Cc: Deucher, Alexander <Alexander.Deucher@xxxxxxx>
> Subject: RE: [PATCH V2 3/5] drm/amdgpu: correct the audio function initial
> Dstate
>
> [Public]
>
> Thanks, that explains.
>
> You may modify the comment to something like " amdgpu_get_audio_func()
> makes a PMFW-aware D-state transition to update audio dev's D-state in
> PMFW" (now it gives the impression that function makes audio dev to stay in
> D0 state).
[Quan, Evan] Sure.
Thanks,
Evan
>
> > > + * Via amdgpu_get_audio_func() below, the audio function
> > > + * is guarded to be in D0 correctly.
>
> Thanks,
> Lijo
>
> -----Original Message-----
> From: Quan, Evan <Evan.Quan@xxxxxxx>
> Sent: Monday, June 7, 2021 12:40 PM
> To: Lazar, Lijo <Lijo.Lazar@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx
> Cc: Deucher, Alexander <Alexander.Deucher@xxxxxxx>
> Subject: RE: [PATCH V2 3/5] drm/amdgpu: correct the audio function initial
> Dstate
>
> [AMD Official Use Only]
>
>
>
> > -----Original Message-----
> > From: Lazar, Lijo <Lijo.Lazar@xxxxxxx>
> > Sent: Friday, June 4, 2021 8:24 PM
> > To: Quan, Evan <Evan.Quan@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx
> > Cc: Deucher, Alexander <Alexander.Deucher@xxxxxxx>
> > Subject: Re: [PATCH V2 3/5] drm/amdgpu: correct the audio function
> > initial Dstate
> >
> >
> >
> > On 6/4/2021 3:28 PM, Evan Quan wrote:
> > > On driver loading, the ASIC is in D0 state. The bundled audio
> > > function should be in the same state also.
> > >
> > > Change-Id: I136e196be7633e95883a7f6c33963f7583e9bad1
> > > Signed-off-by: Evan Quan <evan.quan@xxxxxxx>
> > > ---
> > > V1->V2:
> > > - Lijo: include the code in a seperate API for better readability
> > > - limit the change for Navi10 and later dGPUs
> > > - comments more about the background
> > > ---
> > > drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 40
> > +++++++++++++++++++++++++
> > > 1 file changed, 40 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> > > index c354ffa62483..e9f2161738d1 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> > > @@ -123,6 +123,22 @@ void amdgpu_register_gpu_instance(struct
> > amdgpu_device *adev)
> > > mutex_unlock(&mgpu_info.mutex);
> > > }
> > >
> > > +static void amdgpu_get_audio_func(struct amdgpu_device *adev) {
> > > + struct pci_dev *p = NULL;
> > > +
> > > + p = pci_get_domain_bus_and_slot(pci_domain_nr(adev->pdev-
> > >bus),
> > > + adev->pdev->bus->number, 1);
> > > + if (p) {
> > > + pm_runtime_get_sync(&p->dev);
> > > +
> > > + pm_runtime_mark_last_busy(&p->dev);
> > > + pm_runtime_put_autosuspend(&p->dev);
> > > +
> > > + pci_dev_put(p);
> > > + }
> > > +}
> > > +
> > > /**
> > > * amdgpu_driver_load_kms - Main load function for KMS.
> > > *
> > > @@ -212,9 +228,33 @@ int amdgpu_driver_load_kms(struct
> > amdgpu_device *adev, unsigned long flags)
> > >
> > DPM_FLAG_MAY_SKIP_RESUME);
> > > pm_runtime_use_autosuspend(dev->dev);
> > > pm_runtime_set_autosuspend_delay(dev->dev, 5000);
> > > +
> > > pm_runtime_allow(dev->dev);
> > > +
> > > pm_runtime_mark_last_busy(dev->dev);
> > > pm_runtime_put_autosuspend(dev->dev);
> > > +
> > > + /*
> > > + * For runpm implemented via BACO, PMFW will handle the
> > > + * timing for BACO in and out:
> > > + * - put ASIC into BACO state only when both video and
> > > + * audio functions are in D3 state.
> > > + * - pull ASIC out of BACO state when either video or
> > > + * audio function is in D0 state.
> > > + * Also, at startup, PMFW assumes both functions are in
> > > + * D0 state.
> > > + *
> > > + * So if snd driver was loaded prior to amdgpu driver
> > > + * and audio function was put into D3 state behind PMFW's
> > > + * back, the BACO may not correctly kicks in and out.
> > > + *
> > > + * Via amdgpu_get_audio_func() below, the audio function
> > > + * is guarded to be in D0 correctly.
> > > + */
> >
> > On a second look at the comments - should runpm _get on audio dev be
> > done before doing device_init (so that it's in D0 while FW is getting
> > loaded) and put done here?
> [Quan, Evan] It does not matter as long as all those are performed
> before .runtime_suspend() kicks.
> The issue around here is : if the audio dev is already in D3 state before PMFW
> alive, during .runtime_suspend there will be no Dstate transfer(D0->D3).
> Thus no interrupt will go to PMFW and PMFW will be not aware of the audio
> dev D3 state. With the proper audio dev D3 state, the baco state will never
> kick in.
> Via the amdgpu_get_audio_func(), the audio dev is put into D0 state. Then
> on the succeeding .runtime_suspend(), there will be Dstate transfer(D0->D3)
> and thus interrupt for PMFW.
>
> BR
> Evan
> >
> > Thanks,
> > Lijo
> >
> > > + if (amdgpu_device_supports_baco(dev) &&
> > > + !(adev->flags & AMD_IS_APU) &&
> > > + (adev->asic_type >= CHIP_NAVI10))
> > > + amdgpu_get_audio_func(adev);
> > > }
> > >
> > > if (amdgpu_acpi_smart_shift_update(dev,
> > AMDGPU_SS_DRV_LOAD))
> > >
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
