[alsa-devel] [PATCH v2 05/14] ALSA: pcm: Add DRM helper to set constraint for format

[Subhransu S. Prusty]

On Sat, Dec 05, 2015 at 09:12:34AM +0100, Takashi Iwai wrote:
> On Sat, 05 Dec 2015 12:27:03 +0100,
> Subhransu S. Prusty wrote:
> > 
> > On Fri, Dec 04, 2015 at 06:59:19AM +0100, Takashi Iwai wrote:
> > > On Fri, 04 Dec 2015 12:08:26 +0100,
> > > Subhransu S. Prusty wrote:
> > > > 
> > > > On Thu, Dec 03, 2015 at 04:57:14PM +0100, Takashi Iwai wrote:
> > > > > On Thu, 03 Dec 2015 22:08:53 +0100,
> > > > > Subhransu S. Prusty wrote:
> > > > > > 
> > > > > > Setting the constraint format based on ELD was missing bit in
> > > > > > the sound/core pcm drm. Added with this patch.
> > > > > 
> > > > > No, you can't define these here.  The format really depends on the
> > > > > hardware, while the rate and the channels are independent.
> > > > 
> > > > Probably then I will move this definition to driver.
> > > > 
> > > > > How do you know it's little-endian?  And why it must be S24_LE for
> > > > > 24bit, not S32_LE?
> > > > 
> > > > Regarding the little-endian, In the driver I think I should set the
> > > > constraint for both LE and BE. And the platform as it only supports LE alone
> > > > it will set the constraint accordingly and edianness will be taken care of.
> > > > 
> > > > Regarding the sample size, from short audio descriptor, the samples can be
> > > > one of 16/20/24 bit. I could use the format bits for 16 and 24 bits but
> > > > don't know which format bits macro is suitable for 20bits. So kept it as
> > > > S32_LE for 20bits. Should I fix the format bits for 20bits to use S24?
> > > 
> > > No you seem misunderstanding the concept...
> > 
> > Sorry about that. 
> > 
> > I re-read the spec, it doesn't mention the container size for the samples.
> > Assuming the container will be 32 bits, then I think we should use S24_LE
> > for both 20 and 24 bits.
> 
> You can't limit easily from the supported bits.  In theory, all
> formats that may fit with the given bit should be set.  For example, 
> for a 20bit sample, S24, U24, S32, U32, S24_3, U24_3, S20_3, etc would
> match, and even for both endianess.
> 
> The format type doesn't specify only the *max* bit it can pack, but
> also only the position of bits to be packed.  For example, S24 packs
> max 24 bits in the lower 24 bits in a 32bit container.  And, S24_3
> packs max 24 bits in a 24bit container.  Most of Intel chips takes the
> upper bits in a container, so usually they need S32 or S16 no matter
> how many bits are used.

Thanks for the quick reply.

I am thinking to set the format as S16 for 16bits and S32 for 20/24bits.

But then we can set S32 for everything as well including 16bits.

What do you recommend?


> 
> 
> Takashi

--