Nico Huber has uploaded this change for review. ( https://review.coreboot.org/c/libgfxinit/+/38264 )
Change subject: gma bxt panel: Correct power-cycle delay programming ......................................................................
gma bxt panel: Correct power-cycle delay programming
Change-Id: Iff53ef5730691c3be1ebcda71428e2bfe9ae16fd Signed-off-by: Nico Huber nico.huber@secunet.com --- M common/hw-gfx-gma-config.ads.template M common/hw-gfx-gma-panel.adb 2 files changed, 27 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/libgfxinit refs/changes/64/38264/1
diff --git a/common/hw-gfx-gma-config.ads.template b/common/hw-gfx-gma-config.ads.template index 83dd6a6..01a67c1 100644 --- a/common/hw-gfx-gma-config.ads.template +++ b/common/hw-gfx-gma-config.ads.template @@ -181,6 +181,7 @@ Has_PP_Port_Select : <genbool> := Up_To_Ironlake; Use_PP_VDD_Override : <genbool> := Up_To_Ironlake; Has_PCH_Panel_Power : <genbool> := Ironlake_On; + Has_PP_Divisor_Reg : <genbool> := not Gen_Broxton;
----------- PCH/FDI: --------- Has_PCH : <genbool> := not Gen_Broxton and not Gen_G45; diff --git a/common/hw-gfx-gma-panel.adb b/common/hw-gfx-gma-panel.adb index 359f580..c53fd3f 100644 --- a/common/hw-gfx-gma-panel.adb +++ b/common/hw-gfx-gma-panel.adb @@ -76,6 +76,13 @@ PCH_PP_CONTROL_POWER_DOWN_ON_RESET : constant := 16#00_0001# * 2 ** 1; PCH_PP_CONTROL_TARGET_ON : constant := 16#00_0001# * 2 ** 0;
+ BXT_PP_CONTROL_PWR_CYC_DELAY_SHIFT : constant := 4; + BXT_PP_CONTROL_PWR_CYC_DELAY_MASK : constant := 16#00_001f# * 2 ** 4; + function BXT_PP_CONTROL_PWR_CYC_DELAY (US : Natural) return Word32 is + begin + return Shift_Left (Div_Round_Up32 (US, 100_000) + 1, BXT_PP_CONTROL_PWR_CYC_DELAY_SHIFT); + end BXT_PP_CONTROL_PWR_CYC_DELAY; + PCH_PP_ON_DELAYS_PORT_SELECT_MASK : constant := 16#00_0003# * 2 ** 30; PCH_PP_ON_DELAYS_PORT_SELECT_LVDS : constant := 16#00_0000# * 2 ** 30; PCH_PP_ON_DELAYS_PORT_SELECT_DP_A : constant := 16#00_0001# * 2 ** 30; @@ -206,7 +213,12 @@ Delays_US (BL_Off_To_Power_Down) := 100 * Natural (Power_Delay and PCH_PP_OFF_DELAYS_BL_OFF_PWR_DOWN_MASK);
- Registers.Read (Panel_PP_Regs.DIVISOR, Power_Delay); + if Config.Has_PP_Divisor_Reg then + Registers.Read (Panel_PP_Regs.DIVISOR, Power_Delay); + else + Registers.Read (Panel_PP_Regs.CONTROL, Power_Delay); + Power_Delay := Shift_Right (Power_Delay, BXT_PP_CONTROL_PWR_CYC_DELAY_SHIFT); + end if; if (Power_Delay and PCH_PP_DIVISOR_PWR_CYC_DELAY_MASK) > 1 then Delays_US (Power_Cycle_Delay) := 100_000 * (Natural (Power_Delay and PCH_PP_DIVISOR_PWR_CYC_DELAY_MASK) - 1); @@ -247,11 +259,19 @@ PCH_PP_OFF_DELAYS_BL_OFF_PWR_DOWN (Delays_US (BL_Off_To_Power_Down)));
- Registers.Unset_And_Set_Mask - (Register => Panel_PP_Regs.DIVISOR, - Mask_Unset => PCH_PP_DIVISOR_PWR_CYC_DELAY_MASK, - Mask_Set => PCH_PP_DIVISOR_PWR_CYC_DELAY - (Delays_US (Power_Cycle_Delay))); + if Config.Has_PP_Divisor_Reg then + Registers.Unset_And_Set_Mask + (Register => Panel_PP_Regs.DIVISOR, + Mask_Unset => PCH_PP_DIVISOR_PWR_CYC_DELAY_MASK, + Mask_Set => PCH_PP_DIVISOR_PWR_CYC_DELAY + (Delays_US (Power_Cycle_Delay))); + else + Registers.Unset_And_Set_Mask + (Register => Panel_PP_Regs.CONTROL, + Mask_Unset => BXT_PP_CONTROL_PWR_CYC_DELAY_MASK, + Mask_Set => BXT_PP_CONTROL_PWR_CYC_DELAY + (Delays_US (Power_Cycle_Delay))); + end if; end if;
if Config.Has_PP_Write_Protection then
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/libgfxinit/+/38264 )
Change subject: gma bxt panel: Correct power-cycle delay programming ......................................................................
Patch Set 1: Code-Review+2
(1 comment)
https://review.coreboot.org/c/libgfxinit/+/38264/1/common/hw-gfx-gma-panel.a... File common/hw-gfx-gma-panel.adb:
https://review.coreboot.org/c/libgfxinit/+/38264/1/common/hw-gfx-gma-panel.a... PS1, Line 81: function BXT_PP_CONTROL_PWR_CYC_DELAY (US : Natural) return Word32 is Wouldn't this function look better outside of the register definitions?
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/libgfxinit/+/38264 )
Change subject: gma bxt panel: Correct power-cycle delay programming ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/libgfxinit/+/38264/1/common/hw-gfx-gma-panel.a... File common/hw-gfx-gma-panel.adb:
https://review.coreboot.org/c/libgfxinit/+/38264/1/common/hw-gfx-gma-panel.a... PS1, Line 81: function BXT_PP_CONTROL_PWR_CYC_DELAY (US : Natural) return Word32 is
Wouldn't this function look better outside of the register definitions?
To me, Ada will probably never look better. I wanted it here because it's part of the register description.
However, I realize now that we could also write this as expression function:
function BXT_PP_CONTROL_PWR_CYC_DELAY (US : Natural) return Word32 is (Shift_Left (Div_Round_Up32 (US, 100_000) + 1));
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/libgfxinit/+/38264 )
Change subject: gma bxt panel: Correct power-cycle delay programming ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/libgfxinit/+/38264/1/common/hw-gfx-gma-panel.a... File common/hw-gfx-gma-panel.adb:
https://review.coreboot.org/c/libgfxinit/+/38264/1/common/hw-gfx-gma-panel.a... PS1, Line 81: function BXT_PP_CONTROL_PWR_CYC_DELAY (US : Natural) return Word32 is
To me, Ada will probably never look better. I wanted it here because it's […]
You could also move the whole block at the end of the register descriptions, which should be good enough.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/libgfxinit/+/38264 )
Change subject: gma bxt panel: Correct power-cycle delay programming ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/libgfxinit/+/38264/1/common/hw-gfx-gma-panel.a... File common/hw-gfx-gma-panel.adb:
https://review.coreboot.org/c/libgfxinit/+/38264/1/common/hw-gfx-gma-panel.a... PS1, Line 81: function BXT_PP_CONTROL_PWR_CYC_DELAY (US : Natural) return Word32 is
You could also move the whole block at the end of the register descriptions, which should be good en […]
That would interleave descriptions of different registers...
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/libgfxinit/+/38264 )
Change subject: gma bxt panel: Correct power-cycle delay programming ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/libgfxinit/+/38264/1/common/hw-gfx-gma-panel.a... File common/hw-gfx-gma-panel.adb:
https://review.coreboot.org/c/libgfxinit/+/38264/1/common/hw-gfx-gma-panel.a... PS1, Line 81: function BXT_PP_CONTROL_PWR_CYC_DELAY (US : Natural) return Word32 is
That would interleave descriptions of different registers...
Right. I guess I am used to having a bunch of #define for all registers.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/libgfxinit/+/38264 )
Change subject: gma bxt panel: Correct power-cycle delay programming ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/libgfxinit/+/38264/1/common/hw-gfx-gma-panel.a... File common/hw-gfx-gma-panel.adb:
https://review.coreboot.org/c/libgfxinit/+/38264/1/common/hw-gfx-gma-panel.a... PS1, Line 81: function BXT_PP_CONTROL_PWR_CYC_DELAY (US : Natural) return Word32 is
Right. I guess I am used to having a bunch of #define for all registers.
Actually, I could make it an expression function, would at least save some distracting boilerplate.
Hello Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/libgfxinit/+/38264
to look at the new patch set (#2).
Change subject: gma bxt panel: Correct power-cycle delay programming ......................................................................
gma bxt panel: Correct power-cycle delay programming
Change-Id: Iff53ef5730691c3be1ebcda71428e2bfe9ae16fd Signed-off-by: Nico Huber nico.huber@secunet.com --- M common/hw-gfx-gma-config.ads.template M common/hw-gfx-gma-panel.adb 2 files changed, 25 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/libgfxinit refs/changes/64/38264/2
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/libgfxinit/+/38264 )
Change subject: gma bxt panel: Correct power-cycle delay programming ......................................................................
Patch Set 2: Verified+1
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/libgfxinit/+/38264 )
Change subject: gma bxt panel: Correct power-cycle delay programming ......................................................................
Patch Set 2: Code-Review+2
(1 comment)
https://review.coreboot.org/c/libgfxinit/+/38264/1/common/hw-gfx-gma-panel.a... File common/hw-gfx-gma-panel.adb:
https://review.coreboot.org/c/libgfxinit/+/38264/1/common/hw-gfx-gma-panel.a... PS1, Line 81: function BXT_PP_CONTROL_PWR_CYC_DELAY (US : Natural) return Word32 is
Actually, I could make it an expression function, would at least save some […]
That looks much better, thanks!
Nico Huber has submitted this change. ( https://review.coreboot.org/c/libgfxinit/+/38264 )
Change subject: gma bxt panel: Correct power-cycle delay programming ......................................................................
gma bxt panel: Correct power-cycle delay programming
Change-Id: Iff53ef5730691c3be1ebcda71428e2bfe9ae16fd Signed-off-by: Nico Huber nico.huber@secunet.com Reviewed-on: https://review.coreboot.org/c/libgfxinit/+/38264 Tested-by: Nico Huber nico.h@gmx.de Reviewed-by: Angel Pons th3fanbus@gmail.com --- M common/hw-gfx-gma-config.ads.template M common/hw-gfx-gma-panel.adb 2 files changed, 25 insertions(+), 6 deletions(-)
Approvals: Nico Huber: Verified Angel Pons: Looks good to me, approved
diff --git a/common/hw-gfx-gma-config.ads.template b/common/hw-gfx-gma-config.ads.template index 83dd6a6..01a67c1 100644 --- a/common/hw-gfx-gma-config.ads.template +++ b/common/hw-gfx-gma-config.ads.template @@ -181,6 +181,7 @@ Has_PP_Port_Select : <genbool> := Up_To_Ironlake; Use_PP_VDD_Override : <genbool> := Up_To_Ironlake; Has_PCH_Panel_Power : <genbool> := Ironlake_On; + Has_PP_Divisor_Reg : <genbool> := not Gen_Broxton;
----------- PCH/FDI: --------- Has_PCH : <genbool> := not Gen_Broxton and not Gen_G45; diff --git a/common/hw-gfx-gma-panel.adb b/common/hw-gfx-gma-panel.adb index 359f580..1cd0d55 100644 --- a/common/hw-gfx-gma-panel.adb +++ b/common/hw-gfx-gma-panel.adb @@ -76,6 +76,11 @@ PCH_PP_CONTROL_POWER_DOWN_ON_RESET : constant := 16#00_0001# * 2 ** 1; PCH_PP_CONTROL_TARGET_ON : constant := 16#00_0001# * 2 ** 0;
+ BXT_PP_CONTROL_PWR_CYC_DELAY_SHIFT : constant := 4; + BXT_PP_CONTROL_PWR_CYC_DELAY_MASK : constant := 16#00_001f# * 2 ** 4; + function BXT_PP_CONTROL_PWR_CYC_DELAY (US : Natural) return Word32 is + (Shift_Left (Div_Round_Up32 (US, 100_000) + 1, BXT_PP_CONTROL_PWR_CYC_DELAY_SHIFT)); + PCH_PP_ON_DELAYS_PORT_SELECT_MASK : constant := 16#00_0003# * 2 ** 30; PCH_PP_ON_DELAYS_PORT_SELECT_LVDS : constant := 16#00_0000# * 2 ** 30; PCH_PP_ON_DELAYS_PORT_SELECT_DP_A : constant := 16#00_0001# * 2 ** 30; @@ -206,7 +211,12 @@ Delays_US (BL_Off_To_Power_Down) := 100 * Natural (Power_Delay and PCH_PP_OFF_DELAYS_BL_OFF_PWR_DOWN_MASK);
- Registers.Read (Panel_PP_Regs.DIVISOR, Power_Delay); + if Config.Has_PP_Divisor_Reg then + Registers.Read (Panel_PP_Regs.DIVISOR, Power_Delay); + else + Registers.Read (Panel_PP_Regs.CONTROL, Power_Delay); + Power_Delay := Shift_Right (Power_Delay, BXT_PP_CONTROL_PWR_CYC_DELAY_SHIFT); + end if; if (Power_Delay and PCH_PP_DIVISOR_PWR_CYC_DELAY_MASK) > 1 then Delays_US (Power_Cycle_Delay) := 100_000 * (Natural (Power_Delay and PCH_PP_DIVISOR_PWR_CYC_DELAY_MASK) - 1); @@ -247,11 +257,19 @@ PCH_PP_OFF_DELAYS_BL_OFF_PWR_DOWN (Delays_US (BL_Off_To_Power_Down)));
- Registers.Unset_And_Set_Mask - (Register => Panel_PP_Regs.DIVISOR, - Mask_Unset => PCH_PP_DIVISOR_PWR_CYC_DELAY_MASK, - Mask_Set => PCH_PP_DIVISOR_PWR_CYC_DELAY - (Delays_US (Power_Cycle_Delay))); + if Config.Has_PP_Divisor_Reg then + Registers.Unset_And_Set_Mask + (Register => Panel_PP_Regs.DIVISOR, + Mask_Unset => PCH_PP_DIVISOR_PWR_CYC_DELAY_MASK, + Mask_Set => PCH_PP_DIVISOR_PWR_CYC_DELAY + (Delays_US (Power_Cycle_Delay))); + else + Registers.Unset_And_Set_Mask + (Register => Panel_PP_Regs.CONTROL, + Mask_Unset => BXT_PP_CONTROL_PWR_CYC_DELAY_MASK, + Mask_Set => BXT_PP_CONTROL_PWR_CYC_DELAY + (Delays_US (Power_Cycle_Delay))); + end if; end if;
if Config.Has_PP_Write_Protection then
-
Angel Pons (Code Review) -
Nico Huber (Code Review)