Re: [RFC 00/37] ASoC: Intel: AVS - Audio DSP for cAVS

From: Mark Brown <broonie@kernel.org>
To: "Amadeusz Sławiński" <amadeuszx.slawinski@linux.intel.com>
Cc: Cezary Rojewski <cezary.rojewski@intel.com>,
	upstream@semihalf.com, harshapriya.n@intel.com,
	peter.ujfalusi@linux.intel.com, rad@semihalf.com,
	alsa-devel@alsa-project.org,
	pierre-louis.bossart@linux.intel.com, hdegoede@redhat.com,
	ranjani.sridharan@linux.intel.com, tiwai@suse.com,
	yung-chuan.liao@linux.intel.com, cujomalainey@chromium.org,
	lma@semihalf.com
Subject: Re: [RFC 00/37] ASoC: Intel: AVS - Audio DSP for cAVS
Date: Wed, 2 Feb 2022 16:08:14 +0000	[thread overview]
Message-ID: <YfqsbnroVj8ln66g@sirena.org.uk> (raw)
In-Reply-To: <7695fc34-143d-7715-85cb-7790386bbacc@linux.intel.com>

[-- Attachment #1: Type: text/plain, Size: 2345 bytes --]

On Wed, Feb 02, 2022 at 02:26:01PM +0100, Amadeusz Sławiński wrote:

> Although Cezary wrote that avs_path_reset/_pause/_run maps nicely to trigger
> operation it's not direct mapping. AVS FW has requirements on order of
> operations on pipelines (which are grouped in paths on kernel side). For
> example on TRIGGER_STOP we need to first pause all pipelines before issuing
> reset to any of them. This is required by FW, so that if there are two
> pipelines it doesn't pause and reset one of them, while the other one is
> still in running state, as this causes xruns on FW side.

...

> I would say that such behavior doesn't translate nicely to generic API.

This doesn't sound particularly strange, it's not a million miles away
from the requirements we have from hardware around keeping clocks alive.

> I tried looking once again at how one would split the path concept to make
> it more generic, but it is hard. On one hand paths are tied to AVS driver
> topology design, on the other hand we have (mentioned above) FW
> requirements.

Please understand that it is incredibly common for people to belive that
their system is somehow unique and needs to special case things that on
further examination turn out to be perfectly reasonable to handle in a
generic fashion.  Sometimes it's simply a case of just needing to do the
work, sometimes small enhancements are needed to the generic framework
and sometimes it's a case of refactoring the code so that some bits land
in generic code and some bits land in the driver.  Especially with
enormous amounts of code like you've got here there's a natural bias
towards wanting to make minimal changes.

> now for the concept of paths the most interesting field is
> "path_tmpl2_path0_ppl0_bindid0_tuples" as it describes to which path we want
> to bind. It is done this way as FW modules internally have pins, and while
> in most cases one wants to just bind on pin 0, sometimes there is a need to
> describe more complicated connections. And so we circled back to FW
> requirements.

The idea of an algorithm having multiple inputs or outputs seems logical
and generic - the examples that spring to mind are things like mixers,
beam forming or echo/noise cancellation.  These seem like they're going
to be present in a wide range of DSP firmwares.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]