Skip to content

Commit

Permalink
ALSA: pci: rme: Fix unaligned buffer addresses
Browse files Browse the repository at this point in the history 
The recent fix for setting up the DMA buffer type on RME drivers tried
to address the non-standard memory managements and changed the DMA
buffer information to the standard snd_dma_buffer object that is
allocated at the probe time.  However, I overlooked that the RME
drivers handle the buffer addresses based on 64k alignment, and the
previous conversion broke that silently.

This patch is an attempt to fix the regression.  The snd_dma_buffer
objects are copied to the original data with the correction to the
aligned accesses, and those are passed to snd_pcm_set_runtime_buffer()
helpers instead.  The original snd_dma_buffer objects are managed by
devres, hence they'll be released automagically.

Fixes: 0899a7a ("ALSA: pci: rme: Set up buffer type properly")
Cc: <stable@vger.kernel.org>
Link: https://lore.kernel.org/r/20211108145752.30572-1-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
  • Loading branch information
Takashi Iwai committed Nov 8, 2021
1 parent 411ac29 commit 43d35cc
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 36 deletions.
41 changes: 23 additions & 18 deletions sound/pci/rme9652/hdsp.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -468,8 +468,11 @@ struct hdsp {
unsigned char ss_out_channels;
u32 io_loopback; /* output loopback channel states*/

struct snd_dma_buffer *capture_dma_buf;
struct snd_dma_buffer *playback_dma_buf;
/* DMA buffers; those are copied instances from the original snd_dma_buf
* objects (which are managed via devres) for the address alignments
*/
struct snd_dma_buffer capture_dma_buf;
struct snd_dma_buffer playback_dma_buf;
unsigned char *capture_buffer; /* suitably aligned address */
unsigned char *playback_buffer; /* suitably aligned address */

Expand Down Expand Up @@ -3764,30 +3767,32 @@ static void snd_hdsp_proc_init(struct hdsp *hdsp)

static int snd_hdsp_initialize_memory(struct hdsp *hdsp)
{
unsigned long pb_bus, cb_bus;
struct snd_dma_buffer *capture_dma, *playback_dma;

hdsp->capture_dma_buf =
snd_hammerfall_get_buffer(hdsp->pci, HDSP_DMA_AREA_BYTES);
hdsp->playback_dma_buf =
snd_hammerfall_get_buffer(hdsp->pci, HDSP_DMA_AREA_BYTES);
if (!hdsp->capture_dma_buf || !hdsp->playback_dma_buf) {
capture_dma = snd_hammerfall_get_buffer(hdsp->pci, HDSP_DMA_AREA_BYTES);
playback_dma = snd_hammerfall_get_buffer(hdsp->pci, HDSP_DMA_AREA_BYTES);
if (!capture_dma || !playback_dma) {
dev_err(hdsp->card->dev,
"%s: no buffers available\n", hdsp->card_name);
return -ENOMEM;
}

/* Align to bus-space 64K boundary */
/* copy to the own data for alignment */
hdsp->capture_dma_buf = *capture_dma;
hdsp->playback_dma_buf = *playback_dma;

cb_bus = ALIGN(hdsp->capture_dma_buf->addr, 0x10000ul);
pb_bus = ALIGN(hdsp->playback_dma_buf->addr, 0x10000ul);
/* Align to bus-space 64K boundary */
hdsp->capture_dma_buf.addr = ALIGN(capture_dma->addr, 0x10000ul);
hdsp->playback_dma_buf.addr = ALIGN(playback_dma->addr, 0x10000ul);

/* Tell the card where it is */
hdsp_write(hdsp, HDSP_inputBufferAddress, hdsp->capture_dma_buf.addr);
hdsp_write(hdsp, HDSP_outputBufferAddress, hdsp->playback_dma_buf.addr);

hdsp_write(hdsp, HDSP_inputBufferAddress, cb_bus);
hdsp_write(hdsp, HDSP_outputBufferAddress, pb_bus);

hdsp->capture_buffer = hdsp->capture_dma_buf->area + (cb_bus - hdsp->capture_dma_buf->addr);
hdsp->playback_buffer = hdsp->playback_dma_buf->area + (pb_bus - hdsp->playback_dma_buf->addr);
hdsp->capture_dma_buf.area += hdsp->capture_dma_buf.addr - capture_dma->addr;
hdsp->playback_dma_buf.area += hdsp->playback_dma_buf.addr - playback_dma->addr;
hdsp->capture_buffer = hdsp->capture_dma_buf.area;
hdsp->playback_buffer = hdsp->playback_dma_buf.area;

return 0;
}
Expand Down Expand Up @@ -4507,7 +4512,7 @@ static int snd_hdsp_playback_open(struct snd_pcm_substream *substream)
snd_pcm_set_sync(substream);

runtime->hw = snd_hdsp_playback_subinfo;
snd_pcm_set_runtime_buffer(substream, hdsp->playback_dma_buf);
snd_pcm_set_runtime_buffer(substream, &hdsp->playback_dma_buf);

hdsp->playback_pid = current->pid;
hdsp->playback_substream = substream;
Expand Down Expand Up @@ -4583,7 +4588,7 @@ static int snd_hdsp_capture_open(struct snd_pcm_substream *substream)
snd_pcm_set_sync(substream);

runtime->hw = snd_hdsp_capture_subinfo;
snd_pcm_set_runtime_buffer(substream, hdsp->capture_dma_buf);
snd_pcm_set_runtime_buffer(substream, &hdsp->capture_dma_buf);

hdsp->capture_pid = current->pid;
hdsp->capture_substream = substream;
Expand Down
41 changes: 23 additions & 18 deletions sound/pci/rme9652/rme9652.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -208,8 +208,11 @@ struct snd_rme9652 {
unsigned char ds_channels;
unsigned char ss_channels; /* different for hammerfall/hammerfall-light */

struct snd_dma_buffer *playback_dma_buf;
struct snd_dma_buffer *capture_dma_buf;
/* DMA buffers; those are copied instances from the original snd_dma_buf
* objects (which are managed via devres) for the address alignments
*/
struct snd_dma_buffer playback_dma_buf;
struct snd_dma_buffer capture_dma_buf;

unsigned char *capture_buffer; /* suitably aligned address */
unsigned char *playback_buffer; /* suitably aligned address */
Expand Down Expand Up @@ -1719,30 +1722,32 @@ static void snd_rme9652_card_free(struct snd_card *card)

static int snd_rme9652_initialize_memory(struct snd_rme9652 *rme9652)
{
unsigned long pb_bus, cb_bus;
struct snd_dma_buffer *capture_dma, *playback_dma;

rme9652->capture_dma_buf =
snd_hammerfall_get_buffer(rme9652->pci, RME9652_DMA_AREA_BYTES);
rme9652->playback_dma_buf =
snd_hammerfall_get_buffer(rme9652->pci, RME9652_DMA_AREA_BYTES);
if (!rme9652->capture_dma_buf || !rme9652->playback_dma_buf) {
capture_dma = snd_hammerfall_get_buffer(rme9652->pci, RME9652_DMA_AREA_BYTES);
playback_dma = snd_hammerfall_get_buffer(rme9652->pci, RME9652_DMA_AREA_BYTES);
if (!capture_dma || !playback_dma) {
dev_err(rme9652->card->dev,
"%s: no buffers available\n", rme9652->card_name);
return -ENOMEM;
}

/* Align to bus-space 64K boundary */
/* copy to the own data for alignment */
rme9652->capture_dma_buf = *capture_dma;
rme9652->playback_dma_buf = *playback_dma;

cb_bus = ALIGN(rme9652->capture_dma_buf->addr, 0x10000ul);
pb_bus = ALIGN(rme9652->playback_dma_buf->addr, 0x10000ul);
/* Align to bus-space 64K boundary */
rme9652->capture_dma_buf.addr = ALIGN(capture_dma->addr, 0x10000ul);
rme9652->playback_dma_buf.addr = ALIGN(playback_dma->addr, 0x10000ul);

/* Tell the card where it is */
rme9652_write(rme9652, RME9652_rec_buffer, rme9652->capture_dma_buf.addr);
rme9652_write(rme9652, RME9652_play_buffer, rme9652->playback_dma_buf.addr);

rme9652_write(rme9652, RME9652_rec_buffer, cb_bus);
rme9652_write(rme9652, RME9652_play_buffer, pb_bus);

rme9652->capture_buffer = rme9652->capture_dma_buf->area + (cb_bus - rme9652->capture_dma_buf->addr);
rme9652->playback_buffer = rme9652->playback_dma_buf->area + (pb_bus - rme9652->playback_dma_buf->addr);
rme9652->capture_dma_buf.area += rme9652->capture_dma_buf.addr - capture_dma->addr;
rme9652->playback_dma_buf.area += rme9652->playback_dma_buf.addr - playback_dma->addr;
rme9652->capture_buffer = rme9652->capture_dma_buf.area;
rme9652->playback_buffer = rme9652->playback_dma_buf.area;

return 0;
}
Expand Down Expand Up @@ -2259,7 +2264,7 @@ static int snd_rme9652_playback_open(struct snd_pcm_substream *substream)
snd_pcm_set_sync(substream);

runtime->hw = snd_rme9652_playback_subinfo;
snd_pcm_set_runtime_buffer(substream, rme9652->playback_dma_buf);
snd_pcm_set_runtime_buffer(substream, &rme9652->playback_dma_buf);

if (rme9652->capture_substream == NULL) {
rme9652_stop(rme9652);
Expand Down Expand Up @@ -2318,7 +2323,7 @@ static int snd_rme9652_capture_open(struct snd_pcm_substream *substream)
snd_pcm_set_sync(substream);

runtime->hw = snd_rme9652_capture_subinfo;
snd_pcm_set_runtime_buffer(substream, rme9652->capture_dma_buf);
snd_pcm_set_runtime_buffer(substream, &rme9652->capture_dma_buf);

if (rme9652->playback_substream == NULL) {
rme9652_stop(rme9652);
Expand Down

0 comments on commit 43d35cc

Please sign in to comment.