ipc4: set fw_ready flag for host #8342
Conversation
RanderWang
requested review from
lgirdwood,
plbossart,
mmaka1,
lbetlej,
dbaluta and
kv2019i
as code owners
RanderWang
force-pushed
the
lib_reload
branch
from
006594b to
ab82231
Compare
RanderWang
force-pushed
the
lib_reload
branch
from
ab82231 to
41844a4
Compare
RanderWang
force-pushed
the
lib_reload
branch
from
41844a4 to
6792a57
Compare
RanderWang
force-pushed
the
lib_reload
branch
2 times, most recently
from
2c46361 to
abfcab8
Compare
|
update flags only, thanks |
Set SOF_IPC_INFO_CONTEXT_SAVE for fw_ready event to host if IMR_CONTEXT_SAVE is supported so that host can sync up with FW, .e.g. avoid library reload if IMR_CONTEXT_SAVE is supported. Currently CAVS platforms don't support IMR_CONTEXT_SAVE but ACE platforms support it. It doesn't affect windows driver since the fw_ready data is not used by it. Signed-off-by: Rander Wang <[email protected]>
RanderWang
force-pushed
the
lib_reload
branch
from
abfcab8 to
db2b846
Compare
| #if CONFIG_CAVS_IMR_D3_PERSISTENT && CONFIG_ADSP_IMR_CONTEXT_SAVE | ||
| .flags = DEBUG_SET_FW_READY_FLAGS | SOF_IPC_INFO_D3_PERSISTENT | | ||
| SOF_IPC_INFO_CONTEXT_SAVE, | ||
| #elif CONFIG_CAVS_IMR_D3_PERSISTENT |
There was a problem hiding this comment.
We are not reading the IPC3 sof_ipc_fw_ready message from mailbox in case of IPC4.
Reference firmware do not passes any information via mailbox for FW_READY, iow IPC4 does not define payload for FW_READY.
There was a problem hiding this comment.
If we need out of manifest flags at boot (based on important Kconfig selections) then we need to add them.
There was a problem hiding this comment.
The IPC4 way of fw configuration query is via LAGE_CONFIG_GET:FW_CONFIG message. It has definition for various firmware parameters as tlv tuple bundle.
The reference IPC4 FW_READY notification does not have payload and I think if we want to keep the IPC4 name we should not introduce incompatible changes to the basic and common messages, notifications.
There was a problem hiding this comment.
IPC4 is not immutable, it needs to be backwards compatible but grow as features and needs arise.
There was a problem hiding this comment.
Yes, I agree. An IPC3 (SOF specific) message in the mailbox as payload for IPC4 FW_READY notification is not going to be backwards compatible with the reference, it is not going to be compatible at all since the reference is not placing anything in the mailbox for FW_READY, especially it is not going to place an IPC3 message in there which is part of the IPC3 protocol - in a similar way that IPC4 FW_READY does not have payload.
There was a problem hiding this comment.
@ujfalusi why we can't development IPC4 without breaking windows driver ? And if we are stick to current IPC4, we can't use LAGE_CONFIG_GET:FW_CONFIG since it doesn't support this query and we can't add more config, right ?
There was a problem hiding this comment.
@RanderWang I think direction now is to take this flag as a manifest flag. We should make it clear and add a comment to the FW ready message that the fields in FW ready should only be for runtime generated information (i.e. build time information would go in the manifest). Thanks !
| .flags = DEBUG_SET_FW_READY_FLAGS, | ||
| #endif |
There was a problem hiding this comment.
should line 59-61 will be removed, since this is ace/platform.c, it should support CONTEXT_SAVE?
| #if CONFIG_CAVS_IMR_D3_PERSISTENT && CONFIG_ADSP_IMR_CONTEXT_SAVE | ||
| .flags = DEBUG_SET_FW_READY_FLAGS | SOF_IPC_INFO_D3_PERSISTENT | | ||
| SOF_IPC_INFO_CONTEXT_SAVE, | ||
| #elif CONFIG_CAVS_IMR_D3_PERSISTENT | ||
| .flags = DEBUG_SET_FW_READY_FLAGS | SOF_IPC_INFO_D3_PERSISTENT, | ||
| #else |
There was a problem hiding this comment.
can we change code from 70 --> 76 like below to avoid repeated code?
.flags = DEBUG_SET_FW_READY_FLAGS,
#if CONFIG_CAVS_IMR_D3_PERSISTENT
.flags |= SOF_IPC_INFO_D3_PERSISTENT,
#endif
#if CONFIG_ADSP_IMR_CONTEXT_SAVE
.flags |= SOF_IPC_INFO_CONTEXT_SAVE,
#endif
|
no need |
Set SOF_IPC_INFO_FW_CONTEXT_SAVE for fw_ready event to
host if IMR_CONTEXT_SAVE is supported so that host can sync up with
FW, .e.g. avoid library reload if IMR_CONTEXT_SAVE is supported.
Currently CAVS platforms don't support IMR_CONTEXT_SAVE but ACE
platforms support it. It doesn't affect windows driver since the
fw_ready data is not used by it.
kernel PR: thesofproject/linux#4643