[alsa-devel] [PATCH 0/4] ALSA: hda/hdmi: Clean up jack handling
Hi,
this is a series of cleanup of the jack handling in HD-audio HDMI codec driver, which I realized after the recent regression fix. Basically this changes the jack handling from hda_jack to the own jack objects as Intel HDMI did but applying now to all (generic) codecs. It resulted in a good amount of code reduction in the end.
As this is no real fixes, and I'd like to rather share the code between 5.5 and 5.6, it's targeted for 5.7.
Only lightly tested on Intel and nouveau.
Takashi
===
Takashi Iwai (4): ALSA: hda/hdmi: Reduce hda_jack_tbl lookup at unsol event handling ALSA: hda/hdmi: Don't use standard hda_jack for generic HDMI jacks ALSA: hda/hdmi: Move runtime PM resume into hdmi_present_sense_via_verbs() ALSA: hda/hdmi: Move ELD parse and jack reporting into update_eld()
sound/pci/hda/patch_hdmi.c | 313 +++++++++++++-------------------------------- 1 file changed, 90 insertions(+), 223 deletions(-)
Pass hda_jack_tbl object to hdmi_intrinsic_event() along with res from hdmi_unsol_event() so that we can reduce the lookup of the same hda_jack_tbl object again.
Minor code refactoring.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/pci/hda/patch_hdmi.c | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-)
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 5119a9ae3d8a..c65d16dea08c 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -779,21 +779,9 @@ static void jack_callback(struct hda_codec *codec, check_presence_and_report(codec, jack->nid, jack->dev_id); }
-static void hdmi_intrinsic_event(struct hda_codec *codec, unsigned int res) +static void hdmi_intrinsic_event(struct hda_codec *codec, unsigned int res, + struct hda_jack_tbl *jack) { - int tag = res >> AC_UNSOL_RES_TAG_SHIFT; - struct hda_jack_tbl *jack; - - if (codec->dp_mst) { - int dev_entry = - (res & AC_UNSOL_RES_DE) >> AC_UNSOL_RES_DE_SHIFT; - - jack = snd_hda_jack_tbl_get_from_tag(codec, tag, dev_entry); - } else { - jack = snd_hda_jack_tbl_get_from_tag(codec, tag, 0); - } - if (!jack) - return; jack->jack_dirty = 1;
codec_dbg(codec, @@ -853,7 +841,7 @@ static void hdmi_unsol_event(struct hda_codec *codec, unsigned int res) }
if (subtag == 0) - hdmi_intrinsic_event(codec, res); + hdmi_intrinsic_event(codec, res, jack); else hdmi_non_intrinsic_event(codec, res); }
Looks good to me.
Reviewed-by: Nikhil Mahale nmahale@nvidia.com
On 2/6/20 9:58 PM, Takashi Iwai wrote:
External email: Use caution opening links or attachments
Pass hda_jack_tbl object to hdmi_intrinsic_event() along with res from hdmi_unsol_event() so that we can reduce the lookup of the same hda_jack_tbl object again.
Minor code refactoring.
Signed-off-by: Takashi Iwai tiwai@suse.de
sound/pci/hda/patch_hdmi.c | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-)
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 5119a9ae3d8a..c65d16dea08c 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -779,21 +779,9 @@ static void jack_callback(struct hda_codec *codec, check_presence_and_report(codec, jack->nid, jack->dev_id); }
-static void hdmi_intrinsic_event(struct hda_codec *codec, unsigned int res) +static void hdmi_intrinsic_event(struct hda_codec *codec, unsigned int res,
struct hda_jack_tbl *jack)
{
int tag = res >> AC_UNSOL_RES_TAG_SHIFT;
struct hda_jack_tbl *jack;
if (codec->dp_mst) {
int dev_entry =
(res & AC_UNSOL_RES_DE) >> AC_UNSOL_RES_DE_SHIFT;
jack = snd_hda_jack_tbl_get_from_tag(codec, tag, dev_entry);
} else {
jack = snd_hda_jack_tbl_get_from_tag(codec, tag, 0);
}
if (!jack)
return; jack->jack_dirty = 1; codec_dbg(codec,
@@ -853,7 +841,7 @@ static void hdmi_unsol_event(struct hda_codec *codec, unsigned int res) }
if (subtag == 0)
hdmi_intrinsic_event(codec, res);
hdmi_intrinsic_event(codec, res, jack); else hdmi_non_intrinsic_event(codec, res);
}
2.16.4
----------------------------------------------------------------------------------- This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message. -----------------------------------------------------------------------------------
The current HDMI codec driver code manages the jack detection in two different ways: for Intel codecs with audio component, the driver creates snd_jack objects by itself while the standard hda_jack stuff is used for the rest. This was basically because the audio component doesn't need the pin sense reading and the unsol event handling, hence it just needs to report the corresponding jacks directly.
It was a bit messy but not too messy until the driver got DP-MST support for Nvidia that re-uses the part of dyn_pcm_assign feature while keeping the pin sense and the unsol event handling. Now, for DP-MST, we use hda_jack for pin sensing and unsol events but use the own snd_jack objects. Meanwhile for non-DP-MST, hda_jack is used for pin sense and unsol events, and the jacks are bound on hda_jack.
Moreover, there is a polling mode support where the unsol event isn't used. For those, we also have special handling.
For simplifying those messes, this patch unifies the snd_jack handling over all generic HDMI codes. The driver creates snd_jack objects just like Intel codecs did in the past but now for all devices. For the system without audio component binding, we still need the pin sense and the unsol event handling, and those are still done with the hda_jack table as before. But hda_jack is no longer used for the actual snd_jack handling.
Since the hda_jack is no longer used for jack reporting, we removed snd_hda_jack_report_sync() calls, which also allowed to simplify the return type of hda_present_sense() and co. pin_idx_to_pcm_jack() was simplified as well because it behaves same for all cases now.
Note that the hda_jack is still used for the simple HDMI codecs; they are really simple enough, so no big reason to change intrusively.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/pci/hda/patch_hdmi.c | 219 ++++++++++++--------------------------------- 1 file changed, 57 insertions(+), 162 deletions(-)
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index c65d16dea08c..98a8c4f97d6b 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -154,7 +154,6 @@ struct hdmi_spec { struct hda_multi_out multiout; struct hda_pcm_stream pcm_playback;
- bool use_jack_detect; /* jack detection enabled */ bool use_acomp_notifier; /* use eld_notify callback for hotplug */ bool acomp_registered; /* audio component registered in this driver */ struct drm_audio_component_audio_ops drm_audio_ops; @@ -753,7 +752,7 @@ static void hdmi_setup_audio_infoframe(struct hda_codec *codec, * Unsolicited events */
-static bool hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll); +static void hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll);
static void check_presence_and_report(struct hda_codec *codec, hda_nid_t nid, int dev_id) @@ -764,8 +763,7 @@ static void check_presence_and_report(struct hda_codec *codec, hda_nid_t nid, if (pin_idx < 0) return; mutex_lock(&spec->pcm_lock); - if (hdmi_present_sense(get_pin(spec, pin_idx), 1)) - snd_hda_jack_report_sync(codec); + hdmi_present_sense(get_pin(spec, pin_idx), 1); mutex_unlock(&spec->pcm_lock); }
@@ -1542,35 +1540,38 @@ static struct snd_jack *pin_idx_to_pcm_jack(struct hda_codec *codec, struct hdmi_spec_per_pin *per_pin) { struct hdmi_spec *spec = codec->spec; - struct snd_jack *jack = NULL; - struct hda_jack_tbl *jack_tbl; - - /* if !dyn_pcm_assign, get jack from hda_jack_tbl - * in !dyn_pcm_assign case, spec->pcm_rec[].jack is not - * NULL even after snd_hda_jack_tbl_clear() is called to - * free snd_jack. This may cause access invalid memory - * when calling snd_jack_report + + if (per_pin->pcm_idx >= 0) + return spec->pcm_rec[per_pin->pcm_idx].jack; + else + return NULL; +} + +static void do_update_eld(struct hda_codec *codec, + struct hdmi_spec_per_pin *per_pin, + struct hdmi_eld *eld) +{ + struct snd_jack *pcm_jack; + bool changed; + + /* + * pcm_idx >=0 before update_eld() means it is in monitor + * disconnected event. Jack must be fetched before update_eld(). */ - if (per_pin->pcm_idx >= 0 && spec->dyn_pcm_assign) { - jack = spec->pcm_rec[per_pin->pcm_idx].jack; - } else if (!spec->dyn_pcm_assign) { - /* - * jack tbl doesn't support DP MST - * DP MST will use dyn_pcm_assign, - * so DP MST will never come here - */ - jack_tbl = snd_hda_jack_tbl_get_mst(codec, per_pin->pin_nid, - per_pin->dev_id); - if (jack_tbl) - jack = jack_tbl->jack; - } - return jack; + pcm_jack = pin_idx_to_pcm_jack(codec, per_pin); + changed = update_eld(codec, per_pin, eld); + if (!pcm_jack) + pcm_jack = pin_idx_to_pcm_jack(codec, per_pin); + if (changed && pcm_jack) + snd_jack_report(pcm_jack, + (eld->monitor_present && eld->eld_valid) ? + SND_JACK_AVOUT : 0); } + /* update ELD and jack state via HD-audio verbs */ -static bool hdmi_present_sense_via_verbs(struct hdmi_spec_per_pin *per_pin, +static void hdmi_present_sense_via_verbs(struct hdmi_spec_per_pin *per_pin, int repoll) { - struct hda_jack_tbl *jack; struct hda_codec *codec = per_pin->codec; struct hdmi_spec *spec = codec->spec; struct hdmi_eld *eld = &spec->temp_eld; @@ -1585,9 +1586,7 @@ static bool hdmi_present_sense_via_verbs(struct hdmi_spec_per_pin *per_pin, * the unsolicited response to avoid custom WARs. */ int present; - bool ret; bool do_repoll = false; - struct snd_jack *pcm_jack = NULL;
present = snd_hda_jack_pin_sense(codec, pin_nid, dev_id);
@@ -1615,53 +1614,12 @@ static bool hdmi_present_sense_via_verbs(struct hdmi_spec_per_pin *per_pin, do_repoll = true; }
- if (do_repoll) { + if (do_repoll) schedule_delayed_work(&per_pin->work, msecs_to_jiffies(300)); - } else { - /* - * pcm_idx >=0 before update_eld() means it is in monitor - * disconnected event. Jack must be fetched before - * update_eld(). - */ - pcm_jack = pin_idx_to_pcm_jack(codec, per_pin); - update_eld(codec, per_pin, eld); - if (!pcm_jack) - pcm_jack = pin_idx_to_pcm_jack(codec, per_pin); - } - - ret = !repoll || !eld->monitor_present || eld->eld_valid; - - jack = snd_hda_jack_tbl_get_mst(codec, pin_nid, per_pin->dev_id); - if (jack) { - jack->block_report = !ret; - jack->pin_sense = (eld->monitor_present && eld->eld_valid) ? - AC_PINSENSE_PRESENCE : 0; - - if (spec->dyn_pcm_assign && pcm_jack && !do_repoll) { - int state = 0; - - if (jack->pin_sense & AC_PINSENSE_PRESENCE) - state = SND_JACK_AVOUT; - snd_jack_report(pcm_jack, state); - } + else + do_update_eld(codec, per_pin, eld);
- /* - * snd_hda_jack_pin_sense() call at the beginning of this - * function, updates jack->pins_sense and clears - * jack->jack_dirty, therefore snd_hda_jack_report_sync() will - * not override the jack->pin_sense. - * - * snd_hda_jack_report_sync() is superfluous for dyn_pcm_assign - * case. The jack->pin_sense update was already performed, and - * hda_jack->jack is NULL for dyn_pcm_assign. - * - * Don't call snd_hda_jack_report_sync() for - * dyn_pcm_assign. - */ - ret = ret && !spec->dyn_pcm_assign; - } mutex_unlock(&per_pin->lock); - return ret; }
/* update ELD and jack state via audio component */ @@ -1670,8 +1628,6 @@ static void sync_eld_via_acomp(struct hda_codec *codec, { struct hdmi_spec *spec = codec->spec; struct hdmi_eld *eld = &spec->temp_eld; - struct snd_jack *jack = NULL; - bool changed; int size;
mutex_lock(&per_pin->lock); @@ -1694,21 +1650,11 @@ static void sync_eld_via_acomp(struct hda_codec *codec, eld->eld_size = 0; }
- /* pcm_idx >=0 before update_eld() means it is in monitor - * disconnected event. Jack must be fetched before update_eld() - */ - jack = pin_idx_to_pcm_jack(codec, per_pin); - changed = update_eld(codec, per_pin, eld); - if (jack == NULL) - jack = pin_idx_to_pcm_jack(codec, per_pin); - if (changed && jack) - snd_jack_report(jack, - (eld->monitor_present && eld->eld_valid) ? - SND_JACK_AVOUT : 0); + do_update_eld(codec, per_pin, eld); mutex_unlock(&per_pin->lock); }
-static bool hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll) +static void hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll) { struct hda_codec *codec = per_pin->codec; int ret; @@ -1718,16 +1664,13 @@ static bool hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll) ret = snd_hda_power_up_pm(codec); if (ret < 0 && pm_runtime_suspended(hda_codec_dev(codec))) { snd_hda_power_down_pm(codec); - return false; + return; } - ret = hdmi_present_sense_via_verbs(per_pin, repoll); + hdmi_present_sense_via_verbs(per_pin, repoll); snd_hda_power_down_pm(codec); } else { sync_eld_via_acomp(codec, per_pin); - ret = false; /* don't call snd_hda_jack_report_sync() */ } - - return ret; }
static void hdmi_repoll_eld(struct work_struct *work) @@ -1747,8 +1690,7 @@ static void hdmi_repoll_eld(struct work_struct *work) per_pin->repoll_count = 0;
mutex_lock(&spec->pcm_lock); - if (hdmi_present_sense(per_pin, per_pin->repoll_count)) - snd_hda_jack_report_sync(per_pin->codec); + hdmi_present_sense(per_pin, per_pin->repoll_count); mutex_unlock(&spec->pcm_lock); }
@@ -2194,15 +2136,23 @@ static void free_hdmi_jack_priv(struct snd_jack *jack) pcm->jack = NULL; }
-static int add_hdmi_jack_kctl(struct hda_codec *codec, - struct hdmi_spec *spec, - int pcm_idx, - const char *name) +static int generic_hdmi_build_jack(struct hda_codec *codec, int pcm_idx) { + char hdmi_str[32] = "HDMI/DP"; + struct hdmi_spec *spec = codec->spec; + struct hdmi_spec_per_pin *per_pin = get_pin(spec, pcm_idx); struct snd_jack *jack; + int pcmdev = get_pcm_rec(spec, pcm_idx)->device; int err;
- err = snd_jack_new(codec->card, name, SND_JACK_AVOUT, &jack, + if (pcmdev > 0) + sprintf(hdmi_str + strlen(hdmi_str), ",pcm=%d", pcmdev); + if (!spec->dyn_pcm_assign && + !is_jack_detectable(codec, per_pin->pin_nid)) + strncat(hdmi_str, " Phantom", + sizeof(hdmi_str) - strlen(hdmi_str) - 1); + + err = snd_jack_new(codec->card, hdmi_str, SND_JACK_AVOUT, &jack, true, false); if (err < 0) return err; @@ -2213,48 +2163,6 @@ static int add_hdmi_jack_kctl(struct hda_codec *codec, return 0; }
-static int generic_hdmi_build_jack(struct hda_codec *codec, int pcm_idx) -{ - char hdmi_str[32] = "HDMI/DP"; - struct hdmi_spec *spec = codec->spec; - struct hdmi_spec_per_pin *per_pin; - struct hda_jack_tbl *jack; - int pcmdev = get_pcm_rec(spec, pcm_idx)->device; - bool phantom_jack; - int ret; - - if (pcmdev > 0) - sprintf(hdmi_str + strlen(hdmi_str), ",pcm=%d", pcmdev); - - if (spec->dyn_pcm_assign) - return add_hdmi_jack_kctl(codec, spec, pcm_idx, hdmi_str); - - /* for !dyn_pcm_assign, we still use hda_jack for compatibility */ - /* if !dyn_pcm_assign, it must be non-MST mode. - * This means pcms and pins are statically mapped. - * And pcm_idx is pin_idx. - */ - per_pin = get_pin(spec, pcm_idx); - phantom_jack = !is_jack_detectable(codec, per_pin->pin_nid); - if (phantom_jack) - strncat(hdmi_str, " Phantom", - sizeof(hdmi_str) - strlen(hdmi_str) - 1); - ret = snd_hda_jack_add_kctl_mst(codec, per_pin->pin_nid, - per_pin->dev_id, hdmi_str, phantom_jack, - 0, NULL); - if (ret < 0) - return ret; - jack = snd_hda_jack_tbl_get_mst(codec, per_pin->pin_nid, - per_pin->dev_id); - if (jack == NULL) - return 0; - /* assign jack->jack to pcm_rec[].jack to - * align with dyn_pcm_assign mode - */ - spec->pcm_rec[pcm_idx].jack = jack->jack; - return 0; -} - static int generic_hdmi_build_controls(struct hda_codec *codec) { struct hdmi_spec *spec = codec->spec; @@ -2343,7 +2251,6 @@ static int generic_hdmi_init(struct hda_codec *codec) int pin_idx;
mutex_lock(&spec->bind_lock); - spec->use_jack_detect = !codec->jackpoll_interval; for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++) { struct hdmi_spec_per_pin *per_pin = get_pin(spec, pin_idx); hda_nid_t pin_nid = per_pin->pin_nid; @@ -2353,12 +2260,8 @@ static int generic_hdmi_init(struct hda_codec *codec) hdmi_init_pin(codec, pin_nid); if (codec_has_acomp(codec)) continue; - if (spec->use_jack_detect) - snd_hda_jack_detect_enable(codec, pin_nid, dev_id); - else - snd_hda_jack_detect_enable_callback_mst(codec, pin_nid, - dev_id, - jack_callback); + snd_hda_jack_detect_enable_callback_mst(codec, pin_nid, dev_id, + jack_callback); } mutex_unlock(&spec->bind_lock); return 0; @@ -2520,12 +2423,6 @@ static void reprogram_jack_detect(struct hda_codec *codec, hda_nid_t nid, unsigned int val = use_acomp ? 0 : (AC_USRSP_EN | tbl->tag); snd_hda_codec_write_cache(codec, nid, 0, AC_VERB_SET_UNSOLICITED_ENABLE, val); - } else { - /* if no jack entry was defined beforehand, create a new one - * at need (i.e. only when notifier is cleared) - */ - if (!use_acomp) - snd_hda_jack_detect_enable(codec, nid, dev_id); } }
@@ -2541,13 +2438,11 @@ static void generic_acomp_notifier_set(struct drm_audio_component *acomp, spec->use_acomp_notifier = use_acomp; spec->codec->relaxed_resume = use_acomp; /* reprogram each jack detection logic depending on the notifier */ - if (spec->use_jack_detect) { - for (i = 0; i < spec->num_pins; i++) - reprogram_jack_detect(spec->codec, - get_pin(spec, i)->pin_nid, - get_pin(spec, i)->dev_id, - use_acomp); - } + for (i = 0; i < spec->num_pins; i++) + reprogram_jack_detect(spec->codec, + get_pin(spec, i)->pin_nid, + get_pin(spec, i)->dev_id, + use_acomp); mutex_unlock(&spec->bind_lock); }
Looks good to me.
Reviewed-by: Nikhil Mahale nmahale@nvidia.com
On 2/6/20 9:58 PM, Takashi Iwai wrote:
External email: Use caution opening links or attachments
The current HDMI codec driver code manages the jack detection in two different ways: for Intel codecs with audio component, the driver creates snd_jack objects by itself while the standard hda_jack stuff is used for the rest. This was basically because the audio component doesn't need the pin sense reading and the unsol event handling, hence it just needs to report the corresponding jacks directly.
It was a bit messy but not too messy until the driver got DP-MST support for Nvidia that re-uses the part of dyn_pcm_assign feature while keeping the pin sense and the unsol event handling. Now, for DP-MST, we use hda_jack for pin sensing and unsol events but use the own snd_jack objects. Meanwhile for non-DP-MST, hda_jack is used for pin sense and unsol events, and the jacks are bound on hda_jack.
Moreover, there is a polling mode support where the unsol event isn't used. For those, we also have special handling.
For simplifying those messes, this patch unifies the snd_jack handling over all generic HDMI codes. The driver creates snd_jack objects just like Intel codecs did in the past but now for all devices. For the system without audio component binding, we still need the pin sense and the unsol event handling, and those are still done with the hda_jack table as before. But hda_jack is no longer used for the actual snd_jack handling.
Since the hda_jack is no longer used for jack reporting, we removed snd_hda_jack_report_sync() calls, which also allowed to simplify the return type of hda_present_sense() and co. pin_idx_to_pcm_jack() was simplified as well because it behaves same for all cases now.
Note that the hda_jack is still used for the simple HDMI codecs; they are really simple enough, so no big reason to change intrusively.
Signed-off-by: Takashi Iwai tiwai@suse.de
sound/pci/hda/patch_hdmi.c | 219 ++++++++++++--------------------------------- 1 file changed, 57 insertions(+), 162 deletions(-)
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index c65d16dea08c..98a8c4f97d6b 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -154,7 +154,6 @@ struct hdmi_spec { struct hda_multi_out multiout; struct hda_pcm_stream pcm_playback;
bool use_jack_detect; /* jack detection enabled */ bool use_acomp_notifier; /* use eld_notify callback for hotplug */ bool acomp_registered; /* audio component registered in this driver */ struct drm_audio_component_audio_ops drm_audio_ops;
@@ -753,7 +752,7 @@ static void hdmi_setup_audio_infoframe(struct hda_codec *codec,
- Unsolicited events
*/
-static bool hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll); +static void hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll);
static void check_presence_and_report(struct hda_codec *codec, hda_nid_t nid, int dev_id) @@ -764,8 +763,7 @@ static void check_presence_and_report(struct hda_codec *codec, hda_nid_t nid, if (pin_idx < 0) return; mutex_lock(&spec->pcm_lock);
if (hdmi_present_sense(get_pin(spec, pin_idx), 1))
snd_hda_jack_report_sync(codec);
hdmi_present_sense(get_pin(spec, pin_idx), 1); mutex_unlock(&spec->pcm_lock);
}
@@ -1542,35 +1540,38 @@ static struct snd_jack *pin_idx_to_pcm_jack(struct hda_codec *codec, struct hdmi_spec_per_pin *per_pin) { struct hdmi_spec *spec = codec->spec;
struct snd_jack *jack = NULL;
struct hda_jack_tbl *jack_tbl;
/* if !dyn_pcm_assign, get jack from hda_jack_tbl
* in !dyn_pcm_assign case, spec->pcm_rec[].jack is not
* NULL even after snd_hda_jack_tbl_clear() is called to
* free snd_jack. This may cause access invalid memory
* when calling snd_jack_report
if (per_pin->pcm_idx >= 0)
return spec->pcm_rec[per_pin->pcm_idx].jack;
else
return NULL;
+}
+static void do_update_eld(struct hda_codec *codec,
struct hdmi_spec_per_pin *per_pin,
struct hdmi_eld *eld)
+{
struct snd_jack *pcm_jack;
bool changed;
/*
* pcm_idx >=0 before update_eld() means it is in monitor
* disconnected event. Jack must be fetched before update_eld(). */
if (per_pin->pcm_idx >= 0 && spec->dyn_pcm_assign) {
jack = spec->pcm_rec[per_pin->pcm_idx].jack;
} else if (!spec->dyn_pcm_assign) {
/*
* jack tbl doesn't support DP MST
* DP MST will use dyn_pcm_assign,
* so DP MST will never come here
*/
jack_tbl = snd_hda_jack_tbl_get_mst(codec, per_pin->pin_nid,
per_pin->dev_id);
if (jack_tbl)
jack = jack_tbl->jack;
}
return jack;
pcm_jack = pin_idx_to_pcm_jack(codec, per_pin);
changed = update_eld(codec, per_pin, eld);
if (!pcm_jack)
pcm_jack = pin_idx_to_pcm_jack(codec, per_pin);
if (changed && pcm_jack)
snd_jack_report(pcm_jack,
(eld->monitor_present && eld->eld_valid) ?
SND_JACK_AVOUT : 0);
}
/* update ELD and jack state via HD-audio verbs */ -static bool hdmi_present_sense_via_verbs(struct hdmi_spec_per_pin *per_pin, +static void hdmi_present_sense_via_verbs(struct hdmi_spec_per_pin *per_pin, int repoll) {
struct hda_jack_tbl *jack; struct hda_codec *codec = per_pin->codec; struct hdmi_spec *spec = codec->spec; struct hdmi_eld *eld = &spec->temp_eld;
@@ -1585,9 +1586,7 @@ static bool hdmi_present_sense_via_verbs(struct hdmi_spec_per_pin *per_pin, * the unsolicited response to avoid custom WARs. */ int present;
bool ret; bool do_repoll = false;
struct snd_jack *pcm_jack = NULL; present = snd_hda_jack_pin_sense(codec, pin_nid, dev_id);
@@ -1615,53 +1614,12 @@ static bool hdmi_present_sense_via_verbs(struct hdmi_spec_per_pin *per_pin, do_repoll = true; }
if (do_repoll) {
if (do_repoll) schedule_delayed_work(&per_pin->work, msecs_to_jiffies(300));
} else {
/*
* pcm_idx >=0 before update_eld() means it is in monitor
* disconnected event. Jack must be fetched before
* update_eld().
*/
pcm_jack = pin_idx_to_pcm_jack(codec, per_pin);
update_eld(codec, per_pin, eld);
if (!pcm_jack)
pcm_jack = pin_idx_to_pcm_jack(codec, per_pin);
}
ret = !repoll || !eld->monitor_present || eld->eld_valid;
jack = snd_hda_jack_tbl_get_mst(codec, pin_nid, per_pin->dev_id);
if (jack) {
jack->block_report = !ret;
jack->pin_sense = (eld->monitor_present && eld->eld_valid) ?
AC_PINSENSE_PRESENCE : 0;
if (spec->dyn_pcm_assign && pcm_jack && !do_repoll) {
int state = 0;
if (jack->pin_sense & AC_PINSENSE_PRESENCE)
state = SND_JACK_AVOUT;
snd_jack_report(pcm_jack, state);
}
else
do_update_eld(codec, per_pin, eld);
/*
* snd_hda_jack_pin_sense() call at the beginning of this
* function, updates jack->pins_sense and clears
* jack->jack_dirty, therefore snd_hda_jack_report_sync() will
* not override the jack->pin_sense.
*
* snd_hda_jack_report_sync() is superfluous for dyn_pcm_assign
* case. The jack->pin_sense update was already performed, and
* hda_jack->jack is NULL for dyn_pcm_assign.
*
* Don't call snd_hda_jack_report_sync() for
* dyn_pcm_assign.
*/
ret = ret && !spec->dyn_pcm_assign;
} mutex_unlock(&per_pin->lock);
return ret;
}
/* update ELD and jack state via audio component */ @@ -1670,8 +1628,6 @@ static void sync_eld_via_acomp(struct hda_codec *codec, { struct hdmi_spec *spec = codec->spec; struct hdmi_eld *eld = &spec->temp_eld;
struct snd_jack *jack = NULL;
bool changed; int size; mutex_lock(&per_pin->lock);
@@ -1694,21 +1650,11 @@ static void sync_eld_via_acomp(struct hda_codec *codec, eld->eld_size = 0; }
/* pcm_idx >=0 before update_eld() means it is in monitor
* disconnected event. Jack must be fetched before update_eld()
*/
jack = pin_idx_to_pcm_jack(codec, per_pin);
changed = update_eld(codec, per_pin, eld);
if (jack == NULL)
jack = pin_idx_to_pcm_jack(codec, per_pin);
if (changed && jack)
snd_jack_report(jack,
(eld->monitor_present && eld->eld_valid) ?
SND_JACK_AVOUT : 0);
do_update_eld(codec, per_pin, eld); mutex_unlock(&per_pin->lock);
}
-static bool hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll) +static void hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll) { struct hda_codec *codec = per_pin->codec; int ret; @@ -1718,16 +1664,13 @@ static bool hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll) ret = snd_hda_power_up_pm(codec); if (ret < 0 && pm_runtime_suspended(hda_codec_dev(codec))) { snd_hda_power_down_pm(codec);
return false;
return; }
ret = hdmi_present_sense_via_verbs(per_pin, repoll);
hdmi_present_sense_via_verbs(per_pin, repoll); snd_hda_power_down_pm(codec); } else { sync_eld_via_acomp(codec, per_pin);
ret = false; /* don't call snd_hda_jack_report_sync() */ }
return ret;
}
static void hdmi_repoll_eld(struct work_struct *work) @@ -1747,8 +1690,7 @@ static void hdmi_repoll_eld(struct work_struct *work) per_pin->repoll_count = 0;
mutex_lock(&spec->pcm_lock);
if (hdmi_present_sense(per_pin, per_pin->repoll_count))
snd_hda_jack_report_sync(per_pin->codec);
hdmi_present_sense(per_pin, per_pin->repoll_count); mutex_unlock(&spec->pcm_lock);
}
@@ -2194,15 +2136,23 @@ static void free_hdmi_jack_priv(struct snd_jack *jack) pcm->jack = NULL; }
-static int add_hdmi_jack_kctl(struct hda_codec *codec,
struct hdmi_spec *spec,
int pcm_idx,
const char *name)
+static int generic_hdmi_build_jack(struct hda_codec *codec, int pcm_idx) {
char hdmi_str[32] = "HDMI/DP";
struct hdmi_spec *spec = codec->spec;
struct hdmi_spec_per_pin *per_pin = get_pin(spec, pcm_idx); struct snd_jack *jack;
int pcmdev = get_pcm_rec(spec, pcm_idx)->device; int err;
err = snd_jack_new(codec->card, name, SND_JACK_AVOUT, &jack,
if (pcmdev > 0)
sprintf(hdmi_str + strlen(hdmi_str), ",pcm=%d", pcmdev);
if (!spec->dyn_pcm_assign &&
!is_jack_detectable(codec, per_pin->pin_nid))
strncat(hdmi_str, " Phantom",
sizeof(hdmi_str) - strlen(hdmi_str) - 1);
err = snd_jack_new(codec->card, hdmi_str, SND_JACK_AVOUT, &jack, true, false); if (err < 0) return err;
@@ -2213,48 +2163,6 @@ static int add_hdmi_jack_kctl(struct hda_codec *codec, return 0; }
-static int generic_hdmi_build_jack(struct hda_codec *codec, int pcm_idx) -{
char hdmi_str[32] = "HDMI/DP";
struct hdmi_spec *spec = codec->spec;
struct hdmi_spec_per_pin *per_pin;
struct hda_jack_tbl *jack;
int pcmdev = get_pcm_rec(spec, pcm_idx)->device;
bool phantom_jack;
int ret;
if (pcmdev > 0)
sprintf(hdmi_str + strlen(hdmi_str), ",pcm=%d", pcmdev);
if (spec->dyn_pcm_assign)
return add_hdmi_jack_kctl(codec, spec, pcm_idx, hdmi_str);
/* for !dyn_pcm_assign, we still use hda_jack for compatibility */
/* if !dyn_pcm_assign, it must be non-MST mode.
* This means pcms and pins are statically mapped.
* And pcm_idx is pin_idx.
*/
per_pin = get_pin(spec, pcm_idx);
phantom_jack = !is_jack_detectable(codec, per_pin->pin_nid);
if (phantom_jack)
strncat(hdmi_str, " Phantom",
sizeof(hdmi_str) - strlen(hdmi_str) - 1);
ret = snd_hda_jack_add_kctl_mst(codec, per_pin->pin_nid,
per_pin->dev_id, hdmi_str, phantom_jack,
0, NULL);
if (ret < 0)
return ret;
jack = snd_hda_jack_tbl_get_mst(codec, per_pin->pin_nid,
per_pin->dev_id);
if (jack == NULL)
return 0;
/* assign jack->jack to pcm_rec[].jack to
* align with dyn_pcm_assign mode
*/
spec->pcm_rec[pcm_idx].jack = jack->jack;
return 0;
-}
static int generic_hdmi_build_controls(struct hda_codec *codec) { struct hdmi_spec *spec = codec->spec; @@ -2343,7 +2251,6 @@ static int generic_hdmi_init(struct hda_codec *codec) int pin_idx;
mutex_lock(&spec->bind_lock);
spec->use_jack_detect = !codec->jackpoll_interval; for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++) { struct hdmi_spec_per_pin *per_pin = get_pin(spec, pin_idx); hda_nid_t pin_nid = per_pin->pin_nid;
@@ -2353,12 +2260,8 @@ static int generic_hdmi_init(struct hda_codec *codec) hdmi_init_pin(codec, pin_nid); if (codec_has_acomp(codec)) continue;
if (spec->use_jack_detect)
snd_hda_jack_detect_enable(codec, pin_nid, dev_id);
else
snd_hda_jack_detect_enable_callback_mst(codec, pin_nid,
dev_id,
jack_callback);
snd_hda_jack_detect_enable_callback_mst(codec, pin_nid, dev_id,
jack_callback); } mutex_unlock(&spec->bind_lock); return 0;
@@ -2520,12 +2423,6 @@ static void reprogram_jack_detect(struct hda_codec *codec, hda_nid_t nid, unsigned int val = use_acomp ? 0 : (AC_USRSP_EN | tbl->tag); snd_hda_codec_write_cache(codec, nid, 0, AC_VERB_SET_UNSOLICITED_ENABLE, val);
} else {
/* if no jack entry was defined beforehand, create a new one
* at need (i.e. only when notifier is cleared)
*/
if (!use_acomp)
snd_hda_jack_detect_enable(codec, nid, dev_id); }
}
@@ -2541,13 +2438,11 @@ static void generic_acomp_notifier_set(struct drm_audio_component *acomp, spec->use_acomp_notifier = use_acomp; spec->codec->relaxed_resume = use_acomp; /* reprogram each jack detection logic depending on the notifier */
if (spec->use_jack_detect) {
for (i = 0; i < spec->num_pins; i++)
reprogram_jack_detect(spec->codec,
get_pin(spec, i)->pin_nid,
get_pin(spec, i)->dev_id,
use_acomp);
}
for (i = 0; i < spec->num_pins; i++)
reprogram_jack_detect(spec->codec,
get_pin(spec, i)->pin_nid,
get_pin(spec, i)->dev_id,
use_acomp); mutex_unlock(&spec->bind_lock);
}
-- 2.16.4
----------------------------------------------------------------------------------- This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message. -----------------------------------------------------------------------------------
For improving the readability, move the runtime PM handling code from hdmi_present_sense() to hdmi_present_sense_via_verbs(). Now hdmi_present_sense() became symmetric for both audio-component and legacy cases.
Just a minor code refactoring.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/pci/hda/patch_hdmi.c | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 98a8c4f97d6b..437177294d78 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -1587,6 +1587,11 @@ static void hdmi_present_sense_via_verbs(struct hdmi_spec_per_pin *per_pin, */ int present; bool do_repoll = false; + int ret; + + ret = snd_hda_power_up_pm(codec); + if (ret < 0 && pm_runtime_suspended(hda_codec_dev(codec))) + goto out;
present = snd_hda_jack_pin_sense(codec, pin_nid, dev_id);
@@ -1620,6 +1625,8 @@ static void hdmi_present_sense_via_verbs(struct hdmi_spec_per_pin *per_pin, do_update_eld(codec, per_pin, eld);
mutex_unlock(&per_pin->lock); + out: + snd_hda_power_down_pm(codec); }
/* update ELD and jack state via audio component */ @@ -1657,20 +1664,11 @@ static void sync_eld_via_acomp(struct hda_codec *codec, static void hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll) { struct hda_codec *codec = per_pin->codec; - int ret;
- /* no temporary power up/down needed for component notifier */ - if (!codec_has_acomp(codec)) { - ret = snd_hda_power_up_pm(codec); - if (ret < 0 && pm_runtime_suspended(hda_codec_dev(codec))) { - snd_hda_power_down_pm(codec); - return; - } + if (!codec_has_acomp(codec)) hdmi_present_sense_via_verbs(per_pin, repoll); - snd_hda_power_down_pm(codec); - } else { + else sync_eld_via_acomp(codec, per_pin); - } }
static void hdmi_repoll_eld(struct work_struct *work)
Looks good to me.
Reviewed-by: Nikhil Mahale nmahale@nvidia.com
On 2/6/20 9:58 PM, Takashi Iwai wrote:
External email: Use caution opening links or attachments
For improving the readability, move the runtime PM handling code from hdmi_present_sense() to hdmi_present_sense_via_verbs(). Now hdmi_present_sense() became symmetric for both audio-component and legacy cases.
Just a minor code refactoring.
Signed-off-by: Takashi Iwai tiwai@suse.de
sound/pci/hda/patch_hdmi.c | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 98a8c4f97d6b..437177294d78 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -1587,6 +1587,11 @@ static void hdmi_present_sense_via_verbs(struct hdmi_spec_per_pin *per_pin, */ int present; bool do_repoll = false;
int ret;
ret = snd_hda_power_up_pm(codec);
if (ret < 0 && pm_runtime_suspended(hda_codec_dev(codec)))
goto out; present = snd_hda_jack_pin_sense(codec, pin_nid, dev_id);
@@ -1620,6 +1625,8 @@ static void hdmi_present_sense_via_verbs(struct hdmi_spec_per_pin *per_pin, do_update_eld(codec, per_pin, eld);
mutex_unlock(&per_pin->lock);
- out:
snd_hda_power_down_pm(codec);
}
/* update ELD and jack state via audio component */ @@ -1657,20 +1664,11 @@ static void sync_eld_via_acomp(struct hda_codec *codec, static void hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll) { struct hda_codec *codec = per_pin->codec;
int ret;
/* no temporary power up/down needed for component notifier */
if (!codec_has_acomp(codec)) {
ret = snd_hda_power_up_pm(codec);
if (ret < 0 && pm_runtime_suspended(hda_codec_dev(codec))) {
snd_hda_power_down_pm(codec);
return;
}
if (!codec_has_acomp(codec)) hdmi_present_sense_via_verbs(per_pin, repoll);
snd_hda_power_down_pm(codec);
} else {
else sync_eld_via_acomp(codec, per_pin);
}
}
static void hdmi_repoll_eld(struct work_struct *work)
2.16.4
This is a final step of the cleanup series: move the HDMI ELD parser call into update_eld() function so that we can unify the calls. The ELD validity check is unified in update_eld(), too.
Along with it, the repoll scheduling is moved to update_eld() as well, where sync_eld_via_acomp() just passes 0 for skipping it.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/pci/hda/patch_hdmi.c | 110 ++++++++++++++++++++------------------------- 1 file changed, 48 insertions(+), 62 deletions(-)
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 437177294d78..bb287a916dae 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -1466,21 +1466,60 @@ static void hdmi_pcm_reset_pin(struct hdmi_spec *spec, per_pin->channels = 0; }
+static struct snd_jack *pin_idx_to_pcm_jack(struct hda_codec *codec, + struct hdmi_spec_per_pin *per_pin) +{ + struct hdmi_spec *spec = codec->spec; + + if (per_pin->pcm_idx >= 0) + return spec->pcm_rec[per_pin->pcm_idx].jack; + else + return NULL; +} + /* update per_pin ELD from the given new ELD; * setup info frame and notification accordingly + * also notify ELD kctl and report jack status changes */ -static bool update_eld(struct hda_codec *codec, +static void update_eld(struct hda_codec *codec, struct hdmi_spec_per_pin *per_pin, - struct hdmi_eld *eld) + struct hdmi_eld *eld, + int repoll) { struct hdmi_eld *pin_eld = &per_pin->sink_eld; struct hdmi_spec *spec = codec->spec; + struct snd_jack *pcm_jack; bool old_eld_valid = pin_eld->eld_valid; bool eld_changed; int pcm_idx;
+ if (eld->eld_valid) { + if (eld->eld_size <= 0 || + snd_hdmi_parse_eld(codec, &eld->info, eld->eld_buffer, + eld->eld_size) < 0) { + eld->eld_valid = false; + if (repoll) { + schedule_delayed_work(&per_pin->work, + msecs_to_jiffies(300)); + return; + } + } + } + + if (!eld->eld_valid || eld->eld_size <= 0) { + eld->eld_valid = false; + eld->eld_size = 0; + } + /* for monitor disconnection, save pcm_idx firstly */ pcm_idx = per_pin->pcm_idx; + + /* + * pcm_idx >=0 before update_eld() means it is in monitor + * disconnected event. Jack must be fetched before update_eld(). + */ + pcm_jack = pin_idx_to_pcm_jack(codec, per_pin); + if (spec->dyn_pcm_assign) { if (eld->eld_valid) { hdmi_attach_hda_pcm(spec, per_pin); @@ -1495,6 +1534,8 @@ static bool update_eld(struct hda_codec *codec, */ if (pcm_idx == -1) pcm_idx = per_pin->pcm_idx; + if (!pcm_jack) + pcm_jack = pin_idx_to_pcm_jack(codec, per_pin);
if (eld->eld_valid) snd_hdmi_show_eld(codec, &eld->info); @@ -1533,36 +1574,8 @@ static bool update_eld(struct hda_codec *codec, SNDRV_CTL_EVENT_MASK_VALUE | SNDRV_CTL_EVENT_MASK_INFO, &get_hdmi_pcm(spec, pcm_idx)->eld_ctl->id); - return eld_changed; -} - -static struct snd_jack *pin_idx_to_pcm_jack(struct hda_codec *codec, - struct hdmi_spec_per_pin *per_pin) -{ - struct hdmi_spec *spec = codec->spec;
- if (per_pin->pcm_idx >= 0) - return spec->pcm_rec[per_pin->pcm_idx].jack; - else - return NULL; -} - -static void do_update_eld(struct hda_codec *codec, - struct hdmi_spec_per_pin *per_pin, - struct hdmi_eld *eld) -{ - struct snd_jack *pcm_jack; - bool changed; - - /* - * pcm_idx >=0 before update_eld() means it is in monitor - * disconnected event. Jack must be fetched before update_eld(). - */ - pcm_jack = pin_idx_to_pcm_jack(codec, per_pin); - changed = update_eld(codec, per_pin, eld); - if (!pcm_jack) - pcm_jack = pin_idx_to_pcm_jack(codec, per_pin); - if (changed && pcm_jack) + if (eld_changed && pcm_jack) snd_jack_report(pcm_jack, (eld->monitor_present && eld->eld_valid) ? SND_JACK_AVOUT : 0); @@ -1586,7 +1599,6 @@ static void hdmi_present_sense_via_verbs(struct hdmi_spec_per_pin *per_pin, * the unsolicited response to avoid custom WARs. */ int present; - bool do_repoll = false; int ret;
ret = snd_hda_power_up_pm(codec); @@ -1610,20 +1622,9 @@ static void hdmi_present_sense_via_verbs(struct hdmi_spec_per_pin *per_pin, if (spec->ops.pin_get_eld(codec, pin_nid, dev_id, eld->eld_buffer, &eld->eld_size) < 0) eld->eld_valid = false; - else { - if (snd_hdmi_parse_eld(codec, &eld->info, eld->eld_buffer, - eld->eld_size) < 0) - eld->eld_valid = false; - } - if (!eld->eld_valid && repoll) - do_repoll = true; }
- if (do_repoll) - schedule_delayed_work(&per_pin->work, msecs_to_jiffies(300)); - else - do_update_eld(codec, per_pin, eld); - + update_eld(codec, per_pin, eld, repoll); mutex_unlock(&per_pin->lock); out: snd_hda_power_down_pm(codec); @@ -1635,29 +1636,14 @@ static void sync_eld_via_acomp(struct hda_codec *codec, { struct hdmi_spec *spec = codec->spec; struct hdmi_eld *eld = &spec->temp_eld; - int size;
mutex_lock(&per_pin->lock); eld->monitor_present = false; - size = snd_hdac_acomp_get_eld(&codec->core, per_pin->pin_nid, + eld->eld_size = snd_hdac_acomp_get_eld(&codec->core, per_pin->pin_nid, per_pin->dev_id, &eld->monitor_present, eld->eld_buffer, ELD_MAX_SIZE); - if (size > 0) { - size = min(size, ELD_MAX_SIZE); - if (snd_hdmi_parse_eld(codec, &eld->info, - eld->eld_buffer, size) < 0) - size = -EINVAL; - } - - if (size > 0) { - eld->eld_valid = true; - eld->eld_size = size; - } else { - eld->eld_valid = false; - eld->eld_size = 0; - } - - do_update_eld(codec, per_pin, eld); + eld->eld_valid = (eld->eld_size > 0); + update_eld(codec, per_pin, eld, 0); mutex_unlock(&per_pin->lock); }
Looks good to me.
Reviewed-by: Nikhil Mahale nmahale@nvidia.com
On 2/6/20 9:58 PM, Takashi Iwai wrote:
External email: Use caution opening links or attachments
This is a final step of the cleanup series: move the HDMI ELD parser call into update_eld() function so that we can unify the calls. The ELD validity check is unified in update_eld(), too.
Along with it, the repoll scheduling is moved to update_eld() as well, where sync_eld_via_acomp() just passes 0 for skipping it.
Signed-off-by: Takashi Iwai tiwai@suse.de
sound/pci/hda/patch_hdmi.c | 110 ++++++++++++++++++++------------------------- 1 file changed, 48 insertions(+), 62 deletions(-)
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 437177294d78..bb287a916dae 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -1466,21 +1466,60 @@ static void hdmi_pcm_reset_pin(struct hdmi_spec *spec, per_pin->channels = 0; }
+static struct snd_jack *pin_idx_to_pcm_jack(struct hda_codec *codec,
struct hdmi_spec_per_pin *per_pin)
+{
struct hdmi_spec *spec = codec->spec;
if (per_pin->pcm_idx >= 0)
return spec->pcm_rec[per_pin->pcm_idx].jack;
else
return NULL;
+}
/* update per_pin ELD from the given new ELD;
- setup info frame and notification accordingly
*/
- also notify ELD kctl and report jack status changes
-static bool update_eld(struct hda_codec *codec, +static void update_eld(struct hda_codec *codec, struct hdmi_spec_per_pin *per_pin,
struct hdmi_eld *eld)
struct hdmi_eld *eld,
int repoll)
{ struct hdmi_eld *pin_eld = &per_pin->sink_eld; struct hdmi_spec *spec = codec->spec;
struct snd_jack *pcm_jack; bool old_eld_valid = pin_eld->eld_valid; bool eld_changed; int pcm_idx;
if (eld->eld_valid) {
if (eld->eld_size <= 0 ||
snd_hdmi_parse_eld(codec, &eld->info, eld->eld_buffer,
eld->eld_size) < 0) {
eld->eld_valid = false;
if (repoll) {
schedule_delayed_work(&per_pin->work,
msecs_to_jiffies(300));
return;
}
}
}
if (!eld->eld_valid || eld->eld_size <= 0) {
eld->eld_valid = false;
eld->eld_size = 0;
}
/* for monitor disconnection, save pcm_idx firstly */ pcm_idx = per_pin->pcm_idx;
/*
* pcm_idx >=0 before update_eld() means it is in monitor
* disconnected event. Jack must be fetched before update_eld().
*/
pcm_jack = pin_idx_to_pcm_jack(codec, per_pin);
if (spec->dyn_pcm_assign) { if (eld->eld_valid) { hdmi_attach_hda_pcm(spec, per_pin);
@@ -1495,6 +1534,8 @@ static bool update_eld(struct hda_codec *codec, */ if (pcm_idx == -1) pcm_idx = per_pin->pcm_idx;
if (!pcm_jack)
pcm_jack = pin_idx_to_pcm_jack(codec, per_pin); if (eld->eld_valid) snd_hdmi_show_eld(codec, &eld->info);
@@ -1533,36 +1574,8 @@ static bool update_eld(struct hda_codec *codec, SNDRV_CTL_EVENT_MASK_VALUE | SNDRV_CTL_EVENT_MASK_INFO, &get_hdmi_pcm(spec, pcm_idx)->eld_ctl->id);
return eld_changed;
-}
-static struct snd_jack *pin_idx_to_pcm_jack(struct hda_codec *codec,
struct hdmi_spec_per_pin *per_pin)
-{
struct hdmi_spec *spec = codec->spec;
if (per_pin->pcm_idx >= 0)
return spec->pcm_rec[per_pin->pcm_idx].jack;
else
return NULL;
-}
-static void do_update_eld(struct hda_codec *codec,
struct hdmi_spec_per_pin *per_pin,
struct hdmi_eld *eld)
-{
struct snd_jack *pcm_jack;
bool changed;
/*
* pcm_idx >=0 before update_eld() means it is in monitor
* disconnected event. Jack must be fetched before update_eld().
*/
pcm_jack = pin_idx_to_pcm_jack(codec, per_pin);
changed = update_eld(codec, per_pin, eld);
if (!pcm_jack)
pcm_jack = pin_idx_to_pcm_jack(codec, per_pin);
if (changed && pcm_jack)
if (eld_changed && pcm_jack) snd_jack_report(pcm_jack, (eld->monitor_present && eld->eld_valid) ? SND_JACK_AVOUT : 0);
@@ -1586,7 +1599,6 @@ static void hdmi_present_sense_via_verbs(struct hdmi_spec_per_pin *per_pin, * the unsolicited response to avoid custom WARs. */ int present;
bool do_repoll = false; int ret; ret = snd_hda_power_up_pm(codec);
@@ -1610,20 +1622,9 @@ static void hdmi_present_sense_via_verbs(struct hdmi_spec_per_pin *per_pin, if (spec->ops.pin_get_eld(codec, pin_nid, dev_id, eld->eld_buffer, &eld->eld_size) < 0) eld->eld_valid = false;
else {
if (snd_hdmi_parse_eld(codec, &eld->info, eld->eld_buffer,
eld->eld_size) < 0)
eld->eld_valid = false;
}
if (!eld->eld_valid && repoll)
do_repoll = true; }
if (do_repoll)
schedule_delayed_work(&per_pin->work, msecs_to_jiffies(300));
else
do_update_eld(codec, per_pin, eld);
out: snd_hda_power_down_pm(codec);update_eld(codec, per_pin, eld, repoll); mutex_unlock(&per_pin->lock);
@@ -1635,29 +1636,14 @@ static void sync_eld_via_acomp(struct hda_codec *codec, { struct hdmi_spec *spec = codec->spec; struct hdmi_eld *eld = &spec->temp_eld;
int size; mutex_lock(&per_pin->lock); eld->monitor_present = false;
size = snd_hdac_acomp_get_eld(&codec->core, per_pin->pin_nid,
eld->eld_size = snd_hdac_acomp_get_eld(&codec->core, per_pin->pin_nid, per_pin->dev_id, &eld->monitor_present, eld->eld_buffer, ELD_MAX_SIZE);
if (size > 0) {
size = min(size, ELD_MAX_SIZE);
if (snd_hdmi_parse_eld(codec, &eld->info,
eld->eld_buffer, size) < 0)
size = -EINVAL;
}
if (size > 0) {
eld->eld_valid = true;
eld->eld_size = size;
} else {
eld->eld_valid = false;
eld->eld_size = 0;
}
do_update_eld(codec, per_pin, eld);
eld->eld_valid = (eld->eld_size > 0);
update_eld(codec, per_pin, eld, 0); mutex_unlock(&per_pin->lock);
}
-- 2.16.4
Hi,
On Thu, 6 Feb 2020, Takashi Iwai wrote:
this is a series of cleanup of the jack handling in HD-audio HDMI codec driver, which I realized after the recent regression fix.
another welcome change to the hda/hdmi code!
For patches 1, 3 and 4:
Reviewed-by: Kai Vehmanen kai.vehmanen@linux.intel.com
For second patch, the code modified is less familiar to me, so review will take more time. Great if Nikhil you can go over this.
I did do a test of the whole series with SOF on some devices and no regressions found.
Br, Kai
participants (3)
-
Kai Vehmanen
-
Nikhil Mahale
-
Takashi Iwai