Skip to content

Virtualizetion of the whole BallDomain#17

Merged
Mikemy666 merged 1 commit into
DangoSys:mainfrom
Mikemy666:BallDomain
Dec 8, 2025
Merged

Virtualizetion of the whole BallDomain#17
Mikemy666 merged 1 commit into
DangoSys:mainfrom
Mikemy666:BallDomain

Conversation

@Mikemy666
Copy link
Copy Markdown
Contributor

Finish the Virtualization of the whole BallDomain. The ToPhysicaline is applied in fronot of the Controller.scala module

Copilot AI review requested due to automatic review settings December 8, 2025 06:24
@Mikemy666 Mikemy666 merged commit a10a666 into DangoSys:main Dec 8, 2025
4 of 5 checks passed
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR completes the virtualization of the BallDomain by relocating the ToPhysicalLine module from the BBus to the MemDomain, positioning it in front of the MemController. This architectural change unifies the memory interface using SramReadWithInfo and SramWriteWithInfo bundles that carry additional metadata (rob_id, is_acc, bank_id) through the system.

Key Changes:

  • Moved ToPhysicalLine instantiation from BBus to MemDomain for better separation of concerns
  • Updated BallDomain and BBus interfaces to use unified SramReadWithInfo/SramWriteWithInfo bundles instead of separate SRAM/ACC interfaces
  • Removed obsolete accRead and accWrite connections throughout the hierarchy

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

File Description
arch/src/main/scala/framework/memdomain/MemDomain.scala Adds ToPhysicalLine module to convert unified virtual interfaces to physical SRAM/ACC interfaces before MemController
arch/src/main/scala/framework/balldomain/bbus/bbus.scala Removes ToPhysicalLine instantiation and updates IO to use unified virtual interfaces
arch/src/main/scala/examples/toy/balldomain/BallDomain.scala Updates BallDomainIO to use unified virtual interfaces, removes obsolete ACC connections
arch/src/main/scala/examples/toy/ToyBuckyBall.scala Removes obsolete accRead and accWrite connections between BallDomain and MemDomain

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +116 to 120
// val toPhysicalLines = Module(new ToPhysicalLine()(b, p))

// toPhysicalLines.io.sramRead_i <> memoryrouter.io.sramRead_o
// toPhysicalLines.io.sramWrite_i <> memoryrouter.io.sramWrite_o

Copy link

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

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

Consider removing this commented-out code. The ToPhysicalLine module has been moved to MemDomain as part of this PR, so these commented lines serve no purpose and clutter the codebase.

Suggested change
// val toPhysicalLines = Module(new ToPhysicalLine()(b, p))
// toPhysicalLines.io.sramRead_i <> memoryrouter.io.sramRead_o
// toPhysicalLines.io.sramWrite_i <> memoryrouter.io.sramWrite_o

Copilot uses AI. Check for mistakes.
Comment on lines +35 to +36
// val accRead = Vec(b.acc_banks, Flipped(new SramReadIO(b.acc_bank_entries, b.acc_w)))
// val accWrite = Vec(b.acc_banks, Flipped(new SramWriteIO(b.acc_bank_entries, b.acc_w, b.acc_mask_len)))
Copy link

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

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

Consider removing this commented-out code for accRead and accWrite. Since the virtualization now uses unified numBanks interfaces, these old separate accumulator interfaces are obsolete and keeping them commented out may cause confusion.

Suggested change
// val accRead = Vec(b.acc_banks, Flipped(new SramReadIO(b.acc_bank_entries, b.acc_w)))
// val accWrite = Vec(b.acc_banks, Flipped(new SramWriteIO(b.acc_bank_entries, b.acc_w, b.acc_mask_len)))

Copilot uses AI. Check for mistakes.
Comment on lines +28 to +29
// val accRead = Vec(b.acc_banks, Flipped(new SramReadIO(b.acc_bank_entries, b.acc_w)))
// val accWrite = Vec(b.acc_banks, Flipped(new SramWriteIO(b.acc_bank_entries, b.acc_w, b.acc_mask_len)))
Copy link

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

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

Consider removing this commented-out code for accRead and accWrite. Since the virtualization now uses unified numBanks interfaces, these old separate accumulator interfaces are obsolete and keeping them commented out may cause confusion.

Suggested change
// val accRead = Vec(b.acc_banks, Flipped(new SramReadIO(b.acc_bank_entries, b.acc_w)))
// val accWrite = Vec(b.acc_banks, Flipped(new SramWriteIO(b.acc_bank_entries, b.acc_w, b.acc_mask_len)))

Copilot uses AI. Check for mistakes.
memStorer.io.accRead <> memController.io.dma.accRead


// ToPhysical interface
Copy link

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

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

The comment has a spelling error: "ToPhysical" should be "ToPhysicalLine" for clarity and consistency with the module name.

Suggested change
// ToPhysical interface
// ToPhysicalLine interface

Copilot uses AI. Check for mistakes.
@@ -11,6 +11,7 @@ import framework.balldomain.bbus.pmc.BallCyclePMC
import framework.balldomain.bbus.cmdrouter.CmdRouter
import framework.balldomain.bbus.memrouter.MemRouter
import framework.switcher.{ToPhysicalLine, ToVirtualLine}
Copy link

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

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

The ToPhysicalLine import is no longer used in this file since the ToPhysicalLine module instantiation has been removed and moved to MemDomain. Consider removing this unused import to keep the code clean.

Suggested change
import framework.switcher.{ToPhysicalLine, ToVirtualLine}
import framework.switcher.ToVirtualLine

Copilot uses AI. Check for mistakes.
shirohasuki pushed a commit that referenced this pull request May 19, 2026
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