Attention is currently required from: Dinesh Gehlot, Tim Wawrzynczak.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/libgfxinit/+/82139?usp=email )
The change is no longer submittable: All-Comments-Resolved is unsatisfied now.
Change subject: gma tgl: Add combo PHY programming sequence ......................................................................
Patch Set 1: Code-Review+1
(3 comments)
File common/tigerlake/hw-gfx-gma-combo_phy.adb:
https://review.coreboot.org/c/libgfxinit/+/82139/comment/29d4cde4_cceff9f1 : PS1, Line 45: PORT_COMP_DW9 : Registers.Registers_Index; : PORT_COMP_DW10 : Registers.Registers_Index; nit: alignment
https://review.coreboot.org/c/libgfxinit/+/82139/comment/5fb26dd9_6265ca01 : PS1, Line 94: DW1 : Word32; I would suggest defining an auxiliary procedure:
``` procedure Propagate_To_Group (Lane0_Register : Registers_Index; Mask_Unset : Word32; Mask_Set : Word32; Group_Register : Registers_Index) is Value : Word32; begin Registers.Read (Register, Value); Value := (Value and not Mask_Unset) or Mask_Set; Registers.Write (Register, Value); end Propagate_To_Group; ``` Which would be used as follows:
``` Propagate_To_Group (Lane0_Register => Phy_Regs (Phy).PORT_TX_DW8_LN0, Mask_Unset => PORT_TX_DW8_ODCC_DIV_SEL_MASK, Mask_Set => PORT_TX_DW8_ODCC_CLKSEL or ICL_PORT_TX_DW8_ODCC_CLK_DIV_SEL_DIV2, Group_Register => Phy_Regs (Phy).PORT_TX_DW8_GRP);
Propagate_To_Group (Lane0_Register => Phy_Regs (Phy).PORT_PCS_DW1_LN0, Mask_Unset => PORT_PCS_DW1_DCC_MODE_SELECT_MASK, Mask_Set => PORT_PCS_DW1_DCC_MODE_SELECT_CONTINUOUS, Group_Register => Phy_Regs (Phy).PORT_PCS_DW1_GRP); ```
https://review.coreboot.org/c/libgfxinit/+/82139/comment/864b1718_04cb1620 : PS1, Line 119: DOT0_VOLT_0_85 : constant Procmon_References := : (DW1 => 16#0000_0000#, : DW9 => 16#62ab_67bb#, : DW10 => 16#5191_4f96#); : DOT0_VOLT_0_95 : constant Procmon_References := : (DW1 => 16#0000_0000#, : DW9 => 16#86e1_72c7#, : DW10 => 16#77ca_5eab#); : DOT0_VOLT_1_05 : constant Procmon_References := : (DW1 => 16#0000_0000#, : DW9 => 16#98fa_82dd#, : DW10 => 16#89e4_6dc1#); : DOT1_VOLT_0_95 : constant Procmon_References := : (DW1 => 16#0000_0000#, : DW9 => 16#93f8_7fe1#, : DW10 => 16#8ae8_71c5#); : DOT1_VOLT_1_05 : constant Procmon_References := : (DW1 => 16#0044_0000#, : DW9 => 16#9a00_ab25#, : DW10 => 16#8ae3_8ff1#); I would put this in a 2D array, but it doesn't really improve things that much