Patch Set 6:

(3 comments)

Overall this change looks quite good - and, considering that the built binaries do not change as you said, I just need more understanding about some options before giving it a +2

I honestly don't know what over half of these options mean. I just compared the values and promoted the most commonly used value as default.

View Change

5 comments:

• File src/vendorcode/amd/agesa/f14/Config/PlatformInstall.h:

  • Patch Set #6, Line 210: OPTION_GFX_RECOVERY

    What this OPTION_GFX_RECOVERY means, and why it has been removed?

    AGESA f14 only has one socket, so this would be the equivalent of disabling it by default. Since all boards disabled it, I flipped the default to disabled. Note that the BLDOPT macro that originally disabled this option is now inverted, so as to allow enabling this option.

  • Patch Set #6, Line 239: OPTION_DMI

    More info about OPTION_DMI is also needed for review, to understand its' change from TRUE to FALSE

    IIRC, it's about DMI tables. All boards disabled DMI, so I disabled it by default and flipped the BLDOPT macros to allow enabling it.

• File src/vendorcode/amd/agesa/f16kb/Config/OptionGnbInstall.h:

  • Patch Set #6, Line 163: PcieTrainingDistributed

    Which algorithm is better - PcieTrainingStandard or PcieTrainingDistributed ?

    No idea, but all boards seem to use the latter, so I made it the default value.

• File src/vendorcode/amd/agesa/f16kb/Config/PlatformInstall.h:

  • Patch Set #6, Line 860: CStateModeDisabled

    Is this a better default? In my experience, both CStateModeC6 and CStateModeDisabled are working

    IIRC all f16kb boards were choosing this value. To be reproducible, I've taken this value as the default, and it's what the boards would have been tested with.

    If CStateModeC6 is working as intended, we can flip it later for all boards from here, in a separate commit. Does it sound reasonable?

• File src/vendorcode/amd/agesa/f16kb/Proc/CPU/cpuLateInit.h:

  • Patch Set #6, Line 232: SCOPE_NAME_P

    What does it mean - these changes from SCOPE_NAME_C to SCOPE_NAME_P ?

    f16kb boards seem to use "P0" instead of "C0" on older families. I factored it out for reproducibility. If I had to take a guess, P0 is a performance state, which older platforms may lack.

To view, visit change 41667. To unsubscribe, or for help writing mail filters, visit settings.