From mboxrd@z Thu Jan 1 00:00:00 1970 From: Pierre-Louis Bossart Subject: Re: [PATCH v4 02/13] soundwire: Add support for SoundWire stream management Date: Sat, 21 Apr 2018 06:02:52 -0700 Message-ID: References: <1524049146-8725-1-git-send-email-vinod.koul@intel.com> <1524049146-8725-3-git-send-email-vinod.koul@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by alsa0.perex.cz (Postfix) with ESMTP id 5EC40267230 for ; Sat, 21 Apr 2018 17:02:33 +0200 (CEST) In-Reply-To: <1524049146-8725-3-git-send-email-vinod.koul@intel.com> Content-Language: en-US List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: alsa-devel-bounces@alsa-project.org To: Vinod Koul , Greg KH Cc: ALSA , tiwai@suse.de, liam.r.girdwood@linux.intel.com, patches.audio@intel.com, broonie@kernel.org, Sanyog Kale List-Id: alsa-devel@alsa-project.org On 4/18/18 3:58 AM, Vinod Koul wrote: > From: Sanyog Kale > > This patch adds APIs and relevant stream data structures > for initialization and release of stream. > > Signed-off-by: Hardik T Shah > Signed-off-by: Sanyog Kale > Signed-off-by: Shreyas NC > Signed-off-by: Vinod Koul > --- > drivers/soundwire/Makefile | 2 +- > drivers/soundwire/bus.c | 1 + > drivers/soundwire/bus.h | 36 +++++ > drivers/soundwire/stream.c | 357 ++++++++++++++++++++++++++++++++++++++++++ > include/linux/soundwire/sdw.h | 109 +++++++++++++ > 5 files changed, 504 insertions(+), 1 deletion(-) > create mode 100644 drivers/soundwire/stream.c > > diff --git a/drivers/soundwire/Makefile b/drivers/soundwire/Makefile > index e1a74c5692aa..5817beaca0e1 100644 > --- a/drivers/soundwire/Makefile > +++ b/drivers/soundwire/Makefile > @@ -3,7 +3,7 @@ > # > > #Bus Objs > -soundwire-bus-objs := bus_type.o bus.o slave.o mipi_disco.o > +soundwire-bus-objs := bus_type.o bus.o slave.o mipi_disco.o stream.o > obj-$(CONFIG_SOUNDWIRE_BUS) += soundwire-bus.o > > #Cadence Objs > diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c > index d6dc8e7a8614..abf046f6b188 100644 > --- a/drivers/soundwire/bus.c > +++ b/drivers/soundwire/bus.c > @@ -32,6 +32,7 @@ int sdw_add_bus_master(struct sdw_bus *bus) > mutex_init(&bus->msg_lock); > mutex_init(&bus->bus_lock); > INIT_LIST_HEAD(&bus->slaves); > + INIT_LIST_HEAD(&bus->m_rt_list); > > if (bus->ops->read_prop) { > ret = bus->ops->read_prop(bus); > diff --git a/drivers/soundwire/bus.h b/drivers/soundwire/bus.h > index 345c34d697e9..2e5043de9a4b 100644 > --- a/drivers/soundwire/bus.h > +++ b/drivers/soundwire/bus.h > @@ -45,6 +45,42 @@ struct sdw_msg { > bool page; > }; > > +/** > + * sdw_slave_runtime: Runtime Stream parameters for Slave > + * > + * @slave: Slave handle > + * @direction: Data direction for Slave > + * @ch_count: Number of channels handled by the Slave for > + * this stream > + * @m_rt_node: sdw_master_runtime list node > + */ > +struct sdw_slave_runtime { > + struct sdw_slave *slave; > + enum sdw_data_direction direction; > + unsigned int ch_count; > + struct list_head m_rt_node; > +}; > + > +/** > + * sdw_master_runtime: Runtime stream parameters for Master > + * > + * @bus: Bus handle > + * @stream: Stream runtime handle > + * @direction: Data direction for Master > + * @ch_count: Number of channels handled by the Master for > + * this stream, can be zero. > + * @slave_rt_list: Slave runtime list > + * @bus_node: sdw_bus m_rt_list node > + */ > +struct sdw_master_runtime { > + struct sdw_bus *bus; > + struct sdw_stream_runtime *stream; > + enum sdw_data_direction direction; > + unsigned int ch_count; > + struct list_head slave_rt_list; > + struct list_head bus_node; > +}; > + > int sdw_transfer(struct sdw_bus *bus, struct sdw_msg *msg); > int sdw_transfer_defer(struct sdw_bus *bus, struct sdw_msg *msg, > struct sdw_defer *defer); > diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c > new file mode 100644 > index 000000000000..1a7fcac8e213 > --- /dev/null > +++ b/drivers/soundwire/stream.c > @@ -0,0 +1,357 @@ > +// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause) > +// Copyright(c) 2015-18 Intel Corporation. > + > +/* > + * stream.c - SoundWire Bus stream operations. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include "bus.h" > + > +/** > + * sdw_release_stream() - Free the assigned stream runtime > + * > + * @stream: SoundWire stream runtime > + * > + * sdw_release_stream should be called only once per stream > + */ > +void sdw_release_stream(struct sdw_stream_runtime *stream) > +{ > + kfree(stream); > +} > +EXPORT_SYMBOL(sdw_release_stream); > + > +/** > + * sdw_alloc_stream() - Allocate and return stream runtime > + * > + * @stream_name: SoundWire stream name > + * > + * Allocates a SoundWire stream runtime instance. > + * 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 *stream; > + > + stream = kzalloc(sizeof(*stream), GFP_KERNEL); > + if (!stream) > + return NULL; > + > + stream->name = stream_name; > + stream->state = SDW_STREAM_ALLOCATED; > + > + return stream; > +} > +EXPORT_SYMBOL(sdw_alloc_stream); > + > +/** > + * sdw_alloc_master_rt() - Allocates and initialize Master runtime handle > + * > + * @bus: SDW bus instance > + * @stream_config: Stream configuration > + * @stream: Stream runtime handle. > + * > + * This function is to be called with bus_lock held. > + */ > +static struct sdw_master_runtime > +*sdw_alloc_master_rt(struct sdw_bus *bus, > + struct sdw_stream_config *stream_config, > + struct sdw_stream_runtime *stream) > +{ > + struct sdw_master_runtime *m_rt; > + > + m_rt = stream->m_rt; > + > + /* > + * check if Master is already allocated (as a result of Slave adding > + * it first), if so skip allocation and go to configure > + */ > + if (m_rt) > + goto stream_config; > + > + m_rt = kzalloc(sizeof(*m_rt), GFP_KERNEL); > + if (!m_rt) > + return NULL; > + > + /* Initialization of Master runtime handle */ > + INIT_LIST_HEAD(&m_rt->slave_rt_list); > + stream->m_rt = m_rt; > + > + list_add_tail(&m_rt->bus_node, &bus->m_rt_list); > + > +stream_config: > + m_rt->ch_count = stream_config->ch_count; > + m_rt->bus = bus; > + m_rt->stream = stream; > + m_rt->direction = stream_config->direction; > + > + return m_rt; > +} > + > +/** > + * sdw_alloc_slave_rt() - Allocate and initialize Slave runtime handle. > + * > + * @slave: Slave handle > + * @stream_config: Stream configuration > + * @stream: Stream runtime handle > + * > + * This function is to be called with bus_lock held. > + */ > +static struct sdw_slave_runtime > +*sdw_alloc_slave_rt(struct sdw_slave *slave, > + struct sdw_stream_config *stream_config, > + struct sdw_stream_runtime *stream) > +{ > + struct sdw_slave_runtime *s_rt = NULL; > + > + s_rt = kzalloc(sizeof(*s_rt), GFP_KERNEL); > + if (!s_rt) > + return NULL; > + > + s_rt->ch_count = stream_config->ch_count; > + s_rt->direction = stream_config->direction; > + s_rt->slave = slave; > + > + return s_rt; > +} > + > +/** > + * sdw_release_slave_stream() - Free Slave(s) runtime handle > + * > + * @slave: Slave handle. > + * @stream: Stream runtime handle. > + * > + * This function is to be called with bus_lock held. > + */ > +static void sdw_release_slave_stream(struct sdw_slave *slave, > + struct sdw_stream_runtime *stream) > +{ > + struct sdw_slave_runtime *s_rt, *_s_rt; > + struct sdw_master_runtime *m_rt = stream->m_rt; > + > + /* Retrieve Slave runtime handle */ > + list_for_each_entry_safe(s_rt, _s_rt, > + &m_rt->slave_rt_list, m_rt_node) { > + > + if (s_rt->slave == slave) { > + list_del(&s_rt->m_rt_node); > + kfree(s_rt); > + return; > + } > + } > +} > + > +/** > + * sdw_release_master_stream() - Free Master runtime handle > + * > + * @stream: Stream runtime handle. > + * > + * This function is to be called with bus_lock held > + * It frees the Master runtime handle and associated Slave(s) runtime > + * handle. If this is called first then sdw_release_slave_stream() will have > + * no effect as Slave(s) runtime handle would already be freed up. > + */ > +static void sdw_release_master_stream(struct sdw_stream_runtime *stream) > +{ > + struct sdw_master_runtime *m_rt = stream->m_rt; > + struct sdw_slave_runtime *s_rt, *_s_rt; > + > + list_for_each_entry_safe(s_rt, _s_rt, > + &m_rt->slave_rt_list, m_rt_node) > + sdw_stream_remove_slave(s_rt->slave, stream); > + > + list_del(&m_rt->bus_node); > +} > + > +/** > + * sdw_stream_remove_master() - Remove master from sdw_stream > + * > + * @bus: SDW Bus instance > + * @stream: SoundWire stream > + * > + * This removes and frees master_rt from a stream > + */ > +int sdw_stream_remove_master(struct sdw_bus *bus, > + struct sdw_stream_runtime *stream) > +{ > + mutex_lock(&bus->bus_lock); > + > + sdw_release_master_stream(stream); > + stream->state = SDW_STREAM_RELEASED; > + kfree(stream->m_rt); > + stream->m_rt = NULL; > + > + mutex_unlock(&bus->bus_lock); > + > + return 0; > +} > +EXPORT_SYMBOL(sdw_stream_remove_master); > + > +/** > + * sdw_stream_remove_slave() - Remove slave from sdw_stream > + * > + * @slave: SDW Slave instance > + * @stream: SoundWire stream > + * > + * This removes and frees slave_rt from a stream > + */ > +int sdw_stream_remove_slave(struct sdw_slave *slave, > + struct sdw_stream_runtime *stream) > +{ > + mutex_lock(&slave->bus->bus_lock); > + > + sdw_release_slave_stream(slave, stream); > + > + mutex_unlock(&slave->bus->bus_lock); > + > + return 0; > +} > +EXPORT_SYMBOL(sdw_stream_remove_slave); > + > +/** > + * sdw_config_stream() - Configure the allocated stream > + * > + * @dev: SDW device > + * @stream: SoundWire stream > + * @stream_config: Stream configuration for audio stream > + * @is_slave: is API called from Slave or Master > + * > + * This function is to be called with bus_lock held. > + */ > +static int sdw_config_stream(struct device *dev, > + struct sdw_stream_runtime *stream, > + struct sdw_stream_config *stream_config, bool is_slave) > +{ > + > + /* > + * Update the stream rate, channel and bps based on data > + * source. For more than one data source (multilink), > + * match the rate, bps, stream type and increment number of channels. > + */ > + if ((stream->params.rate) && so if the rate is zero there is no error? > + (stream->params.rate != stream_config->frame_rate)) { > + dev_err(dev, "rate not matching, stream:%s", stream->name); > + return -EINVAL; > + } > + > + if ((stream->params.bps) && same here, what is the intent behind the first test? > + (stream->params.bps != stream_config->bps)) { > + dev_err(dev, "bps not matching, stream:%s", stream->name); > + return -EINVAL; > + } > + > + stream->type = stream_config->type; > + stream->params.rate = stream_config->frame_rate; > + stream->params.bps = stream_config->bps; > + if (is_slave) > + stream->params.ch_count += stream_config->ch_count; Add comment or TODO that this does not work in device-to-device communication. This is a known limitation that needs to be tracked. > + > + return 0; > +} > + > +/** > + * sdw_stream_add_master() - Allocate and add master runtime to a stream > + * > + * @bus: SDW Bus instance > + * @stream_config: Stream configuration for audio stream > + * @stream: SoundWire stream > + */ > +int sdw_stream_add_master(struct sdw_bus *bus, > + struct sdw_stream_config *stream_config, > + struct sdw_stream_runtime *stream) > +{ > + struct sdw_master_runtime *m_rt = NULL; > + int ret; > + > + mutex_lock(&bus->bus_lock); > + > + m_rt = sdw_alloc_master_rt(bus, stream_config, stream); > + if (!m_rt) { > + dev_err(bus->dev, > + "Master runtime config failed for stream:%s", > + stream->name); > + ret = -ENOMEM; > + goto error; > + } > + > + ret = sdw_config_stream(bus->dev, stream, stream_config, false); > + if (ret) > + goto stream_error; > + > + stream->state = SDW_STREAM_CONFIGURED; > + > +stream_error: > + sdw_release_master_stream(stream); > +error: > + mutex_unlock(&bus->bus_lock); > + return ret; > + > +} > +EXPORT_SYMBOL(sdw_stream_add_master); > + > +/** > + * sdw_stream_add_slave() - Allocate and add master/slave runtime to a stream > + * > + * @slave: SDW Slave instance > + * @stream_config: Stream configuration for audio stream > + * @stream: SoundWire stream > + */ > +int sdw_stream_add_slave(struct sdw_slave *slave, > + struct sdw_stream_config *stream_config, > + struct sdw_stream_runtime *stream) > +{ > + struct sdw_slave_runtime *s_rt; > + struct sdw_master_runtime *m_rt; > + int ret; > + > + mutex_lock(&slave->bus->bus_lock); > + > + /* > + * If this API is invoked by Slave first then m_rt is not valid. > + * So, allocate m_rt and add Slave to it. > + */ > + m_rt = sdw_alloc_master_rt(slave->bus, stream_config, stream); > + if (!m_rt) { > + dev_err(&slave->dev, > + "alloc master runtime failed for stream:%s", > + stream->name); > + ret = -ENOMEM; > + goto error; > + } > + > + s_rt = sdw_alloc_slave_rt(slave, stream_config, stream); > + if (!s_rt) { > + dev_err(&slave->dev, > + "Slave runtime config failed for stream:%s", > + stream->name); > + ret = -ENOMEM; > + goto stream_error; > + } > + > + ret = sdw_config_stream(&slave->dev, stream, stream_config, true); > + if (ret) > + goto stream_error; > + > + list_add_tail(&s_rt->m_rt_node, &m_rt->slave_rt_list); > + > + stream->state = SDW_STREAM_CONFIGURED; > + goto error; > + > +stream_error: > + /* > + * we hit error so cleanup the stream, release all Slave(s) and > + * Master runtime > + */ > + sdw_release_master_stream(stream); > +error: > + mutex_unlock(&slave->bus->bus_lock); > + return ret; > +} > +EXPORT_SYMBOL(sdw_stream_add_slave); > diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h > index e91fdcf41049..7a41e3860efc 100644 > --- a/include/linux/soundwire/sdw.h > +++ b/include/linux/soundwire/sdw.h > @@ -61,6 +61,30 @@ enum sdw_command_response { > SDW_CMD_FAIL_OTHER = 4, > }; > > +/** > + * enum sdw_stream_type: data stream type > + * > + * @SDW_STREAM_PCM: PCM data stream > + * @SDW_STREAM_PDM: PDM data stream > + * > + * spec doesn't define this, but is used in implementation > + */ > +enum sdw_stream_type { > + SDW_STREAM_PCM = 0, > + SDW_STREAM_PDM = 1, > +}; > + > +/** > + * enum sdw_data_direction: Data direction > + * > + * @SDW_DATA_DIR_RX: Data into Port > + * @SDW_DATA_DIR_TX: Data out of Port > + */ > +enum sdw_data_direction { > + SDW_DATA_DIR_RX = 0, > + SDW_DATA_DIR_TX = 1, > +}; > + > /* > * SDW properties, defined in MIPI DisCo spec v1.0 > */ > @@ -450,6 +474,9 @@ struct sdw_master_ops { > * @msg_lock: message lock > * @ops: Master callback ops > * @prop: Master properties > + * @m_rt_list: List of Master instance of all stream(s) running on Bus. This > + * is used to compute and program bus bandwidth, clock, frame shape, > + * transport and port parameters > * @defer_msg: Defer message > * @clk_stop_timeout: Clock stop timeout computed > */ > @@ -462,6 +489,7 @@ struct sdw_bus { > struct mutex msg_lock; > const struct sdw_master_ops *ops; > struct sdw_master_prop prop; > + struct list_head m_rt_list; > struct sdw_defer defer_msg; > unsigned int clk_stop_timeout; > }; > @@ -469,6 +497,87 @@ struct sdw_bus { > int sdw_add_bus_master(struct sdw_bus *bus); > void sdw_delete_bus_master(struct sdw_bus *bus); > > +/** > + * sdw_stream_config: Master or Slave stream configuration > + * > + * @frame_rate: Audio frame rate of the stream, in Hz > + * @ch_count: Channel count of the stream > + * @bps: Number of bits per audio sample > + * @direction: Data direction > + * @type: Stream type PCM or PDM > + */ > +struct sdw_stream_config { > + unsigned int frame_rate; > + unsigned int ch_count; > + unsigned int bps; > + enum sdw_data_direction direction; > + enum sdw_stream_type type; > +}; > + > +/** > + * sdw_stream_state: Stream states > + * > + * @SDW_STREAM_RELEASED: Stream released > + * @SDW_STREAM_ALLOCATED: New stream allocated. > + * @SDW_STREAM_CONFIGURED: Stream configured > + * @SDW_STREAM_PREPARED: Stream prepared > + * @SDW_STREAM_ENABLED: Stream enabled > + * @SDW_STREAM_DISABLED: Stream disabled > + * @SDW_STREAM_DEPREPARED: Stream de-prepared > + */ > +enum sdw_stream_state { > + SDW_STREAM_ALLOCATED = 0, with the kzalloc used in sdw_alloc_stream() the default state is ALLOCATED...You may want to use values which don't include zero. > + SDW_STREAM_CONFIGURED = 1, > + SDW_STREAM_PREPARED = 2, > + SDW_STREAM_ENABLED = 3, > + SDW_STREAM_DISABLED = 4, > + SDW_STREAM_DEPREPARED = 5, > + SDW_STREAM_RELEASED = 6, > +}; > + > +/** > + * sdw_stream_params: Stream parameters > + * > + * @rate: Sampling frequency, in Hz > + * @ch_count: Number of channels > + * @bps: bits per channel sample > + */ > +struct sdw_stream_params { > + unsigned int rate; > + unsigned int ch_count; > + unsigned int bps; > +}; > + > +/** > + * sdw_stream_runtime: Runtime stream parameters > + * > + * @name: SoundWire stream name > + * @params: Stream parameters > + * @state: Current state of the stream > + * @type: Stream type PCM or PDM > + * @m_rt: Master runtime > + */ > +struct sdw_stream_runtime { > + char *name; > + struct sdw_stream_params params; > + enum sdw_stream_state state; > + enum sdw_stream_type type; > + struct sdw_master_runtime *m_rt; > +}; > + > +struct sdw_stream_runtime *sdw_alloc_stream(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, > + struct sdw_stream_runtime *stream); > +int sdw_stream_add_slave(struct sdw_slave *slave, > + struct sdw_stream_config *stream_config, > + struct sdw_stream_runtime *stream); > +int sdw_stream_remove_master(struct sdw_bus *bus, > + struct sdw_stream_runtime *stream); > +int sdw_stream_remove_slave(struct sdw_slave *slave, > + struct sdw_stream_runtime *stream); > + > /* messaging and data APIs */ > > int sdw_read(struct sdw_slave *slave, u32 addr); >