ipc: remove waiti running in IRQ level higher than passive level by RanderWang · Pull Request #335 · thesofproject/sof

RanderWang · 2018-09-10T06:59:20Z

waiti would cause FW panic if it is called in irq level
higher than passive level

Signed-off-by: Rander Wang rander.wang@linux.intel.com

lgirdwood · 2018-09-10T12:59:18Z

you need to idelay(TIMEOUT / TRIES); Make tries a macro.

PLATFORM_HOST_DMA_TRIES

ok, I will refine my patch

lgirdwood · 2018-09-10T12:59:38Z

compare this to TRIES macro

ok, I will refine my patch

lgirdwood · 2018-09-10T13:00:01Z

trace_error needed here.

ok, I will refine my patch

RanderWang · 2018-09-11T03:20:40Z

update my patch, thanks for review

lgirdwood · 2018-09-11T09:53:37Z

int clk should be named delay_ms or delay_us depending on time measurement.

yes, I will refine my patch

lgirdwood · 2018-09-11T09:54:48Z

clk must be divided by tries. otherwise we will wait for multiples of the timeout and not the timeout.

yes, I will refine my patch

RanderWang · 2018-09-12T02:14:36Z

update my patch, thanks for review

lgirdwood · 2018-09-12T12:34:07Z

why not pass the completion_t here instead of a callback, makes it simpler

because in some cases, there are no completion, just like waiting for a register idle

please refer to my second commit in this PR

waiting for a register is called polling, and this is probably better named poll_for_condition_timeout()

I just want to make a generic solution. Ok, I will refine my patch

RanderWang · 2018-09-13T03:00:44Z

update my patch, thanks for review

lgirdwood · 2018-09-13T07:10:34Z

Why did you remove the tries ? and while loop ? This will now block for timeout and be horribly inefficient. We need to try an return as soon as condition completes.

RanderWang · 2018-09-13T08:07:13Z

update my patch, thanks for review

lgirdwood · 2018-09-13T13:54:26Z

while (tries--)

yes, it is good for me

lgirdwood · 2018-09-13T13:56:52Z

how do we guarantee delta is not 0 ?

en, a very tricky corner case, I will refine it.

lgirdwood · 2018-09-13T13:57:12Z

poll_for_completion_delay

lgirdwood · 2018-09-13T13:58:27Z

RanderWang · 2018-09-14T07:00:39Z

update my patch, thanks for review

lgirdwood · 2018-09-14T11:51:36Z

us should be unsigned

lgirdwood · 2018-09-14T11:51:57Z

clock_us_to_ticks returns a uint64_t

lgirdwood · 2018-09-14T11:52:23Z

make tries and delta unsigned

lgirdwood · 2018-09-14T12:10:38Z

We should not delay if completion is initially true. This is better.

while (!wait_is_completed(comp)) { if (!tries--) { trace_error(); return error } delay(); } return 0;

waiti would cause FW panic if it is called in irq level higher than passive level Signed-off-by: Rander Wang <rander.wang@linux.intel.com>

tlauda · 2018-09-21T12:53:58Z


+static inline int poll_for_completion_delay(completion_t *comp, uint64_t us)
+{
+	uint64_t tick = clock_us_to_ticks(CLK_CPU, us);


@RanderWang @lgirdwood I think this function won't work correctly, because on cAVS platforms wait_delay function is based on XTAL (e.g. 19.2 MHZ for APL) and CLK_CPU has entirely different freq.

@RanderWang can you fix, use the PLATFORM_CLK thet we use fr work/scheduling.

lgirdwood requested changes Sep 10, 2018

View reviewed changes

RanderWang force-pushed the waiti_1.2 branch from 683c442 to bd30cba Compare September 11, 2018 03:18

lgirdwood requested changes Sep 11, 2018

View reviewed changes

RanderWang force-pushed the waiti_1.2 branch from bd30cba to 9f1aeb5 Compare September 12, 2018 02:13

lgirdwood reviewed Sep 12, 2018

View reviewed changes

RanderWang force-pushed the waiti_1.2 branch from 9f1aeb5 to 19f5b1d Compare September 13, 2018 02:00

lgirdwood requested changes Sep 13, 2018

View reviewed changes

RanderWang force-pushed the waiti_1.2 branch 2 times, most recently from 5c014b8 to 6f355a7 Compare September 13, 2018 08:04

lgirdwood requested changes Sep 13, 2018

View reviewed changes

RanderWang force-pushed the waiti_1.2 branch from 6f355a7 to 1b6e295 Compare September 14, 2018 06:58

lgirdwood requested changes Sep 14, 2018

View reviewed changes

ipc: remove waiti running in IRQ level higher than passive level

387befd

waiti would cause FW panic if it is called in irq level higher than passive level Signed-off-by: Rander Wang <rander.wang@linux.intel.com>

RanderWang force-pushed the waiti_1.2 branch from 1b6e295 to 387befd Compare September 17, 2018 02:49

xiulipan mentioned this pull request Sep 17, 2018

DSP panic after booting up #274

Closed

lgirdwood merged commit ef3ab58 into thesofproject:stable-1.2 Sep 17, 2018

xiulipan mentioned this pull request Sep 18, 2018

Merge stable 1.2 (part 3) scheduler and BYT part into master #386

Merged

tlauda reviewed Sep 21, 2018

View reviewed changes

Conversation

RanderWang commented Sep 10, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RanderWang commented Sep 11, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lgirdwood Sep 11, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RanderWang commented Sep 12, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RanderWang Sep 12, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RanderWang commented Sep 13, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RanderWang commented Sep 13, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RanderWang commented Sep 14, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

lgirdwood Sep 11, 2018 •

edited

Loading

RanderWang Sep 12, 2018 •

edited

Loading