On Mon, 25 Jun 2018 12:58:56 +0200,
Shreyas NC wrote:
>
> From: Sanyog Kale <sanyog.r.kale@xxxxxxxxx>
>
> Currently, the stream concept is limited to single Master and one
> or more Codecs.
>
> This patch extends the concept to support multiple Master(s)
> sharing the same reference clock and synchronized in the hardware.
> Modify sdw_stream_runtime to support a list of sdw_master_runtime
> for the same. The existing reference to a single m_rt is removed
> in the next patch.
>
> Typically to lock, one would acquire a global lock and then lock
> bus instances. In this case, the caller framework(ASoC DPCM)
> guarantees that stream operations on a card are always serialized.
> So, there is no race condition and hence no need for global lock.
>
> Bus lock(s) are acquired to reconfigure the bus while the stream
> is set-up.
> So, we add sdw_acquire_bus_lock()/sdw_release_bus_lock() APIs which
> are used only to reconfigure the bus.
>
> Signed-off-by: Sanyog Kale <sanyog.r.kale@xxxxxxxxx>
> Signed-off-by: Vinod Koul <vkoul@xxxxxxxxxx>
> Signed-off-by: Shreyas NC <shreyas.nc@xxxxxxxxx>
> ---
> drivers/soundwire/bus.h | 2 ++
> drivers/soundwire/stream.c | 41 +++++++++++++++++++++++++++++++++++++++++
> include/linux/soundwire/sdw.h | 4 ++++
> 3 files changed, 47 insertions(+)
>
> diff --git a/drivers/soundwire/bus.h b/drivers/soundwire/bus.h
> index 3b15c4e..b6cfbdf 100644
> --- a/drivers/soundwire/bus.h
> +++ b/drivers/soundwire/bus.h
> @@ -99,6 +99,7 @@ struct sdw_slave_runtime {
> * this stream, can be zero.
> * @slave_rt_list: Slave runtime list
> * @port_list: List of Master Ports configured for this stream, can be zero.
> + * @stream_node: sdw_stream_runtime master_list node
> * @bus_node: sdw_bus m_rt_list node
> */
> struct sdw_master_runtime {
> @@ -108,6 +109,7 @@ struct sdw_master_runtime {
> unsigned int ch_count;
> struct list_head slave_rt_list;
> struct list_head port_list;
> + struct list_head stream_node;
> struct list_head bus_node;
> };
>
> diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
> index 8974a0f..eb942c6 100644
> --- a/drivers/soundwire/stream.c
> +++ b/drivers/soundwire/stream.c
> @@ -747,6 +747,7 @@ struct sdw_stream_runtime *sdw_alloc_stream(char *stream_name)
> return NULL;
>
> stream->name = stream_name;
> + INIT_LIST_HEAD(&stream->master_list);
> stream->state = SDW_STREAM_ALLOCATED;
>
> return stream;
> @@ -1234,6 +1235,46 @@ struct sdw_dpn_prop *sdw_get_slave_dpn_prop(struct sdw_slave *slave,
> return NULL;
> }
>
> +/**
> + * sdw_acquire_bus_lock: Acquire bus lock for all Master runtime(s)
> + *
> + * @stream: SoundWire stream
> + *
> + * Acquire bus_lock for each of the master runtime(m_rt) part of this
> + * stream to reconfigure the bus.
> + */
> +static void sdw_acquire_bus_lock(struct sdw_stream_runtime *stream)
> +{
> + struct sdw_master_runtime *m_rt = NULL;
> + struct sdw_bus *bus = NULL;
> +
> + /* Iterate for all Master(s) in Master list */
> + list_for_each_entry(m_rt, &stream->master_list, stream_node) {
> + bus = m_rt->bus;
> +
> + mutex_lock(&bus->bus_lock);
> + }
> +}
So it's nested locks? Then you'd need some more trick to deal with
the lockdep. I guess you'll get the false-positive deadlock detection
by this code when the mutex lock debug is enabled.
Also, is the linked order assured not to lead to a real deadlock?
> +/**
> + * sdw_release_bus_lock: Release bus lock for all Master runtime(s)
> + *
> + * @stream: SoundWire stream
> + *
> + * Release the previously held bus_lock after reconfiguring the bus.
> + */
> +static void sdw_release_bus_lock(struct sdw_stream_runtime *stream)
> +{
> + struct sdw_master_runtime *m_rt = NULL;
> + struct sdw_bus *bus = NULL;
> +
> + /* Iterate for all Master(s) in Master list */
> + list_for_each_entry(m_rt, &stream->master_list, stream_node) {
> + bus = m_rt->bus;
> + mutex_unlock(&bus->bus_lock);
> + }
... and this looks bad. The loop for unlocking should be traversed
reversely.
thanks,
Takashi
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
