[PATCH] ASoC: fsl_sai: Use physical format width
Slot width should follow the physical width of the format instead of the data width.
This is needed for formats like SNDRV_PCM_FMTBIT_S24_LE where physical width is 32 and data width is 24. By using the physical width, data won't get misaligned.
Signed-off-by: Emil Svendsen emas@bang-olufsen.dk --- sound/soc/fsl/fsl_sai.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c index 939c6bdd22c4..213e2d462076 100644 --- a/sound/soc/fsl/fsl_sai.c +++ b/sound/soc/fsl/fsl_sai.c @@ -519,13 +519,13 @@ static int fsl_sai_hw_params(struct snd_pcm_substream *substream, unsigned int channels = params_channels(params); struct snd_dmaengine_dai_dma_data *dma_params; struct fsl_sai_dl_cfg *dl_cfg = sai->dl_cfg; + u32 slot_width = params_physical_width(params); u32 word_width = params_width(params); int trce_mask = 0, dl_cfg_idx = 0; int dl_cfg_cnt = sai->dl_cfg_cnt; u32 dl_type = FSL_SAI_DL_I2S; u32 val_cr4 = 0, val_cr5 = 0; u32 slots = (channels == 1) ? 2 : channels; - u32 slot_width = word_width; int adir = tx ? RX : TX; u32 pins, bclk; u32 watermark;
On Thu, Mar 30, 2023 at 4:30 PM Emil Abildgaard Svendsen < EMAS@bang-olufsen.dk> wrote:
Slot width should follow the physical width of the format instead of the data width.
This is needed for formats like SNDRV_PCM_FMTBIT_S24_LE where physical width is 32 and data width is 24. By using the physical width, data won't get misaligned.
There are different requirements for this slot width. Some need physical width, Some need format width. We need to be careful about change here.
Actually there is .set_tdm_slot API for slot specific setting, please use this API.
best regards wang shengjiu
Signed-off-by: Emil Svendsen emas@bang-olufsen.dk
sound/soc/fsl/fsl_sai.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c index 939c6bdd22c4..213e2d462076 100644 --- a/sound/soc/fsl/fsl_sai.c +++ b/sound/soc/fsl/fsl_sai.c @@ -519,13 +519,13 @@ static int fsl_sai_hw_params(struct snd_pcm_substream *substream, unsigned int channels = params_channels(params); struct snd_dmaengine_dai_dma_data *dma_params; struct fsl_sai_dl_cfg *dl_cfg = sai->dl_cfg;
u32 slot_width = params_physical_width(params); u32 word_width = params_width(params); int trce_mask = 0, dl_cfg_idx = 0; int dl_cfg_cnt = sai->dl_cfg_cnt; u32 dl_type = FSL_SAI_DL_I2S; u32 val_cr4 = 0, val_cr5 = 0; u32 slots = (channels == 1) ? 2 : channels;
u32 slot_width = word_width; int adir = tx ? RX : TX; u32 pins, bclk; u32 watermark;-- 2.34.1
On 3/31/23 04:55, Shengjiu Wang wrote:
On Thu, Mar 30, 2023 at 4:30 PM Emil Abildgaard Svendsen < EMAS@bang-olufsen.dk> wrote:
Slot width should follow the physical width of the format instead of the data width.
This is needed for formats like SNDRV_PCM_FMTBIT_S24_LE where physical width is 32 and data width is 24. By using the physical width, data won't get misaligned.
There are different requirements for this slot width. Some need physical width, Some need format width. We need to be careful about change here.
I might be wrong but shouldn't physical width always correspond to what can be physically measured. If you need the slot width, the same as format width you use a format with matching widths like SNDRV_PCM_FORMAT_S24_3LE?
Actually there is .set_tdm_slot API for slot specific setting, please use this API.
But if you're using the generic sound cards. You need to add it to the DTS. Thus, if you want a static TDM slot width, all is fine. But if you want to change slot width runtime then it isn't enough. Unless I missed something.
Kind regards, Emil Svendsen
best regards wang shengjiu
Signed-off-by: Emil Svendsen emas@bang-olufsen.dk
sound/soc/fsl/fsl_sai.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c index 939c6bdd22c4..213e2d462076 100644 --- a/sound/soc/fsl/fsl_sai.c +++ b/sound/soc/fsl/fsl_sai.c @@ -519,13 +519,13 @@ static int fsl_sai_hw_params(struct snd_pcm_substream *substream, unsigned int channels = params_channels(params); struct snd_dmaengine_dai_dma_data *dma_params; struct fsl_sai_dl_cfg *dl_cfg = sai->dl_cfg;
u32 slot_width = params_physical_width(params); u32 word_width = params_width(params); int trce_mask = 0, dl_cfg_idx = 0; int dl_cfg_cnt = sai->dl_cfg_cnt; u32 dl_type = FSL_SAI_DL_I2S; u32 val_cr4 = 0, val_cr5 = 0; u32 slots = (channels == 1) ? 2 : channels;
u32 slot_width = word_width; int adir = tx ? RX : TX; u32 pins, bclk; u32 watermark;-- 2.34.1
On Fri, Mar 31, 2023 at 02:26:33PM +0000, Emil Abildgaard Svendsen wrote:
On 3/31/23 04:55, Shengjiu Wang wrote:
There are different requirements for this slot width. Some need physical width, Some need format width. We need to be careful about change here.
I might be wrong but shouldn't physical width always correspond to what can be physically measured. If you need the slot width, the same as format width you use a format with matching widths like SNDRV_PCM_FORMAT_S24_3LE?
The point is more that things are likely to be relying on the current behaviour, for example CODECs that don't actually support 24 bit audio due to a power of two requirement but cope fine when you play 24 bit audio in a 32 bit timeslot. This creates issues changing things even if the new behaviour is in some sense better. This is one of the unfortunate consequences of DT.
On 3/31/23 16:34, Mark Brown wrote:
On Fri, Mar 31, 2023 at 02:26:33PM +0000, Emil Abildgaard Svendsen wrote:
On 3/31/23 04:55, Shengjiu Wang wrote:
There are different requirements for this slot width. Some need physical width, Some need format width. We need to be careful about change here.
I might be wrong but shouldn't physical width always correspond to what can be physically measured. If you need the slot width, the same as format width you use a format with matching widths like SNDRV_PCM_FORMAT_S24_3LE?
The point is more that things are likely to be relying on the current behaviour, for example CODECs that don't actually support 24 bit audio due to a power of two requirement but cope fine when you play 24 bit audio in a 32 bit timeslot. This creates issues changing things even if the new behaviour is in some sense better. This is one of the unfortunate consequences of DT.
I guess if you want to do it runtime you have to create an audio card that will do it for you. I would have preferred it to be handled as close to hardware as possible. But that's a good enough compromise for not breaking current behavior.
Thanks!
Kind regards, Emil Svendsen
participants (3)
-
Emil Abildgaard Svendsen -
Mark Brown -
Shengjiu Wang