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

[Alexander Koskovich]

On Thursday, March 19th, 2026 at 1:23 PM, Jonathan Marek <jonathan@xxxxxxxx> wrote:

> 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.
> 
> >>>>
> >>>> 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);
> >>>> +
> >>> 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.

Checked against downstream and the clocks seem to match more or less:

downstream:
pclk: 110070156
byte: 103190771

upstream:
pclk: 110073457
byte: 103193865


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