* [alsa-devel] [PATCH v3 00/11] ASoC: SOF: Data probing
@ 2020-01-28 10:43 Cezary Rojewski
2020-01-28 10:43 ` [alsa-devel] [PATCH v3 01/11] ALSA: hda: Allow for compress stream to hdac_ext_stream assignment Cezary Rojewski
` (11 more replies)
0 siblings, 12 replies; 31+ messages in thread
From: Cezary Rojewski @ 2020-01-28 10:43 UTC (permalink / raw)
To: alsa-devel
Cc: lgirdwood, Cezary Rojewski, broonie, tiwai, pierre-louis.bossart
This set of patches achieves few goals in order to enable data probing
feature for audio DSP.
First, provide new and alter existing interfaces (page allocation,
runtime flow adjustments) to make them compress friendly.
For HDA part, work has been done to account for compress streams when
servicing IRQs, setting up BDLs and assigning DMAs.
Finally, the end goal which are the probe APIs and usage itself. Probes
can be treated as endpoints which allow for data extraction from or
injection to target module - a great ally when debugging problematic
audio issues such as distortions, glitches or gaps.
Compress streams are a weapon of choice here to provide a lightweight
implementation.
While all available IPCs have been defined, current implementation
covers extraction only, with injection scheduled for a later date.
Initial review and development of probes can be found under:
https://github.com/thesofproject/linux/pull/1276
with the hda-compress-enable set of patches being separated and
reviewed on:
https://github.com/thesofproject/linux/pull/1571
Tested on CML-U with rt5682 i2s board.
Changes in v3:
- Addressed sparse and all doc related warnings as reported by Pierre
- Moved _DEBUG_PROBES config outside of _DEVELOPER_SUPPORT block
- Renamed 'extractor' field to 'extractor_stream_tag' as requested
- Relocated 'extractor_stream_tag' declaration from patch 07 to 08
Changes in v2:
- No changes to ALSA core and hda patches
- Removed "ASoC: Intel: sof_rt5682: Add compress probe DAI links" from
the patchset list as requested by Pierre
- updated copyright header for newly added files (dates)
- probes over HDA no longer require SND_SOC_SOF_HDA_LINK enabled
- renamed debugfs probe functions as requested by Pierre
- probe IPC API has been updated to align with newest SOF FW & probe
debug app (struct_size macro has been enlisted to make the size
calculations transparent). This targets only "ASoC: SOF: Implement
Probe IPC API" patch
Cezary Rojewski (11):
ALSA: hda: Allow for compress stream to hdac_ext_stream assignment
ALSA: hda: Prepare for compress stream support
ALSA: hda: Interrupt servicing and BDL setup for compress streams
ALSA: core: Expand DMA buffer information
ALSA: core: Implement compress page allocation and free routines
ASoC: SOF: Intel: Account for compress streams when servicing IRQs
ASoC: SOF: Implement Probe IPC API
ASoC: SOF: Generic probe compress operations
ASoC: SOF: Intel: Probe compress operations
ASoC: SOF: Provide probe debugfs support
ASoC: SOF: Intel: Add Probe compress CPU DAIs
include/sound/compress_driver.h | 40 +++-
include/sound/hdaudio.h | 2 +
include/sound/hdaudio_ext.h | 2 +
include/sound/sof/header.h | 11 ++
sound/core/compress_offload.c | 42 +++++
sound/hda/ext/hdac_ext_stream.c | 49 ++++-
sound/hda/hdac_controller.c | 4 +-
sound/hda/hdac_stream.c | 52 ++++--
sound/soc/sof/Kconfig | 9 +
sound/soc/sof/Makefile | 3 +-
sound/soc/sof/compress.c | 140 ++++++++++++++
sound/soc/sof/compress.h | 29 +++
sound/soc/sof/debug.c | 208 +++++++++++++++++++++
sound/soc/sof/intel/Kconfig | 9 +
sound/soc/sof/intel/Makefile | 1 +
sound/soc/sof/intel/apl.c | 9 +
sound/soc/sof/intel/cnl.c | 9 +
sound/soc/sof/intel/hda-compress.c | 132 +++++++++++++
sound/soc/sof/intel/hda-dai.c | 28 +++
sound/soc/sof/intel/hda-ipc.c | 4 +-
sound/soc/sof/intel/hda-stream.c | 26 ++-
sound/soc/sof/intel/hda.h | 30 +++
sound/soc/sof/ops.h | 43 +++++
sound/soc/sof/pcm.c | 11 +-
sound/soc/sof/probe.c | 286 +++++++++++++++++++++++++++++
sound/soc/sof/probe.h | 85 +++++++++
sound/soc/sof/sof-priv.h | 24 +++
27 files changed, 1247 insertions(+), 41 deletions(-)
create mode 100644 sound/soc/sof/compress.c
create mode 100644 sound/soc/sof/compress.h
create mode 100644 sound/soc/sof/intel/hda-compress.c
create mode 100644 sound/soc/sof/probe.c
create mode 100644 sound/soc/sof/probe.h
--
2.17.1
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 31+ messages in thread
* [alsa-devel] [PATCH v3 01/11] ALSA: hda: Allow for compress stream to hdac_ext_stream assignment
2020-01-28 10:43 [alsa-devel] [PATCH v3 00/11] ASoC: SOF: Data probing Cezary Rojewski
@ 2020-01-28 10:43 ` Cezary Rojewski
2020-01-28 10:43 ` [alsa-devel] [PATCH v3 02/11] ALSA: hda: Prepare for compress stream support Cezary Rojewski
` (10 subsequent siblings)
11 siblings, 0 replies; 31+ messages in thread
From: Cezary Rojewski @ 2020-01-28 10:43 UTC (permalink / raw)
To: alsa-devel
Cc: lgirdwood, Cezary Rojewski, broonie, tiwai, pierre-louis.bossart
Currently only PCM streams can enlist hdac_stream for their data
transfer. Add cstream field to hdac_ext_stream to expose possibility of
compress stream assignment in place of PCM one.
Limited to HOST-type only.
Rather than copying entire hdac_ext_host_stream_assign, declare separate
PCM and compress wrappers and reuse it for both cases.
Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
include/sound/hdaudio.h | 1 +
include/sound/hdaudio_ext.h | 2 ++
sound/hda/ext/hdac_ext_stream.c | 49 +++++++++++++++++++++++++++++----
3 files changed, 46 insertions(+), 6 deletions(-)
diff --git a/include/sound/hdaudio.h b/include/sound/hdaudio.h
index e05b95e83d5a..9a8bf1eb7d69 100644
--- a/include/sound/hdaudio.h
+++ b/include/sound/hdaudio.h
@@ -481,6 +481,7 @@ struct hdac_stream {
struct snd_pcm_substream *substream; /* assigned substream,
* set in PCM open
*/
+ struct snd_compr_stream *cstream;
unsigned int format_val; /* format value to be set in the
* controller and the codec
*/
diff --git a/include/sound/hdaudio_ext.h b/include/sound/hdaudio_ext.h
index ef88b20c7b0a..ec01f2024f0b 100644
--- a/include/sound/hdaudio_ext.h
+++ b/include/sound/hdaudio_ext.h
@@ -84,6 +84,8 @@ int snd_hdac_ext_stream_init_all(struct hdac_bus *bus, int start_idx,
int num_stream, int dir);
void snd_hdac_stream_free_all(struct hdac_bus *bus);
void snd_hdac_link_free_all(struct hdac_bus *bus);
+struct hdac_ext_stream *snd_hdac_ext_cstream_assign(struct hdac_bus *bus,
+ struct snd_compr_stream *cstream);
struct hdac_ext_stream *snd_hdac_ext_stream_assign(struct hdac_bus *bus,
struct snd_pcm_substream *substream,
int type);
diff --git a/sound/hda/ext/hdac_ext_stream.c b/sound/hda/ext/hdac_ext_stream.c
index 6b1b4b834bae..488a52570062 100644
--- a/sound/hda/ext/hdac_ext_stream.c
+++ b/sound/hda/ext/hdac_ext_stream.c
@@ -14,6 +14,7 @@
#include <sound/pcm.h>
#include <sound/hda_register.h>
#include <sound/hdaudio_ext.h>
+#include <sound/compress_driver.h>
/**
* snd_hdac_ext_stream_init - initialize each stream (aka device)
@@ -281,8 +282,7 @@ hdac_ext_link_stream_assign(struct hdac_bus *bus,
}
static struct hdac_ext_stream *
-hdac_ext_host_stream_assign(struct hdac_bus *bus,
- struct snd_pcm_substream *substream)
+hdac_ext_host_stream_assign(struct hdac_bus *bus, int direction)
{
struct hdac_ext_stream *res = NULL;
struct hdac_stream *stream = NULL;
@@ -296,12 +296,13 @@ hdac_ext_host_stream_assign(struct hdac_bus *bus,
struct hdac_ext_stream *hstream = container_of(stream,
struct hdac_ext_stream,
hstream);
- if (stream->direction != substream->stream)
+ if (stream->direction != direction)
continue;
if (!stream->opened) {
if (!hstream->decoupled)
- snd_hdac_ext_stream_decouple(bus, hstream, true);
+ snd_hdac_ext_stream_decouple(bus,
+ hstream, true);
res = hstream;
break;
}
@@ -310,13 +311,49 @@ hdac_ext_host_stream_assign(struct hdac_bus *bus,
spin_lock_irq(&bus->reg_lock);
res->hstream.opened = 1;
res->hstream.running = 0;
- res->hstream.substream = substream;
+ res->hstream.substream = NULL;
+ res->hstream.cstream = NULL;
spin_unlock_irq(&bus->reg_lock);
}
return res;
}
+static struct hdac_ext_stream *
+hdac_ext_host_stream_pcm_assign(struct hdac_bus *bus,
+ struct snd_pcm_substream *substream)
+{
+ struct hdac_ext_stream *res;
+
+ res = hdac_ext_host_stream_assign(bus, substream->stream);
+ if (res)
+ res->hstream.substream = substream;
+
+ return res;
+}
+
+/**
+ * snd_hdac_ext_cstream_assign - assign a host stream for compress
+ * @bus: HD-audio core bus
+ * @cstream: Compress stream to assign
+ *
+ * Assign an unused host stream for the given compress stream.
+ * If no stream is free, NULL is returned. Stream is decoupled
+ * before assignment.
+ */
+struct hdac_ext_stream *snd_hdac_ext_cstream_assign(struct hdac_bus *bus,
+ struct snd_compr_stream *cstream)
+{
+ struct hdac_ext_stream *res;
+
+ res = hdac_ext_host_stream_assign(bus, cstream->direction);
+ if (res)
+ res->hstream.cstream = cstream;
+
+ return res;
+}
+EXPORT_SYMBOL_GPL(snd_hdac_ext_cstream_assign);
+
/**
* snd_hdac_ext_stream_assign - assign a stream for the PCM
* @bus: HD-audio core bus
@@ -350,7 +387,7 @@ struct hdac_ext_stream *snd_hdac_ext_stream_assign(struct hdac_bus *bus,
return hstream;
case HDAC_EXT_STREAM_TYPE_HOST:
- return hdac_ext_host_stream_assign(bus, substream);
+ return hdac_ext_host_stream_pcm_assign(bus, substream);
case HDAC_EXT_STREAM_TYPE_LINK:
return hdac_ext_link_stream_assign(bus, substream);
--
2.17.1
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [alsa-devel] [PATCH v3 02/11] ALSA: hda: Prepare for compress stream support
2020-01-28 10:43 [alsa-devel] [PATCH v3 00/11] ASoC: SOF: Data probing Cezary Rojewski
2020-01-28 10:43 ` [alsa-devel] [PATCH v3 01/11] ALSA: hda: Allow for compress stream to hdac_ext_stream assignment Cezary Rojewski
@ 2020-01-28 10:43 ` Cezary Rojewski
2020-01-28 10:43 ` [alsa-devel] [PATCH v3 03/11] ALSA: hda: Interrupt servicing and BDL setup for compress streams Cezary Rojewski
` (9 subsequent siblings)
11 siblings, 0 replies; 31+ messages in thread
From: Cezary Rojewski @ 2020-01-28 10:43 UTC (permalink / raw)
To: alsa-devel
Cc: lgirdwood, Cezary Rojewski, broonie, tiwai, pierre-louis.bossart
Before introducing compress specific changes, adjust BDL and parameters
setup functions so these are not tightly coupled with PCM streams.
Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
sound/hda/hdac_stream.c | 32 ++++++++++++++++----------------
1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/sound/hda/hdac_stream.c b/sound/hda/hdac_stream.c
index 682ed39f79b0..5259cf366011 100644
--- a/sound/hda/hdac_stream.c
+++ b/sound/hda/hdac_stream.c
@@ -408,11 +408,15 @@ int snd_hdac_stream_setup_periods(struct hdac_stream *azx_dev)
{
struct hdac_bus *bus = azx_dev->bus;
struct snd_pcm_substream *substream = azx_dev->substream;
- struct snd_pcm_runtime *runtime = substream->runtime;
+ struct snd_pcm_runtime *runtime;
+ struct snd_dma_buffer *dmab;
__le32 *bdl;
int i, ofs, periods, period_bytes;
int pos_adj, pos_align;
+ runtime = substream->runtime;
+ dmab = snd_pcm_get_dma_buf(substream);
+
/* reset BDL address */
snd_hdac_stream_writel(azx_dev, SD_BDLPL, 0);
snd_hdac_stream_writel(azx_dev, SD_BDLPU, 0);
@@ -426,7 +430,7 @@ int snd_hdac_stream_setup_periods(struct hdac_stream *azx_dev)
azx_dev->frags = 0;
pos_adj = bus->bdl_pos_adj;
- if (!azx_dev->no_period_wakeup && pos_adj > 0) {
+ if (runtime && !azx_dev->no_period_wakeup && pos_adj > 0) {
pos_align = pos_adj;
pos_adj = (pos_adj * runtime->rate + 47999) / 48000;
if (!pos_adj)
@@ -440,8 +444,7 @@ int snd_hdac_stream_setup_periods(struct hdac_stream *azx_dev)
pos_adj);
pos_adj = 0;
} else {
- ofs = setup_bdle(bus, snd_pcm_get_dma_buf(substream),
- azx_dev,
+ ofs = setup_bdle(bus, dmab, azx_dev,
&bdl, ofs, pos_adj, true);
if (ofs < 0)
goto error;
@@ -451,13 +454,11 @@ int snd_hdac_stream_setup_periods(struct hdac_stream *azx_dev)
for (i = 0; i < periods; i++) {
if (i == periods - 1 && pos_adj)
- ofs = setup_bdle(bus, snd_pcm_get_dma_buf(substream),
- azx_dev, &bdl, ofs,
- period_bytes - pos_adj, 0);
+ ofs = setup_bdle(bus, dmab, azx_dev,
+ &bdl, ofs, period_bytes - pos_adj, 0);
else
- ofs = setup_bdle(bus, snd_pcm_get_dma_buf(substream),
- azx_dev, &bdl, ofs,
- period_bytes,
+ ofs = setup_bdle(bus, dmab, azx_dev,
+ &bdl, ofs, period_bytes,
!azx_dev->no_period_wakeup);
if (ofs < 0)
goto error;
@@ -482,26 +483,25 @@ EXPORT_SYMBOL_GPL(snd_hdac_stream_setup_periods);
int snd_hdac_stream_set_params(struct hdac_stream *azx_dev,
unsigned int format_val)
{
-
- unsigned int bufsize, period_bytes;
struct snd_pcm_substream *substream = azx_dev->substream;
- struct snd_pcm_runtime *runtime;
+ unsigned int bufsize, period_bytes;
+ unsigned int no_period_wakeup;
int err;
if (!substream)
return -EINVAL;
- runtime = substream->runtime;
bufsize = snd_pcm_lib_buffer_bytes(substream);
period_bytes = snd_pcm_lib_period_bytes(substream);
+ no_period_wakeup = substream->runtime->no_period_wakeup;
if (bufsize != azx_dev->bufsize ||
period_bytes != azx_dev->period_bytes ||
format_val != azx_dev->format_val ||
- runtime->no_period_wakeup != azx_dev->no_period_wakeup) {
+ no_period_wakeup != azx_dev->no_period_wakeup) {
azx_dev->bufsize = bufsize;
azx_dev->period_bytes = period_bytes;
azx_dev->format_val = format_val;
- azx_dev->no_period_wakeup = runtime->no_period_wakeup;
+ azx_dev->no_period_wakeup = no_period_wakeup;
err = snd_hdac_stream_setup_periods(azx_dev);
if (err < 0)
return err;
--
2.17.1
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [alsa-devel] [PATCH v3 03/11] ALSA: hda: Interrupt servicing and BDL setup for compress streams
2020-01-28 10:43 [alsa-devel] [PATCH v3 00/11] ASoC: SOF: Data probing Cezary Rojewski
2020-01-28 10:43 ` [alsa-devel] [PATCH v3 01/11] ALSA: hda: Allow for compress stream to hdac_ext_stream assignment Cezary Rojewski
2020-01-28 10:43 ` [alsa-devel] [PATCH v3 02/11] ALSA: hda: Prepare for compress stream support Cezary Rojewski
@ 2020-01-28 10:43 ` Cezary Rojewski
2020-01-28 10:43 ` [alsa-devel] [PATCH v3 04/11] ALSA: core: Expand DMA buffer information Cezary Rojewski
` (8 subsequent siblings)
11 siblings, 0 replies; 31+ messages in thread
From: Cezary Rojewski @ 2020-01-28 10:43 UTC (permalink / raw)
To: alsa-devel
Cc: pierre-louis.bossart, Cezary Rojewski, Divya Prakash, lgirdwood,
tiwai, broonie
Account for compress streams when receiving and servicing buffer
completed interrupts. In case of compress stream enlisting hdac_stream
for data transfer, setup BDL entries much like it is the case for PCM
streams.
Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
Signed-off-by: Divya Prakash <divya1.prakash@intel.com>
---
sound/hda/hdac_controller.c | 4 ++--
sound/hda/hdac_stream.c | 26 ++++++++++++++++++++------
2 files changed, 22 insertions(+), 8 deletions(-)
diff --git a/sound/hda/hdac_controller.c b/sound/hda/hdac_controller.c
index 7e7be8e4dcf9..585908f58028 100644
--- a/sound/hda/hdac_controller.c
+++ b/sound/hda/hdac_controller.c
@@ -553,8 +553,8 @@ int snd_hdac_bus_handle_stream_irq(struct hdac_bus *bus, unsigned int status,
sd_status = snd_hdac_stream_readb(azx_dev, SD_STS);
snd_hdac_stream_writeb(azx_dev, SD_STS, SD_INT_MASK);
handled |= 1 << azx_dev->index;
- if (!azx_dev->substream || !azx_dev->running ||
- !(sd_status & SD_INT_COMPLETE))
+ if ((!azx_dev->substream && !azx_dev->cstream) ||
+ !azx_dev->running || !(sd_status & SD_INT_COMPLETE))
continue;
if (ack)
ack(bus, azx_dev);
diff --git a/sound/hda/hdac_stream.c b/sound/hda/hdac_stream.c
index 5259cf366011..1858b96fdb69 100644
--- a/sound/hda/hdac_stream.c
+++ b/sound/hda/hdac_stream.c
@@ -7,6 +7,7 @@
#include <linux/delay.h>
#include <linux/export.h>
#include <linux/clocksource.h>
+#include <sound/compress_driver.h>
#include <sound/core.h>
#include <sound/pcm.h>
#include <sound/hdaudio.h>
@@ -408,14 +409,20 @@ int snd_hdac_stream_setup_periods(struct hdac_stream *azx_dev)
{
struct hdac_bus *bus = azx_dev->bus;
struct snd_pcm_substream *substream = azx_dev->substream;
+ struct snd_compr_stream *cstream = azx_dev->cstream;
struct snd_pcm_runtime *runtime;
struct snd_dma_buffer *dmab;
__le32 *bdl;
int i, ofs, periods, period_bytes;
int pos_adj, pos_align;
- runtime = substream->runtime;
- dmab = snd_pcm_get_dma_buf(substream);
+ if (substream) {
+ runtime = substream->runtime;
+ dmab = snd_pcm_get_dma_buf(substream);
+ } else {
+ runtime = NULL;
+ dmab = snd_pcm_get_dma_buf(cstream);
+ }
/* reset BDL address */
snd_hdac_stream_writel(azx_dev, SD_BDLPL, 0);
@@ -484,15 +491,22 @@ int snd_hdac_stream_set_params(struct hdac_stream *azx_dev,
unsigned int format_val)
{
struct snd_pcm_substream *substream = azx_dev->substream;
+ struct snd_compr_stream *cstream = azx_dev->cstream;
unsigned int bufsize, period_bytes;
unsigned int no_period_wakeup;
int err;
- if (!substream)
+ if (substream) {
+ bufsize = snd_pcm_lib_buffer_bytes(substream);
+ period_bytes = snd_pcm_lib_period_bytes(substream);
+ no_period_wakeup = substream->runtime->no_period_wakeup;
+ } else if (cstream) {
+ bufsize = cstream->runtime->buffer_size;
+ period_bytes = cstream->runtime->fragment_size;
+ no_period_wakeup = 0;
+ } else {
return -EINVAL;
- bufsize = snd_pcm_lib_buffer_bytes(substream);
- period_bytes = snd_pcm_lib_period_bytes(substream);
- no_period_wakeup = substream->runtime->no_period_wakeup;
+ }
if (bufsize != azx_dev->bufsize ||
period_bytes != azx_dev->period_bytes ||
--
2.17.1
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [alsa-devel] [PATCH v3 04/11] ALSA: core: Expand DMA buffer information
2020-01-28 10:43 [alsa-devel] [PATCH v3 00/11] ASoC: SOF: Data probing Cezary Rojewski
` (2 preceding siblings ...)
2020-01-28 10:43 ` [alsa-devel] [PATCH v3 03/11] ALSA: hda: Interrupt servicing and BDL setup for compress streams Cezary Rojewski
@ 2020-01-28 10:43 ` Cezary Rojewski
2020-01-28 10:59 ` Vinod Koul
2020-01-28 10:43 ` [alsa-devel] [PATCH v3 05/11] ALSA: core: Implement compress page allocation and free routines Cezary Rojewski
` (7 subsequent siblings)
11 siblings, 1 reply; 31+ messages in thread
From: Cezary Rojewski @ 2020-01-28 10:43 UTC (permalink / raw)
To: alsa-devel
Cc: lgirdwood, Cezary Rojewski, broonie, tiwai, pierre-louis.bossart
Update DMA buffer definition for snd_compr_runtime so it is represented
similarly as in snd_pcm_runtime. While at it, modify
snd_compr_set_runtime_buffer to account for newly added members.
Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
include/sound/compress_driver.h | 35 ++++++++++++++++++++++++---------
1 file changed, 26 insertions(+), 9 deletions(-)
diff --git a/include/sound/compress_driver.h b/include/sound/compress_driver.h
index bc88d6f964da..00f633c0c3ba 100644
--- a/include/sound/compress_driver.h
+++ b/include/sound/compress_driver.h
@@ -23,7 +23,6 @@ struct snd_compr_ops;
* struct snd_compr_runtime: runtime stream description
* @state: stream state
* @ops: pointer to DSP callbacks
- * @dma_buffer_p: runtime dma buffer pointer
* @buffer: pointer to kernel buffer, valid only when not in mmap mode or
* DSP doesn't implement copy
* @buffer_size: size of the above buffer
@@ -34,11 +33,14 @@ struct snd_compr_ops;
* @total_bytes_transferred: cumulative bytes transferred by offload DSP
* @sleep: poll sleep
* @private_data: driver private data pointer
+ * @dma_area: virtual buffer address
+ * @dma_addr: physical buffer address (not accessible from main CPU)
+ * @dma_bytes: size of DMA area
+ * @dma_buffer_p: runtime dma buffer pointer
*/
struct snd_compr_runtime {
snd_pcm_state_t state;
struct snd_compr_ops *ops;
- struct snd_dma_buffer *dma_buffer_p;
void *buffer;
u64 buffer_size;
u32 fragment_size;
@@ -47,6 +49,11 @@ struct snd_compr_runtime {
u64 total_bytes_transferred;
wait_queue_head_t sleep;
void *private_data;
+
+ unsigned char *dma_area;
+ dma_addr_t dma_addr;
+ size_t dma_bytes;
+ struct snd_dma_buffer *dma_buffer_p;
};
/**
@@ -180,19 +187,29 @@ static inline void snd_compr_drain_notify(struct snd_compr_stream *stream)
/**
* snd_compr_set_runtime_buffer - Set the Compress runtime buffer
- * @substream: compress substream to set
+ * @stream: compress stream to set
* @bufp: the buffer information, NULL to clear
*
* Copy the buffer information to runtime buffer when @bufp is non-NULL.
* Otherwise it clears the current buffer information.
*/
-static inline void snd_compr_set_runtime_buffer(
- struct snd_compr_stream *substream,
- struct snd_dma_buffer *bufp)
+static inline void
+snd_compr_set_runtime_buffer(struct snd_compr_stream *stream,
+ struct snd_dma_buffer *bufp)
{
- struct snd_compr_runtime *runtime = substream->runtime;
-
- runtime->dma_buffer_p = bufp;
+ struct snd_compr_runtime *runtime = stream->runtime;
+
+ if (bufp) {
+ runtime->dma_buffer_p = bufp;
+ runtime->dma_area = bufp->area;
+ runtime->dma_addr = bufp->addr;
+ runtime->dma_bytes = bufp->bytes;
+ } else {
+ runtime->dma_buffer_p = NULL;
+ runtime->dma_area = NULL;
+ runtime->dma_addr = 0;
+ runtime->dma_bytes = 0;
+ }
}
int snd_compr_stop_error(struct snd_compr_stream *stream,
--
2.17.1
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [alsa-devel] [PATCH v3 05/11] ALSA: core: Implement compress page allocation and free routines
2020-01-28 10:43 [alsa-devel] [PATCH v3 00/11] ASoC: SOF: Data probing Cezary Rojewski
` (3 preceding siblings ...)
2020-01-28 10:43 ` [alsa-devel] [PATCH v3 04/11] ALSA: core: Expand DMA buffer information Cezary Rojewski
@ 2020-01-28 10:43 ` Cezary Rojewski
2020-01-28 11:05 ` Vinod Koul
2020-01-28 10:43 ` [alsa-devel] [PATCH v3 06/11] ASoC: SOF: Intel: Account for compress streams when servicing IRQs Cezary Rojewski
` (6 subsequent siblings)
11 siblings, 1 reply; 31+ messages in thread
From: Cezary Rojewski @ 2020-01-28 10:43 UTC (permalink / raw)
To: alsa-devel
Cc: pierre-louis.bossart, Cezary Rojewski, Divya Prakash, lgirdwood,
tiwai, broonie
Add simple malloc and free methods for memory management for compress
streams. Based on snd_pcm_lib_malloc_pages and snd_pcm_lib_free_pages
implementation.
Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
Signed-off-by: Divya Prakash <divya1.prakash@intel.com>
---
include/sound/compress_driver.h | 5 ++++
sound/core/compress_offload.c | 42 +++++++++++++++++++++++++++++++++
2 files changed, 47 insertions(+)
diff --git a/include/sound/compress_driver.h b/include/sound/compress_driver.h
index 00f633c0c3ba..6ce8effa0b12 100644
--- a/include/sound/compress_driver.h
+++ b/include/sound/compress_driver.h
@@ -67,6 +67,7 @@ struct snd_compr_runtime {
* @metadata_set: metadata set flag, true when set
* @next_track: has userspace signal next track transition, true when set
* @private_data: pointer to DSP private data
+ * @dma_buffer: allocated buffer if any
*/
struct snd_compr_stream {
const char *name;
@@ -78,6 +79,7 @@ struct snd_compr_stream {
bool metadata_set;
bool next_track;
void *private_data;
+ struct snd_dma_buffer dma_buffer;
};
/**
@@ -212,6 +214,9 @@ snd_compr_set_runtime_buffer(struct snd_compr_stream *stream,
}
}
+int snd_compr_malloc_pages(struct snd_compr_stream *stream, size_t size);
+int snd_compr_free_pages(struct snd_compr_stream *stream);
+
int snd_compr_stop_error(struct snd_compr_stream *stream,
snd_pcm_state_t state);
diff --git a/sound/core/compress_offload.c b/sound/core/compress_offload.c
index f34ce564d92c..638c9314284f 100644
--- a/sound/core/compress_offload.c
+++ b/sound/core/compress_offload.c
@@ -488,6 +488,48 @@ snd_compr_get_codec_caps(struct snd_compr_stream *stream, unsigned long arg)
}
#endif /* !COMPR_CODEC_CAPS_OVERFLOW */
+int snd_compr_malloc_pages(struct snd_compr_stream *stream, size_t size)
+{
+ struct snd_dma_buffer *dmab;
+ int ret;
+
+ if (snd_BUG_ON(!(stream) || !(stream)->runtime))
+ return -EINVAL;
+ dmab = kzalloc(sizeof(*dmab), GFP_KERNEL);
+ if (!dmab)
+ return -ENOMEM;
+ dmab->dev = stream->dma_buffer.dev;
+ ret = snd_dma_alloc_pages(dmab->dev.type, dmab->dev.dev, size, dmab);
+ if (ret < 0) {
+ kfree(dmab);
+ return ret;
+ }
+
+ snd_compr_set_runtime_buffer(stream, dmab);
+ stream->runtime->dma_bytes = size;
+ return 1;
+}
+EXPORT_SYMBOL(snd_compr_malloc_pages);
+
+int snd_compr_free_pages(struct snd_compr_stream *stream)
+{
+ struct snd_compr_runtime *runtime = stream->runtime;
+
+ if (snd_BUG_ON(!(stream) || !(stream)->runtime))
+ return -EINVAL;
+ if (runtime->dma_area == NULL)
+ return 0;
+ if (runtime->dma_buffer_p != &stream->dma_buffer) {
+ /* It's a newly allocated buffer. Release it now. */
+ snd_dma_free_pages(runtime->dma_buffer_p);
+ kfree(runtime->dma_buffer_p);
+ }
+
+ snd_compr_set_runtime_buffer(stream, NULL);
+ return 0;
+}
+EXPORT_SYMBOL(snd_compr_free_pages);
+
/* revisit this with snd_pcm_preallocate_xxx */
static int snd_compr_allocate_buffer(struct snd_compr_stream *stream,
struct snd_compr_params *params)
--
2.17.1
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [alsa-devel] [PATCH v3 06/11] ASoC: SOF: Intel: Account for compress streams when servicing IRQs
2020-01-28 10:43 [alsa-devel] [PATCH v3 00/11] ASoC: SOF: Data probing Cezary Rojewski
` (4 preceding siblings ...)
2020-01-28 10:43 ` [alsa-devel] [PATCH v3 05/11] ALSA: core: Implement compress page allocation and free routines Cezary Rojewski
@ 2020-01-28 10:43 ` Cezary Rojewski
2020-01-28 10:43 ` [alsa-devel] [PATCH v3 07/11] ASoC: SOF: Implement Probe IPC API Cezary Rojewski
` (5 subsequent siblings)
11 siblings, 0 replies; 31+ messages in thread
From: Cezary Rojewski @ 2020-01-28 10:43 UTC (permalink / raw)
To: alsa-devel
Cc: lgirdwood, Cezary Rojewski, broonie, tiwai, pierre-louis.bossart
Update stream irq handler definition to correctly set hdac_stream
current position when servicing stream interrupts for compress streams.
Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
include/sound/hdaudio.h | 1 +
sound/soc/sof/intel/hda-stream.c | 26 ++++++++++++++++++++++++--
2 files changed, 25 insertions(+), 2 deletions(-)
diff --git a/include/sound/hdaudio.h b/include/sound/hdaudio.h
index 9a8bf1eb7d69..9a24d57f0cf2 100644
--- a/include/sound/hdaudio.h
+++ b/include/sound/hdaudio.h
@@ -496,6 +496,7 @@ struct hdac_stream {
bool locked:1;
bool stripe:1; /* apply stripe control */
+ unsigned long curr_pos;
/* timestamp */
unsigned long start_wallclk; /* start + minimum wallclk */
unsigned long period_wallclk; /* wallclk for period */
diff --git a/sound/soc/sof/intel/hda-stream.c b/sound/soc/sof/intel/hda-stream.c
index c0ab9bb2a797..c8920a60e346 100644
--- a/sound/soc/sof/intel/hda-stream.c
+++ b/sound/soc/sof/intel/hda-stream.c
@@ -571,6 +571,23 @@ bool hda_dsp_check_stream_irq(struct snd_sof_dev *sdev)
return ret;
}
+static void hda_dsp_set_bytes_transferred(struct hdac_stream *hstream,
+ u64 buffer_size)
+{
+ unsigned int prev_pos;
+ int pos, num_bytes;
+
+ div_u64_rem(hstream->curr_pos, buffer_size, &prev_pos);
+ pos = snd_hdac_stream_get_pos_posbuf(hstream);
+
+ if (pos < prev_pos)
+ num_bytes = (buffer_size - prev_pos) + pos;
+ else
+ num_bytes = pos - prev_pos;
+
+ hstream->curr_pos += num_bytes;
+}
+
static bool hda_dsp_stream_check(struct hdac_bus *bus, u32 status)
{
struct sof_intel_hda_dev *sof_hda = bus_to_sof_hda(bus);
@@ -588,14 +605,19 @@ static bool hda_dsp_stream_check(struct hdac_bus *bus, u32 status)
snd_hdac_stream_writeb(s, SD_STS, sd_status);
active = true;
- if (!s->substream ||
+ if ((!s->substream && !s->cstream) ||
!s->running ||
(sd_status & SOF_HDA_CL_DMA_SD_INT_COMPLETE) == 0)
continue;
/* Inform ALSA only in case not do that with IPC */
- if (sof_hda->no_ipc_position)
+ if (s->substream && sof_hda->no_ipc_position) {
snd_sof_pcm_period_elapsed(s->substream);
+ } else if (s->cstream) {
+ hda_dsp_set_bytes_transferred(s,
+ s->cstream->runtime->buffer_size);
+ snd_compr_fragment_elapsed(s->cstream);
+ }
}
}
--
2.17.1
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [alsa-devel] [PATCH v3 07/11] ASoC: SOF: Implement Probe IPC API
2020-01-28 10:43 [alsa-devel] [PATCH v3 00/11] ASoC: SOF: Data probing Cezary Rojewski
` (5 preceding siblings ...)
2020-01-28 10:43 ` [alsa-devel] [PATCH v3 06/11] ASoC: SOF: Intel: Account for compress streams when servicing IRQs Cezary Rojewski
@ 2020-01-28 10:43 ` Cezary Rojewski
2020-01-28 10:43 ` [alsa-devel] [PATCH v3 08/11] ASoC: SOF: Generic probe compress operations Cezary Rojewski
` (4 subsequent siblings)
11 siblings, 0 replies; 31+ messages in thread
From: Cezary Rojewski @ 2020-01-28 10:43 UTC (permalink / raw)
To: alsa-devel
Cc: lgirdwood, Cezary Rojewski, broonie, tiwai, pierre-louis.bossart
Add all required types and methods to support each and every request
that driver could sent to firmware. Probe is one of SOF firmware
features which allows for data extraction and injection directly from
or to DMA stream.
Exposes eight IPCs:
- addition and removal of injection DMAs
- addition and removal of probe points
- info retrieval of injection DMAs and probe points
- probe initialization and cleanup
Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
Changes in v3:
- fixed sparse and all doc related warnings as reported by Pierre
- 'extractor' field has been renamed and its declaration relocated to
patch: "[PATCH v3 08/11] ASoC: SOF: Generic probe compress operations"
Changes in v2:
- probe IPC API has been updated to align with newest SOF FW & probe
debug app (struct_size macro has been enlisted to make the size
calculations transparent)
| 11 ++
sound/soc/sof/Makefile | 2 +-
sound/soc/sof/intel/hda-ipc.c | 4 +-
sound/soc/sof/probe.c | 286 ++++++++++++++++++++++++++++++++++
sound/soc/sof/probe.h | 85 ++++++++++
5 files changed, 386 insertions(+), 2 deletions(-)
create mode 100644 sound/soc/sof/probe.c
create mode 100644 sound/soc/sof/probe.h
--git a/include/sound/sof/header.h b/include/sound/sof/header.h
index bf3edd9c08b4..b79479575cc8 100644
--- a/include/sound/sof/header.h
+++ b/include/sound/sof/header.h
@@ -51,6 +51,7 @@
#define SOF_IPC_GLB_TRACE_MSG SOF_GLB_TYPE(0x9U)
#define SOF_IPC_GLB_GDB_DEBUG SOF_GLB_TYPE(0xAU)
#define SOF_IPC_GLB_TEST_MSG SOF_GLB_TYPE(0xBU)
+#define SOF_IPC_GLB_PROBE SOF_GLB_TYPE(0xCU)
/*
* DSP Command Message Types
@@ -102,6 +103,16 @@
#define SOF_IPC_STREAM_VORBIS_PARAMS SOF_CMD_TYPE(0x010)
#define SOF_IPC_STREAM_VORBIS_FREE SOF_CMD_TYPE(0x011)
+/* probe */
+#define SOF_IPC_PROBE_INIT SOF_CMD_TYPE(0x001)
+#define SOF_IPC_PROBE_DEINIT SOF_CMD_TYPE(0x002)
+#define SOF_IPC_PROBE_DMA_ADD SOF_CMD_TYPE(0x003)
+#define SOF_IPC_PROBE_DMA_INFO SOF_CMD_TYPE(0x004)
+#define SOF_IPC_PROBE_DMA_REMOVE SOF_CMD_TYPE(0x005)
+#define SOF_IPC_PROBE_POINT_ADD SOF_CMD_TYPE(0x006)
+#define SOF_IPC_PROBE_POINT_INFO SOF_CMD_TYPE(0x007)
+#define SOF_IPC_PROBE_POINT_REMOVE SOF_CMD_TYPE(0x008)
+
/* trace */
#define SOF_IPC_TRACE_DMA_PARAMS SOF_CMD_TYPE(0x001)
#define SOF_IPC_TRACE_DMA_POSITION SOF_CMD_TYPE(0x002)
diff --git a/sound/soc/sof/Makefile b/sound/soc/sof/Makefile
index 0a8bc72c28a5..99c26420d9e5 100644
--- a/sound/soc/sof/Makefile
+++ b/sound/soc/sof/Makefile
@@ -1,7 +1,7 @@
# SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
snd-sof-objs := core.o ops.o loader.o ipc.o pcm.o pm.o debug.o topology.o\
- control.o trace.o utils.o sof-audio.o
+ control.o probe.o trace.o utils.o sof-audio.o
snd-sof-pci-objs := sof-pci-dev.o
snd-sof-acpi-objs := sof-acpi-dev.o
diff --git a/sound/soc/sof/intel/hda-ipc.c b/sound/soc/sof/intel/hda-ipc.c
index 1837f66e361f..922052883b0a 100644
--- a/sound/soc/sof/intel/hda-ipc.c
+++ b/sound/soc/sof/intel/hda-ipc.c
@@ -106,7 +106,9 @@ void hda_dsp_ipc_get_reply(struct snd_sof_dev *sdev)
ret = reply.error;
} else {
/* reply correct size ? */
- if (reply.hdr.size != msg->reply_size) {
+ if (reply.hdr.size != msg->reply_size &&
+ /* getter payload is never known upfront */
+ !(reply.hdr.cmd & SOF_IPC_GLB_PROBE)) {
dev_err(sdev->dev, "error: reply expected %zu got %u bytes\n",
msg->reply_size, reply.hdr.size);
ret = -EINVAL;
diff --git a/sound/soc/sof/probe.c b/sound/soc/sof/probe.c
new file mode 100644
index 000000000000..2b2f3dcfc7e9
--- /dev/null
+++ b/sound/soc/sof/probe.c
@@ -0,0 +1,286 @@
+// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
+//
+// This file is provided under a dual BSD/GPLv2 license. When using or
+// redistributing this file, you may do so under either license.
+//
+// Copyright(c) 2019-2020 Intel Corporation. All rights reserved.
+//
+// Author: Cezary Rojewski <cezary.rojewski@intel.com>
+//
+
+#include "sof-priv.h"
+#include "probe.h"
+
+/**
+ * sof_ipc_probe_init - initialize data probing
+ * @sdev: SOF sound device
+ * @stream_tag: Extractor stream tag
+ * @buffer_size: DMA buffer size to set for extractor
+ *
+ * Host chooses whether extraction is supported or not by providing
+ * valid stream tag to DSP. Once specified, stream described by that
+ * tag will be tied to DSP for extraction for the entire lifetime of
+ * probe.
+ *
+ * Probing is initialized only once and each INIT request must be
+ * matched by DEINIT call.
+ */
+int sof_ipc_probe_init(struct snd_sof_dev *sdev,
+ u32 stream_tag, size_t buffer_size)
+{
+ struct sof_ipc_probe_dma_add_params *msg;
+ struct sof_ipc_reply reply;
+ size_t size = struct_size(msg, dma, 1);
+ int ret;
+
+ msg = kmalloc(size, GFP_KERNEL);
+ if (!msg)
+ return -ENOMEM;
+ msg->hdr.size = size;
+ msg->hdr.cmd = SOF_IPC_GLB_PROBE | SOF_IPC_PROBE_INIT;
+ msg->num_elems = 1;
+ msg->dma[0].stream_tag = stream_tag;
+ msg->dma[0].dma_buffer_size = buffer_size;
+
+ ret = sof_ipc_tx_message(sdev->ipc, msg->hdr.cmd, msg, msg->hdr.size,
+ &reply, sizeof(reply));
+ kfree(msg);
+ return ret;
+}
+EXPORT_SYMBOL(sof_ipc_probe_init);
+
+/**
+ * sof_ipc_probe_deinit - cleanup after data probing
+ * @sdev: SOF sound device
+ *
+ * Host sends DEINIT request to free previously initialized probe
+ * on DSP side once it is no longer needed. DEINIT only when there
+ * are no probes connected and with all injectors detached.
+ */
+int sof_ipc_probe_deinit(struct snd_sof_dev *sdev)
+{
+ struct sof_ipc_cmd_hdr msg;
+ struct sof_ipc_reply reply;
+
+ msg.size = sizeof(msg);
+ msg.cmd = SOF_IPC_GLB_PROBE | SOF_IPC_PROBE_DEINIT;
+
+ return sof_ipc_tx_message(sdev->ipc, msg.cmd, &msg, msg.size,
+ &reply, sizeof(reply));
+}
+EXPORT_SYMBOL(sof_ipc_probe_deinit);
+
+static int sof_ipc_probe_info(struct snd_sof_dev *sdev, unsigned int cmd,
+ void **params, size_t *num_params)
+{
+ struct sof_ipc_probe_info_params msg = {{{0}}};
+ struct sof_ipc_probe_info_params *reply;
+ size_t bytes;
+ int ret;
+
+ *params = NULL;
+ *num_params = 0;
+
+ reply = kzalloc(SOF_IPC_MSG_MAX_SIZE, GFP_KERNEL);
+ if (!reply)
+ return -ENOMEM;
+ msg.rhdr.hdr.size = sizeof(msg);
+ msg.rhdr.hdr.cmd = SOF_IPC_GLB_PROBE | cmd;
+
+ ret = sof_ipc_tx_message(sdev->ipc, msg.rhdr.hdr.cmd, &msg,
+ msg.rhdr.hdr.size, reply, SOF_IPC_MSG_MAX_SIZE);
+ if (ret < 0 || reply->rhdr.error < 0)
+ goto exit;
+
+ if (!reply->num_elems)
+ goto exit;
+
+ bytes = reply->num_elems * sizeof(reply->dma[0]);
+ *params = kmemdup(&reply->dma[0], bytes, GFP_KERNEL);
+ if (!*params) {
+ ret = -ENOMEM;
+ goto exit;
+ }
+ *num_params = msg.num_elems;
+
+exit:
+ kfree(reply);
+ return ret;
+}
+
+/**
+ * sof_ipc_probe_dma_info - retrieve list of active injection dmas
+ * @sdev: SOF sound device
+ * @dma: Returned list of active dmas
+ * @num_dma: Returned count of active dmas
+ *
+ * Host sends DMA_INFO request to obtain list of injection dmas it
+ * can use to transfer data over with.
+ *
+ * Note that list contains only injection dmas as there is only one
+ * extractor (dma) and it is always assigned on probing init.
+ * DSP knows exactly where data from extraction probes is going to,
+ * which is not the case for injection where multiple streams
+ * could be engaged.
+ */
+int sof_ipc_probe_dma_info(struct snd_sof_dev *sdev,
+ struct sof_probe_dma **dma, size_t *num_dma)
+{
+ return sof_ipc_probe_info(sdev, SOF_IPC_PROBE_DMA_INFO,
+ (void **)dma, num_dma);
+}
+EXPORT_SYMBOL(sof_ipc_probe_dma_info);
+
+/**
+ * sof_ipc_probe_dma_add - attach to specified dmas
+ * @sdev: SOF sound device
+ * @dma: List of streams (dmas) to attach to
+ * @num_dma: Number of elements in @dma
+ *
+ * Contrary to extraction, injection streams are never assigned
+ * on init. Before attempting any data injection, host is responsible
+ * for specifying streams which will be later used to transfer data
+ * to connected probe points.
+ */
+int sof_ipc_probe_dma_add(struct snd_sof_dev *sdev,
+ struct sof_probe_dma *dma, size_t num_dma)
+{
+ struct sof_ipc_probe_dma_add_params *msg;
+ struct sof_ipc_reply reply;
+ size_t size = struct_size(msg, dma, num_dma);
+ int ret;
+
+ msg = kmalloc(size, GFP_KERNEL);
+ if (!msg)
+ return -ENOMEM;
+ msg->hdr.size = size;
+ msg->num_elems = num_dma;
+ msg->hdr.cmd = SOF_IPC_GLB_PROBE | SOF_IPC_PROBE_DMA_ADD;
+ memcpy(&msg->dma[0], dma, size - sizeof(*msg));
+
+ ret = sof_ipc_tx_message(sdev->ipc, msg->hdr.cmd, msg, msg->hdr.size,
+ &reply, sizeof(reply));
+ kfree(msg);
+ return ret;
+}
+EXPORT_SYMBOL(sof_ipc_probe_dma_add);
+
+/**
+ * sof_ipc_probe_dma_remove - detach from specified dmas
+ * @sdev: SOF sound device
+ * @stream_tag: List of stream tags to detach from
+ * @num_stream_tag: Number of elements in @stream_tag
+ *
+ * Host sends DMA_REMOVE request to free previously attached stream
+ * from being occupied for injection. Each detach operation should
+ * match equivalent DMA_ADD. Detach only when all probes tied to
+ * given stream have been disconnected.
+ */
+int sof_ipc_probe_dma_remove(struct snd_sof_dev *sdev,
+ unsigned int *stream_tag, size_t num_stream_tag)
+{
+ struct sof_ipc_probe_dma_remove_params *msg;
+ struct sof_ipc_reply reply;
+ size_t size = struct_size(msg, stream_tag, num_stream_tag);
+ int ret;
+
+ msg = kmalloc(size, GFP_KERNEL);
+ if (!msg)
+ return -ENOMEM;
+ msg->hdr.size = size;
+ msg->num_elems = num_stream_tag;
+ msg->hdr.cmd = SOF_IPC_GLB_PROBE | SOF_IPC_PROBE_DMA_REMOVE;
+ memcpy(&msg->stream_tag[0], stream_tag, size - sizeof(*msg));
+
+ ret = sof_ipc_tx_message(sdev->ipc, msg->hdr.cmd, msg, msg->hdr.size,
+ &reply, sizeof(reply));
+ kfree(msg);
+ return ret;
+}
+EXPORT_SYMBOL(sof_ipc_probe_dma_remove);
+
+/**
+ * sof_ipc_probe_points_info - retrieve list of active probe points
+ * @sdev: SOF sound device
+ * @desc: Returned list of active probes
+ * @num_desc: Returned count of active probes
+ *
+ * Host sends PROBE_POINT_INFO request to obtain list of active probe
+ * points, valid for disconnection when given probe is no longer
+ * required.
+ */
+int sof_ipc_probe_points_info(struct snd_sof_dev *sdev,
+ struct sof_probe_point_desc **desc, size_t *num_desc)
+{
+ return sof_ipc_probe_info(sdev, SOF_IPC_PROBE_POINT_INFO,
+ (void **)desc, num_desc);
+}
+EXPORT_SYMBOL(sof_ipc_probe_points_info);
+
+/**
+ * sof_ipc_probe_points_add - connect specified probes
+ * @sdev: SOF sound device
+ * @desc: List of probe points to connect
+ * @num_desc: Number of elements in @desc
+ *
+ * Dynamically connects to provided set of endpoints. Immediately
+ * after connection is established, host must be prepared to
+ * transfer data from or to target stream given the probing purpose.
+ *
+ * Each probe point should be removed using PROBE_POINT_REMOVE
+ * request when no longer needed.
+ */
+int sof_ipc_probe_points_add(struct snd_sof_dev *sdev,
+ struct sof_probe_point_desc *desc, size_t num_desc)
+{
+ struct sof_ipc_probe_point_add_params *msg;
+ struct sof_ipc_reply reply;
+ size_t size = struct_size(msg, desc, num_desc);
+ int ret;
+
+ msg = kmalloc(size, GFP_KERNEL);
+ if (!msg)
+ return -ENOMEM;
+ msg->hdr.size = size;
+ msg->num_elems = num_desc;
+ msg->hdr.cmd = SOF_IPC_GLB_PROBE | SOF_IPC_PROBE_POINT_ADD;
+ memcpy(&msg->desc[0], desc, size - sizeof(*msg));
+
+ ret = sof_ipc_tx_message(sdev->ipc, msg->hdr.cmd, msg, msg->hdr.size,
+ &reply, sizeof(reply));
+ kfree(msg);
+ return ret;
+}
+EXPORT_SYMBOL(sof_ipc_probe_points_add);
+
+/**
+ * sof_ipc_probe_points_remove - disconnect specified probes
+ * @sdev: SOF sound device
+ * @buffer_id: List of probe points to disconnect
+ * @num_buffer_id: Number of elements in @desc
+ *
+ * Removes previously connected probes from list of active probe
+ * points and frees all resources on DSP side.
+ */
+int sof_ipc_probe_points_remove(struct snd_sof_dev *sdev,
+ unsigned int *buffer_id, size_t num_buffer_id)
+{
+ struct sof_ipc_probe_point_remove_params *msg;
+ struct sof_ipc_reply reply;
+ size_t size = struct_size(msg, buffer_id, num_buffer_id);
+ int ret;
+
+ msg = kmalloc(size, GFP_KERNEL);
+ if (!msg)
+ return -ENOMEM;
+ msg->hdr.size = size;
+ msg->num_elems = num_buffer_id;
+ msg->hdr.cmd = SOF_IPC_GLB_PROBE | SOF_IPC_PROBE_POINT_REMOVE;
+ memcpy(&msg->buffer_id[0], buffer_id, size - sizeof(*msg));
+
+ ret = sof_ipc_tx_message(sdev->ipc, msg->hdr.cmd, msg, msg->hdr.size,
+ &reply, sizeof(reply));
+ kfree(msg);
+ return ret;
+}
+EXPORT_SYMBOL(sof_ipc_probe_points_remove);
diff --git a/sound/soc/sof/probe.h b/sound/soc/sof/probe.h
new file mode 100644
index 000000000000..45daa5552834
--- /dev/null
+++ b/sound/soc/sof/probe.h
@@ -0,0 +1,85 @@
+/* SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause) */
+/*
+ * This file is provided under a dual BSD/GPLv2 license. When using or
+ * redistributing this file, you may do so under either license.
+ *
+ * Copyright(c) 2019-2020 Intel Corporation. All rights reserved.
+ *
+ * Author: Cezary Rojewski <cezary.rojewski@intel.com>
+ */
+
+#ifndef __SOF_PROBE_H
+#define __SOF_PROBE_H
+
+#include <sound/sof/header.h>
+
+struct snd_sof_dev;
+
+#define SOF_PROBE_INVALID_NODE_ID UINT_MAX
+
+struct sof_probe_dma {
+ unsigned int stream_tag;
+ unsigned int dma_buffer_size;
+} __packed;
+
+enum sof_connection_purpose {
+ SOF_CONNECTION_PURPOSE_EXTRACT = 1,
+ SOF_CONNECTION_PURPOSE_INJECT,
+};
+
+struct sof_probe_point_desc {
+ unsigned int buffer_id;
+ unsigned int purpose;
+ unsigned int stream_tag;
+} __packed;
+
+struct sof_ipc_probe_dma_add_params {
+ struct sof_ipc_cmd_hdr hdr;
+ unsigned int num_elems;
+ struct sof_probe_dma dma[0];
+} __packed;
+
+struct sof_ipc_probe_info_params {
+ struct sof_ipc_reply rhdr;
+ unsigned int num_elems;
+ union {
+ struct sof_probe_dma dma[0];
+ struct sof_probe_point_desc desc[0];
+ };
+} __packed;
+
+struct sof_ipc_probe_dma_remove_params {
+ struct sof_ipc_cmd_hdr hdr;
+ unsigned int num_elems;
+ unsigned int stream_tag[0];
+} __packed;
+
+struct sof_ipc_probe_point_add_params {
+ struct sof_ipc_cmd_hdr hdr;
+ unsigned int num_elems;
+ struct sof_probe_point_desc desc[0];
+} __packed;
+
+struct sof_ipc_probe_point_remove_params {
+ struct sof_ipc_cmd_hdr hdr;
+ unsigned int num_elems;
+ unsigned int buffer_id[0];
+} __packed;
+
+int sof_ipc_probe_init(struct snd_sof_dev *sdev,
+ u32 stream_tag, size_t buffer_size);
+int sof_ipc_probe_deinit(struct snd_sof_dev *sdev);
+int sof_ipc_probe_dma_info(struct snd_sof_dev *sdev,
+ struct sof_probe_dma **dma, size_t *num_dma);
+int sof_ipc_probe_dma_add(struct snd_sof_dev *sdev,
+ struct sof_probe_dma *dma, size_t num_dma);
+int sof_ipc_probe_dma_remove(struct snd_sof_dev *sdev,
+ unsigned int *stream_tag, size_t num_stream_tag);
+int sof_ipc_probe_points_info(struct snd_sof_dev *sdev,
+ struct sof_probe_point_desc **desc, size_t *num_desc);
+int sof_ipc_probe_points_add(struct snd_sof_dev *sdev,
+ struct sof_probe_point_desc *desc, size_t num_desc);
+int sof_ipc_probe_points_remove(struct snd_sof_dev *sdev,
+ unsigned int *buffer_id, size_t num_buffer_id);
+
+#endif
--
2.17.1
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [alsa-devel] [PATCH v3 08/11] ASoC: SOF: Generic probe compress operations
2020-01-28 10:43 [alsa-devel] [PATCH v3 00/11] ASoC: SOF: Data probing Cezary Rojewski
` (6 preceding siblings ...)
2020-01-28 10:43 ` [alsa-devel] [PATCH v3 07/11] ASoC: SOF: Implement Probe IPC API Cezary Rojewski
@ 2020-01-28 10:43 ` Cezary Rojewski
2020-01-29 7:48 ` Daniel Baluta
2020-01-28 10:43 ` [alsa-devel] [PATCH v3 09/11] ASoC: SOF: Intel: Probe " Cezary Rojewski
` (3 subsequent siblings)
11 siblings, 1 reply; 31+ messages in thread
From: Cezary Rojewski @ 2020-01-28 10:43 UTC (permalink / raw)
To: alsa-devel
Cc: lgirdwood, Cezary Rojewski, broonie, tiwai, pierre-louis.bossart
Define system-agnostic probe compress flow which serves as a base for
actual, hardware-dependent implementations.
As per firmware spec, maximum of one extraction stream is allowed, while
for injection, there can be plenty.
Apart from probe_pointer, all probe compress operations are mandatory.
Copy operation is defined as unified as its flow should be shared across
all SOF systems.
Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
Changes in v3:
- 'extractor' field has been relocated to this patch from: "[PATCH v3
07/11] ASoC: SOF: Implement Probe IPC API"
- 'extractor' field has been renamed to 'extractor_stream_tag'
- _DEBUG_PROBES kconfig moved outside of _DEVELOPER_SUPPORT block
sound/soc/sof/Kconfig | 9 +++
sound/soc/sof/Makefile | 1 +
sound/soc/sof/compress.c | 140 +++++++++++++++++++++++++++++++++++++++
sound/soc/sof/compress.h | 29 ++++++++
sound/soc/sof/ops.h | 43 ++++++++++++
sound/soc/sof/sof-priv.h | 24 +++++++
6 files changed, 246 insertions(+)
create mode 100644 sound/soc/sof/compress.c
create mode 100644 sound/soc/sof/compress.h
diff --git a/sound/soc/sof/Kconfig b/sound/soc/sof/Kconfig
index 827b0ec92522..4dda4b62509f 100644
--- a/sound/soc/sof/Kconfig
+++ b/sound/soc/sof/Kconfig
@@ -41,6 +41,15 @@ config SND_SOC_SOF_OF
required to enable i.MX8 devices.
Say Y if you need this option. If unsure select "N".
+config SND_SOC_SOF_DEBUG_PROBES
+ bool "SOF enable data probing"
+ select SND_SOC_COMPRESS
+ help
+ This option enables the data probing feature that can be used to
+ gather data directly from specific points of the audio pipeline.
+ Say Y if you want to enable probes.
+ If unsure, select "N".
+
config SND_SOC_SOF_DEVELOPER_SUPPORT
bool "SOF developer options support"
depends on EXPERT
diff --git a/sound/soc/sof/Makefile b/sound/soc/sof/Makefile
index 99c26420d9e5..437ec1b40c14 100644
--- a/sound/soc/sof/Makefile
+++ b/sound/soc/sof/Makefile
@@ -2,6 +2,7 @@
snd-sof-objs := core.o ops.o loader.o ipc.o pcm.o pm.o debug.o topology.o\
control.o probe.o trace.o utils.o sof-audio.o
+snd-sof-$(CONFIG_SND_SOC_SOF_DEBUG_PROBES) += compress.o
snd-sof-pci-objs := sof-pci-dev.o
snd-sof-acpi-objs := sof-acpi-dev.o
diff --git a/sound/soc/sof/compress.c b/sound/soc/sof/compress.c
new file mode 100644
index 000000000000..a5f644711aff
--- /dev/null
+++ b/sound/soc/sof/compress.c
@@ -0,0 +1,140 @@
+// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
+//
+// This file is provided under a dual BSD/GPLv2 license. When using or
+// redistributing this file, you may do so under either license.
+//
+// Copyright(c) 2019-2020 Intel Corporation. All rights reserved.
+//
+// Author: Cezary Rojewski <cezary.rojewski@intel.com>
+//
+
+#include <sound/soc.h>
+#include "compress.h"
+#include "ops.h"
+#include "probe.h"
+
+int sof_probe_compr_open(struct snd_compr_stream *cstream,
+ struct snd_soc_dai *dai)
+{
+ struct snd_sof_dev *sdev =
+ snd_soc_component_get_drvdata(dai->component);
+ int ret;
+
+ ret = snd_sof_probe_compr_assign(sdev, cstream, dai);
+ if (ret < 0) {
+ dev_err(dai->dev, "Failed to assign probe stream: %d\n", ret);
+ return ret;
+ }
+
+ sdev->extractor_stream_tag = ret;
+ return 0;
+}
+EXPORT_SYMBOL(sof_probe_compr_open);
+
+int sof_probe_compr_free(struct snd_compr_stream *cstream,
+ struct snd_soc_dai *dai)
+{
+ struct snd_sof_dev *sdev =
+ snd_soc_component_get_drvdata(dai->component);
+ struct sof_probe_point_desc *desc;
+ size_t num_desc;
+ int i, ret;
+
+ /* disconnect all probe points */
+ ret = sof_ipc_probe_points_info(sdev, &desc, &num_desc);
+ if (ret < 0) {
+ dev_err(dai->dev, "Failed to get probe points: %d\n", ret);
+ goto exit;
+ }
+
+ for (i = 0; i < num_desc; i++)
+ sof_ipc_probe_points_remove(sdev, &desc[i].buffer_id, 1);
+ kfree(desc);
+
+exit:
+ ret = sof_ipc_probe_deinit(sdev);
+ if (ret < 0)
+ dev_err(dai->dev, "Failed to deinit probe: %d\n", ret);
+
+ snd_compr_free_pages(cstream);
+
+ return snd_sof_probe_compr_free(sdev, cstream, dai);
+}
+EXPORT_SYMBOL(sof_probe_compr_free);
+
+int sof_probe_compr_set_params(struct snd_compr_stream *cstream,
+ struct snd_compr_params *params, struct snd_soc_dai *dai)
+{
+ struct snd_compr_runtime *rtd = cstream->runtime;
+ struct snd_sof_dev *sdev =
+ snd_soc_component_get_drvdata(dai->component);
+ int ret;
+
+ cstream->dma_buffer.dev.type = SNDRV_DMA_TYPE_DEV_SG;
+ cstream->dma_buffer.dev.dev = sdev->dev;
+ ret = snd_compr_malloc_pages(cstream, rtd->buffer_size);
+ if (ret < 0)
+ return ret;
+
+ ret = snd_sof_probe_compr_set_params(sdev, cstream, params, dai);
+ if (ret < 0)
+ return ret;
+
+ ret = sof_ipc_probe_init(sdev, sdev->extractor_stream_tag,
+ rtd->dma_bytes);
+ if (ret < 0) {
+ dev_err(dai->dev, "Failed to init probe: %d\n", ret);
+ return ret;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(sof_probe_compr_set_params);
+
+int sof_probe_compr_trigger(struct snd_compr_stream *cstream, int cmd,
+ struct snd_soc_dai *dai)
+{
+ struct snd_sof_dev *sdev =
+ snd_soc_component_get_drvdata(dai->component);
+
+ return snd_sof_probe_compr_trigger(sdev, cstream, cmd, dai);
+}
+EXPORT_SYMBOL(sof_probe_compr_trigger);
+
+int sof_probe_compr_pointer(struct snd_compr_stream *cstream,
+ struct snd_compr_tstamp *tstamp, struct snd_soc_dai *dai)
+{
+ struct snd_sof_dev *sdev =
+ snd_soc_component_get_drvdata(dai->component);
+
+ return snd_sof_probe_compr_pointer(sdev, cstream, tstamp, dai);
+}
+EXPORT_SYMBOL(sof_probe_compr_pointer);
+
+int sof_probe_compr_copy(struct snd_compr_stream *cstream,
+ char __user *buf, size_t count)
+{
+ struct snd_compr_runtime *rtd = cstream->runtime;
+ unsigned int offset, n;
+ void *ptr;
+ int ret;
+
+ if (count > rtd->buffer_size)
+ count = rtd->buffer_size;
+
+ div_u64_rem(rtd->total_bytes_transferred, rtd->buffer_size, &offset);
+ ptr = rtd->dma_area + offset;
+ n = rtd->buffer_size - offset;
+
+ if (count < n) {
+ ret = copy_to_user(buf, ptr, count);
+ } else {
+ ret = copy_to_user(buf, ptr, n);
+ ret += copy_to_user(buf + n, rtd->dma_area, count - n);
+ }
+
+ if (ret)
+ return count - ret;
+ return count;
+}
+EXPORT_SYMBOL(sof_probe_compr_copy);
diff --git a/sound/soc/sof/compress.h b/sound/soc/sof/compress.h
new file mode 100644
index 000000000000..dccc9e008f81
--- /dev/null
+++ b/sound/soc/sof/compress.h
@@ -0,0 +1,29 @@
+/* SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause) */
+/*
+ * This file is provided under a dual BSD/GPLv2 license. When using or
+ * redistributing this file, you may do so under either license.
+ *
+ * Copyright(c) 2019-2020 Intel Corporation. All rights reserved.
+ *
+ * Author: Cezary Rojewski <cezary.rojewski@intel.com>
+ */
+
+#ifndef __SOF_COMPRESS_H
+#define __SOF_COMPRESS_H
+
+#include <sound/compress_driver.h>
+
+int sof_probe_compr_open(struct snd_compr_stream *cstream,
+ struct snd_soc_dai *dai);
+int sof_probe_compr_free(struct snd_compr_stream *cstream,
+ struct snd_soc_dai *dai);
+int sof_probe_compr_set_params(struct snd_compr_stream *cstream,
+ struct snd_compr_params *params, struct snd_soc_dai *dai);
+int sof_probe_compr_trigger(struct snd_compr_stream *cstream, int cmd,
+ struct snd_soc_dai *dai);
+int sof_probe_compr_pointer(struct snd_compr_stream *cstream,
+ struct snd_compr_tstamp *tstamp, struct snd_soc_dai *dai);
+int sof_probe_compr_copy(struct snd_compr_stream *cstream,
+ char __user *buf, size_t count);
+
+#endif
diff --git a/sound/soc/sof/ops.h b/sound/soc/sof/ops.h
index e929a6e0058f..33e399c36825 100644
--- a/sound/soc/sof/ops.h
+++ b/sound/soc/sof/ops.h
@@ -391,6 +391,49 @@ snd_sof_pcm_platform_pointer(struct snd_sof_dev *sdev,
return 0;
}
+#if IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_PROBES)
+static inline int
+snd_sof_probe_compr_assign(struct snd_sof_dev *sdev,
+ struct snd_compr_stream *cstream, struct snd_soc_dai *dai)
+{
+ return sof_ops(sdev)->probe_assign(sdev, cstream, dai);
+}
+
+static inline int
+snd_sof_probe_compr_free(struct snd_sof_dev *sdev,
+ struct snd_compr_stream *cstream, struct snd_soc_dai *dai)
+{
+ return sof_ops(sdev)->probe_free(sdev, cstream, dai);
+}
+
+static inline int
+snd_sof_probe_compr_set_params(struct snd_sof_dev *sdev,
+ struct snd_compr_stream *cstream,
+ struct snd_compr_params *params, struct snd_soc_dai *dai)
+{
+ return sof_ops(sdev)->probe_set_params(sdev, cstream, params, dai);
+}
+
+static inline int
+snd_sof_probe_compr_trigger(struct snd_sof_dev *sdev,
+ struct snd_compr_stream *cstream, int cmd,
+ struct snd_soc_dai *dai)
+{
+ return sof_ops(sdev)->probe_trigger(sdev, cstream, cmd, dai);
+}
+
+static inline int
+snd_sof_probe_compr_pointer(struct snd_sof_dev *sdev,
+ struct snd_compr_stream *cstream,
+ struct snd_compr_tstamp *tstamp, struct snd_soc_dai *dai)
+{
+ if (sof_ops(sdev) && sof_ops(sdev)->probe_pointer)
+ return sof_ops(sdev)->probe_pointer(sdev, cstream, tstamp, dai);
+
+ return 0;
+}
+#endif
+
/* machine driver */
static inline int
snd_sof_machine_register(struct snd_sof_dev *sdev, void *pdata)
diff --git a/sound/soc/sof/sof-priv.h b/sound/soc/sof/sof-priv.h
index bc2337cf1142..b08e1811ac06 100644
--- a/sound/soc/sof/sof-priv.h
+++ b/sound/soc/sof/sof-priv.h
@@ -154,6 +154,27 @@ struct snd_sof_dsp_ops {
snd_pcm_uframes_t (*pcm_pointer)(struct snd_sof_dev *sdev,
struct snd_pcm_substream *substream); /* optional */
+#if IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_PROBES)
+ /* Except for probe_pointer, all probe ops are mandatory */
+ int (*probe_assign)(struct snd_sof_dev *sdev,
+ struct snd_compr_stream *cstream,
+ struct snd_soc_dai *dai); /* mandatory */
+ int (*probe_free)(struct snd_sof_dev *sdev,
+ struct snd_compr_stream *cstream,
+ struct snd_soc_dai *dai); /* mandatory */
+ int (*probe_set_params)(struct snd_sof_dev *sdev,
+ struct snd_compr_stream *cstream,
+ struct snd_compr_params *params,
+ struct snd_soc_dai *dai); /* mandatory */
+ int (*probe_trigger)(struct snd_sof_dev *sdev,
+ struct snd_compr_stream *cstream, int cmd,
+ struct snd_soc_dai *dai); /* mandatory */
+ int (*probe_pointer)(struct snd_sof_dev *sdev,
+ struct snd_compr_stream *cstream,
+ struct snd_compr_tstamp *tstamp,
+ struct snd_soc_dai *dai); /* optional */
+#endif
+
/* host read DSP stream data */
void (*ipc_msg_data)(struct snd_sof_dev *sdev,
struct snd_pcm_substream *substream,
@@ -387,6 +408,9 @@ struct snd_sof_dev {
wait_queue_head_t waitq;
int code_loading;
+ /* probes */
+ unsigned int extractor_stream_tag;
+
/* DMA for Trace */
struct snd_dma_buffer dmatb;
struct snd_dma_buffer dmatp;
--
2.17.1
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [alsa-devel] [PATCH v3 09/11] ASoC: SOF: Intel: Probe compress operations
2020-01-28 10:43 [alsa-devel] [PATCH v3 00/11] ASoC: SOF: Data probing Cezary Rojewski
` (7 preceding siblings ...)
2020-01-28 10:43 ` [alsa-devel] [PATCH v3 08/11] ASoC: SOF: Generic probe compress operations Cezary Rojewski
@ 2020-01-28 10:43 ` Cezary Rojewski
2020-01-29 7:55 ` Daniel Baluta
2020-01-28 10:43 ` [alsa-devel] [PATCH v3 10/11] ASoC: SOF: Provide probe debugfs support Cezary Rojewski
` (2 subsequent siblings)
11 siblings, 1 reply; 31+ messages in thread
From: Cezary Rojewski @ 2020-01-28 10:43 UTC (permalink / raw)
To: alsa-devel
Cc: lgirdwood, Cezary Rojewski, broonie, tiwai, pierre-louis.bossart
Add HDA handlers for soc_compr_ops and snd_compr_ops which cover probe
related operations. Implementation supports both connection purposes.
These merely define stream setups as core flow is covered by SOF
compress core.
Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
Changes in v2:
- probes over HDA no longer require SND_SOC_SOF_HDA_LINK enabled
sound/soc/sof/intel/Kconfig | 9 ++
sound/soc/sof/intel/Makefile | 1 +
sound/soc/sof/intel/apl.c | 9 ++
sound/soc/sof/intel/cnl.c | 9 ++
sound/soc/sof/intel/hda-compress.c | 132 +++++++++++++++++++++++++++++
sound/soc/sof/intel/hda.h | 24 ++++++
6 files changed, 184 insertions(+)
create mode 100644 sound/soc/sof/intel/hda-compress.c
diff --git a/sound/soc/sof/intel/Kconfig b/sound/soc/sof/intel/Kconfig
index 56a837d2cb95..3bc64dee7c39 100644
--- a/sound/soc/sof/intel/Kconfig
+++ b/sound/soc/sof/intel/Kconfig
@@ -305,6 +305,15 @@ config SND_SOC_SOF_HDA_AUDIO_CODEC
Say Y if you want to enable HDAudio codecs with SOF.
If unsure select "N".
+config SND_SOC_SOF_HDA_PROBES
+ bool "SOF enable probes over HDA"
+ depends on SND_SOC_SOF_DEBUG_PROBES
+ help
+ This option enables the data probing for Intel(R).
+ Intel(R) Skylake and newer platforms.
+ Say Y if you want to enable probes.
+ If unsure, select "N".
+
config SND_SOC_SOF_HDA_ALWAYS_ENABLE_DMI_L1
bool "SOF enable DMI Link L1"
help
diff --git a/sound/soc/sof/intel/Makefile b/sound/soc/sof/intel/Makefile
index b8f58e006e29..cee02a2e00f4 100644
--- a/sound/soc/sof/intel/Makefile
+++ b/sound/soc/sof/intel/Makefile
@@ -9,6 +9,7 @@ snd-sof-intel-hda-common-objs := hda.o hda-loader.o hda-stream.o hda-trace.o \
hda-dsp.o hda-ipc.o hda-ctrl.o hda-pcm.o \
hda-dai.o hda-bus.o \
apl.o cnl.o
+snd-sof-intel-hda-common-$(CONFIG_SND_SOC_SOF_HDA_PROBES) += hda-compress.o
snd-sof-intel-hda-objs := hda-codec.o
diff --git a/sound/soc/sof/intel/apl.c b/sound/soc/sof/intel/apl.c
index 2483b15699e7..02218d22e51f 100644
--- a/sound/soc/sof/intel/apl.c
+++ b/sound/soc/sof/intel/apl.c
@@ -73,6 +73,15 @@ const struct snd_sof_dsp_ops sof_apl_ops = {
.pcm_trigger = hda_dsp_pcm_trigger,
.pcm_pointer = hda_dsp_pcm_pointer,
+#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA_PROBES)
+ /* probe callbacks */
+ .probe_assign = hda_probe_compr_assign,
+ .probe_free = hda_probe_compr_free,
+ .probe_set_params = hda_probe_compr_set_params,
+ .probe_trigger = hda_probe_compr_trigger,
+ .probe_pointer = hda_probe_compr_pointer,
+#endif
+
/* firmware loading */
.load_firmware = snd_sof_load_firmware_raw,
diff --git a/sound/soc/sof/intel/cnl.c b/sound/soc/sof/intel/cnl.c
index 9e2d8afe0535..74d07a7b5e34 100644
--- a/sound/soc/sof/intel/cnl.c
+++ b/sound/soc/sof/intel/cnl.c
@@ -259,6 +259,15 @@ const struct snd_sof_dsp_ops sof_cnl_ops = {
.pcm_trigger = hda_dsp_pcm_trigger,
.pcm_pointer = hda_dsp_pcm_pointer,
+#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA_PROBES)
+ /* probe callbacks */
+ .probe_assign = hda_probe_compr_assign,
+ .probe_free = hda_probe_compr_free,
+ .probe_set_params = hda_probe_compr_set_params,
+ .probe_trigger = hda_probe_compr_trigger,
+ .probe_pointer = hda_probe_compr_pointer,
+#endif
+
/* firmware loading */
.load_firmware = snd_sof_load_firmware_raw,
diff --git a/sound/soc/sof/intel/hda-compress.c b/sound/soc/sof/intel/hda-compress.c
new file mode 100644
index 000000000000..da7de867d0af
--- /dev/null
+++ b/sound/soc/sof/intel/hda-compress.c
@@ -0,0 +1,132 @@
+// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
+//
+// This file is provided under a dual BSD/GPLv2 license. When using or
+// redistributing this file, you may do so under either license.
+//
+// Copyright(c) 2019-2020 Intel Corporation. All rights reserved.
+//
+// Author: Cezary Rojewski <cezary.rojewski@intel.com>
+//
+
+#include <sound/hdaudio_ext.h>
+#include <sound/soc.h>
+#include "../sof-priv.h"
+#include "hda.h"
+
+static inline struct hdac_ext_stream *
+hda_compr_get_stream(struct snd_compr_stream *cstream)
+{
+ return cstream->runtime->private_data;
+}
+
+int hda_probe_compr_assign(struct snd_sof_dev *sdev,
+ struct snd_compr_stream *cstream,
+ struct snd_soc_dai *dai)
+{
+ struct hdac_ext_stream *stream;
+ struct hdac_bus *bus = sof_to_bus(sdev);
+
+ stream = snd_hdac_ext_cstream_assign(bus, cstream);
+ if (!stream)
+ return -EBUSY;
+
+ hdac_stream(stream)->curr_pos = 0;
+ cstream->runtime->private_data = stream;
+
+ return hdac_stream(stream)->stream_tag;
+}
+
+int hda_probe_compr_free(struct snd_sof_dev *sdev,
+ struct snd_compr_stream *cstream,
+ struct snd_soc_dai *dai)
+{
+ struct hdac_ext_stream *stream = hda_compr_get_stream(cstream);
+
+ snd_hdac_stream_cleanup(hdac_stream(stream));
+ hdac_stream(stream)->prepared = 0;
+ snd_hdac_ext_stream_release(stream, HDAC_EXT_STREAM_TYPE_HOST);
+
+ return 0;
+}
+
+int hda_probe_compr_set_params(struct snd_sof_dev *sdev,
+ struct snd_compr_stream *cstream,
+ struct snd_compr_params *params,
+ struct snd_soc_dai *dai)
+{
+ struct hdac_ext_stream *stream = hda_compr_get_stream(cstream);
+ unsigned int format_val;
+ int bps, ret;
+ /* compr params do not store bit depth, default to S32_LE */
+ snd_pcm_format_t format = SNDRV_PCM_FORMAT_S32_LE;
+
+ hdac_stream(stream)->bufsize = 0;
+ hdac_stream(stream)->period_bytes = 0;
+ hdac_stream(stream)->format_val = 0;
+
+ bps = snd_pcm_format_physical_width(format);
+ if (bps < 0)
+ return bps;
+ format_val = snd_hdac_calc_stream_format(params->codec.sample_rate,
+ params->codec.ch_out, format, bps, 0);
+ ret = snd_hdac_stream_set_params(hdac_stream(stream), format_val);
+ if (ret < 0)
+ return ret;
+ ret = snd_hdac_stream_setup(hdac_stream(stream));
+ if (ret < 0)
+ return ret;
+
+ hdac_stream(stream)->prepared = 1;
+
+ return 0;
+}
+
+int hda_probe_compr_trigger(struct snd_sof_dev *sdev,
+ struct snd_compr_stream *cstream, int cmd,
+ struct snd_soc_dai *dai)
+{
+ struct hdac_ext_stream *stream = hda_compr_get_stream(cstream);
+ struct hdac_bus *bus = sof_to_bus(sdev);
+ unsigned long cookie;
+
+ if (!hdac_stream(stream)->prepared)
+ return -EPIPE;
+
+ switch (cmd) {
+ case SNDRV_PCM_TRIGGER_START:
+ case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
+ case SNDRV_PCM_TRIGGER_RESUME:
+ spin_lock_irqsave(&bus->reg_lock, cookie);
+ snd_hdac_stream_start(hdac_stream(stream), true);
+ spin_unlock_irqrestore(&bus->reg_lock, cookie);
+ break;
+
+ case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
+ case SNDRV_PCM_TRIGGER_SUSPEND:
+ case SNDRV_PCM_TRIGGER_STOP:
+ spin_lock_irqsave(&bus->reg_lock, cookie);
+ snd_hdac_stream_stop(hdac_stream(stream));
+ spin_unlock_irqrestore(&bus->reg_lock, cookie);
+ break;
+
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+int hda_probe_compr_pointer(struct snd_sof_dev *sdev,
+ struct snd_compr_stream *cstream,
+ struct snd_compr_tstamp *tstamp,
+ struct snd_soc_dai *dai)
+{
+ struct hdac_ext_stream *stream = hda_compr_get_stream(cstream);
+ struct snd_soc_pcm_stream *pstream;
+
+ pstream = &dai->driver->capture;
+ tstamp->copied_total = hdac_stream(stream)->curr_pos;
+ tstamp->sampling_rate = snd_pcm_rate_bit_to_rate(pstream->rates);
+
+ return 0;
+}
diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h
index a4d030bfeee1..80aaf4172e34 100644
--- a/sound/soc/sof/intel/hda.h
+++ b/sound/soc/sof/intel/hda.h
@@ -11,6 +11,7 @@
#ifndef __SOF_INTEL_HDA_H
#define __SOF_INTEL_HDA_H
+#include <sound/compress_driver.h>
#include <sound/hda_codec.h>
#include <sound/hdaudio_ext.h>
#include "shim.h"
@@ -533,6 +534,29 @@ int hda_ipc_pcm_params(struct snd_sof_dev *sdev,
struct snd_pcm_substream *substream,
const struct sof_ipc_pcm_params_reply *reply);
+#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA_PROBES)
+/*
+ * Probe Compress Operations.
+ */
+int hda_probe_compr_assign(struct snd_sof_dev *sdev,
+ struct snd_compr_stream *cstream,
+ struct snd_soc_dai *dai);
+int hda_probe_compr_free(struct snd_sof_dev *sdev,
+ struct snd_compr_stream *cstream,
+ struct snd_soc_dai *dai);
+int hda_probe_compr_set_params(struct snd_sof_dev *sdev,
+ struct snd_compr_stream *cstream,
+ struct snd_compr_params *params,
+ struct snd_soc_dai *dai);
+int hda_probe_compr_trigger(struct snd_sof_dev *sdev,
+ struct snd_compr_stream *cstream, int cmd,
+ struct snd_soc_dai *dai);
+int hda_probe_compr_pointer(struct snd_sof_dev *sdev,
+ struct snd_compr_stream *cstream,
+ struct snd_compr_tstamp *tstamp,
+ struct snd_soc_dai *dai);
+#endif
+
/*
* DSP IPC Operations.
*/
--
2.17.1
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [alsa-devel] [PATCH v3 10/11] ASoC: SOF: Provide probe debugfs support
2020-01-28 10:43 [alsa-devel] [PATCH v3 00/11] ASoC: SOF: Data probing Cezary Rojewski
` (8 preceding siblings ...)
2020-01-28 10:43 ` [alsa-devel] [PATCH v3 09/11] ASoC: SOF: Intel: Probe " Cezary Rojewski
@ 2020-01-28 10:43 ` Cezary Rojewski
2020-01-28 17:54 ` Sridharan, Ranjani
2020-01-28 10:43 ` [alsa-devel] [PATCH v3 11/11] ASoC: SOF: Intel: Add Probe compress CPU DAIs Cezary Rojewski
2020-01-28 17:55 ` [alsa-devel] [PATCH v3 00/11] ASoC: SOF: Data probing Pierre-Louis Bossart
11 siblings, 1 reply; 31+ messages in thread
From: Cezary Rojewski @ 2020-01-28 10:43 UTC (permalink / raw)
To: alsa-devel
Cc: lgirdwood, Cezary Rojewski, broonie, tiwai, pierre-louis.bossart
Define debugfs subdirectory delegated for IPC communication with DSP.
Input format: uint,uint,(...) which are later translated into DWORDS
sequence and further into instances of struct of interest given the IPC
type.
For Extractor probes, following have been enabled:
- PROBE_POINT_ADD (echo <..> probe_points)
- PROBE_POINT_REMOVE (echo <..> probe_points_remove)
- PROBE_POINT_INFO (cat probe_points)
Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
Changes in v2:
- renamed debugfs probe functions as requested by Pierre
sound/soc/sof/debug.c | 208 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 208 insertions(+)
diff --git a/sound/soc/sof/debug.c b/sound/soc/sof/debug.c
index d2b3b99d3a20..d38ab59e9a98 100644
--- a/sound/soc/sof/debug.c
+++ b/sound/soc/sof/debug.c
@@ -17,6 +17,203 @@
#include "sof-priv.h"
#include "ops.h"
+#if IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_PROBES)
+#include "probe.h"
+
+/**
+ * strsplit_u32 - Split string into sequence of u32 tokens
+ * @buf: String to split into tokens.
+ * @delim: String containing delimiter characters.
+ * @tkns: Returned u32 sequence pointer.
+ * @num_tkns: Returned number of tokens obtained.
+ */
+static int
+strsplit_u32(char **buf, const char *delim, u32 **tkns, size_t *num_tkns)
+{
+ char *s;
+ u32 *data, *tmp;
+ size_t count = 0;
+ size_t cap = 32;
+ int ret = 0;
+
+ *tkns = NULL;
+ *num_tkns = 0;
+ data = kcalloc(cap, sizeof(*data), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ while ((s = strsep(buf, delim)) != NULL) {
+ ret = kstrtouint(s, 0, data + count);
+ if (ret)
+ goto exit;
+ if (++count >= cap) {
+ cap *= 2;
+ tmp = krealloc(data, cap * sizeof(*data), GFP_KERNEL);
+ if (!tmp) {
+ ret = -ENOMEM;
+ goto exit;
+ }
+ data = tmp;
+ }
+ }
+
+ if (!count)
+ goto exit;
+ *tkns = kmemdup(data, count * sizeof(*data), GFP_KERNEL);
+ if (*tkns == NULL) {
+ ret = -ENOMEM;
+ goto exit;
+ }
+ *num_tkns = count;
+
+exit:
+ kfree(data);
+ return ret;
+}
+
+static int tokenize_input(const char __user *from, size_t count,
+ loff_t *ppos, u32 **tkns, size_t *num_tkns)
+{
+ char *buf;
+ int ret;
+
+ buf = kmalloc(count + 1, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ ret = simple_write_to_buffer(buf, count, ppos, from, count);
+ if (ret != count) {
+ ret = ret >= 0 ? -EIO : ret;
+ goto exit;
+ }
+
+ buf[count] = '\0';
+ ret = strsplit_u32((char **)&buf, ",", tkns, num_tkns);
+exit:
+ kfree(buf);
+ return ret;
+}
+
+static ssize_t probe_points_read(struct file *file,
+ char __user *to, size_t count, loff_t *ppos)
+{
+ struct snd_sof_dfsentry *dfse = file->private_data;
+ struct sof_probe_point_desc *desc;
+ size_t num_desc, len = 0;
+ char *buf;
+ int i, ret;
+
+ buf = kzalloc(PAGE_SIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ ret = sof_ipc_probe_points_info(dfse->sdev, &desc, &num_desc);
+ if (ret < 0)
+ goto exit;
+
+ for (i = 0; i < num_desc; i++) {
+ ret = snprintf(buf + len, PAGE_SIZE - len,
+ "Id: %#010x Purpose: %d Node id: %#x\n",
+ desc[i].buffer_id, desc[i].purpose, desc[i].stream_tag);
+ if (ret < 0)
+ goto free_desc;
+ len += ret;
+ }
+
+ ret = simple_read_from_buffer(to, count, ppos, buf, len);
+free_desc:
+ kfree(desc);
+exit:
+ kfree(buf);
+ return ret;
+}
+
+static ssize_t probe_points_write(struct file *file,
+ const char __user *from, size_t count, loff_t *ppos)
+{
+ struct snd_sof_dfsentry *dfse = file->private_data;
+ struct sof_probe_point_desc *desc;
+ size_t num_tkns, bytes;
+ u32 *tkns;
+ int ret;
+
+ ret = tokenize_input(from, count, ppos, &tkns, &num_tkns);
+ if (ret < 0)
+ return ret;
+ bytes = sizeof(*tkns) * num_tkns;
+ if (!num_tkns || (bytes % sizeof(*desc))) {
+ ret = -EINVAL;
+ goto exit;
+ }
+
+ desc = (struct sof_probe_point_desc *)tkns;
+ ret = sof_ipc_probe_points_add(dfse->sdev,
+ desc, bytes / sizeof(*desc));
+ if (!ret)
+ ret = count;
+exit:
+ kfree(tkns);
+ return ret;
+}
+
+static const struct file_operations probe_points_fops = {
+ .open = simple_open,
+ .read = probe_points_read,
+ .write = probe_points_write,
+ .llseek = default_llseek,
+};
+
+static ssize_t probe_points_remove_write(struct file *file,
+ const char __user *from, size_t count, loff_t *ppos)
+{
+ struct snd_sof_dfsentry *dfse = file->private_data;
+ size_t num_tkns;
+ u32 *tkns;
+ int ret;
+
+ ret = tokenize_input(from, count, ppos, &tkns, &num_tkns);
+ if (ret < 0)
+ return ret;
+ if (!num_tkns) {
+ ret = -EINVAL;
+ goto exit;
+ }
+
+ ret = sof_ipc_probe_points_remove(dfse->sdev, tkns, num_tkns);
+ if (!ret)
+ ret = count;
+exit:
+ kfree(tkns);
+ return ret;
+}
+
+static const struct file_operations probe_points_remove_fops = {
+ .open = simple_open,
+ .write = probe_points_remove_write,
+ .llseek = default_llseek,
+};
+
+static int snd_sof_debugfs_probe_item(struct snd_sof_dev *sdev,
+ const char *name, mode_t mode,
+ const struct file_operations *fops)
+{
+ struct snd_sof_dfsentry *dfse;
+
+ dfse = devm_kzalloc(sdev->dev, sizeof(*dfse), GFP_KERNEL);
+ if (!dfse)
+ return -ENOMEM;
+
+ dfse->type = SOF_DFSENTRY_TYPE_BUF;
+ dfse->sdev = sdev;
+
+ debugfs_create_file(name, mode, sdev->debugfs_root, dfse, fops);
+ /* add to dfsentry list */
+ list_add(&dfse->list, &sdev->dfsentry_list);
+
+ return 0;
+}
+#endif
+
#if IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_IPC_FLOOD_TEST)
#define MAX_IPC_FLOOD_DURATION_MS 1000
#define MAX_IPC_FLOOD_COUNT 10000
@@ -436,6 +633,17 @@ int snd_sof_dbg_init(struct snd_sof_dev *sdev)
return err;
}
+#if IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_PROBES)
+ err = snd_sof_debugfs_probe_item(sdev, "probe_points",
+ 0644, &probe_points_fops);
+ if (err < 0)
+ return err;
+ err = snd_sof_debugfs_probe_item(sdev, "probe_points_remove",
+ 0200, &probe_points_remove_fops);
+ if (err < 0)
+ return err;
+#endif
+
#if IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_IPC_FLOOD_TEST)
/* create read-write ipc_flood_count debugfs entry */
err = snd_sof_debugfs_buf_item(sdev, NULL, 0,
--
2.17.1
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [alsa-devel] [PATCH v3 11/11] ASoC: SOF: Intel: Add Probe compress CPU DAIs
2020-01-28 10:43 [alsa-devel] [PATCH v3 00/11] ASoC: SOF: Data probing Cezary Rojewski
` (9 preceding siblings ...)
2020-01-28 10:43 ` [alsa-devel] [PATCH v3 10/11] ASoC: SOF: Provide probe debugfs support Cezary Rojewski
@ 2020-01-28 10:43 ` Cezary Rojewski
2020-01-29 8:04 ` Daniel Baluta
2020-01-28 17:55 ` [alsa-devel] [PATCH v3 00/11] ASoC: SOF: Data probing Pierre-Louis Bossart
11 siblings, 1 reply; 31+ messages in thread
From: Cezary Rojewski @ 2020-01-28 10:43 UTC (permalink / raw)
To: alsa-devel
Cc: lgirdwood, Cezary Rojewski, broonie, tiwai, pierre-louis.bossart
Declare extraction CPU DAI as well as sof_probe_compr_ops. FE DAIs can
link against these new CPU DAI to create new compress devices.
Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
sound/soc/sof/intel/hda-dai.c | 28 ++++++++++++++++++++++++++++
sound/soc/sof/intel/hda.h | 6 ++++++
sound/soc/sof/pcm.c | 11 ++++++++++-
3 files changed, 44 insertions(+), 1 deletion(-)
diff --git a/sound/soc/sof/intel/hda-dai.c b/sound/soc/sof/intel/hda-dai.c
index 9c6e3f990ee3..ed5e7d2c0d43 100644
--- a/sound/soc/sof/intel/hda-dai.c
+++ b/sound/soc/sof/intel/hda-dai.c
@@ -399,6 +399,19 @@ static const struct snd_soc_dai_ops hda_link_dai_ops = {
.trigger = hda_link_pcm_trigger,
.prepare = hda_link_pcm_prepare,
};
+
+#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA_PROBES)
+#include "../compress.h"
+
+static struct snd_soc_cdai_ops sof_probe_compr_ops = {
+ .startup = sof_probe_compr_open,
+ .shutdown = sof_probe_compr_free,
+ .set_params = sof_probe_compr_set_params,
+ .trigger = sof_probe_compr_trigger,
+ .pointer = sof_probe_compr_pointer,
+};
+
+#endif
#endif
/*
@@ -460,5 +473,20 @@ struct snd_soc_dai_driver skl_dai[] = {
.name = "Alt Analog CPU DAI",
.ops = &hda_link_dai_ops,
},
+#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA_PROBES)
+{
+ .name = "Probe Extraction CPU DAI",
+ .compress_new = snd_soc_new_compress,
+ .cops = &sof_probe_compr_ops,
+ .capture = {
+ .stream_name = "Probe Extraction",
+ .channels_min = 1,
+ .channels_max = 8,
+ .rates = SNDRV_PCM_RATE_48000,
+ .rate_min = 48000,
+ .rate_max = 48000,
+ },
+},
+#endif
#endif
};
diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h
index 80aaf4172e34..955775c4fcda 100644
--- a/sound/soc/sof/intel/hda.h
+++ b/sound/soc/sof/intel/hda.h
@@ -349,7 +349,13 @@
/* Number of DAIs */
#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA)
+
+#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA_PROBES)
+#define SOF_SKL_NUM_DAIS 16
+#else
#define SOF_SKL_NUM_DAIS 15
+#endif
+
#else
#define SOF_SKL_NUM_DAIS 8
#endif
diff --git a/sound/soc/sof/pcm.c b/sound/soc/sof/pcm.c
index 314f3095c12f..d423fb404a3d 100644
--- a/sound/soc/sof/pcm.c
+++ b/sound/soc/sof/pcm.c
@@ -756,6 +756,15 @@ static void sof_pcm_remove(struct snd_soc_component *component)
snd_soc_tplg_component_remove(component, SND_SOC_TPLG_INDEX_ALL);
}
+#if IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_PROBES)
+#include "compress.h"
+
+struct snd_compr_ops sof_compressed_ops = {
+ .copy = sof_probe_compr_copy,
+};
+EXPORT_SYMBOL(sof_compressed_ops);
+#endif
+
void snd_sof_new_platform_drv(struct snd_sof_dev *sdev)
{
struct snd_soc_component_driver *pd = &sdev->plat_drv;
@@ -775,7 +784,7 @@ void snd_sof_new_platform_drv(struct snd_sof_dev *sdev)
pd->trigger = sof_pcm_trigger;
pd->pointer = sof_pcm_pointer;
-#if IS_ENABLED(CONFIG_SND_SOC_SOF_COMPRESS)
+#if IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_PROBES)
pd->compr_ops = &sof_compressed_ops;
#endif
pd->pcm_construct = sof_pcm_new;
--
2.17.1
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [alsa-devel] [PATCH v3 04/11] ALSA: core: Expand DMA buffer information
2020-01-28 10:43 ` [alsa-devel] [PATCH v3 04/11] ALSA: core: Expand DMA buffer information Cezary Rojewski
@ 2020-01-28 10:59 ` Vinod Koul
2020-01-28 11:58 ` Cezary Rojewski
0 siblings, 1 reply; 31+ messages in thread
From: Vinod Koul @ 2020-01-28 10:59 UTC (permalink / raw)
To: Cezary Rojewski
Cc: pierre-louis.bossart, alsa-devel, broonie, lgirdwood, tiwai
On 28-01-20, 11:43, Cezary Rojewski wrote:
> Update DMA buffer definition for snd_compr_runtime so it is represented
> similarly as in snd_pcm_runtime. While at it, modify
> snd_compr_set_runtime_buffer to account for newly added members.
Please run ./scripts/get_maintainer.pl, it will tell you the people you
should CC on a patch.
Also Takashi already acked, so you should add the acks/reviews received
in subsequent versions (unless they changed)
And for this:
Acked-by: Vinod Koul <vkoul@kernel.org>
> Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
> ---
> include/sound/compress_driver.h | 35 ++++++++++++++++++++++++---------
> 1 file changed, 26 insertions(+), 9 deletions(-)
>
> diff --git a/include/sound/compress_driver.h b/include/sound/compress_driver.h
> index bc88d6f964da..00f633c0c3ba 100644
> --- a/include/sound/compress_driver.h
> +++ b/include/sound/compress_driver.h
> @@ -23,7 +23,6 @@ struct snd_compr_ops;
> * struct snd_compr_runtime: runtime stream description
> * @state: stream state
> * @ops: pointer to DSP callbacks
> - * @dma_buffer_p: runtime dma buffer pointer
> * @buffer: pointer to kernel buffer, valid only when not in mmap mode or
> * DSP doesn't implement copy
> * @buffer_size: size of the above buffer
> @@ -34,11 +33,14 @@ struct snd_compr_ops;
> * @total_bytes_transferred: cumulative bytes transferred by offload DSP
> * @sleep: poll sleep
> * @private_data: driver private data pointer
> + * @dma_area: virtual buffer address
> + * @dma_addr: physical buffer address (not accessible from main CPU)
> + * @dma_bytes: size of DMA area
> + * @dma_buffer_p: runtime dma buffer pointer
> */
> struct snd_compr_runtime {
> snd_pcm_state_t state;
> struct snd_compr_ops *ops;
> - struct snd_dma_buffer *dma_buffer_p;
> void *buffer;
> u64 buffer_size;
> u32 fragment_size;
> @@ -47,6 +49,11 @@ struct snd_compr_runtime {
> u64 total_bytes_transferred;
> wait_queue_head_t sleep;
> void *private_data;
> +
> + unsigned char *dma_area;
> + dma_addr_t dma_addr;
> + size_t dma_bytes;
> + struct snd_dma_buffer *dma_buffer_p;
> };
>
> /**
> @@ -180,19 +187,29 @@ static inline void snd_compr_drain_notify(struct snd_compr_stream *stream)
>
> /**
> * snd_compr_set_runtime_buffer - Set the Compress runtime buffer
> - * @substream: compress substream to set
> + * @stream: compress stream to set
> * @bufp: the buffer information, NULL to clear
> *
> * Copy the buffer information to runtime buffer when @bufp is non-NULL.
> * Otherwise it clears the current buffer information.
> */
> -static inline void snd_compr_set_runtime_buffer(
> - struct snd_compr_stream *substream,
> - struct snd_dma_buffer *bufp)
> +static inline void
> +snd_compr_set_runtime_buffer(struct snd_compr_stream *stream,
> + struct snd_dma_buffer *bufp)
> {
> - struct snd_compr_runtime *runtime = substream->runtime;
> -
> - runtime->dma_buffer_p = bufp;
> + struct snd_compr_runtime *runtime = stream->runtime;
> +
> + if (bufp) {
> + runtime->dma_buffer_p = bufp;
> + runtime->dma_area = bufp->area;
> + runtime->dma_addr = bufp->addr;
> + runtime->dma_bytes = bufp->bytes;
> + } else {
> + runtime->dma_buffer_p = NULL;
> + runtime->dma_area = NULL;
> + runtime->dma_addr = 0;
> + runtime->dma_bytes = 0;
> + }
> }
>
> int snd_compr_stop_error(struct snd_compr_stream *stream,
> --
> 2.17.1
>
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
--
~Vinod
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [alsa-devel] [PATCH v3 05/11] ALSA: core: Implement compress page allocation and free routines
2020-01-28 10:43 ` [alsa-devel] [PATCH v3 05/11] ALSA: core: Implement compress page allocation and free routines Cezary Rojewski
@ 2020-01-28 11:05 ` Vinod Koul
0 siblings, 0 replies; 31+ messages in thread
From: Vinod Koul @ 2020-01-28 11:05 UTC (permalink / raw)
To: Cezary Rojewski
Cc: alsa-devel, lgirdwood, Divya Prakash, tiwai,
pierre-louis.bossart, broonie
On 28-01-20, 11:43, Cezary Rojewski wrote:
> Add simple malloc and free methods for memory management for compress
> streams. Based on snd_pcm_lib_malloc_pages and snd_pcm_lib_free_pages
> implementation.
Acked-by: Vinod Koul <vkoul@kernel.org>
--
~Vinod
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [alsa-devel] [PATCH v3 04/11] ALSA: core: Expand DMA buffer information
2020-01-28 10:59 ` Vinod Koul
@ 2020-01-28 11:58 ` Cezary Rojewski
2020-01-29 9:08 ` Takashi Iwai
0 siblings, 1 reply; 31+ messages in thread
From: Cezary Rojewski @ 2020-01-28 11:58 UTC (permalink / raw)
To: Vinod Koul; +Cc: pierre-louis.bossart, alsa-devel, broonie, lgirdwood, tiwai
On 2020-01-28 11:59, Vinod Koul wrote:
> On 28-01-20, 11:43, Cezary Rojewski wrote:
>> Update DMA buffer definition for snd_compr_runtime so it is represented
>> similarly as in snd_pcm_runtime. While at it, modify
>> snd_compr_set_runtime_buffer to account for newly added members.
>
> Please run ./scripts/get_maintainer.pl, it will tell you the people you
> should CC on a patch.
>
> Also Takashi already acked, so you should add the acks/reviews received
> in subsequent versions (unless they changed)
>
> And for this:
>
> Acked-by: Vinod Koul <vkoul@kernel.org>
>
No ALSA core & hda patches changed since v1. Sorry for missing the
Acked-by signature from Takashi. Should I resend and add the missing ack
in v4?
Czarek
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [alsa-devel] [PATCH v3 10/11] ASoC: SOF: Provide probe debugfs support
2020-01-28 10:43 ` [alsa-devel] [PATCH v3 10/11] ASoC: SOF: Provide probe debugfs support Cezary Rojewski
@ 2020-01-28 17:54 ` Sridharan, Ranjani
2020-01-28 18:59 ` Cezary Rojewski
2020-01-29 11:43 ` Mark Brown
0 siblings, 2 replies; 31+ messages in thread
From: Sridharan, Ranjani @ 2020-01-28 17:54 UTC (permalink / raw)
To: Cezary Rojewski
Cc: Pierre-Louis Bossart, Linux-ALSA, Mark Brown, Liam Girdwood, tiwai
On Tue, Jan 28, 2020 at 2:51 AM Cezary Rojewski <cezary.rojewski@intel.com>
wrote:
> Define debugfs subdirectory delegated for IPC communication with DSP.
> Input format: uint,uint,(...) which are later translated into DWORDS
> sequence and further into instances of struct of interest given the IPC
> type.
>
> For Extractor probes, following have been enabled:
> - PROBE_POINT_ADD (echo <..> probe_points)
> - PROBE_POINT_REMOVE (echo <..> probe_points_remove)
> - PROBE_POINT_INFO (cat probe_points)
>
> Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
> ---
>
> Changes in v2:
> - renamed debugfs probe functions as requested by Pierre
>
>
> sound/soc/sof/debug.c | 208 ++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 208 insertions(+)
>
> diff --git a/sound/soc/sof/debug.c b/sound/soc/sof/debug.c
> index d2b3b99d3a20..d38ab59e9a98 100644
> --- a/sound/soc/sof/debug.c
> +++ b/sound/soc/sof/debug.c
> @@ -17,6 +17,203 @@
> #include "sof-priv.h"
> #include "ops.h"
>
> +#if IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_PROBES)
> +#include "probe.h"
> +
> +/**
> + * strsplit_u32 - Split string into sequence of u32 tokens
> + * @buf: String to split into tokens.
> + * @delim: String containing delimiter characters.
> + * @tkns: Returned u32 sequence pointer.
> + * @num_tkns: Returned number of tokens obtained.
> + */
> +static int
> +strsplit_u32(char **buf, const char *delim, u32 **tkns, size_t *num_tkns)
> +{
> + char *s;
> + u32 *data, *tmp;
> + size_t count = 0;
> + size_t cap = 32;
> + int ret = 0;
> +
> + *tkns = NULL;
> + *num_tkns = 0;
> + data = kcalloc(cap, sizeof(*data), GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
> +
> + while ((s = strsep(buf, delim)) != NULL) {
> + ret = kstrtouint(s, 0, data + count);
> + if (ret)
> + goto exit;
> + if (++count >= cap) {
> + cap *= 2;
> + tmp = krealloc(data, cap * sizeof(*data),
> GFP_KERNEL);
> + if (!tmp) {
> + ret = -ENOMEM;
> + goto exit;
> + }
> + data = tmp;
> + }
> + }
> +
> + if (!count)
> + goto exit;
> + *tkns = kmemdup(data, count * sizeof(*data), GFP_KERNEL);
> + if (*tkns == NULL) {
> + ret = -ENOMEM;
> + goto exit;
> + }
> + *num_tkns = count;
> +
> +exit:
> + kfree(data);
> + return ret;
> +}
> +
> +static int tokenize_input(const char __user *from, size_t count,
> + loff_t *ppos, u32 **tkns, size_t *num_tkns)
> +{
> + char *buf;
> + int ret;
> +
> + buf = kmalloc(count + 1, GFP_KERNEL);
> + if (!buf)
> + return -ENOMEM;
> +
> + ret = simple_write_to_buffer(buf, count, ppos, from, count);
> + if (ret != count) {
> + ret = ret >= 0 ? -EIO : ret;
> + goto exit;
> + }
> +
> + buf[count] = '\0';
> + ret = strsplit_u32((char **)&buf, ",", tkns, num_tkns);
> +exit:
> + kfree(buf);
> + return ret;
> +}
> +
> +static ssize_t probe_points_read(struct file *file,
> + char __user *to, size_t count, loff_t *ppos)
> +{
> + struct snd_sof_dfsentry *dfse = file->private_data;
> + struct sof_probe_point_desc *desc;
> + size_t num_desc, len = 0;
> + char *buf;
> + int i, ret;
> +
> + buf = kzalloc(PAGE_SIZE, GFP_KERNEL);
> + if (!buf)
> + return -ENOMEM;
> +
> + ret = sof_ipc_probe_points_info(dfse->sdev, &desc, &num_desc);
> + if (ret < 0)
> + goto exit;
> +
> + for (i = 0; i < num_desc; i++) {
> + ret = snprintf(buf + len, PAGE_SIZE - len,
> + "Id: %#010x Purpose: %d Node id: %#x\n",
> + desc[i].buffer_id, desc[i].purpose,
> desc[i].stream_tag);
> + if (ret < 0)
> + goto free_desc;
> + len += ret;
> + }
> +
> + ret = simple_read_from_buffer(to, count, ppos, buf, len);
> +free_desc:
> + kfree(desc);
> +exit:
> + kfree(buf);
> + return ret;
> +}
> +
> +static ssize_t probe_points_write(struct file *file,
> + const char __user *from, size_t count, loff_t *ppos)
> +{
> + struct snd_sof_dfsentry *dfse = file->private_data;
> + struct sof_probe_point_desc *desc;
> + size_t num_tkns, bytes;
> + u32 *tkns;
> + int ret;
> +
> + ret = tokenize_input(from, count, ppos, &tkns, &num_tkns);
> + if (ret < 0)
> + return ret;
> + bytes = sizeof(*tkns) * num_tkns;
> + if (!num_tkns || (bytes % sizeof(*desc))) {
> + ret = -EINVAL;
> + goto exit;
> + }
> +
> + desc = (struct sof_probe_point_desc *)tkns;
> + ret = sof_ipc_probe_points_add(dfse->sdev,
> + desc, bytes / sizeof(*desc));
> + if (!ret)
> + ret = count;
> +exit:
> + kfree(tkns);
> + return ret;
> +}
> +
> +static const struct file_operations probe_points_fops = {
> + .open = simple_open,
> + .read = probe_points_read,
> + .write = probe_points_write,
> + .llseek = default_llseek,
> +};
> +
> +static ssize_t probe_points_remove_write(struct file *file,
> + const char __user *from, size_t count, loff_t *ppos)
> +{
> + struct snd_sof_dfsentry *dfse = file->private_data;
> + size_t num_tkns;
> + u32 *tkns;
> + int ret;
> +
> + ret = tokenize_input(from, count, ppos, &tkns, &num_tkns);
> + if (ret < 0)
> + return ret;
> + if (!num_tkns) {
> + ret = -EINVAL;
> + goto exit;
> + }
> +
> + ret = sof_ipc_probe_points_remove(dfse->sdev, tkns, num_tkns);
> + if (!ret)
> + ret = count;
> +exit:
> + kfree(tkns);
> + return ret;
> +}
> +
> +static const struct file_operations probe_points_remove_fops = {
> + .open = simple_open,
> + .write = probe_points_remove_write,
> + .llseek = default_llseek,
> +};
> +
> +static int snd_sof_debugfs_probe_item(struct snd_sof_dev *sdev,
> + const char *name, mode_t mode,
> + const struct file_operations *fops)
Hi Cezary,
Any particular reason to not use the existing snd_sof_debugfs_buf_item()
and adding a new one that does pretty much the same thing?
Thanks,
Ranjani
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [alsa-devel] [PATCH v3 00/11] ASoC: SOF: Data probing
2020-01-28 10:43 [alsa-devel] [PATCH v3 00/11] ASoC: SOF: Data probing Cezary Rojewski
` (10 preceding siblings ...)
2020-01-28 10:43 ` [alsa-devel] [PATCH v3 11/11] ASoC: SOF: Intel: Add Probe compress CPU DAIs Cezary Rojewski
@ 2020-01-28 17:55 ` Pierre-Louis Bossart
2020-01-28 19:29 ` Cezary Rojewski
11 siblings, 1 reply; 31+ messages in thread
From: Pierre-Louis Bossart @ 2020-01-28 17:55 UTC (permalink / raw)
To: Cezary Rojewski, alsa-devel; +Cc: broonie, tiwai, lgirdwood
On 1/28/20 4:43 AM, Cezary Rojewski wrote:
> This set of patches achieves few goals in order to enable data probing
> feature for audio DSP.
I am fine with this update, but I just thought of an obscure case and
couldn't find the answer on my own.
These probe points are enabled/disabled with direct IPC calls. Once
those routines have been called, I don't see any context maintained by
the driver (other than the stream tag for the extractor).
So here's my question: what happens if there is a pm_runtime or system
suspend after playing with debugfs to configure those probes? Would all
context be lost and the user needs to re-enable all probes?
Also what happens if there is a system suspend while an extractor is
opened, would it be restored? I imagine a pm_runtime suspend is not
possible since the device is active then.
I'd be fine if this was handled in a follow-up series, just want to see
if my question is relevant and if yes what it takes to support suspend.
Thanks
-Pierre
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [alsa-devel] [PATCH v3 10/11] ASoC: SOF: Provide probe debugfs support
2020-01-28 17:54 ` Sridharan, Ranjani
@ 2020-01-28 18:59 ` Cezary Rojewski
2020-01-29 11:43 ` Mark Brown
1 sibling, 0 replies; 31+ messages in thread
From: Cezary Rojewski @ 2020-01-28 18:59 UTC (permalink / raw)
To: Sridharan, Ranjani
Cc: Linux-ALSA, Mark Brown, tiwai, Pierre-Louis Bossart, Liam Girdwood
On 2020-01-28 18:54, Sridharan, Ranjani wrote:
> On Tue, Jan 28, 2020 at 2:51 AM Cezary Rojewski <cezary.rojewski@intel.com>
> wrote:
>
>> Define debugfs subdirectory delegated for IPC communication with DSP.
>> Input format: uint,uint,(...) which are later translated into DWORDS
>> sequence and further into instances of struct of interest given the IPC
>> type.
>>
>> For Extractor probes, following have been enabled:
>> - PROBE_POINT_ADD (echo <..> probe_points)
>> - PROBE_POINT_REMOVE (echo <..> probe_points_remove)
>> - PROBE_POINT_INFO (cat probe_points)
>>
>> Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
>> ---
>>
>> Changes in v2:
>> - renamed debugfs probe functions as requested by Pierre
>>
>>
>> +
>> +static int snd_sof_debugfs_probe_item(struct snd_sof_dev *sdev,
>> + const char *name, mode_t mode,
>> + const struct file_operations *fops)
>
> Hi Cezary,
>
> Any particular reason to not use the existing snd_sof_debugfs_buf_item()
> and adding a new one that does pretty much the same thing?
>
> Thanks,
> Ranjani
Thanks for the review Ranjani!
_buf_item() makes use of sof_dfs_fops - while probe items take different
handlers - and adds a special case for _IPC_FLOOT_TEST which I have no
knowledge of. In consequence, providing _probe_item() seems like a right
choice to me.
Czarek
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [alsa-devel] [PATCH v3 00/11] ASoC: SOF: Data probing
2020-01-28 17:55 ` [alsa-devel] [PATCH v3 00/11] ASoC: SOF: Data probing Pierre-Louis Bossart
@ 2020-01-28 19:29 ` Cezary Rojewski
2020-01-28 21:11 ` Pierre-Louis Bossart
0 siblings, 1 reply; 31+ messages in thread
From: Cezary Rojewski @ 2020-01-28 19:29 UTC (permalink / raw)
To: Pierre-Louis Bossart; +Cc: alsa-devel, broonie, tiwai, lgirdwood
On 2020-01-28 18:55, Pierre-Louis Bossart wrote:
> On 1/28/20 4:43 AM, Cezary Rojewski wrote:
>> This set of patches achieves few goals in order to enable data probing
>> feature for audio DSP.
>
> I am fine with this update, but I just thought of an obscure case and
> couldn't find the answer on my own.
>
> These probe points are enabled/disabled with direct IPC calls. Once
> those routines have been called, I don't see any context maintained by
> the driver (other than the stream tag for the extractor).
These do not need to be maintained by the driver at all, it's FW's job
actually. Notice the existence of _INFO getters for probe_points and
dmas. FW caches all the necessary info for us and when required, host
can request for it via IPCs.
Driver makes use of such operation during sof_probe_compr_free(). Before
_probe_deinit() is called, all the probe_points should be disconnected
and all the dmas detached. Since this patchset addresses extraction-only
(from the runtime point of view), the later is ignored.
>
> So here's my question: what happens if there is a pm_runtime or system
> suspend after playing with debugfs to configure those probes? Would all
> context be lost and the user needs to re-enable all probes?
>
> Also what happens if there is a system suspend while an extractor is
> opened, would it be restored? I imagine a pm_runtime suspend is not
> possible since the device is active then.
>
> I'd be fine if this was handled in a follow-up series, just want to see
> if my question is relevant and if yes what it takes to support suspend.
>
> Thanks
> -Pierre
As stated, there is no cache on the host side, caching is left up to FW
alone. Debugfs files act only as a ipc-proxies. As probes require device
to be up and running, suspend is not possible. After suspend, I believe
FW context would be lost so all the actions had to be repeated.
I'd suggest consulting SOF FW team in regard to probes design if you
want to pursue the suspend case - whether it is achievable or not.
Czarek
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [alsa-devel] [PATCH v3 00/11] ASoC: SOF: Data probing
2020-01-28 19:29 ` Cezary Rojewski
@ 2020-01-28 21:11 ` Pierre-Louis Bossart
2020-01-31 12:44 ` Cezary Rojewski
0 siblings, 1 reply; 31+ messages in thread
From: Pierre-Louis Bossart @ 2020-01-28 21:11 UTC (permalink / raw)
To: Cezary Rojewski; +Cc: alsa-devel, broonie, tiwai, lgirdwood
>> I am fine with this update, but I just thought of an obscure case and
>> couldn't find the answer on my own.
>>
>> These probe points are enabled/disabled with direct IPC calls. Once
>> those routines have been called, I don't see any context maintained by
>> the driver (other than the stream tag for the extractor).
>
> These do not need to be maintained by the driver at all, it's FW's job
> actually. Notice the existence of _INFO getters for probe_points and
> dmas. FW caches all the necessary info for us and when required, host
> can request for it via IPCs.
D'oh. That's a major disconnect I am afraid.
When the the PCI device enters D3 on APL+, the power rails are turned
off and the SOF firmware does not save any context. On D0 resume, the
power rails are turned back on, firmware is downloaded again, and
ALSA/ASoC/topology cores restore the context with a set of IPCs.
The behavior you describe might be relevant for previous versions of the
closed-source firmware but not for SOF as of today. The
firmware-initiated context save/restore just does not exist.
I also wonder if a firmware-only solution would work, the DMA stream
tags are allocated on the host side, so on resume you could have a
coherency issue with possibly mismatches.
> Driver makes use of such operation during sof_probe_compr_free(). Before
> _probe_deinit() is called, all the probe_points should be disconnected
> and all the dmas detached. Since this patchset addresses extraction-only
> (from the runtime point of view), the later is ignored.
>
>>
>> So here's my question: what happens if there is a pm_runtime or system
>> suspend after playing with debugfs to configure those probes? Would
>> all context be lost and the user needs to re-enable all probes?
>>
>> Also what happens if there is a system suspend while an extractor is
>> opened, would it be restored? I imagine a pm_runtime suspend is not
>> possible since the device is active then.
> As stated, there is no cache on the host side, caching is left up to FW
> alone. Debugfs files act only as a ipc-proxies. As probes require device
> to be up and running, suspend is not possible. After suspend, I believe
> FW context would be lost so all the actions had to be repeated.
>
> I'd suggest consulting SOF FW team in regard to probes design if you
> want to pursue the suspend case - whether it is achievable or not.
That team is in your building :-)
suspend/resume support is a requirement for all SOF capabilities - no
exceptions. At minimum, you would want to do a pm_runtime_get_sync() to
prevent runtime_pm from kicking-in while probes are enabled.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [alsa-devel] [PATCH v3 08/11] ASoC: SOF: Generic probe compress operations
2020-01-28 10:43 ` [alsa-devel] [PATCH v3 08/11] ASoC: SOF: Generic probe compress operations Cezary Rojewski
@ 2020-01-29 7:48 ` Daniel Baluta
2020-01-29 9:04 ` Cezary Rojewski
0 siblings, 1 reply; 31+ messages in thread
From: Daniel Baluta @ 2020-01-29 7:48 UTC (permalink / raw)
To: cezary.rojewski, alsa-devel
Cc: tiwai, broonie, lgirdwood, pierre-louis.bossart
One comment below:
> +int sof_probe_compr_set_params(struct snd_compr_stream *cstream,
> + struct snd_compr_params *params, struct snd_soc_dai
> *dai)
> +{
> + struct snd_compr_runtime *rtd = cstream->runtime;
> + struct snd_sof_dev *sdev =
> + snd_soc_component_get_drvdata(dai-
> >component);
> + int ret;
> +
> + cstream->dma_buffer.dev.type = SNDRV_DMA_TYPE_DEV_SG;
> + cstream->dma_buffer.dev.dev = sdev->dev;
> + ret = snd_compr_malloc_pages(cstream, rtd->buffer_size);
> + if (ret < 0)
> + return ret;
> +
> + ret = snd_sof_probe_compr_set_params(sdev, cstream, params,
> dai);
> + if (ret < 0)
> + return ret;
> +
> + ret = sof_ipc_probe_init(sdev, sdev->extractor_stream_tag,
> + rtd->dma_bytes);
> + if (ret < 0) {
> + dev_err(dai->dev, "Failed to init probe: %d\n", ret);
> + return ret;
> + }
>
Should we call snd_compr_free_pages on error case?
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [alsa-devel] [PATCH v3 09/11] ASoC: SOF: Intel: Probe compress operations
2020-01-28 10:43 ` [alsa-devel] [PATCH v3 09/11] ASoC: SOF: Intel: Probe " Cezary Rojewski
@ 2020-01-29 7:55 ` Daniel Baluta
2020-01-29 9:09 ` Cezary Rojewski
0 siblings, 1 reply; 31+ messages in thread
From: Daniel Baluta @ 2020-01-29 7:55 UTC (permalink / raw)
To: cezary.rojewski, alsa-devel
Cc: tiwai, broonie, lgirdwood, pierre-louis.bossart
On Tue, 2020-01-28 at 11:43 +0100, Cezary Rojewski wrote:
> diff --git a/sound/soc/sof/intel/hda-compress.c
> b/sound/soc/sof/intel/hda-compress.c
> new file mode 100644
> index 000000000000..da7de867d0af
> --- /dev/null
> +++ b/sound/soc/sof/intel/hda-compress.c
> @@ -0,0 +1,132 @@
> +// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
I think SPDX is enough.
> +//
> +// This file is provided under a dual BSD/GPLv2 license. When using
> or
> +// redistributing this file, you may do so under either license.
No need to add this boilerplate code.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [alsa-devel] [PATCH v3 11/11] ASoC: SOF: Intel: Add Probe compress CPU DAIs
2020-01-28 10:43 ` [alsa-devel] [PATCH v3 11/11] ASoC: SOF: Intel: Add Probe compress CPU DAIs Cezary Rojewski
@ 2020-01-29 8:04 ` Daniel Baluta
2020-01-29 9:24 ` Cezary Rojewski
0 siblings, 1 reply; 31+ messages in thread
From: Daniel Baluta @ 2020-01-29 8:04 UTC (permalink / raw)
To: cezary.rojewski, alsa-devel
Cc: tiwai, broonie, lgirdwood, pierre-louis.bossart
I'm not really happy with the changes inside pcm.c
On Tue, 2020-01-28 at 11:43 +0100, Cezary Rojewski wrote:
> --- a/sound/soc/sof/pcm.c
> +++ b/sound/soc/sof/pcm.c
> @@ -756,6 +756,15 @@ static void sof_pcm_remove(struct
> snd_soc_component *component)
> snd_soc_tplg_component_remove(component,
> SND_SOC_TPLG_INDEX_ALL);
> }
>
> +#if IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_PROBES)
> +#include "compress.h"
> +
> struct snd_compr_ops sof_compressed_ops = {+
> + .copy = sof_probe_compr_copy,
> +};
> +EXPORT_SYMBOL(sof_compressed_ops);
> +#endif
> +
Maybe call this structure sof_probe_compressed ops. Othwerwise you will
conflict with the real sof_compressed_ops.
> void snd_sof_new_platform_drv(struct snd_sof_dev *sdev)
> {
> struct snd_soc_component_driver *pd = &sdev->plat_drv;
> @@ -775,7 +784,7 @@ void snd_sof_new_platform_drv(struct snd_sof_dev
> *sdev)
> pd->trigger = sof_pcm_trigger;
> pd->pointer = sof_pcm_pointer;
>
> -#if IS_ENABLED(CONFIG_SND_SOC_SOF_COMPRESS)
> +#if IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_PROBES)
> pd->compr_ops = &sof_compressed_ops;
> #endif
> pd->pcm_construct = sof_pcm_new;
>
Here you are breaking the non-existent yet compressed support. I would
leave:
#if IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_PROBES)
pd->compr_ops = &sof_compressed_ops;
#endif
and only override compr_ops if SND_SOC_SOF_DEBUG_PROBES is set like
this:
#if IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_PROBES)
pd->compr_ops = &sof_probe_compressed_ops;
#endif
Also does this mean we cannot support both "real" compressed sound card
and probes in the same time?
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [alsa-devel] [PATCH v3 08/11] ASoC: SOF: Generic probe compress operations
2020-01-29 7:48 ` Daniel Baluta
@ 2020-01-29 9:04 ` Cezary Rojewski
0 siblings, 0 replies; 31+ messages in thread
From: Cezary Rojewski @ 2020-01-29 9:04 UTC (permalink / raw)
To: Daniel Baluta; +Cc: alsa-devel, broonie, tiwai, lgirdwood, pierre-louis.bossart
On 2020-01-29 08:48, Daniel Baluta wrote:
> One comment below:
Thanks for review Daniel!
>
>> +int sof_probe_compr_set_params(struct snd_compr_stream *cstream,
>> + struct snd_compr_params *params, struct snd_soc_dai
>> *dai)
>> +{
>> + struct snd_compr_runtime *rtd = cstream->runtime;
>> + struct snd_sof_dev *sdev =
>> + snd_soc_component_get_drvdata(dai-
>>> component);
>> + int ret;
>> +
>> + cstream->dma_buffer.dev.type = SNDRV_DMA_TYPE_DEV_SG;
>> + cstream->dma_buffer.dev.dev = sdev->dev;
>> + ret = snd_compr_malloc_pages(cstream, rtd->buffer_size);
>> + if (ret < 0)
>> + return ret;
>> +
>> + ret = snd_sof_probe_compr_set_params(sdev, cstream, params,
>> dai);
>> + if (ret < 0)
>> + return ret;
>> +
>> + ret = sof_ipc_probe_init(sdev, sdev->extractor_stream_tag,
>> + rtd->dma_bytes);
>> + if (ret < 0) {
>> + dev_err(dai->dev, "Failed to init probe: %d\n", ret);
>> + return ret;
>> + }
>>
>
> Should we call snd_compr_free_pages on error case?
>
_set_params() failure is automatically followed by _free(). This is done
by the compr core. As you can see in the _free() itself, regardless of
IPCs outcome, code will get to _free_pages() and thus all occupied
resources are released.
Czarek
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [alsa-devel] [PATCH v3 04/11] ALSA: core: Expand DMA buffer information
2020-01-28 11:58 ` Cezary Rojewski
@ 2020-01-29 9:08 ` Takashi Iwai
0 siblings, 0 replies; 31+ messages in thread
From: Takashi Iwai @ 2020-01-29 9:08 UTC (permalink / raw)
To: Cezary Rojewski
Cc: alsa-devel, lgirdwood, pierre-louis.bossart, tiwai, Vinod Koul, broonie
On Tue, 28 Jan 2020 12:58:18 +0100,
Cezary Rojewski wrote:
>
> On 2020-01-28 11:59, Vinod Koul wrote:
> > On 28-01-20, 11:43, Cezary Rojewski wrote:
> >> Update DMA buffer definition for snd_compr_runtime so it is represented
> >> similarly as in snd_pcm_runtime. While at it, modify
> >> snd_compr_set_runtime_buffer to account for newly added members.
> >
> > Please run ./scripts/get_maintainer.pl, it will tell you the people you
> > should CC on a patch.
> >
> > Also Takashi already acked, so you should add the acks/reviews received
> > in subsequent versions (unless they changed)
> >
> > And for this:
> >
> > Acked-by: Vinod Koul <vkoul@kernel.org>
> >
>
> No ALSA core & hda patches changed since v1. Sorry for missing the
> Acked-by signature from Takashi. Should I resend and add the missing
> ack in v4?
Not necessary just for the lack of my ack.
Of course, if you plan to submit v4 in anyway later, feel free to add
them there :)
thanks,
Takashi
>
> Czarek
>
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [alsa-devel] [PATCH v3 09/11] ASoC: SOF: Intel: Probe compress operations
2020-01-29 7:55 ` Daniel Baluta
@ 2020-01-29 9:09 ` Cezary Rojewski
0 siblings, 0 replies; 31+ messages in thread
From: Cezary Rojewski @ 2020-01-29 9:09 UTC (permalink / raw)
To: Daniel Baluta; +Cc: alsa-devel, broonie, tiwai, lgirdwood, pierre-louis.bossart
On 2020-01-29 08:55, Daniel Baluta wrote:
> On Tue, 2020-01-28 at 11:43 +0100, Cezary Rojewski wrote:
>> diff --git a/sound/soc/sof/intel/hda-compress.c
>> b/sound/soc/sof/intel/hda-compress.c
>> new file mode 100644
>> index 000000000000..da7de867d0af
>> --- /dev/null
>> +++ b/sound/soc/sof/intel/hda-compress.c
>> @@ -0,0 +1,132 @@
>> +// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
>
> I think SPDX is enough.
>
>> +//
>> +// This file is provided under a dual BSD/GPLv2 license. When using
>> or
>> +// redistributing this file, you may do so under either license.
>
> No need to add this boilerplate code.
>
Header shares structure of all other headers within sof directory. If
your claim is true, then all headers have to be updated, not just this
one. Such change should be provided in a separate patch/ series.
Czarek
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [alsa-devel] [PATCH v3 11/11] ASoC: SOF: Intel: Add Probe compress CPU DAIs
2020-01-29 8:04 ` Daniel Baluta
@ 2020-01-29 9:24 ` Cezary Rojewski
2020-01-29 9:46 ` Daniel Baluta
0 siblings, 1 reply; 31+ messages in thread
From: Cezary Rojewski @ 2020-01-29 9:24 UTC (permalink / raw)
To: Daniel Baluta; +Cc: alsa-devel, broonie, tiwai, lgirdwood, pierre-louis.bossart
On 2020-01-29 09:04, Daniel Baluta wrote:
> I'm not really happy with the changes inside pcm.c
>
>
> On Tue, 2020-01-28 at 11:43 +0100, Cezary Rojewski wrote:
>> --- a/sound/soc/sof/pcm.c
>> +++ b/sound/soc/sof/pcm.c
>> @@ -756,6 +756,15 @@ static void sof_pcm_remove(struct
>> snd_soc_component *component)
>> snd_soc_tplg_component_remove(component,
>> SND_SOC_TPLG_INDEX_ALL);
>> }
>>
>> +#if IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_PROBES)
>> +#include "compress.h"
>> +
>> struct snd_compr_ops sof_compressed_ops = {+
>> + .copy = sof_probe_compr_copy,
>> +};
>> +EXPORT_SYMBOL(sof_compressed_ops);
>> +#endif
>> +
>
> Maybe call this structure sof_probe_compressed ops. Othwerwise you will
> conflict with the real sof_compressed_ops.
>
>
>> void snd_sof_new_platform_drv(struct snd_sof_dev *sdev)
>> {
>> struct snd_soc_component_driver *pd = &sdev->plat_drv;
>> @@ -775,7 +784,7 @@ void snd_sof_new_platform_drv(struct snd_sof_dev
>> *sdev)
>> pd->trigger = sof_pcm_trigger;
>> pd->pointer = sof_pcm_pointer;
>>
>> -#if IS_ENABLED(CONFIG_SND_SOC_SOF_COMPRESS)
>> +#if IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_PROBES)
>> pd->compr_ops = &sof_compressed_ops;
>> #endif
>> pd->pcm_construct = sof_pcm_new;
>>
>
> Here you are breaking the non-existent yet compressed support. I would
> leave:
>
> #if IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_PROBES)
> pd->compr_ops = &sof_compressed_ops;
> #endif
>
> and only override compr_ops if SND_SOC_SOF_DEBUG_PROBES is set like
> this:
>
>
> #if IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_PROBES)
> pd->compr_ops = &sof_probe_compressed_ops;
> #endif
>
> Also does this mean we cannot support both "real" compressed sound card
> and probes in the same time?
>
>
Thanks for the review Daniel!
Tthe example you provided is not very clear to me - same condition is
added for both assignments, but I'll try to answer your question.
Existing "sof_compressed_ops" don't even exist, as you said it yourself
so nothing is lost. Changes within pcm.c are gated by _DEBUG_PROBES
anyway so the solution is actually very clean.
While DAI can have different cops, platform driver cannot. I'm aware of
the problem but till actual compress support for SOF comes out, minimal,
probe-only changes were a better choice IMHO. After all, that's not a
problem to make this code smarter in the future - not a farseer though,
can't predict what you're going to add for SOF-compr :)
Czarek
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [alsa-devel] [PATCH v3 11/11] ASoC: SOF: Intel: Add Probe compress CPU DAIs
2020-01-29 9:24 ` Cezary Rojewski
@ 2020-01-29 9:46 ` Daniel Baluta
2020-01-31 12:34 ` Cezary Rojewski
0 siblings, 1 reply; 31+ messages in thread
From: Daniel Baluta @ 2020-01-29 9:46 UTC (permalink / raw)
To: cezary.rojewski
Cc: pierre-louis.bossart, daniel.baluta, alsa-devel, tiwai,
lgirdwood, broonie
On Wed, 2020-01-29 at 10:24 +0100, Cezary Rojewski wrote:
> On 2020-01-29 09:04, Daniel Baluta wrote:
> > I'm not really happy with the changes inside pcm.c
> >
> >
> > On Tue, 2020-01-28 at 11:43 +0100, Cezary Rojewski wrote:
> > > --- a/sound/soc/sof/pcm.c
> > > +++ b/sound/soc/sof/pcm.c
> > > @@ -756,6 +756,15 @@ static void sof_pcm_remove(struct
> > > snd_soc_component *component)
> > > snd_soc_tplg_component_remove(component,
> > > SND_SOC_TPLG_INDEX_ALL);
> > > }
> > >
> > > +#if IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_PROBES)
> > > +#include "compress.h"
> > > +
> > > struct snd_compr_ops sof_compressed_ops = {+
> > > + .copy = sof_probe_compr_copy,
> > > +};
> > > +EXPORT_SYMBOL(sof_compressed_ops);
> > > +#endif
> > > +
> >
> > Maybe call this structure sof_probe_compressed ops. Othwerwise you
> > will
> > conflict with the real sof_compressed_ops.
> >
> >
> > > void snd_sof_new_platform_drv(struct snd_sof_dev *sdev)
> > > {
> > > struct snd_soc_component_driver *pd = &sdev->plat_drv;
> > > @@ -775,7 +784,7 @@ void snd_sof_new_platform_drv(struct
> > > snd_sof_dev
> > > *sdev)
> > > pd->trigger = sof_pcm_trigger;
> > > pd->pointer = sof_pcm_pointer;
> > >
> > > -#if IS_ENABLED(CONFIG_SND_SOC_SOF_COMPRESS)
> > > +#if IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_PROBES)
> > > pd->compr_ops = &sof_compressed_ops;
> > > #endif
> > > pd->pcm_construct = sof_pcm_new;
> > >
> >
> > Here you are breaking the non-existent yet compressed support. I
> > would
> > leave:
> >
> > #if IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_PROBES)
> > pd->compr_ops = &sof_compressed_ops;
> > #endif
> >
> > and only override compr_ops if SND_SOC_SOF_DEBUG_PROBES is set like
> > this:
> >
> >
> > #if IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_PROBES)
> > pd->compr_ops = &sof_probe_compressed_ops;
> > #endif
> >
> > Also does this mean we cannot support both "real" compressed sound
> > card
> > and probes in the same time?
> >
> >
>
> Thanks for the review Daniel!
>
> Tthe example you provided is not very clear to me - same condition
> is
> added for both assignments, but I'll try to answer your question.
>
> Existing "sof_compressed_ops" don't even exist, as you said it
> yourself
> so nothing is lost. Changes within pcm.c are gated by _DEBUG_PROBES
> anyway so the solution is actually very clean.
>
> While DAI can have different cops, platform driver cannot. I'm aware
> of
> the problem but till actual compress support for SOF comes out,
> minimal,
> probe-only changes were a better choice IMHO. After all, that's not
> a
> problem to make this code smarter in the future - not a farseer
> though,
> can't predict what you're going to add for SOF-compr :)
>
Indeed, we can make the code smarter later but I want to do the code
cleaner now.
I think I had a copy paste error when providing the example.
So, my proposal is to override the platform driver compr_ops field
with probe compressed ops when CONFIG_SND_SOC_SOF_DEBUG_PROBES is set.
The code could look like this in the end:
#if IS_ENABLED(CONFIG_SND_SOC_SOF_COMPRESS)
pd->compr_ops = &sof_compressed_ops;
#endif
/* Add a comment explaining that we are doing override in case of
probes */
#if IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_PROBES)
pd->compr_ops = &sof_probe_compressed_ops;
#endif
Also, I think there is no need to define the probe compressed ops
inside pcm.c
So, instead of
#if IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_PROBES)
#include "compress.h"
struct snd_compr_ops sof_compressed_ops = {
.copy = sof_probe_compr_copy,
};
EXPORT_SYMBOL(sof_compressed_ops);
#endif
We can do:
#if IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_PROBES)
extern snd_compr_ops sof_probe_compressed_ops;
#endif
or even better include a header with the declaration.
And add the definition of sof_probe_compressed_ops would be somewhere
in the compressed probe file.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [alsa-devel] [PATCH v3 10/11] ASoC: SOF: Provide probe debugfs support
2020-01-28 17:54 ` Sridharan, Ranjani
2020-01-28 18:59 ` Cezary Rojewski
@ 2020-01-29 11:43 ` Mark Brown
1 sibling, 0 replies; 31+ messages in thread
From: Mark Brown @ 2020-01-29 11:43 UTC (permalink / raw)
To: Sridharan, Ranjani
Cc: Pierre-Louis Bossart, Cezary Rojewski, tiwai, Linux-ALSA, Liam Girdwood
[-- Attachment #1.1: Type: text/plain, Size: 613 bytes --]
On Tue, Jan 28, 2020 at 09:54:41AM -0800, Sridharan, Ranjani wrote:
> On Tue, Jan 28, 2020 at 2:51 AM Cezary Rojewski <cezary.rojewski@intel.com>
> wrote:
>
> > Define debugfs subdirectory delegated for IPC communication with DSP.
> > Input format: uint,uint,(...) which are later translated into DWORDS
> > sequence and further into instances of struct of interest given the IPC
Please delete unneeded context from mails when replying. Doing this
makes it much easier to find your reply in the message, helping ensure
it won't be missed by people scrolling through the irrelevant quoted
material.
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 161 bytes --]
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [alsa-devel] [PATCH v3 11/11] ASoC: SOF: Intel: Add Probe compress CPU DAIs
2020-01-29 9:46 ` Daniel Baluta
@ 2020-01-31 12:34 ` Cezary Rojewski
0 siblings, 0 replies; 31+ messages in thread
From: Cezary Rojewski @ 2020-01-31 12:34 UTC (permalink / raw)
To: Daniel Baluta
Cc: pierre-louis.bossart, daniel.baluta, alsa-devel, tiwai,
lgirdwood, broonie
On 2020-01-29 10:46, Daniel Baluta wrote:
> On Wed, 2020-01-29 at 10:24 +0100, Cezary Rojewski wrote:
>>
>> Thanks for the review Daniel!
>>
>> Tthe example you provided is not very clear to me - same condition
>> is
>> added for both assignments, but I'll try to answer your question.
>>
>> Existing "sof_compressed_ops" don't even exist, as you said it
>> yourself
>> so nothing is lost. Changes within pcm.c are gated by _DEBUG_PROBES
>> anyway so the solution is actually very clean.
>>
>> While DAI can have different cops, platform driver cannot. I'm aware
>> of
>> the problem but till actual compress support for SOF comes out,
>> minimal,
>> probe-only changes were a better choice IMHO. After all, that's not
>> a
>> problem to make this code smarter in the future - not a farseer
>> though,
>> can't predict what you're going to add for SOF-compr :)
>>
>
> Indeed, we can make the code smarter later but I want to do the code
> cleaner now.
>
> I think I had a copy paste error when providing the example.
>
> So, my proposal is to override the platform driver compr_ops field
> with probe compressed ops when CONFIG_SND_SOC_SOF_DEBUG_PROBES is set.
>
> The code could look like this in the end:
>
> #if IS_ENABLED(CONFIG_SND_SOC_SOF_COMPRESS)
> pd->compr_ops = &sof_compressed_ops;
> #endif
>
> /* Add a comment explaining that we are doing override in case of
> probes */
>
> #if IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_PROBES)
> pd->compr_ops = &sof_probe_compressed_ops;
> #endif
>
> Also, I think there is no need to define the probe compressed ops
> inside pcm.c
>
> So, instead of
>
> #if IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_PROBES)
> #include "compress.h"
>
> struct snd_compr_ops sof_compressed_ops = {
> .copy = sof_probe_compr_copy,
> };
> EXPORT_SYMBOL(sof_compressed_ops);
> #endif
>
> We can do:
> #if IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_PROBES)
> extern snd_compr_ops sof_probe_compressed_ops;
> #endif
>
> or even better include a header with the declaration.
>
> And add the definition of sof_probe_compressed_ops would be somewhere
> in the compressed probe file.
>
>
Your comments have been addressed in v4. Non-existant cops usage have
been re-added and is now overridden by probes when enabled.
sof_probe_compressed_ops declaration moved to compress.c file as requested.
Czarek
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [alsa-devel] [PATCH v3 00/11] ASoC: SOF: Data probing
2020-01-28 21:11 ` Pierre-Louis Bossart
@ 2020-01-31 12:44 ` Cezary Rojewski
0 siblings, 0 replies; 31+ messages in thread
From: Cezary Rojewski @ 2020-01-31 12:44 UTC (permalink / raw)
To: Pierre-Louis Bossart; +Cc: alsa-devel, broonie, tiwai, lgirdwood
On 2020-01-28 22:11, Pierre-Louis Bossart wrote:
>
>>> I am fine with this update, but I just thought of an obscure case and
>>> couldn't find the answer on my own.
>>>
>>> These probe points are enabled/disabled with direct IPC calls. Once
>>> those routines have been called, I don't see any context maintained
>>> by the driver (other than the stream tag for the extractor).
>>
>> These do not need to be maintained by the driver at all, it's FW's job
>> actually. Notice the existence of _INFO getters for probe_points and
>> dmas. FW caches all the necessary info for us and when required, host
>> can request for it via IPCs.
>
> D'oh. That's a major disconnect I am afraid.
>
> When the the PCI device enters D3 on APL+, the power rails are turned
> off and the SOF firmware does not save any context. On D0 resume, the
> power rails are turned back on, firmware is downloaded again, and
> ALSA/ASoC/topology cores restore the context with a set of IPCs.
>
> The behavior you describe might be relevant for previous versions of the
> closed-source firmware but not for SOF as of today. The
> firmware-initiated context save/restore just does not exist.
>
> I also wonder if a firmware-only solution would work, the DMA stream
> tags are allocated on the host side, so on resume you could have a
> coherency issue with possibly mismatches.
>
>> Driver makes use of such operation during sof_probe_compr_free().
>> Before _probe_deinit() is called, all the probe_points should be
>> disconnected and all the dmas detached. Since this patchset addresses
>> extraction-only (from the runtime point of view), the later is ignored.
>>
>>>
>>> So here's my question: what happens if there is a pm_runtime or
>>> system suspend after playing with debugfs to configure those probes?
>>> Would all context be lost and the user needs to re-enable all probes?
>>>
>>> Also what happens if there is a system suspend while an extractor is
>>> opened, would it be restored? I imagine a pm_runtime suspend is not
>>> possible since the device is active then.
>
>> As stated, there is no cache on the host side, caching is left up to
>> FW alone. Debugfs files act only as a ipc-proxies. As probes require
>> device to be up and running, suspend is not possible. After suspend, I
>> believe FW context would be lost so all the actions had to be repeated.
>>
>> I'd suggest consulting SOF FW team in regard to probes design if you
>> want to pursue the suspend case - whether it is achievable or not.
>
> That team is in your building :-)
>
> suspend/resume support is a requirement for all SOF capabilities - no
> exceptions. At minimum, you would want to do a pm_runtime_get_sync() to
> prevent runtime_pm from kicking-in while probes are enabled.
To make it clear: probes are not supported during D3/ S3.
Now to be transparent with the rest of the readers: change keeping the
device alive throughout the entire lifetime of extractor compress stream
has been merged on 2019-12-17:
https://lore.kernel.org/alsa-devel/20191217095851.19629-7-cezary.rojewski@intel.com/
so we're sure the suspend scenario will never occur.
However, indeed we could be more verbose - so user is not confused why
his actions executed on probe-debugfs files are failing. To address
this, v4 debugfs handlers output additional messages when invoked with
no extraction stream running.
Czarek
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 31+ messages in thread
end of thread, other threads:[~2020-01-31 12:46 UTC | newest]
Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-28 10:43 [alsa-devel] [PATCH v3 00/11] ASoC: SOF: Data probing Cezary Rojewski
2020-01-28 10:43 ` [alsa-devel] [PATCH v3 01/11] ALSA: hda: Allow for compress stream to hdac_ext_stream assignment Cezary Rojewski
2020-01-28 10:43 ` [alsa-devel] [PATCH v3 02/11] ALSA: hda: Prepare for compress stream support Cezary Rojewski
2020-01-28 10:43 ` [alsa-devel] [PATCH v3 03/11] ALSA: hda: Interrupt servicing and BDL setup for compress streams Cezary Rojewski
2020-01-28 10:43 ` [alsa-devel] [PATCH v3 04/11] ALSA: core: Expand DMA buffer information Cezary Rojewski
2020-01-28 10:59 ` Vinod Koul
2020-01-28 11:58 ` Cezary Rojewski
2020-01-29 9:08 ` Takashi Iwai
2020-01-28 10:43 ` [alsa-devel] [PATCH v3 05/11] ALSA: core: Implement compress page allocation and free routines Cezary Rojewski
2020-01-28 11:05 ` Vinod Koul
2020-01-28 10:43 ` [alsa-devel] [PATCH v3 06/11] ASoC: SOF: Intel: Account for compress streams when servicing IRQs Cezary Rojewski
2020-01-28 10:43 ` [alsa-devel] [PATCH v3 07/11] ASoC: SOF: Implement Probe IPC API Cezary Rojewski
2020-01-28 10:43 ` [alsa-devel] [PATCH v3 08/11] ASoC: SOF: Generic probe compress operations Cezary Rojewski
2020-01-29 7:48 ` Daniel Baluta
2020-01-29 9:04 ` Cezary Rojewski
2020-01-28 10:43 ` [alsa-devel] [PATCH v3 09/11] ASoC: SOF: Intel: Probe " Cezary Rojewski
2020-01-29 7:55 ` Daniel Baluta
2020-01-29 9:09 ` Cezary Rojewski
2020-01-28 10:43 ` [alsa-devel] [PATCH v3 10/11] ASoC: SOF: Provide probe debugfs support Cezary Rojewski
2020-01-28 17:54 ` Sridharan, Ranjani
2020-01-28 18:59 ` Cezary Rojewski
2020-01-29 11:43 ` Mark Brown
2020-01-28 10:43 ` [alsa-devel] [PATCH v3 11/11] ASoC: SOF: Intel: Add Probe compress CPU DAIs Cezary Rojewski
2020-01-29 8:04 ` Daniel Baluta
2020-01-29 9:24 ` Cezary Rojewski
2020-01-29 9:46 ` Daniel Baluta
2020-01-31 12:34 ` Cezary Rojewski
2020-01-28 17:55 ` [alsa-devel] [PATCH v3 00/11] ASoC: SOF: Data probing Pierre-Louis Bossart
2020-01-28 19:29 ` Cezary Rojewski
2020-01-28 21:11 ` Pierre-Louis Bossart
2020-01-31 12:44 ` Cezary Rojewski
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).