On Thu, 2015-09-17 at 17:08 +0530, Vinod Koul wrote:
On Thu, Sep 17, 2015 at 10:47:58AM +0100, Liam Girdwood wrote:
Hi Liam,
On Mon, 2015-08-17 at 22:56 +0530, Vinod Koul wrote:
From: Jeeja KP jeeja.kp@intel.com
The Skylake driver topology model tries to model the firmware rule for pipeline and module creation. The creation rule is:
- Create Pipe
- Add modules to Pipe
- Connect the modules (bind)
- Start the pipes
Similarly destroy rule is:
- Stop the pipe
- Disconnect it (unbind)
- Delete the pipe
In driver we use Mixer, as there will always be ONE mixer in a pipeline to model a pipe. The modules in pipe are modelled as PGA widgets. The DAPM sequencing rules (mixer and then PGA) are used to create the sequence DSP expects as depicted above, and then widget handlers for PMU and PMD events help in that.
This patch adds widget event handlers for PRE/POST PMU and PRE/POST PMD event for mixer and pga modules. These event handlers invoke pipeline creation, destroy, module creation, module bind, unbind and pipeline bind unbind
Event handler sequencing is implement to target the DSP FW sequence expectations to enable path from source to sink pipe for Playback/Capture.
Signed-off-by: Jeeja KP jeeja.kp@intel.com Signed-off-by: Hardik T Shah hardik.t.shah@intel.com Signed-off-by: Subhransu S. Prusty subhransu.s.prusty@intel.com Signed-off-by: Vinod Koul vinod.koul@intel.com
sound/soc/intel/skylake/skl-topology.c | 416 +++++++++++++++++++++++++++++++++ 1 file changed, 416 insertions(+)
snip...
+/*
- in the Pre-PMD event of mixer we need to do following:
- Stop the pipe
- find the source connections and remove that from dapm_path_list
- unbind with source pipelines if still connected
- */
+static int skl_tplg_mixer_dapm_pre_pmd_event(struct snd_soc_dapm_widget *w,
struct skl *skl)+{
- struct snd_soc_dapm_widget *source, *sink;
- struct skl_module_cfg *src_mconfig, *sink_mconfig;
- int ret = 0, path_found = 0;
- struct skl_dapm_path_list *path_list, *tmp_list;
- struct skl_sst *ctx = skl->skl_sst;
- dev_dbg(ctx->dev, "%s: widget = %s\n", __func__, w->name);
- sink = w;
- sink_mconfig = sink->priv;
- /* Stop the pipe */
- ret = skl_stop_pipe(ctx, sink_mconfig->pipe);
- if (ret)
return ret;- list_for_each_entry_safe(path_list, tmp_list, &skl->dapm_path_list, node) {
if (path_list->dapm_path->sink == sink) {dev_dbg(ctx->dev, "Path found = %s\n", path_list->dapm_path->name);source = path_list->dapm_path->source;src_mconfig = source->priv;path_found = 1;list_del(&path_list->node);kfree(path_list);break;}- }
There is a lot of list walking and manipulation in this series and it's not clear where any locks are being held to prevent list corruption. I'm assuming list items are being added and removed as part of loading/unloading the topology data but it looks like we are also manipulating component lists during DAPM events ?
We have a driver list dapm_path_list where we store the widgets powered up. This gives us a very quick reference of the paths which are powered up in the graph and helps fast traversal to check if we should power up a path as path is connected to something else which is powered up already (mixng two paths) and similarly while disconnecting.
Please note all these are handled only in event handlers for widgets, so they will be invoked by dapm with mutex, dapm_mutex held, so we didn't think we would need another lock here
Ok, I was thinking that may be the case. It may be worth while stating this in comments where this applies.
Liam