Re: [alsa-devel] [PATCH v2 4/6] soundwire: Handle multiple master instances in a stream

On 06/12/2018 05:53 AM, Shreyas NC wrote:

From: Vinod Koul vkoul@kernel.org

For each SoundWire stream operation, we need to parse master list and operate upon all master runtime.

This patch does the boilerplate conversion of stream handling from single master runtime to handle a list of master runtime.

Signed-off-by: Sanyog Kale sanyog.r.kale@intel.com Signed-off-by: Vinod Koul vkoul@kernel.org Signed-off-by: Shreyas NC shreyas.nc@intel.com

---

drivers/soundwire/stream.c | 308 +++++++++++++++++++++++++----------------- include/linux/soundwire/sdw.h | 2 - 2 files changed, 185 insertions(+), 125 deletions(-)

diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c index eb942c6..36506a7 100644 --- a/drivers/soundwire/stream.c +++ b/drivers/soundwire/stream.c @@ -681,35 +681,44 @@ static int sdw_bank_switch(struct sdw_bus *bus)

static int do_bank_switch(struct sdw_stream_runtime *stream) {

• struct sdw_master_runtime *m_rt = stream->m_rt;

• struct sdw_master_runtime *m_rt = NULL; const struct sdw_master_ops *ops;

• struct sdw_bus *bus = m_rt->bus;

• struct sdw_bus *bus = NULL; int ret = 0;

• ops = bus->ops;

• /* Pre-bank switch */

• if (ops->pre_bank_switch) {

• ret = ops->pre_bank_switch(bus);
  

• list_for_each_entry(m_rt, &stream->master_list, stream_node) {

• bus = m_rt->bus;
  

• ops = bus->ops;
  

• /* Pre-bank switch */
  

• if (ops->pre_bank_switch) {
  

• 	ret = ops->pre_bank_switch(bus);
  

• 	if (ret < 0) {
  

• 		dev_err(bus->dev,
  

• 			"Pre bank switch op failed: %d", ret);
  

• 		return ret;
  

• 	}
  

• }
  

• /* Bank switch */
  

• ret = sdw_bank_switch(bus);
  

  if (ret < 0) {

• 	dev_err(bus->dev, "Pre bank switch op failed: %d", ret);
  

• 	dev_err(bus->dev, "Bank switch failed: %d", ret);
  return ret;
  

  }

You probably want to add a comment that in multi-link operation the actual bank_switch happens later and is done in a synchronized manner. This is lost if you just move code around and move the bank_switch into a loop.

I am actually no longer clear just looking at this code on when the bank_switch happens (i know we've discussed this before but I am trying to find out just based on this v2 instead of projecting how I think it should work): In Patch 6/6 it's pretty obvious that the bank switch happens when the SyncGO bit is set, but there is no comment or explanation on how we reach the intel_post_bank_switch() routine once for all masters handling a stream when everything is based on loops. Clearly the intel_pre_bank_switch is called multiple times (once per master), I guess I am missing what the trigger is for the intel_post_bank_switch() routine to be invoked?

}

• /* Bank switch */
• ret = sdw_bank_switch(bus);
• if (ret < 0) {

• dev_err(bus->dev, "Bank switch failed: %d", ret);
  

• return ret;
  

• }
• /* Post-bank switch */
• if (ops->post_bank_switch) {

• ret = ops->post_bank_switch(bus);
  

• if (ret < 0) {
  

• 	dev_err(bus->dev,
  

• list_for_each_entry(m_rt, &stream->master_list, stream_node) {

• bus = m_rt->bus;
  

• ops = bus->ops;
  

• if (ops->post_bank_switch) {
  

• 	ret = ops->post_bank_switch(bus);
  

• 	if (ret < 0) {
  

• 		dev_err(bus->dev,
  		"Post bank switch op failed: %d", ret);
  

• 	}
  

  } }

@@ -754,6 +763,21 @@ struct sdw_stream_runtime *sdw_alloc_stream(char *stream_name) } EXPORT_SYMBOL(sdw_alloc_stream);

+static struct sdw_master_runtime +*sdw_find_master_rt(struct sdw_bus *bus,

• 	struct sdw_stream_runtime *stream)
  

+{

• struct sdw_master_runtime *m_rt = NULL;
• /* Retrieve Bus handle if already available */
• list_for_each_entry(m_rt, &stream->master_list, stream_node) {

• if (m_rt->bus == bus)
  

• 	return m_rt;
  

• }
• return NULL;

+}

• /**
  • sdw_alloc_master_rt() - Allocates and initialize Master runtime handle

@@ -770,12 +794,11 @@ static struct sdw_master_runtime { 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
  */

• m_rt = sdw_find_master_rt(bus, stream); if (m_rt) goto stream_config;

@@ -786,7 +809,7 @@ static struct sdw_master_runtime /* Initialization of Master runtime handle */ INIT_LIST_HEAD(&m_rt->port_list); INIT_LIST_HEAD(&m_rt->slave_rt_list);

• stream->m_rt = m_rt;

• list_add_tail(&m_rt->stream_node, &stream->master_list);

  list_add_tail(&m_rt->bus_node, &bus->m_rt_list);

@@ -844,17 +867,21 @@ static void sdw_slave_port_release(struct sdw_bus *bus, struct sdw_stream_runtime *stream) { struct sdw_port_runtime *p_rt, *_p_rt;

• struct sdw_master_runtime *m_rt = stream->m_rt;

• struct sdw_master_runtime *m_rt; struct sdw_slave_runtime *s_rt;

• list_for_each_entry(s_rt, &m_rt->slave_rt_list, m_rt_node) {

• if (s_rt->slave != slave)
  

• 	continue;
  

• list_for_each_entry(m_rt, &stream->master_list, stream_node) {

• list_for_each_entry(s_rt, &m_rt->slave_rt_list, m_rt_node) {
  
  

• list_for_each_entry_safe(p_rt, _p_rt,
  

• 		&s_rt->port_list, port_node) {
  

• 	list_del(&p_rt->port_node);
  

• 	kfree(p_rt);
  

• 	if (s_rt->slave != slave)
  

• 		continue;
  

• 	list_for_each_entry_safe(p_rt, _p_rt,
  

• 			&s_rt->port_list, port_node) {
  

• 		list_del(&p_rt->port_node);
  

• 		kfree(p_rt);
  

• 	}
  

  } } }

@@ -871,16 +898,18 @@ 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) {
  

• struct sdw_master_runtime *m_rt;

• if (s_rt->slave == slave) {
  

• 	list_del(&s_rt->m_rt_node);
  

• 	kfree(s_rt);
  

• 	return;
  

• list_for_each_entry(m_rt, &stream->master_list, stream_node) {

• /* 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;
  

• 	}
  

  } } }

@@ -888,6 +917,7 @@ static void sdw_release_slave_stream(struct sdw_slave *slave, /**

• sdw_release_master_stream() - Free Master runtime handle

• • @m_rt: Master runtime node
  • @stream: Stream runtime handle.
  • This function is to be called with bus_lock held

@@ -895,16 +925,18 @@ static void sdw_release_slave_stream(struct sdw_slave *slave,

• 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) +static void sdw_release_master_stream(struct sdw_master_runtime *m_rt,

• 	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->stream_node); list_del(&m_rt->bus_node);

• kfree(m_rt); }

  /**

@@ -918,13 +950,22 @@ static void sdw_release_master_stream(struct sdw_stream_runtime *stream) int sdw_stream_remove_master(struct sdw_bus *bus, struct sdw_stream_runtime *stream) {

• struct sdw_master_runtime *m_rt, *_m_rt;
• mutex_lock(&bus->bus_lock);

• sdw_release_master_stream(stream);
• sdw_master_port_release(bus, stream->m_rt);
• stream->state = SDW_STREAM_RELEASED;
• kfree(stream->m_rt);
• stream->m_rt = NULL;

• list_for_each_entry_safe(m_rt, _m_rt,

• 	&stream->master_list, stream_node) {
  

• if (m_rt->bus != bus)
  

• 	continue;
  

• sdw_master_port_release(bus, m_rt);
  

• sdw_release_master_stream(m_rt, stream);
  

• }

• if (list_empty(&stream->master_list))

• stream->state = SDW_STREAM_RELEASED;
  
  

  mutex_unlock(&bus->bus_lock);

@@ -1127,7 +1168,7 @@ int sdw_stream_add_master(struct sdw_bus *bus, stream->state = SDW_STREAM_CONFIGURED;

stream_error:

• sdw_release_master_stream(stream);

• sdw_release_master_stream(m_rt, stream); error: mutex_unlock(&bus->bus_lock); return ret;

@@ -1195,7 +1236,7 @@ int sdw_stream_add_slave(struct sdw_slave *slave, * we hit error so cleanup the stream, release all Slave(s) and * Master runtime */

• sdw_release_master_stream(stream);

• sdw_release_master_stream(m_rt, stream); error: mutex_unlock(&slave->bus->bus_lock); return ret;

@@ -1277,31 +1318,36 @@ static void sdw_release_bus_lock(struct sdw_stream_runtime *stream)

static int _sdw_prepare_stream(struct sdw_stream_runtime *stream) {

• struct sdw_master_runtime *m_rt = stream->m_rt;
• struct sdw_bus *bus = m_rt->bus;

• struct sdw_master_runtime *m_rt = NULL;
• struct sdw_bus *bus = NULL; struct sdw_master_prop *prop = NULL; struct sdw_bus_params params; int ret;

• prop = &bus->prop;
• memcpy(&params, &bus->params, sizeof(params));

• /* Prepare Master(s) and Slave(s) port(s) associated with stream */
• list_for_each_entry(m_rt, &stream->master_list, stream_node) {

• bus = m_rt->bus;
  

• prop = &bus->prop;
  

• memcpy(&params, &bus->params, sizeof(params));
  
  

• /* TODO: Support Asynchronous mode */
• if ((prop->max_freq % stream->params.rate) != 0) {

• dev_err(bus->dev, "Async mode not supported");
  

• return -EINVAL;
  

• }

• /* TODO: Support Asynchronous mode */
  

• if ((prop->max_freq % stream->params.rate) != 0) {
  

• 	dev_err(bus->dev, "Async mode not supported");
  

• 	return -EINVAL;
  

• }
  
  

• /* Increment cumulative bus bandwidth */
• /* TODO: Update this during Device-Device support */
• bus->params.bandwidth += m_rt->stream->params.rate *

• m_rt->ch_count * m_rt->stream->params.bps;
  

• /* Increment cumulative bus bandwidth */
  

• /* TODO: Update this during Device-Device support */
  

• bus->params.bandwidth += m_rt->stream->params.rate *
  

• 	m_rt->ch_count * m_rt->stream->params.bps;
  

• /* Program params */
  

• ret = sdw_program_params(bus);
  

• if (ret < 0) {
  

• 	dev_err(bus->dev, "Program params failed: %d", ret);
  

• 	goto restore_params;
  

• }
  
  

• /* Program params */

• ret = sdw_program_params(bus);

• if (ret < 0) {

• dev_err(bus->dev, "Program params failed: %d", ret);
  

• goto restore_params;
  

  }

  ret = do_bank_switch(stream);

@@ -1310,12 +1356,16 @@ static int _sdw_prepare_stream(struct sdw_stream_runtime *stream) goto restore_params; }

• /* Prepare port(s) on the new clock configuration */
• ret = sdw_prep_deprep_ports(m_rt, true);
• if (ret < 0) {

• dev_err(bus->dev, "Prepare port(s) failed ret = %d",
  

• 		ret);
  

• return ret;
  

• list_for_each_entry(m_rt, &stream->master_list, stream_node) {

• bus = m_rt->bus;
  

• /* Prepare port(s) on the new clock configuration */
  

• ret = sdw_prep_deprep_ports(m_rt, true);
  

• if (ret < 0) {
  

• 	dev_err(bus->dev, "Prepare port(s) failed ret = %d",
  

• 			ret);
  

• 	return ret;
  

• }
  

  }

  stream->state = SDW_STREAM_PREPARED;

@@ -1343,35 +1393,40 @@ int sdw_prepare_stream(struct sdw_stream_runtime *stream) return -EINVAL; }

• mutex_lock(&stream->m_rt->bus->bus_lock);

• sdw_acquire_bus_lock(stream);

  ret = _sdw_prepare_stream(stream); if (ret < 0) pr_err("Prepare for stream:%s failed: %d", stream->name, ret);

• mutex_unlock(&stream->m_rt->bus->bus_lock);

• sdw_release_bus_lock(stream); return ret; } EXPORT_SYMBOL(sdw_prepare_stream);

  static int _sdw_enable_stream(struct sdw_stream_runtime *stream) {

• struct sdw_master_runtime *m_rt = stream->m_rt;
• struct sdw_bus *bus = m_rt->bus;

• struct sdw_master_runtime *m_rt = NULL;
• struct sdw_bus *bus = NULL; int ret;

• /* Program params */
• ret = sdw_program_params(bus);
• if (ret < 0) {

• dev_err(bus->dev, "Program params failed: %d", ret);
  

• return ret;
  

• }

• /* Enable Master(s) and Slave(s) port(s) associated with stream */
• list_for_each_entry(m_rt, &stream->master_list, stream_node) {

• bus = m_rt->bus;
  
  

• /* Enable port(s) */
• ret = sdw_enable_disable_ports(m_rt, true);
• if (ret < 0) {

• dev_err(bus->dev, "Enable port(s) failed ret: %d", ret);
  

• return ret;
  

• /* Program params */
  

• ret = sdw_program_params(bus);
  

• if (ret < 0) {
  

• 	dev_err(bus->dev, "Program params failed: %d", ret);
  

• 	return ret;
  

• }
  

• /* Enable port(s) */
  

• ret = sdw_enable_disable_ports(m_rt, true);
  

• if (ret < 0) {
  

• 	dev_err(bus->dev, "Enable port(s) failed ret: %d", ret);
  

• 	return ret;
  

• }
  

  }

  ret = do_bank_switch(stream);

@@ -1400,37 +1455,42 @@ int sdw_enable_stream(struct sdw_stream_runtime *stream) return -EINVAL; }

• mutex_lock(&stream->m_rt->bus->bus_lock);

• sdw_acquire_bus_lock(stream);

  ret = _sdw_enable_stream(stream); if (ret < 0) pr_err("Enable for stream:%s failed: %d", stream->name, ret);

• mutex_unlock(&stream->m_rt->bus->bus_lock);

• sdw_release_bus_lock(stream); return ret; } EXPORT_SYMBOL(sdw_enable_stream);

  static int _sdw_disable_stream(struct sdw_stream_runtime *stream) {

• struct sdw_master_runtime *m_rt = stream->m_rt;
• struct sdw_bus *bus = m_rt->bus;

• struct sdw_master_runtime *m_rt = NULL;
• struct sdw_bus *bus = NULL; int ret;

• /* Disable port(s) */
• ret = sdw_enable_disable_ports(m_rt, false);
• if (ret < 0) {

• dev_err(bus->dev, "Disable port(s) failed: %d", ret);
  

• return ret;
  

• list_for_each_entry(m_rt, &stream->master_list, stream_node) {

• bus = m_rt->bus;
  

• /* Disable port(s) */
  

• ret = sdw_enable_disable_ports(m_rt, false);
  

• if (ret < 0) {
  

• 	dev_err(bus->dev, "Disable port(s) failed: %d", ret);
  

• 	return ret;
  

• }
  

  }

• stream->state = SDW_STREAM_DISABLED;

• /* Program params */

• ret = sdw_program_params(bus);

• if (ret < 0) {

• dev_err(bus->dev, "Program params failed: %d", ret);
  

• return ret;
  

• list_for_each_entry(m_rt, &stream->master_list, stream_node) {

• bus = m_rt->bus;
  

• /* Program params */
  

• ret = sdw_program_params(bus);
  

• if (ret < 0) {
  

• 	dev_err(bus->dev, "Program params failed: %d", ret);
  

• 	return ret;
  

• }
  

  }

  return do_bank_switch(stream);

@@ -1452,43 +1512,46 @@ int sdw_disable_stream(struct sdw_stream_runtime *stream) return -EINVAL; }

• mutex_lock(&stream->m_rt->bus->bus_lock);

• sdw_acquire_bus_lock(stream);

  ret = _sdw_disable_stream(stream); if (ret < 0) pr_err("Disable for stream:%s failed: %d", stream->name, ret);

• mutex_unlock(&stream->m_rt->bus->bus_lock);

• sdw_release_bus_lock(stream); return ret; } EXPORT_SYMBOL(sdw_disable_stream);

  static int _sdw_deprepare_stream(struct sdw_stream_runtime *stream) {

• struct sdw_master_runtime *m_rt = stream->m_rt;
• struct sdw_bus *bus = m_rt->bus;

• struct sdw_master_runtime *m_rt = NULL;
• struct sdw_bus *bus = NULL; int ret = 0;

• /* De-prepare port(s) */
• ret = sdw_prep_deprep_ports(m_rt, false);
• if (ret < 0) {

• dev_err(bus->dev, "De-prepare port(s) failed: %d", ret);
  

• return ret;
  

• }

• list_for_each_entry(m_rt, &stream->master_list, stream_node) {

• bus = m_rt->bus;
  

• /* De-prepare port(s) */
  

• ret = sdw_prep_deprep_ports(m_rt, false);
  

• if (ret < 0) {
  

• 	dev_err(bus->dev, "De-prepare port(s) failed: %d", ret);
  

• 	return ret;
  

• }
  
  

• stream->state = SDW_STREAM_DEPREPARED;

• /* TODO: Update this during Device-Device support */
  

• bus->params.bandwidth -= m_rt->stream->params.rate *
  

• 	m_rt->ch_count * m_rt->stream->params.bps;
  
  

• /* TODO: Update this during Device-Device support */
• bus->params.bandwidth -= m_rt->stream->params.rate *

• m_rt->ch_count * m_rt->stream->params.bps;
  

• /* Program params */
  

• ret = sdw_program_params(bus);
  

• if (ret < 0) {
  

• 	dev_err(bus->dev, "Program params failed: %d", ret);
  

• 	return ret;
  

• }
  
  

• /* Program params */
• ret = sdw_program_params(bus);
• if (ret < 0) {

• dev_err(bus->dev, "Program params failed: %d", ret);
  

• return ret;
  

  }

• stream->state = SDW_STREAM_DEPREPARED; return do_bank_switch(stream); }

@@ -1508,13 +1571,12 @@ int sdw_deprepare_stream(struct sdw_stream_runtime *stream) return -EINVAL; }

• mutex_lock(&stream->m_rt->bus->bus_lock);

• sdw_acquire_bus_lock(stream); ret = _sdw_deprepare_stream(stream); if (ret < 0) pr_err("De-prepare for stream:%d failed: %d", ret, ret);

• mutex_unlock(&stream->m_rt->bus->bus_lock);

• sdw_release_bus_lock(stream); return ret; } EXPORT_SYMBOL(sdw_deprepare_stream);

diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h index ccd8dcdf..03df709 100644 --- a/include/linux/soundwire/sdw.h +++ b/include/linux/soundwire/sdw.h @@ -768,7 +768,6 @@ struct sdw_stream_params {

• @params: Stream parameters
• @state: Current state of the stream
• @type: Stream type PCM or PDM

• • @m_rt: Master runtime
  • @master_list: List of Master runtime(s) in this stream.
  • master_list can contain only one m_rt per Master instance
  • for a stream

@@ -778,7 +777,6 @@ struct sdw_stream_runtime { struct sdw_stream_params params; enum sdw_stream_state state; enum sdw_stream_type type;

• struct sdw_master_runtime *m_rt; struct list_head master_list; };