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.

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

Fix deadlock in the DMA read pipeline #363

Merged
merged 1 commit into from
Jul 25, 2024
Merged

Fix deadlock in the DMA read pipeline #363

merged 1 commit into from
Jul 25, 2024

Conversation

richardyrh
Copy link
Collaborator

When the write_issue_q dequeue is not valid, the random .bits may still indicate it is an acc address or may have its garbage bit set. This will erroneously cause the read pipeline output ready to go low, which combinationally causes the SRAM read response interface to deassert ready. As a result, there is a cycle of stalling reading from the SRAMs; however, writeData.valid does not accommodate this stall as it expects a continuous stream of data. This leads to the write queues with one extra data element not dequeued properly every once in a while, which fills up over time and leads to a deadlock. The fix gates the random bits with the write_issue_q output valid.

@SeahK
Copy link
Collaborator

SeahK commented Jul 17, 2024

@richardyrh Can you check whether the performance is not affected by this change?

@vikramjain236
Copy link
Collaborator

The code also fixes a bug in mvout from spad address
The FP tiled matmul perf seems to be the same

@richardyrh
Copy link
Collaborator Author

Yes, performance seems to be unaffected (well it's an infinite% speedup since previously the whole thing would just lock up). I do not have perms to merge this, perhaps one of you could do it, or granting me the permissions would be nice.

@SeahK SeahK merged commit 53d0d16 into dev Jul 25, 2024
5 checks passed
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.

3 participants