Re: [PATCH RFC v4 3/8] ASoC: hdmi-codec: Add hdmi-codec for external HDMI-encoders

From: Jyri Sarha <jsarha@ti.com>
To: Russell King - ARM Linux <linux@arm.linux.org.uk>,
	Mark Brown <broonie@kernel.org>
Cc: devicetree@vger.kernel.org, alsa-devel@alsa-project.org,
	peter.ujfalusi@ti.com, dri-devel@lists.freedesktop.org,
	liam.r.girdwood@linux.intel.com, tony@atomide.com,
	tomi.valkeinen@ti.com, bcousson@baylibre.com,
	linux-omap@vger.kernel.org
Subject: Re: [PATCH RFC v4 3/8] ASoC: hdmi-codec: Add hdmi-codec for external HDMI-encoders
Date: Mon, 21 Sep 2015 16:41:20 +0300	[thread overview]
Message-ID: <56000900.3070404@ti.com> (raw)
In-Reply-To: <20150921093107.GF21098@n2100.arm.linux.org.uk>

On 09/21/15 12:31, Russell King - ARM Linux wrote:
> On Sat, Sep 19, 2015 at 10:54:51AM -0700, Mark Brown wrote:
>> On Fri, Sep 18, 2015 at 02:06:40PM +0300, Jyri Sarha wrote:
>>> +#define SPDIF_FORMATS	(SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S16_BE |\
>>> +			 SNDRV_PCM_FMTBIT_S20_3LE | SNDRV_PCM_FMTBIT_S20_3BE |\
>>> +			 SNDRV_PCM_FMTBIT_S24_3LE | SNDRV_PCM_FMTBIT_S24_3BE |\
>>> +			 SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S24_BE)
>>> +
>>> +#define I2S_FORMATS	(SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S16_BE |\
>>> +			 SNDRV_PCM_FMTBIT_S18_3LE | SNDRV_PCM_FMTBIT_S18_3BE |\
>>> +			 SNDRV_PCM_FMTBIT_S20_3LE | SNDRV_PCM_FMTBIT_S20_3BE |\
>>> +			 SNDRV_PCM_FMTBIT_S24_3LE | SNDRV_PCM_FMTBIT_S24_3BE |\
>>> +			 SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S24_BE |\
>>> +			 SNDRV_PCM_FMTBIT_S32_LE | SNDRV_PCM_FMTBIT_S32_BE)
>>
>> I'm not sure how abstracted this I2S and S/PDIF DAI business is - it
>> doesn't feel like something that's going to be a property of all HDMI
>> devices, and the specific sets of formats above cause me to raise a bit
>> of an eyebrow.  Should this be an interface where the HDMI chip
>> registers multiple interfaces if it has them instead of automatically
>> getting this split as is?
>
> The inclusion of the 32-bit formats does raise an eyebrow here too.
> Audio transmission across the HDMI link is S/PDIF, supporting up to
> 24-bit uncompressed audio (aka L-PCM).
>
> The device may accept 32 bit I2S, but it would have to be truncated to
> 24 bit before transmitting it to the sink.  This should be mentioned
> somewhere.
>

There is ".sig_bits = 24" in hdmi_i2s_dai, but I can add an explicit 
comment about it.

We needed 32bit format in practice until Peter got 24_LE properly 
working with McASP. I just thought the may still be some platforms out 
there that can not produce 24bit i2s samples for some reason, but work 
just fine with 32bit samples. Those platforms would be limited to less 
than 24bit precision without 32bit formats. But as said, it is not 
critical to us any more and I can drop the 32bit formats as well.

BR,
Jyri

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