Re: [PATCH RESEND v4 4/4] drm/bridge: nwl-dsi: Correct MIPI DSI horizontal sync timing

[Liu Ying <victor.liu@nxp.com>]

On Wed, Oct 14, 2026 at 09:44:45AM +0200, Esben Haabendal wrote:
> The NWL MIPI Host controller registers specifies the horizontal front
> porch, sync pulse, and back porch in DSI packet payload size in bytes (ie.
> not in pixel clocks).
> 
> The calculation for this is (mostly) described in section 13.6.3.5.1.2 of
> the i.MX 8M Dual/8M QuadLite/8M Quad Applications Processors Reference
> Manual (rev. 3.1). The formula shown there does not take packet header size
> into account though.
> 
> The formula implemented here converts the hfp, hsa, and hbp to DSI packet
> bytes and then subtracts the number of packet header bytes.
> 
> It is worth noting that these values only needs to match approximately,
> according to the reference manual.
> 
> Based on commit in NXP vendor tree
> commit f2a61699749d ("LF-7981-4: drm/bridge: nwl-dsi: Correct MIPI DSI horizontal sync timing")
> 
> Signed-off-by: Esben Haabendal <esben@geanix.com>
> Cc: Robert Chiras <robert.chiras@nxp.com>
> Cc: Oliver F. Brown <oliver.brown@oss.nxp.com>

Worth a Fixes tag?

> ---
>  drivers/gpu/drm/bridge/nwl-dsi.c | 60 ++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 57 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/nwl-dsi.c b/drivers/gpu/drm/bridge/nwl-dsi.c
> index f3f4334136c0..eb604733eea0 100644
> --- a/drivers/gpu/drm/bridge/nwl-dsi.c
> +++ b/drivers/gpu/drm/bridge/nwl-dsi.c
> @@ -22,6 +22,7 @@
>  #include <linux/reset.h>
>  #include <linux/sys_soc.h>
>  #include <linux/time64.h>
> +#include <linux/math64.h>

Sort the header files alphabetically.

>  
>  #include <drm/drm_atomic_state_helper.h>
>  #include <drm/drm_bridge.h>
> @@ -264,6 +265,9 @@ static int nwl_dsi_config_dpi(struct nwl_dsi *dsi)
>  	bool burst_mode;
>  	int hfront_porch, hback_porch, vfront_porch, vback_porch;
>  	int hsync_len, vsync_len;
> +	int hfp, hbp, hsa;
> +	u64 hs_clk_rate, pclk_rate;
> +	int h_blank, pkt_hdr_len, pkt_len;
>  
>  	hfront_porch = dsi->mode.hsync_start - dsi->mode.hdisplay;
>  	hsync_len = dsi->mode.hsync_end - dsi->mode.hsync_start;
> @@ -317,9 +321,59 @@ static int nwl_dsi_config_dpi(struct nwl_dsi *dsi)
>  			      dsi->mode.hdisplay);
>  	}
>  
> -	nwl_dsi_write(dsi, NWL_DSI_HFP, hfront_porch);
> -	nwl_dsi_write(dsi, NWL_DSI_HBP, hback_porch);
> -	nwl_dsi_write(dsi, NWL_DSI_HSA, hsync_len);
> +	hs_clk_rate = dsi->phy_cfg.mipi_dphy.hs_clk_rate;
> +	pclk_rate = (u64)dsi->mode.clock * 1000;
> +	DRM_DEV_DEBUG_DRIVER(dsi->dev, "hs_clk_rate: %llu Hz\n", hs_clk_rate);
> +
> +	/*
> +	 * Calculate the bytes needed, according to the RM formula:
> +	 * Time of DPI event = time to transmit x number of bytes on the DSI interface
> +	 * dpi_event_size * dpi_pclk_period = dsi_bytes * 8 * hs_bit_period / num_lanes
> +	 * ===>
> +	 * dsi_bytes = dpi_event_size * dpi_pclk_period * num_lanes / (8 * hs_bit_period)
> +	 * ===> (substituting period = 1 / rate)
> +	 * dsi_bytes = dpi_event_size * num_lanes * hs_clk_rate / (8 * dpi_pclk_rate)
> +	 */
> +	hfp = DIV64_U64_ROUND_UP((u64)hfront_porch * dsi->lanes * hs_clk_rate, 8 * pclk_rate);
> +	hbp = DIV64_U64_ROUND_UP((u64)hback_porch * dsi->lanes * hs_clk_rate, 8 * pclk_rate);
> +	hsa = DIV64_U64_ROUND_UP((u64)hsync_len * dsi->lanes * hs_clk_rate, 8 * pclk_rate);
> +
> +	/* Make sure horizontal blankins are even numbers */

s/blankins/blankings/

> +	hfp = roundup(hfp, 2);
> +	hbp = roundup(hbp, 2);
> +	hsa = roundup(hsa, 2);
> +
> +	/*
> +	 * We need to subtract the packet header length: 32
> +	 * In order to make sure we don't get negative values,
> +	 * subtract a proportional value to the total length of the
> +	 * horizontal blanking duration.
> +	 */
> +	h_blank = hfp + hbp + hsa;
> +
> +	if (h_blank) {
> +		pkt_len = min(hfp, roundup(((hfp * 100 / h_blank) * 32) / 100, 2));
> +		pkt_hdr_len = pkt_len;
> +		hfp -= pkt_len;
> +
> +		pkt_len = min(hbp, roundup(((hbp * 100 / h_blank) * 32) / 100, 2));
> +		pkt_hdr_len += pkt_len;
> +		hbp -= pkt_len;
> +
> +		pkt_len = min(hsa, 32 - pkt_hdr_len);
> +		pkt_hdr_len += pkt_len;
> +		hsa -= pkt_len;
> +		if (pkt_hdr_len != 32)
> +			dev_warn(dsi->dev,
> +				 "Unable to subtract packet header length fully: %d left\n",
> +				 32 - pkt_hdr_len);
> +	}
> +
> +	DRM_DEV_DEBUG_DRIVER(dsi->dev, "Register values: hfp=%d hbp=%d hsa=%d\n", hfp, hbp, hsa);
> +
> +	nwl_dsi_write(dsi, NWL_DSI_HFP, hfp);
> +	nwl_dsi_write(dsi, NWL_DSI_HBP, hbp);
> +	nwl_dsi_write(dsi, NWL_DSI_HSA, hsa);
>  
>  	nwl_dsi_write(dsi, NWL_DSI_ENABLE_MULT_PKTS, 0x0);
>  	nwl_dsi_write(dsi, NWL_DSI_BLLP_MODE, 0x1);
> 
> -- 
> 2.56.0
> 

Not sure if the fix is 100% correct, but it is a correct direction to
change the blanking packet payload size from in pixel clocks to in bytes,
so better than no fix:
Acked-by: Liu Ying <victor.liu@nxp.com>

-- 
Regards,
Liu Ying