BeardOverflow / msi-ec

GNU General Public License v2.0
149 stars 45 forks source link

Add MSI Bravo 15 A4DDR Support and Documentation #135

Closed Jiogo18 closed 4 months ago

Jiogo18 commented 5 months ago

Working notes in issue #134

This pull request adds support for MSI Bravo 15 A4DDR but doesn't solve the issues of target vs RPM and 150% fan speed.

glpnk commented 5 months ago

Fan mode: values can also be 0x81... when booting on Linux

Some devices have unspecified shift, hard to tell which one is it.

@teackot I think it's ready to merge

teackot commented 5 months ago

0x80 is often applied after reboot, when the system doesn't know what mode is active.

teackot commented 5 months ago

CONF134

Okay, now this gave me an idea. Right now we use simple ordered names like CONF0, 1, 2, 3 ... so this one should've been 23rd. But this causes some problems: if 2 or more PRs are being developed simultaneously this will cause more conflicts to resolve as they will use the same conf number (like when we had around 4 CONF20s in different PRs). It's also difficult to reorder them (if we ever need it for some reason)

So, I think maybe we can change the naming scheme? Not using the issue numbers though, as it doesn't make sense outside of this repo (e.g. upstream in the kernel).

Maybe the board name? This one would be CONF_16WK (when we find an incompatible FW we'll just call it CONF_16WK_2). There might be a better way.

Or maybe I'm overthinking it and we can just keep going with the simple ordered numbers.

glpnk commented 5 months ago

Anyway, 2+ simultaneous PRs started from same commit require manual conflict fix, even if different numbers are used

Maybe the board name? This one would be CONF_16WK (when we find an incompatible FW we'll just call it CONF_16WK_2). There might be a better way.

I think there are no incompatible FW, except 2 generations and models which don't have some features

So we can start matching WMI GUIDs to get generation

glpnk commented 5 months ago

image

Map of values covered by each WMI generation (DSDT souced)

teackot commented 5 months ago

Anyway, 2+ simultaneous PRs started from same commit require manual conflict fix, even if different numbers are used

Okay, so we'll stay with ordered numbers then

So we can start matching WMI GUIDs to get generation

Map of values covered by each WMI generation (DSDT souced)

Awesome

glpnk commented 5 months ago

We need better place for discussions, other than random PRs/Issues, so https://github.com/BeardOverflow/msi-ec/discussions/98#discussioncomment-9817838

glpnk commented 5 months ago

By design, 0x80 values is unspecified shift, so you need to use 0xC? for shifts managed by msi-ec

0x8? may work but original app not use it

Jiogo18 commented 5 months ago

Every time my computer restart, the value goes back to 0x8?. /sys/devices/platform/msi-ec/shift_mode outputs unknown (129), so power mode in MControlCenter is greyed out.

I just need to be able to read 0x8? values, shift mode can be set to 0xC? if you think it's better. Should I revert to 0xC? and I manage "unknown" another way?

glpnk commented 5 months ago

Yes, 0x80 is no shift setting, you need to manually set it to 0xC?

My laptops BIOS (modern c5m) have setting for default shift on startup. On my device it called User Scenario in Advanced tab

so power mode in MControlCenter is greyed out

check Mcontrol issues and if there are no similar issue, create it

You can open Extended BIOS menu with rAlt + lCtrl + rShift + F2 (or different combo of right/left keys combo), but you can break something there, be careful, your warranty might be voided

Jiogo18 commented 5 months ago

I don't think I have the extended BIOS, and I don't have User Scenario in Advanced neither. If shift mode is C4, after a restart it is 84.

check Mcontrol issues and if there are no similar issue, create it

It's about the qdbus I added, so it's quite recent, and I don't think "unknown" should be handled by MControlCenter. Some issues report that shift mode is not working, but this is because MControlCenter only reads at 0xD2 and not 0xF2 for now.

glpnk commented 5 months ago

but this is because MControlCenter only reads at 0xD2 and not 0xF2 for now.

Yeah, sorry, I forgot about this

If shift mode is C4, after a restart it is 84.

Weird, but I think you need to write 0xC? to properly trigger shift change, even if it resets to unspecified. But it's interesting case, to restore state on reboot

glpnk commented 5 months ago

Do you haven't 0xC0 shift?

Jiogo18 commented 5 months ago

Is it be possible to read:

But only write 0xC? If so, we can move in a new issue

Jiogo18 commented 5 months ago

Do you haven't 0xC0 shift?

No, MSI Center doesn't have "Sport" mode, only "Extreme Performance" with 0xC4.

glpnk commented 5 months ago

Ok, thanks

Is it be possible to read:

Good idea

Jiogo18 commented 5 months ago

What about

if ((rdata | 0x40) == conf.shift_mode.modes[i].value) {

https://github.com/BeardOverflow/msi-ec/blob/42cab47c10664e23916601ea32981bcdc3772968/msi-ec.c#L2445-L2447

0x80 values is unspecified shift

It means that the value would be interpreted as "sport"... And adding && rdata != 0x80 feels obnoxious.

Or a new variable in msi_ec_shift_mode_conf to enable or not the artificial bit 0x40?

glpnk commented 5 months ago

@Jiogo18 You can prove is 0x8? / 0xC? changing shift with RyzenAdj. For my device, STAPM LIMIT in eco/comfort/sport shift is set to 13/15/48. But changes occurs only when 0xC? is written to EC, 0x8? is ignored.

Don't change anything in RyzenAdj, just make dump with sudo ryzenadj -i and look for STAPM LIMIT values

Also, can you send me output of next command. It might help us to figure how to detect order of FN-WIN buttons. Sudo not required.

efivar -n dd96baaf-145e-4f56-b1cf-193256298e99-MsiDCVarData -p
Jiogo18 commented 5 months ago

STAPM LIMIT:

I don't feel the difference on Linux, expect when using intensive tasks (benchmarks, maybe games).

Also, can you send me output of next command. It might help us to figure how to detect order of FN-WIN buttons. Sudo not required.

GUID: dd96baaf-145e-4f56-b1cf-193256298e99
Name: "MsiDCVarData"
Attributes:
Non-Volatile
Boot Service Access
Runtime Service Access
Value:
00000000  01 01 00 00 03 00 00 00  00 00 00 00 00 00 00 00  |................|
00000010  00 00 00 00                                       |....            |

For the record: Physically, Win/Super key is on the left, fn key is on the right. Default is 0x00. Value 0x10 set fn on the left key and Super on the right key.

Jiogo18 commented 5 months ago

STAPM LIMIT is also set at 45 after sleep, restart or shutdown.

mutchiko commented 5 months ago

@Jiogo18 could you copy the ouput of this command: sudo cat /sys/power/mem_sleep

glpnk commented 5 months ago

So, 0x8? values not changing shift at all or reset it to performance?

As far as I understand, 0x00 address of EFI var you dumped used for fnwin swap. 0x01 = fn physically on right side, win on left 0x00 = fn left, win right

Jiogo18 commented 5 months ago

@Jiogo18 could you copy the ouput of this command: sudo cat /sys/power/mem_sleep

s2idle [deep]

So, 0x8? values not changing shift at all or reset it to performance?

It seems no, indeed. I haven't checked on Windows, but with what you said it's unlikely.

As far as I understand, 0x00 address of EFI var you dumped used for fnwin swap. 0x01 = fn physically on right side, win on left 0x00 = fn left, win right

The EFI dump sent had 0x00 = no swap. Currently, physically and by default (pattern on the keys): win left, fn right = 0x00 Swapped: 0x10 = fn left, win right

glpnk commented 5 months ago

@Jiogo18 Thank you! How you swapped WIN-FN from Dragon Center/other MSI app or from BIOS? Or by manual editing of EFI var? Is swapping from EC changes affects EFI variable?

Is your PR ready for merge?

Jiogo18 commented 5 months ago

@Jiogo18 Thank you! How you swapped WIN-FN from Dragon Center/other MSI app or from BIOS? Or by manual editing of EFI var?

Thanks for sharing the knowledge! On Windows with MSI Center. (Had Dragon Center 2 years ago too, I think it was also possible.) And manual editing with isw when on Linux (so EC, I don't know about EFI) I don't think there is a BIOS option.

Is your PR ready for merge?

Except for the 0xC? => 0x8? issue when entering sleep/restarting (which doesn't only affect this computer, if I understood correctly), yes.

mutchiko commented 5 months ago

@Jiogo18 you seem to use S3 sleep, did you change bios settings? or maybe your laptop doesn't support modern standby?

if the laptop wakes up from S3 sleep state it will reset SMU settings (STAPM and other stuff) so that's why you see the 45W limit, from my testing this doesn't happen with modern standby.

glpnk commented 5 months ago

@Jiogo18 I've thought that seen commit to MControlCenter which applies settings after resuming from sleep. But it was your commit... You can apply shift after resuming from sleep like you did for cooler.

And manual editing with isw when on Linux (so EC, I don't know about EFI)

Ok, we just spoke about different things, thanks for explanation.

BIOS have hidden settings to enable/disable EC-WIN swap, it's hidden by variable which you dumped. Address 1 of this var (I've marked it as YY below) hides setting from BIOS menu. Address 0 (this is XX value) shows Swap state, where 1 is right FN, left Win. 0 - left FN, right Win. Seems to be same on both keyboard types.

Editing this variable seem to be only way to make swap persistent. Another question - who really use keys swap IRL.

GUID: dd96baaf-145e-4f56-b1cf-193256298e99
Name: "MsiDCVarData"
Attributes:
Non-Volatile
Boot Service Access
Runtime Service Access
Value:
00000000  XX YY 00 00 03 00 00 00  00 00 00 00 00 00 00 00  |................|
00000010  00 00 00 00                                       |....            |
Jiogo18 commented 5 months ago

@Jiogo18 you seem to use S3 sleep, did you change bios settings? or maybe your laptop doesn't support modern standby?

I did have changed settings in the BIOS, but there is nothing about S3/modern standby. On Windows, powercfg /a only gives available: S3 So no modern standby support, despite having Bravo 15 in the list.

Reapply shift should be the easiest way, thanks.

Address 0 (this is XX value) shows Swap state

Nothing changed, value is still 0x01 here. Even FN-WIN swap reset to default after sleep mode

glpnk commented 5 months ago

Address 0 (this is XX value) shows Swap state

Nothing changed, value is still 0x01 here. Even FN-WIN swap reset to default after sleep mode

Ok, this is normal.

@teackot ready for merge

teackot commented 4 months ago

Great