cavs: memory bank powerup flow adjustment #7065
Conversation
mbukowsk
requested review from
dbaluta,
lbetlej,
lgirdwood,
mmaka1 and
plbossart
as code owners
mbukowsk
force-pushed
the
mbukowsk/memory-powerup
branch
from
02f6d3b to
90d5316
Compare
lgirdwood
left a comment
lgirdwood
left a comment
There was a problem hiding this comment.
@mwasko @cujomalainey LGTM.
|
@cujomalainey Could you please verify this fix on your setup? |
abonislawski
left a comment
abonislawski
left a comment
There was a problem hiding this comment.
@wszypelt can you check CI issue?
|
@mbukowsk still draft ? |
|
@lgirdwood this PR looks like a prime candidate for a stable-2.2 update? |
Still fix is not verified and CI not working |
mbukowsk
force-pushed
the
mbukowsk/memory-powerup
branch
from
90d5316 to
43a4f19
Compare
|
I've done cosmetic change. for loop is added instead of code duplication. Primarily code was duplicated due to ifdef for cavs 2_5. Fix is applicable for all cavs versions, define is removed so same code is executed in simple loop. |
|
Got it cycling, I'll check it tonight, probably will hit about 2k cycles by then |
mbukowsk
force-pushed
the
mbukowsk/memory-powerup
branch
from
43a4f19 to
4c77545
Compare
|
Just shy of 400 cycles right now. Probably will hit 1k tonight. If its still going by then I will give my +1 so we can submit, but will still let it run through the night |
cujomalainey
left a comment
cujomalainey
left a comment
There was a problem hiding this comment.
935 cycles passed so far
|
SOFCI TEST |
|
#6864 hit once in https://sof-ci.01.org/sofpr/PR7065/build3847/devicetest/index.html The qemu-failures are more complicated, the changed code affects CNL and ICL and the qemu check in CI now fails, so this seems to be related to the code. OTOH, we have #7000 review which will remove CNL and ICL from mainline, making the failures moot. @lgirdwood @mwasko guidance how to proceed? I can't immediately see how the change of this PR can break the qemu test, but apparently it does. |
There was a problem hiding this comment.
not sure I understand. Does this mean, that once we successfully identified the register value to match what we expect, it can change and we have to run the loop again to catch a second such change? Or do we just have to
idelay();
while (io_reg_read() != expected)
idelay();
idelay();
io_reg_read();
? If this is the case, I wouldn't use a loop there but just add that additional register read (possibly with a delay) after the loop.
There was a problem hiding this comment.
regarding your recommendation, is that not what the outer loop enforces?
Memory banks enablement flow is adjusted to recommended approach. Power status should be read twice to ensure ebb readiness Signed-off-by: Michal Bukowski <michal.bukowski@intel.com>
mbukowsk
force-pushed
the
mbukowsk/memory-powerup
branch
from
4c77545 to
cbf5199
Compare
|
stopped my test at 4600 reboots, i think we are good |
|
This needs to be cherry picked to Rpl-001 asap |
|
created #7086 for picking to RPL-001 |
Memory banks enablement flow is adjusted to reccommended approach. Power status should be read twice to ensure ebb readiness
Signed-off-by: Michal Bukowski michal.bukowski@intel.com