On Fri, Mar 20, 2026 at 4:17 PM Alexander Koskovich <[email protected]> wrote:
> >
> > On Friday, March 20th, 2026 at 3:36 AM, Dmitry Baryshkov
> > <[email protected]> wrote:
> >
> > > On Fri, Mar 20, 2026 at 07:02:54AM +0000, Alexander Koskovich wrote:
> > > > On Friday, March 20th, 2026 at 2:50 AM, Alexander Koskovich
> > > > <[email protected]> wrote:
> > > >
> > > > > On Friday, March 20th, 2026 at 2:38 AM, Dmitry Baryshkov
> > > > > <[email protected]> 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
> > > > > > > <[email protected]> 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...
> > >
> > > Do you have any of the patches by Marijn or Pengyu?
> > >
> > > -
> > > https://lore.kernel.org/linux-arm-msm/20260311-dsi-dsc-regresses-again-v1-1-6a422141e...@somainline.org/
> > >
> > > -
> > > https://lore.kernel.org/linux-arm-msm/[email protected]/
> >
> > Nope, I can test with them though if you'd like
>
> Tested the following 3 patches on top of this series:
> drm/msm/dsi: fix hdisplay calculation when programming dsi registers
> drm/msm/dsi: fix bits_per_pclk
> drm/msm/dsi: fix hdisplay calculation for CMD mode panel
>
> Getting constant FIFO errors with them applied:
> [ 64.851635] dsi_err_worker: status=4
> [ 64.851692] dsi_err_worker: status=4
> [ 64.851730] dsi_err_worker: status=4
> [ 64.851766] dsi_err_worker: status=4
> [ 64.851802] dsi_err_worker: status=4
> [ 64.851837] dsi_err_worker: status=4
> [ 64.851903] dsi_err_worker: status=4
> [ 64.851940] dsi_err_worker: status=4
> [ 64.851976] dsi_err_worker: status=4
> [ 64.852011] dsi_err_worker: status=4
Personally, I use
timing->width = DIV_ROUND_UP(timing->width * drm_dsc_get_bpp_int(dsc),
dsc->bits_per_component * 3);
DIV_ROUND_UP is magic for me. If not, I got status=4 too.
This works for 8-bit dst bpc with 10-bit src bpc.
Best wishes,
Pengyu
