[alsa-devel] [PATCH] ALSA: hda - add DP mst verb support

[Takashi Iwai]

On Thu, 14 Jan 2016 07:09:42 +0100,
Yang, Libin wrote:
> 
> 
> > -----Original Message-----
> > From: Takashi Iwai [mailto:tiwai at suse.de]
> > Sent: Wednesday, January 13, 2016 7:25 PM
> > To: libin.yang at linux.intel.com
> > Cc: alsa-devel at alsa-project.org; Lin, Mengdong; Yang, Libin
> > Subject: Re: [PATCH] ALSA: hda - add DP mst verb support
> > 
> > On Wed, 13 Jan 2016 12:09:49 +0100,
> > libin.yang at linux.intel.com wrote:
> > >
> > > From: Libin Yang <libin.yang at linux.intel.com>
> > >
> > > Add snd_hda_get_dev_select() and snd_hda_set_dev_select()
> > functions
> > > for DP MST audio support.
> > >
> > > Signed-off-by: Libin Yang <libin.yang at linux.intel.com>
> > > ---
> > >  sound/pci/hda/hda_codec.c | 54
> > +++++++++++++++++++++++++++++++++++++++++++++++
> > >  sound/pci/hda/hda_codec.h |  2 ++
> > >  2 files changed, 56 insertions(+)
> > >
> > > diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
> > > index 8374188..95baaba 100644
> > > --- a/sound/pci/hda/hda_codec.c
> > > +++ b/sound/pci/hda/hda_codec.c
> > > @@ -368,6 +368,60 @@ int snd_hda_get_devices(struct hda_codec
> > *codec, hda_nid_t nid,
> > >  	return devices;
> > >  }
> > >
> > > +/**
> > > + * snd_hda_get_dev_select - get device entry select on the pin
> > > + * @codec: the HDA codec
> > > + * @nid: NID of the pin to get device entry select
> > > + *
> > > + * Get the devcie entry select on the pin. Return the device entry
> > > + * id selected on the pin. Return 0 means the first device entry
> > > + * is selected or MST is not supported.
> > > + */
> > > +int snd_hda_get_dev_select(struct hda_codec *codec, hda_nid_t nid)
> > > +{
> > > +	/* not support dp_mst will always return 0, using first dev_entry
> > */
> > > +	if (!codec->dp_mst)
> > > +		return 0;
> > > +
> > > +	return snd_hda_codec_read(codec, nid, 0,
> > AC_VERB_GET_DEVICE_SEL, 0);
> > > +}
> > > +EXPORT_SYMBOL_GPL(snd_hda_get_dev_select);
> > > +
> > > +/**
> > > + * snd_hda_set_dev_select - set device entry select on the pin
> > > + * @codec: the HDA codec
> > > + * @nid: NID of the pin to set device entry select
> > > + * @dev_id: device entry id to be set
> > > + *
> > > + * Set the device entry select on the pin nid.
> > 
> > The return code isn't clearly defined.
> > 
> > Other than that, it looks OK.  
> 
> What about return dev_id returned from snd_hda_get_dev_select()

... plus a negative error code.

> Or return true if it sets dev_id successfully and false for failure. 

I don't mind in either way.

> > But I prefer merging this rather
> > together with its caller (unless a complex dependency exists).
> 
> OK. I will submit the patch later.

OK, thanks.


Takashi

> Regards,
> Libin
> 
> > 
> > 
> > thanks,
> > 
> > Takashi
> > 
> > > + */
> > > +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;
> > > +
> > > +	timeout = jiffies + msecs_to_jiffies(500);
> > > +
> > > +	/* make sure the device entry selection takes effect */
> > > +	do {
> > > +		ret = snd_hda_codec_write(codec, nid, 0,
> > > +			AC_VERB_SET_DEVICE_SEL, dev_id);
> > > +		udelay(20);
> > > +
> > > +		dev = snd_hda_get_dev_select(codec, nid);
> > > +		if (dev == dev_id)
> > > +			break;
> > > +		msleep(20);
> > > +	} while (time_before(jiffies, timeout));
> > > +
> > > +	return ret;
> > > +}
> > > +EXPORT_SYMBOL_GPL(snd_hda_set_dev_select);
> > > +
> > >  /*
> > >   * read widget caps for each widget and store in cache
> > >   */
> > > diff --git a/sound/pci/hda/hda_codec.h b/sound/pci/hda/hda_codec.h
> > > index 373fcad..22bcad6 100644
> > > --- a/sound/pci/hda/hda_codec.h
> > > +++ b/sound/pci/hda/hda_codec.h
> > > @@ -349,6 +349,8 @@ int snd_hda_get_conn_index(struct
> > hda_codec *codec, hda_nid_t mux,
> > >  			   hda_nid_t nid, int recursive);
> > >  int snd_hda_get_devices(struct hda_codec *codec, hda_nid_t nid,
> > >  			u8 *dev_list, int max_devices);
> > > +int snd_hda_get_dev_select(struct hda_codec *codec, hda_nid_t nid);
> > > +int snd_hda_set_dev_select(struct hda_codec *codec, hda_nid_t nid,
> > int dev_id);
> > >
> > >  struct hda_verb {
> > >  	hda_nid_t nid;
> > > --
> > > 1.9.1
> > >
>