* [PATCH v2 0/2] ASoC: hdmi-codec: Allow driver to restrict sample size @ 2017-07-31 22:49 ` srinivas.kandagatla 0 siblings, 0 replies; 16+ messages in thread From: srinivas.kandagatla @ 2017-07-31 22:49 UTC (permalink / raw) To: Jyri Sarha, Mark Brown, alsa-devel Cc: Archit Taneja, Andrzej Hajda, David Airlie, Jaroslav Kysela, Takashi Iwai, Liam Girdwood, dri-devel, linux-kernel, Srinivas Kandagatla From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> This patch adds option to allow hdmi-codec drivers to restrict the audio sample size based on the values that are suppored in hardware. First patch is adding that formats option to hdmi_codec_pdata and second patch is a fix in adv7511 codec driver which only supports 16 and 24 bit samples. Changes since v1: - Added Review by Jyri - Updated comment in code suggested by Jyri Srinivas Kandagatla (2): ASoC: hdmi-codec: Allow drivers to restrict sample sizes. drm/bridge: adv7511: restrict audio sample sizes drivers/gpu/drm/bridge/adv7511/adv7511_audio.c | 2 ++ include/sound/hdmi-codec.h | 1 + sound/soc/codecs/hdmi-codec.c | 8 +++++--- 3 files changed, 8 insertions(+), 3 deletions(-) -- 2.11.0 ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 0/2] ASoC: hdmi-codec: Allow driver to restrict sample size @ 2017-07-31 22:49 ` srinivas.kandagatla 0 siblings, 0 replies; 16+ messages in thread From: srinivas.kandagatla @ 2017-07-31 22:49 UTC (permalink / raw) To: Jyri Sarha, Mark Brown, alsa-devel Cc: linux-kernel, Liam Girdwood, dri-devel, Takashi Iwai, Srinivas Kandagatla, Jaroslav Kysela From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> This patch adds option to allow hdmi-codec drivers to restrict the audio sample size based on the values that are suppored in hardware. First patch is adding that formats option to hdmi_codec_pdata and second patch is a fix in adv7511 codec driver which only supports 16 and 24 bit samples. Changes since v1: - Added Review by Jyri - Updated comment in code suggested by Jyri Srinivas Kandagatla (2): ASoC: hdmi-codec: Allow drivers to restrict sample sizes. drm/bridge: adv7511: restrict audio sample sizes drivers/gpu/drm/bridge/adv7511/adv7511_audio.c | 2 ++ include/sound/hdmi-codec.h | 1 + sound/soc/codecs/hdmi-codec.c | 8 +++++--- 3 files changed, 8 insertions(+), 3 deletions(-) -- 2.11.0 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 1/2] ASoC: hdmi-codec: Allow drivers to restrict sample sizes. 2017-07-31 22:49 ` srinivas.kandagatla @ 2017-07-31 22:49 ` srinivas.kandagatla -1 siblings, 0 replies; 16+ messages in thread From: srinivas.kandagatla @ 2017-07-31 22:49 UTC (permalink / raw) To: Jyri Sarha, Mark Brown, alsa-devel Cc: Archit Taneja, Andrzej Hajda, David Airlie, Jaroslav Kysela, Takashi Iwai, Liam Girdwood, dri-devel, linux-kernel, Srinivas Kandagatla From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> Currently hdmi client drivers does have means to limit the sample sizes that it can only support. Having formats parameter option would solve this. This issue was noticed on DB410c board when adv7511 hdmi codec driver failed to play a 32 bits audio samples, as it does not support them. Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> Reviewed-by: Jyri Sarha <jsarha@ti.com> --- include/sound/hdmi-codec.h | 1 + sound/soc/codecs/hdmi-codec.c | 8 +++++--- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/include/sound/hdmi-codec.h b/include/sound/hdmi-codec.h index 9483c55f871b..89fc4cce5785 100644 --- a/include/sound/hdmi-codec.h +++ b/include/sound/hdmi-codec.h @@ -104,6 +104,7 @@ struct hdmi_codec_pdata { uint i2s:1; uint spdif:1; int max_i2s_channels; + u64 formats; void *data; }; diff --git a/sound/soc/codecs/hdmi-codec.c b/sound/soc/codecs/hdmi-codec.c index 22ed0dc88f0a..a7b4d6757ff1 100644 --- a/sound/soc/codecs/hdmi-codec.c +++ b/sound/soc/codecs/hdmi-codec.c @@ -653,9 +653,8 @@ static const struct snd_soc_dai_ops hdmi_dai_ops = { * This list is only for formats allowed on the I2S bus. So there is * some formats listed that are not supported by HDMI interface. For * instance allowing the 32-bit formats enables 24-precision with CPU - * DAIs that do not support 24-bit formats. If the extra formats cause - * problems, we should add the video side driver an option to disable - * them. + * DAIs that do not support 24-bit formats. Driver can either use this + * list or specify supported formats in formats field of hdmi_codec_pdata. */ #define I2S_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S16_BE |\ SNDRV_PCM_FMTBIT_S20_3LE | SNDRV_PCM_FMTBIT_S20_3BE |\ @@ -780,6 +779,9 @@ static int hdmi_codec_probe(struct platform_device *pdev) hcp->daidrv[i] = hdmi_i2s_dai; hcp->daidrv[i].playback.channels_max = hcd->max_i2s_channels; + + if (hcd->formats) + hcp->daidrv[i].playback.formats = hcd->formats; i++; } -- 2.11.0 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2 1/2] ASoC: hdmi-codec: Allow drivers to restrict sample sizes. @ 2017-07-31 22:49 ` srinivas.kandagatla 0 siblings, 0 replies; 16+ messages in thread From: srinivas.kandagatla @ 2017-07-31 22:49 UTC (permalink / raw) To: Jyri Sarha, Mark Brown, alsa-devel Cc: linux-kernel, Liam Girdwood, dri-devel, Takashi Iwai, Srinivas Kandagatla, Jaroslav Kysela From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> Currently hdmi client drivers does have means to limit the sample sizes that it can only support. Having formats parameter option would solve this. This issue was noticed on DB410c board when adv7511 hdmi codec driver failed to play a 32 bits audio samples, as it does not support them. Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> Reviewed-by: Jyri Sarha <jsarha@ti.com> --- include/sound/hdmi-codec.h | 1 + sound/soc/codecs/hdmi-codec.c | 8 +++++--- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/include/sound/hdmi-codec.h b/include/sound/hdmi-codec.h index 9483c55f871b..89fc4cce5785 100644 --- a/include/sound/hdmi-codec.h +++ b/include/sound/hdmi-codec.h @@ -104,6 +104,7 @@ struct hdmi_codec_pdata { uint i2s:1; uint spdif:1; int max_i2s_channels; + u64 formats; void *data; }; diff --git a/sound/soc/codecs/hdmi-codec.c b/sound/soc/codecs/hdmi-codec.c index 22ed0dc88f0a..a7b4d6757ff1 100644 --- a/sound/soc/codecs/hdmi-codec.c +++ b/sound/soc/codecs/hdmi-codec.c @@ -653,9 +653,8 @@ static const struct snd_soc_dai_ops hdmi_dai_ops = { * This list is only for formats allowed on the I2S bus. So there is * some formats listed that are not supported by HDMI interface. For * instance allowing the 32-bit formats enables 24-precision with CPU - * DAIs that do not support 24-bit formats. If the extra formats cause - * problems, we should add the video side driver an option to disable - * them. + * DAIs that do not support 24-bit formats. Driver can either use this + * list or specify supported formats in formats field of hdmi_codec_pdata. */ #define I2S_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S16_BE |\ SNDRV_PCM_FMTBIT_S20_3LE | SNDRV_PCM_FMTBIT_S20_3BE |\ @@ -780,6 +779,9 @@ static int hdmi_codec_probe(struct platform_device *pdev) hcp->daidrv[i] = hdmi_i2s_dai; hcp->daidrv[i].playback.channels_max = hcd->max_i2s_channels; + + if (hcd->formats) + hcp->daidrv[i].playback.formats = hcd->formats; i++; } -- 2.11.0 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2 2/2] drm/bridge: adv7511: restrict audio sample sizes 2017-07-31 22:49 ` srinivas.kandagatla @ 2017-07-31 22:49 ` srinivas.kandagatla -1 siblings, 0 replies; 16+ messages in thread From: srinivas.kandagatla @ 2017-07-31 22:49 UTC (permalink / raw) To: Jyri Sarha, Mark Brown, alsa-devel Cc: Archit Taneja, Andrzej Hajda, David Airlie, Jaroslav Kysela, Takashi Iwai, Liam Girdwood, dri-devel, linux-kernel, Srinivas Kandagatla From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> ADV7533 only supports audio samples word width from 16-24 bits. This patch restricts the audio sample sizes to the supported ones, so that sound card does not report wrong list of supported hwparms. Without this patch aplay would fail when playing a 32 bit audio. Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> --- drivers/gpu/drm/bridge/adv7511/adv7511_audio.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c b/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c index 67469c26bae8..d01d0aa0eef7 100644 --- a/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c @@ -214,6 +214,8 @@ static struct hdmi_codec_pdata codec_data = { .ops = &adv7511_codec_ops, .max_i2s_channels = 2, .i2s = 1, + .formats = (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S16_BE | + SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S24_BE), }; int adv7511_audio_init(struct device *dev, struct adv7511 *adv7511) -- 2.11.0 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2 2/2] drm/bridge: adv7511: restrict audio sample sizes @ 2017-07-31 22:49 ` srinivas.kandagatla 0 siblings, 0 replies; 16+ messages in thread From: srinivas.kandagatla @ 2017-07-31 22:49 UTC (permalink / raw) To: Jyri Sarha, Mark Brown, alsa-devel Cc: linux-kernel, Liam Girdwood, dri-devel, Takashi Iwai, Srinivas Kandagatla, Jaroslav Kysela From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> ADV7533 only supports audio samples word width from 16-24 bits. This patch restricts the audio sample sizes to the supported ones, so that sound card does not report wrong list of supported hwparms. Without this patch aplay would fail when playing a 32 bit audio. Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> --- drivers/gpu/drm/bridge/adv7511/adv7511_audio.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c b/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c index 67469c26bae8..d01d0aa0eef7 100644 --- a/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c @@ -214,6 +214,8 @@ static struct hdmi_codec_pdata codec_data = { .ops = &adv7511_codec_ops, .max_i2s_channels = 2, .i2s = 1, + .formats = (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S16_BE | + SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S24_BE), }; int adv7511_audio_init(struct device *dev, struct adv7511 *adv7511) -- 2.11.0 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [alsa-devel] [PATCH v2 2/2] drm/bridge: adv7511: restrict audio sample sizes 2017-07-31 22:49 ` srinivas.kandagatla @ 2017-08-01 8:42 ` Arnaud Pouliquen -1 siblings, 0 replies; 16+ messages in thread From: Arnaud Pouliquen @ 2017-08-01 8:42 UTC (permalink / raw) To: srinivas.kandagatla, Jyri Sarha, Mark Brown, alsa-devel Cc: Archit Taneja, David Airlie, linux-kernel, Liam Girdwood, dri-devel, Takashi Iwai, Andrzej Hajda Hello Srinivas, On 08/01/2017 12:49 AM, srinivas.kandagatla@linaro.org wrote: > From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> > > ADV7533 only supports audio samples word width from 16-24 bits. > This patch restricts the audio sample sizes to the supported ones, > so that sound card does not report wrong list of supported hwparms. > > Without this patch aplay would fail when playing a 32 bit audio. > > Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> > --- > drivers/gpu/drm/bridge/adv7511/adv7511_audio.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c b/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c > index 67469c26bae8..d01d0aa0eef7 100644 > --- a/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c > +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c > @@ -214,6 +214,8 @@ static struct hdmi_codec_pdata codec_data = { > .ops = &adv7511_codec_ops, > .max_i2s_channels = 2, > .i2s = 1, > + .formats = (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S16_BE | > + SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S24_BE), > }; I'm not sure that this limitation is needed. I already used ADV7513 and i did not observe this limitation. I had a look to ADV7533 data-sheet. it should support 32 and 64 bits I2S format bus, with 16 or 24 bits precision sample. So it should support SNDRV_PCM_FMTBIT_S32_LE and SNDRV_PCM_FMTBIT_S32_BE As example, if you configure bus in Left justified format with 24 bits sample length, 32 bits application samples should be truncated to 24 bits samples at ADV7533 I2S interface level (LSB dropped). Perhaps i missed something... but look like more an I2S bus configuration issue in your DT card description, that that a miss in the drivers. Regards Arnaud > > int adv7511_audio_init(struct device *dev, struct adv7511 *adv7511) > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 2/2] drm/bridge: adv7511: restrict audio sample sizes @ 2017-08-01 8:42 ` Arnaud Pouliquen 0 siblings, 0 replies; 16+ messages in thread From: Arnaud Pouliquen @ 2017-08-01 8:42 UTC (permalink / raw) To: srinivas.kandagatla, Jyri Sarha, Mark Brown, alsa-devel Cc: Archit Taneja, Liam Girdwood, David Airlie, Takashi Iwai, dri-devel, linux-kernel, Andrzej Hajda Hello Srinivas, On 08/01/2017 12:49 AM, srinivas.kandagatla@linaro.org wrote: > From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> > > ADV7533 only supports audio samples word width from 16-24 bits. > This patch restricts the audio sample sizes to the supported ones, > so that sound card does not report wrong list of supported hwparms. > > Without this patch aplay would fail when playing a 32 bit audio. > > Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> > --- > drivers/gpu/drm/bridge/adv7511/adv7511_audio.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c b/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c > index 67469c26bae8..d01d0aa0eef7 100644 > --- a/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c > +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c > @@ -214,6 +214,8 @@ static struct hdmi_codec_pdata codec_data = { > .ops = &adv7511_codec_ops, > .max_i2s_channels = 2, > .i2s = 1, > + .formats = (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S16_BE | > + SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S24_BE), > }; I'm not sure that this limitation is needed. I already used ADV7513 and i did not observe this limitation. I had a look to ADV7533 data-sheet. it should support 32 and 64 bits I2S format bus, with 16 or 24 bits precision sample. So it should support SNDRV_PCM_FMTBIT_S32_LE and SNDRV_PCM_FMTBIT_S32_BE As example, if you configure bus in Left justified format with 24 bits sample length, 32 bits application samples should be truncated to 24 bits samples at ADV7533 I2S interface level (LSB dropped). Perhaps i missed something... but look like more an I2S bus configuration issue in your DT card description, that that a miss in the drivers. Regards Arnaud > > int adv7511_audio_init(struct device *dev, struct adv7511 *adv7511) > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [alsa-devel] [PATCH v2 2/2] drm/bridge: adv7511: restrict audio sample sizes 2017-08-01 8:42 ` Arnaud Pouliquen (?) @ 2017-08-01 12:24 ` Srinivas Kandagatla 2017-08-01 12:28 ` Mark Brown -1 siblings, 1 reply; 16+ messages in thread From: Srinivas Kandagatla @ 2017-08-01 12:24 UTC (permalink / raw) To: Arnaud Pouliquen, Jyri Sarha, Mark Brown, alsa-devel Cc: Archit Taneja, David Airlie, linux-kernel, Liam Girdwood, dri-devel, Takashi Iwai, Andrzej Hajda On 01/08/17 09:42, Arnaud Pouliquen wrote: > Hello Srinivas, > > On 08/01/2017 12:49 AM, srinivas.kandagatla@linaro.org wrote: >> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> >> >> ADV7533 only supports audio samples word width from 16-24 bits. >> This patch restricts the audio sample sizes to the supported ones, >> so that sound card does not report wrong list of supported hwparms. >> >> Without this patch aplay would fail when playing a 32 bit audio. >> >> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> >> --- >> drivers/gpu/drm/bridge/adv7511/adv7511_audio.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c b/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c >> index 67469c26bae8..d01d0aa0eef7 100644 >> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c >> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c >> @@ -214,6 +214,8 @@ static struct hdmi_codec_pdata codec_data = { >> .ops = &adv7511_codec_ops, >> .max_i2s_channels = 2, >> .i2s = 1, >> + .formats = (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S16_BE | >> + SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S24_BE), >> }; > > I'm not sure that this limitation is needed. > I already used ADV7513 and i did not observe this limitation. > > I had a look to ADV7533 data-sheet. it should support 32 and 64 bits I2S ADV7511_REG_AUDIO_CFG3(0x14) register definition in datasheet and the code in this driver suggest that It only supports 16Bit and 24Bit samples. > format bus, with 16 or 24 bits precision sample. So it should support > SNDRV_PCM_FMTBIT_S32_LE and SNDRV_PCM_FMTBIT_S32_BE > > As example, if you configure bus in Left justified format with 24 bits > sample length, 32 bits application samples should be truncated to 24 > bits samples at ADV7533 I2S interface level (LSB dropped). May be we can do that to make the user happy but isn't this just truncate the resolution to 24Bit then? And it's a false indication that we are supporting 32bit samples. Which am not very happy with. > > Perhaps i missed something... but look like more an I2S bus > configuration issue in your DT card description, that that a miss in the > drivers. Nothing to do with DT or other driver, adv7533 audio driver does not support 32 bit sample size which is why it would return -EINVAL from adv7511_hdmi_hw_params(). This patch is fixing the loose ends, by which I mean the card would report the exact supported sample sizes to user rather than every thing in the I2S_FORMATS list. Thanks, srini > > Regards > Arnaud >> >> int adv7511_audio_init(struct device *dev, struct adv7511 *adv7511) >> ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [alsa-devel] [PATCH v2 2/2] drm/bridge: adv7511: restrict audio sample sizes 2017-08-01 12:24 ` [alsa-devel] " Srinivas Kandagatla @ 2017-08-01 12:28 ` Mark Brown 2017-08-01 12:52 ` Srinivas Kandagatla 0 siblings, 1 reply; 16+ messages in thread From: Mark Brown @ 2017-08-01 12:28 UTC (permalink / raw) To: Srinivas Kandagatla Cc: Arnaud Pouliquen, Jyri Sarha, alsa-devel, Archit Taneja, David Airlie, linux-kernel, Liam Girdwood, dri-devel, Takashi Iwai, Andrzej Hajda [-- Attachment #1: Type: text/plain, Size: 1189 bytes --] On Tue, Aug 01, 2017 at 01:24:03PM +0100, Srinivas Kandagatla wrote: > On 01/08/17 09:42, Arnaud Pouliquen wrote: > > On 08/01/2017 12:49 AM, srinivas.kandagatla@linaro.org wrote: > > I already used ADV7513 and i did not observe this limitation. > > I had a look to ADV7533 data-sheet. it should support 32 and 64 bits I2S > ADV7511_REG_AUDIO_CFG3(0x14) register definition in datasheet and the code > in this driver suggest that It only supports 16Bit and 24Bit samples. The amount of data it pays attention to in the frame is not the same as the size of the frame. > > format bus, with 16 or 24 bits precision sample. So it should support > > SNDRV_PCM_FMTBIT_S32_LE and SNDRV_PCM_FMTBIT_S32_BE > > As example, if you configure bus in Left justified format with 24 bits > > sample length, 32 bits application samples should be truncated to 24 > > bits samples at ADV7533 I2S interface level (LSB dropped). > May be we can do that to make the user happy but isn't this just truncate > the resolution to 24Bit then? > And it's a false indication that we are supporting 32bit samples. > Which am not very happy with. This is what the sample_bits field in the DAI structure is for. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [alsa-devel] [PATCH v2 2/2] drm/bridge: adv7511: restrict audio sample sizes 2017-08-01 12:28 ` Mark Brown @ 2017-08-01 12:52 ` Srinivas Kandagatla 0 siblings, 0 replies; 16+ messages in thread From: Srinivas Kandagatla @ 2017-08-01 12:52 UTC (permalink / raw) To: Mark Brown Cc: Arnaud Pouliquen, Jyri Sarha, alsa-devel, Archit Taneja, David Airlie, linux-kernel, Liam Girdwood, dri-devel, Takashi Iwai, Andrzej Hajda On 01/08/17 13:28, Mark Brown wrote: > On Tue, Aug 01, 2017 at 01:24:03PM +0100, Srinivas Kandagatla wrote: >> On 01/08/17 09:42, Arnaud Pouliquen wrote: >>> On 08/01/2017 12:49 AM, srinivas.kandagatla@linaro.org wrote: > >>> I already used ADV7513 and i did not observe this limitation. > >>> I had a look to ADV7533 data-sheet. it should support 32 and 64 bits I2S > >> ADV7511_REG_AUDIO_CFG3(0x14) register definition in datasheet and the code >> in this driver suggest that It only supports 16Bit and 24Bit samples. > > The amount of data it pays attention to in the frame is not the same as > the size of the frame. I agree. > >>> format bus, with 16 or 24 bits precision sample. So it should support >>> SNDRV_PCM_FMTBIT_S32_LE and SNDRV_PCM_FMTBIT_S32_BE > >>> As example, if you configure bus in Left justified format with 24 bits >>> sample length, 32 bits application samples should be truncated to 24 >>> bits samples at ADV7533 I2S interface level (LSB dropped). > >> May be we can do that to make the user happy but isn't this just truncate >> the resolution to 24Bit then? > >> And it's a false indication that we are supporting 32bit samples. >> Which am not very happy with. > > This is what the sample_bits field in the DAI structure is for.ya. But still reporting that driver supports 32 bit samples when it does not really support all 32 bits, is kinda misleading to user. Isn't it? And the driver would be end up with hacked up code for each case. --srini > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 2/2] drm/bridge: adv7511: restrict audio sample sizes @ 2017-08-01 12:52 ` Srinivas Kandagatla 0 siblings, 0 replies; 16+ messages in thread From: Srinivas Kandagatla @ 2017-08-01 12:52 UTC (permalink / raw) To: Mark Brown Cc: alsa-devel, Liam Girdwood, David Airlie, Takashi Iwai, Arnaud Pouliquen, Archit Taneja, Jyri Sarha, linux-kernel, Andrzej Hajda, dri-devel On 01/08/17 13:28, Mark Brown wrote: > On Tue, Aug 01, 2017 at 01:24:03PM +0100, Srinivas Kandagatla wrote: >> On 01/08/17 09:42, Arnaud Pouliquen wrote: >>> On 08/01/2017 12:49 AM, srinivas.kandagatla@linaro.org wrote: > >>> I already used ADV7513 and i did not observe this limitation. > >>> I had a look to ADV7533 data-sheet. it should support 32 and 64 bits I2S > >> ADV7511_REG_AUDIO_CFG3(0x14) register definition in datasheet and the code >> in this driver suggest that It only supports 16Bit and 24Bit samples. > > The amount of data it pays attention to in the frame is not the same as > the size of the frame. I agree. > >>> format bus, with 16 or 24 bits precision sample. So it should support >>> SNDRV_PCM_FMTBIT_S32_LE and SNDRV_PCM_FMTBIT_S32_BE > >>> As example, if you configure bus in Left justified format with 24 bits >>> sample length, 32 bits application samples should be truncated to 24 >>> bits samples at ADV7533 I2S interface level (LSB dropped). > >> May be we can do that to make the user happy but isn't this just truncate >> the resolution to 24Bit then? > >> And it's a false indication that we are supporting 32bit samples. >> Which am not very happy with. > > This is what the sample_bits field in the DAI structure is for.ya. But still reporting that driver supports 32 bit samples when it does not really support all 32 bits, is kinda misleading to user. Isn't it? And the driver would be end up with hacked up code for each case. --srini > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [alsa-devel] [PATCH v2 2/2] drm/bridge: adv7511: restrict audio sample sizes 2017-08-01 12:52 ` Srinivas Kandagatla @ 2017-08-01 13:15 ` Mark Brown -1 siblings, 0 replies; 16+ messages in thread From: Mark Brown @ 2017-08-01 13:15 UTC (permalink / raw) To: Srinivas Kandagatla Cc: Arnaud Pouliquen, Jyri Sarha, alsa-devel, Archit Taneja, David Airlie, linux-kernel, Liam Girdwood, dri-devel, Takashi Iwai, Andrzej Hajda [-- Attachment #1: Type: text/plain, Size: 510 bytes --] On Tue, Aug 01, 2017 at 01:52:35PM +0100, Srinivas Kandagatla wrote: > On 01/08/17 13:28, Mark Brown wrote: > > > And it's a false indication that we are supporting 32bit samples. > > > Which am not very happy with. > > This is what the sample_bits field in the DAI structure is for.ya. > But still reporting that driver supports 32 bit samples when it does not > really support all 32 bits, is kinda misleading to user. > Isn't it? No. Please read what I wrote and look at what setting sample_bits does. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [alsa-devel] [PATCH v2 2/2] drm/bridge: adv7511: restrict audio sample sizes @ 2017-08-01 13:15 ` Mark Brown 0 siblings, 0 replies; 16+ messages in thread From: Mark Brown @ 2017-08-01 13:15 UTC (permalink / raw) To: Srinivas Kandagatla Cc: alsa-devel, Liam Girdwood, Takashi Iwai, Arnaud Pouliquen, Jyri Sarha, linux-kernel, dri-devel [-- Attachment #1.1: Type: text/plain, Size: 510 bytes --] On Tue, Aug 01, 2017 at 01:52:35PM +0100, Srinivas Kandagatla wrote: > On 01/08/17 13:28, Mark Brown wrote: > > > And it's a false indication that we are supporting 32bit samples. > > > Which am not very happy with. > > This is what the sample_bits field in the DAI structure is for.ya. > But still reporting that driver supports 32 bit samples when it does not > really support all 32 bits, is kinda misleading to user. > Isn't it? No. Please read what I wrote and look at what setting sample_bits does. [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] [-- Attachment #2: Type: text/plain, Size: 160 bytes --] _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [alsa-devel] [PATCH v2 2/2] drm/bridge: adv7511: restrict audio sample sizes 2017-08-01 12:52 ` Srinivas Kandagatla @ 2017-08-01 15:50 ` Arnaud Pouliquen -1 siblings, 0 replies; 16+ messages in thread From: Arnaud Pouliquen @ 2017-08-01 15:50 UTC (permalink / raw) To: Srinivas Kandagatla, Mark Brown Cc: Jyri Sarha, alsa-devel, Archit Taneja, David Airlie, linux-kernel, Liam Girdwood, dri-devel, Takashi Iwai, Andrzej Hajda Hello Srinivas, On 08/01/2017 02:52 PM, Srinivas Kandagatla wrote: >>>> As example, if you configure bus in Left justified format with 24 bits >>>> sample length, 32 bits application samples should be truncated to 24 >>>> bits samples at ADV7533 I2S interface level (LSB dropped). >> >>> May be we can do that to make the user happy but isn't this just truncate >>> the resolution to 24Bit then? >> >>> And it's a false indication that we are supporting 32bit samples. >>> Which am not very happy with. >> >> This is what the sample_bits field in the DAI structure is for.ya. > But still reporting that driver supports 32 bit samples when it does not > really support all 32 bits, is kinda misleading to user. > Isn't it? > > And the driver would be end up with hacked up code for each case. By experience, this is usual. As example, if you have a look to codec ad193x (i take one randomly) it support 16, 20, 24 and 32 bits frames. But if you have a look to AD1939 data-sheet it supports 24-bits conversion. Some other examples could be 13-bits DAC/ADC with 16 bits samples. In term of audio quality, truncation to a 24 bits sample should generate an negligible additional error equal to the LSB bit: -20log(2^24)= -144dB. It is just a personal opinion, but if have the choice between do truncation in software (application or alsa-lib) and in hardware, i would prefer the second one. Regards arnaud ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 2/2] drm/bridge: adv7511: restrict audio sample sizes @ 2017-08-01 15:50 ` Arnaud Pouliquen 0 siblings, 0 replies; 16+ messages in thread From: Arnaud Pouliquen @ 2017-08-01 15:50 UTC (permalink / raw) To: Srinivas Kandagatla, Mark Brown Cc: alsa-devel, David Airlie, linux-kernel, Takashi Iwai, Liam Girdwood, Jyri Sarha, Archit Taneja, Andrzej Hajda, dri-devel Hello Srinivas, On 08/01/2017 02:52 PM, Srinivas Kandagatla wrote: >>>> As example, if you configure bus in Left justified format with 24 bits >>>> sample length, 32 bits application samples should be truncated to 24 >>>> bits samples at ADV7533 I2S interface level (LSB dropped). >> >>> May be we can do that to make the user happy but isn't this just truncate >>> the resolution to 24Bit then? >> >>> And it's a false indication that we are supporting 32bit samples. >>> Which am not very happy with. >> >> This is what the sample_bits field in the DAI structure is for.ya. > But still reporting that driver supports 32 bit samples when it does not > really support all 32 bits, is kinda misleading to user. > Isn't it? > > And the driver would be end up with hacked up code for each case. By experience, this is usual. As example, if you have a look to codec ad193x (i take one randomly) it support 16, 20, 24 and 32 bits frames. But if you have a look to AD1939 data-sheet it supports 24-bits conversion. Some other examples could be 13-bits DAC/ADC with 16 bits samples. In term of audio quality, truncation to a 24 bits sample should generate an negligible additional error equal to the LSB bit: -20log(2^24)= -144dB. It is just a personal opinion, but if have the choice between do truncation in software (application or alsa-lib) and in hardware, i would prefer the second one. Regards arnaud ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2017-08-01 15:51 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-07-31 22:49 [PATCH v2 0/2] ASoC: hdmi-codec: Allow driver to restrict sample size srinivas.kandagatla 2017-07-31 22:49 ` srinivas.kandagatla 2017-07-31 22:49 ` [PATCH v2 1/2] ASoC: hdmi-codec: Allow drivers to restrict sample sizes srinivas.kandagatla 2017-07-31 22:49 ` srinivas.kandagatla 2017-07-31 22:49 ` [PATCH v2 2/2] drm/bridge: adv7511: restrict audio " srinivas.kandagatla 2017-07-31 22:49 ` srinivas.kandagatla 2017-08-01 8:42 ` [alsa-devel] " Arnaud Pouliquen 2017-08-01 8:42 ` Arnaud Pouliquen 2017-08-01 12:24 ` [alsa-devel] " Srinivas Kandagatla 2017-08-01 12:28 ` Mark Brown 2017-08-01 12:52 ` Srinivas Kandagatla 2017-08-01 12:52 ` Srinivas Kandagatla 2017-08-01 13:15 ` [alsa-devel] " Mark Brown 2017-08-01 13:15 ` Mark Brown 2017-08-01 15:50 ` Arnaud Pouliquen 2017-08-01 15:50 ` Arnaud Pouliquen
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.