On Tue, 04 Feb 2020 08:46:17 +0100, Takashi Iwai wrote: > > On Tue, 04 Feb 2020 06:08:19 +0100, > Nikhil Mahale wrote: > > > > On 2/3/20 4:10 PM, Takashi Iwai wrote: > > > External email: Use caution opening links or attachments > > > > > > > > > On Mon, 03 Feb 2020 11:06:17 +0100, > > > Nikhil Mahale wrote: > > >> > > >> If dyn_pcm_assign is set, different jack objects are being created > > >> for pcm and pins. > > >> > > >> If dyn_pcm_assign is set, generic_hdmi_build_jack() calls into > > >> add_hdmi_jack_kctl() to create and track separate jack object for > > >> pcm. Like sync_eld_via_acomp(), hdmi_present_sense_via_verbs() also > > >> need to report status change of the pcm jack. > > >> > > >> Rename pin_idx_to_jack() to pin_idx_to_pcm_jack(). The code to > > >> report status change of pcm jack, move it to update_eld() which is > > >> common for acomp and !acomp code paths. > > > > > > Thanks, that's the cause of the regression. > > > However, this needs yet more careful handling, I'm afraid. > > > > > > - hdmi_present_sense_via_verbs() may return true, and its callers > > > (both check_presence_and_report() and hdmi_repoll_eld()) do call > > > snd_hda_jack_report_sync() again. > > > > > > - For non-dyn_pcm_assign case, we shouldn't call jack report there, > > > but rather simply return true for calling report sync. > > > > > > - There is another workaround to block the jack report in > > > hdmi_present_sense_via_verbs() which is applied after update_eld(), > > > and this will be ignored. > > > > > > So, I guess that we need the conditional application of the individual > > > snd_jack_report() only for spec->dyn_pcm_assign==true, and assure that > > > hdmi_present() returns false. > > Yeah, you are right. But I don't think we should return false from > > hdmi_present(). > > > > Before dyn_pcm_assign support for non-acomp drivers: > > 1) pcm and pin plug detection were controlled by same jack object, and > > 2) change in plug status was reported from snd_hda_jack_report_sync(). > > > > If dyn_pcm_assign support is enabled for non-acomp drivers, then pcm > > and pin detection are controlled by different jack object. Now, report > > for plug status change of both jack object, requires to be in sync. > > That's my concern. Basically we're seeing two different jacks for a > single hotplug. OTOH, from the user-space POV, it must be one jack > that should report back. IOW, with dyn_pcm_assign, you should ignore > the jack triggered from the unsol handler but report back only for the > jack that is assigned to the PCM. Scratch this. The code is so complex and fuzzing me. Oh well. For dyn_pcm_assign, the snd_hda_jack stuff is used only for the unsol event notification but it's not bound with snd_jack object. Hence snd_hda_jack_report_sync() is harmless -- but it's also useless for dyn_pcm_assign, too. So basically the logic of your patch 4 is OK. But it's still misleading in a few points. - The snd_hda_jack state was already updated via snd_hda_jack_pin_sense() call at the beginning of hdmi_present_sense_via_verbs() before calling snd_hda_jack_report_sync(). That implies that what's jack->pinse_sense assignment at the end of this function does is to override the jack->pin_sense value that was already updated. - snd_hda_jack_report_sync() is superfluous for dyn_pcm_assign case. The jack update was already performed, as mentioned in the above, and hda_jack->jack is NULL for dyn_pcm_assign. Moreover, snd_hda_jack_report_sync() can be replaced with the individual snd_jack_report() call even for non-dyn_pcm_assign case, too. The difference is only how to get snd_jack object; for dyn_pcm_assign, it's pin_idx_to_jack() while non-dyn_pcm_assign, it's hda_jack->jack. I guess the call of snd_jack_report() can be unified in a helper (that can be called from sync_eld_via_acomp() too). thanks, Takashi _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
