[lyakh]

The new SOF_IPC_PM_VFE_POWER_STATUS IPC command is only used by
VirtIO guests to request DSP power status from the host. It is never
sent to the DSP, therefore there is no need to update the ABI
version.

[monocodus]

This is autogenerated code-style review, all good, approving

[lgirdwood]

Why does the VBE need to know VFE PM state ?

[lyakh]

Why does the VBE need to know VFE PM state ?

@lgirdwood to control DSP runtime PM. I think it was your idea, that resuming SOF on the guest should also resume the host and vice versa. Replies to this IPC also deliver a DSP reset count back to the vFE to be able to decide whether or not to send topology down to the vBE.

[lyakh]

@lgirdwood there's also a proposal thesofproject/linux#1992 (comment) to make it 0x100 instead to leave more space for potential other PM IPCs, what do you think?

[lgirdwood]

@lyakh sorry, the VFE should not need to manage VBE PM state. We should have minimal special IPCs (e.g for FW/topology loading) between VBE and VFE only. What usecase do you have that needs this atm ?

The way I see PM working is as follows

1. VBE is in D3 iff all VFEs are in D3
2. If 1 VFE is in D0 then VBE is in D0
3. VBE enters D3/D0 based on IPCs from VFE

3.1) D3 entry - VFE -> VBE IPC_PM_CTX SAVE can be used so that VBE decrements PM usage count (and enters D3 is needed).
3.2) D0 entry - VFE -> VBE IPC_PM_CTX_LOAD can be used so that VBE incrment usage count (load FW etc if needed)

IPC_PM_CTX_LOAD is not used today by FW or driver, its intended to load any context information but could be used by VFE to signal VBE.

[lyakh]

@lyakh sorry, the VFE should not need to manage VBE PM state. We should have minimal special IPCs (e.g for FW/topology loading) between VBE and VFE only. What usecase do you have that needs this atm ?

The way I see PM working is as follows

1. VBE is in D3 iff all VFEs are in D3

2. If 1 VFE is in D0 then VBE is in D0

3. VBE enters D3/D0 based on IPCs from VFE

3.1) D3 entry - VFE -> VBE IPC_PM_CTX SAVE can be used so that VBE decrements PM usage count (and enters D3 is needed).
3.2) D0 entry - VFE -> VBE IPC_PM_CTX_LOAD can be used so that VBE incrment usage count (load FW etc if needed)

IPC_PM_CTX_LOAD is not used today by FW or driver, its intended to load any context information but could be used by VFE to signal VBE.

@lgirdwood at the moment CTX_SAVE and CTX_LOAD aren't sent by vFEs because they don't probe as SOF PCI devices. They probe as VirtIO devices, handled by the SOF VirtIO driver. So they have their own set of PM ops. This doesn't mean of course that vFE cannot use the same IPCs as the "real" SOF, but there is at least one point there: we need a reset status back from the vBE as a reply to "resume." Would that be better to modify CTX_LOAD for that than to add a new IPC?

[lgirdwood]

@lgirdwood at the moment CTX_SAVE and CTX_LOAD aren't sent by vFEs because they don't probe as SOF PCI devices.

IIRC CTX_SAVE is sent by runtime PM code, so I would expect the same on VFE. The runtime PM logic should not be tied to whether we are a PCI device or not. i.e. I would still expect runtime PM code to run when VFE is idle.

They probe as VirtIO devices, handled by the SOF VirtIO driver. So they have their own set of PM ops. This doesn't mean of course that vFE cannot use the same IPCs as the "real" SOF, but there is at least one point there: we need a reset status back from the vBE as a reply to "resume." Would that be better to modify CTX_LOAD for that than to add a new IPC?

Yes I think so.

[lyakh]

@lgirdwood at the moment CTX_SAVE and CTX_LOAD aren't sent by vFEs because they don't probe as SOF PCI devices.

IIRC CTX_SAVE is sent by runtime PM code,

@lgirdwood Yes, runtime PM code of SOF PCI devices.

so I would expect the same on VFE. The runtime PM logic should not be tied to whether we are a PCI device or not. i.e. I would still expect runtime PM code to run when VFE is idle.

Are you sure you aren't mixing a couple of things here? Namely SOF RT PM on the host and SOF RT PM on the guest. They are 2 quite separate things.

SOF RT PM on the host certainly runs regardless whether or not any guests are running.

SOF RT PM on the guest is obviously only running when the respective guest is running. And there it isn't the usual HDA SOF PCI device, it is a VirtIO SOF device, managed by a different device driver. And each device driver has its own RT PM ops. So, as a separate device driver the SOF VirtIO driver on the guest has its own set of RT PM ops. We can reuse the same functions of parts of them, of course, especially in what concerns sending PM IPC messages, but still those are two different drivers and two different sets of PM ops. The VirtIO SOF driver (and its PM ops) is in thesofproject/linux@59b908e

They probe as VirtIO devices, handled by the SOF VirtIO driver. So they have their own set of PM ops. This doesn't mean of course that vFE cannot use the same IPCs as the "real" SOF, but there is at least one point there: we need a reset status back from the vBE as a reply to "resume." Would that be better to modify CTX_LOAD for that than to add a new IPC?

Yes I think so.

Hm, ok, in principle I have nothing against this. The change WRT the current patch stack would be minimal. Just replace one IPC with another. It seems a bit unclean to add a field to an IPC structure, that is sent to the DSP, that the DSP doesn't need and that instead only needed for VirtIO, but if that's your preference, I'm fine with that too. I'm closing this PR then and I'll remove the new IPC from thesofproject/linux@2a0bdbd

[lgirdwood]

SOF RT PM on the guest is obviously only running when the respective guest is running. And there it isn't the usual HDA SOF PCI device, it is a VirtIO SOF device, managed by a different device driver. And each device driver has its own RT PM ops

No matter the device model being used, the same PM ops flow should be used on guest and host. The intention is that all client drivers (like audio) should be 99% (or more) the same code regardless if they run on guest or host.

[lyakh]

SOF RT PM on the guest is obviously only running when the respective guest is running. And there it isn't the usual HDA SOF PCI device, it is a VirtIO SOF device, managed by a different device driver. And each device driver has its own RT PM ops

No matter the device model being used, the same PM ops flow should be used on guest and host. The intention is that all client drivers (like audio) should be 99% (or more) the same code regardless if they run on guest or host.

@lgirdwood I don't think re-using the same PM ops between host and guest would make sense. The host deals with a physical DSP, which is in our case a PCI device. The guest is a device on the virtio bus. They share some common code - sending the IPC and restoring the topology on resume, but the host has quite a bit more code too. Of course we can add a bunch of ifs to it, but I really think having separate methods is cleaner. But we can return to this once we are at that stage, then it will be easy to compare them. And if they're similar enough - sure, we can merge them.

[lgirdwood]

@lyakh Sorry, I mean here at the client level above the HW abstraction level. Client PM ops should be the same regardless of VFE or VBE client.