On Wed, Nov 10, 2010 at 8:35 PM, Jaroslav Kysela <perex@xxxxxxxx> wrote: > On Wed, 10 Nov 2010, Manu Abraham wrote: > >> On Wed, Nov 10, 2010 at 5:00 PM, Jaroslav Kysela <perex@xxxxxxxx> wrote: >>> >>> On Wed, 10 Nov 2010, Manu Abraham wrote: >>> >>>> Ok, so period should be the interval at which an IRQ is expected. >>>> ie, if i set a buffer size for 40mS, the period should be 40, I guess. >>>> >>>> That said, what should be a recommended value for period ? If hardware >>>> needs to generate an IRQ at the end of each period ? >>> >>> It's good to allow the lowledevel driver to pass all possible period >>> sizes >>> to the user space. The application does decision how many wakeups >>> expects. >> >> >> Ok, I adapted it to look thus, >> >> static struct snd_pcm_hardware saa7231_capture_info = { >> .info = (SNDRV_PCM_INFO_MMAP | >> SNDRV_PCM_INFO_INTERLEAVED | >> SNDRV_PCM_INFO_BLOCK_TRANSFER | >> SNDRV_PCM_INFO_MMAP_VALID), >> >> .formats = (SNDRV_PCM_FMTBIT_S16_LE | >> SNDRV_PCM_FMTBIT_S16_BE | >> SNDRV_PCM_FMTBIT_S8 | >> SNDRV_PCM_FMTBIT_U8 | >> SNDRV_PCM_FMTBIT_U16_LE | >> SNDRV_PCM_FMTBIT_U16_BE), >> >> .rates = SNDRV_PCM_RATE_32000, >> >> .rate_min = 32000, >> .rate_max = 48000, >> >> .channels_min = 2, >> .channels_max = 2, >> >> .buffer_bytes_max = 512 * 4096, >> #if 0 >> .period_bytes_min = 192, >> .period_bytes_max = 1536, >> .periods_min = 2, >> .periods_max = 1024, >> #endif >> .period_bytes_min = 1920, /* 10mS @ 48khz */ >> .period_bytes_max = 768000, /* 4S @ 48kHz */ >> >> .periods_min = 10, /* 10mS */ >> .periods_max = 4000000, /* 4S */ > > ??? 10 (period_bytes_min) * 10 (periods_min) = 100mS . Note that > period_bytes_* limits periods size, but periods_* count of periods per the > whole audio DMA ring buffer. Define smallest number of periods (it's usually > value one or two for periods_min). Ok. >> }; >> >> /* >> * 10mS Buffer lengths >> * @48kHz 1920 bytes >> * @44.1kHz 1764 bytes >> * @32kHz 1280 bytes >> * >> * >> * period min = 10mS @48k: 1920 bytes @44.1k: 1764 bytes @32k: 1280 >> bytes >> * period max = 1S @48k:192000 bytes @44.1k:176400 bytes @32k:128000 >> bytes >> * period = 4S @48k:768000 bytes @44.1k:705600 bytes @32k:512000 >> bytes >> */ >> static int saa7231_capture_open(struct snd_pcm_substream *pcm) >> { >> struct saa7231_audio *audio = snd_pcm_substream_chip(pcm); >> struct snd_pcm_runtime *rt = pcm->runtime; >> struct saa7231_dev *saa7231 = audio->saa7231; >> int err; >> >> dprintk(SAA7231_DEBUG, 1, "()"); >> rt->hw = saa7231_capture_info; >> snd_pcm_hw_constraint_list(rt, 0, SNDRV_PCM_HW_PARAM_RATE, >> &hw_constraint_rates); >> err = snd_pcm_hw_constraint_integer(rt, >> SNDRV_PCM_HW_PARAM_PERIODS); >> if (err < 0) { >> dprintk(SAA7231_ERROR, 1, "snd_pcm_hw_constraint_integer() >> failed. >> ret=%d", err); >> return err; >> } > >> err = snd_pcm_hw_constraint_minmax(rt, >> SNDRV_PCM_HW_PARAM_PERIOD_SIZE, 10, 4000000); >> if (err < 0) { >> dprintk(SAA7231_ERROR, 1, "snd_pcm_hw_constraint_minmax() >> failed. >> ret=%d", err); >> return err; >> } >> err = snd_pcm_hw_constraint_minmax(rt, >> SNDRV_PCM_HW_PARAM_PERIOD_BYTES, 1920, 768000); >> if (err < 0) { >> dprintk(SAA7231_ERROR, 1, "snd_pcm_hw_constraint_minmax() >> failed. >> ret=%d", err); >> return err; >> } > > Remove these minmax contrains. You already do limiting in hw->ops. Ok. >> return 0; >> } >> >> /* >> * saa7231_hw_params() >> * >> * This callback is called when the hardware parameter (hw_params) is setup >> * by the application, ie., once when the buffer size, the period size, >> * the format etc are defined for the PCM substream. >> */ >> static int saa7231_hw_params(struct snd_pcm_substream *pcm, struct >> snd_pcm_hw_params *params) >> { >> struct saa7231_audio *audio = snd_pcm_substream_chip(pcm); >> struct saa7231_dev *saa7231 = audio->saa7231; >> struct snd_pcm_runtime *rt = pcm->runtime; >> >> struct saa7231_dmabuf *buffer; >> struct saa7231_stream *stream; >> struct saa7231_dmabuf *dmabuf; >> >> struct page **ptable; >> >> void *mem; >> void *dma_area; >> >> int i, j, pages, bytes, periods, bufsiz, pt_size; >> >> dprintk(SAA7231_DEBUG, 1, "DEBUG: ()"); >> periods = params_periods(params); >> bytes = params_period_bytes(params); >> >> bufsiz = periods * bytes; >> dprintk(SAA7231_DEBUG, 1, "bufsiz=%d periods=%d bytes=%d", >> bufsiz, >> periods, >> bytes); >> >> pages = bufsiz / PAGE_SIZE; > > This is wrong. It should be 'pages = (bufsize + PAGE_SIZE - 1) / PAGE_SIZE'. Ok. >> /* enable stream */ >> /* initializing 8 buffer with "pages" pages each .. */ >> stream = saa7231_stream_init(saa7231, AUDIO_CAPTURE, ADAPTER_INT, >> 0, pages); >> if (!stream) { >> dprintk(SAA7231_ERROR, 1, "ERROR: Registering stream"); >> return -ENOMEM; >> } >> audio->stream = stream; >> buffer = stream->dmabuf; >> saa7231_add_irqevent(saa7231, 43, SAA7231_EDGE_RISING, >> saa7231_audio_evhandler, "AS2D_AVIS"); >> dprintk(SAA7231_DEBUG, 1, "Mapping %d buffers with %d pages each", >> XS2D_BUFFERS, pages); > > Unfortunately, I don't understand the role of XS2D_BUFFERS. The ALSA bufsize > is the whole DMA area (you should use params_buffer_bytes() to get this > value instead of calculating this using periods * period_size). Ok. The hardware has 8 SG buffers each. each of the buffer heads are setup on to the MMU. each of these buffers XS2D_BUFFERS can handle a maximum of 512 pages, ie page table for the SG buffer can be a maximum of 4096 bytes long. > It means: Just allocate number of pages required for buffer_bytes. Don't > play with periods (except the interrupts). > >> /* >> * For a single DMA buffer: >> * each page takes a u64 size in a page table >> * number of entries a single page can hold = PAGE_SIZE / entry >> size >> * ie entries.max = PAGE_SIZE / 8 => 4096/8 = 512 >> * Now, we have entries.req = 7 ("pages") per buffer >> * >> * For all in XS2D_BUFFERS: >> * page table should be large enough to hold all the pages in each >> DMA buffer >> * total number of pages = pages * XS2D_BUFFERS >> * max buffers that we need to consider = 512 * 8, this needs 8 >> pages >> for a page table >> * >> * On a general note, we can calculate pages for page table as >> * page_table_size = total_pages / 512, with a minimum of a single >> page >> */ >> >> #define MAX_ENTRIES_PER_PAGE (PAGE_SIZE / 8) >> >> pt_size = (pages * XS2D_BUFFERS) / MAX_ENTRIES_PER_PAGE; >> /* minimum 1 page required for the table */ >> if (pt_size < 1) >> pt_size = 1; >> >> dprintk(SAA7231_DEBUG, 1, "Page Table array size=%d", pt_size); >> ptable = kzalloc((sizeof (struct page) * pt_size), GFP_KERNEL); >> if (!ptable) { >> dprintk(SAA7231_ERROR, 1, "ERROR: No memory to allocate >> virtual map"); >> return -ENOMEM; >> } >> audio->ptable = ptable; >> >> for (i = 0; i < XS2D_BUFFERS; i ++) { >> dmabuf = &buffer[i]; >> mem = dmabuf->vmalloc; >> for (j = 0; j < pages; j++) >> ptable[j] = virt_to_page(mem); >> } > > This looks broken. ptable is overwritten for each XS2D_BUFFERS iteration. True. >> #if 0 >> dma_area = vmap(ptable, (pages * XS2D_BUFFERS), VM_MAP, >> PAGE_KERNEL); >> rt->dma_area = dma_area; >> rt->dma_bytes = XS2D_BUFFERS * pages * SAA7231_PAGE_SIZE; >> rt->dma_addr = 0; >> #endif >> return 0; >> } > > I think that the second argument is number of linux mem pages. Your value > seems strange. Uh, oh. I had assumed that each of the 8 XS2D_Buffers would contain "pages" each and hence.. Regards, Manu _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx http://mailman.alsa-project.org/mailman/listinfo/alsa-devel