| Message ID | 20231025144601.268645-1-krzysztof.kozlowski@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [1/3] soundwire: qcom: drop unneeded DAI .set_stream callback | expand |
On 10/25/23 09:46, Krzysztof Kozlowski wrote: > From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> > > Store the pointer to struct device of Soundwire controller owning this > runtime stream. This can be later used by Soundwire devices, to check > if their DAI prepare callback is called for the same bus, in cases where > multiple Soundwire buses are used in same soundcard codec list. > > Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> > Co-developed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > --- > drivers/soundwire/qcom.c | 1 + > include/linux/soundwire/sdw.h | 2 ++ > 2 files changed, 3 insertions(+) > > diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c > index fe65c26c5281..a95f39563b47 100644 > --- a/drivers/soundwire/qcom.c > +++ b/drivers/soundwire/qcom.c > @@ -1298,6 +1298,7 @@ static int qcom_swrm_startup(struct snd_pcm_substream *substream, > goto err_alloc; > } > > + sruntime->dev = ctrl->bus.dev; > ctrl->sruntime[dai->id] = sruntime; > > for_each_rtd_codec_dais(rtd, i, codec_dai) { > diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h > index 4f3d14bb1538..650334adc261 100644 > --- a/include/linux/soundwire/sdw.h > +++ b/include/linux/soundwire/sdw.h > @@ -1023,6 +1023,7 @@ struct sdw_stream_params { > * master_list can contain only one m_rt per Master instance > * for a stream > * @m_rt_count: Count of Master runtime(s) in this stream > + * @dev: SoundWire controller owning this runtime stream A stream connects multiple managers and multiple peripherals. The definition above does not make a lot of sense and doesn't work in general since there's no 'owner' really. And nothing prevents the use of multiple controllers, there are not restrictions in the MIPI DisCo spec that prevent a stream from relying on different controllers. > */ > struct sdw_stream_runtime { > const char *name; > @@ -1031,6 +1032,7 @@ struct sdw_stream_runtime { > enum sdw_stream_type type; > struct list_head master_list; > int m_rt_count; > + struct device *dev; > }; > > struct sdw_stream_runtime *sdw_alloc_stream(const char *stream_name);
On 10/25/23 09:45, Krzysztof Kozlowski wrote: > Qualcomm Soundwire controller drivers do not support multi-link setups, > so DAI .set_stream() callback will not be used. What's more, if called > it will overwrite the sdw_stream_runtime runtime set in DAI .startup > (qcom_swrm_startup()) causing issues (unsupported multi-link error) when > two Soundwire controllers are passed as codec DAIs. This last sentence is confusing at best. A controller can have one or more managers, each of whom can have one or more peripherals. only peripherals should expose codec DAIs, managers should expose CPU DAIs. Put differently, the controller is the host part while the peripheral is the codec part. "controllers passed as codec DAIs" is not really possible, or this was a typo? > Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > --- > drivers/soundwire/qcom.c | 11 ----------- > 1 file changed, 11 deletions(-) > > diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c > index f1b8d6ac5140..fe65c26c5281 100644 > --- a/drivers/soundwire/qcom.c > +++ b/drivers/soundwire/qcom.c > @@ -1267,16 +1267,6 @@ static int qcom_swrm_hw_free(struct snd_pcm_substream *substream, > return 0; > } > > -static int qcom_swrm_set_sdw_stream(struct snd_soc_dai *dai, > - void *stream, int direction) > -{ > - struct qcom_swrm_ctrl *ctrl = dev_get_drvdata(dai->dev); > - > - ctrl->sruntime[dai->id] = stream; > - > - return 0; > -} > - > static void *qcom_swrm_get_sdw_stream(struct snd_soc_dai *dai, int direction) > { > struct qcom_swrm_ctrl *ctrl = dev_get_drvdata(dai->dev); > @@ -1349,7 +1339,6 @@ 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_stream = qcom_swrm_set_sdw_stream, > .get_stream = qcom_swrm_get_sdw_stream, > }; >
On 25/10/2023 17:12, Pierre-Louis Bossart wrote: > > > On 10/25/23 09:45, Krzysztof Kozlowski wrote: >> Qualcomm Soundwire controller drivers do not support multi-link setups, >> so DAI .set_stream() callback will not be used. What's more, if called >> it will overwrite the sdw_stream_runtime runtime set in DAI .startup >> (qcom_swrm_startup()) causing issues (unsupported multi-link error) when >> two Soundwire controllers are passed as codec DAIs. > > This last sentence is confusing at best. > > A controller can have one or more managers, each of whom can have one or > more peripherals. > > only peripherals should expose codec DAIs, managers should expose CPU DAIs. > > Put differently, the controller is the host part while the peripheral is > the codec part. "controllers passed as codec DAIs" is not really > possible, or this was a typo? No, it wasn't a typo. Take a look here: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts#n1023 The <&swr0 0> is the controller, although probably I should call it manager, but in case of Qualcomm I think they are 1-to-1. Best regards, Krzysztof
>>> Qualcomm Soundwire controller drivers do not support multi-link setups, >>> so DAI .set_stream() callback will not be used. What's more, if called >>> it will overwrite the sdw_stream_runtime runtime set in DAI .startup >>> (qcom_swrm_startup()) causing issues (unsupported multi-link error) when >>> two Soundwire controllers are passed as codec DAIs. >> >> This last sentence is confusing at best. >> >> A controller can have one or more managers, each of whom can have one or >> more peripherals. >> >> only peripherals should expose codec DAIs, managers should expose CPU DAIs. >> >> Put differently, the controller is the host part while the peripheral is >> the codec part. "controllers passed as codec DAIs" is not really >> possible, or this was a typo? > > No, it wasn't a typo. Take a look here: > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts#n1023 > > The <&swr0 0> is the controller, although probably I should call it > manager, but in case of Qualcomm I think they are 1-to-1. Is this a case where the SoundWire manager is part of a codec? In that case, how are the SoundWire peripheral modeled? The .set_stream callback was really meant to be used when you have a CPU DAI for the manager and a codec DAI for the peripheral(s). This seems to be a different configuration where CPU and codec DAIs are mixed.
diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c index f1b8d6ac5140..fe65c26c5281 100644 --- a/drivers/soundwire/qcom.c +++ b/drivers/soundwire/qcom.c @@ -1267,16 +1267,6 @@ static int qcom_swrm_hw_free(struct snd_pcm_substream *substream, return 0; } -static int qcom_swrm_set_sdw_stream(struct snd_soc_dai *dai, - void *stream, int direction) -{ - struct qcom_swrm_ctrl *ctrl = dev_get_drvdata(dai->dev); - - ctrl->sruntime[dai->id] = stream; - - return 0; -} - static void *qcom_swrm_get_sdw_stream(struct snd_soc_dai *dai, int direction) { struct qcom_swrm_ctrl *ctrl = dev_get_drvdata(dai->dev); @@ -1349,7 +1339,6 @@ 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_stream = qcom_swrm_set_sdw_stream, .get_stream = qcom_swrm_get_sdw_stream, };
Qualcomm Soundwire controller drivers do not support multi-link setups, so DAI .set_stream() callback will not be used. What's more, if called it will overwrite the sdw_stream_runtime runtime set in DAI .startup (qcom_swrm_startup()) causing issues (unsupported multi-link error) when two Soundwire controllers are passed as codec DAIs. Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> --- drivers/soundwire/qcom.c | 11 ----------- 1 file changed, 11 deletions(-)