| Message ID | 1442979683-9441-2-git-send-email-sugar.zhang@rock-chips.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Sep 23, 2015 at 11:41:22AM +0800, Sugar Zhang wrote: > + /* configure tx/rx lrck use mode */ > + if (!of_property_read_u32(node, "rockchip,lrck-mode", &val)) { > + if (val >= LRCK_TXRX && val <= LRCK_RX_SHARE) > + regmap_update_bits(i2s->regmap, I2S_CKR, > + I2S_CKR_TRCM_MASK, > + I2S_CKR_TRCM(val)); > + } This looks like it's for a board configuration thing so I'd not really expect this to be handled in a device specific property - it's fairly common to have this situation and we already have the symmetric_rates flag for the DAI to handle it (and if we do end up adding this property we'd need the driver to set that flag so that the core can handle things properly and make sure that userspace doesn't try to set different rates in different directions). My initial thought here is that the machine driver should be responsible for setting this and then the DAI driver should check to see if symmetric_rates are in use and configure itself appropriately. Is there a reason why this won't work here?
Hi Mark Brown, ? 9/24/2015 00:24, Mark Brown ??: > On Wed, Sep 23, 2015 at 11:41:22AM +0800, Sugar Zhang wrote: > >> + /* configure tx/rx lrck use mode */ >> + if (!of_property_read_u32(node, "rockchip,lrck-mode", &val)) { >> + if (val >= LRCK_TXRX && val <= LRCK_RX_SHARE) >> + regmap_update_bits(i2s->regmap, I2S_CKR, >> + I2S_CKR_TRCM_MASK, >> + I2S_CKR_TRCM(val)); >> + } > > This looks like it's for a board configuration thing so I'd not really > expect this to be handled in a device specific property - it's fairly > common to have this situation and we already have the symmetric_rates > flag for the DAI to handle it (and if we do end up adding this property > we'd need the driver to set that flag so that the core can handle things > properly and make sure that userspace doesn't try to set different rates > in different directions). > > My initial thought here is that the machine driver should be responsible > for setting this and then the DAI driver should check to see if > symmetric_rates are in use and configure itself appropriately. Is there > a reason why this won't work here? > It's for i2s ip configuration, in the most situation, there is no need to use this property, except one case: In order to save gpio pins for other function use, we may use single lrck(tx or rx) pin. of course, it depends on product design. when in i2s slave mode, we need to configure this to share lrck with tx/rx inside i2s logic. symmetric_rates flag works fine on rockchip platform, but it can't cover the above case. Do you have any suggestion about this or maybe there is no need to upstream this special part? Best Regards Sugar
On Mon, Sep 28, 2015 at 04:16:12PM +0800, sugar wrote: > ? 9/24/2015 00:24, Mark Brown ??: > >My initial thought here is that the machine driver should be responsible > >for setting this and then the DAI driver should check to see if > >symmetric_rates are in use and configure itself appropriately. Is there > >a reason why this won't work here? > It's for i2s ip configuration, in the most situation, there is no need > to use this property, except one case: > In order to save gpio pins for other function use, we may use single > lrck(tx or rx) pin. of course, it depends on product design. when in i2s > slave mode, we need to configure this to share lrck with tx/rx inside i2s > logic. > symmetric_rates flag works fine on rockchip platform, but it can't cover the > above case. > Do you have any suggestion about this or maybe there is no need to upstream > this special part? What makes you say that the symmetric_rates flag can't be used to cover this case? What you describe above is hte normal reason for needing to enforce symmetric_rates. The driver should be able to check if the flag has been set just as well as the core is.
Sorry for late reply, we were in the National Day Holiday. ? 10/1/2015 02:46, Mark Brown ??: > On Mon, Sep 28, 2015 at 04:16:12PM +0800, sugar wrote: >> ? 9/24/2015 00:24, Mark Brown ??: > >>> My initial thought here is that the machine driver should be responsible >>> for setting this and then the DAI driver should check to see if >>> symmetric_rates are in use and configure itself appropriately. Is there >>> a reason why this won't work here? > >> It's for i2s ip configuration, in the most situation, there is no need >> to use this property, except one case: > >> In order to save gpio pins for other function use, we may use single >> lrck(tx or rx) pin. of course, it depends on product design. when in i2s >> slave mode, we need to configure this to share lrck with tx/rx inside i2s >> logic. > >> symmetric_rates flag works fine on rockchip platform, but it can't cover the >> above case. > >> Do you have any suggestion about this or maybe there is no need to upstream >> this special part? > > What makes you say that the symmetric_rates flag can't be used to cover > this case? What you describe above is hte normal reason for needing to > enforce symmetric_rates. The driver should be able to check if the flag > has been set just as well as the core is. > Got it, How about the following modify? if (dai->symmetric_rates) regmap_update_bits(i2s->regmap, I2S_CKR, I2S_CKR_TRCM_MASK, I2S_CKR_TRCM(val));
On Wed, Oct 07, 2015 at 04:01:38PM +0800, sugar wrote: > ? 10/1/2015 02:46, Mark Brown ??: > >What makes you say that the symmetric_rates flag can't be used to cover > >this case? What you describe above is hte normal reason for needing to > >enforce symmetric_rates. The driver should be able to check if the flag > >has been set just as well as the core is. > Got it, How about the following modify? > if (dai->symmetric_rates) > regmap_update_bits(i2s->regmap, I2S_CKR, > I2S_CKR_TRCM_MASK, > I2S_CKR_TRCM(val)); Yes, something like that. You'll need to check both links in the DAI and the DAI link itself rather than just your own DAI but we should have a helper function for that - I'll add one, look out for a patch shortly.
On Wed, Oct 07, 2015 at 10:27:14AM +0100, Mark Brown wrote: > On Wed, Oct 07, 2015 at 04:01:38PM +0800, sugar wrote: > > if (dai->symmetric_rates) > > regmap_update_bits(i2s->regmap, I2S_CKR, > > I2S_CKR_TRCM_MASK, > > I2S_CKR_TRCM(val)); > Yes, something like that. You'll need to check both links in the DAI > and the DAI link itself rather than just your own DAI but we should have > a helper function for that - I'll add one, look out for a patch shortly. Actually no, now I look at the code you probably want to just clone the check from soc_pcm_apply_symmetry() so have that be: struct snd_soc_pcm_runtime *rtd = substream->private_data; if (rtd->dai_link->symmetrict_rates) { } instead.
? 10/7/2015 17:39, Mark Brown ??: > On Wed, Oct 07, 2015 at 10:27:14AM +0100, Mark Brown wrote: >> On Wed, Oct 07, 2015 at 04:01:38PM +0800, sugar wrote: > >>> if (dai->symmetric_rates) >>> regmap_update_bits(i2s->regmap, I2S_CKR, >>> I2S_CKR_TRCM_MASK, >>> I2S_CKR_TRCM(val)); > >> Yes, something like that. You'll need to check both links in the DAI >> and the DAI link itself rather than just your own DAI but we should have >> a helper function for that - I'll add one, look out for a patch shortly. > > Actually no, now I look at the code you probably want to just clone the > check from soc_pcm_apply_symmetry() so have that be: > > struct snd_soc_pcm_runtime *rtd = substream->private_data; > > if (rtd->dai_link->symmetrict_rates) { > } > > instead. > OK, this will be done in patchset v2, thanks.
Hi sugar, Can the rockchip support more than 8 channels? Ideally, we would like to capture 16 channels and playback 16 channels simultaneously. Thanks, Caleb Sent from my iPhone > On Oct 7, 2015, at 1:01 AM, sugar <sugar.zhang@rock-chips.com> wrote: > > Sorry for late reply, we were in the National Day Holiday. > >> ? 10/1/2015 02:46, Mark Brown ??: >>> On Mon, Sep 28, 2015 at 04:16:12PM +0800, sugar wrote: >>> ? 9/24/2015 00:24, Mark Brown ??: >> >>>> My initial thought here is that the machine driver should be responsible >>>> for setting this and then the DAI driver should check to see if >>>> symmetric_rates are in use and configure itself appropriately. Is there >>>> a reason why this won't work here? >> >>> It's for i2s ip configuration, in the most situation, there is no need >>> to use this property, except one case: >> >>> In order to save gpio pins for other function use, we may use single >>> lrck(tx or rx) pin. of course, it depends on product design. when in i2s >>> slave mode, we need to configure this to share lrck with tx/rx inside i2s >>> logic. >> >>> symmetric_rates flag works fine on rockchip platform, but it can't cover the >>> above case. >> >>> Do you have any suggestion about this or maybe there is no need to upstream >>> this special part? >> >> What makes you say that the symmetric_rates flag can't be used to cover >> this case? What you describe above is hte normal reason for needing to >> enforce symmetric_rates. The driver should be able to check if the flag >> has been set just as well as the core is. > > Got it, How about the following modify? > > if (dai->symmetric_rates) > regmap_update_bits(i2s->regmap, I2S_CKR, > I2S_CKR_TRCM_MASK, > I2S_CKR_TRCM(val)); > > > _______________________________________________ > Alsa-devel mailing list > Alsa-devel@alsa-project.org > http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
Hi Caleb, we haven't support 16 channels capture and playback yet, would you mind to detail the use case? if must, we may support this feature on next generation chip. ? 10/7/2015 21:04, Caleb Crome ??: > Hi sugar, > Can the rockchip support more than 8 channels? Ideally, we would like to capture 16 channels and playback 16 channels simultaneously. > > Thanks, Caleb > > Sent from my iPhone
We make microphone arrays with lots of microphones. Some as few as 2, many with 7 channels, and a few with 14 or more.
Can you support 8 channels with 32 bits/sample? That would work, we can just set the codecs for 16-bit samples, which would be 256 bits/frame.
Hope you had a great golden week holiday!
Thanks,
Caleb Crome
Sent from my iPhone
> On Oct 7, 2015, at 6:47 PM, sugar <sugar.zhang@rock-chips.com> wrote:
>
> Hi Caleb,
>
> we haven't support 16 channels capture and playback yet, would you mind to detail the use case? if must, we may support this feature on next generation chip.
>
>> ? 10/7/2015 21:04, Caleb Crome ??:
>> Hi sugar,
>> Can the rockchip support more than 8 channels? Ideally, we would like to capture 16 channels and playback 16 channels simultaneously.
>>
>> Thanks, Caleb
>>
>> Sent from my iPhone
>
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
Yes, we can support 8 channels with 32bits/sample, thanks. ? 10/8/2015 10:36, Caleb Crome ??: > We make microphone arrays with lots of microphones. Some as few as 2, many with 7 channels, and a few with 14 or more. > > Can you support 8 channels with 32 bits/sample? That would work, we can just set the codecs for 16-bit samples, which would be 256 bits/frame. > > Hope you had a great golden week holiday! > > Thanks, > Caleb Crome > > > > > Sent from my iPhone
That will work! Do all your cortex-a parts support that? Thanks, Caleb Sent from my iPhone > On Oct 7, 2015, at 7:44 PM, sugar <sugar.zhang@rock-chips.com> wrote: > > Yes, we can support 8 channels with 32bits/sample, thanks. > >> ? 10/8/2015 10:36, Caleb Crome ??: >> We make microphone arrays with lots of microphones. Some as few as 2, many with 7 channels, and a few with 14 or more. >> >> Can you support 8 channels with 32 bits/sample? That would work, we can just set the codecs for 16-bit samples, which would be 256 bits/frame. >> >> Hope you had a great golden week holiday! >> >> Thanks, >> Caleb Crome >> >> >> >> >> Sent from my iPhone > > _______________________________________________ > Alsa-devel mailing list > Alsa-devel@alsa-project.org > http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
Only the new design support 8 channels capture now, the others support 2 channels capture. but all of them support 32bits/sample. On 10/8/2015 11:19, Caleb Crome wrote: > That will work! Do all your cortex-a parts support that? > > Thanks, Caleb > Sent from my iPhone
diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c index b936102..a8cb414 100644 --- a/sound/soc/rockchip/rockchip_i2s.c +++ b/sound/soc/rockchip/rockchip_i2s.c @@ -245,8 +245,34 @@ static int rockchip_i2s_hw_params(struct snd_pcm_substream *substream, return -EINVAL; } - regmap_update_bits(i2s->regmap, I2S_TXCR, I2S_TXCR_VDW_MASK, val); - regmap_update_bits(i2s->regmap, I2S_RXCR, I2S_RXCR_VDW_MASK, val); + switch (params_channels(params)) { + case 8: + val |= I2S_CHN_8; + break; + case 6: + val |= I2S_CHN_6; + break; + case 4: + val |= I2S_CHN_4; + break; + case 2: + val |= I2S_CHN_2; + break; + default: + dev_err(i2s->dev, "invalid channel: %d\n", + params_channels(params)); + return -EINVAL; + } + + if (substream->stream == SNDRV_PCM_STREAM_CAPTURE) + regmap_update_bits(i2s->regmap, I2S_RXCR, + I2S_RXCR_VDW_MASK | I2S_RXCR_CSR_MASK, + val); + else + regmap_update_bits(i2s->regmap, I2S_TXCR, + I2S_TXCR_VDW_MASK | I2S_TXCR_CSR_MASK, + val); + regmap_update_bits(i2s->regmap, I2S_DMACR, I2S_DMACR_TDL_MASK, I2S_DMACR_TDL(16)); regmap_update_bits(i2s->regmap, I2S_DMACR, I2S_DMACR_RDL_MASK, @@ -415,10 +441,12 @@ static const struct regmap_config rockchip_i2s_regmap_config = { static int rockchip_i2s_probe(struct platform_device *pdev) { + struct device_node *node = pdev->dev.of_node; struct rk_i2s_dev *i2s; struct resource *res; void __iomem *regs; int ret; + int val; i2s = devm_kzalloc(&pdev->dev, sizeof(*i2s), GFP_KERNEL); if (!i2s) { @@ -475,6 +503,22 @@ static int rockchip_i2s_probe(struct platform_device *pdev) goto err_pm_disable; } + /* refine capture channels */ + if (!of_property_read_u32(node, "rockchip,capture-channels", &val)) { + if (val >= 2 && val <= 8) + rockchip_i2s_dai.capture.channels_max = val; + else + rockchip_i2s_dai.capture.channels_max = 2; + } + + /* configure tx/rx lrck use mode */ + if (!of_property_read_u32(node, "rockchip,lrck-mode", &val)) { + if (val >= LRCK_TXRX && val <= LRCK_RX_SHARE) + regmap_update_bits(i2s->regmap, I2S_CKR, + I2S_CKR_TRCM_MASK, + I2S_CKR_TRCM(val)); + } + ret = devm_snd_soc_register_component(&pdev->dev, &rockchip_i2s_component, &rockchip_i2s_dai, 1); diff --git a/sound/soc/rockchip/rockchip_i2s.h b/sound/soc/rockchip/rockchip_i2s.h index 93f456f..0d285d1 100644 --- a/sound/soc/rockchip/rockchip_i2s.h +++ b/sound/soc/rockchip/rockchip_i2s.h @@ -49,6 +49,9 @@ * RXCR * receive operation control register */ +#define I2S_RXCR_CSR_SHIFT 15 +#define I2S_RXCR_CSR(x) (x << I2S_RXCR_CSR_SHIFT) +#define I2S_RXCR_CSR_MASK (3 << I2S_RXCR_CSR_SHIFT) #define I2S_RXCR_HWT BIT(14) #define I2S_RXCR_SJM_SHIFT 12 #define I2S_RXCR_SJM_R (0 << I2S_RXCR_SJM_SHIFT) @@ -75,6 +78,12 @@ * CKR * clock generation register */ +#define I2S_CKR_TRCM_SHIFT 28 +#define I2S_CKR_TRCM(x) (x << I2S_CKR_TRCM_SHIFT) +#define I2S_CKR_TRCM_TXRX (0 << I2S_CKR_TRCM_SHIFT) +#define I2S_CKR_TRCM_TXSHARE (1 << I2S_CKR_TRCM_SHIFT) +#define I2S_CKR_TRCM_RXSHARE (2 << I2S_CKR_TRCM_SHIFT) +#define I2S_CKR_TRCM_MASK (3 << I2S_CKR_TRCM_SHIFT) #define I2S_CKR_MSS_SHIFT 27 #define I2S_CKR_MSS_MASTER (0 << I2S_CKR_MSS_SHIFT) #define I2S_CKR_MSS_SLAVE (1 << I2S_CKR_MSS_SHIFT) @@ -207,6 +216,20 @@ enum { ROCKCHIP_DIV_BCLK, }; +/* channel select */ +#define I2S_CSR_SHIFT 15 +#define I2S_CHN_2 (0 << I2S_CSR_SHIFT) +#define I2S_CHN_4 (1 << I2S_CSR_SHIFT) +#define I2S_CHN_6 (2 << I2S_CSR_SHIFT) +#define I2S_CHN_8 (3 << I2S_CSR_SHIFT) + +/* lrck use mode */ +enum { + LRCK_TXRX = 0, + LRCK_TX_SHARE, + LRCK_RX_SHARE, +}; + /* I2S REGS */ #define I2S_TXCR (0x0000) #define I2S_RXCR (0x0004)
support max 8 channels capture, please add property 'rockchip,capture-channels' in dts to enable this, if not, support 2 channels capture default. support lrck clk mode configuration, there are 3 modes: - txrx: lrck_tx and lrck_rx are different. - tx_share: lrck_tx is shared with lrck_rx. - rx_share: lrck_rx is shared with lrck_tx. to enable this, please add property 'rockchip,lrck-mode' in dts, if not, use 'txrx' lrck mode default. Signed-off-by: Sugar Zhang <sugar.zhang@rock-chips.com> --- sound/soc/rockchip/rockchip_i2s.c | 48 +++++++++++++++++++++++++++++++++++++-- sound/soc/rockchip/rockchip_i2s.h | 23 +++++++++++++++++++ 2 files changed, 69 insertions(+), 2 deletions(-)