* [PATCH v2 0/5] soundwire: Add support to Qualcomm SoundWire master @ 2019-08-13 8:35 Srinivas Kandagatla 2019-08-13 8:35 ` [PATCH v2 1/5] soundwire: Add compute_params callback Srinivas Kandagatla ` (4 more replies) 0 siblings, 5 replies; 44+ messages in thread From: Srinivas Kandagatla @ 2019-08-13 8:35 UTC (permalink / raw) To: vkoul, broonie Cc: bgoswami, plai, pierre-louis.bossart, robh+dt, devicetree, lgirdwood, alsa-devel, linux-kernel, spapothi, Srinivas Kandagatla Thanks for reviewing the RFC patchset. Here is new patchset addressing all the comments from RFC. This patchset adds support for Qualcomm SoundWire Master Controller found in most of Qualcomm SoCs and WCD audio codecs. This driver along with WCD934x codec and WSA881x Class-D Smart Speaker Amplifier drivers is on DragonBoard DB845c based of SDM845 SoC. WCD934x and WSA881x patches will be posted soon. SoundWire controller on SDM845 is integrated in WCD934x audio codec via SlimBus interface. Currently this driver is very minimal and only supports PDM. Most of the code in this driver is rework of Qualcomm downstream drivers used in Andriod. Credits to Banajit Goswami and Patrick Lai's Team. TODO: Test and add PCM support. Thanks, srini Changes since RFC: - updated bindings as suggested to take care of more bus parameters. - fixed error code of snd_soc_dai_get_sdw_stream() dummy function. - Cleaned up driver to handle read/writes in same way without special casing. - removed unused defines Srinivas Kandagatla (4): soundwire: stream: make stream name a const pointer ASoC: core: add support to snd_soc_dai_get_sdw_stream() dt-bindings: soundwire: add bindings for Qcom controller soundwire: qcom: add support for SoundWire controller Vinod Koul (1): soundwire: Add compute_params callback .../bindings/soundwire/qcom,sdw.txt | 167 ++++ drivers/soundwire/Kconfig | 9 + drivers/soundwire/Makefile | 4 + drivers/soundwire/qcom.c | 919 ++++++++++++++++++ drivers/soundwire/stream.c | 12 +- include/linux/soundwire/sdw.h | 6 +- include/sound/soc-dai.h | 10 + 7 files changed, 1124 insertions(+), 3 deletions(-) create mode 100644 Documentation/devicetree/bindings/soundwire/qcom,sdw.txt create mode 100644 drivers/soundwire/qcom.c -- 2.21.0 ^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH v2 1/5] soundwire: Add compute_params callback 2019-08-13 8:35 [PATCH v2 0/5] soundwire: Add support to Qualcomm SoundWire master Srinivas Kandagatla @ 2019-08-13 8:35 ` Srinivas Kandagatla 2019-08-13 14:34 ` Pierre-Louis Bossart 2019-09-04 9:28 ` [alsa-devel] " Vinod Koul 2019-08-13 8:35 ` [PATCH v2 2/5] soundwire: stream: make stream name a const pointer Srinivas Kandagatla ` (3 subsequent siblings) 4 siblings, 2 replies; 44+ messages in thread From: Srinivas Kandagatla @ 2019-08-13 8:35 UTC (permalink / raw) To: vkoul, broonie Cc: bgoswami, plai, pierre-louis.bossart, robh+dt, devicetree, lgirdwood, alsa-devel, linux-kernel, spapothi, Srinivas Kandagatla From: Vinod Koul <vkoul@kernel.org> This callback allows masters to compute the bus parameters required. Signed-off-by: Vinod Koul <vkoul@kernel.org> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> --- drivers/soundwire/stream.c | 10 ++++++++++ include/linux/soundwire/sdw.h | 2 ++ 2 files changed, 12 insertions(+) diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c index a0476755a459..60bc2fe42928 100644 --- a/drivers/soundwire/stream.c +++ b/drivers/soundwire/stream.c @@ -1483,6 +1483,16 @@ static int _sdw_prepare_stream(struct sdw_stream_runtime *stream) bus->params.bandwidth += m_rt->stream->params.rate * m_rt->ch_count * m_rt->stream->params.bps; + /* Compute params */ + if (bus->compute_params) { + ret = bus->compute_params(bus); + if (ret < 0) { + dev_err(bus->dev, "Compute params failed: %d", + ret); + return ret; + } + } + /* Program params */ ret = sdw_program_params(bus); if (ret < 0) { diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h index bea46bd8b6ce..aac68e879fae 100644 --- a/include/linux/soundwire/sdw.h +++ b/include/linux/soundwire/sdw.h @@ -718,6 +718,7 @@ struct sdw_master_ops { * Bit set implies used number, bit clear implies unused number. * @bus_lock: bus lock * @msg_lock: message lock + * @compute_params: points to Bus resource management implementation * @ops: Master callback ops * @port_ops: Master port callback ops * @params: Current bus parameters @@ -739,6 +740,7 @@ struct sdw_bus { DECLARE_BITMAP(assigned, SDW_MAX_DEVICES); struct mutex bus_lock; struct mutex msg_lock; + int (*compute_params)(struct sdw_bus *bus); const struct sdw_master_ops *ops; const struct sdw_master_port_ops *port_ops; struct sdw_bus_params params; -- 2.21.0 ^ permalink raw reply related [flat|nested] 44+ messages in thread
* Re: [PATCH v2 1/5] soundwire: Add compute_params callback 2019-08-13 8:35 ` [PATCH v2 1/5] soundwire: Add compute_params callback Srinivas Kandagatla @ 2019-08-13 14:34 ` Pierre-Louis Bossart 2019-08-13 18:17 ` Srinivas Kandagatla 2019-09-04 9:28 ` [alsa-devel] " Vinod Koul 1 sibling, 1 reply; 44+ messages in thread From: Pierre-Louis Bossart @ 2019-08-13 14:34 UTC (permalink / raw) To: Srinivas Kandagatla, vkoul, broonie Cc: devicetree, alsa-devel, bgoswami, linux-kernel, plai, lgirdwood, robh+dt, spapothi On 8/13/19 3:35 AM, Srinivas Kandagatla wrote: > From: Vinod Koul <vkoul@kernel.org> > > This callback allows masters to compute the bus parameters required. This looks like a partial use of the patch ('soundwire: Add Intel resource management algorithm')? see comments below > > Signed-off-by: Vinod Koul <vkoul@kernel.org> > Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> > --- > drivers/soundwire/stream.c | 10 ++++++++++ > include/linux/soundwire/sdw.h | 2 ++ > 2 files changed, 12 insertions(+) > > diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c > index a0476755a459..60bc2fe42928 100644 > --- a/drivers/soundwire/stream.c > +++ b/drivers/soundwire/stream.c > @@ -1483,6 +1483,16 @@ static int _sdw_prepare_stream(struct sdw_stream_runtime *stream) > bus->params.bandwidth += m_rt->stream->params.rate * > m_rt->ch_count * m_rt->stream->params.bps; > > + /* Compute params */ > + if (bus->compute_params) { > + ret = bus->compute_params(bus); > + if (ret < 0) { > + dev_err(bus->dev, "Compute params failed: %d", > + ret); > + return ret; > + } > + } > + This would need to be duplicated for deprepare (as was done in the Intel patch). > /* Program params */ > ret = sdw_program_params(bus); > if (ret < 0) { > diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h > index bea46bd8b6ce..aac68e879fae 100644 > --- a/include/linux/soundwire/sdw.h > +++ b/include/linux/soundwire/sdw.h > @@ -718,6 +718,7 @@ struct sdw_master_ops { > * Bit set implies used number, bit clear implies unused number. > * @bus_lock: bus lock > * @msg_lock: message lock > + * @compute_params: points to Bus resource management implementation > * @ops: Master callback ops > * @port_ops: Master port callback ops > * @params: Current bus parameters > @@ -739,6 +740,7 @@ struct sdw_bus { > DECLARE_BITMAP(assigned, SDW_MAX_DEVICES); > struct mutex bus_lock; > struct mutex msg_lock; > + int (*compute_params)(struct sdw_bus *bus); not sure I understand how it's set? We have a default in the Intel patch. > const struct sdw_master_ops *ops; > const struct sdw_master_port_ops *port_ops; > struct sdw_bus_params params; > ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v2 1/5] soundwire: Add compute_params callback 2019-08-13 14:34 ` Pierre-Louis Bossart @ 2019-08-13 18:17 ` Srinivas Kandagatla 2019-08-13 19:08 ` Pierre-Louis Bossart 0 siblings, 1 reply; 44+ messages in thread From: Srinivas Kandagatla @ 2019-08-13 18:17 UTC (permalink / raw) To: Pierre-Louis Bossart, vkoul, broonie Cc: devicetree, alsa-devel, bgoswami, linux-kernel, plai, lgirdwood, robh+dt, spapothi On 13/08/2019 15:34, Pierre-Louis Bossart wrote: > On 8/13/19 3:35 AM, Srinivas Kandagatla wrote: >> From: Vinod Koul <vkoul@kernel.org> >> >> This callback allows masters to compute the bus parameters required. > > This looks like a partial use of the patch ('soundwire: Add Intel > resource management algorithm')? see comments below > Yes it duplicate indeed! I will use that patch! --srini ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v2 1/5] soundwire: Add compute_params callback 2019-08-13 18:17 ` Srinivas Kandagatla @ 2019-08-13 19:08 ` Pierre-Louis Bossart 2019-08-14 4:05 ` Vinod Koul 0 siblings, 1 reply; 44+ messages in thread From: Pierre-Louis Bossart @ 2019-08-13 19:08 UTC (permalink / raw) To: Srinivas Kandagatla, vkoul, broonie Cc: devicetree, alsa-devel, bgoswami, linux-kernel, plai, lgirdwood, robh+dt, spapothi On 8/13/19 1:17 PM, Srinivas Kandagatla wrote: > > > On 13/08/2019 15:34, Pierre-Louis Bossart wrote: >> On 8/13/19 3:35 AM, Srinivas Kandagatla wrote: >>> From: Vinod Koul <vkoul@kernel.org> >>> >>> This callback allows masters to compute the bus parameters required. >> >> This looks like a partial use of the patch ('soundwire: Add Intel >> resource management algorithm')? see comments below >> > > Yes it duplicate indeed! > > I will use that patch! Actually please don't... we found issues with the Intel allocation so I'd rather have the big Intel patch split into two parts, with callbacks+prepare/deprepare changes going in first. It'll be much faster/nicer for everyone. ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v2 1/5] soundwire: Add compute_params callback 2019-08-13 19:08 ` Pierre-Louis Bossart @ 2019-08-14 4:05 ` Vinod Koul 0 siblings, 0 replies; 44+ messages in thread From: Vinod Koul @ 2019-08-14 4:05 UTC (permalink / raw) To: Pierre-Louis Bossart Cc: Srinivas Kandagatla, broonie, devicetree, alsa-devel, bgoswami, linux-kernel, plai, lgirdwood, robh+dt, spapothi On 13-08-19, 14:08, Pierre-Louis Bossart wrote: > On 8/13/19 1:17 PM, Srinivas Kandagatla wrote: > > > > > > On 13/08/2019 15:34, Pierre-Louis Bossart wrote: > > > On 8/13/19 3:35 AM, Srinivas Kandagatla wrote: > > > > From: Vinod Koul <vkoul@kernel.org> > > > > > > > > This callback allows masters to compute the bus parameters required. > > > > > > This looks like a partial use of the patch ('soundwire: Add Intel > > > resource management algorithm')? see comments below > > > > > > > Yes it duplicate indeed! > > > > I will use that patch! > > Actually please don't... > we found issues with the Intel allocation so I'd rather have the big Intel > patch split into two parts, with callbacks+prepare/deprepare changes going > in first. It'll be much faster/nicer for everyone. I was about to say that as well. I think lets take this as is and Intel algo can be on top of this. That seems easier for everyone to sort dependencies -- ~Vinod ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [alsa-devel] [PATCH v2 1/5] soundwire: Add compute_params callback 2019-08-13 8:35 ` [PATCH v2 1/5] soundwire: Add compute_params callback Srinivas Kandagatla 2019-08-13 14:34 ` Pierre-Louis Bossart @ 2019-09-04 9:28 ` Vinod Koul 2019-09-04 13:36 ` Pierre-Louis Bossart 1 sibling, 1 reply; 44+ messages in thread From: Vinod Koul @ 2019-09-04 9:28 UTC (permalink / raw) To: Srinivas Kandagatla Cc: devicetree, alsa-devel, bgoswami, linux-kernel, plai, pierre-louis.bossart, robh+dt, lgirdwood, broonie, spapothi On 13-08-19, 09:35, Srinivas Kandagatla wrote: > From: Vinod Koul <vkoul@kernel.org> > > This callback allows masters to compute the bus parameters required. Applied this to help manage cross dependencies with various folks better, thanks -- ~Vinod _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org https://mailman.alsa-project.org/mailman/listinfo/alsa-devel ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [alsa-devel] [PATCH v2 1/5] soundwire: Add compute_params callback 2019-09-04 9:28 ` [alsa-devel] " Vinod Koul @ 2019-09-04 13:36 ` Pierre-Louis Bossart 0 siblings, 0 replies; 44+ messages in thread From: Pierre-Louis Bossart @ 2019-09-04 13:36 UTC (permalink / raw) To: Vinod Koul, Srinivas Kandagatla Cc: devicetree, alsa-devel, bgoswami, plai, lgirdwood, robh+dt, linux-kernel, broonie, spapothi On 9/4/19 4:28 AM, Vinod Koul wrote: > On 13-08-19, 09:35, Srinivas Kandagatla wrote: >> From: Vinod Koul <vkoul@kernel.org> >> >> This callback allows masters to compute the bus parameters required. > > Applied this to help manage cross dependencies with various folks better, thanks Meh. doesn't really help, see my earlier comment, it's missing a call to compute_params in deprepare() so not sure this is works... _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org https://mailman.alsa-project.org/mailman/listinfo/alsa-devel ^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH v2 2/5] soundwire: stream: make stream name a const pointer 2019-08-13 8:35 [PATCH v2 0/5] soundwire: Add support to Qualcomm SoundWire master Srinivas Kandagatla 2019-08-13 8:35 ` [PATCH v2 1/5] soundwire: Add compute_params callback Srinivas Kandagatla @ 2019-08-13 8:35 ` Srinivas Kandagatla 2019-09-04 9:29 ` [alsa-devel] " Vinod Koul 2019-08-13 8:35 ` [PATCH v2 3/5] ASoC: core: add support to snd_soc_dai_get_sdw_stream() Srinivas Kandagatla ` (2 subsequent siblings) 4 siblings, 1 reply; 44+ messages in thread From: Srinivas Kandagatla @ 2019-08-13 8:35 UTC (permalink / raw) To: vkoul, broonie Cc: bgoswami, plai, pierre-louis.bossart, robh+dt, devicetree, lgirdwood, alsa-devel, linux-kernel, spapothi, Srinivas Kandagatla Make stream name const pointer Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> --- drivers/soundwire/stream.c | 2 +- include/linux/soundwire/sdw.h | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c index 60bc2fe42928..49ce21320f52 100644 --- a/drivers/soundwire/stream.c +++ b/drivers/soundwire/stream.c @@ -863,7 +863,7 @@ EXPORT_SYMBOL(sdw_release_stream); * sdw_alloc_stream should be called only once per stream. Typically * invoked from ALSA/ASoC machine/platform driver. */ -struct sdw_stream_runtime *sdw_alloc_stream(char *stream_name) +struct sdw_stream_runtime *sdw_alloc_stream(const char *stream_name) { struct sdw_stream_runtime *stream; diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h index aac68e879fae..5e61ad065d32 100644 --- a/include/linux/soundwire/sdw.h +++ b/include/linux/soundwire/sdw.h @@ -830,7 +830,7 @@ struct sdw_stream_params { * @m_rt_count: Count of Master runtime(s) in this stream */ struct sdw_stream_runtime { - char *name; + const char *name; struct sdw_stream_params params; enum sdw_stream_state state; enum sdw_stream_type type; @@ -838,7 +838,7 @@ struct sdw_stream_runtime { int m_rt_count; }; -struct sdw_stream_runtime *sdw_alloc_stream(char *stream_name); +struct sdw_stream_runtime *sdw_alloc_stream(const char *stream_name); void sdw_release_stream(struct sdw_stream_runtime *stream); int sdw_stream_add_master(struct sdw_bus *bus, struct sdw_stream_config *stream_config, -- 2.21.0 ^ permalink raw reply related [flat|nested] 44+ messages in thread
* Re: [alsa-devel] [PATCH v2 2/5] soundwire: stream: make stream name a const pointer 2019-08-13 8:35 ` [PATCH v2 2/5] soundwire: stream: make stream name a const pointer Srinivas Kandagatla @ 2019-09-04 9:29 ` Vinod Koul 0 siblings, 0 replies; 44+ messages in thread From: Vinod Koul @ 2019-09-04 9:29 UTC (permalink / raw) To: Srinivas Kandagatla Cc: devicetree, alsa-devel, bgoswami, linux-kernel, plai, pierre-louis.bossart, robh+dt, lgirdwood, broonie, spapothi On 13-08-19, 09:35, Srinivas Kandagatla wrote: > Make stream name const pointer Good catch, Applied, thanks -- ~Vinod _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org https://mailman.alsa-project.org/mailman/listinfo/alsa-devel ^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH v2 3/5] ASoC: core: add support to snd_soc_dai_get_sdw_stream() 2019-08-13 8:35 [PATCH v2 0/5] soundwire: Add support to Qualcomm SoundWire master Srinivas Kandagatla 2019-08-13 8:35 ` [PATCH v2 1/5] soundwire: Add compute_params callback Srinivas Kandagatla 2019-08-13 8:35 ` [PATCH v2 2/5] soundwire: stream: make stream name a const pointer Srinivas Kandagatla @ 2019-08-13 8:35 ` Srinivas Kandagatla 2019-08-13 14:44 ` [alsa-devel] " Pierre-Louis Bossart 2019-08-13 16:03 ` Cezary Rojewski 2019-08-13 8:35 ` [PATCH v2 4/5] dt-bindings: soundwire: add bindings for Qcom controller Srinivas Kandagatla 2019-08-13 8:35 ` [PATCH v2 5/5] soundwire: qcom: add support for SoundWire controller Srinivas Kandagatla 4 siblings, 2 replies; 44+ messages in thread From: Srinivas Kandagatla @ 2019-08-13 8:35 UTC (permalink / raw) To: vkoul, broonie Cc: bgoswami, plai, pierre-louis.bossart, robh+dt, devicetree, lgirdwood, alsa-devel, linux-kernel, spapothi, Srinivas Kandagatla On platforms which have smart speaker amplifiers connected via soundwire and modeled as aux devices in ASoC, in such usecases machine driver should be able to get sdw master stream from dai so that it can use the runtime stream to setup slave streams. soundwire already as a set function, get function would provide more flexibility to above configurations. Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> --- include/sound/soc-dai.h | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h index dc48fe081a20..1e01f4a302e0 100644 --- a/include/sound/soc-dai.h +++ b/include/sound/soc-dai.h @@ -202,6 +202,7 @@ struct snd_soc_dai_ops { int (*set_sdw_stream)(struct snd_soc_dai *dai, void *stream, int direction); + void *(*get_sdw_stream)(struct snd_soc_dai *dai, int direction); /* * DAI digital mute - optional. * Called by soc-core to minimise any pops. @@ -410,4 +411,13 @@ static inline int snd_soc_dai_set_sdw_stream(struct snd_soc_dai *dai, return -ENOTSUPP; } +static inline void *snd_soc_dai_get_sdw_stream(struct snd_soc_dai *dai, + int direction) +{ + if (dai->driver->ops->get_sdw_stream) + return dai->driver->ops->get_sdw_stream(dai, direction); + else + return ERR_PTR(-ENOTSUPP); +} + #endif -- 2.21.0 ^ permalink raw reply related [flat|nested] 44+ messages in thread
* Re: [alsa-devel] [PATCH v2 3/5] ASoC: core: add support to snd_soc_dai_get_sdw_stream() 2019-08-13 8:35 ` [PATCH v2 3/5] ASoC: core: add support to snd_soc_dai_get_sdw_stream() Srinivas Kandagatla @ 2019-08-13 14:44 ` Pierre-Louis Bossart 2019-08-13 16:50 ` Srinivas Kandagatla 2019-08-13 16:03 ` Cezary Rojewski 1 sibling, 1 reply; 44+ messages in thread From: Pierre-Louis Bossart @ 2019-08-13 14:44 UTC (permalink / raw) To: Srinivas Kandagatla, vkoul, broonie Cc: bgoswami, plai, robh+dt, devicetree, lgirdwood, alsa-devel, linux-kernel, spapothi On 8/13/19 3:35 AM, Srinivas Kandagatla wrote: > On platforms which have smart speaker amplifiers connected via > soundwire and modeled as aux devices in ASoC, in such usecases machine > driver should be able to get sdw master stream from dai so that it can > use the runtime stream to setup slave streams. using the _set_sdw_stream? I don't fully get the sequence with the wording above. > > soundwire already as a set function, get function would provide more > flexibility to above configurations. I am not clear if you are asking for both to be used, or get only or set only? > > Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> > --- > include/sound/soc-dai.h | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h > index dc48fe081a20..1e01f4a302e0 100644 > --- a/include/sound/soc-dai.h > +++ b/include/sound/soc-dai.h > @@ -202,6 +202,7 @@ struct snd_soc_dai_ops { > > int (*set_sdw_stream)(struct snd_soc_dai *dai, > void *stream, int direction); > + void *(*get_sdw_stream)(struct snd_soc_dai *dai, int direction); > /* > * DAI digital mute - optional. > * Called by soc-core to minimise any pops. > @@ -410,4 +411,13 @@ static inline int snd_soc_dai_set_sdw_stream(struct snd_soc_dai *dai, > return -ENOTSUPP; > } > > +static inline void *snd_soc_dai_get_sdw_stream(struct snd_soc_dai *dai, > + int direction) > +{ > + if (dai->driver->ops->get_sdw_stream) > + return dai->driver->ops->get_sdw_stream(dai, direction); > + else > + return ERR_PTR(-ENOTSUPP); > +} > + > #endif > ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [alsa-devel] [PATCH v2 3/5] ASoC: core: add support to snd_soc_dai_get_sdw_stream() 2019-08-13 14:44 ` [alsa-devel] " Pierre-Louis Bossart @ 2019-08-13 16:50 ` Srinivas Kandagatla 2019-08-13 17:51 ` Pierre-Louis Bossart 0 siblings, 1 reply; 44+ messages in thread From: Srinivas Kandagatla @ 2019-08-13 16:50 UTC (permalink / raw) To: Pierre-Louis Bossart, vkoul, broonie Cc: bgoswami, plai, robh+dt, devicetree, lgirdwood, alsa-devel, linux-kernel, spapothi Thanks for the review, On 13/08/2019 15:44, Pierre-Louis Bossart wrote: > On 8/13/19 3:35 AM, Srinivas Kandagatla wrote: >> On platforms which have smart speaker amplifiers connected via >> soundwire and modeled as aux devices in ASoC, in such usecases machine >> driver should be able to get sdw master stream from dai so that it can >> use the runtime stream to setup slave streams. > > using the _set_sdw_stream? I don't fully get the sequence with the > wording above. Yes, using set_sdw_stream(). > >> >> soundwire already as a set function, get function would provide more >> flexibility to above configurations. > > I am not clear if you are asking for both to be used, or get only or set > only? It depends on the usecase, in db845c usecase [1] as Aux device is dai less, machine driver is using get function to get hold of master stream so that it can setup slave port config. Looks like there is a typo in above like This was supposed to be "soundwire already has a set function, get function would provide more flexibility to above configurations" [1] https://git.linaro.org/landing-teams/working/qualcomm/kernel.git/tree/sound/soc/qcom/db845c.c?h=integration-linux-qcomlt thanks, srini > >> >> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> >> --- >> include/sound/soc-dai.h | 10 ++++++++++ >> 1 file changed, 10 insertions(+) >> >> diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h >> index dc48fe081a20..1e01f4a302e0 100644 >> --- a/include/sound/soc-dai.h >> +++ b/include/sound/soc-dai.h >> @@ -202,6 +202,7 @@ struct snd_soc_dai_ops { >> int (*set_sdw_stream)(struct snd_soc_dai *dai, >> void *stream, int direction); >> + void *(*get_sdw_stream)(struct snd_soc_dai *dai, int direction); >> /* >> * DAI digital mute - optional. >> * Called by soc-core to minimise any pops. >> @@ -410,4 +411,13 @@ static inline int >> snd_soc_dai_set_sdw_stream(struct snd_soc_dai *dai, >> return -ENOTSUPP; >> } >> +static inline void *snd_soc_dai_get_sdw_stream(struct snd_soc_dai *dai, >> + int direction) >> +{ >> + if (dai->driver->ops->get_sdw_stream) >> + return dai->driver->ops->get_sdw_stream(dai, direction); >> + else >> + return ERR_PTR(-ENOTSUPP); >> +} >> + >> #endif >> > ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [alsa-devel] [PATCH v2 3/5] ASoC: core: add support to snd_soc_dai_get_sdw_stream() 2019-08-13 16:50 ` Srinivas Kandagatla @ 2019-08-13 17:51 ` Pierre-Louis Bossart 2019-08-13 18:06 ` Srinivas Kandagatla 0 siblings, 1 reply; 44+ messages in thread From: Pierre-Louis Bossart @ 2019-08-13 17:51 UTC (permalink / raw) To: Srinivas Kandagatla, vkoul, broonie Cc: devicetree, alsa-devel, bgoswami, plai, lgirdwood, linux-kernel, robh+dt, spapothi On 8/13/19 11:50 AM, Srinivas Kandagatla wrote: > Thanks for the review, > > On 13/08/2019 15:44, Pierre-Louis Bossart wrote: >> On 8/13/19 3:35 AM, Srinivas Kandagatla wrote: >>> On platforms which have smart speaker amplifiers connected via >>> soundwire and modeled as aux devices in ASoC, in such usecases machine >>> driver should be able to get sdw master stream from dai so that it can >>> use the runtime stream to setup slave streams. >> >> using the _set_sdw_stream? I don't fully get the sequence with the >> wording above. > > Yes, using set_sdw_stream(). Maybe I am missing something here, but I don't see where the set_sdw_stream() is called. Also I don't fully get the rule. set_sdw_stream() looks required, get_sdw_stream() is optional, is this what you are suggesting? >> >>> >>> soundwire already as a set function, get function would provide more >>> flexibility to above configurations. >> >> I am not clear if you are asking for both to be used, or get only or >> set only? > > It depends on the usecase, in db845c usecase [1] as Aux device is dai > less, machine driver is using get function to get hold of master stream > so that it can setup slave port config. > > > Looks like there is a typo in above like > > This was supposed to be "soundwire already has a set function, get > function would provide more flexibility to above configurations" > > [1] > https://git.linaro.org/landing-teams/working/qualcomm/kernel.git/tree/sound/soc/qcom/db845c.c?h=integration-linux-qcomlt > > > thanks, > srini > >> >>> >>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> >>> --- >>> include/sound/soc-dai.h | 10 ++++++++++ >>> 1 file changed, 10 insertions(+) >>> >>> diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h >>> index dc48fe081a20..1e01f4a302e0 100644 >>> --- a/include/sound/soc-dai.h >>> +++ b/include/sound/soc-dai.h >>> @@ -202,6 +202,7 @@ struct snd_soc_dai_ops { >>> int (*set_sdw_stream)(struct snd_soc_dai *dai, >>> void *stream, int direction); >>> + void *(*get_sdw_stream)(struct snd_soc_dai *dai, int direction); >>> /* >>> * DAI digital mute - optional. >>> * Called by soc-core to minimise any pops. >>> @@ -410,4 +411,13 @@ static inline int >>> snd_soc_dai_set_sdw_stream(struct snd_soc_dai *dai, >>> return -ENOTSUPP; >>> } >>> +static inline void *snd_soc_dai_get_sdw_stream(struct snd_soc_dai *dai, >>> + int direction) >>> +{ >>> + if (dai->driver->ops->get_sdw_stream) >>> + return dai->driver->ops->get_sdw_stream(dai, direction); >>> + else >>> + return ERR_PTR(-ENOTSUPP); >>> +} >>> + >>> #endif >>> >> > _______________________________________________ > Alsa-devel mailing list > Alsa-devel@alsa-project.org > https://mailman.alsa-project.org/mailman/listinfo/alsa-devel ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [alsa-devel] [PATCH v2 3/5] ASoC: core: add support to snd_soc_dai_get_sdw_stream() 2019-08-13 17:51 ` Pierre-Louis Bossart @ 2019-08-13 18:06 ` Srinivas Kandagatla 2019-08-13 19:15 ` Pierre-Louis Bossart 0 siblings, 1 reply; 44+ messages in thread From: Srinivas Kandagatla @ 2019-08-13 18:06 UTC (permalink / raw) To: Pierre-Louis Bossart, vkoul, broonie Cc: devicetree, alsa-devel, bgoswami, plai, lgirdwood, linux-kernel, robh+dt, spapothi On 13/08/2019 18:51, Pierre-Louis Bossart wrote: > On 8/13/19 11:50 AM, Srinivas Kandagatla wrote: >> Thanks for the review, >> >> On 13/08/2019 15:44, Pierre-Louis Bossart wrote: >>> On 8/13/19 3:35 AM, Srinivas Kandagatla wrote: >>>> On platforms which have smart speaker amplifiers connected via >>>> soundwire and modeled as aux devices in ASoC, in such usecases machine >>>> driver should be able to get sdw master stream from dai so that it can >>>> use the runtime stream to setup slave streams. >>> >>> using the _set_sdw_stream? I don't fully get the sequence with the >>> wording above. >> >> Yes, using set_sdw_stream(). > > Maybe I am missing something here, but I don't see where the > set_sdw_stream() is called. sorry for the confusion. It was too quick reply. :-) I was suppose to say sdw_stream_add_slave() instead of set_sdw_stream(). As Aux device is dailess there is no way to get hold of sdw stream runtime for slave device associated with it. Having snd_soc_dai_get_sdw_stream() would help machine driver to get hold of sdw_stream_runtime from controller dai and setup slave streams using sdw_stream_add_slave(). thanks, srini > > Also I don't fully get the rule. set_sdw_stream() looks required, > get_sdw_stream() is optional, is this what you are suggesting? > >>> >>>> >>>> soundwire already as a set function, get function would provide more >>>> flexibility to above configurations. >>> >>> I am not clear if you are asking for both to be used, or get only or >>> set only? >> >> It depends on the usecase, in db845c usecase [1] as Aux device is dai >> less, machine driver is using get function to get hold of master >> stream so that it can setup slave port config. >> >> >> Looks like there is a typo in above like >> >> This was supposed to be "soundwire already has a set function, get >> function would provide more flexibility to above configurations" >> >> [1] >> https://git.linaro.org/landing-teams/working/qualcomm/kernel.git/tree/sound/soc/qcom/db845c.c?h=integration-linux-qcomlt >> >> >> thanks, >> srini >> >>> >>>> >>>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> >>>> --- >>>> include/sound/soc-dai.h | 10 ++++++++++ >>>> 1 file changed, 10 insertions(+) >>>> >>>> diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h >>>> index dc48fe081a20..1e01f4a302e0 100644 >>>> --- a/include/sound/soc-dai.h >>>> +++ b/include/sound/soc-dai.h >>>> @@ -202,6 +202,7 @@ struct snd_soc_dai_ops { >>>> int (*set_sdw_stream)(struct snd_soc_dai *dai, >>>> void *stream, int direction); >>>> + void *(*get_sdw_stream)(struct snd_soc_dai *dai, int direction); >>>> /* >>>> * DAI digital mute - optional. >>>> * Called by soc-core to minimise any pops. >>>> @@ -410,4 +411,13 @@ static inline int >>>> snd_soc_dai_set_sdw_stream(struct snd_soc_dai *dai, >>>> return -ENOTSUPP; >>>> } >>>> +static inline void *snd_soc_dai_get_sdw_stream(struct snd_soc_dai >>>> *dai, >>>> + int direction) >>>> +{ >>>> + if (dai->driver->ops->get_sdw_stream) >>>> + return dai->driver->ops->get_sdw_stream(dai, direction); >>>> + else >>>> + return ERR_PTR(-ENOTSUPP); >>>> +} >>>> + >>>> #endif >>>> >>> >> _______________________________________________ >> Alsa-devel mailing list >> Alsa-devel@alsa-project.org >> https://mailman.alsa-project.org/mailman/listinfo/alsa-devel > ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [alsa-devel] [PATCH v2 3/5] ASoC: core: add support to snd_soc_dai_get_sdw_stream() 2019-08-13 18:06 ` Srinivas Kandagatla @ 2019-08-13 19:15 ` Pierre-Louis Bossart 2019-08-13 19:18 ` Mark Brown 0 siblings, 1 reply; 44+ messages in thread From: Pierre-Louis Bossart @ 2019-08-13 19:15 UTC (permalink / raw) To: Srinivas Kandagatla, vkoul, broonie Cc: devicetree, alsa-devel, bgoswami, plai, lgirdwood, linux-kernel, robh+dt, spapothi On 8/13/19 1:06 PM, Srinivas Kandagatla wrote: > > > On 13/08/2019 18:51, Pierre-Louis Bossart wrote: >> On 8/13/19 11:50 AM, Srinivas Kandagatla wrote: >>> Thanks for the review, >>> >>> On 13/08/2019 15:44, Pierre-Louis Bossart wrote: >>>> On 8/13/19 3:35 AM, Srinivas Kandagatla wrote: >>>>> On platforms which have smart speaker amplifiers connected via >>>>> soundwire and modeled as aux devices in ASoC, in such usecases machine >>>>> driver should be able to get sdw master stream from dai so that it can >>>>> use the runtime stream to setup slave streams. >>>> >>>> using the _set_sdw_stream? I don't fully get the sequence with the >>>> wording above. >>> >>> Yes, using set_sdw_stream(). >> >> Maybe I am missing something here, but I don't see where the >> set_sdw_stream() is called. > > sorry for the confusion. It was too quick reply. :-) > I was suppose to say sdw_stream_add_slave() instead of set_sdw_stream(). ok, so get_sdw_stream() and set_sdw_stream() are not meant to be mirrors or both implemented. It's just a helper to respectively get a context or set a context but a get-modify-set type of operation is not expected. Do I get this right? > > As Aux device is dailess there is no way to get hold of sdw stream > runtime for slave device associated with it. > > Having snd_soc_dai_get_sdw_stream() would help machine driver to get > hold of sdw_stream_runtime from controller dai and setup slave streams > using sdw_stream_add_slave(). > > > thanks, > srini > > >> >> Also I don't fully get the rule. set_sdw_stream() looks required, >> get_sdw_stream() is optional, is this what you are suggesting? >> >>>> >>>>> >>>>> soundwire already as a set function, get function would provide more >>>>> flexibility to above configurations. >>>> >>>> I am not clear if you are asking for both to be used, or get only or >>>> set only? >>> >>> It depends on the usecase, in db845c usecase [1] as Aux device is >>> dai less, machine driver is using get function to get hold of master >>> stream so that it can setup slave port config. >>> >>> >>> Looks like there is a typo in above like >>> >>> This was supposed to be "soundwire already has a set function, get >>> function would provide more flexibility to above configurations" >>> >>> [1] >>> https://git.linaro.org/landing-teams/working/qualcomm/kernel.git/tree/sound/soc/qcom/db845c.c?h=integration-linux-qcomlt >>> >>> >>> thanks, >>> srini >>> >>>> >>>>> >>>>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> >>>>> --- >>>>> include/sound/soc-dai.h | 10 ++++++++++ >>>>> 1 file changed, 10 insertions(+) >>>>> >>>>> diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h >>>>> index dc48fe081a20..1e01f4a302e0 100644 >>>>> --- a/include/sound/soc-dai.h >>>>> +++ b/include/sound/soc-dai.h >>>>> @@ -202,6 +202,7 @@ struct snd_soc_dai_ops { >>>>> int (*set_sdw_stream)(struct snd_soc_dai *dai, >>>>> void *stream, int direction); >>>>> + void *(*get_sdw_stream)(struct snd_soc_dai *dai, int direction); >>>>> /* >>>>> * DAI digital mute - optional. >>>>> * Called by soc-core to minimise any pops. >>>>> @@ -410,4 +411,13 @@ static inline int >>>>> snd_soc_dai_set_sdw_stream(struct snd_soc_dai *dai, >>>>> return -ENOTSUPP; >>>>> } >>>>> +static inline void *snd_soc_dai_get_sdw_stream(struct snd_soc_dai >>>>> *dai, >>>>> + int direction) >>>>> +{ >>>>> + if (dai->driver->ops->get_sdw_stream) >>>>> + return dai->driver->ops->get_sdw_stream(dai, direction); >>>>> + else >>>>> + return ERR_PTR(-ENOTSUPP); >>>>> +} >>>>> + >>>>> #endif >>>>> >>>> >>> _______________________________________________ >>> Alsa-devel mailing list >>> Alsa-devel@alsa-project.org >>> https://mailman.alsa-project.org/mailman/listinfo/alsa-devel >> ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [alsa-devel] [PATCH v2 3/5] ASoC: core: add support to snd_soc_dai_get_sdw_stream() 2019-08-13 19:15 ` Pierre-Louis Bossart @ 2019-08-13 19:18 ` Mark Brown 2019-08-13 19:38 ` Pierre-Louis Bossart 0 siblings, 1 reply; 44+ messages in thread From: Mark Brown @ 2019-08-13 19:18 UTC (permalink / raw) To: Pierre-Louis Bossart Cc: Srinivas Kandagatla, vkoul, devicetree, alsa-devel, bgoswami, plai, lgirdwood, linux-kernel, robh+dt, spapothi [-- Attachment #1: Type: text/plain, Size: 548 bytes --] On Tue, Aug 13, 2019 at 02:15:18PM -0500, Pierre-Louis Bossart wrote: > On 8/13/19 1:06 PM, Srinivas Kandagatla wrote: > > sorry for the confusion. It was too quick reply. :-) > > I was suppose to say sdw_stream_add_slave() instead of set_sdw_stream(). > ok, so get_sdw_stream() and set_sdw_stream() are not meant to be mirrors or > both implemented. It's just a helper to respectively get a context or set a > context but a get-modify-set type of operation is not expected. > Do I get this right? This seems like it's going to be confusing... [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [alsa-devel] [PATCH v2 3/5] ASoC: core: add support to snd_soc_dai_get_sdw_stream() 2019-08-13 19:18 ` Mark Brown @ 2019-08-13 19:38 ` Pierre-Louis Bossart 2019-08-13 19:58 ` Mark Brown 0 siblings, 1 reply; 44+ messages in thread From: Pierre-Louis Bossart @ 2019-08-13 19:38 UTC (permalink / raw) To: Mark Brown Cc: Srinivas Kandagatla, vkoul, devicetree, alsa-devel, bgoswami, plai, lgirdwood, linux-kernel, robh+dt, spapothi On 8/13/19 2:18 PM, Mark Brown wrote: > On Tue, Aug 13, 2019 at 02:15:18PM -0500, Pierre-Louis Bossart wrote: >> On 8/13/19 1:06 PM, Srinivas Kandagatla wrote: > >>> sorry for the confusion. It was too quick reply. :-) >>> I was suppose to say sdw_stream_add_slave() instead of set_sdw_stream(). > >> ok, so get_sdw_stream() and set_sdw_stream() are not meant to be mirrors or >> both implemented. It's just a helper to respectively get a context or set a >> context but a get-modify-set type of operation is not expected. > >> Do I get this right? > > This seems like it's going to be confusing... Indeed. I don't have a full understanding of that part to be honest, nor why we need something SoundWire-specific. We already abused the set_tdm_slot API to store an HDaudio stream, now we have a rather confusing stream information for SoundWire and I have about 3 other 'stream' contexts in SOF... I am still doing basic cleanups but this has been on my radar for a while. ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [alsa-devel] [PATCH v2 3/5] ASoC: core: add support to snd_soc_dai_get_sdw_stream() 2019-08-13 19:38 ` Pierre-Louis Bossart @ 2019-08-13 19:58 ` Mark Brown 2019-08-14 4:11 ` Vinod Koul 0 siblings, 1 reply; 44+ messages in thread From: Mark Brown @ 2019-08-13 19:58 UTC (permalink / raw) To: Pierre-Louis Bossart Cc: Srinivas Kandagatla, vkoul, devicetree, alsa-devel, bgoswami, plai, lgirdwood, linux-kernel, robh+dt, spapothi [-- Attachment #1: Type: text/plain, Size: 659 bytes --] On Tue, Aug 13, 2019 at 02:38:53PM -0500, Pierre-Louis Bossart wrote: > Indeed. I don't have a full understanding of that part to be honest, nor why > we need something SoundWire-specific. We already abused the set_tdm_slot API > to store an HDaudio stream, now we have a rather confusing stream > information for SoundWire and I have about 3 other 'stream' contexts in > SOF... I am still doing basic cleanups but this has been on my radar for a > while. There is something to be said for not abusing the TDM slot API if it can make things clearer by using bus-idiomatic mechanisms, but it does mean everything needs to know about each individual bus :/ . [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [alsa-devel] [PATCH v2 3/5] ASoC: core: add support to snd_soc_dai_get_sdw_stream() 2019-08-13 19:58 ` Mark Brown @ 2019-08-14 4:11 ` Vinod Koul 2019-08-14 9:08 ` Mark Brown 2019-08-14 14:09 ` Pierre-Louis Bossart 0 siblings, 2 replies; 44+ messages in thread From: Vinod Koul @ 2019-08-14 4:11 UTC (permalink / raw) To: Mark Brown Cc: Pierre-Louis Bossart, Srinivas Kandagatla, devicetree, alsa-devel, bgoswami, plai, lgirdwood, linux-kernel, robh+dt, spapothi On 13-08-19, 20:58, Mark Brown wrote: > On Tue, Aug 13, 2019 at 02:38:53PM -0500, Pierre-Louis Bossart wrote: > > > Indeed. I don't have a full understanding of that part to be honest, nor why > > we need something SoundWire-specific. We already abused the set_tdm_slot API > > to store an HDaudio stream, now we have a rather confusing stream > > information for SoundWire and I have about 3 other 'stream' contexts in > > SOF... I am still doing basic cleanups but this has been on my radar for a > > while. > > There is something to be said for not abusing the TDM slot API if it can > make things clearer by using bus-idiomatic mechanisms, but it does mean > everything needs to know about each individual bus :/ . Here ASoC doesn't need to know about sdw bus. As Srini explained, this helps in the case for him to get the stream context and set the stream context from the machine driver. Nothing else is expected to be done from this API. We already do a set using snd_soc_dai_set_sdw_stream(). Here we add the snd_soc_dai_get_sdw_stream() to query Thanks -- ~Vinod ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [alsa-devel] [PATCH v2 3/5] ASoC: core: add support to snd_soc_dai_get_sdw_stream() 2019-08-14 4:11 ` Vinod Koul @ 2019-08-14 9:08 ` Mark Brown 2019-08-14 14:09 ` Pierre-Louis Bossart 1 sibling, 0 replies; 44+ messages in thread From: Mark Brown @ 2019-08-14 9:08 UTC (permalink / raw) To: Vinod Koul Cc: Pierre-Louis Bossart, Srinivas Kandagatla, devicetree, alsa-devel, bgoswami, plai, lgirdwood, linux-kernel, robh+dt, spapothi [-- Attachment #1: Type: text/plain, Size: 870 bytes --] On Wed, Aug 14, 2019 at 09:41:42AM +0530, Vinod Koul wrote: > On 13-08-19, 20:58, Mark Brown wrote: > > There is something to be said for not abusing the TDM slot API if it can > > make things clearer by using bus-idiomatic mechanisms, but it does mean > > everything needs to know about each individual bus :/ . > Here ASoC doesn't need to know about sdw bus. As Srini explained, this > helps in the case for him to get the stream context and set the stream > context from the machine driver. Other drivers interoperating with the Soundwire DAI might want to do something, it looks like that's the case for SOF. > Nothing else is expected to be done from this API. We already do a set > using snd_soc_dai_set_sdw_stream(). Here we add the snd_soc_dai_get_sdw_stream() to query Well, if the API is not expected to do anything we can optimize it and just remove it! [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [alsa-devel] [PATCH v2 3/5] ASoC: core: add support to snd_soc_dai_get_sdw_stream() 2019-08-14 4:11 ` Vinod Koul 2019-08-14 9:08 ` Mark Brown @ 2019-08-14 14:09 ` Pierre-Louis Bossart 2019-10-09 8:32 ` Srinivas Kandagatla 1 sibling, 1 reply; 44+ messages in thread From: Pierre-Louis Bossart @ 2019-08-14 14:09 UTC (permalink / raw) To: Vinod Koul, Mark Brown Cc: devicetree, alsa-devel, bgoswami, linux-kernel, plai, lgirdwood, robh+dt, Srinivas Kandagatla, spapothi On 8/13/19 11:11 PM, Vinod Koul wrote: > On 13-08-19, 20:58, Mark Brown wrote: >> On Tue, Aug 13, 2019 at 02:38:53PM -0500, Pierre-Louis Bossart wrote: >> >>> Indeed. I don't have a full understanding of that part to be honest, nor why >>> we need something SoundWire-specific. We already abused the set_tdm_slot API >>> to store an HDaudio stream, now we have a rather confusing stream >>> information for SoundWire and I have about 3 other 'stream' contexts in >>> SOF... I am still doing basic cleanups but this has been on my radar for a >>> while. >> >> There is something to be said for not abusing the TDM slot API if it can >> make things clearer by using bus-idiomatic mechanisms, but it does mean >> everything needs to know about each individual bus :/ . > > Here ASoC doesn't need to know about sdw bus. As Srini explained, this > helps in the case for him to get the stream context and set the stream > context from the machine driver. > > Nothing else is expected to be done from this API. We already do a set > using snd_soc_dai_set_sdw_stream(). Here we add the snd_soc_dai_get_sdw_stream() to query I didn't see a call to snd_soc_dai_set_sdw_stream() in Srini's code? ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [alsa-devel] [PATCH v2 3/5] ASoC: core: add support to snd_soc_dai_get_sdw_stream() 2019-08-14 14:09 ` Pierre-Louis Bossart @ 2019-10-09 8:32 ` Srinivas Kandagatla 2019-10-09 14:29 ` Pierre-Louis Bossart 0 siblings, 1 reply; 44+ messages in thread From: Srinivas Kandagatla @ 2019-10-09 8:32 UTC (permalink / raw) To: Pierre-Louis Bossart, Vinod Koul, Mark Brown Cc: devicetree, alsa-devel, bgoswami, plai, linux-kernel, lgirdwood, robh+dt, spapothi Hi Pierre, On 14/08/2019 15:09, Pierre-Louis Bossart wrote: > > > On 8/13/19 11:11 PM, Vinod Koul wrote: >> On 13-08-19, 20:58, Mark Brown wrote: >>> On Tue, Aug 13, 2019 at 02:38:53PM -0500, Pierre-Louis Bossart wrote: >>> >>>> Indeed. I don't have a full understanding of that part to be honest, >>>> nor why >>>> we need something SoundWire-specific. We already abused the >>>> set_tdm_slot API >>>> to store an HDaudio stream, now we have a rather confusing stream >>>> information for SoundWire and I have about 3 other 'stream' contexts in >>>> SOF... I am still doing basic cleanups but this has been on my radar >>>> for a >>>> while. >>> >>> There is something to be said for not abusing the TDM slot API if it can >>> make things clearer by using bus-idiomatic mechanisms, but it does mean >>> everything needs to know about each individual bus :/ . >> >> Here ASoC doesn't need to know about sdw bus. As Srini explained, this >> helps in the case for him to get the stream context and set the stream >> context from the machine driver. >> >> Nothing else is expected to be done from this API. We already do a set >> using snd_soc_dai_set_sdw_stream(). Here we add the >> snd_soc_dai_get_sdw_stream() to query > > I didn't see a call to snd_soc_dai_set_sdw_stream() in Srini's code? There is a snd_soc_dai_get_sdw_stream() to get stream context and we add slave streams(amplifier in this case) to that context using sdw_stream_add_slave() in machine driver[1]. Without this helper there is no way to link slave streams to stream context in non dai based setup like smart speaker amplifiers. Currently this driver is blocked on this patch, If you think there are other ways to do this, am happy to try them out. Thanks, srini [1] https://git.linaro.org/people/srinivas.kandagatla/linux.git/tree/sound/soc/qcom/db845c.c?h=release/db845c/qcomlt-5.2 _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org https://mailman.alsa-project.org/mailman/listinfo/alsa-devel ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [alsa-devel] [PATCH v2 3/5] ASoC: core: add support to snd_soc_dai_get_sdw_stream() 2019-10-09 8:32 ` Srinivas Kandagatla @ 2019-10-09 14:29 ` Pierre-Louis Bossart 2019-10-09 14:29 ` Pierre-Louis Bossart 2019-10-09 16:01 ` Srinivas Kandagatla 0 siblings, 2 replies; 44+ messages in thread From: Pierre-Louis Bossart @ 2019-10-09 14:29 UTC (permalink / raw) To: Srinivas Kandagatla, Vinod Koul, Mark Brown Cc: devicetree, alsa-devel, bgoswami, plai, linux-kernel, lgirdwood, robh+dt, spapothi On 10/9/19 3:32 AM, Srinivas Kandagatla wrote: > Hi Pierre, > > On 14/08/2019 15:09, Pierre-Louis Bossart wrote: >> >> >> On 8/13/19 11:11 PM, Vinod Koul wrote: >>> On 13-08-19, 20:58, Mark Brown wrote: >>>> On Tue, Aug 13, 2019 at 02:38:53PM -0500, Pierre-Louis Bossart wrote: >>>> >>>>> Indeed. I don't have a full understanding of that part to be >>>>> honest, nor why >>>>> we need something SoundWire-specific. We already abused the >>>>> set_tdm_slot API >>>>> to store an HDaudio stream, now we have a rather confusing stream >>>>> information for SoundWire and I have about 3 other 'stream' >>>>> contexts in >>>>> SOF... I am still doing basic cleanups but this has been on my >>>>> radar for a >>>>> while. >>>> >>>> There is something to be said for not abusing the TDM slot API if it >>>> can >>>> make things clearer by using bus-idiomatic mechanisms, but it does mean >>>> everything needs to know about each individual bus :/ . >>> >>> Here ASoC doesn't need to know about sdw bus. As Srini explained, this >>> helps in the case for him to get the stream context and set the stream >>> context from the machine driver. >>> >>> Nothing else is expected to be done from this API. We already do a set >>> using snd_soc_dai_set_sdw_stream(). Here we add the >>> snd_soc_dai_get_sdw_stream() to query >> >> I didn't see a call to snd_soc_dai_set_sdw_stream() in Srini's code? > > > There is a snd_soc_dai_get_sdw_stream() to get stream context and we add > slave streams(amplifier in this case) to that context using > sdw_stream_add_slave() in machine driver[1]. > > Without this helper there is no way to link slave streams to stream > context in non dai based setup like smart speaker amplifiers. > > Currently this driver is blocked on this patch, If you think there are > other ways to do this, am happy to try them out. So to be clear, you are *not* using snd_soc_dai_set_sdw_stream? _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org https://mailman.alsa-project.org/mailman/listinfo/alsa-devel ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [alsa-devel] [PATCH v2 3/5] ASoC: core: add support to snd_soc_dai_get_sdw_stream() 2019-10-09 14:29 ` Pierre-Louis Bossart @ 2019-10-09 14:29 ` Pierre-Louis Bossart 2019-10-09 16:01 ` Srinivas Kandagatla 1 sibling, 0 replies; 44+ messages in thread From: Pierre-Louis Bossart @ 2019-10-09 14:29 UTC (permalink / raw) To: Srinivas Kandagatla, Vinod Koul, Mark Brown Cc: devicetree, alsa-devel, bgoswami, plai, linux-kernel, lgirdwood, robh+dt, spapothi On 10/9/19 3:32 AM, Srinivas Kandagatla wrote: > Hi Pierre, > > On 14/08/2019 15:09, Pierre-Louis Bossart wrote: >> >> >> On 8/13/19 11:11 PM, Vinod Koul wrote: >>> On 13-08-19, 20:58, Mark Brown wrote: >>>> On Tue, Aug 13, 2019 at 02:38:53PM -0500, Pierre-Louis Bossart wrote: >>>> >>>>> Indeed. I don't have a full understanding of that part to be >>>>> honest, nor why >>>>> we need something SoundWire-specific. We already abused the >>>>> set_tdm_slot API >>>>> to store an HDaudio stream, now we have a rather confusing stream >>>>> information for SoundWire and I have about 3 other 'stream' >>>>> contexts in >>>>> SOF... I am still doing basic cleanups but this has been on my >>>>> radar for a >>>>> while. >>>> >>>> There is something to be said for not abusing the TDM slot API if it >>>> can >>>> make things clearer by using bus-idiomatic mechanisms, but it does mean >>>> everything needs to know about each individual bus :/ . >>> >>> Here ASoC doesn't need to know about sdw bus. As Srini explained, this >>> helps in the case for him to get the stream context and set the stream >>> context from the machine driver. >>> >>> Nothing else is expected to be done from this API. We already do a set >>> using snd_soc_dai_set_sdw_stream(). Here we add the >>> snd_soc_dai_get_sdw_stream() to query >> >> I didn't see a call to snd_soc_dai_set_sdw_stream() in Srini's code? > > > There is a snd_soc_dai_get_sdw_stream() to get stream context and we add > slave streams(amplifier in this case) to that context using > sdw_stream_add_slave() in machine driver[1]. > > Without this helper there is no way to link slave streams to stream > context in non dai based setup like smart speaker amplifiers. > > Currently this driver is blocked on this patch, If you think there are > other ways to do this, am happy to try them out. So to be clear, you are *not* using snd_soc_dai_set_sdw_stream? _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org https://mailman.alsa-project.org/mailman/listinfo/alsa-devel ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [alsa-devel] [PATCH v2 3/5] ASoC: core: add support to snd_soc_dai_get_sdw_stream() 2019-10-09 14:29 ` Pierre-Louis Bossart 2019-10-09 14:29 ` Pierre-Louis Bossart @ 2019-10-09 16:01 ` Srinivas Kandagatla 2019-10-09 16:01 ` Srinivas Kandagatla 2019-10-09 18:53 ` Pierre-Louis Bossart 1 sibling, 2 replies; 44+ messages in thread From: Srinivas Kandagatla @ 2019-10-09 16:01 UTC (permalink / raw) To: Pierre-Louis Bossart, Vinod Koul, Mark Brown Cc: devicetree, alsa-devel, bgoswami, plai, linux-kernel, lgirdwood, robh+dt, spapothi On 09/10/2019 15:29, Pierre-Louis Bossart wrote: > > > On 10/9/19 3:32 AM, Srinivas Kandagatla wrote: >> Hi Pierre, >> >> On 14/08/2019 15:09, Pierre-Louis Bossart wrote: >>> >>> >>> On 8/13/19 11:11 PM, Vinod Koul wrote: >>>> On 13-08-19, 20:58, Mark Brown wrote: >>>>> On Tue, Aug 13, 2019 at 02:38:53PM -0500, Pierre-Louis Bossart wrote: >>>>> >>>>>> Indeed. I don't have a full understanding of that part to be >>>>>> honest, nor why >>>>>> we need something SoundWire-specific. We already abused the >>>>>> set_tdm_slot API >>>>>> to store an HDaudio stream, now we have a rather confusing stream >>>>>> information for SoundWire and I have about 3 other 'stream' >>>>>> contexts in >>>>>> SOF... I am still doing basic cleanups but this has been on my >>>>>> radar for a >>>>>> while. >>>>> >>>>> There is something to be said for not abusing the TDM slot API if >>>>> it can >>>>> make things clearer by using bus-idiomatic mechanisms, but it does >>>>> mean >>>>> everything needs to know about each individual bus :/ . >>>> >>>> Here ASoC doesn't need to know about sdw bus. As Srini explained, this >>>> helps in the case for him to get the stream context and set the stream >>>> context from the machine driver. >>>> >>>> Nothing else is expected to be done from this API. We already do a set >>>> using snd_soc_dai_set_sdw_stream(). Here we add the >>>> snd_soc_dai_get_sdw_stream() to query >>> >>> I didn't see a call to snd_soc_dai_set_sdw_stream() in Srini's code? >> >> >> There is a snd_soc_dai_get_sdw_stream() to get stream context and we >> add slave streams(amplifier in this case) to that context using >> sdw_stream_add_slave() in machine driver[1]. >> >> Without this helper there is no way to link slave streams to stream >> context in non dai based setup like smart speaker amplifiers. >> >> Currently this driver is blocked on this patch, If you think there are >> other ways to do this, am happy to try them out. > > So to be clear, you are *not* using snd_soc_dai_set_sdw_stream? Yes, am not using snd_soc_dai_set_sdw_stream(). --srini > > > > > _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org https://mailman.alsa-project.org/mailman/listinfo/alsa-devel ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [alsa-devel] [PATCH v2 3/5] ASoC: core: add support to snd_soc_dai_get_sdw_stream() 2019-10-09 16:01 ` Srinivas Kandagatla @ 2019-10-09 16:01 ` Srinivas Kandagatla 2019-10-09 18:53 ` Pierre-Louis Bossart 1 sibling, 0 replies; 44+ messages in thread From: Srinivas Kandagatla @ 2019-10-09 16:01 UTC (permalink / raw) To: Pierre-Louis Bossart, Vinod Koul, Mark Brown Cc: devicetree, alsa-devel, bgoswami, plai, linux-kernel, lgirdwood, robh+dt, spapothi On 09/10/2019 15:29, Pierre-Louis Bossart wrote: > > > On 10/9/19 3:32 AM, Srinivas Kandagatla wrote: >> Hi Pierre, >> >> On 14/08/2019 15:09, Pierre-Louis Bossart wrote: >>> >>> >>> On 8/13/19 11:11 PM, Vinod Koul wrote: >>>> On 13-08-19, 20:58, Mark Brown wrote: >>>>> On Tue, Aug 13, 2019 at 02:38:53PM -0500, Pierre-Louis Bossart wrote: >>>>> >>>>>> Indeed. I don't have a full understanding of that part to be >>>>>> honest, nor why >>>>>> we need something SoundWire-specific. We already abused the >>>>>> set_tdm_slot API >>>>>> to store an HDaudio stream, now we have a rather confusing stream >>>>>> information for SoundWire and I have about 3 other 'stream' >>>>>> contexts in >>>>>> SOF... I am still doing basic cleanups but this has been on my >>>>>> radar for a >>>>>> while. >>>>> >>>>> There is something to be said for not abusing the TDM slot API if >>>>> it can >>>>> make things clearer by using bus-idiomatic mechanisms, but it does >>>>> mean >>>>> everything needs to know about each individual bus :/ . >>>> >>>> Here ASoC doesn't need to know about sdw bus. As Srini explained, this >>>> helps in the case for him to get the stream context and set the stream >>>> context from the machine driver. >>>> >>>> Nothing else is expected to be done from this API. We already do a set >>>> using snd_soc_dai_set_sdw_stream(). Here we add the >>>> snd_soc_dai_get_sdw_stream() to query >>> >>> I didn't see a call to snd_soc_dai_set_sdw_stream() in Srini's code? >> >> >> There is a snd_soc_dai_get_sdw_stream() to get stream context and we >> add slave streams(amplifier in this case) to that context using >> sdw_stream_add_slave() in machine driver[1]. >> >> Without this helper there is no way to link slave streams to stream >> context in non dai based setup like smart speaker amplifiers. >> >> Currently this driver is blocked on this patch, If you think there are >> other ways to do this, am happy to try them out. > > So to be clear, you are *not* using snd_soc_dai_set_sdw_stream? Yes, am not using snd_soc_dai_set_sdw_stream(). --srini > > > > > _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org https://mailman.alsa-project.org/mailman/listinfo/alsa-devel ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [alsa-devel] [PATCH v2 3/5] ASoC: core: add support to snd_soc_dai_get_sdw_stream() 2019-10-09 16:01 ` Srinivas Kandagatla 2019-10-09 16:01 ` Srinivas Kandagatla @ 2019-10-09 18:53 ` Pierre-Louis Bossart 2019-10-10 8:50 ` Srinivas Kandagatla 1 sibling, 1 reply; 44+ messages in thread From: Pierre-Louis Bossart @ 2019-10-09 18:53 UTC (permalink / raw) To: Srinivas Kandagatla, Vinod Koul, Mark Brown Cc: devicetree, alsa-devel, bgoswami, plai, lgirdwood, linux-kernel, robh+dt, spapothi On 10/9/19 11:01 AM, Srinivas Kandagatla wrote: > > > On 09/10/2019 15:29, Pierre-Louis Bossart wrote: >> >> >> On 10/9/19 3:32 AM, Srinivas Kandagatla wrote: >>> Hi Pierre, >>> >>> On 14/08/2019 15:09, Pierre-Louis Bossart wrote: >>>> >>>> >>>> On 8/13/19 11:11 PM, Vinod Koul wrote: >>>>> On 13-08-19, 20:58, Mark Brown wrote: >>>>>> On Tue, Aug 13, 2019 at 02:38:53PM -0500, Pierre-Louis Bossart wrote: >>>>>> >>>>>>> Indeed. I don't have a full understanding of that part to be >>>>>>> honest, nor why >>>>>>> we need something SoundWire-specific. We already abused the >>>>>>> set_tdm_slot API >>>>>>> to store an HDaudio stream, now we have a rather confusing stream >>>>>>> information for SoundWire and I have about 3 other 'stream' >>>>>>> contexts in >>>>>>> SOF... I am still doing basic cleanups but this has been on my >>>>>>> radar for a >>>>>>> while. >>>>>> >>>>>> There is something to be said for not abusing the TDM slot API if >>>>>> it can >>>>>> make things clearer by using bus-idiomatic mechanisms, but it does >>>>>> mean >>>>>> everything needs to know about each individual bus :/ . >>>>> >>>>> Here ASoC doesn't need to know about sdw bus. As Srini explained, this >>>>> helps in the case for him to get the stream context and set the stream >>>>> context from the machine driver. >>>>> >>>>> Nothing else is expected to be done from this API. We already do a set >>>>> using snd_soc_dai_set_sdw_stream(). Here we add the >>>>> snd_soc_dai_get_sdw_stream() to query >>>> >>>> I didn't see a call to snd_soc_dai_set_sdw_stream() in Srini's code? >>> >>> >>> There is a snd_soc_dai_get_sdw_stream() to get stream context and we >>> add slave streams(amplifier in this case) to that context using >>> sdw_stream_add_slave() in machine driver[1]. >>> >>> Without this helper there is no way to link slave streams to stream >>> context in non dai based setup like smart speaker amplifiers. >>> >>> Currently this driver is blocked on this patch, If you think there >>> are other ways to do this, am happy to try them out. >> >> So to be clear, you are *not* using snd_soc_dai_set_sdw_stream? > Yes, am not using snd_soc_dai_set_sdw_stream(). It's been a while since this thread started, and I still don't quite get the concepts or logic. First, I don't understand what the problem with "aux" devices is. All the SoundWire stuff is based on the concept of DAI, so I guess I am missing why introducing the "aux" device changes anything. Next, a 'stream' connects multiple DAIs to transmit information from sources to sinks. A DAI in itself is created without having any notion of which stream it will be associated with. This can only be done with machine level information. If you query a DAI to get a stream context, then how is this stream context allocated in the first place? It looks like a horse and cart problem. Or you are assuming that a specific DAI provides the context, and that all other DAIs do not expose this .get_sdw_stream()? What if more that 1 DAI provide a stream context? And last, I am even more lost since w/ the existing codec drivers we have, sdw_stream_add_slave() is called from the dai .hw_params callback, once you have information on formats/rates, etc. using sdw_stream_add_slave() from a machine driver looks like an even bigger stretch. I really thought the machine driver would only propagate the notion of stream to all DAIs that are part of the same dailink. I am not trying to block the Qualcomm implementation, just would like to make sure the assumptions are clear and that we are not using the same API in completely different ways. _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org https://mailman.alsa-project.org/mailman/listinfo/alsa-devel ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [alsa-devel] [PATCH v2 3/5] ASoC: core: add support to snd_soc_dai_get_sdw_stream() 2019-10-09 18:53 ` Pierre-Louis Bossart @ 2019-10-10 8:50 ` Srinivas Kandagatla 2019-10-10 12:03 ` Charles Keepax 0 siblings, 1 reply; 44+ messages in thread From: Srinivas Kandagatla @ 2019-10-10 8:50 UTC (permalink / raw) To: Pierre-Louis Bossart, Vinod Koul, Mark Brown Cc: devicetree, alsa-devel, bgoswami, plai, lgirdwood, linux-kernel, robh+dt, spapothi On 09/10/2019 19:53, Pierre-Louis Bossart wrote: > > > On 10/9/19 11:01 AM, Srinivas Kandagatla wrote: >> >> >> On 09/10/2019 15:29, Pierre-Louis Bossart wrote: >>> >>> >>> On 10/9/19 3:32 AM, Srinivas Kandagatla wrote: >>>> Hi Pierre, >>>> >>>> On 14/08/2019 15:09, Pierre-Louis Bossart wrote: >>>>> >>>>> >>>>> On 8/13/19 11:11 PM, Vinod Koul wrote: >>>>>> On 13-08-19, 20:58, Mark Brown wrote: >>>>>>> On Tue, Aug 13, 2019 at 02:38:53PM -0500, Pierre-Louis Bossart >>>>>>> wrote: >>>>>>> >>>>>>>> Indeed. I don't have a full understanding of that part to be >>>>>>>> honest, nor why >>>>>>>> we need something SoundWire-specific. We already abused the >>>>>>>> set_tdm_slot API >>>>>>>> to store an HDaudio stream, now we have a rather confusing stream >>>>>>>> information for SoundWire and I have about 3 other 'stream' >>>>>>>> contexts in >>>>>>>> SOF... I am still doing basic cleanups but this has been on my >>>>>>>> radar for a >>>>>>>> while. >>>>>>> >>>>>>> There is something to be said for not abusing the TDM slot API if >>>>>>> it can >>>>>>> make things clearer by using bus-idiomatic mechanisms, but it >>>>>>> does mean >>>>>>> everything needs to know about each individual bus :/ . >>>>>> >>>>>> Here ASoC doesn't need to know about sdw bus. As Srini explained, >>>>>> this >>>>>> helps in the case for him to get the stream context and set the >>>>>> stream >>>>>> context from the machine driver. >>>>>> >>>>>> Nothing else is expected to be done from this API. We already do a >>>>>> set >>>>>> using snd_soc_dai_set_sdw_stream(). Here we add the >>>>>> snd_soc_dai_get_sdw_stream() to query >>>>> >>>>> I didn't see a call to snd_soc_dai_set_sdw_stream() in Srini's code? >>>> >>>> >>>> There is a snd_soc_dai_get_sdw_stream() to get stream context and we >>>> add slave streams(amplifier in this case) to that context using >>>> sdw_stream_add_slave() in machine driver[1]. >>>> >>>> Without this helper there is no way to link slave streams to stream >>>> context in non dai based setup like smart speaker amplifiers. >>>> >>>> Currently this driver is blocked on this patch, If you think there >>>> are other ways to do this, am happy to try them out. >>> >>> So to be clear, you are *not* using snd_soc_dai_set_sdw_stream? >> Yes, am not using snd_soc_dai_set_sdw_stream(). > > It's been a while since this thread started, and I still don't quite get > the concepts or logic. > > First, I don't understand what the problem with "aux" devices is. All > the SoundWire stuff is based on the concept of DAI, so I guess I am That is the actual problem! Some aux devices does not have dais. > missing why introducing the "aux" device changes anything. > Problem is that aux devices(amplifier) are dai less but connected via SoundWire. So question is how do we attach those SoundWire streams to SoundWire master stream? My Idea was to get handle to the SoundWire stream from controller dai and adding these aux SoundWire slave devices as slave to them in machine driver. This was much less intrusive than other solutions. Is there a better way to associate a dai-less codecs to SoundWire master stream? > Next, a 'stream' connects multiple DAIs to transmit information from > sources to sinks. A DAI in itself is created without having any notion > of which stream it will be associated with. This can only be done with > machine level information. > > If you query a DAI to get a stream context, then how is this stream > context allocated in the first place? It looks like a horse and cart > problem. Or you are assuming that a specific DAI provides the context, > and that all other DAIs do not expose this .get_sdw_stream()? What if > more that 1 DAI provide a stream context? In this particular board setup. Soundwire master is allocating the stream runtime for each of it dais, and this runtime is used in machine driver to attach Auxdev Soundwire slaves. Other setups where the codec has dai should work as expected! > > And last, I am even more lost since w/ the existing codec drivers we > have, sdw_stream_add_slave() is called from the dai .hw_params callback, > once you have information on formats/rates, etc. using > sdw_stream_add_slave() from a machine driver looks like an even bigger > stretch. I really thought the machine driver would only propagate the > notion of stream to all DAIs that are part of the same dailink. > > I am not trying to block the Qualcomm implementation, just would like to > make sure the assumptions are clear and that we are not using the same > API in completely different ways. Am open for any suggestions on dealing with dai less setups like what we have on our board. --srini > > > > _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org https://mailman.alsa-project.org/mailman/listinfo/alsa-devel ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [alsa-devel] [PATCH v2 3/5] ASoC: core: add support to snd_soc_dai_get_sdw_stream() 2019-10-10 8:50 ` Srinivas Kandagatla @ 2019-10-10 12:03 ` Charles Keepax 2019-10-10 13:51 ` Mark Brown 2019-10-10 14:01 ` Pierre-Louis Bossart 0 siblings, 2 replies; 44+ messages in thread From: Charles Keepax @ 2019-10-10 12:03 UTC (permalink / raw) To: Srinivas Kandagatla Cc: devicetree, alsa-devel, bgoswami, linux-kernel, plai, Pierre-Louis Bossart, robh+dt, lgirdwood, Vinod Koul, Mark Brown, spapothi On Thu, Oct 10, 2019 at 09:50:22AM +0100, Srinivas Kandagatla wrote: > On 09/10/2019 19:53, Pierre-Louis Bossart wrote: > >On 10/9/19 11:01 AM, Srinivas Kandagatla wrote: > >>On 09/10/2019 15:29, Pierre-Louis Bossart wrote: > >>>On 10/9/19 3:32 AM, Srinivas Kandagatla wrote: > >>>>On 14/08/2019 15:09, Pierre-Louis Bossart wrote: > >>>>>On 8/13/19 11:11 PM, Vinod Koul wrote: > >>>>>>On 13-08-19, 20:58, Mark Brown wrote: > >>>>>>>On Tue, Aug 13, 2019 at 02:38:53PM -0500, Pierre-Louis > >>>>>>>>Indeed. I don't have a full understanding of that > >>>>>>>>part to be honest, nor why > >>>>>>>>we need something SoundWire-specific. We already > >>>>>>>>abused the set_tdm_slot API > >>>>>>>>to store an HDaudio stream, now we have a rather confusing stream > >>>>>>>>information for SoundWire and I have about 3 other > >>>>>>>>'stream' contexts in > >>>>>>>>SOF... I am still doing basic cleanups but this has > >>>>>>>>been on my radar for a > >>>>>>>>while. > >>>>>>> > >>>>>>>There is something to be said for not abusing the TDM > >>>>>>>slot API if it can > >>>>>>>make things clearer by using bus-idiomatic mechanisms, > >>>>>>>but it does mean > >>>>>>>everything needs to know about each individual bus :/ . > >>>>>> > >>>>>>Here ASoC doesn't need to know about sdw bus. As Srini > >>>>>>explained, this > >>>>>>helps in the case for him to get the stream context and > >>>>>>set the stream > >>>>>>context from the machine driver. > >>>>>> > >>>>>>Nothing else is expected to be done from this API. We > >>>>>>already do a set > >>>>>>using snd_soc_dai_set_sdw_stream(). Here we add the > >>>>>>snd_soc_dai_get_sdw_stream() to query > >>>>> > >>>>>I didn't see a call to snd_soc_dai_set_sdw_stream() in Srini's code? > >>>> > >>>> > >>>>There is a snd_soc_dai_get_sdw_stream() to get stream > >>>>context and we add slave streams(amplifier in this case) to > >>>>that context using sdw_stream_add_slave() in machine > >>>>driver[1]. > >>>> > >>>>Without this helper there is no way to link slave streams to > >>>>stream context in non dai based setup like smart speaker > >>>>amplifiers. > >>>> > >>>>Currently this driver is blocked on this patch, If you think > >>>>there are other ways to do this, am happy to try them out. > >>> > >>>So to be clear, you are *not* using snd_soc_dai_set_sdw_stream? > >>Yes, am not using snd_soc_dai_set_sdw_stream(). > > > >It's been a while since this thread started, and I still don't > >quite get the concepts or logic. > > > >First, I don't understand what the problem with "aux" devices is. > >All the SoundWire stuff is based on the concept of DAI, so I guess > >I am > > That is the actual problem! Some aux devices does not have dais. > Usually aux devices are used for things like analog amplifiers that clearly don't have a digital interface, thus it really makes no sense to have a DAI link connecting them. So I guess my question here would be what is the thinking on making the "smart amplifier" dailess? It feels like having a CODEC to CODEC DAI between the CODEC and the AMP would be a more obvious way to connect the two devices and would presumably avoid the issues being discussed around the patch. Thanks, Charles _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org https://mailman.alsa-project.org/mailman/listinfo/alsa-devel ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [alsa-devel] [PATCH v2 3/5] ASoC: core: add support to snd_soc_dai_get_sdw_stream() 2019-10-10 12:03 ` Charles Keepax @ 2019-10-10 13:51 ` Mark Brown 2019-10-10 14:01 ` Pierre-Louis Bossart 1 sibling, 0 replies; 44+ messages in thread From: Mark Brown @ 2019-10-10 13:51 UTC (permalink / raw) To: Charles Keepax Cc: devicetree, alsa-devel, bgoswami, linux-kernel, plai, Pierre-Louis Bossart, lgirdwood, Vinod Koul, robh+dt, Srinivas Kandagatla, spapothi [-- Attachment #1.1: Type: text/plain, Size: 724 bytes --] On Thu, Oct 10, 2019 at 12:03:37PM +0000, Charles Keepax wrote: > Usually aux devices are used for things like analog amplifiers that > clearly don't have a digital interface, thus it really makes no sense > to have a DAI link connecting them. So I guess my question here > would be what is the thinking on making the "smart amplifier" dailess? > It feels like having a CODEC to CODEC DAI between the CODEC and > the AMP would be a more obvious way to connect the two devices > and would presumably avoid the issues being discussed around the > patch. Or is this a device where for some reason (consistency?) there really is no DAI and someone has decided to make an analogue amplifier with a soundwire control interface? [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] [-- Attachment #2: Type: text/plain, Size: 161 bytes --] _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org https://mailman.alsa-project.org/mailman/listinfo/alsa-devel ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [alsa-devel] [PATCH v2 3/5] ASoC: core: add support to snd_soc_dai_get_sdw_stream() 2019-10-10 12:03 ` Charles Keepax 2019-10-10 13:51 ` Mark Brown @ 2019-10-10 14:01 ` Pierre-Louis Bossart 2019-10-10 14:52 ` Srinivas Kandagatla 1 sibling, 1 reply; 44+ messages in thread From: Pierre-Louis Bossart @ 2019-10-10 14:01 UTC (permalink / raw) To: Charles Keepax, Srinivas Kandagatla Cc: devicetree, alsa-devel, bgoswami, plai, linux-kernel, lgirdwood, Vinod Koul, robh+dt, Mark Brown, spapothi >>> It's been a while since this thread started, and I still don't >>> quite get the concepts or logic. >>> >>> First, I don't understand what the problem with "aux" devices is. >>> All the SoundWire stuff is based on the concept of DAI, so I guess >>> I am >> >> That is the actual problem! Some aux devices does not have dais. >> > > Usually aux devices are used for things like analog amplifiers that > clearly don't have a digital interface, thus it really makes no sense > to have a DAI link connecting them. So I guess my question here > would be what is the thinking on making the "smart amplifier" dailess? > It feels like having a CODEC to CODEC DAI between the CODEC and > the AMP would be a more obvious way to connect the two devices > and would presumably avoid the issues being discussed around the > patch. Ah, now I get it, I missed the point about not having DAIs for the amplifier. I will second Charles' point, the code you have in the machine driver at [1] gets a SoundWire stream context from the SLIMbus codec dai. It's a bit odd to mix layers this way. And if I look at the code below, taken from [1], if you have more than one codec, then your code looks problematic: data->sruntime would be overriden and you'd use the info from the last codec dai on the dailink... case SLIMBUS_0_RX...SLIMBUS_6_TX: for (i = 0 ; i < dai_link->num_codecs; i++) { [snip] data->sruntime[cpu_dai->id] = snd_soc_dai_get_sdw_stream(rtd->codec_dais[i], 0); << same destination for multiple codec_dais... If you keep the amp dai-less, then the stream concept should be somehow allocated on the master (or machine) side and passed to the codec dais that are associated to the same 'stream'. [1] https://git.linaro.org/people/srinivas.kandagatla/linux.git/tree/sound/soc/qcom/db845c.c?h=release/db845c/qcomlt-5.2 _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org https://mailman.alsa-project.org/mailman/listinfo/alsa-devel ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [alsa-devel] [PATCH v2 3/5] ASoC: core: add support to snd_soc_dai_get_sdw_stream() 2019-10-10 14:01 ` Pierre-Louis Bossart @ 2019-10-10 14:52 ` Srinivas Kandagatla 2019-10-10 15:49 ` Pierre-Louis Bossart 0 siblings, 1 reply; 44+ messages in thread From: Srinivas Kandagatla @ 2019-10-10 14:52 UTC (permalink / raw) To: Pierre-Louis Bossart, Charles Keepax Cc: devicetree, alsa-devel, bgoswami, plai, linux-kernel, lgirdwood, Vinod Koul, robh+dt, Mark Brown, spapothi On 10/10/2019 15:01, Pierre-Louis Bossart wrote: > >>>> It's been a while since this thread started, and I still don't >>>> quite get the concepts or logic. >>>> >>>> First, I don't understand what the problem with "aux" devices is. >>>> All the SoundWire stuff is based on the concept of DAI, so I guess >>>> I am >>> >>> That is the actual problem! Some aux devices does not have dais. >>> >> >> Usually aux devices are used for things like analog amplifiers that >> clearly don't have a digital interface, thus it really makes no sense >> to have a DAI link connecting them. So I guess my question here >> would be what is the thinking on making the "smart amplifier" dailess? >> It feels like having a CODEC to CODEC DAI between the CODEC and >> the AMP would be a more obvious way to connect the two devices >> and would presumably avoid the issues being discussed around the >> patch. > > Ah, now I get it, I missed the point about not having DAIs for the > amplifier. > > I will second Charles' point, the code you have in the machine driver at I agree with Charles, WSA8810/WSA8815 is connected via SoundWire digital interface, so I can try to model this amplifier with dais and see how it ends up. I still need to figure out prefixing multiple instances of this Amplifier controls with "Left" and "Right" > [1] gets a SoundWire stream context from the SLIMbus codec dai. It's a > bit odd to mix layers this way. Yep we have a very mixed setup on this SoC. So it looks like this. Main WCD934X Codec which is connected via SLIMBus which has SoundWire Controller block along with other analog + digital blocks. Again the SoundWire Controller from that WCD934X codec is wired up to WSA881X Smart speaker amplifiers. > > > And if I look at the code below, taken from [1], if you have more than > one codec, then your code looks problematic: data->sruntime would be > overriden and you'd use the info from the last codec dai on the dailink... This code has been written very much specific to DB845c which has only one codec. But I agree with your point. --srini > > case SLIMBUS_0_RX...SLIMBUS_6_TX: > for (i = 0 ; i < dai_link->num_codecs; i++) { > [snip] > data->sruntime[cpu_dai->id] = > snd_soc_dai_get_sdw_stream(rtd->codec_dais[i], 0); << same > destination for multiple codec_dais... > > If you keep the amp dai-less, then the stream concept should be somehow > allocated on the master (or machine) side and passed to the codec dais > that are associated to the same 'stream'. > > > [1] > https://git.linaro.org/people/srinivas.kandagatla/linux.git/tree/sound/soc/qcom/db845c.c?h=release/db845c/qcomlt-5.2 > > _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org https://mailman.alsa-project.org/mailman/listinfo/alsa-devel ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [alsa-devel] [PATCH v2 3/5] ASoC: core: add support to snd_soc_dai_get_sdw_stream() 2019-10-10 14:52 ` Srinivas Kandagatla @ 2019-10-10 15:49 ` Pierre-Louis Bossart 2019-10-11 12:30 ` Srinivas Kandagatla 0 siblings, 1 reply; 44+ messages in thread From: Pierre-Louis Bossart @ 2019-10-10 15:49 UTC (permalink / raw) To: Srinivas Kandagatla, Charles Keepax Cc: devicetree, alsa-devel, bgoswami, plai, linux-kernel, lgirdwood, Vinod Koul, robh+dt, Mark Brown, spapothi > I still need to figure out prefixing multiple instances of this > Amplifier controls with "Left" and "Right" FWIW we use the "snd_codec_conf" stuff to add a prefix for each amplifier, so that the controls are not mixed up between instances of the same amp, see e.g. static struct snd_soc_codec_conf codec_conf[] = { { .dev_name = "sdw:0:25d:711:0:1", .name_prefix = "rt711", }, { .dev_name = "sdw:1:25d:1308:0:0", .name_prefix = "rt1308-1", }, { .dev_name = "sdw:2:25d:1308:0:2", .name_prefix = "rt1308-2", }, { .dev_name = "sdw:3:25d:715:0:1", .name_prefix = "rt715", }, }; https://github.com/thesofproject/linux/pull/1142/commits/9ff9cf9d8994333df2250641c95431261bc66d69#diff-892560f80d603420baec7395e0b45d81R212 _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org https://mailman.alsa-project.org/mailman/listinfo/alsa-devel ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [alsa-devel] [PATCH v2 3/5] ASoC: core: add support to snd_soc_dai_get_sdw_stream() 2019-10-10 15:49 ` Pierre-Louis Bossart @ 2019-10-11 12:30 ` Srinivas Kandagatla 0 siblings, 0 replies; 44+ messages in thread From: Srinivas Kandagatla @ 2019-10-11 12:30 UTC (permalink / raw) To: Pierre-Louis Bossart, Charles Keepax Cc: devicetree, alsa-devel, bgoswami, plai, linux-kernel, lgirdwood, Vinod Koul, robh+dt, Mark Brown, spapothi On 10/10/2019 16:49, Pierre-Louis Bossart wrote: > > >> I still need to figure out prefixing multiple instances of this >> Amplifier controls with "Left" and "Right" > > FWIW we use the "snd_codec_conf" stuff to add a prefix for each > amplifier, so that the controls are not mixed up between instances of > the same amp, see e.g. > Thanks Pierre for the inputs. Am using Documentation/devicetree/bindings/sound/name-prefix.txt for dt and it works! I will send new set of patches by dropping this patch! --srini > > static struct snd_soc_codec_conf codec_conf[] = { > { > .dev_name = "sdw:0:25d:711:0:1", > .name_prefix = "rt711", > }, > { > .dev_name = "sdw:1:25d:1308:0:0", > .name_prefix = "rt1308-1", > }, > { > .dev_name = "sdw:2:25d:1308:0:2", > .name_prefix = "rt1308-2", > }, > { > .dev_name = "sdw:3:25d:715:0:1", > .name_prefix = "rt715", > }, > }; > > > https://github.com/thesofproject/linux/pull/1142/commits/9ff9cf9d8994333df2250641c95431261bc66d69#diff-892560f80d603420baec7395e0b45d81R212 > _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org https://mailman.alsa-project.org/mailman/listinfo/alsa-devel ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v2 3/5] ASoC: core: add support to snd_soc_dai_get_sdw_stream() 2019-08-13 8:35 ` [PATCH v2 3/5] ASoC: core: add support to snd_soc_dai_get_sdw_stream() Srinivas Kandagatla 2019-08-13 14:44 ` [alsa-devel] " Pierre-Louis Bossart @ 2019-08-13 16:03 ` Cezary Rojewski 2019-08-13 16:52 ` Srinivas Kandagatla 1 sibling, 1 reply; 44+ messages in thread From: Cezary Rojewski @ 2019-08-13 16:03 UTC (permalink / raw) To: Srinivas Kandagatla Cc: vkoul, broonie, bgoswami, plai, pierre-louis.bossart, robh+dt, devicetree, lgirdwood, alsa-devel, linux-kernel, spapothi On 2019-08-13 10:35, Srinivas Kandagatla wrote: > On platforms which have smart speaker amplifiers connected via > soundwire and modeled as aux devices in ASoC, in such usecases machine > driver should be able to get sdw master stream from dai so that it can > use the runtime stream to setup slave streams. > > soundwire already as a set function, get function would provide more > flexibility to above configurations. > > Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> > --- > include/sound/soc-dai.h | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h > index dc48fe081a20..1e01f4a302e0 100644 > --- a/include/sound/soc-dai.h > +++ b/include/sound/soc-dai.h > @@ -202,6 +202,7 @@ struct snd_soc_dai_ops { > > int (*set_sdw_stream)(struct snd_soc_dai *dai, > void *stream, int direction); > + void *(*get_sdw_stream)(struct snd_soc_dai *dai, int direction); > /* > * DAI digital mute - optional. > * Called by soc-core to minimise any pops. > @@ -410,4 +411,13 @@ static inline int snd_soc_dai_set_sdw_stream(struct snd_soc_dai *dai, > return -ENOTSUPP; > } > > +static inline void *snd_soc_dai_get_sdw_stream(struct snd_soc_dai *dai, > + int direction) > +{ > + if (dai->driver->ops->get_sdw_stream) > + return dai->driver->ops->get_sdw_stream(dai, direction); > + else > + return ERR_PTR(-ENOTSUPP); > +} Drop redundant else. ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v2 3/5] ASoC: core: add support to snd_soc_dai_get_sdw_stream() 2019-08-13 16:03 ` Cezary Rojewski @ 2019-08-13 16:52 ` Srinivas Kandagatla 2019-08-13 17:29 ` Cezary Rojewski 0 siblings, 1 reply; 44+ messages in thread From: Srinivas Kandagatla @ 2019-08-13 16:52 UTC (permalink / raw) To: Cezary Rojewski Cc: vkoul, broonie, bgoswami, plai, pierre-louis.bossart, robh+dt, devicetree, lgirdwood, alsa-devel, linux-kernel, spapothi Thanks for the review, On 13/08/2019 17:03, Cezary Rojewski wrote: > On 2019-08-13 10:35, Srinivas Kandagatla wrote: >> On platforms which have smart speaker amplifiers connected via >> soundwire and modeled as aux devices in ASoC, in such usecases machine >> driver should be able to get sdw master stream from dai so that it can >> use the runtime stream to setup slave streams. >> >> soundwire already as a set function, get function would provide more >> flexibility to above configurations. >> >> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> >> --- >> include/sound/soc-dai.h | 10 ++++++++++ >> 1 file changed, 10 insertions(+) >> >> diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h >> index dc48fe081a20..1e01f4a302e0 100644 >> --- a/include/sound/soc-dai.h >> +++ b/include/sound/soc-dai.h >> @@ -202,6 +202,7 @@ struct snd_soc_dai_ops { >> int (*set_sdw_stream)(struct snd_soc_dai *dai, >> void *stream, int direction); >> + void *(*get_sdw_stream)(struct snd_soc_dai *dai, int direction); >> /* >> * DAI digital mute - optional. >> * Called by soc-core to minimise any pops. >> @@ -410,4 +411,13 @@ static inline int >> snd_soc_dai_set_sdw_stream(struct snd_soc_dai *dai, >> return -ENOTSUPP; >> } >> +static inline void *snd_soc_dai_get_sdw_stream(struct snd_soc_dai *dai, >> + int direction) >> +{ >> + if (dai->driver->ops->get_sdw_stream) >> + return dai->driver->ops->get_sdw_stream(dai, direction); >> + else >> + return ERR_PTR(-ENOTSUPP); >> +} > > Drop redundant else. Not all the dai drivers would implement this function, I guess else is not redundant here! --srini > ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v2 3/5] ASoC: core: add support to snd_soc_dai_get_sdw_stream() 2019-08-13 16:52 ` Srinivas Kandagatla @ 2019-08-13 17:29 ` Cezary Rojewski 2019-08-13 19:19 ` Mark Brown 0 siblings, 1 reply; 44+ messages in thread From: Cezary Rojewski @ 2019-08-13 17:29 UTC (permalink / raw) To: Srinivas Kandagatla Cc: vkoul, broonie, bgoswami, plai, pierre-louis.bossart, robh+dt, devicetree, lgirdwood, alsa-devel, linux-kernel, spapothi On 2019-08-13 18:52, Srinivas Kandagatla wrote: > Thanks for the review, > > On 13/08/2019 17:03, Cezary Rojewski wrote: >> On 2019-08-13 10:35, Srinivas Kandagatla wrote: >>> On platforms which have smart speaker amplifiers connected via >>> soundwire and modeled as aux devices in ASoC, in such usecases machine >>> driver should be able to get sdw master stream from dai so that it can >>> use the runtime stream to setup slave streams. >>> >>> soundwire already as a set function, get function would provide more >>> flexibility to above configurations. >>> >>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> >>> --- >>> +static inline void *snd_soc_dai_get_sdw_stream(struct snd_soc_dai *dai, >>> + int direction) >>> +{ >>> + if (dai->driver->ops->get_sdw_stream) >>> + return dai->driver->ops->get_sdw_stream(dai, direction); >>> + else >>> + return ERR_PTR(-ENOTSUPP); >>> +} >> >> Drop redundant else. > Not all the dai drivers would implement this function, I guess else is > not redundant here! > > --srini >> Eh. By that I meant dropping "else" keyword and reducing indentation for "return ERR_PTR(-ENOTSUPP);" Czarek ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v2 3/5] ASoC: core: add support to snd_soc_dai_get_sdw_stream() 2019-08-13 17:29 ` Cezary Rojewski @ 2019-08-13 19:19 ` Mark Brown 0 siblings, 0 replies; 44+ messages in thread From: Mark Brown @ 2019-08-13 19:19 UTC (permalink / raw) To: Cezary Rojewski Cc: Srinivas Kandagatla, vkoul, bgoswami, plai, pierre-louis.bossart, robh+dt, devicetree, lgirdwood, alsa-devel, linux-kernel, spapothi [-- Attachment #1: Type: text/plain, Size: 721 bytes --] On Tue, Aug 13, 2019 at 07:29:50PM +0200, Cezary Rojewski wrote: > On 2019-08-13 18:52, Srinivas Kandagatla wrote: > > On 13/08/2019 17:03, Cezary Rojewski wrote: > > > On 2019-08-13 10:35, Srinivas Kandagatla wrote: > > > > + if (dai->driver->ops->get_sdw_stream) > > > > + return dai->driver->ops->get_sdw_stream(dai, direction); > > > > + else > > > > + return ERR_PTR(-ENOTSUPP); > > > Drop redundant else. > > Not all the dai drivers would implement this function, I guess else is > > not redundant here! > Eh. By that I meant dropping "else" keyword and reducing indentation for > "return ERR_PTR(-ENOTSUPP);" The above is the idiom used throughout the rest of the file. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH v2 4/5] dt-bindings: soundwire: add bindings for Qcom controller 2019-08-13 8:35 [PATCH v2 0/5] soundwire: Add support to Qualcomm SoundWire master Srinivas Kandagatla ` (2 preceding siblings ...) 2019-08-13 8:35 ` [PATCH v2 3/5] ASoC: core: add support to snd_soc_dai_get_sdw_stream() Srinivas Kandagatla @ 2019-08-13 8:35 ` Srinivas Kandagatla 2019-08-23 7:28 ` Vinod Koul 2019-08-13 8:35 ` [PATCH v2 5/5] soundwire: qcom: add support for SoundWire controller Srinivas Kandagatla 4 siblings, 1 reply; 44+ messages in thread From: Srinivas Kandagatla @ 2019-08-13 8:35 UTC (permalink / raw) To: vkoul, broonie Cc: bgoswami, plai, pierre-louis.bossart, robh+dt, devicetree, lgirdwood, alsa-devel, linux-kernel, spapothi, Srinivas Kandagatla This patch adds bindings for Qualcomm soundwire controller. Qualcomm SoundWire Master controller is present in most Qualcomm SoCs either integrated as part of WCD audio codecs via slimbus or as part of SOC I/O. Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> --- .../bindings/soundwire/qcom,sdw.txt | 167 ++++++++++++++++++ 1 file changed, 167 insertions(+) create mode 100644 Documentation/devicetree/bindings/soundwire/qcom,sdw.txt diff --git a/Documentation/devicetree/bindings/soundwire/qcom,sdw.txt b/Documentation/devicetree/bindings/soundwire/qcom,sdw.txt new file mode 100644 index 000000000000..436547f3b155 --- /dev/null +++ b/Documentation/devicetree/bindings/soundwire/qcom,sdw.txt @@ -0,0 +1,167 @@ +Qualcomm SoundWire Controller Bindings + + +This binding describes the Qualcomm SoundWire Controller along with its +board specific bus parameters. + +- compatible: + Usage: required + Value type: <stringlist> + Definition: must be "qcom,soundwire-v<MAJOR>.<MINOR>.<STEP>", + Example: + "qcom,soundwire-v1.3.0" + "qcom,soundwire-v1.5.0" + "qcom,soundwire-v1.6.0" +- reg: + Usage: required + Value type: <prop-encoded-array> + Definition: the base address and size of SoundWire controller + address space. + +- interrupts: + Usage: required + Value type: <prop-encoded-array> + Definition: should specify the SoundWire Controller IRQ + +- clock-names: + Usage: required + Value type: <stringlist> + Definition: should be "iface" for SoundWire Controller interface clock + +- clocks: + Usage: required + Value type: <prop-encoded-array> + Definition: should specify the SoundWire Controller interface clock + +- #sound-dai-cells: + Usage: required + Value type: <u32> + Definition: must be 1 for digital audio interfaces on the controller. + +- qcom,dout-ports: + Usage: required + Value type: <u32> + Definition: must be count of data out ports + +- qcom,din-ports: + Usage: required + Value type: <u32> + Definition: must be count of data in ports + +- qcom,ports-offset1: + Usage: required + Value type: <prop-encoded-array> + Definition: should specify payload transport window offset1 of each + data port. Out ports followed by In ports. + More info in MIPI Alliance SoundWire 1.0 Specifications. + +- qcom,ports-offset2: + Usage: required + Value type: <prop-encoded-array> + Definition: should specify payload transport window offset2 of each + data port. Out ports followed by In ports. + More info in MIPI Alliance SoundWire 1.0 Specifications. + +- qcom,ports-sinterval-low: + Usage: required + Value type: <prop-encoded-array> + Definition: should be sample interval low of each data port. + Out ports followed by In ports. Used for Sample Interval + calculation. + More info in MIPI Alliance SoundWire 1.0 Specifications. + +- qcom,ports-word-length: + Usage: optional + Value type: <prop-encoded-array> + Definition: should be size of payload channel sample. + More info in MIPI Alliance SoundWire 1.0 Specifications. + +- qcom,ports-block-pack-mode: + Usage: optional + Value type: <prop-encoded-array> + Definition: should be 0 or 1 to indicate the block packing mode. + 0 to indicate Blocks are per Channel + 1 to indicate Blocks are per Port. + Out ports followed by In ports. + More info in MIPI Alliance SoundWire 1.0 Specifications. + +- qcom,ports-block-group-count: + Usage: optional + Value type: <prop-encoded-array> + Definition: should be in range 1 to 4 to indicate how many sample + intervals are combined into a payload. + Out ports followed by In ports. + More info in MIPI Alliance SoundWire 1.0 Specifications. + +- qcom,ports-lane-control: + Usage: optional + Value type: <prop-encoded-array> + Definition: should be in range 0 to 7 to identify which data lane + the data port uses. + Out ports followed by In ports. + More info in MIPI Alliance SoundWire 1.0 Specifications. + +- qcom,ports-hstart: + Usage: optional + Value type: <prop-encoded-array> + Definition: should be number identifying lowerst numbered coloum in + SoundWire Frame, i.e. left edge of the Transport sub-frame + for each port. Values between 0 and 15 are valid. + Out ports followed by In ports. + More info in MIPI Alliance SoundWire 1.0 Specifications. + +- qcom,ports-hstop: + Usage: optional + Value type: <prop-encoded-array> + Definition: should be number identifying highest numbered coloum in + SoundWire Frame, i.e. the right edge of the Transport + sub-frame for each port. Values between 0 and 15 are valid. + Out ports followed by In ports. + More info in MIPI Alliance SoundWire 1.0 Specifications. + +- qcom,dports-type: + Usage: optional + Value type: <prop-encoded-array> + Definition: should be one of the following types + 0 for reduced port + 1 for simple ports + 2 for full port + Out ports followed by In ports. + More info in MIPI Alliance SoundWire 1.0 Specifications. + +Note: + More Information on detail of encoding of these fields can be +found in MIPI Alliance SoundWire 1.0 Specifications. + += SoundWire devices +Each subnode of the bus represents SoundWire device attached to it. +The properties of these nodes are defined by the individual bindings. + += EXAMPLE +The following example represents a SoundWire controller on DB845c board +which has controller integrated inside WCD934x codec on SDM845 SoC. + +soundwire: soundwire@c85 { + compatible = "qcom,soundwire-v1.3.0"; + reg = <0xc85 0x20>; + interrupts = <20 IRQ_TYPE_EDGE_RISING>; + clocks = <&wcc>; + clock-names = "iface"; + #sound-dai-cells = <1>; + qcom,dports-type = <0>; + qcom,dout-ports = <6>; + qcom,din-ports = <2>; + qcom,ports-sinterval-low = /bits/ 8 <0x07 0x1F 0x3F 0x7 0x1F 0x3F 0x0F 0x0F>; + qcom,ports-offset1 = /bits/ 8 <0x01 0x02 0x0C 0x6 0x12 0x0D 0x07 0x0A >; + qcom,ports-offset2 = /bits/ 8 <0x00 0x00 0x1F 0x00 0x00 0x1F 0x00 0x00>; + + /* Left Speaker */ + left{ + .... + }; + + /* Right Speaker */ + right{ + .... + }; +}; -- 2.21.0 ^ permalink raw reply related [flat|nested] 44+ messages in thread
* Re: [PATCH v2 4/5] dt-bindings: soundwire: add bindings for Qcom controller 2019-08-13 8:35 ` [PATCH v2 4/5] dt-bindings: soundwire: add bindings for Qcom controller Srinivas Kandagatla @ 2019-08-23 7:28 ` Vinod Koul 2019-08-30 8:05 ` Srinivas Kandagatla 0 siblings, 1 reply; 44+ messages in thread From: Vinod Koul @ 2019-08-23 7:28 UTC (permalink / raw) To: Srinivas Kandagatla, robh+dt Cc: broonie, bgoswami, plai, pierre-louis.bossart, devicetree, lgirdwood, alsa-devel, linux-kernel, spapothi On 13-08-19, 09:35, Srinivas Kandagatla wrote: > This patch adds bindings for Qualcomm soundwire controller. > > Qualcomm SoundWire Master controller is present in most Qualcomm SoCs > either integrated as part of WCD audio codecs via slimbus or > as part of SOC I/O. Rob.. ? > > Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> > --- > .../bindings/soundwire/qcom,sdw.txt | 167 ++++++++++++++++++ > 1 file changed, 167 insertions(+) > create mode 100644 Documentation/devicetree/bindings/soundwire/qcom,sdw.txt > > diff --git a/Documentation/devicetree/bindings/soundwire/qcom,sdw.txt b/Documentation/devicetree/bindings/soundwire/qcom,sdw.txt > new file mode 100644 > index 000000000000..436547f3b155 > --- /dev/null > +++ b/Documentation/devicetree/bindings/soundwire/qcom,sdw.txt > @@ -0,0 +1,167 @@ > +Qualcomm SoundWire Controller Bindings > + > + > +This binding describes the Qualcomm SoundWire Controller along with its > +board specific bus parameters. > + > +- compatible: > + Usage: required > + Value type: <stringlist> > + Definition: must be "qcom,soundwire-v<MAJOR>.<MINOR>.<STEP>", > + Example: > + "qcom,soundwire-v1.3.0" > + "qcom,soundwire-v1.5.0" > + "qcom,soundwire-v1.6.0" > +- reg: > + Usage: required > + Value type: <prop-encoded-array> > + Definition: the base address and size of SoundWire controller > + address space. > + > +- interrupts: > + Usage: required > + Value type: <prop-encoded-array> > + Definition: should specify the SoundWire Controller IRQ > + > +- clock-names: > + Usage: required > + Value type: <stringlist> > + Definition: should be "iface" for SoundWire Controller interface clock > + > +- clocks: > + Usage: required > + Value type: <prop-encoded-array> > + Definition: should specify the SoundWire Controller interface clock > + > +- #sound-dai-cells: > + Usage: required > + Value type: <u32> > + Definition: must be 1 for digital audio interfaces on the controller. > + > +- qcom,dout-ports: > + Usage: required > + Value type: <u32> > + Definition: must be count of data out ports > + > +- qcom,din-ports: > + Usage: required > + Value type: <u32> > + Definition: must be count of data in ports > + > +- qcom,ports-offset1: > + Usage: required > + Value type: <prop-encoded-array> > + Definition: should specify payload transport window offset1 of each > + data port. Out ports followed by In ports. > + More info in MIPI Alliance SoundWire 1.0 Specifications. > + > +- qcom,ports-offset2: > + Usage: required > + Value type: <prop-encoded-array> > + Definition: should specify payload transport window offset2 of each > + data port. Out ports followed by In ports. > + More info in MIPI Alliance SoundWire 1.0 Specifications. > + > +- qcom,ports-sinterval-low: > + Usage: required > + Value type: <prop-encoded-array> > + Definition: should be sample interval low of each data port. > + Out ports followed by In ports. Used for Sample Interval > + calculation. > + More info in MIPI Alliance SoundWire 1.0 Specifications. > + > +- qcom,ports-word-length: > + Usage: optional > + Value type: <prop-encoded-array> > + Definition: should be size of payload channel sample. > + More info in MIPI Alliance SoundWire 1.0 Specifications. > + > +- qcom,ports-block-pack-mode: > + Usage: optional > + Value type: <prop-encoded-array> > + Definition: should be 0 or 1 to indicate the block packing mode. > + 0 to indicate Blocks are per Channel > + 1 to indicate Blocks are per Port. > + Out ports followed by In ports. > + More info in MIPI Alliance SoundWire 1.0 Specifications. > + > +- qcom,ports-block-group-count: > + Usage: optional > + Value type: <prop-encoded-array> > + Definition: should be in range 1 to 4 to indicate how many sample > + intervals are combined into a payload. > + Out ports followed by In ports. > + More info in MIPI Alliance SoundWire 1.0 Specifications. > + > +- qcom,ports-lane-control: > + Usage: optional > + Value type: <prop-encoded-array> > + Definition: should be in range 0 to 7 to identify which data lane > + the data port uses. > + Out ports followed by In ports. > + More info in MIPI Alliance SoundWire 1.0 Specifications. > + > +- qcom,ports-hstart: > + Usage: optional > + Value type: <prop-encoded-array> > + Definition: should be number identifying lowerst numbered coloum in > + SoundWire Frame, i.e. left edge of the Transport sub-frame > + for each port. Values between 0 and 15 are valid. > + Out ports followed by In ports. > + More info in MIPI Alliance SoundWire 1.0 Specifications. > + > +- qcom,ports-hstop: > + Usage: optional > + Value type: <prop-encoded-array> > + Definition: should be number identifying highest numbered coloum in > + SoundWire Frame, i.e. the right edge of the Transport > + sub-frame for each port. Values between 0 and 15 are valid. > + Out ports followed by In ports. > + More info in MIPI Alliance SoundWire 1.0 Specifications. > + > +- qcom,dports-type: > + Usage: optional > + Value type: <prop-encoded-array> > + Definition: should be one of the following types > + 0 for reduced port > + 1 for simple ports > + 2 for full port > + Out ports followed by In ports. > + More info in MIPI Alliance SoundWire 1.0 Specifications. > + > +Note: > + More Information on detail of encoding of these fields can be > +found in MIPI Alliance SoundWire 1.0 Specifications. > + > += SoundWire devices > +Each subnode of the bus represents SoundWire device attached to it. > +The properties of these nodes are defined by the individual bindings. > + > += EXAMPLE > +The following example represents a SoundWire controller on DB845c board > +which has controller integrated inside WCD934x codec on SDM845 SoC. > + > +soundwire: soundwire@c85 { > + compatible = "qcom,soundwire-v1.3.0"; > + reg = <0xc85 0x20>; > + interrupts = <20 IRQ_TYPE_EDGE_RISING>; > + clocks = <&wcc>; > + clock-names = "iface"; > + #sound-dai-cells = <1>; > + qcom,dports-type = <0>; > + qcom,dout-ports = <6>; > + qcom,din-ports = <2>; > + qcom,ports-sinterval-low = /bits/ 8 <0x07 0x1F 0x3F 0x7 0x1F 0x3F 0x0F 0x0F>; > + qcom,ports-offset1 = /bits/ 8 <0x01 0x02 0x0C 0x6 0x12 0x0D 0x07 0x0A >; > + qcom,ports-offset2 = /bits/ 8 <0x00 0x00 0x1F 0x00 0x00 0x1F 0x00 0x00>; > + > + /* Left Speaker */ > + left{ > + .... > + }; > + > + /* Right Speaker */ > + right{ > + .... > + }; > +}; > -- > 2.21.0 -- ~Vinod ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v2 4/5] dt-bindings: soundwire: add bindings for Qcom controller 2019-08-23 7:28 ` Vinod Koul @ 2019-08-30 8:05 ` Srinivas Kandagatla 2019-08-30 8:05 ` [alsa-devel] " Srinivas Kandagatla 0 siblings, 1 reply; 44+ messages in thread From: Srinivas Kandagatla @ 2019-08-30 8:05 UTC (permalink / raw) To: Vinod Koul, robh+dt Cc: broonie, bgoswami, plai, pierre-louis.bossart, devicetree, lgirdwood, alsa-devel, linux-kernel, spapothi Hi Rob, On 23/08/2019 08:28, Vinod Koul wrote: > On 13-08-19, 09:35, Srinivas Kandagatla wrote: >> This patch adds bindings for Qualcomm soundwire controller. >> >> Qualcomm SoundWire Master controller is present in most Qualcomm SoCs >> either integrated as part of WCD audio codecs via slimbus or >> as part of SOC I/O. > > Rob.. ? > >> >> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> >> --- >> .../bindings/soundwire/qcom,sdw.txt | 167 ++++++++++++++++++ >> 1 file changed, 167 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/soundwire/qcom,sdw.txt >> >> diff --git a/Documentation/devicetree/bindings/soundwire/qcom,sdw.txt b/Documentation/devicetree/bindings/soundwire/qcom,sdw.txt Do you see any issues with the these bindings? --srini >> new file mode 100644 >> index 000000000000..436547f3b155 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/soundwire/qcom,sdw.txt >> @@ -0,0 +1,167 @@ >> +Qualcomm SoundWire Controller Bindings >> + >> + >> +This binding describes the Qualcomm SoundWire Controller along with its >> +board specific bus parameters. >> + >> +- compatible: >> + Usage: required >> + Value type: <stringlist> >> + Definition: must be "qcom,soundwire-v<MAJOR>.<MINOR>.<STEP>", >> + Example: >> + "qcom,soundwire-v1.3.0" >> + "qcom,soundwire-v1.5.0" >> + "qcom,soundwire-v1.6.0" >> +- reg: >> + Usage: required >> + Value type: <prop-encoded-array> >> + Definition: the base address and size of SoundWire controller >> + address space. >> + >> +- interrupts: >> + Usage: required >> + Value type: <prop-encoded-array> >> + Definition: should specify the SoundWire Controller IRQ >> + >> +- clock-names: >> + Usage: required >> + Value type: <stringlist> >> + Definition: should be "iface" for SoundWire Controller interface clock >> + >> +- clocks: >> + Usage: required >> + Value type: <prop-encoded-array> >> + Definition: should specify the SoundWire Controller interface clock >> + >> +- #sound-dai-cells: >> + Usage: required >> + Value type: <u32> >> + Definition: must be 1 for digital audio interfaces on the controller. >> + >> +- qcom,dout-ports: >> + Usage: required >> + Value type: <u32> >> + Definition: must be count of data out ports >> + >> +- qcom,din-ports: >> + Usage: required >> + Value type: <u32> >> + Definition: must be count of data in ports >> + >> +- qcom,ports-offset1: >> + Usage: required >> + Value type: <prop-encoded-array> >> + Definition: should specify payload transport window offset1 of each >> + data port. Out ports followed by In ports. >> + More info in MIPI Alliance SoundWire 1.0 Specifications. >> + >> +- qcom,ports-offset2: >> + Usage: required >> + Value type: <prop-encoded-array> >> + Definition: should specify payload transport window offset2 of each >> + data port. Out ports followed by In ports. >> + More info in MIPI Alliance SoundWire 1.0 Specifications. >> + >> +- qcom,ports-sinterval-low: >> + Usage: required >> + Value type: <prop-encoded-array> >> + Definition: should be sample interval low of each data port. >> + Out ports followed by In ports. Used for Sample Interval >> + calculation. >> + More info in MIPI Alliance SoundWire 1.0 Specifications. >> + >> +- qcom,ports-word-length: >> + Usage: optional >> + Value type: <prop-encoded-array> >> + Definition: should be size of payload channel sample. >> + More info in MIPI Alliance SoundWire 1.0 Specifications. >> + >> +- qcom,ports-block-pack-mode: >> + Usage: optional >> + Value type: <prop-encoded-array> >> + Definition: should be 0 or 1 to indicate the block packing mode. >> + 0 to indicate Blocks are per Channel >> + 1 to indicate Blocks are per Port. >> + Out ports followed by In ports. >> + More info in MIPI Alliance SoundWire 1.0 Specifications. >> + >> +- qcom,ports-block-group-count: >> + Usage: optional >> + Value type: <prop-encoded-array> >> + Definition: should be in range 1 to 4 to indicate how many sample >> + intervals are combined into a payload. >> + Out ports followed by In ports. >> + More info in MIPI Alliance SoundWire 1.0 Specifications. >> + >> +- qcom,ports-lane-control: >> + Usage: optional >> + Value type: <prop-encoded-array> >> + Definition: should be in range 0 to 7 to identify which data lane >> + the data port uses. >> + Out ports followed by In ports. >> + More info in MIPI Alliance SoundWire 1.0 Specifications. >> + >> +- qcom,ports-hstart: >> + Usage: optional >> + Value type: <prop-encoded-array> >> + Definition: should be number identifying lowerst numbered coloum in >> + SoundWire Frame, i.e. left edge of the Transport sub-frame >> + for each port. Values between 0 and 15 are valid. >> + Out ports followed by In ports. >> + More info in MIPI Alliance SoundWire 1.0 Specifications. >> + >> +- qcom,ports-hstop: >> + Usage: optional >> + Value type: <prop-encoded-array> >> + Definition: should be number identifying highest numbered coloum in >> + SoundWire Frame, i.e. the right edge of the Transport >> + sub-frame for each port. Values between 0 and 15 are valid. >> + Out ports followed by In ports. >> + More info in MIPI Alliance SoundWire 1.0 Specifications. >> + >> +- qcom,dports-type: >> + Usage: optional >> + Value type: <prop-encoded-array> >> + Definition: should be one of the following types >> + 0 for reduced port >> + 1 for simple ports >> + 2 for full port >> + Out ports followed by In ports. >> + More info in MIPI Alliance SoundWire 1.0 Specifications. >> + >> +Note: >> + More Information on detail of encoding of these fields can be >> +found in MIPI Alliance SoundWire 1.0 Specifications. >> + >> += SoundWire devices >> +Each subnode of the bus represents SoundWire device attached to it. >> +The properties of these nodes are defined by the individual bindings. >> + >> += EXAMPLE >> +The following example represents a SoundWire controller on DB845c board >> +which has controller integrated inside WCD934x codec on SDM845 SoC. >> + >> +soundwire: soundwire@c85 { >> + compatible = "qcom,soundwire-v1.3.0"; >> + reg = <0xc85 0x20>; >> + interrupts = <20 IRQ_TYPE_EDGE_RISING>; >> + clocks = <&wcc>; >> + clock-names = "iface"; >> + #sound-dai-cells = <1>; >> + qcom,dports-type = <0>; >> + qcom,dout-ports = <6>; >> + qcom,din-ports = <2>; >> + qcom,ports-sinterval-low = /bits/ 8 <0x07 0x1F 0x3F 0x7 0x1F 0x3F 0x0F 0x0F>; >> + qcom,ports-offset1 = /bits/ 8 <0x01 0x02 0x0C 0x6 0x12 0x0D 0x07 0x0A >; >> + qcom,ports-offset2 = /bits/ 8 <0x00 0x00 0x1F 0x00 0x00 0x1F 0x00 0x00>; >> + >> + /* Left Speaker */ >> + left{ >> + .... >> + }; >> + >> + /* Right Speaker */ >> + right{ >> + .... >> + }; >> +}; >> -- >> 2.21.0 > ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [alsa-devel] [PATCH v2 4/5] dt-bindings: soundwire: add bindings for Qcom controller 2019-08-30 8:05 ` Srinivas Kandagatla @ 2019-08-30 8:05 ` Srinivas Kandagatla 0 siblings, 0 replies; 44+ messages in thread From: Srinivas Kandagatla @ 2019-08-30 8:05 UTC (permalink / raw) To: Vinod Koul, robh+dt Cc: devicetree, alsa-devel, bgoswami, linux-kernel, plai, pierre-louis.bossart, lgirdwood, broonie, spapothi Hi Rob, On 23/08/2019 08:28, Vinod Koul wrote: > On 13-08-19, 09:35, Srinivas Kandagatla wrote: >> This patch adds bindings for Qualcomm soundwire controller. >> >> Qualcomm SoundWire Master controller is present in most Qualcomm SoCs >> either integrated as part of WCD audio codecs via slimbus or >> as part of SOC I/O. > > Rob.. ? > >> >> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> >> --- >> .../bindings/soundwire/qcom,sdw.txt | 167 ++++++++++++++++++ >> 1 file changed, 167 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/soundwire/qcom,sdw.txt >> >> diff --git a/Documentation/devicetree/bindings/soundwire/qcom,sdw.txt b/Documentation/devicetree/bindings/soundwire/qcom,sdw.txt Do you see any issues with the these bindings? --srini >> new file mode 100644 >> index 000000000000..436547f3b155 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/soundwire/qcom,sdw.txt >> @@ -0,0 +1,167 @@ >> +Qualcomm SoundWire Controller Bindings >> + >> + >> +This binding describes the Qualcomm SoundWire Controller along with its >> +board specific bus parameters. >> + >> +- compatible: >> + Usage: required >> + Value type: <stringlist> >> + Definition: must be "qcom,soundwire-v<MAJOR>.<MINOR>.<STEP>", >> + Example: >> + "qcom,soundwire-v1.3.0" >> + "qcom,soundwire-v1.5.0" >> + "qcom,soundwire-v1.6.0" >> +- reg: >> + Usage: required >> + Value type: <prop-encoded-array> >> + Definition: the base address and size of SoundWire controller >> + address space. >> + >> +- interrupts: >> + Usage: required >> + Value type: <prop-encoded-array> >> + Definition: should specify the SoundWire Controller IRQ >> + >> +- clock-names: >> + Usage: required >> + Value type: <stringlist> >> + Definition: should be "iface" for SoundWire Controller interface clock >> + >> +- clocks: >> + Usage: required >> + Value type: <prop-encoded-array> >> + Definition: should specify the SoundWire Controller interface clock >> + >> +- #sound-dai-cells: >> + Usage: required >> + Value type: <u32> >> + Definition: must be 1 for digital audio interfaces on the controller. >> + >> +- qcom,dout-ports: >> + Usage: required >> + Value type: <u32> >> + Definition: must be count of data out ports >> + >> +- qcom,din-ports: >> + Usage: required >> + Value type: <u32> >> + Definition: must be count of data in ports >> + >> +- qcom,ports-offset1: >> + Usage: required >> + Value type: <prop-encoded-array> >> + Definition: should specify payload transport window offset1 of each >> + data port. Out ports followed by In ports. >> + More info in MIPI Alliance SoundWire 1.0 Specifications. >> + >> +- qcom,ports-offset2: >> + Usage: required >> + Value type: <prop-encoded-array> >> + Definition: should specify payload transport window offset2 of each >> + data port. Out ports followed by In ports. >> + More info in MIPI Alliance SoundWire 1.0 Specifications. >> + >> +- qcom,ports-sinterval-low: >> + Usage: required >> + Value type: <prop-encoded-array> >> + Definition: should be sample interval low of each data port. >> + Out ports followed by In ports. Used for Sample Interval >> + calculation. >> + More info in MIPI Alliance SoundWire 1.0 Specifications. >> + >> +- qcom,ports-word-length: >> + Usage: optional >> + Value type: <prop-encoded-array> >> + Definition: should be size of payload channel sample. >> + More info in MIPI Alliance SoundWire 1.0 Specifications. >> + >> +- qcom,ports-block-pack-mode: >> + Usage: optional >> + Value type: <prop-encoded-array> >> + Definition: should be 0 or 1 to indicate the block packing mode. >> + 0 to indicate Blocks are per Channel >> + 1 to indicate Blocks are per Port. >> + Out ports followed by In ports. >> + More info in MIPI Alliance SoundWire 1.0 Specifications. >> + >> +- qcom,ports-block-group-count: >> + Usage: optional >> + Value type: <prop-encoded-array> >> + Definition: should be in range 1 to 4 to indicate how many sample >> + intervals are combined into a payload. >> + Out ports followed by In ports. >> + More info in MIPI Alliance SoundWire 1.0 Specifications. >> + >> +- qcom,ports-lane-control: >> + Usage: optional >> + Value type: <prop-encoded-array> >> + Definition: should be in range 0 to 7 to identify which data lane >> + the data port uses. >> + Out ports followed by In ports. >> + More info in MIPI Alliance SoundWire 1.0 Specifications. >> + >> +- qcom,ports-hstart: >> + Usage: optional >> + Value type: <prop-encoded-array> >> + Definition: should be number identifying lowerst numbered coloum in >> + SoundWire Frame, i.e. left edge of the Transport sub-frame >> + for each port. Values between 0 and 15 are valid. >> + Out ports followed by In ports. >> + More info in MIPI Alliance SoundWire 1.0 Specifications. >> + >> +- qcom,ports-hstop: >> + Usage: optional >> + Value type: <prop-encoded-array> >> + Definition: should be number identifying highest numbered coloum in >> + SoundWire Frame, i.e. the right edge of the Transport >> + sub-frame for each port. Values between 0 and 15 are valid. >> + Out ports followed by In ports. >> + More info in MIPI Alliance SoundWire 1.0 Specifications. >> + >> +- qcom,dports-type: >> + Usage: optional >> + Value type: <prop-encoded-array> >> + Definition: should be one of the following types >> + 0 for reduced port >> + 1 for simple ports >> + 2 for full port >> + Out ports followed by In ports. >> + More info in MIPI Alliance SoundWire 1.0 Specifications. >> + >> +Note: >> + More Information on detail of encoding of these fields can be >> +found in MIPI Alliance SoundWire 1.0 Specifications. >> + >> += SoundWire devices >> +Each subnode of the bus represents SoundWire device attached to it. >> +The properties of these nodes are defined by the individual bindings. >> + >> += EXAMPLE >> +The following example represents a SoundWire controller on DB845c board >> +which has controller integrated inside WCD934x codec on SDM845 SoC. >> + >> +soundwire: soundwire@c85 { >> + compatible = "qcom,soundwire-v1.3.0"; >> + reg = <0xc85 0x20>; >> + interrupts = <20 IRQ_TYPE_EDGE_RISING>; >> + clocks = <&wcc>; >> + clock-names = "iface"; >> + #sound-dai-cells = <1>; >> + qcom,dports-type = <0>; >> + qcom,dout-ports = <6>; >> + qcom,din-ports = <2>; >> + qcom,ports-sinterval-low = /bits/ 8 <0x07 0x1F 0x3F 0x7 0x1F 0x3F 0x0F 0x0F>; >> + qcom,ports-offset1 = /bits/ 8 <0x01 0x02 0x0C 0x6 0x12 0x0D 0x07 0x0A >; >> + qcom,ports-offset2 = /bits/ 8 <0x00 0x00 0x1F 0x00 0x00 0x1F 0x00 0x00>; >> + >> + /* Left Speaker */ >> + left{ >> + .... >> + }; >> + >> + /* Right Speaker */ >> + right{ >> + .... >> + }; >> +}; >> -- >> 2.21.0 > _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org https://mailman.alsa-project.org/mailman/listinfo/alsa-devel ^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH v2 5/5] soundwire: qcom: add support for SoundWire controller 2019-08-13 8:35 [PATCH v2 0/5] soundwire: Add support to Qualcomm SoundWire master Srinivas Kandagatla ` (3 preceding siblings ...) 2019-08-13 8:35 ` [PATCH v2 4/5] dt-bindings: soundwire: add bindings for Qcom controller Srinivas Kandagatla @ 2019-08-13 8:35 ` Srinivas Kandagatla 4 siblings, 0 replies; 44+ messages in thread From: Srinivas Kandagatla @ 2019-08-13 8:35 UTC (permalink / raw) To: vkoul, broonie Cc: bgoswami, plai, pierre-louis.bossart, robh+dt, devicetree, lgirdwood, alsa-devel, linux-kernel, spapothi, Srinivas Kandagatla Qualcomm SoundWire Master controller is present in most Qualcomm SoCs either integrated as part of WCD audio codecs via slimbus or as part of SOC I/O. This patchset adds support to a very basic controller which has been tested with WCD934x SoundWire controller connected to WSA881x smart speaker amplifiers. Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> --- drivers/soundwire/Kconfig | 9 + drivers/soundwire/Makefile | 4 + drivers/soundwire/qcom.c | 919 +++++++++++++++++++++++++++++++++++++ 3 files changed, 932 insertions(+) create mode 100644 drivers/soundwire/qcom.c diff --git a/drivers/soundwire/Kconfig b/drivers/soundwire/Kconfig index f518273cfbe3..69f3b0e36b3d 100644 --- a/drivers/soundwire/Kconfig +++ b/drivers/soundwire/Kconfig @@ -30,4 +30,13 @@ config SOUNDWIRE_INTEL enable this config option to get the SoundWire support for that device. +config SOUNDWIRE_QCOM + tristate "Qualcomm SoundWire Master driver" + depends on SLIMBUS + depends on SND_SOC + help + SoundWire Qualcomm Master driver. + If you have an Qualcomm platform which has a SoundWire Master then + enable this config option to get the SoundWire support for that + device endif diff --git a/drivers/soundwire/Makefile b/drivers/soundwire/Makefile index 45b7e5001653..e2028a1df6d5 100644 --- a/drivers/soundwire/Makefile +++ b/drivers/soundwire/Makefile @@ -17,3 +17,7 @@ obj-$(CONFIG_SOUNDWIRE_INTEL) += soundwire-intel.o soundwire-intel-init-objs := intel_init.o obj-$(CONFIG_SOUNDWIRE_INTEL) += soundwire-intel-init.o + +#Qualcomm driver +soundwire-qcom-objs := qcom.o +obj-$(CONFIG_SOUNDWIRE_QCOM) += soundwire-qcom.o diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c new file mode 100644 index 000000000000..5634912e113c --- /dev/null +++ b/drivers/soundwire/qcom.c @@ -0,0 +1,919 @@ +// SPDX-License-Identifier: GPL-2.0 +// Copyright (c) 2019, Linaro Limited + +#include <linux/clk.h> +#include <linux/completion.h> +#include <linux/interrupt.h> +#include <linux/io.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/of_irq.h> +#include <linux/of_device.h> +#include <linux/regmap.h> +#include <linux/slab.h> +#include <linux/slimbus.h> +#include <linux/soundwire/sdw.h> +#include <linux/soundwire/sdw_registers.h> +#include <sound/pcm_params.h> +#include <sound/soc.h> +#include "bus.h" + +#define SWRM_COMP_HW_VERSION 0x00 +#define SWRM_COMP_CFG_ADDR 0x04 +#define SWRM_COMP_CFG_IRQ_LEVEL_OR_PULSE_MSK BIT(1) +#define SWRM_COMP_CFG_ENABLE_MSK BIT(0) +#define SWRM_COMP_PARAMS 0x100 +#define SWRM_COMP_PARAMS_DOUT_PORTS_MASK GENMASK(4, 0) +#define SWRM_COMP_PARAMS_DIN_PORTS_MASK GENMASK(9, 5) +#define SWRM_INTERRUPT_STATUS 0x200 +#define SWRM_INTERRUPT_STATUS_RMSK GENMASK(16, 0) +#define SWRM_INTERRUPT_STATUS_NEW_SLAVE_ATTACHED BIT(1) +#define SWRM_INTERRUPT_STATUS_CHANGE_ENUM_SLAVE_STATUS BIT(2) +#define SWRM_INTERRUPT_STATUS_CMD_ERROR BIT(7) +#define SWRM_INTERRUPT_STATUS_SPECIAL_CMD_ID_FINISHED BIT(10) +#define SWRM_INTERRUPT_MASK_ADDR 0x204 +#define SWRM_INTERRUPT_CLEAR 0x208 +#define SWRM_CMD_FIFO_WR_CMD 0x300 +#define SWRM_CMD_FIFO_RD_CMD 0x304 +#define SWRM_CMD_FIFO_CMD 0x308 +#define SWRM_CMD_FIFO_STATUS 0x30C +#define SWRM_CMD_FIFO_CFG_ADDR 0x314 +#define SWRM_RD_WR_CMD_RETRIES 0x7 +#define SWRM_CMD_FIFO_RD_FIFO_ADDR 0x318 +#define SWRM_ENUMERATOR_CFG_ADDR 0x500 +#define SWRM_MCP_FRAME_CTRL_BANK_ADDR(m) (0x101C + 0x40 * (m)) +#define SWRM_MCP_FRAME_CTRL_BANK_ROW_CTRL_SHFT 3 +#define SWRM_MCP_FRAME_CTRL_BANK_COL_CTRL_BMSK GENMASK(2, 0) +#define SWRM_MCP_FRAME_CTRL_BANK_ROW_CTRL_BMSK GENMASK(7, 3) +#define SWRM_MCP_FRAME_CTRL_BANK_COL_CTRL_SHFT 0 +#define SWRM_MCP_CFG_ADDR 0x1048 +#define SWRM_MCP_CFG_MAX_NUM_OF_CMD_NO_PINGS_BMSK GENMASK(21, 17) +#define SWRM_MCP_CFG_MAX_NUM_OF_CMD_NO_PINGS_SHFT 0x11 +#define SWRM_DEF_CMD_NO_PINGS 0x1f +#define SWRM_MCP_STATUS 0x104C +#define SWRM_MCP_STATUS_BANK_NUM_MASK BIT(0) +#define SWRM_MCP_SLV_STATUS 0x1090 +#define SWRM_MCP_SLV_STATUS_MASK GENMASK(1, 0) +#define SWRM_DP_PORT_CTRL_BANK(n, m) (0x1124 + 0x100 * (n - 1) + 0x40 * m) +#define SWRM_DP_PORT_CTRL_EN_CHAN_SHFT 0x18 +#define SWRM_DP_PORT_CTRL_OFFSET2_SHFT 0x10 +#define SWRM_DP_PORT_CTRL_OFFSET1_SHFT 0x08 +#define SWRM_AHB_BRIDGE_WR_DATA_0 0xc85 +#define SWRM_AHB_BRIDGE_WR_ADDR_0 0xc89 +#define SWRM_AHB_BRIDGE_RD_ADDR_0 0xc8d +#define SWRM_AHB_BRIDGE_RD_DATA_0 0xc91 + +#define SWRM_REG_VAL_PACK(data, dev, id, reg) \ + ((reg) | ((id) << 16) | ((dev) << 20) | ((data) << 24)) + +#define SWRM_MAX_ROW_VAL 0 /* Rows = 48 */ +#define SWRM_DEFAULT_ROWS 48 +#define SWRM_MIN_COL_VAL 0 /* Cols = 2 */ +#define SWRM_DEFAULT_COL 16 +#define SWRM_MAX_COL_VAL 7 +#define SWRM_SPECIAL_CMD_ID 0xF +#define MAX_FREQ_NUM 1 +#define TIMEOUT_MS (2 * HZ) +#define QCOM_SWRM_MAX_RD_LEN 0xf +#define QCOM_SDW_MAX_PORTS 14 +#define DEFAULT_CLK_FREQ 9600000 +#define SWRM_MAX_DAIS 0xF + +struct qcom_swrm_port_config { + u8 si; + u8 off1; + u8 off2; +}; + +struct qcom_swrm_ctrl { + struct sdw_bus bus; + struct device *dev; + struct regmap *regmap; + struct completion *comp; + struct work_struct slave_work; + /* read/write lock */ + spinlock_t comp_lock; + /* Port alloc/free lock */ + struct mutex port_lock; + struct clk *hclk; + void __iomem *base; + u8 wr_cmd_id; + u8 rd_cmd_id; + int irq; + unsigned int version; + int num_din_ports; + int num_dout_ports; + unsigned long dout_port_mask; + unsigned long din_port_mask; + struct qcom_swrm_port_config pconfig[QCOM_SDW_MAX_PORTS]; + struct sdw_stream_runtime *sruntime[SWRM_MAX_DAIS]; + enum sdw_slave_status status[SDW_MAX_DEVICES]; + int (*reg_read)(struct qcom_swrm_ctrl *ctrl, int reg, u32 *val); + int (*reg_write)(struct qcom_swrm_ctrl *ctrl, int reg, int val); +}; + +#define to_qcom_sdw(b) container_of(b, struct qcom_swrm_ctrl, bus) + +static int qcom_swrm_abh_reg_read(struct qcom_swrm_ctrl *ctrl, int reg, + u32 *val) +{ + struct regmap *wcd_regmap = ctrl->regmap; + int ret; + + /* pg register + offset */ + ret = regmap_bulk_write(wcd_regmap, SWRM_AHB_BRIDGE_RD_ADDR_0, + (u8 *)®, 4); + if (ret < 0) + return SDW_CMD_FAIL; + + ret = regmap_bulk_read(wcd_regmap, SWRM_AHB_BRIDGE_RD_DATA_0, + val, 4); + if (ret < 0) + return SDW_CMD_FAIL; + + return SDW_CMD_OK; +} + +static int qcom_swrm_mmio_reg_read(struct qcom_swrm_ctrl *ctrl, int reg, + u32 *val) +{ + *val = readl_relaxed(ctrl->base + reg); + + return 0; +} + +static int qcom_swrm_mmio_reg_write(struct qcom_swrm_ctrl *ctrl, + int reg, int val) +{ + writel_relaxed(val, ctrl->base + reg); + + return 0; +} + +static int qcom_swrm_ahb_reg_write(struct qcom_swrm_ctrl *ctrl, + int reg, int val) +{ + struct regmap *wcd_regmap = ctrl->regmap; + int ret; + /* pg register + offset */ + ret = regmap_bulk_write(wcd_regmap, SWRM_AHB_BRIDGE_WR_DATA_0, + (u8 *)&val, 4); + if (ret) + return SDW_CMD_FAIL; + + /* write address register */ + ret = regmap_bulk_write(wcd_regmap, SWRM_AHB_BRIDGE_WR_ADDR_0, + (u8 *)®, 4); + if (ret) + return SDW_CMD_FAIL; + + return SDW_CMD_OK; +} + +static int qcom_swrm_cmd_fifo_wr_cmd(struct qcom_swrm_ctrl *ctrl, u8 cmd_data, + u8 dev_addr, u16 reg_addr) +{ + DECLARE_COMPLETION_ONSTACK(comp); + unsigned long flags; + u32 val; + int ret; + + spin_lock_irqsave(&ctrl->comp_lock, flags); + ctrl->comp = ∁ + spin_unlock_irqrestore(&ctrl->comp_lock, flags); + val = SWRM_REG_VAL_PACK(cmd_data, dev_addr, + SWRM_SPECIAL_CMD_ID, reg_addr); + ret = ctrl->reg_write(ctrl, SWRM_CMD_FIFO_WR_CMD, val); + if (ret) + goto err; + + ret = wait_for_completion_timeout(ctrl->comp, + msecs_to_jiffies(TIMEOUT_MS)); + + if (!ret) + ret = SDW_CMD_IGNORED; + else + ret = SDW_CMD_OK; +err: + spin_lock_irqsave(&ctrl->comp_lock, flags); + ctrl->comp = NULL; + spin_unlock_irqrestore(&ctrl->comp_lock, flags); + + return ret; +} + +static int qcom_swrm_cmd_fifo_rd_cmd(struct qcom_swrm_ctrl *ctrl, + u8 dev_addr, u16 reg_addr, + u32 len, u8 *rval) +{ + int i, ret; + u32 val; + DECLARE_COMPLETION_ONSTACK(comp); + unsigned long flags; + + spin_lock_irqsave(&ctrl->comp_lock, flags); + ctrl->comp = ∁ + spin_unlock_irqrestore(&ctrl->comp_lock, flags); + + val = SWRM_REG_VAL_PACK(len, dev_addr, SWRM_SPECIAL_CMD_ID, reg_addr); + ret = ctrl->reg_write(ctrl, SWRM_CMD_FIFO_RD_CMD, val); + if (ret) + goto err; + + ret = wait_for_completion_timeout(ctrl->comp, + msecs_to_jiffies(TIMEOUT_MS)); + + if (!ret) { + ret = SDW_CMD_IGNORED; + goto err; + } else { + ret = SDW_CMD_OK; + } + + for (i = 0; i < len; i++) { + ret = ctrl->reg_read(ctrl, SWRM_CMD_FIFO_RD_FIFO_ADDR, &val); + if (ret) + return ret; + + rval[i] = val & 0xFF; + } + +err: + spin_lock_irqsave(&ctrl->comp_lock, flags); + ctrl->comp = NULL; + spin_unlock_irqrestore(&ctrl->comp_lock, flags); + + return ret; +} + +static void qcom_swrm_get_device_status(struct qcom_swrm_ctrl *ctrl) +{ + u32 val; + int i; + + ctrl->reg_read(ctrl, SWRM_MCP_SLV_STATUS, &val); + + for (i = 0; i < SDW_MAX_DEVICES; i++) { + u32 s; + + s = (val >> (i * 2)); + s &= SWRM_MCP_SLV_STATUS_MASK; + ctrl->status[i] = s; + } +} + +static irqreturn_t qcom_swrm_irq_handler(int irq, void *dev_id) +{ + struct qcom_swrm_ctrl *ctrl = dev_id; + u32 sts, value; + unsigned long flags; + + ctrl->reg_read(ctrl, SWRM_INTERRUPT_STATUS, &sts); + + if (sts & SWRM_INTERRUPT_STATUS_CMD_ERROR) { + ctrl->reg_read(ctrl, SWRM_CMD_FIFO_STATUS, &value); + dev_err_ratelimited(ctrl->dev, + "CMD error, fifo status 0x%x\n", + value); + ctrl->reg_write(ctrl, SWRM_CMD_FIFO_CMD, 0x1); + } + + if ((sts & SWRM_INTERRUPT_STATUS_NEW_SLAVE_ATTACHED) || + sts & SWRM_INTERRUPT_STATUS_CHANGE_ENUM_SLAVE_STATUS) + schedule_work(&ctrl->slave_work); + + ctrl->reg_write(ctrl, SWRM_INTERRUPT_CLEAR, sts); + + if (sts & SWRM_INTERRUPT_STATUS_SPECIAL_CMD_ID_FINISHED) { + spin_lock_irqsave(&ctrl->comp_lock, flags); + if (ctrl->comp) + complete(ctrl->comp); + spin_unlock_irqrestore(&ctrl->comp_lock, flags); + } + + return IRQ_HANDLED; +} +static int qcom_swrm_init(struct qcom_swrm_ctrl *ctrl) +{ + u32 val; + + /* Clear Rows and Cols */ + val = (SWRM_MAX_ROW_VAL << SWRM_MCP_FRAME_CTRL_BANK_ROW_CTRL_SHFT | + SWRM_MIN_COL_VAL << SWRM_MCP_FRAME_CTRL_BANK_COL_CTRL_SHFT); + + ctrl->reg_write(ctrl, SWRM_MCP_FRAME_CTRL_BANK_ADDR(0), val); + + /* Disable Auto enumeration */ + ctrl->reg_write(ctrl, SWRM_ENUMERATOR_CFG_ADDR, 0); + + /* Mask soundwire interrupts */ + ctrl->reg_write(ctrl, SWRM_INTERRUPT_MASK_ADDR, + SWRM_INTERRUPT_STATUS_RMSK); + + /* Configure No pings */ + ctrl->reg_read(ctrl, SWRM_MCP_CFG_ADDR, &val); + val &= ~SWRM_MCP_CFG_MAX_NUM_OF_CMD_NO_PINGS_BMSK; + val |= (SWRM_DEF_CMD_NO_PINGS << + SWRM_MCP_CFG_MAX_NUM_OF_CMD_NO_PINGS_SHFT); + ctrl->reg_write(ctrl, SWRM_MCP_CFG_ADDR, val); + + /* Configure number of retries of a read/write cmd */ + ctrl->reg_write(ctrl, SWRM_CMD_FIFO_CFG_ADDR, SWRM_RD_WR_CMD_RETRIES); + + /* Set IRQ to PULSE */ + ctrl->reg_write(ctrl, SWRM_COMP_CFG_ADDR, + SWRM_COMP_CFG_IRQ_LEVEL_OR_PULSE_MSK | + SWRM_COMP_CFG_ENABLE_MSK); + return 0; +} + +static enum sdw_command_response qcom_swrm_xfer_msg(struct sdw_bus *bus, + struct sdw_msg *msg) +{ + struct qcom_swrm_ctrl *ctrl = to_qcom_sdw(bus); + int ret, i, len; + + if (msg->flags == SDW_MSG_FLAG_READ) { + for (i = 0; i < msg->len;) { + if ((msg->len - i) < QCOM_SWRM_MAX_RD_LEN) + len = msg->len - i; + else + len = QCOM_SWRM_MAX_RD_LEN; + + ret = qcom_swrm_cmd_fifo_rd_cmd(ctrl, msg->dev_num, + msg->addr + i, len, + &msg->buf[i]); + if (ret) + return ret; + + i = i + len; + } + } else if (msg->flags == SDW_MSG_FLAG_WRITE) { + for (i = 0; i < msg->len; i++) { + ret = qcom_swrm_cmd_fifo_wr_cmd(ctrl, msg->buf[i], + msg->dev_num, + msg->addr + i); + if (ret) + return SDW_CMD_IGNORED; + } + } + + return SDW_CMD_OK; +} + +static int qcom_swrm_pre_bank_switch(struct sdw_bus *bus) +{ + u32 reg = SWRM_MCP_FRAME_CTRL_BANK_ADDR(bus->params.next_bank); + struct qcom_swrm_ctrl *ctrl = to_qcom_sdw(bus); + u32 val; + int ret; + + ret = ctrl->reg_read(ctrl, reg, &val); + if (ret) + return ret; + + val &= ~SWRM_MCP_FRAME_CTRL_BANK_COL_CTRL_BMSK; + val &= ~SWRM_MCP_FRAME_CTRL_BANK_ROW_CTRL_BMSK; + + val |= (SWRM_MAX_ROW_VAL << SWRM_MCP_FRAME_CTRL_BANK_ROW_CTRL_SHFT | + SWRM_MAX_COL_VAL << SWRM_MCP_FRAME_CTRL_BANK_COL_CTRL_SHFT); + + return ctrl->reg_write(ctrl, reg, val); +} + +static int qcom_swrm_port_params(struct sdw_bus *bus, + struct sdw_port_params *p_params, + unsigned int bank) +{ + /* TBD */ + return 0; +} + +static int qcom_swrm_transport_params(struct sdw_bus *bus, + struct sdw_transport_params *params, + enum sdw_reg_bank bank) +{ + struct qcom_swrm_ctrl *ctrl = to_qcom_sdw(bus); + u32 value; + + value = params->offset1 << SWRM_DP_PORT_CTRL_OFFSET1_SHFT; + value |= params->offset2 << SWRM_DP_PORT_CTRL_OFFSET2_SHFT; + value |= params->sample_interval - 1; + + return ctrl->reg_write(ctrl, + SWRM_DP_PORT_CTRL_BANK((params->port_num), bank), + value); +} + +static int qcom_swrm_port_enable(struct sdw_bus *bus, + struct sdw_enable_ch *enable_ch, + unsigned int bank) +{ + u32 reg = SWRM_DP_PORT_CTRL_BANK(enable_ch->port_num, bank); + struct qcom_swrm_ctrl *ctrl = to_qcom_sdw(bus); + u32 val; + int ret; + + ret = ctrl->reg_read(ctrl, reg, &val); + if (ret) + return ret; + + if (enable_ch->enable) + val |= (enable_ch->ch_mask << SWRM_DP_PORT_CTRL_EN_CHAN_SHFT); + else + val &= ~(enable_ch->ch_mask << SWRM_DP_PORT_CTRL_EN_CHAN_SHFT); + + return ctrl->reg_write(ctrl, reg, val); +} + +static struct sdw_master_port_ops qcom_swrm_port_ops = { + .dpn_set_port_params = qcom_swrm_port_params, + .dpn_set_port_transport_params = qcom_swrm_transport_params, + .dpn_port_enable_ch = qcom_swrm_port_enable, +}; + +static struct sdw_master_ops qcom_swrm_ops = { + .xfer_msg = qcom_swrm_xfer_msg, + .pre_bank_switch = qcom_swrm_pre_bank_switch, +}; + +static int qcom_swrm_compute_params(struct sdw_bus *bus) +{ + struct qcom_swrm_ctrl *ctrl = to_qcom_sdw(bus); + struct sdw_master_runtime *m_rt; + struct sdw_slave_runtime *s_rt; + struct sdw_port_runtime *p_rt; + struct qcom_swrm_port_config *pcfg; + int i = 0; + + list_for_each_entry(m_rt, &bus->m_rt_list, bus_node) { + list_for_each_entry(p_rt, &m_rt->port_list, port_node) { + pcfg = &ctrl->pconfig[p_rt->num - 1]; + p_rt->transport_params.port_num = p_rt->num; + p_rt->transport_params.sample_interval = pcfg->si + 1; + p_rt->transport_params.offset1 = pcfg->off1; + p_rt->transport_params.offset2 = pcfg->off2; + } + + list_for_each_entry(s_rt, &m_rt->slave_rt_list, m_rt_node) { + list_for_each_entry(p_rt, &s_rt->port_list, port_node) { + pcfg = &ctrl->pconfig[i]; + p_rt->transport_params.port_num = p_rt->num; + p_rt->transport_params.sample_interval = + pcfg->si + 1; + p_rt->transport_params.offset1 = pcfg->off1; + p_rt->transport_params.offset2 = pcfg->off2; + i++; + } + } + } + + return 0; +} + +static u32 qcom_swrm_freq_tbl[MAX_FREQ_NUM] = { + DEFAULT_CLK_FREQ, +}; + +static void qcom_swrm_slave_wq(struct work_struct *work) +{ + struct qcom_swrm_ctrl *ctrl = + container_of(work, struct qcom_swrm_ctrl, slave_work); + + qcom_swrm_get_device_status(ctrl); + sdw_handle_slave_status(&ctrl->bus, ctrl->status); +} + +static int qcom_swrm_prepare(struct snd_pcm_substream *substream, + struct snd_soc_dai *dai) +{ + struct qcom_swrm_ctrl *ctrl = dev_get_drvdata(dai->dev); + + if (!ctrl->sruntime[dai->id]) + return -EINVAL; + + return sdw_enable_stream(ctrl->sruntime[dai->id]); +} + +static void qcom_swrm_stream_free_ports(struct qcom_swrm_ctrl *ctrl, + struct sdw_stream_runtime *stream) +{ + struct sdw_master_runtime *m_rt; + struct sdw_port_runtime *p_rt; + unsigned long *port_mask; + + mutex_lock(&ctrl->port_lock); + + list_for_each_entry(m_rt, &stream->master_list, stream_node) { + if (m_rt->direction == SDW_DATA_DIR_RX) + port_mask = &ctrl->dout_port_mask; + else + port_mask = &ctrl->din_port_mask; + + list_for_each_entry(p_rt, &m_rt->port_list, port_node) + clear_bit(p_rt->num - 1, port_mask); + } + + mutex_unlock(&ctrl->port_lock); +} + +static int qcom_swrm_stream_alloc_ports(struct qcom_swrm_ctrl *ctrl, + struct sdw_stream_runtime *stream, + struct snd_pcm_hw_params *params, + int direction) +{ + struct sdw_port_config pconfig[QCOM_SDW_MAX_PORTS]; + struct sdw_stream_config sconfig; + struct sdw_master_runtime *m_rt; + struct sdw_slave_runtime *s_rt; + struct sdw_port_runtime *p_rt; + unsigned long *port_mask; + int i, maxport, pn, nports = 0, ret = 0; + + mutex_lock(&ctrl->port_lock); + list_for_each_entry(m_rt, &stream->master_list, stream_node) { + if (m_rt->direction == SDW_DATA_DIR_RX) { + maxport = ctrl->num_dout_ports; + port_mask = &ctrl->dout_port_mask; + } else { + maxport = ctrl->num_din_ports; + port_mask = &ctrl->din_port_mask; + } + + list_for_each_entry(s_rt, &m_rt->slave_rt_list, m_rt_node) { + list_for_each_entry(p_rt, &s_rt->port_list, port_node) { + /* Port numbers start from 1 - 14*/ + pn = find_first_zero_bit(port_mask, maxport); + if (pn > (maxport - 1)) { + dev_err(ctrl->dev, "All ports busy\n"); + ret = -EBUSY; + goto err; + } + set_bit(pn, port_mask); + pconfig[nports].num = pn + 1; + pconfig[nports].ch_mask = p_rt->ch_mask; + nports++; + } + } + } + + if (direction == SNDRV_PCM_STREAM_CAPTURE) + sconfig.direction = SDW_DATA_DIR_TX; + else + sconfig.direction = SDW_DATA_DIR_RX; + + sconfig.ch_count = 1; + sconfig.frame_rate = params_rate(params); + sconfig.type = stream->type; + sconfig.bps = 1; + sdw_stream_add_master(&ctrl->bus, &sconfig, pconfig, + nports, stream); +err: + if (ret) { + for (i = 0; i < nports; i++) + clear_bit(pconfig[i].num - 1, port_mask); + } + + mutex_unlock(&ctrl->port_lock); + + return ret; +} + +static int qcom_swrm_hw_params(struct snd_pcm_substream *substream, + struct snd_pcm_hw_params *params, + struct snd_soc_dai *dai) +{ + struct qcom_swrm_ctrl *ctrl = dev_get_drvdata(dai->dev); + struct sdw_stream_runtime *sruntime = ctrl->sruntime[dai->id]; + int ret; + + ret = qcom_swrm_stream_alloc_ports(ctrl, sruntime, params, + substream->stream); + if (ret) + return ret; + + ret = sdw_prepare_stream(sruntime); + if (ret) + qcom_swrm_stream_free_ports(ctrl, sruntime); + + return ret; +} + +static int qcom_swrm_hw_free(struct snd_pcm_substream *substream, + struct snd_soc_dai *dai) +{ + struct qcom_swrm_ctrl *ctrl = dev_get_drvdata(dai->dev); + struct sdw_stream_runtime *sruntime = ctrl->sruntime[dai->id]; + + qcom_swrm_stream_free_ports(ctrl, sruntime); + sdw_stream_remove_master(&ctrl->bus, sruntime); + sdw_deprepare_stream(sruntime); + sdw_disable_stream(sruntime); + + return 0; +} + +static void *qcom_pdm_get_sdw_stream(struct snd_soc_dai *dai, + int direction) +{ + struct qcom_swrm_ctrl *ctrl = dev_get_drvdata(dai->dev); + + return ctrl->sruntime[dai->id]; +} + +static int qcom_swrm_startup(struct snd_pcm_substream *stream, + struct snd_soc_dai *dai) +{ + struct qcom_swrm_ctrl *ctrl = dev_get_drvdata(dai->dev); + struct sdw_stream_runtime *sruntime; + + sruntime = sdw_alloc_stream(dai->name); + if (!sruntime) + return -ENOMEM; + + ctrl->sruntime[dai->id] = sruntime; + + return 0; +} + +static void qcom_swrm_shutdown(struct snd_pcm_substream *stream, + struct snd_soc_dai *dai) +{ + struct qcom_swrm_ctrl *ctrl = dev_get_drvdata(dai->dev); + + sdw_release_stream(ctrl->sruntime[dai->id]); + ctrl->sruntime[dai->id] = NULL; +} + +static const struct snd_soc_dai_ops qcom_swrm_pdm_dai_ops = { + .hw_params = qcom_swrm_hw_params, + .prepare = qcom_swrm_prepare, + .hw_free = qcom_swrm_hw_free, + .startup = qcom_swrm_startup, + .shutdown = qcom_swrm_shutdown, + .get_sdw_stream = qcom_pdm_get_sdw_stream, +}; + +static const struct snd_soc_component_driver qcom_swrm_dai_component = { + .name = "soundwire", +}; + +static int qcom_swrm_register_dais(struct qcom_swrm_ctrl *ctrl) +{ + int num_dais = ctrl->num_dout_ports + ctrl->num_din_ports; + struct snd_soc_dai_driver *dais; + struct snd_soc_pcm_stream *stream; + struct device *dev = ctrl->dev; + int i; + + /* PDM dais are only tested for now */ + dais = devm_kcalloc(dev, num_dais, sizeof(*dais), GFP_KERNEL); + if (!dais) + return -ENOMEM; + + for (i = 0; i < num_dais; i++) { + dais[i].name = devm_kasprintf(dev, GFP_KERNEL, "SDW Pin%d", i); + if (!dais[i].name) + return -ENOMEM; + + if (i < ctrl->num_dout_ports) { + stream = &dais[i].playback; + stream->stream_name = devm_kasprintf(dev, GFP_KERNEL, + "SDW Tx%d", i); + } else { + stream = &dais[i].capture; + stream->stream_name = devm_kasprintf(dev, GFP_KERNEL, + "SDW Rx%d", i); + } + + if (!stream->stream_name) + return -ENOMEM; + + stream->channels_min = 1; + stream->channels_max = 1; + stream->rates = SNDRV_PCM_RATE_48000; + stream->formats = SNDRV_PCM_FMTBIT_S16_LE; + + dais[i].ops = &qcom_swrm_pdm_dai_ops; + dais[i].id = i; + } + + return devm_snd_soc_register_component(dev, &qcom_swrm_dai_component, + dais, num_dais); +} + +static int qcom_swrm_get_port_config(struct qcom_swrm_ctrl *ctrl) +{ + struct device_node *np = ctrl->dev->of_node; + u8 off1[QCOM_SDW_MAX_PORTS]; + u8 off2[QCOM_SDW_MAX_PORTS]; + u8 si[QCOM_SDW_MAX_PORTS]; + int i, ret, nports, val; + + ret = ctrl->reg_read(ctrl, SWRM_COMP_PARAMS, &val); + if (ret) + return ret; + + ctrl->num_dout_ports = val & SWRM_COMP_PARAMS_DOUT_PORTS_MASK; + ctrl->num_din_ports = (val & SWRM_COMP_PARAMS_DIN_PORTS_MASK) >> 5; + + ret = of_property_read_u32(np, "qcom,din-ports", &val); + if (ret) + return ret; + + if (val > ctrl->num_din_ports) + return -EINVAL; + + ctrl->num_din_ports = val; + + ret = of_property_read_u32(np, "qcom,dout-ports", &val); + if (ret) + return ret; + + if (val > ctrl->num_dout_ports) + return -EINVAL; + + ctrl->num_dout_ports = val; + + nports = ctrl->num_dout_ports + ctrl->num_din_ports; + + ret = of_property_read_u8_array(np, "qcom,ports-offset1", + off1, nports); + if (ret) + return ret; + + ret = of_property_read_u8_array(np, "qcom,ports-offset2", + off2, nports); + if (ret) + return ret; + + ret = of_property_read_u8_array(np, "qcom,ports-sinterval-low", + si, nports); + if (ret) + return ret; + + for (i = 0; i < nports; i++) { + ctrl->pconfig[i].si = si[i]; + ctrl->pconfig[i].off1 = off1[i]; + ctrl->pconfig[i].off2 = off2[i]; + } + + return 0; +} + +static int qcom_swrm_probe(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + struct sdw_master_prop *prop; + struct sdw_bus_params *params; + struct qcom_swrm_ctrl *ctrl; + struct resource *res; + int ret; + u32 val; + + ctrl = devm_kzalloc(dev, sizeof(*ctrl), GFP_KERNEL); + if (!ctrl) + return -ENOMEM; + + if (dev->parent->bus == &slimbus_bus) { + ctrl->reg_read = qcom_swrm_abh_reg_read; + ctrl->reg_write = qcom_swrm_ahb_reg_write; + ctrl->regmap = dev_get_regmap(dev->parent, NULL); + if (!ctrl->regmap) + return -EINVAL; + } else { + ctrl->reg_read = qcom_swrm_mmio_reg_read; + ctrl->reg_write = qcom_swrm_mmio_reg_write; + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + ctrl->base = devm_ioremap_resource(dev, res); + if (IS_ERR(ctrl->base)) + return PTR_ERR(ctrl->base); + } + + ctrl->irq = of_irq_get(dev->of_node, 0); + if (ctrl->irq < 0) + return ctrl->irq; + + ctrl->hclk = devm_clk_get(dev, "iface"); + if (IS_ERR(ctrl->hclk)) + return PTR_ERR(ctrl->hclk); + + clk_prepare_enable(ctrl->hclk); + + ctrl->dev = dev; + dev_set_drvdata(&pdev->dev, ctrl); + spin_lock_init(&ctrl->comp_lock); + mutex_init(&ctrl->port_lock); + INIT_WORK(&ctrl->slave_work, qcom_swrm_slave_wq); + + ctrl->bus.dev = dev; + ctrl->bus.ops = &qcom_swrm_ops; + ctrl->bus.port_ops = &qcom_swrm_port_ops; + ctrl->bus.compute_params = &qcom_swrm_compute_params; + + ret = qcom_swrm_get_port_config(ctrl); + if (ret) + return ret; + + params = &ctrl->bus.params; + params->max_dr_freq = DEFAULT_CLK_FREQ; + params->curr_dr_freq = DEFAULT_CLK_FREQ; + params->col = SWRM_DEFAULT_COL; + params->row = SWRM_DEFAULT_ROWS; + ctrl->reg_read(ctrl, SWRM_MCP_STATUS, &val); + params->curr_bank = val & SWRM_MCP_STATUS_BANK_NUM_MASK; + params->next_bank = !params->curr_bank; + + prop = &ctrl->bus.prop; + prop->max_clk_freq = DEFAULT_CLK_FREQ; + prop->num_clk_gears = 0; + prop->num_clk_freq = MAX_FREQ_NUM; + prop->clk_freq = &qcom_swrm_freq_tbl[0]; + prop->default_col = SWRM_DEFAULT_COL; + prop->default_row = SWRM_DEFAULT_ROWS; + + ctrl->reg_read(ctrl, SWRM_COMP_HW_VERSION, &ctrl->version); + + ret = devm_request_threaded_irq(dev, ctrl->irq, NULL, + qcom_swrm_irq_handler, + IRQF_TRIGGER_RISING, + "soundwire", ctrl); + if (ret) { + dev_err(dev, "Failed to request soundwire irq\n"); + goto err; + } + + ret = sdw_add_bus_master(&ctrl->bus); + if (ret) { + dev_err(dev, "Failed to register Soundwire controller (%d)\n", + ret); + goto err; + } + + qcom_swrm_init(ctrl); + ret = qcom_swrm_register_dais(ctrl); + if (ret) + goto err; + + dev_info(dev, "Qualcomm Soundwire controller v%x.%x.%x Registered\n", + (ctrl->version >> 24) & 0xff, (ctrl->version >> 16) & 0xff, + ctrl->version & 0xffff); + + return 0; +err: + clk_disable_unprepare(ctrl->hclk); + return ret; +} + +static int qcom_swrm_remove(struct platform_device *pdev) +{ + struct qcom_swrm_ctrl *ctrl = dev_get_drvdata(&pdev->dev); + + sdw_delete_bus_master(&ctrl->bus); + clk_disable_unprepare(ctrl->hclk); + + return 0; +} + +static int qcom_swrm_runtime_suspend(struct device *device) +{ + /* TBD */ + return 0; +} + +static int qcom_swrm_runtime_resume(struct device *device) +{ + /* TBD */ + return 0; +} + +static const struct dev_pm_ops qcom_swrm_dev_pm_ops = { + SET_RUNTIME_PM_OPS(qcom_swrm_runtime_suspend, + qcom_swrm_runtime_resume, + NULL + ) +}; + +static const struct of_device_id qcom_swrm_of_match[] = { + { .compatible = "qcom,soundwire-v1.3.0",}, + { .compatible = "qcom,soundwire-v1.5.0",}, + { .compatible = "qcom,soundwire-v1.6.0",}, + {/* sentinel */}, +}; + +MODULE_DEVICE_TABLE(of, qcom_swrm_of_match); + +static struct platform_driver qcom_swrm_driver = { + .probe = &qcom_swrm_probe, + .remove = &qcom_swrm_remove, + .driver = { + .name = "qcom-soundwire", + .of_match_table = qcom_swrm_of_match, + .pm = &qcom_swrm_dev_pm_ops, + } +}; +module_platform_driver(qcom_swrm_driver); + +MODULE_DESCRIPTION("Qualcomm soundwire driver"); +MODULE_LICENSE("GPL v2"); -- 2.21.0 ^ permalink raw reply related [flat|nested] 44+ messages in thread
end of thread, other threads:[~2019-10-13 1:44 UTC | newest] Thread overview: 44+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-08-13 8:35 [PATCH v2 0/5] soundwire: Add support to Qualcomm SoundWire master Srinivas Kandagatla 2019-08-13 8:35 ` [PATCH v2 1/5] soundwire: Add compute_params callback Srinivas Kandagatla 2019-08-13 14:34 ` Pierre-Louis Bossart 2019-08-13 18:17 ` Srinivas Kandagatla 2019-08-13 19:08 ` Pierre-Louis Bossart 2019-08-14 4:05 ` Vinod Koul 2019-09-04 9:28 ` [alsa-devel] " Vinod Koul 2019-09-04 13:36 ` Pierre-Louis Bossart 2019-08-13 8:35 ` [PATCH v2 2/5] soundwire: stream: make stream name a const pointer Srinivas Kandagatla 2019-09-04 9:29 ` [alsa-devel] " Vinod Koul 2019-08-13 8:35 ` [PATCH v2 3/5] ASoC: core: add support to snd_soc_dai_get_sdw_stream() Srinivas Kandagatla 2019-08-13 14:44 ` [alsa-devel] " Pierre-Louis Bossart 2019-08-13 16:50 ` Srinivas Kandagatla 2019-08-13 17:51 ` Pierre-Louis Bossart 2019-08-13 18:06 ` Srinivas Kandagatla 2019-08-13 19:15 ` Pierre-Louis Bossart 2019-08-13 19:18 ` Mark Brown 2019-08-13 19:38 ` Pierre-Louis Bossart 2019-08-13 19:58 ` Mark Brown 2019-08-14 4:11 ` Vinod Koul 2019-08-14 9:08 ` Mark Brown 2019-08-14 14:09 ` Pierre-Louis Bossart 2019-10-09 8:32 ` Srinivas Kandagatla 2019-10-09 14:29 ` Pierre-Louis Bossart 2019-10-09 14:29 ` Pierre-Louis Bossart 2019-10-09 16:01 ` Srinivas Kandagatla 2019-10-09 16:01 ` Srinivas Kandagatla 2019-10-09 18:53 ` Pierre-Louis Bossart 2019-10-10 8:50 ` Srinivas Kandagatla 2019-10-10 12:03 ` Charles Keepax 2019-10-10 13:51 ` Mark Brown 2019-10-10 14:01 ` Pierre-Louis Bossart 2019-10-10 14:52 ` Srinivas Kandagatla 2019-10-10 15:49 ` Pierre-Louis Bossart 2019-10-11 12:30 ` Srinivas Kandagatla 2019-08-13 16:03 ` Cezary Rojewski 2019-08-13 16:52 ` Srinivas Kandagatla 2019-08-13 17:29 ` Cezary Rojewski 2019-08-13 19:19 ` Mark Brown 2019-08-13 8:35 ` [PATCH v2 4/5] dt-bindings: soundwire: add bindings for Qcom controller Srinivas Kandagatla 2019-08-23 7:28 ` Vinod Koul 2019-08-30 8:05 ` Srinivas Kandagatla 2019-08-30 8:05 ` [alsa-devel] " Srinivas Kandagatla 2019-08-13 8:35 ` [PATCH v2 5/5] soundwire: qcom: add support for SoundWire controller Srinivas Kandagatla
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).