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

[Lin, Mengdong]

Hi Takashi,

The patches are revised as you suggested and submitted. Please have a review.

There is a new patch to add 'Device Entry' and 'Inactive' fields to the unsolicited response for a display audio codec.

Thanks
Mengdong

> -----Original Message-----
> From: alsa-devel-bounces at alsa-project.org
> [mailto:alsa-devel-bounces at alsa-project.org] On Behalf Of Lin, Mengdong
> Sent: Friday, August 23, 2013 11:25 PM
> To: Takashi Iwai
> Cc: alsa-devel at alsa-project.org
> Subject: Re: [alsa-devel] [RFC PATCH 1/3] ALSA: hda - add flags and routines to
> get devices selection info for DP1.2 MST
> 
> > -----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
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel at alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel