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

From: Takashi Iwai <tiwai@suse.de>
To: "Subhransu S. Prusty" <subhransu.s.prusty@intel.com>
Cc: alsa-devel@alsa-project.org, patches.audio@intel.com,
	lgirdwood@gmail.com, dri-devel@lists.freedesktop.org,
	broonie@kernel.org, Daniel Vetter <daniel.vetter@intel.com>,
	Vinod Koul <vinod.koul@intel.com>
Subject: Re: [PATCH v2 05/14] ALSA: pcm: Add DRM helper to set constraint for format
Date: Sat, 05 Dec 2015 09:12:34 +0100	[thread overview]
Message-ID: <s5h1tb148lp.wl-tiwai@suse.de> (raw)
In-Reply-To: <20151205112655.GA2441@subhransu-desktop>

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.

Takashi
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel