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

Fixed high fan speed displayed on low RPM #175

Merged
merged 1 commit into from
Jun 15, 2024

Conversation

Jiogo18
Copy link
Contributor

@Jiogo18 Jiogo18 commented Jun 12, 2024

Configuration

Computer: MSI Bravo 15 A4DDR
Motherboard: 16WKEMS1.105
GPU Fan address: 0xCA and 0xCB (in green, fan 2)
CPU Fan address: 0xCC and 0xCD (in cyan/blue, fan 1)
This is the same address as described in isw msi-ec datasheet

The issue

With my configuration, the fan speed is represented by 2 bytes as described above.
MControlCenter only reads the second byte.
In general, this is fine, since the first byte has the value 0x00 (e.g. CPU RPM in cyan is correct)

But my GPU fan is often in a low RPM (e.g. transition between on and off).
Which means it is possible to have the value 0x0115, where the RPM should be 1696,
while MControlCenter calculates 470000/0x15 = 22k RPM ! (different from the picture below because the value changes quickly).

The value was 0x0123 when MControlCenter read, RPM should be 1696.
rpm_GPU_inf
The value was 0x0101 when MControlCenter read, RPM should be 1828.
rpm_GPU_inf5
When the value is 0x00ff, the RPM displayed is ~1843 as expected.

Fix

Read the 2 bytes and combine them value = (value1 << 8) | value0

Additional information

I have no idea if this is the same in every scenario.
For instance, in case fan1 is at address 0xC9, is 0xC8 the first byte..?
Command used: watch -d=cumulative -n 0.1 sudo isw -c (with the original isw)

@dmitry-s93 dmitry-s93 merged commit a261160 into dmitry-s93:main Jun 15, 2024
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