Skip to content

bcm2835-dma: Fixes for dma_abort #5482

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.

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

Merged
merged 2 commits into from
May 30, 2023
Merged

Conversation

popcornmix
Copy link
Collaborator

@popcornmix popcornmix commented May 25, 2023

There is a problem with the current abort scheme
when dma is blocked on a DREQ which prevents halting.

This is triggered by SPI driver which aborts dma
in this state and so leads to a halt timeout.

Discussion with Broadcom suggests the sequence:

CS.ACTIVE=0
while (CS.OUTSTANDING_TRANSACTIONS == 0)
  wait()
DEBUG.RESET=1

should be safe on a dma40 channel.

Unfortunately the non-dma40 channels don't have
OUTSTANDING_TRANSACTIONS, only WAITING_FOR_OUTSTANDING_WRITES which doesn't consider reads.

@popcornmix
Copy link
Collaborator Author

This seems to behave as desired for HDMI audio and SPI display.
I'm waiting for feedback on the non-dma40 case which may be unsafe with outstanding reads.

@popcornmix popcornmix force-pushed the dma_abort2 branch 2 times, most recently from 7792504 to 6f3a8de Compare May 26, 2023 14:48
@popcornmix popcornmix marked this pull request as ready for review May 26, 2023 15:47
There is a problem with the current abort scheme
when dma is blocked on a DREQ which prevents halting.

This is triggered by SPI driver which aborts dma
in this state and so leads to a halt timeout.

Discussion with Broadcom suggests the sequence:

CS.ACTIVE=0
while (CS.OUTSTANDING_TRANSACTIONS == 0)
  wait()
DEBUG.RESET=1

should be safe on a dma40 channel.

Unfortunately the non-dma40 channels don't have
OUTSTANDING_TRANSACTIONS, so we need a more
complicated scheme.

We attempt to abort the channel, which will work
if there is no blocked DREQ.

It it times out, we can assume there is no AXI
transfer in progress and reset anyway.

The length of the timeout is observed at ~20us.

Signed-off-by: Dom Cobley <[email protected]>
@popcornmix
Copy link
Collaborator Author

Updated to do something plausible for non-dma40 case.
We do have to rely on a timeout (I've chosen 100 loops which equates to 20us, which could be increased).

@popcornmix
Copy link
Collaborator Author

@pelwell any comments?

Copy link
Contributor

@pelwell pelwell left a comment

Choose a reason for hiding this comment

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

Sorry, forgot to submit the final review. The rest looks OK, but then it looked OK before.

@@ -245,6 +245,7 @@ struct bcm2835_desc {
#define BCM2711_DMA40_ERR BIT(10)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we have the BCM2711_DMA40_PROT bits here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added commit to move PROT bits definition.

Fixes: 654368f ("bcm2835-dma: Need to keep PROT bits set in CS on 40bit controller")

Signed-off-by: Dom Cobley <[email protected]>
@pelwell pelwell merged commit 6735fb0 into raspberrypi:rpi-6.1.y May 30, 2023
@popcornmix popcornmix deleted the dma_abort2 branch May 30, 2023 15:30
popcornmix added a commit to raspberrypi/rpi-firmware that referenced this pull request May 30, 2023
See: raspberrypi/linux#5482

kernel: Fix I2C issues for DSI touch and V3 camera
See: raspberrypi/linux#5479

kernel: pps: Compatibility hack should be X86-specific
See: raspberrypi/linux#5478
popcornmix added a commit to raspberrypi/firmware that referenced this pull request May 30, 2023
See: raspberrypi/linux#5482

kernel: Fix I2C issues for DSI touch and V3 camera
See: raspberrypi/linux#5479

kernel: pps: Compatibility hack should be X86-specific
See: raspberrypi/linux#5478
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.

2 participants