sibradzic / upp

A tool for parsing, dumping and modifying data in Radeon PowerPlay tables
GNU General Public License v3.0
154 stars 24 forks source link

PP voltages #34

Open PJVol opened 2 years ago

PJVol commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 https://github.com/sibradzic/upp/blob/80337d3a288acc3b7b2225e4a812d0e009f8490b/src/upp/decode.py#L25-L28

PJVol commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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

PJVol commented 2 years ago

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.