At Tue, 09 Oct 2012 16:28:07 +0200, David Henningsson wrote:
On 10/09/2012 03:32 PM, Takashi Iwai wrote:
At Tue, 9 Oct 2012 15:19:44 +0200, David Henningsson wrote:
Now that we have a generic unsol mechanism, we can implement a generic poll loop, which can be used for debugging, or if a codec's unsol mechanism is broken.
Oh, and I was also considering not turning on unsol at all (in snd_hda_jack_detect_enable) when this is enabled. Then it would help against "unstable" pins which trigger repeatedly on/off even though nothing is happening physically.
In that case, it's fine. But it's no broken codec's unsol mechanism. It's a broken jack detection in hardware, i.e. it's no fault of codec.
Signed-off-by: David Henningsson david.henningsson@canonical.com
The patch almost looks good, but I postpone as a 3.8 thing.
Ok. I never understood the release timings, but I figured anything before rc1 would be okay to merge as a feature...
No, basically when the merge window opens, all new features should be frozen. I took a few your patches yesterday just because I've been traveling in the last weeks, and the generic unsol event code itself is mostly nothing but a code shuffling from here to there.
(And I couldn't start a week earlier; because I didn't know if you would accept the generic unsol event.) That said, it's not very important to me personally which kernel this goes into.
OK.
I'm also considering activating it by default if we go into single_cmd mode due to codec not responding. What do you think?
Well, I don't buy it. Falling back to single_cmd doesn't mean that we are saving the world perfectly.I recently even think it might be even better not to do that, otherwise people won't notice the breakage.
This is almost philosophical, but if a bug occurs and no user notices, is it really a bug? :-)
Yes, it performs worse secretly. The fallback to single_cmd is really a last resort and it's no peaceful place to live at all.
Or, to make it a bit more pragmatic, what other things are broken with single_cmd? Could you give an example where this change would be harmful?
The single cmd mode itself was introduced as a sort of rescue command without CORB/RIRB. We shouldn't use it in normal situations. It's simply no considered use case.
With a lack of unsol event, you can't handle the volume knob, or any GPIO events, too.
Takashi
codec->jackpoll_interval =msecs_to_jiffies(jackpoll_ms[chip->dev_index]);Better to add a sanity check of the interval value.
Ok. Attaching modified patch.
Well, do you want to allow 1ms polling?
Takashi
-- David Henningsson, Canonical Ltd. https://launchpad.net/~diwic [2 0001-ALSA-hda-Implement-a-poll-loop-for-jacks-as-a-module.patch <text/x-patch (7bit)>]
From 67a8012f1e8eef1b67119f8d6699ddf5b88d9fa0 Mon Sep 17 00:00:00 2001
From: David Henningsson david.henningsson@canonical.com Date: Tue, 9 Oct 2012 15:04:21 +0200 Subject: [PATCH] ALSA: hda - Implement a poll loop for jacks as a module parameter
Now that we have a generic unsol mechanism, we can implement a generic poll loop, which can be used for debugging, or if a codec's unsol mechanism is broken.
Signed-off-by: David Henningsson david.henningsson@canonical.com
sound/pci/hda/hda_codec.c | 32 ++++++++++++++++++++++++++++---- sound/pci/hda/hda_codec.h | 2 ++ sound/pci/hda/hda_intel.c | 11 +++++++++++ sound/pci/hda/hda_jack.c | 22 ++++++++++++++++++++++ sound/pci/hda/hda_jack.h | 2 ++ 5 files changed, 65 insertions(+), 4 deletions(-)
diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c index c0ab72c..626e45b 100644 --- a/sound/pci/hda/hda_codec.c +++ b/sound/pci/hda/hda_codec.c @@ -1135,6 +1135,19 @@ static void restore_shutup_pins(struct hda_codec *codec) } #endif
+static void hda_jackpoll_work(struct work_struct *work) +{
- struct hda_codec *codec =
container_of(work, struct hda_codec, jackpoll_work.work);- if (!codec->jackpoll_interval)
return;- snd_hda_jack_set_dirty_all(codec);
- snd_hda_jack_poll_all(codec);
- queue_delayed_work(codec->bus->workq, &codec->jackpoll_work,
codec->jackpoll_interval);+}
static void init_hda_cache(struct hda_cache_rec *cache, unsigned int record_size); static void free_hda_cache(struct hda_cache_rec *cache); @@ -1190,6 +1203,7 @@ static void snd_hda_codec_free(struct hda_codec *codec) { if (!codec) return;
- cancel_delayed_work_sync(&codec->jackpoll_work); snd_hda_jack_tbl_clear(codec); restore_init_pincfgs(codec);
#ifdef CONFIG_PM @@ -1273,6 +1287,7 @@ int /*__devinit*/ snd_hda_codec_new(struct hda_bus *bus, snd_array_init(&codec->cvt_setups, sizeof(struct hda_cvt_setup), 8); snd_array_init(&codec->conn_lists, sizeof(hda_nid_t), 64); snd_array_init(&codec->spdif_out, sizeof(struct hda_spdif_out), 16);
- INIT_DELAYED_WORK(&codec->jackpoll_work, hda_jackpoll_work);
#ifdef CONFIG_PM spin_lock_init(&codec->power_lock); @@ -2349,7 +2364,7 @@ int snd_hda_codec_reset(struct hda_codec *codec) return -EBUSY;
/* OK, let it free */
- cancel_delayed_work_sync(&codec->jackpoll_work);
#ifdef CONFIG_PM cancel_delayed_work_sync(&codec->power_work); codec->power_on = 0; @@ -3646,7 +3661,6 @@ static void hda_call_codec_resume(struct hda_codec *codec) restore_pincfgs(codec); /* restore all current pin configs */ restore_shutup_pins(codec); hda_exec_init_verbs(codec);
- snd_hda_jack_set_dirty_all(codec); if (codec->patch_ops.resume) codec->patch_ops.resume(codec); else {
@@ -3655,7 +3669,13 @@ static void hda_call_codec_resume(struct hda_codec *codec) snd_hda_codec_resume_amp(codec); snd_hda_codec_resume_cache(codec); }
- snd_hda_jack_report_sync(codec);
- if (codec->jackpoll_interval)
hda_jackpoll_work(&codec->jackpoll_work.work);- else {
snd_hda_jack_set_dirty_all(codec);snd_hda_jack_report_sync(codec);- } snd_hda_power_down(codec); /* flag down before returning */
} #endif /* CONFIG_PM */ @@ -3737,7 +3757,10 @@ int snd_hda_codec_build_controls(struct hda_codec *codec) if (err < 0) return err;
- snd_hda_jack_report_sync(codec); /* call at the last init point */
- if (codec->jackpoll_interval)
hda_jackpoll_work(&codec->jackpoll_work.work);- else
snd_hda_jack_report_sync(codec); /* call at the last init point */}
@@ -5128,6 +5151,7 @@ int snd_hda_suspend(struct hda_bus *bus) struct hda_codec *codec;
list_for_each_entry(codec, &bus->codec_list, list) {
cancel_delayed_work_sync(&codec->jackpoll_work);diff --git a/sound/pci/hda/hda_codec.h b/sound/pci/hda/hda_codec.h index 507fe8a..10a03b0 100644 --- a/sound/pci/hda/hda_codec.h +++ b/sound/pci/hda/hda_codec.h @@ -884,6 +884,8 @@ struct hda_codec {
/* jack detection */ struct snd_array jacktbl;
- unsigned long jackpoll_interval; /* In jiffies. Zero means no poll, rely on unsol events */
- struct delayed_work jackpoll_work;
#ifdef CONFIG_SND_HDA_INPUT_JACK /* jack detection */ diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c index f09ff6c..5f0f276 100644 --- a/sound/pci/hda/hda_intel.c +++ b/sound/pci/hda/hda_intel.c @@ -68,6 +68,7 @@ static int position_fix[SNDRV_CARDS] = {[0 ... (SNDRV_CARDS-1)] = -1}; static int bdl_pos_adj[SNDRV_CARDS] = {[0 ... (SNDRV_CARDS-1)] = -1}; static int probe_mask[SNDRV_CARDS] = {[0 ... (SNDRV_CARDS-1)] = -1}; static int probe_only[SNDRV_CARDS]; +static int jackpoll_ms[SNDRV_CARDS]; static bool single_cmd; static int enable_msi = -1; #ifdef CONFIG_SND_HDA_PATCH_LOADER @@ -95,6 +96,8 @@ module_param_array(probe_mask, int, NULL, 0444); MODULE_PARM_DESC(probe_mask, "Bitmask to probe codecs (default = -1)."); module_param_array(probe_only, int, NULL, 0444); MODULE_PARM_DESC(probe_only, "Only probing and no codec initialization."); +module_param_array(jackpoll_ms, int, NULL, 0444); +MODULE_PARM_DESC(jackpoll_ms, "Ms between polling for jack events (default = 0, using unsol events only)"); module_param(single_cmd, bool, 0444); MODULE_PARM_DESC(single_cmd, "Use single command to communicate with codecs " "(for debugging only)."); @@ -1597,6 +1600,7 @@ static int DELAYED_INIT_MARK azx_codec_create(struct azx *chip, const char *mode struct hda_bus_template bus_temp; int c, codecs, err; int max_slots;
unsigned int jackpoll_interval = 0;
memset(&bus_temp, 0, sizeof(bus_temp)); bus_temp.private_data = chip;
@@ -1625,6 +1629,12 @@ static int DELAYED_INIT_MARK azx_codec_create(struct azx *chip, const char *mode if (!max_slots) max_slots = AZX_DEFAULT_CODECS;
- if (jackpoll_ms[chip->dev_index] < 0 || jackpoll_ms[chip->dev_index] > 60000)
snd_printk(KERN_WARNING SFX "jackpoll_ms value out of range: %d\n",jackpoll_ms[chip->dev_index]);- else if (jackpoll_ms[chip->dev_index] > 0)
jackpoll_interval = msecs_to_jiffies(jackpoll_ms[chip->dev_index]);- /* First try to probe all given codec slots */ for (c = 0; c < max_slots; c++) { if ((chip->codec_mask & (1 << c)) & chip->codec_probe_mask) {
@@ -1666,6 +1676,7 @@ static int DELAYED_INIT_MARK azx_codec_create(struct azx *chip, const char *mode err = snd_hda_codec_new(chip->bus, c, &codec); if (err < 0) continue;
codec->jackpoll_interval = jackpoll_interval; codec->beep_mode = chip->beep_mode; codecs++;diff --git a/sound/pci/hda/hda_jack.c b/sound/pci/hda/hda_jack.c index 5c690cb..07d5288 100644 --- a/sound/pci/hda/hda_jack.c +++ b/sound/pci/hda/hda_jack.c @@ -439,3 +439,25 @@ void snd_hda_jack_unsol_event(struct hda_codec *codec, unsigned int res) } EXPORT_SYMBOL_HDA(snd_hda_jack_unsol_event);
+void snd_hda_jack_poll_all(struct hda_codec *codec) +{
- struct hda_jack_tbl *jack = codec->jacktbl.list;
- int i, changes = 0;
- for (i = 0; i < codec->jacktbl.used; i++, jack++) {
unsigned int old_sense;if (!jack->nid || !jack->jack_dirty || jack->phantom_jack)continue;old_sense = get_jack_plug_state(jack->pin_sense);jack_detect_update(codec, jack);if (old_sense == get_jack_plug_state(jack->pin_sense))continue;changes = 1;if (jack->callback)jack->callback(codec, jack);- }
- if (changes)
snd_hda_jack_report_sync(codec);+} +EXPORT_SYMBOL_HDA(snd_hda_jack_poll_all);
diff --git a/sound/pci/hda/hda_jack.h b/sound/pci/hda/hda_jack.h index af8dd47..4487785 100644 --- a/sound/pci/hda/hda_jack.h +++ b/sound/pci/hda/hda_jack.h @@ -84,4 +84,6 @@ void snd_hda_jack_report_sync(struct hda_codec *codec);
void snd_hda_jack_unsol_event(struct hda_codec *codec, unsigned int res);
+void snd_hda_jack_poll_all(struct hda_codec *codec);
#endif /* __SOUND_HDA_JACK_H */
1.7.9.5