Skip to content

Conversation

@tlauda
Copy link
Contributor

@tlauda tlauda commented Mar 30, 2020

Improves host component performance by extracting DMA L1 exit
to be executed commonly for all active DMA channels as registered
callback in low latency scheduler for timer domain. There is no
need to wait so many cycles for every channel separately.

Signed-off-by: Tomasz Lauda tomasz.lauda@linux.intel.com

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer to have this notifier related piece in if (hda_chan->irq_disabled) rather than double return (just mho).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's kernel like approach to avoid large indentation. Most of the reviewers seem to like it this way.

@tlauda tlauda force-pushed the topic/host-l1-exit-move branch from 25313a6 to 586b0f9 Compare April 6, 2020 12:17
@tlauda tlauda requested a review from mmaka1 April 6, 2020 12:20
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we doing this in the scheduler ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can be done once for all DMA channels, so no point in doing it after every DMA copy separately. We're just wasting cycles. With timer driven pipelines we are sure that it will be executed only once for all channels.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, but I'm not following the usage here of L1 exit and when we need to use it. i.e. does it apply to all channels together or individually, do we exit L1 after DMA has stopped etc. Can you outline the rules a little more. Thanks

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It applies to all channels on all cores together and we should exit L1 after DMA pointer increment. The usage is exactly the same as before, but now it only happens once after all pipeline_copy tasks has been executed and not individually after every DMA channel copy.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PM L1 entry/exit should be handled directly by the HDA-DMA driver and not via LL scheduler state changes (as these can be done for any task unrelated to HDA DMA state). What's stopping us running an unrelated task via the LL scheduler during HDA work when we need to be in L1.
I'm also not following why we can't reference count pm_runtime_get(L1)/put(L1) within the HDA code ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lgirdwood Not true. If there is no LL work that uses HDA DMA, there will be no L1 entry/exit.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But wont any LL work invoke the notifier? This is regardless of HDA state, it's then a race between HDA pipelines and any LL work.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are invoked before processing list of LL tasks and after. Not before and after every single task.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, so my question is still the same, but rather than after/before every LL task, it's running the LL scheduler. Wont this put restrictions on when the scheduler can be used ? i.e. code that wants to schedule work on the LL scheduler will have to know about the state of any pipelines with HDA DMA ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are keeping ref count, so no special knowledge is needed. It should happen automatically.

@tlauda
Copy link
Contributor Author

tlauda commented Apr 8, 2020

@lgirdwood KWD tests are failing due to the KPB drain timing issue. I think @mrajwa should have some quick fix soon.

tlauda added 7 commits April 22, 2020 09:17
Adds possibility of defining flags, when performing
notifier registration.

Signed-off-by: Tomasz Lauda <tomasz.lauda@linux.intel.com>
Implements handling of new notifier aggregate flag.
With this flag set the notifier will automatically use
existing handle instead of creating a new one. It's helpful
when we want to register only once for an event, but it's
very hard to explicitly keep track of number of registrations.

Signed-off-by: Tomasz Lauda <tomasz.lauda@linux.intel.com>
Adds irq_local_disable/enable during notifier_register
and notifier_unregister calls to protect notify list
from preemption.

Signed-off-by: Tomasz Lauda <tomasz.lauda@linux.intel.com>
Saves irq_disabled config flag in private channel data.

Signed-off-by: Tomasz Lauda <tomasz.lauda@linux.intel.com>
Improves host component performance by extracting DMA L1 exit
to be executed commonly for all active DMA channels as registered
callback in low latency scheduler for timer domain. There is no
need to wait so many cycles for every channel separately.

Signed-off-by: Tomasz Lauda <tomasz.lauda@linux.intel.com>
Allocates cavs_pm_runtime_data as shared to make it usable
by multicore code.

Signed-off-by: Tomasz Lauda <tomasz.lauda@linux.intel.com>
Implements multicore synchronization mechanism for Host DMA L1 Exit.
Simple reference counter is added to avoid a situation, where shorter
processing on one core forces Host DMA bus to exit L1, when there is
still transfer happening on other core. PM_RUNTIME_HOST_DMA_L1 with get
is called in new NOTIFIER_ID_LL_PRE_RUN notification event.

Signed-off-by: Tomasz Lauda <tomasz.lauda@linux.intel.com>
@tlauda tlauda force-pushed the topic/host-l1-exit-move branch from 586b0f9 to de843c9 Compare April 22, 2020 07:26
@tlauda
Copy link
Contributor Author

tlauda commented Apr 22, 2020

SOFCI TEST

@tlauda
Copy link
Contributor Author

tlauda commented Apr 22, 2020

@lgirdwood Ready to be merged.

@jajanusz jajanusz merged commit 3cba349 into thesofproject:master Apr 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants