[alsa-devel] [RFC PATCH 1/3] ALSA: hda - add flags and routines to get devices selection info for DP1.2 MST

[Lin, Mengdong]

> -----Original Message-----
> From: Takashi Iwai [mailto:tiwai at suse.de]
> Sent: Friday, August 23, 2013 8:22 PM
> > +/**
> > + * snd_hda_get_devices - copy device list without cache
> > + * @codec: the HDA codec
> > + * @nid: NID of the pin to parse
> > + * @dev_list: device list array
> > + * @max_devices: max. number of devices to store
> > + *
> > + * Copy the device list. This info is dynamic and so not cached.
> > + * Currently called only from hda_proc.c, so not exported.
> > + */
> > +int snd_hda_get_devices(struct hda_codec *codec, hda_nid_t nid,
> > +			union hda_devlist_entry *dev_list, int max_devices) {
> > +	unsigned int parm;
> > +	int i, dev_len, devices;
> > +
> > +	parm = get_num_devices(codec, nid);
> > +	if (!parm)	/* not multi-stream capable */
> > +		return 0;
> > +
> > +	dev_len = parm + 1;
> 
> Check dev_len with max_devices here.

Will fix it. Sorry, I forgot to check.

> 
> > +	devices = 0;
> > +
> > +	while (devices < dev_len) {
> > +		parm = snd_hda_codec_read(codec, nid, 0,
> > +					  AC_VERB_GET_DEVICE_LIST, devices);
> > +		if (parm == -1 && codec->bus->rirb_error)
> > +			break;
> > +
> > +		for (i = 0; i < 8; i++) {
> > +			dev_list[devices].value = parm & 0x7;
> > +			parm >>= 4;
> > +			devices++;
> > +			if (devices >= dev_len)
> > +				break;
> > +		}
> > +	}
> > +	return devices;
> > +}
> > +
> >  /**
> >   * snd_hda_queue_unsol_event - add an unsolicited event to queue
> >   * @bus: the BUS
> > diff --git a/sound/pci/hda/hda_codec.h b/sound/pci/hda/hda_codec.h
> > index 701c2e0..736b55a 100644
> > --- a/sound/pci/hda/hda_codec.h
> > +++ b/sound/pci/hda/hda_codec.h
> > @@ -94,6 +94,8 @@ enum {
> >  #define AC_VERB_GET_HDMI_DIP_XMIT		0x0f32
> >  #define AC_VERB_GET_HDMI_CP_CTRL		0x0f33
> >  #define AC_VERB_GET_HDMI_CHAN_SLOT		0x0f34
> > +#define AC_VERB_GET_DEVICE_SEL			0xf35
> > +#define AC_VERB_GET_DEVICE_LIST			0xf36
> >
> >  /*
> >   * SET verbs
> > @@ -133,6 +135,7 @@ enum {
> >  #define AC_VERB_SET_HDMI_DIP_XMIT		0x732
> >  #define AC_VERB_SET_HDMI_CP_CTRL		0x733
> >  #define AC_VERB_SET_HDMI_CHAN_SLOT		0x734
> > +#define AC_VERB_SET_DEVICE_SEL			0x735
> >
> >  /*
> >   * Parameter IDs
> > @@ -154,6 +157,7 @@ enum {
> >  #define AC_PAR_GPIO_CAP			0x11
> >  #define AC_PAR_AMP_OUT_CAP		0x12
> >  #define AC_PAR_VOL_KNB_CAP		0x13
> > +#define AC_PAR_DEVLIST_LEN		0x15
> >  #define AC_PAR_HDMI_LPCM_CAP		0x20
> >
> >  /*
> > @@ -795,6 +799,19 @@ struct hda_pcm {
> >  	bool own_chmap;		/* codec driver provides own channel maps */
> >  };
> >
> > +/* display pin's device list entry */ union  hda_devlist_entry {
> > +	unsigned int value;
> > +	struct  {
> > +		unsigned int present_detect:1;
> > +		unsigned int eld_valid:1;
> > +		unsigned int inactive:1;
> > +	} bits;
> > +};
> 
> The bit fields are not really portable.
> Please use the explicit bit operation instead.

You're right. It's compiler-dependent. I'll revise and verify the patch next Monday.

Thanks for the review!
Mengdong