[alsa-devel] [PATCH 0/3] ALSA: hda/ca0132 - Better handle a failed DSP firmware load.
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(-)
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; }
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 */
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);
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
participants (2)
-
Dylan Reid -
Takashi Iwai