[alsa-devel] HD-Audio: How to reduce driver initializaton time if multiple codecs present on the bus?

[Takashi Iwai]

At Mon, 2 Dec 2013 07:23:11 +0000,
Lin, Mengdong wrote:
> 
> > -----Original Message-----
> > From: Takashi Iwai [mailto:tiwai at suse.de]
> > Sent: Friday, November 29, 2013 4:08 PM
> > To: Lin, Mengdong
> > Cc: David Henningsson; alsa-devel at alsa-project.org
> > Subject: Re: [alsa-devel] HD-Audio: How to reduce driver initializaton time
> > if multiple codecs present on the bus?
> > 
> > At Fri, 29 Nov 2013 08:05:35 +0000,
> > Lin, Mengdong wrote:
> > >
> > > > -----Original Message-----
> > > > From: Takashi Iwai [mailto:tiwai at suse.de]
> > > > Sent: Friday, November 29, 2013 3:50 PM
> > > > To: David Henningsson
> > > > Cc: Lin, Mengdong; alsa-devel at alsa-project.org
> > > > Subject: Re: [alsa-devel] HD-Audio: How to reduce driver
> > > > initializaton time if multiple codecs present on the bus?
> > > >
> > > > At Fri, 29 Nov 2013 15:31:59 +0800,
> > > > David Henningsson wrote:
> > > > >
> > > > > 2013-11-29 14:40, Takashi Iwai skrev:
> > > > > > At Fri, 29 Nov 2013 14:30:48 +0800, David Henningsson wrote:
> > > > > >> 2013-11-28 22:57, Lin, Mengdong skrev:
> > > > > >>> Hi Takashi,
> > > > > >>>
> > > > > >>> We're trying to reduce the HD-A driver initialization time
> > > > > >>> when more
> > > > than one codecs are connected to the bus, but are blocked.
> > > > > >>> Would you please share some advices on this?
> > > > > >>>
> > > > > >>> Usually, there is one HD-A controller connecting to two
> > > > > >>> codecs: one
> > > > on-board codec and one integrated display codec.
> > > > > >>> During initialization, the codecs are created and configured
> > > > > >>> in a
> > > > serial way.
> > > > > >>>
> > > > > >>> Creating a codec may cost 6~20ms, and then building controls
> > > > > >>> make
> > > > cost about 15~30ms.
> > > > > >> Sorry for interrupting, but I just wonder - I assume you have a
> > > > > >> maximum of 4 CPUs. Can't the other 3 CPUs be used to load other
> > > > > >> non-audio hardware in parallel instead? It sounds you're going
> > > > > >> to run into lock contention instead if you try to modify the
> > > > > >> same card from two threads simultaneously.
> > > > > > IIRC, PCI device probes are done sequentially because
> > > > > > asynchronous probe caused too many troubles.  And if it's a
> > > > > > module, it's anyway more strictly serialized.
> > > > >
> > > > > Okay. It just seems to me that from a bird's eye view that
> > > > > parallelizing module loading and pci probing would give us much
> > > > > bigger benefits than trying to parallellize internally in the
> > snd-hda-intel driver.
> > > >
> > > > Yeah, for a long run, this would be far benefit.  But be warned,
> > > > several attempts in the past failed due to weird hardwares :)
> > > >
> > > > > Btw, using a deferred azx_probe_continue (like we do if there's a
> > > > > firmware file), would one get more parallelization with other drivers?
> > > > > If so we could consider making that the default.
> > > >
> > > > This sounds like a good idea as we have already that code.  The
> > > > patch would be just a few-liners.
> > >
> > > If azx_probe_continue() is delayed, could user space get incomplete
> > audio information after boot?
> > > I mean is it possible that azx_probe_continue() does not complete
> > before user space check ALSA info?
> > 
> > Not impossible.  But then user-space should be triggered in event driven
> > way by udev or such.
> 
> Hi Takashi/David,
> 
> So is it okay to defer azx_probe_continue() by default?

Yes.

> If yes, is it also necessary to defer snd_card_create() at the end of azx_probe_continue()? 

No, that's a significant difference.  The snd_card_create() invocation
isn't related with the hardware access itself, thus it's no point to
delay.  If you delay the card instance creation, this may result in
races between the card order, which we'd like to avoid.

> So user space can only see the sound card when azx_probe_continue() completes.

The completion of the card creation is notified only after
snd_card_register(), so it doesn't matter when to call
snd_card_create().

The below is an untested patch.  I should have added in the previous
reply...


Takashi

---
diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index dfdb96603636..3deec907bf1a 100644
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -3805,7 +3805,7 @@ static int azx_probe(struct pci_dev *pci,
 	static int dev;
 	struct snd_card *card;
 	struct azx *chip;
-	bool probe_now;
+	bool schedule_probe;
 	int err;
 
 	if (dev >= SNDRV_CARDS)
@@ -3844,7 +3844,7 @@ static int azx_probe(struct pci_dev *pci,
 		chip->disabled = true;
 	}
 
-	probe_now = !chip->disabled;
+	schedule_probe = !chip->disabled;
 
 #ifdef CONFIG_SND_HDA_PATCH_LOADER
 	if (patch[dev] && *patch[dev]) {
@@ -3855,25 +3855,17 @@ static int azx_probe(struct pci_dev *pci,
 					      azx_firmware_cb);
 		if (err < 0)
 			goto out_free;
-		probe_now = false; /* continued in azx_firmware_cb() */
+		schedule_probe = false; /* continued in azx_firmware_cb() */
 	}
 #endif /* CONFIG_SND_HDA_PATCH_LOADER */
 
-	/* continue probing in work context, avoid request_module deadlock */
-	if (probe_now && (chip->driver_caps & AZX_DCAPS_I915_POWERWELL)) {
-#ifdef CONFIG_SND_HDA_I915
-		probe_now = false;
-		schedule_work(&chip->probe_work);
-#else
+#ifndef CONFIG_SND_HDA_I915
+	if (chip->driver_caps & AZX_DCAPS_I915_POWERWELL)
 		snd_printk(KERN_ERR SFX "Haswell must build in CONFIG_SND_HDA_I915\n");
 #endif
-	}
 
-	if (probe_now) {
-		err = azx_probe_continue(chip);
-		if (err < 0)
-			goto out_free;
-	}
+	if (schedule_probe)
+		schedule_work(&chip->probe_work);
 
 	dev++;
 	complete_all(&chip->probe_wait);