Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/38827 )
Change subject: soc/mediatek: dsi: Increase pcw precision ......................................................................
soc/mediatek: dsi: Increase pcw precision
When configuring MIPI DSI Tx, the value of pcw was calculated from data rate in MHz, leading to loss of precision. This patch changes to use data rate in Hz for the calculation so that the resulting value should be consistent with the one in kernel (CL:1786327).
In addition, change the type of data rate to u32, and calculation of data rate from pixel clock is changed to use DIV_ROUND_UP for consistency with kernel (CL:1761843).
Also remove unused variable txdiv.
BRANCH=kukui BUG=b:149051882 TEST=emerge-jacuzzi coreboot TEST=No scrolling issue on Juniper AUO and InnoLux panels
Change-Id: I23220d446833b956431006027bbc8cb20fc696a5 Signed-off-by: Yu-Ping Wu yupingso@chromium.org Reviewed-on: https://review.coreboot.org/c/coreboot/+/38827 Reviewed-by: Hung-Te Lin hungte@chromium.org Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/soc/mediatek/common/dsi.c M src/soc/mediatek/common/include/soc/dsi_common.h M src/soc/mediatek/mt8173/dsi.c M src/soc/mediatek/mt8183/dsi.c 4 files changed, 39 insertions(+), 38 deletions(-)
Approvals: build bot (Jenkins): Verified Hung-Te Lin: Looks good to me, approved
diff --git a/src/soc/mediatek/common/dsi.c b/src/soc/mediatek/common/dsi.c index 238b1eb..9222cb0 100644 --- a/src/soc/mediatek/common/dsi.c +++ b/src/soc/mediatek/common/dsi.c @@ -39,29 +39,32 @@ return 24; }
-static int mtk_dsi_get_data_rate(u32 bits_per_pixel, u32 lanes, +static u32 mtk_dsi_get_data_rate(u32 bits_per_pixel, u32 lanes, const struct edid *edid) { /* data_rate = pixel_clock * bits_per_pixel * mipi_ratio / lanes - * Note pixel_clock comes in kHz and returned data_rate is in Mbps. + * Note pixel_clock comes in kHz and returned data_rate is in bps. * mipi_ratio is the clk coefficient to balance the pixel clk in MIPI * for older platforms which do not have complete implementation in HFP. * Newer platforms should just set that to 1.0 (100 / 100). */ - int data_rate = (u64)edid->mode.pixel_clock * bits_per_pixel * - MTK_DSI_MIPI_RATIO_NUMERATOR / - (1000 * lanes * MTK_DSI_MIPI_RATIO_DENOMINATOR); - printk(BIOS_INFO, "DSI data_rate: %d Mbps\n", data_rate); + u32 data_rate = DIV_ROUND_UP((u64)edid->mode.pixel_clock * + bits_per_pixel * 1000 * + MTK_DSI_MIPI_RATIO_NUMERATOR, + (u64)lanes * + MTK_DSI_MIPI_RATIO_DENOMINATOR); + printk(BIOS_INFO, "DSI data_rate: %u bps\n", data_rate);
- if (data_rate < MTK_DSI_DATA_RATE_MIN_MHZ) { - printk(BIOS_ERR, "data rate (%dMbps) must be >=%dMbps. " - "Please check the pixel clock (%u), bits per pixel(%u), " + if (data_rate < MTK_DSI_DATA_RATE_MIN_MHZ * MHz) { + printk(BIOS_ERR, "data rate (%ubps) must be >= %ubps. " + "Please check the pixel clock (%u), " + "bits per pixel (%u), " "mipi_ratio (%d%%) and number of lanes (%d)\n", - data_rate, MTK_DSI_DATA_RATE_MIN_MHZ, + data_rate, MTK_DSI_DATA_RATE_MIN_MHZ * MHz, edid->mode.pixel_clock, bits_per_pixel, (100 * MTK_DSI_MIPI_RATIO_NUMERATOR / MTK_DSI_MIPI_RATIO_DENOMINATOR), lanes); - return -1; + return 0; } return data_rate; } @@ -71,12 +74,13 @@ /* Do nothing. */ }
-static void mtk_dsi_phy_timing(int data_rate, struct mtk_phy_timing *phy_timing) +static void mtk_dsi_phy_timing(u32 data_rate, struct mtk_phy_timing *phy_timing) { u32 cycle_time, ui; + u32 data_rate_mhz = DIV_ROUND_UP(data_rate, MHz);
- ui = 1000 / data_rate + 0x01; - cycle_time = 8000 / data_rate + 0x01; + ui = 1000 / data_rate_mhz + 0x01; + cycle_time = 8000 / data_rate_mhz + 0x01;
memset(phy_timing, 0, sizeof(*phy_timing));
@@ -401,11 +405,11 @@ int mtk_dsi_init(u32 mode_flags, u32 format, u32 lanes, const struct edid *edid, const u8 *init_commands) { - int data_rate; + u32 data_rate; u32 bits_per_pixel = mtk_dsi_get_bits_per_pixel(format);
data_rate = mtk_dsi_get_data_rate(bits_per_pixel, lanes, edid); - if (data_rate < 0) + if (!data_rate) return -1;
mtk_dsi_configure_mipi_tx(data_rate, lanes); diff --git a/src/soc/mediatek/common/include/soc/dsi_common.h b/src/soc/mediatek/common/include/soc/dsi_common.h index 3052689..25727b8 100644 --- a/src/soc/mediatek/common/include/soc/dsi_common.h +++ b/src/soc/mediatek/common/include/soc/dsi_common.h @@ -16,6 +16,7 @@ #ifndef SOC_MEDIATEK_DSI_COMMON_H #define SOC_MEDIATEK_DSI_COMMON_H
+#include <commonlib/helpers.h> #include <edid.h> #include <types.h> #include <soc/addressmap.h> @@ -358,7 +359,7 @@
/* Functions that each SOC should provide. */ void mtk_dsi_reset(void); -void mtk_dsi_configure_mipi_tx(int data_rate, u32 lanes); +void mtk_dsi_configure_mipi_tx(u32 data_rate, u32 lanes);
/* Functions as weak no-ops that can be overridden. */ void mtk_dsi_override_phy_timing(struct mtk_phy_timing *timing); diff --git a/src/soc/mediatek/mt8173/dsi.c b/src/soc/mediatek/mt8173/dsi.c index dae23f5..48bfbef 100644 --- a/src/soc/mediatek/mt8173/dsi.c +++ b/src/soc/mediatek/mt8173/dsi.c @@ -20,7 +20,7 @@ #include <soc/dsi.h> #include <timer.h>
-void mtk_dsi_configure_mipi_tx(int data_rate, u32 lanes) +void mtk_dsi_configure_mipi_tx(u32 data_rate, u32 lanes) { u32 txdiv0, txdiv1; u64 pcw; @@ -51,21 +51,21 @@
clrbits32(&mipi_tx0->dsi_pll_con0, RG_DSI0_MPPLL_PLL_EN);
- if (data_rate > 500) { + if (data_rate > 500 * MHz) { txdiv0 = 0; txdiv1 = 0; - } else if (data_rate >= 250) { + } else if (data_rate >= 250 * MHz) { txdiv0 = 1; txdiv1 = 0; - } else if (data_rate >= 125) { + } else if (data_rate >= 125 * MHz) { txdiv0 = 2; txdiv1 = 0; - } else if (data_rate >= 62) { + } else if (data_rate >= 62 * MHz) { txdiv0 = 2; txdiv1 = 1; } else { /* MIN = 50 */ - assert(data_rate >= MTK_DSI_DATA_RATE_MIN_MHZ); + assert(data_rate >= MTK_DSI_DATA_RATE_MIN_MHZ * MHz); txdiv0 = 2; txdiv1 = 2; } @@ -83,7 +83,7 @@ * Ref_clk is 26MHz */ pcw = (u64)(data_rate * (1 << txdiv0) * (1 << txdiv1)) << 24; - pcw /= 13; + pcw /= 13 * MHz; write32(&mipi_tx0->dsi_pll_con2, pcw);
setbits32(&mipi_tx0->dsi_pll_con1, RG_DSI0_MPPLL_SDM_FRA_EN); diff --git a/src/soc/mediatek/mt8183/dsi.c b/src/soc/mediatek/mt8183/dsi.c index 7f5ac0a..3710fc65 100644 --- a/src/soc/mediatek/mt8183/dsi.c +++ b/src/soc/mediatek/mt8183/dsi.c @@ -19,32 +19,28 @@ #include <soc/dsi.h> #include <soc/pll.h>
-void mtk_dsi_configure_mipi_tx(int data_rate, u32 lanes) +void mtk_dsi_configure_mipi_tx(u32 data_rate, u32 lanes) { - unsigned int txdiv, txdiv0, txdiv1; + unsigned int txdiv0, txdiv1; u64 pcw;
- if (data_rate >= 2000) { - txdiv = 1; + if (data_rate >= 2000 * MHz) { txdiv0 = 0; txdiv1 = 0; - } else if (data_rate >= 1000) { - txdiv = 2; + } else if (data_rate >= 1000 * MHz) { txdiv0 = 1; txdiv1 = 0; - } else if (data_rate >= 500) { - txdiv = 4; + } else if (data_rate >= 500 * MHz) { txdiv0 = 2; txdiv1 = 0; - } else if (data_rate > 250) { - /* Be aware that 250 is a special case that must use txdiv=4. */ - txdiv = 8; + } else if (data_rate > 250 * MHz) { + /* (data_rate == 250MHz) is a special case that should go to the + else-block below (txdiv0 = 4) */ txdiv0 = 3; txdiv1 = 0; } else { /* MIN = 125 */ - assert(data_rate >= MTK_DSI_DATA_RATE_MIN_MHZ); - txdiv = 16; + assert(data_rate >= MTK_DSI_DATA_RATE_MIN_MHZ * MHz); txdiv0 = 4; txdiv1 = 0; } @@ -56,7 +52,7 @@
pcw = (u64)data_rate * (1 << txdiv0) * (1 << txdiv1); pcw <<= 24; - pcw /= CLK26M_HZ / MHz; + pcw /= CLK26M_HZ;
write32(&mipi_tx->pll_con0, pcw); clrsetbits32(&mipi_tx->pll_con1, RG_DSI_PLL_POSDIV, txdiv0 << 8);