* [PATCH 0/3] Add SPIB Support for Intel Skylake platforms
@ 2018-01-30 9:36 Sriram Periyasamy
2018-01-30 9:36 ` [PATCH 1/3] ALSA: core: let low-level driver or userspace disable rewinds Sriram Periyasamy
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Sriram Periyasamy @ 2018-01-30 9:36 UTC (permalink / raw)
To: ALSA ML, Mark Brown
Cc: Takashi Iwai, Sriram Periyasamy, Takashi Sakamoto, Liam Girdwood,
Patches Audio, Vinod Koul
Skylake audio controller supports SPIB (Software Position in buffer)
capability, which can be used to inform position of application pointer
to host DMA controller. When SPIB mode is enabled, driver could write
the application pointer position in SPIB register. Host DMA will make
sure it won't read/write beyond bytes specified in SPIB register.
SPIB mode will be useful in low power use cases, where DSP could
pre-fetch large buffers to avoid frequent wakes caused due to interrupts.
To support SPIB in the driver, save the spib values in stream context
which can be restored during resume from S3. Add new hw_params flag to
explicitly tell driver that rewinds will never be used.
Pierre-Louis Bossart (1):
ALSA: core: let low-level driver or userspace disable rewinds
Ramesh Babu (2):
ALSA: hda: ext: add spib to stream context
ASoC: Intel: Skylake: Add support for spib mode
include/sound/hdaudio_ext.h | 1 +
include/sound/pcm.h | 1 +
include/uapi/sound/asound.h | 1 +
sound/core/pcm_native.c | 8 ++++++++
sound/hda/ext/hdac_ext_stream.c | 2 ++
sound/soc/intel/skylake/skl-pcm.c | 43 ++++++++++++++++++++++++++++++++++++++-
6 files changed, 55 insertions(+), 1 deletion(-)
--
2.7.4
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH 1/3] ALSA: core: let low-level driver or userspace disable rewinds 2018-01-30 9:36 [PATCH 0/3] Add SPIB Support for Intel Skylake platforms Sriram Periyasamy @ 2018-01-30 9:36 ` Sriram Periyasamy 2018-01-30 9:36 ` [PATCH 2/3] ALSA: hda: ext: add spib to stream context Sriram Periyasamy ` (2 subsequent siblings) 3 siblings, 0 replies; 10+ messages in thread From: Sriram Periyasamy @ 2018-01-30 9:36 UTC (permalink / raw) To: ALSA ML, Mark Brown Cc: Takashi Iwai, Sriram Periyasamy, Pierre-Louis Bossart, Ramesh Babu, Takashi Sakamoto, Liam Girdwood, Patches Audio, Vinod Koul, Subhransu S . Prusty From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Add new hw_params flag to explicitly tell driver that rewinds will never be used. This can be used by low-level driver to optimize DMA operations and reduce power consumption. Use this flag only when data written in ring buffer will never be invalidated, e.g. any update of appl_ptr is final. Note that the update of appl_ptr include both a read/write data operation as well as snd_pcm_forward() whose behavior is not modified. Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Signed-off-by: Ramesh Babu <ramesh.babu@intel.com> Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com> Signed-off-by: Sriram Periyasamy <sriramx.periyasamy@intel.com> --- include/sound/pcm.h | 1 + include/uapi/sound/asound.h | 1 + sound/core/pcm_native.c | 8 ++++++++ 3 files changed, 10 insertions(+) diff --git a/include/sound/pcm.h b/include/sound/pcm.h index e054c583d3b3..f60397eb4aab 100644 --- a/include/sound/pcm.h +++ b/include/sound/pcm.h @@ -379,6 +379,7 @@ struct snd_pcm_runtime { unsigned int rate_num; unsigned int rate_den; unsigned int no_period_wakeup: 1; + unsigned int no_rewinds:1; /* -- SW params -- */ int tstamp_mode; /* mmap timestamp is updated */ diff --git a/include/uapi/sound/asound.h b/include/uapi/sound/asound.h index 07d61583fd02..8ead564cee7c 100644 --- a/include/uapi/sound/asound.h +++ b/include/uapi/sound/asound.h @@ -376,6 +376,7 @@ 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 */ +#define SNDRV_PCM_HW_PARAMS_NO_REWINDS (1<<3) /* disable rewinds */ struct snd_interval { unsigned int min, max; diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index f08772568c17..ae356cef1cbd 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -692,6 +692,8 @@ static int snd_pcm_hw_params(struct snd_pcm_substream *substream, runtime->no_period_wakeup = (params->info & SNDRV_PCM_INFO_NO_PERIOD_WAKEUP) && (params->flags & SNDRV_PCM_HW_PARAMS_NO_PERIOD_WAKEUP); + runtime->no_rewinds = + (params->flags & SNDRV_PCM_HW_PARAMS_NO_REWINDS) ? 1 : 0; bits = snd_pcm_format_physical_width(runtime->format); runtime->sample_bits = bits; @@ -2614,6 +2616,9 @@ static snd_pcm_sframes_t snd_pcm_playback_rewind(struct snd_pcm_substream *subst if (frames == 0) return 0; + if (runtime->no_rewinds) + return -ENODEV; + snd_pcm_stream_lock_irq(substream); ret = do_pcm_hwsync(substream); if (!ret) @@ -2632,6 +2637,9 @@ static snd_pcm_sframes_t snd_pcm_capture_rewind(struct snd_pcm_substream *substr if (frames == 0) return 0; + if (runtime->no_rewinds) + return -ENODEV; + snd_pcm_stream_lock_irq(substream); ret = do_pcm_hwsync(substream); if (!ret) -- 2.7.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/3] ALSA: hda: ext: add spib to stream context 2018-01-30 9:36 [PATCH 0/3] Add SPIB Support for Intel Skylake platforms Sriram Periyasamy 2018-01-30 9:36 ` [PATCH 1/3] ALSA: core: let low-level driver or userspace disable rewinds Sriram Periyasamy @ 2018-01-30 9:36 ` Sriram Periyasamy 2018-01-30 9:36 ` [PATCH 3/3] ASoC: Intel: Skylake: Add support for spib mode Sriram Periyasamy 2018-01-30 10:38 ` [PATCH 0/3] Add SPIB Support for Intel Skylake platforms Takashi Sakamoto 3 siblings, 0 replies; 10+ messages in thread From: Sriram Periyasamy @ 2018-01-30 9:36 UTC (permalink / raw) To: ALSA ML, Mark Brown Cc: Takashi Iwai, Sriram Periyasamy, Ramesh Babu, Takashi Sakamoto, Liam Girdwood, Patches Audio, Vinod Koul, Subhransu S . Prusty From: Ramesh Babu <ramesh.babu@intel.com> Platforms like skylake support SPIB (software position index in Buffer) capability, through which application pointer can be programmed in DMA. This helps DMA stop rendering stale data. This patch saves spib values in stream context which can be restored during resume from S3. Signed-off-by: Ramesh Babu <ramesh.babu@intel.com> Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com> Signed-off-by: Sriram Periyasamy <sriramx.periyasamy@intel.com> --- include/sound/hdaudio_ext.h | 1 + sound/hda/ext/hdac_ext_stream.c | 2 ++ 2 files changed, 3 insertions(+) diff --git a/include/sound/hdaudio_ext.h b/include/sound/hdaudio_ext.h index 9c14e21dda85..34c41496fbc7 100644 --- a/include/sound/hdaudio_ext.h +++ b/include/sound/hdaudio_ext.h @@ -89,6 +89,7 @@ struct hdac_ext_stream { u32 dpib; u32 lpib; + u32 spib; bool decoupled:1; bool link_locked:1; bool link_prepared; diff --git a/sound/hda/ext/hdac_ext_stream.c b/sound/hda/ext/hdac_ext_stream.c index c96d7a7a36af..c5212709bdb7 100644 --- a/sound/hda/ext/hdac_ext_stream.c +++ b/sound/hda/ext/hdac_ext_stream.c @@ -461,6 +461,8 @@ int snd_hdac_ext_stream_set_spib(struct hdac_ext_bus *ebus, } writel(value, stream->spib_addr); + /* save the spib value in stream context */ + stream->spib = value; return 0; } -- 2.7.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 3/3] ASoC: Intel: Skylake: Add support for spib mode 2018-01-30 9:36 [PATCH 0/3] Add SPIB Support for Intel Skylake platforms Sriram Periyasamy 2018-01-30 9:36 ` [PATCH 1/3] ALSA: core: let low-level driver or userspace disable rewinds Sriram Periyasamy 2018-01-30 9:36 ` [PATCH 2/3] ALSA: hda: ext: add spib to stream context Sriram Periyasamy @ 2018-01-30 9:36 ` Sriram Periyasamy 2018-01-30 10:38 ` [PATCH 0/3] Add SPIB Support for Intel Skylake platforms Takashi Sakamoto 3 siblings, 0 replies; 10+ messages in thread From: Sriram Periyasamy @ 2018-01-30 9:36 UTC (permalink / raw) To: ALSA ML, Mark Brown Cc: Takashi Iwai, Sriram Periyasamy, Ramesh Babu, Takashi Sakamoto, Liam Girdwood, Patches Audio, Sanyog Kale, Vinod Koul, Subhransu S . Prusty From: Ramesh Babu <ramesh.babu@intel.com> Skylake audio controller supports SPIB (Software Position in buffer) capability, which can be used to inform position of application pointer to host DMA controller. When SPIB mode is enabled, driver could write the application pointer position in SPIB register. Host DMA will make sure it won't read/write beyond bytes specified in SPIB register. SPIB mode will be useful in low power use cases, where DSP could pre-fetch large buffers to avoid frequent wakes caused due to interrupts. Skylake driver makes use of no_rewind flag and appl_ptr_update callback to enable and update SPIB register respectively. Signed-off-by: Ramesh Babu <ramesh.babu@intel.com> Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com> Signed-off-by: Sanyog Kale <sanyog.r.kale@intel.com> Signed-off-by: Sriram Periyasamy <sriramx.periyasamy@intel.com> --- sound/soc/intel/skylake/skl-pcm.c | 43 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 42 insertions(+), 1 deletion(-) diff --git a/sound/soc/intel/skylake/skl-pcm.c b/sound/soc/intel/skylake/skl-pcm.c index df824224261e..346f9ac8053b 100644 --- a/sound/soc/intel/skylake/skl-pcm.c +++ b/sound/soc/intel/skylake/skl-pcm.c @@ -43,7 +43,8 @@ static const struct snd_pcm_hardware azx_pcm_hw = { SNDRV_PCM_INFO_SYNC_START | SNDRV_PCM_INFO_HAS_WALL_CLOCK | /* legacy */ SNDRV_PCM_INFO_HAS_LINK_ATIME | - SNDRV_PCM_INFO_NO_PERIOD_WAKEUP), + SNDRV_PCM_INFO_NO_PERIOD_WAKEUP | + SNDRV_PCM_INFO_SYNC_APPLPTR), .formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S32_LE | SNDRV_PCM_FMTBIT_S24_LE, @@ -145,6 +146,7 @@ int skl_pcm_host_dma_prepare(struct device *dev, struct skl_pipe_params *params) unsigned int format_val; struct hdac_stream *hstream; struct hdac_ext_stream *stream; + struct snd_pcm_runtime *runtime; int err; hstream = snd_hdac_get_stream(bus, params->stream, @@ -170,6 +172,11 @@ int skl_pcm_host_dma_prepare(struct device *dev, struct skl_pipe_params *params) if (err < 0) return err; + /* Enable SPIB if no_rewinds flag is set */ + runtime = hdac_stream(stream)->substream->runtime; + if (runtime->no_rewinds) + snd_hdac_ext_stream_spbcap_enable(ebus, true, hstream->index); + hdac_stream(stream)->prepared = 1; return 0; @@ -366,9 +373,14 @@ static int skl_pcm_hw_free(struct snd_pcm_substream *substream, { struct hdac_ext_bus *ebus = dev_get_drvdata(dai->dev); struct hdac_ext_stream *stream = get_hdac_ext_stream(substream); + struct snd_pcm_runtime *runtime = substream->runtime; + struct hdac_stream *hstream = hdac_stream(stream); dev_dbg(dai->dev, "%s: %s\n", __func__, dai->name); + if (runtime->no_rewinds) + snd_hdac_ext_stream_spbcap_enable(ebus, false, hstream->index); + snd_hdac_stream_cleanup(hdac_stream(stream)); hdac_stream(stream)->prepared = 0; @@ -444,6 +456,7 @@ static int skl_pcm_trigger(struct snd_pcm_substream *substream, int cmd, struct skl_module_cfg *mconfig; struct hdac_ext_bus *ebus = get_bus_ctx(substream); struct hdac_ext_stream *stream = get_hdac_ext_stream(substream); + struct snd_pcm_runtime *runtime = substream->runtime; struct snd_soc_dapm_widget *w; int ret; @@ -469,6 +482,10 @@ static int skl_pcm_trigger(struct snd_pcm_substream *substream, int cmd, snd_hdac_ext_stream_set_dpibr(ebus, stream, stream->lpib); snd_hdac_ext_stream_set_lpib(stream, stream->lpib); + + if (runtime->no_rewinds) + snd_hdac_ext_stream_set_spib(ebus, + stream, stream->spib); } case SNDRV_PCM_TRIGGER_START: @@ -1095,6 +1112,29 @@ static int skl_platform_pcm_trigger(struct snd_pcm_substream *substream, return 0; } +/* Update SPIB register with application position */ +static int skl_platform_ack(struct snd_pcm_substream *substream) +{ + struct hdac_ext_stream *estream = get_hdac_ext_stream(substream); + struct snd_pcm_runtime *runtime = substream->runtime; + struct hdac_ext_bus *ebus = get_bus_ctx(substream); + ssize_t appl_pos, buf_size; + u32 spib; + + /* Use spib mode only if no_rewind mode is set */ + if (!runtime->no_rewinds) + return 0; + + appl_pos = frames_to_bytes(runtime, runtime->control->appl_ptr); + buf_size = frames_to_bytes(runtime, runtime->buffer_size); + + spib = appl_pos % buf_size; + + /* Allowable value for SPIB is 1 byte to max buffer size */ + spib = (spib == 0) ? buf_size : spib; + return snd_hdac_ext_stream_set_spib(ebus, estream, spib); +} + static snd_pcm_uframes_t skl_platform_pcm_pointer (struct snd_pcm_substream *substream) { @@ -1202,6 +1242,7 @@ static const struct snd_pcm_ops skl_platform_ops = { .get_time_info = skl_get_time_info, .mmap = snd_pcm_lib_default_mmap, .page = snd_pcm_sgbuf_ops_page, + .ack = skl_platform_ack, }; static void skl_pcm_free(struct snd_pcm *pcm) -- 2.7.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 0/3] Add SPIB Support for Intel Skylake platforms 2018-01-30 9:36 [PATCH 0/3] Add SPIB Support for Intel Skylake platforms Sriram Periyasamy ` (2 preceding siblings ...) 2018-01-30 9:36 ` [PATCH 3/3] ASoC: Intel: Skylake: Add support for spib mode Sriram Periyasamy @ 2018-01-30 10:38 ` Takashi Sakamoto 2018-01-30 11:07 ` Subhransu S. Prusty 3 siblings, 1 reply; 10+ messages in thread From: Takashi Sakamoto @ 2018-01-30 10:38 UTC (permalink / raw) To: Sriram Periyasamy, ALSA ML, Mark Brown Cc: Takashi Iwai, Liam Girdwood, Vinod Koul, Patches Audio Hi, On Jan 30 2018 18:36, Sriram Periyasamy wrote: > Skylake audio controller supports SPIB (Software Position in buffer) > capability, which can be used to inform position of application pointer > to host DMA controller. When SPIB mode is enabled, driver could write > the application pointer position in SPIB register. Host DMA will make > sure it won't read/write beyond bytes specified in SPIB register. > > SPIB mode will be useful in low power use cases, where DSP could > pre-fetch large buffers to avoid frequent wakes caused due to interrupts. > > To support SPIB in the driver, save the spib values in stream context > which can be restored during resume from S3. Add new hw_params flag to > explicitly tell driver that rewinds will never be used. > > Pierre-Louis Bossart (1): > ALSA: core: let low-level driver or userspace disable rewinds > > Ramesh Babu (2): > ALSA: hda: ext: add spib to stream context > ASoC: Intel: Skylake: Add support for spib mode > > include/sound/hdaudio_ext.h | 1 + > include/sound/pcm.h | 1 + > include/uapi/sound/asound.h | 1 + > sound/core/pcm_native.c | 8 ++++++++ > sound/hda/ext/hdac_ext_stream.c | 2 ++ > sound/soc/intel/skylake/skl-pcm.c | 43 ++++++++++++++++++++++++++++++++++++++- > 6 files changed, 55 insertions(+), 1 deletion(-) In my opinion, when drivers return appropriate values at implementations of "struct snd_pcm_ops.pointer" and "struct snd_pcm_ops.ack", your aim is satisfied. In short, you can let ALSA PCM core to handle rewinding/forwarding requests from userland for zero number of handled frames in result. So the 'SNDRV_PCM_HW_PARAMS_NO_REWINDS' flag is useless. >From me, please refer to our previous discussion about this flag[1][2][3], then describe your insistence of this flag. At least, it's not better idea to abandon the old discussion when posting this kind of patches. Additionally you should add 'v4' in title of this patchset to represent this patchset is a part of series of your work for the flag and your Intel platform. [1] http://mailman.alsa-project.org/pipermail/alsa-devel/2017-May/120676.html [2] http://mailman.alsa-project.org/pipermail/alsa-devel/2017-June/121683.html [3] http://mailman.alsa-project.org/pipermail/alsa-devel/2017-June/121967.html Regards Takashi Sakamoto ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/3] Add SPIB Support for Intel Skylake platforms 2018-01-30 10:38 ` [PATCH 0/3] Add SPIB Support for Intel Skylake platforms Takashi Sakamoto @ 2018-01-30 11:07 ` Subhransu S. Prusty 2018-01-30 11:44 ` Vinod Koul 0 siblings, 1 reply; 10+ messages in thread From: Subhransu S. Prusty @ 2018-01-30 11:07 UTC (permalink / raw) To: Takashi Sakamoto Cc: ALSA ML, Takashi Iwai, Periyasamy, SriramX, Patches Audio, Liam Girdwood, Koul, Vinod, Mark Brown On Tue, Jan 30, 2018 at 04:08:39PM +0530, Takashi Sakamoto wrote: > Hi, > > On Jan 30 2018 18:36, Sriram Periyasamy wrote: > > Skylake audio controller supports SPIB (Software Position in buffer) > > capability, which can be used to inform position of application pointer > > to host DMA controller. When SPIB mode is enabled, driver could write > > the application pointer position in SPIB register. Host DMA will make > > sure it won't read/write beyond bytes specified in SPIB register. > > > > SPIB mode will be useful in low power use cases, where DSP could > > pre-fetch large buffers to avoid frequent wakes caused due to interrupts. > > > > To support SPIB in the driver, save the spib values in stream context > > which can be restored during resume from S3. Add new hw_params flag to > > explicitly tell driver that rewinds will never be used. > > > > Pierre-Louis Bossart (1): > > ALSA: core: let low-level driver or userspace disable rewinds > > > > Ramesh Babu (2): > > ALSA: hda: ext: add spib to stream context > > ASoC: Intel: Skylake: Add support for spib mode > > > > include/sound/hdaudio_ext.h | 1 + > > include/sound/pcm.h | 1 + > > include/uapi/sound/asound.h | 1 + > > sound/core/pcm_native.c | 8 ++++++++ > > sound/hda/ext/hdac_ext_stream.c | 2 ++ > > sound/soc/intel/skylake/skl-pcm.c | 43 ++++++++++++++++++++++++++++++++++++++- > > 6 files changed, 55 insertions(+), 1 deletion(-) > > In my opinion, when drivers > return appropriate values at implementations of > "struct snd_pcm_ops.pointer" and "struct snd_pcm_ops.ack", your aim is > satisfied. In short, you can let ALSA PCM core to handle > rewinding/forwarding requests from userland for zero number of handled > frames in result. So the 'SNDRV_PCM_HW_PARAMS_NO_REWINDS' flag is > useless. Based on the earlier discussion in v3, this series includes the usage of the 'SNDRV_PCM_HW_PARAMS_NO_REWINDS' flag. Please refer to the discussion on https://patchwork.kernel.org/patch/9795233/ > > From me, please refer to our previous discussion about this > flag[1][2][3], then describe your insistence of this flag. At least, > it's not better idea to abandon the old discussion when posting this > kind of patches. Additionally you should add 'v4' in title of this Yes should have added the reference and v4. Sorry to have missed it. Regards, Subhransu > patchset to represent this patchset is a part of series of your work for > the flag and your Intel platform. > > [1] > http://mailman.alsa-project.org/pipermail/alsa-devel/2017-May/120676.html > [2] > http://mailman.alsa-project.org/pipermail/alsa-devel/2017-June/121683.html > [3] > http://mailman.alsa-project.org/pipermail/alsa-devel/2017-June/121967.html > > > Regards > > Takashi Sakamoto > _______________________________________________ > Alsa-devel mailing list > Alsa-devel@alsa-project.org > http://mailman.alsa-project.org/mailman/listinfo/alsa-devel -- ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/3] Add SPIB Support for Intel Skylake platforms 2018-01-30 11:07 ` Subhransu S. Prusty @ 2018-01-30 11:44 ` Vinod Koul 0 siblings, 0 replies; 10+ messages in thread From: Vinod Koul @ 2018-01-30 11:44 UTC (permalink / raw) To: Takashi Sakamoto Cc: ALSA ML, Takashi Iwai, Periyasamy, SriramX, Liam Girdwood, Patches Audio, Mark Brown, Subhransu S. Prusty On Tue, Jan 30, 2018 at 04:37:44PM +0530, Subhransu S. Prusty wrote: > On Tue, Jan 30, 2018 at 04:08:39PM +0530, Takashi Sakamoto wrote: > > Hi, Hi Takashi Sakamoto, > > In my opinion, when drivers > > return appropriate values at implementations of > > "struct snd_pcm_ops.pointer" and "struct snd_pcm_ops.ack", your aim is > > satisfied. In short, you can let ALSA PCM core to handle > > rewinding/forwarding requests from userland for zero number of handled > > frames in result. So the 'SNDRV_PCM_HW_PARAMS_NO_REWINDS' flag is > > useless. > > Based on the earlier discussion in v3, this series includes the usage of the > 'SNDRV_PCM_HW_PARAMS_NO_REWINDS' flag. Please refer to the discussion on > https://patchwork.kernel.org/patch/9795233/ Also it is worth mentioning that this supports a HW feature which requires the knowledge of data available in ring buffer to be provided to hardware. With the feature enabled, we cannot rewind/forward, hence the support is dependent upon application querying about no rewind capability and setting it, otherwise this feature is not enabled... > > > > > From me, please refer to our previous discussion about this > > flag[1][2][3], then describe your insistence of this flag. At least, > > it's not better idea to abandon the old discussion when posting this > > kind of patches. Additionally you should add 'v4' in title of this > > Yes should have added the reference and v4. Sorry to have missed it. -- ~Vinod ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 0/3] ALSA: Add rewind disable support
@ 2017-05-16 1:01 Subhransu S. Prusty
2017-05-16 1:01 ` [PATCH 1/3] ALSA: core: let low-level driver or userspace disable rewinds Subhransu S. Prusty
0 siblings, 1 reply; 10+ messages in thread
From: Subhransu S. Prusty @ 2017-05-16 1:01 UTC (permalink / raw)
To: alsa-devel; +Cc: tiwai, patches.audio, broonie, Subhransu S. Prusty, lgirdwood
Rewinds can be disabled when data written in ring buffer will never be
validated. This allows for new HDaudio SPIB DMA functionality(allow fetch
up to the application pointer, no rewind supported). Skylake driver
changes using SPIB will be posted once the core changes are merged.
Based on discussion on community, .ack callback is extended with new
attribute argument, to allow fetch up to the application pointer. Also
drop the mmap of status and control if SPIB functionality is reported by
driver. There may be some slight performance downside, but it's likely
very small.
Verified the changes with tinyalsa and alsa-lib, with both mmap and
without mmap support and it works fine.
Pierre-Louis Bossart (3):
ALSA: core: let low-level driver or userspace disable rewinds
ALSA: core: modify .ack callback to take arguments for updating appl
ptr
ALSA: pcm: conditionally avoid mmap of control data
include/sound/pcm-indirect.h | 4 ++--
include/sound/pcm.h | 9 +++++++-
include/uapi/sound/asound.h | 2 ++
sound/core/pcm_lib.c | 6 ++++--
sound/core/pcm_native.c | 49 ++++++++++++++++++++++++++++++++++++++++++-
sound/mips/hal2.c | 14 ++++++++++---
sound/pci/cs46xx/cs46xx_lib.c | 18 ++++++++++++----
sound/pci/emu10k1/emupcm.c | 8 +++++--
sound/pci/rme32.c | 15 ++++++++++---
9 files changed, 107 insertions(+), 18 deletions(-)
--
1.9.1
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH 1/3] ALSA: core: let low-level driver or userspace disable rewinds 2017-05-16 1:01 [PATCH 0/3] ALSA: Add rewind disable support Subhransu S. Prusty @ 2017-05-16 1:01 ` Subhransu S. Prusty 2017-05-16 5:53 ` Takashi Iwai 0 siblings, 1 reply; 10+ messages in thread From: Subhransu S. Prusty @ 2017-05-16 1:01 UTC (permalink / raw) To: alsa-devel Cc: tiwai, lgirdwood, Ramesh Babu, Pierre-Louis Bossart, patches.audio, broonie, Subhransu S. Prusty From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Add new hw_params flag to explicitly tell driver that rewinds will never be used. This can be used by low-level driver to optimize DMA operations and reduce power consumption. Use this flag only when data written in ring buffer will never be invalidated, e.g. any update of appl_ptr is final. Note that the update of appl_ptr include both a read/write data operation as well as snd_pcm_forward() whose behavior is not modified. Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Signed-off-by: Ramesh Babu <ramesh.babu@intel.com> Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com> --- include/sound/pcm.h | 1 + include/uapi/sound/asound.h | 1 + sound/core/pcm_native.c | 8 ++++++++ 3 files changed, 10 insertions(+) diff --git a/include/sound/pcm.h b/include/sound/pcm.h index 361749e60799..a2682c5f5b72 100644 --- a/include/sound/pcm.h +++ b/include/sound/pcm.h @@ -368,6 +368,7 @@ struct snd_pcm_runtime { unsigned int rate_num; unsigned int rate_den; unsigned int no_period_wakeup: 1; + unsigned int no_rewinds:1; /* -- SW params -- */ int tstamp_mode; /* mmap timestamp is updated */ diff --git a/include/uapi/sound/asound.h b/include/uapi/sound/asound.h index fd41697cb4d3..c697ff90450d 100644 --- a/include/uapi/sound/asound.h +++ b/include/uapi/sound/asound.h @@ -365,6 +365,7 @@ struct snd_pcm_info { #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 */ +#define SNDRV_PCM_HW_PARAMS_NO_REWINDS (1<<3) /* disable rewinds */ struct snd_interval { unsigned int min, max; diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index 13dec5ec93f2..35dd4ca93f84 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -566,6 +566,8 @@ static int snd_pcm_hw_params(struct snd_pcm_substream *substream, runtime->no_period_wakeup = (params->info & SNDRV_PCM_INFO_NO_PERIOD_WAKEUP) && (params->flags & SNDRV_PCM_HW_PARAMS_NO_PERIOD_WAKEUP); + runtime->no_rewinds = + (params->flags & SNDRV_PCM_HW_PARAMS_NO_REWINDS) ? 1 : 0; bits = snd_pcm_format_physical_width(runtime->format); runtime->sample_bits = bits; @@ -2439,6 +2441,9 @@ static snd_pcm_sframes_t snd_pcm_playback_rewind(struct snd_pcm_substream *subst if (frames == 0) return 0; + if (runtime->no_rewinds) + return 0; + snd_pcm_stream_lock_irq(substream); switch (runtime->status->state) { case SNDRV_PCM_STATE_PREPARED: @@ -2487,6 +2492,9 @@ static snd_pcm_sframes_t snd_pcm_capture_rewind(struct snd_pcm_substream *substr if (frames == 0) return 0; + if (runtime->no_rewinds) + return 0; + snd_pcm_stream_lock_irq(substream); switch (runtime->status->state) { case SNDRV_PCM_STATE_PREPARED: -- 1.9.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] ALSA: core: let low-level driver or userspace disable rewinds 2017-05-16 1:01 ` [PATCH 1/3] ALSA: core: let low-level driver or userspace disable rewinds Subhransu S. Prusty @ 2017-05-16 5:53 ` Takashi Iwai 2017-05-16 7:40 ` Subhransu S. Prusty 0 siblings, 1 reply; 10+ messages in thread From: Takashi Iwai @ 2017-05-16 5:53 UTC (permalink / raw) To: Subhransu S. Prusty Cc: alsa-devel, patches.audio, lgirdwood, Ramesh Babu, Pierre-Louis Bossart, broonie On Tue, 16 May 2017 03:01:56 +0200, Subhransu S. Prusty wrote: > > From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> > > Add new hw_params flag to explicitly tell driver that rewinds will never > be used. This can be used by low-level driver to optimize DMA operations > and reduce power consumption. Use this flag only when data written in > ring buffer will never be invalidated, e.g. any update of appl_ptr is > final. > > Note that the update of appl_ptr include both a read/write data > operation as well as snd_pcm_forward() whose behavior is not modified. > > Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> > Signed-off-by: Ramesh Babu <ramesh.babu@intel.com> > Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com> > --- > include/sound/pcm.h | 1 + > include/uapi/sound/asound.h | 1 + > sound/core/pcm_native.c | 8 ++++++++ > 3 files changed, 10 insertions(+) > > diff --git a/include/sound/pcm.h b/include/sound/pcm.h > index 361749e60799..a2682c5f5b72 100644 > --- a/include/sound/pcm.h > +++ b/include/sound/pcm.h > @@ -368,6 +368,7 @@ struct snd_pcm_runtime { > unsigned int rate_num; > unsigned int rate_den; > unsigned int no_period_wakeup: 1; > + unsigned int no_rewinds:1; > > /* -- SW params -- */ > int tstamp_mode; /* mmap timestamp is updated */ > diff --git a/include/uapi/sound/asound.h b/include/uapi/sound/asound.h > index fd41697cb4d3..c697ff90450d 100644 > --- a/include/uapi/sound/asound.h > +++ b/include/uapi/sound/asound.h > @@ -365,6 +365,7 @@ struct snd_pcm_info { > #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 */ > +#define SNDRV_PCM_HW_PARAMS_NO_REWINDS (1<<3) /* disable rewinds */ > > struct snd_interval { > unsigned int min, max; > diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c > index 13dec5ec93f2..35dd4ca93f84 100644 > --- a/sound/core/pcm_native.c > +++ b/sound/core/pcm_native.c > @@ -566,6 +566,8 @@ static int snd_pcm_hw_params(struct snd_pcm_substream *substream, > runtime->no_period_wakeup = > (params->info & SNDRV_PCM_INFO_NO_PERIOD_WAKEUP) && > (params->flags & SNDRV_PCM_HW_PARAMS_NO_PERIOD_WAKEUP); > + runtime->no_rewinds = > + (params->flags & SNDRV_PCM_HW_PARAMS_NO_REWINDS) ? 1 : 0; > > bits = snd_pcm_format_physical_width(runtime->format); > runtime->sample_bits = bits; > @@ -2439,6 +2441,9 @@ static snd_pcm_sframes_t snd_pcm_playback_rewind(struct snd_pcm_substream *subst > if (frames == 0) > return 0; > > + if (runtime->no_rewinds) > + return 0; I'd return an error here, because it is a clear error -- you declared that you won't do it but you did. Also, I wonder whether we should add FORWARD disablement flag as well. It's not needed in your case, yes, but FORWARD and REWIND operations are usually paired. thanks, Takashi ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] ALSA: core: let low-level driver or userspace disable rewinds 2017-05-16 5:53 ` Takashi Iwai @ 2017-05-16 7:40 ` Subhransu S. Prusty 0 siblings, 0 replies; 10+ messages in thread From: Subhransu S. Prusty @ 2017-05-16 7:40 UTC (permalink / raw) To: Takashi Iwai Cc: alsa-devel, patches.audio, lgirdwood, Ramesh Babu, Pierre-Louis Bossart, broonie On Tue, May 16, 2017 at 07:53:38AM +0200, Takashi Iwai wrote: > On Tue, 16 May 2017 03:01:56 +0200, > Subhransu S. Prusty wrote: > > > > From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> > > > > Add new hw_params flag to explicitly tell driver that rewinds will never > > be used. This can be used by low-level driver to optimize DMA operations > > and reduce power consumption. Use this flag only when data written in > > ring buffer will never be invalidated, e.g. any update of appl_ptr is > > final. > > > > Note that the update of appl_ptr include both a read/write data > > operation as well as snd_pcm_forward() whose behavior is not modified. > > > > Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> > > Signed-off-by: Ramesh Babu <ramesh.babu@intel.com> > > Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com> > > --- > > include/sound/pcm.h | 1 + > > include/uapi/sound/asound.h | 1 + > > sound/core/pcm_native.c | 8 ++++++++ > > 3 files changed, 10 insertions(+) > > > > diff --git a/include/sound/pcm.h b/include/sound/pcm.h > > index 361749e60799..a2682c5f5b72 100644 > > --- a/include/sound/pcm.h > > +++ b/include/sound/pcm.h > > @@ -368,6 +368,7 @@ struct snd_pcm_runtime { > > unsigned int rate_num; > > unsigned int rate_den; > > unsigned int no_period_wakeup: 1; > > + unsigned int no_rewinds:1; > > > > /* -- SW params -- */ > > int tstamp_mode; /* mmap timestamp is updated */ > > diff --git a/include/uapi/sound/asound.h b/include/uapi/sound/asound.h > > index fd41697cb4d3..c697ff90450d 100644 > > --- a/include/uapi/sound/asound.h > > +++ b/include/uapi/sound/asound.h > > @@ -365,6 +365,7 @@ struct snd_pcm_info { > > #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 */ > > +#define SNDRV_PCM_HW_PARAMS_NO_REWINDS (1<<3) /* disable rewinds */ > > > > struct snd_interval { > > unsigned int min, max; > > diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c > > index 13dec5ec93f2..35dd4ca93f84 100644 > > --- a/sound/core/pcm_native.c > > +++ b/sound/core/pcm_native.c > > @@ -566,6 +566,8 @@ static int snd_pcm_hw_params(struct snd_pcm_substream *substream, > > runtime->no_period_wakeup = > > (params->info & SNDRV_PCM_INFO_NO_PERIOD_WAKEUP) && > > (params->flags & SNDRV_PCM_HW_PARAMS_NO_PERIOD_WAKEUP); > > + runtime->no_rewinds = > > + (params->flags & SNDRV_PCM_HW_PARAMS_NO_REWINDS) ? 1 : 0; > > > > bits = snd_pcm_format_physical_width(runtime->format); > > runtime->sample_bits = bits; > > @@ -2439,6 +2441,9 @@ static snd_pcm_sframes_t snd_pcm_playback_rewind(struct snd_pcm_substream *subst > > if (frames == 0) > > return 0; > > > > + if (runtime->no_rewinds) > > + return 0; > > I'd return an error here, because it is a clear error -- you declared > that you won't do it but you did. This is done based on the suggestion from Pierre: "This forces the application to both set the NOREWIND flag and change the error handling code on a rewind, when the latter point is unnecessary. if no rewind is performed then there is no harm." > > Also, I wonder whether we should add FORWARD disablement flag as > well. It's not needed in your case, yes, but FORWARD and REWIND > operations are usually paired. Either ways is ok for us. Please suggest the approach. Regards, Subhransu > > > thanks, > > Takashi -- ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2018-01-30 11:40 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-01-30 9:36 [PATCH 0/3] Add SPIB Support for Intel Skylake platforms Sriram Periyasamy 2018-01-30 9:36 ` [PATCH 1/3] ALSA: core: let low-level driver or userspace disable rewinds Sriram Periyasamy 2018-01-30 9:36 ` [PATCH 2/3] ALSA: hda: ext: add spib to stream context Sriram Periyasamy 2018-01-30 9:36 ` [PATCH 3/3] ASoC: Intel: Skylake: Add support for spib mode Sriram Periyasamy 2018-01-30 10:38 ` [PATCH 0/3] Add SPIB Support for Intel Skylake platforms Takashi Sakamoto 2018-01-30 11:07 ` Subhransu S. Prusty 2018-01-30 11:44 ` Vinod Koul -- strict thread matches above, loose matches on Subject: below -- 2017-05-16 1:01 [PATCH 0/3] ALSA: Add rewind disable support Subhransu S. Prusty 2017-05-16 1:01 ` [PATCH 1/3] ALSA: core: let low-level driver or userspace disable rewinds Subhransu S. Prusty 2017-05-16 5:53 ` Takashi Iwai 2017-05-16 7:40 ` Subhransu S. Prusty
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).