mbox series

[0/3,RESEND] ASoC: SOF: preparatory patches

Message ID 20200309170749.32313-1-guennadi.liakhovetski@linux.intel.com (mailing list archive)
Headers show
Series ASoC: SOF: preparatory patches | expand

Message

Guennadi Liakhovetski March 9, 2020, 5:07 p.m. UTC
This is the first set of patches for the SOF virtualisation work. We
send these patches first because they touch the ASoC core. 2 of them
are mostly cosmetic with no functional changes, but patch 2/3 might
cause some discussions. Please review and comment.

Thanks
Guennadi

Guennadi Liakhovetski (3):
  ASoC: (cosmetic) simplify dpcm_prune_paths()
  ASoC: add function parameters to enable forced path pruning
  ASoC: export DPCM runtime update functions

 include/sound/soc-dpcm.h |  30 ++++++++---
 sound/soc/soc-compress.c |   2 +-
 sound/soc/soc-dapm.c     |   8 +--
 sound/soc/soc-pcm.c      | 130 +++++++++++++++++++++++++++++------------------
 4 files changed, 109 insertions(+), 61 deletions(-)

Comments

Pierre-Louis Bossart March 9, 2020, 10:05 p.m. UTC | #1
On 3/9/20 12:07 PM, Guennadi Liakhovetski wrote:
> This is the first set of patches for the SOF virtualisation work. We
> send these patches first because they touch the ASoC core. 2 of them
> are mostly cosmetic with no functional changes, but patch 2/3 might
> cause some discussions. Please review and comment.

Sorry about the delay in reviewing.

To get a better picture of the directions, reviewers are invited to take 
a look at the in-depth documentation written by Guennadi since the 
initial patches were shared. This documentation was reviewed by Liam and 
me and is really required to understand the concepts:

https://thesofproject.github.io/latest/developer_guides/virtualization/virtualization.html

I added a couple of comments on the three patches, this looks good to me.

> Thanks
> Guennadi
> 
> Guennadi Liakhovetski (3):
>    ASoC: (cosmetic) simplify dpcm_prune_paths()
>    ASoC: add function parameters to enable forced path pruning
>    ASoC: export DPCM runtime update functions
> 
>   include/sound/soc-dpcm.h |  30 ++++++++---
>   sound/soc/soc-compress.c |   2 +-
>   sound/soc/soc-dapm.c     |   8 +--
>   sound/soc/soc-pcm.c      | 130 +++++++++++++++++++++++++++++------------------
>   4 files changed, 109 insertions(+), 61 deletions(-)
>
Mark Brown March 10, 2020, 12:50 p.m. UTC | #2
On Mon, Mar 09, 2020 at 05:05:06PM -0500, Pierre-Louis Bossart wrote:

> To get a better picture of the directions, reviewers are invited to take a
> look at the in-depth documentation written by Guennadi since the initial
> patches were shared. This documentation was reviewed by Liam and me and is
> really required to understand the concepts:

> https://thesofproject.github.io/latest/developer_guides/virtualization/virtualization.html

How does this relate to the virtio audio spec that's currently under
review?  It looks to be doing something much lower level than that.
I am concerned that this looks to be exposing DPCM as a virtio ABI,
we're trying to replace it as an internal API never mind ABI.
Pierre-Louis Bossart March 10, 2020, 8:13 p.m. UTC | #3
On 3/10/20 7:50 AM, Mark Brown wrote:
> On Mon, Mar 09, 2020 at 05:05:06PM -0500, Pierre-Louis Bossart wrote:
> 
>> To get a better picture of the directions, reviewers are invited to take a
>> look at the in-depth documentation written by Guennadi since the initial
>> patches were shared. This documentation was reviewed by Liam and me and is
>> really required to understand the concepts:
> 
>> https://thesofproject.github.io/latest/developer_guides/virtualization/virtualization.html
> 
> How does this relate to the virtio audio spec that's currently under
> review?  It looks to be doing something much lower level than that.

Mark, to avoid any ambiguity, what 'virtio audio spec that's currently 
under review' are you referring to?

> I am concerned that this looks to be exposing DPCM as a virtio ABI,
> we're trying to replace it as an internal API never mind ABI.

Valid point indeed.
Mark Brown March 11, 2020, 12:16 p.m. UTC | #4
On Tue, Mar 10, 2020 at 03:13:12PM -0500, Pierre-Louis Bossart wrote:
> On 3/10/20 7:50 AM, Mark Brown wrote:

> > How does this relate to the virtio audio spec that's currently under
> > review?  It looks to be doing something much lower level than that.

> Mark, to avoid any ambiguity, what 'virtio audio spec that's currently under
> review' are you referring to?

https://lists.oasis-open.org/archives/virtio-dev/202003/msg00045.html
Guennadi Liakhovetski March 12, 2020, 11:45 a.m. UTC | #5
Hi,

On Tue, Mar 10, 2020 at 12:50:56PM +0000, Mark Brown wrote:
> On Mon, Mar 09, 2020 at 05:05:06PM -0500, Pierre-Louis Bossart wrote:
> 
> > To get a better picture of the directions, reviewers are invited to take a
> > look at the in-depth documentation written by Guennadi since the initial
> > patches were shared. This documentation was reviewed by Liam and me and is
> > really required to understand the concepts:
> 
> > https://thesofproject.github.io/latest/developer_guides/virtualization/virtualization.html
> 
> How does this relate to the virtio audio spec that's currently under
> review?

The spec under discussion is only for simple audio virtualisation with fixed 
roles and topologies. With our approach guests get access to advanced DSP 
capabilities. The virtualisation approach under discussion can be a fallback 
for cases when no DSP has been detected on the host.

> It looks to be doing something much lower level than that.
> I am concerned that this looks to be exposing DPCM as a virtio ABI,
> we're trying to replace it as an internal API never mind ABI.

You mean that our approach works at the widget level, which is a part of the 
DPCM API? Well there is a translation layer between our ABI and DPCM. And by 
virtue of the same argument don't we already have DPCM as an ABI on the 
opposite side of SOF - in its IPC ABI? Largely this virtualisation approach 
doesn't add new interfaces, it re-uses the SOF IPC ABI, which is also one of 
its advantages.

Thanks
Guennadi
Mark Brown March 12, 2020, 12:15 p.m. UTC | #6
On Thu, Mar 12, 2020 at 12:45:37PM +0100, Guennadi Liakhovetski wrote:
> On Tue, Mar 10, 2020 at 12:50:56PM +0000, Mark Brown wrote:

> > How does this relate to the virtio audio spec that's currently under
> > review?

> The spec under discussion is only for simple audio virtualisation with fixed 
> roles and topologies. With our approach guests get access to advanced DSP 
> capabilities. The virtualisation approach under discussion can be a fallback 
> for cases when no DSP has been detected on the host.

So they're orthogonal :/  Have you proposed your spec yet?

> > It looks to be doing something much lower level than that.
> > I am concerned that this looks to be exposing DPCM as a virtio ABI,
> > we're trying to replace it as an internal API never mind ABI.

> You mean that our approach works at the widget level, which is a part of the 
> DPCM API? Well there is a translation layer between our ABI and DPCM. And by 
> virtue of the same argument don't we already have DPCM as an ABI on the 
> opposite side of SOF - in its IPC ABI? Largely this virtualisation approach 
> doesn't add new interfaces, it re-uses the SOF IPC ABI, which is also one of 
> its advantages.

Please bear in mind that the page you linked to is very high level and
I've not seen the actual spec or anything.  The page and your mails both
talk about DPCM so it sounds like that's a part of the interface.
Guennadi Liakhovetski March 12, 2020, 1:09 p.m. UTC | #7
On Thu, Mar 12, 2020 at 12:15:49PM +0000, Mark Brown wrote:
> On Thu, Mar 12, 2020 at 12:45:37PM +0100, Guennadi Liakhovetski wrote:
> > On Tue, Mar 10, 2020 at 12:50:56PM +0000, Mark Brown wrote:
> 
> > > How does this relate to the virtio audio spec that's currently under
> > > review?
> 
> > The spec under discussion is only for simple audio virtualisation with fixed 
> > roles and topologies. With our approach guests get access to advanced DSP 
> > capabilities. The virtualisation approach under discussion can be a fallback 
> > for cases when no DSP has been detected on the host.
> 
> So they're orthogonal :/  Have you proposed your spec yet?
> 
> > > It looks to be doing something much lower level than that.
> > > I am concerned that this looks to be exposing DPCM as a virtio ABI,
> > > we're trying to replace it as an internal API never mind ABI.
> 
> > You mean that our approach works at the widget level, which is a part of the 
> > DPCM API? Well there is a translation layer between our ABI and DPCM. And by 
> > virtue of the same argument don't we already have DPCM as an ABI on the 
> > opposite side of SOF - in its IPC ABI? Largely this virtualisation approach 
> > doesn't add new interfaces, it re-uses the SOF IPC ABI, which is also one of 
> > its advantages.
> 
> Please bear in mind that the page you linked to is very high level and
> I've not seen the actual spec or anything.  The page and your mails both
> talk about DPCM so it sounds like that's a part of the interface.

Let me rebase my current stack and post it here, stay tuned :-)

Thanks
Guennadi