On 2/28/22 09:30, Cezary Rojewski wrote:
On 2022-02-25 9:21 PM, 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.
the comment was about 'without main core'.
This is difficult to follow, because you've discarded the main code in the if (!mask), so that's an always-true case, which makes the rest of the explanations not so clear.
I'm afraid, not seeing the problem. It's clearly not always-true statement as 'mask' can be non-zero after discarding the MAIN_CORE.
mask = core_mask & ~AVS_MAIN_CORE_MASK;
-> main core is ignored.
comment says "without main core, fw is dead anyway"
since you ignored the main core, is fw dead in all cases?