Remove data duplication between mailbox and extended manifest - save memory #3497
Conversation
ktrzcinx
requested review from
RanderWang,
bardliao,
dbaluta,
jajanusz,
lbetlej,
lgirdwood,
mmaka1,
plbossart and
xiulipan
as code owners
|
You claim extended manifest is enabled by default, not force-enabled. So what's gonna happen on systems where it is disabled? I'd say this patch is welcome when extended manifest loses its kconfig and is always available. |
|
@paulstelian97 there's no kconfig option for extended manifest on/off, after #2957 merge each build should contain this part. I have edited initial PR description. |
kv2019i
left a comment
kv2019i
left a comment
There was a problem hiding this comment.
Looks good from kernel point of view. SOF1.7 will anyways require a recent kernel to load the FW, and these kernels can parse the info from ext_manifest, so we have no strict need to keep this around.
And as far as I can see, the IPC message ABI is not modified (which is good).
|
@kv2019i just to confirm, this is good to merge today with no kernel update ? |
This information is already provided by extended manifest, so there is no need to double it in runtime code. It allows to save 92 bytes from .DATA and 128 bytes from .TEXT for cnl platform. Signed-off-by: Karol Trzcinski <karolx.trzcinski@linux.intel.com>
This information is already provided by extended manifest, so there is no need to double it in runtime code. It allows to save 28 bytes from .DATA and 128 bytes from .TEXT for cnl platform. Signed-off-by: Karol Trzcinski <karolx.trzcinski@linux.intel.com>
ktrzcinx
force-pushed
the
slim-fw
branch
2 times, most recently
from
628f6f4 to
acf6454
Compare
This information is already provided by extended manifest, so there is no need to double it in runtime code. It allows to save 400 bytes from data .DATA and 64 bytes from .TEXT for cnl platform. EXT_MAP_PORT() macro is no longer needed. Created __unused macro in compiler_attributes.h as compiler attributes shouldn't be used directly, to allow easy compiler change. Signed-off-by: Karol Trzcinski <karolx.trzcinski@linux.intel.com>
This information is already provided by extended manifest, so there is no need to double it in runtime code. It allows to save 16 bytes from .DATA and 128 bytes from .TEXT for cnl platform. `data_structs` library from src/ipc/CMakeLists.txt is no longer needed. Moreover empty library may lead to cmake fail. SMEX needs little update, to read DBG_ABI from .fw_metadata section instead of .fw_ready. Signed-off-by: Karol Trzcinski <karolx.trzcinski@linux.intel.com>
|
@lgirdwood kernel is prepared for this for a long time (I add kernel support for this while extended manifest introduction) - jenkins CI results are good proof of it. Internal Intel CI must be updated, this work is in progress (near finish). |
|
@ktrzcinx ok, please ping when CI is finished. jenkins looks like a HW DUT is not booting. |
|
CI is ready, all tests are PASS, PR is good to merge. |
|
CI jenkins looks like DUT HW issues as it's not booting. |
|
@lgirdwood @ktrzcinx this broke Zephyr boot |
6 participants
After merge #2957, extended manifest is included in each firmware image. Data structs from
fw_ready_metadatahas already been ported to extended manifest so there's no need to duplicate the same data in mailbox in thefw_ready_metadatasection. Kernel code scan has support for both data location, so it shouldn't be an issue.After changes 448 bytes has been saved from .TEXT segment and 536 bytes from .DATA and data duplication has been deleted.
I didn't delete
fw_ready_metadatamechanism to keep possibility of passing eg. hardware related information (doesn't known at compilation time) to kernel, at start-up.Kernel can parse: