- Sound-open-firmware - mailman.alsa-project.org

[PATCH AUTOSEL 6.6 04/43] ASoC: SOF: pcm: Restrict DSP D0i3 during S0ix to IPC3
by Sasha Levin 08 May '24

08 May '24

From: Ranjani Sridharan <ranjani.sridharan(a)linux.intel.com> [ Upstream commit 90a2353080eedec855d63f6aadfda14104ee9b06 ] Introduce a new field in struct sof_ipc_pcm_ops that can be used to restrict DSP D0i3 during S0ix suspend to IPC3. With IPC4, all streams must be stopped before S0ix suspend. Reviewed-by: Uday M Bhat <uday.m.bhat(a)intel.com> Reviewed-by: Bard Liao <yung-chuan.liao(a)linux.intel.com> Signed-off-by: Ranjani Sridharan <ranjani.sridharan(a)linux.intel.com> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart(a)linux.intel.com> Link: https://msgid.link/r/20240408194147.28919-3-pierre-louis.bossart@linux.inte… Signed-off-by: Mark Brown <broonie(a)kernel.org> Signed-off-by: Sasha Levin <sashal(a)kernel.org> --- sound/soc/sof/ipc3-pcm.c | 1 + sound/soc/sof/pcm.c | 13 ++++++------- sound/soc/sof/sof-audio.h | 2 ++ 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/sound/soc/sof/ipc3-pcm.c b/sound/soc/sof/ipc3-pcm.c index cb58ee8c158a5..720bd9bd2667a 100644 --- a/sound/soc/sof/ipc3-pcm.c +++ b/sound/soc/sof/ipc3-pcm.c @@ -398,4 +398,5 @@ const struct sof_ipc_pcm_ops ipc3_pcm_ops = { .trigger = sof_ipc3_pcm_trigger, .dai_link_fixup = sof_ipc3_pcm_dai_link_fixup, .reset_hw_params_during_stop = true, + .d0i3_supported_in_s0ix = true, }; diff --git a/sound/soc/sof/pcm.c b/sound/soc/sof/pcm.c index d778717cab10b..8e602e42afee2 100644 --- a/sound/soc/sof/pcm.c +++ b/sound/soc/sof/pcm.c @@ -325,14 +325,13 @@ static int sof_pcm_trigger(struct snd_soc_component *component, ipc_first = true; break; case SNDRV_PCM_TRIGGER_SUSPEND: - if (sdev->system_suspend_target == SOF_SUSPEND_S0IX && + /* + * If DSP D0I3 is allowed during S0iX, set the suspend_ignored flag for + * D0I3-compatible streams to keep the firmware pipeline running + */ + if (pcm_ops && pcm_ops->d0i3_supported_in_s0ix && + sdev->system_suspend_target == SOF_SUSPEND_S0IX && spcm->stream[substream->stream].d0i3_compatible) { - /* - * trap the event, not sending trigger stop to - * prevent the FW pipelines from being stopped, - * and mark the flag to ignore the upcoming DAPM - * PM events. - */ spcm->stream[substream->stream].suspend_ignored = true; return 0; } diff --git a/sound/soc/sof/sof-audio.h b/sound/soc/sof/sof-audio.h index a6d6bcd00ceec..3606595a7500c 100644 --- a/sound/soc/sof/sof-audio.h +++ b/sound/soc/sof/sof-audio.h @@ -113,6 +113,7 @@ struct snd_sof_dai_config_data { * triggers. The FW keeps the host DMA running in this case and * therefore the host must do the same and should stop the DMA during * hw_free. + * @d0i3_supported_in_s0ix: Allow DSP D0I3 during S0iX */ struct sof_ipc_pcm_ops { int (*hw_params)(struct snd_soc_component *component, struct snd_pcm_substream *substream, @@ -129,6 +130,7 @@ struct sof_ipc_pcm_ops { bool reset_hw_params_during_stop; bool ipc_first_on_start; bool platform_stop_during_hw_free; + bool d0i3_supported_in_s0ix; }; /** -- 2.43.0

1 0

[PATCH AUTOSEL 6.8 07/52] ASoC: SOF: pcm: Restrict DSP D0i3 during S0ix to IPC3
by Sasha Levin 08 May '24

08 May '24

From: Ranjani Sridharan <ranjani.sridharan(a)linux.intel.com> [ Upstream commit 90a2353080eedec855d63f6aadfda14104ee9b06 ] Introduce a new field in struct sof_ipc_pcm_ops that can be used to restrict DSP D0i3 during S0ix suspend to IPC3. With IPC4, all streams must be stopped before S0ix suspend. Reviewed-by: Uday M Bhat <uday.m.bhat(a)intel.com> Reviewed-by: Bard Liao <yung-chuan.liao(a)linux.intel.com> Signed-off-by: Ranjani Sridharan <ranjani.sridharan(a)linux.intel.com> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart(a)linux.intel.com> Link: https://msgid.link/r/20240408194147.28919-3-pierre-louis.bossart@linux.inte… Signed-off-by: Mark Brown <broonie(a)kernel.org> Signed-off-by: Sasha Levin <sashal(a)kernel.org> --- sound/soc/sof/ipc3-pcm.c | 1 + sound/soc/sof/pcm.c | 13 ++++++------- sound/soc/sof/sof-audio.h | 2 ++ 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/sound/soc/sof/ipc3-pcm.c b/sound/soc/sof/ipc3-pcm.c index 330f04bcd75d2..a7cf52fd76455 100644 --- a/sound/soc/sof/ipc3-pcm.c +++ b/sound/soc/sof/ipc3-pcm.c @@ -409,4 +409,5 @@ const struct sof_ipc_pcm_ops ipc3_pcm_ops = { .trigger = sof_ipc3_pcm_trigger, .dai_link_fixup = sof_ipc3_pcm_dai_link_fixup, .reset_hw_params_during_stop = true, + .d0i3_supported_in_s0ix = true, }; diff --git a/sound/soc/sof/pcm.c b/sound/soc/sof/pcm.c index f03cee94bce62..8804e00e7251b 100644 --- a/sound/soc/sof/pcm.c +++ b/sound/soc/sof/pcm.c @@ -325,14 +325,13 @@ static int sof_pcm_trigger(struct snd_soc_component *component, ipc_first = true; break; case SNDRV_PCM_TRIGGER_SUSPEND: - if (sdev->system_suspend_target == SOF_SUSPEND_S0IX && + /* + * If DSP D0I3 is allowed during S0iX, set the suspend_ignored flag for + * D0I3-compatible streams to keep the firmware pipeline running + */ + if (pcm_ops && pcm_ops->d0i3_supported_in_s0ix && + sdev->system_suspend_target == SOF_SUSPEND_S0IX && spcm->stream[substream->stream].d0i3_compatible) { - /* - * trap the event, not sending trigger stop to - * prevent the FW pipelines from being stopped, - * and mark the flag to ignore the upcoming DAPM - * PM events. - */ spcm->stream[substream->stream].suspend_ignored = true; return 0; } diff --git a/sound/soc/sof/sof-audio.h b/sound/soc/sof/sof-audio.h index 85b26e3fefa8f..05e2a899d746c 100644 --- a/sound/soc/sof/sof-audio.h +++ b/sound/soc/sof/sof-audio.h @@ -116,6 +116,7 @@ struct snd_sof_dai_config_data { * triggers. The FW keeps the host DMA running in this case and * therefore the host must do the same and should stop the DMA during * hw_free. + * @d0i3_supported_in_s0ix: Allow DSP D0I3 during S0iX */ struct sof_ipc_pcm_ops { int (*hw_params)(struct snd_soc_component *component, struct snd_pcm_substream *substream, @@ -135,6 +136,7 @@ struct sof_ipc_pcm_ops { bool reset_hw_params_during_stop; bool ipc_first_on_start; bool platform_stop_during_hw_free; + bool d0i3_supported_in_s0ix; }; /** -- 2.43.0

1 0

[PATCH AUTOSEL 6.8 06/52] ASoC: SOF: ipc4-pcm: Introduce generic sof_ipc4_pcm_stream_priv
by Sasha Levin 08 May '24

08 May '24

From: Peter Ujfalusi <peter.ujfalusi(a)linux.intel.com> [ Upstream commit 551af3280c16166244425bbb1d73028f3a907e1f ] Using the sof_ipc4_timestamp_info struct directly as sps->private data is too restrictive, add a new generic sof_ipc4_pcm_stream_priv struct containing the time_info to allow new information to be stored in a generic way. Signed-off-by: Peter Ujfalusi <peter.ujfalusi(a)linux.intel.com> Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart(a)linux.intel.com> Reviewed-by: Ranjani Sridharan <ranjani.sridharan(a)linux.intel.com> Link: https://msgid.link/r/20240409110036.9411-4-peter.ujfalusi@linux.intel.com Signed-off-by: Mark Brown <broonie(a)kernel.org> Signed-off-by: Sasha Levin <sashal(a)kernel.org> --- sound/soc/sof/ipc4-pcm.c | 43 ++++++++++++++++++++++++++++++++-------- 1 file changed, 35 insertions(+), 8 deletions(-) diff --git a/sound/soc/sof/ipc4-pcm.c b/sound/soc/sof/ipc4-pcm.c index 3f8b9443496ce..d07c1b06207a5 100644 --- a/sound/soc/sof/ipc4-pcm.c +++ b/sound/soc/sof/ipc4-pcm.c @@ -37,6 +37,22 @@ struct sof_ipc4_timestamp_info { snd_pcm_sframes_t delay; }; +/** + * struct sof_ipc4_pcm_stream_priv - IPC4 specific private data + * @time_info: pointer to time info struct if it is supported, otherwise NULL + */ +struct sof_ipc4_pcm_stream_priv { + struct sof_ipc4_timestamp_info *time_info; +}; + +static inline struct sof_ipc4_timestamp_info * +sof_ipc4_sps_to_time_info(struct snd_sof_pcm_stream *sps) +{ + struct sof_ipc4_pcm_stream_priv *stream_priv = sps->private; + + return stream_priv->time_info; +} + static int sof_ipc4_set_multi_pipeline_state(struct snd_sof_dev *sdev, u32 state, struct ipc4_pipeline_set_state_data *trigger_list) { @@ -435,7 +451,7 @@ static int sof_ipc4_trigger_pipelines(struct snd_soc_component *component, * Invalidate the stream_start_offset to make sure that it is * going to be updated if the stream resumes */ - time_info = spcm->stream[substream->stream].private; + time_info = sof_ipc4_sps_to_time_info(&spcm->stream[substream->stream]); if (time_info) time_info->stream_start_offset = SOF_IPC4_INVALID_STREAM_POSITION; @@ -689,12 +705,16 @@ static int sof_ipc4_pcm_dai_link_fixup(struct snd_soc_pcm_runtime *rtd, static void sof_ipc4_pcm_free(struct snd_sof_dev *sdev, struct snd_sof_pcm *spcm) { struct snd_sof_pcm_stream_pipeline_list *pipeline_list; + struct sof_ipc4_pcm_stream_priv *stream_priv; int stream; for_each_pcm_streams(stream) { pipeline_list = &spcm->stream[stream].pipeline_list; kfree(pipeline_list->pipelines); pipeline_list->pipelines = NULL; + + stream_priv = spcm->stream[stream].private; + kfree(stream_priv->time_info); kfree(spcm->stream[stream].private); spcm->stream[stream].private = NULL; } @@ -704,6 +724,7 @@ static int sof_ipc4_pcm_setup(struct snd_sof_dev *sdev, struct snd_sof_pcm *spcm { struct snd_sof_pcm_stream_pipeline_list *pipeline_list; struct sof_ipc4_fw_data *ipc4_data = sdev->private; + struct sof_ipc4_pcm_stream_priv *stream_priv; struct sof_ipc4_timestamp_info *time_info; bool support_info = true; u32 abi_version; @@ -732,6 +753,14 @@ static int sof_ipc4_pcm_setup(struct snd_sof_dev *sdev, struct snd_sof_pcm *spcm return -ENOMEM; } + stream_priv = kzalloc(sizeof(*stream_priv), GFP_KERNEL); + if (!stream_priv) { + sof_ipc4_pcm_free(sdev, spcm); + return -ENOMEM; + } + + spcm->stream[stream].private = stream_priv; + if (!support_info) continue; @@ -741,7 +770,7 @@ static int sof_ipc4_pcm_setup(struct snd_sof_dev *sdev, struct snd_sof_pcm *spcm return -ENOMEM; } - spcm->stream[stream].private = time_info; + stream_priv->time_info = time_info; } return 0; @@ -778,7 +807,7 @@ static void sof_ipc4_build_time_info(struct snd_sof_dev *sdev, struct snd_sof_pc return; } - time_info = sps->private; + time_info = sof_ipc4_sps_to_time_info(sps); time_info->host_copier = host_copier; time_info->dai_copier = dai_copier; time_info->llp_offset = offsetof(struct sof_ipc4_fw_registers, @@ -832,7 +861,7 @@ static int sof_ipc4_pcm_hw_params(struct snd_soc_component *component, if (!spcm) return -EINVAL; - time_info = spcm->stream[substream->stream].private; + time_info = sof_ipc4_sps_to_time_info(&spcm->stream[substream->stream]); /* delay calculation is not supported by current fw_reg ABI */ if (!time_info) return 0; @@ -911,7 +940,7 @@ static int sof_ipc4_pcm_pointer(struct snd_soc_component *component, return -EOPNOTSUPP; sps = &spcm->stream[substream->stream]; - time_info = sps->private; + time_info = sof_ipc4_sps_to_time_info(sps); if (!time_info) return -EOPNOTSUPP; @@ -1013,15 +1042,13 @@ static snd_pcm_sframes_t sof_ipc4_pcm_delay(struct snd_soc_component *component, { struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream); struct sof_ipc4_timestamp_info *time_info; - struct snd_sof_pcm_stream *sps; struct snd_sof_pcm *spcm; spcm = snd_sof_find_spcm_dai(component, rtd); if (!spcm) return 0; - sps = &spcm->stream[substream->stream]; - time_info = sps->private; + time_info = sof_ipc4_sps_to_time_info(&spcm->stream[substream->stream]); /* * Report the stored delay value calculated in the pointer callback. * In the unlikely event that the calculation was skipped/aborted, the -- 2.43.0

1 0

[PATCH AUTOSEL 6.8 05/52] ASoC: SOF: ipc4-pcm: Use consistent name for sof_ipc4_timestamp_info pointer
by Sasha Levin 08 May '24

08 May '24

From: Peter Ujfalusi <peter.ujfalusi(a)linux.intel.com> [ Upstream commit 36e980050b0733829e4e0f97b97f7907ba9f00bb ] The pointer to sof_ipc4_timestamp_info named most of the time as 'time_info' only to be named as 'stream_info' or 'info' in two function. Use the consistent name of 'time_info' throughout the file. Signed-off-by: Peter Ujfalusi <peter.ujfalusi(a)linux.intel.com> Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart(a)linux.intel.com> Reviewed-by: Ranjani Sridharan <ranjani.sridharan(a)linux.intel.com> Link: https://msgid.link/r/20240409110036.9411-3-peter.ujfalusi@linux.intel.com Signed-off-by: Mark Brown <broonie(a)kernel.org> Signed-off-by: Sasha Levin <sashal(a)kernel.org> --- sound/soc/sof/ipc4-pcm.c | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/sound/soc/sof/ipc4-pcm.c b/sound/soc/sof/ipc4-pcm.c index 31133c5dcbf50..3f8b9443496ce 100644 --- a/sound/soc/sof/ipc4-pcm.c +++ b/sound/soc/sof/ipc4-pcm.c @@ -704,7 +704,7 @@ static int sof_ipc4_pcm_setup(struct snd_sof_dev *sdev, struct snd_sof_pcm *spcm { struct snd_sof_pcm_stream_pipeline_list *pipeline_list; struct sof_ipc4_fw_data *ipc4_data = sdev->private; - struct sof_ipc4_timestamp_info *stream_info; + struct sof_ipc4_timestamp_info *time_info; bool support_info = true; u32 abi_version; u32 abi_offset; @@ -735,13 +735,13 @@ static int sof_ipc4_pcm_setup(struct snd_sof_dev *sdev, struct snd_sof_pcm *spcm if (!support_info) continue; - stream_info = kzalloc(sizeof(*stream_info), GFP_KERNEL); - if (!stream_info) { + time_info = kzalloc(sizeof(*time_info), GFP_KERNEL); + if (!time_info) { sof_ipc4_pcm_free(sdev, spcm); return -ENOMEM; } - spcm->stream[stream].private = stream_info; + spcm->stream[stream].private = time_info; } return 0; @@ -752,7 +752,7 @@ static void sof_ipc4_build_time_info(struct snd_sof_dev *sdev, struct snd_sof_pc struct sof_ipc4_copier *host_copier = NULL; struct sof_ipc4_copier *dai_copier = NULL; struct sof_ipc4_llp_reading_slot llp_slot; - struct sof_ipc4_timestamp_info *info; + struct sof_ipc4_timestamp_info *time_info; struct snd_soc_dapm_widget *widget; struct snd_sof_dai *dai; int i; @@ -778,44 +778,44 @@ static void sof_ipc4_build_time_info(struct snd_sof_dev *sdev, struct snd_sof_pc return; } - info = sps->private; - info->host_copier = host_copier; - info->dai_copier = dai_copier; - info->llp_offset = offsetof(struct sof_ipc4_fw_registers, llp_gpdma_reading_slots) + - sdev->fw_info_box.offset; + time_info = sps->private; + time_info->host_copier = host_copier; + time_info->dai_copier = dai_copier; + time_info->llp_offset = offsetof(struct sof_ipc4_fw_registers, + llp_gpdma_reading_slots) + sdev->fw_info_box.offset; /* find llp slot used by current dai */ for (i = 0; i < SOF_IPC4_MAX_LLP_GPDMA_READING_SLOTS; i++) { - sof_mailbox_read(sdev, info->llp_offset, &llp_slot, sizeof(llp_slot)); + sof_mailbox_read(sdev, time_info->llp_offset, &llp_slot, sizeof(llp_slot)); if (llp_slot.node_id == dai_copier->data.gtw_cfg.node_id) break; - info->llp_offset += sizeof(llp_slot); + time_info->llp_offset += sizeof(llp_slot); } if (i < SOF_IPC4_MAX_LLP_GPDMA_READING_SLOTS) return; /* if no llp gpdma slot is used, check aggregated sdw slot */ - info->llp_offset = offsetof(struct sof_ipc4_fw_registers, llp_sndw_reading_slots) + - sdev->fw_info_box.offset; + time_info->llp_offset = offsetof(struct sof_ipc4_fw_registers, + llp_sndw_reading_slots) + sdev->fw_info_box.offset; for (i = 0; i < SOF_IPC4_MAX_LLP_SNDW_READING_SLOTS; i++) { - sof_mailbox_read(sdev, info->llp_offset, &llp_slot, sizeof(llp_slot)); + sof_mailbox_read(sdev, time_info->llp_offset, &llp_slot, sizeof(llp_slot)); if (llp_slot.node_id == dai_copier->data.gtw_cfg.node_id) break; - info->llp_offset += sizeof(llp_slot); + time_info->llp_offset += sizeof(llp_slot); } if (i < SOF_IPC4_MAX_LLP_SNDW_READING_SLOTS) return; /* check EVAD slot */ - info->llp_offset = offsetof(struct sof_ipc4_fw_registers, llp_evad_reading_slot) + - sdev->fw_info_box.offset; - sof_mailbox_read(sdev, info->llp_offset, &llp_slot, sizeof(llp_slot)); + time_info->llp_offset = offsetof(struct sof_ipc4_fw_registers, + llp_evad_reading_slot) + sdev->fw_info_box.offset; + sof_mailbox_read(sdev, time_info->llp_offset, &llp_slot, sizeof(llp_slot)); if (llp_slot.node_id != dai_copier->data.gtw_cfg.node_id) - info->llp_offset = 0; + time_info->llp_offset = 0; } static int sof_ipc4_pcm_hw_params(struct snd_soc_component *component, -- 2.43.0

1 0

[PATCH AUTOSEL 6.8 04/52] ASoC: SOF: ipc4-pcm: Use consistent name for snd_sof_pcm_stream pointer
by Sasha Levin 08 May '24

08 May '24

From: Peter Ujfalusi <peter.ujfalusi(a)linux.intel.com> [ Upstream commit 965e49cdf8c19f21b8308adeded3a8139cff5c84 ] Throughout the file the pointer for snd_sof_pcm_stream is named either 'stream' or (wrongly) 'spcm' which confuses the reader. Use 'sps' for the pointer name as it is the most common name used in SOF codebase. Signed-off-by: Peter Ujfalusi <peter.ujfalusi(a)linux.intel.com> Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart(a)linux.intel.com> Reviewed-by: Ranjani Sridharan <ranjani.sridharan(a)linux.intel.com> Link: https://msgid.link/r/20240409110036.9411-2-peter.ujfalusi@linux.intel.com Signed-off-by: Mark Brown <broonie(a)kernel.org> Signed-off-by: Sasha Levin <sashal(a)kernel.org> --- sound/soc/sof/ipc4-pcm.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/sound/soc/sof/ipc4-pcm.c b/sound/soc/sof/ipc4-pcm.c index a99fced908251..31133c5dcbf50 100644 --- a/sound/soc/sof/ipc4-pcm.c +++ b/sound/soc/sof/ipc4-pcm.c @@ -747,7 +747,7 @@ static int sof_ipc4_pcm_setup(struct snd_sof_dev *sdev, struct snd_sof_pcm *spcm return 0; } -static void sof_ipc4_build_time_info(struct snd_sof_dev *sdev, struct snd_sof_pcm_stream *spcm) +static void sof_ipc4_build_time_info(struct snd_sof_dev *sdev, struct snd_sof_pcm_stream *sps) { struct sof_ipc4_copier *host_copier = NULL; struct sof_ipc4_copier *dai_copier = NULL; @@ -758,7 +758,7 @@ static void sof_ipc4_build_time_info(struct snd_sof_dev *sdev, struct snd_sof_pc int i; /* find host & dai to locate info in memory window */ - for_each_dapm_widgets(spcm->list, i, widget) { + for_each_dapm_widgets(sps->list, i, widget) { struct snd_sof_widget *swidget = widget->dobj.private; if (!swidget) @@ -778,7 +778,7 @@ static void sof_ipc4_build_time_info(struct snd_sof_dev *sdev, struct snd_sof_pc return; } - info = spcm->private; + info = sps->private; info->host_copier = host_copier; info->dai_copier = dai_copier; info->llp_offset = offsetof(struct sof_ipc4_fw_registers, llp_gpdma_reading_slots) + @@ -847,7 +847,7 @@ static int sof_ipc4_pcm_hw_params(struct snd_soc_component *component, static int sof_ipc4_get_stream_start_offset(struct snd_sof_dev *sdev, struct snd_pcm_substream *substream, - struct snd_sof_pcm_stream *stream, + struct snd_sof_pcm_stream *sps, struct sof_ipc4_timestamp_info *time_info) { struct sof_ipc4_copier *host_copier = time_info->host_copier; @@ -901,7 +901,7 @@ static int sof_ipc4_pcm_pointer(struct snd_soc_component *component, struct sof_ipc4_timestamp_info *time_info; struct sof_ipc4_llp_reading_slot llp; snd_pcm_uframes_t head_cnt, tail_cnt; - struct snd_sof_pcm_stream *stream; + struct snd_sof_pcm_stream *sps; u64 dai_cnt, host_cnt, host_ptr; struct snd_sof_pcm *spcm; int ret; @@ -910,8 +910,8 @@ static int sof_ipc4_pcm_pointer(struct snd_soc_component *component, if (!spcm) return -EOPNOTSUPP; - stream = &spcm->stream[substream->stream]; - time_info = stream->private; + sps = &spcm->stream[substream->stream]; + time_info = sps->private; if (!time_info) return -EOPNOTSUPP; @@ -921,7 +921,7 @@ static int sof_ipc4_pcm_pointer(struct snd_soc_component *component, * the statistics is complete. And it will not change after the first initiailization. */ if (time_info->stream_start_offset == SOF_IPC4_INVALID_STREAM_POSITION) { - ret = sof_ipc4_get_stream_start_offset(sdev, substream, stream, time_info); + ret = sof_ipc4_get_stream_start_offset(sdev, substream, sps, time_info); if (ret < 0) return -EOPNOTSUPP; } @@ -1013,15 +1013,15 @@ static snd_pcm_sframes_t sof_ipc4_pcm_delay(struct snd_soc_component *component, { struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream); struct sof_ipc4_timestamp_info *time_info; - struct snd_sof_pcm_stream *stream; + struct snd_sof_pcm_stream *sps; struct snd_sof_pcm *spcm; spcm = snd_sof_find_spcm_dai(component, rtd); if (!spcm) return 0; - stream = &spcm->stream[substream->stream]; - time_info = stream->private; + sps = &spcm->stream[substream->stream]; + time_info = sps->private; /* * Report the stored delay value calculated in the pointer callback. * In the unlikely event that the calculation was skipped/aborted, the -- 2.43.0

1 0

Re: [PATCH v2 00/14] ASoC: Constify local snd_sof_dsp_ops
by Mark Brown 01 May '24

01 May '24

On Fri, 26 Apr 2024 11:03:47 +0200, Krzysztof Kozlowski wrote: > Changes in v2 > ============= > - Only rebase on for-next of broonie/sound.git > - Add tags. > - Link to v1: https://lore.kernel.org/r/20240414-n-const-ops-var-v1-0-8f53ee5d981c@kernel… > > Description > =========== > The core code does not modify the 'struct snd_sof_dsp_ops' passed via > pointer in various places, so this can be made pointer to const in few > places. This in turn allows few drivers to have the local (usually > static) 'struct snd_sof_dsp_ops' as const which increased code safety, > as it is now part of rodata. > > [...] Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next Thanks! [01/14] ASoC: SOF: debug: Constify local snd_sof_dsp_ops commit: ffca099bbff1978bc9c97b076f0d35b4fe6ddf27 [02/14] ASoC: SOF: ipc3: Constify local snd_sof_dsp_ops commit: ee5acc1e035ec5ed5d9f0f63fda9d627220090c2 [03/14] ASoC: SOF: pcm: Constify local snd_sof_dsp_ops commit: a0db037df9630edad76153c7937c6f5ca04ed44f [04/14] ASoC: SOF: Constify stored pointer to snd_sof_dsp_ops commit: 8bbc692d1abce5bc949dea9acba85fc686601c04 [05/14] ASoC: SOF: intel: pci-tng: Constify snd_sof_dsp_ops commit: 8f2b0d55abc44676b076128903a5dc730aab23c6 [06/14] ASoC: SOF: intel: hda: Constify snd_sof_dsp_ops commit: 6032eefc2c478243a511352dda04495c9a9fec6a [07/14] ASoC: SOF: amd: acp: Constify snd_sof_dsp_ops commit: 04f2f516be09d5493d764e0020a771c46b5470d8 [08/14] ASoC: SOF: imx8: Constify snd_sof_dsp_ops commit: ab85c44973298b69eb32795de11ce4d426705532 [09/14] ASoC: SOF: imx8m: Constify snd_sof_dsp_ops commit: 66d49ab5fb51bb8d1b4c2c9c8fa0fbe8e4c8ca1c [10/14] ASoC: SOF: imx8ulp: Constify snd_sof_dsp_ops commit: 232e0da9fa778233358586617bd22173bcac6bcc [11/14] ASoC: SOF: intel: bdw: Constify snd_sof_dsp_ops commit: 936cc56044a87ae7fbd0e4098a7daefa0f2f4e8e [12/14] ASoC: SOF: intel: byt: Constify snd_sof_dsp_ops commit: 48d5f1800d0cbda0212c5a58177918c419a24f8a [13/14] ASoC: SOF: mediatek: mt8186: Constify snd_sof_dsp_ops commit: fe80673f59da01776a1402e4b508a66fca43a24d [14/14] ASoC: SOF: mediatek: mt8195: Constify snd_sof_dsp_ops commit: 8b6d678fede700db6466d73f11fcbad496fa515e All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark

1 0

Re: [PATCH 00/14] ASoC: Constify local snd_sof_dsp_ops
by Mark Brown 01 May '24

01 May '24

On Sun, 14 Apr 2024 20:47:25 +0200, Krzysztof Kozlowski wrote: > The core code does not modify the 'struct snd_sof_dsp_ops' passed via > pointer in various places, so this can be made pointer to const. > > All further patches depend on the first four patches. > > Best regards, > Krzysztof > > [...] Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next Thanks! [01/14] ASoC: SOF: debug: Constify local snd_sof_dsp_ops commit: ffca099bbff1978bc9c97b076f0d35b4fe6ddf27 [02/14] ASoC: SOF: ipc3: Constify local snd_sof_dsp_ops commit: ee5acc1e035ec5ed5d9f0f63fda9d627220090c2 [03/14] ASoC: SOF: pcm: Constify local snd_sof_dsp_ops commit: a0db037df9630edad76153c7937c6f5ca04ed44f [04/14] ASoC: SOF: Constify stored pointer to snd_sof_dsp_ops commit: 8bbc692d1abce5bc949dea9acba85fc686601c04 [05/14] ASoC: SOF: intel: pci-tng: Constify snd_sof_dsp_ops commit: 8f2b0d55abc44676b076128903a5dc730aab23c6 [06/14] ASoC: SOF: intel: hda: Constify snd_sof_dsp_ops commit: 6032eefc2c478243a511352dda04495c9a9fec6a [07/14] ASoC: SOF: amd: acp: Constify snd_sof_dsp_ops commit: 04f2f516be09d5493d764e0020a771c46b5470d8 [08/14] ASoC: SOF: imx8: Constify snd_sof_dsp_ops commit: ab85c44973298b69eb32795de11ce4d426705532 [09/14] ASoC: SOF: imx8m: Constify snd_sof_dsp_ops commit: 66d49ab5fb51bb8d1b4c2c9c8fa0fbe8e4c8ca1c [10/14] ASoC: SOF: imx8ulp: Constify snd_sof_dsp_ops commit: 232e0da9fa778233358586617bd22173bcac6bcc [11/14] ASoC: SOF: intel: bdw: Constify snd_sof_dsp_ops commit: 936cc56044a87ae7fbd0e4098a7daefa0f2f4e8e [12/14] ASoC: SOF: intel: byt: Constify snd_sof_dsp_ops commit: 48d5f1800d0cbda0212c5a58177918c419a24f8a [13/14] ASoC: SOF: mediatek: mt8186: Constify snd_sof_dsp_ops commit: fe80673f59da01776a1402e4b508a66fca43a24d [14/14] ASoC: SOF: mediatek: mt8195: Constify snd_sof_dsp_ops commit: 8b6d678fede700db6466d73f11fcbad496fa515e All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark

1 0

Re: [PATCH 00/14] ASoC: Constify local snd_sof_dsp_ops
by Pierre-Louis Bossart 25 Apr '24

25 Apr '24

On 4/23/24 11:06, Krzysztof Kozlowski wrote: > On 23/04/2024 17:57, Pierre-Louis Bossart wrote: >> >>> Are we talking about basic C now? Of course it does not mean that and I >>> already explained what is the goal of this - the static or global memory >>> in the driver can be moved to rodata. >> >> the dsp_ops used by the Intel drivers cannot be moved to rodata in all >> cases. the baseline is constant, all extensions for TGL+ are dynamically >> allocated and modified. > > Yes and these drivers will not benefit. Did I changed them here? I think > I didn't, because then it would not compile. So we do agree that's there's no benefit on any Intel platform released in the last 4 years... The CI tests [1] don't show any regression, nothing was broken on our devices so I guess there's no further objection. Tested-by: Pierre-Louis Bossart <pierre-louis.bossart(a)linux.intel.com> [1] https://github.com/thesofproject/linux/pull/4949

1 0

Re: [PATCH 00/14] ASoC: Constify local snd_sof_dsp_ops
by Pierre-Louis Bossart 23 Apr '24

23 Apr '24

> Are we talking about basic C now? Of course it does not mean that and I > already explained what is the goal of this - the static or global memory > in the driver can be moved to rodata. the dsp_ops used by the Intel drivers cannot be moved to rodata in all cases. the baseline is constant, all extensions for TGL+ are dynamically allocated and modified.

1 0

Re: [PATCH 00/14] ASoC: Constify local snd_sof_dsp_ops
by Pierre-Louis Bossart 22 Apr '24

22 Apr '24

>>> There are multiple reasons and benefits for const, like compiler >>> optimization, code readability (meaning) up to security improvements, >>> e.g. by some GCC plugins or marking rodata as really non-writeable, so >>> closing some ways of exploits. There are many opportunities here, even >>> if they are not yet enabled. >> >> Possibly, but the SOF core does not know if the structure it uses is >> rodata or not. Using the 'const' identifier would be misleading. > > How so? If core does not modify structure, it should take it via ops, > just like 100 other widely known structures (see checkpatch). Why is > this different? I don't understand "it should take it via ops" We are already fetching the structure with private_data. >>>> that's a different interpretation to the 'software' view you're >>>> describing. "this structure will not modified by this function" is not >>>> the same thing as "this structure CANNOT be modified". >>> >>> Yes, but can we please discuss specific patchset then? Patches which >>> change pointers to const have one "interpretation". Patches which modify >>> static or global data have another. >> >> Just look at sound/soc/sof/intel/mtl.c... The core will sometimes use a > > That's a driver (or specific implementation), not core. You are making an assumption on what the SOF core is. The core is used by ACPI or PCI drivers as a library. The structures are all allocated in ACPI/PCI drivers and passed to the core library. >> constant structure and sometimes not, depending on the PCI ID reported >> by hardware. This was intentional to override common defaults and make >> the differences limited in scope between hardware generations. > > >> >> int sof_mtl_ops_init(struct snd_sof_dev *sdev) >> { >> struct sof_ipc4_fw_data *ipc4_data; >> >> /* common defaults */ >> memcpy(&sof_mtl_ops, &sof_hda_common_ops, sizeof(struct >> snd_sof_dsp_ops)); <<<< THE BASELINE IS CONSTANT > > Yes, I saw it and such users are not changed. They won't receive any > safety. But all others are getting safer. Maybe there's a misunderstanding on what the 'SOF core' is. This is just a helper library that are used by the PCI drivers. The core has zero knowledge on anything really. > I really do not understand what is the problem here. In entire Linux all > of such changes are welcomed with open arms. So what is different here? Adding 'const' at the SOF core level does not mean that we can treat structures as rodata. It only means that the structure is not modified by the core library. Not the same thing.

1 0