[alsa-devel] [PATCH 00/14] Revised patchset for PCM in-kernel copy support
Hi,
this is a sort of RFC, a revised patchset of the previous two patches for killing set_fs() usages in PCM. Instead of converting to the merged copy_silence ops, this adds a new copy_kernel ops instead. At the same time, copy/silence are changed to receive the position and size in bytes instead of frames. This allows us to simplify the PCM core code. As a result, a good amount of code could be removed from pcm_lib.c.
Note that this patchset isn't complete, containing only two driver conversions (nm256 for interleaved and rme9652 for non-interleaved) as a demonstration.
Takashi
===
Takashi Iwai (14): ALSA: pcm: Introduce copy_user, copy_kernel and fill_silence ops ALSA: nm256: Convert to new PCM copy ops ALSA: rme9652: Convert to the new PCM ops ALSA: pcm: Drop the old copy and silence ops ALSA: pcm: Check PCM state by a common helper function ALSA: pcm: Shuffle codes ALSA: pcm: Call directly the common read/write helpers ALSA: pcm: More unification of PCM transfer codes ALSA: pcm: Unify read/write loop ALSA: pcm: Simplify snd_pcm_playback_silence() ALSA: pcm: Direct in-kernel read/write support usb: gadget: u_uac1: Kill set_fs() usage ALSA: pcm: Kill set_fs() in PCM OSS layer ALSA: pcm: Build OSS writev/readv helpers conditionally
drivers/usb/gadget/function/u_uac1.c | 7 +- include/sound/pcm.h | 80 ++++- sound/core/oss/io.c | 4 +- sound/core/oss/pcm_oss.c | 81 +---- sound/core/oss/pcm_plugin.h | 6 +- sound/core/pcm_lib.c | 551 +++++++++++++---------------------- sound/pci/nm256/nm256.c | 57 ++-- sound/pci/rme9652/rme9652.c | 71 +++-- sound/soc/soc-pcm.c | 5 +- 9 files changed, 386 insertions(+), 476 deletions(-)
For supporting the explicit in-kernel copy of PCM buffer data, and also for further code refactoring, three new PCM ops, copy_user, copy_kernel and fill_silence, are introduced. The old copy and silence ops will be deprecated and removed later.
The new ops take bytes instead of frames, so that the driver can call directly memcpy() & co.
As of this stage, copy_kernel ops isn't used yet, but only copy_user is used.
Signed-off-by: Takashi Iwai tiwai@suse.de --- include/sound/pcm.h | 7 +++++ sound/core/pcm_lib.c | 89 +++++++++++++++++++++++++++++++++++++++++++--------- sound/soc/soc-pcm.c | 3 ++ 3 files changed, 84 insertions(+), 15 deletions(-)
diff --git a/include/sound/pcm.h b/include/sound/pcm.h index c609b891c4c2..a065415191d8 100644 --- a/include/sound/pcm.h +++ b/include/sound/pcm.h @@ -83,6 +83,13 @@ struct snd_pcm_ops { void __user *buf, snd_pcm_uframes_t count); int (*silence)(struct snd_pcm_substream *substream, int channel, snd_pcm_uframes_t pos, snd_pcm_uframes_t count); + int (*fill_silence)(struct snd_pcm_substream *substream, int channel, + unsigned long pos, unsigned long bytes); + int (*copy_user)(struct snd_pcm_substream *substream, int channel, + unsigned long pos, void __user *buf, + unsigned long bytes); + int (*copy_kernel)(struct snd_pcm_substream *substream, int channel, + unsigned long pos, void *buf, unsigned long bytes); struct page *(*page)(struct snd_pcm_substream *substream, unsigned long offset); int (*mmap)(struct snd_pcm_substream *substream, struct vm_area_struct *vma); diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c index ab4b1d1e44ee..9334fc2c20c8 100644 --- a/sound/core/pcm_lib.c +++ b/sound/core/pcm_lib.c @@ -55,6 +55,8 @@ void snd_pcm_playback_silence(struct snd_pcm_substream *substream, snd_pcm_ufram { struct snd_pcm_runtime *runtime = substream->runtime; snd_pcm_uframes_t frames, ofs, transfer; + char *hwbuf; + int err;
if (runtime->silence_size < runtime->boundary) { snd_pcm_sframes_t noise_dist, n; @@ -109,27 +111,37 @@ void snd_pcm_playback_silence(struct snd_pcm_substream *substream, snd_pcm_ufram transfer = ofs + frames > runtime->buffer_size ? runtime->buffer_size - ofs : frames; if (runtime->access == SNDRV_PCM_ACCESS_RW_INTERLEAVED || runtime->access == SNDRV_PCM_ACCESS_MMAP_INTERLEAVED) { - if (substream->ops->silence) { - int err; + if (substream->ops->fill_silence) { + err = substream->ops->fill_silence(substream, 0, + frames_to_bytes(runtime, ofs), + frames_to_bytes(runtime, transfer)); + snd_BUG_ON(err < 0); + } else if (substream->ops->silence) { err = substream->ops->silence(substream, -1, ofs, transfer); snd_BUG_ON(err < 0); } else { - char *hwbuf = runtime->dma_area + frames_to_bytes(runtime, ofs); + hwbuf = runtime->dma_area + frames_to_bytes(runtime, ofs); snd_pcm_format_set_silence(runtime->format, hwbuf, transfer * runtime->channels); } } else { unsigned int c; unsigned int channels = runtime->channels; - if (substream->ops->silence) { + if (substream->ops->fill_silence) { + for (c = 0; c < channels; ++c) { + err = substream->ops->fill_silence(substream, c, + samples_to_bytes(runtime, ofs), + samples_to_bytes(runtime, transfer)); + snd_BUG_ON(err < 0); + } + } else if (substream->ops->silence) { for (c = 0; c < channels; ++c) { - int err; err = substream->ops->silence(substream, c, ofs, transfer); snd_BUG_ON(err < 0); } } else { size_t dma_csize = runtime->dma_bytes / channels; for (c = 0; c < channels; ++c) { - char *hwbuf = runtime->dma_area + (c * dma_csize) + samples_to_bytes(runtime, ofs); + hwbuf = runtime->dma_area + (c * dma_csize) + samples_to_bytes(runtime, ofs); snd_pcm_format_set_silence(runtime->format, hwbuf, transfer); } } @@ -1995,7 +2007,13 @@ static int snd_pcm_lib_write_transfer(struct snd_pcm_substream *substream, struct snd_pcm_runtime *runtime = substream->runtime; int err; char __user *buf = (char __user *) data + frames_to_bytes(runtime, off); - if (substream->ops->copy) { + if (substream->ops->copy_user) { + hwoff = frames_to_bytes(runtime, hwoff); + frames = frames_to_bytes(runtime, frames); + err = substream->ops->copy_user(substream, 0, hwoff, buf, frames); + if (err < 0) + return err; + } else if (substream->ops->copy) { if ((err = substream->ops->copy(substream, -1, hwoff, buf, frames)) < 0) return err; } else { @@ -2119,7 +2137,8 @@ static int pcm_sanity_check(struct snd_pcm_substream *substream) if (PCM_RUNTIME_CHECK(substream)) return -ENXIO; runtime = substream->runtime; - if (snd_BUG_ON(!substream->ops->copy && !runtime->dma_area)) + if (snd_BUG_ON(!substream->ops->copy_user && !substream->ops->copy + && !runtime->dma_area)) return -EINVAL; if (runtime->status->state == SNDRV_PCM_STATE_OPEN) return -EBADFD; @@ -2156,8 +2175,30 @@ static int snd_pcm_lib_writev_transfer(struct snd_pcm_substream *substream, int err; void __user **bufs = (void __user **)data; int channels = runtime->channels; + char __user *buf; int c; - if (substream->ops->copy) { + + if (substream->ops->copy_user) { + hwoff = samples_to_bytes(runtime, hwoff); + off = samples_to_bytes(runtime, off); + frames = samples_to_bytes(runtime, frames); + for (c = 0; c < channels; ++c, ++bufs) { + buf = *bufs + off; + if (!*bufs) { + if (snd_BUG_ON(!substream->ops->fill_silence)) + return -EINVAL; + err = substream->ops->fill_silence(substream, c, + hwoff, + frames); + } else { + err = substream->ops->copy_user(substream, c, + hwoff, buf, + frames); + } + if (err < 0) + return err; + } + } else if (substream->ops->copy) { if (snd_BUG_ON(!substream->ops->silence)) return -EINVAL; for (c = 0; c < channels; ++c, ++bufs) { @@ -2165,7 +2206,7 @@ static int snd_pcm_lib_writev_transfer(struct snd_pcm_substream *substream, if ((err = substream->ops->silence(substream, c, hwoff, frames)) < 0) return err; } else { - char __user *buf = *bufs + samples_to_bytes(runtime, off); + buf = *bufs + samples_to_bytes(runtime, off); if ((err = substream->ops->copy(substream, c, hwoff, buf, frames)) < 0) return err; } @@ -2217,7 +2258,13 @@ static int snd_pcm_lib_read_transfer(struct snd_pcm_substream *substream, struct snd_pcm_runtime *runtime = substream->runtime; int err; char __user *buf = (char __user *) data + frames_to_bytes(runtime, off); - if (substream->ops->copy) { + if (substream->ops->copy_user) { + hwoff = frames_to_bytes(runtime, hwoff); + frames = frames_to_bytes(runtime, frames); + err = substream->ops->copy_user(substream, 0, hwoff, buf, frames); + if (err < 0) + return err; + } else if (substream->ops->copy) { if ((err = substream->ops->copy(substream, -1, hwoff, buf, frames)) < 0) return err; } else { @@ -2365,10 +2412,24 @@ static int snd_pcm_lib_readv_transfer(struct snd_pcm_substream *substream, int err; void __user **bufs = (void __user **)data; int channels = runtime->channels; + char __user *buf; + char *hwbuf; int c; - if (substream->ops->copy) { + + if (substream->ops->copy_user) { + hwoff = samples_to_bytes(runtime, hwoff); + off = samples_to_bytes(runtime, off); + frames = samples_to_bytes(runtime, frames); + for (c = 0; c < channels; ++c, ++bufs) { + if (!*bufs) + continue; + err = substream->ops->copy_user(substream, c, hwoff, + *bufs + off, frames); + if (err < 0) + return err; + } + } else if (substream->ops->copy) { for (c = 0; c < channels; ++c, ++bufs) { - char __user *buf; if (*bufs == NULL) continue; buf = *bufs + samples_to_bytes(runtime, off); @@ -2378,8 +2439,6 @@ static int snd_pcm_lib_readv_transfer(struct snd_pcm_substream *substream, } else { snd_pcm_uframes_t dma_csize = runtime->dma_bytes / channels; for (c = 0; c < channels; ++c, ++bufs) { - char *hwbuf; - char __user *buf; if (*bufs == NULL) continue;
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index efc5831f205d..8867ed9e5f56 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -2743,6 +2743,9 @@ int soc_new_pcm(struct snd_soc_pcm_runtime *rtd, int num)
if (platform->driver->ops) { rtd->ops.ack = platform->driver->ops->ack; + rtd->ops.copy_user = platform->driver->ops->copy_user; + rtd->ops.copy_kernel = platform->driver->ops->copy_kernel; + rtd->ops.fill_silence = platform->driver->ops->fill_silence; rtd->ops.copy = platform->driver->ops->copy; rtd->ops.silence = platform->driver->ops->silence; rtd->ops.page = platform->driver->ops->page;
Replace the copy and the silence ops with the new merged ops. The conversion is straightforward with standard helper functions.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/pci/nm256/nm256.c | 57 ++++++++++++++++++++++++++++++++----------------- 1 file changed, 37 insertions(+), 20 deletions(-)
diff --git a/sound/pci/nm256/nm256.c b/sound/pci/nm256/nm256.c index 103fe311e5a9..63f0985dae27 100644 --- a/sound/pci/nm256/nm256.c +++ b/sound/pci/nm256/nm256.c @@ -695,53 +695,68 @@ snd_nm256_capture_pointer(struct snd_pcm_substream *substream) */ static int snd_nm256_playback_silence(struct snd_pcm_substream *substream, - int channel, /* not used (interleaved data) */ - snd_pcm_uframes_t pos, - snd_pcm_uframes_t count) + int channel, unsigned long pos, unsigned long count) { struct snd_pcm_runtime *runtime = substream->runtime; struct nm256_stream *s = runtime->private_data; - count = frames_to_bytes(runtime, count); - pos = frames_to_bytes(runtime, pos); + memset_io(s->bufptr + pos, 0, count); return 0; }
static int snd_nm256_playback_copy(struct snd_pcm_substream *substream, - int channel, /* not used (interleaved data) */ - snd_pcm_uframes_t pos, - void __user *src, - snd_pcm_uframes_t count) + int channel, unsigned long pos, + void __user *src, unsigned long count) { struct snd_pcm_runtime *runtime = substream->runtime; struct nm256_stream *s = runtime->private_data; - count = frames_to_bytes(runtime, count); - pos = frames_to_bytes(runtime, pos); + if (copy_from_user_toio(s->bufptr + pos, src, count)) return -EFAULT; return 0; }
+static int +snd_nm256_playback_copy_kernel(struct snd_pcm_substream *substream, + int channel, unsigned long pos, + void *src, unsigned long count) +{ + struct snd_pcm_runtime *runtime = substream->runtime; + struct nm256_stream *s = runtime->private_data; + + memcpy_toio(s->bufptr + pos, src, count); + return 0; +} + /* * copy to user */ static int snd_nm256_capture_copy(struct snd_pcm_substream *substream, - int channel, /* not used (interleaved data) */ - snd_pcm_uframes_t pos, - void __user *dst, - snd_pcm_uframes_t count) + int channel, unsigned long pos, + void __user *dst, unsigned long count) { struct snd_pcm_runtime *runtime = substream->runtime; struct nm256_stream *s = runtime->private_data; - count = frames_to_bytes(runtime, count); - pos = frames_to_bytes(runtime, pos); + if (copy_to_user_fromio(dst, s->bufptr + pos, count)) return -EFAULT; return 0; }
+static int +snd_nm256_capture_copy_kernel(struct snd_pcm_substream *substream, + int channel, unsigned long pos, + void *dst, unsigned long count) +{ + struct snd_pcm_runtime *runtime = substream->runtime; + struct nm256_stream *s = runtime->private_data; + + memcpy_fromio(dst, s->bufptr + pos, count); + return 0; +} + #endif /* !__i386__ */
@@ -911,8 +926,9 @@ static const struct snd_pcm_ops snd_nm256_playback_ops = { .trigger = snd_nm256_playback_trigger, .pointer = snd_nm256_playback_pointer, #ifndef __i386__ - .copy = snd_nm256_playback_copy, - .silence = snd_nm256_playback_silence, + .copy_user = snd_nm256_playback_copy, + .copy_kernel = snd_nm256_playback_copy_kernel, + .fill_silence = snd_nm256_playback_silence, #endif .mmap = snd_pcm_lib_mmap_iomem, }; @@ -926,7 +942,8 @@ static const struct snd_pcm_ops snd_nm256_capture_ops = { .trigger = snd_nm256_capture_trigger, .pointer = snd_nm256_capture_pointer, #ifndef __i386__ - .copy = snd_nm256_capture_copy, + .copy_user = snd_nm256_capture_copy, + .copy_kernel = snd_nm256_capture_copy_kernel, #endif .mmap = snd_pcm_lib_mmap_iomem, };
Replace the copy and the silence ops with the new PCM ops. The conversion is straightforward with standard helper functions.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/pci/rme9652/rme9652.c | 71 ++++++++++++++++++++++++++++++++++----------- 1 file changed, 54 insertions(+), 17 deletions(-)
diff --git a/sound/pci/rme9652/rme9652.c b/sound/pci/rme9652/rme9652.c index 55172c689991..59684bf5cac0 100644 --- a/sound/pci/rme9652/rme9652.c +++ b/sound/pci/rme9652/rme9652.c @@ -1883,13 +1883,14 @@ static char *rme9652_channel_buffer_location(struct snd_rme9652 *rme9652, } }
-static int snd_rme9652_playback_copy(struct snd_pcm_substream *substream, int channel, - snd_pcm_uframes_t pos, void __user *src, snd_pcm_uframes_t count) +static int snd_rme9652_playback_copy(struct snd_pcm_substream *substream, + int channel, unsigned long pos, + void __user *src, unsigned long count) { struct snd_rme9652 *rme9652 = snd_pcm_substream_chip(substream); char *channel_buf;
- if (snd_BUG_ON(pos + count > RME9652_CHANNEL_BUFFER_BYTES / 4)) + if (snd_BUG_ON(pos + count > RME9652_CHANNEL_BUFFER_BYTES)) return -EINVAL;
channel_buf = rme9652_channel_buffer_location (rme9652, @@ -1897,18 +1898,35 @@ static int snd_rme9652_playback_copy(struct snd_pcm_substream *substream, int ch channel); if (snd_BUG_ON(!channel_buf)) return -EIO; - if (copy_from_user(channel_buf + pos * 4, src, count * 4)) + if (copy_from_user(channel_buf + pos, src, count)) return -EFAULT; - return count; + return 0; }
-static int snd_rme9652_capture_copy(struct snd_pcm_substream *substream, int channel, - snd_pcm_uframes_t pos, void __user *dst, snd_pcm_uframes_t count) +static int snd_rme9652_playback_copy_kernel(struct snd_pcm_substream *substream, + int channel, unsigned long pos, + void *src, unsigned long count) { struct snd_rme9652 *rme9652 = snd_pcm_substream_chip(substream); char *channel_buf;
- if (snd_BUG_ON(pos + count > RME9652_CHANNEL_BUFFER_BYTES / 4)) + channel_buf = rme9652_channel_buffer_location(rme9652, + substream->pstr->stream, + channel); + if (snd_BUG_ON(!channel_buf)) + return -EIO; + memcpy(channel_buf + pos, src, count); + return 0; +} + +static int snd_rme9652_capture_copy(struct snd_pcm_substream *substream, + int channel, unsigned long pos, + void __user *dst, unsigned long count) +{ + struct snd_rme9652 *rme9652 = snd_pcm_substream_chip(substream); + char *channel_buf; + + if (snd_BUG_ON(pos + count > RME9652_CHANNEL_BUFFER_BYTES)) return -EINVAL;
channel_buf = rme9652_channel_buffer_location (rme9652, @@ -1916,13 +1934,30 @@ static int snd_rme9652_capture_copy(struct snd_pcm_substream *substream, int cha channel); if (snd_BUG_ON(!channel_buf)) return -EIO; - if (copy_to_user(dst, channel_buf + pos * 4, count * 4)) + if (copy_to_user(dst, channel_buf + pos, count)) return -EFAULT; - return count; + return 0; }
-static int snd_rme9652_hw_silence(struct snd_pcm_substream *substream, int channel, - snd_pcm_uframes_t pos, snd_pcm_uframes_t count) +static int snd_rme9652_capture_copy_kernel(struct snd_pcm_substream *substream, + int channel, unsigned long pos, + void *dst, unsigned long count) +{ + struct snd_rme9652 *rme9652 = snd_pcm_substream_chip(substream); + char *channel_buf; + + channel_buf = rme9652_channel_buffer_location(rme9652, + substream->pstr->stream, + channel); + if (snd_BUG_ON(!channel_buf)) + return -EIO; + memcpy(dst, channel_buf + pos, count); + return 0; +} + +static int snd_rme9652_hw_silence(struct snd_pcm_substream *substream, + int channel, unsigned long pos, + unsigned long count) { struct snd_rme9652 *rme9652 = snd_pcm_substream_chip(substream); char *channel_buf; @@ -1932,8 +1967,8 @@ static int snd_rme9652_hw_silence(struct snd_pcm_substream *substream, int chann channel); if (snd_BUG_ON(!channel_buf)) return -EIO; - memset(channel_buf + pos * 4, 0, count * 4); - return count; + memset(channel_buf + pos, 0, count); + return 0; }
static int snd_rme9652_reset(struct snd_pcm_substream *substream) @@ -2376,8 +2411,9 @@ static const struct snd_pcm_ops snd_rme9652_playback_ops = { .prepare = snd_rme9652_prepare, .trigger = snd_rme9652_trigger, .pointer = snd_rme9652_hw_pointer, - .copy = snd_rme9652_playback_copy, - .silence = snd_rme9652_hw_silence, + .copy_user = snd_rme9652_playback_copy, + .copy_kernel = snd_rme9652_playback_copy_kernel, + .fill_silence = snd_rme9652_hw_silence, };
static const struct snd_pcm_ops snd_rme9652_capture_ops = { @@ -2388,7 +2424,8 @@ static const struct snd_pcm_ops snd_rme9652_capture_ops = { .prepare = snd_rme9652_prepare, .trigger = snd_rme9652_trigger, .pointer = snd_rme9652_hw_pointer, - .copy = snd_rme9652_capture_copy, + .copy_user = snd_rme9652_capture_copy, + .copy_kernel = snd_rme9652_capture_copy_kernel, };
static int snd_rme9652_create_pcm(struct snd_card *card,
Now that all users of old copy and silence ops have been converted to the new PCM ops, the old stuff can be retired and go away.
Signed-off-by: Takashi Iwai tiwai@suse.de --- include/sound/pcm.h | 5 ----- sound/core/pcm_lib.c | 38 +------------------------------------- sound/soc/soc-pcm.c | 2 -- 3 files changed, 1 insertion(+), 44 deletions(-)
diff --git a/include/sound/pcm.h b/include/sound/pcm.h index a065415191d8..953ebfc83184 100644 --- a/include/sound/pcm.h +++ b/include/sound/pcm.h @@ -78,11 +78,6 @@ struct snd_pcm_ops { struct timespec *system_ts, struct timespec *audio_ts, struct snd_pcm_audio_tstamp_config *audio_tstamp_config, struct snd_pcm_audio_tstamp_report *audio_tstamp_report); - int (*copy)(struct snd_pcm_substream *substream, int channel, - snd_pcm_uframes_t pos, - void __user *buf, snd_pcm_uframes_t count); - int (*silence)(struct snd_pcm_substream *substream, int channel, - snd_pcm_uframes_t pos, snd_pcm_uframes_t count); int (*fill_silence)(struct snd_pcm_substream *substream, int channel, unsigned long pos, unsigned long bytes); int (*copy_user)(struct snd_pcm_substream *substream, int channel, diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c index 9334fc2c20c8..0db8d4e0fca2 100644 --- a/sound/core/pcm_lib.c +++ b/sound/core/pcm_lib.c @@ -116,9 +116,6 @@ void snd_pcm_playback_silence(struct snd_pcm_substream *substream, snd_pcm_ufram frames_to_bytes(runtime, ofs), frames_to_bytes(runtime, transfer)); snd_BUG_ON(err < 0); - } else if (substream->ops->silence) { - err = substream->ops->silence(substream, -1, ofs, transfer); - snd_BUG_ON(err < 0); } else { hwbuf = runtime->dma_area + frames_to_bytes(runtime, ofs); snd_pcm_format_set_silence(runtime->format, hwbuf, transfer * runtime->channels); @@ -133,11 +130,6 @@ void snd_pcm_playback_silence(struct snd_pcm_substream *substream, snd_pcm_ufram samples_to_bytes(runtime, transfer)); snd_BUG_ON(err < 0); } - } else if (substream->ops->silence) { - for (c = 0; c < channels; ++c) { - err = substream->ops->silence(substream, c, ofs, transfer); - snd_BUG_ON(err < 0); - } } else { size_t dma_csize = runtime->dma_bytes / channels; for (c = 0; c < channels; ++c) { @@ -2013,9 +2005,6 @@ static int snd_pcm_lib_write_transfer(struct snd_pcm_substream *substream, err = substream->ops->copy_user(substream, 0, hwoff, buf, frames); if (err < 0) return err; - } else if (substream->ops->copy) { - if ((err = substream->ops->copy(substream, -1, hwoff, buf, frames)) < 0) - return err; } else { char *hwbuf = runtime->dma_area + frames_to_bytes(runtime, hwoff); if (copy_from_user(hwbuf, buf, frames_to_bytes(runtime, frames))) @@ -2137,8 +2126,7 @@ static int pcm_sanity_check(struct snd_pcm_substream *substream) if (PCM_RUNTIME_CHECK(substream)) return -ENXIO; runtime = substream->runtime; - if (snd_BUG_ON(!substream->ops->copy_user && !substream->ops->copy - && !runtime->dma_area)) + if (snd_BUG_ON(!substream->ops->copy_user && !runtime->dma_area)) return -EINVAL; if (runtime->status->state == SNDRV_PCM_STATE_OPEN) return -EBADFD; @@ -2198,19 +2186,6 @@ static int snd_pcm_lib_writev_transfer(struct snd_pcm_substream *substream, if (err < 0) return err; } - } else if (substream->ops->copy) { - if (snd_BUG_ON(!substream->ops->silence)) - return -EINVAL; - for (c = 0; c < channels; ++c, ++bufs) { - if (*bufs == NULL) { - if ((err = substream->ops->silence(substream, c, hwoff, frames)) < 0) - return err; - } else { - buf = *bufs + samples_to_bytes(runtime, off); - if ((err = substream->ops->copy(substream, c, hwoff, buf, frames)) < 0) - return err; - } - } } else { /* default transfer behaviour */ size_t dma_csize = runtime->dma_bytes / channels; @@ -2264,9 +2239,6 @@ static int snd_pcm_lib_read_transfer(struct snd_pcm_substream *substream, err = substream->ops->copy_user(substream, 0, hwoff, buf, frames); if (err < 0) return err; - } else if (substream->ops->copy) { - if ((err = substream->ops->copy(substream, -1, hwoff, buf, frames)) < 0) - return err; } else { char *hwbuf = runtime->dma_area + frames_to_bytes(runtime, hwoff); if (copy_to_user(buf, hwbuf, frames_to_bytes(runtime, frames))) @@ -2428,14 +2400,6 @@ static int snd_pcm_lib_readv_transfer(struct snd_pcm_substream *substream, if (err < 0) return err; } - } else if (substream->ops->copy) { - for (c = 0; c < channels; ++c, ++bufs) { - if (*bufs == NULL) - continue; - buf = *bufs + samples_to_bytes(runtime, off); - if ((err = substream->ops->copy(substream, c, hwoff, buf, frames)) < 0) - return err; - } } else { snd_pcm_uframes_t dma_csize = runtime->dma_bytes / channels; for (c = 0; c < channels; ++c, ++bufs) { diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 8867ed9e5f56..dcc5ece08668 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -2746,8 +2746,6 @@ int soc_new_pcm(struct snd_soc_pcm_runtime *rtd, int num) rtd->ops.copy_user = platform->driver->ops->copy_user; rtd->ops.copy_kernel = platform->driver->ops->copy_kernel; rtd->ops.fill_silence = platform->driver->ops->fill_silence; - rtd->ops.copy = platform->driver->ops->copy; - rtd->ops.silence = platform->driver->ops->silence; rtd->ops.page = platform->driver->ops->page; rtd->ops.mmap = platform->driver->ops->mmap; }
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/core/pcm_lib.c | 81 +++++++++++++++++++--------------------------------- 1 file changed, 29 insertions(+), 52 deletions(-)
diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c index 0db8d4e0fca2..e4f5c43b6448 100644 --- a/sound/core/pcm_lib.c +++ b/sound/core/pcm_lib.c @@ -2017,6 +2017,22 @@ typedef int (*transfer_f)(struct snd_pcm_substream *substream, unsigned int hwof unsigned long data, unsigned int off, snd_pcm_uframes_t size);
+static int pcm_accessible_state(struct snd_pcm_runtime *runtime) +{ + switch (runtime->status->state) { + case SNDRV_PCM_STATE_PREPARED: + case SNDRV_PCM_STATE_RUNNING: + case SNDRV_PCM_STATE_PAUSED: + return 0; + case SNDRV_PCM_STATE_XRUN: + return -EPIPE; + case SNDRV_PCM_STATE_SUSPENDED: + return -ESTRPIPE; + default: + return -EBADFD; + } +} + static snd_pcm_sframes_t snd_pcm_lib_write1(struct snd_pcm_substream *substream, unsigned long data, snd_pcm_uframes_t size, @@ -2033,21 +2049,9 @@ static snd_pcm_sframes_t snd_pcm_lib_write1(struct snd_pcm_substream *substream, return 0;
snd_pcm_stream_lock_irq(substream); - switch (runtime->status->state) { - case SNDRV_PCM_STATE_PREPARED: - case SNDRV_PCM_STATE_RUNNING: - case SNDRV_PCM_STATE_PAUSED: - break; - case SNDRV_PCM_STATE_XRUN: - err = -EPIPE; - goto _end_unlock; - case SNDRV_PCM_STATE_SUSPENDED: - err = -ESTRPIPE; - goto _end_unlock; - default: - err = -EBADFD; + err = pcm_accessible_state(runtime); + if (err < 0) goto _end_unlock; - }
runtime->twake = runtime->control->avail_min ? : 1; if (runtime->status->state == SNDRV_PCM_STATE_RUNNING) @@ -2083,16 +2087,9 @@ static snd_pcm_sframes_t snd_pcm_lib_write1(struct snd_pcm_substream *substream, snd_pcm_stream_lock_irq(substream); if (err < 0) goto _end_unlock; - switch (runtime->status->state) { - case SNDRV_PCM_STATE_XRUN: - err = -EPIPE; - goto _end_unlock; - case SNDRV_PCM_STATE_SUSPENDED: - err = -ESTRPIPE; + err = pcm_accessible_state(runtime); + if (err < 0) goto _end_unlock; - default: - break; - } appl_ptr += frames; if (appl_ptr >= runtime->boundary) appl_ptr -= runtime->boundary; @@ -2263,27 +2260,14 @@ static snd_pcm_sframes_t snd_pcm_lib_read1(struct snd_pcm_substream *substream, return 0;
snd_pcm_stream_lock_irq(substream); - switch (runtime->status->state) { - case SNDRV_PCM_STATE_PREPARED: - if (size >= runtime->start_threshold) { - err = snd_pcm_start(substream); - if (err < 0) - goto _end_unlock; - } - break; - case SNDRV_PCM_STATE_DRAINING: - case SNDRV_PCM_STATE_RUNNING: - case SNDRV_PCM_STATE_PAUSED: - break; - case SNDRV_PCM_STATE_XRUN: - err = -EPIPE; - goto _end_unlock; - case SNDRV_PCM_STATE_SUSPENDED: - err = -ESTRPIPE; - goto _end_unlock; - default: - err = -EBADFD; + err = pcm_accessible_state(runtime); + if (err < 0) goto _end_unlock; + if (runtime->status->state == SNDRV_PCM_STATE_PREPARED && + size >= runtime->start_threshold) { + err = snd_pcm_start(substream); + if (err < 0) + goto _end_unlock; }
runtime->twake = runtime->control->avail_min ? : 1; @@ -2327,16 +2311,9 @@ static snd_pcm_sframes_t snd_pcm_lib_read1(struct snd_pcm_substream *substream, snd_pcm_stream_lock_irq(substream); if (err < 0) goto _end_unlock; - switch (runtime->status->state) { - case SNDRV_PCM_STATE_XRUN: - err = -EPIPE; - goto _end_unlock; - case SNDRV_PCM_STATE_SUSPENDED: - err = -ESTRPIPE; + err = pcm_accessible_state(runtime); + if (err < 0) goto _end_unlock; - default: - break; - } appl_ptr += frames; if (appl_ptr >= runtime->boundary) appl_ptr -= runtime->boundary;
Just shuffle the codes, without any change otherwise.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/core/pcm_lib.c | 210 +++++++++++++++++++++++++-------------------------- 1 file changed, 105 insertions(+), 105 deletions(-)
diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c index e4f5c43b6448..5db086748515 100644 --- a/sound/core/pcm_lib.c +++ b/sound/core/pcm_lib.c @@ -1991,6 +1991,10 @@ static int wait_for_avail(struct snd_pcm_substream *substream, return err; } +typedef int (*transfer_f)(struct snd_pcm_substream *substream, unsigned int hwoff, + unsigned long data, unsigned int off, + snd_pcm_uframes_t size); + static int snd_pcm_lib_write_transfer(struct snd_pcm_substream *substream, unsigned int hwoff, unsigned long data, unsigned int off, @@ -2013,9 +2017,67 @@ static int snd_pcm_lib_write_transfer(struct snd_pcm_substream *substream, return 0; }
-typedef int (*transfer_f)(struct snd_pcm_substream *substream, unsigned int hwoff, - unsigned long data, unsigned int off, - snd_pcm_uframes_t size); +static int snd_pcm_lib_writev_transfer(struct snd_pcm_substream *substream, + unsigned int hwoff, + unsigned long data, unsigned int off, + snd_pcm_uframes_t frames) +{ + struct snd_pcm_runtime *runtime = substream->runtime; + int err; + void __user **bufs = (void __user **)data; + int channels = runtime->channels; + char __user *buf; + int c; + + if (substream->ops->copy_user) { + hwoff = samples_to_bytes(runtime, hwoff); + off = samples_to_bytes(runtime, off); + frames = samples_to_bytes(runtime, frames); + for (c = 0; c < channels; ++c, ++bufs) { + buf = *bufs + off; + if (!*bufs) { + if (snd_BUG_ON(!substream->ops->fill_silence)) + return -EINVAL; + err = substream->ops->fill_silence(substream, c, + hwoff, + frames); + } else { + err = substream->ops->copy_user(substream, c, + hwoff, buf, + frames); + } + if (err < 0) + return err; + } + } else { + /* default transfer behaviour */ + size_t dma_csize = runtime->dma_bytes / channels; + for (c = 0; c < channels; ++c, ++bufs) { + char *hwbuf = runtime->dma_area + (c * dma_csize) + samples_to_bytes(runtime, hwoff); + if (*bufs == NULL) { + snd_pcm_format_set_silence(runtime->format, hwbuf, frames); + } else { + char __user *buf = *bufs + samples_to_bytes(runtime, off); + if (copy_from_user(hwbuf, buf, samples_to_bytes(runtime, frames))) + return -EFAULT; + } + } + } + return 0; +} + +static int pcm_sanity_check(struct snd_pcm_substream *substream) +{ + struct snd_pcm_runtime *runtime; + if (PCM_RUNTIME_CHECK(substream)) + return -ENXIO; + runtime = substream->runtime; + if (snd_BUG_ON(!substream->ops->copy_user && !runtime->dma_area)) + return -EINVAL; + if (runtime->status->state == SNDRV_PCM_STATE_OPEN) + return -EBADFD; + return 0; +}
static int pcm_accessible_state(struct snd_pcm_runtime *runtime) { @@ -2117,19 +2179,6 @@ static snd_pcm_sframes_t snd_pcm_lib_write1(struct snd_pcm_substream *substream, }
/* sanity-check for read/write methods */ -static int pcm_sanity_check(struct snd_pcm_substream *substream) -{ - struct snd_pcm_runtime *runtime; - if (PCM_RUNTIME_CHECK(substream)) - return -ENXIO; - runtime = substream->runtime; - if (snd_BUG_ON(!substream->ops->copy_user && !runtime->dma_area)) - return -EINVAL; - if (runtime->status->state == SNDRV_PCM_STATE_OPEN) - return -EBADFD; - return 0; -} - snd_pcm_sframes_t snd_pcm_lib_write(struct snd_pcm_substream *substream, const void __user *buf, snd_pcm_uframes_t size) { struct snd_pcm_runtime *runtime; @@ -2151,55 +2200,6 @@ snd_pcm_sframes_t snd_pcm_lib_write(struct snd_pcm_substream *substream, const v
EXPORT_SYMBOL(snd_pcm_lib_write);
-static int snd_pcm_lib_writev_transfer(struct snd_pcm_substream *substream, - unsigned int hwoff, - unsigned long data, unsigned int off, - snd_pcm_uframes_t frames) -{ - struct snd_pcm_runtime *runtime = substream->runtime; - int err; - void __user **bufs = (void __user **)data; - int channels = runtime->channels; - char __user *buf; - int c; - - if (substream->ops->copy_user) { - hwoff = samples_to_bytes(runtime, hwoff); - off = samples_to_bytes(runtime, off); - frames = samples_to_bytes(runtime, frames); - for (c = 0; c < channels; ++c, ++bufs) { - buf = *bufs + off; - if (!*bufs) { - if (snd_BUG_ON(!substream->ops->fill_silence)) - return -EINVAL; - err = substream->ops->fill_silence(substream, c, - hwoff, - frames); - } else { - err = substream->ops->copy_user(substream, c, - hwoff, buf, - frames); - } - if (err < 0) - return err; - } - } else { - /* default transfer behaviour */ - size_t dma_csize = runtime->dma_bytes / channels; - for (c = 0; c < channels; ++c, ++bufs) { - char *hwbuf = runtime->dma_area + (c * dma_csize) + samples_to_bytes(runtime, hwoff); - if (*bufs == NULL) { - snd_pcm_format_set_silence(runtime->format, hwbuf, frames); - } else { - char __user *buf = *bufs + samples_to_bytes(runtime, off); - if (copy_from_user(hwbuf, buf, samples_to_bytes(runtime, frames))) - return -EFAULT; - } - } - } - return 0; -} - snd_pcm_sframes_t snd_pcm_lib_writev(struct snd_pcm_substream *substream, void __user **bufs, snd_pcm_uframes_t frames) @@ -2244,6 +2244,46 @@ static int snd_pcm_lib_read_transfer(struct snd_pcm_substream *substream, return 0; }
+static int snd_pcm_lib_readv_transfer(struct snd_pcm_substream *substream, + unsigned int hwoff, + unsigned long data, unsigned int off, + snd_pcm_uframes_t frames) +{ + struct snd_pcm_runtime *runtime = substream->runtime; + int err; + void __user **bufs = (void __user **)data; + int channels = runtime->channels; + char __user *buf; + char *hwbuf; + int c; + + if (substream->ops->copy_user) { + hwoff = samples_to_bytes(runtime, hwoff); + off = samples_to_bytes(runtime, off); + frames = samples_to_bytes(runtime, frames); + for (c = 0; c < channels; ++c, ++bufs) { + if (!*bufs) + continue; + err = substream->ops->copy_user(substream, c, hwoff, + *bufs + off, frames); + if (err < 0) + return err; + } + } else { + snd_pcm_uframes_t dma_csize = runtime->dma_bytes / channels; + for (c = 0; c < channels; ++c, ++bufs) { + if (*bufs == NULL) + continue; + + hwbuf = runtime->dma_area + (c * dma_csize) + samples_to_bytes(runtime, hwoff); + buf = *bufs + samples_to_bytes(runtime, off); + if (copy_to_user(buf, hwbuf, samples_to_bytes(runtime, frames))) + return -EFAULT; + } + } + return 0; +} + static snd_pcm_sframes_t snd_pcm_lib_read1(struct snd_pcm_substream *substream, unsigned long data, snd_pcm_uframes_t size, @@ -2352,46 +2392,6 @@ snd_pcm_sframes_t snd_pcm_lib_read(struct snd_pcm_substream *substream, void __u
EXPORT_SYMBOL(snd_pcm_lib_read);
-static int snd_pcm_lib_readv_transfer(struct snd_pcm_substream *substream, - unsigned int hwoff, - unsigned long data, unsigned int off, - snd_pcm_uframes_t frames) -{ - struct snd_pcm_runtime *runtime = substream->runtime; - int err; - void __user **bufs = (void __user **)data; - int channels = runtime->channels; - char __user *buf; - char *hwbuf; - int c; - - if (substream->ops->copy_user) { - hwoff = samples_to_bytes(runtime, hwoff); - off = samples_to_bytes(runtime, off); - frames = samples_to_bytes(runtime, frames); - for (c = 0; c < channels; ++c, ++bufs) { - if (!*bufs) - continue; - err = substream->ops->copy_user(substream, c, hwoff, - *bufs + off, frames); - if (err < 0) - return err; - } - } else { - snd_pcm_uframes_t dma_csize = runtime->dma_bytes / channels; - for (c = 0; c < channels; ++c, ++bufs) { - if (*bufs == NULL) - continue; - - hwbuf = runtime->dma_area + (c * dma_csize) + samples_to_bytes(runtime, hwoff); - buf = *bufs + samples_to_bytes(runtime, off); - if (copy_to_user(buf, hwbuf, samples_to_bytes(runtime, frames))) - return -EFAULT; - } - } - return 0; -} - snd_pcm_sframes_t snd_pcm_lib_readv(struct snd_pcm_substream *substream, void __user **bufs, snd_pcm_uframes_t frames)
On May 26 2017 04:17, Takashi Iwai wrote:
Just shuffle the codes, without any change otherwise.
Signed-off-by: Takashi Iwai tiwai@suse.de
sound/core/pcm_lib.c | 210 +++++++++++++++++++++++++-------------------------- 1 file changed, 105 insertions(+), 105 deletions(-)
diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c index e4f5c43b6448..5db086748515 100644 --- a/sound/core/pcm_lib.c +++ b/sound/core/pcm_lib.c > ... @@ -2117,19 +2179,6 @@ static snd_pcm_sframes_t snd_pcm_lib_write1(struct snd_pcm_substream *substream, }
/* sanity-check for read/write methods */ -static int pcm_sanity_check(struct snd_pcm_substream *substream) -{
- struct snd_pcm_runtime *runtime;
- if (PCM_RUNTIME_CHECK(substream))
return -ENXIO;
- runtime = substream->runtime;
- if (snd_BUG_ON(!substream->ops->copy_user && !runtime->dma_area))
return -EINVAL;
- if (runtime->status->state == SNDRV_PCM_STATE_OPEN)
return -EBADFD;
- return 0;
-}
The function comment should be involved to this move.
Regards
Takashi Sakamoto
On Fri, 26 May 2017 15:06:29 +0200, Takashi Sakamoto wrote:
On May 26 2017 04:17, Takashi Iwai wrote:
Just shuffle the codes, without any change otherwise.
Signed-off-by: Takashi Iwai tiwai@suse.de
sound/core/pcm_lib.c | 210 +++++++++++++++++++++++++-------------------------- 1 file changed, 105 insertions(+), 105 deletions(-)
diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c index e4f5c43b6448..5db086748515 100644 --- a/sound/core/pcm_lib.c +++ b/sound/core/pcm_lib.c > ... @@ -2117,19 +2179,6 @@ static snd_pcm_sframes_t snd_pcm_lib_write1(struct snd_pcm_substream *substream, } /* sanity-check for read/write methods */ -static int pcm_sanity_check(struct snd_pcm_substream *substream) -{
- struct snd_pcm_runtime *runtime;
- if (PCM_RUNTIME_CHECK(substream))
return -ENXIO;
- runtime = substream->runtime;
- if (snd_BUG_ON(!substream->ops->copy_user && !runtime->dma_area))
return -EINVAL;
- if (runtime->status->state == SNDRV_PCM_STATE_OPEN)
return -EBADFD;
- return 0;
-}
The function comment should be involved to this move.
Yep, corrected.
thanks,
Takashi
Make snd_pcm_lib_read() and *_write() static inline functions that call the common helper functions directly. This reduces a slight amount of codes, and at the same time, it's a preparation for the further cleanups / fixes.
Signed-off-by: Takashi Iwai tiwai@suse.de --- include/sound/pcm.h | 43 +++++++++++--- sound/core/pcm_lib.c | 157 ++++++++++++++++++--------------------------------- 2 files changed, 89 insertions(+), 111 deletions(-)
diff --git a/include/sound/pcm.h b/include/sound/pcm.h index 953ebfc83184..cb4eff8ffd2e 100644 --- a/include/sound/pcm.h +++ b/include/sound/pcm.h @@ -1088,15 +1088,40 @@ int snd_pcm_update_state(struct snd_pcm_substream *substream, int snd_pcm_update_hw_ptr(struct snd_pcm_substream *substream); void snd_pcm_playback_silence(struct snd_pcm_substream *substream, snd_pcm_uframes_t new_hw_ptr); void snd_pcm_period_elapsed(struct snd_pcm_substream *substream); -snd_pcm_sframes_t snd_pcm_lib_write(struct snd_pcm_substream *substream, - const void __user *buf, - snd_pcm_uframes_t frames); -snd_pcm_sframes_t snd_pcm_lib_read(struct snd_pcm_substream *substream, - void __user *buf, snd_pcm_uframes_t frames); -snd_pcm_sframes_t snd_pcm_lib_writev(struct snd_pcm_substream *substream, - void __user **bufs, snd_pcm_uframes_t frames); -snd_pcm_sframes_t snd_pcm_lib_readv(struct snd_pcm_substream *substream, - void __user **bufs, snd_pcm_uframes_t frames); +snd_pcm_sframes_t __snd_pcm_lib_write(struct snd_pcm_substream *substream, + void *buf, bool interleaved, + snd_pcm_uframes_t frames); +snd_pcm_sframes_t __snd_pcm_lib_read(struct snd_pcm_substream *substream, + void *buf, bool interleaved, + snd_pcm_uframes_t frames); + +static inline snd_pcm_sframes_t +snd_pcm_lib_write(struct snd_pcm_substream *substream, + const void __user *buf, snd_pcm_uframes_t frames) +{ + return __snd_pcm_lib_write(substream, (void *)buf, true, frames); +} + +static inline snd_pcm_sframes_t +snd_pcm_lib_read(struct snd_pcm_substream *substream, + void __user *buf, snd_pcm_uframes_t frames) +{ + return __snd_pcm_lib_read(substream, (void *)buf, true, frames); +} + +static inline snd_pcm_sframes_t +snd_pcm_lib_writev(struct snd_pcm_substream *substream, + void __user **bufs, snd_pcm_uframes_t frames) +{ + return __snd_pcm_lib_write(substream, (void *)bufs, false, frames); +} + +static inline snd_pcm_sframes_t +snd_pcm_lib_readv(struct snd_pcm_substream *substream, + void __user **bufs, snd_pcm_uframes_t frames) +{ + return __snd_pcm_lib_read(substream, (void *)bufs, false, frames); +}
extern const struct snd_pcm_hw_constraint_list snd_pcm_known_rates;
diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c index 5db086748515..7013e3d625ab 100644 --- a/sound/core/pcm_lib.c +++ b/sound/core/pcm_lib.c @@ -1992,12 +1992,12 @@ static int wait_for_avail(struct snd_pcm_substream *substream, } typedef int (*transfer_f)(struct snd_pcm_substream *substream, unsigned int hwoff, - unsigned long data, unsigned int off, + void *data, unsigned int off, snd_pcm_uframes_t size);
static int snd_pcm_lib_write_transfer(struct snd_pcm_substream *substream, unsigned int hwoff, - unsigned long data, unsigned int off, + void *data, unsigned int off, snd_pcm_uframes_t frames) { struct snd_pcm_runtime *runtime = substream->runtime; @@ -2019,7 +2019,7 @@ static int snd_pcm_lib_write_transfer(struct snd_pcm_substream *substream,
static int snd_pcm_lib_writev_transfer(struct snd_pcm_substream *substream, unsigned int hwoff, - unsigned long data, unsigned int off, + void *data, unsigned int off, snd_pcm_uframes_t frames) { struct snd_pcm_runtime *runtime = substream->runtime; @@ -2095,21 +2095,39 @@ static int pcm_accessible_state(struct snd_pcm_runtime *runtime) } }
-static snd_pcm_sframes_t snd_pcm_lib_write1(struct snd_pcm_substream *substream, - unsigned long data, - snd_pcm_uframes_t size, - int nonblock, - transfer_f transfer) +snd_pcm_sframes_t __snd_pcm_lib_write(struct snd_pcm_substream *substream, + void *data, bool interleaved, + snd_pcm_uframes_t size) { struct snd_pcm_runtime *runtime = substream->runtime; snd_pcm_uframes_t xfer = 0; snd_pcm_uframes_t offset = 0; snd_pcm_uframes_t avail; - int err = 0; + transfer_f transfer; + bool nonblock; + int err; + + err = pcm_sanity_check(substream); + if (err < 0) + return err; + runtime = substream->runtime; + + if (interleaved) { + if (runtime->access != SNDRV_PCM_ACCESS_RW_INTERLEAVED && + runtime->channels > 1) + return -EINVAL; + transfer = snd_pcm_lib_write_transfer; + } else { + if (runtime->access != SNDRV_PCM_ACCESS_RW_NONINTERLEAVED) + return -EINVAL; + transfer = snd_pcm_lib_writev_transfer; + }
if (size == 0) return 0;
+ nonblock = !!(substream->f_flags & O_NONBLOCK); + snd_pcm_stream_lock_irq(substream); err = pcm_accessible_state(runtime); if (err < 0) @@ -2177,54 +2195,11 @@ static snd_pcm_sframes_t snd_pcm_lib_write1(struct snd_pcm_substream *substream, snd_pcm_stream_unlock_irq(substream); return xfer > 0 ? (snd_pcm_sframes_t)xfer : err; } - -/* sanity-check for read/write methods */ -snd_pcm_sframes_t snd_pcm_lib_write(struct snd_pcm_substream *substream, const void __user *buf, snd_pcm_uframes_t size) -{ - struct snd_pcm_runtime *runtime; - int nonblock; - int err; - - err = pcm_sanity_check(substream); - if (err < 0) - return err; - runtime = substream->runtime; - nonblock = !!(substream->f_flags & O_NONBLOCK); - - if (runtime->access != SNDRV_PCM_ACCESS_RW_INTERLEAVED && - runtime->channels > 1) - return -EINVAL; - return snd_pcm_lib_write1(substream, (unsigned long)buf, size, nonblock, - snd_pcm_lib_write_transfer); -} - -EXPORT_SYMBOL(snd_pcm_lib_write); - -snd_pcm_sframes_t snd_pcm_lib_writev(struct snd_pcm_substream *substream, - void __user **bufs, - snd_pcm_uframes_t frames) -{ - struct snd_pcm_runtime *runtime; - int nonblock; - int err; - - err = pcm_sanity_check(substream); - if (err < 0) - return err; - runtime = substream->runtime; - nonblock = !!(substream->f_flags & O_NONBLOCK); - - if (runtime->access != SNDRV_PCM_ACCESS_RW_NONINTERLEAVED) - return -EINVAL; - return snd_pcm_lib_write1(substream, (unsigned long)bufs, frames, - nonblock, snd_pcm_lib_writev_transfer); -} - -EXPORT_SYMBOL(snd_pcm_lib_writev); +EXPORT_SYMBOL(__snd_pcm_lib_write);
static int snd_pcm_lib_read_transfer(struct snd_pcm_substream *substream, unsigned int hwoff, - unsigned long data, unsigned int off, + void *data, unsigned int off, snd_pcm_uframes_t frames) { struct snd_pcm_runtime *runtime = substream->runtime; @@ -2246,7 +2221,7 @@ static int snd_pcm_lib_read_transfer(struct snd_pcm_substream *substream,
static int snd_pcm_lib_readv_transfer(struct snd_pcm_substream *substream, unsigned int hwoff, - unsigned long data, unsigned int off, + void *data, unsigned int off, snd_pcm_uframes_t frames) { struct snd_pcm_runtime *runtime = substream->runtime; @@ -2284,21 +2259,39 @@ static int snd_pcm_lib_readv_transfer(struct snd_pcm_substream *substream, return 0; }
-static snd_pcm_sframes_t snd_pcm_lib_read1(struct snd_pcm_substream *substream, - unsigned long data, - snd_pcm_uframes_t size, - int nonblock, - transfer_f transfer) +snd_pcm_sframes_t __snd_pcm_lib_read(struct snd_pcm_substream *substream, + void *data, bool interleaved, + snd_pcm_uframes_t size) { struct snd_pcm_runtime *runtime = substream->runtime; snd_pcm_uframes_t xfer = 0; snd_pcm_uframes_t offset = 0; snd_pcm_uframes_t avail; - int err = 0; + transfer_f transfer; + bool nonblock; + int err; + + err = pcm_sanity_check(substream); + if (err < 0) + return err; + runtime = substream->runtime; + + if (interleaved) { + if (runtime->access != SNDRV_PCM_ACCESS_RW_INTERLEAVED && + runtime->channels > 1) + return -EINVAL; + transfer = snd_pcm_lib_read_transfer; + } else { + if (runtime->access != SNDRV_PCM_ACCESS_RW_NONINTERLEAVED) + return -EINVAL; + transfer = snd_pcm_lib_readv_transfer; + }
if (size == 0) return 0;
+ nonblock = !!(substream->f_flags & O_NONBLOCK); + snd_pcm_stream_lock_irq(substream); err = pcm_accessible_state(runtime); if (err < 0) @@ -2373,47 +2366,7 @@ static snd_pcm_sframes_t snd_pcm_lib_read1(struct snd_pcm_substream *substream, snd_pcm_stream_unlock_irq(substream); return xfer > 0 ? (snd_pcm_sframes_t)xfer : err; } - -snd_pcm_sframes_t snd_pcm_lib_read(struct snd_pcm_substream *substream, void __user *buf, snd_pcm_uframes_t size) -{ - struct snd_pcm_runtime *runtime; - int nonblock; - int err; - - err = pcm_sanity_check(substream); - if (err < 0) - return err; - runtime = substream->runtime; - nonblock = !!(substream->f_flags & O_NONBLOCK); - if (runtime->access != SNDRV_PCM_ACCESS_RW_INTERLEAVED) - return -EINVAL; - return snd_pcm_lib_read1(substream, (unsigned long)buf, size, nonblock, snd_pcm_lib_read_transfer); -} - -EXPORT_SYMBOL(snd_pcm_lib_read); - -snd_pcm_sframes_t snd_pcm_lib_readv(struct snd_pcm_substream *substream, - void __user **bufs, - snd_pcm_uframes_t frames) -{ - struct snd_pcm_runtime *runtime; - int nonblock; - int err; - - err = pcm_sanity_check(substream); - if (err < 0) - return err; - runtime = substream->runtime; - if (runtime->status->state == SNDRV_PCM_STATE_OPEN) - return -EBADFD; - - nonblock = !!(substream->f_flags & O_NONBLOCK); - if (runtime->access != SNDRV_PCM_ACCESS_RW_NONINTERLEAVED) - return -EINVAL; - return snd_pcm_lib_read1(substream, (unsigned long)bufs, frames, nonblock, snd_pcm_lib_readv_transfer); -} - -EXPORT_SYMBOL(snd_pcm_lib_readv); +EXPORT_SYMBOL(__snd_pcm_lib_read);
/* * standard channel mapping helpers
This patch proceeds more abstraction of PCM read/write loop codes.
For both interleaved and non-interleaved transfers, the same copy or silence transfer code (which is defined as pcm_transfer_f) is used now. This became possible since we switched to byte size to copy_* and fill_silence ops argument instead of frames.
And, for both read and write, we can use the same copy function (which is defined as pcm_copy_f), just depending on whether interleaved or non-interleaved mode.
The transfer function is determined at the beginning of the loop, depending on whether the driver gives the specific copy ops or it's the standard read/write.
Another bonus by this change is that we now guarantee the silencing behavior when NULL buffer is passed to write helpers. It'll simplify some codes later.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/core/pcm_lib.c | 254 +++++++++++++++++++++++++-------------------------- 1 file changed, 123 insertions(+), 131 deletions(-)
diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c index 7013e3d625ab..32cc933be6b2 100644 --- a/sound/core/pcm_lib.c +++ b/sound/core/pcm_lib.c @@ -1991,77 +1991,100 @@ static int wait_for_avail(struct snd_pcm_substream *substream, return err; } -typedef int (*transfer_f)(struct snd_pcm_substream *substream, unsigned int hwoff, - void *data, unsigned int off, - snd_pcm_uframes_t size); +typedef int (*pcm_transfer_f)(struct snd_pcm_substream *substream, + int channel, unsigned long hwoff, + void *buf, unsigned long bytes);
-static int snd_pcm_lib_write_transfer(struct snd_pcm_substream *substream, - unsigned int hwoff, - void *data, unsigned int off, - snd_pcm_uframes_t frames) +typedef int (*pcm_copy_f)(struct snd_pcm_substream *, snd_pcm_uframes_t, void *, + snd_pcm_uframes_t, snd_pcm_uframes_t, pcm_transfer_f); + +/* calculate the target DMA-buffer position to be written/read */ +static void *get_dma_ptr(struct snd_pcm_runtime *runtime, + int channel, unsigned long hwoff) +{ + return runtime->dma_area + hwoff + + channel * (runtime->dma_bytes / runtime->channels); +} + +/* default copy_user ops for write */ +static int default_write_copy_user(struct snd_pcm_substream *substream, + int channel, unsigned long hwoff, + void __user *buf, unsigned long bytes) +{ + if (copy_from_user(get_dma_ptr(substream->runtime, channel, hwoff), + buf, bytes)) + return -EFAULT; + return 0; +} + +/* fill silence instead of copy data; called as a transfer helper + * from __snd_pcm_lib_write() or directly from noninterleaved_copy() when + * a NULL buffer is passed + */ +static int fill_silence(struct snd_pcm_substream *substream, int channel, + unsigned long hwoff, void *buf, unsigned long bytes) { struct snd_pcm_runtime *runtime = substream->runtime; - int err; - char __user *buf = (char __user *) data + frames_to_bytes(runtime, off); - if (substream->ops->copy_user) { - hwoff = frames_to_bytes(runtime, hwoff); - frames = frames_to_bytes(runtime, frames); - err = substream->ops->copy_user(substream, 0, hwoff, buf, frames); - if (err < 0) - return err; - } else { - char *hwbuf = runtime->dma_area + frames_to_bytes(runtime, hwoff); - if (copy_from_user(hwbuf, buf, frames_to_bytes(runtime, frames))) - return -EFAULT; - } + + if (substream->stream != SNDRV_PCM_STREAM_PLAYBACK) + return 0; + if (substream->ops->fill_silence) + return substream->ops->fill_silence(substream, channel, + hwoff, bytes); + + snd_pcm_format_set_silence(runtime->format, + get_dma_ptr(runtime, channel, hwoff), + bytes_to_samples(runtime, bytes)); return 0; } - -static int snd_pcm_lib_writev_transfer(struct snd_pcm_substream *substream, - unsigned int hwoff, - void *data, unsigned int off, - snd_pcm_uframes_t frames) + +/* call transfer function with the converted pointers and sizes; + * for interleaved mode, it's one shot for all samples + */ +static int interleaved_copy(struct snd_pcm_substream *substream, + snd_pcm_uframes_t hwoff, void *data, + snd_pcm_uframes_t off, + snd_pcm_uframes_t frames, + pcm_transfer_f transfer) +{ + struct snd_pcm_runtime *runtime = substream->runtime; + + /* convert to bytes */ + hwoff = frames_to_bytes(runtime, hwoff); + off = frames_to_bytes(runtime, off); + frames = frames_to_bytes(runtime, frames); + return transfer(substream, 0, hwoff, data + off, frames); +} + +/* call transfer function with the converted pointers and sizes for each + * non-interleaved channel; when buffer is NULL, silencing instead of copying + */ +static int noninterleaved_copy(struct snd_pcm_substream *substream, + snd_pcm_uframes_t hwoff, void *data, + snd_pcm_uframes_t off, + snd_pcm_uframes_t frames, + pcm_transfer_f transfer) { struct snd_pcm_runtime *runtime = substream->runtime; - int err; - void __user **bufs = (void __user **)data; int channels = runtime->channels; - char __user *buf; - int c; - - if (substream->ops->copy_user) { - hwoff = samples_to_bytes(runtime, hwoff); - off = samples_to_bytes(runtime, off); - frames = samples_to_bytes(runtime, frames); - for (c = 0; c < channels; ++c, ++bufs) { - buf = *bufs + off; - if (!*bufs) { - if (snd_BUG_ON(!substream->ops->fill_silence)) - return -EINVAL; - err = substream->ops->fill_silence(substream, c, - hwoff, - frames); - } else { - err = substream->ops->copy_user(substream, c, - hwoff, buf, - frames); - } - if (err < 0) - return err; - } - } else { - /* default transfer behaviour */ - size_t dma_csize = runtime->dma_bytes / channels; - for (c = 0; c < channels; ++c, ++bufs) { - char *hwbuf = runtime->dma_area + (c * dma_csize) + samples_to_bytes(runtime, hwoff); - if (*bufs == NULL) { - snd_pcm_format_set_silence(runtime->format, hwbuf, frames); - } else { - char __user *buf = *bufs + samples_to_bytes(runtime, off); - if (copy_from_user(hwbuf, buf, samples_to_bytes(runtime, frames))) - return -EFAULT; - } - } + void **bufs = data; + int c, err; + + /* convert to bytes; note that it's not frames_to_bytes() here. + * in non-interleaved mode, we copy for each channel, thus + * each copy is n_samples bytes x channels = whole frames. + */ + off = samples_to_bytes(runtime, off); + frames = samples_to_bytes(runtime, frames); + hwoff = samples_to_bytes(runtime, hwoff); + for (c = 0; c < channels; ++c, ++bufs) { + if (!data || !*bufs) + err = fill_silence(substream, c, hwoff, NULL, frames); + else + err = transfer(substream, c, hwoff, *bufs + off, + frames); + if (err < 0) + return err; } return 0; } @@ -2103,24 +2126,33 @@ snd_pcm_sframes_t __snd_pcm_lib_write(struct snd_pcm_substream *substream, snd_pcm_uframes_t xfer = 0; snd_pcm_uframes_t offset = 0; snd_pcm_uframes_t avail; - transfer_f transfer; + pcm_copy_f writer; + pcm_transfer_f transfer; bool nonblock; int err;
err = pcm_sanity_check(substream); if (err < 0) return err; - runtime = substream->runtime;
if (interleaved) { if (runtime->access != SNDRV_PCM_ACCESS_RW_INTERLEAVED && runtime->channels > 1) return -EINVAL; - transfer = snd_pcm_lib_write_transfer; + writer = interleaved_copy; } else { if (runtime->access != SNDRV_PCM_ACCESS_RW_NONINTERLEAVED) return -EINVAL; - transfer = snd_pcm_lib_writev_transfer; + writer = noninterleaved_copy; + } + + if (!data) { + transfer = fill_silence; + } else { + if (substream->ops->copy_user) + transfer = (pcm_transfer_f)substream->ops->copy_user; + else + transfer = default_write_copy_user; }
if (size == 0) @@ -2163,7 +2195,8 @@ snd_pcm_sframes_t __snd_pcm_lib_write(struct snd_pcm_substream *substream, appl_ptr = runtime->control->appl_ptr; appl_ofs = appl_ptr % runtime->buffer_size; snd_pcm_stream_unlock_irq(substream); - err = transfer(substream, appl_ofs, data, offset, frames); + err = writer(substream, appl_ofs, data, offset, frames, + transfer); snd_pcm_stream_lock_irq(substream); if (err < 0) goto _end_unlock; @@ -2197,65 +2230,15 @@ snd_pcm_sframes_t __snd_pcm_lib_write(struct snd_pcm_substream *substream, } EXPORT_SYMBOL(__snd_pcm_lib_write);
-static int snd_pcm_lib_read_transfer(struct snd_pcm_substream *substream, - unsigned int hwoff, - void *data, unsigned int off, - snd_pcm_uframes_t frames) -{ - struct snd_pcm_runtime *runtime = substream->runtime; - int err; - char __user *buf = (char __user *) data + frames_to_bytes(runtime, off); - if (substream->ops->copy_user) { - hwoff = frames_to_bytes(runtime, hwoff); - frames = frames_to_bytes(runtime, frames); - err = substream->ops->copy_user(substream, 0, hwoff, buf, frames); - if (err < 0) - return err; - } else { - char *hwbuf = runtime->dma_area + frames_to_bytes(runtime, hwoff); - if (copy_to_user(buf, hwbuf, frames_to_bytes(runtime, frames))) - return -EFAULT; - } - return 0; -} - -static int snd_pcm_lib_readv_transfer(struct snd_pcm_substream *substream, - unsigned int hwoff, - void *data, unsigned int off, - snd_pcm_uframes_t frames) +/* default copy_user ops for read */ +static int default_read_copy_user(struct snd_pcm_substream *substream, + int channel, unsigned long hwoff, + void *buf, unsigned long bytes) { - struct snd_pcm_runtime *runtime = substream->runtime; - int err; - void __user **bufs = (void __user **)data; - int channels = runtime->channels; - char __user *buf; - char *hwbuf; - int c; - - if (substream->ops->copy_user) { - hwoff = samples_to_bytes(runtime, hwoff); - off = samples_to_bytes(runtime, off); - frames = samples_to_bytes(runtime, frames); - for (c = 0; c < channels; ++c, ++bufs) { - if (!*bufs) - continue; - err = substream->ops->copy_user(substream, c, hwoff, - *bufs + off, frames); - if (err < 0) - return err; - } - } else { - snd_pcm_uframes_t dma_csize = runtime->dma_bytes / channels; - for (c = 0; c < channels; ++c, ++bufs) { - if (*bufs == NULL) - continue; - - hwbuf = runtime->dma_area + (c * dma_csize) + samples_to_bytes(runtime, hwoff); - buf = *bufs + samples_to_bytes(runtime, off); - if (copy_to_user(buf, hwbuf, samples_to_bytes(runtime, frames))) - return -EFAULT; - } - } + if (copy_to_user((void __user *)buf, + get_dma_ptr(substream->runtime, channel, hwoff), + bytes)) + return -EFAULT; return 0; }
@@ -2267,26 +2250,34 @@ snd_pcm_sframes_t __snd_pcm_lib_read(struct snd_pcm_substream *substream, snd_pcm_uframes_t xfer = 0; snd_pcm_uframes_t offset = 0; snd_pcm_uframes_t avail; - transfer_f transfer; + pcm_copy_f reader; + pcm_transfer_f transfer; bool nonblock; int err;
err = pcm_sanity_check(substream); if (err < 0) return err; - runtime = substream->runtime; + + if (!data) + return -EINVAL;
if (interleaved) { if (runtime->access != SNDRV_PCM_ACCESS_RW_INTERLEAVED && runtime->channels > 1) return -EINVAL; - transfer = snd_pcm_lib_read_transfer; + reader = interleaved_copy; } else { if (runtime->access != SNDRV_PCM_ACCESS_RW_NONINTERLEAVED) return -EINVAL; - transfer = snd_pcm_lib_readv_transfer; + reader = noninterleaved_copy; }
+ if (substream->ops->copy_user) + transfer = (pcm_transfer_f)substream->ops->copy_user; + else + transfer = default_read_copy_user; + if (size == 0) return 0;
@@ -2340,7 +2331,8 @@ snd_pcm_sframes_t __snd_pcm_lib_read(struct snd_pcm_substream *substream, appl_ptr = runtime->control->appl_ptr; appl_ofs = appl_ptr % runtime->buffer_size; snd_pcm_stream_unlock_irq(substream); - err = transfer(substream, appl_ofs, data, offset, frames); + err = reader(substream, appl_ofs, data, offset, frames, + transfer); snd_pcm_stream_lock_irq(substream); if (err < 0) goto _end_unlock;
Both __snd_pcm_lib_read() and __snd_pcm_write() functions have almost the same code to loop over samples. For simplification, this patch unifies both as the single helper, __snd_pcm_lib_xfer().
Other than that, there should be no functional change by this patch.
Signed-off-by: Takashi Iwai tiwai@suse.de --- include/sound/pcm.h | 13 ++-- sound/core/pcm_lib.c | 184 +++++++++++++-------------------------------------- 2 files changed, 51 insertions(+), 146 deletions(-)
diff --git a/include/sound/pcm.h b/include/sound/pcm.h index cb4eff8ffd2e..173c6a6ebf35 100644 --- a/include/sound/pcm.h +++ b/include/sound/pcm.h @@ -1088,10 +1088,7 @@ int snd_pcm_update_state(struct snd_pcm_substream *substream, int snd_pcm_update_hw_ptr(struct snd_pcm_substream *substream); void snd_pcm_playback_silence(struct snd_pcm_substream *substream, snd_pcm_uframes_t new_hw_ptr); void snd_pcm_period_elapsed(struct snd_pcm_substream *substream); -snd_pcm_sframes_t __snd_pcm_lib_write(struct snd_pcm_substream *substream, - void *buf, bool interleaved, - snd_pcm_uframes_t frames); -snd_pcm_sframes_t __snd_pcm_lib_read(struct snd_pcm_substream *substream, +snd_pcm_sframes_t __snd_pcm_lib_xfer(struct snd_pcm_substream *substream, void *buf, bool interleaved, snd_pcm_uframes_t frames);
@@ -1099,28 +1096,28 @@ static inline snd_pcm_sframes_t snd_pcm_lib_write(struct snd_pcm_substream *substream, const void __user *buf, snd_pcm_uframes_t frames) { - return __snd_pcm_lib_write(substream, (void *)buf, true, frames); + return __snd_pcm_lib_xfer(substream, (void *)buf, true, frames); }
static inline snd_pcm_sframes_t snd_pcm_lib_read(struct snd_pcm_substream *substream, void __user *buf, snd_pcm_uframes_t frames) { - return __snd_pcm_lib_read(substream, (void *)buf, true, frames); + return __snd_pcm_lib_xfer(substream, (void *)buf, true, frames); }
static inline snd_pcm_sframes_t snd_pcm_lib_writev(struct snd_pcm_substream *substream, void __user **bufs, snd_pcm_uframes_t frames) { - return __snd_pcm_lib_write(substream, (void *)bufs, false, frames); + return __snd_pcm_lib_xfer(substream, (void *)bufs, false, frames); }
static inline snd_pcm_sframes_t snd_pcm_lib_readv(struct snd_pcm_substream *substream, void __user **bufs, snd_pcm_uframes_t frames) { - return __snd_pcm_lib_read(substream, (void *)bufs, false, frames); + return __snd_pcm_lib_xfer(substream, (void *)bufs, false, frames); }
extern const struct snd_pcm_hw_constraint_list snd_pcm_known_rates; diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c index 32cc933be6b2..c24a78e39b88 100644 --- a/sound/core/pcm_lib.c +++ b/sound/core/pcm_lib.c @@ -2006,13 +2006,13 @@ static void *get_dma_ptr(struct snd_pcm_runtime *runtime, channel * (runtime->dma_bytes / runtime->channels); }
-/* default copy_user ops for write */ -static int default_write_copy_user(struct snd_pcm_substream *substream, - int channel, unsigned long hwoff, - void __user *buf, unsigned long bytes) +/* default copy_user ops for write; used for both interleaved and non- modes */ +static int default_write_copy(struct snd_pcm_substream *substream, + int channel, unsigned long hwoff, + void *buf, unsigned long bytes) { if (copy_from_user(get_dma_ptr(substream->runtime, channel, hwoff), - buf, bytes)) + (void __user *)buf, bytes)) return -EFAULT; return 0; } @@ -2038,6 +2038,18 @@ static int fill_silence(struct snd_pcm_substream *substream, int channel, return 0; }
+/* default copy_user ops for read; used for both interleaved and non- modes */ +static int default_read_copy(struct snd_pcm_substream *substream, + int channel, unsigned long hwoff, + void *buf, unsigned long bytes) +{ + if (copy_to_user((void __user *)buf, + get_dma_ptr(substream->runtime, channel, hwoff), + bytes)) + return -EFAULT; + return 0; +} + /* call transfer function with the converted pointers and sizes; * for interleaved mode, it's one shot for all samples */ @@ -2118,9 +2130,10 @@ static int pcm_accessible_state(struct snd_pcm_runtime *runtime) } }
-snd_pcm_sframes_t __snd_pcm_lib_write(struct snd_pcm_substream *substream, - void *data, bool interleaved, - snd_pcm_uframes_t size) +/* the common loop for read/write data */ +snd_pcm_sframes_t __snd_pcm_lib_xfer(struct snd_pcm_substream *substream, + void *data, bool interleaved, + snd_pcm_uframes_t size) { struct snd_pcm_runtime *runtime = substream->runtime; snd_pcm_uframes_t xfer = 0; @@ -2129,12 +2142,14 @@ snd_pcm_sframes_t __snd_pcm_lib_write(struct snd_pcm_substream *substream, pcm_copy_f writer; pcm_transfer_f transfer; bool nonblock; + bool is_playback; int err;
err = pcm_sanity_check(substream); if (err < 0) return err;
+ is_playback = substream->stream == SNDRV_PCM_STREAM_PLAYBACK; if (interleaved) { if (runtime->access != SNDRV_PCM_ACCESS_RW_INTERLEAVED && runtime->channels > 1) @@ -2147,12 +2162,16 @@ snd_pcm_sframes_t __snd_pcm_lib_write(struct snd_pcm_substream *substream, }
if (!data) { - transfer = fill_silence; + if (is_playback) + transfer = fill_silence; + else + return -EINVAL; } else { if (substream->ops->copy_user) transfer = (pcm_transfer_f)substream->ops->copy_user; else - transfer = default_write_copy_user; + transfer = is_playback ? + default_write_copy : default_read_copy; }
if (size == 0) @@ -2165,129 +2184,8 @@ snd_pcm_sframes_t __snd_pcm_lib_write(struct snd_pcm_substream *substream, if (err < 0) goto _end_unlock;
- runtime->twake = runtime->control->avail_min ? : 1; - if (runtime->status->state == SNDRV_PCM_STATE_RUNNING) - snd_pcm_update_hw_ptr(substream); - avail = snd_pcm_playback_avail(runtime); - while (size > 0) { - snd_pcm_uframes_t frames, appl_ptr, appl_ofs; - snd_pcm_uframes_t cont; - if (!avail) { - if (nonblock) { - err = -EAGAIN; - goto _end_unlock; - } - runtime->twake = min_t(snd_pcm_uframes_t, size, - runtime->control->avail_min ? : 1); - err = wait_for_avail(substream, &avail); - if (err < 0) - goto _end_unlock; - } - frames = size > avail ? avail : size; - cont = runtime->buffer_size - runtime->control->appl_ptr % runtime->buffer_size; - if (frames > cont) - frames = cont; - if (snd_BUG_ON(!frames)) { - runtime->twake = 0; - snd_pcm_stream_unlock_irq(substream); - return -EINVAL; - } - appl_ptr = runtime->control->appl_ptr; - appl_ofs = appl_ptr % runtime->buffer_size; - snd_pcm_stream_unlock_irq(substream); - err = writer(substream, appl_ofs, data, offset, frames, - transfer); - snd_pcm_stream_lock_irq(substream); - if (err < 0) - goto _end_unlock; - err = pcm_accessible_state(runtime); - if (err < 0) - goto _end_unlock; - appl_ptr += frames; - if (appl_ptr >= runtime->boundary) - appl_ptr -= runtime->boundary; - runtime->control->appl_ptr = appl_ptr; - if (substream->ops->ack) - substream->ops->ack(substream); - - offset += frames; - size -= frames; - xfer += frames; - avail -= frames; - if (runtime->status->state == SNDRV_PCM_STATE_PREPARED && - snd_pcm_playback_hw_avail(runtime) >= (snd_pcm_sframes_t)runtime->start_threshold) { - err = snd_pcm_start(substream); - if (err < 0) - goto _end_unlock; - } - } - _end_unlock: - runtime->twake = 0; - if (xfer > 0 && err >= 0) - snd_pcm_update_state(substream, runtime); - snd_pcm_stream_unlock_irq(substream); - return xfer > 0 ? (snd_pcm_sframes_t)xfer : err; -} -EXPORT_SYMBOL(__snd_pcm_lib_write); - -/* default copy_user ops for read */ -static int default_read_copy_user(struct snd_pcm_substream *substream, - int channel, unsigned long hwoff, - void *buf, unsigned long bytes) -{ - if (copy_to_user((void __user *)buf, - get_dma_ptr(substream->runtime, channel, hwoff), - bytes)) - return -EFAULT; - return 0; -} - -snd_pcm_sframes_t __snd_pcm_lib_read(struct snd_pcm_substream *substream, - void *data, bool interleaved, - snd_pcm_uframes_t size) -{ - struct snd_pcm_runtime *runtime = substream->runtime; - snd_pcm_uframes_t xfer = 0; - snd_pcm_uframes_t offset = 0; - snd_pcm_uframes_t avail; - pcm_copy_f reader; - pcm_transfer_f transfer; - bool nonblock; - int err; - - err = pcm_sanity_check(substream); - if (err < 0) - return err; - - if (!data) - return -EINVAL; - - if (interleaved) { - if (runtime->access != SNDRV_PCM_ACCESS_RW_INTERLEAVED && - runtime->channels > 1) - return -EINVAL; - reader = interleaved_copy; - } else { - if (runtime->access != SNDRV_PCM_ACCESS_RW_NONINTERLEAVED) - return -EINVAL; - reader = noninterleaved_copy; - } - - if (substream->ops->copy_user) - transfer = (pcm_transfer_f)substream->ops->copy_user; - else - transfer = default_read_copy_user; - - if (size == 0) - return 0; - - nonblock = !!(substream->f_flags & O_NONBLOCK); - - snd_pcm_stream_lock_irq(substream); - err = pcm_accessible_state(runtime); - if (err < 0) - goto _end_unlock; - if (runtime->status->state == SNDRV_PCM_STATE_PREPARED && + if (!is_playback && + runtime->status->state == SNDRV_PCM_STATE_PREPARED && size >= runtime->start_threshold) { err = snd_pcm_start(substream); if (err < 0) @@ -2297,13 +2195,16 @@ snd_pcm_sframes_t __snd_pcm_lib_read(struct snd_pcm_substream *substream, runtime->twake = runtime->control->avail_min ? : 1; if (runtime->status->state == SNDRV_PCM_STATE_RUNNING) snd_pcm_update_hw_ptr(substream); - avail = snd_pcm_capture_avail(runtime); + if (is_playback) + avail = snd_pcm_playback_avail(runtime); + else + avail = snd_pcm_capture_avail(runtime); while (size > 0) { snd_pcm_uframes_t frames, appl_ptr, appl_ofs; snd_pcm_uframes_t cont; if (!avail) { - if (runtime->status->state == - SNDRV_PCM_STATE_DRAINING) { + if (!is_playback && + runtime->status->state == SNDRV_PCM_STATE_DRAINING) { snd_pcm_stop(substream, SNDRV_PCM_STATE_SETUP); goto _end_unlock; } @@ -2331,7 +2232,7 @@ snd_pcm_sframes_t __snd_pcm_lib_read(struct snd_pcm_substream *substream, appl_ptr = runtime->control->appl_ptr; appl_ofs = appl_ptr % runtime->buffer_size; snd_pcm_stream_unlock_irq(substream); - err = reader(substream, appl_ofs, data, offset, frames, + err = writer(substream, appl_ofs, data, offset, frames, transfer); snd_pcm_stream_lock_irq(substream); if (err < 0) @@ -2350,6 +2251,13 @@ snd_pcm_sframes_t __snd_pcm_lib_read(struct snd_pcm_substream *substream, size -= frames; xfer += frames; avail -= frames; + if (is_playback && + runtime->status->state == SNDRV_PCM_STATE_PREPARED && + snd_pcm_playback_hw_avail(runtime) >= (snd_pcm_sframes_t)runtime->start_threshold) { + err = snd_pcm_start(substream); + if (err < 0) + goto _end_unlock; + } } _end_unlock: runtime->twake = 0; @@ -2358,7 +2266,7 @@ snd_pcm_sframes_t __snd_pcm_lib_read(struct snd_pcm_substream *substream, snd_pcm_stream_unlock_irq(substream); return xfer > 0 ? (snd_pcm_sframes_t)xfer : err; } -EXPORT_SYMBOL(__snd_pcm_lib_read); +EXPORT_SYMBOL(__snd_pcm_lib_xfer);
/* * standard channel mapping helpers
Use the existing silence helper codes for simplification.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/core/pcm_lib.c | 40 +++++++++++++--------------------------- 1 file changed, 13 insertions(+), 27 deletions(-)
diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c index c24a78e39b88..094447ae8486 100644 --- a/sound/core/pcm_lib.c +++ b/sound/core/pcm_lib.c @@ -42,6 +42,9 @@ #define trace_hw_ptr_error(substream, reason) #endif
+static int fill_silence(struct snd_pcm_substream *substream, int channel, + unsigned long hwoff, void *buf, unsigned long bytes); + /* * fill ring buffer with silence * runtime->silence_start: starting pointer to silence area @@ -55,8 +58,7 @@ void snd_pcm_playback_silence(struct snd_pcm_substream *substream, snd_pcm_ufram { struct snd_pcm_runtime *runtime = substream->runtime; snd_pcm_uframes_t frames, ofs, transfer; - char *hwbuf; - int err; + int c;
if (runtime->silence_size < runtime->boundary) { snd_pcm_sframes_t noise_dist, n; @@ -111,33 +113,17 @@ void snd_pcm_playback_silence(struct snd_pcm_substream *substream, snd_pcm_ufram transfer = ofs + frames > runtime->buffer_size ? runtime->buffer_size - ofs : frames; if (runtime->access == SNDRV_PCM_ACCESS_RW_INTERLEAVED || runtime->access == SNDRV_PCM_ACCESS_MMAP_INTERLEAVED) { - if (substream->ops->fill_silence) { - err = substream->ops->fill_silence(substream, 0, - frames_to_bytes(runtime, ofs), - frames_to_bytes(runtime, transfer)); - snd_BUG_ON(err < 0); - } else { - hwbuf = runtime->dma_area + frames_to_bytes(runtime, ofs); - snd_pcm_format_set_silence(runtime->format, hwbuf, transfer * runtime->channels); - } + fill_silence(substream, 0, + frames_to_bytes(runtime, ofs), NULL, + frames_to_bytes(runtime, transfer)); } else { - unsigned int c; - unsigned int channels = runtime->channels; - if (substream->ops->fill_silence) { - for (c = 0; c < channels; ++c) { - err = substream->ops->fill_silence(substream, c, - samples_to_bytes(runtime, ofs), - samples_to_bytes(runtime, transfer)); - snd_BUG_ON(err < 0); - } - } else { - size_t dma_csize = runtime->dma_bytes / channels; - for (c = 0; c < channels; ++c) { - hwbuf = runtime->dma_area + (c * dma_csize) + samples_to_bytes(runtime, ofs); - snd_pcm_format_set_silence(runtime->format, hwbuf, transfer); - } - } + int byte_ofs = samples_to_bytes(runtime, ofs); + int byte_xfer = samples_to_bytes(runtime, transfer); + for (c = 0; c < runtime->channels; ++c) + fill_silence(substream, c, byte_ofs, NULL, + byte_xfer); } + runtime->silence_filled += transfer; frames -= transfer; ofs = 0;
On May 26 2017 04:17, Takashi Iwai wrote:
Use the existing silence helper codes for simplification.
Signed-off-by: Takashi Iwai tiwai@suse.de
sound/core/pcm_lib.c | 40 +++++++++++++--------------------------- 1 file changed, 13 insertions(+), 27 deletions(-)
diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c index c24a78e39b88..094447ae8486 100644 --- a/sound/core/pcm_lib.c +++ b/sound/core/pcm_lib.c @@ -42,6 +42,9 @@ #define trace_hw_ptr_error(substream, reason) #endif
+static int fill_silence(struct snd_pcm_substream *substream, int channel,
unsigned long hwoff, void *buf, unsigned long bytes);
- /*
- fill ring buffer with silence
- runtime->silence_start: starting pointer to silence area
@@ -55,8 +58,7 @@ void snd_pcm_playback_silence(struct snd_pcm_substream *substream, snd_pcm_ufram { struct snd_pcm_runtime *runtime = substream->runtime; snd_pcm_uframes_t frames, ofs, transfer;
- char *hwbuf;
- int err;
int c;
if (runtime->silence_size < runtime->boundary) { snd_pcm_sframes_t noise_dist, n;
@@ -111,33 +113,17 @@ void snd_pcm_playback_silence(struct snd_pcm_substream *substream, snd_pcm_ufram transfer = ofs + frames > runtime->buffer_size ? runtime->buffer_size - ofs : frames; if (runtime->access == SNDRV_PCM_ACCESS_RW_INTERLEAVED || runtime->access == SNDRV_PCM_ACCESS_MMAP_INTERLEAVED) {
if (substream->ops->fill_silence) {
err = substream->ops->fill_silence(substream, 0,
frames_to_bytes(runtime, ofs),
frames_to_bytes(runtime, transfer));
snd_BUG_ON(err < 0);
} else {
hwbuf = runtime->dma_area + frames_to_bytes(runtime, ofs);
snd_pcm_format_set_silence(runtime->format, hwbuf, transfer * runtime->channels);
}
fill_silence(substream, 0,
frames_to_bytes(runtime, ofs), NULL,
} else {frames_to_bytes(runtime, transfer));
unsigned int c;
unsigned int channels = runtime->channels;
if (substream->ops->fill_silence) {
for (c = 0; c < channels; ++c) {
err = substream->ops->fill_silence(substream, c,
samples_to_bytes(runtime, ofs),
samples_to_bytes(runtime, transfer));
snd_BUG_ON(err < 0);
}
} else {
size_t dma_csize = runtime->dma_bytes / channels;
for (c = 0; c < channels; ++c) {
hwbuf = runtime->dma_area + (c * dma_csize) + samples_to_bytes(runtime, ofs);
snd_pcm_format_set_silence(runtime->format, hwbuf, transfer);
}
}
int byte_ofs = samples_to_bytes(runtime, ofs);
int byte_xfer = samples_to_bytes(runtime, transfer);
for (c = 0; c < runtime->channels; ++c)
fill_silence(substream, c, byte_ofs, NULL,
}byte_xfer);
- runtime->silence_filled += transfer; frames -= transfer; ofs = 0;
A part of the content inner the while loop can be replaced with added 'writer' functions. And the purpose of this patchset is a kind of refactoring, however this drops snd_BUG_ON(). It should be remained. Below patch can be squashed to your patch for the points.
diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c index 094447ae8486..7758bffdc4e2 100644 --- a/sound/core/pcm_lib.c +++ b/sound/core/pcm_lib.c @@ -42,9 +42,27 @@ #define trace_hw_ptr_error(substream, reason) #endif
+typedef int (*pcm_transfer_f)(struct snd_pcm_substream *substream, + int channel, unsigned long hwoff, + void *buf, unsigned long bytes); + +typedef int (*pcm_copy_f)(struct snd_pcm_substream *, snd_pcm_uframes_t, void *, + snd_pcm_uframes_t, snd_pcm_uframes_t, pcm_transfer_f); + static int fill_silence(struct snd_pcm_substream *substream, int channel, unsigned long hwoff, void *buf, unsigned long bytes);
+static int interleaved_copy(struct snd_pcm_substream *substream, + snd_pcm_uframes_t hwoff, void *data, + snd_pcm_uframes_t off, + snd_pcm_uframes_t frames, + pcm_transfer_f transfer); +static int noninterleaved_copy(struct snd_pcm_substream *substream, + snd_pcm_uframes_t hwoff, void *data, + snd_pcm_uframes_t off, + snd_pcm_uframes_t frames, + pcm_transfer_f transfer); + /* * fill ring buffer with silence * runtime->silence_start: starting pointer to silence area @@ -58,7 +76,8 @@ void snd_pcm_playback_silence(struct snd_pcm_substream *substream, snd_pcm_ufram { struct snd_pcm_runtime *runtime = substream->runtime; snd_pcm_uframes_t frames, ofs, transfer; - int c; + pcm_copy_f copy; + int err;
if (runtime->silence_size < runtime->boundary) { snd_pcm_sframes_t noise_dist, n; @@ -109,20 +128,18 @@ void snd_pcm_playback_silence(struct snd_pcm_substream *substream, snd_pcm_ufram if (frames == 0) return; ofs = runtime->silence_start % runtime->buffer_size; + + if (runtime->access == SNDRV_PCM_ACCESS_RW_INTERLEAVED || + runtime->access == SNDRV_PCM_ACCESS_MMAP_INTERLEAVED) + copy = interleaved_copy; + else + copy = noninterleaved_copy; + while (frames > 0) { transfer = ofs + frames > runtime->buffer_size ? runtime->buffer_size - ofs : frames; - if (runtime->access == SNDRV_PCM_ACCESS_RW_INTERLEAVED || - runtime->access == SNDRV_PCM_ACCESS_MMAP_INTERLEAVED) { - fill_silence(substream, 0, - frames_to_bytes(runtime, ofs), NULL, - frames_to_bytes(runtime, transfer)); - } else { - int byte_ofs = samples_to_bytes(runtime, ofs); - int byte_xfer = samples_to_bytes(runtime, transfer); - for (c = 0; c < runtime->channels; ++c) - fill_silence(substream, c, byte_ofs, NULL, - byte_xfer); - } + err = copy(substream, ofs, (void *)NULL, 0, transfer, + fill_silence); + snd_BUG_ON(err < 0);
runtime->silence_filled += transfer; frames -= transfer; @@ -1977,13 +1994,6 @@ static int wait_for_avail(struct snd_pcm_substream *substream, return err; } -typedef int (*pcm_transfer_f)(struct snd_pcm_substream *substream, - int channel, unsigned long hwoff, - void *buf, unsigned long bytes); - -typedef int (*pcm_copy_f)(struct snd_pcm_substream *, snd_pcm_uframes_t, void *, - snd_pcm_uframes_t, snd_pcm_uframes_t, pcm_transfer_f); - /* calculate the target DMA-buffer position to be written/read */ static void *get_dma_ptr(struct snd_pcm_runtime *runtime, int channel, unsigned long hwoff)
Regards
Takashi Sakamoto
On Fri, 26 May 2017 15:21:54 +0200, Takashi Sakamoto wrote:
On May 26 2017 04:17, Takashi Iwai wrote:
Use the existing silence helper codes for simplification.
Signed-off-by: Takashi Iwai tiwai@suse.de
sound/core/pcm_lib.c | 40 +++++++++++++--------------------------- 1 file changed, 13 insertions(+), 27 deletions(-)
diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c index c24a78e39b88..094447ae8486 100644 --- a/sound/core/pcm_lib.c +++ b/sound/core/pcm_lib.c @@ -42,6 +42,9 @@ #define trace_hw_ptr_error(substream, reason) #endif +static int fill_silence(struct snd_pcm_substream *substream, int channel,
unsigned long hwoff, void *buf, unsigned long bytes);
- /*
- fill ring buffer with silence
- runtime->silence_start: starting pointer to silence area
@@ -55,8 +58,7 @@ void snd_pcm_playback_silence(struct snd_pcm_substream *substream, snd_pcm_ufram { struct snd_pcm_runtime *runtime = substream->runtime; snd_pcm_uframes_t frames, ofs, transfer;
- char *hwbuf;
- int err;
- int c; if (runtime->silence_size < runtime->boundary) { snd_pcm_sframes_t noise_dist, n;
@@ -111,33 +113,17 @@ void snd_pcm_playback_silence(struct snd_pcm_substream *substream, snd_pcm_ufram transfer = ofs + frames > runtime->buffer_size ? runtime->buffer_size - ofs : frames; if (runtime->access == SNDRV_PCM_ACCESS_RW_INTERLEAVED || runtime->access == SNDRV_PCM_ACCESS_MMAP_INTERLEAVED) {
if (substream->ops->fill_silence) {
err = substream->ops->fill_silence(substream, 0,
frames_to_bytes(runtime, ofs),
frames_to_bytes(runtime, transfer));
snd_BUG_ON(err < 0);
} else {
hwbuf = runtime->dma_area + frames_to_bytes(runtime, ofs);
snd_pcm_format_set_silence(runtime->format, hwbuf, transfer * runtime->channels);
}
fill_silence(substream, 0,
frames_to_bytes(runtime, ofs), NULL,
} else {frames_to_bytes(runtime, transfer));
unsigned int c;
unsigned int channels = runtime->channels;
if (substream->ops->fill_silence) {
for (c = 0; c < channels; ++c) {
err = substream->ops->fill_silence(substream, c,
samples_to_bytes(runtime, ofs),
samples_to_bytes(runtime, transfer));
snd_BUG_ON(err < 0);
}
} else {
size_t dma_csize = runtime->dma_bytes / channels;
for (c = 0; c < channels; ++c) {
hwbuf = runtime->dma_area + (c * dma_csize) + samples_to_bytes(runtime, ofs);
snd_pcm_format_set_silence(runtime->format, hwbuf, transfer);
}
}
int byte_ofs = samples_to_bytes(runtime, ofs);
int byte_xfer = samples_to_bytes(runtime, transfer);
for (c = 0; c < runtime->channels; ++c)
fill_silence(substream, c, byte_ofs, NULL,
}byte_xfer);
- runtime->silence_filled += transfer; frames -= transfer; ofs = 0;
A part of the content inner the while loop can be replaced with added 'writer' functions.
Right, we can reuse that.
And the purpose of this patchset is a kind of refactoring, however this drops snd_BUG_ON(). It should be remained. Below patch can be squashed to your patch for the points.
Thanks. I prefer just creating a new function instead of calling the function pointer there. Basically it's easier to read/understand.
The revised patch is below.
Takashi
-- 8< -- From: Takashi Iwai tiwai@suse.de Subject: [PATCH v2] ALSA: pcm: Simplify snd_pcm_playback_silence()
Use the existing silence helper codes for simplification.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/core/pcm_lib.c | 50 ++++++++++++++++++++------------------------------ 1 file changed, 20 insertions(+), 30 deletions(-)
diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c index f15460eaf8b5..a592d3308474 100644 --- a/sound/core/pcm_lib.c +++ b/sound/core/pcm_lib.c @@ -42,6 +42,9 @@ #define trace_hw_ptr_error(substream, reason) #endif
+static int fill_silence_frames(struct snd_pcm_substream *substream, + snd_pcm_uframes_t off, snd_pcm_uframes_t frames); + /* * fill ring buffer with silence * runtime->silence_start: starting pointer to silence area @@ -55,7 +58,6 @@ void snd_pcm_playback_silence(struct snd_pcm_substream *substream, snd_pcm_ufram { struct snd_pcm_runtime *runtime = substream->runtime; snd_pcm_uframes_t frames, ofs, transfer; - char *hwbuf; int err;
if (runtime->silence_size < runtime->boundary) { @@ -109,35 +111,8 @@ void snd_pcm_playback_silence(struct snd_pcm_substream *substream, snd_pcm_ufram ofs = runtime->silence_start % runtime->buffer_size; while (frames > 0) { transfer = ofs + frames > runtime->buffer_size ? runtime->buffer_size - ofs : frames; - if (runtime->access == SNDRV_PCM_ACCESS_RW_INTERLEAVED || - runtime->access == SNDRV_PCM_ACCESS_MMAP_INTERLEAVED) { - if (substream->ops->fill_silence) { - err = substream->ops->fill_silence(substream, 0, - frames_to_bytes(runtime, ofs), - frames_to_bytes(runtime, transfer)); - snd_BUG_ON(err < 0); - } else { - hwbuf = runtime->dma_area + frames_to_bytes(runtime, ofs); - snd_pcm_format_set_silence(runtime->format, hwbuf, transfer * runtime->channels); - } - } else { - unsigned int c; - unsigned int channels = runtime->channels; - if (substream->ops->fill_silence) { - for (c = 0; c < channels; ++c) { - err = substream->ops->fill_silence(substream, c, - samples_to_bytes(runtime, ofs), - samples_to_bytes(runtime, transfer)); - snd_BUG_ON(err < 0); - } - } else { - size_t dma_csize = runtime->dma_bytes / channels; - for (c = 0; c < channels; ++c) { - hwbuf = runtime->dma_area + (c * dma_csize) + samples_to_bytes(runtime, ofs); - snd_pcm_format_set_silence(runtime->format, hwbuf, transfer); - } - } - } + err = fill_silence_frames(substream, ofs, transfer); + snd_BUG_ON(err < 0); runtime->silence_filled += transfer; frames -= transfer; ofs = 0; @@ -2101,6 +2076,21 @@ static int noninterleaved_copy(struct snd_pcm_substream *substream, return 0; }
+/* fill silence on the given buffer position; + * called from snd_pcm_playback_silence() + */ +static int fill_silence_frames(struct snd_pcm_substream *substream, + snd_pcm_uframes_t off, snd_pcm_uframes_t frames) +{ + if (substream->runtime->access == SNDRV_PCM_ACCESS_RW_INTERLEAVED || + substream->runtime->access == SNDRV_PCM_ACCESS_MMAP_INTERLEAVED) + return interleaved_copy(substream, off, NULL, 0, frames, + fill_silence); + else + return noninterleaved_copy(substream, off, NULL, 0, frames, + fill_silence); +} + /* sanity-check for read/write methods */ static int pcm_sanity_check(struct snd_pcm_substream *substream) {
Now all materials are ready, let's allow the direct in-kernel read/write, i.e. a kernel-space buffer is passed for read or write, instead of the normal user-space buffer. This feature is used by OSS layer and UAC1 driver, for example.
The __snd_pcm_lib_xfer() takes in_kernel argument that indicates the in-kernel buffer copy. When this flag is set, another transfer code is used. It's either via copy_kernel PCM ops or the normal memcpy(), depending on the driver setup.
As external API, snd_pcm_kernel_read(), *_write() and other variants are provided.
That's all. This support is really simple because of the code refactoring until now.
Signed-off-by: Takashi Iwai tiwai@suse.de --- include/sound/pcm.h | 38 +++++++++++++++++++++++++++++++++----- sound/core/pcm_lib.c | 26 +++++++++++++++++++++++++- 2 files changed, 58 insertions(+), 6 deletions(-)
diff --git a/include/sound/pcm.h b/include/sound/pcm.h index 173c6a6ebf35..e3a7269824c7 100644 --- a/include/sound/pcm.h +++ b/include/sound/pcm.h @@ -1090,34 +1090,62 @@ void snd_pcm_playback_silence(struct snd_pcm_substream *substream, snd_pcm_ufram void snd_pcm_period_elapsed(struct snd_pcm_substream *substream); snd_pcm_sframes_t __snd_pcm_lib_xfer(struct snd_pcm_substream *substream, void *buf, bool interleaved, - snd_pcm_uframes_t frames); + snd_pcm_uframes_t frames, bool in_kernel);
static inline snd_pcm_sframes_t snd_pcm_lib_write(struct snd_pcm_substream *substream, const void __user *buf, snd_pcm_uframes_t frames) { - return __snd_pcm_lib_xfer(substream, (void *)buf, true, frames); + return __snd_pcm_lib_xfer(substream, (void *)buf, true, frames, false); }
static inline snd_pcm_sframes_t snd_pcm_lib_read(struct snd_pcm_substream *substream, void __user *buf, snd_pcm_uframes_t frames) { - return __snd_pcm_lib_xfer(substream, (void *)buf, true, frames); + return __snd_pcm_lib_xfer(substream, (void *)buf, true, frames, false); }
static inline snd_pcm_sframes_t snd_pcm_lib_writev(struct snd_pcm_substream *substream, void __user **bufs, snd_pcm_uframes_t frames) { - return __snd_pcm_lib_xfer(substream, (void *)bufs, false, frames); + return __snd_pcm_lib_xfer(substream, (void *)bufs, false, frames, false); }
static inline snd_pcm_sframes_t snd_pcm_lib_readv(struct snd_pcm_substream *substream, void __user **bufs, snd_pcm_uframes_t frames) { - return __snd_pcm_lib_xfer(substream, (void *)bufs, false, frames); + return __snd_pcm_lib_xfer(substream, (void *)bufs, false, frames, false); +} + +static inline snd_pcm_sframes_t +snd_pcm_kernel_write(struct snd_pcm_substream *substream, + const void *buf, snd_pcm_uframes_t frames) +{ + return __snd_pcm_lib_xfer(substream, (void *)buf, true, frames, true); +} + +static inline snd_pcm_sframes_t +snd_pcm_kernel_read(struct snd_pcm_substream *substream, + void *buf, snd_pcm_uframes_t frames) +{ + return __snd_pcm_lib_xfer(substream, buf, true, frames, true); +} + +static inline snd_pcm_sframes_t +snd_pcm_kernel_writev(struct snd_pcm_substream *substream, + void **bufs, snd_pcm_uframes_t frames) +{ + return __snd_pcm_lib_xfer(substream, bufs, false, frames, true); +} + +static inline snd_pcm_sframes_t +snd_pcm_kernel_readv(struct snd_pcm_substream *substream, + void **bufs, snd_pcm_uframes_t frames) +{ + return __snd_pcm_lib_xfer(substream, bufs, false, frames, true); }
extern const struct snd_pcm_hw_constraint_list snd_pcm_known_rates; diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c index 094447ae8486..4ad8f5aee74f 100644 --- a/sound/core/pcm_lib.c +++ b/sound/core/pcm_lib.c @@ -2003,6 +2003,15 @@ static int default_write_copy(struct snd_pcm_substream *substream, return 0; }
+/* default copy_kernel ops for write */ +static int default_write_copy_kernel(struct snd_pcm_substream *substream, + int channel, unsigned long hwoff, + void *buf, unsigned long bytes) +{ + memcpy(get_dma_ptr(substream->runtime, channel, hwoff), buf, bytes); + return 0; +} + /* fill silence instead of copy data; called as a transfer helper * from __snd_pcm_lib_write() or directly from noninterleaved_copy() when * a NULL buffer is passed @@ -2036,6 +2045,15 @@ static int default_read_copy(struct snd_pcm_substream *substream, return 0; }
+/* default copy_kernel ops for read */ +static int default_read_copy_kernel(struct snd_pcm_substream *substream, + int channel, unsigned long hwoff, + void *buf, unsigned long bytes) +{ + memcpy(buf, get_dma_ptr(substream->runtime, channel, hwoff), bytes); + return 0; +} + /* call transfer function with the converted pointers and sizes; * for interleaved mode, it's one shot for all samples */ @@ -2119,7 +2137,7 @@ static int pcm_accessible_state(struct snd_pcm_runtime *runtime) /* the common loop for read/write data */ snd_pcm_sframes_t __snd_pcm_lib_xfer(struct snd_pcm_substream *substream, void *data, bool interleaved, - snd_pcm_uframes_t size) + snd_pcm_uframes_t size, bool in_kernel) { struct snd_pcm_runtime *runtime = substream->runtime; snd_pcm_uframes_t xfer = 0; @@ -2152,6 +2170,12 @@ snd_pcm_sframes_t __snd_pcm_lib_xfer(struct snd_pcm_substream *substream, transfer = fill_silence; else return -EINVAL; + } else if (in_kernel) { + if (substream->ops->copy_kernel) + transfer = substream->ops->copy_kernel; + else + transfer = is_playback ? + default_write_copy_kernel : default_read_copy_kernel; } else { if (substream->ops->copy_user) transfer = (pcm_transfer_f)substream->ops->copy_user;
With the new API to perform the in-kernel buffer copy, we can get rid of set_fs() usage in this driver, finally.
Signed-off-by: Takashi Iwai tiwai@suse.de --- drivers/usb/gadget/function/u_uac1.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/drivers/usb/gadget/function/u_uac1.c b/drivers/usb/gadget/function/u_uac1.c index c78c84138a28..ca88e4c0fd1e 100644 --- a/drivers/usb/gadget/function/u_uac1.c +++ b/drivers/usb/gadget/function/u_uac1.c @@ -157,7 +157,6 @@ size_t u_audio_playback(struct gaudio *card, void *buf, size_t count) struct gaudio_snd_dev *snd = &card->playback; struct snd_pcm_substream *substream = snd->substream; struct snd_pcm_runtime *runtime = substream->runtime; - mm_segment_t old_fs; ssize_t result; snd_pcm_sframes_t frames;
@@ -174,15 +173,11 @@ size_t u_audio_playback(struct gaudio *card, void *buf, size_t count) }
frames = bytes_to_frames(runtime, count); - old_fs = get_fs(); - set_fs(KERNEL_DS); - result = snd_pcm_lib_write(snd->substream, (void __user *)buf, frames); + result = snd_pcm_kernel_write(snd->substream, buf, frames); if (result != frames) { ERROR(card, "Playback error: %d\n", (int)result); - set_fs(old_fs); goto try_again; } - set_fs(old_fs);
return 0; }
This is the last-standing one: kill the set_fs() usage in PCM OSS layer by replacing with the new API functions to deal with the direct in-kernel buffer copying.
The code to fill the silence can be replaced even to a one-liner to pass NULL buffer instead of the manual copying.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/core/oss/pcm_oss.c | 77 ++++++++---------------------------------------- 1 file changed, 12 insertions(+), 65 deletions(-)
diff --git a/sound/core/oss/pcm_oss.c b/sound/core/oss/pcm_oss.c index e306f05ce51d..2d6a825cfe88 100644 --- a/sound/core/oss/pcm_oss.c +++ b/sound/core/oss/pcm_oss.c @@ -67,18 +67,6 @@ static int snd_pcm_oss_get_rate(struct snd_pcm_oss_file *pcm_oss_file); static int snd_pcm_oss_get_channels(struct snd_pcm_oss_file *pcm_oss_file); static int snd_pcm_oss_get_format(struct snd_pcm_oss_file *pcm_oss_file);
-static inline mm_segment_t snd_enter_user(void) -{ - mm_segment_t fs = get_fs(); - set_fs(get_ds()); - return fs; -} - -static inline void snd_leave_user(mm_segment_t fs) -{ - set_fs(fs); -} - /* * helper functions to process hw_params */ @@ -1191,14 +1179,8 @@ snd_pcm_sframes_t snd_pcm_oss_write3(struct snd_pcm_substream *substream, const if (ret < 0) break; } - if (in_kernel) { - mm_segment_t fs; - fs = snd_enter_user(); - ret = snd_pcm_lib_write(substream, (void __force __user *)ptr, frames); - snd_leave_user(fs); - } else { - ret = snd_pcm_lib_write(substream, (void __force __user *)ptr, frames); - } + ret = __snd_pcm_lib_xfer(substream, (void *)ptr, true, + frames, in_kernel); if (ret != -EPIPE && ret != -ESTRPIPE) break; /* test, if we can't store new data, because the stream */ @@ -1234,14 +1216,8 @@ snd_pcm_sframes_t snd_pcm_oss_read3(struct snd_pcm_substream *substream, char *p ret = snd_pcm_oss_capture_position_fixup(substream, &delay); if (ret < 0) break; - if (in_kernel) { - mm_segment_t fs; - fs = snd_enter_user(); - ret = snd_pcm_lib_read(substream, (void __force __user *)ptr, frames); - snd_leave_user(fs); - } else { - ret = snd_pcm_lib_read(substream, (void __force __user *)ptr, frames); - } + ret = __snd_pcm_lib_xfer(substream, (void *)ptr, true, + frames, in_kernel); if (ret == -EPIPE) { if (runtime->status->state == SNDRV_PCM_STATE_DRAINING) { ret = snd_pcm_kernel_ioctl(substream, SNDRV_PCM_IOCTL_DROP, NULL); @@ -1273,14 +1249,8 @@ snd_pcm_sframes_t snd_pcm_oss_writev3(struct snd_pcm_substream *substream, void if (ret < 0) break; } - if (in_kernel) { - mm_segment_t fs; - fs = snd_enter_user(); - ret = snd_pcm_lib_writev(substream, (void __user **)bufs, frames); - snd_leave_user(fs); - } else { - ret = snd_pcm_lib_writev(substream, (void __user **)bufs, frames); - } + ret = __snd_pcm_lib_xfer(substream, bufs, false, frames, + in_kernel); if (ret != -EPIPE && ret != -ESTRPIPE) break;
@@ -1313,14 +1283,8 @@ snd_pcm_sframes_t snd_pcm_oss_readv3(struct snd_pcm_substream *substream, void * if (ret < 0) break; } - if (in_kernel) { - mm_segment_t fs; - fs = snd_enter_user(); - ret = snd_pcm_lib_readv(substream, (void __user **)bufs, frames); - snd_leave_user(fs); - } else { - ret = snd_pcm_lib_readv(substream, (void __user **)bufs, frames); - } + ret = __snd_pcm_lib_xfer(substream, bufs, false, frames, + in_kernel); if (ret != -EPIPE && ret != -ESTRPIPE) break; } @@ -1650,27 +1614,10 @@ static int snd_pcm_oss_sync(struct snd_pcm_oss_file *pcm_oss_file) size = runtime->control->appl_ptr % runtime->period_size; if (size > 0) { size = runtime->period_size - size; - if (runtime->access == SNDRV_PCM_ACCESS_RW_INTERLEAVED) { - size = (runtime->frame_bits * size) / 8; - while (size > 0) { - mm_segment_t fs; - size_t size1 = size < runtime->oss.period_bytes ? size : runtime->oss.period_bytes; - size -= size1; - size1 *= 8; - size1 /= runtime->sample_bits; - snd_pcm_format_set_silence(runtime->format, - runtime->oss.buffer, - size1); - size1 /= runtime->channels; /* frames */ - fs = snd_enter_user(); - snd_pcm_lib_write(substream, (void __force __user *)runtime->oss.buffer, size1); - snd_leave_user(fs); - } - } else if (runtime->access == SNDRV_PCM_ACCESS_RW_NONINTERLEAVED) { - void __user *buffers[runtime->channels]; - memset(buffers, 0, runtime->channels * sizeof(void *)); - snd_pcm_lib_writev(substream, buffers, size); - } + if (runtime->access == SNDRV_PCM_ACCESS_RW_INTERLEAVED) + snd_pcm_lib_write(substream, NULL, size); + else if (runtime->access == SNDRV_PCM_ACCESS_RW_NONINTERLEAVED) + snd_pcm_lib_writev(substream, NULL, size); } mutex_unlock(&runtime->oss.params_lock); /*
The snd_pcm_oss_writev3() and snd_pcm_oss_readv3() are used only in io.c with CONFIG_SND_PCM_OSS_PLUGINS=y. Add an ifdef to reduce the build of these functions.
Along with it, since they are called always for in-kernel copy, reduce the argument and call snd_pcm_kernel_writev() and *_readv() directly instead.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/core/oss/io.c | 4 ++-- sound/core/oss/pcm_oss.c | 12 ++++++------ sound/core/oss/pcm_plugin.h | 6 ++---- 3 files changed, 10 insertions(+), 12 deletions(-)
diff --git a/sound/core/oss/io.c b/sound/core/oss/io.c index 6faa1d719206..d870b2d93135 100644 --- a/sound/core/oss/io.c +++ b/sound/core/oss/io.c @@ -26,9 +26,9 @@ #include "pcm_plugin.h"
#define pcm_write(plug,buf,count) snd_pcm_oss_write3(plug,buf,count,1) -#define pcm_writev(plug,vec,count) snd_pcm_oss_writev3(plug,vec,count,1) +#define pcm_writev(plug,vec,count) snd_pcm_oss_writev3(plug,vec,count) #define pcm_read(plug,buf,count) snd_pcm_oss_read3(plug,buf,count,1) -#define pcm_readv(plug,vec,count) snd_pcm_oss_readv3(plug,vec,count,1) +#define pcm_readv(plug,vec,count) snd_pcm_oss_readv3(plug,vec,count)
/* * Basic io plugin diff --git a/sound/core/oss/pcm_oss.c b/sound/core/oss/pcm_oss.c index 2d6a825cfe88..5e1009d959a8 100644 --- a/sound/core/oss/pcm_oss.c +++ b/sound/core/oss/pcm_oss.c @@ -1232,7 +1232,8 @@ snd_pcm_sframes_t snd_pcm_oss_read3(struct snd_pcm_substream *substream, char *p return ret; }
-snd_pcm_sframes_t snd_pcm_oss_writev3(struct snd_pcm_substream *substream, void **bufs, snd_pcm_uframes_t frames, int in_kernel) +#ifdef CONFIG_SND_PCM_OSS_PLUGINS +snd_pcm_sframes_t snd_pcm_oss_writev3(struct snd_pcm_substream *substream, void **bufs, snd_pcm_uframes_t frames) { struct snd_pcm_runtime *runtime = substream->runtime; int ret; @@ -1249,8 +1250,7 @@ snd_pcm_sframes_t snd_pcm_oss_writev3(struct snd_pcm_substream *substream, void if (ret < 0) break; } - ret = __snd_pcm_lib_xfer(substream, bufs, false, frames, - in_kernel); + ret = snd_pcm_kernel_writev(substream, bufs, frames); if (ret != -EPIPE && ret != -ESTRPIPE) break;
@@ -1262,7 +1262,7 @@ snd_pcm_sframes_t snd_pcm_oss_writev3(struct snd_pcm_substream *substream, void return ret; } -snd_pcm_sframes_t snd_pcm_oss_readv3(struct snd_pcm_substream *substream, void **bufs, snd_pcm_uframes_t frames, int in_kernel) +snd_pcm_sframes_t snd_pcm_oss_readv3(struct snd_pcm_substream *substream, void **bufs, snd_pcm_uframes_t frames) { struct snd_pcm_runtime *runtime = substream->runtime; int ret; @@ -1283,13 +1283,13 @@ snd_pcm_sframes_t snd_pcm_oss_readv3(struct snd_pcm_substream *substream, void * if (ret < 0) break; } - ret = __snd_pcm_lib_xfer(substream, bufs, false, frames, - in_kernel); + ret = snd_pcm_kernel_readv(substream, bufs, frames); if (ret != -EPIPE && ret != -ESTRPIPE) break; } return ret; } +#endif /* CONFIG_SND_PCM_OSS_PLUGINS */
static ssize_t snd_pcm_oss_write2(struct snd_pcm_substream *substream, const char *buf, size_t bytes, int in_kernel) { diff --git a/sound/core/oss/pcm_plugin.h b/sound/core/oss/pcm_plugin.h index 73c068abaca5..c9cd29d86efd 100644 --- a/sound/core/oss/pcm_plugin.h +++ b/sound/core/oss/pcm_plugin.h @@ -162,11 +162,9 @@ snd_pcm_sframes_t snd_pcm_oss_write3(struct snd_pcm_substream *substream, snd_pcm_sframes_t snd_pcm_oss_read3(struct snd_pcm_substream *substream, char *ptr, snd_pcm_uframes_t size, int in_kernel); snd_pcm_sframes_t snd_pcm_oss_writev3(struct snd_pcm_substream *substream, - void **bufs, snd_pcm_uframes_t frames, - int in_kernel); + void **bufs, snd_pcm_uframes_t frames); snd_pcm_sframes_t snd_pcm_oss_readv3(struct snd_pcm_substream *substream, - void **bufs, snd_pcm_uframes_t frames, - int in_kernel); + void **bufs, snd_pcm_uframes_t frames);
#else
Hi,
On May 26 2017 04:17, Takashi Iwai wrote:
Hi,
this is a sort of RFC, a revised patchset of the previous two patches for killing set_fs() usages in PCM. Instead of converting to the merged copy_silence ops, this adds a new copy_kernel ops instead. At the same time, copy/silence are changed to receive the position and size in bytes instead of frames. This allows us to simplify the PCM core code. As a result, a good amount of code could be removed from pcm_lib.c.
Note that this patchset isn't complete, containing only two driver conversions (nm256 for interleaved and rme9652 for non-interleaved) as a demonstration.
Takashi
===
Takashi Iwai (14): ALSA: pcm: Introduce copy_user, copy_kernel and fill_silence ops ALSA: nm256: Convert to new PCM copy ops ALSA: rme9652: Convert to the new PCM ops ALSA: pcm: Drop the old copy and silence ops ALSA: pcm: Check PCM state by a common helper function ALSA: pcm: Shuffle codes ALSA: pcm: Call directly the common read/write helpers ALSA: pcm: More unification of PCM transfer codes ALSA: pcm: Unify read/write loop ALSA: pcm: Simplify snd_pcm_playback_silence() ALSA: pcm: Direct in-kernel read/write support usb: gadget: u_uac1: Kill set_fs() usage ALSA: pcm: Kill set_fs() in PCM OSS layer ALSA: pcm: Build OSS writev/readv helpers conditionally
drivers/usb/gadget/function/u_uac1.c | 7 +- include/sound/pcm.h | 80 ++++- sound/core/oss/io.c | 4 +- sound/core/oss/pcm_oss.c | 81 +---- sound/core/oss/pcm_plugin.h | 6 +- sound/core/pcm_lib.c | 551 +++++++++++++---------------------- sound/pci/nm256/nm256.c | 57 ++-- sound/pci/rme9652/rme9652.c | 71 +++-- sound/soc/soc-pcm.c | 5 +- 9 files changed, 386 insertions(+), 476 deletions(-)
Overall, I prefer this patchset to your previous one, especially for patch 05-10 in a point of code refactoring. If patches of 00-04 and 05-10 were inverted in order, I would give my reviewed-by signature as soon...
Patch 00-04 looks good. Even if drivers transfer PCM frames for kernel space by PCM substream, 'struct snd_pcm_ops.fill_silence' has no influence. It's reasonable to have three operations, instead of two which we've proposed. But as you said, there's a lack of patches for the other drivers.
As I stated, patch 11-14 are pending from my current reviewing. It's for next week. (I need a batch of time to consider about it.)
Good night
Takashi Sakamoto
participants (2)
-
Takashi Iwai
-
Takashi Sakamoto