[PATCH] drm/amd/pm: fix runpm hang when amdgpu loaded prior to sound driver

[Lazar, Lijo]

On 9/10/2021 8:47 AM, Evan Quan wrote:
> Current RUNPM mechanism relies on PMFW to master the timing for BACO
> in/exit. And that needs cooperation from sound driver for dstate
> change notification for function 1(audio). Otherwise(on sound driver
> missing), BACO cannot be kicked in correctly and hang will be observed
> on RUNPM exit.
> 
> By switching back to legacy message way on sound driver missing,
> we are able to fix the runpm hang observed for the scenario below:
> amdgpu driver loaded -> runpm suspend kicked -> sound driver loaded
> 
> Change-Id: I0e44fef11349b5e45e6102913eb46c8c7d279c65
> Signed-off-by: Evan Quan <evan.quan at amd.com>
> Reported-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer at amd.com>

Reviewed-by: Lijo Lazar <lijo.lazar at amd.com>

Thanks,
Lijo

> ---
>   .../gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c   | 24 +++++++++++++++++--
>   .../amd/pm/swsmu/smu11/sienna_cichlid_ppt.c   |  4 ++--
>   drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c        | 21 ++++++++++++++++
>   drivers/gpu/drm/amd/pm/swsmu/smu_cmn.h        |  2 ++
>   4 files changed, 47 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
> index 7bc90f841a11..bcafccf7f07a 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
> @@ -2272,7 +2272,27 @@ static int navi10_baco_enter(struct smu_context *smu)
>   {
>   	struct amdgpu_device *adev = smu->adev;
>   
> -	if (adev->in_runpm)
> +	/*
> +	 * This aims the case below:
> +	 *   amdgpu driver loaded -> runpm suspend kicked -> sound driver loaded
> +	 *
> +	 * For NAVI10 and later ASICs, we rely on PMFW to handle the runpm. To
> +	 * make that possible, PMFW needs to acknowledge the dstate transition
> +	 * process for both gfx(function 0) and audio(function 1) function of
> +	 * the ASIC.
> +	 *
> +	 * The PCI device's initial runpm status is RUNPM_SUSPENDED. So as the
> +	 * device representing the audio function of the ASIC. And that means
> +	 * even if the sound driver(snd_hda_intel) was not loaded yet, it's still
> +	 * possible runpm suspend kicked on the ASIC. However without the dstate
> +	 * transition notification from audio function, pmfw cannot handle the
> +	 * BACO in/exit correctly. And that will cause driver hang on runpm
> +	 * resuming.
> +	 *
> +	 * To address this, we revert to legacy message way(driver masters the
> +	 * timing for BACO in/exit) on sound driver missing.
> +	 */
> +	if (adev->in_runpm && smu_cmn_is_audio_func_enabled(adev))
>   		return smu_v11_0_baco_set_armd3_sequence(smu, BACO_SEQ_BACO);
>   	else
>   		return smu_v11_0_baco_enter(smu);
> @@ -2282,7 +2302,7 @@ static int navi10_baco_exit(struct smu_context *smu)
>   {
>   	struct amdgpu_device *adev = smu->adev;
>   
> -	if (adev->in_runpm) {
> +	if (adev->in_runpm && smu_cmn_is_audio_func_enabled(adev)) {
>   		/* Wait for PMFW handling for the Dstate change */
>   		msleep(10);
>   		return smu_v11_0_baco_set_armd3_sequence(smu, BACO_SEQ_ULPS);
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
> index 43c7580a4ea6..f9b730c5ba9e 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
> @@ -2361,7 +2361,7 @@ static int sienna_cichlid_baco_enter(struct smu_context *smu)
>   {
>   	struct amdgpu_device *adev = smu->adev;
>   
> -	if (adev->in_runpm)
> +	if (adev->in_runpm && smu_cmn_is_audio_func_enabled(adev))
>   		return smu_v11_0_baco_set_armd3_sequence(smu, BACO_SEQ_BACO);
>   	else
>   		return smu_v11_0_baco_enter(smu);
> @@ -2371,7 +2371,7 @@ static int sienna_cichlid_baco_exit(struct smu_context *smu)
>   {
>   	struct amdgpu_device *adev = smu->adev;
>   
> -	if (adev->in_runpm) {
> +	if (adev->in_runpm && smu_cmn_is_audio_func_enabled(adev)) {
>   		/* Wait for PMFW handling for the Dstate change */
>   		msleep(10);
>   		return smu_v11_0_baco_set_armd3_sequence(smu, BACO_SEQ_ULPS);
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
> index 69da9a7b665f..d61403e917df 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
> @@ -1055,3 +1055,24 @@ int smu_cmn_set_mp1_state(struct smu_context *smu,
>   
>   	return ret;
>   }
> +
> +bool smu_cmn_is_audio_func_enabled(struct amdgpu_device *adev)
> +{
> +	struct pci_dev *p = NULL;
> +	bool snd_driver_loaded;
> +
> +	/*
> +	 * If the ASIC comes with no audio function, we always assume
> +	 * it is "enabled".
> +	 */
> +	p = pci_get_domain_bus_and_slot(pci_domain_nr(adev->pdev->bus),
> +			adev->pdev->bus->number, 1);
> +	if (!p)
> +		return true;
> +
> +	snd_driver_loaded = pci_is_enabled(p) ? true : false;
> +
> +	pci_dev_put(p);
> +
> +	return snd_driver_loaded;
> +}
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.h b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.h
> index 16993daa2ae0..b1d41360a389 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.h
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.h
> @@ -110,5 +110,7 @@ void smu_cmn_init_soft_gpu_metrics(void *table, uint8_t frev, uint8_t crev);
>   int smu_cmn_set_mp1_state(struct smu_context *smu,
>   			  enum pp_mp1_state mp1_state);
>   
> +bool smu_cmn_is_audio_func_enabled(struct amdgpu_device *adev);
> +
>   #endif
>   #endif
>