[alsa-devel] Allocating DMA buffer for non-PCM
Hi!
Would you say that the following is the proper way to allocate a DMA buffer used to hold level data?
struct hdspm { struct pci_dev *pci; /* and an pci info */ struct snd_dma_buffer dmaLevelBuffer; u32 *level_buffer; /* suitably aligned address */ [..] }
/* allocate level buffer */ err = snd_dma_alloc_pages(SNDRV_DMA_TYPE_DEV_SG, snd_dma_pci_data(hdspm->pci), MADIFX_LEVEL_BUFFER_SIZE, &hdspm->dmaLevelBuffer); if (err < 0) { /* error */ [..] }
hdspm->level_buffer = snd_sgbuf_get_ptr(&(hdspm->dmaLevelBuffer), 0);
memset(hdspm->level_buffer, 0, MADIFX_LEVEL_BUFFER_SIZE);
This used to work on my development machine (kernel 3.6.x), but now a kernel 3.2.0 user reports a NULL pointer dereference of hdspm->level_buffer, so apparently, snd_sgbuf_get_ptr() returned NULL for him.
How could this possibly happen? Am I missing something? Better use SNDRV_DMA_TYPE_DEV instead?
TIA
At Thu, 14 Feb 2013 18:03:53 +0100, Adrian Knoth wrote:
Hi!
Would you say that the following is the proper way to allocate a DMA buffer used to hold level data?
struct hdspm { struct pci_dev *pci; /* and an pci info */ struct snd_dma_buffer dmaLevelBuffer; u32 *level_buffer; /* suitably aligned address */ [..] }
/* allocate level buffer */ err = snd_dma_alloc_pages(SNDRV_DMA_TYPE_DEV_SG, snd_dma_pci_data(hdspm->pci), MADIFX_LEVEL_BUFFER_SIZE, &hdspm->dmaLevelBuffer); if (err < 0) { /* error */ [..] }
hdspm->level_buffer = snd_sgbuf_get_ptr(&(hdspm->dmaLevelBuffer), 0);
hdspm->level_buffer = (u32*)hdspm.dmaLevelBuffer.area;
memset(hdspm->level_buffer, 0, MADIFX_LEVEL_BUFFER_SIZE);
This used to work on my development machine (kernel 3.6.x), but now a kernel 3.2.0 user reports a NULL pointer dereference of hdspm->level_buffer, so apparently, snd_sgbuf_get_ptr() returned NULL for him.
What's level_buffer? Is a SG-buffer for the audio stream?
How could this possibly happen?
A wrong usage :)
Takashi
Am I missing something? Better use SNDRV_DMA_TYPE_DEV instead?
TIA _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
On 02/14/2013 06:14 PM, Takashi Iwai wrote:
/* allocate level buffer */ err = snd_dma_alloc_pages(SNDRV_DMA_TYPE_DEV_SG, snd_dma_pci_data(hdspm->pci), MADIFX_LEVEL_BUFFER_SIZE, &hdspm->dmaLevelBuffer); if (err < 0) { /* error */ [..] }
hdspm->level_buffer = snd_sgbuf_get_ptr(&(hdspm->dmaLevelBuffer), 0);hdspm->level_buffer = (u32*)hdspm.dmaLevelBuffer.area;
Looks good, TNX.
This used to work on my development machine (kernel 3.6.x), but now a kernel 3.2.0 user reports a NULL pointer dereference of hdspm->level_buffer, so apparently, snd_sgbuf_get_ptr() returned NULL for him.
What's level_buffer?
The card does hardware metering and stores RMS/peak values in a DMA buffer. I want to later pass this to userspace to show signal levels, either via memcpy()ing the DMA buffer or maybe via mmap(). But since I no longer have access to such a card, work on this is on halt atm.
Is a SG-buffer for the audio stream?
The audio buffers use
snd_pcm_lib_preallocate_pages_for_all(pcm, SNDRV_DMA_TYPE_DEV_SG, snd_dma_pci_data(hdspm->pci),
so they're SG, yes.
How could this possibly happen?
A wrong usage :)
I thought so, that's why I was asking. ;)
Cheers and thanks again
At Thu, 14 Feb 2013 18:26:53 +0100, Adrian Knoth wrote:
On 02/14/2013 06:14 PM, Takashi Iwai wrote:
/* allocate level buffer */ err = snd_dma_alloc_pages(SNDRV_DMA_TYPE_DEV_SG, snd_dma_pci_data(hdspm->pci), MADIFX_LEVEL_BUFFER_SIZE, &hdspm->dmaLevelBuffer); if (err < 0) { /* error */ [..] }
hdspm->level_buffer = snd_sgbuf_get_ptr(&(hdspm->dmaLevelBuffer), 0);hdspm->level_buffer = (u32*)hdspm.dmaLevelBuffer.area;
Looks good, TNX.
This used to work on my development machine (kernel 3.6.x), but now a kernel 3.2.0 user reports a NULL pointer dereference of hdspm->level_buffer, so apparently, snd_sgbuf_get_ptr() returned NULL for him.
What's level_buffer?
The card does hardware metering and stores RMS/peak values in a DMA buffer. I want to later pass this to userspace to show signal levels, either via memcpy()ing the DMA buffer or maybe via mmap(). But since I no longer have access to such a card, work on this is on halt atm.
Is a SG-buffer for the audio stream?
The audio buffers use
snd_pcm_lib_preallocate_pages_for_all(pcm, SNDRV_DMA_TYPE_DEV_SG, snd_dma_pci_data(hdspm->pci),so they're SG, yes.
But isn't the level meter buffer a single page? If the buffer is only for peak meters, it can't be that big even for multi-channel cards. If so, SNDRV_DMA_TYPE_DEV_SG is utterly nonsense. Otherwise, if it's a SG buffer, you'll have to some code to set up TLB in anyway.
Takashi
Adrian Knoth wrote:
Would you say that the following is the proper way to allocate a DMA buffer used to hold level data?
err = snd_dma_alloc_pages(SNDRV_DMA_TYPE_DEV_SG, snd_dma_pci_data(hdspm->pci), MADIFX_LEVEL_BUFFER_SIZE, &hdspm->dmaLevelBuffer);
hdspm->level_buffer = snd_sgbuf_get_ptr(&(hdspm->dmaLevelBuffer), 0);
I don't see the code asking for the address of the second page, so I guess there isn't one. But then you don't need SG in the first place.
Better use SNDRV_DMA_TYPE_DEV instead?
Yes.
Regards, Clemens
On Thu, Feb 14, 2013 at 06:46:30PM +0100, Clemens Ladisch wrote:
Would you say that the following is the proper way to allocate a DMA buffer used to hold level data?
err = snd_dma_alloc_pages(SNDRV_DMA_TYPE_DEV_SG, snd_dma_pci_data(hdspm->pci), MADIFX_LEVEL_BUFFER_SIZE, &hdspm->dmaLevelBuffer);
hdspm->level_buffer = snd_sgbuf_get_ptr(&(hdspm->dmaLevelBuffer), 0);I don't see the code asking for the address of the second page, so I guess there isn't one.
Exactly.
Just to be sure: Takashi has recommended to use (u32*)dmaLevelBuffer.area. Even in the case of SG buffers, is this virtual address continuous and safe for memset?
Cheers
Adrian Knoth wrote:
On Thu, Feb 14, 2013 at 06:46:30PM +0100, Clemens Ladisch wrote:
Would you say that the following is the proper way to allocate a DMA buffer used to hold level data?
err = snd_dma_alloc_pages(SNDRV_DMA_TYPE_DEV_SG, snd_dma_pci_data(hdspm->pci), MADIFX_LEVEL_BUFFER_SIZE, &hdspm->dmaLevelBuffer);
hdspm->level_buffer = snd_sgbuf_get_ptr(&(hdspm->dmaLevelBuffer), 0);I don't see the code asking for the address of the second page, so I guess there isn't one.
Exactly.
So MADIFX_LEVEL_BUFFER_SIZE is guaranteed to not exceed the page size on all architectures?
Just to be sure: Takashi has recommended to use (u32*)dmaLevelBuffer.area. Even in the case of SG buffers, is this virtual address continuous and safe for memset?
Yes, the SG buffer's pages are vmap()ed there.
Regards, Clemens
On 02/14/2013 07:01 PM, Clemens Ladisch wrote:
On Thu, Feb 14, 2013 at 06:46:30PM +0100, Clemens Ladisch wrote:
Would you say that the following is the proper way to allocate a DMA buffer used to hold level data?
err = snd_dma_alloc_pages(SNDRV_DMA_TYPE_DEV_SG, snd_dma_pci_data(hdspm->pci), MADIFX_LEVEL_BUFFER_SIZE, &hdspm->dmaLevelBuffer);
hdspm->level_buffer = snd_sgbuf_get_ptr(&(hdspm->dmaLevelBuffer), 0);I don't see the code asking for the address of the second page, so I guess there isn't one.
Exactly.
So MADIFX_LEVEL_BUFFER_SIZE is guaranteed to not exceed the page size on all architectures?
Absolutely not, it's known to be multiples of the page size, but the purpose of hdspm->level_buffer is to hold the continuous virtual address, so apparently, I was using the wrong getter function in the first place (I was actually looking for .area).
The code for accessing the other pages was omitted for the sake of simplicity, but since you've asked, here it is:
/* Fill level page table */ for (i = 0; i < MADIFX_NUM_LEVEL_PAGES; i++) { levelPageTable[i] = snd_sgbuf_get_addr(&(hdspm->dmaLevelBuffer), i * MADIFX_HARDWARE_PAGE_SIZE);
}
/* Write level page table to device */ lpti = (MADIFX == hdspm->io_type) ? MADIFX_LPTI_HMFX : MADIFX_LPTI_MFXT;
for (i = 0; i < MADIFX_NUM_LEVEL_PAGES; i++) { madifx_write(hdspm, MADIFX_PAGE_ADDRESS_LIST + (4 * (lpti + i)), levelPageTable[i]); }
Cheers
At Thu, 14 Feb 2013 19:46:39 +0100, Adrian Knoth wrote:
On 02/14/2013 07:01 PM, Clemens Ladisch wrote:
On Thu, Feb 14, 2013 at 06:46:30PM +0100, Clemens Ladisch wrote:
Would you say that the following is the proper way to allocate a DMA buffer used to hold level data?
err = snd_dma_alloc_pages(SNDRV_DMA_TYPE_DEV_SG, snd_dma_pci_data(hdspm->pci), MADIFX_LEVEL_BUFFER_SIZE, &hdspm->dmaLevelBuffer);
hdspm->level_buffer = snd_sgbuf_get_ptr(&(hdspm->dmaLevelBuffer), 0);I don't see the code asking for the address of the second page, so I guess there isn't one.
Exactly.
So MADIFX_LEVEL_BUFFER_SIZE is guaranteed to not exceed the page size on all architectures?
Absolutely not, it's known to be multiples of the page size, but the purpose of hdspm->level_buffer is to hold the continuous virtual address, so apparently, I was using the wrong getter function in the first place (I was actually looking for .area).
OK, then disregard my previous comment. The SG-buffer is the right thing.
FWIW, snd_sgbuf_get_ptr() return the pointer of the particular page. If you access only the data in the page size, it'll work even with that.
Takashi
The code for accessing the other pages was omitted for the sake of simplicity, but since you've asked, here it is:
/* Fill level page table */for (i = 0; i < MADIFX_NUM_LEVEL_PAGES; i++) { levelPageTable[i] = snd_sgbuf_get_addr(&(hdspm->dmaLevelBuffer), i * MADIFX_HARDWARE_PAGE_SIZE);
}
/* Write level page table to device */ lpti = (MADIFX == hdspm->io_type) ? MADIFX_LPTI_HMFX : MADIFX_LPTI_MFXT;
for (i = 0; i < MADIFX_NUM_LEVEL_PAGES; i++) { madifx_write(hdspm, MADIFX_PAGE_ADDRESS_LIST + (4 * (lpti + i)), levelPageTable[i]); }
Cheers _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
participants (3)
-
Adrian Knoth -
Clemens Ladisch -
Takashi Iwai