Due to integer calculations, the rounding off can cause errors in the final value propagated in the registers. Considering the example of 1080p (very common resolution), the mode->clock is 148500, dsi->lanes = 4, and bpp = 24, with the previous logic, the DSI clock frequency would come as 444 when we are expecting the value 445.5 which would reflect in SN_DSIA_CLK_FREQ_REG. So move the division to be the last operation where rounding off will not impact the register value.
Also according to the SN65DSI86 datasheet[0], the minimum value for that reg is 0x08 (inclusive) and the maximum value is 0x97 (exclusive). So add check for that. [0]: <https://www.ti.com/lit/gpn/sn65dsi86> Fixes: ca1b885cbe9e ("drm/bridge: ti-sn65dsi86: Split the setting of the dp and dsi rates") Signed-off-by: Jayesh Choudhary <j-choudh...@ti.com> --- drivers/gpu/drm/bridge/ti-sn65dsi86.c | 48 +++++++++++++++++++++------ 1 file changed, 37 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c index 84698a0b27a8..f9cf6b14d85e 100644 --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c @@ -111,7 +111,14 @@ #define AUX_IRQ_STATUS_AUX_SHORT BIT(5) #define AUX_IRQ_STATUS_NAT_I2C_FAIL BIT(6) -#define MIN_DSI_CLK_FREQ_MHZ 40 +/* + * NOTE: DSI clock frequency range: [40MHz,755MHz) + * DSI clock frequency range is in 5-MHz increments + * So minimum frequency 40MHz translates to 0x08 + * And maximum frequency 755MHz translates to 0x97 + */ +#define MIN_DSI_CLK_RANGE 0x8 +#define MAX_DSI_CLK_RANGE 0x97 /* fudge factor required to account for 8b/10b encoding */ #define DP_CLK_FUDGE_NUM 10 @@ -820,22 +827,37 @@ static void ti_sn_bridge_atomic_disable(struct drm_bridge *bridge, regmap_update_bits(pdata->regmap, SN_ENH_FRAME_REG, VSTREAM_ENABLE, 0); } -static void ti_sn_bridge_set_dsi_rate(struct ti_sn65dsi86 *pdata) +static int ti_sn_bridge_set_dsi_rate(struct ti_sn65dsi86 *pdata) { - unsigned int bit_rate_mhz, clk_freq_mhz; + unsigned int bit_rate_khz; unsigned int val; struct drm_display_mode *mode = &pdata->bridge.encoder->crtc->state->adjusted_mode; - /* set DSIA clk frequency */ - bit_rate_mhz = (mode->clock / 1000) * - mipi_dsi_pixel_format_to_bpp(pdata->dsi->format); - clk_freq_mhz = bit_rate_mhz / (pdata->dsi->lanes * 2); + /* + * Set DSIA clk frequency + * Maximum supported value of bit_rate_khz turns out to be + * 6040000 which can be put in 32-bit variable so no overflow + * possible in this calculation. + */ + bit_rate_khz = mode->clock * + mipi_dsi_pixel_format_to_bpp(pdata->dsi->format); + + /* + * For each increment in val, frequency increases by 5MHz + * and the factor of 1000 comes from kHz to MHz conversion + */ + val = (bit_rate_khz / (pdata->dsi->lanes * 2 * 1000 * 5)) & 0xFF; + + if (val >= MAX_DSI_CLK_RANGE || val < MIN_DSI_CLK_RANGE) { + drm_err(pdata->bridge.dev, + "DSI clock frequency not in the supported range\n"); + return -EINVAL; + } - /* for each increment in val, frequency increases by 5MHz */ - val = (MIN_DSI_CLK_FREQ_MHZ / 5) + - (((clk_freq_mhz - MIN_DSI_CLK_FREQ_MHZ) / 5) & 0xFF); regmap_write(pdata->regmap, SN_DSIA_CLK_FREQ_REG, val); + + return 0; } static unsigned int ti_sn_bridge_get_bpp(struct drm_connector *connector) @@ -1104,7 +1126,11 @@ static void ti_sn_bridge_atomic_enable(struct drm_bridge *bridge, pdata->ln_polrs << LN_POLRS_OFFSET); /* set dsi clk frequency value */ - ti_sn_bridge_set_dsi_rate(pdata); + ret = ti_sn_bridge_set_dsi_rate(pdata); + if (ret) { + DRM_DEV_ERROR(pdata->dev, "Failed to set dsi rate :%d\n", ret); + return; + } /* * The SN65DSI86 only supports ASSR Display Authentication method and -- 2.25.1
