[alsa-devel] [PATCH RFC 00/26] Kill set_fs() in ALSA codes
Hi,
this is a slightly lengthy patchset, basically targeted for 4.13, thus it's still in RFC. The main purpose of this patchset is to get rid of get_fs() / set_fs() usages in ALSA codes.
The patchset starts from a patch that looks fairly irrelevant with the goal (the conversion of HD-audio bind-mute code), but it's a step for the long trail. The biggest change in the patchset is the conversion of copy & silence PCM ops into the new copy_silence ops. It helped for reducing the code size and for handling the in-kernel buffer copies.
The current patches are found in topic/kill-set_fs branch of my sound.git tree.
thanks,
Takashi
===
Takashi Iwai (26): ALSA: hda - Simplify bound-beep mute control for ALC268 ALSA: hda - Move bind-mixer switch codes to generic parser ALSA: hda - Remove the generic bind ctl helpers ALSA: hda - Remove the use of set_fs() ALSA: hda - Fix a typo in comment ALSA: hda - Remove superfluous header inclusions ALSA: opl3: Kill unused set_fs() ALSA: emu10k1: Get rid of set_fs() usage ALSA: pcm: Remove set_fs() in PCM core code ALSA: pcm: Introduce copy_silence PCM ops ALSA: Update document about copy_silence PCM ops ALSA: dummy: Convert to copy_silence ops ALSA: es1938: Convert to copy_silence ops ALSA: korg1212: Convert to copy_silence ops ALSA: nm256: Convert to copy_silence ops ALSA: rme32: Convert to copy_silence ops ALSA: rme96: Convert to copy_silence ops ALSA: rme9652: Convert to copy_silence ops ALSA: hdsp: Convert to copy_silence ops ALSA: gus: Convert to copy_silence ops ALSA: sb: Convert to copy_silence ops ALSA: sh: Convert to copy_silence ops ASoC: blackfin: Convert to copy_silence ops [media] solo6x10: Convert to copy_silence ops ALSA: pcm: Drop the old copy and silence ops ALSA: pcm: Kill set_fs() usage in OSS layer and USB gadget
.../sound/kernel-api/writing-an-alsa-driver.rst | 110 +++++---- drivers/media/pci/solo6x10/solo6x10-g723.c | 11 +- drivers/usb/gadget/function/u_uac1.c | 8 +- include/sound/pcm.h | 19 +- sound/core/oss/pcm_oss.c | 62 ++--- sound/core/pcm_compat.c | 22 +- sound/core/pcm_lib.c | 174 +++++++++----- sound/core/pcm_native.c | 122 ++++++---- sound/drivers/dummy.c | 13 +- sound/drivers/opl3/opl3_oss.c | 14 -- sound/isa/gus/gus_pcm.c | 43 +--- sound/isa/sb/emu8000_pcm.c | 99 +++----- sound/pci/emu10k1/emufx.c | 127 ++++++----- sound/pci/es1938.c | 11 +- sound/pci/hda/hda_codec.c | 251 +++------------------ sound/pci/hda/hda_generic.c | 46 +++- sound/pci/hda/hda_local.h | 61 ----- sound/pci/hda/patch_realtek.c | 37 ++- sound/pci/korg1212/korg1212.c | 128 +++-------- sound/pci/nm256/nm256.c | 35 ++- sound/pci/rme32.c | 49 ++-- sound/pci/rme96.c | 52 ++--- sound/pci/rme9652/hdsp.c | 44 ++-- sound/pci/rme9652/rme9652.c | 46 ++-- sound/sh/sh_dac_audio.c | 40 +--- sound/soc/blackfin/bf5xx-ac97-pcm.c | 6 +- sound/soc/blackfin/bf5xx-ac97.c | 18 +- sound/soc/blackfin/bf5xx-i2s-pcm.c | 46 ++-- sound/soc/soc-pcm.c | 3 +- 29 files changed, 696 insertions(+), 1001 deletions(-)
The beep mute switch for ALC268 needs to touch two NIDs, and we used to apply the bind-mixer stuff. But the use case for ALC268 is fairly easy to convert to an open-code in a shorter form. Since this is the only user of the bind-ctls, we can clean up the common helper codes later.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/pci/hda/patch_realtek.c | 37 ++++++++++++++++++++++++++++--------- 1 file changed, 28 insertions(+), 9 deletions(-)
diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c index 58df440013c5..87e1368de4d5 100644 --- a/sound/pci/hda/patch_realtek.c +++ b/sound/pci/hda/patch_realtek.c @@ -2573,18 +2573,37 @@ static int patch_alc262(struct hda_codec *codec) * ALC268 */ /* bind Beep switches of both NID 0x0f and 0x10 */ -static const struct hda_bind_ctls alc268_bind_beep_sw = { - .ops = &snd_hda_bind_sw, - .values = { - HDA_COMPOSE_AMP_VAL(0x0f, 3, 1, HDA_INPUT), - HDA_COMPOSE_AMP_VAL(0x10, 3, 1, HDA_INPUT), - 0 - }, -}; +static int alc268_beep_switch_put(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct hda_codec *codec = snd_kcontrol_chip(kcontrol); + unsigned long pval; + int err; + + mutex_lock(&codec->control_mutex); + pval = kcontrol->private_value; + kcontrol->private_value = (pval & ~0xff) | 0x0f; + err = snd_hda_mixer_amp_switch_put(kcontrol, ucontrol); + if (err >= 0) { + kcontrol->private_value = (pval & ~0xff) | 0x10; + err = snd_hda_mixer_amp_switch_put(kcontrol, ucontrol); + } + kcontrol->private_value = pval; + mutex_unlock(&codec->control_mutex); + return err; +}
static const struct snd_kcontrol_new alc268_beep_mixer[] = { HDA_CODEC_VOLUME("Beep Playback Volume", 0x1d, 0x0, HDA_INPUT), - HDA_BIND_SW("Beep Playback Switch", &alc268_bind_beep_sw), + { + .iface = SNDRV_CTL_ELEM_IFACE_MIXER, + .name = "Beep Playback Switch", + .subdevice = HDA_SUBDEV_AMP_FLAG, + .info = snd_hda_mixer_amp_switch_info, + .get = snd_hda_mixer_amp_switch_get, + .put = alc268_beep_switch_put, + .private_value = HDA_COMPOSE_AMP_VAL(0x0f, 3, 1, HDA_INPUT) + }, { } };
The generic parser is the only user of the bind-mixer controls, so we can move the code there and clean up the core helper.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/pci/hda/hda_codec.c | 66 --------------------------------------------- sound/pci/hda/hda_generic.c | 46 +++++++++++++++++++++++++++++-- sound/pci/hda/hda_local.h | 17 ------------ 3 files changed, 44 insertions(+), 85 deletions(-)
diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c index 70bb365a08d2..29f243679a21 100644 --- a/sound/pci/hda/hda_codec.c +++ b/sound/pci/hda/hda_codec.c @@ -2117,72 +2117,6 @@ int snd_hda_mixer_amp_switch_put(struct snd_kcontrol *kcontrol, } EXPORT_SYMBOL_GPL(snd_hda_mixer_amp_switch_put);
-/* - * bound volume controls - * - * bind multiple volumes (# indices, from 0) - */ - -#define AMP_VAL_IDX_SHIFT 19 -#define AMP_VAL_IDX_MASK (0x0f<<19) - -/** - * snd_hda_mixer_bind_switch_get - Get callback for a bound volume control - * @kcontrol: ctl element - * @ucontrol: pointer to get/store the data - * - * The control element is supposed to have the private_value field - * set up via HDA_BIND_MUTE*() macros. - */ -int snd_hda_mixer_bind_switch_get(struct snd_kcontrol *kcontrol, - struct snd_ctl_elem_value *ucontrol) -{ - struct hda_codec *codec = snd_kcontrol_chip(kcontrol); - unsigned long pval; - int err; - - mutex_lock(&codec->control_mutex); - pval = kcontrol->private_value; - kcontrol->private_value = pval & ~AMP_VAL_IDX_MASK; /* index 0 */ - err = snd_hda_mixer_amp_switch_get(kcontrol, ucontrol); - kcontrol->private_value = pval; - mutex_unlock(&codec->control_mutex); - return err; -} -EXPORT_SYMBOL_GPL(snd_hda_mixer_bind_switch_get); - -/** - * snd_hda_mixer_bind_switch_put - Put callback for a bound volume control - * @kcontrol: ctl element - * @ucontrol: pointer to get/store the data - * - * The control element is supposed to have the private_value field - * set up via HDA_BIND_MUTE*() macros. - */ -int snd_hda_mixer_bind_switch_put(struct snd_kcontrol *kcontrol, - struct snd_ctl_elem_value *ucontrol) -{ - struct hda_codec *codec = snd_kcontrol_chip(kcontrol); - unsigned long pval; - int i, indices, err = 0, change = 0; - - mutex_lock(&codec->control_mutex); - pval = kcontrol->private_value; - indices = (pval & AMP_VAL_IDX_MASK) >> AMP_VAL_IDX_SHIFT; - for (i = 0; i < indices; i++) { - kcontrol->private_value = (pval & ~AMP_VAL_IDX_MASK) | - (i << AMP_VAL_IDX_SHIFT); - err = snd_hda_mixer_amp_switch_put(kcontrol, ucontrol); - if (err < 0) - break; - change |= err; - } - kcontrol->private_value = pval; - mutex_unlock(&codec->control_mutex); - return err < 0 ? err : change; -} -EXPORT_SYMBOL_GPL(snd_hda_mixer_bind_switch_put); - /** * snd_hda_mixer_bind_ctls_info - Info callback for a generic bound control * @kcontrol: referred ctl element diff --git a/sound/pci/hda/hda_generic.c b/sound/pci/hda/hda_generic.c index 2842c82363c0..557ecfcad158 100644 --- a/sound/pci/hda/hda_generic.c +++ b/sound/pci/hda/hda_generic.c @@ -948,6 +948,8 @@ static void resume_path_from_idx(struct hda_codec *codec, int path_idx)
static int hda_gen_mixer_mute_put(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol); +static int hda_gen_bind_mute_get(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol); static int hda_gen_bind_mute_put(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol);
@@ -970,7 +972,7 @@ static const struct snd_kcontrol_new control_templates[] = { { .iface = SNDRV_CTL_ELEM_IFACE_MIXER, .info = snd_hda_mixer_amp_switch_info, - .get = snd_hda_mixer_bind_switch_get, + .get = hda_gen_bind_mute_get, .put = hda_gen_bind_mute_put, /* replaced */ .private_value = HDA_COMPOSE_AMP_VAL(0, 3, 0, 0), }, @@ -1101,11 +1103,51 @@ static int hda_gen_mixer_mute_put(struct snd_kcontrol *kcontrol, return snd_hda_mixer_amp_switch_put(kcontrol, ucontrol); }
+/* + * Bound mute controls + */ +#define AMP_VAL_IDX_SHIFT 19 +#define AMP_VAL_IDX_MASK (0x0f<<19) + +static int hda_gen_bind_mute_get(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct hda_codec *codec = snd_kcontrol_chip(kcontrol); + unsigned long pval; + int err; + + mutex_lock(&codec->control_mutex); + pval = kcontrol->private_value; + kcontrol->private_value = pval & ~AMP_VAL_IDX_MASK; /* index 0 */ + err = snd_hda_mixer_amp_switch_get(kcontrol, ucontrol); + kcontrol->private_value = pval; + mutex_unlock(&codec->control_mutex); + return err; +} + static int hda_gen_bind_mute_put(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol) { + struct hda_codec *codec = snd_kcontrol_chip(kcontrol); + unsigned long pval; + int i, indices, err = 0, change = 0; + sync_auto_mute_bits(kcontrol, ucontrol); - return snd_hda_mixer_bind_switch_put(kcontrol, ucontrol); + + mutex_lock(&codec->control_mutex); + pval = kcontrol->private_value; + indices = (pval & AMP_VAL_IDX_MASK) >> AMP_VAL_IDX_SHIFT; + for (i = 0; i < indices; i++) { + kcontrol->private_value = (pval & ~AMP_VAL_IDX_MASK) | + (i << AMP_VAL_IDX_SHIFT); + err = snd_hda_mixer_amp_switch_put(kcontrol, ucontrol); + if (err < 0) + break; + change |= err; + } + kcontrol->private_value = pval; + mutex_unlock(&codec->control_mutex); + return err < 0 ? err : change; }
/* any ctl assigned to the path with the given index? */ diff --git a/sound/pci/hda/hda_local.h b/sound/pci/hda/hda_local.h index d0e066e4c985..b73339199a8b 100644 --- a/sound/pci/hda/hda_local.h +++ b/sound/pci/hda/hda_local.h @@ -178,23 +178,6 @@ void snd_hda_sync_vmaster_hook(struct hda_vmaster_mute_hook *hook); #define HDA_AMP_UNMUTE 0x00 #define HDA_AMP_VOLMASK 0x7f
-/* mono switch binding multiple inputs */ -#define HDA_BIND_MUTE_MONO(xname, nid, channel, indices, direction) \ - { .iface = SNDRV_CTL_ELEM_IFACE_MIXER, .name = xname, .index = 0, \ - .info = snd_hda_mixer_amp_switch_info, \ - .get = snd_hda_mixer_bind_switch_get, \ - .put = snd_hda_mixer_bind_switch_put, \ - .private_value = HDA_COMPOSE_AMP_VAL(nid, channel, indices, direction) } - -/* stereo switch binding multiple inputs */ -#define HDA_BIND_MUTE(xname,nid,indices,dir) \ - HDA_BIND_MUTE_MONO(xname,nid,3,indices,dir) - -int snd_hda_mixer_bind_switch_get(struct snd_kcontrol *kcontrol, - struct snd_ctl_elem_value *ucontrol); -int snd_hda_mixer_bind_switch_put(struct snd_kcontrol *kcontrol, - struct snd_ctl_elem_value *ucontrol); - /* more generic bound controls */ struct hda_ctl_ops { snd_kcontrol_info_t *info;
Now all the users of this workaround code is gone, and we can finally remove the legacy codes from the core HD-audio module.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/pci/hda/hda_codec.c | 124 ---------------------------------------------- sound/pci/hda/hda_local.h | 44 ---------------- 2 files changed, 168 deletions(-)
diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c index 29f243679a21..187a9c717fb5 100644 --- a/sound/pci/hda/hda_codec.c +++ b/sound/pci/hda/hda_codec.c @@ -2117,130 +2117,6 @@ int snd_hda_mixer_amp_switch_put(struct snd_kcontrol *kcontrol, } EXPORT_SYMBOL_GPL(snd_hda_mixer_amp_switch_put);
-/** - * snd_hda_mixer_bind_ctls_info - Info callback for a generic bound control - * @kcontrol: referred ctl element - * @uinfo: pointer to get/store the data - * - * The control element is supposed to have the private_value field - * set up via HDA_BIND_VOL() or HDA_BIND_SW() macros. - */ -int snd_hda_mixer_bind_ctls_info(struct snd_kcontrol *kcontrol, - struct snd_ctl_elem_info *uinfo) -{ - struct hda_codec *codec = snd_kcontrol_chip(kcontrol); - struct hda_bind_ctls *c; - int err; - - mutex_lock(&codec->control_mutex); - c = (struct hda_bind_ctls *)kcontrol->private_value; - kcontrol->private_value = *c->values; - err = c->ops->info(kcontrol, uinfo); - kcontrol->private_value = (long)c; - mutex_unlock(&codec->control_mutex); - return err; -} -EXPORT_SYMBOL_GPL(snd_hda_mixer_bind_ctls_info); - -/** - * snd_hda_mixer_bind_ctls_get - Get callback for a generic bound control - * @kcontrol: ctl element - * @ucontrol: pointer to get/store the data - * - * The control element is supposed to have the private_value field - * set up via HDA_BIND_VOL() or HDA_BIND_SW() macros. - */ -int snd_hda_mixer_bind_ctls_get(struct snd_kcontrol *kcontrol, - struct snd_ctl_elem_value *ucontrol) -{ - struct hda_codec *codec = snd_kcontrol_chip(kcontrol); - struct hda_bind_ctls *c; - int err; - - mutex_lock(&codec->control_mutex); - c = (struct hda_bind_ctls *)kcontrol->private_value; - kcontrol->private_value = *c->values; - err = c->ops->get(kcontrol, ucontrol); - kcontrol->private_value = (long)c; - mutex_unlock(&codec->control_mutex); - return err; -} -EXPORT_SYMBOL_GPL(snd_hda_mixer_bind_ctls_get); - -/** - * snd_hda_mixer_bind_ctls_put - Put callback for a generic bound control - * @kcontrol: ctl element - * @ucontrol: pointer to get/store the data - * - * The control element is supposed to have the private_value field - * set up via HDA_BIND_VOL() or HDA_BIND_SW() macros. - */ -int snd_hda_mixer_bind_ctls_put(struct snd_kcontrol *kcontrol, - struct snd_ctl_elem_value *ucontrol) -{ - struct hda_codec *codec = snd_kcontrol_chip(kcontrol); - struct hda_bind_ctls *c; - unsigned long *vals; - int err = 0, change = 0; - - mutex_lock(&codec->control_mutex); - c = (struct hda_bind_ctls *)kcontrol->private_value; - for (vals = c->values; *vals; vals++) { - kcontrol->private_value = *vals; - err = c->ops->put(kcontrol, ucontrol); - if (err < 0) - break; - change |= err; - } - kcontrol->private_value = (long)c; - mutex_unlock(&codec->control_mutex); - return err < 0 ? err : change; -} -EXPORT_SYMBOL_GPL(snd_hda_mixer_bind_ctls_put); - -/** - * snd_hda_mixer_bind_tlv - TLV callback for a generic bound control - * @kcontrol: ctl element - * @op_flag: operation flag - * @size: byte size of input TLV - * @tlv: TLV data - * - * The control element is supposed to have the private_value field - * set up via HDA_BIND_VOL() macro. - */ -int snd_hda_mixer_bind_tlv(struct snd_kcontrol *kcontrol, int op_flag, - unsigned int size, unsigned int __user *tlv) -{ - struct hda_codec *codec = snd_kcontrol_chip(kcontrol); - struct hda_bind_ctls *c; - int err; - - mutex_lock(&codec->control_mutex); - c = (struct hda_bind_ctls *)kcontrol->private_value; - kcontrol->private_value = *c->values; - err = c->ops->tlv(kcontrol, op_flag, size, tlv); - kcontrol->private_value = (long)c; - mutex_unlock(&codec->control_mutex); - return err; -} -EXPORT_SYMBOL_GPL(snd_hda_mixer_bind_tlv); - -struct hda_ctl_ops snd_hda_bind_vol = { - .info = snd_hda_mixer_amp_volume_info, - .get = snd_hda_mixer_amp_volume_get, - .put = snd_hda_mixer_amp_volume_put, - .tlv = snd_hda_mixer_amp_tlv -}; -EXPORT_SYMBOL_GPL(snd_hda_bind_vol); - -struct hda_ctl_ops snd_hda_bind_sw = { - .info = snd_hda_mixer_amp_switch_info, - .get = snd_hda_mixer_amp_switch_get, - .put = snd_hda_mixer_amp_switch_put, - .tlv = snd_hda_mixer_amp_tlv -}; -EXPORT_SYMBOL_GPL(snd_hda_bind_sw); - /* * SPDIF out controls */ diff --git a/sound/pci/hda/hda_local.h b/sound/pci/hda/hda_local.h index b73339199a8b..5b5c324c99b9 100644 --- a/sound/pci/hda/hda_local.h +++ b/sound/pci/hda/hda_local.h @@ -178,50 +178,6 @@ void snd_hda_sync_vmaster_hook(struct hda_vmaster_mute_hook *hook); #define HDA_AMP_UNMUTE 0x00 #define HDA_AMP_VOLMASK 0x7f
-/* more generic bound controls */ -struct hda_ctl_ops { - snd_kcontrol_info_t *info; - snd_kcontrol_get_t *get; - snd_kcontrol_put_t *put; - snd_kcontrol_tlv_rw_t *tlv; -}; - -extern struct hda_ctl_ops snd_hda_bind_vol; /* for bind-volume with TLV */ -extern struct hda_ctl_ops snd_hda_bind_sw; /* for bind-switch */ - -struct hda_bind_ctls { - struct hda_ctl_ops *ops; - unsigned long values[]; -}; - -int snd_hda_mixer_bind_ctls_info(struct snd_kcontrol *kcontrol, - struct snd_ctl_elem_info *uinfo); -int snd_hda_mixer_bind_ctls_get(struct snd_kcontrol *kcontrol, - struct snd_ctl_elem_value *ucontrol); -int snd_hda_mixer_bind_ctls_put(struct snd_kcontrol *kcontrol, - struct snd_ctl_elem_value *ucontrol); -int snd_hda_mixer_bind_tlv(struct snd_kcontrol *kcontrol, int op_flag, - unsigned int size, unsigned int __user *tlv); - -#define HDA_BIND_VOL(xname, bindrec) \ - { .iface = SNDRV_CTL_ELEM_IFACE_MIXER, \ - .name = xname, \ - .access = SNDRV_CTL_ELEM_ACCESS_READWRITE |\ - SNDRV_CTL_ELEM_ACCESS_TLV_READ |\ - SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK,\ - .info = snd_hda_mixer_bind_ctls_info,\ - .get = snd_hda_mixer_bind_ctls_get,\ - .put = snd_hda_mixer_bind_ctls_put,\ - .tlv = { .c = snd_hda_mixer_bind_tlv },\ - .private_value = (long) (bindrec) } -#define HDA_BIND_SW(xname, bindrec) \ - { .iface = SNDRV_CTL_ELEM_IFACE_MIXER,\ - .name = xname, \ - .info = snd_hda_mixer_bind_ctls_info,\ - .get = snd_hda_mixer_bind_ctls_get,\ - .put = snd_hda_mixer_bind_ctls_put,\ - .private_value = (long) (bindrec) } - /* * SPDIF I/O */
set_fs() is used in HD-audio vmaster code to retrieve the TLV data of each slave kctl. Since the slave is supposed to be a standard amp kctl, we can call directly the supposed tlv callback instead of the indirect call, so that we can remove the set_fs() hack.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/pci/hda/hda_codec.c | 59 +++++++++++++++++++++++++---------------------- 1 file changed, 31 insertions(+), 28 deletions(-)
diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c index 187a9c717fb5..ff9c7968b562 100644 --- a/sound/pci/hda/hda_codec.c +++ b/sound/pci/hda/hda_codec.c @@ -1477,18 +1477,8 @@ int snd_hda_mixer_amp_volume_put(struct snd_kcontrol *kcontrol, } EXPORT_SYMBOL_GPL(snd_hda_mixer_amp_volume_put);
-/** - * snd_hda_mixer_amp_volume_put - TLV callback for a standard AMP mixer volume - * @kcontrol: ctl element - * @op_flag: operation flag - * @size: byte size of input TLV - * @_tlv: TLV data - * - * The control element is supposed to have the private_value field - * set up via HDA_COMPOSE_AMP_VAL*() or related macros. - */ -int snd_hda_mixer_amp_tlv(struct snd_kcontrol *kcontrol, int op_flag, - unsigned int size, unsigned int __user *_tlv) +/* inquiry the amp caps and convert to TLV */ +static void get_ctl_amp_tlv(struct snd_kcontrol *kcontrol, unsigned int *tlv) { struct hda_codec *codec = snd_kcontrol_chip(kcontrol); hda_nid_t nid = get_amp_nid(kcontrol); @@ -1497,8 +1487,6 @@ int snd_hda_mixer_amp_tlv(struct snd_kcontrol *kcontrol, int op_flag, bool min_mute = get_amp_min_mute(kcontrol); u32 caps, val1, val2;
- if (size < 4 * sizeof(unsigned int)) - return -ENOMEM; caps = query_amp_caps(codec, nid, dir); val2 = (caps & AC_AMPCAP_STEP_SIZE) >> AC_AMPCAP_STEP_SIZE_SHIFT; val2 = (val2 + 1) * 25; @@ -1507,13 +1495,31 @@ int snd_hda_mixer_amp_tlv(struct snd_kcontrol *kcontrol, int op_flag, val1 = ((int)val1) * ((int)val2); if (min_mute || (caps & AC_AMPCAP_MIN_MUTE)) val2 |= TLV_DB_SCALE_MUTE; - if (put_user(SNDRV_CTL_TLVT_DB_SCALE, _tlv)) - return -EFAULT; - if (put_user(2 * sizeof(unsigned int), _tlv + 1)) - return -EFAULT; - if (put_user(val1, _tlv + 2)) - return -EFAULT; - if (put_user(val2, _tlv + 3)) + tlv[0] = SNDRV_CTL_TLVT_DB_SCALE; + tlv[1] = 2 * sizeof(unsigned int); + tlv[2] = val1; + tlv[3] = val2; +} + +/** + * snd_hda_mixer_amp_volume_put - TLV callback for a standard AMP mixer volume + * @kcontrol: ctl element + * @op_flag: operation flag + * @size: byte size of input TLV + * @_tlv: TLV data + * + * The control element is supposed to have the private_value field + * set up via HDA_COMPOSE_AMP_VAL*() or related macros. + */ +int snd_hda_mixer_amp_tlv(struct snd_kcontrol *kcontrol, int op_flag, + unsigned int size, unsigned int __user *_tlv) +{ + unsigned int tlv[4]; + + if (size < 4 * sizeof(unsigned int)) + return -ENOMEM; + get_ctl_amp_tlv(kcontrol, tlv); + if (copy_to_user(_tlv, tlv, sizeof(tlv))) return -EFAULT; return 0; } @@ -1807,13 +1813,10 @@ static int get_kctl_0dB_offset(struct hda_codec *codec, const int *tlv = NULL; int val = -1;
- if (kctl->vd[0].access & SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK) { - /* FIXME: set_fs() hack for obtaining user-space TLV data */ - mm_segment_t fs = get_fs(); - set_fs(get_ds()); - if (!kctl->tlv.c(kctl, 0, sizeof(_tlv), _tlv)) - tlv = _tlv; - set_fs(fs); + if ((kctl->vd[0].access & SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK) && + kctl->tlv.c == snd_hda_mixer_amp_tlv) { + get_ctl_amp_tlv(kctl, _tlv); + tlv = _tlv; } else if (kctl->vd[0].access & SNDRV_CTL_ELEM_ACCESS_TLV_READ) tlv = kctl->tlv.p; if (tlv && tlv[0] == SNDRV_CTL_TLVT_DB_SCALE) {
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/pci/hda/hda_codec.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c index ff9c7968b562..0593d674de95 100644 --- a/sound/pci/hda/hda_codec.c +++ b/sound/pci/hda/hda_codec.c @@ -1502,7 +1502,7 @@ static void get_ctl_amp_tlv(struct snd_kcontrol *kcontrol, unsigned int *tlv) }
/** - * snd_hda_mixer_amp_volume_put - TLV callback for a standard AMP mixer volume + * snd_hda_mixer_amp_tlv - TLV callback for a standard AMP mixer volume * @kcontrol: ctl element * @op_flag: operation flag * @size: byte size of input TLV
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/pci/hda/hda_codec.c | 2 -- 1 file changed, 2 deletions(-)
diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c index 0593d674de95..821aad374a06 100644 --- a/sound/pci/hda/hda_codec.c +++ b/sound/pci/hda/hda_codec.c @@ -19,13 +19,11 @@ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA */
-#include <linux/mm.h> #include <linux/init.h> #include <linux/delay.h> #include <linux/slab.h> #include <linux/mutex.h> #include <linux/module.h> -#include <linux/async.h> #include <linux/pm.h> #include <linux/pm_runtime.h> #include <sound/core.h>
snd_enter_user() and snd_leave_user() that call set_fs() are the dead code in opl3 driver. Let's rip them off.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/drivers/opl3/opl3_oss.c | 14 -------------- 1 file changed, 14 deletions(-)
diff --git a/sound/drivers/opl3/opl3_oss.c b/sound/drivers/opl3/opl3_oss.c index c1cb249acfaa..22c3e4bca220 100644 --- a/sound/drivers/opl3/opl3_oss.c +++ b/sound/drivers/opl3/opl3_oss.c @@ -27,20 +27,6 @@ static int snd_opl3_ioctl_seq_oss(struct snd_seq_oss_arg *arg, unsigned int cmd, static int snd_opl3_load_patch_seq_oss(struct snd_seq_oss_arg *arg, int format, const char __user *buf, int offs, int count); static int snd_opl3_reset_seq_oss(struct snd_seq_oss_arg *arg);
-/* */ - -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); -} - /* operators */
extern struct snd_midi_op opl3_ops;
Instead of set_fs() hackery, do the straight memcpy() by passing a flag indicating the kernel space operation.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/pci/emu10k1/emufx.c | 127 ++++++++++++++++++++++++++-------------------- 1 file changed, 73 insertions(+), 54 deletions(-)
diff --git a/sound/pci/emu10k1/emufx.c b/sound/pci/emu10k1/emufx.c index 56fc47bd6dba..dc585959ca32 100644 --- a/sound/pci/emu10k1/emufx.c +++ b/sound/pci/emu10k1/emufx.c @@ -311,21 +311,6 @@ static const u32 onoff_table[2] = { };
/* - */ - -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); -} - -/* * controls */
@@ -538,7 +523,8 @@ unsigned int snd_emu10k1_efx_read(struct snd_emu10k1 *emu, unsigned int pc) }
static int snd_emu10k1_gpr_poke(struct snd_emu10k1 *emu, - struct snd_emu10k1_fx8010_code *icode) + struct snd_emu10k1_fx8010_code *icode, + bool in_kernel) { int gpr; u32 val; @@ -546,7 +532,9 @@ static int snd_emu10k1_gpr_poke(struct snd_emu10k1 *emu, for (gpr = 0; gpr < (emu->audigy ? 0x200 : 0x100); gpr++) { if (!test_bit(gpr, icode->gpr_valid)) continue; - if (get_user(val, &icode->gpr_map[gpr])) + if (in_kernel) + val = *(u32 *)&icode->gpr_map[gpr]; + else if (get_user(val, &icode->gpr_map[gpr])) return -EFAULT; snd_emu10k1_ptr_write(emu, emu->gpr_base + gpr, 0, val); } @@ -569,7 +557,8 @@ static int snd_emu10k1_gpr_peek(struct snd_emu10k1 *emu, }
static int snd_emu10k1_tram_poke(struct snd_emu10k1 *emu, - struct snd_emu10k1_fx8010_code *icode) + struct snd_emu10k1_fx8010_code *icode, + bool in_kernel) { int tram; u32 addr, val; @@ -577,9 +566,14 @@ static int snd_emu10k1_tram_poke(struct snd_emu10k1 *emu, for (tram = 0; tram < (emu->audigy ? 0x100 : 0xa0); tram++) { if (!test_bit(tram, icode->tram_valid)) continue; - if (get_user(val, &icode->tram_data_map[tram]) || - get_user(addr, &icode->tram_addr_map[tram])) - return -EFAULT; + if (in_kernel) { + val = *(u32 *)&icode->tram_data_map[tram]; + addr = *(u32 *)&icode->tram_addr_map[tram]; + } else { + if (get_user(val, &icode->tram_data_map[tram]) || + get_user(addr, &icode->tram_addr_map[tram])) + return -EFAULT; + } snd_emu10k1_ptr_write(emu, TANKMEMDATAREGBASE + tram, 0, val); if (!emu->audigy) { snd_emu10k1_ptr_write(emu, TANKMEMADDRREGBASE + tram, 0, addr); @@ -615,16 +609,22 @@ static int snd_emu10k1_tram_peek(struct snd_emu10k1 *emu, }
static int snd_emu10k1_code_poke(struct snd_emu10k1 *emu, - struct snd_emu10k1_fx8010_code *icode) + struct snd_emu10k1_fx8010_code *icode, + bool in_kernel) { u32 pc, lo, hi;
for (pc = 0; pc < (emu->audigy ? 2*1024 : 2*512); pc += 2) { if (!test_bit(pc / 2, icode->code_valid)) continue; - if (get_user(lo, &icode->code[pc + 0]) || - get_user(hi, &icode->code[pc + 1])) - return -EFAULT; + if (in_kernel) { + lo = *(u32 *)&icode->code[pc + 0]; + hi = *(u32 *)&icode->code[pc + 1]; + } else { + if (get_user(lo, &icode->code[pc + 0]) || + get_user(hi, &icode->code[pc + 1])) + return -EFAULT; + } snd_emu10k1_efx_write(emu, pc + 0, lo); snd_emu10k1_efx_write(emu, pc + 1, hi); } @@ -665,14 +665,16 @@ snd_emu10k1_look_for_ctl(struct snd_emu10k1 *emu, struct snd_ctl_elem_id *id)
#define MAX_TLV_SIZE 256
-static unsigned int *copy_tlv(const unsigned int __user *_tlv) +static unsigned int *copy_tlv(const unsigned int __user *_tlv, bool in_kernel) { unsigned int data[2]; unsigned int *tlv;
if (!_tlv) return NULL; - if (copy_from_user(data, _tlv, sizeof(data))) + if (in_kernel) + memcpy(data, (void *)_tlv, sizeof(data)); + else if (copy_from_user(data, _tlv, sizeof(data))) return NULL; if (data[1] >= MAX_TLV_SIZE) return NULL; @@ -680,7 +682,9 @@ static unsigned int *copy_tlv(const unsigned int __user *_tlv) if (!tlv) return NULL; memcpy(tlv, data, sizeof(data)); - if (copy_from_user(tlv + 2, _tlv + 2, data[1])) { + if (in_kernel) { + memcpy(tlv + 2, (void *)(_tlv + 2), data[1]); + } else if (copy_from_user(tlv + 2, _tlv + 2, data[1])) { kfree(tlv); return NULL; } @@ -690,7 +694,7 @@ static unsigned int *copy_tlv(const unsigned int __user *_tlv) static int copy_gctl(struct snd_emu10k1 *emu, struct snd_emu10k1_fx8010_control_gpr *gctl, struct snd_emu10k1_fx8010_control_gpr __user *_gctl, - int idx) + int idx, bool in_kernel) { struct snd_emu10k1_fx8010_control_old_gpr __user *octl;
@@ -718,7 +722,8 @@ static int copy_gctl_to_user(struct snd_emu10k1 *emu, }
static int snd_emu10k1_verify_controls(struct snd_emu10k1 *emu, - struct snd_emu10k1_fx8010_code *icode) + struct snd_emu10k1_fx8010_code *icode, + bool in_kernel) { unsigned int i; struct snd_ctl_elem_id __user *_id; @@ -728,7 +733,9 @@ static int snd_emu10k1_verify_controls(struct snd_emu10k1 *emu, for (i = 0, _id = icode->gpr_del_controls; i < icode->gpr_del_control_count; i++, _id++) { - if (copy_from_user(&id, _id, sizeof(id))) + if (in_kernel) + id = *(struct snd_ctl_elem_id *)_id; + else if (copy_from_user(&id, _id, sizeof(id))) return -EFAULT; if (snd_emu10k1_look_for_ctl(emu, &id) == NULL) return -ENOENT; @@ -738,7 +745,8 @@ static int snd_emu10k1_verify_controls(struct snd_emu10k1 *emu, return -ENOMEM; err = 0; for (i = 0; i < icode->gpr_add_control_count; i++) { - if (copy_gctl(emu, gctl, icode->gpr_add_controls, i)) { + if (copy_gctl(emu, gctl, icode->gpr_add_controls, i, + in_kernel)) { err = -EFAULT; goto __error; } @@ -759,7 +767,8 @@ static int snd_emu10k1_verify_controls(struct snd_emu10k1 *emu, } for (i = 0; i < icode->gpr_list_control_count; i++) { /* FIXME: we need to check the WRITE access */ - if (copy_gctl(emu, gctl, icode->gpr_list_controls, i)) { + if (copy_gctl(emu, gctl, icode->gpr_list_controls, i, + in_kernel)) { err = -EFAULT; goto __error; } @@ -781,7 +790,8 @@ static void snd_emu10k1_ctl_private_free(struct snd_kcontrol *kctl) }
static int snd_emu10k1_add_controls(struct snd_emu10k1 *emu, - struct snd_emu10k1_fx8010_code *icode) + struct snd_emu10k1_fx8010_code *icode, + bool in_kernel) { unsigned int i, j; struct snd_emu10k1_fx8010_control_gpr *gctl; @@ -800,7 +810,8 @@ static int snd_emu10k1_add_controls(struct snd_emu10k1 *emu, }
for (i = 0; i < icode->gpr_add_control_count; i++) { - if (copy_gctl(emu, gctl, icode->gpr_add_controls, i)) { + if (copy_gctl(emu, gctl, icode->gpr_add_controls, i, + in_kernel)) { err = -EFAULT; goto __error; } @@ -821,7 +832,7 @@ static int snd_emu10k1_add_controls(struct snd_emu10k1 *emu, knew.device = gctl->id.device; knew.subdevice = gctl->id.subdevice; knew.info = snd_emu10k1_gpr_ctl_info; - knew.tlv.p = copy_tlv(gctl->tlv); + knew.tlv.p = copy_tlv(gctl->tlv, in_kernel); if (knew.tlv.p) knew.access = SNDRV_CTL_ELEM_ACCESS_READWRITE | SNDRV_CTL_ELEM_ACCESS_TLV_READ; @@ -873,7 +884,8 @@ static int snd_emu10k1_add_controls(struct snd_emu10k1 *emu, }
static int snd_emu10k1_del_controls(struct snd_emu10k1 *emu, - struct snd_emu10k1_fx8010_code *icode) + struct snd_emu10k1_fx8010_code *icode, + bool in_kernel) { unsigned int i; struct snd_ctl_elem_id id; @@ -883,7 +895,9 @@ static int snd_emu10k1_del_controls(struct snd_emu10k1 *emu, for (i = 0, _id = icode->gpr_del_controls; i < icode->gpr_del_control_count; i++, _id++) { - if (copy_from_user(&id, _id, sizeof(id))) + if (in_kernel) + id = *(struct snd_ctl_elem_id *)_id; + else if (copy_from_user(&id, _id, sizeof(id))) return -EFAULT; down_write(&card->controls_rwsem); ctl = snd_emu10k1_look_for_ctl(emu, &id); @@ -941,12 +955,14 @@ static int snd_emu10k1_list_controls(struct snd_emu10k1 *emu, }
static int snd_emu10k1_icode_poke(struct snd_emu10k1 *emu, - struct snd_emu10k1_fx8010_code *icode) + struct snd_emu10k1_fx8010_code *icode, + bool in_kernel) { int err = 0;
mutex_lock(&emu->fx8010.lock); - if ((err = snd_emu10k1_verify_controls(emu, icode)) < 0) + err = snd_emu10k1_verify_controls(emu, icode, in_kernel); + if (err < 0) goto __error; strlcpy(emu->fx8010.name, icode->name, sizeof(emu->fx8010.name)); /* stop FX processor - this may be dangerous, but it's better to miss @@ -956,11 +972,20 @@ static int snd_emu10k1_icode_poke(struct snd_emu10k1 *emu, else snd_emu10k1_ptr_write(emu, DBG, 0, emu->fx8010.dbg | EMU10K1_DBG_SINGLE_STEP); /* ok, do the main job */ - if ((err = snd_emu10k1_del_controls(emu, icode)) < 0 || - (err = snd_emu10k1_gpr_poke(emu, icode)) < 0 || - (err = snd_emu10k1_tram_poke(emu, icode)) < 0 || - (err = snd_emu10k1_code_poke(emu, icode)) < 0 || - (err = snd_emu10k1_add_controls(emu, icode)) < 0) + err = snd_emu10k1_del_controls(emu, icode, in_kernel); + if (err < 0) + goto __error; + err = snd_emu10k1_gpr_poke(emu, icode, in_kernel); + if (err < 0) + goto __error; + err = snd_emu10k1_tram_poke(emu, icode, in_kernel); + if (err < 0) + goto __error; + err = snd_emu10k1_code_poke(emu, icode, in_kernel); + if (err < 0) + goto __error; + err = snd_emu10k1_add_controls(emu, icode, in_kernel); + if (err < 0) goto __error; /* start FX processor when the DSP code is updated */ if (emu->audigy) @@ -1179,7 +1204,6 @@ static int _snd_emu10k1_audigy_init_efx(struct snd_emu10k1 *emu) struct snd_emu10k1_fx8010_code *icode = NULL; struct snd_emu10k1_fx8010_control_gpr *controls = NULL, *ctl; u32 *gpr_map; - mm_segment_t seg;
err = -ENOMEM; icode = kzalloc(sizeof(*icode), GFP_KERNEL); @@ -1739,13 +1763,11 @@ A_OP(icode, &ptr, iMAC0, A_GPR(var), A_GPR(var), A_GPR(vol), A_EXTIN(input)) while (ptr < 0x400) A_OP(icode, &ptr, 0x0f, 0xc0, 0xc0, 0xcf, 0xc0);
- seg = snd_enter_user(); icode->gpr_add_control_count = nctl; icode->gpr_add_controls = (struct snd_emu10k1_fx8010_control_gpr __user *)controls; emu->support_tlv = 1; /* support TLV */ - err = snd_emu10k1_icode_poke(emu, icode); + err = snd_emu10k1_icode_poke(emu, icode, true); emu->support_tlv = 0; /* clear again */ - snd_leave_user(seg);
__err: kfree(controls); @@ -1817,7 +1839,6 @@ static int _snd_emu10k1_init_efx(struct snd_emu10k1 *emu) struct snd_emu10k1_fx8010_pcm_rec *ipcm = NULL; struct snd_emu10k1_fx8010_control_gpr *controls = NULL, *ctl; u32 *gpr_map; - mm_segment_t seg;
err = -ENOMEM; icode = kzalloc(sizeof(*icode), GFP_KERNEL); @@ -2368,13 +2389,11 @@ static int _snd_emu10k1_init_efx(struct snd_emu10k1 *emu)
if ((err = snd_emu10k1_fx8010_tram_setup(emu, ipcm->buffer_size)) < 0) goto __err; - seg = snd_enter_user(); icode->gpr_add_control_count = i; icode->gpr_add_controls = (struct snd_emu10k1_fx8010_control_gpr __user *)controls; emu->support_tlv = 1; /* support TLV */ - err = snd_emu10k1_icode_poke(emu, icode); + err = snd_emu10k1_icode_poke(emu, icode, true); emu->support_tlv = 0; /* clear again */ - snd_leave_user(seg); if (err >= 0) err = snd_emu10k1_ipcm_poke(emu, ipcm); __err: @@ -2537,7 +2556,7 @@ static int snd_emu10k1_fx8010_ioctl(struct snd_hwdep * hw, struct file *file, un icode = memdup_user(argp, sizeof(*icode)); if (IS_ERR(icode)) return PTR_ERR(icode); - res = snd_emu10k1_icode_poke(emu, icode); + res = snd_emu10k1_icode_poke(emu, icode, false); kfree(icode); return res; case SNDRV_EMU10K1_IOCTL_CODE_PEEK:
PCM core code has a few usages of set_fs(), mostly for two purposes: - The DELAY ioctl call from pcm_compat.c - The ioctl wrapper in kernel context for PCM OSS and other
This patch removes the set_fs() usage in these places by a slight code refactoring. For the former point, snd_pcm_delay() is changed to return the value directly instead of putting the value to the given address. The each caller stores the result in an appropriate manner.
For fixing the latter, snd_pcm_lib_kernel_ioctl() is changed to call the functions directly as well. Now it accepts now only the limited set of ioctls: primarily it's used by PCM OSS layer, and the only other user is USB UAC1 gadget driver, where both drivers don't need the full set of ioctls.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/core/pcm_compat.c | 12 ++---- sound/core/pcm_native.c | 102 ++++++++++++++++++++++++++++++------------------ 2 files changed, 67 insertions(+), 47 deletions(-)
diff --git a/sound/core/pcm_compat.c b/sound/core/pcm_compat.c index 1f64ab0c2a95..8a0f8d51e95d 100644 --- a/sound/core/pcm_compat.c +++ b/sound/core/pcm_compat.c @@ -27,17 +27,13 @@ static int snd_pcm_ioctl_delay_compat(struct snd_pcm_substream *substream, s32 __user *src) { snd_pcm_sframes_t delay; - mm_segment_t fs; - int err;
- fs = snd_enter_user(); - err = snd_pcm_delay(substream, &delay); - snd_leave_user(fs); - if (err < 0) - return err; + delay = snd_pcm_delay(substream); + if (delay < 0) + return delay; if (put_user(delay, src)) return -EFAULT; - return err; + return 0; }
static int snd_pcm_ioctl_rewind_compat(struct snd_pcm_substream *substream, diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index 13dec5ec93f2..f9bd56958cad 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -181,20 +181,6 @@ void snd_pcm_stream_unlock_irqrestore(struct snd_pcm_substream *substream, } EXPORT_SYMBOL_GPL(snd_pcm_stream_unlock_irqrestore);
-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); -} - - - int snd_pcm_info(struct snd_pcm_substream *substream, struct snd_pcm_info *info) { struct snd_pcm_runtime *runtime; @@ -1081,6 +1067,7 @@ static const struct action_ops snd_pcm_action_start = { * @substream: the PCM substream instance * * Return: Zero if successful, or a negative error code. + * The stream lock must be acquired before calling this function. */ int snd_pcm_start(struct snd_pcm_substream *substream) { @@ -1088,6 +1075,13 @@ int snd_pcm_start(struct snd_pcm_substream *substream) SNDRV_PCM_STATE_RUNNING); }
+/* take the stream lock and start the streams */ +static int snd_pcm_start_lock_irq(struct snd_pcm_substream *substream) +{ + return snd_pcm_action_lock_irq(&snd_pcm_action_start, substream, + SNDRV_PCM_STATE_RUNNING); +} + /* * stop callbacks */ @@ -2655,8 +2649,7 @@ static int snd_pcm_hwsync(struct snd_pcm_substream *substream) return err; } -static int snd_pcm_delay(struct snd_pcm_substream *substream, - snd_pcm_sframes_t __user *res) +static snd_pcm_sframes_t snd_pcm_delay(struct snd_pcm_substream *substream) { struct snd_pcm_runtime *runtime = substream->runtime; int err; @@ -2690,10 +2683,7 @@ static int snd_pcm_delay(struct snd_pcm_substream *substream, break; } snd_pcm_stream_unlock_irq(substream); - if (!err) - if (put_user(n, res)) - err = -EFAULT; - return err; + return err < 0 ? err : n; } static int snd_pcm_sync_ptr(struct snd_pcm_substream *substream, @@ -2781,7 +2771,7 @@ static int snd_pcm_common_ioctl1(struct file *file, case SNDRV_PCM_IOCTL_RESET: return snd_pcm_reset(substream); case SNDRV_PCM_IOCTL_START: - return snd_pcm_action_lock_irq(&snd_pcm_action_start, substream, SNDRV_PCM_STATE_RUNNING); + return snd_pcm_start_lock_irq(substream); case SNDRV_PCM_IOCTL_LINK: return snd_pcm_link(substream, (int)(unsigned long) arg); case SNDRV_PCM_IOCTL_UNLINK: @@ -2793,7 +2783,16 @@ static int snd_pcm_common_ioctl1(struct file *file, case SNDRV_PCM_IOCTL_HWSYNC: return snd_pcm_hwsync(substream); case SNDRV_PCM_IOCTL_DELAY: - return snd_pcm_delay(substream, arg); + { + snd_pcm_sframes_t delay = snd_pcm_delay(substream); + snd_pcm_sframes_t __user *res = arg; + + if (delay < 0) + return delay; + if (put_user(delay, res)) + return -EFAULT; + return 0; + } case SNDRV_PCM_IOCTL_SYNC_PTR: return snd_pcm_sync_ptr(substream, arg); #ifdef CONFIG_SND_SUPPORT_OLD_API @@ -3007,30 +3006,55 @@ static long snd_pcm_capture_ioctl(struct file *file, unsigned int cmd, (void __user *)arg); }
+/** + * snd_pcm_kernel_ioctl - Execute PCM ioctl in the kernel-space + * @substream: PCM substream + * @cmd: IOCTL cmd + * @arg: IOCTL argument + * + * The function is provided primarily for OSS layer and USB gadget drivers, + * and it allows only the limited set of ioctls (hw_params, sw_params, + * prepare, start, drain, drop, forward). + */ int snd_pcm_kernel_ioctl(struct snd_pcm_substream *substream, unsigned int cmd, void *arg) { - mm_segment_t fs; - int result; + snd_pcm_uframes_t *frames = arg; + snd_pcm_sframes_t result; - fs = snd_enter_user(); - switch (substream->stream) { - case SNDRV_PCM_STREAM_PLAYBACK: - result = snd_pcm_playback_ioctl1(NULL, substream, cmd, - (void __user *)arg); - break; - case SNDRV_PCM_STREAM_CAPTURE: - result = snd_pcm_capture_ioctl1(NULL, substream, cmd, - (void __user *)arg); - break; + switch (cmd) { + case SNDRV_PCM_IOCTL_FORWARD: + { + /* provided only for OSS; capture-only and no value returned */ + if (substream->stream != SNDRV_PCM_STREAM_CAPTURE) + return -EINVAL; + result = snd_pcm_capture_forward(substream, *frames); + return result < 0 ? result : 0; + } + case SNDRV_PCM_IOCTL_HW_PARAMS: + return snd_pcm_hw_params(substream, arg); + case SNDRV_PCM_IOCTL_SW_PARAMS: + return snd_pcm_sw_params(substream, arg); + case SNDRV_PCM_IOCTL_PREPARE: + return snd_pcm_prepare(substream, NULL); + case SNDRV_PCM_IOCTL_START: + return snd_pcm_start_lock_irq(substream); + case SNDRV_PCM_IOCTL_DRAIN: + return snd_pcm_drain(substream, NULL); + case SNDRV_PCM_IOCTL_DROP: + return snd_pcm_drop(substream); + case SNDRV_PCM_IOCTL_DELAY: + { + result = snd_pcm_delay(substream); + if (result < 0) + return result; + *frames = result; + return 0; + } default: - result = -EINVAL; - break; + return -EINVAL; } - snd_leave_user(fs); - return result; } - EXPORT_SYMBOL(snd_pcm_kernel_ioctl);
static ssize_t snd_pcm_read(struct file *file, char __user *buf, size_t count,
For supporting the explicit in-kernel copy, and also for reducing the redundant codes for both copy and silence callbacks, a new ops, copy_silence is introduced in this patch. This is supposed to serve for both copy and silence. The silence operation is distinguished by NULL buffer passed (required only in playback direction).
Also, the callback receives a new flag, in_kernel, which indicates that the callback gets called for copying the data from/to the kernel buffer instead of the user-space buffer. The in_kernel flag will be used mainly in PCM OSS code for the on-the-fly conversion. Currently, only in_kernel=false is passed.
Signed-off-by: Takashi Iwai tiwai@suse.de --- include/sound/pcm.h | 3 +++ sound/core/pcm_lib.c | 74 +++++++++++++++++++++++++++++++++++++++++----------- sound/soc/soc-pcm.c | 1 + 3 files changed, 63 insertions(+), 15 deletions(-)
diff --git a/include/sound/pcm.h b/include/sound/pcm.h index 361749e60799..065ea81904b0 100644 --- a/include/sound/pcm.h +++ b/include/sound/pcm.h @@ -83,6 +83,9 @@ 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 (*copy_silence)(struct snd_pcm_substream *substream, int channel, + snd_pcm_uframes_t pos, void __user *buf, + snd_pcm_uframes_t count, bool in_kernel); 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 5088d4b8db22..ed2002bfea6a 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,35 @@ 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->copy_silence) { + err = substream->ops->copy_silence(substream, + -1, ofs, NULL, transfer, false); + 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->copy_silence) { + for (c = 0; c < channels; ++c) { + err = substream->ops->copy_silence(substream, + c, ofs, NULL, transfer, false); + 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); } } @@ -1993,7 +2003,12 @@ 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_silence) { + err = substream->ops->copy_silence(substream, -1, hwoff, buf, + frames, false); + if (err < 0) + return err; + } else if (substream->ops->copy) { if ((err = substream->ops->copy(substream, -1, hwoff, buf, frames)) < 0) return err; } else { @@ -2117,7 +2132,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_silence && !substream->ops->copy + && !runtime->dma_area)) return -EINVAL; if (runtime->status->state == SNDRV_PCM_STATE_OPEN) return -EBADFD; @@ -2154,8 +2170,21 @@ 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_silence) { + for (c = 0; c < channels; ++c, ++bufs) { + if (!*bufs) + buf = NULL; + else + buf = *bufs + samples_to_bytes(runtime, off); + err = substream->ops->copy_silence(substream, c, hwoff, + buf, frames, false); + 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) { @@ -2163,7 +2192,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; } @@ -2215,7 +2244,12 @@ 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_silence) { + err = substream->ops->copy_silence(substream, -1, hwoff, buf, + frames, false); + if (err < 0) + return err; + } else if (substream->ops->copy) { if ((err = substream->ops->copy(substream, -1, hwoff, buf, frames)) < 0) return err; } else { @@ -2363,10 +2397,22 @@ 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_silence) { + for (c = 0; c < channels; ++c, ++bufs) { + if (!*bufs) + continue; + buf = *bufs + samples_to_bytes(runtime, off); + err = substream->ops->copy_silence(substream, c, hwoff, + buf, frames, false); + 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); @@ -2376,8 +2422,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..3cfb9aa1203b 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -2743,6 +2743,7 @@ 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_silence = platform->driver->ops->copy_silence; rtd->ops.copy = platform->driver->ops->copy; rtd->ops.silence = platform->driver->ops->silence; rtd->ops.page = platform->driver->ops->page;
Signed-off-by: Takashi Iwai tiwai@suse.de --- .../sound/kernel-api/writing-an-alsa-driver.rst | 110 ++++++++++++--------- 1 file changed, 63 insertions(+), 47 deletions(-)
diff --git a/Documentation/sound/kernel-api/writing-an-alsa-driver.rst b/Documentation/sound/kernel-api/writing-an-alsa-driver.rst index 95c5443eff38..ebaf8b1e0079 100644 --- a/Documentation/sound/kernel-api/writing-an-alsa-driver.rst +++ b/Documentation/sound/kernel-api/writing-an-alsa-driver.rst @@ -2080,18 +2080,18 @@ sleeping poll threads, etc.
This callback is also atomic as default.
-copy and silence callbacks -~~~~~~~~~~~~~~~~~~~~~~~~~~ +copy_silence callback +~~~~~~~~~~~~~~~~~~~~~
-These callbacks are not mandatory, and can be omitted in most cases. -These callbacks are used when the hardware buffer cannot be in the +This callback is not mandatory, and can be omitted in most cases. +This callback is used when the hardware buffer cannot be in the normal memory space. Some chips have their own buffer on the hardware which is not mappable. In such a case, you have to transfer the data manually from the memory buffer to the hardware buffer. Or, if the buffer is non-contiguous on both physical and virtual memory spaces, these callbacks must be defined, too.
-If these two callbacks are defined, copy and set-silence operations +If this callback is defined, copy and set-silence operations are done by them. The detailed will be described in the later section `Buffer and Memory Management`_.
@@ -3545,30 +3545,34 @@ Another case is when the chip uses a PCI memory-map region for the buffer instead of the host memory. In this case, mmap is available only on certain architectures like the Intel one. In non-mmap mode, the data cannot be transferred as in the normal way. Thus you need to define the -``copy`` and ``silence`` callbacks as well, as in the cases above. The +``copy_silence`` callback as well, as in the cases above. The examples are found in ``rme32.c`` and ``rme96.c``.
-The implementation of the ``copy`` and ``silence`` callbacks depends +The implementation of the ``copy_silence`` callback depends upon whether the hardware supports interleaved or non-interleaved -samples. The ``copy`` callback is defined like below, a bit +samples. The ``copy_silence`` callback is defined like below, a bit differently depending whether the direction is playback or capture:
::
static int playback_copy(struct snd_pcm_substream *substream, int channel, - snd_pcm_uframes_t pos, void *src, snd_pcm_uframes_t count); + snd_pcm_uframes_t pos, void __user *src, + snd_pcm_uframes_t count, bool in_kernel); static int capture_copy(struct snd_pcm_substream *substream, int channel, - snd_pcm_uframes_t pos, void *dst, snd_pcm_uframes_t count); + snd_pcm_uframes_t pos, void __user *dst, + snd_pcm_uframes_t count, bool in_kernel);
In the case of interleaved samples, the second argument (``channel``) is -not used. The third argument (``pos``) points the current position -offset in frames. +not used, and -1 is passed. The third argument (``pos``) points the +current position offset in frames.
The meaning of the fourth argument is different between playback and capture. For playback, it holds the source data pointer, and for capture, it's the destination data pointer.
-The last argument is the number of frames to be copied. +The fifth argument is the number of frames to be copied. +And the last argument indicates whether the passed buffer pointer is in +user-space or in kernel-space. The copy operation depends on this.
What you have to do in this callback is again different between playback and capture directions. In the playback case, you copy the given amount @@ -3578,52 +3582,64 @@ way, the copy would be like:
::
- my_memcpy(my_buffer + frames_to_bytes(runtime, pos), src, - frames_to_bytes(runtime, count)); - -For the capture direction, you copy the given amount of data (``count``) -at the specified offset (``pos``) on the hardware buffer to the -specified pointer (``dst``). - -:: + if (!src) + my_memset(my_buffer + frames_to_bytes(runtime, pos), 0, + frames_to_bytes(runtime, count)); + else if (in_kernel) + memcpy_toio(my_buffer + frames_to_bytes(runtime, pos), + (void *)src, frames_to_bytes(runtime, count)); + else if (copy_from_user_toio(my_buffer + frames_to_bytes(runtime, pos), + src, frames_to_bytes(runtime, count))) + return -EFAULT; + return 0;
- my_memcpy(dst, my_buffer + frames_to_bytes(runtime, pos), - frames_to_bytes(runtime, count)); +Here we prepared three different memory operations operations.
-Note that both the position and the amount of data are given in frames. +The first one, with the NULL ``src`` pointer, is for silencing the +buffer. In this case, we clear the samples for the given position and +portion.
-In the case of non-interleaved samples, the implementation will be a bit -more complicated. +The second one, with ``in_kernel`` check, is for the in-kernel memory +copying. In this case, the given buffer pointer (``src``) is a kernel +pointer despite of being declared with ``__user`` prefix. When this +flag is set, you have to copy the memory from the kernel space. +Typically, a simple :c:func:`memcpy()` or :c:func`memcpy_toio()` can +be used. Note the explicit cast at the function call there to drop +``__user`` prefix.
-You need to check the channel argument, and if it's -1, copy the whole -channels. Otherwise, you have to copy only the specified channel. Please -check ``isa/gus/gus_pcm.c`` as an example. +The last one is the usual operation, to copy from the user-space +buffer to the hardware buffer.
-The ``silence`` callback is also implemented in a similar way +For the capture direction, you copy the given amount of data (``count``) +at the specified offset (``pos``) on the hardware buffer to the +specified pointer (``dst``).
::
- static int silence(struct snd_pcm_substream *substream, int channel, - snd_pcm_uframes_t pos, snd_pcm_uframes_t count); - -The meanings of arguments are the same as in the ``copy`` callback, -although there is no ``src/dst`` argument. In the case of interleaved -samples, the channel argument has no meaning, as well as on ``copy`` -callback. + if (in_kernel) + memcpy_fromio((void *)dst, + my_buffer + frames_to_bytes(runtime, pos), + frames_to_bytes(runtime, count)); + else if (copy_to_user_fromio(dst, + my_buffer + frames_to_bytes(runtime, pos), + frames_to_bytes(runtime, count))) + return -EFAULT; + return 0;
-The role of ``silence`` callback is to set the given amount -(``count``) of silence data at the specified offset (``pos``) on the -hardware buffer. Suppose that the data format is signed (that is, the -silent-data is 0), and the implementation using a memset-like function -would be like: +A clear difference from the playback is that there is no silencing +mode. For the capture direction, ``dst`` is always non-NULL.
-:: +Other than that, the two memory operations are similar, but just in +different direction. And, note that both the position and the amount +of data are given in frames.
- my_memcpy(my_buffer + frames_to_bytes(runtime, pos), 0, - frames_to_bytes(runtime, count)); +In the case of non-interleaved samples, the implementation will be a bit +more complicated. First off, the operation depends on ``channel`` +argument. When -1 is passed there, copy the whole channels. +Otherwise, copy only the specified channel.
-In the case of non-interleaved samples, again, the implementation -becomes a bit more complicated. See, for example, ``isa/gus/gus_pcm.c``. +As an implementation example, please take a look at the code in +``sound/isa/gus/gus_pcm.c``.
Non-Contiguous Buffers ----------------------
It's a dummy ops, so just replacing it.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/drivers/dummy.c | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-)
diff --git a/sound/drivers/dummy.c b/sound/drivers/dummy.c index 172dacd925f5..68519689a9ea 100644 --- a/sound/drivers/dummy.c +++ b/sound/drivers/dummy.c @@ -645,14 +645,8 @@ static int alloc_fake_buffer(void)
static int dummy_pcm_copy(struct snd_pcm_substream *substream, int channel, snd_pcm_uframes_t pos, - void __user *dst, snd_pcm_uframes_t count) -{ - return 0; /* do nothing */ -} - -static int dummy_pcm_silence(struct snd_pcm_substream *substream, - int channel, snd_pcm_uframes_t pos, - snd_pcm_uframes_t count) + void __user *dst, snd_pcm_uframes_t count, + bool in_kernel) { return 0; /* do nothing */ } @@ -683,8 +677,7 @@ static struct snd_pcm_ops dummy_pcm_ops_no_buf = { .prepare = dummy_pcm_prepare, .trigger = dummy_pcm_trigger, .pointer = dummy_pcm_pointer, - .copy = dummy_pcm_copy, - .silence = dummy_pcm_silence, + .copy_silence = dummy_pcm_copy, .page = dummy_pcm_page, };
Replace the copy and the silence ops with the new merged ops. It's used only for a capture stream (for some hardware workaround), thus we need no silence operation but only to add the in_kernel memcpy() handling.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/pci/es1938.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/sound/pci/es1938.c b/sound/pci/es1938.c index e8d943071a8c..d79ac13d6f70 100644 --- a/sound/pci/es1938.c +++ b/sound/pci/es1938.c @@ -842,7 +842,8 @@ static int snd_es1938_capture_copy(struct snd_pcm_substream *substream, int channel, snd_pcm_uframes_t pos, void __user *dst, - snd_pcm_uframes_t count) + snd_pcm_uframes_t count, + bool in_kernel) { struct snd_pcm_runtime *runtime = substream->runtime; struct es1938 *chip = snd_pcm_substream_chip(substream); @@ -850,8 +851,10 @@ static int snd_es1938_capture_copy(struct snd_pcm_substream *substream, count <<= chip->dma1_shift; if (snd_BUG_ON(pos + count > chip->dma1_size)) return -EINVAL; - if (pos + count < chip->dma1_size) { - if (copy_to_user(dst, runtime->dma_area + pos + 1, count)) + if (in_kernel || pos + count < chip->dma1_size) { + if (in_kernel) + memcpy((void *)dst, runtime->dma_area + pos + 1, count); + else if (copy_to_user(dst, runtime->dma_area + pos + 1, count)) return -EFAULT; } else { if (copy_to_user(dst, runtime->dma_area + pos + 1, count - 1)) @@ -1012,7 +1015,7 @@ static const struct snd_pcm_ops snd_es1938_capture_ops = { .prepare = snd_es1938_capture_prepare, .trigger = snd_es1938_capture_trigger, .pointer = snd_es1938_capture_pointer, - .copy = snd_es1938_capture_copy, + .copy_silence = snd_es1938_capture_copy, };
static int snd_es1938_new_pcm(struct es1938 *chip, int device)
Replace the copy and the silence ops with the new merged ops. The redundant function calls are reduced and the copy/silence are handled directly in callback functions now.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/pci/korg1212/korg1212.c | 128 ++++++++++++------------------------------ 1 file changed, 36 insertions(+), 92 deletions(-)
diff --git a/sound/pci/korg1212/korg1212.c b/sound/pci/korg1212/korg1212.c index 1e25095fd144..865ff553dc87 100644 --- a/sound/pci/korg1212/korg1212.c +++ b/sound/pci/korg1212/korg1212.c @@ -1273,43 +1273,24 @@ static struct snd_pcm_hardware snd_korg1212_capture_info = .fifo_size = 0, };
-static int snd_korg1212_silence(struct snd_korg1212 *korg1212, int pos, int count, int offset, int size) -{ - struct KorgAudioFrame * dst = korg1212->playDataBufsPtr[0].bufferData + pos; - int i; - - K1212_DEBUG_PRINTK_VERBOSE("K1212_DEBUG: snd_korg1212_silence pos=%d offset=%d size=%d count=%d\n", - pos, offset, size, count); - if (snd_BUG_ON(pos + count > K1212_MAX_SAMPLES)) - return -EINVAL; - - for (i=0; i < count; i++) { -#if K1212_DEBUG_LEVEL > 0 - if ( (void *) dst < (void *) korg1212->playDataBufsPtr || - (void *) dst > (void *) korg1212->playDataBufsPtr[8].bufferData ) { - printk(KERN_DEBUG "K1212_DEBUG: snd_korg1212_silence KERNEL EFAULT dst=%p iter=%d\n", - dst, i); - return -EFAULT; - } -#endif - memset((void*) dst + offset, 0, size); - dst++; - } - - return 0; -} - -static int snd_korg1212_copy_to(struct snd_korg1212 *korg1212, void __user *dst, int pos, int count, int offset, int size) +static int snd_korg1212_capture_copy(struct snd_pcm_substream *substream, + int channel, + snd_pcm_uframes_t pos, + void __user *dst, + snd_pcm_uframes_t count, + bool in_kernel) { + struct snd_korg1212 *korg1212 = snd_pcm_substream_chip(substream); struct KorgAudioFrame * src = korg1212->recordDataBufsPtr[0].bufferData + pos; - int i, rc; + int size, i;
- K1212_DEBUG_PRINTK_VERBOSE("K1212_DEBUG: snd_korg1212_copy_to pos=%d offset=%d size=%d\n", - pos, offset, size); + size = korg1212->channels * 2; + K1212_DEBUG_PRINTK_VERBOSE("K1212_DEBUG: snd_korg1212_copy_to pos=%ld size=%d\n", + pos, size); if (snd_BUG_ON(pos + count > K1212_MAX_SAMPLES)) return -EINVAL;
- for (i=0; i < count; i++) { + for (i = 0; i < count; i++) { #if K1212_DEBUG_LEVEL > 0 if ( (void *) src < (void *) korg1212->recordDataBufsPtr || (void *) src > (void *) korg1212->recordDataBufsPtr[8].bufferData ) { @@ -1317,11 +1298,10 @@ static int snd_korg1212_copy_to(struct snd_korg1212 *korg1212, void __user *dst, return -EFAULT; } #endif - rc = copy_to_user(dst + offset, src, size); - if (rc) { - K1212_DEBUG_PRINTK("K1212_DEBUG: snd_korg1212_copy_to USER EFAULT src=%p dst=%p iter=%d\n", src, dst, i); + if (in_kernel) + memcpy((char *)dst, src, size); + else if (copy_to_user(dst, src, size)) return -EFAULT; - } src++; dst += size; } @@ -1329,18 +1309,25 @@ static int snd_korg1212_copy_to(struct snd_korg1212 *korg1212, void __user *dst, return 0; }
-static int snd_korg1212_copy_from(struct snd_korg1212 *korg1212, void __user *src, int pos, int count, int offset, int size) +static int snd_korg1212_playback_copy(struct snd_pcm_substream *substream, + int channel, + snd_pcm_uframes_t pos, + void __user *src, + snd_pcm_uframes_t count, + bool in_kernel) { + struct snd_korg1212 *korg1212 = snd_pcm_substream_chip(substream); struct KorgAudioFrame * dst = korg1212->playDataBufsPtr[0].bufferData + pos; - int i, rc; + int size, i;
- K1212_DEBUG_PRINTK_VERBOSE("K1212_DEBUG: snd_korg1212_copy_from pos=%d offset=%d size=%d count=%d\n", - pos, offset, size, count); + size = korg1212->channels * 2; + K1212_DEBUG_PRINTK_VERBOSE("K1212_DEBUG: snd_korg1212_copy_from pos=%ld size=%d count=%ld\n", + pos, size, count);
if (snd_BUG_ON(pos + count > K1212_MAX_SAMPLES)) return -EINVAL;
- for (i=0; i < count; i++) { + for (i = 0; i < count; i++) { #if K1212_DEBUG_LEVEL > 0 if ( (void *) dst < (void *) korg1212->playDataBufsPtr || (void *) dst > (void *) korg1212->playDataBufsPtr[8].bufferData ) { @@ -1348,13 +1335,15 @@ static int snd_korg1212_copy_from(struct snd_korg1212 *korg1212, void __user *sr return -EFAULT; } #endif - rc = copy_from_user((void*) dst + offset, src, size); - if (rc) { - K1212_DEBUG_PRINTK("K1212_DEBUG: snd_korg1212_copy_from USER EFAULT src=%p dst=%p iter=%d\n", src, dst, i); + if (!src) + memset((void *)dst, 0, size); + else if (in_kernel) + memcpy((void *)dst, src, size); + else if (copy_from_user((void *)dst, src, size)) return -EFAULT; - } dst++; - src += size; + if (src) + src += size; }
return 0; @@ -1437,8 +1426,6 @@ static int snd_korg1212_playback_close(struct snd_pcm_substream *substream) K1212_DEBUG_PRINTK("K1212_DEBUG: snd_korg1212_playback_close [%s]\n", stateName[korg1212->cardState]);
- snd_korg1212_silence(korg1212, 0, K1212_MAX_SAMPLES, 0, korg1212->channels * 2); - spin_lock_irqsave(&korg1212->lock, flags);
korg1212->playback_pid = -1; @@ -1639,48 +1626,6 @@ static snd_pcm_uframes_t snd_korg1212_capture_pointer(struct snd_pcm_substream * return pos; }
-static int snd_korg1212_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) -{ - struct snd_korg1212 *korg1212 = snd_pcm_substream_chip(substream); - - K1212_DEBUG_PRINTK_VERBOSE("K1212_DEBUG: snd_korg1212_playback_copy [%s] %ld %ld\n", - stateName[korg1212->cardState], pos, count); - - return snd_korg1212_copy_from(korg1212, src, pos, count, 0, korg1212->channels * 2); - -} - -static int snd_korg1212_playback_silence(struct snd_pcm_substream *substream, - int channel, /* not used (interleaved data) */ - snd_pcm_uframes_t pos, - snd_pcm_uframes_t count) -{ - struct snd_korg1212 *korg1212 = snd_pcm_substream_chip(substream); - - K1212_DEBUG_PRINTK_VERBOSE("K1212_DEBUG: snd_korg1212_playback_silence [%s]\n", - stateName[korg1212->cardState]); - - return snd_korg1212_silence(korg1212, pos, count, 0, korg1212->channels * 2); -} - -static int snd_korg1212_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) -{ - struct snd_korg1212 *korg1212 = snd_pcm_substream_chip(substream); - - K1212_DEBUG_PRINTK_VERBOSE("K1212_DEBUG: snd_korg1212_capture_copy [%s] %ld %ld\n", - stateName[korg1212->cardState], pos, count); - - return snd_korg1212_copy_to(korg1212, dst, pos, count, 0, korg1212->channels * 2); -} - static const struct snd_pcm_ops snd_korg1212_playback_ops = { .open = snd_korg1212_playback_open, .close = snd_korg1212_playback_close, @@ -1689,8 +1634,7 @@ static const struct snd_pcm_ops snd_korg1212_playback_ops = { .prepare = snd_korg1212_prepare, .trigger = snd_korg1212_trigger, .pointer = snd_korg1212_playback_pointer, - .copy = snd_korg1212_playback_copy, - .silence = snd_korg1212_playback_silence, + .copy_silence = snd_korg1212_playback_copy, };
static const struct snd_pcm_ops snd_korg1212_capture_ops = { @@ -1701,7 +1645,7 @@ static const struct snd_pcm_ops snd_korg1212_capture_ops = { .prepare = snd_korg1212_prepare, .trigger = snd_korg1212_trigger, .pointer = snd_korg1212_capture_pointer, - .copy = snd_korg1212_capture_copy, + .copy_silence = snd_korg1212_capture_copy, };
/*
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 | 35 ++++++++++++++--------------------- 1 file changed, 14 insertions(+), 21 deletions(-)
diff --git a/sound/pci/nm256/nm256.c b/sound/pci/nm256/nm256.c index 103fe311e5a9..d8e765f7e758 100644 --- a/sound/pci/nm256/nm256.c +++ b/sound/pci/nm256/nm256.c @@ -694,31 +694,22 @@ snd_nm256_capture_pointer(struct snd_pcm_substream *substream) * silence / copy for playback */ 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) -{ - 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) + snd_pcm_uframes_t count, + bool in_kernel) { 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)) + if (!src) + memset_io(s->bufptr + pos, 0, count); + else if (in_kernel) + memcpy_toio(s->bufptr + pos, (void *)src, count); + else if (copy_from_user_toio(s->bufptr + pos, src, count)) return -EFAULT; return 0; } @@ -731,13 +722,16 @@ 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) + snd_pcm_uframes_t count, + bool in_kernel) { 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)) + if (in_kernel) + memcpy_fromio((void *)dst, s->bufptr + pos, count); + else if (copy_to_user_fromio(dst, s->bufptr + pos, count)) return -EFAULT; return 0; } @@ -911,8 +905,7 @@ 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_silence = snd_nm256_playback_copy, #endif .mmap = snd_pcm_lib_mmap_iomem, }; @@ -926,7 +919,7 @@ 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_silence = snd_nm256_capture_copy, #endif .mmap = snd_pcm_lib_mmap_iomem, };
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/rme32.c | 49 ++++++++++++++++++++++++------------------------- 1 file changed, 24 insertions(+), 25 deletions(-)
diff --git a/sound/pci/rme32.c b/sound/pci/rme32.c index 96d15db65dfd..d2b4a3ef0bd3 100644 --- a/sound/pci/rme32.c +++ b/sound/pci/rme32.c @@ -253,41 +253,42 @@ static inline unsigned int snd_rme32_pcm_byteptr(struct rme32 * rme32) & RME32_RCR_AUDIO_ADDR_MASK); }
-/* silence callback for halfduplex mode */ -static int snd_rme32_playback_silence(struct snd_pcm_substream *substream, int channel, /* not used (interleaved data) */ - snd_pcm_uframes_t pos, - snd_pcm_uframes_t count) -{ - struct rme32 *rme32 = snd_pcm_substream_chip(substream); - count <<= rme32->playback_frlog; - pos <<= rme32->playback_frlog; - memset_io(rme32->iobase + RME32_IO_DATA_BUFFER + pos, 0, count); - return 0; -} - /* copy callback for halfduplex mode */ -static int snd_rme32_playback_copy(struct snd_pcm_substream *substream, int channel, /* not used (interleaved data) */ +static int snd_rme32_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) + void __user *src, snd_pcm_uframes_t count, + bool in_kernel) { struct rme32 *rme32 = snd_pcm_substream_chip(substream); count <<= rme32->playback_frlog; pos <<= rme32->playback_frlog; - if (copy_from_user_toio(rme32->iobase + RME32_IO_DATA_BUFFER + pos, - src, count)) + if (!src) + memset_io(rme32->iobase + RME32_IO_DATA_BUFFER + pos, 0, count); + else if (in_kernel) + memcpy_toio(rme32->iobase + RME32_IO_DATA_BUFFER + pos, + (void *)src, count); + else if (copy_from_user_toio(rme32->iobase + RME32_IO_DATA_BUFFER + pos, + src, count)) return -EFAULT; return 0; }
/* copy callback for halfduplex mode */ -static int snd_rme32_capture_copy(struct snd_pcm_substream *substream, int channel, /* not used (interleaved data) */ +static int snd_rme32_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) + void __user *dst, snd_pcm_uframes_t count, + bool in_kernel) { struct rme32 *rme32 = snd_pcm_substream_chip(substream); count <<= rme32->capture_frlog; pos <<= rme32->capture_frlog; - if (copy_to_user_fromio(dst, + if (in_kernel) + memcpy_fromio((void *)dst, + rme32->iobase + RME32_IO_DATA_BUFFER + pos, + count); + else if (copy_to_user_fromio(dst, rme32->iobase + RME32_IO_DATA_BUFFER + pos, count)) return -EFAULT; @@ -1205,8 +1206,7 @@ static const struct snd_pcm_ops snd_rme32_playback_spdif_ops = { .prepare = snd_rme32_playback_prepare, .trigger = snd_rme32_pcm_trigger, .pointer = snd_rme32_playback_pointer, - .copy = snd_rme32_playback_copy, - .silence = snd_rme32_playback_silence, + .copy_silence = snd_rme32_playback_copy, .mmap = snd_pcm_lib_mmap_iomem, };
@@ -1219,7 +1219,7 @@ static const struct snd_pcm_ops snd_rme32_capture_spdif_ops = { .prepare = snd_rme32_capture_prepare, .trigger = snd_rme32_pcm_trigger, .pointer = snd_rme32_capture_pointer, - .copy = snd_rme32_capture_copy, + .copy_silence = snd_rme32_capture_copy, .mmap = snd_pcm_lib_mmap_iomem, };
@@ -1231,8 +1231,7 @@ static const struct snd_pcm_ops snd_rme32_playback_adat_ops = { .prepare = snd_rme32_playback_prepare, .trigger = snd_rme32_pcm_trigger, .pointer = snd_rme32_playback_pointer, - .copy = snd_rme32_playback_copy, - .silence = snd_rme32_playback_silence, + .copy_silence = snd_rme32_playback_copy, .mmap = snd_pcm_lib_mmap_iomem, };
@@ -1244,7 +1243,7 @@ static const struct snd_pcm_ops snd_rme32_capture_adat_ops = { .prepare = snd_rme32_capture_prepare, .trigger = snd_rme32_pcm_trigger, .pointer = snd_rme32_capture_pointer, - .copy = snd_rme32_capture_copy, + .copy_silence = snd_rme32_capture_copy, .mmap = snd_pcm_lib_mmap_iomem, };
Hello everyone,
to have another digital audio i/o card for our studio / office, I got a pair of RME32 the other week from ebay. (mostly as reference to implement ADAT for the RAD1 Sgi/Octane ALSA driver, …)
Unfortunately they do not work with Linux. They are recognised and all the usual devices and /proc/… entries show up, however, the hardware pointer does not move during playback or capture no matter what clock source I choose. I tried attaching coax s/pdif as well as an 8-channel Behringer Ultragain ADAT source w/ clock.
The two cards came from the same seller, look ok and both behave the same. I went so far to install a Windows XP test install where both cards work “more”. (They are not perfect in windows, however, at least s/pdif can come out with or with-out external ADAT clock source. However, the digital signal strangely unclean, but that may be a bug in the window system sound device emulation, I only tested with foobar2k and not some Pro audio app which I do not really have).
Long story short, does someone still have such a card, or not moving hardware ptr (or missing interrupts?) does ring a bell regarding this RME generation? Instead of a working ADAT reference card, I apparently have two ALSA driver to hack on, … ;-)
I tested two different “PC” boards and the results were the same, too.
Thanks, René
On 11 May 2017, at 23:09, Takashi Iwai tiwai@suse.de wrote:
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/rme32.c | 49 ++++++++++++++++++++++++------------------------- 1 file changed, 24 insertions(+), 25 deletions(-)
diff --git a/sound/pci/rme32.c b/sound/pci/rme32.c index 96d15db65dfd..d2b4a3ef0bd3 100644 --- a/sound/pci/rme32.c +++ b/sound/pci/rme32.c @@ -253,41 +253,42 @@ static inline unsigned int snd_rme32_pcm_byteptr(struct rme32 * rme32) & RME32_RCR_AUDIO_ADDR_MASK); }
-/* silence callback for halfduplex mode */ -static int snd_rme32_playback_silence(struct snd_pcm_substream *substream, int channel, /* not used (interleaved data) */
snd_pcm_uframes_t pos,snd_pcm_uframes_t count)-{
- struct rme32 *rme32 = snd_pcm_substream_chip(substream);
- count <<= rme32->playback_frlog;
- pos <<= rme32->playback_frlog;
- memset_io(rme32->iobase + RME32_IO_DATA_BUFFER + pos, 0, count);
- return 0;
-}
/* copy callback for halfduplex mode */ -static int snd_rme32_playback_copy(struct snd_pcm_substream *substream, int channel, /* not used (interleaved data) */ +static int snd_rme32_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)
void __user *src, snd_pcm_uframes_t count,bool in_kernel){ struct rme32 *rme32 = snd_pcm_substream_chip(substream); count <<= rme32->playback_frlog; pos <<= rme32->playback_frlog;
- if (copy_from_user_toio(rme32->iobase + RME32_IO_DATA_BUFFER + pos,
src, count))
- if (!src)
memset_io(rme32->iobase + RME32_IO_DATA_BUFFER + pos, 0, count);- else if (in_kernel)
memcpy_toio(rme32->iobase + RME32_IO_DATA_BUFFER + pos,(void *)src, count);- else if (copy_from_user_toio(rme32->iobase + RME32_IO_DATA_BUFFER + pos,
src, count))}
/* copy callback for halfduplex mode */ -static int snd_rme32_capture_copy(struct snd_pcm_substream *substream, int channel, /* not used (interleaved data) */ +static int snd_rme32_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)
void __user *dst, snd_pcm_uframes_t count,bool in_kernel){ struct rme32 *rme32 = snd_pcm_substream_chip(substream); count <<= rme32->capture_frlog; pos <<= rme32->capture_frlog;
- if (copy_to_user_fromio(dst,
- if (in_kernel)
memcpy_fromio((void *)dst,rme32->iobase + RME32_IO_DATA_BUFFER + pos,count);- else if (copy_to_user_fromio(dst, rme32->iobase + RME32_IO_DATA_BUFFER + pos, count)) return -EFAULT;
@@ -1205,8 +1206,7 @@ static const struct snd_pcm_ops snd_rme32_playback_spdif_ops = { .prepare = snd_rme32_playback_prepare, .trigger = snd_rme32_pcm_trigger, .pointer = snd_rme32_playback_pointer,
- .copy = snd_rme32_playback_copy,
- .silence = snd_rme32_playback_silence,
- .copy_silence = snd_rme32_playback_copy, .mmap = snd_pcm_lib_mmap_iomem,
};
@@ -1219,7 +1219,7 @@ static const struct snd_pcm_ops snd_rme32_capture_spdif_ops = { .prepare = snd_rme32_capture_prepare, .trigger = snd_rme32_pcm_trigger, .pointer = snd_rme32_capture_pointer,
- .copy = snd_rme32_capture_copy,
- .copy_silence = snd_rme32_capture_copy, .mmap = snd_pcm_lib_mmap_iomem,
};
@@ -1231,8 +1231,7 @@ static const struct snd_pcm_ops snd_rme32_playback_adat_ops = { .prepare = snd_rme32_playback_prepare, .trigger = snd_rme32_pcm_trigger, .pointer = snd_rme32_playback_pointer,
- .copy = snd_rme32_playback_copy,
- .silence = snd_rme32_playback_silence,
- .copy_silence = snd_rme32_playback_copy, .mmap = snd_pcm_lib_mmap_iomem,
};
@@ -1244,7 +1243,7 @@ static const struct snd_pcm_ops snd_rme32_capture_adat_ops = { .prepare = snd_rme32_capture_prepare, .trigger = snd_rme32_pcm_trigger, .pointer = snd_rme32_capture_pointer,
- .copy = snd_rme32_capture_copy,
- .copy_silence = snd_rme32_capture_copy, .mmap = snd_pcm_lib_mmap_iomem,
};
-- 2.12.2
Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
On Wed, 18 Jul 2018 12:22:11 +0200, René Rebe wrote:
Hello everyone,
to have another digital audio i/o card for our studio / office, I got a pair of RME32 the other week from ebay. (mostly as reference to implement ADAT for the RAD1 Sgi/Octane ALSA driver, …)
Unfortunately they do not work with Linux. They are recognised and all the usual devices and /proc/… entries show up, however, the hardware pointer does not move during playback or capture no matter what clock source I choose. I tried attaching coax s/pdif as well as an 8-channel Behringer Ultragain ADAT source w/ clock.
Does the /proc/asound/card*/rme32 entry show the right setup? RME32 seems to have only few registers, and it behaves differently for read and write. Maybe you should try to watch the register 0x20000. The hwptr is the LSB 33 bits.
Takashi
The two cards came from the same seller, look ok and both behave the same. I went so far to install a Windows XP test install where both cards work “more”. (They are not perfect in windows, however, at least s/pdif can come out with or with-out external ADAT clock source. However, the digital signal strangely unclean, but that may be a bug in the window system sound device emulation, I only tested with foobar2k and not some Pro audio app which I do not really have).
Long story short, does someone still have such a card, or not moving hardware ptr (or missing interrupts?) does ring a bell regarding this RME generation? Instead of a working ADAT reference card, I apparently have two ALSA driver to hack on, … ;-)
I tested two different “PC” boards and the results were the same, too.
Thanks, René
On 11 May 2017, at 23:09, Takashi Iwai tiwai@suse.de wrote:
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/rme32.c | 49 ++++++++++++++++++++++++------------------------- 1 file changed, 24 insertions(+), 25 deletions(-)
diff --git a/sound/pci/rme32.c b/sound/pci/rme32.c index 96d15db65dfd..d2b4a3ef0bd3 100644 --- a/sound/pci/rme32.c +++ b/sound/pci/rme32.c @@ -253,41 +253,42 @@ static inline unsigned int snd_rme32_pcm_byteptr(struct rme32 * rme32) & RME32_RCR_AUDIO_ADDR_MASK); }
-/* silence callback for halfduplex mode */ -static int snd_rme32_playback_silence(struct snd_pcm_substream *substream, int channel, /* not used (interleaved data) */
snd_pcm_uframes_t pos,snd_pcm_uframes_t count)-{
- struct rme32 *rme32 = snd_pcm_substream_chip(substream);
- count <<= rme32->playback_frlog;
- pos <<= rme32->playback_frlog;
- memset_io(rme32->iobase + RME32_IO_DATA_BUFFER + pos, 0, count);
- return 0;
-}
/* copy callback for halfduplex mode */ -static int snd_rme32_playback_copy(struct snd_pcm_substream *substream, int channel, /* not used (interleaved data) */ +static int snd_rme32_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)
void __user *src, snd_pcm_uframes_t count,bool in_kernel){ struct rme32 *rme32 = snd_pcm_substream_chip(substream); count <<= rme32->playback_frlog; pos <<= rme32->playback_frlog;
- if (copy_from_user_toio(rme32->iobase + RME32_IO_DATA_BUFFER + pos,
src, count))
- if (!src)
memset_io(rme32->iobase + RME32_IO_DATA_BUFFER + pos, 0, count);- else if (in_kernel)
memcpy_toio(rme32->iobase + RME32_IO_DATA_BUFFER + pos,(void *)src, count);- else if (copy_from_user_toio(rme32->iobase + RME32_IO_DATA_BUFFER + pos,
src, count))}
/* copy callback for halfduplex mode */ -static int snd_rme32_capture_copy(struct snd_pcm_substream *substream, int channel, /* not used (interleaved data) */ +static int snd_rme32_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)
void __user *dst, snd_pcm_uframes_t count,bool in_kernel){ struct rme32 *rme32 = snd_pcm_substream_chip(substream); count <<= rme32->capture_frlog; pos <<= rme32->capture_frlog;
- if (copy_to_user_fromio(dst,
- if (in_kernel)
memcpy_fromio((void *)dst,rme32->iobase + RME32_IO_DATA_BUFFER + pos,count);- else if (copy_to_user_fromio(dst, rme32->iobase + RME32_IO_DATA_BUFFER + pos, count)) return -EFAULT;
@@ -1205,8 +1206,7 @@ static const struct snd_pcm_ops snd_rme32_playback_spdif_ops = { .prepare = snd_rme32_playback_prepare, .trigger = snd_rme32_pcm_trigger, .pointer = snd_rme32_playback_pointer,
- .copy = snd_rme32_playback_copy,
- .silence = snd_rme32_playback_silence,
- .copy_silence = snd_rme32_playback_copy, .mmap = snd_pcm_lib_mmap_iomem,
};
@@ -1219,7 +1219,7 @@ static const struct snd_pcm_ops snd_rme32_capture_spdif_ops = { .prepare = snd_rme32_capture_prepare, .trigger = snd_rme32_pcm_trigger, .pointer = snd_rme32_capture_pointer,
- .copy = snd_rme32_capture_copy,
- .copy_silence = snd_rme32_capture_copy, .mmap = snd_pcm_lib_mmap_iomem,
};
@@ -1231,8 +1231,7 @@ static const struct snd_pcm_ops snd_rme32_playback_adat_ops = { .prepare = snd_rme32_playback_prepare, .trigger = snd_rme32_pcm_trigger, .pointer = snd_rme32_playback_pointer,
- .copy = snd_rme32_playback_copy,
- .silence = snd_rme32_playback_silence,
- .copy_silence = snd_rme32_playback_copy, .mmap = snd_pcm_lib_mmap_iomem,
};
@@ -1244,7 +1243,7 @@ static const struct snd_pcm_ops snd_rme32_capture_adat_ops = { .prepare = snd_rme32_capture_prepare, .trigger = snd_rme32_pcm_trigger, .pointer = snd_rme32_capture_pointer,
- .copy = snd_rme32_capture_copy,
- .copy_silence = snd_rme32_capture_copy, .mmap = snd_pcm_lib_mmap_iomem,
};
-- 2.12.2
Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
-- ExactCODE GmbH, Lietzenburger Str. 42, DE-10789 Berlin DE Legal: Amtsgericht Berlin (Charlottenburg) HRB 105123B, Tax-ID#: DE251602478 Managing Director: René Rebe http://exactcode.com | http://exactscan.com | http://ocrkit.com | http://t2-project.org | http://rene.rebe.de
Hi,
On 18 Jul 2018, at 12:56, Takashi Iwai tiwai@suse.de wrote:
On Wed, 18 Jul 2018 12:22:11 +0200, René Rebe wrote:
Hello everyone,
to have another digital audio i/o card for our studio / office, I got a pair of RME32 the other week from ebay. (mostly as reference to implement ADAT for the RAD1 Sgi/Octane ALSA driver, …)
Unfortunately they do not work with Linux. They are recognised and all the usual devices and /proc/… entries show up, however, the hardware pointer does not move during playback or capture no matter what clock source I choose. I tried attaching coax s/pdif as well as an 8-channel Behringer Ultragain ADAT source w/ clock.
Does the /proc/asound/card*/rme32 entry show the right setup?
Well, I think it never showed external clock when I set it to external ADAT, right now, freshly booted (without the ext. ADAT) just trying to play stereo on the s/pdif it shows:
RME Digi32/8 (Rev. 101) at 0xec000000, irq 5 (index #1)
General settings Half-duplex mode receiver: CS8412 format: 16 bit, Stereo
Input settings input: optical sample rate: no valid signal
Output settings output signal: normal playback (muted) sample rate: 44100 Hz sample clock source: Internal format: IEC958 (consumer) emphasis: off
Which should be right for internally word clock’ed s/pdif output. However, aplay does not play anything, after some 20? seconds or so of hanging it even prints:
aplay: pcm_write:2011: write error: Input/output error
While it is “trying to playback” /proc shows:
root@hostname:/proc/asound/card0# grep . pcm*p/sub0/* pcm0p/sub0/hw_params:access: RW_INTERLEAVED pcm0p/sub0/hw_params:format: S16_LE pcm0p/sub0/hw_params:subformat: STD pcm0p/sub0/hw_params:channels: 2 pcm0p/sub0/hw_params:rate: 44100 (44100/1) pcm0p/sub0/hw_params:period_size: 2048 pcm0p/sub0/hw_params:buffer_size: 32768 pcm0p/sub0/info:card: 0 pcm0p/sub0/info:device: 0 pcm0p/sub0/info:subdevice: 0 pcm0p/sub0/info:stream: PLAYBACK pcm0p/sub0/info:id: Digi32 IEC958 pcm0p/sub0/info:name: Digi32 IEC958 pcm0p/sub0/info:subname: subdevice #0 pcm0p/sub0/info:class: 0 pcm0p/sub0/info:subclass: 0 pcm0p/sub0/info:subdevices_count: 1 pcm0p/sub0/info:subdevices_avail: 0 pcm0p/sub0/status:state: RUNNING pcm0p/sub0/status:owner_pid : 753 pcm0p/sub0/status:trigger_time: 1195.174967384 pcm0p/sub0/status:tstamp : 0.000000000 pcm0p/sub0/status:delay : 32768 pcm0p/sub0/status:avail : 0 pcm0p/sub0/status:avail_max : 30720 pcm0p/sub0/status:----- pcm0p/sub0/status:hw_ptr : 10243 pcm0p/sub0/status:appl_ptr : 43011 pcm0p/sub0/sw_params:tstamp_mode: NONE pcm0p/sub0/sw_params:period_step: 1 pcm0p/sub0/sw_params:avail_min: 2048 pcm0p/sub0/sw_params:start_threshold: 32768 pcm0p/sub0/sw_params:stop_threshold: 32768 pcm0p/sub0/sw_params:silence_threshold: 0 pcm0p/sub0/sw_params:silence_size: 0 pcm0p/sub0/sw_params:boundary: 1073741824
I tried many kernels, down to 2.6.31, which I had initially on the box. So unless some this hwparams never worked, this driver appears to not work a looong looooong time.
I doubt they are fully defect, as both behave the same and at least output “something” on Windows XP, ...
RME32 seems to have only few registers, and it behaves differently for read and write. Maybe you should try to watch the register 0x20000. The hwptr is the LSB 33 bits.
Yeah, guess I have to instrument this driver to track what is happening, maybe register not flushed to the hw?
Takashi
The two cards came from the same seller, look ok and both behave the same. I went so far to install a Windows XP test install where both cards work “more”. (They are not perfect in windows, however, at least s/pdif can come out with or with-out external ADAT clock source. However, the digital signal strangely unclean, but that may be a bug in the window system sound device emulation, I only tested with foobar2k and not some Pro audio app which I do not really have).
Long story short, does someone still have such a card, or not moving hardware ptr (or missing interrupts?) does ring a bell regarding this RME generation? Instead of a working ADAT reference card, I apparently have two ALSA driver to hack on, … ;-)
I tested two different “PC” boards and the results were the same, too.
Thanks, René
On 11 May 2017, at 23:09, Takashi Iwai tiwai@suse.de wrote:
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/rme32.c | 49 ++++++++++++++++++++++++------------------------- 1 file changed, 24 insertions(+), 25 deletions(-)
diff --git a/sound/pci/rme32.c b/sound/pci/rme32.c index 96d15db65dfd..d2b4a3ef0bd3 100644 --- a/sound/pci/rme32.c +++ b/sound/pci/rme32.c @@ -253,41 +253,42 @@ static inline unsigned int snd_rme32_pcm_byteptr(struct rme32 * rme32) & RME32_RCR_AUDIO_ADDR_MASK); }
-/* silence callback for halfduplex mode */ -static int snd_rme32_playback_silence(struct snd_pcm_substream *substream, int channel, /* not used (interleaved data) */
snd_pcm_uframes_t pos,snd_pcm_uframes_t count)-{
- struct rme32 *rme32 = snd_pcm_substream_chip(substream);
- count <<= rme32->playback_frlog;
- pos <<= rme32->playback_frlog;
- memset_io(rme32->iobase + RME32_IO_DATA_BUFFER + pos, 0, count);
- return 0;
-}
/* copy callback for halfduplex mode */ -static int snd_rme32_playback_copy(struct snd_pcm_substream *substream, int channel, /* not used (interleaved data) */ +static int snd_rme32_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)
void __user *src, snd_pcm_uframes_t count,bool in_kernel){ struct rme32 *rme32 = snd_pcm_substream_chip(substream); count <<= rme32->playback_frlog; pos <<= rme32->playback_frlog;
- if (copy_from_user_toio(rme32->iobase + RME32_IO_DATA_BUFFER + pos,
src, count))
- if (!src)
memset_io(rme32->iobase + RME32_IO_DATA_BUFFER + pos, 0, count);- else if (in_kernel)
memcpy_toio(rme32->iobase + RME32_IO_DATA_BUFFER + pos,(void *)src, count);- else if (copy_from_user_toio(rme32->iobase + RME32_IO_DATA_BUFFER + pos,
src, count))}
/* copy callback for halfduplex mode */ -static int snd_rme32_capture_copy(struct snd_pcm_substream *substream, int channel, /* not used (interleaved data) */ +static int snd_rme32_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)
void __user *dst, snd_pcm_uframes_t count,bool in_kernel){ struct rme32 *rme32 = snd_pcm_substream_chip(substream); count <<= rme32->capture_frlog; pos <<= rme32->capture_frlog;
- if (copy_to_user_fromio(dst,
- if (in_kernel)
memcpy_fromio((void *)dst,rme32->iobase + RME32_IO_DATA_BUFFER + pos,count);- else if (copy_to_user_fromio(dst, rme32->iobase + RME32_IO_DATA_BUFFER + pos, count)) return -EFAULT;
@@ -1205,8 +1206,7 @@ static const struct snd_pcm_ops snd_rme32_playback_spdif_ops = { .prepare = snd_rme32_playback_prepare, .trigger = snd_rme32_pcm_trigger, .pointer = snd_rme32_playback_pointer,
- .copy = snd_rme32_playback_copy,
- .silence = snd_rme32_playback_silence,
- .copy_silence = snd_rme32_playback_copy, .mmap = snd_pcm_lib_mmap_iomem,
};
@@ -1219,7 +1219,7 @@ static const struct snd_pcm_ops snd_rme32_capture_spdif_ops = { .prepare = snd_rme32_capture_prepare, .trigger = snd_rme32_pcm_trigger, .pointer = snd_rme32_capture_pointer,
- .copy = snd_rme32_capture_copy,
- .copy_silence = snd_rme32_capture_copy, .mmap = snd_pcm_lib_mmap_iomem,
};
@@ -1231,8 +1231,7 @@ static const struct snd_pcm_ops snd_rme32_playback_adat_ops = { .prepare = snd_rme32_playback_prepare, .trigger = snd_rme32_pcm_trigger, .pointer = snd_rme32_playback_pointer,
- .copy = snd_rme32_playback_copy,
- .silence = snd_rme32_playback_silence,
- .copy_silence = snd_rme32_playback_copy, .mmap = snd_pcm_lib_mmap_iomem,
};
@@ -1244,7 +1243,7 @@ static const struct snd_pcm_ops snd_rme32_capture_adat_ops = { .prepare = snd_rme32_capture_prepare, .trigger = snd_rme32_pcm_trigger, .pointer = snd_rme32_capture_pointer,
- .copy = snd_rme32_capture_copy,
- .copy_silence = snd_rme32_capture_copy, .mmap = snd_pcm_lib_mmap_iomem,
};
-- 2.12.2
Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
-- ExactCODE GmbH, Lietzenburger Str. 42, DE-10789 Berlin DE Legal: Amtsgericht Berlin (Charlottenburg) HRB 105123B, Tax-ID#: DE251602478 Managing Director: René Rebe http://exactcode.com | http://exactscan.com | http://ocrkit.com | http://t2-project.org | http://rene.rebe.de
Hi,
On 18 Jul 2018, at 20:10, René Rebe rene@exactcode.de wrote:
Hi,
On 18 Jul 2018, at 12:56, Takashi Iwai tiwai@suse.de wrote:
On Wed, 18 Jul 2018 12:22:11 +0200, René Rebe wrote:
Hello everyone,
to have another digital audio i/o card for our studio / office, I got a pair of RME32 the other week from ebay. (mostly as reference to implement ADAT for the RAD1 Sgi/Octane ALSA driver, …)
Unfortunately they do not work with Linux. They are recognised and all the usual devices and /proc/… entries show up, however, the hardware pointer does not move during playback or capture no matter what clock source I choose. I tried attaching coax s/pdif as well as an 8-channel Behringer Ultragain ADAT source w/ clock.
Does the /proc/asound/card*/rme32 entry show the right setup?
Well, I think it never showed external clock when I set it to external ADAT, right now, freshly booted (without the ext. ADAT) just trying to play stereo on the s/pdif it shows:
as initial step getting familiar with this thing, I added some prink’s, I get exactly one interrupt each time I start a playback:
$ dmesg [ 0.000000] rme32 int: 86008004 80000000 [ 0.000000] CONFIRM_IRQ [ 0.000000] IRQ_HANDLED $ grep rme32 /proc/interrupts 5: 4 XT-PIC snd_rme32
hm, … at least some io access and interrupt is happening, ...
RME Digi32/8 (Rev. 101) at 0xec000000, irq 5 (index #1)
General settings Half-duplex mode receiver: CS8412 format: 16 bit, Stereo
Input settings input: optical sample rate: no valid signal
Output settings output signal: normal playback (muted) sample rate: 44100 Hz sample clock source: Internal format: IEC958 (consumer) emphasis: off
Which should be right for internally word clock’ed s/pdif output. However, aplay does not play anything, after some 20? seconds or so of hanging it even prints:
aplay: pcm_write:2011: write error: Input/output error
While it is “trying to playback” /proc shows:
root@hostname:/proc/asound/card0# grep . pcm*p/sub0/* pcm0p/sub0/hw_params:access: RW_INTERLEAVED pcm0p/sub0/hw_params:format: S16_LE pcm0p/sub0/hw_params:subformat: STD pcm0p/sub0/hw_params:channels: 2 pcm0p/sub0/hw_params:rate: 44100 (44100/1) pcm0p/sub0/hw_params:period_size: 2048 pcm0p/sub0/hw_params:buffer_size: 32768 pcm0p/sub0/info:card: 0 pcm0p/sub0/info:device: 0 pcm0p/sub0/info:subdevice: 0 pcm0p/sub0/info:stream: PLAYBACK pcm0p/sub0/info:id: Digi32 IEC958 pcm0p/sub0/info:name: Digi32 IEC958 pcm0p/sub0/info:subname: subdevice #0 pcm0p/sub0/info:class: 0 pcm0p/sub0/info:subclass: 0 pcm0p/sub0/info:subdevices_count: 1 pcm0p/sub0/info:subdevices_avail: 0 pcm0p/sub0/status:state: RUNNING pcm0p/sub0/status:owner_pid : 753 pcm0p/sub0/status:trigger_time: 1195.174967384 pcm0p/sub0/status:tstamp : 0.000000000 pcm0p/sub0/status:delay : 32768 pcm0p/sub0/status:avail : 0 pcm0p/sub0/status:avail_max : 30720 pcm0p/sub0/status:----- pcm0p/sub0/status:hw_ptr : 10243 pcm0p/sub0/status:appl_ptr : 43011 pcm0p/sub0/sw_params:tstamp_mode: NONE pcm0p/sub0/sw_params:period_step: 1 pcm0p/sub0/sw_params:avail_min: 2048 pcm0p/sub0/sw_params:start_threshold: 32768 pcm0p/sub0/sw_params:stop_threshold: 32768 pcm0p/sub0/sw_params:silence_threshold: 0 pcm0p/sub0/sw_params:silence_size: 0 pcm0p/sub0/sw_params:boundary: 1073741824
I tried many kernels, down to 2.6.31, which I had initially on the box. So unless some this hwparams never worked, this driver appears to not work a looong looooong time.
I doubt they are fully defect, as both behave the same and at least output “something” on Windows XP, ...
RME32 seems to have only few registers, and it behaves differently for read and write. Maybe you should try to watch the register 0x20000. The hwptr is the LSB 33 bits.
Yeah, guess I have to instrument this driver to track what is happening, maybe register not flushed to the hw?
Takashi
The two cards came from the same seller, look ok and both behave the same. I went so far to install a Windows XP test install where both cards work “more”. (They are not perfect in windows, however, at least s/pdif can come out with or with-out external ADAT clock source. However, the digital signal strangely unclean, but that may be a bug in the window system sound device emulation, I only tested with foobar2k and not some Pro audio app which I do not really have).
Long story short, does someone still have such a card, or not moving hardware ptr (or missing interrupts?) does ring a bell regarding this RME generation? Instead of a working ADAT reference card, I apparently have two ALSA driver to hack on, … ;-)
I tested two different “PC” boards and the results were the same, too.
Thanks, René
On 11 May 2017, at 23:09, Takashi Iwai tiwai@suse.de wrote:
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/rme32.c | 49 ++++++++++++++++++++++++------------------------- 1 file changed, 24 insertions(+), 25 deletions(-)
diff --git a/sound/pci/rme32.c b/sound/pci/rme32.c index 96d15db65dfd..d2b4a3ef0bd3 100644 --- a/sound/pci/rme32.c +++ b/sound/pci/rme32.c @@ -253,41 +253,42 @@ static inline unsigned int snd_rme32_pcm_byteptr(struct rme32 * rme32) & RME32_RCR_AUDIO_ADDR_MASK); }
-/* silence callback for halfduplex mode */ -static int snd_rme32_playback_silence(struct snd_pcm_substream *substream, int channel, /* not used (interleaved data) */
snd_pcm_uframes_t pos,snd_pcm_uframes_t count)-{
- struct rme32 *rme32 = snd_pcm_substream_chip(substream);
- count <<= rme32->playback_frlog;
- pos <<= rme32->playback_frlog;
- memset_io(rme32->iobase + RME32_IO_DATA_BUFFER + pos, 0, count);
- return 0;
-}
/* copy callback for halfduplex mode */ -static int snd_rme32_playback_copy(struct snd_pcm_substream *substream, int channel, /* not used (interleaved data) */ +static int snd_rme32_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)
void __user *src, snd_pcm_uframes_t count,bool in_kernel){ struct rme32 *rme32 = snd_pcm_substream_chip(substream); count <<= rme32->playback_frlog; pos <<= rme32->playback_frlog;
- if (copy_from_user_toio(rme32->iobase + RME32_IO_DATA_BUFFER + pos,
src, count))
- if (!src)
memset_io(rme32->iobase + RME32_IO_DATA_BUFFER + pos, 0, count);- else if (in_kernel)
memcpy_toio(rme32->iobase + RME32_IO_DATA_BUFFER + pos,(void *)src, count);- else if (copy_from_user_toio(rme32->iobase + RME32_IO_DATA_BUFFER + pos,
src, count))}
/* copy callback for halfduplex mode */ -static int snd_rme32_capture_copy(struct snd_pcm_substream *substream, int channel, /* not used (interleaved data) */ +static int snd_rme32_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)
void __user *dst, snd_pcm_uframes_t count,bool in_kernel){ struct rme32 *rme32 = snd_pcm_substream_chip(substream); count <<= rme32->capture_frlog; pos <<= rme32->capture_frlog;
- if (copy_to_user_fromio(dst,
- if (in_kernel)
memcpy_fromio((void *)dst,rme32->iobase + RME32_IO_DATA_BUFFER + pos,count);- else if (copy_to_user_fromio(dst, rme32->iobase + RME32_IO_DATA_BUFFER + pos, count)) return -EFAULT;
@@ -1205,8 +1206,7 @@ static const struct snd_pcm_ops snd_rme32_playback_spdif_ops = { .prepare = snd_rme32_playback_prepare, .trigger = snd_rme32_pcm_trigger, .pointer = snd_rme32_playback_pointer,
- .copy = snd_rme32_playback_copy,
- .silence = snd_rme32_playback_silence,
- .copy_silence = snd_rme32_playback_copy, .mmap = snd_pcm_lib_mmap_iomem,
};
@@ -1219,7 +1219,7 @@ static const struct snd_pcm_ops snd_rme32_capture_spdif_ops = { .prepare = snd_rme32_capture_prepare, .trigger = snd_rme32_pcm_trigger, .pointer = snd_rme32_capture_pointer,
- .copy = snd_rme32_capture_copy,
- .copy_silence = snd_rme32_capture_copy, .mmap = snd_pcm_lib_mmap_iomem,
};
@@ -1231,8 +1231,7 @@ static const struct snd_pcm_ops snd_rme32_playback_adat_ops = { .prepare = snd_rme32_playback_prepare, .trigger = snd_rme32_pcm_trigger, .pointer = snd_rme32_playback_pointer,
- .copy = snd_rme32_playback_copy,
- .silence = snd_rme32_playback_silence,
- .copy_silence = snd_rme32_playback_copy, .mmap = snd_pcm_lib_mmap_iomem,
};
@@ -1244,7 +1243,7 @@ static const struct snd_pcm_ops snd_rme32_capture_adat_ops = { .prepare = snd_rme32_capture_prepare, .trigger = snd_rme32_pcm_trigger, .pointer = snd_rme32_capture_pointer,
- .copy = snd_rme32_capture_copy,
- .copy_silence = snd_rme32_capture_copy, .mmap = snd_pcm_lib_mmap_iomem,
};
-- 2.12.2
Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
-- ExactCODE GmbH, Lietzenburger Str. 42, DE-10789 Berlin DE Legal: Amtsgericht Berlin (Charlottenburg) HRB 105123B, Tax-ID#: DE251602478 Managing Director: René Rebe http://exactcode.com | http://exactscan.com | http://ocrkit.com | http://t2-project.org | http://rene.rebe.de
-- ExactCODE GmbH, Lietzenburger Str. 42, DE-10789 Berlin DE Legal: Amtsgericht Berlin (Charlottenburg) HRB 105123B, Tax-ID#: DE251602478 Managing Director: René Rebe http://exactcode.com | http://exactscan.com | http://ocrkit.com | http://t2-project.org | http://rene.rebe.de
Hi,
On 18 Jul 2018, at 20:43, René Rebe rene@exactcode.de wrote:
Hi,
On 18 Jul 2018, at 20:10, René Rebe rene@exactcode.de wrote:
Hi,
On 18 Jul 2018, at 12:56, Takashi Iwai tiwai@suse.de wrote:
On Wed, 18 Jul 2018 12:22:11 +0200, René Rebe wrote:
Hello everyone,
to have another digital audio i/o card for our studio / office, I got a pair of RME32 the other week from ebay. (mostly as reference to implement ADAT for the RAD1 Sgi/Octane ALSA driver, …)
Unfortunately they do not work with Linux. They are recognised and all the usual devices and /proc/… entries show up, however, the hardware pointer does not move during playback or capture no matter what clock source I choose. I tried attaching coax s/pdif as well as an 8-channel Behringer Ultragain ADAT source w/ clock.
Does the /proc/asound/card*/rme32 entry show the right setup?
Well, I think it never showed external clock when I set it to external ADAT, right now, freshly booted (without the ext. ADAT) just trying to play stereo on the s/pdif it shows:
as initial step getting familiar with this thing, I added some prink’s, I get exactly one interrupt each time I start a playback:
$ dmesg [ 0.000000] rme32 int: 86008004 80000000 [ 0.000000] CONFIRM_IRQ [ 0.000000] IRQ_HANDLED $ grep rme32 /proc/interrupts 5: 4 XT-PIC snd_rme32
also dumping the read and write control words, I realize:
rcreg: 6000000 wcreg: 218
that the error bit is set, … #define RME32_RCR_ERF (1 << 26) /* 1=Error, 0=no Error */
as well as not documented as macro bit 25, … hm, …
Does anyone still happen to have some documentation for this card ;-)?
René
hm, … at least some io access and interrupt is happening, ...
RME Digi32/8 (Rev. 101) at 0xec000000, irq 5 (index #1)
General settings Half-duplex mode receiver: CS8412 format: 16 bit, Stereo
Input settings input: optical sample rate: no valid signal
Output settings output signal: normal playback (muted) sample rate: 44100 Hz sample clock source: Internal format: IEC958 (consumer) emphasis: off
Which should be right for internally word clock’ed s/pdif output. However, aplay does not play anything, after some 20? seconds or so of hanging it even prints:
aplay: pcm_write:2011: write error: Input/output error
While it is “trying to playback” /proc shows:
root@hostname:/proc/asound/card0# grep . pcm*p/sub0/* pcm0p/sub0/hw_params:access: RW_INTERLEAVED pcm0p/sub0/hw_params:format: S16_LE pcm0p/sub0/hw_params:subformat: STD pcm0p/sub0/hw_params:channels: 2 pcm0p/sub0/hw_params:rate: 44100 (44100/1) pcm0p/sub0/hw_params:period_size: 2048 pcm0p/sub0/hw_params:buffer_size: 32768 pcm0p/sub0/info:card: 0 pcm0p/sub0/info:device: 0 pcm0p/sub0/info:subdevice: 0 pcm0p/sub0/info:stream: PLAYBACK pcm0p/sub0/info:id: Digi32 IEC958 pcm0p/sub0/info:name: Digi32 IEC958 pcm0p/sub0/info:subname: subdevice #0 pcm0p/sub0/info:class: 0 pcm0p/sub0/info:subclass: 0 pcm0p/sub0/info:subdevices_count: 1 pcm0p/sub0/info:subdevices_avail: 0 pcm0p/sub0/status:state: RUNNING pcm0p/sub0/status:owner_pid : 753 pcm0p/sub0/status:trigger_time: 1195.174967384 pcm0p/sub0/status:tstamp : 0.000000000 pcm0p/sub0/status:delay : 32768 pcm0p/sub0/status:avail : 0 pcm0p/sub0/status:avail_max : 30720 pcm0p/sub0/status:----- pcm0p/sub0/status:hw_ptr : 10243 pcm0p/sub0/status:appl_ptr : 43011 pcm0p/sub0/sw_params:tstamp_mode: NONE pcm0p/sub0/sw_params:period_step: 1 pcm0p/sub0/sw_params:avail_min: 2048 pcm0p/sub0/sw_params:start_threshold: 32768 pcm0p/sub0/sw_params:stop_threshold: 32768 pcm0p/sub0/sw_params:silence_threshold: 0 pcm0p/sub0/sw_params:silence_size: 0 pcm0p/sub0/sw_params:boundary: 1073741824
I tried many kernels, down to 2.6.31, which I had initially on the box. So unless some this hwparams never worked, this driver appears to not work a looong looooong time.
I doubt they are fully defect, as both behave the same and at least output “something” on Windows XP, ...
RME32 seems to have only few registers, and it behaves differently for read and write. Maybe you should try to watch the register 0x20000. The hwptr is the LSB 33 bits.
Yeah, guess I have to instrument this driver to track what is happening, maybe register not flushed to the hw?
Takashi
The two cards came from the same seller, look ok and both behave the same. I went so far to install a Windows XP test install where both cards work “more”. (They are not perfect in windows, however, at least s/pdif can come out with or with-out external ADAT clock source. However, the digital signal strangely unclean, but that may be a bug in the window system sound device emulation, I only tested with foobar2k and not some Pro audio app which I do not really have).
Long story short, does someone still have such a card, or not moving hardware ptr (or missing interrupts?) does ring a bell regarding this RME generation? Instead of a working ADAT reference card, I apparently have two ALSA driver to hack on, … ;-)
I tested two different “PC” boards and the results were the same, too.
Thanks, René
On 11 May 2017, at 23:09, Takashi Iwai tiwai@suse.de wrote:
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/rme32.c | 49 ++++++++++++++++++++++++------------------------- 1 file changed, 24 insertions(+), 25 deletions(-)
diff --git a/sound/pci/rme32.c b/sound/pci/rme32.c index 96d15db65dfd..d2b4a3ef0bd3 100644 --- a/sound/pci/rme32.c +++ b/sound/pci/rme32.c @@ -253,41 +253,42 @@ static inline unsigned int snd_rme32_pcm_byteptr(struct rme32 * rme32) & RME32_RCR_AUDIO_ADDR_MASK); }
-/* silence callback for halfduplex mode */ -static int snd_rme32_playback_silence(struct snd_pcm_substream *substream, int channel, /* not used (interleaved data) */
snd_pcm_uframes_t pos,snd_pcm_uframes_t count)-{
- struct rme32 *rme32 = snd_pcm_substream_chip(substream);
- count <<= rme32->playback_frlog;
- pos <<= rme32->playback_frlog;
- memset_io(rme32->iobase + RME32_IO_DATA_BUFFER + pos, 0, count);
- return 0;
-}
/* copy callback for halfduplex mode */ -static int snd_rme32_playback_copy(struct snd_pcm_substream *substream, int channel, /* not used (interleaved data) */ +static int snd_rme32_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)
void __user *src, snd_pcm_uframes_t count,bool in_kernel){ struct rme32 *rme32 = snd_pcm_substream_chip(substream); count <<= rme32->playback_frlog; pos <<= rme32->playback_frlog;
- if (copy_from_user_toio(rme32->iobase + RME32_IO_DATA_BUFFER + pos,
src, count))
- if (!src)
memset_io(rme32->iobase + RME32_IO_DATA_BUFFER + pos, 0, count);- else if (in_kernel)
memcpy_toio(rme32->iobase + RME32_IO_DATA_BUFFER + pos,(void *)src, count);- else if (copy_from_user_toio(rme32->iobase + RME32_IO_DATA_BUFFER + pos,
src, count))}
/* copy callback for halfduplex mode */ -static int snd_rme32_capture_copy(struct snd_pcm_substream *substream, int channel, /* not used (interleaved data) */ +static int snd_rme32_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)
void __user *dst, snd_pcm_uframes_t count,bool in_kernel){ struct rme32 *rme32 = snd_pcm_substream_chip(substream); count <<= rme32->capture_frlog; pos <<= rme32->capture_frlog;
- if (copy_to_user_fromio(dst,
- if (in_kernel)
memcpy_fromio((void *)dst,rme32->iobase + RME32_IO_DATA_BUFFER + pos,count);- else if (copy_to_user_fromio(dst, rme32->iobase + RME32_IO_DATA_BUFFER + pos, count)) return -EFAULT;
@@ -1205,8 +1206,7 @@ static const struct snd_pcm_ops snd_rme32_playback_spdif_ops = { .prepare = snd_rme32_playback_prepare, .trigger = snd_rme32_pcm_trigger, .pointer = snd_rme32_playback_pointer,
- .copy = snd_rme32_playback_copy,
- .silence = snd_rme32_playback_silence,
- .copy_silence = snd_rme32_playback_copy, .mmap = snd_pcm_lib_mmap_iomem,
};
@@ -1219,7 +1219,7 @@ static const struct snd_pcm_ops snd_rme32_capture_spdif_ops = { .prepare = snd_rme32_capture_prepare, .trigger = snd_rme32_pcm_trigger, .pointer = snd_rme32_capture_pointer,
- .copy = snd_rme32_capture_copy,
- .copy_silence = snd_rme32_capture_copy, .mmap = snd_pcm_lib_mmap_iomem,
};
@@ -1231,8 +1231,7 @@ static const struct snd_pcm_ops snd_rme32_playback_adat_ops = { .prepare = snd_rme32_playback_prepare, .trigger = snd_rme32_pcm_trigger, .pointer = snd_rme32_playback_pointer,
- .copy = snd_rme32_playback_copy,
- .silence = snd_rme32_playback_silence,
- .copy_silence = snd_rme32_playback_copy, .mmap = snd_pcm_lib_mmap_iomem,
};
@@ -1244,7 +1243,7 @@ static const struct snd_pcm_ops snd_rme32_capture_adat_ops = { .prepare = snd_rme32_capture_prepare, .trigger = snd_rme32_pcm_trigger, .pointer = snd_rme32_capture_pointer,
- .copy = snd_rme32_capture_copy,
- .copy_silence = snd_rme32_capture_copy, .mmap = snd_pcm_lib_mmap_iomem,
};
-- 2.12.2
Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
-- ExactCODE GmbH, Lietzenburger Str. 42, DE-10789 Berlin DE Legal: Amtsgericht Berlin (Charlottenburg) HRB 105123B, Tax-ID#: DE251602478 Managing Director: René Rebe http://exactcode.com | http://exactscan.com | http://ocrkit.com | http://t2-project.org | http://rene.rebe.de
-- ExactCODE GmbH, Lietzenburger Str. 42, DE-10789 Berlin DE Legal: Amtsgericht Berlin (Charlottenburg) HRB 105123B, Tax-ID#: DE251602478 Managing Director: René Rebe http://exactcode.com | http://exactscan.com | http://ocrkit.com | http://t2-project.org | http://rene.rebe.de
-- ExactCODE GmbH, Lietzenburger Str. 42, DE-10789 Berlin DE Legal: Amtsgericht Berlin (Charlottenburg) HRB 105123B, Tax-ID#: DE251602478 Managing Director: René Rebe http://exactcode.com | http://exactscan.com | http://ocrkit.com | http://t2-project.org | http://rene.rebe.de
Hi Takashi,
On 18 Jul 2018, at 12:56, Takashi Iwai tiwai@suse.de wrote:
to have another digital audio i/o card for our studio / office, I got a pair of RME32 the other week from ebay. (mostly as reference to implement ADAT for the RAD1 Sgi/Octane ALSA driver, …)
Unfortunately they do not work with Linux. They are recognised and all the usual devices and /proc/… entries show up, however, the hardware pointer does not move during playback or capture no matter what clock source I choose. I tried attaching coax s/pdif as well as an 8-channel Behringer Ultragain ADAT source w/ clock.
Does the /proc/asound/card*/rme32 entry show the right setup? RME32 seems to have only few registers, and it behaves differently for read and write. Maybe you should try to watch the register 0x20000. The hwptr is the LSB 33 bits.
thank you again for your reply, and I finally took a deep breath to "shortly" take a closer look at this again. So with current linux-kernel :HEAD nothing has changed, and I get one IRQ when the playback starts.
As I do not have the spec I did a quick hack and commented out the IRQ acknowledge around rme32.c line 829:
- writel(0, rme32->iobase + RME32_IO_CONFIRM_ACTION_IRQ);
surprisingly this “works” in that I have more than the first frame coming out of the optical fibre, aka quite constant running pcm stream.
"Quite constant” because I think I sometimes (rarely) here some samples getting lost, probably because the IRQ keeps firing as it was not acknowledged. However, I’m surprised this works 99%ish anyway.
Of course the questions is now to actually properly fix this, as acknowledging this IRQ somehow makes the card stop entirely, I guess, somehow? At least that is how it looks like. Maybe you have an idea? Maybe some ALSA stream helper refactoring broke something a decade ago?
I should probably also mention that this works with aplay, while with sox’s “play” this hack is somehow producing constant under-run’s:
In:85.6% 00:03:18.02 [00:00:33.35] Out:8.73M [!=====|=====!] Hd:0.0 Clip:0 play WARN alsa: under-run play WARN alsa: under-run play WARN alsa: under-run
which might be an indication of some stream pointer helper glue not being wired up correctly anymore (I tested some seriously old version the last time, like 2.6.29’ish, so this might be broken since over a decade or so already)?
Thanks again for any tips, as otherwise I might spend many more hours trying to understand the FPGA and ALSA API.
Have a nice weekend, René
Hey,
On 02 Mar 2019, at 18:19, René Rebe rene@exactcode.de wrote:
Hi Takashi,
On 18 Jul 2018, at 12:56, Takashi Iwai tiwai@suse.de wrote:
to have another digital audio i/o card for our studio / office, I got a pair of RME32 the other week from ebay. (mostly as reference to implement ADAT for the RAD1 Sgi/Octane ALSA driver, …)
Unfortunately they do not work with Linux. They are recognised and all the usual devices and /proc/… entries show up, however, the hardware pointer does not move during playback or capture no matter what clock source I choose. I tried attaching coax s/pdif as well as an 8-channel Behringer Ultragain ADAT source w/ clock.
Does the /proc/asound/card*/rme32 entry show the right setup? RME32 seems to have only few registers, and it behaves differently for read and write. Maybe you should try to watch the register 0x20000. The hwptr is the LSB 33 bits.
thank you again for your reply, and I finally took a deep breath to "shortly" take a closer look at this again. So with current linux-kernel :HEAD nothing has changed, and I get one IRQ when the playback starts.
As I do not have the spec I did a quick hack and commented out the IRQ acknowledge around rme32.c line 829:
writel(0, rme32->iobase + RME32_IO_CONFIRM_ACTION_IRQ);surprisingly this “works” in that I have more than the first frame coming out of the optical fibre, aka quite constant running pcm stream.
"Quite constant” because I think I sometimes (rarely) here some samples getting lost, probably because the IRQ keeps firing as it was not acknowledged. However, I’m surprised this works 99%ish anyway.
Of course the questions is now to actually properly fix this, as acknowledging this IRQ somehow makes the card stop entirely, I guess, somehow? At least that is how it looks like. Maybe you have an idea? Maybe some ALSA stream helper refactoring broke something a decade ago?
I should probably also mention that this works with aplay, while with sox’s “play” this hack is somehow producing constant under-run’s:
In:85.6% 00:03:18.02 [00:00:33.35] Out:8.73M [!=====|=====!] Hd:0.0 Clip:0 play WARN alsa: under-run play WARN alsa: under-run play WARN alsa: under-run
which might be an indication of some stream pointer helper glue not being wired up correctly anymore (I tested some seriously old version the last time, like 2.6.29’ish, so this might be broken since over a decade or so already)?
Thanks again for any tips, as otherwise I might spend many more hours trying to understand the FPGA and ALSA API.
So as the card sort of works in Windows,
https://www.youtube.com/watch?v=dxMH3MMyKZ4
I spent some more time finding a workaround.
https://www.youtube.com/watch?v=78-JxDzbHX8
Writing something else then 0 to the interrupt acknowledge register keeps the stream running. Writing 0xffffffff causes the stream to corrupt. So I thought maybe this FPGA bitstream copies this value into the command register, and writing that register copy at least playback work for me now. Capture does still not work, but that is probably a story to be continued another month:
--- linux-4.20/sound/pci/rme32.c.vanilla 2019-03-03 15:09:34.485653177 +0000 +++ linux-4.20/sound/pci/rme32.c 2019-03-03 15:09:51.077653422 +0000 @@ -863,7 +863,7 @@ if (rme32->playback_substream) { snd_pcm_period_elapsed(rme32->playback_substream); } - writel(0, rme32->iobase + RME32_IO_CONFIRM_ACTION_IRQ); + writel(rme32->wcreg, rme32->iobase + RME32_IO_CONFIRM_ACTION_IRQ); }
return IRQ_HANDLED;
In case this stupid Mail.app causes white space damages: https://svn.exactcode.de/t2/trunk/package/base/linux/rme32-hothack.patch
PS: as per the above 3h video I read the disassembly and confirmed the value is written into the RME32_IO_CONFIRM_ACTION_IRQ MMMIO location.
René
On Sun, 24 Mar 2019 12:19:49 +0100, René Rebe wrote:
Hey,
On 02 Mar 2019, at 18:19, René Rebe rene@exactcode.de wrote:
Hi Takashi,
On 18 Jul 2018, at 12:56, Takashi Iwai tiwai@suse.de wrote:
to have another digital audio i/o card for our studio / office, I got a pair of RME32 the other week from ebay. (mostly as reference to implement ADAT for the RAD1 Sgi/Octane ALSA driver, …)
Unfortunately they do not work with Linux. They are recognised and all the usual devices and /proc/… entries show up, however, the hardware pointer does not move during playback or capture no matter what clock source I choose. I tried attaching coax s/pdif as well as an 8-channel Behringer Ultragain ADAT source w/ clock.
Does the /proc/asound/card*/rme32 entry show the right setup? RME32 seems to have only few registers, and it behaves differently for read and write. Maybe you should try to watch the register 0x20000. The hwptr is the LSB 33 bits.
thank you again for your reply, and I finally took a deep breath to "shortly" take a closer look at this again. So with current linux-kernel :HEAD nothing has changed, and I get one IRQ when the playback starts.
As I do not have the spec I did a quick hack and commented out the IRQ acknowledge around rme32.c line 829:
writel(0, rme32->iobase + RME32_IO_CONFIRM_ACTION_IRQ);surprisingly this “works” in that I have more than the first frame coming out of the optical fibre, aka quite constant running pcm stream.
"Quite constant” because I think I sometimes (rarely) here some samples getting lost, probably because the IRQ keeps firing as it was not acknowledged. However, I’m surprised this works 99%ish anyway.
Of course the questions is now to actually properly fix this, as acknowledging this IRQ somehow makes the card stop entirely, I guess, somehow? At least that is how it looks like. Maybe you have an idea? Maybe some ALSA stream helper refactoring broke something a decade ago?
I should probably also mention that this works with aplay, while with sox’s “play” this hack is somehow producing constant under-run’s:
In:85.6% 00:03:18.02 [00:00:33.35] Out:8.73M [!=====|=====!] Hd:0.0 Clip:0 play WARN alsa: under-run play WARN alsa: under-run play WARN alsa: under-run
which might be an indication of some stream pointer helper glue not being wired up correctly anymore (I tested some seriously old version the last time, like 2.6.29’ish, so this might be broken since over a decade or so already)?
Thanks again for any tips, as otherwise I might spend many more hours trying to understand the FPGA and ALSA API.
So as the card sort of works in Windows,
https://www.youtube.com/watch?v=dxMH3MMyKZ4
I spent some more time finding a workaround.
https://www.youtube.com/watch?v=78-JxDzbHX8
Writing something else then 0 to the interrupt acknowledge register keeps the stream running. Writing 0xffffffff causes the stream to corrupt. So I thought maybe this FPGA bitstream copies this value into the command register, and writing that register copy at least playback work for me now. Capture does still not work, but that is probably a story to be continued another month:
--- linux-4.20/sound/pci/rme32.c.vanilla 2019-03-03 15:09:34.485653177 +0000 +++ linux-4.20/sound/pci/rme32.c 2019-03-03 15:09:51.077653422 +0000 @@ -863,7 +863,7 @@ if (rme32->playback_substream) { snd_pcm_period_elapsed(rme32->playback_substream); }
writel(0, rme32->iobase + RME32_IO_CONFIRM_ACTION_IRQ);
writel(rme32->wcreg, rme32->iobase + RME32_IO_CONFIRM_ACTION_IRQ);}
return IRQ_HANDLED;
In case this stupid Mail.app causes white space damages: https://svn.exactcode.de/t2/trunk/package/base/linux/rme32-hothack.patch
PS: as per the above 3h video I read the disassembly and confirmed the value is written into the RME32_IO_CONFIRM_ACTION_IRQ MMMIO location.
OK, that sounds reasonable. Actually writing 0 for ACK is somewhat weird, I guess the driver code took the behavior from rme96.c.
Possibly the behavior depends on boards, but as long as you can see it working with the change, it's fine to apply the change now, of course.
Would you submit a proper patch, or rather let me do that in my side?
thanks,
Takashi
Hi,
thanks for the reply:
On 27 Mar 2019, at 12:14, Takashi Iwai tiwai@suse.de wrote:
On Sun, 24 Mar 2019 12:19:49 +0100, René Rebe wrote:
Hey,
On 02 Mar 2019, at 18:19, René Rebe rene@exactcode.de wrote:
Hi Takashi,
On 18 Jul 2018, at 12:56, Takashi Iwai tiwai@suse.de wrote:
to have another digital audio i/o card for our studio / office, I got a pair of RME32 the other week from ebay. (mostly as reference to implement ADAT for the RAD1 Sgi/Octane ALSA driver, …)
Unfortunately they do not work with Linux. They are recognised and all the usual devices and /proc/… entries show up, however, the hardware pointer does not move during playback or capture no matter what clock source I choose. I tried attaching coax s/pdif as well as an 8-channel Behringer Ultragain ADAT source w/ clock.
Does the /proc/asound/card*/rme32 entry show the right setup? RME32 seems to have only few registers, and it behaves differently for read and write. Maybe you should try to watch the register 0x20000. The hwptr is the LSB 33 bits.
thank you again for your reply, and I finally took a deep breath to "shortly" take a closer look at this again. So with current linux-kernel :HEAD nothing has changed, and I get one IRQ when the playback starts.
As I do not have the spec I did a quick hack and commented out the IRQ acknowledge around rme32.c line 829:
writel(0, rme32->iobase + RME32_IO_CONFIRM_ACTION_IRQ);surprisingly this “works” in that I have more than the first frame coming out of the optical fibre, aka quite constant running pcm stream.
"Quite constant” because I think I sometimes (rarely) here some samples getting lost, probably because the IRQ keeps firing as it was not acknowledged. However, I’m surprised this works 99%ish anyway.
Of course the questions is now to actually properly fix this, as acknowledging this IRQ somehow makes the card stop entirely, I guess, somehow? At least that is how it looks like. Maybe you have an idea? Maybe some ALSA stream helper refactoring broke something a decade ago?
I should probably also mention that this works with aplay, while with sox’s “play” this hack is somehow producing constant under-run’s:
In:85.6% 00:03:18.02 [00:00:33.35] Out:8.73M [!=====|=====!] Hd:0.0 Clip:0 play WARN alsa: under-run play WARN alsa: under-run play WARN alsa: under-run
which might be an indication of some stream pointer helper glue not being wired up correctly anymore (I tested some seriously old version the last time, like 2.6.29’ish, so this might be broken since over a decade or so already)?
Thanks again for any tips, as otherwise I might spend many more hours trying to understand the FPGA and ALSA API.
So as the card sort of works in Windows,
https://www.youtube.com/watch?v=dxMH3MMyKZ4
I spent some more time finding a workaround.
https://www.youtube.com/watch?v=78-JxDzbHX8
Writing something else then 0 to the interrupt acknowledge register keeps the stream running. Writing 0xffffffff causes the stream to corrupt. So I thought maybe this FPGA bitstream copies this value into the command register, and writing that register copy at least playback work for me now. Capture does still not work, but that is probably a story to be continued another month:
--- linux-4.20/sound/pci/rme32.c.vanilla 2019-03-03 15:09:34.485653177 +0000 +++ linux-4.20/sound/pci/rme32.c 2019-03-03 15:09:51.077653422 +0000 @@ -863,7 +863,7 @@ if (rme32->playback_substream) { snd_pcm_period_elapsed(rme32->playback_substream); }
writel(0, rme32->iobase + RME32_IO_CONFIRM_ACTION_IRQ);
writel(rme32->wcreg, rme32->iobase + RME32_IO_CONFIRM_ACTION_IRQ);}
return IRQ_HANDLED;
In case this stupid Mail.app causes white space damages: https://svn.exactcode.de/t2/trunk/package/base/linux/rme32-hothack.patch
PS: as per the above 3h video I read the disassembly and confirmed the value is written into the RME32_IO_CONFIRM_ACTION_IRQ MMMIO location.
OK, that sounds reasonable. Actually writing 0 for ACK is somewhat weird, I guess the driver code took the behavior from rme96.c.
Possibly the behavior depends on boards, but as long as you can see it working with the change, it's fine to apply the change now, of course.
Would you submit a proper patch, or rather let me do that in my side?
We can wait until next week, maybe I find time this weekend to further poke around to get capture working. I will resend a well formatted patch in either case ;-)
René
On Wed, 27 Mar 2019 12:20:30 +0100, René Rebe wrote:
Hi,
thanks for the reply:
On 27 Mar 2019, at 12:14, Takashi Iwai tiwai@suse.de wrote:
On Sun, 24 Mar 2019 12:19:49 +0100, René Rebe wrote: Hey, On 02 Mar 2019, at 18:19, René Rebe <rene@exactcode.de> wrote: Hi Takashi, On 18 Jul 2018, at 12:56, Takashi Iwai <tiwai@suse.de> wrote: to have another digital audio i/o card for our studio / office, I got a pair of RME32 the other week from ebay. (mostly as reference to implement ADAT for the RAD1 Sgi/ Octane ALSA driver, …) Unfortunately they do not work with Linux. They are recognised and all the usual devices and /proc/… entries show up, however, the hardware pointer does not move during playback or capture no matter what clock source I choose. I tried attaching coax s/pdif as well as an 8-channel Behringer Ultragain ADAT source w/ clock. Does the /proc/asound/card*/rme32 entry show the right setup? RME32 seems to have only few registers, and it behaves differently for read and write. Maybe you should try to watch the register 0x20000. The hwptr is the LSB 33 bits. thank you again for your reply, and I finally took a deep breath to "shortly" take a closer look at this again. So with current linux-kernel :HEAD nothing has changed, and I get one IRQ when the playback starts. As I do not have the spec I did a quick hack and commented out the IRQ acknowledge around rme32.c line 829: - writel(0, rme32->iobase + RME32_IO_CONFIRM_ACTION_IRQ); surprisingly this “works” in that I have more than the first frame coming out of the optical fibre, aka quite constant running pcm stream. "Quite constant” because I think I sometimes (rarely) here some samples getting lost, probably because the IRQ keeps firing as it was not acknowledged. However, I’m surprised this works 99%ish anyway. Of course the questions is now to actually properly fix this, as acknowledging this IRQ somehow makes the card stop entirely, I guess, somehow? At least that is how it looks like. Maybe you have an idea? Maybe some ALSA stream helper refactoring broke something a decade ago? I should probably also mention that this works with aplay, while with sox’s “play” this hack is somehow producing constant under-run’s: In:85.6% 00:03:18.02 [00:00:33.35] Out:8.73M [!=====|=====!] Hd:0.0 Clip:0 play WARN alsa: under-run play WARN alsa: under-run play WARN alsa: under-run which might be an indication of some stream pointer helper glue not being wired up correctly anymore (I tested some seriously old version the last time, like 2.6.29’ish, so this might be broken since over a decade or so already)? Thanks again for any tips, as otherwise I might spend many more hours trying to understand the FPGA and ALSA API. So as the card sort of works in Windows, https://www.youtube.com/watch?v=dxMH3MMyKZ4 I spent some more time finding a workaround. https://www.youtube.com/watch?v=78-JxDzbHX8 Writing something else then 0 to the interrupt acknowledge register keeps the stream running. Writing 0xffffffff causes the stream to corrupt. So I thought maybe this FPGA bitstream copies this value into the command register, and writing that register copy at least playback work for me now. Capture does still not work, but that is probably a story to be continued another month: --- linux-4.20/sound/pci/rme32.c.vanilla 2019-03-03 15:09:34.485653177 +0000 +++ linux-4.20/sound/pci/rme32.c 2019-03-03 15:09:51.077653422 +0000 @@ -863,7 +863,7 @@ if (rme32->playback_substream) { snd_pcm_period_elapsed(rme32->playback_substream); } - writel(0, rme32->iobase + RME32_IO_CONFIRM_ACTION_IRQ); + writel(rme32->wcreg, rme32->iobase + RME32_IO_CONFIRM_ACTION_IRQ); } return IRQ_HANDLED; In case this stupid Mail.app causes white space damages: https://svn.exactcode.de/t2/trunk/package/base/linux/rme32-hothack.patch PS: as per the above 3h video I read the disassembly and confirmed the value is written into the RME32_IO_CONFIRM_ACTION_IRQ MMMIO location. OK, that sounds reasonable. Actually writing 0 for ACK is somewhat weird, I guess the driver code took the behavior from rme96.c. Possibly the behavior depends on boards, but as long as you can see it working with the change, it's fine to apply the change now, of course. Would you submit a proper patch, or rather let me do that in my side?We can wait until next week, maybe I find time this weekend to further poke around to get capture working. I will resend a well formatted patch in either case ;-)
OK, have fun :)
For the capture, you might need to try both full-duplex and half-duplex modes. They behave completely differently.
I'd start from checking with the direct mmap in half-duplex mode (default).
thanks,
Takashi
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/rme96.c | 52 ++++++++++++++++++++++++++-------------------------- 1 file changed, 26 insertions(+), 26 deletions(-)
diff --git a/sound/pci/rme96.c b/sound/pci/rme96.c index 05b9da30990d..2161f6aad532 100644 --- a/sound/pci/rme96.c +++ b/sound/pci/rme96.c @@ -326,31 +326,25 @@ snd_rme96_capture_ptr(struct rme96 *rme96) }
static int -snd_rme96_playback_silence(struct snd_pcm_substream *substream, - int channel, /* not used (interleaved data) */ - snd_pcm_uframes_t pos, - snd_pcm_uframes_t count) -{ - struct rme96 *rme96 = snd_pcm_substream_chip(substream); - count <<= rme96->playback_frlog; - pos <<= rme96->playback_frlog; - memset_io(rme96->iobase + RME96_IO_PLAY_BUFFER + pos, - 0, count); - return 0; -} - -static int snd_rme96_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) + snd_pcm_uframes_t count, + bool in_kernel) { struct rme96 *rme96 = snd_pcm_substream_chip(substream); count <<= rme96->playback_frlog; pos <<= rme96->playback_frlog; - return copy_from_user_toio(rme96->iobase + RME96_IO_PLAY_BUFFER + pos, src, - count); + if (!src) + memset_io(rme96->iobase + RME96_IO_PLAY_BUFFER + pos, 0, count); + else if (in_kernel) + memcpy_toio(rme96->iobase + RME96_IO_PLAY_BUFFER + pos, + (void *)src, count); + else if (copy_from_user_toio(rme96->iobase + RME96_IO_PLAY_BUFFER + pos, + src, count)) + return -EFAULT; + return 0; }
static int @@ -358,13 +352,21 @@ snd_rme96_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) + snd_pcm_uframes_t count, + bool in_kernel) { struct rme96 *rme96 = snd_pcm_substream_chip(substream); count <<= rme96->capture_frlog; pos <<= rme96->capture_frlog; - return copy_to_user_fromio(dst, rme96->iobase + RME96_IO_REC_BUFFER + pos, - count); + if (in_kernel) + memcpy_fromio((void *)dst, + rme96->iobase + RME96_IO_REC_BUFFER + pos, + count); + else if (copy_to_user_fromio(dst, + rme96->iobase + RME96_IO_REC_BUFFER + pos, + count)) + return -EFAULT; + return 0; }
/* @@ -1513,8 +1515,7 @@ static const struct snd_pcm_ops snd_rme96_playback_spdif_ops = { .prepare = snd_rme96_playback_prepare, .trigger = snd_rme96_playback_trigger, .pointer = snd_rme96_playback_pointer, - .copy = snd_rme96_playback_copy, - .silence = snd_rme96_playback_silence, + .copy_silence = snd_rme96_playback_copy, .mmap = snd_pcm_lib_mmap_iomem, };
@@ -1526,7 +1527,7 @@ static const struct snd_pcm_ops snd_rme96_capture_spdif_ops = { .prepare = snd_rme96_capture_prepare, .trigger = snd_rme96_capture_trigger, .pointer = snd_rme96_capture_pointer, - .copy = snd_rme96_capture_copy, + .copy_silence = snd_rme96_capture_copy, .mmap = snd_pcm_lib_mmap_iomem, };
@@ -1538,8 +1539,7 @@ static const struct snd_pcm_ops snd_rme96_playback_adat_ops = { .prepare = snd_rme96_playback_prepare, .trigger = snd_rme96_playback_trigger, .pointer = snd_rme96_playback_pointer, - .copy = snd_rme96_playback_copy, - .silence = snd_rme96_playback_silence, + .copy_silence = snd_rme96_playback_copy, .mmap = snd_pcm_lib_mmap_iomem, };
@@ -1551,7 +1551,7 @@ static const struct snd_pcm_ops snd_rme96_capture_adat_ops = { .prepare = snd_rme96_capture_prepare, .trigger = snd_rme96_capture_trigger, .pointer = snd_rme96_capture_pointer, - .copy = snd_rme96_capture_copy, + .copy_silence = snd_rme96_capture_copy, .mmap = snd_pcm_lib_mmap_iomem, };
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/rme9652/rme9652.c | 46 ++++++++++++++++++++------------------------- 1 file changed, 20 insertions(+), 26 deletions(-)
diff --git a/sound/pci/rme9652/rme9652.c b/sound/pci/rme9652/rme9652.c index 55172c689991..ce9faa32c7ed 100644 --- a/sound/pci/rme9652/rme9652.c +++ b/sound/pci/rme9652/rme9652.c @@ -1883,8 +1883,10 @@ 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, snd_pcm_uframes_t pos, + void __user *src, snd_pcm_uframes_t count, + bool in_kernel) { struct snd_rme9652 *rme9652 = snd_pcm_substream_chip(substream); char *channel_buf; @@ -1897,13 +1899,19 @@ 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 (!src) + memset(channel_buf + pos * 4, 0, count * 4); + else if (in_kernel) + memcpy(channel_buf + pos * 4, (void *)src, count * 4); + else if (copy_from_user(channel_buf + pos * 4, src, count * 4)) 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_capture_copy(struct snd_pcm_substream *substream, + int channel, snd_pcm_uframes_t pos, + void __user *dst, snd_pcm_uframes_t count, + bool in_kernel) { struct snd_rme9652 *rme9652 = snd_pcm_substream_chip(substream); char *channel_buf; @@ -1916,24 +1924,11 @@ 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 (in_kernel) + memcpy((void *)dst, channel_buf + pos * 4, count * 4); + else if (copy_to_user(dst, channel_buf + pos * 4, count * 4)) return -EFAULT; - return count; -} - -static int snd_rme9652_hw_silence(struct snd_pcm_substream *substream, int channel, - snd_pcm_uframes_t pos, snd_pcm_uframes_t 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; - memset(channel_buf + pos * 4, 0, count * 4); - return count; + return 0; }
static int snd_rme9652_reset(struct snd_pcm_substream *substream) @@ -2376,8 +2371,7 @@ 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_silence = snd_rme9652_playback_copy, };
static const struct snd_pcm_ops snd_rme9652_capture_ops = { @@ -2388,7 +2382,7 @@ 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_silence = snd_rme9652_capture_copy, };
static int snd_rme9652_create_pcm(struct snd_card *card,
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/rme9652/hdsp.c | 44 ++++++++++++++++++++------------------------ 1 file changed, 20 insertions(+), 24 deletions(-)
diff --git a/sound/pci/rme9652/hdsp.c b/sound/pci/rme9652/hdsp.c index fc0face6cdc6..5325e91fc3a8 100644 --- a/sound/pci/rme9652/hdsp.c +++ b/sound/pci/rme9652/hdsp.c @@ -3913,8 +3913,10 @@ static char *hdsp_channel_buffer_location(struct hdsp *hdsp, return hdsp->playback_buffer + (mapped_channel * HDSP_CHANNEL_BUFFER_BYTES); }
-static int snd_hdsp_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_hdsp_playback_copy(struct snd_pcm_substream *substream, + int channel, snd_pcm_uframes_t pos, + void __user *src, snd_pcm_uframes_t count, + bool in_kernel) { struct hdsp *hdsp = snd_pcm_substream_chip(substream); char *channel_buf; @@ -3925,13 +3927,19 @@ static int snd_hdsp_playback_copy(struct snd_pcm_substream *substream, int chann channel_buf = hdsp_channel_buffer_location (hdsp, substream->pstr->stream, channel); if (snd_BUG_ON(!channel_buf)) return -EIO; - if (copy_from_user(channel_buf + pos * 4, src, count * 4)) + if (!src) + memset(channel_buf + pos * 4, 0, count * 4); + else if (in_kernel) + memcpy(channel_buf + pos * 4, (void *)src, count * 4); + else if (copy_from_user(channel_buf + pos * 4, src, count * 4)) return -EFAULT; - return count; + return 0; }
-static int snd_hdsp_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_hdsp_capture_copy(struct snd_pcm_substream *substream, + int channel, snd_pcm_uframes_t pos, + void __user *dst, snd_pcm_uframes_t count, + bool in_kernel) { struct hdsp *hdsp = snd_pcm_substream_chip(substream); char *channel_buf; @@ -3942,22 +3950,11 @@ static int snd_hdsp_capture_copy(struct snd_pcm_substream *substream, int channe channel_buf = hdsp_channel_buffer_location (hdsp, substream->pstr->stream, channel); if (snd_BUG_ON(!channel_buf)) return -EIO; - if (copy_to_user(dst, channel_buf + pos * 4, count * 4)) + if (in_kernel) + memcpy((void *)dst, channel_buf + pos * 4, count * 4); + else if (copy_to_user(dst, channel_buf + pos * 4, count * 4)) return -EFAULT; - return count; -} - -static int snd_hdsp_hw_silence(struct snd_pcm_substream *substream, int channel, - snd_pcm_uframes_t pos, snd_pcm_uframes_t count) -{ - struct hdsp *hdsp = snd_pcm_substream_chip(substream); - char *channel_buf; - - channel_buf = hdsp_channel_buffer_location (hdsp, substream->pstr->stream, channel); - if (snd_BUG_ON(!channel_buf)) - return -EIO; - memset(channel_buf + pos * 4, 0, count * 4); - return count; + return 0; }
static int snd_hdsp_reset(struct snd_pcm_substream *substream) @@ -4869,8 +4866,7 @@ static const struct snd_pcm_ops snd_hdsp_playback_ops = { .prepare = snd_hdsp_prepare, .trigger = snd_hdsp_trigger, .pointer = snd_hdsp_hw_pointer, - .copy = snd_hdsp_playback_copy, - .silence = snd_hdsp_hw_silence, + .copy_silence = snd_hdsp_playback_copy, };
static const struct snd_pcm_ops snd_hdsp_capture_ops = { @@ -4881,7 +4877,7 @@ static const struct snd_pcm_ops snd_hdsp_capture_ops = { .prepare = snd_hdsp_prepare, .trigger = snd_hdsp_trigger, .pointer = snd_hdsp_hw_pointer, - .copy = snd_hdsp_capture_copy, + .copy_silence = snd_hdsp_capture_copy, };
static int snd_hdsp_create_hwdep(struct snd_card *card, struct hdsp *hdsp)
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/isa/gus/gus_pcm.c | 43 +++++++++---------------------------------- 1 file changed, 9 insertions(+), 34 deletions(-)
diff --git a/sound/isa/gus/gus_pcm.c b/sound/isa/gus/gus_pcm.c index 06505999155f..c58d26295543 100644 --- a/sound/isa/gus/gus_pcm.c +++ b/sound/isa/gus/gus_pcm.c @@ -359,7 +359,8 @@ static int snd_gf1_pcm_playback_copy(struct snd_pcm_substream *substream, int voice, snd_pcm_uframes_t pos, void __user *src, - snd_pcm_uframes_t count) + snd_pcm_uframes_t count, + bool in_kernel) { struct snd_pcm_runtime *runtime = substream->runtime; struct gus_pcm_private *pcmp = runtime->private_data; @@ -371,7 +372,12 @@ static int snd_gf1_pcm_playback_copy(struct snd_pcm_substream *substream, return -EIO; if (snd_BUG_ON(bpos + len > pcmp->dma_size)) return -EIO; - if (copy_from_user(runtime->dma_area + bpos, src, len)) + if (!src) + snd_pcm_format_set_silence(runtime->format, + runtime->dma_area + bpos, count); + else if (in_kernel) + memcpy(runtime->dma_area + bpos, (void *)src, len); + else if (copy_from_user(runtime->dma_area + bpos, src, len)) return -EFAULT; if (snd_gf1_pcm_use_dma && len > 32) { return snd_gf1_pcm_block_change(substream, bpos, pcmp->memory + bpos, len); @@ -387,36 +393,6 @@ static int snd_gf1_pcm_playback_copy(struct snd_pcm_substream *substream, return 0; }
-static int snd_gf1_pcm_playback_silence(struct snd_pcm_substream *substream, - int voice, - snd_pcm_uframes_t pos, - snd_pcm_uframes_t count) -{ - struct snd_pcm_runtime *runtime = substream->runtime; - struct gus_pcm_private *pcmp = runtime->private_data; - unsigned int bpos, len; - - bpos = samples_to_bytes(runtime, pos) + (voice * (pcmp->dma_size / 2)); - len = samples_to_bytes(runtime, count); - if (snd_BUG_ON(bpos > pcmp->dma_size)) - return -EIO; - if (snd_BUG_ON(bpos + len > pcmp->dma_size)) - return -EIO; - snd_pcm_format_set_silence(runtime->format, runtime->dma_area + bpos, count); - if (snd_gf1_pcm_use_dma && len > 32) { - return snd_gf1_pcm_block_change(substream, bpos, pcmp->memory + bpos, len); - } else { - struct snd_gus_card *gus = pcmp->gus; - int err, w16, invert; - - w16 = (snd_pcm_format_width(runtime->format) == 16); - invert = snd_pcm_format_unsigned(runtime->format); - if ((err = snd_gf1_pcm_poke_block(gus, runtime->dma_area + bpos, pcmp->memory + bpos, len, w16, invert)) < 0) - return err; - } - return 0; -} - static int snd_gf1_pcm_playback_hw_params(struct snd_pcm_substream *substream, struct snd_pcm_hw_params *hw_params) { @@ -836,8 +812,7 @@ static struct snd_pcm_ops snd_gf1_pcm_playback_ops = { .prepare = snd_gf1_pcm_playback_prepare, .trigger = snd_gf1_pcm_playback_trigger, .pointer = snd_gf1_pcm_playback_pointer, - .copy = snd_gf1_pcm_playback_copy, - .silence = snd_gf1_pcm_playback_silence, + .copy_silence = snd_gf1_pcm_playback_copy, };
static struct snd_pcm_ops snd_gf1_pcm_capture_ops = {
Replace the copy and the silence ops with the new merged ops. We could reduce the redundant silence code by that.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/isa/sb/emu8000_pcm.c | 99 ++++++++++++++-------------------------------- 1 file changed, 30 insertions(+), 69 deletions(-)
diff --git a/sound/isa/sb/emu8000_pcm.c b/sound/isa/sb/emu8000_pcm.c index 32f234f494e5..fd42ae2f73b8 100644 --- a/sound/isa/sb/emu8000_pcm.c +++ b/sound/isa/sb/emu8000_pcm.c @@ -422,16 +422,28 @@ do { \ return -EAGAIN;\ } while (0)
+static inline int get_val(unsigned short *sval, unsigned short __user *buf, + bool in_kernel) +{ + if (!buf) + *sval = 0; + else if (in_kernel) + *sval = *(unsigned short *)buf; + else if (get_user(*sval, buf)) + return -EFAULT; + return 0; +}
#ifdef USE_NONINTERLEAVE /* copy one channel block */ -static int emu8k_transfer_block(struct snd_emu8000 *emu, int offset, unsigned short *buf, int count) +static int emu8k_transfer_block(struct snd_emu8000 *emu, int offset, + unsigned short *buf, int count, bool in_kernel) { EMU8000_SMALW_WRITE(emu, offset); while (count > 0) { unsigned short sval; CHECK_SCHEDULER(); - if (get_user(sval, buf)) + if (get_val(&sval, buf, in_kernel)) return -EFAULT; EMU8000_SMLD_WRITE(emu, sval); buf++; @@ -455,48 +467,18 @@ static int emu8k_pcm_copy(struct snd_pcm_substream *subs, int i, err; count /= rec->voices; for (i = 0; i < rec->voices; i++) { - err = emu8k_transfer_block(emu, pos + rec->loop_start[i], buf, count); + err = emu8k_transfer_block(emu, + pos + rec->loop_start[i], + buf, count, in_kernel); if (err < 0) return err; - buf += count; + if (buf) + buf += count; } return 0; } else { - return emu8k_transfer_block(emu, pos + rec->loop_start[voice], src, count); - } -} - -/* make a channel block silence */ -static int emu8k_silence_block(struct snd_emu8000 *emu, int offset, int count) -{ - EMU8000_SMALW_WRITE(emu, offset); - while (count > 0) { - CHECK_SCHEDULER(); - EMU8000_SMLD_WRITE(emu, 0); - count--; - } - return 0; -} - -static int emu8k_pcm_silence(struct snd_pcm_substream *subs, - int voice, - snd_pcm_uframes_t pos, - snd_pcm_uframes_t count) -{ - struct snd_emu8k_pcm *rec = subs->runtime->private_data; - struct snd_emu8000 *emu = rec->emu; - - snd_emu8000_write_wait(emu, 1); - if (voice == -1 && rec->voices == 1) - voice = 0; - if (voice == -1) { - int err; - err = emu8k_silence_block(emu, pos + rec->loop_start[0], count / 2); - if (err < 0) - return err; - return emu8k_silence_block(emu, pos + rec->loop_start[1], count / 2); - } else { - return emu8k_silence_block(emu, pos + rec->loop_start[voice], count); + return emu8k_transfer_block(emu, pos + rec->loop_start[voice], + src, count, in_kernel); } }
@@ -510,7 +492,8 @@ static int emu8k_pcm_copy(struct snd_pcm_substream *subs, int voice, snd_pcm_uframes_t pos, void __user *src, - snd_pcm_uframes_t count) + snd_pcm_uframes_t count, + bool in_kernel) { struct snd_emu8k_pcm *rec = subs->runtime->private_data; struct snd_emu8000 *emu = rec->emu; @@ -524,39 +507,18 @@ static int emu8k_pcm_copy(struct snd_pcm_substream *subs, while (count-- > 0) { unsigned short sval; CHECK_SCHEDULER(); - if (get_user(sval, buf)) + if (get_val(&sval, buf, in_kernel)) return -EFAULT; EMU8000_SMLD_WRITE(emu, sval); - buf++; + if (buf) + buf++; if (rec->voices > 1) { CHECK_SCHEDULER(); - if (get_user(sval, buf)) + if (get_val(&sval, buf, in_kernel)) return -EFAULT; EMU8000_SMRD_WRITE(emu, sval); - buf++; - } - } - return 0; -} - -static int emu8k_pcm_silence(struct snd_pcm_substream *subs, - int voice, - snd_pcm_uframes_t pos, - snd_pcm_uframes_t count) -{ - struct snd_emu8k_pcm *rec = subs->runtime->private_data; - struct snd_emu8000 *emu = rec->emu; - - snd_emu8000_write_wait(emu, 1); - EMU8000_SMALW_WRITE(emu, rec->loop_start[0] + pos); - if (rec->voices > 1) - EMU8000_SMARW_WRITE(emu, rec->loop_start[1] + pos); - while (count-- > 0) { - CHECK_SCHEDULER(); - EMU8000_SMLD_WRITE(emu, 0); - if (rec->voices > 1) { - CHECK_SCHEDULER(); - EMU8000_SMRD_WRITE(emu, 0); + if (buf) + buf++; } } return 0; @@ -674,8 +636,7 @@ static struct snd_pcm_ops emu8k_pcm_ops = { .prepare = emu8k_pcm_prepare, .trigger = emu8k_pcm_trigger, .pointer = emu8k_pcm_pointer, - .copy = emu8k_pcm_copy, - .silence = emu8k_pcm_silence, + .copy_silence = emu8k_pcm_copy, };
Replace the copy and the silence ops with the new merged ops. A straightforward conversion with standard helper functions.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/sh/sh_dac_audio.c | 40 +++++++++------------------------------- 1 file changed, 9 insertions(+), 31 deletions(-)
diff --git a/sound/sh/sh_dac_audio.c b/sound/sh/sh_dac_audio.c index 461b310c7872..a4014a4548d0 100644 --- a/sound/sh/sh_dac_audio.c +++ b/sound/sh/sh_dac_audio.c @@ -185,7 +185,8 @@ static int snd_sh_dac_pcm_trigger(struct snd_pcm_substream *substream, int cmd) }
static int snd_sh_dac_pcm_copy(struct snd_pcm_substream *substream, int channel, - snd_pcm_uframes_t pos, void __user *src, snd_pcm_uframes_t count) + snd_pcm_uframes_t pos, void __user *src, + snd_pcm_uframes_t count, bool in_kernel) { /* channel is not used (interleaved data) */ struct snd_sh_dac *chip = snd_pcm_substream_chip(substream); @@ -199,34 +200,12 @@ static int snd_sh_dac_pcm_copy(struct snd_pcm_substream *substream, int channel, if (!count) return 0;
- memcpy_toio(chip->data_buffer + b_pos, src, b_count); - chip->buffer_end = chip->data_buffer + b_pos + b_count; - - if (chip->empty) { - chip->empty = 0; - dac_audio_start_timer(chip); - } - - return 0; -} - -static int snd_sh_dac_pcm_silence(struct snd_pcm_substream *substream, - int channel, snd_pcm_uframes_t pos, - snd_pcm_uframes_t count) -{ - /* channel is not used (interleaved data) */ - struct snd_sh_dac *chip = snd_pcm_substream_chip(substream); - struct snd_pcm_runtime *runtime = substream->runtime; - ssize_t b_count = frames_to_bytes(runtime , count); - ssize_t b_pos = frames_to_bytes(runtime , pos); - - if (count < 0) - return -EINVAL; - - if (!count) - return 0; - - memset_io(chip->data_buffer + b_pos, 0, b_count); + if (!src) + memset_io(chip->data_buffer + b_pos, 0, b_count); + else if (in_kernel) + memcpy_toio(chip->data_buffer + b_pos, (void *)src, b_count); + else if (copy_from_user_toio(chip->data_buffer + b_pos, src, b_count)) + return -EFAULT; chip->buffer_end = chip->data_buffer + b_pos + b_count;
if (chip->empty) { @@ -256,8 +235,7 @@ static struct snd_pcm_ops snd_sh_dac_pcm_ops = { .prepare = snd_sh_dac_pcm_prepare, .trigger = snd_sh_dac_pcm_trigger, .pointer = snd_sh_dac_pcm_pointer, - .copy = snd_sh_dac_pcm_copy, - .silence = snd_sh_dac_pcm_silence, + .copy_silence = snd_sh_dac_pcm_copy, .mmap = snd_pcm_lib_mmap_iomem, };
Replace the copy and the silence ops with the new merged ops. The silence is performed only CONFIG_SND_BF5XX_MMAP_SUPPORT is set (since copy_silence ops is set only with this config), so in bf5xx-ac97.c we have a bit tricky macro for a slight optimization.
Note that we don't need to take in_kernel into account on this architecture, so the conversion is easy otherwise.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/soc/blackfin/bf5xx-ac97-pcm.c | 6 ++--- sound/soc/blackfin/bf5xx-ac97.c | 18 ++++++++++----- sound/soc/blackfin/bf5xx-i2s-pcm.c | 46 ++++++++++++------------------------- 3 files changed, 30 insertions(+), 40 deletions(-)
diff --git a/sound/soc/blackfin/bf5xx-ac97-pcm.c b/sound/soc/blackfin/bf5xx-ac97-pcm.c index 02ad2606fa19..2fdffa7d376c 100644 --- a/sound/soc/blackfin/bf5xx-ac97-pcm.c +++ b/sound/soc/blackfin/bf5xx-ac97-pcm.c @@ -280,8 +280,8 @@ static int bf5xx_pcm_mmap(struct snd_pcm_substream *substream, } #else static int bf5xx_pcm_copy(struct snd_pcm_substream *substream, int channel, - snd_pcm_uframes_t pos, - void __user *buf, snd_pcm_uframes_t count) + snd_pcm_uframes_t pos, void __user *buf, + snd_pcm_uframes_t count, bool in_kernel) { struct snd_pcm_runtime *runtime = substream->runtime; unsigned int chan_mask = ac97_chan_mask[runtime->channels - 1]; @@ -309,7 +309,7 @@ static struct snd_pcm_ops bf5xx_pcm_ac97_ops = { #if defined(CONFIG_SND_BF5XX_MMAP_SUPPORT) .mmap = bf5xx_pcm_mmap, #else - .copy = bf5xx_pcm_copy, + .copy_silence = bf5xx_pcm_copy, #endif };
diff --git a/sound/soc/blackfin/bf5xx-ac97.c b/sound/soc/blackfin/bf5xx-ac97.c index a040cfe29fc0..d1f11d9ecb07 100644 --- a/sound/soc/blackfin/bf5xx-ac97.c +++ b/sound/soc/blackfin/bf5xx-ac97.c @@ -43,35 +43,41 @@
static struct sport_device *ac97_sport_handle;
+#ifdef CONFIG_SND_BF5XX_MMAP_SUPPORT +#define GET_VAL(src) (*src++) /* copy only */ +#else +#define GET_VAL(src) (src ? *src++ : 0) /* copy/silence */ +#endif + void bf5xx_pcm_to_ac97(struct ac97_frame *dst, const __u16 *src, size_t count, unsigned int chan_mask) { while (count--) { dst->ac97_tag = TAG_VALID; if (chan_mask & SP_FL) { - dst->ac97_pcm_r = *src++; + dst->ac97_pcm_r = GET_VAL(src); dst->ac97_tag |= TAG_PCM_RIGHT; } if (chan_mask & SP_FR) { - dst->ac97_pcm_l = *src++; + dst->ac97_pcm_l = GET_VAL(src); dst->ac97_tag |= TAG_PCM_LEFT;
} #if defined(CONFIG_SND_BF5XX_MULTICHAN_SUPPORT) if (chan_mask & SP_SR) { - dst->ac97_sl = *src++; + dst->ac97_sl = GET_VAL(src); dst->ac97_tag |= TAG_PCM_SL; } if (chan_mask & SP_SL) { - dst->ac97_sr = *src++; + dst->ac97_sr = GET_VAL(src); dst->ac97_tag |= TAG_PCM_SR; } if (chan_mask & SP_LFE) { - dst->ac97_lfe = *src++; + dst->ac97_lfe = GET_VAL(src); dst->ac97_tag |= TAG_PCM_LFE; } if (chan_mask & SP_FC) { - dst->ac97_center = *src++; + dst->ac97_center = GET_VAL(src); dst->ac97_tag |= TAG_PCM_CENTER; } #endif diff --git a/sound/soc/blackfin/bf5xx-i2s-pcm.c b/sound/soc/blackfin/bf5xx-i2s-pcm.c index 6cba211da32e..5686c29fb058 100644 --- a/sound/soc/blackfin/bf5xx-i2s-pcm.c +++ b/sound/soc/blackfin/bf5xx-i2s-pcm.c @@ -226,7 +226,8 @@ static int bf5xx_pcm_mmap(struct snd_pcm_substream *substream, }
static int bf5xx_pcm_copy(struct snd_pcm_substream *substream, int channel, - snd_pcm_uframes_t pos, void *buf, snd_pcm_uframes_t count) + snd_pcm_uframes_t pos, void *buf, + snd_pcm_uframes_t count, bool in_kernel) { struct snd_soc_pcm_runtime *rtd = substream->private_data; struct snd_pcm_runtime *runtime = substream->runtime; @@ -245,8 +246,14 @@ static int bf5xx_pcm_copy(struct snd_pcm_substream *substream, int channel,
while (count--) { for (i = 0; i < runtime->channels; i++) { - memcpy(dst + dma_data->map[i] * - sample_size, src, sample_size); + if (!buf) + memset(dst + dma_data->map[i] * + sample_size, 0, + sample_size); + else + memcpy(dst + dma_data->map[i] * + sample_size, src, + sample_size); src += sample_size; } dst += 8 * sample_size; @@ -276,34 +283,12 @@ static int bf5xx_pcm_copy(struct snd_pcm_substream *substream, int channel, dst = buf; }
- memcpy(dst, src, frames_to_bytes(runtime, count)); - } - - return 0; -} - -static int bf5xx_pcm_silence(struct snd_pcm_substream *substream, - int channel, snd_pcm_uframes_t pos, snd_pcm_uframes_t count) -{ - struct snd_soc_pcm_runtime *rtd = substream->private_data; - struct snd_pcm_runtime *runtime = substream->runtime; - unsigned int sample_size = runtime->sample_bits / 8; - void *buf = runtime->dma_area; - struct bf5xx_i2s_pcm_data *dma_data; - unsigned int offset, samples; - - dma_data = snd_soc_dai_get_dma_data(rtd->cpu_dai, substream); - - if (dma_data->tdm_mode) { - offset = pos * 8 * sample_size; - samples = count * 8; - } else { - offset = frames_to_bytes(runtime, pos); - samples = count * runtime->channels; + if (!buf) + memset(dst, 0, frames_to_bytes(runtime, count)); + else + memcpy(dst, src, frames_to_bytes(runtime, count)); }
- snd_pcm_format_set_silence(runtime->format, buf + offset, samples); - return 0; }
@@ -316,8 +301,7 @@ static struct snd_pcm_ops bf5xx_pcm_i2s_ops = { .trigger = bf5xx_pcm_trigger, .pointer = bf5xx_pcm_pointer, .mmap = bf5xx_pcm_mmap, - .copy = bf5xx_pcm_copy, - .silence = bf5xx_pcm_silence, + .copy_silence = bf5xx_pcm_copy, };
static int bf5xx_pcm_i2s_new(struct snd_soc_pcm_runtime *rtd)
Replace the copy and the silence ops with the new merged ops. It's a capture stream, thus no silence is needed.
Signed-off-by: Takashi Iwai tiwai@suse.de --- drivers/media/pci/solo6x10/solo6x10-g723.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/media/pci/solo6x10/solo6x10-g723.c b/drivers/media/pci/solo6x10/solo6x10-g723.c index 36e93540bb49..9bc4142481d8 100644 --- a/drivers/media/pci/solo6x10/solo6x10-g723.c +++ b/drivers/media/pci/solo6x10/solo6x10-g723.c @@ -225,7 +225,7 @@ static snd_pcm_uframes_t snd_solo_pcm_pointer(struct snd_pcm_substream *ss)
static int snd_solo_pcm_copy(struct snd_pcm_substream *ss, int channel, snd_pcm_uframes_t pos, void __user *dst, - snd_pcm_uframes_t count) + snd_pcm_uframes_t count, bool in_kernel) { struct solo_snd_pcm *solo_pcm = snd_pcm_substream_chip(ss); struct solo_dev *solo_dev = solo_pcm->solo_dev; @@ -242,10 +242,11 @@ static int snd_solo_pcm_copy(struct snd_pcm_substream *ss, int channel, if (err) return err;
- err = copy_to_user(dst + (i * G723_PERIOD_BYTES), + if (in_kernel) + memcpy((void *)dst + (i * G723_PERIOD_BYTES), + solo_pcm->g723_buf, G723_PERIOD_BYTES); + else if (copy_to_user(dst + (i * G723_PERIOD_BYTES), solo_pcm->g723_buf, G723_PERIOD_BYTES); - - if (err) return -EFAULT; }
@@ -261,7 +262,7 @@ static const struct snd_pcm_ops snd_solo_pcm_ops = { .prepare = snd_solo_pcm_prepare, .trigger = snd_solo_pcm_trigger, .pointer = snd_solo_pcm_pointer, - .copy = snd_solo_pcm_copy, + .copy_silence = snd_solo_pcm_copy, };
static int snd_solo_capture_volume_info(struct snd_kcontrol *kcontrol,
On Thu, 11 May 2017 23:09:23 +0200, Takashi Iwai wrote:
@@ -242,10 +242,11 @@ static int snd_solo_pcm_copy(struct snd_pcm_substream *ss, int channel, if (err) return err;
err = copy_to_user(dst + (i * G723_PERIOD_BYTES),
if (in_kernel)memcpy((void *)dst + (i * G723_PERIOD_BYTES),solo_pcm->g723_buf, G723_PERIOD_BYTES);else if (copy_to_user(dst + (i * G723_PERIOD_BYTES), solo_pcm->g723_buf, G723_PERIOD_BYTES);
Here is an obvious error, as kbuild bot spotted. I fixed in topic/kill-set_fs branch now.
Takashi
Now that all users of old copy and silence ops have been converted to the new copy_silence 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 065ea81904b0..67ba55c1062b 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 (*copy_silence)(struct snd_pcm_substream *substream, int channel, snd_pcm_uframes_t pos, void __user *buf, snd_pcm_uframes_t count, bool in_kernel); diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c index ed2002bfea6a..a2f796c8f39e 100644 --- a/sound/core/pcm_lib.c +++ b/sound/core/pcm_lib.c @@ -115,9 +115,6 @@ void snd_pcm_playback_silence(struct snd_pcm_substream *substream, snd_pcm_ufram err = substream->ops->copy_silence(substream, -1, ofs, NULL, transfer, false); 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); @@ -131,11 +128,6 @@ void snd_pcm_playback_silence(struct snd_pcm_substream *substream, snd_pcm_ufram c, ofs, NULL, transfer, false); 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) { @@ -2008,9 +2000,6 @@ static int snd_pcm_lib_write_transfer(struct snd_pcm_substream *substream, frames, false); 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))) @@ -2132,8 +2121,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_silence && !substream->ops->copy - && !runtime->dma_area)) + if (snd_BUG_ON(!substream->ops->copy_silence && !runtime->dma_area)) return -EINVAL; if (runtime->status->state == SNDRV_PCM_STATE_OPEN) return -EBADFD; @@ -2184,19 +2172,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; @@ -2249,9 +2224,6 @@ static int snd_pcm_lib_read_transfer(struct snd_pcm_substream *substream, frames, false); 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))) @@ -2411,14 +2383,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 3cfb9aa1203b..5d58d8434971 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -2744,8 +2744,6 @@ 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_silence = platform->driver->ops->copy_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; }
Along with the conversion to copy_silence PCM ops, the direct call of the callback from/to the kernel-space buffer is supported without set_fs() hack in all places. Now let's kill the last-standing set_fs() usage in the OSS layer and USB gadget driver.
This patch adds the in_kernel argument to each read/write PCM helper function, and the flag is passed through to the copy_silence ops.
Signed-off-by: Takashi Iwai tiwai@suse.de --- drivers/usb/gadget/function/u_uac1.c | 8 +-- include/sound/pcm.h | 11 ++-- sound/core/oss/pcm_oss.c | 62 ++++--------------- sound/core/pcm_compat.c | 10 +-- sound/core/pcm_lib.c | 114 ++++++++++++++++++++++++----------- sound/core/pcm_native.c | 20 +++--- 6 files changed, 118 insertions(+), 107 deletions(-)
diff --git a/drivers/usb/gadget/function/u_uac1.c b/drivers/usb/gadget/function/u_uac1.c index c78c84138a28..d73654407730 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,12 @@ 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_lib_write(snd->substream, (void __user *)buf, frames, + true); if (result != frames) { ERROR(card, "Playback error: %d\n", (int)result); - set_fs(old_fs); goto try_again; } - set_fs(old_fs);
return 0; } diff --git a/include/sound/pcm.h b/include/sound/pcm.h index 67ba55c1062b..06e6e0125f36 100644 --- a/include/sound/pcm.h +++ b/include/sound/pcm.h @@ -1085,13 +1085,16 @@ 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_write(struct snd_pcm_substream *substream, const void __user *buf, - snd_pcm_uframes_t frames); + snd_pcm_uframes_t frames, bool in_kernel); snd_pcm_sframes_t snd_pcm_lib_read(struct snd_pcm_substream *substream, - void __user *buf, snd_pcm_uframes_t frames); + void __user *buf, snd_pcm_uframes_t frames, + bool in_kernel); snd_pcm_sframes_t snd_pcm_lib_writev(struct snd_pcm_substream *substream, - void __user **bufs, snd_pcm_uframes_t frames); + void __user **bufs, snd_pcm_uframes_t frames, + bool in_kernel); snd_pcm_sframes_t snd_pcm_lib_readv(struct snd_pcm_substream *substream, - void __user **bufs, snd_pcm_uframes_t frames); + void __user **bufs, snd_pcm_uframes_t frames, + bool in_kernel);
extern const struct snd_pcm_hw_constraint_list snd_pcm_known_rates;
diff --git a/sound/core/oss/pcm_oss.c b/sound/core/oss/pcm_oss.c index 36baf962f9b0..8598acb79d07 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_write(substream, (void __force __user *)ptr, + 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_read(substream, (void __force __user *)ptr, + 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_writev(substream, (void __user **)bufs, + 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_readv(substream, (void __user **)bufs, + frames, in_kernel); if (ret != -EPIPE && ret != -ESTRPIPE) break; } @@ -1653,7 +1617,6 @@ static int snd_pcm_oss_sync(struct snd_pcm_oss_file *pcm_oss_file) 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; @@ -1662,14 +1625,15 @@ static int snd_pcm_oss_sync(struct snd_pcm_oss_file *pcm_oss_file) 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); + snd_pcm_lib_write(substream, + (void __force __user *)runtime->oss.buffer, + size1, true); } } 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); + snd_pcm_lib_writev(substream, buffers, size, + false); } } mutex_unlock(&runtime->oss.params_lock); diff --git a/sound/core/pcm_compat.c b/sound/core/pcm_compat.c index 8a0f8d51e95d..6f2a50a53d94 100644 --- a/sound/core/pcm_compat.c +++ b/sound/core/pcm_compat.c @@ -384,9 +384,11 @@ static int snd_pcm_ioctl_xferi_compat(struct snd_pcm_substream *substream, return -EFAULT;
if (dir == SNDRV_PCM_STREAM_PLAYBACK) - err = snd_pcm_lib_write(substream, compat_ptr(buf), frames); + err = snd_pcm_lib_write(substream, compat_ptr(buf), frames, + false); else - err = snd_pcm_lib_read(substream, compat_ptr(buf), frames); + err = snd_pcm_lib_read(substream, compat_ptr(buf), frames, + false); if (err < 0) return err; /* copy the result */ @@ -442,9 +444,9 @@ static int snd_pcm_ioctl_xfern_compat(struct snd_pcm_substream *substream, bufptr++; } if (dir == SNDRV_PCM_STREAM_PLAYBACK) - err = snd_pcm_lib_writev(substream, bufs, frames); + err = snd_pcm_lib_writev(substream, bufs, frames, false); else - err = snd_pcm_lib_readv(substream, bufs, frames); + err = snd_pcm_lib_readv(substream, bufs, frames, false); if (err >= 0) { if (put_user(err, &data32->result)) err = -EFAULT; diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c index a2f796c8f39e..2820205416d6 100644 --- a/sound/core/pcm_lib.c +++ b/sound/core/pcm_lib.c @@ -1990,19 +1990,26 @@ static int wait_for_avail(struct snd_pcm_substream *substream, static int snd_pcm_lib_write_transfer(struct snd_pcm_substream *substream, unsigned int hwoff, unsigned long data, unsigned int off, - snd_pcm_uframes_t frames) + snd_pcm_uframes_t frames, + bool in_kernel) { struct snd_pcm_runtime *runtime = substream->runtime; int err; char __user *buf = (char __user *) data + frames_to_bytes(runtime, off); if (substream->ops->copy_silence) { err = substream->ops->copy_silence(substream, -1, hwoff, buf, - frames, false); + frames, in_kernel); 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))) + char *hwbuf = runtime->dma_area + + frames_to_bytes(runtime, hwoff); + + if (in_kernel) + memcpy(hwbuf, (void *)buf, + frames_to_bytes(runtime, frames)); + else if (copy_from_user(hwbuf, buf, + frames_to_bytes(runtime, frames))) return -EFAULT; } return 0; @@ -2010,13 +2017,14 @@ static int snd_pcm_lib_write_transfer(struct snd_pcm_substream *substream,
typedef int (*transfer_f)(struct snd_pcm_substream *substream, unsigned int hwoff, unsigned long data, unsigned int off, - snd_pcm_uframes_t size); + snd_pcm_uframes_t size, bool in_kernel);
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) + transfer_f transfer, + bool in_kernel) { struct snd_pcm_runtime *runtime = substream->runtime; snd_pcm_uframes_t xfer = 0; @@ -2074,7 +2082,8 @@ static snd_pcm_sframes_t snd_pcm_lib_write1(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 = transfer(substream, appl_ofs, data, offset, frames, + in_kernel); snd_pcm_stream_lock_irq(substream); if (err < 0) goto _end_unlock; @@ -2128,7 +2137,9 @@ static int pcm_sanity_check(struct snd_pcm_substream *substream) return 0; }
-snd_pcm_sframes_t snd_pcm_lib_write(struct snd_pcm_substream *substream, const void __user *buf, snd_pcm_uframes_t size) +snd_pcm_sframes_t snd_pcm_lib_write(struct snd_pcm_substream *substream, + const void __user *buf, + snd_pcm_uframes_t size, bool in_kernel) { struct snd_pcm_runtime *runtime; int nonblock; @@ -2144,15 +2155,15 @@ snd_pcm_sframes_t snd_pcm_lib_write(struct snd_pcm_substream *substream, const v runtime->channels > 1) return -EINVAL; return snd_pcm_lib_write1(substream, (unsigned long)buf, size, nonblock, - snd_pcm_lib_write_transfer); + snd_pcm_lib_write_transfer, in_kernel); } - 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) + snd_pcm_uframes_t frames, + bool in_kernel) { struct snd_pcm_runtime *runtime = substream->runtime; int err; @@ -2168,7 +2179,8 @@ static int snd_pcm_lib_writev_transfer(struct snd_pcm_substream *substream, else buf = *bufs + samples_to_bytes(runtime, off); err = substream->ops->copy_silence(substream, c, hwoff, - buf, frames, false); + buf, frames, + in_kernel); if (err < 0) return err; } @@ -2176,12 +2188,21 @@ static int snd_pcm_lib_writev_transfer(struct snd_pcm_substream *substream, /* 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); + char *hwbuf = runtime->dma_area + (c * dma_csize) + + samples_to_bytes(runtime, hwoff); + + if (!*bufs) { + 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))) + char __user *buf = *bufs + + samples_to_bytes(runtime, off); + + if (in_kernel) + memcpy(hwbuf, (void *)buf, + samples_to_bytes(runtime, frames)); + else if (copy_from_user(hwbuf, buf, + samples_to_bytes(runtime, frames))) return -EFAULT; } } @@ -2191,7 +2212,8 @@ static int snd_pcm_lib_writev_transfer(struct snd_pcm_substream *substream,
snd_pcm_sframes_t snd_pcm_lib_writev(struct snd_pcm_substream *substream, void __user **bufs, - snd_pcm_uframes_t frames) + snd_pcm_uframes_t frames, + bool in_kernel) { struct snd_pcm_runtime *runtime; int nonblock; @@ -2206,27 +2228,34 @@ snd_pcm_sframes_t snd_pcm_lib_writev(struct snd_pcm_substream *substream, 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); + nonblock, snd_pcm_lib_writev_transfer, + in_kernel); } - EXPORT_SYMBOL(snd_pcm_lib_writev);
static int snd_pcm_lib_read_transfer(struct snd_pcm_substream *substream, unsigned int hwoff, unsigned long data, unsigned int off, - snd_pcm_uframes_t frames) + snd_pcm_uframes_t frames, + bool in_kernel) { struct snd_pcm_runtime *runtime = substream->runtime; int err; char __user *buf = (char __user *) data + frames_to_bytes(runtime, off); if (substream->ops->copy_silence) { err = substream->ops->copy_silence(substream, -1, hwoff, buf, - frames, false); + frames, in_kernel); 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))) + char *hwbuf = runtime->dma_area + + frames_to_bytes(runtime, hwoff); + + if (in_kernel) + memcpy((void *)buf, hwbuf, + frames_to_bytes(runtime, frames)); + else if (copy_to_user(buf, hwbuf, + frames_to_bytes(runtime, frames))) return -EFAULT; } return 0; @@ -2236,7 +2265,8 @@ 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) + transfer_f transfer, + bool in_kernel) { struct snd_pcm_runtime *runtime = substream->runtime; snd_pcm_uframes_t xfer = 0; @@ -2308,7 +2338,8 @@ static snd_pcm_sframes_t snd_pcm_lib_read1(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 = transfer(substream, appl_ofs, data, offset, frames, + in_kernel); snd_pcm_stream_lock_irq(substream); if (err < 0) goto _end_unlock; @@ -2342,7 +2373,9 @@ static snd_pcm_sframes_t snd_pcm_lib_read1(struct snd_pcm_substream *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) +snd_pcm_sframes_t snd_pcm_lib_read(struct snd_pcm_substream *substream, + void __user *buf, snd_pcm_uframes_t size, + bool in_kernel) { struct snd_pcm_runtime *runtime; int nonblock; @@ -2355,7 +2388,8 @@ snd_pcm_sframes_t snd_pcm_lib_read(struct snd_pcm_substream *substream, void __u 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); + return snd_pcm_lib_read1(substream, (unsigned long)buf, size, nonblock, + snd_pcm_lib_read_transfer, in_kernel); }
EXPORT_SYMBOL(snd_pcm_lib_read); @@ -2363,7 +2397,8 @@ 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) + snd_pcm_uframes_t frames, + bool in_kernel) { struct snd_pcm_runtime *runtime = substream->runtime; int err; @@ -2379,19 +2414,25 @@ static int snd_pcm_lib_readv_transfer(struct snd_pcm_substream *substream, continue; buf = *bufs + samples_to_bytes(runtime, off); err = substream->ops->copy_silence(substream, c, hwoff, - buf, frames, false); + buf, frames, + in_kernel); 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) + if (!*bufs) continue;
- hwbuf = runtime->dma_area + (c * dma_csize) + samples_to_bytes(runtime, hwoff); + 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))) + if (in_kernel) + memcpy((void *)buf, hwbuf, + samples_to_bytes(runtime, frames)); + else if (copy_to_user(buf, hwbuf, + samples_to_bytes(runtime, frames))) return -EFAULT; } } @@ -2400,7 +2441,7 @@ static int snd_pcm_lib_readv_transfer(struct snd_pcm_substream *substream,
snd_pcm_sframes_t snd_pcm_lib_readv(struct snd_pcm_substream *substream, void __user **bufs, - snd_pcm_uframes_t frames) + snd_pcm_uframes_t frames, bool in_kernel) { struct snd_pcm_runtime *runtime; int nonblock; @@ -2416,9 +2457,10 @@ snd_pcm_sframes_t snd_pcm_lib_readv(struct snd_pcm_substream *substream, 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); + return snd_pcm_lib_read1(substream, (unsigned long)bufs, frames, + nonblock, snd_pcm_lib_readv_transfer, + in_kernel); } - EXPORT_SYMBOL(snd_pcm_lib_readv);
/* diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index f9bd56958cad..6397fd1e414d 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -2839,7 +2839,8 @@ static int snd_pcm_playback_ioctl1(struct file *file, return -EFAULT; if (copy_from_user(&xferi, _xferi, sizeof(xferi))) return -EFAULT; - result = snd_pcm_lib_write(substream, xferi.buf, xferi.frames); + result = snd_pcm_lib_write(substream, xferi.buf, xferi.frames, + false); __put_user(result, &_xferi->result); return result < 0 ? result : 0; } @@ -2863,7 +2864,8 @@ static int snd_pcm_playback_ioctl1(struct file *file, sizeof(void *) * runtime->channels); if (IS_ERR(bufs)) return PTR_ERR(bufs); - result = snd_pcm_lib_writev(substream, bufs, xfern.frames); + result = snd_pcm_lib_writev(substream, bufs, xfern.frames, + false); kfree(bufs); __put_user(result, &_xfern->result); return result < 0 ? result : 0; @@ -2919,7 +2921,8 @@ static int snd_pcm_capture_ioctl1(struct file *file, return -EFAULT; if (copy_from_user(&xferi, _xferi, sizeof(xferi))) return -EFAULT; - result = snd_pcm_lib_read(substream, xferi.buf, xferi.frames); + result = snd_pcm_lib_read(substream, xferi.buf, xferi.frames, + false); __put_user(result, &_xferi->result); return result < 0 ? result : 0; } @@ -2943,7 +2946,8 @@ static int snd_pcm_capture_ioctl1(struct file *file, sizeof(void *) * runtime->channels); if (IS_ERR(bufs)) return PTR_ERR(bufs); - result = snd_pcm_lib_readv(substream, bufs, xfern.frames); + result = snd_pcm_lib_readv(substream, bufs, xfern.frames, + false); kfree(bufs); __put_user(result, &_xfern->result); return result < 0 ? result : 0; @@ -3075,7 +3079,7 @@ static ssize_t snd_pcm_read(struct file *file, char __user *buf, size_t count, if (!frame_aligned(runtime, count)) return -EINVAL; count = bytes_to_frames(runtime, count); - result = snd_pcm_lib_read(substream, buf, count); + result = snd_pcm_lib_read(substream, buf, count, false); if (result > 0) result = frames_to_bytes(runtime, result); return result; @@ -3099,7 +3103,7 @@ static ssize_t snd_pcm_write(struct file *file, const char __user *buf, if (!frame_aligned(runtime, count)) return -EINVAL; count = bytes_to_frames(runtime, count); - result = snd_pcm_lib_write(substream, buf, count); + result = snd_pcm_lib_write(substream, buf, count, false); if (result > 0) result = frames_to_bytes(runtime, result); return result; @@ -3134,7 +3138,7 @@ static ssize_t snd_pcm_readv(struct kiocb *iocb, struct iov_iter *to) return -ENOMEM; for (i = 0; i < to->nr_segs; ++i) bufs[i] = to->iov[i].iov_base; - result = snd_pcm_lib_readv(substream, bufs, frames); + result = snd_pcm_lib_readv(substream, bufs, frames, false); if (result > 0) result = frames_to_bytes(runtime, result); kfree(bufs); @@ -3169,7 +3173,7 @@ static ssize_t snd_pcm_writev(struct kiocb *iocb, struct iov_iter *from) return -ENOMEM; for (i = 0; i < from->nr_segs; ++i) bufs[i] = from->iov[i].iov_base; - result = snd_pcm_lib_writev(substream, bufs, frames); + result = snd_pcm_lib_writev(substream, bufs, frames, false); if (result > 0) result = frames_to_bytes(runtime, result); kfree(bufs);
Hi,
On May 12 2017 06:08, Takashi Iwai wrote:
this is a slightly lengthy patchset, basically targeted for 4.13, thus it's still in RFC. The main purpose of this patchset is to get rid of get_fs() / set_fs() usages in ALSA codes.
The patchset starts from a patch that looks fairly irrelevant with the goal (the conversion of HD-audio bind-mute code), but it's a step for the long trail. The biggest change in the patchset is the conversion of copy & silence PCM ops into the new copy_silence ops. It helped for reducing the code size and for handling the in-kernel buffer copies.
The current patches are found in topic/kill-set_fs branch of my sound.git tree.
thanks,
Takashi
===
Takashi Iwai (26): ALSA: hda - Simplify bound-beep mute control for ALC268 ALSA: hda - Move bind-mixer switch codes to generic parser ALSA: hda - Remove the generic bind ctl helpers ALSA: hda - Remove the use of set_fs() ALSA: hda - Fix a typo in comment ALSA: hda - Remove superfluous header inclusions
Patch 01-06 look good to me, and they can be applied regardless of the rest as a refactoring of PCI HDA driver. I have had concern about usage of TLV feature with the set_fs() for a bit time. I note that patch 06 includes a fixup to a commit 59ed1eade1d6 ("ALSA: hda - Move codec suspend/resume to codec driver").
ALSA: opl3: Kill unused set_fs()
This is also good to me. These lines were forgot to remove at a commit 224a033252bb ("[ALSA] opl3 - Use hwdep for patch loading").
ALSA: emu10k1: Get rid of set_fs() usage
Looks good to me, but I'm not so good for emu10k1 driver and its hwdep functionality. I think existent code is basically designed to run on process contexts of the hwdep ioctl and not expected on device probing.
ALSA: pcm: Remove set_fs() in PCM core code ALSA: pcm: Introduce copy_silence PCM ops ALSA: Update document about copy_silence PCM ops ALSA: dummy: Convert to copy_silence ops ALSA: es1938: Convert to copy_silence ops ALSA: korg1212: Convert to copy_silence ops ALSA: nm256: Convert to copy_silence ops ALSA: rme32: Convert to copy_silence ops ALSA: rme96: Convert to copy_silence ops ALSA: rme9652: Convert to copy_silence ops ALSA: hdsp: Convert to copy_silence ops ALSA: gus: Convert to copy_silence ops ALSA: sb: Convert to copy_silence ops ALSA: sh: Convert to copy_silence ops ASoC: blackfin: Convert to copy_silence ops [media] solo6x10: Convert to copy_silence ops ALSA: pcm: Drop the old copy and silence ops ALSA: pcm: Kill set_fs() usage in OSS layer and USB gadget
I prefer the idea to unify 'struct snd_pcm_ops.copy' and 'struct snd_pcm_ops.silence', but we should consider better name for it so that developers can easily get its intension; e.g. just naming 'copy_frames'.
About the way to copy PCM frames between several regions, I have a concern about disadvantage due to adding extra branching because the write/read operations repeatedly during runtime of PCM substream for devices which doesn't support memory mapping of DMA-dedicated buffer.
When letting me assume 'proxy' driver such ad OSS compatibility layer and UAC1 gadget driver (as a peripheral), below diagram show copy operation of PCM frames between memory regions. I also illustrate an use case for applications.
applications (__user *) <-----+ v usual driver (__kernel */__iomem *) <-----> hardware proxy drivers ^ (__kernel *) <-----+
The case to handle data for the proxy drivers is not general use cases in ALSA PCM core and drivers. On the other hand, this patchset adds extra branching in runtime of PCM substream. Additionally, increase of codes in each function is not good in an aspect of hit rate of i-cache. Even if practically these codes are used for minor drivers which doesn't support memory mapping of DMA-dedicated buffer to user processes, it's better to care of such efficiencies somehow in a point of applications. Furthermore, such efficiency is good to the proxy drivers because currently they don't utilize 'struct snd_pcm_runtime.dma_area' directly. (If allowing the proxy drivers to have full PCM functionality, the other ioctl commands should be available via snd_pcm_kernel_ioctl().)
This is a callgraph for relevant functions:
(sound/core/pcm_lib.c) snd_pcm_lib_read() snd_pcm_lib_readv() ->snd_pcm_lib_read1() ->snd_pcm_lib_read_transfer() ->struct snd_pcm_ops.copy() ->copy_to_user() ->snd_pcm_lib_readv_transfer() for(**bufs) ->struct snd_pcm_ops.copy() ->copy_to_user()
(sound/core/pcm_lib.c) snd_pcm_lib_write() snd_pcm_lib_writev() ->snd_pcm_lib_write1() ->snd_pcm_lib_write_transfer() ->struct snd_pcm_ops.copy() ->copy_from_user() ->snd_pcm_lib_writev_transfer() for(**bufs) ->struct snd_pcm_ops.copy() ->copy_from_user()
Some helper functions can be assumed to perform copy operation between below three cases: - struct snd_pcm_ops.copy() - between __kernel * and __user * - between __kernel * and __kernel *
When adding a member for a callback function into 'struct snd_pcm_runtime' and assigning one of the helper functions in a call of HW_PARAMS or PREPARE, then call it from each call of the exported functions, needless branching can be removed in runtime of PCM substream.
In the above reasons, I think it better to apply refactoring at first, then get rid of set_fs() fro the 'proxy' drivers. All of ALSA drivers can potentially get some merit from the refactoring because the drivers support simple READ_FRAME/WRITE_FRAME ioctl commands as a default.
Regards
Takashi Sakamoto
On Sun, 14 May 2017 10:23:01 +0200, Takashi Sakamoto wrote:
Hi,
On May 12 2017 06:08, Takashi Iwai wrote:
this is a slightly lengthy patchset, basically targeted for 4.13, thus it's still in RFC. The main purpose of this patchset is to get rid of get_fs() / set_fs() usages in ALSA codes.
The patchset starts from a patch that looks fairly irrelevant with the goal (the conversion of HD-audio bind-mute code), but it's a step for the long trail. The biggest change in the patchset is the conversion of copy & silence PCM ops into the new copy_silence ops. It helped for reducing the code size and for handling the in-kernel buffer copies.
The current patches are found in topic/kill-set_fs branch of my sound.git tree.
thanks,
Takashi
===
Takashi Iwai (26): ALSA: hda - Simplify bound-beep mute control for ALC268 ALSA: hda - Move bind-mixer switch codes to generic parser ALSA: hda - Remove the generic bind ctl helpers ALSA: hda - Remove the use of set_fs() ALSA: hda - Fix a typo in comment ALSA: hda - Remove superfluous header inclusions
Patch 01-06 look good to me, and they can be applied regardless of the rest as a refactoring of PCI HDA driver. I have had concern about usage of TLV feature with the set_fs() for a bit time. I note that patch 06 includes a fixup to a commit 59ed1eade1d6 ("ALSA: hda - Move codec suspend/resume to codec driver").
Yes, the patch 06 is merely a cleanup.
ALSA: opl3: Kill unused set_fs()
This is also good to me. These lines were forgot to remove at a commit 224a033252bb ("[ALSA] opl3 - Use hwdep for patch loading").
ALSA: emu10k1: Get rid of set_fs() usage
Looks good to me, but I'm not so good for emu10k1 driver and its hwdep functionality. I think existent code is basically designed to run on process contexts of the hwdep ioctl and not expected on device probing.
ALSA: pcm: Remove set_fs() in PCM core code ALSA: pcm: Introduce copy_silence PCM ops ALSA: Update document about copy_silence PCM ops ALSA: dummy: Convert to copy_silence ops ALSA: es1938: Convert to copy_silence ops ALSA: korg1212: Convert to copy_silence ops ALSA: nm256: Convert to copy_silence ops ALSA: rme32: Convert to copy_silence ops ALSA: rme96: Convert to copy_silence ops ALSA: rme9652: Convert to copy_silence ops ALSA: hdsp: Convert to copy_silence ops ALSA: gus: Convert to copy_silence ops ALSA: sb: Convert to copy_silence ops ALSA: sh: Convert to copy_silence ops ASoC: blackfin: Convert to copy_silence ops [media] solo6x10: Convert to copy_silence ops ALSA: pcm: Drop the old copy and silence ops ALSA: pcm: Kill set_fs() usage in OSS layer and USB gadget
I prefer the idea to unify 'struct snd_pcm_ops.copy' and 'struct snd_pcm_ops.silence', but we should consider better name for it so that developers can easily get its intension; e.g. just naming 'copy_frames'.
Well, the callback doesn't only copy but also silence the buffer. Your new name misses that.
About the way to copy PCM frames between several regions, I have a concern about disadvantage due to adding extra branching because the write/read operations repeatedly during runtime of PCM substream for devices which doesn't support memory mapping of DMA-dedicated buffer.
When letting me assume 'proxy' driver such ad OSS compatibility layer and UAC1 gadget driver (as a peripheral), below diagram show copy operation of PCM frames between memory regions. I also illustrate an use case for applications.
applications (__user *) <-----+ v usual driver (__kernel */__iomem *) <-----> hardware proxy drivers ^ (__kernel *) <-----+
The case to handle data for the proxy drivers is not general use cases in ALSA PCM core and drivers. On the other hand, this patchset adds extra branching in runtime of PCM substream. Additionally, increase of codes in each function is not good in an aspect of hit rate of i-cache. Even if practically these codes are used for minor drivers which doesn't support memory mapping of DMA-dedicated buffer to user processes, it's better to care of such efficiencies somehow in a point of applications. Furthermore, such efficiency is good to the proxy drivers because currently they don't utilize 'struct snd_pcm_runtime.dma_area' directly. (If allowing the proxy drivers to have full PCM functionality, the other ioctl commands should be available via snd_pcm_kernel_ioctl().)
This is a callgraph for relevant functions:
(sound/core/pcm_lib.c) snd_pcm_lib_read() snd_pcm_lib_readv() ->snd_pcm_lib_read1() ->snd_pcm_lib_read_transfer() ->struct snd_pcm_ops.copy() ->copy_to_user() ->snd_pcm_lib_readv_transfer() for(**bufs) ->struct snd_pcm_ops.copy() ->copy_to_user()
(sound/core/pcm_lib.c) snd_pcm_lib_write() snd_pcm_lib_writev() ->snd_pcm_lib_write1() ->snd_pcm_lib_write_transfer() ->struct snd_pcm_ops.copy() ->copy_from_user() ->snd_pcm_lib_writev_transfer() for(**bufs) ->struct snd_pcm_ops.copy() ->copy_from_user()
Some helper functions can be assumed to perform copy operation between below three cases:
- struct snd_pcm_ops.copy()
- between __kernel * and __user *
- between __kernel * and __kernel *
When adding a member for a callback function into 'struct snd_pcm_runtime' and assigning one of the helper functions in a call of HW_PARAMS or PREPARE, then call it from each call of the exported functions, needless branching can be removed in runtime of PCM substream.
In the above reasons, I think it better to apply refactoring at first, then get rid of set_fs() fro the 'proxy' drivers. All of ALSA drivers can potentially get some merit from the refactoring because the drivers support simple READ_FRAME/WRITE_FRAME ioctl commands as a default.
I don't understand yet clearly how your proxy driver is supposed to work. But at this moment, the higher priority is to get rid of set_fs() in the time frame for 4.13 kernel. For that purpose, my approach is rather straightforward. There is no big code flow change, and all changes can be applied gradually, and less risk of regressions.
I don't object against the redesign of the PCM transfer if it really works better. But please let the code speaks.
thanks,
Takashi
On May 15 2017 17:25, Takashi Iwai wrote:
I don't understand yet clearly how your proxy driver is supposed to work. But at this moment, the higher priority is to get rid of set_fs() in the time frame for 4.13 kernel. For that purpose, my approach is rather straightforward. There is no big code flow change, and all changes can be applied gradually, and less risk of regressions.
I don't object against the redesign of the PCM transfer if it really works better. But please let the code speaks.
This is it. http://mailman.alsa-project.org/pipermail/alsa-devel/2017-May/120984.html
Regards
Takashi Sakamoto
participants (3)
-
René Rebe -
Takashi Iwai -
Takashi Sakamoto