Re: [alsa-devel] [PATCH v2 2/5] ALSA: hda - Add DP-MST jack support

[Nikhil Mahale <nmahale@nvidia.com>]

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;
    }
    [...]
}

Thanks,
Nikhil Mahale

> thanks,
> 
> Takashi
> 

-----------------------------------------------------------------------------------
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.
-----------------------------------------------------------------------------------
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel