Mark Brown wrote:
On Sat, Jul 04, 2009 at 07:30:01PM -0700, Troy Kisky wrote: If you mean that it should start and stop the clocks
Yes.
that causes issues in situations like TDM since there can be transfers going on independantly of the CPU which may need the clocks running. Not everything will be able to gate the clocks, too.
What is TDM? time division multiplexing? You mean the frame carries more than just audio data? I think I'm misunderstanding something. Can you elaborate please.
In any case, this looks like a separate patch that should be split out of this one.
- if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
/* reading ram before asp should be safe* as long as the asp transfers less than a ping size* of bytes between the 2 reads*/edma_get_position(prtd->ram_master_lch,&ram_src, &ram_dst);edma_get_position(prtd->asp_master_lch,&asp_src, &asp_dst);count_asp = asp_src - prtd->asp_params.src;count_ram = ram_src - prtd->ram_params.src;mod_ram = count_ram % period_size;mod_ram -= count_asp;Is it perhaps saner to just look at the current position as being the position of the transfer to SRAM? This does mean more variation from the point at which data is audibly playing but on the other hand as far as the CPU is concerned once the audio gets to SRAM it can't work with it any longer so it's potentially misleading to report the SRAM position. Not all hardware will be able to report the position at all so userspace ought to be able to cope.
Hmm. Good point. I just wanted as accurate lip-sync as possible.
- iram_virt = sram_alloc(iram_size, &iram_phys);
- if (!iram_virt)
goto exit2;Should this perhaps be configured via platform data? You've currently picked 7 * 512 bytes but there appears to be no urgent reason for that particular number and presumably SRAM is a limited resource which people may want to prioritise differently. That may mean having bigger buffers for audio as well as less - things like MP3 players can get better battery life by setting up very big DMAs. On a similar vein is it worth considering making this feature entirely optional and/or falling back to non-SRAM if SRAM allocation fails?
Yes Chris Ring also asked for that, and it is fairly easy to allocate another SDRAM buffer to use for the SRAM. But I'd rather another patch after this if I need to use plaform data. Maybe audio_sram_buffer_size = 0 in platform data to mean use SDRAM?