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

From: Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
To: Jyri Sarha <jsarha-l0cyMroinI0@public.gmane.org>
Cc: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org,
	airlied-cv59FeDIM0c@public.gmane.org,
	linux-omap-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	bcousson-rdvid1DuHRBWk0Htik3J/w@public.gmane.org,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw@public.gmane.org,
	tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org,
	liam.r.girdwood-VuQAYsv1563Yd54FQh9/CA@public.gmane.org,
	peter.ujfalusi-l0cyMroinI0@public.gmane.org,
	tomi.valkeinen-l0cyMroinI0@public.gmane.org,
	moinejf-GANU6spQydw@public.gmane.org,
	rmk+kernel-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org
Subject: Re: [PATCH RFC v4 3/8] ASoC: hdmi-codec: Add hdmi-codec for external HDMI-encoders
Date: Sat, 19 Sep 2015 10:54:51 -0700	[thread overview]
Message-ID: <20150919175450.GL30445@sirena.org.uk> (raw)
In-Reply-To: <6fe9ab5dff972e6f228259d6818f3f481a11577d.1442572860.git.jsarha-l0cyMroinI0@public.gmane.org>

[-- Attachment #1: Type: text/plain, Size: 1331 bytes --]

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?

We can probably skip more specific constraint stuff I guess on the basis
that things tend to support the full range of HDMI functionality, or at
least that can be added later?

Otherwise this looks mostly OK on a first pass, thanks for working on
it.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]