[igt-dev] [PATCH i-g-t] lib: Poll for snd_hda_intel discovery

[Imre Deak]

On Thu, Aug 16, 2018 at 07:01:36PM +0100, Chris Wilson wrote:
> Loading the sounds modules is asynchronous with the sysfs device
> hierarchy being instantiated sometime after modprobe returns. As such
> while we are probing for the sound device, poll a few times to
> accommodate the async discovery.
> 
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Imre Deak <imre.deak at intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin at linux.intel.com>

> ---
>  lib/igt_pm.c | 65 +++++++++++++++++++++++++++++++++-------------------
>  1 file changed, 41 insertions(+), 24 deletions(-)
> 
> diff --git a/lib/igt_pm.c b/lib/igt_pm.c
> index 6e96db22b..f82707231 100644
> --- a/lib/igt_pm.c
> +++ b/lib/igt_pm.c
> @@ -138,31 +138,17 @@ static void strchomp(char *str)
>  		str[len - 1] = 0;
>  }
>  
> -/**
> - * igt_pm_enable_audio_runtime_pm:
> - *
> - * We know that if we don't enable audio runtime PM, snd_hda_intel will never
> - * release its power well refcount, and we'll never reach the LPSP state.
> - * There's no guarantee that it will release the power well if we enable
> - * runtime PM, but at least we can try.
> - *
> - * We don't have any assertions on open since the user may not even have
> - * snd_hda_intel loaded, which is not a problem.
> - */
> -void igt_pm_enable_audio_runtime_pm(void)
> +static int __igt_pm_enable_audio_runtime_pm(void)
>  {
>  	char *path = NULL;
>  	struct dirent *de;
>  	DIR *dir;
> +	int err;
>  	int fd;
>  
> -	/* Check if already enabled. */
> -	if (__igt_pm_audio_runtime_power_save[0])
> -		return;
> -
>  	dir = opendir("/sys/class/sound");
>  	if (!dir)
> -		return;
> +		return 0;
>  
>  	/* Find PCI device claimed by snd_hda_intel and tied to i915. */
>  	while ((de = readdir(dir))) {
> @@ -196,18 +182,17 @@ void igt_pm_enable_audio_runtime_pm(void)
>  				    de->d_name));
>  
>  		igt_debug("Audio device path is %s\n", path);
> -
>  		break;
>  	}

Could close dir while at it.

Reviewed-by: Imre Deak <imre.deak at intel.com>

This time the catch was that snd_hda_intel's second half of probe is run
from a scheduled work. (even though the first half returns synchronously
due to modprobe being synchronous as you found last time)

>  
>  	fd = open("/sys/module/snd_hda_intel/parameters/power_save", O_RDWR);
>  	if (fd < 0)
> -		return;
> +		return 0;
>  
>  	/* snd_hda_intel loaded but no path found is an error. */
>  	if (!path) {
>  		close(fd);
> -		errno = ESRCH;
> +		err = -ESRCH;
>  		goto err;
>  	}
>  
> @@ -219,8 +204,10 @@ void igt_pm_enable_audio_runtime_pm(void)
>  	close(fd);
>  
>  	fd = open(path, O_RDWR);
> -	if (fd < 0)
> +	if (fd < 0) {
> +		err = -errno;
>  		goto err;
> +	}
>  
>  	igt_assert(read(fd, __igt_pm_audio_runtime_control,
>  			sizeof(__igt_pm_audio_runtime_control)) > 0);
> @@ -236,12 +223,42 @@ void igt_pm_enable_audio_runtime_pm(void)
>  
>  	/* Give some time for it to react. */
>  	sleep(1);
> -
> -	return;
> +	return 0;
>  
>  err:
> -	igt_warn("Failed to enable audio runtime PM! (%d)", errno);
>  	free(path);
> +	return err;
> +}
> +
> +/**
> + * igt_pm_enable_audio_runtime_pm:
> + *
> + * We know that if we don't enable audio runtime PM, snd_hda_intel will never
> + * release its power well refcount, and we'll never reach the LPSP state.
> + * There's no guarantee that it will release the power well if we enable
> + * runtime PM, but at least we can try.
> + *
> + * We don't have any assertions on open since the user may not even have
> + * snd_hda_intel loaded, which is not a problem.
> + */
> +void igt_pm_enable_audio_runtime_pm(void)
> +{
> +	int err;
> +
> +	/* Check if already enabled. */
> +	if (__igt_pm_audio_runtime_power_save[0])
> +		return;
> +
> +	for (int count = 0; count < 5; count++) {
> +		err = __igt_pm_enable_audio_runtime_pm();
> +		if (!err)
> +			return;
> +
> +		/* modprobe(sna-hda-intel) is async so poll for sysfs */
> +		sleep(1);
> +	}
> +
> +	igt_warn("Failed to enable audio runtime PM! (%d)\n", -err);
>  }
>  
>  /**
> -- 
> 2.18.0
>