Re: [PATCH 1/8] soundwire: bus_type: add master_device/driver support

[Pierre-Louis Bossart]

> > > > the ASoC layer does require a driver with a 'name' for the components
> > > > registered with the master device. So if you don't have a driver for the
> > > > master device, the DAIs will be associated with the PCI device.
> > > >
> > > > But the ASoC core does make pm_runtime calls on its own,
> > > >
> > > > soc_pcm_open(struct snd_pcm_substream *substream)
> > > > {
> > > > ...
> > > > 	for_each_rtd_components(rtd, i, component)
> > > > 		pm_runtime_get_sync(component->dev);
> > > >
> > > > and if the device that's associated with the DAI is the PCI device, then
> > > > that will not result in the relevant master IP being activated, only the PCI
> > > > device refcount will be increased - meaning there is no hook that would tell
> > > > the PCI layer to turn on a specific link.
> > > >
> > > > What you are recommending would be an all-or-nothing solution with all links
> > > > on or all links off, which beats the purpose of having independent
> > > > link-level power management.
> > >
> > > Why can't you use dai .startup callback for this?
> > >
> > > The ASoC core will do pm_runtime calls that will ensure PCI device is
> > > up, DSP firmware downloaded and running.
> > >
> > > You can use .startup() to turn on your link and .shutdown to turn off
> > > the link.
> >
> > There are multiple dais per link, and multiple Slave per link, so we would
> > have to refcount and track active dais to understand when the link needs to
> > be turned on/off. It's a duplication of what the pm framework can do at the
> > device/link level, and will likely introduce race conditions.
> >
> > Not to mention that we'd need to introduce workqueues to turn the link off
> > with a delay, with pm_runtime_put_autosuspend() does for free.
>
> Yes sure, that seems to be the cost unfortunately. While it might feel I
> am blocking but the real block here is the hw design which gives you a
> monolith whereas it should have been different devices. If you have a
> 'device' for sdw or a standalone controller we would not be debating
> this..

The hardware is what it is. The ACPI spec is what it is.

I am just pragmatic and making platforms work with that's available 
*today*, and I don't have time or interest in revisiting what might have 
been.

> > Linux is all about frameworks. For power management, we shall use the power
> > management framework, not reinvent it.
>
> This reminds me, please talk to Mika and Rafael, they had similar
> problems with lpss etc and IIRC they were working on splices to solve
> this.. Its been some time (few years now) so maybe they have a
> solution..

We've been discussing this since October, I don't really have any 
appetite for looking into new concepts when the existing framework just 
does what we need.

It's really down to your objection to the use of 'struct driver'... For 
ASoC support we only need the .name and .pm_ops, so there's really no 
possible path forward otherwise.

Like I said, we have 3 options

a) stay with platform devices for now. You will need to have a 
conversation with Greg on this.

b) use a minimal sdw_master_device with a minimal 'struct driver' use.

c) use a more elaborate solution suggested in this patchset and yes that 
means the Qualcomm driver would need to change a bit.

Pick one or suggest something that is implementable. The first version 
of the patches was provided in October, the last RFC was provided on 
January 31, time's up. At the moment you are preventing ASoC integration 
from moving forward.

Thanks
-Pierre