On 2022-02-25 3:02 AM, Pierre-Louis Bossart wrote:
+static int avs_dsp_enable(struct avs_dev *adev, u32 core_mask)
+{
+ u32 mask;
+ int ret;
+
+ ret = avs_dsp_core_enable(adev, core_mask);
+ if (ret < 0)
+ return ret;
+
+ mask = core_mask & ~AVS_MAIN_CORE_MASK;
so here BIT(MAIN_CORE) is zero in mask
What's wrong with AVS_MAIN_CORE_MASK being used explicitly?
+ if (!mask)
+ /*
+ * without main core, fw is dead anyway
+ * so setting D0 for it is futile.
I don't get the comment, you explicitly discarded the main core with
your logical AND above, so this test means that all other non-main cores
are disabled.
There is no setting D0 for MAIN_CORE as firmware is not operational
without it. Firmware needs to be notified about D3 -> D0 transitions
only in case of non-MAIN_COREs.
+ */
+ return 0;
+
+ ret = avs_ipc_set_dx(adev, mask, true);
+ return AVS_IPC_RET(ret);
+}
+
+static int avs_dsp_disable(struct avs_dev *adev, u32 core_mask)
+{
+ int ret;
+
+ ret = avs_ipc_set_dx(adev, core_mask, false);
+ if (ret)
+ return AVS_IPC_RET(ret);
+
+ return avs_dsp_core_disable(adev, core_mask);
+}
+
+static int avs_dsp_get_core(struct avs_dev *adev, u32 core_id)
+{
+ u32 mask;
+ int ret;
+
+ mask = BIT_MASK(core_id);
+ if (mask == AVS_MAIN_CORE_MASK)
+ /* nothing to do for main core */
+ return 0;
+ if (core_id >= adev->hw_cfg.dsp_cores) {
+ ret = -EINVAL;
+ goto err;
+ }
+
+ adev->core_refs[core_id]++;
+ if (adev->core_refs[core_id] == 1) {
+ ret = avs_dsp_enable(adev, mask);
+ if (ret)
+ goto err_enable_dsp;
+ }
+
+ return 0;
+
+err_enable_dsp:
+ adev->core_refs[core_id]--;
+err:
+ dev_err(adev->dev, "get core failed: %d\n", ret);
you should log which core could not be enabled
Good catch! Ack.
+ return ret;
+}
+
+static int avs_dsp_put_core(struct avs_dev *adev, u32 core_id)
+{
+ u32 mask;
+ int ret;
+
+ mask = BIT_MASK(core_id);
+ if (mask == AVS_MAIN_CORE_MASK)
+ /* nothing to do for main core */
+ return 0;
+ if (core_id >= adev->hw_cfg.dsp_cores) {
+ ret = -EINVAL;
+ goto err;
+ }
+
+ adev->core_refs[core_id]--;
+ if (!adev->core_refs[core_id]) {
+ ret = avs_dsp_disable(adev, mask);
+ if (ret)
+ goto err;
+ }
+
+ return 0;
+err:
+ dev_err(adev->dev, "put core failed: %d\n", ret);
put core %d
Ack.
+ return ret;
+}
MODULE_LICENSE("GPL v2");
"GPL"
Ack.
