| Message ID | 20180816180136.9040-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [i-g-t] lib: Poll for snd_hda_intel discovery | expand |
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@chris-wilson.co.uk> > Cc: Imre Deak <imre.deak@intel.com> > Cc: Tvrtko Ursulin <tvrtko.ursulin@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@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 >
Quoting Imre Deak (2018-08-17 10:14:51) > 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@chris-wilson.co.uk> > > Cc: Imre Deak <imre.deak@intel.com> > > Cc: Tvrtko Ursulin <tvrtko.ursulin@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. Good point. > Reviewed-by: Imre Deak <imre.deak@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) The results from the CI didn't look too promising, but I'm a bit dubious as to what exactly was tested and so pushed anyway ;) What next if it still doesn't discover an audio device? Hmm, is the /sys/class/sound construction async? I was assuming it was constructed by the parent snd module long before sna_hda_intel does it thing. -Chris
On Fri, Aug 17, 2018 at 10:24:30AM +0100, Chris Wilson wrote: > Quoting Imre Deak (2018-08-17 10:14:51) > > 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@chris-wilson.co.uk> > > > Cc: Imre Deak <imre.deak@intel.com> > > > Cc: Tvrtko Ursulin <tvrtko.ursulin@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. > > Good point. > > > Reviewed-by: Imre Deak <imre.deak@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) > > The results from the CI didn't look too promising, but I'm a bit dubious > as to what exactly was tested and so pushed anyway ;) > > What next if it still doesn't discover an audio device? We need to register the i915 audio component (in its current form or just a stub version) even with disable_display=1, so that the audio driver can continue its own probing (before timing out on the 10sec wait for the audio component). Atm the if (INTEL_INFO(dev_priv)->num_pipes == 0) return; in i915_audio_component_init() prevents the registration. > Hmm, is the /sys/class/sound construction async? I was assuming it was > constructed by the parent snd module long before sna_hda_intel does it > thing. AFAICS, azx_probe_continue() is the scheduled work and it will do snd_card_register()->device_add() which will add the sysfs entries. Adding Takashi to confirm. --Imre
Quoting Imre Deak (2018-08-17 10:38:01) > On Fri, Aug 17, 2018 at 10:24:30AM +0100, Chris Wilson wrote: > > Quoting Imre Deak (2018-08-17 10:14:51) > > > 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@chris-wilson.co.uk> > > > > Cc: Imre Deak <imre.deak@intel.com> > > > > Cc: Tvrtko Ursulin <tvrtko.ursulin@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. > > > > Good point. > > > > > Reviewed-by: Imre Deak <imre.deak@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) > > > > The results from the CI didn't look too promising, but I'm a bit dubious > > as to what exactly was tested and so pushed anyway ;) > > > > What next if it still doesn't discover an audio device? > > We need to register the i915 audio component (in its current form or > just a stub version) even with disable_display=1, so that the audio > driver can continue its own probing (before timing out on the 10sec > wait for the audio component). Atm the > > if (INTEL_INFO(dev_priv)->num_pipes == 0) > return; > > in i915_audio_component_init() prevents the registration. Right, that's the other question: do we need that at all... > > Hmm, is the /sys/class/sound construction async? I was assuming it was > > constructed by the parent snd module long before sna_hda_intel does it > > thing. > > AFAICS, azx_probe_continue() is the scheduled work and it will do > snd_card_register()->device_add() which will add the sysfs entries. As far as the probe we do here, the key question is whether opendir("/sys/class/sound") will succeed after modprobe but before the devices have been constructed. We using that as a predicate as to whether there are any sound modules to wait for. -Chris
On Fri, 17 Aug 2018 11:38:01 +0200, Imre Deak wrote: > > On Fri, Aug 17, 2018 at 10:24:30AM +0100, Chris Wilson wrote: > > Quoting Imre Deak (2018-08-17 10:14:51) > > > 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@chris-wilson.co.uk> > > > > Cc: Imre Deak <imre.deak@intel.com> > > > > Cc: Tvrtko Ursulin <tvrtko.ursulin@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. > > > > Good point. > > > > > Reviewed-by: Imre Deak <imre.deak@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) > > > > The results from the CI didn't look too promising, but I'm a bit dubious > > as to what exactly was tested and so pushed anyway ;) > > > > What next if it still doesn't discover an audio device? > > We need to register the i915 audio component (in its current form or > just a stub version) even with disable_display=1, so that the audio > driver can continue its own probing (before timing out on the 10sec > wait for the audio component). Atm the > > if (INTEL_INFO(dev_priv)->num_pipes == 0) > return; > > in i915_audio_component_init() prevents the registration. > > > Hmm, is the /sys/class/sound construction async? I was assuming it was > > constructed by the parent snd module long before sna_hda_intel does it > > thing. > > AFAICS, azx_probe_continue() is the scheduled work and it will do > snd_card_register()->device_add() which will add the sysfs entries. > > Adding Takashi to confirm. The root /sys/class/sound directory should have been visible at loading soundcore module, and this one is loaded before snd-hda-intel due to the dependency. Takashi
On Fri, Aug 17, 2018 at 10:48:29AM +0100, Chris Wilson wrote: > Quoting Imre Deak (2018-08-17 10:38:01) > > On Fri, Aug 17, 2018 at 10:24:30AM +0100, Chris Wilson wrote: > > > Quoting Imre Deak (2018-08-17 10:14:51) > > > > 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@chris-wilson.co.uk> > > > > > Cc: Imre Deak <imre.deak@intel.com> > > > > > Cc: Tvrtko Ursulin <tvrtko.ursulin@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. > > > > > > Good point. > > > > > > > Reviewed-by: Imre Deak <imre.deak@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) > > > > > > The results from the CI didn't look too promising, but I'm a bit dubious > > > as to what exactly was tested and so pushed anyway ;) > > > > > > What next if it still doesn't discover an audio device? > > > > We need to register the i915 audio component (in its current form or > > just a stub version) even with disable_display=1, so that the audio > > driver can continue its own probing (before timing out on the 10sec > > wait for the audio component). Atm the > > > > if (INTEL_INFO(dev_priv)->num_pipes == 0) > > return; > > > > in i915_audio_component_init() prevents the registration. > > Right, that's the other question: do we need that at all... > > > > Hmm, is the /sys/class/sound construction async? I was assuming it was > > > constructed by the parent snd module long before sna_hda_intel does it > > > thing. > > > > AFAICS, azx_probe_continue() is the scheduled work and it will do > > snd_card_register()->device_add() which will add the sysfs entries. > > As far as the probe we do here, the key question is whether > opendir("/sys/class/sound") will succeed after modprobe but before the > devices have been constructed. We using that as a predicate as to > whether there are any sound modules to wait for. Ok, /sys/class/sound appears immediately after modprobe, I meant /sys/class/sound/hwC* which appears only after component binding. In any case both dirs are polled for after your igt patch so that part is ok now imo. > -Chris
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; } 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); } /**
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@chris-wilson.co.uk> Cc: Imre Deak <imre.deak@intel.com> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> --- lib/igt_pm.c | 65 +++++++++++++++++++++++++++++++++------------------- 1 file changed, 41 insertions(+), 24 deletions(-)