* [PATCH RFC 00/26] Kill set_fs() in ALSA codes
@ 2017-05-11 21:08 Takashi Iwai
2017-05-11 21:09 ` [PATCH RFC 01/26] ALSA: hda - Simplify bound-beep mute control for ALC268 Takashi Iwai
` (26 more replies)
0 siblings, 27 replies; 41+ messages in thread
From: Takashi Iwai @ 2017-05-11 21:08 UTC (permalink / raw)
To: alsa-devel
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(-)
--
2.12.2
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH RFC 01/26] ALSA: hda - Simplify bound-beep mute control for ALC268
2017-05-11 21:08 [PATCH RFC 00/26] Kill set_fs() in ALSA codes Takashi Iwai
@ 2017-05-11 21:09 ` Takashi Iwai
2017-05-11 21:09 ` [PATCH RFC 02/26] ALSA: hda - Move bind-mixer switch codes to generic parser Takashi Iwai
` (25 subsequent siblings)
26 siblings, 0 replies; 41+ messages in thread
From: Takashi Iwai @ 2017-05-11 21:09 UTC (permalink / raw)
To: alsa-devel
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)
+ },
{ }
};
--
2.12.2
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH RFC 02/26] ALSA: hda - Move bind-mixer switch codes to generic parser
2017-05-11 21:08 [PATCH RFC 00/26] Kill set_fs() in ALSA codes Takashi Iwai
2017-05-11 21:09 ` [PATCH RFC 01/26] ALSA: hda - Simplify bound-beep mute control for ALC268 Takashi Iwai
@ 2017-05-11 21:09 ` Takashi Iwai
2017-05-11 21:09 ` [PATCH RFC 03/26] ALSA: hda - Remove the generic bind ctl helpers Takashi Iwai
` (24 subsequent siblings)
26 siblings, 0 replies; 41+ messages in thread
From: Takashi Iwai @ 2017-05-11 21:09 UTC (permalink / raw)
To: alsa-devel
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;
--
2.12.2
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH RFC 03/26] ALSA: hda - Remove the generic bind ctl helpers
2017-05-11 21:08 [PATCH RFC 00/26] Kill set_fs() in ALSA codes Takashi Iwai
2017-05-11 21:09 ` [PATCH RFC 01/26] ALSA: hda - Simplify bound-beep mute control for ALC268 Takashi Iwai
2017-05-11 21:09 ` [PATCH RFC 02/26] ALSA: hda - Move bind-mixer switch codes to generic parser Takashi Iwai
@ 2017-05-11 21:09 ` Takashi Iwai
2017-05-11 21:09 ` [PATCH RFC 04/26] ALSA: hda - Remove the use of set_fs() Takashi Iwai
` (23 subsequent siblings)
26 siblings, 0 replies; 41+ messages in thread
From: Takashi Iwai @ 2017-05-11 21:09 UTC (permalink / raw)
To: alsa-devel
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
*/
--
2.12.2
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH RFC 04/26] ALSA: hda - Remove the use of set_fs()
2017-05-11 21:08 [PATCH RFC 00/26] Kill set_fs() in ALSA codes Takashi Iwai
` (2 preceding siblings ...)
2017-05-11 21:09 ` [PATCH RFC 03/26] ALSA: hda - Remove the generic bind ctl helpers Takashi Iwai
@ 2017-05-11 21:09 ` Takashi Iwai
2017-05-11 21:09 ` [PATCH RFC 05/26] ALSA: hda - Fix a typo in comment Takashi Iwai
` (22 subsequent siblings)
26 siblings, 0 replies; 41+ messages in thread
From: Takashi Iwai @ 2017-05-11 21:09 UTC (permalink / raw)
To: alsa-devel
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) {
--
2.12.2
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH RFC 05/26] ALSA: hda - Fix a typo in comment
2017-05-11 21:08 [PATCH RFC 00/26] Kill set_fs() in ALSA codes Takashi Iwai
` (3 preceding siblings ...)
2017-05-11 21:09 ` [PATCH RFC 04/26] ALSA: hda - Remove the use of set_fs() Takashi Iwai
@ 2017-05-11 21:09 ` Takashi Iwai
2017-05-11 21:09 ` [PATCH RFC 06/26] ALSA: hda - Remove superfluous header inclusions Takashi Iwai
` (21 subsequent siblings)
26 siblings, 0 replies; 41+ messages in thread
From: Takashi Iwai @ 2017-05-11 21:09 UTC (permalink / raw)
To: alsa-devel
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
--
2.12.2
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH RFC 06/26] ALSA: hda - Remove superfluous header inclusions
2017-05-11 21:08 [PATCH RFC 00/26] Kill set_fs() in ALSA codes Takashi Iwai
` (4 preceding siblings ...)
2017-05-11 21:09 ` [PATCH RFC 05/26] ALSA: hda - Fix a typo in comment Takashi Iwai
@ 2017-05-11 21:09 ` Takashi Iwai
2017-05-11 21:09 ` [PATCH RFC 07/26] ALSA: opl3: Kill unused set_fs() Takashi Iwai
` (20 subsequent siblings)
26 siblings, 0 replies; 41+ messages in thread
From: Takashi Iwai @ 2017-05-11 21:09 UTC (permalink / raw)
To: alsa-devel
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>
--
2.12.2
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH RFC 07/26] ALSA: opl3: Kill unused set_fs()
2017-05-11 21:08 [PATCH RFC 00/26] Kill set_fs() in ALSA codes Takashi Iwai
` (5 preceding siblings ...)
2017-05-11 21:09 ` [PATCH RFC 06/26] ALSA: hda - Remove superfluous header inclusions Takashi Iwai
@ 2017-05-11 21:09 ` Takashi Iwai
2017-05-11 21:09 ` [PATCH RFC 08/26] ALSA: emu10k1: Get rid of set_fs() usage Takashi Iwai
` (19 subsequent siblings)
26 siblings, 0 replies; 41+ messages in thread
From: Takashi Iwai @ 2017-05-11 21:09 UTC (permalink / raw)
To: alsa-devel
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;
--
2.12.2
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH RFC 08/26] ALSA: emu10k1: Get rid of set_fs() usage
2017-05-11 21:08 [PATCH RFC 00/26] Kill set_fs() in ALSA codes Takashi Iwai
` (6 preceding siblings ...)
2017-05-11 21:09 ` [PATCH RFC 07/26] ALSA: opl3: Kill unused set_fs() Takashi Iwai
@ 2017-05-11 21:09 ` Takashi Iwai
2017-05-11 21:09 ` [PATCH RFC 09/26] ALSA: pcm: Remove set_fs() in PCM core code Takashi Iwai
` (18 subsequent siblings)
26 siblings, 0 replies; 41+ messages in thread
From: Takashi Iwai @ 2017-05-11 21:09 UTC (permalink / raw)
To: alsa-devel
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:
--
2.12.2
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH RFC 09/26] ALSA: pcm: Remove set_fs() in PCM core code
2017-05-11 21:08 [PATCH RFC 00/26] Kill set_fs() in ALSA codes Takashi Iwai
` (7 preceding siblings ...)
2017-05-11 21:09 ` [PATCH RFC 08/26] ALSA: emu10k1: Get rid of set_fs() usage Takashi Iwai
@ 2017-05-11 21:09 ` Takashi Iwai
2017-05-11 21:09 ` [PATCH RFC 10/26] ALSA: pcm: Introduce copy_silence PCM ops Takashi Iwai
` (17 subsequent siblings)
26 siblings, 0 replies; 41+ messages in thread
From: Takashi Iwai @ 2017-05-11 21:09 UTC (permalink / raw)
To: alsa-devel
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,
--
2.12.2
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH RFC 10/26] ALSA: pcm: Introduce copy_silence PCM ops
2017-05-11 21:08 [PATCH RFC 00/26] Kill set_fs() in ALSA codes Takashi Iwai
` (8 preceding siblings ...)
2017-05-11 21:09 ` [PATCH RFC 09/26] ALSA: pcm: Remove set_fs() in PCM core code Takashi Iwai
@ 2017-05-11 21:09 ` Takashi Iwai
2017-05-11 21:09 ` [PATCH RFC 11/26] ALSA: Update document about " Takashi Iwai
` (16 subsequent siblings)
26 siblings, 0 replies; 41+ messages in thread
From: Takashi Iwai @ 2017-05-11 21:09 UTC (permalink / raw)
To: alsa-devel
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;
--
2.12.2
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH RFC 11/26] ALSA: Update document about copy_silence PCM ops
2017-05-11 21:08 [PATCH RFC 00/26] Kill set_fs() in ALSA codes Takashi Iwai
` (9 preceding siblings ...)
2017-05-11 21:09 ` [PATCH RFC 10/26] ALSA: pcm: Introduce copy_silence PCM ops Takashi Iwai
@ 2017-05-11 21:09 ` Takashi Iwai
2017-05-11 21:09 ` [PATCH RFC 12/26] ALSA: dummy: Convert to copy_silence ops Takashi Iwai
` (15 subsequent siblings)
26 siblings, 0 replies; 41+ messages in thread
From: Takashi Iwai @ 2017-05-11 21:09 UTC (permalink / raw)
To: alsa-devel
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
----------------------
--
2.12.2
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH RFC 12/26] ALSA: dummy: Convert to copy_silence ops
2017-05-11 21:08 [PATCH RFC 00/26] Kill set_fs() in ALSA codes Takashi Iwai
` (10 preceding siblings ...)
2017-05-11 21:09 ` [PATCH RFC 11/26] ALSA: Update document about " Takashi Iwai
@ 2017-05-11 21:09 ` Takashi Iwai
2017-05-11 21:09 ` [PATCH RFC 13/26] ALSA: es1938: " Takashi Iwai
` (14 subsequent siblings)
26 siblings, 0 replies; 41+ messages in thread
From: Takashi Iwai @ 2017-05-11 21:09 UTC (permalink / raw)
To: alsa-devel
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,
};
--
2.12.2
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH RFC 13/26] ALSA: es1938: Convert to copy_silence ops
2017-05-11 21:08 [PATCH RFC 00/26] Kill set_fs() in ALSA codes Takashi Iwai
` (11 preceding siblings ...)
2017-05-11 21:09 ` [PATCH RFC 12/26] ALSA: dummy: Convert to copy_silence ops Takashi Iwai
@ 2017-05-11 21:09 ` Takashi Iwai
2017-05-11 21:09 ` [PATCH RFC 14/26] ALSA: korg1212: " Takashi Iwai
` (13 subsequent siblings)
26 siblings, 0 replies; 41+ messages in thread
From: Takashi Iwai @ 2017-05-11 21:09 UTC (permalink / raw)
To: alsa-devel
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)
--
2.12.2
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH RFC 14/26] ALSA: korg1212: Convert to copy_silence ops
2017-05-11 21:08 [PATCH RFC 00/26] Kill set_fs() in ALSA codes Takashi Iwai
` (12 preceding siblings ...)
2017-05-11 21:09 ` [PATCH RFC 13/26] ALSA: es1938: " Takashi Iwai
@ 2017-05-11 21:09 ` Takashi Iwai
2017-05-11 21:09 ` [PATCH RFC 15/26] ALSA: nm256: " Takashi Iwai
` (12 subsequent siblings)
26 siblings, 0 replies; 41+ messages in thread
From: Takashi Iwai @ 2017-05-11 21:09 UTC (permalink / raw)
To: alsa-devel
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,
};
/*
--
2.12.2
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH RFC 15/26] ALSA: nm256: Convert to copy_silence ops
2017-05-11 21:08 [PATCH RFC 00/26] Kill set_fs() in ALSA codes Takashi Iwai
` (13 preceding siblings ...)
2017-05-11 21:09 ` [PATCH RFC 14/26] ALSA: korg1212: " Takashi Iwai
@ 2017-05-11 21:09 ` Takashi Iwai
2017-05-11 21:09 ` [PATCH RFC 16/26] ALSA: rme32: " Takashi Iwai
` (11 subsequent siblings)
26 siblings, 0 replies; 41+ messages in thread
From: Takashi Iwai @ 2017-05-11 21:09 UTC (permalink / raw)
To: alsa-devel
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,
};
--
2.12.2
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH RFC 16/26] ALSA: rme32: Convert to copy_silence ops
2017-05-11 21:08 [PATCH RFC 00/26] Kill set_fs() in ALSA codes Takashi Iwai
` (14 preceding siblings ...)
2017-05-11 21:09 ` [PATCH RFC 15/26] ALSA: nm256: " Takashi Iwai
@ 2017-05-11 21:09 ` Takashi Iwai
2018-07-18 10:22 ` René Rebe
2017-05-11 21:09 ` [PATCH RFC 17/26] ALSA: rme96: Convert to copy_silence ops Takashi Iwai
` (10 subsequent siblings)
26 siblings, 1 reply; 41+ messages in thread
From: Takashi Iwai @ 2017-05-11 21:09 UTC (permalink / raw)
To: alsa-devel
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,
};
--
2.12.2
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH RFC 17/26] ALSA: rme96: Convert to copy_silence ops
2017-05-11 21:08 [PATCH RFC 00/26] Kill set_fs() in ALSA codes Takashi Iwai
` (15 preceding siblings ...)
2017-05-11 21:09 ` [PATCH RFC 16/26] ALSA: rme32: " Takashi Iwai
@ 2017-05-11 21:09 ` Takashi Iwai
2017-05-11 21:09 ` [PATCH RFC 18/26] ALSA: rme9652: " Takashi Iwai
` (9 subsequent siblings)
26 siblings, 0 replies; 41+ messages in thread
From: Takashi Iwai @ 2017-05-11 21:09 UTC (permalink / raw)
To: alsa-devel
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,
};
--
2.12.2
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH RFC 18/26] ALSA: rme9652: Convert to copy_silence ops
2017-05-11 21:08 [PATCH RFC 00/26] Kill set_fs() in ALSA codes Takashi Iwai
` (16 preceding siblings ...)
2017-05-11 21:09 ` [PATCH RFC 17/26] ALSA: rme96: Convert to copy_silence ops Takashi Iwai
@ 2017-05-11 21:09 ` Takashi Iwai
2017-05-11 21:09 ` [PATCH RFC 19/26] ALSA: hdsp: " Takashi Iwai
` (8 subsequent siblings)
26 siblings, 0 replies; 41+ messages in thread
From: Takashi Iwai @ 2017-05-11 21:09 UTC (permalink / raw)
To: alsa-devel
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,
--
2.12.2
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH RFC 19/26] ALSA: hdsp: Convert to copy_silence ops
2017-05-11 21:08 [PATCH RFC 00/26] Kill set_fs() in ALSA codes Takashi Iwai
` (17 preceding siblings ...)
2017-05-11 21:09 ` [PATCH RFC 18/26] ALSA: rme9652: " Takashi Iwai
@ 2017-05-11 21:09 ` Takashi Iwai
2017-05-11 21:09 ` [PATCH RFC 20/26] ALSA: gus: " Takashi Iwai
` (7 subsequent siblings)
26 siblings, 0 replies; 41+ messages in thread
From: Takashi Iwai @ 2017-05-11 21:09 UTC (permalink / raw)
To: alsa-devel
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)
--
2.12.2
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH RFC 20/26] ALSA: gus: Convert to copy_silence ops
2017-05-11 21:08 [PATCH RFC 00/26] Kill set_fs() in ALSA codes Takashi Iwai
` (18 preceding siblings ...)
2017-05-11 21:09 ` [PATCH RFC 19/26] ALSA: hdsp: " Takashi Iwai
@ 2017-05-11 21:09 ` Takashi Iwai
2017-05-11 21:09 ` [PATCH RFC 21/26] ALSA: sb: " Takashi Iwai
` (6 subsequent siblings)
26 siblings, 0 replies; 41+ messages in thread
From: Takashi Iwai @ 2017-05-11 21:09 UTC (permalink / raw)
To: alsa-devel
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 = {
--
2.12.2
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH RFC 21/26] ALSA: sb: Convert to copy_silence ops
2017-05-11 21:08 [PATCH RFC 00/26] Kill set_fs() in ALSA codes Takashi Iwai
` (19 preceding siblings ...)
2017-05-11 21:09 ` [PATCH RFC 20/26] ALSA: gus: " Takashi Iwai
@ 2017-05-11 21:09 ` Takashi Iwai
2017-05-11 21:09 ` [PATCH RFC 22/26] ALSA: sh: " Takashi Iwai
` (5 subsequent siblings)
26 siblings, 0 replies; 41+ messages in thread
From: Takashi Iwai @ 2017-05-11 21:09 UTC (permalink / raw)
To: alsa-devel
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,
};
--
2.12.2
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH RFC 22/26] ALSA: sh: Convert to copy_silence ops
2017-05-11 21:08 [PATCH RFC 00/26] Kill set_fs() in ALSA codes Takashi Iwai
` (20 preceding siblings ...)
2017-05-11 21:09 ` [PATCH RFC 21/26] ALSA: sb: " Takashi Iwai
@ 2017-05-11 21:09 ` Takashi Iwai
2017-05-11 21:09 ` [PATCH RFC 23/26] ASoC: blackfin: " Takashi Iwai
` (4 subsequent siblings)
26 siblings, 0 replies; 41+ messages in thread
From: Takashi Iwai @ 2017-05-11 21:09 UTC (permalink / raw)
To: alsa-devel
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,
};
--
2.12.2
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH RFC 23/26] ASoC: blackfin: Convert to copy_silence ops
2017-05-11 21:08 [PATCH RFC 00/26] Kill set_fs() in ALSA codes Takashi Iwai
` (21 preceding siblings ...)
2017-05-11 21:09 ` [PATCH RFC 22/26] ALSA: sh: " Takashi Iwai
@ 2017-05-11 21:09 ` Takashi Iwai
2017-05-11 21:09 ` [PATCH RFC 24/26] [media] solo6x10: " Takashi Iwai
` (3 subsequent siblings)
26 siblings, 0 replies; 41+ messages in thread
From: Takashi Iwai @ 2017-05-11 21:09 UTC (permalink / raw)
To: alsa-devel
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)
--
2.12.2
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH RFC 24/26] [media] solo6x10: Convert to copy_silence ops
2017-05-11 21:08 [PATCH RFC 00/26] Kill set_fs() in ALSA codes Takashi Iwai
` (22 preceding siblings ...)
2017-05-11 21:09 ` [PATCH RFC 23/26] ASoC: blackfin: " Takashi Iwai
@ 2017-05-11 21:09 ` Takashi Iwai
2017-05-11 21:21 ` Takashi Iwai
2017-05-11 21:09 ` [PATCH RFC 25/26] ALSA: pcm: Drop the old copy and silence ops Takashi Iwai
` (2 subsequent siblings)
26 siblings, 1 reply; 41+ messages in thread
From: Takashi Iwai @ 2017-05-11 21:09 UTC (permalink / raw)
To: alsa-devel
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,
--
2.12.2
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH RFC 25/26] ALSA: pcm: Drop the old copy and silence ops
2017-05-11 21:08 [PATCH RFC 00/26] Kill set_fs() in ALSA codes Takashi Iwai
` (23 preceding siblings ...)
2017-05-11 21:09 ` [PATCH RFC 24/26] [media] solo6x10: " Takashi Iwai
@ 2017-05-11 21:09 ` Takashi Iwai
2017-05-11 21:09 ` [PATCH RFC 26/26] ALSA: pcm: Kill set_fs() usage in OSS layer and USB gadget Takashi Iwai
2017-05-14 8:23 ` [PATCH RFC 00/26] Kill set_fs() in ALSA codes Takashi Sakamoto
26 siblings, 0 replies; 41+ messages in thread
From: Takashi Iwai @ 2017-05-11 21:09 UTC (permalink / raw)
To: alsa-devel
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;
}
--
2.12.2
^ permalink raw reply related [flat|nested] 41+ messages in thread
* [PATCH RFC 26/26] ALSA: pcm: Kill set_fs() usage in OSS layer and USB gadget
2017-05-11 21:08 [PATCH RFC 00/26] Kill set_fs() in ALSA codes Takashi Iwai
` (24 preceding siblings ...)
2017-05-11 21:09 ` [PATCH RFC 25/26] ALSA: pcm: Drop the old copy and silence ops Takashi Iwai
@ 2017-05-11 21:09 ` Takashi Iwai
2017-05-14 8:23 ` [PATCH RFC 00/26] Kill set_fs() in ALSA codes Takashi Sakamoto
26 siblings, 0 replies; 41+ messages in thread
From: Takashi Iwai @ 2017-05-11 21:09 UTC (permalink / raw)
To: alsa-devel
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);
--
2.12.2
^ permalink raw reply related [flat|nested] 41+ messages in thread
* Re: [PATCH RFC 24/26] [media] solo6x10: Convert to copy_silence ops
2017-05-11 21:09 ` [PATCH RFC 24/26] [media] solo6x10: " Takashi Iwai
@ 2017-05-11 21:21 ` Takashi Iwai
0 siblings, 0 replies; 41+ messages in thread
From: Takashi Iwai @ 2017-05-11 21:21 UTC (permalink / raw)
To: alsa-devel
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
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH RFC 00/26] Kill set_fs() in ALSA codes
2017-05-11 21:08 [PATCH RFC 00/26] Kill set_fs() in ALSA codes Takashi Iwai
` (25 preceding siblings ...)
2017-05-11 21:09 ` [PATCH RFC 26/26] ALSA: pcm: Kill set_fs() usage in OSS layer and USB gadget Takashi Iwai
@ 2017-05-14 8:23 ` Takashi Sakamoto
2017-05-15 8:25 ` Takashi Iwai
26 siblings, 1 reply; 41+ messages in thread
From: Takashi Sakamoto @ 2017-05-14 8:23 UTC (permalink / raw)
To: Takashi Iwai, alsa-devel
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
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH RFC 00/26] Kill set_fs() in ALSA codes
2017-05-14 8:23 ` [PATCH RFC 00/26] Kill set_fs() in ALSA codes Takashi Sakamoto
@ 2017-05-15 8:25 ` Takashi Iwai
2017-05-24 2:12 ` Takashi Sakamoto
0 siblings, 1 reply; 41+ messages in thread
From: Takashi Iwai @ 2017-05-15 8:25 UTC (permalink / raw)
To: Takashi Sakamoto; +Cc: alsa-devel
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
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH RFC 00/26] Kill set_fs() in ALSA codes
2017-05-15 8:25 ` Takashi Iwai
@ 2017-05-24 2:12 ` Takashi Sakamoto
0 siblings, 0 replies; 41+ messages in thread
From: Takashi Sakamoto @ 2017-05-24 2:12 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel
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
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH RFC 16/26] ALSA: rme32: Convert to copy_silence ops
2017-05-11 21:09 ` [PATCH RFC 16/26] ALSA: rme32: " Takashi Iwai
@ 2018-07-18 10:22 ` René Rebe
2018-07-18 10:56 ` Takashi Iwai
0 siblings, 1 reply; 41+ messages in thread
From: René Rebe @ 2018-07-18 10:22 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel
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))
> 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,
> };
>
> --
> 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
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH RFC 16/26] ALSA: rme32: Convert to copy_silence ops
2018-07-18 10:22 ` René Rebe
@ 2018-07-18 10:56 ` Takashi Iwai
2018-07-18 18:10 ` René Rebe
2019-03-02 17:19 ` René Rebe
0 siblings, 2 replies; 41+ messages in thread
From: Takashi Iwai @ 2018-07-18 10:56 UTC (permalink / raw)
To: René Rebe; +Cc: 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))
> > 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,
> > };
> >
> > --
> > 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
>
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH RFC 16/26] ALSA: rme32: Convert to copy_silence ops
2018-07-18 10:56 ` Takashi Iwai
@ 2018-07-18 18:10 ` René Rebe
2018-07-18 18:43 ` René Rebe
2019-03-02 17:19 ` René Rebe
1 sibling, 1 reply; 41+ messages in thread
From: René Rebe @ 2018-07-18 18:10 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel
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))
>>> 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,
>>> };
>>>
>>> --
>>> 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
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH RFC 16/26] ALSA: rme32: Convert to copy_silence ops
2018-07-18 18:10 ` René Rebe
@ 2018-07-18 18:43 ` René Rebe
2018-07-19 8:09 ` René Rebe
0 siblings, 1 reply; 41+ messages in thread
From: René Rebe @ 2018-07-18 18:43 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel
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))
>>>> 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,
>>>> };
>>>>
>>>> --
>>>> 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
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH RFC 16/26] ALSA: rme32: Convert to copy_silence ops
2018-07-18 18:43 ` René Rebe
@ 2018-07-19 8:09 ` René Rebe
0 siblings, 0 replies; 41+ messages in thread
From: René Rebe @ 2018-07-19 8:09 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel
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))
>>>>> 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,
>>>>> };
>>>>>
>>>>> --
>>>>> 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
>
--
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
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH RFC 16/26] ALSA: rme32: Convert to copy_silence ops
2018-07-18 10:56 ` Takashi Iwai
2018-07-18 18:10 ` René Rebe
@ 2019-03-02 17:19 ` René Rebe
2019-03-24 11:19 ` [PATCH RFC] ALSA: rme32: fix interrupt ack for me René Rebe
1 sibling, 1 reply; 41+ messages in thread
From: René Rebe @ 2019-03-02 17:19 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel
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é
--
ExactCODE GmbH, Lietzenburger Str. 42, DE-10789 Berlin, https://exactcode.com
https://exactscan.com | https://ocrkit.com | https://t2sde.org | https://rene.rebe.de
^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH RFC] ALSA: rme32: fix interrupt ack for me
2019-03-02 17:19 ` René Rebe
@ 2019-03-24 11:19 ` René Rebe
2019-03-27 11:14 ` Takashi Iwai
0 siblings, 1 reply; 41+ messages in thread
From: René Rebe @ 2019-03-24 11:19 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel
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é
--
ExactCODE GmbH, Lietzenburger Str. 42, DE-10789 Berlin, https://exactcode.com
https://exactscan.com | https://ocrkit.com | https://t2sde.org | https://rene.rebe.de
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH RFC] ALSA: rme32: fix interrupt ack for me
2019-03-24 11:19 ` [PATCH RFC] ALSA: rme32: fix interrupt ack for me René Rebe
@ 2019-03-27 11:14 ` Takashi Iwai
2019-03-27 11:20 ` René Rebe
0 siblings, 1 reply; 41+ messages in thread
From: Takashi Iwai @ 2019-03-27 11:14 UTC (permalink / raw)
To: René Rebe; +Cc: alsa-devel
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
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH RFC] ALSA: rme32: fix interrupt ack for me
2019-03-27 11:14 ` Takashi Iwai
@ 2019-03-27 11:20 ` René Rebe
2019-03-27 11:24 ` Takashi Iwai
0 siblings, 1 reply; 41+ messages in thread
From: René Rebe @ 2019-03-27 11:20 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel
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é
--
ExactCODE GmbH, Lietzenburger Str. 42, DE-10789 Berlin, https://exactcode.com
https://exactscan.com | https://ocrkit.com | https://t2sde.org | https://rene.rebe.de
^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH RFC] ALSA: rme32: fix interrupt ack for me
2019-03-27 11:20 ` René Rebe
@ 2019-03-27 11:24 ` Takashi Iwai
0 siblings, 0 replies; 41+ messages in thread
From: Takashi Iwai @ 2019-03-27 11:24 UTC (permalink / raw)
To: René Rebe; +Cc: alsa-devel
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
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 41+ messages in thread
end of thread, other threads:[~2019-03-27 11:24 UTC | newest]
Thread overview: 41+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-11 21:08 [PATCH RFC 00/26] Kill set_fs() in ALSA codes Takashi Iwai
2017-05-11 21:09 ` [PATCH RFC 01/26] ALSA: hda - Simplify bound-beep mute control for ALC268 Takashi Iwai
2017-05-11 21:09 ` [PATCH RFC 02/26] ALSA: hda - Move bind-mixer switch codes to generic parser Takashi Iwai
2017-05-11 21:09 ` [PATCH RFC 03/26] ALSA: hda - Remove the generic bind ctl helpers Takashi Iwai
2017-05-11 21:09 ` [PATCH RFC 04/26] ALSA: hda - Remove the use of set_fs() Takashi Iwai
2017-05-11 21:09 ` [PATCH RFC 05/26] ALSA: hda - Fix a typo in comment Takashi Iwai
2017-05-11 21:09 ` [PATCH RFC 06/26] ALSA: hda - Remove superfluous header inclusions Takashi Iwai
2017-05-11 21:09 ` [PATCH RFC 07/26] ALSA: opl3: Kill unused set_fs() Takashi Iwai
2017-05-11 21:09 ` [PATCH RFC 08/26] ALSA: emu10k1: Get rid of set_fs() usage Takashi Iwai
2017-05-11 21:09 ` [PATCH RFC 09/26] ALSA: pcm: Remove set_fs() in PCM core code Takashi Iwai
2017-05-11 21:09 ` [PATCH RFC 10/26] ALSA: pcm: Introduce copy_silence PCM ops Takashi Iwai
2017-05-11 21:09 ` [PATCH RFC 11/26] ALSA: Update document about " Takashi Iwai
2017-05-11 21:09 ` [PATCH RFC 12/26] ALSA: dummy: Convert to copy_silence ops Takashi Iwai
2017-05-11 21:09 ` [PATCH RFC 13/26] ALSA: es1938: " Takashi Iwai
2017-05-11 21:09 ` [PATCH RFC 14/26] ALSA: korg1212: " Takashi Iwai
2017-05-11 21:09 ` [PATCH RFC 15/26] ALSA: nm256: " Takashi Iwai
2017-05-11 21:09 ` [PATCH RFC 16/26] ALSA: rme32: " Takashi Iwai
2018-07-18 10:22 ` René Rebe
2018-07-18 10:56 ` Takashi Iwai
2018-07-18 18:10 ` René Rebe
2018-07-18 18:43 ` René Rebe
2018-07-19 8:09 ` René Rebe
2019-03-02 17:19 ` René Rebe
2019-03-24 11:19 ` [PATCH RFC] ALSA: rme32: fix interrupt ack for me René Rebe
2019-03-27 11:14 ` Takashi Iwai
2019-03-27 11:20 ` René Rebe
2019-03-27 11:24 ` Takashi Iwai
2017-05-11 21:09 ` [PATCH RFC 17/26] ALSA: rme96: Convert to copy_silence ops Takashi Iwai
2017-05-11 21:09 ` [PATCH RFC 18/26] ALSA: rme9652: " Takashi Iwai
2017-05-11 21:09 ` [PATCH RFC 19/26] ALSA: hdsp: " Takashi Iwai
2017-05-11 21:09 ` [PATCH RFC 20/26] ALSA: gus: " Takashi Iwai
2017-05-11 21:09 ` [PATCH RFC 21/26] ALSA: sb: " Takashi Iwai
2017-05-11 21:09 ` [PATCH RFC 22/26] ALSA: sh: " Takashi Iwai
2017-05-11 21:09 ` [PATCH RFC 23/26] ASoC: blackfin: " Takashi Iwai
2017-05-11 21:09 ` [PATCH RFC 24/26] [media] solo6x10: " Takashi Iwai
2017-05-11 21:21 ` Takashi Iwai
2017-05-11 21:09 ` [PATCH RFC 25/26] ALSA: pcm: Drop the old copy and silence ops Takashi Iwai
2017-05-11 21:09 ` [PATCH RFC 26/26] ALSA: pcm: Kill set_fs() usage in OSS layer and USB gadget Takashi Iwai
2017-05-14 8:23 ` [PATCH RFC 00/26] Kill set_fs() in ALSA codes Takashi Sakamoto
2017-05-15 8:25 ` Takashi Iwai
2017-05-24 2:12 ` Takashi Sakamoto
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.