Re: [PATCH v3 4/4] drm/msm/dpu: fix video mode DSC INTF timing width calculation

[Neil Armstrong <neil.armstrong@linaro.org>]

On 3/19/26 15:40, Alexander Koskovich wrote:
> On Thursday, March 19th, 2026 at 10:13 AM, Neil Armstrong <neil.armstrong@linaro.org> 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.
>>
>>>
>>> Signed-off-by: Alexander Koskovich <akoskovich@pm.me>
>>> ---
>>>    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);
>>> +
>> Drop this change
>>
>>>    		/*
>>>    		 * 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.

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.

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