[PATCH 4/7] ASoC/SoundWire: dai: expand 'stream' concept beyond SoundWire

Vinod Koul vkoul at kernel.org
Thu Dec 23 17:35:21 CET 2021


On 13-12-21, 13:46, Bard Liao wrote:
> From: Pierre-Louis Bossart <pierre-louis.bossart at linux.intel.com>
> 
> The HDAudio ASoC support relies on the set_tdm_slots() helper to store
> the HDaudio stream tag in the tx_mask. This only works because of the
> pre-existing order in soc-pcm.c, where the hw_params() is handled for
> codec_dais *before* cpu_dais. When the order is reversed, the
> stream_tag is used as a mask in the codec fixup functions:
> 
> 	/* fixup params based on TDM slot masks */
> 	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK &&
> 	    codec_dai->tx_mask)
> 		soc_pcm_codec_params_fixup(&codec_params,
> 					   codec_dai->tx_mask);
> 
> As a result of this confusion, the codec_params_fixup() ends-up
> generating bad channel masks, depending on what stream_tag was
> allocated.

When we started this, I recall that Mark wanted the use of tdm_slot for
stream tag... Even if you fix the problem by making stream generic, we
might still have the same issue with tdm_slot where wrong slot is
passed..

Would it not make sense to have it consistent in tdm_slot and have it same
order everywhere...

> 
> We could add a flag to state that the tx_mask is really not a mask,
> but it would be quite ugly to persist in overloading concepts.
> 
> Instead, this patch suggests a more generic get/set 'stream' API based
> on the existing model for SoundWire. We can expand the concept to
> store 'stream' opaque information that is specific to different DAI
> types. In the case of HDAudio DAIs, we only need to store a stream tag
> as an unsigned char pointer. The TDM rx_ and tx_masks should really
> only be used to store masks.
> 
> Rename get_sdw_stream/set_sdw_stream callbacks and helpers as
> get_stream/set_stream. No functionality change beyond the rename.

If Mark n you folks feel this is a fine idea, then no objections from
me. In that case pls do use:

Acked-By: Vinod Koul <vkoul at kernel.org>

> 
> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart at linux.intel.com>
> Reviewed-by: Rander Wang <rander.wang at intel.com>
> Reviewed-by: Ranjani Sridharan <ranjani.sridharan at intel.com>
> Signed-off-by: Bard Liao <yung-chuan.liao at linux.intel.com>
> ---
>  drivers/soundwire/intel.c        |  4 ++--
>  drivers/soundwire/qcom.c         |  8 ++++----
>  drivers/soundwire/stream.c       |  4 ++--
>  include/sound/soc-dai.h          | 32 ++++++++++++++++----------------
>  sound/soc/codecs/max98373-sdw.c  |  2 +-
>  sound/soc/codecs/rt1308-sdw.c    |  2 +-
>  sound/soc/codecs/rt1316-sdw.c    |  2 +-
>  sound/soc/codecs/rt5682-sdw.c    |  2 +-
>  sound/soc/codecs/rt700.c         |  2 +-
>  sound/soc/codecs/rt711-sdca.c    |  2 +-
>  sound/soc/codecs/rt711.c         |  2 +-
>  sound/soc/codecs/rt715-sdca.c    |  2 +-
>  sound/soc/codecs/rt715.c         |  2 +-
>  sound/soc/codecs/sdw-mockup.c    |  2 +-
>  sound/soc/codecs/wcd938x.c       |  2 +-
>  sound/soc/codecs/wsa881x.c       |  2 +-
>  sound/soc/intel/boards/sof_sdw.c |  6 +++---
>  sound/soc/qcom/sdm845.c          |  4 ++--
>  sound/soc/qcom/sm8250.c          |  4 ++--
>  19 files changed, 43 insertions(+), 43 deletions(-)
> 
> diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
> index 3d29f02ad5a6..e946d1283892 100644
> --- a/drivers/soundwire/intel.c
> +++ b/drivers/soundwire/intel.c
> @@ -1129,8 +1129,8 @@ static const struct snd_soc_dai_ops intel_pcm_dai_ops = {
>  	.hw_free = intel_hw_free,
>  	.trigger = intel_trigger,
>  	.shutdown = intel_shutdown,
> -	.set_sdw_stream = intel_pcm_set_sdw_stream,
> -	.get_sdw_stream = intel_get_sdw_stream,
> +	.set_stream = intel_pcm_set_sdw_stream,
> +	.get_stream = intel_get_sdw_stream,
>  };
>  
>  static const struct snd_soc_dai_ops intel_pdm_dai_ops = {
> diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
> index 46995bb382eb..54813417ef8e 100644
> --- a/drivers/soundwire/qcom.c
> +++ b/drivers/soundwire/qcom.c
> @@ -1024,8 +1024,8 @@ static int qcom_swrm_startup(struct snd_pcm_substream *substream,
>  	ctrl->sruntime[dai->id] = sruntime;
>  
>  	for_each_rtd_codec_dais(rtd, i, codec_dai) {
> -		ret = snd_soc_dai_set_sdw_stream(codec_dai, sruntime,
> -						 substream->stream);
> +		ret = snd_soc_dai_set_stream(codec_dai, sruntime,
> +					     substream->stream);
>  		if (ret < 0 && ret != -ENOTSUPP) {
>  			dev_err(dai->dev, "Failed to set sdw stream on %s\n",
>  				codec_dai->name);
> @@ -1051,8 +1051,8 @@ static const struct snd_soc_dai_ops qcom_swrm_pdm_dai_ops = {
>  	.hw_free = qcom_swrm_hw_free,
>  	.startup = qcom_swrm_startup,
>  	.shutdown = qcom_swrm_shutdown,
> -	.set_sdw_stream = qcom_swrm_set_sdw_stream,
> -	.get_sdw_stream = qcom_swrm_get_sdw_stream,
> +	.set_stream = qcom_swrm_set_sdw_stream,
> +	.get_stream = qcom_swrm_get_sdw_stream,
>  };
>  
>  static const struct snd_soc_component_driver qcom_swrm_dai_component = {
> diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
> index 5d4f6b308ef7..980f26d49b66 100644
> --- a/drivers/soundwire/stream.c
> +++ b/drivers/soundwire/stream.c
> @@ -1863,7 +1863,7 @@ static int set_stream(struct snd_pcm_substream *substream,
>  
>  	/* Set stream pointer on all DAIs */
>  	for_each_rtd_dais(rtd, i, dai) {
> -		ret = snd_soc_dai_set_sdw_stream(dai, sdw_stream, substream->stream);
> +		ret = snd_soc_dai_set_stream(dai, sdw_stream, substream->stream);
>  		if (ret < 0) {
>  			dev_err(rtd->dev, "failed to set stream pointer on dai %s\n", dai->name);
>  			break;
> @@ -1934,7 +1934,7 @@ void sdw_shutdown_stream(void *sdw_substream)
>  	/* Find stream from first CPU DAI */
>  	dai = asoc_rtd_to_cpu(rtd, 0);
>  
> -	sdw_stream = snd_soc_dai_get_sdw_stream(dai, substream->stream);
> +	sdw_stream = snd_soc_dai_get_stream(dai, substream->stream);
>  
>  	if (IS_ERR(sdw_stream)) {
>  		dev_err(rtd->dev, "no stream found for DAI %s\n", dai->name);
> diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h
> index 0dcb361a98bb..ef3bb1bcea4e 100644
> --- a/include/sound/soc-dai.h
> +++ b/include/sound/soc-dai.h
> @@ -295,9 +295,9 @@ struct snd_soc_dai_ops {
>  			unsigned int *rx_num, unsigned int *rx_slot);
>  	int (*set_tristate)(struct snd_soc_dai *dai, int tristate);
>  
> -	int (*set_sdw_stream)(struct snd_soc_dai *dai,
> -			void *stream, int direction);
> -	void *(*get_sdw_stream)(struct snd_soc_dai *dai, int direction);
> +	int (*set_stream)(struct snd_soc_dai *dai,
> +			  void *stream, int direction);
> +	void *(*get_stream)(struct snd_soc_dai *dai, int direction);
>  
>  	/*
>  	 * DAI digital mute - optional.
> @@ -515,42 +515,42 @@ static inline void *snd_soc_dai_get_drvdata(struct snd_soc_dai *dai)
>  }
>  
>  /**
> - * snd_soc_dai_set_sdw_stream() - Configures a DAI for SDW stream operation
> + * snd_soc_dai_set_stream() - Configures a DAI for stream operation
>   * @dai: DAI
> - * @stream: STREAM
> + * @stream: STREAM (opaque structure depending on DAI type)
>   * @direction: Stream direction(Playback/Capture)
> - * SoundWire subsystem doesn't have a notion of direction and we reuse
> + * Some subsystems, such as SoundWire, don't have a notion of direction and we reuse
>   * the ASoC stream direction to configure sink/source ports.
>   * Playback maps to source ports and Capture for sink ports.
>   *
>   * This should be invoked with NULL to clear the stream set previously.
>   * Returns 0 on success, a negative error code otherwise.
>   */
> -static inline int snd_soc_dai_set_sdw_stream(struct snd_soc_dai *dai,
> -				void *stream, int direction)
> +static inline int snd_soc_dai_set_stream(struct snd_soc_dai *dai,
> +					 void *stream, int direction)
>  {
> -	if (dai->driver->ops->set_sdw_stream)
> -		return dai->driver->ops->set_sdw_stream(dai, stream, direction);
> +	if (dai->driver->ops->set_stream)
> +		return dai->driver->ops->set_stream(dai, stream, direction);
>  	else
>  		return -ENOTSUPP;
>  }
>  
>  /**
> - * snd_soc_dai_get_sdw_stream() - Retrieves SDW stream from DAI
> + * snd_soc_dai_get_stream() - Retrieves stream from DAI
>   * @dai: DAI
>   * @direction: Stream direction(Playback/Capture)
>   *
>   * This routine only retrieves that was previously configured
> - * with snd_soc_dai_get_sdw_stream()
> + * with snd_soc_dai_get_stream()
>   *
>   * Returns pointer to stream or an ERR_PTR value, e.g.
>   * ERR_PTR(-ENOTSUPP) if callback is not supported;
>   */
> -static inline void *snd_soc_dai_get_sdw_stream(struct snd_soc_dai *dai,
> -					       int direction)
> +static inline void *snd_soc_dai_get_stream(struct snd_soc_dai *dai,
> +					   int direction)
>  {
> -	if (dai->driver->ops->get_sdw_stream)
> -		return dai->driver->ops->get_sdw_stream(dai, direction);
> +	if (dai->driver->ops->get_stream)
> +		return dai->driver->ops->get_stream(dai, direction);
>  	else
>  		return ERR_PTR(-ENOTSUPP);
>  }
> diff --git a/sound/soc/codecs/max98373-sdw.c b/sound/soc/codecs/max98373-sdw.c
> index dc520effc61c..f47e956d4f55 100644
> --- a/sound/soc/codecs/max98373-sdw.c
> +++ b/sound/soc/codecs/max98373-sdw.c
> @@ -741,7 +741,7 @@ static int max98373_sdw_set_tdm_slot(struct snd_soc_dai *dai,
>  static const struct snd_soc_dai_ops max98373_dai_sdw_ops = {
>  	.hw_params = max98373_sdw_dai_hw_params,
>  	.hw_free = max98373_pcm_hw_free,
> -	.set_sdw_stream = max98373_set_sdw_stream,
> +	.set_stream = max98373_set_sdw_stream,
>  	.shutdown = max98373_shutdown,
>  	.set_tdm_slot = max98373_sdw_set_tdm_slot,
>  };
> diff --git a/sound/soc/codecs/rt1308-sdw.c b/sound/soc/codecs/rt1308-sdw.c
> index f716668de640..149a76075c76 100644
> --- a/sound/soc/codecs/rt1308-sdw.c
> +++ b/sound/soc/codecs/rt1308-sdw.c
> @@ -613,7 +613,7 @@ static const struct snd_soc_component_driver soc_component_sdw_rt1308 = {
>  static const struct snd_soc_dai_ops rt1308_aif_dai_ops = {
>  	.hw_params = rt1308_sdw_hw_params,
>  	.hw_free	= rt1308_sdw_pcm_hw_free,
> -	.set_sdw_stream	= rt1308_set_sdw_stream,
> +	.set_stream	= rt1308_set_sdw_stream,
>  	.shutdown	= rt1308_sdw_shutdown,
>  	.set_tdm_slot	= rt1308_sdw_set_tdm_slot,
>  };
> diff --git a/sound/soc/codecs/rt1316-sdw.c b/sound/soc/codecs/rt1316-sdw.c
> index 09b4914bba1b..c66d7b20cb4d 100644
> --- a/sound/soc/codecs/rt1316-sdw.c
> +++ b/sound/soc/codecs/rt1316-sdw.c
> @@ -602,7 +602,7 @@ static const struct snd_soc_component_driver soc_component_sdw_rt1316 = {
>  static const struct snd_soc_dai_ops rt1316_aif_dai_ops = {
>  	.hw_params = rt1316_sdw_hw_params,
>  	.hw_free	= rt1316_sdw_pcm_hw_free,
> -	.set_sdw_stream	= rt1316_set_sdw_stream,
> +	.set_stream	= rt1316_set_sdw_stream,
>  	.shutdown	= rt1316_sdw_shutdown,
>  };
>  
> diff --git a/sound/soc/codecs/rt5682-sdw.c b/sound/soc/codecs/rt5682-sdw.c
> index 31a4f286043e..248257a2e4e0 100644
> --- a/sound/soc/codecs/rt5682-sdw.c
> +++ b/sound/soc/codecs/rt5682-sdw.c
> @@ -272,7 +272,7 @@ static int rt5682_sdw_hw_free(struct snd_pcm_substream *substream,
>  static const struct snd_soc_dai_ops rt5682_sdw_ops = {
>  	.hw_params	= rt5682_sdw_hw_params,
>  	.hw_free	= rt5682_sdw_hw_free,
> -	.set_sdw_stream	= rt5682_set_sdw_stream,
> +	.set_stream	= rt5682_set_sdw_stream,
>  	.shutdown	= rt5682_sdw_shutdown,
>  };
>  
> diff --git a/sound/soc/codecs/rt700.c b/sound/soc/codecs/rt700.c
> index 921382724f9c..e61a8257bf64 100644
> --- a/sound/soc/codecs/rt700.c
> +++ b/sound/soc/codecs/rt700.c
> @@ -1005,7 +1005,7 @@ static int rt700_pcm_hw_free(struct snd_pcm_substream *substream,
>  static const struct snd_soc_dai_ops rt700_ops = {
>  	.hw_params	= rt700_pcm_hw_params,
>  	.hw_free	= rt700_pcm_hw_free,
> -	.set_sdw_stream	= rt700_set_sdw_stream,
> +	.set_stream	= rt700_set_sdw_stream,
>  	.shutdown	= rt700_shutdown,
>  };
>  
> diff --git a/sound/soc/codecs/rt711-sdca.c b/sound/soc/codecs/rt711-sdca.c
> index 2e992589f1e4..bdb1375f0338 100644
> --- a/sound/soc/codecs/rt711-sdca.c
> +++ b/sound/soc/codecs/rt711-sdca.c
> @@ -1358,7 +1358,7 @@ static int rt711_sdca_pcm_hw_free(struct snd_pcm_substream *substream,
>  static const struct snd_soc_dai_ops rt711_sdca_ops = {
>  	.hw_params	= rt711_sdca_pcm_hw_params,
>  	.hw_free	= rt711_sdca_pcm_hw_free,
> -	.set_sdw_stream	= rt711_sdca_set_sdw_stream,
> +	.set_stream	= rt711_sdca_set_sdw_stream,
>  	.shutdown	= rt711_sdca_shutdown,
>  };
>  
> diff --git a/sound/soc/codecs/rt711.c b/sound/soc/codecs/rt711.c
> index a7c5608a0ef8..6770825d037a 100644
> --- a/sound/soc/codecs/rt711.c
> +++ b/sound/soc/codecs/rt711.c
> @@ -1089,7 +1089,7 @@ static int rt711_pcm_hw_free(struct snd_pcm_substream *substream,
>  static const struct snd_soc_dai_ops rt711_ops = {
>  	.hw_params	= rt711_pcm_hw_params,
>  	.hw_free	= rt711_pcm_hw_free,
> -	.set_sdw_stream	= rt711_set_sdw_stream,
> +	.set_stream	= rt711_set_sdw_stream,
>  	.shutdown	= rt711_shutdown,
>  };
>  
> diff --git a/sound/soc/codecs/rt715-sdca.c b/sound/soc/codecs/rt715-sdca.c
> index 66e166568c50..bfa536bd7196 100644
> --- a/sound/soc/codecs/rt715-sdca.c
> +++ b/sound/soc/codecs/rt715-sdca.c
> @@ -938,7 +938,7 @@ static int rt715_sdca_pcm_hw_free(struct snd_pcm_substream *substream,
>  static const struct snd_soc_dai_ops rt715_sdca_ops = {
>  	.hw_params	= rt715_sdca_pcm_hw_params,
>  	.hw_free	= rt715_sdca_pcm_hw_free,
> -	.set_sdw_stream	= rt715_sdca_set_sdw_stream,
> +	.set_stream	= rt715_sdca_set_sdw_stream,
>  	.shutdown	= rt715_sdca_shutdown,
>  };
>  
> diff --git a/sound/soc/codecs/rt715.c b/sound/soc/codecs/rt715.c
> index 1352869cc086..a64d11a74751 100644
> --- a/sound/soc/codecs/rt715.c
> +++ b/sound/soc/codecs/rt715.c
> @@ -909,7 +909,7 @@ static int rt715_pcm_hw_free(struct snd_pcm_substream *substream,
>  static const struct snd_soc_dai_ops rt715_ops = {
>  	.hw_params	= rt715_pcm_hw_params,
>  	.hw_free	= rt715_pcm_hw_free,
> -	.set_sdw_stream	= rt715_set_sdw_stream,
> +	.set_stream	= rt715_set_sdw_stream,
>  	.shutdown	= rt715_shutdown,
>  };
>  
> diff --git a/sound/soc/codecs/sdw-mockup.c b/sound/soc/codecs/sdw-mockup.c
> index 8ea13cfa9f8e..7c612aaf31c7 100644
> --- a/sound/soc/codecs/sdw-mockup.c
> +++ b/sound/soc/codecs/sdw-mockup.c
> @@ -138,7 +138,7 @@ static int sdw_mockup_pcm_hw_free(struct snd_pcm_substream *substream,
>  static const struct snd_soc_dai_ops sdw_mockup_ops = {
>  	.hw_params	= sdw_mockup_pcm_hw_params,
>  	.hw_free	= sdw_mockup_pcm_hw_free,
> -	.set_sdw_stream	= sdw_mockup_set_sdw_stream,
> +	.set_stream	= sdw_mockup_set_sdw_stream,
>  	.shutdown	= sdw_mockup_shutdown,
>  };
>  
> diff --git a/sound/soc/codecs/wcd938x.c b/sound/soc/codecs/wcd938x.c
> index 52de7d14b139..ca30e5ce5c69 100644
> --- a/sound/soc/codecs/wcd938x.c
> +++ b/sound/soc/codecs/wcd938x.c
> @@ -4284,7 +4284,7 @@ static int wcd938x_codec_set_sdw_stream(struct snd_soc_dai *dai,
>  static const struct snd_soc_dai_ops wcd938x_sdw_dai_ops = {
>  	.hw_params = wcd938x_codec_hw_params,
>  	.hw_free = wcd938x_codec_free,
> -	.set_sdw_stream = wcd938x_codec_set_sdw_stream,
> +	.set_stream = wcd938x_codec_set_sdw_stream,
>  };
>  
>  static struct snd_soc_dai_driver wcd938x_dais[] = {
> diff --git a/sound/soc/codecs/wsa881x.c b/sound/soc/codecs/wsa881x.c
> index 2da4a5fa7a18..ffc025e01bce 100644
> --- a/sound/soc/codecs/wsa881x.c
> +++ b/sound/soc/codecs/wsa881x.c
> @@ -1018,7 +1018,7 @@ static const struct snd_soc_dai_ops wsa881x_dai_ops = {
>  	.hw_params = wsa881x_hw_params,
>  	.hw_free = wsa881x_hw_free,
>  	.mute_stream = wsa881x_digital_mute,
> -	.set_sdw_stream = wsa881x_set_sdw_stream,
> +	.set_stream = wsa881x_set_sdw_stream,
>  };
>  
>  static struct snd_soc_dai_driver wsa881x_dais[] = {
> diff --git a/sound/soc/intel/boards/sof_sdw.c b/sound/soc/intel/boards/sof_sdw.c
> index f10496206cee..e960ba211040 100644
> --- a/sound/soc/intel/boards/sof_sdw.c
> +++ b/sound/soc/intel/boards/sof_sdw.c
> @@ -280,7 +280,7 @@ int sdw_prepare(struct snd_pcm_substream *substream)
>  	/* Find stream from first CPU DAI */
>  	dai = asoc_rtd_to_cpu(rtd, 0);
>  
> -	sdw_stream = snd_soc_dai_get_sdw_stream(dai, substream->stream);
> +	sdw_stream = snd_soc_dai_get_stream(dai, substream->stream);
>  
>  	if (IS_ERR(sdw_stream)) {
>  		dev_err(rtd->dev, "no stream found for DAI %s", dai->name);
> @@ -300,7 +300,7 @@ int sdw_trigger(struct snd_pcm_substream *substream, int cmd)
>  	/* Find stream from first CPU DAI */
>  	dai = asoc_rtd_to_cpu(rtd, 0);
>  
> -	sdw_stream = snd_soc_dai_get_sdw_stream(dai, substream->stream);
> +	sdw_stream = snd_soc_dai_get_stream(dai, substream->stream);
>  
>  	if (IS_ERR(sdw_stream)) {
>  		dev_err(rtd->dev, "no stream found for DAI %s", dai->name);
> @@ -339,7 +339,7 @@ int sdw_hw_free(struct snd_pcm_substream *substream)
>  	/* Find stream from first CPU DAI */
>  	dai = asoc_rtd_to_cpu(rtd, 0);
>  
> -	sdw_stream = snd_soc_dai_get_sdw_stream(dai, substream->stream);
> +	sdw_stream = snd_soc_dai_get_stream(dai, substream->stream);
>  
>  	if (IS_ERR(sdw_stream)) {
>  		dev_err(rtd->dev, "no stream found for DAI %s", dai->name);
> diff --git a/sound/soc/qcom/sdm845.c b/sound/soc/qcom/sdm845.c
> index 0adfc5708949..4da5ad609fce 100644
> --- a/sound/soc/qcom/sdm845.c
> +++ b/sound/soc/qcom/sdm845.c
> @@ -56,8 +56,8 @@ static int sdm845_slim_snd_hw_params(struct snd_pcm_substream *substream,
>  	int ret = 0, i;
>  
>  	for_each_rtd_codec_dais(rtd, i, codec_dai) {
> -		sruntime = snd_soc_dai_get_sdw_stream(codec_dai,
> -						      substream->stream);
> +		sruntime = snd_soc_dai_get_stream(codec_dai,
> +						  substream->stream);
>  		if (sruntime != ERR_PTR(-ENOTSUPP))
>  			pdata->sruntime[cpu_dai->id] = sruntime;
>  
> diff --git a/sound/soc/qcom/sm8250.c b/sound/soc/qcom/sm8250.c
> index b2ca2579810b..114a29e01c0f 100644
> --- a/sound/soc/qcom/sm8250.c
> +++ b/sound/soc/qcom/sm8250.c
> @@ -136,8 +136,8 @@ static int sm8250_snd_hw_params(struct snd_pcm_substream *substream,
>  	case TX_CODEC_DMA_TX_2:
>  	case TX_CODEC_DMA_TX_3:
>  		for_each_rtd_codec_dais(rtd, i, codec_dai) {
> -			sruntime = snd_soc_dai_get_sdw_stream(codec_dai,
> -						      substream->stream);
> +			sruntime = snd_soc_dai_get_stream(codec_dai,
> +							  substream->stream);
>  			if (sruntime != ERR_PTR(-ENOTSUPP))
>  				pdata->sruntime[cpu_dai->id] = sruntime;
>  		}
> -- 
> 2.17.1

-- 
~Vinod


More information about the Alsa-devel mailing list