Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/libgfxinit/+/44786 ) Change subject: gma registers: Allow to specify an offset for display registers ...................................................................... Patch Set 3: (2 comments) https://review.coreboot.org/c/libgfxinit/+/44786/3/common/hw-gfx-gma-config.... File common/hw-gfx-gma-config.ads.template: https://review.coreboot.org/c/libgfxinit/+/44786/3/common/hw-gfx-gma-config.... PS3, Line 357: Base_Offset Nit `base` and `offset` sound redundant. Maybe just `Display_Base` like the already present `Fence_Base`? https://review.coreboot.org/c/libgfxinit/+/44786/3/common/hw-gfx-gma-registe... File common/hw-gfx-gma-registers.adb: https://review.coreboot.org/c/libgfxinit/+/44786/3/common/hw-gfx-gma-registe... PS3, Line 214: return Reg'Enum_Rep + Config.Display_Base_Offset / Register_Width; I don't understand. This Index() function is used for all registers, not just the display registers. Shouldn't the offset be added to individual registers? -- To view, visit https://review.coreboot.org/c/libgfxinit/+/44786 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: libgfxinit Gerrit-Branch: master Gerrit-Change-Id: Id5497a23776e9aa9e2fd5ca0479030cb8e55712f Gerrit-Change-Number: 44786 Gerrit-PatchSet: 3 Gerrit-Owner: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Wed, 26 Aug 2020 17:07:46 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment