diff --git a/sound/core/compress_offload.c b/sound/core/compress_offload.c index ad68132b36f637..5c0db178d635f2 100644 --- a/sound/core/compress_offload.c +++ b/sound/core/compress_offload.c @@ -102,7 +102,14 @@ static int snd_compr_open(struct inode *inode, struct file *f) return -ENODEV; } + ret = snd_card_file_add(compr->card, f); + if (ret < 0) { + snd_card_unref(compr->card); + return ret; + } + if (!try_module_get(compr->card->module)) { + snd_card_file_remove(compr->card, f); snd_card_unref(compr->card); return -EFAULT; } @@ -149,6 +156,7 @@ static int snd_compr_open(struct inode *inode, struct file *f) kfree(runtime); kfree(data); module_put(compr->card->module); + snd_card_file_remove(compr->card, f); } snd_card_unref(compr->card); return ret; @@ -185,6 +193,7 @@ static int snd_compr_free(struct inode *inode, struct file *f) module_put(data->stream.device->card->module); kfree(data->stream.runtime); kfree(data); + snd_card_file_remove(compr->card, f); return 0; } @@ -938,8 +947,10 @@ static int snd_compress_wait_for_drain(struct snd_compr_stream *stream) * It is expected that driver will notify the drain completion and then * stream will be moved to SETUP state, even if draining resulted in an * error. We can trigger next track after this. + * + * The state has been set to SNDRV_PCM_STATE_DRAINING by the caller, + * before the drain trigger was issued. */ - stream->runtime->state = SNDRV_PCM_STATE_DRAINING; mutex_unlock(&stream->device->lock); /* we wait for drain to complete here, drain can return when @@ -964,6 +975,7 @@ static int snd_compress_wait_for_drain(struct snd_compr_stream *stream) static int snd_compr_drain(struct snd_compr_stream *stream) { + snd_pcm_state_t state; int retval; switch (stream->runtime->state) { @@ -978,9 +990,20 @@ static int snd_compr_drain(struct snd_compr_stream *stream) break; } + /* + * The state must be set to draining before the trigger is issued as the + * drain can be completed by the driver either from the trigger callback + * itself or from a different context as soon as the trigger is called. + * A snd_compr_drain_notify() arriving before the state is updated would + * be lost and the wait for the drain to finish would never be woken up. + */ + state = stream->runtime->state; + stream->runtime->state = SNDRV_PCM_STATE_DRAINING; + retval = stream->ops->trigger(stream, SND_COMPR_TRIGGER_DRAIN); if (retval) { pr_debug("SND_COMPR_TRIGGER_DRAIN failed %d\n", retval); + stream->runtime->state = state; wake_up(&stream->runtime->sleep); return retval; } @@ -1016,6 +1039,7 @@ static int snd_compr_next_track(struct snd_compr_stream *stream) static int snd_compr_partial_drain(struct snd_compr_stream *stream) { + snd_pcm_state_t state; int retval; switch (stream->runtime->state) { @@ -1039,9 +1063,16 @@ static int snd_compr_partial_drain(struct snd_compr_stream *stream) return -EPERM; stream->partial_drain = true; + + /* See the comment in snd_compr_drain() on the state handling */ + state = stream->runtime->state; + stream->runtime->state = SNDRV_PCM_STATE_DRAINING; + retval = stream->ops->trigger(stream, SND_COMPR_TRIGGER_PARTIAL_DRAIN); if (retval) { pr_debug("Partial drain returned failure\n"); + stream->partial_drain = false; + stream->runtime->state = state; wake_up(&stream->runtime->sleep); return retval; } diff --git a/sound/soc/soc-compress.c b/sound/soc/soc-compress.c index 9fac3cf645df27..a941ed0908c667 100644 --- a/sound/soc/soc-compress.c +++ b/sound/soc/soc-compress.c @@ -147,6 +147,8 @@ static int soc_compr_open(struct snd_compr_stream *cstream) static int soc_compr_alloc_fe_runtime(struct snd_soc_pcm_runtime *fe, int stream) { struct snd_pcm_substream *fe_substream = snd_soc_dpcm_get_substream(fe, stream); + struct snd_pcm_hw_constraints *constrs; + int i; if (!fe_substream || fe_substream->runtime) return 0; @@ -155,6 +157,23 @@ static int soc_compr_alloc_fe_runtime(struct snd_soc_pcm_runtime *fe, int stream if (!fe_substream->runtime) return -ENOMEM; + /* + * snd_pcm_open() initializes the constraints of a PCM runtime to + * 'anything goes'. A zeroed one means an empty mask and a [0, 0] + * interval instead, which the constraint helpers refine against and + * reject, so initialize them the same way the PCM core does. + * + * The hw rules snd_pcm_hw_constraints_init() installs on top are only + * evaluated by snd_pcm_hw_refine(), which never runs for this + * substream, so they are not needed here. + */ + constrs = &fe_substream->runtime->hw_constraints; + for (i = SNDRV_PCM_HW_PARAM_FIRST_MASK; i <= SNDRV_PCM_HW_PARAM_LAST_MASK; i++) + snd_mask_any(constrs_mask(constrs, i)); + + for (i = SNDRV_PCM_HW_PARAM_FIRST_INTERVAL; i <= SNDRV_PCM_HW_PARAM_LAST_INTERVAL; i++) + snd_interval_any(constrs_interval(constrs, i)); + return 0; } @@ -418,9 +437,15 @@ static int soc_compr_trigger_fe(struct snd_compr_stream *cstream, int cmd) bool fe_first; int ret; - if (cmd == SND_COMPR_TRIGGER_PARTIAL_DRAIN || - cmd == SND_COMPR_TRIGGER_DRAIN) + /* FE only triggers */ + switch (cmd) { + case SND_COMPR_TRIGGER_PARTIAL_DRAIN: + case SND_COMPR_TRIGGER_NEXT_TRACK: + case SND_COMPR_TRIGGER_DRAIN: return snd_soc_component_compr_trigger(cstream, cmd); + default: + break; + } if (fe->dai_link->trigger[stream] == SND_SOC_DPCM_TRIGGER_POST) fe_first = false; diff --git a/sound/soc/sof/ipc3-dtrace.c b/sound/soc/sof/ipc3-dtrace.c index 22053357731a89..e8c998c09dc30f 100644 --- a/sound/soc/sof/ipc3-dtrace.c +++ b/sound/soc/sof/ipc3-dtrace.c @@ -534,7 +534,7 @@ static int ipc3_dtrace_init(struct snd_sof_dev *sdev) /* create compressed page table for audio firmware */ ret = snd_sof_create_page_table(sdev->dev, &priv->dmatb, - priv->dmatp.area, priv->dmatb.bytes); + &priv->dmatp, priv->dmatb.bytes); if (ret < 0) goto table_err; diff --git a/sound/soc/sof/ipc4-compress.c b/sound/soc/sof/ipc4-compress.c index ffa987143af4aa..97a4ec7519055e 100644 --- a/sound/soc/sof/ipc4-compress.c +++ b/sound/soc/sof/ipc4-compress.c @@ -64,6 +64,12 @@ static u32 sof_ipc4_compr_calc_min_fragment_size(struct snd_sof_pcm_stream *sps) return max(SOF_IPC4_COMPR_MAX_PROCESSING_SIZE, host_buffer_estimate); } +/* The buffer must be described by the page table allocated for the stream */ +static u64 sof_ipc4_compr_calc_max_buffer_size(struct snd_sof_pcm_stream *sps) +{ + return (u64)SOF_PAGE_TABLE_MAX_PFNS(sps->page_table.bytes) * PAGE_SIZE; +} + static int sof_ipc4_compr_open(struct snd_soc_component *component, struct snd_compr_stream *cstream) { @@ -94,6 +100,7 @@ static int sof_ipc4_compr_open(struct snd_soc_component *component, spcm->stream[dir].posn.dai_posn = 0; spcm->prepared[dir] = false; spcm->pending_stop[dir] = false; + spcm->compr_started[dir] = false; return 0; } @@ -122,6 +129,7 @@ static int sof_ipc4_compr_stream_free(struct snd_sof_dev *sdev, spcm->prepared[dir] = false; spcm->pending_stop[dir] = false; + spcm->compr_started[dir] = false; spcm->stream[dir].cstream = NULL; /* reset the DMA */ @@ -193,6 +201,8 @@ static int sof_ipc4_compr_get_caps(struct snd_soc_component *component, struct sof_ipc4_fw_data *ipc4_data = sdev->private; struct sof_ipc4_codec_info_data *codec_info = ipc4_data->codec_info; int dir = cstream->direction; + u64 max_buffer_size; + u64 max_fragments; struct snd_sof_pcm *spcm; int i; @@ -233,6 +243,14 @@ static int sof_ipc4_compr_get_caps(struct snd_soc_component *component, caps->min_fragments = SOF_IPC4_COMPR_MIN_FRAGMENTS; caps->max_fragments = SOF_IPC4_COMPR_MAX_FRAGMENTS; + /* + * A buffer of max_fragments number of max_fragment_size fragments must + * be describable by the page table allocated for the stream + */ + max_buffer_size = sof_ipc4_compr_calc_max_buffer_size(&spcm->stream[dir]); + max_fragments = div_u64(max_buffer_size, caps->max_fragment_size); + if (caps->max_fragments > max_fragments) + caps->max_fragments = max_fragments; spcm_dbg(spcm, dir, "num_codecs: %u, fragment_size: %u-%u, fragments: %u-%u\n", @@ -248,6 +266,7 @@ static int sof_ipc4_compr_alloc_pages(struct device *dev, struct snd_compr_stream *cstream) { u32 min_fragment_size = sof_ipc4_compr_calc_min_fragment_size(sps); + u64 max_buffer_size = sof_ipc4_compr_calc_max_buffer_size(sps); struct snd_compr_runtime *crtd = cstream->runtime; u64 fragments = crtd->buffer_size; int ret; @@ -259,6 +278,12 @@ static int sof_ipc4_compr_alloc_pages(struct device *dev, return -EINVAL; } + if (crtd->buffer_size > max_buffer_size) { + dev_err(dev, "%s: Buffer size is too big: %llu (maximum is %llu)\n", + __func__, crtd->buffer_size, max_buffer_size); + return -EINVAL; + } + if (crtd->fragment_size < min_fragment_size) { dev_err(dev, "%s: Fragment size is too small: %u (minimum is %u)\n", __func__, crtd->fragment_size, min_fragment_size); @@ -376,6 +401,12 @@ static int sof_ipc4_compr_set_params(struct snd_soc_component *component, /* Use correct format based on the used codec */ switch (params->codec.id) { case SND_AUDIOCODEC_PCM: + /* The format is provided by user space, it must be validated */ + if (snd_pcm_format_width((snd_pcm_format_t)params->codec.format) <= 0) { + spcm_err(spcm, dir, "Invalid PCM format: %u\n", + params->codec.format); + return -EINVAL; + } snd_mask_set_format(fmt, (snd_pcm_format_t)params->codec.format); break; case SND_AUDIOCODEC_VORBIS: @@ -545,22 +576,35 @@ static int sof_ipc4_compr_trigger(struct snd_soc_component *component, return -EINVAL; } - spcm->pending_stop[dir] = false; - switch (cmd) { case SNDRV_PCM_TRIGGER_START: + /* + * The host DMA is left running on stop and drain to match the + * firmware, which only pauses the pipeline, and it is only + * stopped during hw_free. A stream restarted after stop or + * drain - which the compress core allows for capture - would + * continue from a position the two sides no longer agree on. + */ + if (spcm->compr_started[dir]) { + spcm_err(spcm, dir, "Stream restart is not supported\n"); + return -EINVAL; + } + fallthrough; case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: + spcm->pending_stop[dir] = false; trigger_platform = true; break; case SNDRV_PCM_TRIGGER_STOP: case SNDRV_PCM_TRIGGER_SUSPEND: case SNDRV_PCM_TRIGGER_PAUSE_PUSH: + spcm->pending_stop[dir] = false; break; case SND_COMPR_TRIGGER_DRAIN: - case SND_COMPR_TRIGGER_PARTIAL_DRAIN: spcm->pending_stop[dir] = true; break; + case SND_COMPR_TRIGGER_PARTIAL_DRAIN: case SND_COMPR_TRIGGER_NEXT_TRACK: + /* Gapless playback is not supported */ spcm_dbg(spcm, dir, "Unsupported trigger cmd: %d\n", cmd); return -EOPNOTSUPP; default: @@ -584,6 +628,9 @@ static int sof_ipc4_compr_trigger(struct snd_soc_component *component, ret); } + if (!ret && cmd == SNDRV_PCM_TRIGGER_START) + spcm->compr_started[dir] = true; + return ret; } diff --git a/sound/soc/sof/ipc4-loader.c b/sound/soc/sof/ipc4-loader.c index 8778cac0d52520..1da51f7490e579 100644 --- a/sound/soc/sof/ipc4-loader.c +++ b/sound/soc/sof/ipc4-loader.c @@ -422,45 +422,31 @@ static int sof_ipc4_query_sof_info(struct snd_sof_dev *sdev, { struct sof_ipc4_fw_data *ipc4_data = sdev->private; struct sof_ipc4_tuple *tuple; - size_t tuple_size; size_t offset = 0; - int ret = 0; + /* + * No size validation is needed as the sof_info data is coming from + * firmware as part of the firmware config, it is guarantied to be + * correct. + */ while (offset < sof_info_size) { - if (sof_info_size - offset < sizeof(*tuple)) { - dev_err(sdev->dev, "Invalid SOF info tuple header at offset %zu\n", offset); - ret = -EINVAL; - goto out; - } - tuple = (struct sof_ipc4_tuple *)((u8 *)sof_info_data + offset); - tuple_size = sizeof(*tuple) + tuple->size; - if (tuple_size < sizeof(*tuple) || tuple_size > sof_info_size - offset) { - dev_err(sdev->dev, - "Invalid SOF info tuple size %u at offset %zu\n", - tuple->size, offset); - ret = -EINVAL; - goto out; - } switch (tuple->type) { case SOF_IPC4_SOF_CODEC_INFO: ipc4_data->codec_info = devm_kmemdup(sdev->dev, tuple->value, tuple->size, GFP_KERNEL); - if (!ipc4_data->codec_info) { - ret = -ENOMEM; - goto out; - } + if (!ipc4_data->codec_info) + return -ENOMEM; break; default: break; } - offset += tuple_size; + offset += sizeof(*tuple) + tuple->size; } -out: - return ret; + return 0; } int sof_ipc4_query_fw_configuration(struct snd_sof_dev *sdev) diff --git a/sound/soc/sof/ipc4-pcm.c b/sound/soc/sof/ipc4-pcm.c index 5d8fc9b25bcc82..cbf948d8479148 100644 --- a/sound/soc/sof/ipc4-pcm.c +++ b/sound/soc/sof/ipc4-pcm.c @@ -419,9 +419,6 @@ static int sof_ipc4_trigger_pipelines(struct snd_soc_component *component, return 0; spipe = pipeline_list->pipelines[0]; - if (!spipe || !spipe->pipe_widget || !spipe->pipe_widget->private) - return 0; - pipe_widget = spipe->pipe_widget; pipeline = pipe_widget->private; @@ -485,16 +482,12 @@ static int sof_ipc4_trigger_pipelines(struct snd_soc_component *component, if (state == SOF_IPC4_PIPE_RUNNING || state == SOF_IPC4_PIPE_RESET) for (i = pipeline_list->count - 1; i >= 0; i--) { spipe = pipeline_list->pipelines[i]; - if (!spipe || !spipe->pipe_widget || !spipe->pipe_widget->private) - continue; sof_ipc4_add_pipeline_to_trigger_list(sdev, state, spipe, trigger_list, pipe_priority); } else for (i = 0; i < pipeline_list->count; i++) { spipe = pipeline_list->pipelines[i]; - if (!spipe || !spipe->pipe_widget || !spipe->pipe_widget->private) - continue; sof_ipc4_add_pipeline_to_trigger_list(sdev, state, spipe, trigger_list, pipe_priority); } @@ -535,8 +528,6 @@ static int sof_ipc4_trigger_pipelines(struct snd_soc_component *component, /* update PAUSED state for all pipelines just triggered */ for (i = 0; i < pipeline_list->count ; i++) { spipe = pipeline_list->pipelines[i]; - if (!spipe || !spipe->pipe_widget || !spipe->pipe_widget->private) - continue; sof_ipc4_update_pipeline_state(sdev, SOF_IPC4_PIPE_PAUSED, cmd, spipe, trigger_list); } @@ -580,8 +571,6 @@ static int sof_ipc4_trigger_pipelines(struct snd_soc_component *component, /* update RUNNING/RESET state for all pipelines that were just triggered */ for (i = 0; i < pipeline_list->count; i++) { spipe = pipeline_list->pipelines[i]; - if (!spipe || !spipe->pipe_widget || !spipe->pipe_widget->private) - continue; sof_ipc4_update_pipeline_state(sdev, state, cmd, spipe, trigger_list); } diff --git a/sound/soc/sof/pcm.c b/sound/soc/sof/pcm.c index 129c698d87beba..0161c0ae8bbdb4 100644 --- a/sound/soc/sof/pcm.c +++ b/sound/soc/sof/pcm.c @@ -194,7 +194,7 @@ static int sof_pcm_hw_params(struct snd_soc_component *component, struct snd_dma_buffer *dmab = snd_pcm_get_dma_buf(substream); ret = snd_sof_create_page_table(component->dev, dmab, - spcm->stream[substream->stream].page_table.area, + &spcm->stream[substream->stream].page_table, runtime->dma_bytes); if (ret < 0) return ret; diff --git a/sound/soc/sof/sof-audio.c b/sound/soc/sof/sof-audio.c index 0b2f41f4a59bfa..8cbeca471542a6 100644 --- a/sound/soc/sof/sof-audio.c +++ b/sound/soc/sof/sof-audio.c @@ -629,6 +629,10 @@ static int sof_set_up_widgets_in_path(struct snd_sof_dev *sdev, struct snd_soc_d if (!pipeline_list->pipelines) goto sink_setup; + /* a widget which is not part of a pipeline has nothing to trigger */ + if (!swidget->spipe) + goto sink_setup; + /* * Add the widget's pipe_widget to the list of pipelines to be triggered if not * already in the list. This will result in the pipelines getting added in the @@ -641,8 +645,8 @@ static int sof_set_up_widgets_in_path(struct snd_sof_dev *sdev, struct snd_soc_d } if (i == pipeline_list->count) { - pipeline_list->count++; pipeline_list->pipelines[i] = swidget->spipe; + pipeline_list->count++; } } @@ -1128,6 +1132,6 @@ int snd_sof_compr_create_page_table(struct snd_soc_component *component, return -EINVAL; return snd_sof_create_page_table(component->dev, dmab, - spcm->stream[dir].page_table.area, size); + &spcm->stream[dir].page_table, size); } #endif diff --git a/sound/soc/sof/sof-audio.h b/sound/soc/sof/sof-audio.h index 0d3650f7a67002..10b41ab18b7990 100644 --- a/sound/soc/sof/sof-audio.h +++ b/sound/soc/sof/sof-audio.h @@ -363,6 +363,7 @@ struct snd_sof_pcm { bool prepared[2]; /* PCM_PARAMS set successfully */ bool setup_done[2]; /* the setup of the SOF PCM device is done */ bool pending_stop[2]; /* only used if (!pcm_ops->platform_stop_during_hw_free) */ + bool compr_started[2]; /* compress stream has been started */ /* Must be last - ends in a flex-array member. */ struct snd_soc_tplg_pcm pcm; diff --git a/sound/soc/sof/sof-utils.c b/sound/soc/sof/sof-utils.c index f70089317b8c52..59dc1019695859 100644 --- a/sound/soc/sof/sof-utils.c +++ b/sound/soc/sof/sof-utils.c @@ -24,12 +24,20 @@ int snd_sof_create_page_table(struct device *dev, struct snd_dma_buffer *dmab, - unsigned char *page_table, size_t size) + struct snd_dma_buffer *page_table, size_t size) { int i, pages; pages = snd_sgbuf_aligned_pages(size); + if (pages < 1 || page_table->bytes < sizeof(u32) || + pages > SOF_PAGE_TABLE_MAX_PFNS(page_table->bytes)) { + dev_err(dev, + "Can not store %d pages in a %zu bytes page table\n", + pages, page_table->bytes); + return -EINVAL; + } + dev_dbg(dev, "generating page table for %p size 0x%zx pages %d\n", dmab->area, size, pages); @@ -45,7 +53,7 @@ int snd_sof_create_page_table(struct device *dev, u32 pfn = snd_sgbuf_get_addr(dmab, i * PAGE_SIZE) >> PAGE_SHIFT; u8 *pg_table; - pg_table = (u8 *)(page_table + idx); + pg_table = (u8 *)(page_table->area + idx); /* * pagetable compression: diff --git a/sound/soc/sof/sof-utils.h b/sound/soc/sof/sof-utils.h index 9ac6de9a6d6a3a..58e5822a63f047 100644 --- a/sound/soc/sof/sof-utils.h +++ b/sound/soc/sof/sof-utils.h @@ -12,8 +12,17 @@ struct snd_dma_buffer; struct device; +/* + * Number of PFNs which can be stored in a page table of @bytes size. + * The PFNs are compressed to 2.5 bytes each but they are written with 32 bit + * accesses, therefore the last PFN can reach up to 3 bytes past the space it + * needs for itself. + */ +#define SOF_PAGE_TABLE_MAX_PFNS(bytes) \ + (((((bytes) - sizeof(u32)) << 1) + 1) / 5 + 1) + int snd_sof_create_page_table(struct device *dev, struct snd_dma_buffer *dmab, - unsigned char *page_table, size_t size); + struct snd_dma_buffer *page_table, size_t size); #endif