Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

ace: gpdma: power down routine #54505

Merged
merged 7 commits into from
Apr 25, 2023
Merged

ace: gpdma: power down routine #54505

merged 7 commits into from
Apr 25, 2023

Conversation

tmleman
Copy link
Collaborator

@tmleman tmleman commented Feb 6, 2023 •
edited
Loading

We want to power up GPDMA only when we are planning to use it. Device power manager (PM) will wake up the power domain and then power up the DMA instance. When device is no longer in use, PM can disable device and allow the power domain to enter power gaiting state.

@tmleman tmleman force-pushed the topic/upstream/pr/ace/gpdma/power_down branch 3 times, most recently from 399ca4e to 5313da0 Compare February 7, 2023 13:44
@tmleman tmleman marked this pull request as ready for review February 7, 2023 14:36
@zephyrbot zephyrbot added the area: DMA Direct Memory Access label Feb 7, 2023
@tmleman tmleman added the platform: Intel ADSP Intel Audio platforms label Feb 7, 2023
teburd
teburd reviewed Feb 7, 2023
View reviewed changes
Copy link
Collaborator

@teburd teburd left a comment •
edited
Loading

Choose a reason for hiding this comment

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

The changes look good.

I'd really love to see a test verifying this power logic as part of the change set.

So something like...

  1. verify dma is powered off on start
  2. verify on start dma is clock enabled and powered on
  3. verify on stop dma is clock gated and powered off
  4. verify repeated configure/start/stops doesn't break anything.

Perhaps a slight modification of the memory to memory test could do this testing selectively given the particular DMA supports PM and PM is enabled.

abonislawski
abonislawski requested changes Mar 8, 2023
View reviewed changes
Copy link
Member

@abonislawski abonislawski left a comment

Choose a reason for hiding this comment

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

Blocking just to not accidentally merge without dma_ctx init fix, example:
thesofproject@9c22efe

@tmleman tmleman force-pushed the topic/upstream/pr/ace/gpdma/power_down branch from 5313da0 to c9f2b0f Compare March 13, 2023 12:49
@zephyrbot zephyrbot requested review from gmarull and nashif March 13, 2023 12:49
@tmleman tmleman force-pushed the topic/upstream/pr/ace/gpdma/power_down branch from c9f2b0f to c57bc4e Compare March 21, 2023 15:24
@tmleman
Copy link
Collaborator Author

tmleman commented Mar 21, 2023

Blocking just to not accidentally merge without dma_ctx init fix

@abonislawski fixed in newest version

@tmleman tmleman force-pushed the topic/upstream/pr/ace/gpdma/power_down branch 2 times, most recently from d9c20c0 to 2e19fbb Compare March 23, 2023 13:14
@tmleman
drivers: gpdma: release dma ownership
11eaa69 
Adding function that is allowing to release ownership of the DMA. When
DSP is no longer using dma instance it ownership can be released.

Signed-off-by: Tomasz Leman <tomasz.m.leman@intel.com>
@tmleman tmleman force-pushed the topic/upstream/pr/ace/gpdma/power_down branch from 1e0bca4 to bc69d7f Compare April 6, 2023 17:27
@teburd
Copy link
Collaborator

teburd commented Apr 6, 2023

@tmleman let me know when I should retry/retest, happy to do so

aborisovich
aborisovich previously approved these changes Apr 12, 2023
View reviewed changes
tmleman added 4 commits April 18, 2023 11:56
@tmleman
drivers: gpdma: enable clock gating
d97abc9 
This patch is adding function enabling DMA clock gating.

Signed-off-by: Tomasz Leman <tomasz.m.leman@intel.com>
@tmleman
tests: drivers: dma: loop_transfer: power state check
beaf64f 
This patch modifies the existing loop transfer test to allow testing for
device power state changes when starting and stopping a transfer.

Signed-off-by: Tomasz Leman <tomasz.m.leman@intel.com>
@tmleman
tests: dma: loop_transfer: add mtl board
9cfa9b0 
Adding meteorlake board overlay.

Signed-off-by: Tomasz Leman <tomasz.m.leman@intel.com>
@tmleman
drivers: gpdma: pm runtime works only on ace
7b08f42 
CAVS platforms are not fully integrated with zephyr. Some of the
registers are still programed from SOF side. This feature can be enabled
for those platforms later when integration is fully done.

Signed-off-by: Tomasz Leman <tomasz.m.leman@intel.com>
@tmleman tmleman force-pushed the topic/upstream/pr/ace/gpdma/power_down branch from bc69d7f to 7b08f42 Compare April 18, 2023 09:57
@tmleman tmleman requested a review from dcpleung as a code owner April 18, 2023 09:57
@zephyrbot zephyrbot added the area: Xtensa Xtensa Architecture label Apr 18, 2023
@tmleman
Copy link
Collaborator Author

tmleman commented Apr 18, 2023 •
edited
Loading

@teburd unfortunately you won't be able to test it on any CAVS platform. First problem is that builds with enabled PM doesn't work (those without SOF) #56415. And second one is that in case of a TGL some registers are still programed from SOF side.

I made comparison with closed source version and power management aspects of those drivers does not match to each other. Those features are needed for ACE and I don't want to invest more time into CAVS version.

I will make SOF build with those changes and run test on it.

@tmleman
Copy link
Collaborator Author

tmleman commented Apr 19, 2023

I have tested FW with those changes on MTL and TGL. I didn't find any issue.

thesofproject/sof#7353

@teburd teburd dismissed their stale review April 19, 2023 15:54

No longer relevant

@teburd
Copy link
Collaborator

teburd commented Apr 19, 2023

@ceolin please take a look

kv2019i
kv2019i approved these changes Apr 25, 2023
View reviewed changes
Copy link
Collaborator

@kv2019i kv2019i left a comment

Choose a reason for hiding this comment

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

@ceolin Can you check? I'll update #55738 once this is in.

teburd
teburd approved these changes Apr 25, 2023
View reviewed changes
Copy link
Collaborator

@teburd teburd left a comment

Choose a reason for hiding this comment

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

Verified working on cavs25 using the dma loopback test without PM enabled

@carlescufi carlescufi merged commit 9028ad5 into zephyrproject-rtos:main Apr 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@softwarecki softwarecki softwarecki approved these changes

@kv2019i kv2019i kv2019i approved these changes

@abonislawski abonislawski abonislawski approved these changes

@aborisovich aborisovich aborisovich approved these changes

@teburd teburd teburd approved these changes

@ceolin ceolin Awaiting requested review from ceolin

@gmarull gmarull Awaiting requested review from gmarull

@nashif nashif Awaiting requested review from nashif

@marcinszkudlinski marcinszkudlinski Awaiting requested review from marcinszkudlinski

@dcpleung dcpleung Awaiting requested review from dcpleung

@andyross andyross Awaiting requested review from andyross

@lgirdwood lgirdwood Awaiting requested review from lgirdwood

@lyakh lyakh Awaiting requested review from lyakh

@marc-hb marc-hb Awaiting requested review from marc-hb

Assignees

@teburd teburd

Labels
area: DMA Direct Memory Access area: Power Management area: Xtensa Xtensa Architecture platform: Intel ADSP Intel Audio platforms
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@tmleman @teburd @ceolin @aborisovich @abonislawski @kv2019i @marcinszkudlinski @softwarecki @carlescufi @zephyrbot