On Tue, 08 Mar 2016 07:54:44 +0100, Yang, Libin wrote:
Hi Takashi,
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Tuesday, March 08, 2016 12:46 AM To: libin.yang@linux.intel.com Cc: alsa-devel@alsa-project.org; Lin, Mengdong; Yang, Libin Subject: Re: [alsa-devel] [PATCH 2/4] ALSA: hda - add DP mst verb support
On Mon, 07 Mar 2016 15:57:44 +0100, libin.yang@linux.intel.com wrote:
+int snd_hda_set_dev_select(struct hda_codec *codec, hda_nid_t nid,
int dev_id)
+{
- int ret, dev;
- unsigned long timeout;
- /* not support dp_mst will always return 0, using first dev_entry
*/
- if (!codec->dp_mst)
return 0;- /* If Device List Length is 0, the pin is not multi stream capable.
*/- if ((snd_hda_get_num_devices(codec, nid) + 1) == 0)
return 0;I don't understand what this code is for. Why +1?
Sorry, it should be if ((snd_hda_get_num_devices(codec, nid)) == 0). 0 means not MST capable. Thanks for point it out.
- /* Behavior of setting index being equal to or greater than
* Device List Length is not predictable*/- if ((snd_hda_get_num_devices(codec, nid) + 1) <= dev_id)
return 0;Ditto. And, calling the same function twice?
(snd_hda_get_num_devices(codec, nid) + 1) means the device entry number. If the (snd_hda_get_num_devices(codec, nid) + 1) == 3, it means there are only 3 device entries. And dev_id being 0, 1, 2 is legal. It dev_id is 3 or greater, it is illegal.
So, shouldn't it be like below? if (dev_id >= num_devices) return 0;
Also my point is that you don't need to call snd_hda_get_num_devices() twice. It can be stored locally.
Takashi