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