On 13-03-20, 11:54, Pierre-Louis Bossart wrote:
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.
yes they do but add an intrusive platform specific change into soundwire core, something I would not like to add.
You should really be willing to talk to your colleagues to see if there is something you can reuse.
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.
It means that we cannot have a solution which is Intel specific into core. If you has a standalone controller you do not need this.
Like I said, we have 3 options
Repeating the already discussed doesn't help. I have already told you the constraint to work is not to add Intel specific change into core.
I have already said that expect the driver part I dont have objections to rest of this series and am ready to merge
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.
In opensource review we go back and forth and we debate and come to a common conclusion. Choosing a specific set of solutions and constraining yourself to pick one does not help.
I have only _one_ constraint no platform specific change in core. If that is satisfied I will go with it. Sorry but this is non-negotiable for me.
Ask yourself, do you need this intrusive core change if you had this exact same controller(s) but only as standalone one...