Change in libgfxinit[master]: gma registers: Allow to specify an offset for display registers
Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/libgfxinit/+/44786 )
Change subject: gma registers: Allow to specify an offset for display registers ......................................................................
gma registers: Allow to specify an offset for display registers
Certain platforms, namely Bay Trail and Braswell, have the display engine registers at an offset relative to GTTMMADR base. Apart from that, the registers are rather similar to the ones on GMCH platforms.
Allow platforms to specify at which offset within GTTMM the display registers are located. Use zero for all currently-supported platforms.
Change-Id: Id5497a23776e9aa9e2fd5ca0479030cb8e55712f Signed-off-by: Angel Pons th3fanbus@gmail.com --- M common/hw-gfx-gma-config.ads.template M common/hw-gfx-gma-registers.adb 2 files changed, 3 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/libgfxinit refs/changes/86/44786/1
diff --git a/common/hw-gfx-gma-config.ads.template b/common/hw-gfx-gma-config.ads.template index 241fe8e..ecbd908 100644 --- a/common/hw-gfx-gma-config.ads.template +++ b/common/hw-gfx-gma-config.ads.template @@ -55,6 +55,8 @@
Default_MMIO_Base : constant := <<DEFAULT_MMIO_BASE>>;
+ Display_Base_Offset : constant := 0; + LVDS_Dual_Threshold : constant := 95_000_000;
Ignore_Presence_Straps : constant Boolean := <<IGNORE_STRAPS>>; diff --git a/common/hw-gfx-gma-registers.adb b/common/hw-gfx-gma-registers.adb index 3f0d7ae..5ea134a 100644 --- a/common/hw-gfx-gma-registers.adb +++ b/common/hw-gfx-gma-registers.adb @@ -211,7 +211,7 @@ SPARK_Mode => Off is begin - return Reg'Enum_Rep; + return Reg'Enum_Rep + Config.Display_Base_Offset; end Index; end Rep;
Angel Pons 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 1:
(1 comment)
https://review.coreboot.org/c/libgfxinit/+/44786/1/common/hw-gfx-gma-registe... File common/hw-gfx-gma-registers.adb:
https://review.coreboot.org/c/libgfxinit/+/44786/1/common/hw-gfx-gma-registe... PS1, Line 214: Config.Display_Base_Offset Needs to be divided by Register_Width
Angel Pons 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 1:
(1 comment)
https://review.coreboot.org/c/libgfxinit/+/44786/1/common/hw-gfx-gma-registe... File common/hw-gfx-gma-registers.adb:
https://review.coreboot.org/c/libgfxinit/+/44786/1/common/hw-gfx-gma-registe... PS1, Line 214: Config.Display_Base_Offset
Needs to be divided by Register_Width
Done
Angel Pons has uploaded a new patch set (#2). ( https://review.coreboot.org/c/libgfxinit/+/44786 )
Change subject: gma registers: Allow to specify an offset for display registers ......................................................................
gma registers: Allow to specify an offset for display registers
Certain platforms, namely Bay Trail and Braswell, have the display engine registers at an offset relative to GTTMMADR base. Apart from that, the registers are rather similar to the ones on GMCH platforms.
Allow platforms to specify at which offset within GTTMM the display registers are located. Use zero for all currently-supported platforms.
Change-Id: Id5497a23776e9aa9e2fd5ca0479030cb8e55712f Signed-off-by: Angel Pons th3fanbus@gmail.com --- M common/hw-gfx-gma-config.ads.template M common/hw-gfx-gma-registers.adb 2 files changed, 3 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/libgfxinit refs/changes/86/44786/2
Angel Pons has uploaded a new patch set (#3). ( https://review.coreboot.org/c/libgfxinit/+/44786 )
Change subject: gma registers: Allow to specify an offset for display registers ......................................................................
gma registers: Allow to specify an offset for display registers
Certain platforms, namely Bay Trail and Braswell, have the display engine registers at an offset relative to GTTMMADR base. Apart from that, the registers are rather similar to the ones on GMCH platforms.
Allow platforms to specify at which offset within GTTMM the display registers are located. Use zero for all currently-supported platforms.
Change-Id: Id5497a23776e9aa9e2fd5ca0479030cb8e55712f Signed-off-by: Angel Pons th3fanbus@gmail.com --- M common/hw-gfx-gma-config.ads.template M common/hw-gfx-gma-registers.adb 2 files changed, 3 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/libgfxinit refs/changes/86/44786/3
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?
Angel Pons 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:
(1 comment)
I haven't had time to check this on hardware yet
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 […]
I guess I should constrain the register range to remap here
Angel Pons 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:
(1 comment)
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 […]
Will do
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:
(1 comment)
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 guess I should constrain the register range to remap here
It all depends on whether registers will be used that exist on other platforms and don't have that offset. I seem to remember that there were, but am not 100% sure. (Registers that you add for Bay Trail can all be adjustet to the offset.)
Hello Nico Huber,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/libgfxinit/+/44786
to look at the new patch set (#4).
Change subject: gma registers: Allow to specify an offset for display registers ......................................................................
gma registers: Allow to specify an offset for display registers
Certain platforms, namely Bay Trail and Braswell, have the display engine registers at an offset relative to GTTMMADR base. Apart from that, the registers are rather similar to the ones on GMCH platforms.
Allow platforms to specify at which offset within GTTMM the display registers are located. Use zero for all currently-supported platforms.
Change-Id: Id5497a23776e9aa9e2fd5ca0479030cb8e55712f Signed-off-by: Angel Pons th3fanbus@gmail.com --- M common/hw-gfx-gma-config.ads.template M common/hw-gfx-gma-registers.adb 2 files changed, 4 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/libgfxinit refs/changes/86/44786/4
Angel Pons 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 4:
(1 comment)
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;
It all depends on whether registers will be used that exist on other […]
I can't see any. I've added a comment, though.
Angel Pons 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 4:
(1 comment)
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 can't see any. I've added a comment, though.
Oh, looks like the EMGD people already wrote the function we would need:
static bool IS_DISPLAYREG(u32 reg) { /* * This should make it easier to transition modules over to the * new register block scheme, since we can do it incrementally. */ if (reg >= VLV_DISPLAY_BASE) return false; if (reg >= RENDER_RING_BASE && reg < RENDER_RING_BASE + 0xff) return false; if (reg >= GEN6_BSD_RING_BASE && reg < GEN6_BSD_RING_BASE + 0xff) return false; if (reg >= BLT_RING_BASE && reg < BLT_RING_BASE + 0xff) return false; if (reg >= IPEIR_I965 && reg < HWSTAM) return false; if (reg >= GTFIFODBG && reg <= GTFIFODBG+0xff) return false; if (reg >= GEN7_GT_GFX_RC6_LOCKED_TO_RPN && reg <= GEN7_GT_GFX_RC6PP) return false; if (reg == RENDER_RC0_COUNTER) return false; if (reg == MEDIA_RC0_COUNTER) return false; if (reg == MI_MODE) return false; if (reg == GEN7_CXT_SIZE) return false; if (reg == GFX_MODE_GEN7) return false; if (reg == PGTBL_ER) return false; if (reg >= FENCE_REG_SANDYBRIDGE_0 && reg <= GFX_FLSH_CNTL_GEN6) { return false; } if (reg == RENDER_HWS_PGA_GEN7 || reg == BSD_HWS_PGA_GEN7 || reg == BLT_HWS_PGA_GEN7) return false; if (reg == GEN6_BSD_SLEEP_PSMI_CONTROL || reg == GEN6_BSD_RNCID) return false; if (reg == GEN6_BLITTER_ECOSKPD) return false; if (reg >= GEN6_RPNSWREQ && reg < GEN6_PMINTRMSK + 4) return false; if (reg >= 0x4000c && reg <= 0x4002c) return false; if (reg >= 0x4f000 && reg <= 0x4f08f) return false; if (reg >= 0x4f100 && reg <= 0x4f11f) return false; if (reg >= VLV_MASTER_IER && reg <= GEN6_PMIER) return false; if (reg >= FENCE_REG_SANDYBRIDGE_0 && reg < (FENCE_REG_SANDYBRIDGE_0 + (16 * 8))) return false; if (reg == FORCEWAKE_VLV || reg == FORCEWAKE_ACK_VLV || reg == 0x130090) return false; if (reg == GEN6_GDRST) return false; if (reg > 0x9400 && reg <= 0x9418) return false; switch (reg) { case _3D_CHICKEN3: case IVB_CHICKEN3: case GEN7_HALF_SLICE_CHICKEN1: case GEN7_COMMON_SLICE_CHICKEN1: case GEN7_L3CNTLREG1: case GEN7_L3_CHICKEN_MODE_REGISTER: case GEN7_ROW_CHICKEN2: case GEN7_L3SQCREG4: case GEN7_SQ_CHICKEN_MBCUNIT_CONFIG: case GEN6_MBCTL: case GEN6_UCGCTL2: case GEN7_UCGCTL4: case FORCEWAKE_MEDIA_VLV: case FORCEWAKE_ACK_MEDIA_VLV: case VLV_GTLC_PW_STATUS: return false; default: break; } return true; }
Yeah, no. That thing is huge.
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 4:
(1 comment)
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;
Oh, looks like the EMGD people already wrote the function we would need: […]
For now, I would just double check the log against one from before you introduced the offset.
Angel Pons 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 4:
(1 comment)
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;
For now, I would just double check the log against one from before you […]
It works, and I found several bugs regarding handling of DPIO and other stuff, so it's not really comparable. Plus, the register names often do not match, which is annoying.
Hello Nico Huber,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/libgfxinit/+/44786
to look at the new patch set (#5).
Change subject: gma registers: Allow to specify an offset for display registers ......................................................................
gma registers: Allow to specify an offset for display registers
Certain platforms, namely Bay Trail and Braswell, have the display engine registers at an offset relative to GTTMMADR base. Apart from that, the registers are rather similar to the ones on GMCH platforms.
Allow platforms to specify at which offset within GTTMM the display registers are located. Use zero for all currently-supported platforms. Add several registers that will need to be remapped as an example.
Change-Id: Id5497a23776e9aa9e2fd5ca0479030cb8e55712f Signed-off-by: Angel Pons th3fanbus@gmail.com --- M common/hw-gfx-gma-config.ads.template M common/hw-gfx-gma-registers.adb 2 files changed, 55 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/libgfxinit refs/changes/86/44786/5
Angel Pons 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 5:
(1 comment)
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;
It works, and I found several bugs regarding handling of DPIO and other stuff, so it's not really co […]
I've added a list of registers to remap, just in case. Bay Trail still works
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 6: Code-Review+2
Attention is currently required from: Angel Pons. 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 7:
(1 comment)
File common/hw-gfx-gma-registers.adb:
https://review.coreboot.org/c/libgfxinit/+/44786/comment/8d1cfef5_64cf5f78 PS3, Line 214: return Reg'Enum_Rep + Config.Display_Base_Offset / Register_Width;
I've added a list of registers to remap, just in case. […]
Looks like the original comment is addressed.
Attention is currently required from: Angel Pons.
Hello Nico Huber,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/libgfxinit/+/44786?usp=email
to look at the new patch set (#8).
Change subject: gma registers: Allow to specify an offset for display registers ......................................................................
gma registers: Allow to specify an offset for display registers
Certain platforms, namely Bay Trail and Braswell, have the display engine registers at an offset relative to GTTMMADR base. Apart from that, the registers are rather similar to the ones on GMCH platforms.
Allow platforms to specify at which offset within GTTMM the display registers are located. Use zero for all currently-supported platforms. Add several registers that will need to be remapped as an example.
Change-Id: Id5497a23776e9aa9e2fd5ca0479030cb8e55712f Signed-off-by: Angel Pons th3fanbus@gmail.com --- M common/hw-gfx-gma-config.ads.template M common/hw-gfx-gma-registers.adb 2 files changed, 55 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/libgfxinit refs/changes/86/44786/8
-
Angel Pons (Code Review) -
Nico Huber (Code Review)