[v4,02/13] soundwire: Add support for SoundWire stream management

Commit Message

Vinod Koul April 18, 2018, 10:58 a.m. UTC

From: Sanyog Kale <[e-mail]>

This patch adds APIs and relevant stream data structures
for initialization and release of stream.

Signed-off-by: Hardik T Shah <[e-mail]>
Signed-off-by: Sanyog Kale <[e-mail]>
Signed-off-by: Shreyas NC <[e-mail]>
Signed-off-by: Vinod Koul <[e-mail]>
---
 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

Comments

Pierre-Louis Bossart April 21, 2018, 1:02 p.m. UTC | #1

On 4/18/18 3:58 AM, Vinod Koul wrote:
> From: Sanyog Kale <[e-mail]>
> 
> This patch adds APIs and relevant stream data structures
> for initialization and release of stream.
> 
> Signed-off-by: Hardik T Shah <[e-mail]>
> Signed-off-by: Sanyog Kale <[e-mail]>
> Signed-off-by: Shreyas NC <[e-mail]>
> Signed-off-by: Vinod Koul <[e-mail]>
> ---
>   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 <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/slab.h>
> +#include <linux/soundwire/sdw.h>
> +#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);
>

Vinod Koul April 21, 2018, 4:17 p.m. UTC | #2

On Sat, Apr 21, 2018 at 06:02:52AM -0700, Pierre-Louis Bossart wrote:
> On 4/18/18 3:58 AM, Vinod Koul wrote:

> >+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?

IIRC this was to check for valid values, but am not too sure, let me get
back to you on these two..

> 
> >+			(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.

Yes limitation for a feature that we dont support yet. So i dont think we
need to do anything atm for this. When the support for device-to-device
shows up, that would update this and other conditions valid only for specific
cases.

> >+/**
> >+ * 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.

How would that help. If we use kzalloc then stream init to
SDW_STREAM_ALLOCATED is no op. If we dont then it initializes... so seems
better to me!

Pierre-Louis Bossart April 23, 2018, 1:25 p.m. UTC | #3

>>
>>> +			(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.
> 
> Yes limitation for a feature that we dont support yet. So i dont think we
> need to do anything atm for this. When the support for device-to-device
> shows up, that would update this and other conditions valid only for specific
> cases.

so if you don't support it yet, add a comment that this will have to be 
modified. This is all I am asking so that this limitation is known and 
the parts that have to be changed identified.

Vinod Koul April 24, 2018, 9:02 a.m. UTC | #4

On Mon, Apr 23, 2018 at 08:25:21AM -0500, Pierre-Louis Bossart wrote:
> 
> >>
> >>>+			(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.
> >
> >Yes limitation for a feature that we dont support yet. So i dont think we
> >need to do anything atm for this. When the support for device-to-device
> >shows up, that would update this and other conditions valid only for specific
> >cases.
> 
> so if you don't support it yet, add a comment that this will have to be
> modified. This is all I am asking so that this limitation is known and the
> parts that have to be changed identified.

adding a TODO here

Vinod Koul April 24, 2018, 9:25 a.m. UTC | #5

On Sat, Apr 21, 2018 at 09:47:56PM +0530, Vinod Koul wrote:
> 
> On Sat, Apr 21, 2018 at 06:02:52AM -0700, Pierre-Louis Bossart wrote:
> > On 4/18/18 3:58 AM, Vinod Koul wrote:
> 
> > >+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?
> 
> IIRC this was to check for valid values, but am not too sure, let me get
> back to you on these two..

So if the value previously is not set, we want to allow it be set and then
compare. We are not checking for zero value or wrong values here, we are checking
fir values to be matched

Since you have asked this second time, I am going to add a comment here to
clarify..

Patch

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 <linux/delay.h>
+#include <linux/device.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/slab.h>
+#include <linux/soundwire/sdw.h>
+#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) &&
+			(stream->params.rate != stream_config->frame_rate)) {
+		dev_err(dev, "rate not matching, stream:%s", stream->name);
+		return -EINVAL;
+	}
+
+	if ((stream->params.bps) &&
+			(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;
+
+	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,
+	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);