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

PP voltages #34

Open
PJVol opened this issue Aug 6, 2022 · 8 comments
Open

PP voltages #34

PJVol opened this issue Aug 6, 2022 · 8 comments

Comments

@PJVol
Copy link

PJVol commented Aug 6, 2022

Hi!
When looking at the pp_table dump files in test folder, I noticed that all reported voltages kinda high, except those stored in float )
https://github.com/sibradzic/upp/blob/master/test/AMD.RX6900.16384.201104.rom.dump

  MinVoltageGfx: 3300
  MinVoltageSoc: 3300
  MaxVoltageGfx: 4700
  MaxVoltageSoc: 4600

Don't you think that something like ">> 2" is missing here ?

@sibradzic
Copy link
Owner

sibradzic commented Aug 6, 2022

For RDNAx cards, most of the voltages in PowePlay tables are represented as 4x of the actual voltage, in millivolts. For example, a MinVoltageGfx: 3300 represents the 3300/4 = 825 millivolts = 0.825V.

Don't you think that something like ">> 2" is missing here ?

Sorry, I don't get what do you mean by that, please elaborate...

@PJVol
Copy link
Author

PJVol commented Aug 7, 2022

the voltages in PowePlay tables are represented as 4x of the actual voltage, in millivolts

Fine, if you know it, ain't it make sense to report (value_in_the_table >> 2) ? ( right shift )
After all, you do report these as floats

    Vdroop:
      Vdroop 0: 0.04
      Vdroop 1: 0.0655
      Vdroop 2: 0.089
      Vdroop 3: 0.098
      Vdroop 4: 0.358

@sibradzic
Copy link
Owner

After all, you do report these as floats

In Linux PPTable headers for Radeon VII (Vega 20) and newer cards, some values such as voltage/freq quadratic curve parameters (a, b & c) or droop values are represented as unsigned int, but they are obviously float values. Reading / writing such parameters with upp in its uint form does not make much sense, as they pretty much not human-readable representations of float numbers.

For details on which particular values are interpreted as float by upp, check

upp/src/upp/decode.py

Lines 25 to 28 in 80337d3

# Defined as uint in kernel, but in reality these are float
float_fields = ['a', 'b', 'c', 'm',
'VcBtcPsmA', 'VcBtcPsmB', 'VcBtcVminA', 'VcBtcVminB']
float_arrays = ['Fset', 'Vdroop']

@PJVol
Copy link
Author

PJVol commented Aug 8, 2022

For details on which particular values are interpreted as float by upp, check

Sorry if I wasn't clear enough.
The question was why do you report these values as uints, when its explicitly stated they are Q2?

MinVoltageGfx: 3300
MinVoltageSoc: 3300
MaxVoltageGfx: 4700
MaxVoltageSoc: 4600

https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/gpu/drm/amd/pm/inc/smu11_driver_if_sienna_cichlid.h?h=linux-5.13.y#n639

  // SECTION: Voltage Control Parameters
  uint16_t     MinVoltageGfx;     // In mV(Q2) Minimum Voltage ("Vmin") of VDD_GFX
  uint16_t     MinVoltageSoc;     // In mV(Q2) Minimum Voltage ("Vmin") of VDD_SOC
  uint16_t     MaxVoltageGfx;     // In mV(Q2) Maximum Voltage allowable of VDD_GFX
  uint16_t     MaxVoltageSoc;     // In mV(Q2) Maximum Voltage allowable of VDD_SOC

@sibradzic
Copy link
Owner

sibradzic commented Aug 8, 2022

why do you report these values as uints, when its explicitly stated they are Q2?

I am not reporting anything, these parameters are all obviously declared as uint16_t. The mV(Q2) in the comment is only a relatively recent addition to the Linux headers code, and it only appears in some of the headers, and only as a comments, and as such there is no point in "reporting" (interpreting) this Q2 in some special way, especially without any clue elsewhere in the kernel sources what Q2 really mean.

Please note that the goal of the upp project is not to interpret any particular parameter, but to allow decoding, reading and writing any parameter in the PPTable, even for the future cards, regardless of what any particular parameter really mean.

The only "exceptions" to this principle are the cases where raw values make no sense to an average human (float interpretation rules) and few cases where there are bugs in the data structures (mainly version overrides for Polaris era tables).

If you think that interpreting mV(Q2) may be useful, contributions are welcome m(_ _)m

@PJVol
Copy link
Author

PJVol commented Aug 8, 2022

I am not reporting anything

The dump command de-serializes PowerPlay binary data into a human-readable text output.

If you think that interpreting mV(Q2) may be useful

Well... (unless you're just kidding :) )

Q(N) is a commonly used notation to describe binary fixed point numbers (signed) - Qn.m, where n bits used for the integer part except sign bit, and m - fraction bits.
AMD just do count a sign bit, i.e. Q20.12 means 20 bit signed int and 12 bit fraction.

So what Q2 means here is the 32-bit value used to store fixed point number where 2 lsb is a fraction, and int part is the rest, with the scaling factor 2^-2 = 0.25

@PJVol
Copy link
Author

PJVol commented Aug 8, 2022

And this notation was there since the initial rdna 2 support in 5.9 in 2020.

@PJVol
Copy link
Author

PJVol commented Aug 8, 2022

I also do realize that the idea was in using amdgpu driver headers to deal with pp_table in windows, that is imo pretty smart.
I just deemed it appropriate to take the actual format of the numeric data into account.
That's basicslly it.

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

No branches or pull requests

2 participants