[alsa-devel] [PATCH] ALSA: hda/hdmi - Don't report Jack event if no need to do that
Hui Wang
hui.wang at canonical.com
Sat May 4 04:45:36 CEST 2019
On 2019/5/3 下午11:57, Takashi Iwai wrote:
> On Fri, 03 May 2019 06:05:09 +0200,
> Hui Wang wrote:
>>
>> On 2019/5/2 下午11:49, Takashi Iwai wrote:
>>> On Thu, 02 May 2019 04:52:46 +0200,
>>> Hui Wang wrote:
>>>> On 2019/4/30 下午5:02, Takashi Iwai wrote:
>>>>> On Tue, 30 Apr 2019 10:42:55 +0200,
>>>>> Hui Wang wrote:
>>>>>> On 2019/4/30 下午3:35, Takashi Iwai wrote:
>>>>>>> On Tue, 30 Apr 2019 08:57:11 +0200,
>>>>>>> Hui Wang wrote:
>>>>>>>> On the machines with AMD GPU or Nvidia GPU, we often meet this issues:
>>>>>>>> after s3, there are 4 HDMI/DP audio devices in the gnome-sound-setting
>>>>>>>> even there is no any monitors plugged.
>>>>>>>>
>>>>>>>> When this problem happens, we check the /proc/asound/cardX/eld#N.M, we
>>>>>>>> will find the monitor_present=1, eld_valid=0.
>>>>>>>>
>>>>>>>> So the fix is putting your change and my change together like below?
>>> Well, removing "!repoll" should be treated as a separate patch, and
>>> I'm not sure whether we want to get rid of it.
>>>
>>> The "!repoll" check is a bit misleading. This condition indicates
>>> that the jack status sync is required, e.g. it's the case where
>>> polling goes to timeout or at the beginning of the probe. For
>>> example, in the case of resume, it starts with repoll=1. If an
>>> incomplete state (monitor=1, eld_valid=0) is seen at this point, the
>>> code schedules for the retry at a later point. And if it reaches to
>>> the max count, it clears to repoll=0 and try the last attempt, which
>>> won't schedule any longer.
>>>
>>> So, at this point, we *have to* notify the result no matter whether
>>> it's satisfying or not.
>>>
>>>
>>> thanks,
>>>
>>> Takashi
>> So let us do sth in the hda_jack.c, adding two members check_eld and
>> eld_valid in the struct hda_jack_tbl{},
>>
>> When building new jacks, since there is no eld_data yet, just follow
>> old rules (only checking AC_PINSENSE_PRESENCE)
>>
>> After building new jacks, if it is not a phantom one, set check_eld to 1
>>
>> every time we get eld (via verbs or acomp), we will sync
>> jack->eld_valid to eld->eld_valid.
>>
>> in the report_sync(), we will check check_eld and eld_valid, not
>> depends on AC_PINSENSE_PRESENCE only.
>>
>> No matter interrupt, s3 or poll, the jack->eld_valid will be synced
>> and the report_sync() will report jack state depeneds on both
>> AC_PINSENSE_PRESENCE and eld_valid
>>
>> Does it sound good?
> OK, I was confused -- I overlooked the fact that hda_jack.c updates
> the pin sense locally. And thanks for the suggestion, we're heading
> to the right direction.
>
> One thing that doesn't fit is introducing HDMI-specific things like
> eld_valid and check_eld. Basically this is about the falsely reported
> jack detection, after all. So, what we need is the correction of the
> incorrect jack detection -- or a way to override the value.
>
> Actually, for HDMI, there is no reason to read the pin sense at all.
> All we need is already provided by the HDMI codec driver (monitor
> present and eld valid).
>
> Then I can think of adding a new flag hda_jack_tbl.manual_pin_sense,
> for example, indicating that the pin sense is updated by the driver.
> That's a change something like below.
>
Looks like the hdmi_present_sense_via_verbs() still has some issue. At
the entry of this function, it calls snd_hda_pin_sense() and read the
eld_data depending on the return value, here it expects reading the pin
sense register, after applying your change, this function returns a
shadow value instead of a register value, if we set the pin_sense to 0,
then it is possible that the pin_get_eld() will not be called anymore.
And inspired by your change, maybe we just make this change, then it is
enough to fix the falsely report issue here.
@@ -1551,8 +1551,11 @@ static bool hdmi_present_sense_via_verbs(struct hdmi_spec_per_pin *per_pin,
ret = !repoll || !eld->monitor_present || eld->eld_valid;
jack = snd_hda_jack_tbl_get(codec, pin_nid);
- if (jack)
+ if (jack) {
jack->block_report = !ret;
+ jack->pin_sense = (eld->monitor_present && eld->eld_valid) ?
+ AC_PINSENSE_PRESENCE : 0;
+ }
because in the snd_hda_pin_sense(), the jack_dirty is set to 0, then we
change the jack->pin_sense, and in the report_sync() it will decide the
jack state according to the jack->pin_sense we changed.
Thanks,
Hui.
> thanks,
>
> Takashi
>
> --- a/sound/pci/hda/hda_jack.c
> +++ b/sound/pci/hda/hda_jack.c
> @@ -189,7 +189,7 @@ void snd_hda_jack_set_dirty_all(struct hda_codec *codec)
> int i;
>
> for (i = 0; i < codec->jacktbl.used; i++, jack++)
> - if (jack->nid)
> + if (jack->nid && !jack->manual_pin_sense)
> jack->jack_dirty = 1;
> }
> EXPORT_SYMBOL_GPL(snd_hda_jack_set_dirty_all);
> @@ -568,7 +568,8 @@ void snd_hda_jack_unsol_event(struct hda_codec *codec, unsigned int res)
> event = snd_hda_jack_tbl_get_from_tag(codec, tag);
> if (!event)
> return;
> - event->jack_dirty = 1;
> + if (!event->manual_pin_sense)
> + event->jack_dirty = 1;
>
> call_jack_callback(codec, res, event);
> snd_hda_jack_report_sync(codec);
> --- a/sound/pci/hda/hda_jack.h
> +++ b/sound/pci/hda/hda_jack.h
> @@ -40,6 +40,7 @@ struct hda_jack_tbl {
> unsigned int jack_dirty:1; /* needs to update? */
> unsigned int phantom_jack:1; /* a fixed, always present port? */
> unsigned int block_report:1; /* in a transitional state - do not report to userspace */
> + unsigned int manual_pin_sense:1; /* pin_sense is updated by codec driver */
> hda_nid_t gating_jack; /* valid when gating jack plugged */
> hda_nid_t gated_jack; /* gated is dependent on this jack */
> int type;
> --- a/sound/pci/hda/patch_hdmi.c
> +++ b/sound/pci/hda/patch_hdmi.c
> @@ -170,6 +170,7 @@ struct hdmi_spec {
>
> bool dyn_pin_out;
> bool dyn_pcm_assign;
> + bool use_pin_sense;
> /*
> * Non-generic VIA/NVIDIA specific
> */
> @@ -797,7 +798,6 @@ static void hdmi_intrinsic_event(struct hda_codec *codec, unsigned int res)
> jack = snd_hda_jack_tbl_get_from_tag(codec, tag);
> if (!jack)
> return;
> - jack->jack_dirty = 1;
>
> codec_dbg(codec,
> "HDMI hot plug event: Codec=%d Pin=%d Device=%d Inactive=%d Presence_Detect=%d ELD_Valid=%d\n",
> @@ -1551,8 +1551,11 @@ static bool hdmi_present_sense_via_verbs(struct hdmi_spec_per_pin *per_pin,
> ret = !repoll || !eld->monitor_present || eld->eld_valid;
>
> jack = snd_hda_jack_tbl_get(codec, pin_nid);
> - if (jack)
> + if (jack) {
> jack->block_report = !ret;
> + jack->pin_sense = (eld->monitor_present && eld->eld_valid) ?
> + AC_PINSENSE_PRESENCE : 0;
> + }
>
> mutex_unlock(&per_pin->lock);
> return ret;
> @@ -2163,6 +2166,7 @@ static int generic_hdmi_build_jack(struct hda_codec *codec, int pcm_idx)
> * align with dyn_pcm_assign mode
> */
> spec->pcm_rec[pcm_idx].jack = jack->jack;
> + jack->manual_pin_sense = !spec->use_pin_sense;
> return 0;
> }
>
> @@ -2972,6 +2976,7 @@ static int patch_simple_hdmi(struct hda_codec *codec,
> spec->multiout.dig_out_nid = cvt_nid;
> spec->num_cvts = 1;
> spec->num_pins = 1;
> + spec->use_pin_sense = true;
> per_pin = snd_array_new(&spec->pins);
> per_cvt = snd_array_new(&spec->cvts);
> if (!per_pin || !per_cvt) {
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel at alsa-project.org
> https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
More information about the Alsa-devel
mailing list