[PATCH v2 1/5] soundwire: stream: update state machine and add state checks
From: Pierre-Louis Bossart
Date: Tue Jan 14 2020 - 18:52:45 EST
The state machine and notes don't accurately explain or allow
transitions from STREAM_DEPREPARED and STREAM_DISABLED.
Add more explanations and allow for more transitions as a result of a
trigger_stop(), trigger_suspend() and prepare(), depending on the
ALSA/ASoC layer behavior defined by the INFO_RESUME and INFO_PAUSE
flags.
Also add basic checks to help debug inconsistent states and illegal
state machine transitions.
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx>
---
Documentation/driver-api/soundwire/stream.rst | 61 +++++++++++++------
drivers/soundwire/stream.c | 37 +++++++++++
2 files changed, 81 insertions(+), 17 deletions(-)
diff --git a/Documentation/driver-api/soundwire/stream.rst b/Documentation/driver-api/soundwire/stream.rst
index 5351bd2f34a8..8bceece51554 100644
--- a/Documentation/driver-api/soundwire/stream.rst
+++ b/Documentation/driver-api/soundwire/stream.rst
@@ -156,22 +156,27 @@ Below shows the SoundWire stream states and state transition diagram. ::
+-----------+ +------------+ +----------+ +----------+
| ALLOCATED +---->| CONFIGURED +---->| PREPARED +---->| ENABLED |
| STATE | | STATE | | STATE | | STATE |
- +-----------+ +------------+ +----------+ +----+-----+
- ^
- |
- |
- v
- +----------+ +------------+ +----+-----+
+ +-----------+ +------------+ +---+--+---+ +----+-----+
+ ^ ^ ^
+ | | |
+ __| |___________ |
+ | | |
+ v | v
+ +----------+ +-----+------+ +-+--+-----+
| RELEASED |<----------+ DEPREPARED |<-------+ DISABLED |
| STATE | | STATE | | STATE |
+----------+ +------------+ +----------+
-NOTE: State transition between prepare and deprepare is supported in Spec
-but not in the software (subsystem)
+NOTE: State transitions between ``SDW_STREAM_ENABLED`` and
+``SDW_STREAM_DISABLED`` are only relevant when then INFO_PAUSE flag is
+supported at the ALSA/ASoC level. Likewise the transition between
+``SDW_DISABLED_STATE`` and ``SDW_PREPARED_STATE`` depends on the
+INFO_RESUME flag.
-NOTE2: Stream state transition checks need to be handled by caller
-framework, for example ALSA/ASoC. No checks for stream transition exist in
-SoundWire subsystem.
+NOTE2: The framework implements basic state transition checks, but
+does not e.g. check if a transition from DISABLED to ENABLED is valid
+on a specific platform. Such tests need to be added at the ALSA/ASoC
+level.
Stream State Operations
-----------------------
@@ -246,6 +251,9 @@ SDW_STREAM_PREPARED
Prepare state of stream. Operations performed before entering in this state:
+ (0) Steps 1 and 2 are omitted in the case of a resume operation,
+ where the bus bandwidth is known.
+
(1) Bus parameters such as bandwidth, frame shape, clock frequency,
are computed based on current stream as well as already active
stream(s) on Bus. Re-computation is required to accommodate current
@@ -270,9 +278,11 @@ Prepare state of stream. Operations performed before entering in this state:
After all above operations are successful, stream state is set to
``SDW_STREAM_PREPARED``.
-Bus implements below API for PREPARE state which needs to be called once per
-stream. From ASoC DPCM framework, this stream state is linked to
-.prepare() operation.
+Bus implements below API for PREPARE state which needs to be called
+once per stream. From ASoC DPCM framework, this stream state is linked
+to .prepare() operation. Since the .trigger() operations may not
+follow the .prepare(), a direct transition from
+``SDW_STREAM_PREPARED`` to ``SDW_STREAM_DEPREPARED`` is allowed.
.. code-block:: c
@@ -332,6 +342,14 @@ Bus implements below API for DISABLED state which needs to be called once
per stream. From ASoC DPCM framework, this stream state is linked to
.trigger() stop operation.
+When the INFO_PAUSE flag is supported, a direct transition to
+``SDW_STREAM_ENABLED`` is allowed.
+
+For resume operations where ASoC will use the .prepare() callback, the
+stream can transition from ``SDW_STREAM_DISABLED`` to
+``SDW_STREAM_PREPARED``, with all required settings restored but
+without updating the bandwidth and bit allocation.
+
.. code-block:: c
int sdw_disable_stream(struct sdw_stream_runtime * stream);
@@ -353,9 +371,18 @@ state:
After all above operations are successful, stream state is set to
``SDW_STREAM_DEPREPARED``.
-Bus implements below API for DEPREPARED state which needs to be called once
-per stream. From ASoC DPCM framework, this stream state is linked to
-.trigger() stop operation.
+Bus implements below API for DEPREPARED state which needs to be called
+once per stream. ALSA/ASoC do not have a concept of 'deprepare', and
+the mapping from this stream state to ALSA/ASoC operation may be
+implementation specific.
+
+When the INFO_PAUSE flag is supported, the stream state is linked to
+the .hw_free() operation - the stream is not deprepared on a
+TRIGGER_STOP.
+
+Other implementations may transition to the ``SDW_STREAM_DEPREPARED``
+state on TRIGGER_STOP, should they require a transition through the
+``SDW_STREAM_PREPARED`` state.
.. code-block:: c
diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index 178ae92b8cc1..6aa0b5d370c0 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -1553,8 +1553,18 @@ int sdw_prepare_stream(struct sdw_stream_runtime *stream)
sdw_acquire_bus_lock(stream);
+ if (stream->state != SDW_STREAM_CONFIGURED &&
+ stream->state != SDW_STREAM_DEPREPARED &&
+ stream->state != SDW_STREAM_DISABLED) {
+ pr_err("%s: %s: inconsistent state state %d\n",
+ __func__, stream->name, stream->state);
+ ret = -EINVAL;
+ goto state_err;
+ }
+
ret = _sdw_prepare_stream(stream);
+state_err:
sdw_release_bus_lock(stream);
return ret;
}
@@ -1619,8 +1629,17 @@ int sdw_enable_stream(struct sdw_stream_runtime *stream)
sdw_acquire_bus_lock(stream);
+ if (stream->state != SDW_STREAM_PREPARED &&
+ stream->state != SDW_STREAM_DISABLED) {
+ pr_err("%s: %s: inconsistent state state %d\n",
+ __func__, stream->name, stream->state);
+ ret = -EINVAL;
+ goto state_err;
+ }
+
ret = _sdw_enable_stream(stream);
+state_err:
sdw_release_bus_lock(stream);
return ret;
}
@@ -1693,8 +1712,16 @@ int sdw_disable_stream(struct sdw_stream_runtime *stream)
sdw_acquire_bus_lock(stream);
+ if (stream->state != SDW_STREAM_ENABLED) {
+ pr_err("%s: %s: inconsistent state state %d\n",
+ __func__, stream->name, stream->state);
+ ret = -EINVAL;
+ goto state_err;
+ }
+
ret = _sdw_disable_stream(stream);
+state_err:
sdw_release_bus_lock(stream);
return ret;
}
@@ -1749,8 +1776,18 @@ int sdw_deprepare_stream(struct sdw_stream_runtime *stream)
}
sdw_acquire_bus_lock(stream);
+
+ if (stream->state != SDW_STREAM_PREPARED &&
+ stream->state != SDW_STREAM_DISABLED) {
+ pr_err("%s: %s: inconsistent state state %d\n",
+ __func__, stream->name, stream->state);
+ ret = -EINVAL;
+ goto state_err;
+ }
+
ret = _sdw_deprepare_stream(stream);
+state_err:
sdw_release_bus_lock(stream);
return ret;
}
--
2.20.1