[alsa-devel] [PATCH v2] ALSA: hda - Update chmap tlv to report sink's capability

[Takashi Iwai]

On Mon, 04 Apr 2016 06:57:16 +0200,
Subhransu S. Prusty wrote:
> 
> On Fri, Apr 01, 2016 at 02:26:32PM +0200, Takashi Iwai wrote:
> > On Fri, 01 Apr 2016 12:18:51 +0200,
> > Subhransu S. Prusty wrote:
> > > -			dst += 2;
> > > -			size -= 8;
> > > -			count += 8;
> > > -			if (size < chs_bytes)
> > > -				return -ENOMEM;
> > > -			size -= chs_bytes;
> > > -			count += chs_bytes;
> > > -			chmap->ops.cea_alloc_to_tlv_chmap(chmap, cap,
> > > -						tlv_chmap, chs);
> > > -			if (copy_to_user(dst, tlv_chmap, chs_bytes))
> > > -				return -EFAULT;
> > > -			dst += chs;
> > > -		}
> > > -	}
> > > +
> > > +	if (size < 8)
> > > +		return -ENOMEM;
> > > +
> > > +	spk_alloc = chmap->ops.get_spk_alloc(chmap->hdac, pcm_idx);
> > > +	cap = get_cap_from_spk_alloc(spk_alloc);
> > > +	chs = cap->channels;
> > > +
> > > +	chs_bytes = chs * 4;
> > > +	type = chmap->ops.chmap_cea_alloc_validate_get_type(chmap, cap, chs);
> > > +	if (type < 0)
> > > +		return -ENODEV;
> > > +
> > > +	if (put_user(type, dst) ||
> > > +	    put_user(chs_bytes, dst + 1))
> > > +		return -EFAULT;
> > > +
> > > +	dst += 2;
> > > +	size -= 8;
> > > +	count += 8;
> > > +	if (size < chs_bytes)
> > > +		return -ENOMEM;
> > > +
> > > +	count += chs_bytes;
> > > +	chmap->ops.cea_alloc_to_tlv_chmap(chmap, cap, tlv_chmap, chs);
> > > +	if (copy_to_user(dst, tlv_chmap, chs_bytes))
> > > +		return -EFAULT;
> > > +
> > >  	if (put_user(count, tlv + 1))
> > >  		return -EFAULT;
> > > +
> > >  	return 0;
> > 
> > So, this change means that TLV returns only a single channel map
> > corresponding to the given speaker bits?  This doesn't sound right.
> > TLV should return the all possible channel maps the device are capable
> > of.  A device with four channels can drive also two channels streams,
> > too, for example.
> 
> Hi Takashi,
> 
> The device does report only the speaker allocation i.e. only the channels it
> support. So I think it is good to return only a single channel map that best
> matches the device's capability. As the device reports the channels it is
> capable of, user should be able to set any number of channels in any order
> looking at this map.

Well, excluding the entries means that it doesn't support any other
chmaps.  But the device supports less channels and you may configure
it so, e.g. 5.1 outputs can be 2.0 or 4.0 outputs.  Allowing this
choice and exposing the all capability (so that user-space may choose
*one of them*) are the exact purpose of TLV.


Takashi