On Tue, 19 Nov 2019 05:45:26 +0100,
Nikhil Mahale wrote:
>
> On 11/15/19 6:42 PM, Takashi Iwai wrote:
> > On Fri, 15 Nov 2019 10:56:03 +0100,
> > Nikhil Mahale wrote:
> >>
> >> static struct hda_jack_tbl *
> >> -snd_hda_jack_tbl_new(struct hda_codec *codec, hda_nid_t nid)
> >> +snd_hda_jack_tbl_new(struct hda_codec *codec, hda_nid_t nid, int dev_id)
> >> {
> >> - struct hda_jack_tbl *jack = snd_hda_jack_tbl_get(codec, nid);
> >> + struct hda_jack_tbl *jack =
> >> + snd_hda_jack_tbl_get_mst(codec, nid, dev_id);
> >> + struct hda_jack_tbl *existing_nid_jack =
> >> + any_jack_tbl_get_from_nid(codec, nid);
> >> +
> >> + WARN_ON(dev_id != 0 && !codec->dp_mst);
> >> +
> >> if (jack)
> >> return jack;
> >> jack = snd_array_new(&codec->jacktbl);
> >> if (!jack)
> >> return NULL;
> >> jack->nid = nid;
> >> + jack->dev_id = dev_id;
> >> jack->jack_dirty = 1;
> >> - jack->tag = codec->jacktbl.used;
> >> + if (!existing_nid_jack)
> >> + jack->tag = codec->jacktbl.used;
> >> + else
> >> + jack->tag = existing_nid_jack->tag;
> >> +
> >> return jack;
> >> }
> >
> > In this logic, we assign the same tag to multiple jack objects, and
> > this will lead to the multiple calls of SET_UNSOLICITED_ENABLE on the
> > same pin. But this should be only once at parsing, and the rest
> > resume init will be done from the codec regmap cache, so it's not too
> > bad, I guess.
> >
> > Alternatively we can set 0 to jack->tag when existing_nid_jack!=NULL,
> > too, and skip SET_UNSOLICITED_ENABLE for tag==0. But this will make
> > *_get_from_tag() won't work as you intended, hence the step will be
> > - get the jack once via snd_hda_jack_tbl_get_from_tag(tag)
> > - get the jack again with snd_hda_jack_tbl_get_mst(jack->nid, dev_id)
> > But it's more complex, and not better, either.
>
> I am bit skeptical over this idea. One can build hypothetical case like - you create 2 jack objects, first one for (nid=1, dev_id=0) with jack->tag!=0 and second one for (nid=1, dev_id=1) with jack->tag==0. If you want free first jack object and continue working with second one then that would create problem because you lose tag with destruction of first object?
>
> If objective is not to make multiple calls of SET_UNSOLICITED_ENABLE on the same pin, then just like jack->tag we can inherit jack->jack_detect from existing_nid_jack. That can make sure snd_hda_jack_detect_enable_callback_mst() not to do multiple calls of SET_UNSOLICITED_ENABLE on the same pin, right?
>
> I am suggesting code change like -
>
> ...
> snd_hda_jack_tbl_new(...)
> {
> [...]
> if (!existing_nid_jack) {
> jack->tag = codec->jacktbl.used;
> } else {
> jack->tag = existing_nid_jack->tag;
> jack->jack_detect = existing_nid_jack->jack_detect;
> }
> [...]
> }
Yes, this looks like a good idea.
thanks,
Takashi
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
