[PATCH 00/10] ALSA/ASoC: hda: cleanup sound/hda/ext code
The code in sound/hda/ext can be cleaned-up. This patchset suggests a more consistent use of prefixes and variable names. It also removes useless code, uses helpers when possible, and clarifies an incorrect assumption that SPIB/DRMS capabilities are tied to Intel multi-link and DSP extensions (they are not).
This is a rather invasive set of changes touching sound/hda, sound/soc/intel/skylake, sound/soc/intel/avs and sound/soc/sof/intel, but there should be no change in terms of functionality.
I would recommend merging these changes through the ASoC tree.
Pierre-Louis Bossart (10): ALSA: hda: ext: hdac_ext_controller: use helpers in loop ALSA: hda: ext: hda_ext_controller: use hlink variable/parameter ASoC: SOF: Intel: hda-dai: use hlink variable/parameter ALSA/ASoC: hda: clarify bus_get_link() and bus_link_get() helpers ALSA/ASoC: hda: ext: add 'ext' prefix to snd_hdac_link_free_all ALSA/ASoC: hda: ext: remove 'link' prefix for stream-related operations ALSA/ASoC: hda: ext: add 'bus' prefix for multi-link stream setting ALSA: hda: ext: reduce ambiguity between 'multi-link' and 'link' DMA ALSA: hda: hdac_ext_controller: remove useless loop ALSA/ASoC: hda: move SPIB/DRMS functionality from ext layer
include/sound/hdaudio.h | 26 ++++ include/sound/hdaudio_ext.h | 66 ++------ sound/hda/ext/hdac_ext_controller.c | 116 ++++++++------ sound/hda/ext/hdac_ext_stream.c | 203 +++---------------------- sound/hda/hdac_stream.c | 136 +++++++++++++++++ sound/soc/codecs/hda.c | 4 +- sound/soc/codecs/hdac_hda.c | 6 +- sound/soc/codecs/hdac_hdmi.c | 8 +- sound/soc/intel/avs/core.c | 2 +- sound/soc/intel/avs/loader.c | 16 +- sound/soc/intel/avs/pcm.c | 16 +- sound/soc/intel/skylake/skl-messages.c | 6 +- sound/soc/intel/skylake/skl-pcm.c | 35 +++-- sound/soc/intel/skylake/skl.c | 2 +- sound/soc/sof/intel/hda-dai.c | 42 ++--- sound/soc/sof/intel/hda-pcm.c | 3 +- sound/soc/sof/intel/hda-stream.c | 16 +- sound/soc/sof/intel/hda.c | 2 +- 18 files changed, 343 insertions(+), 362 deletions(-)
No need to copy/paste code, use helper instead.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Reviewed-by: Rander Wang rander.wang@intel.com Reviewed-by: Péter Ujfalusi peter.ujfalusi@linux.intel.com --- sound/hda/ext/hdac_ext_controller.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/sound/hda/ext/hdac_ext_controller.c b/sound/hda/ext/hdac_ext_controller.c index 80876b9a87f46..f521d286a3942 100644 --- a/sound/hda/ext/hdac_ext_controller.c +++ b/sound/hda/ext/hdac_ext_controller.c @@ -225,9 +225,7 @@ int snd_hdac_ext_bus_link_power_up_all(struct hdac_bus *bus) int ret;
list_for_each_entry(hlink, &bus->hlink_list, list) { - snd_hdac_updatel(hlink->ml_addr, AZX_REG_ML_LCTL, - AZX_ML_LCTL_SPA, AZX_ML_LCTL_SPA); - ret = check_hdac_link_power_active(hlink, true); + ret = snd_hdac_ext_bus_link_power_up(hlink); if (ret < 0) return ret; } @@ -246,9 +244,7 @@ int snd_hdac_ext_bus_link_power_down_all(struct hdac_bus *bus) int ret;
list_for_each_entry(hlink, &bus->hlink_list, list) { - snd_hdac_updatel(hlink->ml_addr, AZX_REG_ML_LCTL, - AZX_ML_LCTL_SPA, 0); - ret = check_hdac_link_power_active(hlink, false); + ret = snd_hdac_ext_bus_link_power_down(hlink); if (ret < 0) return ret; }
On 2022-10-19 6:21 PM, Pierre-Louis Bossart wrote:
No need to copy/paste code, use helper instead.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Reviewed-by: Rander Wang rander.wang@intel.com Reviewed-by: Péter Ujfalusi peter.ujfalusi@linux.intel.com
LGTM
Reviewed-by: Cezary Rojewski cezary.rojewski@intel.com
Follow the convention and use hlink for consistency. No functionality change.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Reviewed-by: Rander Wang rander.wang@intel.com Reviewed-by: Péter Ujfalusi peter.ujfalusi@linux.intel.com --- include/sound/hdaudio_ext.h | 16 +++++----- sound/hda/ext/hdac_ext_controller.c | 48 ++++++++++++++--------------- 2 files changed, 32 insertions(+), 32 deletions(-)
diff --git a/include/sound/hdaudio_ext.h b/include/sound/hdaudio_ext.h index 83aed26ab1433..8e48a4decfa08 100644 --- a/include/sound/hdaudio_ext.h +++ b/include/sound/hdaudio_ext.h @@ -117,17 +117,17 @@ struct hdac_ext_link { struct list_head list; };
-int snd_hdac_ext_bus_link_power_up(struct hdac_ext_link *link); -int snd_hdac_ext_bus_link_power_down(struct hdac_ext_link *link); +int snd_hdac_ext_bus_link_power_up(struct hdac_ext_link *hlink); +int snd_hdac_ext_bus_link_power_down(struct hdac_ext_link *hlink); int snd_hdac_ext_bus_link_power_up_all(struct hdac_bus *bus); int snd_hdac_ext_bus_link_power_down_all(struct hdac_bus *bus); -void snd_hdac_ext_link_set_stream_id(struct hdac_ext_link *link, - int stream); -void snd_hdac_ext_link_clear_stream_id(struct hdac_ext_link *link, - int stream); +void snd_hdac_ext_link_set_stream_id(struct hdac_ext_link *hlink, + int stream); +void snd_hdac_ext_link_clear_stream_id(struct hdac_ext_link *hlink, + int stream);
-int snd_hdac_ext_bus_link_get(struct hdac_bus *bus, struct hdac_ext_link *link); -int snd_hdac_ext_bus_link_put(struct hdac_bus *bus, struct hdac_ext_link *link); +int snd_hdac_ext_bus_link_get(struct hdac_bus *bus, struct hdac_ext_link *hlink); +int snd_hdac_ext_bus_link_put(struct hdac_bus *bus, struct hdac_ext_link *hlink);
void snd_hdac_ext_bus_link_power(struct hdac_device *codec, bool enable);
diff --git a/sound/hda/ext/hdac_ext_controller.c b/sound/hda/ext/hdac_ext_controller.c index f521d286a3942..d3e9c4ae19ee3 100644 --- a/sound/hda/ext/hdac_ext_controller.c +++ b/sound/hda/ext/hdac_ext_controller.c @@ -115,12 +115,12 @@ EXPORT_SYMBOL_GPL(snd_hdac_ext_bus_get_ml_capabilities);
void snd_hdac_link_free_all(struct hdac_bus *bus) { - struct hdac_ext_link *l; + struct hdac_ext_link *hlink;
while (!list_empty(&bus->hlink_list)) { - l = list_first_entry(&bus->hlink_list, struct hdac_ext_link, list); - list_del(&l->list); - kfree(l); + hlink = list_first_entry(&bus->hlink_list, struct hdac_ext_link, list); + list_del(&hlink->list); + kfree(hlink); } } EXPORT_SYMBOL_GPL(snd_hdac_link_free_all); @@ -166,7 +166,7 @@ struct hdac_ext_link *snd_hdac_ext_bus_get_link(struct hdac_bus *bus, } EXPORT_SYMBOL_GPL(snd_hdac_ext_bus_get_link);
-static int check_hdac_link_power_active(struct hdac_ext_link *link, bool enable) +static int check_hdac_link_power_active(struct hdac_ext_link *hlink, bool enable) { int timeout; u32 val; @@ -176,7 +176,7 @@ static int check_hdac_link_power_active(struct hdac_ext_link *link, bool enable) timeout = 150;
do { - val = readl(link->ml_addr + AZX_REG_ML_LCTL); + val = readl(hlink->ml_addr + AZX_REG_ML_LCTL); if (enable) { if (((val & mask) >> AZX_ML_LCTL_CPA_SHIFT)) return 0; @@ -192,26 +192,26 @@ static int check_hdac_link_power_active(struct hdac_ext_link *link, bool enable)
/** * snd_hdac_ext_bus_link_power_up -power up hda link - * @link: HD-audio extended link + * @hlink: HD-audio extended link */ -int snd_hdac_ext_bus_link_power_up(struct hdac_ext_link *link) +int snd_hdac_ext_bus_link_power_up(struct hdac_ext_link *hlink) { - snd_hdac_updatel(link->ml_addr, AZX_REG_ML_LCTL, + snd_hdac_updatel(hlink->ml_addr, AZX_REG_ML_LCTL, AZX_ML_LCTL_SPA, AZX_ML_LCTL_SPA);
- return check_hdac_link_power_active(link, true); + return check_hdac_link_power_active(hlink, true); } EXPORT_SYMBOL_GPL(snd_hdac_ext_bus_link_power_up);
/** * snd_hdac_ext_bus_link_power_down -power down hda link - * @link: HD-audio extended link + * @hlink: HD-audio extended link */ -int snd_hdac_ext_bus_link_power_down(struct hdac_ext_link *link) +int snd_hdac_ext_bus_link_power_down(struct hdac_ext_link *hlink) { - snd_hdac_updatel(link->ml_addr, AZX_REG_ML_LCTL, AZX_ML_LCTL_SPA, 0); + snd_hdac_updatel(hlink->ml_addr, AZX_REG_ML_LCTL, AZX_ML_LCTL_SPA, 0);
- return check_hdac_link_power_active(link, false); + return check_hdac_link_power_active(hlink, false); } EXPORT_SYMBOL_GPL(snd_hdac_ext_bus_link_power_down);
@@ -254,7 +254,7 @@ int snd_hdac_ext_bus_link_power_down_all(struct hdac_bus *bus) EXPORT_SYMBOL_GPL(snd_hdac_ext_bus_link_power_down_all);
int snd_hdac_ext_bus_link_get(struct hdac_bus *bus, - struct hdac_ext_link *link) + struct hdac_ext_link *hlink) { unsigned long codec_mask; int ret = 0; @@ -265,18 +265,18 @@ int snd_hdac_ext_bus_link_get(struct hdac_bus *bus, * if we move from 0 to 1, count will be 1 so power up this link * as well, also check the dma status and trigger that */ - if (++link->ref_count == 1) { + if (++hlink->ref_count == 1) { if (!bus->cmd_dma_state) { snd_hdac_bus_init_cmd_io(bus); bus->cmd_dma_state = true; }
- ret = snd_hdac_ext_bus_link_power_up(link); + ret = snd_hdac_ext_bus_link_power_up(hlink);
/* * clear the register to invalidate all the output streams */ - snd_hdac_updatew(link->ml_addr, AZX_REG_ML_LOSIDV, + snd_hdac_updatew(hlink->ml_addr, AZX_REG_ML_LOSIDV, AZX_ML_LOSIDV_STREAM_MASK, 0); /* * wait for 521usec for codec to report status @@ -296,10 +296,10 @@ int snd_hdac_ext_bus_link_get(struct hdac_bus *bus, EXPORT_SYMBOL_GPL(snd_hdac_ext_bus_link_get);
int snd_hdac_ext_bus_link_put(struct hdac_bus *bus, - struct hdac_ext_link *link) + struct hdac_ext_link *hlink) { int ret = 0; - struct hdac_ext_link *hlink; + struct hdac_ext_link *hlink_tmp; bool link_up = false;
mutex_lock(&bus->lock); @@ -308,15 +308,15 @@ int snd_hdac_ext_bus_link_put(struct hdac_bus *bus, * if we move from 1 to 0, count will be 0 * so power down this link as well */ - if (--link->ref_count == 0) { - ret = snd_hdac_ext_bus_link_power_down(link); + if (--hlink->ref_count == 0) { + ret = snd_hdac_ext_bus_link_power_down(hlink);
/* * now check if all links are off, if so turn off * cmd dma as well */ - list_for_each_entry(hlink, &bus->hlink_list, list) { - if (hlink->ref_count) { + list_for_each_entry(hlink_tmp, &bus->hlink_list, list) { + if (hlink_tmp->ref_count) { link_up = true; break; }
On 2022-10-19 6:21 PM, Pierre-Louis Bossart wrote:
Follow the convention and use hlink for consistency. No functionality change.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Reviewed-by: Rander Wang rander.wang@intel.com Reviewed-by: Péter Ujfalusi peter.ujfalusi@linux.intel.com
LGTM
Reviewed-by: Cezary Rojewski cezary.rojewski@intel.com
Follow the convention and use hlink for consistency. No functionality change.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Reviewed-by: Rander Wang rander.wang@intel.com Reviewed-by: Péter Ujfalusi peter.ujfalusi@linux.intel.com --- sound/soc/sof/intel/hda-dai.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/sound/soc/sof/intel/hda-dai.c b/sound/soc/sof/intel/hda-dai.c index 556e883a32edb..0fadf99f2efad 100644 --- a/sound/soc/sof/intel/hda-dai.c +++ b/sound/soc/sof/intel/hda-dai.c @@ -146,11 +146,11 @@ static int hda_link_dma_cleanup(struct snd_pcm_substream *substream, struct hdac_ext_stream *hext_stream = snd_soc_dai_get_dma_data(cpu_dai, substream); struct hdac_bus *bus = hstream->bus; struct sof_intel_hda_stream *hda_stream; - struct hdac_ext_link *link; + struct hdac_ext_link *hlink; int stream_tag;
- link = snd_hdac_ext_bus_get_link(bus, codec_dai->component->name); - if (!link) + hlink = snd_hdac_ext_bus_get_link(bus, codec_dai->component->name); + if (!hlink) return -EINVAL;
if (trigger_suspend_stop) @@ -158,7 +158,7 @@ static int hda_link_dma_cleanup(struct snd_pcm_substream *substream,
if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { stream_tag = hdac_stream(hext_stream)->stream_tag; - snd_hdac_ext_link_clear_stream_id(link, stream_tag); + snd_hdac_ext_link_clear_stream_id(hlink, stream_tag); } snd_soc_dai_set_dma_data(cpu_dai, substream, NULL); snd_hdac_ext_stream_release(hext_stream, HDAC_EXT_STREAM_TYPE_LINK); @@ -177,7 +177,7 @@ static int hda_link_dma_params(struct hdac_ext_stream *hext_stream, struct hdac_stream *hstream = &hext_stream->hstream; unsigned char stream_tag = hstream->stream_tag; struct hdac_bus *bus = hstream->bus; - struct hdac_ext_link *link; + struct hdac_ext_link *hlink; unsigned int format_val;
snd_hdac_ext_link_stream_reset(hext_stream); @@ -192,9 +192,9 @@ static int hda_link_dma_params(struct hdac_ext_stream *hext_stream, snd_hdac_ext_link_stream_setup(hext_stream, format_val);
if (hext_stream->hstream.direction == SNDRV_PCM_STREAM_PLAYBACK) { - list_for_each_entry(link, &bus->hlink_list, list) { - if (link->index == params->link_index) - snd_hdac_ext_link_set_stream_id(link, + list_for_each_entry(hlink, &bus->hlink_list, list) { + if (hlink->index == params->link_index) + snd_hdac_ext_link_set_stream_id(hlink, stream_tag); } } @@ -214,7 +214,7 @@ static int hda_link_dma_hw_params(struct snd_pcm_substream *substream, struct snd_soc_dai *codec_dai = asoc_rtd_to_codec(rtd, 0); struct hda_pipe_params p_params = {0}; struct hdac_bus *bus = hstream->bus; - struct hdac_ext_link *link; + struct hdac_ext_link *hlink;
hext_stream = snd_soc_dai_get_dma_data(cpu_dai, substream); if (!hext_stream) { @@ -225,8 +225,8 @@ static int hda_link_dma_hw_params(struct snd_pcm_substream *substream, snd_soc_dai_set_dma_data(cpu_dai, substream, (void *)hext_stream); }
- link = snd_hdac_ext_bus_get_link(bus, codec_dai->component->name); - if (!link) + hlink = snd_hdac_ext_bus_get_link(bus, codec_dai->component->name); + if (!hlink) return -EINVAL;
/* set the hdac_stream in the codec dai */ @@ -236,7 +236,7 @@ static int hda_link_dma_hw_params(struct snd_pcm_substream *substream, p_params.ch = params_channels(params); p_params.s_freq = params_rate(params); p_params.stream = substream->stream; - p_params.link_index = link->index; + p_params.link_index = hlink->index; p_params.format = params_format(params);
if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
On Wed, Oct 19, 2022 at 11:21:08AM -0500, Pierre-Louis Bossart wrote:
Follow the convention and use hlink for consistency. No functionality change.
Acked-by: Mark Brown broonie@kernel.org
We have two helpers with confusing names and different purposes.
Rename bus_get_link() and bus_get_link_at() as bus_get_hlink_by_name() and bus_get_hlink_by_addr() respectively.
No functionality change
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Reviewed-by: Rander Wang rander.wang@intel.com Reviewed-by: Péter Ujfalusi peter.ujfalusi@linux.intel.com --- include/sound/hdaudio_ext.h | 6 +++--- sound/hda/ext/hdac_ext_controller.c | 24 ++++++++++++------------ sound/soc/codecs/hda.c | 4 ++-- sound/soc/codecs/hdac_hda.c | 6 +++--- sound/soc/codecs/hdac_hdmi.c | 8 ++++---- sound/soc/intel/avs/pcm.c | 4 ++-- sound/soc/intel/skylake/skl-pcm.c | 4 ++-- sound/soc/sof/intel/hda-dai.c | 4 ++-- 8 files changed, 30 insertions(+), 30 deletions(-)
diff --git a/include/sound/hdaudio_ext.h b/include/sound/hdaudio_ext.h index 8e48a4decfa08..397ab4fc98283 100644 --- a/include/sound/hdaudio_ext.h +++ b/include/sound/hdaudio_ext.h @@ -27,9 +27,9 @@ void snd_hdac_ext_stream_spbcap_enable(struct hdac_bus *chip, bool enable, int index);
int snd_hdac_ext_bus_get_ml_capabilities(struct hdac_bus *bus); -struct hdac_ext_link *snd_hdac_ext_bus_link_at(struct hdac_bus *bus, int addr); -struct hdac_ext_link *snd_hdac_ext_bus_get_link(struct hdac_bus *bus, - const char *codec_name); +struct hdac_ext_link *snd_hdac_ext_bus_get_hlink_by_addr(struct hdac_bus *bus, int addr); +struct hdac_ext_link *snd_hdac_ext_bus_get_hlink_by_name(struct hdac_bus *bus, + const char *codec_name);
enum hdac_ext_stream_type { HDAC_EXT_STREAM_TYPE_COUPLED = 0, diff --git a/sound/hda/ext/hdac_ext_controller.c b/sound/hda/ext/hdac_ext_controller.c index d3e9c4ae19ee3..3730c30470f12 100644 --- a/sound/hda/ext/hdac_ext_controller.c +++ b/sound/hda/ext/hdac_ext_controller.c @@ -126,13 +126,13 @@ void snd_hdac_link_free_all(struct hdac_bus *bus) EXPORT_SYMBOL_GPL(snd_hdac_link_free_all);
/** - * snd_hdac_ext_bus_link_at - get link at specified address - * @bus: link's parent bus device + * snd_hdac_ext_bus_get_hlink_by_addr - get hlink at specified address + * @bus: hlink's parent bus device * @addr: codec device address * - * Returns link object or NULL if matching link is not found. + * Returns hlink object or NULL if matching hlink is not found. */ -struct hdac_ext_link *snd_hdac_ext_bus_link_at(struct hdac_bus *bus, int addr) +struct hdac_ext_link *snd_hdac_ext_bus_get_hlink_by_addr(struct hdac_bus *bus, int addr) { struct hdac_ext_link *hlink; int i; @@ -143,15 +143,15 @@ struct hdac_ext_link *snd_hdac_ext_bus_link_at(struct hdac_bus *bus, int addr) return hlink; return NULL; } -EXPORT_SYMBOL_GPL(snd_hdac_ext_bus_link_at); +EXPORT_SYMBOL_GPL(snd_hdac_ext_bus_get_hlink_by_addr);
/** - * snd_hdac_ext_bus_get_link - get link based on codec name + * snd_hdac_ext_bus_get_hlink_by_name - get hlink based on codec name * @bus: the pointer to HDAC bus object * @codec_name: codec name */ -struct hdac_ext_link *snd_hdac_ext_bus_get_link(struct hdac_bus *bus, - const char *codec_name) +struct hdac_ext_link *snd_hdac_ext_bus_get_hlink_by_name(struct hdac_bus *bus, + const char *codec_name) { int bus_idx, addr;
@@ -162,9 +162,9 @@ struct hdac_ext_link *snd_hdac_ext_bus_get_link(struct hdac_bus *bus, if (addr < 0 || addr > 31) return NULL;
- return snd_hdac_ext_bus_link_at(bus, addr); + return snd_hdac_ext_bus_get_hlink_by_addr(bus, addr); } -EXPORT_SYMBOL_GPL(snd_hdac_ext_bus_get_link); +EXPORT_SYMBOL_GPL(snd_hdac_ext_bus_get_hlink_by_name);
static int check_hdac_link_power_active(struct hdac_ext_link *hlink, bool enable) { @@ -337,7 +337,7 @@ static void hdac_ext_codec_link_up(struct hdac_device *codec) { const char *devname = dev_name(&codec->dev); struct hdac_ext_link *hlink = - snd_hdac_ext_bus_get_link(codec->bus, devname); + snd_hdac_ext_bus_get_hlink_by_name(codec->bus, devname);
if (hlink) snd_hdac_ext_bus_link_get(codec->bus, hlink); @@ -347,7 +347,7 @@ static void hdac_ext_codec_link_down(struct hdac_device *codec) { const char *devname = dev_name(&codec->dev); struct hdac_ext_link *hlink = - snd_hdac_ext_bus_get_link(codec->bus, devname); + snd_hdac_ext_bus_get_hlink_by_name(codec->bus, devname);
if (hlink) snd_hdac_ext_bus_link_put(codec->bus, hlink); diff --git a/sound/soc/codecs/hda.c b/sound/soc/codecs/hda.c index 61e8e9be6b8d7..0b8ccc5be4806 100644 --- a/sound/soc/codecs/hda.c +++ b/sound/soc/codecs/hda.c @@ -181,7 +181,7 @@ static int hda_codec_probe(struct snd_soc_component *component) !pm_runtime_status_suspended(&hdev->dev)); #endif
- hlink = snd_hdac_ext_bus_link_at(bus, hdev->addr); + hlink = snd_hdac_ext_bus_get_hlink_by_addr(bus, hdev->addr); if (!hlink) { dev_err(&hdev->dev, "hdac link not found\n"); return -EIO; @@ -289,7 +289,7 @@ static void hda_codec_remove(struct snd_soc_component *component) if (hda_codec_is_display(codec)) snd_hdac_display_power(bus, hdev->addr, false);
- hlink = snd_hdac_ext_bus_link_at(bus, hdev->addr); + hlink = snd_hdac_ext_bus_get_hlink_by_addr(bus, hdev->addr); if (hlink) snd_hdac_ext_bus_link_put(bus, hlink); /* diff --git a/sound/soc/codecs/hdac_hda.c b/sound/soc/codecs/hdac_hda.c index 8af434e14bfba..be66853afbe2b 100644 --- a/sound/soc/codecs/hdac_hda.c +++ b/sound/soc/codecs/hdac_hda.c @@ -400,7 +400,7 @@ static int hdac_hda_codec_probe(struct snd_soc_component *component) hda_codec_patch_t patch; int ret;
- hlink = snd_hdac_ext_bus_get_link(hdev->bus, dev_name(&hdev->dev)); + hlink = snd_hdac_ext_bus_get_hlink_by_name(hdev->bus, dev_name(&hdev->dev)); if (!hlink) { dev_err(&hdev->dev, "hdac link not found\n"); return -EIO; @@ -516,7 +516,7 @@ static void hdac_hda_codec_remove(struct snd_soc_component *component) struct hda_codec *codec = hda_pvt->codec; struct hdac_ext_link *hlink = NULL;
- hlink = snd_hdac_ext_bus_get_link(hdev->bus, dev_name(&hdev->dev)); + hlink = snd_hdac_ext_bus_get_hlink_by_name(hdev->bus, dev_name(&hdev->dev)); if (!hlink) { dev_err(&hdev->dev, "hdac link not found\n"); return; @@ -584,7 +584,7 @@ static int hdac_hda_dev_probe(struct hdac_device *hdev) int ret;
/* hold the ref while we probe */ - hlink = snd_hdac_ext_bus_get_link(hdev->bus, dev_name(&hdev->dev)); + hlink = snd_hdac_ext_bus_get_hlink_by_name(hdev->bus, dev_name(&hdev->dev)); if (!hlink) { dev_err(&hdev->dev, "hdac link not found\n"); return -EIO; diff --git a/sound/soc/codecs/hdac_hdmi.c b/sound/soc/codecs/hdac_hdmi.c index cb23650ad5223..ed4f7cdda04ff 100644 --- a/sound/soc/codecs/hdac_hdmi.c +++ b/sound/soc/codecs/hdac_hdmi.c @@ -1967,7 +1967,7 @@ static int hdmi_codec_probe(struct snd_soc_component *component) * hold the ref while we probe, also no need to drop the ref on * exit, we call pm_runtime_suspend() so that will do for us */ - hlink = snd_hdac_ext_bus_get_link(hdev->bus, dev_name(&hdev->dev)); + hlink = snd_hdac_ext_bus_get_hlink_by_name(hdev->bus, dev_name(&hdev->dev)); if (!hlink) { dev_err(&hdev->dev, "hdac link not found\n"); return -EIO; @@ -2144,7 +2144,7 @@ static int hdac_hdmi_dev_probe(struct hdac_device *hdev) const struct hda_device_id *hdac_id = hdac_get_device_id(hdev, hdrv);
/* hold the ref while we probe */ - hlink = snd_hdac_ext_bus_get_link(hdev->bus, dev_name(&hdev->dev)); + hlink = snd_hdac_ext_bus_get_hlink_by_name(hdev->bus, dev_name(&hdev->dev)); if (!hlink) { dev_err(&hdev->dev, "hdac link not found\n"); return -EIO; @@ -2244,7 +2244,7 @@ static int hdac_hdmi_runtime_suspend(struct device *dev) snd_hdac_codec_read(hdev, hdev->afg, 0, AC_VERB_SET_POWER_STATE, AC_PWRST_D3);
- hlink = snd_hdac_ext_bus_get_link(bus, dev_name(dev)); + hlink = snd_hdac_ext_bus_get_hlink_by_name(bus, dev_name(dev)); if (!hlink) { dev_err(dev, "hdac link not found\n"); return -EIO; @@ -2270,7 +2270,7 @@ static int hdac_hdmi_runtime_resume(struct device *dev) if (!bus) return 0;
- hlink = snd_hdac_ext_bus_get_link(bus, dev_name(dev)); + hlink = snd_hdac_ext_bus_get_hlink_by_name(bus, dev_name(dev)); if (!hlink) { dev_err(dev, "hdac link not found\n"); return -EIO; diff --git a/sound/soc/intel/avs/pcm.c b/sound/soc/intel/avs/pcm.c index 8fe5917b1e263..4e5849535a8c6 100644 --- a/sound/soc/intel/avs/pcm.c +++ b/sound/soc/intel/avs/pcm.c @@ -292,7 +292,7 @@ static int avs_dai_hda_be_hw_free(struct snd_pcm_substream *substream, struct sn
/* clear link <-> stream mapping */ codec = dev_to_hda_codec(asoc_rtd_to_codec(rtd, 0)->dev); - link = snd_hdac_ext_bus_link_at(&codec->bus->core, codec->core.addr); + link = snd_hdac_ext_bus_get_hlink_by_addr(&codec->bus->core, codec->core.addr); if (!link) return -EINVAL;
@@ -325,7 +325,7 @@ static int avs_dai_hda_be_prepare(struct snd_pcm_substream *substream, struct sn snd_hdac_ext_link_stream_reset(link_stream); snd_hdac_ext_link_stream_setup(link_stream, format_val);
- link = snd_hdac_ext_bus_link_at(bus, codec->core.addr); + link = snd_hdac_ext_bus_get_hlink_by_addr(bus, codec->core.addr); if (!link) return -EINVAL;
diff --git a/sound/soc/intel/skylake/skl-pcm.c b/sound/soc/intel/skylake/skl-pcm.c index 1015716f93361..b07c5b58e0a62 100644 --- a/sound/soc/intel/skylake/skl-pcm.c +++ b/sound/soc/intel/skylake/skl-pcm.c @@ -558,7 +558,7 @@ static int skl_link_hw_params(struct snd_pcm_substream *substream,
snd_soc_dai_set_dma_data(dai, substream, (void *)link_dev);
- link = snd_hdac_ext_bus_get_link(bus, codec_dai->component->name); + link = snd_hdac_ext_bus_get_hlink_by_name(bus, codec_dai->component->name); if (!link) return -EINVAL;
@@ -643,7 +643,7 @@ static int skl_link_hw_free(struct snd_pcm_substream *substream,
link_dev->link_prepared = 0;
- link = snd_hdac_ext_bus_get_link(bus, asoc_rtd_to_codec(rtd, 0)->component->name); + link = snd_hdac_ext_bus_get_hlink_by_name(bus, asoc_rtd_to_codec(rtd, 0)->component->name); if (!link) return -EINVAL;
diff --git a/sound/soc/sof/intel/hda-dai.c b/sound/soc/sof/intel/hda-dai.c index 0fadf99f2efad..b71bb29e764b3 100644 --- a/sound/soc/sof/intel/hda-dai.c +++ b/sound/soc/sof/intel/hda-dai.c @@ -149,7 +149,7 @@ static int hda_link_dma_cleanup(struct snd_pcm_substream *substream, struct hdac_ext_link *hlink; int stream_tag;
- hlink = snd_hdac_ext_bus_get_link(bus, codec_dai->component->name); + hlink = snd_hdac_ext_bus_get_hlink_by_name(bus, codec_dai->component->name); if (!hlink) return -EINVAL;
@@ -225,7 +225,7 @@ static int hda_link_dma_hw_params(struct snd_pcm_substream *substream, snd_soc_dai_set_dma_data(cpu_dai, substream, (void *)hext_stream); }
- hlink = snd_hdac_ext_bus_get_link(bus, codec_dai->component->name); + hlink = snd_hdac_ext_bus_get_hlink_by_name(bus, codec_dai->component->name); if (!hlink) return -EINVAL;
On Wed, Oct 19, 2022 at 11:21:09AM -0500, Pierre-Louis Bossart wrote:
We have two helpers with confusing names and different purposes.
Rename bus_get_link() and bus_get_link_at() as bus_get_hlink_by_name() and bus_get_hlink_by_addr() respectively.
Acked-by: Mark Brown broonie@kernel.org
No functionality change, just prefix addition to clearly identify that the helper only applies to the 'ext' part for Intel platforms.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Bard Liao yung-chuan.liao@linux.intel.com Reviewed-by: Péter Ujfalusi peter.ujfalusi@linux.intel.com --- include/sound/hdaudio_ext.h | 2 +- sound/hda/ext/hdac_ext_controller.c | 6 +++--- sound/soc/intel/avs/core.c | 2 +- sound/soc/intel/skylake/skl.c | 2 +- sound/soc/sof/intel/hda.c | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/include/sound/hdaudio_ext.h b/include/sound/hdaudio_ext.h index 397ab4fc98283..f0edbadc99023 100644 --- a/include/sound/hdaudio_ext.h +++ b/include/sound/hdaudio_ext.h @@ -80,7 +80,7 @@ struct hdac_ext_stream { int snd_hdac_ext_stream_init_all(struct hdac_bus *bus, int start_idx, int num_stream, int dir); void snd_hdac_ext_stream_free_all(struct hdac_bus *bus); -void snd_hdac_link_free_all(struct hdac_bus *bus); +void snd_hdac_ext_link_free_all(struct hdac_bus *bus); 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_controller.c b/sound/hda/ext/hdac_ext_controller.c index 3730c30470f12..c0b801fa93c09 100644 --- a/sound/hda/ext/hdac_ext_controller.c +++ b/sound/hda/ext/hdac_ext_controller.c @@ -108,12 +108,12 @@ int snd_hdac_ext_bus_get_ml_capabilities(struct hdac_bus *bus) EXPORT_SYMBOL_GPL(snd_hdac_ext_bus_get_ml_capabilities);
/** - * snd_hdac_link_free_all- free hdac extended link objects + * snd_hdac_ext_link_free_all- free hdac extended link objects * * @bus: the pointer to HDAC bus object */
-void snd_hdac_link_free_all(struct hdac_bus *bus) +void snd_hdac_ext_link_free_all(struct hdac_bus *bus) { struct hdac_ext_link *hlink;
@@ -123,7 +123,7 @@ void snd_hdac_link_free_all(struct hdac_bus *bus) kfree(hlink); } } -EXPORT_SYMBOL_GPL(snd_hdac_link_free_all); +EXPORT_SYMBOL_GPL(snd_hdac_ext_link_free_all);
/** * snd_hdac_ext_bus_get_hlink_by_addr - get hlink at specified address diff --git a/sound/soc/intel/avs/core.c b/sound/soc/intel/avs/core.c index 58a51361c4b11..04a7f49ea00af 100644 --- a/sound/soc/intel/avs/core.c +++ b/sound/soc/intel/avs/core.c @@ -504,7 +504,7 @@ static void avs_pci_remove(struct pci_dev *pci) snd_hdac_bus_free_stream_pages(bus); snd_hdac_ext_stream_free_all(bus); /* reverse ml_capabilities */ - snd_hdac_link_free_all(bus); + snd_hdac_ext_link_free_all(bus); snd_hdac_ext_bus_exit(bus);
avs_dsp_core_disable(adev, GENMASK(adev->hw_cfg.dsp_cores - 1, 0)); diff --git a/sound/soc/intel/skylake/skl.c b/sound/soc/intel/skylake/skl.c index 1cfdb04f589fe..c6f319bcd2c4e 100644 --- a/sound/soc/intel/skylake/skl.c +++ b/sound/soc/intel/skylake/skl.c @@ -436,7 +436,7 @@ static int skl_free(struct hdac_bus *bus) free_irq(bus->irq, (void *)bus); snd_hdac_bus_free_stream_pages(bus); snd_hdac_ext_stream_free_all(bus); - snd_hdac_link_free_all(bus); + snd_hdac_ext_link_free_all(bus);
if (bus->remap_addr) iounmap(bus->remap_addr); diff --git a/sound/soc/sof/intel/hda.c b/sound/soc/sof/intel/hda.c index 1188ec51816bd..c6c6ea8a73f60 100644 --- a/sound/soc/sof/intel/hda.c +++ b/sound/soc/sof/intel/hda.c @@ -1224,7 +1224,7 @@ int hda_dsp_remove(struct snd_sof_dev *sdev)
hda_dsp_stream_free(sdev); #if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA) - snd_hdac_link_free_all(bus); + snd_hdac_ext_link_free_all(bus); #endif
iounmap(sdev->bar[HDA_DSP_BAR]);
On Wed, Oct 19, 2022 at 11:21:10AM -0500, Pierre-Louis Bossart wrote:
No functionality change, just prefix addition to clearly identify that the helper only applies to the 'ext' part for Intel platforms.
Acked-by: Mark Brown broonie@kernel.org
We should only use 'link' in the context of multi-link configurations. Streams are configured from a different register space and are not dependent on link except for LOSIDV settings.
Not functionality change, just pure rename.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Bard Liao yung-chuan.liao@linux.intel.com Reviewed-by: Péter Ujfalusi peter.ujfalusi@linux.intel.com --- include/sound/hdaudio_ext.h | 8 ++++---- sound/hda/ext/hdac_ext_stream.c | 28 ++++++++++++++-------------- sound/soc/intel/avs/pcm.c | 8 ++++---- sound/soc/intel/skylake/skl-pcm.c | 8 ++++---- sound/soc/sof/intel/hda-dai.c | 16 ++++++++-------- 5 files changed, 34 insertions(+), 34 deletions(-)
diff --git a/include/sound/hdaudio_ext.h b/include/sound/hdaudio_ext.h index f0edbadc99023..472fed072e985 100644 --- a/include/sound/hdaudio_ext.h +++ b/include/sound/hdaudio_ext.h @@ -100,10 +100,10 @@ int snd_hdac_ext_stream_set_dpibr(struct hdac_bus *bus, struct hdac_ext_stream *hext_stream, u32 value); int snd_hdac_ext_stream_set_lpib(struct hdac_ext_stream *hext_stream, u32 value);
-void snd_hdac_ext_link_stream_start(struct hdac_ext_stream *hext_stream); -void snd_hdac_ext_link_stream_clear(struct hdac_ext_stream *hext_stream); -void snd_hdac_ext_link_stream_reset(struct hdac_ext_stream *hext_stream); -int snd_hdac_ext_link_stream_setup(struct hdac_ext_stream *hext_stream, int fmt); +void snd_hdac_ext_stream_start(struct hdac_ext_stream *hext_stream); +void snd_hdac_ext_stream_clear(struct hdac_ext_stream *hext_stream); +void snd_hdac_ext_stream_reset(struct hdac_ext_stream *hext_stream); +int snd_hdac_ext_stream_setup(struct hdac_ext_stream *hext_stream, int fmt);
struct hdac_ext_link { struct hdac_bus *bus; diff --git a/sound/hda/ext/hdac_ext_stream.c b/sound/hda/ext/hdac_ext_stream.c index 70f3ad71aaf0d..c06beaaffe0a1 100644 --- a/sound/hda/ext/hdac_ext_stream.c +++ b/sound/hda/ext/hdac_ext_stream.c @@ -140,36 +140,36 @@ void snd_hdac_ext_stream_decouple(struct hdac_bus *bus, EXPORT_SYMBOL_GPL(snd_hdac_ext_stream_decouple);
/** - * snd_hdac_ext_link_stream_start - start a stream + * snd_hdac_ext_stream_start - start a stream * @hext_stream: HD-audio ext core stream to start */ -void snd_hdac_ext_link_stream_start(struct hdac_ext_stream *hext_stream) +void snd_hdac_ext_stream_start(struct hdac_ext_stream *hext_stream) { snd_hdac_updatel(hext_stream->pplc_addr, AZX_REG_PPLCCTL, AZX_PPLCCTL_RUN, AZX_PPLCCTL_RUN); } -EXPORT_SYMBOL_GPL(snd_hdac_ext_link_stream_start); +EXPORT_SYMBOL_GPL(snd_hdac_ext_stream_start);
/** - * snd_hdac_ext_link_stream_clear - stop a stream DMA + * snd_hdac_ext_stream_clear - stop a stream DMA * @hext_stream: HD-audio ext core stream to stop */ -void snd_hdac_ext_link_stream_clear(struct hdac_ext_stream *hext_stream) +void snd_hdac_ext_stream_clear(struct hdac_ext_stream *hext_stream) { snd_hdac_updatel(hext_stream->pplc_addr, AZX_REG_PPLCCTL, AZX_PPLCCTL_RUN, 0); } -EXPORT_SYMBOL_GPL(snd_hdac_ext_link_stream_clear); +EXPORT_SYMBOL_GPL(snd_hdac_ext_stream_clear);
/** - * snd_hdac_ext_link_stream_reset - reset a stream + * snd_hdac_ext_stream_reset - reset a stream * @hext_stream: HD-audio ext core stream to reset */ -void snd_hdac_ext_link_stream_reset(struct hdac_ext_stream *hext_stream) +void snd_hdac_ext_stream_reset(struct hdac_ext_stream *hext_stream) { unsigned char val; int timeout;
- snd_hdac_ext_link_stream_clear(hext_stream); + snd_hdac_ext_stream_clear(hext_stream);
snd_hdac_updatel(hext_stream->pplc_addr, AZX_REG_PPLCCTL, AZX_PPLCCTL_STRST, AZX_PPLCCTL_STRST); @@ -196,20 +196,20 @@ void snd_hdac_ext_link_stream_reset(struct hdac_ext_stream *hext_stream) } while (--timeout);
} -EXPORT_SYMBOL_GPL(snd_hdac_ext_link_stream_reset); +EXPORT_SYMBOL_GPL(snd_hdac_ext_stream_reset);
/** - * snd_hdac_ext_link_stream_setup - set up the SD for streaming + * snd_hdac_ext_stream_setup - set up the SD for streaming * @hext_stream: HD-audio ext core stream to set up * @fmt: stream format */ -int snd_hdac_ext_link_stream_setup(struct hdac_ext_stream *hext_stream, int fmt) +int snd_hdac_ext_stream_setup(struct hdac_ext_stream *hext_stream, int fmt) { struct hdac_stream *hstream = &hext_stream->hstream; unsigned int val;
/* make sure the run bit is zero for SD */ - snd_hdac_ext_link_stream_clear(hext_stream); + snd_hdac_ext_stream_clear(hext_stream); /* program the stream_tag */ val = readl(hext_stream->pplc_addr + AZX_REG_PPLCCTL); val = (val & ~AZX_PPLCCTL_STRM_MASK) | @@ -221,7 +221,7 @@ int snd_hdac_ext_link_stream_setup(struct hdac_ext_stream *hext_stream, int fmt)
return 0; } -EXPORT_SYMBOL_GPL(snd_hdac_ext_link_stream_setup); +EXPORT_SYMBOL_GPL(snd_hdac_ext_stream_setup);
/** * snd_hdac_ext_link_set_stream_id - maps stream id to link output diff --git a/sound/soc/intel/avs/pcm.c b/sound/soc/intel/avs/pcm.c index 4e5849535a8c6..d77afe5c65b96 100644 --- a/sound/soc/intel/avs/pcm.c +++ b/sound/soc/intel/avs/pcm.c @@ -322,8 +322,8 @@ static int avs_dai_hda_be_prepare(struct snd_pcm_substream *substream, struct sn runtime->sample_bits, 0);
snd_hdac_ext_stream_decouple(bus, link_stream, true); - snd_hdac_ext_link_stream_reset(link_stream); - snd_hdac_ext_link_stream_setup(link_stream, format_val); + snd_hdac_ext_stream_reset(link_stream); + snd_hdac_ext_stream_setup(link_stream, format_val);
link = snd_hdac_ext_bus_get_hlink_by_addr(bus, codec->core.addr); if (!link) @@ -355,7 +355,7 @@ static int avs_dai_hda_be_trigger(struct snd_pcm_substream *substream, int cmd, switch (cmd) { case SNDRV_PCM_TRIGGER_START: case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: - snd_hdac_ext_link_stream_start(link_stream); + snd_hdac_ext_stream_start(link_stream);
ret = avs_path_run(data->path, AVS_TPLG_TRIGGER_AUTO); if (ret < 0) @@ -368,7 +368,7 @@ static int avs_dai_hda_be_trigger(struct snd_pcm_substream *substream, int cmd, if (ret < 0) dev_err(dai->dev, "pause BE path failed: %d\n", ret);
- snd_hdac_ext_link_stream_clear(link_stream); + snd_hdac_ext_stream_clear(link_stream);
if (cmd == SNDRV_PCM_TRIGGER_STOP) { ret = avs_path_reset(data->path); diff --git a/sound/soc/intel/skylake/skl-pcm.c b/sound/soc/intel/skylake/skl-pcm.c index b07c5b58e0a62..f7e97b5a449fc 100644 --- a/sound/soc/intel/skylake/skl-pcm.c +++ b/sound/soc/intel/skylake/skl-pcm.c @@ -190,9 +190,9 @@ int skl_pcm_link_dma_prepare(struct device *dev, struct skl_pipe_params *params) dev_dbg(dev, "format_val=%d, rate=%d, ch=%d, format=%d\n", format_val, params->s_freq, params->ch, params->format);
- snd_hdac_ext_link_stream_reset(stream); + snd_hdac_ext_stream_reset(stream);
- snd_hdac_ext_link_stream_setup(stream, format_val); + snd_hdac_ext_stream_setup(stream, format_val);
stream_tag = hstream->stream_tag; if (stream->hstream.direction == SNDRV_PCM_STREAM_PLAYBACK) { @@ -612,13 +612,13 @@ static int skl_link_pcm_trigger(struct snd_pcm_substream *substream, case SNDRV_PCM_TRIGGER_RESUME: case SNDRV_PCM_TRIGGER_START: case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: - snd_hdac_ext_link_stream_start(link_dev); + snd_hdac_ext_stream_start(link_dev); break;
case SNDRV_PCM_TRIGGER_PAUSE_PUSH: case SNDRV_PCM_TRIGGER_SUSPEND: case SNDRV_PCM_TRIGGER_STOP: - snd_hdac_ext_link_stream_clear(link_dev); + snd_hdac_ext_stream_clear(link_dev); if (cmd == SNDRV_PCM_TRIGGER_SUSPEND) snd_hdac_ext_stream_decouple(bus, stream, false); break; diff --git a/sound/soc/sof/intel/hda-dai.c b/sound/soc/sof/intel/hda-dai.c index b71bb29e764b3..575adf3133c27 100644 --- a/sound/soc/sof/intel/hda-dai.c +++ b/sound/soc/sof/intel/hda-dai.c @@ -154,7 +154,7 @@ static int hda_link_dma_cleanup(struct snd_pcm_substream *substream, return -EINVAL;
if (trigger_suspend_stop) - snd_hdac_ext_link_stream_clear(hext_stream); + snd_hdac_ext_stream_clear(hext_stream);
if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { stream_tag = hdac_stream(hext_stream)->stream_tag; @@ -180,7 +180,7 @@ static int hda_link_dma_params(struct hdac_ext_stream *hext_stream, struct hdac_ext_link *hlink; unsigned int format_val;
- snd_hdac_ext_link_stream_reset(hext_stream); + snd_hdac_ext_stream_reset(hext_stream);
format_val = snd_hdac_calc_stream_format(params->s_freq, params->ch, params->format, @@ -189,7 +189,7 @@ static int hda_link_dma_params(struct hdac_ext_stream *hext_stream, dev_dbg(bus->dev, "format_val=%d, rate=%d, ch=%d, format=%d\n", format_val, params->s_freq, params->ch, params->format);
- snd_hdac_ext_link_stream_setup(hext_stream, format_val); + snd_hdac_ext_stream_setup(hext_stream, format_val);
if (hext_stream->hstream.direction == SNDRV_PCM_STREAM_PLAYBACK) { list_for_each_entry(hlink, &bus->hlink_list, list) { @@ -270,7 +270,7 @@ static int hda_link_dma_trigger(struct snd_pcm_substream *substream, int cmd) switch (cmd) { case SNDRV_PCM_TRIGGER_START: case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: - snd_hdac_ext_link_stream_start(hext_stream); + snd_hdac_ext_stream_start(hext_stream); break; case SNDRV_PCM_TRIGGER_SUSPEND: case SNDRV_PCM_TRIGGER_STOP: @@ -280,7 +280,7 @@ static int hda_link_dma_trigger(struct snd_pcm_substream *substream, int cmd)
break; case SNDRV_PCM_TRIGGER_PAUSE_PUSH: - snd_hdac_ext_link_stream_clear(hext_stream); + snd_hdac_ext_stream_clear(hext_stream);
break; default: @@ -476,7 +476,7 @@ static int ipc4_hda_dai_trigger(struct snd_pcm_substream *substream, switch (cmd) { case SNDRV_PCM_TRIGGER_START: case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: - snd_hdac_ext_link_stream_start(hext_stream); + snd_hdac_ext_stream_start(hext_stream); break; case SNDRV_PCM_TRIGGER_SUSPEND: case SNDRV_PCM_TRIGGER_STOP: @@ -491,7 +491,7 @@ static int ipc4_hda_dai_trigger(struct snd_pcm_substream *substream,
pipeline->state = SOF_IPC4_PIPE_PAUSED;
- snd_hdac_ext_link_stream_clear(hext_stream); + snd_hdac_ext_stream_clear(hext_stream);
ret = sof_ipc4_set_pipeline_state(sdev, swidget->pipeline_id, SOF_IPC4_PIPE_RESET); @@ -519,7 +519,7 @@ static int ipc4_hda_dai_trigger(struct snd_pcm_substream *substream,
pipeline->state = SOF_IPC4_PIPE_PAUSED;
- snd_hdac_ext_link_stream_clear(hext_stream); + snd_hdac_ext_stream_clear(hext_stream); break; } default:
On Wed, Oct 19, 2022 at 11:21:11AM -0500, Pierre-Louis Bossart wrote:
We should only use 'link' in the context of multi-link configurations. Streams are configured from a different register space and are not dependent on link except for LOSIDV settings.
Acked-by: Mark Brown broonie@kernel.org
All the helpers dealing with multi-link configurations are located in the hdac_ext_controller.c, except the two set/clear routines that modify the LOSIDV registers.
For consistency, move the two helpers and add the 'bus' prefix. One could argue that the 'ml' prefix might be more relevant but that would be a larger code change.
No functionality change, just move and rename.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Bard Liao yung-chuan.liao@linux.intel.com Reviewed-by: Péter Ujfalusi peter.ujfalusi@linux.intel.com --- include/sound/hdaudio_ext.h | 8 ++++---- sound/hda/ext/hdac_ext_controller.c | 24 ++++++++++++++++++++++++ sound/hda/ext/hdac_ext_stream.c | 24 ------------------------ sound/soc/intel/avs/pcm.c | 4 ++-- sound/soc/intel/skylake/skl-pcm.c | 6 +++--- sound/soc/sof/intel/hda-dai.c | 6 +++--- 6 files changed, 36 insertions(+), 36 deletions(-)
diff --git a/include/sound/hdaudio_ext.h b/include/sound/hdaudio_ext.h index 472fed072e985..79aea619adda4 100644 --- a/include/sound/hdaudio_ext.h +++ b/include/sound/hdaudio_ext.h @@ -121,10 +121,10 @@ int snd_hdac_ext_bus_link_power_up(struct hdac_ext_link *hlink); int snd_hdac_ext_bus_link_power_down(struct hdac_ext_link *hlink); int snd_hdac_ext_bus_link_power_up_all(struct hdac_bus *bus); int snd_hdac_ext_bus_link_power_down_all(struct hdac_bus *bus); -void snd_hdac_ext_link_set_stream_id(struct hdac_ext_link *hlink, - int stream); -void snd_hdac_ext_link_clear_stream_id(struct hdac_ext_link *hlink, - int stream); +void snd_hdac_ext_bus_link_set_stream_id(struct hdac_ext_link *hlink, + int stream); +void snd_hdac_ext_bus_link_clear_stream_id(struct hdac_ext_link *hlink, + int stream);
int snd_hdac_ext_bus_link_get(struct hdac_bus *bus, struct hdac_ext_link *hlink); int snd_hdac_ext_bus_link_put(struct hdac_bus *bus, struct hdac_ext_link *hlink); diff --git a/sound/hda/ext/hdac_ext_controller.c b/sound/hda/ext/hdac_ext_controller.c index c0b801fa93c09..08d3313f2df7e 100644 --- a/sound/hda/ext/hdac_ext_controller.c +++ b/sound/hda/ext/hdac_ext_controller.c @@ -253,6 +253,30 @@ int snd_hdac_ext_bus_link_power_down_all(struct hdac_bus *bus) } EXPORT_SYMBOL_GPL(snd_hdac_ext_bus_link_power_down_all);
+/** + * snd_hdac_ext_bus_link_set_stream_id - maps stream id to link output + * @link: HD-audio ext link to set up + * @stream: stream id + */ +void snd_hdac_ext_bus_link_set_stream_id(struct hdac_ext_link *link, + int stream) +{ + snd_hdac_updatew(link->ml_addr, AZX_REG_ML_LOSIDV, (1 << stream), 1 << stream); +} +EXPORT_SYMBOL_GPL(snd_hdac_ext_bus_link_set_stream_id); + +/** + * snd_hdac_ext_bus_link_clear_stream_id - maps stream id to link output + * @link: HD-audio ext link to set up + * @stream: stream id + */ +void snd_hdac_ext_bus_link_clear_stream_id(struct hdac_ext_link *link, + int stream) +{ + snd_hdac_updatew(link->ml_addr, AZX_REG_ML_LOSIDV, (1 << stream), 0); +} +EXPORT_SYMBOL_GPL(snd_hdac_ext_bus_link_clear_stream_id); + int snd_hdac_ext_bus_link_get(struct hdac_bus *bus, struct hdac_ext_link *hlink) { diff --git a/sound/hda/ext/hdac_ext_stream.c b/sound/hda/ext/hdac_ext_stream.c index c06beaaffe0a1..da2a9b54a5bea 100644 --- a/sound/hda/ext/hdac_ext_stream.c +++ b/sound/hda/ext/hdac_ext_stream.c @@ -223,30 +223,6 @@ int snd_hdac_ext_stream_setup(struct hdac_ext_stream *hext_stream, int fmt) } EXPORT_SYMBOL_GPL(snd_hdac_ext_stream_setup);
-/** - * snd_hdac_ext_link_set_stream_id - maps stream id to link output - * @link: HD-audio ext link to set up - * @stream: stream id - */ -void snd_hdac_ext_link_set_stream_id(struct hdac_ext_link *link, - int stream) -{ - snd_hdac_updatew(link->ml_addr, AZX_REG_ML_LOSIDV, (1 << stream), 1 << stream); -} -EXPORT_SYMBOL_GPL(snd_hdac_ext_link_set_stream_id); - -/** - * snd_hdac_ext_link_clear_stream_id - maps stream id to link output - * @link: HD-audio ext link to set up - * @stream: stream id - */ -void snd_hdac_ext_link_clear_stream_id(struct hdac_ext_link *link, - int stream) -{ - snd_hdac_updatew(link->ml_addr, AZX_REG_ML_LOSIDV, (1 << stream), 0); -} -EXPORT_SYMBOL_GPL(snd_hdac_ext_link_clear_stream_id); - static struct hdac_ext_stream * hdac_ext_link_stream_assign(struct hdac_bus *bus, struct snd_pcm_substream *substream) diff --git a/sound/soc/intel/avs/pcm.c b/sound/soc/intel/avs/pcm.c index d77afe5c65b96..95cb87339400b 100644 --- a/sound/soc/intel/avs/pcm.c +++ b/sound/soc/intel/avs/pcm.c @@ -297,7 +297,7 @@ static int avs_dai_hda_be_hw_free(struct snd_pcm_substream *substream, struct sn return -EINVAL;
if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) - snd_hdac_ext_link_clear_stream_id(link, hdac_stream(link_stream)->stream_tag); + snd_hdac_ext_bus_link_clear_stream_id(link, hdac_stream(link_stream)->stream_tag);
return 0; } @@ -330,7 +330,7 @@ static int avs_dai_hda_be_prepare(struct snd_pcm_substream *substream, struct sn return -EINVAL;
if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) - snd_hdac_ext_link_set_stream_id(link, hdac_stream(link_stream)->stream_tag); + snd_hdac_ext_bus_link_set_stream_id(link, hdac_stream(link_stream)->stream_tag);
ret = avs_dai_prepare(to_avs_dev(dai->dev), substream, dai); if (ret) diff --git a/sound/soc/intel/skylake/skl-pcm.c b/sound/soc/intel/skylake/skl-pcm.c index f7e97b5a449fc..27b03c34abd09 100644 --- a/sound/soc/intel/skylake/skl-pcm.c +++ b/sound/soc/intel/skylake/skl-pcm.c @@ -198,8 +198,8 @@ int skl_pcm_link_dma_prepare(struct device *dev, struct skl_pipe_params *params) if (stream->hstream.direction == SNDRV_PCM_STREAM_PLAYBACK) { list_for_each_entry(link, &bus->hlink_list, list) { if (link->index == params->link_index) - snd_hdac_ext_link_set_stream_id(link, - stream_tag); + snd_hdac_ext_bus_link_set_stream_id(link, + stream_tag); } }
@@ -649,7 +649,7 @@ static int skl_link_hw_free(struct snd_pcm_substream *substream,
if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { stream_tag = hdac_stream(link_dev)->stream_tag; - snd_hdac_ext_link_clear_stream_id(link, stream_tag); + snd_hdac_ext_bus_link_clear_stream_id(link, stream_tag); }
snd_hdac_ext_stream_release(link_dev, HDAC_EXT_STREAM_TYPE_LINK); diff --git a/sound/soc/sof/intel/hda-dai.c b/sound/soc/sof/intel/hda-dai.c index 575adf3133c27..64e8ca016b216 100644 --- a/sound/soc/sof/intel/hda-dai.c +++ b/sound/soc/sof/intel/hda-dai.c @@ -158,7 +158,7 @@ static int hda_link_dma_cleanup(struct snd_pcm_substream *substream,
if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { stream_tag = hdac_stream(hext_stream)->stream_tag; - snd_hdac_ext_link_clear_stream_id(hlink, stream_tag); + snd_hdac_ext_bus_link_clear_stream_id(hlink, stream_tag); } snd_soc_dai_set_dma_data(cpu_dai, substream, NULL); snd_hdac_ext_stream_release(hext_stream, HDAC_EXT_STREAM_TYPE_LINK); @@ -194,8 +194,8 @@ static int hda_link_dma_params(struct hdac_ext_stream *hext_stream, if (hext_stream->hstream.direction == SNDRV_PCM_STREAM_PLAYBACK) { list_for_each_entry(hlink, &bus->hlink_list, list) { if (hlink->index == params->link_index) - snd_hdac_ext_link_set_stream_id(hlink, - stream_tag); + snd_hdac_ext_bus_link_set_stream_id(hlink, + stream_tag); } }
On Wed, Oct 19, 2022 at 11:21:12AM -0500, Pierre-Louis Bossart wrote:
All the helpers dealing with multi-link configurations are located in the hdac_ext_controller.c, except the two set/clear routines that modify the LOSIDV registers.
Acked-by: Mark Brown broonie@kernel.org
My esteemed colleagues keep using the same words for different things.
The multi-link structure needs to be handled whether the DSP is enabled or not.
The host and link DMAs are only relevant when the DSP is enabled.
Things get convoluted when there's an ambiguity between the LOSIDV settings in the multi-link register space and the selection of the stream_tag for the link DMA.
Clarify with a rename that the static functions used are related to the host and link DMAs only.
No functionality change, pure rename.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Bard Liao yung-chuan.liao@linux.intel.com Reviewed-by: Péter Ujfalusi peter.ujfalusi@linux.intel.com --- sound/hda/ext/hdac_ext_stream.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/sound/hda/ext/hdac_ext_stream.c b/sound/hda/ext/hdac_ext_stream.c index da2a9b54a5bea..b36378bf6da6c 100644 --- a/sound/hda/ext/hdac_ext_stream.c +++ b/sound/hda/ext/hdac_ext_stream.c @@ -224,8 +224,8 @@ int snd_hdac_ext_stream_setup(struct hdac_ext_stream *hext_stream, int fmt) EXPORT_SYMBOL_GPL(snd_hdac_ext_stream_setup);
static struct hdac_ext_stream * -hdac_ext_link_stream_assign(struct hdac_bus *bus, - struct snd_pcm_substream *substream) +hdac_ext_link_dma_stream_assign(struct hdac_bus *bus, + struct snd_pcm_substream *substream) { struct hdac_ext_stream *res = NULL; struct hdac_stream *hstream = NULL; @@ -260,8 +260,8 @@ 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_dma_stream_assign(struct hdac_bus *bus, + struct snd_pcm_substream *substream) { struct hdac_ext_stream *res = NULL; struct hdac_stream *hstream = NULL; @@ -329,10 +329,10 @@ struct hdac_ext_stream *snd_hdac_ext_stream_assign(struct hdac_bus *bus, return hext_stream;
case HDAC_EXT_STREAM_TYPE_HOST: - return hdac_ext_host_stream_assign(bus, substream); + return hdac_ext_host_dma_stream_assign(bus, substream);
case HDAC_EXT_STREAM_TYPE_LINK: - return hdac_ext_link_stream_assign(bus, substream); + return hdac_ext_link_dma_stream_assign(bus, substream);
default: return NULL;
commit 0b00a5615dc40 ("ALSA: hdac_ext: add hdac extended controller") introduced a for() loop on the number of HDaudio codecs that seems completely useless.
a) the body of the loop does not make use of the loop index, and b) the LSDIID register is related to the SDI line, so there can only be one codec per multi-link descriptor.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Bard Liao yung-chuan.liao@linux.intel.com Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com --- sound/hda/ext/hdac_ext_controller.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/sound/hda/ext/hdac_ext_controller.c b/sound/hda/ext/hdac_ext_controller.c index 08d3313f2df7e..6199bb60ccf00 100644 --- a/sound/hda/ext/hdac_ext_controller.c +++ b/sound/hda/ext/hdac_ext_controller.c @@ -135,12 +135,10 @@ EXPORT_SYMBOL_GPL(snd_hdac_ext_link_free_all); struct hdac_ext_link *snd_hdac_ext_bus_get_hlink_by_addr(struct hdac_bus *bus, int addr) { struct hdac_ext_link *hlink; - int i;
list_for_each_entry(hlink, &bus->hlink_list, list) - for (i = 0; i < HDA_MAX_CODECS; i++) - if (hlink->lsdiid & (0x1 << addr)) - return hlink; + if (hlink->lsdiid & (0x1 << addr)) + return hlink; return NULL; } EXPORT_SYMBOL_GPL(snd_hdac_ext_bus_get_hlink_by_addr);
The SPIB and DRMS capabilities are orthogonal to the DSP enablement and can be used whether the stream is coupled or not.
The existing code partitioning makes limited sense, the capabilities are parsed at the sound/hda level but helpers are located in sound/hda/ext.
This patch moves all the SPIB/DRMS functionality to the sound/hda layer. This reduces the complexity of the sound/hda/ext layer which is now limited to handling the multi-link extensions and stream coupling/decoupling helpers.
Note that this is an iso-functionality code move and rename, the HDaudio legacy driver would need additional changes to make use of these capabilities.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Reviewed-by: Bard Liao yung-chuan.liao@linux.intel.com --- include/sound/hdaudio.h | 26 +++++ include/sound/hdaudio_ext.h | 34 ------ sound/hda/ext/hdac_ext_stream.c | 139 ------------------------- sound/hda/hdac_stream.c | 136 ++++++++++++++++++++++++ sound/soc/intel/avs/loader.c | 16 +-- sound/soc/intel/skylake/skl-messages.c | 6 +- sound/soc/intel/skylake/skl-pcm.c | 17 ++- sound/soc/sof/intel/hda-pcm.c | 3 +- sound/soc/sof/intel/hda-stream.c | 16 +-- 9 files changed, 190 insertions(+), 203 deletions(-)
diff --git a/include/sound/hdaudio.h b/include/sound/hdaudio.h index ddff03e546e9f..6be0fd37cd35c 100644 --- a/include/sound/hdaudio.h +++ b/include/sound/hdaudio.h @@ -495,6 +495,13 @@ static inline u16 snd_hdac_reg_readw(struct hdac_bus *bus, void __iomem *addr) snd_hdac_chip_writeb(chip, reg, \ (snd_hdac_chip_readb(chip, reg) & ~(mask)) | (val))
+/* update register macro */ +#define snd_hdac_updatel(addr, reg, mask, val) \ + writel(((readl(addr + reg) & ~(mask)) | (val)), addr + reg) + +#define snd_hdac_updatew(addr, reg, mask, val) \ + writew(((readw(addr + reg) & ~(mask)) | (val)), addr + reg) + /* * HD-audio stream */ @@ -511,6 +518,13 @@ struct hdac_stream {
void __iomem *sd_addr; /* stream descriptor pointer */
+ void __iomem *spib_addr; /* software position in buffers stream pointer */ + void __iomem *fifo_addr; /* software position Max fifos stream pointer */ + + void __iomem *dpibr_addr; /* DMA position in buffer resume pointer */ + u32 dpib; /* DMA position in buffer */ + u32 lpib; /* Linear position in buffer */ + u32 sd_int_sta_mask; /* stream int status mask */
/* pcm support */ @@ -575,6 +589,18 @@ void snd_hdac_stream_timecounter_init(struct hdac_stream *azx_dev, int snd_hdac_get_stream_stripe_ctl(struct hdac_bus *bus, struct snd_pcm_substream *substream);
+void snd_hdac_stream_spbcap_enable(struct hdac_bus *chip, + bool enable, int index); +int snd_hdac_stream_set_spib(struct hdac_bus *bus, + struct hdac_stream *azx_dev, u32 value); +int snd_hdac_stream_get_spbmaxfifo(struct hdac_bus *bus, + struct hdac_stream *azx_dev); +void snd_hdac_stream_drsm_enable(struct hdac_bus *bus, + bool enable, int index); +int snd_hdac_stream_set_dpibr(struct hdac_bus *bus, + struct hdac_stream *azx_dev, u32 value); +int snd_hdac_stream_set_lpib(struct hdac_stream *azx_dev, u32 value); + /* * macros for easy use */ diff --git a/include/sound/hdaudio_ext.h b/include/sound/hdaudio_ext.h index 79aea619adda4..90fd47e053703 100644 --- a/include/sound/hdaudio_ext.h +++ b/include/sound/hdaudio_ext.h @@ -23,9 +23,6 @@ void snd_hdac_ext_bus_device_remove(struct hdac_bus *bus); void snd_hdac_ext_bus_ppcap_enable(struct hdac_bus *chip, bool enable); void snd_hdac_ext_bus_ppcap_int_enable(struct hdac_bus *chip, bool enable);
-void snd_hdac_ext_stream_spbcap_enable(struct hdac_bus *chip, - bool enable, int index); - int snd_hdac_ext_bus_get_ml_capabilities(struct hdac_bus *bus); struct hdac_ext_link *snd_hdac_ext_bus_get_hlink_by_addr(struct hdac_bus *bus, int addr); struct hdac_ext_link *snd_hdac_ext_bus_get_hlink_by_name(struct hdac_bus *bus, @@ -43,11 +40,6 @@ enum hdac_ext_stream_type { * @hstream: hdac_stream * @pphc_addr: processing pipe host stream pointer * @pplc_addr: processing pipe link stream pointer - * @spib_addr: software position in buffers stream pointer - * @fifo_addr: software position Max fifos stream pointer - * @dpibr_addr: DMA position in buffer resume pointer - * @dpib: DMA position in buffer - * @lpib: Linear position in buffer * @decoupled: stream host and link is decoupled * @link_locked: link is locked * @link_prepared: link is prepared @@ -59,13 +51,6 @@ struct hdac_ext_stream { void __iomem *pphc_addr; void __iomem *pplc_addr;
- void __iomem *spib_addr; - void __iomem *fifo_addr; - - void __iomem *dpibr_addr; - - u32 dpib; - u32 lpib; bool decoupled:1; bool link_locked:1; bool link_prepared; @@ -90,16 +75,6 @@ void snd_hdac_ext_stream_decouple_locked(struct hdac_bus *bus, void snd_hdac_ext_stream_decouple(struct hdac_bus *bus, struct hdac_ext_stream *azx_dev, bool decouple);
-int snd_hdac_ext_stream_set_spib(struct hdac_bus *bus, - struct hdac_ext_stream *hext_stream, u32 value); -int snd_hdac_ext_stream_get_spbmaxfifo(struct hdac_bus *bus, - struct hdac_ext_stream *hext_stream); -void snd_hdac_ext_stream_drsm_enable(struct hdac_bus *bus, - bool enable, int index); -int snd_hdac_ext_stream_set_dpibr(struct hdac_bus *bus, - struct hdac_ext_stream *hext_stream, u32 value); -int snd_hdac_ext_stream_set_lpib(struct hdac_ext_stream *hext_stream, u32 value); - void snd_hdac_ext_stream_start(struct hdac_ext_stream *hext_stream); void snd_hdac_ext_stream_clear(struct hdac_ext_stream *hext_stream); void snd_hdac_ext_stream_reset(struct hdac_ext_stream *hext_stream); @@ -131,15 +106,6 @@ int snd_hdac_ext_bus_link_put(struct hdac_bus *bus, struct hdac_ext_link *hlink)
void snd_hdac_ext_bus_link_power(struct hdac_device *codec, bool enable);
-/* update register macro */ -#define snd_hdac_updatel(addr, reg, mask, val) \ - writel(((readl(addr + reg) & ~(mask)) | (val)), \ - addr + reg) - -#define snd_hdac_updatew(addr, reg, mask, val) \ - writew(((readw(addr + reg) & ~(mask)) | (val)), \ - addr + reg) - #define snd_hdac_adsp_writeb(chip, reg, value) \ snd_hdac_reg_writeb(chip, (chip)->dsp_ba + (reg), value) #define snd_hdac_adsp_readb(chip, reg) \ diff --git a/sound/hda/ext/hdac_ext_stream.c b/sound/hda/ext/hdac_ext_stream.c index b36378bf6da6c..2a071a09224de 100644 --- a/sound/hda/ext/hdac_ext_stream.c +++ b/sound/hda/ext/hdac_ext_stream.c @@ -39,20 +39,6 @@ static void snd_hdac_ext_stream_init(struct hdac_bus *bus, AZX_PPLC_INTERVAL * idx; }
- if (bus->spbcap) { - hext_stream->spib_addr = bus->spbcap + AZX_SPB_BASE + - AZX_SPB_INTERVAL * idx + - AZX_SPB_SPIB; - - hext_stream->fifo_addr = bus->spbcap + AZX_SPB_BASE + - AZX_SPB_INTERVAL * idx + - AZX_SPB_MAXFIFO; - } - - if (bus->drsmcap) - hext_stream->dpibr_addr = bus->drsmcap + AZX_DRSM_BASE + - AZX_DRSM_INTERVAL * idx; - hext_stream->decoupled = false; snd_hdac_stream_init(bus, &hext_stream->hstream, idx, direction, tag); } @@ -381,128 +367,3 @@ void snd_hdac_ext_stream_release(struct hdac_ext_stream *hext_stream, int type)
} EXPORT_SYMBOL_GPL(snd_hdac_ext_stream_release); - -/** - * snd_hdac_ext_stream_spbcap_enable - enable SPIB for a stream - * @bus: HD-audio core bus - * @enable: flag to enable/disable SPIB - * @index: stream index for which SPIB need to be enabled - */ -void snd_hdac_ext_stream_spbcap_enable(struct hdac_bus *bus, - bool enable, int index) -{ - u32 mask = 0; - - if (!bus->spbcap) { - dev_err(bus->dev, "Address of SPB capability is NULL\n"); - return; - } - - mask |= (1 << index); - - if (enable) - snd_hdac_updatel(bus->spbcap, AZX_REG_SPB_SPBFCCTL, mask, mask); - else - snd_hdac_updatel(bus->spbcap, AZX_REG_SPB_SPBFCCTL, mask, 0); -} -EXPORT_SYMBOL_GPL(snd_hdac_ext_stream_spbcap_enable); - -/** - * snd_hdac_ext_stream_set_spib - sets the spib value of a stream - * @bus: HD-audio core bus - * @hext_stream: hdac_ext_stream - * @value: spib value to set - */ -int snd_hdac_ext_stream_set_spib(struct hdac_bus *bus, - struct hdac_ext_stream *hext_stream, u32 value) -{ - - if (!bus->spbcap) { - dev_err(bus->dev, "Address of SPB capability is NULL\n"); - return -EINVAL; - } - - writel(value, hext_stream->spib_addr); - - return 0; -} -EXPORT_SYMBOL_GPL(snd_hdac_ext_stream_set_spib); - -/** - * snd_hdac_ext_stream_get_spbmaxfifo - gets the spib value of a stream - * @bus: HD-audio core bus - * @hext_stream: hdac_ext_stream - * - * Return maxfifo for the stream - */ -int snd_hdac_ext_stream_get_spbmaxfifo(struct hdac_bus *bus, - struct hdac_ext_stream *hext_stream) -{ - - if (!bus->spbcap) { - dev_err(bus->dev, "Address of SPB capability is NULL\n"); - return -EINVAL; - } - - return readl(hext_stream->fifo_addr); -} -EXPORT_SYMBOL_GPL(snd_hdac_ext_stream_get_spbmaxfifo); - -/** - * snd_hdac_ext_stream_drsm_enable - enable DMA resume for a stream - * @bus: HD-audio core bus - * @enable: flag to enable/disable DRSM - * @index: stream index for which DRSM need to be enabled - */ -void snd_hdac_ext_stream_drsm_enable(struct hdac_bus *bus, - bool enable, int index) -{ - u32 mask = 0; - - if (!bus->drsmcap) { - dev_err(bus->dev, "Address of DRSM capability is NULL\n"); - return; - } - - mask |= (1 << index); - - if (enable) - snd_hdac_updatel(bus->drsmcap, AZX_REG_DRSM_CTL, mask, mask); - else - snd_hdac_updatel(bus->drsmcap, AZX_REG_DRSM_CTL, mask, 0); -} -EXPORT_SYMBOL_GPL(snd_hdac_ext_stream_drsm_enable); - -/** - * snd_hdac_ext_stream_set_dpibr - sets the dpibr value of a stream - * @bus: HD-audio core bus - * @hext_stream: hdac_ext_stream - * @value: dpib value to set - */ -int snd_hdac_ext_stream_set_dpibr(struct hdac_bus *bus, - struct hdac_ext_stream *hext_stream, u32 value) -{ - - if (!bus->drsmcap) { - dev_err(bus->dev, "Address of DRSM capability is NULL\n"); - return -EINVAL; - } - - writel(value, hext_stream->dpibr_addr); - - return 0; -} -EXPORT_SYMBOL_GPL(snd_hdac_ext_stream_set_dpibr); - -/** - * snd_hdac_ext_stream_set_lpib - sets the lpib value of a stream - * @hext_stream: hdac_ext_stream - * @value: lpib value to set - */ -int snd_hdac_ext_stream_set_lpib(struct hdac_ext_stream *hext_stream, u32 value) -{ - snd_hdac_stream_writel(&hext_stream->hstream, SD_LPIB, value); - - return 0; -} -EXPORT_SYMBOL_GPL(snd_hdac_ext_stream_set_lpib); diff --git a/sound/hda/hdac_stream.c b/sound/hda/hdac_stream.c index 1b8be39c38a96..35fe2bd582aca 100644 --- a/sound/hda/hdac_stream.c +++ b/sound/hda/hdac_stream.c @@ -103,6 +103,20 @@ void snd_hdac_stream_init(struct hdac_bus *bus, struct hdac_stream *azx_dev, azx_dev->stream_tag = tag; snd_hdac_dsp_lock_init(azx_dev); list_add_tail(&azx_dev->list, &bus->stream_list); + + if (bus->spbcap) { + azx_dev->spib_addr = bus->spbcap + AZX_SPB_BASE + + AZX_SPB_INTERVAL * idx + + AZX_SPB_SPIB; + + azx_dev->fifo_addr = bus->spbcap + AZX_SPB_BASE + + AZX_SPB_INTERVAL * idx + + AZX_SPB_MAXFIFO; + } + + if (bus->drsmcap) + azx_dev->dpibr_addr = bus->drsmcap + AZX_DRSM_BASE + + AZX_DRSM_INTERVAL * idx; } EXPORT_SYMBOL_GPL(snd_hdac_stream_init);
@@ -718,6 +732,128 @@ void snd_hdac_stream_sync(struct hdac_stream *azx_dev, bool start, } EXPORT_SYMBOL_GPL(snd_hdac_stream_sync);
+/** + * snd_hdac_stream_spbcap_enable - enable SPIB for a stream + * @bus: HD-audio core bus + * @enable: flag to enable/disable SPIB + * @index: stream index for which SPIB need to be enabled + */ +void snd_hdac_stream_spbcap_enable(struct hdac_bus *bus, + bool enable, int index) +{ + u32 mask = 0; + + if (!bus->spbcap) { + dev_err(bus->dev, "Address of SPB capability is NULL\n"); + return; + } + + mask |= (1 << index); + + if (enable) + snd_hdac_updatel(bus->spbcap, AZX_REG_SPB_SPBFCCTL, mask, mask); + else + snd_hdac_updatel(bus->spbcap, AZX_REG_SPB_SPBFCCTL, mask, 0); +} +EXPORT_SYMBOL_GPL(snd_hdac_stream_spbcap_enable); + +/** + * snd_hdac_stream_set_spib - sets the spib value of a stream + * @bus: HD-audio core bus + * @azx_dev: hdac_stream + * @value: spib value to set + */ +int snd_hdac_stream_set_spib(struct hdac_bus *bus, + struct hdac_stream *azx_dev, u32 value) +{ + if (!bus->spbcap) { + dev_err(bus->dev, "Address of SPB capability is NULL\n"); + return -EINVAL; + } + + writel(value, azx_dev->spib_addr); + + return 0; +} +EXPORT_SYMBOL_GPL(snd_hdac_stream_set_spib); + +/** + * snd_hdac_stream_get_spbmaxfifo - gets the spib value of a stream + * @bus: HD-audio core bus + * @azx_dev: hdac_stream + * + * Return maxfifo for the stream + */ +int snd_hdac_stream_get_spbmaxfifo(struct hdac_bus *bus, + struct hdac_stream *azx_dev) +{ + if (!bus->spbcap) { + dev_err(bus->dev, "Address of SPB capability is NULL\n"); + return -EINVAL; + } + + return readl(azx_dev->fifo_addr); +} +EXPORT_SYMBOL_GPL(snd_hdac_stream_get_spbmaxfifo); + +/** + * snd_hdac_stream_drsm_enable - enable DMA resume for a stream + * @bus: HD-audio core bus + * @enable: flag to enable/disable DRSM + * @index: stream index for which DRSM need to be enabled + */ +void snd_hdac_stream_drsm_enable(struct hdac_bus *bus, + bool enable, int index) +{ + u32 mask = 0; + + if (!bus->drsmcap) { + dev_err(bus->dev, "Address of DRSM capability is NULL\n"); + return; + } + + mask |= (1 << index); + + if (enable) + snd_hdac_updatel(bus->drsmcap, AZX_REG_DRSM_CTL, mask, mask); + else + snd_hdac_updatel(bus->drsmcap, AZX_REG_DRSM_CTL, mask, 0); +} +EXPORT_SYMBOL_GPL(snd_hdac_stream_drsm_enable); + +/** + * snd_hdac_stream_set_dpibr - sets the dpibr value of a stream + * @bus: HD-audio core bus + * @azx_dev: hdac_stream + * @value: dpib value to set + */ +int snd_hdac_stream_set_dpibr(struct hdac_bus *bus, + struct hdac_stream *azx_dev, u32 value) +{ + if (!bus->drsmcap) { + dev_err(bus->dev, "Address of DRSM capability is NULL\n"); + return -EINVAL; + } + + writel(value, azx_dev->dpibr_addr); + + return 0; +} +EXPORT_SYMBOL_GPL(snd_hdac_stream_set_dpibr); + +/** + * snd_hdac_stream_set_lpib - sets the lpib value of a stream + * @azx_dev: hdac_stream + * @value: lpib value to set + */ +int snd_hdac_stream_set_lpib(struct hdac_stream *azx_dev, u32 value) +{ + snd_hdac_stream_writel(azx_dev, SD_LPIB, value); + + return 0; +} +EXPORT_SYMBOL_GPL(snd_hdac_stream_set_lpib); + #ifdef CONFIG_SND_HDA_DSP_LOADER /** * snd_hdac_dsp_prepare - prepare for DSP loading diff --git a/sound/soc/intel/avs/loader.c b/sound/soc/intel/avs/loader.c index 9e3f8ff33a87a..34923558dfa57 100644 --- a/sound/soc/intel/avs/loader.c +++ b/sound/soc/intel/avs/loader.c @@ -369,8 +369,8 @@ int avs_hda_load_basefw(struct avs_dev *adev, struct firmware *fw) goto release_stream;
/* enable SPIB for hda stream */ - snd_hdac_ext_stream_spbcap_enable(bus, true, hstream->index); - ret = snd_hdac_ext_stream_set_spib(bus, estream, fw->size); + snd_hdac_stream_spbcap_enable(bus, true, hstream->index); + ret = snd_hdac_stream_set_spib(bus, hstream, fw->size); if (ret) goto cleanup_resources;
@@ -400,8 +400,8 @@ int avs_hda_load_basefw(struct avs_dev *adev, struct firmware *fw)
cleanup_resources: /* disable SPIB for hda stream */ - snd_hdac_ext_stream_spbcap_enable(bus, false, hstream->index); - snd_hdac_ext_stream_set_spib(bus, estream, 0); + snd_hdac_stream_spbcap_enable(bus, false, hstream->index); + snd_hdac_stream_set_spib(bus, hstream, 0);
snd_hdac_dsp_cleanup(hstream, &dmab); release_stream: @@ -436,8 +436,8 @@ int avs_hda_load_library(struct avs_dev *adev, struct firmware *lib, u32 id) goto release_stream;
/* enable SPIB for hda stream */ - snd_hdac_ext_stream_spbcap_enable(bus, true, stream->index); - snd_hdac_ext_stream_set_spib(bus, estream, lib->size); + snd_hdac_stream_spbcap_enable(bus, true, stream->index); + snd_hdac_stream_set_spib(bus, stream, lib->size);
memcpy(dmab.area, lib->data, lib->size);
@@ -451,8 +451,8 @@ int avs_hda_load_library(struct avs_dev *adev, struct firmware *lib, u32 id) }
/* disable SPIB for hda stream */ - snd_hdac_ext_stream_spbcap_enable(bus, false, stream->index); - snd_hdac_ext_stream_set_spib(bus, estream, 0); + snd_hdac_stream_spbcap_enable(bus, false, stream->index); + snd_hdac_stream_set_spib(bus, stream, 0);
snd_hdac_dsp_cleanup(stream, &dmab); release_stream: diff --git a/sound/soc/intel/skylake/skl-messages.c b/sound/soc/intel/skylake/skl-messages.c index eaad180af42ed..5ab0917a2b3de 100644 --- a/sound/soc/intel/skylake/skl-messages.c +++ b/sound/soc/intel/skylake/skl-messages.c @@ -53,17 +53,15 @@ static int skl_dsp_setup_spib(struct device *dev, unsigned int size, struct hdac_bus *bus = dev_get_drvdata(dev); struct hdac_stream *stream = snd_hdac_get_stream(bus, SNDRV_PCM_STREAM_PLAYBACK, stream_tag); - struct hdac_ext_stream *estream;
if (!stream) return -EINVAL;
- estream = stream_to_hdac_ext_stream(stream); /* enable/disable SPIB for this hdac stream */ - snd_hdac_ext_stream_spbcap_enable(bus, enable, stream->index); + snd_hdac_stream_spbcap_enable(bus, enable, stream->index);
/* set the spib value */ - snd_hdac_ext_stream_set_spib(bus, estream, size); + snd_hdac_stream_set_spib(bus, stream, size);
return 0; } diff --git a/sound/soc/intel/skylake/skl-pcm.c b/sound/soc/intel/skylake/skl-pcm.c index 27b03c34abd09..dc627d18518d8 100644 --- a/sound/soc/intel/skylake/skl-pcm.c +++ b/sound/soc/intel/skylake/skl-pcm.c @@ -467,6 +467,7 @@ static int skl_pcm_trigger(struct snd_pcm_substream *substream, int cmd, struct skl_module_cfg *mconfig; struct hdac_bus *bus = get_bus_ctx(substream); struct hdac_ext_stream *stream = get_hdac_ext_stream(substream); + struct hdac_stream *hstream = hdac_stream(stream); struct snd_soc_dapm_widget *w; int ret;
@@ -484,11 +485,9 @@ static int skl_pcm_trigger(struct snd_pcm_substream *substream, int cmd, * dpib & lpib position to resume before starting the * DMA */ - snd_hdac_ext_stream_drsm_enable(bus, true, - hdac_stream(stream)->index); - snd_hdac_ext_stream_set_dpibr(bus, stream, - stream->lpib); - snd_hdac_ext_stream_set_lpib(stream, stream->lpib); + snd_hdac_stream_drsm_enable(bus, true, hstream->index); + snd_hdac_stream_set_dpibr(bus, hstream, hstream->lpib); + snd_hdac_stream_set_lpib(hstream, hstream->lpib); } fallthrough;
@@ -520,13 +519,13 @@ static int skl_pcm_trigger(struct snd_pcm_substream *substream, int cmd, ret = skl_decoupled_trigger(substream, cmd); if ((cmd == SNDRV_PCM_TRIGGER_SUSPEND) && !w->ignore_suspend) { /* save the dpib and lpib positions */ - stream->dpib = readl(bus->remap_addr + + hstream->dpib = readl(bus->remap_addr + AZX_REG_VS_SDXDPIB_XBASE + (AZX_REG_VS_SDXDPIB_XINTERVAL * - hdac_stream(stream)->index)); + hstream->index)); + + hstream->lpib = snd_hdac_stream_get_pos_lpib(hstream);
- stream->lpib = snd_hdac_stream_get_pos_lpib( - hdac_stream(stream)); snd_hdac_ext_stream_decouple(bus, stream, false); } break; diff --git a/sound/soc/sof/intel/hda-pcm.c b/sound/soc/sof/intel/hda-pcm.c index 0a9c80216a8c2..dc0b359ed9b6c 100644 --- a/sound/soc/sof/intel/hda-pcm.c +++ b/sound/soc/sof/intel/hda-pcm.c @@ -142,7 +142,6 @@ int hda_dsp_pcm_hw_params(struct snd_sof_dev *sdev, int hda_dsp_pcm_ack(struct snd_sof_dev *sdev, struct snd_pcm_substream *substream) { struct hdac_stream *hstream = substream->runtime->private_data; - struct hdac_ext_stream *hext_stream = stream_to_hdac_ext_stream(hstream); struct snd_pcm_runtime *runtime = substream->runtime; ssize_t appl_pos, buf_size; u32 spib; @@ -156,7 +155,7 @@ int hda_dsp_pcm_ack(struct snd_sof_dev *sdev, struct snd_pcm_substream *substrea if (!spib) spib = buf_size;
- sof_io_write(sdev, hext_stream->spib_addr, spib); + sof_io_write(sdev, hstream->spib_addr, spib);
return 0; } diff --git a/sound/soc/sof/intel/hda-stream.c b/sound/soc/sof/intel/hda-stream.c index be60e7785da94..8cb91788912cb 100644 --- a/sound/soc/sof/intel/hda-stream.c +++ b/sound/soc/sof/intel/hda-stream.c @@ -173,7 +173,7 @@ int hda_dsp_stream_spib_config(struct snd_sof_dev *sdev, enable << hstream->index);
/* set the SPIB value */ - sof_io_write(sdev, hext_stream->spib_addr, size); + sof_io_write(sdev, hstream->spib_addr, size);
return 0; } @@ -883,18 +883,19 @@ int hda_dsp_stream_init(struct snd_sof_dev *sdev) SOF_HDA_PPLC_BASE + SOF_HDA_PPLC_MULTI * num_total + SOF_HDA_PPLC_INTERVAL * i;
+ hstream = &hext_stream->hstream; + /* do we support SPIB */ if (sdev->bar[HDA_DSP_SPIB_BAR]) { - hext_stream->spib_addr = sdev->bar[HDA_DSP_SPIB_BAR] + + hstream->spib_addr = sdev->bar[HDA_DSP_SPIB_BAR] + SOF_HDA_SPIB_BASE + SOF_HDA_SPIB_INTERVAL * i + SOF_HDA_SPIB_SPIB;
- hext_stream->fifo_addr = sdev->bar[HDA_DSP_SPIB_BAR] + + hstream->fifo_addr = sdev->bar[HDA_DSP_SPIB_BAR] + SOF_HDA_SPIB_BASE + SOF_HDA_SPIB_INTERVAL * i + SOF_HDA_SPIB_MAXFIFO; }
- hstream = &hext_stream->hstream; hstream->bus = bus; hstream->sd_int_sta_mask = 1 << i; hstream->index = i; @@ -939,18 +940,19 @@ int hda_dsp_stream_init(struct snd_sof_dev *sdev) SOF_HDA_PPLC_BASE + SOF_HDA_PPLC_MULTI * num_total + SOF_HDA_PPLC_INTERVAL * i;
+ hstream = &hext_stream->hstream; + /* do we support SPIB */ if (sdev->bar[HDA_DSP_SPIB_BAR]) { - hext_stream->spib_addr = sdev->bar[HDA_DSP_SPIB_BAR] + + hstream->spib_addr = sdev->bar[HDA_DSP_SPIB_BAR] + SOF_HDA_SPIB_BASE + SOF_HDA_SPIB_INTERVAL * i + SOF_HDA_SPIB_SPIB;
- hext_stream->fifo_addr = sdev->bar[HDA_DSP_SPIB_BAR] + + hstream->fifo_addr = sdev->bar[HDA_DSP_SPIB_BAR] + SOF_HDA_SPIB_BASE + SOF_HDA_SPIB_INTERVAL * i + SOF_HDA_SPIB_MAXFIFO; }
- hstream = &hext_stream->hstream; hstream->bus = bus; hstream->sd_int_sta_mask = 1 << i; hstream->index = i;
On Wed, Oct 19, 2022 at 11:21:15AM -0500, Pierre-Louis Bossart wrote:
The SPIB and DRMS capabilities are orthogonal to the DSP enablement and can be used whether the stream is coupled or not.
The existing code partitioning makes limited sense, the capabilities are parsed at the sound/hda level but helpers are located in sound/hda/ext.
Acked-by: Mark Brown broonie@kernel.org
On Wed, 19 Oct 2022 18:21:05 +0200, Pierre-Louis Bossart wrote:
The code in sound/hda/ext can be cleaned-up. This patchset suggests a more consistent use of prefixes and variable names. It also removes useless code, uses helpers when possible, and clarifies an incorrect assumption that SPIB/DRMS capabilities are tied to Intel multi-link and DSP extensions (they are not).
This is a rather invasive set of changes touching sound/hda, sound/soc/intel/skylake, sound/soc/intel/avs and sound/soc/sof/intel, but there should be no change in terms of functionality.
I would recommend merging these changes through the ASoC tree.
Pierre-Louis Bossart (10): ALSA: hda: ext: hdac_ext_controller: use helpers in loop ALSA: hda: ext: hda_ext_controller: use hlink variable/parameter ASoC: SOF: Intel: hda-dai: use hlink variable/parameter ALSA/ASoC: hda: clarify bus_get_link() and bus_link_get() helpers ALSA/ASoC: hda: ext: add 'ext' prefix to snd_hdac_link_free_all ALSA/ASoC: hda: ext: remove 'link' prefix for stream-related operations ALSA/ASoC: hda: ext: add 'bus' prefix for multi-link stream setting ALSA: hda: ext: reduce ambiguity between 'multi-link' and 'link' DMA ALSA: hda: hdac_ext_controller: remove useless loop ALSA/ASoC: hda: move SPIB/DRMS functionality from ext layer
Applied now to for-next branch.
Mark, if you need those changes in your tree, feel free to pull from topic/hda-ext-cleanup branch of my tree. I can add the tag if needed, too.
thanks,
Takashi
On 2022-10-20 3:38 PM, Takashi Iwai wrote:
...
Applied now to for-next branch.
Mark, if you need those changes in your tree, feel free to pull from topic/hda-ext-cleanup branch of my tree. I can add the tag if needed, too.
I'd very much appreciate if you merged this series into your tree, Mark. As Pierre mentioned, recent PCM suspend/resume() series for the avs-driver needs to be rebased onto this one to eliminate any conflicts.
Regards, Czarek
On Thu, Oct 20, 2022 at 04:05:32PM +0200, Cezary Rojewski wrote:
I'd very much appreciate if you merged this series into your tree, Mark. As Pierre mentioned, recent PCM suspend/resume() series for the avs-driver needs to be rebased onto this one to eliminate any conflicts.
Pulled now.
participants (4)
-
Cezary Rojewski
-
Mark Brown
-
Pierre-Louis Bossart
-
Takashi Iwai