dp: delayed start should apply to the first DP cycle only #8842
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
marcinszkudlinski
requested review from
pblaszko,
dbaluta,
LaurentiuM1234,
ranj063 and
jxstelter
as code owners
jxstelter
approved these changes
Feb 8, 2024
lyakh
requested changes
Feb 9, 2024
| @@ -253,7 +252,6 @@ void scheduler_dp_ll_tick(void *receiver_data, enum notify_id event_type, void * | |||
| /* set a deadline for given num of ticks, starting now */ | |||
| k_thread_deadline_set(pdata->thread_id, | |||
| pdata->deadline_clock_ticks); | |||
| pdata->ll_cycles_to_deadline = pdata->deadline_ll_cycles; | |||
There was a problem hiding this comment.
I can see, that this re-arming is useless, but it seems it should be harmless, since the dp_startup_delay flag is only set to true once during task initialisation? What am I missing?
There was a problem hiding this comment.
flag may never be set to false if re-arming occurs too early. That hurts.
In case of theoretically impossible event of LL-DP copy failure, an error message should be issued Message is better than assert - as stated this should never happen, but if it happens its better to have a glitch + message than a crash Signed-off-by: Marcin Szkudlinski <[email protected]>
When DP start, copy from DP to LL should be delayed till first DP deadline time. In current implementation the cycle counter is set at every DP scheduler trigger. If DP is - for any reason - scheduled again before its deadline passes, the counter will be set again and copying delay time will be too long. In extreme situation (i.e. if OBS is set to long value when IBS is short - in this case DP will proceed a short data chunks with a long deadline), it may lead to permanent stuck in processing. Signed-off-by: Marcin Szkudlinski <[email protected]>
marcinszkudlinski
force-pushed
the
dp-delayed-start-fix
branch
from
1f9042b to
bdcbad5
Compare
This file is implementing SOF-like abstraction called "scheduler": so the filename is rather accurate in my opinion |
lyakh
approved these changes
Feb 12, 2024
dnikodem
approved these changes
Feb 12, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When DP start, copy from DP to LL should be delayed till first
DP deadline time.
In current implementation the cycle counter is set at every
DP scheduler trigger. If DP is - for any reason - scheduled
again before its deadline passes, the counter will be set again
and copying delay time will be too long. In extreme situation
(i.e. if OBS is set to long value when IBS is short - in this case
DP will proceed a short data chunks with a long deadline), it may
lead to permanent stuck in processing.