On 8/24/19 4:17 AM, Cezary Rojewski wrote:
On 2019-08-23 20:24, Pierre-Louis Bossart wrote:
On 8/22/19 2:03 PM, Cezary Rojewski wrote:
Implement interface for retrieving firmware configuration. Skylake driver will use this data instead of hardcoded values in updates to come.
Most params are currently unused. In time driver dependency on fw config will increase, and with it, more parsing will be unveiled.
Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com
sound/soc/intel/skylake/skl-sst-ipc.c | 122 ++++++++++++++++++++++++++ sound/soc/intel/skylake/skl-sst-ipc.h | 72 +++++++++++++++ sound/soc/intel/skylake/skl.h | 1 + 3 files changed, 195 insertions(+)
diff --git a/sound/soc/intel/skylake/skl-sst-ipc.c b/sound/soc/intel/skylake/skl-sst-ipc.c index 667cdddc289f..e9e11ec4c97b 100644 --- a/sound/soc/intel/skylake/skl-sst-ipc.c +++ b/sound/soc/intel/skylake/skl-sst-ipc.c @@ -11,6 +11,7 @@ #include "skl.h" #include "skl-sst-dsp.h" #include "skl-sst-ipc.h" +#include "skl-topology.h" #include "sound/hdaudio_ext.h" @@ -1067,3 +1068,124 @@ int skl_ipc_set_d0ix(struct sst_generic_ipc *ipc, struct skl_ipc_d0ix_msg *msg) return ret; } EXPORT_SYMBOL_GPL(skl_ipc_set_d0ix);
+int skl_ipc_fw_cfg_get(struct sst_generic_ipc *ipc, struct skl_fw_cfg *cfg) +{ + struct skl_ipc_large_config_msg msg = {0}; + struct skl_tlv *tlv; + size_t bytes = 0, offset = 0; + u8 *payload = NULL; + int ret;
+ msg.module_id = 0; + msg.instance_id = 0; + msg.large_param_id = SKL_BASEFW_FIRMWARE_CONFIG;
+ ret = skl_ipc_get_large_config(ipc, &msg, (u32 **)&payload, &bytes); + if (ret) + goto exit;
+ while (offset < bytes) { + tlv = (struct skl_tlv *)(payload + offset);
+ switch (tlv->type) { + case SKL_FW_CFG_FW_VERSION: + memcpy(&cfg->fw_version, tlv->value, + sizeof(cfg->fw_version)); + break;
+ case SKL_FW_CFG_MEMORY_RECLAIMED: + cfg->memory_reclaimed = *tlv->value; + break;
+ case SKL_FW_CFG_SLOW_CLOCK_FREQ_HZ: + cfg->slow_clock_freq_hz = *tlv->value; + break;
+ case SKL_FW_CFG_FAST_CLOCK_FREQ_HZ: + cfg->fast_clock_freq_hz = *tlv->value; + break;
+ case SKL_FW_CFG_ALH_SUPPORT_LEVEL: + cfg->alh_support = *tlv->value; + break;
+ case SKL_FW_CFG_IPC_DL_MAILBOX_BYTES: + cfg->ipc_dl_mailbox_bytes = *tlv->value; + break;
+ case SKL_FW_CFG_IPC_UL_MAILBOX_BYTES: + cfg->ipc_ul_mailbox_bytes = *tlv->value; + break;
+ case SKL_FW_CFG_TRACE_LOG_BYTES: + cfg->trace_log_bytes = *tlv->value; + break;
+ case SKL_FW_CFG_MAX_PPL_COUNT: + cfg->max_ppl_count = *tlv->value; + break;
+ case SKL_FW_CFG_MAX_ASTATE_COUNT: + cfg->max_astate_count = *tlv->value; + break;
+ case SKL_FW_CFG_MAX_MODULE_PIN_COUNT: + cfg->max_module_pin_count = *tlv->value; + break;
+ case SKL_FW_CFG_MODULES_COUNT: + cfg->modules_count = *tlv->value; + break;
+ case SKL_FW_CFG_MAX_MOD_INST_COUNT: + cfg->max_mod_inst_count = *tlv->value; + break;
+ case SKL_FW_CFG_MAX_LL_TASKS_PER_PRI_COUNT: + cfg->max_ll_tasks_per_pri_count = *tlv->value; + break;
+ case SKL_FW_CFG_LL_PRI_COUNT: + cfg->ll_pri_count = *tlv->value; + break;
+ case SKL_FW_CFG_MAX_DP_TASKS_COUNT: + cfg->max_dp_tasks_count = *tlv->value; + break;
+ case SKL_FW_CFG_MAX_LIBS_COUNT: + cfg->max_libs_count = *tlv->value; + break;
+ case SKL_FW_CFG_XTAL_FREQ_HZ: + cfg->xtal_freq_hz = *tlv->value; + break;
+ case SKL_FW_CFG_UAOL_SUPPORT: + cfg->uaol_support = *tlv->value; + break;
+ case SKL_FW_CFG_POWER_GATING_POLICY: + cfg->power_gating_policy = *tlv->value; + break;
+ case SKL_FW_CFG_DMA_BUFFER_CONFIG: + case SKL_FW_CFG_SCHEDULER_CONFIG: + case SKL_FW_CFG_CLOCKS_CONFIG: + break;
+ default: + dev_info(ipc->dev, "Unrecognized fw param: %d\n", + tlv->type); + break;
Isn't this an error? If there are other possible values, why not list them and skip them, as done above?
Pretty sure I cannot share names for all capabilities as these are EMBARGOed. Moreover, both FW_CFG and HW_CFG and constantly being updated and thus new constants are appended. I find it best to "break" when encountering known and un-EMBARGOED constant and simply dump info value if the opposite is true.
New capabilities are always tied to newer platforms. If there will be a need for adding them, these won't be even present in SKL/KBL and such FWs. At the same time if given binary drop contains something new but not required to be handled by host in generic case, dumping error is counter-intuitive.
It goes back to the compatibility issue, if you have a new firmware reporting a capability that your driver cannot handle then it's an error. if you have a new firmware that reports a new capability that can be ignored then it's fine to just skip.
I am just concerned that you have no checks at all.
+ }
+ offset += sizeof(*tlv) + tlv->length; + }
+exit: + kfree(payload); + return ret; +} +EXPORT_SYMBOL_GPL(skl_ipc_fw_cfg_get);
+enum skl_alh_support_level { + ALH_NO_SUPPORT = 0x00000, + ALH_CAVS_1_8_CNL = 0x10000, +};
Support for ALH hasn't changed even past 1.8, and references to CNL are probably not needed.
These are FW types and are here to be left untouched. Ensures parsed values on host side match FW side.
Adding a comment would help then.