build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39613 )
Change subject: sc7180: Add display 10nm phy & pll programming support [Patch 1 of 3] ......................................................................
Patch Set 1:
(64 comments)
https://review.coreboot.org/c/coreboot/+/39613/1/src/soc/qualcomm/sc7180/dis... File src/soc/qualcomm/sc7180/display/dsi_phy.c:
https://review.coreboot.org/c/coreboot/+/39613/1/src/soc/qualcomm/sc7180/dis... PS1, Line 80: uint32_t pemph_bottom; /* Determines how many pre-emphasis branches for bottom termination */ line over 96 characters
https://review.coreboot.org/c/coreboot/+/39613/1/src/soc/qualcomm/sc7180/dis... PS1, Line 81: uint32_t pemph_top; /* Determines how many pre-emphasis branches for top termination */ line over 96 characters
https://review.coreboot.org/c/coreboot/+/39613/1/src/soc/qualcomm/sc7180/dis... PS1, Line 82: bool pemph_enable; /* Enable driver pre-emphasis */ Statements should start on a tabstop
https://review.coreboot.org/c/coreboot/+/39613/1/src/soc/qualcomm/sc7180/dis... PS1, Line 83: bool strength_override; /* Strength override to use DSIPHY_LNn_TEST_STR */ Statements should start on a tabstop
https://review.coreboot.org/c/coreboot/+/39613/1/src/soc/qualcomm/sc7180/dis... PS1, Line 124: uint32_t desired_bitclk_freq; /* desired bit clock frequency in Hz */ line over 96 characters
https://review.coreboot.org/c/coreboot/+/39613/1/src/soc/qualcomm/sc7180/dis... PS1, Line 129: bool dcdc_mode; /* Regulator mode, TRUE=DCDC ,FALSE=LDO */ line over 96 characters
https://review.coreboot.org/c/coreboot/+/39613/1/src/soc/qualcomm/sc7180/dis... PS1, Line 148: uint32_t pclk_divdenominator; /* denominator of PClk divider ratio */ line over 96 characters
https://review.coreboot.org/c/coreboot/+/39613/1/src/soc/qualcomm/sc7180/dis... PS1, Line 179: dsi_phy_pllconfigtype *pll_cfg, code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/39613/1/src/soc/qualcomm/sc7180/dis... PS1, Line 350: dsi_phy_laneconfig_type lane_cfg, code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/39613/1/src/soc/qualcomm/sc7180/dis... PS1, Line 451: writel((uint32_t)mipi->mdss_dsi_phy_db->timing[1], offset + DSI_0_PHY_CMN_TIMING_CTRL_1_ADDR); line over 96 characters
https://review.coreboot.org/c/coreboot/+/39613/1/src/soc/qualcomm/sc7180/dis... PS1, Line 454: writel((uint32_t)mipi->mdss_dsi_phy_db->timing[2], offset + DSI_0_PHY_CMN_TIMING_CTRL_2_ADDR); line over 96 characters
https://review.coreboot.org/c/coreboot/+/39613/1/src/soc/qualcomm/sc7180/dis... PS1, Line 457: writel((uint32_t)mipi->mdss_dsi_phy_db->timing[3], offset + DSI_0_PHY_CMN_TIMING_CTRL_3_ADDR); line over 96 characters
https://review.coreboot.org/c/coreboot/+/39613/1/src/soc/qualcomm/sc7180/dis... PS1, Line 460: writel((uint32_t)mipi->mdss_dsi_phy_db->timing[4], offset + DSI_0_PHY_CMN_TIMING_CTRL_4_ADDR); line over 96 characters
https://review.coreboot.org/c/coreboot/+/39613/1/src/soc/qualcomm/sc7180/dis... PS1, Line 463: writel((uint32_t)mipi->mdss_dsi_phy_db->timing[5], offset + DSI_0_PHY_CMN_TIMING_CTRL_5_ADDR); line over 96 characters
https://review.coreboot.org/c/coreboot/+/39613/1/src/soc/qualcomm/sc7180/dis... PS1, Line 466: writel((uint32_t)mipi->mdss_dsi_phy_db->timing[6], offset + DSI_0_PHY_CMN_TIMING_CTRL_6_ADDR); line over 96 characters
https://review.coreboot.org/c/coreboot/+/39613/1/src/soc/qualcomm/sc7180/dis... PS1, Line 469: writel((uint32_t)mipi->mdss_dsi_phy_db->timing[7], offset + DSI_0_PHY_CMN_TIMING_CTRL_7_ADDR); line over 96 characters
https://review.coreboot.org/c/coreboot/+/39613/1/src/soc/qualcomm/sc7180/dis... PS1, Line 472: writel((uint32_t)mipi->mdss_dsi_phy_db->timing[8], offset + DSI_0_PHY_CMN_TIMING_CTRL_8_ADDR); line over 96 characters
https://review.coreboot.org/c/coreboot/+/39613/1/src/soc/qualcomm/sc7180/dis... PS1, Line 475: writel((uint32_t)mipi->mdss_dsi_phy_db->timing[9], offset + DSI_0_PHY_CMN_TIMING_CTRL_9_ADDR); line over 96 characters
https://review.coreboot.org/c/coreboot/+/39613/1/src/soc/qualcomm/sc7180/dis... PS1, Line 478: writel((uint32_t)mipi->mdss_dsi_phy_db->timing[10], offset + DSI_0_PHY_CMN_TIMING_CTRL_10_ADDR); line over 96 characters
https://review.coreboot.org/c/coreboot/+/39613/1/src/soc/qualcomm/sc7180/dis... PS1, Line 637: pll_status = readl(offset + DSI_0_PHY_PLL_COMMON_STATUS_ONE_ADDR)& 0x1; need consistent spacing around '&' (ctx:VxW)
https://review.coreboot.org/c/coreboot/+/39613/1/src/soc/qualcomm/sc7180/dis... PS1, Line 909: else if ((phy_cfg->dev_id != DSI_DeviceID_0) && else should follow close brace '}'
https://review.coreboot.org/c/coreboot/+/39613/1/src/soc/qualcomm/sc7180/dis... PS1, Line 914: else if ((phy_cfg->bits_per_pixel != 16) && else should follow close brace '}'
https://review.coreboot.org/c/coreboot/+/39613/1/src/soc/qualcomm/sc7180/dis... PS1, Line 922: else if ((phy_cfg->num_data_lanes == 0) || else should follow close brace '}'
https://review.coreboot.org/c/coreboot/+/39613/1/src/soc/qualcomm/sc7180/dis... PS1, Line 934: printk(BIOS_INFO, "calc vco rate with desired_bitclk_freq: %u\n",phy_cfg_info->bitclk_freq); line over 96 characters
https://review.coreboot.org/c/coreboot/+/39613/1/src/soc/qualcomm/sc7180/dis... PS1, Line 934: printk(BIOS_INFO, "calc vco rate with desired_bitclk_freq: %u\n",phy_cfg_info->bitclk_freq); space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39613/1/src/soc/qualcomm/sc7180/dis... PS1, Line 1000: else { else should follow close brace '}'
https://review.coreboot.org/c/coreboot/+/39613/1/src/soc/qualcomm/sc7180/dis... PS1, Line 1080: if(clk_list != NULL){ space required before the open brace '{'
https://review.coreboot.org/c/coreboot/+/39613/1/src/soc/qualcomm/sc7180/dis... PS1, Line 1080: if(clk_list != NULL){ space required before the open parenthesis '('
https://review.coreboot.org/c/coreboot/+/39613/1/src/soc/qualcomm/sc7180/dis... PS1, Line 1093: printk(BIOS_ERR,"mdss_clock_configure fialed for %s\n",clk_list[i].szName); line over 96 characters
https://review.coreboot.org/c/coreboot/+/39613/1/src/soc/qualcomm/sc7180/dis... PS1, Line 1093: printk(BIOS_ERR,"mdss_clock_configure fialed for %s\n",clk_list[i].szName); space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39613/1/src/soc/qualcomm/sc7180/dis... PS1, Line 1093: printk(BIOS_ERR,"mdss_clock_configure fialed for %s\n",clk_list[i].szName); space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39613/1/src/soc/qualcomm/sc7180/dis... PS1, Line 1100: printk(BIOS_ERR,"mdss_clock_enable fialed for %s\n",clk_list[i].szName); line over 96 characters
https://review.coreboot.org/c/coreboot/+/39613/1/src/soc/qualcomm/sc7180/dis... PS1, Line 1100: printk(BIOS_ERR,"mdss_clock_enable fialed for %s\n",clk_list[i].szName); space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39613/1/src/soc/qualcomm/sc7180/dis... PS1, Line 1100: printk(BIOS_ERR,"mdss_clock_enable fialed for %s\n",clk_list[i].szName); space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39613/1/src/soc/qualcomm/sc7180/dis... File src/soc/qualcomm/sc7180/display/dsi_phy_pll.c:
https://review.coreboot.org/c/coreboot/+/39613/1/src/soc/qualcomm/sc7180/dis... PS1, Line 382: MDSS_PLL_REG_W(pll_base, PLL_DECIMAL_DIV_START_1,reg->decimal_div_start); space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39613/1/src/soc/qualcomm/sc7180/dis... PS1, Line 383: MDSS_PLL_REG_W(pll_base, PLL_FRAC_DIV_START_LOW_1,reg->frac_div_start_low); space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39613/1/src/soc/qualcomm/sc7180/dis... PS1, Line 384: MDSS_PLL_REG_W(pll_base, PLL_FRAC_DIV_START_MID_1,reg->frac_div_start_mid); space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39613/1/src/soc/qualcomm/sc7180/dis... PS1, Line 385: MDSS_PLL_REG_W(pll_base, PLL_FRAC_DIV_START_HIGH_1,reg->frac_div_start_high); space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39613/1/src/soc/qualcomm/sc7180/dis... PS1, Line 426: MDSS_PLL_REG_W(pll_base, PLL_SSC_STEPSIZE_LOW_1,regs->ssc_stepsize_low); space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39613/1/src/soc/qualcomm/sc7180/dis... PS1, Line 427: MDSS_PLL_REG_W(pll_base, PLL_SSC_STEPSIZE_HIGH_1,regs->ssc_stepsize_high); space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39613/1/src/soc/qualcomm/sc7180/dis... PS1, Line 428: MDSS_PLL_REG_W(pll_base, PLL_SSC_DIV_PER_LOW_1,regs->ssc_div_per_low); space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39613/1/src/soc/qualcomm/sc7180/dis... PS1, Line 429: MDSS_PLL_REG_W(pll_base, PLL_SSC_DIV_PER_HIGH_1,regs->ssc_div_per_high); space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39613/1/src/soc/qualcomm/sc7180/dis... PS1, Line 430: MDSS_PLL_REG_W(pll_base, PLL_SSC_DIV_ADJPER_LOW_1,regs->ssc_adjper_low); space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39613/1/src/soc/qualcomm/sc7180/dis... PS1, Line 431: MDSS_PLL_REG_W(pll_base, PLL_SSC_DIV_ADJPER_HIGH_1,regs->ssc_adjper_high); space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39613/1/src/soc/qualcomm/sc7180/dis... PS1, Line 432: MDSS_PLL_REG_W(pll_base, PLL_SSC_CONTROL,SSC_EN | regs->ssc_control); space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39613/1/src/soc/qualcomm/sc7180/inc... File src/soc/qualcomm/sc7180/include/soc/display/display_resources.h:
https://review.coreboot.org/c/coreboot/+/39613/1/src/soc/qualcomm/sc7180/inc... PS1, Line 44: {byte0_intf_clk_name,1, 0, 2, 0, 0, 0, 2}, space required after that ',' (ctx:VxV)
https://review.coreboot.org/c/coreboot/+/39613/1/src/soc/qualcomm/sc7180/inc... File src/soc/qualcomm/sc7180/include/soc/display/dsi_phy_pll.h:
https://review.coreboot.org/c/coreboot/+/39613/1/src/soc/qualcomm/sc7180/inc... PS1, Line 28: writel_relaxed(((base) + (offset)), \ code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/39613/1/src/soc/qualcomm/sc7180/inc... File src/soc/qualcomm/sc7180/include/soc/display/panel.h:
https://review.coreboot.org/c/coreboot/+/39613/1/src/soc/qualcomm/sc7180/inc... PS1, Line 47: uint32_t panel_clockrate; /* panel_clockrate is deprecated in favor of panel_bitclock_freq */ line over 96 characters
https://review.coreboot.org/c/coreboot/+/39613/1/src/soc/qualcomm/sc7180/inc... File src/soc/qualcomm/sc7180/include/soc/mdss_6_2_0.h:
https://review.coreboot.org/c/coreboot/+/39613/1/src/soc/qualcomm/sc7180/inc... PS1, Line 33: #define HWIO_OUT_FLD(regVal, io, field, fldVal) (uint32_t)((((uint32_t)(fldVal) << HWIO_##io##_##field##_SHFT) & HWIO_##io##_##field##_BMSK) | ((uint32_t)(regVal) & ~ HWIO_##io##_##field##_BMSK)) line over 96 characters
https://review.coreboot.org/c/coreboot/+/39613/1/src/soc/qualcomm/sc7180/inc... PS1, Line 33: #define HWIO_OUT_FLD(regVal, io, field, fldVal) (uint32_t)((((uint32_t)(fldVal) << HWIO_##io##_##field##_SHFT) & HWIO_##io##_##field##_BMSK) | ((uint32_t)(regVal) & ~ HWIO_##io##_##field##_BMSK)) space prohibited after that '~' (ctx:WxW)
https://review.coreboot.org/c/coreboot/+/39613/1/src/soc/qualcomm/sc7180/inc... PS1, Line 459: #define CDM_HDMI_PACK_OP_MODE REG_MDP (0x7A400) space prohibited between function name and open parenthesis '('
https://review.coreboot.org/c/coreboot/+/39613/1/src/soc/qualcomm/sc7180/inc... PS1, Line 459: #define CDM_HDMI_PACK_OP_MODE REG_MDP (0x7A400) Macros with complex values should be enclosed in parentheses
https://review.coreboot.org/c/coreboot/+/39613/1/src/soc/qualcomm/sc7180/inc... PS1, Line 460: #define MDP_OUT_CTL_0 REG_MDP (0x01410) space prohibited between function name and open parenthesis '('
https://review.coreboot.org/c/coreboot/+/39613/1/src/soc/qualcomm/sc7180/inc... PS1, Line 460: #define MDP_OUT_CTL_0 REG_MDP (0x01410) Macros with complex values should be enclosed in parentheses
https://review.coreboot.org/c/coreboot/+/39613/1/src/soc/qualcomm/sc7180/inc... PS1, Line 461: #define MDP_INTF_3_INTF_CONFIG REG_MDP (0x6C804) space prohibited between function name and open parenthesis '('
https://review.coreboot.org/c/coreboot/+/39613/1/src/soc/qualcomm/sc7180/inc... PS1, Line 461: #define MDP_INTF_3_INTF_CONFIG REG_MDP (0x6C804) Macros with complex values should be enclosed in parentheses
https://review.coreboot.org/c/coreboot/+/39613/1/src/soc/qualcomm/sc7180/inc... PS1, Line 462: #define CDM_CDWN2_OUT_SIZE REG_MDP (0x7A330) space prohibited between function name and open parenthesis '('
https://review.coreboot.org/c/coreboot/+/39613/1/src/soc/qualcomm/sc7180/inc... PS1, Line 462: #define CDM_CDWN2_OUT_SIZE REG_MDP (0x7A330) Macros with complex values should be enclosed in parentheses
https://review.coreboot.org/c/coreboot/+/39613/1/src/soc/qualcomm/sc7180/inc... PS1, Line 463: #define CDM_CDWN2_OP_MODE REG_MDP (0x7A300) space prohibited between function name and open parenthesis '('
https://review.coreboot.org/c/coreboot/+/39613/1/src/soc/qualcomm/sc7180/inc... PS1, Line 463: #define CDM_CDWN2_OP_MODE REG_MDP (0x7A300) Macros with complex values should be enclosed in parentheses
https://review.coreboot.org/c/coreboot/+/39613/1/src/soc/qualcomm/sc7180/inc... PS1, Line 464: #define CDM_CDWN2_CLAMP_OUT REG_MDP (0x7A304) space prohibited between function name and open parenthesis '('
https://review.coreboot.org/c/coreboot/+/39613/1/src/soc/qualcomm/sc7180/inc... PS1, Line 464: #define CDM_CDWN2_CLAMP_OUT REG_MDP (0x7A304) Macros with complex values should be enclosed in parentheses
https://review.coreboot.org/c/coreboot/+/39613/1/src/soc/qualcomm/sc7180/inc... PS1, Line 465: #define CDM_CSC_10_OP_MODE REG_MDP (0x7A200) space prohibited between function name and open parenthesis '('
https://review.coreboot.org/c/coreboot/+/39613/1/src/soc/qualcomm/sc7180/inc... PS1, Line 465: #define CDM_CSC_10_OP_MODE REG_MDP (0x7A200) Macros with complex values should be enclosed in parentheses