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

[Jonathan Marek <jonathan@marek.ca>]

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@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.
>>>

DSI widebus is implemented, and always used when available. The 
adjustment for DSI widebus just doesn't happen in this function.

>>>>
>>>> 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.
> 

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
>