[PATCH 0/3] ALSA: hda/ca0132 - Better handle a failed DSP firmware load.

[PATCH 0/3] ALSA: hda/ca0132 - Better handle a failed DSP firmware load.

[Dylan Reid]

Enabling CONFIG_SND_HDA_CODEC_CA0132_DSP on the Chromebook Pixel leads
to 10+ a second delay between running aplay and hearing audio.  This
was caused by the driver thinking the DSP was loaded when it had
actually failed.  The first two patches avoid that situation, the DSP
still fails to load every time, but at least the system continues
without a painful delay.  The third patch is just a cleanup that
touches the same state variable.

Dylan Reid (3):
  ALSA: hda/ca0132 - Check if dspload_image succeeded.
  ALSA: hda/ca0132 - Check download state of DSP.
  ALSA: hda/ca0132 - Remove extra setting of dsp_state.

 sound/pci/hda/patch_ca0132.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

-- 
1.8.1.3.605.g02339dd

[PATCH 1/3] ALSA: hda/ca0132 - Check if dspload_image succeeded.

[Dylan Reid]

If dspload_image() fails, it was ignored and dspload_wait_loaded() was
still called.  dsp_loaded should never be set to true in this case,
skip it.  The check in dspload_wait_loaded() return true if the DSP is
loaded or if it never started.

Signed-off-by: Dylan Reid <dgreid@chromium.org>
---
 sound/pci/hda/patch_ca0132.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/sound/pci/hda/patch_ca0132.c b/sound/pci/hda/patch_ca0132.c
index eefc456..cf24b75 100644
--- a/sound/pci/hda/patch_ca0132.c
+++ b/sound/pci/hda/patch_ca0132.c
@@ -4351,12 +4351,16 @@ static bool ca0132_download_dsp_images(struct hda_codec *codec)
 		return false;
 
 	dsp_os_image = (struct dsp_image_seg *)(fw_entry->data);
-	dspload_image(codec, dsp_os_image, 0, 0, true, 0);
+	if (dspload_image(codec, dsp_os_image, 0, 0, true, 0)) {
+		pr_err("ca0132 dspload_image failed.\n");
+		goto exit_download;
+	}
+
 	dsp_loaded = dspload_wait_loaded(codec);
 
+exit_download:
 	release_firmware(fw_entry);
 
-
 	return dsp_loaded;
 }
 
-- 
1.8.1.3.605.g02339dd

[PATCH 2/3] ALSA: hda/ca0132 - Check download state of DSP.

[Dylan Reid]

Instead of using the dspload_is_loaded() function, check the dsp_state
that is kept in the spec.  The dspload_is_loaded() function returns
true if the DSP transfer was never started.  This false-positive leads
to multiple second delays when ca0132_setup_efaults() times out on
each write.

Signed-off-by: Dylan Reid <dgreid@chromium.org>
---
 sound/pci/hda/patch_ca0132.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/sound/pci/hda/patch_ca0132.c b/sound/pci/hda/patch_ca0132.c
index cf24b75..225d1d5 100644
--- a/sound/pci/hda/patch_ca0132.c
+++ b/sound/pci/hda/patch_ca0132.c
@@ -3239,7 +3239,7 @@ static int ca0132_set_vipsource(struct hda_codec *codec, int val)
 	struct ca0132_spec *spec = codec->spec;
 	unsigned int tmp;
 
-	if (!dspload_is_loaded(codec))
+	if (spec->dsp_state != DSP_DOWNLOADED)
 		return 0;
 
 	/* if CrystalVoice if off, vipsource should be 0 */
@@ -4267,11 +4267,12 @@ static void ca0132_refresh_widget_caps(struct hda_codec *codec)
  */
 static void ca0132_setup_defaults(struct hda_codec *codec)
 {
+	struct ca0132_spec *spec = codec->spec;
 	unsigned int tmp;
 	int num_fx;
 	int idx, i;
 
-	if (!dspload_is_loaded(codec))
+	if (spec->dsp_state != DSP_DOWNLOADED)
 		return;
 
 	/* out, in effects + voicefx */
-- 
1.8.1.3.605.g02339dd

[PATCH 3/3] ALSA: hda/ca0132 - Remove extra setting of dsp_state.

[Dylan Reid]

spec->dsp_state is initialized to DSP_DOWNLOAD_INIT, no need to reset
and check it in ca0132_download_dsp().

Signed-off-by: Dylan Reid <dgreid@chromium.org>
---
 sound/pci/hda/patch_ca0132.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/sound/pci/hda/patch_ca0132.c b/sound/pci/hda/patch_ca0132.c
index 225d1d5..0792b57 100644
--- a/sound/pci/hda/patch_ca0132.c
+++ b/sound/pci/hda/patch_ca0132.c
@@ -4372,16 +4372,13 @@ static void ca0132_download_dsp(struct hda_codec *codec)
 #ifndef CONFIG_SND_HDA_CODEC_CA0132_DSP
 	return; /* NOP */
 #endif
-	spec->dsp_state = DSP_DOWNLOAD_INIT;
 
-	if (spec->dsp_state == DSP_DOWNLOAD_INIT) {
-		chipio_enable_clocks(codec);
-		spec->dsp_state = DSP_DOWNLOADING;
-		if (!ca0132_download_dsp_images(codec))
-			spec->dsp_state = DSP_DOWNLOAD_FAILED;
-		else
-			spec->dsp_state = DSP_DOWNLOADED;
-	}
+	chipio_enable_clocks(codec);
+	spec->dsp_state = DSP_DOWNLOADING;
+	if (!ca0132_download_dsp_images(codec))
+		spec->dsp_state = DSP_DOWNLOAD_FAILED;
+	else
+		spec->dsp_state = DSP_DOWNLOADED;
 
 	if (spec->dsp_state == DSP_DOWNLOADED)
 		ca0132_set_dsp_msr(codec, true);
-- 
1.8.1.3.605.g02339dd

Re: [PATCH 0/3] ALSA: hda/ca0132 - Better handle a failed DSP firmware load.

[Takashi Iwai]

At Thu, 14 Mar 2013 17:27:43 -0700,
Dylan Reid wrote:
> 
> Enabling CONFIG_SND_HDA_CODEC_CA0132_DSP on the Chromebook Pixel leads
> to 10+ a second delay between running aplay and hearing audio.  This
> was caused by the driver thinking the DSP was loaded when it had
> actually failed.  The first two patches avoid that situation, the DSP
> still fails to load every time, but at least the system continues
> without a painful delay.  The third patch is just a cleanup that
> touches the same state variable.
> 
> Dylan Reid (3):
>   ALSA: hda/ca0132 - Check if dspload_image succeeded.
>   ALSA: hda/ca0132 - Check download state of DSP.
>   ALSA: hda/ca0132 - Remove extra setting of dsp_state.

Applied all three patches.  Thanks!


Takashi

> 
>  sound/pci/hda/patch_ca0132.c | 26 +++++++++++++-------------
>  1 file changed, 13 insertions(+), 13 deletions(-)
> 
> -- 
> 1.8.1.3.605.g02339dd
>