Himbaechel Gowin: HCLK Support #1340
Conversation
himbaechel/uarch/gowin/cst.cc
Outdated
| @@ -166,6 +194,27 @@ struct GowinCstReader | |||
| } | |||
| } | |||
| } break; | |||
| case hclk: { | |||
| IdString cell_type = it->second->type; | |||
| if (cell_type != IdString(ID_CLKDIV)) { | |||
There was a problem hiding this comment.
id_CLKDIV instead of IdString(ID_CLKDIV)
himbaechel/uarch/gowin/pack.cc
Outdated
| auto ci = cell.second.get(); | ||
| if (ci->type == id_CLKDIV) { | ||
| NetInfo *in = ci->getPort(id_HCLKIN); | ||
| CellInfo *this_driver = in->driver.cell; |
There was a problem hiding this comment.
this should probably have some error handling for the case of HCLKIN being disconnected/undriven, i.e. either in or in->driver.cell being nullptr
himbaechel/uarch/gowin/pack.cc
Outdated
| if (out->users.entries() > 1) { | ||
| // We could do as the IDE does sometimes and replicate the CLKDIV2 cell | ||
| // as many times as we need. For now, we keep things simple | ||
| log_error("CLKDIV2 that drives CLKDIV should drive no other cells"); |
There was a problem hiding this comment.
log messages always need to end in \n
|
This all looks reasonable to me - @yrabbit do you have any thoughts? |
I don't see anything criminal. It's unusual for me to see something in the prePlace() function, there is probably some philosophical reason for this :) |
|
Thank you, @gatecat. I've added the null checks and corrected the log statement. |
Video always feels magical! :) |
|
Should this be merged before or after the apicula PR? |
|
I think it's best to wait for Apicula. I'll add a comment here when the PR there gets merged. Thanks for all the work you've put into nextpnr, @gatecat 🙌 |
himbaechel/uarch/gowin/gowin.h
Outdated
| inline bool type_is_clkdiv2(IdString cell_type) { return cell_type == id_CLKDIV2; } | ||
| inline bool is_clkdiv2(const CellInfo *cell) { return type_is_clkdiv2(cell->type); } | ||
|
|
||
| // Return true for HCLK BELs |
Apicula CI wont pass until this is merged, so I'd prefer to merge this first. |
|
There's a bug here that I need to address. Please don't merge yet. |
All clear now. |
|
Seems like there is a merge conflict |
I've now resolved it |
|
Thanks! |
|
It's my pleasure. Thanks for merging! |
HCLK Support
This pull request introduces support for the CLKDIV and CLKDIV2 BELs in Gowin FPGAs. The implementation has been successfully tested on the GW2AR-18 and GW1N-9C models and is expected to be compatible with (most) other Gowin devices as well.
This PR implements a custom placer for CLKDIV/CLKDIV2 (
place_constrained_hclk_cells) due to the nature of the implicit constraints around placing them. Said placer uses bipartite matching so this PR also adds some code to support that. Every other change is, by my observation, par for the course for adding support for new bels.Here's a link to the companion PR on Apicula: YosysHQ/apicula#258