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

[Alexander Koskovich <akoskovich@pm.me>]

On Friday, March 20th, 2026 at 2:50 AM, Alexander Koskovich <akoskovich@pm.me> wrote:

> On Friday, March 20th, 2026 at 2:38 AM, Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> wrote:
> 
> > On Fri, Mar 20, 2026 at 04:46:02AM +0000, Alexander Koskovich wrote:
> > > On Friday, March 20th, 2026 at 12:25 AM, Jonathan Marek <jonathan@marek.ca> wrote:
> > >
> > > > On 3/19/26 9:45 PM, Dmitry Baryshkov wrote:
> > > > > On Thu, Mar 19, 2026 at 01:23:03PM -0400, Jonathan Marek wrote:
> > > > ...
> > > > >>
> > > > >> 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.)
> > > > >>
> > > > > Okay. After reading the docs (sorry, it took a while).
> > > > >
> > > > > - When widebus is not enabled, the transfer is always 24 bit of
> > > > >    compressed data. Thus if it is not in play, pclk and timing->width
> > > > >    should be scaled by source_pixel_depth / compression_ratio / 24. In
> > > > >    case of the code it is 'drm_dsc_get_bpp_int(dsc) / 24'.
> > > > >
> > > > >    For RGB101010 / 8bpp DSC this should result in the PCLK being lowered
> > > > >    by the factor of 3 (= 24 / (30 / 3.75))
> > > > >
> > > > > - When widebus is in play (MDSS 6.x+, DSI 2.4+), the transfer takes
> > > > >    more than 24 bits. In this case the PCLK and timing->width should be
> > > > >    scaled exactly by the DSC compression ratio, which is
> > > > >    'drm_dsc_get_bpp_int(dsc) / (3 * dsc->bits_per_component).
> > > > >
> > > > > So, this piece of code needs to be adjusted to check for the widebus
> > > > > being enabled or not.
> > > > >
> > > >
> > > > The widebus adjustment on the MDP/INTF side is already in
> > > > dpu_hw_intf_setup_timing_engine: the "data width" is divided by 2 for
> > > > 48-bit widebus instead of 24-bit. there shouldn't be any other
> > > > adjustment (downstream doesn't have any other adjustment).
> > > >
> > > > a relevant downstream comment: "In DATABUS-WIDEN mode, MDP always sends
> > > > out 48-bit compressed data per pclk and on average, DSI consumes an
> > > > amount of compressed data equivalent to the uncompressed pixel depth per
> > > > pclk."
> > > >
> > > > Based on that comment, this patch is correct, and the
> > > > ''drm_dsc_get_bpp_int(dsc) / (3 * dsc->bits_per_component)' adjustment
> > > > only applies to DSI.
> > >
> > > If I keep the INTF side at /24 and change the DSI side to:
> > >
> > > if (wide_bus)
> > >         new_hdisplay = DIV_ROUND_UP(mode->hdisplay * drm_dsc_get_bpp_int(dsc), dsc->bits_per_component * 3);
> > > else
> > >         new_hdisplay = DIV_ROUND_UP(mode->hdisplay * drm_dsc_get_bpp_int(dsc), 24);
> >
> > Please check the actual fps (I'm usually using a vblank-synced GL, e.g.
> > kmscube).
> >
> > At least this matches my expectations.
> 
> Hmmm with kmscube I am getting 120FPS with 24 and 100FPS with 30. So I guess that's a no go

Although it was using dsc->bits_per_component * 3 regardless before for
dsi_adjust_pclk_for_compression so I guess that's what Jonathan was
referring to earlier...

> 
> >
> > >
> > > This also works on my panel.
> > >
> > > Should I send this in a v4 for this series or just leave it for a seperate
> > > change as panel seems to work with /24 here anyways?
> > >
> > > > (note: newer downstream looks like it would divide
> > > > by 3.75 here, which doesn't make sense. older downstream would divide by
> > > > 3 here. I guess downstream is broken now and video mode + 10-bit dsc
> > > > doesn't get tested?)
> > > >
> > > > on DSI side, "uncompressed pixel depth" shouldn't matter either: DSI
> > > > only sees the compressed data. But based on that comment, when widebus
> > > > is enabled, by setting DSI_VID_CFG0_DST_FORMAT(?) to 30bpp, then the DSI
> > > > pclk is in 30-bit units instead of 24-bits. And with this series DSI
> > > > side ends up with the right result if 30bpp format and widebus is enabled.
> > > >
> > > >
> > >
> > > Thanks,
> > > Alex
> >
> > --
> > With best wishes
> > Dmitry
> >
> 
>