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

[i2c] Reduce counter sizes to save area #22577

Merged
merged 1 commit into from
Apr 23, 2024
Merged

Conversation

a-will
Copy link
Contributor

@a-will a-will commented Apr 16, 2024

Reduce counter sizes to only the widths needed to support Standard mode at a 1 GHz core clock.

In the FPGA implementation, this saves 44 flops (~4%) and over 100 LUTs (~6.6%). The granularity of the FPGA area is large, so the size of the savings can be misleading, but compared to what we had prior to the FSM split, this brings us to +29 flops, -59 LUTs for the whole chain of commits.

This builds on #22551. Only the last commit is new.

@a-will a-will requested review from a team as code owners April 16, 2024 04:16
@a-will a-will requested review from hcallahan-lowrisc and timothytrippel and removed request for a team and timothytrippel April 16, 2024 04:16
hw/ip/i2c/data/i2c.hjson Show resolved Hide resolved
hw/ip/i2c/data/i2c.hjson Show resolved Hide resolved
Copy link
Contributor

@andreaskurth andreaskurth left a comment

Choose a reason for hiding this comment

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

Thanks for these optimizations, @a-will. Area matters these days :-)

LGTM; I just have suggestions about capturing this in the documentation. (And obviously please only rebase & merge this after the PR that this depends on got merged.)

hw/ip/i2c/data/i2c.hjson Show resolved Hide resolved
hw/ip/i2c/data/i2c.hjson Show resolved Hide resolved
@a-will
Copy link
Contributor Author

a-will commented Apr 22, 2024

Ah, kokoro provided results. I'll fix the lint errors.

Reduce counter sizes to only the widths needed to support Standard mode
at a 1 GHz core clock.

Signed-off-by: Alexander Williams <[email protected]>
@a-will a-will merged commit b2239fc into lowRISC:master Apr 23, 2024
32 checks passed
@a-will a-will deleted the i2c-slim-down branch April 23, 2024 02:53
@jwnrt
Copy link
Contributor

jwnrt commented Apr 24, 2024

FYI, the //sw/device/tests:rv_dm_access_after_wakeup_dev_fpga_cw310_sival test fails in CI after commit b2239fc38e0725f17b1155d7e48ec6403facf7f6 (but passes before). It must be due to some combination with another commit that was merged before this one since this PR passed CI.

We're not really sure why the test fails yet, but OpenOCD can't connect through JTAG after the chip wakes up from deep sleep:

[2024-04-24T13:57:20.971Z INFO  opentitanlib::debug::openocd::stderr] Error: JTAG scan chain interrogation failed: all ones

@a-will
Copy link
Contributor Author

a-will commented Apr 24, 2024

Given the content of this PR, it is very highly unlikely to be the cause, though.

@jwnrt
Copy link
Contributor

jwnrt commented Apr 24, 2024

I think you're right. Maybe it's just due to the bitstreams available in the cache before and after this PR merged. Apologies for the noise.

@hcallahan-lowrisc hcallahan-lowrisc mentioned this pull request Jun 6, 2024
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants