Skip to content

schedule: zephyr_dma_domain: Add support for shared IRQs#7923

Merged
lgirdwood merged 1 commit intothesofproject:mainfrom
LaurentiuM1234:zephyr_dma_domain_arm64_shared_irq_fix
Jul 31, 2023
Merged

schedule: zephyr_dma_domain: Add support for shared IRQs#7923
lgirdwood merged 1 commit intothesofproject:mainfrom
LaurentiuM1234:zephyr_dma_domain_arm64_shared_irq_fix

Conversation

@LaurentiuM1234
Copy link
Copy Markdown
Contributor

In the case of DMA channels using the same IRQ line the same interrupt handler with different data is
registered multiple times for the same interrupt.

This approach works perfectly fine when using the IRQ_STEER IP since the way its driver works is it allows registering multiple handlers+data for the same INTID.

When switching to ARM64, this approach no longer works since the last irq_handler/irq_data pair will overwrite the previous one for the same INTID. Because of this, the IRQ bit from the DMA channel may not get cleared when multiple pipeline tasks are scheduled. This reasoning applies to the ARM64 architecture with GICv3 interrupt controller.

To overcome this, the Zephyr DMA domain now holds a list of channels using the same IRQ. When the DMA IRQ gets triggered, the handler will iterate through the list of channels using the same IRQ and clear the interrupt.

This PR still requires more testing. For now, it has only been tested on ARM64-based i.MX93 and i.MX8QM and works fine for simple audio cases (i.e: simple playback, simple capture, simple playback with pause/resume)

@LaurentiuM1234 LaurentiuM1234 force-pushed the zephyr_dma_domain_arm64_shared_irq_fix branch from 3e56a9e to 29f3196 Compare July 11, 2023 14:01
Copy link
Copy Markdown
Member

@lgirdwood lgirdwood left a comment

Choose a reason for hiding this comment

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

Quick read through, LGTM, does this need any west update ?

@LaurentiuM1234
Copy link
Copy Markdown
Contributor Author

Quick read through, LGTM, does this need any west update ?

No west updates needed

In the case of DMA channels using the same IRQ line
the same interrupt handler with different data is
registered multiple times for the same interrupt.

This approach works perfectly fine when using the IRQ_STEER
IP since the way its driver works is it allows registering
multiple handlers+data for the same INTID.

When switching to ARM64, this approach no longer works since
the last irq_handler/irq_data pair will overwrite the previous
one for the same INTID. Because of this, the IRQ bit from the
DMA channel may not get cleared when multiple pipeline tasks
are scheduled. This reasoning applies to the ARM64 architecture
with GICv3 interrupt controller.

To overcome this, the Zephyr DMA domain now holds a list
of channels using the same IRQ. When the DMA IRQ gets triggered,
the handler will iterate through the list of channels using the
same IRQ and clear the interrupt.

Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
@LaurentiuM1234
Copy link
Copy Markdown
Contributor Author

FYI: currently waiting on #7976 before I can test the changes from this PR. Hopefully that PR will fix the annoying crashes during stress testing.

@lgirdwood lgirdwood added this to the v2.7 milestone Jul 26, 2023
@LaurentiuM1234
Copy link
Copy Markdown
Contributor Author

Update: tested on i.MX8QM and doesn't break anything. From my POV this can be merged after #7976. @dbaluta has this been passed through our internal CI?

@dbaluta
Copy link
Copy Markdown
Collaborator

dbaluta commented Jul 31, 2023

@LaurentiuM1234 Yes, this got through one round of internal CI. Looks good to me. Just take it out of Draft state.

@LaurentiuM1234 LaurentiuM1234 marked this pull request as ready for review July 31, 2023 08:34
@lgirdwood
Copy link
Copy Markdown
Member

@juimonen any comments ?

@juimonen
Copy link
Copy Markdown

@juimonen any comments ?

No, fine by me. I see this is IMX only.

@lgirdwood lgirdwood merged commit cfd7861 into thesofproject:main Jul 31, 2023
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.

6 participants