[alsa-devel] [RFC PATCH] ASoC: pcm512x: Implement the set_bclk_ratio interface
From 5e6ad135fd063d4b22cd962de43499a161bbc7db Mon Sep 17 00:00:00 2001
From: Dimitris Papavasiliou dpapavas@gmail.com Date: Fri, 11 Jan 2019 22:13:27 +0200 Subject: [RFC PATCH] ASoC: pcm512x: Implement the set_bclk_ratio interface
Some boards, such as the HiFiBerry DAC+ Pro, use a pair of external oscillators, to generate 44.1 or 48kHz multiples and are forced to resort to hacks[1] in order to support 24-bit data without ending up with fractional dividers. This patch allows the machine driver to use 32-bit frames for 24-bit data to avoid such issues.
[1] http://mailman.alsa-project.org/pipermail/alsa-devel/2018-December/143442.ht...
Signed-off-by: Dimitris Papavasiliou dpapavas@gmail.com ---
Notes: The patch also includes a related change in the calculation of the bclk divider. The current calculation is not very clear to me, as I can't find much concrete information on what this sample rate fraction is, how it's calculated and how, therefore, its denominator would fit into the calculation. It also yields slightly wrong dividers in certain cases, which the machine driver for my HiFiBerry DAC+ Pro board seemingly tries to circumvent, by updating the rate fraction so as to suit this calculation.
The updated calculation seems to work fine in my tests and, as far as I can see, should correctly yield the smallest bit clock rate that would fit the frame. Please comment if anything looks off.
sound/soc/codecs/pcm512x.c | 29 +++++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-)
diff --git a/sound/soc/codecs/pcm512x.c b/sound/soc/codecs/pcm512x.c index 4cc24a5d5c31..8cd728f9a1eb 100644 --- a/sound/soc/codecs/pcm512x.c +++ b/sound/soc/codecs/pcm512x.c @@ -55,6 +55,7 @@ struct pcm512x_priv { unsigned long overclock_dsp; int mute; struct mutex mutex; + unsigned int bclk_ratio; };
/* @@ -915,16 +916,21 @@ static int pcm512x_set_dividers(struct snd_soc_dai *dai, int fssp; int gpio;
- lrclk_div = snd_soc_params_to_frame_size(params); - if (lrclk_div == 0) { - dev_err(dev, "No LRCLK?\n"); - return -EINVAL; + if (pcm512x->bclk_ratio > 0) { + lrclk_div = pcm512x->bclk_ratio; + } else { + lrclk_div = snd_soc_params_to_frame_size(params); + + if (lrclk_div == 0) { + dev_err(dev, "No LRCLK?\n"); + return -EINVAL; + } }
if (!pcm512x->pll_out) { sck_rate = clk_get_rate(pcm512x->sclk); - bclk_div = params->rate_den * 64 / lrclk_div; - bclk_rate = DIV_ROUND_CLOSEST(sck_rate, bclk_div); + bclk_rate = params_rate(params) * lrclk_div; + bclk_div = DIV_ROUND_CLOSEST(sck_rate, bclk_rate);
mck_rate = sck_rate; } else { @@ -1383,6 +1389,16 @@ static int pcm512x_set_fmt(struct snd_soc_dai *dai, unsigned int fmt) return 0; }
+static int pcm512x_set_bclk_ratio(struct snd_soc_dai *dai, unsigned int ratio) +{ + struct snd_soc_component *component = dai->component; + struct pcm512x_priv *pcm512x = snd_soc_component_get_drvdata(component); + + pcm512x->bclk_ratio = ratio; + + return 0; +} + static int pcm512x_digital_mute(struct snd_soc_dai *dai, int mute) { struct snd_soc_component *component = dai->component; @@ -1435,6 +1451,7 @@ static const struct snd_soc_dai_ops pcm512x_dai_ops = { .hw_params = pcm512x_hw_params, .set_fmt = pcm512x_set_fmt, .digital_mute = pcm512x_digital_mute, + .set_bclk_ratio = pcm512x_set_bclk_ratio, };
static struct snd_soc_dai_driver pcm512x_dai = {
On 2019-01-13 21:16, Dimitris Papavasiliou wrote:
From 5e6ad135fd063d4b22cd962de43499a161bbc7db Mon Sep 17 00:00:00 2001 From: Dimitris Papavasiliou dpapavas@gmail.com Date: Fri, 11 Jan 2019 22:13:27 +0200 Subject: [RFC PATCH] ASoC: pcm512x: Implement the set_bclk_ratio interface
Some boards, such as the HiFiBerry DAC+ Pro, use a pair of external oscillators, to generate 44.1 or 48kHz multiples and are forced to
Ouch, is the PLL not fine-grained enough? That seems unlikely. Did they perhaps simply not know how to wire the chip for the PLL to work?
resort to hacks[1] in order to support 24-bit data without ending up with fractional dividers. This patch allows the machine driver to use 32-bit frames for 24-bit data to avoid such issues.
[1] http://mailman.alsa-project.org/pipermail/alsa-devel/2018-December/143442.ht...
Signed-off-by: Dimitris Papavasiliou dpapavas@gmail.com
Notes: The patch also includes a related change in the calculation of the bclk divider. The current calculation is not very clear to me, as I can't find much concrete information on what this sample rate fraction is, how it's calculated and how, therefore, its denominator would fit into the calculation. It also yields slightly wrong dividers in certain cases, which the machine driver for my HiFiBerry DAC+ Pro board seemingly tries to circumvent, by updating the rate fraction so as to suit this calculation.
The updated calculation seems to work fine in my tests and, as far as I can see, should correctly yield the smallest bit clock rate that would fit the frame. Please comment if anything looks off.
On reading this, I think this should be a separate patch. But maybe that's just me?
sound/soc/codecs/pcm512x.c | 29 +++++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-)
diff --git a/sound/soc/codecs/pcm512x.c b/sound/soc/codecs/pcm512x.c index 4cc24a5d5c31..8cd728f9a1eb 100644 --- a/sound/soc/codecs/pcm512x.c +++ b/sound/soc/codecs/pcm512x.c @@ -55,6 +55,7 @@ struct pcm512x_priv { unsigned long overclock_dsp; int mute; struct mutex mutex;
- unsigned int bclk_ratio;
};
/* @@ -915,16 +916,21 @@ static int pcm512x_set_dividers(struct snd_soc_dai *dai, int fssp; int gpio;
- lrclk_div = snd_soc_params_to_frame_size(params);
- if (lrclk_div == 0) {
dev_err(dev, "No LRCLK?\n");return -EINVAL;
if (pcm512x->bclk_ratio > 0) {
lrclk_div = pcm512x->bclk_ratio;} else {
lrclk_div = snd_soc_params_to_frame_size(params);if (lrclk_div == 0) {dev_err(dev, "No LRCLK?\n");return -EINVAL;}}
if (!pcm512x->pll_out) { sck_rate = clk_get_rate(pcm512x->sclk);
bclk_div = params->rate_den * 64 / lrclk_div;bclk_rate = DIV_ROUND_CLOSEST(sck_rate, bclk_div);
bclk_rate = params_rate(params) * lrclk_div;bclk_div = DIV_ROUND_CLOSEST(sck_rate, bclk_rate);
Ok, so I apparently wrote this. Half a decade ago or so. It should be known that my focus was entirely on the pll-case. I added the non-pll-case mostly as a courtesy and we are not using it over here. If you don't have an even divider, you should be using the pll anyway, right? :-) At least if you knew you had to wire the HW so that the pll is an option... The datasheets are not very clear, at least they were not way back when for the chip we focused on (the pcm5142, big relief when I discovered the pcm5242 datasheet).
Anyway, the value of bclk_rate is obviously not what matters, that's just a helper variable in the pll-case and (previously) not used at all in the non-pll-case. It is the value of bclk_div that matters. So, whatever code gets you to the desired bclk_div is what you should aim for and AFAICT your above change makes sense.
I do wonder why I hard-coded the 64, I have a vague memory of it being quite deliberate. It's all a haze now though...
mck_rate = sck_rate;} else { @@ -1383,6 +1389,16 @@ static int pcm512x_set_fmt(struct snd_soc_dai *dai, unsigned int fmt) return 0; }
+static int pcm512x_set_bclk_ratio(struct snd_soc_dai *dai, unsigned int ratio) +{
- struct snd_soc_component *component = dai->component;
- struct pcm512x_priv *pcm512x = snd_soc_component_get_drvdata(component);
- pcm512x->bclk_ratio = ratio;
You should perhaps check if (ratio >= 1 && ratio <= 256) prior to accepting a divider that can't be programmed? But maybe that's enforced somewhere else? And perhaps the sanity check should be even stricter?
Cheers, Peter
- return 0;
+}
static int pcm512x_digital_mute(struct snd_soc_dai *dai, int mute) { struct snd_soc_component *component = dai->component; @@ -1435,6 +1451,7 @@ static const struct snd_soc_dai_ops pcm512x_dai_ops = { .hw_params = pcm512x_hw_params, .set_fmt = pcm512x_set_fmt, .digital_mute = pcm512x_digital_mute,
- .set_bclk_ratio = pcm512x_set_bclk_ratio,
};
static struct snd_soc_dai_driver pcm512x_dai = {
Thank you for your comments Peter.
On 1/14/19 1:13 PM, Peter Rosin wrote:> On 2019-01-13 21:16, Dimitris Papavasiliou wrote:
Some boards, such as the HiFiBerry DAC+ Pro, use a pair of external oscillators, to generate 44.1 or 48kHz multiples and are forced to
Ouch, is the PLL not fine-grained enough? That seems unlikely. Did they perhaps simply not know how to wire the chip for the PLL to work?
There's a cheaper version of the board, which operates in slave mode, without external clocks. My understanding, based on their marketing blurb, is that it's done in order to improve audio quality by minimizing clock jitter. The datasheet mentions that using an external clock would be desirable, in order to "keep the jitter evident in the PLL (in all PLLs) out of the DAC", but it suggests using the external clock only for the DAC clock and the clocks derived from it and keep the PLL for the audio processing block.
Whether this would have been preferable, but they couldn't figure out how to get it to work, I can't really tell. What I can say, is that the current design is causing all sorts of problems, none of which seem to be evident when the device is configured to run in slave mode.
Notes: The patch also includes a related change in the calculation of the bclk divider. The current calculation is not very clear to me, as I can't find much concrete information on what this sample rate fraction is, how it's calculated and how, therefore, its denominator would fit into the calculation. It also yields slightly wrong dividers in certain cases, which the machine driver for my HiFiBerry DAC+ Pro board seemingly tries to circumvent, by updating the rate fraction so as to suit this calculation.
The updated calculation seems to work fine in my tests and, as far as I can see, should correctly yield the smallest bit clock rate that would fit the frame. Please comment if anything looks off.On reading this, I think this should be a separate patch. But maybe that's just me?
No, I also considered splitting this into two patches, but decided to keep the RFC in one piece, to keep the discussion in one place. The patches are related after all, in the sense that it probably wouldn't make much sense to just change the calculation, if the set_bclk_ratio patch is rejected. If the patch proves to be, in principle at least, acceptable, I'll probably submit it as two separate patches.
- lrclk_div = snd_soc_params_to_frame_size(params);
- if (lrclk_div == 0) {
dev_err(dev, "No LRCLK?\n");return -EINVAL;
if (pcm512x->bclk_ratio > 0) {
lrclk_div = pcm512x->bclk_ratio;} else {
lrclk_div = snd_soc_params_to_frame_size(params);if (lrclk_div == 0) {dev_err(dev, "No LRCLK?\n");return -EINVAL;} } if (!pcm512x->pll_out) { sck_rate = clk_get_rate(pcm512x->sclk);
bclk_div = params->rate_den * 64 / lrclk_div;bclk_rate = DIV_ROUND_CLOSEST(sck_rate, bclk_div);
bclk_rate = params_rate(params) * lrclk_div;bclk_div = DIV_ROUND_CLOSEST(sck_rate, bclk_rate);Ok, so I apparently wrote this. Half a decade ago or so. It should be known that my focus was entirely on the pll-case. I added the non-pll-case mostly as a courtesy and we are not using it over here. If you don't have an even divider, you should be using the pll anyway, right? :-) At least if you knew you had to wire the HW so that the pll is an option... The datasheets are not very clear, at least they were not way back when for the chip we focused on (the pcm5142, big relief when I discovered the pcm5242 datasheet).
It's good to know this code path doesn't affect you. The TSE-850 seems to be the only driver using the pcm512x CODEC driver in the mainline kernel and I was worrying that the change might cause trouble for you.
+static int pcm512x_set_bclk_ratio(struct snd_soc_dai *dai, unsigned int ratio) +{
- struct snd_soc_component *component = dai->component;
- struct pcm512x_priv *pcm512x = snd_soc_component_get_drvdata(component);
- pcm512x->bclk_ratio = ratio;
You should perhaps check if (ratio >= 1 && ratio <= 256) prior to accepting a divider that can't be programmed? But maybe that's enforced somewhere else? And perhaps the sanity check should be even stricter?
Yes, you're probably right. This RFC was mostly meant as a proof-of-concept implementation, in order to get some feedback on whether this approach is appropriate, or at least makes some sense. If it does, I'll have to look further into what checks are necessary.
On 2019-01-14 15:53, Dimitris Papavasiliou wrote:
Thank you for your comments Peter.
On 1/14/19 1:13 PM, Peter Rosin wrote:> On 2019-01-13 21:16, Dimitris Papavasiliou wrote:
Some boards, such as the HiFiBerry DAC+ Pro, use a pair of external oscillators, to generate 44.1 or 48kHz multiples and are forced to
Ouch, is the PLL not fine-grained enough? That seems unlikely. Did they perhaps simply not know how to wire the chip for the PLL to work?
There's a cheaper version of the board, which operates in slave mode, without external clocks. My understanding, based on their marketing blurb, is that it's done in order to improve audio quality by minimizing clock jitter. The datasheet mentions that using an external clock would be desirable, in order to "keep the jitter evident in the PLL (in all PLLs) out of the DAC", but it suggests using the external clock only for the DAC clock and the clocks derived from it and keep the PLL for the audio processing block.
Whether this would have been preferable, but they couldn't figure out how to get it to work, I can't really tell. What I can say, is that the current design is causing all sorts of problems, none of which seem to be evident when the device is configured to run in slave mode.
Right, the slave part of the driver is way simpler than all the nasty clock handling required when mastering the AIF...
Notes: The patch also includes a related change in the calculation of the bclk divider. The current calculation is not very clear to me, as I can't find much concrete information on what this sample rate fraction is, how it's calculated and how, therefore, its denominator would fit into the calculation. It also yields slightly wrong dividers in certain cases, which the machine driver for my HiFiBerry DAC+ Pro board seemingly tries to circumvent, by updating the rate fraction so as to suit this calculation.
The updated calculation seems to work fine in my tests and, as far as I can see, should correctly yield the smallest bit clock rate that would fit the frame. Please comment if anything looks off.On reading this, I think this should be a separate patch. But maybe that's just me?
No, I also considered splitting this into two patches, but decided to keep the RFC in one piece, to keep the discussion in one place. The patches are related after all, in the sense that it probably wouldn't make much sense to just change the calculation, if the set_bclk_ratio patch is rejected. If the patch proves to be, in principle at least, acceptable, I'll probably submit it as two separate patches.
- lrclk_div = snd_soc_params_to_frame_size(params);
- if (lrclk_div == 0) {
dev_err(dev, "No LRCLK?\n");return -EINVAL;
if (pcm512x->bclk_ratio > 0) {
lrclk_div = pcm512x->bclk_ratio;} else {
lrclk_div = snd_soc_params_to_frame_size(params);if (lrclk_div == 0) {dev_err(dev, "No LRCLK?\n");return -EINVAL;} } if (!pcm512x->pll_out) { sck_rate = clk_get_rate(pcm512x->sclk);
bclk_div = params->rate_den * 64 / lrclk_div;bclk_rate = DIV_ROUND_CLOSEST(sck_rate, bclk_div);
bclk_rate = params_rate(params) * lrclk_div;bclk_div = DIV_ROUND_CLOSEST(sck_rate, bclk_rate);Ok, so I apparently wrote this. Half a decade ago or so. It should be known that my focus was entirely on the pll-case. I added the non-pll-case mostly as a courtesy and we are not using it over here. If you don't have an even divider, you should be using the pll anyway, right? :-) At least if you knew you had to wire the HW so that the pll is an option... The datasheets are not very clear, at least they were not way back when for the chip we focused on (the pcm5142, big relief when I discovered the pcm5242 datasheet).
It's good to know this code path doesn't affect you. The TSE-850 seems to be the only driver using the pcm512x CODEC driver in the mainline kernel and I was worrying that the change might cause trouble for you.
Yes, we are typically only using a single sample frequency (250kHz) and have naturally selected an external xtal so that the pll can be avoided (sck is 16MHz, but since we have the pll option, the branch you changed is not affecting our case). At least not if pcm512x->bclk_ratio is left at zero (or set to 64)...
+static int pcm512x_set_bclk_ratio(struct snd_soc_dai *dai, unsigned int ratio) +{
- struct snd_soc_component *component = dai->component;
- struct pcm512x_priv *pcm512x = snd_soc_component_get_drvdata(component);
- pcm512x->bclk_ratio = ratio;
You should perhaps check if (ratio >= 1 && ratio <= 256) prior to accepting a divider that can't be programmed? But maybe that's enforced somewhere else? And perhaps the sanity check should be even stricter?
...and when double-checking that, this dai op is only ever called from snd_soc_dai_set_bclk_ratio, and that function is in turn not called from anywhere AFAICT. What's the point of this dai op anyway? Or, what am I missing?
Cheers, Peter
On Tue, Jan 15, 2019 at 10:06 AM Peter Rosin peda@axentia.se wrote:
...and when double-checking that, this dai op is only ever called from snd_soc_dai_set_bclk_ratio, and that function is in turn not called from anywhere AFAICT. What's the point of this dai op anyway? Or, what am I missing?
Well, my use for this, would be to allow the machine driver to call snd_soc_dai_set_bclk_ratio, in order to explicitly set the frame width. This would allow it to use 32-bit frames for 24-bit data and avoid the fractional dividers that would otherwise result. Right now, the machine driver (which lives in the Raspberry Pi kernel tree) uses a workaround for this, that relies on a patch made to the CODEC driver to half-support TDM slots, which hasn't be pushed upstream, and wouldn't be accepted if it had, since it's essentially a hack. You can find more details here:
http://mailman.alsa-project.org/pipermail/alsa-devel/2018-December/143507.ht...
What the intended use for the snd_soc_dai_set_bclk_ratio is, I'm not so clear about, hence this RFC.
On 2019-01-15 12:11, Dimitris Papavasiliou wrote:
On Tue, Jan 15, 2019 at 10:06 AM Peter Rosin peda@axentia.se wrote:
...and when double-checking that, this dai op is only ever called from snd_soc_dai_set_bclk_ratio, and that function is in turn not called from anywhere AFAICT. What's the point of this dai op anyway? Or, what am I missing?
Well, my use for this, would be to allow the machine driver to call snd_soc_dai_set_bclk_ratio, in order to explicitly set the frame width. This would allow it to use 32-bit frames for 24-bit data and avoid the fractional dividers that would otherwise result. Right now, the machine driver (which lives in the Raspberry Pi kernel tree) uses a workaround for this, that relies on a patch made to the CODEC driver to half-support TDM slots, which hasn't be pushed upstream, and wouldn't be accepted if it had, since it's essentially a hack. You can find more details here:
http://mailman.alsa-project.org/pipermail/alsa-devel/2018-December/143507.ht...
What the intended use for the snd_soc_dai_set_bclk_ratio is, I'm not so clear about, hence this RFC.
I did some digging. The dai op was added in 3.13 (without users), then one user appeared in 3.19 (sound/soc/intel/boards/bytcr_rt5640.c) and another (presumably copy-pasted) in 4.5 (sound/soc/intel/boards/bytcr_rt5651.c). Both these users are suspect since neither the rt5640 nor the rt5651 codec have ever sported a set_bclk_ratio dai op, and presumably the boards have the named codecs on them. But what do I know...
The user added in 3.19 was removed for 4.9 and the other for 4.17, so that's why there are no upstream users at the moment.
Cheers, Peter
mck_rate = sck_rate;} else { @@ -1383,6 +1389,16 @@ static int pcm512x_set_fmt(struct snd_soc_dai *dai, unsigned int fmt) return 0; }
+static int pcm512x_set_bclk_ratio(struct snd_soc_dai *dai, unsigned int ratio) +{
- struct snd_soc_component *component = dai->component;
- struct pcm512x_priv *pcm512x = snd_soc_component_get_drvdata(component);
- pcm512x->bclk_ratio = ratio;
You should perhaps check if (ratio >= 1 && ratio <= 256) prior to accepting a divider that can't be programmed? But maybe that's enforced somewhere else? And perhaps the sanity check should be even stricter?
Yes it should be stricter with a power of two only. I tried really hard to make this codec work with ratios of 50 (and a 19.2 MHz mclk) on an Up board and it's just not possible, probably not supported by hardware.
On 2019-01-14 18:36, Pierre-Louis Bossart wrote:
mck_rate = sck_rate;} else { @@ -1383,6 +1389,16 @@ static int pcm512x_set_fmt(struct snd_soc_dai *dai, unsigned int fmt) return 0; }
+static int pcm512x_set_bclk_ratio(struct snd_soc_dai *dai, unsigned int ratio) +{
- struct snd_soc_component *component = dai->component;
- struct pcm512x_priv *pcm512x = snd_soc_component_get_drvdata(component);
- pcm512x->bclk_ratio = ratio;
You should perhaps check if (ratio >= 1 && ratio <= 256) prior to accepting a divider that can't be programmed? But maybe that's enforced somewhere else? And perhaps the sanity check should be even stricter?
Yes it should be stricter with a power of two only. I tried really hard to make this codec work with ratios of 50 (and a 19.2 MHz mclk) on an Up board and it's just not possible, probably not supported by hardware.
Disallowing anything but powers of two just because 50 doesn't work seems pretty wild. According to docs I think 48 should work just fine.
Section 8.3.2 Audio Data Interface page 16. http://www.ti.com/lit/ds/symlink/pcm5142.pdf
Or?
Cheers, Peter
On Tue, Jan 15, 2019 at 10:08 AM Peter Rosin peda@axentia.se wrote:
On 2019-01-14 18:36, Pierre-Louis Bossart wrote:
You should perhaps check if (ratio >= 1 && ratio <= 256) prior to accepting a divider that can't be programmed? But maybe that's enforced somewhere else? And perhaps the sanity check should be even stricter?
Yes it should be stricter with a power of two only. I tried really hard to make this codec work with ratios of 50 (and a 19.2 MHz mclk) on an Up board and it's just not possible, probably not supported by hardware.
Disallowing anything but powers of two just because 50 doesn't work seems pretty wild. According to docs I think 48 should work just fine.
Section 8.3.2 Audio Data Interface page 16. http://www.ti.com/lit/ds/symlink/pcm5142.pdf
Or?
According to the datasheet[1], you can program any divider between 1 and 256, via register 33 (page 80), but, as Peter points out, in the description of the audio data interface (page 15), only the values 32, 48, 64, 128, 256 are mentioned as permissible. So one approach would be to restrict the values accepted by the callback to this set. Since the datasheet is not terribly clear on whether only these are allowed, and since the ratio is set explicitly in the machine driver, it could be argued that it would be preferable to allow all values that can be set in the register, to minimize the risk of needlessly rejecting valid configurations.
On 1/15/19 6:19 AM, Dimitris Papavasiliou wrote:
On Tue, Jan 15, 2019 at 10:08 AM Peter Rosin peda@axentia.se wrote:
On 2019-01-14 18:36, Pierre-Louis Bossart wrote:
You should perhaps check if (ratio >= 1 && ratio <= 256) prior to accepting a divider that can't be programmed? But maybe that's enforced somewhere else? And perhaps the sanity check should be even stricter?
Yes it should be stricter with a power of two only. I tried really hard to make this codec work with ratios of 50 (and a 19.2 MHz mclk) on an Up board and it's just not possible, probably not supported by hardware.
Disallowing anything but powers of two just because 50 doesn't work seems pretty wild. According to docs I think 48 should work just fine.
Section 8.3.2 Audio Data Interface page 16. http://www.ti.com/lit/ds/symlink/pcm5142.pdf
Or?
According to the datasheet[1], you can program any divider between 1 and 256, via register 33 (page 80), but, as Peter points out, in the description of the audio data interface (page 15), only the values 32, 48, 64, 128, 256 are mentioned as permissible. So one approach would be to restrict the values accepted by the callback to this set. Since the datasheet is not terribly clear on whether only these are allowed, and since the ratio is set explicitly in the machine driver, it could be argued that it would be preferable to allow all values that can be set in the register, to minimize the risk of needlessly rejecting valid configurations.
I meant multiple of 16, sorry. multiples of 25/50 commonly used with other codecs will not work.
On Sun, Jan 13, 2019 at 10:16:21PM +0200, Dimitris Papavasiliou wrote:
Some boards, such as the HiFiBerry DAC+ Pro, use a pair of external oscillators, to generate 44.1 or 48kHz multiples and are forced to resort to hacks[1] in order to support 24-bit data without ending up with fractional dividers. This patch allows the machine driver to use 32-bit frames for 24-bit data to avoid such issues.
This seems fine to me. I think when I accepted the patch for setting bclk ratio that there was hardware which specified the ratio directly as part of the configuration but it seems that this was not the case; on the other hand this seems like a reasonable use for it.
participants (4)
-
Dimitris Papavasiliou -
Mark Brown -
Peter Rosin -
Pierre-Louis Bossart