On Wed, Dec 08, 2021 at 10:27:43AM -0600, Pierre-Louis Bossart wrote:

> @@ -0,0 +1,24 @@
> +What:	
> /sys/devices/pci0000:00/<dev>/<tplg_name>/<path_template>:<path>/<pipeline>/state

> That was my biggest worry in internal reviews, I do not see any
> rationale for exposing an interface to userspace to modify pipeline
> states. I believe the intent is to have a follow-up series on this
> topic, but it's not clear what problem this is trying to solve. There's
> a fundamental disconnect here as to why the kernel driver could not
> control states on its own, and it begs the question if the 37 patches
> actually work without this odd userspace interface.

If it's mainly used for debugging then it could be exposed through
debugfs with less worry.

> b) the concept of 'path' is totally specific to this driver and will not
> be used by any other Intel solution. The notion of having more
> flexibility in dynamic reconfiguration of a pipeline, e.g. to avoid
> instantiating an unnecessary sample-rate conversion, is on paper a good
> one and is used in Windows solutions, but in practice all the existing
> end-to-end integrations in Linux/Chrome do require fairly static
> configurations with fixed sample rates. In other words, it's debatable
> whether any end-user will see any benefits in terms of
> experience/power/performance, and the added complexity is handled with a
> custom solution instead of improvements to DAPM/DPCM - which as we found
> out does need significant love to support multiple streams being
> mixed/demuxed. At the ALSA/ASoC level, I believe we have more important
> priorities such as the notion of 'DAPM domain', constraint propagation
> and hardening for complex use-cases, and improvements to the pipeline
> handling shall be done at the framework level, not the platform-specific
> driver level.

I've not meaningfully looked at the series yet (it's quite large!) but
commenting generally I do agree that if we're adding interfaces offering
detailed control of the digital domain we should be doing this at the
framework level - it's a common problem affecting a bunch of SoCs and
some CODECs too and it's only going to get harder to address in a
generic fashion if we add per driver interfaces.  On the other hand if
there's good interfaces that work for people in practice with driver
specific implementations perhaps they can be adapted to be more generic.

> I completely disagree with Cezary and his management's decision to float
> 37 patches upstream as RFC, with more coming. This goes against
> everything we've tried to do in the last 3 years to improve Intel's
> standing. I don't think it's right to ask for feedback from the
> maintainers and community when internally we were unable to make
> progress. What can I say other than 'this is really sad'.

> The work in the SOF driver will continue regardless of what happens with
> this patchset, which I am not going to comment further on.

This is obviously not ideal, I would like to have a consistent view from
at least Intel about the direction this is heading but I understand that
this might be difficult to achieve in such a large organization.  Input
from users like the distributions and PulseAudio/PipeWire is also very
important here, they'll face a lot of the complexity and hassle from end
users.  What conversations have been had thus far?  I guess ChromeOS is
going to prefer some combination of sticking with what it's got for
stability and transitioning to SoF for control of the firmware?

I do see that the code is using snd_intel_dsp_driver_probe() so we
should be able to manage any transition between implementations here,
though for that to be fully effective we'd need to be able to build both
from once.