[alsa-devel] [RFC][PATCH] ALSA: pcm: introduce ACK_APPLPTR flag and bump up interface version to v2.0.14
Hi,
This patchset includes commits for both kernel/user land for an issue addressed in this message thread[1].
In a development period for v4.13, it's clear that there's a type of devices which demand userspace applications to change their behaviour. These devices can take care of application-side pointer on PCM buffer for their data transmission. As a result, these devices achieve some advantages. For these devices, drivers need to get feedback about current position of the pointer from applications when they queue/dequeue PCM frames on the buffer.
ALSA PCM core partly assist the above design. The core can call a handler in driver side when applications operate PCM frames by ioctl(2) with SNDRV_PCM_IOCTL_[READ|WRITE][N|I] command. However, when operating for mapped PCM buffer, it can't assist applications for the above design. In this scenario, applications should call ioctl(2) with SNDRV_PCM_IOCTL_SYNC_PTR after handle any PCM frames on the buffer.
This patchset bumps up interface version with new info flag to notify the above change to user land developers. This patchset is written with below ideas:
1. Stuffs in user land can tell their assists for the above design to kernel land by using hw_param flag. When they don't assist, they see on hw_params data that mmap operation is not supported. 2. Drivers can acknowledge the above flag. When registering appropriate PCM rules in advance, they can calculate values for each parameters in both cases that stuffs in user land can/cannot assist it. This is useful for packet-oriented drivers to judge utilizing any interrupt context for packetization. 3. This is device-specific issue, independently of platform architecture. On x86/ppc/alpha platforms, status/control data of PCM substream is still available in process' VMA of application via page frame mapping.
[1] http://mailman.alsa-project.org/pipermail/alsa-devel/2017-June/121438.html
For kernel land: Takashi Sakamoto (1): ALSA: pcm: introduce ACK_APPLPTR flag and bump up interface version to 2.0.14
include/uapi/sound/asound.h | 11 ++++++++++- sound/core/pcm_native.c | 25 +++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-)
For user land: Takashi Sakamoto (2): ALSA: pcm: introduce ACK_APPLPTR flag and bump up interface version to 2.0.14 pcm: add support for interface version 2.0.14
include/sound/asound.h | 11 ++++++++++- src/pcm/pcm_hw.c | 43 +++++++++++++++++++++++++++++++++++++------ 2 files changed, 47 insertions(+), 7 deletions(-)
In a development period for v4.13, it's clear that there's a type of devices which demand userspace applications to change their behaviour. These devices can take care of application-side pointer on PCM buffer for their data transmission. As a result, these devices achieve some advantages. For these devices, drivers need to get feedback about current position of the pointer from applications when they queue/dequeue PCM frames on the buffer.
When applications operates with SNDRV_PCM_IOCTL_[READ|WRITE][N|I] command for ioctl(2) to transmit/receive PCM frames, the above is achieved with an assist of ALSA PCM core. On the other hand, when applications run with mmap operations, the above is not achieved. In this case, they should call ioctl(2) with SNDRV_PCM_IOCTL_SYNC_PTR after operating any PCM frames on the buffer. But existent applications are not programmed to do it.
This commit adds a new flag; SNDRV_PCM_INFO_ACK_APPLPTR. When applications find this flag on 'struct snd_pcm_hw_params.info', they're expected to perform as the above. When programmed so, they should set SNDRV_PCM_HW_PARAMS_ACK_APPLPTR flag to the structure in advance. Else, they're not allowed to do mmap operation. This is for backward compatibility because existent stuffs in user land don't perform like the above.
As of this type of device, digital signal processor (DSP) integrated to Intel's processor is reported. The DSP can prefetch the PCM frames on the buffer and go to a deep sleep mode till next awakening. The awakening is done by drivers. When applications deliver information about the number of handled PCM frames to a corresponding driver in kernel land, then the driver calculates according to the number, then awakens the DSP from the sleep.
Additionally, there's two types of devices which potentially receive advantages from this change. One is packet-oriented drivers, and another is drivers with ALSA indirect layer. The former may process packets immediately in process context when handling the feedback. This results in reduction of audio latency. The latter may copy PCM frames in process context. This results in reduction CPU time on any interrupt context.
Anyway, this flag requires userspace applications to change their behaviour. This commit bumps up interface version to v2.0.14.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- include/uapi/sound/asound.h | 11 ++++++++++- sound/core/pcm_native.c | 25 +++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-)
diff --git a/include/uapi/sound/asound.h b/include/uapi/sound/asound.h index fd41697cb4d3..ff7bb89e65a6 100644 --- a/include/uapi/sound/asound.h +++ b/include/uapi/sound/asound.h @@ -152,7 +152,7 @@ struct snd_hwdep_dsp_image { * * *****************************************************************************/
-#define SNDRV_PCM_VERSION SNDRV_PROTOCOL_VERSION(2, 0, 13) +#define SNDRV_PCM_VERSION SNDRV_PROTOCOL_VERSION(2, 0, 14)
typedef unsigned long snd_pcm_uframes_t; typedef signed long snd_pcm_sframes_t; @@ -268,6 +268,8 @@ typedef int __bitwise snd_pcm_subformat_t; #define SNDRV_PCM_INFO_MMAP_VALID 0x00000002 /* period data are valid during transfer */ #define SNDRV_PCM_INFO_DOUBLE 0x00000004 /* Double buffering needed for PCM start/stop */ #define SNDRV_PCM_INFO_BATCH 0x00000010 /* double buffering */ +/* Driver/hardware acknowledge current appl_ptr for specific purposes. */ +#define SNDRV_PCM_INFO_ACK_APPLPTR 0x00000012 #define SNDRV_PCM_INFO_INTERLEAVED 0x00000100 /* channels are interleaved */ #define SNDRV_PCM_INFO_NONINTERLEAVED 0x00000200 /* channels are not interleaved */ #define SNDRV_PCM_INFO_COMPLEX 0x00000400 /* complex frame organization (mmap only) */ @@ -365,6 +367,13 @@ typedef int snd_pcm_hw_param_t; #define SNDRV_PCM_HW_PARAMS_NORESAMPLE (1<<0) /* avoid rate resampling */ #define SNDRV_PCM_HW_PARAMS_EXPORT_BUFFER (1<<1) /* export buffer */ #define SNDRV_PCM_HW_PARAMS_NO_PERIOD_WAKEUP (1<<2) /* disable period wakeups */ +/* + * When applications are programmed to execute ioctl(2) with + * SNDRV_PCM_IOCTL_SYNC_PTR for notification of current appl_ptr after handling + * any frames on mapped PCM buffer for driver/hardware with + * SNDRV_PCM_INFO_ACK_APPLPTR, this should be set. + */ +#define SNDRV_PCM_HW_PARAMS_ACK_APPLPTR (1<<3)
struct snd_interval { unsigned int min, max; diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index 7e8f3656b695..48772a2bd027 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -249,6 +249,27 @@ static bool hw_support_mmap(struct snd_pcm_substream *substream) return true; }
+static int constrain_params_with_precondition(struct snd_pcm_substream *substream, + struct snd_pcm_hw_params *params) +{ + struct snd_mask *mask; + + /* + * When drivers require applications' cooperation to report current + * appl_ptr and the applications voluntarily perform it, mmap operation + * is allowed. Else, read/write operations are allowed. + */ + if ((substream->runtime->hw.info & SNDRV_PCM_INFO_ACK_APPLPTR) && + !(params->flags & SNDRV_PCM_HW_PARAMS_ACK_APPLPTR)) { + mask = hw_param_mask(params, SNDRV_PCM_HW_PARAM_ACCESS); + snd_mask_reset(mask, SNDRV_PCM_ACCESS_MMAP_INTERLEAVED); + snd_mask_reset(mask, SNDRV_PCM_ACCESS_MMAP_NONINTERLEAVED); + snd_mask_reset(mask, SNDRV_PCM_ACCESS_MMAP_COMPLEX); + } + + return 0; +} + static int constrain_mask_params(struct snd_pcm_substream *substream, struct snd_pcm_hw_params *params) { @@ -495,6 +516,10 @@ int snd_pcm_hw_refine(struct snd_pcm_substream *substream, params->rate_den = 0; }
+ err = constrain_params_with_precondition(substream, params); + if (err < 0) + return err; + err = constrain_mask_params(substream, params); if (err < 0) return err;
On Sun, 18 Jun 2017 18:49:19 +0200, Takashi Sakamoto wrote:
In a development period for v4.13, it's clear that there's a type of devices which demand userspace applications to change their behaviour. These devices can take care of application-side pointer on PCM buffer for their data transmission. As a result, these devices achieve some advantages. For these devices, drivers need to get feedback about current position of the pointer from applications when they queue/dequeue PCM frames on the buffer.
When applications operates with SNDRV_PCM_IOCTL_[READ|WRITE][N|I] command for ioctl(2) to transmit/receive PCM frames, the above is achieved with an assist of ALSA PCM core. On the other hand, when applications run with mmap operations, the above is not achieved. In this case, they should call ioctl(2) with SNDRV_PCM_IOCTL_SYNC_PTR after operating any PCM frames on the buffer. But existent applications are not programmed to do it.
... they are programmed to behave so, when the PCM status mmap fails.
This commit adds a new flag; SNDRV_PCM_INFO_ACK_APPLPTR. When applications find this flag on 'struct snd_pcm_hw_params.info', they're expected to perform as the above. When programmed so, they should set SNDRV_PCM_HW_PARAMS_ACK_APPLPTR flag to the structure in advance. Else, they're not allowed to do mmap operation. This is for backward compatibility because existent stuffs in user land don't perform like the above.
As of this type of device, digital signal processor (DSP) integrated to Intel's processor is reported. The DSP can prefetch the PCM frames on the buffer and go to a deep sleep mode till next awakening. The awakening is done by drivers. When applications deliver information about the number of handled PCM frames to a corresponding driver in kernel land, then the driver calculates according to the number, then awakens the DSP from the sleep.
Additionally, there's two types of devices which potentially receive advantages from this change. One is packet-oriented drivers, and another is drivers with ALSA indirect layer. The former may process packets immediately in process context when handling the feedback. This results in reduction of audio latency. The latter may copy PCM frames in process context. This results in reduction CPU time on any interrupt context.
Anyway, this flag requires userspace applications to change their behaviour. This commit bumps up interface version to v2.0.14.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp
include/uapi/sound/asound.h | 11 ++++++++++- sound/core/pcm_native.c | 25 +++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-)
diff --git a/include/uapi/sound/asound.h b/include/uapi/sound/asound.h index fd41697cb4d3..ff7bb89e65a6 100644 --- a/include/uapi/sound/asound.h +++ b/include/uapi/sound/asound.h @@ -152,7 +152,7 @@ struct snd_hwdep_dsp_image {
******************************************************************************/
-#define SNDRV_PCM_VERSION SNDRV_PROTOCOL_VERSION(2, 0, 13) +#define SNDRV_PCM_VERSION SNDRV_PROTOCOL_VERSION(2, 0, 14)
typedef unsigned long snd_pcm_uframes_t; typedef signed long snd_pcm_sframes_t; @@ -268,6 +268,8 @@ typedef int __bitwise snd_pcm_subformat_t; #define SNDRV_PCM_INFO_MMAP_VALID 0x00000002 /* period data are valid during transfer */ #define SNDRV_PCM_INFO_DOUBLE 0x00000004 /* Double buffering needed for PCM start/stop */ #define SNDRV_PCM_INFO_BATCH 0x00000010 /* double buffering */ +/* Driver/hardware acknowledge current appl_ptr for specific purposes. */ +#define SNDRV_PCM_INFO_ACK_APPLPTR 0x00000012 #define SNDRV_PCM_INFO_INTERLEAVED 0x00000100 /* channels are interleaved */ #define SNDRV_PCM_INFO_NONINTERLEAVED 0x00000200 /* channels are not interleaved */ #define SNDRV_PCM_INFO_COMPLEX 0x00000400 /* complex frame organization (mmap only) */ @@ -365,6 +367,13 @@ typedef int snd_pcm_hw_param_t; #define SNDRV_PCM_HW_PARAMS_NORESAMPLE (1<<0) /* avoid rate resampling */ #define SNDRV_PCM_HW_PARAMS_EXPORT_BUFFER (1<<1) /* export buffer */ #define SNDRV_PCM_HW_PARAMS_NO_PERIOD_WAKEUP (1<<2) /* disable period wakeups */ +/*
- When applications are programmed to execute ioctl(2) with
- SNDRV_PCM_IOCTL_SYNC_PTR for notification of current appl_ptr after handling
- any frames on mapped PCM buffer for driver/hardware with
- SNDRV_PCM_INFO_ACK_APPLPTR, this should be set.
- */
+#define SNDRV_PCM_HW_PARAMS_ACK_APPLPTR (1<<3)
struct snd_interval { unsigned int min, max; diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index 7e8f3656b695..48772a2bd027 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -249,6 +249,27 @@ static bool hw_support_mmap(struct snd_pcm_substream *substream) return true; }
+static int constrain_params_with_precondition(struct snd_pcm_substream *substream,
struct snd_pcm_hw_params *params)+{
- struct snd_mask *mask;
- /*
* When drivers require applications' cooperation to report current* appl_ptr and the applications voluntarily perform it, mmap operation* is allowed. Else, read/write operations are allowed.*/- if ((substream->runtime->hw.info & SNDRV_PCM_INFO_ACK_APPLPTR) &&
!(params->flags & SNDRV_PCM_HW_PARAMS_ACK_APPLPTR)) {mask = hw_param_mask(params, SNDRV_PCM_HW_PARAM_ACCESS);snd_mask_reset(mask, SNDRV_PCM_ACCESS_MMAP_INTERLEAVED);snd_mask_reset(mask, SNDRV_PCM_ACCESS_MMAP_NONINTERLEAVED);snd_mask_reset(mask, SNDRV_PCM_ACCESS_MMAP_COMPLEX);
Well, it's only the lack of sync_ptr, and the mmap of the ring buffer itself is fine. Disabling the ring-buffer mmap is a much bigger hammer and the significant performance penalty than disabling PCM control/status mmap.
thanks,
Takashi
On Jun 19 2017 02:15, Takashi Iwai wrote:
On Sun, 18 Jun 2017 18:49:19 +0200, Takashi Sakamoto wrote:
In a development period for v4.13, it's clear that there's a type of devices which demand userspace applications to change their behaviour. These devices can take care of application-side pointer on PCM buffer for their data transmission. As a result, these devices achieve some advantages. For these devices, drivers need to get feedback about current position of the pointer from applications when they queue/dequeue PCM frames on the buffer.
When applications operates with SNDRV_PCM_IOCTL_[READ|WRITE][N|I] command for ioctl(2) to transmit/receive PCM frames, the above is achieved with an assist of ALSA PCM core. On the other hand, when applications run with mmap operations, the above is not achieved. In this case, they should call ioctl(2) with SNDRV_PCM_IOCTL_SYNC_PTR after operating any PCM frames on the buffer. But existent applications are not programmed to do it.
... they are programmed to behave so, when the PCM status mmap fails.
As I said, the solution to disable mmap for status/control data has different scope from this issue.
This commit adds a new flag; SNDRV_PCM_INFO_ACK_APPLPTR. When applications find this flag on 'struct snd_pcm_hw_params.info', they're expected to perform as the above. When programmed so, they should set SNDRV_PCM_HW_PARAMS_ACK_APPLPTR flag to the structure in advance. Else, they're not allowed to do mmap operation. This is for backward compatibility because existent stuffs in user land don't perform like the above.
As of this type of device, digital signal processor (DSP) integrated to Intel's processor is reported. The DSP can prefetch the PCM frames on the buffer and go to a deep sleep mode till next awakening. The awakening is done by drivers. When applications deliver information about the number of handled PCM frames to a corresponding driver in kernel land, then the driver calculates according to the number, then awakens the DSP from the sleep.
Additionally, there's two types of devices which potentially receive advantages from this change. One is packet-oriented drivers, and another is drivers with ALSA indirect layer. The former may process packets immediately in process context when handling the feedback. This results in reduction of audio latency. The latter may copy PCM frames in process context. This results in reduction CPU time on any interrupt context.
Anyway, this flag requires userspace applications to change their behaviour. This commit bumps up interface version to v2.0.14.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp
include/uapi/sound/asound.h | 11 ++++++++++- sound/core/pcm_native.c | 25 +++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-)
diff --git a/include/uapi/sound/asound.h b/include/uapi/sound/asound.h index fd41697cb4d3..ff7bb89e65a6 100644 --- a/include/uapi/sound/asound.h +++ b/include/uapi/sound/asound.h @@ -152,7 +152,7 @@ struct snd_hwdep_dsp_image {
******************************************************************************/
-#define SNDRV_PCM_VERSION SNDRV_PROTOCOL_VERSION(2, 0, 13) +#define SNDRV_PCM_VERSION SNDRV_PROTOCOL_VERSION(2, 0, 14)
typedef unsigned long snd_pcm_uframes_t; typedef signed long snd_pcm_sframes_t; @@ -268,6 +268,8 @@ typedef int __bitwise snd_pcm_subformat_t; #define SNDRV_PCM_INFO_MMAP_VALID 0x00000002 /* period data are valid during transfer */ #define SNDRV_PCM_INFO_DOUBLE 0x00000004 /* Double buffering needed for PCM start/stop */ #define SNDRV_PCM_INFO_BATCH 0x00000010 /* double buffering */ +/* Driver/hardware acknowledge current appl_ptr for specific purposes. */ +#define SNDRV_PCM_INFO_ACK_APPLPTR 0x00000012 #define SNDRV_PCM_INFO_INTERLEAVED 0x00000100 /* channels are interleaved */ #define SNDRV_PCM_INFO_NONINTERLEAVED 0x00000200 /* channels are not interleaved */ #define SNDRV_PCM_INFO_COMPLEX 0x00000400 /* complex frame organization (mmap only) */ @@ -365,6 +367,13 @@ typedef int snd_pcm_hw_param_t; #define SNDRV_PCM_HW_PARAMS_NORESAMPLE (1<<0) /* avoid rate resampling */ #define SNDRV_PCM_HW_PARAMS_EXPORT_BUFFER (1<<1) /* export buffer */ #define SNDRV_PCM_HW_PARAMS_NO_PERIOD_WAKEUP (1<<2) /* disable period wakeups */ +/*
- When applications are programmed to execute ioctl(2) with
- SNDRV_PCM_IOCTL_SYNC_PTR for notification of current appl_ptr after handling
- any frames on mapped PCM buffer for driver/hardware with
- SNDRV_PCM_INFO_ACK_APPLPTR, this should be set.
- */
+#define SNDRV_PCM_HW_PARAMS_ACK_APPLPTR (1<<3)
struct snd_interval { unsigned int min, max; diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index 7e8f3656b695..48772a2bd027 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -249,6 +249,27 @@ static bool hw_support_mmap(struct snd_pcm_substream *substream) return true; }
+static int constrain_params_with_precondition(struct snd_pcm_substream *substream,
struct snd_pcm_hw_params *params)+{
- struct snd_mask *mask;
- /*
* When drivers require applications' cooperation to report current* appl_ptr and the applications voluntarily perform it, mmap operation* is allowed. Else, read/write operations are allowed.*/- if ((substream->runtime->hw.info & SNDRV_PCM_INFO_ACK_APPLPTR) &&
!(params->flags & SNDRV_PCM_HW_PARAMS_ACK_APPLPTR)) {mask = hw_param_mask(params, SNDRV_PCM_HW_PARAM_ACCESS);snd_mask_reset(mask, SNDRV_PCM_ACCESS_MMAP_INTERLEAVED);snd_mask_reset(mask, SNDRV_PCM_ACCESS_MMAP_NONINTERLEAVED);snd_mask_reset(mask, SNDRV_PCM_ACCESS_MMAP_COMPLEX);Well, it's only the lack of sync_ptr, and the mmap of the ring buffer itself is fine. Disabling the ring-buffer mmap is a much bigger hammer and the significant performance penalty than disabling PCM control/status mmap.
No. Old stuffs don't satisfy requirements of the new devices/drivers. Constraint to read/write operation is fair enough and not penalty because programs are written so that they cannot assist the new devices/drivers.
Well-programmed applications can handle a case in which mmap operation is not allowed. This patchset is enough reasonable.
Regards
Takashi Sakamoto
On Mon, 19 Jun 2017 14:26:32 +0200, Takashi Sakamoto wrote:
On Jun 19 2017 02:15, Takashi Iwai wrote:
On Sun, 18 Jun 2017 18:49:19 +0200, Takashi Sakamoto wrote:
In a development period for v4.13, it's clear that there's a type of devices which demand userspace applications to change their behaviour. These devices can take care of application-side pointer on PCM buffer for their data transmission. As a result, these devices achieve some advantages. For these devices, drivers need to get feedback about current position of the pointer from applications when they queue/dequeue PCM frames on the buffer.
When applications operates with SNDRV_PCM_IOCTL_[READ|WRITE][N|I] command for ioctl(2) to transmit/receive PCM frames, the above is achieved with an assist of ALSA PCM core. On the other hand, when applications run with mmap operations, the above is not achieved. In this case, they should call ioctl(2) with SNDRV_PCM_IOCTL_SYNC_PTR after operating any PCM frames on the buffer. But existent applications are not programmed to do it.
... they are programmed to behave so, when the PCM status mmap fails.
As I said, the solution to disable mmap for status/control data has different scope from this issue.
The mechanism is used not only for coherency mmap issues, but already deployed for other purpose in the situation that require the explicit status/contorl copy. It's already generically used for x86.
This commit adds a new flag; SNDRV_PCM_INFO_ACK_APPLPTR. When applications find this flag on 'struct snd_pcm_hw_params.info', they're expected to perform as the above. When programmed so, they should set SNDRV_PCM_HW_PARAMS_ACK_APPLPTR flag to the structure in advance. Else, they're not allowed to do mmap operation. This is for backward compatibility because existent stuffs in user land don't perform like the above.
As of this type of device, digital signal processor (DSP) integrated to Intel's processor is reported. The DSP can prefetch the PCM frames on the buffer and go to a deep sleep mode till next awakening. The awakening is done by drivers. When applications deliver information about the number of handled PCM frames to a corresponding driver in kernel land, then the driver calculates according to the number, then awakens the DSP from the sleep.
Additionally, there's two types of devices which potentially receive advantages from this change. One is packet-oriented drivers, and another is drivers with ALSA indirect layer. The former may process packets immediately in process context when handling the feedback. This results in reduction of audio latency. The latter may copy PCM frames in process context. This results in reduction CPU time on any interrupt context.
Anyway, this flag requires userspace applications to change their behaviour. This commit bumps up interface version to v2.0.14.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp
include/uapi/sound/asound.h | 11 ++++++++++- sound/core/pcm_native.c | 25 +++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-)
diff --git a/include/uapi/sound/asound.h b/include/uapi/sound/asound.h index fd41697cb4d3..ff7bb89e65a6 100644 --- a/include/uapi/sound/asound.h +++ b/include/uapi/sound/asound.h @@ -152,7 +152,7 @@ struct snd_hwdep_dsp_image {
******************************************************************************/ -#define SNDRV_PCM_VERSION SNDRV_PROTOCOL_VERSION(2, 0, 13) +#define SNDRV_PCM_VERSION SNDRV_PROTOCOL_VERSION(2, 0, 14) typedef unsigned long snd_pcm_uframes_t; typedef signed long snd_pcm_sframes_t; @@ -268,6 +268,8 @@ typedef int __bitwise snd_pcm_subformat_t; #define SNDRV_PCM_INFO_MMAP_VALID 0x00000002 /* period data are valid during transfer */ #define SNDRV_PCM_INFO_DOUBLE 0x00000004 /* Double buffering needed for PCM start/stop */ #define SNDRV_PCM_INFO_BATCH 0x00000010 /* double buffering */ +/* Driver/hardware acknowledge current appl_ptr for specific purposes. */ +#define SNDRV_PCM_INFO_ACK_APPLPTR 0x00000012 #define SNDRV_PCM_INFO_INTERLEAVED 0x00000100 /* channels are interleaved */ #define SNDRV_PCM_INFO_NONINTERLEAVED 0x00000200 /* channels are not interleaved */ #define SNDRV_PCM_INFO_COMPLEX 0x00000400 /* complex frame organization (mmap only) */ @@ -365,6 +367,13 @@ typedef int snd_pcm_hw_param_t; #define SNDRV_PCM_HW_PARAMS_NORESAMPLE (1<<0) /* avoid rate resampling */ #define SNDRV_PCM_HW_PARAMS_EXPORT_BUFFER (1<<1) /* export buffer */ #define SNDRV_PCM_HW_PARAMS_NO_PERIOD_WAKEUP (1<<2) /* disable period wakeups */ +/*
- When applications are programmed to execute ioctl(2) with
- SNDRV_PCM_IOCTL_SYNC_PTR for notification of current appl_ptr after handling
- any frames on mapped PCM buffer for driver/hardware with
- SNDRV_PCM_INFO_ACK_APPLPTR, this should be set.
- */
+#define SNDRV_PCM_HW_PARAMS_ACK_APPLPTR (1<<3) struct snd_interval { unsigned int min, max; diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index 7e8f3656b695..48772a2bd027 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -249,6 +249,27 @@ static bool hw_support_mmap(struct snd_pcm_substream *substream) return true; } +static int constrain_params_with_precondition(struct snd_pcm_substream *substream,
struct snd_pcm_hw_params *params)+{
- struct snd_mask *mask;
- /*
* When drivers require applications' cooperation to report current* appl_ptr and the applications voluntarily perform it, mmap operation* is allowed. Else, read/write operations are allowed.*/- if ((substream->runtime->hw.info & SNDRV_PCM_INFO_ACK_APPLPTR) &&
!(params->flags & SNDRV_PCM_HW_PARAMS_ACK_APPLPTR)) {mask = hw_param_mask(params, SNDRV_PCM_HW_PARAM_ACCESS);snd_mask_reset(mask, SNDRV_PCM_ACCESS_MMAP_INTERLEAVED);snd_mask_reset(mask, SNDRV_PCM_ACCESS_MMAP_NONINTERLEAVED);snd_mask_reset(mask, SNDRV_PCM_ACCESS_MMAP_COMPLEX);Well, it's only the lack of sync_ptr, and the mmap of the ring buffer itself is fine. Disabling the ring-buffer mmap is a much bigger hammer and the significant performance penalty than disabling PCM control/status mmap.
No. Old stuffs don't satisfy requirements of the new devices/drivers. Constraint to read/write operation is fair enough and not penalty because programs are written so that they cannot assist the new devices/drivers.
The disablement of mmap *CAN* assist the new devices/drivers. "they cannot"? Well, it works as is.
Well-programmed applications can handle a case in which mmap operation is not allowed. This patchset is enough reasonable.
Oh, this is a big "NO". The behavior difference between mmap and read/write is enormous, and we can't blame the application to be broken if it doesn't support both.
That is, the probability of breakage is much bigger than the disablement of status/control mmap, which is absorbed already in the existing alsa-lib code, and it's been proven to work for decades.
thanks,
Takashi
In a development period for v4.13, it's clear that there's a type of devices which demand userspace applications to change their behaviour. These devices can take care of application-side pointer on PCM buffer for their data transmission. As a result, these devices achieve some advantages. For these devices, drivers need to get feedback about current position of the pointer from applications when they queue/dequeue PCM frames on the buffer.
When applications operates with SNDRV_PCM_IOCTL_[READ|WRITE][N|I] command for ioctl(2) to transmit/receive PCM frames, the above is achieved with an assist of ALSA PCM core. On the other hand, when applications run with mmap operations, the above is not achieved. In this case, they should call ioctl(2) with SNDRV_PCM_IOCTL_SYNC_PTR after operating any PCM frames on the buffer. But existent applications are not programmed to do it.
This commit adds a new flag; SNDRV_PCM_INFO_ACK_APPLPTR. When applications find this flag on 'struct snd_pcm_hw_params.info', they're expected to perform as the above. When programmed so, they should set SNDRV_PCM_HW_PARAMS_ACK_APPLPTR flag to the structure in advance. Else, they're not allowed to do mmap operation. This is for backward compatibility because existent stuffs in user land don't perform like the above.
As of this type of device, digital signal processor (DSP) integrated to Intel's processor is reported. The DSP can prefetch the PCM frames on the buffer and go to a deep sleep mode till next awakening. The awakening is done by drivers. When applications deliver information about the number of handled PCM frames to a corresponding driver in kernel land, then the driver calculates according to the number, then awakens the DSP from the sleep.
Additionally, there's two types of devices which potentially receive advantages from this change. One is packet-oriented drivers, and another is drivers with ALSA indirect layer. The former may process packets immediately in process context when handling the feedback. This results in reduction of audio latency. The latter may copy PCM frames in process context. This results in reduction CPU time on any interrupt context.
Anyway, this flag requires userspace applications to change their behaviour. This commit bumps up interface version to v2.0.14.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- include/sound/asound.h | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/include/sound/asound.h b/include/sound/asound.h index fb8d7d7e..dc367cf0 100644 --- a/include/sound/asound.h +++ b/include/sound/asound.h @@ -152,7 +152,7 @@ struct snd_hwdep_dsp_image { * * *****************************************************************************/
-#define SNDRV_PCM_VERSION SNDRV_PROTOCOL_VERSION(2, 0, 13) +#define SNDRV_PCM_VERSION SNDRV_PROTOCOL_VERSION(2, 0, 14)
typedef unsigned long snd_pcm_uframes_t; typedef signed long snd_pcm_sframes_t; @@ -268,6 +268,8 @@ typedef int __bitwise snd_pcm_subformat_t; #define SNDRV_PCM_INFO_MMAP_VALID 0x00000002 /* period data are valid during transfer */ #define SNDRV_PCM_INFO_DOUBLE 0x00000004 /* Double buffering needed for PCM start/stop */ #define SNDRV_PCM_INFO_BATCH 0x00000010 /* double buffering */ +/* Driver/hardware acknowledge current appl_ptr for specific purposes. */ +#define SNDRV_PCM_INFO_ACK_APPLPTR 0x00000012 #define SNDRV_PCM_INFO_INTERLEAVED 0x00000100 /* channels are interleaved */ #define SNDRV_PCM_INFO_NONINTERLEAVED 0x00000200 /* channels are not interleaved */ #define SNDRV_PCM_INFO_COMPLEX 0x00000400 /* complex frame organization (mmap only) */ @@ -365,6 +367,13 @@ typedef int snd_pcm_hw_param_t; #define SNDRV_PCM_HW_PARAMS_NORESAMPLE (1<<0) /* avoid rate resampling */ #define SNDRV_PCM_HW_PARAMS_EXPORT_BUFFER (1<<1) /* export buffer */ #define SNDRV_PCM_HW_PARAMS_NO_PERIOD_WAKEUP (1<<2) /* disable period wakeups */ +/* + * When applications are programmed to execute ioctl(2) with + * SNDRV_PCM_IOCTL_SYNC_PTR for notification of current appl_ptr after handling + * any frames on mapped PCM buffer for driver/hardware with + * SNDRV_PCM_INFO_ACK_APPLPTR, this should be set. + */ +#define SNDRV_PCM_HW_PARAMS_ACK_APPLPTR (1<<3)
struct snd_interval { unsigned int min, max;
In interface version 2.0.14, ALSA PCM core request feedback to applications when they operate any PCM frames in mmap operation for a type of devices with SNDRV_PCM_INFO_ACK_APPLPTR flag.
This commit adds support for this change to keep compatibility for existent applications which uses alsa-lib API.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- src/pcm/pcm_hw.c | 43 +++++++++++++++++++++++++++++++++++++------ 1 file changed, 37 insertions(+), 6 deletions(-)
diff --git a/src/pcm/pcm_hw.c b/src/pcm/pcm_hw.c index 30cd5d0f..880109ad 100644 --- a/src/pcm/pcm_hw.c +++ b/src/pcm/pcm_hw.c @@ -105,6 +105,7 @@ typedef struct { /* for chmap */ unsigned int chmap_caps; snd_pcm_chmap_query_t **chmap_override; + int ack_applptr; } snd_pcm_hw_t;
#define SNDRV_FILE_PCM_STREAM_PLAYBACK ALSA_DEVICE_DIRECTORY "pcmC%iD%ip" @@ -128,11 +129,12 @@ struct timespec snd_pcm_hw_fast_tstamp(snd_pcm_t *pcm) } #endif /* DOC_HIDDEN */
-static int sync_ptr1(snd_pcm_hw_t *hw, unsigned int flags) +static int sync_ptr1(snd_pcm_hw_t *hw, struct snd_pcm_sync_ptr *sync_ptr, + unsigned int flags) { int err; - hw->sync_ptr->flags = flags; - err = ioctl((hw)->fd, SNDRV_PCM_IOCTL_SYNC_PTR, (hw)->sync_ptr); + sync_ptr->flags = flags; + err = ioctl((hw)->fd, SNDRV_PCM_IOCTL_SYNC_PTR, sync_ptr); if (err < 0) { err = -errno; SYSMSG("SNDRV_PCM_IOCTL_SYNC_PTR failed (%i)", err); @@ -143,7 +145,7 @@ static int sync_ptr1(snd_pcm_hw_t *hw, unsigned int flags)
static inline int sync_ptr(snd_pcm_hw_t *hw, unsigned int flags) { - return hw->sync_ptr ? sync_ptr1(hw, flags) : 0; + return hw->sync_ptr ? sync_ptr1(hw, hw->sync_ptr, flags) : 0; }
static int snd_pcm_hw_clear_timer_queue(snd_pcm_hw_t *hw) @@ -295,6 +297,14 @@ static int snd_pcm_hw_hw_refine(snd_pcm_t *pcm, snd_pcm_hw_params_t *params) return err; }
+ /* + * For drivers with SNDRV_PCM_INFO_ACK_APPLPTR, this has an effect + * to tell that user land can perform MMAP operation as corresponding + * hardware expects. For the other drivers, this has no side-effect. + */ + if (hw->version >= SNDRV_PROTOCOL_VERSION(2, 0, 14)) + params->flags |= SNDRV_PCM_INFO_ACK_APPLPTR; + if (hw_refine_call(hw, params) < 0) { err = -errno; // SYSMSG("SNDRV_PCM_IOCTL_HW_REFINE failed"); @@ -322,6 +332,15 @@ static int snd_pcm_hw_hw_params(snd_pcm_t *pcm, snd_pcm_hw_params_t * params) { snd_pcm_hw_t *hw = pcm->private_data; int err; + + /* + * For drivers with SNDRV_PCM_INFO_ACK_APPLPTR, this has an effect + * to tell that user land can perform MMAP operation as corresponding + * hardware expects. For the other drivers, this has no side-effect. + */ + if (hw->version >= SNDRV_PROTOCOL_VERSION(2, 0, 14)) + params->flags |= SNDRV_PCM_INFO_ACK_APPLPTR; + if (hw_params_call(hw, params) < 0) { err = -errno; SYSMSG("SNDRV_PCM_IOCTL_HW_PARAMS failed (%i)", err); @@ -337,6 +356,11 @@ static int snd_pcm_hw_hw_params(snd_pcm_t *pcm, snd_pcm_hw_params_t * params) snd_pcm_set_appl_ptr(pcm, &hw->mmap_control->appl_ptr, hw->fd, SNDRV_PCM_MMAP_OFFSET_CONTROL); } + + if (hw->version >= SNDRV_PROTOCOL_VERSION(2, 0, 14) && + (params->info & SNDRV_PCM_INFO_ACK_APPLPTR)) + hw->ack_applptr = 1; + return 0; }
@@ -568,7 +592,8 @@ static int snd_pcm_hw_hwsync(snd_pcm_t *pcm) int fd = hw->fd, err; if (SNDRV_PROTOCOL_VERSION(2, 0, 3) <= hw->version) { if (hw->sync_ptr) { - err = sync_ptr1(hw, SNDRV_PCM_SYNC_PTR_HWSYNC); + err = sync_ptr1(hw, hw->sync_ptr, + SNDRV_PCM_SYNC_PTR_HWSYNC); if (err < 0) return err; } else { @@ -995,7 +1020,13 @@ static snd_pcm_sframes_t snd_pcm_hw_mmap_commit(snd_pcm_t *pcm, snd_pcm_hw_t *hw = pcm->private_data;
snd_pcm_mmap_appl_forward(pcm, size); - sync_ptr(hw, 0); + if (hw->sync_ptr) { + sync_ptr1(hw, hw->sync_ptr, 0); + } else if (hw->ack_applptr) { + struct snd_pcm_sync_ptr sync_ptr = {0}; + sync_ptr.c.control = *hw->mmap_control; + sync_ptr1(hw, &sync_ptr, 0); + } #ifdef DEBUG_MMAP fprintf(stderr, "appl_forward: hw_ptr = %li, appl_ptr = %li, size = %li\n", *pcm->hw.ptr, *pcm->appl.ptr, size); #endif
On Sun, 18 Jun 2017 18:49:18 +0200, Takashi Sakamoto wrote:
Hi,
This patchset includes commits for both kernel/user land for an issue addressed in this message thread[1].
In a development period for v4.13, it's clear that there's a type of devices which demand userspace applications to change their behaviour. These devices can take care of application-side pointer on PCM buffer for their data transmission. As a result, these devices achieve some advantages. For these devices, drivers need to get feedback about current position of the pointer from applications when they queue/dequeue PCM frames on the buffer.
ALSA PCM core partly assist the above design. The core can call a handler in driver side when applications operate PCM frames by ioctl(2) with SNDRV_PCM_IOCTL_[READ|WRITE][N|I] command. However, when operating for mapped PCM buffer, it can't assist applications for the above design. In this scenario, applications should call ioctl(2) with SNDRV_PCM_IOCTL_SYNC_PTR after handle any PCM frames on the buffer.
This patchset bumps up interface version with new info flag to notify the above change to user land developers. This patchset is written with below ideas:
- Stuffs in user land can tell their assists for the above design to kernel land by using hw_param flag. When they don't assist, they see on hw_params data that mmap operation is not supported.
- Drivers can acknowledge the above flag. When registering appropriate PCM rules in advance, they can calculate values for each parameters in both cases that stuffs in user land can/cannot assist it. This is useful for packet-oriented drivers to judge utilizing any interrupt context for packetization.
- This is device-specific issue, independently of platform architecture. On x86/ppc/alpha platforms, status/control data of PCM substream is still available in process' VMA of application via page frame mapping.
[1] http://mailman.alsa-project.org/pipermail/alsa-devel/2017-June/121438.html
For kernel land: Takashi Sakamoto (1): ALSA: pcm: introduce ACK_APPLPTR flag and bump up interface version to 2.0.14
include/uapi/sound/asound.h | 11 ++++++++++- sound/core/pcm_native.c | 25 +++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-)
For user land: Takashi Sakamoto (2): ALSA: pcm: introduce ACK_APPLPTR flag and bump up interface version to 2.0.14 pcm: add support for interface version 2.0.14
include/sound/asound.h | 11 ++++++++++- src/pcm/pcm_hw.c | 43 +++++++++++++++++++++++++++++++++++++------ 2 files changed, 47 insertions(+), 7 deletions(-)
Well, these mandate both kernel and user-side changes, so if you run the old applications with old alsa-lib, it can't work, right?
A similar patchset is already in my local queue (my version called it differently, SNDRV_PCM_INFO_SYNC_APPLPTR :), but it wasn't submitted yet because we wanted to fix the issue without affecting alsa-lib at first. Do you have the patch for that?
thanks,
Takashi
On Sun, 18 Jun 2017 19:08:05 +0200, Takashi Iwai wrote:
On Sun, 18 Jun 2017 18:49:18 +0200, Takashi Sakamoto wrote:
Hi,
This patchset includes commits for both kernel/user land for an issue addressed in this message thread[1].
In a development period for v4.13, it's clear that there's a type of devices which demand userspace applications to change their behaviour. These devices can take care of application-side pointer on PCM buffer for their data transmission. As a result, these devices achieve some advantages. For these devices, drivers need to get feedback about current position of the pointer from applications when they queue/dequeue PCM frames on the buffer.
ALSA PCM core partly assist the above design. The core can call a handler in driver side when applications operate PCM frames by ioctl(2) with SNDRV_PCM_IOCTL_[READ|WRITE][N|I] command. However, when operating for mapped PCM buffer, it can't assist applications for the above design. In this scenario, applications should call ioctl(2) with SNDRV_PCM_IOCTL_SYNC_PTR after handle any PCM frames on the buffer.
This patchset bumps up interface version with new info flag to notify the above change to user land developers. This patchset is written with below ideas:
- Stuffs in user land can tell their assists for the above design to kernel land by using hw_param flag. When they don't assist, they see on hw_params data that mmap operation is not supported.
- Drivers can acknowledge the above flag. When registering appropriate PCM rules in advance, they can calculate values for each parameters in both cases that stuffs in user land can/cannot assist it. This is useful for packet-oriented drivers to judge utilizing any interrupt context for packetization.
- This is device-specific issue, independently of platform architecture. On x86/ppc/alpha platforms, status/control data of PCM substream is still available in process' VMA of application via page frame mapping.
[1] http://mailman.alsa-project.org/pipermail/alsa-devel/2017-June/121438.html
For kernel land: Takashi Sakamoto (1): ALSA: pcm: introduce ACK_APPLPTR flag and bump up interface version to 2.0.14
include/uapi/sound/asound.h | 11 ++++++++++- sound/core/pcm_native.c | 25 +++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-)
For user land: Takashi Sakamoto (2): ALSA: pcm: introduce ACK_APPLPTR flag and bump up interface version to 2.0.14 pcm: add support for interface version 2.0.14
include/sound/asound.h | 11 ++++++++++- src/pcm/pcm_hw.c | 43 +++++++++++++++++++++++++++++++++++++------ 2 files changed, 47 insertions(+), 7 deletions(-)
Well, these mandate both kernel and user-side changes, so if you run the old applications with old alsa-lib, it can't work, right?
I see now that you achieved it by dropping the whole mmap.
This is a very big concern from the performance POV. It merely kills the whole merit of adding the appl_ptr notification (e.g. PA switches to a completely different mode), so this is likely no-go, sorry.
thanks,
Takashi
On Jun 19 2017 02:17, Takashi Iwai wrote:
On Sun, 18 Jun 2017 19:08:05 +0200, Takashi Iwai wrote:
On Sun, 18 Jun 2017 18:49:18 +0200, Takashi Sakamoto wrote:
Hi,
This patchset includes commits for both kernel/user land for an issue addressed in this message thread[1].
In a development period for v4.13, it's clear that there's a type of devices which demand userspace applications to change their behaviour. These devices can take care of application-side pointer on PCM buffer for their data transmission. As a result, these devices achieve some advantages. For these devices, drivers need to get feedback about current position of the pointer from applications when they queue/dequeue PCM frames on the buffer.
ALSA PCM core partly assist the above design. The core can call a handler in driver side when applications operate PCM frames by ioctl(2) with SNDRV_PCM_IOCTL_[READ|WRITE][N|I] command. However, when operating for mapped PCM buffer, it can't assist applications for the above design. In this scenario, applications should call ioctl(2) with SNDRV_PCM_IOCTL_SYNC_PTR after handle any PCM frames on the buffer.
This patchset bumps up interface version with new info flag to notify the above change to user land developers. This patchset is written with below ideas:
- Stuffs in user land can tell their assists for the above design to kernel land by using hw_param flag. When they don't assist, they see on hw_params data that mmap operation is not supported.
- Drivers can acknowledge the above flag. When registering appropriate PCM rules in advance, they can calculate values for each parameters in both cases that stuffs in user land can/cannot assist it. This is useful for packet-oriented drivers to judge utilizing any interrupt context for packetization.
- This is device-specific issue, independently of platform architecture. On x86/ppc/alpha platforms, status/control data of PCM substream is still available in process' VMA of application via page frame mapping.
[1] http://mailman.alsa-project.org/pipermail/alsa-devel/2017-June/121438.html
For kernel land: Takashi Sakamoto (1): ALSA: pcm: introduce ACK_APPLPTR flag and bump up interface version to 2.0.14
include/uapi/sound/asound.h | 11 ++++++++++- sound/core/pcm_native.c | 25 +++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-)
For user land: Takashi Sakamoto (2): ALSA: pcm: introduce ACK_APPLPTR flag and bump up interface version to 2.0.14 pcm: add support for interface version 2.0.14
include/sound/asound.h | 11 ++++++++++- src/pcm/pcm_hw.c | 43 +++++++++++++++++++++++++++++++++++++------ 2 files changed, 47 insertions(+), 7 deletions(-)
Well, these mandate both kernel and user-side changes, so if you run the old applications with old alsa-lib, it can't work, right?
I see now that you achieved it by dropping the whole mmap.
Just for drivers/hardwares with the new flag. I won't disable page frame mapping of control/status data indiscriminately for drivers with .ack callback.
This is a very big concern from the performance POV. It merely kills the whole merit of adding the appl_ptr notification (e.g. PA switches to a completely different mode), so this is likely no-go, sorry.
Old stuffs were not programmed to assist the new devices/drivers. It's quite natural to get a constrain to its working mode for several drivers/devices.
I focus on describing hardware/driver capabilities in interface for user land reasonably, instead of introducing them ex post facto.
...Well, I've already done for my proposal to this issue. Nothing left. I think you can understand what I insists for this issue and my concerns about your patch.
If you updates change log in the patch and post it, I'm willing to review it. I don't oppose it so hard (I understand the intention correctly), however it's important to pay enough attention to interfaces for applications. If the new stuffs bring changes to interface and applications get affects, we, developers for kernel land, should record and explain about it.
Regards
Takashi Sakamoto
On Mon, 19 Jun 2017 14:54:12 +0200, Takashi Sakamoto wrote:
On Jun 19 2017 02:17, Takashi Iwai wrote:
On Sun, 18 Jun 2017 19:08:05 +0200, Takashi Iwai wrote:
On Sun, 18 Jun 2017 18:49:18 +0200, Takashi Sakamoto wrote:
Hi,
This patchset includes commits for both kernel/user land for an issue addressed in this message thread[1].
In a development period for v4.13, it's clear that there's a type of devices which demand userspace applications to change their behaviour. These devices can take care of application-side pointer on PCM buffer for their data transmission. As a result, these devices achieve some advantages. For these devices, drivers need to get feedback about current position of the pointer from applications when they queue/dequeue PCM frames on the buffer.
ALSA PCM core partly assist the above design. The core can call a handler in driver side when applications operate PCM frames by ioctl(2) with SNDRV_PCM_IOCTL_[READ|WRITE][N|I] command. However, when operating for mapped PCM buffer, it can't assist applications for the above design. In this scenario, applications should call ioctl(2) with SNDRV_PCM_IOCTL_SYNC_PTR after handle any PCM frames on the buffer.
This patchset bumps up interface version with new info flag to notify the above change to user land developers. This patchset is written with below ideas:
- Stuffs in user land can tell their assists for the above design to kernel land by using hw_param flag. When they don't assist, they see on hw_params data that mmap operation is not supported.
- Drivers can acknowledge the above flag. When registering appropriate PCM rules in advance, they can calculate values for each parameters in both cases that stuffs in user land can/cannot assist it. This is useful for packet-oriented drivers to judge utilizing any interrupt context for packetization.
- This is device-specific issue, independently of platform architecture. On x86/ppc/alpha platforms, status/control data of PCM substream is still available in process' VMA of application via page frame mapping.
[1] http://mailman.alsa-project.org/pipermail/alsa-devel/2017-June/121438.html
For kernel land: Takashi Sakamoto (1): ALSA: pcm: introduce ACK_APPLPTR flag and bump up interface version to 2.0.14
include/uapi/sound/asound.h | 11 ++++++++++- sound/core/pcm_native.c | 25 +++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-)
For user land: Takashi Sakamoto (2): ALSA: pcm: introduce ACK_APPLPTR flag and bump up interface version to 2.0.14 pcm: add support for interface version 2.0.14
include/sound/asound.h | 11 ++++++++++- src/pcm/pcm_hw.c | 43 +++++++++++++++++++++++++++++++++++++------ 2 files changed, 47 insertions(+), 7 deletions(-)
Well, these mandate both kernel and user-side changes, so if you run the old applications with old alsa-lib, it can't work, right?
I see now that you achieved it by dropping the whole mmap.
Just for drivers/hardwares with the new flag. I won't disable page frame mapping of control/status data indiscriminately for drivers with .ack callback.
This is a very big concern from the performance POV. It merely kills the whole merit of adding the appl_ptr notification (e.g. PA switches to a completely different mode), so this is likely no-go, sorry.
Old stuffs were not programmed to assist the new devices/drivers. It's quite natural to get a constrain to its working mode for several drivers/devices.
If there is no other way, I'd agree with such a change. But in this case, this isn't.
I focus on describing hardware/driver capabilities in interface for user land reasonably, instead of introducing them ex post facto.
...Well, I've already done for my proposal to this issue. Nothing left. I think you can understand what I insists for this issue and my concerns about your patch.
I'm not a big fan of my proposed change, either. It's not very intuitive if you don't know of the mechanism behind the scene. But, judging from the practice (as a similar technique already deployed, also on x86 for long time), it's a sensible solution.
If you updates change log in the patch and post it, I'm willing to review it. I don't oppose it so hard (I understand the intention correctly), however it's important to pay enough attention to interfaces for applications. If the new stuffs bring changes to interface and applications get affects, we, developers for kernel land, should record and explain about it.
Sure, the revised patchset will be post later.
thanks,
Takashi
On 2017年06月19日 02:08, Takashi Iwai wrote:
On Sun, 18 Jun 2017 18:49:18 +0200, Takashi Sakamoto wrote:
Hi,
This patchset includes commits for both kernel/user land for an issue addressed in this message thread[1].
In a development period for v4.13, it's clear that there's a type of devices which demand userspace applications to change their behaviour. These devices can take care of application-side pointer on PCM buffer for their data transmission. As a result, these devices achieve some advantages. For these devices, drivers need to get feedback about current position of the pointer from applications when they queue/dequeue PCM frames on the buffer.
ALSA PCM core partly assist the above design. The core can call a handler in driver side when applications operate PCM frames by ioctl(2) with SNDRV_PCM_IOCTL_[READ|WRITE][N|I] command. However, when operating for mapped PCM buffer, it can't assist applications for the above design. In this scenario, applications should call ioctl(2) with SNDRV_PCM_IOCTL_SYNC_PTR after handle any PCM frames on the buffer.
This patchset bumps up interface version with new info flag to notify the above change to user land developers. This patchset is written with below ideas:
- Stuffs in user land can tell their assists for the above design to kernel land by using hw_param flag. When they don't assist, they see on hw_params data that mmap operation is not supported.
- Drivers can acknowledge the above flag. When registering appropriate PCM rules in advance, they can calculate values for each parameters in both cases that stuffs in user land can/cannot assist it. This is useful for packet-oriented drivers to judge utilizing any interrupt context for packetization.
- This is device-specific issue, independently of platform architecture. On x86/ppc/alpha platforms, status/control data of PCM substream is still available in process' VMA of application via page frame mapping.
[1] http://mailman.alsa-project.org/pipermail/alsa-devel/2017-June/121438.html
For kernel land: Takashi Sakamoto (1): ALSA: pcm: introduce ACK_APPLPTR flag and bump up interface version to 2.0.14
include/uapi/sound/asound.h | 11 ++++++++++- sound/core/pcm_native.c | 25 +++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-)
For user land: Takashi Sakamoto (2): ALSA: pcm: introduce ACK_APPLPTR flag and bump up interface version to 2.0.14 pcm: add support for interface version 2.0.14
include/sound/asound.h | 11 ++++++++++- src/pcm/pcm_hw.c | 43 +++++++++++++++++++++++++++++++++++++------ 2 files changed, 47 insertions(+), 7 deletions(-)
Well, these mandate both kernel and user-side changes, so if you run the old applications with old alsa-lib, it can't work, right?
Applications still works well because they receive proper set of hw_params. They works as programmed, thus for the new devices/drivers they work without mmap operation.
A similar patchset is already in my local queue (my version called it differently, SNDRV_PCM_INFO_SYNC_APPLPTR :), but it wasn't submitted yet because we wanted to fix the issue without affecting alsa-lib at first. Do you have the patch for that?
I will not have this direction, past and future, because this issue demands userland stuffs to perform as the new drivers/devices expects, thus userland stuffs _should_ be changed.
# When I investigated name of the flag, I dropped # 'SNDRV_PCM_INFO_SYNC_APPLPTR' at first, because hardware doesn't # need to synchronize to applptr always. It's enough for hardwares to # get it when requires. Drivers and applications assist it.
Regards
Takashi Sakamoto
On Mon, 19 Jun 2017 14:33:50 +0200, Takashi Sakamoto wrote:
On 2017年06月19日 02:08, Takashi Iwai wrote:
On Sun, 18 Jun 2017 18:49:18 +0200, Takashi Sakamoto wrote:
Hi,
This patchset includes commits for both kernel/user land for an issue addressed in this message thread[1].
In a development period for v4.13, it's clear that there's a type of devices which demand userspace applications to change their behaviour. These devices can take care of application-side pointer on PCM buffer for their data transmission. As a result, these devices achieve some advantages. For these devices, drivers need to get feedback about current position of the pointer from applications when they queue/dequeue PCM frames on the buffer.
ALSA PCM core partly assist the above design. The core can call a handler in driver side when applications operate PCM frames by ioctl(2) with SNDRV_PCM_IOCTL_[READ|WRITE][N|I] command. However, when operating for mapped PCM buffer, it can't assist applications for the above design. In this scenario, applications should call ioctl(2) with SNDRV_PCM_IOCTL_SYNC_PTR after handle any PCM frames on the buffer.
This patchset bumps up interface version with new info flag to notify the above change to user land developers. This patchset is written with below ideas:
- Stuffs in user land can tell their assists for the above design to kernel land by using hw_param flag. When they don't assist, they see on hw_params data that mmap operation is not supported.
- Drivers can acknowledge the above flag. When registering appropriate PCM rules in advance, they can calculate values for each parameters in both cases that stuffs in user land can/cannot assist it. This is useful for packet-oriented drivers to judge utilizing any interrupt context for packetization.
- This is device-specific issue, independently of platform architecture. On x86/ppc/alpha platforms, status/control data of PCM substream is still available in process' VMA of application via page frame mapping.
[1] http://mailman.alsa-project.org/pipermail/alsa-devel/2017-June/121438.html
For kernel land: Takashi Sakamoto (1): ALSA: pcm: introduce ACK_APPLPTR flag and bump up interface version to 2.0.14
include/uapi/sound/asound.h | 11 ++++++++++- sound/core/pcm_native.c | 25 +++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-)
For user land: Takashi Sakamoto (2): ALSA: pcm: introduce ACK_APPLPTR flag and bump up interface version to 2.0.14 pcm: add support for interface version 2.0.14
include/sound/asound.h | 11 ++++++++++- src/pcm/pcm_hw.c | 43 +++++++++++++++++++++++++++++++++++++------ 2 files changed, 47 insertions(+), 7 deletions(-)
Well, these mandate both kernel and user-side changes, so if you run the old applications with old alsa-lib, it can't work, right?
Applications still works well because they receive proper set of hw_params. They works as programmed, thus for the new devices/drivers they work without mmap operation.
This solution is obviously worse from most of perspectives (the performance, the possible regressions).
A similar patchset is already in my local queue (my version called it differently, SNDRV_PCM_INFO_SYNC_APPLPTR :), but it wasn't submitted yet because we wanted to fix the issue without affecting alsa-lib at first. Do you have the patch for that?
I will not have this direction, past and future, because this issue demands userland stuffs to perform as the new drivers/devices expects, thus userland stuffs _should_ be changed.
Have you ever heard of a distribution named "Debian"? Wait for another 10 years until the fix in a newer alsa-lib version will be distributed as stable :)
thanks,
Takashi
participants (2)
-
Takashi Iwai -
Takashi Sakamoto