Re: [PATCH v3 4/4] drm/msm/dpu: fix video mode DSC INTF timing width calculation
From: Jonathan Marek
Date: Thu Mar 19 2026 - 13:30:41 EST
On 3/19/26 10:54 AM, Neil Armstrong wrote:
On 3/19/26 15:40, Alexander Koskovich wrote:
On Thursday, March 19th, 2026 at 10:13 AM, Neil Armstrong <neil.armstrong@xxxxxxxxxx> wrote:
Hi,
On 3/19/26 12:58, Alexander Koskovich wrote:
Using bits_per_component * 3 as the divisor for the compressed INTF
timing width produces constant FIFO errors for the BOE BF068MWM-TD0
panel due to bits_per_component being 10 which results in a divisor
of 30 instead of 24.
Regardless of the compression ratio and pixel depth, 24 bits of
compressed data are transferred per pclk, so the divisor should
always be 24.
Not true with widebus, specify why 24 and because DSI widebus is not implemented yet.
DSI widebus is implemented, and always used when available. The adjustment for DSI widebus just doesn't happen in this function.
Drop this change
Signed-off-by: Alexander Koskovich <akoskovich@xxxxx>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
index 0ba777bda253..5419ef0be137 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
@@ -122,19 +122,18 @@ static void drm_mode_to_intf_timing_params(
}
/*
- * for DSI, if compression is enabled, then divide the horizonal active
- * timing parameters by compression ratio. bits of 3 components(R/G/B)
- * is compressed into bits of 1 pixel.
+ * For DSI, if DSC is enabled, 24 bits of compressed data are
+ * transferred per pclk regardless of the source pixel depth.
*/
if (phys_enc->hw_intf->cap->type != INTF_DP && timing->compression_en) {
struct drm_dsc_config *dsc =
dpu_encoder_get_dsc_config(phys_enc->parent);
+
/*
* TODO: replace drm_dsc_get_bpp_int with logic to handle
* fractional part if there is fraction
*/
- timing->width = timing->width * drm_dsc_get_bpp_int(dsc) /
- (dsc->bits_per_component * 3);
+ timing->width = timing->width * drm_dsc_get_bpp_int(dsc) / 24;
It would be helpful to somehow show that 24 is 8 * 3, 8 being the byte width and 3 the compression ratio.
Can you clarify what the 3 represents? My panel should have a 3.75:1 compression
ratio (30/8) so the final divisor of 24 would be wrong for my panel if its the
compression ratio?
So my guess is that while the exact ratio on the DSI lanes is 3.75:1, the ratio
used to calculate the INTF timings is 3, then the DSC encoder probably does the
magic to feed 10bpp into a 3.75:1 ratio over the DSI lanes.
That's not how it works. INTF (which feeds DSI) is after DSC compression.
INTF timings are always in RGB888 (24-bit) units. Ignoring widebus details, the INTF timing should match what is programmed on the DSI side (hdisplay, which is calculated as bytes per line / 3).
(fwiw, the current "timing->width = ..." calculation here blames to me, but what I wrote originally was just "timing->width = timing->width / 3" with a comment about being incomplete.)
In dsi_adjust_pclk_for_compression, the pclk is adjusted to take in account bits_per_component,
so I presume the actual DSI pclk _is_ timing->width * drm_dsc_get_bpp_int(dsc) / (dsc->bits_per_component * 3),
which is your 3.75:1, but the INTF needs to generate timing->width * drm_dsc_get_bpp_int(dsc) / (8 * 3) pixels
to the DSC encoder which will emit timing->width * drm_dsc_get_bpp_int(dsc) / (dsc->bits_per_component * 3) pixels.
The hdisplay calculation in dsi_adjust_pclk_for_compression (which only affects the clock rate) seems to be wrong, and I think Alexander's panel must be running at a 20% lower clock because of it. dsi_timing_setup has the right hdisplay adjustment.
In any case, 24 _is_ 3 * 8, 3 being the DSC compression ratio on the INTF side.
Dmitry, Konrad, could you help confirming this ?
Neil
timing->xres = timing->width;
}
}
Thanks,
Alex