Skip to content

WIP: vc4: Avoid hdmi audio underrun with heavy sdram traffic #4053

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 4 commits into from
Jan 7, 2021
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 17 additions & 4 deletions drivers/dma/bcm2835-dma.c
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,17 @@ struct bcm2835_desc {
#define WAIT_RESP(x) ((x & BCM2835_DMA_NO_WAIT_RESP) ? \
0 : BCM2835_DMA_WAIT_RESP)

/* A fake bit to request that the driver requires wide reads */
#define BCM2835_DMA_WIDE_SOURCE BIT(28)
#define WIDE_SOURCE(x) ((x & BCM2835_DMA_WIDE_SOURCE) ? \
Copy link
Contributor

Choose a reason for hiding this comment

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

It might be wise to put some parentheses around the x here (and WAIT_RESP and WIDE_DEST), just in case.

BCM2835_DMA_S_WIDTH : 0)

/* A fake bit to request that the driver requires wide writes */
#define BCM2835_DMA_WIDE_DEST BIT(29)
#define WIDE_DEST(x) ((x & BCM2835_DMA_WIDE_DEST) ? \
BCM2835_DMA_D_WIDTH : 0)


/* debug register bits */
#define BCM2835_DMA_DEBUG_LAST_NOT_SET_ERR BIT(0)
#define BCM2835_DMA_DEBUG_FIFO_ERR BIT(1)
Expand Down Expand Up @@ -850,8 +861,9 @@ static struct dma_async_tx_descriptor *bcm2835_dma_prep_dma_memcpy(
{
struct bcm2835_chan *c = to_bcm2835_dma_chan(chan);
struct bcm2835_desc *d;
u32 info = BCM2835_DMA_D_INC | BCM2835_DMA_S_INC;
u32 extra = BCM2835_DMA_INT_EN | WAIT_RESP(c->dreq);
u32 info = BCM2835_DMA_D_INC | BCM2835_DMA_S_INC | WAIT_RESP(c->dreq) |
WIDE_SOURCE(c->dreq) | WIDE_DEST(c->dreq);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the moving of WAIT_RESP to every transfer (and not just the last) and considered change?

And the new WIDE flags won't get applied to DMA40 channels until to_bcm2711_srci and to_bcm2711_dsti are also changed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It seemed to make more sense to me that way although was intended for discussion (PR got merged before that though). @HiassofT did report that reverting this change didn't avoid the corruption. And excluding this change the commit should be a no-op on Pi2/3 with c->dreq not including bits 28/29, so I'm a bit confused exactly where the cause of corruption is.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, so am I.

u32 extra = BCM2835_DMA_INT_EN;
size_t max_len = bcm2835_dma_max_frame_length(c);
size_t frames;

Expand Down Expand Up @@ -881,7 +893,8 @@ static struct dma_async_tx_descriptor *bcm2835_dma_prep_slave_sg(
struct bcm2835_chan *c = to_bcm2835_dma_chan(chan);
struct bcm2835_desc *d;
dma_addr_t src = 0, dst = 0;
u32 info = WAIT_RESP(c->dreq);
u32 info = WAIT_RESP(c->dreq) |
WIDE_SOURCE(c->dreq) | WIDE_DEST(c->dreq);
u32 extra = BCM2835_DMA_INT_EN;
size_t frames;

Expand Down Expand Up @@ -943,7 +956,7 @@ static struct dma_async_tx_descriptor *bcm2835_dma_prep_dma_cyclic(
struct bcm2835_chan *c = to_bcm2835_dma_chan(chan);
struct bcm2835_desc *d;
dma_addr_t src, dst;
u32 info = WAIT_RESP(c->dreq);
u32 info = WAIT_RESP(c->dreq) | WIDE_SOURCE(c->dreq) | WIDE_DEST(c->dreq);
u32 extra = 0;
size_t max_len = bcm2835_dma_max_frame_length(c);
size_t frames;
Expand Down